From c42edd80552712f797202656ba9c8bccc63c5ecc Mon Sep 17 00:00:00 2001 From: ethernet Date: Mon, 11 May 2026 19:00:17 -0400 Subject: [PATCH 1/2] fix(tui): clipboard copy on linux/wayland MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- ui-tui/packages/hermes-ink/src/ink/ink.tsx | 13 +- .../packages/hermes-ink/src/ink/termio/osc.ts | 39 +++-- .../src/utils/execFileNoThrow.test.ts | 146 ++++++++++++++++++ .../hermes-ink/src/utils/execFileNoThrow.ts | 75 +++++++-- ui-tui/src/app/slash/commands/core.ts | 2 +- 5 files changed, 238 insertions(+), 37 deletions(-) create mode 100644 ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.test.ts 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' ) } } From f7441f9c42254bdf1712e99bbe2cc15b0f825d16 Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 20 May 2026 10:37:06 -0400 Subject: [PATCH 2/2] fix(nix): add xclip and wl-copy --- nix/hermes-agent.nix | 9 +++++++++ 1 file changed, 9 insertions(+) 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;