From 2d59dad52d191a67b10aa158fc628f5a7f765b47 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 14 May 2026 19:26:32 -0400 Subject: [PATCH] fix(terminal): prioritize xterm writes over metadata --- src/renderer/src/assets/terminal.css | 7 - .../terminal-pane/pty-connection.test.ts | 113 +++++----------- .../terminal-pane/pty-connection.ts | 5 - .../pty-dispatcher-pi-routing.test.ts | 10 ++ .../pty-transport-pi-coalesce.test.ts | 6 + .../pty-transport-pi-spinner.test.ts | 7 + .../terminal-pane/pty-transport.test.ts | 31 +++++ .../components/terminal-pane/pty-transport.ts | 124 ++++++++++++------ .../pane-terminal-cursor-suppression.ts | 50 ------- .../pane-terminal-output-scheduler.test.ts | 72 +--------- .../pane-terminal-output-scheduler.ts | 105 +-------------- 11 files changed, 176 insertions(+), 354 deletions(-) delete mode 100644 src/renderer/src/lib/pane-manager/pane-terminal-cursor-suppression.ts diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index 8e44a2ec715..05931b02b59 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -25,13 +25,6 @@ width: 100%; } -.pane-manager-root .xterm.terminal-foreground-write-pending .xterm-cursor, -.pane-manager-root .xterm.terminal-foreground-write-pending .xterm-cursor-layer { - /* Why: active Windows TUI repaints can briefly expose xterm's intermediate - cursor positions; hide the visual cursor only while foreground bytes drain. */ - visibility: hidden; -} - .pane-manager-root .xterm-viewport { /* Why: xterm/VS Code keep the scrollbar gutter stable. `auto` lets Linux DOM scrollbars appear/disappear, which can feed resize/reflow jitter. */ diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 4481a079ed5..5cb40457f98 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -947,98 +947,53 @@ describe('connectPanePty', () => { } }) - it('writes visible split-pane PTY bytes on the foreground flush even when the tab is not active', async () => { - const pendingTimeouts: (() => void)[] = [] - const originalSetTimeout = globalThis.setTimeout - globalThis.setTimeout = vi.fn((fn: () => void) => { - pendingTimeouts.push(fn) - return 999 as unknown as ReturnType - }) as unknown as typeof setTimeout - + it('writes visible split-pane PTY bytes immediately even when the tab is not active', async () => { const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) - try { - const transport = createMockTransport() - const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } - transport.connect.mockImplementation( - async ({ callbacks }: { callbacks: ConnectCallbacks }) => { - capturedDataCallback.current = callbacks.onData ?? null - return 'pty-id' - } - ) - transportFactoryQueue.push(transport) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isActiveRef: { current: false }, + isVisibleRef: { current: true } + }) - const pane = createPane(1) - const manager = createManager(1) - const deps = createDeps({ - isActiveRef: { current: false }, - isVisibleRef: { current: true } - }) + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) - connectPanePty(pane as never, manager as never, deps as never) - await flushAsyncTicks(6) + expect(capturedDataCallback.current).not.toBeNull() + capturedDataCallback.current?.('visible split output\r\n') - expect(capturedDataCallback.current).not.toBeNull() - capturedDataCallback.current?.('visible split output\r\n') - expect(pane.terminal.write).not.toHaveBeenCalledWith('visible split output\r\n') - - for (const fn of pendingTimeouts) { - fn() - } - - expect(pane.terminal.write).toHaveBeenCalledWith( - 'visible split output\r\n', - expect.any(Function) - ) - } finally { - globalThis.setTimeout = originalSetTimeout - } + expect(pane.terminal.write).toHaveBeenCalledWith('visible split output\r\n') }) it('marks panes that receive Arabic output for DOM rendering', async () => { - const pendingTimeouts: (() => void)[] = [] - const originalSetTimeout = globalThis.setTimeout - globalThis.setTimeout = vi.fn((fn: () => void) => { - pendingTimeouts.push(fn) - return 999 as unknown as ReturnType - }) as unknown as typeof setTimeout - const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-id' + }) + transportFactoryQueue.push(transport) - try { - const transport = createMockTransport() - const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } - transport.connect.mockImplementation( - async ({ callbacks }: { callbacks: ConnectCallbacks }) => { - capturedDataCallback.current = callbacks.onData ?? null - return 'pty-id' - } - ) - transportFactoryQueue.push(transport) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() - const pane = createPane(1) - const manager = createManager(1) - const deps = createDeps() + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) - connectPanePty(pane as never, manager as never, deps as never) - await flushAsyncTicks(6) + capturedDataCallback.current?.('Arabic: السلام عليكم\r\n') - capturedDataCallback.current?.('Arabic: السلام عليكم\r\n') - - expect(manager.markPaneHasComplexScriptOutput).toHaveBeenCalledWith(1) - expect(pane.terminal.write).not.toHaveBeenCalledWith('Arabic: السلام عليكم\r\n') - - for (const fn of pendingTimeouts) { - fn() - } - - expect(pane.terminal.write).toHaveBeenCalledWith( - 'Arabic: السلام عليكم\r\n', - expect.any(Function) - ) - } finally { - globalThis.setTimeout = originalSetTimeout - } + expect(manager.markPaneHasComplexScriptOutput).toHaveBeenCalledWith(1) + expect(pane.terminal.write).toHaveBeenCalledWith('Arabic: السلام عليكم\r\n') }) it('reattaches via daemon sessionId when an in-session PTY is live', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 5457b1b9895..199aede386a 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -21,7 +21,6 @@ import { getRemoteRuntimePtyEnvironmentId } from '@/runtime/runtime-terminal-str import { discardTerminalOutput, flushTerminalOutput, - suppressTerminalCursorUntilOutputSettles, waitForTerminalOutputParsed, writeTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-scheduler' @@ -464,10 +463,6 @@ export function connectPanePty( // auto-replies never count as interaction. deps.clearTerminalTabUnread(deps.tabId) deps.clearWorktreeUnread(deps.worktreeId) - flushTerminalOutput(pane.terminal) - // Why: on Windows, key-repeat can leave xterm's old visual cursor painted - // until the TUI echoes its next repaint. Hide that stale frame immediately. - suppressTerminalCursorUntilOutputSettles(pane.terminal) transport.sendInput(data) }) diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts index 1bfeae94eed..882db26e74f 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts @@ -11,6 +11,10 @@ const BEL = '\x07' const workingFrame = (frame: string): string => `${ESC}]0;${frame} π - cwd${BEL}` const idleTitle = (): string => `${ESC}]0;π - cwd${BEL}` +function flushPtySideEffects(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)) +} + describe('dispatcher → transport → onTitleChange for Pi spinner', () => { const originalWindow = (globalThis as { window?: typeof window }).window @@ -74,6 +78,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { dispatcherCallback?.({ id: 'pty-pi', data: workingFrame('⠋') }) dispatcherCallback?.({ id: 'pty-pi', data: workingFrame('⠙') }) dispatcherCallback?.({ id: 'pty-pi', data: idleTitle() }) + await flushPtySideEffects() const seenTitles = onTitleChange.mock.calls.map((c) => c[0]) expect(seenTitles).toContain('⠋ Pi') @@ -96,6 +101,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { id: 'pty-pi', data: `assistant output line 1\r\n${workingFrame('⠋')}more body text` }) + await flushPtySideEffects() const seenTitles = onTitleChange.mock.calls.map((c) => c[0]) expect(seenTitles).toContain('⠋ Pi') @@ -121,6 +127,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { onTitleChange.mockClear() dispatcherCallback?.({ id: 'pty-pi', data: workingFrame('⠋') }) + await flushPtySideEffects() const seenTitles = onTitleChange.mock.calls.map((c) => c[0]) expect(seenTitles).toContain('⠋ Pi') @@ -143,6 +150,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { dispatcherCallback?.({ id: 'pty-pi', data: workingFrame('⠋') }) dispatcherCallback?.({ id: 'pty-pi', data: workingFrame('⠙') }) dispatcherCallback?.({ id: 'pty-pi', data: idleTitle() }) + await flushPtySideEffects() const seenTitles = onTitleChange.mock.calls.map((c) => c[0]) const workingIdx = seenTitles.findIndex((t) => t === '⠋ Pi') @@ -177,6 +185,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { dispatcherCallback?.({ id: 'pty-pi', data: `${ESC}]0;Cursor Agent${BEL}` }) dispatcherCallback?.({ id: 'pty-pi', data: `${ESC}]0;⠙ Cursor Agent${BEL}` }) dispatcherCallback?.({ id: 'pty-pi', data: `${ESC}]0;Cursor Agent${BEL}` }) + await flushPtySideEffects() const seenTitles = onTitleChange.mock.calls.map((c) => c[0]) // The two bare "Cursor Agent" titles must NOT reach the title-change @@ -200,6 +209,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { dispatcherCallback?.({ id: 'pty-pi', data: `${ESC}]0;⠋ Cursor Agent${BEL}` }) dispatcherCallback?.({ id: 'pty-pi', data: `${ESC}]0;Cursor Agent${BEL}` }) dispatcherCallback?.({ id: 'pty-pi', data: `${ESC}]0;Cursor ready${BEL}${BEL}` }) + await flushPtySideEffects() const seenTitles = onTitleChange.mock.calls.map((c) => c[0]) expect(seenTitles).toContain('⠋ Cursor Agent') diff --git a/src/renderer/src/components/terminal-pane/pty-transport-pi-coalesce.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-pi-coalesce.test.ts index 398fa668e29..808cedad5e9 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-pi-coalesce.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-pi-coalesce.test.ts @@ -14,6 +14,10 @@ const BEL = '\x07' const workingFrame = (frame: string): string => `${ESC}]0;${frame} π - cwd${BEL}` const idleTitle = (): string => `${ESC}]0;π - cwd${BEL}` +function flushPtySideEffects(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)) +} + describe('pty-transport — coalesced OSC titles from Pi', () => { const originalWindow = (globalThis as { window?: typeof window }).window let onData: ((payload: { id: string; data: string }) => void) | null = null @@ -67,6 +71,7 @@ describe('pty-transport — coalesced OSC titles from Pi', () => { // agent_end fires stopAnimation -> trailing idle title after working frames const chunk = `${workingFrame('⠋')}some response text\r\n${workingFrame('⠙')}more response text\r\n${idleTitle()}` onData?.({ id: 'pty-pi', data: chunk }) + await flushPtySideEffects() const seen = onTitleChange.mock.calls.map((c) => c[0]) // Users expect the working state to register SOMEWHERE in the sequence, @@ -93,6 +98,7 @@ describe('pty-transport — coalesced OSC titles from Pi', () => { const framesChars = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'] const chunk = framesChars.map(workingFrame).join('body\r\n') onData?.({ id: 'pty-pi', data: chunk }) + await flushPtySideEffects() const seen = onTitleChange.mock.calls.map((c) => c[0]) expect(seen).toContain('⠋ Pi') diff --git a/src/renderer/src/components/terminal-pane/pty-transport-pi-spinner.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-pi-spinner.test.ts index 23c55648f32..c556993c8c9 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-pi-spinner.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-pi-spinner.test.ts @@ -13,6 +13,10 @@ const BEL = '\x07' const workingFrame = (frame: string): string => `${ESC}]0;${frame} π - cwd${BEL}` const idleTitle = (): string => `${ESC}]0;π - cwd${BEL}` +function flushPtySideEffects(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)) +} + describe('createIpcPtyTransport — Pi titlebar spinner signal', () => { const originalWindow = (globalThis as { window?: typeof window }).window let onData: ((payload: { id: string; data: string }) => void) | null = null @@ -64,6 +68,7 @@ describe('createIpcPtyTransport — Pi titlebar spinner signal', () => { onData?.({ id: 'pty-pi', data: workingFrame('⠙') }) // ⠙ onData?.({ id: 'pty-pi', data: workingFrame('⠹') }) // ⠹ onData?.({ id: 'pty-pi', data: idleTitle() }) + await flushPtySideEffects() const normalized = onTitleChange.mock.calls.map((c) => c[0]) // The store only stores the normalized label, which is what the worktree @@ -91,6 +96,7 @@ describe('createIpcPtyTransport — Pi titlebar spinner signal', () => { onData?.({ id: 'pty-pi', data: workingFrame('⠋') }) onData?.({ id: 'pty-pi', data: workingFrame('⠙') }) + await flushPtySideEffects() const calls = onTitleChange.mock.calls.map((c) => c[0]) expect(calls).toContain('⠋ Pi') @@ -115,6 +121,7 @@ describe('createIpcPtyTransport — Pi titlebar spinner signal', () => { const coalescedWorking = `${workingFrame('⠋')}output\r\n${workingFrame('⠙')}` onData?.({ id: 'pty-pi', data: coalescedWorking }) + await flushPtySideEffects() const calls = onTitleChange.mock.calls.map((c) => c[0]) expect(calls).toContain('⠋ Pi') diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 1164b439282..1fa7cf84e49 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -14,6 +14,10 @@ describe('createIpcPtyTransport', () => { let onData: ((payload: { id: string; data: string }) => void) | null = null let onExit: ((payload: { id: string; code: number }) => void) | null = null + function flushPtySideEffects(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)) + } + beforeEach(() => { vi.resetModules() onData = null @@ -66,6 +70,27 @@ describe('createIpcPtyTransport', () => { transport.disconnect() }) + it('defers title side effects until after terminal data is delivered', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const onTitleChange = vi.fn() + const onDataCallback = vi.fn(() => { + expect(onTitleChange).not.toHaveBeenCalled() + }) + const transport = createIpcPtyTransport({ onTitleChange }) + + await transport.connect({ url: '', callbacks: { onData: onDataCallback } }) + + onData?.({ id: 'pty-1', data: ']0;title-onebody' }) + + expect(onDataCallback).toHaveBeenCalledWith(']0;title-onebody') + expect(onTitleChange).not.toHaveBeenCalled() + + await flushPtySideEffects() + + expect(onTitleChange).toHaveBeenCalledWith('title-one', 'title-one') + transport.disconnect() + }) + it('suppresses attention side effects when replaying eager-buffered data during attach', async () => { // Why: eager PTY buffers capture output produced before the pane mounted — // typically catch-up bytes from a previous app session. A BEL or @@ -95,6 +120,7 @@ describe('createIpcPtyTransport', () => { }) expect(handle.flush()).toBe('') + await flushPtySideEffects() expect(onTitleChange).toHaveBeenCalledWith('* Claude done', '* Claude done') expect(onBell).not.toHaveBeenCalled() expect(onAgentBecameIdle).not.toHaveBeenCalled() @@ -135,10 +161,12 @@ describe('createIpcPtyTransport', () => { onData?.({ id: 'pty-1', data: ']0;title-one' }) onData?.({ id: 'pty-1', data: ']0;title-two' }) onData?.({ id: 'pty-1', data: ']0;title-three' }) + await flushPtySideEffects() expect(onBell).not.toHaveBeenCalled() // Bare BEL outside any OSC: fires once. onData?.({ id: 'pty-1', data: '' }) + await flushPtySideEffects() expect(onBell).toHaveBeenCalledTimes(1) }) @@ -393,6 +421,7 @@ describe('createIpcPtyTransport', () => { // Agent starts working onData?.({ id: 'pty-1', data: ']0;. Claude working' }) + await flushPtySideEffects() expect(onAgentBecameWorking).toHaveBeenCalledTimes(1) // Simulate shutdownWorktreeTerminals: unregister data handlers before kill. @@ -428,10 +457,12 @@ describe('createIpcPtyTransport', () => { // Agent starts working — sets the title to a working indicator onData?.({ id: 'pty-1', data: ']0;. Claude working' }) + vi.advanceTimersByTime(0) expect(onAgentBecameWorking).toHaveBeenCalledTimes(1) // Data arrives without a title change — starts the 3 s staleTitleTimer onData?.({ id: 'pty-1', data: 'some output without title\r\n' }) + vi.advanceTimersByTime(0) // Simulate shutdownWorktreeTerminals: unregister handlers which should // cancel the pending staleTitleTimer AND reset the agent tracker so the diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 8ccc6e8a300..954a04841b3 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -81,6 +81,12 @@ export function createPtyOutputProcessor({ const processAgentStatusChunk = createAgentStatusOscProcessor() let lastEmittedTitle: string | null = null let staleTitleTimer: ReturnType | null = null + let sideEffectDrainTimer: ReturnType | null = null + const pendingSideEffects: { + data: string + payloads: ReturnType['payloads'] + suppressAttentionEvents: boolean + }[] = [] const agentTracker = onAgentBecameIdle || onAgentBecameWorking || onAgentExited ? createAgentStatusTracker( @@ -92,7 +98,7 @@ export function createPtyOutputProcessor({ ) : null - function applyObservedTerminalTitle(title: string): void { + function applyObservedTerminalTitle(title: string, suppressAgentTracker = false): void { // Why: cursor-agent's native OSC title is the literal string "Cursor Agent" // and it re-emits that title many times per turn (on every internal redraw) // even while it's actively working. Orca drives the cursor spinner/unread @@ -110,7 +116,9 @@ export function createPtyOutputProcessor({ } lastEmittedTitle = normalizeTerminalTitle(title) onTitleChange?.(lastEmittedTitle, title) - agentTracker?.handleTitle(title) + if (!suppressAgentTracker) { + agentTracker?.handleTitle(title) + } } function clearStaleTitleTimer(): void { @@ -120,6 +128,71 @@ export function createPtyOutputProcessor({ } } + function schedulePtySideEffects( + data: string, + payloads: ReturnType['payloads'], + suppressAttentionEvents: boolean + ): void { + pendingSideEffects.push({ data, payloads, 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) + } + + function drainPtySideEffects(): void { + sideEffectDrainTimer = null + while (pendingSideEffects.length > 0) { + const next = pendingSideEffects.shift() + if (!next) { + continue + } + if (onAgentStatus && !next.suppressAttentionEvents) { + for (const payload of next.payloads) { + onAgentStatus(payload) + } + } + processObservedTitles(next.data, next.suppressAttentionEvents) + if (onBell && bellDetector.chunkContainsBell(next.data) && !next.suppressAttentionEvents) { + onBell() + } + } + } + + function processObservedTitles(data: string, suppressAgentTracker: boolean): void { + if (!onTitleChange) { + return + } + // Why: feed EVERY OSC title in the chunk through the observer, not just + // the last one. node-pty + the main-process 8ms batch window commonly + // coalesce multiple title updates into a single IPC payload; processing + // titles in order preserves working-to-idle transitions. + const titles = extractAllOscTitles(data) + if (titles.length > 0) { + clearStaleTitleTimer() + for (const title of titles) { + applyObservedTerminalTitle(title, suppressAgentTracker) + } + } else if ( + !suppressAgentTracker && + lastEmittedTitle && + detectAgentStatusFromTitle(lastEmittedTitle) === 'working' + ) { + clearStaleTitleTimer() + staleTitleTimer = setTimeout(() => { + staleTitleTimer = null + if (lastEmittedTitle && detectAgentStatusFromTitle(lastEmittedTitle) === 'working') { + const cleared = clearWorkingIndicators(lastEmittedTitle) + lastEmittedTitle = cleared + onTitleChange(cleared, cleared) + agentTracker?.handleTitle(cleared) + } + }, STALE_TITLE_TIMEOUT) + } + } + function processData( data: string, callbacks: PtyOutputCallbacks, @@ -135,55 +208,20 @@ export function createPtyOutputProcessor({ // replay we must not surface stale agent-status payloads from a prior app // session into the live store. The parser still consumes the bytes so they // do not leak into xterm, we just suppress the callback. - if (onAgentStatus && !suppressAttentionEvents) { - for (const payload of processed.payloads) { - onAgentStatus(payload) - } - } if (options.replayingBufferedData && callbacks.onReplayData) { callbacks.onReplayData(data) } else { callbacks.onData?.(data) } - if (onTitleChange) { - // Why: feed EVERY OSC title in the chunk through the observer, not just - // the last one. node-pty + the main-process 8ms batch window commonly - // coalesce multiple title updates into a single IPC payload — for Pi's - // 80ms spinner + agent_end idle cycle, the last title in the chunk is - // the idle one and the intermediate working frames were silently - // dropped, so the worktree card never observed the working state. - // Processing titles in order preserves the working→idle transition - // that detectAgentStatusFromTitle and agentTracker both key off. - const titles = extractAllOscTitles(data) - if (titles.length > 0) { - clearStaleTitleTimer() - for (const title of titles) { - applyObservedTerminalTitle(title) - } - } else if (lastEmittedTitle && detectAgentStatusFromTitle(lastEmittedTitle) === 'working') { - clearStaleTitleTimer() - staleTitleTimer = setTimeout(() => { - staleTitleTimer = null - if (lastEmittedTitle && detectAgentStatusFromTitle(lastEmittedTitle) === 'working') { - const cleared = clearWorkingIndicators(lastEmittedTitle) - lastEmittedTitle = cleared - onTitleChange(cleared, cleared) - agentTracker?.handleTitle(cleared) - } - }, STALE_TITLE_TIMEOUT) - } - } - // Why: BEL is the attention signal. The detector is stateful across - // chunks so a BEL sitting inside an OSC sequence (e.g. Claude's - // `\e]0;title\a`) is correctly ignored — only true terminal bells raise - // attention. suppressAttentionEvents gates this during eager-buffer replay - // so historical BELs do not produce fresh alerts on cold reattach. - if (onBell && bellDetector.chunkContainsBell(data) && !suppressAttentionEvents) { - onBell() - } + schedulePtySideEffects(data, processed.payloads, suppressAttentionEvents) } function clearAccumulatedState(): void { + if (sideEffectDrainTimer) { + clearTimeout(sideEffectDrainTimer) + sideEffectDrainTimer = null + } + pendingSideEffects.length = 0 clearStaleTitleTimer() agentTracker?.reset() bellDetector.reset() diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-cursor-suppression.ts b/src/renderer/src/lib/pane-manager/pane-terminal-cursor-suppression.ts deleted file mode 100644 index edcf087e111..00000000000 --- a/src/renderer/src/lib/pane-manager/pane-terminal-cursor-suppression.ts +++ /dev/null @@ -1,50 +0,0 @@ -export type TerminalCursorSuppressionTarget = { - element?: { classList: Pick } | null -} - -const FOREGROUND_CURSOR_RESTORE_DELAY_MS = 64 -const FOREGROUND_CURSOR_RESTORE_SAFETY_MS = 500 - -export const FOREGROUND_CURSOR_SUPPRESSED_CLASS = 'terminal-foreground-write-pending' -export const FOREGROUND_CURSOR_RESTORE_SAFETY_DELAY_MS = FOREGROUND_CURSOR_RESTORE_SAFETY_MS - -const restoreTimerByTerminal = new WeakMap< - TerminalCursorSuppressionTarget, - ReturnType ->() - -function clearRestoreTimer(terminal: TerminalCursorSuppressionTarget): void { - const timer = restoreTimerByTerminal.get(terminal) - if (timer) { - clearTimeout(timer) - restoreTimerByTerminal.delete(terminal) - } -} - -export function restoreForegroundTerminalCursor(terminal: TerminalCursorSuppressionTarget): void { - clearRestoreTimer(terminal) - terminal.element?.classList.remove(FOREGROUND_CURSOR_SUPPRESSED_CLASS) -} - -export function scheduleForegroundTerminalCursorRestore( - terminal: TerminalCursorSuppressionTarget, - delayMs = FOREGROUND_CURSOR_RESTORE_DELAY_MS -): void { - if (!terminal.element) { - return - } - clearRestoreTimer(terminal) - const timer = setTimeout(() => { - restoreTimerByTerminal.delete(terminal) - terminal.element?.classList.remove(FOREGROUND_CURSOR_SUPPRESSED_CLASS) - }, delayMs) - restoreTimerByTerminal.set(terminal, timer) -} - -export function suppressForegroundTerminalCursor(terminal: TerminalCursorSuppressionTarget): void { - if (!terminal.element) { - return - } - clearRestoreTimer(terminal) - terminal.element.classList.add(FOREGROUND_CURSOR_SUPPRESSED_CLASS) -} diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts index 457621d47f2..11ecafa89b0 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -1,19 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' function createTerminal() { - const classes = new Set() return { - classes, - element: { - classList: { - add: vi.fn((className: string) => { - classes.add(className) - }), - remove: vi.fn((className: string) => { - classes.delete(className) - }) - } - }, write: vi.fn((_data: string, callback?: () => void) => { callback?.() }) @@ -30,67 +18,13 @@ describe('pane terminal output scheduler', () => { vi.useRealTimers() }) - it('coalesces foreground output until the next frame flush', async () => { - vi.useFakeTimers() + it('writes foreground output immediately', async () => { const { writeTerminalOutput } = await loadScheduler() const terminal = createTerminal() - writeTerminalOutput(terminal, 'a', { foreground: true }) - writeTerminalOutput(terminal, 'b', { foreground: true }) - - expect(terminal.write).not.toHaveBeenCalled() - vi.advanceTimersByTime(16) - - expect(terminal.write).toHaveBeenCalledTimes(1) - expect(terminal.write).toHaveBeenCalledWith('ab', expect.any(Function)) - }) - - it('hides the foreground cursor until output parsing has gone quiet', async () => { - vi.useFakeTimers() - const { writeTerminalOutput } = await loadScheduler() - const terminal = createTerminal() - - writeTerminalOutput(terminal, 'frame', { foreground: true }) - - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(true) - - vi.advanceTimersByTime(16) - - expect(terminal.write).toHaveBeenCalledWith('frame', expect.any(Function)) - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(true) - - vi.advanceTimersByTime(63) - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(true) - - vi.advanceTimersByTime(1) - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(false) - }) - - it('can hide the cursor immediately while input waits for echoed output', async () => { - vi.useFakeTimers() - const { suppressTerminalCursorUntilOutputSettles } = await loadScheduler() - const terminal = createTerminal() - - suppressTerminalCursorUntilOutputSettles(terminal) - - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(true) - - vi.advanceTimersByTime(499) - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(true) - - vi.advanceTimersByTime(1) - expect(terminal.classes.has('terminal-foreground-write-pending')).toBe(false) - }) - - it('flushes foreground output synchronously when requested', async () => { - vi.useFakeTimers() - const { flushTerminalOutput, writeTerminalOutput } = await loadScheduler() - const terminal = createTerminal() - writeTerminalOutput(terminal, 'foreground', { foreground: true }) - flushTerminalOutput(terminal) - expect(terminal.write).toHaveBeenCalledWith('foreground', expect.any(Function)) + expect(terminal.write).toHaveBeenCalledWith('foreground') }) it('coalesces background output until the shared drain runs', async () => { @@ -157,8 +91,6 @@ describe('pane terminal output scheduler', () => { writeTerminalOutput(terminal, 'old', { foreground: false }) writeTerminalOutput(terminal, 'new', { foreground: true }) - expect(terminal.write.mock.calls.map(([data]) => data)).toEqual(['old']) - vi.advanceTimersByTime(16) expect(terminal.write.mock.calls.map(([data]) => data)).toEqual(['old', 'new']) }) diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 4713f89c15a..8e1f02a581c 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -1,13 +1,6 @@ import { e2eConfig } from '@/lib/e2e-config' -import { - FOREGROUND_CURSOR_RESTORE_SAFETY_DELAY_MS, - restoreForegroundTerminalCursor, - scheduleForegroundTerminalCursorRestore, - suppressForegroundTerminalCursor, - type TerminalCursorSuppressionTarget -} from './pane-terminal-cursor-suppression' -type TerminalOutputTarget = TerminalCursorSuppressionTarget & { +type TerminalOutputTarget = { write(data: string, callback?: () => void): void } @@ -19,14 +12,11 @@ type QueueEntry = { const BACKGROUND_FLUSH_DELAY_MS = 50 const BACKGROUND_DRAIN_INTERVAL_MS = 16 const BACKGROUND_CHUNK_CHARS = 16 * 1024 -const FOREGROUND_FLUSH_DELAY_MS = 16 const MAX_WRITES_PER_DRAIN = 2 const PARSE_SETTLE_TIMEOUT_MS = 250 const queuedByTerminal = new Map() -const foregroundQueuedByTerminal = new Map() let drainTimer: ReturnType | null = null -let foregroundDrainTimer: ReturnType | null = null const debugEnabled = e2eConfig.exposeStore // Why no lossy queue cap: dropping raw terminal bytes can corrupt parser state @@ -38,10 +28,8 @@ type TerminalOutputSchedulerDebugSnapshot = { backgroundEnqueueCount: number foregroundWriteCount: number backgroundWriteCount: number - foregroundBatchedWriteCount: number flushWriteCount: number scheduledDrainCount: number - scheduledForegroundDrainCount: number drainWrites: number[] } @@ -54,10 +42,8 @@ const debugState: TerminalOutputSchedulerDebugSnapshot = { backgroundEnqueueCount: 0, foregroundWriteCount: 0, backgroundWriteCount: 0, - foregroundBatchedWriteCount: 0, flushWriteCount: 0, scheduledDrainCount: 0, - scheduledForegroundDrainCount: 0, drainWrites: [] } @@ -65,10 +51,8 @@ function resetDebugState(): void { debugState.backgroundEnqueueCount = 0 debugState.foregroundWriteCount = 0 debugState.backgroundWriteCount = 0 - debugState.foregroundBatchedWriteCount = 0 debugState.flushWriteCount = 0 debugState.scheduledDrainCount = 0 - debugState.scheduledForegroundDrainCount = 0 debugState.drainWrites = [] } @@ -100,19 +84,6 @@ function scheduleDrain(delayMs: number): void { drainTimer = setTimeout(drainQueuedOutput, delayMs) } -function scheduleForegroundDrain(): void { - if (foregroundDrainTimer !== null) { - return - } - if (debugEnabled) { - debugState.scheduledForegroundDrainCount++ - } - // Why: terminal TUIs repaint by moving the cursor through intermediate - // positions. Coalescing visible PTY bursts to one frame keeps those parser - // states from being painted as cursor flicker, especially on Windows xterm. - foregroundDrainTimer = setTimeout(drainForegroundOutput, FOREGROUND_FLUSH_DELAY_MS) -} - function takeQueuedChunk(entry: QueueEntry, limit: number): string { let remaining = limit let data = '' @@ -134,29 +105,6 @@ function takeQueuedChunk(entry: QueueEntry, limit: number): string { return data } -function writeForegroundQueuedChunks(entry: QueueEntry): void { - let data = takeQueuedChunk(entry, Number.POSITIVE_INFINITY) - while (data) { - suppressForegroundTerminalCursor(entry.terminal) - // Why: xterm's write callback marks parser completion, but a disposed - // terminal may never call it. The safety restore avoids a stuck hidden cursor. - scheduleForegroundTerminalCursorRestore( - entry.terminal, - FOREGROUND_CURSOR_RESTORE_SAFETY_DELAY_MS - ) - try { - entry.terminal.write(data, () => { - scheduleForegroundTerminalCursorRestore(entry.terminal) - }) - } catch { - entry.chunks.length = 0 - restoreForegroundTerminalCursor(entry.terminal) - return - } - data = takeQueuedChunk(entry, Number.POSITIVE_INFINITY) - } -} - function writeQueuedChunk(entry: QueueEntry): boolean { const data = takeQueuedChunk(entry, BACKGROUND_CHUNK_CHARS) if (!data) { @@ -174,19 +122,6 @@ function writeQueuedChunk(entry: QueueEntry): boolean { return true } -function drainForegroundOutput(): void { - foregroundDrainTimer = null - const entries = [...foregroundQueuedByTerminal.values()] - foregroundQueuedByTerminal.clear() - - for (const entry of entries) { - if (debugEnabled) { - debugState.foregroundBatchedWriteCount++ - } - writeForegroundQueuedChunks(entry) - } -} - function drainQueuedOutput(): void { drainTimer = null let writes = 0 @@ -228,21 +163,11 @@ export function writeTerminalOutput( } if (options.foreground) { - flushBackgroundTerminalOutput(terminal) + flushTerminalOutput(terminal) if (debugEnabled) { debugState.foregroundWriteCount++ } - let entry = foregroundQueuedByTerminal.get(terminal) - if (!entry) { - entry = { terminal, chunks: [] } - foregroundQueuedByTerminal.set(terminal, entry) - } - entry.chunks.push(data) - // Why: Windows can paint xterm's cursor at intermediate TUI repaint - // positions before the foreground batch drains. Hide only the cursor layer; - // terminal bytes still parse normally and the cursor returns after quiet. - suppressForegroundTerminalCursor(terminal) - scheduleForegroundDrain() + terminal.write(data) return } @@ -261,7 +186,8 @@ export function writeTerminalOutput( scheduleDrain(BACKGROUND_FLUSH_DELAY_MS) } -function flushBackgroundTerminalOutput(terminal: TerminalOutputTarget): void { +export function flushTerminalOutput(terminal: TerminalOutputTarget): void { + exposeDebugApi() const entry = queuedByTerminal.get(terminal) if (!entry) { return @@ -285,25 +211,6 @@ function flushBackgroundTerminalOutput(terminal: TerminalOutputTarget): void { } } -export function flushTerminalOutput(terminal: TerminalOutputTarget): void { - exposeDebugApi() - const foregroundEntry = foregroundQueuedByTerminal.get(terminal) - if (foregroundEntry) { - foregroundQueuedByTerminal.delete(terminal) - if (debugEnabled) { - debugState.flushWriteCount++ - } - writeForegroundQueuedChunks(foregroundEntry) - } - - flushBackgroundTerminalOutput(terminal) -} - -export function suppressTerminalCursorUntilOutputSettles(terminal: TerminalOutputTarget): void { - suppressForegroundTerminalCursor(terminal) - scheduleForegroundTerminalCursorRestore(terminal, FOREGROUND_CURSOR_RESTORE_SAFETY_DELAY_MS) -} - export function waitForTerminalOutputParsed(terminal: TerminalOutputTarget): Promise { flushTerminalOutput(terminal) @@ -331,9 +238,7 @@ export function waitForTerminalOutputParsed(terminal: TerminalOutputTarget): Pro export function discardTerminalOutput(terminal: TerminalOutputTarget): void { exposeDebugApi() - foregroundQueuedByTerminal.delete(terminal) queuedByTerminal.delete(terminal) - restoreForegroundTerminalCursor(terminal) } exposeDebugApi()