fix(actions): handle merge commits in verify-signed-commit-authors - #40
Merged
Merged
Conversation
A PR branch with a merge commit (e.g. merging the base branch in) can have its true merge-base much further back on the merged-in side than the initial shallow fetch's depth budget (prCommitCount + 1) reaches, since that budget assumes linear history. When the budget is exhausted mid-chain, git grafts a parentless shallow boundary that severs the link proving shared ancestry with the base branch, so rev-list overcounts and the check fails even though every commit is validly signed. Retry with a deepened fetch (doubling^2 each attempt, capped) before falling back to a full unshallow fetch as a last resort, so the common linear-history case stays cheap and only merge-commit branches pay for the extra round trips. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
darkgnotic
requested review from
0xcadams,
aboodman,
arv,
cesara,
grgbkr and
tantaman
as code owners
September 23, 2026 16:42
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
2 participants
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
verify-signed-commit-authorsfails PRs whose branch contains a merge commit (e.g.Merge branch 'main' into feature-branch), even when every commit is validly signed.Root cause: the action shallow-fetches the PR head with
--depth=(commitCount + 1), a hop-count budget that assumes linear history — distance to the merge-base equals the PR's commit count. A merge commit breaks that assumption: the true divergence point from the base branch can sit much further back down the merged-in side thancommitCount + 1hops. When the shallow fetch's depth budget runs out first, git grafts a parentless shallow boundary that severs the graph link proving that commit is also an ancestor of the base branch.git rev-list base..headthen can't prove shared ancestry across that boundary, so it overcounts (Expected N, found N+1) — with no relation to whether any commit is actually signed correctly.Repro'd against
rocicorp/monoPR #6651 and traced the exact shallow-graft mechanism; details in the linked internal conversation.--shallow-exclude=<baseSha>was evaluated as an exact-boundary alternative to the depth heuristic, but GitHub's smart-HTTP server doesn't supportdeepen-not— confirmed via packet trace (fatal: expected 'packfile'). A fully unbounded fetch of the head ref also works but roughly doubles fetch cost on every run (measured ~22s/620MB → ~44s/1.1GB onmono), since it doesn't dedupe against the already-fetched base.Fix
Keep the cheap depth-limited fetch as the fast path (no behavior change for ordinary linear-history PRs). On a count mismatch, retry with
git fetch --deepen=N(N starting atcommitCount × 4, growing ×4 per attempt, capped at 4 attempts) before falling back to a full--unshallowfetch as a last resort. Only fails for real if the count still doesn't converge after that.Validated live against
rocicorp/monoPR #6651 (previously failing with "Expected 4, found 5"): now passes, converging via a single deepen call, with fetch time/size in the same ballpark as today's baseline (not the ~2x cost of the always-unbounded alternative).Test plan
node --test .github/actions/verify-signed-commit-authors/verify-signed-commit-authors.test.mjs— 20/20 pass, including 3 new tests covering deepen-convergence, unshallow-fallback, and fail-closedrocicorp/monoPR #6651's real base/head SHAs with a live GitHub token — now verifies all 4 commits correctly🤖 Generated with Claude Code