From a711cb8b60e9cdf3b2e0aa2480d7ae2a2f1fcfe2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:25:22 -0700 Subject: [PATCH] perf(renderer): gate the tab strip's worktree subscriptions and fix the orchestration batch's self-invalidating cache (#18428) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(renderer): gate the tab strip's worktree subscriptions and stop the orchestration batch invalidating itself Two store-subscription hot paths. The tab strip subscribed to projects/repos/worktreesByRepo for the Windows shell menu's local project runtime, which is never built unless that menu is on. On macOS/Linux every worktree write therefore re-rendered and re-committed every mounted tab strip. Gate the three on the condition that already gates their only consumer. The runtime-orchestration batch keyed its cache on agentStatusByPaneKey identity, which `agentStatus:set` replaces by definition, so it missed 100% of the time on the only event that calls it. Key on the paneKey -> worktreeId pairs the batch actually reads instead, and hang the requested-id array off the existing activeWorkspaces memo so the O(worktrees) prologue stops running per event. * 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. * refactor(renderer): move the orchestration projection key onto the shared index The batch builder and `worktree-agent-orchestration-index.ts` were near-duplicate implementations of the same attribution walk, and both had the self-invalidating `liveSource === agentStatusByPaneKey` gate. Fixing only the batch left the index — which every mounted WorktreeCard hits on every `agentStatus:set` — still rebuilding per publication. Put `paneWorktreeIds` on the index instead and reduce the batch to a `.get`-compatible view of it. That deletes the whole `requestedWorktreeIds` apparatus the batch fix needed (the `worktreeIds` memo threading, the optional `selectDashboardOrchestration` param, the `uniqueWorktreeIdsByInput` WeakMap and its no-mutation contract, `getRequestedTabMembership`), leaves one builder guarded by the index's randomized oracle test, and extends the fix to the sidebar. The projection is memoised on the live/retained map identities so it is computed once per publication rather than once per card, and a successful ordered compare adopts the new array so the remaining cards compare by identity. --- .../build-dashboard-snapshot.test.ts | 18 +- ...worktree-agent-orchestration-batch.test.ts | 196 +++++++++++-- .../worktree-agent-orchestration-batch.ts | 263 ++---------------- ...worktree-agent-orchestration-index.test.ts | 88 ++++++ .../worktree-agent-orchestration-index.ts | 136 ++++++--- .../TabBar.worktree-write-gate.test.tsx | 111 ++++++++ ...abBar.worktree-write-gate.windows.test.tsx | 84 ++++++ ...-bar-runtime-model-worktree-write-probe.ts | 164 +++++++++++ .../tab-bar/use-tab-bar-runtime-model.ts | 21 +- 9 files changed, 774 insertions(+), 307 deletions(-) create mode 100644 src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.test.tsx create mode 100644 src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.windows.test.tsx create mode 100644 src/renderer/src/components/tab-bar/use-tab-bar-runtime-model-worktree-write-probe.ts diff --git a/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts b/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts index bae9892736d..698f759d7ab 100644 --- a/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts +++ b/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts @@ -10,6 +10,7 @@ import { makePaneKey } from '../../../../shared/stable-pane-id' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import type { Worktree } from '../../../../shared/worktree/types' import { selectRuntimeAgentOrchestrationBatch } from '../sidebar/worktree-agent-orchestration-batch' +import { selectRuntimeAgentOrchestrationForWorktree } from '../sidebar/worktree-agent-row-selectors' import type * as DashboardSnapshotWorkspacesModule from './dashboard-snapshot-workspaces' import type * as AgentRowLineageModule from './agent-row-lineage' @@ -753,7 +754,10 @@ describe('buildDashboardSnapshot', () => { expect(snapshot.cards[0].task).toBe('Batched orchestration task') }) - it('releases stale batch references when production moves from multi to singleton to zero', () => { + // Why identity, not release: the batch is a view of the shared orchestration index, which + // mounted sidebar cards read through. A dashboard that drops below two worktrees must not + // invalidate it, and nothing the index reads changed across these transitions. + it('keeps batch records live and correct when production moves from multi to singleton to zero', () => { const secondLeafId = '77777777-7777-4777-8777-777777777777' const firstPaneKey = makePaneKey('tab-w1', LEAF_ID) const secondPaneKey = makePaneKey('tab-w2', secondLeafId) @@ -788,13 +792,17 @@ describe('buildDashboardSnapshot', () => { NOW ) const afterSingleton = selectRuntimeAgentOrchestrationBatch(multiState, requested) - expect(afterSingleton).not.toBe(firstBatch) - expect(afterSingleton.get('w1')).not.toBe(firstW1) + expect(afterSingleton).toBe(firstBatch) + expect(afterSingleton.get('w1')).toBe(firstW1) buildDashboardSnapshot(baseState({ repos: [], worktreesByRepo: {} }), NOW) const afterZero = selectRuntimeAgentOrchestrationBatch(multiState, requested) - expect(afterZero).not.toBe(afterSingleton) - expect(afterZero.get('w1')).not.toBe(afterSingleton.get('w1')) + expect(afterZero).toBe(firstBatch) + for (const worktreeId of requested) { + expect(afterZero.get(worktreeId)).toBe( + selectRuntimeAgentOrchestrationForWorktree(multiState, worktreeId) + ) + } }) it('scans orchestration runtime once for a dashboard snapshot', () => { 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 665cbb07896..705a1e0c43e 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 @@ -10,6 +10,10 @@ import { EMPTY_WORKTREE_AGENT_ORCHESTRATION, selectRuntimeAgentOrchestrationBatch } from './worktree-agent-orchestration-batch' +import { + _getWorktreeAgentOrchestrationIndexBuildCountForTest, + releaseWorktreeAgentOrchestrationIndexCache +} from './worktree-agent-orchestration-index' import { selectRuntimeAgentOrchestrationForWorktree } from './worktree-agent-row-selectors' type BatchState = Parameters[0] @@ -285,7 +289,9 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { expect(getBatchRecord(replacedBatch, 'wt-2')).toBe(firstWt2) }) - it('releases raw and derived caches for empty requests and empty runtime', () => { + // Why this matters now that the batch is a view of the shared index: an empty dashboard must + // not drop a cache that every mounted sidebar card is still reading through. + it('leaves the shared index intact for an empty request and rebuilds after an empty runtime', () => { let tabIdReads = 0 const state = { tabsByWorktree: { @@ -305,14 +311,15 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { const first = getBatchRecord(selectRuntimeAgentOrchestrationBatch(state, ['target']), 'target') expect(tabIdReads).toBe(1) - selectRuntimeAgentOrchestrationBatch(state, []) + expect(selectRuntimeAgentOrchestrationBatch(state, []).size).toBe(0) const afterEmptyRequest = getBatchRecord( selectRuntimeAgentOrchestrationBatch(state, ['target']), 'target' ) - expect(tabIdReads).toBe(2) - expect(afterEmptyRequest).not.toBe(first) + expect(tabIdReads).toBe(1) + expect(afterEmptyRequest).toBe(first) + // An emptied orchestration map is a real change of the index's own domain, so it does drop. selectRuntimeAgentOrchestrationBatch({ ...state, runtimeAgentOrchestrationByPaneKey: {} }, [ 'target' ]) @@ -320,11 +327,11 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { selectRuntimeAgentOrchestrationBatch(state, ['target']), 'target' ) - expect(tabIdReads).toBe(3) - expect(afterEmptyRuntime).not.toBe(afterEmptyRequest) + expect(tabIdReads).toBe(2) + expect(afterEmptyRuntime).not.toBe(first) }) - it('keeps singleton tab work target-local', () => { + it('matches the per-worktree selector for a single requested worktree', () => { const tabCount = 10 const contextCount = 8 const makeCountedState = () => { @@ -403,22 +410,17 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { ) expect(Object.keys(actual)).toEqual(Object.keys(expected)) - // Why the batch stays tighter: it knows which worktrees are on screen. The - // shared index covers all of them, so it saves per *card*, not per worktree. - expect(batched.counts()).toEqual({ - runtimeEnumerations: 1, - runtimeValueReads: contextCount, - contextVisits: contextCount, - targetTabIdReads: 1, - unrelatedTabIdReads: 0 - }) - expect(reference.counts()).toEqual({ + // Why identical: the batch is the shared index, which walks every worktree's tabs once per + // tabs-slice identity — not once per request — so a one-worktree request costs the same. + const singleWorktreeCounts = { runtimeEnumerations: 1, runtimeValueReads: contextCount, contextVisits: contextCount, targetTabIdReads: 1, unrelatedTabIdReads: tabCount - 1 - }) + } + expect(batched.counts()).toEqual(singleWorktreeCounts) + expect(reference.counts()).toEqual(singleWorktreeCounts) }) it('collapses multi-worktree runtime scans and caches unchanged publications', () => { @@ -521,15 +523,19 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { requested ) } + // The batch reads nothing but each orchestrated pane's worktreeId out of the live + // map, so publications that leave those alone never revisit a context at all. expect(batched.counts()).toEqual({ runtimeEnumerations: 1, runtimeValueReads: contextCount, - contextVisits: contextCount * (publicationCount + 1), + contextVisits: contextCount, tabIdReads: worktreeCount }) // Publications that change nothing the index reads cost nothing, however - // many cards call in. + // many cards call in. The warm-up pass is the cost of the batch loop above having left the + // one cache slot on a different fixture store; production has a single store. + selectRuntimeAgentOrchestrationForWorktree(reference.state, requested[0]) const referenceBefore = reference.counts() for (let publication = 0; publication < publicationCount; publication += 1) { for (const worktreeId of requested) { @@ -538,11 +544,9 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { } expect(reference.counts()).toEqual(referenceBefore) - // Why this is the honest claim: a real live-status ping replaces - // agentStatusByPaneKey, so the index does rebuild once per publication. What - // the shared index removes is the mounted-card multiplier, not the - // per-publication rebuild. Tab reads stay flat because tab membership is - // keyed on the tabs slice, which a live-status ping does not replace. + // A real live-status ping replaces agentStatusByPaneKey wholesale. The index is keyed on + // what it reads out of that map, not on its identity, so an unrelated pane's ping costs + // nothing: no rebuild, no context revisit, however many cards call in. const churn = makeCountedState() for (const worktreeId of requested) { selectRuntimeAgentOrchestrationForWorktree(churn.state, worktreeId) @@ -561,8 +565,148 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { expect(churn.counts()).toEqual({ runtimeEnumerations: 1, runtimeValueReads: contextCount, - contextVisits: contextCount * (publicationCount + 1), + contextVisits: contextCount, tabIdReads: worktreeCount }) }) }) + +describe('selectRuntimeAgentOrchestrationBatch live-map churn', () => { + const ORCHESTRATED_CONTEXT = makeContext('orchestrated') + const SECOND_CONTEXT = makeContext('second') + const requested = ['wt-1', 'wt-2'] + // Held by identity so only the live map churns, as it does under `agentStatus:set`. + const TABS_BY_WORKTREE = { 'wt-1': [makeTab('unrelated-tab')], 'wt-2': [] } + const RUNTIME_ONE = { [CHILD_KEY]: ORCHESTRATED_CONTEXT } + const RUNTIME_TWO = { [CHILD_KEY]: ORCHESTRATED_CONTEXT, [SECOND_CHILD_KEY]: SECOND_CONTEXT } + const RETAINED = {} + + function makeChurnState( + agentStatusByPaneKey: Record, + runtimeAgentOrchestrationByPaneKey: Record< + string, + AgentStatusOrchestrationContext + > = RUNTIME_ONE + ): BatchState { + return { + tabsByWorktree: TABS_BY_WORKTREE, + runtimeAgentOrchestrationByPaneKey, + agentStatusByPaneKey, + retainedAgentsByPaneKey: RETAINED + } as BatchState + } + + function builds(): number { + return _getWorktreeAgentOrchestrationIndexBuildCountForTest() + } + + it('rebuilds once across repeated agentStatus:set identity churn on unrelated panes', () => { + releaseWorktreeAgentOrchestrationIndexCache() + const first = selectRuntimeAgentOrchestrationBatch( + makeChurnState({ [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1') }), + requested + ) + const buildsAfterFirst = builds() + + for (let index = 0; index < 25; index += 1) { + // A fresh live map on every tick, exactly as `agentStatus:set` replaces the slice. + const churned = selectRuntimeAgentOrchestrationBatch( + makeChurnState({ + [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1'), + [`unrelated-${index}`]: makeEntry(`unrelated-${index}`, 'wt-9') + }), + requested + ) + expect(churned).toBe(first) + } + expect(builds()).toBe(buildsAfterFirst) + 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', () => { + releaseWorktreeAgentOrchestrationIndexCache() + 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', () => { + releaseWorktreeAgentOrchestrationIndexCache() + const first = selectRuntimeAgentOrchestrationBatch( + makeChurnState({ [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1') }), + requested + ) + expect(getBatchRecord(first, 'wt-1')[CHILD_KEY]).toBe(ORCHESTRATED_CONTEXT) + expect(first.has('wt-2')).toBe(false) + + const movedBuilds = builds() + const moved = selectRuntimeAgentOrchestrationBatch( + makeChurnState({ [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-2') }), + requested + ) + expect(builds()).toBe(movedBuilds + 1) + expect(moved).not.toBe(first) + expect(moved.has('wt-1')).toBe(false) + expect(getBatchRecord(moved, 'wt-2')[CHILD_KEY]).toBe(ORCHESTRATED_CONTEXT) + + const addedBuilds = builds() + const added = selectRuntimeAgentOrchestrationBatch( + makeChurnState( + { + [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-2'), + [SECOND_CHILD_KEY]: makeEntry(SECOND_CHILD_KEY, 'wt-1') + }, + RUNTIME_TWO + ), + requested + ) + expect(builds()).toBe(addedBuilds + 1) + expect(Object.keys(getBatchRecord(added, 'wt-1'))).toEqual([SECOND_CHILD_KEY]) + + const removedBuilds = builds() + const removed = selectRuntimeAgentOrchestrationBatch( + makeChurnState({ + [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-2'), + [SECOND_CHILD_KEY]: makeEntry(SECOND_CHILD_KEY, 'wt-1') + }), + requested + ) + expect(builds()).toBe(removedBuilds + 1) + expect(removed.has('wt-1')).toBe(false) + }) +}) 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 d1d66c3be6e..4891632eeb3 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts @@ -1,6 +1,11 @@ import type { AppState } from '@/store/types' import type { AgentStatusOrchestrationContext } from '../../../../shared/agent-status-types' -import { parsePaneKey } from '../../../../shared/stable-pane-id' +import { + EMPTY_WORKTREE_AGENT_ORCHESTRATION_INDEX, + selectWorktreeAgentOrchestrationIndex +} from './worktree-agent-orchestration-index' + +export { EMPTY_WORKTREE_AGENT_ORCHESTRATION } from './worktree-agent-orchestration-index' type RuntimeOrchestrationState = Pick< AppState, @@ -10,246 +15,28 @@ type RuntimeOrchestrationState = Pick< | 'tabsByWorktree' > -type RuntimeOrchestrationMap = RuntimeOrchestrationState['runtimeAgentOrchestrationByPaneKey'] -type RuntimeOrchestrationRecord = Record - -type RuntimeDomainCache = { - source: RuntimeOrchestrationMap - orderedEntries: [string, AgentStatusOrchestrationContext][] -} - -type RequestedTabMembershipCache = { - tabsSource: RuntimeOrchestrationState['tabsByWorktree'] - requestedWorktreeIds: string[] - requestedIds: Set - worktreeIdsByTabId: Map> -} - -type RuntimeBatchCache = { - runtimeSource: RuntimeOrchestrationMap - tabsSource: RuntimeOrchestrationState['tabsByWorktree'] - liveSource: RuntimeOrchestrationState['agentStatusByPaneKey'] - retainedSource: RuntimeOrchestrationState['retainedAgentsByPaneKey'] - requestedWorktreeIds: string[] - recordsByWorktree: ReadonlyMap -} - -const EMPTY_RUNTIME_ORCHESTRATION: RuntimeOrchestrationMap = {} -const EMPTY_TABS_BY_WORKTREE: RuntimeOrchestrationState['tabsByWorktree'] = {} -const EMPTY_AGENT_STATUS: RuntimeOrchestrationState['agentStatusByPaneKey'] = {} -const EMPTY_RETAINED_AGENTS: RuntimeOrchestrationState['retainedAgentsByPaneKey'] = {} -const EMPTY_BATCH: ReadonlyMap = new Map() - -export const EMPTY_WORKTREE_AGENT_ORCHESTRATION: RuntimeOrchestrationRecord = Object.freeze({}) - -// Why null-prototype: a pane key of `__proto__` is a plain data key here; on a -// normal object the write vanishes into the prototype setter and repoints it. -function createRecord(): RuntimeOrchestrationRecord { - return Object.create(null) as RuntimeOrchestrationRecord -} - -let runtimeDomainCache: RuntimeDomainCache | null = null -let requestedTabMembershipCache: RequestedTabMembershipCache | null = null -let runtimeBatchCache: RuntimeBatchCache | null = null - -export function releaseRuntimeAgentOrchestrationBatchCache(): void { - runtimeDomainCache = null - requestedTabMembershipCache = null - runtimeBatchCache = null -} - -function getOrderedRuntimeEntries( - runtimeAgentOrchestrationByPaneKey: RuntimeOrchestrationMap -): [string, AgentStatusOrchestrationContext][] { - if (runtimeDomainCache?.source === runtimeAgentOrchestrationByPaneKey) { - return runtimeDomainCache.orderedEntries - } - const orderedEntries = Object.entries(runtimeAgentOrchestrationByPaneKey) - runtimeDomainCache = { source: runtimeAgentOrchestrationByPaneKey, orderedEntries } - return orderedEntries -} - -function uniqueWorktreeIds(worktreeIds: readonly string[]): string[] { - const uniqueIds: string[] = [] - const seen = new Set() - for (const worktreeId of worktreeIds) { - if (!seen.has(worktreeId)) { - seen.add(worktreeId) - uniqueIds.push(worktreeId) - } - } - return uniqueIds -} - -function hasSameWorktreeIds(previous: readonly string[], next: readonly string[]): boolean { - if (previous.length !== next.length) { - return false - } - return previous.every((worktreeId, index) => worktreeId === next[index]) -} - -function getRequestedTabMembership( - tabsByWorktree: RuntimeOrchestrationState['tabsByWorktree'], - requestedWorktreeIds: string[] -): RequestedTabMembershipCache { - if ( - requestedTabMembershipCache?.tabsSource === tabsByWorktree && - hasSameWorktreeIds(requestedTabMembershipCache.requestedWorktreeIds, requestedWorktreeIds) - ) { - return requestedTabMembershipCache - } - - const requestedIds = new Set(requestedWorktreeIds) - const worktreeIdsByTabId = new Map>() - for (const worktreeId of requestedWorktreeIds) { - // Why: the batch must not make a singleton dashboard scan unrelated tabs. - for (const tab of tabsByWorktree[worktreeId] ?? []) { - const tabId = tab.id - const existing = worktreeIdsByTabId.get(tabId) - if (existing) { - existing.add(worktreeId) - } else { - worktreeIdsByTabId.set(tabId, new Set([worktreeId])) - } - } - } - requestedTabMembershipCache = { - tabsSource: tabsByWorktree, - requestedWorktreeIds, - requestedIds, - worktreeIdsByTabId - } - return requestedTabMembershipCache -} - -function reuseRecordIfOrderedEqual( - previous: RuntimeOrchestrationRecord | undefined, - next: RuntimeOrchestrationRecord -): RuntimeOrchestrationRecord { - if (!previous) { - return next - } - const previousEntries = Object.entries(previous) - const nextEntries = Object.entries(next) - if (previousEntries.length !== nextEntries.length) { - return next - } - for (let index = 0; index < nextEntries.length; index += 1) { - if ( - previousEntries[index]?.[0] !== nextEntries[index]?.[0] || - previousEntries[index]?.[1] !== nextEntries[index]?.[1] - ) { - return next - } - } - return previous -} - -function buildRuntimeBatch( - requestedWorktreeIds: string[], - orderedRuntimeEntries: [string, AgentStatusOrchestrationContext][], - tabsByWorktree: RuntimeOrchestrationState['tabsByWorktree'], - agentStatusByPaneKey: RuntimeOrchestrationState['agentStatusByPaneKey'], - retainedAgentsByPaneKey: RuntimeOrchestrationState['retainedAgentsByPaneKey'] -): ReadonlyMap { - const { requestedIds, worktreeIdsByTabId } = getRequestedTabMembership( - tabsByWorktree, - requestedWorktreeIds - ) - - const recordsByWorktree = new Map() - for (const [paneKey, orchestration] of orderedRuntimeEntries) { - const targets = new Set() - const parsed = parsePaneKey(paneKey) - const parsedParent = orchestration.parentPaneKey - ? parsePaneKey(orchestration.parentPaneKey) - : null - if (parsed) { - for (const worktreeId of worktreeIdsByTabId.get(parsed.tabId) ?? []) { - targets.add(worktreeId) - } - } - if (parsedParent) { - for (const worktreeId of worktreeIdsByTabId.get(parsedParent.tabId) ?? []) { - targets.add(worktreeId) - } - } - - // 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 - if (typeof liveWorktreeId === 'string' && requestedIds.has(liveWorktreeId)) { - targets.add(liveWorktreeId) - } - if (typeof retainedWorktreeId === 'string' && requestedIds.has(retainedWorktreeId)) { - targets.add(retainedWorktreeId) - } - - for (const worktreeId of targets) { - let record = recordsByWorktree.get(worktreeId) - if (!record) { - record = createRecord() - recordsByWorktree.set(worktreeId, record) - } - record[paneKey] = orchestration - } - } - - const previousRecords = runtimeBatchCache?.recordsByWorktree - for (const [worktreeId, record] of recordsByWorktree) { - recordsByWorktree.set( - worktreeId, - reuseRecordIfOrderedEqual(previousRecords?.get(worktreeId), record) - ) - } - return recordsByWorktree -} +/** + * No-op: the batch has no cache of its own. Kept because the dashboard's singleton and + * zero-worktree branches still announce that they are done with the batch view, and the shared + * index behind it must survive that — mounted sidebar cards are reading the same records. + */ +export function releaseRuntimeAgentOrchestrationBatchCache(): void {} +/** + * The dashboard's multi-worktree orchestration view. + * + * Why this is the shared index verbatim: the batch used to build its own worktree-keyed records + * from the same four slices, restricted to the requested ids. Callers only ever `.get(id)`, so + * the extra keys are unobservable, and one builder means one cache to keep honest and one + * correctness oracle to satisfy. `worktreeIds` survives only as the empty-dashboard + * short-circuit, which keeps the runtime map unread when nothing is on screen. + */ export function selectRuntimeAgentOrchestrationBatch( state: RuntimeOrchestrationState, worktreeIds: readonly string[] -): ReadonlyMap { - const requestedWorktreeIds = uniqueWorktreeIds(worktreeIds) - if (requestedWorktreeIds.length === 0) { - releaseRuntimeAgentOrchestrationBatchCache() - return EMPTY_BATCH +): ReadonlyMap> { + if (worktreeIds.length === 0) { + return EMPTY_WORKTREE_AGENT_ORCHESTRATION_INDEX } - - const runtimeAgentOrchestrationByPaneKey = - state.runtimeAgentOrchestrationByPaneKey ?? EMPTY_RUNTIME_ORCHESTRATION - const orderedRuntimeEntries = getOrderedRuntimeEntries(runtimeAgentOrchestrationByPaneKey) - if (orderedRuntimeEntries.length === 0) { - releaseRuntimeAgentOrchestrationBatchCache() - 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 - if ( - runtimeBatchCache?.runtimeSource === runtimeAgentOrchestrationByPaneKey && - runtimeBatchCache.tabsSource === tabsByWorktree && - runtimeBatchCache.liveSource === agentStatusByPaneKey && - runtimeBatchCache.retainedSource === retainedAgentsByPaneKey && - hasSameWorktreeIds(runtimeBatchCache.requestedWorktreeIds, requestedWorktreeIds) - ) { - return runtimeBatchCache.recordsByWorktree - } - - runtimeBatchCache = { - runtimeSource: runtimeAgentOrchestrationByPaneKey, - tabsSource: tabsByWorktree, - liveSource: agentStatusByPaneKey, - retainedSource: retainedAgentsByPaneKey, - requestedWorktreeIds, - recordsByWorktree: buildRuntimeBatch( - requestedWorktreeIds, - orderedRuntimeEntries, - tabsByWorktree, - agentStatusByPaneKey, - retainedAgentsByPaneKey - ) - } - return runtimeBatchCache.recordsByWorktree + return selectWorktreeAgentOrchestrationIndex(state) } diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts index e7881d95aff..2d9ba917520 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts @@ -7,6 +7,7 @@ import type { RetainedAgentEntry } from '@/store/slices/agent-status' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { makePaneKey, parsePaneKey } from '../../../../shared/stable-pane-id' import { + _getWorktreeAgentOrchestrationIndexBuildCountForTest, EMPTY_WORKTREE_AGENT_ORCHESTRATION, releaseWorktreeAgentOrchestrationIndexCache, selectWorktreeAgentOrchestration @@ -247,6 +248,93 @@ describe('selectWorktreeAgentOrchestration', () => { expect(selectWorktreeAgentOrchestration(retainedChurn, 'wt-1')).toBe(first) }) + // Why a build counter and not record identity: `reuseRecordIfOrderedEqual` hides a rebuild + // from every identity assertion, so the wasted O(tabs + contexts) pass under `agentStatus:set` + // — several a second on a busy install, with a fresh live map each time — was invisible. + it('does not rebuild when agentStatus:set replaces the live map without moving a pane', () => { + const paneKey = paneKeyFor('tab-1', 0) + const context = { taskId: 't', dispatchId: 'd' } + const tabsByWorktree = { 'wt-1': [makeTab('tab-1')] } + const runtimeAgentOrchestrationByPaneKey = { [paneKey]: context } + const publish = (agentStatusByPaneKey: Record): IndexState => + ({ + tabsByWorktree, + runtimeAgentOrchestrationByPaneKey, + agentStatusByPaneKey, + retainedAgentsByPaneKey: {} + }) as unknown as IndexState + + const first = selectWorktreeAgentOrchestration( + publish({ [paneKey]: makeEntry(paneKey, 'wt-1') }), + 'wt-1' + ) + const buildsAfterFirst = _getWorktreeAgentOrchestrationIndexBuildCountForTest() + + for (let tick = 0; tick < 25; tick += 1) { + // A fresh live map every tick, exactly as `agentStatus:set` replaces the slice, plus a + // stable entry for the orchestrated pane so the projection is non-trivially equal. + const published = publish({ + [paneKey]: makeEntry(paneKey, 'wt-1'), + [`unrelated-${tick}`]: makeEntry(`unrelated-${tick}`, 'wt-9') + }) + expect(selectWorktreeAgentOrchestration(published, 'wt-1')).toBe(first) + } + expect(_getWorktreeAgentOrchestrationIndexBuildCountForTest()).toBe(buildsAfterFirst) + + // ...and the projection is still load-bearing: moving that pane must re-attribute it. + const moved = publish({ [paneKey]: makeEntry(paneKey, 'wt-2') }) + expect(selectWorktreeAgentOrchestration(moved, 'wt-2')[paneKey]).toBe(context) + expect(_getWorktreeAgentOrchestrationIndexBuildCountForTest()).toBe(buildsAfterFirst + 1) + }) + + // Why counted rather than timed: the projection is the index's per-publication work, and + // recomputing it per card would put the O(contexts) scan back on the per-card path that the + // index exists to remove — which no identity or correctness assertion would notice. + it('projects the live and retained maps once per publication, not once per card', () => { + const cardCount = 8 + const contextCount = 6 + const tabsByWorktree: Record = {} + const runtimeAgentOrchestrationByPaneKey: Record = {} + for (let index = 0; index < cardCount; index += 1) { + tabsByWorktree[`wt-${index}`] = [makeTab(`tab-${index}`)] + } + for (let index = 0; index < contextCount; index += 1) { + runtimeAgentOrchestrationByPaneKey[paneKeyFor(`tab-${index}`, index)] = { + taskId: `t-${index}`, + dispatchId: `d-${index}` + } + } + let liveReads = 0 + let retainedReads = 0 + const countReads = (target: object, onRead: () => void): object => + new Proxy(target, { + get(source, key, receiver) { + if (typeof key === 'string') { + onRead() + } + return Reflect.get(source, key, receiver) + } + }) + const state = { + tabsByWorktree, + runtimeAgentOrchestrationByPaneKey, + agentStatusByPaneKey: countReads({}, () => { + liveReads += 1 + }), + retainedAgentsByPaneKey: countReads({}, () => { + retainedReads += 1 + }) + } as unknown as IndexState + + for (let card = 0; card < cardCount; card += 1) { + selectWorktreeAgentOrchestration(state, `wt-${card}`) + } + expect({ liveReads, retainedReads }).toEqual({ + liveReads: contextCount, + retainedReads: contextCount + }) + }) + it('rebuilds when a source it reads actually changes', () => { const context = { taskId: 't', dispatchId: 'd' } const paneKey = paneKeyFor('tab-1', 0) diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts index 5cad5ec1f48..00e0c8af9e3 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts @@ -22,11 +22,18 @@ type TabMembershipCache = { worktreeIdsByTabId: Map> } +type PaneWorktreeProjectionCache = { + runtimeSource: OrchestrationIndexState['runtimeAgentOrchestrationByPaneKey'] + liveSource: OrchestrationIndexState['agentStatusByPaneKey'] + retainedSource: OrchestrationIndexState['retainedAgentsByPaneKey'] + paneWorktreeIds: readonly (string | undefined)[] +} + type OrchestrationIndexCache = { runtimeSource: OrchestrationIndexState['runtimeAgentOrchestrationByPaneKey'] tabsSource: OrchestrationIndexState['tabsByWorktree'] - liveSource: OrchestrationIndexState['agentStatusByPaneKey'] - retainedSource: OrchestrationIndexState['retainedAgentsByPaneKey'] + /** @see projectPaneWorktreeIds — the build's whole view of the live and retained maps. */ + paneWorktreeIds: readonly (string | undefined)[] recordsByWorktree: ReadonlyMap } @@ -51,14 +58,79 @@ function createRecord(): RuntimeOrchestrationRecord { let runtimeEntriesCache: RuntimeEntriesCache | null = null let tabMembershipCache: TabMembershipCache | null = null +let paneWorktreeProjectionCache: PaneWorktreeProjectionCache | null = null let orchestrationIndexCache: OrchestrationIndexCache | null = null +let indexBuildCount = 0 export function releaseWorktreeAgentOrchestrationIndexCache(): void { runtimeEntriesCache = null tabMembershipCache = null + paneWorktreeProjectionCache = null orchestrationIndexCache = null } +export function _getWorktreeAgentOrchestrationIndexBuildCountForTest(): number { + return indexBuildCount +} + +/** + * The build's whole view of the live and retained maps: the `worktreeId` each orchestrated pane + * key resolves to, as live,retained pairs in entry order. A status write for any other pane + * cannot change the index, so this projection — not the map identities — is the correct cache + * key, and `agentStatus:set` replaces those maps several times a second. + * + * Why exact runtime keys: this preserves early SSH attribution and ignores stale `entry.paneKey` + * fields carried by a live or retained row. + */ +function projectPaneWorktreeIds( + runtimeSource: OrchestrationIndexState['runtimeAgentOrchestrationByPaneKey'], + runtimeEntries: readonly [string, AgentStatusOrchestrationContext][], + agentStatusByPaneKey: OrchestrationIndexState['agentStatusByPaneKey'], + retainedAgentsByPaneKey: OrchestrationIndexState['retainedAgentsByPaneKey'] +): readonly (string | undefined)[] { + // Why memoised on the map identities: every mounted card calls this selector on the same + // publication, and re-walking the contexts per card is the per-card cost the index removes. + if ( + paneWorktreeProjectionCache?.runtimeSource === runtimeSource && + paneWorktreeProjectionCache.liveSource === agentStatusByPaneKey && + paneWorktreeProjectionCache.retainedSource === retainedAgentsByPaneKey + ) { + return paneWorktreeProjectionCache.paneWorktreeIds + } + const paneWorktreeIds: (string | undefined)[] = [] + for (const [paneKey] of runtimeEntries) { + paneWorktreeIds.push( + agentStatusByPaneKey[paneKey]?.worktreeId, + retainedAgentsByPaneKey[paneKey]?.worktreeId + ) + } + paneWorktreeProjectionCache = { + runtimeSource, + liveSource: agentStatusByPaneKey, + retainedSource: retainedAgentsByPaneKey, + paneWorktreeIds + } + return paneWorktreeIds +} + +function hasSameOrderedValues( + previous: readonly (string | undefined)[], + next: readonly (string | undefined)[] +): boolean { + if (previous === next) { + return true + } + if (previous.length !== next.length) { + return false + } + for (let index = 0; index < next.length; index += 1) { + if (previous[index] !== next[index]) { + return false + } + } + return true +} + function reuseRecordIfOrderedEqual( previous: RuntimeOrchestrationRecord | undefined, next: RuntimeOrchestrationRecord @@ -109,12 +181,13 @@ function getWorktreeIdsByTabId( function buildIndex( runtimeEntries: [string, AgentStatusOrchestrationContext][], tabsByWorktree: OrchestrationIndexState['tabsByWorktree'], - agentStatusByPaneKey: OrchestrationIndexState['agentStatusByPaneKey'], - retainedAgentsByPaneKey: OrchestrationIndexState['retainedAgentsByPaneKey'] + paneWorktreeIds: readonly (string | undefined)[] ): ReadonlyMap { + indexBuildCount += 1 const worktreeIdsByTabId = getWorktreeIdsByTabId(tabsByWorktree) const recordsByWorktree = new Map() + let projectionCursor = 0 for (const [paneKey, orchestration] of runtimeEntries) { const parsed = parsePaneKey(paneKey) const parsedParent = orchestration.parentPaneKey @@ -134,13 +207,12 @@ function buildIndex( targets.add(worktreeId) } } - // Why exact runtime keys: this preserves early SSH attribution and ignores - // stale entry.paneKey fields carried by a live or retained row. - const liveWorktreeId = agentStatusByPaneKey[paneKey]?.worktreeId + const liveWorktreeId = paneWorktreeIds[projectionCursor] + const retainedWorktreeId = paneWorktreeIds[projectionCursor + 1] + projectionCursor += 2 if (typeof liveWorktreeId === 'string') { targets.add(liveWorktreeId) } - const retainedWorktreeId = retainedAgentsByPaneKey[paneKey]?.worktreeId if (typeof retainedWorktreeId === 'string') { targets.add(retainedWorktreeId) } @@ -166,16 +238,15 @@ function buildIndex( } /** - * Worktree-keyed index of runtime agent orchestration contexts, rebuilt only - * when one of its four source maps changes identity. + * Worktree-keyed index of runtime agent orchestration contexts, rebuilt only when the context + * map, the tabs slice, or the per-pane worktree projection of the live/retained maps changes. * - * Why: every mounted worktree card subscribes to its own orchestration slice, - * and Zustand re-runs every subscriber's selector on every store publication. - * Scanning the whole context map per card made that O(cards x contexts). What - * this removes is the per-card multiplier, not the rebuild itself: an agent - * ping replaces the live map, so the index still rebuilds once per publication. - * The first caller through a given store version pays O(tabs + contexts); the - * rest are a Map lookup. + * Why: every mounted worktree card subscribes to its own orchestration slice, and Zustand + * re-runs every subscriber's selector on every store publication. Scanning the whole context + * map per card made that O(cards x contexts). Keying on the live and retained map identities + * then made the index rebuild once per `agentStatus:set` even though a status write for an + * unorchestrated pane cannot change a single record; keying on the projection instead is what + * makes those publications free. */ export function selectWorktreeAgentOrchestrationIndex( state: OrchestrationIndexState @@ -184,7 +255,7 @@ export function selectWorktreeAgentOrchestrationIndex( state.runtimeAgentOrchestrationByPaneKey ?? EMPTY_SOURCE // Why cached separately from the index: enumerating the context map is the // per-publication cost this index exists to remove, and the entry list stays - // valid even when a churning live/retained slice forces an index rebuild. + // valid across the live/retained churn the projection absorbs. if (runtimeEntriesCache?.source !== runtimeAgentOrchestrationByPaneKey) { runtimeEntriesCache = { source: runtimeAgentOrchestrationByPaneKey, @@ -198,35 +269,38 @@ export function selectWorktreeAgentOrchestrationIndex( // Why the entries cache survives: dropping it would re-enumerate the empty // map once per card, which is the per-publication cost this index removes. tabMembershipCache = null + paneWorktreeProjectionCache = null orchestrationIndexCache = null return EMPTY_WORKTREE_AGENT_ORCHESTRATION_INDEX } const tabsByWorktree = state.tabsByWorktree ?? EMPTY_SOURCE - const agentStatusByPaneKey = state.agentStatusByPaneKey ?? EMPTY_SOURCE - const retainedAgentsByPaneKey = state.retainedAgentsByPaneKey ?? EMPTY_SOURCE + const paneWorktreeIds = projectPaneWorktreeIds( + runtimeAgentOrchestrationByPaneKey, + runtimeEntries, + state.agentStatusByPaneKey ?? EMPTY_SOURCE, + state.retainedAgentsByPaneKey ?? EMPTY_SOURCE + ) if ( orchestrationIndexCache?.runtimeSource === runtimeAgentOrchestrationByPaneKey && orchestrationIndexCache.tabsSource === tabsByWorktree && - orchestrationIndexCache.liveSource === agentStatusByPaneKey && - orchestrationIndexCache.retainedSource === retainedAgentsByPaneKey + hasSameOrderedValues(orchestrationIndexCache.paneWorktreeIds, paneWorktreeIds) ) { + // Why adopt the equal array: the remaining cards on this publication then compare by + // identity instead of walking it again. + orchestrationIndexCache.paneWorktreeIds = paneWorktreeIds return orchestrationIndexCache.recordsByWorktree } + // buildIndex reuses the previous build's records, so publish the new cache only after it runs. + const recordsByWorktree = buildIndex(runtimeEntries, tabsByWorktree, paneWorktreeIds) orchestrationIndexCache = { runtimeSource: runtimeAgentOrchestrationByPaneKey, tabsSource: tabsByWorktree, - liveSource: agentStatusByPaneKey, - retainedSource: retainedAgentsByPaneKey, - recordsByWorktree: buildIndex( - runtimeEntries, - tabsByWorktree, - agentStatusByPaneKey, - retainedAgentsByPaneKey - ) + paneWorktreeIds, + recordsByWorktree } - return orchestrationIndexCache.recordsByWorktree + return recordsByWorktree } export function selectWorktreeAgentOrchestration( diff --git a/src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.test.tsx b/src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.test.tsx new file mode 100644 index 00000000000..66e283cf754 --- /dev/null +++ b/src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.test.tsx @@ -0,0 +1,111 @@ +// @vitest-environment happy-dom + +/** + * The tab strip must not wake on worktree writes. `projects`/`repos`/`worktreesByRepo` + * exist in the runtime model only to build the Windows shell menu's local project + * runtime; `worktreesByRepo` gets a new identity on every poller result, head-identity + * refresh and git-status write, so an ungated subscription re-renders and re-commits + * every mounted tab strip continuously on a large install. + * + * Runs as a non-Windows client. The menu-on branch is exercised through a `win32` + * host platform, which is how a paired web client on macOS legitimately gets the menu. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, render } from '@testing-library/react' +import { + createTabBarProbeStore, + localProjectRuntimeSpy, + probeWindowsCapabilities, + pushWorktreeWrite, + TAB_BAR_PROBE_PROPS, + tabBarRuntimeModelStubs, + tabBarShellStubs, + tabBarSurfaceRenders, + type TabBarProbeStore +} from './use-tab-bar-runtime-model-worktree-write-probe' +import type { TabBarProps } from './tab-bar-props' + +vi.mock('@/store', async () => ({ useAppStore: await createTabBarProbeStore() })) +vi.mock('../../store', async () => ({ useAppStore: await createTabBarProbeStore() })) +vi.mock('@/hooks/useShortcutLabel', () => tabBarRuntimeModelStubs().shortcutLabels()) +vi.mock('@/hooks/useDetectedAgents', () => tabBarRuntimeModelStubs().detectedAgents()) +vi.mock('@/hooks/useAgentDetectionTarget', () => tabBarRuntimeModelStubs().detectionTarget()) +vi.mock('@/lib/connection-context', () => tabBarRuntimeModelStubs().connectionContext()) +vi.mock('@/lib/worktree-runtime-owner', () => tabBarRuntimeModelStubs().runtimeOwner()) +vi.mock('@/runtime/runtime-rpc-client', () => tabBarRuntimeModelStubs().runtimeRpcClient()) +vi.mock('@/lib/native-chat-transcript-readability', () => + tabBarRuntimeModelStubs().nativeChatReadability() +) +vi.mock('@/lib/client-creation-action-policy', () => tabBarRuntimeModelStubs().creationPolicy()) +vi.mock('./tab-agent-types-by-tab-id', () => tabBarRuntimeModelStubs().agentProjections()) +vi.mock('@/lib/local-preflight-context', () => tabBarRuntimeModelStubs().localPreflight()) +vi.mock('@/lib/windows-terminal-capabilities', () => + tabBarRuntimeModelStubs().windowsCapabilities() +) +vi.mock('./tab-bar-surface', () => tabBarShellStubs().surface()) +vi.mock('./use-tab-bar-create-menu-controller', () => tabBarShellStubs().createMenuController()) +vi.mock('./use-tab-bar-item-projection', () => tabBarShellStubs().itemProjection()) +vi.mock('./tab-strip-overflow-navigation', () => tabBarShellStubs().overflowNavigation()) +vi.mock('./tab-strip-drag-scroll', () => tabBarShellStubs().dragScroll()) +vi.mock('@/lib/pane-manager/client-hosted-browser-row-state', () => + tabBarShellStubs().clientHostedBrowserRows() +) + +const WORKTREE_WRITES = 25 + +async function renderTabBar(): Promise { + const { default: TabBar } = await import('./TabBar') + render() +} + +/** One commit per write, not one batched commit, so the render count is the real one. */ +async function pushWorktreeWrites(store: TabBarProbeStore): Promise { + for (let tick = 0; tick < WORKTREE_WRITES; tick += 1) { + await act(async () => { + pushWorktreeWrite(store, tick) + }) + } +} + +describe('TabBar worktree-write gate (non-Windows client)', () => { + beforeEach(async () => { + Object.defineProperty(navigator, 'userAgent', { + configurable: true, + value: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)' + }) + probeWindowsCapabilities.hostPlatform = 'darwin' + tabBarSurfaceRenders.count = 0 + localProjectRuntimeSpy.mockClear() + ;(await createTabBarProbeStore()).setState({ worktreesByRepo: {}, projects: [], repos: [] }) + }) + + afterEach(() => { + cleanup() + }) + + it('does not re-render the tab strip when worktree writes republish worktreesByRepo', async () => { + const store = await createTabBarProbeStore() + await renderTabBar() + const rendersAtMount = tabBarSurfaceRenders.count + expect(rendersAtMount).toBeGreaterThan(0) + + await pushWorktreeWrites(store) + + expect(tabBarSurfaceRenders.count).toBe(rendersAtMount) + expect(localProjectRuntimeSpy).not.toHaveBeenCalled() + }) + + it('still tracks worktree writes when the Windows shell menu is on', async () => { + const store = await createTabBarProbeStore() + probeWindowsCapabilities.hostPlatform = 'win32' + await renderTabBar() + const rendersAtMount = tabBarSurfaceRenders.count + const runtimeCallsAtMount = localProjectRuntimeSpy.mock.calls.length + expect(runtimeCallsAtMount).toBeGreaterThan(0) + + await pushWorktreeWrites(store) + + expect(tabBarSurfaceRenders.count).toBe(rendersAtMount + WORKTREE_WRITES) + expect(localProjectRuntimeSpy.mock.calls.length).toBe(runtimeCallsAtMount + WORKTREE_WRITES) + }) +}) diff --git a/src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.windows.test.tsx b/src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.windows.test.tsx new file mode 100644 index 00000000000..75a132e47c0 --- /dev/null +++ b/src/renderer/src/components/tab-bar/TabBar.worktree-write-gate.windows.test.tsx @@ -0,0 +1,84 @@ +// @vitest-environment happy-dom + +/** + * Windows half of the tab-strip worktree-write gate. `isWindows` is read once at module + * load, so the two client platforms cannot share a file; see + * TabBar.worktree-write-gate.test.tsx for the non-Windows half and the full rationale. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, render } from '@testing-library/react' +import { + createTabBarProbeStore, + localProjectRuntimeSpy, + probeWindowsCapabilities, + pushWorktreeWrite, + TAB_BAR_PROBE_PROPS, + tabBarRuntimeModelStubs, + tabBarShellStubs, + tabBarSurfaceRenders +} from './use-tab-bar-runtime-model-worktree-write-probe' +import type { TabBarProps } from './tab-bar-props' + +vi.mock('@/store', async () => ({ useAppStore: await createTabBarProbeStore() })) +vi.mock('../../store', async () => ({ useAppStore: await createTabBarProbeStore() })) +vi.mock('@/hooks/useShortcutLabel', () => tabBarRuntimeModelStubs().shortcutLabels()) +vi.mock('@/hooks/useDetectedAgents', () => tabBarRuntimeModelStubs().detectedAgents()) +vi.mock('@/hooks/useAgentDetectionTarget', () => tabBarRuntimeModelStubs().detectionTarget()) +vi.mock('@/lib/connection-context', () => tabBarRuntimeModelStubs().connectionContext()) +vi.mock('@/lib/worktree-runtime-owner', () => tabBarRuntimeModelStubs().runtimeOwner()) +vi.mock('@/runtime/runtime-rpc-client', () => tabBarRuntimeModelStubs().runtimeRpcClient()) +vi.mock('@/lib/native-chat-transcript-readability', () => + tabBarRuntimeModelStubs().nativeChatReadability() +) +vi.mock('@/lib/client-creation-action-policy', () => tabBarRuntimeModelStubs().creationPolicy()) +vi.mock('./tab-agent-types-by-tab-id', () => tabBarRuntimeModelStubs().agentProjections()) +vi.mock('@/lib/local-preflight-context', () => tabBarRuntimeModelStubs().localPreflight()) +vi.mock('@/lib/windows-terminal-capabilities', () => + tabBarRuntimeModelStubs().windowsCapabilities() +) +vi.mock('./tab-bar-surface', () => tabBarShellStubs().surface()) +vi.mock('./use-tab-bar-create-menu-controller', () => tabBarShellStubs().createMenuController()) +vi.mock('./use-tab-bar-item-projection', () => tabBarShellStubs().itemProjection()) +vi.mock('./tab-strip-overflow-navigation', () => tabBarShellStubs().overflowNavigation()) +vi.mock('./tab-strip-drag-scroll', () => tabBarShellStubs().dragScroll()) +vi.mock('@/lib/pane-manager/client-hosted-browser-row-state', () => + tabBarShellStubs().clientHostedBrowserRows() +) + +const WORKTREE_WRITES = 25 + +describe('TabBar worktree-write gate (Windows client)', () => { + beforeEach(async () => { + Object.defineProperty(navigator, 'userAgent', { + configurable: true, + value: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' + }) + // A Windows client gets the shell menu before its capability probe resolves. + probeWindowsCapabilities.hostPlatform = null + tabBarSurfaceRenders.count = 0 + localProjectRuntimeSpy.mockClear() + ;(await createTabBarProbeStore()).setState({ worktreesByRepo: {}, projects: [], repos: [] }) + }) + + afterEach(() => { + cleanup() + }) + + it('keeps recomputing the local project runtime on every worktree write', async () => { + const store = await createTabBarProbeStore() + const { default: TabBar } = await import('./TabBar') + render() + const rendersAtMount = tabBarSurfaceRenders.count + const runtimeCallsAtMount = localProjectRuntimeSpy.mock.calls.length + expect(runtimeCallsAtMount).toBeGreaterThan(0) + + for (let tick = 0; tick < WORKTREE_WRITES; tick += 1) { + await act(async () => { + pushWorktreeWrite(store, tick) + }) + } + + expect(tabBarSurfaceRenders.count).toBe(rendersAtMount + WORKTREE_WRITES) + expect(localProjectRuntimeSpy.mock.calls.length).toBe(runtimeCallsAtMount + WORKTREE_WRITES) + }) +}) diff --git a/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model-worktree-write-probe.ts b/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model-worktree-write-probe.ts new file mode 100644 index 00000000000..028316b9b84 --- /dev/null +++ b/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model-worktree-write-probe.ts @@ -0,0 +1,164 @@ +import { vi } from 'vitest' +import type { WindowsTerminalCapabilities } from '@/lib/windows-terminal-capabilities' + +/** + * Shared rig for the tab-strip worktree-write gate tests. The platform check in + * `use-tab-bar-runtime-model` is read once at module load, so each platform needs + * its own test file; everything but the `vi.mock` calls lives here. + */ +export type TabBarProbeState = { + settings: Record | null + persistedUIReady: boolean + mobileEmulatorTabIntroDismissed: boolean + gitStatusByWorktree: Record + unifiedTabsByWorktree: Record + activeGroupIdByWorktree: Record + activeRepoId: string | null + activeWorktreeId: string | null + projects: unknown[] + repos: unknown[] + worktreesByRepo: Record + sshConnectionStates: Map + pinTab: (tabId: string) => void + unpinTab: (tabId: string) => void + toggleTabViewMode: (tabId: string) => void +} + +export type TabBarProbeStore = { + (selector: (state: TabBarProbeState) => unknown): unknown + getState: () => TabBarProbeState + setState: (partial: Partial) => void +} + +const noop = (): void => {} + +// Both specifiers resolve to the same store module; memoize so they share one instance. +export async function createTabBarProbeStore(): Promise { + const globalKey = '__tabBarRuntimeModelProbeStore' + const globals = globalThis as Record + if (!globals[globalKey]) { + const { create } = await import('zustand') + globals[globalKey] = create(() => ({ + settings: null, + persistedUIReady: true, + mobileEmulatorTabIntroDismissed: true, + gitStatusByWorktree: {}, + unifiedTabsByWorktree: {}, + activeGroupIdByWorktree: {}, + activeRepoId: null, + activeWorktreeId: null, + projects: [], + repos: [], + worktreesByRepo: {}, + sshConnectionStates: new Map(), + pinTab: noop, + unpinTab: noop, + toggleTabViewMode: noop + })) + } + return globals[globalKey] as TabBarProbeStore +} + +/** Mutable so a test can flip the probed host platform without changing identity. */ +export const probeWindowsCapabilities: WindowsTerminalCapabilities = { + wslAvailable: false, + wslDistros: [], + pwshAvailable: false, + gitBashAvailable: false, + hostPlatform: 'darwin', + isLoading: false +} + +export const localProjectRuntimeSpy = vi.fn(() => undefined) + +const AGENT_PROJECTIONS = Object.freeze({ + nativeChatEnabled: false, + tabAgentTypesByTabId: Object.freeze({}), + nativeChatTabWideFallbackUnsafeTabsById: Object.freeze({}) +}) +const CREATION_POLICY = Object.freeze({ + 'managed-browser': { state: 'enabled' }, + 'mobile-emulator': { state: 'enabled' } +}) +const DETECTED_AGENTS = Object.freeze({ detectedIds: Object.freeze([]) }) +const RUNTIME_TARGET = Object.freeze({ kind: 'local' }) +const CREATE_MENU = Object.freeze({}) +const ITEM_PROJECTION = Object.freeze({ + orderedItems: Object.freeze([]), + activeVisibleTabId: null, + tabStripLayoutKey: 'probe' +}) +const OVERFLOW_NAVIGATION = Object.freeze({ + scrollTabStrip: noop, + tabStripOverflowState: Object.freeze({ canScrollStart: false, canScrollEnd: false }) +}) +const DRAG_SCROLL = Object.freeze({ + isTabDragActive: false, + onDragScrollStartEnter: noop, + onDragScrollEndEnter: noop, + onDragScrollLeave: noop +}) + +export function tabBarRuntimeModelStubs(): Record Record> { + return { + shortcutLabels: () => ({ + useShortcutLabel: () => '', + useOptionalShortcutLabel: () => null + }), + detectedAgents: () => ({ useDetectedAgents: () => DETECTED_AGENTS }), + detectionTarget: () => ({ useAgentDetectionTargetForWorktree: () => null }), + connectionContext: () => ({ getConnectionIdFromState: () => null }), + runtimeOwner: () => ({ getRuntimeEnvironmentIdForWorktree: () => null }), + runtimeRpcClient: () => ({ getActiveRuntimeTarget: () => RUNTIME_TARGET }), + nativeChatReadability: () => ({ isNativeChatTranscriptLocalReadable: () => false }), + creationPolicy: () => ({ getClientCreationActionPolicy: () => CREATION_POLICY }), + agentProjections: () => ({ selectTabBarAgentProjections: () => AGENT_PROJECTIONS }), + localPreflight: () => ({ + getLocalProjectExecutionRuntimeContext: localProjectRuntimeSpy + }), + windowsCapabilities: () => ({ + getWindowsTerminalCapabilityOwnerKey: () => 'probe', + useWindowsTerminalCapabilities: () => probeWindowsCapabilities + }) + } +} + +export const tabBarSurfaceRenders = { count: 0 } + +export function tabBarShellStubs(): Record Record> { + return { + surface: () => ({ + renderTabBarSurface: () => { + tabBarSurfaceRenders.count += 1 + return null + } + }), + createMenuController: () => ({ useTabBarCreateMenuController: () => CREATE_MENU }), + itemProjection: () => ({ useTabBarItemProjection: () => ITEM_PROJECTION }), + overflowNavigation: () => ({ useTabStripOverflowNavigation: () => OVERFLOW_NAVIGATION }), + dragScroll: () => ({ useTabStripDragScrollHandlers: () => DRAG_SCROLL }), + clientHostedBrowserRows: () => ({ useActiveClientHostedBrowserRowId: () => null }) + } +} + +export const TAB_BAR_PROBE_PROPS = { + tabs: [], + activeTabId: null, + worktreeId: 'wt-target', + expandedPaneByTabId: {}, + onActivate: noop, + onClose: noop, + onCloseOthers: noop, + onCloseToRight: noop, + onCloseToLeft: noop, + onNewTerminalTab: noop, + onNewBrowserTab: noop, + onSetCustomTitle: noop, + onSetTabColor: noop, + onTogglePaneExpand: noop +} as const + +/** One fresh `worktreesByRepo` identity, exactly as a worktree write publishes it. */ +export function pushWorktreeWrite(store: TabBarProbeStore, tick: number): void { + store.setState({ worktreesByRepo: { 'repo-1': [{ id: `wt-${tick}`, repoId: 'repo-1' }] } }) +} diff --git a/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model.ts b/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model.ts index f4dfb67fb49..3436b2200f2 100644 --- a/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model.ts +++ b/src/renderer/src/components/tab-bar/use-tab-bar-runtime-model.ts @@ -39,6 +39,9 @@ type GitStatusEntries = AppStoreState['gitStatusByWorktree'][string] const EMPTY_GIT_STATUS_ENTRIES: GitStatusEntries = [] const EMPTY_AGENT_CMD_OVERRIDES: Partial> = {} const EMPTY_UNIFIED_TABS: readonly Tab[] = [] +const EMPTY_PROJECTS: AppStoreState['projects'] = [] +const EMPTY_REPOS: AppStoreState['repos'] = [] +const EMPTY_WORKTREES_BY_REPO: AppStoreState['worktreesByRepo'] = {} export function getProjectRuntimeShellMenuMode( projectRuntime: ProjectExecutionRuntimeResolution | undefined @@ -123,10 +126,7 @@ export function useTabBarRuntimeModel({ ) const activeRepoId = useAppStore((s) => s.activeRepoId) const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) - const projects = useAppStore((s) => s.projects) - const repos = useAppStore((s) => s.repos) const settings = useAppStore((s) => s.settings) - const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) // Why: use the worktree's owning host so offered Windows shells match the host that actually runs the terminal. const activeRuntimeEnvironmentId = useAppStore( (s) => getRuntimeEnvironmentIdForWorktree(s, worktreeId)?.trim() || null @@ -188,8 +188,17 @@ export function useTabBarRuntimeModel({ isWindowsClient: isWindows, worktreeHasRemoteConnection: Boolean(worktreeConnectionId) }) + // Why: `projects`/`repos`/`worktreesByRepo` feed nothing but the local runtime context below, and + // `worktreesByRepo` churns on every worktree write; ungated, each write re-renders every tab strip. + const needsLocalProjectRuntime = + showWindowsShellMenu && !activeRuntimeEnvironmentId?.trim() && !worktreeConnectionId + const projects = useAppStore((s) => (needsLocalProjectRuntime ? s.projects : EMPTY_PROJECTS)) + const repos = useAppStore((s) => (needsLocalProjectRuntime ? s.repos : EMPTY_REPOS)) + const worktreesByRepo = useAppStore((s) => + needsLocalProjectRuntime ? s.worktreesByRepo : EMPTY_WORKTREES_BY_REPO + ) const localProjectRuntime = useMemo(() => { - if (!showWindowsShellMenu || activeRuntimeEnvironmentId?.trim() || worktreeConnectionId) { + if (!needsLocalProjectRuntime) { return undefined } return getLocalProjectExecutionRuntimeContext( @@ -207,13 +216,11 @@ export function useTabBarRuntimeModel({ ) }, [ activeRepoId, - activeRuntimeEnvironmentId, activeWorktreeId, + needsLocalProjectRuntime, projects, repos, settings, - showWindowsShellMenu, - worktreeConnectionId, windowsTerminalCapabilities.isLoading, windowsTerminalCapabilities.wslAvailable, windowsTerminalCapabilities.wslDistros,