feat(#48): pre-push drift hook + branch-scan CLI subcommand#117
Merged
Knapp-Kevin merged 4 commits intoApr 29, 2026
Conversation
…plan Three phases: Phase 0 — branch-scan CLI subcommand (cli/branch_scan.py + server.py wiring); Phase 1 — setup_wizard --with-push-hook flag + .git/hooks/pre-push template; Phase 2 — CHANGELOG + user guide. Five open questions surfaced for audit: - Q1: branch-scan CLI placement — cli/branch_scan.py mirroring the cli/classify.py + cli/drift_report.py pattern from PRs BicameralAI#107 and BicameralAI#113. Wired via server.py:cli_main subparser. - Q2: existing post-commit hook may be silently no-op'ing — its `bicameral-mcp link_commit HEAD` command isn't a registered subcommand. Flagged for separate follow-up issue; NOT in scope for BicameralAI#48. - Q3: v1 = HEAD-only drift scan; multi-commit-range walk is v2. - Q4: TTY/no-TTY/no-ledger graceful behaviors specified. - Q5: setup_wizard pattern modeled exactly on existing _install_git_post_commit_hook idempotent install. Risk grade L2 (new CLI subcommand surface; modifies setup_wizard + server.py; no MCP tool changes, no schema, no contracts). Branched off BicameralAI/dev tip 77b9ee3 post-BicameralAI#113. SG-PLAN-GROUNDING-DRIFT mitigated: ran `ls -d */` and confirmed cli/ exists, setup_wizard.py exists, no fictional package/file claims in plan.
…attempt GATE TRIBUNAL entry for plan-48-pre-push-drift-hook.md. Verdict: PASS first-attempt — no remediation cycle needed. Chain hash bf890347 extends from BicameralAI#16 (BicameralAI#44 SEAL, 567170e0) on dev. Three non-blocking observations recorded: - O1: run_setup parameter-name cosmetic nit (functionally fine). - O2: latent post-commit-hook bug — bicameral-mcp link_commit HEAD is not a registered subcommand. Recommend separate issue. Out of scope for BicameralAI#48. - O3: two-renderer modules (cli/drift_report.py for PR comments vs cli/branch_scan.py for terminal hooks) accepted as parallel non-duplication; different output formats and exit semantics. SG-PLAN-GROUNDING-DRIFT instance BicameralAI#4 prevented — first plan this session where author-side grounding mitigation worked rather than audit-side catching. Issue BicameralAI#114 (CI lint enforcement) remains the durable countermeasure. Plan PASS at 79abcc2; chain to /qor-implement.
Phase 0 — branch-scan CLI subcommand: - cli/branch_scan.py (177 LOC): pure-function render_terminal_summary + main() CLI entry. Lazy-imports handle_link_commit; graceful skip when no ~/.bicameral/ledger.db. Exit codes 0/1/2 documented in module header. - server.py: add `branch-scan` subparser to cli_main, dispatch to cli.branch_scan:main. - tests/test_branch_scan_cli.py (144 LOC, 7 tests): renderer shape + exit-code matrix (no drift, drift+block-env, drift+non-TTY, etc.) Phase 1 — setup_wizard pre-push hook: - setup_wizard.py: new _GIT_PRE_PUSH_HOOK constant + _install_git_pre_push_hook function modeled on _install_git_post_commit_hook. Idempotent install with append-on-existing-hook-without-bicameral semantics. - run_setup() gains keyword-only `with_push_hook: bool = False`. Setup wizard step 7b conditionally installs the hook. - server.py setup_parser gains `--with-push-hook` flag. - tests/test_setup_pre_push_hook.py (92 LOC, 5 tests): fresh-install, idempotent re-install, append-when-non-bicameral, no-git-root path, executable-bit (POSIX-only). Phase 2 — docs: - CHANGELOG.md [Unreleased] entry - docs/guides/pre-push-drift-hook.md (129 LOC): What/When/Quickstart/ Reference/Pitfalls/See-also user guide Validation: - 11 new tests + 8 BicameralAI#49 regression = 19/20 green (1 Windows-only chmod test skipped on this platform) - ruff check + format: clean on all 5 changed/new source files - mypy on cli/branch_scan.py: no issues - End-to-end smoke: `bicameral-mcp branch-scan` dispatches via cli_main → cli.branch_scan.main → graceful skip with exit 0 (no ledger) Razor: cli/branch_scan.py 177 LOC (≤250); all entry funcs ≤25 LOC; helpers ≤20 LOC; nesting ≤2; zero nested ternaries. Maintainer-locked design (audit-PASS at META_LEDGER BicameralAI#17, chain bf890347): Q1 cli/ placement, Q2 deliberate non-modeling on possibly- broken post-commit-hook predecessor, Q3 HEAD-only v1, Q4-Q6 TTY/skip behaviors all per plan. Audit's separate-issue recommendation (post-commit hook command not registered as CLI subcommand) NOT addressed in this PR — out of scope. Closes BicameralAI#48 Plan: plan-48-pre-push-drift-hook.md (audit PASS, chain hash bf890347). Implementation chains to seal in /qor-substantiate.
Substantiation seal for plan-48-pre-push-drift-hook.md (Issue BicameralAI#48, audit PASS at META_LEDGER BicameralAI#17, chain hash bf890347). Verification gates (10 of 12 passed; 2 advisory skipped per capability shortfalls): - Reality vs Promise: ✓ all 4 new + 3 modified files exist; zero plan deviations - Test audit: 27/28 (11 new + 16 regression on PR BicameralAI#113 drift_report tests; 1 chmod test skipped on Windows non-POSIX) - Razor final: cli/branch_scan.py 177 LOC (≤250); entry funcs ≤25; helpers ≤20; nesting ≤2; zero nested ternaries - Skill file integrity: N/A (no MCP tool changes) - SYSTEM_STATE.md synced - Merkle seal computed: eacc6f89f707ce958fa2485177c9706808fdfeb32 - Step 4.6 reliability sweep: skipped (qor/reliability/ absent) - Step 7.5 version bump: skipped (per maintainer direction) This is the first implementation in the session with ZERO plan deviations — plan was thorough enough that implementation was direct. Pairs with the prior turn's first-attempt audit PASS (Entry BicameralAI#17). Both ends of the QOR cycle clean. Audit's separate-issue recommendation (post-commit hook command not a registered subcommand) tracked but NOT addressed in this PR — separate workstream. Chain: 18 entries on this branch; integrity VALID. Next: /qor-document.
Knapp-Kevin
added
enhancement
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Knapp-Kevin
added a commit
to Knapp-Kevin/bicameral-mcp
that referenced
this pull request
May 21, 2026
Phase 0 — plan-grounding lint (Check A, blocking): - scripts/lint_plan_grounding.py (212 LOC, 8 tests): walks plan-*.md for backtick-wrapped path tokens, verifies each resolves on the working tree. Markers (**new**, (planned)/(future)/(v2)/ (nonexistent)/(example)) exempt the line. Skips HTML comments, blockquotes, multi-word tokens, glob patterns. - .github/workflows/lint-and-typecheck.yml: new step lints only plan-*.md files modified in the current PR (git diff against base ref). Blocks merge if any modified plan has unresolved paths. Phase 1 — PR-body refs lint (Check B, advisory): - .github/scripts/lint_pr_body_refs.py (162 LOC, 6 tests): warns on bare #NUMBER mentions outside Closes/Refs/etc keywords or Linked-issues section. SECURITY: --from-env PR_BODY reads via os.environ directly (no shell interpolation; OWASP A03 mitigation per BicameralAI#114 audit v1). - .github/workflows/pr-body-refs-lint.yml: advisory workflow on pull_request: [opened, edited, reopened]. continue-on-error: true. Phase 2 — CI integration (above). Phase 3 — docs: - docs/DEV_CYCLE.md §2.1: plan-grounding lint callout - docs/DEV_CYCLE.md §4.3: PR-body keyword discipline (Closes/Refs/ Linked-issues section) - CHANGELOG.md [Unreleased] entry Validation: - 29/29 tests green (8 + 6 + 15 regression on BicameralAI#117/BicameralAI#113) - Self-test: scripts/lint_plan_grounding.py against plan-114-grounding-lint.md exits 0 (clean) — the plan that builds the lint passes the lint - ruff check + format: clean on all 4 new files - mypy: clean on both new modules - Razor: scripts/lint_plan_grounding.py 212 LOC; .github/scripts/ lint_pr_body_refs.py 162 LOC; entry funcs ≤30 LOC; helpers ≤25. Plan: plan-114-grounding-lint.md (audit v1 VETO at a5e6a05 on OWASP A03; v2 PASS at 4ea06be after --from-env remediation; chain bf890347 → 850ec57f). Implementation chains to seal in /qor-substantiate. Closes BicameralAI#114
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
bicameral-mcp branch-scanconsole subcommand prints a terminal-friendly summary of drifted decisions for HEAD. Callslink_commitunder the hood; exit codes (0/1/2) drive the pre-push hook's prompt-or-block logic.bicameral-mcp setup --with-push-hook. Surfaces drift warnings beforegit pushcompletes — when the developer can still amend or annotate. Non-blocking by default (TTY prompt or non-TTY warn-only);BICAMERAL_PUSH_HOOK_BLOCK=1forces hard-block.~/.bicameral/ledger.dbexists. Hook short-circuits on[ -d .bicameral ] || exit 0first line, so repos without bicameral configured see zero impact.cli/branch_scan.pyis 177 LOC pure functions.Linked issues
Closes #48
Refs:
cli/drift_report.pyandcli/branch_scan.pyare parallel renderer modules in the same package; audit accepted as non-duplication (different output formats, different exit-code semantics).cli/as canonical home for CLI sub-tools. This PR uses the same pattern.Plan / Audit / Seal
plan-48-pre-push-drift-hook.md(commit79abcc2, content hashsha256:96045a11…)bf890347…. SG-PLAN-GROUNDING-DRIFT instance refactor: BicameralContext — request-scoped snapshot isolation #4 prevented at author-time vials -d */grounding before submission.sha256:eacc6f89…. Reality matches Promise; zero plan deviations. First impl in this session where both the audit and the substantiation came back clean on first attempt.Test plan
pytest tests/test_branch_scan_cli.py -q(7/7 — Phase 0 renderer + exit-code matrix)pytest tests/test_setup_pre_push_hook.py -q(5/5 — Phase 1 install/idempotent/permissions; 1 chmod test skipped on Windows non-POSIX, runs on Linux CI)pytest tests/test_drift_report_*.py -q(16/16 — regression on PR feat(#49): sticky PR-comment drift report — GitHub Action + renderer + poster #113's drift-report renderer/poster/integration; both modules sit incli/and need to coexist cleanly)ruff check cli/branch_scan.py setup_wizard.py server.py tests/test_branch_scan_cli.py tests/test_setup_pre_push_hook.py— cleanruff format --check— cleanmypy cli/branch_scan.py— no issuespython -m server branch-scandispatches viacli_main→cli.branch_scan.main→ graceful skip with exit 0 (no ledger configured locally). Confirmed the dispatch chain works.bicameral-mcp setup --with-push-hook,git push, observe warning + prompt, decline → push aborts; accept → push proceeds. Non-TTY (git push 2>&1 | cat) does not block. Qualitative gate, not CI-blocking.What this PR is NOT
bicameral-mcp link_commit HEADinvocation. Audit identified this as a latent bug (link_commitis not a registeredcli_mainsubcommand; the hook silently no-ops under|| true). Out of scope for Pre-push git hook: surface drift warnings before git push #48; recommended follow-up issue.local_sha/remote_sharange on stdin; multi-commit walk is a v2 enhancement.Soft dependencies
cli/drift_report.py(PR feat(#49): sticky PR-comment drift report — GitHub Action + renderer + poster #113) andcli/branch_scan.py(this PR) are sibling renderer modules. Audit accepted them as parallel non-duplication; only two consumers, different output shapes, sharing a common formatter would be premature abstraction.decision_level = NULLrows #77) — withoutcli/as a real package, this PR would have needed to invent a new top-level package (caught at audit v1 of GitHub Action: sticky PR-comment drift report on open/update #49 as F-1 / SG-PLAN-GROUNDING-DRIFT instance fix: ingest pipeline — input contracts, payload normalization, freshness guards #3) or live at root level.Workflow security
/dev/ttyfor thePush anyway? [y/N]prompt. Input matched against fixed regex[yY]|[yY][eE][sS]; no shell expansion; no command construction from user input.bicameral-mcp branch-scanfromPATH— same trust model as the existing post-commit hook (and any other system tool).0o755(executable, world-readable). No secrets in hook content.[ -d .bicameral ] || exit 0) when no ledger configured — zero impact on repos without bicameral.pull_request_targettriggers introduced.Files changed (8)
cli/branch_scan.pytests/test_branch_scan_cli.pytests/test_setup_pre_push_hook.pydocs/guides/pre-push-drift-hook.mdsetup_wizard.pyserver.pyCHANGELOG.md[Unreleased]entry added)plan-48-pre-push-drift-hook.mdPlus governance (META_LEDGER #17 + #18 on this branch's chain, SYSTEM_STATE.md #48 snapshot).
Reviewer ask
@jinhongkuan — risk grade L2 (new CLI subcommand surface; modifies setup_wizard + server.py; consumes existing
handle_link_commitunchanged). Single-reviewer gate perDEV_CYCLE.md§4.4.Three specific decisions worth a sanity check:
Q1 placement — renderer lives in
cli/branch_scan.py(sibling ofcli/drift_report.pyandcli/classify.py). Audit accepted as the natural home for CLI-invoked tools. If you'd prefer atools/or root-level location, this is the place to say so.Q2 latent bug flagged — the existing post-commit hook command
bicameral-mcp link_commit HEADdoesn't appear to be a registeredcli_mainsubcommand. The hook's|| trueswallows the argparse error, which means the post-commit hook may be silently no-op'ing today. This PR deliberately doesn't model on that pattern;branch-scanis registered correctly. If you want me to file a separate issue for the post-commit hook bug, say so and I'll do it post-merge.Q3 scope — v1 is HEAD-only. Multi-commit-range push walk is a documented v2 enhancement. If you'd rather have multi-commit from day one, this PR can be expanded; otherwise the v2 issue files itself naturally when someone hits the limitation.