fix: installer only removes or overwrites skill dirs it created (#8)

bin/install.js

@@ -53,6 +53,49 @@ function slugify(name) {
     || 'engagement'
 }
 
+// Written into every skill directory this installer creates. Nothing is ever
+// removed or overwritten without it: `healthcare-fde` and `fintech-fde` are
+// plausible names for a skill the user wrote themselves, and a name collision
+// must be reported, not silently resolved by deleting their work.
+const MANAGED_MARKER = '.fdeops-managed'
+
+function markManaged(dir) {
+  let version = 'unknown'
+  try { version = require(path.join(__dirname, '..', 'package.json')).version } catch (_) {}
+  try {
+    fs.writeFileSync(
+      path.join(dir, MANAGED_MARKER),
+      `managed-by: fdeops\nversion: ${version}\ninstalled: ${new Date().toISOString()}\n` +
+      'Delete this file to make fdeops treat the directory as yours and leave it alone.\n',
+    )
+  } catch (_) {}
+}
+
+function isManaged(dir) {
+  return fs.existsSync(path.join(dir, MANAGED_MARKER))
+}
+
+// A skill "directory" that is really a symlink points somewhere outside
+// ~/.claude/skills that fdeops has no claim on. Writing through it would edit
+// files in the user's own tree - refuse even under --force, which is permission
+// to take over this location, not to follow it elsewhere.
+function isLink(p) {
+  try { return fs.lstatSync(p).isSymbolicLink() } catch (_) { return false }
+}
+
+// Fingerprint of a skill fdeops itself wrote before markers existed. Anchored on
+// the shipped frontmatter, not a bare "fdeops" substring: a skill of the user's
+// that merely mentions fdeops in prose is theirs, not ours. Only consulted for a
+// directory whose name matches one we ship, and only to overwrite - never to delete.
+function wasInstalledByUs(dir) {
+  try {
+    const md = fs.readFileSync(path.join(dir, 'SKILL.md'), 'utf8')
+    const fm = (md.match(/^---\n([\s\S]*?)\n---/) || [])[1]
+    if (!fm) return false
+    return /^description:\s*Engagement fieldbook for Forward Deployed Engineers\b/im.test(fm)
+  } catch (_) { return false }
+}
+
 // v2 shipped 16 standalone skills; v3 is one `fde` skill + references.
 // Leaving the old ones in place would route users to stale content.
 const LEGACY_SKILL_DIRS = [
@@ -62,22 +105,95 @@ const LEGACY_SKILL_DIRS = [
   'gov-fde',
 ]
 
-function removeLegacySkills() {
+function removeLegacySkills(opts = {}) {
   let removed = 0
+  const skipped = []
+  const links = []
   for (const dir of LEGACY_SKILL_DIRS) {
     const p = path.join(GLOBAL_SKILLS_DIR, dir)
-    if (fs.existsSync(path.join(p, 'SKILL.md'))) {
+    if (isLink(p)) { links.push(dir); continue }
+    if (!fs.existsSync(path.join(p, 'SKILL.md'))) continue
+    if (isManaged(p) || opts.force) {
       fs.rmSync(p, { recursive: true, force: true })
       removed++
+    } else {
+      skipped.push(dir)
     }
   }
-  return removed
+  return { removed, skipped, links }
+}
+
+// Copy each skill in, but never over a directory fdeops did not create.
+function installSkillDirs(opts = {}) {
+  const skipped = []
+  const links = []
+  const failed = []
+  fs.mkdirSync(GLOBAL_SKILLS_DIR, { recursive: true })
+  for (const entry of fs.readdirSync(SKILLS_SRC, { withFileTypes: true })) {
+    const src = path.join(SKILLS_SRC, entry.name)
+    const dest = path.join(GLOBAL_SKILLS_DIR, entry.name)
+    if (!entry.isDirectory()) { fs.copyFileSync(src, dest); continue }
+    if (isLink(dest)) { links.push(entry.name); continue }
+    if (fs.existsSync(dest) && !isManaged(dest) && !opts.force) {
+      // Installs predating the marker are still ours: adopt a same-named dir
+      // whose SKILL.md is recognizably fdeops', so upgrades keep working.
+      if (!wasInstalledByUs(dest)) {
+        skipped.push(entry.name)
+        continue
+      }
+      console.log(`  adopt  ~/.claude/skills/${entry.name} (earlier fdeops install)`)
+    }
+    // One unwritable skill dir must not abort the install with a stack trace:
+    // say it in human terms, place the rest, and exit non-zero at the end.
+    try {
+      copyDir(src, dest)
+      markManaged(dest)
+    } catch (e) {
+      failed.push({ name: entry.name, code: e.code || 'error', path: e.path || dest })
+    }
+  }
+  return { skipped, links, failed }
+}
+
+function reportCollisions(paths, verb) {
+  if (!paths.length) return
+  console.log(`  skip   ${paths.length} skill dir(s) fdeops did not create - ${verb} would destroy your own work:`)
+  for (const name of paths) console.log(`           ~/.claude/skills/${name}`)
+  console.log('         move or delete them yourself, or re-run with --force to let fdeops take them over')
+}
+
+function reportLinks(names) {
+  if (!names.length) return
+  console.log(`  skip   ${names.length} skill path(s) that are symlinks - fdeops will not write through them:`)
+  for (const name of names) console.log(`           ~/.claude/skills/${name} -> ${readLinkQuiet(path.join(GLOBAL_SKILLS_DIR, name))}`)
+  console.log('         remove the link if you want fdeops to install at that path itself')
+}
+
+function readLinkQuiet(p) {
+  try { return fs.readlinkSync(p) } catch (_) { return '(unreadable)' }
+}
+
+// Anything here means part of the install did not land; cmdInstall exits non-zero.
+let installIncomplete = false
+function reportFailures(failures) {
+  if (!failures.length) return
+  installIncomplete = true
+  console.log(`  error  ${failures.length} skill dir(s) could not be written:`)
+  for (const f of failures) {
+    const why = f.code === 'EACCES' || f.code === 'EPERM' ? 'permission denied' : f.code
+    console.log(`           ~/.claude/skills/${f.name} - ${why} at ${f.path}`)
+  }
+  console.log('         fix the permissions (or remove the directory) and re-run - the rest of the install continued')
 }
 
-function installSkills() {
-  const removed = removeLegacySkills()
-  if (removed > 0) console.log(`  Removed ${removed} v2 skill dir(s) (now covered by @fde)`)
-  copyDir(SKILLS_SRC, GLOBAL_SKILLS_DIR)
+function installSkills(opts = {}) {
+  const legacy = removeLegacySkills(opts)
+  if (legacy.removed > 0) console.log(`  Removed ${legacy.removed} v2 skill dir(s) (now covered by @fde)`)
+  const placed = installSkillDirs(opts)
+  reportCollisions(legacy.skipped, 'removing them')
+  reportCollisions(placed.skipped, 'overwriting them')
+  reportLinks([...new Set([...legacy.links, ...placed.links])])
+  reportFailures(placed.failed)
   fs.mkdirSync(GLOBAL_HOOKS_DIR, { recursive: true })
   for (const name of HOOK_SCRIPTS) {
     const src = path.join(HOOKS_SRC, name)
@@ -138,7 +254,7 @@ function placePointer(destPath, content, label, appendable) {
   console.log(`  write  ${label}`)
 }
 
-function cmdAdapters(targetDir) {
+function cmdAdapters(targetDir, opts = {}) {
   const dest = path.resolve(targetDir || process.cwd())
   console.log('')
   console.log(`  fdeops cross-platform adapters → ${dest}`)
@@ -150,7 +266,7 @@ function cmdAdapters(targetDir) {
   // yet, a dangling reference for anyone following the documented Cursor/Codex
   // path. installSkills() is idempotent (safe to call every run).
   if (!fs.existsSync(path.join(GLOBAL_SKILLS_DIR, 'fde', 'SKILL.md'))) {
-    installSkills()
+    installSkills(opts)
     console.log('  Skills → ~/.claude/skills/  (installed - the pointers below need this)')
     console.log('')
   }
@@ -198,11 +314,11 @@ function cmdInit(engagementName) {
   console.log('')
 }
 
-function cmdInstall() {
+function cmdInstall(opts = {}) {
   console.log('')
   console.log('  fdeops - installs on YOUR machine only')
   console.log('')
-  installSkills()
+  installSkills(opts)
   console.log('  Skills → ~/.claude/skills/')
   console.log('  Hooks → ~/.claude/hooks/fdeops-*')
   console.log('  CLI → ~/.claude/fdeops/fde.js  (try: node ~/.claude/fdeops/fde.js scan)')
@@ -220,6 +336,9 @@ function cmdInstall() {
   console.log('  Then open your workspace and use @fde')
   console.log('  Docs: docs/install.md')
   console.log('')
+  // A partly-installed skill set is not success - a script that ran this must be
+  // able to tell, and the reason is already printed above.
+  if (installIncomplete) process.exit(1)
 }
 
 // `npx fdeops scan` must recon, not install - any fde subcommand passes straight
@@ -229,13 +348,16 @@ const FDE_SUBCOMMANDS = [
   'garden', 'owner', 'receipts', 'capture', 'status', 'dashboard', 'help',
 ]
 
-const arg = process.argv[2]
+const argv = process.argv.slice(2)
+const force = argv.includes('--force')
+const positional = argv.filter(a => a !== '--force')
+const arg = positional[0]
 if (arg === 'init') {
-  cmdInit(process.argv[3])
+  cmdInit(positional[1])
 } else if (arg === 'adapters') {
-  cmdAdapters(process.argv[3])
+  cmdAdapters(positional[1], { force })
 } else if (FDE_SUBCOMMANDS.includes(arg)) {
   require(path.join(__dirname, 'fde.js'))
 } else {
-  cmdInstall()
+  cmdInstall({ force })
 }

docs/install.md

@@ -161,6 +161,23 @@ cd fdeops && git pull && node bin/install.js
 
 Or via npm: `npx fdeops@latest` (fetches the latest published fdeops).
 
+### What the installer will not touch
+
+Every skill directory fdeops creates under `~/.claude/skills/` carries a `.fdeops-managed` marker, and the installer only removes or overwrites directories that have it. If you wrote your own skill whose name collides with one fdeops ships or shipped in v2 (`healthcare-fde`, `fintech-fde`, `gov-fde`, `fde-*`), it is left untouched and reported:
+
+```text
+  skip   1 skill dir(s) fdeops did not create - removing them would destroy your own work:
+           ~/.claude/skills/healthcare-fde
+         move or delete them yourself, or re-run with --force to let fdeops take them over
+```
+
+Delete a `.fdeops-managed` marker to make fdeops treat that directory as yours from then on. `node bin/install.js --force` overrides the check.
+
+Two things `--force` does **not** override:
+
+- **Symlinks.** If `~/.claude/skills/fde` is a link into your own tree, fdeops refuses to write through it and tells you where it points. `--force` is permission to take over that location, not to follow it somewhere else.
+- **Permissions.** An unwritable skill directory is reported (`permission denied at …`), the rest of the install still lands, and the installer exits non-zero so a script can tell it was incomplete.
+
 ---
 
 ## Usage

test/fde-cli.test.js

@@ -502,6 +502,119 @@ test('resume bounds a long context.md and survives <private> redaction (anchor n
   assert.doesNotMatch(r.stdout, /OLD session log line 5\b/, 'the old middle must be hidden, not dumped')
 })
 
+test('install never deletes or overwrites a skill dir the user wrote (issue #8)', () => {
+  const sandbox = makeSandbox('install-ownership')
+  const skills = path.join(sandbox.home, '.claude', 'skills')
+  const mine = path.join(skills, 'healthcare-fde')
+  fs.mkdirSync(mine, { recursive: true })
+  fs.writeFileSync(path.join(mine, 'SKILL.md'), '---\nname: healthcare-fde\n---\nmy own hard-won prompt\n')
+  fs.writeFileSync(path.join(mine, 'notes.md'), 'irreplaceable\n')
+  const ownFde = path.join(skills, 'fde')
+  fs.mkdirSync(ownFde, { recursive: true })
+  fs.writeFileSync(path.join(ownFde, 'SKILL.md'), '---\nname: fde\n---\nunrelated skill of mine\n')
+
+  const first = runInstall(sandbox, [])
+  assert.equal(first.status, 0, first.stderr)
+  assert.equal(fs.readFileSync(path.join(mine, 'SKILL.md'), 'utf8'), '---\nname: healthcare-fde\n---\nmy own hard-won prompt\n')
+  assert.equal(fs.existsSync(path.join(mine, 'notes.md')), true, 'user-authored skill must survive install')
+  assert.equal(fs.readFileSync(path.join(ownFde, 'SKILL.md'), 'utf8'), '---\nname: fde\n---\nunrelated skill of mine\n')
+  assert.match(first.stdout, /fdeops did not create/)
+  assert.match(first.stdout, /healthcare-fde/)
+
+  // --force is the documented escape hatch
+  const forced = runInstall(sandbox, ['--force'])
+  assert.equal(forced.status, 0, forced.stderr)
+  assert.equal(fs.existsSync(mine), false, '--force removes the legacy dir')
+  assert.match(fs.readFileSync(path.join(ownFde, 'SKILL.md'), 'utf8'), /fdeops|fieldbook/i)
+  assert.equal(fs.existsSync(path.join(ownFde, '.fdeops-managed')), true)
+
+  // dirs fdeops created are marked, so the next run can clean them up on its own
+  const legacy = path.join(skills, 'fde-land')
+  fs.mkdirSync(legacy, { recursive: true })
+  fs.writeFileSync(path.join(legacy, 'SKILL.md'), 'stale v2 content\n')
+  fs.writeFileSync(path.join(legacy, '.fdeops-managed'), 'managed-by: fdeops\n')
+  const second = runInstall(sandbox, [])
+  assert.equal(second.status, 0, second.stderr)
+  assert.equal(fs.existsSync(legacy), false, 'a marked v2 dir is still cleaned up')
+  assert.match(second.stdout, /Removed 1 v2 skill dir/)
+})
+
+test('install adopts an earlier unmarked fdeops skill so upgrades still apply', () => {
+  const sandbox = makeSandbox('install-adopt')
+  const ownFde = path.join(sandbox.home, '.claude', 'skills', 'fde')
+  fs.mkdirSync(ownFde, { recursive: true })
+  // what a pre-marker install actually left behind: our shipped frontmatter
+  fs.writeFileSync(path.join(ownFde, 'SKILL.md'),
+    '---\nname: fde\ndescription: Engagement fieldbook for Forward Deployed Engineers. Use when …\n---\nold fdeops brain from a pre-marker install\n')
+
+  const r = runInstall(sandbox, [])
+  assert.equal(r.status, 0, r.stderr)
+  assert.match(r.stdout, /adopt/)
+  assert.equal(fs.existsSync(path.join(ownFde, '.fdeops-managed')), true)
+  assert.doesNotMatch(fs.readFileSync(path.join(ownFde, 'SKILL.md'), 'utf8'), /pre-marker install/)
+
+  // A skill of the user's that merely mentions fdeops in prose is not ours: the
+  // fingerprint is the shipped frontmatter, not the word "fdeops" somewhere.
+  const mentions = makeSandbox('install-mentions')
+  const theirs = path.join(mentions.home, '.claude', 'skills', 'fde')
+  fs.mkdirSync(theirs, { recursive: true })
+  const prose = '---\nname: fde\ndescription: My own wrapper around fdeops for retainer clients\n---\nirreplaceable\n'
+  fs.writeFileSync(path.join(theirs, 'SKILL.md'), prose)
+  const m = runInstall(mentions, [])
+  assert.equal(m.status, 0, m.stderr)
+  assert.equal(fs.readFileSync(path.join(theirs, 'SKILL.md'), 'utf8'), prose, 'prose mentioning fdeops must not authorize an overwrite')
+  assert.match(m.stdout, /fdeops did not create/)
+})
+
+test('install refuses to write through a symlinked skill dir, even with --force', () => {
+  const sandbox = makeSandbox('install-symlink')
+  const skills = path.join(sandbox.home, '.claude', 'skills')
+  // the user keeps skills in their own tree and links them into ~/.claude
+  const real = path.join(sandbox.dir, 'my-skills', 'fde')
+  fs.mkdirSync(real, { recursive: true })
+  const mine = '---\nname: fde\ndescription: Engagement fieldbook for Forward Deployed Engineers - my fork\n---\nsentinel\n'
+  fs.writeFileSync(path.join(real, 'SKILL.md'), mine)
+  fs.mkdirSync(skills, { recursive: true })
+  fs.symlinkSync(real, path.join(skills, 'fde'))
+  // and a legacy-named one, which the removal path must not follow either
+  const realLegacy = path.join(sandbox.dir, 'my-skills', 'gov-fde')
+  fs.mkdirSync(realLegacy, { recursive: true })
+  fs.writeFileSync(path.join(realLegacy, 'SKILL.md'), 'my gov skill\n')
+  fs.symlinkSync(realLegacy, path.join(skills, 'gov-fde'))
+
+  for (const args of [[], ['--force']]) {
+    const r = runInstall(sandbox, args)
+    assert.equal(fs.readFileSync(path.join(real, 'SKILL.md'), 'utf8'), mine,
+      `fdeops wrote through the symlink (${args.join(' ') || 'no flags'})`)
+    assert.equal(fs.existsSync(path.join(real, '.fdeops-managed')), false,
+      'no marker may be planted outside ~/.claude/skills')
+    assert.equal(fs.existsSync(path.join(real, 'references')), false)
+    assert.equal(fs.existsSync(path.join(realLegacy, 'SKILL.md')), true, 'a symlinked legacy dir must survive')
+    assert.equal(fs.lstatSync(path.join(skills, 'fde')).isSymbolicLink(), true, 'the link itself stays the user\'s')
+    assert.match(r.stdout, /symlinks - fdeops will not write through them/)
+  }
+})
+
+test('install reports an unwritable skill dir in human terms and finishes the rest', () => {
+  const sandbox = makeSandbox('install-readonly')
+  const dest = path.join(sandbox.home, '.claude', 'skills', 'fde')
+  fs.mkdirSync(dest, { recursive: true })
+  fs.writeFileSync(path.join(dest, 'SKILL.md'),
+    '---\nname: fde\ndescription: Engagement fieldbook for Forward Deployed Engineers. Use when …\n---\nold\n')
+  fs.chmodSync(dest, 0o500)
+  try {
+    const r = runInstall(sandbox, [])
+    assert.equal(r.status, 1, 'a partial install must not report success')
+    assert.match(r.stdout, /permission denied/)
+    assert.doesNotMatch(r.stderr, /at Object\.|node:internal/, 'no stack trace')
+    // the rest of the install still landed
+    assert.equal(fs.existsSync(path.join(sandbox.home, '.claude', 'fdeops', 'fde.js')), true)
+    assert.equal(fs.existsSync(path.join(sandbox.home, '.claude', 'FDEOPS-CLAUDE.md')), true)
+  } finally {
+    fs.chmodSync(dest, 0o700)
+  }
+})
+
 test('demo runs the real CLI on a fake client, leaks no private block, and stays out of the portfolio', () => {
   const sandbox = makeSandbox('demo')
   const root = path.join(sandbox.dir, 'engagements')