feat(ci): refuse to push a branch that is behind origin/main - #2056
Merged
Merged
Conversation
A branch behind the base produces a diff that no longer describes what will merge: reviewers read the wrong merge base, CI passes against code nobody will ship, and conflicts surface at merge time. The sync-branch skill asks an agent to rebase first; this is the same rule below the agent, so it holds for every agent and for a human typing `git push`. Runs from pre-commit's pre-push stage, NOT from core.hooksPath. Setting core.hooksPath would make git ignore .git/hooks/ entirely, silently disabling the pre-commit hook already installed there — trading one guard for another. Two properties in .pre-commit-config.yaml are load-bearing: - `default_install_hook_types` — `pre-commit install` alone installs only the pre-commit hook, so without this the guard would silently never run for anyone who had already installed. Verified: a plain install now wires both. - `default_stages: [pre-commit]` — a hook with no `stages` runs at EVERY stage, so installing a pre-push hook made all 14 existing hooks (terraform validate, tflint, detect-secrets) fire on every push too. Measured 15 hooks at push time before this line, 1 after. The fetch before comparing is the point: a local origin/main last updated hours ago reports "up to date" on a branch that is not, which is the exact failure being guarded against. Verified end to end by replaying the stdin git feeds a pre-push hook, not just by calling the script: behind by 2 -> refuses, exit 1, names the count and the fix up to date -> allows branch deletion -> allows (zero sha) pushing main -> allows, as a full ref, before the staleness check pushing a tag -> allows, before the staleness check fetch fails -> warns and allows, so offline work is not blocked shellcheck -> clean CI unaffected -> "No hook with id check-rebased in stage pre-commit" Known gap, documented in the script: pre-commit's pre-push stage only runs when the push carries commits not already on a remote, and `always_run` does not override that. A branch with no unique commits is therefore unchecked — found by actually pushing one, which sailed through. Accepted rather than worked around: such a push carries no work to review, and both the ordinary push and the post-rebase force-push have commits of their own.
Contributor
🔍 Rendered manifest diff — this PR vs
|
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
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
AGENTS.mdsays "rebase ontoorigin/mainbefore every review, push and PR", andsync-branchdoes it — but only if an agent chooses to run it. This puts the same rule below the agent, so it
holds for every agent and for a human typing
git push.A branch behind the base produces a diff that no longer describes what will merge: reviewers read
the wrong merge base, CI passes against code nobody will ship, and conflicts surface at merge time.
That happened twice while writing this PR — main moved under it both times, and the guard caught
the second one on itself.
flowchart TD P["git push"] --> D{"pushing main,<br/>a tag, or a deletion?"} D -->|yes| A["allow"] D -->|no| F["fetch origin/main"] F -->|fetch failed| W["warn + allow<br/>(offline work isn't blocked)"] F --> M{"is origin/main an<br/>ancestor of HEAD?"} M -->|yes| A2["allow"] M -->|no| R["refuse — name the count,<br/>print the rebase command"] classDef ok fill:#1f3d2f,stroke:#4ad98a,color:#fff classDef no fill:#4a1f1f,stroke:#d94a4a,color:#fff classDef neutral fill:#1e3a5f,stroke:#4a90d9,color:#fff class A,A2 ok class R no class W,P,F,D,M neutralWhy pre-commit's pre-push stage, not
.githooks+core.hooksPathcore.hooksPathwas the obvious approach and it is a trap here: it makes git ignore.git/hooks/entirely, which is exactly where pre-commit installs. Wiring the rebase guard that waywould have silently disabled the pre-commit hook — trading one guard for another, with no error.
The repo already runs pre-commit, and CI already runs it, so the pre-push stage is free integration.
Two config lines that are load-bearing
default_install_hook_types: [pre-commit, pre-push]pre-commit installinstalls only the pre-commit hook, so the guard silently never runs for anyone who had already installeddefault_stages: [pre-commit]stagesruns at every stage — installing a pre-push hook made all 14 existing hooks (terraform validate, tflint, detect-secrets) fire on every push. Measured: 15 hooks at push time before this line, 1 afterEscape hatches
Both are legitimate — a WIP branch nobody will review, or a commit added to someone else's branch
that you must not rebase.
Verification
Exercised by replaying the stdin git actually feeds a pre-push hook, not just by calling the script
with hand-set variables — the first round of testing did the latter and gave false confidence
(see the gap below).
main, as a full refshellcheckNo hook with id check-rebased in stage pre-commitgit rebase origin/mainKnown gap, and why it's accepted
pre-commit's pre-push stage only runs when the push carries commits not already on a remote
(
git rev-list <local> --not --remotes); when that set is empty it returns before running any hook,and
always_run: truedoes not override it. So a branch with no unique commits is unchecked — foundby actually pushing one at
origin/main~2, which sailed straight through.Accepted rather than worked around: such a push carries no work to review, which is the only thing a
stale merge base can misrepresent. Every real feature branch has commits of its own, and a rebase
gives them new SHAs, so the ordinary push and the post-rebase force-push are both covered. Recorded
in the script header so the next reader doesn't assume wider coverage than exists.
Note for you
I ran
pre-commit installlocally while testing, which wiredpre-pushinto the shared gitdir —so it applies to your other worktrees too. They're unaffected until this merges: their
.pre-commit-config.yamlhas no pre-push hook, so the installed hook finds nothing to run.