Skip to content

Commit f23e446

Browse files
committed
refactor(release): reuse the shared win-exec launcher
The release script carried its own copy of the Windows shim resolution that src/lib/win-exec.ts already owns. Two copies of the same PATHEXT reasoning drift apart, and this one sits in the preflight that runs before anything else, so a divergence there aborts a release before a single command executes. Behaviour is unchanged; the logic now has one home.
1 parent 0dfd3ae commit f23e446

2 files changed

Lines changed: 56 additions & 24 deletions

File tree

scripts/release.ts

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

2122
const args = process.argv.slice(2);
2223
interface GhRun {
@@ -39,30 +40,22 @@ const SERVICE_WORKFLOW = "service-lifecycle.yml";
3940
const CI_WAIT_TIMEOUT_MS = 20 * 60 * 1000;
4041
const CI_POLL_MS = 10 * 1000;
4142

42-
/**
43-
* On Windows, `npm`, `gh`, and `git` are usually `.cmd`/`.exe` shims rather than
44-
* bare files on PATH. `Bun.$` resolves those, but `Bun.spawn` does not: it looks
45-
* for a literal `npm` and fails. The preflight below is the FIRST thing the
46-
* release runs, so that mismatch aborted the script before a single shim was
47-
* invoked — which is why the release-helper tests saw exit 1 and an empty call
48-
* log on windows-latest while every other platform passed.
49-
*
50-
* `.cmd` is tried ahead of `.exe` because that is what npm and gh actually ship
51-
* on Windows; a real `.exe` (git) still resolves through PATHEXT lookup below.
52-
*/
53-
function resolveCommandForPlatform(command: string[]): string[] {
54-
if (process.platform !== "win32") return command;
55-
const [bin, ...rest] = command;
56-
if (!bin || bin.includes("\\") || bin.includes("/") || /\.[a-z]+$/i.test(bin)) return command;
57-
for (const extension of [".cmd", ".exe", ".bat"]) {
58-
const resolved = Bun.which(`${bin}${extension}`);
59-
if (resolved) return [resolved, ...rest];
60-
}
61-
return [Bun.which(bin) ?? bin, ...rest];
62-
}
63-
6443
async function runQuiet(command: string[]): Promise<CommandResult> {
65-
const proc = Bun.spawn(resolveCommandForPlatform(command), { stdout: "pipe", stderr: "pipe" });
44+
// Windows exposes npm and gh as `.cmd` shims. A shell-less spawn of a bare
45+
// `npm` skips PATHEXT entirely and refuses `.cmd` targets outright, so this
46+
// preflight — the first thing a release does — aborted before invoking a
47+
// single command, and the release-helper tests saw exit 1 with an empty call
48+
// log. `commandInvocation` is the module the CLI already uses for exactly
49+
// this, escaping included; do not hand-roll a second resolver here.
50+
const [bin, ...rest] = command;
51+
const invocation = commandInvocation(bin ?? "", rest);
52+
const proc = Bun.spawn([invocation.file, ...invocation.args], {
53+
stdout: "pipe",
54+
stderr: "pipe",
55+
// Load-bearing on the `cmd.exe /d /s /c` path: the invocation is already a
56+
// fully escaped command LINE, so re-quoting it would corrupt the arguments.
57+
...(invocation.options.windowsVerbatimArguments ? { windowsVerbatimArguments: true } : {}),
58+
});
6659
const [stdout, stderr, exitCode] = await Promise.all([
6760
new Response(proc.stdout).text(),
6861
new Response(proc.stderr).text(),

tests/release-helper.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { spawnSync } from "node:child_process";
44
import { tmpdir } from "node:os";
55
import { dirname, join } from "node:path";
66
import { fileURLToPath } from "node:url";
7+
import { commandInvocation } from "../src/lib/win-exec";
78

89
setDefaultTimeout(30_000);
910

@@ -216,7 +217,10 @@ describe("release helper", () => {
216217
test("preflight runs typecheck, test suite, and privacy scan before version bump on main dry-runs", () => {
217218
const { calls, result } = runRelease("9.9.9");
218219

219-
expect(result.status).toBe(0);
220+
// Report what the script actually said. A bare status assertion turned a
221+
// Windows-only spawn failure into "Expected: 0 Received: 1" with no cause,
222+
// which cost a full CI round to diagnose.
223+
expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0");
220224

221225
const typecheckIndex = findCallIndex(calls, "bun", call => call.args.join(" ") === "x tsc --noEmit");
222226
const testIndex = findCallIndex(calls, "bun", call => call.args.join(" ") === "test --isolate tests");
@@ -282,4 +286,39 @@ describe("release helper", () => {
282286
expect(result.stderr + result.stdout).toContain("moved while waiting for CI");
283287
expect(findCallIndex(calls, "gh", call => call.args[0] === "workflow" && call.args[1] === "run")).toBe(-1);
284288
});
289+
290+
/**
291+
* The preflight's `runQuiet` callers (`npm view`, `git ls-remote`, `gh release
292+
* view`) are the first commands a release runs. On Windows they are `.cmd`
293+
* shims, and a shell-less spawn of a bare `npm` neither consults PATHEXT nor
294+
* accepts a `.cmd` target — so the script died before invoking anything and
295+
* the four tests above failed with an empty call log on windows-latest only.
296+
*
297+
* The rest of this suite runs on the host platform, so on macOS/Linux it can
298+
* never exercise that path. Pin the win32 resolution directly instead of
299+
* waiting for CI to tell us.
300+
*/
301+
test("preflight commands resolve through the Windows .cmd launcher", () => {
302+
const env = { PATH: "C:\\shims", PATHEXT: ".COM;.EXE;.BAT;.CMD" };
303+
const cmdShim = (name: string) => (path: string) => path.toLowerCase() === `c:\\shims\\${name}.cmd`;
304+
305+
const npm = commandInvocation("npm", ["view", "pkg@9.9.9", "version"], "win32", { env, exists: cmdShim("npm") });
306+
expect(npm.file).toBe("cmd.exe");
307+
expect(npm.options.windowsVerbatimArguments).toBe(true);
308+
expect(npm.args.join(" ")).toContain("npm.cmd");
309+
// A bare name would have survived unresolved and ENOENT'd at spawn time.
310+
expect(npm.args.join(" ")).not.toBe("npm");
311+
312+
const gh = commandInvocation("gh", ["release", "view", "v9.9.9"], "win32", { env, exists: cmdShim("gh") });
313+
expect(gh.file).toBe("cmd.exe");
314+
expect(gh.args.join(" ")).toContain("gh.cmd");
315+
316+
// A real `.exe` (git) must NOT be wrapped: direct spawn keeps arg boundaries.
317+
const git = commandInvocation("git", ["ls-remote", "origin"], "win32", {
318+
env,
319+
exists: (path: string) => path.toLowerCase() === "c:\\shims\\git.exe",
320+
});
321+
expect(git.file.toLowerCase()).toBe("c:\\shims\\git.exe");
322+
expect(git.options.windowsVerbatimArguments).toBeUndefined();
323+
});
285324
});

0 commit comments

Comments
 (0)