Code Review: PR #423

Commit: ad0846a768a149c0edfecb03d077ee91c58186d9

Summary

This PR addresses bootstrap exemption scope creep (#419) by adding two complementary defences: (1) an explicit "Bootstrap scope" section in /handover's SKILL.md that enumerates exactly which writes the exemption covers and which it does not, and (2) a new advisory PreToolUse hook (warn-bootstrap-scope.sh) that fires on git commit when the bootstrap marker is active but the commit message doesn't reference handover output. 11 test cases cover the hook, all passing.

Checklist Results

• ✅ Architecture & Design: Pass — hook is advisory-only (exit 0 always), follows the established pattern of detect-role-trigger.sh and check-upstream-drift.sh. No domain/infrastructure boundary concerns (this is framework tooling, not application code).
• ✅ Code Quality: Pass — set -u for undefined-var safety, consistent jq -r JSON parsing, sed -nE for commit-message extraction matches the established pattern in validate-commit-format.sh. Functions are focused; the hook is 96 lines with clear single responsibility.
• ✅ Testing: Pass — 11 test cases covering: marker-present + non-bootstrap commit (warns), marker-present + each of 5 bootstrap keyword families (silent), no-marker (silent), non-Bash tool (silent), non-git command (silent), double-quoted -m (warns), and a dedicated non-blocking-guarantee assertion. Good edge-case coverage.
• ✅ Security: Pass — no secrets, no user input handling, no injection surfaces. The hook reads only from a local marker file and the tool-call JSON payload.
• ✅ Performance: Pass — the hook short-circuits early on non-Bash tools, non-git commands, and missing bootstrap markers. No external calls, no loops over files.
• ✅ PR Description & Glossary: Pass — glossary defines "Bootstrap exemption" and "Scope creep". Summary bullets are narrative with what-changed and why-it-matters.
• ✅ Summary Bullet Narrative: Pass — both bullets explain what changed and why.
• ✅ Technical Decisions (AgDR): N/A — no new dependencies, frameworks, or architecture choices introduced. The hook follows the established advisory-hook pattern already documented in the codebase.
• ✅ Adopter Handbooks: Pass — see Handbook Findings below (no violations found).

Issues Found

None.

Handbook Findings

Clean Architecture Layers — handbooks/architecture/clean-architecture-layers.md

• N/A — this PR adds framework hooks and skill documentation, not domain/application/infrastructure code.

Migration Safety — handbooks/architecture/migration-safety.md

• N/A — no migration files touched.

Commit Message Quality — handbooks/general/commit-message-quality.md

• Single commit ad0846a7: subject fix(#419): guard bootstrap exemption scope — /handover only is well-formed (type + ticket + descriptive subject). Diff is 334 lines (additions only) — a commit body explaining the "why" would be ideal per the handbook, but the PR body itself covers the rationale thoroughly. Advisory: consider adding a commit body for future git blame consumers.

Suggestions

1. suggestion: .claude/hooks/warn-bootstrap-scope.sh:56-60 — the commit-message extraction duplicates logic from validate-commit-format.sh. Consider extracting to a shared _lib-extract-commit-msg.sh helper to keep the two in sync. Non-blocking; the current implementation is correct.

2. suggestion: .claude/hooks/warn-bootstrap-scope.sh:83 — the keyword regex includes /setup, /update, and split.?portfolio for the other bootstrap skills, but the SKILL.md "Bootstrap scope" section is written specifically for /handover. If the other bootstrap skills also benefit from scope-guarding, their SKILL.md files could get equivalent sections in a follow-up. Non-blocking.

3. nit: .claude/hooks/tests/test_warn_bootstrap_scope.sh:29 — the test header comment says "6+ cases" but there are actually 11. Minor staleness in the prose.

Verdict

APPROVED

The hook is well-structured, follows established advisory-hook patterns, tests are comprehensive, and the SKILL.md documentation clearly delineates the bootstrap scope boundary. Good defensive work against a real failure mode.

🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: ad0846a768a149c0edfecb03d077ee91c58186d9