mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 16:59:43 +02:00
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>
This commit is contained in:
Codeman maintainer
@@ -1,5 +0,0 @@
|
||||
---
|
||||
"aicodeman": patch
|
||||
---
|
||||
|
||||
Fix text being duplicated when an Android keyboard autocorrects while typing straight into the terminal prompt. SwiftKey and Gboard autocorrect on space by deleting the word and inserting the corrected one; xterm answered by sending the whole helper-textarea value (it diffs with `newValue.replace(oldValue, '')`, which only works for appends) and then the inserted text a second time, so `testing the peompt` + space reached the shell as `testing the peompttesting the prompt rompt `. A multi-character delete was also sent as a single DEL. The keyCode-229 controller now replaces xterm's `_handleAnyTextareaChanges` with an edit-based diff against the value already sent (DEL per deleted character, then the new text, once), and puts xterm's own handler back on teardown.
|
||||
@@ -327,7 +327,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-tile.js`(7.4) → `terminal-split.js`(7.5) → `tile-grid.js`(7.6) → `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.
|
||||
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-tile.js`(7.4) → `terminal-split.js`(7.5) → `tile-grid.js`(7.6) → `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 edit-based diff described below is settled first at that keydown, so the drain decides with the evidence the timer had and Enter's textarea clear cannot turn the pending line into DELs. 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 (except a composition xterm finalizes synchronously at that key, which stays xterm's). `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)
|
||||
|
||||
|
||||
@@ -180,7 +180,7 @@
|
||||
// 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();
|
||||
settleEdit(event);
|
||||
flushPending();
|
||||
keydownSnapshot = canonicalCount;
|
||||
}
|
||||
@@ -266,7 +266,7 @@
|
||||
};
|
||||
|
||||
// Apply the pending edit NOW instead of on its timer (see handleKeyEvent).
|
||||
settleEdit = () => {
|
||||
settleEdit = (event) => {
|
||||
if (waiting.size === 0) return;
|
||||
for (const entry of waiting) {
|
||||
try {
|
||||
@@ -275,7 +275,16 @@
|
||||
// A broken timer host must not break input handling.
|
||||
}
|
||||
}
|
||||
// Cleared BEFORE the return below, on purpose: left pending, the timer would fire after
|
||||
// Enter clears the textarea and send one DEL per character ahead of the submitted line.
|
||||
waiting.clear();
|
||||
// A composition just ended and this key makes xterm finalize it SYNCHRONOUSLY, through
|
||||
// `_finalizeComposition(false)`, which ignores `_dataAlreadySent`: that text is xterm's, and
|
||||
// sending the edit too would deliver it twice. On 229, CapsLock and the modifiers xterm keeps
|
||||
// the composition on its async path, which honours `_dataAlreadySent`, so the edit still
|
||||
// applies there. Only this settle path is guarded: on the timer path xterm always finalizes
|
||||
// asynchronously, and skipping the edit there would drop a byte master delivers.
|
||||
if (helper._isSendingComposition && ![229, 20, 16, 17, 18].includes(event?.keyCode)) return;
|
||||
applyEdit();
|
||||
};
|
||||
|
||||
|
||||
@@ -571,6 +571,47 @@ describe('edit-based sync of the helper textarea (autocorrect replacements)', ()
|
||||
expect(h.sent.join('')).toBe('o');
|
||||
});
|
||||
|
||||
// compositionend and the Enter keydown in one task: xterm's keydown then finalizes the
|
||||
// composition SYNCHRONOUSLY via `_finalizeComposition(false)`, which ignores `_dataAlreadySent`,
|
||||
// so the settle must leave that text to xterm or it is sent twice.
|
||||
it('does not resend a composition xterm finalizes itself at the Enter keydown', () => {
|
||||
const h = editSyncHarness();
|
||||
const controller = h.create(true);
|
||||
typeKeys(h, 'ab ');
|
||||
h.keydown();
|
||||
h.edit('ab word');
|
||||
(h.helper as any)._isSendingComposition = true;
|
||||
controller.handleKeyEvent({ type: 'keydown', key: 'Enter', keyCode: 13 });
|
||||
h.flush();
|
||||
expect(h.sent.join('')).toBe('ab ');
|
||||
});
|
||||
|
||||
// On the timer path (and at a 229 keydown) xterm finalizes the composition ASYNCHRONOUSLY and
|
||||
// skips `_dataAlreadySent`, so the edit must still be applied there: guarding it would drop the
|
||||
// non-composing `x` typed before the composition.
|
||||
it('still applies the edit while xterm finalizes a composition asynchronously', () => {
|
||||
const h = editSyncHarness();
|
||||
h.create(true);
|
||||
typeKeys(h, 'ab ');
|
||||
h.keydown();
|
||||
h.edit('ab xword');
|
||||
(h.helper as any)._isSendingComposition = true;
|
||||
h.flush();
|
||||
expect(h.sent.join('')).toBe('ab xword');
|
||||
expect(h.helper._dataAlreadySent).toBe('xword');
|
||||
|
||||
const k = editSyncHarness();
|
||||
const controller = k.create(true);
|
||||
typeKeys(k, 'ab ');
|
||||
k.keydown();
|
||||
k.edit('ab xword');
|
||||
(k.helper as any)._isSendingComposition = true;
|
||||
controller.handleKeyEvent({ type: 'keydown', key: 'Unidentified', keyCode: 229 });
|
||||
k.flush();
|
||||
expect(k.sent.join('')).toBe('ab xword');
|
||||
expect(k.helper._dataAlreadySent).toBe('xword');
|
||||
});
|
||||
|
||||
it('control: xterm alone duplicates the line when the keyboard autocorrects', () => {
|
||||
const h = editSyncHarness();
|
||||
h.create(false);
|
||||
|
||||
@@ -49,18 +49,19 @@ describe('xterm private-API dependency guard', () => {
|
||||
'xterm moved off the verified version — re-verify _kickRenderer in a real browser ' +
|
||||
'(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 ' +
|
||||
'_handleAnyTextareaChanges, _coreService, _isComposing, _isSendingComposition, _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.
|
||||
// append-only diff duplicates). It reaches `_compositionHelper`, `_coreService`, `_isComposing`,
|
||||
// `_isSendingComposition` and `_dataAlreadySent`; if xterm renames any of them the install quietly
|
||||
// falls back to xterm's own handler and the duplication returns (or, for `_isSendingComposition`, a
|
||||
// composition xterm finalizes at an Enter keydown is sent twice). 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 [
|
||||
@@ -68,6 +69,7 @@ describe('xterm private-API dependency guard', () => {
|
||||
'_compositionHelper',
|
||||
'_coreService',
|
||||
'_isComposing',
|
||||
'_isSendingComposition',
|
||||
'_dataAlreadySent',
|
||||
]) {
|
||||
expect(
|
||||
|
||||
Reference in New Issue
Block a user