From 202d9ac050422017fa592fc1d65043a1bf13caa3 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Tue, 1 Sep 2026 17:49:27 -0600 Subject: [PATCH] fix(scripts): measure a version bump against the last published version, not the base branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ported verbatim from agent-runtime#1074. `scripts/check-version-bump.mjs` and `tests/version-bump-check.test.ts` stay byte-identical with that repository, as the script's own header requires. The gate asked that a consumer-visible change ship under a version strictly higher than the BASE BRANCH's. That reads an unreleased version as if it were released. main routinely carries a bump that has not shipped, and under the old rule the first consumer-visible change claimed that open slot while every later change in the same train was told to open another. Measured 2026-09-01 on agent-runtime, which carries the same file. main declared 0.190.0 while the registry's newest version was 0.189.0. Two export-adding pull requests were open against it and both were refused — each for adding exports to a version no consumer could yet resolve — and both had to be merged with admin over a red gate. One unpublished bump can absorb every consumer-visible change until it ships, which is what a release train is for. A version that IS on the registry still demands its own bump. That is the defect this file exists for and it is untouched. "Published" resolves in three steps: the highest `v*` tag reachable from the base, with each package's version read from ITS OWN manifest at that tag; else the npm registry's `latest`, best effort with a five-second timeout; else the base branch's version, exactly as before, because unknown publication state must never weaken the check. This repository is unaffected today — it declares 13.0.0, npm's latest is 13.0.0, and the newest reachable tag v13.0.0 carries 13.0.0, so the baseline is the same number the check already used. The fix applies the first time a release here carries an unshipped bump. Tests: the same seven cases. Four fail without this change, verified by stashing the script. Three pass before and after on purpose, pinning what must not move. The existing 29 now run hermetically — `PACKAGE_VERSION_BUMP_REGISTRY=0` keeps a tagless fixture off the real registry. --- scripts/check-version-bump.mjs | 127 +++++++++++++++++++-- tests/version-bump-check.test.ts | 182 +++++++++++++++++++++++++++++++ 2 files changed, 301 insertions(+), 8 deletions(-) diff --git a/scripts/check-version-bump.mjs b/scripts/check-version-bump.mjs index 0605df4..faece75 100644 --- a/scripts/check-version-bump.mjs +++ b/scripts/check-version-bump.mjs @@ -65,10 +65,42 @@ * everything repo-specific out of the manifests they inspect, so an edit to one * belongs in all three. Nothing here may name a single repository. * + * The version a change is measured against is the LAST PUBLISHED one, not the + * base branch's. Those differ whenever main already carries a bump that has not + * shipped, and reading the base branch instead made an unreleased version behave + * like a released one: the first consumer-visible change claimed the open slot + * and every later one in the same train was told to open another. + * + * Measured 2026-09-01 on agent-runtime. Two export-adding pull requests were + * open against a main that declared 0.190.0 while the registry's newest version + * was 0.189.0. Nothing could ship under 0.190.0 without a second bump, so #1065 + * and #1066 were each refused, each for adding exports to a version no consumer + * could yet resolve, and both had to be merged with admin over a red gate. One + * unpublished bump can absorb every consumer-visible change until it ships, + * which is what a release train is for. + * + * A version that IS on the registry still demands its own bump. That is the + * defect this file exists for, and it is unchanged: 0.119.0 moved a peer floor + * under a version npm already held, and `publish.yml` skips a version already + * published, so re-tagging could never correct it. + * + * What "published" means here, in order: + * + * 1. The highest `v*` tag reachable from the base — the repository's own + * record of what it released, since `publish.yml` fires on that tag. Each + * package's released version is read from ITS manifest at that tag, so a + * workspace package with its own version line is never handed the root's. + * 2. The npm registry's `latest`, consulted only when no tag is reachable and + * only as a best effort. A registry that cannot be reached changes nothing. + * 3. Neither: fall back to the base branch's version, which is the behavior + * this check has always had. Unknown publication state must not weaken it. + * * Usage: pnpm run check:version-bump - * PACKAGE_VERSION_BUMP_BASE base ref to compare against (default: the - * CI base branch, else origin/main, else main) - * PACKAGE_VERSION_BUMP_ROOT repository to inspect (default: this repo) + * PACKAGE_VERSION_BUMP_BASE base ref to compare against (default: the + * CI base branch, else origin/main, else main) + * PACKAGE_VERSION_BUMP_ROOT repository to inspect (default: this repo) + * PACKAGE_VERSION_BUMP_REGISTRY `0` skips the registry fallback entirely, + * for a hermetic or offline run */ import { execFileSync } from 'node:child_process' import { readFileSync } from 'node:fs' @@ -340,6 +372,52 @@ const surfaceAtRef = (ref, manifestPath) => { /** Rank a level, treating `lower` and `unorderable` as paying nothing. */ const rankOf = (level) => LEVEL_RANK[level] ?? 0 +/** True when `candidate` is strictly higher than `reference`. */ +const isHigher = (reference, candidate) => rankOf(versionBumpLevel(reference, candidate)) > 0 + +/** + * The newest `v*` tag reachable from `ref`, or `null`. + * + * Reachability is the point: a tag on a branch nobody merged describes nothing + * this base contains. Ordering is semantic, never lexical — `v0.99.0` sorts + * above `v0.100.0` as a string, and picking the wrong tag would compare against + * a version older than the one that actually shipped. + */ +const lastReleaseTag = (ref) => { + const tags = (git(['tag', '--list', 'v*', '--merged', ref], { allowFailure: true }) ?? '') + .split('\n') + .map((tag) => tag.trim()) + .filter((tag) => tag.length > 0) + let newest = null + for (const tag of tags) { + if (newest === null || isHigher(newest.slice(1), tag.slice(1))) newest = tag + } + return newest +} + +/** + * `latest` from the npm registry for one package, or `undefined`. + * + * Best effort by construction. This runs only when no release tag is reachable, + * and every failure — offline, private package, never published, a slow + * registry — returns `undefined` and leaves the check exactly as it was. + */ +const registryVersion = (name) => { + if (process.env.PACKAGE_VERSION_BUMP_REGISTRY === '0') return undefined + try { + const stdout = execFileSync('npm', ['view', name, 'version', '--silent'], { + encoding: 'utf8', + timeout: 5_000, + stdio: ['ignore', 'pipe', 'ignore'], + }) + const version = stdout.trim() + return version.length > 0 ? version : undefined + } catch { + return undefined + } +} + + const base = resolveBase() if (base === null) { process.stdout.write('No base ref to compare against; version-bump check does not apply.\n') @@ -364,6 +442,20 @@ const headWorkspace = workspaceAtRef(head) const baseManifests = manifestsAtRef(mergeBase, baseWorkspace) const headManifests = manifestsAtRef(head, headWorkspace) +// What a consumer can ALREADY resolve, per package name. The base branch's +// version is not that: main routinely carries a bump that has not shipped, and +// one unpublished bump absorbs every consumer-visible change until it does. +const releaseTag = lastReleaseTag(mergeBase) +// Each package's released version comes from ITS OWN manifest at the tag, so a +// workspace package on a separate version line is never handed the root's. +const releasedManifests = + releaseTag === null ? null : manifestsAtRef(releaseTag, workspaceAtRef(releaseTag)) +const releasedFrom = releaseTag === null ? 'the npm registry' : `${releaseTag}` +const releasedVersionOf = (name) => + releasedManifests === null + ? registryVersion(name) + : releasedManifests.get(name)?.manifest.version + const publishable = (entry) => entry !== undefined && entry.manifest.private !== true const failures = [] const inspected = [] @@ -432,20 +524,29 @@ for (const name of names) { continue } + // Measure the bump against the last PUBLISHED version. When the base already + // carries a bump nobody can resolve yet, that bump pays for this change too; + // when the base's version is on the registry, it pays for nothing. + const releasedVersion = releasedVersionOf(name) + const absorbing = + releasedVersion !== undefined && isHigher(releasedVersion, baseEntry.manifest.version) + const comparisonVersion = absorbing ? releasedVersion : baseEntry.manifest.version + // The manifest rule asks only for a higher version; the export rule asks for a // level. The stronger of the two governs. const requiredLevel = (() => { - const forExports = requiredBumpLevel(severity, baseEntry.manifest.version) + const forExports = requiredBumpLevel(severity, comparisonVersion) const forManifest = changes.length > 0 ? 'patch' : 'none' return rankOf(forExports) >= rankOf(forManifest) ? forExports : forManifest })() - const paidLevel = versionBumpLevel(baseEntry.manifest.version, headEntry.manifest.version) + const paidLevel = versionBumpLevel(comparisonVersion, headEntry.manifest.version) if (rankOf(paidLevel) >= rankOf(requiredLevel)) { inspected.push( `${headEntry.path}: ${changes.length} manifest and ${exportLines.length} export change(s) ` + `needing a ${requiredLevel} bump, paid for by ` + - `${baseEntry.manifest.version} -> ${headEntry.manifest.version} (${paidLevel})`, + `${comparisonVersion} -> ${headEntry.manifest.version} (${paidLevel})` + + (absorbing ? ` — ${comparisonVersion} is the last published version (${releasedFrom})` : ''), ) continue } @@ -453,6 +554,8 @@ for (const name of names) { path: headEntry.path, name, baseVersion: baseEntry.manifest.version, + comparisonVersion, + absorbing, headVersion: headEntry.manifest.version, changes: [...changes, ...exportLines], requiredLevel, @@ -474,8 +577,10 @@ if (failures.length > 0) { continue } const versionState = (() => { - if (failure.baseVersion === failure.headVersion) return `still declares ${failure.headVersion}` - const move = `moves ${failure.baseVersion} -> ${failure.headVersion}` + if (failure.comparisonVersion === failure.headVersion) { + return `still declares ${failure.headVersion}` + } + const move = `moves ${failure.comparisonVersion} -> ${failure.headVersion}` return rankOf(failure.paidLevel) === 0 ? `${move}, which is not higher` : `${move}, only a ${failure.paidLevel} bump` @@ -486,6 +591,12 @@ if (failures.length > 0) { `${severityLabel === 'additive' ? 'an' : 'a'} ${severityLabel} change needing a ` + `${failure.requiredLevel} bump:`, ) + if (failure.absorbing) { + lines.push( + ` measured against the last published version ${failure.comparisonVersion} ` + + `(${releasedFrom}); the base declares ${failure.baseVersion}, which is not published yet`, + ) + } for (const change of failure.changes) lines.push(` ${change}`) lines.push('') } diff --git a/tests/version-bump-check.test.ts b/tests/version-bump-check.test.ts index ae12f98..ccb3439 100644 --- a/tests/version-bump-check.test.ts +++ b/tests/version-bump-check.test.ts @@ -129,10 +129,19 @@ async function check(root: string, base: string) { ...process.env, PACKAGE_VERSION_BUMP_ROOT: root, PACKAGE_VERSION_BUMP_BASE: base, + // Hermetic: a fixture with no release tag would otherwise fall back to the + // real registry, where these package names ARE published, and every + // fixture version would be measured against a stranger's release. + PACKAGE_VERSION_BUMP_REGISTRY: '0', }, }) } +/** Mark a fixture commit as released, the way `publish.yml` marks a real one. */ +async function tag(root: string, name: string): Promise { + await git(root, 'tag', name) +} + afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) }) @@ -666,3 +675,176 @@ describe('a change to the exported symbols requires a version bump', () => { }) }) }) + +/** + * The version a change is measured against is the LAST PUBLISHED one, not the + * base branch's. + * + * Reading the base branch instead made an unreleased version behave like a + * released one. Measured 2026-09-01 on agent-runtime: main declared 0.190.0 + * while the registry's newest was 0.189.0, and two export-adding pull requests + * open against that main were each refused for adding exports to a version no + * consumer could yet resolve. One unpublished bump absorbs every consumer-visible + * change until it ships; a published version still buys nothing. + */ +describe('the baseline is the last published version, not the base branch', () => { + /** Release `v1.0.0`, then put an unshipped `1.1.0` on the base. */ + async function repoWithUnpublishedBump(): Promise<{ root: string; base: string }> { + const root = await createRepo() + await tag(root, 'v1.0.0') + await writeManifests(root, { + version: '1.1.0', + evalPeer: '>=0.140.1 <0.141.0', + knowledgeCatalog: '7.0.4', + benchVersion: '0.4.9', + }) + await commit(root, 'prepare 1.1.0') + return { root, base: (await git(root, 'rev-parse', 'HEAD')).trim() } + } + + it('accepts added exports when the base carries a bump that has not shipped', async () => { + const { root, base } = await repoWithUnpublishedBump() + await writeSurface(root, '.', '@tangle-network/agent-runtime', { + '.': { runAgent: 'value', AgentSpec: 'type', supervise: 'value' }, + }) + await commit(root, 'add an export under the unshipped 1.1.0') + + // 1.1.0 is not on the registry, so it is still paying for its own contents. + const result = await check(root, base) + expect(result.stdout).toContain('paid for by 1.0.0 -> 1.1.0 (minor)') + expect(result.stdout).toContain('1.0.0 is the last published version (v1.0.0)') + }) + + it('accepts a SECOND change under the same unshipped bump — the release-train case', async () => { + const { root, base } = await repoWithUnpublishedBump() + // The first change already landed on the base; this is the one that used to + // be told to open a version of its own. + await writeSurface(root, '.', '@tangle-network/agent-runtime', { + '.': { runAgent: 'value', AgentSpec: 'type', supervise: 'value' }, + }) + await commit(root, 'first export-adding change') + const secondBase = (await git(root, 'rev-parse', 'HEAD')).trim() + await writeSurface(root, '.', '@tangle-network/agent-runtime', { + '.': { runAgent: 'value', AgentSpec: 'type', supervise: 'value', runGraph: 'value' }, + }) + await commit(root, 'second export-adding change') + + await expect(check(root, secondBase)).resolves.toMatchObject({ + stdout: expect.stringContaining('paid for by 1.0.0 -> 1.1.0 (minor)'), + }) + }) + + it('still refuses added exports when the base version IS published', async () => { + const root = await createRepo() + await tag(root, 'v1.0.0') + const base = (await git(root, 'rev-parse', 'HEAD')).trim() + await writeSurface(root, '.', '@tangle-network/agent-runtime', { + '.': { runAgent: 'value', AgentSpec: 'type', supervise: 'value' }, + }) + await commit(root, 'add an export under the published 1.0.0') + + // The defect this file exists for is unchanged: npm already holds 1.0.0, and + // publish.yml skips a version already on the registry. + const failure = await check(root, base).catch((error) => error) + expect(failure.stderr).toContain('still declares 1.0.0') + expect(failure.stderr).toContain('export added: . supervise') + }) + + it('names the published baseline in the refusal when the base is ahead but still short', async () => { + const root = await createRepo() + await tag(root, 'v1.0.0') + await writeManifests(root, { + version: '1.0.1', + evalPeer: '>=0.140.1 <0.141.0', + knowledgeCatalog: '7.0.4', + benchVersion: '0.4.9', + }) + await commit(root, 'prepare 1.0.1') + const base = (await git(root, 'rev-parse', 'HEAD')).trim() + // A 1.x package needs a MINOR for an addition, and the unshipped bump is only + // a patch, so the absorbing case applies and still falls short. + await writeSurface(root, '.', '@tangle-network/agent-runtime', { + '.': { runAgent: 'value', AgentSpec: 'type', supervise: 'value' }, + }) + await commit(root, 'add an export under the unshipped 1.0.1') + + const failure = await check(root, base).catch((error) => error) + expect(failure.stderr).toContain('moves 1.0.0 -> 1.0.1, only a patch bump') + expect(failure.stderr).toContain( + 'measured against the last published version 1.0.0 (v1.0.0); the base declares 1.0.1, ' + + 'which is not published yet', + ) + }) + + it('passes a change with no consumer surface, published or not', async () => { + const { root, base } = await repoWithUnpublishedBump() + await writeFile(join(root, 'source.ts'), 'export const value = 2\n') + await commit(root, 'change only source') + + await expect(check(root, base)).resolves.toMatchObject({ + stdout: expect.stringContaining('consumer surface unchanged at 1.1.0'), + }) + }) + + it('orders release tags semantically, so v0.100.0 outranks v0.99.0', async () => { + const root = await createRepo() + await writeManifests(root, { + version: '0.99.0', + evalPeer: '>=0.140.1 <0.141.0', + knowledgeCatalog: '7.0.4', + benchVersion: '0.4.9', + }) + await commit(root, 'release 0.99.0') + await tag(root, 'v0.99.0') + await writeManifests(root, { + version: '0.100.0', + evalPeer: '>=0.140.1 <0.141.0', + knowledgeCatalog: '7.0.4', + benchVersion: '0.4.9', + }) + await commit(root, 'release 0.100.0') + await tag(root, 'v0.100.0') + await writeManifests(root, { + version: '0.101.0', + evalPeer: '>=0.140.1 <0.141.0', + knowledgeCatalog: '7.0.4', + benchVersion: '0.4.9', + }) + await commit(root, 'prepare 0.101.0') + const base = (await git(root, 'rev-parse', 'HEAD')).trim() + await writeSurface(root, '.', '@tangle-network/agent-runtime', { + '.': { runAgent: 'value', AgentSpec: 'type', supervise: 'value' }, + }) + await commit(root, 'add an export') + + // Lexically v0.99.0 is the larger string. Picking it would measure against a + // version two releases stale. + await expect(check(root, base)).resolves.toMatchObject({ + stdout: expect.stringContaining('paid for by 0.100.0 -> 0.101.0'), + }) + }) + + it('reads each package own released version at the tag, never the root one', async () => { + const root = await createRepo() + await tag(root, 'v1.0.0') + // The root ships an unpublished bump; bench does not move at all. + await writeManifests(root, { + version: '1.1.0', + evalPeer: '>=0.140.1 <0.141.0', + knowledgeCatalog: '7.0.4', + benchVersion: '0.4.9', + }) + await commit(root, 'prepare 1.1.0') + const base = (await git(root, 'rev-parse', 'HEAD')).trim() + await writeSurface(root, 'bench', '@tangle-network/agent-bench', { + '.': { runBench: 'value', runSuite: 'value' }, + }) + await commit(root, 'add a bench export') + + // bench released 0.4.9 and still declares 0.4.9, so the root's unshipped + // 1.1.0 pays nothing for it. + const failure = await check(root, base).catch((error) => error) + expect(failure.stderr).toContain('bench/package.json (@tangle-network/agent-bench)') + expect(failure.stderr).toContain('still declares 0.4.9') + }) +})