hooks: full-suite pre-push test gate (both unittest roots, git-env sanitized) - #86
Conversation
Fail-fast both-roots test gate before the compliance-trace check; fail-closed opt-out (exact AGENT_COLLAB_PREPUSH_TESTS=0 only, loud warning, compliance always runs via exec preserving stdin); main-only skip with detection-failure running tests; 7 wrapper-level test cases. Closes prevention debt for ledger partial.suite.run.hides.ci.failure in this repository. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c985edf99f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ( cd "$REPO_ROOT" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX && \ | ||
| python3 -m unittest discover -s tests -t . -q < /dev/null ) || { |
There was a problem hiding this comment.
Test the objects named by the push
When the working tree contains uncommitted changes, these commands test the mutable checkout rather than the <local-object-name> supplied by the pre-push hook input. An uncommitted fix can therefore make both suites pass while the still-failing committed snapshot is pushed, recreating the red-CI failure this gate is intended to prevent; test a detached worktree for the pushed object or reject a dirty checkout.
Useful? React with 👍 / 👎.
| branch="$(git -C "$REPO_ROOT" symbolic-ref --quiet --short HEAD 2>/dev/null)" | ||
| if [ "$branch" = "main" ]; then | ||
| run_tests=0 |
There was a problem hiding this comment.
Decide the main exemption from pushed refs
When HEAD is main, an explicit git push origin feature-x or git push --all reaches this condition and disables the test gate even though a feature ref is being sent. The pre-push contract provides every pushed local and remote ref on stdin, so the exemption should be based on those records rather than the currently checked-out branch; otherwise this bypasses the fail-closed gate without the opt-out warning.
Useful? React with 👍 / 👎.
Defense-in-depth follow-up to #86: any runner of the tests/ suite is now safe from hook-inherited GIT_DIR poisoning, not just the sanitized pre-push wrapper. Regression tests pin in-process absence and a freshly-poisoned child-import scrub. scripts/ root verified unexposed (327/327 under poisoned GIT_DIR, zero config writes) and left as-is. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Operator-directed guard install: the tracked
.githooks/pre-pushwrapper now runs both unittest roots (tests/658 +scripts/327, mirroring CI's exact discovery invocations) as a fail-fast gate before the existing compliance-trace check. Closes the prevention gap behind ledgerpartial.suite.run.hides.ci.failure(recurred on #83, where one complete root was run without knowing the second existed).Design (all five cross-check concerns integrated):
AGENT_COLLAB_PREPUSH_TESTS=0skips the test gate (loud warning; unset/empty/other values run tests); the compliance check always runs and is nowexec'd so the hook's stdin ref data reaches it.git symbolic-ref --quiet --short HEAD; detached HEAD or detection failure runs the tests.unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX. Without this, git's exported hook environment made temp-dir git operations inside the tests target the real repository — the first (pre-fix) blocked push setcore.bare=trueand a test-fixtureuser.emailin the live.git/config(repaired; damage limited to those two keys; ledgerplugin.tests.inherit.git.hook.env.mutate.real.repo, verified, with repair recipe).scripts/test_hook_pre_push_wrapper.py): both-suites order + compliance chain, either-suite failure blocks, exact-opt-out semantics, non-exact values run tests, main-skip, detection-failure-runs, and hook-env sanitization (suites seeGIT_DIRunset; the exec'd compliance checker keeps the hook env).Candidate follow-up (not in this PR): sanitize the git env in the test suites' own setUp so every runner is safe, not just this hook.
Validation
unittest discover -s scripts327/327 OK (incl. 8 new);-s tests -t .658/658 OK; changelog dry-run OK; pre-commit consistency PASS; push traversed the new gate successfully.Cross-check (verbatim)
Codex (
gpt-5.6-sol, effortlow, directcodex execunder operator broker-bypass authorization):Concerns: (1) fail-closed opt-out — integrated; (2) robust main detection with detached/failure running tests — integrated; (3) fail-fast ordering +
execpreserving stdin — integrated; (4) wrapper-level tests for the five behaviors — integrated (8 cases); (5) match CI's authoritative unittest surface — verified against.github/workflows/ci.ymland mirrored exactly.author: claude
author_model: claude-fable-5 (effort: default)
standing_directives: operator-directed guard (2026-08-03 "Install the plugin-repo pre-push guard"); directive #9 prevention-debt closure; #7 model+effort stated; #8 no fan-out
tier: 2
cross_check: PROCEED-WITH-MODIFICATIONS (codex gpt-5.6-sol, effort low, direct codex exec under operator broker-bypass authorization 2026-08-03; raw verdict quoted in PR body; all 5 concerns integrated)
post_condition: 327/327 + 658/658 tests OK; changelog dry-run OK; consistency PASS; new gate dogfooded on this branch's own push; git-config poisoning incident repaired and ledgered with recipe
mcp_coverage_gap: none — repo tooling; gemini_mcp_path: retired / not applicable
contributor_rights: OWNER-AUTHORED
operator_reserved: no
plugin_affected: NONE — hook tooling, no distributed plugin content
readme_refresh_status: README unaffected (no release, no version bump)
🤖 Generated with Claude Code