fix: reclaim skill links from a moved install, and stop setup failing in silence (0.6.4)

.claude-plugin/marketplace.json

@@ -10,7 +10,7 @@
       "name": "agent-memory",
       "source": "./",
       "description": "Durable cross-repository memory for coding agents — built to survive an IT security review. Zero runtime dependencies, zero dev dependencies, no install script: nothing runs when you install it, and granting it your agents is a separate explicit command. Independently scanned, with a passing verdict. Adds /handoff, /remember and /recall over one local markdown store. Requires the CLI: npm install -g @vib795/agent-memory (Node >= 22.5).",
-      "version": "0.6.3",
+      "version": "0.6.4",
       "author": {
         "name": "Utkarsh Singh",
         "url": "https://github.com/vib795"

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "agent-memory",
   "displayName": "agent-memory",
-  "version": "0.6.3",
+  "version": "0.6.4",
   "description": "Durable cross-repository memory for coding agents — built to survive an IT security review. Zero runtime dependencies, zero dev dependencies, no install script: nothing runs when you install it, and granting it your agents is a separate explicit command. Independently scanned, with a passing verdict. Adds /handoff, /remember and /recall over one local markdown store. Requires the CLI: npm install -g @vib795/agent-memory (Node >= 22.5).",
   "author": {
     "name": "Utkarsh Singh",

ARCHITECTURE.md

@@ -3,8 +3,8 @@
 How agent-memory is built, and why it is built that way.
 
 Every figure here was read from the source rather than written from memory, at
-v0.6.2: 15 modules, 3,494 lines of JavaScript, zero runtime dependencies and zero
-dev dependencies, 93 tests.
+v0.6.4: 15 modules, 3,574 lines of JavaScript, zero runtime dependencies and zero
+dev dependencies, 97 tests.
 
 ---
 
@@ -456,12 +456,12 @@ The properties that must stay true. Each is covered by the test suite.
 
 | Module | Lines | Responsibility |
 |---|--:|---|
-| `cli.js` | 960 | 13 commands; one process, one answer |
+| `cli.js` | 980 | 13 commands; one process, one answer |
 | `index-db.js` | 317 | SQLite cache: DDL, reindex, FTS5 search |
 | `pii.js` | 274 | export-time disclosure control |
 | `store.js` | 256 | notes as source of truth; frontmatter; atomic upsert |
 | `compact.js` | 283 | dedupe, supersede, decay, regenerate |
-| `setup.js` | 222 | install into detected agent surfaces |
+| `setup.js` | 282 | install into detected agent surfaces |
 | `staleness.js` | 213 | commit-distance staleness and capture gap |
 | `digest.js` | 209 | two-tier routing: description + tree |
 | `config.js` | 191 | engagement resolution; two-file config split |
@@ -471,7 +471,7 @@ The properties that must stay true. Each is covered by the test suite.
 | `redact.js` | 97 | capture-time fail-closed secret redaction |
 | `promptfile.js` | 78 | VS Code prompt files derived from SKILL.md |
 | `atomic.js` | 57 | atomic write; imports nothing from this package |
-| **total** | **3,494** | zero dependencies, 93 tests |
+| **total** | **3,574** | zero dependencies, 97 tests |
 
 `atomic.js` deliberately imports nothing from the package: `store.js` already imports
 `config.js`, so putting the atomic write in either would create a cycle.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@vib795/agent-memory",
-  "version": "0.6.3",
+  "version": "0.6.4",
   "description": "Durable cross-repo knowledge graph for GitHub Copilot and Claude Code. Markdown source of truth, disposable SQLite index, zero runtime dependencies.",
   "keywords": [
     "github-copilot",

src/cli.js

@@ -164,6 +164,26 @@ function cmdSetup() {
     );
   }
 
+  // Naming what did not install is the whole point. A run that silently installs
+  // eleven of twelve reads as a success and sends the user back to an editor that is
+  // still missing a skill.
+  if (r.failed.length) {
+    lines.push('', '  Failed:');
+    for (const f of r.failed) lines.push(`    [failed] ${f.path} — ${f.error}`);
+    lines.push(
+      '',
+      '  A path that will not clear is usually a leftover link from an install that has',
+      '  since moved. `agent-memory uninstall` reclaims those, then run setup again.',
+    );
+  }
+  if (r.compactError) {
+    lines.push(
+      '',
+      `  Skills are installed, but refreshing the /recall description failed: ${r.compactError}`,
+      '  Run `agent-memory index` then `agent-memory compact` to retry just that step.',
+    );
+  }
+
   return {
     ok: true,
     ...r,

src/setup.js

@@ -55,11 +55,45 @@ function clear(path) {
       // end stay put — which is exactly what passing `recursive` here would risk.
       rmdirSync(path);
     }
+    // `force` tells rmSync to swallow ENOENT, and a junction whose target is gone can
+    // answer the stat rmSync makes with ENOENT while the reparse point itself stays on
+    // disk. The removal then reports success and changes nothing, after which
+    // symlinkSync fails EEXIST and the copy fallback lands on a path that still
+    // exists. Checking is what turns that silent no-op into an error naming the path.
+    if (isLink(path)) rmdirSync(path);
   } else if (existsSync(path)) {
     rmSync(path, { recursive: true, force: true });
   }
 }
 
+/**
+ * Decide whether a skill link at a path we manage was put there by this tool.
+ *
+ * Identity cannot be "points at where we live right now". An install that has since
+ * moved, or a checkout that was deleted, leaves a link that test would disown — and a
+ * disowned link is unreclaimable: `uninstall` keeps it as "not ours" and `setup`
+ * cannot replace what it will not clear, so the user is left holding a broken link
+ * that no command in this tool will fix.
+ */
+function ownsLink(link, name) {
+  let dest;
+  try {
+    dest = resolve(readlinkSync(link));
+  } catch {
+    // A link sitting at one of our paths whose target cannot even be read is ours to
+    // clear. Nothing downstream can use it either.
+    return true;
+  }
+  if (dest === join(packagedSkillsDir(), name)) return true;
+  // Dangling. Nothing is lost by reclaiming a link that points nowhere, and leaving it
+  // is precisely what deadlocks both commands.
+  if (!existsSync(dest)) return true;
+  // Live, but pointing into some other agent-memory tree — an older global install, or
+  // a checkout the user set up from. The signature is that its parent holds all three
+  // of our skills, which a hand-written skill directory would not.
+  return SKILLS.every((s) => existsSync(join(dirname(dest), s, 'SKILL.md')));
+}
+
 /**
  * Link one skill into one agent directory.
  *
@@ -130,7 +164,6 @@ export function danglingSkillLinks() {
  * that indexed them; removing them is a separate, deliberate act.
  */
 export function unlinkSkills() {
-  const packaged = packagedSkillsDir();
   const removed = [];
   const kept = [];
 
@@ -141,11 +174,9 @@ export function unlinkSkills() {
         if (!existsSync(link) && !isLink(link)) continue;
         let owned = false;
         try {
-          owned = isLink(link)
-            ? resolve(readlinkSync(link)) === join(packaged, name)
-            : existsSync(join(link, 'SKILL.md'));
+          owned = isLink(link) ? ownsLink(link, name) : existsSync(join(link, 'SKILL.md'));
         } catch {
-          // A link we cannot read is a link we cannot claim. Leave it.
+          // A directory we cannot stat is one we cannot claim. Leave it.
           owned = false;
         }
         if (owned) {
@@ -182,15 +213,30 @@ export function setup({ compactFn } = {}) {
   const targets = installableTargets();
   const installed = [];
   const copies = [];
+  const failed = [];
 
   for (const target of targets) {
     for (const name of SKILLS) {
-      const r =
-        target.kind === 'skill-dir'
-          ? linkSkill(name, target.dir)
-          : writePromptFile(name, target.dir);
-      installed.push({ ...r, target: target.id, label: target.label });
-      if (r.mode === 'copy') copies.push(r);
+      // Isolated per skill. One unwritable target used to abort the whole run and
+      // discard the report with it, so a user whose `.copilot` link was wedged got no
+      // output at all and no hint that the other eleven had been fine. A failure here
+      // is data to print, not a reason to stop.
+      try {
+        const r =
+          target.kind === 'skill-dir'
+            ? linkSkill(name, target.dir)
+            : writePromptFile(name, target.dir);
+        installed.push({ ...r, target: target.id, label: target.label });
+        if (r.mode === 'copy') copies.push(r);
+      } catch (err) {
+        failed.push({
+          name,
+          target: target.id,
+          label: target.label,
+          path: join(target.dir, target.kind === 'skill-dir' ? name : `${name}.prompt.md`),
+          error: err.message,
+        });
+      }
     }
   }
 
@@ -209,11 +255,25 @@ export function setup({ compactFn } = {}) {
 
   // compact is passed in so this module does not pull the database into memory just
   // to make some symlinks.
-  const result = compactFn ? compactFn() : null;
+  //
+  // Its failure must not sink the run. Linking is already done and written to disk by
+  // this point, so throwing here would throw away an accurate report of work that
+  // actually happened and leave the user with no idea any of it succeeded. compact
+  // touches every registered skill path, which is exactly the set most likely to hold
+  // a stale entry, so it is the step most likely to throw.
+  let result = null;
+  let compactError = null;
+  try {
+    result = compactFn ? compactFn() : null;
+  } catch (err) {
+    compactError = err.message;
+  }
   return {
     targets,
     installed,
     copies,
+    failed,
+    compactError,
     skillPaths,
     home: agentHome(),
     digest: result?.digest ?? null,

test/integration.test.js

@@ -1078,6 +1078,93 @@ test('uninstall removes our links and prompt files, spares foreign ones, keeps n
   }
 });
 
+test('uninstall reclaims a link whose install location moved or vanished', () => {
+  // The field report this fixes: skills installed from a checkout, the checkout later
+  // deleted, and every link then disowned because it no longer resolved to the current
+  // package. `uninstall` kept them as "not ours" and `setup` would not replace what it
+  // would not clear, so no command in the tool could repair the machine.
+  seed().close();
+  const home = fakeHome({ claude: true });
+  try {
+    const dir = skillTargets()[0];
+    mkdirSync(dir, { recursive: true });
+    const stale = join(dir, 'handoff');
+    symlinkSync(join(home, 'deleted-checkout', 'skills', 'handoff'), stale);
+
+    const r = unlinkSkills();
+    assert.ok(r.removed.includes(stale), 'a link pointing nowhere is ours to clear');
+    assert.ok(!r.kept.includes(stale), "and must not be reported as someone else's");
+    assert.equal(existsSync(stale), false);
+  } finally {
+    delete process.env.AGENT_MEMORY_SKILLS_HOME;
+    rmSync(home, { recursive: true, force: true });
+  }
+});
+
+test('a hand-made skill directory is still not ours to remove', () => {
+  // The other half of the same rule. Widening ownership to reclaim stale links must
+  // not widen it to a skill the user wrote, which is why a lone SKILL.md with no
+  // siblings stays put.
+  seed().close();
+  const home = fakeHome({ claude: true });
+  try {
+    const dir = skillTargets()[0];
+    mkdirSync(dir, { recursive: true });
+    const mine = join(home, 'my-own-skills', 'handoff');
+    mkdirSync(mine, { recursive: true });
+    writeFileSync(join(mine, 'SKILL.md'), '---\nname: handoff\n---\nmine\n', 'utf8');
+    const link = join(dir, 'handoff');
+    symlinkSync(mine, link);
+
+    const r = unlinkSkills();
+    assert.ok(r.kept.includes(link), 'a live link outside any agent-memory tree is left alone');
+    assert.equal(existsSync(join(mine, 'SKILL.md')), true, 'and its target survives');
+  } finally {
+    delete process.env.AGENT_MEMORY_SKILLS_HOME;
+    rmSync(home, { recursive: true, force: true });
+  }
+});
+
+test('setup replaces a link left behind by a vanished install', () => {
+  seed().close();
+  const home = fakeHome({ claude: true });
+  try {
+    const dir = skillTargets()[0];
+    mkdirSync(dir, { recursive: true });
+    symlinkSync(join(home, 'gone', 'skills', 'handoff'), join(dir, 'handoff'));
+
+    const r = setup({});
+    assert.deepEqual(r.failed, [], 'a stale link is cleared, not a failure');
+    assert.ok(existsSync(join(dir, 'handoff', 'SKILL.md')), 'link now resolves to the package');
+  } finally {
+    delete process.env.AGENT_MEMORY_SKILLS_HOME;
+    rmSync(home, { recursive: true, force: true });
+  }
+});
+
+test('setup reports a failing compact instead of discarding the whole run', () => {
+  // compact runs last and touches every registered skill path, so it is the step most
+  // likely to throw on a machine with a stale registration. Linking is already on disk
+  // by then; throwing away the report of it leaves the user with no output at all.
+  seed().close();
+  const home = fakeHome({ claude: true });
+  try {
+    const r = setup({
+      compactFn: () => {
+        throw new Error('ENOENT: no such file or directory');
+      },
+    });
+    assert.match(r.compactError, /ENOENT/);
+    assert.ok(r.installed.length > 0, 'the skills that did install are still reported');
+    for (const name of SKILLS) {
+      assert.ok(existsSync(join(skillTargets()[0], name)), `${name} installed despite compact`);
+    }
+  } finally {
+    delete process.env.AGENT_MEMORY_SKILLS_HOME;
+    rmSync(home, { recursive: true, force: true });
+  }
+});
+
 test('a link left pointing at a removed package is detected', () => {
   seed().close();
   const home = mkdtempSync(join(tmpdir(), 'agent-memory-home-'));