Skip to content

Commit a267597

Browse files
author
Spencer
committed
Use spawn for provider runtime commands
1 parent ee95b9c commit a267597

13 files changed

Lines changed: 271 additions & 311 deletions

packages/server/src/__tests__/provider-runtime/command-check.test.ts

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { describe, expect, it, vi } from "vitest";
22
import {
33
checkCommandAvailable,
44
getCommandLookupExecutable,
5-
resolveCommand,
65
} from "../../provider-runtime/command-check.js";
76

87
describe("getCommandLookupExecutable", () => {
@@ -25,9 +24,9 @@ describe("checkCommandAvailable", () => {
2524
})
2625
);
2726

28-
await expect(checkCommandAvailable("codex", { platform: "linux", execFile })).resolves.toBe(
29-
true
30-
);
27+
await expect(
28+
checkCommandAvailable("codex", { platform: "linux", runCommand: execFile })
29+
).resolves.toBe(true);
3130
expect(execFile).toHaveBeenCalledWith("which", ["codex"], { windowsHide: true });
3231
});
3332

@@ -38,9 +37,9 @@ describe("checkCommandAvailable", () => {
3837
}
3938
);
4039

41-
await expect(checkCommandAvailable("claude", { platform: "win32", execFile })).resolves.toBe(
42-
false
43-
);
40+
await expect(
41+
checkCommandAvailable("claude", { platform: "win32", runCommand: execFile })
42+
).resolves.toBe(false);
4443
expect(execFile).toHaveBeenCalledWith("where", ["claude"], { windowsHide: true });
4544
});
4645

@@ -52,27 +51,10 @@ describe("checkCommandAvailable", () => {
5251
})
5352
);
5453

55-
await expect(checkCommandAvailable("codex", { platform: "win32", execFile })).resolves.toBe(
56-
true
57-
);
54+
await expect(
55+
checkCommandAvailable("codex", { platform: "win32", runCommand: execFile })
56+
).resolves.toBe(true);
5857

5958
expect(execFile).toHaveBeenCalledWith("where", ["codex"], { windowsHide: true });
6059
});
6160
});
62-
63-
describe("resolveCommand", () => {
64-
it("returns the first resolved executable path from command lookup output", async () => {
65-
const execFile = vi.fn(
66-
async (_file: string, _args: string[], _options?: { windowsHide: boolean }) => ({
67-
stdout: "C:\\npm\\npm.cmd\r\nC:\\Program Files\\nodejs\\npm.cmd\r\n",
68-
stderr: "",
69-
})
70-
);
71-
72-
await expect(resolveCommand("npm", { platform: "win32", execFile })).resolves.toEqual({
73-
command: "npm",
74-
executable: "C:\\npm\\npm.cmd",
75-
});
76-
expect(execFile).toHaveBeenCalledWith("where", ["npm"], { windowsHide: true });
77-
});
78-
});
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { Buffer } from "node:buffer";
2+
import { EventEmitter } from "node:events";
3+
import { describe, expect, it, vi } from "vitest";
4+
5+
const { spawnMock } = vi.hoisted(() => ({
6+
spawnMock: vi.fn(),
7+
}));
8+
9+
vi.mock("node:child_process", async () => {
10+
const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
11+
return {
12+
...actual,
13+
spawn: spawnMock,
14+
};
15+
});
16+
17+
import { runCommandAsString } from "../../provider-runtime/command-runner.js";
18+
19+
function createChildProcessMock() {
20+
const stdout = new EventEmitter();
21+
const stderr = new EventEmitter();
22+
const child = new EventEmitter() as EventEmitter & {
23+
stdout: EventEmitter;
24+
stderr: EventEmitter;
25+
};
26+
child.stdout = stdout;
27+
child.stderr = stderr;
28+
return child;
29+
}
30+
31+
describe("runCommandAsString", () => {
32+
it("normalizes stdout and stderr to strings", async () => {
33+
spawnMock.mockImplementation(
34+
(_file: string, _args: string[], _options: { windowsHide?: boolean }) => {
35+
const child = createChildProcessMock();
36+
queueMicrotask(() => {
37+
child.stdout.emit("data", Buffer.from("ok\n"));
38+
child.stderr.emit("data", Buffer.from("warn\n"));
39+
child.emit("close", 0);
40+
});
41+
return child;
42+
}
43+
);
44+
45+
const result = await runCommandAsString("demo", ["--version"], { windowsHide: true });
46+
47+
expect(result).toEqual({ stdout: "ok\n", stderr: "warn\n" });
48+
expect(spawnMock).toHaveBeenCalledWith("demo", ["--version"], {
49+
shell: false,
50+
windowsHide: true,
51+
});
52+
});
53+
});

packages/server/src/__tests__/provider-runtime/exec-file.test.ts

Lines changed: 0 additions & 59 deletions
This file was deleted.

packages/server/src/__tests__/provider-runtime/install-manager.test.ts

Lines changed: 27 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe("ProviderInstallManager", () => {
88
const manager = new ProviderInstallManager([codexDefinition], {
99
platform: "win32",
1010
commandExists,
11-
execFile: vi.fn(async () => ({ stdout: "", stderr: "" })),
11+
runCommand: vi.fn(async () => ({ stdout: "", stderr: "" })),
1212
});
1313

1414
const job = await manager.start("codex");
@@ -26,7 +26,7 @@ describe("ProviderInstallManager", () => {
2626
const manager = new ProviderInstallManager([codexDefinition], {
2727
platform: "linux",
2828
commandExists,
29-
execFile: vi.fn(async () => ({ stdout: "", stderr: "" })),
29+
runCommand: vi.fn(async () => ({ stdout: "", stderr: "" })),
3030
});
3131

3232
const job = await manager.start("codex");
@@ -45,7 +45,7 @@ describe("ProviderInstallManager", () => {
4545
const manager = new ProviderInstallManager([codexDefinition], {
4646
platform: "aix",
4747
commandExists: vi.fn(async (command: string) => command === "npm"),
48-
execFile: vi.fn(async () => ({ stdout: "", stderr: "" })),
48+
runCommand: vi.fn(async () => ({ stdout: "", stderr: "" })),
4949
});
5050

5151
const job = await manager.start("codex");
@@ -71,7 +71,7 @@ describe("ProviderInstallManager", () => {
7171
const manager = new ProviderInstallManager([codexDefinition], {
7272
platform: "darwin",
7373
commandExists: vi.fn(async (command: string) => command === "npm"),
74-
execFile,
74+
runCommand: execFile,
7575
});
7676

7777
const first = await manager.start("codex");
@@ -96,7 +96,7 @@ describe("ProviderInstallManager", () => {
9696
const manager = new ProviderInstallManager([codexDefinition], {
9797
platform: "linux",
9898
commandExists,
99-
execFile: vi.fn(async () => ({ stdout: "", stderr: "" })),
99+
runCommand: vi.fn(async () => ({ stdout: "", stderr: "" })),
100100
});
101101

102102
const firstPromise = manager.start("codex");
@@ -119,7 +119,7 @@ describe("ProviderInstallManager", () => {
119119
const manager = new ProviderInstallManager([codexDefinition], {
120120
platform: "linux",
121121
commandExists: vi.fn(async (command: string) => command === "npm"),
122-
execFile: vi.fn(async () => {
122+
runCommand: vi.fn(async () => {
123123
throw installError;
124124
}),
125125
});
@@ -147,49 +147,33 @@ describe("ProviderInstallManager", () => {
147147
expect(stored?.steps[0]?.status).toBe("failed");
148148
});
149149

150-
it("re-resolves and executes the installed npm executable on Windows install steps", async () => {
150+
it("executes Windows install steps with the declared command names", async () => {
151151
let npmInstalled = false;
152152
let codexInstalled = false;
153+
const commandExists = vi.fn(async (command: string) => {
154+
if (command === "winget") {
155+
return true;
156+
}
157+
if (command === "npm") {
158+
return npmInstalled;
159+
}
160+
if (command === "codex") {
161+
return codexInstalled;
162+
}
163+
return false;
164+
});
153165
const execFile = vi.fn(
154166
async (file: string, args: string[], _options?: { windowsHide: boolean }) => {
155-
if (file === "where" && args[0] === "winget") {
156-
return {
157-
stdout: "C:\\Users\\test\\AppData\\Local\\Microsoft\\WindowsApps\\winget.exe\r\n",
158-
stderr: "",
159-
};
160-
}
161-
162-
if (file === "where" && args[0] === "npm") {
163-
if (!npmInstalled) {
164-
throw new Error("npm unavailable");
165-
}
166-
167-
return {
168-
stdout: "C:\\npm\\npm.cmd\r\n",
169-
stderr: "",
170-
};
171-
}
172-
173-
if (file === "where" && args[0] === "codex") {
174-
if (!codexInstalled) {
175-
throw new Error("codex unavailable");
176-
}
177-
178-
return {
179-
stdout: "C:\\codex\\codex.exe\r\n",
180-
stderr: "",
181-
};
182-
}
183-
184167
if (
185-
file === "C:\\Users\\test\\AppData\\Local\\Microsoft\\WindowsApps\\winget.exe" &&
168+
file === "winget" &&
186169
args.join(" ") === "install --id OpenJS.NodeJS.LTS --exact --silent"
187170
) {
188171
npmInstalled = true;
189172
return { stdout: "installed node", stderr: "" };
190173
}
191174

192-
if (file === "C:\\npm\\npm.cmd" && args.join(" ") === "install -g @openai/codex") {
175+
if (file === "npm" && args.join(" ") === "install -g @openai/codex") {
176+
expect(npmInstalled).toBe(true);
193177
codexInstalled = true;
194178
return { stdout: "installed codex", stderr: "" };
195179
}
@@ -199,7 +183,8 @@ describe("ProviderInstallManager", () => {
199183
);
200184
const manager = new ProviderInstallManager([codexDefinition], {
201185
platform: "win32",
202-
execFile,
186+
commandExists,
187+
runCommand: execFile,
203188
});
204189

205190
const job = await manager.start("codex");
@@ -208,10 +193,8 @@ describe("ProviderInstallManager", () => {
208193
expect(manager.get(job.jobId)?.status).toBe("succeeded");
209194
});
210195

211-
expect(execFile).toHaveBeenCalledWith(
212-
"C:\\npm\\npm.cmd",
213-
["install", "-g", "@openai/codex"],
214-
{ windowsHide: true }
215-
);
196+
expect(execFile).toHaveBeenCalledWith("npm", ["install", "-g", "@openai/codex"], {
197+
windowsHide: true,
198+
});
216199
});
217200
});

0 commit comments

Comments
 (0)