fix(windows): probe absolute powershell path so PATH-degraded children don't fall through to cmd.exe
The dashboard (Next.js) sometimes spawns the runtime-process pty-host with a PATH that lacks C:\Windows\System32. Both `pwsh` and `powershell.exe` probes in resolveWindowsShell() then fail and we drop to cmd.exe — which can't execute the PowerShell-syntax launch commands agents emit (e.g. Codex's `& 'codex' ...`), producing `'&' was unexpected at this time.` and an immediately-exited orchestrator. Probe %SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe directly via existsSync — this path is guaranteed on Windows 10+ and doesn't depend on PATH. Also add an AO_SHELL env override as an explicit escape hatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
43de59de23
commit
0f5df48f06
|
|
@ -19,11 +19,17 @@ vi.mock("node:os", () => ({
|
|||
userInfo: vi.fn(() => ({ username: "mockuser" })),
|
||||
}));
|
||||
|
||||
vi.mock("node:fs", () => ({
|
||||
existsSync: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
// Stable mock references — safe across the module cache lifetime.
|
||||
const mockExecFile = vi.mocked(childProcess.execFile);
|
||||
const mockExecFileSync = vi.mocked(childProcess.execFileSync);
|
||||
const mockHomedir = vi.mocked(os.homedir);
|
||||
const mockUserInfo = vi.mocked(os.userInfo);
|
||||
const fs = await import("node:fs");
|
||||
const mockExistsSync = vi.mocked(fs.existsSync);
|
||||
|
||||
const originalPlatform = process.platform;
|
||||
|
||||
|
|
@ -63,6 +69,11 @@ function setPlatform(p: string): void {
|
|||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
setPlatform(originalPlatform);
|
||||
// Default: absolute powershell path doesn't exist (let probe-based tests run).
|
||||
// Specific tests override this when exercising the absolute-path branch.
|
||||
mockExistsSync.mockReturnValue(false);
|
||||
// AO_SHELL short-circuits auto-detection; clear unless a test sets it.
|
||||
delete process.env["AO_SHELL"];
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
|
|
@ -81,13 +92,15 @@ describe("resolveWindowsShell", () => {
|
|||
mod._resetShellCache();
|
||||
});
|
||||
|
||||
it("falls back to powershell.exe when pwsh is not available", async () => {
|
||||
it("falls back to powershell.exe via PATH probe when pwsh missing and absolute path missing", async () => {
|
||||
setPlatform("win32");
|
||||
// pwsh throws, powershell.exe succeeds
|
||||
// pwsh throws, powershell.exe (PATH probe) succeeds
|
||||
mockExecFileSync.mockImplementationOnce(() => {
|
||||
throw new Error("pwsh not found");
|
||||
});
|
||||
mockExecFileSync.mockImplementationOnce(() => "");
|
||||
// Absolute path doesn't exist, forcing the bare-name probe
|
||||
mockExistsSync.mockReturnValue(false);
|
||||
|
||||
const mod = await import("../platform.js");
|
||||
mod._resetShellCache();
|
||||
|
|
@ -97,6 +110,49 @@ describe("resolveWindowsShell", () => {
|
|||
expect(shell.args("echo hi")).toEqual(["-Command", "echo hi"]);
|
||||
});
|
||||
|
||||
it("uses absolute powershell.exe path when pwsh fails (PATH-degraded process)", async () => {
|
||||
setPlatform("win32");
|
||||
// pwsh throws, then absolute path exists — never reaches the bare-name probe
|
||||
mockExecFileSync.mockImplementationOnce(() => {
|
||||
throw new Error("pwsh not found");
|
||||
});
|
||||
mockExistsSync.mockReturnValue(true);
|
||||
|
||||
const savedRoot = process.env["SystemRoot"];
|
||||
process.env["SystemRoot"] = "C:\\Windows";
|
||||
|
||||
try {
|
||||
const mod = await import("../platform.js");
|
||||
mod._resetShellCache();
|
||||
const shell = mod.getShell();
|
||||
|
||||
expect(shell.cmd).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe");
|
||||
expect(shell.args("echo hi")).toEqual(["-Command", "echo hi"]);
|
||||
} finally {
|
||||
if (savedRoot !== undefined) process.env["SystemRoot"] = savedRoot;
|
||||
else delete process.env["SystemRoot"];
|
||||
}
|
||||
});
|
||||
|
||||
it("honors AO_SHELL override before any auto-detection", async () => {
|
||||
setPlatform("win32");
|
||||
const saved = process.env["AO_SHELL"];
|
||||
process.env["AO_SHELL"] = "C:\\custom\\pwsh.exe";
|
||||
|
||||
try {
|
||||
const mod = await import("../platform.js");
|
||||
mod._resetShellCache();
|
||||
const shell = mod.getShell();
|
||||
expect(shell.cmd).toBe("C:\\custom\\pwsh.exe");
|
||||
expect(shell.args("echo hi")).toEqual(["-Command", "echo hi"]);
|
||||
// Auto-detection probes are skipped entirely
|
||||
expect(mockExecFileSync).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
if (saved !== undefined) process.env["AO_SHELL"] = saved;
|
||||
else delete process.env["AO_SHELL"];
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back to cmd.exe when both pwsh and powershell.exe are unavailable", async () => {
|
||||
setPlatform("win32");
|
||||
mockExecFileSync.mockImplementation(() => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { execFile as execFileCb, execFileSync } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { homedir, userInfo } from "node:os";
|
||||
import { existsSync } from "node:fs";
|
||||
|
||||
const execFileAsync = promisify(execFileCb);
|
||||
|
||||
|
|
@ -29,15 +30,34 @@ interface ShellInfo {
|
|||
let cachedShell: ShellInfo | null = null;
|
||||
|
||||
function resolveWindowsShell(): ShellInfo {
|
||||
// Explicit override — set AO_SHELL to an absolute path or shell name
|
||||
// (e.g. "powershell.exe", "pwsh", "C:\\Path\\To\\pwsh.exe") to bypass auto-detection.
|
||||
const override = process.env["AO_SHELL"];
|
||||
if (override) {
|
||||
return { cmd: override, args: (c) => ["-Command", c] };
|
||||
}
|
||||
|
||||
// Prefer pwsh (PowerShell Core, cross-platform)
|
||||
try {
|
||||
execFileSync("pwsh", ["-Version"], { timeout: 5000, stdio: "ignore", windowsHide: true });
|
||||
return { cmd: "pwsh", args: (c) => ["-Command", c] };
|
||||
} catch {
|
||||
// not installed
|
||||
// not installed or not on PATH
|
||||
}
|
||||
|
||||
// Fall back to powershell.exe (Windows PowerShell, always on Win 10+)
|
||||
// Fall back to powershell.exe (Windows PowerShell, always on Win 10+).
|
||||
// Use the absolute path because the spawning process may have a degraded
|
||||
// PATH that doesn't include C:\Windows\System32 (e.g. Next.js dashboard
|
||||
// children spawned without full system PATH inheritance). Without this,
|
||||
// both probes fail and we'd fall through to cmd.exe — which breaks any
|
||||
// launch command that uses PowerShell syntax (e.g. Codex's `& 'codex' ...`).
|
||||
const systemRoot = process.env["SystemRoot"] || "C:\\Windows";
|
||||
const psAbsolute = `${systemRoot}\\System32\\WindowsPowerShell\\v1.0\\powershell.exe`;
|
||||
if (existsSync(psAbsolute)) {
|
||||
return { cmd: psAbsolute, args: (c) => ["-Command", c] };
|
||||
}
|
||||
|
||||
// Try the bare name as a final PowerShell attempt (in case of unusual installs)
|
||||
try {
|
||||
execFileSync("powershell.exe", ["-Command", "echo ok"], {
|
||||
timeout: 5000,
|
||||
|
|
@ -49,7 +69,9 @@ function resolveWindowsShell(): ShellInfo {
|
|||
// not available (very unlikely on Win 10+)
|
||||
}
|
||||
|
||||
// Last resort: cmd.exe
|
||||
// Last resort: cmd.exe. Note that agent launch commands often use PowerShell
|
||||
// syntax (e.g. the `&` call operator) and will fail under cmd.exe. Setting
|
||||
// AO_SHELL is the supported escape hatch.
|
||||
const comspec = process.env["ComSpec"] || "cmd.exe";
|
||||
return { cmd: comspec, args: (c) => ["/c", c] };
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue