Conversation
resolveBaseline() asked the API for the closest commit with an eligible baseline build and sent that commit alone, with parentCommits: null. Two things follow from naming a commit there. The choice is frozen to the eligibility of that instant. /baseline only matches builds that are already complete with a complete, valid bucket, so on a busy base branch the closest ones are routinely still running when the CLI executes and the search walks straight past them. Minutes later, when the server processes the build, they are complete - but the server stops at the reference commit as soon as it has a bucket for it, so the stale pick stands. The choice is also final. Without remote content access there is no ancestor list to fall back on: listParentCommitShas() returns [] for a light installation, so one commit is all the server ever has. On one production project 0 of 4379 builds carried parentCommits, and 90% of its base branch builds were baselined against a commit that was not the closest eligible one, up to five builds back. So send the merge base with the ancestors the server searches, and let it pick when it processes the build. /baseline keeps the job it is actually needed for: telling us whether a baseline is reachable at all. Only when none is do we search deeper and name a commit ourselves, because past PARENT_COMMITS_LIMIT the server cannot walk there. findReferenceCommit() also stopped widening as soon as a listing came back shorter than the batch size. That is normal on the shallow clones CI uses, where git stops at the boundary of the fetched history and a deeper fetch does bring more commits, so the search could give up early. It now widens while the listing grows, and skips the batches the caller already inspected - otherwise it would re-list the history at a shallower depth than it has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Heads-up before anyone reviews this: the premise is wrong, this should not be merged as-is. I claimed the closest base branch builds were still running when The walk is not at fault either. In run 34122664365 it behaves exactly as designed: 20 kits stop at The real fault is upstream, in So this is still #372's territory: |
Description
Follow-up to #372, which is working: on the production build that prompted this, the merge base was master's tip. The stale baseline comes from what happens after the search succeeds.
resolveBaseline()asked/baselinefor the closest commit with an eligible baseline build and sent that commit alone, withparentCommits: null. Naming a commit there has two consequences.The choice is frozen to the eligibility of that instant.
/baselineonly matches builds that are alreadycompletewith acomplete && validbucket. On a busy base branch the closest ones are routinely still running when the CLI executes, so the search walks past them. Minutes later, when the server processes the build, they are complete — butresolveCIBasestops at the reference commit as soon as it has a bucket for it, so the stale pick stands.The choice is also final. Without remote content access there is no ancestor list to fall back on:
listParentCommitShas()returns[]for a light installation, so a single commit is all the server ever has. That is the risk flagged in #372's reviewer notes — it turns out to fire on every build, not in an edge case.So the CLI now sends the merge base plus the ancestors the server searches, and lets the server pick when it processes the build.
/baselinekeeps the job it is actually needed for: telling us whether a baseline is reachable at all. Only when none is do we search deeper and name a commit ourselves, because pastPARENT_COMMITS_LIMITthe server cannot walk there.findReferenceCommit()also stopped widening as soon as a listing came back shorter than the batch size. That is normal on the shallow clones CI uses, wheregit logstops at the boundary of the fetched history and a deeper fetch does bring more commits — so the search could give up early. It now widens while the listing grows, and skips the batches the caller already inspected, which also keeps the fetch depths monotonic (a smaller--depthafterwards re-shallows the clone).Evidence
Measured on a production project (light app, GitHub Actions, 24 build names in one monorepo, a push to the base branch every ~2 minutes):
parentCommits, so the server never had a fallback.Type of changes
bugChecklist
Further comments
Behaviour change worth a second opinion
The server's ancestor walk (
getBucketFromCommits) does not filter onapproved, whereas/baselinedoes. For base branch history this is the same set — those builds aretype: "reference", which is auto-approved — but a merged pull request'scheckbuild whose bucket commit is an ancestor could now be picked where the CLI would have skipped it. Rejected builds stay excluded either way. This also makes the light-app path behave like the Git-provider path, which has always walked ancestors server-side.Ruled out along the way
git log --topo-orderdoes not help. The candidate list is documented as "closest to furthest ancestor" and the server ranks by position, so committer-date order looked suspect. It is not, for ancestors: a parent only enters git's walk through a child, so a parent is never emitted first. For incomparable commits (a side branch merged into the base branch) the order genuinely is by date — but--topo-orderproduces the identical order there; only--first-parentmoves the mainline commit ahead. That is a real semantic change, so it is left out of this fix rather than bundled into it.GITHUB_SHAcan go stale the same way the payload did.getTestMergeBaseCommitSha()requiresgetHeadSha() === GITHUB_SHAandgetCommitParents()deepens by fetching a bare SHA. GitHub recomputesrefs/pull/<n>/mergewhenever either branch moves, so the runner can check out a newer test-merge commit than the run was created for, and the previous one stops being advertised. Both guards then bail to the merge base — the fork point, the very outcome fix(core): identify the test merge from the merge ref, not the payload #372 removed. I could not show it firing on the build investigated here (the merge base was fresh), so it is worth its own change rather than an unproven one here.🤖 Generated with Claude Code