fix(inbox): drop pre-logout list snapshots by session epoch and report dropped detail reads

[Chris0Jeky]

Summary

Five follow-ups from the read-only verification pass on PR #2631's round-2 fixes, all edges of the session epoch that #2631 introduced.

The session epoch covered the detail reads but not the list path, so resetForLogout() clearing latestSummaryGenerationById inverted the summary half of the #2301 guard exactly as it had inverted the detail half. The batch poll's tick now captures the epoch when it issues its request. Alongside that, fetchDetail now reports which guard rejected a response, so neither useInboxOrchestrator.selectItemById nor the batch poll's refreshedDetailIds can claim a read succeeded when the store dropped it — and a drop that is recoverable is recovered rather than surfaced.

Closes #2640
Refs #2571, PR #2631

Changes

One commit per issue item, plus one round-2 commit.

8f3bc0ca5 item 5 — bound the never-resolving detail mock to its own test. captureStore.spec.ts's 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. Three mockImplementationOnce calls now cover its three reads exactly. A guard test directly below races the surviving implementation against a macrotask: a leaked mockResolvedValue from any earlier test settles and is harmless, a leaked never-resolving one does not.

dc6abc8c4 item 3 — report a dropped detail read and stop selecting on it. fetchDetail has three paths that resolve with the body they fetched and write nothing: 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 — the state InboxDetailPanel renders as "Unable to load capture detail.", for a read that succeeded. Reported through a new DetailLoadOptions.onCacheOutcome.

The report is a callback, not a return value or an outcome object. fetchDetail resolves with the CaptureItem and views/paper/inbox/PaperTriageRowEdit.vue:190 annotates that type (const detail: CaptureItem = await captureStore.fetchDetail(...)), so widening the return would have changed a view — outside this issue's scope. A sibling outcome-returning function would have moved selectItemById off fetchDetail and silently un-configured the 25-plus fetchDetail mock implementations in InboxView.spec.ts. The callback is additive, every existing caller compiles untouched, and a store that reports nothing is read as having cached, which is the pre-change behaviour.

d85a7680a item 2 — 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.

e2425f3af item 1 — drop a background list snapshot the logout crossed. pollBatchTriageCompletion's tick() captures sessionEpoch when it issues its list request and carries it in isCurrent(), which already gates both applyBackgroundListSnapshot and the detail reconciliation that follows it. The spec that asserted the regressed behaviour as intended ("nothing stale-high is left to pin the row") is retargeted, and the other half of the contract is pinned beside it.

c295e1a02 item 4 — state exactly which reads capture the session epoch. The resetForLogout docstring said every detail read captures the epoch, which peekDetail does not. Wording qualified rather than the capture added; reasoning in Boundaries below.

d230b9989 round 2 — report the drop REASON and re-read a live generation collision. Two MEDIUMs from review, in one commit.

MEDIUM 1. The restore-and-return-false branch was reachable in a live session, not only post-logout. A first open observes write generation 0 for an uncached item; a batch triage that includes the item moves that generation through recordCaptureWrite, and refreshTerminalDetails skips the item because it has a list row and no cached detail. The read therefore resolved as dropped with detailById still empty and no body cached anywhere — unlike a detail-path write, which caches its own newer body. Round 1 restored the selection there, so openItemFromHash silently stripped the user's deep-link hash and openItemFromList turned the click into a no-op that a second click undid.

onCacheOutcome now carries the reason, exported as DetailCacheOutcome = 'cached' | 'superseded' | 'generation' | 'epoch' — a plain string union, not an object, keeping the callback shape additive and the unreporting-store-means-cached default intact. selectItemById re-reads once on a generation drop that left no detail for the id, keeps the selection on superseded (a newer read for the same id is already the authority), and restores-and-returns-false only on epoch. A second collision inside that window is left to the user's next click rather than looped over.

MEDIUM 2. The restore set selectedItemId but not activeItemIndex. 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. Both are captured before primeSelection and restored together.

Also: peekDetail carries one line saying it accepts onCacheOutcome and never calls it, because it writes neither cache. And useInboxOrchestrator.spec.ts now resets fetchDetail per test — an unconsumed mockImplementationOnce survives vi.clearAllMocks(), and the new re-read cases queue one, which leaked into the next test (it surfaced as a real red during this round).

Test plan

All commands run from frontend/taskdeck-web in the issue worktree.

Round-1 red-first, recorded before each fix:

• item 5 — with the persistent mockImplementation restored: AssertionError: expected 'hangs' to be 'settles'
• item 3 — AssertionError: expected 'dropped' to be 'kept'
• item 2 — AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times
• item 1 — AssertionError: expected 'Triaging' to be 'Failed'

Round-2 red-first, all four before the fix:

• holds the previous selection and active row when a logout dropped the read — AssertionError: expected 'dropped' to be 'kept'
• re-reads once and keeps the deep-link hash when a batch write crossed the read — AssertionError: expected "vi.fn()" to be called 2 times, but got 1 times
• re-reads once so a click still opens an item a batch write crossed — AssertionError: expected "vi.fn()" to be called 2 times, but got 1 times
• still treats a rejected detail read as a failure — AssertionError: expected 'broken' to be null (the leaked unconsumed mockImplementationOnce, fixed by the per-test reset)

Verified, green at head d230b9989:

• npx vitest --run --maxWorkers=2 src/tests/store/captureStore.spec.ts src/tests/composables/useInboxOrchestrator.spec.ts src/tests/views/InboxView.spec.ts src/tests/components/AppShell.spec.ts src/tests/components/AppShell.paperVariant.spec.ts — 5 files, 288 tests passed
• npx vitest --run --maxWorkers=2 src/tests/store src/tests/composables/useInboxOrchestrator.spec.ts src/tests/views/InboxView.spec.ts src/tests/views/InboxView.paperMode.spec.ts src/tests/views/paper/inbox src/tests/resilience src/tests/property/storeResilience.spec.ts — 61 files, 1181 tests passed
• npm run typecheck — clean
• npx eslint src/store/captureStore.ts src/composables/useInboxOrchestrator.ts src/tests/store/captureStore.spec.ts src/tests/composables/useInboxOrchestrator.spec.ts src/tests/views/InboxView.spec.ts — clean
• npm run build — built, PWA precache 142 entries
• git diff --check — clean

NOT verified:

• Playwright E2E. Not run; no E2E covers a logout or a batch write crossing an in-flight Inbox read.
• The rendered Legacy detail panel and list in a real browser. The selectItemById changes are proven at the orchestrator level only.
• The full frontend vitest suite and the backend suite. Neither is touched; CI covers both.

Boundaries and risks

The post-logout contract, both halves of the #2301 guard. A response issued before resetForLogout() is dropped: the detail half by fetchDetail's epoch compare, the summary half by the batch poll tick's epoch in isCurrent(). A response issued after the reset caches normally — the epoch drops what a reset crossed, not everything that follows it. Both halves are pinned by tests in the resetForLogout describe.

What selectItemById does per drop reason, after round 2. cached opens. superseded keeps the selection — a newer read for the same id is already the authority. generation with no detail re-reads once and then behaves as that re-read reports; with a detail present it opens, because the crossing write cached its own newer body. epoch with no detail restores the pre-call selectedItemId and activeItemIndex and returns false. A rejected read still clears the selection outright, as before. The drop-and-restore path is therefore reachable only for the epoch case after this change — every live-session drop either opens or recovers by re-reading. The invariant throughout: selectedItemId is never left pointing at an id detailById holds nothing for.

Residual LOW, accepted. A caller-supplied onCacheOutcome that throws does so inside fetchDetail's try, so it would be reported to the user as a fetch failure. Both in-repo callers pass a total one-line assignment, so this is unreachable today; closing it would mean wrapping the callback, which buys nothing for the two call sites that exist. peekDetail accepting and ignoring the option is now documented in one line at its head rather than left to be inferred.

Why peekDetail was not given the epoch capture. 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, reaching cacheDetail guarded only by the route-hash re-check. An epoch capture inside peekDetail would not cover a write that 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. cacheSummary: false keeps that path out of items, so no row of a previous session's list can survive it. That residual is now named in the resetForLogout docstring instead of being contradicted by it.

Untouched, as scoped. The bounded poll's other guards, the scoped refresh from #2614, the AppShell wiring from #2631, and every view. fetchItems's request-id supersession is unchanged: item 1 lives entirely in the background poll.

Spec-side churn. Nine toHaveBeenCalledWith assertions across useInboxOrchestrator.spec.ts and InboxView.spec.ts gained onCacheOutcome: expect.any(Function), because selectItemById's options object grew a key and those assertions are exact matches. No assertion was loosened to objectContaining.

Gitignored files in the worktree. Only frontend/taskdeck-web/dist/ and frontend/taskdeck-web/node_modules/, both regenerable. Nothing was copied out; nothing needs to survive the worktree's removal.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Review record (alpha product-trust lane, review-and-ship round 1 at head c295e1a).

Reviewer: one fresh-context independent reviewer subagent (read-only), input = merge-base..head diff plus the worktree at the head. Verdict: SHIP, no CRITICAL or HIGH; two MEDIUMs taken in round 2.

Confirmed clean: the session epoch increments exactly once in the tree, inside resetForLogout under the shell's unauthenticated transition, and the docstring's enumeration of the epoch-capturing reads is exhaustive (fetchItems is correctly excluded: its request-id supersession is not cleared by the reset); the batch poll neither dies at logout nor applies a stale-epoch snapshot (the tick's finally reschedules and the next tick re-captures); the currency check is strictly additive; the fetchDetail refactor preserves the drop order; onCacheOutcome fires exactly once on each caching exit and never on the throw path, with an unreporting store reading as cached so every untouched mock keeps passing; the cached-and-current gate in refreshTerminalDetails cannot stall a healthy poll; item 2's isolation is genuine (keepItem with no list row moves only the detail generation); the retargeted spec is red-first and both guard halves now agree; the settles-versus-hangs guard is not timing-sensitive (microtasks drain before any macrotask); the peekDetail docstring claim holds (the orchestrator re-checks the route hash before the preloaded cacheDetail); the return-type widening was correctly avoided (a Paper row editor annotates the awaited result as a CaptureItem); every exact fetchDetail assertion in the tree was updated; no view changed and the #2614 and #2631 seams are untouched.

Triage:

• MEDIUM, taken in round 2: the restore-and-return-false branch is reachable in a live session, not only post-logout: a first-open detail read crossed by a batch triage that includes the item (the write moves the generation; the terminal-detail reconciliation skips an item with no cached detail) resolves as not cached with an empty detail, so openItemFromHash strips the deep-link hash silently and openItemFromList turns the click into a silent no-op. Round 2 makes the store report the drop reason; a generation drop with no detail re-reads once, a superseded read keeps today's behaviour, and only the epoch case restores and returns false; specs for the hash and the click paths.
• MEDIUM, taken in round 2: the restore set selectedItemId but not activeItemIndex, desynchronising the list's active row from the selected row and the panel; both are restored through the same path and asserted.
• LOW, taken as one comment: onCacheOutcome is accepted and ignored by peekDetail (which caches nothing by design).
• LOW, recorded: a throwing onCacheOutcome on the network path reports as a fetch failure (no in-repo callback can throw).
• LOW: the STATUS line is the coordinator's (thirteenth block).

Merge gate: round-2 push, one verification pass scoped to the fix diff (the outcome reporting and the re-read are new logic), ci-required green at the round-2 head, aged three minutes, then merge commit.

[Chris0Jeky]

Round-2 verification record (scoped to the fix diff c295e1a..d230b99; read-only pass).

Verdict: SHIP for the fix diff, no CRITICAL or HIGH. Confirmed: exactly one re-read (a straight-line if, not a loop), with every outcome of the re-read handled and a throw landing in the existing catch that clears the selection; both reads pass identical options and loadingDetail is cleared in fetchDetail's finally on every path; the re-read writes only store caches and never touches the selection, so a user who clicked elsewhere keeps the newer selection; the outcome ordering preserves the caching decision exactly (cached if and only if shouldCache, epoch equal and generation equal, same short-circuit order); the outcome holder is a per-call local and the callback fires synchronously before fetchDetail resolves; the active-row restore is captured before primeSelection and restored only in the epoch branch under the same guard, and the re-read path cannot re-desync the two halves; refreshTerminalDetails stays fail-closed (#2640 item 2 holds); the peekDetail comment is accurate; the spec's mockReset cannot wipe a default other tests depend on and the three new specs construct the crossing correctly; the diff is three files with no view change and the #2614 and #2631 seams untouched.

Recorded, no third round (the two-round ceiling is reached; none is a merge blocker):

• MEDIUM, contract-doc accuracy on new exported surface: 'superseded' is tested first and its only real source is the batch poll's isCurrent, which is false for six reasons including a moved session epoch, so a terminal-detail read crossed by a logout reports 'superseded', not 'epoch', contradicting the DetailCacheOutcome docstring and the orchestrator comment. No live path today (selectItemById passes no shouldCache; refreshTerminalDetails collapses every non-cached value to false). The docstring is corrected by the next PR that touches captureStore, tracked on [Frontend][Inbox] Session-epoch follow-ups: list snapshot path, dropped-read reporting, peekDetail wording, spec mock hygiene #2640's closing thread.
• LOW: a second generation collision inside the re-read window returns true with an empty panel and a retained deep link (two writes for the same id inside one round trip; the retained selection and Refresh Detail remain available); explicitly chosen and documented in the code.
• LOW: the spec block's docstring about vi.clearAllMocks keeping implementations now contradicts the mockReset the same diff added; a comment fix for the next touch.

Merge gate unchanged: ci-required green at d230b99, aged, merge commit.