test(paste): pin the main pane's keydown hand-off, and say which engines still get two pastes (#799)

The two edits promised in the review of #799:

- The main pane's Ctrl+V branch hands the keydown to the paste trap, and
  nothing tested that call: with it reverted to `_handleImagePaste()` every
  test still passed and Firefox would paste twice again. A source pin now
  fails on that, like the Ctrl+C one in terminal-copy-clean.test.ts.
- architecture-invariants and src/web/CLAUDE.md described the key's own paste
  as reaching the trap in every browser. Since #799 that is Chromium and
  WebKit only: in Firefox the trap cancels the keydown.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-11-11 10:23:02 +01:00
1 parent 1d7288dc4f
commit fbcf8eba72
3 files changed
+19 -3

No files matched your search

+2 -2
View File
@@ -956,9 +956,9 @@ Feedback is silent on success except ONCE per page load (a feature that works by
**The Ctrl+V paste trap consumes exactly ONE paste event.** `_handleImagePaste()` (image-input.js) appends a hidden `contenteditable` div, focuses it, and reads the clipboard out of the paste event that lands there. An image becomes an upload whose saved path is typed into the session; plain text is a Codeman-built bracketed paste (`\x1b[200~` … `\x1b[201~`, nested markers stripped, newlines as `\r`) handed to that pane's terminal via `terminal.input(framed)`. `terminal.paste()` only emits those markers while xterm's DECSET 2004 mirror is on, and a terminal replay clears that mirror (#716), so Ctrl+V builds the frame itself. `input()` fires onData as user input, which scrolls to the bottom and clears the selection. The main pane's onData bracketed-paste branch and a tile's `_onTerminalData` deliver the frame, which is where the local-echo flush and the delayed write live (#219). A frame wrapped in CSI 200~/201~ does not match the query-reply or mouse-report filters. The terminal is refocused; the paste is not written into the xterm buffer. Two independent routes deliver that event for one keypress, and a browser may fire both:
1. **`document.execCommand('paste')`**, which the function calls itself. ⚠️ Its return value proves nothing about whether it fired. Firefox dispatches a trusted `paste` event carrying the real clipboard and still returns `false`, because the trap cancels the event and the command therefore never completes. Chromium refuses the command outright and dispatches nothing.
2. **The keydown's own default action.** The `Ctrl+V` branch in `attachCustomKeyEventHandler` returns `false`, which stops xterm from evaluating the key into `^V` but does not cancel the DOM event, for the same reason rule 1 of smart copy above spells out. `trap.focus()` has already run by then, so the browser sends its own paste to the trap as well.
2. **The keydown's own default action.** The `Ctrl+V` branch in `attachCustomKeyEventHandler` returns `false`, which stops xterm from evaluating the key into `^V` but does not cancel the DOM event, for the same reason rule 1 of smart copy above spells out. `trap.focus()` has already run by then, so the browser sends its own paste to the trap as well. That holds for Chromium and WebKit, where it is the only paste. In Firefox the trap cancels the keydown once route 1 has delivered (below), so route 2 no longer happens there.
Measured against a live install, one `Ctrl+V` each: Firefox delivers two paste events, Chromium and WebKit one. Handling both wrote the clipboard text to the PTY twice, which is why `Ctrl+V` pasted twice while right-click → Paste pasted once. That menu path involves no keydown, so route 2 cannot exist for it.
Measured against a live install before the keydown was cancelled, one `Ctrl+V` each: Firefox delivered two paste events, Chromium and WebKit one. Handling both wrote the clipboard text to the PTY twice, which is why `Ctrl+V` pasted twice while right-click → Paste pasted once. That menu path involves no keydown, so route 2 cannot exist for it.
⚠️ **Once route 1 has delivered, the trap cancels the keydown.** Firefox dispatches the `execCommand` paste synchronously, inside the call. It delivers the key's own paste as a separate task about 10 ms later, measured in Playwright's Firefox. That later event races the `setTimeout(0)` that removes the trap and refocuses the terminal. When the timer wins, the event lands on xterm's textarea, and xterm's own `handlePasteEvent` sends the clipboard a second time; the trap's one-shot flag never sees that event. This race is why a one-shot flag alone left `Ctrl+V` pasting twice "sometimes". Both callers, the main pane's key handler and a tile's, pass the keydown as `target.keyEvent`. `_handleImagePaste()` calls its `preventDefault()` when the paste listener has already run by the time `execCommand` returns, so the browser never issues route 2. Chromium and WebKit dispatch nothing from route 1, so they keep the default action, which delivers their only paste event. ⚠️ Never cancel the keydown unconditionally: in those engines that loses the paste entirely.
+1 -1
View File
@@ -20,7 +20,7 @@
**OSC 52 clipboard** (`_installOsc52Clipboard`, terminal-ui.js; main terminal + every `TerminalTile`): a program in the PTY (Claude Code's `/copy`, tmux yanks, vim) sets the browser clipboard with `ESC ] 52`. ⚠️ Writes only: a `52;?` read query is always refused, never answered. ⚠️ A replay must never copy: `chunkedTerminalWrite` strips every OSC 52 (`OSC52_STRIP_RE`, constants.js) because xterm parses after `_isLoadingBuffer` is cleared, so the flag alone does not hold; a tile gates on its own `_bufferLoading`, which does. ⚠️ One copy arrives up to three times (Claude's own OSC 52, `tmux load-buffer -w`, set-clipboard forwarding), so identical text within `OSC52_DEDUPE_MS` is dropped. ⚠️ Every tab on the session receives it; only the focused one opens the manual-copy fallback. ⚠️ Restore prior focus after `_copyText` (its execCommand fallback focuses a temp textarea). Tests: `test/terminal-osc52-clipboard.test.ts` (replays drive the real `chunkedTerminalWrite` over `@xterm/headless`).
**Ctrl+V paste trap** (`image-input.js`): `Ctrl+V` routes through `_handleImagePaste()`, which focuses a hidden `contenteditable` trap and reads the clipboard from the paste event landing there; images upload and their paths are typed in, plain text is a Codeman-built bracketed paste handed to that pane's terminal via `terminal.input(framed)` (strip nested 200/201 markers, newlines to `\r`, wrap CSI 200~/201~) because `terminal.paste()` drops those markers after a replay clears xterm's DECSET 2004 mirror (#716). `input()` fires onData as user input, so the main pane's bracketed-paste branch and a tile's `_onTerminalData` deliver the frame, including the local-echo flush (#219). The terminal is only refocused. ⚠️ **The trap must consume exactly ONE paste event** (Firefox delivers two per keypress: the `execCommand('paste')` event and the keydown's default action); the one-shot flag lives on the trap, never on a browser check. ⚠️ **Once `execCommand` has delivered, the trap cancels the keydown** (`target.keyEvent`, passed by both the main pane and a tile): Firefox's second paste is a separate task that races the trap's removal, and when it loses it lands on xterm's textarea and pastes again, which the flag cannot see. Never cancel the keydown unconditionally; Chromium and WebKit get their only paste from its default action. ⚠️ Do not remove the `execCommand('paste')` call: on some mobile engines it is the only route into the trap, and the trap is the only place image blobs are read. Tests: `test/image-paste-trap.test.ts`. → [architecture-invariants#terminal-paste-ctrlv](../../docs/architecture-invariants.md#terminal-paste-ctrlv)
**Ctrl+V paste trap** (`image-input.js`): `Ctrl+V` routes through `_handleImagePaste()`, which focuses a hidden `contenteditable` trap and reads the clipboard from the paste event landing there; images upload and their paths are typed in, plain text is a Codeman-built bracketed paste handed to that pane's terminal via `terminal.input(framed)` (strip nested 200/201 markers, newlines to `\r`, wrap CSI 200~/201~) because `terminal.paste()` drops those markers after a replay clears xterm's DECSET 2004 mirror (#716). `input()` fires onData as user input, so the main pane's bracketed-paste branch and a tile's `_onTerminalData` deliver the frame, including the local-echo flush (#219). The terminal is only refocused. ⚠️ **The trap must consume exactly ONE paste event** (Firefox delivered two per keypress, the `execCommand('paste')` event and the keydown's default action, until the trap began cancelling the keydown, and the flag still guards a browser that sends both); the one-shot flag lives on the trap, never on a browser check. ⚠️ **Once `execCommand` has delivered, the trap cancels the keydown** (`target.keyEvent`, passed by both the main pane and a tile): Firefox's second paste is a separate task that races the trap's removal, and when it loses it lands on xterm's textarea and pastes again, which the flag cannot see. Never cancel the keydown unconditionally; Chromium and WebKit get their only paste from its default action. ⚠️ Do not remove the `execCommand('paste')` call: on some mobile engines it is the only route into the trap, and the trap is the only place image blobs are read. Tests: `test/image-paste-trap.test.ts`. → [architecture-invariants#terminal-paste-ctrlv](../../docs/architecture-invariants.md#terminal-paste-ctrlv)
**File Viewer text view: rendered markdown + Lines/Wrap toggles** (`_renderFilePreviewText()` in panels-ui.js): a `.md`/`.markdown` opens RENDERED by default with an `MD` pill back to source; the plain-text view has `Lines` (CSS-counter gutter) and `Wrap` toggles. ⚠️ ONE markdown pipeline: the viewer calls `_renderMarkdown(text, { breaks: false })` (marked + the DOMPurify allowlist, the Response Viewer's; chat keeps the default `breaks: true`, a file must not turn every hard wrap into a `<br>`) and binds the Response Viewer's click delegate (`_bindResponseViewerInteractions`) on the preview body for code-copy buttons and path links; never a second parser or handler. ⚠️ The document is built inside a `<template>` (a detached div with `innerHTML` set starts fetching every `<img src>` before the rewrite), then `_rebaseFilePreviewMarkdownRefs()` points relative images at the workspace-confined `file-raw` under the document's directory and root-relative ones under the workspace root (never a widened route; a failed load degrades to alt text), after `decodeURIComponent`ing the ref and dropping `?query`/`#fragment` (marked percent-encodes destinations, and the route encodes again), and turns workspace links into `a.rv-path` carrying `data-session-id` for the delegate (the linkifier's absolute paths get it too), stripping the `target` marked gave them. ⚠️ A preview opened by attachment id under a bare file name (attachment cards, history drawer: the registry keeps no relative path) has no directory, so its workspace refs degrade (images to alt text, links to their text), never resolve against the workspace root. ⚠️ The container carries `data-i18n-skip` or the translator rewrites the document's prose. ⚠️ Toggles are per-device localStorage keys (`codeman:filePreview*`), never `SettingsUpdateSchema`; Lines/Wrap are class flips on the ONE `<pre>`, with rules scoped `.file-preview-body > pre.file-preview-text` so they never leak into the document's code blocks. Markdown fetches `lines=10000` (the route ceiling), other text keeps 500. ⚠️ `md`/`markdown` are IN `FILE_PREVIEW_EXTENSIONS` (owner decision 2026-10-13): a markdown path clicked or tapped in the TERMINAL (main pane, grid tiles and split Pane B share `registerFilePathLinkProvider`) opens this rendered view like every other surface, never the streaming log viewer; other in-workspace text keeps the tail viewer (live follow). The terminal pattern links `.markdown` and `.htm` too. ⚠️ **Right-to-left text** (#715) has three parts on that one pipeline: `_renderMarkdown` pins `pre, code` to `dir="ltr"`, `_linkifyFilePaths` pins the anchors it builds the same way (`dir="auto"` skips a descendant that carries a `dir`, so `npm install` or a path at the head of a Persian sentence does not decide its direction; a markdown link's label is prose and stays unpinned), and `_applyBlockDirection()` gives each OUTERMOST prose block `dir="auto"`, keeping a `dir` the agent wrote and what it covers. ⚠️ That pass runs LAST, on the DOM, in both consumers (`_buildResponseViewerMessage`, `_renderFilePreviewText`), never in `_renderMarkdown` (its string must hold no `dir="auto"`) and never inside the linkifier (which returns early on a stale constants.js): an element already carrying `dir="auto"` re-resolves its direction on every insertion under it, so a code-only or path-only document went quadratic and a 200 KB list froze the tab for 40 s. ⚠️ The `.rv-text` block rules use logical properties (`padding-inline-start`, `border-inline-start`, `border-inline-end`, `text-align: start`, logical corner radii), never physical left and right, or an RTL block keeps its bullets and quote bar on the wrong side. The terminal pane is out of reach: xterm.js has no RTL support (xtermjs/xterm.js#701). Tests: `test/file-preview-markdown.test.ts`, `test/response-viewer-rtl.test.ts`. → [architecture-invariants#file-viewer-text-view-rendered-markdown-and-text-toggles](../../docs/architecture-invariants.md#file-viewer-text-view-rendered-markdown-and-text-toggles)
+16
View File
@@ -455,6 +455,22 @@ describe('Ctrl+V paste trap', () => {
expect(h.inputted).toEqual(['\x1b[200~hello world\x1b[201~']);
});
it('is handed the keydown by the main pane, pinned at the source', () => {
// The Ctrl+V branch lives inside initTerminal's attachCustomKeyEventHandler
// closure, over a real xterm this harness cannot build, so the call is pinned
// at the source, like the Ctrl+C rule in terminal-copy-clean.test.ts. A tile's
// call is driven for real in terminal-tile-input.test.ts. Without the keydown
// the trap cannot cancel it, and Firefox pastes twice again.
const terminalUi = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8');
const start = terminalUi.indexOf('// Ctrl+V / Cmd+V: intercept before xterm sends ^V to PTY.');
expect(start).toBeGreaterThan(-1);
const end = terminalUi.indexOf('// Ctrl+Z (SIGTSTP/job-control suspend)', start);
expect(end).toBeGreaterThan(start);
const branch = terminalUi.slice(start, end);
expect(branch).toContain('this._handleImagePaste({ keyEvent: ev });');
expect(branch).not.toContain('this._handleImagePaste()');
});
});
describe('image upload insertion policy', () => {