diff --git a/ui/src/components/documents/DocumentReviewRail.tsx b/ui/src/components/documents/DocumentReviewRail.tsx index c6e8519d6b6..cd59258848a 100644 --- a/ui/src/components/documents/DocumentReviewRail.tsx +++ b/ui/src/components/documents/DocumentReviewRail.tsx @@ -3,7 +3,7 @@ import type { DocumentReviewIndex, DocumentSuggestionWithComments, } from "@paperclipai/shared"; -import { CheckCircle2, ChevronDown, ChevronRight, MessageSquarePlus } from "lucide-react"; +import { CheckCircle2, ChevronDown, ChevronRight, Lock, MessageSquarePlus } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Textarea } from "@/components/ui/textarea"; @@ -46,6 +46,12 @@ function suggestionBucket(suggestion: DocumentSuggestionWithComments): ReviewFil export interface DocumentReviewRailProps { reviewIndex?: DocumentReviewIndex; loading?: boolean; + /** + * When the viewer can browse the document but can't read its review thread + * (the review-index endpoint is gated by parent-issue access), surface this + * message instead of a misleading "no feedback" empty state. + */ + accessError?: string | null; /** Viewer can reply/resolve/accept/reject. */ canReview: boolean; /** Viewer has edit rights (gates the Done reviewing CTA). */ @@ -105,6 +111,7 @@ export function DocumentReviewRail(props: DocumentReviewRailProps) { function ReviewRailBody({ reviewIndex, loading, + accessError, canReview, canFinishReview, doneReviewingDisabledReason, @@ -210,6 +217,16 @@ function ReviewRailBody({ } }; + if (accessError) { + return ( +
+
+ ); + } + return (
diff --git a/ui/src/components/documents/DoneReviewingDialog.tsx b/ui/src/components/documents/DoneReviewingDialog.tsx index 90f66df50c1..e6f909f97b1 100644 --- a/ui/src/components/documents/DoneReviewingDialog.tsx +++ b/ui/src/components/documents/DoneReviewingDialog.tsx @@ -27,6 +27,11 @@ export interface DoneReviewingDialogProps { open: boolean; onOpenChange: (open: boolean) => void; counts?: DocumentReviewIndexCounts; + /** + * When the review index couldn't be loaded (e.g. the viewer lacks parent-issue + * access), surface this message instead of misleading zero stats. + */ + accessError?: string | null; /** Linked issue identifier, used to label the "comment on issue" toggle. */ issueIdentifier?: string | null; /** Owner agent display name, used to label the "wake owner" toggle. */ @@ -57,6 +62,7 @@ export function DoneReviewingDialog({ open, onOpenChange, counts, + accessError, issueIdentifier, ownerName, canWakeOwner = true, @@ -97,7 +103,16 @@ export function DoneReviewingDialog({ - {/* Review summary */} + {/* Review summary — suppressed when the counts couldn't be loaded, since + zeros would falsely read as "nothing to hand off". */} + {accessError ? ( +

+ {accessError} +

+ ) : (
{stats.map((stat) => (
))}
- {!hasUnresolved ? ( + )} + {!accessError && !hasUnresolved ? (

No open feedback — this is a clean handoff.

diff --git a/ui/src/pages/DocumentDetail.test.tsx b/ui/src/pages/DocumentDetail.test.tsx index fac4c91ea65..55f740efd2b 100644 --- a/ui/src/pages/DocumentDetail.test.tsx +++ b/ui/src/pages/DocumentDetail.test.tsx @@ -213,6 +213,23 @@ describe("DocumentDetail", () => { expect(edit?.disabled).toBe(true); }); + it("surfaces a review-access error instead of an empty 'no feedback' state on 403", async () => { + mocks.documentsApi.get.mockResolvedValue(makeDoc()); + mocks.documentReviewsApi.reviewIndex.mockRejectedValue(new ApiError("Forbidden", 403, null)); + await renderPage(); + await waitFor( + () => container.querySelector('[data-testid="rail-access-error"]') !== null, + "review access error", + ); + // References the parent issue so the user knows whom to ask. + expect(container.textContent).toContain("don't have access to this document's review thread"); + expect(container.textContent).toContain("PAP-10520"); + // The misleading empty state and the Done-reviewing CTA must NOT render — + // there are no stats to hand off when the thread can't be read. + expect(container.querySelector('[data-testid="rail-empty"]')).toBeNull(); + expect(container.querySelector('[data-testid="rail-done-reviewing"]')).toBeNull(); + }); + it("shows a locked-by-other badge and disables Edit", async () => { mocks.documentsApi.get.mockResolvedValue(makeDoc({ lockedByAgentId: "agent-1", lockedAt: new Date() })); await renderPage(); diff --git a/ui/src/pages/DocumentDetail.tsx b/ui/src/pages/DocumentDetail.tsx index 7db8f25716e..441fb16abc0 100644 --- a/ui/src/pages/DocumentDetail.tsx +++ b/ui/src/pages/DocumentDetail.tsx @@ -167,9 +167,23 @@ export function DocumentDetail() { queryKey: binding ? queryKeys.documents.reviewIndex(binding.issueId, binding.key) : ["documents", "review-index", "none"], queryFn: () => documentReviewsApi.reviewIndex(binding!.issueId, binding!.key, { status: "all", includeComments: true }), enabled: !!binding, + retry: (count, error) => !(error instanceof ApiError && [403, 404].includes(error.status)) && count < 2, }); const reviewIndex = reviewIndexQuery.data; + // The review-index endpoint is gated by the parent issue's authorization + // boundary, but the document itself is listable via the company-scoped library. + // A board user who can browse the doc but not its issue gets a 403 here; surface + // that explicitly instead of falling through to a misleading "no feedback" state. + const reviewAccessError = useMemo(() => { + const error = reviewIndexQuery.error; + if (!(error instanceof ApiError) || error.status !== 403) return null; + const issueIdentifier = doc?.backlinks.find((b) => b.identifier && b.targetType === "issue")?.identifier; + return issueIdentifier + ? `You don't have access to this document's review thread. Ask the owner of ${issueIdentifier} for access.` + : "You don't have access to this document's review thread. Ask the parent issue's owner for access."; + }, [reviewIndexQuery.error, doc?.backlinks]); + const agentMap = useMemo(() => { const map = new Map>(); for (const agent of agents ?? []) map.set(agent.id, agent); @@ -638,8 +652,10 @@ export function DocumentDetail() { b.identifier && b.targetType === "issue")?.identifier ?? null} ownerName={ownerAgent?.name ?? null} canWakeOwner={!!doc.ownerAgentId && !!doc.latestRevisionId}