diff --git a/docs/commands.md b/docs/commands.md index 06dd1ca..8e8947c 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1145,6 +1145,11 @@ missing), and for every other supported host directory that already exists. any newly detected supported host that is missing it. - Local skills: agent-native local skills are not synchronized during startup. A local skill belongs to the agent skill directory where it was created. +- Shared host directories: supported hosts whose skill directories resolve to + the same location — for example `~/.claude/skills` symlinked to + `~/.agents/skills` — are one host. The skill is installed there once and is + reported under the more specific agent: the universal `~/.agents` host yields + to a detected agent that shares its directory. - Migration: startup synchronization does not rewrite same-version legacy symlink targets. Use `oo skills add` for bundled skills and `oo skills update` for registry skills to replace legacy symlinks explicitly. diff --git a/docs/commands.zh-CN.md b/docs/commands.zh-CN.md index 84ad9f9..a4b50ca 100644 --- a/docs/commands.zh-CN.md +++ b/docs/commands.zh-CN.md @@ -963,6 +963,10 @@ CLI 默认记录受隐私约束的命令使用 telemetry。事件不包含 free- 到且尚未安装它的受支持 Agent。 - 本地 skill：agent-native local skill 不会在启动时同步。本地 skill 归属于创建它 的 Agent skill 目录。 +- 共享 Agent 目录：如果多个受支持 Agent 的 skill 目录指向同一个位置（例如 + `~/.claude/skills` 软链接到 `~/.agents/skills`），它们会被视为同一个 host： + skill 只安装一次，并归属到更具体的 Agent，即通用 `~/.agents` host 让位于与 + 它共享目录的已检测 Agent。 - 迁移：启动同步不会改写同版本的历史软链接目标。请用 `oo skills add` 刷新内置 skill，用 `oo skills update` 刷新 registry skill，从而显式替换历史软链接。成功 的 `oo install` / `oo update` 会运行这两个维护步骤。 diff --git a/src/application/commands/skills/__tests__/helpers.ts b/src/application/commands/skills/__tests__/helpers.ts index 9505480..8354742 100644 --- a/src/application/commands/skills/__tests__/helpers.ts +++ b/src/application/commands/skills/__tests__/helpers.ts @@ -62,6 +62,20 @@ export async function readBundledSkillSourceContent( return await readBundledSkillFileContent(file); } +// Links an existing directory without touching its contents. Windows CI does +// not grant directory symlink privileges; junctions resolve through realpath() +// the same way. +export async function createDirectorySymbolicLinkForTest( + targetPath: string, + linkPath: string, +): Promise { + await symlink( + targetPath, + linkPath, + process.platform === "win32" ? "junction" : "dir", + ); +} + export async function createSymbolicLinkForTest( targetPath: string, linkPath: string, diff --git a/src/application/commands/skills/index.test.ts b/src/application/commands/skills/index.test.ts index a6fea4f..75fe241 100644 --- a/src/application/commands/skills/index.test.ts +++ b/src/application/commands/skills/index.test.ts @@ -17,7 +17,11 @@ import { parseTelemetryRowPayload, readTelemetryRowsForTest, } from "../../telemetry/outbox.ts"; -import { readBundledSkillSourceContent, seedRegistrySkill } from "./__tests__/helpers.ts"; +import { + createDirectorySymbolicLinkForTest, + readBundledSkillSourceContent, + seedRegistrySkill, +} from "./__tests__/helpers.ts"; import { canonicalLocalSkillsDirectoryName, resolveBundledSkillCanonicalDirectoryPath, @@ -348,39 +352,7 @@ describe("skills commands", () => { const result = await sandbox.run( ["skills", "install", "oo@0.0.2"], - { - fetcher: async (input, init) => { - const request = toRequest(input, init); - - requests.push(request); - - if (request.url.includes("/package-info/")) { - return new Response(JSON.stringify({ - packageName: "oo", - version: "0.0.2", - skills: [ - { - description: "Run OO workflows", - name: "runtime", - title: "Runtime", - }, - ], - })); - } - - if (request.url.endsWith("/oo/-/meta/oo-0.0.2.tgz")) { - return new Response(await createRegistrySkillArchiveBytes({ - "package/package/skills/runtime/SKILL.md": "# Runtime\n", - })); - } - - if (isRegistryPackageDownloadCountRequest(request)) { - return new Response(null, { status: 204 }); - } - - throw new Error(`Unexpected request: ${request.url}`); - }, - }, + { fetcher: createRuntimeRegistryFetcher(requests) }, ); expect(result.exitCode).toBe(0); @@ -399,6 +371,51 @@ describe("skills commands", () => { } }); + test("installs a registry skill once when two agent homes share a skills directory", async () => { + const sandbox = await createCliSandbox(); + const universalHomeDirectory = resolveManagedSkillAgentHomeDirectory(sandbox.env, "universal"); + const claudeHomeDirectory = resolveManagedSkillAgentHomeDirectory(sandbox.env, "claude"); + const sharedSkillsDirectoryPath = join(sandbox.env.HOME!, ".shared", "skills"); + const skillDirectoryPath = resolveManagedSkillDirectoryPath( + claudeHomeDirectory, + "runtime", + ); + + try { + await mkdir(sharedSkillsDirectoryPath, { recursive: true }); + await mkdir(universalHomeDirectory, { recursive: true }); + await mkdir(claudeHomeDirectory, { recursive: true }); + await createDirectorySymbolicLinkForTest( + sharedSkillsDirectoryPath, + join(universalHomeDirectory, "skills"), + ); + await createDirectorySymbolicLinkForTest( + sharedSkillsDirectoryPath, + join(claudeHomeDirectory, "skills"), + ); + await writeAuthFile(sandbox); + + const result = await sandbox.run( + ["skills", "install", "oo@0.0.2"], + { fetcher: createRuntimeRegistryFetcher() }, + ); + + // Both homes publish into the same directory: publishing them as two + // hosts copied into the same path concurrently and failed with EEXIST. + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toBe( + `Skill: runtime\nInstalled skill runtime to ${skillDirectoryPath}.\n`, + ); + expect( + await readFile(join(sharedSkillsDirectoryPath, "runtime", "SKILL.md"), "utf8"), + ).toContain("# Runtime"); + } + finally { + await sandbox.cleanup(); + } + }); + test("explicit bundled skill install overwrites a managed development-version installation", async () => { const sandbox = await createCliSandbox(); const universalHomeDirectory = resolveManagedSkillAgentHomeDirectory(sandbox.env, "universal"); @@ -3820,6 +3837,45 @@ function isRegistryPackageDownloadCountRequest(request: Request): boolean { && request.url.endsWith("/download-count"); } +// Serves the `oo@0.0.2` registry package that publishes a single `runtime` +// skill. Pass a request sink to assert the request sequence. +function createRuntimeRegistryFetcher(requests?: Request[]): ( + input: string | URL | Request, + init?: RequestInit, +) => Promise { + return async (input, init) => { + const request = toRequest(input, init); + + requests?.push(request); + + if (request.url.includes("/package-info/")) { + return new Response(JSON.stringify({ + packageName: "oo", + version: "0.0.2", + skills: [ + { + description: "Run OO workflows", + name: "runtime", + title: "Runtime", + }, + ], + })); + } + + if (request.url.endsWith("/oo/-/meta/oo-0.0.2.tgz")) { + return new Response(await createRegistrySkillArchiveBytes({ + "package/package/skills/runtime/SKILL.md": "# Runtime\n", + })); + } + + if (isRegistryPackageDownloadCountRequest(request)) { + return new Response(null, { status: 204 }); + } + + throw new Error(`Unexpected request: ${request.url}`); + }; +} + function createMultiPackageRegistryFetcher(): ( input: string | URL | Request, init?: RequestInit, diff --git a/src/application/commands/skills/managed-skill-hosts.test.ts b/src/application/commands/skills/managed-skill-hosts.test.ts index 5717c05..94c5c6a 100644 --- a/src/application/commands/skills/managed-skill-hosts.test.ts +++ b/src/application/commands/skills/managed-skill-hosts.test.ts @@ -4,32 +4,33 @@ import { join } from "node:path"; import { describe, expect, test } from "bun:test"; import { createTemporaryDirectory } from "../../../../__tests__/helpers.ts"; +import { createDirectorySymbolicLinkForTest } from "./__tests__/helpers.ts"; import { resolveAvailableManagedSkillHosts } from "./managed-skill-hosts.ts"; describe("resolveAvailableManagedSkillHosts", () => { test("always includes the universal host even when its home directory does not exist", async () => { - const rootDirectory = await createTemporaryDirectory("oo-managed-hosts"); - const env = { HOME: rootDirectory, USERPROFILE: rootDirectory }; + const hostSandbox = await createManagedSkillHostSandbox(); + const rootDirectory = hostSandbox.rootDirectory; try { - const hosts = await resolveAvailableManagedSkillHosts(env); + const hosts = await resolveAvailableManagedSkillHosts(hostSandbox.env); expect(hosts.map(host => host.agentName)).toEqual(["universal"]); expect(hosts[0]!.homeDirectory).toBe(join(rootDirectory, ".agents")); } finally { - await rm(rootDirectory, { force: true, recursive: true }); + await hostSandbox.cleanup(); } }); test("includes non-always-provision hosts only when their home directory exists", async () => { - const rootDirectory = await createTemporaryDirectory("oo-managed-hosts"); - const env = { HOME: rootDirectory, USERPROFILE: rootDirectory }; + const hostSandbox = await createManagedSkillHostSandbox(); + const rootDirectory = hostSandbox.rootDirectory; try { await mkdir(join(rootDirectory, ".claude"), { recursive: true }); - const hosts = await resolveAvailableManagedSkillHosts(env); + const hosts = await resolveAvailableManagedSkillHosts(hostSandbox.env); const agentNames = hosts.map(host => host.agentName); // universal is always provisioned; claude is included because its home @@ -39,7 +40,101 @@ describe("resolveAvailableManagedSkillHosts", () => { expect(agentNames).not.toContain("hermes"); } finally { - await rm(rootDirectory, { force: true, recursive: true }); + await hostSandbox.cleanup(); + } + }); + + test("collapses hosts whose skills directories resolve to the same path", async () => { + const hostSandbox = await createManagedSkillHostSandbox(); + const rootDirectory = hostSandbox.rootDirectory; + + try { + const sharedSkillsDirectory = join(rootDirectory, ".shared", "skills"); + + await mkdir(sharedSkillsDirectory, { recursive: true }); + await mkdir(join(rootDirectory, ".agents"), { recursive: true }); + await mkdir(join(rootDirectory, ".claude"), { recursive: true }); + await createDirectorySymbolicLinkForTest( + sharedSkillsDirectory, + join(rootDirectory, ".agents", "skills"), + ); + await createDirectorySymbolicLinkForTest( + sharedSkillsDirectory, + join(rootDirectory, ".claude", "skills"), + ); + + const hosts = await resolveAvailableManagedSkillHosts(hostSandbox.env); + + // Both homes publish into the same directory, so they are one host; + // the universal fallback yields to the concrete agent. + expect(hosts.map(host => host.agentName)).toEqual(["claude"]); + expect(hosts[0]!.homeDirectory).toBe(join(rootDirectory, ".claude")); + } + finally { + await hostSandbox.cleanup(); + } + }); + + test("keeps the first concrete agent when two concrete agents share a skills directory", async () => { + const hostSandbox = await createManagedSkillHostSandbox(); + const rootDirectory = hostSandbox.rootDirectory; + + try { + const claudeSkillsDirectory = join(rootDirectory, ".claude", "skills"); + + await mkdir(claudeSkillsDirectory, { recursive: true }); + await mkdir(join(rootDirectory, ".trae-cn"), { recursive: true }); + await createDirectorySymbolicLinkForTest( + claudeSkillsDirectory, + join(rootDirectory, ".trae-cn", "skills"), + ); + + const hosts = await resolveAvailableManagedSkillHosts(hostSandbox.env); + + // The universal host has its own (still missing) skills directory, so + // only the two aliased concrete agents collapse. + expect(hosts.map(host => host.agentName)).toEqual(["universal", "claude"]); + } + finally { + await hostSandbox.cleanup(); + } + }); + + test("collapses hosts whose home directories are aliased before any skills directory exists", async () => { + const hostSandbox = await createManagedSkillHostSandbox(); + const rootDirectory = hostSandbox.rootDirectory; + + try { + await mkdir(join(rootDirectory, ".agents"), { recursive: true }); + await createDirectorySymbolicLinkForTest( + join(rootDirectory, ".agents"), + join(rootDirectory, ".claude"), + ); + + const hosts = await resolveAvailableManagedSkillHosts(hostSandbox.env); + + expect(hosts.map(host => host.agentName)).toEqual(["claude"]); + } + finally { + await hostSandbox.cleanup(); } }); }); + +// Every host test needs an empty home directory tree plus the environment that +// points home resolution at it, and must remove the tree afterwards. +async function createManagedSkillHostSandbox(): Promise<{ + cleanup: () => Promise; + env: Record; + rootDirectory: string; +}> { + const rootDirectory = await createTemporaryDirectory("oo-managed-hosts"); + + return { + cleanup: async () => { + await rm(rootDirectory, { force: true, recursive: true }); + }, + env: { HOME: rootDirectory, USERPROFILE: rootDirectory }, + rootDirectory, + }; +} diff --git a/src/application/commands/skills/managed-skill-hosts.ts b/src/application/commands/skills/managed-skill-hosts.ts index a399331..32af086 100644 --- a/src/application/commands/skills/managed-skill-hosts.ts +++ b/src/application/commands/skills/managed-skill-hosts.ts @@ -1,5 +1,6 @@ import type { BundledSkillAgentName } from "./managed-skill-agents.ts"; +import { realpath } from "node:fs/promises"; import { CliUserError } from "../../contracts/cli.ts"; import { directoryExists } from "./bundled-skill-observation.ts"; import { @@ -7,7 +8,10 @@ import { readManagedSkillAgent, resolveManagedSkillAgentHomeDirectory, } from "./managed-skill-agents.ts"; -import { resolveManagedSkillDirectoryPath } from "./managed-skill-paths.ts"; +import { + resolveManagedSkillDirectoryPath, + resolveManagedSkillsDirectoryPath, +} from "./managed-skill-paths.ts"; export interface ManagedSkillHost { agentName: BundledSkillAgentName; @@ -43,7 +47,86 @@ export async function resolveAvailableManagedSkillHosts( }), ); - return hosts.filter(host => host !== undefined); + return collapseAliasedManagedSkillHosts( + hosts.filter(host => host !== undefined), + ); +} + +// Several supported agents can share one physical skills directory: a +// symlinked `~/.claude/skills -> ~/.agents/skills`, or two `*_HOME` overrides +// pointing at the same place. Such hosts are a single install target, and +// treating them as separate ones publishes the same skill into the same +// directory twice — concurrently, so one copy fails with EEXIST while the +// other is still writing. Hosts are therefore collapsed by the real path of +// their skills directory. The always-provisioned universal host yields to a +// concrete agent sharing its directory, because skill content is rendered per +// agent and the concrete agent's rendering is the more specific one; otherwise +// the first host in agent declaration order is kept. +async function collapseAliasedManagedSkillHosts( + hosts: readonly ManagedSkillHost[], +): Promise { + const identifiedHosts = await Promise.all( + hosts.map(async host => ({ + host, + skillsDirectoryIdentity: await resolveManagedSkillsDirectoryIdentity( + host.homeDirectory, + ), + })), + ); + const hostsByIdentity = new Map(); + + for (const identifiedHost of identifiedHosts) { + const collapsedHost = hostsByIdentity.get( + identifiedHost.skillsDirectoryIdentity, + ); + + if ( + collapsedHost !== undefined + && readManagedSkillAgent(collapsedHost.agentName).alwaysProvision !== true + ) { + continue; + } + + hostsByIdentity.set( + identifiedHost.skillsDirectoryIdentity, + identifiedHost.host, + ); + } + + return [...hostsByIdentity.values()]; +} + +// A host is identified by the real path of the directory its skills are +// published into. The skills directory is created on demand, so a host that +// does not have one yet falls back to its resolved home directory, which still +// collapses an aliased home before its first publication. A path that cannot be +// resolved at all is its own identity, which simply leaves the host uncollapsed. +async function resolveManagedSkillsDirectoryIdentity( + homeDirectory: string, +): Promise { + const skillsDirectoryPath = resolveManagedSkillsDirectoryPath(homeDirectory); + const resolvedSkillsDirectoryPath = await readRealPath(skillsDirectoryPath); + + if (resolvedSkillsDirectoryPath !== undefined) { + return resolvedSkillsDirectoryPath; + } + + const resolvedHomeDirectory = await readRealPath(homeDirectory); + + return resolvedHomeDirectory === undefined + ? skillsDirectoryPath + : resolveManagedSkillsDirectoryPath(resolvedHomeDirectory); +} + +async function readRealPath(path: string): Promise { + try { + return await realpath(path); + } + catch { + // Missing, inaccessible, or otherwise unresolvable paths carry no + // identity to compare; the caller falls back to the literal path. + return undefined; + } } export function resolveManagedSkillHostInstallation(