mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-01 07:01:41 +00:00
c42edd8055
`probeLinuxCopy` and `copyNative` in `osc.ts` await `execFileNoThrow` for wl-copy / xclip / xsel. Those tools double-fork a daemon that holds the system selection live, and the daemon inherits stdio pipes from `spawn(stdio: 'pipe')`. Node's 'close' event only fires when stdio is fully closed → the daemon keeps the pipes open → 'close' never fires → the await leaks past the timeout (kill(SIGTERM) on an already-exited child is a no-op, daemon survives). Result: `linuxCopy` cache stays `undefined` permanently, the actual copy never runs, ctrl-c silently does nothing on wayland/x11. Reproduced in isolation, confirmed across wl-copy and a daemonization-shaped fixture. Fix: add `resolveOnExit` option to `execFileNoThrow`. When set, the promise settles on the immediate child's 'exit' event instead of waiting for stdio drainage. Wired into both the probe and the actual copy spawns for every clipboard tool (pbcopy, wl-copy, xclip, xsel, clip). Tests: 5 new vitest cases covering daemon-style child handling, non-zero exit propagation, timeout behavior, and double-resolve guard. The forever-hang case is committed as `it.skip` with documentation so a reviewer can verify the bug by hand.
115 lines
3.4 KiB
TypeScript
115 lines
3.4 KiB
TypeScript
import { spawn } from 'child_process'
|
|
type ExecFileOptions = {
|
|
input?: string
|
|
timeout?: number
|
|
useCwd?: boolean
|
|
env?: NodeJS.ProcessEnv
|
|
/** Resolve as soon as the child *exits*, instead of waiting for its
|
|
* stdio streams to close. Use this for tools that fork a daemon and
|
|
* let the daemon inherit the parent's stdio (e.g. `wl-copy`): the
|
|
* child exits immediately, but `'close'` never fires because the
|
|
* daemon holds the pipes open.
|
|
*
|
|
* When true, stdout and stderr are set to 'ignore' to prevent the
|
|
* daemon from inheriting those pipe FDs — the caller must not
|
|
* depend on collecting stdout/stderr content. Both will always be
|
|
* empty strings in this mode. */
|
|
resolveOnExit?: boolean
|
|
}
|
|
|
|
export function execFileNoThrow(
|
|
file: string,
|
|
args: string[],
|
|
options: ExecFileOptions = {}
|
|
): Promise<{
|
|
stdout: string
|
|
stderr: string
|
|
code: number
|
|
error?: string
|
|
}> {
|
|
return new Promise(resolve => {
|
|
// When resolveOnExit is true, ignore stdout/stderr so the daemon
|
|
// doesn't inherit those pipe FDs — prevents handle leaks that can
|
|
// keep the parent process alive. No output data is collected in
|
|
// this mode; both stdout and stderr will be empty strings.
|
|
const stdioConfig = options.resolveOnExit
|
|
? ['pipe', 'ignore', 'ignore'] as const
|
|
: 'pipe' as const
|
|
|
|
const child = spawn(file, args, {
|
|
cwd: options.useCwd ? process.cwd() : undefined,
|
|
env: options.env,
|
|
stdio: stdioConfig
|
|
})
|
|
|
|
let stdout = ''
|
|
let stderr = ''
|
|
let timedOut = false
|
|
let settled = false
|
|
|
|
const settle = (code: number, error?: string) => {
|
|
if (settled) {
|
|
return
|
|
}
|
|
|
|
settled = true
|
|
|
|
if (timer) {
|
|
clearTimeout(timer)
|
|
}
|
|
|
|
// Destroy any remaining streams to release FDs promptly.
|
|
// After settle(), nobody reads from these anymore.
|
|
child.stdout?.destroy()
|
|
child.stderr?.destroy()
|
|
|
|
resolve({ stdout, stderr, code, ...(error ? { error } : {}) })
|
|
}
|
|
|
|
const timer = options.timeout
|
|
? setTimeout(() => {
|
|
timedOut = true
|
|
child.kill('SIGTERM')
|
|
|
|
// When resolving on exit, SIGTERM-ing a child that has already
|
|
// exited is a no-op and `'exit'` won't fire again — settle here
|
|
// so the promise doesn't leak. Safe under settled-guard.
|
|
if (options.resolveOnExit) {
|
|
settle(124)
|
|
}
|
|
}, options.timeout)
|
|
: null
|
|
|
|
child.stdout?.on('data', chunk => {
|
|
stdout += String(chunk)
|
|
})
|
|
child.stderr?.on('data', chunk => {
|
|
stderr += String(chunk)
|
|
})
|
|
child.on('error', error => {
|
|
settle(1, String(error))
|
|
})
|
|
|
|
if (options.resolveOnExit) {
|
|
// 'exit' fires when the child process itself exits — even if the
|
|
// daemon it forked still holds the inherited stdio pipes open.
|
|
// When a signal kills the child, code is null — map that to 1
|
|
// so callers don't mistake a signal-terminated run for success.
|
|
child.on('exit', (code, signal) => {
|
|
const exitCode = timedOut ? 124 : (code ?? (signal ? 1 : 0))
|
|
settle(exitCode)
|
|
})
|
|
} else {
|
|
child.on('close', (code, signal) => {
|
|
const exitCode = timedOut ? 124 : (code ?? (signal ? 1 : 0))
|
|
settle(exitCode)
|
|
})
|
|
}
|
|
|
|
if (options.input) {
|
|
child.stdin?.write(options.input)
|
|
}
|
|
|
|
child.stdin?.end()
|
|
})
|
|
}
|