mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 22:48:57 +02:00
fix(next-version): git fallback queries the live remote, never mutates, and keeps 3-digit width
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
7f749f94fe
commit
bdc0b11664
+95
-32
@@ -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/<base> 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}'`);
|
||||
|
||||
@@ -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/<base> 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/<base> 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.
|
||||
|
||||
Reference in New Issue
Block a user