From d7ab20ac06c37e33a2cda2d2fd1dfb71822a43bd Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 09:22:09 -0700 Subject: [PATCH] feat: accept 3-digit semver + package.json version sources (#2501) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two version-source shapes failed CLOSED in a way that silently disabled /ship's queue-collision check: 1. A --version-path / .gstack/version-path target that is a package.json was read as raw text: the whitespace strip turned the JSON into '{"name":"frontend",... which parseVersion rejected, so every read — local, `git show`, and rival PRs' claims through the GitHub/GitLab Contents APIs — fell back to 0.0.0.0 and competing claims were dropped as "malformed". 2. parseVersion required exactly four components, so gstack-next-version exited 2 on EVERY invocation in a 3-digit repo. That CLI IS the queue-collision check; /ship then took its documented offline path of naive local arithmetic, two branches cut from the same base picked the same version, and git merged the duplicate without a conflict. New lib/version-source.ts holds the shared semantics so both CLIs agree by construction: parseVersion accepts 3- or 4-digit (3 pads the micro slot for uniform comparison), versionWidth/fmtVersion keep a 3-digit repo 3-digit through bumping and formatting, micro coerces to patch on 3-digit repos (with a warning in the output), and extractVersion reads a .json version-path as JSON (.version) from any byte source. gstack-version-bump treats a package.json version-path as that repo's single source of truth (written in place, DRIFT_* states can't arise — no second file to drift from). Detection is by shape, not new configuration. Scope per the wave plan's version-tooling end-state spec (decision 11, ENG-OV1): this is the READING capability + 3-digit acceptance ONLY. gstack's own VERSION file stays the 4-digit source of truth; nothing here flips authority to package.json. The PR's bundled fix for the .gstack/version-path pin being ignored by classify's base read lands separately (#2462) — these tests drive the JSON version-path through the explicit --version-path flag. Re-derived from PR #2501 by @YiftahR (73 tests pass across test/gstack-version-bump.test.ts, test/gstack-next-version.test.ts, test/ship-version-sync.test.ts). Fixes #2501 Co-authored-by: YR Co-Authored-By: Claude Fable 5 --- bin/gstack-next-version | 97 +++++++++++++------------ bin/gstack-version-bump | 59 +++++++++++++--- lib/version-source.ts | 118 +++++++++++++++++++++++++++++++ test/gstack-next-version.test.ts | 59 +++++++++++++++- test/gstack-version-bump.test.ts | 85 +++++++++++++++++++++- 5 files changed, 359 insertions(+), 59 deletions(-) create mode 100644 lib/version-source.ts diff --git a/bin/gstack-next-version b/bin/gstack-next-version index 76879ddbd..c59a9dcaa 100755 --- a/bin/gstack-next-version +++ b/bin/gstack-next-version @@ -19,6 +19,13 @@ // committed so all collaborators benefit) // 3. "VERSION" at the repo root (default, backward-compatible) // +// The pinned path may be a package.json (any depth) rather than a plain-text +// VERSION file: a path ending in .json is read as JSON and its .version taken. +// 3-digit semver is accepted as well as 4-digit, and stays 3-digit through +// bumping. See lib/version-source.ts for why both mattered — each used to fail +// closed, which silently disabled the queue-collision check this CLI exists to +// provide (#2501). +// // Exit codes: // 0 — emitted JSON successfully (may include "offline":true or "host":"unknown") // 2 — invalid arguments @@ -28,9 +35,18 @@ import { execFileSync, spawnSync } from "node:child_process"; import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; - -type Bump = "major" | "minor" | "patch" | "micro"; -type Version = [number, number, number, number]; +import { + parseVersion, + versionWidth, + fmtVersion, + bumpVersion, + cmpVersion, + bumpWasCoerced, + extractVersion, + type Bump, + type Version, + type VersionWidth, +} from "../lib/version-source"; type ClaimedPR = { pr: number; @@ -66,48 +82,20 @@ type Output = { const ACTIVE_SIBLING_MAX_AGE_S = 24 * 60 * 60; const GH_API_CONCURRENCY = 10; -function parseVersion(s: string): Version | null { - const m = s.trim().match(/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/); - if (!m) return null; - return [Number(m[1]), Number(m[2]), Number(m[3]), Number(m[4])]; -} - -function fmtVersion(v: Version): string { - return v.join("."); -} - -function bumpVersion(v: Version, level: Bump): Version { - switch (level) { - case "major": - return [v[0] + 1, 0, 0, 0]; - case "minor": - return [v[0], v[1] + 1, 0, 0]; - case "patch": - return [v[0], v[1], v[2] + 1, 0]; - case "micro": - return [v[0], v[1], v[2], v[3] + 1]; - } -} - -function cmpVersion(a: Version, b: Version): number { - for (let i = 0; i < 4; i++) { - if (a[i] !== b[i]) return a[i] - b[i]; - } - return 0; -} - // Collision resolution: bump past the highest claimed within the same level. // Semantics: if my bump is MINOR and the queue claims 1.7.0.0, I advance to // 1.8.0.0 (still a MINOR relative to main). Preserves ship-time intent. -function pickNextSlot(base: Version, claimed: Version[], level: Bump): { version: Version; reason: string } { - let candidate = bumpVersion(base, level); +// `width` keeps a 3-digit repo 3-digit (see lib/version-source.ts); it +// defaults to 4 so existing callers and tests are unaffected. +function pickNextSlot(base: Version, claimed: Version[], level: Bump, width: VersionWidth = 4): { version: Version; reason: string } { + let candidate = bumpVersion(base, level, width); const sortedClaimed = [...claimed].sort(cmpVersion); const highest = sortedClaimed[sortedClaimed.length - 1]; if (highest && cmpVersion(highest, base) > 0) { // Queue already advanced past base; bump past the highest claim. - const bumpedPastHighest = bumpVersion(highest, level); + const bumpedPastHighest = bumpVersion(highest, level, width); if (cmpVersion(bumpedPastHighest, candidate) > 0) { - return { version: bumpedPastHighest, reason: `bumped past claimed ${fmtVersion(highest)}` }; + return { version: bumpedPastHighest, reason: `bumped past claimed ${fmtVersion(highest, width)}` }; } } return { version: candidate, reason: "no collision; clean bump from base" }; @@ -167,7 +155,12 @@ function readBaseVersion(base: string, versionPath: string, warnings: string[]): warnings.push(`could not read ${versionPath} at origin/${base}; assuming 0.0.0.0`); return "0.0.0.0"; } - return r.stdout.trim(); + const v = extractVersion(r.stdout, versionPath); + if (!v) { + warnings.push(`${versionPath} at origin/${base} has no readable version; assuming 0.0.0.0`); + return "0.0.0.0"; + } + return v; } async function fetchGithubClaimed(base: string, versionPath: string, excludePR: number | null, warnings: string[]): Promise<{ claimed: ClaimedPR[]; offline: boolean }> { @@ -233,7 +226,7 @@ async function fetchGithubClaimed(base: string, versionPath: string, excludePR: } let versionStr: string; try { - versionStr = Buffer.from(content.stdout.trim(), "base64").toString("utf8").trim(); + versionStr = extractVersion(Buffer.from(content.stdout.trim(), "base64").toString("utf8"), versionPath); } catch { warnings.push(`PR #${pr.number}: VERSION is not valid base64`); continue; @@ -290,7 +283,7 @@ async function fetchGitlabClaimed(base: string, versionPath: string, excludePR: } try { const j = JSON.parse(content.stdout); - const versionStr = Buffer.from(j.content, "base64").toString("utf8").trim(); + const versionStr = extractVersion(Buffer.from(j.content, "base64").toString("utf8"), versionPath); if (!parseVersion(versionStr)) { warnings.push(`MR !${mr.iid}: VERSION malformed (${versionStr})`); continue; @@ -349,7 +342,7 @@ function scanSiblings(root: string | null, versionPath: string, claimed: Claimed if (!existsSync(versionFile)) continue; let version: string; try { - version = readFileSync(versionFile, "utf8").trim(); + version = extractVersion(readFileSync(versionFile, "utf8"), versionPath); if (!parseVersion(version)) continue; } catch { continue; @@ -469,6 +462,13 @@ async function main() { console.error(`Error: could not parse base version '${baseVersion}'`); process.exit(2); } + // The repo's own width governs everything downstream: a 3-digit repo must + // not be handed a 4-digit slot, or /ship writes a version the repo's tooling + // can't read back (#2501). + const width = versionWidth(baseVersion); + if (bumpWasCoerced(args.bump, width)) { + warnings.push(`--bump micro has no component to move in a ${width}-digit version; treated as patch`); + } const excludePR = args.excludePR ?? autoDetectExcludePR(); if (excludePR !== null && args.excludePR === null) { @@ -495,7 +495,7 @@ async function main() { .map((c) => parseVersion(c.version)) .filter((v): v is Version => v !== null); - const { version: picked, reason } = pickNextSlot(baseParsed, claimedVersions, args.bump); + const { version: picked, reason } = pickNextSlot(baseParsed, claimedVersions, args.bump, width); const workspaceRoot = resolveWorkspaceRoot(args.workspaceRoot); const siblings = markActiveSiblings(scanSiblings(workspaceRoot, versionPath, claimed, warnings), baseParsed); @@ -510,12 +510,12 @@ async function main() { .filter((v) => cmpVersion(v, finalVersion) >= 0); if (activeAhead.length) { const highest = activeAhead.sort(cmpVersion)[activeAhead.length - 1]; - finalVersion = bumpVersion(highest, args.bump); - finalReason = `bumped past active sibling ${fmtVersion(highest)}`; + finalVersion = bumpVersion(highest, args.bump, width); + finalReason = `bumped past active sibling ${fmtVersion(highest, width)}`; } const out: Output = { - version: fmtVersion(finalVersion), + version: fmtVersion(finalVersion, width), current_version: args.current || baseVersion, base_version: baseVersion, version_path: versionPath, @@ -531,8 +531,11 @@ async function main() { process.stdout.write(JSON.stringify(out, null, 2) + "\n"); } -// Pure-function exports for testing -export { parseVersion, fmtVersion, bumpVersion, cmpVersion, pickNextSlot, markActiveSiblings, resolveVersionPath }; +// Pure-function exports for testing. The version primitives are re-exported +// from lib/version-source so existing importers of this module keep working +// unchanged. +export { parseVersion, fmtVersion, bumpVersion, cmpVersion, versionWidth, extractVersion }; +export { pickNextSlot, markActiveSiblings, resolveVersionPath }; // Only run main() when invoked as a script, not when imported by tests. if (import.meta.main) { diff --git a/bin/gstack-version-bump b/bin/gstack-version-bump index 298fab17d..1e47105cb 100755 --- a/bin/gstack-version-bump +++ b/bin/gstack-version-bump @@ -37,8 +37,13 @@ import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { join } from "node:path"; +import { extractVersion, isJsonVersionPath, setVersionInJson } from "../lib/version-source"; -const VERSION_RE = /^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/; +// 3- or 4-digit (#2501). gstack's own VERSION stays 4-digit MAJOR.MINOR.PATCH. +// MICRO and stays the source of truth, but a repo whose pinned version source +// is a package.json holds plain 3-digit semver, and rejecting it here meant +// /ship could not write a version at all in such a repo. See lib/version-source.ts. +const VERSION_RE = /^[0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?$/; const DEFAULT = "0.0.0.0"; type State = "FRESH" | "ALREADY_BUMPED" | "DRIFT_STALE_PKG" | "DRIFT_UNEXPECTED"; @@ -64,9 +69,11 @@ function resolveVersionPath(cwd: string, explicit?: string): string { return join(cwd, "VERSION"); } -function readVersionFile(p: string): string { +function readVersionFile(p: string, versionRel = "VERSION"): string { try { - const v = readFileSync(p, "utf-8").replace(/[\r\n\s]/g, ""); + // extractVersion (#2501): a .json version-path is read as JSON (.version), + // not whitespace-stripped raw text that turns a package.json into garbage. + const v = extractVersion(readFileSync(p, "utf-8"), versionRel); return v || DEFAULT; } catch { return DEFAULT; @@ -110,8 +117,7 @@ function baseVersion(cwd: string, base: string, versionRel: string): string { } try { const out = execFileSync("git", ["show", `origin/${base}:${versionRel}`], { cwd }).toString(); - const v = out.replace(/[\r\n\s]/g, ""); - return v || DEFAULT; + return extractVersion(out, versionRel) || DEFAULT; } catch { // VERSION absent on base (new repo / new file) → treat as 0.0.0.0. return DEFAULT; @@ -135,9 +141,16 @@ function cmdClassify(args: string[], cwd: string): void { if (!base) fail("classify requires --base ", 2); const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path")); const versionRel = argVal(args, "--version-path") ?? "VERSION"; - const current = readVersionFile(versionPath); + const current = readVersionFile(versionPath, versionRel); const baseV = baseVersion(cwd, base!, versionRel); - const pkg = readPkgVersion(cwd); + // When the version-path IS a package.json (#2501), that file is the single + // source of truth and the "VERSION vs package.json" drift states cannot + // arise — they are the same file. Reporting it as its own pkg keeps DRIFT_* + // out of the classification instead of inventing a disagreement between a + // file and itself. + const pkg = isJsonVersionPath(versionRel) + ? { exists: existsSync(versionPath), version: current === DEFAULT ? "" : current } + : readPkgVersion(cwd); const state = classifyState(current, baseV, pkg.exists, pkg.version); process.stdout.write( JSON.stringify({ @@ -160,6 +173,26 @@ function cmdWrite(args: string[], cwd: string): void { fail(`NEW_VERSION (${version}) does not match MAJOR.MINOR.PATCH.MICRO. Aborting.`, 2); } const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path")); + const versionRel = argVal(args, "--version-path") ?? "VERSION"; + + // A package.json version-path (#2501) is written in place, keeping the rest + // of the file intact — and it is the ONLY file written. Also syncing a root + // package.json here would be a guess about which of two JSON files the repo + // actually publishes from; in a monorepo whose truth is frontend/package.json + // the root one either doesn't exist or isn't the version users see. + if (isJsonVersionPath(versionRel)) { + if (!existsSync(versionPath)) { + fail(`write: ${versionRel} does not exist. Check --version-path / .gstack/version-path.`, 2); + } + try { + writeFileSync(versionPath, setVersionInJson(readFileSync(versionPath, "utf-8"), version!)); + } catch { + fail(`write: failed to update ${versionRel} (is it valid JSON?).`, 3); + } + process.stdout.write(JSON.stringify({ wrote: version, versionPath: versionRel, packageJson: true }) + "\n"); + return; + } + writeFileSync(versionPath, version + "\n"); if (existsSync(join(cwd, "package.json"))) { try { @@ -177,7 +210,17 @@ function cmdWrite(args: string[], cwd: string): void { function cmdRepair(args: string[], cwd: string): void { const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path")); - const current = readVersionFile(versionPath); + const versionRel = argVal(args, "--version-path") ?? "VERSION"; + // Nothing to repair when the version lives in a package.json (#2501): there + // is no second file to drift from, and classify never reports DRIFT_* for + // that shape. + if (isJsonVersionPath(versionRel)) { + process.stdout.write( + JSON.stringify({ repaired: null, reason: `${versionRel} is the single source of truth; no drift possible` }) + "\n", + ); + return; + } + const current = readVersionFile(versionPath, versionRel); if (!VERSION_RE.test(current)) { fail( `VERSION file contents (${current}) do not match MAJOR.MINOR.PATCH.MICRO. ` + diff --git a/lib/version-source.ts b/lib/version-source.ts new file mode 100644 index 000000000..2d61fa0f5 --- /dev/null +++ b/lib/version-source.ts @@ -0,0 +1,118 @@ +// version-source — where a repo's version lives, and how wide it is. +// +// gstack's native shape is a plain-text VERSION file at the repo root holding a +// 4-digit MAJOR.MINOR.PATCH.MICRO — and for gstack itself that file STAYS the +// source of truth (decision pinned in the v1.67 fix-wave plan: package.json is +// a translated mirror, never the authority). This module exists for the two +// real-world shapes that did not fit and both failed CLOSED in a way that +// silently disabled /ship's version tooling (#2501): +// +// 1. The version's home is a package.json — often not at the root (a monorepo +// whose frontend/package.json is the single source of truth because the +// build injects it). The --version-path / .gstack/version-path pin already +// let you point anywhere, but the readers treated the target as raw text, +// so a JSON file was whitespace-stripped into `{"name":"frontend",...` and +// every version read came back as the 0.0.0.0 fallback — including rival +// PRs' claims fetched through the GitHub Contents API, which were then +// dropped as "malformed". +// 2. The version is 3-digit semver. parseVersion() required exactly four +// components, so gstack-next-version exited 2 ("could not parse base +// version") on every invocation — and that CLI *is* the queue-collision +// check, so /ship fell through to its documented "offline" path of naive +// local arithmetic. Two branches cut from the same base then pick the same +// version, and git merges that without a conflict because both sides set +// one line to identical text. The duplicate slot ships silently. +// +// Both are handled here rather than in each CLI so the two agree by construction. +// +// Detection is by shape, not configuration: a version-path ending in .json is +// read as JSON (.version), anything else as trimmed text; a version string with +// three components stays three components through bumping and formatting. A +// repo with a root VERSION file and 4-digit versions sees no behaviour change. +// +// Re-derived from PR #2501 by @YiftahR. + +export type Version = [number, number, number, number]; +export type VersionWidth = 3 | 4; +export type Bump = "major" | "minor" | "patch" | "micro"; + +/** Parse 3- or 4-component versions. 3-digit pads to [a,b,c,0] so comparison stays uniform. */ +export function parseVersion(s: string): Version | null { + const m = s.trim().match(/^(\d+)\.(\d+)\.(\d+)(?:\.(\d+))?$/); + if (!m) return null; + return [Number(m[1]), Number(m[2]), Number(m[3]), Number(m[4] ?? 0)]; +} + +/** How many components the string actually had — what to format back out as. */ +export function versionWidth(s: string): VersionWidth { + return /^\d+\.\d+\.\d+\.\d+$/.test(s.trim()) ? 4 : 3; +} + +export function fmtVersion(v: Version, width: VersionWidth = 4): string { + return v.slice(0, width).join("."); +} + +export function cmpVersion(a: Version, b: Version): number { + for (let i = 0; i < 4; i++) { + if (a[i] !== b[i]) return a[i] - b[i]; + } + return 0; +} + +/** + * Bump one level. In a 3-digit repo there is no MICRO component to move, so + * `micro` is carried out as a PATCH: /ship auto-picks MICRO by default, and + * erroring there would make it unusable in every 3-digit repo — a silent no-op + * would be worse still, since the caller would then write back the version it + * started with and claim a taken slot. + */ +export function bumpVersion(v: Version, level: Bump, width: VersionWidth = 4): Version { + const effective: Bump = width === 3 && level === "micro" ? "patch" : level; + switch (effective) { + case "major": + return [v[0] + 1, 0, 0, 0]; + case "minor": + return [v[0], v[1] + 1, 0, 0]; + case "patch": + return [v[0], v[1], v[2] + 1, 0]; + case "micro": + return [v[0], v[1], v[2], v[3] + 1]; + } +} + +/** True when the effective bump differs from the one asked for (so callers can say so). */ +export function bumpWasCoerced(level: Bump, width: VersionWidth): boolean { + return width === 3 && level === "micro"; +} + +/** A version-path pointing at a .json is read as JSON, not as raw text. */ +export function isJsonVersionPath(versionPath: string): boolean { + return /\.json$/i.test(versionPath.trim()); +} + +/** + * Pull the version out of whatever the version-path resolves to. `text` is the + * file's contents from anywhere — local read, `git show`, or a base64-decoded + * API response — so every reader agrees on interpretation. Returns "" when + * there is no usable version, which callers map to their own fallback. + */ +export function extractVersion(text: string, versionPath: string): string { + if (!isJsonVersionPath(versionPath)) return text.replace(/[\r\n\s]/g, ""); + try { + const parsed = JSON.parse(text) as { version?: unknown }; + return typeof parsed?.version === "string" ? parsed.version.trim() : ""; + } catch { + return ""; + } +} + +/** + * Write a version back into a JSON file, preserving the rest of it. Deliberately + * key-order-preserving (JSON.parse/stringify keeps insertion order) and 2-space + * indented with a trailing newline, matching what package managers write. + */ +export function setVersionInJson(raw: string, version: string): string { + const parsed = JSON.parse(raw) as Record; + parsed.version = version; + return JSON.stringify(parsed, null, 2) + "\n"; +} diff --git a/test/gstack-next-version.test.ts b/test/gstack-next-version.test.ts index f72ec0149..81cd180d0 100644 --- a/test/gstack-next-version.test.ts +++ b/test/gstack-next-version.test.ts @@ -13,6 +13,8 @@ import { fmtVersion, bumpVersion, cmpVersion, + versionWidth, + extractVersion, pickNextSlot, markActiveSiblings, resolveVersionPath, @@ -29,8 +31,20 @@ describe("parseVersion", () => { expect(parseVersion(" 1.2.3.4 \n")).toEqual([1, 2, 3, 4]); }); + test("accepts 3-digit semver, padding the micro slot (#2501)", () => { + // 3-digit repos (a package.json holding plain semver) used to fail parsing + // outright, which exited this CLI 2 on EVERY run — and since this CLI is + // the queue-collision check, /ship then fell back to naive local + // arithmetic and duplicate version slots shipped silently. The pad keeps + // comparison uniform; versionWidth narrows output back. + expect(parseVersion("0.99.2")).toEqual([0, 99, 2, 0]); + expect(parseVersion("1.2.3")).toEqual([1, 2, 3, 0]); + expect(versionWidth("0.99.2")).toBe(3); + expect(versionWidth("1.6.3.0")).toBe(4); + }); + test("rejects malformed", () => { - expect(parseVersion("1.2.3")).toBeNull(); + expect(parseVersion("1.2")).toBeNull(); expect(parseVersion("1.2.3.4.5")).toBeNull(); expect(parseVersion("v1.2.3.4")).toBeNull(); expect(parseVersion("")).toBeNull(); @@ -39,6 +53,49 @@ describe("parseVersion", () => { }); }); +describe("3-digit repos keep their width (#2501)", () => { + test("formatting narrows to the repo's own width", () => { + expect(fmtVersion([0, 99, 3, 0], 3)).toBe("0.99.3"); + expect(fmtVersion([0, 99, 3, 0], 4)).toBe("0.99.3.0"); + expect(fmtVersion([0, 99, 3, 0])).toBe("0.99.3.0"); // default stays 4-digit + }); + + test("micro is carried out as patch when there is no micro component", () => { + // /ship auto-picks MICRO by default. Erroring would make it unusable in + // every 3-digit repo; a no-op would be worse — it would write back the + // version it started with and claim a slot already taken. + expect(bumpVersion([0, 99, 2, 0], "micro", 3)).toEqual([0, 99, 3, 0]); + expect(bumpVersion([0, 99, 2, 0], "patch", 3)).toEqual([0, 99, 3, 0]); + expect(bumpVersion([0, 99, 2, 3], "micro", 4)).toEqual([0, 99, 2, 4]); // 4-digit unchanged + }); + + test("slot picking stays inside the repo's width", () => { + const { version } = pickNextSlot([0, 99, 2, 0], [[0, 99, 5, 0]], "patch", 3); + expect(fmtVersion(version, 3)).toBe("0.99.6"); + }); +}); + +describe("extractVersion (#2501)", () => { + test("reads .version when the version-path is a package.json", () => { + const pkg = JSON.stringify({ name: "frontend", version: "0.99.2", private: true }); + expect(extractVersion(pkg, "frontend/package.json")).toBe("0.99.2"); + expect(extractVersion(pkg, "deep/nested/package.json")).toBe("0.99.2"); + }); + + test("reads raw text for a plain VERSION file", () => { + expect(extractVersion("1.6.3.0\n", "VERSION")).toBe("1.6.3.0"); + expect(extractVersion(" 1.6.3.0 ", "version/CURRENT")).toBe("1.6.3.0"); + }); + + test("a JSON path that isn't valid JSON yields empty, not garbage", () => { + // The old readers ran a package.json through a whitespace strip and handed + // the caller '{"name":"frontend",...' as if it were a version. Empty lets + // callers fall back loudly. + expect(extractVersion("{ not json", "package.json")).toBe(""); + expect(extractVersion(JSON.stringify({ name: "x" }), "package.json")).toBe(""); + }); +}); + describe("bumpVersion", () => { test("major zeros everything right", () => { expect(bumpVersion([1, 6, 3, 0], "major")).toEqual([2, 0, 0, 0]); diff --git a/test/gstack-version-bump.test.ts b/test/gstack-version-bump.test.ts index ffcecd1a7..41eae0468 100644 --- a/test/gstack-version-bump.test.ts +++ b/test/gstack-version-bump.test.ts @@ -39,10 +39,18 @@ describe('VERSION_RE', () => { test('accepts 4-digit semver', () => { expect(VERSION_RE.test('1.2.3.4')).toBe(true); }); - test('rejects 3-digit and garbage', () => { - expect(VERSION_RE.test('1.2.3')).toBe(false); + test('accepts 3-digit semver too (#2501)', () => { + // A repo whose pinned version source is a package.json holds plain + // 3-digit semver. Rejecting it meant /ship could not write a version in + // such a repo at all. + expect(VERSION_RE.test('1.2.3')).toBe(true); + expect(VERSION_RE.test('0.99.2')).toBe(true); + }); + test('rejects garbage', () => { + expect(VERSION_RE.test('1.2')).toBe(false); expect(VERSION_RE.test('v1.2.3.4')).toBe(false); expect(VERSION_RE.test('1.2.3.4-rc')).toBe(false); + expect(VERSION_RE.test('1.2.3.4.5')).toBe(false); }); }); @@ -63,7 +71,7 @@ describe('write (FRESH bump)', () => { test('rejects a malformed version with exit 2', () => { let code = 0; - try { execFileSync('bun', [BIN, 'write', '--version', '1.2.3'], { cwd: dir, stdio: 'pipe' }); } + try { execFileSync('bun', [BIN, 'write', '--version', '1.2.3.4.5'], { cwd: dir, stdio: 'pipe' }); } catch (e: any) { code = e.status; } expect(code).toBe(2); }); @@ -131,3 +139,74 @@ describe('classify (idempotency over a real git base)', () => { expect(parsed.currentVersion).toBe('1.1.0.0'); }); }); + +/** + * A repo whose single source of truth is a package.json at a non-root path, + * holding plain 3-digit semver — the shape gstack's native VERSION-file + * assumption failed closed on (#2501). Before this, classify reported + * {state: FRESH, baseVersion: "0.0.0.0", pkgExists: false} no matter what the + * repo's real version was: it looked for a root VERSION file and a root + * package.json, found neither, and reported a pristine repo at version zero. + * + * These cases pass --version-path explicitly; the .gstack/version-path pin + * flows through the same reader once classify/write/repair resolve the pin's + * repo-relative form (#2462, covered in its own suite below the pin fix). + */ +describe('package.json as the version source (monorepo, 3-digit, #2501)', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-pkgsrc-')); + afterAll(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } }); + + const pkgRel = 'frontend/package.json'; + const pkgAbs = path.join(dir, pkgRel); + fs.mkdirSync(path.join(dir, 'frontend'), { recursive: true }); + fs.writeFileSync(pkgAbs, JSON.stringify({ name: 'frontend', version: '0.99.2', private: true, scripts: { dev: 'next dev' } }, null, 2) + '\n'); + + execFileSync('git', ['init', '-q', '-b', 'main'], { cwd: dir }); + execFileSync('git', ['config', 'user.email', 't@e.com'], { cwd: dir }); + execFileSync('git', ['config', 'user.name', 't'], { cwd: dir }); + execFileSync('git', ['add', '-A'], { cwd: dir }); + execFileSync('git', ['commit', '-qm', 'v0.99.2 base'], { cwd: dir }); + const head = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir }).toString().trim(); + fs.mkdirSync(path.join(dir, '.git', 'refs', 'remotes', 'origin'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.git', 'refs', 'remotes', 'origin', 'main'), head + '\n'); + + test('classify reads the real version from the package.json version-path', () => { + const out = execFileSync('bun', [BIN, 'classify', '--base', 'main', '--version-path', pkgRel], { cwd: dir }).toString(); + const parsed = JSON.parse(out); + expect(parsed.state).toBe('FRESH'); + expect(parsed.baseVersion).toBe('0.99.2'); // was "0.0.0.0" + expect(parsed.currentVersion).toBe('0.99.2'); // was "0.0.0.0" + expect(parsed.pkgExists).toBe(true); // was false + }); + + test('write updates the package.json in place and creates no VERSION file', () => { + const out = execFileSync('bun', [BIN, 'write', '--version', '0.99.3', '--version-path', pkgRel], { cwd: dir }).toString(); + expect(JSON.parse(out)).toEqual({ wrote: '0.99.3', versionPath: pkgRel, packageJson: true }); + const pkg = JSON.parse(fs.readFileSync(pkgAbs, 'utf-8')); + expect(pkg.version).toBe('0.99.3'); + expect(pkg.scripts).toEqual({ dev: 'next dev' }); // rest of the file untouched + expect(pkg.name).toBe('frontend'); + expect(fs.existsSync(path.join(dir, 'VERSION'))).toBe(false); + }); + + test('classify reports ALREADY_BUMPED after that write, not a drift state', () => { + const out = execFileSync('bun', [BIN, 'classify', '--base', 'main', '--version-path', pkgRel], { cwd: dir }).toString(); + const parsed = JSON.parse(out); + expect(parsed.state).toBe('ALREADY_BUMPED'); + expect(parsed.baseVersion).toBe('0.99.2'); + expect(parsed.currentVersion).toBe('0.99.3'); + }); + + test('repair is a no-op: there is no second file to drift from', () => { + const out = execFileSync('bun', [BIN, 'repair', '--version-path', pkgRel], { cwd: dir }).toString(); + expect(JSON.parse(out).repaired).toBeNull(); + }); + + test('write refuses a version-path that does not exist', () => { + let code = 0; + try { + execFileSync('bun', [BIN, 'write', '--version', '1.0.0', '--version-path', 'nope/package.json'], { cwd: dir, stdio: 'pipe' }); + } catch (e: any) { code = e.status; } + expect(code).toBe(2); + }); +});