Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 63 additions & 1 deletion src/add.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { execFileSync } from 'child_process';
import { existsSync, rmSync, mkdirSync, writeFileSync, lstatSync, readFileSync } from 'fs';
import {
existsSync,
rmSync,
mkdirSync,
writeFileSync,
lstatSync,
readFileSync,
symlinkSync,
} from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import { runCli, stripAnsi } from './test-utils.ts';
Expand Down Expand Up @@ -332,6 +340,60 @@ description: Shared install path regression test
expect(countPathLinesForSkill(result.stdout, 'shared-skill')).toBe(1);
});

// Regression test for #2098: installing into a global skills dir reached
// through a symlinked ~/.agents (e.g. moved to another drive/location and
// linked back) must succeed, including when several selected agents all
// resolve to that same shared universal directory.
it('installs global skills when ~/.agents is a symlinked directory', () => {
const sourceDir = join(testDir, 'source');
const skillDir = join(sourceDir, 'skills', 'symlinked-home-skill');
mkdirSync(skillDir, { recursive: true });
writeFileSync(
join(skillDir, 'SKILL.md'),
`---
name: symlinked-home-skill
description: Symlinked home directory regression test
---

# Symlinked Home Skill
`
);

const homeDir = join(testDir, 'home');
const realAgentsHome = join(testDir, 'actual-agents-home');
mkdirSync(homeDir, { recursive: true });
mkdirSync(realAgentsHome, { recursive: true });
symlinkSync(realAgentsHome, join(homeDir, '.agents'));

const result = runCli(
[
'add',
sourceDir,
'-y',
'-g',
'--copy',
'--agent',
'codex',
'cursor',
'cline',
'warp',
'zed',
],
testDir,
{ HOME: homeDir, USERPROFILE: homeDir }
);

expect(result.exitCode).toBe(0);
expect(result.stdout).not.toContain('Failed to install');
expect(result.stdout).toContain('Installed 1 skill');
expect(result.stdout).toContain('✓ symlinked-home-skill (copied)');

// Content must land at the real (symlink-resolved) location.
const installedSkillMd = join(realAgentsHome, 'skills', 'symlinked-home-skill', 'SKILL.md');
expect(existsSync(installedSkillMd)).toBe(true);
expect(readFileSync(installedSkillMd, 'utf-8')).toContain('name: symlinked-home-skill');
});

it('preserves distinct copied install paths when --copy targets different agent directories', () => {
const sourceDir = join(testDir, 'source');
const skillDir = join(sourceDir, 'skills', 'multi-target-skill');
Expand Down
86 changes: 81 additions & 5 deletions src/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
installBlobSkillForAgent,
isSkillInstalled,
getCanonicalPath,
getInstallPath,
installWellKnownSkillForAgent,
type InstallMode,
} from './installer.ts';
Expand Down Expand Up @@ -959,12 +960,46 @@ async function handleWellKnownSkills(
error?: string;
}[] = [];

// See the writeCache comment in installToTargets: multiple targets can resolve
// to the same physical directory, so install once per resolved directory and
// reuse the result for the rest.
const writeCache = new Map<
string,
{
path: string;
canonicalPath?: string;
mode: InstallMode;
symlinkFailed?: boolean;
skipped?: boolean;
}
>();

for (const skill of selectedSkills) {
for (const agent of targetAgents) {
const result = await installWellKnownSkillForAgent(skill, agent, {
global: installGlobally,
mode: installMode,
});
const writeKey =
installMode === 'copy'
? getInstallPath(skill.installName, agent, { global: installGlobally })
: getCanonicalPath(skill.installName, { global: installGlobally, agent });

let result;
const cached = writeCache.get(writeKey);
if (cached) {
result = { success: true as const, ...cached };
} else {
result = await installWellKnownSkillForAgent(skill, agent, {
global: installGlobally,
mode: installMode,
});
if (result.success) {
writeCache.set(writeKey, {
path: result.path,
canonicalPath: result.canonicalPath,
mode: result.mode,
symlinkFailed: result.symlinkFailed,
skipped: result.skipped,
});
}
}
results.push({
skill: skill.installName,
agent: agents[agent].displayName,
Expand Down Expand Up @@ -1241,6 +1276,24 @@ async function installToTargets(
}
): Promise<TargetInstallResult[]> {
const results: TargetInstallResult[] = [];

// Multiple selected targets can resolve to the identical physical directory
// (e.g. several universal agents all writing to ~/.agents/skills). Without this
// cache, each target redundantly wipes and rewrites that shared directory in a
// tight loop, which is at best wasteful and, when an ancestor directory is a
// symlink to a location watched by sync software, can fail outright. Install
// once per resolved directory and reuse the result for the rest.
const writeCache = new Map<
string,
{
path: string;
canonicalPath?: string;
mode: InstallMode;
symlinkFailed?: boolean;
skipped?: boolean;
}
>();

for (const skill of skills) {
for (const target of targets) {
const { agent, subagent } = target;
Expand All @@ -1250,8 +1303,22 @@ async function installToTargets(
eveSubagent: subagent,
createMissingAgentRoot: options.createMissingAgentRoot(agent),
};
const isBlob = resolved.blobResult && 'files' in skill;
const installName = isBlob ? (skill as BlobSkill).name : skill.name;
const writeKey =
options.mode === 'copy'
? getInstallPath(installName, agent, { global: options.global, eveSubagent: subagent })
: getCanonicalPath(installName, {
global: options.global,
agent,
eveSubagent: subagent,
});

let result;
if (resolved.blobResult && 'files' in skill) {
const cached = writeCache.get(writeKey);
if (cached) {
result = { success: true as const, ...cached };
} else if (isBlob) {
// Blob-based install: write files from snapshot
const blobSkill = skill as BlobSkill;
result = await installBlobSkillForAgent(
Expand All @@ -1267,6 +1334,15 @@ async function installToTargets(
// assets/, etc. are installed too. See issue #1603.
result = await installSkillForAgent(skill, agent, installOptions);
}
if (!cached && result.success) {
writeCache.set(writeKey, {
path: result.path,
canonicalPath: result.canonicalPath,
mode: result.mode,
symlinkFailed: result.symlinkFailed,
skipped: result.skipped,
});
}
results.push({
skill: getSkillDisplayName(skill),
agent: targetDisplayName(target),
Expand Down