From 70ea40dced28f27486c8d3046061dbe1537e334c Mon Sep 17 00:00:00 2001 From: AmirF194 Date: Wed, 1 Jul 2026 23:34:41 -0600 Subject: [PATCH] fix(uninstall): match managed hook basenames, not substring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit removeCavemanHooks stripped any settings.json hook whose command contained the substring 'caveman' anywhere — a user-authored hook like 'node ~/Projects/caveman-notes/my-hook.js' was silently deleted by --uninstall. Match tokens against MANAGED_HOOK_BASENAMES by exact basename instead (win32.basename so Windows-written configs match anywhere), the same pattern pruneOrphanedManagedHooks already uses. Hoist the tokenizer to module scope and reuse it in the prune pass. Add caveman-statusline.ps1 to the managed set so the Windows statusline wiring is covered by removal and orphan-pruning too. Fixes #593 --- bin/install.js | 2 +- bin/lib/settings.js | 72 +++++++++++++++++--------- tests/installer/unit.settings.test.mjs | 41 +++++++++++++-- 3 files changed, 84 insertions(+), 31 deletions(-) diff --git a/bin/install.js b/bin/install.js index 75223dc..e13d273 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1049,7 +1049,7 @@ function uninstall(ctx) { if (fs.existsSync(settingsPath)) { const settings = SETTINGS.readSettings(settingsPath); if (settings) { - const removed = SETTINGS.removeCavemanHooks(settings, 'caveman'); + const removed = SETTINGS.removeCavemanHooks(settings); // Drop our statusline if it points at our script if (settings.statusLine) { const cmd = typeof settings.statusLine === 'string' ? settings.statusLine : (settings.statusLine.command || ''); diff --git a/bin/lib/settings.js b/bin/lib/settings.js index db9d271..770cf01 100644 --- a/bin/lib/settings.js +++ b/bin/lib/settings.js @@ -149,12 +149,52 @@ function addCommandHook(settings, event, opts) { return true; } +// ── Managed hook scripts ────────────────────────────────────────────────── +// The exact script basenames this installer wires into settings.json. Every +// helper that decides "is this hook ours?" must match against these — never +// against a bare "caveman" substring, which also matches user-authored hooks +// that merely mention the word in a path (issue #593). +const MANAGED_HOOK_BASENAMES = new Set([ + 'caveman-activate.js', + 'caveman-mode-tracker.js', + 'caveman-stats.js', + 'caveman-statusline.sh', + 'caveman-statusline.ps1', +]); + +// Split a command into shell-ish tokens, honoring single/double quotes so a +// path containing spaces survives intact. Good enough for hook commands we +// generate (`node "/a/x.js"`, `"/abs/node" "/a/x.js"`, `bash /a/x.sh`); not +// a full shell parser. +function tokenizeCommand(command) { + const out = []; + const re = /"([^"]*)"|'([^']*)'|(\S+)/g; + let m; + while ((m = re.exec(command)) !== null) out.push(m[1] ?? m[2] ?? m[3]); + return out; +} + +// True iff some token's BASENAME exactly equals a managed script name. Exact +// match — not substring — so `mycaveman-activate.js` or a user hook living +// under a `caveman-notes/` directory is never treated as ours. win32.basename +// splits on both / and \ so a settings.json written on Windows still matches +// when processed elsewhere. +function referencesManagedScript(command) { + try { + for (const tok of tokenizeCommand(command)) { + if (tok && typeof tok === 'string' && MANAGED_HOOK_BASENAMES.has(path.win32.basename(tok))) return true; + } + } catch (_) { /* malformed command — treat as not ours */ } + return false; +} + // ── removeCavemanHooks ──────────────────────────────────────────────────── -// Strip every entry whose any hook command mentions `marker`. Empties events. -// Tolerates malformed pre-existing settings (non-array hook lists, foreign -// shapes) — those get dropped by validateHookFields first so we never call -// .length / .filter on a non-array. -function removeCavemanHooks(settings, marker = 'caveman') { +// Strip every entry whose any hook command targets one of our managed hook +// scripts (exact basename match, see above). Empties events. Tolerates +// malformed pre-existing settings (non-array hook lists, foreign shapes) — +// those get dropped by validateHookFields first so we never call .length / +// .filter on a non-array. +function removeCavemanHooks(settings) { if (!settings || !settings.hooks) return 0; validateHookFields(settings); if (!settings.hooks) return 0; // validate may have deleted the whole tree @@ -164,7 +204,7 @@ function removeCavemanHooks(settings, marker = 'caveman') { const before = settings.hooks[ev].length; settings.hooks[ev] = settings.hooks[ev].filter(entry => { if (!entry || !Array.isArray(entry.hooks)) return true; - return !entry.hooks.some(h => h && typeof h.command === 'string' && h.command.includes(marker)); + return !entry.hooks.some(h => h && typeof h.command === 'string' && referencesManagedScript(h.command)); }); removed += before - settings.hooks[ev].length; if (settings.hooks[ev].length === 0) delete settings.hooks[ev]; @@ -179,12 +219,6 @@ function removeCavemanHooks(settings, marker = 'caveman') { // `absoluteNode` so GUI launchers with minimal PATH still find Node. Only // touches commands matching the exact bare-node shape — won't false-positive // on user-authored hooks that just happen to mention "caveman". -const MANAGED_HOOK_BASENAMES = new Set([ - 'caveman-activate.js', - 'caveman-mode-tracker.js', - 'caveman-stats.js', - 'caveman-statusline.sh', -]); function rewriteLegacyManagedHookCommands(settings, absoluteNode) { if (!settings || !settings.hooks || !absoluteNode) return 0; let rewritten = 0; @@ -229,18 +263,6 @@ function pruneOrphanedManagedHooks(settings, configDir) { const baseDir = configDir || claudeConfigDir(); let removed = 0; - // Split a command into shell-ish tokens, honoring single/double quotes so a - // path containing spaces survives intact. Good enough for hook commands we - // generate (`node "/a/x.js"`, `"/abs/node" "/a/x.js"`, `bash /a/x.sh`); not - // a full shell parser. - const tokenize = (command) => { - const out = []; - const re = /"([^"]*)"|'([^']*)'|(\S+)/g; - let m; - while ((m = re.exec(command)) !== null) out.push(m[1] ?? m[2] ?? m[3]); - return out; - }; - // A command is a missing managed target iff some token's BASENAME exactly // equals a managed script (exact match — not substring — so a user hook like // `mycaveman-activate.js` is never touched) and that resolved path is absent. @@ -248,7 +270,7 @@ function pruneOrphanedManagedHooks(settings, configDir) { // so a malformed command or fs error never throws out of the prune pass. const targetMissing = (command) => { try { - for (const tok of tokenize(command)) { + for (const tok of tokenizeCommand(command)) { if (!tok || typeof tok !== 'string') continue; if (!MANAGED_HOOK_BASENAMES.has(path.basename(tok))) continue; const scriptPath = path.isAbsolute(tok) ? tok : path.join(baseDir, tok); diff --git a/tests/installer/unit.settings.test.mjs b/tests/installer/unit.settings.test.mjs index 960f2f7..dd34705 100644 --- a/tests/installer/unit.settings.test.mjs +++ b/tests/installer/unit.settings.test.mjs @@ -119,27 +119,58 @@ test('removeCavemanHooks tolerates malformed hook event values without throwing' // validateHookFields first + adds Array.isArray guard. const s = { hooks: { SessionStart: "oops", UserPromptSubmit: { not: 'an array either' } } }; let removed; - assert.doesNotThrow(() => { removed = SETTINGS.removeCavemanHooks(s, 'caveman'); }); + assert.doesNotThrow(() => { removed = SETTINGS.removeCavemanHooks(s); }); assert.equal(removed, 0); assert.equal(s.hooks, undefined); }); -test('removeCavemanHooks strips by marker and cleans empties', () => { +test('removeCavemanHooks strips managed scripts and cleans empties', () => { const s = { hooks: { SessionStart: [ - { hooks: [{ type: 'command', command: 'caveman-x' }] }, + { hooks: [{ type: 'command', command: 'node /x/hooks/caveman-activate.js' }] }, { hooks: [{ type: 'command', command: 'other' }] }, ], - UserPromptSubmit: [{ hooks: [{ type: 'command', command: 'caveman-y' }] }], + UserPromptSubmit: [{ hooks: [{ type: 'command', command: '"/usr/bin/node" "/x/hooks/caveman-mode-tracker.js"' }] }], }, }; - const removed = SETTINGS.removeCavemanHooks(s, 'caveman'); + const removed = SETTINGS.removeCavemanHooks(s); assert.equal(removed, 2); assert.equal(s.hooks.SessionStart.length, 1); assert.equal(s.hooks.UserPromptSubmit, undefined); }); +test('removeCavemanHooks leaves user hooks that merely mention caveman (issue #593)', () => { + const s = { + hooks: { + PreToolUse: [ + // Path contains the word "caveman" but targets a user-authored script. + { hooks: [{ type: 'command', command: 'node /Users/me/Projects/caveman-notes/my-hook.js' }] }, + // Basename is a superstring of a managed name — still not ours. + { hooks: [{ type: 'command', command: 'node /x/mycaveman-activate.js' }] }, + ], + SessionStart: [ + { hooks: [{ type: 'command', command: '"/usr/bin/node" "/x/hooks/caveman-activate.js"' }] }, + ], + }, + }; + const removed = SETTINGS.removeCavemanHooks(s); + assert.equal(removed, 1, 'only the managed SessionStart hook should be removed'); + assert.equal(s.hooks.PreToolUse.length, 2, 'user hooks mentioning caveman must survive uninstall'); + assert.equal(s.hooks.SessionStart, undefined); +}); + +test('removeCavemanHooks removes the Windows statusline-stats wiring (caveman-stats.js / .ps1)', () => { + const s = { + hooks: { + Stop: [{ hooks: [{ type: 'command', command: '"C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\me\\.claude\\hooks\\caveman-stats.js"' }] }], + }, + }; + const removed = SETTINGS.removeCavemanHooks(s); + assert.equal(removed, 1); + assert.equal(s.hooks, undefined); +}); + test('rewriteLegacyManagedHookCommands rewrites bare-node managed scripts', () => { const s = { hooks: {