Commit Graph
8 Commits
Author SHA1 Message Date
Codeman maintainer 312a8faa06 fix(tiles): wire the Android soft-keyboard controller into every tile (#541 parity)
#541 fixed Android autocorrect duplicating the typed line in the primary
pane: xterm's keyCode-229 textarea diff is append-only, so an autocorrect on
space (delete a word, insert the corrected one) sent the whole line again.
The fix, an edit-based diff that sends one DEL per deleted code point and
then the inserted text, lives in terminal-keycode229-recovery.js together
with #441's next-keydown drain (a character committed in the same task as
Enter goes out ahead of the \r) and the original orphaned-insertText
recovery. Only the primary pane created that controller, so a grid tile or
the split's Pane B still ran xterm's stock behaviour. Both are gated on
width alone (1180 CSS px), which a wide Android tablet clears.

TerminalTile now creates its own controller in connect(), after the xterm
opens and before the first await, handed this tile's textarea, this tile's
CompositionHelper and _onTerminalData as the send path, so recovered bytes
go to the tile's own session through the exactly-once queue. As in the
primary pane, handleKeyEvent runs first in the custom key handler, above the
keyCode-229 early return, and notifyCanonicalData sits in the onData lambda,
gated on the same two CodemanTerminalInput predicates, never in
_onTerminalData, which the recovered bytes also take. destroy() tears the
controller down before disposing the xterm, which restores xterm's own diff
and removes the capture listeners. No mode or device gate, matching the
primary. The module itself is unchanged apart from its header; terminal-ui.js
gains only a comment naming the twin.

Tests: test/terminal-tile-input.test.ts now loads the real module into its
vm harness (with window timers, without which create() would silently throw
and every test would run against no controller) and drives a fake
CompositionHelper carrying xterm's own append-only diff. It covers install
and restore on the tile's own helper and textarea, autocorrect sent as an
edit (with a control reproducing the device-log duplicate), the last
character and an autocorrect each followed by Enter in one task, a
self-rescued 229 key delivered once, the onData gate ignoring query replies
and focus reports, two refused inserts after one keydown both recovered,
robustness when the controller throws, per-tile controllers, and a source pin
keeping the call above the early return. Removing the create, the
handleKeyEvent call, the notify, its gate, or the destroy each turns at least
one of them red, as does moving the notify into _onTerminalData. The browser
suite gains a TerminalTile block in
test/terminal-keycode229-recovery.browser.test.ts (real xterm, trusted
execCommand input, chunks asserted to address the tile's session, with a
destroyed-controller control).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-09 08:20:02 +02:00
Codeman maintainer d7140c32b4 fix(terminal): #541 landing fixes
A composition that ends in the same task as an Enter keydown was sent
twice. The keydown settled the pending edit (sending the composed word
and setting _dataAlreadySent), then xterm's own keydown finalized the
composition synchronously through _finalizeComposition(false), which
ignores _dataAlreadySent and sent the word again. settleEdit() now takes
the keydown event and, while xterm has a composition in flight
(_isSendingComposition), leaves the text to xterm for any key that makes
it finalize synchronously. On 229, CapsLock and the modifiers xterm keeps
the composition on its async path, which honours _dataAlreadySent, so the
edit still applies there. The waiting timers are cleared before that
early return, so a timer cannot fire after Enter's textarea clear and
send a run of DELs.

The guard sits in settleEdit(), not in applyEdit() as the bot proposed.
In applyEdit() it would also silence the timer path, where xterm always
finalizes asynchronously and skips _dataAlreadySent, so a non-composing
character typed just before a composition (the x in xword) would be lost
where master and the PR head both deliver it.

Two unit tests pin it, both measured: one fails without the guard
(the Enter keydown sends 'ab word' instead of 'ab '), and one fails with
the guard moved into applyEdit() (the timer path sends 'ab ' instead of
'ab xword'; a 229 settle must also still send the edit).

The xterm private-API guard test now also checks the bundle still ships
_isSendingComposition, and names it in its failure message and comment.

CLAUDE.md: the surviving #441 sentence said a keydown decides before
xterm's 229 rescue has run and that Enter's clear makes the pending diff
emit nothing. Neither holds any more (the edit diff is settled first, and
master already sent one DEL there), so it now says the edit diff is
settled first at that keydown. The PR's sentence notes the composition
exception.

The PR's own changeset is removed; its text goes into the single
combined release changeset.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-09 05:46:56 +02:00
DevvynandClaude Sonnet 5.5 f21ab39a89 fix(terminal): settle the edit-sync diff at the next keydown so Enter cannot erase the line (#541 review)
A pending 229 edit plus Enter in one page task: xterm clears the textarea for CR before the edit timer runs, so the timer diffed the whole line against '' and sent one DEL per character ahead of the submitted line. The pending diff is now applied synchronously from handleKeyEvent, before flushPending() (which keeps the orphan candidate from sending the character twice).

Tests: unit (3) and browser (4, local echo on and off, plain last character and autocorrect), each verified to fail without the settle call. xterm-private-api guard now names the CompositionHelper fields this depends on and checks the shipped bundle; CLAUDE.md notes the edit-based 229 diff.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS
2026-10-06 17:56:16 +08:00
DevvynandClaude Sonnet 5.5 0c71b753ef fix(terminal): stop Android autocorrect duplicating the typed line
xterm diffs the helper textarea with newValue.replace(oldValue, ''), which only works when the keyboard appended. SwiftKey/Gboard autocorrect on space deletes a word and inserts the corrected one, so xterm sent the whole value and then the inserted text again (testing the peompt + space became 'testing the peompttesting the prompt rompt '), and a multi-character delete was one DEL. The keyCode-229 controller now swaps in an edit-based diff against what was already sent.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS
2026-10-06 15:51:26 +08:00
codeman-localandClaude Opus 5 a1c35da0d8 fix(input): deliver a recovered keystroke before the Enter that submits it
Every message typed on an Android phone lost its last character.

An Android soft keyboard commits the last typed character and sends the
Enter key in ONE InputConnection transaction, so the committed-text
`input` event and the Enter keydown are both processed before any
zero-delay timer runs. The orphaned-input recovery from #388 resolved
its candidate only on such a timer, and that lost the character twice
over:

  * ORDER — xterm emits `\r` synchronously from the Enter keydown, and
    the local-echo composer submits `pendingText` right there. The
    recovered character arrived one macrotask too late to be part of the
    prompt.
  * LOSS — that same `\r` bumps the canonical counter, so by the time
    the candidate resolved, `canonicalCount > snapshot` read as "xterm
    spoke for this keystroke" and stood the recovery down. The character
    was not merely late, it was dropped.

Drain pending candidates synchronously at the next keydown instead, from
xterm's custom key handler, which runs before xterm processes that key.
The counter then still holds the value it had while the candidate's own
keystroke was current, so the stand-down decision is made against the
right keystroke, and the recovered byte reaches the composer ahead of
whatever the new key emits. The timer stays as the fallback for a
keystroke with no key after it.

Physical keyboards are unaffected: there the timer has already resolved
the candidate long before the next key arrives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 09:26:30 +08:00
Codeman maintainer 65ddedd1d4 fix: act on the 1.27.0 pre-release review
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 02b0e278 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>
2026-09-12 06:09:43 +02:00
Aamer Akhter e8a93ada1f fix(terminal): forward the orphaned input event instead of replaying 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.
2026-09-07 19:11:20 -04:00
Aamer Akhter 82b090c74a fix(terminal): recover dropped keyCode 229 input
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.
2026-09-07 12:27:36 -04:00