On Windows, `spawn()` with `detached: true` sets the CREATE_NEW_PROCESS_GROUP flag in CreateProcess. In certain terminal contexts — notably VSCode's integrated terminal (ConPTY), Windows Terminal, and some MSYS2/Git Bash configurations — this flag conflicts with the parent process group hierarchy and causes a synchronous EINVAL from libuv, making *every* bash/async_bash/bg_shell command fail immediately with `spawn EINVAL`. The bg-shell extension already guards against this with `detached: process.platform !== "win32"` (process-manager.ts:109), but three other spawn sites were missed: - `packages/pi-coding-agent/src/core/tools/bash.ts` (bash tool) - `packages/pi-coding-agent/src/core/bash-executor.ts` (RPC executor) - `src/resources/extensions/async-jobs/async-bash-tool.ts` (async_bash) This commit aligns all spawn sites with the bg-shell pattern. Additionally fixes two related issues: 1. `killProcessTree()` in shell.ts used `detached: true` on its own `taskkill` spawn call — unnecessary and potentially problematic in the same terminal contexts. Removed. 2. `killTree()` in async-bash-tool.ts used Unix-only `process.kill(-pid)` with no Windows fallback. On Windows, negative PIDs (process group kill) are not supported, so orphaned child processes could survive timeout kills. Now uses `taskkill /F /T` on Windows, matching the bg-shell and shell.ts implementations. Includes a regression test that statically verifies no spawn site uses unconditional `detached: true`, plus a smoke test confirming the platform-guarded pattern works on all platforms. Reproduction: Run GSD v2.42-v2.51 inside VSCode on Windows 11 with Git Bash as the shell. Any bash tool call fails with `spawn EINVAL`. The error is 100% reproducible and affects all shell operations (bash, async_bash, bg_shell start). Co-authored-by: Matt Haynes <matt@auroraventures.io> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
212 lines
6.4 KiB
TypeScript
212 lines
6.4 KiB
TypeScript
import { existsSync } from "node:fs";
|
|
import { delimiter } from "node:path";
|
|
import { spawn, spawnSync } from "child_process";
|
|
import { getBinDir, getSettingsPath } from "../config.js";
|
|
import { SettingsManager } from "../core/settings-manager.js";
|
|
|
|
let cachedShellConfig: { shell: string; args: string[] } | null = null;
|
|
|
|
/**
|
|
* Find bash executable on PATH (cross-platform)
|
|
*/
|
|
function findBashOnPath(): string | null {
|
|
if (process.platform === "win32") {
|
|
// Windows: Use 'where' and verify file exists (where can return non-existent paths)
|
|
try {
|
|
const result = spawnSync("where", ["bash.exe"], { encoding: "utf-8", timeout: 5000 });
|
|
if (result.status === 0 && result.stdout) {
|
|
const firstMatch = result.stdout.trim().split(/\r?\n/)[0];
|
|
if (firstMatch && existsSync(firstMatch)) {
|
|
return firstMatch;
|
|
}
|
|
}
|
|
} catch {
|
|
// Ignore errors
|
|
}
|
|
return null;
|
|
}
|
|
|
|
// Unix: Use 'which' and trust its output (handles Termux and special filesystems)
|
|
try {
|
|
const result = spawnSync("which", ["bash"], { encoding: "utf-8", timeout: 5000 });
|
|
if (result.status === 0 && result.stdout) {
|
|
const firstMatch = result.stdout.trim().split(/\r?\n/)[0];
|
|
if (firstMatch) {
|
|
return firstMatch;
|
|
}
|
|
}
|
|
} catch {
|
|
// Ignore errors
|
|
}
|
|
return null;
|
|
}
|
|
|
|
/**
|
|
* Get shell configuration based on platform.
|
|
* Resolution order:
|
|
* 1. User-specified shellPath in settings.json
|
|
* 2. On Windows: Git Bash in known locations, then bash on PATH
|
|
* 3. On Unix: /bin/bash, then bash on PATH, then fallback to sh
|
|
*/
|
|
export function getShellConfig(): { shell: string; args: string[] } {
|
|
if (cachedShellConfig) {
|
|
return cachedShellConfig;
|
|
}
|
|
|
|
const settings = SettingsManager.create();
|
|
const customShellPath = settings.getShellPath();
|
|
|
|
// 1. Check user-specified shell path
|
|
if (customShellPath) {
|
|
if (existsSync(customShellPath)) {
|
|
cachedShellConfig = { shell: customShellPath, args: ["-c"] };
|
|
return cachedShellConfig;
|
|
}
|
|
throw new Error(
|
|
`Custom shell path not found: ${customShellPath}\nPlease update shellPath in ${getSettingsPath()}`,
|
|
);
|
|
}
|
|
|
|
if (process.platform === "win32") {
|
|
// 2. Try Git Bash in known locations
|
|
const paths: string[] = [];
|
|
const programFiles = process.env.ProgramFiles;
|
|
if (programFiles) {
|
|
paths.push(`${programFiles}\\Git\\bin\\bash.exe`);
|
|
}
|
|
const programFilesX86 = process.env["ProgramFiles(x86)"];
|
|
if (programFilesX86) {
|
|
paths.push(`${programFilesX86}\\Git\\bin\\bash.exe`);
|
|
}
|
|
|
|
for (const path of paths) {
|
|
if (existsSync(path)) {
|
|
cachedShellConfig = { shell: path, args: ["-c"] };
|
|
return cachedShellConfig;
|
|
}
|
|
}
|
|
|
|
// 3. Fallback: search bash.exe on PATH (Cygwin, MSYS2, WSL, etc.)
|
|
const bashOnPath = findBashOnPath();
|
|
if (bashOnPath) {
|
|
cachedShellConfig = { shell: bashOnPath, args: ["-c"] };
|
|
return cachedShellConfig;
|
|
}
|
|
|
|
throw new Error(
|
|
`No bash shell found. Options:\n` +
|
|
` 1. Install Git for Windows: https://git-scm.com/download/win\n` +
|
|
` 2. Add your bash to PATH (Cygwin, MSYS2, etc.)\n` +
|
|
` 3. Set shellPath in ${getSettingsPath()}\n\n` +
|
|
`Searched Git Bash in:\n${paths.map((p) => ` ${p}`).join("\n")}`,
|
|
);
|
|
}
|
|
|
|
// Unix: try /bin/bash, then bash on PATH, then fallback to sh
|
|
if (existsSync("/bin/bash")) {
|
|
cachedShellConfig = { shell: "/bin/bash", args: ["-c"] };
|
|
return cachedShellConfig;
|
|
}
|
|
|
|
const bashOnPath = findBashOnPath();
|
|
if (bashOnPath) {
|
|
cachedShellConfig = { shell: bashOnPath, args: ["-c"] };
|
|
return cachedShellConfig;
|
|
}
|
|
|
|
cachedShellConfig = { shell: "sh", args: ["-c"] };
|
|
return cachedShellConfig;
|
|
}
|
|
|
|
/**
|
|
* On Windows + Git Bash, rewrite Windows-style NUL redirects to /dev/null.
|
|
* Git Bash doesn't recognize NUL as a device name and creates a literal file
|
|
* that is undeletable due to NUL being a reserved Windows device name.
|
|
* No-op on non-Windows platforms.
|
|
*/
|
|
export function sanitizeCommand(command: string): string {
|
|
if (process.platform !== "win32") return command;
|
|
return command.replace(/(\d*>>?) *\bNUL\b(?=\s|;|\||&|\)|$)/gi, "$1 /dev/null");
|
|
}
|
|
|
|
export function getShellEnv(): NodeJS.ProcessEnv {
|
|
const binDir = getBinDir();
|
|
const pathKey = Object.keys(process.env).find((key) => key.toLowerCase() === "path") ?? "PATH";
|
|
const currentPath = process.env[pathKey] ?? "";
|
|
const pathEntries = currentPath.split(delimiter).filter(Boolean);
|
|
const hasBinDir = pathEntries.includes(binDir);
|
|
const updatedPath = hasBinDir ? currentPath : [binDir, currentPath].filter(Boolean).join(delimiter);
|
|
|
|
return {
|
|
...process.env,
|
|
[pathKey]: updatedPath,
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Sanitize binary output for display/storage.
|
|
* Removes characters that crash string-width or cause display issues:
|
|
* - Control characters (except tab, newline, carriage return)
|
|
* - Lone surrogates
|
|
* - Unicode Format characters (crash string-width due to a bug)
|
|
* - Characters with undefined code points
|
|
*/
|
|
export function sanitizeBinaryOutput(str: string): string {
|
|
// Use Array.from to properly iterate over code points (not code units)
|
|
// This handles surrogate pairs correctly and catches edge cases where
|
|
// codePointAt() might return undefined
|
|
return Array.from(str)
|
|
.filter((char) => {
|
|
// Filter out characters that cause string-width to crash
|
|
// This includes:
|
|
// - Unicode format characters
|
|
// - Lone surrogates (already filtered by Array.from)
|
|
// - Control chars except \t \n \r
|
|
// - Characters with undefined code points
|
|
|
|
const code = char.codePointAt(0);
|
|
|
|
// Skip if code point is undefined (edge case with invalid strings)
|
|
if (code === undefined) return false;
|
|
|
|
// Allow tab, newline, carriage return
|
|
if (code === 0x09 || code === 0x0a || code === 0x0d) return true;
|
|
|
|
// Filter out control characters (0x00-0x1F, except 0x09, 0x0a, 0x0x0d)
|
|
if (code <= 0x1f) return false;
|
|
|
|
// Filter out Unicode format characters
|
|
if (code >= 0xfff9 && code <= 0xfffb) return false;
|
|
|
|
return true;
|
|
})
|
|
.join("");
|
|
}
|
|
|
|
/**
|
|
* Kill a process and all its children (cross-platform)
|
|
*/
|
|
export function killProcessTree(pid: number): void {
|
|
if (process.platform === "win32") {
|
|
// Use taskkill on Windows to kill process tree
|
|
try {
|
|
spawn("taskkill", ["/F", "/T", "/PID", String(pid)], {
|
|
stdio: "ignore",
|
|
});
|
|
} catch {
|
|
// Ignore errors if taskkill fails
|
|
}
|
|
} else {
|
|
// Use SIGKILL on Unix/Linux/Mac
|
|
try {
|
|
process.kill(-pid, "SIGKILL");
|
|
} catch {
|
|
// Fallback to killing just the child if process group kill fails
|
|
try {
|
|
process.kill(pid, "SIGKILL");
|
|
} catch {
|
|
// Process already dead
|
|
}
|
|
}
|
|
}
|
|
}
|