diff --git a/bin/install.js b/bin/install.js index c797b58..92211d7 100755 --- a/bin/install.js +++ b/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 }) } diff --git a/docs/install.md b/docs/install.md index 7f65ddd..c4c9fb1 100644 --- a/docs/install.md +++ b/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 diff --git a/test/fde-cli.test.js b/test/fde-cli.test.js index a1ab106..71dcf01 100644 --- a/test/fde-cli.test.js +++ b/test/fde-cli.test.js @@ -502,6 +502,119 @@ test('resume bounds a long context.md and survives 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')