From a3604803ccfc599bcd263dfc4ac171b8f7503c0d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:42:02 -0700 Subject: [PATCH] perf(runtime): gate terminal.list visual layouts and stop the false writable claim visualLayouts is ~31% of a large terminal.list payload (44,208 B of 137,412 B on a live 134-terminal remote runtime) and has exactly one consumer: the human-readable CLI formatter. Gate it behind an includeVisualLayouts request param that defaults to included, so pre-flag clients are unaffected, and have every --json/internal caller opt out. Also drop the record-backed builder's writable, which was a verbatim copy of connected. terminal.show now states writability explicitly as exactly what terminal.send's PTY gate enforces. --- .../app/h/[hostId]/session/[worktreeId].tsx | 3 +- src/cli/handlers/terminal.ts | 4 +- src/cli/index.test.ts | 20 +- src/main/runtime/orca-runtime.test.ts | 45 +++++ src/main/runtime/orca-runtime.ts | 25 ++- src/main/runtime/orchestration/coordinator.ts | 6 +- src/main/runtime/rpc/methods/terminal.ts | 8 +- src/main/runtime/runtime-rpc.test.ts | 26 +++ .../terminal-list-payload-size.test.ts | 181 ++++++++++++++++++ src/main/ssh/ssh-remote-orca-cli.ts | 4 +- .../src/lib/active-agent-note-send.test.ts | 2 +- .../src/lib/active-agent-note-target.ts | 6 +- .../src/lib/agent-hibernation-coordinator.ts | 3 +- .../src/runtime/remote-worktree-sleep.ts | 3 +- .../web-session-terminal-orphan-recovery.ts | 3 +- src/shared/runtime-types.ts | 7 +- 16 files changed, 325 insertions(+), 21 deletions(-) create mode 100644 src/main/runtime/terminal-list-payload-size.test.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 4d1f5319636..79854da931e 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -1693,7 +1693,8 @@ export default function SessionScreen() { try { const response = await client.sendRequest('terminal.list', { - worktree: `id:${worktreeId}` + worktree: `id:${worktreeId}`, + includeVisualLayouts: false }) if (response.ok) { const result = (response as RpcSuccess).result as { terminals: Terminal[] } diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index f8c9d482858..d829e7546e2 100644 --- a/src/cli/handlers/terminal.ts +++ b/src/cli/handlers/terminal.ts @@ -55,7 +55,9 @@ export const TERMINAL_HANDLERS: Record = { 'terminal list': async ({ flags, client, cwd, json }) => { const result = await client.call('terminal.list', { worktree: await getOptionalWorktreeSelector(flags, 'worktree', cwd, client), - limit: getOptionalPositiveIntegerFlag(flags, 'limit') + limit: getOptionalPositiveIntegerFlag(flags, 'limit'), + // Why: only the human formatter renders layouts; --json ships and discards them. + includeVisualLayouts: !json }) printResult(result, json, formatTerminalList) }, diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index bca1910cb10..825369e1345 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -3702,10 +3702,28 @@ describe('orca cli worktree awareness', () => { expect(callMock).toHaveBeenNthCalledWith(2, 'terminal.list', { worktree: 'id:repo::/tmp/repo/feature', - limit: undefined + limit: undefined, + includeVisualLayouts: false }) }) + it('requests visual layouts only for the human-readable terminal list', async () => { + queueFixtures( + callMock, + worktreeListFixture([buildWorktree('/tmp/repo/feature', 'feature/foo')]), + okFixture('req_term', { terminals: [], totalCount: 0, truncated: false }) + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await main(['terminal', 'list', '--worktree', 'active'], '/tmp/repo/feature/src') + + expect(callMock).toHaveBeenNthCalledWith( + 2, + 'terminal.list', + expect.objectContaining({ includeVisualLayouts: true }) + ) + }) + it('rejects implicit remote terminal create instead of resolving from client cwd', async () => { const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 722acf67937..357d6cacf31 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -20759,6 +20759,51 @@ describe('OrcaRuntimeService', () => { ).rejects.toThrow('terminal_topology_conflict') }) + it('never claims writable for an orphaned listing entry, and keeps show aligned with the send gate', async () => { + const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [TEST_WORKTREE_ID]: [] } + }) + const runtime = new OrcaRuntimeService({ ...runtimeStore, flushOrThrow: vi.fn() } as never) + const writes: [string, string][] = [] + runtime.setPtyController({ + write: (ptyId: string, data: string) => { + writes.push([ptyId, data]) + return true + }, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses: async () => [ + { + id: 'pty-orphan', + incarnationId: 'inc-orphan', + terminalHandle: 'term_orphan', + title: 'shell', + cwd: TEST_WORKTREE_PATH, + worktreeId: TEST_WORKTREE_ID, + wslDistro: null + } + ] + } as never) + runtime.registerPty('pty-orphan', TEST_WORKTREE_ID) + runtime.onPtySpawned('pty-orphan', 'inc-orphan', { awaitsRegistration: false }) + + const listed = await runtime.listTerminals(`id:${TEST_WORKTREE_ID}`) + const entry = listed.terminals.find((terminal) => terminal.ptyId === 'pty-orphan') + expect(entry).toMatchObject({ orphaned: true, connected: true }) + // The listing makes no writability claim it cannot back: `writable` would + // only restate `connected` here. + expect(entry?.writable).toBeUndefined() + + // ...but the orphan is genuinely writable, so show and send must agree. + const shown = await runtime.showTerminal(entry!.handle) + expect(shown.writable).toBe(true) + await expect(runtime.sendTerminal(entry!.handle, { text: 'hi' })).resolves.toMatchObject({ + accepted: true + }) + expect(writes).toEqual([['pty-orphan', 'hi']]) + }) + it('rejects connection mismatch and reused handles while allowing a WSL-owned orphan', async () => { const makeRuntime = (): OrcaRuntimeService => { const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession({ diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 849cfdc9c72..89b5fb76837 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -14851,7 +14851,11 @@ export class OrcaRuntimeService { async listTerminals( worktreeSelector?: string, limit = DEFAULT_TERMINAL_LIST_LIMIT, - opts: { handles?: readonly string[]; requireFreshPtyLiveness?: boolean } = {} + opts: { + handles?: readonly string[] + requireFreshPtyLiveness?: boolean + includeVisualLayouts?: boolean + } = {} ): Promise { if (!Number.isInteger(limit) || limit <= 0) { throw new Error('invalid_limit') @@ -14968,11 +14972,12 @@ export class OrcaRuntimeService { ? terminals.filter((terminal) => requestedHandles.has(terminal.handle)) : terminals const listedTerminals = matchingTerminals.slice(0, limit) - const visualLayouts = this.buildTerminalVisualLayouts( - listedTerminals, - worktreesById, - targetWorktreeId - ) + // Why: undefined (pre-flag client) must still get layouts; only an explicit + // `false` opts out. + const visualLayouts = + opts.includeVisualLayouts === false + ? [] + : this.buildTerminalVisualLayouts(listedTerminals, worktreesById, targetWorktreeId) return { terminals: listedTerminals, @@ -15963,6 +15968,9 @@ export class OrcaRuntimeService { return { ...summary, preview, + // Why: mirrors terminal.send's PTY gate exactly (connected only — + // orphaned handles still accept writes and adoption re-attaches them). + writable: pty.pty.connected, tabId: pty.pty.tabId ?? pty.record.tabId, leafId: parsePaneKey(pty.pty.paneKey ?? '')?.leafId ?? pty.record.leafId, paneRuntimeId: -1, @@ -15985,6 +15993,7 @@ export class OrcaRuntimeService { return { ...summary, preview, + writable: leaf.writable, paneRuntimeId: leaf.paneRuntimeId, ptyId: leaf.ptyId, rendererGraphEpoch: this.rendererGraphEpoch @@ -30582,7 +30591,9 @@ export class OrcaRuntimeService { leafId: orphaned ? `pty:${pty.ptyId}` : pane.leafId, title: getLatestPtyTitle(pty), connected: pty.connected, - writable: pty.connected, + // Why: no `writable` — for a PTY record it would only restate `connected`, + // which is exactly what terminal.send gates on. Callers needing the claim + // use terminal.show. lastOutputAt: pty.lastOutputAt, preview: pty.preview } diff --git a/src/main/runtime/orchestration/coordinator.ts b/src/main/runtime/orchestration/coordinator.ts index df69ed9ef7b..4175647d02e 100644 --- a/src/main/runtime/orchestration/coordinator.ts +++ b/src/main/runtime/orchestration/coordinator.ts @@ -10,7 +10,7 @@ export type CoordinatorRuntime = { worktreeSelector?: string, limit?: number ): Promise<{ - terminals: { handle: string; worktreeId: string; connected: boolean; writable: boolean }[] + terminals: { handle: string; worktreeId: string; connected: boolean; writable?: boolean }[] }> createTerminal( worktreeSelector?: string, @@ -501,7 +501,9 @@ export class Coordinator { t.handle !== this.opts.coordinatorHandle && !busyHandles.has(t.handle) && t.connected && - t.writable + // Why: absent means the runtime makes no claim beyond `connected` + // (record-backed entry); only an explicit `false` disqualifies. + t.writable !== false ) .map((t) => t.handle) } catch { diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index edef5f44083..bd6cad49a32 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -813,7 +813,10 @@ const TerminalListParams = z.object({ .array(requiredString('Missing terminal handle').pipe(z.string().max(256))) .max(64) .optional(), - requireFreshPtyLiveness: z.boolean().optional() + requireFreshPtyLiveness: z.boolean().optional(), + // Why: layouts are ~31% of a large listing and only the human CLI formatter + // reads them. Absent means "include" so pre-flag clients keep rendering them. + includeVisualLayouts: z.boolean().optional() }) const TerminalResolveActive = z.object({ @@ -1108,7 +1111,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ handler: async (params, { runtime }) => runtime.listTerminals(params.worktree, params.limit, { handles: params.handles, - requireFreshPtyLiveness: params.requireFreshPtyLiveness + requireFreshPtyLiveness: params.requireFreshPtyLiveness, + includeVisualLayouts: params.includeVisualLayouts }) }), defineMethod({ diff --git a/src/main/runtime/runtime-rpc.test.ts b/src/main/runtime/runtime-rpc.test.ts index 55d03efbd50..7b487e74d4b 100644 --- a/src/main/runtime/runtime-rpc.test.ts +++ b/src/main/runtime/runtime-rpc.test.ts @@ -3999,6 +3999,32 @@ describe('OrcaRuntimeRpcServer', () => { } ]) + // Pins the opt-out half of the compat contract: the request above omits + // the flag and still gets layouts; only an explicit `false` drops them. + const optedOutResponse = await sendRequest(metadata!.transports[0]!.endpoint, { + id: 'req_list_layout_opt_out', + authToken: metadata!.authToken, + method: 'terminal.list', + params: { worktree: `id:${worktreeId}`, includeVisualLayouts: false } + }) + const optedOut = optedOutResponse.result as { + visualLayouts?: unknown[] + terminals: unknown[] + } + expect(optedOutResponse).toMatchObject({ id: 'req_list_layout_opt_out', ok: true }) + expect(optedOut.visualLayouts).toBeUndefined() + expect(optedOut.terminals).toHaveLength(result.terminals.length) + + const explicitIncludeResponse = await sendRequest(metadata!.transports[0]!.endpoint, { + id: 'req_list_layout_opt_in', + authToken: metadata!.authToken, + method: 'terminal.list', + params: { worktree: `id:${worktreeId}`, includeVisualLayouts: true } + }) + expect( + (explicitIncludeResponse.result as { visualLayouts?: unknown[] }).visualLayouts + ).toHaveLength(1) + const resolvePaneResponse = await sendRequest(metadata!.transports[0]!.endpoint, { id: 'req_resolve_pane', authToken: metadata!.authToken, diff --git a/src/main/runtime/terminal-list-payload-size.test.ts b/src/main/runtime/terminal-list-payload-size.test.ts new file mode 100644 index 00000000000..1ff3811e12f --- /dev/null +++ b/src/main/runtime/terminal-list-payload-size.test.ts @@ -0,0 +1,181 @@ +import { describe, expect, it } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' + +// Measures the real `terminal.list` wire payload for a large listing, so the +// visualLayouts opt-out keeps paying for itself. Sized against a live +// 134-terminal remote runtime (137,412 B on the wire, visualLayouts 44,208 B). +const TERMINAL_COUNT = 134 +const PANES_PER_TAB = 2 +const TAB_COUNT = TERMINAL_COUNT / PANES_PER_TAB +const REPO_ID = 'repo-7f3a91c2e4b85d60' +const WORKTREE_PATH = '/Users/dev/orca/workspaces/orca/perf-terminal-list-diet' +const WORKTREE_ID = `${REPO_ID}::${WORKTREE_PATH}` + +const uuid = (n: number): string => { + const hex = n.toString(16).padStart(12, '0') + return `11111111-1111-4111-8111-${hex}` +} + +const leafIdFor = (index: number): string => uuid(index) +const tabIdFor = (tab: number): string => `tab-${uuid(1_000 + tab)}` +const ptyIdFor = (index: number): string => `pty-${uuid(2_000 + index)}` +const groupIdFor = (tab: number): string => `group-${uuid(3_000 + (tab % 2))}` +const titleFor = (index: number): string => `claude — orca/perf-terminal-list-diet #${index}` + +const makeStore = () => ({ + getRepo: (id: string) => makeStore().getRepos()[0] ?? (id as never), + getRepos: () => [ + { id: REPO_ID, path: '/tmp/repo', displayName: 'repo', badgeColor: 'blue', addedAt: 1 } + ], + addRepo: () => {}, + updateRepo: (id: string) => makeStore().getRepo(id) as never, + getAllWorktreeMeta: () => ({ + [WORKTREE_ID]: { + displayName: 'foo', + comment: '', + linkedIssue: 123, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } + }), + getWorktreeMeta: (worktreeId: string) => + (makeStore().getAllWorktreeMeta() as Record)[worktreeId] as never, + setWorktreeMeta: () => ({}) as never, + removeWorktreeMeta: () => {}, + getSettings: () => ({ + workspaceDir: '/tmp/workspaces', + nestWorkspaces: false, + branchPrefix: 'none', + branchPrefixCustom: '' + }) +}) + +function buildLoadedRuntime(): OrcaRuntimeService { + const runtime = new OrcaRuntimeService(makeStore() as never) + const tabs = [] + const leaves = [] + const mobileTabs = [] + + for (let tab = 0; tab < TAB_COUNT; tab += 1) { + const tabId = tabIdFor(tab) + const first = leafIdFor(tab * PANES_PER_TAB) + const second = leafIdFor(tab * PANES_PER_TAB + 1) + const root = { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: first }, + second: { type: 'leaf' as const, leafId: second } + } + tabs.push({ + tabId, + worktreeId: WORKTREE_ID, + title: `Tab ${tab}`, + activeLeafId: second, + layout: root + }) + for (let pane = 0; pane < PANES_PER_TAB; pane += 1) { + const index = tab * PANES_PER_TAB + pane + const leafId = leafIdFor(index) + leaves.push({ + tabId, + worktreeId: WORKTREE_ID, + leafId, + paneRuntimeId: pane + 1, + ptyId: ptyIdFor(index), + title: titleFor(index) + }) + mobileTabs.push({ + type: 'terminal' as const, + id: `${tabId}::${leafId}`, + title: titleFor(index), + parentTabId: tabId, + leafId, + ptyId: ptyIdFor(index), + parentLayout: { + root, + activeLeafId: second, + expandedLeafId: null, + ptyIdsByLeafId: { + [first]: ptyIdFor(tab * PANES_PER_TAB), + [second]: ptyIdFor(tab * PANES_PER_TAB + 1) + } + }, + isActive: index === 0 + }) + } + } + + const groupIds = [groupIdFor(0), groupIdFor(1)] + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { + tabs, + leaves, + mobileSessionTabs: [ + { + worktree: WORKTREE_ID, + publicationEpoch: 'test', + snapshotVersion: 1, + activeGroupId: groupIds[0], + activeTabId: mobileTabs[0]!.id, + activeTabType: 'terminal', + tabGroups: groupIds.map((id, groupIndex) => ({ + id, + activeTabId: tabIdFor(groupIndex), + tabOrder: tabs + .filter((_tab, tabIndex) => tabIndex % 2 === groupIndex) + .map((tab) => tab.tabId) + })), + tabGroupLayout: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', groupId: groupIds[0] }, + second: { type: 'leaf', groupId: groupIds[1] } + }, + tabs: mobileTabs + } + ] + } as never) + + // Realistic previews: the live sample carried ~27 B of tail text per terminal. + for (let index = 0; index < TERMINAL_COUNT; index += 1) { + const ptyId = ptyIdFor(index) + runtime.registerPty(ptyId, WORKTREE_ID) + runtime.onPtySpawned(ptyId, `inc-${uuid(4_000 + index)}`, { awaitsRegistration: false }) + runtime.onPtyData(ptyId, `esc to interrupt · ${index}\n`, 1) + } + return runtime +} + +describe('terminal.list payload size', () => { + it('drops ~30% of the wire payload when a caller opts out of visualLayouts', async () => { + const runtime = buildLoadedRuntime() + + const withLayouts = await runtime.listTerminals(`id:${WORKTREE_ID}`, 10_000) + const withoutLayouts = await runtime.listTerminals(`id:${WORKTREE_ID}`, 10_000, { + includeVisualLayouts: false + }) + + expect(withLayouts.terminals).toHaveLength(TERMINAL_COUNT) + expect(withoutLayouts.terminals).toHaveLength(TERMINAL_COUNT) + expect(withLayouts.visualLayouts).toHaveLength(1) + expect(withoutLayouts.visualLayouts).toBeUndefined() + + const beforeBytes = Buffer.byteLength(JSON.stringify(withLayouts), 'utf8') + const afterBytes = Buffer.byteLength(JSON.stringify(withoutLayouts), 'utf8') + const layoutBytes = Buffer.byteLength(JSON.stringify(withLayouts.visualLayouts), 'utf8') + const reduction = (beforeBytes - afterBytes) / beforeBytes + + console.log( + `terminal.list ${TERMINAL_COUNT} terminals: ${beforeBytes} B -> ${afterBytes} B ` + + `(-${(reduction * 100).toFixed(1)}%); visualLayouts alone ${layoutBytes} B` + ) + + expect(beforeBytes).toBeGreaterThan(100_000) + expect(reduction).toBeGreaterThan(0.25) + }) +}) diff --git a/src/main/ssh/ssh-remote-orca-cli.ts b/src/main/ssh/ssh-remote-orca-cli.ts index 709ec8655e3..1aeb10d2d78 100644 --- a/src/main/ssh/ssh-remote-orca-cli.ts +++ b/src/main/ssh/ssh-remote-orca-cli.ts @@ -188,7 +188,9 @@ async function dispatchRemoteCli( case 'terminal list': return await call(dispatcher, 'terminal.list', { worktree: optionalRemoteCliString(parsed.flags, 'worktree'), - limit: optionalRemoteCliNumber(parsed.flags, 'limit') + limit: optionalRemoteCliNumber(parsed.flags, 'limit'), + // Why: only the human formatter renders layouts; --json ships and discards them. + includeVisualLayouts: !parsed.flags.has('json') }) case 'orchestration send': { const type = optionalRemoteCliString(parsed.flags, 'type') diff --git a/src/renderer/src/lib/active-agent-note-send.test.ts b/src/renderer/src/lib/active-agent-note-send.test.ts index 82083d515ab..2abcdd0d772 100644 --- a/src/renderer/src/lib/active-agent-note-send.test.ts +++ b/src/renderer/src/lib/active-agent-note-send.test.ts @@ -347,7 +347,7 @@ describe('active agent note send', () => { expect(testState.callRuntimeRpc).toHaveBeenCalledWith( { kind: 'local' }, 'terminal.list', - { worktree: 'id:wt-1', limit: 200 }, + { worktree: 'id:wt-1', limit: 200, includeVisualLayouts: false }, { timeoutMs: 15000 } ) expect(testState.callRuntimeRpc).toHaveBeenCalledWith( diff --git a/src/renderer/src/lib/active-agent-note-target.ts b/src/renderer/src/lib/active-agent-note-target.ts index c9e7a833bad..326a44dbb0b 100644 --- a/src/renderer/src/lib/active-agent-note-target.ts +++ b/src/renderer/src/lib/active-agent-note-target.ts @@ -167,7 +167,11 @@ export async function findActiveRuntimeTerminal( runtimeTarget, 'terminal.list', // Why: worktree ids can look like branch names or paths; keep the lookup unambiguous. - { worktree: toRuntimeWorktreeSelector(worktreeId), limit: ACTIVE_AGENT_TERMINAL_LIST_LIMIT }, + { + worktree: toRuntimeWorktreeSelector(worktreeId), + limit: ACTIVE_AGENT_TERMINAL_LIST_LIMIT, + includeVisualLayouts: false + }, { timeoutMs } ) return ( diff --git a/src/renderer/src/lib/agent-hibernation-coordinator.ts b/src/renderer/src/lib/agent-hibernation-coordinator.ts index b8dd54337b6..5f9f3278706 100644 --- a/src/renderer/src/lib/agent-hibernation-coordinator.ts +++ b/src/renderer/src/lib/agent-hibernation-coordinator.ts @@ -116,7 +116,8 @@ async function collectRuntimePtyLiveness(state: AppState): Promise