From dd4e90519ba59c16b483af02aaca5b5ab853f4b1 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 04:45:02 +0100 Subject: [PATCH 1/6] test(inbox): correct the batch reconciliation comment framing The comment above the #2304 spec said the open detail was unrelated while the test reconciles the same id. The assertion is right; the framing was not. --- frontend/taskdeck-web/src/tests/store/captureStore.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index 0254176d0..323de21a9 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2459,8 +2459,10 @@ describe('captureStore', () => { store.pollBatchTriageCompletion(['c-1'], { limit: 200 }) await vi.advanceTimersByTimeAsync(3_000) - // The reconciliation read is in flight. An unrelated open detail must - // keep its panel and its refresh control. + // The reconciliation read is in flight. It is a quiet read, so + // whatever detail the panel has open keeps its body and its Refresh + // Detail control — including this row, which is the one being + // reconciled here. expect(captureApi.getItem).toHaveBeenCalledTimes(1) expect(store.loadingDetail).toBe(false) From bcd591647760c653016ae2aae83adc32577dd3dd Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 04:46:56 +0100 Subject: [PATCH 2/6] test(inbox): pin the quiet detail reconciliation for foreground batch triage batchTriage is a foreground caller of refreshTerminalDetails and no spec pinned that its reconciliation leaves loadingDetail alone. loadingDetail is one store-wide boolean, so raising it for the batch would blank the panel of an open capture outside the selection and would be cleared by the first parallel read to settle. batchBusy is the foreground state for a batch. --- .../taskdeck-web/src/store/captureStore.ts | 17 +++++++-- .../src/tests/store/captureStore.spec.ts | 35 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index 7f2151a3d..918b4b6c6 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -611,9 +611,20 @@ export const useCaptureStore = defineStore('capture', () => { // endpoint lags it briefly, do not regress the row back to Triaging // or reinsert an item that fell beyond the visible list cap. syncSummary: false, - // Quiet in every respect: this reconciliation runs for items the - // user may not have open, so it must not take the panel-wide - // loading flag away from whatever detail IS open (#2304). + // Quiet in every respect, for BOTH callers: this reconciliation + // runs over the tracked batch, not over whatever detail is open, + // so it must not take the panel-wide loading flag away from that + // detail (#2304). + // + // `batchTriage` is a FOREGROUND caller and still reconciles + // quietly on purpose (#2571). `loadingDetail` is one store-wide + // boolean: raising it here would blank the panel and disable + // Refresh Detail for an open capture that is not in the batch + // selection, and the first of these parallel reads to settle would + // clear the flag under any genuine foreground detail load still in + // flight. The foreground feedback for a batch is `batchBusy`, + // which stays true for the whole `batchTriage` body and renders + // "Processing" in the list panel. trackLoading: false, requestOptions: options.requestOptions, shouldCache: isCurrent, diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index 323de21a9..eb896acf6 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2475,6 +2475,41 @@ describe('captureStore', () => { } }) + it('leaves the store-wide detail loading flag alone through a foreground batch triage', async () => { + const store = useCaptureStore() + store.detailById['c-1'] = detailFor('c-1', 'Triaging') + vi.mocked(captureApi.batchTriage).mockResolvedValue({ + total: 1, + succeeded: 1, + failed: 0, + results: [{ itemId: 'c-1', success: true }], + }) + vi.mocked(captureApi.listItems).mockResolvedValue([ + summaryRow('c-1', 'ProposalCreated'), + ] as never) + let resolveDetail!: (value: unknown) => void + vi.mocked(captureApi.getItem).mockReturnValueOnce( + new Promise((resolve) => { resolveDetail = resolve }) as never, + ) + + const batch = store.batchTriage(['c-1'], 'triage') + await vi.waitFor(() => expect(captureApi.getItem).toHaveBeenCalledTimes(1)) + + // `batchTriage` is a foreground action, and its reconciliation is still + // quiet (#2571). `loadingDetail` is one store-wide flag, so raising it + // for the batch would spin an open capture outside the selection and + // would be cleared by the first parallel read to settle. `batchBusy` is + // the foreground state for a batch and it is up for the whole body. + expect(store.batchBusy).toBe(true) + expect(store.loadingDetail).toBe(false) + + resolveDetail(detailFor('c-1', 'ProposalCreated')) + await batch + + expect(store.loadingDetail).toBe(false) + expect(store.batchBusy).toBe(false) + }) + it('still raises the detail loading flag for a foreground detail load', async () => { const store = useCaptureStore() let resolveDetail!: (value: unknown) => void From b86234c0a3a7872fe3cbfbb768f95e7c92994de6 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 04:47:37 +0100 Subject: [PATCH 3/6] test(inbox): pin the per-tick coalescing of the batch count refresh The poll already refreshes the workload count at most once per tick, but no spec covered a single tick observing several new terminal outcomes at once. --- .../src/tests/store/captureStore.spec.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index eb896acf6..b72a74fef 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2562,6 +2562,42 @@ describe('captureStore', () => { } }) + it('coalesces the workload count refresh to one call per poll tick', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + vi.mocked(captureApi.listItems).mockResolvedValue([ + summaryRow('c-a', 'ProposalCreated'), + summaryRow('c-b', 'ProposalCreated'), + summaryRow('c-c', 'ProposalCreated'), + summaryRow('c-d', 'Triaging'), + ] as never) + + store.pollBatchTriageCompletion(['c-a', 'c-b', 'c-c', 'c-d'], { limit: 200 }) + await vi.advanceTimersByTimeAsync(3_000) + + // Three tracked ids reached a terminal outcome in the SAME tick. + // `New + Failed` is one number read from one endpoint — the heaviest on + // the surface — so the tick refreshes it once, not once per id (#2571). + expect(workspaceMocks.refreshWorkloadCounts).toHaveBeenCalledTimes(1) + + vi.mocked(captureApi.listItems).mockResolvedValue([ + summaryRow('c-a', 'ProposalCreated'), + summaryRow('c-b', 'ProposalCreated'), + summaryRow('c-c', 'ProposalCreated'), + summaryRow('c-d', 'Failed'), + ] as never) + await vi.advanceTimersByTimeAsync(3_000) + + // A second tick with its own new outcome is a second refresh: the + // coalescing is per tick, which is what keeps the badge honest while + // a batch finishes over the poll window. + expect(workspaceMocks.refreshWorkloadCounts).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + it('does not refresh workload counts from cached state when every poll read fails', async () => { vi.useFakeTimers() try { From 24811aba645b23b2863c06324197acf677d4de57 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 04:48:24 +0100 Subject: [PATCH 4/6] test(inbox): add the aborted negative control for the poll list error clear Superseded and 401/403 were proven not to clear a standing foreground listError. A poll stopped by the orchestrator, or aborted by the 60 s deadline, while its read is in flight was not. --- .../src/tests/store/captureStore.spec.ts | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index b72a74fef..f3e9db250 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2443,6 +2443,70 @@ describe('captureStore', () => { } }) + it('keeps a foreground list error when the poll is stopped mid-flight', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + let resolveList!: (value: unknown[]) => void + vi.mocked(captureApi.listItems) + .mockRejectedValueOnce(new Error('foreground-load-failed')) + .mockReturnValueOnce(new Promise((resolve) => { resolveList = resolve }) as never) + + await expect(store.fetchItems({ limit: 200 })).rejects.toThrow('foreground-load-failed') + expect(store.listError).toBe('Failed to load inbox items') + + const stop = store.pollBatchTriageCompletion(['c-1'], { limit: 200 }) + await vi.advanceTimersByTimeAsync(3_000) + expect(captureApi.listItems).toHaveBeenCalledTimes(2) + + // The orchestrator cancels this poll on a board or archived-history + // change and on unmount, which aborts the tick's read in flight. A + // response that lands after that is not proof the list the user is + // looking at became readable, so it must not clear their error (#2305). + stop() + resolveList([summaryRow('c-1', 'ProposalCreated')]) + await Promise.resolve() + await Promise.resolve() + + expect(store.listError).toBe('Failed to load inbox items') + expect(store.items).toEqual([]) + } finally { + vi.useRealTimers() + } + }) + + it('keeps a foreground list error when the deadline aborts a poll read mid-flight', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + let resolveList!: (value: unknown[]) => void + vi.mocked(captureApi.listItems) + .mockRejectedValueOnce(new Error('foreground-load-failed')) + .mockReturnValueOnce(new Promise((resolve) => { resolveList = resolve }) as never) + + await expect(store.fetchItems({ limit: 200 })).rejects.toThrow('foreground-load-failed') + expect(store.listError).toBe('Failed to load inbox items') + + store.pollBatchTriageCompletion(['c-1'], { limit: 200 }) + await vi.advanceTimersByTimeAsync(BATCH_TRIAGE_POLL_MAX_DURATION_MS) + + // The 60 s deadline aborted the one read that was still in flight. + expect(captureApi.listItems).toHaveBeenCalledTimes(2) + expect(store.batchError).toBe( + 'Automatic checking stopped after 60 seconds. Triage may still be running. Use Refresh Detail to check the result.', + ) + + resolveList([summaryRow('c-1', 'ProposalCreated')]) + await Promise.resolve() + await Promise.resolve() + + expect(store.listError).toBe('Failed to load inbox items') + expect(store.items).toEqual([]) + } finally { + vi.useRealTimers() + } + }) + it('leaves the store-wide detail loading flag alone during batch reconciliation', async () => { vi.useFakeTimers() try { From 12afa8f749d8bde9978cc4930b7f6078a168d166 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 04:51:37 +0100 Subject: [PATCH 5/6] fix(inbox): bound the capture store's per-item generation maps at logout Both generation guards are keyed by capture id and grow for the lifetime of the store with no eviction. resetForLogout clears them where the workspace store's own logout reset already runs. The shared clock stays monotonic, so a detail read still in flight from the previous session is dropped rather than cached. --- .../src/components/shell/AppShell.vue | 5 ++ .../taskdeck-web/src/store/captureStore.ts | 19 +++++ .../components/AppShell.paperVariant.spec.ts | 4 ++ .../src/tests/components/AppShell.spec.ts | 28 +++++++- .../src/tests/store/captureStore.spec.ts | 70 +++++++++++++++++++ 5 files changed, 123 insertions(+), 3 deletions(-) diff --git a/frontend/taskdeck-web/src/components/shell/AppShell.vue b/frontend/taskdeck-web/src/components/shell/AppShell.vue index 395ea78d0..ea64b183c 100644 --- a/frontend/taskdeck-web/src/components/shell/AppShell.vue +++ b/frontend/taskdeck-web/src/components/shell/AppShell.vue @@ -3,6 +3,7 @@ import { computed, onMounted, onUnmounted, ref, watch } from 'vue' import { useRouter } from 'vue-router' import { useSessionStore } from '../../store/sessionStore' import { useWorkspaceStore } from '../../store/workspaceStore' +import { useCaptureStore } from '../../store/captureStore' import { usePaperThemeStore } from '../../store/paperThemeStore' import { useCaptureQueueSync } from '../../composables/useCaptureQueueSync' import { registerEscapeHandler } from '../../composables/useEscapeStack' @@ -42,6 +43,7 @@ type SidebarRef = { const router = useRouter() const session = useSessionStore() const workspace = useWorkspaceStore() +const capture = useCaptureStore() const paperTheme = usePaperThemeStore() const { mode: viewportMode } = useViewportMode() @@ -303,6 +305,9 @@ watch( (isAuthenticated) => { if (!isAuthenticated) { workspace.resetForLogout() + // The capture store's per-item generation guards are keyed by capture id + // and belong to the session that recorded them (#2571). + capture.resetForLogout() return } diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index 918b4b6c6..ee09aa58a 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -934,6 +934,24 @@ export const useCaptureStore = defineStore('capture', () => { } } + /** + * Drop the per-item generation guards when the session ends (#2571). + * + * Both maps are keyed by capture id and take an entry for every summary + * write and every successful mutation, so they otherwise grow for the + * lifetime of the store with no eviction on scope change, list replacement + * or logout. They are guards, not data: an empty map reads as generation 0, + * which is the same "nothing recorded yet" a fresh store starts from. + * + * The shared clock itself stays monotonic on purpose. A detail read still in + * flight from the previous session then compares against a generation it can + * no longer match, so its response is dropped instead of cached. + */ + function resetForLogout() { + latestDetailWriteGenerationById.clear() + latestSummaryGenerationById.clear() + } + return { items, detailById, @@ -961,5 +979,6 @@ export const useCaptureStore = defineStore('capture', () => { pollBatchTriageCompletion, batchTriage, updateSuggestion, + resetForLogout, } }) diff --git a/frontend/taskdeck-web/src/tests/components/AppShell.paperVariant.spec.ts b/frontend/taskdeck-web/src/tests/components/AppShell.paperVariant.spec.ts index b3b24e821..557547ee6 100644 --- a/frontend/taskdeck-web/src/tests/components/AppShell.paperVariant.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/AppShell.paperVariant.spec.ts @@ -84,6 +84,10 @@ vi.mock('../../store/workspaceStore', () => ({ useWorkspaceStore: () => mockWorkspace, })) +vi.mock('../../store/captureStore', () => ({ + useCaptureStore: () => ({ resetForLogout: vi.fn() }), +})) + vi.mock('../../store/paperThemeStore', () => ({ usePaperThemeStore: () => mockPaperTheme, })) diff --git a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts index 93b692705..7e722bb71 100644 --- a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import { mount } from '@vue/test-utils' -import { defineComponent, h, reactive } from 'vue' +import { defineComponent, h, nextTick, reactive } from 'vue' import AppShell from '../../components/shell/AppShell.vue' import { useShellKeyboardHelp, @@ -30,11 +30,11 @@ const mockRoute = reactive({ path: '/workspace/home', }) -const mockSession = { +const mockSession = reactive({ isAuthenticated: true, username: 'test-user', logout: vi.fn(), -} +}) const mockFeatureFlags = { flags: { @@ -80,6 +80,14 @@ vi.mock('../../store/workspaceStore', () => ({ useWorkspaceStore: () => mockWorkspace, })) +const mockCapture = { + resetForLogout: vi.fn(), +} + +vi.mock('../../store/captureStore', () => ({ + useCaptureStore: () => mockCapture, +})) + const mockPaperTheme = reactive({ mode: 'off' as 'off' | 'paper' | 'paper-night' | 'auto', isOn: false, @@ -149,6 +157,7 @@ describe('AppShell workspace navigation and command palette', () => { mockWorkspace.preferencesHydrated = false mockFeatureFlags.isEnabled = vi.fn((_flag: keyof FeatureFlags) => true) mockPaperTheme.isOn = false + mockSession.isAuthenticated = true injectedShellHelp = null }) @@ -666,4 +675,17 @@ describe('AppShell workspace navigation and command palette', () => { expect(mockWorkspace.fetchHomeSummary).not.toHaveBeenCalled() }) + + it('resets the workspace and capture stores when the session ends', async () => { + mountedWrapper = mountShell() + expect(mockCapture.resetForLogout).not.toHaveBeenCalled() + + mockSession.isAuthenticated = false + await nextTick() + + // The capture store carries per-item generation guards keyed by capture + // id, so they belong to the session that recorded them (#2571). + expect(mockWorkspace.resetForLogout).toHaveBeenCalledOnce() + expect(mockCapture.resetForLogout).toHaveBeenCalledOnce() + }) }) diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index f3e9db250..7915e6bf8 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2740,6 +2740,76 @@ describe('captureStore', () => { } }, ) + + describe('resetForLogout', () => { + it('clears the per-item summary generation guard', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + store.items = [summaryRow('c-1', 'Triaging')] + + let resolveList!: (value: unknown[]) => void + vi.mocked(captureApi.listItems).mockReturnValueOnce( + new Promise((resolve) => { resolveList = resolve }) as never, + ) + + const stop = store.pollBatchTriageCompletion(['c-1'], { limit: 200 }) + await vi.advanceTimersByTimeAsync(3_000) + expect(captureApi.listItems).toHaveBeenCalledTimes(1) + + // A newer explicit read stamps c-1's summary generation above the + // in-flight snapshot's, which is what pins the row (#2301). + vi.mocked(captureApi.getItem).mockResolvedValue(detailFor('c-1', 'Failed')) + await store.fetchDetail('c-1', { forceRefresh: true }) + expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('Failed') + + store.resetForLogout() + + resolveList([summaryRow('c-1', 'Triaging')]) + await Promise.resolve() + await Promise.resolve() + + // The entry is gone, so nothing stale-high is left to pin the row. + expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('Triaging') + stop() + } finally { + vi.useRealTimers() + } + }) + + it('clears the per-item detail write guard', async () => { + const store = useCaptureStore() + vi.mocked(captureApi.batchTriage).mockResolvedValue({ + total: 1, + succeeded: 1, + failed: 0, + results: [{ itemId: 'c-1', success: true }], + }) + vi.mocked(captureApi.listItems).mockResolvedValue([ + summaryRow('c-1', 'ProposalCreated'), + ] as never) + + // The batch write records a detail write generation for c-1. + await store.batchTriage(['c-1'], 'triage') + expect(store.detailById['c-1']).toBeUndefined() + + let resolveDetail!: (value: unknown) => void + vi.mocked(captureApi.getItem).mockReturnValueOnce( + new Promise((resolve) => { resolveDetail = resolve }) as never, + ) + const load = store.fetchDetail('c-1', { forceRefresh: true }) + + store.resetForLogout() + + resolveDetail(detailFor('c-1', 'ProposalCreated')) + await load + + // That entry is gone too, so a read from the previous session no + // longer matches the generation it observed and is dropped rather + // than cached. + expect(store.detailById['c-1']).toBeUndefined() + }) + }) }) }) From 6af0a347c4994adf804f9533e323a20cdc2462b4 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 05:09:01 +0100 Subject: [PATCH 6/6] fix(inbox): invalidate in-flight detail reads at logout with a session epoch Clearing the generation maps alone inverted the detail write guard: a read that started with no recorded write observed generation 0, and after a write landed and the map was cleared the compare became 0 !== 0, so the pre-write body was cached over the newer one. A session epoch moves in resetForLogout after the clear, and fetchDetail and the single-item triage poll capture it when they issue their request and drop the response when it moved, before any generation compare. The docstring now says the maps are evicted at logout rather than bounded, in-session growth is unchanged, the batchBusy justification names the Legacy skin, and the foreground batch spec runs two stale ids against a concurrent genuine detail load. --- .../taskdeck-web/src/store/captureStore.ts | 48 +++++-- .../src/tests/store/captureStore.spec.ts | 119 +++++++++++++----- 2 files changed, 126 insertions(+), 41 deletions(-) diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index ee09aa58a..3d0e6c45e 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -119,6 +119,10 @@ export const useCaptureStore = defineStore('capture', () => { let latestListWriteGeneration = 0 const latestDetailWriteGenerationById = new Map() const latestSummaryGenerationById = new Map() + // Bumped by `resetForLogout`. Every detail read captures it when the request + // is issued, so a read that crosses a logout is dropped outright instead of + // being compared against generations the reset has already discarded. + let sessionEpoch = 0 const actionBusyItemId = ref(null) const listError = ref(null) const detailError = ref(null) @@ -272,6 +276,7 @@ export const useCaptureStore = defineStore('capture', () => { } const observedDetailWriteGeneration = detailWriteGeneration(itemId) + const observedSessionEpoch = sessionEpoch try { if (trackLoading) { loadingDetail.value = true @@ -283,6 +288,9 @@ export const useCaptureStore = defineStore('capture', () => { ? await captureApi.getItem(itemId, requestOptions) : await captureApi.getItem(itemId) if (!shouldCache()) return detail + // Before any generation compare: the reset discards the generations this + // read observed, so after a logout the compare is no longer meaningful. + if (observedSessionEpoch !== sessionEpoch) return detail if (observedDetailWriteGeneration !== detailWriteGeneration(itemId)) return detail cacheDetail(detail, syncSummary) return detail @@ -463,9 +471,15 @@ export const useCaptureStore = defineStore('capture', () => { try { const observedDetailWriteGeneration = detailWriteGeneration(itemId) + const observedSessionEpoch = sessionEpoch const detail = await captureApi.getItem(itemId) if (stopped) return - if (observedDetailWriteGeneration === detailWriteGeneration(itemId)) { + // The epoch is checked first: a logout discards the generations this + // read observed, so its response can no longer be reconciled. + if ( + observedSessionEpoch === sessionEpoch && + observedDetailWriteGeneration === detailWriteGeneration(itemId) + ) { cacheDetail(detail) if (isTriageTerminalStatus(detail.status)) { @@ -624,7 +638,12 @@ export const useCaptureStore = defineStore('capture', () => { // clear the flag under any genuine foreground detail load still in // flight. The foreground feedback for a batch is `batchBusy`, // which stays true for the whole `batchTriage` body and renders - // "Processing" in the list panel. + // "Processing..." on the Legacy skin's batch buttons + // (`LegacyInboxView.vue` -> `InboxListPanel.vue`). That is the only + // skin either flag reaches: nothing under `views/paper/` binds + // `batchBusy` or `loadingDetail`, so the Paper inbox neither gains + // nor loses anything here. The `trackLoading` docstring above + // describes the same Legacy panel. trackLoading: false, requestOptions: options.requestOptions, shouldCache: isCurrent, @@ -935,21 +954,28 @@ export const useCaptureStore = defineStore('capture', () => { } /** - * Drop the per-item generation guards when the session ends (#2571). + * End the session's per-item generation bookkeeping (#2571). * - * Both maps are keyed by capture id and take an entry for every summary - * write and every successful mutation, so they otherwise grow for the - * lifetime of the store with no eviction on scope change, list replacement - * or logout. They are guards, not data: an empty map reads as generation 0, - * which is the same "nothing recorded yet" a fresh store starts from. + * EVICTION, not a bound. Both maps are keyed by capture id and take an entry + * for every summary write and every successful mutation, and nothing else + * ever removes one, so a session's entries would otherwise live as long as + * the store. This is the only eviction point. WITHIN a session both still + * grow by one entry per distinct capture id touched, exactly as before. * - * The shared clock itself stays monotonic on purpose. A detail read still in - * flight from the previous session then compares against a generation it can - * no longer match, so its response is dropped instead of cached. + * INVALIDATION is what makes an in-flight read safe, and clearing the maps + * alone would not: it would INVERT the detail write guard. A read that + * starts with no recorded write for its id observes generation 0; if a write + * then lands (generation N) and the map is cleared, the read's compare + * becomes 0 !== 0, which is false, and the PRE-write body would be cached + * over the newer one and its status pushed back into the list. So the epoch + * moves here, after the clear: every detail read captures it when it issues + * its request and drops its response when it moved, before any generation + * compare. The shared clock itself stays monotonic. */ function resetForLogout() { latestDetailWriteGenerationById.clear() latestSummaryGenerationById.clear() + sessionEpoch += 1 } return { diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index 7915e6bf8..4a2e1936d 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2542,36 +2542,54 @@ describe('captureStore', () => { it('leaves the store-wide detail loading flag alone through a foreground batch triage', async () => { const store = useCaptureStore() store.detailById['c-1'] = detailFor('c-1', 'Triaging') + store.detailById['c-2'] = detailFor('c-2', 'Triaging') vi.mocked(captureApi.batchTriage).mockResolvedValue({ - total: 1, - succeeded: 1, + total: 2, + succeeded: 2, failed: 0, - results: [{ itemId: 'c-1', success: true }], + results: [ + { itemId: 'c-1', success: true }, + { itemId: 'c-2', success: true }, + ], }) vi.mocked(captureApi.listItems).mockResolvedValue([ summaryRow('c-1', 'ProposalCreated'), + summaryRow('c-2', 'ProposalCreated'), ] as never) - let resolveDetail!: (value: unknown) => void - vi.mocked(captureApi.getItem).mockReturnValueOnce( - new Promise((resolve) => { resolveDetail = resolve }) as never, - ) + const pendingDetails = new Map void>() + vi.mocked(captureApi.getItem).mockImplementation(((itemId: string) => + new Promise((resolve) => { pendingDetails.set(itemId, resolve) })) as never) - const batch = store.batchTriage(['c-1'], 'triage') - await vi.waitFor(() => expect(captureApi.getItem).toHaveBeenCalledTimes(1)) + // Two stale ids, so the reconciliation runs two reads in parallel. + const batch = store.batchTriage(['c-1', 'c-2'], 'triage') + await vi.waitFor(() => expect(captureApi.getItem).toHaveBeenCalledTimes(2)) - // `batchTriage` is a foreground action, and its reconciliation is still - // quiet (#2571). `loadingDetail` is one store-wide flag, so raising it - // for the batch would spin an open capture outside the selection and - // would be cleared by the first parallel read to settle. `batchBusy` is - // the foreground state for a batch and it is up for the whole body. + // `batchTriage` is a foreground action and its reconciliation is still + // quiet (#2571). `batchBusy` is the foreground state for a batch and it + // is up for the whole body. expect(store.batchBusy).toBe(true) expect(store.loadingDetail).toBe(false) - resolveDetail(detailFor('c-1', 'ProposalCreated')) + // A genuine foreground detail load starts while both reconciliation + // reads are still in flight. It owns the store-wide flag. + const foreground = store.fetchDetail('c-3', { forceRefresh: true }) + expect(store.loadingDetail).toBe(true) + + // The first reconciliation leg settles. Were those reads tracking the + // flag, this would clear it under the load that still owns it. + pendingDetails.get('c-1')!(detailFor('c-1', 'ProposalCreated')) + await Promise.resolve() + await Promise.resolve() + expect(store.loadingDetail).toBe(true) + + pendingDetails.get('c-2')!(detailFor('c-2', 'ProposalCreated')) await batch + expect(store.batchBusy).toBe(false) + expect(store.loadingDetail).toBe(true) + pendingDetails.get('c-3')!(detailFor('c-3', 'ProposalCreated')) + await foreground expect(store.loadingDetail).toBe(false) - expect(store.batchBusy).toBe(false) }) it('still raises the detail loading flag for a foreground detail load', async () => { @@ -2777,8 +2795,19 @@ describe('captureStore', () => { } }) - it('clears the per-item detail write guard', async () => { + it('drops a detail read that a write and a logout crossed', async () => { const store = useCaptureStore() + + // Nothing has been written for c-1 yet, so this read observes + // generation 0 — the value a cleared map also reads back. + let resolveDetail!: (value: unknown) => void + vi.mocked(captureApi.getItem).mockReturnValueOnce( + new Promise((resolve) => { resolveDetail = resolve }) as never, + ) + const load = store.fetchDetail('c-1', { forceRefresh: true }) + + // A successful write lands while that read is in flight. It is the + // write generation that would normally reject the older response. vi.mocked(captureApi.batchTriage).mockResolvedValue({ total: 1, succeeded: 1, @@ -2788,26 +2817,56 @@ describe('captureStore', () => { vi.mocked(captureApi.listItems).mockResolvedValue([ summaryRow('c-1', 'ProposalCreated'), ] as never) - - // The batch write records a detail write generation for c-1. await store.batchTriage(['c-1'], 'triage') - expect(store.detailById['c-1']).toBeUndefined() - - let resolveDetail!: (value: unknown) => void - vi.mocked(captureApi.getItem).mockReturnValueOnce( - new Promise((resolve) => { resolveDetail = resolve }) as never, - ) - const load = store.fetchDetail('c-1', { forceRefresh: true }) + expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('ProposalCreated') store.resetForLogout() - resolveDetail(detailFor('c-1', 'ProposalCreated')) + resolveDetail(detailFor('c-1', 'Triaging')) await load - // That entry is gone too, so a read from the previous session no - // longer matches the generation it observed and is dropped rather - // than cached. + // Clearing the maps alone would put the compare back at 0 !== 0 and + // cache this pre-write body. The session epoch drops it instead. expect(store.detailById['c-1']).toBeUndefined() + expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('ProposalCreated') + }) + + it('drops a single-item triage poll read that a write and a logout crossed', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + let resolveDetail!: (value: unknown) => void + vi.mocked(captureApi.getItem).mockReturnValueOnce( + new Promise((resolve) => { resolveDetail = resolve }) as never, + ) + + const stop = store.pollTriageCompletion('c-1') + await vi.advanceTimersByTimeAsync(2_000) + expect(captureApi.getItem).toHaveBeenCalledTimes(1) + + vi.mocked(captureApi.batchTriage).mockResolvedValue({ + total: 1, + succeeded: 1, + failed: 0, + results: [{ itemId: 'c-1', success: true }], + }) + vi.mocked(captureApi.listItems).mockResolvedValue([ + summaryRow('c-1', 'ProposalCreated'), + ] as never) + await store.batchTriage(['c-1'], 'triage') + + store.resetForLogout() + + resolveDetail(detailFor('c-1', 'Triaging')) + await Promise.resolve() + await Promise.resolve() + + expect(store.detailById['c-1']).toBeUndefined() + expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('ProposalCreated') + stop() + } finally { + vi.useRealTimers() + } }) }) })