From 51bdad5e207fdeef57d99bbe293b458f21151508 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 29 Aug 2026 05:42:57 +0000 Subject: [PATCH] fix(next-version): a configured origin advertising zero heads is never trusted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The originConfigured guard covered only the no-origin laundering case. With origin configured (the normal Conductor worktree state), the laundering shim makes a failed ls-remote exit 0 with empty stdout — read as 'the queue is empty', the exact duplicate-allocation bug (#2545) one layer up. A reachable remote always advertises at least its default branch, so an exit-0 zero-head probe now falls back to local refs/remotes/origin with a laundering-specific warning. Regression test shims git for both configurations. Co-Authored-By: Claude Fable 5 --- bin/gstack-next-version | 28 +++++++++++++---- test/gstack-next-version.test.ts | 53 ++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/bin/gstack-next-version b/bin/gstack-next-version index b2b85abcd..4b8756329 100755 --- a/bin/gstack-next-version +++ b/bin/gstack-next-version @@ -520,14 +520,27 @@ function fetchGitClaimed( : null; const lsOk = lsRemote !== null && lsRemote.status === 0 && !lsRemote.error; + // A configured origin that "successfully" advertises ZERO heads is a + // contradiction — every reachable remote advertises at least its default + // branch. It means the exit code was laundered (observed: the Conductor + // /conductor/bin/git shim exits 0 on every network failure), so an exit-0 + // empty probe must NOT be believed as "the queue is empty" — that is the + // duplicate-allocation bug (#2545) again, one layer up from the + // originConfigured guard. Parse first; trust only a non-empty head list. + const parsedHeads = lsOk + ? (lsRemote.stdout ?? "") + .split("\n") + .map((line) => line.trim().match(/^([0-9a-f]{40,64})\trefs\/heads\/(.+)$/)) + .filter((m): m is RegExpMatchArray => m !== null) + : []; + const lsTrusted = lsOk && parsedHeads.length > 0; + // 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 (lsTrusted) { + for (const m of parsedHeads) { if (m[2] === baseShort) continue; candidates.push({ branch: m[2], sha: m[1] }); } @@ -536,8 +549,11 @@ function fetchGitClaimed( // 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)", + lsOk + ? "git ls-remote origin exited 0 but advertised zero heads (exit-code-laundering shim suspected); " + + "using stale local refs/remotes/origin — branches deleted on the remote may still be counted as claims (run `git fetch --prune origin` to refresh)" + : "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", diff --git a/test/gstack-next-version.test.ts b/test/gstack-next-version.test.ts index 4783d7f7f..319047287 100644 --- a/test/gstack-next-version.test.ts +++ b/test/gstack-next-version.test.ts @@ -5,7 +5,7 @@ import { test, expect, describe } from "bun:test"; import { execFileSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -992,7 +992,6 @@ describe("fetchGitClaimed — laundered ls-remote (exit 0, empty output) is neve // and unguarded paths are indistinguishable (ls-remote genuinely fails), so // only a laundering shim can pin the guard against reverts. test("no origin + shim that lies: claims still come from local refs, with the staleness warning", () => { - const { chmodSync } = require("node:fs") as typeof import("node:fs"); const dir = mkdtempSync(join(tmpdir(), "nextver-launder-")); const stubDir = join(dir, "stub-bin"); mkdirSync(stubDir); @@ -1035,4 +1034,54 @@ describe("fetchGitClaimed — laundered ls-remote (exit 0, empty output) is neve rmSync(dir, { recursive: true, force: true }); } }); + + test("origin CONFIGURED + shim that lies: a zero-head exit-0 probe is distrusted, not read as an empty queue", () => { + // The normal Conductor worktree state: origin IS configured, but the + // laundering shim makes a failed ls-remote exit 0 with empty stdout. A + // configured origin that advertises zero heads is contradictory (every + // reachable remote advertises at least its default branch), so the + // allocator must fall back to local refs with the laundering warning. + const dir = mkdtempSync(join(tmpdir(), "nextver-launder-cfg-")); + const stubDir = join(dir, "stub-bin"); + mkdirSync(stubDir); + const realGit = execFileSync("sh", ["-c", "command -v git"]).toString().trim(); + writeFileSync( + join(stubDir, "git"), + `#!/bin/sh\nif [ "$1" = "ls-remote" ]; then exit 0; fi\nexec ${realGit} "$@"\n`, + ); + chmodSync(join(stubDir, "git"), 0o755); + + const git = (cwd: string, ...args: string[]) => + Bun.spawnSync(["git", "-c", "user.email=t@t", "-c", "user.name=t", ...args], { cwd }); + + const cwd = process.cwd(); + const oldPath = process.env.PATH; + 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", "main"); + git(dir, "update-ref", "refs/remotes/origin/sibling", sibSha); + // Configured origin (unreachable path — the shim intercepts before git tries it). + git(dir, "remote", "add", "origin", "/nonexistent/laundered-origin.git"); + + process.chdir(dir); + process.env.PATH = `${stubDir}:${oldPath}`; + const warnings: string[] = []; + const claims = fetchGitClaimed("main", "VERSION", warnings); + const versions = claims.map((c) => c.version); + expect(versions).toContain("0.1.67.0"); + expect(warnings.join(" ")).toContain("advertised zero heads"); + } finally { + process.env.PATH = oldPath; + process.chdir(cwd); + rmSync(dir, { recursive: true, force: true }); + } + }); });