From 2965080cb7f33b621832d7b032916b5bcac9fb2a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E6=95=85=E7=92=83?= <gujieye.gjy@alibaba-inc.com>
Date: Tue, 11 Aug 2026 14:29:10 +0800
Subject: [PATCH] feat(skills): replace foreign skill dirs containing SKILL.md
 during fan-out

Previously, fan-out skipped any existing real directory not recorded in
the lock file, treating it as user content. This left stale skill copies
installed by other tools (e.g. npx skills add) permanently out of date.

Now: if the directory contains a SKILL.md, it is recognized as a skill
artifact and replaced with a symlink to the canonical dir. Directories
without SKILL.md are still preserved (user content safety boundary).
---
 packages/core/src/skills/agents.ts        | 20 ++++++++++
 packages/core/tests/skills-agents.test.ts | 45 +++++++++++++++++++----
 2 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/packages/core/src/skills/agents.ts b/packages/core/src/skills/agents.ts
index 1ddd369..16b2e8d 100644
--- a/packages/core/src/skills/agents.ts
+++ b/packages/core/src/skills/agents.ts
@@ -285,6 +285,21 @@ function isRecordedCopy(linkPath: string, recordedLinks: string[]): boolean {
   }
 }
 
+/**
+ * Whether linkPath is a real directory containing a SKILL.md — indicating it is a
+ * skill artifact installed by another tool (e.g. `npx skills add`) or an older
+ * version predating bl's symlink management. These are safe to replace: they are
+ * not arbitrary user content but the same kind of artifact we manage.
+ */
+function isForeignSkillDir(linkPath: string): boolean {
+  try {
+    if (!lstatSync(linkPath).isDirectory()) return false;
+    return existsSync(join(linkPath, "SKILL.md"));
+  } catch {
+    return false;
+  }
+}
+
 export interface LinkResult {
   agent: string;
   path: string;
@@ -326,6 +341,11 @@ export function linkSkillToAgents(
           // Copy-fallback artifact from a previous install → replace so updates
           // reach agents that have no symlink permission
           rmSync(linkPath, { recursive: true, force: true });
+        } else if (isForeignSkillDir(linkPath)) {
+          // A real directory containing SKILL.md — a skill installed by another
+          // tool (e.g. `npx skills add`) or predating bl's symlink management.
+          // Replace with our symlink so future updates propagate automatically.
+          rmSync(linkPath, { recursive: true, force: true });
         } else {
           results.push({
             agent: agent.id,
diff --git a/packages/core/tests/skills-agents.test.ts b/packages/core/tests/skills-agents.test.ts
index dc80ca3..7fa8125 100644
--- a/packages/core/tests/skills-agents.test.ts
+++ b/packages/core/tests/skills-agents.test.ts
@@ -293,21 +293,52 @@ test("agents: Amp-style XDG config dir lights up the universal-xdg shared target
   });
 });
 
-test("agents: recorded copy-fallback artifact is replaced; unrecorded dir stays skipped", async () => {
+test("agents: foreign skill dir (contains SKILL.md) is replaced even without lock record", async () => {
   await inFakeHome(async (home) => {
     mkdirSync(join(home, ".claude"), { recursive: true });
     const canonical = seedCanonicalSkill("demo");
     const copyPath = join(home, ".claude", "skills", "demo");
 
-    // Simulate a previous install that fell back to copy (no symlink permission, e.g. Windows)
+    // Simulate a skill installed by another tool (e.g. `npx skills add`) — a real
+    // directory containing SKILL.md, not recorded in our lock.
+    mkdirSync(copyPath, { recursive: true });
+    writeFileSync(join(copyPath, "SKILL.md"), "stale copy from another tool");
+
+    // New behavior: contains SKILL.md → recognized as a skill artifact → replaced
+    const result = linkSkillToAgents("demo");
+    expect(result[0]).toMatchObject({ agent: "claude-code", mode: "symlink" });
+    expect(lstatSync(copyPath).isSymbolicLink()).toBe(true);
+    expect(readlinkSync(copyPath)).toBe(canonical);
+  });
+});
+
+test("agents: foreign non-skill dir (no SKILL.md) stays skipped", async () => {
+  await inFakeHome(async (home) => {
+    mkdirSync(join(home, ".claude"), { recursive: true });
+    seedCanonicalSkill("demo");
+    const foreignPath = join(home, ".claude", "skills", "demo");
+
+    // A user's own directory that happens to share the skill name but has no SKILL.md
+    mkdirSync(foreignPath, { recursive: true });
+    writeFileSync(join(foreignPath, "my-notes.txt"), "user content");
+
+    const result = linkSkillToAgents("demo");
+    expect(result[0].mode).toBe("skipped");
+    // User content untouched
+    expect(readFileSync(join(foreignPath, "my-notes.txt"), "utf-8")).toBe("user content");
+  });
+});
+
+test("agents: recorded copy-fallback artifact is replaced with symlink", async () => {
+  await inFakeHome(async (home) => {
+    mkdirSync(join(home, ".claude"), { recursive: true });
+    const canonical = seedCanonicalSkill("demo");
+    const copyPath = join(home, ".claude", "skills", "demo");
+
+    // Simulate a previous install that fell back to copy (no symlink permission)
     mkdirSync(copyPath, { recursive: true });
     writeFileSync(join(copyPath, "SKILL.md"), "stale copy");
 
-    // Without a lock record the dir is foreign → skipped, content untouched
-    const unrecorded = linkSkillToAgents("demo");
-    expect(unrecorded[0].mode).toBe("skipped");
-    expect(readFileSync(join(copyPath, "SKILL.md"), "utf-8")).toBe("stale copy");
-
     // With the recorded link the artifact is rebuilt and points at canonical again
     const recorded = linkSkillToAgents("demo", detectInstalledAgents(), [copyPath]);
     expect(recorded[0]).toMatchObject({ agent: "claude-code", mode: "symlink" });