mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-25 05:52:34 +00:00
fix(ui-tui): heal same-dimension alt-screen resize drift
- Treat same-dimension resize events in alt-screen mode as a repaint signal, because terminal hosts can reflow or restore the physical buffer without changing columns/rows. - Ensure pending resize erases are emitted even when the virtual diff is empty, so stale physical glyphs are still cleared. - Extract alt-screen resize repaint into prepareAltScreenResizeRepaint() for readability. - Add defensive clearTimeout in prepareAltScreenResizeRepaint so rapid resize bursts don't stack redundant delayed repaints. - Add a focused regression test for same-dimension alt-screen resize healing. Addresses #18449 Related to #17961
This commit is contained in:
parent
2844c888f1
commit
4813aaf0ba
2 changed files with 97 additions and 30 deletions
50
ui-tui/packages/hermes-ink/src/ink/ink-resize.test.ts
Normal file
50
ui-tui/packages/hermes-ink/src/ink/ink-resize.test.ts
Normal file
|
|
@ -0,0 +1,50 @@
|
||||||
|
import { EventEmitter } from 'events'
|
||||||
|
import React from 'react'
|
||||||
|
import { describe, expect, it } from 'vitest'
|
||||||
|
|
||||||
|
import Text from './components/Text.js'
|
||||||
|
import Ink from './ink.js'
|
||||||
|
import { CURSOR_HOME, ERASE_SCREEN } from './termio/csi.js'
|
||||||
|
|
||||||
|
class FakeTty extends EventEmitter {
|
||||||
|
chunks: string[] = []
|
||||||
|
columns = 20
|
||||||
|
rows = 5
|
||||||
|
isTTY = true
|
||||||
|
|
||||||
|
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 => queueMicrotask(resolve))
|
||||||
|
|
||||||
|
describe('Ink resize healing', () => {
|
||||||
|
it('heals same-dimension alt-screen resize events with an erase before repaint', 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
|
||||||
|
})
|
||||||
|
|
||||||
|
ink.setAltScreenActive(true)
|
||||||
|
ink.render(React.createElement(Text, null, 'hello'))
|
||||||
|
ink.onRender()
|
||||||
|
stdout.chunks = []
|
||||||
|
|
||||||
|
stdout.emit('resize')
|
||||||
|
ink.onRender()
|
||||||
|
await tick()
|
||||||
|
|
||||||
|
expect(stdout.chunks.join('')).toContain(ERASE_SCREEN + CURSOR_HOME)
|
||||||
|
|
||||||
|
ink.unmount()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -484,17 +484,22 @@ export default class Ink {
|
||||||
private handleResize = () => {
|
private handleResize = () => {
|
||||||
const cols = this.options.stdout.columns || 80
|
const cols = this.options.stdout.columns || 80
|
||||||
const rows = this.options.stdout.rows || 24
|
const rows = this.options.stdout.rows || 24
|
||||||
|
const dimsChanged = cols !== this.terminalColumns || rows !== this.terminalRows
|
||||||
|
|
||||||
// Terminals often emit 2+ resize events for one user action (window
|
// Terminals often emit 2+ resize events for one user action
|
||||||
// settling). Same-dimension events are no-ops; skip to avoid redundant
|
// (window settling). Same-dimension events are usually no-ops,
|
||||||
// frame resets and renders.
|
// but in alt-screen mode a same-dimension resize can signal a
|
||||||
if (cols === this.terminalColumns && rows === this.terminalRows) {
|
// terminal host reflow or buffer restore that leaves stale glyphs
|
||||||
|
// on the physical screen — treat it as a repaint signal.
|
||||||
|
if (!dimsChanged && !(this.altScreenActive && !this.isPaused && this.options.stdout.isTTY)) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
this.terminalColumns = cols
|
if (dimsChanged) {
|
||||||
this.terminalRows = rows
|
this.terminalColumns = cols
|
||||||
this.altScreenParkPatch = makeAltScreenParkPatch(this.terminalRows)
|
this.terminalRows = rows
|
||||||
|
this.altScreenParkPatch = makeAltScreenParkPatch(this.terminalRows)
|
||||||
|
}
|
||||||
|
|
||||||
// Pending throttled/drain work captured stale dims — cancel so
|
// Pending throttled/drain work captured stale dims — cancel so
|
||||||
// the upcoming microtask owns the next frame.
|
// the upcoming microtask owns the next frame.
|
||||||
|
|
@ -521,26 +526,7 @@ export default class Ink {
|
||||||
// doesn't exit alt-screen. Do NOT write ERASE_SCREEN: render() below
|
// doesn't exit alt-screen. Do NOT write ERASE_SCREEN: render() below
|
||||||
// can take ~80ms; erasing first leaves the screen blank that whole time.
|
// can take ~80ms; erasing first leaves the screen blank that whole time.
|
||||||
if (this.altScreenActive && !this.isPaused && this.options.stdout.isTTY) {
|
if (this.altScreenActive && !this.isPaused && this.options.stdout.isTTY) {
|
||||||
if (this.altScreenMouseTracking) {
|
this.prepareAltScreenResizeRepaint()
|
||||||
this.options.stdout.write(ENABLE_MOUSE_TRACKING)
|
|
||||||
}
|
|
||||||
|
|
||||||
this.resetFramesForAltScreen()
|
|
||||||
this.needsEraseBeforePaint = true
|
|
||||||
|
|
||||||
// One last repaint after the resize burst settles closes any host-side
|
|
||||||
// reflow drift the normal diff path can't see.
|
|
||||||
this.resizeSettleTimer = setTimeout(() => {
|
|
||||||
this.resizeSettleTimer = null
|
|
||||||
|
|
||||||
if (!this.canAltScreenRepaint()) {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
this.resetFramesForAltScreen()
|
|
||||||
this.needsEraseBeforePaint = true
|
|
||||||
this.render(this.currentNode!)
|
|
||||||
}, 160)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Already queued: later events in this burst updated dims/alt-screen
|
// Already queued: later events in this burst updated dims/alt-screen
|
||||||
|
|
@ -573,6 +559,36 @@ export default class Ink {
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private prepareAltScreenResizeRepaint(): void {
|
||||||
|
// Clear any pending settle timer from a previous resize burst so
|
||||||
|
// rapid events don't stack redundant delayed repaints. (handleResize
|
||||||
|
// also clears this, but the defensive clear keeps the method safe
|
||||||
|
// if it's ever called from other code paths.)
|
||||||
|
if (this.resizeSettleTimer !== null) {
|
||||||
|
clearTimeout(this.resizeSettleTimer)
|
||||||
|
this.resizeSettleTimer = null
|
||||||
|
}
|
||||||
|
|
||||||
|
if (this.altScreenMouseTracking) {
|
||||||
|
this.options.stdout.write(ENABLE_MOUSE_TRACKING)
|
||||||
|
}
|
||||||
|
|
||||||
|
this.resetFramesForAltScreen()
|
||||||
|
this.needsEraseBeforePaint = true
|
||||||
|
|
||||||
|
this.resizeSettleTimer = setTimeout(() => {
|
||||||
|
this.resizeSettleTimer = null
|
||||||
|
|
||||||
|
if (!this.canAltScreenRepaint()) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
this.resetFramesForAltScreen()
|
||||||
|
this.needsEraseBeforePaint = true
|
||||||
|
this.render(this.currentNode!)
|
||||||
|
}, 160)
|
||||||
|
}
|
||||||
|
|
||||||
resolveExitPromise: () => void = () => {}
|
resolveExitPromise: () => void = () => {}
|
||||||
rejectExitPromise: (reason?: Error) => void = () => {}
|
rejectExitPromise: (reason?: Error) => void = () => {}
|
||||||
unsubscribeExit: () => void = () => {}
|
unsubscribeExit: () => void = () => {}
|
||||||
|
|
@ -919,8 +935,9 @@ export default class Ink {
|
||||||
const optimized = optimize(diff)
|
const optimized = optimize(diff)
|
||||||
const optimizeMs = performance.now() - tOptimize
|
const optimizeMs = performance.now() - tOptimize
|
||||||
const hasDiff = optimized.length > 0
|
const hasDiff = optimized.length > 0
|
||||||
|
const needsAltScreenErase = this.altScreenActive && this.needsEraseBeforePaint
|
||||||
|
|
||||||
if (this.altScreenActive && hasDiff) {
|
if (this.altScreenActive && (hasDiff || needsAltScreenErase)) {
|
||||||
// Prepend CSI H to anchor the physical cursor to (0,0) so
|
// Prepend CSI H to anchor the physical cursor to (0,0) so
|
||||||
// log-update's relative moves compute from a known spot (self-healing
|
// log-update's relative moves compute from a known spot (self-healing
|
||||||
// against out-of-band cursor drift, see the ALT_SCREEN_ANCHOR_CURSOR
|
// against out-of-band cursor drift, see the ALT_SCREEN_ANCHOR_CURSOR
|
||||||
|
|
@ -940,7 +957,7 @@ export default class Ink {
|
||||||
// resize, so it gets CSI 3J in this one recovery path. When BSU/ESU is
|
// resize, so it gets CSI 3J in this one recovery path. When BSU/ESU is
|
||||||
// supported, the clear+paint lands atomically; otherwise the final state
|
// supported, the clear+paint lands atomically; otherwise the final state
|
||||||
// is still healed even if the repaint is visible.
|
// is still healed even if the repaint is visible.
|
||||||
if (this.needsEraseBeforePaint) {
|
if (needsAltScreenErase) {
|
||||||
this.needsEraseBeforePaint = false
|
this.needsEraseBeforePaint = false
|
||||||
optimized.unshift(needsAltScreenResizeScrollbackClear() ? DEEP_ERASE_THEN_HOME_PATCH : ERASE_THEN_HOME_PATCH)
|
optimized.unshift(needsAltScreenResizeScrollbackClear() ? DEEP_ERASE_THEN_HOME_PATCH : ERASE_THEN_HOME_PATCH)
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -1062,7 +1079,7 @@ export default class Ink {
|
||||||
this.lastDrainMs = 0
|
this.lastDrainMs = 0
|
||||||
|
|
||||||
// Only track drain on TTY. Piped/non-TTY stdout bypasses flow control.
|
// Only track drain on TTY. Piped/non-TTY stdout bypasses flow control.
|
||||||
const trackDrain = this.options.stdout.isTTY && hasDiff
|
const trackDrain = this.options.stdout.isTTY && optimized.length > 0
|
||||||
const drainStart = trackDrain ? tWrite : 0
|
const drainStart = trackDrain ? tWrite : 0
|
||||||
|
|
||||||
if (trackDrain) {
|
if (trackDrain) {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue