fix(coding-agent): run legacy WSL bash commands via stdin

closes #5893
This commit is contained in:
Vegard Stikbakke
2026-06-19 15:05:16 +02:00
parent 6e6ce70caf
commit 1287b69fe0
10 changed files with 235 additions and 28 deletions

View File

@@ -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)).

View File

@@ -152,11 +152,13 @@ export function resolveConfigValue(config: string, env?: Record<string, string>)
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,
});

View File

@@ -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;

View File

@@ -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"] };

View File

@@ -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

View File

@@ -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",