mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(hermes-ink): disable mouse tracking on raw-mode teardown to stop SGR leak (#42527)
The raw-mode teardown path (rawModeEnabledCount -> 0) disabled modifyOtherKeys, kitty keyboard, focus reporting, and bracketed paste, then dropped raw mode and detached the readable listener -- but left DEC mouse tracking (1000/1002/1003/1006) asserted. With raw mode off and no reader attached, the terminal falls back to cooked-mode echo, so every mouse move emits a hover report (DEC 1003) that prints as literal text: a flood of '35;col;row M' shards over the prompt in a long session. handleSuspend() already guards against exactly this (it writes DISABLE_MOUSE_TRACKING before SIGSTOP); the ordinary teardown path missed the same guard. Add DISABLE_MOUSE_TRACKING to the teardown, and re-assert tracking on raw-mode re-entry (via the Ink instance's reassertTerminalModes, which is gated on altScreenActive and idempotent) so a transient drop->re-add round-trips cleanly instead of silently leaving the mouse dead. Adds a regression test driving a real Ink mount: the last raw-mode consumer detaching must emit DISABLE_MOUSE_TRACKING. Reported via a community bug report.
This commit is contained in:
parent
6a8dda171c
commit
1e5ff4a577
2 changed files with 116 additions and 0 deletions
91
ui-tui/packages/hermes-ink/src/ink/app-rawmode-mouse.test.ts
Normal file
91
ui-tui/packages/hermes-ink/src/ink/app-rawmode-mouse.test.ts
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
import { EventEmitter } from 'events'
|
||||
import React, { useContext, useEffect } from 'react'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import StdinContext from './components/StdinContext.js'
|
||||
import Text from './components/Text.js'
|
||||
import Ink from './ink.js'
|
||||
import { DISABLE_MOUSE_TRACKING } from './termio/dec.js'
|
||||
|
||||
class FakeTty extends EventEmitter {
|
||||
chunks: string[] = []
|
||||
columns = 80
|
||||
rows = 24
|
||||
isTTY = true
|
||||
isRaw = false
|
||||
|
||||
ref(): void {}
|
||||
unref(): void {}
|
||||
read(): null {
|
||||
return null
|
||||
}
|
||||
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<void>(resolve => setImmediate(resolve))
|
||||
|
||||
// A child that grabs the last useInput consumer's raw-mode toggle. Mounting
|
||||
// enables raw mode (count 0→1); unmounting disables it (count 1→0), which is
|
||||
// the teardown path that must DISABLE_MOUSE_TRACKING so DEC 1003 hover can't
|
||||
// leak as cooked-mode `35;col;row M` text over the prompt.
|
||||
function RawModeConsumer({ active }: { active: boolean }) {
|
||||
const { setRawMode, isRawModeSupported } = useContext(StdinContext)
|
||||
|
||||
useEffect(() => {
|
||||
if (!active || !isRawModeSupported) {
|
||||
return
|
||||
}
|
||||
|
||||
setRawMode(true)
|
||||
|
||||
return () => setRawMode(false)
|
||||
}, [active, isRawModeSupported, setRawMode])
|
||||
|
||||
return React.createElement(Text, null, 'x')
|
||||
}
|
||||
|
||||
describe('App raw-mode teardown', () => {
|
||||
it('disables mouse tracking when the last raw-mode consumer detaches', async () => {
|
||||
const stdout = new FakeTty()
|
||||
const stdin = new FakeTty()
|
||||
const stderr = new FakeTty()
|
||||
const ink = new Ink({
|
||||
exitOnCtrlC: false,
|
||||
patchConsole: false,
|
||||
stderr: stderr as unknown as NodeJS.WriteStream,
|
||||
stdin: stdin as unknown as NodeJS.ReadStream,
|
||||
stdout: stdout as unknown as NodeJS.WriteStream
|
||||
})
|
||||
|
||||
// Mouse tracking is asserted on the alt screen; the teardown path lives in
|
||||
// App, independent of who enabled tracking.
|
||||
ink.setAltScreenActive(true, 'all')
|
||||
ink.render(React.createElement(RawModeConsumer, { active: true }))
|
||||
ink.onRender()
|
||||
await tick()
|
||||
expect(stdin.isRaw).toBe(true)
|
||||
|
||||
stdout.chunks = []
|
||||
|
||||
// Drop the consumer → raw-mode count hits 0 → teardown runs.
|
||||
ink.render(React.createElement(RawModeConsumer, { active: false }))
|
||||
ink.onRender()
|
||||
await tick()
|
||||
|
||||
expect(stdin.isRaw).toBe(false)
|
||||
expect(stdout.chunks.join('')).toContain(DISABLE_MOUSE_TRACKING)
|
||||
|
||||
ink.unmount()
|
||||
})
|
||||
})
|
||||
|
|
@ -9,6 +9,7 @@ import type { DOMElement } from '../dom.js'
|
|||
import { EventEmitter } from '../events/emitter.js'
|
||||
import { InputEvent } from '../events/input-event.js'
|
||||
import { TerminalFocusEvent } from '../events/terminal-focus-event.js'
|
||||
import instances from '../instances.js'
|
||||
import {
|
||||
INITIAL_STATE,
|
||||
type ParsedInput,
|
||||
|
|
@ -309,6 +310,21 @@ export default class App extends PureComponent<Props, State> {
|
|||
}
|
||||
})
|
||||
})
|
||||
|
||||
// Re-assert mouse tracking on raw-mode re-entry. <AlternateScreen>
|
||||
// owns the initial enable, but its effect only re-runs on a
|
||||
// mode/writeRaw change — NOT on a raw-mode bounce (count 1→0→1, e.g.
|
||||
// an overlay that briefly drops the last useInput consumer). The
|
||||
// teardown above now DISABLE_MOUSE_TRACKING's to stop the cooked-echo
|
||||
// leak, so without this the terminal would be left with tracking off
|
||||
// and the mouse silently dead until the next stdin-gap/resize
|
||||
// re-assert. reassertTerminalModes() is gated on altScreenActive and
|
||||
// idempotent, so it's a no-op when there's nothing to restore.
|
||||
// Deferred (same setImmediate discipline as the XTVERSION probe) so it
|
||||
// lands after any alt-screen enable writes in this render cycle.
|
||||
setImmediate(() => {
|
||||
instances.get(this.props.stdout)?.reassertTerminalModes()
|
||||
})
|
||||
}
|
||||
|
||||
this.rawModeEnabledCount++
|
||||
|
|
@ -324,6 +340,15 @@ export default class App extends PureComponent<Props, State> {
|
|||
this.props.stdout.write(DFE)
|
||||
// Disable bracketed paste mode
|
||||
this.props.stdout.write(DBP)
|
||||
// Disable mouse tracking. Tracking is asserted by <AlternateScreen> /
|
||||
// the Ink instance, NOT here — but dropping raw mode + detaching the
|
||||
// readable listener while DEC 1003 hover stays on means the terminal
|
||||
// falls back to cooked-mode echo and every mouse move leaks as text
|
||||
// (`35;col;row M` shards over the prompt). Same hazard handleSuspend()
|
||||
// already guards against; this teardown path missed it. Idempotent
|
||||
// (no-op if tracking was never on), and re-enabling raw mode below
|
||||
// re-asserts tracking so a transient drop→re-add round-trips cleanly.
|
||||
this.props.stdout.write(DISABLE_MOUSE_TRACKING)
|
||||
stdin.setRawMode(false)
|
||||
stdin.removeListener('readable', this.handleReadable)
|
||||
stdin.unref()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue