diff --git a/packages/coding-agent/docs/windows.md b/packages/coding-agent/docs/windows.md index c4cfdc9c7..26f2f961c 100644 --- a/packages/coding-agent/docs/windows.md +++ b/packages/coding-agent/docs/windows.md @@ -19,8 +19,9 @@ For most native Windows users, installing [Git for Windows](https://git-scm.com/ Pi resolves Bash in this order: 1. `shellPath` from `~/.pi/agent/settings.json` -2. Git Bash under `Program Files` or `Program Files (x86)` -3. `bash.exe` on `PATH`, including Cygwin, MSYS2, or legacy WSL Bash +2. Git Bash at `%ProgramFiles%\Git\bin\bash.exe` +3. Git Bash at `%ProgramFiles(x86)%\Git\bin\bash.exe` +4. `bash.exe` on `PATH`, including Cygwin, MSYS2, or legacy WSL Bash Start Pi and enter this command to verify the shell: @@ -34,6 +35,13 @@ If Pi cannot find Bash, it reports the locations it checked. Install Git for Win The optional `powershell` tool runs commands through `pwsh.exe` when available, then falls back to Windows PowerShell. It starts PowerShell with `-NoProfile -NonInteractive -ExecutionPolicy Bypass`. Administrator-enforced execution policies can still take precedence. +PowerShell discovery checks these locations in order: + +1. `pwsh.exe` on `PATH` +2. `%ProgramFiles%\PowerShell\7\pwsh.exe` +3. `powershell.exe` on `PATH` +4. `%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe` + To replace the model-facing `bash` tool with `powershell`, add this to `~/.pi/agent/settings.json`: ```json @@ -48,6 +56,12 @@ Restart Pi, then ask it to run a harmless PowerShell command. The `!` and `!!` e See [Settings](settings.md#tools) for other tool combinations. +## WSL + +WSL path conversion on Windows and extra Git branch polling in WSL recognize `/mnt/`, not custom mount roots. + +When using the legacy WSL `bash.exe` launcher from Windows, Pi sends commands over stdin to avoid command-line quoting problems. + ## Use a custom Bash executable Set `shellPath` when Bash is installed somewhere Pi does not discover automatically: diff --git a/packages/coding-agent/src/utils/shell.ts b/packages/coding-agent/src/utils/shell.ts index ab30cdacd..fbe81f965 100644 --- a/packages/coding-agent/src/utils/shell.ts +++ b/packages/coding-agent/src/utils/shell.ts @@ -1,5 +1,5 @@ import { accessSync, constants } from "node:fs"; -import { delimiter, join } from "node:path"; +import { delimiter, join, win32 } from "node:path"; import { spawn, spawnSync } from "child_process"; import { getBinDir } from "../config.ts"; @@ -9,11 +9,17 @@ export interface ShellConfig { commandTransport?: "argv" | "stdin"; } -/** - * Find bash executable on PATH (cross-platform) - */ +// Keep in sync with packages/durable/src/env/node.ts. function isLegacyWslBashPath(path: string): boolean { - const normalized = path.replace(/\//g, "\\").toLowerCase(); + const normalized = win32.normalize(path).toLowerCase(); + const systemRoot = process.env.SystemRoot; + if (systemRoot) { + if (!/^[a-z]:[\\/]/i.test(systemRoot)) return false; + return ["System32", "Sysnative"].some( + (directory) => normalized === win32.join(systemRoot, directory, "bash.exe").toLowerCase(), + ); + } + // Fall back to conventional launcher paths only when the Windows directory is unknown. return /^[a-z]:\\windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized); } @@ -140,7 +146,21 @@ export function getPowerShellConfig(): ShellConfig { throw new Error("The powershell tool is only available on Windows."); } - const shell = findExecutableOnPath("pwsh.exe") ?? findExecutableOnPath("powershell.exe"); + let shell = findExecutableOnPath("pwsh.exe"); + // Installation directories may be on any drive and need not be on PATH. + const programFiles = process.env.ProgramFiles; + if (!shell && programFiles) { + const pwsh7 = win32.join(programFiles, "PowerShell", "7", "pwsh.exe"); + if (shellPathExists(pwsh7)) shell = pwsh7; + } + + shell ??= findExecutableOnPath("powershell.exe"); + const systemRoot = process.env.SystemRoot; + if (!shell && systemRoot) { + const windowsPowerShell = win32.join(systemRoot, "System32", "WindowsPowerShell", "v1.0", "powershell.exe"); + if (shellPathExists(windowsPowerShell)) shell = windowsPowerShell; + } + if (!shell) { throw new Error("No PowerShell executable found. Install PowerShell or add powershell.exe/pwsh.exe to PATH."); } @@ -202,7 +222,7 @@ export function killProcessTree(pid: number): void { // Use the trusted System32 executable so cleanup does not depend on PATH. try { const child = spawn( - join(process.env.SystemRoot ?? "C:\\Windows", "System32", "taskkill.exe"), + join(process.env.SystemRoot || "C:\\Windows", "System32", "taskkill.exe"), ["/F", "/T", "/PID", String(pid)], { stdio: "ignore", diff --git a/packages/coding-agent/src/utils/tools-manager.ts b/packages/coding-agent/src/utils/tools-manager.ts index f040f5247..3f204489b 100644 --- a/packages/coding-agent/src/utils/tools-manager.ts +++ b/packages/coding-agent/src/utils/tools-manager.ts @@ -206,7 +206,7 @@ function extractTarGzArchive(archivePath: string, extractDir: string, assetName: } function getWindowsTarCommand(): string { - const systemRoot = process.env.SystemRoot ?? process.env.WINDIR; + const systemRoot = process.env.SystemRoot; if (systemRoot) { const systemTar = join(systemRoot, "System32", "tar.exe"); if (existsSync(systemTar)) { diff --git a/packages/coding-agent/test/powershell-config.test.ts b/packages/coding-agent/test/powershell-config.test.ts new file mode 100644 index 000000000..1d703df40 --- /dev/null +++ b/packages/coding-agent/test/powershell-config.test.ts @@ -0,0 +1,90 @@ +import type * as ChildProcess from "node:child_process"; +import type * as Fs from "node:fs"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { getPowerShellConfig, POWERSHELL_ARGS } from "../src/utils/shell.ts"; + +const { accessSyncMock, spawnSyncMock } = vi.hoisted(() => ({ + accessSyncMock: vi.fn(), + spawnSyncMock: vi.fn(), +})); + +vi.mock("node:fs", async (importOriginal) => ({ + ...(await importOriginal()), + accessSync: accessSyncMock, +})); + +vi.mock("child_process", async (importOriginal) => ({ + ...(await importOriginal()), + spawnSync: spawnSyncMock, +})); + +const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform")!; +const pwsh7 = "D:\\Applications\\PowerShell\\7\\pwsh.exe"; +const windowsPowerShell = "E:\\WinNT\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"; + +beforeEach(() => { + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + vi.stubEnv("ProgramFiles", "D:\\Applications"); + vi.stubEnv("SystemRoot", "E:\\WinNT"); + accessSyncMock.mockReset().mockImplementation(() => { + throw Object.assign(new Error("not found"), { code: "ENOENT" }); + }); + spawnSyncMock.mockReset().mockReturnValue({ status: 1, stdout: "" }); +}); + +afterEach(() => { + Object.defineProperty(process, "platform", platformDescriptor); + vi.unstubAllEnvs(); +}); + +// #9490: discover PowerShell outside PATH without assuming a drive or Windows directory name. +describe("getPowerShellConfig", () => { + it("prefers PowerShell 7 on PATH over known installation locations", () => { + const shell = "E:\\Tools\\pwsh.exe"; + spawnSyncMock.mockReturnValue({ status: 0, stdout: `${shell}\r\n` }); + accessSyncMock.mockReturnValue(undefined); + + expect(getPowerShellConfig()).toEqual({ shell, args: [...POWERSHELL_ARGS] }); + }); + + it("finds PowerShell 7 under ProgramFiles before Windows PowerShell on PATH", () => { + spawnSyncMock.mockImplementation((_command: string, args: string[]) => ({ + status: args[0] === "powershell.exe" ? 0 : 1, + stdout: args[0] === "powershell.exe" ? windowsPowerShell : "", + })); + accessSyncMock.mockImplementation((path: string) => { + if (![pwsh7, windowsPowerShell].includes(path as string)) { + throw Object.assign(new Error("not found"), { code: "ENOENT" }); + } + }); + + expect(getPowerShellConfig()).toEqual({ shell: pwsh7, args: [...POWERSHELL_ARGS] }); + }); + + it("prefers Windows PowerShell on PATH over its system installation", () => { + const shell = "E:\\Tools\\powershell.exe"; + spawnSyncMock.mockReturnValueOnce({ status: 1, stdout: "" }).mockReturnValueOnce({ status: 0, stdout: shell }); + accessSyncMock.mockImplementation((path: string) => { + if (![shell, windowsPowerShell].includes(path as string)) { + throw Object.assign(new Error("not found"), { code: "ENOENT" }); + } + }); + + expect(getPowerShellConfig()).toEqual({ shell, args: [...POWERSHELL_ARGS] }); + }); + + it("finds Windows PowerShell under SystemRoot when PATH lookup is unavailable", () => { + spawnSyncMock.mockReturnValue({ status: null, stdout: null, error: new Error("spawn where ENOENT") }); + accessSyncMock.mockImplementation((path: string) => { + if (path !== windowsPowerShell) { + throw Object.assign(new Error("not found"), { code: "ENOENT" }); + } + }); + + expect(getPowerShellConfig()).toEqual({ shell: windowsPowerShell, args: [...POWERSHELL_ARGS] }); + }); + + it("reports an error when no PowerShell installation exists", () => { + expect(() => getPowerShellConfig()).toThrow("No PowerShell executable found"); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/6596-taskkill-enoent.test.ts b/packages/coding-agent/test/suite/regressions/6596-taskkill-enoent.test.ts index c5e47f98e..2a6dfe3c5 100644 --- a/packages/coding-agent/test/suite/regressions/6596-taskkill-enoent.test.ts +++ b/packages/coding-agent/test/suite/regressions/6596-taskkill-enoent.test.ts @@ -1,4 +1,4 @@ -import type { ChildProcess } from "node:child_process"; +import type * as ChildProcess from "node:child_process"; import { EventEmitter } from "node:events"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -6,7 +6,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() })); vi.mock("child_process", async (importOriginal) => { - const actual = await importOriginal(); + const actual = await importOriginal(); return { ...actual, spawn: spawnMock }; }); @@ -24,26 +24,26 @@ function withWindowsPlatform(test: () => void): void { afterEach(() => { spawnMock.mockReset(); + vi.unstubAllEnvs(); }); describe("issue #6596 taskkill spawn failures", () => { - it("uses System32 taskkill and consumes its asynchronous spawn error", () => { - const child = new EventEmitter() as ChildProcess; - const previousSystemRoot = process.env.SystemRoot; - process.env.SystemRoot = "C:\\CustomWindows"; + // #9490: missing or empty SystemRoot must retain an absolute taskkill fallback. + it.each([ + ["D:\\CustomWindows", "D:\\CustomWindows"], + [undefined, "C:\\Windows"], + ["", "C:\\Windows"], + ] as const)("uses System32 taskkill and consumes its spawn error with SystemRoot=%j", (systemRoot, expectedRoot) => { + const child = new EventEmitter(); + vi.stubEnv("SystemRoot", systemRoot); spawnMock.mockReturnValue(child); - try { - withWindowsPlatform(() => { - killProcessTree(1234); - }); - } finally { - if (previousSystemRoot === undefined) delete process.env.SystemRoot; - else process.env.SystemRoot = previousSystemRoot; - } + withWindowsPlatform(() => { + killProcessTree(1234); + }); expect(spawnMock).toHaveBeenCalledWith( - join("C:\\CustomWindows", "System32", "taskkill.exe"), + join(expectedRoot, "System32", "taskkill.exe"), ["/F", "/T", "/PID", "1234"], { detached: true, stdio: "ignore", windowsHide: true }, ); diff --git a/packages/coding-agent/test/windows-shell-config.test.ts b/packages/coding-agent/test/windows-shell-config.test.ts new file mode 100644 index 000000000..52006eab1 --- /dev/null +++ b/packages/coding-agent/test/windows-shell-config.test.ts @@ -0,0 +1,37 @@ +import type * as Fs from "node:fs"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { getShellConfig } from "../src/utils/shell.ts"; + +vi.mock("node:fs", async (importOriginal) => ({ + ...(await importOriginal()), + accessSync: () => {}, +})); + +const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform")!; + +beforeEach(() => { + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); +}); + +afterEach(() => { + Object.defineProperty(process, "platform", platformDescriptor); + vi.unstubAllEnvs(); +}); + +// #9490: identify legacy WSL launchers under the actual Windows directory. +describe("legacy WSL bash detection", () => { + it.each([ + ["D:\\WinNT\\", "d:/WINNT/System32/BASH.EXE", true], + ["D:\\WinNT", "D:\\WinNT\\Sysnative\\bash.exe", true], + [undefined, "E:\\Windows\\System32\\bash.exe", true], + ["D:\\WinNT", "D:\\Git\\bin\\bash.exe", false], + ["D:\\WinNT", "E:\\Windows\\System32\\bash.exe", false], + ["WinNT", "C:\\Windows\\System32\\bash.exe", false], + ] as const)("resolves %j / %j to stdin=%j", (systemRoot, shell, stdin) => { + vi.stubEnv("SystemRoot", systemRoot); + + expect(getShellConfig(shell)).toEqual( + stdin ? { shell, args: ["-s"], commandTransport: "stdin" } : { shell, args: ["-c"] }, + ); + }); +}); diff --git a/packages/durable/src/env/node.ts b/packages/durable/src/env/node.ts index bb9776e72..fe93ce4ed 100644 --- a/packages/durable/src/env/node.ts +++ b/packages/durable/src/env/node.ts @@ -18,7 +18,7 @@ import { writeFile, } from "node:fs/promises"; import { homedir, constants as osConstants, tmpdir } from "node:os"; -import { basename, isAbsolute, join, resolve } from "node:path"; +import { basename, isAbsolute, join, resolve, win32 } from "node:path"; import { fileURLToPath } from "node:url"; import type { Context } from "@earendil-works/chord"; import { StreamDecoder } from "./decode.ts"; @@ -205,8 +205,17 @@ interface ShellConfig { commandTransport?: "argv" | "stdin"; } +// Keep in sync with packages/coding-agent/src/utils/shell.ts. function isLegacyWslBashPath(path: string): boolean { - const normalized = path.replace(/\//g, "\\").toLowerCase(); + const normalized = win32.normalize(path).toLowerCase(); + const systemRoot = process.env.SystemRoot; + if (systemRoot) { + if (!/^[a-z]:[\\/]/i.test(systemRoot)) return false; + return ["System32", "Sysnative"].some( + (directory) => normalized === win32.join(systemRoot, directory, "bash.exe").toLowerCase(), + ); + } + // Fall back to conventional launcher paths only when the Windows directory is unknown. return /^[a-z]:\\windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized); } @@ -275,7 +284,7 @@ function killProcessTree(pid: number): void { if (process.platform === "win32") { try { const child = spawn( - join(process.env.SystemRoot ?? "C:\\Windows", "System32", "taskkill.exe"), + join(process.env.SystemRoot || "C:\\Windows", "System32", "taskkill.exe"), ["/F", "/T", "/PID", String(pid)], { stdio: "ignore", diff --git a/packages/durable/test/env-node.test.ts b/packages/durable/test/env-node.test.ts index 37410f05c..4287987de 100644 --- a/packages/durable/test/env-node.test.ts +++ b/packages/durable/test/env-node.test.ts @@ -654,10 +654,16 @@ describe("NodeExecutionEnv shell", () => { } }); - it("uses stdin command transport for legacy WSL bash paths", async () => { + // #9490: legacy WSL detection must also handle a custom Windows directory. + it.each([ + ["C:\\Windows\\System32\\bash.exe", undefined, "-s"], + ["D:\\WinNT\\System32\\bash.exe", "D:\\WinNT", "-s"], + ["D:\\WinNT\\Sysnative\\bash.exe", "D:\\WinNT", "-s"], + ["D:\\WinNT\\System32\\bash.exe", "", "-c"], + ["E:\\Windows\\System32\\bash.exe", "D:\\WinNT", "-c"], + ] as const)("selects command transport for %s (SystemRoot=%j)", async (shellPath, systemRoot, expectedArg) => { if (process.platform === "win32") return; const root = createTempDir(); - const shellPath = "C:\\Windows\\System32\\bash.exe"; const env = new NodeExecutionEnv({ cwd: root }); getOrThrow( await env.writeFile( @@ -674,6 +680,7 @@ describe("NodeExecutionEnv shell", () => { try { process.chdir(root); process.env.PATH = `${root}${delimiter}${originalPath ?? ""}`; + vi.stubEnv("SystemRoot", systemRoot); Object.defineProperty(process, "platform", { configurable: true, value: "win32", @@ -689,11 +696,12 @@ describe("NodeExecutionEnv shell", () => { ); const result = getOrThrow(collected.result); expect(collected.output).toContain("Hello, World!"); - expect(collected.output).toContain("args:-s"); + expect(collected.output).toContain(`args:${expectedArg}`); expect(result.exitCode).toBe(0); } finally { process.chdir(originalCwd); process.env.PATH = originalPath; + vi.unstubAllEnvs(); if (platformDescriptor) { Object.defineProperty(process, "platform", platformDescriptor); }