diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 7f1f0b79923..cf1f063ec8a 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -216,7 +216,7 @@ function App(): React.JSX.Element { // inline agents section. The retention hooks are hosted inside // (a leaf component that renders null) rather // than being called inline here so its high-churn store subscriptions - // (agentStatusByPaneKey + agentStatusEpoch tick at PTY event frequency) + // (agentStatusByPaneKey ticks at PTY event frequency) // do not re-render the App tree on every agent status update. // Why: git conflict-operation state also drives the worktree cards. Polling // cannot live under RightSidebar because App unmounts that subtree when the @@ -927,9 +927,8 @@ function App(): React.JSX.Element { } > - {/* Why: leaf-mounted retention sync — hosting useDashboardData() + - useRetainedAgentsSync() inside a null-rendering leaf keeps their - high-churn store subscriptions from re-rendering the App tree. */} + {/* Why: leaf-mounted retention sync keeps agent-status retention + subscriptions from re-rendering the App tree. */}
{/* Why: the non-workspace titlebar lives inside this left+center diff --git a/src/renderer/src/components/dashboard/RetainedAgentsSyncGate.tsx b/src/renderer/src/components/dashboard/RetainedAgentsSyncGate.tsx index ca0efecc6fc..cf9a231397c 100644 --- a/src/renderer/src/components/dashboard/RetainedAgentsSyncGate.tsx +++ b/src/renderer/src/components/dashboard/RetainedAgentsSyncGate.tsx @@ -1,15 +1,11 @@ -import { useDashboardData } from './useDashboardData' import { useRetainedAgentsSync } from './useRetainedAgents' // Why: isolate the retention subscriptions in a leaf component that renders -// null, so the high-churn slices read by useDashboardData -// (agentStatusByPaneKey + agentStatusEpoch, which tick at PTY event frequency) -// do not re-render the entire App tree. Retention must still run at the App -// level — if it only ran when a single card was mounted, "done" agents would -// vanish from the inline agents list any time the user scrolled that card -// out of view. +// null, so agent-status retention work does not re-render the entire App tree. +// Retention must still run at the App level — if it only ran when a single +// card was mounted, "done" agents would vanish from the inline agents list any +// time the user scrolled that card out of view. export default function RetainedAgentsSyncGate(): null { - const dashboardLiveGroups = useDashboardData() - useRetainedAgentsSync(dashboardLiveGroups) + useRetainedAgentsSync() return null } diff --git a/src/renderer/src/components/dashboard/useDashboardData.ts b/src/renderer/src/components/dashboard/useDashboardData.ts index fb62d30edfb..1385f8a9d05 100644 --- a/src/renderer/src/components/dashboard/useDashboardData.ts +++ b/src/renderer/src/components/dashboard/useDashboardData.ts @@ -23,10 +23,9 @@ export type DashboardAgentRow = { startedAt: number } -// Why: the shape here is deliberately minimal — just what useRetainedAgentsSync -// needs to diff liveGroups and decide which vanished agents to retain. The -// per-card rendering pipeline is separate (WorktreeCardAgents + -// useWorktreeAgentRows read retained entries directly from the store). +// Why: the shape here is deliberately minimal. The per-card rendering pipeline +// is separate (WorktreeCardAgents + useWorktreeAgentRows read retained entries +// directly from the store). export type DashboardWorktreeCard = { repo: Repo worktree: Worktree @@ -134,10 +133,7 @@ function buildDashboardData( // ─── Hook ───────────────────────────────────────────────────────────────────── /** - * Cross-worktree aggregate of live agent rows. Used by useRetainedAgentsSync - * to drive retention: when a previously-live 'done' agent disappears from - * this set, its snapshot is moved into retainedAgentsByPaneKey so the inline - * per-card list can still render it. + * Cross-worktree aggregate of live agent rows. * * Not used to render anything directly — the inline list reads its own * worktree-scoped slice via useWorktreeAgentRows. diff --git a/src/renderer/src/components/dashboard/useRetainedAgents.test.ts b/src/renderer/src/components/dashboard/useRetainedAgents.test.ts index 64f2864d5d4..cfab009a172 100644 --- a/src/renderer/src/components/dashboard/useRetainedAgents.test.ts +++ b/src/renderer/src/components/dashboard/useRetainedAgents.test.ts @@ -1,12 +1,8 @@ import { describe, expect, it } from 'vitest' -import type { AgentStatusEntry } from '../../../../shared/agent-status-types' +import type { AgentStatusEntry, AgentStatusState } from '../../../../shared/agent-status-types' import { collectRetainedAgentsOnDisappear } from './useRetainedAgents' -function makeAgentRow(args: { - paneKey: string - state: 'working' | 'blocked' | 'waiting' | 'done' - interrupted?: boolean -}) { +function makeAgentRow(args: { paneKey: string; state: AgentStatusState; interrupted?: boolean }) { const entry: AgentStatusEntry = { state: args.state, prompt: 'Fix it', diff --git a/src/renderer/src/components/dashboard/useRetainedAgents.ts b/src/renderer/src/components/dashboard/useRetainedAgents.ts index b8c11460b70..bb3d0f8271c 100644 --- a/src/renderer/src/components/dashboard/useRetainedAgents.ts +++ b/src/renderer/src/components/dashboard/useRetainedAgents.ts @@ -1,47 +1,187 @@ import { useEffect, useRef } from 'react' import { useAppStore } from '@/store' -import { type DashboardRepoGroup, type DashboardAgentRow } from './useDashboardData' +import { isExplicitAgentStatusFresh } from '@/lib/agent-status' +import { type DashboardAgentRow } from './useDashboardData' import type { RetainedAgentEntry } from '@/store/slices/agent-status' +import type { Repo, TerminalTab, Worktree } from '../../../../shared/types' +import { + AGENT_STATUS_STALE_AFTER_MS, + type AgentStatusEntry +} from '../../../../shared/agent-status-types' // Why: when an agent finishes or its terminal closes, the store cleans up the -// explicit status entry and the agent vanishes from useDashboardData. Retaining -// the last-known "done" snapshot in the store lets the inline per-card agents -// list render the done row until the user dismisses it, rather than having the -// row wink out the moment the terminal process exits. +// explicit status entry and the agent vanishes from the live status set. +// Retaining the last-known "done" snapshot in the store lets the inline +// per-card agents list render the done row until the user dismisses it, rather +// than having the row wink out the moment the terminal process exits. -export function useRetainedAgentsSync(liveGroups: DashboardRepoGroup[]): void { +type RetainedAgentSnapshot = Map + +type RetainedAgentsSyncInputs = { + repos: Repo[] + worktreesByRepo: Record + tabsByWorktree: Record + agentStatusByPaneKey: Record + agentStatusEpoch?: number +} + +type RetainedAgentsSyncSnapshotInputs = RetainedAgentsSyncInputs & { + now: number +} + +function paneKeyTabId(paneKey: string): string | null { + const colonIndex = paneKey.indexOf(':') + if (colonIndex <= 0) { + return null + } + return paneKey.slice(0, colonIndex) +} + +function buildLiveTabIndex(args: { + repos: Repo[] + worktreesByRepo: Record + tabsByWorktree: Record +}): { + existingWorktreeIds: Set + tabIndex: Map +} { + const existingWorktreeIds = new Set() + const tabIndex = new Map() + + for (const repo of args.repos) { + const worktrees = args.worktreesByRepo[repo.id] ?? [] + for (const worktree of worktrees) { + if (worktree.isArchived) { + continue + } + existingWorktreeIds.add(worktree.id) + const tabs = args.tabsByWorktree[worktree.id] ?? [] + for (const tab of tabs) { + tabIndex.set(tab.id, { tab, worktreeId: worktree.id }) + } + } + } + + return { existingWorktreeIds, tabIndex } +} + +function agentStartedAt(entry: AgentStatusEntry): number { + return entry.stateHistory[0]?.startedAt ?? entry.stateStartedAt +} + +export function buildRetainedAgentsSyncSignature(args: RetainedAgentsSyncInputs): string { + const { existingWorktreeIds, tabIndex } = buildLiveTabIndex(args) + const worktreeParts = [...existingWorktreeIds].sort() + const tabParts = [...tabIndex.entries()] + .map(([tabId, owner]) => `${owner.worktreeId}:${tabId}`) + .sort() + const agentParts: string[] = [] + + for (const [paneKey, entry] of Object.entries(args.agentStatusByPaneKey)) { + const tabId = paneKeyTabId(paneKey) + if (!tabId) { + continue + } + const owner = tabIndex.get(tabId) + if (!owner) { + continue + } + // Why: working/blocked/waiting pings can update prompt/tool text dozens of + // times per second; retention only cares about identity, state, freshness, + // and final done payloads. + const doneUpdatedAt = entry.state === 'done' ? entry.updatedAt : '' + agentParts.push( + [ + owner.worktreeId, + paneKey, + entry.state, + entry.interrupted === true ? 'interrupted' : '', + agentStartedAt(entry), + doneUpdatedAt + ].join(':') + ) + } + + agentParts.sort() + return [ + `epoch:${args.agentStatusEpoch ?? 0}`, + `worktrees:${worktreeParts.join(',')}`, + `tabs:${tabParts.join(',')}`, + `agents:${agentParts.join(',')}` + ].join('|') +} + +export function buildRetainedAgentsSyncSnapshot(args: RetainedAgentsSyncSnapshotInputs): { + currentAgents: RetainedAgentSnapshot + existingWorktreeIds: Set +} { + const { existingWorktreeIds, tabIndex } = buildLiveTabIndex(args) + const currentAgents: RetainedAgentSnapshot = new Map() + + for (const [paneKey, entry] of Object.entries(args.agentStatusByPaneKey)) { + const tabId = paneKeyTabId(paneKey) + if (!tabId) { + continue + } + const owner = tabIndex.get(tabId) + if (!owner) { + continue + } + const isFresh = isExplicitAgentStatusFresh(entry, args.now, AGENT_STATUS_STALE_AFTER_MS) + const shouldDecay = + !isFresh && + (entry.state === 'working' || entry.state === 'blocked' || entry.state === 'waiting') + currentAgents.set(paneKey, { + row: { + paneKey, + entry, + tab: owner.tab, + agentType: entry.agentType ?? 'unknown', + state: shouldDecay ? 'idle' : entry.state, + startedAt: agentStartedAt(entry) + }, + worktreeId: owner.worktreeId + }) + } + + return { currentAgents, existingWorktreeIds } +} + +export function useRetainedAgentsSync(): void { const retainAgents = useAppStore((s) => s.retainAgents) const pruneRetainedAgents = useAppStore((s) => s.pruneRetainedAgents) const clearRetentionSuppressedPaneKeys = useAppStore((s) => s.clearRetentionSuppressedPaneKeys) - const prevAgentsRef = useRef>( - new Map() + const retentionSignature = useAppStore((s) => + buildRetainedAgentsSyncSignature({ + repos: s.repos, + worktreesByRepo: s.worktreesByRepo, + tabsByWorktree: s.tabsByWorktree, + agentStatusByPaneKey: s.agentStatusByPaneKey, + agentStatusEpoch: s.agentStatusEpoch + }) ) + const prevAgentsRef = useRef(new Map()) useEffect(() => { - const current = new Map() - const existingWorktreeIds = new Set() - for (const group of liveGroups) { - for (const wt of group.worktrees) { - existingWorktreeIds.add(wt.worktree.id) - for (const agent of wt.agents) { - current.set(agent.paneKey, { row: agent, worktreeId: wt.worktree.id }) - } - } - } + const state = useAppStore.getState() + const { currentAgents, existingWorktreeIds } = buildRetainedAgentsSyncSnapshot({ + repos: state.repos, + worktreesByRepo: state.worktreesByRepo, + tabsByWorktree: state.tabsByWorktree, + agentStatusByPaneKey: state.agentStatusByPaneKey, + agentStatusEpoch: state.agentStatusEpoch, + now: Date.now() + }) // Why: read retention state via getState() instead of subscribing. This - // effect's driving input is liveGroups — retention decisions only need to - // happen when an agent appears/disappears from the live set. Subscribing - // to retainedAgentsByPaneKey would create a feedback loop (this effect - // calls retainAgents which updates that map, re-firing the effect). - // retentionSuppressedPaneKeys is only acted on when the corresponding - // pane disappears from liveGroups, so its changes are naturally picked - // up on the next liveGroups-driven run via this fresh getState() read. - const { retainedAgentsByPaneKey: retainedNow, retentionSuppressedPaneKeys } = - useAppStore.getState() + // effect's driving input is the retention signature — retention decisions + // only need to happen when live identity/state/freshness or worktree + // membership changes. Subscribing to retainedAgentsByPaneKey would create + // a feedback loop because this effect calls retainAgents. + const { retainedAgentsByPaneKey: retainedNow, retentionSuppressedPaneKeys } = state const { toRetain, consumedSuppressedPaneKeys } = collectRetainedAgentsOnDisappear({ previousAgents: prevAgentsRef.current, - currentAgents: current, + currentAgents, retainedAgentsByPaneKey: retainedNow, retentionSuppressedPaneKeys }) @@ -52,12 +192,12 @@ export function useRetainedAgentsSync(liveGroups: DashboardRepoGroup[]): void { // atomic update keeps the inline agents list visually stable. retainAgents(toRetain) - prevAgentsRef.current = current + prevAgentsRef.current = currentAgents pruneRetainedAgents(existingWorktreeIds) if (consumedSuppressedPaneKeys.length > 0) { clearRetentionSuppressedPaneKeys(consumedSuppressedPaneKeys) } - }, [liveGroups, retainAgents, pruneRetainedAgents, clearRetentionSuppressedPaneKeys]) + }, [retentionSignature, retainAgents, pruneRetainedAgents, clearRetentionSuppressedPaneKeys]) } export function collectRetainedAgentsOnDisappear(args: { diff --git a/src/renderer/src/components/dashboard/useRetainedAgentsSync.test.ts b/src/renderer/src/components/dashboard/useRetainedAgentsSync.test.ts new file mode 100644 index 00000000000..beb25518362 --- /dev/null +++ b/src/renderer/src/components/dashboard/useRetainedAgentsSync.test.ts @@ -0,0 +1,198 @@ +import { describe, expect, it } from 'vitest' +import { + AGENT_STATUS_STALE_AFTER_MS, + type AgentStatusEntry, + type AgentStatusState +} from '../../../../shared/agent-status-types' +import type { Repo, TerminalTab, Worktree } from '../../../../shared/types' +import { + buildRetainedAgentsSyncSignature, + buildRetainedAgentsSyncSnapshot +} from './useRetainedAgents' + +function makeRepo(): Repo { + return { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: 1 + } +} + +function makeWorktree(overrides?: Partial): Worktree { + return { + id: 'wt-1', + repoId: 'repo-1', + path: '/repo/wt-1', + head: 'abc123', + branch: 'feature', + isBare: false, + isMainWorktree: false, + displayName: 'feature', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 1, + ...overrides + } +} + +function makeTab(overrides?: Partial): TerminalTab { + return { + id: 'tab-1', + ptyId: null, + worktreeId: 'wt-1', + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ...overrides + } +} + +function makeEntry(args: { + paneKey: string + state: AgentStatusState + updatedAt: number + stateStartedAt?: number + prompt?: string + toolName?: string +}): AgentStatusEntry { + return { + state: args.state, + prompt: args.prompt ?? 'Fix it', + updatedAt: args.updatedAt, + stateStartedAt: args.stateStartedAt ?? args.updatedAt, + paneKey: args.paneKey, + terminalTitle: 'Claude', + stateHistory: [], + agentType: 'claude', + toolName: args.toolName + } +} + +function makeSyncInputs(entries: Record) { + const repo = makeRepo() + const worktree = makeWorktree() + const tab = makeTab() + return { + repos: [repo], + worktreesByRepo: { [repo.id]: [worktree] }, + tabsByWorktree: { [worktree.id]: [tab] }, + agentStatusByPaneKey: entries, + agentStatusEpoch: 1 + } +} + +describe('buildRetainedAgentsSyncSignature', () => { + it('ignores fresh same-state working ping details but changes on state transitions', () => { + const first = buildRetainedAgentsSyncSignature( + makeSyncInputs({ + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'working', + updatedAt: 1_000, + stateStartedAt: 1_000, + prompt: 'one', + toolName: 'Read' + }) + }) + ) + const sameState = buildRetainedAgentsSyncSignature( + makeSyncInputs({ + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'working', + updatedAt: 2_000, + stateStartedAt: 1_000, + prompt: 'two', + toolName: 'Edit' + }) + }) + ) + const done = buildRetainedAgentsSyncSignature( + makeSyncInputs({ + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'done', + updatedAt: 3_000, + stateStartedAt: 3_000, + prompt: 'two' + }) + }) + ) + + expect(sameState).toBe(first) + expect(done).not.toBe(first) + }) + + it('tracks same-state done updates so retention keeps the final snapshot', () => { + const done = buildRetainedAgentsSyncSignature( + makeSyncInputs({ + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'done', + updatedAt: 3_000, + stateStartedAt: 3_000 + }) + }) + ) + const updatedDone = buildRetainedAgentsSyncSignature( + makeSyncInputs({ + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'done', + updatedAt: 4_000, + stateStartedAt: 3_000 + }) + }) + ) + + expect(updatedDone).not.toBe(done) + }) +}) + +describe('buildRetainedAgentsSyncSnapshot', () => { + it('builds live rows for non-archived worktrees and stale-decays active states', () => { + const repo = makeRepo() + const activeWorktree = makeWorktree({ id: 'wt-active' }) + const archivedWorktree = makeWorktree({ id: 'wt-archived', isArchived: true }) + const activeTab = makeTab({ id: 'tab-active', worktreeId: 'wt-active' }) + const archivedTab = makeTab({ id: 'tab-archived', worktreeId: 'wt-archived' }) + + const snapshot = buildRetainedAgentsSyncSnapshot({ + repos: [repo], + worktreesByRepo: { [repo.id]: [activeWorktree, archivedWorktree] }, + tabsByWorktree: { + [activeWorktree.id]: [activeTab], + [archivedWorktree.id]: [archivedTab] + }, + agentStatusByPaneKey: { + 'tab-active:1': makeEntry({ + paneKey: 'tab-active:1', + state: 'working', + updatedAt: 10_000, + stateStartedAt: 10_000 + }), + 'tab-archived:1': makeEntry({ + paneKey: 'tab-archived:1', + state: 'done', + updatedAt: 20_000, + stateStartedAt: 20_000 + }) + }, + now: 10_000 + AGENT_STATUS_STALE_AFTER_MS + 1 + }) + + expect([...snapshot.existingWorktreeIds]).toEqual(['wt-active']) + expect(snapshot.currentAgents.get('tab-active:1')?.row.state).toBe('idle') + expect(snapshot.currentAgents.get('tab-archived:1')).toBeUndefined() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 8997bddfc3f..61592d47da2 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -36,6 +36,7 @@ import { } from '@/lib/pane-manager/mobile-fit-overrides' import { getDriverForPty, onDriverChange } from '@/lib/pane-manager/mobile-driver-state' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' +import { captureTerminalShutdownLayout } from './terminal-shutdown-layout-capture' // Why: registry lives in a leaf module so the store slice can import it // without re-entering the `slice → TerminalPane → store → slice` cycle @@ -43,8 +44,6 @@ import { safeFit } from '@/lib/pane-manager/pane-tree-ops' import { shutdownBufferCaptures } from './shutdown-buffer-captures' import { mergeCapturedLeafState } from './merge-captured-leaf-state' -const MAX_BUFFER_BYTES = 512 * 1024 - type TerminalPaneProps = { tabId: string worktreeId: string @@ -834,41 +833,6 @@ export default function TerminalPane({ if (panes.length === 0) { return } - // No renderer-side pending writes to flush — PTY output writes live - // into xterm regardless of visibility, so the SerializeAddon already - // sees the freshest possible state at this point. - const buffers: Record = {} - for (const pane of panes) { - try { - const leafId = paneLeafId(pane.id) - let scrollback = pane.terminal.options.scrollback ?? 10_000 - let serialized = pane.serializeAddon.serialize({ scrollback }) - // Cap at 512KB — binary search for largest scrollback that fits. - if (serialized.length > MAX_BUFFER_BYTES && scrollback > 1) { - let lo = 1 - let hi = scrollback - let best = '' - while (lo <= hi) { - const mid = Math.floor((lo + hi) / 2) - const attempt = pane.serializeAddon.serialize({ scrollback: mid }) - if (attempt.length <= MAX_BUFFER_BYTES) { - best = attempt - lo = mid + 1 - } else { - hi = mid - 1 - } - } - serialized = best - } - if (serialized.length > 0) { - buffers[leafId] = serialized - } - } catch { - // Serialization failure for one pane should not block others. - } - } - const activePaneId = manager.getActivePane()?.id ?? panes[0]?.id ?? null - const layout = serializeTerminalLayout(container, activePaneId, expandedPaneIdRef.current) // Why: setTabLayout REPLACES — it doesn't merge. captureBuffers can // run during a transient window (post-remount, just-attached, // mid-replay) where xterm hasn't rendered yet so serialize returns 0 @@ -876,42 +840,14 @@ export default function TerminalPane({ // buffer. Merge prior state in for leaves whose live capture came back // empty. Same shape as persistLayoutSnapshot. const existing = useAppStore.getState().terminalLayoutsByTabId[tabId] - const currentLeafIds = new Set(panes.map((p) => paneLeafId(p.id))) - const ptyEntries = panes - .map( - (pane) => - [ - paneLeafId(pane.id), - paneTransportsRef.current.get(pane.id)?.getPtyId() ?? null - ] as const - ) - .filter((entry): entry is readonly [string, string] => entry[1] !== null) - const mergedBuffers = mergeCapturedLeafState({ - prior: existing?.buffersByLeafId, - fresh: buffers, - currentLeafIds + const layout = captureTerminalShutdownLayout({ + manager, + container, + expandedPaneId: expandedPaneIdRef.current, + paneTransports: paneTransportsRef.current, + paneTitlesByPaneId: paneTitlesRef.current, + existingLayout: existing }) - const mergedPtyIds = mergeCapturedLeafState({ - prior: existing?.ptyIdsByLeafId, - fresh: Object.fromEntries(ptyEntries), - currentLeafIds - }) - if (Object.keys(mergedBuffers).length > 0) { - layout.buffersByLeafId = mergedBuffers - } - if (Object.keys(mergedPtyIds).length > 0) { - layout.ptyIdsByLeafId = mergedPtyIds - } - // Merge pane titles so the shutdown snapshot doesn't silently drop them. - // Why: the old early-return on empty buffers skipped this entirely, which - // meant titles were lost on restart when the terminal had no scrollback - // content (e.g. fresh pane, cleared screen). - const titleEntries = panes - .filter((p) => paneTitlesRef.current[p.id]) - .map((p) => [paneLeafId(p.id), paneTitlesRef.current[p.id]] as const) - if (titleEntries.length > 0) { - layout.titlesByLeafId = Object.fromEntries(titleEntries) - } setTabLayout(tabId, layout) } shutdownBufferCaptures.set(tabId, captureBuffers) 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 038c9356c28..bfb6d718076 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -738,12 +738,54 @@ describe('connectPanePty', () => { expect(window.api.pty.ackColdRestore).toHaveBeenCalledWith('tab-pty') }) - // Regression for the dim-mismatch bug — guarantees that we never - // reintroduce visibility-gated buffering. Bytes go straight to xterm, - // which lets the WebGL/DOM-fallback renderer parse them at live cols. - // Hidden panes still get writes; the visibility prop only controls - // WebGL suspend/resume in use-terminal-pane-global-effects. - it('writes PTY bytes straight to xterm regardless of visibility', async () => { + // Regression for foreground input lag with many background terminals: + // hidden panes still feed xterm, but their writes are scheduled through + // the shared output drain so 100 panes cannot all start xterm WriteBuffer + // setTimeout handlers in the same event-loop burst. + it('queues non-visible PTY bytes before writing them into xterm', 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 + + try { + 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) + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + isVisibleRef: { current: false } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(6) + + expect(capturedDataCallback.current).not.toBeNull() + capturedDataCallback.current?.('hello\r\n') + expect(pane.terminal.write).not.toHaveBeenCalledWith('hello\r\n') + + for (const fn of pendingTimeouts) { + fn() + } + + expect(pane.terminal.write).toHaveBeenCalledWith('hello\r\n') + } finally { + globalThis.setTimeout = originalSetTimeout + } + }) + + 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 } @@ -756,16 +798,17 @@ describe('connectPanePty', () => { const pane = createPane(1) const manager = createManager(1) const deps = createDeps({ - isVisibleRef: { current: false } + isActiveRef: { current: false }, + isVisibleRef: { current: true } }) connectPanePty(pane as never, manager as never, deps as never) await flushAsyncTicks(6) expect(capturedDataCallback.current).not.toBeNull() - capturedDataCallback.current?.('hello\r\n') + capturedDataCallback.current?.('visible split output\r\n') - expect(pane.terminal.write).toHaveBeenCalledWith('hello\r\n') + expect(pane.terminal.write).toHaveBeenCalledWith('visible split output\r\n') }) it('marks panes that receive Arabic output for DOM rendering', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 4df722708c0..c09ea19c51d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -21,6 +21,12 @@ import { } from './layout-serialization' import { warnTerminalLifecycleAnomaly } from './terminal-lifecycle-diagnostics' import { registerPtySerializer, registerPtyTitleSource } from './pty-buffer-serializer' +import { + discardTerminalOutput, + flushTerminalOutput, + waitForTerminalOutputParsed, + writeTerminalOutput +} from '@/lib/pane-manager/pane-terminal-output-scheduler' const pendingSpawnByPaneKey = new Map>() @@ -487,6 +493,7 @@ export function connectPanePty( } const unregisterSerializer = registerPtySerializer(ptyId, async (opts) => { try { + await waitForTerminalOutputParsed(pane.terminal) // Why: alt-screen TUIs (vim, claude-code) hold transient state in // the alternate screen. The hydration path requests // altScreenForcesZeroRows so normal-buffer scrollback isn't bled @@ -582,6 +589,9 @@ export function connectPanePty( // regardless of DOM visibility and the guard stays engaged via the // write-completion callback until xterm finishes parsing. const writeReplayData = (data: string): void => { + // Why: drain any queued background bytes BEFORE the replay paint, so the + // scheduler's deferred drain cannot land older bytes on top of the replay. + flushTerminalOutput(pane.terminal) if (terminalOutputRequiresDomRenderer(data)) { manager.markPaneHasComplexScriptOutput(pane.id) } @@ -597,16 +607,15 @@ export function connectPanePty( } const dataCallback = (data: string): void => { - // Always-live writes: PTY output goes straight into xterm regardless - // of visibility. xterm's internal write queue handles batching, and - // suspending WebGL while hidden (use-terminal-pane-global-effects) - // keeps GPU resources from leaking. Visibility-gated buffering used - // to feed bytes into xterm at stale dimensions on resume, which was - // the root of the cursor-on-strange-line and broken-wide-char bugs. if (terminalOutputRequiresDomRenderer(data)) { manager.markPaneHasComplexScriptOutput(pane.id) } - pane.terminal.write(data) + // Why: visibility is the right gate — split-pane layouts have multiple + // visible-but-inactive panes whose output the user is watching. Only + // hidden panes (background tabs) should be throttled. + writeTerminalOutput(pane.terminal, data, { + foreground: deps.isVisibleRef.current + }) if (pendingStartupCommand) { if (startupInjectTimer !== null) { @@ -1148,6 +1157,7 @@ export function connectPanePty( clearTimeout(startupInjectTimer) startupInjectTimer = null } + discardTerminalOutput(pane.terminal) if (connectFrame !== null) { // Why: StrictMode and split-group remounts can dispose a pane binding // before its deferred PTY attach/spawn work runs. Cancel that queued diff --git a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts new file mode 100644 index 00000000000..0eb2861e97c --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts @@ -0,0 +1,91 @@ +import { beforeAll, describe, expect, it, vi } from 'vitest' +import type { TerminalLayoutSnapshot } from '../../../../shared/types' + +const mocks = vi.hoisted(() => ({ + flushTerminalOutput: vi.fn() +})) + +vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({ + flushTerminalOutput: mocks.flushTerminalOutput +})) + +class MockHTMLElement { + classList: { contains: (cls: string) => boolean } + dataset: Record + children: MockHTMLElement[] + style: Record + firstElementChild: MockHTMLElement | null + + constructor(opts: { + classList?: string[] + dataset?: Record + children?: MockHTMLElement[] + style?: Record + firstElementChild?: MockHTMLElement | null + }) { + const classes = opts.classList ?? [] + this.classList = { contains: (cls: string) => classes.includes(cls) } + this.dataset = opts.dataset ?? {} + this.children = opts.children ?? [] + this.style = opts.style ?? {} + this.firstElementChild = opts.firstElementChild ?? null + } +} + +beforeAll(() => { + ;(globalThis as unknown as Record).HTMLElement = MockHTMLElement +}) + +function mockRootForPane(paneId: number): HTMLDivElement { + const pane = new MockHTMLElement({ classList: ['pane'], dataset: { paneId: String(paneId) } }) + return new MockHTMLElement({ firstElementChild: pane }) as unknown as HTMLDivElement +} + +describe('captureTerminalShutdownLayout', () => { + it('flushes queued terminal output before serializing shutdown scrollback', async () => { + const { captureTerminalShutdownLayout } = await import('./terminal-shutdown-layout-capture') + const order: string[] = [] + const terminal = { + options: { scrollback: 1_000 }, + pendingOutput: '' + } + const pane = { + id: 1, + terminal, + serializeAddon: { + serialize: vi.fn(() => { + order.push('serialize') + return `snapshot:${terminal.pendingOutput}` + }) + } + } + const manager = { + getPanes: vi.fn(() => [pane]), + getActivePane: vi.fn(() => pane) + } + mocks.flushTerminalOutput.mockImplementation((target: typeof terminal) => { + expect(target).toBe(terminal) + order.push('flush') + terminal.pendingOutput = 'queued-before-quit' + }) + + const layout = captureTerminalShutdownLayout({ + manager: manager as never, + container: mockRootForPane(1), + expandedPaneId: null, + paneTransports: new Map([[1, { getPtyId: vi.fn(() => 'pty-1') }]]), + paneTitlesByPaneId: { 1: 'build logs' }, + existingLayout: undefined + }) + + expect(order).toEqual(['flush', 'serialize']) + expect(layout).toMatchObject({ + root: { type: 'leaf', leafId: 'pane:1' }, + activeLeafId: 'pane:1', + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'snapshot:queued-before-quit' }, + ptyIdsByLeafId: { 'pane:1': 'pty-1' }, + titlesByLeafId: { 'pane:1': 'build logs' } + }) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts new file mode 100644 index 00000000000..00f6673d2e0 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts @@ -0,0 +1,102 @@ +import type { TerminalLayoutSnapshot } from '../../../../shared/types' +import type { ManagedPane } from '@/lib/pane-manager/pane-manager' +import type { PtyTransport } from './pty-transport' +import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-scheduler' +import { paneLeafId, serializeTerminalLayout } from './layout-serialization' +import { mergeCapturedLeafState } from './merge-captured-leaf-state' + +const MAX_BUFFER_BYTES = 512 * 1024 + +type ShutdownPane = Pick + +type ShutdownPaneManager = { + getPanes(): ShutdownPane[] + getActivePane(): ShutdownPane | null +} + +type CaptureTerminalShutdownLayoutArgs = { + manager: ShutdownPaneManager + container: HTMLDivElement + expandedPaneId: number | null + paneTransports: ReadonlyMap> + paneTitlesByPaneId: Record + existingLayout: TerminalLayoutSnapshot | undefined +} + +export function captureTerminalShutdownLayout({ + manager, + container, + expandedPaneId, + paneTransports, + paneTitlesByPaneId, + existingLayout +}: CaptureTerminalShutdownLayoutArgs): TerminalLayoutSnapshot { + const panes = manager.getPanes() + const buffers: Record = {} + + for (const pane of panes) { + try { + // Why: non-focused panes may have renderer-throttled PTY bytes queued; + // push them into xterm before taking the shutdown scrollback snapshot. + flushTerminalOutput(pane.terminal) + const leafId = paneLeafId(pane.id) + let scrollback = pane.terminal.options.scrollback ?? 10_000 + let serialized = pane.serializeAddon.serialize({ scrollback }) + // Cap at 512KB — binary search for largest scrollback that fits. + if (serialized.length > MAX_BUFFER_BYTES && scrollback > 1) { + let lo = 1 + let hi = scrollback + let best = '' + while (lo <= hi) { + const mid = Math.floor((lo + hi) / 2) + const attempt = pane.serializeAddon.serialize({ scrollback: mid }) + if (attempt.length <= MAX_BUFFER_BYTES) { + best = attempt + lo = mid + 1 + } else { + hi = mid - 1 + } + } + serialized = best + } + if (serialized.length > 0) { + buffers[leafId] = serialized + } + } catch { + // Serialization failure for one pane should not block others. + } + } + + const activePaneId = manager.getActivePane()?.id ?? panes[0]?.id ?? null + const layout = serializeTerminalLayout(container, activePaneId, expandedPaneId) + const currentLeafIds = new Set(panes.map((p) => paneLeafId(p.id))) + const ptyEntries = panes + .map((pane) => [paneLeafId(pane.id), paneTransports.get(pane.id)?.getPtyId() ?? null] as const) + .filter((entry): entry is readonly [string, string] => entry[1] !== null) + + const mergedBuffers = mergeCapturedLeafState({ + prior: existingLayout?.buffersByLeafId, + fresh: buffers, + currentLeafIds + }) + const mergedPtyIds = mergeCapturedLeafState({ + prior: existingLayout?.ptyIdsByLeafId, + fresh: Object.fromEntries(ptyEntries), + currentLeafIds + }) + if (Object.keys(mergedBuffers).length > 0) { + layout.buffersByLeafId = mergedBuffers + } + if (Object.keys(mergedPtyIds).length > 0) { + layout.ptyIdsByLeafId = mergedPtyIds + } + + const titleEntries = panes + .filter((p) => paneTitlesByPaneId[p.id]) + .map((p) => [paneLeafId(p.id), paneTitlesByPaneId[p.id]] as const) + if (titleEntries.length > 0) { + layout.titlesByLeafId = Object.fromEntries(titleEntries) + } + + return layout +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts new file mode 100644 index 00000000000..b28b0d56842 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.test.ts @@ -0,0 +1,96 @@ +import type * as ReactModule from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + fitAndFocusPanes: vi.fn(), + fitPanes: vi.fn(), + flushTerminalOutput: vi.fn() +})) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useEffect: (effect: () => void | (() => void)) => { + effect() + }, + useRef: (value: T) => ({ current: value }) + } +}) + +vi.mock('./pane-helpers', () => ({ + fitAndFocusPanes: mocks.fitAndFocusPanes, + fitPanes: mocks.fitPanes +})) + +vi.mock('@/lib/pane-manager/pane-terminal-output-scheduler', () => ({ + flushTerminalOutput: mocks.flushTerminalOutput +})) + +class MockResizeObserver { + observe = vi.fn() + disconnect = vi.fn() +} + +describe('useTerminalPaneGlobalEffects', () => { + beforeEach(() => { + vi.clearAllMocks() + ;(globalThis as unknown as { window: unknown }).window = { + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + api: { + ui: { + onFileDrop: vi.fn(() => vi.fn()) + } + } + } + ;(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = MockResizeObserver + }) + + afterEach(() => { + delete (globalThis as unknown as { window?: unknown }).window + delete (globalThis as unknown as { ResizeObserver?: unknown }).ResizeObserver + }) + + it('flushes visible terminal panes before resuming rendering and fitting', async () => { + const { useTerminalPaneGlobalEffects } = await import('./use-terminal-pane-global-effects') + const order: string[] = [] + const terminalA = { name: 'terminal-a' } + const terminalB = { name: 'terminal-b' } + const manager = { + getPanes: vi.fn(() => [ + { id: 1, terminal: terminalA }, + { id: 2, terminal: terminalB } + ]), + resumeRendering: vi.fn(() => order.push('resume')), + suspendRendering: vi.fn(), + fitAllPanes: vi.fn(), + getActivePane: vi.fn(() => null), + setActivePane: vi.fn() + } + mocks.flushTerminalOutput.mockImplementation((terminal: { name: string }) => { + order.push(`flush:${terminal.name}`) + }) + mocks.fitAndFocusPanes.mockImplementation(() => order.push('fit-focus')) + + const isActiveRef = { current: false } + const isVisibleRef = { current: false } + useTerminalPaneGlobalEffects({ + tabId: 'tab-1', + worktreeId: 'wt-1', + isActive: true, + isVisible: true, + managerRef: { current: manager as never }, + containerRef: { current: null }, + paneTransportsRef: { current: new Map() }, + isActiveRef, + isVisibleRef, + toggleExpandPane: vi.fn() + }) + + expect(order).toEqual(['flush:terminal-a', 'flush:terminal-b', 'resume', 'fit-focus']) + expect(mocks.fitPanes).not.toHaveBeenCalled() + expect(isActiveRef.current).toBe(true) + expect(isVisibleRef.current).toBe(true) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index 1ebdfbd0936..00bf1813a9c 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -9,6 +9,7 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { fitAndFocusPanes, fitPanes } from './pane-helpers' import type { PtyTransport } from './pty-transport' import { handleTerminalFileDrop } from './terminal-drop-handler' +import { flushTerminalOutput } from '@/lib/pane-manager/pane-terminal-output-scheduler' type UseTerminalPaneGlobalEffectsArgs = { tabId: string @@ -53,16 +54,19 @@ export function useTerminalPaneGlobalEffects({ return } if (isVisible) { + // Why: background PTY output is throttled while a pane is not focused; + // flush it before fitting so newly visible terminals paint current state. + for (const pane of manager.getPanes()) { + flushTerminalOutput(pane.terminal) + } // Resume WebGL immediately so the terminal shows its last-known state // on the first painted frame. macOS context creation is ~5 ms; on // Windows (ANGLE → D3D11) it can be 100–500 ms but a deferred resume // would paint a stretched DOM-fallback flash, which is worse UX. manager.resumeRendering() - // Single fit on resume. xterm has been writing live the whole time - // (no visibility-gated buffering), so cols/rows are already correct - // for the new container; this fit is just to absorb any container - // dimension change that happened while we were hidden (e.g. sidebar - // toggle on another worktree). + // Single fit on resume. Background bytes have been pushed into xterm + // above, so this fit only absorbs container dimension changes that + // happened while hidden (e.g. sidebar toggle on another worktree). if (isActive) { fitAndFocusPanes(manager) } else { 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 new file mode 100644 index 00000000000..11ecafa89b0 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -0,0 +1,129 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +function createTerminal() { + return { + write: vi.fn((_data: string, callback?: () => void) => { + callback?.() + }) + } +} + +async function loadScheduler() { + vi.resetModules() + return import('./pane-terminal-output-scheduler') +} + +describe('pane terminal output scheduler', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('writes foreground output immediately', async () => { + const { writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + + writeTerminalOutput(terminal, 'foreground', { foreground: true }) + + expect(terminal.write).toHaveBeenCalledWith('foreground') + }) + + it('coalesces background output until the shared drain runs', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + + writeTerminalOutput(terminal, 'a', { foreground: false }) + writeTerminalOutput(terminal, 'b', { foreground: false }) + + expect(terminal.write).not.toHaveBeenCalled() + vi.advanceTimersByTime(50) + + expect(terminal.write).toHaveBeenCalledTimes(1) + expect(terminal.write).toHaveBeenCalledWith('ab') + }) + + it('limits how many background terminals begin xterm writes per drain tick', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminals = [createTerminal(), createTerminal(), createTerminal()] + + terminals.forEach((terminal, index) => { + writeTerminalOutput(terminal, `pane-${index}`, { foreground: false }) + }) + + vi.advanceTimersByTime(50) + expect(terminals[0].write).toHaveBeenCalledWith('pane-0') + expect(terminals[1].write).toHaveBeenCalledWith('pane-1') + expect(terminals[2].write).not.toHaveBeenCalled() + + vi.advanceTimersByTime(16) + expect(terminals[2].write).toHaveBeenCalledWith('pane-2') + }) + + it('rotates terminals with remaining backlog behind untouched queued terminals', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminals = [createTerminal(), createTerminal(), createTerminal()] + const largeChunk = 'x'.repeat(20 * 1024) + + writeTerminalOutput(terminals[0], largeChunk, { foreground: false }) + writeTerminalOutput(terminals[1], 'pane-1', { foreground: false }) + writeTerminalOutput(terminals[2], 'pane-2', { foreground: false }) + + vi.advanceTimersByTime(50) + expect(terminals[0].write).toHaveBeenCalledTimes(1) + expect(terminals[1].write).toHaveBeenCalledWith('pane-1') + expect(terminals[2].write).not.toHaveBeenCalled() + + // Why: a terminal with leftover bytes is deleted/re-set after each drain + // chunk, moving it to the back of the Map so a big burst cannot starve + // other queued panes. + vi.advanceTimersByTime(16) + expect(terminals[2].write).toHaveBeenCalledWith('pane-2') + expect(terminals[0].write).toHaveBeenCalledTimes(2) + }) + + it('flushes queued output before foreground output on the same terminal', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + + writeTerminalOutput(terminal, 'old', { foreground: false }) + writeTerminalOutput(terminal, 'new', { foreground: true }) + + expect(terminal.write.mock.calls.map(([data]) => data)).toEqual(['old', 'new']) + }) + + it('discards queued output for disposed terminals', async () => { + vi.useFakeTimers() + const { discardTerminalOutput, writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + + writeTerminalOutput(terminal, 'stale', { foreground: false }) + discardTerminalOutput(terminal) + vi.advanceTimersByTime(50) + + expect(terminal.write).not.toHaveBeenCalled() + }) + + it('survives a write to a disposed terminal during background drain', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + const throwing = { + write: vi.fn(() => { + throw new Error('terminal disposed') + }) + } + + writeTerminalOutput(throwing, 'late-ping', { foreground: false }) + + // Why: drain runs inside setTimeout; if the throw escapes drainQueuedOutput + // it would crash the timer callback and leave the scheduler poisoned. + expect(() => vi.advanceTimersByTime(50)).not.toThrow() + expect(throwing.write).toHaveBeenCalledTimes(1) + + // Advancing further must not rediscover the dead entry. + vi.advanceTimersByTime(100) + expect(throwing.write).toHaveBeenCalledTimes(1) + }) +}) 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 new file mode 100644 index 00000000000..8e1f02a581c --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -0,0 +1,244 @@ +import { e2eConfig } from '@/lib/e2e-config' + +type TerminalOutputTarget = { + write(data: string, callback?: () => void): void +} + +type QueueEntry = { + terminal: TerminalOutputTarget + chunks: string[] +} + +const BACKGROUND_FLUSH_DELAY_MS = 50 +const BACKGROUND_DRAIN_INTERVAL_MS = 16 +const BACKGROUND_CHUNK_CHARS = 16 * 1024 +const MAX_WRITES_PER_DRAIN = 2 +const PARSE_SETTLE_TIMEOUT_MS = 250 + +const queuedByTerminal = new Map() +let drainTimer: ReturnType | null = null +const debugEnabled = e2eConfig.exposeStore + +// Why no lossy queue cap: dropping raw terminal bytes can corrupt parser state +// (half an escape sequence, missed mode reset, wrong scrollback). A pathological +// background producer can still consume memory/CPU; preserving terminal +// correctness means that case needs adaptive/backpressure work, not truncation. + +type TerminalOutputSchedulerDebugSnapshot = { + backgroundEnqueueCount: number + foregroundWriteCount: number + backgroundWriteCount: number + flushWriteCount: number + scheduledDrainCount: number + drainWrites: number[] +} + +type TerminalOutputSchedulerDebugApi = { + reset: () => void + snapshot: () => TerminalOutputSchedulerDebugSnapshot +} + +const debugState: TerminalOutputSchedulerDebugSnapshot = { + backgroundEnqueueCount: 0, + foregroundWriteCount: 0, + backgroundWriteCount: 0, + flushWriteCount: 0, + scheduledDrainCount: 0, + drainWrites: [] +} + +function resetDebugState(): void { + debugState.backgroundEnqueueCount = 0 + debugState.foregroundWriteCount = 0 + debugState.backgroundWriteCount = 0 + debugState.flushWriteCount = 0 + debugState.scheduledDrainCount = 0 + debugState.drainWrites = [] +} + +function exposeDebugApi(): void { + if (!debugEnabled || typeof window === 'undefined') { + return + } + // Why: the e2e repro needs to prove background output used the shared drain, + // but production must not accumulate diagnostic counters indefinitely. + const target = window as unknown as { + __terminalOutputSchedulerDebug?: TerminalOutputSchedulerDebugApi + } + target.__terminalOutputSchedulerDebug ??= { + reset: resetDebugState, + snapshot: () => ({ + ...debugState, + drainWrites: [...debugState.drainWrites] + }) + } +} + +function scheduleDrain(delayMs: number): void { + if (drainTimer !== null) { + return + } + if (debugEnabled) { + debugState.scheduledDrainCount++ + } + drainTimer = setTimeout(drainQueuedOutput, delayMs) +} + +function takeQueuedChunk(entry: QueueEntry, limit: number): string { + let remaining = limit + let data = '' + + while (remaining > 0 && entry.chunks.length > 0) { + const chunk = entry.chunks[0] + if (chunk.length <= remaining) { + data += chunk + remaining -= chunk.length + entry.chunks.shift() + continue + } + + data += chunk.slice(0, remaining) + entry.chunks[0] = chunk.slice(remaining) + remaining = 0 + } + + return data +} + +function writeQueuedChunk(entry: QueueEntry): boolean { + const data = takeQueuedChunk(entry, BACKGROUND_CHUNK_CHARS) + if (!data) { + return false + } + try { + entry.terminal.write(data) + } catch { + // Why: pane.terminal.dispose() can race with a queued late-arriving PTY ping; + // a write to a disposed terminal throws. Drop the entry rather than crashing + // the scheduler for other panes still draining. + entry.chunks.length = 0 + return false + } + return true +} + +function drainQueuedOutput(): void { + drainTimer = null + let writes = 0 + + while (queuedByTerminal.size > 0 && writes < MAX_WRITES_PER_DRAIN) { + const entry = queuedByTerminal.values().next().value + if (!entry) { + break + } + + queuedByTerminal.delete(entry.terminal) + if (writeQueuedChunk(entry)) { + writes++ + if (debugEnabled) { + debugState.backgroundWriteCount++ + } + } + if (entry.chunks.length > 0) { + queuedByTerminal.set(entry.terminal, entry) + } + } + + if (debugEnabled && writes > 0) { + debugState.drainWrites.push(writes) + } + if (queuedByTerminal.size > 0) { + scheduleDrain(BACKGROUND_DRAIN_INTERVAL_MS) + } +} + +export function writeTerminalOutput( + terminal: TerminalOutputTarget, + data: string, + options: { foreground: boolean } +): void { + exposeDebugApi() + if (!data) { + return + } + + if (options.foreground) { + flushTerminalOutput(terminal) + if (debugEnabled) { + debugState.foregroundWriteCount++ + } + terminal.write(data) + return + } + + let entry = queuedByTerminal.get(terminal) + if (!entry) { + entry = { terminal, chunks: [] } + queuedByTerminal.set(terminal, entry) + } + entry.chunks.push(data) + if (debugEnabled) { + debugState.backgroundEnqueueCount++ + } + // Why: non-focused panes can produce output continuously. Letting every + // pane call xterm.write immediately schedules one xterm WriteBuffer timer + // per pane, which starves the focused terminal on the shared renderer thread. + scheduleDrain(BACKGROUND_FLUSH_DELAY_MS) +} + +export function flushTerminalOutput(terminal: TerminalOutputTarget): void { + exposeDebugApi() + const entry = queuedByTerminal.get(terminal) + if (!entry) { + return + } + queuedByTerminal.delete(terminal) + + let data = takeQueuedChunk(entry, BACKGROUND_CHUNK_CHARS) + while (data) { + if (debugEnabled) { + debugState.flushWriteCount++ + } + try { + terminal.write(data) + } catch { + // Why: pane.terminal.dispose() can race with a queued late-arriving PTY ping; + // a write to a disposed terminal throws. Drop the entry rather than crashing + // the scheduler for other panes still draining. + return + } + data = takeQueuedChunk(entry, BACKGROUND_CHUNK_CHARS) + } +} + +export function waitForTerminalOutputParsed(terminal: TerminalOutputTarget): Promise { + flushTerminalOutput(terminal) + + return new Promise((resolve) => { + let settled = false + let timer: ReturnType | null = null + const finish = (): void => { + if (settled) { + return + } + settled = true + if (timer !== null) { + clearTimeout(timer) + } + resolve() + } + timer = setTimeout(finish, PARSE_SETTLE_TIMEOUT_MS) + try { + terminal.write('', finish) + } catch { + finish() + } + }) +} + +export function discardTerminalOutput(terminal: TerminalOutputTarget): void { + exposeDebugApi() + queuedByTerminal.delete(terminal) +} + +exposeDebugApi() diff --git a/src/renderer/src/store/slices/agent-status.test.ts b/src/renderer/src/store/slices/agent-status.test.ts index 629d36b3e54..1b1eaa07c25 100644 --- a/src/renderer/src/store/slices/agent-status.test.ts +++ b/src/renderer/src/store/slices/agent-status.test.ts @@ -147,6 +147,50 @@ describe('agent status tool + assistant fields', () => { .setAgentStatus('tab-1:1', { state: 'working', prompt: 'p2', agentType: 'cursor' }) expect(store.getState().agentStatusByPaneKey['tab-1:1'].agentType).toBe('cursor') }) + + it('keeps global epochs stable for fresh same-state pings while updating the entry', () => { + vi.useFakeTimers() + const store = createTestStore() + store + .getState() + .setAgentStatus( + 'tab-1:1', + { state: 'working', prompt: 'p1', agentType: 'claude', toolName: 'Read' }, + 'claude', + { updatedAt: 1_000, stateStartedAt: 1_000 } + ) + const firstEpoch = store.getState().agentStatusEpoch + const firstSortEpoch = store.getState().sortEpoch + + store + .getState() + .setAgentStatus( + 'tab-1:1', + { state: 'working', prompt: 'p2', agentType: 'claude', toolName: 'Edit' }, + 'claude', + { updatedAt: 2_000, stateStartedAt: 1_000 } + ) + + const sameStateEntry = store.getState().agentStatusByPaneKey['tab-1:1'] + expect(sameStateEntry.prompt).toBe('p2') + expect(sameStateEntry.toolName).toBe('Edit') + expect(sameStateEntry.updatedAt).toBe(2_000) + // Why: same-state hook pings are high-frequency and already update the + // owning row through agentStatusByPaneKey. The global epochs are reserved + // for state/freshness changes that can affect aggregate dashboard/sidebar + // calculations. + expect(store.getState().agentStatusEpoch).toBe(firstEpoch) + expect(store.getState().sortEpoch).toBe(firstSortEpoch) + + store + .getState() + .setAgentStatus('tab-1:1', { state: 'done', prompt: 'p2', agentType: 'claude' }, 'claude', { + updatedAt: 3_000, + stateStartedAt: 3_000 + }) + expect(store.getState().agentStatusEpoch).toBe(firstEpoch + 1) + expect(store.getState().sortEpoch).toBe(firstSortEpoch + 1) + }) }) describe('agent status stateStartedAt', () => { diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index 1ab99cc8425..e363c0ebd91 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -234,12 +234,11 @@ export const createAgentStatusSlice: StateCreatorfresh. Same-state + // tool/prompt pings still update agentStatusByPaneKey for the owning + // row, but they must not fan out through dashboard/sidebar aggregate + // work across every card. Sort-relevant inputs are: // 1. `state` transitions — sort score is a function of state. // 2. Freshness transitions (stale → fresh) — `computeSmartScoreFromSignals` // in smart-sort.ts filters entries through @@ -269,7 +268,7 @@ export const createAgentStatusSlice: StateCreator void + snapshot: () => SchedulerDebugSnapshot + } +} + +const SORTABLE_TAB = '[data-testid="sortable-tab"]' +const TAB_COUNT = 5 + +function tabLocator(page: Page, tabId: string) { + return page.locator(`${SORTABLE_TAB}[data-tab-id="${tabId}"]`).first() +} + +async function countRenderedTabs(page: Page): Promise { + return page.locator(SORTABLE_TAB).count() +} + +async function getDomActiveTabId(page: Page): Promise { + return page.evaluate((selector) => { + const match = document.querySelector(`${selector}[data-active="true"]`) + return match?.getAttribute('data-tab-id') ?? null + }, SORTABLE_TAB) +} + +function nodeConsoleCommand(expression: string): string { + return `node -e "console.log(${expression})"` +} + +async function createTerminalTab(page: Page): Promise { + const tabsBefore = await countRenderedTabs(page) + const activeBefore = await getActiveTabId(page) + + await page.getByRole('button', { name: 'New tab' }).click() + await page + .getByRole('menuitem', { name: /New Terminal/i }) + .first() + .click() + + await expect + .poll(() => countRenderedTabs(page), { + timeout: 5_000, + message: 'New Terminal did not render a new tab in the tab bar' + }) + .toBe(tabsBefore + 1) + + let tabId: string | null = null + await expect + .poll( + async () => { + tabId = await getActiveTabId(page) + return Boolean(tabId && tabId !== activeBefore) + }, + { + timeout: 5_000, + message: 'New Terminal did not become the active tab' + } + ) + .toBe(true) + + if (!tabId) { + throw new Error('createTerminalTab: active tab id was unavailable after creating terminal') + } + return tabId +} + +async function waitForTabPtyId(page: Page, tabId: string): Promise { + let ptyId: string | null = null + await expect + .poll( + async () => { + ptyId = await page.evaluate((targetTabId) => { + const manager = window.__paneManagers?.get(targetTabId) + const pane = manager?.getPanes?.()[0] ?? null + return pane?.container?.dataset?.ptyId ?? null + }, tabId) + return ptyId + }, + { + timeout: 15_000, + message: `Terminal tab ${tabId} did not receive a PTY binding` + } + ) + .not.toBeNull() + + if (!ptyId) { + throw new Error(`waitForTabPtyId: tab ${tabId} has no PTY id`) + } + return ptyId +} + +async function resetSchedulerDebug(page: Page): Promise { + await page.evaluate(() => { + const debug = (window as SchedulerDebugWindow).__terminalOutputSchedulerDebug + if (!debug) { + throw new Error('terminal output scheduler debug API is unavailable') + } + debug.reset() + }) +} + +async function getSchedulerDebug(page: Page): Promise { + return page.evaluate(() => { + const debug = (window as SchedulerDebugWindow).__terminalOutputSchedulerDebug + if (!debug) { + throw new Error('terminal output scheduler debug API is unavailable') + } + return debug.snapshot() + }) +} + +async function sendPtyCommands( + page: Page, + commands: { ptyId: string; command: string }[] +): Promise { + await page.evaluate((items) => { + for (const item of items) { + window.api.pty.write(item.ptyId, `${item.command}\r`) + } + }, commands) +} + +test.describe('Terminal output scheduler', () => { + test('background tab output bursts use the shared drain while the active tab renders', async ({ + orcaPage + }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + + const firstTabId = await getActiveTabId(orcaPage) + if (!firstTabId) { + throw new Error('Expected an initial terminal tab') + } + + const tabIds = [firstTabId] + const ptyIdsByTabId: Record = { + [firstTabId]: await waitForTabPtyId(orcaPage, firstTabId) + } + + while (tabIds.length < TAB_COUNT) { + const tabId = await createTerminalTab(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + tabIds.push(tabId) + ptyIdsByTabId[tabId] = await waitForTabPtyId(orcaPage, tabId) + } + + await tabLocator(orcaPage, firstTabId).click() + await expect + .poll(() => getDomActiveTabId(orcaPage), { + timeout: 5_000, + message: 'First terminal tab did not become active before the burst repro' + }) + .toBe(firstTabId) + + await resetSchedulerDebug(orcaPage) + + const runId = Date.now() + const foregroundMarker = `FG_SCHED_${runId}` + // Why: the marker is appended AFTER the burst payload so it survives + // getTerminalContent's tail-only truncation (charLimit defaults to 4000). + // A leading marker would be evicted by the 50000-char x-burst. + const backgroundCommands = tabIds.slice(1).map((tabId, index) => ({ + ptyId: ptyIdsByTabId[tabId], + marker: `BG_SCHED_${runId}_${index}`, + command: nodeConsoleCommand(`'x'.repeat(50000) + ':BG_SCHED_${runId}_${index}'`) + })) + + await sendPtyCommands( + orcaPage, + backgroundCommands.map(({ ptyId, command }) => ({ ptyId, command })) + ) + await sendPtyCommands(orcaPage, [ + { + ptyId: ptyIdsByTabId[firstTabId], + command: nodeConsoleCommand(`'${foregroundMarker}'`) + } + ]) + + await expect + .poll(async () => (await getTerminalContent(orcaPage)).includes(foregroundMarker), { + timeout: 5_000, + message: 'Active terminal did not render foreground output during background bursts' + }) + .toBe(true) + + await expect + .poll(async () => (await getSchedulerDebug(orcaPage)).backgroundEnqueueCount, { + timeout: 5_000, + message: 'Background PTY output did not enter the scheduler queue' + }) + .toBeGreaterThanOrEqual(backgroundCommands.length) + + await expect + .poll(async () => (await getSchedulerDebug(orcaPage)).backgroundWriteCount, { + timeout: 10_000, + message: 'Queued background PTY output did not drain into xterm' + }) + .toBeGreaterThanOrEqual(backgroundCommands.length) + + const debug = await getSchedulerDebug(orcaPage) + expect(debug.foregroundWriteCount).toBeGreaterThan(0) + if (debug.drainWrites.length > 0) { + expect(Math.max(...debug.drainWrites)).toBeLessThanOrEqual(2) + } + + const firstBackground = backgroundCommands[0] + const firstBackgroundTabId = tabIds[1] + await tabLocator(orcaPage, firstBackgroundTabId).click() + await expect + .poll(() => getDomActiveTabId(orcaPage), { + timeout: 5_000, + message: 'Background terminal tab did not become active for content verification' + }) + .toBe(firstBackgroundTabId) + await expect + .poll(async () => (await getTerminalContent(orcaPage)).includes(firstBackground.marker), { + timeout: 5_000, + message: 'Background terminal output was not preserved after scheduler drain' + }) + .toBe(true) + }) +})