From 9769b85ae717d0a341782e7cdcc64a416e6b051d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 1 Jun 2026 16:06:21 -0700 Subject: [PATCH] Skip visit writes for active worktree reselect (#4417) --- docs/renderer-memory-profile-2026-06-01.md | 12 ++ .../worktree-activation-created-agent.test.ts | 118 ++++++++++++++++++ src/renderer/src/lib/worktree-activation.ts | 16 ++- 3 files changed, 144 insertions(+), 2 deletions(-) diff --git a/docs/renderer-memory-profile-2026-06-01.md b/docs/renderer-memory-profile-2026-06-01.md index 2501e7b8b..f31929cf4 100644 --- a/docs/renderer-memory-profile-2026-06-01.md +++ b/docs/renderer-memory-profile-2026-06-01.md @@ -142,3 +142,15 @@ 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. + +## Follow-up: Activation Helper Visit Writes + +After the store-level no-op fix, the higher-level `activateAndRevealWorktree` +helper could still restamp focus-recency and append navigation history for a +plain reselect of the already-active worktree in terminal view. That path did +not change the visible workspace, but the recency stamp is part of the persisted +session payload and can still wake the session writer. + +The follow-up skips only that true no-op visit write. Activations that switch +repo, leave another app view, or carry startup/setup/default-tab work still +record the visit, and the sidebar reveal still runs for the no-op case. diff --git a/src/renderer/src/lib/worktree-activation-created-agent.test.ts b/src/renderer/src/lib/worktree-activation-created-agent.test.ts index 09c0eee30..3243776fb 100644 --- a/src/renderer/src/lib/worktree-activation-created-agent.test.ts +++ b/src/renderer/src/lib/worktree-activation-created-agent.test.ts @@ -34,7 +34,125 @@ function makeWorktree(): Worktree { } } +function seedAlreadyActiveWorktree( + worktree: Worktree, + overrides: Partial> = {} +): { + markWorktreeVisited: ReturnType + recordWorktreeVisit: ReturnType + revealWorktreeInSidebar: ReturnType +} { + const markWorktreeVisited = vi.fn() + const recordWorktreeVisit = vi.fn() + const revealWorktreeInSidebar = vi.fn() + + useAppStore.setState({ + repos: [ + { + id: worktree.repoId, + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000000', + addedAt: 0 + } + ], + worktreesByRepo: { [worktree.repoId]: [worktree] }, + activeRepoId: worktree.repoId, + activeView: 'terminal', + activeWorktreeId: worktree.id, + activeTabId: 'tab-1', + activeTabType: 'terminal', + tabsByWorktree: { + [worktree.id]: [ + { + id: 'tab-1', + ptyId: 'pty-1', + worktreeId: worktree.id, + title: 'Terminal 1', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + unifiedTabsByWorktree: { + [worktree.id]: [ + { + id: 'tab-1', + entityId: 'tab-1', + groupId: 'group-1', + worktreeId: worktree.id, + contentType: 'terminal', + label: 'Terminal 1', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + groupsByWorktree: { + [worktree.id]: [ + { + id: 'group-1', + worktreeId: worktree.id, + activeTabId: 'tab-1', + tabOrder: ['tab-1'] + } + ] + }, + activeGroupIdByWorktree: { [worktree.id]: 'group-1' }, + activeTabTypeByWorktree: { [worktree.id]: 'terminal' }, + everActivatedWorktreeIds: new Set([worktree.id]), + openFiles: [], + browserTabsByWorktree: {}, + activeFileIdByWorktree: {}, + activeBrowserTabIdByWorktree: {}, + activeTabIdByWorktree: { [worktree.id]: 'tab-1' }, + tabBarOrderByWorktree: {}, + settings: { + agentCmdOverrides: {}, + setupScriptLaunchMode: 'new-tab' + } as unknown as ReturnType['settings'], + markWorktreeVisited, + recordWorktreeVisit, + refreshGitHubForWorktreeIfStale: vi.fn(), + revealWorktreeInSidebar, + ...overrides + }) + + return { markWorktreeVisited, recordWorktreeVisit, revealWorktreeInSidebar } +} + describe('activateAndRevealWorktree created agent reopen', () => { + it('does not restamp focus recency when reselecting the already-active terminal worktree', () => { + const worktree = makeWorktree() + const { markWorktreeVisited, recordWorktreeVisit, revealWorktreeInSidebar } = + seedAlreadyActiveWorktree(worktree) + + const result = activateAndRevealWorktree(worktree.id) + + expect(result).toEqual({ primaryTabId: null }) + expect(markWorktreeVisited).not.toHaveBeenCalled() + expect(recordWorktreeVisit).not.toHaveBeenCalled() + expect(revealWorktreeInSidebar).toHaveBeenCalledWith(worktree.id) + }) + + it('records a visit when activating the same worktree changes the current view', () => { + const worktree = makeWorktree() + const { markWorktreeVisited, recordWorktreeVisit } = seedAlreadyActiveWorktree(worktree, { + activeView: 'tasks' + }) + + const result = activateAndRevealWorktree(worktree.id) + + expect(result).toEqual({ primaryTabId: null }) + expect(markWorktreeVisited).toHaveBeenCalledWith(worktree.id) + expect(recordWorktreeVisit).toHaveBeenCalledWith(worktree.id) + }) + it('reopens an empty worktree with the agent selected at creation time', () => { const worktree = makeWorktree() const revealWorktreeInSidebar = vi.fn() diff --git a/src/renderer/src/lib/worktree-activation.ts b/src/renderer/src/lib/worktree-activation.ts index dcf93e53c..b210a4e55 100644 --- a/src/renderer/src/lib/worktree-activation.ts +++ b/src/renderer/src/lib/worktree-activation.ts @@ -154,6 +154,16 @@ export function activateAndRevealWorktree( if (!wt) { return false } + const hasActivationWork = Boolean( + opts?.startup || opts?.setup || opts?.defaultTabs || opts?.issueCommand + ) + // Why: a plain reselect of the already-visible workspace should still reveal + // the sidebar row, but it must not restamp focus recency and wake persistence. + const isPlainAlreadyActiveTerminal = + !hasActivationWork && + state.activeRepoId === wt.repoId && + state.activeWorktreeId === worktreeId && + state.activeView === 'terminal' // 1. Set activeRepoId if crossing repos if (wt.repoId !== state.activeRepoId) { @@ -181,14 +191,16 @@ export function activateAndRevealWorktree( // flips, so the recency stamp must land with the same guarantee. Separate // from recordWorktreeVisit (nav-history) and from worktree.lastActivityAt // (background signal) on purpose — see docs/cmd-j-empty-query-ordering.md. - state.markWorktreeVisited(worktreeId) + if (!isPlainAlreadyActiveTerminal) { + state.markWorktreeVisited(worktreeId) + } // Why: activateAndRevealWorktree always ends in 'terminal' view (step 2), // and Settings/Tasks transitions do not pass through this function, so no // view-guard is needed here. The guard skips re-recording when the caller // is goBackWorktree/goForwardWorktree, which mutate the history index // directly instead of treating the target as a new visit. - if (!state.isNavigatingHistory) { + if (!isPlainAlreadyActiveTerminal && !state.isNavigatingHistory) { state.recordWorktreeVisit(worktreeId) }