diff --git a/src/renderer/src/lib/session-write-subscriber.test.ts b/src/renderer/src/lib/session-write-subscriber.test.ts index 2f31550c6c0..dda5c01f44c 100644 --- a/src/renderer/src/lib/session-write-subscriber.test.ts +++ b/src/renderer/src/lib/session-write-subscriber.test.ts @@ -102,6 +102,27 @@ describe('createSessionWriteSubscriber', () => { cleanup() }) + it('writes when the gate is already open at creation, with no store tick to wake it', () => { + // Why the store-write spy: every other case here opens the gate *after* creating the + // subscriber, so the opening setState is itself the tick that produces the first write. With + // the gate already open only the creation-time seed can, and equal catalogs no longer publish + // a tick to stand in for it. + useAppStore.setState({ workspaceSessionReady: true, hydrationSucceeded: true }) + + const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>() + const storeTicks = vi.fn() + const unsubStoreTicks = useAppStore.subscribe(storeTicks) + const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist }) + + vi.advanceTimersByTime(200) + + expect(storeTicks).not.toHaveBeenCalled() + expect(persist).toHaveBeenCalledTimes(1) + + unsubStoreTicks() + cleanup() + }) + it('re-checks the hydration gate when a pending debounce fires', () => { const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>() const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist }) diff --git a/src/renderer/src/lib/session-write-subscriber.ts b/src/renderer/src/lib/session-write-subscriber.ts index 685a8e56e3f..7e7e9cf7360 100644 --- a/src/renderer/src/lib/session-write-subscriber.ts +++ b/src/renderer/src/lib/session-write-subscriber.ts @@ -201,7 +201,7 @@ export function createSessionWriteSubscriber({ return false } - const unsub = store.subscribe((state) => { + const evaluateSessionState = (state: AppState): void => { if (!shouldPersistWorkspaceSession(state)) { return } @@ -255,7 +255,14 @@ export function createSessionWriteSubscriber({ return } armFlushTimer() - }) + } + + // Why evaluate once here: `prev === null` is what bootstraps the first full write, so a writer + // created when the session gate is *already* open owed that write to whatever unrelated store + // tick happened to arrive next. Catalog refreshes no longer publish when nothing changed, so + // that incidental wake-up is not guaranteed; seed from the current state instead. + evaluateSessionState(store.getState()) + const unsub = store.subscribe(evaluateSessionState) const unsubGateOpen = subscribeToPersistGateOpen?.(() => { if (pendingChangedFields.size === 0 || timer !== null) { diff --git a/src/renderer/src/store/folder-workspaces/folder-workspace-catalog-actions.ts b/src/renderer/src/store/folder-workspaces/folder-workspace-catalog-actions.ts index a5636882ea5..83678c50946 100644 --- a/src/renderer/src/store/folder-workspaces/folder-workspace-catalog-actions.ts +++ b/src/renderer/src/store/folder-workspaces/folder-workspace-catalog-actions.ts @@ -13,6 +13,7 @@ import { mergeFetchedFolderWorkspaceCatalog } from './folder-workspace-catalog' import { listRuntimeEnvironmentsForAllHostLoad } from '../runtime-catalog-hosts' +import { reuseEqualRecordMap } from '../slices/repo-identity-reconcile' import { getFolderWorkspaceUpdateCoordinator } from './folder-workspace-mutations' export function createFolderWorkspaceCatalogActions( @@ -47,11 +48,12 @@ export function createFolderWorkspaceCatalogActions( current.folderWorkspaces, current.projectGroups ) + if (arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces)) { + return current + } return { folderWorkspaces, - ...(arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces) - ? {} - : { folderWorkspacePathStatuses: {} }) + folderWorkspacePathStatuses: {} } }) } catch (err) { @@ -85,11 +87,12 @@ export function createFolderWorkspaceCatalogActions( current.folderWorkspaces, current.projectGroups ) + if (arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces)) { + return current + } return { folderWorkspaces, - ...(arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces) - ? {} - : { folderWorkspacePathStatuses: {} }) + folderWorkspacePathStatuses: {} } }) } @@ -130,12 +133,22 @@ export function createFolderWorkspaceCatalogActions( }) ) if (!failed) { - set((s) => ({ - restoredRuntimeHostIdByWorkspaceSessionKey: clearRestoredFolderWorkspaceSessionOwners( + set((s) => { + // Why reuseEqualRecordMap: the cleanup rebuilds the record every refresh, and + // reference-equality readers (live dashboard selector, popout bridge) re-run on a + // fresh-but-equal identity, so the equal case must keep the previous one. + const restoredRuntimeHostIdByWorkspaceSessionKey = reuseEqualRecordMap( s.restoredRuntimeHostIdByWorkspaceSessionKey, - s + clearRestoredFolderWorkspaceSessionOwners( + s.restoredRuntimeHostIdByWorkspaceSessionKey, + s + ) ) - })) + return restoredRuntimeHostIdByWorkspaceSessionKey === + s.restoredRuntimeHostIdByWorkspaceSessionKey + ? s + : { restoredRuntimeHostIdByWorkspaceSessionKey } + }) } } } diff --git a/src/renderer/src/store/project-groups/project-group-catalog-actions.ts b/src/renderer/src/store/project-groups/project-group-catalog-actions.ts index c7841c85e0c..d19ac185766 100644 --- a/src/renderer/src/store/project-groups/project-group-catalog-actions.ts +++ b/src/renderer/src/store/project-groups/project-group-catalog-actions.ts @@ -32,11 +32,12 @@ export function createProjectGroupCatalogActions( return current } const { projectGroups } = mergeFetchedProjectGroupCatalog(catalog, current.projectGroups) + if (arrayElementsUnchanged(projectGroups, current.projectGroups)) { + return current + } return { projectGroups, - ...(arrayElementsUnchanged(projectGroups, current.projectGroups) - ? {} - : { folderWorkspacePathStatuses: {} }) + folderWorkspacePathStatuses: {} } }) } catch (err) { @@ -55,11 +56,12 @@ export function createProjectGroupCatalogActions( return s } const { projectGroups } = mergeFetchedProjectGroupCatalog(catalog, s.projectGroups) + if (arrayElementsUnchanged(projectGroups, s.projectGroups)) { + return s + } return { projectGroups, - ...(arrayElementsUnchanged(projectGroups, s.projectGroups) - ? {} - : { folderWorkspacePathStatuses: {} }) + folderWorkspacePathStatuses: {} } }) } diff --git a/src/renderer/src/store/slices/repos-catalog-notifications.test.ts b/src/renderer/src/store/slices/repos-catalog-notifications.test.ts new file mode 100644 index 00000000000..88fbd339ec3 --- /dev/null +++ b/src/renderer/src/store/slices/repos-catalog-notifications.test.ts @@ -0,0 +1,229 @@ +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type { FolderWorkspace } from '../../../../shared/folder-workspace-types' +import type { ProjectGroup } from '../../../../shared/project-group-types' +import { createTestStore } from './store-test-helpers' + +const group: ProjectGroup = { + id: 'group-1', + name: 'Group', + parentPath: '/parent', + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 3 +} +const folder: FolderWorkspace = { + id: 'folder-1', + projectGroupId: group.id, + name: 'Folder', + folderPath: '/parent/folder', + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 1, + createdAt: 1, + updatedAt: 3 +} +const groupsList = vi.fn<() => Promise>() +const foldersList = vi.fn<() => Promise>() +const folderUpdate = vi.fn() +type TestStore = ReturnType +type CatalogCase = { + label: string + catalog: 'projectGroups' | 'folderWorkspaces' + refresh: (store: TestStore) => Promise +} +const cases: CatalogCase[] = [ + { + label: 'selected groups', + catalog: 'projectGroups', + refresh: (s) => s.getState().fetchProjectGroups() + }, + { + label: 'all-host local groups', + catalog: 'projectGroups', + refresh: (s) => s.getState().fetchProjectGroupsForAllHosts({ remoteHosts: 'skip' }) + }, + { + label: 'selected folders', + catalog: 'folderWorkspaces', + refresh: (s) => s.getState().fetchFolderWorkspaces() + }, + { + label: 'all-host local folders', + catalog: 'folderWorkspaces', + refresh: (s) => s.getState().fetchFolderWorkspacesForAllHosts({ remoteHosts: 'skip' }) + } +] + +beforeEach(() => { + groupsList.mockReset().mockImplementation(async () => structuredClone([group])) + foldersList.mockReset().mockImplementation(async () => structuredClone([folder])) + folderUpdate.mockReset() + vi.stubGlobal('window', { + api: { + projectGroups: { list: groupsList }, + folderWorkspaces: { list: foldersList, update: folderUpdate }, + runtimeEnvironments: { list: async () => [] } + }, + dispatchEvent: vi.fn() + }) + vi.spyOn(console, 'error').mockImplementation(() => {}) +}) +afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) + +function seed(): TestStore { + const store = createTestStore() + store.setState({ + projectGroups: [{ ...group, executionHostId: 'local' }], + folderWorkspaces: [{ ...folder, executionHostId: 'local' }], + folderWorkspacePathStatuses: { + cached: { + status: { path: folder.folderPath, exists: true }, + checkedAt: 1, + requestSnapshot: 'snapshot' + } + } + }) + return store +} + +it.each(cases)('does not notify for ten equal $label refreshes', async ({ refresh, catalog }) => { + const store = seed() + const initial = store.getState() + const changed = vi.fn() + const unsubscribe = store.subscribe(changed) + for (let index = 0; index < 10; index += 1) { + await refresh(store) + } + unsubscribe() + expect(changed).not.toHaveBeenCalled() + expect(store.getState()).toBe(initial) + expect(store.getState()[catalog]).toBe(initial[catalog]) + expect(store.getState().folderWorkspacePathStatuses).toBe(initial.folderWorkspacePathStatuses) +}) + +it.each(cases)( + 'publishes changed $label and invalidates path statuses', + async ({ refresh, catalog }) => { + const store = seed() + groupsList.mockResolvedValue([{ ...group, name: 'Changed group' }]) + foldersList.mockResolvedValue([{ ...folder, name: 'Changed folder' }]) + const changed = vi.fn() + const unsubscribe = store.subscribe(changed) + await refresh(store) + unsubscribe() + expect(changed).toHaveBeenCalledOnce() + expect(store.getState()[catalog][0]?.name).toMatch(/^Changed /) + expect(store.getState().folderWorkspacePathStatuses).toEqual({}) + } +) + +it.each(cases)('retains state after a failed $label read', async ({ refresh }) => { + const store = seed() + const initial = store.getState() + groupsList.mockRejectedValue(new Error('offline')) + foldersList.mockRejectedValue(new Error('offline')) + const changed = vi.fn() + const unsubscribe = store.subscribe(changed) + await refresh(store) + unsubscribe() + expect(store.getState()).toBe(initial) + expect(changed).not.toHaveBeenCalled() +}) + +it.each(cases)( + 'fences an older $label response after a newer no-op response', + async ({ refresh, catalog }) => { + const store = seed() + const initial = store.getState() + const olderGroups = Promise.withResolvers() + const olderFolders = Promise.withResolvers() + if (catalog === 'projectGroups') { + groupsList.mockReturnValueOnce(olderGroups.promise) + } else { + foldersList.mockReturnValueOnce(olderFolders.promise) + } + const older = refresh(store) + const changed = vi.fn() + const unsubscribe = store.subscribe(changed) + await refresh(store) + olderGroups.resolve([{ ...group, name: 'Obsolete group' }]) + olderFolders.resolve([{ ...folder, name: 'Obsolete folder' }]) + await older + unsubscribe() + expect(store.getState()).toBe(initial) + expect(changed).not.toHaveBeenCalled() + } +) + +it.each(cases.filter((entry) => entry.catalog === 'folderWorkspaces'))( + 'keeps update revision fencing after an equal $label response', + async ({ refresh }) => { + const store = seed() + const pending = Promise.withResolvers() + folderUpdate.mockReturnValueOnce(pending.promise) + const update = store.getState().updateFolderWorkspace(folder.id, { isUnread: true }) + const initial = store.getState() + await refresh(store) + expect(store.getState()).toBe(initial) + pending.resolve({ ...folder, isUnread: true, updatedAt: 2 }) + await update + expect(store.getState().folderWorkspaces[0]?.isUnread).toBe(false) + expect(store.getState().folderWorkspaces[0]?.updatedAt).toBe(3) + } +) + +it('accepts a newer update after an equal catalog response', async () => { + const store = seed() + const pending = Promise.withResolvers() + folderUpdate.mockReturnValueOnce(pending.promise) + const update = store.getState().updateFolderWorkspace(folder.id, { isUnread: true }) + await store.getState().fetchFolderWorkspaces() + pending.resolve({ ...folder, isUnread: true, updatedAt: 4 }) + await update + expect(store.getState().folderWorkspaces[0]?.isUnread).toBe(true) + expect(store.getState().folderWorkspaces[0]?.updatedAt).toBe(4) +}) + +it('still clears restored folder owners after a successful all-host refresh', async () => { + const store = seed() + store.setState({ + restoredRuntimeHostIdByWorkspaceSessionKey: { + 'folder:folder-1': 'runtime:retired', + unrelated: 'runtime:kept' + } + }) + const changed = vi.fn() + const unsubscribe = store.subscribe(changed) + await store.getState().fetchFolderWorkspacesForAllHosts() + unsubscribe() + expect(changed).toHaveBeenCalledOnce() + expect(store.getState().restoredRuntimeHostIdByWorkspaceSessionKey).toEqual({ + unrelated: 'runtime:kept' + }) +}) + +it('does not publish an all-host refresh whose restored owners need no cleanup', async () => { + const store = seed() + store.setState({ restoredRuntimeHostIdByWorkspaceSessionKey: { unrelated: 'runtime:kept' } }) + const initial = store.getState() + const changed = vi.fn() + const unsubscribe = store.subscribe(changed) + await store.getState().fetchFolderWorkspacesForAllHosts() + unsubscribe() + expect(changed).not.toHaveBeenCalled() + expect(store.getState()).toBe(initial) + expect(store.getState().restoredRuntimeHostIdByWorkspaceSessionKey).toBe( + initial.restoredRuntimeHostIdByWorkspaceSessionKey + ) +})