fix(tui): clipboard copy on linux/wayland#29342
Merged
Merged
Conversation
Contributor
🔎 Lint report:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes clipboard-copy silently failing in the TUI on Linux/Wayland by preventing execFileNoThrow from hanging on daemonizing clipboard tools (e.g. wl-copy) that keep inherited stdio pipes open, and wires that behavior into the native clipboard path. It also updates Nix runtime deps and cleans up now-dead debug messaging.
Changes:
- Add
resolveOnExittoexecFileNoThrowand corresponding unit tests covering daemon-style subprocess behavior and timeout races. - Use
resolveOnExit: truefor native clipboard probing/copy across platforms in OSC clipboard code; remove unused debug logging. - Add Linux clipboard tools to the Nix derivation and simplify user-facing failure messaging.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui-tui/src/app/slash/commands/core.ts | Simplifies /copy failure hint after debug path removal. |
| ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.ts | Adds resolveOnExit settling behavior to avoid hangs on daemonizing tools. |
| ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.test.ts | New tests for resolveOnExit, timeout, non-zero exit, and race behavior. |
| ui-tui/packages/hermes-ink/src/ink/termio/osc.ts | Wires resolveOnExit into native clipboard probe/copy; removes dead debug logging. |
| ui-tui/packages/hermes-ink/src/ink/ink.tsx | Removes dead debug logging; swallows clipboard exceptions with comment. |
| nix/hermes-agent.nix | Adds wl-clipboard and xclip to Linux runtime dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+30
| // Posix sh: the `sleep 30 &` 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. | ||
| writeFileSync(daemonScript, '#!/bin/sh\nsleep 30 &\nexit 0\n') | ||
| chmodSync(daemonScript, 0o755) |
Collaborator
Author
There was a problem hiding this comment.
Fixed in e17da78. Shortened to sleep 3, the daemon script now writes the sleeper PID to a file, and afterEach reads it back and SIGKILLs the orphan. No more accumulation across watch runs.
Comment on lines
392
to
394
| // Actually perform the copy with the discovered tool. | ||
| if (winner) { | ||
| void execFileNoThrow(winner, winner === 'wl-copy' ? [] : ['-selection', 'clipboard'], opts) |
Collaborator
Author
There was a problem hiding this comment.
Fixed in e17da78. Extracted a linuxCopyArgs() helper with per-tool argument sets: wl-copy: [], xclip: [-selection clipboard], xsel: [--clipboard --input]. Both call sites (cached-path and probe-winner) now use it instead of the old ternary that passed xclip args to xsel.
Comment on lines
+7
to
+14
| /** 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. The caller must not depend on | ||
| * collecting stdout/stderr from that point on — only what arrived | ||
| * before exit is included in the resolved value. */ | ||
| resolveOnExit?: boolean |
Collaborator
Author
There was a problem hiding this comment.
Fixed in e17da78. When resolveOnExit=true, stdio is now ['pipe', 'ignore', 'ignore'] so daemonizing children can't inherit stdout/stderr pipe FDs. Also added child.stdout?.destroy() / child.stderr?.destroy() after settle() to release remaining FDs promptly. Safe because resolveOnExit already documents that the caller must not depend on stdout/stderr after exit.
e17da78 to
2c5adc0
Compare
Collaborator
Author
|
@copilot review |
Comment on lines
+95
to
+104
| if (options.resolveOnExit) { | ||
| // 'exit' fires when the child process itself exits — even if the | ||
| // daemon it forked still holds the inherited stdio pipes open. | ||
| child.on('exit', code => { | ||
| settle(timedOut ? 124 : (code ?? 0)) | ||
| }) | ||
| } else { | ||
| child.on('close', code => { | ||
| settle(timedOut ? 124 : (code ?? 0)) | ||
| }) |
Comment on lines
+7
to
+17
| /** 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. Only what arrived | ||
| * before exit is included in the resolved value (typically empty | ||
| * since the child exits immediately). */ |
Comment on lines
+1
to
+6
| 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' | ||
|
|
Comment on lines
+311
to
+315
| /** 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 [] |
`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.
172d74d to
f7441f9
Compare
19 tasks
gweeteve
pushed a commit
to gweeteve/hermes-agent
that referenced
this pull request
Jun 2, 2026
…-copy fix(tui): clipboard copy on linux/wayland
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes
ctrl-c → clipboardsilently failing on linux/wayland inside the TUI.Native clipboard tools (
wl-copy,xclip,xsel,pbcopy,clip) double-fork a daemon that holds the system selection live. When we spawn them withstdio: 'pipe', the daemon inherits those stdio pipes, so node's'close'event never fires — even after the immediate child has exited.execFileNoThrowwas awaiting'close', so the await leaked past its timeout (andkill(SIGTERM)on the already-exited immediate child was a no-op while the daemon kept living). Result:probeLinuxCopynever returned, thelinuxCopycache stayedundefinedforever, and the actual copy spawn never ran. ctrl-c → nothing.Fix is small and surgical: add a
resolveOnExitoption toexecFileNoThrowthat settles on the immediate child's'exit'event instead of waiting for stdio drainage. Wire it into every clipboard-tool spawn site (probe + actual copy, across all five tools).'exit'is what we actually care about for daemonizing tools — once the immediate child is gone, the daemon has the data and the OS owns the rest.Adds clipboard deps to the Nix derivation.
Related Issue
Fixes #
Type of Change
Changes Made
nix/hermes-agent.nix— addedwl-clipboardandxclipto hermes-agent derivationui-tui/packages/hermes-ink/src/utils/execFileNoThrow.ts— addedresolveOnExitopt; when set, the returned promise settles on the child's'exit'event instead of'close'. Double-resolve guarded.ui-tui/packages/hermes-ink/src/utils/execFileNoThrow.test.ts— new file, 4 passing cases (daemon-style child, non-zero exit propagation, timeout, double-resolve) + 1it.skipdocumenting the forever-hang so a reviewer can reproduce the bug by hand.ui-tui/packages/hermes-ink/src/ink/termio/osc.ts— wiredresolveOnExit: trueintoprobeLinuxCopy(500ms timeout) andcopyNative(2000ms); dropped two deadHERMES_TUI_DEBUG_CLIPBOARDconsole.error sites that emitted into the alt-screenpatchStderrno-op.ui-tui/packages/hermes-ink/src/ink/ink.tsx— dropped two more deadHERMES_TUI_DEBUG_CLIPBOARDconsole.error sites in the OSC52 fallback path; replaced with a cleancatch {}+ comment.ui-tui/src/app/slash/commands/core.ts— dropped theHERMES_TUI_DEBUG_CLIPBOARD=1hint from theclipboard copy failedslash-command message (the env var no longer routes anywhere useful).Net diff: +165 / −32, 5 files.
How to Test
wl-copy/xclip/xselinstalled.nix run .# -- --tui(or however you launch the TUI on your box).ctrl-c.cd ui-tui && npm test -- packages/hermes-ink/src/utils/—execFileNoThrow.test.tscovers daemon-style children, timeout, non-zero exit, and double-resolve.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A