Fix terminal side-effect drain stalls (#2975)
This commit is contained in:
parent
1371402bc6
commit
b3a1535f6d
|
|
@ -124,6 +124,113 @@ describe('createIpcPtyTransport', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('limits deferred PTY side-effect work per timer tick', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const { createPtyOutputProcessor } = await import('./pty-transport')
|
||||
const onTitleChange = vi.fn()
|
||||
const processor = createPtyOutputProcessor({ onTitleChange })
|
||||
const callbacks = { onData: vi.fn() }
|
||||
|
||||
for (let i = 0; i < 200; i++) {
|
||||
processor.processData(`\x1b]0;title-${i}\x07`, callbacks)
|
||||
}
|
||||
|
||||
expect(onTitleChange).not.toHaveBeenCalled()
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
|
||||
expect(onTitleChange.mock.calls.length).toBeGreaterThan(0)
|
||||
expect(onTitleChange.mock.calls.length).toBeLessThan(200)
|
||||
|
||||
await vi.runAllTimersAsync()
|
||||
expect(onTitleChange).toHaveBeenCalledTimes(200)
|
||||
expect(onTitleChange).toHaveBeenLastCalledWith('title-199', 'title-199')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('limits coalesced OSC titles in one PTY chunk per timer tick', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const { createPtyOutputProcessor } = await import('./pty-transport')
|
||||
const onTitleChange = vi.fn()
|
||||
const processor = createPtyOutputProcessor({ onTitleChange })
|
||||
const callbacks = { onData: vi.fn() }
|
||||
const titles = Array.from({ length: 200 }, (_, i) => `\x1b]0;chunk-title-${i}\x07`).join('')
|
||||
|
||||
processor.processData(titles, callbacks)
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
|
||||
expect(onTitleChange.mock.calls.length).toBeGreaterThan(0)
|
||||
expect(onTitleChange.mock.calls.length).toBeLessThan(200)
|
||||
|
||||
await vi.runAllTimersAsync()
|
||||
expect(onTitleChange).toHaveBeenCalledTimes(200)
|
||||
expect(onTitleChange).toHaveBeenLastCalledWith('chunk-title-199', 'chunk-title-199')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('flushes all remaining PTY side effects after a partial bounded drain', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const { createPtyOutputProcessor } = await import('./pty-transport')
|
||||
const onTitleChange = vi.fn()
|
||||
const processor = createPtyOutputProcessor({ onTitleChange })
|
||||
const callbacks = { onData: vi.fn() }
|
||||
|
||||
for (let i = 0; i < 200; i++) {
|
||||
processor.processData(`\x1b]0;flush-title-${i}\x07`, callbacks)
|
||||
}
|
||||
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
expect(onTitleChange.mock.calls.length).toBeLessThan(200)
|
||||
|
||||
processor.flushPendingSideEffects()
|
||||
|
||||
expect(onTitleChange).toHaveBeenCalledTimes(200)
|
||||
expect(onTitleChange).toHaveBeenLastCalledWith('flush-title-199', 'flush-title-199')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('still runs stale-title detection when an OSC status chunk has no title', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const { createPtyOutputProcessor } = await import('./pty-transport')
|
||||
const onTitleChange = vi.fn()
|
||||
const onAgentStatus = vi.fn()
|
||||
const onAgentBecameIdle = vi.fn()
|
||||
const processor = createPtyOutputProcessor({
|
||||
onTitleChange,
|
||||
onAgentStatus,
|
||||
onAgentBecameIdle
|
||||
})
|
||||
const callbacks = { onData: vi.fn() }
|
||||
|
||||
processor.processData('\x1b]0;. Claude working\x07', callbacks)
|
||||
processor.processData(
|
||||
'\x1b]9999;{"state":"working","prompt":"ship it","agentType":"codex"}\x07plain output\r\n',
|
||||
callbacks
|
||||
)
|
||||
|
||||
await vi.runOnlyPendingTimersAsync()
|
||||
expect(onAgentStatus).toHaveBeenCalledWith({
|
||||
state: 'working',
|
||||
prompt: 'ship it',
|
||||
agentType: 'codex'
|
||||
})
|
||||
|
||||
vi.advanceTimersByTime(3_000)
|
||||
expect(onAgentBecameIdle).toHaveBeenCalledTimes(1)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('uses acknowledged writes only for local IPC PTYs', async () => {
|
||||
const { createIpcPtyTransport } = await import('./pty-transport')
|
||||
const localTransport = createIpcPtyTransport({})
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ export { extractLastOscTitle } from '../../../../shared/agent-detection'
|
|||
|
||||
const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
|
||||
const STALE_TITLE_TIMEOUT = 3000 // ms before stale working title is cleared
|
||||
const MAX_PTY_SIDE_EFFECTS_PER_DRAIN = 64
|
||||
|
||||
// Why: onAgentStatus callback added to IpcPtyTransportOptions in pty-dispatcher
|
||||
// so the OSC 9999 status payloads can be forwarded to the store.
|
||||
|
|
@ -99,6 +100,7 @@ export function createPtyOutputProcessor({
|
|||
let staleTitleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let sideEffectDrainTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let pendingSideEffects: PendingPtySideEffect[] = []
|
||||
let pendingSideEffectIndex = 0
|
||||
const agentTracker =
|
||||
onAgentBecameIdle || onAgentBecameWorking || onAgentExited
|
||||
? createAgentStatusTracker(
|
||||
|
|
@ -140,6 +142,33 @@ export function createPtyOutputProcessor({
|
|||
}
|
||||
}
|
||||
|
||||
function scheduleSideEffectDrain(): void {
|
||||
if (sideEffectDrainTimer !== null) {
|
||||
return
|
||||
}
|
||||
// Why: xterm.write() buffers parsing onto its own timer. Defer Orca's
|
||||
// title/status/BEL store work so live terminal rendering gets the next turn.
|
||||
sideEffectDrainTimer = setTimeout(drainPtySideEffects, 0)
|
||||
}
|
||||
|
||||
function enqueuePtySideEffect(next: PendingPtySideEffect): void {
|
||||
const prior = pendingSideEffects.at(-1)
|
||||
if (
|
||||
prior &&
|
||||
prior.titles.length === 0 &&
|
||||
prior.payloads.length === 0 &&
|
||||
!prior.containsBell &&
|
||||
prior.suppressAttentionEvents === next.suppressAttentionEvents &&
|
||||
next.titles.length === 0 &&
|
||||
next.payloads.length === 0 &&
|
||||
!next.containsBell
|
||||
) {
|
||||
prior.scannedForTitles ||= next.scannedForTitles
|
||||
return
|
||||
}
|
||||
pendingSideEffects.push(next)
|
||||
}
|
||||
|
||||
function schedulePtySideEffects(
|
||||
data: string,
|
||||
payloads: ReturnType<typeof processAgentStatusChunk>['payloads'],
|
||||
|
|
@ -156,36 +185,56 @@ export function createPtyOutputProcessor({
|
|||
return
|
||||
}
|
||||
|
||||
const prior = pendingSideEffects.at(-1)
|
||||
if (
|
||||
prior &&
|
||||
prior.titles.length === 0 &&
|
||||
prior.payloads.length === 0 &&
|
||||
!prior.containsBell &&
|
||||
prior.suppressAttentionEvents === suppressAttentionEvents &&
|
||||
titles.length === 0 &&
|
||||
deliveredPayloads.length === 0 &&
|
||||
!containsBell
|
||||
) {
|
||||
prior.scannedForTitles ||= scannedForTitles
|
||||
} else {
|
||||
// Why: keep only compact derived side-effect facts here. Retaining raw
|
||||
// PTY chunks duplicates the terminal scheduler backlog while timers are
|
||||
// throttled in a backgrounded Electron window.
|
||||
pendingSideEffects.push({
|
||||
titles,
|
||||
payloads: deliveredPayloads,
|
||||
// Why: keep only compact derived side-effect facts here. Retaining raw
|
||||
// PTY chunks duplicates the terminal scheduler backlog while timers are
|
||||
// throttled in a backgrounded Electron window.
|
||||
if (deliveredPayloads.length === 0 && titles.length === 0) {
|
||||
enqueuePtySideEffect({
|
||||
payloads: [],
|
||||
titles: [],
|
||||
scannedForTitles,
|
||||
containsBell,
|
||||
suppressAttentionEvents
|
||||
})
|
||||
} else {
|
||||
for (const payload of deliveredPayloads) {
|
||||
enqueuePtySideEffect({
|
||||
payloads: [payload],
|
||||
titles: [],
|
||||
scannedForTitles: false,
|
||||
containsBell: false,
|
||||
suppressAttentionEvents
|
||||
})
|
||||
}
|
||||
if (titles.length === 0 && scannedForTitles) {
|
||||
enqueuePtySideEffect({
|
||||
payloads: [],
|
||||
titles: [],
|
||||
scannedForTitles: true,
|
||||
containsBell: false,
|
||||
suppressAttentionEvents
|
||||
})
|
||||
}
|
||||
for (const title of titles) {
|
||||
enqueuePtySideEffect({
|
||||
payloads: [],
|
||||
titles: [title],
|
||||
scannedForTitles,
|
||||
containsBell: false,
|
||||
suppressAttentionEvents
|
||||
})
|
||||
}
|
||||
if (containsBell) {
|
||||
enqueuePtySideEffect({
|
||||
payloads: [],
|
||||
titles: [],
|
||||
scannedForTitles: false,
|
||||
containsBell: true,
|
||||
suppressAttentionEvents
|
||||
})
|
||||
}
|
||||
}
|
||||
if (sideEffectDrainTimer !== null) {
|
||||
return
|
||||
}
|
||||
// Why: xterm.write() buffers parsing onto its own timer. Defer Orca's
|
||||
// title/status/BEL store work so live terminal rendering gets the next turn.
|
||||
sideEffectDrainTimer = setTimeout(drainPtySideEffects, 0)
|
||||
scheduleSideEffectDrain()
|
||||
}
|
||||
|
||||
function clearSideEffectDrainTimer(): void {
|
||||
|
|
@ -195,26 +244,58 @@ export function createPtyOutputProcessor({
|
|||
}
|
||||
}
|
||||
|
||||
function drainPtySideEffects(): void {
|
||||
function compactPendingSideEffectsIfNeeded(force = false): void {
|
||||
if (pendingSideEffectIndex === 0) {
|
||||
return
|
||||
}
|
||||
if (pendingSideEffectIndex >= pendingSideEffects.length) {
|
||||
pendingSideEffects = []
|
||||
pendingSideEffectIndex = 0
|
||||
return
|
||||
}
|
||||
if (force || pendingSideEffectIndex >= MAX_PTY_SIDE_EFFECTS_PER_DRAIN * 4) {
|
||||
pendingSideEffects = pendingSideEffects.slice(pendingSideEffectIndex)
|
||||
pendingSideEffectIndex = 0
|
||||
}
|
||||
}
|
||||
|
||||
function applyPtySideEffect(next: PendingPtySideEffect): void {
|
||||
if (onAgentStatus) {
|
||||
for (const payload of next.payloads) {
|
||||
onAgentStatus(payload)
|
||||
}
|
||||
}
|
||||
processObservedTitles(next.titles, next.scannedForTitles, next.suppressAttentionEvents)
|
||||
if (onBell && next.containsBell) {
|
||||
onBell()
|
||||
}
|
||||
}
|
||||
|
||||
function drainPtySideEffects(options: { flushAll?: boolean } = {}): void {
|
||||
sideEffectDrainTimer = null
|
||||
const effects = pendingSideEffects
|
||||
pendingSideEffects = []
|
||||
for (const next of effects) {
|
||||
if (onAgentStatus) {
|
||||
for (const payload of next.payloads) {
|
||||
onAgentStatus(payload)
|
||||
}
|
||||
}
|
||||
processObservedTitles(next.titles, next.scannedForTitles, next.suppressAttentionEvents)
|
||||
if (onBell && next.containsBell) {
|
||||
onBell()
|
||||
const maxEffects = options.flushAll ? Number.POSITIVE_INFINITY : MAX_PTY_SIDE_EFFECTS_PER_DRAIN
|
||||
let processed = 0
|
||||
while (pendingSideEffectIndex < pendingSideEffects.length && processed < maxEffects) {
|
||||
const next = pendingSideEffects[pendingSideEffectIndex]
|
||||
if (!next) {
|
||||
break
|
||||
}
|
||||
pendingSideEffectIndex += 1
|
||||
processed += 1
|
||||
applyPtySideEffect(next)
|
||||
}
|
||||
compactPendingSideEffectsIfNeeded(options.flushAll === true)
|
||||
if (pendingSideEffectIndex < pendingSideEffects.length) {
|
||||
// Why: long-idle agent CLIs can queue thousands of OSC title/status
|
||||
// facts while Chromium throttles timers. Bound each callback so cursor
|
||||
// blink, paint, and terminal input get chances to run between batches.
|
||||
scheduleSideEffectDrain()
|
||||
}
|
||||
}
|
||||
|
||||
function flushPendingSideEffects(): void {
|
||||
clearSideEffectDrainTimer()
|
||||
drainPtySideEffects()
|
||||
drainPtySideEffects({ flushAll: true })
|
||||
}
|
||||
|
||||
function processObservedTitles(
|
||||
|
|
@ -285,6 +366,7 @@ export function createPtyOutputProcessor({
|
|||
function clearAccumulatedState(): void {
|
||||
clearSideEffectDrainTimer()
|
||||
pendingSideEffects.length = 0
|
||||
pendingSideEffectIndex = 0
|
||||
clearStaleTitleTimer()
|
||||
agentTracker?.reset()
|
||||
bellDetector.reset()
|
||||
|
|
|
|||
Loading…
Reference in New Issue