Skip to content

fix(core): let the server resolve the baseline from the ancestor chain - #374

Closed
gregberge wants to merge 1 commit into
mainfrom
fix/baseline-resolved-server-side
Closed

gregberge wants to merge 1 commit into
mainfrom
fix/baseline-resolved-server-side

Conversation

@gregberge

Copy link
Copy Markdown
Member

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 /baseline for the closest commit with an eligible baseline build and sent that commit alone, with parentCommits: null. Naming a commit there has two consequences.

The choice is frozen to the eligibility of that instant. /baseline only matches builds that are already complete with a complete && valid bucket. 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 — but resolveCIBase 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 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. /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 log 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, which also keeps the fetch depths monotonic (a smaller --depth afterwards 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):

  • 0 of 4379 builds carried parentCommits, so the server never had a fallback.
  • 704 of 782 (90%) base branch builds were baselined against a commit that was not the closest eligible one for their build name — skipping 1–5 nearer baselines that had already concluded more than 5 minutes earlier, so this is not a race with the upload.
  • The commits every build converges on are the ones where all build names ran: one was the baseline of 21 distinct pull requests across 27 runs. It cannot be 21 branches' fork point — it is where the walk stops.
  • In a single workflow run (one test-merge commit, therefore one merge base) the 23 build names resolved three different reference commits: two got a base branch commit from ~25 minutes earlier, the other 21 landed 85 minutes back.

Type of changes

bug

Checklist

  • I have read the CONTRIBUTING doc
  • The commits message follows the Conventional Commits' policy
  • Lint and unit tests pass locally
  • I have added tests if needed

Further comments

Behaviour change worth a second opinion

The server's ancestor walk (getBucketFromCommits) does not filter on approved, whereas /baseline does. For base branch history this is the same set — those builds are type: "reference", which is auto-approved — but a merged pull request's check build 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-order does 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-order produces the identical order there; only --first-parent moves the mainline commit ahead. That is a real semantic change, so it is left out of this fix rather than bundled into it.
  • GITHUB_SHA can go stale the same way the payload did. getTestMergeBaseCommitSha() requires getHeadSha() === GITHUB_SHA and getCommitParents() deepens by fetching a bare SHA. GitHub recomputes refs/pull/<n>/merge whenever 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

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>
@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
argos-js-sdk-reference Ready Ready Preview Sep 7, 2026 3:26pm UTC

Request Review

@gregberge

Copy link
Copy Markdown
Member Author

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 /baseline is called. They were not — the skipped candidates had concluded more than 5 minutes before the call. My own measurement had already shown that and I wrote the opposite into the description.

The walk is not at fault either. In run 34122664365 it behaves exactly as designed: 20 kits stop at a59619df, and design-system-kit — the one kit with no build there — correctly falls back to 65ec16f2.

The real fault is upstream, in getMergeBase(). In run 34118305062 two jobs resolved 400f4496/0344ca3e (the test-merge commit's first parent) while the other 21 resolved 65ec16f2, the branch's fork point — two different merge bases inside one workflow run, from one GITHUB_SHA. accounting-onboarding-kit picking 400f4496 over an eligible a59619df proves a59619df is an ancestor of 400f4496, so it was in the fresh list and outranked 65ec16f2; the 21 kits that landed on 65ec16f2 therefore never had it — their merge base was 65ec16f2.

So this is still #372's territory: getTestMergeBaseCommitSha() bails in most jobs and silently falls back to the fork point.

@gregberge gregberge closed this Sep 7, 2026
@gregberge
gregberge deleted the fix/baseline-resolved-server-side branch September 7, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants