Skip to content

Commit 0af17fb

Browse files
committed
fix(windows): route every gh and release command through the shared launcher
The previous attempt fixed runQuiet and left the rest on Bun.$, which looked equivalent and is not. The built-in shell resolves PATH itself: the release tests put shims on PATH as an extension-less launcher, a .js, and a .cmd, and Windows can execute none of the first, does not retry as .cmd, and so reached the real git. The branch guard then saw dev instead of the faked main and aborted before logging a single call -- four release-helper tests failing on windows-latest only, with an empty call log, while macOS stayed green. The same defect was costing two sidebar tests five seconds each. A bare gh spawn against a .cmd shim does not fail fast; it hangs until the timeout. That is the star-state poll and the startup star prompt, so both move over too. Three source-assertion tests pinned the old spellings. Their invariants are about which resolver runs and which arguments are passed, not about a command being a shell string, so they now assert the argv form and the launcher. Rewriting the assertion rather than the code would have re-broken Windows. A source assertion is the honest check here: no host-platform run can observe which resolver executed, which is exactly how the suite stayed green on macOS through three red CI rounds.
1 parent d12fb7d commit 0af17fb

6 files changed

Lines changed: 130 additions & 26 deletions

File tree

scripts/release.ts

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
*
1717
* Requires: gh CLI (authed). Publishing is tokenless via Trusted Publishing (OIDC) — no NPM_TOKEN.
1818
*/
19-
import { $ } from "bun";
2019
import { commandInvocation } from "../src/lib/win-exec";
2120

2221
const args = process.argv.slice(2);
@@ -64,6 +63,44 @@ async function runQuiet(command: string[]): Promise<CommandResult> {
6463
return { exitCode, stdout: stdout.trim(), stderr: stderr.trim() };
6564
}
6665

66+
/**
67+
* Capture stdout from a command, failing loudly on a non-zero exit.
68+
*
69+
* Everything in this script goes through `commandInvocation` rather than
70+
* `Bun.$`. The shell form looked equivalent but is not on Windows: a test that
71+
* puts shims on PATH writes an extension-less launcher (shebang), a `.js`, and a
72+
* `.cmd`. Unix honours the shebang launcher; Windows cannot execute it and the
73+
* built-in shell does not retry as `.cmd`, so `$` walked past the shim straight
74+
* to the real `git` — the branch guard then saw `dev` instead of the faked
75+
* `main` and aborted before a single command was logged. That is what made four
76+
* release-helper tests fail on windows-latest only, with an empty call log.
77+
*/
78+
async function capture(command: string[]): Promise<string> {
79+
const result = await runQuiet(command);
80+
if (result.exitCode !== 0) {
81+
console.error(`✗ ${command.join(" ")} failed (exit ${result.exitCode})`);
82+
if (result.stderr) console.error(result.stderr);
83+
process.exit(1);
84+
}
85+
return result.stdout;
86+
}
87+
88+
/** Run a command with its output attached to this terminal; abort on failure. */
89+
async function runLoud(command: string[]): Promise<void> {
90+
const [bin, ...rest] = command;
91+
const invocation = commandInvocation(bin ?? "", rest);
92+
const proc = Bun.spawn([invocation.file, ...invocation.args], {
93+
stdout: "inherit",
94+
stderr: "inherit",
95+
...(invocation.options.windowsVerbatimArguments ? { windowsVerbatimArguments: true } : {}),
96+
});
97+
const exitCode = await proc.exited;
98+
if (exitCode !== 0) {
99+
console.error(`✗ ${command.join(" ")} failed (exit ${exitCode})`);
100+
process.exit(1);
101+
}
102+
}
103+
67104
async function readPackageName(): Promise<string> {
68105
try {
69106
const pkg = JSON.parse(await Bun.file("package.json").text()) as { name?: unknown };
@@ -139,21 +176,21 @@ async function assertUnusedReleaseVersion(packageName: string, version: string):
139176
}
140177

141178
async function watchLatest(): Promise<void> {
142-
const id = (await $`gh run list --workflow release.yml --limit 1 --json databaseId -q '.[0].databaseId'`.text()).trim();
179+
const id = await capture(["gh", "run", "list", "--workflow", "release.yml", "--limit", "1", "--json", "databaseId", "-q", ".[0].databaseId"]);
143180
if (!id) { console.error("No Release runs found yet."); process.exit(1); }
144181
await watchRun(id);
145182
}
146183

147184
async function watchRun(id: string | number): Promise<void> {
148185
console.log(`→ watching Release run ${id}`);
149-
await $`gh run watch ${String(id)} --exit-status --interval 10`;
186+
await runLoud(["gh", "run", "watch", String(id), "--exit-status", "--interval", "10"]);
150187
}
151188

152189
async function waitForReleaseWorkflowRun(sha: string, branch: string, createdAfterIso: string): Promise<GhRun> {
153190
const deadline = Date.now() + 2 * 60 * 1000;
154191
let attempt = 1;
155192
while (Date.now() < deadline) {
156-
const raw = await $`gh run list --workflow release.yml --branch ${branch} --commit ${sha} --limit 20 --json createdAt,databaseId,headSha,status,url`.text();
193+
const raw = await capture(["gh", "run", "list", "--workflow", "release.yml", "--branch", branch, "--commit", sha, "--limit", "20", "--json", "createdAt,databaseId,headSha,status,url"]);
157194
const runs = (JSON.parse(raw) as GhRun[])
158195
.filter(run => run.headSha === sha)
159196
.filter(run => !run.createdAt || run.createdAt >= createdAfterIso)
@@ -172,7 +209,7 @@ async function waitForReleaseWorkflowRun(sha: string, branch: string, createdAft
172209
}
173210

174211
async function listCiRuns(sha: string, workflow: string = CI_WORKFLOW): Promise<GhRun[]> {
175-
const raw = await $`gh run list --workflow ${workflow} --commit ${sha} --limit 20 --json conclusion,databaseId,headSha,status,url`.text();
212+
const raw = await capture(["gh", "run", "list", "--workflow", workflow, "--commit", sha, "--limit", "20", "--json", "conclusion,databaseId,headSha,status,url"]);
176213
const runs = JSON.parse(raw) as GhRun[];
177214
return runs.filter(run => run.headSha === sha);
178215
}
@@ -207,7 +244,7 @@ async function waitForSuccessfulCi(sha: string, workflow: string = CI_WORKFLOW,
207244
}
208245

209246
async function _remoteMainSha(): Promise<string> {
210-
const out = (await $`git ls-remote origin refs/heads/main`.text()).trim();
247+
const out = await capture(["git", "ls-remote", "origin", "refs/heads/main"]);
211248
const [sha] = out.split(/\s+/);
212249
if (!sha) {
213250
console.error("✗ could not resolve origin/main");
@@ -218,7 +255,7 @@ async function _remoteMainSha(): Promise<string> {
218255

219256
/** Live (network) head of a remote branch — never the local remote-tracking ref. */
220257
async function remoteBranchHead(branch: string): Promise<string> {
221-
const out = (await $`git ls-remote origin refs/heads/${branch}`.text()).trim();
258+
const out = await capture(["git", "ls-remote", "origin", `refs/heads/${branch}`]);
222259
const [sha] = out.split(/\s+/);
223260
if (!sha) {
224261
console.error(`✗ could not resolve origin/${branch}`);
@@ -240,7 +277,7 @@ if (!version || !/^\d+\.\d+\.\d+(-[\w.]+)?$/.test(version)) {
240277
const dryRun = !args.includes("--publish");
241278

242279
// 1. Preflight — must be on main or preview, and local verification must pass.
243-
const branch = (await $`git rev-parse --abbrev-ref HEAD`.text()).trim();
280+
const branch = await capture(["git", "rev-parse", "--abbrev-ref", "HEAD"]);
244281
const allowedBranches = ["main", "preview"];
245282
const expectedTag = branch === "preview" ? "preview" : "latest";
246283
const tag = args.includes("--tag") ? (args[args.indexOf("--tag") + 1] ?? expectedTag) : expectedTag;
@@ -257,27 +294,27 @@ if (branch === "main" && version.includes("-")) {
257294
process.exit(1);
258295
}
259296
if (!allowedBranches.includes(branch)) { console.error(`✗ must be on ${allowedBranches.join(" or ")} (currently ${branch}).`); process.exit(1); }
260-
if ((await $`git status --porcelain`.text()).trim()) { console.error("✗ working tree not clean — commit or stash first."); process.exit(1); }
297+
if ((await capture(["git", "status", "--porcelain"])).trim()) { console.error("✗ working tree not clean — commit or stash first."); process.exit(1); }
261298
const packageName = await readPackageName();
262299
console.log(`→ release metadata preflight (${packageName}@${version})`);
263300
await assertUnusedReleaseVersion(packageName, version);
264301
console.log("→ typecheck");
265-
await $`bun x tsc --noEmit`;
302+
await runLoud(["bun", "x", "tsc", "--noEmit"]);
266303
console.log("→ test suite");
267-
await $`bun test --isolate tests`;
304+
await runLoud(["bun", "test", "--isolate", "tests"]);
268305
console.log("→ privacy scan");
269-
await $`bun run privacy:scan`;
306+
await runLoud(["bun", "run", "privacy:scan"]);
270307

271308
// 2. Bump package.json only; the workflow creates the version tag after npm publish.
272309
console.log(`→ bump package.json → ${version}`);
273-
await $`npm version ${version} --no-git-tag-version`;
310+
await runLoud(["npm", "version", version, "--no-git-tag-version"]);
274311

275312
// 3. Commit + push the version bump.
276-
await $`git add package.json`;
277-
await $`git commit -m ${`release: v${version}`}`;
278-
const releaseSha = (await $`git rev-parse HEAD`.text()).trim();
313+
await runLoud(["git", "add", "package.json"]);
314+
await runLoud(["git", "commit", "-m", `release: v${version}`]);
315+
const releaseSha = await capture(["git", "rev-parse", "HEAD"]);
279316
console.log(`→ push origin ${branch}`);
280-
await $`git push origin ${branch}`;
317+
await runLoud(["git", "push", "origin", branch]);
281318

282319
// 4. Wait for the pushed release commit to pass CI, then dispatch the Release workflow.
283320
console.log(`→ wait for Cross-platform CI (${releaseSha})`);
@@ -301,7 +338,7 @@ if (liveOriginSha !== releaseSha) {
301338

302339
console.log(`→ dispatch Release (tag=${tag}, dry-run=${dryRun})`);
303340
const dispatchStartedAt = new Date(Date.now() - 5_000).toISOString();
304-
await $`gh workflow run release.yml --ref ${branch} -f version=${version} -f tag=${tag} -f expected-sha=${releaseSha} -f dry-run=${String(dryRun)}`;
341+
await runLoud(["gh", "workflow", "run", "release.yml", "--ref", branch, "-f", `version=${version}`, "-f", `tag=${tag}`, "-f", `expected-sha=${releaseSha}`, "-f", `dry-run=${String(dryRun)}`]);
305342

306343
// 5. Watch it.
307344
const releaseRun = await waitForReleaseWorkflowRun(releaseSha, branch, dispatchStartedAt);

src/cli/star-prompt.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { join } from "node:path";
33
import { spawnSync } from "node:child_process";
44
import { getConfigDir } from "../config";
55
import { recordOwnedConfigPath } from "../lib/config-ownership";
6+
import { commandInvocation } from "../lib/win-exec";
67
import { isAgentDriven } from "./agent-driven";
78
import { interactiveConfirm } from "./interactive-confirm";
89

@@ -29,16 +30,37 @@ export function hasStarPromptRun(): boolean {
2930
* that case the prompt stays silent instead of asking for something it would
3031
* then fail to do.
3132
*/
33+
/**
34+
* On Windows `gh` is a `.cmd` shim; a shell-less spawn of the bare name skips
35+
* PATHEXT and refuses `.cmd` targets, so it stalls until the timeout instead of
36+
* failing fast. Route every call through the launcher the rest of the CLI uses.
37+
*/
38+
/** Resolve `gh` once; callers keep their own spawnSync overload. */
39+
function ghInvocation(args: string[]) {
40+
const invocation = commandInvocation("gh", args);
41+
return {
42+
file: invocation.file,
43+
args: invocation.args,
44+
verbatim: invocation.options.windowsVerbatimArguments === true,
45+
};
46+
}
47+
3248
function ghAvailable(): boolean {
33-
const version = spawnSync("gh", ["--version"], { stdio: "ignore", timeout: 3000, windowsHide: true });
49+
const v = ghInvocation(["--version"]);
50+
const version = spawnSync(v.file, v.args,
51+
{ stdio: "ignore", timeout: 3000, windowsHide: true, windowsVerbatimArguments: v.verbatim });
3452
if (version.error || version.status !== 0) return false;
35-
const auth = spawnSync("gh", ["auth", "status"], { stdio: "ignore", timeout: 5000, windowsHide: true });
53+
const a = ghInvocation(["auth", "status"]);
54+
const auth = spawnSync(a.file, a.args,
55+
{ stdio: "ignore", timeout: 5000, windowsHide: true, windowsVerbatimArguments: a.verbatim });
3656
return !auth.error && auth.status === 0;
3757
}
3858

3959
function starRepo(): { ok: boolean; error?: string } {
40-
const r = spawnSync("gh", ["api", "-X", "PUT", `/user/starred/${REPO}`],
41-
{ encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], timeout: 10000, windowsHide: true });
60+
const star = ghInvocation(["api", "-X", "PUT", `/user/starred/${REPO}`]);
61+
const r = spawnSync(star.file, star.args,
62+
{ encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], timeout: 10000, windowsHide: true,
63+
windowsVerbatimArguments: star.verbatim });
4264
if (r.error) return { ok: false, error: r.error.message };
4365
if (r.status !== 0) return { ok: false, error: (r.stderr || r.stdout || "").trim() || `gh exited ${r.status}` };
4466
return { ok: true };

src/github/star-state.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* invalidates the cache immediately, which is why the click path never has to
1414
* wait for the TTL to see its own result.
1515
*/
16+
import { commandInvocation } from "../lib/win-exec";
17+
1618
export const STAR_REPO = "lidge-jun/opencodex";
1719
export const STAR_REPO_URL = `https://github.com/${STAR_REPO}`;
1820
/**
@@ -52,11 +54,18 @@ export interface StarDeps {
5254
*/
5355
async function spawnGh(args: string[], timeoutMs: number): Promise<{ status: number | null } | null> {
5456
try {
55-
const proc = Bun.spawn(["gh", ...args], {
57+
// On Windows `gh` is a `.cmd` shim, and a shell-less spawn of the bare name
58+
// neither consults PATHEXT nor accepts a `.cmd` target. It does not fail
59+
// fast either — it hangs until the timeout below fires, which is how these
60+
// sidebar tests turned into 5s timeouts on windows-latest while passing
61+
// everywhere else. `commandInvocation` is the resolver the CLI already uses.
62+
const invocation = commandInvocation("gh", args);
63+
const proc = Bun.spawn([invocation.file, ...invocation.args], {
5664
stdin: "ignore",
5765
stdout: "ignore",
5866
stderr: "ignore",
5967
windowsHide: true,
68+
...(invocation.options.windowsVerbatimArguments ? { windowsVerbatimArguments: true } : {}),
6069
});
6170
const timer = setTimeout(() => { try { proc.kill(); } catch { /* already gone */ } }, timeoutMs);
6271
try {

tests/install-scripts.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,12 @@ describe("install scripts", () => {
106106
const script = await readText("scripts/release.ts");
107107

108108
expect(script).toContain("waitForReleaseWorkflowRun");
109-
expect(script).toContain("gh run list --workflow release.yml --branch");
110-
expect(script).toContain("--commit");
109+
// The invariant is that the dispatched run is located by workflow, branch
110+
// and commit — not that the call is a shell string. Every external command
111+
// now goes through the shared launcher as an argv array, because Bun.$
112+
// resolved PATH itself and walked past the Windows `.cmd` test shims.
113+
expect(script).toContain('"gh", "run", "list", "--workflow", "release.yml", "--branch"');
114+
expect(script).toContain('"--commit"');
111115
expect(script).toContain("createdAt,databaseId,headSha,status,url");
112116
expect(script).toContain("await watchRun(releaseRun.databaseId)");
113117
});

tests/release-helper.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,4 +321,31 @@ describe("release helper", () => {
321321
expect(git.file.toLowerCase()).toBe("c:\\shims\\git.exe");
322322
expect(git.options.windowsVerbatimArguments).toBeUndefined();
323323
});
324+
325+
/**
326+
* The test above proves the LAUNCHER is correct; this one proves the release
327+
* script actually uses it. That distinction is not academic: `runQuiet` was
328+
* already routed through `commandInvocation` while every `git`/`bun`/`npm`
329+
* call still went through `Bun.$`, and the suite stayed green on macOS while
330+
* windows-latest failed. The built-in shell resolved PATH itself, walked past
331+
* the extension-less shim it could not execute, and reached the real `git` —
332+
* so the branch guard saw `dev` rather than the faked `main` and aborted
333+
* before logging a single call.
334+
*
335+
* A source assertion is the honest check here: the failure is "which resolver
336+
* ran", and no host-platform execution can observe that.
337+
*/
338+
test("every external command goes through the shared launcher, not the built-in shell", () => {
339+
const source = readFileSync(releaseScriptPath, "utf8");
340+
const withoutComments = source
341+
.replace(/\/\*[\s\S]*?\*\//g, "")
342+
.replace(/^\s*\/\/.*$/gm, "");
343+
344+
// Bun.$ resolves PATH with its own shell; that is exactly the bypass.
345+
expect(withoutComments).not.toMatch(/\$`/);
346+
expect(withoutComments).not.toMatch(/from\s+"bun"/);
347+
348+
// And the launcher must still be the thing it reaches for.
349+
expect(withoutComments).toContain("commandInvocation");
350+
});
324351
});

tests/startup-prompt.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,12 @@ describe("startup star prompt", () => {
5555
test("the star prompt only appears when gh can actually star", async () => {
5656
const prompt = await readText("src/cli/star-prompt.ts");
5757

58-
expect(prompt).toContain('spawnSync("gh", ["auth", "status"]');
58+
// The invariant is that the prompt is gated on a real `gh auth status`
59+
// check, not that the call is spelled a particular way. The call now goes
60+
// through the shared Windows launcher (a bare `gh` spawn stalls on a `.cmd`
61+
// shim), so assert the arguments and the resolver rather than the literal.
62+
expect(prompt).toContain('ghInvocation(["auth", "status"])');
63+
expect(prompt).toContain('commandInvocation("gh"');
5964
expect(prompt).toContain("if (!ghAvailable()) return;");
6065
});
6166

0 commit comments

Comments
 (0)