fix(retrofit): dedupe hook groups so repeated --retrofit stops inflating settings.json - #61
Merged
Merged
Conversation
…ing settings.json
`deep_merge_settings` historically concatenated hook groups without
dedup (configure.py:1149-1151 docstring). The rationale ("preserve
user customizations that happen to share a matcher") held when the
existing hooks were genuinely user-authored, but broke on retrofit —
the existing hooks are CONFIGURATOR-shipped from a prior scaffold.
After N retrofits, every configurator hook fires N+1 times.
Dogfood empirically confirmed: a fresh solo-experienced scaffold has
3 PreToolUse + 3 PostToolUse + 1 Stop + 4 SessionStart hook groups;
after 3 retrofits those inflate to 12, 12, 4, 16 respectively.
Fix: hook-list merge now goes through the existing
_merge_unique_list helper (which already handles
permissions.allow/ask/deny correctly) — structural equality via
Python's `==` on dicts. Two-step merge:
1. _merge_unique_list(existing, new) — appends only what's new
2. _merge_unique_list([], merged) — collapses any prior-retrofit
duplicates already in the existing list (self-heal)
Self-healing: a user whose settings.json already accumulated N
duplicates from prior versions sees them collapse to 1 on their
next retrofit.
User customizations survive: a hook group with a different matcher,
different command, or different timeout is structurally distinct
from configurator-shipped ones and is preserved.
New `test/retrofit-hooks/` directory with 3 fixtures:
- no-op retrofit doesn't inflate (3 retrofits, counts stable)
- prior-buildup collapses on next retrofit (12→3)
- three flavors of user customization preserved across retrofit
Known limitation (out of scope): when the configurator changes a
shipped hook between releases (e.g., bumps a timeout from 5→10),
the user's old version + new version both survive structural dedup.
Rare in practice; same-release retrofits — the dominant case — are
fully fixed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tigers1997
added a commit
that referenced
this pull request
May 30, 2026
… F1+F3) (#73) deep_merge_settings deduped hook groups by whole-dict ==, so when the configurator moves a hook from a standalone matcher group to a bundled one (post-2.6.0 safety: lone block-dangerous-bash -> [block-dangerous-bash, check-package-availability]), both groups survived a retrofit and the shared command fired twice on every matching call. PR #61 had scoped this out as "rare"; the dogfood shows it's the dominant upgrade for safety-module users. - New _merge_hook_groups: key by matcher, union inner hooks[] by command; append a command only when absent from every same-matcher group. Preserves user customizations (PR #61 test still passes) + old N+1 self-heal. - F3: check_settings_validates gains _find_duplicate_hook_commands, flagging a command wired >1x under the same (event, matcher); cross-matcher allowed. - [ MERGED ] summary gains an "N hook command(s)" counter. - CI: wire test/retrofit-hooks/ and test/schema-hygiene/ into check.yml — created by PRs #60/#61 but never run in CI until now. - New TDD fixtures: test-standalone-bundled-merge.sh (fails on old code), test-duplicate-hook-commands.sh. Adversarially reviewed; non-list hooks[] append path hardened. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
Dogfood from retrofitting an adjacent project found
cc-configure --retrofitaccumulates duplicate hook entries on every run. Root cause:deep_merge_settings(configure.py:1149-1151) intentionally concatenated hook groups without dedup on the theory that user-authored hooks sharing a matcher should both run. The rationale held for one-off retrofits onto user-authored settings, but broke for the dominant case — repeated retrofits onto a prior cc-configure-shipped settings.json where the "existing" hooks are the same configurator hooks the new merge is about to add again.Empirical result: A fresh
solo-experiencedscaffold has 3 PreToolUse + 3 PostToolUse + 1 Stop + 4 SessionStart hook groups. After 3 retrofits those inflated to 12, 12, 4, 16 respectively. Every hook fires N+1 times after N retrofits.Fix
Hook merge now goes through the existing
_merge_unique_listhelper (already used forpermissions.allow/ask/denylists). Structural equality via Python's==operator on dicts. Two-step:_merge_unique_list(existing, new)— appends only what's not already present_merge_unique_list([], merged)— collapses any prior-retrofit duplicates that were already in the existing list (self-heals users who've been bitten by this bug)User customizations survive — a hook group with a different matcher, command, or timeout is structurally distinct from configurator-shipped ones and gets preserved.
Test plan
test/retrofit-hooks/)python3 configure.py --checkpassessolo-experiencedscaffold leave hook counts at baseline (3, 3, 1, 4)For the user currently affected
You restored the backup, so you're at a clean pre-retrofit state. Once this merges, the next
cc-configure --retrofitwill:So no manual cleanup needed once merged. Just re-retrofit.
Known limitation (out of scope)
When the configurator changes a shipped hook between releases (e.g., bumps a timeout from 5 → 10), the user's old version + the new version BOTH survive structural dedup. Rare in practice; same-release retrofits — the dominant case — are fully fixed. A future fix would track configurator provenance per hook group, but that conflicts with the schema-hygiene retired
//-pattern from PR #60. Captured as a CHANGELOG note for future revisit.🤖 Generated with Claude Code