mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-06-30 19:00:57 +08:00
fix(windows): prefer PowerShell over bash to prevent zombie process accumulation (#2346)
* fix(windows): prefer PowerShell over bash to prevent zombie process accumulation On Windows, ECC hook scripts were spawning bash.exe (MSYS2/Git Bash) on every tool use via findShellBinary(). These processes were not reaped by Windows, causing 40+ zombie bash.exe/conhost.exe processes per session with noticeable system lag. Changes to scripts/hooks/plugin-hook-bootstrap.js: - Add isPowerShellBin(bin) helper: basename-based detection so full paths like C:\Windows\...\powershell.exe are handled correctly - findShellBinary(): check BASH env var first (preserves escape hatch), then on win32 probe pwsh.exe -> powershell.exe -> bash.exe -> bash; use correct probe args per shell type; cache result in _cachedShell - findBashBinary(): separate cached bash-only finder used by spawnShell .sh fallback; skips PowerShell binaries even if BASH points to one - spawnShell(): use isPowerShellBin() to select -NoProfile -NonInteractive -File args for PowerShell; .sh scripts fall back to findBashBinary() with a skip-warning if no bash found on Windows observe-runner.js is intentionally unchanged: it always invokes observe.sh which is bash-only; routing it through PowerShell would silently break it. The observe.sh -> observe.js migration is tracked separately. Fixes #2345 * fix(windows): address CodeRabbit and Greptile review comments - Add timeout: 30000 to all spawnSync probe calls in findShellBinary and findBashBinary to prevent hangs on broken/stalled shell candidates - Add -ExecutionPolicy Bypass to PowerShell -File invocation to fix execution on machines with the default Restricted policy (Win10/11) - Add PowerShell availability skip guard to PS selection test (mirrors existing bash skip guard) - Fix no-bash test to keep PowerShell on PATH so the .sh fallback branch is actually exercised rather than hitting shell-unavailable early exit * test: add timeout to spawnSync probes in Windows test skip guards --------- Co-authored-by: Christopher J Diamond <diamondcj@leidos.com>
This commit is contained in:
parent
88f6894267
commit
64797fd895
@ -58,28 +58,73 @@ function resolveTarget(rootDir, relPath) {
|
||||
return resolvedTarget;
|
||||
}
|
||||
|
||||
let _cachedShell = undefined;
|
||||
let _cachedBash = undefined;
|
||||
|
||||
function isPowerShellBin(bin) {
|
||||
const base = path.basename(bin).toLowerCase();
|
||||
return base === 'pwsh.exe' || base === 'pwsh' || base === 'powershell.exe' || base === 'powershell';
|
||||
}
|
||||
|
||||
function findShellBinary() {
|
||||
if (_cachedShell !== undefined) return _cachedShell;
|
||||
|
||||
const candidates = [];
|
||||
|
||||
// Explicit override always wins — check before any platform probing.
|
||||
// Warning: setting BASH to a bash binary on Windows bypasses the PowerShell
|
||||
// preference and may reintroduce bash.exe zombie accumulation.
|
||||
if (process.env.BASH && process.env.BASH.trim()) {
|
||||
candidates.push(process.env.BASH.trim());
|
||||
}
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
candidates.push('bash.exe', 'bash');
|
||||
// Prefer PowerShell on Windows — it is native and does not leave zombie
|
||||
// bash.exe / conhost.exe processes the way MSYS2/Git Bash does.
|
||||
// Note: PowerShell is only suitable for .ps1 scripts; callers that need
|
||||
// to run .sh scripts (e.g. observe-runner.js) must not use this function.
|
||||
candidates.push('pwsh.exe', 'powershell.exe', 'bash.exe', 'bash');
|
||||
} else {
|
||||
candidates.push('bash', 'sh');
|
||||
}
|
||||
|
||||
const psProbeArgs = ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'];
|
||||
const shProbeArgs = ['-c', ':'];
|
||||
|
||||
for (const candidate of candidates) {
|
||||
const probe = spawnSync(candidate, ['-c', ':'], {
|
||||
const probe = spawnSync(candidate, isPowerShellBin(candidate) ? psProbeArgs : shProbeArgs, {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true,
|
||||
timeout: 30000,
|
||||
});
|
||||
if (!probe.error) {
|
||||
return candidate;
|
||||
_cachedShell = candidate;
|
||||
return _cachedShell;
|
||||
}
|
||||
}
|
||||
|
||||
_cachedShell = null;
|
||||
return null;
|
||||
}
|
||||
|
||||
function findBashBinary() {
|
||||
if (_cachedBash !== undefined) return _cachedBash;
|
||||
|
||||
const candidates = [];
|
||||
if (process.env.BASH && process.env.BASH.trim() && !isPowerShellBin(process.env.BASH.trim())) {
|
||||
candidates.push(process.env.BASH.trim());
|
||||
}
|
||||
candidates.push('bash.exe', 'bash');
|
||||
|
||||
for (const candidate of candidates) {
|
||||
const probe = spawnSync(candidate, ['-c', ':'], { stdio: 'ignore', windowsHide: true, timeout: 30000 });
|
||||
if (!probe.error) {
|
||||
_cachedBash = candidate;
|
||||
return _cachedBash;
|
||||
}
|
||||
}
|
||||
|
||||
_cachedBash = null;
|
||||
return null;
|
||||
}
|
||||
|
||||
@ -100,6 +145,10 @@ function spawnNode(rootDir, relPath, raw, args) {
|
||||
});
|
||||
}
|
||||
|
||||
// spawnShell is not used by any hook in the shipped hooks.json configuration
|
||||
// (all hooks use 'node' mode). It is provided for third-party plugins that
|
||||
// register shell-backed hooks. Plugins should supply .ps1 scripts on Windows
|
||||
// and .sh scripts on Unix; mixing them will produce a skip with a stderr warning.
|
||||
function spawnShell(rootDir, relPath, raw, args) {
|
||||
const shell = findShellBinary();
|
||||
if (!shell) {
|
||||
@ -116,7 +165,37 @@ function spawnShell(rootDir, relPath, raw, args) {
|
||||
CLAUDE_PLUGIN_ROOT: rootDir,
|
||||
ECC_PLUGIN_ROOT: rootDir,
|
||||
};
|
||||
return spawnSync(shell, [resolveTarget(rootDir, relPath), ...args], {
|
||||
const scriptPath = resolveTarget(rootDir, relPath);
|
||||
const isPs = isPowerShellBin(shell);
|
||||
|
||||
// PowerShell cannot interpret bash scripts — fall back to a bash candidate
|
||||
// rather than silently failing the hook.
|
||||
if (isPs && scriptPath.endsWith('.sh')) {
|
||||
const bash = findBashBinary();
|
||||
if (!bash) {
|
||||
return {
|
||||
status: 0,
|
||||
stdout: '',
|
||||
stderr: '[Hook] .sh script requested but no bash binary found on Windows; skipping\n',
|
||||
};
|
||||
}
|
||||
return spawnSync(bash, [scriptPath, ...args], {
|
||||
input: raw,
|
||||
encoding: 'utf8',
|
||||
env: hookEnv,
|
||||
cwd: process.cwd(),
|
||||
timeout: 30000,
|
||||
windowsHide: true,
|
||||
});
|
||||
}
|
||||
|
||||
const shellArgs = isPs
|
||||
// -ExecutionPolicy Bypass: default Windows policy (Restricted) blocks -File
|
||||
// execution of .ps1 scripts; Bypass scopes only to this child process.
|
||||
? ['-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-File', scriptPath, ...args]
|
||||
: [scriptPath, ...args];
|
||||
|
||||
return spawnSync(shell, shellArgs, {
|
||||
input: raw,
|
||||
encoding: 'utf8',
|
||||
env: hookEnv,
|
||||
|
||||
@ -292,6 +292,101 @@ process.exit(7);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// Windows-only: PowerShell preference and .sh fallback behaviour.
|
||||
if (process.platform === 'win32') {
|
||||
if (test('shell mode selects PowerShell when BASH is unset on Windows', () => {
|
||||
// Skip if no PowerShell is available.
|
||||
const psProbe = spawnSync('pwsh.exe', ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'], { stdio: 'ignore', timeout: 5000 });
|
||||
const ps = psProbe.error
|
||||
? spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'], { stdio: 'ignore', timeout: 5000 }).error
|
||||
? null : 'powershell.exe'
|
||||
: 'pwsh.exe';
|
||||
if (!ps) {
|
||||
console.log(' SKIP: no PowerShell found');
|
||||
return;
|
||||
}
|
||||
|
||||
const root = createTempDir();
|
||||
try {
|
||||
// UTF8 encoding set explicitly — PowerShell 5.1 defaults to UTF-16LE.
|
||||
writeFile(root, path.join('scripts', 'hook.ps1'), [
|
||||
'[Console]::OutputEncoding = [System.Text.Encoding]::UTF8',
|
||||
'$OutputEncoding = [System.Text.Encoding]::UTF8',
|
||||
'$input_data = [Console]::In.ReadToEnd()',
|
||||
'Write-Host -NoNewline ("ps1:" + $args[0] + ":" + $input_data)',
|
||||
].join('\n'));
|
||||
|
||||
const result = run(['shell', path.join('scripts', 'hook.ps1'), 'arg'], {
|
||||
root,
|
||||
input: 'payload',
|
||||
env: { BASH: '' },
|
||||
});
|
||||
|
||||
assert.strictEqual(result.status, 0, result.stderr);
|
||||
assert.strictEqual(result.stdout, 'ps1:arg:payload');
|
||||
} finally {
|
||||
cleanup(root);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('shell mode falls back to bash for .sh scripts when PowerShell is the resolved shell', () => {
|
||||
// Skip if no bash is available (headless CI without Git for Windows).
|
||||
const bashProbe = spawnSync('bash.exe', ['-c', ':'], { stdio: 'ignore', timeout: 5000 });
|
||||
if (bashProbe.error) {
|
||||
console.log(' SKIP: bash.exe not found');
|
||||
return;
|
||||
}
|
||||
|
||||
const root = createTempDir();
|
||||
try {
|
||||
writeFile(root, path.join('scripts', 'hook.sh'), [
|
||||
'input=$(cat)',
|
||||
'printf "sh:%s:%s" "$1" "$input"',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
// Clear BASH so PowerShell is resolved first, but script is .sh.
|
||||
const result = run(['shell', path.join('scripts', 'hook.sh'), 'arg'], {
|
||||
root,
|
||||
input: 'payload',
|
||||
env: { BASH: '' },
|
||||
});
|
||||
|
||||
assert.strictEqual(result.status, 0, result.stderr);
|
||||
assert.strictEqual(result.stdout, 'sh:arg:payload');
|
||||
} finally {
|
||||
cleanup(root);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('shell mode emits skip warning for .sh script when no bash found on Windows', () => {
|
||||
const root = createTempDir();
|
||||
try {
|
||||
writeFile(root, path.join('scripts', 'hook.sh'), 'printf unreachable\n');
|
||||
|
||||
// Keep PowerShell on PATH so it is resolved as the shell, then strip
|
||||
// bash candidates so the .sh fallback path hits the skip-warning branch.
|
||||
const result = run(['shell', path.join('scripts', 'hook.sh')], {
|
||||
root,
|
||||
input: 'raw-input',
|
||||
env: { BASH: '', PATH: process.env.SystemRoot
|
||||
? `${process.env.SystemRoot}\\System32\\WindowsPowerShell\\v1.0;${process.env.SystemRoot}\\System32`
|
||||
: '' },
|
||||
});
|
||||
|
||||
assert.strictEqual(result.status, 0);
|
||||
assert.strictEqual(result.stdout, 'raw-input');
|
||||
assert.ok(
|
||||
result.stderr.includes('no bash binary found') ||
|
||||
result.stderr.includes('shell runtime unavailable'),
|
||||
`unexpected stderr: ${result.stderr}`
|
||||
);
|
||||
} finally {
|
||||
cleanup(root);
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
}
|
||||
|
||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user