Files
Codeman/src/web/public/terminal-keycode229-recovery.js
T
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

196 lines
8.0 KiB
JavaScript

/**
* @fileoverview Orphaned-input forwarder for xterm's helper textarea.
*
* xterm's `CoreBrowserTerminal._inputEvent` only forwards an `insertText`
* input event while `(!ev.composed || !this._keyDownSeen)` holds. A soft
* keyboard that delivers a `composed: true` input event after a keydown fails
* that guard, so xterm returns without emitting and the committed character is
* silently dropped.
*
* ⚠ The gap is NARROWER than "keyCode 229", and assuming otherwise produces a
* controller that looks useful while doing nothing. For a keydown that really
* does report `keyCode: 229`, xterm ALREADY self-rescues: `CompositionHelper
* .keydown()` calls `_handleAnyTextareaChanges()`, which snapshots
* `textarea.value` and diffs it on a 0 ms timer, emitting the difference
* itself. Measured in headless chromium against a real terminal: for a 229
* keydown xterm emits and this controller correctly stands down. What is left
* unrescued is a refused `insertText` where NO 229 diff was scheduled — that is
* the case this module exists for, and the case its browser test asserts by
* checking WHO delivered the byte rather than merely that one arrived.
*
* The recovery never guesses the character: the `input` event already carries
* the real committed text in `ev.data`, which is exactly what xterm itself
* would have forwarded. We only decide WHETHER to forward it, by asking
* whether xterm produced any canonical data since the keydown that started the
* keystroke. That snapshot must be taken at KEYDOWN, not at the input event:
* xterm's `_keyPress` emits and sets `_keyPressHandled` before `input` fires,
* so a snapshot read at input time would already contain that emission and the
* character would be delivered twice.
*
* Listener registration is load-bearing, in BOTH phase and order. xterm
* registers its own `input` listener in `terminal.open()` with `capture:
* true`, and ours is added afterwards, so at-target it runs second. It must
* also be a CAPTURE listener; see the measured table at the addEventListener
* call below.
*
* @dependency none (standalone IIFE; consumed by terminal-ui.js)
* @loadorder 5.55 (before app.js/terminal-ui.js, which create the controller)
*/
(function (global) {
'use strict';
function create(options) {
const textarea = options?.textarea;
const emitRecovered = options?.emitRecovered;
if (!textarea?.addEventListener || !textarea?.removeEventListener || typeof emitRecovered !== 'function') {
return null;
}
const isScreenReaderMode = options.isScreenReaderMode;
const setTimer = options.setTimer || global.setTimeout.bind(global);
const clearTimer = options.clearTimer || global.clearTimeout.bind(global);
let destroyed = false;
// Number of canonical data events xterm has emitted, bumped by the caller's
// onData hook. Only its ORDER relative to a keydown matters.
let canonicalCount = 0;
// ⚠️ 0, never null. With `null` the `?? canonicalCount` fallback at the input
// event reads a count xterm has ALREADY bumped: on a fresh page load with no
// keydown yet (dictation, Android voice typing, any `insertText` with no key
// held) xterm's own capture listener runs first, forwards the text itself and
// bumps the counter, then this snapshot equals it, `count > snapshot` is false,
// and the text is emitted a SECOND time. A baseline of 0 makes that comparison
// true and stands the recovery down, which restores this file's invariant: a
// missed recovery is acceptable, a duplicated keystroke is not.
let keydownSnapshot = 0;
let composing = false;
const pending = [];
function cancelPending() {
for (const candidate of pending.splice(0)) {
candidate.active = false;
if (candidate.timer !== null) {
try {
clearTimer(candidate.timer);
} catch {
// A broken timer host must not break input handling.
}
candidate.timer = null;
}
}
}
function resolveCandidate(candidate) {
const index = pending.indexOf(candidate);
if (index !== -1) pending.splice(index, 1);
candidate.timer = null;
if (!candidate.active || destroyed) return;
candidate.active = false;
// xterm (or its keypress path) spoke for this keystroke — it is already
// on its way to the PTY, so there is nothing to recover.
if (canonicalCount > candidate.snapshot) return;
try {
emitRecovered(candidate.data);
} catch {
// Recovery is best effort; a failed delivery must never throw into the
// browser's input handling.
}
}
/** Called from xterm's onData hook: xterm produced canonical data. */
function notifyCanonicalData() {
canonicalCount += 1;
}
/**
* Snapshot the canonical counter at every keydown. This deliberately reads
* NOTHING else off the event — not `key`, not `keyCode`. Gating it on
* keyCode 229 would make the recovery inert on exactly the devices it
* exists for, whose keydowns report `key: 'Unidentified'`. It is a single
* assignment, so running it for every keydown costs nothing.
*/
function handleKeyEvent(event) {
if (destroyed || event?.type !== 'keydown') return;
keydownSnapshot = canonicalCount;
}
function onInput(event) {
if (destroyed || composing || event?.isComposing) return;
if (event.inputType !== 'insertText') return;
const data = event.data;
if (typeof data !== 'string' || data === '') return;
try {
if (isScreenReaderMode?.()) return;
} catch {
return;
}
const candidate = {
data,
snapshot: keydownSnapshot ?? canonicalCount,
active: true,
timer: null,
};
pending.push(candidate);
try {
candidate.timer = setTimer(() => resolveCandidate(candidate), 0);
} catch {
cancelPending();
}
}
function onCompositionStart() {
if (destroyed) return;
composing = true;
cancelPending();
}
function onCompositionEnd() {
if (destroyed) return;
composing = false;
}
function destroy() {
if (destroyed) return;
destroyed = true;
cancelPending();
try {
textarea.removeEventListener('input', onInput, true);
textarea.removeEventListener('compositionstart', onCompositionStart, true);
textarea.removeEventListener('compositionend', onCompositionEnd, true);
} catch {
// Teardown is best effort; the terminal is being replaced anyway.
}
}
// capture: true, not bubble. The target (the textarea) is visited TWICE in
// the event path, so a capture-phase listener on it calling
// stopPropagation() still stops later BUBBLE-phase listeners on that same
// target. xterm's `_inputEvent` calls `this.cancel(ev)` (preventDefault +
// stopPropagation) exactly in the branch where it HANDLED the input, so on
// bubble we would never see handled events — and whether we saw them at
// all would hang off xterm's `options.cancelEvents`, which Codeman does not
// set. Measured (jsdom and headless chromium agree):
//
// capture-then-BUBBLE, no stop: xterm -> ours
// capture-then-BUBBLE, stopPropagation: xterm (ours never fires)
// capture-then-CAPTURE, no stop: xterm -> ours
// capture-then-CAPTURE, stopPropagation: xterm -> ours (still fires)
//
// On capture we therefore observe EVERY input event uniformly, and the
// canonicalCount snapshot alone decides whether to forward.
try {
textarea.addEventListener('input', onInput, true);
textarea.addEventListener('compositionstart', onCompositionStart, true);
textarea.addEventListener('compositionend', onCompositionEnd, true);
} catch {
destroy();
return null;
}
return Object.freeze({ handleKeyEvent, notifyCanonicalData, destroy });
}
global.CodemanKeyCode229Recovery = Object.freeze({ create });
})(typeof window !== 'undefined' ? window : globalThis);