fix: prune orphaned settings.json hooks on plugin migration#485
Open
code-with-rashid wants to merge 1 commit into
Open
Conversation
When a user switches from the standalone hook install to the Claude Code plugin, old settings.json entries referencing hook files under ~/.claude/hooks/ persist after those files are removed. Every subsequent session then emits "Cannot find module" errors until the user manually edits settings.json. Two related bugs in `rewriteLegacyManagedHookCommands` made the problem worse: the function only matched bare `node <script>` patterns, silently skipping commands written with an absolute node path (common with Homebrew and nvm installs), so stale absolute paths from old Node versions also went unrewritten. Fix: - Add `extractManagedScriptPath()` helper that parses any node command variant (bare, absolute, quoted/unquoted) to extract the script path. - Rewrite `rewriteLegacyManagedHookCommands()` using the new helper so it handles all node-path formats and skips only commands already using the target binary — not all absolute-path commands. - Add `pruneDeadHookFiles()` which removes hook entries referencing managed scripts that no longer exist on disk. Only touches entries whose script basename is in MANAGED_HOOK_BASENAMES — never removes user-authored hooks. - Call `pruneDeadHookFiles()` in `installClaude()` (plugin path) so migrating to the plugin cleans up orphaned manual entries automatically. - Call `pruneDeadHookFiles()` in `installHooks()` before `rewriteLegacyManagedHookCommands()` so reinstalls and node-upgrade runs also self-heal. Closes JuliusBrussee#471
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When migrating from a standalone hook install to the Claude Code plugin, old
settings.jsonentries referencing hook files under~/.claude/hooks/are not cleaned up. The plugin system takes over hook delivery, but the manual entries remain pointing at files that may no longer exist. Every subsequent session emits noise like:Two related root causes:
No cleanup on plugin install —
removeCavemanHooks()only runs during--uninstall, never during plugin install, so orphaned entries from the previous standalone install persist indefinitely.rewriteLegacyManagedHookCommandsonly matched barenodecommands — the regex^node\s+...silently skipped hooks written with an absolute node path (e.g./opt/homebrew/Cellar/node/26.0.0/bin/node), which is the format written by Homebrew, nvm, and the current installer itself. Stale absolute paths from old Node versions were never updated.Closes #471
Changes
bin/lib/settings.jsextractManagedScriptPath(command)— parses any node hook command variant (barenode, absolute path, quoted/unquoted) and returns the script path. Used by both functions below.rewriteLegacyManagedHookCommands()— now handles all node executable formats, not just barenode. Skips only commands already using the exact target binary; rewrites everything else to the canonical"<absoluteNode>" "<scriptPath>"form.pruneDeadHookFiles(settings)— walks all hook entries, checksfs.existsSyncfor each managed script path, and removes entries whose files are gone. Only touches basenames inMANAGED_HOOK_BASENAMES; user-authored hooks are always left alone.bin/install.jsinstallClaude()— after plugin install (fresh or already-installed), readssettings.jsonand callspruneDeadHookFiles. If orphaned entries are found, writes the cleaned file back and logs how many were removed.installHooks()— callspruneDeadHookFilesbeforerewriteLegacyManagedHookCommandsso dead entries and stale node paths are both resolved before idempotency checks run.Tests
Added 11 new tests in
tests/installer/unit.settings.test.mjs(all 28 unit tests pass):rewriteLegacyManagedHookCommandsrewrites stale absolute-node scripts (the old "ignores absolute node" behaviour was the bug)rewriteLegacyManagedHookCommandsskips commands already using the target node binaryrewriteLegacyManagedHookCommandsrewrites unquoted absolute-node scriptspruneDeadHookFilesremoves entries whose managed scripts are missingpruneDeadHookFileskeeps entries whose managed scripts exist on diskpruneDeadHookFilesdoes not touch user-authored hooks even if their files are missingpruneDeadHookFileshandles multiple events and mixed live/dead entriespruneDeadHookFileshandles the bare-node command formatpruneDeadHookFilesis a no-op on empty or missing hookspruneDeadHookFilespreserves non-hook settings keys (theme, statusLine, etc.)