Fix Linux clipboard copy by adding OSC52 fallback#225
Open
Aditya190803 wants to merge 3 commits intodavis7dotsh:mainfrom
Open
Fix Linux clipboard copy by adding OSC52 fallback#225Aditya190803 wants to merge 3 commits intodavis7dotsh:mainfrom
Aditya190803 wants to merge 3 commits intodavis7dotsh:mainfrom
Conversation
|
@Aditya190803 is attempting to deploy a commit to the davis7dotsh Team on Vercel. A member of the Team first needs to authorize it. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds OSC52 terminal-clipboard fallback and tests, introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
89
to
100
| const xclipResult = await runClipboard(['xclip', '-selection', 'clipboard']); | ||
| if (!xclipResult) { | ||
| const xselResult = await runClipboard(['xsel', '--clipboard', '--input']); | ||
| if (!xselResult) { | ||
| throw new Error('Failed to copy to clipboard: no compatible clipboard command succeeded.'); | ||
| } | ||
| if (xselResult) return; | ||
| } | ||
|
|
||
| const osc52Result = tryOsc52Copy(text); | ||
| if (!osc52Result) { | ||
| throw new Error( | ||
| 'Failed to copy to clipboard: no compatible clipboard command succeeded and terminal OSC52 fallback is unavailable.' | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
When xclipResult is true (xclip succeeds), the code skips the inner if (!xclipResult) block but still reaches tryOsc52Copy, writing a raw OSC52 escape sequence to stdout on top of the already-completed copy. Before this PR, a successful xclip caused the function to return via implicit fall-off. The refactor dropped that early-exit path.
Suggested change
| const xclipResult = await runClipboard(['xclip', '-selection', 'clipboard']); | |
| if (!xclipResult) { | |
| const xselResult = await runClipboard(['xsel', '--clipboard', '--input']); | |
| if (!xselResult) { | |
| throw new Error('Failed to copy to clipboard: no compatible clipboard command succeeded.'); | |
| } | |
| if (xselResult) return; | |
| } | |
| const osc52Result = tryOsc52Copy(text); | |
| if (!osc52Result) { | |
| throw new Error( | |
| 'Failed to copy to clipboard: no compatible clipboard command succeeded and terminal OSC52 fallback is unavailable.' | |
| ); | |
| } | |
| const xclipResult = await runClipboard(['xclip', '-selection', 'clipboard']); | |
| if (xclipResult) return; | |
| const xselResult = await runClipboard(['xsel', '--clipboard', '--input']); | |
| if (xselResult) return; | |
| const osc52Result = tryOsc52Copy(text); | |
| if (!osc52Result) { | |
| throw new Error( | |
| 'Failed to copy to clipboard: no compatible clipboard command succeeded and terminal OSC52 fallback is unavailable.' | |
| ); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/cli/src/tui/clipboard.ts
Line: 89-100
Comment:
**xclip success falls through to OSC52**
When `xclipResult` is `true` (xclip succeeds), the code skips the inner `if (!xclipResult)` block but **still reaches `tryOsc52Copy`**, writing a raw OSC52 escape sequence to stdout on top of the already-completed copy. Before this PR, a successful `xclip` caused the function to return via implicit fall-off. The refactor dropped that early-exit path.
```suggestion
const xclipResult = await runClipboard(['xclip', '-selection', 'clipboard']);
if (xclipResult) return;
const xselResult = await runClipboard(['xsel', '--clipboard', '--input']);
if (xselResult) return;
const osc52Result = tryOsc52Copy(text);
if (!osc52Result) {
throw new Error(
'Failed to copy to clipboard: no compatible clipboard command succeeded and terminal OSC52 fallback is unavailable.'
);
}
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/src/tui/clipboard.ts (1)
64-73:⚠️ Potential issue | 🟠 MajorExit code not checked for clipboard command execution.
At lines 57, 62, and 69,
spawnprocesses are awaited but their exit codes are ignored. Line 69 is particularly problematic—runClipboardalways returnstrueregardless of command success, which prevents the OSC52 fallback at line 95 from executing when all clipboard commands fail. This results in false "copied" outcomes.Proposed fix
const runClipboard = async (command: string[]) => { try { const proc = spawn(command, { stdin: 'pipe' }); proc.stdin.write(text); proc.stdin.end(); - await proc.exited; - return true; + const exitCode = await proc.exited; + return exitCode === 0; } catch { return false; } };Also apply the same fix to the
darwinandwin32branches at lines 54–57 and 59–62 respectively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/tui/clipboard.ts` around lines 64 - 73, runClipboard currently awaits the spawned process but always returns true, so clipboard command failures are ignored; update runClipboard to inspect the process exit code/status (from the spawned proc) and return true only when the exit code is zero (success), otherwise return false so the OSC52 fallback can run. Apply the same fix to the platform-specific clipboard branches (the darwin and win32 spawn/await blocks) so they also check the process exit code/status and only treat zero as success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/btca.spec.md`:
- Around line 190-191: Update the docs to reflect the actual clipboard detection
order used in apps/cli/src/tui/clipboard.ts: on WSL the code checks for clip.exe
(via Windows paths) first, then falls back to Wayland/X11 tools (wl-copy, xclip,
xsel) and finally OSC52; change the sentence that currently lists wl-copy first
to state the WSL-first ordering (clip.exe → wl-copy → xclip → xsel → OSC52) so
it matches the implementation in the clipboard handling code (see the
copyToClipboard / clipboard command resolution logic in
apps/cli/src/tui/clipboard.ts).
In `@apps/docs/guides/cli-reference.mdx`:
- Around line 46-47: Update the Linux clipboard paragraph to include the WSL
branch: state that on WSL btca first tries Windows clipboard via clip.exe, then
proceeds to Wayland (`wl-copy`), then `xclip`, then `xsel`, and finally falls
back to terminal OSC52; ensure the text mentions `clip.exe` and OSC52 so the
order matches the implementation.
---
Outside diff comments:
In `@apps/cli/src/tui/clipboard.ts`:
- Around line 64-73: runClipboard currently awaits the spawned process but
always returns true, so clipboard command failures are ignored; update
runClipboard to inspect the process exit code/status (from the spawned proc) and
return true only when the exit code is zero (success), otherwise return false so
the OSC52 fallback can run. Apply the same fix to the platform-specific
clipboard branches (the darwin and win32 spawn/await blocks) so they also check
the process exit code/status and only treat zero as success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6c2bf23-bbb9-450c-b7fc-543564b5dff3
📒 Files selected for processing (6)
apps/cli/src/tui/clipboard.test.tsapps/cli/src/tui/clipboard.tsapps/cli/src/tui/context/messages-context.tsxapps/cli/src/tui/services.tsapps/docs/btca.spec.mdapps/docs/guides/cli-reference.mdx
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 happened
I tried using btca on Linux, and clipboard copy failed in my terminal setup.
Root cause
On Linux, clipboard copy relied on external binaries (
wl-copy,xclip,xsel). In minimal or terminal-only environments, these may be unavailable, causing copy to fail.What I changed
Errorvalues.Why this fix
OSC52 works through compatible terminals without requiring external clipboard binaries, making clipboard copy more reliable for Linux users.
Note
Fix Linux clipboard copy by adding OSC52 terminal fallback
getOsc52SequenceandtryOsc52Copyhelpers in clipboard.ts that base64-encode text and write an OSC52 escape sequence to stdout as a last-resort clipboard mechanism.copyToClipboardon Linux now trieswl-copy,xclip,xsel, and OSC52 in order before throwing; the final error message text has also changed.copyToClipboardin messages-context.tsx by coercing them toErrorinstances before propagation.TERMisdumb.Macroscope summarized 527536c.
Greptile Summary
This PR adds an OSC52 terminal escape sequence fallback for Linux clipboard copy when external binaries (
wl-copy,xclip,xsel) are unavailable, along with improvedEffect.tryPromiseerror coercion and documentation updates. The early-return control flow for xclip/xsel is correct and the previously flagged xclip fallthrough regression is resolved.Confidence Score: 5/5
Safe to merge — the OSC52 fallback logic is correct, the xclip early-return regression is fixed, and error coercion improvements are sound.
All findings are P2 style suggestions. No regressions in existing behavior were introduced; the only note is a pre-existing limitation in
runClipboardthat reduces OSC52 fallback coverage for installed-but-failing binaries.apps/cli/src/tui/clipboard.ts — the
runClipboardhelper should check exit codes to make OSC52 fallback fully effective.Important Files Changed
runClipboardstill silently succeeds on non-zero exit codes (pre-existing issue)Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix(docs): update Linux clipboard behavi..." | Re-trigger Greptile