fix(ui-tui): read a coalesced byte run as typing, not as a paste - #355
Conversation
Typing and pasting are the same bytes on stdin, so the parser merged any multi-character run into one nameless keypress and the composer treated it as an unbracketed paste held behind a 50ms debounce. SSH, tmux and a blocked event loop all coalesce keystrokes, so continuous typing kept resetting that debounce and the input box stalled for the whole burst. Split a plain byte run into one keypress per code point. Bracketed paste (DEC 2004) is the only reliable paste signal, so App now asks the terminal with DECRQM whether the mode took, riding the batch that already carries XTVERSION, and the parser records the answer in its own state. Confirmed: every unmarked run is typing. Not confirmed: only runs that could not be a paste line -- free of control bytes and no longer than a typing burst -- split, so multi-line paste stays whole on terminals without the markers. Also fixes three input bugs with the same root cause: an auto-repeated backspace inserted literal DEL characters instead of deleting, a return arriving in the same read as the text before it was swallowed, and a control key arriving alongside text inserted a literal control character. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Fast echo writes a character straight to the terminal and defers the React update to the next frame, so a keystroke that takes it costs no render. Its guard required the display width to equal the string length, which a CJK character or an emoji can never satisfy, so every wide character forced a full tree reconcile instead. The terminal advances two cells for a wide character on its own, so the guard only has to hold what the terminal cannot: a single grapheme, at the end of a single line that still has room. This pairs with the byte-run split. Without it a two-character IME commit would trade one delayed render for two immediate ones. Memoize the useVirtualHistory return value as well: useMainApp feeds it into the appTranscript memo, so a fresh object on every render re-rendered the whole transcript on every keystroke. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: confirmed bracketed-paste mode still breaks raw compound keys; see the inline finding.
This change correctly targets coalesced typing by splitting unmarked byte runs after DEC 2004 is confirmed, and the accompanying fast-echo and virtual-history changes are internally consistent. The new head only merges current main; the reviewed TUI patch is unchanged from 7a7dc1a34c67.
I covered AGENTS.md, CONTEXT-MAP.md, ui-tui/CONTEXT.md, the full diff against the PR base, callers through InputEvent and TextInput, relevant history, backward compatibility, and the test changes. No tests were removed or weakened, but the confirmed-state suite omits the existing raw compound-key contracts. No CLAUDE.md is present.
Verification:
npm test --prefix ui-tui: 90 files and 1031 tests passed.npm run type-check --prefix ui-tui: passed.- Direct parser probe on
f8905957c534: reproducedESC+CRandESC+DELsplitting after DECRPM 2004 SET. git diff --check: passed.
Low-confidence boundary: I did not manually test a terminal/multiplexer matrix. That does not affect the blocker because the parser transition and caller behavior were reproduced deterministically.
| return false | ||
| } | ||
|
|
||
| if (bracketedPaste === 'confirmed') { |
There was a problem hiding this comment.
[P1] Preserve compound raw keys before confirmed-state splitting
When DEC 2004 is confirmed, this unconditional branch splits every multi-character text token, but raw compound keys are text tokens too. Reproduced on this head: ESC+CR changes from one empty-named event to Escape plus an unmodified Return; ESC+DEL changes from Meta+Backspace to Escape plus an unmodified Backspace. TextInput therefore submits instead of inserting a newline, and word deletion falls back to deleting one character.
The existing raw Alt+Enter tests use INITIAL_STATE; the confirmed-state block never repeats those contracts, so the full suite stays green. Extended-key protocols do not remove the fallback requirement: textInput.tsx explicitly documents that they are not reliable across SSH, zellij, and VS Code chains.
Please preserve recognized ESC plus control-key chords before code-point splitting, and add confirmed-state cases for ESC+CR, ESC+LF, ESC+DEL, and ESC+BS.
There was a problem hiding this comment.
Resolved in b93caedd8fe3. I reran the confirmed state directly: ESC+CR, ESC+LF, ESC+DEL, and ESC+BS now stay compound, including when embedded in a mixed typing burst. The new tests cover all four forms, and the full TUI suite and type-check pass.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The prior compound-key finding is resolved in b93caedd8fe3. pushTextKeys now preserves ESC+CR, ESC+LF, ESC+DEL, and ESC+BS before splitting the surrounding run, so Alt+Enter keeps the newline path and Meta+Backspace keeps its modifier. The confirmed-state tests cover all four contracts.
I reviewed the new delta and rechecked the full change against AGENTS.md, CONTEXT-MAP.md, ui-tui/CONTEXT.md, the tokenizer/parser callers through InputEvent and TextInput, relevant history, backward compatibility, and the test changes. No CLAUDE.md is present. No tests were removed or weakened.
Verification:
npm test --prefix ui-tui: 90 files and 1035 tests passed.npm run type-check --prefix ui-tui: passed.- Direct confirmed-state probes preserved all four raw compound keys and a compound key embedded in a mixed typing burst.
git diff --check: passed.
Low-confidence boundary: I did not manually repeat a terminal/multiplexer matrix; the parser state transition and downstream contract were exercised deterministically.
…lipboard path lying (#360) ## Summary A TUI that enables mouse tracking owns the drag, so the terminal never builds a native selection and its own copy shortcut has nothing to copy. Copy-on-select is what makes a transcript selection copyable at all -- but the subscription bailed out on `!isMac`, so on Linux and Windows a drag highlighted text and copied nothing. This lifts that gate, and fixes what was found while making the result observable. **Copy-on-select on every platform.** The subscription moves out of `useMainApp` into `subscribeCopyOnSelect()`. Inlined in a hook that needs a live gateway to start, none of its four guards could be tested; each now has a case, and each was checked by removing only that guard and watching the suite go red. The read of the bus state stops being an unchecked cast: the ambient `useSelection()` declaration types it as `unknown`, so the module narrows it instead. **The copy is reported to the transcript.** Nothing on screen changes when a drag ends, so a line is written once a clipboard path has actually taken the text. The callback fires on a non-empty result only, since `copySelectionNoClear()` resolves to an empty text when nothing reached the clipboard. The first report of a session carries the path caveat, later ones stay terse: OSC 52 is the one path a terminal can still refuse, and the first copy is when a user is looking for the reason a paste came up empty. **The reported path comes from the write, not from the environment.** A path predicted from env state cannot be right in general, because the environment does not record what happened: inside tmux a `load-buffer` that fails falls through to raw OSC 52 and leaves `TMUX` set exactly as the case that worked. With a stale tmux socket the bytes went out as OSC 52 while both callers said "copied to the tmux buffer", pointing the user at a `set-clipboard` setting that was never involved. `setClipboard()` already computed the three facts that decide this -- whether native was attempted, whether the buffer loaded, whether a sequence was emitted -- so it now reports which one took the text, and null when none did. The value travels with the copied text out of `copySelectionNoClear()` and `copySelection()` to the two call sites that display it. The predictor is deleted rather than left exported from the package: after the rewiring it had no callers, and a helper that can name a path the data did not take is a defect waiting for the next caller. Its env-matrix cases were assertions about the predictor itself, so what replaces them drives the real `setClipboard()` with `tmux` stubbed, including the failed-load fallback. Reporting an observed path also has to wait for the observation. On Linux the native tool is discovered by a probe, and `copyNative()` answered the first call before that probe settled -- a display server means a tool could exist, not that one does. Naming the path from that optimism claimed a native write on a machine with `DISPLAY` and no `wl-copy`, `xclip` or `xsel`, and the same call reported failure once the probe finished. The first call now answers with the probe's own result. It is still started before the tmux await, so the probe runs alongside `load-buffer` rather than ahead of the report, and later calls still answer synchronously from the cache. The copy never ran ahead of that probe either -- the tool is spawned inside it -- so only the report did. **The path caveat is scoped to a session, not to the process.** The first copy of a session carries the caveat and later ones stay terse, but the flag holding that lived in a hook that outlives the session: `newSession()` and `resumeById()` replace `ui.sid` without remounting `useMainApp`, so every session after the first opened with the terse line and the user never learned an OSC 52 paste can come up empty. The tally moves into a reporter keyed on a session identifier, so the state and the thing it is scoped to live together and the boundary is unit-testable. The hook reads the sid through `getUiState()`, which keeps a session change from tearing down the bus subscription. **The report must not overclaim.** OSC 52 hands bytes to the terminal and the terminal decides whether to keep them, and nothing in the write path caps or chunks the payload. Measured on the real code: one 200x50 viewport of CJK is a 40 KB escape sequence, and a selection dragged through 2000 rows is 536 KB in a single sequence. Terminals drop an oversized sequence without a word, and `setClipboard()` reports success for all of it because bytes were written to stdout. So that path reports what it sent; native and tmux, which really did write a clipboard, still say copied. The count was UTF-16 code units, which reads three emoji as six characters and a combining accent as two, so it is counted by grapheme now. CJK was already right, being one code unit per character. **The documented env knobs were dead.** The OSC 52 override and the clipboard debug switch were readable only under the upstream `HERMES_TUI_` names, while every knob this project documents uses `RAVEN_TUI_` and `/copy`'s own failure hint names `RAVEN_TUI_FORCE_OSC52` and `RAVEN_TUI_DEBUG_CLIPBOARD`. Following that hint changed nothing. Both spellings are now read, `RAVEN_TUI_` first, with the `HERMES_TUI_` names kept as aliases so an environment that worked before still works. The debug switch moves behind `clipboardDebugEnabled()` so its four call sites cannot drift apart. ## Type - [x] Fix - [x] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other Two boxes: the platform gate is the feature, the clipboard-path and env-name defects are fixes found while making it observable. ## Verification Run from `ui-tui/`, on Linux, node 22, at branch head after the rebase. ``` npx vitest run --no-file-parallelism Test Files 91 passed (91) Tests 1066 passed | 13 skipped (1079) npx tsc --noEmit -p tsconfig.json clean, exit 0 npm run lint 22 problems (0 errors, 22 warnings) python3 -m scripts.check_commit_messages github/main..HEAD exit 0 ``` The suite is run with `--no-file-parallelism` deliberately: the ink render tests fail under default worker parallelism at this suite size, and no CI job runs them. The 22 lint warnings are the baseline, not new: 22 before the branch and 22 after, none of them in a file this branch touches. Every file this branch touches passes `prettier --check`. Two of them did not at first, and that was this branch's doing rather than the base's: dropping a name from the `@hermes/ink` import and adding a wide function signature left both inside the print width while still wrapped. They are formatted in their own commit. The files already unformatted on `github/main`, `chatStream.ts` among them, are left alone rather than reformatted into this diff. Suites that exercise the changed code directly: `src/__tests__/copyOnSelect.test.ts` (12 cases), `src/__tests__/clipboard.test.ts` (30 cases), `packages/hermes-ink/src/ink/termio/osc.test.ts` (39 cases). An environment gap that made the first pass of this verification worthless, since it would silently affect anyone reviewing from a worktree: `ui-tui/node_modules` here was a symlink to another checkout's, so `node_modules/@hermes/ink -> ../../packages/hermes-ink` resolved against the symlink target and landed on that other checkout's copy of the package. `tsc` and the dist build therefore read an unmodified `hermes-ink` while the app code under test was the edited one. Fixed by giving the worktree a real `node_modules` whose `@hermes/ink` points at its own package, confirmed with `tsc --listFiles` naming the edited `ink.tsx`, then rebuilt and re-ran everything. Every number above is from after that. New tests were proved load-bearing rather than assumed: - Each of the four guards in `subscribeCopyOnSelect()` was removed on its own and the suite watched go red (1, 1, 2 and 2 failures), then restored. - The three cases that assert a copy did *not* happen are each paired with a positive assertion on the same subscription, because a bus that was never wired up satisfies the negative half by itself. - The path-reporting cases were checked by reverting the derivation to the deleted predictor's rule and watching the failed-load case report `tmux-buffer` again. - The Linux first-probe case was checked by reverting `copyNative()` to its optimistic `return true` and watching it fail on `success: true, path: 'native'` where it now asserts `success: false, path: null`. Establishing that this one was introduced here rather than inherited took running the same scenario against `github/main`, which reported `path: 'osc52'` for it: `success: true` with nothing written predates the branch, the native claim does not. - Of the three session-scoping cases, one is load-bearing: reverting the reporter to a process-wide flag fails "spends the caveat once per session". The other two -- a session returned to, and two reporters not sharing a tally -- pass with the flag as well, and are there to pin the property against a later refactor. Said plainly rather than implying all three prove the fix. One pre-existing assertion is flipped rather than kept. It pinned the terse copy-on-select line to "copied" on the OSC 52 path, which is the overclaim this branch removes, so the assertion encoded the defect instead of the behaviour worth holding. Rebased onto `22b71765`. The base gained #361, #362 and #355; the only file this branch and those commits both touch is `ui-tui/src/app/useMainApp.ts`, where #361 reworked the cover and prompt block while this branch replaced the copy-on-select effect in the same hook. The replay was clean and the whole verification above was re-run at the new head rather than carried forward. `git merge-tree --write-tree` needs git 2.38 and this box has 2.34.1, so the check was the file-overlap table plus the re-run, not a dry-run merge. Manual check of the rendered artifact, since `raven tui` runs the prebuilt `ui-tui/dist/entry.js` and nothing rebuilds it automatically: `npm run build`, then confirmed the new strings are in `dist/entry.js`. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed No doc change. `CONTEXT.md` has no clipboard or selection entry, and copy-on-select was already the term in use in the vendored fork, so nothing new is coined. The `/help` hotkey table is deliberately untouched: select-then-copy is ordinary terminal behaviour and does not need advertising. ## Risk Behaviour changes for users who are not on macOS: a drag now writes the clipboard and adds a transcript line. On macOS nothing changes except the wording of that line. `/copy` keeps its shape and gains the path the write actually took. One API change inside the vendored fork: `Ink.copySelection()` and `copySelectionNoClear()` return `{ text, path }` instead of the copied string, and `getClipboardPath()` is gone from the package's exports. Internally `copyNative()` may now answer with a promise on Linux's first copy, which `setClipboard()` awaits after starting the tmux write; no caller outside that function sees it. Both are consumed only within this repo, and the ambient declaration, the app interface and the two call sites move with them. Rollback is per concern. Reverting the feature commit restores the platform gate and leaves the fixes, which stand on their own. Reverting all of them returns the files to their base state; no data, config or on-disk format is involved, and no migration exists to undo. Checked and deliberately not fixed: - The OSC 52 write has no size cap and no chunking, so a large selection is handed to the terminal whole and silently dropped past whatever that terminal tolerates. This is why the OSC 52 path reports what it sent rather than claiming a copy. Capping or chunking it means changing the vendored write path and picking a threshold per terminal, neither of which belongs in this branch. - `Ink.copySelectionNoClear()` calls `getSelectedText()` outside its own `try`, so a throw there rejects the promise. The base already left that rejection unhandled at a bare `void` call, and adding a `.then()` does not change it. Fixing it means restructuring a vendored method this branch has no other reason to touch. - The effect's dependency array gains `sys`. It is provably stable (`useCallback` over `appendMessage`, itself `useCallback` with `[]`), so the subscription is not re-created in practice. If it ever were, the version de-dupe would reset and one extra line could be written; the path caveat would not repeat, because that tally now lives in a ref that survives the effect. - Running `/copy` before the first drag of a session shows the path caveat twice, once from each lane. Sharing that state between an explicit command and an automatic one to save a duplicated sentence is not worth the coupling. - Two narrative comments in `copyOnSelect.test.ts` were reduced rather than removed: each kept the constraint it was wrapped around (that the platform case can only mean something on a non-macOS runner, and that an empty result is the whole signal that nothing reached the clipboard) and lost the "defect this closes" framing. Reading section 1.1 as rejecting the task-context wrapper rather than the constraint underneath is a judgement call, flagged as one. The selection bus is faked in the new tests. The real one returns no-ops outside a fullscreen Ink instance bound to a TTY, so there is no way to drive it from vitest; the fake mirrors the ambient `useSelection()` contract, including `getState(): unknown`, which is why the narrowing is exercised rather than assumed. Flagging it because a hand-rolled stand-in for a real type is exactly the blind spot that hides a defect. Security: no new surface. Selected transcript text already reached the terminal through this path on macOS and through `/copy` everywhere; the same bytes now travel on the other platforms, on a user-initiated drag, which is what a native terminal selection would have done anyway. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Summary
A colleague reported the TUI composer feeling laggy while typing; it did not
reproduce on a fast local terminal with short English input. Two causes, and
they amplify each other.
Typing was being read as a paste. Typing and pasting are the same bytes on
stdin, so the parser merged any multi-character run into one nameless keypress
and the composer treated it as an unbracketed paste held behind a 50ms
debounce. SSH, tmux and a blocked event loop all coalesce keystrokes, so
continuous typing kept resetting that debounce and the input box stalled for
the whole burst. A CJK input method commits several characters at once, so it
took this path on almost every word.
The fix stops guessing from length. A plain byte run is split into one keypress
per code point, and bracketed paste (DEC 2004) becomes the paste signal it was
always meant to be. App asks the terminal with DECRQM whether the mode actually
took, riding the query batch that already carries XTVERSION, and the parser
records the answer in its own state. Confirmed: every unmarked run is typing.
Not confirmed: only runs that could not be a paste line - free of control bytes
and no longer than a typing burst (32 characters) - split, so multi-line paste
still stays whole on terminals without the markers.
Three input bugs shared that root cause and go away with it: an auto-repeated
backspace inserted literal DEL characters instead of deleting, a return
arriving in the same read as the text before it was swallowed without sending,
and a control key arriving alongside text inserted a literal control character.
Wide characters could never take the fast-echo path. Fast echo writes a
character straight to the terminal and defers the React update to the next
frame, so a keystroke that takes it costs no render. Its guard required display
width to equal string length, which a CJK character or an emoji can never
satisfy, so every wide character forced a full tree reconcile. The terminal
advances two cells for a wide character on its own, so the guard now only holds
what the terminal cannot: a single grapheme, at the end of a single line that
still has room.
The two halves ship together on purpose. Without the second, a two-character
IME commit would trade one delayed render for two immediate ones.
Also memoizes the
useVirtualHistoryreturn value.useMainAppfeeds it intothe
appTranscriptmemo, so a fresh object identity on every renderre-rendered the whole transcript on every keystroke - the amplifier that made
long sessions worse.
Note for review: this is the first change to the ink fork's implementation in
the repository's visible history (four earlier commits touched only its README,
export surface and lockfile). Its README asks for the same test and review bar
as first-party code, so
parse-keypress.tsis the part worth reading closely.Type
Verification
Run from
ui-tui/, on this branch rebased ontomainat 0e544ec, after aclean
npm ci:npm test- 90 files, 1031 tests passed, 0 failednpm run type-check- cleannpm run lint- 0 errors, 22 warnings, identical to the pre-change baseline(compared warning-by-warning against a clean checkout of
main; only linenumbers moved)
Three load-bearing new tests were mutation-checked: the implementation was
broken on purpose and each test was confirmed to fail, then restored. One of
them silently passed on the first attempt because component tests load the
bundled
packages/hermes-ink/distrather than its source; it was re-checkedafter rebuilding.
Manual, in
raven tui --devafter rebuilding the ink bundle: holding backspacedeletes instead of inserting
^?; typing a sentence and pressing returnimmediately sends it; CJK input keeps up with typing; a several-hundred-line
paste still collapses into a single placeholder; a dropped file path is still
recognised.
New coverage:
parse-keypress.test.tstextInputFastAppend.test.tstextInputTypingBurst.test.tsxvirtualHistoryIdentity.test.tsxThree existing tests in
parse-keypress.test.tswere updated. They assertedthat a text run arrives as a single key, which is the shape this change
replaces; they now assert the intent instead - every event is a key and their
sequences join back to the original text. That also checks each event's kind,
which the old form did not.
Risk
User-visible behaviour changes, all in the composer:
what they say instead of inserting literal control characters.
The regression surface is paste. On a terminal that confirms DEC 2004 nothing
about paste handling changes: markers still delimit it and the parser already
reassembled marked pastes across reads. On a terminal that does not confirm it,
short printable runs now arrive as typing, which is why the 32-character
threshold is there - it keeps long single-line pastes whole so placeholder
collapsing and dropped-path detection still work. A paste of a short
single-line string on such a terminal is the one case that changes: it inserts
character by character instead of going through the paste handler.
Rollback is a revert of the two commits; they touch no state, no protocol and
no persisted format.
Related Issues
N/A