Problem
PR #2267 landed (73d2f2b). Its core design is sound — I verified empirically that the ratchet direction survives (max(ceiling, base) still fails a PR that adds lines on top of inherited drift) and that the fallbacks fail closed in CI and in shallow clones. One rung of resolve_base_commit is wrong, and it makes the pre-push gate false-green on a developer's own growth.
scripts/check-file-size.sh:187-190 comments that "A merge commit means a refs/pull/N/merge checkout". That is false locally. Reproduction:
- On a feature branch, grow a god file by +100 lines without touching
scripts/file-size-baseline.txt.
- Merge trunk into the branch — the ordinary update-branch shape, i.e. the commit immediately before
git push.
HEAD^2 now exists, so the guard picks base = HEAD^1 = your own pre-merge tip, which already contains the growth.
The guard prints
src/big.rs is 1700 lines against a ceiling of 1600 — already so at the base ... another change put them there
and exits 0. The pre-push hook passes, CI reds an hour later, and the diagnostic blames a change that does not exist. scripts/check-deleted-tests.sh uses the same HEAD^1/HEAD^2 pair safely only because it runs solely on pull_request in CI; this guard borrowed the pair without that precondition.
Fix
Reorder resolve_base_commit: try the merge-base HEAD origin/main rung (currently line 199) before the HEAD^2 rung.
- In CI's PR / merge-queue checkout
origin/main does not exist (single-ref fetch), so HEAD^2 still fires there — unchanged behavior.
- Even if a future checkout config materializes
origin/main, merge-base(refs/pull/N/merge, origin/main) is the base tip = HEAD^1, so the reorder is a no-op there.
- Locally it yields the true fork base and catches the growth.
Also fix the misattributing summary line: "another change put them there" should only be said when the base genuinely predates this branch.
Verify
Add a case B7 to scripts/test-file-size.sh, hermetic like B1–B6: grow a baselined file on a feature branch, merge trunk into the branch, with an origin/main ref present — expect fail. It must fail against the guard as it stands at 73d2f2b and pass after the reorder.
Constraints discovered
Related: #2310 (nothing retightens the baseline now that drift is non-fatal), #2311 (OBSOLETE/STALE did not inherit the base-relative reasoning).
Refs #2267, #2004.
Problem
PR #2267 landed (73d2f2b). Its core design is sound — I verified empirically that the ratchet direction survives (
max(ceiling, base)still fails a PR that adds lines on top of inherited drift) and that the fallbacks fail closed in CI and in shallow clones. One rung ofresolve_base_commitis wrong, and it makes the pre-push gate false-green on a developer's own growth.scripts/check-file-size.sh:187-190comments that "A merge commit means arefs/pull/N/mergecheckout". That is false locally. Reproduction:scripts/file-size-baseline.txt.git push.HEAD^2now exists, so the guard picks base =HEAD^1= your own pre-merge tip, which already contains the growth.The guard prints
and exits 0. The pre-push hook passes, CI reds an hour later, and the diagnostic blames a change that does not exist.
scripts/check-deleted-tests.shuses the sameHEAD^1/HEAD^2pair safely only because it runs solely onpull_requestin CI; this guard borrowed the pair without that precondition.Fix
Reorder
resolve_base_commit: try themerge-base HEAD origin/mainrung (currently line 199) before theHEAD^2rung.origin/maindoes not exist (single-ref fetch), soHEAD^2still fires there — unchanged behavior.origin/main,merge-base(refs/pull/N/merge, origin/main)is the base tip =HEAD^1, so the reorder is a no-op there.Also fix the misattributing summary line: "another change put them there" should only be said when the base genuinely predates this branch.
Verify
Add a case B7 to
scripts/test-file-size.sh, hermetic like B1–B6: grow a baselined file on a feature branch, merge trunk into the branch, with anorigin/mainref present — expect fail. It must fail against the guard as it stands at 73d2f2b and pass after the reorder.Constraints discovered
resolve_base_commitfails closed by design (badFILE_SIZE_BASE_REF→ exit 1; shallow depth-1 → strict fallback). Keep that posture..github/workflows/file-size.ymlneedsfetch-depth: 2; that landed with fix(gate): the file-size ratchet judges the change, not the tree (#2004) #2267 and must not regress.Related: #2310 (nothing retightens the baseline now that drift is non-fatal), #2311 (OBSOLETE/STALE did not inherit the base-relative reasoning).
Refs #2267, #2004.