fix(review): name an unavailable pinned proposal in Legacy and hold the loading announcement

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

@@ -1441,4 +1441,156 @@ describe('ReviewView', () => {
     expect(invalid.text()).not.toContain('no operations')
     expect(mocks.errorToast).not.toHaveBeenCalled()
   })
+
+  // --- #2214: the hash-pinned target's own unavailable state ----------------
+
+  it('names a hash-pinned proposal that is unavailable instead of the generic empty queue (#2214)', async () => {
+    // The queue read succeeds and simply does not contain the pinned proposal;
+    // the proposal-level read then refuses it. That is an identity failure of
+    // the link the reviewer followed, not an empty queue.
+    mocks.getProposals.mockResolvedValue([])
+    mocks.getProposal.mockRejectedValue({ response: { status: 404 } })
+
+    const { wrapper } = await mountAt('/workspace/review#proposal-proposal-gone')
+
+    const unavailable = wrapper.find('[data-testid="review-unavailable-target"]')
+    expect(unavailable.exists()).toBe(true)
+    expect(unavailable.attributes('role')).toBe('status')
+    expect(unavailable.text()).toContain('This proposal is unavailable.')
+    expect(unavailable.text()).toContain('proposal-gone')
+    // The ordinary empty queue must not stand in for a refused deep link.
+    expect(wrapper.find('.td-review-empty').exists()).toBe(false)
+  })
+
+  it('returns to the unpinned queue from the unavailable state (#2214)', async () => {
+    mocks.getProposals.mockResolvedValue([])
+    mocks.getProposal.mockRejectedValue({ response: { status: 404 } })
+
+    const { wrapper, router } = await mountAt('/workspace/review#proposal-proposal-gone')
+
+    const back = wrapper.get('[data-testid="review-unavailable-return"]')
+    await back.trigger('click')
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    expect(router.currentRoute.value.hash).toBe('')
+    expect(wrapper.find('[data-testid="review-unavailable-target"]').exists()).toBe(false)
+  })
+
+  it('recovers to the pinned proposal when the target resolves again (#2214)', async () => {
+    vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval', 'Date'] })
+    try {
+      mocks.getProposals.mockResolvedValue([])
+      mocks.getProposal.mockRejectedValue({ response: { status: 404 } })
+
+      const { wrapper } = await mountAt('/workspace/review#proposal-proposal-flaky')
+      expect(wrapper.find('[data-testid="review-unavailable-target"]').exists()).toBe(true)
+
+      // The target is readable again and the next background read carries it,
+      // so the queue read alone settles the pin.
+      const restored = buildProposal({ id: 'proposal-flaky' })
+      mocks.getProposals.mockResolvedValue([restored])
+      mocks.getProposal.mockResolvedValue(restored)
+
+      vi.advanceTimersByTime(REVIEW_QUEUE_REFRESH_MS)
+      await flushPromises()
+      await wrapper.vm.$nextTick()
+
+      expect(wrapper.find('[data-testid="review-unavailable-target"]').exists()).toBe(false)
+      expect(wrapper.find('#proposal-proposal-flaky').exists()).toBe(true)
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('announces nothing from the queue live region while the queue is loading (#2214)', async () => {
+    // The live region sits above the skeleton, so an ungated one reads "0
+    // proposals awaiting review." under the loading state and then the real
+    // count — the first of which was never true.
+    const pending = createDeferred<Proposal[]>()
+    mocks.getProposals.mockReturnValue(pending.promise)
+
+    const { wrapper } = await mountAt('/workspace/review')
+
+    expect(wrapper.find('.td-review__skeleton').exists()).toBe(true)
+    const live = wrapper.get('[data-testid="review-queue-live"]')
+    // The region itself stays mounted so a later count change is announced in
+    // an already-present live region; only its content is withheld.
+    expect(live.attributes('role')).toBe('status')
+    expect(live.text()).toBe('')
+
+    pending.resolve([buildProposal({ id: 'proposal-loaded' })])
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    expect(wrapper.get('[data-testid="review-queue-live"]').text()).toContain(
+      '1 proposal awaiting review',
+    )
+  })
+
+  it('announces nothing from the queue live region once queue access is revoked (#2214)', async () => {
+    // A current-scope 403 sets queueAccessRevoked AND clears the queue, so the
+    // announcement changes from a real count to 0 — a change, therefore spoken —
+    // while the panel beside it says the queue is gone and has stopped updating.
+    vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval', 'Date'] })
+    try {
+      mocks.getProposals.mockResolvedValue([buildProposal({ id: 'proposal-visible' })])
+      const { wrapper } = await mountAt('/workspace/review')
+      expect(wrapper.get('[data-testid="review-queue-live"]').text()).toContain(
+        '1 proposal awaiting review',
+      )
+
+      mocks.getProposals.mockRejectedValue({ response: { status: 403 } })
+      vi.advanceTimersByTime(REVIEW_QUEUE_REFRESH_MS)
+      await flushPromises()
+      await wrapper.vm.$nextTick()
+
+      expect(wrapper.find('[data-testid="review-access-revoked"]').exists()).toBe(true)
+      const live = wrapper.get('[data-testid="review-queue-live"]')
+      expect(live.attributes('role')).toBe('status')
+      expect(live.text()).toBe('')
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  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.
+    //
+    // What it does NOT pin, measured: the `renderedProposals.length === 0` half
+    // of the panel's own condition. Deleting that half leaves this test green.
+    // The window where the recorded id still names X while the hash names a
+    // renderable Y exists only while `proposalsLoading` is true, because
+    // `openProposalFromHash` early-returns there — and the loading branch
+    // precedes the panel in the same v-if chain, so the skeleton renders
+    // instead. When the read settles, `proposalsLoading = false` and the
+    // clearing of the recorded id happen in one synchronous block, so no
+    // intermediate state is ever rendered. The length half is therefore
+    // defence-in-depth against that ordering changing, not a live guard.
+    const y = buildProposal({ id: 'proposal-y' })
+    mocks.getProposals.mockResolvedValue([y])
+    mocks.getProposal.mockRejectedValue({ response: { status: 404 } })
+
+    const { wrapper, router } = await mountAt('/workspace/review#proposal-proposal-x')
+    expect(wrapper.find('[data-testid="review-unavailable-target"]').exists()).toBe(true)
+
+    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()
+
+    await router.push('/workspace/review#proposal-proposal-y')
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    pending.resolve([y])
+    await flushPromises()
+    await wrapper.vm.$nextTick()
+
+    // The panel must never stand in front of a proposal that renders.
+    expect(wrapper.find('#proposal-proposal-y').exists()).toBe(true)
+    expect(wrapper.find('[data-testid="review-unavailable-target"]').exists()).toBe(false)
+  })
 })

frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts

@@ -3030,6 +3030,66 @@ describe('PaperReviewView', () => {
     wrapper.unmount()
   })
 
+  it('speaks the awaiting count only while it is a real count (#2214)', async () => {
+    // The rail's count is 0 both before the first read lands and after a 403
+    // clears the queue, and neither 0 means "nothing is awaiting review". The
+    // second is the worse of the two: it is a CHANGE from a real count, so an
+    // ungated region speaks it beside a panel saying the queue is gone.
+    vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval', 'Date'] })
+    let wrapper: ReturnType<typeof mount> | null = null
+    try {
+      let resolveQueue!: (value: Proposal[]) => void
+      const pendingQueue = new Promise<Proposal[]>((resolve) => {
+        resolveQueue = resolve
+      })
+      mocks.getProposals.mockReturnValue(pendingQueue)
+      mocks.getBoards.mockResolvedValue([])
+      mocks.getColumns.mockResolvedValue([])
+
+      const router = createRouter({
+        history: createMemoryHistory(),
+        routes: [
+          { path: '/workspace/review', name: 'workspace-review', component: PaperReviewView },
+        ],
+      })
+      router.push('/workspace/review')
+      await router.isReady()
+      wrapper = mount(PaperReviewView, { global: { plugins: [router] } })
+      await nextTick()
+
+      // Still reading: the region is mounted (so a later change lands in a live
+      // region that was already there) and says nothing.
+      const loadingLive = wrapper.find('[data-testid="paper-review-queue-live"]')
+      expect(loadingLive.exists()).toBe(true)
+      expect(loadingLive.attributes('role')).toBe('status')
+      expect(loadingLive.text()).toBe('')
+
+      const proposal = makeProposal({ id: 'announce-1', status: 'PendingReview' })
+      mocks.getProposals.mockResolvedValue([proposal])
+      resolveQueue([proposal])
+      await flushPromises()
+      await wrapper.vm.$nextTick()
+
+      expect(wrapper.find('[data-testid="paper-review-queue-live"]').text()).toContain(
+        '1 proposal awaiting review',
+      )
+
+      // Access is withdrawn: the queue is cleared, so the count would fall to 0.
+      mocks.getProposals.mockRejectedValue({ response: { status: 403 } })
+      vi.advanceTimersByTime(REVIEW_QUEUE_REFRESH_MS)
+      await flushPromises()
+      await wrapper.vm.$nextTick()
+
+      expect(wrapper.find('[data-testid="paper-review-access-revoked"]').exists()).toBe(true)
+      const revokedLive = wrapper.find('[data-testid="paper-review-queue-live"]')
+      expect(revokedLive.attributes('role')).toBe('status')
+      expect(revokedLive.text()).toBe('')
+    } finally {
+      wrapper?.unmount()
+      vi.useRealTimers()
+    }
+  })
+
   it('says the queue is no longer available when a poll is refused with 403 (#2194)', async () => {
     vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval', 'Date'] })
     try {

frontend/taskdeck-web/src/tests/views/paper/review/ReviewQueueRail.spec.ts

@@ -31,13 +31,18 @@ function mountRail(props?: Partial<{
   cadence: number[]
   scopeLabel: string
   scopeClearLabel: string
+  loading: boolean
+  queueUnavailable: boolean
+  awaitingCount: number
 }>) {
   return mount(ReviewQueueRail, {
     props: {
       items: props?.items ?? [makeItem()],
       activeId: props?.activeId ?? null,
-      awaitingCount: 3,
+      awaitingCount: props?.awaitingCount ?? 3,
       staleCount: 2,
+      ...(props?.loading !== undefined ? { loading: props.loading } : {}),
+      ...(props?.queueUnavailable !== undefined ? { queueUnavailable: props.queueUnavailable } : {}),
       dismissableCount: props?.dismissableCount ?? 0,
       busy: props?.busy ?? false,
       batchSelectedCount: props?.batchSelectedCount ?? 0,
@@ -295,3 +300,42 @@ describe('ReviewQueueRail apply-approved action (#1307)', () => {
     expect(wrapper.find('[data-testid="queue-batch-execute"]').exists()).toBe(true)
   })
 })
+
+describe('ReviewQueueRail queue announcement (#2214)', () => {
+  it('announces the awaiting count once the queue is loaded', () => {
+    const wrapper = mountRail({ awaitingCount: 3 })
+    const live = wrapper.get('[data-testid="paper-review-queue-live"]')
+    expect(live.attributes('role')).toBe('status')
+    expect(live.text()).toContain('3 proposals awaiting review')
+  })
+
+  it('announces nothing while the queue is still loading', () => {
+    // A loading rail carries awaitingCount 0 because nothing has been read yet,
+    // so an ungated region reads "0 proposals awaiting review." and then the
+    // real count. Only the content is withheld: the region stays mounted so a
+    // later change lands in a live region that was already present.
+    const wrapper = mountRail({ loading: true, awaitingCount: 0 })
+    const live = wrapper.get('[data-testid="paper-review-queue-live"]')
+    expect(live.attributes('role')).toBe('status')
+    expect(live.text()).toBe('')
+  })
+
+  it('announces nothing once queue access is revoked', () => {
+    // The revoked state clears the queue, so awaitingCount drops to 0 for a
+    // reason that is not "nothing is awaiting review". Same defect as loading,
+    // one branch over.
+    const wrapper = mountRail({ queueUnavailable: true, awaitingCount: 0, items: [] })
+    const live = wrapper.get('[data-testid="paper-review-queue-live"]')
+    expect(live.attributes('role')).toBe('status')
+    expect(live.text()).toBe('')
+  })
+
+  it('keeps announcing when no loading flag is supplied', () => {
+    // The prop is optional so the parent view needs no change to keep today's
+    // behaviour; an omitted flag must never silence the announcement.
+    const wrapper = mountRail({ awaitingCount: 2 })
+    expect(wrapper.get('[data-testid="paper-review-queue-live"]').text()).toContain(
+      '2 proposals awaiting review',
+    )
+  })
+})

frontend/taskdeck-web/src/views/LegacyReviewView.vue

@@ -29,6 +29,7 @@ const {
   summaryCards,
   queueAccessRevoked,
   queueRefreshStale,
+  unavailableProposalId,
   dismissableProposalIds,
   isProposalExpired,
   clearProposalDeepLink,
@@ -113,6 +114,18 @@ const renderedProposals = computed(() => {
   return target ? [target] : []
 })
 
+/**
+ * Leave the refused deep link (#2214). Mirror of `PaperReviewView.returnToReview`
+ * so the two skins cannot drift (#1124 / ADR-0038): clearing the hash is the only
+ * action offered, and it is taken against the id the unavailable state names —
+ * never against whatever the hash happens to hold by then.
+ */
+function returnToReview() {
+  const proposalId = unavailableProposalId.value
+  if (!proposalId) return
+  void clearProposalDeepLink(proposalId)
+}
+
 async function dismissProposalAndReconcileHash(proposalId: string) {
   await handleDismissProposal(proposalId)
   if (!proposals.value.some((proposal) => proposalIdsEqual(proposal.id, proposalId))) {
@@ -194,9 +207,10 @@ function handleReviewKeydown(event: KeyboardEvent) {
 
 const workspace = useWorkspaceStore()
 
-// This skin renders no translated strings (`$t` appears nowhere in this file and
-// its specs install no i18n plugin), so the announcement is plain text, matching
-// the existing hardcoded copy above.
+// This skin's own copy is hardcoded English, so the announcement is plain text,
+// matching the existing hardcoded copy above. (The refused-deep-link state
+// below is the one exception: it reuses the `review.empty.unavailable.*` keys
+// Paper already renders, rather than forking that wording per skin.)
 const awaitingCount = computed(
   () => summaryCards.value.find((card) => card.id === 'pending-review')?.value ?? 0,
 )
@@ -206,6 +220,15 @@ const awaitingAnnouncement = computed(() =>
     : `${awaitingCount.value} proposals awaiting review.`,
 )
 
+/**
+ * Whether `awaitingCount` is a real count right now (#2214). Kept in step with
+ * `ReviewQueueRail.countIsAnnounceable`, which gates the Paper skin's identical
+ * region on the same two states (#1124 / ADR-0038).
+ */
+const countIsAnnounceable = computed(
+  () => !proposalsLoading.value && !queueAccessRevoked.value,
+)
+
 /**
  * Same badge contract as the Paper skin (#2194 acceptance 3): the shell's
  * `Review · N` count is a home-summary workload figure AppShell reads once at
@@ -285,9 +308,16 @@ onUnmounted(() => {
 
     <ReviewSummaryCards :cards="summaryCards" />
 
-    <!-- The queue now changes without user action (#2194); announce it politely. -->
+    <!-- The queue now changes without user action (#2194); announce it politely.
+         The count is only speakable when it is a real count (#2214). While the
+         read is in flight it is 0 because nothing has been read yet, and once
+         access is revoked it is 0 because the queue was withdrawn and cleared —
+         a CHANGE, so an ungated region speaks "0 proposals awaiting review."
+         beside a panel saying the queue is gone. The region withholds its
+         CONTENT rather than being unmounted, because a live region inserted at
+         the same moment its text appears is unreliably announced. -->
     <p class="sr-only" role="status" aria-live="polite" data-testid="review-queue-live">
-      {{ awaitingAnnouncement }}
+      {{ countIsAnnounceable ? awaitingAnnouncement : '' }}
     </p>
 
     <div
@@ -333,6 +363,31 @@ onUnmounted(() => {
       </div>
     </div>
 
+    <!-- A hash-pinned proposal the server refused (403/404) is an identity
+         failure of the link the reviewer followed, not an empty queue (#2214).
+         Saying so — and offering the way back — is the whole difference between
+         "your link is dead" and "there is nothing to review". Ordered before
+         the empty state, and still requiring an empty render, so a pin that has
+         already resolved shows its proposal instead. -->
+    <div
+      v-else-if="unavailableProposalId && renderedProposals.length === 0"
+      class="td-panel"
+      role="status"
+      data-testid="review-unavailable-target"
+    >
+      <p>{{ $t('review.empty.unavailable.eyebrow') }}</p>
+      <p>{{ $t('review.empty.unavailable.title') }}</p>
+      <p>{{ $t('review.empty.unavailable.body', { id: unavailableProposalId }) }}</p>
+      <button
+        type="button"
+        class="td-btn td-btn--secondary td-btn--sm"
+        data-testid="review-unavailable-return"
+        @click="returnToReview"
+      >
+        {{ $t('review.empty.unavailable.return') }}
+      </button>
+    </div>
+
     <ReviewEmptyState
       v-else-if="renderedProposals.length === 0"
       @open-inbox="openInbox"