From 1287b69fe026a9c3f9cec8a220ad9405851f7dc3 Mon Sep 17 00:00:00 2001 From: Vegard Stikbakke Date: Fri, 19 Jun 2026 15:05:16 +0200 Subject: [PATCH] fix(coding-agent): run legacy WSL bash commands via stdin closes #5893 --- packages/agent/CHANGELOG.md | 4 ++ packages/agent/src/harness/env/nodejs.ts | 52 +++++++++++++----- .../agent/test/harness/nodejs-env.test.ts | 35 +++++++++++- packages/coding-agent/CHANGELOG.md | 1 + .../src/core/resolve-config-value.ts | 8 ++- packages/coding-agent/src/core/tools/bash.ts | 11 +++- packages/coding-agent/src/utils/shell.ts | 20 +++++-- .../coding-agent/test/auth-storage.test.ts | 27 ++++++++- packages/coding-agent/test/tools.test.ts | 50 +++++++++++++++++ scripts/repro-5893-wsl-bash.mjs | 55 +++++++++++++++++++ 10 files changed, 235 insertions(+), 28 deletions(-) create mode 100644 scripts/repro-5893-wsl-bash.mjs diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 0ac61b2f..18d0e561 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Node execution environment commands through legacy WSL `bash.exe` to pass scripts over stdin so shell variables expand in the target bash ([#5893](https://github.com/earendil-works/pi/issues/5893)). + ## [0.79.8] - 2026-06-19 ### Added diff --git a/packages/agent/src/harness/env/nodejs.ts b/packages/agent/src/harness/env/nodejs.ts index e56e7aeb..3d929c82 100644 --- a/packages/agent/src/harness/env/nodejs.ts +++ b/packages/agent/src/harness/env/nodejs.ts @@ -144,12 +144,25 @@ async function findBashOnPath(): Promise { return firstMatch && (await pathExists(firstMatch)) ? firstMatch : null; } -async function getShellConfig( - customShellPath?: string, -): Promise> { +interface ShellConfig { + shell: string; + args: string[]; + commandTransport?: "argv" | "stdin"; +} + +function isLegacyWslBashPath(path: string): boolean { + const normalized = path.replace(/\//g, "\\").toLowerCase(); + return /^[a-z]:\\windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized); +} + +function getBashShellConfig(shell: string): ShellConfig { + return isLegacyWslBashPath(shell) ? { shell, args: ["-s"], commandTransport: "stdin" } : { shell, args: ["-c"] }; +} + +async function getShellConfig(customShellPath?: string): Promise> { if (customShellPath) { if (await pathExists(customShellPath)) { - return ok({ shell: customShellPath, args: ["-c"] }); + return ok(getBashShellConfig(customShellPath)); } return err(new ExecutionError("shell_unavailable", `Custom shell path not found: ${customShellPath}`)); } @@ -161,22 +174,22 @@ async function getShellConfig( if (programFilesX86) candidates.push(`${programFilesX86}\\Git\\bin\\bash.exe`); for (const candidate of candidates) { if (await pathExists(candidate)) { - return ok({ shell: candidate, args: ["-c"] }); + return ok(getBashShellConfig(candidate)); } } const bashOnPath = await findBashOnPath(); if (bashOnPath) { - return ok({ shell: bashOnPath, args: ["-c"] }); + return ok(getBashShellConfig(bashOnPath)); } return err(new ExecutionError("shell_unavailable", "No bash shell found")); } if (await pathExists("/bin/bash")) { - return ok({ shell: "/bin/bash", args: ["-c"] }); + return ok(getBashShellConfig("/bin/bash")); } const bashOnPath = await findBashOnPath(); if (bashOnPath) { - return ok({ shell: bashOnPath, args: ["-c"] }); + return ok(getBashShellConfig(bashOnPath)); } return ok({ shell: "sh", args: ["-c"] }); } @@ -274,13 +287,22 @@ export class NodeExecutionEnv implements ExecutionEnv { }; try { - child = spawn(shellConfig.value.shell, [...shellConfig.value.args, command], { - cwd, - detached: process.platform !== "win32", - env: getShellEnv(this.shellEnv, options?.env), - stdio: ["ignore", "pipe", "pipe"], - windowsHide: true, - }); + const commandFromStdin = shellConfig.value.commandTransport === "stdin"; + child = spawn( + shellConfig.value.shell, + commandFromStdin ? shellConfig.value.args : [...shellConfig.value.args, command], + { + cwd, + detached: process.platform !== "win32", + env: getShellEnv(this.shellEnv, options?.env), + stdio: [commandFromStdin ? "pipe" : "ignore", "pipe", "pipe"], + windowsHide: true, + }, + ); + if (commandFromStdin) { + child.stdin?.on("error", () => {}); + child.stdin?.end(command); + } } catch (error) { const cause = toError(error); settle(err(new ExecutionError("spawn_error", cause.message, cause))); diff --git a/packages/agent/test/harness/nodejs-env.test.ts b/packages/agent/test/harness/nodejs-env.test.ts index 758d5f59..d2d33a6f 100644 --- a/packages/agent/test/harness/nodejs-env.test.ts +++ b/packages/agent/test/harness/nodejs-env.test.ts @@ -1,5 +1,5 @@ import { access, chmod, realpath, symlink } from "node:fs/promises"; -import { join } from "node:path"; +import { delimiter, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { NodeExecutionEnv } from "../../src/harness/env/nodejs.ts"; import { FileError, getOrThrow } from "../../src/harness/types.ts"; @@ -201,6 +201,39 @@ describe("NodeExecutionEnv", () => { expect(result).toEqual({ stdout: `${await realpath(root)}:ok`, stderr: "", exitCode: 0 }); }); + it("uses stdin command transport for legacy WSL bash paths", async () => { + 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(shellPath, '#!/bin/sh\nprintf \'args:%s\\n\' "$*" >&2\nexec /bin/bash "$@"\n')); + await chmod(join(root, shellPath), 0o755); + + const originalCwd = process.cwd(); + const originalPath = process.env.PATH; + const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + try { + process.chdir(root); + process.env.PATH = `${root}${delimiter}${originalPath ?? ""}`; + Object.defineProperty(process, "platform", { + configurable: true, + value: "win32", + }); + + const wslEnv = new NodeExecutionEnv({ cwd: root, shellPath }); + const nameExpansion = "$" + "{name}"; + const result = getOrThrow(await wslEnv.exec(`name='World'; echo "Hello, ${nameExpansion}!"`)); + + expect(result).toEqual({ stdout: "Hello, World!\n", stderr: "args:-s\n", exitCode: 0 }); + } finally { + process.chdir(originalCwd); + process.env.PATH = originalPath; + if (platformDescriptor) { + Object.defineProperty(process, "platform", platformDescriptor); + } + } + }); + it("streams stdout and stderr chunks", async () => { const root = createTempDir(); const env = new NodeExecutionEnv({ cwd: root }); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d1245f20..c5aae0f9 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- Fixed bash commands through legacy WSL `bash.exe` to pass scripts over stdin so shell variables expand in the target bash ([#5893](https://github.com/earendil-works/pi/issues/5893)). - Fixed `/model` to hide GitHub Copilot models that are unavailable to the authenticated account ([#5897](https://github.com/earendil-works/pi/issues/5897)). - Fixed `/model` selector search to rank exact provider-prefixed matches before proxy-provider model ID matches ([#5892](https://github.com/earendil-works/pi/issues/5892)). diff --git a/packages/coding-agent/src/core/resolve-config-value.ts b/packages/coding-agent/src/core/resolve-config-value.ts index 9d47cff3..6d75b001 100644 --- a/packages/coding-agent/src/core/resolve-config-value.ts +++ b/packages/coding-agent/src/core/resolve-config-value.ts @@ -152,11 +152,13 @@ export function resolveConfigValue(config: string, env?: Record) function executeWithConfiguredShell(command: string): { executed: boolean; value: string | undefined } { try { - const { shell, args } = getShellConfig(); - const result = spawnSync(shell, [...args, command], { + const { shell, args, commandTransport } = getShellConfig(); + const commandFromStdin = commandTransport === "stdin"; + const result = spawnSync(shell, commandFromStdin ? args : [...args, command], { encoding: "utf-8", + input: commandFromStdin ? command : undefined, timeout: 10000, - stdio: ["ignore", "pipe", "ignore"], + stdio: [commandFromStdin ? "pipe" : "ignore", "pipe", "ignore"], shell: false, windowsHide: true, }); diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index e56291bb..da6934e7 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -66,7 +66,7 @@ export interface BashOperations { export function createLocalBashOperations(options?: { shellPath?: string }): BashOperations { return { exec: async (command, cwd, { onData, signal, timeout, env }) => { - const { shell, args } = getShellConfig(options?.shellPath); + const shellConfig = getShellConfig(options?.shellPath); try { await fsAccess(cwd, constants.F_OK); } catch { @@ -76,13 +76,18 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas throw new Error("aborted"); } - const child = spawn(shell, [...args, command], { + const commandFromStdin = shellConfig.commandTransport === "stdin"; + const child = spawn(shellConfig.shell, commandFromStdin ? shellConfig.args : [...shellConfig.args, command], { cwd, detached: process.platform !== "win32", env: env ?? getShellEnv(), - stdio: ["ignore", "pipe", "pipe"], + stdio: [commandFromStdin ? "pipe" : "ignore", "pipe", "pipe"], windowsHide: true, }); + if (commandFromStdin) { + child.stdin?.on("error", () => {}); + child.stdin?.end(command); + } if (child.pid) trackDetachedChildPid(child.pid); let timedOut = false; let timeoutHandle: NodeJS.Timeout | undefined; diff --git a/packages/coding-agent/src/utils/shell.ts b/packages/coding-agent/src/utils/shell.ts index 817ba4f9..2cafa595 100644 --- a/packages/coding-agent/src/utils/shell.ts +++ b/packages/coding-agent/src/utils/shell.ts @@ -6,11 +6,21 @@ import { getBinDir } from "../config.ts"; export interface ShellConfig { shell: string; args: string[]; + commandTransport?: "argv" | "stdin"; } /** * Find bash executable on PATH (cross-platform) */ +function isLegacyWslBashPath(path: string): boolean { + const normalized = path.replace(/\//g, "\\").toLowerCase(); + return /^[a-z]:\\windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized); +} + +function getBashShellConfig(shell: string): ShellConfig { + return isLegacyWslBashPath(shell) ? { shell, args: ["-s"], commandTransport: "stdin" } : { shell, args: ["-c"] }; +} + function findBashOnPath(): string | null { if (process.platform === "win32") { // Windows: Use 'where' and verify file exists (where can return non-existent paths) @@ -58,7 +68,7 @@ export function getShellConfig(customShellPath?: string): ShellConfig { // 1. Check user-specified shell path if (customShellPath) { if (existsSync(customShellPath)) { - return { shell: customShellPath, args: ["-c"] }; + return getBashShellConfig(customShellPath); } throw new Error(`Custom shell path not found: ${customShellPath}`); } @@ -77,14 +87,14 @@ export function getShellConfig(customShellPath?: string): ShellConfig { for (const path of paths) { if (existsSync(path)) { - return { shell: path, args: ["-c"] }; + return getBashShellConfig(path); } } // 3. Fallback: search bash.exe on PATH (Cygwin, MSYS2, WSL, etc.) const bashOnPath = findBashOnPath(); if (bashOnPath) { - return { shell: bashOnPath, args: ["-c"] }; + return getBashShellConfig(bashOnPath); } throw new Error( @@ -98,12 +108,12 @@ export function getShellConfig(customShellPath?: string): ShellConfig { // Unix: try /bin/bash, then bash on PATH, then fallback to sh if (existsSync("/bin/bash")) { - return { shell: "/bin/bash", args: ["-c"] }; + return getBashShellConfig("/bin/bash"); } const bashOnPath = findBashOnPath(); if (bashOnPath) { - return { shell: bashOnPath, args: ["-c"] }; + return getBashShellConfig(bashOnPath); } return { shell: "sh", args: ["-c"] }; diff --git a/packages/coding-agent/test/auth-storage.test.ts b/packages/coding-agent/test/auth-storage.test.ts index bcc353c1..651f9a17 100644 --- a/packages/coding-agent/test/auth-storage.test.ts +++ b/packages/coding-agent/test/auth-storage.test.ts @@ -5,7 +5,8 @@ import { registerOAuthProvider } from "@earendil-works/pi-ai/oauth"; import lockfile from "proper-lockfile"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { AuthStorage } from "../src/core/auth-storage.ts"; -import { clearConfigValueCache } from "../src/core/resolve-config-value.ts"; +import { clearConfigValueCache, resolveConfigValueUncached } from "../src/core/resolve-config-value.ts"; +import * as shellModule from "../src/utils/shell.ts"; describe("AuthStorage", () => { let tempDir: string; @@ -321,6 +322,30 @@ describe("AuthStorage", () => { expect(apiKey).toBe("hello-world"); }); + test("command config uses stdin when configured shell requires it", () => { + if (process.platform === "win32") return; + const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + vi.spyOn(shellModule, "getShellConfig").mockReturnValue({ + shell: "/bin/bash", + args: ["-s"], + commandTransport: "stdin", + }); + + try { + Object.defineProperty(process, "platform", { + configurable: true, + value: "win32", + }); + const nameExpansion = "$" + "{name}"; + + expect(resolveConfigValueUncached(`!name='World'; echo "Hello, ${nameExpansion}!"`)).toBe("Hello, World!"); + } finally { + if (platformDescriptor) { + Object.defineProperty(process, "platform", platformDescriptor); + } + } + }); + describe("caching", () => { test("command is only executed once per process", async () => { // Use a command that writes to a file to count invocations diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index f187da62..b06590f7 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -536,6 +536,56 @@ describe("Coding Agent Tools", () => { expect(getShellConfigSpy).toHaveBeenCalledWith("/custom/bash"); }); + it("should send commands over stdin when shell resolution requires it", async () => { + vi.spyOn(shellModule, "getShellConfig").mockReturnValue({ + shell: process.execPath, + args: [ + "-e", + 'let input = ""; process.stdin.setEncoding("utf8"); process.stdin.on("data", (chunk) => { input += chunk; }); process.stdin.on("end", () => { process.stdout.write(input); });', + ], + commandTransport: "stdin", + }); + const chunks: Buffer[] = []; + const ops = createLocalBashOperations({ shellPath: "C:\\Windows\\System32\\bash.exe" }); + const nameExpansion = "$" + "{name}"; + const countExpansion = "$" + "{count}"; + const iExpansion = "$" + "{i}"; + const command = `name='World'; echo "Hello, ${nameExpansion}!"; count=3; for i in $(seq 1 ${countExpansion}); do echo "Iteration ${iExpansion} of ${countExpansion}"; done`; + + const result = await ops.exec(command, testDir, { + onData: (data) => chunks.push(data), + }); + + expect(result.exitCode).toBe(0); + expect(Buffer.concat(chunks).toString("utf-8")).toBe(command); + }); + + it("should resolve legacy WSL bash.exe to stdin command transport", () => { + if (process.platform === "win32") return; + const originalCwd = process.cwd(); + const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + const shellPath = "C:\\Windows\\System32\\bash.exe"; + writeFileSync(join(testDir, shellPath), ""); + try { + process.chdir(testDir); + Object.defineProperty(process, "platform", { + configurable: true, + value: "win32", + }); + + expect(shellModule.getShellConfig(shellPath)).toEqual({ + shell: shellPath, + args: ["-s"], + commandTransport: "stdin", + }); + } finally { + process.chdir(originalCwd); + if (platformDescriptor) { + Object.defineProperty(process, "platform", platformDescriptor); + } + } + }); + it("should prepend command prefix when configured", async () => { const bashWithPrefix = createBashTool(testDir, { commandPrefix: "export TEST_VAR=hello", diff --git a/scripts/repro-5893-wsl-bash.mjs b/scripts/repro-5893-wsl-bash.mjs new file mode 100644 index 00000000..b5048de7 --- /dev/null +++ b/scripts/repro-5893-wsl-bash.mjs @@ -0,0 +1,55 @@ +#!/usr/bin/env node +import { existsSync } from "node:fs"; +import { createBashTool } from "../packages/coding-agent/src/core/tools/bash.ts"; + +const shellPath = "C:\\Windows\\System32\\bash.exe"; +const nameExpansion = "$" + "{name}"; +const countExpansion = "$" + "{count}"; +const iExpansion = "$" + "{i}"; + +function getTextOutput(result) { + return result.content + .filter((content) => content.type === "text") + .map((content) => content.text ?? "") + .join("\n"); +} + +async function runCase(label, command, expectedOutput) { + const tool = createBashTool(process.cwd(), { shellPath }); + const result = await tool.execute(label, { command }); + const output = getTextOutput(result).trimEnd(); + if (output !== expectedOutput) { + throw new Error( + [ + `${label} failed`, + "Expected:", + expectedOutput, + "Actual:", + output, + ].join("\n"), + ); + } + console.log(output); +} + +if (process.platform !== "win32") { + throw new Error("This repro must run from Windows PowerShell/CMD, not macOS/Linux or inside WSL."); +} + +if (!existsSync(shellPath)) { + throw new Error(`WSL bash launcher not found at ${shellPath}. Install/enable WSL first.`); +} + +await runCase( + "issue-5893-simple-variable", + `name='World'; echo "Hello, ${nameExpansion}!"`, + "Hello, World!", +); + +await runCase( + "issue-5893-loop-variable", + `count=3; for i in $(seq 1 ${countExpansion}); do echo "Iteration ${iExpansion} of ${countExpansion}"; done`, + "Iteration 1 of 3\nIteration 2 of 3\nIteration 3 of 3", +); + +console.log("issue #5893 WSL bash repro passed");