From fc513233cbeabe3231e669f426ea23956791246d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 25 Jul 2026 03:50:25 -0700 Subject: [PATCH] fix(release-cut): gate an explicit RC against its own series (#10525) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(release-cut): gate an explicit RC against its own series semver_gt compares through strip_pre(), so the explicit-version override only ever checked the stable line: 1.4.156-rc.0 read as 1.4.156, cleared a 1.4.155 stable, and republished an RC below what clients already run. Anchor a prerelease request on highest_rc_for_base -- the same rc history the kind path uses -- so the override can only advance the series. Two sibling gaps in the same block: - version_suffix was silently dropped when version was set, because the append lives in the kind branch the override skips. - the shape regex rejected X.Y.Z-rc.N.suffix, so a suffixed RC the rc path can produce could never be re-cut explicitly. * fix(release-cut): close both ends of the rc-number range the gate compares The new explicit-rc gate compares with `[[ -le ]]`, i.e. bash machine-width integers, and the author closed only the low end. Past INTMAX bash saturates, so `version=1.4.156-rc.99999999999999999999` reads as "above the published rc.3" and the gate falls open — then the tag it cuts pins highest_rc_for_base at 1e20 for that base forever, and every later cut wraps to a lower rc the fleet never updates to. Bound the rc number to nine digits. Also reject leading zeros on an all-digit prerelease identifier. `npm version` renormalizes rc.4.01 to rc.4.1 while the tag step keeps the literal input, so the shipped package.json version and its own release tag name different releases. The explicit path's embedded identifier now goes through the same validator the kind path uses instead of only the shape regex. * fix(release-cut): stop the refusal pointing minor/major RCs at the wrong series kind=rc derives its base from bump(latest_stable, patch), so the remedy the refusal suggested only works when the requested base *is* that next patch. A 1.5.0-rc.N series exists only because this override created it, so an operator resuming a stuck 1.5.0-rc.2 was told to dispatch kind=rc, which would have cut an unrelated 1.4.156-rc.4. Spell the condition out and give the fallback that does work for a non-patch base. Also correct the mechanism in the comment I added in 698c5beeaa: bash wraps two's-complement, it does not saturate, which is why the hole is value-dependent (rc.10000000000000000000 wraps negative and failed closed, rc.99999999999999999999 wraps to 7766279631452241919 and sailed through). And name both inputs in the suffix error, which now serves version_suffix and the trailing identifier in version. * fix(release-cut): count a suffixed RC from its commit subject, not just its tag The new explicit-version gate only fails closed on a deleted tag because highest_rc_for_base also reads `release: v-rc.N` subjects. That fallback did not parse the suffixed form: rcNumberFromTag accepts an optional .identifier, rcNumberFromReleaseSubject did not, so `4.perf` failed its `(\d+)(\s|$)` anchor and returned null. So deleting a v1.4.156-rc.4.perf tag dropped the series back to rc.3, and an explicit 1.4.156-rc.4 was waved through — below the rc.4.perf build perf-channel clients already run. Same under-count already made kind=rc recompute rc.4 over a deleted suffixed tag. Mirror the tag form's optional identifier. Covered by a unit assertion and a git-fixture test that both fail with this reverted. * docs(release-cut): correct four operator-facing claims in the explicit path All four are wording or consistency, no behavior change (harness: 26/26 before and after, on bash 3.2 and bash 5.2). - The trailing-identifier comment justified itself as preserving a shape that "can never be re-cut through the override", but re-cutting a suffixed rc at or below the series head is exactly what the new gate refuses. State what it actually admits: a second spelling of version=X.Y.Z-rc.N + version_suffix. - version_suffix's input description still said "rc kind only" after this PR made it apply to an explicit bare X.Y.Z-rc.N. - The suffix guard's own rc pattern was unbounded while the shape check twelve lines up is bounded to nine digits; reuse the bounded one so a later edit to either cannot silently drift. - "which recovers the existing tag" was unconditional, but kind=rc recovery is also gated on tag_matches_current_ref, so a tag cut from a ref main has moved past advances to rc.N+1 instead. --- .github/workflows/release-cut.yml | 96 +++++++++++++++++++--- config/scripts/release-rc-history.mjs | 7 +- config/scripts/release-rc-history.test.mjs | 23 ++++++ 3 files changed, 112 insertions(+), 14 deletions(-) diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 967f83c8897..730583e3404 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -42,12 +42,12 @@ on: default: false type: boolean version_suffix: - description: Extra prerelease identifier appended to an rc version (e.g. "perf" -> 1.2.3-rc.4.perf). rc kind only. + description: Extra prerelease identifier appended to an rc version (e.g. "perf" -> 1.2.3-rc.4.perf). Applies to kind=rc, or to an explicit version that is a bare X.Y.Z-rc.N. required: false type: string default: '' version: - description: Exact version to cut (e.g. 1.4.155 or 1.4.155-rc.0), bypassing kind-based computation. Use to leapfrog a deleted/rolled-back stable that regressed the release list. Must be greater than the latest published stable. + description: Exact version to cut (e.g. 1.4.155 or 1.4.155-rc.4), bypassing kind-based computation. Use to leapfrog a deleted/rolled-back stable that regressed the release list. Must be greater than the latest published stable, and an -rc.N must be above the highest RC already cut for its own base. required: false type: string default: '' @@ -388,6 +388,23 @@ jobs: node config/scripts/release-rc-history.mjs "$1" } + require_valid_version_suffix() { + # Why a dot-appended identifier (rc.N.perf): it sorts just + # above its own base rc.N but BELOW rc.N+1, so suffixed side- + # branch builds never outrank the main RC series and cannot + # hijack the update channel; clients find them by matching the + # identifier ("perf") in the prerelease components. + # Why the numeric alternation rather than plain [0-9A-Za-z]+: + # semver forbids a leading zero on an all-digit identifier, and + # `npm version` silently renormalizes rc.4.01 to rc.4.1 while the + # tag step keeps the literal input — so the shipped package.json + # version and its own release tag would name different releases. + if [[ ! "$1" =~ ^(0|[1-9][0-9]*|[0-9A-Za-z]*[A-Za-z][0-9A-Za-z]*)$ ]]; then + echo "::error::version_suffix (or the trailing .identifier in version) must be alphanumeric with no leading zero on an all-digit identifier, got: $1" >&2 + exit 1 + fi + } + current_package_stable() { node -e ' const { version } = require("./package.json"); @@ -498,10 +515,34 @@ jobs: new="" if [[ -n "${EXPLICIT_VERSION:-}" ]]; then explicit="${EXPLICIT_VERSION#v}" - if [[ ! "$explicit" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-rc\.[0-9]+)?$ ]]; then - echo "::error::version must be X.Y.Z or X.Y.Z-rc.N, got: $EXPLICIT_VERSION" >&2 + # Why the optional trailing identifier: it lets an operator name a + # suffixed side-branch RC (X.Y.Z-rc.N.perf) directly, the same shape + # the rc path cuts. Note this only ever admits one *above* the + # series head — the gate below refuses a suffixed rc at or below it + # just like a bare one, so this is a second spelling of + # `version=X.Y.Z-rc.N` + `version_suffix`, not a way back into a + # series that already shipped. + # Why rc.(0|[1-9][0-9]{0,8}): the `-le` below compares with bash's + # machine-width integers, so both ends of that range fall *open* on + # exactly the RCs this gate must catch. A leading zero (rc.08) is an + # invalid octal literal, and the failed test makes the `if` false. + # Past INTMAX the literal wraps two's-complement, so whether it + # reads as above or below the published rc depends on the value: + # rc.99999999999999999999 wraps to 7766279631452241919 and sails + # through. The cut then lands a tag that pins highest_rc_for_base + # at 1e20 forever, and every later cut wraps to a *lower* rc that + # sorts below it, so the fleet never updates again. Nine digits is + # far above any real series and exact in bash math either way. + if [[ ! "$explicit" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-rc\.(0|[1-9][0-9]{0,8})(\.[0-9A-Za-z]+)?)?$ ]]; then + echo "::error::version must be X.Y.Z, X.Y.Z-rc.N, or X.Y.Z-rc.N.suffix, got: $EXPLICIT_VERSION" >&2 exit 1 fi + # Why route the embedded identifier through the same validator the + # kind path uses: the regex above only checks shape, and rc.4.01 + # is a shape-valid identifier that is not valid semver. + if [[ "$explicit" == *-rc.*.* ]]; then + require_valid_version_suffix "${explicit##*.}" + fi # Same updater-safety gate the kind path enforces: stable line must # strictly increase over the latest published stable (prerelease # identifiers ignored for the comparison). @@ -509,7 +550,44 @@ jobs: echo "::error::Refusing explicit version $explicit: not greater than latest stable $latest_stable." >&2 exit 1 fi + # Why a second gate for prereleases: semver_gt compares through + # strip_pre(), so the stable-line check reads 1.4.156-rc.0 as + # 1.4.156 and waves it past a 1.4.155 stable even when rc.0..rc.3 + # already shipped — republishing an RC *below* what clients run, + # the same regression class as the rc.4 cut that orphaned live + # daemons. Anchor on the same rc history the kind path uses so the + # override can only ever advance the series it targets. + if [[ "$explicit" == *-rc.* ]]; then + explicit_base="${explicit%%-*}" + explicit_rc="${explicit#*-rc.}" + explicit_rc="${explicit_rc%%.*}" + highest_explicit_rc="$(highest_rc_for_base "$explicit_base")" + if [[ -n "$highest_explicit_rc" && "$explicit_rc" -le "$highest_explicit_rc" ]]; then + # Why the remedy is spelled this narrowly: kind=rc derives its + # base from bump(latest_stable, patch), so it can only resume a + # series on that base. A minor/major series (1.5.0-rc.N) exists + # only because this override created it, and pointing an + # operator at kind=rc there would cut an unrelated release. + echo "::error::Refusing explicit version $explicit: rc.$explicit_rc is not above rc.$highest_explicit_rc, the highest already cut for $explicit_base. Request rc.$((highest_explicit_rc + 1)) or higher. If you are resuming an unpublished tag and $explicit_base is the next patch after latest stable $latest_stable, dispatch kind=rc instead, which recovers that tag when it was cut from the ref you dispatch; otherwise cut rc.$((highest_explicit_rc + 1)) and leave the unpublished tag alone." >&2 + exit 1 + fi + fi new="$explicit" + # Why here too: the suffix append below lives in the kind path the + # override skips, so an operator passing both inputs used to get + # their suffix silently dropped. Only a bare rc can take one — a + # stable X.Y.Z.perf is not valid semver, and re-suffixing an + # already-suffixed rc would produce rc.N.perf.perf. + if [[ -n "${VERSION_SUFFIX:-}" ]]; then + # Same bounded rc pattern as the shape check above, so the two + # cannot drift apart under a later edit. + if [[ ! "$explicit" =~ ^[0-9]+\.[0-9]+\.[0-9]+-rc\.(0|[1-9][0-9]{0,8})$ ]]; then + echo "::error::version_suffix applies only to a bare X.Y.Z-rc.N version, got: $explicit" >&2 + exit 1 + fi + require_valid_version_suffix "$VERSION_SUFFIX" + new="${new}.${VERSION_SUFFIX}" + fi echo "Explicit version override: $new" fi @@ -539,15 +617,7 @@ jobs: new="${base}-rc.$((highest_rc + 1))" fi if [[ -n "${VERSION_SUFFIX:-}" ]]; then - # Why a dot-appended identifier (rc.N.perf): it sorts just - # above its own base rc.N but BELOW rc.N+1, so suffixed side- - # branch builds never outrank the main RC series and cannot - # hijack the update channel; clients find them by matching the - # identifier ("perf") in the prerelease components. - if [[ ! "$VERSION_SUFFIX" =~ ^[0-9A-Za-z]+$ ]]; then - echo "::error::version_suffix must be alphanumeric, got: $VERSION_SUFFIX" >&2 - exit 1 - fi + require_valid_version_suffix "$VERSION_SUFFIX" new="${new}.${VERSION_SUFFIX}" fi ;; diff --git a/config/scripts/release-rc-history.mjs b/config/scripts/release-rc-history.mjs index c6a0e7b4782..4a0db3f03db 100644 --- a/config/scripts/release-rc-history.mjs +++ b/config/scripts/release-rc-history.mjs @@ -38,7 +38,12 @@ export function rcNumberFromReleaseSubject(base, subject) { return null } - const match = /^(\d+)(?:\s|$)/.exec(subject.slice(prefix.length)) + // Why the same optional .identifier as the tag form: the commit subject is + // the only record left once a tag is deleted, and that is exactly when the + // explicit-version gate leans on this. Without it, deleting a + // v1.2.3-rc.4.perf tag drops the series back to rc.3 and an explicit + // 1.2.3-rc.4 is waved through — below what perf-channel clients already run. + const match = /^(\d+)(?:\.[0-9A-Za-z]+)?(?:\s|$)/.exec(subject.slice(prefix.length)) return match ? Number(match[1]) : null } diff --git a/config/scripts/release-rc-history.test.mjs b/config/scripts/release-rc-history.test.mjs index 7ceb28f3410..5e2e051c238 100644 --- a/config/scripts/release-rc-history.test.mjs +++ b/config/scripts/release-rc-history.test.mjs @@ -53,6 +53,29 @@ describe('release RC history', () => { expect(rcNumberFromReleaseSubject('1.4.36', 'fix: v1.4.36-rc.6')).toBeNull() }) + it('counts a suffixed side-branch RC from its subject as well as its tag', () => { + expect(rcNumberFromTag('1.4.36', 'v1.4.36-rc.6.perf')).toBe(6) + expect(rcNumberFromReleaseSubject('1.4.36', 'release: v1.4.36-rc.6.perf')).toBe(6) + expect( + rcNumberFromReleaseSubject('1.4.36', 'release: v1.4.36-rc.6.perf [rc-slot:2026-05-30-03]') + ).toBe(6) + }) + + it('keeps a suffixed RC counted once its tag is deleted', () => { + withGitRepo((repo) => { + commit(repo, 'initial') + commit(repo, 'release: v1.4.36-rc.5') + git(repo, ['tag', 'v1.4.36-rc.5']) + // Why this case: the subject is the only record left after the tag goes, + // and that is precisely when release-cut's explicit-version gate reads + // this. Under-reporting rc.6 here lets an explicit 1.4.36-rc.6 cut land + // below the v1.4.36-rc.6.perf build that clients already run. + commit(repo, 'release: v1.4.36-rc.6.perf') + + expect(highestRcForBase('1.4.36', { cwd: repo })).toBe(6) + }) + }) + it('keeps RC numbers monotonic after a stale tag is deleted', () => { withGitRepo((repo) => { commit(repo, 'initial')