From 350e4f7416753abd9d0b3ea88a09b83dcf4eabd4 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:51:38 -0700 Subject: [PATCH] refactor(renderer): make the orchestration batch's cache key its build's only inputs buildRuntimeBatch no longer receives agentStatusByPaneKey/retainedAgentsByPaneKey. It takes a RuntimeBatchInputs record whose paneWorktreeIds projection is its whole view of those maps, and that same record is the cache key, so the key cannot drift from the read set. Adds a guard asserting one read per orchestrated pane per map. --- ...worktree-agent-orchestration-batch.test.ts | 42 ++++++++ .../worktree-agent-orchestration-batch.ts | 99 ++++++++++--------- 2 files changed, 93 insertions(+), 48 deletions(-) diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts index e1194a634ce..d3a3bc9be2e 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts @@ -623,6 +623,48 @@ describe('selectRuntimeAgentOrchestrationBatch live-map churn', () => { expect(getBatchRecord(first, 'wt-1')[CHILD_KEY]).toBe(ORCHESTRATED_CONTEXT) }) + // Why this is a structural guard: the cache key is the projection, so anything the build + // reads straight out of the live/retained maps is unkeyed and can go stale. The build no + // longer receives those maps at all, which shows up here as exactly one read per pane. + it('reads each orchestrated pane out of the live and retained maps once per build', () => { + releaseRuntimeAgentOrchestrationBatchCache() + const liveReads: string[] = [] + const retainedReads: string[] = [] + const countReads = (target: Value, reads: string[]): Value => + new Proxy(target, { + get(source, key, receiver) { + if (typeof key === 'string') { + reads.push(key) + } + return Reflect.get(source, key, receiver) + } + }) + const state = { + tabsByWorktree: TABS_BY_WORKTREE, + runtimeAgentOrchestrationByPaneKey: RUNTIME_TWO, + agentStatusByPaneKey: countReads( + { + [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1'), + [SECOND_CHILD_KEY]: makeEntry(SECOND_CHILD_KEY, 'wt-2') + }, + liveReads + ), + retainedAgentsByPaneKey: countReads( + { [CHILD_KEY]: makeRetained(CHILD_KEY, 'wt-1') }, + retainedReads + ) + } as BatchState + + const buildsBefore = builds() + const batch = selectRuntimeAgentOrchestrationBatch(state, requested) + + expect(builds()).toBe(buildsBefore + 1) + expect(liveReads).toEqual([CHILD_KEY, SECOND_CHILD_KEY]) + expect(retainedReads).toEqual([CHILD_KEY, SECOND_CHILD_KEY]) + expect(getBatchRecord(batch, 'wt-1')[CHILD_KEY]).toBe(ORCHESTRATED_CONTEXT) + expect(getBatchRecord(batch, 'wt-2')[SECOND_CHILD_KEY]).toBe(SECOND_CONTEXT) + }) + it('rebuilds when an orchestrated pane changes worktree or the entry set changes', () => { releaseRuntimeAgentOrchestrationBatchCache() const first = selectRuntimeAgentOrchestrationBatch( diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts index ba30457fedc..6b759592cd0 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts @@ -20,17 +20,26 @@ type RuntimeDomainCache = { type RequestedTabMembershipCache = { tabsSource: RuntimeOrchestrationState['tabsByWorktree'] - requestedWorktreeIds: string[] + requestedWorktreeIds: readonly string[] requestedIds: Set worktreeIdsByTabId: Map> } -type RuntimeBatchCache = { +/** + * Everything `buildRuntimeBatch` is allowed to read. The live and retained maps are not in + * here and not in its scope; they reach it only as `paneWorktreeIds`. Anything a future build + * needs has to be added here, and this record is also the cache key, so the key cannot drift + * from the read set. + */ +type RuntimeBatchInputs = { runtimeSource: RuntimeOrchestrationMap tabsSource: RuntimeOrchestrationState['tabsByWorktree'] - /** What the batch actually reads out of the live/retained maps; see projectPaneWorktreeIds. */ paneWorktreeIds: readonly (string | undefined)[] - requestedWorktreeIds: string[] + requestedWorktreeIds: readonly string[] +} + +type RuntimeBatchCache = { + inputs: RuntimeBatchInputs recordsByWorktree: ReadonlyMap } @@ -111,11 +120,21 @@ function hasSameOrderedValues(previous: readonly unknown[], next: readonly unkno return previous.every((value, index) => value === next[index]) } +function runtimeBatchInputsEqual(previous: RuntimeBatchInputs, next: RuntimeBatchInputs): boolean { + return ( + previous.runtimeSource === next.runtimeSource && + previous.tabsSource === next.tabsSource && + hasSameOrderedValues(previous.paneWorktreeIds, next.paneWorktreeIds) && + hasSameOrderedValues(previous.requestedWorktreeIds, next.requestedWorktreeIds) + ) +} + /** - * The only thing `buildRuntimeBatch` reads out of the live and retained maps: the - * `worktreeId` each orchestrated pane key resolves to, in ordered-entry order. A - * status write for any other pane cannot change the batch, so this projection — - * not the map identities — is the correct cache key. + * The batch's whole view of the live and retained maps: the `worktreeId` each orchestrated + * pane key resolves to, as live,retained pairs in ordered-entry order. A status write for any + * other pane cannot change the batch, so this projection — not the map identities — is the + * correct cache key. Exact runtime keys preserve early SSH attribution and ignore stale + * `entry.paneKey` fields carried by a live or retained row. */ function projectPaneWorktreeIds( orderedRuntimeEntries: readonly [string, AgentStatusOrchestrationContext][], @@ -134,7 +153,7 @@ function projectPaneWorktreeIds( function getRequestedTabMembership( tabsByWorktree: RuntimeOrchestrationState['tabsByWorktree'], - requestedWorktreeIds: string[] + requestedWorktreeIds: readonly string[] ): RequestedTabMembershipCache { if ( requestedTabMembershipCache?.tabsSource === tabsByWorktree && @@ -190,19 +209,17 @@ function reuseRecordIfOrderedEqual( } function buildRuntimeBatch( - requestedWorktreeIds: string[], - orderedRuntimeEntries: [string, AgentStatusOrchestrationContext][], - tabsByWorktree: RuntimeOrchestrationState['tabsByWorktree'], - agentStatusByPaneKey: RuntimeOrchestrationState['agentStatusByPaneKey'], - retainedAgentsByPaneKey: RuntimeOrchestrationState['retainedAgentsByPaneKey'] + inputs: RuntimeBatchInputs, + orderedRuntimeEntries: [string, AgentStatusOrchestrationContext][] ): ReadonlyMap { runtimeBatchBuildCount += 1 const { requestedIds, worktreeIdsByTabId } = getRequestedTabMembership( - tabsByWorktree, - requestedWorktreeIds + inputs.tabsSource, + inputs.requestedWorktreeIds ) const recordsByWorktree = new Map() + let projectionCursor = 0 for (const [paneKey, orchestration] of orderedRuntimeEntries) { const targets = new Set() const parsed = parsePaneKey(paneKey) @@ -220,10 +237,9 @@ function buildRuntimeBatch( } } - // Why: exact runtime keys preserve early SSH attribution and ignore stale - // entry.paneKey fields carried by a live or retained row. - const liveWorktreeId = agentStatusByPaneKey[paneKey]?.worktreeId - const retainedWorktreeId = retainedAgentsByPaneKey[paneKey]?.worktreeId + const liveWorktreeId = inputs.paneWorktreeIds[projectionCursor] + const retainedWorktreeId = inputs.paneWorktreeIds[projectionCursor + 1] + projectionCursor += 2 if (typeof liveWorktreeId === 'string' && requestedIds.has(liveWorktreeId)) { targets.add(liveWorktreeId) } @@ -269,35 +285,22 @@ export function selectRuntimeAgentOrchestrationBatch( return EMPTY_BATCH } - const tabsByWorktree = state.tabsByWorktree ?? EMPTY_TABS_BY_WORKTREE - const agentStatusByPaneKey = state.agentStatusByPaneKey ?? EMPTY_AGENT_STATUS - const retainedAgentsByPaneKey = state.retainedAgentsByPaneKey ?? EMPTY_RETAINED_AGENTS - const paneWorktreeIds = projectPaneWorktreeIds( - orderedRuntimeEntries, - agentStatusByPaneKey, - retainedAgentsByPaneKey - ) - if ( - runtimeBatchCache?.runtimeSource === runtimeAgentOrchestrationByPaneKey && - runtimeBatchCache.tabsSource === tabsByWorktree && - hasSameOrderedValues(runtimeBatchCache.paneWorktreeIds, paneWorktreeIds) && - hasSameOrderedValues(runtimeBatchCache.requestedWorktreeIds, requestedWorktreeIds) - ) { + const inputs: RuntimeBatchInputs = { + runtimeSource: runtimeAgentOrchestrationByPaneKey, + tabsSource: state.tabsByWorktree ?? EMPTY_TABS_BY_WORKTREE, + paneWorktreeIds: projectPaneWorktreeIds( + orderedRuntimeEntries, + state.agentStatusByPaneKey ?? EMPTY_AGENT_STATUS, + state.retainedAgentsByPaneKey ?? EMPTY_RETAINED_AGENTS + ), + requestedWorktreeIds + } + if (runtimeBatchCache && runtimeBatchInputsEqual(runtimeBatchCache.inputs, inputs)) { return runtimeBatchCache.recordsByWorktree } - runtimeBatchCache = { - runtimeSource: runtimeAgentOrchestrationByPaneKey, - tabsSource: tabsByWorktree, - paneWorktreeIds, - requestedWorktreeIds, - recordsByWorktree: buildRuntimeBatch( - requestedWorktreeIds, - orderedRuntimeEntries, - tabsByWorktree, - agentStatusByPaneKey, - retainedAgentsByPaneKey - ) - } - return runtimeBatchCache.recordsByWorktree + // buildRuntimeBatch reuses the previous records, so publish the new cache only after it runs. + const recordsByWorktree = buildRuntimeBatch(inputs, orderedRuntimeEntries) + runtimeBatchCache = { inputs, recordsByWorktree } + return recordsByWorktree }