From dede24df46a80ced43b7a732d820f768cbbd72e7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 14 Sep 2026 13:33:19 -0700 Subject: [PATCH] fix(store): preserve state identity for no-op updater branches (#20703) --- .../tab-drag-preview-activation.test.ts | 29 +++- .../tab-group/tab-drag-preview-activation.ts | 36 ++++- .../src/store/github/project-cache.ts | 4 +- .../store/github/pull-request-execution.ts | 4 +- .../github/work-item-mutation-actions.ts | 2 +- .../slices/browser/browser-host-actions.ts | 4 +- .../slices/browser/browser-host-state.ts | 2 +- .../browser/browser-hydration-actions.ts | 2 +- .../browser/browser-profile-import-actions.ts | 8 +- .../store/slices/commit-message-generation.ts | 4 +- .../store/slices/diff-comment-persistence.ts | 18 +-- .../slices/empty-update-notifications.test.ts | 81 ++++++++++ ...hub-pr-branch-linked-pr-divergence.test.ts | 4 + .../github-work-item-cache-identity.test.ts | 9 ++ .../slices/hosted-review-cache-race.test.ts | 4 + .../src/store/slices/hosted-review.ts | 2 +- .../store/slices/jira-issue-patch-action.ts | 2 +- .../linear/linear-invalidation-actions.ts | 2 +- .../store/slices/pull-request-generation.ts | 4 +- .../src/store/slices/sparse-presets.ts | 2 +- .../store/slices/tabs/tabs-create-actions.ts | 2 +- .../store/slices/tabs/tabs-drop-actions.ts | 8 +- .../store/slices/tabs/tabs-focus-actions.ts | 2 +- .../store/slices/tabs/tabs-label-actions.ts | 35 +++-- .../store/slices/tabs/tabs-move-actions.ts | 6 +- .../create/pending-worktree-creation.ts | 8 +- .../metadata/hosted-review-link-mutation.ts | 2 +- .../metadata/update-worktree-meta.ts | 4 +- .../metadata/update-worktrees-meta.ts | 2 +- .../session/migrate-worktree-identity.ts | 5 +- .../worktrees/session/set-active-worktree.ts | 6 +- .../worktree-no-op-notifications.test.ts | 64 ++++++++ .../session/worktree-slice-lookups.ts | 6 +- .../session/worktree-unread-activity.ts | 4 +- .../session/worktree-visit-recency.ts | 8 +- .../teardown/worktree-delete-state.ts | 6 +- .../terminal-disowned-pty-sources.ts | 2 +- .../terminals/terminal-ephemeral-state.ts | 16 +- .../store/terminals/terminal-layout-state.ts | 2 +- .../terminal-no-op-subscriber.test.ts | 146 ++++++++++++++++++ .../store/terminals/terminal-restart-state.ts | 10 +- .../terminals/terminal-startup-queues.ts | 2 +- .../terminals/terminal-unverified-pty-loss.ts | 2 +- 43 files changed, 472 insertions(+), 99 deletions(-) create mode 100644 src/renderer/src/store/slices/empty-update-notifications.test.ts create mode 100644 src/renderer/src/store/slices/worktrees/session/worktree-no-op-notifications.test.ts create mode 100644 src/renderer/src/store/terminals/terminal-no-op-subscriber.test.ts diff --git a/src/renderer/src/components/tab-group/tab-drag-preview-activation.test.ts b/src/renderer/src/components/tab-group/tab-drag-preview-activation.test.ts index 019b3e297ee..1269ade41a4 100644 --- a/src/renderer/src/components/tab-group/tab-drag-preview-activation.test.ts +++ b/src/renderer/src/components/tab-group/tab-drag-preview-activation.test.ts @@ -1,10 +1,11 @@ -import { beforeEach, describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import type { Tab } from '../../../../shared/tab-types' import { useAppStore } from '../../store' import { applyDragPreviewTab, captureTabDragActivationSnapshot, - restoreTabDragActivationSnapshot + restoreTabDragActivationSnapshot, + restoreSourceGroupActiveTabAfterCrossGroupDrop } from './tab-drag-preview-activation' const WT = 'wt-preview-restore' @@ -59,6 +60,30 @@ describe('restoreTabDragActivationSnapshot', () => { }) }) + it('does not publish repeated preview and restore actions', () => { + const snapshot = captureTabDragActivationSnapshot(WT) + const subscriber = vi.fn() + const unsubscribe = useAppStore.subscribe(subscriber) + try { + applyDragPreviewTab({ + worktreeId: WT, + groupId: 'group-1', + tabId: 'tab-1', + activeGroupId: 'group-1' + }) + restoreTabDragActivationSnapshot(WT, snapshot) + restoreSourceGroupActiveTabAfterCrossGroupDrop({ + worktreeId: WT, + snapshot, + sourceGroupId: 'group-1', + movedTabId: 'tab-2' + }) + expect(subscriber).not.toHaveBeenCalled() + } finally { + unsubscribe() + } + }) + it('restores active-surface fields after a drag preview is cancelled', () => { const snapshot = captureTabDragActivationSnapshot(WT) diff --git a/src/renderer/src/components/tab-group/tab-drag-preview-activation.ts b/src/renderer/src/components/tab-group/tab-drag-preview-activation.ts index a0060271c99..31726672db2 100644 --- a/src/renderer/src/components/tab-group/tab-drag-preview-activation.ts +++ b/src/renderer/src/components/tab-group/tab-drag-preview-activation.ts @@ -30,6 +30,14 @@ function previewActiveSurfacePatch( }) if (unifiedTab.contentType === 'terminal') { + if ( + state.activeTabType === 'terminal' && + state.activeTabTypeByWorktree[worktreeId] === 'terminal' && + state.activeTabId === unifiedTab.entityId && + state.activeTabIdByWorktree[worktreeId] === unifiedTab.entityId + ) { + return {} + } return { activeTabId: unifiedTab.entityId, activeTabType: 'terminal', @@ -41,6 +49,14 @@ function previewActiveSurfacePatch( } } if (unifiedTab.contentType === 'browser') { + if ( + state.activeTabType === 'browser' && + state.activeTabTypeByWorktree[worktreeId] === 'browser' && + state.activeBrowserTabId === unifiedTab.entityId && + state.activeBrowserTabIdByWorktree[worktreeId] === unifiedTab.entityId + ) { + return {} + } return { activeBrowserTabId: unifiedTab.entityId, activeTabType: 'browser', @@ -52,11 +68,25 @@ function previewActiveSurfacePatch( } } if (unifiedTab.contentType === 'simulator') { + if ( + state.activeTabType === 'simulator' && + state.activeTabTypeByWorktree[worktreeId] === 'simulator' + ) { + return {} + } return { activeTabType: 'simulator', activeTabTypeByWorktree: nextActiveTabTypeByWorktree('simulator') } } + if ( + state.activeTabType === 'editor' && + state.activeTabTypeByWorktree[worktreeId] === 'editor' && + state.activeFileId === unifiedTab.entityId && + state.activeFileIdByWorktree[worktreeId] === unifiedTab.entityId + ) { + return {} + } return { activeFileId: unifiedTab.entityId, activeTabType: 'editor', @@ -95,7 +125,7 @@ export function applyDragPreviewTab({ const focusUnchanged = (state.activeGroupIdByWorktree[worktreeId] ?? null) === activeGroupId const surfacePatch = previewActiveSurfacePatch(state, worktreeId, groupId, tabId) if (groupUnchanged && focusUnchanged) { - return Object.keys(surfacePatch).length > 0 ? surfacePatch : {} + return Object.keys(surfacePatch).length > 0 ? surfacePatch : state } const next: Partial = { ...surfacePatch } @@ -162,7 +192,7 @@ export function restoreTabDragActivationSnapshot( } if (Object.keys(next).length === 0) { - return {} + return state } return next @@ -191,7 +221,7 @@ export function restoreSourceGroupActiveTabAfterCrossGroupDrop({ const groups = state.groupsByWorktree[worktreeId] ?? [] const sourceGroup = groups.find((group) => group.id === sourceGroupId) if (!sourceGroup || sourceGroup.activeTabId === preDragActiveTabId) { - return {} + return state } return { groupsByWorktree: { diff --git a/src/renderer/src/store/github/project-cache.ts b/src/renderer/src/store/github/project-cache.ts index df8ac361025..eb296633201 100644 --- a/src/renderer/src/store/github/project-cache.ts +++ b/src/renderer/src/store/github/project-cache.ts @@ -76,11 +76,11 @@ export function applyRowPatch( set((s) => { const entry = s.projectViewCache[cacheKey] if (!entry?.data) { - return {} + return s } const rowIndex = entry.data.rows.findIndex((r) => r.id === rowId) if (rowIndex === -1) { - return {} + return s } const rows = [...entry.data.rows] rows[rowIndex] = nextRow diff --git a/src/renderer/src/store/github/pull-request-execution.ts b/src/renderer/src/store/github/pull-request-execution.ts index 72069802064..553ef5c1e13 100644 --- a/src/renderer/src/store/github/pull-request-execution.ts +++ b/src/renderer/src/store/github/pull-request-execution.ts @@ -153,7 +153,7 @@ export function startPullRequestLookup(args: { // Why: unlinking a PR mid exact-linked-PR-lookup must stop the older result from restoring the manual link UI. if (isStaleExactLinkedPRLookup(s, options?.worktreeId, linkedPRNumber)) { skippedStaleLinkedPRLookup = true - return {} + return s } const updates = setGitHubPRResultCaches(s, { prCacheKey: cacheKey, @@ -174,7 +174,7 @@ export function startPullRequestLookup(args: { requestStartedEntry: requestStartedHostedReviewEntry }) didUpdatePRCache = updates.prCache !== undefined - return updates + return updates.prCache || updates.hostedReviewCache ? updates : s }) if (skippedStaleLinkedPRLookup) { return null diff --git a/src/renderer/src/store/github/work-item-mutation-actions.ts b/src/renderer/src/store/github/work-item-mutation-actions.ts index e1d02a09d3b..97a6d3b66f3 100644 --- a/src/renderer/src/store/github/work-item-mutation-actions.ts +++ b/src/renderer/src/store/github/work-item-mutation-actions.ts @@ -45,7 +45,7 @@ export const createWorkItemMutationActions = ( nextCache[key] = { ...entry, data: updatedItems } changed = true } - return changed ? { workItemsCache: nextCache } : {} + return changed ? { workItemsCache: nextCache } : s }) }, diff --git a/src/renderer/src/store/slices/browser/browser-host-actions.ts b/src/renderer/src/store/slices/browser/browser-host-actions.ts index 2977717d38c..193dfdafe60 100644 --- a/src/renderer/src/store/slices/browser/browser-host-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-host-actions.ts @@ -24,7 +24,7 @@ export function createBrowserHostActions( closes, Date.now() ) - return next ? { clientHostedBrowserCloseIntentsByEnvironment: next } : {} + return next ? { clientHostedBrowserCloseIntentsByEnvironment: next } : s }) }, @@ -34,7 +34,7 @@ export function createBrowserHostActions( s.clientHostedBrowserCloseIntentsByEnvironment, { environmentId, browserPageIds, now: Date.now() } ) - return next ? { clientHostedBrowserCloseIntentsByEnvironment: next } : {} + return next ? { clientHostedBrowserCloseIntentsByEnvironment: next } : s }) }, diff --git a/src/renderer/src/store/slices/browser/browser-host-state.ts b/src/renderer/src/store/slices/browser/browser-host-state.ts index 1b4b3a70e9a..7ee75ac54a1 100644 --- a/src/renderer/src/store/slices/browser/browser-host-state.ts +++ b/src/renderer/src/store/slices/browser/browser-host-state.ts @@ -153,7 +153,7 @@ export function browserImportStateForHostUpdate( hostId: ExecutionHostId, browserSessionImportState: BrowserSlice['browserSessionImportState'] ): Partial { - return getBrowserSettingsHostId(state) === hostId ? { browserSessionImportState } : {} + return getBrowserSettingsHostId(state) === hostId ? { browserSessionImportState } : state } export function getFallbackTabTypeForWorktree( diff --git a/src/renderer/src/store/slices/browser/browser-hydration-actions.ts b/src/renderer/src/store/slices/browser/browser-hydration-actions.ts index a2ed3f5bbb1..13beee05872 100644 --- a/src/renderer/src/store/slices/browser/browser-hydration-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-hydration-actions.ts @@ -257,7 +257,7 @@ export function createBrowserHydrationActions( } } } - return {} + return s }) } } diff --git a/src/renderer/src/store/slices/browser/browser-profile-import-actions.ts b/src/renderer/src/store/slices/browser/browser-profile-import-actions.ts index ec03690bf8e..9b79ec9adbc 100644 --- a/src/renderer/src/store/slices/browser/browser-profile-import-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-profile-import-actions.ts @@ -133,13 +133,13 @@ export function createBrowserProfileImportActions( set((s) => getBrowserSettingsHostId(s) === hostId ? { detectedBrowsers: browsers, detectedBrowsersLoaded: true, detectedBrowsersHost } - : {} + : s ) } catch { set((s) => getBrowserSettingsHostId(s) === hostId ? { detectedBrowsers: [], detectedBrowsersLoaded: true, detectedBrowsersHost: null } - : {} + : s ) } return @@ -161,11 +161,11 @@ export function createBrowserProfileImportActions( detectedBrowsersLoaded: true, detectedBrowsersHost: null } - : {} + : s ) } catch { /* best-effort — empty list is acceptable fallback */ - set((s) => (getBrowserSettingsHostId(s) === hostId ? { detectedBrowsersLoaded: true } : {})) + set((s) => (getBrowserSettingsHostId(s) === hostId ? { detectedBrowsersLoaded: true } : s)) } } } diff --git a/src/renderer/src/store/slices/commit-message-generation.ts b/src/renderer/src/store/slices/commit-message-generation.ts index e361958932c..4e7ef3e722d 100644 --- a/src/renderer/src/store/slices/commit-message-generation.ts +++ b/src/renderer/src/store/slices/commit-message-generation.ts @@ -160,7 +160,7 @@ export const createCommitMessageGenerationSlice: StateCreator< set((state) => { const nextRecord = updater(state.commitMessageGenerationRecords[key] ?? null) if (!nextRecord) { - return {} + return state } return { commitMessageGenerationRecords: { @@ -184,6 +184,6 @@ export const createCommitMessageGenerationSlice: StateCreator< changed = true } } - return changed ? { commitMessageGenerationRecords: nextRecords } : {} + return changed ? { commitMessageGenerationRecords: nextRecords } : state }) }) diff --git a/src/renderer/src/store/slices/diff-comment-persistence.ts b/src/renderer/src/store/slices/diff-comment-persistence.ts index 92e1eb9ea9f..4d28d7b7f1d 100644 --- a/src/renderer/src/store/slices/diff-comment-persistence.ts +++ b/src/renderer/src/store/slices/diff-comment-persistence.ts @@ -239,13 +239,13 @@ export function mutateDiffComments( if (scope?.type === 'folder') { const target = findFolderWorkspaceOwner(s, scope.folderWorkspaceId) if (!target) { - return {} + return s } folderExecutionHostId = getExecutionHostIdForFolderWorkspace(s, scope.folderWorkspaceId) previous = target.diffComments const computed = mutate(previous ?? []) if (computed === null) { - return {} + return s } next = computed return { @@ -256,16 +256,16 @@ export function mutateDiffComments( } const repoList = s.worktreesByRepo[repoId] if (!repoList) { - return {} + return s } const target = repoList.find((w) => w.id === worktreeId) if (!target) { - return {} + return s } previous = target.diffComments const computed = mutate(previous ?? []) if (computed === null) { - return {} + return s } next = computed const nextList: Worktree[] = repoList.map((w) => @@ -293,7 +293,7 @@ function rollback( if (scope?.type === 'folder') { const target = findFolderWorkspaceOwner(s, scope.folderWorkspaceId, folderExecutionHostId) if (!target || target.diffComments !== expectedCurrent) { - return {} + return s } return { folderWorkspaces: s.folderWorkspaces.map((workspace) => @@ -303,16 +303,16 @@ function rollback( } const repoList = s.worktreesByRepo[repoId] if (!repoList) { - return {} + return s } const target = repoList.find((w) => w.id === worktreeId) // Why: worktree gone since the mutation; bail before remapping so we don't allocate a new array identity and fire spurious notifications. if (!target) { - return {} + return s } // Why: only roll back if no later mutation replaced the array, else our stale `previous` would erase newer state. if (target.diffComments !== expectedCurrent) { - return {} + return s } const nextList: Worktree[] = repoList.map((w) => w.id === worktreeId ? { ...w, diffComments: previous } : w diff --git a/src/renderer/src/store/slices/empty-update-notifications.test.ts b/src/renderer/src/store/slices/empty-update-notifications.test.ts new file mode 100644 index 00000000000..692fa50b7d4 --- /dev/null +++ b/src/renderer/src/store/slices/empty-update-notifications.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest' +import { createTestStore } from './store-test-helpers' +import { createTabsSliceMockApi } from './tabs-slice-test-harness' +import { browserImportStateForHostUpdate } from './browser/browser-host-state' +import { mutateDiffComments } from './diff-comment-persistence' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +createTabsSliceMockApi() + +describe('empty store updates', () => { + it('does not notify for missing tab actions', () => { + const store = createTestStore() + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + before.setTabLabel('missing', 'label') + before.setTabCustomLabel('missing', 'label') + before.setUnifiedTabColor('missing', null) + before.setTabViewMode('missing', 'chat') + before.toggleTabViewMode('missing') + before.pinTab('missing') + before.unpinTab('missing') + before.reorderUnifiedTabs('missing', []) + before.moveUnifiedTabToGroup('missing', 'missing') + + expect(store.getState()).toBe(before) + expect(listener).not.toHaveBeenCalled() + }) + + it('does not notify for unchanged labels but publishes changed labels', () => { + const store = createTestStore() + const tab = store + .getState() + .createUnifiedTab('folder-workspace', 'terminal', { label: 'label' }) + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + before.setTabLabel(tab.id, 'label') + expect(store.getState()).toBe(before) + expect(listener).not.toHaveBeenCalled() + + before.setTabLabel(tab.id, 'new label') + expect(listener).toHaveBeenCalledTimes(1) + expect(store.getState().getTab(tab.id)?.label).toBe('new label') + }) + + it('does not notify for rejected generation updates or empty pruning', () => { + const store = createTestStore() + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + before.updateCommitMessageGenerationRecord('missing', () => null) + before.updatePullRequestGenerationRecord('missing', () => null) + before.pruneCommitMessageGenerationRecords(new Set()) + before.prunePullRequestGenerationRecords(new Set()) + + expect(store.getState()).toBe(before) + expect(listener).not.toHaveBeenCalled() + }) + + it('does not notify for absent Jira issues, browser pages or diff comments', () => { + const store = createTestStore() + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + before.patchJiraIssue('MISSING-1', {}) + before.patchLinearIssue('missing', {}) + before.switchBrowserTabProfile('missing', null, 'persist:missing') + before.recordClientHostedBrowserCloseIntents([]) + before.clearClientHostedBrowserCloseIntents('missing', []) + mutateDiffComments(store.setState, 'missing', () => null) + store.setState((state) => browserImportStateForHostUpdate(state, 'runtime:other', null)) + + expect(store.getState()).toBe(before) + expect(listener).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/store/slices/github-pr-branch-linked-pr-divergence.test.ts b/src/renderer/src/store/slices/github-pr-branch-linked-pr-divergence.test.ts index 418b6648fa0..8b42bf3e56f 100644 --- a/src/renderer/src/store/slices/github-pr-branch-linked-pr-divergence.test.ts +++ b/src/renderer/src/store/slices/github-pr-branch-linked-pr-divergence.test.ts @@ -86,6 +86,8 @@ describe('createGitHubSlice.fetchPRForBranch', () => { hostedReviewCache: {}, prCache: {} } as unknown as Partial) + const subscriber = vi.fn() + const unsubscribe = store.subscribe(subscriber) resolveRefresh({ kind: 'found', pr: makePR({ number: 12, title: 'Stale exact linked PR' }), @@ -93,6 +95,8 @@ describe('createGitHubSlice.fetchPRForBranch', () => { }) await expect(request).resolves.toBeNull() + unsubscribe() + expect(subscriber).not.toHaveBeenCalled() expect(store.getState().prCache[`${repoId}::${branch}`]).toBeUndefined() expect(store.getState().hostedReviewCache[hostedReviewCacheKey]).toBeUndefined() }) diff --git a/src/renderer/src/store/slices/github-work-item-cache-identity.test.ts b/src/renderer/src/store/slices/github-work-item-cache-identity.test.ts index ef22da072d1..ba741de5cfb 100644 --- a/src/renderer/src/store/slices/github-work-item-cache-identity.test.ts +++ b/src/renderer/src/store/slices/github-work-item-cache-identity.test.ts @@ -15,6 +15,15 @@ describe('createGitHubSlice.patchWorkItem', () => { resetRemoteRuntimeMocks() }) + it('does not notify when a patch has no matching cached work item', () => { + const store = createTestStore() + const subscriber = vi.fn() + const unsubscribe = store.subscribe(subscriber) + store.getState().patchWorkItem('pr:missing', { title: 'Missing' }, 'repo-1') + unsubscribe() + expect(subscriber).not.toHaveBeenCalled() + }) + it('can scope patches to one repo when different repos have the same work-item id', () => { const store = createTestStore() const repoOneItem = { diff --git a/src/renderer/src/store/slices/hosted-review-cache-race.test.ts b/src/renderer/src/store/slices/hosted-review-cache-race.test.ts index 59e253e1823..16d765fe0d5 100644 --- a/src/renderer/src/store/slices/hosted-review-cache-race.test.ts +++ b/src/renderer/src/store/slices/hosted-review-cache-race.test.ts @@ -112,10 +112,14 @@ describe('hosted review cache race protection', () => { } } }) + const subscriber = vi.fn() + const unsubscribe = store.subscribe(subscriber) vi.setSystemTime(300) resolveFetch(olderReview) await expect(request).resolves.toEqual(olderReview) + unsubscribe() + expect(subscriber).not.toHaveBeenCalled() expect(store.getState().hostedReviewCache[cacheKey]).toEqual({ data: newerReview, fetchedAt: 200, diff --git a/src/renderer/src/store/slices/hosted-review.ts b/src/renderer/src/store/slices/hosted-review.ts index 53cdb00db55..cad350d986b 100644 --- a/src/renderer/src/store/slices/hosted-review.ts +++ b/src/renderer/src/store/slices/hosted-review.ts @@ -234,7 +234,7 @@ export const createHostedReviewSlice: StateCreator { const nextRecord = updater(state.pullRequestGenerationRecords[key] ?? null) if (!nextRecord) { - return {} + return state } return { pullRequestGenerationRecords: { @@ -312,6 +312,6 @@ export const createPullRequestGenerationSlice: StateCreator< changed = true } } - return changed ? { pullRequestGenerationRecords: nextRecords } : {} + return changed ? { pullRequestGenerationRecords: nextRecords } : state }) }) diff --git a/src/renderer/src/store/slices/sparse-presets.ts b/src/renderer/src/store/slices/sparse-presets.ts index b9e763bf72e..6a4a2d68245 100644 --- a/src/renderer/src/store/slices/sparse-presets.ts +++ b/src/renderer/src/store/slices/sparse-presets.ts @@ -145,7 +145,7 @@ export const createSparsePresetsSlice: StateCreator { const existing = s.sparsePresetsByRepo[args.repoId] if (existing === undefined) { - return {} + return s } const without = existing.filter((preset) => preset.id !== saved.id) return { diff --git a/src/renderer/src/store/slices/tabs/tabs-create-actions.ts b/src/renderer/src/store/slices/tabs/tabs-create-actions.ts index b4f1d611b2f..f8736ed250b 100644 --- a/src/renderer/src/store/slices/tabs/tabs-create-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-create-actions.ts @@ -120,7 +120,7 @@ export function createTabsCreateActions( target.sourceGroupId ) if (!sourceGroup) { - return {} + return state } const existingTabs = state.unifiedTabsByWorktree[worktreeId] ?? [] const currentGroups = state.groupsByWorktree[worktreeId] ?? [] diff --git a/src/renderer/src/store/slices/tabs/tabs-drop-actions.ts b/src/renderer/src/store/slices/tabs/tabs-drop-actions.ts index 2cadf76365e..612bee0b538 100644 --- a/src/renderer/src/store/slices/tabs/tabs-drop-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-drop-actions.ts @@ -25,19 +25,19 @@ export function createTabsDropActions( const foundTab = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) const foundTarget = findGroupAndWorktree(state.groupsByWorktree, target.groupId) if (!foundTab || !foundTarget || foundTab.worktreeId !== foundTarget.worktreeId) { - return {} + return state } const { tab, worktreeId } = foundTab const sourceGroup = findGroupForTab(state.groupsByWorktree, worktreeId, tab.groupId) const targetGroup = foundTarget.group if (!sourceGroup) { - return {} + return state } const isSplitDrop = Boolean(target.splitDirection) if (!isSplitDrop && tab.groupId === target.groupId) { - return {} + return state } const layout = state.layoutByWorktree[worktreeId] if ( @@ -51,7 +51,7 @@ export function createTabsDropActions( }) ) { // Why: dropping a group's last tab on its own/sibling matching edge only makes a transient column that immediately collapses. - return {} + return state } moved = true diff --git a/src/renderer/src/store/slices/tabs/tabs-focus-actions.ts b/src/renderer/src/store/slices/tabs/tabs-focus-actions.ts index 39a0dd38858..6e4134c6a1b 100644 --- a/src/renderer/src/store/slices/tabs/tabs-focus-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-focus-actions.ts @@ -53,7 +53,7 @@ export function createTabsFocusActions( found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) } if (!found) { - return {} + return state } const { tab, worktreeId } = found // Why: activating a terminal tab dismisses its tab-level bell — the user has moved their eyes here. diff --git a/src/renderer/src/store/slices/tabs/tabs-label-actions.ts b/src/renderer/src/store/slices/tabs/tabs-label-actions.ts index 42b8e3d7163..3ee06aa105c 100644 --- a/src/renderer/src/store/slices/tabs/tabs-label-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-label-actions.ts @@ -50,7 +50,7 @@ export function createTabsLabelActions( } } } - return {} + return state }) if (reordered && opts?.recordInteraction !== false) { get().recordFeatureInteraction?.('terminal-tabs') @@ -58,17 +58,24 @@ export function createTabsLabelActions( }, setTabLabel: (tabId, label) => { - set((state) => patchTab(state.unifiedTabsByWorktree, tabId, { label }) ?? {}) + set((state) => patchTab(state.unifiedTabsByWorktree, tabId, { label }) ?? state) }, setTabViewMode: (tabId, mode) => { - set((state) => ({ - ...patchTab(state.unifiedTabsByWorktree, tabId, { viewMode: mode }), - // Why the row too: viewMode is declared on both types and host-sync - // already writes it to the row. Only these local toggles skipped it, so - // readers had to OR the two indices to find out who owns the surface. - ...patchTerminalTabRow(state.tabsByWorktree, tabId, { viewMode: mode }) - })) + set((state) => { + const tabPatch = patchTab(state.unifiedTabsByWorktree, tabId, { viewMode: mode }) + const rowPatch = patchTerminalTabRow(state.tabsByWorktree, tabId, { viewMode: mode }) + if (!tabPatch && !rowPatch.tabsByWorktree) { + return state + } + return { + ...tabPatch, + // Why the row too: viewMode is declared on both types and host-sync + // already writes it to the row. Only these local toggles skipped it, so + // readers had to OR the two indices to find out who owns the surface. + ...rowPatch + } + }) mirrorTabViewModeToHost(get(), tabId, mode) }, @@ -81,7 +88,7 @@ export function createTabsLabelActions( set((state) => { const found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) if (!found) { - return {} + return state } // Why: viewMode defaults to 'terminal' for legacy/missing, so the first toggle flips to 'chat'. const fromMode: 'terminal' | 'chat' = found.tab.viewMode === 'chat' ? 'chat' : 'terminal' @@ -111,7 +118,7 @@ export function createTabsLabelActions( setTabCustomLabel: (tabId, label, opts) => { const exists = get().getTab(tabId) !== null - set((state) => patchTab(state.unifiedTabsByWorktree, tabId, { customLabel: label }) ?? {}) + set((state) => patchTab(state.unifiedTabsByWorktree, tabId, { customLabel: label }) ?? state) if (exists && opts?.recordInteraction !== false) { get().recordFeatureInteraction?.('terminal-tabs') } @@ -119,7 +126,7 @@ export function createTabsLabelActions( setUnifiedTabColor: (tabId, color) => { const exists = get().getTab(tabId) !== null - set((state) => patchTab(state.unifiedTabsByWorktree, tabId, { color }) ?? {}) + set((state) => patchTab(state.unifiedTabsByWorktree, tabId, { color }) ?? state) if (exists) { get().recordFeatureInteraction?.('terminal-tabs') } @@ -130,7 +137,7 @@ export function createTabsLabelActions( set((state) => { const found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) if (!found) { - return {} + return state } const { tab, worktreeId } = found const tabs = (state.unifiedTabsByWorktree[worktreeId] ?? []).map((candidate) => @@ -168,7 +175,7 @@ export function createTabsLabelActions( set((state) => { const found = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) if (!found) { - return {} + return state } const { tab, worktreeId } = found const tabs = (state.unifiedTabsByWorktree[worktreeId] ?? []).map((candidate) => diff --git a/src/renderer/src/store/slices/tabs/tabs-move-actions.ts b/src/renderer/src/store/slices/tabs/tabs-move-actions.ts index 4c651f613ac..8e8a51f5c0c 100644 --- a/src/renderer/src/store/slices/tabs/tabs-move-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-move-actions.ts @@ -22,16 +22,16 @@ export function createTabsMoveActions( const foundTab = findTabAndWorktree(state.unifiedTabsByWorktree, tabId) const foundTarget = findGroupAndWorktree(state.groupsByWorktree, targetGroupId) if (!foundTab || !foundTarget || foundTab.worktreeId !== foundTarget.worktreeId) { - return {} + return state } const { tab, worktreeId } = foundTab if (tab.groupId === targetGroupId) { - return {} + return state } const sourceGroup = findGroupForTab(state.groupsByWorktree, worktreeId, tab.groupId) const targetGroup = foundTarget.group if (!sourceGroup) { - return {} + return state } moved = true diff --git a/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts b/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts index 28530219cce..435eadec4ec 100644 --- a/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts +++ b/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts @@ -23,14 +23,14 @@ export function createUpdatePendingWorktreeCreation( set((s) => { const entry = s.pendingWorktreeCreations[creationId] if (!entry) { - return {} + return s } // Why: the main process re-emits the same phase; skip no-op writes so the strip and panel don't re-render. const hasChange = (Object.keys(patch) as (keyof typeof patch)[]).some( (key) => patch[key] !== entry[key] ) if (!hasChange) { - return {} + return s } return { pendingWorktreeCreations: { @@ -51,7 +51,7 @@ export function createRemovePendingWorktreeCreation( set((s) => { const entry = s.pendingWorktreeCreations[creationId] if (!entry) { - return {} + return s } removedEntry = entry const { [creationId]: _removed, ...rest } = s.pendingWorktreeCreations @@ -90,7 +90,7 @@ export function createSetActivePendingWorktreeCreation( return (creationId) => { set((s) => { if (creationId !== null && !s.pendingWorktreeCreations[creationId]) { - return {} + return s } return { activePendingCreationId: creationId } }) diff --git a/src/renderer/src/store/slices/worktrees/metadata/hosted-review-link-mutation.ts b/src/renderer/src/store/slices/worktrees/metadata/hosted-review-link-mutation.ts index fdf17d38f8f..35464523ba5 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/hosted-review-link-mutation.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/hosted-review-link-mutation.ts @@ -261,7 +261,7 @@ export function applyHostedReviewLinkClear( nextWorktrees === s.worktreesByRepo && nextDetectedWorktrees === s.detectedWorktreesByRepo ) { - return {} + return s } return { ...(nextWorktrees !== s.worktreesByRepo diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts index 5e5c38a71e2..414a2ef206a 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts @@ -149,7 +149,7 @@ export function createUpdateWorktreeMeta( shouldApplyUpdate && !shouldApplyUpdate(findKnownWorktreeById(s, worktreeId, executionHostId)) ) { - return {} + return s } didApply = true const nextWorktrees = applyWorktreeUpdates( @@ -204,7 +204,7 @@ export function createUpdateWorktreeMeta( !cacheKey && !prCacheKey ) { - return {} + return s } const nextHostedReviewCache = diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts index 78254535106..8ae7be3ea8b 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts @@ -73,7 +73,7 @@ export function createUpdateWorktreesMeta( } return nextWorktrees === s.worktreesByRepo && nextDetectedWorktrees === s.detectedWorktreesByRepo - ? {} + ? s : { ...(nextWorktrees !== s.worktreesByRepo ? { worktreesByRepo: nextWorktrees, sortEpoch: s.sortEpoch + 1 } diff --git a/src/renderer/src/store/slices/worktrees/session/migrate-worktree-identity.ts b/src/renderer/src/store/slices/worktrees/session/migrate-worktree-identity.ts index 75d6059f309..d14392e72d9 100644 --- a/src/renderer/src/store/slices/worktrees/session/migrate-worktree-identity.ts +++ b/src/renderer/src/store/slices/worktrees/session/migrate-worktree-identity.ts @@ -14,7 +14,10 @@ export function createMigrateWorktreeIdentity( } // Why: invalidate pre-rename toast actions before publishing the new path, carrying the dismissal forward. migrateHugeRepoWarningDismissal(oldWorktreeId, newWorktreeId) - set((s) => buildWorktreeRenameState(s, oldWorktreeId, newWorktreeId)) + set((s) => { + const patch = buildWorktreeRenameState(s, oldWorktreeId, newWorktreeId) + return Object.keys(patch).length > 0 ? patch : s + }) migrateHostedReviewLinkMutationGeneration(oldWorktreeId, newWorktreeId) } } diff --git a/src/renderer/src/store/slices/worktrees/session/set-active-worktree.ts b/src/renderer/src/store/slices/worktrees/session/set-active-worktree.ts index 31c7e8bbb37..84c85c45a3c 100644 --- a/src/renderer/src/store/slices/worktrees/session/set-active-worktree.ts +++ b/src/renderer/src/store/slices/worktrees/session/set-active-worktree.ts @@ -217,15 +217,15 @@ export function createSetActiveWorktree( pendingActivationTerminalPrepCancels.delete(worktreeId) set((s) => { if (s.activeWorktreeId !== worktreeId) { - return {} + return s } const tabs = s.tabsByWorktree[worktreeId] ?? [] if (tabs.length === 0) { - return {} + return s } const allDead = tabs.every((tab) => !tabHasLivePty(s.ptyIdsByTabId, tab.id)) if (!allDead && !shouldTagTerminalTabs) { - return {} + return s } return { tabsByWorktree: { diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-no-op-notifications.test.ts b/src/renderer/src/store/slices/worktrees/session/worktree-no-op-notifications.test.ts new file mode 100644 index 00000000000..7fdaea7999f --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/session/worktree-no-op-notifications.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from 'vitest' +import { createTestStore } from '../../worktrees-slice-test-harness' +import { makeWorktree } from '../../worktrees-slice-test-fixtures' + +vi.mock('sonner', () => ({ + toast: { warning: vi.fn(), info: vi.fn(), success: vi.fn(), error: vi.fn(), dismiss: vi.fn() } +})) +vi.mock('@/components/worktree-base-fallback-notice', () => ({ + requestWorktreeBaseFallbackNotice: vi.fn() +})) + +describe('worktree no-op notifications', () => { + it('keeps missing creation, recovery, activity, deletion and visit updates silent', () => { + const store = createTestStore() + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + before.updatePendingWorktreeCreation('missing', { phase: 'fetching' }) + before.removePendingWorktreeCreation('missing') + before.setActivePendingWorktreeCreation('missing') + before.remountTerminalTabForRecovery('missing') + before.settleTerminalTabRecovery('missing', 1, 'success') + before.markWorktreeUnread('missing') + before.bumpWorktreeActivity('missing') + before.clearWorktreeDeleteState('missing') + before.seedActiveWorktreeLastVisitedIfMissing() + before.pruneLastVisitedTimestamps() + before.migrateWorktreeIdentity('missing-old', 'missing-new') + + expect(store.getState()).toBe(before) + expect(listener).not.toHaveBeenCalled() + }) + + it.each(['local', 'ssh:test'] as const)( + 'keeps repeated %s deletion and visit updates silent', + (hostId) => { + const store = createTestStore() + const worktree = makeWorktree({ id: 'repo1::/path/wt', repoId: 'repo1', hostId }) + store.setState({ worktreesByRepo: { repo1: [worktree] } }) + const target = { id: worktree.id, hostId } + store.getState().markWorktreesQueuedForDeletion([target]) + store.getState().markWorktreeVisited(worktree.id, 100, hostId) + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + before.markWorktreesQueuedForDeletion([target]) + before.markWorktreeVisited(worktree.id, 100, hostId) + before.markWorktreeVisited(worktree.id, 99, hostId) + expect(store.getState()).toBe(before) + expect(listener).not.toHaveBeenCalled() + + before.markWorktreesDeleting([target]) + expect(listener).toHaveBeenCalledTimes(1) + const deleting = store.getState() + deleting.markWorktreesDeleting([target]) + expect(store.getState()).toBe(deleting) + expect(listener).toHaveBeenCalledTimes(1) + deleting.clearWorktreeDeleteState(worktree.id, hostId) + expect(listener).toHaveBeenCalledTimes(2) + } + ) +}) diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts b/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts index 78e0e397bca..66e6ddf5b51 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts @@ -55,7 +55,7 @@ export function createRemountTerminalTabForRecovery( const { admitted: _admitted, ...decline } = admission result = { remounted: false, ...decline } } - return {} + return s } const { worktreeId, index, tab } = location const nextTabs = s.tabsByWorktree[worktreeId].slice() @@ -110,12 +110,12 @@ export function createSettleTerminalTabRecovery( set((s) => { const location = locateTerminalTab(s.tabsByWorktree, tabId) if (!location) { - return {} + return s } const { worktreeId, index, tab } = location const recovery = settledTerminalRecoveryLedger(tab, generation, outcome) if (!recovery) { - return {} + return s } const nextTabs = s.tabsByWorktree[worktreeId].slice() nextTabs[index] = { ...tab, recovery } diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts b/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts index 72d95b6a0a4..bea375b8292 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-unread-activity.ts @@ -59,7 +59,7 @@ export function createMarkWorktreeUnread( set((s) => { const worktree = findKnownWorktreeById(s, worktreeId) if (!worktree || worktree.isUnread) { - return {} + return s } shouldPersist = true const nextWorktrees = applyWorktreeUpdates(s.worktreesByRepo, worktreeId, { @@ -266,7 +266,7 @@ export function createBumpWorktreeActivity( set((s) => { const worktree = findKnownWorktreeById(s, worktreeId) if (!worktree) { - return {} + return s } shouldPersist = true // Why: skip sortEpoch bump for the active worktree — its PTY events are click side-effects (reorder-on-click bug, PR #209). diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts b/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts index 5cfc5eef3d2..7a9d94c89fd 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts @@ -29,7 +29,7 @@ export function createMarkWorktreeVisited( hostId: ownerHostId }) ?? 0 if (!(now > prev)) { - return {} + return s } return { lastVisitedAtByWorktreeId: { @@ -124,7 +124,7 @@ export function createPruneLastVisitedTimestamps( patch.activeWorkspaceExecutionHostId = null } } - return Object.keys(patch).length > 0 ? patch : {} + return Object.keys(patch).length > 0 ? patch : s }) } } @@ -137,12 +137,12 @@ export function createSeedActiveWorktreeLastVisitedIfMissing( set((s) => { const id = s.activeWorktreeId if (!id) { - return {} + return s } const hostId = s.activeWorkspaceExecutionHostId ?? s.getKnownWorktreeById(id)?.hostId const key = getWorktreeVisitKey(id, hostId) if (getWorktreeVisitTimestamp(s.lastVisitedAtByWorktreeId, { id, hostId }) != null) { - return {} + return s } return { lastVisitedAtByWorktreeId: { diff --git a/src/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.ts b/src/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.ts index 30975b958e6..7e89bb2e34b 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/worktree-delete-state.ts @@ -78,7 +78,7 @@ export function createMarkWorktreesDeleting( } changed = true } - return changed ? { deleteStateByWorktreeId: nextDeleteState } : {} + return changed ? { deleteStateByWorktreeId: nextDeleteState } : s }) } } @@ -113,7 +113,7 @@ export function createMarkWorktreesQueuedForDeletion( } changed = true } - return changed ? { deleteStateByWorktreeId: nextDeleteState } : {} + return changed ? { deleteStateByWorktreeId: nextDeleteState } : s }) } } @@ -128,7 +128,7 @@ export function createClearWorktreeDeleteState( : worktreeId set((s) => { if (!s.deleteStateByWorktreeId[key]) { - return {} + return s } const next = { ...s.deleteStateByWorktreeId } delete next[key] diff --git a/src/renderer/src/store/terminals/terminal-disowned-pty-sources.ts b/src/renderer/src/store/terminals/terminal-disowned-pty-sources.ts index d67a382a421..fc3abb5edd6 100644 --- a/src/renderer/src/store/terminals/terminal-disowned-pty-sources.ts +++ b/src/renderer/src/store/terminals/terminal-disowned-pty-sources.ts @@ -16,7 +16,7 @@ export function createTerminalDisownedPtySourceActions( markPtySourceDisowned: (ptyId) => { set((state) => state.disownedPtyIds[ptyId] - ? {} + ? state : { disownedPtyIds: { ...state.disownedPtyIds, [ptyId]: true } } ) } diff --git a/src/renderer/src/store/terminals/terminal-ephemeral-state.ts b/src/renderer/src/store/terminals/terminal-ephemeral-state.ts index 89a67bd72ae..80de6b6899a 100644 --- a/src/renderer/src/store/terminals/terminal-ephemeral-state.ts +++ b/src/renderer/src/store/terminals/terminal-ephemeral-state.ts @@ -30,7 +30,7 @@ export function createTerminalEphemeralActions( markDefaultTerminalTabsApplied: (worktreeId) => set((s) => { if (s.defaultTerminalTabsAppliedByWorktreeId[worktreeId]) { - return {} + return s } return { defaultTerminalTabsAppliedByWorktreeId: { @@ -70,7 +70,7 @@ export function createTerminalEphemeralActions( set((s) => { const current = s.nativeChatLaunchPromptByTabId[tabId] if (!current || current.failed) { - return {} + return s } return { nativeChatLaunchPromptByTabId: { @@ -83,7 +83,7 @@ export function createTerminalEphemeralActions( clearNativeChatLaunchPrompt: (tabId) => { set((s) => { if (!s.nativeChatLaunchPromptByTabId[tabId]) { - return {} + return s } const next = { ...s.nativeChatLaunchPromptByTabId } delete next[tabId] @@ -102,7 +102,7 @@ export function createTerminalEphemeralActions( set((s) => { const current = s.nativeChatLaunchDraftByTabId[tabId] if (!current || current.adopted) { - return {} + return s } return { nativeChatLaunchDraftByTabId: { @@ -121,7 +121,7 @@ export function createTerminalEphemeralActions( current.createdAt !== resolution.createdAt || current.text !== resolution.text ) { - return {} + return s } return { nativeChatLaunchDraftByTabId: { @@ -134,7 +134,7 @@ export function createTerminalEphemeralActions( clearNativeChatLaunchDraft: (tabId) => { set((s) => { if (!s.nativeChatLaunchDraftByTabId[tabId]) { - return {} + return s } const next = { ...s.nativeChatLaunchDraftByTabId } delete next[tabId] @@ -168,7 +168,7 @@ export function createTerminalEphemeralActions( next ??= { ...s.lastTerminalInputAtByPaneKey } next[key] = at } - return next ? { lastTerminalInputAtByPaneKey: next } : {} + return next ? { lastTerminalInputAtByPaneKey: next } : s }) } }) @@ -227,7 +227,7 @@ export function createTerminalEphemeralActions( removeDeferredSshSessionId: (tabId) => set((s) => { if (!s.deferredSshSessionIdsByTabId[tabId]) { - return {} + return s } const next = { ...s.deferredSshSessionIdsByTabId } delete next[tabId] diff --git a/src/renderer/src/store/terminals/terminal-layout-state.ts b/src/renderer/src/store/terminals/terminal-layout-state.ts index 9b5cf8323c2..42ad3dbc659 100644 --- a/src/renderer/src/store/terminals/terminal-layout-state.ts +++ b/src/renderer/src/store/terminals/terminal-layout-state.ts @@ -30,7 +30,7 @@ export function createTerminalLayoutActions( set((s) => { const layout = s.terminalLayoutsByTabId[tabId] if (!layout || layout.ptyIdsByLeafId?.[leafId] === ptyId) { - return {} + return s } return { terminalLayoutsByTabId: { diff --git a/src/renderer/src/store/terminals/terminal-no-op-subscriber.test.ts b/src/renderer/src/store/terminals/terminal-no-op-subscriber.test.ts new file mode 100644 index 00000000000..4d68e867770 --- /dev/null +++ b/src/renderer/src/store/terminals/terminal-no-op-subscriber.test.ts @@ -0,0 +1,146 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + flushTerminalInputActivity, + resetTerminalInputActivityCoalescingForTests +} from '@/lib/terminal-input-activity-coalescing' +import { createTestStore, makeLayout } from '../slices/store-test-helpers' + +afterEach(resetTerminalInputActivityCoalescingForTests) + +describe('terminal no-op subscriber budget', () => { + it('does not publish missing-entry cleanup and restart actions', () => { + const store = createTestStore() + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + for (let i = 0; i < 25; i += 1) { + const s = store.getState() + s.replaceTerminalLayoutPanePtyId('missing', 'leaf', 'pty') + expect(s.consumeSuppressedPtyExit('missing')).toBe(false) + expect(s.consumePendingCodexPaneRestart('missing')).toBe(false) + s.clearCodexRestartNotice('missing') + s.dismissCodexRestartNotices(['missing']) + s.reopenCodexRestartPrompt('missing') + s.markNativeChatLaunchPromptFailed('missing') + s.clearNativeChatLaunchPrompt('missing') + s.markNativeChatLaunchDraftAdopted('missing') + s.resolveNativeChatLaunchDraft('missing', { text: 'draft', createdAt: 1 }) + s.clearNativeChatLaunchDraft('missing') + s.removeDeferredSshSessionId('missing') + } + + expect(listener).not.toHaveBeenCalled() + expect(store.getState()).toBe(before) + }) + + it('publishes real mutations once and keeps repeated actions silent', () => { + const store = createTestStore() + const draft = { tabId: 'tab', agent: 'codex', text: 'draft', createdAt: 1 } as const + store.getState().seedNativeChatLaunchPrompt(draft) + store.getState().seedNativeChatLaunchDraft(draft) + store.getState().setTabLayout('tab', makeLayout()) + const listener = vi.fn() + store.subscribe(listener) + + const actions = [ + () => store.getState().markDefaultTerminalTabsApplied('folder-workspace'), + () => store.getState().markUnverifiedPtyLoss('tab'), + () => store.getState().markPtySourceDisowned('pty'), + () => store.getState().markNativeChatLaunchPromptFailed('tab'), + () => store.getState().markNativeChatLaunchDraftAdopted('tab'), + () => store.getState().resolveNativeChatLaunchDraft('tab', draft), + () => store.getState().replaceTerminalLayoutPanePtyId('tab', 'leaf', 'pty'), + () => store.getState().clearNativeChatLaunchPrompt('tab'), + () => store.getState().clearNativeChatLaunchDraft('tab') + ] + for (const action of actions) { + listener.mockClear() + action() + expect(listener).toHaveBeenCalledTimes(1) + const before = store.getState() + action() + expect(listener).toHaveBeenCalledTimes(1) + expect(store.getState()).toBe(before) + } + }) + + it('retains draft generations when stale resolutions arrive without notifying', () => { + const store = createTestStore() + const draft = { tabId: 'tab', agent: 'codex', text: 'new draft', createdAt: 2 } as const + store.getState().seedNativeChatLaunchDraft(draft) + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + store.getState().resolveNativeChatLaunchDraft('tab', { ...draft, createdAt: 1 }) + store.getState().resolveNativeChatLaunchDraft('tab', { ...draft, text: 'old draft' }) + + expect(listener).not.toHaveBeenCalled() + expect(store.getState()).toBe(before) + expect(store.getState().nativeChatLaunchDraftByTabId.tab).toBe(draft) + }) + + it('consumes real restart entries and leaves repeated consumes silent', () => { + const store = createTestStore() + store.getState().suppressPtyExit('pty') + store.getState().queueCodexPaneRestarts(['pty']) + const listener = vi.fn() + store.subscribe(listener) + + expect(store.getState().consumeSuppressedPtyExit('pty')).toBe(true) + expect(store.getState().consumePendingCodexPaneRestart('pty')).toBe(true) + expect(listener).toHaveBeenCalledTimes(2) + const before = store.getState() + expect(store.getState().consumeSuppressedPtyExit('pty')).toBe(false) + expect(store.getState().consumePendingCodexPaneRestart('pty')).toBe(false) + expect(listener).toHaveBeenCalledTimes(2) + expect(store.getState()).toBe(before) + }) + + it('drops a trailing input flush after pane teardown without publishing', () => { + const store = createTestStore() + store.getState().recordTerminalInput('tab:leaf', 1000) + store.getState().recordTerminalInput('tab:leaf', 1001) + store.setState({ lastTerminalInputAtByPaneKey: {} }) + const before = store.getState() + const listener = vi.fn() + store.subscribe(listener) + + flushTerminalInputActivity() + + expect(listener).not.toHaveBeenCalled() + expect(store.getState()).toBe(before) + expect(store.getState().lastTerminalInputAtByPaneKey['tab:leaf']).toBeUndefined() + }) + + it('dismisses, reopens and clears restart notices without replaying no-op notifications', () => { + const store = createTestStore() + store + .getState() + .markCodexRestartNotices([ + { ptyId: 'pty', previousAccountLabel: 'old', nextAccountLabel: 'new' } + ]) + const listener = vi.fn() + store.subscribe(listener) + const actions = [ + () => store.getState().dismissCodexRestartNotices(['pty']), + () => store.getState().reopenCodexRestartPrompt('pty'), + () => store.getState().clearCodexRestartNotice('pty') + ] + for (const [index, action] of actions.entries()) { + if (index === 1) { + store.getState().queueCodexPaneRestarts(['pty']) + } + listener.mockClear() + action() + expect(listener).toHaveBeenCalledTimes(1) + const before = store.getState() + action() + expect(listener).toHaveBeenCalledTimes(1) + expect(store.getState()).toBe(before) + } + expect(store.getState().codexRestartNoticeByPtyId.pty).toBeUndefined() + expect(store.getState().pendingCodexPaneRestartIds.pty).toBeUndefined() + }) +}) diff --git a/src/renderer/src/store/terminals/terminal-restart-state.ts b/src/renderer/src/store/terminals/terminal-restart-state.ts index f1075bdb698..2b712b6ccdb 100644 --- a/src/renderer/src/store/terminals/terminal-restart-state.ts +++ b/src/renderer/src/store/terminals/terminal-restart-state.ts @@ -21,7 +21,7 @@ export function createTerminalRestartActions( let wasSuppressed = false set((s) => { if (!s.suppressedPtyExitIds[ptyId]) { - return {} + return s } wasSuppressed = true const next = { ...s.suppressedPtyExitIds } @@ -68,7 +68,7 @@ export function createTerminalRestartActions( let wasQueued = false set((s) => { if (!s.pendingCodexPaneRestartIds[ptyId]) { - return {} + return s } wasQueued = true const next = { ...s.pendingCodexPaneRestartIds } @@ -144,7 +144,7 @@ export function createTerminalRestartActions( clearCodexRestartNotice: (ptyId) => { set((s) => { if (!s.codexRestartNoticeByPtyId[ptyId]) { - return {} + return s } const next = { ...s.codexRestartNoticeByPtyId } const nextPendingCodexPaneRestartIds = { ...s.pendingCodexPaneRestartIds } @@ -175,7 +175,7 @@ export function createTerminalRestartActions( changed = true } if (!changed) { - return {} + return s } return { codexRestartNoticeByPtyId: next, @@ -187,7 +187,7 @@ export function createTerminalRestartActions( set((s) => { const notice = s.codexRestartNoticeByPtyId[ptyId] if (!notice?.restartRequested) { - return {} + return s } const { restartRequested: _restartRequested, ...kept } = notice const nextPendingCodexPaneRestartIds = { ...s.pendingCodexPaneRestartIds } diff --git a/src/renderer/src/store/terminals/terminal-startup-queues.ts b/src/renderer/src/store/terminals/terminal-startup-queues.ts index 0f684e3f8b7..af56050ef23 100644 --- a/src/renderer/src/store/terminals/terminal-startup-queues.ts +++ b/src/renderer/src/store/terminals/terminal-startup-queues.ts @@ -62,7 +62,7 @@ export function createTerminalStartupQueueActions( } set((s) => { if (s.pendingStartupByTabId[tabId] !== pending) { - return {} + return s } const next = { ...s.pendingStartupByTabId } delete next[tabId] diff --git a/src/renderer/src/store/terminals/terminal-unverified-pty-loss.ts b/src/renderer/src/store/terminals/terminal-unverified-pty-loss.ts index 9f381a02c4e..075d349a528 100644 --- a/src/renderer/src/store/terminals/terminal-unverified-pty-loss.ts +++ b/src/renderer/src/store/terminals/terminal-unverified-pty-loss.ts @@ -8,7 +8,7 @@ export function createTerminalUnverifiedPtyLossActions( markUnverifiedPtyLoss: (tabId) => { set((state) => state.unverifiedPtyLossTabIds[tabId] - ? {} + ? state : { unverifiedPtyLossTabIds: { ...state.unverifiedPtyLossTabIds, [tabId]: true } } ) }