mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): resolve Windows shells from installation directories
Use installation environment variables for PowerShell discovery and legacy WSL launcher detection.
This commit is contained in:
parent
28bea006be
commit
61060ddf8c
8 changed files with 209 additions and 31 deletions
|
|
@ -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/<drive>`, 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:
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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)) {
|
||||
|
|
|
|||
90
packages/coding-agent/test/powershell-config.test.ts
Normal file
90
packages/coding-agent/test/powershell-config.test.ts
Normal file
|
|
@ -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<typeof Fs>()),
|
||||
accessSync: accessSyncMock,
|
||||
}));
|
||||
|
||||
vi.mock("child_process", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof ChildProcess>()),
|
||||
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");
|
||||
});
|
||||
});
|
||||
|
|
@ -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<typeof import("child_process")>();
|
||||
const actual = await importOriginal<typeof ChildProcess>();
|
||||
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 },
|
||||
);
|
||||
|
|
|
|||
37
packages/coding-agent/test/windows-shell-config.test.ts
Normal file
37
packages/coding-agent/test/windows-shell-config.test.ts
Normal file
|
|
@ -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<typeof Fs>()),
|
||||
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"] },
|
||||
);
|
||||
});
|
||||
});
|
||||
15
packages/durable/src/env/node.ts
vendored
15
packages/durable/src/env/node.ts
vendored
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue