From 36384f17f41caa5cdbdf12c3e860d64ef6fe622c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:56:01 -0700 Subject: [PATCH] Avoid no-op worktree activation fanout (#4416) --- docs/renderer-memory-profile-2026-06-01.md | 13 +++++ .../src/store/slices/store-cascades.test.ts | 56 +++++++++++++++++++ src/renderer/src/store/slices/worktrees.ts | 23 +++++++- 3 files changed, 91 insertions(+), 1 deletion(-) diff --git a/docs/renderer-memory-profile-2026-06-01.md b/docs/renderer-memory-profile-2026-06-01.md index d047ee525..2501e7b8b 100644 --- a/docs/renderer-memory-profile-2026-06-01.md +++ b/docs/renderer-memory-profile-2026-06-01.md @@ -129,3 +129,16 @@ The follow-up extends the existing tab-registration wait from page-specific creation to worktree/global wake flows. Runtime commands now wait for the renderer's actual `browser:registerGuest` IPC before routing automation, with the same timeout fallback used by tab creation. + +## Follow-up: Worktree Activation No-Op Fanout + +The next renderer-store check found that repeated activation of an already +active, already-reconciled worktree could still publish a new Zustand root state +because `setActiveWorktree` rebuilt `activeTabTypeByWorktree` even when its +stored value was unchanged. That woke every store subscriber, including session +persistence and runtime graph sync, for a visible no-op. + +The follow-up preserves the existing state reference when all derived active +fields, unread state, and first-activation bookkeeping are unchanged. A +regression test subscribes to the store and asserts that reselecting the +already-active reconciled worktree does not notify subscribers. diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index db91aa744..15f64b127 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -761,6 +761,62 @@ describe('setActiveWorktree', () => { expect(store.getState().rightSidebarTab).toBe('checks') }) + it('does not notify subscribers when reselecting the already-active reconciled worktree', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + const tabId = 'terminal-1' + const groupId = 'group-1' + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + activeWorktreeId: wt, + activeTabId: tabId, + activeTabType: 'terminal', + activeTabTypeByWorktree: { [wt]: 'terminal' }, + tabsByWorktree: { + [wt]: [makeTab({ id: tabId, worktreeId: wt, ptyId: 'pty-1' })] + }, + ptyIdsByTabId: { [tabId]: ['pty-1'] }, + unifiedTabsByWorktree: { + [wt]: [ + makeUnifiedTab({ + id: tabId, + entityId: tabId, + worktreeId: wt, + groupId, + contentType: 'terminal' + }) + ] + }, + groupsByWorktree: { + [wt]: [ + makeTabGroup({ + id: groupId, + worktreeId: wt, + activeTabId: tabId, + tabOrder: [tabId] + }) + ] + }, + activeGroupIdByWorktree: { [wt]: groupId }, + everActivatedWorktreeIds: new Set([wt]), + refreshGitHubForWorktree: vi.fn(), + refreshGitHubForWorktreeIfStale: vi.fn() + }) + + const before = store.getState() + const listener = vi.fn() + const unsubscribe = store.subscribe(listener) + + store.getState().setActiveWorktree(wt) + + unsubscribe() + expect(listener).not.toHaveBeenCalled() + expect(store.getState()).toBe(before) + }) + it('does not clobber the current right sidebar tab when clearing the active worktree', () => { const store = createTestStore() diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 39005b286..5a8a8899e 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -2145,12 +2145,33 @@ export const createWorktreeSlice: StateCreator ? applyDetectedWorktreeUpdates(s.detectedWorktreesByRepo, worktreeId, metaUpdates) : s.detectedWorktreesByRepo + const nextActiveTabTypeByWorktree = + s.activeTabTypeByWorktree[worktreeId] === activeTabType + ? s.activeTabTypeByWorktree + : { ...s.activeTabTypeByWorktree, [worktreeId]: activeTabType } + const hasStateChange = + s.activeWorktreeId !== worktreeId || + s.activeFileId !== activeFileId || + s.activeBrowserTabId !== activeBrowserTabId || + s.activeTabType !== activeTabType || + s.activeTabId !== activeTabId || + nextActiveTabTypeByWorktree !== s.activeTabTypeByWorktree || + nextEverActivated !== s.everActivatedWorktreeIds || + nextWorktrees !== s.worktreesByRepo || + nextDetectedWorktrees !== s.detectedWorktreesByRepo + if (!hasStateChange) { + // Why: repeated activation of the already-active worktree can come from + // clicks, IPC, and automation restore paths. Preserve the root Zustand + // reference so session persistence/runtime sync do not fan out on a no-op. + return s + } + return { activeWorktreeId: worktreeId, activeFileId, activeBrowserTabId, activeTabType, - activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: activeTabType }, + activeTabTypeByWorktree: nextActiveTabTypeByWorktree, activeTabId, everActivatedWorktreeIds: nextEverActivated, ...(nextWorktrees !== s.worktreesByRepo ? { worktreesByRepo: nextWorktrees } : {}),