mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(hermes-ink): reassemble split mouse sequences at the tokenizer; drop the regex sink
Root-cause fix for the SGR mouse fragment leak (`46M35;40M...` typed into the prompt). The leak was never really about the fragments — it was the flush emitting them. When App's 50ms watchdog fires mid-CSI during a render stall, the tokenizer was force-emitting the buffered partial as a token and resetting to ground, so both the prefix and the ESC-less remainder surfaced as unparseable input. Make the flush state-aware (xterm.js discipline): a bare ESC still flushes to the Escape key (the legitimate ESCDELAY case), but a buffer still inside a multi-byte control sequence (csi/osc/dcs/apc/ss3/intermediate) is NOT emitted — it's kept so the continuation reassembles on the next feed. A one-tick truncation valve in createTokenizer.flush() drops a partial that survives a second flush with no progress, so a genuinely truncated write can't fuse into the next keypress. With partials never entering the input stream, the downstream scrubber is dead code: remove the SGR fragment guard from input-event.ts (both the original `/^\[<\d+;\d+;\d+[Mm]/` and the consolidated form added earlier in this PR). The parse-keypress burst-recovery regexes (MOUSE_BURST_*) are now also redundant but left in place as a safety net for one release; they can be removed in a follow-up once this soaks. Tests: tokenize.test.ts proves a mid-CSI flush keeps/reassembles and that a stale partial is dropped after a second flush and a bare ESC still emits; parse-keypress.test.ts adds the end-to-end split-then-reassemble case yielding a single clean mouse event with no leaked key. Supersedes #29337.
This commit is contained in:
parent
01c010e233
commit
f354323547
4 changed files with 133 additions and 38 deletions
|
|
@ -5,32 +5,6 @@ import { Event } from './event.js'
|
|||
const inputForSpecialSequence = (name: string): string =>
|
||||
name === 'space' ? ' ' : name === 'return' || name === 'escape' ? '' : name
|
||||
|
||||
// SGR mouse-report fragment that leaked into a nameless text/sequence token.
|
||||
// In alt-screen Ink enables MOUSE_ANY (DEC 1003), so every pixel of motion
|
||||
// emits a CSI mouse report (ESC[<btn;col;row M/m). When a heavy React commit
|
||||
// blocks the event loop past App's 50ms flush watchdog, that CSI can be split
|
||||
// across stdin chunks at ANY byte boundary. The tokenizer flush force-emits
|
||||
// the buffered prefix and resets to ground, so BOTH halves can surface as
|
||||
// unparseable tokens that parseKeypress can't classify (name=''):
|
||||
//
|
||||
// - flushed prefix — ESC[< / [< / < + partial params, no terminator yet
|
||||
// (e.g. `ESC[<0;35;`, `[<0;`, `<0;35;46`)
|
||||
// - ESC-less tail — 1-, 2-, or 3-field digit run ending in M/m
|
||||
// (e.g. `46M`, `;46M`, `35;46M`, `;35;46M`, `0;35;46M`)
|
||||
//
|
||||
// One regex covers every split position. The leading-`;` and 1-/2-field tails
|
||||
// are the cases the older `/^\[<\d+;\d+;\d+[Mm]/` guard missed, which is how
|
||||
// `46M35;40M...` ends up typed into the prompt during long sessions.
|
||||
//
|
||||
// Safety: the `(?=…\d)` lookahead requires at least one digit, so a typed `<`,
|
||||
// `[`, `;`, or `M` (none of which carry a coordinate digit) is never matched;
|
||||
// the embedded `M`/`m` in `[\d;]+` means a run like `1;2;3M9;10M` (two stuck-
|
||||
// together fragments / prose) can't satisfy the `$` anchor and is left intact.
|
||||
// Combined with the caller's `!keypress.name` gate — real typing arrives one
|
||||
// char per chunk with a name set — no genuine keystroke is swallowed.
|
||||
// eslint-disable-next-line no-control-regex
|
||||
const SGR_MOUSE_FRAGMENT_LEAK_RE = /^(?:\x1b)?(?=(?:\[<|<)?[\d;]*\d)(?:\[<|<)?[\d;]+[Mm]?$/
|
||||
|
||||
export type Key = {
|
||||
upArrow: boolean
|
||||
downArrow: boolean
|
||||
|
|
@ -109,15 +83,10 @@ function parseKey(keypress: ParsedKey): [Key, string] {
|
|||
input = ''
|
||||
}
|
||||
|
||||
// Suppress SGR mouse-report fragments left over from a flush-boundary split
|
||||
// (see SGR_MOUSE_FRAGMENT_LEAK_RE). Both the flushed CSI prefix and the
|
||||
// ESC-less remainder reach here as nameless tokens that parseKeypress can't
|
||||
// classify, so without this sink they leak into the prompt as `46M35;40M…`.
|
||||
// This is the same defensive sink as the F13 guard above; the underlying
|
||||
// tokenizer-flush race is upstream of this layer.
|
||||
if (!keypress.name && SGR_MOUSE_FRAGMENT_LEAK_RE.test(input)) {
|
||||
input = ''
|
||||
}
|
||||
// (SGR mouse-report fragments used to be scrubbed here. They no longer reach
|
||||
// this layer: the tokenizer keeps an incomplete CSI buffered across a
|
||||
// watchdog flush and reassembles it on the next feed instead of force-
|
||||
// emitting the partial as input. See termio/tokenize.ts.)
|
||||
|
||||
// Strip meta if it's still remaining after `parseKeypress`
|
||||
// TODO(vadimdemedes): remove this in the next major version.
|
||||
|
|
|
|||
|
|
@ -143,6 +143,7 @@ describe('fragmented SGR mouse recovery', () => {
|
|||
// blob types into the composer and locks the user out.
|
||||
const blob =
|
||||
'M6M35;220;56M6M35;218;56M169;48M;157;47M;44M20;43M79;40M78;40M0M7M35;49;41M48;41M;47;40M9;15;32M[I;31M5;211;26M35;211;25M7M;220;1MM0M09;25M24M23M3;22MM18M99;26M32MM38M63;44M47MM1;51M M4M54M'
|
||||
|
||||
const [events] = parseMultipleKeypresses(INITIAL_STATE, blob)
|
||||
|
||||
expect(events).toEqual([])
|
||||
|
|
@ -165,3 +166,29 @@ describe('fragmented SGR mouse recovery', () => {
|
|||
expect(events).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('flush-boundary SGR mouse reassembly', () => {
|
||||
it('reassembles a report split by a mid-sequence watchdog flush into one mouse event', () => {
|
||||
// chunk 1: heavy render stalls the loop, only the prefix is read
|
||||
let [keys, state] = parseMultipleKeypresses(INITIAL_STATE, '\x1b[<0;35;')
|
||||
expect(keys).toEqual([])
|
||||
|
||||
// App's 50ms watchdog flushes (input=null) — must NOT emit the partial
|
||||
;[keys, state] = parseMultipleKeypresses(state, null)
|
||||
expect(keys).toEqual([])
|
||||
|
||||
// continuation arrives; the whole report reassembles, nothing leaks
|
||||
;[keys, state] = parseMultipleKeypresses(state, '46M')
|
||||
expect(keys).toEqual([expect.objectContaining({ kind: 'mouse', button: 0, col: 35, row: 46, action: 'press' })])
|
||||
})
|
||||
|
||||
it('drops a truncated mouse prefix after a second flush instead of leaking it', () => {
|
||||
let [keys, state] = parseMultipleKeypresses(INITIAL_STATE, '\x1b[<0;35;')
|
||||
|
||||
;[keys, state] = parseMultipleKeypresses(state, null) // first flush keeps it
|
||||
;[keys, state] = parseMultipleKeypresses(state, null) // second flush drops it
|
||||
|
||||
expect(keys).toEqual([])
|
||||
expect(state.incomplete).toBe('')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
65
ui-tui/packages/hermes-ink/src/ink/termio/tokenize.test.ts
Normal file
65
ui-tui/packages/hermes-ink/src/ink/termio/tokenize.test.ts
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { createTokenizer } from './tokenize.js'
|
||||
|
||||
describe('tokenizer escape-sequence boundaries', () => {
|
||||
it('reassembles a CSI mouse sequence split across two feeds', () => {
|
||||
const t = createTokenizer({ x10Mouse: true })
|
||||
|
||||
expect(t.feed('\x1b[<0;35;')).toEqual([])
|
||||
expect(t.feed('46M')).toEqual([{ type: 'sequence', value: '\x1b[<0;35;46M' }])
|
||||
expect(t.buffer()).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
describe('tokenizer state-aware flush', () => {
|
||||
it('does not emit an incomplete CSI on flush — it keeps it for reassembly', () => {
|
||||
const t = createTokenizer({ x10Mouse: true })
|
||||
|
||||
// A render stall lets App's watchdog flush mid-sequence. The buffered CSI
|
||||
// prefix must NOT be emitted (that is the `46M…` leak); it stays buffered.
|
||||
expect(t.feed('\x1b[<0;35;')).toEqual([])
|
||||
expect(t.flush()).toEqual([])
|
||||
expect(t.buffer()).toBe('\x1b[<0;35;')
|
||||
|
||||
// The continuation arrives on the next feed and the whole report
|
||||
// reassembles into a single clean sequence token — nothing leaked.
|
||||
expect(t.feed('46M')).toEqual([{ type: 'sequence', value: '\x1b[<0;35;46M' }])
|
||||
expect(t.buffer()).toBe('')
|
||||
})
|
||||
|
||||
it('drops a partial control sequence that survives a second flush (truncation)', () => {
|
||||
const t = createTokenizer({ x10Mouse: true })
|
||||
|
||||
expect(t.feed('\x1b[<0;35;')).toEqual([])
|
||||
expect(t.flush()).toEqual([]) // first flush keeps the buffer
|
||||
expect(t.buffer()).toBe('\x1b[<0;35;')
|
||||
|
||||
// Continuation never arrived: the next flush sees the same buffer and
|
||||
// drops it so it can't fuse with the next keypress's bytes.
|
||||
expect(t.flush()).toEqual([])
|
||||
expect(t.buffer()).toBe('')
|
||||
})
|
||||
|
||||
it('still emits a bare ESC on flush so the Escape key works', () => {
|
||||
const t = createTokenizer({ x10Mouse: true })
|
||||
|
||||
expect(t.feed('\x1b')).toEqual([])
|
||||
expect(t.flush()).toEqual([{ type: 'sequence', value: '\x1b' }])
|
||||
expect(t.buffer()).toBe('')
|
||||
})
|
||||
|
||||
it('reassembles even when a flush fires between every byte of the report', () => {
|
||||
const t = createTokenizer({ x10Mouse: true })
|
||||
|
||||
// Pathological stall: a flush between each chunk. As long as the
|
||||
// continuation eventually arrives, no fragment is ever emitted as input.
|
||||
for (const chunk of ['\x1b[', '<', '0;', '35;', '46']) {
|
||||
expect(t.feed(chunk)).toEqual([])
|
||||
expect(t.flush()).toEqual([])
|
||||
}
|
||||
|
||||
expect(t.feed('M')).toEqual([{ type: 'sequence', value: '\x1b[<0;35;46M' }])
|
||||
expect(t.buffer()).toBe('')
|
||||
})
|
||||
})
|
||||
|
|
@ -47,10 +47,18 @@ type TokenizerOptions = {
|
|||
export function createTokenizer(options?: TokenizerOptions): Tokenizer {
|
||||
let currentState: State = 'ground'
|
||||
let currentBuffer = ''
|
||||
// The control-sequence buffer kept across the previous flush, if any. Used
|
||||
// as a one-tick truncation valve: a partial CSI mouse report normally
|
||||
// reassembles on the very next feed, so if a flush sees the exact same
|
||||
// buffer it kept last time (the continuation never arrived), we drop it.
|
||||
let lastFlushedBuffer = ''
|
||||
const x10Mouse = options?.x10Mouse ?? false
|
||||
|
||||
return {
|
||||
feed(input: string): Token[] {
|
||||
// Real bytes arrived — any kept partial is no longer stale.
|
||||
lastFlushedBuffer = ''
|
||||
|
||||
const result = tokenize(input, currentState, currentBuffer, false, x10Mouse)
|
||||
|
||||
currentState = result.state.state
|
||||
|
|
@ -64,12 +72,25 @@ export function createTokenizer(options?: TokenizerOptions): Tokenizer {
|
|||
currentState = result.state.state
|
||||
currentBuffer = result.state.buffer
|
||||
|
||||
// tokenize() keeps (doesn't emit) an incomplete control sequence on
|
||||
// flush. If two consecutive flushes see the same buffer with no feed in
|
||||
// between, the continuation is never coming (truncated write / killed
|
||||
// process) — drop it so it can't fuse with the next keypress's bytes.
|
||||
if (currentBuffer && currentBuffer === lastFlushedBuffer) {
|
||||
currentState = 'ground'
|
||||
currentBuffer = ''
|
||||
lastFlushedBuffer = ''
|
||||
} else {
|
||||
lastFlushedBuffer = currentBuffer
|
||||
}
|
||||
|
||||
return result.tokens
|
||||
},
|
||||
|
||||
reset(): void {
|
||||
currentState = 'ground'
|
||||
currentBuffer = ''
|
||||
lastFlushedBuffer = ''
|
||||
},
|
||||
|
||||
buffer(): string {
|
||||
|
|
@ -298,8 +319,10 @@ function tokenize(
|
|||
// Handle end of input
|
||||
if (result.state === 'ground') {
|
||||
flushText()
|
||||
} else if (flush) {
|
||||
// Force output incomplete sequence
|
||||
} else if (flush && result.state === 'escape') {
|
||||
// A bare ESC with nothing after it is the Escape key — the one incomplete
|
||||
// state a flush should turn into input (the classic ESCDELAY lone-ESC
|
||||
// disambiguation: ESC alone vs. ESC as a sequence/meta prefix).
|
||||
const remaining = data.slice(seqStart)
|
||||
|
||||
if (remaining) {
|
||||
|
|
@ -308,7 +331,18 @@ function tokenize(
|
|||
|
||||
result.state = 'ground'
|
||||
} else {
|
||||
// Buffer incomplete sequence for next call
|
||||
// Buffer the incomplete sequence. Two paths land here:
|
||||
// - streaming (flush=false): normal carry-over to the next feed.
|
||||
// - flush=true while still inside a multi-byte control sequence
|
||||
// (csi/osc/dcs/apc/ss3/escapeIntermediate): we deliberately do NOT
|
||||
// emit it. A half-arrived CSI mouse report (ESC[<btn;col;row M) is an
|
||||
// unfinished sequence, not user input — force-emitting it is what
|
||||
// injects `46M`/`35;46M` shards into the prompt during a render stall.
|
||||
// Keeping it buffered lets the continuation reassemble on the next
|
||||
// feed (the xterm.js state-machine discipline — partial sequences
|
||||
// never become text). createTokenizer.flush() drops the buffer if it
|
||||
// survives a second flush with no progress (a genuine truncation), so
|
||||
// a stuck partial can never merge into the next keypress's bytes.
|
||||
result.buffer = data.slice(seqStart)
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue