What
Both checkers require the base SHA to be an ancestor of head:
scripts/check-pr-size.py:546 — require_ancestor() → raise ValueError("base must be an ancestor of head")scripts/check-commit-trailers.py:281 — raise ValueError("range base must be an ancestor of range head")
CI passes github.event.pull_request.base.sha, which is the tip of the base branch. That stops being an ancestor of head the moment main moves after the branch was cut — which on this repo is constantly.
Measured on three open PRs:
| PR |
base is ancestor of head? |
merge-base exists? |
| #506 |
NO |
yes — e1087a881 |
| #523 |
NO |
yes — fdd452637 |
| #559 |
NO |
yes — fafa16f0f |
A merge base exists every time.
Consequence, and it is worse than a red check
agent-record runs the trailer checker over the PR range. When the range cannot be computed the checker aborts before validating anything, and the job fails with an ancestry message. So:
CI has never actually validated commit trailers on an external contribution. The check that is supposed to enforce the FOLLOWING_AGENTS_PROTOCOL / Assisted-by: contract exits before it looks at a single commit. On the external PRs reviewed this week, hand-checking by a reviewer was the only verification those trailers received.
The same abort hits pr-size, so path classification and the checker-evidence contract also go unenforced on fork PRs.
Contributors meanwhile see two permanently red checks that carry no information, which is how a gate teaches people to ignore it.
Fix
Diff from the merge base, which is what a pull request actually is and what GitHub itself shows (three-dot diff):
git diff base...head # equivalently: git diff $(git merge-base base head) head
Two-dot diffing against a moved main is not merely stricter — it is wrong: it renders main's own commits as reversions inside the contributor's diff, so paths the contributor never touched get classified and counted against them.
Both checkers should compute git merge-base base head and use it as the range base. Where no merge base exists at all (genuinely unrelated histories) they should still fail closed — absence of information must not look like absence of work.
Per AGENTS.md this is a checker-semantics change: spec, red-before test, green-after evidence.
Found while reviewing the external-contributor backlog; hit live while pushing to a contributor's fork, where the trailer checker refused the range and I had to verify the trailers by hand from the merge base.
What
Both checkers require the base SHA to be an ancestor of head:
scripts/check-pr-size.py:546—require_ancestor()→raise ValueError("base must be an ancestor of head")scripts/check-commit-trailers.py:281—raise ValueError("range base must be an ancestor of range head")CI passes
github.event.pull_request.base.sha, which is the tip of the base branch. That stops being an ancestor of head the momentmainmoves after the branch was cut — which on this repo is constantly.Measured on three open PRs:
e1087a881fdd452637fafa16f0fA merge base exists every time.
Consequence, and it is worse than a red check
agent-recordruns the trailer checker over the PR range. When the range cannot be computed the checker aborts before validating anything, and the job fails with an ancestry message. So:CI has never actually validated commit trailers on an external contribution. The check that is supposed to enforce the
FOLLOWING_AGENTS_PROTOCOL/Assisted-by:contract exits before it looks at a single commit. On the external PRs reviewed this week, hand-checking by a reviewer was the only verification those trailers received.The same abort hits
pr-size, so path classification and the checker-evidence contract also go unenforced on fork PRs.Contributors meanwhile see two permanently red checks that carry no information, which is how a gate teaches people to ignore it.
Fix
Diff from the merge base, which is what a pull request actually is and what GitHub itself shows (three-dot diff):
Two-dot diffing against a moved
mainis not merely stricter — it is wrong: it renders main's own commits as reversions inside the contributor's diff, so paths the contributor never touched get classified and counted against them.Both checkers should compute
git merge-base base headand use it as the range base. Where no merge base exists at all (genuinely unrelated histories) they should still fail closed — absence of information must not look like absence of work.Per AGENTS.md this is a checker-semantics change: spec, red-before test, green-after evidence.
Found while reviewing the external-contributor backlog; hit live while pushing to a contributor's fork, where the trailer checker refused the range and I had to verify the trailers by hand from the merge base.