fix(ci): give push builds the parent commit their audits diff against - #7033
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Default Reviewed by Cursor Bugbot for commit 4b3d90b. Configure here. |
Greptile SummaryThe PR centralizes resolution of the audit base and uses the pre-push SHA for ordinary push events.
Confidence Score: 4/5The PR is not yet safe to merge because multi-commit branch-creation pushes can still bypass both diff-based audits for changes before the tip. The previous reply, whose author is not shown in the supplied thread, says the tip-only audit issue was fixed, but an all-zero Files Needing Attention: .github/workflows/test-build.yml
|
| Filename | Overview |
|---|---|
| .github/workflows/test-build.yml | Centralizes audit-base selection and fixes ordinary multi-commit pushes, but the all-zero before fallback still checks only the tip commit on branch creation. |
Reviews (2): Last reviewed commit: "fix(ci): give push builds a base their a..." | Re-trigger Greptile
Push builds fail the migration audit:
✗ Migration safety check could not run.
Cannot diff against 'HEAD~1'.
`actions/checkout` sets no `fetch-depth`, so it defaults to 1 — a single-commit
clone in which `HEAD~1` does not resolve. Both diff-based audits named `HEAD~1`
as their push base, so neither has ever had a base to read. The migration audit
answered that with `✓ No new migrations to check` and exit 0, so it had never
run on a push build at all; #7022 made it say it could not run instead, which is
what surfaced this. The block-registry check reports `⚠ … skipping` on the same
input — visible, and equally never run.
`HEAD~1` was the wrong base regardless. It names the last commit, so a push
carrying several commits audits the tip and lets every earlier commit through:
3-commit push, HEAD~1 base: mig3.sql
3-commit push, before base: mig1.sql mig2.sql mig3.sql
The base is now `github.event.before` — the tip the branch had before the push,
which is what GitHub provides for exactly this. It is fetched by SHA at depth 1;
the audits diff two tips and need no common ancestry between them. Resolved once
in a step both audits read, so the two cannot drift apart.
`HEAD~1` survives only as the fallback for an all-zero `before` (a new branch,
with no predecessor to diff), which is what `fetch-depth: 2` now covers.
Verified: both audits accept a raw SHA base and pass; the multi-commit case above
is a real reproduction, not a description.
4b3d90b to
73926a9
Compare
… guessing one Follow-up to #7033, which merged with this thread open. A push that creates a branch reports an all-zero `github.event.before`, and the fallback answered that with `HEAD~1` — auditing the single tip commit while reporting on the whole push. That is the same partial-audit-reported-as-complete failure #7033 set out to remove, one case further along. There is no correct base to substitute. Nothing precedes the push, and diffing the whole history would lint every migration ever written. So the audits skip with a `::notice::` naming the reason. A stated skip is honest; a partial audit wearing a green check is not. The same branch also covers `workflow_dispatch`, where `before` is empty rather than all-zero because there is no push payload at all. The guard has to test both — an empty `before` reaching the fetch would run `git fetch origin ""` and fail the job outright, which this workflow allows since it declares `workflow_dispatch`. Dropping the fallback drops its only consumer: `fetch-depth: 2` existed to give `HEAD~1` something to resolve to, and `before` is fetched by SHA, so the checkout returns to the default depth. Traced all four event shapes through the branch — PR, ordinary push, branch creation, manual dispatch — and verified both audits still pass against a raw SHA base.
What is failing
Why
actions/checkoutin the audit job sets nofetch-depth, so it defaults to 1 — a single-commit clone in whichHEAD~1does not resolve. Both diff-based audits useHEAD~1as their base on push events, so neither has ever had a base to read.Proven in a two-commit repo:
This is #7022 doing its job, uncomfortably
Before #7022 the audit answered that failure with
✓ No new migrations to checkand exit 0. It had never actually run on a push build — the green was a report about what it could see, not about the migrations.#7022 made it say it cannot run rather than claim success, and the first push build after it merged said exactly that. The bug is the missing history; #7022 only stopped it being invisible.
The block-registry check has the identical input and reports
⚠ Could not diff against base ref — skipping. Visible, so it never blocked anything — and equally never ran on push.Fix
fetch-depth: 2on the audit job's checkout. That is the minimum that makesHEAD~1name something, and on this repo's push events (squash merges)HEAD~1..HEADis exactly the merged change.Only the
Lint and Testjob changes.Build Appkeeps its depth-1 checkout — it does no diffing and a deeper fetch would slow it for nothing.Verification
Both audits against
HEAD~1on a checkout with history:Since both already run on every PR against
origin/<base>, and a push tostagingcarries the same content that just passed as a PR, neither should newly fail — they will simply run for the first time.