mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-16 18:05:31 +02:00
fix(next-version): a configured origin advertising zero heads is never trusted
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
7e1061c6dd
commit
51bdad5e20
+22
-6
@@ -520,14 +520,27 @@ function fetchGitClaimed(
|
|||||||
: null;
|
: null;
|
||||||
const lsOk = lsRemote !== null && lsRemote.status === 0 && !lsRemote.error;
|
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
|
// 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
|
// the VERSION read prefers the fresh commit (present locally after any
|
||||||
// prior fetch/clone) and only falls back to the local remote-tracking ref.
|
// prior fetch/clone) and only falls back to the local remote-tracking ref.
|
||||||
const candidates: { branch: string; sha?: string }[] = [];
|
const candidates: { branch: string; sha?: string }[] = [];
|
||||||
if (lsOk) {
|
if (lsTrusted) {
|
||||||
for (const line of (lsRemote.stdout ?? "").split("\n")) {
|
for (const m of parsedHeads) {
|
||||||
const m = line.trim().match(/^([0-9a-f]{40,64})\trefs\/heads\/(.+)$/);
|
|
||||||
if (!m) continue;
|
|
||||||
if (m[2] === baseShort) continue;
|
if (m[2] === baseShort) continue;
|
||||||
candidates.push({ branch: m[2], sha: m[1] });
|
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
|
// the LOCAL refs/remotes/origin snapshot ONLY (never other remotes — an
|
||||||
// `upstream` remote's branches are not claims against OUR queue).
|
// `upstream` remote's branches are not claims against OUR queue).
|
||||||
warnings.push(
|
warnings.push(
|
||||||
"git ls-remote origin failed; using stale local refs/remotes/origin — " +
|
lsOk
|
||||||
"branches deleted on the remote may still be counted as claims (run `git fetch --prune origin` to refresh)",
|
? "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", [
|
const refs = runCommand("git", [
|
||||||
"for-each-ref",
|
"for-each-ref",
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
|
|
||||||
import { test, expect, describe } from "bun:test";
|
import { test, expect, describe } from "bun:test";
|
||||||
import { execFileSync } from "node:child_process";
|
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 { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
import {
|
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
|
// and unguarded paths are indistinguishable (ls-remote genuinely fails), so
|
||||||
// only a laundering shim can pin the guard against reverts.
|
// 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", () => {
|
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 dir = mkdtempSync(join(tmpdir(), "nextver-launder-"));
|
||||||
const stubDir = join(dir, "stub-bin");
|
const stubDir = join(dir, "stub-bin");
|
||||||
mkdirSync(stubDir);
|
mkdirSync(stubDir);
|
||||||
@@ -1035,4 +1034,54 @@ describe("fetchGitClaimed — laundered ls-remote (exit 0, empty output) is neve
|
|||||||
rmSync(dir, { recursive: true, force: true });
|
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 });
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user