fix(skill): align remove dry-run links with real unlink scan

Author: hopelynd

packages/commands/src/commands/skill/remove.ts

@@ -5,6 +5,7 @@ import {
   getSkillsDir,
   listSkillDirsOnDisk,
   parseSkillNames,
+  planUnlinkSkillFromAgents,
   readSkillLock,
   removeSkillDir,
   unlinkSkillFromAgents,
@@ -79,7 +80,8 @@ export default defineCommand({
           name,
           status: "remove",
           canonical: join(skillsDir, name),
-          links: locked.links ?? [],
+          // 与真实 unlink 同一候选集：含 lock 外的历史托管 symlink
+          links: planUnlinkSkillFromAgents(name, locked.links ?? []),
         };
       });
 

packages/commands/tests/e2e/skill.e2e.test.ts

@@ -1,4 +1,4 @@
-import { existsSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, mkdtempSync, symlinkSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { describe, expect, test } from "vite-plus/test";
@@ -142,10 +142,14 @@ describe("e2e: skill (local, no credentials)", () => {
   test("skill remove --dry-run 仅输出计划且不删盘", async () => {
     const configDir = makeTempConfigDir();
     const fakeHome = makeTempConfigDir();
-    const linkPath = join(fakeHome, ".claude", "skills", "seeded-skill");
-    seedInstalledSkill(configDir, "seeded-skill", [linkPath]);
+    const recordedLink = join(fakeHome, ".claude", "skills", "seeded-skill");
+    const historicalLink = join(fakeHome, ".agents", "skills", "seeded-skill");
+    seedInstalledSkill(configDir, "seeded-skill", [recordedLink]);
     mkdirSync(join(fakeHome, ".claude", "skills"), { recursive: true });
-    writeFileSync(linkPath, "link-placeholder");
+    mkdirSync(join(fakeHome, ".agents", "skills"), { recursive: true });
+    // recorded：普通占位文件；historical：指向 canonical 的托管 symlink（不在 lock 里）
+    writeFileSync(recordedLink, "link-placeholder");
+    symlinkSync(join(configDir, "skills", "seeded-skill"), historicalLink, "dir");
 
     const { stdout, stderr, exitCode } = await runCommandE2e(
       SKILL_ROUTES,
@@ -166,10 +170,11 @@ describe("e2e: skill (local, no credentials)", () => {
     expect(data.skills?.[0]?.status).toBe("remove");
     expect(data.skills?.[0]?.name).toBe("seeded-skill");
     expect(data.skills?.[0]?.canonical).toBe(join(configDir, "skills", "seeded-skill"));
-    expect(data.skills?.[0]?.links).toEqual([linkPath]);
+    expect(data.skills?.[0]?.links?.sort()).toEqual([recordedLink, historicalLink].sort());
     expect(existsSync(join(configDir, "skills", "seeded-skill", "SKILL.md"))).toBe(true);
     expect(existsSync(join(configDir, "skills", "skill-lock.json"))).toBe(true);
-    expect(existsSync(linkPath)).toBe(true);
+    expect(existsSync(recordedLink)).toBe(true);
+    expect(existsSync(historicalLink)).toBe(true);
   });
 
   test("skill update --dry-run 未安装时退出码与真实一致 (1)", async () => {

packages/core/src/skills/agents.ts

@@ -423,13 +423,13 @@ export function fanOutSkillToAgents(
 }
 
 /**
- * Reclaim fan-out artifacts for a skill across all agent dirs.
- * Symlinks pointing to canonical are removed (including historical links not in lock,
- * via defensive scan of the full registry); real directories are only removed if recorded
- * in lock (copy-fallback artifacts). A single failure does not block the rest.
+ * Collect paths that unlinkSkillFromAgents would reclaim (read-only).
+ * Same candidate set and eligibility rules as the mutating unlink: managed
+ * symlinks anywhere under the agent registry (including historical links not
+ * in lock), plus recorded copy-fallback directories.
  */
-export function unlinkSkillFromAgents(name: string, recordedLinks: string[] = []): string[] {
-  const removed: string[] = [];
+export function planUnlinkSkillFromAgents(name: string, recordedLinks: string[] = []): string[] {
+  const planned: string[] = [];
   const candidates = new Set(recordedLinks);
   for (const agent of getAgentTargets()) candidates.add(join(agent.skillsDir, name));
   for (const linkPath of candidates) {
@@ -441,14 +441,34 @@ export function unlinkSkillFromAgents(name: string, recordedLinks: string[] = []
         continue;
       }
       if (stat.isSymbolicLink()) {
-        if (isManagedLink(linkPath)) {
-          rmSync(linkPath);
-          removed.push(linkPath);
-        }
+        if (isManagedLink(linkPath)) planned.push(linkPath);
       } else if (recordedLinks.some((recorded) => samePath(recorded, linkPath))) {
+        planned.push(linkPath);
+      }
+    } catch {
+      /* single failure does not block remaining scan */
+    }
+  }
+  return planned;
+}
+
+/**
+ * Reclaim fan-out artifacts for a skill across all agent dirs.
+ * Symlinks pointing to canonical are removed (including historical links not in lock,
+ * via defensive scan of the full registry); real directories are only removed if recorded
+ * in lock (copy-fallback artifacts). A single failure does not block the rest.
+ */
+export function unlinkSkillFromAgents(name: string, recordedLinks: string[] = []): string[] {
+  const removed: string[] = [];
+  for (const linkPath of planUnlinkSkillFromAgents(name, recordedLinks)) {
+    try {
+      const stat = lstatSync(linkPath);
+      if (stat.isSymbolicLink()) {
+        rmSync(linkPath);
+      } else {
         rmSync(linkPath, { recursive: true, force: true });
-        removed.push(linkPath);
       }
+      removed.push(linkPath);
     } catch {
       /* single failure does not block remaining cleanup */
     }

packages/core/src/skills/index.ts

@@ -30,6 +30,7 @@ export {
   detectInstalledAgents,
   linkSkillToAgents,
   fanOutSkillToAgents,
+  planUnlinkSkillFromAgents,
   unlinkSkillFromAgents,
   type AgentTarget,
   type LinkResult,

packages/core/tests/skills-agents.test.ts

@@ -16,6 +16,7 @@ import {
   fanOutSkillToAgents,
   getAgentTargets,
   linkSkillToAgents,
+  planUnlinkSkillFromAgents,
   unlinkSkillFromAgents,
 } from "../src/skills/agents.ts";
 import { getSkillsDir } from "../src/skills/lock.ts";
@@ -250,6 +251,31 @@ test("agents: unlink reclaims managed links, leaves foreign content untouched",
   });
 });
 
+test("agents: planUnlink includes historical managed symlinks missing from lock", async () => {
+  await inFakeHome(async (home) => {
+    mkdirSync(join(home, ".claude"), { recursive: true });
+    mkdirSync(join(home, ".agents"), { recursive: true });
+    const canonical = seedCanonicalSkill("demo");
+    const allLinks = linkSkillToAgents("demo")
+      .filter((link) => link.mode === "symlink")
+      .map((link) => link.path);
+    expect(allLinks.length).toBeGreaterThanOrEqual(2);
+
+    // Lock only recorded one path; the rest are historical managed symlinks
+    const recordedOnly = [allLinks[0]!];
+    const planned = planUnlinkSkillFromAgents("demo", recordedOnly);
+    expect(planned.sort()).toEqual([...allLinks].sort());
+
+    // plan 与真实 unlink 范围一致
+    const removed = unlinkSkillFromAgents("demo", recordedOnly);
+    expect(removed.sort()).toEqual(planned.sort());
+    expect(existsSync(canonical)).toBe(true);
+    for (const linkPath of allLinks) {
+      expect(existsSync(linkPath)).toBe(false);
+    }
+  });
+});
+
 test("agents: official config-dir env vars relocate detection and fan-out", async () => {
   await inFakeHome(async (home) => {
     const customClaude = join(home, "relocated-claude");