From bdc0b1166467612ea80c52a3b3125923ba678441 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 10:52:23 -0700 Subject: [PATCH] fix(next-version): git fallback queries the live remote, never mutates, and keeps 3-digit width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The degraded path counted every remote-tracking ref on every remote — stale experiment branches and second remotes inflated version allocation, and a failed base read flipped 3-digit repos to 4-digit slots. Now: ls-remote --heads origin first (GIT_TERMINAL_PROMPT=0, 5s timeout, zero local ref mutation); on failure, local refs/remotes/origin ONLY with an explicit stale-refs warning; a failed base read zeroes at the LOCAL version file's width so a 3-digit repo allocates 0.0.1, not 0.0.1.0. Co-Authored-By: Claude Fable 5 --- bin/gstack-next-version | 127 ++++++++++++++++++------- test/gstack-next-version.test.ts | 157 +++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+), 32 deletions(-) diff --git a/bin/gstack-next-version b/bin/gstack-next-version index e19781594..3820edb9c 100755 --- a/bin/gstack-next-version +++ b/bin/gstack-next-version @@ -147,19 +147,36 @@ function detectHost(): "github" | "gitlab" | "unknown" { return "unknown"; } -function readBaseVersion(base: string, versionPath: string, warnings: string[]): string { +// When the base-version read fails we assume a zero base — but a literal +// "0.0.0.0" is 4-digit, which flips versionWidth() to 4 and hands a 3-digit +// repo a 4-digit slot (the exact width class of bug #2501 fixed in parsing). +// The LOCAL version file at versionPath knows the repo's own width; shape the +// zero from it. No local file either → keep the 4-digit default. +function zeroBaseAtLocalWidth(versionPath: string, repoRoot: string): string { + try { + const local = extractVersion(readFileSync(join(repoRoot, versionPath), "utf8"), versionPath); + if (local && parseVersion(local) && versionWidth(local) === 3) return "0.0.0"; + } catch { + // unreadable/absent local version file — 4-digit default below + } + return "0.0.0.0"; +} + +function readBaseVersion(base: string, versionPath: string, repoRoot: string, warnings: string[]): string { // git fetch is best-effort; we tolerate failure and fall back to whatever // origin/ currently points at. runCommand("git", ["fetch", "origin", base, "--quiet"], 10000); const r = runCommand("git", ["show", `origin/${base}:${versionPath}`]); if (!r.ok) { - warnings.push(`could not read ${versionPath} at origin/${base}; assuming 0.0.0.0`); - return "0.0.0.0"; + const assumed = zeroBaseAtLocalWidth(versionPath, repoRoot); + warnings.push(`could not read ${versionPath} at origin/${base}; assuming ${assumed}`); + return assumed; } 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"; + const assumed = zeroBaseAtLocalWidth(versionPath, repoRoot); + warnings.push(`${versionPath} at origin/${base} has no readable version; assuming ${assumed}`); + return assumed; } return v; } @@ -460,40 +477,85 @@ function autoDetectExcludePR(): number | null { // v0.1.57.0. Auditing that repo's history found FOUR such pairs going back // three weeks, so the silent fallback had been mis-allocating for a while. // -// Git already knows what the API was asked for. Remote-tracking refs carry -// each branch's VERSION file, and the base's own history records every version -// already shipped. Neither needs a token, a network round-trip, or a working -// `gh`. So "offline" degrades the QUEUE VIEW (no PR numbers, no draft status) -// without degrading the ALLOCATION. +// Git already knows what the API was asked for. `git ls-remote --heads origin` +// returns the remote's LIVE branch list with zero local mutation (no fetch, no +// ref updates), each branch's VERSION file is readable from the local object +// store, and the base's own history records every version already shipped. +// None of it needs a token or a working `gh`. So "offline" degrades the QUEUE +// VIEW (no PR numbers, no draft status) without degrading the ALLOCATION. function fetchGitClaimed( base: string, versionPath: string, warnings: string[], ): ClaimedPR[] { const claims: ClaimedPR[] = []; + const baseShort = base.replace(/^origin\//, ""); - // 1. Every remote-tracking branch's VERSION file. These are the open PRs' - // branches, whether or not the API can be reached to enumerate them. - // Read through extractVersion so a JSON version-path (#2501) resolves on - // remote refs too, and the branch's own width is preserved in the claim. - const refs = runCommand("git", [ - "for-each-ref", - "--format=%(refname:short)", - "refs/remotes", - ]); - if (refs.ok) { - const baseShort = base.replace(/^origin\//, ""); - for (const ref of refs.stdout.split("\n").map((r) => r.trim()).filter(Boolean)) { - if (ref.endsWith("/HEAD")) continue; - if (ref === base || ref.replace(/^origin\//, "") === baseShort) continue; - const show = runCommand("git", ["show", `${ref}:${versionPath}`]); - if (!show.ok) continue; - const raw = extractVersion(show.stdout, versionPath); - if (!raw || !parseVersion(raw)) continue; - claims.push({ pr: 0, branch: ref, version: raw }); + // 1. The version-claim branches. FIRST try `git ls-remote --heads origin`: + // fresh remote data, zero local mutation. This scopes claims to branches + // that actually EXIST on origin right now — the previous implementation + // counted every remote-tracking ref on EVERY remote, so stale local refs + // (deleted PR branches, an unrelated `upstream` remote) inflated the + // claim set and pushed the allocation further than the real queue. + // GIT_TERMINAL_PROMPT=0 + a 5s timeout keep a dead/credential-prompting + // remote from hanging the allocator. + const lsRemote = spawnSync("git", ["ls-remote", "--heads", "origin"], { + encoding: "utf8", + timeout: 5000, + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + }); + const lsOk = lsRemote.status === 0 && !lsRemote.error; + + // Each candidate carries the LIVE tip sha when it came from ls-remote, so + // the VERSION read prefers the fresh commit (present locally after any + // prior fetch/clone) and only falls back to the local remote-tracking ref. + const candidates: { branch: string; sha?: string }[] = []; + if (lsOk) { + for (const line of (lsRemote.stdout ?? "").split("\n")) { + const m = line.trim().match(/^([0-9a-f]{40,64})\trefs\/heads\/(.+)$/); + if (!m) continue; + if (m[2] === baseShort) continue; + candidates.push({ branch: m[2], sha: m[1] }); } } else { - warnings.push("git for-each-ref failed; branch claims unavailable"); + // Degraded twice over: no host API AND no reachable remote. Fall back to + // the LOCAL refs/remotes/origin snapshot ONLY (never other remotes — an + // `upstream` remote's branches are not claims against OUR queue). + warnings.push( + "git ls-remote origin failed; using stale local refs/remotes/origin — " + + "branches deleted on the remote may still be counted as claims (run `git fetch --prune origin` to refresh)", + ); + const refs = runCommand("git", [ + "for-each-ref", + "--format=%(refname:short)", + "refs/remotes/origin", + ]); + if (refs.ok) { + for (const ref of refs.stdout.split("\n").map((r) => r.trim()).filter(Boolean)) { + if (ref.endsWith("/HEAD")) continue; + const branch = ref.replace(/^origin\//, ""); + if (branch === baseShort) continue; + candidates.push({ branch }); + } + } else { + warnings.push("git for-each-ref failed; branch claims unavailable"); + } + } + + // Read each candidate's VERSION through extractVersion so a JSON + // version-path (#2501) resolves on remote refs too, and the branch's own + // width is preserved in the claim. + for (const { branch, sha } of candidates) { + let show = sha ? runCommand("git", ["show", `${sha}:${versionPath}`]) : { ok: false, stdout: "", stderr: "" }; + if (!show.ok) { + // Live tip not fetched yet (or no sha in the fallback path): best-effort + // read from the local remote-tracking ref. + show = runCommand("git", ["show", `refs/remotes/origin/${branch}:${versionPath}`]); + } + if (!show.ok) continue; + const raw = extractVersion(show.stdout, versionPath); + if (!raw || !parseVersion(raw)) continue; + claims.push({ pr: 0, branch: `origin/${branch}`, version: raw }); } // 2. Versions already shipped, read from the base's commit subjects. Catches @@ -534,8 +596,9 @@ async function main() { } const warnings: string[] = []; const host = detectHost(); - const versionPath = resolveVersionPath(args.versionPath, repoToplevel()); - const baseVersion = args.current || readBaseVersion(args.base, versionPath, warnings); + const repoRoot = repoToplevel(); + const versionPath = resolveVersionPath(args.versionPath, repoRoot); + const baseVersion = args.current || readBaseVersion(args.base, versionPath, repoRoot, warnings); const baseParsed = parseVersion(baseVersion); if (!baseParsed) { console.error(`Error: could not parse base version '${baseVersion}'`); diff --git a/test/gstack-next-version.test.ts b/test/gstack-next-version.test.ts index ec3c34b2b..8cae867f9 100644 --- a/test/gstack-next-version.test.ts +++ b/test/gstack-next-version.test.ts @@ -542,6 +542,163 @@ describe("fetchGitClaimed (offline allocation — the anti-duplicate fallback, # }); }); +describe("fetchGitClaimed — non-mutating live remote query (ls-remote first)", () => { + // The degraded git-fallback used to count EVERY remote-tracking ref on EVERY + // remote: branches deleted on origin (stale local refs) and an unrelated + // `upstream` remote's branches all inflated the claim set, pushing the + // allocation past the real queue. `git ls-remote --heads origin` returns the + // remote's LIVE branch list with zero local mutation — a path/file remote + // answers it offline, which is exactly what these fixtures use. + function git(cwd: string, ...args: string[]) { + return Bun.spawnSync(["git", "-c", "user.email=t@t", "-c", "user.name=t", ...args], { cwd }); + } + + // Local origin with: main (0.1.66.0), sibling (0.1.67.0, live claim), and + // dead (0.1.98.0) — deleted on origin AFTER the clone, so the clone keeps a + // stale refs/remotes/origin/dead. Plus a second remote's stale claim ref. + function liveFixture(): { root: string; clone: string } { + const root = mkdtempSync(join(tmpdir(), "nextver-lsremote-")); + const origin = join(root, "origin"); + mkdirSync(origin); + git(origin, "init", "-q", "-b", "main"); + writeFileSync(join(origin, "VERSION"), "0.1.66.0\n"); + git(origin, "add", "-A"); + git(origin, "commit", "-qm", "v0.1.66.0 chore: base"); + git(origin, "checkout", "-q", "-b", "sibling"); + writeFileSync(join(origin, "VERSION"), "0.1.67.0\n"); + git(origin, "add", "-A"); + git(origin, "commit", "-qm", "v0.1.67.0 feat: sibling claimed this"); + git(origin, "checkout", "-q", "-b", "dead"); + writeFileSync(join(origin, "VERSION"), "0.1.98.0\n"); + git(origin, "add", "-A"); + git(origin, "commit", "-qm", "v0.1.98.0 feat: deleted later"); + git(origin, "checkout", "-q", "main"); + const clone = join(root, "clone"); + git(root, "clone", "-q", origin, clone); + // Deleted on the REMOTE after the clone — the stale local ref survives. + git(origin, "branch", "-qD", "dead"); + // A second remote carrying a stale claim branch: must never be counted. + git(clone, "checkout", "-q", "-b", "tmp-upstream"); + writeFileSync(join(clone, "VERSION"), "0.1.99.0\n"); + git(clone, "add", "-A"); + git(clone, "commit", "-qm", "v0.1.99.0 upstream stale claim"); + const upSha = new TextDecoder().decode(git(clone, "rev-parse", "HEAD").stdout).trim(); + git(clone, "checkout", "-q", "main"); + git(clone, "branch", "-qD", "tmp-upstream"); + git(clone, "update-ref", "refs/remotes/upstream/stale", upSha); + return { root, clone }; + } + + test("live path: only branches that exist on origin RIGHT NOW are claims", () => { + const { root, clone } = liveFixture(); + const cwd = process.cwd(); + try { + process.chdir(clone); + const warnings: string[] = []; + const claims = fetchGitClaimed("main", "VERSION", warnings); + const versions = claims.map((c) => c.version); + expect(versions).toContain("0.1.67.0"); // live sibling claim + expect(versions).not.toContain("0.1.98.0"); // deleted on origin — stale local ref ignored + expect(versions).not.toContain("0.1.99.0"); // second remote's refs are not our queue + // The live path emits no staleness warning. + expect(warnings.join(" ")).not.toContain("ls-remote"); + } finally { + process.chdir(cwd); + rmSync(root, { recursive: true, force: true }); + } + }); + + test("zero local mutation: the stale remote-tracking ref survives the query", () => { + // ls-remote reads the remote without fetch/prune — an allocator run must + // never rewrite local refs as a side effect. + const { root, clone } = liveFixture(); + const cwd = process.cwd(); + try { + process.chdir(clone); + fetchGitClaimed("main", "VERSION", []); + const ref = git(clone, "rev-parse", "--verify", "-q", "refs/remotes/origin/dead"); + expect(ref.exitCode).toBe(0); + } finally { + process.chdir(cwd); + rmSync(root, { recursive: true, force: true }); + } + }); + + test("fallback: ls-remote failure uses LOCAL refs/remotes/origin only, with a staleness warning", () => { + // No origin remote configured at all — ls-remote must fail, and the + // fallback must scan refs/remotes/origin ONLY (never other remotes). + const dir = mkdtempSync(join(tmpdir(), "nextver-lsfallback-")); + const cwd = process.cwd(); + try { + git(dir, "init", "-q", "-b", "main"); + writeFileSync(join(dir, "VERSION"), "0.1.66.0\n"); + git(dir, "add", "-A"); + git(dir, "commit", "-qm", "v0.1.66.0 chore: base"); + git(dir, "checkout", "-q", "-b", "sibling"); + writeFileSync(join(dir, "VERSION"), "0.1.67.0\n"); + git(dir, "add", "-A"); + git(dir, "commit", "-qm", "v0.1.67.0 feat: sibling claimed this"); + const sibSha = new TextDecoder().decode(git(dir, "rev-parse", "HEAD").stdout).trim(); + git(dir, "checkout", "-q", "-b", "stale2"); + writeFileSync(join(dir, "VERSION"), "0.1.99.0\n"); + git(dir, "add", "-A"); + git(dir, "commit", "-qm", "v0.1.99.0 upstream stale claim"); + const upSha = new TextDecoder().decode(git(dir, "rev-parse", "HEAD").stdout).trim(); + git(dir, "checkout", "-q", "main"); + git(dir, "update-ref", "refs/remotes/origin/sibling", sibSha); + git(dir, "update-ref", "refs/remotes/upstream/stale", upSha); + + process.chdir(dir); + const warnings: string[] = []; + const claims = fetchGitClaimed("main", "VERSION", warnings); + const versions = claims.map((c) => c.version); + expect(versions).toContain("0.1.67.0"); // origin's local snapshot still counts + expect(versions).not.toContain("0.1.99.0"); // upstream remote is ignored + expect(warnings.join(" ")).toContain("stale local refs/remotes/origin"); + } finally { + process.chdir(cwd); + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe("width pinned on failed base read (3-digit repos)", () => { + // readBaseVersion used to return a literal "0.0.0.0" when origin/ was + // unreadable — a 4-digit string, which flipped versionWidth() to 4 and + // handed a 3-digit repo a 4-digit slot its tooling can't read back (#2501's + // width class, resurfacing through the failure path). The zero base is now + // shaped by the LOCAL version file's width. + const SCRIPT = join(import.meta.dir, "..", "bin", "gstack-next-version"); + + test("a 3-digit repo keeps 3-digit allocation when origin/ is unreadable", () => { + const dir = mkdtempSync(join(tmpdir(), "nextver-width3-")); + const stubDir = mkdtempSync(join(tmpdir(), "nextver-width3-stub-")); + try { + // gh/glab stubs fail → host unknown → git fallback; no origin remote → + // the base read fails too, which is the path under test. + writeFileSync(join(stubDir, "gh"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + writeFileSync(join(stubDir, "glab"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + Bun.spawnSync(["git", "init", "-q", "-b", "main"], { cwd: dir }); + writeFileSync(join(dir, "VERSION"), "0.99.2\n"); + Bun.spawnSync(["git", "-c", "user.email=t@t", "-c", "user.name=t", "add", "-A"], { cwd: dir }); + Bun.spawnSync(["git", "-c", "user.email=t@t", "-c", "user.name=t", "commit", "-qm", "init"], { cwd: dir }); + + const proc = Bun.spawnSync( + ["bun", "run", SCRIPT, "--base", "main", "--bump", "patch", "--workspace-root", "null"], + { cwd: dir, env: { ...process.env, PATH: `${stubDir}:${process.env.PATH}` } }, + ); + const out = JSON.parse(new TextDecoder().decode(proc.stdout)); + // Zero base at the repo's OWN width — never "0.0.0.0" in a 3-digit repo. + expect(out.base_version).toBe("0.0.0"); + expect(out.version).toBe("0.0.1"); // 3-digit allocation, not 0.0.1.0 + expect(out.warnings.join(" ")).not.toContain("0.0.0.0"); + } finally { + rmSync(dir, { recursive: true, force: true }); + rmSync(stubDir, { recursive: true, force: true }); + } + }, 30_000); +}); + describe("integration (smoke)", () => { // Bumps timeout to 30s — the test spawns a real `bun run` subprocess that // does a `gh pr list` against the live GitHub API to inspect claimed slots.