fix(store): preserve state identity for no-op updater branches (#20703)

This commit is contained in:
Neil
2026-09-14 13:33:19 -07:00
committed by GitHub
parent 1ba9801574
commit dede24df46
43 changed files with 472 additions and 99 deletions
@@ -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)
@@ -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<AppState> = { ...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: {
@@ -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
@@ -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
@@ -45,7 +45,7 @@ export const createWorkItemMutationActions = (
nextCache[key] = { ...entry, data: updatedItems }
changed = true
}
return changed ? { workItemsCache: nextCache } : {}
return changed ? { workItemsCache: nextCache } : s
})
},
@@ -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
})
},
@@ -153,7 +153,7 @@ export function browserImportStateForHostUpdate(
hostId: ExecutionHostId,
browserSessionImportState: BrowserSlice['browserSessionImportState']
): Partial<BrowserSlice> {
return getBrowserSettingsHostId(state) === hostId ? { browserSessionImportState } : {}
return getBrowserSettingsHostId(state) === hostId ? { browserSessionImportState } : state
}
export function getFallbackTabTypeForWorktree(
@@ -257,7 +257,7 @@ export function createBrowserHydrationActions(
}
}
}
return {}
return s
})
}
}
@@ -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))
}
}
}
@@ -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
})
})
@@ -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
@@ -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()
})
})
@@ -86,6 +86,8 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
hostedReviewCache: {},
prCache: {}
} as unknown as Partial<AppState>)
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()
})
@@ -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 = {
@@ -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,
@@ -234,7 +234,7 @@ export const createHostedReviewSlice: StateCreator<AppState, [], [], HostedRevie
requestStartedEntry
)
) {
return {}
return state
}
const currentPRCache = state.prCache ?? {}
const prCache = clearHostedReviewConflictingPrCache({
@@ -34,7 +34,7 @@ export function createJiraIssuePatchAction(set: JiraSliceSet): Pick<JiraSlice, '
jiraSearchCache[key] = { ...entry, data: updatedIssues }
changed = true
}
return changed ? { jiraIssueCache, jiraSearchCache } : {}
return changed ? { jiraIssueCache, jiraSearchCache } : state
})
}
}
@@ -148,7 +148,7 @@ export function createLinearInvalidationActions(
? nextCustomViewIssueCache.cache
: s.linearCustomViewIssueCache
}
: {}
: s
})
}
}
@@ -288,7 +288,7 @@ export const createPullRequestGenerationSlice: StateCreator<
set((state) => {
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
})
})
@@ -145,7 +145,7 @@ export const createSparsePresetsSlice: StateCreator<AppState, [], [], SparsePres
set((s) => {
const existing = s.sparsePresetsByRepo[args.repoId]
if (existing === undefined) {
return {}
return s
}
const without = existing.filter((preset) => preset.id !== saved.id)
return {
@@ -120,7 +120,7 @@ export function createTabsCreateActions(
target.sourceGroupId
)
if (!sourceGroup) {
return {}
return state
}
const existingTabs = state.unifiedTabsByWorktree[worktreeId] ?? []
const currentGroups = state.groupsByWorktree[worktreeId] ?? []
@@ -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
@@ -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.
@@ -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) =>
@@ -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
@@ -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 }
})
@@ -261,7 +261,7 @@ export function applyHostedReviewLinkClear(
nextWorktrees === s.worktreesByRepo &&
nextDetectedWorktrees === s.detectedWorktreesByRepo
) {
return {}
return s
}
return {
...(nextWorktrees !== s.worktreesByRepo
@@ -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 =
@@ -73,7 +73,7 @@ export function createUpdateWorktreesMeta(
}
return nextWorktrees === s.worktreesByRepo &&
nextDetectedWorktrees === s.detectedWorktreesByRepo
? {}
? s
: {
...(nextWorktrees !== s.worktreesByRepo
? { worktreesByRepo: nextWorktrees, sortEpoch: s.sortEpoch + 1 }
@@ -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)
}
}
@@ -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: {
@@ -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)
}
)
})
@@ -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 }
@@ -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).
@@ -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: {
@@ -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]
@@ -16,7 +16,7 @@ export function createTerminalDisownedPtySourceActions(
markPtySourceDisowned: (ptyId) => {
set((state) =>
state.disownedPtyIds[ptyId]
? {}
? state
: { disownedPtyIds: { ...state.disownedPtyIds, [ptyId]: true } }
)
}
@@ -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]
@@ -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: {
@@ -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()
})
})
@@ -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 }
@@ -62,7 +62,7 @@ export function createTerminalStartupQueueActions(
}
set((s) => {
if (s.pendingStartupByTabId[tabId] !== pending) {
return {}
return s
}
const next = { ...s.pendingStartupByTabId }
delete next[tabId]
@@ -8,7 +8,7 @@ export function createTerminalUnverifiedPtyLossActions(
markUnverifiedPtyLoss: (tabId) => {
set((state) =>
state.unverifiedPtyLossTabIds[tabId]
? {}
? state
: { unverifiedPtyLossTabIds: { ...state.unverifiedPtyLossTabIds, [tabId]: true } }
)
}