diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 85454407dc3..a83575a49b8 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -523,6 +523,37 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { } }) + it('clears a pending checkpoint timer when the last dirty session closes', async () => { + const adapterClass = DaemonPtyAdapter as unknown as { CHECKPOINT_INTERVAL_MS: number } + const previousInterval = adapterClass.CHECKPOINT_INTERVAL_MS + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout') + adapterClass.CHECKPOINT_INTERVAL_MS = 10_000 + + try { + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const { id } = await historyAdapter.spawn({ + cols: 80, + rows: 24, + cwd: '/home/user', + sessionId: 'close-dirty-checkpoint' + }) + const internals = historyAdapter as unknown as { + dirtySessionVersions: Map + } + + lastSubprocess._simulateData('dirty before close\r\n') + await waitFor(() => internals.dirtySessionVersions.has(id)) + const callsBeforeClose = clearTimeoutSpy.mock.calls.length + + await historyAdapter.shutdown(id, { immediate: true }) + + expect(clearTimeoutSpy.mock.calls.length).toBeGreaterThan(callsBeforeClose) + } finally { + adapterClass.CHECKPOINT_INTERVAL_MS = previousInterval + clearTimeoutSpy.mockRestore() + } + }) + it('writes meta.json with endedAt on exit', async () => { historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index ca492a9538f..f5c6cedb617 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -66,7 +66,7 @@ export class DaemonPtyAdapter implements IPtyProvider { private coldRestoreCache = new Map() private activeSessionIds = new Set() private dirtySessionVersions = new Map() - private checkpointInterval: ReturnType | null = null + private checkpointTimer: ReturnType | null = null private checkpointInFlight: Promise | null = null // Why: checkpoint-based persistence requires the getSnapshot RPC (v4+). // Legacy daemons reject it, causing noisy log spam every 5 seconds. @@ -246,6 +246,7 @@ export class DaemonPtyAdapter implements IPtyProvider { await this.client.request('kill', { sessionId: id }) this.activeSessionIds.delete(id) this.dirtySessionVersions.delete(id) + this.stopCheckpointTimerIfIdle() this.initialCwds.delete(id) // Why: history removal is for the "user explicitly closed this terminal" // path. Sleep also calls shutdown but expects scrollback to survive — wake @@ -407,6 +408,7 @@ export class DaemonPtyAdapter implements IPtyProvider { const ids = [...this.activeSessionIds] this.activeSessionIds.clear() this.dirtySessionVersions.clear() + this.stopCheckpointTimer() for (const id of ids) { // Why: listener throws are intentionally *not* caught — matches the // natural onExit fanout in setupEventRouting, so synthetic exits don't @@ -462,10 +464,7 @@ export class DaemonPtyAdapter implements IPtyProvider { } dispose(): void { - if (this.checkpointInterval) { - clearInterval(this.checkpointInterval) - this.checkpointInterval = null - } + this.stopCheckpointTimer() this.dirtySessionVersions.clear() this.removeEventListener?.() this.removeEventListener = null @@ -487,10 +486,7 @@ export class DaemonPtyAdapter implements IPtyProvider { // We write a final checkpoint before disconnecting so that if the daemon // later crashes while Orca is closed, checkpoint.json has recovery data. async disconnectOnly(): Promise { - if (this.checkpointInterval) { - clearInterval(this.checkpointInterval) - this.checkpointInterval = null - } + this.stopCheckpointTimer() // Why: wait for any in-flight timer pass to finish before starting // the final checkpoint. Otherwise both passes race on the shared tmp // file, risking ENOENT on rename and disabling future writes. @@ -512,23 +508,47 @@ export class DaemonPtyAdapter implements IPtyProvider { private async ensureConnected(): Promise { await this.client.ensureConnected() this.setupEventRouting() - this.startCheckpointTimer() + this.scheduleCheckpointTimer() } - private startCheckpointTimer(): void { - if (this.checkpointInterval || !this.historyManager || !this.supportsCheckpoints) { + private stopCheckpointTimer(): void { + if (!this.checkpointTimer) { return } - this.checkpointInterval = setInterval(() => { + clearTimeout(this.checkpointTimer) + this.checkpointTimer = null + } + + private stopCheckpointTimerIfIdle(): void { + if (this.dirtySessionVersions.size === 0) { + this.stopCheckpointTimer() + } + } + + private scheduleCheckpointTimer(): void { + if ( + this.checkpointTimer || + !this.historyManager || + !this.supportsCheckpoints || + this.dirtySessionVersions.size === 0 + ) { + return + } + // Why: checkpointing is only needed after terminal data/resize/write marks + // a session dirty. A permanent interval woke the main process every 5s for + // idle daemon-backed terminals just to discover there was nothing to write. + this.checkpointTimer = setTimeout(() => { + this.checkpointTimer = null // Why: if the previous pass is still in-flight (slow RPC or disk), - // skip this tick. Overlapping passes race on the shared tmp file - // in checkpoint(), and a lost rename triggers handleWriteError which - // permanently disables the session's history writes. + // retry later instead of overlapping checkpoint() writes to the same tmp + // file, which can lose a rename and disable future history writes. if (this.checkpointInFlight) { + this.scheduleCheckpointTimer() return } this.checkpointInFlight = this.checkpointDirtySessions().finally(() => { this.checkpointInFlight = null + this.scheduleCheckpointTimer() }) }, DaemonPtyAdapter.CHECKPOINT_INTERVAL_MS) } @@ -538,6 +558,7 @@ export class DaemonPtyAdapter implements IPtyProvider { return } this.dirtySessionVersions.set(sessionId, (this.dirtySessionVersions.get(sessionId) ?? 0) + 1) + this.scheduleCheckpointTimer() } private async checkpointDirtySessions(): Promise { @@ -552,6 +573,8 @@ export class DaemonPtyAdapter implements IPtyProvider { [...this.dirtySessionVersions].filter(([sessionId]) => this.activeSessionIds.has(sessionId)) ) if (versions.size === 0) { + this.dirtySessionVersions.clear() + this.stopCheckpointTimer() return } const completed = await this.checkpointSessions(versions.keys()) @@ -560,6 +583,7 @@ export class DaemonPtyAdapter implements IPtyProvider { this.dirtySessionVersions.delete(sessionId) } } + this.stopCheckpointTimerIfIdle() } // Why: the adapter runs in the Electron main process and does not have direct @@ -650,6 +674,7 @@ export class DaemonPtyAdapter implements IPtyProvider { } else if (event.event === 'exit') { this.activeSessionIds.delete(event.sessionId) this.dirtySessionVersions.delete(event.sessionId) + this.stopCheckpointTimerIfIdle() if (this.historyManager) { void this.historyManager .closeSession(event.sessionId, event.payload.code) diff --git a/src/main/daemon/daemon-server.test.ts b/src/main/daemon/daemon-server.test.ts index 0290cd9436e..df547bad81a 100644 --- a/src/main/daemon/daemon-server.test.ts +++ b/src/main/daemon/daemon-server.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- Why: daemon server RPC, auth, stream batching, and shutdown behavior share one socket/client harness; splitting would duplicate setup. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { connect, type Socket } from 'net' import { tmpdir } from 'os' @@ -15,6 +16,7 @@ function createTestDir(): string { function createMockSubprocess(): SubprocessHandle & { _simulateData: (data: string) => void + _simulateExit: (code: number) => void } { let onDataCb: ((data: string) => void) | null = null let onExitCb: ((code: number) => void) | null = null @@ -34,6 +36,9 @@ function createMockSubprocess(): SubprocessHandle & { dispose: vi.fn(), _simulateData(data: string) { onDataCb?.(data) + }, + _simulateExit(code: number) { + onExitCb?.(code) } } } @@ -280,6 +285,53 @@ describe('DaemonServer', () => { vi.useRealTimers() } }) + + it('flushes pending batched stream output before the exit event', async () => { + vi.useFakeTimers() + try { + let subprocess: ReturnType + server = new DaemonServer({ + socketPath, + tokenPath, + spawnSubprocess: () => { + subprocess = createMockSubprocess() + return subprocess + } + }) + const daemon = server as unknown as DaemonServerPrivate + const controlSocket = { destroy: vi.fn() } as unknown as Socket + const streamSocket = { + destroyed: false, + destroy: vi.fn(), + write: vi.fn() + } as unknown as Socket & { write: ReturnType } + + daemon.clients.set('client-1', { + clientId: 'client-1', + controlSocket, + streamSocket + }) + + await daemon.routeRequest('client-1', { + id: 'req-1', + type: 'createOrAttach', + payload: { sessionId: 'test-session', cols: 80, rows: 24 } + }) + + subprocess!._simulateData('final-output') + subprocess!._simulateExit(42) + + expect(streamSocket.write).toHaveBeenCalledTimes(2) + expect(String(streamSocket.write.mock.calls[0]?.[0])).toContain('"event":"data"') + expect(String(streamSocket.write.mock.calls[0]?.[0])).toContain('"data":"final-output"') + expect(String(streamSocket.write.mock.calls[1]?.[0])).toContain('"event":"exit"') + expect(String(streamSocket.write.mock.calls[1]?.[0])).toContain('"code":42') + vi.advanceTimersByTime(8) + expect(streamSocket.write).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) }) describe('authentication', () => { diff --git a/src/main/daemon/daemon-stream-data-batcher.test.ts b/src/main/daemon/daemon-stream-data-batcher.test.ts index 408143d94e0..08b4624eb78 100644 --- a/src/main/daemon/daemon-stream-data-batcher.test.ts +++ b/src/main/daemon/daemon-stream-data-batcher.test.ts @@ -71,4 +71,33 @@ describe('DaemonStreamDataBatcher', () => { vi.useRealTimers() } }) + + it('flushes interactive output for one session while another session has large pending output', () => { + vi.useFakeTimers() + try { + const { batcher, streamSocket } = createBatcher() + const background = 'x'.repeat(2048) + + batcher.enqueue('client-1', 'session-background', background) + batcher.enqueue('client-1', 'session-interactive', 'echo', { + flushImmediately: true, + flushMaxChars: 1024 + }) + + expect(streamSocket.write).toHaveBeenCalledTimes(1) + expect(String(streamSocket.write.mock.calls[0]?.[0])).toContain( + '"sessionId":"session-interactive"' + ) + expect(String(streamSocket.write.mock.calls[0]?.[0])).toContain('"data":"echo"') + + vi.advanceTimersByTime(8) + expect(streamSocket.write).toHaveBeenCalledTimes(2) + expect(String(streamSocket.write.mock.calls[1]?.[0])).toContain( + '"sessionId":"session-background"' + ) + expect(String(streamSocket.write.mock.calls[1]?.[0])).toContain(`"data":"${background}"`) + } finally { + vi.useRealTimers() + } + }) }) diff --git a/src/main/daemon/daemon-stream-data-batcher.ts b/src/main/daemon/daemon-stream-data-batcher.ts index f5631e827a9..b9b1f6d92c9 100644 --- a/src/main/daemon/daemon-stream-data-batcher.ts +++ b/src/main/daemon/daemon-stream-data-batcher.ts @@ -50,9 +50,10 @@ export class DaemonStreamDataBatcher { if ( options.flushImmediately === true && - batch.queuedChars <= (options.flushMaxChars ?? Number.POSITIVE_INFINITY) + this.queuedCharsForSession(batch, sessionId) <= + (options.flushMaxChars ?? Number.POSITIVE_INFINITY) ) { - this.flush(clientId) + this.flushSession(clientId, sessionId) return } if (!batch.timer) { @@ -89,6 +90,64 @@ export class DaemonStreamDataBatcher { } } + private queuedCharsForSession(batch: PendingStreamDataBatch, sessionId: string): number { + let chars = 0 + for (const entry of batch.queue) { + if (entry.sessionId === sessionId) { + chars += entry.data.length + } + } + return chars + } + + private flushSession(clientId: string, sessionId: string): void { + const batch = this.pendingByClient.get(clientId) + if (!batch) { + return + } + + const flushed: PendingStreamDataBatch['queue'] = [] + const retained: PendingStreamDataBatch['queue'] = [] + let flushedChars = 0 + for (const entry of batch.queue) { + if (entry.sessionId === sessionId) { + flushed.push(entry) + flushedChars += entry.data.length + } else { + retained.push(entry) + } + } + if (flushed.length === 0) { + return + } + + batch.queue = retained + batch.queuedChars -= flushedChars + if (batch.queue.length === 0) { + if (batch.timer) { + clearTimeout(batch.timer) + batch.timer = null + } + this.pendingByClient.delete(clientId) + } + + const client = this.getClient(clientId) + if (!client?.streamSocket || client.streamSocket.destroyed) { + return + } + + for (const entry of flushed) { + client.streamSocket.write( + encodeNdjson({ + type: 'event', + event: 'data', + sessionId: entry.sessionId, + payload: { data: entry.data } + }) + ) + } + } + clear(clientId?: string): void { const batches = clientId === undefined diff --git a/src/main/index.ts b/src/main/index.ts index 5b752a82c4e..4f7564a7b0a 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -87,6 +87,11 @@ import { shouldRecoverRendererAfterProcessGone, type ExpectedTeardownScope } from './crash-reporting/process-gone-classification' +import { + advanceSyntheticTitleSpinnerEntries, + type SyntheticTitleSpinnerEntry +} from './synthetic-title-spinner' +import { shouldSendSyntheticTitleFrame } from './synthetic-title-visibility' import { isCrashReportReason } from '../shared/crash-reporting' let mainWindow: BrowserWindow | null = null @@ -415,14 +420,11 @@ function openMainWindow(): BrowserWindow { // window recreations instead of stacking on top of stale listeners. agentHookServer.setListener(null) setMigrationUnsupportedPtyListener(null) - // Why: any running synthesized-title spinner intervals would fire into a - // destroyed webContents; stop them all here instead of deferring to - // per-pane teardown, which may never run for restored-but-never-torn-down - // panes when the window goes away. stopSyntheticTitleSpinner deletes only - // the current entry, which the Map iterator handles safely. - for (const paneKey of syntheticTitleSpinnerByPaneKey.keys()) { - stopSyntheticTitleSpinner(paneKey) - } + // Why: any running synthesized-title spinner timer would fire into a + // destroyed webContents; stop it here instead of deferring to per-pane + // teardown, which may never run for restored-but-never-torn-down panes + // when the window goes away. + stopAllSyntheticTitleSpinners() }) mainWindow = window agentHookServer.setListener( @@ -627,8 +629,9 @@ const SYNTHETIC_TITLE_PROFILES: Record = { const syntheticTitleSpinnerByPaneKey = new Map< string, - { timer: ReturnType; frame: number; profile: SyntheticTitleProfile } + SyntheticTitleSpinnerEntry >() +let syntheticTitleSpinnerTimer: ReturnType | null = null type ServeOptions = { json: boolean @@ -741,29 +744,95 @@ function installServeSignalHandlers(): void { process.once('SIGTERM', quit) } -// Why: on PTY teardown the paneKey→ptyId mapping is dropped, so the spinner -// interval would keep firing but sendSyntheticTitle would no-op forever. -// Stop the interval explicitly so the process doesn't carry a timer per dead -// pane. +// Why: on PTY teardown the paneKey mapping is dropped, so the spinner tick +// would keep firing but sendSyntheticTitle would no-op forever. Drop the +// entry explicitly so the shared timer shuts down once no panes are active. registerPaneKeyTeardownListener((paneKey) => { stopSyntheticTitleSpinner(paneKey) }) -function sendSyntheticTitle(ptyId: string, data: string): void { +function sendSyntheticTitle(ptyId: string, data: string, options: { force?: boolean } = {}): void { if (!mainWindow || mainWindow.isDestroyed()) { return } + // Why: repeated working-spinner frames are decorative and can arrive every + // 80ms per agent. Final/permission frames are forced because they drive BEL. + if ( + !shouldSendSyntheticTitleFrame({ + force: options.force === true, + windowVisible: mainWindow.isVisible(), + windowFocused: mainWindow.isFocused() + }) + ) { + return + } mainWindow.webContents.send('pty:data', { id: ptyId, data }) } +function canSendDecorativeSyntheticTitle(): boolean { + return ( + mainWindow !== null && + !mainWindow.isDestroyed() && + shouldSendSyntheticTitleFrame({ + force: false, + windowVisible: mainWindow.isVisible(), + windowFocused: mainWindow.isFocused() + }) + ) +} + function stopSyntheticTitleSpinner(paneKey: string): void { - const entry = syntheticTitleSpinnerByPaneKey.get(paneKey) - if (entry) { - clearInterval(entry.timer) - syntheticTitleSpinnerByPaneKey.delete(paneKey) + if (syntheticTitleSpinnerByPaneKey.delete(paneKey)) { + stopSyntheticTitleSpinnerTimerIfIdle() } } +function stopAllSyntheticTitleSpinners(): void { + syntheticTitleSpinnerByPaneKey.clear() + stopSyntheticTitleSpinnerTimer() +} + +function stopSyntheticTitleSpinnerTimer(): void { + if (!syntheticTitleSpinnerTimer) { + return + } + clearInterval(syntheticTitleSpinnerTimer) + syntheticTitleSpinnerTimer = null +} + +function stopSyntheticTitleSpinnerTimerIfIdle(): void { + if (syntheticTitleSpinnerByPaneKey.size === 0) { + stopSyntheticTitleSpinnerTimer() + } +} + +function tickSyntheticTitleSpinners(): void { + if (!canSendDecorativeSyntheticTitle()) { + return + } + const ticks = advanceSyntheticTitleSpinnerEntries({ + entries: syntheticTitleSpinnerByPaneKey, + frameCount: SPINNER_FRAMES.length, + getPtyIdForPaneKey + }) + for (const tick of ticks) { + sendSyntheticTitle( + tick.ptyId, + `\x1b]0;${SPINNER_FRAMES[tick.frame]} ${tick.profile.workingLabel}\x07` + ) + } + stopSyntheticTitleSpinnerTimerIfIdle() +} + +function ensureSyntheticTitleSpinnerTimer(): void { + if (syntheticTitleSpinnerTimer) { + return + } + // Why: a single process timer covers all synthesized title spinners; per-pane + // intervals multiplied idle wakeups when several retained agents were working. + syntheticTitleSpinnerTimer = setInterval(tickSyntheticTitleSpinners, SPINNER_INTERVAL_MS) +} + function driveSyntheticTitleFromHook( paneKey: string, state: string, @@ -787,23 +856,8 @@ function driveSyntheticTitleFromHook( existing.profile = profile return } - const timer = setInterval(() => { - const ptyIdNow = getPtyIdForPaneKey(paneKey) - if (!ptyIdNow) { - stopSyntheticTitleSpinner(paneKey) - return - } - const cur = syntheticTitleSpinnerByPaneKey.get(paneKey) - if (!cur) { - return - } - cur.frame = (cur.frame + 1) % SPINNER_FRAMES.length - sendSyntheticTitle( - ptyIdNow, - `\x1b]0;${SPINNER_FRAMES[cur.frame]} ${cur.profile.workingLabel}\x07` - ) - }, SPINNER_INTERVAL_MS) - syntheticTitleSpinnerByPaneKey.set(paneKey, { timer, frame, profile }) + syntheticTitleSpinnerByPaneKey.set(paneKey, { frame, profile }) + ensureSyntheticTitleSpinnerTimer() return } // Why: leaving the spinner running after a `blocked`/`waiting`/`done` event @@ -819,7 +873,7 @@ function driveSyntheticTitleFromHook( stopSyntheticTitleSpinner(paneKey) const label = state === 'blocked' || state === 'waiting' ? profile.permissionLabel : profile.idleLabel - sendSyntheticTitle(ptyId, `\x1b]0;${label}\x07\x07`) + sendSyntheticTitle(ptyId, `\x1b]0;${label}\x07\x07`, { force: true }) } app.whenReady().then(async () => { diff --git a/src/main/synthetic-title-spinner.test.ts b/src/main/synthetic-title-spinner.test.ts new file mode 100644 index 00000000000..f79afa1b2cb --- /dev/null +++ b/src/main/synthetic-title-spinner.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { + advanceSyntheticTitleSpinnerEntries, + type SyntheticTitleSpinnerEntry +} from './synthetic-title-spinner' + +describe('advanceSyntheticTitleSpinnerEntries', () => { + it('advances live panes in one shared tick and wraps frames', () => { + const entries = new Map>([ + ['pane-a', { frame: 1, profile: { label: 'A' } }], + ['pane-b', { frame: 2, profile: { label: 'B' } }] + ]) + + const ticks = advanceSyntheticTitleSpinnerEntries({ + entries, + frameCount: 3, + getPtyIdForPaneKey: (paneKey) => `pty-${paneKey}` + }) + + expect(ticks).toEqual([ + { paneKey: 'pane-a', ptyId: 'pty-pane-a', frame: 2, profile: { label: 'A' } }, + { paneKey: 'pane-b', ptyId: 'pty-pane-b', frame: 0, profile: { label: 'B' } } + ]) + expect(entries.get('pane-a')?.frame).toBe(2) + expect(entries.get('pane-b')?.frame).toBe(0) + }) + + it('drops panes whose pty mapping disappeared', () => { + const entries = new Map>([ + ['live-pane', { frame: 0, profile: { label: 'live' } }], + ['stale-pane', { frame: 0, profile: { label: 'stale' } }] + ]) + + const ticks = advanceSyntheticTitleSpinnerEntries({ + entries, + frameCount: 4, + getPtyIdForPaneKey: (paneKey) => (paneKey === 'stale-pane' ? null : `pty-${paneKey}`) + }) + + expect(ticks).toEqual([ + { paneKey: 'live-pane', ptyId: 'pty-live-pane', frame: 1, profile: { label: 'live' } } + ]) + expect(entries.has('live-pane')).toBe(true) + expect(entries.has('stale-pane')).toBe(false) + }) +}) diff --git a/src/main/synthetic-title-spinner.ts b/src/main/synthetic-title-spinner.ts new file mode 100644 index 00000000000..df8379a29d3 --- /dev/null +++ b/src/main/synthetic-title-spinner.ts @@ -0,0 +1,33 @@ +export type SyntheticTitleSpinnerEntry = { + frame: number + profile: TProfile +} + +export type SyntheticTitleSpinnerTick = { + paneKey: string + ptyId: string + frame: number + profile: TProfile +} + +export function advanceSyntheticTitleSpinnerEntries(args: { + entries: Map> + frameCount: number + getPtyIdForPaneKey: (paneKey: string) => string | null | undefined +}): SyntheticTitleSpinnerTick[] { + if (args.frameCount <= 0) { + return [] + } + + const ticks: SyntheticTitleSpinnerTick[] = [] + for (const [paneKey, entry] of args.entries) { + const ptyId = args.getPtyIdForPaneKey(paneKey) + if (!ptyId) { + args.entries.delete(paneKey) + continue + } + entry.frame = (entry.frame + 1) % args.frameCount + ticks.push({ paneKey, ptyId, frame: entry.frame, profile: entry.profile }) + } + return ticks +} diff --git a/src/main/synthetic-title-visibility.test.ts b/src/main/synthetic-title-visibility.test.ts new file mode 100644 index 00000000000..b176bf9aab4 --- /dev/null +++ b/src/main/synthetic-title-visibility.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest' +import { shouldSendSyntheticTitleFrame } from './synthetic-title-visibility' + +describe('shouldSendSyntheticTitleFrame', () => { + it('skips decorative spinner frames only while the window is hidden', () => { + expect( + shouldSendSyntheticTitleFrame({ force: false, windowVisible: false, windowFocused: true }) + ).toBe(false) + expect( + shouldSendSyntheticTitleFrame({ force: false, windowVisible: true, windowFocused: false }) + ).toBe(true) + expect( + shouldSendSyntheticTitleFrame({ force: false, windowVisible: true, windowFocused: true }) + ).toBe(true) + }) + + it('always sends forced terminal-state frames', () => { + expect( + shouldSendSyntheticTitleFrame({ force: true, windowVisible: false, windowFocused: false }) + ).toBe(true) + }) +}) diff --git a/src/main/synthetic-title-visibility.ts b/src/main/synthetic-title-visibility.ts new file mode 100644 index 00000000000..6546d5248f9 --- /dev/null +++ b/src/main/synthetic-title-visibility.ts @@ -0,0 +1,8 @@ +export function shouldSendSyntheticTitleFrame(args: { + force: boolean + windowVisible: boolean + windowFocused: boolean +}): boolean { + void args.windowFocused + return args.force || args.windowVisible +} diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 4680731b9b2..ae3bac914ff 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -33,6 +33,7 @@ import { } from '@/components/ui/context-menu' import { useAppStore } from './store' import { useShallow } from 'zustand/react/shallow' +import { useActiveTerminalTabs } from './store/selectors' import { isRemoteWorkspaceSnapshotApplyInProgress, useIpcEvents } from './hooks/useIpcEvents' import { useAutomationDispatchEvents } from './hooks/useAutomationDispatchEvents' import RetainedAgentsSyncGate from './components/dashboard/RetainedAgentsSyncGate' @@ -76,6 +77,7 @@ import { usePrimarySelectionPaste } from './hooks/usePrimarySelectionPaste' import { + canSkipRuntimeMobileSessionSyncKeyBuild, getRuntimeMobileSessionSyncKey, runtimeMobileSessionSyncKeysEqual, scheduleRuntimeGraphSync, @@ -290,7 +292,7 @@ function App(): React.JSX.Element { // that remount so the left workspace list doesn't restart at scrollTop 0. const worktreeSidebarScrollOffsetRef = useRef(0) const worktreeSidebarScrollAnchorRef = useRef(null) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) + const tabs = useActiveTerminalTabs() const floatingUnifiedTabCount = useAppStore( (s) => s.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID]?.length ?? 0 ) @@ -755,24 +757,7 @@ function App(): React.JSX.Element { // by reference. Mirrors every field used by // getRuntimeMobileSessionSyncKey so this gate covers every "could the // key have changed?" case. - if ( - state.tabsByWorktree === previousState.tabsByWorktree && - state.groupsByWorktree === previousState.groupsByWorktree && - state.activeGroupIdByWorktree === previousState.activeGroupIdByWorktree && - state.layoutByWorktree === previousState.layoutByWorktree && - state.unifiedTabsByWorktree === previousState.unifiedTabsByWorktree && - state.tabBarOrderByWorktree === previousState.tabBarOrderByWorktree && - state.activeFileId === previousState.activeFileId && - state.activeFileIdByWorktree === previousState.activeFileIdByWorktree && - state.browserTabsByWorktree === previousState.browserTabsByWorktree && - state.browserPagesByWorkspace === previousState.browserPagesByWorkspace && - state.activeBrowserTabIdByWorktree === previousState.activeBrowserTabIdByWorktree && - state.openFiles === previousState.openFiles && - state.editorDrafts === previousState.editorDrafts && - state.activeTabId === previousState.activeTabId && - state.terminalLayoutsByTabId === previousState.terminalLayoutsByTabId && - state.runtimePaneTitlesByTabId === previousState.runtimePaneTitlesByTabId - ) { + if (canSkipRuntimeMobileSessionSyncKeyBuild(state, previousState)) { return } const nextKey = getRuntimeMobileSessionSyncKey(state, previousState, previousKey) @@ -957,7 +942,6 @@ function App(): React.JSX.Element { return () => document.removeEventListener('visibilitychange', handler) }, [actions]) - const tabs = activeWorktreeId ? (tabsByWorktree[activeWorktreeId] ?? []) : [] const hasTabBar = tabs.length >= 2 const effectiveActiveTabId = activeTabId ?? tabs[0]?.id ?? null const activeTabCanExpand = effectiveActiveTabId diff --git a/src/renderer/src/components/CodexRestartChip.tsx b/src/renderer/src/components/CodexRestartChip.tsx index 42c6c6a23cf..7c38922c51a 100644 --- a/src/renderer/src/components/CodexRestartChip.tsx +++ b/src/renderer/src/components/CodexRestartChip.tsx @@ -1,7 +1,23 @@ import { RefreshCw } from 'lucide-react' -import { useMemo } from 'react' +import { useShallow } from 'zustand/react/shallow' import { useAppStore } from '../store' +const EMPTY_TABS: { id: string }[] = [] + +export function collectStalePtyIdsForTabs({ + tabs, + ptyIdsByTabId, + codexRestartNoticeByPtyId +}: { + tabs: { id: string }[] + ptyIdsByTabId: Record + codexRestartNoticeByPtyId: Record +}): string[] { + return tabs.flatMap((tab) => + (ptyIdsByTabId[tab.id] ?? []).filter((ptyId) => Boolean(codexRestartNoticeByPtyId[ptyId])) + ) +} + export function collectStaleWorktreePtyIds({ tabsByWorktree, ptyIdsByTabId, @@ -13,9 +29,11 @@ export function collectStaleWorktreePtyIds({ codexRestartNoticeByPtyId: Record worktreeId: string }): string[] { - return (tabsByWorktree[worktreeId] ?? []).flatMap((tab) => - (ptyIdsByTabId[tab.id] ?? []).filter((ptyId) => Boolean(codexRestartNoticeByPtyId[ptyId])) - ) + return collectStalePtyIdsForTabs({ + tabs: tabsByWorktree[worktreeId] ?? EMPTY_TABS, + ptyIdsByTabId, + codexRestartNoticeByPtyId + }) } export function dismissStaleWorktreePtyIds( @@ -35,23 +53,18 @@ export default function CodexRestartChip({ }: { worktreeId: string }): React.JSX.Element | null { - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) - const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId) - const codexRestartNoticeByPtyId = useAppStore((s) => s.codexRestartNoticeByPtyId) + const staleWorktreePtyIds = useAppStore( + useShallow((s) => + collectStalePtyIdsForTabs({ + tabs: s.tabsByWorktree[worktreeId] ?? EMPTY_TABS, + ptyIdsByTabId: s.ptyIdsByTabId, + codexRestartNoticeByPtyId: s.codexRestartNoticeByPtyId + }) + ) + ) const queueCodexPaneRestarts = useAppStore((s) => s.queueCodexPaneRestarts) const clearCodexRestartNotice = useAppStore((s) => s.clearCodexRestartNotice) - const staleWorktreePtyIds = useMemo( - () => - collectStaleWorktreePtyIds({ - tabsByWorktree, - ptyIdsByTabId, - codexRestartNoticeByPtyId, - worktreeId - }), - [codexRestartNoticeByPtyId, ptyIdsByTabId, tabsByWorktree, worktreeId] - ) - if (staleWorktreePtyIds.length === 0) { return null } diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index dfabd4ca92f..51a90bbff48 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -1,6 +1,6 @@ /* eslint-disable max-lines */ -import React, { useEffect, useCallback, useMemo, useRef, useState, lazy, Suspense } from 'react' +import React, { useEffect, useCallback, useRef, useState, lazy, Suspense } from 'react' import { createPortal } from 'react-dom' import { toast } from 'sonner' import { @@ -9,7 +9,6 @@ import { type BackgroundMountTerminalWorktreeDetail } from '@/constants/terminal' import { useAppStore } from '../store' -import { useAllWorktrees } from '../store/selectors' import { findWorktreeById } from '../store/slices/worktree-helpers' import { createUntitledMarkdownFile } from '../lib/create-untitled-markdown' import { getConnectionId } from '../lib/connection-context' @@ -52,6 +51,10 @@ import { import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout' import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal' import { shouldRepairActiveTerminalTab } from './terminal/active-terminal-repair' +import { getActiveWorktreeOpenFiles } from './terminal/active-worktree-open-files' +import { getTerminalBrowserTabSlices } from './terminal/terminal-browser-tab-slices' +import { getTerminalMountedWorktreeSnapshot } from './terminal/terminal-mounted-worktrees' +import { getTerminalTabSlices } from './terminal/terminal-tab-slices' import { addBackgroundMountedTerminalWorktree } from './terminal/background-terminal-worktree-mount' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { @@ -88,10 +91,8 @@ const EditorPanel = lazy(() => import('./editor/EditorPanel')) const CLOSE_DIALOG_DEBOUNCE_MS = 200 function Terminal(): React.JSX.Element | null { - const allWorktrees = useAllWorktrees() const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const activeView = useAppStore((s) => s.activeView) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) const activeTabId = useAppStore((s) => s.activeTabId) const createTab = useAppStore((s) => s.createTab) const closeTab = useAppStore((s) => s.closeTab) @@ -105,7 +106,37 @@ function Terminal(): React.JSX.Element | null { const consumeSuppressedPtyExit = useAppStore((s) => s.consumeSuppressedPtyExit) const expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId) const workspaceSessionReady = useAppStore((s) => s.workspaceSessionReady) - const openFiles = useAppStore((s) => s.openFiles) + // Track which worktrees have been activated during this app session. + // Only mount TerminalPanes for visited worktrees to prevent mass PTY + // spawning when restoring a session with many saved worktree tabs. + const mountedWorktreeIdsRef = useRef(new Set()) + const measurableBackgroundWorktreeIdsRef = useRef(new Set()) + const measurableBackgroundWorktreeTimersRef = useRef(new Map()) + const [, setBackgroundMountRevision] = useState(0) + // Why: gated on workspaceSessionReady to prevent TerminalPane from mounting + // before reconnectPersistedTerminals() has finished eagerly spawning PTYs. + // Without this gate, Phase 1 (hydrateWorkspaceSession) sets activeWorktreeId + // with ptyId: null, and TerminalPane would call connectPanePty → pty:spawn, + // creating a duplicate PTY for the same tab. + if (activeWorktreeId && workspaceSessionReady) { + mountedWorktreeIdsRef.current.add(activeWorktreeId) + } + const terminalWorktreeSnapshot = useAppStore((s) => + getTerminalMountedWorktreeSnapshot(s.worktreesByRepo, mountedWorktreeIdsRef.current) + ) + const terminalTabSlices = useAppStore((s) => + getTerminalTabSlices(s.tabsByWorktree, mountedWorktreeIdsRef.current, activeWorktreeId) + ) + const terminalBrowserTabSlices = useAppStore((s) => + getTerminalBrowserTabSlices( + s.browserTabsByWorktree, + mountedWorktreeIdsRef.current, + activeWorktreeId + ) + ) + const worktreeFiles = useAppStore((s) => + getActiveWorktreeOpenFiles(s.openFiles, activeWorktreeId) + ) const activeFileId = useAppStore((s) => s.activeFileId) const activeBrowserTabId = useAppStore((s) => s.activeBrowserTabId) const activeTabType = useAppStore((s) => s.activeTabType) @@ -114,7 +145,6 @@ function Terminal(): React.JSX.Element | null { const openFile = useAppStore((s) => s.openFile) const closeFile = useAppStore((s) => s.closeFile) const pinFile = useAppStore((s) => s.pinFile) - const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) const createBrowserTab = useAppStore((s) => s.createBrowserTab) const closeBrowserTab = useAppStore((s) => s.closeBrowserTab) const setActiveBrowserTab = useAppStore((s) => s.setActiveBrowserTab) @@ -138,10 +168,7 @@ function Terminal(): React.JSX.Element | null { activeView === 'activity' ) - const tabs = useMemo( - () => (activeWorktreeId ? (tabsByWorktree[activeWorktreeId] ?? []) : []), - [activeWorktreeId, tabsByWorktree] - ) + const tabs = terminalTabSlices.activeTabs // Why: the TabBar is rendered into the titlebar via a portal so tabs share // the same row as the "Orca" title. The target element is created by App.tsx. @@ -161,13 +188,7 @@ function Terminal(): React.JSX.Element | null { ensureWorktreeRootGroup(activeWorktreeId) }, [activeWorktreeId, ensureWorktreeRootGroup]) - // Filter editor files to only show those belonging to the active worktree - const worktreeFiles = activeWorktreeId - ? openFiles.filter((f) => f.worktreeId === activeWorktreeId) - : [] - const worktreeBrowserTabs = activeWorktreeId - ? (browserTabsByWorktree[activeWorktreeId] ?? []) - : [] + const worktreeBrowserTabs = terminalBrowserTabSlices.activeBrowserTabs const getEffectiveLayoutForWorktree = useCallback( (worktreeId: string) => getEffectiveLayout(worktreeId, layoutByWorktree, groupsByWorktree, activeGroupIdByWorktree), @@ -176,13 +197,13 @@ function Terminal(): React.JSX.Element | null { const effectiveActiveLayout = activeWorktreeId ? getEffectiveLayoutForWorktree(activeWorktreeId) : undefined - const activeWorktreeBrowserTabIdsKey = activeWorktreeId - ? (browserTabsByWorktree[activeWorktreeId] ?? []).map((tab) => tab.id).join(',') - : '' + const activeWorktreeBrowserTabIdsKey = worktreeBrowserTabs.map((tab) => tab.id).join(',') // Save confirmation dialog state const [saveDialogFileId, setSaveDialogFileId] = useState(null) - const saveDialogFile = saveDialogFileId ? openFiles.find((f) => f.id === saveDialogFileId) : null + const saveDialogFile = useAppStore((s) => + saveDialogFileId ? (s.openFiles.find((file) => file.id === saveDialogFileId) ?? null) : null + ) const pendingEditorCloseQueueRef = useRef([]) // Why: while a save-and-close is awaiting the file to disappear from @@ -515,13 +536,6 @@ function Terminal(): React.JSX.Element | null { // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeTabId, activeTabType, setActiveTab, tabs]) - // Track which worktrees have been activated during this app session. - // Only mount TerminalPanes for visited worktrees to prevent mass PTY - // spawning when restoring a session with many saved worktree tabs. - const mountedWorktreeIdsRef = useRef(new Set()) - const measurableBackgroundWorktreeIdsRef = useRef(new Set()) - const measurableBackgroundWorktreeTimersRef = useRef(new Map()) - const [, setBackgroundMountRevision] = useState(0) useEffect(() => { const timers = measurableBackgroundWorktreeTimersRef.current const onBackgroundMountTerminalWorktree = (event: Event): void => { @@ -564,23 +578,15 @@ function Terminal(): React.JSX.Element | null { timers.clear() } }, []) - // Why: gated on workspaceSessionReady to prevent TerminalPane from mounting - // before reconnectPersistedTerminals() has finished eagerly spawning PTYs. - // Without this gate, Phase 1 (hydrateWorkspaceSession) sets activeWorktreeId - // with ptyId: null, and TerminalPane would call connectPanePty → pty:spawn, - // creating a duplicate PTY for the same tab. - if (activeWorktreeId && workspaceSessionReady) { - mountedWorktreeIdsRef.current.add(activeWorktreeId) - } // Prune IDs of worktrees that no longer exist (deleted/removed) - const allWorktreeIds = new Set(allWorktrees.map((wt) => wt.id)) + const allWorktreeIds = new Set(terminalWorktreeSnapshot.worktreeIds) for (const id of mountedWorktreeIdsRef.current) { if (!allWorktreeIds.has(id)) { mountedWorktreeIdsRef.current.delete(id) } } const anyMountedWorktreeHasLayout = computeAnyMountedWorktreeHasLayout( - allWorktrees.map((wt) => wt.id), + terminalWorktreeSnapshot.worktreeIds, mountedWorktreeIdsRef.current, layoutByWorktree, groupsByWorktree, @@ -1399,31 +1405,29 @@ function Terminal(): React.JSX.Element | null { can preserve hidden trees without reflowing the active one. Keep a relative anchor here so those panes size to the workspace body rather than some outer ancestor when split groups are enabled. */} - {allWorktrees - .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id)) - .map((worktree) => { - const layout = getEffectiveLayoutForWorktree(worktree.id) - if (!layout) { - return null - } - // Why: use strict equality with 'terminal' instead of !== 'settings' - // so the terminal/browser surface hides on the tasks page too. - const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId - const shouldMeasureHiddenWorktree = - !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) - return ( - - ) - })} + {terminalWorktreeSnapshot.mountedWorktrees.map((worktree) => { + const layout = getEffectiveLayoutForWorktree(worktree.id) + if (!layout) { + return null + } + // Why: use strict equality with 'terminal' instead of !== 'settings' + // so the terminal/browser surface hides on the tasks page too. + const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId + const shouldMeasureHiddenWorktree = + !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) + return ( + + ) + })} ) : null} @@ -1461,67 +1465,65 @@ function Terminal(): React.JSX.Element | null { : '' }`} > - {allWorktrees - .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id)) - .map((worktree) => { - // Why: use strict equality with 'terminal' instead of !== 'settings' - // so the terminal/browser surface hides on the tasks page too. - const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId - const shouldMeasureHiddenWorktree = - !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) - return ( -
{ + // Why: use strict equality with 'terminal' instead of !== 'settings' + // so the terminal/browser surface hides on the tasks page too. + const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId + const shouldMeasureHiddenWorktree = + !isVisible && measurableBackgroundWorktreeIdsRef.current.has(worktree.id) + return ( +
+ + {(terminalTabSlices.mountedTabsByWorktree[worktree.id] ?? []).map((tab) => { + const activityTerminalPortal = findActivityTerminalPortal( + activityTerminalPortals, + { worktreeId: worktree.id, tabId: tab.id } + ) + const isActivityPortalTab = activityTerminalPortal !== null + const isActiveTerminalTab = + isVisible && tab.id === activeTabId && activeTabType === 'terminal' + const terminalPane = ( + handlePtyExit(tab.id, ptyId)} + onCloseTab={() => handleCloseTab(tab.id)} + /> + ) + if (activityTerminalPortal) { + return createPortal( + terminalPane, + activityTerminalPortal.target, + `activity-terminal-${tab.id}` + ) } - aria-hidden={!isVisible} - > - - {(tabsByWorktree[worktree.id] ?? []).map((tab) => { - const activityTerminalPortal = findActivityTerminalPortal( - activityTerminalPortals, - { worktreeId: worktree.id, tabId: tab.id } - ) - const isActivityPortalTab = activityTerminalPortal !== null - const isActiveTerminalTab = - isVisible && tab.id === activeTabId && activeTabType === 'terminal' - const terminalPane = ( - handlePtyExit(tab.id, ptyId)} - onCloseTab={() => handleCloseTab(tab.id)} - /> - ) - if (activityTerminalPortal) { - return createPortal( - terminalPane, - activityTerminalPortal.target, - `activity-terminal-${tab.id}` - ) - } - return terminalPane - })} -
- ) - })} + return terminalPane + })} +
+ ) + })} {/* Browser panes container — all browser panes for the active worktree @@ -1532,8 +1534,9 @@ function Terminal(): React.JSX.Element | null { activeTabType !== 'browser' ? 'hidden' : '' }`} > - {allWorktrees.map((worktree) => { - const browserTabs = browserTabsByWorktree[worktree.id] ?? [] + {terminalWorktreeSnapshot.mountedWorktrees.map((worktree) => { + const browserTabs = + terminalBrowserTabSlices.mountedBrowserTabsByWorktree[worktree.id] ?? [] // Why: use strict equality with 'terminal' instead of !== 'settings' // so browser panes also hide on the tasks page. const isVisibleWorktree = diff --git a/src/renderer/src/components/WorktreeJumpPalette.test.ts b/src/renderer/src/components/WorktreeJumpPalette.test.ts new file mode 100644 index 00000000000..45fc0e61615 --- /dev/null +++ b/src/renderer/src/components/WorktreeJumpPalette.test.ts @@ -0,0 +1,52 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const storeBox = vi.hoisted(() => ({ + state: { + activeModal: null as string | null + } +})) + +const selectorMocks = vi.hoisted(() => ({ + getRepoMapFromState: vi.fn(() => new Map()), + useAllWorktrees: vi.fn(() => []) +})) + +vi.mock('@/store', () => ({ + useAppStore: Object.assign( + (selector: (state: typeof storeBox.state) => unknown) => selector(storeBox.state), + { + getState: () => storeBox.state + } + ) +})) + +vi.mock('@/store/selectors', () => ({ + getRepoMapFromState: selectorMocks.getRepoMapFromState, + useAllWorktrees: selectorMocks.useAllWorktrees +})) + +vi.mock('@/components/ui/command', () => ({ + CommandDialog: () => null, + CommandEmpty: () => null, + CommandInput: () => null, + CommandItem: () => null, + CommandList: () => null +})) + +vi.mock('@/components/sidebar/StatusIndicator', () => ({ + default: () => null +})) + +describe('WorktreeJumpPalette', () => { + beforeEach(() => { + vi.clearAllMocks() + storeBox.state.activeModal = null + }) + + it('does not mount broad workspace selectors while the lazy palette is closed', async () => { + const { default: WorktreeJumpPalette } = await import('./WorktreeJumpPalette') + + expect(WorktreeJumpPalette()).toBeNull() + expect(selectorMocks.useAllWorktrees).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 57a0236f18a..4906fabf1ad 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -78,6 +78,13 @@ type BrowserSelection = { page: BrowserPage } +type PaletteFocusSnapshot = { + worktreeId: string | null + tabType: 'browser' | 'editor' | 'terminal' + browserPageId: string | null + browserFocusTarget: 'webview' | 'address-bar' +} + function HighlightedText({ text, matchRange @@ -140,8 +147,40 @@ function findBrowserSelection( return { page, workspace, worktree } } +function getPaletteFocusSnapshot(): PaletteFocusSnapshot { + const state = useAppStore.getState() + const browserPageId = + state.activeWorktreeId && state.activeTabType === 'browser' + ? ((state.browserTabsByWorktree[state.activeWorktreeId] ?? []).find( + (workspace) => workspace.id === state.activeBrowserTabId + )?.activePageId ?? null) + : null + + return { + worktreeId: state.activeWorktreeId, + tabType: state.activeTabType, + browserPageId, + // Why: capture during render, before the portaled Dialog moves focus, so + // closing Cmd+J can restore the exact browser target that opened it. + browserFocusTarget: + state.activeTabType === 'browser' && + typeof document !== 'undefined' && + document.activeElement instanceof HTMLElement && + document.activeElement.closest('[data-orca-browser-address-bar="true"]') + ? 'address-bar' + : 'webview' + } +} + export default function WorktreeJumpPalette(): React.JSX.Element | null { const visible = useAppStore((s) => s.activeModal === 'worktree-palette') + // Why: App keeps lazily loaded modals mounted after first use. The palette's + // search indexes subscribe to broad workspace slices, so drop the heavy + // content while closed to avoid rebuilding hidden jump results on hot ticks. + return visible ? : null +} + +function WorktreeJumpPaletteContent(): React.JSX.Element { const closeModal = useAppStore((s) => s.closeModal) const openModal = useAppStore((s) => s.openModal) const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) @@ -161,10 +200,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { const terminalLayoutsByTabId = useAppStore((s) => s.terminalLayoutsByTabId) const prCache = useAppStore((s) => s.prCache) const issueCache = useAppStore((s) => s.issueCache) - const agentStatusByPaneKey = useAppStore((s) => s.agentStatusByPaneKey) + const agentStatusEpoch = useAppStore((s) => s.agentStatusEpoch) const migrationUnsupportedByPtyId = useAppStore((s) => s.migrationUnsupportedByPtyId) const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) - const activeTabType = useAppStore((s) => s.activeTabType) const activeBrowserTabId = useAppStore((s) => s.activeBrowserTabId) const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) const browserPagesByWorkspace = useAppStore((s) => s.browserPagesByWorkspace) @@ -177,11 +215,20 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { const [query, setQuery] = useState('') const deferredQuery = useDeferredValue(query) const [selectedItemId, setSelectedItemId] = useState('') - const previousWorktreeIdRef = useRef(null) - const previousActiveTabTypeRef = useRef<'browser' | 'editor' | 'terminal'>('terminal') - const previousBrowserPageIdRef = useRef(null) - const previousBrowserFocusTargetRef = useRef<'webview' | 'address-bar'>('webview') - const wasVisibleRef = useRef(false) + const initialFocusSnapshotRef = useRef(null) + if (initialFocusSnapshotRef.current === null) { + initialFocusSnapshotRef.current = getPaletteFocusSnapshot() + } + const previousWorktreeIdRef = useRef(initialFocusSnapshotRef.current.worktreeId) + const previousActiveTabTypeRef = useRef<'browser' | 'editor' | 'terminal'>( + initialFocusSnapshotRef.current.tabType + ) + const previousBrowserPageIdRef = useRef( + initialFocusSnapshotRef.current.browserPageId + ) + const previousBrowserFocusTargetRef = useRef<'webview' | 'address-bar'>( + initialFocusSnapshotRef.current.browserFocusTarget + ) const skipRestoreFocusRef = useRef(false) const prevQueryRef = useRef('') const listRef = useRef(null) @@ -246,33 +293,35 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { // Why: typed queries still route through sortWorktreesSmart — switcher // ranking only diverges from smart-sort on the empty-query branch. - const sortedWorktrees = useMemo( - () => - hasQuery - ? sortWorktreesSmart( - visibleWorktrees, - tabsByWorktree, - repoMap, - agentStatusByPaneKey, - runtimePaneTitlesByTabId, - ptyIdsByTabId, - migrationUnsupportedByPtyId, - terminalLayoutsByTabId - ) - : switchableWorktreesForRows, - [ - hasQuery, - visibleWorktrees, - switchableWorktreesForRows, - tabsByWorktree, - repoMap, - agentStatusByPaneKey, - runtimePaneTitlesByTabId, - ptyIdsByTabId, - migrationUnsupportedByPtyId, - terminalLayoutsByTabId - ] - ) + const sortedWorktrees = useMemo(() => { + // Why: same-state prompt/tool pings cannot change smart-sort order; the + // epoch is the cheap invalidation signal for state/freshness changes. + void agentStatusEpoch + const agentStatusByPaneKey = useAppStore.getState().agentStatusByPaneKey + return hasQuery + ? sortWorktreesSmart( + visibleWorktrees, + tabsByWorktree, + repoMap, + agentStatusByPaneKey, + runtimePaneTitlesByTabId, + ptyIdsByTabId, + migrationUnsupportedByPtyId, + terminalLayoutsByTabId + ) + : switchableWorktreesForRows + }, [ + agentStatusEpoch, + hasQuery, + visibleWorktrees, + switchableWorktreesForRows, + tabsByWorktree, + repoMap, + runtimePaneTitlesByTabId, + ptyIdsByTabId, + migrationUnsupportedByPtyId, + terminalLayoutsByTabId + ]) const browserSortedWorktrees = useMemo(() => { // Why: browser-tab search is explicitly cross-worktree, so it must keep @@ -281,6 +330,8 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { // tab on the default-branch worktree before toggling hide-on should still // be able to Cmd+J back to it — the setting hides the *workspace row*, // not the browser tabs that live inside it. + void agentStatusEpoch + const agentStatusByPaneKey = useAppStore.getState().agentStatusByPaneKey return sortWorktreesSmart( allWorktrees, tabsByWorktree, @@ -293,9 +344,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { ) }, [ allWorktrees, + agentStatusEpoch, tabsByWorktree, repoMap, - agentStatusByPaneKey, runtimePaneTitlesByTabId, ptyIdsByTabId, migrationUnsupportedByPtyId, @@ -486,37 +537,6 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { const hasAnyBrowserPages = browserPageEntries.length > 0 useEffect(() => { - if (visible && !wasVisibleRef.current) { - previousWorktreeIdRef.current = activeWorktreeId - previousActiveTabTypeRef.current = activeTabType - previousBrowserPageIdRef.current = - activeWorktreeId && activeTabType === 'browser' - ? ((browserTabsByWorktree[activeWorktreeId] ?? []).find( - (workspace) => workspace.id === activeBrowserTabId - )?.activePageId ?? null) - : null - // Why: capture which browser surface had focus *before* Radix Dialog - // steals it. By onOpenAutoFocus time, document.activeElement has already - // moved to the dialog content, so address-bar detection must happen here. - previousBrowserFocusTargetRef.current = - activeTabType === 'browser' && - document.activeElement instanceof HTMLElement && - document.activeElement.closest('[data-orca-browser-address-bar="true"]') - ? 'address-bar' - : 'webview' - skipRestoreFocusRef.current = false - prevQueryRef.current = '' - setQuery('') - setSelectedItemId('') - } - - wasVisibleRef.current = visible - }, [activeBrowserTabId, activeTabType, activeWorktreeId, browserTabsByWorktree, visible]) - - useEffect(() => { - if (!visible) { - return - } const queryChanged = deferredQuery !== prevQueryRef.current prevQueryRef.current = deferredQuery @@ -547,7 +567,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { ) { setSelectedItemId(firstSelectableId ?? selectableItems[0].id) } - }, [deferredQuery, selectedItemId, showCreateAction, visible, selectableItems]) + }, [deferredQuery, selectedItemId, showCreateAction, selectableItems]) const focusFallbackSurface = useCallback(() => { requestAnimationFrame(() => { @@ -842,7 +862,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { return ( s.openSettingsPage) const openSettingsTarget = useAppStore((s) => s.openSettingsTarget) const closeAutomationsPage = useAppStore((s) => s.closeAutomationsPage) - const agentStatusByPaneKey = useAppStore((s) => s.agentStatusByPaneKey) + const agentStatusEpoch = useAppStore((s) => s.agentStatusEpoch) const retainedAgentsByPaneKey = useAppStore((s) => s.retainedAgentsByPaneKey) const sshConnectionStates = useAppStore((s) => s.sshConnectionStates) const settings = useAppStore((s) => s.settings) @@ -549,6 +549,11 @@ export default function AutomationsPage(): React.JSX.Element { }, [refresh]) useEffect(() => { + // Why: completion detection only changes on agent state transitions. + // Same-state prompt/tool pings clone agentStatusByPaneKey frequently but + // cannot mark an automation complete; agentStatusEpoch is the cheap signal. + void agentStatusEpoch + const { agentStatusByPaneKey } = useAppStore.getState() const inFlight = completionInFlightRef.current const completedRuns = runs.filter((run) => { if (run.status !== 'dispatched' || !run.terminalSessionId) { @@ -604,7 +609,7 @@ export default function AutomationsPage(): React.JSX.Element { inFlight.delete(run.id) } }) - }, [agentStatusByPaneKey, retainedAgentsByPaneKey, refresh, runs]) + }, [agentStatusEpoch, retainedAgentsByPaneKey, refresh, runs]) useEffect(() => { if (!draft.projectId) { diff --git a/src/renderer/src/components/browser-pane/BrowserPane.test.ts b/src/renderer/src/components/browser-pane/BrowserPane.test.ts new file mode 100644 index 00000000000..276c8654f42 --- /dev/null +++ b/src/renderer/src/components/browser-pane/BrowserPane.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { getBrowserPagesForWorkspace, shouldPollChromiumErrorPage } from './BrowserPane' +import type { BrowserPage } from '../../../../shared/types' + +describe('shouldPollChromiumErrorPage', () => { + it('runs the fallback chrome-error poll only for the active loading browser pane', () => { + expect(shouldPollChromiumErrorPage({ isActive: true, loading: true })).toBe(true) + expect(shouldPollChromiumErrorPage({ isActive: false, loading: true })).toBe(false) + expect(shouldPollChromiumErrorPage({ isActive: true, loading: false })).toBe(false) + expect(shouldPollChromiumErrorPage({ isActive: false, loading: false })).toBe(false) + }) +}) + +describe('getBrowserPagesForWorkspace', () => { + it('returns only the owning workspace page array so unrelated page updates keep the selector stable', () => { + const pages = [{ id: 'page-1' }] as BrowserPage[] + const browserPagesByWorkspace = { + workspaceA: pages, + workspaceB: [{ id: 'page-2' }] as BrowserPage[] + } + + expect(getBrowserPagesForWorkspace(browserPagesByWorkspace, 'workspaceA')).toBe(pages) + expect( + getBrowserPagesForWorkspace( + { ...browserPagesByWorkspace, workspaceB: [{ id: 'page-3' }] as BrowserPage[] }, + 'workspaceA' + ) + ).toBe(pages) + expect(getBrowserPagesForWorkspace(browserPagesByWorkspace, 'missing')).toBe( + getBrowserPagesForWorkspace({}, 'missing') + ) + }) +}) diff --git a/src/renderer/src/components/browser-pane/BrowserPane.tsx b/src/renderer/src/components/browser-pane/BrowserPane.tsx index 7b7cb2a2447..dcff0117b1e 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPane.tsx @@ -247,6 +247,13 @@ const PENDING_ANNOTATION_CARD_HEIGHT = 330 const WHEEL_DELTA_LINE = 1 const WHEEL_DELTA_PAGE = 2 +export function getBrowserPagesForWorkspace( + browserPagesByWorkspace: Record, + workspaceId: string +): BrowserPageState[] { + return browserPagesByWorkspace[workspaceId] ?? EMPTY_BROWSER_PAGES +} + function createBrowserAnnotationId(): string { return `browser-annotation-${Date.now()}-${Math.random().toString(36).slice(2, 8)}` } @@ -503,6 +510,13 @@ function isChromiumErrorPage(url: string): boolean { return url.startsWith('chrome-error://') } +export function shouldPollChromiumErrorPage(args: { + isActive: boolean + loading: boolean +}): boolean { + return args.isActive && args.loading +} + function fileUrlToAbsolutePath(url: string): string | null { try { const parsed = new URL(url) @@ -780,8 +794,9 @@ export default function BrowserPane({ const activeRuntimeEnvironmentId = useAppStore( (s) => s.settings?.activeRuntimeEnvironmentId ?? null ) - const browserPagesByWorkspace = useAppStore((s) => s.browserPagesByWorkspace) - const browserPages = browserPagesByWorkspace[browserTab.id] ?? EMPTY_BROWSER_PAGES + const browserPages = useAppStore((s) => + getBrowserPagesForWorkspace(s.browserPagesByWorkspace, browserTab.id) + ) const activeBrowserPage = browserPages.find((page) => page.id === browserTab.activePageId) ?? browserPages[0] ?? null const updateBrowserPageState = useAppStore((s) => s.updateBrowserPageState) @@ -3658,7 +3673,7 @@ function BrowserPagePane({ }, [browserTab.url, focusWebviewNow]) useEffect(() => { - if (!browserTab.loading) { + if (!shouldPollChromiumErrorPage({ isActive, loading: browserTab.loading })) { return } @@ -3691,12 +3706,13 @@ function BrowserPagePane({ // Why: some Electron builds paint Chromium's internal chrome-error page // without delivering a timely did-fail-load event to the renderer webview. - // Polling only while the tab is "loading" gives Orca a last-resort path to - // swap the black guest surface for the explicit unreachable-page overlay. + // Polling only while the active tab is "loading" gives Orca a last-resort + // path to swap the black guest surface without waking every retained + // inactive browser pane on a 250ms loop. detectChromiumErrorPage() const intervalId = window.setInterval(detectChromiumErrorPage, 250) return () => window.clearInterval(intervalId) - }, [browserTab.id, browserTab.loading]) + }, [browserTab.id, browserTab.loading, isActive]) const startGrabIntent = useCallback( (nextIntent: GrabIntent): void => { diff --git a/src/renderer/src/components/codex-restart-chip.test.ts b/src/renderer/src/components/codex-restart-chip.test.ts index ce18af16a3f..456e6db8b17 100644 --- a/src/renderer/src/components/codex-restart-chip.test.ts +++ b/src/renderer/src/components/codex-restart-chip.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from 'vitest' -import { collectStaleWorktreePtyIds, dismissStaleWorktreePtyIds } from './CodexRestartChip' +import { + collectStalePtyIdsForTabs, + collectStaleWorktreePtyIds, + dismissStaleWorktreePtyIds +} from './CodexRestartChip' describe('CodexRestartChip helpers', () => { it('collects all stale PTY ids for tabs in a worktree', () => { @@ -39,6 +43,22 @@ describe('CodexRestartChip helpers', () => { ).toEqual([]) }) + it('collects from one worktree tab slice without scanning the whole tab map', () => { + expect( + collectStalePtyIdsForTabs({ + tabs: [{ id: 'tab-1' }], + ptyIdsByTabId: { + 'tab-1': ['pty-1'], + 'tab-2': ['pty-2'] + }, + codexRestartNoticeByPtyId: { + 'pty-1': { previousAccountLabel: 'a', nextAccountLabel: 'b' }, + 'pty-2': { previousAccountLabel: 'a', nextAccountLabel: 'b' } + } + }) + ).toEqual(['pty-1']) + }) + it('dismisses every stale PTY notice in the worktree prompt', () => { const clearCodexRestartNotice = vi.fn() diff --git a/src/renderer/src/components/dashboard/useNow.test.ts b/src/renderer/src/components/dashboard/useNow.test.ts new file mode 100644 index 00000000000..b36d6dc19a0 --- /dev/null +++ b/src/renderer/src/components/dashboard/useNow.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it, vi } from 'vitest' +import { createSharedNowClock } from './useNow' + +describe('createSharedNowClock', () => { + it('shares one timer across subscribers and clears it when idle', () => { + let now = 1_000 + const intervalCallbacks: (() => void)[] = [] + const handle = {} as ReturnType + const setIntervalMock = vi.fn((callback: () => void) => { + intervalCallbacks.push(callback) + return handle + }) + const clearIntervalMock = vi.fn() + const first = vi.fn() + const second = vi.fn() + const clock = createSharedNowClock(30_000, { + now: () => now, + setInterval: setIntervalMock, + clearInterval: clearIntervalMock + }) + + const unsubscribeFirst = clock.subscribe(first) + const unsubscribeSecond = clock.subscribe(second) + + expect(setIntervalMock).toHaveBeenCalledTimes(1) + expect(first).toHaveBeenCalledTimes(1) + expect(second).not.toHaveBeenCalled() + now = 31_000 + const intervalCallback = intervalCallbacks[0] + if (!intervalCallback) { + throw new Error('expected shared clock to schedule an interval') + } + intervalCallback() + expect(clock.getSnapshot()).toBe(31_000) + expect(first).toHaveBeenCalledTimes(2) + expect(second).toHaveBeenCalledTimes(1) + + unsubscribeFirst() + expect(clearIntervalMock).not.toHaveBeenCalled() + unsubscribeSecond() + expect(clearIntervalMock).toHaveBeenCalledWith(handle) + }) + + it('refreshes the snapshot when a new subscriber restarts an idle clock', () => { + let now = 1_000 + const handle = {} as ReturnType + const setIntervalMock = vi.fn(() => handle) + const clearIntervalMock = vi.fn() + const first = vi.fn() + const second = vi.fn() + const clock = createSharedNowClock(30_000, { + now: () => now, + setInterval: setIntervalMock, + clearInterval: clearIntervalMock + }) + + const unsubscribeFirst = clock.subscribe(first) + expect(clock.getSnapshot()).toBe(1_000) + unsubscribeFirst() + + now = 61_000 + clock.subscribe(second) + + expect(clock.getSnapshot()).toBe(61_000) + expect(second).toHaveBeenCalledTimes(1) + expect(setIntervalMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/dashboard/useNow.ts b/src/renderer/src/components/dashboard/useNow.ts index 69772999a0a..bb7c1ad4221 100644 --- a/src/renderer/src/components/dashboard/useNow.ts +++ b/src/renderer/src/components/dashboard/useNow.ts @@ -1,4 +1,67 @@ -import { useEffect, useState } from 'react' +import { useSyncExternalStore } from 'react' + +type ClockDeps = { + now: () => number + setInterval: (callback: () => void, intervalMs: number) => ReturnType + clearInterval: (handle: ReturnType) => void +} + +type SharedNowClock = { + getSnapshot: () => number + subscribe: (listener: () => void) => () => void +} + +const nowClocks = new Map() + +export function createSharedNowClock( + intervalMs: number, + deps: ClockDeps = { + now: () => Date.now(), + setInterval: (callback, ms) => setInterval(callback, ms), + clearInterval: (handle) => clearInterval(handle) + } +): SharedNowClock { + let now = deps.now() + let timer: ReturnType | null = null + const listeners = new Set<() => void>() + + const tick = (): void => { + now = deps.now() + for (const listener of listeners) { + listener() + } + } + + return { + getSnapshot: () => now, + subscribe: (listener) => { + listeners.add(listener) + if (!timer) { + // Why: all mounted relative-time labels at the same cadence can share + // one timer. Refresh immediately on restart so remounted labels don't + // display the stale timestamp left from the previous subscriber set. + tick() + timer = deps.setInterval(tick, intervalMs) + } + return () => { + listeners.delete(listener) + if (listeners.size === 0 && timer) { + deps.clearInterval(timer) + timer = null + } + } + } + } +} + +function getSharedNowClock(intervalMs: number): SharedNowClock { + let clock = nowClocks.get(intervalMs) + if (!clock) { + clock = createSharedNowClock(intervalMs) + nowClocks.set(intervalMs, clock) + } + return clock +} // Why: relative timestamps drift once mounted. A 30s tick keeps the "Xm // ago" labels honest without burning a render every second. @@ -9,10 +72,6 @@ import { useEffect, useState } from 'react' // which meant N timers firing at staggered mount times for N rows on // screen — turning one logical tick into N independent React commits. export function useNow(intervalMs: number): number { - const [now, setNow] = useState(() => Date.now()) - useEffect(() => { - const id = setInterval(() => setNow(Date.now()), intervalMs) - return () => clearInterval(id) - }, [intervalMs]) - return now + const clock = getSharedNowClock(intervalMs) + return useSyncExternalStore(clock.subscribe, clock.getSnapshot, clock.getSnapshot) } diff --git a/src/renderer/src/components/dashboard/useRetainedAgents.ts b/src/renderer/src/components/dashboard/useRetainedAgents.ts index 8a68b3638db..9f7dd80ddb2 100644 --- a/src/renderer/src/components/dashboard/useRetainedAgents.ts +++ b/src/renderer/src/components/dashboard/useRetainedAgents.ts @@ -1,4 +1,5 @@ import { useEffect, useRef } from 'react' +import { useShallow } from 'zustand/react/shallow' import { useAppStore } from '@/store' import { isExplicitAgentStatusFresh } from '@/lib/agent-status' import { type DashboardAgentRow } from './useDashboardData' @@ -148,14 +149,8 @@ export function useRetainedAgentsSync(): void { const retainAgents = useAppStore((s) => s.retainAgents) const pruneRetainedAgents = useAppStore((s) => s.pruneRetainedAgents) const clearRetentionSuppressedPaneKeys = useAppStore((s) => s.clearRetentionSuppressedPaneKeys) - const retentionSignature = useAppStore((s) => - buildRetainedAgentsSyncSignature({ - repos: s.repos, - worktreesByRepo: s.worktreesByRepo, - tabsByWorktree: s.tabsByWorktree, - agentStatusByPaneKey: s.agentStatusByPaneKey, - agentStatusEpoch: s.agentStatusEpoch - }) + const [repos, worktreesByRepo, tabsByWorktree, agentStatusEpoch] = useAppStore( + useShallow((s) => [s.repos, s.worktreesByRepo, s.tabsByWorktree, s.agentStatusEpoch] as const) ) const prevAgentsRef = useRef(new Map()) @@ -170,9 +165,9 @@ export function useRetainedAgentsSync(): void { now: Date.now() }) - // Why: read retention state via getState() instead of subscribing. This - // effect's driving input is the retention signature — retention decisions - // only need to happen when live identity/state/freshness or worktree + // Why: read retention state via getState() after the cheap ref/epoch gate + // fires. Building the full retention snapshot scans all agents, so do it + // only when live identity/state/freshness/final-done data or worktree // membership changes. Subscribing to retainedAgentsByPaneKey would create // a feedback loop because this effect calls retainAgents. const { retainedAgentsByPaneKey: retainedNow, retentionSuppressedPaneKeys } = state @@ -194,7 +189,15 @@ export function useRetainedAgentsSync(): void { if (consumedSuppressedPaneKeys.length > 0) { clearRetentionSuppressedPaneKeys(consumedSuppressedPaneKeys) } - }, [retentionSignature, retainAgents, pruneRetainedAgents, clearRetentionSuppressedPaneKeys]) + }, [ + repos, + worktreesByRepo, + tabsByWorktree, + agentStatusEpoch, + retainAgents, + pruneRetainedAgents, + clearRetentionSuppressedPaneKeys + ]) } export function collectRetainedAgentsOnDisappear(args: { diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index ce9896c0e2d..9363fcd7062 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -63,6 +63,7 @@ import { getMaximizedFloatingTerminalBounds, type FloatingTerminalPanelBounds } from './floating-terminal-panel-bounds' +import { getFloatingTerminalOpenFiles } from './floating-terminal-open-files' const EMPTY_TERMINAL_TABS: TerminalTab[] = [] const EMPTY_BROWSER_TABS: BrowserTabState[] = [] const EMPTY_GROUPS: TabGroup[] = [] @@ -88,11 +89,19 @@ export function FloatingTerminalPanel({ open, onOpenChange }: FloatingTerminalPanelProps): React.JSX.Element | null { - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) - const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) - const groupsByWorktree = useAppStore((s) => s.groupsByWorktree) - const unifiedTabsByWorktree = useAppStore((s) => s.unifiedTabsByWorktree) - const openFiles = useAppStore((s) => s.openFiles) + const tabs = useAppStore( + (s) => s.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_TERMINAL_TABS + ) + const browserTabs = useAppStore( + (s) => s.browserTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_BROWSER_TABS + ) + const groups = useAppStore( + (s) => s.groupsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_GROUPS + ) + const unifiedTabs = useAppStore( + (s) => s.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_UNIFIED_TABS + ) + const floatingFiles = useAppStore((s) => getFloatingTerminalOpenFiles(s.openFiles)) const expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId) const createTab = useAppStore((s) => s.createTab) const createBrowserTab = useAppStore((s) => s.createBrowserTab) @@ -131,14 +140,6 @@ export function FloatingTerminalPanel({ top: number } | null>(null) - const tabs = tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_TERMINAL_TABS - const browserTabs = browserTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_BROWSER_TABS - const groups = groupsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_GROUPS - const unifiedTabs = unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? EMPTY_UNIFIED_TABS - const floatingFiles = useMemo( - () => openFiles.filter((file) => file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID), - [openFiles] - ) const activeGroup = useMemo( () => groups.find((group) => group.activeTabId != null) ?? @@ -241,7 +242,7 @@ export function FloatingTerminalPanel({ handleSaveDialogSave, handleSaveDialogDiscard, handleSaveDialogCancel - } = useTerminalSaveDialog({ openFiles, closeFile, markFileDirty }) + } = useTerminalSaveDialog({ openFiles: floatingFiles, closeFile, markFileDirty }) const getNextQueuedEditorClose = useCallback((): string | null => { while (pendingEditorCloseQueueRef.current.length > 0) { diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts new file mode 100644 index 00000000000..e3521bceb3b --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import type { OpenFile } from '@/store/slices/editor' +import { getFloatingTerminalOpenFiles } from './floating-terminal-open-files' + +const file = (id: string, worktreeId: string): OpenFile => + ({ + id, + filePath: `/tmp/${id}.md`, + relativePath: `${id}.md`, + worktreeId, + language: 'markdown', + content: '', + isDirty: false, + isPinned: false, + mode: 'edit', + mtime: 0, + runtimeEnvironmentId: null + }) as OpenFile + +describe('getFloatingTerminalOpenFiles', () => { + it('preserves the filtered array when unrelated worktree files change', () => { + const floating = file('floating', FLOATING_TERMINAL_WORKTREE_ID) + const first = getFloatingTerminalOpenFiles([floating, file('main-a', 'wt-1')]) + const second = getFloatingTerminalOpenFiles([floating, file('main-b', 'wt-2')]) + + expect(second).toBe(first) + expect(second).toEqual([floating]) + }) +}) diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts new file mode 100644 index 00000000000..d3d44ba9f3d --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-open-files.ts @@ -0,0 +1,27 @@ +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import type { OpenFile } from '@/store/slices/editor' + +let cachedOpenFiles: OpenFile[] | null = null +let cachedFloatingFiles: OpenFile[] = [] + +export function getFloatingTerminalOpenFiles(openFiles: OpenFile[]): OpenFile[] { + if (openFiles === cachedOpenFiles) { + return cachedFloatingFiles + } + + const nextFloatingFiles = openFiles.filter( + (file) => file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID + ) + if ( + cachedOpenFiles !== null && + nextFloatingFiles.length === cachedFloatingFiles.length && + nextFloatingFiles.every((file, index) => file === cachedFloatingFiles[index]) + ) { + cachedOpenFiles = openFiles + return cachedFloatingFiles + } + + cachedOpenFiles = openFiles + cachedFloatingFiles = nextFloatingFiles + return cachedFloatingFiles +} diff --git a/src/renderer/src/components/github/github-rate-limit-display.tsx b/src/renderer/src/components/github/github-rate-limit-display.tsx index 815ca5dabc5..4e8f454df23 100644 --- a/src/renderer/src/components/github/github-rate-limit-display.tsx +++ b/src/renderer/src/components/github/github-rate-limit-display.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useEffect, useRef, useState } from 'react' import { Gauge, RefreshCw } from 'lucide-react' import { cn } from '@/lib/utils' +import { installFocusedVisibilityInterval } from '@/lib/focused-visibility-interval' import { useAppStore } from '@/store' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import type { GetRateLimitResult, GitHubRateLimitSnapshot } from '../../../../shared/types' @@ -96,20 +97,10 @@ export function useGitHubRateLimitSnapshot(options?: { autoRefresh?: boolean }): if (!autoRefresh) { return } - const fetchIfVisible = (): void => { - if (document.visibilityState === 'visible' && document.hasFocus()) { - void refresh(false) - } - } - void refresh(false) - const handle = window.setInterval(fetchIfVisible, REFRESH_INTERVAL_MS) - window.addEventListener('focus', fetchIfVisible) - document.addEventListener('visibilitychange', fetchIfVisible) - return () => { - window.clearInterval(handle) - window.removeEventListener('focus', fetchIfVisible) - document.removeEventListener('visibilitychange', fetchIfVisible) - } + return installFocusedVisibilityInterval({ + run: () => void refresh(false), + intervalMs: REFRESH_INTERVAL_MS + }) }, [autoRefresh, refresh]) return { snapshot, hasError, isFetching, refresh } diff --git a/src/renderer/src/components/pet/PetOverlay.tsx b/src/renderer/src/components/pet/PetOverlay.tsx index d434410d943..fa323dd3a2e 100644 --- a/src/renderer/src/components/pet/PetOverlay.tsx +++ b/src/renderer/src/components/pet/PetOverlay.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId, useRef, useState } from 'react' +import { useEffect, useId, useMemo, useRef, useState } from 'react' import { usePetUrl } from './usePetUrl' import type { DetectedSpriteCacheEntry } from './pet-blob-cache' import type { CustomPet } from '../../../../shared/types' @@ -9,21 +9,23 @@ import { selectPetAnimationName, type PetAnimationName } from './pet-agent-state type Sprite = NonNullable function usePetAnimationName(dragging: boolean): PetAnimationName { - const agentStatusByPaneKey = useAppStore((s) => s.agentStatusByPaneKey) const agentStatusEpoch = useAppStore((s) => s.agentStatusEpoch) const retainedAgentsByPaneKey = useAppStore((s) => s.retainedAgentsByPaneKey) - // Re-render when the freshness scheduler ticks so stale live states stop - // driving pet animations even if no other store value changes. - void agentStatusEpoch - - return selectPetAnimationName({ - entries: Object.values(agentStatusByPaneKey), - retainedCount: Object.keys(retainedAgentsByPaneKey).length, - dragging, - now: Date.now(), - staleAfterMs: AGENT_STATUS_STALE_AFTER_MS - }) + return useMemo(() => { + // Why: same-state prompt/tool pings clone agentStatusByPaneKey at PTY-event + // frequency, but they cannot change the coarse pet animation. Recompute on + // agentStatusEpoch instead, which also ticks when freshness boundaries pass. + void agentStatusEpoch + const { agentStatusByPaneKey } = useAppStore.getState() + return selectPetAnimationName({ + entries: Object.values(agentStatusByPaneKey), + retainedCount: Object.keys(retainedAgentsByPaneKey).length, + dragging, + now: Date.now(), + staleAfterMs: AGENT_STATUS_STALE_AFTER_MS + }) + }, [agentStatusEpoch, dragging, retainedAgentsByPaneKey]) } // Why: pet bundles ship a sprite sheet — animate by stepping a CSS background diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.test.ts b/src/renderer/src/components/ports/WorkspacePortScanner.test.ts new file mode 100644 index 00000000000..980e4d4ea3b --- /dev/null +++ b/src/renderer/src/components/ports/WorkspacePortScanner.test.ts @@ -0,0 +1,11 @@ +import { describe, expect, it } from 'vitest' +import { shouldRunWorkspacePortScan } from './WorkspacePortScanner' + +describe('shouldRunWorkspacePortScan', () => { + it('runs only while the document is visible', () => { + expect(shouldRunWorkspacePortScan({ documentVisible: true, windowFocused: true })).toBe(true) + expect(shouldRunWorkspacePortScan({ documentVisible: false, windowFocused: true })).toBe(false) + expect(shouldRunWorkspacePortScan({ documentVisible: true, windowFocused: false })).toBe(true) + expect(shouldRunWorkspacePortScan({ documentVisible: false, windowFocused: false })).toBe(false) + }) +}) diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.tsx index 74424471c8d..3f98dd88d25 100644 --- a/src/renderer/src/components/ports/WorkspacePortScanner.tsx +++ b/src/renderer/src/components/ports/WorkspacePortScanner.tsx @@ -6,10 +6,22 @@ import { scanWorkspacePortsForTarget, workspacePortRuntimeTargetKey } from '@/lib/workspace-port-actions' +import { installFocusedVisibilityInterval } from '@/lib/focused-visibility-interval' import type { WorkspacePortScanResult } from '../../../../shared/workspace-ports' const WORKSPACE_PORT_SCAN_INTERVAL_MS = 5_000 +export function shouldRunWorkspacePortScan({ + documentVisible, + windowFocused +}: { + documentVisible: boolean + windowFocused: boolean +}): boolean { + void windowFocused + return documentVisible +} + function makeUnavailableScan(reason: string): WorkspacePortScanResult { return { platform: 'unknown', @@ -71,42 +83,21 @@ export function WorkspacePortScanner(): null { }, [hasWorktrees, runtimeTarget, scanKey, setWorkspacePortScan, setWorkspacePortScanRefreshing]) useEffect(() => { - let cancelled = false - let timeout: ReturnType | null = null generationRef.current += 1 setWorkspacePortScan(null) - const run = async (): Promise => { - try { - if (document.visibilityState === 'visible') { - await refresh() - } - } finally { - if (!cancelled) { - timeout = setTimeout(() => void run(), WORKSPACE_PORT_SCAN_INTERVAL_MS) - } - } - } - - void run() - - const refreshWhenVisible = (): void => { - if (document.visibilityState === 'visible' && document.hasFocus()) { - void refresh() - } - } - window.addEventListener('focus', refreshWhenVisible) - document.addEventListener('visibilitychange', refreshWhenVisible) + // Why: workspace port scans can cross runtime IPC or shell out remotely. + // Keep the timer stopped while no UI can display the result; visibility + // changes run one immediate refresh on return. + const stopFocusedInterval = installFocusedVisibilityInterval({ + run: () => void refresh(), + intervalMs: WORKSPACE_PORT_SCAN_INTERVAL_MS + }) return () => { - cancelled = true generationRef.current += 1 inFlightRef.current = null - if (timeout) { - clearTimeout(timeout) - } - window.removeEventListener('focus', refreshWhenVisible) - document.removeEventListener('visibilitychange', refreshWhenVisible) + stopFocusedInterval() } }, [refresh, setWorkspacePortScan]) diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index ec4895d2c59..984eb9d1345 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -36,7 +36,8 @@ import { import { hostedReviewSummaryFromGitHubPRInfo } from '../../../../shared/hosted-review-github' import { checksPanelAsyncResultKey, - shouldCommitChecksPanelAsyncResult + shouldCommitChecksPanelAsyncResult, + shouldPollChecksPanel } from './checks-panel-async-result-key' export default function ChecksPanel(): React.JSX.Element { @@ -51,9 +52,15 @@ export default function ChecksPanel(): React.JSX.Element { (s) => s.getHostedReviewCreationEligibility ) const enqueueGitHubPRRefresh = useAppStore((s) => s.enqueueGitHubPRRefresh) - const gitConflictOperationByWorktree = useAppStore((s) => s.gitConflictOperationByWorktree) - const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) - const remoteStatusesByWorktree = useAppStore((s) => s.remoteStatusesByWorktree) + const conflictOperation = useAppStore((s) => + activeWorktreeId ? (s.gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') : 'unknown' + ) + const hasUncommittedChanges = useAppStore((s) => + activeWorktreeId ? (s.gitStatusByWorktree[activeWorktreeId]?.length ?? 0) > 0 : false + ) + const remoteStatus = useAppStore((s) => + activeWorktreeId ? s.remoteStatusesByWorktree[activeWorktreeId] : undefined + ) const pushBranch = useAppStore((s) => s.pushBranch) const fetchUpstreamStatus = useAppStore((s) => s.fetchUpstreamStatus) const setRightSidebarOpen = useAppStore((s) => s.setRightSidebarOpen) @@ -135,13 +142,6 @@ export default function ChecksPanel(): React.JSX.Element { prCacheKey ? s.prRefreshStates[prCacheKey] : undefined ) const prNumber = pr?.number ?? null - const remoteStatus = activeWorktreeId ? remoteStatusesByWorktree[activeWorktreeId] : undefined - const hasUncommittedChanges = activeWorktreeId - ? (gitStatusByWorktree[activeWorktreeId]?.length ?? 0) > 0 - : false - const conflictOperation = activeWorktreeId - ? (gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') - : 'unknown' // Why: select only timestamps (not whole cache records) so the entry-refresh // effect doesn't re-run on every cache mutation. See @@ -389,24 +389,62 @@ export default function ChecksPanel(): React.JSX.Element { pollIntervalRef.current = 30_000 prevChecksRef.current = '' let cancelled = false - void fetchChecks() - const schedulePoll = (): void => { + const shouldPollNow = (): boolean => + shouldPollChecksPanel({ + documentVisible: document.visibilityState === 'visible', + windowFocused: document.hasFocus() + }) + + const clearScheduledPoll = (): void => { + if (pollRef.current) { + clearTimeout(pollRef.current) + pollRef.current = null + } + } + + let fetchInFlight: Promise | null = null + function schedulePoll(): void { + clearScheduledPoll() + if (cancelled || !shouldPollNow()) { + return + } pollRef.current = setTimeout(() => { - void fetchChecks().then(() => { - if (!cancelled) { - schedulePoll() - } - }) + pollRef.current = null + fetchAndSchedule() }, pollIntervalRef.current) } - schedulePoll() + + function fetchAndSchedule(): void { + clearScheduledPoll() + if (cancelled || !shouldPollNow() || fetchInFlight) { + return + } + fetchInFlight = fetchChecks().finally(() => { + fetchInFlight = null + schedulePoll() + }) + } + + const reconcileVisibility = (): void => { + if (shouldPollNow()) { + fetchAndSchedule() + } else { + clearScheduledPoll() + } + } + + fetchAndSchedule() + window.addEventListener('focus', reconcileVisibility) + window.addEventListener('blur', reconcileVisibility) + document.addEventListener('visibilitychange', reconcileVisibility) return () => { cancelled = true - if (pollRef.current) { - clearTimeout(pollRef.current) - } + clearScheduledPoll() + window.removeEventListener('focus', reconcileVisibility) + window.removeEventListener('blur', reconcileVisibility) + document.removeEventListener('visibilitychange', reconcileVisibility) } }, [fetchChecks, isPanelVisible, prNumber]) diff --git a/src/renderer/src/components/right-sidebar/FileExplorer.tsx b/src/renderer/src/components/right-sidebar/FileExplorer.tsx index 4fd3d40c830..fbae4e49fd7 100644 --- a/src/renderer/src/components/right-sidebar/FileExplorer.tsx +++ b/src/renderer/src/components/right-sidebar/FileExplorer.tsx @@ -29,6 +29,10 @@ import { useFileExplorerTree } from './useFileExplorerTree' import { useFileExplorerWatch } from './useFileExplorerWatch' import { useFileExplorerSelection } from './useFileExplorerSelection' import { useFileExplorerGitIgnoredRows } from './useFileExplorerGitIgnoredRows' +import { getActiveWorktreeOpenFiles } from '@/components/terminal/active-worktree-open-files' +import type { GitStatusEntry } from '../../../../shared/git-status-types' + +const EMPTY_GIT_STATUS_ENTRIES: GitStatusEntry[] = [] function FileExplorerInner(): React.JSX.Element { const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) @@ -44,8 +48,12 @@ function FileExplorerInner(): React.JSX.Element { const openFile = useAppStore((s) => s.openFile) const pinFile = useAppStore((s) => s.pinFile) const activeFileId = useAppStore((s) => s.activeFileId) - const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) - const openFiles = useAppStore((s) => s.openFiles) + const entries = useAppStore((s) => + activeWorktreeId + ? (s.gitStatusByWorktree[activeWorktreeId] ?? EMPTY_GIT_STATUS_ENTRIES) + : EMPTY_GIT_STATUS_ENTRIES + ) + const openFiles = useAppStore((s) => getActiveWorktreeOpenFiles(s.openFiles, activeWorktreeId)) const closeFile = useAppStore((s) => s.closeFile) const worktreePath = activeWorktree?.path ?? null @@ -111,10 +119,6 @@ function FileExplorerInner(): React.JSX.Element { } }, []) - const entries = useMemo( - () => (activeWorktreeId ? (gitStatusByWorktree[activeWorktreeId] ?? []) : []), - [activeWorktreeId, gitStatusByWorktree] - ) const statusByRelativePath = useMemo(() => buildStatusMap(entries), [entries]) const folderStatusByRelativePath = useMemo(() => buildFolderStatusMap(entries), [entries]) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 241405fabe3..4ec7d852f8e 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -107,6 +107,7 @@ import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { DiffNotesSendMenu } from '@/components/editor/DiffNotesSendMenu' import { AGENT_CATALOG } from '@/lib/agent-catalog' import { launchAgentInNewTab } from '@/lib/launch-agent-in-new-tab' +import { installFocusedVisibilityInterval } from '@/lib/focused-visibility-interval' import { notifyEditorExternalFileChange, requestEditorSaveQuiesce @@ -142,7 +143,6 @@ import type { GitConflictKind, GitConflictOperation, GitStatusEntry, - GitUpstreamStatus, GlobalSettings, SourceControlViewMode, TuiAgent @@ -161,6 +161,9 @@ import { hasExpandedCommitFailureDetails, summarizeCommitFailure } from './commi export type SourceControlScope = 'all' | 'uncommitted' type RemoteActionError = { kind: RemoteOpKind; message: string } +const EMPTY_GIT_STATUS_ENTRIES: GitStatusEntry[] = [] +const EMPTY_BRANCH_CHANGE_ENTRIES: GitBranchChangeEntry[] = [] + // Why: directional signifiers ahead of each primary action label. Commit // (✓) is affirmative; Push (↑) points in the direction data flows; Sync // (↕) is bidirectional; Publish gets a cloud-up to distinguish the @@ -764,11 +767,27 @@ function SourceControlInner(): React.JSX.Element { const worktreeMap = useWorktreeMap() const rightSidebarTab = useAppStore((s) => s.rightSidebarTab) const activeRepo = useRepoById(activeWorktree?.repoId ?? null) - const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) - const gitConflictOperationByWorktree = useAppStore((s) => s.gitConflictOperationByWorktree) - const gitBranchChangesByWorktree = useAppStore((s) => s.gitBranchChangesByWorktree) - const gitBranchCompareSummaryByWorktree = useAppStore((s) => s.gitBranchCompareSummaryByWorktree) - const remoteStatusesByWorktree = useAppStore((s) => s.remoteStatusesByWorktree) + const entries = useAppStore((s) => + activeWorktreeId + ? (s.gitStatusByWorktree[activeWorktreeId] ?? EMPTY_GIT_STATUS_ENTRIES) + : EMPTY_GIT_STATUS_ENTRIES + ) + const branchEntries = useAppStore((s) => + activeWorktreeId + ? (s.gitBranchChangesByWorktree[activeWorktreeId] ?? EMPTY_BRANCH_CHANGE_ENTRIES) + : EMPTY_BRANCH_CHANGE_ENTRIES + ) + const branchSummary = useAppStore((s) => + activeWorktreeId ? (s.gitBranchCompareSummaryByWorktree[activeWorktreeId] ?? null) : null + ) + const conflictOperation = useAppStore((s) => + activeWorktreeId ? (s.gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') : 'unknown' + ) + // Why: leave undefined until fetchUpstreamStatus resolves for this worktree. + // A synthetic "no upstream" flashes "Publish Branch" during worktree switches. + const remoteStatus = useAppStore((s) => + activeWorktreeId ? s.remoteStatusesByWorktree[activeWorktreeId] : undefined + ) const isRemoteOperationActive = useAppStore((s) => s.isRemoteOperationActive) const inFlightRemoteOpKind = useAppStore((s) => s.inFlightRemoteOpKind) const settings = useAppStore((s) => s.settings) @@ -1035,27 +1054,6 @@ function SourceControlInner(): React.JSX.Element { activePullRequestGenerationRecordCandidate.context.branch === branchName ? activePullRequestGenerationRecordCandidate : null - const entries = useMemo( - () => (activeWorktreeId ? (gitStatusByWorktree[activeWorktreeId] ?? []) : []), - [activeWorktreeId, gitStatusByWorktree] - ) - const branchEntries = useMemo( - () => (activeWorktreeId ? (gitBranchChangesByWorktree[activeWorktreeId] ?? []) : []), - [activeWorktreeId, gitBranchChangesByWorktree] - ) - const branchSummary = activeWorktreeId - ? (gitBranchCompareSummaryByWorktree[activeWorktreeId] ?? null) - : null - const conflictOperation = activeWorktreeId - ? (gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') - : 'unknown' - // Why: leave undefined until fetchUpstreamStatus resolves for this worktree. - // Substituting a synthetic { hasUpstream: false } flashes "Publish Branch" - // on every worktree switch — resolvePrimaryAction treats it as an - // unpublished branch until the real status lands a moment later. - const remoteStatus: GitUpstreamStatus | undefined = activeWorktreeId - ? remoteStatusesByWorktree[activeWorktreeId] - : undefined const rightSidebarOpen = useAppStore((s) => s.rightSidebarOpen) // Why: gate polling on both the active tab AND the sidebar being open. // The sidebar now stays mounted when closed (for performance), so without @@ -2810,7 +2808,12 @@ function SourceControlInner(): React.JSX.Element { refreshActiveGitStatusAfterMutation ]) - const refreshBranchCompare = useCallback(async () => { + const branchCompareInFlightRef = useRef(false) + const branchCompareRerunRef = useRef(false) + const branchCompareRunPromiseRef = useRef | null>(null) + const refreshBranchCompareRef = useRef<() => Promise>(async () => {}) + + const runBranchCompare = useCallback(async () => { if (!activeWorktreeId || !worktreePath || !effectiveBaseRef || isFolder) { return } @@ -2876,7 +2879,38 @@ function SourceControlInner(): React.JSX.Element { worktreePath ]) - const refreshBranchCompareRef = useRef(refreshBranchCompare) + const refreshBranchCompare = useCallback(async () => { + if (branchCompareInFlightRef.current) { + branchCompareRerunRef.current = true + return branchCompareRunPromiseRef.current ?? undefined + } + + branchCompareInFlightRef.current = true + const runPromise = (async (): Promise => { + // Why: branch compare shells out to git on a timer and can exceed the + // 5s poll interval on large repos. Keep one compare chain in flight and + // collapse skipped ticks into one trailing refresh instead of stacking + // subprocesses while preserving the await contract for direct callers. + try { + await runBranchCompare() + } finally { + branchCompareInFlightRef.current = false + if (branchCompareRerunRef.current) { + branchCompareRerunRef.current = false + await refreshBranchCompareRef.current() + } + } + })() + branchCompareRunPromiseRef.current = runPromise + try { + await runPromise + } finally { + if (branchCompareRunPromiseRef.current === runPromise) { + branchCompareRunPromiseRef.current = null + } + } + }, [runBranchCompare]) + refreshBranchCompareRef.current = refreshBranchCompare const refreshGitHistory = useCallback(async (): Promise => { @@ -2938,21 +2972,13 @@ function SourceControlInner(): React.JSX.Element { return } - void refreshBranchCompareRef.current() - const refreshIfFocused = (): void => { - if (document.hasFocus()) { - void refreshBranchCompareRef.current() - } - } // Why: branch compare shells out to git every tick. The panel only needs - // background freshness while Orca is focused; on focus we refresh - // immediately so hidden-window time does not burn subprocess work. - const intervalId = window.setInterval(refreshIfFocused, BRANCH_REFRESH_INTERVAL_MS) - window.addEventListener('focus', refreshIfFocused) - return () => { - window.clearInterval(intervalId) - window.removeEventListener('focus', refreshIfFocused) - } + // background freshness while Orca is visible; hidden-window time should not + // burn subprocess work or timer wakeups. + return installFocusedVisibilityInterval({ + run: () => void refreshBranchCompareRef.current(), + intervalMs: BRANCH_REFRESH_INTERVAL_MS + }) }, [activeWorktreeId, effectiveBaseRef, isBranchVisible, isFolder, worktreePath]) useEffect(() => { diff --git a/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.test.ts b/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.test.ts index d6877ed424b..e5daf0082ec 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.test.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it } from 'vitest' import { checksPanelAsyncResultKey, - shouldCommitChecksPanelAsyncResult + shouldCommitChecksPanelAsyncResult, + shouldPollChecksPanel } from './checks-panel-async-result-key' describe('checksPanelAsyncResultKey', () => { @@ -33,6 +34,14 @@ describe('checksPanelAsyncResultKey', () => { }) }) +describe('shouldPollChecksPanel', () => { + it('skips checks polling only while the window is hidden', () => { + expect(shouldPollChecksPanel({ documentVisible: false, windowFocused: true })).toBe(false) + expect(shouldPollChecksPanel({ documentVisible: true, windowFocused: false })).toBe(true) + expect(shouldPollChecksPanel({ documentVisible: true, windowFocused: true })).toBe(true) + }) +}) + describe('shouldCommitChecksPanelAsyncResult', () => { it('suppresses stale async completions', () => { expect( diff --git a/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.ts b/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.ts index 470020680f6..5a8668aa2bf 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-async-result-key.ts @@ -25,3 +25,11 @@ export function shouldCommitChecksPanelAsyncResult( ): boolean { return currentKey === requestKey } + +export function shouldPollChecksPanel(args: { + documentVisible: boolean + windowFocused: boolean +}): boolean { + void args.windowFocused + return args.documentVisible +} diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerDragDrop.ts b/src/renderer/src/components/right-sidebar/useFileExplorerDragDrop.ts index f28f9ca1b90..8cbf1d5d68d 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerDragDrop.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerDragDrop.ts @@ -12,6 +12,7 @@ import { remapOpenEditorTabsForPathChange } from '@/lib/remap-open-editor-tabs-f import { requestEditorSaveQuiesce } from '@/components/editor/editor-autosave' import { commitFileExplorerOp } from './fileExplorerUndoRedo' import { renameRuntimePath } from '@/runtime/runtime-file-client' +import { getActiveWorktreeOpenFiles } from '@/components/terminal/active-worktree-open-files' function extractIpcErrorMessage(err: unknown, fallback: string): string { if (!(err instanceof Error)) { @@ -100,7 +101,7 @@ export function useFileExplorerDragDrop({ refreshDir, scrollRef }: UseFileExplorerDragDropParams): UseFileExplorerDragDropResult { - const openFiles = useAppStore((s) => s.openFiles) + const openFiles = useAppStore((s) => getActiveWorktreeOpenFiles(s.openFiles, activeWorktreeId)) const [isRootDragOver, setIsRootDragOver] = useState(false) const rootDragCounterRef = useRef(0) diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts index 798b669f0a3..08c3e686d6e 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts @@ -243,4 +243,50 @@ describe('useGitStatusPolling', () => { await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(2)) await vi.waitFor(() => expect(state.setGitStatus).toHaveBeenCalledTimes(2)) }) + + it('keeps the git poll interval scoped to visible windows', async () => { + vi.resetModules() + let visibilityState: DocumentVisibilityState = 'hidden' + const listeners = new Map void>() + const clearIntervalMock = vi.fn() + const setIntervalMock = vi.fn(() => 1 as unknown as ReturnType) + const run = vi.fn() + + vi.stubGlobal('window', { + addEventListener: vi.fn(), + removeEventListener: vi.fn() + }) + vi.stubGlobal('document', { + get visibilityState() { + return visibilityState + }, + hasFocus: () => false, + addEventListener: vi.fn((event: string, listener: () => void) => { + listeners.set(event, listener) + }), + removeEventListener: vi.fn() + }) + + const { installFocusedGitPoll } = await import('./useGitStatusPolling') + const cleanup = installFocusedGitPoll({ + run, + intervalMs: 3000, + setIntervalFn: setIntervalMock, + clearIntervalFn: clearIntervalMock + }) + + expect(run).not.toHaveBeenCalled() + expect(setIntervalMock).not.toHaveBeenCalled() + + visibilityState = 'visible' + listeners.get('visibilitychange')?.() + expect(run).toHaveBeenCalledTimes(1) + expect(setIntervalMock).toHaveBeenCalledTimes(1) + + visibilityState = 'hidden' + listeners.get('visibilitychange')?.() + expect(clearIntervalMock).toHaveBeenCalledWith(1) + + cleanup() + }) }) diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts index f39d9272e37..541669e9a83 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts @@ -7,8 +7,14 @@ import { getConnectionId } from '@/lib/connection-context' import { getRuntimeGitConflictOperation } from '@/runtime/runtime-git-client' import { refreshGitStatusForWorktree } from './git-status-refresh' import { createCoalescedPollRunner } from './coalesced-poll-runner' +import { + installFocusedVisibilityInterval, + isWindowVisible +} from '@/lib/focused-visibility-interval' const POLL_INTERVAL_MS = 3000 +export const isGitPollWindowVisible = isWindowVisible +export const installFocusedGitPoll = installFocusedVisibilityInterval export function useGitStatusPolling(): void { const activeWorktree = useActiveWorktree() @@ -115,24 +121,7 @@ export function useGitStatusPolling(): void { fetchStatusRef.current = fetchStatus useEffect(() => { - void fetchStatus() - // Why: skip IPC-heavy git status calls when the window is not focused. - // These intervals run at the App root level regardless of which sidebar tab - // is open, so gating on document.hasFocus() prevents wasted CPU and IPC - // traffic while the user is working in another application. - const intervalId = setInterval(() => { - if (document.hasFocus()) { - void fetchStatus() - } - }, POLL_INTERVAL_MS) - // Why: when the user returns to the window, poll immediately so the sidebar - // shows up-to-date status without waiting up to POLL_INTERVAL_MS. - const onFocus = (): void => void fetchStatus() - window.addEventListener('focus', onFocus) - return () => { - clearInterval(intervalId) - window.removeEventListener('focus', onFocus) - } + return installFocusedVisibilityInterval({ run: fetchStatus, intervalMs: POLL_INTERVAL_MS }) }, [fetchStatus]) // Why: poll conflict operation for non-active worktrees that have a stale @@ -169,18 +158,13 @@ export function useGitStatusPolling(): void { // flight and coalesce skipped ticks into one trailing pass so stale badges // catch up without stacking SSH/RPC work. const pollRunner = createCoalescedPollRunner(pollStale) - pollRunner.run() - const intervalId = setInterval(() => { - if (document.hasFocus()) { - pollRunner.run() - } - }, POLL_INTERVAL_MS) - const onFocus = (): void => pollRunner.run() - window.addEventListener('focus', onFocus) + const stopFocusedPoll = installFocusedVisibilityInterval({ + run: () => pollRunner.run(), + intervalMs: POLL_INTERVAL_MS + }) return () => { pollRunner.dispose() - clearInterval(intervalId) - window.removeEventListener('focus', onFocus) + stopFocusedPoll() } }, [staleConflictWorktrees, setConflictOperation, isConnectionReady]) } diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index 6336c713505..e761db02c8d 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -33,7 +33,12 @@ import type { IssueInfo, LinearIssue } from '../../../../shared/types' -import { branchDisplayName, CONFLICT_OPERATION_LABELS, FilledBellIcon } from './WorktreeCardHelpers' +import { + branchDisplayName, + CONFLICT_OPERATION_LABELS, + FilledBellIcon, + shouldRefreshWorktreeCardDecoration +} from './WorktreeCardHelpers' import { WorktreeCardDetailsHover, WorktreeCardMetaBadges, @@ -45,9 +50,11 @@ import { writeWorkspaceDragData } from './workspace-status' import { getWorktreeCardPrDisplay } from './worktree-card-pr-display' import { getWorkspacePortsByWorktreeId } from '@/lib/workspace-port-groups' import { hasActiveWorkspaceActivity } from '@/lib/worktree-activity-state' +import { installFocusedVisibilityInterval } from '@/lib/focused-visibility-interval' import { runWorktreeDelete } from './delete-worktree-flow' import { runSleepWorktree } from './sleep-worktree-flow' import { getWorkspaceQuickActionKind } from './worktree-card-quick-action' +import { useMacOptionKeyPressed } from './mac-option-key-state' type WorktreeCardProps = { worktree: Worktree @@ -160,26 +167,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ }) const isSshDisconnected = sshStatus != null && sshStatus !== 'connected' const [showDisconnectedDialog, setShowDisconnectedDialog] = useState(false) - const [isMacOptionPressed, setIsMacOptionPressed] = useState(false) - - useEffect(() => { - const isMac = navigator.userAgent.includes('Mac') - if (!isMac) { - return - } - const handleKeyChange = (event: KeyboardEvent): void => { - setIsMacOptionPressed(event.altKey) - } - const handleWindowBlur = (): void => setIsMacOptionPressed(false) - window.addEventListener('keydown', handleKeyChange, true) - window.addEventListener('keyup', handleKeyChange, true) - window.addEventListener('blur', handleWindowBlur) - return () => { - window.removeEventListener('keydown', handleKeyChange, true) - window.removeEventListener('keyup', handleKeyChange, true) - window.removeEventListener('blur', handleWindowBlur) - } - }, []) + const isMacOptionPressed = useMacOptionKeyPressed() // Why: on restart the previously-active worktree is auto-restored without a // click, so the dialog never opens. Auto-show it for the active card when SSH @@ -282,10 +270,21 @@ const WorktreeCard = React.memo(function WorktreeCard({ if (isWebClient()) { return } - if (repo && !isFolder && !worktree.isBare && hostedReviewCacheKey && showPR) { + if (!repo || isFolder || worktree.isBare || !hostedReviewCacheKey || !showPR) { + return + } + const refreshHostedReviewIfVisible = (): void => { + if ( + !shouldRefreshWorktreeCardDecoration({ + documentVisible: document.visibilityState === 'visible', + windowFocused: document.hasFocus() + }) + ) { + return + } // Why: branch lookup is lossy for fork/deleted-head PRs; reuse a known PR // number from metadata or the visible cache whenever we have one. - fetchHostedReviewForBranch(repo.path, branch, { + void fetchHostedReviewForBranch(repo.path, branch, { repoId: repo.id, linkedGitHubPR: worktree.linkedPR ?? null, fallbackGitHubPR: fallbackGitHubPRNumber, @@ -293,6 +292,13 @@ const WorktreeCard = React.memo(function WorktreeCard({ staleWhileRevalidate: true }) } + refreshHostedReviewIfVisible() + window.addEventListener('focus', refreshHostedReviewIfVisible) + document.addEventListener('visibilitychange', refreshHostedReviewIfVisible) + return () => { + window.removeEventListener('focus', refreshHostedReviewIfVisible) + document.removeEventListener('visibilitychange', refreshHostedReviewIfVisible) + } }, [ repo, isFolder, @@ -323,21 +329,51 @@ const WorktreeCard = React.memo(function WorktreeCard({ return } - fetchIssue(repo.path, worktree.linkedIssue, { repoId: repo.id }) + const issueNumber = worktree.linkedIssue + const refreshIssueIfVisible = (): void => { + if ( + !shouldRefreshWorktreeCardDecoration({ + documentVisible: document.visibilityState === 'visible', + windowFocused: document.hasFocus() + }) + ) { + return + } + void fetchIssue(repo.path, issueNumber, { repoId: repo.id }) + } - // Background poll as fallback (activity triggers handle the fast path) - const interval = setInterval(() => { - fetchIssue(repo.path, worktree.linkedIssue!, { repoId: repo.id }) - }, 5 * 60_000) // 5 minutes - - return () => clearInterval(interval) + // Background poll as fallback (activity triggers handle the fast path). + // The interval itself is stopped while hidden so issue cards do not keep + // long-lived workspaces waking just to skip their fetch. + return installFocusedVisibilityInterval({ + run: refreshIssueIfVisible, + intervalMs: 5 * 60_000 + }) }, [repo, isFolder, worktree.linkedIssue, fetchIssue, issueCacheKey, showIssue]) useEffect(() => { if (!worktree.linkedLinearIssue || !showIssue) { return } - void fetchLinearIssue(worktree.linkedLinearIssue) + const linearIssueId = worktree.linkedLinearIssue + const refreshLinearIssueIfVisible = (): void => { + if ( + !shouldRefreshWorktreeCardDecoration({ + documentVisible: document.visibilityState === 'visible', + windowFocused: document.hasFocus() + }) + ) { + return + } + void fetchLinearIssue(linearIssueId) + } + refreshLinearIssueIfVisible() + window.addEventListener('focus', refreshLinearIssueIfVisible) + document.addEventListener('visibilitychange', refreshLinearIssueIfVisible) + return () => { + window.removeEventListener('focus', refreshLinearIssueIfVisible) + document.removeEventListener('visibilitychange', refreshLinearIssueIfVisible) + } }, [worktree.linkedLinearIssue, fetchLinearIssue, showIssue]) // Stable click handler – ignore clicks that are really text selections. diff --git a/src/renderer/src/components/sidebar/WorktreeCardHelpers.test.ts b/src/renderer/src/components/sidebar/WorktreeCardHelpers.test.ts new file mode 100644 index 00000000000..5b0014d7974 --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeCardHelpers.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'vitest' +import { shouldRefreshWorktreeCardDecoration } from './WorktreeCardHelpers' + +describe('shouldRefreshWorktreeCardDecoration', () => { + it('skips card decoration refreshes only while the app is hidden', () => { + expect( + shouldRefreshWorktreeCardDecoration({ documentVisible: false, windowFocused: true }) + ).toBe(false) + expect( + shouldRefreshWorktreeCardDecoration({ documentVisible: true, windowFocused: false }) + ).toBe(true) + expect( + shouldRefreshWorktreeCardDecoration({ documentVisible: true, windowFocused: true }) + ).toBe(true) + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeCardHelpers.tsx b/src/renderer/src/components/sidebar/WorktreeCardHelpers.tsx index 46de66e50fa..b577a5f36aa 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardHelpers.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardHelpers.tsx @@ -30,6 +30,14 @@ export function checksLabel(status: CheckStatus): string { } } +export function shouldRefreshWorktreeCardDecoration(args: { + documentVisible: boolean + windowFocused: boolean +}): boolean { + void args.windowFocused + return args.documentVisible +} + export const CONFLICT_OPERATION_LABELS: Record, string> = { merge: 'Merging', rebase: 'Rebasing', diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts b/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts index 335cea689a3..504efaec0a0 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + getSleepableWorkspaceIds, hasSleepableWorkspaceActivity, shouldUseNativeContextMenu, shouldIgnoreNestedWorktreeContextMenuScope, @@ -113,3 +114,25 @@ describe('hasSleepableWorkspaceActivity', () => { ) }) }) + +describe('getSleepableWorkspaceIds', () => { + it('selects only requested worktrees with live activity', () => { + expect( + getSleepableWorkspaceIds( + ['wt-1', 'wt-2'], + { + 'wt-1': [{ id: 'tab-1' }], + 'wt-3': [{ id: 'tab-3' }] + }, + { + 'tab-1': ['pty-1'], + 'tab-3': ['pty-3'] + }, + { + 'wt-2': [], + 'wt-4': [{ id: 'browser-4' }] + } + ) + ).toEqual(['wt-1']) + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index 550554f9794..e1e1114c6b6 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -1,5 +1,6 @@ /* eslint-disable max-lines -- Why: this menu keeps row targeting, batch actions, and ctrl-click event guards together so nested worktree menus share one event policy. */ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useShallow } from 'zustand/react/shallow' import { DropdownMenu, DropdownMenuContent, @@ -101,6 +102,17 @@ function hasSleepableWorkspaceActivity( return hasLiveTerminal || hasBrowser } +function getSleepableWorkspaceIds( + worktreeIds: readonly string[], + tabsByWorktree: Record, + ptyIdsByTabId: Record, + browserTabsByWorktree: Record +): string[] { + return worktreeIds.filter((worktreeId) => + hasSleepableWorkspaceActivity(worktreeId, tabsByWorktree, ptyIdsByTabId, browserTabsByWorktree) + ) +} + function findSidebarVirtualRowByKey(sidebar: Element, rowKey: string): HTMLElement | null { return ( Array.from(sidebar.querySelectorAll('[data-worktree-virtual-row]')).find( @@ -179,26 +191,38 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ const [menuOpen, setMenuOpen] = useState(false) const [menuPoint, setMenuPoint] = useState({ x: 0, y: 0 }) const [contextWorktrees, setContextWorktrees] = useState(selectedWorktrees) + const activeContextWorktrees = menuOpen ? contextWorktrees : selectedWorktrees + const activeContextWorktreeIds = useMemo( + () => activeContextWorktrees.map((item) => item.id), + [activeContextWorktrees] + ) const isDeleting = deleteState?.isDeleting ?? false const isFolder = repo ? isFolderRepo(repo) : false const repoMap = useRepoMap() const worktreeMap = useWorktreeMap() const worktreeLineageById = useAppStore((s) => s.worktreeLineageById) const updateWorktreeLineage = useAppStore((s) => s.updateWorktreeLineage) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) - const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId) - const browserTabsByWorktree = useAppStore((s) => s.browserTabsByWorktree) + const sleepableWorktreeIds = useAppStore( + useShallow((s) => + getSleepableWorkspaceIds( + activeContextWorktreeIds, + s.tabsByWorktree, + s.ptyIdsByTabId, + s.browserTabsByWorktree + ) + ) + ) const deleteStateByWorktreeId = useAppStore((s) => s.deleteStateByWorktreeId) const scopeRef = useRef(null) const contextMenuOpenedAtRef = useRef(null) - const activeContextWorktrees = menuOpen ? contextWorktrees : selectedWorktrees const isMultiContext = activeContextWorktrees.length > 1 + const sleepableWorktreeIdSet = useMemo( + () => new Set(sleepableWorktreeIds), + [sleepableWorktreeIds] + ) const sleepableWorktrees = useMemo( - () => - activeContextWorktrees.filter((item) => - hasSleepableWorkspaceActivity(item.id, tabsByWorktree, ptyIdsByTabId, browserTabsByWorktree) - ), - [activeContextWorktrees, browserTabsByWorktree, ptyIdsByTabId, tabsByWorktree] + () => activeContextWorktrees.filter((item) => sleepableWorktreeIdSet.has(item.id)), + [activeContextWorktrees, sleepableWorktreeIdSet] ) const deletingContext = useMemo( () => activeContextWorktrees.some((item) => deleteStateByWorktreeId[item.id]?.isDeleting), @@ -580,6 +604,7 @@ export { CLOSE_ALL_CONTEXT_MENUS_EVENT, WORKTREE_CONTEXT_MENU_SCOPE_ATTR, WORKTREE_NATIVE_CONTEXT_MENU_ATTR, + getSleepableWorkspaceIds, hasSleepableWorkspaceActivity, shouldUseNativeContextMenu, shouldSuppressContextMenuFollowUpClick, diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 73d0c2b3517..e183ae6a70f 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -76,6 +76,10 @@ import { setVisibleWorktreeIds, sidebarHasActiveFilters } from './visible-worktrees' +import { + getVisibleWorktreeBrowserActivityTabs, + getVisibleWorktreeTerminalActivityTabs +} from './visible-worktree-activity-inputs' import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT, useVirtualizedScrollAnchor, @@ -123,6 +127,18 @@ const WORKTREE_SIDEBAR_SCROLL_STYLE: React.CSSProperties = { overflowAnchor: 'none' } +const recordKeyCountCache = new WeakMap, number>() + +export function countRecordKeysByReference(record: Record): number { + const cached = recordKeyCountCache.get(record) + if (cached !== undefined) { + return cached + } + const count = Object.keys(record).length + recordKeyCountCache.set(record, count) + return count +} + export function shouldAdjustWorktreeSidebarMeasuredRowScroll(args: { isScrolling: boolean now: number @@ -469,7 +485,7 @@ function getVirtualRowKey(element: Element): string | null { return element.getAttribute('data-worktree-virtual-row-key') } -const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewport({ +export const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewport({ rows, activeWorktreeId, groupBy, @@ -512,6 +528,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp const [worktreeDragState, setWorktreeDragState] = useState( WORKTREE_ROW_DRAG_INITIAL_STATE ) + const [documentVisibilityRevision, setDocumentVisibilityRevision] = useState(0) const worktreeDragSessionRef = useRef(null) const worktreePointerDragRef = useRef(null) const suppressWorktreeClickUntilRef = useRef(0) @@ -525,6 +542,14 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp const prVisibleRefreshGeneration = useAppStore((s) => s.prVisibleRefreshGeneration) const settings = useAppStore((s) => s.settings) + useEffect( + () => + installWorktreeVisibleRefreshVisibilityListener(() => { + setDocumentVisibilityRevision((revision) => revision + 1) + }), + [] + ) + // Drag is only meaningful when repo headers are using manual order. The // controller is still constructed for hook order stability when inert. const repoDrag = useRepoHeaderDrag({ @@ -885,8 +910,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp settings ]) - const prCacheLen = useAppStore((s) => Object.keys(s.prCache).length) - const issueCacheLen = useAppStore((s) => Object.keys(s.issueCache).length) + const prCacheLen = useAppStore((s) => countRecordKeysByReference(s.prCache)) + const issueCacheLen = useAppStore((s) => countRecordKeysByReference(s.issueCache)) const renderRowKeySignature = useMemo( () => renderRows.map(getRenderRowKey).join('\n'), [renderRows] @@ -1511,6 +1536,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp reportVisibleGitHubPRRefreshCandidates(visibleWorktreeIds, Date.now()) }, [ cardProps, + documentVisibilityRevision, groupBy, renderRows, reportVisibleGitHubPRRefreshCandidates, @@ -2254,6 +2280,11 @@ type WorktreeListProps = { scrollAnchorRef: React.MutableRefObject } +export function installWorktreeVisibleRefreshVisibilityListener(onChange: () => void): () => void { + document.addEventListener('visibilitychange', onChange) + return () => document.removeEventListener('visibilitychange', onChange) +} + const WorktreeList = React.memo(function WorktreeList({ scrollOffsetRef, scrollAnchorRef @@ -2282,10 +2313,12 @@ const WorktreeList = React.memo(function WorktreeList({ // Read tabsByWorktree when needed for filtering or sorting const needsActivityMaps = !showSleepingWorkspaces || sortBy === 'smart' - const tabsByWorktree = useAppStore((s) => (needsActivityMaps ? s.tabsByWorktree : null)) + const tabsByWorktree = useAppStore((s) => + needsActivityMaps ? getVisibleWorktreeTerminalActivityTabs(s.tabsByWorktree) : null + ) const ptyIdsByTabId = useAppStore((s) => (needsActivityMaps ? s.ptyIdsByTabId : null)) const browserTabsByWorktree = useAppStore((s) => - !showSleepingWorkspaces ? s.browserTabsByWorktree : null + !showSleepingWorkspaces ? getVisibleWorktreeBrowserActivityTabs(s.browserTabsByWorktree) : null ) const cardProps = useAppStore((s) => s.worktreeCardProperties) diff --git a/src/renderer/src/components/sidebar/mac-option-key-state.test.ts b/src/renderer/src/components/sidebar/mac-option-key-state.test.ts new file mode 100644 index 00000000000..a5f7f0ae2d7 --- /dev/null +++ b/src/renderer/src/components/sidebar/mac-option-key-state.test.ts @@ -0,0 +1,80 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +type StoredListener = (event: KeyboardEvent) => void + +function createWindowStub(): { + addEventListener: ReturnType + removeEventListener: ReturnType + dispatch: (type: string, event: KeyboardEvent) => void +} { + const listeners = new Map>() + return { + addEventListener: vi.fn((type: string, listener: StoredListener) => { + const bucket = listeners.get(type) ?? new Set() + bucket.add(listener) + listeners.set(type, bucket) + }), + removeEventListener: vi.fn((type: string, listener: StoredListener) => { + listeners.get(type)?.delete(listener) + }), + dispatch: (type, event) => { + for (const listener of listeners.get(type) ?? []) { + listener(event) + } + } + } +} + +describe('mac option key state', () => { + afterEach(() => { + vi.unstubAllGlobals() + vi.resetModules() + }) + + it('shares one window listener set across subscribers and only notifies on value changes', async () => { + const windowStub = createWindowStub() + vi.stubGlobal('navigator', { userAgent: 'Macintosh' }) + vi.stubGlobal('window', windowStub) + const { getMacOptionKeySnapshot, subscribeMacOptionKey } = + await import('./mac-option-key-state') + const first = vi.fn() + const second = vi.fn() + + const unsubscribeFirst = subscribeMacOptionKey(first) + const unsubscribeSecond = subscribeMacOptionKey(second) + + expect(windowStub.addEventListener).toHaveBeenCalledTimes(3) + windowStub.dispatch('keydown', { altKey: true } as KeyboardEvent) + expect(getMacOptionKeySnapshot()).toBe(true) + expect(first).toHaveBeenCalledTimes(1) + expect(second).toHaveBeenCalledTimes(1) + + windowStub.dispatch('keydown', { altKey: true } as KeyboardEvent) + expect(first).toHaveBeenCalledTimes(1) + expect(second).toHaveBeenCalledTimes(1) + + unsubscribeFirst() + expect(windowStub.removeEventListener).not.toHaveBeenCalled() + windowStub.dispatch('keyup', { altKey: false } as KeyboardEvent) + expect(first).toHaveBeenCalledTimes(1) + expect(second).toHaveBeenCalledTimes(2) + + unsubscribeSecond() + expect(windowStub.removeEventListener).toHaveBeenCalledTimes(3) + expect(getMacOptionKeySnapshot()).toBe(false) + }) + + it('does not attach keyboard listeners on non-mac platforms', async () => { + const windowStub = createWindowStub() + vi.stubGlobal('navigator', { userAgent: 'Windows' }) + vi.stubGlobal('window', windowStub) + const { getMacOptionKeySnapshot, subscribeMacOptionKey } = + await import('./mac-option-key-state') + + const unsubscribe = subscribeMacOptionKey(vi.fn()) + + expect(windowStub.addEventListener).not.toHaveBeenCalled() + expect(getMacOptionKeySnapshot()).toBe(false) + unsubscribe() + }) +}) diff --git a/src/renderer/src/components/sidebar/mac-option-key-state.ts b/src/renderer/src/components/sidebar/mac-option-key-state.ts new file mode 100644 index 00000000000..10096eec064 --- /dev/null +++ b/src/renderer/src/components/sidebar/mac-option-key-state.ts @@ -0,0 +1,65 @@ +import { useSyncExternalStore } from 'react' + +type OptionKeyListener = () => void + +let optionPressed = false +const listeners = new Set() +let disposeWindowListeners: (() => void) | null = null + +function isMacPlatform(): boolean { + return typeof navigator !== 'undefined' && navigator.userAgent.includes('Mac') +} + +function setOptionPressed(nextPressed: boolean): void { + if (optionPressed === nextPressed) { + return + } + optionPressed = nextPressed + for (const listener of listeners) { + listener() + } +} + +function startWindowListeners(): void { + if (disposeWindowListeners || !isMacPlatform() || typeof window === 'undefined') { + return + } + + const handleKeyChange = (event: KeyboardEvent): void => setOptionPressed(event.altKey) + const handleWindowBlur = (): void => setOptionPressed(false) + window.addEventListener('keydown', handleKeyChange, true) + window.addEventListener('keyup', handleKeyChange, true) + window.addEventListener('blur', handleWindowBlur) + disposeWindowListeners = () => { + window.removeEventListener('keydown', handleKeyChange, true) + window.removeEventListener('keyup', handleKeyChange, true) + window.removeEventListener('blur', handleWindowBlur) + } +} + +export function subscribeMacOptionKey(listener: OptionKeyListener): () => void { + if (!isMacPlatform()) { + return () => undefined + } + listeners.add(listener) + startWindowListeners() + return () => { + listeners.delete(listener) + if (listeners.size > 0) { + return + } + disposeWindowListeners?.() + disposeWindowListeners = null + setOptionPressed(false) + } +} + +export function getMacOptionKeySnapshot(): boolean { + return isMacPlatform() ? optionPressed : false +} + +export function useMacOptionKeyPressed(): boolean { + // Why: the sidebar can render dozens of cards. One shared external store + // avoids a global key listener per card and only re-renders on Option flips. + return useSyncExternalStore(subscribeMacOptionKey, getMacOptionKeySnapshot, () => false) +} diff --git a/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts b/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts index a9827e3d217..bc27faae0ef 100644 --- a/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts +++ b/src/renderer/src/components/sidebar/useWorktreeAgentRows.test.ts @@ -8,6 +8,7 @@ import type { TerminalTab } from '../../../../shared/types' import type { RetainedAgentEntry } from '@/store/slices/agent-status' import { buildWorktreeAgentRows, + selectLiveAgentStatusEntriesForWorktree, selectMigrationUnsupportedEntriesForWorktree } from './useWorktreeAgentRows' import { makePaneKey } from '../../../../shared/stable-pane-id' @@ -140,6 +141,49 @@ describe('selectMigrationUnsupportedEntriesForWorktree', () => { // records preserves element identity for useShallow. expect(first).toEqual([unsupported]) expect(second).toEqual([unsupported]) + expect(first).toBe(second) expect(first[0]).toBe(second[0]) }) }) + +describe('selectLiveAgentStatusEntriesForWorktree', () => { + it('reuses unaffected worktree arrays when another worktree receives a same-state ping', () => { + const wt1Entry = makeEntry(PANE_KEY_1, 1000, { state: 'working', prompt: 'first' }) + const wt2Entry = makeEntry(PANE_KEY_2, 1000, { state: 'working', prompt: 'first' }) + const state = { + tabsByWorktree: { + 'wt-1': [makeTab('tab-1')], + 'wt-2': [makeTab('tab-2')] + }, + agentStatusByPaneKey: { + [PANE_KEY_1]: wt1Entry, + [PANE_KEY_2]: wt2Entry + }, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {} + } + + const firstWt1 = selectLiveAgentStatusEntriesForWorktree(state, 'wt-1') + const firstWt2 = selectLiveAgentStatusEntriesForWorktree(state, 'wt-2') + const nextState = { + ...state, + agentStatusByPaneKey: { + [PANE_KEY_1]: wt1Entry, + [PANE_KEY_2]: { + ...wt2Entry, + prompt: 'updated prompt preview', + updatedAt: 1100 + } + } + } + + const secondWt1 = selectLiveAgentStatusEntriesForWorktree(nextState, 'wt-1') + const secondWt2 = selectLiveAgentStatusEntriesForWorktree(nextState, 'wt-2') + + // Why: WorktreeCard mounts one selector per visible card. A same-state + // hook ping for wt-2 must not make wt-1 pay a fresh array/render cost. + expect(secondWt1).toBe(firstWt1) + expect(secondWt2).not.toBe(firstWt2) + expect(secondWt2[0]?.prompt).toBe('updated prompt preview') + }) +}) diff --git a/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts b/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts index 196e6a116ca..ab3540d856e 100644 --- a/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts +++ b/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts @@ -18,7 +18,6 @@ import { migrationUnsupportedToAgentStatusEntry } from '@/lib/migration-unsuppor // reference when there's nothing for this worktree. Without stable empties, // zustand's shallow equality would see a new `[]` every render and trigger // unnecessary re-renders — defeating the purpose of the narrow selector. -const EMPTY_TABS: TerminalTab[] = [] const EMPTY_LIVE_ENTRIES: AgentStatusEntry[] = [] const EMPTY_MIGRATION_UNSUPPORTED_ENTRIES: MigrationUnsupportedPtyEntry[] = [] const EMPTY_RETAINED: RetainedAgentEntry[] = [] @@ -31,63 +30,188 @@ type WorktreeAgentRowsState = Pick< | 'tabsByWorktree' > -export function selectLiveAgentStatusEntriesForWorktree( - state: WorktreeAgentRowsState, - worktreeId: string -): AgentStatusEntry[] { - const wtTabs = state.tabsByWorktree[worktreeId] ?? EMPTY_TABS - if (wtTabs.length === 0) { - return EMPTY_LIVE_ENTRIES +type TabWorktreeIndexCache = { + tabsByWorktree: WorktreeAgentRowsState['tabsByWorktree'] + tabIdToWorktreeId: Map +} + +type LiveEntriesByWorktreeCache = { + tabsByWorktree: WorktreeAgentRowsState['tabsByWorktree'] + agentStatusByPaneKey: WorktreeAgentRowsState['agentStatusByPaneKey'] + entriesByWorktree: Map +} + +type MigrationUnsupportedByWorktreeCache = { + tabsByWorktree: WorktreeAgentRowsState['tabsByWorktree'] + migrationUnsupportedByPtyId: WorktreeAgentRowsState['migrationUnsupportedByPtyId'] + entriesByWorktree: Map +} + +type RetainedEntriesByWorktreeCache = { + retainedAgentsByPaneKey: WorktreeAgentRowsState['retainedAgentsByPaneKey'] + entriesByWorktree: Map +} + +let tabWorktreeIndexCache: TabWorktreeIndexCache | null = null +let liveEntriesByWorktreeCache: LiveEntriesByWorktreeCache | null = null +let migrationUnsupportedByWorktreeCache: MigrationUnsupportedByWorktreeCache | null = null +let retainedEntriesByWorktreeCache: RetainedEntriesByWorktreeCache | null = null + +function reuseArrayIfEqual(previous: T[] | undefined, next: T[]): T[] { + if (!previous || previous.length !== next.length) { + return next } - const tabIds = new Set(wtTabs.map((t) => t.id)) - const out: AgentStatusEntry[] = [] + for (let i = 0; i < next.length; i += 1) { + if (previous[i] !== next[i]) { + return next + } + } + return previous +} + +function getTabIdToWorktreeId( + tabsByWorktree: WorktreeAgentRowsState['tabsByWorktree'] +): Map { + if (tabWorktreeIndexCache?.tabsByWorktree === tabsByWorktree) { + return tabWorktreeIndexCache.tabIdToWorktreeId + } + const tabIdToWorktreeId = new Map() + for (const [worktreeId, tabs] of Object.entries(tabsByWorktree)) { + for (const tab of tabs) { + tabIdToWorktreeId.set(tab.id, worktreeId) + } + } + tabWorktreeIndexCache = { tabsByWorktree, tabIdToWorktreeId } + return tabIdToWorktreeId +} + +function getLiveEntriesByWorktree(state: WorktreeAgentRowsState): Map { + if ( + liveEntriesByWorktreeCache?.tabsByWorktree === state.tabsByWorktree && + liveEntriesByWorktreeCache.agentStatusByPaneKey === state.agentStatusByPaneKey + ) { + return liveEntriesByWorktreeCache.entriesByWorktree + } + + const tabIdToWorktreeId = getTabIdToWorktreeId(state.tabsByWorktree) + const previous = liveEntriesByWorktreeCache?.entriesByWorktree + const entriesByWorktree = new Map() for (const [paneKey, entry] of Object.entries(state.agentStatusByPaneKey)) { const parsed = parsePaneKey(paneKey) if (!parsed) { continue } - if (!tabIds.has(parsed.tabId)) { + const worktreeId = tabIdToWorktreeId.get(parsed.tabId) + if (!worktreeId) { continue } - out.push(entry) + const bucket = entriesByWorktree.get(worktreeId) + if (bucket) { + bucket.push(entry) + } else { + entriesByWorktree.set(worktreeId, [entry]) + } } - return out.length > 0 ? out : EMPTY_LIVE_ENTRIES + for (const [worktreeId, entries] of entriesByWorktree) { + entriesByWorktree.set(worktreeId, reuseArrayIfEqual(previous?.get(worktreeId), entries)) + } + liveEntriesByWorktreeCache = { + tabsByWorktree: state.tabsByWorktree, + agentStatusByPaneKey: state.agentStatusByPaneKey, + entriesByWorktree + } + return entriesByWorktree +} + +function getMigrationUnsupportedByWorktree( + state: WorktreeAgentRowsState +): Map { + if ( + migrationUnsupportedByWorktreeCache?.tabsByWorktree === state.tabsByWorktree && + migrationUnsupportedByWorktreeCache.migrationUnsupportedByPtyId === + state.migrationUnsupportedByPtyId + ) { + return migrationUnsupportedByWorktreeCache.entriesByWorktree + } + + const tabIdToWorktreeId = getTabIdToWorktreeId(state.tabsByWorktree) + const previous = migrationUnsupportedByWorktreeCache?.entriesByWorktree + const entriesByWorktree = new Map() + for (const unsupported of Object.values(state.migrationUnsupportedByPtyId)) { + if (!unsupported.paneKey) { + continue + } + const parsed = parsePaneKey(unsupported.paneKey) + const worktreeId = parsed ? tabIdToWorktreeId.get(parsed.tabId) : undefined + if (!worktreeId) { + continue + } + const bucket = entriesByWorktree.get(worktreeId) + if (bucket) { + bucket.push(unsupported) + } else { + entriesByWorktree.set(worktreeId, [unsupported]) + } + } + for (const [worktreeId, entries] of entriesByWorktree) { + entriesByWorktree.set(worktreeId, reuseArrayIfEqual(previous?.get(worktreeId), entries)) + } + migrationUnsupportedByWorktreeCache = { + tabsByWorktree: state.tabsByWorktree, + migrationUnsupportedByPtyId: state.migrationUnsupportedByPtyId, + entriesByWorktree + } + return entriesByWorktree +} + +function getRetainedEntriesByWorktree( + state: WorktreeAgentRowsState +): Map { + if (retainedEntriesByWorktreeCache?.retainedAgentsByPaneKey === state.retainedAgentsByPaneKey) { + return retainedEntriesByWorktreeCache.entriesByWorktree + } + + const previous = retainedEntriesByWorktreeCache?.entriesByWorktree + const entriesByWorktree = new Map() + for (const retained of Object.values(state.retainedAgentsByPaneKey)) { + const bucket = entriesByWorktree.get(retained.worktreeId) + if (bucket) { + bucket.push(retained) + } else { + entriesByWorktree.set(retained.worktreeId, [retained]) + } + } + for (const [worktreeId, entries] of entriesByWorktree) { + entriesByWorktree.set(worktreeId, reuseArrayIfEqual(previous?.get(worktreeId), entries)) + } + retainedEntriesByWorktreeCache = { + retainedAgentsByPaneKey: state.retainedAgentsByPaneKey, + entriesByWorktree + } + return entriesByWorktree +} + +export function selectLiveAgentStatusEntriesForWorktree( + state: WorktreeAgentRowsState, + worktreeId: string +): AgentStatusEntry[] { + return getLiveEntriesByWorktree(state).get(worktreeId) ?? EMPTY_LIVE_ENTRIES } export function selectMigrationUnsupportedEntriesForWorktree( state: WorktreeAgentRowsState, worktreeId: string ): MigrationUnsupportedPtyEntry[] { - const wtTabs = state.tabsByWorktree[worktreeId] ?? EMPTY_TABS - if (wtTabs.length === 0) { - return EMPTY_MIGRATION_UNSUPPORTED_ENTRIES - } - const tabIds = new Set(wtTabs.map((t) => t.id)) - const out: MigrationUnsupportedPtyEntry[] = [] - for (const unsupported of Object.values(state.migrationUnsupportedByPtyId)) { - if (!unsupported.paneKey) { - continue - } - const parsed = parsePaneKey(unsupported.paneKey) - if (!parsed || !tabIds.has(parsed.tabId)) { - continue - } - out.push(unsupported) - } - return out.length > 0 ? out : EMPTY_MIGRATION_UNSUPPORTED_ENTRIES + return ( + getMigrationUnsupportedByWorktree(state).get(worktreeId) ?? EMPTY_MIGRATION_UNSUPPORTED_ENTRIES + ) } export function selectRetainedAgentEntriesForWorktree( state: WorktreeAgentRowsState, worktreeId: string ): RetainedAgentEntry[] { - const out: RetainedAgentEntry[] = [] - for (const ra of Object.values(state.retainedAgentsByPaneKey)) { - if (ra.worktreeId === worktreeId) { - out.push(ra) - } - } - return out.length > 0 ? out : EMPTY_RETAINED + return getRetainedEntriesByWorktree(state).get(worktreeId) ?? EMPTY_RETAINED } export function buildWorktreeAgentRows(args: { @@ -155,10 +279,10 @@ export function buildWorktreeAgentRows(args: { * list. Produces live hook-reported agents plus retained "done" snapshots, * stale-decayed to 'idle' when the hook stream has gone quiet. * - * Uses per-worktree selectors rather than reusing useDashboardData's - * cross-worktree aggregate — that pipeline is O(repos × worktrees × agents) - * and would recompute once per sidebar card on every agent-status event. - * Scoped selectors keep the cost O(this-worktree-entries) per card. + * Uses indexed per-worktree selectors rather than reusing useDashboardData's + * cross-worktree aggregate. The index is rebuilt once per relevant immutable + * store slice and then shared by every visible card, avoiding O(cards × agents) + * selector work on high-frequency agent status pings. */ export function useWorktreeAgentRows(worktreeId: string): DashboardAgentRow[] { const tabs = useAppStore((s) => s.tabsByWorktree[worktreeId]) diff --git a/src/renderer/src/components/sidebar/visible-worktree-activity-inputs.test.ts b/src/renderer/src/components/sidebar/visible-worktree-activity-inputs.test.ts new file mode 100644 index 00000000000..c9854c9eed6 --- /dev/null +++ b/src/renderer/src/components/sidebar/visible-worktree-activity-inputs.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest' +import type { BrowserWorkspace, TerminalTab } from '../../../../shared/types' +import { + getVisibleWorktreeBrowserActivityTabs, + getVisibleWorktreeTerminalActivityTabs +} from './visible-worktree-activity-inputs' + +function terminalTab(id: string, title: string): TerminalTab { + return { + id, + ptyId: id, + worktreeId: 'wt-1', + title, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} + +function browserTab(id: string, title: string): BrowserWorkspace { + return { + id, + worktreeId: 'wt-1', + activePageId: `${id}-page`, + pageIds: [`${id}-page`], + url: 'https://example.com', + title, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 0 + } +} + +describe('visible worktree activity inputs', () => { + it('preserves terminal activity projection when only tab metadata changes', () => { + const first = getVisibleWorktreeTerminalActivityTabs({ + 'wt-1': [terminalTab('tab-1', 'First')] + }) + + const second = getVisibleWorktreeTerminalActivityTabs({ + 'wt-1': [terminalTab('tab-1', 'Renamed')] + }) + + expect(second).toBe(first) + expect(second['wt-1']).toBe(first['wt-1']) + }) + + it('updates terminal activity projection when tab ids change', () => { + const first = getVisibleWorktreeTerminalActivityTabs({ + 'wt-1': [terminalTab('tab-1', 'First')] + }) + + const second = getVisibleWorktreeTerminalActivityTabs({ + 'wt-1': [terminalTab('tab-1', 'First'), terminalTab('tab-2', 'Second')] + }) + + expect(second).not.toBe(first) + expect(second['wt-1']?.map((tab) => tab.id)).toEqual(['tab-1', 'tab-2']) + }) + + it('preserves browser activity projection when only browser metadata changes', () => { + const first = getVisibleWorktreeBrowserActivityTabs({ + 'wt-1': [browserTab('browser-1', 'First')] + }) + + const second = getVisibleWorktreeBrowserActivityTabs({ + 'wt-1': [browserTab('browser-1', 'Renamed')] + }) + + expect(second).toBe(first) + expect(second['wt-1']).toBe(first['wt-1']) + }) + + it('updates browser activity projection when browser ids change', () => { + const first = getVisibleWorktreeBrowserActivityTabs({ + 'wt-1': [browserTab('browser-1', 'First')] + }) + + const second = getVisibleWorktreeBrowserActivityTabs({ + 'wt-1': [browserTab('browser-2', 'Second')] + }) + + expect(second).not.toBe(first) + expect(second['wt-1']?.map((tab) => tab.id)).toEqual(['browser-2']) + }) +}) diff --git a/src/renderer/src/components/sidebar/visible-worktree-activity-inputs.ts b/src/renderer/src/components/sidebar/visible-worktree-activity-inputs.ts new file mode 100644 index 00000000000..d3ca4416568 --- /dev/null +++ b/src/renderer/src/components/sidebar/visible-worktree-activity-inputs.ts @@ -0,0 +1,77 @@ +import type { BrowserWorkspace, TerminalTab } from '../../../../shared/types' + +type TerminalActivityTab = Pick +type BrowserActivityTab = Pick + +function haveSameIds( + previous: readonly T[] | undefined, + next: readonly { id: string }[] +): boolean { + if (!previous || previous.length !== next.length) { + return false + } + for (let index = 0; index < next.length; index++) { + if (previous[index]?.id !== next[index]?.id) { + return false + } + } + return true +} + +function projectIdTabs( + tabsByWorktree: Record, + previousProjection: Record | null +): { projection: Record; unchanged: boolean } { + const nextProjection: Record = {} + let unchanged = + previousProjection !== null && + Object.keys(previousProjection).length === Object.keys(tabsByWorktree).length + + for (const [worktreeId, tabs] of Object.entries(tabsByWorktree)) { + const previousTabs = previousProjection?.[worktreeId] + if (haveSameIds(previousTabs, tabs)) { + nextProjection[worktreeId] = previousTabs as U[] + continue + } + unchanged = false + nextProjection[worktreeId] = tabs.map((tab) => ({ id: tab.id }) as U) + } + + return { projection: nextProjection, unchanged } +} + +let cachedTerminalSource: Record | null = null +let cachedTerminalProjection: Record | null = null + +export function getVisibleWorktreeTerminalActivityTabs( + tabsByWorktree: Record +): Record { + if (cachedTerminalSource === tabsByWorktree && cachedTerminalProjection) { + return cachedTerminalProjection + } + const { projection, unchanged } = projectIdTabs(tabsByWorktree, cachedTerminalProjection) + cachedTerminalSource = tabsByWorktree + if (unchanged && cachedTerminalProjection) { + return cachedTerminalProjection + } + cachedTerminalProjection = projection + return projection +} + +let cachedBrowserSource: Record | null = null +let cachedBrowserProjection: Record | null = null + +export function getVisibleWorktreeBrowserActivityTabs( + browserTabsByWorktree: Record +): Record { + if (cachedBrowserSource === browserTabsByWorktree && cachedBrowserProjection) { + return cachedBrowserProjection + } + const { projection, unchanged } = projectIdTabs(browserTabsByWorktree, cachedBrowserProjection) + cachedBrowserSource = browserTabsByWorktree + if (unchanged && cachedBrowserProjection) { + return cachedBrowserProjection + } + cachedBrowserProjection = projection + return projection +} diff --git a/src/renderer/src/components/sidebar/visible-worktrees.ts b/src/renderer/src/components/sidebar/visible-worktrees.ts index bf333af5486..a4ce1d05549 100644 --- a/src/renderer/src/components/sidebar/visible-worktrees.ts +++ b/src/renderer/src/components/sidebar/visible-worktrees.ts @@ -84,7 +84,7 @@ export function computeVisibleWorktreeIds( opts: { filterRepoIds: string[] showSleepingWorkspaces: boolean - tabsByWorktree: Record | null + tabsByWorktree: Record[]> | null ptyIdsByTabId: Record | null browserTabsByWorktree?: Record | null // Why required: every caller (WorktreeList, getVisibleWorktreeIds diff --git a/src/renderer/src/components/sidebar/worktree-agent-activity-summary.test.ts b/src/renderer/src/components/sidebar/worktree-agent-activity-summary.test.ts index 75234b739ce..42b6ac672c1 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-activity-summary.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-activity-summary.test.ts @@ -52,4 +52,80 @@ describe('selectWorktreeAgentActivitySummary', () => { }) expect(nowSpy).toHaveBeenCalledTimes(1) }) + + it('reuses the cached summary when same-state agent pings only clone the status map', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(2_000) + const paneKey = makePaneKey('tab-1', LEAF_ID) + const tabsByWorktree = { + 'repo::/wt-1': [{ id: 'tab-1' }] + } + const migrationUnsupportedByPtyId = {} + const retainedAgentsByPaneKey = {} + const entry = makeAgentStatusEntry({ paneKey, state: 'working' }) + const state = { + tabsByWorktree, + agentStatusEpoch: 0, + agentStatusByPaneKey: { + [paneKey]: entry + }, + migrationUnsupportedByPtyId, + retainedAgentsByPaneKey + } + const sameStatePing = { + ...state, + agentStatusByPaneKey: { + [paneKey]: { + ...entry, + prompt: 'new prompt preview', + updatedAt: 1_500 + } + } + } + + expect(selectWorktreeAgentActivitySummary(state as never, 'repo::/wt-1')).toMatchObject({ + hasLiveWorking: true + }) + expect(selectWorktreeAgentActivitySummary(sameStatePing as never, 'repo::/wt-1')).toMatchObject( + { + hasLiveWorking: true + } + ) + expect(nowSpy).toHaveBeenCalledTimes(1) + }) + + it('rebuilds the summary when the agent status epoch changes', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(2_000) + const paneKey = makePaneKey('tab-1', LEAF_ID) + const tabsByWorktree = { + 'repo::/wt-1': [{ id: 'tab-1' }] + } + const migrationUnsupportedByPtyId = {} + const retainedAgentsByPaneKey = {} + const state = { + tabsByWorktree, + agentStatusEpoch: 0, + agentStatusByPaneKey: { + [paneKey]: makeAgentStatusEntry({ paneKey, state: 'working' }) + }, + migrationUnsupportedByPtyId, + retainedAgentsByPaneKey + } + const changedState = { + ...state, + agentStatusEpoch: 1, + agentStatusByPaneKey: { + [paneKey]: makeAgentStatusEntry({ paneKey, state: 'done' }) + } + } + + expect(selectWorktreeAgentActivitySummary(state as never, 'repo::/wt-1')).toMatchObject({ + hasLiveWorking: true, + hasLiveDone: false + }) + expect(selectWorktreeAgentActivitySummary(changedState as never, 'repo::/wt-1')).toMatchObject({ + hasLiveWorking: false, + hasLiveDone: true + }) + expect(nowSpy).toHaveBeenCalledTimes(2) + }) }) diff --git a/src/renderer/src/components/sidebar/worktree-agent-activity-summary.ts b/src/renderer/src/components/sidebar/worktree-agent-activity-summary.ts index 52dab2e125e..f390f480a8a 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-activity-summary.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-activity-summary.ts @@ -33,7 +33,6 @@ type AgentActivityInput = Pick< type AgentActivityCache = { tabsByWorktree: AppState['tabsByWorktree'] agentStatusEpoch: number - agentStatusByPaneKey: AppState['agentStatusByPaneKey'] migrationUnsupportedByPtyId: AppState['migrationUnsupportedByPtyId'] retainedAgentsByPaneKey: AppState['retainedAgentsByPaneKey'] summaries: Map @@ -55,7 +54,6 @@ function getWorktreeAgentActivitySummaries( agentActivityCache && agentActivityCache.tabsByWorktree === state.tabsByWorktree && agentActivityCache.agentStatusEpoch === state.agentStatusEpoch && - agentActivityCache.agentStatusByPaneKey === state.agentStatusByPaneKey && agentActivityCache.migrationUnsupportedByPtyId === state.migrationUnsupportedByPtyId && agentActivityCache.retainedAgentsByPaneKey === state.retainedAgentsByPaneKey ) { @@ -106,7 +104,6 @@ function getWorktreeAgentActivitySummaries( agentActivityCache = { tabsByWorktree: state.tabsByWorktree, agentStatusEpoch: state.agentStatusEpoch, - agentStatusByPaneKey: state.agentStatusByPaneKey, migrationUnsupportedByPtyId: state.migrationUnsupportedByPtyId, retainedAgentsByPaneKey: state.retainedAgentsByPaneKey, summaries diff --git a/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts b/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts index d9a778c610f..0655b524b83 100644 --- a/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-scroll-adjustment.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { + countRecordKeysByReference, resolvePendingSidebarReveal, shouldAdjustWorktreeSidebarMeasuredRowScroll } from './WorktreeList' @@ -15,6 +16,21 @@ const makeHeaderRow = (key: string) => }) as const describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => { + it('counts record keys once per object reference', () => { + const keysSpy = vi.spyOn(Object, 'keys') + const first = { a: 1, b: 2 } + const second = { ...first, c: 3 } + + try { + expect(countRecordKeysByReference(first)).toBe(2) + expect(countRecordKeysByReference(first)).toBe(2) + expect(countRecordKeysByReference(second)).toBe(3) + expect(keysSpy).toHaveBeenCalledTimes(2) + } finally { + keysSpy.mockRestore() + } + }) + it('suppresses measured-row scroll correction while TanStack is scrolling', () => { expect( shouldAdjustWorktreeSidebarMeasuredRowScroll({ diff --git a/src/renderer/src/components/sidebar/worktree-list-visible-refresh.test.ts b/src/renderer/src/components/sidebar/worktree-list-visible-refresh.test.ts new file mode 100644 index 00000000000..87b5ec85c1d --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list-visible-refresh.test.ts @@ -0,0 +1,43 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { installWorktreeVisibleRefreshVisibilityListener } from './WorktreeList' + +describe('installWorktreeVisibleRefreshVisibilityListener', () => { + beforeEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() + }) + + it('subscribes to document visibility changes so visible PR refresh can rerun on return', () => { + const listeners = new Map void>() + const onChange = vi.fn() + const removeEventListener = vi.fn() + + vi.stubGlobal('document', { + addEventListener: vi.fn((event: string, listener: () => void) => { + listeners.set(event, listener) + }), + removeEventListener + }) + + const cleanup = installWorktreeVisibleRefreshVisibilityListener(onChange) + + listeners.get('visibilitychange')?.() + expect(onChange).toHaveBeenCalledTimes(1) + + cleanup() + expect(removeEventListener).toHaveBeenCalledWith('visibilitychange', onChange) + }) + + it('keeps the visibility listener wired into the visible PR refresh effect', () => { + const source = readFileSync( + fileURLToPath(new URL('./WorktreeList.tsx', import.meta.url)), + 'utf-8' + ) + + expect(source).toContain('installWorktreeVisibleRefreshVisibilityListener(() => {') + expect(source).toContain('setDocumentVisibilityRevision((revision) => revision + 1)') + expect(source).toMatch(/documentVisibilityRevision,\n\s+groupBy,/) + }) +}) diff --git a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx index 36308d1632a..6501e187e2a 100644 --- a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx @@ -31,8 +31,9 @@ import { import { cn } from '@/lib/utils' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' +import { installFocusedVisibilityInterval } from '@/lib/focused-visibility-interval' import { useAppStore } from '../../store' -import { useAllWorktrees, useWorktreeMap } from '../../store/selectors' +import { useWorktreeMap } from '../../store/selectors' import { runWorktreeDelete } from '../sidebar/delete-worktree-flow' import { runSleepWorktree } from '../sidebar/sleep-worktree-flow' import { useDaemonActions, DaemonActionDialog } from '../shared/useDaemonActions' @@ -55,6 +56,12 @@ import { isResourceSessionActivationKey, navigateResourceSessionToTab } from './resource-session-navigation' +import { + getResourceUsageAllWorktrees, + getResourceUsageRepos, + getResourceUsageRuntimePaneTitlesByTabId, + getResourceUsageTabsByWorktree +} from './resource-usage-open-slices' const POLL_MS = 2_000 const SESSIONS_POLL_MS = 10_000 @@ -659,16 +666,12 @@ export function ResourceUsageStatusSegment({ const fetchSnapshot = useAppStore((s) => s.fetchMemorySnapshot) const workspaceSessionReady = useAppStore((s) => s.workspaceSessionReady) const ptyIdsByTabId = useAppStore((s) => s.ptyIdsByTabId) - const tabsByWorktree = useAppStore((s) => s.tabsByWorktree) - const runtimePaneTitlesByTabId = useAppStore((s) => s.runtimePaneTitlesByTabId) const setActiveView = useAppStore((s) => s.setActiveView) const openModal = useAppStore((s) => s.openModal) const openSpacePage = useAppStore((s) => s.openSpacePage) const activeView = useAppStore((s) => s.activeView) const workspaceSpaceScannedAt = useAppStore((s) => s.workspaceSpaceAnalysis?.scannedAt ?? null) const workspaceSpaceScanning = useAppStore((s) => s.workspaceSpaceScanning) - const repos = useAppStore((s) => s.repos) - const allWorktrees = useAllWorktrees() const activeRuntimeEnvironmentId = useAppStore( (s) => s.settings?.activeRuntimeEnvironmentId ?? null ) @@ -684,6 +687,19 @@ export function ResourceUsageStatusSegment({ const [killConfirm, setKillConfirm] = useState(null) const [killing, setKilling] = useState(false) const [spaceScanReady, setSpaceScanReady] = useState(false) + // Why: tab titles can update on terminal keystrokes. The resource popover's + // merged tree needs them only while open, so closed status-bar badges should + // not subscribe to those high-churn maps. + const runtimePaneTitlesByTabId = useAppStore((s) => + getResourceUsageRuntimePaneTitlesByTabId(s, open, runtimeEnvironmentActive) + ) + const repos = useAppStore((s) => getResourceUsageRepos(s, open, runtimeEnvironmentActive)) + const allWorktrees = useAppStore((s) => + getResourceUsageAllWorktrees(s, open, runtimeEnvironmentActive) + ) + const tabsByWorktree = useAppStore((s) => + getResourceUsageTabsByWorktree(s, open, runtimeEnvironmentActive) + ) const previousSpaceScanningRef = useRef(workspaceSpaceScanning) const lastSeenSpaceScanAtRef = useRef(workspaceSpaceScannedAt) // Why: this segment only understands the local Electron PTY/resource daemon. @@ -783,23 +799,13 @@ export function ResourceUsageStatusSegment({ setSessionsError(false) return } - const refreshIfVisible = (): void => { - if (document.visibilityState === 'visible' && document.hasFocus()) { - void refreshSessions() - } - } - void refreshSessions() // Why: the closed-popover badge is informational. Polling daemon sessions // while the whole window is hidden keeps IPC and daemon list calls hot for - // no visible UI; focus/visibility refreshes catch the badge up immediately. - const interval = setInterval(refreshIfVisible, SESSIONS_POLL_MS) - window.addEventListener('focus', refreshIfVisible) - document.addEventListener('visibilitychange', refreshIfVisible) - return () => { - clearInterval(interval) - window.removeEventListener('focus', refreshIfVisible) - document.removeEventListener('visibilitychange', refreshIfVisible) - } + // no visible UI; visibility refreshes catch the badge up immediately. + return installFocusedVisibilityInterval({ + run: () => void refreshSessions(), + intervalMs: SESSIONS_POLL_MS + }) }, [runtimeEnvironmentActive, refreshSessions]) const repoDisplayNameById = useMemo(() => { diff --git a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx index adf5a28f2e8..d7dd385281f 100644 --- a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx +++ b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx @@ -1063,7 +1063,6 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { const worktreeMap = useAppStore((state) => getWorktreeMapFromState(state)) const tabsByWorktree = useAppStore((state) => state.tabsByWorktree) const ptyIdsByTabId = useAppStore((state) => state.ptyIdsByTabId) - const agentStatusByPaneKey = useAppStore((state) => state.agentStatusByPaneKey) const migrationUnsupportedByPtyId = useAppStore((state) => state.migrationUnsupportedByPtyId) const runtimePaneTitlesByTabId = useAppStore((state) => state.runtimePaneTitlesByTabId) const agentStatusEpoch = useAppStore((state) => state.agentStatusEpoch) @@ -1108,7 +1107,10 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { const decisionDetailsByWorktreeId = useMemo(() => { // Why: active-agent freshness is time-based. The epoch bumps when fresh // hook entries cross the stale boundary so delete readiness recomputes. + // Same-state prompt/tool pings clone the live status map but cannot change + // whether a workspace is safe to delete, so read the map only on epoch. void agentStatusEpoch + const agentStatusByPaneKey = useAppStore.getState().agentStatusByPaneKey const details = new Map() const now = Date.now() for (const worktree of sourceRows) { @@ -1140,7 +1142,6 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { }, [ activeWorktreeId, agentStatusEpoch, - agentStatusByPaneKey, browserTabsByWorktree, editorDrafts, gitStatusByWorktree, @@ -1698,7 +1699,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { worktreeMap, tabsByWorktree, ptyIdsByTabId, - agentStatusByPaneKey, + agentStatusByPaneKey: useAppStore.getState().agentStatusByPaneKey, migrationUnsupportedByPtyId, runtimePaneTitlesByTabId, retainedAgentsByPaneKey, diff --git a/src/renderer/src/components/status-bar/resource-usage-open-slices.test.ts b/src/renderer/src/components/status-bar/resource-usage-open-slices.test.ts new file mode 100644 index 00000000000..b727dc48a03 --- /dev/null +++ b/src/renderer/src/components/status-bar/resource-usage-open-slices.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' +import { + getResourceUsageAllWorktrees, + getResourceUsageRepos, + getResourceUsageRuntimePaneTitlesByTabId, + getResourceUsageTabsByWorktree +} from './resource-usage-open-slices' +import type { AppState } from '../../store' + +describe('resource usage open slices', () => { + it('returns stable empty slices while the popover is closed', () => { + const tabsByWorktree = { 'wt-1': [{ id: 'tab-1' }] } as unknown as AppState['tabsByWorktree'] + const runtimePaneTitlesByTabId = { + 'tab-1': { 'tab-1:0': 'Working' } + } as AppState['runtimePaneTitlesByTabId'] + + const closedTabs = getResourceUsageTabsByWorktree({ tabsByWorktree }, false) + const closedTitles = getResourceUsageRuntimePaneTitlesByTabId( + { runtimePaneTitlesByTabId }, + false + ) + + expect(closedTabs).toBe(getResourceUsageTabsByWorktree({ tabsByWorktree: {} }, false)) + expect(closedTitles).toBe( + getResourceUsageRuntimePaneTitlesByTabId({ runtimePaneTitlesByTabId: {} }, false) + ) + expect(closedTabs).toEqual({}) + expect(closedTitles).toEqual({}) + }) + + it('returns live slices while the popover is open', () => { + const tabsByWorktree = { 'wt-1': [{ id: 'tab-1' }] } as unknown as AppState['tabsByWorktree'] + const runtimePaneTitlesByTabId = { + 'tab-1': { 'tab-1:0': 'Working' } + } as AppState['runtimePaneTitlesByTabId'] + + expect(getResourceUsageTabsByWorktree({ tabsByWorktree }, true)).toBe(tabsByWorktree) + expect(getResourceUsageRuntimePaneTitlesByTabId({ runtimePaneTitlesByTabId }, true)).toBe( + runtimePaneTitlesByTabId + ) + }) + + it('gates repo and worktree slices while closed or runtime-backed', () => { + const repos = [{ id: 'repo-1' }] as AppState['repos'] + const worktree = { id: 'wt-1', repoId: 'repo-1' } + const worktreesByRepo = { + 'repo-1': [worktree] + } as unknown as AppState['worktreesByRepo'] + + expect(getResourceUsageRepos({ repos }, false, false)).toBe( + getResourceUsageRepos({ repos: [] }, false, false) + ) + expect(getResourceUsageAllWorktrees({ worktreesByRepo }, false, false)).toBe( + getResourceUsageAllWorktrees({ worktreesByRepo: {} }, false, false) + ) + expect(getResourceUsageRepos({ repos }, true, true)).toEqual([]) + expect(getResourceUsageAllWorktrees({ worktreesByRepo }, true, true)).toEqual([]) + expect(getResourceUsageRepos({ repos }, true, false)).toBe(repos) + expect(getResourceUsageAllWorktrees({ worktreesByRepo }, true, false)).toEqual([worktree]) + }) +}) diff --git a/src/renderer/src/components/status-bar/resource-usage-open-slices.ts b/src/renderer/src/components/status-bar/resource-usage-open-slices.ts new file mode 100644 index 00000000000..1a2b8cefcf5 --- /dev/null +++ b/src/renderer/src/components/status-bar/resource-usage-open-slices.ts @@ -0,0 +1,49 @@ +import type { AppState } from '../../store' +import { getAllWorktreesFromState } from '../../store/selectors' + +const EMPTY_TABS_BY_WORKTREE: AppState['tabsByWorktree'] = {} +const EMPTY_RUNTIME_PANE_TITLES_BY_TAB_ID: AppState['runtimePaneTitlesByTabId'] = {} +const EMPTY_REPOS: AppState['repos'] = [] +const EMPTY_WORKTREES: ReturnType = [] + +function shouldReadPopoverSlices(open: boolean, runtimeEnvironmentActive: boolean): boolean { + return open && !runtimeEnvironmentActive +} + +export function getResourceUsageTabsByWorktree( + state: Pick, + open: boolean, + runtimeEnvironmentActive = false +): AppState['tabsByWorktree'] { + return shouldReadPopoverSlices(open, runtimeEnvironmentActive) + ? state.tabsByWorktree + : EMPTY_TABS_BY_WORKTREE +} + +export function getResourceUsageRuntimePaneTitlesByTabId( + state: Pick, + open: boolean, + runtimeEnvironmentActive = false +): AppState['runtimePaneTitlesByTabId'] { + return shouldReadPopoverSlices(open, runtimeEnvironmentActive) + ? state.runtimePaneTitlesByTabId + : EMPTY_RUNTIME_PANE_TITLES_BY_TAB_ID +} + +export function getResourceUsageRepos( + state: Pick, + open: boolean, + runtimeEnvironmentActive: boolean +): AppState['repos'] { + return shouldReadPopoverSlices(open, runtimeEnvironmentActive) ? state.repos : EMPTY_REPOS +} + +export function getResourceUsageAllWorktrees( + state: Pick, + open: boolean, + runtimeEnvironmentActive: boolean +): ReturnType { + return shouldReadPopoverSlices(open, runtimeEnvironmentActive) + ? getAllWorktreesFromState(state) + : EMPTY_WORKTREES +} diff --git a/src/renderer/src/components/tab-bar/TabBar.tsx b/src/renderer/src/components/tab-bar/TabBar.tsx index d3685179030..833e3b111b0 100644 --- a/src/renderer/src/components/tab-bar/TabBar.tsx +++ b/src/renderer/src/components/tab-bar/TabBar.tsx @@ -41,6 +41,8 @@ const isWindows = navigator.userAgent.includes('Windows') const NEW_TERMINAL_SHORTCUT = isMac ? '⌘T' : 'Ctrl+T' const NEW_BROWSER_SHORTCUT = isMac ? '⌘⇧B' : 'Ctrl+Shift+B' const NEW_FILE_SHORTCUT = isMac ? '⌘⇧M' : 'Ctrl+Shift+M' +type GitStatusEntries = ReturnType['gitStatusByWorktree'][string] +const EMPTY_GIT_STATUS_ENTRIES: GitStatusEntries = [] type TabBarProps = { tabs: (TerminalTab & { unifiedTabId?: string })[] @@ -144,7 +146,9 @@ function TabBarInner({ onCreateSplitGroup, hoveredTabInsertion }: TabBarProps): React.JSX.Element { - const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree) + const gitStatusEntries = useAppStore( + (s) => s.gitStatusByWorktree[worktreeId] ?? EMPTY_GIT_STATUS_ENTRIES + ) const defaultWindowsShell = useAppStore( (s) => s.settings?.terminalWindowsShell ?? 'powershell.exe' ) @@ -154,10 +158,7 @@ function TabBarInner({ const windowsTerminalCapabilities = useWindowsTerminalCapabilities(isWindows) const resolvedGroupId = groupId ?? worktreeId - const statusByRelativePath = useMemo( - () => buildStatusMap(gitStatusByWorktree[worktreeId] ?? []), - [worktreeId, gitStatusByWorktree] - ) + const statusByRelativePath = useMemo(() => buildStatusMap(gitStatusEntries), [gitStatusEntries]) // Why: Electron elements run in a separate process, so clicking // inside one never dispatches a pointerdown on the renderer document. 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 37fe6828f37..542174343a9 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -1251,6 +1251,90 @@ describe('connectPanePty', () => { expect(transport.sendInput).toHaveBeenCalledWith('a') }) + it('does not enumerate every worktree tab for ordinary input without Codex restart notices', async () => { + const { connectPanePty } = await import('./pty-connection') + + const transport = createMockTransport('pty-live') + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: new Proxy( + { + 'wt-1': [{ id: 'tab-1', ptyId: 'pty-live' }], + 'wt-2': [{ id: 'tab-2', ptyId: 'pty-other' }] + }, + { + ownKeys() { + throw new Error('tabsByWorktree should not be enumerated') + } + } + ), + codexRestartNoticeByPtyId: {} + } + + const pane = createPane(1) + let onDataHandler: ((data: string) => void) | null = null + pane.terminal.onData = vi.fn(((handler: (data: string) => void) => { + onDataHandler = handler + return { dispose: vi.fn() } + }) as typeof pane.terminal.onData) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + expect(onDataHandler).toBeDefined() + if (!onDataHandler) { + throw new Error('expected onData handler to be registered') + } + ;(onDataHandler as (data: string) => void)('a') + + expect(transport.sendInput).toHaveBeenCalledWith('a') + }) + + it('uses the current worktree tab for Codex stale fallback without enumerating all worktrees', async () => { + const { connectPanePty } = await import('./pty-connection') + + const transport = createMockTransport(null) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: new Proxy( + { + 'wt-1': [{ id: 'tab-1', ptyId: 'pty-live' }], + 'wt-2': [{ id: 'tab-2', ptyId: 'pty-other' }] + }, + { + ownKeys() { + throw new Error('tabsByWorktree should not be enumerated') + } + } + ), + codexRestartNoticeByPtyId: { + 'pty-other': { previousAccountLabel: 'A', nextAccountLabel: 'B' } + } + } + + const pane = createPane(1) + let onDataHandler: ((data: string) => void) | null = null + pane.terminal.onData = vi.fn(((handler: (data: string) => void) => { + onDataHandler = handler + return { dispose: vi.fn() } + }) as typeof pane.terminal.onData) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + + expect(onDataHandler).toBeDefined() + if (!onDataHandler) { + throw new Error('expected onData handler to be registered') + } + ;(onDataHandler as (data: string) => void)('a') + + expect(transport.sendInput).toHaveBeenCalledWith('a') + }) + it('blocks input to stale Codex panes until they restart', async () => { const { connectPanePty } = await import('./pty-connection') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 952c1c2eade..36cfbb94a59 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -53,6 +53,11 @@ const PTY_CONNECT_DIAG_LIMIT = 200 const AGENT_TASK_COMPLETE_NOTIFICATION_GRACE_MS = 250 const AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS = 1000 const AGENT_TASK_COMPLETE_NOTIFICATION_DETAIL_MAX_AGE_MS = 10_000 +let codexRestartNoticePresenceSource: Record< + string, + { previousAccountLabel: string; nextAccountLabel: string } +> | null = null +let codexRestartNoticePresence = false function isAgentTaskCompleteNotificationEnabled(): boolean { const notifications = useAppStore.getState().settings?.notifications @@ -97,6 +102,16 @@ function isRemoteRuntimePtyId(ptyId: string | null | undefined): boolean { return typeof ptyId === 'string' && ptyId.startsWith(REMOTE_PTY_ID_PREFIX) } +function hasCodexRestartNotices( + noticesByPtyId: Record +): boolean { + if (codexRestartNoticePresenceSource !== noticesByPtyId) { + codexRestartNoticePresenceSource = noticesByPtyId + codexRestartNoticePresence = Object.keys(noticesByPtyId).length > 0 + } + return codexRestartNoticePresence +} + function sshPromptConnectOutcomeForStatus( status: string | undefined, sawNonDisconnected: boolean @@ -145,15 +160,21 @@ async function waitForSshConnection(connectionId: string): Promise entry.id === args.tabId) + const tab = (state.tabsByWorktree[args.worktreeId] ?? []).find((entry) => entry.id === args.tabId) if (tab?.ptyId && codexRestartNoticeByPtyId[tab.ptyId]) { return true } @@ -685,8 +706,10 @@ export function connectPanePty( AGENT_TASK_COMPLETE_NOTIFICATION_MAX_WAIT_MS ) } - agentTaskCompleteSettingsUnsubscribe = useAppStore.subscribe(() => { - syncAgentTaskCompleteNotificationEnabled() + agentTaskCompleteSettingsUnsubscribe = useAppStore.subscribe((state, previousState) => { + if (state.settings?.notifications !== previousState?.settings?.notifications) { + syncAgentTaskCompleteNotificationEnabled() + } }) // ─── Agent task-complete: OS notification, not tab attention ────────── @@ -838,7 +861,13 @@ export function connectPanePty( // still says the pane is stale. Fall back to the tab's persisted PTY ID so // the block still holds during reconnect races before the live transport has // updated its local PTY binding. - if (isCodexPaneStale({ tabId: deps.tabId, panePtyId: currentPtyId })) { + if ( + isCodexPaneStale({ + tabId: deps.tabId, + worktreeId: deps.worktreeId, + panePtyId: currentPtyId + }) + ) { clearPendingTerminalInputIntent() return } diff --git a/src/renderer/src/components/terminal/active-worktree-open-files.test.ts b/src/renderer/src/components/terminal/active-worktree-open-files.test.ts new file mode 100644 index 00000000000..3cb0486a77c --- /dev/null +++ b/src/renderer/src/components/terminal/active-worktree-open-files.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import type { OpenFile } from '@/store/slices/editor' +import { getActiveWorktreeOpenFiles } from './active-worktree-open-files' + +const file = (id: string, worktreeId: string): OpenFile => + ({ + id, + filePath: `/tmp/${id}.md`, + relativePath: `${id}.md`, + worktreeId, + language: 'markdown', + isDirty: false, + runtimeEnvironmentId: null + }) as OpenFile + +describe('getActiveWorktreeOpenFiles', () => { + it('preserves the active slice when unrelated worktree files change', () => { + const active = file('active', 'wt-active') + const first = getActiveWorktreeOpenFiles([active, file('other-a', 'wt-other')], 'wt-active') + const second = getActiveWorktreeOpenFiles([active, file('other-b', 'wt-other')], 'wt-active') + + expect(second).toBe(first) + expect(second).toEqual([active]) + }) + + it('returns a stable empty slice without an active worktree', () => { + const first = getActiveWorktreeOpenFiles([file('active', 'wt-active')], null) + const second = getActiveWorktreeOpenFiles([file('other', 'wt-other')], null) + + expect(second).toBe(first) + expect(second).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/terminal/active-worktree-open-files.ts b/src/renderer/src/components/terminal/active-worktree-open-files.ts new file mode 100644 index 00000000000..48f5342b50a --- /dev/null +++ b/src/renderer/src/components/terminal/active-worktree-open-files.ts @@ -0,0 +1,35 @@ +import type { OpenFile } from '@/store/slices/editor' + +const EMPTY_OPEN_FILES: OpenFile[] = [] + +let cachedOpenFiles: OpenFile[] | null = null +let cachedWorktreeId: string | null = null +let cachedFiles: OpenFile[] = EMPTY_OPEN_FILES + +export function getActiveWorktreeOpenFiles( + openFiles: OpenFile[], + activeWorktreeId: string | null +): OpenFile[] { + if (!activeWorktreeId) { + return EMPTY_OPEN_FILES + } + if (openFiles === cachedOpenFiles && activeWorktreeId === cachedWorktreeId) { + return cachedFiles + } + + const nextFiles = openFiles.filter((file) => file.worktreeId === activeWorktreeId) + if ( + cachedOpenFiles !== null && + activeWorktreeId === cachedWorktreeId && + nextFiles.length === cachedFiles.length && + nextFiles.every((file, index) => file === cachedFiles[index]) + ) { + cachedOpenFiles = openFiles + return cachedFiles + } + + cachedOpenFiles = openFiles + cachedWorktreeId = activeWorktreeId + cachedFiles = nextFiles.length > 0 ? nextFiles : EMPTY_OPEN_FILES + return cachedFiles +} diff --git a/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts b/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts new file mode 100644 index 00000000000..5c2227bc96c --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-browser-tab-slices.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' +import type { BrowserTab } from '../../../../shared/types' +import { getTerminalBrowserTabSlices } from './terminal-browser-tab-slices' + +const browserTab = (id: string): BrowserTab => ({ + id, + worktreeId: 'wt-active', + url: `https://example.com/${id}`, + title: id, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 0 +}) + +describe('getTerminalBrowserTabSlices', () => { + it('preserves slices when an unmounted worktree browser tab array changes', () => { + const activeBrowserTabs = [browserTab('active')] + const mountedIds = new Set(['wt-active']) + const first = getTerminalBrowserTabSlices( + { 'wt-active': activeBrowserTabs, 'wt-hidden': [browserTab('hidden-a')] }, + mountedIds, + 'wt-active' + ) + const second = getTerminalBrowserTabSlices( + { 'wt-active': activeBrowserTabs, 'wt-hidden': [browserTab('hidden-b')] }, + mountedIds, + 'wt-active' + ) + + expect(second).toBe(first) + expect(second.activeBrowserTabs).toBe(activeBrowserTabs) + }) + + it('updates slices when a mounted worktree browser tab array changes', () => { + const mountedIds = new Set(['wt-active', 'wt-mounted']) + const first = getTerminalBrowserTabSlices( + { 'wt-active': [browserTab('active')], 'wt-mounted': [browserTab('mounted-a')] }, + mountedIds, + 'wt-active' + ) + const mountedBrowserTabs = [browserTab('mounted-b')] + const second = getTerminalBrowserTabSlices( + { 'wt-active': first.activeBrowserTabs, 'wt-mounted': mountedBrowserTabs }, + mountedIds, + 'wt-active' + ) + + expect(second).not.toBe(first) + expect(second.mountedBrowserTabsByWorktree['wt-mounted']).toBe(mountedBrowserTabs) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts b/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts new file mode 100644 index 00000000000..df6f23c7828 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-browser-tab-slices.ts @@ -0,0 +1,68 @@ +import type { BrowserTab } from '../../../../shared/types' + +export type TerminalBrowserTabSlices = { + activeBrowserTabs: BrowserTab[] + mountedBrowserTabsByWorktree: Record +} + +const EMPTY_BROWSER_TABS: BrowserTab[] = [] +let cachedBrowserTabsByWorktree: Record | null = null +let cachedMountedIdsKey = '' +let cachedActiveWorktreeId: string | null = null +let cachedSlices: TerminalBrowserTabSlices = { + activeBrowserTabs: EMPTY_BROWSER_TABS, + mountedBrowserTabsByWorktree: {} +} + +function mountedIdsKey(mountedWorktreeIds: ReadonlySet): string { + return [...mountedWorktreeIds].sort().join('\0') +} + +function sameMountedBrowserTabs( + left: Record, + right: Record +): boolean { + const leftKeys = Object.keys(left) + const rightKeys = Object.keys(right) + return leftKeys.length === rightKeys.length && leftKeys.every((key) => left[key] === right[key]) +} + +export function getTerminalBrowserTabSlices( + browserTabsByWorktree: Record, + mountedWorktreeIds: ReadonlySet, + activeWorktreeId: string | null +): TerminalBrowserTabSlices { + const nextMountedIdsKey = mountedIdsKey(mountedWorktreeIds) + if ( + browserTabsByWorktree === cachedBrowserTabsByWorktree && + nextMountedIdsKey === cachedMountedIdsKey && + activeWorktreeId === cachedActiveWorktreeId + ) { + return cachedSlices + } + + const activeBrowserTabs = activeWorktreeId + ? (browserTabsByWorktree[activeWorktreeId] ?? EMPTY_BROWSER_TABS) + : EMPTY_BROWSER_TABS + const mountedBrowserTabsByWorktree: Record = {} + for (const worktreeId of mountedWorktreeIds) { + mountedBrowserTabsByWorktree[worktreeId] = + browserTabsByWorktree[worktreeId] ?? EMPTY_BROWSER_TABS + } + + cachedBrowserTabsByWorktree = browserTabsByWorktree + cachedMountedIdsKey = nextMountedIdsKey + cachedActiveWorktreeId = activeWorktreeId + if ( + activeBrowserTabs === cachedSlices.activeBrowserTabs && + sameMountedBrowserTabs(mountedBrowserTabsByWorktree, cachedSlices.mountedBrowserTabsByWorktree) + ) { + return cachedSlices + } + + // Why: hidden BrowserPanes are retained only for mounted worktrees. Avoid + // rendering or resubscribing the terminal surface when browser tabs in + // unvisited worktrees restore or refresh in the background. + cachedSlices = { activeBrowserTabs, mountedBrowserTabsByWorktree } + return cachedSlices +} diff --git a/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts b/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts new file mode 100644 index 00000000000..ba7f5e65d22 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-mounted-worktrees.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest' +import type { Worktree } from '../../../../shared/types' +import { getTerminalMountedWorktreeSnapshot } from './terminal-mounted-worktrees' + +const worktree = (input: Partial & Pick): Worktree => ({ + id: input.id, + path: input.path, + repoId: input.repoId ?? 'repo-1', + displayName: input.displayName ?? input.id, + comment: input.comment ?? '', + branch: input.branch ?? 'main', + head: input.head ?? 'abc123', + isBare: input.isBare ?? false, + isMainWorktree: input.isMainWorktree ?? false, + linkedIssue: input.linkedIssue ?? null, + linkedPR: input.linkedPR ?? null, + linkedLinearIssue: input.linkedLinearIssue ?? null, + isArchived: input.isArchived ?? false, + isUnread: input.isUnread ?? false, + isPinned: input.isPinned ?? false, + sortOrder: input.sortOrder ?? 0, + lastActivityAt: input.lastActivityAt ?? 0 +}) + +describe('getTerminalMountedWorktreeSnapshot', () => { + it('preserves the snapshot when unrelated worktree metadata changes', () => { + const mountedIds = new Set(['wt-active']) + const first = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active', linkedIssue: 1 }), + worktree({ id: 'wt-other', path: '/repo/other', displayName: 'Other' }) + ] + }, + mountedIds + ) + const second = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active', linkedIssue: 2 }), + worktree({ id: 'wt-other', path: '/repo/other', displayName: 'Renamed' }) + ] + }, + mountedIds + ) + + expect(second).toBe(first) + expect(second.mountedWorktrees).toEqual([{ id: 'wt-active', path: '/repo/active' }]) + }) + + it('returns a new snapshot when a mounted worktree path changes', () => { + const mountedIds = new Set(['wt-active']) + const first = getTerminalMountedWorktreeSnapshot( + { 'repo-1': [worktree({ id: 'wt-active', path: '/repo/active' })] }, + mountedIds + ) + const second = getTerminalMountedWorktreeSnapshot( + { 'repo-1': [worktree({ id: 'wt-active', path: '/repo/moved' })] }, + mountedIds + ) + + expect(second).not.toBe(first) + expect(second.mountedWorktrees).toEqual([{ id: 'wt-active', path: '/repo/moved' }]) + }) + + it('preserves the snapshot when an unmounted worktree path changes', () => { + const mountedIds = new Set(['wt-active']) + const first = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active' }), + worktree({ id: 'wt-hidden', path: '/repo/hidden-a' }) + ] + }, + mountedIds + ) + const second = getTerminalMountedWorktreeSnapshot( + { + 'repo-1': [ + worktree({ id: 'wt-active', path: '/repo/active' }), + worktree({ id: 'wt-hidden', path: '/repo/hidden-b' }) + ] + }, + mountedIds + ) + + expect(second).toBe(first) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts b/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts new file mode 100644 index 00000000000..b93d7e1c86d --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-mounted-worktrees.ts @@ -0,0 +1,67 @@ +import type { Worktree } from '../../../../shared/types' + +export type TerminalMountedWorktreeSnapshot = { + mountedWorktrees: Pick[] + worktreeIds: string[] +} + +let cachedWorktreesByRepo: Record | null = null +let cachedMountedIdsKey = '' +let cachedSnapshot: TerminalMountedWorktreeSnapshot = { + mountedWorktrees: [], + worktreeIds: [] +} + +function mountedIdsKey(mountedWorktreeIds: ReadonlySet): string { + return [...mountedWorktreeIds].sort().join('\0') +} + +function sameWorktreeProjection( + left: Pick[], + right: Pick[] +): boolean { + return ( + left.length === right.length && + left.every((worktree, index) => { + const other = right[index] + return worktree.id === other.id && worktree.path === other.path + }) + ) +} + +export function getTerminalMountedWorktreeSnapshot( + worktreesByRepo: Record, + mountedWorktreeIds: ReadonlySet +): TerminalMountedWorktreeSnapshot { + const nextMountedIdsKey = mountedIdsKey(mountedWorktreeIds) + if (worktreesByRepo === cachedWorktreesByRepo && nextMountedIdsKey === cachedMountedIdsKey) { + return cachedSnapshot + } + + const mountedWorktrees: Pick[] = [] + const worktreeIds: string[] = [] + for (const repoWorktrees of Object.values(worktreesByRepo)) { + for (const worktree of repoWorktrees) { + worktreeIds.push(worktree.id) + if (mountedWorktreeIds.has(worktree.id)) { + mountedWorktrees.push({ id: worktree.id, path: worktree.path }) + } + } + } + + cachedWorktreesByRepo = worktreesByRepo + cachedMountedIdsKey = nextMountedIdsKey + if ( + worktreeIds.length === cachedSnapshot.worktreeIds.length && + worktreeIds.every((id, index) => id === cachedSnapshot.worktreeIds[index]) && + sameWorktreeProjection(mountedWorktrees, cachedSnapshot.mountedWorktrees) + ) { + return cachedSnapshot + } + + // Why: Terminal only needs all IDs for pruning plus id/path for mounted pane + // trees. Preserve the snapshot when unrelated or unmounted worktree metadata + // changes so sidebar/status refreshes don't rerender xterm during typing. + cachedSnapshot = { mountedWorktrees, worktreeIds } + return cachedSnapshot +} diff --git a/src/renderer/src/components/terminal/terminal-tab-slices.test.ts b/src/renderer/src/components/terminal/terminal-tab-slices.test.ts new file mode 100644 index 00000000000..23f5abd6722 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-tab-slices.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' +import type { TerminalTab } from '../../../../shared/types' +import { getTerminalTabSlices } from './terminal-tab-slices' + +const tab = (id: string): TerminalTab => ({ + id, + title: id, + ptyId: null, + worktreeId: 'wt-active', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0, + generation: 0 +}) + +describe('getTerminalTabSlices', () => { + it('preserves slices when an unmounted worktree tab array changes', () => { + const activeTabs = [tab('active')] + const mountedIds = new Set(['wt-active']) + const first = getTerminalTabSlices( + { 'wt-active': activeTabs, 'wt-hidden': [tab('hidden-a')] }, + mountedIds, + 'wt-active' + ) + const second = getTerminalTabSlices( + { 'wt-active': activeTabs, 'wt-hidden': [tab('hidden-b')] }, + mountedIds, + 'wt-active' + ) + + expect(second).toBe(first) + expect(second.activeTabs).toBe(activeTabs) + }) + + it('updates slices when a mounted worktree tab array changes', () => { + const mountedIds = new Set(['wt-active', 'wt-mounted']) + const first = getTerminalTabSlices( + { 'wt-active': [tab('active')], 'wt-mounted': [tab('mounted-a')] }, + mountedIds, + 'wt-active' + ) + const mountedTabs = [tab('mounted-b')] + const second = getTerminalTabSlices( + { 'wt-active': first.activeTabs, 'wt-mounted': mountedTabs }, + mountedIds, + 'wt-active' + ) + + expect(second).not.toBe(first) + expect(second.mountedTabsByWorktree['wt-mounted']).toBe(mountedTabs) + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-tab-slices.ts b/src/renderer/src/components/terminal/terminal-tab-slices.ts new file mode 100644 index 00000000000..a64b234991a --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-tab-slices.ts @@ -0,0 +1,67 @@ +import type { TerminalTab } from '../../../../shared/types' + +export type TerminalTabSlices = { + activeTabs: TerminalTab[] + mountedTabsByWorktree: Record +} + +const EMPTY_TERMINAL_TABS: TerminalTab[] = [] +let cachedTabsByWorktree: Record | null = null +let cachedMountedIdsKey = '' +let cachedActiveWorktreeId: string | null = null +let cachedSlices: TerminalTabSlices = { + activeTabs: EMPTY_TERMINAL_TABS, + mountedTabsByWorktree: {} +} + +function mountedIdsKey(mountedWorktreeIds: ReadonlySet): string { + return [...mountedWorktreeIds].sort().join('\0') +} + +function sameMountedTabs( + left: Record, + right: Record +): boolean { + const leftKeys = Object.keys(left) + const rightKeys = Object.keys(right) + return leftKeys.length === rightKeys.length && leftKeys.every((key) => left[key] === right[key]) +} + +export function getTerminalTabSlices( + tabsByWorktree: Record, + mountedWorktreeIds: ReadonlySet, + activeWorktreeId: string | null +): TerminalTabSlices { + const nextMountedIdsKey = mountedIdsKey(mountedWorktreeIds) + if ( + tabsByWorktree === cachedTabsByWorktree && + nextMountedIdsKey === cachedMountedIdsKey && + activeWorktreeId === cachedActiveWorktreeId + ) { + return cachedSlices + } + + const activeTabs = activeWorktreeId + ? (tabsByWorktree[activeWorktreeId] ?? EMPTY_TERMINAL_TABS) + : EMPTY_TERMINAL_TABS + const mountedTabsByWorktree: Record = {} + for (const worktreeId of mountedWorktreeIds) { + mountedTabsByWorktree[worktreeId] = tabsByWorktree[worktreeId] ?? EMPTY_TERMINAL_TABS + } + + cachedTabsByWorktree = tabsByWorktree + cachedMountedIdsKey = nextMountedIdsKey + cachedActiveWorktreeId = activeWorktreeId + if ( + activeTabs === cachedSlices.activeTabs && + sameMountedTabs(mountedTabsByWorktree, cachedSlices.mountedTabsByWorktree) + ) { + return cachedSlices + } + + // Why: Terminal renders only the active titlebar and mounted pane trees. + // Ignore tab-array churn for unmounted worktrees so background metadata + // updates do not rerender xterm while the user is typing. + cachedSlices = { activeTabs, mountedTabsByWorktree } + return cachedSlices +} diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index 0973efa125c..88bc179cef8 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -24,7 +24,7 @@ function isAgentTaskCompleteNotificationEnabled(): boolean { export function syncAgentHookCompletionNotificationSettings(): boolean { const enabled = isAgentTaskCompleteNotificationEnabled() - if (!enabled || (!wasAgentTaskCompleteNotificationEnabled && enabled)) { + if (enabled !== wasAgentTaskCompleteNotificationEnabled) { requireFreshWorkingForNewCoordinators = true for (const [paneKey, entry] of coordinatorsByPaneKey) { paneKeysRequiringFreshWorking.add(paneKey) diff --git a/src/renderer/src/hooks/useAutomationDispatchEvents.ts b/src/renderer/src/hooks/useAutomationDispatchEvents.ts index 2963e939242..f1564b1d998 100644 --- a/src/renderer/src/hooks/useAutomationDispatchEvents.ts +++ b/src/renderer/src/hooks/useAutomationDispatchEvents.ts @@ -6,9 +6,9 @@ import { launchAgentBackgroundSession } from '@/lib/launch-agent-background-sess import { submitPromptToAgentTab } from '@/lib/agent-paste-draft' import { findReusableAutomationSession } from '@/lib/automation-session-reuse' import { observeExistingAutomationSession } from '@/lib/automation-session-observer' +import { getAutomationAgentCompletionObservation } from '@/lib/automation-agent-completion' import { useAppStore } from '@/store' import type { AutomationDispatchResult } from '../../../shared/automations-types' -import { parsePaneKey } from '../../../shared/stable-pane-id' import { createAutomationRunOutputSnapshotBuffer, selectAutomationRunOutputSnapshot @@ -195,35 +195,35 @@ export function useAutomationDispatchEvents(): void { void markCompletionResult() } const observeAgentStatus = ( - tabId: string, + paneKey: string, startedAfter: number, options?: { requireWorkingAfterStart?: boolean } ): void => { let sawWorkingAfterStart = false const checkCurrentStatus = (): void => { - const { agentStatusByPaneKey } = useAppStore.getState() - for (const [paneKey, entry] of Object.entries(agentStatusByPaneKey)) { - const parsed = parsePaneKey(paneKey) - if (parsed?.tabId !== tabId || entry.updatedAt < startedAfter) { - continue - } - if (entry.state === 'working') { - sawWorkingAfterStart = true - } - if ( - entry.state === 'done' && - (!options?.requireWorkingAfterStart || sawWorkingAfterStart) - ) { - latestAssistantMessage = - entry.lastAssistantMessage?.trim() || latestAssistantMessage - handleAgentDone() - return - } + const entry = useAppStore.getState().agentStatusByPaneKey[paneKey] + const observation = getAutomationAgentCompletionObservation({ + entry, + startedAfter, + sawWorkingAfterStart, + requireWorkingAfterStart: options?.requireWorkingAfterStart + }) + sawWorkingAfterStart = observation.sawWorkingAfterStart + latestAssistantMessage = observation.latestAssistantMessage || latestAssistantMessage + if (observation.done) { + handleAgentDone() } } // Why: Codex/Claude completion normally arrives through the global // hook IPC listener, not the hidden PTY OSC fallback. - unsubscribeAgentStatus = useAppStore.subscribe(checkCurrentStatus) + unsubscribeAgentStatus = useAppStore.subscribe((state, previousState) => { + if ( + state.agentStatusByPaneKey[paneKey] === previousState.agentStatusByPaneKey[paneKey] + ) { + return + } + checkCurrentStatus() + }) checkCurrentStatus() } const dispatchStartedAt = Date.now() @@ -282,7 +282,7 @@ export function useAutomationDispatchEvents(): void { void markExitResult(code) } }) - observeAgentStatus(reusableSession.tabId, reuseCompletionStartedAt, { + observeAgentStatus(reusableSession.paneKey, reuseCompletionStartedAt, { requireWorkingAfterStart: true }) await markDispatchResult({ @@ -338,7 +338,7 @@ export function useAutomationDispatchEvents(): void { if (!result) { throw new Error('Unable to build an agent launch plan.') } - observeAgentStatus(result.tabId, dispatchStartedAt) + observeAgentStatus(result.paneKey, dispatchStartedAt) try { await markDispatchResult({ runId: run.id, diff --git a/src/renderer/src/hooks/useEditorExternalWatch-targets.test.ts b/src/renderer/src/hooks/useEditorExternalWatch-targets.test.ts new file mode 100644 index 00000000000..957abb03063 --- /dev/null +++ b/src/renderer/src/hooks/useEditorExternalWatch-targets.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from 'vitest' +import { getEditorExternalWatchTargets } from './useEditorExternalWatch' + +vi.mock('@/store', () => ({ + useAppStore: { + getState: vi.fn() + } +})) +vi.mock('@/components/editor/editor-autosave', () => ({ + notifyEditorExternalFileChange: vi.fn(), + getOpenFilesForExternalFileChange: vi.fn(() => []) +})) + +describe('getEditorExternalWatchTargets', () => { + const repo = { id: 'repo-1', path: '/repo', kind: 'git', connectionId: null } + const worktree = { id: 'wt-1', repoId: 'repo-1', path: '/repo' } + + const makeState = (isDirty: boolean) => + ({ + openFiles: [ + { + id: 'file-1', + worktreeId: 'wt-1', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + isDirty + } + ], + worktreesByRepo: { 'repo-1': [worktree] }, + repos: [repo], + activeWorktreeId: null, + settings: null + }) as never + + it('preserves the snapshot when open-file metadata changes without changing watched roots', () => { + const first = getEditorExternalWatchTargets(makeState(false)) + const second = getEditorExternalWatchTargets(makeState(true)) + + expect(second).toBe(first) + expect(second.targets).toEqual([ + { + worktreeId: 'wt-1', + worktreePath: '/repo', + connectionId: undefined, + runtimeEnvironmentId: undefined + } + ]) + }) +}) diff --git a/src/renderer/src/hooks/useEditorExternalWatch.ts b/src/renderer/src/hooks/useEditorExternalWatch.ts index 5db3dbe3e3b..0216814ede8 100644 --- a/src/renderer/src/hooks/useEditorExternalWatch.ts +++ b/src/renderer/src/hooks/useEditorExternalWatch.ts @@ -2,8 +2,8 @@ target diffing, fs:changed dispatch, tombstone coalescing, and rename correlation so the end-to-end event-to-store mutation contract stays readable in one file. */ -import { useEffect, useMemo, useRef } from 'react' -import { useAppStore } from '@/store' +import { useEffect, useRef } from 'react' +import { useAppStore, type AppState } from '@/store' import { basename, joinPath } from '@/lib/path' import { getExternalFileChangeRelativePath } from '@/components/right-sidebar/useFileExplorerWatch' import { normalizeRuntimePathForComparison } from '../../../shared/cross-platform-path' @@ -71,6 +71,23 @@ type ExternalWatchNotification = { relativePath: string } +type WatchedTargetsSnapshot = { + targets: WatchedTarget[] + targetsKey: string +} + +type EditorExternalWatchTargetState = Pick< + AppState, + 'openFiles' | 'worktreesByRepo' | 'repos' | 'activeWorktreeId' | 'settings' +> + +let cachedOpenFiles: AppState['openFiles'] | null = null +let cachedWorktreesByRepo: AppState['worktreesByRepo'] | null = null +let cachedRepos: AppState['repos'] | null = null +let cachedActiveWorktreeId: string | null = null +let cachedRuntimeEnvironmentId: string | undefined +let cachedWatchedTargetsSnapshot: WatchedTargetsSnapshot = { targets: [], targetsKey: '' } + export function getWatchedTargetKey(target: WatchedTarget): string { // Why: SSH worktrees can exist in the store before their remote filesystem // provider is ready. Include connectionId so a local/unknown placeholder @@ -78,6 +95,64 @@ export function getWatchedTargetKey(target: WatchedTarget): string { return `${target.worktreeId}::${target.worktreePath}::${target.connectionId ?? 'local'}::${target.runtimeEnvironmentId ?? 'client'}` } +export function getEditorExternalWatchTargets( + state: EditorExternalWatchTargetState +): WatchedTargetsSnapshot { + const runtimeEnvironmentId = state.settings?.activeRuntimeEnvironmentId?.trim() || undefined + if ( + cachedOpenFiles === state.openFiles && + cachedWorktreesByRepo === state.worktreesByRepo && + cachedRepos === state.repos && + cachedActiveWorktreeId === state.activeWorktreeId && + cachedRuntimeEnvironmentId === runtimeEnvironmentId + ) { + return cachedWatchedTargetsSnapshot + } + + const ids = new Set() + // Why: only the set of worktree IDs matters for watcher ownership. Dirty + // flags and editor metadata can churn while typing/saving, but should not + // re-render App or rebuild watch subscriptions. + for (const f of state.openFiles) { + ids.add(f.worktreeId) + } + if (state.activeWorktreeId) { + ids.add(state.activeWorktreeId) + } + + const nextTargets: WatchedTarget[] = [] + const parts: string[] = [] + for (const id of Array.from(ids).sort()) { + const wt = findWorktreeById(state.worktreesByRepo, id) + if (!wt) { + continue + } + const repo = state.repos.find((r) => r.id === wt.repoId) + const target = { + worktreeId: id, + worktreePath: wt.path, + connectionId: repo?.connectionId ?? undefined, + runtimeEnvironmentId + } + nextTargets.push(target) + parts.push(getWatchedTargetKey(target)) + } + + const targetsKey = parts.join('|') + cachedOpenFiles = state.openFiles + cachedWorktreesByRepo = state.worktreesByRepo + cachedRepos = state.repos + cachedActiveWorktreeId = state.activeWorktreeId + cachedRuntimeEnvironmentId = runtimeEnvironmentId + + if (targetsKey === cachedWatchedTargetsSnapshot.targetsKey) { + return cachedWatchedTargetsSnapshot + } + + cachedWatchedTargetsSnapshot = { targets: nextTargets, targetsKey } + return cachedWatchedTargetsSnapshot +} + // Why: macOS atomic writes (Claude Code Edit, vim :w, VSCode save) deliver a // delete event immediately followed by a create event for the same path. When // those two land in separate fs:changed payloads a few ms apart, the tab @@ -108,47 +183,7 @@ type PendingDeleteTimer = { * regardless of which UI panel is visible. */ export function useEditorExternalWatch(): void { - const openFiles = useAppStore((s) => s.openFiles) - const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) - const repos = useAppStore((s) => s.repos) - const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) - const runtimeEnvironmentId = useAppStore((s) => s.settings?.activeRuntimeEnvironmentId) - - // Why: unify the target computation and the dependency key into one memo so - // there's a single source of truth. The derived string key drives the - // watch-diff effect; the array itself is what the effect actually iterates. - const { targets, targetsKey } = useMemo(() => { - const ids = new Set() - // Why: watch every worktree that has an editor tab open, so terminal edits - // in any of those roots reach the editor. Also watch the active worktree - // even when it has no open files — otherwise the File Explorer's tree - // reconciliation loses its event stream the moment the last tab for that - // worktree is closed. - for (const f of openFiles) { - ids.add(f.worktreeId) - } - if (activeWorktreeId) { - ids.add(activeWorktreeId) - } - const nextTargets: WatchedTarget[] = [] - const parts: string[] = [] - for (const id of Array.from(ids).sort()) { - const wt = findWorktreeById(worktreesByRepo, id) - if (!wt) { - continue - } - const repo = repos.find((r) => r.id === wt.repoId) - const target = { - worktreeId: id, - worktreePath: wt.path, - connectionId: repo?.connectionId ?? undefined, - runtimeEnvironmentId: runtimeEnvironmentId?.trim() || undefined - } - nextTargets.push(target) - parts.push(getWatchedTargetKey(target)) - } - return { targets: nextTargets, targetsKey: parts.join('|') } - }, [openFiles, worktreesByRepo, repos, activeWorktreeId, runtimeEnvironmentId]) + const { targets, targetsKey } = useAppStore(getEditorExternalWatchTargets) const targetsRef = useRef([]) const latestTargetsRef = useRef(targets) diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index 0ef243ae2a4..b0d2c8ffa46 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -1897,7 +1897,7 @@ describe('useIpcEvents agent status snapshot integration', () => { stateStartedAt: number } type StoreLike = Record - type StoreSubscribeListener = (state: StoreLike) => void + type StoreSubscribeListener = (state: StoreLike, previousState?: StoreLike) => void function buildStoreState(overrides: StoreLike): StoreLike { // Why: copy the defensive set of getState() fields the hook touches during @@ -2514,6 +2514,85 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(setAgentStatus).not.toHaveBeenCalled() }) + it('gates snapshot and notification-settings work to the store fields they consume', async () => { + const getSnapshot = vi.fn(() => Promise.resolve([])) + const syncAgentHookCompletionNotificationSettings = vi.fn(() => true) + const subscribeListenerRef: { current: StoreSubscribeListener | null } = { current: null } + const notifications = { enabled: true, agentTaskComplete: true } + + let storeState: StoreLike = buildStoreState({ + workspaceSessionReady: true, + settings: { terminalFontSize: 13, notifications } + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn((listener: StoreSubscribeListener) => { + subscribeListenerRef.current = listener + return () => { + subscribeListenerRef.current = null + } + }), + getState: () => storeState + } + })) + vi.doMock('./agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: vi.fn(), + syncAgentHookCompletionNotificationSettings + })) + stubAuxiliaryModules() + vi.stubGlobal( + 'window', + buildWindowApi({ + getSnapshot, + onSet: () => () => {} + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + + useIpcEvents() + await Promise.resolve() + expect(getSnapshot).toHaveBeenCalledTimes(1) + expect(syncAgentHookCompletionNotificationSettings).toHaveBeenCalledTimes(1) + + if (typeof subscribeListenerRef.current !== 'function') { + throw new Error('Expected useAppStore.subscribe listener to be registered') + } + + let previousState = storeState + storeState = { ...storeState, activeView: 'tasks' } + subscribeListenerRef.current(storeState, previousState) + expect(getSnapshot).toHaveBeenCalledTimes(1) + expect(syncAgentHookCompletionNotificationSettings).toHaveBeenCalledTimes(1) + + previousState = storeState + storeState = { + ...storeState, + settings: { + ...(storeState.settings as Record), + notifications: { enabled: true, agentTaskComplete: false } + } + } + subscribeListenerRef.current(storeState, previousState) + expect(getSnapshot).toHaveBeenCalledTimes(1) + expect(syncAgentHookCompletionNotificationSettings).toHaveBeenCalledTimes(2) + + previousState = storeState + storeState = { ...storeState, workspaceSessionReady: false } + subscribeListenerRef.current(storeState, previousState) + expect(getSnapshot).toHaveBeenCalledTimes(1) + expect(syncAgentHookCompletionNotificationSettings).toHaveBeenCalledTimes(2) + + previousState = storeState + storeState = { ...storeState, workspaceSessionReady: true } + subscribeListenerRef.current(storeState, previousState) + await Promise.resolve() + expect(getSnapshot).toHaveBeenCalledTimes(2) + expect(syncAgentHookCompletionNotificationSettings).toHaveBeenCalledTimes(2) + }) + it('waits for the remote workspace client id before dropping self notifications', async () => { const hydrateWorkspaceSession = vi.fn() const hydrateTabsSession = vi.fn() diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 4c913e5299f..1552bf9f47d 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -2036,11 +2036,30 @@ export function useIpcEvents(): void { // can be safely ignored instead of buffered against partially hydrated // renderer state. requestAgentStatusSnapshotIfReady() + syncAgentHookCompletionNotificationSettings() unsubs.push( - useAppStore.subscribe(() => { - requestAgentStatusSnapshotIfReady() - flushPendingAgentStatuses() - syncAgentHookCompletionNotificationSettings() + useAppStore.subscribe((state, previousState) => { + if (!previousState || state.workspaceSessionReady !== previousState.workspaceSessionReady) { + requestAgentStatusSnapshotIfReady() + } + // Why: pending hook events only become resolvable when pane routing + // inputs change; avoid scanning the queue on high-rate terminal updates. + if ( + !previousState || + state.workspaceSessionReady !== previousState.workspaceSessionReady || + state.terminalLayoutsByTabId !== previousState.terminalLayoutsByTabId || + state.tabsByWorktree !== previousState.tabsByWorktree || + state.worktreesByRepo !== previousState.worktreesByRepo || + state.repos !== previousState.repos + ) { + flushPendingAgentStatuses() + } + if ( + !previousState || + state.settings?.notifications !== previousState.settings?.notifications + ) { + syncAgentHookCompletionNotificationSettings() + } }) ) diff --git a/src/renderer/src/lib/automation-agent-completion.test.ts b/src/renderer/src/lib/automation-agent-completion.test.ts new file mode 100644 index 00000000000..b1f39e35fa0 --- /dev/null +++ b/src/renderer/src/lib/automation-agent-completion.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest' +import { getAutomationAgentCompletionObservation } from './automation-agent-completion' +import type { AgentStatusEntry } from '../../../shared/agent-status-types' + +function status(overrides: Partial): AgentStatusEntry { + return { + state: 'working', + prompt: '', + updatedAt: 2, + stateStartedAt: 2, + paneKey: 'tab-1:11111111-1111-4111-8111-111111111111', + stateHistory: [], + ...overrides + } +} + +describe('getAutomationAgentCompletionObservation', () => { + it('ignores stale entries from before the automation completion window', () => { + expect( + getAutomationAgentCompletionObservation({ + entry: status({ state: 'done', updatedAt: 1 }), + startedAfter: 2, + sawWorkingAfterStart: false + }) + ).toEqual({ + sawWorkingAfterStart: false, + latestAssistantMessage: null, + done: false + }) + }) + + it('requires a fresh working state before completing reused sessions', () => { + const waiting = getAutomationAgentCompletionObservation({ + entry: status({ state: 'done', updatedAt: 3 }), + startedAfter: 2, + sawWorkingAfterStart: false, + requireWorkingAfterStart: true + }) + const working = getAutomationAgentCompletionObservation({ + entry: status({ state: 'working', updatedAt: 4 }), + startedAfter: 2, + sawWorkingAfterStart: waiting.sawWorkingAfterStart, + requireWorkingAfterStart: true + }) + const done = getAutomationAgentCompletionObservation({ + entry: status({ state: 'done', updatedAt: 5, lastAssistantMessage: ' finished ' }), + startedAfter: 2, + sawWorkingAfterStart: working.sawWorkingAfterStart, + requireWorkingAfterStart: true + }) + + expect(waiting.done).toBe(false) + expect(working.sawWorkingAfterStart).toBe(true) + expect(done).toEqual({ + sawWorkingAfterStart: true, + latestAssistantMessage: 'finished', + done: true + }) + }) +}) diff --git a/src/renderer/src/lib/automation-agent-completion.ts b/src/renderer/src/lib/automation-agent-completion.ts new file mode 100644 index 00000000000..83dfd1c2f41 --- /dev/null +++ b/src/renderer/src/lib/automation-agent-completion.ts @@ -0,0 +1,30 @@ +import type { AgentStatusEntry } from '../../../shared/agent-status-types' + +export type AutomationAgentCompletionObservation = { + sawWorkingAfterStart: boolean + latestAssistantMessage: string | null + done: boolean +} + +export function getAutomationAgentCompletionObservation(args: { + entry: AgentStatusEntry | undefined + startedAfter: number + sawWorkingAfterStart: boolean + requireWorkingAfterStart?: boolean +}): AutomationAgentCompletionObservation { + const { entry, startedAfter, requireWorkingAfterStart } = args + if (!entry || entry.updatedAt < startedAfter) { + return { + sawWorkingAfterStart: args.sawWorkingAfterStart, + latestAssistantMessage: null, + done: false + } + } + + const sawWorkingAfterStart = args.sawWorkingAfterStart || entry.state === 'working' + return { + sawWorkingAfterStart, + latestAssistantMessage: entry.lastAssistantMessage?.trim() || null, + done: entry.state === 'done' && (!requireWorkingAfterStart || sawWorkingAfterStart) + } +} diff --git a/src/renderer/src/lib/focused-visibility-interval.test.ts b/src/renderer/src/lib/focused-visibility-interval.test.ts new file mode 100644 index 00000000000..e7681ac3002 --- /dev/null +++ b/src/renderer/src/lib/focused-visibility-interval.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { installFocusedVisibilityInterval } from './focused-visibility-interval' + +describe('installFocusedVisibilityInterval', () => { + beforeEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() + }) + + it('runs intervals only while the document is visible', () => { + let visibilityState: DocumentVisibilityState = 'hidden' + const documentListeners = new Map void>() + const clearIntervalMock = vi.fn() + const setIntervalMock = vi.fn(() => 1 as unknown as ReturnType) + const run = vi.fn() + + vi.stubGlobal('window', { + addEventListener: vi.fn(), + removeEventListener: vi.fn() + }) + vi.stubGlobal('document', { + get visibilityState() { + return visibilityState + }, + hasFocus: () => false, + addEventListener: vi.fn((event: string, listener: () => void) => { + documentListeners.set(event, listener) + }), + removeEventListener: vi.fn() + }) + + const cleanup = installFocusedVisibilityInterval({ + run, + intervalMs: 3000, + setIntervalFn: setIntervalMock, + clearIntervalFn: clearIntervalMock + }) + + expect(run).not.toHaveBeenCalled() + expect(setIntervalMock).not.toHaveBeenCalled() + + visibilityState = 'visible' + documentListeners.get('visibilitychange')?.() + expect(run).toHaveBeenCalledTimes(1) + expect(setIntervalMock).toHaveBeenCalledTimes(1) + + visibilityState = 'hidden' + documentListeners.get('visibilitychange')?.() + expect(clearIntervalMock).toHaveBeenCalledWith(1) + + cleanup() + }) +}) diff --git a/src/renderer/src/lib/focused-visibility-interval.ts b/src/renderer/src/lib/focused-visibility-interval.ts new file mode 100644 index 00000000000..9e9d04959af --- /dev/null +++ b/src/renderer/src/lib/focused-visibility-interval.ts @@ -0,0 +1,57 @@ +export type FocusedVisibilityIntervalTimer = ReturnType + +export function isWindowVisible(): boolean { + return typeof document.visibilityState === 'undefined' || document.visibilityState === 'visible' +} + +export function installFocusedVisibilityInterval(args: { + run: () => void + intervalMs: number + setIntervalFn?: (callback: () => void, intervalMs: number) => FocusedVisibilityIntervalTimer + clearIntervalFn?: (handle: FocusedVisibilityIntervalTimer) => void +}): () => void { + const setIntervalFn = + args.setIntervalFn ?? + ((callback: () => void, intervalMs: number): FocusedVisibilityIntervalTimer => + setInterval(callback, intervalMs)) + const clearIntervalFn = + args.clearIntervalFn ?? + ((handle: FocusedVisibilityIntervalTimer): void => clearInterval(handle)) + let intervalId: FocusedVisibilityIntervalTimer | null = null + + const stop = (): void => { + if (!intervalId) { + return + } + clearIntervalFn(intervalId) + intervalId = null + } + const start = (): void => { + if (intervalId || !isWindowVisible()) { + return + } + args.run() + // Why: many callers shell out or cross IPC. Keep their interval alive only + // while Orca can present the refreshed data, but still refresh a visible + // unfocused window so status UI does not go stale on a second display. + intervalId = setIntervalFn(args.run, args.intervalMs) + } + const reconcile = (): void => { + if (isWindowVisible()) { + start() + } else { + stop() + } + } + + start() + if (typeof document.addEventListener === 'function') { + document.addEventListener('visibilitychange', reconcile) + } + return () => { + stop() + if (typeof document.removeEventListener === 'function') { + document.removeEventListener('visibilitychange', reconcile) + } + } +} diff --git a/src/renderer/src/lib/launch-agent-background-session.test.ts b/src/renderer/src/lib/launch-agent-background-session.test.ts index 82cadca7fb5..13057454071 100644 --- a/src/renderer/src/lib/launch-agent-background-session.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session.test.ts @@ -154,7 +154,7 @@ describe('launchAgentBackgroundSession', () => { expect(mockRegisterEagerPtyBuffer).toHaveBeenCalledWith('pty-1', expect.any(Function)) expect(mockSubscribeToPtyData).toHaveBeenCalledWith('pty-1', expect.any(Function)) expect(mockSubscribeToPtyExit).toHaveBeenCalledWith('pty-1', expect.any(Function)) - expect(result).toMatchObject({ tabId: 'tab-1', ptyId: 'pty-1' }) + expect(result).toMatchObject({ tabId: 'tab-1', ptyId: 'pty-1', paneKey }) }) it('pre-marks trust for agents with first-launch trust prompts', async () => { @@ -329,6 +329,6 @@ describe('launchAgentBackgroundSession', () => { }), expect.any(Object) ) - expect(result).toMatchObject({ tabId: 'tab-1', ptyId: 'remote:env-1@@terminal-1' }) + expect(result).toMatchObject({ tabId: 'tab-1', ptyId: 'remote:env-1@@terminal-1', paneKey }) }) }) diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index 03b0659d73b..a260ca746e2 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -38,6 +38,7 @@ export type LaunchAgentBackgroundSessionArgs = { export type LaunchAgentBackgroundSessionResult = { tabId: string ptyId: string + paneKey: string startupPlan: AgentStartupPlan } @@ -252,5 +253,5 @@ export async function launchAgentBackgroundSession( }) } - return { tabId: tab.id, ptyId, startupPlan } + return { tabId: tab.id, ptyId, paneKey, startupPlan } } diff --git a/src/renderer/src/lib/workspace-port-groups.test.ts b/src/renderer/src/lib/workspace-port-groups.test.ts new file mode 100644 index 00000000000..ae76580e868 --- /dev/null +++ b/src/renderer/src/lib/workspace-port-groups.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from 'vitest' +import { + getExternalWorkspacePorts, + getWorkspacePortGroups, + getWorkspacePortsByWorktreeId +} from './workspace-port-groups' + +describe('workspace port group caches', () => { + it('returns stable empty references when no scan result exists', () => { + expect(getWorkspacePortsByWorktreeId(null)).toBe(getWorkspacePortsByWorktreeId(undefined)) + expect(getWorkspacePortGroups(null)).toBe(getWorkspacePortGroups(undefined)) + expect(getExternalWorkspacePorts(null)).toBe(getExternalWorkspacePorts(undefined)) + }) +}) diff --git a/src/renderer/src/lib/workspace-port-groups.ts b/src/renderer/src/lib/workspace-port-groups.ts index d0c88b698f6..23bc2d0da2c 100644 --- a/src/renderer/src/lib/workspace-port-groups.ts +++ b/src/renderer/src/lib/workspace-port-groups.ts @@ -10,6 +10,9 @@ export type WorkspacePortGroup = { const portsByWorktreeCache = new WeakMap>() const workspaceGroupsCache = new WeakMap() const externalPortsCache = new WeakMap() +const EMPTY_PORTS_BY_WORKTREE = new Map() +const EMPTY_WORKSPACE_PORT_GROUPS: WorkspacePortGroup[] = [] +const EMPTY_EXTERNAL_PORTS: WorkspacePort[] = [] function comparePorts(a: WorkspacePort, b: WorkspacePort): number { return a.port - b.port || (a.processName ?? '').localeCompare(b.processName ?? '') @@ -18,6 +21,9 @@ function comparePorts(a: WorkspacePort, b: WorkspacePort): number { export function getWorkspacePortsByWorktreeId( scan: WorkspacePortScanResult | null | undefined ): Map { + if (!scan) { + return EMPTY_PORTS_BY_WORKTREE + } if (scan) { const cached = portsByWorktreeCache.get(scan) if (cached) { @@ -48,6 +54,9 @@ export function getWorkspacePortsByWorktreeId( export function getWorkspacePortGroups( scan: WorkspacePortScanResult | null | undefined ): WorkspacePortGroup[] { + if (!scan) { + return EMPTY_WORKSPACE_PORT_GROUPS + } if (scan) { const cached = workspaceGroupsCache.get(scan) if (cached) { @@ -87,6 +96,9 @@ export function getWorkspacePortGroups( export function getExternalWorkspacePorts( scan: WorkspacePortScanResult | null | undefined ): WorkspacePort[] { + if (!scan) { + return EMPTY_EXTERNAL_PORTS + } if (scan) { const cached = externalPortsCache.get(scan) if (cached) { diff --git a/src/renderer/src/runtime/sync-runtime-graph.test.ts b/src/renderer/src/runtime/sync-runtime-graph.test.ts index 6d87b8c4067..9c907cd4cf8 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.test.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { buildMobileSessionTabSnapshots, + canSkipRuntimeMobileSessionSyncKeyBuild, getRuntimeMobileSessionSyncKey, runtimeMobileSessionSyncKeysEqual } from './sync-runtime-graph' @@ -23,6 +24,8 @@ function makeState(overrides: Partial = {}): AppState { browserPagesByWorkspace: {}, openFiles: [], editorDrafts: {}, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, activeTabId: null, ...overrides } as AppState @@ -49,10 +52,15 @@ function makeSharedOverrides(): Partial { activeFileIdByWorktree: {}, activeBrowserTabIdByWorktree: {}, browserTabsByWorktree: {}, - browserPagesByWorkspace: {} + browserPagesByWorkspace: {}, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0 } } +type BrowserWorkspaceForTest = AppState['browserTabsByWorktree'][string][number] +type BrowserPageForTest = AppState['browserPagesByWorkspace'][string][number] + describe('getRuntimeMobileSessionSyncKey', () => { it('changes when mobile markdown tab state changes', () => { const base = makeState({ @@ -240,21 +248,114 @@ describe('getRuntimeMobileSessionSyncKey', () => { expect(runtimeMobileSessionSyncKeysEqual(before, after)).toBe(false) }) - it('changes when explicit agent status changes', () => { + it('changes when explicit agent status epoch changes', () => { const sharedOverrides = makeSharedOverrides() const before = getRuntimeMobileSessionSyncKey( makeState({ ...sharedOverrides, - agentStatusByPaneKey: {} + agentStatusEpoch: 0 }) ) const after = getRuntimeMobileSessionSyncKey( makeState({ ...sharedOverrides, + agentStatusEpoch: 1 + }) + ) + + expect(runtimeMobileSessionSyncKeysEqual(before, after)).toBe(false) + }) + + it('changes when mobile browser workspace projection fields change', () => { + const sharedOverrides = makeSharedOverrides() + const workspace: BrowserWorkspaceForTest = { + id: 'browser-1', + worktreeId: 'wt-1', + sessionProfileId: null, + activePageId: 'page-1', + pageIds: ['page-1'], + url: 'https://example.com', + title: 'Example', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } + const baseKey = getRuntimeMobileSessionSyncKey( + makeState({ + ...sharedOverrides, + browserTabsByWorktree: { 'wt-1': [workspace] } + }) + ) + + for (const changedWorkspace of [ + { ...workspace, title: 'Changed' }, + { ...workspace, url: 'https://changed.example.com' }, + { ...workspace, loading: true }, + { ...workspace, canGoBack: true }, + { ...workspace, canGoForward: true } + ]) { + const changedKey = getRuntimeMobileSessionSyncKey( + makeState({ + ...sharedOverrides, + browserTabsByWorktree: { 'wt-1': [changedWorkspace] } + }) + ) + expect(runtimeMobileSessionSyncKeysEqual(baseKey, changedKey)).toBe(false) + } + }) + + it('changes when mobile browser page projection fields change', () => { + const sharedOverrides = makeSharedOverrides() + const page: BrowserPageForTest = { + id: 'page-1', + workspaceId: 'browser-1', + worktreeId: 'wt-1', + url: 'https://example.com', + title: 'Example', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 1 + } + const baseKey = getRuntimeMobileSessionSyncKey( + makeState({ + ...sharedOverrides, + browserPagesByWorkspace: { 'browser-1': [page] } + }) + ) + + for (const changedPage of [ + { ...page, title: 'Changed' }, + { ...page, url: 'https://changed.example.com' }, + { ...page, loading: true }, + { ...page, canGoBack: true }, + { ...page, canGoForward: true } + ]) { + const changedKey = getRuntimeMobileSessionSyncKey( + makeState({ + ...sharedOverrides, + browserPagesByWorkspace: { 'browser-1': [changedPage] } + }) + ) + expect(runtimeMobileSessionSyncKeysEqual(baseKey, changedKey)).toBe(false) + } + }) + + it('changes for same-state agent prompt/tool updates because mobile publishes details', () => { + const sharedOverrides = makeSharedOverrides() + const before = getRuntimeMobileSessionSyncKey( + makeState({ + ...sharedOverrides, + agentStatusEpoch: 1, agentStatusByPaneKey: { 'term-1:11111111-1111-4111-8111-111111111111': { state: 'working', - prompt: 'fix parity', + prompt: 'first prompt', updatedAt: 1_700_000_000_000, stateStartedAt: 1_699_999_999_000, agentType: 'codex', @@ -265,9 +366,67 @@ describe('getRuntimeMobileSessionSyncKey', () => { } }) ) + const after = getRuntimeMobileSessionSyncKey( + makeState({ + ...sharedOverrides, + agentStatusEpoch: 1, + agentStatusByPaneKey: { + 'term-1:11111111-1111-4111-8111-111111111111': { + state: 'working', + prompt: 'updated prompt preview', + toolName: 'Edit', + updatedAt: 1_700_000_001_000, + stateStartedAt: 1_699_999_999_000, + agentType: 'codex', + paneKey: 'term-1:11111111-1111-4111-8111-111111111111', + terminalTitle: 'codex [working]', + stateHistory: [] + } + } + }) + ) expect(runtimeMobileSessionSyncKeysEqual(before, after)).toBe(false) }) + + it('does not skip the App subscriber gate for same-epoch agent detail updates', () => { + const sharedOverrides = makeSharedOverrides() + const before = makeState({ + ...sharedOverrides, + agentStatusEpoch: 1, + agentStatusByPaneKey: { + 'term-1:11111111-1111-4111-8111-111111111111': { + state: 'working', + prompt: 'first prompt', + updatedAt: 1_700_000_000_000, + stateStartedAt: 1_699_999_999_000, + agentType: 'codex', + paneKey: 'term-1:11111111-1111-4111-8111-111111111111', + terminalTitle: 'codex [working]', + stateHistory: [] + } + } + }) + const after = makeState({ + ...sharedOverrides, + agentStatusEpoch: 1, + agentStatusByPaneKey: { + 'term-1:11111111-1111-4111-8111-111111111111': { + state: 'working', + prompt: 'updated prompt preview', + toolName: 'Edit', + updatedAt: 1_700_000_001_000, + stateStartedAt: 1_699_999_999_000, + agentType: 'codex', + paneKey: 'term-1:11111111-1111-4111-8111-111111111111', + terminalTitle: 'codex [working]', + stateHistory: [] + } + } + }) + + expect(canSkipRuntimeMobileSessionSyncKeyBuild(after, before)).toBe(false) + }) }) describe('buildMobileSessionTabSnapshots', () => { diff --git a/src/renderer/src/runtime/sync-runtime-graph.ts b/src/renderer/src/runtime/sync-runtime-graph.ts index 9759f0ae187..7b206fc35e3 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.ts @@ -69,7 +69,6 @@ const NO_TRANSPORT_GRACE_MS = 10_000 const EMPTY_ACTIVE_BROWSER_TAB_ID_BY_WORKTREE: AppState['activeBrowserTabIdByWorktree'] = {} const EMPTY_BROWSER_TABS_BY_WORKTREE: AppState['browserTabsByWorktree'] = {} const EMPTY_BROWSER_PAGES_BY_WORKSPACE: AppState['browserPagesByWorkspace'] = {} -const EMPTY_AGENT_STATUS_BY_PANE_KEY: AppState['agentStatusByPaneKey'] = {} const EMPTY_LAYOUT_BY_WORKTREE: AppState['layoutByWorktree'] = {} let syncScheduled = false let syncInFlight = false @@ -179,6 +178,7 @@ export type RuntimeMobileSessionSyncKey = { activeFileIdByWorktree: AppState['activeFileIdByWorktree'] activeTabId: AppState['activeTabId'] activeBrowserTabIdByWorktree: AppState['activeBrowserTabIdByWorktree'] + agentStatusEpoch: number agentStatusByPaneKey: AppState['agentStatusByPaneKey'] // Why: these projections still need value-level inspection because the // underlying references churn even when the mobile-relevant shape is @@ -190,6 +190,32 @@ export type RuntimeMobileSessionSyncKey = { editorDraftsProjection: string } +export function canSkipRuntimeMobileSessionSyncKeyBuild( + state: AppState, + previousState: AppState +): boolean { + return ( + state.tabsByWorktree === previousState.tabsByWorktree && + state.groupsByWorktree === previousState.groupsByWorktree && + state.activeGroupIdByWorktree === previousState.activeGroupIdByWorktree && + state.layoutByWorktree === previousState.layoutByWorktree && + state.unifiedTabsByWorktree === previousState.unifiedTabsByWorktree && + state.tabBarOrderByWorktree === previousState.tabBarOrderByWorktree && + state.activeFileId === previousState.activeFileId && + state.activeFileIdByWorktree === previousState.activeFileIdByWorktree && + state.browserTabsByWorktree === previousState.browserTabsByWorktree && + state.browserPagesByWorkspace === previousState.browserPagesByWorkspace && + state.activeBrowserTabIdByWorktree === previousState.activeBrowserTabIdByWorktree && + state.openFiles === previousState.openFiles && + state.editorDrafts === previousState.editorDrafts && + state.activeTabId === previousState.activeTabId && + state.terminalLayoutsByTabId === previousState.terminalLayoutsByTabId && + state.runtimePaneTitlesByTabId === previousState.runtimePaneTitlesByTabId && + state.agentStatusEpoch === previousState.agentStatusEpoch && + state.agentStatusByPaneKey === previousState.agentStatusByPaneKey + ) +} + export function getRuntimeMobileSessionSyncKey( state: AppState, previousState?: AppState, @@ -218,9 +244,11 @@ export function getRuntimeMobileSessionSyncKey( activeTabId: state.activeTabId, activeBrowserTabIdByWorktree: state.activeBrowserTabIdByWorktree ?? EMPTY_ACTIVE_BROWSER_TAB_ID_BY_WORKTREE, - // Why: explicit hook status is published with terminal surfaces so paired - // web can render the same per-worktree agent rows before a PTY is opened. - agentStatusByPaneKey: state.agentStatusByPaneKey ?? EMPTY_AGENT_STATUS_BY_PANE_KEY, + // Why: paired web/mobile snapshots include full agentStatus details; the + // epoch keeps freshness timers cheap, while map identity preserves prompt + // and tool updates that do not change the visible state enum. + agentStatusEpoch: state.agentStatusEpoch ?? 0, + agentStatusByPaneKey: state.agentStatusByPaneKey, // Why: background agent title ticks can change runtimePaneTitlesByTabId // many times per second while the user types elsewhere. Reuse unchanged // projections so those ticks do not rescan all tabs, files, and drafts. @@ -367,6 +395,7 @@ export function runtimeMobileSessionSyncKeysEqual( a.activeFileIdByWorktree === b.activeFileIdByWorktree && a.activeTabId === b.activeTabId && a.activeBrowserTabIdByWorktree === b.activeBrowserTabIdByWorktree && + a.agentStatusEpoch === b.agentStatusEpoch && a.agentStatusByPaneKey === b.agentStatusByPaneKey && a.tabsProjection === b.tabsProjection && a.openFilesProjection === b.openFilesProjection && diff --git a/src/renderer/src/store/slices/agent-status.test.ts b/src/renderer/src/store/slices/agent-status.test.ts index 0ebbf111580..90b998c7cc1 100644 --- a/src/renderer/src/store/slices/agent-status.test.ts +++ b/src/renderer/src/store/slices/agent-status.test.ts @@ -148,7 +148,7 @@ describe('agent status tool + assistant fields', () => { expect(store.getState().agentStatusByPaneKey['tab-1:1'].agentType).toBe('cursor') }) - it('keeps global epochs stable for fresh same-state pings while updating the entry', () => { + it('keeps global epochs stable for fresh same-state working pings while updating the entry', () => { vi.useFakeTimers() const store = createTestStore() store @@ -177,8 +177,8 @@ describe('agent status tool + assistant fields', () => { 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. + // for state/freshness/final-done changes that can affect aggregate + // dashboard/sidebar calculations. expect(store.getState().agentStatusEpoch).toBe(firstEpoch) expect(store.getState().sortEpoch).toBe(firstSortEpoch) @@ -192,6 +192,39 @@ describe('agent status tool + assistant fields', () => { expect(store.getState().sortEpoch).toBe(firstSortEpoch + 1) }) + it('bumps the status epoch, not sort epoch, for same-state done updates', () => { + vi.useFakeTimers() + const store = createTestStore() + store + .getState() + .setAgentStatus('tab-1:1', { state: 'done', prompt: 'p1', agentType: 'claude' }, 'claude', { + updatedAt: 1_000, + stateStartedAt: 1_000 + }) + const firstEpoch = store.getState().agentStatusEpoch + const firstSortEpoch = store.getState().sortEpoch + + store.getState().setAgentStatus( + 'tab-1:1', + { + state: 'done', + prompt: 'p1', + agentType: 'claude', + lastAssistantMessage: 'final answer' + }, + 'claude', + { updatedAt: 2_000, stateStartedAt: 1_000 } + ) + + expect(store.getState().agentStatusByPaneKey['tab-1:1'].lastAssistantMessage).toBe( + 'final answer' + ) + // Why: retained rows need the final done snapshot, but done->done does not + // change smart-sort class, so only the status/retention epoch should tick. + expect(store.getState().agentStatusEpoch).toBe(firstEpoch + 1) + expect(store.getState().sortEpoch).toBe(firstSortEpoch) + }) + it('bumps global epochs when a stale same-state entry refreshes', () => { vi.useFakeTimers() const store = createTestStore() diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index d61aa81cc3e..b1c23f220c4 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -259,10 +259,12 @@ 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: + // an entry appears, changes state, crosses stale->fresh, or receives + // a same-state `done` update that may carry the final assistant + // message for retained rows. Same-state working prompt/tool 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 — smart-sort class is a function of state. // 2. Freshness transitions (stale → fresh) — `resolveAttention` in // smart-attention.ts filters entries through @@ -276,6 +278,11 @@ export const createAgentStatusSlice: StateCreator { expect(store.getState().browserAnnotationsByPageId[pageId]).toBeUndefined() }) + it('preserves browser map references when a page-state update is unchanged', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + const page = store.getState().browserPagesByWorkspace[tab.id]?.[0] + if (!page) { + throw new Error('Expected page state') + } + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + const browserTabsByWorktree = store.getState().browserTabsByWorktree + + store.getState().updateBrowserPageState(pageId, { + title: page.title, + loading: page.loading, + faviconUrl: page.faviconUrl, + canGoBack: page.canGoBack, + canGoForward: page.canGoForward, + loadError: page.loadError + }) + + expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace) + expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree) + }) + + it('repairs a stale active browser unified-tab label on an otherwise unchanged title update', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + store.setState({ + unifiedTabsByWorktree: { + 'wt-1': [ + { + id: 'unified-browser-tab', + entityId: tab.id, + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'browser', + label: 'Stale label', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + }) + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + const browserTabsByWorktree = store.getState().browserTabsByWorktree + + store.getState().updateBrowserPageState(pageId, { title: 'Example' }) + + expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Example') + expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace) + expect(store.getState().browserTabsByWorktree).toBe(browserTabsByWorktree) + }) + + it('repairs stale active browser workspace metadata on an otherwise unchanged page update', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + store.setState((state) => ({ + browserTabsByWorktree: { + ...state.browserTabsByWorktree, + 'wt-1': (state.browserTabsByWorktree['wt-1'] ?? []).map((workspace) => + workspace.id === tab.id + ? { + ...workspace, + title: 'Stale workspace', + url: 'https://stale.example.com', + loading: false, + canGoBack: true, + canGoForward: true + } + : workspace + ) + } + })) + const browserPagesByWorkspace = store.getState().browserPagesByWorkspace + + store.getState().updateBrowserPageState(pageId, { title: 'Example' }) + + const repaired = store + .getState() + .browserTabsByWorktree['wt-1']?.find((entry) => entry.id === tab.id) + expect(repaired).toMatchObject({ + title: 'Example', + url: 'https://example.com', + loading: true, + canGoBack: false, + canGoForward: false + }) + expect(store.getState().browserPagesByWorkspace).toBe(browserPagesByWorkspace) + }) + + it('updates the active browser unified-tab label without a second tab-label write', () => { + const store = createTestStore() + const tab = store.getState().createBrowserTab('wt-1', 'https://example.com', { + title: 'Example' + }) + const pageId = tab.activePageId + if (!pageId) { + throw new Error('Expected a new browser page') + } + store.setState({ + unifiedTabsByWorktree: { + 'wt-1': [ + { + id: 'unified-browser-tab', + entityId: tab.id, + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'browser', + label: 'Example', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + } + }) + + store.getState().updateBrowserPageState(pageId, { title: 'Next', loading: false }) + + expect(store.getState().unifiedTabsByWorktree['wt-1']?.[0]?.label).toBe('Next') + expect(store.getState().setTabLabel).not.toHaveBeenCalled() + }) + it('caps stored browser annotations per page', () => { const store = createTestStore() const tab = store.getState().createBrowserTab('wt-1', 'https://example.com') diff --git a/src/renderer/src/store/slices/browser.ts b/src/renderer/src/store/slices/browser.ts index 978dd4c9509..d3e687f92e9 100644 --- a/src/renderer/src/store/slices/browser.ts +++ b/src/renderer/src/store/slices/browser.ts @@ -317,6 +317,26 @@ function mirrorWorkspaceFromActivePage( } } +function browserWorkspaceMirrorFieldsEqual( + workspace: BrowserWorkspace, + mirrored: BrowserWorkspace +): boolean { + const workspacePageIds = workspace.pageIds ?? [] + const mirroredPageIds = mirrored.pageIds ?? [] + return ( + workspace.activePageId === mirrored.activePageId && + workspacePageIds.length === mirroredPageIds.length && + workspacePageIds.every((pageId, index) => pageId === mirroredPageIds[index]) && + workspace.url === mirrored.url && + workspace.title === mirrored.title && + workspace.loading === mirrored.loading && + workspace.faviconUrl === mirrored.faviconUrl && + workspace.canGoBack === mirrored.canGoBack && + workspace.canGoForward === mirrored.canGoForward && + workspace.loadError === mirrored.loadError + ) +} + function getFallbackTabTypeForWorktree( worktreeId: string, openFiles: AppState['openFiles'], @@ -335,26 +355,46 @@ function getFallbackTabTypeForWorktree( return 'terminal' } +const browserWorkspaceByIdCache = new WeakMap< + Record, + Map +>() +const browserPageByIdCache = new WeakMap, Map>() + function findWorkspace( browserTabsByWorktree: Record, workspaceId: string ): BrowserWorkspace | null { - return ( - Object.values(browserTabsByWorktree) - .flat() - .find((workspace) => workspace.id === workspaceId) ?? null - ) + const cached = browserWorkspaceByIdCache.get(browserTabsByWorktree) + if (cached) { + return cached.get(workspaceId) ?? null + } + const workspaceById = new Map() + for (const workspaces of Object.values(browserTabsByWorktree)) { + for (const workspace of workspaces) { + workspaceById.set(workspace.id, workspace) + } + } + browserWorkspaceByIdCache.set(browserTabsByWorktree, workspaceById) + return workspaceById.get(workspaceId) ?? null } function findPage( browserPagesByWorkspace: Record, pageId: string ): BrowserPage | null { - return ( - Object.values(browserPagesByWorkspace) - .flat() - .find((page) => page.id === pageId) ?? null - ) + const cached = browserPageByIdCache.get(browserPagesByWorkspace) + if (cached) { + return cached.get(pageId) ?? null + } + const pageById = new Map() + for (const pages of Object.values(browserPagesByWorkspace)) { + for (const page of pages) { + pageById.set(page.id, page) + } + } + browserPageByIdCache.set(browserPagesByWorkspace, pageById) + return pageById.get(pageId) ?? null } export const createBrowserSlice: StateCreator = (set, get) => ({ @@ -1094,24 +1134,67 @@ export const createBrowserSlice: StateCreator = if (!workspace) { return s } + const nextPage = { + ...page, + title: + updates.title === undefined ? page.title : normalizeBrowserTitle(updates.title, page.url), + loading: updates.loading ?? page.loading, + faviconUrl: updates.faviconUrl === undefined ? page.faviconUrl : updates.faviconUrl, + canGoBack: updates.canGoBack ?? page.canGoBack, + canGoForward: updates.canGoForward ?? page.canGoForward, + loadError: updates.loadError === undefined ? page.loadError : updates.loadError + } + const unifiedTabs = s.unifiedTabsByWorktree[workspace.worktreeId] ?? [] + const unifiedIndex = + workspace.activePageId === pageId && updates.title !== undefined + ? unifiedTabs.findIndex( + (entry) => entry.contentType === 'browser' && entry.entityId === workspace.id + ) + : -1 + const unifiedLabelNeedsRepair = + unifiedIndex !== -1 && unifiedTabs[unifiedIndex]?.label !== nextPage.title + const pageStateUnchanged = + nextPage.title === page.title && + nextPage.loading === page.loading && + nextPage.faviconUrl === page.faviconUrl && + nextPage.canGoBack === page.canGoBack && + nextPage.canGoForward === page.canGoForward && + nextPage.loadError === page.loadError + const currentPages = s.browserPagesByWorkspace[workspace.id] ?? [] + const mirroredWorkspace = pageStateUnchanged + ? mirrorWorkspaceFromActivePage(workspace, currentPages) + : null + const workspaceNeedsRepair = + mirroredWorkspace !== null && + !browserWorkspaceMirrorFieldsEqual(workspace, mirroredWorkspace) + if (pageStateUnchanged && !unifiedLabelNeedsRepair && !workspaceNeedsRepair) { + return s + } + if (pageStateUnchanged) { + const nextState: Partial = {} + if (workspaceNeedsRepair && mirroredWorkspace) { + nextState.browserTabsByWorktree = { + ...s.browserTabsByWorktree, + [workspace.worktreeId]: (s.browserTabsByWorktree[workspace.worktreeId] ?? []).map( + (tab) => (tab.id === workspace.id ? mirroredWorkspace : tab) + ) + } + } + if (unifiedLabelNeedsRepair) { + nextState.unifiedTabsByWorktree = { + ...s.unifiedTabsByWorktree, + [workspace.worktreeId]: unifiedTabs.map((entry, index) => + index === unifiedIndex ? { ...entry, label: nextPage.title } : entry + ) + } + } + return nextState + } const nextPages = (s.browserPagesByWorkspace[workspace.id] ?? []).map((entry) => - entry.id === pageId - ? { - ...entry, - title: - updates.title === undefined - ? entry.title - : normalizeBrowserTitle(updates.title, entry.url), - loading: updates.loading ?? entry.loading, - faviconUrl: updates.faviconUrl === undefined ? entry.faviconUrl : updates.faviconUrl, - canGoBack: updates.canGoBack ?? entry.canGoBack, - canGoForward: updates.canGoForward ?? entry.canGoForward, - loadError: updates.loadError === undefined ? entry.loadError : updates.loadError - } - : entry + entry.id === pageId ? nextPage : entry ) const nextWorkspace = mirrorWorkspaceFromActivePage(workspace, nextPages) - return { + const nextState: Partial = { browserPagesByWorkspace: { ...s.browserPagesByWorkspace, [workspace.id]: nextPages @@ -1123,19 +1206,18 @@ export const createBrowserSlice: StateCreator = ) } } + if (workspace.activePageId === pageId && updates.title !== undefined && unifiedIndex !== -1) { + if (unifiedLabelNeedsRepair || unifiedTabs[unifiedIndex]?.label !== nextWorkspace.title) { + nextState.unifiedTabsByWorktree = { + ...s.unifiedTabsByWorktree, + [workspace.worktreeId]: unifiedTabs.map((entry, index) => + index === unifiedIndex ? { ...entry, label: nextWorkspace.title } : entry + ) + } + } + } + return nextState }) - - const page = findPage(get().browserPagesByWorkspace, pageId) - if (!page) { - return - } - const workspace = findWorkspace(get().browserTabsByWorktree, page.workspaceId) - const item = Object.values(get().unifiedTabsByWorktree) - .flat() - .find((entry) => entry.contentType === 'browser' && entry.entityId === page.workspaceId) - if (item && workspace && workspace.activePageId === pageId && updates.title) { - get().setTabLabel(item.id, workspace.title) - } }, setBrowserTabUrl: (pageId, url) => get().setBrowserPageUrl(pageId, url), diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index db1b6a60774..860d8559558 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -2111,6 +2111,78 @@ describe('createGitHubSlice.refreshGitHubForWorktreeIfStale', () => { expect(mockApi.gh.enqueuePRRefresh).not.toHaveBeenCalled() }) + it('does not fetch linked issue details when the issue card section is hidden', async () => { + const store = createTestStore() + const repoPath = '/repo' + const branch = 'feature/test' + const worktreeId = 'wt-1' + + store.setState({ + repos: [{ id: 'repo-1', path: repoPath, name: 'repo', kind: 'git' }], + groupBy: 'repo', + worktreeCardProperties: ['comment'], + rightSidebarOpen: false, + worktreesByRepo: { + 'repo-1': [ + { + id: worktreeId, + repoId: 'repo-1', + path: '/repo/worktrees/test', + branch, + displayName: 'test', + isMainWorktree: false, + isBare: false, + isArchived: false, + linkedIssue: 123 + } + ] + } + } as unknown as Partial) + + store.getState().refreshGitHubForWorktreeIfStale(worktreeId) + await Promise.resolve() + + expect(mockApi.gh.issue).not.toHaveBeenCalled() + }) + + it('fetches linked issue details when the issue card section is visible', async () => { + const store = createTestStore() + const repoPath = '/repo' + const branch = 'feature/test' + const worktreeId = 'wt-1' + + store.setState({ + repos: [{ id: 'repo-1', path: repoPath, name: 'repo', kind: 'git' }], + groupBy: 'repo', + worktreeCardProperties: ['issue'], + rightSidebarOpen: false, + worktreesByRepo: { + 'repo-1': [ + { + id: worktreeId, + repoId: 'repo-1', + path: '/repo/worktrees/test', + branch, + displayName: 'test', + isMainWorktree: false, + isBare: false, + isArchived: false, + linkedIssue: 123 + } + ] + } + } as unknown as Partial) + + store.getState().refreshGitHubForWorktreeIfStale(worktreeId) + await Promise.resolve() + + expect(mockApi.gh.issue).toHaveBeenCalledWith({ + repoPath, + repoId: 'repo-1', + number: 123 + }) + }) + it('enqueues active PR refresh IPC for connected SSH-backed repos', () => { const store = createTestStore() const repoPath = '/repo' @@ -2398,6 +2470,78 @@ describe('createGitHubSlice.refreshAllGitHub', () => { timeoutMs: 30_000 }) }) + + it('does not refresh stale linked issues when the issue card section is hidden', async () => { + const store = createTestStore() + const repoPath = '/repo' + const branch = 'feature/test' + + store.setState({ + repos: [{ id: 'repo-1', path: repoPath, name: 'repo', kind: 'git' }], + groupBy: 'repo', + worktreeCardProperties: ['comment'], + rightSidebarOpen: false, + worktreesByRepo: { + 'repo-1': [ + { + id: 'wt-1', + repoId: 'repo-1', + path: '/repo/worktrees/test', + branch, + displayName: 'test', + isMainWorktree: false, + isBare: false, + isArchived: false, + lastActivityAt: 1, + linkedIssue: 123 + } + ] + } + } as unknown as Partial) + + store.getState().refreshAllGitHub() + await Promise.resolve() + + expect(mockApi.gh.issue).not.toHaveBeenCalled() + }) + + it('refreshes stale linked issues when the issue card section is visible', async () => { + const store = createTestStore() + const repoPath = '/repo' + const branch = 'feature/test' + + store.setState({ + repos: [{ id: 'repo-1', path: repoPath, name: 'repo', kind: 'git' }], + groupBy: 'repo', + worktreeCardProperties: ['issue'], + rightSidebarOpen: false, + worktreesByRepo: { + 'repo-1': [ + { + id: 'wt-1', + repoId: 'repo-1', + path: '/repo/worktrees/test', + branch, + displayName: 'test', + isMainWorktree: false, + isBare: false, + isArchived: false, + lastActivityAt: 1, + linkedIssue: 123 + } + ] + } + } as unknown as Partial) + + store.getState().refreshAllGitHub() + await Promise.resolve() + + expect(mockApi.gh.issue).toHaveBeenCalledWith({ + repoPath, + repoId: 'repo-1', + number: 123 + }) + }) }) describe('createGitHubSlice.refreshGitHubForWorktree', () => { diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 9b902bdf909..d0f9e17849c 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -930,6 +930,10 @@ function evictStaleEntries( return pruned } +function shouldRefreshIssueDecorations(state: AppState): boolean { + return (state.worktreeCardProperties ?? []).includes('issue') +} + let saveTimer: ReturnType | null = null function debouncedSaveCache(state: AppState): void { @@ -2497,6 +2501,7 @@ export const createGitHubSlice: StateCreator = (s const now = Date.now() const stalePRCandidates: { candidate: GitHubPRRefreshCandidate; score: number }[] = [] const cardProps = state.worktreeCardProperties ?? [] + const shouldRefreshIssues = shouldRefreshIssueDecorations(state) const isPRStatusGrouping = state.groupBy === 'pr-status' const rightSidebarShowsPR = state.rightSidebarOpen && @@ -2530,7 +2535,7 @@ export const createGitHubSlice: StateCreator = (s } } } - if (wt.linkedIssue) { + if (shouldRefreshIssues && wt.linkedIssue) { const issueKey = repoScopedCacheKey(repo.path, repo.id, String(wt.linkedIssue)) const issueEntry = state.issueCache[issueKey] if (!issueEntry || now - issueEntry.fetchedAt >= CACHE_TTL) { @@ -2612,7 +2617,7 @@ export const createGitHubSlice: StateCreator = (s } } } - if (worktree.linkedIssue) { + if (shouldRefreshIssueDecorations(state) && worktree.linkedIssue) { void get().fetchIssue(repo.path, worktree.linkedIssue, { repoId: repo.id }) } }, @@ -2795,7 +2800,7 @@ export const createGitHubSlice: StateCreator = (s } } - if (worktree.linkedIssue) { + if (shouldRefreshIssueDecorations(state) && worktree.linkedIssue) { const issueKey = repoScopedCacheKey(repo.path, repo.id, String(worktree.linkedIssue)) const issueEntry = state.issueCache[issueKey] if (!issueEntry || now - issueEntry.fetchedAt >= CACHE_TTL) { diff --git a/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts b/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts index 8f39b0db3b6..c09129ba596 100644 --- a/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts +++ b/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts @@ -59,6 +59,32 @@ describe('runtimePaneTitle → sortEpoch', () => { expect(store.getState().sortEpoch).toBe(baseline) }) + it('does not enumerate terminal tabs when the classification is unchanged', () => { + const store = createTestStore() + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: 'wt-bg', repoId: 'repo1', path: '/path/wt-bg' })] + }, + tabsByWorktree: { + 'wt-bg': [makeTab({ id: 'tab-1', worktreeId: 'wt-bg' })] + } + }) + store.getState().setRuntimePaneTitle('tab-1', 1, '⠋ Claude') + const baseline = store.getState().sortEpoch + store.setState({ + tabsByWorktree: new Proxy(store.getState().tabsByWorktree, { + ownKeys() { + throw new Error('tabsByWorktree should not be enumerated') + } + }) + }) + + store.getState().setRuntimePaneTitle('tab-1', 1, '⠙ Claude') + + expect(store.getState().runtimePaneTitlesByTabId['tab-1']?.[1]).toBe('⠙ Claude') + expect(store.getState().sortEpoch).toBe(baseline) + }) + it('bumps sortEpoch when clearing a classified title back to none', () => { const store = createTestStore() seedStore(store, { @@ -75,6 +101,32 @@ describe('runtimePaneTitle → sortEpoch', () => { expect(store.getState().sortEpoch).toBeGreaterThan(baseline) }) + it('does not enumerate terminal tabs when clearing an unclassified title', () => { + const store = createTestStore() + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: 'wt-bg', repoId: 'repo1', path: '/path/wt-bg' })] + }, + tabsByWorktree: { + 'wt-bg': [makeTab({ id: 'tab-1', worktreeId: 'wt-bg' })] + } + }) + store.getState().setRuntimePaneTitle('tab-1', 1, 'shell prompt') + const baseline = store.getState().sortEpoch + store.setState({ + tabsByWorktree: new Proxy(store.getState().tabsByWorktree, { + ownKeys() { + throw new Error('tabsByWorktree should not be enumerated') + } + }) + }) + + store.getState().clearRuntimePaneTitle('tab-1', 1) + + expect(store.getState().runtimePaneTitlesByTabId['tab-1']).toBeUndefined() + expect(store.getState().sortEpoch).toBe(baseline) + }) + it('does not bump sortEpoch when the changing pane belongs to the active worktree (set)', () => { // Why: clicking a slept worktree wakes it; the PTY remount briefly // reclassifies its title, which must NOT re-rank the active worktree. diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index 69a3110c944..d3927b59d61 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -1358,6 +1358,29 @@ describe('setActiveWorktree', () => { }) }) + it('preserves terminal and unified tab map references when a live title repeats', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + } + }) + + const first = store.getState().createTab(wt) + store.getState().updateTabTitle(first.id, 'Claude Code') + const tabsByWorktree = store.getState().tabsByWorktree + const unifiedTabsByWorktree = store.getState().unifiedTabsByWorktree + const sortEpoch = store.getState().sortEpoch + + store.getState().updateTabTitle(first.id, 'Claude Code') + + expect(store.getState().tabsByWorktree).toBe(tabsByWorktree) + expect(store.getState().unifiedTabsByWorktree).toBe(unifiedTabsByWorktree) + expect(store.getState().sortEpoch).toBe(sortEpoch) + }) + it('clears stale background browser tab type when closing the last browser tab', () => { const store = createTestStore() const wt = 'repo1::/path/wt1' diff --git a/src/renderer/src/store/slices/tab-group-state.ts b/src/renderer/src/store/slices/tab-group-state.ts index b5d634cee61..5a7c7669433 100644 --- a/src/renderer/src/store/slices/tab-group-state.ts +++ b/src/renderer/src/store/slices/tab-group-state.ts @@ -208,6 +208,12 @@ export function patchTab( if (!found) { return null } + const patchChangesTab = (Object.keys(patch) as (keyof Tab)[]).some( + (key) => found.tab[key] !== patch[key] + ) + if (!patchChangesTab) { + return null + } const { worktreeId } = found const tabs = tabsByWorktree[worktreeId] ?? [] return { diff --git a/src/renderer/src/store/slices/tabs.test.ts b/src/renderer/src/store/slices/tabs.test.ts index 3a50d3d8bbf..0401006f79d 100644 --- a/src/renderer/src/store/slices/tabs.test.ts +++ b/src/renderer/src/store/slices/tabs.test.ts @@ -973,6 +973,16 @@ describe('TabsSlice', () => { expect(store.getState().unifiedTabsByWorktree[WT][0].label).toBe('zsh') }) + it('setTabLabel preserves tab map references when the label is unchanged', () => { + const tab = store.getState().createUnifiedTab(WT, 'terminal') + store.getState().setTabLabel(tab.id, 'zsh') + const before = store.getState().unifiedTabsByWorktree + + store.getState().setTabLabel(tab.id, 'zsh') + + expect(store.getState().unifiedTabsByWorktree).toBe(before) + }) + it('setTabCustomLabel updates customLabel', () => { const tab = store.getState().createUnifiedTab(WT, 'terminal') store.getState().setTabCustomLabel(tab.id, 'my-term') diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 5bb9b503e38..46a864c5f5f 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -3,6 +3,7 @@ import type { StateCreator } from 'zustand' import type { AppState } from '../types' import type { SetupSplitDirection, + Tab, TerminalLayoutSnapshot, TerminalTab, Worktree, @@ -67,6 +68,40 @@ function getFallbackTabTitle(tab: TerminalTab, index?: number): string { ) } +let terminalTabOwnerCacheSource: Record | null = null +let terminalTabOwnerCache = new Map() + +function getTerminalTabOwnerWorktreeId( + tabsByWorktree: Record, + tabId: string +): string | null { + if (terminalTabOwnerCacheSource !== tabsByWorktree) { + const nextCache = new Map() + for (const [worktreeId, tabs] of Object.entries(tabsByWorktree)) { + for (const tab of tabs) { + nextCache.set(tab.id, worktreeId) + } + } + terminalTabOwnerCacheSource = tabsByWorktree + terminalTabOwnerCache = nextCache + } + return terminalTabOwnerCache.get(tabId) ?? null +} + +function updateUnifiedTerminalLabel( + unifiedTabs: Tab[], + terminalTabId: string, + label: string +): Tab[] | null { + const unifiedIndex = unifiedTabs.findIndex( + (entry) => entry.contentType === 'terminal' && entry.entityId === terminalTabId + ) + if (unifiedIndex === -1 || unifiedTabs[unifiedIndex]?.label === label) { + return null + } + return unifiedTabs.map((entry, index) => (index === unifiedIndex ? { ...entry, label } : entry)) +} + function isWindowsRendererRuntime(): boolean { return typeof navigator !== 'undefined' && navigator.userAgent.includes('Windows') } @@ -812,38 +847,47 @@ export const createTerminalSlice: StateCreator // unchanged) would break shallow-equality checks in unrelated // selectors and trigger spurious re-renders across background // worktrees on every OSC title frame. - let ownerWorktreeId: string | null = null - let ownerTabs: TerminalTab[] | null = null - for (const [wId, tabs] of Object.entries(s.tabsByWorktree)) { - const idx = tabs.findIndex((t) => t.id === tabId) - if (idx === -1) { - continue - } - const t = tabs[idx] - const nextTitle = title.trim() || getFallbackTabTitle(t) - if (t.title === nextTitle) { - return s - } - ownerWorktreeId = wId - ownerTabs = tabs.map((tab) => - tab.id === tabId - ? { - ...tab, - // Why: PTYs can briefly emit an empty title while an agent exits. - // Keep the stable fallback label instead of rendering a blank tab. - title: nextTitle, - defaultTitle: - tab.defaultTitle ?? - (/^Terminal \d+$/.test(tab.title) ? tab.title : undefined) ?? - (/^Terminal \d+$/.test(nextTitle) ? nextTitle : undefined) - } - : tab - ) - break - } - if (!ownerWorktreeId || !ownerTabs) { + const ownerWorktreeId = getTerminalTabOwnerWorktreeId(s.tabsByWorktree, tabId) + if (!ownerWorktreeId) { return s } + const tabs = s.tabsByWorktree[ownerWorktreeId] ?? [] + const tabIndex = tabs.findIndex((t) => t.id === tabId) + const currentTab = tabs[tabIndex] + if (!currentTab) { + return s + } + const nextTitle = title.trim() || getFallbackTabTitle(currentTab) + const currentUnifiedTabs = s.unifiedTabsByWorktree[ownerWorktreeId] ?? [] + const unifiedTabsWithUpdatedLabel = updateUnifiedTerminalLabel( + currentUnifiedTabs, + tabId, + nextTitle + ) + if (currentTab.title === nextTitle) { + return unifiedTabsWithUpdatedLabel + ? { + unifiedTabsByWorktree: { + ...s.unifiedTabsByWorktree, + [ownerWorktreeId]: unifiedTabsWithUpdatedLabel + } + } + : s + } + const ownerTabs = tabs.map((tab) => + tab.id === tabId + ? { + ...tab, + // Why: PTYs can briefly emit an empty title while an agent exits. + // Keep the stable fallback label instead of rendering a blank tab. + title: nextTitle, + defaultTitle: + tab.defaultTitle ?? + (/^Terminal \d+$/.test(tab.title) ? tab.title : undefined) ?? + (/^Terminal \d+$/.test(nextTitle) ? nextTitle : undefined) + } + : tab + ) scheduleRuntimeGraphSync() const nextTabsByWorktree = { ...s.tabsByWorktree, [ownerWorktreeId]: ownerTabs } // Agent status is derived from terminal titles and affects sort scoring, @@ -854,20 +898,17 @@ export const createTerminalSlice: StateCreator // title update). Bumping sortEpoch here would reorder the sidebar // on click — the exact bug PR #209 intended to fix. const isActive = ownerWorktreeId === s.activeWorktreeId - return isActive + const nextState: Partial = isActive ? { tabsByWorktree: nextTabsByWorktree } : { tabsByWorktree: nextTabsByWorktree, sortEpoch: s.sortEpoch + 1 } + if (unifiedTabsWithUpdatedLabel) { + nextState.unifiedTabsByWorktree = { + ...s.unifiedTabsByWorktree, + [ownerWorktreeId]: unifiedTabsWithUpdatedLabel + } + } + return nextState }) - const item = Object.values(get().unifiedTabsByWorktree) - .flat() - .find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId) - if (item) { - const resolvedTitle = - Object.values(get().tabsByWorktree) - .flat() - .find((tab) => tab.id === tabId)?.title ?? title.trim() - get().setTabLabel(item.id, resolvedTitle) - } }, setRuntimePaneTitle: (tabId, paneId, title) => { @@ -892,13 +933,9 @@ export const createTerminalSlice: StateCreator // re-emits its working title) — bumping would re-rank the sidebar on // click, the exact bug PR #209 fixed for updateTabTitle. If no owner // is found the pane is orphaned; skip the bump as unsafe. - let ownerWorktreeId: string | null = null - for (const [wId, tabs] of Object.entries(s.tabsByWorktree)) { - if (tabs.some((t) => t.id === tabId)) { - ownerWorktreeId = wId - break - } - } + const ownerWorktreeId = classificationChanged + ? getTerminalTabOwnerWorktreeId(s.tabsByWorktree, tabId) + : null const isActive = ownerWorktreeId !== null && ownerWorktreeId === s.activeWorktreeId const shouldBump = classificationChanged && ownerWorktreeId !== null && !isActive return { @@ -936,13 +973,9 @@ export const createTerminalSlice: StateCreator // fire as a side-effect of a click-driven PTY teardown in the active // worktree must not re-rank the sidebar. Skip bumping when no owner is // found (orphaned pane) for the same safety reason. - let ownerWorktreeId: string | null = null - for (const [wId, tabs] of Object.entries(s.tabsByWorktree)) { - if (tabs.some((t) => t.id === tabId)) { - ownerWorktreeId = wId - break - } - } + const ownerWorktreeId = hadClassification + ? getTerminalTabOwnerWorktreeId(s.tabsByWorktree, tabId) + : null const isActive = ownerWorktreeId !== null && ownerWorktreeId === s.activeWorktreeId const shouldBump = hadClassification && ownerWorktreeId !== null && !isActive return { diff --git a/tests/e2e/terminal-typing-latency.spec.ts b/tests/e2e/terminal-typing-latency.spec.ts new file mode 100644 index 00000000000..3b0610f09d5 --- /dev/null +++ b/tests/e2e/terminal-typing-latency.spec.ts @@ -0,0 +1,142 @@ +import type { Page } from '@stablyai/playwright-test' +import { rmSync, writeFileSync } from 'fs' +import path from 'path' +import { test, expect } from './helpers/orca-app' +import { + getTerminalContent, + waitForActivePanePtyId, + waitForActiveTerminalManager, + waitForTerminalOutput, + sendToTerminal +} from './helpers/terminal' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +const KEY_LATENCY_SAMPLES = 'abcdefghijklmnop' +const MAX_MEDIAN_KEY_LATENCY_MS = 250 +const MAX_WORST_KEY_LATENCY_MS = 1_000 + +async function focusActiveTerminalInput(page: Page): Promise { + await page.evaluate(() => { + const state = window.__store?.getState() + const worktreeId = state?.activeWorktreeId + const tabId = + state?.activeTabType === 'terminal' + ? state.activeTabId + : worktreeId + ? (state?.activeTabIdByWorktree?.[worktreeId] ?? null) + : null + const manager = tabId ? window.__paneManagers?.get(tabId) : null + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + if (!pane) { + throw new Error('No active terminal pane to focus') + } + pane.terminal.focus() + const textarea = pane.container.querySelector( + '.xterm-helper-textarea' + ) as HTMLTextAreaElement | null + if (!textarea) { + throw new Error('Active terminal has no xterm helper textarea') + } + textarea.focus() + }) +} + +function codexLikePromptScript(runId: string): string { + return ` +process.stdin.setEncoding('utf8') +if (process.stdin.isTTY) process.stdin.setRawMode(true) +process.stdin.resume() +let seq = 0 +const interrupt = String.fromCharCode(3) +process.stdout.write('\\x1b]0;Codex typing benchmark\\x07') +process.stdout.write('CODEX_TYPING_READY_${runId}\\n') +process.stdin.on('data', (chunk) => { + if (chunk.includes(interrupt)) { + process.exit(0) + } + for (const char of chunk) { + if (char === '\\r' || char === '\\n') continue + seq += 1 + process.stdout.write('\\r\\x1b[2KCodex prompt ' + seq + ': ' + char + ' CODEX_KEY_${runId}_' + seq + '\\n') + } +}) +` +} + +async function waitForMarkerLatency( + page: Page, + marker: string, + timeoutMs: number +): Promise { + const start = performance.now() + while (performance.now() - start < timeoutMs) { + if ((await getTerminalContent(page, 12_000)).includes(marker)) { + return performance.now() - start + } + await page.waitForTimeout(5) + } + throw new Error(`Timed out waiting for terminal marker ${marker}`) +} + +function median(values: number[]): number { + const sorted = [...values].sort((a, b) => a - b) + return sorted[Math.floor(sorted.length / 2)] ?? 0 +} + +test.describe('Terminal typing latency', () => { + test('Codex-like interactive prompt echoes typed keys without visible lag', async ({ + orcaPage, + testRepoPath + }, testInfo) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + + const ptyId = await waitForActivePanePtyId(orcaPage) + const runId = String(Date.now()) + const scriptPath = path.join(testRepoPath, `.orca-typing-benchmark-${runId}.mjs`) + writeFileSync(scriptPath, codexLikePromptScript(runId)) + let commandSent = false + try { + await sendToTerminal(orcaPage, ptyId, `node ${JSON.stringify(scriptPath)}\r`) + commandSent = true + await waitForTerminalOutput(orcaPage, `CODEX_TYPING_READY_${runId}`, 10_000) + await focusActiveTerminalInput(orcaPage) + + const latencies: number[] = [] + for (const [index, char] of [...KEY_LATENCY_SAMPLES].entries()) { + const seq = index + 1 + const marker = `CODEX_KEY_${runId}_${seq}` + const start = performance.now() + await orcaPage.keyboard.type(char) + await waitForMarkerLatency(orcaPage, marker, MAX_WORST_KEY_LATENCY_MS) + latencies.push(performance.now() - start) + } + + const medianLatency = median(latencies) + const worstLatency = Math.max(...latencies) + testInfo.annotations.push({ + type: 'terminal-typing-latency', + description: `median=${medianLatency.toFixed(1)}ms worst=${worstLatency.toFixed(1)}ms samples=${latencies + .map((value) => value.toFixed(1)) + .join(',')}` + }) + console.info( + `[terminal-typing-latency] median=${medianLatency.toFixed( + 1 + )}ms worst=${worstLatency.toFixed(1)}ms samples=${latencies + .map((value) => value.toFixed(1)) + .join(',')}` + ) + + expect(medianLatency).toBeLessThan(MAX_MEDIAN_KEY_LATENCY_MS) + expect(worstLatency).toBeLessThan(MAX_WORST_KEY_LATENCY_MS) + } finally { + if (commandSent) { + await sendToTerminal(orcaPage, ptyId, '\x03').catch(() => undefined) + } + rmSync(scriptPath, { force: true }) + } + }) +})