diff --git a/nix/hermes-agent.nix b/nix/hermes-agent.nix index 6c391878cc..f373c25bcd 100644 --- a/nix/hermes-agent.nix +++ b/nix/hermes-agent.nix @@ -16,6 +16,11 @@ openssh, ffmpeg, tirith, + + # linux-only deps + wl-clipboard, + xclip, + # Flake inputs — passed explicitly by packages.nix and overlays.nix uv2nix, pyproject-nix, @@ -68,6 +73,10 @@ let openssh ffmpeg tirith + ] + ++ lib.optionals stdenv.isLinux [ + wl-clipboard + xclip ]; runtimePath = lib.makeBinPath runtimeDeps; diff --git a/ui-tui/packages/hermes-ink/src/ink/ink.tsx b/ui-tui/packages/hermes-ink/src/ink/ink.tsx index 49fdf70448..5723cdd84e 100644 --- a/ui-tui/packages/hermes-ink/src/ink/ink.tsx +++ b/ui-tui/packages/hermes-ink/src/ink/ink.tsx @@ -1473,16 +1473,9 @@ export default class Ink { if (success) { return text } - - if (process.env.HERMES_TUI_DEBUG_CLIPBOARD) { - console.error( - '[clipboard] no path reached the clipboard (headless + no tmux?) — set HERMES_TUI_FORCE_OSC52=1 to force the escape sequence' - ) - } - } catch (err) { - if (process.env.HERMES_TUI_DEBUG_CLIPBOARD) { - console.error('[clipboard] error:', err) - } + } catch { + // Clipboard failed across every path — caller sees the empty + // return below and surfaces a hint via the slash command. } } diff --git a/ui-tui/packages/hermes-ink/src/ink/termio/osc.ts b/ui-tui/packages/hermes-ink/src/ink/termio/osc.ts index 3f680b6dec..c3322bcfaa 100644 --- a/ui-tui/packages/hermes-ink/src/ink/termio/osc.ts +++ b/ui-tui/packages/hermes-ink/src/ink/termio/osc.ts @@ -308,9 +308,24 @@ export async function setClipboard(text: string): Promise { // Cached after first attempt so repeated mouse-ups skip the probe chain. let linuxCopy: 'wl-copy' | 'xclip' | 'xsel' | null | undefined +/** Per-tool copy arguments: wl-copy reads stdin, xclip/xsel need clipboard flags. */ +function linuxCopyArgs(tool: 'wl-copy' | 'xclip' | 'xsel'): string[] { + switch (tool) { + case 'wl-copy': + return [] + case 'xclip': + return ['-selection', 'clipboard'] + case 'xsel': + return ['--clipboard', '--input'] + } +} + /** Internal: probe once and cache — wl-copy first, then xclip, then xsel. */ async function probeLinuxCopy(): Promise<'wl-copy' | 'xclip' | 'xsel' | null> { - const opts = { useCwd: false, timeout: 500 } + // resolveOnExit: wl-copy daemonizes and the daemon inherits stdio pipes, + // so 'close' never fires and the await would hang past the timeout. + // 'exit' fires on the immediate child's exit — what we actually care about. + const opts = { useCwd: false, timeout: 500, resolveOnExit: true } const r = await execFileNoThrow('wl-copy', [], opts) @@ -318,13 +333,13 @@ async function probeLinuxCopy(): Promise<'wl-copy' | 'xclip' | 'xsel' | null> { return 'wl-copy' } - const r2 = await execFileNoThrow('xclip', ['-selection', 'clipboard'], opts) + const r2 = await execFileNoThrow('xclip', linuxCopyArgs('xclip'), opts) if (r2.code === 0) { return 'xclip' } - const r3 = await execFileNoThrow('xsel', ['--clipboard', '--input'], opts) + const r3 = await execFileNoThrow('xsel', linuxCopyArgs('xsel'), opts) return r3.code === 0 ? 'xsel' : null } @@ -347,7 +362,11 @@ async function probeLinuxCopy(): Promise<'wl-copy' | 'xclip' | 'xsel' | null> { * we skip probing entirely and treat linuxCopy as permanently null. */ function copyNative(text: string): boolean { - const opts = { input: text, useCwd: false, timeout: 2000 } + // resolveOnExit: pbcopy/wl-copy/xclip/xsel/clip all daemonize or hold + // the system selection live in a forked process. Without resolveOnExit, + // the inherited stdio pipes keep node from seeing 'close' → the + // fire-and-forget await never resolves and the actual copy never runs. + const opts = { input: text, useCwd: false, timeout: 2000, resolveOnExit: true } switch (process.platform) { case 'darwin': @@ -363,17 +382,13 @@ function copyNative(text: string): boolean { } // linuxCopy is a known-working tool; fire-and-forget. - void execFileNoThrow(linuxCopy, linuxCopy === 'wl-copy' ? [] : ['-selection', 'clipboard'], opts) + void execFileNoThrow(linuxCopy, linuxCopyArgs(linuxCopy), opts) return true } // No display server → native tools will fail immediately. Cache null. if (!process.env.DISPLAY && !process.env.WAYLAND_DISPLAY) { - if (process.env.HERMES_TUI_DEBUG_CLIPBOARD) { - console.error('[clipboard] [native] Linux: no DISPLAY or WAYLAND_DISPLAY — native clipboard unavailable') - } - linuxCopy = null return false @@ -386,13 +401,9 @@ function copyNative(text: string): boolean { const winner = await probeLinuxCopy() linuxCopy = winner - if (process.env.HERMES_TUI_DEBUG_CLIPBOARD) { - console.error(`[clipboard] [native] Linux: clipboard probe complete → ${winner ?? 'no tool available'}`) - } - // Actually perform the copy with the discovered tool. if (winner) { - void execFileNoThrow(winner, winner === 'wl-copy' ? [] : ['-selection', 'clipboard'], opts) + void execFileNoThrow(winner, linuxCopyArgs(winner), opts) } })() diff --git a/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.test.ts b/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.test.ts new file mode 100644 index 0000000000..74c06c0fb7 --- /dev/null +++ b/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.test.ts @@ -0,0 +1,146 @@ +import { chmodSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { execFileNoThrow } from './execFileNoThrow.js' + +// These tests shell out to /bin/sh, use chmodSync(0o755), and rely on +// POSIX sleep/job control. They will not work on Windows. +const onWindows = process.platform === 'win32' + +// We simulate `wl-copy`'s daemonization behavior with a tiny shell script: +// 1. Fork a short-lived background sleeper that inherits stdio (so the +// parent process's pipes can never close). +// 2. Record the sleeper PID to a file so afterEach can clean it up. +// 3. Exit immediately with status 0. +// +// Without resolveOnExit, the await on `'close'` hangs until SIGTERM at +// timeout — exactly the production wl-copy bug. With resolveOnExit, the +// promise settles on `'exit'` regardless of the inherited pipes. + +let scriptDir: string +let daemonScript: string +let sleeperPids: number[] + +/** Read the PID file the daemon script writes, and track it for afterEach cleanup. */ +function trackSleeperPid(pidFile: string): void { + try { + const pid = parseInt(readFileSync(pidFile, 'utf8').trim(), 10) + if (pid > 0) { + sleeperPids.push(pid) + } + } catch { + // PID file not written or unreadable — sleeper may have already exited. + } +} + +beforeEach(() => { + sleeperPids = [] + scriptDir = join(tmpdir(), `hermes-execfile-test-${process.pid}-${Date.now()}`) + mkdirSync(scriptDir, { recursive: true }) + daemonScript = join(scriptDir, 'fake-daemonizer.sh') + // Posix sh: the `sleep 3 &` child inherits stdin/stdout/stderr from the + // shell, which inherited them from `spawn(stdio: 'pipe')`. The shell + // exits but its child (the sleeper) keeps the pipes open. Mirrors how + // wl-copy double-forks then exits while the daemon holds the selection. + // The sleeper writes its PID to $1 so we can clean it up reliably. + writeFileSync(daemonScript, '#!/bin/sh\nsleep 3 &\necho $! > "$1"\nexit 0\n') + chmodSync(daemonScript, 0o755) +}) + +afterEach(() => { + // Kill orphaned sleepers so they don't accumulate across watch runs. + for (const pid of sleeperPids) { + try { + process.kill(pid, 'SIGKILL') + } catch { + // Already exited — fine. + } + } + rmSync(scriptDir, { recursive: true, force: true }) +}) + +describe.skipIf(onWindows)('execFileNoThrow with daemon-style children', () => { + // Skipped because the bug it documents is a forever-hang. Without + // resolveOnExit, the 'close' event doesn't fire when the immediate + // child has exited but a forked daemon still holds stdio open. Even + // SIGTERM at the timeout doesn't help — the daemon survives it. To + // verify by hand: remove `it.skip` and watch the test timeout. This + // test is here so a reviewer reading the resolveOnExit option knows + // *why* every clipboard-tool spawn in osc.ts wires it on. + it.skip("(documented hang) without resolveOnExit, await never resolves when daemon inherits stdio", async () => { + const pidFile = join(scriptDir, 'sleeper-skip.pid') + const result = await execFileNoThrow(daemonScript, [pidFile], { timeout: 300 }) + trackSleeperPid(pidFile) + + expect(result.code).toBe(124) + }) + + it("settles immediately on 'exit' when resolveOnExit is true, regardless of daemon stdio", async () => { + const pidFile = join(scriptDir, 'sleeper-exit.pid') + const start = Date.now() + + const result = await execFileNoThrow(daemonScript, [pidFile], { + timeout: 2000, + resolveOnExit: true + }) + trackSleeperPid(pidFile) + + const elapsed = Date.now() - start + + // The shell exits in a few ms. resolveOnExit lets us return on exit + // (code 0) instead of waiting for the orphaned sleeper to release + // stdio. Should be well under 200ms even on slow CI. + expect(result.code).toBe(0) + expect(elapsed).toBeLessThan(500) + }) + + it("still surfaces the right code when resolveOnExit'd child exits non-zero", async () => { + const pidFile = join(scriptDir, 'sleeper-fail.pid') + const failScript = join(scriptDir, 'fail.sh') + writeFileSync(failScript, `#!/bin/sh\nsleep 3 &\necho $! > "${pidFile}"\nexit 7\n`) + chmodSync(failScript, 0o755) + + const result = await execFileNoThrow(failScript, [], { + timeout: 2000, + resolveOnExit: true + }) + trackSleeperPid(pidFile) + + expect(result.code).toBe(7) + }) + + it('settles on timeout=124 when the child itself never exits, even with resolveOnExit', async () => { + const slowScript = join(scriptDir, 'slow.sh') + writeFileSync(slowScript, '#!/bin/sh\nsleep 30\n') + chmodSync(slowScript, 0o755) + + const result = await execFileNoThrow(slowScript, [], { + timeout: 200, + resolveOnExit: true + }) + + // Child process never exits on its own → timer fires → SIGTERM → + // child exits → 'exit' fires with non-null signal. The settle() + // call from the timer registers code=124 first. Either way: 124. + expect(result.code).toBe(124) + }) + + it('does not double-resolve when both timer and exit fire', async () => { + const pidFile = join(scriptDir, 'sleeper-race.pid') + // Race: child happens to exit right around the timeout. The settled + // guard ensures only the first resolution wins. + const result = await execFileNoThrow(daemonScript, [pidFile], { + timeout: 50, // very tight + resolveOnExit: true + }) + trackSleeperPid(pidFile) + + // Either code=0 (exit beat timer) or code=124 (timer beat exit). + // Both are valid outcomes; the contract is that the promise settles + // exactly once and doesn't throw. + expect([0, 124]).toContain(result.code) + }) +}) diff --git a/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.ts b/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.ts index 106555b13e..13780c8027 100644 --- a/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.ts +++ b/ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.ts @@ -4,6 +4,17 @@ type ExecFileOptions = { 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( @@ -17,20 +28,55 @@ export function execFileNoThrow( 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: 'pipe' + 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 @@ -41,19 +87,24 @@ export function execFileNoThrow( stderr += String(chunk) }) child.on('error', error => { - if (timer) { - clearTimeout(timer) - } - - resolve({ stdout, stderr, code: 1, error: String(error) }) + settle(1, String(error)) }) - child.on('close', code => { - if (timer) { - clearTimeout(timer) - } - resolve({ stdout, stderr, code: timedOut ? 124 : (code ?? 0) }) - }) + 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) diff --git a/ui-tui/src/app/slash/commands/core.ts b/ui-tui/src/app/slash/commands/core.ts index 85f46028f5..ae2387da61 100644 --- a/ui-tui/src/app/slash/commands/core.ts +++ b/ui-tui/src/app/slash/commands/core.ts @@ -345,7 +345,7 @@ export const coreCommands: SlashCommand[] = [ return sys(`copied ${text.length} characters`) } else { return sys( - 'clipboard copy failed — try HERMES_TUI_FORCE_OSC52=1 to force the escape sequence; HERMES_TUI_DEBUG_CLIPBOARD=1 for details' + 'clipboard copy failed — try HERMES_TUI_FORCE_OSC52=1 to force the escape sequence' ) } }