fix(inbox): repair a dropped scope-replacement load instead of latching it

frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts

@@ -27,6 +27,22 @@ export function useInboxOrchestrator(options: {
   let scopedBoardLoadGeneration = 0
   let latestInboxLoadRequestId = 0
   const isScopeReplacement = ref(false)
+  /**
+   * Which scope a raised `isScopeReplacement` belongs to, and whether that
+   * replacement has already spent its one repair load (#2591).
+   *
+   * Orchestrator bookkeeping rather than a store observable on purpose. The
+   * store keeps its list request id private and publishes only `items`,
+   * `loadingList` and `listError` — and `items` also moves on writes that
+   * prove nothing about the current scope (optimistic summary writes, the
+   * batch poll's reconciliation reader), so watching it would clear the flag
+   * without the new scope's rows ever having landed: the #2501 harm. The
+   * `applied` boolean from `fetchItems` stays the only evidence that a list
+   * response was written, and this record is what tells a LATER load that the
+   * response it is about to report on is the one an outstanding replacement
+   * was waiting for.
+   */
+  let scopeReplacementLatch: { scopeKey: string; repairIssued: boolean } | null = null
 
   // Batch selection state
   const selectedIds = ref<Set<string>>(new Set())
@@ -74,6 +90,18 @@ export function useInboxOrchestrator(options: {
     }
   }
 
+  /**
+   * The Inbox's current scope as a comparable key. Two list loads answer the
+   * same question only when their keys match; a load whose key no longer
+   * matches the current one must not touch scope-replacement state.
+   */
+  function currentScopeKey(): string {
+    return JSON.stringify({
+      boardId: activeBoardId.value,
+      archived: isArchivedHistory.value,
+    })
+  }
+
   const activeBoardName = computed(() => {
     const boardId = activeBoardId.value
     return boardId && scopedBoard.value?.id === boardId ? scopedBoard.value.name : boardId ?? ''
@@ -424,14 +452,15 @@ export function useInboxOrchestrator(options: {
 
   async function loadInboxInternal(scopeReplacement = false) {
     const requestId = ++latestInboxLoadRequestId
-    const requestScopeKey = JSON.stringify({
-      boardId: activeBoardId.value,
-      archived: isArchivedHistory.value,
-    })
+    const requestScopeKey = currentScopeKey()
     if (scopeReplacement) {
       isScopeReplacement.value = true
+      // A fresh replacement, and a fresh repair budget with it. The repair
+      // below is a PLAIN load, so it can never refill its own budget.
+      scopeReplacementLatch = { scopeKey: requestScopeKey, repairIssued: false }
     }
     inboxLoadPerf.start()
+    let repairDroppedReplacement = false
     try {
       // `applied` is the store reporting that THIS call's response was written
       // into `items` (#2501). `fetchItems` resolves without writing anything
@@ -442,18 +471,61 @@ export function useInboxOrchestrator(options: {
       // scope-key checks below stay: they guard against a stale caller, while
       // `applied` guards against a dropped response.
       const applied = await captureStore.fetchItems(currentListQuery())
-      const currentScopeKey = JSON.stringify({
-        boardId: activeBoardId.value,
-        archived: isArchivedHistory.value,
-      })
-      if (applied && requestId === latestInboxLoadRequestId && requestScopeKey === currentScopeKey) {
+      // Still the latest load, still answering the scope the user is looking
+      // at. A stale load reports on nothing.
+      const isLatestForThisScope =
+        requestId === latestInboxLoadRequestId && requestScopeKey === currentScopeKey()
+      if (applied && isLatestForThisScope) {
         isScopeReplacement.value = false
+        scopeReplacementLatch = null
+      } else if (
+        !applied &&
+        isLatestForThisScope &&
+        scopeReplacementLatch?.scopeKey === requestScopeKey
+      ) {
+        // A dropped response under an outstanding replacement for THIS scope
+        // (#2591). Nothing else will clear the flag: the read that superseded
+        // this one is the store's own post-batch refresh, whose `applied`
+        // result the store discards, and no later orchestrator load is coming
+        // (the id check above proved that). Left alone the flag latched
+        // forever, and `PaperTriageTable` then hid the rows AND their count
+        // with no Retry — an empty body under a count-free eyebrow.
+        //
+        // So: repair the scope ONCE with the current scope's query. The flag
+        // stays raised while that read is in flight, so the table shows its
+        // loading state rather than the wrong scope's rows.
+        if (scopeReplacementLatch.repairIssued) {
+          // The repair was dropped too. Do not loop. Clear the flag instead and
+          // let the table state the truth it has: the store's rows with their
+          // count, its empty state, or its error surface with Retry. Clearing
+          // here is safe only while this orchestrator is the sole Paper-side
+          // `fetchItems` caller: a superseding read resolves the same
+          // `currentListQuery` thunk when it is issued, but issued is not
+          // applied, and until it applies `items` still holds the last APPLIED
+          // list, which can be the previous scope's rows. Today the only other
+          // caller is Legacy's `batchTriage` reconciliation read, and Legacy
+          // never renders this flag. A Paper batch surface must clear only once
+          // a read has applied since the latch was raised.
+          isScopeReplacement.value = false
+          scopeReplacementLatch = null
+        } else {
+          scopeReplacementLatch.repairIssued = true
+          repairDroppedReplacement = true
+        }
       }
     } catch {
-      // Store handles toast + error state.
+      // Store handles toast + error state. The flag stays raised deliberately:
+      // a failed load leaves the previous scope's rows in the store, and the
+      // table's error surface carries a Retry while `listError` is set. (A
+      // later background poll success clears `listError` without touching
+      // this flag; that failure-path shape is tracked on #2591.)
     }
     await openItemFromHash()
     inboxLoadPerf.end()
+    if (repairDroppedReplacement) {
+      // A plain load: it must not restart the replacement, only finish it.
+      await loadInboxInternal()
+    }
   }
 
   function loadInbox() {

frontend/taskdeck-web/src/tests/composables/useInboxOrchestrator.spec.ts

@@ -959,30 +959,136 @@ describe('useInboxOrchestrator', () => {
      * throwing. Resolution alone therefore does not mean the new scope's rows
      * arrived, and treating it that way un-hid the retained OLD-scope rows
      * under the NEW scope's chip. Only an applied response clears the flag.
+     *
+     * #2591 keeps that rule and closes what it left open: a dropped response
+     * used to leave the flag latched with nothing coming, so the table hid the
+     * rows AND their count with no Retry. The four cases below pin the whole
+     * contract — a later applied load clears it, a drop with nothing in flight
+     * re-issues ONCE, a second drop stops rather than looping, and a drop under
+     * a changed scope still clears nothing.
      */
-    it('keeps the scope-replacement state when the store drops a superseded response', async () => {
-      const pendingLoad = deferred<boolean>()
-      mockCaptureStore.fetchItems.mockReturnValueOnce(pendingLoad.promise)
+    it('clears the scope-replacement state when the superseding load applies after the dropped one resolves', async () => {
+      const droppedLoad = deferred<boolean>()
+      const supersedingLoad = deferred<boolean>()
+      mockCaptureStore.fetchItems
+        .mockReturnValueOnce(droppedLoad.promise)
+        .mockReturnValueOnce(supersedingLoad.promise)
       const orch = createOrchestrator()
 
-      const load = orch.loadInboxForScopeReplacement()
+      const replacement = orch.loadInboxForScopeReplacement()
+      const superseding = orch.loadInbox()
       expect(orch.isScopeReplacement.value).toBe(true)
 
-      pendingLoad.resolve(false)
-      await load
+      // The store dropped the replacement's response because the second
+      // request superseded it, and it resolves AFTER that second request was
+      // issued.
+      droppedLoad.resolve(false)
+      await replacement
 
+      // The superseding load IS the later load for this scope: no repair read
+      // is owed, and the flag stays set until that load reports it applied.
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(2)
       expect(orch.isScopeReplacement.value).toBe(true)
+
+      mockCaptureStore.items = [{ id: 'a' }, { id: 'b' }]
+      supersedingLoad.resolve(true)
+      await superseding
+
+      // Coherent: rows exposed and the flag down, so the table renders the list
+      // and its count instead of a hidden body under a count-free eyebrow.
+      expect(orch.isScopeReplacement.value).toBe(false)
+      expect(orch.items.value).toHaveLength(2)
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(2)
     })
 
-    it('clears the scope-replacement state on the next applied response', async () => {
-      mockCaptureStore.fetchItems.mockResolvedValueOnce(false).mockResolvedValueOnce(true)
+    it('re-issues the load once when a dropped replacement leaves nothing in flight', async () => {
+      mockRoute.query = { boardId: 'board-1' }
+      const droppedLoad = deferred<boolean>()
+      mockCaptureStore.fetchItems
+        .mockReturnValueOnce(droppedLoad.promise)
+        .mockResolvedValueOnce(true)
       const orch = createOrchestrator()
 
+      const replacement = orch.loadInboxForScopeReplacement()
+      expect(orch.isScopeReplacement.value).toBe(true)
+
+      // Dropped by a read the orchestrator never issued (the store's own
+      // post-batch refresh), so no later orchestrator load will clear this.
+      mockCaptureStore.items = [{ id: 'a' }]
+      droppedLoad.resolve(false)
+      await replacement
+
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(2)
+      expect(mockCaptureStore.fetchItems).toHaveBeenLastCalledWith({ limit: 200, boardId: 'board-1' })
+      expect(orch.isScopeReplacement.value).toBe(false)
+    })
+
+    it('stops after one re-issue when the repair load is dropped too', async () => {
+      mockRoute.query = { boardId: 'board-1' }
+      mockCaptureStore.fetchItems.mockResolvedValue(false)
+      const orch = createOrchestrator()
+      mockCaptureStore.items = [{ id: 'a' }]
+
       await orch.loadInboxForScopeReplacement()
+
+      // Exactly one repair read, never a loop.
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(2)
+      // Honest terminal state: the flag comes down with the store's rows
+      // visible, so the table shows the list and its count (or the store's
+      // error surface with Retry when the read recorded one) — not a hidden
+      // body with no affordance.
+      expect(orch.isScopeReplacement.value).toBe(false)
+      expect(orch.items.value).toHaveLength(1)
+    })
+
+    it('keeps the scope-replacement state when a dropped response resolves under a changed scope', async () => {
+      mockRoute.query = { boardId: 'board-1' }
+      const droppedLoad = deferred<boolean>()
+      const newScopeLoad = deferred<boolean>()
+      mockCaptureStore.fetchItems
+        .mockReturnValueOnce(droppedLoad.promise)
+        .mockReturnValueOnce(newScopeLoad.promise)
+      const orch = createOrchestrator()
+
+      const boardOne = orch.loadInboxForScopeReplacement()
+      mockRoute.query = { boardId: 'board-2' }
+      const boardTwo = orch.loadInboxForScopeReplacement()
+
+      droppedLoad.resolve(false)
+      await boardOne
+
+      // No repair read for a scope nobody is looking at, and board-1's rows
+      // stay hidden under board-2's label (#2501).
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(2)
       expect(orch.isScopeReplacement.value).toBe(true)
 
-      await orch.loadInbox()
+      newScopeLoad.resolve(true)
+      await boardTwo
+
+      expect(orch.isScopeReplacement.value).toBe(false)
+    })
 
+    /**
+     * Replaces the old 'clears the scope-replacement state on the next applied
+     * response' case, whose shape — one dropped response, then a separately
+     * invoked load that applies — is now the repair read itself and is pinned
+     * above. What is left to pin is that the repair budget is per replacement:
+     * an exhausted one must not make the NEXT scope change unrepairable.
+     */
+    it('gives each scope replacement its own single repair read', async () => {
+      mockCaptureStore.fetchItems
+        .mockResolvedValueOnce(false)
+        .mockResolvedValueOnce(false)
+        .mockResolvedValueOnce(false)
+        .mockResolvedValueOnce(true)
+      const orch = createOrchestrator()
+
+      await orch.loadInboxForScopeReplacement()
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(2)
+      expect(orch.isScopeReplacement.value).toBe(false)
+
+      await orch.loadInboxForScopeReplacement()
+      expect(mockCaptureStore.fetchItems).toHaveBeenCalledTimes(4)
       expect(orch.isScopeReplacement.value).toBe(false)
     })
 

frontend/taskdeck-web/src/tests/views/InboxView.spec.ts

@@ -77,7 +77,7 @@ const mockCaptureStore = reactive({
     } | null
     canEditSuggestion?: boolean
   }, syncSummary?: boolean) => void>(),
-  fetchItems: vi.fn<(...args: unknown[]) => Promise<void>>(),
+  fetchItems: vi.fn<(...args: unknown[]) => Promise<boolean>>(),
   fetchDetail: vi.fn<(itemId: string, options?: {
     forceRefresh?: boolean
     recordError?: boolean
@@ -235,7 +235,7 @@ describe('InboxView', () => {
     mockCaptureStore.listError = null
     mockCaptureStore.detailError = null
     mockCaptureStore.actionError = null
-    mockCaptureStore.fetchItems.mockResolvedValue(undefined)
+    mockCaptureStore.fetchItems.mockResolvedValue(true)
     mockCaptureStore.fetchDetail.mockImplementation(async (itemId: string, options) => {
       const forceRefresh = options?.forceRefresh ?? false
       if (!forceRefresh && mockCaptureStore.detailById[itemId]) {

frontend/taskdeck-web/src/tests/views/paper/inbox/PaperInboxScopeTruth.spec.ts

@@ -40,7 +40,7 @@ const mockCaptureStore = reactive({
   listError: null as string | null,
   actionBusyItemId: null as string | null,
   triagePollingItemId: null as string | null,
-  fetchItems: vi.fn<(...args: unknown[]) => Promise<void>>(),
+  fetchItems: vi.fn<(...args: unknown[]) => Promise<boolean>>(),
   fetchDetail: vi.fn(),
   peekDetail: vi.fn(),
   cacheDetail: vi.fn(),
@@ -129,7 +129,12 @@ describe('Paper Inbox scope truth (#1984)', () => {
     mockCaptureStore.detailById = {}
     mockCaptureStore.loadingList = false
     mockCaptureStore.listError = null
-    mockCaptureStore.fetchItems.mockResolvedValue(undefined)
+    // The store REPORTS whether it applied the response (#2501/#2584); this
+    // mock predates that contract and resolved `undefined`, which the
+    // orchestrator now correctly reads as a dropped response and repairs with
+    // a second read (#2591). The ordinary case under test here is an applied
+    // one, so say so.
+    mockCaptureStore.fetchItems.mockResolvedValue(true)
     mockBoardsApi.getBoard.mockResolvedValue(scopedBoard())
     mockBoardStore.fetchBoards.mockResolvedValue(undefined)
     mockSessionStore.userId = 'user-a'