diff --git a/CLAUDE.md b/CLAUDE.md index a665ae61..0e8d84f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 ``; 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) diff --git a/src/web/public/terminal-keycode229-recovery.js b/src/web/public/terminal-keycode229-recovery.js index d45fda10..b2cd94d6 100644 --- a/src/web/public/terminal-keycode229-recovery.js +++ b/src/web/public/terminal-keycode229-recovery.js @@ -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; }; } diff --git a/test/terminal-keycode229-recovery.browser.test.ts b/test/terminal-keycode229-recovery.browser.test.ts index ca22df1a..04d1dc8f 100644 --- a/test/terminal-keycode229-recovery.browser.test.ts +++ b/test/terminal-keycode229-recovery.browser.test.ts @@ -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()); diff --git a/test/terminal-keycode229-recovery.test.ts b/test/terminal-keycode229-recovery.test.ts index f8250e44..7030847d 100644 --- a/test/terminal-keycode229-recovery.test.ts +++ b/test/terminal-keycode229-recovery.test.ts @@ -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); diff --git a/test/xterm-private-api.test.ts b/test/xterm-private-api.test.ts index a02f51df..dda37c26 100644 --- a/test/xterm-private-api.test.ts +++ b/test/xterm-private-api.test.ts @@ -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', () => {