diff --git a/ui/src/api/access.ts b/ui/src/api/access.ts index c77b212d3e1..2333e7bccd1 100644 --- a/ui/src/api/access.ts +++ b/ui/src/api/access.ts @@ -1,5 +1,5 @@ import type { AgentAdapterType, JoinRequest, PermissionKey } from "@paperclipai/shared"; -import { api } from "./client"; +import { api, type RequestOptions } from "./client"; export type HumanCompanyRole = "owner" | "admin" | "operator" | "viewer"; @@ -311,16 +311,18 @@ export const accessApi = { companyId: string, status: "pending_approval" | "approved" | "rejected" = "pending_approval", requestType?: "human" | "agent", + options?: RequestOptions, ) => api.get( `/companies/${companyId}/join-requests?status=${status}${requestType ? `&requestType=${requestType}` : ""}`, + options, ), listMembers: (companyId: string) => api.get(`/companies/${companyId}/members`), - listUserDirectory: (companyId: string) => - api.get(`/companies/${companyId}/user-directory`), + listUserDirectory: (companyId: string, options?: RequestOptions) => + api.get(`/companies/${companyId}/user-directory`, options), updateMember: ( companyId: string, diff --git a/ui/src/api/approvals.ts b/ui/src/api/approvals.ts index 06b4713a82b..bda58c1cf7b 100644 --- a/ui/src/api/approvals.ts +++ b/ui/src/api/approvals.ts @@ -1,10 +1,11 @@ import type { Approval, ApprovalComment, Issue } from "@paperclipai/shared"; -import { api } from "./client"; +import { api, type RequestOptions } from "./client"; export const approvalsApi = { - list: (companyId: string, status?: string) => + list: (companyId: string, status?: string, options?: RequestOptions) => api.get( `/companies/${companyId}/approvals${status ? `?status=${encodeURIComponent(status)}` : ""}`, + options, ), create: (companyId: string, data: Record) => api.post(`/companies/${companyId}/approvals`, data), diff --git a/ui/src/pages/Inbox.test.tsx b/ui/src/pages/Inbox.test.tsx index 91d05505682..7033e2bc572 100644 --- a/ui/src/pages/Inbox.test.tsx +++ b/ui/src/pages/Inbox.test.tsx @@ -10,6 +10,7 @@ import type { Approval, CompactIssue, HeartbeatRun, InboxMineSnapshot, Issue } f import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CompanyJoinRequest } from "../api/access"; import { ApiError } from "../api/client"; +import { shouldRetryRequest } from "../lib/query-client"; import { clearLocalInboxArchive, getLocalInboxArchiveIssueIds, @@ -1893,6 +1894,168 @@ describe("Inbox toolbar", () => { }); }); + describe("optional reads and the reveal gate", () => { + const renderInbox = async (client: QueryClient, root: ReturnType) => { + await act(async () => { + root.render(); + }); + }; + const rowKeys = () => [...container.querySelectorAll("[data-inbox-item]")] + .map((row) => row.querySelector('[data-slot="task-row-identifier"]')?.textContent ?? ""); + const selectedRowIndex = () => [...container.querySelectorAll("[data-inbox-item]")].findIndex((row) => + row.querySelector("a[data-inbox-issue-link]")?.parentElement + ?.className.split(/\s+/).includes("bg-accent/50")); + const twoRows = () => { + const base = new Date("2026-03-11T00:00:00.000Z").getTime(); + setIssueRows([ + createIssue({ id: "issue-a", identifier: "PAP-4001", title: "Directory row A", assigneeUserId: "user-2", lastActivityAt: new Date(base + 2000) }), + createIssue({ id: "issue-b", identifier: "PAP-4002", title: "Directory row B", assigneeUserId: "user-2", lastActivityAt: new Date(base + 1000) }), + ]); + }; + + it("reveals rows while the user directory is pending and keeps their order and selection when it arrives", async () => { + routerMock.location.pathname = "/inbox/mine"; + twoRows(); + let resolveDirectory: (value: unknown) => void = () => {}; + apiMocks.userDirectoryList.mockReturnValue(new Promise((resolve) => { resolveDirectory = resolve; })); + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"])); + for (let press = 0; press < 2; press += 1) { + await act(async () => { + document.body.dispatchEvent(new KeyboardEvent("keydown", { key: "j", bubbles: true })); + }); + } + expect(selectedRowIndex()).toBe(1); + const rowNodes = [...container.querySelectorAll("[data-inbox-item]")]; + + await act(async () => { + resolveDirectory({ users: [{ principalId: "user-2", user: { id: "user-2", name: "Robin Directory", email: "robin@example.test", image: null } }] }); + }); + + expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"]); + expect([...container.querySelectorAll("[data-inbox-item]")]).toEqual(rowNodes); + expect(selectedRowIndex()).toBe(1); + } finally { await act(() => root.unmount()); client.clear(); } + }); + + it("waits for the user directory when rows are grouped by assignee", async () => { + routerMock.location.pathname = "/inbox/mine"; + localStorage.setItem("paperclip:inbox:group-by", "assignee"); + twoRows(); + let resolveDirectory: (value: unknown) => void = () => {}; + apiMocks.userDirectoryList.mockReturnValue(new Promise((resolve) => { resolveDirectory = resolve; })); + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => expect(apiMocks.userDirectoryList).toHaveBeenCalled()); + expect(container.querySelector("[data-inbox-item]")).toBeNull(); + await act(async () => { resolveDirectory({ users: [] }); }); + await vi.waitFor(() => expect(container.textContent).toContain("Directory row A")); + } finally { await act(() => root.unmount()); client.clear(); } + }); + + it("holds rows for a delayed approvals read so a late approval cannot push them down", async () => { + routerMock.location.pathname = "/inbox/mine"; + twoRows(); + let resolveApprovals: (value: Approval[]) => void = () => {}; + apiMocks.approvalsList.mockReturnValue(new Promise((resolve) => { resolveApprovals = resolve; })); + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => { + const mineQueries = client.getQueryCache().findAll({ queryKey: queryKeys.issues.listMineByMe("company-1") }); + expect(mineQueries.some((query) => query.state.status === "success")).toBe(true); + }); + expect(container.querySelector("[data-inbox-item]")).toBeNull(); + await act(async () => { resolveApprovals([]); }); + await vi.waitFor(() => expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"])); + } finally { await act(() => root.unmount()); client.clear(); } + }); + + it.each([ + ["approvals", () => apiMocks.approvalsList], + ["join requests", () => apiMocks.joinRequestsList], + ] as const)("reveals rows and offers a retry when the %s read fails", async (_label, mock) => { + routerMock.location.pathname = "/inbox/mine"; + twoRows(); + mock().mockRejectedValue(new ApiError("Optional read failed", 500, null)); + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"])); + await vi.waitFor(() => expect(document.body.textContent).toContain("Some inbox items couldn’t load.")); + expect(container.querySelector('[aria-label="Loading tasks"]')).toBeNull(); + } finally { await act(() => root.unmount()); client.clear(); } + }); + + it("stops waiting for an approvals read after its first failed attempt while it retries", async () => { + routerMock.location.pathname = "/inbox/mine"; + twoRows(); + apiMocks.approvalsList + .mockRejectedValueOnce(new ApiError("Busy", 503, null)) + .mockReturnValue(new Promise(() => {})); + const client = new QueryClient({ defaultOptions: { queries: { retry: 3, retryDelay: 0, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => expect(apiMocks.approvalsList).toHaveBeenCalledTimes(2)); + await vi.waitFor(() => expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"])); + } finally { await act(() => root.unmount()); client.clear(); } + }); + + it.each([ + ["approvals", () => apiMocks.approvalsList], + ["join requests", () => apiMocks.joinRequestsList], + ] as const)("shows the missing-items notice when %s keep timing out under the app retry policy", async (_label, mock) => { + routerMock.location.pathname = "/inbox/mine"; + twoRows(); + mock().mockRejectedValue(new ApiError("Timed out", 408, { error: "TIMEOUT" })); + const client = new QueryClient({ defaultOptions: { queries: { retry: shouldRetryRequest, retryDelay: 0, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"])); + await vi.waitFor(() => expect(document.body.textContent).toContain("Some inbox items couldn’t load.")); + expect(mock().mock.calls.length).toBeGreaterThan(1); + expect(document.body.textContent).toContain("Retry missing items"); + expect(document.body.textContent).not.toContain("Inbox zero."); + } finally { await act(() => root.unmount()); client.clear(); } + }); + + it("bounds a stalled approvals read so rows do not stay behind a skeleton", async () => { + routerMock.location.pathname = "/inbox/mine"; + twoRows(); + vi.useFakeTimers({ shouldAdvanceTime: true }); + // The API client turns `timeoutMs` into a 408 TIMEOUT error; model that for the stalled read. + apiMocks.approvalsList.mockImplementation((_companyId: string, _status: unknown, options?: { timeoutMs?: number }) => + new Promise((_resolve, reject) => { + if (options?.timeoutMs === undefined) return; + setTimeout(() => reject(new ApiError("Timed out", 408, { error: "TIMEOUT" })), options.timeoutMs); + })); + const client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } }); + const root = createRoot(container); + try { + await renderInbox(client, root); + await vi.waitFor(() => expect(apiMocks.approvalsList).toHaveBeenCalled()); + const options = apiMocks.approvalsList.mock.calls[0]?.[2] as { timeoutMs?: number } | undefined; + expect(options?.timeoutMs).toBeGreaterThan(0); + expect(container.querySelector("[data-inbox-item]")).toBeNull(); + await act(async () => { vi.advanceTimersByTime(options?.timeoutMs ?? 0); }); + await vi.waitFor(() => expect(rowKeys()).toEqual(["PAP-4001", "PAP-4002"])); + } finally { + vi.useRealTimers(); + await act(() => root.unmount()); + client.clear(); + } + }); + }); + it.each([true, false])("uses bold titles without unread dots (Streamlined UI=%s)", async streamlined => { apiMocks.experimentalSettings.mockResolvedValue({ enableIsolatedWorkspaces: false, enableStreamlinedUi: streamlined }); routerMock.location.pathname = "/inbox/mine"; diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index 116920d3e54..52518fa0cda 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -1,4 +1,4 @@ -import { isTransientError } from "@/api/errors"; +import { errorCodes, isTransientError } from "@/api/errors"; import { useQueryView } from "@/components/QueryView"; import { useCoordinatorPresentation } from "@/components/coordinator/CoordinatorPresentation"; import { useWarmTaskDetailCode } from "@/hooks/useWarmTaskDetailCode"; @@ -205,6 +205,8 @@ const INBOX_ISSUE_LIST_LIMIT = 100; const INBOX_HOT_PATH_STALE_MS = 30_000; // Stable fallback so memos over unloaded or disabled issue reads do not rerun on every render. const NO_ISSUES: Issue[] = []; +/** Bounds an optional read that can hold task rows hidden; a timeout reveals rows and retries in the background. */ +const INBOX_OPTIONAL_READ_TIMEOUT_MS = 10_000; const INBOX_COLLECTION_KEY = "inbox"; type StreamlinedInboxViewState = InboxFilterPreferences & { @@ -959,22 +961,28 @@ function StreamlinedInbox({ const { data: approvals, isLoading: isApprovalsLoading, + failureCount: approvalsFailureCount, error: approvalsError, } = useQuery({ queryKey: queryKeys.approvals.list(selectedCompanyId!), - queryFn: () => approvalsApi.list(selectedCompanyId!), + queryFn: ({ signal }) => + approvalsApi.list(selectedCompanyId!, undefined, { signal, timeoutMs: INBOX_OPTIONAL_READ_TIMEOUT_MS }), enabled: !!selectedCompanyId, }); const { data: joinRequests = [], isLoading: isJoinRequestsLoading, + failureCount: joinRequestsFailureCount, error: joinRequestsError, } = useQuery({ queryKey: queryKeys.access.joinRequests(selectedCompanyId!), - queryFn: async () => { + queryFn: async ({ signal }) => { try { - return await accessApi.listJoinRequests(selectedCompanyId!, "pending_approval"); + return await accessApi.listJoinRequests(selectedCompanyId!, "pending_approval", undefined, { + signal, + timeoutMs: INBOX_OPTIONAL_READ_TIMEOUT_MS, + }); } catch (err) { if (err instanceof ApiError && (err.status === 403 || err.status === 401)) { return []; @@ -1160,9 +1168,14 @@ function StreamlinedInbox({ refetchInterval: sharedLiveRuns.refetchInterval, }); usePublishSharedQueryData(sharedLiveRuns, liveRuns, liveRunsUpdatedAt); - const { data: companyMembers, isLoading: isMembersLoading } = useQuery({ + const { + data: companyMembers, + isLoading: isMembersLoading, + failureCount: membersFailureCount, + } = useQuery({ queryKey: queryKeys.access.companyUserDirectory(selectedCompanyId!), - queryFn: () => accessApi.listUserDirectory(selectedCompanyId!), + queryFn: ({ signal }) => + accessApi.listUserDirectory(selectedCompanyId!, { signal, timeoutMs: INBOX_OPTIONAL_READ_TIMEOUT_MS }), enabled: !!selectedCompanyId, }); const [archivingIssueIds, setArchivingIssueIds] = useState>(new Set()); @@ -2557,13 +2570,23 @@ function StreamlinedInbox({ const workspaceDataAffectsRows = groupBy === "workspace" || (visibleIssueColumnSet.has("workspace") && availableIssueColumnSet.has("workspace")) || normalizedSearchQuery.length > 0; + // Approvals and join requests sort in among task rows by time, so a late arrival would push + // rows down. Wait for their first bounded attempt only; a failure or timeout reveals rows. + const approvalsAffectRows = tab !== "blocked" && (tab !== "all" || showApprovalsCategory); + const joinRequestsAffectRows = tab !== "blocked" && (tab !== "all" || showJoinRequestsCategory); + const isApprovalsFirstAttemptPending = isApprovalsLoading && approvalsFailureCount === 0; + const isJoinRequestsFirstAttemptPending = isJoinRequestsLoading && joinRequestsFailureCount === 0; + // User names only fill row text, except when they name and order assignee groups. + const userLabelsAffectRows = groupBy === "assignee"; + const isMembersFirstAttemptPending = isMembersLoading && membersFailureCount === 0; const initialSourcesSettled = ![sourceSearchView, mineIssuesView, touchedIssuesView].some(view => view.kind === "reconnecting") && - !isSourceSearchLoading && !isSessionLoading && !isSettingsLoading && !isDismissalsLoading && !isAgentsLoading && !isProjectsLoading && !isMembersLoading && + !isSourceSearchLoading && !isSessionLoading && !isSettingsLoading && !isDismissalsLoading && !isAgentsLoading && !isProjectsLoading && + !(userLabelsAffectRows && isMembersFirstAttemptPending) && // Previous summaries stand in while a new id set loads; they may lack the new view's workspaces. !(workspaceDataAffectsRows && workspaceLookupEnabled && (isWorkspacesLoading || isWorkspacesPlaceholder)) && - !isJoinRequestsLoading && - !isApprovalsLoading && + !(joinRequestsAffectRows && isJoinRequestsFirstAttemptPending) && + !(approvalsAffectRows && isApprovalsFirstAttemptPending) && !isDashboardLoading && !isContextIssuesLoading && !isContextIssuesPlaceholder && @@ -2578,7 +2601,12 @@ function StreamlinedInbox({ ?? (issueFilters.externalObjectStatuses.length > 0 ? externalObjectSummariesError : null); const rawOptionalSourceError = dismissalsError ?? approvalsError ?? joinRequestsError ?? runsError ?? contextIssuesError ?? (tab === "mine" ? mineIssuesError : touchedIssuesError); - const optionalSourceError = isTransientError(rawOptionalSourceError) ? null : rawOptionalSourceError; + // A query error means retries are over. Transient errors usually clear on their own, but a read + // that keeps hitting the inbox timeout leaves items missing, so say so and offer a retry. + const optionalReadTimeoutError = [approvalsError, joinRequestsError] + .find((error) => errorCodes(error).includes("TIMEOUT")) ?? null; + const optionalSourceError = optionalReadTimeoutError + ?? (isTransientError(rawOptionalSourceError) ? null : rawOptionalSourceError); const retryFailedSources = () => { // Optional failures belong to the aggregate read, whose task rows stay usable. if (tab === "mine" && (mineSnapshot?.failureError || mineSnapshot?.contextError)) {