Synced from GitHub
fix(web): paste terminal text once and clear the shortcut on release#19366
A terminal shortcut can send text twice when its clipboard read resolves before Chromium’s native paste event. Keep one gesture’s sent text so the matching native duplicate is consumed, while native-first paste still cancels a late read.
End the gesture on shortcut or modifier release and blur. This also handles macOS omitting V keyup while Command is held, so a later identical menu paste still works. Native fallback after a denied read and paste encoding are preserved.
Fixes #13346. Maintainer takeover of @t3dotgg’s #19366, retaining the original commits and @r4iju’s earlier design contribution.
Verification:
- Existing surface suite: 77 → 81 passing. Replaced one combined test with five focused parameterized cases covering both arrival orders and cleanup after V release, blur, and Meta release without V release.
- Clipboard-first test fails with base production code; Meta-release test fails on the original PR and passes after cleanup.
- Web typecheck, targeted lint, formatting and diff checks passed.
- Independent Claude Opus 5.5 and Codex GPT-6 Sol whole-PR reviews: one clean round; optional cosmetic suggestions left out.
- Headless Linux Chromium, actual app terminal and PTY: controlled clipboard-read-first / native-paste event order echoes the text twice on the actual base and once on head. A separately simulated Mac modifier sequence accepts the later identical menu paste. Test fonts are explicitly loaded for both captures.
- This controls event order with synthetic keyboard/clipboard events; it does not verify native OS clipboard timing. Real macOS, Windows, iOS, Android and Electron desktop were not verified.
Before (actual base, two sends):
Before: one paste appears twice in the terminal
After (one send):
After: one paste appears once in the terminal
Before interaction recording · After interaction recording
Implemented with Codex GPT-6.1 Sol; regression tests with Codex GPT-6 Astra; reviewed with Claude Opus 5.5 and Codex GPT-6 Sol through T3 Code.
Checks
23 successful · 10 skipped
Added 1 commit
- fix(web): terminal paste no longer lands twice in Chromium4f9cb7a
Synced from GitHub
Approvability
Verdict: Approved at b4e75c1
Macroscope's review found this PR approvable — This is a narrowly scoped terminal paste bug fix that prevents duplicate delivery while preserving fallback and encoding behavior. The production change is localized and covered by focused tests, with no schema, infrastructure, sensitive-area, product-default, or static-analysis configuration impact.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Team
- Run ID:
d507a749-2683-4730-a42c-64ffa68568da
📥 Commits
Reviewing files that changed from the base of the PR and between 4f9cb7a76b0ac198bcc9d46804730a51c8c5f2c3 and b4e75c1e63ae94083d6afcde77b95bb1987f7b50.
📒 Files selected for processing: 2
apps/web/src/terminal/ghostty/surface.test.tsapps/web/src/terminal/ghostty/surface.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
📝 Walkthrough
The terminal surface now tracks text delivered by a clipboard-read paste shortcut and suppresses a matching native paste from the same gesture. Tests cover both event arrival orders and verify that key release or blur clears the gesture.
Changes: Terminal paste deduplication
Layer / File(s) Summary Track and deduplicate shortcut paste
apps/web/src/terminal/ghostty/surface.ts,apps/web/src/terminal/ghostty/surface.test.tsThe surface records clipboard-read text for the active shortcut, clears the gesture on key release or blur, and drops a matching native paste. Tests cover both event orders and confirm that a later native paste is accepted after gesture cleanup. Priority: ➖ Normal
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers:
stienswout
Merge Risk: ⚪ Minimal · up to b4e75
The change stops terminal text from being pasted twice when the clipboard read finishes before the native paste event. It ends the paste gesture on key release or blur so a later identical paste still works. No concrete merge-blocking risk was found. Real-browser timing after key release is untested, but it is not shown to cause a problem.
Pre-merge checks: Passed 4
Check name Status Explanation Linked Issues check Passed Issue #13346requires deduplication when the asynchronous clipboard read arrives before the native paste.surface.tsrecords the sent clipboard text and suppresses one matching native paste. The cod…Out of Scope Changes check Passed The changes are limited to paste gesture handling in apps/web/src/terminal/ghostty/surface.tsand related regression tests insurface.test.ts. These changes directly support issue#13346. No unrel…Title check Passed The title clearly and concisely describes the main change: preventing duplicate terminal paste delivery and clearing the shortcut state on release. Description check Passed The description explains the problem, implementation, scope, linked issues, verification results, limitations, and test evidence. It does not use the template headings explicitly or include a direct a… Full details: Linked Issues check
Explanation
Issue
#13346requires deduplication when the asynchronous clipboard read arrives before the native paste.surface.tsrecords the sent clipboard text and suppresses one matching native paste. The code preserves native-first fallback and clears the gesture on key release, modifier release, and blur.surface.test.tscovers both arrival orders and later identical pastes after cleanup.
Full details: Out of Scope Changes check
Explanation
The changes are limited to paste gesture handling in
apps/web/src/terminal/ghostty/surface.tsand related regression tests insurface.test.ts. These changes directly support issue#13346. No unrelated change is shown.
Full details: Description check
Explanation
The description explains the problem, implementation, scope, linked issues, verification results, limitations, and test evidence. It does not use the template headings explicitly or include a direct approval comment, but it provides the required information in a mostly complete form.
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Synced from GitHub
Thread transfer impact
✅ Thread transfer remains within every enforced ceiling.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | 5.0 KiB | 5.0 KiB | +7 B (+0.1%) | 6.8 KiB | ✅ |
| Codex | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Codex | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | +7 B (+0.6%) | 2.0 KiB | ✅ |
| Codex | Live turn WebSocket decoded | 20.9 KiB | 20.9 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Codex | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
| Claude | Total thread wire | 5.0 KiB | 5.0 KiB | −7 B (−0.1%) | 6.8 KiB | ✅ |
| Claude | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Claude | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | −7 B (−0.6%) | 2.0 KiB | ✅ |
| Claude | Live turn WebSocket decoded | 21.2 KiB | 21.2 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Claude | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
Baseline: 40d43a1 · PR result: 4f9cb7a · Source CI: success
Scenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 108.5 KiB
- Claude decoded thread snapshot: 108.8 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
Taking over this one. I’ll keep your commits and push the maintainer rebuild on this branch.