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
This commit is contained in:
Devvyn
2026-10-06 17:56:16 +08:00
co-authored by Claude Sonnet 5.5
parent 0c71b753ef
commit f21ab39a89
5 changed files with 215 additions and 23 deletions
+1 -1
View File
@@ -325,7 +325,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
### Frontend
Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. Load order: `constants.js`(1) → `i18n.js`(1.5) → `mobile-handlers.js`(2) → `voice-input.js`(3) → `notification-manager.js`(4) → `keyboard-accessory.js`(5) → `input-cjk.js`(5.5) → `mobile-ime-preview.js`(5.52) → `terminal-keycode229-recovery.js`(5.55) → `sanitize-html.js`(5.6) → `tab-layout-browser.js`(5.9) → `app.js`(6) → `tab-rail-resize.js`(6.5) → `terminal-ui.js`(7) → `terminal-split.js`(7.5) → `respawn-ui.js`(8) → `ralph-panel.js`(9) → `orchestrator-panel.js`(9.5) → `cron-ui.js`(9.7) → `settings-ui.js`(10) → `panels-ui.js`(11) → `readmymind-ui.js`(11.3) → `ultracode-panel.js`(11.5) → `approvals-ui.js`(11.6) → `reboot-restore-ui.js`(11.65) → `admin-ui.js`(11.7) → `session-ui.js`(12) → `host-wake-ui.js`(12.2) → `webview-tabs.js`(12.5) → `mobile-overview.js`(12.55) → `home-sessions.js`(12.56) → `git-status-ui.js`(12.57) → `entrance-animations.js`(12.6) → `ralph-wizard.js`(13) → `api-client.js`(14) → `subagent-windows.js`(15) → `ultracode-windows.js`(15.5) → `session-lineage.js`(15.6) → `image-input.js`(16). `i18n.js` translates static + newly inserted application DOM while skipping terminal/response/file/user-name surfaces; `input-cjk.js` handles CJK IME composition via an always-visible textarea below the terminal (`window.cjkActive` blocks xterm's onData). `terminal-keycode229-recovery.js` forwards a committed `input` event that xterm's `_inputEvent` guard drops (Chrome-on-Android soft keyboards send `composed: true` after a keydown), and only when xterm emitted no canonical data for that keystroke. ⚠️ **That decision is settled at the NEXT keydown as well as on its own zero-delay timer** (#441): the drain runs from xterm's custom key handler, which fires BEFORE xterm processes that key, so a soft keyboard that commits the last character and sends Enter in one InputConnection transaction puts the character on the wire ahead of the `\r`. On the timer alone that character is not merely late, it is LOST: xterm emits the `\r` first and bumps the canonical counter past the candidate's snapshot, so the candidate stands down (measured, `hell\r` where the user typed `hello`). The trade is that a keydown decides with less evidence than the timer did, since xterm's own keyCode-229 rescue has not run yet; that is safe for Enter, which clears the textarea so the pending diff emits nothing. Ordering is pinned by `test/terminal-keycode229-recovery.browser.test.ts`, which the CI gate does NOT run. `mobile-ime-preview.js` (iOS WebKit only) paints the text an IME is composing: an iOS IME commit is routed into the local-echo overlay through the ordinary printable/paste branch and then `_transferMobileImeCommitToLocalEcho`, and without local echo the preview clears only on output parsed AFTER the commit (or its 2 s fallback). ⚠️ It watches keydown in the capture phase on `terminal.element`, never on the textarea, because xterm finalizes the composition and emits the commit in its own capture listener on the textarea.
Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. Load order: `constants.js`(1) → `i18n.js`(1.5) → `mobile-handlers.js`(2) → `voice-input.js`(3) → `notification-manager.js`(4) → `keyboard-accessory.js`(5) → `input-cjk.js`(5.5) → `mobile-ime-preview.js`(5.52) → `terminal-keycode229-recovery.js`(5.55) → `sanitize-html.js`(5.6) → `tab-layout-browser.js`(5.9) → `app.js`(6) → `tab-rail-resize.js`(6.5) → `terminal-ui.js`(7) → `terminal-split.js`(7.5) → `respawn-ui.js`(8) → `ralph-panel.js`(9) → `orchestrator-panel.js`(9.5) → `cron-ui.js`(9.7) → `settings-ui.js`(10) → `panels-ui.js`(11) → `readmymind-ui.js`(11.3) → `ultracode-panel.js`(11.5) → `approvals-ui.js`(11.6) → `reboot-restore-ui.js`(11.65) → `admin-ui.js`(11.7) → `session-ui.js`(12) → `host-wake-ui.js`(12.2) → `webview-tabs.js`(12.5) → `mobile-overview.js`(12.55) → `home-sessions.js`(12.56) → `git-status-ui.js`(12.57) → `entrance-animations.js`(12.6) → `ralph-wizard.js`(13) → `api-client.js`(14) → `subagent-windows.js`(15) → `ultracode-windows.js`(15.5) → `session-lineage.js`(15.6) → `image-input.js`(16). `i18n.js` translates static + newly inserted application DOM while skipping terminal/response/file/user-name surfaces; `input-cjk.js` handles CJK IME composition via an always-visible textarea below the terminal (`window.cjkActive` blocks xterm's onData). `terminal-keycode229-recovery.js` forwards a committed `input` event that xterm's `_inputEvent` guard drops (Chrome-on-Android soft keyboards send `composed: true` after a keydown), and only when xterm emitted no canonical data for that keystroke. ⚠️ **That decision is settled at the NEXT keydown as well as on its own zero-delay timer** (#441): the drain runs from xterm's custom key handler, which fires BEFORE xterm processes that key, so a soft keyboard that commits the last character and sends Enter in one InputConnection transaction puts the character on the wire ahead of the `\r`. On the timer alone that character is not merely late, it is LOST: xterm emits the `\r` first and bumps the canonical counter past the candidate's snapshot, so the candidate stands down (measured, `hell\r` where the user typed `hello`). The trade is that a keydown decides with less evidence than the timer did, since xterm's own keyCode-229 rescue has not run yet; that is safe for Enter, which clears the textarea so the pending diff emits nothing. Ordering is pinned by `test/terminal-keycode229-recovery.browser.test.ts`, which the CI gate does NOT run. The same module replaces xterm's `_handleAnyTextareaChanges` (an append-only `newValue.replace(oldValue, '')` diff) with an edit-based one, so an Android autocorrect on space (delete + insert) reaches the PTY once instead of duplicating the line; ⚠️ that diff is settled at the NEXT keydown, before xterm handles that key, because xterm clears the textarea for Enter and a pending diff would then send one DEL per character ahead of the submitted line. `mobile-ime-preview.js` (iOS WebKit only) paints the text an IME is composing: an iOS IME commit is routed into the local-echo overlay through the ordinary printable/paste branch and then `_transferMobileImeCommitToLocalEcho`, and without local echo the preview clears only on output parsed AFTER the commit (or its 2 s fallback). ⚠️ It watches keydown in the capture phase on `terminal.element`, never on the textarea, because xterm finalizes the composition and emits the commit in its own capture listener on the textarea.
**Entrance animations** (`entrance-animations.js`, all OFF by default): opt-in animations for tabs, terminal, windows and connection lines, chosen via `data-tab-anim` / `data-term-anim` / `data-win-anim` / `data-line-anim` on `<html>`; the default `legacy` theme short-circuits every hook. ⚠️ Tabs and lines are destroyed mid-animation on re-render, so re-apply to the fresh element by id with a negative `animation-delay` (resume, never restart). ⚠️ Terminal-pane styles may animate only transform / opacity / clip-path (anything else resizes the PTY via FitAddon); `blur` is the ONE sanctioned `filter` exception, do not generalise it. ⚠️ Line glow lives in `--line-glow` so blur keyframes interpolate. Persisted per-device in `codeman:*Anim` localStorage keys, never in `SettingsUpdateSchema`; lab at `?animlab=1`. Test: `test/entrance-animations.test.ts`. → [architecture-invariants#entrance-animations](docs/architecture-invariants.md#entrance-animations)
+52 -21
View File
@@ -89,6 +89,9 @@
let keydownSnapshot = 0;
let composing = false;
const pending = [];
// Applies any edit-sync diff still waiting on its timer. Assigned by installEditSync() below; a
// no-op when xterm's internals are not available.
let settleEdit = () => {};
/**
* Resolve every candidate still pending, right now, instead of waiting for
@@ -171,6 +174,13 @@
// reach the PTY — see flushPending(). This runs from xterm's custom key
// handler, i.e. before xterm processes the key, so a recovered character
// is always ordered ahead of the bytes this keydown produces.
// ORDER MATTERS. Settle the edit-sync diff first: it bumps `canonicalCount` for the
// keystroke it belongs to, so flushPending() then stands that keystroke's orphan candidate
// down. Swapped, the candidate would resolve first and the character would be sent twice.
// It also has to happen BEFORE xterm handles THIS key: for Enter, xterm clears the textarea
// in its own keydown, and a timer left pending would then diff the whole line against ''
// and send one DEL per character ahead of the submitted line.
settleEdit();
flushPending();
keydownSnapshot = canonicalCount;
}
@@ -219,37 +229,58 @@
if (typeof original !== 'function' || typeof coreService?.triggerDataEvent !== 'function') return null;
let synced = textarea.value;
let outstanding = 0;
const waiting = new Set();
/** Send what changed since `synced`, once, and remember it. */
function applyEdit() {
if (destroyed || helper._isComposing) return; // xterm's composition path owns this one
const current = textarea.value;
if (current === synced) return;
const { deleted, inserted } = editBetween(synced, current);
synced = current;
try {
// One DEL per character, like repeated backspace presses: the local-echo composer and
// the PTY both treat each as a single edit.
for (let i = 0; i < deleted; i += 1) coreService.triggerDataEvent('\x7f', true);
if (inserted) {
helper._dataAlreadySent = inserted;
coreService.triggerDataEvent(inserted, true);
}
} catch {
// Delivery is best effort; never throw into the browser's timer queue.
}
}
helper._handleAnyTextareaChanges = function handleAnyTextareaChanges() {
if (destroyed) return original.call(this);
// No edit in flight and the value is not what we last sent: something outside the IME
// changed it (xterm clears it after Enter, a composition committed). Nothing to send;
// resynchronise.
if (outstanding === 0 && synced !== textarea.value) synced = textarea.value;
outstanding += 1;
setTimer(() => {
outstanding -= 1;
if (destroyed || helper._isComposing) return; // xterm's composition path owns this one
const current = textarea.value;
if (current === synced) return;
const { deleted, inserted } = editBetween(synced, current);
synced = current;
try {
// One DEL per character, like repeated backspace presses: the local-echo composer and
// the PTY both treat each as a single edit.
for (let i = 0; i < deleted; i += 1) coreService.triggerDataEvent('\x7f', true);
if (inserted) {
helper._dataAlreadySent = inserted;
coreService.triggerDataEvent(inserted, true);
}
} catch {
// Delivery is best effort; never throw into the browser's timer queue.
}
if (waiting.size === 0 && synced !== textarea.value) synced = textarea.value;
const entry = { id: null };
waiting.add(entry);
entry.id = setTimer(() => {
waiting.delete(entry);
applyEdit();
}, 0);
};
// Apply the pending edit NOW instead of on its timer (see handleKeyEvent).
settleEdit = () => {
if (waiting.size === 0) return;
for (const entry of waiting) {
try {
clearTimer(entry.id);
} catch {
// A broken timer host must not break input handling.
}
}
waiting.clear();
applyEdit();
};
return () => {
settleEdit = () => {};
if (helper._handleAnyTextareaChanges !== original) helper._handleAnyTextareaChanges = original;
};
}
@@ -301,6 +301,91 @@ describe('orphaned terminal input recovery wiring', () => {
expect(line).toBe('testing the prompt ');
});
/**
* The batched Android shape from #441, now with an edit that REWRITES text: the last
* character's 229 keydown and insertText land in the same page task as Enter's keydown.
* xterm clears the textarea for Enter before the edit-sync timer would run, so a timer
* left pending diffed the whole line against '' and sent one DEL per character ahead of
* the submitted line (with local echo on, `hello` + Enter submitted `h`). The edit is
* settled at the next keydown, before xterm sees it. Run with local echo both ways: it
* changes which of the DELs and the `\r` reaches the wire first.
*/
async function lastEditThenEnter(options: { localEcho: boolean; autocorrect: boolean }) {
return page.evaluate(async ({ localEcho, autocorrect }) => {
const app = (window as any).app;
const textarea = document.querySelector('.xterm-helper-textarea') as HTMLTextAreaElement;
const originalSessionId = app.activeSessionId;
const originalLocalEcho = app._localEchoEnabled;
const originalSendInput = app._sendInputAsync;
const originalPendingInput = app._pendingInput;
const originalLastKeystrokeTime = app._lastKeystrokeTime;
const sent: string[] = [];
const keydown = (init: KeyboardEventInit, keyCode: number) => {
const down = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, ...init });
Object.defineProperties(down, { keyCode: { value: keyCode }, which: { value: keyCode } });
textarea.dispatchEvent(down);
};
const key229 = () => keydown({ key: 'Unidentified' }, 229);
const tick = () => new Promise((resolve) => setTimeout(resolve, 20));
try {
app.activeSessionId = 'cod388-browser-edit-enter';
app._localEchoEnabled = localEcho;
app._pendingInput = '';
app._lastKeystrokeTime = 0;
app._sendInputAsync = (_sessionId: string, chunk: string) => sent.push(chunk);
textarea.value = '';
textarea.focus();
const typed = autocorrect ? 'testing the peompt' : 'hell';
for (const ch of typed) {
key229();
document.execCommand('insertText', false, ch);
await tick();
}
// ONE task: no awaits between the edit(s) and Enter.
if (autocorrect) {
key229();
textarea.setSelectionRange(textarea.value.length - 5, textarea.value.length);
document.execCommand('delete');
key229();
document.execCommand('insertText', false, 'rompt ');
} else {
key229();
document.execCommand('insertText', false, 'o');
}
keydown({ key: 'Enter', code: 'Enter' }, 13);
await new Promise((resolve) => setTimeout(resolve, 250)); // local echo delays the \r by 80 ms
const line: string[] = [];
for (const ch of sent.join('')) {
if (ch === '\x7f') line.pop();
else line.push(ch);
}
return { raw: sent.join(''), line: line.join('') };
} finally {
app.activeSessionId = originalSessionId;
app._localEchoEnabled = originalLocalEcho;
app._sendInputAsync = originalSendInput;
app._pendingInput = originalPendingInput;
app._lastKeystrokeTime = originalLastKeystrokeTime;
textarea.value = '';
}
}, options);
}
for (const localEcho of [true, false]) {
it(`a 229 last character in the same task as Enter submits the whole line (local echo ${localEcho ? 'on' : 'off'})`, async () => {
const { raw, line } = await lastEditThenEnter({ localEcho, autocorrect: false });
expect(raw).not.toContain('\x7f');
expect(line).toBe('hello\r');
});
it(`an autocorrect plus Enter in one task submits the corrected line (local echo ${localEcho ? 'on' : 'off'})`, async () => {
const { line } = await lastEditThenEnter({ localEcho, autocorrect: true });
expect(line).toBe('testing the prompt \r');
});
}
it('control: without the edit sync, xterm alone reproduces the duplicated line', async () => {
// destroy() puts xterm's own handler back. Keep this LAST: it leaves the controller off.
await page.evaluate(() => (window as any).app._keyCode229Recovery.destroy());
+51
View File
@@ -520,6 +520,57 @@ describe('edit-based sync of the helper textarea (autocorrect replacements)', ()
h.flush();
};
// The batched Android shape (#441): the last character's keydown and `insertText` arrive in the
// SAME page task as Enter's keydown. xterm's own Enter handling clears the textarea before the
// edit's timer runs, so a timer left pending would diff the whole line against '' and send one
// DEL per character AHEAD of the submitted line. The edit is therefore settled at the next
// keydown, before xterm sees that key.
it('settles a pending edit at the next keydown, so Enter in the same task cannot erase the line', () => {
const h = editSyncHarness();
const controller = h.create(true);
typeKeys(h, 'hell');
h.keydown();
h.edit('hello');
controller.handleKeyEvent({ type: 'keydown', key: 'Enter', keyCode: 13 });
h.textarea.value = ''; // xterm's CR handling, which runs after the custom key handler
h.flush();
expect(h.sent.join('')).toBe('hello');
expect(h.sent).not.toContain('\x7f');
});
it('autocorrect and Enter in one task submits the corrected line, not a run of DELs', () => {
const h = editSyncHarness();
const controller = h.create(true);
typeKeys(h, 'testing the peompt');
h.keydown();
h.edit('testing the p');
h.keydown();
h.edit('testing the prompt ');
controller.handleKeyEvent({ type: 'keydown', key: 'Enter', keyCode: 13 });
h.textarea.value = '';
h.flush();
expect(h.line()).toBe('testing the prompt ');
expect(h.sent.filter((c) => c === '\x7f')).toHaveLength(5); // the five deleted characters, nothing more
});
it('settling first stands the same keystroke’s orphan candidate down (no double send)', () => {
const h = editSyncHarness();
const controller = h.create(true);
// xterm's canonical-data hook, as terminal-ui.js wires it.
const origTrigger = h.helper._coreService.triggerDataEvent;
h.helper._coreService.triggerDataEvent = (data: string) => {
origTrigger(data);
controller.notifyCanonicalData();
};
controller.handleKeyEvent({ type: 'keydown', key: 'Unidentified', keyCode: 229 });
h.keydown();
h.edit('o');
h.textarea.fire('input', inputEvent('o'));
controller.handleKeyEvent({ type: 'keydown', key: 'Enter', keyCode: 13 });
h.flush();
expect(h.sent.join('')).toBe('o');
});
it('control: xterm alone duplicates the line when the keyboard autocorrects', () => {
const h = editSyncHarness();
h.create(false);
+26 -1
View File
@@ -47,12 +47,37 @@ describe('xterm private-API dependency guard', () => {
expect(
lock.packages['node_modules/@xterm/xterm']?.version,
'xterm moved off the verified version — re-verify _kickRenderer in a real browser ' +
'(terminal-ui.js: _core._renderService._renderDebouncer._animationFrame), then update ' +
'(terminal-ui.js: _core._renderService._renderDebouncer._animationFrame) AND the ' +
'CompositionHelper fields installEditSync() uses (terminal-keycode229-recovery.js: ' +
'_handleAnyTextareaChanges, _coreService, _isComposing, _dataAlreadySent), then update ' +
'VERIFIED_XTERM_VERSION here. The accessor is optional-chained, so a renamed field ' +
'degrades to a silent no-op and the freeze it heals comes back unnoticed.'
).toBe(VERIFIED_XTERM_VERSION);
});
// terminal-keycode229-recovery.js swaps in an edit-based replacement for xterm's
// CompositionHelper._handleAnyTextareaChanges (Android autocorrect = delete + insert, which xterm's
// append-only diff duplicates). It reaches `_compositionHelper`, `_coreService`, `_isComposing`
// and `_dataAlreadySent`; if xterm renames any of them the install quietly falls back to xterm's own
// handler and the duplication returns. Property names survive minification, so a string check on
// the shipped bundle catches a rename on upgrade.
it('still ships the composition-helper fields the edit-based 229 sync depends on', () => {
const bundle = readFileSync(resolve(root, 'node_modules/@xterm/xterm/lib/xterm.js'), 'utf8');
for (const name of [
'_handleAnyTextareaChanges',
'_compositionHelper',
'_coreService',
'_isComposing',
'_dataAlreadySent',
]) {
expect(
bundle,
`xterm no longer mentions ${name}: re-verify terminal-keycode229-recovery.js installEditSync() ` +
'(src/web/public) against the new CompositionHelper before bumping VERIFIED_XTERM_VERSION'
).toContain(name);
}
});
// If someone deletes the watchdog, this guard is pointless noise — keep the
// two tied together so the range check cannot outlive what it protects.
it('is guarding a watchdog that still exists', () => {