Skip to content

ci: judge a new branch by the whole branch, not its last commit - #2061

Merged
jcschaff merged 2 commits into
masterfrom
fix/should-run-gate-merge-base
Sep 4, 2026
Merged

ci: judge a new branch by the whole branch, not its last commit#2061
jcschaff merged 2 commits into
masterfrom
fix/should-run-gate-merge-base

Conversation

@jcschaff

@jcschaff jcschaff commented Sep 4, 2026

Copy link
Copy Markdown
Member

Fixes #2034.

The bug

The should-run gate skips the fast lane when a push changed only documentation. It picked its comparison base like this:

github.event.before, if it resolves   ->  the previous tip of this ref
otherwise HEAD^1                      ->  the previous COMMIT

On a push that creates a branch, github.event.before is all-zeros and does not resolve, so it fell through to HEAD^1 and judged the entire branch by its final commit. A branch ending in a docs commit skipped build, every Fast shard and Quarkus — and reported CI-Test-group-Fast as pass in about two seconds.

PR #2033 shipped exactly that way: four commits, three of them Java across vcell-server, a docs commit last.

It is the bad kind of failure — silent, indistinguishable from a pass, and since regression.yml does not run on pull_request either, the merge queue was the first place that code would ever be compiled. It also gets more likely with better commit hygiene, because ending a branch with a docs commit is a normal shape.

The fix

Base selection now distinguishes three cases rather than two:

situation base why
push to an existing ref github.event.before most precise answer there is. All-zeros now fails this test explicitly — it means the ref did not exist, so it is not a base
merge commit (pull_request, merge_group) HEAD^1 the first parent is exactly the PR's contents, as before
new branch git merge-base with the default branch spans the branch however many commits it has, and reduces to HEAD^1 for a single-commit branch

Verified against a scratch repository

scenario old fixed
new branch, code then docs last SKIP RUN
new branch, genuinely docs-only SKIP SKIP ✅ optimisation preserved
push to existing branch, docs-only push SKIP SKIPbefore still wins
merge commit RUN RUN ✅ first parent still used

This PR is its own regression test

The last commit is deliberately docs-only. Under the bug this branch would have skipped everything and reported green; with the fix the lane runs, because the branch as a whole touches ci.yml.

So: if the fast lane runs on this PR, the fix works.

Also

A skip was indistinguishable from a pass, which is what let this go unnoticed. The gate now writes its decision and its comparison base to the job summary:

### CI lane: SKIPPED - nothing was compiled or tested
Reason: only documentation changed (docs/MESSAGING.md)
Base for the comparison: `a1b2c3d`

docs/BUILDING.md records that a green fast lane has two meanings and where to look to tell them apart.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D71LBYmQNf5J94wPqr81Jx

jcschaff and others added 2 commits September 4, 2026 00:41
Fixes #2034.

The should-run gate skips the fast lane when a push changed only documentation. It
picked its comparison base like this:

    github.event.before, if it resolves      -> the previous tip of this ref
    otherwise HEAD^1                         -> the previous COMMIT

On a push that CREATES a branch, github.event.before is all-zeros and does not resolve,
so it fell through to HEAD^1 and judged the entire branch by its final commit. A branch
ending in a docs commit therefore skipped build, every Fast shard and Quarkus - and
reported CI-Test-group-Fast as pass in about two seconds. PR #2033 shipped that way:
four commits, three of them Java across vcell-server, a docs commit last.

The failure mode is the bad kind. It is silent, it looks identical to a pass, and
regression.yml does not run on pull_request either, so the merge queue was the first
place the code would ever be compiled. It also gets MORE likely with better commit
hygiene, since ending a branch with a docs commit is a normal shape.

Base selection now distinguishes the three cases rather than two:

  - a real github.event.before  -> use it, still the most precise answer for a push to
    an existing ref. An all-zeros value now fails this test explicitly, since it means
    the ref did not exist before and is not a base at all.
  - a merge commit (HEAD^2 exists, i.e. pull_request or merge_group) -> HEAD^1, which
    is exactly the PR's contents, as before.
  - otherwise -> git merge-base with the default branch, which spans the branch however
    many commits it has and reduces to HEAD^1 for a single-commit branch.

Verified against a scratch repository, all four cases:

    new branch, code then docs last   fix: RUN    old: SKIP   <- the bug
    new branch, docs only             fix: SKIP               <- optimisation kept
    push to existing branch, docs     fix: SKIP               <- before still wins
    merge commit                      fix: RUN                <- first parent still used

The gate also now writes its decision to the job summary. A skip was otherwise
indistinguishable from a pass, which is what let this go unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D71LBYmQNf5J94wPqr81Jx
The should-run gate reports CI-Test-group-Fast as pass when it skips, so a green check
has two meanings. Record where to look to tell them apart, now that the gate writes its
decision and its comparison base to the job summary.

This commit is deliberately docs-only and deliberately last: with the bug in #2034 this
branch would have skipped build, every Fast shard and Quarkus, and reported green. With
the fix in the previous commit the lane runs, because the branch as a whole touches
ci.yml. The PR is its own regression test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D71LBYmQNf5J94wPqr81Jx
@jcschaff
jcschaff merged commit c92a15c into master Sep 4, 2026
8 checks passed
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.

should-run gate judges a whole new branch by its last commit, silently skipping CI

1 participant