[Frontend][A11y][Review] Queue live-region residuals: no repeat announcement on a same-scope reload, focus after the unavailable-pin return

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

@@ -617,6 +617,65 @@ export function useReviewProposals() {
    */
   const queueAnnouncementKey = computed(() => awaitingProposalIds.value.join('\n'))
 
+  /**
+   * The board scope of a queue read, as one comparable value. Lower-cased
+   * because the scope is the BOARD, not the casing the query string happened to
+   * carry — the same rule `matchesActiveBoardFilter` applies — and an empty
+   * filter is the unscoped queue, exactly as `boardId: activeBoardFilter.value
+   * || undefined` sends it.
+   */
+  function queueScopeOf(boardId: string | null | undefined): string | null {
+    return boardId ? boardId.toLowerCase() : null
+  }
+
+  /**
+   * The scope a queue read has actually LANDED for, or `undefined` while no read
+   * has landed at all.
+   */
+  const landedQueueScope = ref<string | null | undefined>(undefined)
+
+  /**
+   * Whether a queue read has landed for the board scope currently on screen
+   * (#2599 item 1). This is what the two skins' announcement gates need, and
+   * `!proposalsLoading` was the wrong approximation of it.
+   *
+   * An explicit `loadProposals` raises `proposalsLoading` WITHOUT clearing
+   * `proposals`, so gating on it unmounted the announcement node for the length
+   * of every reload and remounted it with the identical sentence: the live
+   * region wrote count -> '' -> count, and a node addition is exactly what
+   * `aria-live` speaks. The reviewer heard the same figure read back after the
+   * header Refresh and after filing away a settled proposal — reads they asked
+   * for, about a queue that had not moved.
+   *
+   * The states this DOES withhold, all of them cases where the rendered count
+   * is not a count of the queue on screen:
+   *  - before the first read lands (the #2593 skeleton gate — the count is 0
+   *    because nothing has been read, not because nothing awaits review);
+   *  - after a board-filter change, until the new scope's read lands: the rows
+   *    still rendered belong to the previous board;
+   *  - for a scope no read has landed for, including one whose only read
+   *    failed: the entry load, or the first read after a filter change.
+   *
+   * What it deliberately does NOT withhold is the count after a LATER read
+   * failed. `landedQueueScope` is written only where a read replaced the queue
+   * and cleared only by `recordQueueAccessRevoked`; the catch arm leaves it
+   * alone. So an entry load that landed for board A followed by a header
+   * Refresh that 500s keeps the count announceable — correctly, because the
+   * rows on screen are still that landed answer. Re-withholding there would put
+   * back the count -> '' -> count flicker this signal exists to remove, on a
+   * read that changed nothing. A failing refresh has its own reports: the
+   * degraded and refused disclosures (#2214).
+   *
+   * A same-scope reload keeps it settled, because the queue it is about is
+   * still the one being counted. `queueAccessRevoked` keeps its own separate
+   * gate: a revocation is a different fact with a different remedy.
+   */
+  const queueScopeLoaded = computed(
+    () =>
+      landedQueueScope.value !== undefined &&
+      landedQueueScope.value === queueScopeOf(activeBoardFilter.value),
+  )
+
   function isProposalDismissable(proposal: ApiProposal): boolean {
     const status = normalizeProposalStatus(proposal.status)
     return (
@@ -808,6 +867,10 @@ export function useReviewProposals() {
         limit: 200,
         boardId: activeBoardFilter.value || undefined,
       }
+      // The scope this read is ASKING about, snapshotted before the await. A
+      // late answer describes the board it queried, never whichever board is on
+      // screen when it lands (#2599 item 1).
+      const requestedScope = queueScopeOf(filters.boardId)
       // The second argument is forwarded ONLY when a caller supplied options,
       // so every existing call site keeps its exact single-argument shape.
       const loadedProposals = options
@@ -821,6 +884,10 @@ export function useReviewProposals() {
       // authority behind its back, and proves nothing about queue freshness.
       if (signal?.aborted) return 'aborted'
       proposals.value = loadedProposals
+      // A read has now landed for that scope, so the count the surfaces render
+      // is a real count of the queue on screen and may be announced (#2599
+      // item 1).
+      landedQueueScope.value = requestedScope
       // An explicit successful load is as trustworthy as a successful poll and
       // clears any older degraded indication without changing load semantics.
       // It goes through the same accounting as a successful poll so both exits
@@ -967,6 +1034,11 @@ export function useReviewProposals() {
   function recordQueueAccessRevoked() {
     queueAccessRevoked.value = true
     proposals.value = []
+    // What is rendered is no longer any read's answer, so no read has landed
+    // for this scope any more (#2599 item 1). The revoked panel has its own
+    // gate on both surfaces; this only stops the count coming back as speakable
+    // the moment the panel clears for some other reason.
+    landedQueueScope.value = undefined
     suspendQueueRefreshForPermission()
   }
 
@@ -1317,6 +1389,11 @@ export function useReviewProposals() {
         }
       }
       proposals.value = next
+      // `isCurrentRead` has already refused any answer whose scope moved, so
+      // this read landed for the board on screen (#2599 item 1). The poll is a
+      // landing site in its own right: after a failed entry load, it is what
+      // makes the count speakable again without the reviewer reloading.
+      landedQueueScope.value = queueScopeOf(requestedBoardId)
       recordQueueRefreshSuccess({ source: 'poll', recoveryAlreadyRaised: listRecoveryRaised })
       // The queue moved under a reviewer who did not ask for it. Surfaces use
       // this to notice that the row they were rendering has just been dropped
@@ -1594,6 +1671,7 @@ export function useReviewProposals() {
     summaryCards,
     awaitingProposalIds,
     queueAnnouncementKey,
+    queueScopeLoaded,
     dismissableProposalIds,
     matchesActiveBoardFilter,
     isProposalExpired,

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

@@ -1209,6 +1209,124 @@ describe('useReviewProposals', () => {
     })
   })
 
+  // #2599 item 1. Both skins gated the announcement on "no read is in flight",
+  // so every EXPLICIT reload unmounted the sentence and remounted it: the live
+  // region wrote count -> '' -> count and the restore was spoken even when the
+  // queue had not moved (the header Refresh, and filing away a settled proposal
+  // that leaves the pending-review set identical, both reach it). The gate
+  // actually needs "a read has landed for the board scope on screen", which is
+  // what this signal is: a reload of the SAME scope keeps it settled, a scope
+  // change unsettles it until the new scope's read lands, and a failed read
+  // never settles it at all.
+  describe('queue scope load signal (#2599 item 1)', () => {
+    it('settles on the first landed read and stays settled across a same-scope reload', async () => {
+      mockRoute.query = { boardId: 'board-a' }
+      const rp = useReviewProposals()
+      // Nothing has been read yet, so the count is 0 for that reason rather
+      // than because nothing awaits review.
+      expect(rp.queueScopeLoaded.value).toBe(false)
+
+      mockAutomationApi.getProposals.mockResolvedValueOnce([
+        makeProposal({ id: 'p-a', boardId: 'board-a' }),
+      ])
+      await rp.loadProposals()
+      expect(rp.queueScopeLoaded.value).toBe(true)
+
+      // An explicit reload raises `proposalsLoading` WITHOUT clearing
+      // `proposals`, so the rendered count is still the last landed read's and
+      // still true of the board on screen.
+      let releaseReload!: (value: unknown[]) => void
+      mockAutomationApi.getProposals.mockReturnValueOnce(
+        new Promise((resolve) => {
+          releaseReload = resolve as (value: unknown[]) => void
+        }),
+      )
+      const reload = rp.loadProposals()
+      expect(rp.proposalsLoading.value).toBe(true)
+      expect(rp.queueScopeLoaded.value).toBe(true)
+
+      releaseReload([makeProposal({ id: 'p-a', boardId: 'board-a' })])
+      await reload
+      expect(rp.proposalsLoading.value).toBe(false)
+      expect(rp.queueScopeLoaded.value).toBe(true)
+    })
+
+    it('unsettles the moment the board scope changes and settles again when that scope lands', async () => {
+      mockRoute.query = { boardId: 'board-a' }
+      const rp = useReviewProposals()
+      mockAutomationApi.getProposals.mockResolvedValueOnce([
+        makeProposal({ id: 'p-a', boardId: 'board-a' }),
+      ])
+      await rp.loadProposals()
+      expect(rp.queueScopeLoaded.value).toBe(true)
+
+      // The rendered queue is still board-a's, so under board-b it counts
+      // nothing that is on screen -- the one case where withholding is right.
+      mockRoute.query = { boardId: 'board-b' }
+      expect(rp.queueScopeLoaded.value).toBe(false)
+      // The composable's own scope watcher is what issues the reload below.
+      expect(watcherForCurrentSourceValue('board-b')).toBeDefined()
+
+      mockAutomationApi.getProposals.mockResolvedValueOnce([
+        makeProposal({ id: 'p-b', boardId: 'board-b' }),
+      ])
+      await rp.loadProposals()
+      expect(rp.queueScopeLoaded.value).toBe(true)
+
+      // Case is the query string's, not the scope's: the same board asked for
+      // twice is one scope, and `matchesActiveBoardFilter` compares it the same
+      // way.
+      mockRoute.query = { boardId: 'BOARD-B' }
+      expect(rp.queueScopeLoaded.value).toBe(true)
+    })
+
+    it('stays unsettled after a failed entry load and settles on the next successful read', async () => {
+      const rp = useReviewProposals()
+      mockAutomationApi.getProposals.mockRejectedValueOnce(new Error('network down'))
+      await rp.loadProposals()
+
+      // The read is over, so the loading term is false -- and a gate built on
+      // it would announce "0 proposals awaiting review." for a queue nobody has
+      // read.
+      expect(rp.proposalsLoading.value).toBe(false)
+      expect(rp.queueScopeLoaded.value).toBe(false)
+
+      mockAutomationApi.getProposals.mockResolvedValueOnce([makeProposal({ id: 'p-a' })])
+      await rp.loadProposals()
+      expect(rp.queueScopeLoaded.value).toBe(true)
+    })
+
+    it('settles from a background poll, so a failed entry load recovers without an explicit reload', async () => {
+      const rp = useReviewProposals()
+      mockAutomationApi.getProposals.mockRejectedValueOnce(new Error('network down'))
+      await rp.loadProposals()
+      expect(rp.queueScopeLoaded.value).toBe(false)
+
+      mockAutomationApi.getProposals.mockResolvedValueOnce([makeProposal({ id: 'p-a' })])
+      await rp.refreshProposals()
+      expect(rp.queueScopeLoaded.value).toBe(true)
+    })
+
+    it('unsettles when a refusal clears the queue and settles again when access returns', async () => {
+      const rp = useReviewProposals()
+      mockAutomationApi.getProposals.mockResolvedValueOnce([makeProposal({ id: 'p-a' })])
+      await rp.loadProposals()
+      expect(rp.queueScopeLoaded.value).toBe(true)
+
+      // `recordQueueAccessRevoked` empties the queue: what is rendered is no
+      // longer any read's answer.
+      mockAutomationApi.getProposals.mockRejectedValueOnce({ response: { status: 403 } })
+      await rp.loadProposals()
+      expect(rp.queueAccessRevoked.value).toBe(true)
+      expect(rp.queueScopeLoaded.value).toBe(false)
+
+      mockAutomationApi.getProposals.mockResolvedValueOnce([makeProposal({ id: 'p-a' })])
+      await rp.loadProposals()
+      expect(rp.queueAccessRevoked.value).toBe(false)
+      expect(rp.queueScopeLoaded.value).toBe(true)
+    })
+  })
+
   describe('navigation helpers', () => {
     it('openInbox pushes inbox path with board filter', () => {
       mockRoute.query = { boardId: 'board-x' }

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

@@ -1947,6 +1947,135 @@ describe('ReviewView', () => {
     }
   })
 
+  it('keeps the same announcement node across an explicit reload that did not change the queue (#2599 item 1)', async () => {
+    // The gate was `!proposalsLoading`, and an explicit `loadProposals` raises
+    // that flag WITHOUT clearing `proposals`: the node unmounted and remounted
+    // with the same sentence, so the live region wrote count -> '' -> count and
+    // the restore was spoken for a queue that had not moved. Reached by the
+    // header Refresh below, and by filing away a settled proposal.
+    mocks.getProposals.mockResolvedValue([buildProposal({ id: 'proposal-first' })])
+    const { wrapper } = await mountAt('/workspace/review')
+
+    const region = wrapper.get('[data-testid="review-queue-live"]').element
+    const before = wrapper.get('[data-testid="review-queue-announcement"]')
+    expect(before.text()).toContain('1 proposal awaiting review')
+    const beforeEl = before.element
+
+    const pending = createDeferred<Proposal[]>()
+    mocks.getProposals.mockReturnValue(pending.promise)
+    const refresh = wrapper.findAll('button').find((node) => node.text() === 'Refresh Review')!
+    await refresh.trigger('click')
+    await wrapper.vm.$nextTick()
+
+    // Mid-read: the skeleton is up, and the count beside it is still the last
+    // landed read's count of this same board.
+    expect(wrapper.find('.td-review__skeleton').exists()).toBe(true)
+    expect(wrapper.get('[data-testid="review-queue-announcement"]').element).toBe(beforeEl)
+
+    pending.resolve([buildProposal({ id: 'proposal-first' })])
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    // Byte-identical answer: the same node, so nothing was added to the region
+    // and nothing is spoken. A CHANGED reload still re-keys (the item-4 test
+    // above owns that half).
+    expect(wrapper.get('[data-testid="review-queue-announcement"]').element).toBe(beforeEl)
+    expect(wrapper.get('[data-testid="review-queue-live"]').element).toBe(region)
+  })
+
+  it('re-keys the announcement once when an explicit reload did change the queue (#2599 item 1)', async () => {
+    // The other half of the same gate: silence for a reload that changed
+    // nothing must not become silence for one that did. #2710's key still
+    // carries it, and now it carries it without the node being unmounted and
+    // rebuilt around the reload.
+    mocks.getProposals.mockResolvedValue([buildProposal({ id: 'proposal-first' })])
+    const { wrapper } = await mountAt('/workspace/review')
+
+    const region = wrapper.get('[data-testid="review-queue-live"]').element
+    const beforeEl = wrapper.get('[data-testid="review-queue-announcement"]').element
+
+    // Same count, different proposal: the sentence is byte-identical, so only
+    // the key can carry the change.
+    mocks.getProposals.mockResolvedValue([buildProposal({ id: 'proposal-second' })])
+    const refresh = wrapper.findAll('button').find((node) => node.text() === 'Refresh Review')!
+    await refresh.trigger('click')
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    const after = wrapper.get('[data-testid="review-queue-announcement"]')
+    expect(after.text()).toContain('1 proposal awaiting review')
+    expect(after.element).not.toBe(beforeEl)
+    expect(wrapper.get('[data-testid="review-queue-live"]').element).toBe(region)
+  })
+
+  it('withholds the announcement across a board-filter change until the new scope lands (#2599 item 1)', async () => {
+    // The one reload where the rendered count genuinely stops being real: the
+    // queue on screen belongs to the previous board until the new scope's read
+    // replaces it.
+    mocks.getProposals.mockResolvedValue([
+      buildProposal({ id: 'proposal-a', boardId: 'board-a' }),
+    ])
+    const { wrapper, router } = await mountAt('/workspace/review?boardId=board-a')
+    expect(wrapper.get('[data-testid="review-queue-announcement"]').text()).toContain(
+      '1 proposal awaiting review',
+    )
+
+    const pending = createDeferred<Proposal[]>()
+    mocks.getProposals.mockReturnValue(pending.promise)
+    await router.push('/workspace/review?boardId=board-b')
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    expect(wrapper.find('[data-testid="review-queue-announcement"]').exists()).toBe(false)
+    expect(wrapper.get('[data-testid="review-queue-live"]').text()).toBe('')
+
+    pending.resolve([
+      buildProposal({ id: 'proposal-b1', boardId: 'board-b' }),
+      buildProposal({ id: 'proposal-b2', boardId: 'board-b' }),
+    ])
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    expect(wrapper.get('[data-testid="review-queue-announcement"]').text()).toContain(
+      '2 proposals awaiting review',
+    )
+  })
+
+  it('moves focus to the queue after leaving an unavailable pin (#2599 item 2)', async () => {
+    // The panel the return control lives in is removed by the click, so focus
+    // fell to <body>: no announcement, and the next keystroke acts on nothing.
+    mocks.getProposals.mockResolvedValue([buildProposal({ id: 'proposal-live' })])
+    mocks.getProposal.mockRejectedValue({ response: { status: 404 } })
+
+    const { wrapper } = await mountAt('/workspace/review#proposal-proposal-gone')
+    const back = wrapper.get('[data-testid="review-unavailable-return"]')
+    expect(wrapper.find('.td-review__list').exists()).toBe(false)
+
+    await back.trigger('click')
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    // The queue list is this skin's focusable queue: it carries the
+    // "Proposals awaiting review" label and the Arrow cursor that starts on the
+    // first row.
+    const list = wrapper.get('.td-review__list').element
+    expect(document.activeElement).toBe(list)
+  })
+
+  it('moves focus to the empty state after leaving an unavailable pin with nothing left to review (#2599 item 2)', async () => {
+    mocks.getProposals.mockResolvedValue([])
+    mocks.getProposal.mockRejectedValue({ response: { status: 404 } })
+
+    const { wrapper } = await mountAt('/workspace/review#proposal-proposal-gone')
+    await wrapper.get('[data-testid="review-unavailable-return"]').trigger('click')
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    const empty = wrapper.get('.td-review-empty').element
+    expect(empty.getAttribute('tabindex')).toBe('-1')
+    expect(document.activeElement).toBe(empty)
+  })
+
   it('renders the pinned proposal, not the unavailable panel, after moving from a dead pin to a live one (#2214)', async () => {
     // What this pins: navigating from a refused pin X to a resolvable pin Y
     // shows Y's card and no panel.