Repository navigation
chore: add git hooks that refuse commits on main and unsafe pushes - #1172
Conversation
pre-commit refuses a commit on main before any test step, in linked worktrees too. pre-push reads the refs git passes on stdin and refuses a push to main, a non-fast-forward update, and a push under another branch name unless remote.<name>.push maps it. cluckwork.hookTests=false turns the test steps off without turning off the guard.
|
Review round 1 (Codex Every finding below was reproduced with Git 2.53.0 against temporary local repositories and bare remotes.
Contrived, kept as PR notes:
Verified: removing any one of the four core checks turns the table red. Linked worktrees work, including with explicit |
A tag update passes the commit-ancestry check when the tag moves to a descendant or is recreated at the same commit. Any change to an existing refs/tags/* now refuses before that check; new tags still pass. The table compares every remote ref around a refused push.
remote.<name>.push = HEAD:<dst> exempted any source pushed to <dst>, so git push paseo feat/unrelated:refs/heads/feat/y passed. The HEAD form now applies only when the pushed ref is the checked-out branch.
A remote.<name>.push mapping is matched after stripping a leading + (the history check still refuses a forced update) and reading short names as branches, so feat/x:feat/y maps like refs/heads/feat/x:refs/heads/feat/y. Wildcard refspecs stay unsupported and refuse.
git sends pre-push no record when --mirror deletes a remote ref the clone lacks, so a mirror push can delete main unseen. The hook header and AGENTS.md say so and name the layers that refuse it; a test pins the current behaviour so a git change that starts sending the record is noticed.
symbolic-ref status 1 is a detached HEAD; any other failure used to read as detached and let the commit through. It now refuses.
# Conflicts: # AGENTS.md
Head is now
Mutation check: I disabled each of 14 rules one at a time. All 14 turned the table red and went green after revert. The first run of the "guard before Table: 29 pushes (10 allowed, 19 refused, each refusal checking that no remote ref changed), 10 commit cases and the mirror known-limit test, all passing. After the merge, #1158's guard table also passes (161 × 2). The maintainer's three answers are in the body: |
|
Review round 2 (Codex All round 1 repros are closed or documented. The table passes (29 pushes, 10 commits, plus the mirror-limit case), and seven mutations turned it red. Normal
Contrived (PR note): pre-push still treats a branch-lookup error as allowed. |
A detached HEAD set current to the literal HEAD, which matched a configured HEAD:<dst> mapping, so git push paseo published detached work. current is now empty when detached, the local ref stays HEAD, and the HEAD mapping needs the checked-out branch. A branch lookup that fails for any other reason refuses, as in pre-commit.
Fixed in
Mutation check on the new lines: three mutations, all red and all green again after revert: restoring round 2's generic Table: 30 pushes, the detached-worktree and lookup-failure tests, 10 commit cases and the mirror known-limit test, all passing. |
|
Round 2 fix verified at
The review loop stops here. Round 1 found 4 realistic defects and round 2 found 1; all are fixed or documented. |
Part of #1171 (the Pi and Hermes wiring follows in its own PR, which closes it)
.githooks/pre-commitand a new.githooks/pre-pushback up the agent git guard (#1158) for every harness and for people. They work from what git itself knows, the checked-out branch and the refs being pushed, so no command line is parsed.pre-commitrefs/heads/mainis checked out. This runs first, beforeSKIP_HOOKSand before any test step. A branch lookup that fails for any reason other than a detached HEAD also refuses.git switch -c <type>/<topic>, then commitpre-pushrefs/heads/main, including deletesgit switch -c <type>/<topic>, then push that branchpre-pushrefs/tags/*whose sha changes, even to a descendant or the same commitpre-push--force,+refspecand--force-with-leasealike, and a remote that has commits you lack.pre-pushsrc:dstmistake behind #900).HEADis resolved to the checked-out branch first.git branch --show-current, thengit push -u origin <that-branch>Allowed: pushing your branch (
git push,git push -u origin HEAD,git push origin <branch>), new tags, and deleting a branch other than main. A name mismatch is also allowed when aremote.<name>.pushrefspec maps that source. Short names in the refspec read as branches, and a leading+is ignored here because the history check still refuses a forced update. AHEAD:<dst>mapping covers only the checked-out branch. Paseo PR checkouts push that way, from a suffixed local branch (remote.paseo-pr-934.push = HEAD:refs/heads/feat/828-…). Wildcard refspecs are not supported and refuse.Turning it on (the maintainer runs this once)
core.hooksPathis unset on this machine today. The setting lives in the shared repository config, so it also applies to every linked worktree under.cluckwork-slices/and.claude/worktrees/. Because the path is relative, each worktree runs the.githooks/of its own checkout. A branch cut before this PR merges has nopre-pushuntil it mergesmain.It also turns on the existing hooks: the pre-commit test tripwire (domain and application unit tests on staged
.cs,npm run typecheckand lint on stagedweb/) andcommit-msg. To keep the guard without the test steps for every agent, also run:git -C /home/mforce/dev/cluckwork config cluckwork.hookTests falseDecided by the maintainer
pre-pushrefuses--force-with-leasealong with--force; a hook cannot tell them apart, and the rules forbid force pushes either way. chore: block forbidden agent git commands with a PreToolUse hook #1158's command guard still allows the lease form; this hook is the stricter layer.remote.<name>.pushexception stays, limited to the checked-out branch.Pi and Hermes (for the follow-up)
Both harnesses can block a shell command before it runs. Until the follow-up lands, only these git hooks cover them, and nothing stops their
gh pr merge.docs/extensions.mdin the installed package (@earendil-works/pi-coding-agent): "tool_callcan mutate input or block execution", returning{ block: true, reason }. "Atool_callhandler failure blocks the tool as a fail-safe."docs/configuration.md: project extensions live in.pi/extensions/and load "after project trust is granted".website/docs/user-guide/features/hooks.mdandagent/shell_hooks.py: "Apre_tool_callhook that exits with code 2 blocks the tool call". Its payload is{"tool_name": "terminal", "tool_input": {"command": …}, "cwd": …}. Shell hooks are declared only in the profile's~/.hermes/config.yaml, which this repo cannot carry. A project plugin under.hermes/plugins/can register the samepre_tool_call, buthermes_cli/plugins_discovery.pyscans it only withHERMES_ENABLE_PROJECT_PLUGINSset, and plugins also needplugins.enabled.git_guard.pywould also need to accepttool_name: "terminal".Limits and notes for review
--mirrorpush can delete a remotemainunseen. Git sends pre-push no record for a remote ref the clone lacks, so the hook never sees it. chore: block forbidden agent git commands with a PreToolUse hook #1158's command guard refuses--mirror, and GitHub's protection ofmainrefuses the deletion.KnownLimitTestpins the current behaviour (exit 0, remotemaindeleted on a bare remote that allows it). If a future git sends the record, that test fails, and it should then become a refusal row.--no-verifyskips both hooks. Git offers no way to prevent that. chore: block forbidden agent git commands with a PreToolUse hook #1158's harness guard is the layer that sees the command line.pre-commitdoes not run for merge commits.git mergeon main creates one throughpre-merge-commit, which this PR does not add. Rebase and cherry-pick onto main are likewise uncovered; the pre-push main check still stops the result from leaving the machine.git symbolic-ref --quiet HEAD. Under a hook, git exports the worktree's ownGIT_DIR, so HEAD is the worktree's branch. The table proves this both ways.classifier-self-test(noneeds, never gated) next to chore: block forbidden agent git commands with a PreToolUse hook #1158's guard test.Tests run locally
python3 tools/agent-guard/git_hooks_test.py: 29 pushes against a fresh local bare repo each (10 allowed, 19 refused; every refusal checks that no remote ref changed), 10 commit cases, and the mirror known-limit test. All pass.python3 tools/agent-guard/git_guard_test.py(chore: block forbidden agent git commands with a PreToolUse hook #1158's, after mergingmain): 161 commands × 2 payload shapes, all pass.dotnet testfiltered toTenancyDocsFreshnessTestsandRenameAccountDocsTests(19 passed), plus theImagePinandCommittedSchemaDocs_CarryNoEnvironmentSpecificContenttracked-file guards (3 passed).Proof at
a4ce288aMutation table
Each rule was disabled on its own, the table was run (red expected), then the hook was restored and the table re-run (green expected). Run at
f0a59151; mergingmainchanged neither hook nor the table test.Live refused push
core.hooksPathset to this branch's.githooksin a scratch clone of a fresh local bare repo (~/.cache/gh-hooks/live.py):Table test output at
a4ce288a