feat(pre-commit): add a pre-push PR guard to the catalog suite #1146
No reviewers
Labels
No labels
burndown-2026-06
burndown-2026-08
autonomy
async-consult
autonomy
epic
autonomy
headless
autonomy
live-collab
coherence-core
priority
P0
priority
P1
priority
P2
priority
P3
priority
P4
qa-fixture
role/advocate
role/director
role/exec
role/frontend
role/gamedev
role/human
role/platform
role/qa
role/science
role/sysadmin
state
ambient
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
coilyco-flight-deck/agentic-os!1146
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/pr-guard-pre-push"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Keeps the default branch PR-only from the client side. Two rules:
credentials, works against any forge.
allowing further pushes to it. Forgejo API, gated on FORGEJO_TOKEN.
The rule is deliberately not "no push without an open PR". That is
unsatisfiable: a forge cannot open a pull request for a ref it does not have
yet, so the first push of a branch can never have one. Rule 2 is the closest
enforceable form, and the first push is what creates the ref a PR points at.
Ships as language: script rather than a python validator because it inspects
push destinations and the forge API, not repo files.
pre-commit consumes git's stdin ref lines itself and re-exposes a single ref
via PRE_COMMIT_REMOTE_* / PRE_COMMIT_LOCAL_BRANCH, so the hook reads env
rather than stdin. Two consequences worth knowing: deletions never reach the
hook (pre-commit filters them), and a multi-ref push surfaces only its first
ref, so
git push --allis checked on one branch rather than all of them.Failure modes fail open, never closed: a missing FORGEJO_TOKEN, a non-Forgejo
remote, an unreachable API, and invocation outside a real push all exit 0. A
failed PR lookup returns empty, which is explicitly distinguished from a
genuine zero so a network blip cannot masquerade as "no open PR".
This is client-side; git push --no-verify bypasses it. Server-side branch
protection remains the only control that cannot be bypassed.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Keeps the default branch PR-only from the client side. Two rules: 1. Refuse any push whose destination is the remote's default branch. No credentials, works against any forge. 2. Once a branch already exists on the remote, require an open PR before allowing further pushes to it. Forgejo API, gated on FORGEJO_TOKEN. The rule is deliberately not "no push without an open PR". That is unsatisfiable: a forge cannot open a pull request for a ref it does not have yet, so the first push of a branch can never have one. Rule 2 is the closest enforceable form, and the first push is what creates the ref a PR points at. Ships as language: script rather than a python validator because it inspects push destinations and the forge API, not repo files. pre-commit consumes git's stdin ref lines itself and re-exposes a single ref via PRE_COMMIT_REMOTE_* / PRE_COMMIT_LOCAL_BRANCH, so the hook reads env rather than stdin. Two consequences worth knowing: deletions never reach the hook (pre-commit filters them), and a multi-ref push surfaces only its first ref, so `git push --all` is checked on one branch rather than all of them. Failure modes fail open, never closed: a missing FORGEJO_TOKEN, a non-Forgejo remote, an unreachable API, and invocation outside a real push all exit 0. A failed PR lookup returns empty, which is explicitly distinguished from a genuine zero so a network blip cannot masquerade as "no open PR". This is client-side; git push --no-verify bypasses it. Server-side branch protection remains the only control that cannot be bypassed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>CI fixed in
dc069df, but it needed a scope change rather than a repairThe
gatejob failed on one assertion out of 559:That guard is right and the PR was wrong, so I did not touch the test.
The rollout pins
rev=default_rev(), which resolves to the latest release tag at runtime, while the hook list comes fromDEFAULT_HOOK_IDS. Listing a hook the pinned tag does not carry makes every consumer'spre-commitinit fail withpr-guard is not present in repository ... rev aos-precommit-v0.32.0. No release carriespr-guard, because this is the PR introducing it. It is the exact chicken-and-egg #187 exists to catch, and it would have broken every consumer repo on their next init rather than failing here.What changed
One line removed,
"pr-guard"out ofDEFAULT_HOOK_IDS. Nothing else.The hook still ships in full: the
.pre-commit-hooks.yamlentry andscripts/pr-guard-pre-push.share untouched, so any repo can opt in explicitly today by naming the hook.The follow-up this needs
Adding it to the defaults is a one-line change once a release carrying the hook is cut and tagged.
default_rev()resolves the tag at runtime, so noDEFAULT_REVbump is involved and the same guard will pass on its own. Sequence is merge, release, then default it.Worth deciding whether that follow-up is filed now so it is not forgotten, since the hook is inert for consumers until it happens.
Verification
On this branch, before and after, full suite with a private pytest basetemp:
Exactly one test flips, the guard test. The remaining 28 are pre-existing native-Windows failures, mostly
test_statusline.py, that CI does not see. CI itself reported1 failed, 558 passedon the same commit, so the CI-visible failure count goes to zero.One note on the PR body
The description says the rule is deliberately not "no push without an open PR" because a forge cannot open a PR for a ref it does not have. Worth adding that rule 2's fail-open behavior means the guard is advisory on the first push of a branch and enforcing from the second onward, which is a slightly different guarantee than the summary implies. Not a blocker, just worth being explicit since the whole point is that server-side protection stays the real control.
CI is green: all four jobs on
dc069dfpass (aos-cli-tests,aos-eval-tests,ward-doctor,gate).Follow-up filed as #1148 so the hook does not sit inert. It records the ordering, which is mostly automatic: merging here fires
promote.ymltorelease, which firesaos-precommit-release.ymlbecause.pre-commit-hooks.yamlis in itspathsfilter, which cutsaos-precommit-v0.33.0.default_rev()then resolves that tag, and adding"pr-guard"toDEFAULT_HOOK_IDSbecomes a one-line change the #187 guard accepts. Only that last step is manual.While reading the release workflow for that, I found a second gap worth folding into the same pass. Its
pathsfilter names hook scripts individually and includesscripts/trufflehog-scan.sh, but notscripts/pr-guard-pre-push.sh. The first release still fires, since this PR also touches.pre-commit-hooks.yaml. But once the hook entry is stable, a later fix topr-guard-pre-push.shalone touches no listed path, so no release is cut and consumers pinned to a tag keep running the old script with no signal a fix exists. Details in #1148.