From 8f3bc0ca564e5200b69532175bd0509dd4a325a9 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 07:25:31 +0100 Subject: [PATCH 1/6] test(inbox): bound the never-resolving detail mock to its own test The foreground-batch-triage test installed a persistent mockImplementation returning promises that never resolve. The suite's only reset is vi.clearAllMocks(), which clears recorded calls but keeps implementations, so any later test that triggered an unconfigured detail read hung on a promise that test installed instead of receiving undefined. Three mockImplementationOnce calls cover this test's three reads exactly. The guard test that follows races the surviving implementation against a macrotask: a leaked mockResolvedValue settles and is harmless, a leaked never-resolving one does not. Refs #2640 --- .../src/tests/store/captureStore.spec.ts | 31 +++++++++++++++++-- 1 file changed, 29 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 4a2e1936d..b5c4da84a 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2557,8 +2557,19 @@ describe('captureStore', () => { summaryRow('c-2', 'ProposalCreated'), ] as never) const pendingDetails = new Map void>() - vi.mocked(captureApi.getItem).mockImplementation(((itemId: string) => - new Promise((resolve) => { pendingDetails.set(itemId, resolve) })) as never) + // Bounded to this test's three reads (#2640). A PERSISTENT + // `mockImplementation` returning never-resolving promises outlives the + // test: the suite's only reset is `vi.clearAllMocks()`, which clears + // recorded calls but keeps implementations, so any later test that + // triggers an unconfigured detail read would hang on a promise this test + // installed instead of receiving `undefined`. A fourth read here fails on + // an absent `pendingDetails` entry, which is a visible failure. + const pendingDetail = ((itemId: string) => + new Promise((resolve) => { pendingDetails.set(itemId, resolve) })) as never + vi.mocked(captureApi.getItem) + .mockImplementationOnce(pendingDetail) + .mockImplementationOnce(pendingDetail) + .mockImplementationOnce(pendingDetail) // Two stale ids, so the reconciliation runs two reads in parallel. const batch = store.batchTriage(['c-1', 'c-2'], 'triage') @@ -2592,6 +2603,22 @@ describe('captureStore', () => { expect(store.loadingDetail).toBe(false) }) + // Ordering-coupled on purpose (#2640). It sits directly after the test that + // installs never-resolving detail promises. `vi.clearAllMocks()` clears + // recorded calls but keeps implementations, so whatever that test installed + // is what the tests below inherit. A leaked `mockResolvedValue` from some + // earlier test is harmless — it settles — so the property that matters is + // not "unconfigured" but "settles": a leaked never-resolving implementation + // hangs whichever later test triggers an unconfigured detail read. A + // microtask always wins against a macrotask, so this race is deterministic. + it('leaves no never-resolving detail mock behind for the tests below', async () => { + const outcome = await Promise.race([ + Promise.resolve(vi.mocked(captureApi.getItem)('c-unconfigured')).then(() => 'settles'), + new Promise((resolve) => { setTimeout(() => resolve('hangs'), 0) }), + ]) + expect(outcome).toBe('settles') + }) + it('still raises the detail loading flag for a foreground detail load', async () => { const store = useCaptureStore() let resolveDetail!: (value: unknown) => void From dc6abc8c432e25141420327e346f659c9d67f5c6 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 07:28:56 +0100 Subject: [PATCH 2/6] fix(inbox): report a dropped detail read and stop selecting on it fetchDetail has three paths that resolve with the body they fetched and write nothing into the caches: the caller's shouldCache opt-out, a session epoch the logout moved, and a write generation a newer mutation moved. It reported none of them, so selectItemById returned true for a dropped read and left selectedItemId pointing at an id detailById holds nothing for. InboxDetailPanel renders exactly that state as "Unable to load capture detail.", for a read that succeeded. The three drop paths collapse into one cached boolean, reported through a new DetailLoadOptions.onCacheOutcome callback. The report is a callback rather than a return value because fetchDetail resolves with the CaptureItem and a view annotates that type, so widening the return would change a surface outside this seam. selectItemById restores the selection the user had and returns false when a dropped read left the store with no detail for the id. A drop that a newer mutation caused keeps the selection: that write cached its own newer body, so the panel has something honest to render. A rejected read still clears the selection outright, as before. Refs #2640 --- .../src/composables/useInboxOrchestrator.ts | 28 +++++- .../taskdeck-web/src/store/captureStore.ts | 47 +++++++-- .../composables/useInboxOrchestrator.spec.ts | 97 ++++++++++++++++++- .../src/tests/views/InboxView.spec.ts | 25 ++++- 4 files changed, 183 insertions(+), 14 deletions(-) diff --git a/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts b/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts index 880396d0a..e0a2e8ea6 100644 --- a/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts +++ b/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts @@ -298,6 +298,7 @@ export function useInboxOrchestrator(options: { // only the list write is suppressed. Paper loads through `peekDetail` and is // unaffected either way. const syncSummary = cacheSummary && !isArchivedHistory.value + const previousSelectedItemId = selectedItemId.value primeSelection(itemId, preferredIndex) hashLoadFailedItemId.value = null try { @@ -305,7 +306,32 @@ export function useInboxOrchestrator(options: { captureStore.cacheDetail(preloadedDetail, syncSummary) return true } - await captureStore.fetchDetail(itemId, { syncSummary }) + // `onCacheOutcome` is the store reporting that THIS read reached its + // caches (#2640). `fetchDetail` resolves with the body it fetched on + // three paths that write nothing — a `shouldCache` opt-out, a session + // epoch the logout moved, a write generation a newer mutation moved — so + // resolution alone is not evidence the panel has a detail to render. + // Returning `true` there left `selectedItemId` on an id `detailById` + // holds nothing for, which `InboxDetailPanel` renders as "Unable to load + // capture detail." for a read that succeeded. A store that reports + // nothing is read as having cached, which is the behaviour before #2640. + let cached = true + await captureStore.fetchDetail(itemId, { + syncSummary, + onCacheOutcome: (didCache) => { cached = didCache }, + }) + // A drop can still leave a detail here: a mutation that superseded this + // read cached its own newer body under the same id. The panel is honest + // then and the selection stands. Only a drop that left the store with + // nothing for this id is a selection this surface cannot show, and that + // is not a failure either — so restore the selection the user had rather + // than clearing it, and report the read as not opened. + if (!cached && !captureStore.detailById[itemId]) { + if (selectedItemId.value === itemId) { + selectedItemId.value = previousSelectedItemId + } + return false + } return true } catch { if (selectedItemId.value === itemId) { diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index 3d0e6c45e..2049addba 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -41,6 +41,32 @@ type DetailLoadOptions = { * quiet reads pass `false` and leave the flag to foreground loads. */ trackLoading?: boolean + /** + * REPORT whether this call left `detailById` holding this item's detail + * (#2640) — the detail-read counterpart of `fetchItems`'s applied boolean. + * + * `fetchDetail` has three paths that resolve with the body they fetched and + * write nothing: the caller's own `shouldCache` opt-out, a session epoch the + * logout moved, and a write generation a newer mutation moved. Resolution + * alone therefore never meant "the store now holds this detail", and callers + * that assumed it did acted on a response the store had dropped: + * `useInboxOrchestrator.selectItemById` left `selectedItemId` pointing at an + * id with no detail, which the Legacy panel renders as "Unable to load + * capture detail." for a read that succeeded, and `refreshTerminalDetails` + * announced a reconciliation that never reached `detailById`. + * + * `true` also covers the two paths that resolve from state already present — + * the non-forced cached early return and the demo branch — because the + * question a caller asks is whether the store holds this detail now, not + * whether this particular call did the writing. A read that THROWS reports + * nothing: failure and a drop stay distinguishable. + * + * A callback rather than a return value because `fetchDetail` resolves with + * the `CaptureItem` itself and a view annotates that type + * (`views/paper/inbox/PaperTriageRowEdit.vue`), so widening the return would + * change a surface outside this seam. + */ + onCacheOutcome?: (cached: boolean) => void } export const BATCH_TRIAGE_POLL_INTERVAL_MS = 3_000 @@ -260,9 +286,11 @@ export const useCaptureStore = defineStore('capture', () => { requestOptions, shouldCache = () => true, trackLoading = true, + onCacheOutcome, } = options if (!forceRefresh && detailById.value[itemId]) { + onCacheOutcome?.(true) return detailById.value[itemId] } @@ -271,6 +299,7 @@ export const useCaptureStore = defineStore('capture', () => { if (summary) { const detail = { ...summary, rawText: summary.textExcerpt, retryCount: 0, provenance: null } cacheDetail(detail, syncSummary) + onCacheOutcome?.(true) return detail } } @@ -287,12 +316,18 @@ export const useCaptureStore = defineStore('capture', () => { const detail = requestOptions ? 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) + // The three drop paths, in the order they have always been checked and + // reported as one boolean (#2640). The epoch comes before any generation + // compare: the reset discards the generations this read observed, so + // after a logout the compare is no longer meaningful. + const cached = + shouldCache() && + observedSessionEpoch === sessionEpoch && + observedDetailWriteGeneration === detailWriteGeneration(itemId) + if (cached) { + cacheDetail(detail, syncSummary) + } + onCacheOutcome?.(cached) return detail } catch (e: unknown) { const message = getErrorDisplay(e, 'Failed to load inbox item').message diff --git a/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts b/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts index 2b8484ed1..fea6f2619 100644 --- a/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts +++ b/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts @@ -442,6 +442,7 @@ describe('useInboxOrchestrator', () => { await orch.openItemFromList(summaryRow('archived-capture', 'archived-board'), 0) expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('archived-capture', { syncSummary: false, + onCacheOutcome: expect.any(Function), }) // Refresh Detail is a READ affordance and stays available in history mode, @@ -465,6 +466,7 @@ describe('useInboxOrchestrator', () => { await orch.openItemFromList(summaryRow('live-capture', 'live-board'), 0) expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('live-capture', { syncSummary: true, + onCacheOutcome: expect.any(Function), }) mockCaptureStore.fetchDetail.mockClear() @@ -686,7 +688,10 @@ describe('useInboxOrchestrator', () => { const event = { key: 'Enter', preventDefault: vi.fn() } as unknown as KeyboardEvent await orch.handleKeydown(event) expect(event.preventDefault).toHaveBeenCalled() - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('abc', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('abc', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) }) }) @@ -695,7 +700,10 @@ describe('useInboxOrchestrator', () => { mockRoute.hash = '#capture-deep-id' const orch = createOrchestrator() await orch.loadInbox() - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('deep-id', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('deep-id', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) }) it('loadInbox does not fetch detail when hash is absent', async () => { @@ -1026,4 +1034,89 @@ describe('useInboxOrchestrator', () => { expect(mockCaptureStore.fetchDetail).not.toHaveBeenCalled() }) }) + + /** + * A DROPPED read is the store resolving with the body it fetched while + * writing nothing into its caches (#2640). It is neither a failure nor a + * cached detail, and `fetchDetail` used to report it to nobody, so + * `selectItemById` returned `true` for it and left `selectedItemId` pointing + * at an id `detailById` holds nothing for — the state the Legacy detail panel + * renders as "Unable to load capture detail." for a read that succeeded. + * + * `onCacheOutcome` is the store's report. `mockImplementationOnce` on + * purpose: the suite's reset is `vi.clearAllMocks()`, which keeps + * implementations, so a persistent one here would follow the tests below. + */ + describe('dropped detail reads', () => { + const keptDetail = { id: 'kept', rawText: 'kept body', boardId: null, status: 'New' } + + it('holds the previous selection when the store reports a dropped read', async () => { + mockCaptureStore.items = [{ id: 'kept' }, { id: 'dropped' }] + mockCaptureStore.detailById = { kept: keptDetail } + const orch = createOrchestrator() + + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + _itemId: string, + options?: { onCacheOutcome?: (cached: boolean) => void }, + ) => { options?.onCacheOutcome?.(true) }) + await orch.openItemFromList(summaryRow('kept', null), 0) + expect(orch.selectedItemId.value).toBe('kept') + + // The store fetched a body and cached none of it, and nothing else has + // put a detail for this id in `detailById` either. + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + _itemId: string, + options?: { onCacheOutcome?: (cached: boolean) => void }, + ) => { options?.onCacheOutcome?.(false) }) + await orch.openItemFromList(summaryRow('dropped', null), 1) + + expect(orch.selectedItemId.value).toBe('kept') + expect(orch.selectedItem.value).toEqual(keptDetail) + }) + + it('keeps the selection when a dropped read left a newer detail cached', async () => { + mockCaptureStore.items = [{ id: 'superseded' }] + mockCaptureStore.detailById = {} + const orch = createOrchestrator() + + // The other pre-existing drop path: a successful write superseded this + // read's generation. That write cached its OWN newer body, so the panel + // has a detail to render and the selection is honest. + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + itemId: string, + options?: { onCacheOutcome?: (cached: boolean) => void }, + ) => { + mockCaptureStore.detailById[itemId] = { + id: itemId, rawText: 'newer body', boardId: null, status: 'ProposalCreated', + } + options?.onCacheOutcome?.(false) + }) + await orch.openItemFromList(summaryRow('superseded', null), 0) + + expect(orch.selectedItemId.value).toBe('superseded') + expect(orch.selectedItem.value).toEqual({ + id: 'superseded', rawText: 'newer body', boardId: null, status: 'ProposalCreated', + }) + }) + + it('still treats a rejected detail read as a failure', async () => { + mockCaptureStore.items = [{ id: 'kept' }, { id: 'broken' }] + mockCaptureStore.detailById = { kept: keptDetail } + const orch = createOrchestrator() + + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + _itemId: string, + options?: { onCacheOutcome?: (cached: boolean) => void }, + ) => { options?.onCacheOutcome?.(true) }) + await orch.openItemFromList(summaryRow('kept', null), 0) + + mockCaptureStore.fetchDetail.mockRejectedValueOnce(new Error('detail unavailable')) + await orch.openItemFromList(summaryRow('broken', null), 1) + + // A failure clears the selection outright, as before. It does not restore + // the previous one: the user asked for this row and the store reported it + // unreadable, so the panel's error surface is the honest one. + expect(orch.selectedItemId.value).toBeNull() + }) + }) }) diff --git a/frontend/taskdeck-web/src/tests/views/InboxView.spec.ts b/frontend/taskdeck-web/src/tests/views/InboxView.spec.ts index ae9fc421a..ec6ee4ebb 100644 --- a/frontend/taskdeck-web/src/tests/views/InboxView.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/InboxView.spec.ts @@ -409,7 +409,10 @@ describe('InboxView', () => { await waitForUi() expect(mockCaptureStore.fetchItems).toHaveBeenCalledWith({ limit: 200 }) - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-2', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-2', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) expect(wrapper.text()).toContain('Full text for capture-2') }) @@ -684,7 +687,10 @@ describe('InboxView', () => { const wrapper = mount(InboxView) await waitForUi() - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('missing-capture', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('missing-capture', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) expect(wrapper.text()).toContain('Select an item to inspect the captured text') expect(routerMocks.replace).toHaveBeenCalledWith({ name: 'workspace-inbox', @@ -712,7 +718,10 @@ describe('InboxView', () => { await firstRow.trigger('click') await waitForUi() - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-1', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-1', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) expect(wrapper.text()).toContain('Full text for capture-1') }) @@ -725,7 +734,10 @@ describe('InboxView', () => { await listbox.trigger('keydown', { key: 'Enter' }) await waitForUi() - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-2', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-2', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) expect(wrapper.text()).toContain('Full text for capture-2') }) @@ -767,7 +779,10 @@ describe('InboxView', () => { await listbox.trigger('keydown', { key: 'Enter' }) await waitForUi() - expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-2', { syncSummary: true }) + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledWith('capture-2', { + syncSummary: true, + onCacheOutcome: expect.any(Function), + }) expect(wrapper.text()).toContain('Full text for capture-2') }) From d85a7680a6b98ad4f9525d3965b8b1ecd2989173 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 07:31:43 +0100 Subject: [PATCH 3/6] fix(inbox): fire onRefreshed only for a reconciliation that cached refreshTerminalDetails announced an id as refreshed whenever fetchDetail resolved, which a dropped read also does. refreshedDetailIds is the batch poll's record of ids it reconciled, and for a tracked item beyond the newest-first list cap it is the whole of isComplete's and isObservedTerminal's evidence, so the poll could stop and move the workload badge on pre-batch detail state that no reconciliation had touched. The reporter from the previous commit gates it: the id is announced only when the response actually entered detailById. Refs #2640 --- .../taskdeck-web/src/store/captureStore.ts | 10 +++- .../src/tests/store/captureStore.spec.ts | 56 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index 2049addba..ca5b33e71 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -652,6 +652,13 @@ export const useCaptureStore = defineStore('capture', () => { await Promise.all( stale.map(async (id) => { try { + // `onRefreshed` is the caller's record that this id was RECONCILED, + // and for a tracked item with no summary row it is the whole of the + // batch poll's completion evidence. `fetchDetail` resolves on its + // drop paths too, so firing on resolution alone claimed a + // reconciliation that never reached `detailById` and let the poll + // complete on pre-batch state (#2640). + let cached = false await fetchDetail(id, { forceRefresh: true, showToast: false, @@ -682,8 +689,9 @@ export const useCaptureStore = defineStore('capture', () => { trackLoading: false, requestOptions: options.requestOptions, shouldCache: isCurrent, + onCacheOutcome: (didCache) => { cached = didCache }, }) - if (isCurrent()) options.onRefreshed?.(id) + if (cached && isCurrent()) options.onRefreshed?.(id) } catch { // A later poll tick retries transient detail failures. } diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index b5c4da84a..3db03e753 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2786,6 +2786,62 @@ describe('captureStore', () => { }, ) + /** + * `refreshedDetailIds` is the poll's record of ids it RECONCILED, and for a + * tracked item with no summary row it is the whole of `isComplete`'s and + * `isObservedTerminal`'s evidence. `onRefreshed` used to fire on + * `fetchDetail` RESOLVING, which a dropped read also does, so the poll + * could complete and move the badge on pre-batch detail state (#2640). + * + * Isolated on the write-generation drop path on purpose: with no summary + * row, `keepItem` records its write without moving the list write + * generation, so the tick's own `isCurrent()` stays true and the only + * thing rejecting the read is `fetchDetail`'s own compare. + */ + it('does not claim a reconciliation for a detail read the store dropped', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + // Beyond the newest-first list cap, so the detail is the only surface + // this poll can complete on. It holds pre-batch state: the Failed + // outcome the capture was re-triaged for. + store.items = [] + store.detailById['c-1'] = detailFor('c-1', 'Failed') + vi.mocked(captureApi.listItems).mockResolvedValue([] as never) + + let resolveReconcile!: (value: unknown) => void + vi.mocked(captureApi.getItem).mockReturnValueOnce( + new Promise((resolve) => { resolveReconcile = resolve }) as never, + ) + + const stop = store.pollBatchTriageCompletion(['c-1'], { limit: 200 }) + await vi.advanceTimersByTimeAsync(3_000) + expect(captureApi.getItem).toHaveBeenCalledTimes(1) + + // The user keeps the capture while that reconciliation read is in + // flight. Keep leaves the triage status alone. + vi.mocked(captureApi.keepItem).mockResolvedValue(detailFor('c-1', 'Failed')) + await store.keepItem('c-1') + const countRefreshesAfterWrite = workspaceMocks.refreshWorkloadCounts.mock.calls.length + + resolveReconcile(detailFor('c-1', 'ProposalCreated')) + await vi.advanceTimersByTimeAsync(0) + + // The response never entered the caches, so the reconciled body is not + // what the store holds. + expect(store.detailById['c-1']?.status).toBe('Failed') + // Nothing was reconciled, so the poll must neither complete nor move + // the badge on the strength of a reconciliation that did not happen. + expect(workspaceMocks.refreshWorkloadCounts) + .toHaveBeenCalledTimes(countRefreshesAfterWrite) + await vi.advanceTimersByTimeAsync(3_000) + expect(vi.mocked(captureApi.listItems).mock.calls.length).toBeGreaterThan(1) + stop() + } finally { + vi.useRealTimers() + } + }) + describe('resetForLogout', () => { it('clears the per-item summary generation guard', async () => { vi.useFakeTimers() From e2425f3af57da191f3bcab51536f497e23b07071 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 07:33:14 +0100 Subject: [PATCH 4/6] fix(inbox): drop a background list snapshot the logout crossed The session epoch covered the detail reads but not the list path, so the map clear inverted the summary half of the #2301 guard exactly as it had inverted the detail half. applyBackgroundListSnapshot keeps a locally newer row only while latestSummaryGenerationById outranks the snapshot's observed generation, and resetForLogout clears that map: the read comes back 0, so an older in-flight snapshot outranked a row a newer detail read had moved to a terminal status and pushed it back to Triaging. The batch poll's tick now captures the epoch when it issues its request and carries it in isCurrent(), which already gates both the snapshot and the detail reconciliation that follows it. The two halves of the guard now hold one post-logout contract: a response issued before the reset is dropped, a response issued after it caches normally. The spec that asserted the regressed behaviour as intended is retargeted, and the other half of the contract is pinned alongside it. Refs #2640 --- .../taskdeck-web/src/store/captureStore.ts | 23 +++++++++-- .../src/tests/store/captureStore.spec.ts | 40 ++++++++++++++++++- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index ca5b33e71..abf2e5e99 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -145,9 +145,12 @@ 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. + // Bumped by `resetForLogout`. Every read that is compared against the + // generations above captures it when its request is issued — the detail reads + // through `fetchDetail`, the single-item triage poll, and the background list + // snapshot in `pollBatchTriageCompletion` (#2640) — so a response 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) @@ -206,6 +209,11 @@ export const useCaptureStore = defineStore('capture', () => { * authority for membership and order — the list still owns scope and the * newest-first cap — and only a row whose own summary is newer than this * read keeps its local value. + * + * This compare is only meaningful WITHIN one session. `resetForLogout` clears + * `latestSummaryGenerationById`, which puts the read below back at 0 for + * every row, so the caller drops a snapshot the reset crossed before calling + * here at all (#2640) rather than letting the cleared map invert the guard. */ function applyBackgroundListSnapshot( loadedItems: CaptureItemSummary[], @@ -825,13 +833,22 @@ export const useCaptureStore = defineStore('capture', () => { const observedListLoadRequestId = latestListLoadRequestId const observedListWriteGeneration = latestListWriteGeneration const observedSummaryGeneration = nextCaptureGeneration + const observedSessionEpoch = sessionEpoch const controller = new AbortController() activeRequest = controller const requestOptions = { signal: controller.signal, skipRetry: true } + // The epoch is checked first, for the same reason the detail path checks + // it first (#2640). `applyBackgroundListSnapshot` keeps a locally newer + // row only while `latestSummaryGenerationById` outranks this read's + // observed generation, and the reset CLEARS that map: the read comes back + // 0, so an older in-flight snapshot would outrank a row a newer read had + // moved and regress it. Dropping a snapshot the reset crossed leaves both + // halves of the guard with one post-logout contract. const isCurrent = () => !stopped && !controller.signal.aborted && activeRequest === controller && + observedSessionEpoch === sessionEpoch && observedListLoadRequestId === latestListLoadRequestId && observedListWriteGeneration === latestListWriteGeneration diff --git a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts index 3db03e753..2c5f071bb 100644 --- a/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts +++ b/frontend/taskdeck-web/src/tests/store/captureStore.spec.ts @@ -2843,7 +2843,18 @@ describe('captureStore', () => { }) describe('resetForLogout', () => { - it('clears the per-item summary generation guard', async () => { + /** + * Both halves of the #2301 guard now carry ONE post-logout contract: a + * response issued before the reset is dropped, a response issued after it + * caches normally (#2640). + * + * This case used to assert the opposite for the summary half. Clearing + * `latestSummaryGenerationById` puts `applyBackgroundListSnapshot`'s read + * back at 0, so an older in-flight snapshot outranked a row a newer read + * had moved and pushed it back to Triaging — the same inversion the epoch + * was added to stop on the detail half. + */ + it('drops a background list snapshot issued before the logout', async () => { vi.useFakeTimers() try { const store = useCaptureStore() @@ -2870,7 +2881,32 @@ describe('captureStore', () => { await Promise.resolve() await Promise.resolve() - // The entry is gone, so nothing stale-high is left to pin the row. + // The snapshot was issued in the epoch the reset ended, so it is not + // applied at all and the row it would have regressed stands. + expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('Failed') + stop() + } finally { + vi.useRealTimers() + } + }) + + it('applies a background list snapshot issued after the logout', async () => { + vi.useFakeTimers() + try { + const store = useCaptureStore() + store.items = [summaryRow('c-1', 'Failed')] + store.resetForLogout() + + vi.mocked(captureApi.listItems).mockResolvedValue([ + summaryRow('c-1', 'Triaging'), + ] as never) + const stop = store.pollBatchTriageCompletion(['c-1'], { limit: 200 }) + await vi.advanceTimersByTimeAsync(3_000) + + // The epoch drops what a reset CROSSED, not everything after it. A + // request issued in the current epoch is current, and the snapshot is + // still the authority for membership, order and status. + expect(captureApi.listItems).toHaveBeenCalledTimes(1) expect(store.items.find((item) => item.id === 'c-1')?.status).toBe('Triaging') stop() } finally { From c295e1a02fd571b57d89b7563b802c02851035f2 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 07:33:39 +0100 Subject: [PATCH 5/6] docs(inbox): state exactly which reads capture the session epoch The resetForLogout docstring said every detail read captures the epoch, which peekDetail does not. peekDetail writes neither cache, so it is compared against no generation; the detailById write on that path belongs to useInboxOrchestrator.openBoardScopedHashItem, which passes the body to selectItemById as preloadedDetail with cacheSummary false and reaches cacheDetail guarded only by the route-hash re-check. The wording is qualified rather than the capture added: capturing the epoch inside peekDetail would not cover a write that is not peekDetail's to drop, and the guard would have to move into a per-mount composable the logout's route change tears down. cacheSummary false already keeps that path out of items, so no row of a previous session's list can survive it. Refs #2640 --- .../taskdeck-web/src/store/captureStore.ts | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index abf2e5e99..4c75dbc46 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -1027,10 +1027,26 @@ export const useCaptureStore = defineStore('capture', () => { * 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. + * over the newer one and its status pushed back into the list. Clearing + * `latestSummaryGenerationById` inverts the summary half the same way, which + * is why the background list snapshot is guarded too (#2640). So the epoch + * moves here, after the clear: every read that is compared against these + * generations captures it when it issues its request and drops its response + * when it moved, before any generation compare. The shared clock itself stays + * monotonic. + * + * SCOPE, stated exactly: the reads that capture the epoch are `fetchDetail`, + * the single-item triage poll, and the batch poll's list snapshot. NOT + * `peekDetail` — it is compared against no generation because it writes + * neither cache. The `detailById` write on that path is the caller's: + * `useInboxOrchestrator.openBoardScopedHashItem` hands the body to + * `selectItemById` as `preloadedDetail` with `cacheSummary: false`, which + * reaches `cacheDetail` guarded only by the route-hash re-check. Capturing + * the epoch inside `peekDetail` would not cover that write, since the write + * is not `peekDetail`'s to drop; the guard would have to live in the + * composable, a per-mount surface the logout's route change tears down + * anyway. What the `cacheSummary: false` buys is that nothing from that path + * can reach `items`, so no row of a previous session's list survives it. */ function resetForLogout() { latestDetailWriteGenerationById.clear() From d230b99892502a9efacff789aa599cb8a1dc24d2 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 07:53:17 +0100 Subject: [PATCH 6/6] fix(inbox): report the drop reason and re-read a live generation collision Round-2 review, two MEDIUMs. The restore-and-return-false branch was reachable in a live session, not only after a logout. A first open observes write generation 0 for an uncached item; a batch triage that includes it moves that generation, and refreshTerminalDetails skips the item because it has a list row and no cached detail. The read resolved as dropped with detailById still empty and no body cached anywhere, so openItemFromHash stripped the user's deep-link hash and openItemFromList turned the click into a silent no-op that a second click undid. onCacheOutcome now reports which guard rejected the response, not just that one did: cached, superseded, generation, epoch. selectItemById re-reads once on a generation drop that left no detail, keeps the selection on superseded, and restores only on epoch. The additive shape and the unreporting-store-means-cached default are unchanged. The restore now returns activeItemIndex alongside selectedItemId. They drive different things - the active row and aria-activedescendant versus aria-selected and the panel - so restoring one alone desynchronised the list from the panel. peekDetail carries a line saying it accepts onCacheOutcome and never calls it, since it writes neither cache. The orchestrator spec resets fetchDetail per test: an unconsumed mockImplementationOnce survives vi.clearAllMocks() and leaked a queued re-read into the next test. Refs #2640 --- .../src/composables/useInboxOrchestrator.ts | 63 +++++++---- .../taskdeck-web/src/store/captureStore.ts | 79 ++++++++----- .../composables/useInboxOrchestrator.spec.ts | 105 ++++++++++++++++-- 3 files changed, 191 insertions(+), 56 deletions(-) diff --git a/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts b/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts index e0a2e8ea6..503522ce4 100644 --- a/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts +++ b/frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts @@ -1,6 +1,7 @@ import { computed, nextTick, onMounted, onUnmounted, ref, watch } from 'vue' import { useRoute, useRouter } from 'vue-router' import { useCaptureStore } from '../store/captureStore' +import type { DetailCacheOutcome } from '../store/captureStore' import { boardsApi } from '../api/boardsApi' import { isTriageTerminalStatus } from '../types/capture' import type { CaptureItem, CaptureItemSummary, CaptureListQuery } from '../types/capture' @@ -298,7 +299,12 @@ export function useInboxOrchestrator(options: { // only the list write is suppressed. Paper loads through `peekDetail` and is // unaffected either way. const syncSummary = cacheSummary && !isArchivedHistory.value + // Both halves of the selection. `selectedItemId` drives `aria-selected` and + // the detail panel; `activeItemIndex` drives the active row and + // `aria-activedescendant`. Restoring one without the other would leave the + // list pointing at a row the panel is not showing. const previousSelectedItemId = selectedItemId.value + const previousActiveItemIndex = activeItemIndex.value primeSelection(itemId, preferredIndex) hashLoadFailedItemId.value = null try { @@ -306,29 +312,44 @@ export function useInboxOrchestrator(options: { captureStore.cacheDetail(preloadedDetail, syncSummary) return true } - // `onCacheOutcome` is the store reporting that THIS read reached its - // caches (#2640). `fetchDetail` resolves with the body it fetched on - // three paths that write nothing — a `shouldCache` opt-out, a session - // epoch the logout moved, a write generation a newer mutation moved — so - // resolution alone is not evidence the panel has a detail to render. - // Returning `true` there left `selectedItemId` on an id `detailById` - // holds nothing for, which `InboxDetailPanel` renders as "Unable to load - // capture detail." for a read that succeeded. A store that reports - // nothing is read as having cached, which is the behaviour before #2640. - let cached = true - await captureStore.fetchDetail(itemId, { - syncSummary, - onCacheOutcome: (didCache) => { cached = didCache }, - }) - // A drop can still leave a detail here: a mutation that superseded this - // read cached its own newer body under the same id. The panel is honest - // then and the selection stands. Only a drop that left the store with - // nothing for this id is a selection this surface cannot show, and that - // is not a failure either — so restore the selection the user had rather - // than clearing it, and report the read as not opened. - if (!cached && !captureStore.detailById[itemId]) { + // `onCacheOutcome` is the store reporting what it did with THIS read + // (#2640). `fetchDetail` resolves with the body it fetched on three paths + // that write nothing, so resolution alone is not evidence the panel has a + // detail to render: returning `true` regardless left `selectedItemId` on + // an id `detailById` holds nothing for, which `InboxDetailPanel` renders + // as "Unable to load capture detail." for a read that succeeded. A store + // that reports nothing is read as `cached`, the behaviour before #2640. + // A holder, not a bare `let`: TypeScript's control-flow analysis does not + // track an assignment made inside the callback and would narrow a plain + // variable to its initializer for every compare below. + const detailRead: { outcome: DetailCacheOutcome } = { outcome: 'cached' } + const report = (reported: DetailCacheOutcome) => { detailRead.outcome = reported } + await captureStore.fetchDetail(itemId, { syncSummary, onCacheOutcome: report }) + + // A `generation` drop is a LIVE-session case, not a post-logout one: a + // first open observes generation 0 for an uncached item, and a batch + // triage that includes it moves that generation while the read is in + // flight. `refreshTerminalDetails` skips such an item — it has a list row + // and no cached detail — so unlike a detail-path write, the batch write + // has cached no body of its own and there is nothing here to render. Not + // re-reading turned the click into a silent no-op and stripped a live + // deep link. Re-read ONCE, against the generation that caused the drop; + // a second collision in that window is left to the user's next click + // rather than looped over. + if (detailRead.outcome === 'generation' && !captureStore.detailById[itemId]) { + detailRead.outcome = 'cached' + await captureStore.fetchDetail(itemId, { syncSummary, onCacheOutcome: report }) + } + + // `superseded` means a newer read for the same id is already the + // authority, so the selection stands. Only the logout case is terminal: + // there is no newer read coming and no body to show. It is not a failure + // either, so restore the selection the user had — both halves of it — + // rather than clearing it, and report the read as not opened. + if (detailRead.outcome === 'epoch' && !captureStore.detailById[itemId]) { if (selectedItemId.value === itemId) { selectedItemId.value = previousSelectedItemId + activeItemIndex.value = previousActiveItemIndex } return false } diff --git a/frontend/taskdeck-web/src/store/captureStore.ts b/frontend/taskdeck-web/src/store/captureStore.ts index 4c75dbc46..ca0382857 100644 --- a/frontend/taskdeck-web/src/store/captureStore.ts +++ b/frontend/taskdeck-web/src/store/captureStore.ts @@ -24,6 +24,13 @@ function toSummary(item: CaptureItem): CaptureItemSummary { } } +/** + * What `fetchDetail` did with a response it resolved with (#2640). `cached` is + * the only value that means `detailById` holds this item's detail; the other + * three name which guard rejected the response. See `onCacheOutcome`. + */ +export type DetailCacheOutcome = 'cached' | 'superseded' | 'generation' | 'epoch' + type DetailLoadOptions = { forceRefresh?: boolean recordError?: boolean @@ -42,31 +49,44 @@ type DetailLoadOptions = { */ trackLoading?: boolean /** - * REPORT whether this call left `detailById` holding this item's detail - * (#2640) — the detail-read counterpart of `fetchItems`'s applied boolean. + * REPORT what this call did with its response (#2640) — the detail-read + * counterpart of `fetchItems`'s applied boolean. * * `fetchDetail` has three paths that resolve with the body they fetched and - * write nothing: the caller's own `shouldCache` opt-out, a session epoch the - * logout moved, and a write generation a newer mutation moved. Resolution - * alone therefore never meant "the store now holds this detail", and callers - * that assumed it did acted on a response the store had dropped: - * `useInboxOrchestrator.selectItemById` left `selectedItemId` pointing at an - * id with no detail, which the Legacy panel renders as "Unable to load - * capture detail." for a read that succeeded, and `refreshTerminalDetails` - * announced a reconciliation that never reached `detailById`. + * write nothing, and resolution alone therefore never meant "the store now + * holds this detail". Callers that assumed it did acted on a response the + * store had dropped: `useInboxOrchestrator.selectItemById` left + * `selectedItemId` pointing at an id with no detail, which the Legacy panel + * renders as "Unable to load capture detail." for a read that succeeded, and + * `refreshTerminalDetails` announced a reconciliation that never reached + * `detailById`. + * + * The REASON is reported, not just the fact, because the three drops call for + * different handling and only one of them is post-logout-only: + * + * - `cached` — the response is in `detailById` now. Also covers the two paths + * that resolve from state already present, the non-forced cached early + * return and the demo branch: the question a caller asks is whether the + * store holds this detail, not whether this call did the writing. + * - `superseded` — the caller's own `shouldCache` said no, so a newer read + * for the same id is already the authority. + * - `generation` — a successful write for this id landed mid-read. Reachable + * in a LIVE session, not only after a logout: a first open observes + * generation 0 for an uncached item and a batch triage that includes it + * moves that generation. The write has cached its own newer body only when + * it went through the detail path; a batch write has not, so a caller that + * needs a body must re-read rather than treat this as final. + * - `epoch` — `resetForLogout` moved the session epoch under this read. * - * `true` also covers the two paths that resolve from state already present — - * the non-forced cached early return and the demo branch — because the - * question a caller asks is whether the store holds this detail now, not - * whether this particular call did the writing. A read that THROWS reports - * nothing: failure and a drop stay distinguishable. + * A read that THROWS reports nothing: failure and a drop stay + * distinguishable. * * A callback rather than a return value because `fetchDetail` resolves with * the `CaptureItem` itself and a view annotates that type * (`views/paper/inbox/PaperTriageRowEdit.vue`), so widening the return would * change a surface outside this seam. */ - onCacheOutcome?: (cached: boolean) => void + onCacheOutcome?: (outcome: DetailCacheOutcome) => void } export const BATCH_TRIAGE_POLL_INTERVAL_MS = 3_000 @@ -298,7 +318,7 @@ export const useCaptureStore = defineStore('capture', () => { } = options if (!forceRefresh && detailById.value[itemId]) { - onCacheOutcome?.(true) + onCacheOutcome?.('cached') return detailById.value[itemId] } @@ -307,7 +327,7 @@ export const useCaptureStore = defineStore('capture', () => { if (summary) { const detail = { ...summary, rawText: summary.textExcerpt, retryCount: 0, provenance: null } cacheDetail(detail, syncSummary) - onCacheOutcome?.(true) + onCacheOutcome?.('cached') return detail } } @@ -325,17 +345,21 @@ export const useCaptureStore = defineStore('capture', () => { ? await captureApi.getItem(itemId, requestOptions) : await captureApi.getItem(itemId) // The three drop paths, in the order they have always been checked and - // reported as one boolean (#2640). The epoch comes before any generation + // now named for the caller (#2640). The epoch comes before any generation // compare: the reset discards the generations this read observed, so // after a logout the compare is no longer meaningful. - const cached = - shouldCache() && - observedSessionEpoch === sessionEpoch && - observedDetailWriteGeneration === detailWriteGeneration(itemId) - if (cached) { + let outcome: DetailCacheOutcome = 'cached' + if (!shouldCache()) { + outcome = 'superseded' + } else if (observedSessionEpoch !== sessionEpoch) { + outcome = 'epoch' + } else if (observedDetailWriteGeneration !== detailWriteGeneration(itemId)) { + outcome = 'generation' + } + if (outcome === 'cached') { cacheDetail(detail, syncSummary) } - onCacheOutcome?.(cached) + onCacheOutcome?.(outcome) return detail } catch (e: unknown) { const message = getErrorDisplay(e, 'Failed to load inbox item').message @@ -354,6 +378,9 @@ export const useCaptureStore = defineStore('capture', () => { } async function peekDetail(itemId: string, options: DetailLoadOptions = {}) { + // Shares `DetailLoadOptions` and therefore ACCEPTS `onCacheOutcome`, but + // never calls it: this read writes neither cache, so it has no cache + // outcome to report (#2640). const { forceRefresh = false, recordError = true, @@ -697,7 +724,7 @@ export const useCaptureStore = defineStore('capture', () => { trackLoading: false, requestOptions: options.requestOptions, shouldCache: isCurrent, - onCacheOutcome: (didCache) => { cached = didCache }, + onCacheOutcome: (outcome) => { cached = outcome === 'cached' }, }) if (cached && isCurrent()) options.onRefreshed?.(id) } catch { diff --git a/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts b/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts index fea6f2619..78e6b330b 100644 --- a/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts +++ b/frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts @@ -70,6 +70,7 @@ vi.mock('../../utils/navigation', () => ({ })) import { useInboxOrchestrator } from '../../composables/useInboxOrchestrator' +import type { DetailCacheOutcome } from '../../store/captureStore' function createOrchestrator() { mountedCallback = null @@ -169,6 +170,11 @@ describe('useInboxOrchestrator', () => { ], }) mockCaptureStore.pollBatchTriageCompletion.mockReset().mockReturnValue(vi.fn()) + // `vi.clearAllMocks()` clears recorded calls but keeps implementations, + // including UNCONSUMED `mockImplementationOnce` entries. A case that queues + // a re-read the code under test does not take would hand that queued + // implementation to the next test. + mockCaptureStore.fetchDetail.mockReset() // The store reports whether it APPLIED the response (#2501). The default is // the ordinary case: the response was the latest and was written. mockCaptureStore.fetchItems.mockReset().mockResolvedValue(true) @@ -1050,15 +1056,15 @@ describe('useInboxOrchestrator', () => { describe('dropped detail reads', () => { const keptDetail = { id: 'kept', rawText: 'kept body', boardId: null, status: 'New' } - it('holds the previous selection when the store reports a dropped read', async () => { + it('holds the previous selection and active row when a logout dropped the read', async () => { mockCaptureStore.items = [{ id: 'kept' }, { id: 'dropped' }] mockCaptureStore.detailById = { kept: keptDetail } const orch = createOrchestrator() mockCaptureStore.fetchDetail.mockImplementationOnce(async ( _itemId: string, - options?: { onCacheOutcome?: (cached: boolean) => void }, - ) => { options?.onCacheOutcome?.(true) }) + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, + ) => { options?.onCacheOutcome?.('cached') }) await orch.openItemFromList(summaryRow('kept', null), 0) expect(orch.selectedItemId.value).toBe('kept') @@ -1066,12 +1072,17 @@ describe('useInboxOrchestrator', () => { // put a detail for this id in `detailById` either. mockCaptureStore.fetchDetail.mockImplementationOnce(async ( _itemId: string, - options?: { onCacheOutcome?: (cached: boolean) => void }, - ) => { options?.onCacheOutcome?.(false) }) + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, + ) => { options?.onCacheOutcome?.('epoch') }) await orch.openItemFromList(summaryRow('dropped', null), 1) expect(orch.selectedItemId.value).toBe('kept') expect(orch.selectedItem.value).toEqual(keptDetail) + // `activeItemIndex` drives the active row and `aria-activedescendant` + // while `selectedItemId` drives `aria-selected` and the panel. Restoring + // one without the other leaves the list pointing at a row the panel is + // not showing, so both come back. + expect(orch.activeItemIndex.value).toBe(0) }) it('keeps the selection when a dropped read left a newer detail cached', async () => { @@ -1084,12 +1095,12 @@ describe('useInboxOrchestrator', () => { // has a detail to render and the selection is honest. mockCaptureStore.fetchDetail.mockImplementationOnce(async ( itemId: string, - options?: { onCacheOutcome?: (cached: boolean) => void }, + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, ) => { mockCaptureStore.detailById[itemId] = { id: itemId, rawText: 'newer body', boardId: null, status: 'ProposalCreated', } - options?.onCacheOutcome?.(false) + options?.onCacheOutcome?.('generation') }) await orch.openItemFromList(summaryRow('superseded', null), 0) @@ -1099,6 +1110,82 @@ describe('useInboxOrchestrator', () => { }) }) + /** + * The write-generation drop is NOT post-logout-only. A first open observes + * generation 0 for an uncached item; a batch triage that includes it moves + * that generation through `recordCaptureWrite`, and + * `refreshTerminalDetails` skips the item because it has a list row and no + * cached detail. So the read is dropped with `detailById` still empty and + * NOTHING has cached a body for the id — restoring the selection there + * turns a live click into a silent no-op and strips a live deep link. + */ + const reReadDetail = { + id: 'crossed', rawText: 'reconciled body', boardId: null, status: 'ProposalCreated', + } + + function mockBatchCrossedThenCachingReRead() { + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + _itemId: string, + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, + ) => { options?.onCacheOutcome?.('generation') }) + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + itemId: string, + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, + ) => { + mockCaptureStore.detailById[itemId] = { ...reReadDetail, id: itemId } + options?.onCacheOutcome?.('cached') + }) + } + + it('re-reads once and keeps the deep-link hash when a batch write crossed the read', async () => { + mockRoute.hash = '#capture-crossed' + mockCaptureStore.items = [{ id: 'crossed' }] + mockCaptureStore.detailById = {} + const orch = createOrchestrator() + mockBatchCrossedThenCachingReRead() + + await orch.loadInbox() + + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledTimes(2) + expect(orch.selectedItemId.value).toBe('crossed') + expect(orch.selectedItem.value).toEqual(reReadDetail) + // The hash still names a capture the user can see, so nothing clears it. + expect(mockRouter.replace).not.toHaveBeenCalled() + }) + + it('re-reads once so a click still opens an item a batch write crossed', async () => { + mockCaptureStore.items = [{ id: 'crossed' }] + mockCaptureStore.detailById = {} + const orch = createOrchestrator() + mockBatchCrossedThenCachingReRead() + + await orch.openItemFromList(summaryRow('crossed', null), 0) + + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledTimes(2) + expect(orch.selectedItemId.value).toBe('crossed') + expect(orch.activeItemIndex.value).toBe(0) + expect(orch.selectedItem.value).toEqual(reReadDetail) + }) + + it('does not re-read when the crossing write already cached a body', async () => { + mockCaptureStore.items = [{ id: 'crossed' }] + mockCaptureStore.detailById = {} + const orch = createOrchestrator() + + mockCaptureStore.fetchDetail.mockImplementationOnce(async ( + itemId: string, + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, + ) => { + mockCaptureStore.detailById[itemId] = { ...reReadDetail, id: itemId } + options?.onCacheOutcome?.('generation') + }) + + await orch.openItemFromList(summaryRow('crossed', null), 0) + + expect(mockCaptureStore.fetchDetail).toHaveBeenCalledTimes(1) + expect(orch.selectedItemId.value).toBe('crossed') + }) + it('still treats a rejected detail read as a failure', async () => { mockCaptureStore.items = [{ id: 'kept' }, { id: 'broken' }] mockCaptureStore.detailById = { kept: keptDetail } @@ -1106,8 +1193,8 @@ describe('useInboxOrchestrator', () => { mockCaptureStore.fetchDetail.mockImplementationOnce(async ( _itemId: string, - options?: { onCacheOutcome?: (cached: boolean) => void }, - ) => { options?.onCacheOutcome?.(true) }) + options?: { onCacheOutcome?: (outcome: DetailCacheOutcome) => void }, + ) => { options?.onCacheOutcome?.('cached') }) await orch.openItemFromList(summaryRow('kept', null), 0) mockCaptureStore.fetchDetail.mockRejectedValueOnce(new Error('detail unavailable'))