From dbeb89436aec12e0ec03c2ba0a656a10f6730786 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 14 May 2026 01:22:42 -0700 Subject: [PATCH] Reduce serial startup waits before first window (#1823) * Avoid local scrollback serialization on shutdown * Reduce serial startup waits before first window --- src/main/index.ts | 50 +++++++------ .../first-window-startup-services.test.ts | 58 +++++++++++++++ .../startup/first-window-startup-services.ts | 23 ++++++ src/renderer/src/App.tsx | 2 +- .../components/terminal-pane/TerminalPane.tsx | 17 +++-- .../terminal-pane/shutdown-buffer-captures.ts | 9 ++- .../terminal-shutdown-layout-capture.test.ts | 42 ++++++++++- .../terminal-shutdown-layout-capture.ts | 72 ++++++++++--------- ...workspace-session-terminal-buffers.test.ts | 25 ++++++- .../workspace-session-terminal-buffers.ts | 55 ++++++++------ 10 files changed, 263 insertions(+), 90 deletions(-) create mode 100644 src/main/startup/first-window-startup-services.test.ts create mode 100644 src/main/startup/first-window-startup-services.ts diff --git a/src/main/index.ts b/src/main/index.ts index 2f16305dd..0e3abf55f 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -34,6 +34,7 @@ import { installUncaughtPipeErrorGuard, patchPackagedProcessPath } from './startup/configure-process' +import { startFirstWindowStartupServices } from './startup/first-window-startup-services' import { hydrateShellPath, mergePathSegments } from './startup/hydrate-shell-path' import { acquireSingleInstanceLock } from './startup/single-instance-lock' import { RateLimitService } from './rate-limits/service' @@ -611,32 +612,29 @@ app.whenReady().then(async () => { }) registerMobileHandlers(runtimeRpc) - // Why: the persistent-terminal daemon is always started. If it fails, the - // LocalPtyProvider (initialized at module load in ipc/pty.ts) remains as the - // implicit fallback — terminals work, just without cross-restart persistence. - try { - await initDaemonPtyProvider() - } catch (error) { - console.error('[daemon] Failed to start daemon PTY provider, falling back to local:', error) - } - // Why: PTY spawn env reads ORCA_AGENT_HOOK_* from the live server state, - // so the hook server must start before the window opens — otherwise - // restored terminals race ahead without the env on first launch. - try { - await agentHookServer.start({ - env: app.isPackaged ? 'production' : 'development', - // Why: passing the userData path lets the server write its endpoint - // file (PORT/TOKEN/ENV/VERSION) to a stable location. Hook scripts - // source that file at invocation time so they reach the current Orca - // even when the PTY's env was frozen under a prior instance. - userDataPath: app.getPath('userData') - }) - } catch (error) { - // Why: Claude/Codex/Gemini/OpenCode/Cursor hook callbacks are sidebar - // enrichment only. Orca must still boot even if the local loopback - // receiver cannot bind on this launch. - console.error('[agent-hooks] Failed to start local hook server:', error) - } + await startFirstWindowStartupServices({ + // Why: the persistent-terminal daemon is always started. If it fails, the + // LocalPtyProvider remains as the implicit fallback — terminals work, just + // without cross-restart persistence. + startDaemonPtyProvider: () => initDaemonPtyProvider(), + // Why: PTY spawn env reads ORCA_AGENT_HOOK_* from the live server state, + // so the hook server must start before restored terminals can mount. + startAgentHookServer: () => + agentHookServer.start({ + env: app.isPackaged ? 'production' : 'development', + // Why: hooks source this endpoint file at invocation time, so old PTY + // env still reaches the current Orca process after an app restart. + userDataPath: app.getPath('userData') + }), + onDaemonError: (error) => { + console.error('[daemon] Failed to start daemon PTY provider, falling back to local:', error) + }, + onAgentHookServerError: (error) => { + // Why: Claude/Codex/Gemini/OpenCode/Cursor hook callbacks are sidebar + // enrichment only. Orca must still boot if the loopback receiver fails. + console.error('[agent-hooks] Failed to start local hook server:', error) + } + }) // Why: once the hook server is ready (or has already failed open), window // creation and runtime RPC startup are independent. diff --git a/src/main/startup/first-window-startup-services.test.ts b/src/main/startup/first-window-startup-services.test.ts new file mode 100644 index 000000000..ab93eb304 --- /dev/null +++ b/src/main/startup/first-window-startup-services.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it, vi } from 'vitest' +import { startFirstWindowStartupServices } from './first-window-startup-services' + +describe('startFirstWindowStartupServices', () => { + it('starts daemon and hook services concurrently before awaiting either', async () => { + const events: string[] = [] + let resolveDaemon!: () => void + let resolveHooks!: () => void + + const started = startFirstWindowStartupServices({ + startDaemonPtyProvider: () => + new Promise((resolve) => { + events.push('daemon-started') + resolveDaemon = resolve + }), + startAgentHookServer: () => + new Promise((resolve) => { + events.push('hooks-started') + resolveHooks = resolve + }), + onDaemonError: vi.fn(), + onAgentHookServerError: vi.fn() + }) + + await Promise.resolve() + expect(events).toEqual(['daemon-started', 'hooks-started']) + + let completed = false + started.then(() => { + completed = true + }) + + resolveDaemon() + await Promise.resolve() + expect(completed).toBe(false) + + resolveHooks() + await started + expect(completed).toBe(true) + }) + + it('logs each service failure and still resolves the startup barrier', async () => { + const onDaemonError = vi.fn() + const onAgentHookServerError = vi.fn() + + await expect( + startFirstWindowStartupServices({ + startDaemonPtyProvider: () => Promise.reject(new Error('daemon failed')), + startAgentHookServer: () => Promise.reject(new Error('hooks failed')), + onDaemonError, + onAgentHookServerError + }) + ).resolves.toBeUndefined() + + expect(onDaemonError).toHaveBeenCalledWith(expect.any(Error)) + expect(onAgentHookServerError).toHaveBeenCalledWith(expect.any(Error)) + }) +}) diff --git a/src/main/startup/first-window-startup-services.ts b/src/main/startup/first-window-startup-services.ts new file mode 100644 index 000000000..9a6de0783 --- /dev/null +++ b/src/main/startup/first-window-startup-services.ts @@ -0,0 +1,23 @@ +type FirstWindowStartupServices = { + startDaemonPtyProvider: () => Promise + startAgentHookServer: () => Promise + onDaemonError: (error: unknown) => void + onAgentHookServerError: (error: unknown) => void +} + +/** + * Starts the services that must be ready before restored terminal panes mount. + */ +export async function startFirstWindowStartupServices({ + startDaemonPtyProvider, + startAgentHookServer, + onDaemonError, + onAgentHookServerError +}: FirstWindowStartupServices): Promise { + // Why: daemon startup and hook-server binding are independent, but both gate + // restored terminals; run them together so cold-start latency is max(), not sum(). + await Promise.all([ + startDaemonPtyProvider().catch(onDaemonError), + startAgentHookServer().catch(onAgentHookServerError) + ]) +} diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 24d5f9c6c..197abee91 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -560,7 +560,7 @@ function App(): React.JSX.Element { } for (const capture of shutdownBufferCaptures.values()) { try { - capture() + capture({ includeLocalBuffers: false }) } catch { // Don't let one pane's failure block the rest. } diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index b7da79b1b..a07ac5826 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -33,6 +33,7 @@ import { useTerminalPaneLifecycle } from './use-terminal-pane-lifecycle' import { useTerminalPaneContextMenu } from './use-terminal-pane-context-menu' import { useNotificationDispatch } from './use-notification-dispatch' import { connectPanePty } from './pty-connection' +import { shouldPreserveTerminalScrollbackBuffers } from '../../../../shared/workspace-session-terminal-buffers' import { getFitOverrideForPty, getPaneIdsForPty, @@ -905,7 +906,7 @@ export default function TerminalPane({ // Register a capture callback for shutdown. The beforeunload handler in // App.tsx calls all registered callbacks to serialize terminal buffers. useEffect(() => { - const captureBuffers = (): void => { + const captureBuffers = (options?: { includeLocalBuffers?: boolean }): void => { const manager = managerRef.current const container = containerRef.current if (!manager || !container) { @@ -921,14 +922,22 @@ export default function TerminalPane({ // bytes. Without preservation, that empty pass would wipe a known-good // buffer. Merge prior state in for leaves whose live capture came back // empty. Same shape as persistLayoutSnapshot. - const existing = useAppStore.getState().terminalLayoutsByTabId[tabId] + const state = useAppStore.getState() + const existing = state.terminalLayoutsByTabId[tabId] + const includeLocalBuffers = options?.includeLocalBuffers ?? true + const shouldCaptureScrollbackBuffers = includeLocalBuffers + ? true + : shouldPreserveTerminalScrollbackBuffers(worktreeId, state.repos) const layout = captureTerminalShutdownLayout({ manager, container, expandedPaneId: expandedPaneIdRef.current, paneTransports: paneTransportsRef.current, paneTitlesByPaneId: paneTitlesRef.current, - existingLayout: existing + existingLayout: existing, + // Why: beforeunload skips local/floating bytes because session payloads + // immediately prune them; worktree sleep keeps them as defense-in-depth. + captureBuffers: shouldCaptureScrollbackBuffers }) setTabLayout(tabId, layout) } @@ -940,7 +949,7 @@ export default function TerminalPane({ shutdownBufferCaptures.delete(tabId) } } - }, [tabId, setTabLayout]) + }, [tabId, worktreeId, setTabLayout]) const handleStartRename = useCallback((paneId: number) => { setRenameValue(paneTitlesRef.current[paneId] ?? '') diff --git a/src/renderer/src/components/terminal-pane/shutdown-buffer-captures.ts b/src/renderer/src/components/terminal-pane/shutdown-buffer-captures.ts index 67c54e937..f904fef69 100644 --- a/src/renderer/src/components/terminal-pane/shutdown-buffer-captures.ts +++ b/src/renderer/src/components/terminal-pane/shutdown-buffer-captures.ts @@ -1,3 +1,7 @@ +export type ShutdownBufferCaptureOptions = { + includeLocalBuffers?: boolean +} + /** Map of tabId → buffer-capture callback, one per mounted TerminalPane. * The beforeunload handler in App.tsx invokes every callback to populate * Zustand with serialized buffers before flushing the session to disk. @@ -12,4 +16,7 @@ * create a cycle (slice → TerminalPane → store → slice) that breaks the * Zustand store at module-init time. A leaf module with zero imports * has no cycle. */ -export const shutdownBufferCaptures = new Map void>() +export const shutdownBufferCaptures = new Map< + string, + (options?: ShutdownBufferCaptureOptions) => void +>() diff --git a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts index 0eb2861e9..87a5a3ad3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, describe, expect, it, vi } from 'vitest' +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import type { TerminalLayoutSnapshot } from '../../../../shared/types' const mocks = vi.hoisted(() => ({ @@ -36,6 +36,10 @@ beforeAll(() => { ;(globalThis as unknown as Record).HTMLElement = MockHTMLElement }) +beforeEach(() => { + mocks.flushTerminalOutput.mockReset() +}) + function mockRootForPane(paneId: number): HTMLDivElement { const pane = new MockHTMLElement({ classList: ['pane'], dataset: { paneId: String(paneId) } }) return new MockHTMLElement({ firstElementChild: pane }) as unknown as HTMLDivElement @@ -88,4 +92,40 @@ describe('captureTerminalShutdownLayout', () => { titlesByLeafId: { 'pane:1': 'build logs' } }) }) + + it('skips local shutdown scrollback serialization while preserving layout metadata', async () => { + const { captureTerminalShutdownLayout } = await import('./terminal-shutdown-layout-capture') + const pane = { + id: 1, + terminal: { options: { scrollback: 50_000 } }, + serializeAddon: { + serialize: vi.fn(() => 'x'.repeat(512 * 1024)) + } + } + const manager = { + getPanes: vi.fn(() => [pane]), + getActivePane: vi.fn(() => pane) + } + + const layout = captureTerminalShutdownLayout({ + manager: manager as never, + container: mockRootForPane(1), + expandedPaneId: null, + paneTransports: new Map([[1, { getPtyId: vi.fn(() => 'pty-1') }]]), + paneTitlesByPaneId: { 1: 'local shell' }, + existingLayout: { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'previous-local-scrollback' } + }, + captureBuffers: false + }) + + expect(mocks.flushTerminalOutput).not.toHaveBeenCalled() + expect(pane.serializeAddon.serialize).not.toHaveBeenCalled() + expect(layout.buffersByLeafId).toBeUndefined() + expect(layout.ptyIdsByLeafId).toEqual({ 'pane:1': 'pty-1' }) + expect(layout.titlesByLeafId).toEqual({ 'pane:1': 'local shell' }) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts index 00f6673d2..67f9d9bb4 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts @@ -21,6 +21,7 @@ type CaptureTerminalShutdownLayoutArgs = { paneTransports: ReadonlyMap> paneTitlesByPaneId: Record existingLayout: TerminalLayoutSnapshot | undefined + captureBuffers?: boolean } export function captureTerminalShutdownLayout({ @@ -29,41 +30,44 @@ export function captureTerminalShutdownLayout({ expandedPaneId, paneTransports, paneTitlesByPaneId, - existingLayout + existingLayout, + captureBuffers = true }: CaptureTerminalShutdownLayoutArgs): TerminalLayoutSnapshot { const panes = manager.getPanes() const buffers: Record = {} - for (const pane of panes) { - try { - // Why: non-focused panes may have renderer-throttled PTY bytes queued; - // push them into xterm before taking the shutdown scrollback snapshot. - flushTerminalOutput(pane.terminal) - const leafId = paneLeafId(pane.id) - let scrollback = pane.terminal.options.scrollback ?? 10_000 - let serialized = pane.serializeAddon.serialize({ scrollback }) - // Cap at 512KB — binary search for largest scrollback that fits. - if (serialized.length > MAX_BUFFER_BYTES && scrollback > 1) { - let lo = 1 - let hi = scrollback - let best = '' - while (lo <= hi) { - const mid = Math.floor((lo + hi) / 2) - const attempt = pane.serializeAddon.serialize({ scrollback: mid }) - if (attempt.length <= MAX_BUFFER_BYTES) { - best = attempt - lo = mid + 1 - } else { - hi = mid - 1 + if (captureBuffers) { + for (const pane of panes) { + try { + // Why: non-focused panes may have renderer-throttled PTY bytes queued; + // push them into xterm before taking the shutdown scrollback snapshot. + flushTerminalOutput(pane.terminal) + const leafId = paneLeafId(pane.id) + let scrollback = pane.terminal.options.scrollback ?? 10_000 + let serialized = pane.serializeAddon.serialize({ scrollback }) + // Cap at 512KB — binary search for largest scrollback that fits. + if (serialized.length > MAX_BUFFER_BYTES && scrollback > 1) { + let lo = 1 + let hi = scrollback + let best = '' + while (lo <= hi) { + const mid = Math.floor((lo + hi) / 2) + const attempt = pane.serializeAddon.serialize({ scrollback: mid }) + if (attempt.length <= MAX_BUFFER_BYTES) { + best = attempt + lo = mid + 1 + } else { + hi = mid - 1 + } } + serialized = best } - serialized = best + if (serialized.length > 0) { + buffers[leafId] = serialized + } + } catch { + // Serialization failure for one pane should not block others. } - if (serialized.length > 0) { - buffers[leafId] = serialized - } - } catch { - // Serialization failure for one pane should not block others. } } @@ -74,11 +78,13 @@ export function captureTerminalShutdownLayout({ .map((pane) => [paneLeafId(pane.id), paneTransports.get(pane.id)?.getPtyId() ?? null] as const) .filter((entry): entry is readonly [string, string] => entry[1] !== null) - const mergedBuffers = mergeCapturedLeafState({ - prior: existingLayout?.buffersByLeafId, - fresh: buffers, - currentLeafIds - }) + const mergedBuffers = captureBuffers + ? mergeCapturedLeafState({ + prior: existingLayout?.buffersByLeafId, + fresh: buffers, + currentLeafIds + }) + : {} const mergedPtyIds = mergeCapturedLeafState({ prior: existingLayout?.ptyIdsByLeafId, fresh: Object.fromEntries(ptyEntries), diff --git a/src/shared/workspace-session-terminal-buffers.test.ts b/src/shared/workspace-session-terminal-buffers.test.ts index 01d54528a..b2f29fd9c 100644 --- a/src/shared/workspace-session-terminal-buffers.test.ts +++ b/src/shared/workspace-session-terminal-buffers.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it } from 'vitest' import { FLOATING_TERMINAL_WORKTREE_ID } from './constants' import type { WorkspaceSessionState } from './types' -import { pruneLocalTerminalScrollbackBuffers } from './workspace-session-terminal-buffers' +import { + pruneLocalTerminalScrollbackBuffers, + shouldPreserveTerminalScrollbackBuffers +} from './workspace-session-terminal-buffers' function makeSession(overrides: Partial = {}): WorkspaceSessionState { return { @@ -55,6 +58,26 @@ function makeSession(overrides: Partial = {}): WorkspaceS } describe('pruneLocalTerminalScrollbackBuffers', () => { + it('classifies which worktrees need renderer-captured scrollback', () => { + const repos = [ + { id: 'local-repo', connectionId: null }, + { id: 'remote-repo', connectionId: 'ssh-target-1' } + ] + + expect(shouldPreserveTerminalScrollbackBuffers('local-repo::/local/worktree', repos)).toBe( + false + ) + expect(shouldPreserveTerminalScrollbackBuffers('remote-repo::/remote/worktree', repos)).toBe( + true + ) + expect(shouldPreserveTerminalScrollbackBuffers(FLOATING_TERMINAL_WORKTREE_ID, repos)).toBe( + false + ) + expect( + shouldPreserveTerminalScrollbackBuffers('unknown-repo::/maybe-remote/worktree', repos) + ).toBe(true) + }) + it('drops local buffers while preserving SSH buffers and PTY bindings', () => { const result = pruneLocalTerminalScrollbackBuffers(makeSession(), [ { id: 'local-repo', connectionId: null }, diff --git a/src/shared/workspace-session-terminal-buffers.ts b/src/shared/workspace-session-terminal-buffers.ts index 77bc0c48f..fced005b5 100644 --- a/src/shared/workspace-session-terminal-buffers.ts +++ b/src/shared/workspace-session-terminal-buffers.ts @@ -4,6 +4,36 @@ import { getRepoIdFromWorktreeId } from './worktree-id' export type RepoConnection = Pick +function shouldPreserveTerminalScrollbackBuffersForRepoMap( + worktreeId: string | undefined, + connectionIdByRepoId: ReadonlyMap +): boolean { + if (worktreeId === undefined || worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + return false + } + const repoId = getRepoIdFromWorktreeId(worktreeId) + const connectionId = connectionIdByRepoId.get(repoId) + if (connectionId) { + return true + } + if (!connectionIdByRepoId.has(repoId)) { + // Why: when the repo catalog is not hydrated, treating the worktree as SSH + // avoids losing the only scrollback source a relay-backed terminal may have. + return true + } + return false +} + +export function shouldPreserveTerminalScrollbackBuffers( + worktreeId: string | undefined, + repos: readonly RepoConnection[] +): boolean { + return shouldPreserveTerminalScrollbackBuffersForRepoMap( + worktreeId, + new Map(repos.map((repo) => [repo.id, repo.connectionId] as const)) + ) +} + export function pruneLocalTerminalScrollbackBuffers( session: WorkspaceSessionState, repos: readonly RepoConnection[] @@ -22,29 +52,8 @@ export function pruneLocalTerminalScrollbackBuffers( continue } const worktreeId = worktreeIdByTabId.get(tabId) - if (worktreeId !== undefined) { - if (worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { - terminalLayoutsByTabId ??= { ...session.terminalLayoutsByTabId } - const layoutWithoutBuffers = { ...layout } - delete layoutWithoutBuffers.buffersByLeafId - terminalLayoutsByTabId[tabId] = layoutWithoutBuffers - continue - } - const repoId = getRepoIdFromWorktreeId(worktreeId) - const connectionId = connectionIdByRepoId.get(repoId) - if (connectionId) { - continue - } - if (!connectionIdByRepoId.has(repoId)) { - // Why: when the repo catalog does not know this repoId — either because - // it is not yet hydrated, or because the repo has been removed — we - // cannot classify the worktree as local vs SSH. Preserve the buffer - // until a later call with a hydrated catalog can decide. SSH buffers - // are the only authoritative scrollback source, so the cost of a wrong - // prune (lost remote scrollback) is higher than the cost of a wrong - // preserve (extra bytes persisted). - continue - } + if (shouldPreserveTerminalScrollbackBuffersForRepoMap(worktreeId, connectionIdByRepoId)) { + continue } terminalLayoutsByTabId ??= { ...session.terminalLayoutsByTabId }