Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ All notable changes to this project. Format: [Keep a Changelog](https://keepacha

## Unreleased

- **fix(retrofit): dedupe hook groups so repeated `cc-configure --retrofit` stops inflating settings.json.** `deep_merge_settings` historically concatenated hook groups onto the existing list 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 — where 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. Self-healing: a user whose settings.json already accumulated N duplicates from prior versions sees them collapse back to 1 on their next retrofit (the merged list gets re-deduped against itself). 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; prior-buildup collapses on next retrofit; three flavors of user customization all 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. Future fix would track configurator provenance per hook group (e.g., a marker field), but that conflicts with the schema-hygiene retired `//` pattern from PR #60.
- **fix(schema-hygiene): retire //-stub pattern + drop unvalidatable skillOverrides default + add settings preflight.** Dogfood from upgrading an adjacent project surfaced two real validator-rejection bugs introduced by PRs #17, #18, #56, and #57: (1) `templates/token-efficiency/settings-patch.tier-pro.json` shipped `"skillOverrides": "name-only"` (string form), but the current Claude Code schema requires the per-skill object map (`{"skill-name": "name-only"}`) and rejects the string — `name-only` was a legacy/imagined "apply to all skills" form that the official doc never sanctioned; further, per code.claude.com/docs/en/settings the setting doesn't apply to plugin skills at all, narrowing its usefulness as a tier-wide default; (2) the `// foo` "commented opt-in stub" pattern (`// sandbox`, `// env`, `// prUrlTemplate`, `// worktree`, `// subagentStatusLine`, `// hideVimModeIndicator`) propagated as literal top-level keys into the user's `.claude/settings.json`, where Claude Code's editor schema validator flags them as unknown properties. The design intent (PR #56's `_is_doc_label` filter) was to keep `// foo` stubs and strip only bare `//` / `//N` doc labels; the dogfood proves the intent was wrong. **Fixes:** `_is_doc_label` now strips ALL `//`-prefixed keys; new `_strip_doc_labels` recursively scrubs the merged settings before render (catches nested cases like `statusLine.// hideVimModeIndicator` that the shallow per-merge filter missed); `extraSettings` merge path now applies the filter (was previously bypassed — UI module's bare `"//"` was leaking too); `skillOverrides: "name-only"` deleted from tier-pro (replaced with a docstring explaining why no default ships); all `// foo` stubs deleted from source patch files (`safety/settings-patch.json`, `multi-agent/settings-patch.json`, `git-workflow/settings-patch.json`) and from the ui module's inline `extraSettings`; opt-in discovery moved to `templates/core/dot-claude/settings.local.json.example` (the `.example` suffix means Claude Code doesn't parse it directly, so `// foo` stubs there are safe). New `check_settings_validates()` preflight catches both bug classes at scaffold time + emits a `[ SETTINGS WARNINGS ]` block; new static `--check` step asserts no settings-patch ships `skillOverrides` as a non-object (regression guard, caught the current bug when run for the first time). New test directory `test/schema-hygiene/` with 4 fixtures: cross-persona no-leak assertion, source-patch shape assertion, preflight-detects-violations (4 violation classes + clean case), recursive `_strip_doc_labels` invariant. Compat note: users on cc-configure 2.6.0 + a populated `.claude/settings.json` get the cleanup on next `cc-configure --retrofit` — the deep-merge preserves their customizations and the new strip removes the stub-leaks.
- **feat: discipline-skills module — curated 7-skill subset forked from obra/superpowers v5.1.0.** New `templates/discipline-skills/` module ships seven discipline skills as project-level `.claude/skills/<name>/SKILL.md`: `brainstorming`, `writing-plans`, `executing-plans`, `verification-before-completion`, `using-git-worktrees`, `subagent-driven-development`, `finishing-a-development-branch`. Forked verbatim from the MIT-licensed upstream plugin with three surgical edits: visual-companion section stripped from `brainstorming` (the upstream's browser-based companion server is omitted — text-only mode is the default fallback anyway); `superpowers:` prefix stripped from every inter-skill cross-reference so they resolve correctly as project-level skills; the `requesting-code-review` template embedded inline into `subagent-driven-development/code-quality-reviewer-prompt.md` and the cosmetic `test-driven-development` companion line dropped from `subagent-driven-development/SKILL.md`'s Integration section. Module includes a slim SessionStart bootstrap (`hooks/sessionstart-discipline.sh`) that primes the model with a ~400-token seven-skill overview (vs. upstream's ~1,200-token `using-superpowers` injection) and auto-suppresses when the upstream `superpowers` plugin is also installed (detects `~/.claude/plugins/cache/claude-plugins-official/superpowers/`). Wired into `config_schema.py` with `extraSettingsHook` for the SessionStart registration and into the `solo-newer`, `solo-experienced`, `small-team` personas by default. Upstream MIT LICENSE ships at `.claude/skills/_LICENSE-discipline-skills.md`; attribution to Jesse Vincent (© 2025) added under README `## Acknowledgments`. New `/verify-setup` Check #12 flags duplicate installation (both the configurator's module and the upstream plugin present) so users can pick one. New `docs/10-plugin-ecosystem.md` section "Discipline skills: bundled vs. upstream plugin" documents the trade-off (~930 tokens saved per session vs. losing access to the 7 unused upstream skills) and the maintainer-internal `templates/discipline-skills/SYNC.md` documents the upstream-sync workflow. Recommend-plugins copy reframes the upstream `superpowers` row as the "full-suite alternative" rather than the configurator's first-choice recommendation.

Expand Down
21 changes: 16 additions & 5 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1146,9 +1146,16 @@ def deep_merge_settings(existing: dict, new: dict):
existing entries first, new entries appended without duplicates.
- permissions.disableBypassPermissionsMode: ours wins (security default
the user opted into by selecting safety).
- hooks: concatenate per-event groups (existing first, then ours). No
dedupe — if the user has a hook group with the same matcher, both
run; user can manually remove duplicates if undesired.
- hooks: per-event groups merged via _merge_unique_list (existing
first, then ours, structural-equality dedup). Critical for
retrofits: without dedup, every cc-configure --retrofit run
re-appends the configurator's own hook set against the prior
scaffold's identical set, so after N retrofits every hook fires
N+1 times. User-customized hook groups (different matcher,
different command list, different timeout) are structurally
distinct from configurator-shipped ones and survive dedup.
Self-heals existing buildup: a user whose settings.json already
accumulated N duplicates collapses them to 1 on next retrofit.
- env: dict merge with existing keys winning on collision (preserves
user's deliberate overrides).
- statusLine, model: preserve existing if set; otherwise use new.
Expand Down Expand Up @@ -1177,8 +1184,12 @@ def deep_merge_settings(existing: dict, new: dict):
out_hooks = dict(out.get("hooks", {}))
for event, new_groups in new["hooks"].items():
existing_groups = out_hooks.get(event, [])
out_hooks[event] = list(existing_groups) + list(new_groups)
counts["hook_groups_added"] += len(new_groups)
merged = _merge_unique_list(existing_groups, new_groups)
# Also collapse any prior-retrofit duplicates already in
# existing_groups (self-heal). Reapply unique-list against itself.
merged = _merge_unique_list([], merged)
counts["hook_groups_added"] += len(merged) - len(existing_groups)
out_hooks[event] = merged
out["hooks"] = out_hooks

if "env" in new:
Expand Down
51 changes: 51 additions & 0 deletions test/retrofit-hooks/test-no-duplication-on-retrofit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env bash
# Repeated cc-configure --retrofit on the same project must NOT accumulate
# duplicate hook entries. Demonstrates + guards against the dogfood-reported
# bug (configure.py:1149-1151 historically concatenated without dedup, so
# every retrofit re-appended the configurator's own hook set; after N
# retrofits each hook fired N+1 times).
set -euo pipefail

tmp=$(mktemp -d)
trap "rm -rf $tmp" EXIT

# Initial scaffold (writes .claude/settings.json with the configurator's hooks)
python3 configure.py --persona solo-experienced --yes --dir "$tmp" >/dev/null

# Count hook entries in each event after the initial scaffold
count_hooks() {
local event="$1"
python3 -c "
import json
data = json.load(open('$tmp/.claude/settings.json'))
print(len(data.get('hooks', {}).get('$event', [])))
"
}

initial_pretool=$(count_hooks PreToolUse)
initial_posttool=$(count_hooks PostToolUse)
initial_stop=$(count_hooks Stop)
initial_sessstart=$(count_hooks SessionStart)

# Three more retrofits — same persona, same target — should be no-ops for hooks.
for i in 1 2 3; do
python3 configure.py --persona solo-experienced --yes --dir "$tmp" >/dev/null
done

after_pretool=$(count_hooks PreToolUse)
after_posttool=$(count_hooks PostToolUse)
after_stop=$(count_hooks Stop)
after_sessstart=$(count_hooks SessionStart)

fail=0
[ "$after_pretool" = "$initial_pretool" ] \
|| { echo "FAIL: PreToolUse: $initial_pretool → $after_pretool after 3 retrofits"; fail=1; }
[ "$after_posttool" = "$initial_posttool" ] \
|| { echo "FAIL: PostToolUse: $initial_posttool → $after_posttool after 3 retrofits"; fail=1; }
[ "$after_stop" = "$initial_stop" ] \
|| { echo "FAIL: Stop: $initial_stop → $after_stop after 3 retrofits"; fail=1; }
[ "$after_sessstart" = "$initial_sessstart" ] \
|| { echo "FAIL: SessionStart: $initial_sessstart → $after_sessstart after 3 retrofits"; fail=1; }

[ "$fail" -eq 0 ] || exit 1
echo "PASS: 3 retrofits don't duplicate hook entries (PreToolUse=$initial_pretool, PostToolUse=$initial_posttool, Stop=$initial_stop, SessionStart=$initial_sessstart, stable across retrofits)"
51 changes: 51 additions & 0 deletions test/retrofit-hooks/test-self-heal-prior-buildup.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env bash
# A user who upgraded across several pre-fix releases may have a settings.json
# with N duplicate hook entries already. On their NEXT retrofit, the new
# dedup logic must collapse the prior buildup back to 1 entry per group.
set -euo pipefail

tmp=$(mktemp -d)
trap "rm -rf $tmp" EXIT

# Initial scaffold
python3 configure.py --persona solo-experienced --yes --dir "$tmp" >/dev/null

# Simulate prior-version buildup: manually duplicate every hook group 3 times.
python3 -c "
import json
p = '$tmp/.claude/settings.json'
data = json.load(open(p))
for event, groups in data.get('hooks', {}).items():
data['hooks'][event] = groups * 4 # 1 + 3 dupes = 4 total
json.dump(data, open(p, 'w'), indent=2)
"

# Sanity check: file now has inflated counts
inflated_pretool=$(python3 -c "import json; print(len(json.load(open('$tmp/.claude/settings.json'))['hooks']['PreToolUse']))")
[ "$inflated_pretool" -gt 3 ] || { echo "FAIL: test setup didn't actually inflate"; exit 1; }

# One retrofit — should collapse the inflated counts back to the baseline.
python3 configure.py --persona solo-experienced --yes --dir "$tmp" >/dev/null

healed_pretool=$(python3 -c "import json; print(len(json.load(open('$tmp/.claude/settings.json'))['hooks']['PreToolUse']))")
healed_posttool=$(python3 -c "import json; print(len(json.load(open('$tmp/.claude/settings.json'))['hooks']['PostToolUse']))")
healed_stop=$(python3 -c "import json; print(len(json.load(open('$tmp/.claude/settings.json'))['hooks']['Stop']))")
healed_sessstart=$(python3 -c "import json; print(len(json.load(open('$tmp/.claude/settings.json'))['hooks']['SessionStart']))")

fail=0
# Expected baseline counts come from a fresh solo-experienced scaffold.
expected_pretool=3
expected_posttool=3
expected_stop=1
expected_sessstart=4
[ "$healed_pretool" = "$expected_pretool" ] \
|| { echo "FAIL: PreToolUse: inflated to $inflated_pretool, healed to $healed_pretool (expected $expected_pretool)"; fail=1; }
[ "$healed_posttool" = "$expected_posttool" ] \
|| { echo "FAIL: PostToolUse: healed to $healed_posttool (expected $expected_posttool)"; fail=1; }
[ "$healed_stop" = "$expected_stop" ] \
|| { echo "FAIL: Stop: healed to $healed_stop (expected $expected_stop)"; fail=1; }
[ "$healed_sessstart" = "$expected_sessstart" ] \
|| { echo "FAIL: SessionStart: healed to $healed_sessstart (expected $expected_sessstart)"; fail=1; }

[ "$fail" -eq 0 ] || exit 1
echo "PASS: inflated $inflated_pretool entries collapsed to baseline $expected_pretool on next retrofit"
75 changes: 75 additions & 0 deletions test/retrofit-hooks/test-user-customizations-preserved.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
#!/usr/bin/env bash
# Dedup must not destroy genuine user customizations. A user-added hook
# group with a different matcher (or different command, or different
# timeout) is structurally distinct from configurator-shipped ones and
# must survive a retrofit unchanged.
set -euo pipefail

tmp=$(mktemp -d)
trap "rm -rf $tmp" EXIT

python3 configure.py --persona solo-experienced --yes --dir "$tmp" >/dev/null

# Inject three user customizations:
# 1. A wholly new hook group (different matcher than anything we ship)
# 2. A hook with same matcher as a shipped one but a different command
# 3. A timeout-tweaked duplicate of a shipped hook (different value)
python3 -c "
import json
p = '$tmp/.claude/settings.json'
data = json.load(open(p))
data['hooks'].setdefault('PreToolUse', []).extend([
{
'matcher': 'WebSearch',
'hooks': [{'type': 'command', 'command': '/usr/local/bin/log-websearch.sh', 'timeout': 5}]
},
{
'matcher': 'Bash',
'hooks': [{'type': 'command', 'command': '/home/user/scripts/my-custom-bash-guard.sh', 'timeout': 10}]
},
])
# Tweak the timeout of an existing shipped hook to simulate a customization
for group in data['hooks']['PreToolUse']:
if group.get('matcher') == 'Bash' and 'block-dangerous-bash' in str(group):
new_group = json.loads(json.dumps(group))
new_group['hooks'][0]['timeout'] = 999 # user bumped the timeout
data['hooks']['PreToolUse'].append(new_group)
break
json.dump(data, open(p, 'w'), indent=2)
"

# Capture the user-added entries
before_user_websearch=$(python3 -c "
import json
data = json.load(open('$tmp/.claude/settings.json'))
print(any(g.get('matcher') == 'WebSearch' for g in data['hooks']['PreToolUse']))
")
[ "$before_user_websearch" = "True" ] || { echo "FAIL: test setup didn't add WebSearch hook"; exit 1; }

# Run retrofit
python3 configure.py --persona solo-experienced --yes --dir "$tmp" >/dev/null

# All three customizations must survive
after_user_websearch=$(python3 -c "
import json
data = json.load(open('$tmp/.claude/settings.json'))
print(any(g.get('matcher') == 'WebSearch' for g in data['hooks']['PreToolUse']))
")
after_user_custom_bash=$(python3 -c "
import json
data = json.load(open('$tmp/.claude/settings.json'))
print(any('my-custom-bash-guard' in str(g) for g in data['hooks']['PreToolUse']))
")
after_user_timeout=$(python3 -c "
import json
data = json.load(open('$tmp/.claude/settings.json'))
print(any('block-dangerous-bash' in str(g) and any(h.get('timeout') == 999 for h in g.get('hooks', [])) for g in data['hooks']['PreToolUse']))
")

fail=0
[ "$after_user_websearch" = "True" ] || { echo "FAIL: WebSearch hook lost after retrofit"; fail=1; }
[ "$after_user_custom_bash" = "True" ] || { echo "FAIL: custom-bash-guard hook lost after retrofit"; fail=1; }
[ "$after_user_timeout" = "True" ] || { echo "FAIL: timeout=999 customization lost after retrofit"; fail=1; }

[ "$fail" -eq 0 ] || exit 1
echo "PASS: 3 user customizations (new matcher, different command, tweaked timeout) all preserved across retrofit"
Loading