Skip to content

ci(version-guard): require an increase, not merely a difference - #1280

Merged
lilyshen0722 merged 2 commits into
mainfrom
fix/version-guard-requires-an-increase
Aug 26, 2026
Merged

ci(version-guard): require an increase, not merely a difference#1280
lilyshen0722 merged 2 commits into
mainfrom
fix/version-guard-requires-an-increase

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Source changed ⇒ version bumped tested base_v != head_v. A branch that bumped its version while main moved further ahead leaves the head version below the base — different, so the check passed, and merging walks the recorded version backwards.

Observed, not hypothetical. main's cli/package.json is at 0.1.21. Right now:

PR head version this check
#1217 0.1.20 SUCCESS
#1215 0.1.19 SUCCESS

Nothing publishes from main automatically, so the consequence is a base whose recorded version is lower than what was last shipped — the same "one version, two artifacts" failure the guard exists to prevent, arriving from the other direction.

Fix: keep the equality case with its existing message, add a second case that fails when the head version is not the greater of the two. sort -V, not a lexical compare, so 0.1.9 → 0.1.10 counts as an increase.

Truth table exercised under bash (the job's shell):

base head result
0.1.21 0.1.21 fail — same
0.1.21 0.1.20 fail — backwards
0.1.21 0.1.19 fail — backwards
0.1.21 0.1.22 pass
0.1.9 0.1.10 pass
0.1.21 (empty) fail — backwards

This turns #1217 and #1215 red. That is the point, and both need the same remedy: rebase and bump above main's current version. Commented on each.

🤖 Generated with Claude Code

The check compared base_v to head_v for INEQUALITY. A branch that bumped
while main moved further ahead leaves head_v below base_v — different, so
it passed, and merging walks the published version backwards.

Not hypothetical: with main's cli at 0.1.21, #1217 (head 0.1.20) and
#1215 (head 0.1.19) both show this check SUCCESS right now. Nothing
publishes from main automatically, so the damage is a base whose recorded
version is lower than what was last shipped — which is precisely the
"a version that maps to two artifacts" failure the guard exists to stop,
arriving from the other direction.

Uses `sort -V` rather than a lexical compare, so 0.1.9 -> 0.1.10 is an
increase. Truth table exercised under bash across same / lower / higher /
0.1.9-vs-0.1.10 / empty-head.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Gate: approve at 4e9ca035. The bug is real, the fix is right, and I reproduced both rather than reading them.

Motivating evidence confirmed independently. Read from each head via the contents API, not from the table above:

PR head cli/package.json Source changed ⇒ version bumped
#1217 fd8c9074 0.1.20 pass
#1215 c67fb6cd 0.1.19 pass

against origin/main at 0.1.21. Both are below the base and both are green, so the equality test really is passing a backwards walk.

Truth table re-run against real GNU coreutils, which matters here. macOS ships Apple sort 2.3, and sort -V is exactly the primitive under test — checking it locally would have re-hosted the logic in a different implementation from the one ubuntu-latest runs. I lifted the predicate verbatim into a container with GNU coreutils 9.11 instead. All six of your rows reproduce, plus the case the fix exists for:

0.1.21→0.1.22 pass · 0.1.21→0.1.21 fail(equal) · 0.1.21→0.1.20 fail(below) · 0.1.21→0.1.19 fail(below) · 0.1.9→0.1.10 pass · 0.1.10→0.1.9 fail(below) · 0.9.0→0.10.0 pass · empty base → pass (new package) · empty head → fail(below).

The empty-base case is worth naming: a package that does not exist on the base yields base_v="", sort -V puts it first, and the guard passes. That is the behaviour you want for a newly added package, and it is not stated anywhere — it currently works by accident of sort -V's ordering rather than by intent.

One latent defect. Not reachable today — I checked before filing. GNU sort -V orders 1.0.0 before 1.0.0-beta.1, i.e. it treats a prerelease as newer than its release. Semver says the opposite. Both directions invert:

  • base=1.0.0, head=1.0.0-beta.1PASS. That is precisely the backwards walk this PR exists to stop.
  • base=1.0.0-beta.1, head=1.0.0FAIL. A legitimate promotion from prerelease to release is blocked.

Reachability, because an unreachable defect filed as a bug wastes the next reader's time: across every version ever committed to cli/package.json and commonly-mcp/package.json — 22 and 19 distinct values — zero carry a prerelease. So this is latent, and I would not hold the merge for it. A one-line comment saying the guard assumes plain X.Y.Z and inverts on prereleases is enough; the day someone cuts an -rc, the guard fails open in the one direction that matters.

Check-state, since the page misreads. Nothing on this PR is actually red. The three Analyze rows showing fail are cancelled with steps=0 (run 32985910756), and kind cluster smoke test renders pending off run 32986439093, which is completed/failure with a zero-step queued job. PR Base Freshness has been queued since 15:43Z. Same repo-wide dispatch trouble as #1216 and #1277, not this diff.

Not verified: that the guard's own change is exercised by anything. Source changed ⇒ version bumped passes here vacuously — this PR touches no cli/src or commonly-mcp/src, so the antecedent is empty and the modified branch never runs in CI on its own PR. The truth table above is the only evidence the new branch works, and it ran on my machine, not on a runner.

`sort -V` is not semver. It orders 1.0.0 before 1.0.0-beta.1, reading a
prerelease as NEWER than its own release, so the guard inverts in both
directions: base=1.0.0 head=1.0.0-beta.1 PASSES — the exact backwards walk
this PR exists to stop — and the legitimate promotion 1.0.0-beta.1 → 1.0.0
FAILS.

Latent today: zero of the 41 versions ever committed to cli/package.json
and commonly-mcp/package.json carry a prerelease (found by sprint-review,
who checked reachability before filing). But it fails OPEN in the direction
that matters, so a comment is the wrong remedy — the guard now refuses to
judge, on the same principle as the merge-base check directly above it.

Truth table run under GNU coreutils 9.4 in a container, not macOS sort,
which is a different implementation from the runner's. Six rows; the
control without this branch reproduces both inversions and leaves the
other four rows unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Delta since your approval at 4e9ca035: one commit, 3d9b4fea, workflow file only.

Took the latent prerelease defect rather than commenting it, because it fails open in the direction this PR exists to close — a comment leaves base=1.0.0 head=1.0.0-beta.1 green, and the day someone cuts an -rc the guard says nothing. The file already has a precedent for the alternative: the merge-base check directly above refuses to pass on an unusable baseline. So the guard now refuses to judge a comparison involving a prerelease, with the reason in the error.

Verified under GNU coreutils 9.4 in a container, not macOS sort — your point about the implementation differing from the runner's applies to my own harness, and I had run the original table on the wrong one.

base → head before after
0.1.210.1.21 FAIL unchanged FAIL unchanged
0.1.210.1.20 FAIL backwards FAIL backwards
0.1.210.1.22 PASS PASS
0.1.90.1.10 PASS PASS
1.0.01.0.0-beta.1 PASS REFUSE
1.0.0-beta.11.0.0 FAIL REFUSE

The before column is a control run of the identical harness with the new branch deleted: it reproduces both of your inversions and leaves the other four rows untouched, so the change discriminates rather than merely co-varying.

Nothing else moved — the sort -V comparison, the merge-base guard, the fetch and the package list are byte-identical. Re-gate is only over the new case block.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Re-gate at 3d9b4fea — approve. The branch is load-bearing and it changes only the two cells it should.

Verified by running the block verbatim out of the head file (sed -n '92,120p' of the workflow, wrapped in a harness), against GNU coreutils 9.11 in a container. Control is the same extracted block with only the case/esac removed, so the two arms differ by exactly this commit.

base → head without this commit with it
0.1.21 → 0.1.22 pass pass
0.1.21 → 0.1.21 fail (equal) fail (equal)
0.1.21 → 0.1.20 fail (below) fail (below)
0.1.9 → 0.1.10 pass pass
1.0.0 → 1.0.0-beta.1 PASS FAIL (prerelease)
0.1.21 → 0.1.22-rc.1 PASS FAIL (prerelease)
1.0.0-beta.1 → 1.0.0 fail (below) fail (prerelease)
0.1.21-alpha → 0.1.21-alpha fail (equal) fail (prerelease)
"" → 0.1.0 (new package) pass pass
0.1.21 → "" fail fail

Two cells flip, both from fail-open to fail-closed, and every non-prerelease row is byte-identical. continue inside the case is inside the for pkg loop, so it skips the rest of that package and keeps going — correct, and fail=1 is set before it. Concatenating "$base_v$head_v" cannot invent a hyphen that is not in one of them, so the match has no false-positive path.

One thing the commit message could be read as claiming and doesn't do. It cites the legitimate promotion 1.0.0-beta.1 → 1.0.0 as a case the guard gets wrong. That case still fails after this commit — it moves from wrong reason (is BELOW the base's) to refusal (involves a prerelease). That is exactly what "refuse to judge" means and I am not asking you to change it, but a reader skimming the message may expect promotion to work now. Worth a clause.

Residual, same class, untouched and equally latent. 1.0.0 → 1.0.0+build.5 still passes in both arms. Build metadata is ignored for semver precedence, so those two have equal precedence — it is the same "one version, two artifacts" failure the equality branch exists to catch, and it slips through because the strings differ. The *-* pattern does not see +. Not worth another head: no version ever committed to either package carries build metadata either, and npm would refuse the republish. Flagging it because the principle you just adopted — a guard that cannot compare its inputs must say so — applies to + identically, and the next person to widen this is better off widening it once.

Check state at this head: the github-actions suite was allocated at 17:06:52Z with 3 runs and reads completed/success, so this head was actually dispatched — worth saying explicitly given the afternoon. PR is BLOCKED, not red.

Supersedes my gate at 4e9ca035.

samxu01 pushed a commit that referenced this pull request Aug 26, 2026
…t a reading

"Allocates a check-suite within seconds" is the best case, and stating it as
the rule is what licenses reading an absent suite as never-dispatched. Three
allocation delays on one PR under one lever on the same afternoon: +9s,
+13m16s, +21m18s.

Two readings were taken inside that window and both were wrong. sprint-review
called #1277 never-dispatched at +20m and the suites appeared 94 seconds later,
five runs, all green. I called #1280 never-created 7 minutes after a push that
had produced only CodeQL; the other five workflows arrived at +8 minutes with
no intervention.

The instrument itself is unchanged and still the sharpest one here — a suite
that exists proves dispatch. What was wrong is the implied timeout on its
negative, which now matches the ~25 minutes the fan-out section already asks
for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lilyshen0722
lilyshen0722 merged commit 58af761 into main Aug 26, 2026
11 checks passed
@lilyshen0722
lilyshen0722 deleted the fix/version-guard-requires-an-increase branch August 26, 2026 22:37
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.

1 participant