fix: harden tmux clipboard delivery and polling (v0.4.2)#153
Merged
Conversation
- pin set-clipboard on so app copies land in a tmux paste buffer the poll can read (modern 'external' default forwards OSC 52 but stores no buffer), making browser/iOS delivery deterministic (#1) - run the 750ms buffer poll via async Bun.spawn instead of spawnSync so it no longer blocks the event loop / stalls terminal streaming (#2) - arm the clipboard watch only on a real left-button drag, not bare taps, to avoid clobbering the clipboard with unrelated buffer changes during the armed window (#3) - tiebreak same-second buffers by tmux's monotonic buffer index instead of size when picking the latest (#5) Adds regression tests for set-clipboard setup and tap-vs-drag arming.
- revert over-tight drag-only arming: it dropped double/triple-click selections (button-0, no motion bit) which are real copies. Arm on any left-button event again, but gate offers on a new created-time correlation so taps still can't clobber the clipboard (#1) - only offer buffers created at/after the arming gesture, preventing a pre-existing or cross-window buffer from clobbering the clipboard and closing the async-baseline race (#3/#4) - re-validate socket/session freshness after each async tmux read before sending, so a stale/wrong-session offer can't be delivered (#2) - parse the monotonic order only from tmux's 'buffer<N>' names so an explicitly named buffer ending in digits can't outrank a newer one (#5) - readTmuxCapture: ignore stderr (no undrained pipe) and add a kill-timer so a hung tmux can't wedge clipboardPollInFlight forever (#6/#7) - make the server-global set-clipboard opt-out via AGENTBOARD_TMUX_SET_CLIPBOARD=0, documented in README (#8/global side effect) Tests: click-only selection arms+offers; pre-gesture buffer not offered.
Two independent reviewers flagged DO-NOT-SHIP races in the async refactor, all rooted in the lack of a generation token across watch stop/restart: - a stale baseline read could write into a later watch (after a session switch/re-attach), watermarking and suppressing the real copy (high) - an in-flight poll's finally could clear clipboardPollInFlight for a newer watch, allowing overlapping reads + stale state mutation (medium) - the async baseline could watermark a copy the user made before it resolved, silently dropping that offer (medium) Fix: stopClipboardBufferWatch bumps ws.data.clipboardWatchGen; the baseline read and pollClipboardBuffer capture the generation at launch and bail (incl. the finally guard) if it no longer matches. The baseline also skips when the watch is already armed, so it can't watermark the user's own fresh copy. Adds a regression test: a poll closure from a superseded generation is inert while the current-generation poll still delivers the offer.
…ound 3) Round-2 reviewers found the 1s slack was the sole cause of a residual edge: the baseline skips when already armed, so a pre-existing buffer created within the slack window before the gesture could be offered as the user's copy. The slack was unnecessary — handleTerminalInputPersistent arms BEFORE forwarding the input to tmux, so any buffer the gesture creates is stamped at/after the arm second. With slack=0 the poll's created-correlation now rejects every pre-existing buffer from an earlier second, shrinking the edge to an irreducible same-second + gesture-within-ms-of-attach case. No legitimate copy is missed (created >= armSec always holds). Test copy-buffer timestamps widened for CI timing headroom.
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
1 participant
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
Hardens the tmux paste-buffer → browser clipboard path (the relay that serves agent copies to the browser, especially iOS Safari). Originated from an audit of the clipboard machinery added in #151; findings were verified with independent reviewers before and after each change.
Changes
set-clipboard onso an app's OSC 52 copy actually lands in a paste buffer the poll can read (the modernexternaldefault forwards OSC 52 outward but stores no buffer). Server-global, so it's opt-out viaAGENTBOARD_TMUX_SET_CLIPBOARD=0(documented in README).Bun.spawninstead ofspawnSync, with a kill-timer so a hung tmux can't wedge the poll, andstderr: 'ignore'to avoid an undrained pipe.buffer<N>index, not size.Review
Audited, then independently reviewed across three rounds (codex + workflow reviewers). Round 1 caught generation races (fixed); round 2 confirmed them fixed and caught a slack edge (fixed); the residual is an irreducible same-second + sub-attach-window case that fails safe. One efficiency item (coalescing per-connection polls) is deferred as a documented known-limitation.
Verification
bun run lint && bun run typecheck && bun run test— green (lint 0, typecheck clean, 912 pass / 0 fail), including 4 dedicated clipboard tests.🤖 Generated with Claude Code