From c9fe50f39d5af7af054c4528e7d628aef1937546 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Tue, 15 Sep 2026 22:10:36 +0800 Subject: [PATCH] fix(tui): keep stale own-echo flushes from rewinding composer keystrokes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deferred key-burst flush can still be in flight when the parent's re-render lands: the echoed value is the one we emitted, older than vRef because the user typed past it. The [value] effect treated any non-equal incoming value as an external assignment and rewound local state — the cursor jumped backward and freshly typed letters were overwritten (#111934). Track the last value handed to onChange; an echo matching it stays on the own-change path, and the pending flush for the newer local value converges the parent on its next timer. --- .../textInputStaleParentEcho.test.tsx | 149 ++++++++++++++++++ ui-tui/src/components/textInput.tsx | 14 +- 2 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 ui-tui/src/__tests__/textInputStaleParentEcho.test.tsx diff --git a/ui-tui/src/__tests__/textInputStaleParentEcho.test.tsx b/ui-tui/src/__tests__/textInputStaleParentEcho.test.tsx new file mode 100644 index 0000000000..6b39d88cda --- /dev/null +++ b/ui-tui/src/__tests__/textInputStaleParentEcho.test.tsx @@ -0,0 +1,149 @@ +import { EventEmitter } from 'events' + +import { renderSync } from '@hermes/ink' +import React, { useState } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { TextInput } from '../components/textInput.js' +import type { InputCursorSnapshot } from '../components/textInput.js' + +// Regression coverage for the stale own-echo rewind (#111934): while a +// deferred key-burst flush is in flight, the parent's re-render can hand the +// TextInput the value it emitted BEFORE the user typed further characters. +// The `[value]` effect used to treat any non-equal incoming value as an +// external assignment, rewinding local keystrokes (cursor jumps backward, +// freshly typed letters vanish). +// +// Only setTimeout/setInterval/Date are faked — setImmediate stays real so +// React's scheduler still commits renders between ticks. + +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)) + +function Harness({ + onValue, + snapshotRef +}: { + onValue: (value: string) => void + snapshotRef: React.RefObject +}) { + const [value, setValue] = useState('') + + return React.createElement(TextInput, { + cursorSnapshotRef: snapshotRef, + onChange: (next: string) => { + setValue(next) + onValue(next) + }, + value + }) +} + +describe('stale parent own-echo during deferred key-burst flush', () => { + beforeEach(() => { + vi.useFakeTimers({ toFake: ['setTimeout', 'setInterval', 'Date'] }) + // useStdout() resolves to process.stdout (not the FakeTty passed to + // renderSync), so the fast-echo bypass has to be armed on the real stream. + ;(process.stdout as { isTTY?: boolean }).isTTY = true + vi.spyOn(process.stdout, 'write').mockImplementation(() => true) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllEnvs() + vi.restoreAllMocks() + ;(process.stdout as { isTTY?: boolean }).isTTY = undefined + }) + + it('does not rewind local keystrokes when the parent echoes a value older than vRef', async () => { + vi.stubEnv('TERM_PROGRAM', 'iTerm.app') + vi.stubEnv('TMUX', '') + + const stdout = new FakeTty() + const stdin = new FakeTty() + const stderr = new FakeTty() + const values: string[] = [] + const snapshotRef = useRefBridge() + + const instance = renderSync(React.createElement(Harness, { onValue: v => values.push(v), snapshotRef }), { + 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() + + // 1. Type "a", "b", "c". "a" commits synchronously (fast-append needs a + // non-empty line); "b" and "c" ride the deferred 16ms key-burst path, + // so the parent still holds "a". + for (const ch of ['a', 'b', 'c']) { + stdin.send(ch) + await tick() + } + + // 2. Flush the burst: the parent receives "abc" and schedules a re-render. + vi.advanceTimersByTime(16) + + // 3. "d" lands BEFORE that re-render commits — vRef is now "abcd" while + // the incoming echo will still say "abc". + stdin.send('d') + + // 4. The echoed "abc" commits and the [value] effect runs. + await tick() + await tick() + + // 5. One more keystroke must survive too, not land on a rewound value. + stdin.send('e') + await tick() + vi.advanceTimersByTime(16) + await tick() + await tick() + } finally { + instance.unmount() + instance.cleanup() + } + + // unmount published the final {cursor, value} into the snapshot ref. + expect(values.at(-1)).toBe('abcde') + expect(snapshotRef.current).toEqual({ cursor: 5, value: 'abcde' }) + }) +}) + +// The snapshot ref is read after unmount in the assertion above; the helper +// keeps the harness a plain function component without hooks-order concerns. +function useRefBridge(): React.RefObject { + return { current: null } +} diff --git a/ui-tui/src/components/textInput.tsx b/ui-tui/src/components/textInput.tsx index 60e69963cc..dcc49e366a 100644 --- a/ui-tui/src/components/textInput.tsx +++ b/ui-tui/src/components/textInput.tsx @@ -804,6 +804,10 @@ export function TextInput({ const selRef = useRef(null) const vRef = useRef(value) const self = useRef(false) + // The last value handed to onChange. While a deferred key-burst flush is in + // flight the user can type past it, so the parent's echo comes back older + // than vRef; matching against this keeps such echoes on the own-change path. + const emittedValueRef = useRef(null) const keyBurstTimer = useRef | null>(null) const editVersionRef = useRef(0) const parentChangeTimer = useRef | null>(null) @@ -923,7 +927,13 @@ export function TextInput({ }, [accentOpen, cur, display, focus, highlights, nativeCursor, placeholder, placeholderColor, selected]) useEffect(() => { - const ownEcho = self.current && value === vRef.current + // `value === vRef.current` misses a deferred flush still in flight: the + // user typed past the emitted value, so the echo comes back older than + // vRef. Treating it as external rewound local keystrokes (cursor jumped + // backward, letters vanished — #111934). An echo matching the last value + // we emitted is still our own; the pending flush for the newer local + // value converges the parent on its next timer. + const ownEcho = self.current && (value === vRef.current || value === emittedValueRef.current) self.current = false if (ownEcho || value === vRef.current) { @@ -1045,6 +1055,7 @@ export function TextInput({ if (next !== null) { self.current = true + emittedValueRef.current = next cbChange.current(next) } } @@ -1138,6 +1149,7 @@ export function TextInput({ if (syncParent) { flushParentChange() self.current = true + emittedValueRef.current = next 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