mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-11 15:39:04 +02:00
Garry TanandClaude Fable 5
7171f10364
Replace the Windows tasklist shell-out in isProcessAlive with process.kill(pid, 0), unifying all platforms on the POSIX idiom. Node maps signal-0 to an OpenProcess existence check on Windows — and the Windows daemon runs under Node (dist/server-node.mjs + bun-polyfill, the documented oven-sh/bun#4253 fallback) — so the probe is portable. Why the shell-out had to go, beyond the cosmetic conhost flash the watchdog blinked into the foreground every 60s (#1952): a Bun.spawnSync that hits its timeout still RETURNS with partial stdout, so the `.includes()` PID match answered "dead" for LIVE processes under load — the false-negative half of the #2414/#2295 leak chain. Signal 0 spawns nothing, cannot time out, and is ~5 orders of magnitude faster (measurements in #2414). EPERM still reports alive (process exists, we just can't signal it). Layered on the post-#2414-absorb shape: test 3 in process-liveness-windows.test.ts now asserts the probe is subprocess-free on ANY platform (win32 exemption dropped), test 4's static tripwire loses its error-handling.ts exemption (a `tasklist … PID eq` existence probe anywhere in src/ now fails CI), and windows-spawn-hide.test.ts drops its tasklist-in-error-handling needle (nothing spawns, which is stronger than hiding the window). Tests: process-liveness-windows + windows-spawn-hide + error-handling — 17 pass, 0 fail. Fixes #1952. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
73 lines
2.9 KiB
TypeScript
73 lines
2.9 KiB
TypeScript
/**
|
|
* Shared error-handling utilities for browse server and CLI.
|
|
*
|
|
* Each wrapper uses selective catches (checks err.code) to avoid masking
|
|
* unexpected errors. Empty catches would be flagged by slop-scan.
|
|
*/
|
|
|
|
import * as fs from 'fs';
|
|
|
|
// ─── Filesystem ────────────────────────────────────────────────
|
|
|
|
/** Remove a file, ignoring ENOENT (already gone). Rethrows other errors. */
|
|
export function safeUnlink(filePath: string): void {
|
|
try {
|
|
fs.unlinkSync(filePath);
|
|
} catch (err: any) {
|
|
if (err?.code !== 'ENOENT') throw err;
|
|
}
|
|
}
|
|
|
|
/** Remove a file, ignoring ALL errors. Use only in best-effort cleanup (shutdown, emergency). */
|
|
export function safeUnlinkQuiet(filePath: string): void {
|
|
try { fs.unlinkSync(filePath); } catch {}
|
|
}
|
|
|
|
// ─── Process ───────────────────────────────────────────────────
|
|
|
|
/** Send a signal to a process, ignoring ESRCH (already dead). Rethrows other errors. */
|
|
export function safeKill(pid: number, signal: NodeJS.Signals | number): void {
|
|
try {
|
|
process.kill(pid, signal);
|
|
} catch (err: any) {
|
|
if (err?.code !== 'ESRCH') throw err;
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Check if a PID is alive. Pure boolean probe — never throws.
|
|
*
|
|
* Signal 0 on EVERY platform (#1952). Node maps `process.kill(pid, 0)` to an
|
|
* OpenProcess existence check on Windows — and on Windows the browse daemon
|
|
* runs under Node (dist/server-node.mjs + bun-polyfill, the documented
|
|
* fallback for oven-sh/bun#4253) — so the POSIX idiom is portable here.
|
|
*
|
|
* Windows used to shell out to `tasklist /FI "PID eq <pid>"` and
|
|
* string-match the CSV. That was wrong in two ways, both hit in production:
|
|
*
|
|
* 1. FALSE NEGATIVES UNDER LOAD (#2414/#2295): tasklist takes ~700-1700ms
|
|
* on an idle box and far longer under memory pressure. A Bun.spawnSync
|
|
* that hits its `timeout` still RETURNS, carrying partial stdout — so
|
|
* the `.includes()` match came back false and a LIVE process was
|
|
* reported dead. Callers that validate liveness before killing
|
|
* (killAgentByRecord, the terminal-agent watchdog) then skipped the
|
|
* kill and respawned around the survivor — one leaked terminal-agent
|
|
* per tick, self-reinforcing (each orphan slows the next tasklist).
|
|
* 2. A console window per probe (#1952): the watchdog blinked a conhost
|
|
* window into the foreground every 60s for the whole session.
|
|
*
|
|
* Signal 0 spawns nothing, cannot time out, and is orders of magnitude
|
|
* faster (~0.004ms vs ~270ms measured in #2414).
|
|
*
|
|
* EPERM means the process EXISTS but we lack rights to signal it. That is
|
|
* alive — returning false there would reintroduce failure mode 1.
|
|
*/
|
|
export function isProcessAlive(pid: number): boolean {
|
|
try {
|
|
process.kill(pid, 0);
|
|
return true;
|
|
} catch (err: any) {
|
|
return err?.code === 'EPERM';
|
|
}
|
|
}
|