-
Notifications
You must be signed in to change notification settings - Fork 0
fix: forward Ctrl+V to PTY for non-text clipboard paste #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
943e9ac
fix: forward Ctrl+V to PTY for non-text clipboard paste
e57324c
docs: add upstream contribution section to DECSCUSR ADR
d60e0dc
test: replace fixed sleeps with prompt/content polling
e8b3a62
fix: restore cursor.ts deleted by accidental git clean and use sentin…
c50253f
feat: implement shouldForwardPaste function to handle clipboard paste…
1e0ca1f
fix: remove shouldForwardPaste function and adjust paste handling logic
d71be20
fix: update references from "Upstreaming to ghostty-web" to "Fix to g…
eb2f463
fix: remove shouldForwardPaste tests and related imports from cursor.…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| # ADR 014: webtty — Non-text paste via Ctrl+V PTY forwarding | ||
|
|
||
| **SPEC:** [client](../specs/client.md) | ||
| **Status:** Accepted | ||
| **Date:** 2026-03-26 | ||
|
|
||
| --- | ||
|
|
||
| ## Context | ||
|
|
||
| Pasting non-text content (e.g. an image) into the webtty browser terminal silently does nothing. Text paste works fine. The same setup works correctly under ttyd (xterm.js), so the issue is specific to ghostty-web's input handling. | ||
|
|
||
| ### Root cause | ||
|
|
||
| ghostty-web's `InputHandler.handleKeyDown` unconditionally swallows Ctrl+V / Cmd+V without forwarding `\x16` to the PTY: | ||
|
|
||
| ```js | ||
| if ((A.ctrlKey || A.metaKey) && A.code === "KeyV") | ||
| return; // Ctrl+V keydown never reaches the PTY | ||
| ``` | ||
|
|
||
| It relies entirely on the browser `paste` event. xterm.js (used by ttyd) does not do this — Ctrl+V propagates to the PTY as `\x16` regardless of clipboard content. | ||
|
|
||
| For text paste this is fine: the `paste` event fires and ghostty-web's `handlePaste` extracts `text/plain` and sends it. But when the clipboard has no `text/plain` (image, file, etc.), `handlePaste` silently drops the event — the PTY sees nothing at all. | ||
|
|
||
| ### Why the Ctrl+V keydown matters | ||
|
|
||
| opencode's native TUI detects `\x16` from the PTY and calls the **native OS clipboard API** directly — it does not use the browser clipboard event: | ||
|
|
||
| - macOS: `osascript` reads the clipboard as PNG | ||
| - Windows: `powershell.exe System.Windows.Forms.Clipboard.GetImage()` | ||
| - Linux: `wl-paste` / `xclip` | ||
|
|
||
| Because ghostty-web never sends `\x16` to the PTY, opencode's handler never fires. ttyd works because xterm.js forwards the keydown. | ||
|
|
||
| ## Decision | ||
|
|
||
| Add a capture-phase `paste` listener on the terminal container in `src/client/index.ts`. When `clipboardData` has no `text/plain` — the exact condition where ghostty-web would drop the paste: | ||
|
|
||
| 1. `preventDefault()` — prevents the browser from inserting the content into the DOM | ||
| 2. `stopImmediatePropagation()` — prevents ghostty-web's `handlePaste` from running | ||
| 3. `ws.send('\x16')` — forwards Ctrl+V to the PTY so TUI apps can read the clipboard natively | ||
|
|
||
| When `text/plain` is present the listener returns immediately — ghostty-web handles it, avoiding a double-paste. | ||
|
|
||
| ### Sequence of events | ||
|
|
||
| ``` | ||
| user presses Ctrl+V with non-text content (e.g. image) in clipboard | ||
| → ghostty-web keydown: swallows Ctrl+V, \x16 never sent to PTY | ||
| → browser fires paste event | ||
| → capture-phase listener (our code): | ||
| text/plain present → return, ghostty-web handles normally | ||
| no text/plain → preventDefault + stopImmediatePropagation | ||
| ws.send('\x16') | ||
| → PTY receives \x16 | ||
| → opencode: Clipboard.read() → osascript/powershell/wl-paste → [Image 1] ✅ | ||
| ``` | ||
|
|
||
| ## Considered Options | ||
|
|
||
| ### Option A: Capture-phase listener, send `\x16` when no `text/plain` (chosen) | ||
|
|
||
| ~12 lines in `src/client/index.ts`. No server changes. Triggers exactly when ghostty-web would drop the paste — no double-paste for text, generalises to any non-text clipboard content. | ||
|
|
||
| ### Option B: Always send `\x16` on every paste | ||
|
|
||
| Causes double-paste for text: PTY receives both `\x16` and the text from ghostty-web's handler. Rejected. | ||
|
|
||
| ### Option C: Fix ghostty-web upstream (long-term) | ||
|
|
||
| The correct permanent fix. See [Fix to ghostty-web](#fix-to-ghostty-web). | ||
|
|
||
| ## Consequences | ||
|
|
||
| - Non-text paste works in opencode and any TUI that reads the OS clipboard on `\x16` | ||
| - Text paste is entirely unaffected | ||
| - TUI apps that do not handle `\x16` for clipboard are unaffected — same as baseline | ||
|
|
||
| ## Fix to ghostty-web | ||
|
|
||
| ### What the bug is | ||
|
|
||
| `InputHandler.handleKeyDown` in `lib/input-handler.ts` returns early on Ctrl+V without emitting `\x16` to `onDataCallback`. xterm.js forwards the keydown; ghostty-web does not. This breaks any TUI that handles clipboard access via `\x16` in the PTY stream. | ||
|
|
||
| ### What to change | ||
|
|
||
| Emit the encoded keydown before returning, so `\x16` reaches the PTY. The `paste` event still fires afterwards, so text paste via `handlePaste` is unaffected. | ||
|
|
||
| ```ts | ||
| // lib/input-handler.ts — InputHandler.handleKeyDown | ||
| if ((event.ctrlKey || event.metaKey) && event.code === 'KeyV') { | ||
| const encoded = this.encoder.encode({ key: Key.V, mods: Mods.CTRL, action: KeyAction.PRESS }); | ||
| if (encoded.length > 0) { | ||
| this.onDataCallback(new TextDecoder().decode(encoded)); // \x16 → PTY | ||
| } | ||
| return; // browser paste event fires next → handlePaste covers text | ||
| } | ||
| ``` | ||
|
|
||
| ### Why this is the right fix (not the webtty workaround) | ||
|
|
||
| The webtty workaround intercepts the `paste` event to infer that Ctrl+V was pressed. The ghostty-web fix sends `\x16` from the `keydown` handler where it belongs — the same place xterm.js does it. With this upstream fix, the webtty workaround can be removed entirely. | ||
|
|
||
| ### Contribution checklist | ||
|
|
||
| - [ ] Open issue: "Ctrl+V keydown not forwarded to PTY — breaks TUI clipboard handling (e.g. opencode)" | ||
| - [ ] PR: `lib/input-handler.ts` `handleKeyDown` — add `onDataCallback(\x16)` before early return | ||
| - [ ] Test: `keydown` `ctrlKey=true, code='KeyV'` → assert `onDataCallback` receives `\x16` | ||
|
|
||
| ## Related Decisions | ||
|
|
||
| - [ADR 010 — Client UX polish](010.client.ux-polish.md): established the pattern of the client handling terminal I/O edge cases |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| --- | ||
| status: accepted | ||
| date: 2026-03-26 | ||
| decision-makers: Product Engineering Team, Platform Team | ||
| consulted: Product Team, Client Application Teams | ||
| informed: Engineering Leadership, SRE/Operations Team | ||
| --- | ||
|
|
||
| # Pricing Storage for Ordering System | ||
|
|
||
| ## Context and Problem Statement | ||
|
|
||
| The Ordering System serves multiple clients that require accurate, real-time pricing information. Pricing is updated frequently by the Product team and must propagate to all clients immediately. We need a centralized pricing store that is consistent, highly available, low-latency, and scalable. | ||
|
|
||
| ## Decision Drivers | ||
|
|
||
| * Strong consistency — all clients must see the same price at all times | ||
| * Real-time propagation — changes must reach clients with no observable delay | ||
| * High availability — no single point of failure | ||
| * Low-latency reads — sub-100ms for real-time ordering workflows | ||
| * Durability and scalability as client traffic grows | ||
|
|
||
| ## Considered Options | ||
|
|
||
| * PostgreSQL with Redis Cache | ||
| * PostgreSQL Only | ||
| * Elasticsearch with PostgreSQL | ||
| * AWS DynamoDB | ||
|
|
||
| ## Decision Outcome | ||
|
|
||
| Chosen option: **PostgreSQL with Redis Cache**, because it is the only option that meets all three critical requirements together: strong consistency (PostgreSQL ACID), real-time propagation (LISTEN/NOTIFY + cache invalidation), and low-latency reads (Redis sub-10ms) — without vendor lock-in. | ||
|
|
||
| ### Consequences | ||
|
|
||
| * Good, because ACID guarantees ensure consistent, durable pricing data across all clients | ||
| * Good, because LISTEN/NOTIFY enables real-time propagation without a separate message broker | ||
| * Good, because Redis cache delivers sub-10ms reads, well within the 100ms SLA | ||
| * Bad, because two systems increase operational complexity | ||
| * Bad, because cache invalidation requires careful implementation to avoid serving stale prices | ||
|
|
||
| ### Confirmation | ||
|
|
||
| * Integration tests: write a price update, verify all client read paths return the new value within SLA | ||
| * Load tests: p99 read latency must stay below 100ms at peak traffic | ||
| * Production alert: Redis cache hit ratio below 80% or cache staleness above threshold triggers PagerDuty | ||
|
|
||
| ## Pros and Cons of the Options | ||
|
|
||
| ### PostgreSQL with Redis Cache | ||
|
|
||
| Write-through to PostgreSQL; Redis as cache. LISTEN/NOTIFY broadcasts changes; clients invalidate and re-fetch. | ||
|
|
||
| * Good, because ACID consistency and durability | ||
| * Good, because sub-10ms cache reads meet latency SLA | ||
| * Good, because LISTEN/NOTIFY propagates changes in real time without extra infrastructure | ||
| * Bad, because two systems to operate and monitor | ||
| * Bad, because cache invalidation bugs can cause stale price reads | ||
|
|
||
| ### PostgreSQL Only | ||
|
|
||
| All reads and writes go directly to PostgreSQL. | ||
|
|
||
| * Good, because simple — one system, no cache consistency issues | ||
| * Bad, because read latency (5–50ms) may breach SLA under peak load | ||
| * Bad, because becomes a read bottleneck as client count grows; read replicas introduce replication lag | ||
|
|
||
| ### Elasticsearch with PostgreSQL | ||
|
|
||
| PostgreSQL as source of truth; Elasticsearch indexed via background sync for reads. | ||
|
|
||
| * Good, because fast and scalable reads for complex queries | ||
| * Bad, because background sync means updates are not real-time — violates the propagation requirement | ||
| * Bad, because significant operational overhead for a simple key-value pricing lookup pattern | ||
|
|
||
| ### AWS DynamoDB | ||
|
|
||
| DynamoDB as primary store with Streams for change propagation. | ||
|
|
||
| * Good, because fully managed with built-in HA and auto-scaling | ||
| * Bad, because eventual consistency by default; strong consistency costs more | ||
| * Bad, because vendor lock-in and unpredictable cost at scale | ||
|
|
||
| ## More Information | ||
|
|
||
| Change propagation flow: write to PostgreSQL → invalidate Redis key → publish on `pricing_updates` LISTEN/NOTIFY channel → clients refresh from cache. | ||
|
|
||
| Key risks: cache stampede (mitigate with short TTLs + request coalescing), stale data on invalidation failure (mitigate with 5–10 min TTL safety net), Redis node loss (mitigate with Redis Cluster; cache rebuilds from PostgreSQL). | ||
|
|
||
| Revisit if the system expands to 10+ regions or pricing model complexity requires full-text search. | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { describe, expect, test } from 'bun:test'; | ||
| import { applyDecscusr } from './cursor'; | ||
|
|
||
| function makeTerm(): { options: { cursorStyle: string; cursorBlink: boolean } } { | ||
| return { options: { cursorStyle: 'block', cursorBlink: false } }; | ||
| } | ||
|
|
||
| describe('applyDecscusr', () => { | ||
| test('Ps 0 — default reset — sets block blinking', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[0 q'); | ||
| expect(term.options.cursorStyle).toBe('block'); | ||
| expect(term.options.cursorBlink).toBe(true); | ||
| }); | ||
|
|
||
| test('Ps 1 — blinking block', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[1 q'); | ||
| expect(term.options.cursorStyle).toBe('block'); | ||
| expect(term.options.cursorBlink).toBe(true); | ||
| }); | ||
|
|
||
| test('Ps 2 — steady block', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[2 q'); | ||
| expect(term.options.cursorStyle).toBe('block'); | ||
| expect(term.options.cursorBlink).toBe(false); | ||
| }); | ||
|
|
||
| test('Ps 3 — blinking underline', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[3 q'); | ||
| expect(term.options.cursorStyle).toBe('underline'); | ||
| expect(term.options.cursorBlink).toBe(true); | ||
| }); | ||
|
|
||
| test('Ps 4 — steady underline', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[4 q'); | ||
| expect(term.options.cursorStyle).toBe('underline'); | ||
| expect(term.options.cursorBlink).toBe(false); | ||
| }); | ||
|
|
||
| test('Ps 5 — blinking bar', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[5 q'); | ||
| expect(term.options.cursorStyle).toBe('bar'); | ||
| expect(term.options.cursorBlink).toBe(true); | ||
| }); | ||
|
|
||
| test('Ps 6 — steady bar', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[6 q'); | ||
| expect(term.options.cursorStyle).toBe('bar'); | ||
| expect(term.options.cursorBlink).toBe(false); | ||
| }); | ||
|
|
||
| test('empty Ps defaults to 0 (blinking block)', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[ q'); | ||
| expect(term.options.cursorStyle).toBe('block'); | ||
| expect(term.options.cursorBlink).toBe(true); | ||
| }); | ||
|
|
||
| test('out-of-range Ps is ignored', () => { | ||
| const term = makeTerm(); | ||
| term.options.cursorStyle = 'bar'; | ||
| term.options.cursorBlink = false; | ||
| applyDecscusr(term as never, '\x1b[7 q'); | ||
| expect(term.options.cursorStyle).toBe('bar'); | ||
| expect(term.options.cursorBlink).toBe(false); | ||
| }); | ||
|
|
||
| test('no DECSCUSR sequence leaves options unchanged', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, 'hello world'); | ||
| expect(term.options.cursorStyle).toBe('block'); | ||
| expect(term.options.cursorBlink).toBe(false); | ||
| }); | ||
|
|
||
| test('multiple sequences in one chunk — last one wins', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, '\x1b[2 q some output \x1b[5 q'); | ||
| expect(term.options.cursorStyle).toBe('bar'); | ||
| expect(term.options.cursorBlink).toBe(true); | ||
| }); | ||
|
|
||
| test('sequence embedded in regular output', () => { | ||
| const term = makeTerm(); | ||
| applyDecscusr(term as never, 'text before\x1b[6 qtext after'); | ||
| expect(term.options.cursorStyle).toBe('bar'); | ||
| expect(term.options.cursorBlink).toBe(false); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.