Protect skill projections from foreign collisions

.changeset/safe-skill-projections.md

@@ -0,0 +1,5 @@
+---
+'@inkeep/open-knowledge': patch
+---
+
+Protect skill projections from accidental replacement. Installing a project skill now refuses a conflicting editor-skill path before writing any target, while existing links to the same source remain idempotent.

packages/server/src/skill-projection.test.ts

@@ -149,19 +149,37 @@ describe('projectSkill / reverseProjectSkill', () => {
     }
   });
 
-  test('install is authoritative — replaces a legacy real-dir copy with a symlink', () => {
+  test('refuses a foreign real-dir collision without mutating it', () => {
     const dir = makeSkill('s', '# v1');
     const dest = skillHostDir(root, 'claude', 's') as string;
-    // Simulate a legacy copy-install: a real directory at the host path.
+    // A legacy copy may contain user edits; projection cannot prove ownership.
     mkdirSync(dest, { recursive: true });
     writeFileSync(join(dest, 'stale.md'), 'leftover', 'utf-8');
     expect(lstatSync(dest).isSymbolicLink()).toBe(false);
 
-    projectSkill(dir, 's', root, ['claude']);
-    // Now a symlink to the source; the stale real-dir contents are gone.
-    expect(lstatSync(dest).isSymbolicLink()).toBe(true);
-    expect(existsSync(join(dest, 'stale.md'))).toBe(false);
-    expect(existsSync(join(dest, 'SKILL.md'))).toBe(true);
+    expect(() => projectSkill(dir, 's', root, ['claude'])).toThrow('occupied by a non-link');
+    expect(lstatSync(dest).isSymbolicLink()).toBe(false);
+    expect(existsSync(join(dest, 'stale.md'))).toBe(true);
+  });
+
+  test('preflights every target before creating any projection', () => {
+    const dir = makeSkill('atomic', '# v1');
+    const foreign = skillHostDir(root, 'cursor', 'atomic') as string;
+    mkdirSync(foreign, { recursive: true });
+    writeFileSync(join(foreign, 'keep.md'), 'user-owned', 'utf-8');
+
+    expect(() => projectSkill(dir, 'atomic', root, ['claude', 'cursor'])).toThrow(
+      'occupied by a non-link',
+    );
+    expect(existsSync(skillHostDir(root, 'claude', 'atomic') as string)).toBe(false);
+    expect(existsSync(join(foreign, 'keep.md'))).toBe(true);
+  });
+
+  test('keeps an exact existing projection idempotently', () => {
+    const dir = makeSkill('stable', '# v1');
+    expect(projectSkill(dir, 'stable', root, ['codex'])).toEqual(['codex']);
+    expect(projectSkill(dir, 'stable', root, ['codex'])).toEqual(['codex']);
+    expect(lstatSync(skillHostDir(root, 'codex', 'stable') as string).isSymbolicLink()).toBe(true);
   });
 
   test('skillHostDir returns null for claude-desktop', () => {

packages/server/src/skill-projection.ts

@@ -26,7 +26,15 @@
  * in lock-step.
  */
 
-import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync, statSync } from 'node:fs';
+import {
+  existsSync,
+  lstatSync,
+  readdirSync,
+  readFileSync,
+  readlinkSync,
+  realpathSync,
+  statSync,
+} from 'node:fs';
 import { dirname, isAbsolute, join, relative, resolve } from 'node:path';
 import {
   containsXmlTag,
@@ -228,29 +236,65 @@ function skillLinkTarget(cwd: string, hostRoot: string, skillDir: string): strin
   return insideProject ? relative(hostRoot, absSkill) : absSkill;
 }
 
+/**
+ * Inspect one projection destination without following a link. Only an absent
+ * path or an exact existing link to this source is safe: a real directory,
+ * file, dangling link, or link to another source belongs to a user or another
+ * installer and must never be replaced implicitly.
+ */
+function projectionStatus(dest: string, skillDir: string): 'absent' | 'owned' {
+  try {
+    const stat = lstatSync(dest);
+    if (!stat.isSymbolicLink()) {
+      throw new Error(`skill projection destination is occupied by a non-link: ${dest}`);
+    }
+    const target = resolve(dirname(dest), readlinkSync(dest));
+    if (target !== resolve(skillDir)) {
+      throw new Error(`skill projection destination points at a different source: ${dest}`);
+    }
+    return 'owned';
+  } catch (error) {
+    const code = (error as NodeJS.ErrnoException).code;
+    if (code === 'ENOENT') return 'absent';
+    throw error;
+  }
+}
+
 /**
  * Install a skill source dir into each target editor's host dir by SYMLINK.
- * Removes any existing entry first (authoritative replace — a stale/broken
- * link or a legacy real-dir copy is dropped before the link is made). Returns
- * the editor ids actually written (skipping editors with no skill surface).
+ * Every destination is preflighted before the first write. An existing exact
+ * projection is idempotent; every foreign destination is refused untouched,
+ * so callers never get a partial multi-editor install or a silent overwrite.
+ * Returns the editor ids with a valid projection (created or already owned).
  */
 export function projectSkill(
   skillDir: string,
   name: string,
   cwd: string,
   targets: readonly EditorId[],
 ): EditorId[] {
-  const written: EditorId[] = [];
+  const candidates: Array<{
+    editor: EditorId;
+    dest: string;
+    hostRoot: string;
+    status: 'absent' | 'owned';
+  }> = [];
   for (const editor of targets) {
     const dest = skillHostDir(cwd, editor, name);
     if (dest === null) continue;
     const hostRoot = dirname(dest);
     // Refuse to write through a host root that symlink-escapes the project.
     if (hostSkillsRootEscapes(cwd, hostRoot)) continue;
-    tracedRmSync(dest, { recursive: true, force: true });
-    tracedMkdirSync(hostRoot, { recursive: true });
-    tracedSymlinkSync(skillLinkTarget(cwd, hostRoot, skillDir), dest, 'dir');
-    written.push(editor);
+    candidates.push({ editor, dest, hostRoot, status: projectionStatus(dest, skillDir) });
+  }
+
+  const written: EditorId[] = [];
+  for (const candidate of candidates) {
+    if (candidate.status === 'absent') {
+      tracedMkdirSync(candidate.hostRoot, { recursive: true });
+      tracedSymlinkSync(skillLinkTarget(cwd, candidate.hostRoot, skillDir), candidate.dest, 'dir');
+    }
+    written.push(candidate.editor);
   }
   return written;
 }