Skip to content

fix(terminal): recover dropped keyCode 229 input (Android/IME) - #388

Merged
Ark0N merged 2 commits into
Ark0N:masterfrom
aakhter:pr/keycode-229-input-recovery
Sep 12, 2026
Merged

fix(terminal): recover dropped keyCode 229 input (Android/IME)#388
Ark0N merged 2 commits into
Ark0N:masterfrom
aakhter:pr/keycode-229-input-recovery

Conversation

@aakhter

@aakhter aakhter commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

The bug

Android/GBoard-style keyboards fire keydown with keyCode 229 and, on some paths, never mutate xterm's helper textarea. xterm has nothing to diff, so it emits no data — the typed character is silently dropped. It never reaches the PTY and never appears on screen.

Today terminal-ui.js, input-cjk.js, app.js and session-ui.js all deliberately pass through keyCode === 229 so xterm's CompositionHelper can own the event. That is correct when a real composition follows. When none does, the key is simply lost.

The fix

terminal-keycode229-recovery.js is a standalone controller that re-emits exactly those keys, and only once. xterm stays authoritative throughout:

  • Only an explicit keyCode 229 keydown carrying a single printable key (or Enter) is eligible. Process/Unidentified/Dead, modifier chords, AltGraph and a live composition are all left alone.
  • The re-emit is scheduled from a microtask and then a zero-delay timer, so xterm's own textarea diff always gets the first opportunity; canonical data for the same key cancels the pending fallback.
  • A late canonical duplicate arriving within 250ms is suppressed, so a browser that reports the key both ways delivers it once.
  • Recovered data re-enters the entire normal onData path (local echo, CJK gate, predictive echo, flush batching), so a recovered key takes the identical wire path to a canonical one.

The controller is optional at every call site: if the script is absent or construction throws, the terminal behaves exactly as it does today.

Changes

File
src/web/public/terminal-keycode229-recovery.js new, 227 lines, self-contained (needs only {textarea, emitRecovered})
src/web/public/terminal-ui.js +40/-2 — key hook, dedupe guard, teardown on terminal replacement
src/web/public/index.html script tag, before terminal-ui.js
scripts/build.mjs minify + content-hash entry, matching every peer module
test/terminal-keycode229-recovery.test.ts new, 13 unit tests
test/terminal-copy-shortcut.test.ts +1 browser test covering the wiring

The onData body is unchanged; it is now a named const handleTerminalData invoked from a one-line delegate, so the recovery path can re-enter it.

Verification

tsc --noEmit, lint, check:frontend-syntax, check:public-assets, format:check, build — all pass. test/terminal-keycode229-recovery.test.ts 13 passed; test/terminal-copy-shortcut.test.ts 9 passed under test:browser (8 before). Sixteen neighbouring terminal/input test files were run individually and all pass.

The assertions were checked by mutation: forcing duplicateIndex to -1 fails 2 tests, nulling the canonical lookup fails 1, removing the keyCode !== 229 gate fails 1, and breaking the terminal-ui.js wiring fails the browser test.

Reviewer notes

  • The wiring test lives in terminal-copy-shortcut.test.ts, which is excluded from config/vitest.ci.config.ts, so CI runs the 13 unit tests but not the integration one.
  • Not validated on a physical Android device from this branch; the trigger is reproduced synthetically (a keydown with keyCode 229 that never mutates the textarea). The double-delivery guard is the risk-bearing half: a browser reporting canonical data with no beforeinput/input on the helper textarea and later than the 250ms window could double a character. The design fails toward "deliver once, never suppress an unattributed byte" — the safer direction, since dropping input is the bug being fixed.

Android/GBoard-style keyboards fire keydown with keyCode 229 and, on some
paths, never mutate xterm's helper textarea. xterm has nothing to diff, so
it emits no data and the typed character is silently dropped: it never
reaches the PTY and never appears on screen.

terminal-keycode229-recovery.js is a standalone controller that re-emits
exactly those keys, and only once. xterm stays authoritative throughout:

- Only an explicit keyCode 229 keydown carrying a single printable key (or
  Enter) is eligible; Process/Unidentified/Dead, modifiers, AltGraph and a
  live composition are all left alone.
- The re-emit is scheduled from a microtask and then a zero-delay timer, so
  xterm's own textarea diff always gets the first opportunity; canonical
  data for the same key cancels the pending fallback.
- compositionstart and blur drop every pending candidate, so a real IME
  composition lifecycle is never second-guessed.
- After a recovery, one late canonical value attributed to that key token
  (via beforeinput/input on the helper textarea) is suppressed so the
  character cannot be delivered twice; the record expires after 250ms and
  an unattributed byte is never suppressed.

terminal-ui.js wires it at the two existing choke points — the custom key
handler and the onData registration, the latter now a named handler so the
recovery path can re-enter it — with both hooks wrapped so a failure in the
fallback can never break canonical input.

Unit coverage drives the module directly in a vm; the wiring itself is
covered end-to-end in the (browser-only) terminal-copy-shortcut suite.
@Ark0N

Ark0N commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Thanks for chasing this, and for the write-up. Losing a keystroke on an Android soft keyboard is a real and miserable bug. I do not want to merge this version though, because on the trace it targets I could reproduce it dropping a different keystroke, and double-delivering two others.

Reproduced in headless Chromium against the PR's own controller:

1. A recovered key claims the next keystroke's beforeinput/input and swallows its canonical byte (src/web/public/terminal-keycode229-recovery.js:158). The 250 ms dedupe is meant to suppress a late canonical copy of the key it just recovered. But nothing scopes the candidate to the event that created it, so typing the same character again inside that window matches the stale candidate and the real byte is dropped. Net effect on the trace this is written for: you get the dropped key back, and lose the next one.

2. Double delivery whenever the committed text differs from event.key (:22). The controller re-emits event.key, then the dedupe only suppresses a canonical copy that matches it. Enter and IME punctuation commit something other than event.key, so both go through: once from the recovery, once from the real path.

3. The trigger has not been shown on hardware (:23). On the usual Chrome-on-Android soft-keyboard trace the keydown reports key: 'Unidentified', which the controller ignores, so it is not clear it ever fires on the device it is for. A real-device trace would settle that before either of us spends more time on the dedupe.

That last point is the one I would start with, because it may change the design. It is worth saying that the pre-f7447196 approach, forwarding the orphaned input event (the mutation itself), never had to guess the character and cannot double-deliver on a mismatch. A version of that which fires only once xterm's own 0 ms diff has run and _inputEvent stayed silent may be the smaller and safer shape. I would rather look at that than at a more elaborate dedupe, but your call: if the timing evidence says otherwise, show me.

If the current shape stays, the dedupe needs to stop relying on microtask ordering surviving across trusted events, and candidates need to be cancelled on any textarea mutation.

Two smaller things whenever this comes back:

  • src/web/public/terminal-keycode229-recovery.js:1: new frontend modules carry an @fileoverview with @dependency/@loadorder, and the load-order list in CLAUDE.md plus the module inventory need the entry.
  • src/web/public/terminal-ui.js:306: the controller runs for every key event on every platform, ahead of the 229 gate. Cheap to move behind it.
  • test/terminal-copy-shortcut.test.ts:221: the wiring test lives in the Ctrl+C smart-copy browser file. It belongs in its own.

Thanks again, and please do come back with the device trace. The diagnosis is worth landing once the delivery path is safe.

…a guessed key

The previous shape guessed the character from `event.key` on keydown, re-emitted
it, and then tried to suppress a late canonical copy with a 250 ms
character-keyed dedupe. Review found three defects in that, all reproducible:
the dedupe matched on the character alone with nothing scoping a candidate to
the keydown that created it, so the same character typed twice inside the window
had its second, real byte swallowed; anything whose committed text differed from
`event.key` (Enter, IME punctuation) was delivered twice, because the dedupe
could never match it; and the trigger ignored `key === 'Unidentified'`, which is
what a soft keyboard reports, so it may never have fired where it was needed.

The input event already carries the committed text in `ev.data` — exactly what
xterm itself would have forwarded — so nothing has to be guessed. The controller
now only decides WHETHER to forward, by asking whether xterm produced canonical
data since the keydown that began the keystroke. No character-keyed matching
survives, so the first two defects are structurally impossible rather than
defended against, and nothing reads `key`/`keyCode`, so the third cannot recur.

Three details are load-bearing and each has a test that fails without it:

- The "did xterm speak?" snapshot is taken at KEYDOWN, not at the input event.
  `_keyPress` emits and sets `_keyPressHandled` before `input` fires, so a
  snapshot read at input time already contains that emission, reads it as
  silence, and delivers the character twice.
- Our `input` listener is registered with `capture: true`. The target is visited
  twice in the event path, so a capture listener calling `stopPropagation()`
  stops later BUBBLE listeners on that same target; xterm's `cancel()` runs
  exactly in the branch where it handled the input, so on bubble we would never
  observe handled events, and whether we observed them at all would hang off
  `options.cancelEvents`. Measured in jsdom and headless chromium; the table is
  in the module header.
- Enter is deliberately no longer special-cased. That mapping is what made the
  committed text differ from the re-emitted value in the first place.

The scope is also narrower than the old name suggests, and the browser test now
proves it rather than assuming it. For a keydown that reports keyCode 229 xterm
ALREADY self-rescues, via `CompositionHelper._handleAnyTextareaChanges()`
diffing the helper textarea on a 0 ms timer. A test asserting "we recovered it"
there passes while xterm does all the work, so the browser tests assert WHO
delivered the byte: zero canonical emissions for the genuinely orphaned case,
exactly one delivery for the case xterm rescues itself.

Also addresses review notes: the module gains an `@fileoverview` with
`@dependency`/`@loadorder` and an entry in the load-order list and module
inventory, and the wiring test moves out of the Ctrl+C smart-copy file into its
own. The keydown hook deliberately still runs for every key event rather than
moving behind the 229 gate: gating it would reinstate exactly the blindness
described above, and it is now a single counter assignment.
@aakhter

aakhter commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — this was a good catch and all three findings reproduce. I checked each against the branch rather than taking them on trust:

  1. Confirmed. The dedupe matched on record.data === data alone, with nothing tying a candidate to the keydown that created it, so the same character inside the 250 ms window had its second, real byte claimed by the stale record. The module even kept a keySequence counter that the dedupe never consulted.
  2. Confirmed. Enter → '\r' at :22 means the re-emitted value and the canonical copy can never match, so both go through.
  3. Confirmed at the source: :23 explicitly returns null for key === 'Unidentified'.

I've rebuilt it on the shape you suggested — forward the orphaned input event rather than replay a guessed character. Pushed as e8a93ad.

What changed

The controller no longer guesses. The input event already carries the committed text in ev.data — exactly what xterm itself would have forwarded — so it only decides whether to forward, by asking whether xterm produced canonical data since the keydown that began the keystroke. All character-keyed matching is gone, so 1 and 2 are structurally impossible rather than defended against, and nothing reads key/keyCode, so 3 cannot recur. The 250 ms window, the recovered[] records, the beforeinput claim logic and the Enter special case are all deleted.

Three details are load-bearing, each with a test that fails without it:

  • The snapshot is taken at keydown, not at the input event. _keyPress emits and sets _keyPressHandled before input fires, so a snapshot read at input time already contains that emission, reads it as silence, and delivers twice.
  • Our input listener is capture: true. The target is visited twice in the event path, so a capture listener calling stopPropagation() does stop later bubble listeners on that same target. _inputEvent calls cancel() exactly in the branch where it handled the input, so on bubble we would never observe handled events — and whether we observed them at all would hang off options.cancelEvents, which we don't set. Measured in jsdom and headless chromium; the table is in the module header.
  • Enter is no longer special-cased, since that mapping is what made committed text differ from the re-emitted value.

The part worth your attention: the gap is narrower than "keyCode 229"

Verifying the browser test, I found that xterm already self-rescues keyCode 229. CompositionHelper.keydown() calls _handleAnyTextareaChanges(), which snapshots textarea.value and diffs it on a 0 ms timer and emits the difference itself — the 0 ms diff you referred to.

Measured in headless chromium against a real terminal: for a 229 keydown, xterm emits and this controller correctly stands down. Which means my original browser test asserting "recovers a committed character xterm dropped, exactly once" was passing vacuously — xterm delivered the byte, not the recovery. It proved nothing.

So the browser tests now assert who delivered, via a count of xterm's own canonical emissions: zero for the genuinely orphaned case, exactly one for the case xterm rescues itself. The remaining gap is a refused insertText where no 229 diff was scheduled, and I've rewritten the @fileoverview to say that rather than the broader claim it made before.

This does sharpen your third point rather than answer it. If the Android traces behind this are all keyCode 229, xterm may already handle them and this may be unnecessary in the field. I still don't have a device trace, and I'm not going to pretend the synthetic tests substitute for one — happy for this to sit until I can get one, or to close it if you'd rather not carry the surface area on an unproven trigger.

Smaller items

Done: the module has an @fileoverview with @dependency/@loadorder and entries in the load-order list and module inventory; the wiring test has moved out of the Ctrl+C smart-copy file into its own.

One I didn't do, deliberately — moving the keydown hook behind the 229 gate. That was right for the old design, but it would now reinstate exactly the Unidentified blindness from your third point, since the hook is what anchors the snapshot. It's a single counter assignment per keydown, so the cost concern is gone. There's a comment at the call site saying so; flag it if you'd still rather it moved.

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 9d664ff into Ark0N:master Sep 12, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Sep 12, 2026
Each item is from the pre-merge review of the PR it names, applied on master
rather than by pushing to a contributor branch.

#400 (response viewer, shenlvkang-collab)
- The brief view opened at `scrollTop = 0`, right when it was a single card
  holding the last row. Now that it renders the whole turn, the top is the
  turn's first narration line and the answer can be screens below it, while
  loadFullContext already scrolls to the bottom of the same turn. A multi-row
  turn now opens at its newest text; a single card still opens at the top.

#401 (loopback links as web tabs, shenlvkang-collab)
- Drop `*.localhost` from the auto-route set. Every other member is an address
  literal that can only mean this box; a `*.localhost` DNS name is not one, and
  a resolver with a search domain retries `evil.localhost` as
  `evil.localhost.<search domain>`. The link source is agent-written terminal
  output, so that set is the whole confinement on a tap that makes Codeman
  fetch a URL server-side and persist it. The page-side test stays broader
  (`isOnBoxHostname`), where a false positive only declines to proxy.
- A link to the origin root navigated nothing: the path was flattened to '',
  which openWebview reads as "no deep link", leaving an open frame where it was.
- `this.webviews` being set does not mean it is loaded. initWebviews() assigns a
  truthy empty map and only then awaits the list, so a tap during page load
  found nothing to reuse and POSTed a duplicate record. Join the in-flight
  refresh instead.
- One dashboard per dev server rather than per host spelling, which is what the
  method's own comment already promised.
- Toast on the auto-create: it writes webviews.json, broadcasts over SSE and
  adds a Run-dropdown row on every signed-in device, with a new tab as its only
  previous signal.

#362 (remote omp continuation, timkjr)
- Accept the allowlisted `mode === 'omp'` arm as-is; a blanket registry render
  would hand deepseek a locally-resolved --profile and bypass claude's own
  overlay. A registry-declared switch is the follow-up if a third mode needs it.
- Revert the whole-file Prettier reformat of docs/remote-sessions.md (docs/ is
  hand-formatted and outside `npm run format`), keeping only the two new
  sections.
- Correct three stale passages: architecture-invariants' `exec claude
  --dangerously-skip-permissions`, the `exec <cli>` paragraph (claude and omp
  now have their own arms, and the claude pane's PID is the login shell), and
  omp-integration's `-c 'omp'`. RemoteCommandMode gains deepseek and omp.
- Add the missing `_maybeCaptureOmpSessionId` remote-guard test; the sibling
  guard in `_pinOmpRespawnId` had one and this path runs earlier, on the first
  idle turn.

#388 (keyCode 229 recovery, aakhter)
- Gate notifyCanonicalData on shouldSuppressTerminalQueryResponse and
  isTerminalFocusOrMouseReport. onData also carries the DA/DSR/CPR/OSC replies
  xterm answers during Ink redraws and its SGR mouse and focus reports; any of
  those landing between the keydown and the candidate's resolution was read as
  "xterm spoke for this keystroke", standing the recovery down and leaving the
  character dropped, worst on a busy agent pane. Reached through
  window.CodemanTerminalInput: the predicates live in a module IIFE that closes
  long before this call site, so bare references would throw into the
  surrounding try/catch and stop the notify from ever running.

Every fix has a test that fails without it (verified by reverting each).
Full gate green on the combined tree: 358 files, 6849 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Ark0N

Ark0N commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Merged, thank you, and specifically thank you for the part most people would have quietly left alone: finding that your own browser test was passing vacuously, and saying so. xterm's CompositionHelper.keydown() calling _handleAnyTextareaChanges() and self-rescuing the keyCode 229 case means the original test proved nothing, and rewriting the assertions to count who delivered, via xterm's own canonical emissions, is what turned this from a plausible fix into a verified one. That is a higher standard than most contributions hold themselves to.

You offered to let this sit or close it without a device trace. I am taking it, because the gap is nameable from xterm 6.0.0's source without one: _keyDown returns without emitting and without preventDefault when evaluateKeyboardEvent leaves result.key empty (it only assigns when keyCode >= 48 && key.length === 1, so keyCode 0 with a printable key qualifies), and for a single uppercase A-Z key it defers to keypress; when keypress never fires, which is the Android soft-keyboard norm, _inputEvent's (!ev.composed || !this._keyDownSeen) guard refuses the character. The delivery path is also safe by construction rather than by dedupe, which is what makes carrying it cheap even where it turns out to be unnecessary.

One thing applied on master at merge time (02b0e278): notifyCanonicalData() is now gated. The onData wrapper called it for every emission, but onData also fires for output the terminal produces on its own initiative, the DA/DSR/CPR/OSC replies xterm answers during Ink redraws and the SGR mouse and focus reports, which that file documents a few hundred lines up. Any one of those landing between the keydown and the candidate's zero-delay resolution read as "xterm spoke for this keystroke", stood the recovery down, and left the character dropped, worst on a busy agent pane which is exactly the case this exists for. It now goes through the same two predicates the one-shot Ctrl modifier uses for the same question. Narrowing the counter cannot introduce a duplicate, only make the controller less sure it can stand down.

Worth flagging one trap in that fix in case it comes up elsewhere: the predicates live inside the module IIFE that closes long before that call site, so they are reachable only through window.CodemanTerminalInput. Bare references throw a ReferenceError straight into the surrounding try/catch, which swallows it, and notifyCanonicalData would then never run at all, i.e. the exact opposite of the fix. I hit that on the first attempt and there is now a test pinning the global access.

You were right not to move the keydown hook behind the 229 gate, and the comment at the call site saying why is the right place for it. Reinstating the Unidentified blindness to save one counter assignment per keydown would be a bad trade.

Remaining: the PR description still describes the first design (the 250ms window, the recovered[] records, the Enter special case), and since we merge with merge commits rather than squashes, the PR page is the durable public record. Your follow-up comment has the correct account already. Worth moving it into the body when you get a chance, no rush.

Shipping in the next release.

Ark0N pushed a commit that referenced this pull request Sep 12, 2026
A Fable 5.1 reviewer read the whole release diff against 1.26.2 and returned
SHIP WITH FIXES. These are its findings, verified before acting on each.

**The changelog advertised a feature the code refuses (major).** The #401
changeset and docs/web-tabs.md both listed `*.localhost` in the loopback set.
The follow-up in 02b0e27 moved it out of the auto-route set on security
grounds and updated CLAUDE.md but neither of those, and that changeset becomes
the 1.27.0 CHANGELOG entry: a user would have read the release notes, tapped
`http://app.localhost:3000/` on a phone and got a connection error from a
documented feature. Both corrected, and the user guide now says why it is
excluded and that adding such a dashboard by hand still works.

**Dictation delivered its text twice (minor, #388).** `keydownSnapshot` started
`null`, so `keydownSnapshot ?? canonicalCount` at the input event read a counter
xterm had ALREADY bumped: on a fresh page load with no keydown yet, xterm's own
capture listener forwards the `insertText` itself (it is not gated behind a
keydown), then the snapshot equals the bumped count, `count > snapshot` is
false, and the controller emits the same text again. Reproduced directly
against the module: it emitted `hello` for input xterm had already delivered.
A `0` baseline restores that file's own invariant, that a missed recovery is
acceptable and a duplicated keystroke is not. Two regression tests, covering
both the xterm-already-delivered and genuinely-dropped halves.

**The sorted rail's arrow-key walk followed the DOM (minor).** `_tabKeydownHandler`
steps `querySelectorAll` order, which is `sessionOrder`, while a sorted rail
paints its rows with the flex `order` property, so ArrowDown from the top card
landed wherever that session happened to sit in the tab order. It now sorts its
node list by the COMPUTED order first: computed rather than inline, because web
tabs take their `order: 9999` from CSS and would otherwise read as 0 and lead
the walk. This is the one place that follows the paint; the Alt+N badge, the
drag model and the filter all still deliberately read the DOM.

**A trusted dashboard was auto-reused by a tapped link (minor, #401).** The
reuse loop skipped `managed` and direct-mode records but not `trusted`. A
trusted frame is mounted with `allow-same-origin`, i.e. on Codeman's origin
with the user's cookie, and these links come from agent output, which is the
threat model the loopback allowlist was just narrowed for. An agent that can
write into the dev server's tree could print a path that one tap opens inside
that privileged frame. Excluded from auto-reuse, with a test; opening it from
the Run dropdown is still an explicit action and unchanged.

**Two documentation claims that were no longer true.** CLAUDE.md said
test/location-overlay-commands.test.ts pins every remote pane command, but
remote claude and remote omp now have their own arm in `buildRemoteLaunchCommand`
and never reach `defaultRemoteCommandForMode`, which is what that test asserts,
so it pins nothing for them and changing either arm will not fail it. Named the
real pins instead. Also documented the arrow-key-walk exception in the rail
paragraph.

Left as follow-ups, deliberately: `POST /api/webviews` does not dedupe by URL
server-side, so two devices tapping one link concurrently can still save two
dashboards for one origin (pre-existing endpoint behaviour that #401 makes
reachable by a tap), and the location-overlay golden should assert the real
remote claude/omp commands rather than a branch neither reaches.

Full gate green: 359 files, 6869 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Ark0N

Ark0N commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Shipped in 1.27.0. One thing you should know, because it changed your module after merge.

A pre-release review found that the controller duplicates text on the first insertText of a page load when no keydown preceded it (dictation, Android voice typing). I reproduced it directly against the module before acting: it emitted hello for input xterm had already delivered.

The cause is keydownSnapshot starting at null. At the input event, keydownSnapshot ?? canonicalCount then reads a counter xterm has already bumped, because with _keyDownSeen false xterm's own capture listener forwards the text itself and that path is not gated behind a keydown. The snapshot therefore equals the bumped count, count > snapshot is false, the candidate reads "xterm stayed silent", and the text goes out a second time. After any keydown the stale snapshot is <= count and it stands down, so this is exactly once per initTerminal().

Fixed by initialising the baseline to 0, which restores the invariant your own @fileoverview states: a missed recovery is acceptable, a duplicated keystroke is not. Two regression tests added covering both halves (xterm-already-delivered, and genuinely dropped).

Worth being explicit that this is not the notifyCanonicalData gate I added at merge time. That gate cannot cause a duplicate: both predicates are anchored escape-sequence patterns that cannot match a printable keystroke, and a missing window.CodemanTerminalInput degrades to counting everything, i.e. fewer recoveries and never a duplicate. Your header comment's "cannot cause a duplicate" was true of the gate and false of the null baseline, which is a genuinely subtle distinction.

Everything else in the PR held up under review. Thanks again, particularly for catching that your own earlier browser test was passing vacuously and saying so plainly.

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