fix(hooks): merge PreToolUse bundles instead of replacing#2
Draft
Corey-T1000 wants to merge 1 commit intoauthzed:mainfrom
Draft
fix(hooks): merge PreToolUse bundles instead of replacing#2Corey-T1000 wants to merge 1 commit intoauthzed:mainfrom
Corey-T1000 wants to merge 1 commit intoauthzed:mainfrom
Conversation
InstallHooksConfig used to assign a single-entry slice to hooks.PreToolUse, silently wiping any existing guard hooks (security scanners, subagent auto-approvers, commit formatters, etc.) whenever serve-claude ran with --install-hooks. Now it preserves existing bundles, dedups any prior SpiceBox bundle (identified by hookURL) to stay idempotent across re-installs, and appends ours last so user guards run first and SpiceBox's /check gets the final say. Adds three regression tests: - _PreservesExistingPreToolUseBundles — existing guard survives install - _DedupesOnReinstall — no duplicate SpiceBox entries on repeated installs - _ReinstallPreservesOtherBundles — guards and single SpiceBox coexist assertHookURL was updated to scan for the hook by URL rather than assuming index 0, since the SpiceBox bundle is no longer guaranteed to be first. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Corey-T1000
force-pushed
the
fix/install-hooks-merge-not-replace
branch
from
d1303f8 to
a24b39d
Compare
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.
Summary
InstallHooksConfigcurrently assigns a fresh single-entry slice tohooks.PreToolUse, which silently wipes any existing hooks (guard scripts, user-defined hooks, other plugins' hooks) every timespicebox serve-claude --install-hooksruns.This PR changes
InstallHooksConfigto merge: preserve existing bundles, dedup any prior SpiceBox bundle (identified byhookURL) so re-installs stay idempotent, and append ours last so user guards run first and SpiceBox's/checkgets the final say.Why
Users running SpiceBox alongside other Claude Code hooks lose them on first
--install-hooks. This is a silent data-loss regression in a security-critical config file.Test plan
_PreservesExistingPreToolUseBundles— pre-existing guard survives install_DedupesOnReinstall— repeated installs don't duplicate the SpiceBox bundle_ReinstallPreservesOtherBundles— guards and a single SpiceBox entry coexist across re-installsassertHookURLrefactor (scans array by URL instead of index 0)gofmt,go vet,go build,go test ./internal/lifecycle/...all cleanNotes
assertHookURLhad to be updated because the SpiceBox bundle is no longer guaranteed to be at index 0 — it's appended last so user hooks get first crack.any(requires both sidesstring), so malicious settings can't trick it into dropping unrelated entries; worst case it fails to dedup and the user sees duplicates, never silent data loss.[]any(disk round-trip shape) and[]map[string]any(in-memory construction) for symmetry with the pre-refactor tolerance.🤖 Generated with Claude Code