diff --git a/apps/app/src/App.tsx b/apps/app/src/App.tsx index 4bc8707e8c0..1a673e6040b 100644 --- a/apps/app/src/App.tsx +++ b/apps/app/src/App.tsx @@ -68,6 +68,7 @@ import { AppCommandProvider } from "./components/commands/AppCommandProvider"; import { ProviderCliInstallLogDialogHost } from "./components/provider-cli/provider-cli-install"; import { ServerMoveOverlay } from "./components/machines/ServerMoveOverlay"; import { RouteLoadingSkeleton } from "./components/ui/route-loading-skeleton"; +import { RouteContent } from "./components/RouteContent"; const SettingsView = lazy(() => import("./views/SettingsView").then((m) => ({ @@ -264,7 +265,7 @@ export function HashNavigationScroll() { export function AppRoutes() { return ( - + - + ); } diff --git a/apps/app/src/components/AppErrorBoundary.tsx b/apps/app/src/components/AppErrorBoundary.tsx index 46c54b4aa30..57d398e56b5 100644 --- a/apps/app/src/components/AppErrorBoundary.tsx +++ b/apps/app/src/components/AppErrorBoundary.tsx @@ -2,24 +2,43 @@ import { Component, type ErrorInfo, type ReactNode } from "react"; interface AppErrorBoundaryProps { children: ReactNode; + fallback?: (error: Error) => ReactNode; + resetKey?: string; } interface AppErrorBoundaryState { error: Error | null; + resetKey?: string; } export class AppErrorBoundary extends Component< AppErrorBoundaryProps, AppErrorBoundaryState > { - override state: AppErrorBoundaryState = { error: null }; + override state: AppErrorBoundaryState = { + error: null, + resetKey: this.props.resetKey, + }; + + static getDerivedStateFromProps( + props: AppErrorBoundaryProps, + state: AppErrorBoundaryState, + ): AppErrorBoundaryState | null { + return props.resetKey === state.resetKey + ? null + : { error: null, resetKey: props.resetKey }; + } static getDerivedStateFromError(error: unknown): AppErrorBoundaryState { return { error: error instanceof Error ? error : new Error(String(error)) }; } override componentDidCatch(error: Error, info: ErrorInfo): void { - console.error("[bb] the app crashed", error, info.componentStack); + console.error( + this.props.fallback ? "[bb] a page failed to load" : "[bb] the app crashed", + error, + info.componentStack, + ); } override render(): ReactNode { @@ -27,6 +46,9 @@ export class AppErrorBoundary extends Component< if (error === null) { return this.props.children; } + if (this.props.fallback) { + return this.props.fallback(error); + } return (
diff --git a/apps/app/src/components/RouteContent.test.tsx b/apps/app/src/components/RouteContent.test.tsx new file mode 100644 index 00000000000..62759c8a038 --- /dev/null +++ b/apps/app/src/components/RouteContent.test.tsx @@ -0,0 +1,89 @@ +// @vitest-environment jsdom + +import { lazy, useState } from "react"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { Link, MemoryRouter, Route, Routes } from "react-router-dom"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { AppErrorBoundary } from "./AppErrorBoundary"; +import { RouteContent } from "./RouteContent"; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe("RouteContent", () => { + it.each([ + "Failed to fetch dynamically imported module: /assets/settings.js", + "Importing a module script failed.", + "error loading dynamically imported module: /assets/settings.js", + "Unable to preload CSS for /assets/settings.css", + ])("contains a failed page download: %s", async (message) => { + vi.spyOn(console, "error").mockImplementation(() => undefined); + const FailedPage = lazy(async () => { + throw new TypeError(message); + }); + render( + + + + + + } /> + Loaded content} /> + + + + , + ); + + expect((await screen.findByRole("alert")).textContent).toContain( + "Couldn't load this page.", + ); + expect(screen.getByRole("button", { name: "Reload page" })).toBeTruthy(); + expect(screen.queryByText("bb hit an error and stopped")).toBeNull(); + fireEvent.click(screen.getByRole("link", { name: "Working page" })); + expect( + await screen.findByRole("heading", { name: "Loaded content" }), + ).toBeTruthy(); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("preserves mounted content across healthy navigation", () => { + function Page() { + const [count, setCount] = useState(0); + return ; + } + render( + + Change view + + + + , + ); + fireEvent.click(screen.getByRole("button", { name: "Count 0" })); + fireEvent.click(screen.getByRole("link", { name: "Change view" })); + expect(screen.getByRole("button", { name: "Count 1" })).toBeTruthy(); + }); + + it("leaves ordinary render failures to the app recovery screen", () => { + vi.spyOn(console, "error").mockImplementation(() => undefined); + function BrokenPage(): never { + throw new Error("render exploded"); + } + render( + + + + + + + , + ); + expect(screen.getByText("bb hit an error and stopped")).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Reload page" })).toBeNull(); + }); +}); diff --git a/apps/app/src/components/RouteContent.tsx b/apps/app/src/components/RouteContent.tsx new file mode 100644 index 00000000000..d124ad62dbe --- /dev/null +++ b/apps/app/src/components/RouteContent.tsx @@ -0,0 +1,47 @@ +import { Suspense, useEffect, type ReactNode } from "react"; +import { useLocation } from "react-router-dom"; +import { Button } from "@bb/shared-ui/button"; +import { EmptyStatePanel } from "@bb/shared-ui/empty-state"; +import { markRouteContentPainted } from "@/lib/route-content-paint"; +import { AppErrorBoundary } from "./AppErrorBoundary"; + +function PageLoadError() { + useEffect(() => { + markRouteContentPainted(); + }, []); + + return ( + +

Couldn't load this page.

+

Check your connection, then reload to try again.

+ +
+ ); +} + +function renderPageLoadError(error: Error): ReactNode { + if ( + !/^(Failed to fetch dynamically imported module|Importing a module script failed|error loading dynamically imported module|Unable to preload CSS for)/.test( + error.message, + ) + ) { + throw error; + } + return ; +} + +export function RouteContent({ children }: { children: ReactNode }) { + const location = useLocation(); + return ( + + {children} + + ); +} diff --git a/apps/app/src/components/plugin/PluginSettings.test.tsx b/apps/app/src/components/plugin/PluginSettings.test.tsx index 908bd1f59d2..109454e03e6 100644 --- a/apps/app/src/components/plugin/PluginSettings.test.tsx +++ b/apps/app/src/components/plugin/PluginSettings.test.tsx @@ -738,19 +738,22 @@ describe("PluginSettingsPage", () => { expect(screen.queryByTestId("plugin-settings-skeleton")).toBeNull(); }); - it("keeps loaded settings visible when a background plugin-list refresh fails", async () => { - let pluginListRequests = 0; + it("keeps loaded settings through a failed refresh and updates on recovery", async () => { + let failRefresh = false; + let greeting = "hello"; vi.stubGlobal( "fetch", vi.fn(async (url: string) => { - if (url === "/api/v1/plugins/linear/settings") { - return jsonOk(SETTINGS_VIEW); + if (failRefresh) { + return new Response("unavailable", { status: 503 }); } - pluginListRequests += 1; - if (pluginListRequests === 1) { - return jsonOk({ plugins: [installedPlugin(true)] }); + if (url === "/api/v1/plugins/linear/settings") { + return jsonOk({ + ...SETTINGS_VIEW, + values: { ...SETTINGS_VIEW.values, greeting }, + }); } - throw new Error("offline"); + return jsonOk({ plugins: [installedPlugin(true)] }); }), ); @@ -764,12 +767,22 @@ describe("PluginSettingsPage", () => { , ); - expect(await screen.findByRole("heading", { name: "Linear" })).toBeTruthy(); - - await queryClient.invalidateQueries(); + expect(await screen.findByDisplayValue("hello")).toBeTruthy(); + failRefresh = true; + await act(async () => { + await queryClient.invalidateQueries(); + }); expect(screen.getByRole("heading", { name: "Linear" })).toBeTruthy(); + expect(screen.getByDisplayValue("hello")).toBeTruthy(); expect(screen.queryByText("Could not load plugin settings.")).toBeNull(); + + failRefresh = false; + greeting = "updated"; + await act(async () => { + await queryClient.invalidateQueries(); + }); + expect(await screen.findByDisplayValue("updated")).toBeTruthy(); }); it("keeps the not-installed message free of loading affordances", async () => { diff --git a/apps/app/src/components/plugin/PluginsOverview.test.tsx b/apps/app/src/components/plugin/PluginsOverview.test.tsx index 231f5c0c5ad..606bc0dff17 100644 --- a/apps/app/src/components/plugin/PluginsOverview.test.tsx +++ b/apps/app/src/components/plugin/PluginsOverview.test.tsx @@ -21,6 +21,7 @@ import { createQueryClientTestHarness } from "@/test/queryClientTestHarness"; import { makeSystemConfig } from "@/test/fixtures/system-config"; import { SidebarHistoryNavigationControls } from "@/components/sidebar/SidebarHistoryNavigationControls"; import { resetAppRouteHistoryForTest } from "@/lib/app-route-history"; +import { pluginListQueryKey } from "@/hooks/queries/query-keys"; import { PluginsOverview } from "./PluginsOverview"; vi.mock("@/components/plugin/PluginNewThreadComposer", () => ({ @@ -198,6 +199,33 @@ afterEach(() => { }); describe("PluginsOverview", () => { + it("keeps installed plugins visible when a background refresh fails", async () => { + installFetch(); + const { wrapper, queryClient } = createQueryClientTestHarness(); + render( + + + , + { wrapper }, + ); + await screen.findByTestId("plugin-row-automations"); + vi.mocked(fetch).mockResolvedValue( + responseJson({ error: "refresh failed" }, 503), + ); + await act(async () => { + await queryClient.invalidateQueries({ + queryKey: pluginListQueryKey(true), + }); + }); + await waitFor(() => + expect(queryClient.getQueryState(pluginListQueryKey(true))?.status).toBe( + "error", + ), + ); + expect(screen.getByTestId("plugin-row-automations")).toBeTruthy(); + expect(screen.queryByText("Couldn't load plugins.")).toBeNull(); + }); + it("opens on Browse and renders it before Installed", async () => { installFetch(); const { wrapper: QueryClientWrapper } = createQueryClientTestHarness(); diff --git a/apps/app/src/components/plugin/PluginsOverview.tsx b/apps/app/src/components/plugin/PluginsOverview.tsx index 60caedae761..5dae99b089f 100644 --- a/apps/app/src/components/plugin/PluginsOverview.tsx +++ b/apps/app/src/components/plugin/PluginsOverview.tsx @@ -222,7 +222,7 @@ export function PluginsOverview({ } >
- {listQuery.isError ? ( + {listQuery.isError && listQuery.data === undefined ? ( , { wrapper }, ); + return { ...view, queryClient }; } async function openProjectMenu(projectName: string): Promise { @@ -173,6 +180,8 @@ beforeEach(() => { afterEach(() => { cleanup(); + resetSidebarBootstrapCacheForTest(); + window.localStorage.removeItem(SIDEBAR_BOOTSTRAP_CACHE_KEY); vi.unstubAllGlobals(); vi.clearAllMocks(); }); @@ -266,6 +275,25 @@ describe("formatGitRemote", () => { }); describe("ProjectsSettingsSection", () => { + it("keeps cached projects visible when a background refresh fails", async () => { + stubSidebarBootstrapFetch(projects); + const { queryClient } = renderSection(); + await screen.findByRole("link", { name: "Open bb settings" }); + stubSidebarBootstrapFetch([], 503); + await act(async () => { + await queryClient.invalidateQueries({ + queryKey: sidebarNavigationQueryKey(), + }); + }); + await waitFor(() => + expect( + queryClient.getQueryState(sidebarNavigationQueryKey())?.status, + ).toBe("error"), + ); + expect(screen.getByRole("link", { name: "Open bb settings" })).toBeTruthy(); + expect(screen.queryByText("Couldn't load projects.")).toBeNull(); + }); + it("summarises each project's remote, machine coverage, and threads", async () => { stubSidebarBootstrapFetch(projects); diff --git a/apps/app/src/components/settings/ProjectsSettingsSection.tsx b/apps/app/src/components/settings/ProjectsSettingsSection.tsx index 222b682a5a1..f56e7a84635 100644 --- a/apps/app/src/components/settings/ProjectsSettingsSection.tsx +++ b/apps/app/src/components/settings/ProjectsSettingsSection.tsx @@ -288,7 +288,7 @@ export function ProjectsSettingsSection() { } > - {sidebarNavigationQuery.isError ? ( + {sidebarNavigationQuery.isError && projects === undefined ? (

Couldn't load projects.

diff --git a/apps/app/src/components/tools/SkillsLibrary.tsx b/apps/app/src/components/tools/SkillsLibrary.tsx index 833b013ea2f..0d160c510ac 100644 --- a/apps/app/src/components/tools/SkillsLibrary.tsx +++ b/apps/app/src/components/tools/SkillsLibrary.tsx @@ -127,7 +127,7 @@ function SkillDetailPage({ onSelectPath={setSelectedPath} content={contentQuery.data?.content ?? ""} isLoadingContent={contentQuery.isLoading} - isContentError={contentQuery.isError} + isContentError={contentQuery.isError && contentQuery.data === undefined} canEdit={editableScope !== null} canDelete={deletableScope !== null} canOpenInEditor={editableScope !== null && canOpenPreferredFileTarget} @@ -541,8 +541,7 @@ export function SkillsLibrary() { message="Checking skill source" layout="detail" /> - ) : selectedRegistrySkill && - (registryDetailQuery.isError || registryDetail === null) ? ( + ) : selectedRegistrySkill && registryDetail === null ? ( { - try { - const result = await createPluginsClient(fetchImpl).getSettings({ - pluginId, - }); - return { schema: result.schema, values: result.values }; - } catch { - return null; - } +): Promise { + const result = await createPluginsClient(fetchImpl).getSettings({ + pluginId, + }); + return { schema: result.schema, values: result.values }; } export async function updatePluginSettings( diff --git a/apps/app/src/hooks/queries/sidebar-navigation-query.test.tsx b/apps/app/src/hooks/queries/sidebar-navigation-query.test.tsx index 94e6bb688b3..42901f897dc 100644 --- a/apps/app/src/hooks/queries/sidebar-navigation-query.test.tsx +++ b/apps/app/src/hooks/queries/sidebar-navigation-query.test.tsx @@ -6,6 +6,7 @@ import { type SidebarBootstrapResponse, } from "@bb/server-contract"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { createDeferredPromise } from "@bb/test-helpers"; import { request } from "@/lib/api"; import { MAX_CACHED_SIDEBAR_THREADS_PER_PROJECT, @@ -14,7 +15,11 @@ import { } from "@/lib/sidebar-bootstrap-cache"; import { makeThreadListEntry } from "@bb/test-helpers/domain-fixtures"; import { createQueryClientTestHarness } from "@/test/queryClientTestHarness"; -import { useSidebarNavigation } from "./sidebar-navigation-query"; +import { + useProjectDisplayName, + useSidebarNavigation, +} from "./sidebar-navigation-query"; +import { sidebarNavigationQueryKey } from "./query-keys"; import { makeProjectWithThreadsResponse, makeSidebarBootstrapResponse, @@ -68,6 +73,86 @@ afterEach(() => { }); describe("useSidebarNavigation", () => { + it("shares persisted page data and replaces it after a background refresh", async () => { + window.localStorage.setItem( + SIDEBAR_BOOTSTRAP_CACHE_KEY, + JSON.stringify(BOOTSTRAP), + ); + const refresh = createDeferredPromise(); + vi.mocked(request).mockReturnValue(refresh.promise); + const { queryClient, wrapper } = createQueryClientTestHarness(); + const { result } = renderHook( + () => ({ + navigation: useSidebarNavigation(), + name: useProjectDisplayName("proj_felt"), + }), + { wrapper }, + ); + expect(result.current.navigation.data).toEqual(BOOTSTRAP); + expect(result.current.name).toBe("Felt walk"); + expect(result.current.navigation.isFetching).toBe(true); + expect(request).toHaveBeenCalledTimes(1); + const updated = { + ...BOOTSTRAP, + projects: [{ ...BOOTSTRAP.projects[0]!, name: "Refreshed project" }], + }; + await act(async () => { + refresh.resolve(updated); + }); + await waitFor(() => expect(result.current.name).toBe("Refreshed project")); + expect(queryClient.getQueryData(sidebarNavigationQueryKey())).toEqual( + updated, + ); + }); + + it("keeps persisted content after refresh failure and recovers on retry", async () => { + window.localStorage.setItem( + SIDEBAR_BOOTSTRAP_CACHE_KEY, + JSON.stringify(BOOTSTRAP), + ); + vi.mocked(request).mockRejectedValue(new Error("refresh failed")); + const { wrapper } = createQueryClientTestHarness(); + const { result } = renderHook(() => useSidebarNavigation(), { wrapper }); + await waitFor(() => expect(result.current.isError).toBe(true)); + expect(result.current.data).toEqual(BOOTSTRAP); + expect(result.current.isLoadingError).toBe(false); + vi.mocked(request).mockResolvedValue(BOOTSTRAP); + await act(async () => { + await result.current.refetch(); + }); + await waitFor(() => expect(result.current.isError).toBe(false)); + expect(result.current.data).toEqual(BOOTSTRAP); + }); + + it("prefers newer query data to persistence and honors invalidation", async () => { + window.localStorage.setItem( + SIDEBAR_BOOTSTRAP_CACHE_KEY, + JSON.stringify(BOOTSTRAP), + ); + const current = { ...BOOTSTRAP, projects: [] }; + const { queryClient, wrapper } = createQueryClientTestHarness(); + queryClient.setQueryData(sidebarNavigationQueryKey(), current); + vi.mocked(request).mockResolvedValue(BOOTSTRAP); + const { result } = renderHook(() => useSidebarNavigation(), { wrapper }); + expect(result.current.data).toEqual(current); + expect(request).not.toHaveBeenCalled(); + await act(async () => { + await queryClient.invalidateQueries({ + queryKey: sidebarNavigationQueryKey(), + }); + }); + await waitFor(() => expect(result.current.data).toEqual(BOOTSTRAP)); + expect(request).toHaveBeenCalledTimes(1); + }); + + it("preserves errors on a cache miss", async () => { + vi.mocked(request).mockRejectedValue(new Error("load failed")); + const { wrapper } = createQueryClientTestHarness(); + const { result } = renderHook(() => useSidebarNavigation(), { wrapper }); + await waitFor(() => expect(result.current.isLoadingError).toBe(true)); + expect(result.current.data).toBeUndefined(); + }); + it("replays the last bootstrap while the live one loads", async () => { sidebarBootstrapResponseSchema.parse(BOOTSTRAP); @@ -84,7 +169,7 @@ describe("useSidebarNavigation", () => { const { result } = renderHook(() => useSidebarNavigation(), { wrapper: reloadHarness.wrapper, }); - expect(result.current.isPlaceholderData).toBe(true); + expect(result.current.isPlaceholderData).toBe(false); expect(result.current.data?.projects[0]?.name).toBe("Felt walk"); await waitFor(() => expect(request).toHaveBeenCalled()); }); @@ -145,7 +230,7 @@ describe("useSidebarNavigation", () => { const { result } = renderHook(() => useSidebarNavigation(), { wrapper: reloadHarness.wrapper, }); - expect(result.current.isPlaceholderData).toBe(true); + expect(result.current.isPlaceholderData).toBe(false); expect(result.current.data?.projects[0]?.threads).toHaveLength( MAX_CACHED_SIDEBAR_THREADS_PER_PROJECT, ); diff --git a/apps/app/src/hooks/queries/sidebar-navigation-query.ts b/apps/app/src/hooks/queries/sidebar-navigation-query.ts index 0a62120f2eb..18560b678b1 100644 --- a/apps/app/src/hooks/queries/sidebar-navigation-query.ts +++ b/apps/app/src/hooks/queries/sidebar-navigation-query.ts @@ -1,4 +1,4 @@ -import { useQuery } from "@tanstack/react-query"; +import { queryOptions, useQuery } from "@tanstack/react-query"; import { useCallback } from "react"; import { PERSONAL_PROJECT_ID, type ThreadListEntry } from "@bb/domain"; import type { SidebarBootstrapResponse } from "@bb/server-contract"; @@ -13,18 +13,25 @@ import { } from "@/hooks/useRealtimeSubscription"; import type { QueryOptions } from "./query-helpers"; import { sidebarNavigationQueryKey } from "./query-keys"; -import { REALTIME_OWNED_STATIC_CACHE_QUERY_POLICY } from "./query-policies"; import { readCachedSidebarBootstrap, writeCachedSidebarBootstrap, } from "@/lib/sidebar-bootstrap-cache"; -function fetchSidebarNavigation( - signal?: AbortSignal, -): Promise { - return request( - apiClient["sidebar-bootstrap"].$get(undefined, requestOptions(signal)), - ); +function sidebarNavigationQueryOptions() { + return queryOptions({ + queryKey: sidebarNavigationQueryKey(), + queryFn: async ({ signal }) => { + const response = await request( + apiClient["sidebar-bootstrap"].$get(undefined, requestOptions(signal)), + ); + writeCachedSidebarBootstrap(response); + return response; + }, + initialData: () => readCachedSidebarBootstrap() ?? undefined, + initialDataUpdatedAt: 0, + staleTime: (query) => (query.state.dataUpdatedAt === 0 ? 0 : Infinity), + }); } export function useSidebarNavigation(options?: QueryOptions) { @@ -34,26 +41,17 @@ export function useSidebarNavigation(options?: QueryOptions) { useProjectListRealtimeSubscription({ enabled }); useThreadListRealtimeSubscription({ enabled }); - return useQuery({ - queryKey: sidebarNavigationQueryKey(), - queryFn: async ({ signal }) => { - const response = await fetchSidebarNavigation(signal); - writeCachedSidebarBootstrap(response); - return response; - }, + return useQuery({ + ...sidebarNavigationQueryOptions(), enabled, - ...REALTIME_OWNED_STATIC_CACHE_QUERY_POLICY, - placeholderData: () => readCachedSidebarBootstrap() ?? undefined, }); } export function useProjectDisplayName( projectId: string | undefined, ): string | undefined { - const { data } = useQuery({ - queryKey: sidebarNavigationQueryKey(), - queryFn: ({ signal }) => fetchSidebarNavigation(signal), - ...REALTIME_OWNED_STATIC_CACHE_QUERY_POLICY, + const { data } = useQuery({ + ...sidebarNavigationQueryOptions(), enabled: Boolean(projectId), }); if (!data || !projectId) { @@ -78,10 +76,8 @@ export function useSidebarNavigationThreadSelection( select(listSidebarNavigationThreads(navigation)), [select], ); - const result = useQuery({ - queryKey: sidebarNavigationQueryKey(), - queryFn: ({ signal }) => fetchSidebarNavigation(signal), - ...REALTIME_OWNED_STATIC_CACHE_QUERY_POLICY, + const result = useQuery({ + ...sidebarNavigationQueryOptions(), enabled: false, select: selectFromNavigation, }); diff --git a/apps/app/src/hooks/queries/thread-queries.test.tsx b/apps/app/src/hooks/queries/thread-queries.test.tsx index a7be0d344e8..e4ab23e5fb8 100644 --- a/apps/app/src/hooks/queries/thread-queries.test.tsx +++ b/apps/app/src/hooks/queries/thread-queries.test.tsx @@ -13,6 +13,7 @@ import * as api from "@/lib/api"; import { sdk } from "@/lib/sdk"; import { makeThreadListEntry } from "@bb/test-helpers/domain-fixtures"; import { createQueryClientTestHarness } from "@/test/queryClientTestHarness"; +import { conversationRow } from "@/test/fixtures/thread-timeline-rows"; import { ARCHIVED_THREADS_PAGE_SIZE } from "./archived-threads-page-size"; import { sidebarNavigationQueryKey, @@ -55,7 +56,8 @@ vi.mock("@/lib/api", async (importOriginal) => { }; }); -vi.mock("@/lib/sdk", () => ({ +vi.mock("@/lib/sdk", async (importOriginal) => ({ + ...(await importOriginal()), sdk: { threads: { get: vi.fn(), @@ -776,7 +778,81 @@ describe("useThreadStorageLocation", () => { }); }); -describe("useThreadTimeline segment limit", () => { +describe("useThreadTimeline", () => { + it("reopens an inactive cached page immediately and only replaces it after a successful refresh", async () => { + const cached = makeThreadTimelineResponse({ + rows: [conversationRow({ id: "cached-message", text: "Cached message" })], + maxSeq: 1, + }); + vi.mocked(sdk.threads.timeline).mockResolvedValue(cached); + const { queryClient, wrapper } = createQueryClientTestHarness({ + queries: { gcTime: 5 * 60_000 }, + }); + const usePage = () => ({ + thread: useThread("thread-1"), + bootstrap: useThreadDetailBootstrap("thread-1"), + timeline: useThreadTimeline("thread-1"), + }); + const first = renderHook(usePage, { wrapper }); + await waitFor(() => + expect(first.result.current.timeline.data).toEqual(cached), + ); + await waitFor(() => + expect(first.result.current.thread.isSuccess).toBe(true), + ); + await waitFor(() => + expect(first.result.current.bootstrap.isSuccess).toBe(true), + ); + + vi.useFakeTimers(); + try { + first.unmount(); + await vi.advanceTimersByTimeAsync(6 * 60_000); + let rejectRefresh: (error: Error) => void = () => {}; + vi.mocked(sdk.threads.timeline).mockReturnValue( + new Promise((_resolve, reject) => { + rejectRefresh = reject; + }), + ); + const reopened = renderHook(usePage, { wrapper }); + expect(reopened.result.current.thread.data?.id).toBe("thread-1"); + expect(reopened.result.current.bootstrap.data).toEqual( + THREAD_WITH_INCLUDES, + ); + expect(reopened.result.current.timeline.data).toEqual(cached); + expect(reopened.result.current.timeline.isLoading).toBe(false); + expect(reopened.result.current.timeline.isFetching).toBe(true); + + await act(async () => { + rejectRefresh(new Error("HTTP 503")); + await vi.advanceTimersByTimeAsync(1); + }); + expect(reopened.result.current.timeline.isError).toBe(true); + expect(reopened.result.current.timeline.data).toEqual(cached); + + const newer = makeThreadTimelineResponse({ + rows: [ + ...cached.rows, + conversationRow({ id: "new-message", text: "New message" }), + ], + maxSeq: 2, + }); + vi.mocked(sdk.threads.timeline).mockResolvedValue(newer); + await act(async () => { + await reopened.result.current.timeline.refetch(); + await vi.advanceTimersByTimeAsync(1); + }); + expect(reopened.result.current.timeline.data).toEqual(newer); + reopened.unmount(); + await vi.advanceTimersByTimeAsync(31 * 60_000); + expect( + queryClient.getQueryData(threadTimelineQueryKey("thread-1")), + ).toBeUndefined(); + } finally { + vi.useRealTimers(); + } + }); + it("asks for the compact first window on compact viewports and keeps it for deltas", async () => { mockMatchMedia([COMPACT_VIEWPORT_QUERY]); const { queryClient, wrapper } = createQueryClientTestHarness(); diff --git a/apps/app/src/hooks/queries/thread-queries.ts b/apps/app/src/hooks/queries/thread-queries.ts index 0922143f951..18c9047ac2c 100644 --- a/apps/app/src/hooks/queries/thread-queries.ts +++ b/apps/app/src/hooks/queries/thread-queries.ts @@ -97,6 +97,7 @@ interface QueryOptions { const THREAD_LIST_STALE_TIME_MS = 10_000; const THREAD_SEARCH_STALE_TIME_MS = 10_000; const THREAD_DETAIL_STALE_TIME_MS = 5_000; +const THREAD_PAGE_GC_TIME_MS = 30 * 60_000; const THREAD_MENTION_CANDIDATE_LIMIT = 200; const THREAD_SEARCH_DEBOUNCE_MS = 150; export const THREAD_SEARCH_LIMIT_PER_GROUP = 20; @@ -628,6 +629,7 @@ export function useThread(id: string, options?: QueryOptions) { }), enabled, staleTime: THREAD_DETAIL_STALE_TIME_MS, + gcTime: THREAD_PAGE_GC_TIME_MS, refetchOnMount: options?.refetchOnMount ?? true, retry: shouldRetryTransientReadQuery, retryDelay: TRANSIENT_READ_RETRY_DELAY_MS, @@ -693,6 +695,7 @@ export function useThreadDetailBootstrap( }, enabled, staleTime: Infinity, + gcTime: THREAD_PAGE_GC_TIME_MS, retry: shouldRetryTransientReadQuery, retryDelay: TRANSIENT_READ_RETRY_DELAY_MS, }); @@ -944,6 +947,7 @@ export function useThreadTimeline( return useQuery({ queryKey: threadTimelineQueryKey(id), + gcTime: THREAD_PAGE_GC_TIME_MS, queryFn: async ({ signal }) => { const threadId = requireThreadId(id, "useThreadTimeline"); return fetchThreadTimeline({ diff --git a/apps/app/src/lib/system-config-atoms.local-access.test.ts b/apps/app/src/lib/system-config-atoms.local-access.test.ts index 1f0f077f45d..05d1a3f4769 100644 --- a/apps/app/src/lib/system-config-atoms.local-access.test.ts +++ b/apps/app/src/lib/system-config-atoms.local-access.test.ts @@ -1,6 +1,7 @@ import { createStore } from "jotai"; import { QueryObserver } from "@tanstack/react-query"; import type { ChangedMessage } from "@bb/domain"; +import { createDeferredPromise } from "@bb/test-helpers"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ @@ -43,7 +44,8 @@ vi.mock("./api-host-daemon", () => ({ fetchWorkspaceOpenTargets: vi.fn(async () => []), })); -vi.mock("./sdk", () => ({ +vi.mock("./sdk", async (importOriginal) => ({ + ...(await importOriginal()), sdk: { system: { config: mocks.fetchSdkSystemConfig, @@ -107,6 +109,68 @@ afterEach(() => { }); describe("local host daemon access atoms", () => { + it("serves stale config immediately and applies a background refresh", async () => { + const cached = { hostDaemonPort: 38_887, localHelperPorts: [38_887] }; + const updated = { hostDaemonPort: 38_887, localHelperPorts: [] }; + appQueryClient.setQueryData(systemConfigQueryKey(), cached, { + updatedAt: Date.now() - 120_000, + }); + const refresh = createDeferredPromise(); + mocks.fetchSdkSystemConfig.mockReturnValue(refresh.promise); + const store = createStore(); + const unsubscribe = store.sub(localHostDaemonAccessStateAtom, () => {}); + try { + await expect(store.get(localHostDaemonAccessStateAtom)).resolves.toBe( + "permission-required", + ); + expect(mocks.fetchSdkSystemConfig).toHaveBeenCalledTimes(1); + refresh.resolve(updated); + await vi.waitFor(async () => { + await expect(store.get(localHostDaemonAccessStateAtom)).resolves.toBe( + "unavailable", + ); + }); + } finally { + unsubscribe(); + } + }); + + it("retains stale config after a background refresh fails", async () => { + const cached = { hostDaemonPort: 38_887, localHelperPorts: [38_887] }; + appQueryClient.setQueryData(systemConfigQueryKey(), cached, { + updatedAt: Date.now() - 120_000, + }); + mocks.fetchSdkSystemConfig.mockRejectedValue(new Error("refresh failed")); + const store = createStore(); + await expect(store.get(localHostDaemonAccessStateAtom)).resolves.toBe( + "permission-required", + ); + await vi.waitFor(() => { + expect(appQueryClient.getQueryState(systemConfigQueryKey())?.status).toBe( + "error", + ); + }); + expect(appQueryClient.getQueryData(systemConfigQueryKey())).toEqual(cached); + await expect(store.get(localHostDaemonAccessStateAtom)).resolves.toBe( + "permission-required", + ); + }); + + it("waits on a cache miss and preserves the failed-load fallback", async () => { + const load = createDeferredPromise< + Awaited> + >(); + mocks.fetchSdkSystemConfig.mockReturnValue(load.promise); + const store = createStore(); + const loaded = vi.fn(); + const result = store.get(localHostDaemonAccessStateAtom).then(loaded); + await Promise.resolve(); + expect(loaded).not.toHaveBeenCalled(); + load.reject(new Error("config unavailable")); + await result; + expect(loaded).toHaveBeenCalledWith("unavailable"); + }); + it("shares the system config request with the app query owner", async () => { const store = createStore(); const query = appQueryClient.fetchQuery({ diff --git a/apps/app/src/lib/system-config-atoms.ts b/apps/app/src/lib/system-config-atoms.ts index 558bd5fc098..410a9e1bc12 100644 --- a/apps/app/src/lib/system-config-atoms.ts +++ b/apps/app/src/lib/system-config-atoms.ts @@ -89,7 +89,10 @@ function didLastSystemConfigLoadFail(): boolean { async function loadSystemConfig(): Promise { try { - const config = await appQueryClient.fetchQuery(systemConfigQueryOptions()); + const config = await appQueryClient.ensureQueryData({ + ...systemConfigQueryOptions(), + revalidateIfStale: true, + }); markSystemConfigLoadSucceeded(); return config; } catch { diff --git a/apps/app/src/views/ProjectDetailSettingsView.test.tsx b/apps/app/src/views/ProjectDetailSettingsView.test.tsx index a41a445fe43..ee1850b087a 100644 --- a/apps/app/src/views/ProjectDetailSettingsView.test.tsx +++ b/apps/app/src/views/ProjectDetailSettingsView.test.tsx @@ -1,6 +1,7 @@ // @vitest-environment jsdom import { + act, cleanup, fireEvent, render, @@ -15,6 +16,14 @@ import { sdk } from "@/lib/sdk"; import { createQueryClientTestHarness } from "@/test/queryClientTestHarness"; import { makeSystemConfig } from "@/test/fixtures/system-config"; import { SETTINGS_PROJECT_ROUTE_PATH } from "@/lib/route-paths"; +import { + hostsQueryKey, + sidebarNavigationQueryKey, +} from "@/hooks/queries/query-keys"; +import { + resetSidebarBootstrapCacheForTest, + SIDEBAR_BOOTSTRAP_CACHE_KEY, +} from "@/lib/sidebar-bootstrap-cache"; import { ProjectDetailSettingsView } from "./ProjectDetailSettingsView"; vi.mock("@/lib/sdk", () => ({ @@ -124,8 +133,8 @@ function stubSidebarBootstrapFetch( } function renderView(projectId = "proj_bb") { - const { wrapper } = createQueryClientTestHarness(); - return render( + const { wrapper, queryClient } = createQueryClientTestHarness(); + const view = render( , { wrapper }, ); + return { ...view, queryClient }; } beforeEach(() => { @@ -160,11 +170,32 @@ beforeEach(() => { afterEach(() => { cleanup(); + resetSidebarBootstrapCacheForTest(); + window.localStorage.removeItem(SIDEBAR_BOOTSTRAP_CACHE_KEY); vi.unstubAllGlobals(); vi.clearAllMocks(); }); describe("ProjectDetailSettingsView", () => { + it("keeps the cached project visible when a page refresh fails", async () => { + stubSidebarBootstrapFetch([{ hostId: primaryHost.id, path: "/repos/bb" }]); + const { queryClient } = renderView(); + await screen.findByRole("heading", { name: "bb", level: 1 }); + vi.mocked(sdk.hosts.list).mockRejectedValue(new Error("refresh failed")); + stubSidebarBootstrapFetch([], { status: 503 }); + await act(async () => { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: hostsQueryKey() }), + queryClient.invalidateQueries({ + queryKey: sidebarNavigationQueryKey(), + }), + ]); + }); + expect(queryClient.getQueryState(hostsQueryKey())?.status).toBe("error"); + expect(screen.getByRole("heading", { name: "bb", level: 1 })).toBeTruthy(); + expect(screen.queryByText("Couldn't load this project.")).toBeNull(); + }); + it("keeps checkout counts in sync with the show-all machine toggle", async () => { const sandbox = host({ id: "host_sandbox", @@ -282,7 +313,7 @@ describe("ProjectDetailSettingsView", () => { expect(remove.getAttribute("aria-disabled")).toBe("true"); }); - it("shows derived thread defaults when the project has run threads", async () => { + it("keeps loaded thread defaults when a background refresh fails", async () => { stubSidebarBootstrapFetch([ { hostId: "host_primary", path: "/Users/me/bb" }, ]); @@ -295,11 +326,20 @@ describe("ProjectDetailSettingsView", () => { }; vi.mocked(sdk.projects.defaultExecutionOptions).mockResolvedValue(defaults); - renderView(); + const { queryClient } = renderView(); expect(await screen.findByText("gpt-6-astra")).toBeDefined(); expect(screen.getByText("codex")).toBeDefined(); expect(screen.queryByText(/^No threads have run here yet/u)).toBeNull(); + + vi.mocked(sdk.projects.defaultExecutionOptions).mockRejectedValue( + new Error("refresh failed"), + ); + await act(async () => { + await queryClient.invalidateQueries(); + }); + expect(screen.getByText("gpt-6-astra")).toBeDefined(); + expect(screen.queryByText("Couldn't load thread defaults.")).toBeNull(); }); it("distinguishes a failed defaults load from an empty one", async () => { diff --git a/apps/app/src/views/ProjectDetailSettingsView.tsx b/apps/app/src/views/ProjectDetailSettingsView.tsx index c33d0fbf08e..271b13ec353 100644 --- a/apps/app/src/views/ProjectDetailSettingsView.tsx +++ b/apps/app/src/views/ProjectDetailSettingsView.tsx @@ -292,7 +292,11 @@ export function ProjectDetailSettingsView() { [localSourcePicker, pickerHostId, project], ); - if (sidebarNavigationQuery.isError || hostsQuery.isError) { + if ( + (sidebarNavigationQuery.isError && + sidebarNavigationQuery.data === undefined) || + (hostsQuery.isError && hostsQuery.data === undefined) + ) { return (

@@ -466,7 +470,7 @@ export function ProjectDetailSettingsView() { title="Thread defaults" description={DEFAULTS_DESCRIPTION} > - {defaultsQuery.isError ? ( + {defaultsQuery.isError && defaultsQuery.data === undefined ? (

Couldn't load thread defaults.

diff --git a/apps/app/src/views/SkillsView.test.tsx b/apps/app/src/views/SkillsView.test.tsx index 1643787d3fb..ae0338f1a30 100644 --- a/apps/app/src/views/SkillsView.test.tsx +++ b/apps/app/src/views/SkillsView.test.tsx @@ -2,6 +2,7 @@ import type { ComponentProps } from "react"; import { + act, cleanup, fireEvent, render as renderDom, @@ -93,7 +94,7 @@ function LocationStateProbe() { ); } -function renderLibrarySkillRoute() { +function renderLibrarySkillRoute(skillId = "skill_missing") { vi.spyOn(sdk.providers, "list").mockResolvedValue([]); const fetchMock = vi.fn( async () => @@ -109,9 +110,10 @@ function renderLibrarySkillRoute() { ), ); vi.stubGlobal("fetch", fetchMock); - const { wrapper: QueryClientWrapper } = createQueryClientTestHarness(); + const { wrapper: QueryClientWrapper, queryClient } = + createQueryClientTestHarness(); renderDom( - + } /> @@ -119,7 +121,7 @@ function renderLibrarySkillRoute() { , ); - return fetchMock; + return { fetchMock, queryClient }; } const NO_PROVIDER_ROSTER: ReadonlyMap = new Map(); @@ -254,8 +256,9 @@ function stubRegistryFetch( } function renderRegistrySkillRoute() { - const { wrapper: QueryClientWrapper } = createQueryClientTestHarness(); - return renderDom( + const { wrapper: QueryClientWrapper, queryClient } = + createQueryClientTestHarness(); + const view = renderDom( @@ -269,6 +272,7 @@ function renderRegistrySkillRoute() { , ); + return { ...view, queryClient }; } function NavigateButton({ to, label }: { to: string; label: string }) { @@ -834,6 +838,39 @@ describe("SkillsOverview", () => { }); describe("SkillsLibrary library detail routing", () => { + it("keeps cached file content through a failed refresh and updates on success", async () => { + const skill = makeSkill(); + vi.spyOn(sdk.hosts, "list").mockResolvedValue([]); + vi.spyOn(sdk.skills, "list").mockResolvedValue({ skills: [skill] }); + vi.spyOn(sdk.skills, "listFiles").mockResolvedValue({ + files: ["SKILL.md"], + truncated: false, + }); + const content = vi.spyOn(sdk.skills, "getContent").mockResolvedValue({ + content: "Cached instructions", + revision: "a".repeat(64), + }); + const { queryClient } = renderLibrarySkillRoute(skill.id); + await screen.findByText("Cached instructions"); + + content.mockRejectedValue(new Error("HTTP 503")); + await act(async () => { + await queryClient.invalidateQueries(); + }); + expect(screen.getByText("Cached instructions")).toBeTruthy(); + expect(screen.queryByText("Failed to load SKILL.md.")).toBeNull(); + + content.mockResolvedValue({ + content: "Updated instructions", + revision: "b".repeat(64), + }); + await act(async () => { + await queryClient.invalidateQueries(); + }); + expect(await screen.findByText("Updated instructions")).toBeTruthy(); + expect(screen.queryByText("Cached instructions")).toBeNull(); + }); + it("keeps a detail loading state while the skill library resolves", () => { vi.spyOn(sdk.skills, "list").mockImplementation( () => new Promise(() => {}), @@ -860,7 +897,7 @@ describe("SkillsLibrary library detail routing", () => { it("shows not found on an unknown library skill detail route", async () => { vi.spyOn(sdk.skills, "list").mockResolvedValue({ skills: [] }); - const fetchMock = renderLibrarySkillRoute(); + const { fetchMock } = renderLibrarySkillRoute(); const notFound = await screen.findByText("Skill not found."); expect(notFound.closest("[data-resource-detail-state]")).not.toBeNull(); @@ -870,6 +907,24 @@ describe("SkillsLibrary library detail routing", () => { }); describe("SkillsLibrary registry detail lifecycle", () => { + it("keeps cached source content when a background refresh fails", async () => { + vi.spyOn(sdk.skills, "list").mockResolvedValue({ skills: [] }); + const fetchMock = stubRegistryFetch(makeRegistrySkill()); + const { queryClient } = renderRegistrySkillRoute(); + await screen.findByRole("heading", { name: "SKILL.md" }); + + fetchMock.mockImplementation(async () => new Response(null, { status: 503 })); + await act(async () => { + await queryClient.invalidateQueries(); + }); + expect(screen.getByRole("heading", { name: "SKILL.md" })).toBeTruthy(); + expect( + screen.queryByText( + "This registry skill is no longer available from its source.", + ), + ).toBeNull(); + }); + it("does not offer installation when a direct registry source is unavailable", async () => { const registrySkill = makeRegistrySkill(); vi.spyOn(sdk.skills, "list").mockResolvedValue({ skills: [] }); diff --git a/apps/app/src/views/ToolsView.plugin-detail.test.tsx b/apps/app/src/views/ToolsView.plugin-detail.test.tsx index eb82c4e8194..7cda6fd3aa3 100644 --- a/apps/app/src/views/ToolsView.plugin-detail.test.tsx +++ b/apps/app/src/views/ToolsView.plugin-detail.test.tsx @@ -21,6 +21,7 @@ import { useNavigate, } from "react-router-dom"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { createDeferredPromise } from "@bb/test-helpers"; import { EMPTY_PLUGIN_UPDATE_STATE, type PluginListItem, @@ -43,7 +44,10 @@ import { pluginFrontendDiagnosticRequiresFailureBanner, } from "@/components/tools/PluginDetail"; import type { PluginCatalogSearchEntry } from "@/hooks/queries/plugin-catalog-queries"; -import { pluginSourceQueryKey } from "@/hooks/queries/query-keys"; +import { + pluginListQueryKey, + pluginSourceQueryKey, +} from "@/hooks/queries/query-keys"; import type { PluginFrontendDiagnostic } from "@/lib/plugin-frontend"; import { makeInstalledPlugin, @@ -154,6 +158,85 @@ afterEach(() => { vi.unstubAllGlobals(); }); +describe("plugin detail page cached loads", () => { + it.each([ + { cached: true, fails: false }, + { cached: true, fails: true }, + { cached: false, fails: false }, + { cached: false, fails: true }, + ])( + "loads with cached=$cached and refresh failure=$fails", + async ({ cached, fails }) => { + const response = createDeferredPromise(); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + if (String(input) === "/api/v1/plugins") { + return response.promise; + } + if (String(input).includes("plugin-catalog/search")) { + return Response.json({ results: [], collections: [] }); + } + return Response.json({ error: "not found" }, { status: 404 }); + }), + ); + const { queryClient, wrapper } = createQueryClientTestHarness(); + if (cached) { + queryClient.setQueryData( + pluginListQueryKey(true), + [makeInstalledPlugin({ id: "github", name: "Cached GitHub" })], + { updatedAt: Date.now() - 60_000 }, + ); + } + render( + + + , + { wrapper }, + ); + if (cached) { + expect( + screen.getByRole("heading", { name: "Cached GitHub" }), + ).toBeTruthy(); + expect(screen.queryByText("Loading plugin")).toBeNull(); + } else { + expect(screen.getByText("Loading plugin")).toBeTruthy(); + } + await act(async () => { + response.resolve( + fails + ? Response.json({ error: "refresh failed" }, { status: 503 }) + : Response.json({ + plugins: [ + makeInstalledPlugin({ id: "github", name: "Updated GitHub" }), + ], + }), + ); + }); + if (!fails) { + await screen.findByRole("heading", { name: "Updated GitHub" }); + expect( + screen.queryByRole("heading", { name: "Cached GitHub" }), + ).toBeNull(); + } else { + await waitFor(() => + expect( + queryClient.getQueryState(pluginListQueryKey(true))?.status, + ).toBe("error"), + ); + if (cached) { + expect( + screen.getByRole("heading", { name: "Cached GitHub" }), + ).toBeTruthy(); + expect(screen.queryByText("Couldn't load plugin.")).toBeNull(); + } else { + expect(screen.getByText("Couldn't load plugin.")).toBeTruthy(); + } + } + }, + ); +}); + describe("PluginDetail official catalog lifecycle", () => { it("offers Install from an unowned BB Official plugin detail page", () => { const onInstall = vi.fn(); diff --git a/apps/app/src/views/ToolsView.tsx b/apps/app/src/views/ToolsView.tsx index b740c711794..29cfbdaa5fa 100644 --- a/apps/app/src/views/ToolsView.tsx +++ b/apps/app/src/views/ToolsView.tsx @@ -268,7 +268,7 @@ function PluginDetailToolView({ pluginId }: { pluginId: string }) { ); let detailContent: ReactNode; - if (listQuery.isError) { + if (listQuery.isError && listQuery.data === undefined) { detailContent = ( ); - } else if (catalogQuery.isError) { + } else if (catalogQuery.isError && catalogQuery.data === undefined) { detailContent = ( { if (requestRef.current !== requestId) return; - setState({ automation: null, error: errorText(error) }); + setState((current) => + current.automation !== null + ? current + : { automation: null, error: errorText(error) }, + ); }, ); }, [rpc, projectId, automationId]);