diff --git a/frontend/taskdeck-web/src/composables/useReviewProposals.ts b/frontend/taskdeck-web/src/composables/useReviewProposals.ts index a41239455f..4bfefecc47 100644 --- a/frontend/taskdeck-web/src/composables/useReviewProposals.ts +++ b/frontend/taskdeck-web/src/composables/useReviewProposals.ts @@ -302,6 +302,10 @@ export function useReviewProposals() { // permission failure into a fresh false negative, which is the exact class // #2194 exists to remove. const queueAccessRevoked = ref(false) + // Board access is not a global verdict. Keep the board whose list request + // received the 403 so a later request for another board never renders that + // earlier refusal as though it described the new scope (#2214). + const queueAccessRevokedScope = ref(undefined) // Raised only when a user-triggered list read is refused again while the // revoked panel is already up. Background reads and the first refusal keep // the durable authority panel as the only report for that fact. @@ -639,6 +643,10 @@ export function useReviewProposals() { return `${boardScope}:${isArchivedHistory.value ? 'archived' : 'live'}` } + function queueAccessScopeOf(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. @@ -873,6 +881,14 @@ export function useReviewProposals() { if (signal?.aborted) return 'aborted' reviewLoadPerf.start() const requestId = ++latestProposalLoadRequestId + const requestedAccessScope = queueAccessScopeOf(activeBoardFilter.value || undefined) + // A scope change clears the previous board's authority claim before the + // new request starts, but an explicit 403 for that request is still a + // repeated refusal from the reviewer's perspective. Carry only this + // request-local fact into the 403 handler; a 500/other failure never calls + // `recordQueueAccessRevoked`, so it cannot carry the old board into view. + const hadRevokedPreviousScope = + queueAccessRevoked.value && queueAccessRevokedScope.value !== requestedAccessScope let outcome: ProposalLoadOutcome = 'landed' try { @@ -885,6 +901,18 @@ export function useReviewProposals() { // late answer describes the board it queried, never whichever board is on // screen when it lands (#2599 item 1). const requestedScope = queueScopeOf(filters.boardId) + if ( + queueAccessRevoked.value && + queueAccessRevokedScope.value !== requestedAccessScope + ) { + // The new scope has not answered yet, so retain its ordinary loading + // and error semantics. It must not inherit another board's access + // refusal while that request is pending or if it fails transiently. + queueAccessRevoked.value = false + queueAccessRevokedRetry.value = false + queueAccessRevokedScope.value = undefined + resumeQueueRefreshAfterPermissionRecovery() + } // 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 @@ -915,6 +943,7 @@ export function useReviewProposals() { const accessWasRevoked = queueAccessRevoked.value queueAccessRevoked.value = false queueAccessRevokedRetry.value = false + queueAccessRevokedScope.value = undefined if (accessWasRevoked) resumeQueueRefreshAfterPermissionRecovery() } catch (e: unknown) { if (requestId !== latestProposalLoadRequestId) return 'superseded' @@ -945,7 +974,7 @@ export function useReviewProposals() { // that calls `loadProposals` still gets its failure signal and its // 'failed' outcome. if (isForbiddenError(e)) { - recordQueueAccessRevoked(userInitiated) + recordQueueAccessRevoked(requestedAccessScope, userInitiated, hadRevokedPreviousScope) } else { toast.error(getErrorDisplay(e, t('review.toast.loadProposalsFailed')).message) } @@ -1046,10 +1075,17 @@ export function useReviewProposals() { * three statements would be how the two legs drift into telling a reviewer * two different stories about one revocation. */ - function recordQueueAccessRevoked(userInitiated = false) { + function recordQueueAccessRevoked( + scope: string | null, + userInitiated = false, + hadRevokedPreviousScope = false, + ) { const accessWasAlreadyRevoked = queueAccessRevoked.value queueAccessRevoked.value = true - if (accessWasAlreadyRevoked && userInitiated) queueAccessRevokedRetry.value = true + queueAccessRevokedScope.value = scope + if ((accessWasAlreadyRevoked || hadRevokedPreviousScope) && userInitiated) { + queueAccessRevokedRetry.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 @@ -1433,7 +1469,7 @@ export function useReviewProposals() { // Board access was revoked. Stop polling rather than hammering an // endpoint that will keep refusing, drop rows the server no longer // authorises, and let the surface say so. - recordQueueAccessRevoked() + recordQueueAccessRevoked(queueAccessScopeOf(requestedBoardId)) return } // A read for a board the reviewer has already left, or one superseded by diff --git a/frontend/taskdeck-web/src/tests/composables/useReviewProposals.spec.ts b/frontend/taskdeck-web/src/tests/composables/useReviewProposals.spec.ts index cde6bbe610..288fddad50 100644 --- a/frontend/taskdeck-web/src/tests/composables/useReviewProposals.spec.ts +++ b/frontend/taskdeck-web/src/tests/composables/useReviewProposals.spec.ts @@ -1082,6 +1082,25 @@ describe('useReviewProposals', () => { expect(mockToast.error).toHaveBeenCalled() }) + it('does not carry a revoked board claim into another scope that has a transient failure', async () => { + mockRoute.query = { boardId: 'board-b' } + mockAutomationApi.getProposals.mockRejectedValueOnce({ response: { status: 403 } }) + const rp = useReviewProposals() + await rp.loadProposals() + expect(rp.queueAccessRevoked.value).toBe(true) + + mockRoute.query = { boardId: 'board-c' } + mockAutomationApi.getProposals.mockRejectedValueOnce({ response: { status: 500 } }) + await rp.loadProposals() + + // A 500 says C could not be refreshed. It cannot prove that C refused + // access, so the durable panel from B must not describe C. + expect(rp.queueAccessRevoked.value).toBe(false) + expect(rp.queueAccessRevokedRetry.value).toBe(false) + expect(rp.queueScopeLoaded.value).toBe(false) + expect(mockToast.error).toHaveBeenCalled() + }) + it('keeps the pin-leg 403 as the single-proposal outcome #2593 shipped', async () => { // A readable board with one proposal this reviewer may not open is the // opposite case, and it must stay the unavailable pin rather than tearing @@ -1135,6 +1154,26 @@ describe('useReviewProposals', () => { expect(rp.queueAccessRevokedRetry.value).toBe(false) }) + it('keeps repeated refusal feedback when the second explicit read changes scope', async () => { + mockRoute.query = { boardId: 'board-a' } + mockAutomationApi.getProposals.mockRejectedValueOnce({ response: { status: 403 } }) + const rp = useReviewProposals() + await rp.loadProposals() + + expect(rp.queueAccessRevoked.value).toBe(true) + expect(rp.queueAccessRevokedRetry.value).toBe(false) + + mockRoute.query = { boardId: 'board-b' } + mockAutomationApi.getProposals.mockRejectedValueOnce({ response: { status: 403 } }) + await rp.loadProposals() + + // Clearing board A's authority claim before the board B read must not + // erase the fact that this is the second explicit refusal. + expect(rp.queueAccessRevoked.value).toBe(true) + expect(rp.queueAccessRevokedRetry.value).toBe(true) + expect(mockToast.error).not.toHaveBeenCalled() + }) + it('does not raise the retry disclosure for a non-user list read', async () => { mockAutomationApi.getProposals.mockRejectedValueOnce({ response: { status: 403 } }) const rp = useReviewProposals() diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts index 2fd84a6f15..26ffda7be4 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts @@ -1340,10 +1340,12 @@ describe('PaperReviewView', () => { try { expect(wrapper.find('[data-testid="paper-review-access-revoked-retry"]').exists()).toBe(false) - // Changing the board is a deliberate list-read attempt. The queue is - // already refused, so the second refusal needs its own durable sentence. + // Switching this board into its read-only history is a second deliberate + // list-read attempt for the SAME board. A different board must start + // with its own first-refusal state, but this retry keeps the same board + // authority scope and needs its own durable sentence. mocks.getProposals.mockRejectedValueOnce({ response: { status: 403 } }) - await routerOf(wrapper).replace('/workspace/review?boardId=another-board') + await routerOf(wrapper).replace('/workspace/review?boardId=board-revoked&history=archived') await flushPromises() await wrapper.vm.$nextTick()