fix(actions): make copy-to-clipboard resilient - #242
Merged
Conversation
Extract clipboard logic into actions/clipboard.go and harden it: - Bias OSC 52 first when SSH_TTY/SSH_CONNECTION is set; native tools would otherwise target the SSH server's clipboard, not the user's. - Bound each native attempt with a 2s timeout and 1s WaitDelay; treat exec.ErrWaitDelay as success since xclip/xsel daemonize and hold the inherited stdin pipe (golang/go#13155). - Gate wl-copy on a live Wayland socket via XDG_RUNTIME_DIR/WAYLAND_DISPLAY. - Wrap OSC 52 for tmux (\$TMUX) and GNU screen (\$STY+screen TERM) using the go-osc52 library so it works under tmux set-clipboard on / screen DCS pass-through. - Skip OSC 52 when the payload exceeds 64 KB (most terminals silently drop oversized sequences) and surface a clear toast pointing to 'w' (save to file). - Emit OSC 52 to stderr instead of stdout so the escape cannot race with the BubbleTea inline renderer. - Restrict the macOS ladder to pbcopy (no Linux tools). - Add unit tests covering the candidate ladder, SSH bias, OSC 52 length cap, tmux/screen wrapping, and WaitDelay handling.
There was a problem hiding this comment.
Pull request overview
Hardens the Actions TUI “c” (copy task log) clipboard behavior across different terminal/session environments by centralizing clipboard logic, adding environment-aware backend selection (native vs OSC 52), and adding unit tests.
Changes:
- Extract clipboard handling into
actions/clipboard.gowith OSC 52 (tmux/screen wrapping), native-tool ladder, and per-attempt timeouts. - Update the TUI copy toast messaging in
actions/view.goto reflect the backend used and handle oversized OSC 52 payloads. - Add
actions/clipboard_test.goto cover backend ordering, environment gating, OSC 52 wrapping/limits, andexec.ErrWaitDelayhandling; addgo-osc52dependency.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Adds github.com/aymanbagabas/go-osc52/v2 dependency for OSC 52 emission/wrapping. |
| go.sum | Records checksums for the new go-osc52 dependency. |
| actions/view.go | Routes copy behavior through the new clipboard module and expands user-facing toast messages. |
| actions/clipboard.go | New environment-aware clipboard implementation (native tools + OSC 52) with timeouts and test seams. |
| actions/clipboard_test.go | New unit tests for clipboard backend selection, OSC 52 behavior, and timeout/WaitDelay behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Tighten isWaylandLive: skip wl-copy when XDG_RUNTIME_DIR is unset rather than attempting a doomed 2 s invocation. - Thread the resolved binary path from lookPath into exec.CommandContext to eliminate the duplicate PATH search and TOCTOU window. - For oversized payloads OSC 52 now copies the last 64 KB instead of refusing — the tail is where task-log errors usually live, and the full log is still one keypress away via 'w'. Truncation respects UTF-8 rune boundaries. - Toast surfaces truncation explicitly when OSC 52 was used.
There was a problem hiding this comment.
Pull request overview
Hardens the Actions TUI “copy task log” (c) behavior by extracting clipboard support into a dedicated module, adding environment-aware backend selection (native tools vs OSC 52), and bounding external invocations to prevent hangs across common terminal/session setups.
Changes:
- Added
actions/clipboard.goimplementing native clipboard + OSC 52 backends with SSH/tmux/screen/Wayland-aware selection and per-attempt timeouts. - Updated
actions/view.goto use the new clipboard API and show backend-specific copy toasts. - Added
actions/clipboard_test.gocovering backend selection, OSC 52 wrapping, truncation, andexec.ErrWaitDelayhandling; addedgo-osc52dependency.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Adds github.com/aymanbagabas/go-osc52/v2 dependency for OSC 52 emission + tmux/screen wrapping. |
go.sum |
Records checksums for the new OSC 52 dependency. |
actions/view.go |
Switches copy handling to new copyToClipboard API and improves user-facing toast messaging by backend. |
actions/clipboard.go |
New clipboard implementation with native tool ladder, OSC 52 fallback, SSH bias, and bounded exec behavior. |
actions/clipboard_test.go |
New unit tests for OSC 52 emission/wrapping, candidate selection, SSH ordering, truncation, and WaitDelay behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Decode the OSC 52 payload in the tail-truncation test rather than doing a base64 substring match. The substring check only worked because the kept portion happened to start on a 3-byte block boundary; with different sizes it would silently flake. - Reconstruct the rune-boundary test so the naive cut lands on a UTF-8 continuation byte (asymmetric padding around a single "é"), and assert the decoded length to prove the cut walked forward.
firecow
approved these changes
May 7, 2026
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.
Summary
Hardens the
c(copy task log) keybinding in the actions TUI so it works reliably across Wayland, X11, macOS, SSH sessions, tmux, and GNU screen — and never hangs.Changes
actions/view.gointo a dedicatedactions/clipboard.gowith test seams.SSH_TTY/SSH_CONNECTIONis set, try OSC 52 first. Localxclip/pbcopywould otherwise target the server's clipboard, not the user's.WaitDelay(1 s): bounds every native invocation.exec.ErrWaitDelayis treated as success sincexclip/xseldaemonize and hold the inherited stdin pipe (os/exec: CombinedOutput hangs when exec'd task forks a "daemon" golang/go#13155).wl-copyis only attempted whenWAYLAND_DISPLAYresolves to a real socket underXDG_RUNTIME_DIR.github.com/aymanbagabas/go-osc52/v2so OSC 52 reaches the outer terminal under tmuxset-clipboard onand screen DCS pass-through.pbcopyonly.WaitDelayhandling.New dependency
github.com/aymanbagabas/go-osc52/v2 v2.0.1— pure Go, MIT, used as a transitive dep bycharmbracelet/log. Handles the OSC 52 sequence + tmux/screen DCS wrapping; keeps that complexity out of this repo.