Skip to content

fix(paste): handle only the first paste event the Ctrl+V trap receives - #394

Merged
Ark0N merged 1 commit into
Ark0N:masterfrom
irisitymichaelgrundberg:fix/ctrl-v-pastes-twice
Sep 10, 2026
Merged

Ark0N merged 1 commit into
Ark0N:masterfrom
irisitymichaelgrundberg:fix/ctrl-v-pastes-twice

Conversation

@irisitymichaelgrundberg

Copy link
Copy Markdown
Contributor

The bug

Ctrl+V in the terminal inserts the clipboard text twice. Right-click → Paste inserts it once. Paste a line into a Claude Code prompt with the keyboard and the composer holds it twice over; paste the same line from the browser's context menu and the composer is correct.

Reported on Firefox 155.0.1, with Codeman 1.24.7 installed by install.sh, Node 22.22.3, Ubuntu 22.04 under WSL2, and Claude Code CLI 2.1.263.

Why the key duplicates and the menu does not

_handleImagePaste() in src/web/public/image-input.js appends a hidden contenteditable div — the paste trap — focuses it, listens for a paste event on it, and calls document.execCommand('paste') to provoke one. Whatever arrives is inspected for image blobs, and plain text goes to terminal.paste(text) so the bracketed-paste markers survive.

Two independent routes deliver a paste event to that trap for one keypress:

  1. The document.execCommand('paste') call itself. Firefox dispatches a trusted paste event carrying the real clipboard and then returns false, because the trap's listener cancels the event and the command therefore never completes. I checked that in isolation, with no key involved at all: focus a contenteditable trap, call document.execCommand('paste') from a click handler, and Firefox delivers one trusted paste event while reporting false. Chromium and WebKit refuse the command outright and dispatch nothing.
  2. The keydown's own default action. The Ctrl+V branch at terminal-ui.js:359 returns false from xterm's custom key handler, which keeps the key from reaching the PTY as ^V but does not cancel the DOM event. architecture-invariants.md already records that fact under smart copy: xterm's _keyDown calls the custom handler before its own cancel(). The default action therefore still runs, and trap.focus() has already moved focus, so the browser's own paste lands on the trap too.

The trap's listener had no guard, so it ran once per event and each run called terminal.paste(). Right-click → Paste carries no keydown, so route 2 cannot exist for it and only xterm's own handlePasteEvent fires: exactly one insert. That asymmetry is the bug's signature.

The change

The trap now consumes exactly one paste event. The first is handled as before, and every later event for the same keypress is cancelled and dropped:

var pasteConsumed = false;
trap.addEventListener('paste', function(e) {
  e.stopPropagation();
  e.preventDefault();
  if (pasteConsumed) return;
  pasteConsumed = true;
  ...
});

preventDefault() moves to the top of the listener, where it now covers the dropped events as well. Both branches called it already, so the accepted event behaves exactly as it did.

The guard counts events rather than testing for a browser or reading execCommand's return value. That return value is the trap for anyone fixing this a second time: it is false in Firefox even though the event fired, and false in the other two because nothing fired.

Why the execCommand('paste') call stays. Removing it would also end the doubling, at its source, and I measured that it costs nothing in any engine I can drive here: with the call stripped out, Firefox, Chromium and WebKit each still deliver exactly one paste event to the trap, because trap.focus() has already run when the key's default action resolves. So on the desktop the call is pure redundancy. I still left it in, for two reasons. The trap technique came in with #84 for clipboard access over plain HTTP and for mobile, and a desktop measurement says nothing about real iOS Safari or Android Chrome; a browser that resolves the default action against the element focused when the keydown began would send its paste to xterm's textarea instead, where text still pastes through xterm's own handlePasteEvent but images silently stop working, since the trap's listener is the only place clipboard image blobs are read. And with the guard in place the redundant call is harmless. Removing it is a decision about mobile coverage rather than a cleanup, so it seemed yours to make — happy to drop it in this PR if you would rather have the cause gone than guarded.

Tests

test/image-paste-trap.test.ts loads src/web/public/image-input.js into a node:vm context with a fake document, in the style of test/input-cjk.test.ts, and drives the trap's listener directly. Four cases: one text event pastes once, two text events paste once, two image events upload one batch, and the accepted paste still detaches the trap and hands focus back.

On the old code it fails on both duplicate cases — the text reaches terminal.paste() twice and the image uploads twice. It needs no browser, so it runs in the npm test gate rather than test:browser.

Docs

  • docs/architecture-invariants.md gains a ### Terminal paste (Ctrl+V) section under Frontend, next to smart copy and Auto Copy: the two routes, the misleading execCommand return value, the measured event counts, and the test.
  • CLAUDE.md gains the same invariant as a Frontend entry, in the house one-paragraph form.
  • README.md, docs/wiki/Keyboard-Shortcuts.md and docs/wiki/Input-And-Voice.md gain a Ctrl+V row. All three tables listed every other terminal clipboard binding, Ctrl+C and Ctrl+Shift+C included, and omitted paste.

Verification

The gates from CONTRIBUTING, on this branch: npm test (353 files, 6759 passed, 12 skipped), npm run typecheck, npm run lint, npm run format:check, npm run check:frontend-syntax, plus scripts/check-public-assets.mjs for the hand-formatted frontend files — all clean.

Driven for real against a live install, one Ctrl+V per row, with the send path instrumented so the bytes are counted instead of delivered. "Writes to the PTY" counts what _sendInputAsync was handed:

Engine paste events on the trap writes before the fix writes after
Firefox 150.0 (Playwright build) 2 2 1
Chromium 151.0.7922.34 (headless) 1 1 1
WebKit 605.1.15 / Safari 26.4 (Playwright build, Linux) 1 1 1

The Firefox "before" row is the bug: two writes of the clipboard text from one keypress, measured by injecting the pre-fix _handleImagePaste back into the page. The Chromium and WebKit rows are the no-regression half, and they also explain how this survived: the two engines that refuse execCommand('paste') never doubled. With bracketed-paste mode on, the surviving write carries \x1b[200~…\x1b[201~ intact in all three.

The WebKit row is Playwright's Linux WebKit, not Safari on macOS or iOS. It shares WebCore's editing and clipboard commands, so it is good evidence about how Safari treats execCommand('paste'), and it is not a measurement of Safari itself.

Notes

  • One behavioural change, one new test, and the docs for it. No version bump and no CHANGELOG.md edit.
  • The in-app shortcut overlay in index.html also omits Ctrl+V. I left it alone on purpose: static application DOM goes through i18n.js, so a new row there wants a zh-CN entry too, and that felt like a separate change. Happy to add both if you would rather have it in here.
  • Nothing security-relevant was bypassed by the double paste. Both inserts went through terminal.paste(), so both carried the bracketed-paste markers.

Ctrl+V in the terminal inserted the clipboard text twice. Right-click →
Paste inserted it once.

`_handleImagePaste()` appends a hidden contenteditable div, focuses it, and
reads the clipboard out of the paste event that lands there. Two separate
routes deliver that event for a single keypress. The function issues
`document.execCommand('paste')` itself, which in Firefox dispatches a
trusted paste event and then returns false, because the trap cancels the
event and the command never completes; Chromium and WebKit refuse that
command and dispatch nothing. The keydown's own default action delivers the
other, because xterm calls the custom key handler before its own `cancel()`,
so returning false never calls preventDefault. Firefox therefore ran the
trap's listener twice and both runs reached `terminal.paste()`. The
context-menu paste involves no keydown at all, which is why that path stayed
correct.

The trap now accepts the first paste event and cancels every later one, so
how many paste events a browser delivers no longer changes what the PTY
sees. Measured on a live install, one Ctrl+V each: Firefox two events and
two writes before this change, Chromium and WebKit one and one, and every
engine one write after it.

The `execCommand('paste')` call stays. Stripping it out also ends the
doubling, and all three engines still deliver one event without it, since
`trap.focus()` has already run when the key's default action resolves. It is
kept because the trap technique arrived in Ark0N#84 for plain HTTP and for
mobile, and a desktop measurement says nothing about real iOS Safari or
Android Chrome: where a browser aims the default action at the element
focused when the keydown began, the command is the only route into the trap,
and the trap is the only place clipboard image blobs are read.

test/image-paste-trap.test.ts loads image-input.js into a `node:vm` context
with a fake document and fires two paste events at the trap. It covers text
and images, and fails on the old code with the text pasted twice and the
image uploaded twice.

Docs: the invariant goes into docs/architecture-invariants.md as a Terminal
paste section and into CLAUDE.md as a Frontend entry, both recording the
measured event counts and why the redundant call is still there. README.md
and the Keyboard Shortcuts and Input and Voice wiki pages gain a Ctrl+V row,
which all three tables were missing while listing every other clipboard
binding.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Sep 8, 2026
Claude Code answers an exhausted model budget INSIDE the turn ("You've
reached your Fable limit. Run /usage-credits to continue or switch models
with /model.") and then sits there with nothing to write. The reviewer never
produces a report, so `runTurn` waited out its full 40-minute deadline and
reported a bare "timed out after 40 min without a report", which reads as a
hung reviewer rather than an account that needs attention.

Measured on 2026-09-08: #388, #393, #394 and #377 each lost 40 minutes this
way, and because every attempt counted, all four reached MAX_AUTO_RETRIES and
would NOT have been picked up again once the budget returned. One spent
afternoon quietly took the whole queue out of service.

`findModelLimitNotice()` reads the notice off the pane and `runTurn` returns
a new `limit` outcome instead of waiting. It is consulted in exactly two
places, both of which mean "the turn produced nothing": on a stop where
`isDone()` is still false, and on each timed-out wait slice. A review that
merely discusses usage limits in its own findings therefore cannot be
mistaken for one that hit the wall, and the pattern matches neither the model
name nor a straight apostrophe, since the pane renders a typographic one and
every model prints the same sentence.

A spent budget is an account condition, not a bad PR, so it no longer spends
the per-head retry budget: the queue resumes by itself when the budget does.
Telegram now names the cause and the file to change.

Tests use the pane captured verbatim off the run that lost the 40 minutes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Ark0N
Ark0N merged commit a360763 into Ark0N:master Sep 10, 2026
2 checks passed
@Ark0N

Ark0N commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Merged and shipped in v1.26.2. Thank you.

This is the kind of write-up that makes a merge easy: the asymmetry between Ctrl+V and right-click Paste is the whole diagnosis, and you isolated the Firefox behaviour with no key involved at all rather than inferring it. The note that execCommand returns false in every engine, for two opposite reasons, is exactly the trap the next person would fall into, and it belongs in the comment where you put it.

On your open question: keep the execCommand("paste") call, which is what you did. Your reasoning is the right one. A desktop measurement says nothing about real iOS Safari or Android Chrome, the trap listener is the only place clipboard image blobs are read, and with the guard in place the redundant call costs nothing. Removing it would be a decision about mobile coverage, and it should be made with a mobile measurement in hand, not as a cleanup.

Counting events rather than sniffing the browser is also the right call: it stays correct if a future engine starts or stops honouring the command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants