From 5dec501c6e2fdf6fd9cb2a7322b351854c4382f8 Mon Sep 17 00:00:00 2001 From: hanhvs Date: Tue, 30 Jun 2026 12:01:19 +0700 Subject: [PATCH] fix(tui): stop Vietnamese Telex IME from dropping characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third-party Vietnamese IMEs (OpenKey/Unikey/EVKey in Telex mode) recompose a syllable by emitting an erase burst followed by the finished characters. Two layers of the TUI input pipeline mishandled this, dropping letters and leaving a stray space mid-syllable (e.g. "hạnh" rendered as "hạ ", and "vương sỹ hạnh" as "vương sỹ hạ "). Root causes, both confirmed from real captured byte streams: 1. parse-keypress: an IME often fuses a control byte (\x7f/\b, or even the U+202F marker OpenKey injects) with the recomposed text in a single stdin read. parseKeypress only recognizes a control key when the whole string is exactly that byte, so a mixed chunk fell through every branch, returned name:"" with a non-printable sequence, and the composer's printable gate discarded the entire chunk — taking the surrounding letters with it. Split text tokens on every control byte so the printable runs survive. CR/LF are deliberately not split, preserving paste/return semantics. 2. textInput: multi-character (IME/paste) inserts were committed through the 16ms deferred key-burst path, which raced an interleaved re-render and snapped the buffer back to a stale value, dropping the recomposed tail. Commit them synchronously. Additionally, the fast-echo "\b \b" backspace shortcut desynced the screen when it ran right after an Ink repaint (forced by the U+202F marker), stranding the marker glyph; suppress fast-echo for the recompose burst that follows an Ink repaint and resume it on the next real keystroke. Tested with real OpenKey and EVKey captures of "vương sỹ hạnh" across read timings, plus parser unit coverage and an EVKey no-regression guard. --- .../src/ink/parse-keypress-drop-probe.test.ts | 71 +++++++++ .../src/ink/parse-keypress-noregress.test.ts | 40 +++++ .../hermes-ink/src/ink/parse-keypress.test.ts | 65 ++++++++ .../hermes-ink/src/ink/parse-keypress.ts | 54 ++++++- .../src/__tests__/imeVietnameseTelex.test.tsx | 141 ++++++++++++++++++ ui-tui/src/components/textInput.tsx | 51 ++++++- 6 files changed, 419 insertions(+), 3 deletions(-) create mode 100644 ui-tui/packages/hermes-ink/src/ink/parse-keypress-drop-probe.test.ts create mode 100644 ui-tui/packages/hermes-ink/src/ink/parse-keypress-noregress.test.ts create mode 100644 ui-tui/src/__tests__/imeVietnameseTelex.test.tsx diff --git a/ui-tui/packages/hermes-ink/src/ink/parse-keypress-drop-probe.test.ts b/ui-tui/packages/hermes-ink/src/ink/parse-keypress-drop-probe.test.ts new file mode 100644 index 0000000000..6cc013476d --- /dev/null +++ b/ui-tui/packages/hermes-ink/src/ink/parse-keypress-drop-probe.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest' + +import { INITIAL_STATE, parseMultipleKeypresses } from './parse-keypress.js' + +// Probe: feed many exotic IME-ish byte patterns straight through the parser +// and assert NO printable character codepoint silently vanishes. This catches +// the "chunk falls through every branch -> name:'' with a non-printable +// sequence -> composer discards it" failure class for sequences we haven't +// hand-enumerated. + +function keysToText(keys: Array<{ name?: string; sequence?: string }>): string { + // Reconstruct what the composer would insert: backspaces delete, everything + // else with a printable sequence inserts its sequence. + let out = '' + for (const k of keys) { + if (k.name === 'backspace') { + out = out.slice(0, -1) + continue + } + const seq = k.sequence ?? '' + // Mirror the composer's PRINTABLE gate + if (/^[ -~\u00a0-\uffff]+$/.test(seq)) { + out += seq + } else if (seq) { + // Non-printable, non-backspace => composer drops it. Mark it so the + // assertion can show what was lost. + out += `«DROP:${[...seq].map(c => 'U+' + c.codePointAt(0)!.toString(16)).join(',')}»` + } + } + return out +} + +const cases: Array<[string, string, string]> = [ + // [label, input bytes, expected text after composer-emulation] + ['fused bs+char', '\x7fô', 'ô'], // starts empty, bs no-ops in our emul + ['fused bs+2char', '\x7fôi', 'ôi'], + ['embedded bs', 'ab\bç', 'aç'], + ['hard-erase \\b \\b + char', '\b \bô', 'ô'], + ['hard-erase x3 + ạnh (from anh)', 'anh\b \b\b \b\b \bạnh', 'ạnh'], + ['DEL-space-DEL + char', '\x7f \x7fô', 'ô'], + ['trailing text after multi DEL', '\x7f\x7f\x7fươn', 'ươn'], + ['char then DEL then char fused', 'o\x7fô', 'ô'], + ['multiple syllable fused', 'vuon\x7f\x7f\x7fương', 'vương'], + // CR/LF are intentionally NOT split (preserve paste/return semantics), so a + // text token with an embedded CR is left whole; assert it is NOT split into + // surviving letters here — that path is covered by the composer's return / + // paste handling, not parseTextKeypresses. +] + +describe('parser does not silently drop printable codepoints', () => { + for (const [label, input, expected] of cases) { + it(label, () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, input) + const text = keysToText(keys as Array<{ name?: string; sequence?: string }>) + expect(text, `keys=${JSON.stringify(keys)}`).toBe(expected) + }) + } + + it('exhaustive: DEL between every pair of letters never drops a letter', () => { + const letters = [...'aăâeêioôơuưy'] + for (const a of letters) { + for (const b of letters) { + const input = `${a}\x7f${b}` + const [keys] = parseMultipleKeypresses(INITIAL_STATE, input) + const text = keysToText(keys as Array<{ name?: string; sequence?: string }>) + // a inserted, bs removes a, b inserted => "b" + expect(text, `input=${JSON.stringify(input)} keys=${JSON.stringify(keys)}`).toBe(b) + } + } + }) +}) diff --git a/ui-tui/packages/hermes-ink/src/ink/parse-keypress-noregress.test.ts b/ui-tui/packages/hermes-ink/src/ink/parse-keypress-noregress.test.ts new file mode 100644 index 0000000000..ee6c384a32 --- /dev/null +++ b/ui-tui/packages/hermes-ink/src/ink/parse-keypress-noregress.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' + +import { INITIAL_STATE, parseMultipleKeypresses } from './parse-keypress.js' + +// Confirm the control-byte split is a NO-OP for clean input (EVKey-style: +// backspace and recomposed text arrive in separate, non-fused reads). The +// fix must not change behavior for any text token that has no embedded +// control byte — otherwise it could regress IMEs that already work. +describe('control-byte split does not touch clean (EVKey-style) input', () => { + it('a plain printable text token yields exactly one keypress (no spurious split)', () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, 'ạnh') + + expect(keys).toHaveLength(1) + expect(keys[0]).toMatchObject({ raw: 'ạnh' }) + }) + + it('a lone backspace read is unchanged', () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, '\x7f') + + expect(keys).toHaveLength(1) + expect(keys[0]).toMatchObject({ name: 'backspace' }) + }) + + it('separate clean reads (bs read, then text read) each produce one key', () => { + const [k1] = parseMultipleKeypresses(INITIAL_STATE, '\x7f') + const [k2] = parseMultipleKeypresses(INITIAL_STATE, 'ô') + + expect(k1).toHaveLength(1) + expect(k1[0]).toMatchObject({ name: 'backspace' }) + expect(k2).toHaveLength(1) + expect(k2[0]).toMatchObject({ raw: 'ô' }) + }) + + it('a full clean Vietnamese word with no embedded control bytes is one text key', () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, 'vương') + + expect(keys).toHaveLength(1) + expect(keys[0]).toMatchObject({ raw: 'vương' }) + }) +}) diff --git a/ui-tui/packages/hermes-ink/src/ink/parse-keypress.test.ts b/ui-tui/packages/hermes-ink/src/ink/parse-keypress.test.ts index fcd7090b8b..aa07bf9a09 100644 --- a/ui-tui/packages/hermes-ink/src/ink/parse-keypress.test.ts +++ b/ui-tui/packages/hermes-ink/src/ink/parse-keypress.test.ts @@ -40,6 +40,71 @@ describe('parseMultipleKeypresses bracketed paste recovery', () => { }) }) +describe('parseMultipleKeypresses text control splitting', () => { + it('keeps an IME backspace plus composed character in the same read', () => { + const [keys, state] = parseMultipleKeypresses(INITIAL_STATE, '\x7fô') + + expect(keys).toEqual([ + expect.objectContaining({ name: 'backspace', raw: '\x7f' }), + expect.objectContaining({ name: '', raw: 'ô' }) + ]) + expect(state.mode).toBe('NORMAL') + }) + + it('keeps trailing IME text after a backspace in the same read', () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, '\x7fôi') + + expect(keys).toEqual([ + expect.objectContaining({ name: 'backspace', raw: '\x7f' }), + expect.objectContaining({ name: '', raw: 'ôi' }) + ]) + }) + + it('splits embedded backspace control bytes without splitting surrounding text', () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, 'ab\bç') + + expect(keys).toEqual([ + expect.objectContaining({ name: '', raw: 'ab' }), + expect.objectContaining({ name: 'backspace', raw: '\b' }), + expect.objectContaining({ name: '', raw: 'ç' }) + ]) + }) + + it('peels off a non-backspace control byte fused with text instead of dropping the whole chunk', () => { + // An IME can fuse a control byte other than \x7f/\b with the recomposed + // text (here U+0001). The original PR only split on \x7f/\b, so a chunk + // like "a\x01b" fell through every parseKeypress branch, returned + // name:"" with a non-printable sequence, and the composer discarded the + // entire chunk — eating the printable letters 'a' and 'b' too. Every + // control byte must be peeled off so the surrounding text survives. + const [keys] = parseMultipleKeypresses(INITIAL_STATE, 'a\x01b') + + // The leading and trailing printable letters must each survive as their + // own keypress (the control byte in between parses to ctrl+a). The bug was + // the WHOLE "a\x01b" chunk collapsing into one undeliverable key. + expect(keys).toHaveLength(3) + expect(keys[0]).toMatchObject({ name: 'a', raw: 'a' }) + expect(keys[1]).toMatchObject({ raw: '\x01' }) + expect(keys[2]).toMatchObject({ name: 'b', raw: 'b' }) + }) + + it('keeps printable letters around a fused ESC control byte', () => { + const [keys] = parseMultipleKeypresses(INITIAL_STATE, 'vương\x1b') + + // The trailing printable run must still be delivered as its own key. + expect(keys.some(k => 'raw' in k && k.raw === 'vương')).toBe(true) + }) + + it('does NOT split embedded CR/LF (preserves paste/return handling)', () => { + // CR/LF inside a text token come from non-bracketed paste; splitting them + // into `return` keys would prematurely submit the composer. They must stay + // inside the single text token. + const [keys] = parseMultipleKeypresses(INITIAL_STATE, 'a\rb') + + expect(keys).toEqual([expect.objectContaining({ raw: 'a\rb' })]) + }) +}) + describe('mouse wheel modifier decoding', () => { // SGR mouse format: ESC [ < button ; col ; row M // Wheel up = 64 (0x40), wheel down = 65 (0x41). diff --git a/ui-tui/packages/hermes-ink/src/ink/parse-keypress.ts b/ui-tui/packages/hermes-ink/src/ink/parse-keypress.ts index 59981f543f..07e31c6f53 100644 --- a/ui-tui/packages/hermes-ink/src/ink/parse-keypress.ts +++ b/ui-tui/packages/hermes-ink/src/ink/parse-keypress.ts @@ -200,6 +200,58 @@ function splitNumericParams(params: string): number[] { return params.split(';').map(p => parseInt(p, 10)) } +// A text token can carry stray control bytes fused with printable input — +// most commonly when a third-party IME (Vietnamese Telex via OpenKey/Unikey/ +// EVKey, etc.) recomposes a syllable by emitting an erase control byte +// immediately followed by the finished character(s) in a single stdin read +// (e.g. "\x7fô", "ab\bç"). parseKeypress only recognizes a control key when +// the WHOLE string is exactly that control byte, so a mixed chunk falls +// through every branch and returns name:"" with a non-printable sequence, +// which the composer's PRINTABLE gate then discards — taking the surrounding +// letters down with it. Split the token so every control byte becomes its own +// keypress and the printable runs between them survive. +// +// CR (\r) and LF (\n) are deliberately NOT treated as split points: a lone +// Enter already arrives as its own read, while a newline embedded in a text +// token only happens for non-bracketed paste, where peeling it into a +// `return` keypress would prematurely submit the composer. Leaving them in +// the token preserves the existing paste/return handling byte-for-byte. +function isControlChar(ch: string): boolean { + const code = ch.charCodeAt(0) + + if (code === 0x0a || code === 0x0d) { + return false + } + + return code < 0x20 || code === 0x7f +} + +function parseTextKeypresses(text: string): ParsedKey[] { + const keys: ParsedKey[] = [] + let textStart = 0 + + for (let i = 0; i < text.length; i++) { + const ch = text[i]! + + if (!isControlChar(ch)) { + continue + } + + if (i > textStart) { + keys.push(parseKeypress(text.slice(textStart, i))) + } + + keys.push(parseKeypress(ch)) + textStart = i + 1 + } + + if (textStart < text.length) { + keys.push(parseKeypress(text.slice(textStart))) + } + + return keys +} + export type KeyParseState = { mode: 'NORMAL' | 'IN_PASTE' incomplete: string @@ -294,7 +346,7 @@ export function parseMultipleKeypresses( const resynthesized = '\x1b' + token.value keys.push(parseKeypress(resynthesized)) } else { - keys.push(parseKeypress(token.value)) + keys.push(...parseTextKeypresses(token.value)) } } } diff --git a/ui-tui/src/__tests__/imeVietnameseTelex.test.tsx b/ui-tui/src/__tests__/imeVietnameseTelex.test.tsx new file mode 100644 index 0000000000..74499f639f --- /dev/null +++ b/ui-tui/src/__tests__/imeVietnameseTelex.test.tsx @@ -0,0 +1,141 @@ +import { EventEmitter } from 'events' + +import { renderSync } from '@hermes/ink' +import React, { useState } from 'react' +import { describe, expect, it } from 'vitest' + +import { TextInput } from '../components/textInput.js' + +// End-to-end regression coverage for Vietnamese Telex IME recomposition +// (OpenKey / Unikey / EVKey). These IMEs commit a finished syllable by +// emitting a burst of backspaces (and, for OpenKey, a U+202F NARROW NO-BREAK +// SPACE marker) followed by the recomposed characters. The byte streams below +// are real captures taken from OpenKey and EVKey on macOS while typing the +// phrase "vương sỹ hạnh" (Telex: "vuonwg syx hanhj"). +// +// The bug these guard against: characters were dropped and a stray space was +// left mid-syllable (e.g. "hạnh" rendered as "hạ "). Root causes fixed: +// 1. parse-keypress split fused control-byte+text chunks so the recomposed +// text survives instead of being discarded with the control byte. +// 2. textInput commits multi-character (IME/paste) inserts synchronously +// instead of through the 16ms key-burst path that raced re-renders. + +class FakeTty extends EventEmitter { + chunks: string[] = [] + columns = 80 + rows = 24 + isTTY = true + isRaw = false + private pendingReads: string[] = [] + ref(): void {} + unref(): void {} + read(): string | null { + return this.pendingReads.shift() ?? null + } + send(chunk: string): void { + this.pendingReads.push(chunk) + this.emit('readable') + } + setEncoding(): this { + return this + } + setRawMode(mode: boolean): this { + this.isRaw = mode + + return this + } + write(chunk: string | Uint8Array, cb?: (err?: Error | null) => void): boolean { + this.chunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')) + cb?.() + + return true + } +} + +const tick = () => new Promise(resolve => setImmediate(resolve)) +const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + +function Harness({ initial = '', onValue }: { initial?: string; onValue: (value: string) => void }) { + const [value, setValue] = useState(initial) + + return React.createElement(TextInput, { + onChange: (next: string) => { + setValue(next) + onValue(next) + }, + value + }) +} + +async function drive(reads: string[], { initial = '', gapMs = 0 }: { initial?: string; gapMs?: number } = {}): Promise { + const stdout = new FakeTty() + const stdin = new FakeTty() + const stderr = new FakeTty() + const values: string[] = [] + + const instance = renderSync(React.createElement(Harness, { initial, onValue: v => values.push(v) }), { + patchConsole: false, + stderr: stderr as unknown as NodeJS.WriteStream, + stdin: stdin as unknown as NodeJS.ReadStream, + stdout: stdout as unknown as NodeJS.WriteStream + }) + + try { + await tick() + + for (const r of reads) { + stdin.send(r) + await tick() + + if (gapMs) { + await wait(gapMs) + } + } + + await wait(60) + + return values.at(-1) ?? '' + } finally { + instance.unmount() + instance.cleanup() + } +} + +const NNBSP = '\u202f' + +describe('Vietnamese Telex IME recomposition', () => { + it('applies a parser-split backspace plus composed character through useInput', async () => { + // OpenKey fuses the erase + recomposed glyph into a single stdin read. + expect(await drive(['\x7fô'], { initial: 'o' })).toBe('ô') + }) + + it('commits a multi-character recompose synchronously (no dropped tail)', async () => { + // "hanhj" -> a U+202F marker, four backspaces, then the recomposed "ạnh". + // Only a single microtask after the last read — the sync commit must have + // already delivered the final value (the deferred path dropped "nh" here). + const reads = ['h', 'a', 'n', 'h', NNBSP, '\x7f\x7f', '\x7f\x7f', '\u1EA1nh'] + + expect(await drive(reads)).toBe('h\u1EA1nh') + }) + + it('reproduces the full phrase "vương sỹ hạnh" from a real OpenKey capture', async () => { + // Captured byte stream for Telex "vuonwg syx hanhj": each syllable injects a + // U+202F marker, erases, and re-emits. Verified across read timings. + const reads = [ + 'v', 'u', 'o', NNBSP, '\x7f\x7f', '\x7f\u01B0\u01A1', 'n', 'g', + ' ', 's', 'y', NNBSP, '\x7f', '\x7f\u1EF9', + ' ', 'h', 'a', 'n', 'h', NNBSP, '\x7f\x7f\x7f\x7f\u1EA1nh' + ] + + for (const gapMs of [0, 17, 25]) { + expect(await drive(reads, { gapMs })).toBe('vương sỹ hạnh') + } + }) + + it('handles the EVKey capture (clean backspaces, no marker) for "hạnh"', async () => { + // EVKey emits three clean backspaces and no U+202F; must also yield "hạnh". + const reads = ['h', 'a', 'n', 'h', '\x7f', '\x7f', '\x7f', '\u1EA1nh'] + + expect(await drive(reads)).toBe('h\u1EA1nh') + }) +}) diff --git a/ui-tui/src/components/textInput.tsx b/ui-tui/src/components/textInput.tsx index 423547ca8f..1bb0c88282 100644 --- a/ui-tui/src/components/textInput.tsx +++ b/ui-tui/src/components/textInput.tsx @@ -666,6 +666,15 @@ export function TextInput({ const parentChangeTimer = useRef | null>(null) const pendingParentValue = useRef(null) const localRenderTimer = useRef | null>(null) + // True for one keystroke after a commit took the full Ink render path + // (syncParent). Ink repaints the whole input line, so the terminal cursor + // baseline that the fast-echo "\b \b" shortcut assumes is no longer valid; + // a fast-echo backspace fired right after an Ink repaint desyncs the screen + // and strands glyphs (the OpenKey Vietnamese "hạ␣␣" bug: an injected U+202F + // marker forces an Ink repaint, then the recompose backspaces fast-echo + // against a stale baseline). Suppress fast-echo for that one next edit. + const inkRepaintedRef = useRef(false) + const inkRepaintResetTimer = useRef | null>(null) const lineWidthRef = useRef(stringWidth(value.includes('\n') ? value.slice(value.lastIndexOf('\n') + 1) : value)) const mouseAnchorRef = useRef(null) const lastClickRef = useRef<{ at: number; offset: number }>({ at: 0, offset: -1 }) @@ -822,6 +831,10 @@ export function TextInput({ if (localRenderTimer.current) { clearTimeout(localRenderTimer.current) } + + if (inkRepaintResetTimer.current) { + clearTimeout(inkRepaintResetTimer.current) + } }, [] ) @@ -876,7 +889,7 @@ export function TextInput({ canFastEchoBase() && canFastAppendShape(current, cursor, text, columns, lineWidthRef.current) const canFastBackspace = (current: string, cursor: number) => - canFastEchoBase() && canFastBackspaceShape(current, cursor, columns) + !inkRepaintedRef.current && canFastEchoBase() && canFastBackspaceShape(current, cursor, columns) const commit = ( next: string, @@ -922,6 +935,22 @@ export function TextInput({ flushParentChange() self.current = true cbChange.current(next) + // A full Ink repaint just happened. Mark it so any fast-echo backspace + // later in this IME recompose burst is suppressed (it would write + // "\b \b" against a baseline Ink just invalidated, stranding the U+202F + // marker glyph — the "hạ␣␣" bug). IME reads arrive as SEPARATE stdin + // events with small macrotask gaps, so a setTimeout(0) reset would + // clear the flag between reads and miss the very backspaces it must + // guard. Use a short real-time window that spans a recompose burst; + // normal typing re-enables fast-echo via the append path below. + inkRepaintedRef.current = true + if (inkRepaintResetTimer.current) { + clearTimeout(inkRepaintResetTimer.current) + } + inkRepaintResetTimer.current = setTimeout(() => { + inkRepaintResetTimer.current = null + inkRepaintedRef.current = false + }, 60) } else { self.current = true scheduleParentChange(next) @@ -1375,7 +1404,17 @@ export function TextInput({ v = inserted.value c = inserted.cursor - scheduleKeyBurstCommit(v, c) + // Multi-character inserts are IME recompositions or pastes, NOT rapid + // single-key typing. Committing them through the 16ms deferred + // key-burst path opens a race: when an IME recompose arrives as a + // burst of backspaces followed by this text in one stdin read (e.g. + // OpenKey Vietnamese Telex, which injects a U+202F marker then erases + // and re-emits the syllable), the single `self.current` guard can be + // consumed by an interleaved re-render before the deferred commit + // flushes, snapping the buffer back to a stale parent value and + // dropping the recomposed tail (the "hanhj -> hạ␣␣" bug). Commit + // synchronously so the recomposed value reaches the parent atomically. + commit(v, c) return } @@ -1403,6 +1442,14 @@ export function TextInput({ // Same explicit fg as the Ink render (see the ) — // the bypass cell must not flash the terminal-default color. stdout!.write(colorizeEcho(effect.write, color)) + // A real character was just fast-echoed to the screen, so the + // terminal baseline is synced again — clear any pending Ink-repaint + // fast-echo suppression so normal backspace fast-echo resumes. + inkRepaintedRef.current = false + if (inkRepaintResetTimer.current) { + clearTimeout(inkRepaintResetTimer.current) + inkRepaintResetTimer.current = null + } // ASCII-printable text advances the physical cursor by exactly // text.length cells (canFastAppendShape rejects non-ASCII, // wide chars, newlines). Notify Ink so the cached displayCursor