ref(cli): Centralize skill symlink targets (#128)

gricha · codex · web-flow · commit daa3a13bbe9c · 2026-07-11T12:24:45.000-04:00
Move repeated scope, agent, and legacy target enumeration into a shared pure
projection. Keep creation, repair, reporting, and output policy in their
commands while preserving ordering, exact-string deduplication, and native
skill readers.

Verify sync repairs the projected legacy path and cover project and user scope
semantics.

Co-authored-by: OpenAI Codex &lt;noreply@openai.com&gt;
diff --git a/packages/dotagents/src/cli/commands/doctor.test.ts b/packages/dotagents/src/cli/commands/doctor.test.ts
@@ -102,6 +102,20 @@ describe("runDoctor", () => {
     expect(check?.message).toContain("pdf");
   });
 
+  it("detects a missing agent skill symlink", async () => {
+    await writeFile(
+      join(projectRoot, "agents.toml"),
+      `version = 1\nagents = ["claude", "cursor", "codex"]\n`,
+    );
+    await writeFile(join(projectRoot, ".gitignore"), "agents.lock\n.agents/.gitignore\n");
+    await writeFile(join(projectRoot, ".agents", ".gitignore"), "# managed\n");
+
+    const result = await runDoctor({ scope: resolveScope("project", projectRoot) });
+    const check = result.checks.find((c) => c.name === "symlinks");
+    expect(check?.status).toBe("warn");
+    expect(check?.message).toContain("1 symlink(s)");
+  });
+
   it("detects generated files tracked by git", async () => {
     // Initialize a git repo so git ls-files works
     const { execSync } = await import("node:child_process");
diff --git a/packages/dotagents/src/cli/commands/doctor.ts b/packages/dotagents/src/cli/commands/doctor.ts
@@ -10,7 +10,7 @@ import { isWildcardDep } from "../../config/schema.js";
 import { loadLockfile } from "../../lockfile/loader.js";
 import { writeLockfile } from "../../lockfile/writer.js";
 import { verifySymlinks } from "../../symlinks/manager.js";
-import { getAgent } from "../../targets/registry.js";
+import { skillSymlinkTargets } from "../../targets/skill-symlinks.js";
 import { resolveScope, resolveDefaultScope, ScopeError, type ScopeRoot } from "../../scope.js";
 import { exec } from "@sentry/dotagents-lib";
 import { isInPlaceSkill } from "../../utils/fs.js";
@@ -194,19 +194,11 @@ export async function runDoctor(opts: DoctorOptions): Promise<DoctorResult> {
 
   // 9. Symlinks (project scope only)
   if (scope.scope === "project" && existsSync(scope.agentsDir)) {
-    const targets: string[] = [];
-    const seenDirs = new Set<string>();
-
-    for (const target of config.symlinks?.targets ?? []) {
-      seenDirs.add(target);
-      targets.push(`${scope.root}/${target}`);
-    }
-    for (const agentId of config.agents) {
-      const agent = getAgent(agentId);
-      if (!agent?.skillsParentDir || seenDirs.has(agent.skillsParentDir)) {continue;}
-      seenDirs.add(agent.skillsParentDir);
-      targets.push(`${scope.root}/${agent.skillsParentDir}`);
-    }
+    const targets = skillSymlinkTargets(
+      scope,
+      config.agents,
+      config.symlinks?.targets,
+    );
 
     if (targets.length > 0) {
       const issues = await verifySymlinks(scope.agentsDir, targets);
diff --git a/packages/dotagents/src/cli/commands/init.ts b/packages/dotagents/src/cli/commands/init.ts
@@ -1,12 +1,13 @@
 import { existsSync } from "node:fs";
 import { chmod, mkdir, readFile, writeFile } from "node:fs/promises";
-import { join, resolve } from "node:path";
+import { join, relative, resolve } from "node:path";
 import chalk from "chalk";
 import { generateDefaultConfig } from "../../config/writer.js";
 import { writeAgentsGitignore, ensureRootGitignoreEntries } from "../../gitignore/writer.js";
 import { ensureSkillsSymlink } from "../../symlinks/manager.js";
 import { loadConfig } from "../../config/loader.js";
-import { getAgent, allAgentIds, allAgents } from "../../targets/registry.js";
+import { allAgentIds, allAgents } from "../../targets/registry.js";
+import { skillSymlinkTargets } from "../../targets/skill-symlinks.js";
 import { parseArgs } from "node:util";
 import * as clack from "@clack/prompts";
 import { resolveScope, isInsideGitRepo, findGitDir, type ScopeRoot } from "../../scope.js";
@@ -71,36 +72,17 @@ export async function runInit(opts: InitOptions): Promise<void> {
   // Symlinks — create per-agent symlinks so each agent discovers skills
   const symlinkResults: { target: string; created: boolean; migrated: string[] }[] = [];
 
-  if (scope.scope === "user") {
-    const seen = new Set<string>();
-    for (const agentId of config.agents) {
-      const agent = getAgent(agentId);
-      if (!agent?.userSkillsParentDirs) {continue;}
-      for (const dir of agent.userSkillsParentDirs) {
-        if (seen.has(dir)) {continue;}
-        seen.add(dir);
-        const result = await ensureSkillsSymlink(agentsDir, dir);
-        symlinkResults.push({ target: dir, ...result });
-      }
-    }
-  } else {
-    const targets = config.symlinks?.targets ?? [];
-    for (const target of targets) {
-      const targetDir = join(scope.root, target);
-      const result = await ensureSkillsSymlink(agentsDir, targetDir);
-      symlinkResults.push({ target, ...result });
-    }
-
-    const seenParentDirs = new Set(targets);
-    for (const agentId of config.agents) {
-      const agent = getAgent(agentId);
-      if (!agent?.skillsParentDir) {continue;}
-      if (seenParentDirs.has(agent.skillsParentDir)) {continue;}
-      seenParentDirs.add(agent.skillsParentDir);
-      const targetDir = join(scope.root, agent.skillsParentDir);
-      const result = await ensureSkillsSymlink(agentsDir, targetDir);
-      symlinkResults.push({ target: agent.skillsParentDir, ...result });
-    }
+  const symlinkTargets = skillSymlinkTargets(
+    scope,
+    config.agents,
+    config.symlinks?.targets,
+  );
+  for (const target of symlinkTargets) {
+    const result = await ensureSkillsSymlink(agentsDir, target);
+    const displayTarget = scope.scope === "project"
+      ? relative(scope.root, target)
+      : target;
+    symlinkResults.push({ target: displayTarget, ...result });
   }
 
   // Auto-install declared skills (best-effort — may fail offline)
diff --git a/packages/dotagents/src/cli/commands/install/agent-runtime.ts b/packages/dotagents/src/cli/commands/install/agent-runtime.ts
@@ -1,8 +1,7 @@
-import { join } from "node:path";
 import type { AgentsConfig } from "../../../config/schema.js";
 import type { ScopeRoot } from "../../../scope.js";
-import { getAgent } from "../../../targets/registry.js";
 import { ensureSkillsSymlink } from "../../../symlinks/manager.js";
+import { skillSymlinkTargets } from "../../../targets/skill-symlinks.js";
 import { projectMcpResolver, toMcpDeclarations, writeMcpConfigs } from "../../../targets/mcp-writer.js";
 import { projectHookResolver, toHookDeclarations, writeHookConfigs } from "../../../targets/hook-writer.js";
 import { userMcpResolver } from "../../../targets/paths.js";
@@ -19,32 +18,13 @@ export async function writeSkillSymlinks(
   config: AgentsConfig,
   scope: ScopeRoot,
 ): Promise<void> {
-  if (scope.scope === "user") {
-    const seen = new Set<string>();
-    for (const agentId of config.agents) {
-      const agent = getAgent(agentId);
-      if (!agent?.userSkillsParentDirs) {continue;}
-      for (const dir of agent.userSkillsParentDirs) {
-        if (seen.has(dir)) {continue;}
-        seen.add(dir);
-        await ensureSkillsSymlink(scope.agentsDir, dir);
-      }
-    }
-    return;
-  }
-
-  const targets = config.symlinks?.targets ?? [];
+  const targets = skillSymlinkTargets(
+    scope,
+    config.agents,
+    config.symlinks?.targets,
+  );
   for (const target of targets) {
-    await ensureSkillsSymlink(scope.agentsDir, join(scope.root, target));
-  }
-
-  const seenParentDirs = new Set(targets);
-  for (const agentId of config.agents) {
-    const agent = getAgent(agentId);
-    if (!agent?.skillsParentDir) {continue;}
-    if (seenParentDirs.has(agent.skillsParentDir)) {continue;}
-    seenParentDirs.add(agent.skillsParentDir);
-    await ensureSkillsSymlink(scope.agentsDir, join(scope.root, agent.skillsParentDir));
+    await ensureSkillsSymlink(scope.agentsDir, target);
   }
 }
 
diff --git a/packages/dotagents/src/cli/commands/sync.test.ts b/packages/dotagents/src/cli/commands/sync.test.ts
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, afterEach } from "vitest";
-import { mkdtemp, mkdir, readFile, writeFile, rm } from "node:fs/promises";
+import { lstat, mkdtemp, mkdir, readFile, writeFile, rm } from "node:fs/promises";
 import { existsSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
@@ -344,6 +344,7 @@ describe("runSync", () => {
 
     const result = await runSync({ scope: resolveScope("project", projectRoot) });
     expect(result.symlinksRepaired).toBe(1);
+    expect((await lstat(join(projectRoot, ".claude", "skills"))).isSymbolicLink()).toBe(true);
   });
 
   it("regenerates gitignore", async () => {
@@ -386,6 +387,7 @@ describe("runSync", () => {
 
     const result = await runSync({ scope: resolveScope("project", projectRoot) });
     expect(result.symlinksRepaired).toBe(1);
+    expect((await lstat(join(projectRoot, ".claude", "skills"))).isSymbolicLink()).toBe(true);
   });
 
   it("repairs missing hook configs", async () => {
diff --git a/packages/dotagents/src/cli/commands/sync.ts b/packages/dotagents/src/cli/commands/sync.ts
@@ -10,7 +10,7 @@ import { writeLockfile } from "../../lockfile/writer.js";
 import { addSkillToConfig } from "../../config/writer.js";
 import { writeAgentsGitignore, checkRootGitignoreEntries } from "../../gitignore/writer.js";
 import { ensureSkillsSymlink, verifySymlinks } from "../../symlinks/manager.js";
-import { getAgent } from "../../targets/registry.js";
+import { skillSymlinkTargets } from "../../targets/skill-symlinks.js";
 import { verifyMcpConfigs, writeMcpConfigs, toMcpDeclarations, projectMcpResolver } from "../../targets/mcp-writer.js";
 import { verifyHookConfigs, writeHookConfigs, toHookDeclarations, projectHookResolver } from "../../targets/hook-writer.js";
 import { pruneSubagentConfigs, verifySubagentConfigs, writeSubagentConfigs, projectSubagentResolver, userSubagentResolver } from "../../subagents/writer.js";
@@ -164,51 +164,15 @@ export async function runSync(opts: SyncOptions): Promise<SyncResult> {
 
   // 4. Verify and repair symlinks
   let symlinksRepaired = 0;
-
-  if (scope.scope === "user") {
-    const seen = new Set<string>();
-    const targets: string[] = [];
-    for (const agentId of config.agents) {
-      const agent = getAgent(agentId);
-      if (!agent?.userSkillsParentDirs) {continue;}
-      for (const dir of agent.userSkillsParentDirs) {
-        if (seen.has(dir)) {continue;}
-        seen.add(dir);
-        targets.push(dir);
-      }
-    }
-
-    const symlinkIssues = await verifySymlinks(agentsDir, targets);
-    for (const issue of symlinkIssues) {
-      await ensureSkillsSymlink(agentsDir, issue.target);
-      symlinksRepaired++;
-    }
-  } else {
-    const legacyTargets = config.symlinks?.targets ?? [];
-    const legacyIssues = await verifySymlinks(
-      agentsDir,
-      legacyTargets.map((t) => join(scope.root, t)),
-    );
-    for (const issue of legacyIssues) {
-      await ensureSkillsSymlink(agentsDir, join(scope.root, issue.target));
-      symlinksRepaired++;
-    }
-
-    const seenParentDirs = new Set(legacyTargets);
-    const agentTargets: string[] = [];
-    for (const agentId of config.agents) {
-      const agent = getAgent(agentId);
-      if (!agent?.skillsParentDir) {continue;}
-      if (seenParentDirs.has(agent.skillsParentDir)) {continue;}
-      seenParentDirs.add(agent.skillsParentDir);
-      agentTargets.push(join(scope.root, agent.skillsParentDir));
-    }
-
-    const agentSymlinkIssues = await verifySymlinks(agentsDir, agentTargets);
-    for (const issue of agentSymlinkIssues) {
-      await ensureSkillsSymlink(agentsDir, issue.target);
-      symlinksRepaired++;
-    }
+  const symlinkTargets = skillSymlinkTargets(
+    scope,
+    config.agents,
+    config.symlinks?.targets,
+  );
+  const symlinkIssues = await verifySymlinks(agentsDir, symlinkTargets);
+  for (const issue of symlinkIssues) {
+    await ensureSkillsSymlink(agentsDir, issue.target);
+    symlinksRepaired++;
   }
 
   // 5. Verify and repair MCP configs
diff --git a/packages/dotagents/src/targets/skill-symlinks.test.ts b/packages/dotagents/src/targets/skill-symlinks.test.ts
@@ -0,0 +1,45 @@
+import { homedir } from "node:os";
+import { join, resolve } from "node:path";
+import { describe, expect, it } from "vitest";
+import { resolveScope } from "../scope.js";
+import { skillSymlinkTargets } from "./skill-symlinks.js";
+
+describe("skillSymlinkTargets", () => {
+  it("returns legacy targets before deduplicated agent targets", () => {
+    const scope = resolveScope("project", "/workspace/project");
+
+    expect(
+      skillSymlinkTargets(
+        scope,
+        ["claude", "cursor", "codex"],
+        [".legacy", ".claude", ".legacy"],
+      ),
+    ).toEqual([
+      join(scope.root, ".legacy"),
+      join(scope.root, ".claude"),
+    ]);
+  });
+
+  it("returns deduplicated user targets and skips native readers", () => {
+    const scope = resolveScope("user");
+
+    expect(
+      skillSymlinkTargets(
+        scope,
+        ["claude", "cursor", "codex", "vscode", "opencode"],
+        [".legacy"],
+      ),
+    ).toEqual([join(homedir(), ".claude")]);
+  });
+
+  it("returns absolute project targets for a relative scope root", () => {
+    const scope = resolveScope("project", "relative/project");
+
+    expect(
+      skillSymlinkTargets(scope, ["claude"], ["/legacy"]),
+    ).toEqual([
+      resolve(join("relative/project", "/legacy")),
+      resolve(join("relative/project", ".claude")),
+    ]);
+  });
+});
diff --git a/packages/dotagents/src/targets/skill-symlinks.ts b/packages/dotagents/src/targets/skill-symlinks.ts
@@ -0,0 +1,37 @@
+import { join, resolve } from "node:path";
+import type { ScopeRoot } from "../scope.js";
+import { getAgent } from "./registry.js";
+
+/** Owns the shared projection of configured agents and legacy entries to absolute symlink targets. */
+export function skillSymlinkTargets(
+  scope: ScopeRoot,
+  agentIds: readonly string[],
+  legacyTargets: readonly string[] = [],
+): string[] {
+  const seen = new Set<string>();
+  const targets: string[] = [];
+
+  if (scope.scope === "project") {
+    for (const target of legacyTargets) {
+      if (seen.has(target)) {continue;}
+      seen.add(target);
+      targets.push(target);
+    }
+    for (const agentId of agentIds) {
+      const target = getAgent(agentId)?.skillsParentDir;
+      if (!target || seen.has(target)) {continue;}
+      seen.add(target);
+      targets.push(target);
+    }
+    return targets.map((target) => resolve(join(scope.root, target)));
+  }
+
+  for (const agentId of agentIds) {
+    for (const target of getAgent(agentId)?.userSkillsParentDirs ?? []) {
+      if (seen.has(target)) {continue;}
+      seen.add(target);
+      targets.push(target);
+    }
+  }
+  return targets;
+}
