diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index e123b6cc..977673e1 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -550,10 +550,21 @@ export function AgentConfigForm(props: AgentConfigFormProps) { setTestActionPending(false); } }, [selectedCompanyId, isCreate, isDirty, handleSave, testEnvironment]); - const triggerTestEnvironment = useCallback(() => { - if (testEnvironmentDisabled) return; - void runEnvironmentTest().catch(() => undefined); + // `runEnvironmentTest` (and `testEnvironmentDisabled`) change identity on every + // render because `useMutation` returns a fresh result object each time. Hold the + // latest behavior in a ref so the trigger handed to the parent stays referentially + // stable — otherwise the `onTestActionChange` effect below re-runs every render, + // pushing a new function into parent state and causing an infinite update loop. + const triggerRef = useRef<() => void>(() => {}); + useEffect(() => { + triggerRef.current = () => { + if (testEnvironmentDisabled) return; + void runEnvironmentTest().catch(() => undefined); + }; }, [runEnvironmentTest, testEnvironmentDisabled]); + const triggerTestEnvironment = useCallback(() => { + triggerRef.current(); + }, []); useEffect(() => { if (!showAdapterTestEnvironmentButton || !props.onTestActionChange) return; diff --git a/ui/src/components/Layout.tsx b/ui/src/components/Layout.tsx index a03400b1..390b9f44 100644 --- a/ui/src/components/Layout.tsx +++ b/ui/src/components/Layout.tsx @@ -17,6 +17,7 @@ import { MobileBottomNav } from "./MobileBottomNav"; import { WorktreeBanner } from "./WorktreeBanner"; import { DevRestartBanner } from "./DevRestartBanner"; import { StandaloneBrowserControls } from "./StandaloneBrowserControls"; +import { RouteErrorBoundary } from "./RouteErrorBoundary"; import { SidebarShell } from "./SidebarShell"; import { SecondarySidebar } from "./SecondarySidebar"; import { SidebarAccountMenu } from "./SidebarAccountMenu"; @@ -566,7 +567,9 @@ export function Layout() { requestedPrefix={companyPrefix ?? selectedCompany?.issuePrefix} /> ) : ( - + + + )} diff --git a/ui/src/components/RouteErrorBoundary.test.tsx b/ui/src/components/RouteErrorBoundary.test.tsx new file mode 100644 index 00000000..d6195f1e --- /dev/null +++ b/ui/src/components/RouteErrorBoundary.test.tsx @@ -0,0 +1,112 @@ +// @vitest-environment jsdom + +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { RouteErrorBoundary } from "./RouteErrorBoundary"; + +const navigateMock = vi.hoisted(() => vi.fn()); +const routerLocation = vi.hoisted(() => ({ current: { pathname: "/co/agents/new", search: "?adapterType=claude_local" } })); + +vi.mock("@/lib/router", () => ({ + useLocation: () => routerLocation.current, + useNavigate: () => navigateMock, +})); + +function Boom(): never { + throw new Error("Maximum update depth exceeded"); +} + +describe("RouteErrorBoundary", () => { + let container: HTMLDivElement; + let consoleErrorSpy: ReturnType; + + beforeEach(() => { + navigateMock.mockReset(); + routerLocation.current = { pathname: "/co/agents/new", search: "?adapterType=claude_local" }; + container = document.createElement("div"); + document.body.appendChild(container); + // React logs caught render errors to console.error; silence the expected noise. + consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + container.remove(); + }); + + it("renders a recoverable error card instead of a blank page when a child throws", () => { + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + + expect(container.textContent).toContain("This page hit an error"); + expect(container.textContent).toContain("Maximum update depth exceeded"); + expect(container.querySelector("pre")).not.toBeNull(); + + act(() => { + root.unmount(); + }); + }); + + it("navigates back when the user clicks Go back", () => { + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + + const goBack = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Go back", + ); + expect(goBack).toBeDefined(); + + act(() => { + goBack?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(navigateMock).toHaveBeenCalledWith(-1); + + act(() => { + root.unmount(); + }); + }); + + it("recovers and renders children again after the route changes", () => { + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + expect(container.textContent).toContain("This page hit an error"); + + // Simulate back-navigation to a different route, then re-render with a + // healthy child — the boundary should reset off the new resetKey. + routerLocation.current = { pathname: "/co/agents", search: "" }; + act(() => { + root.render( + +
Agents list
+
, + ); + }); + + expect(container.textContent).toContain("Agents list"); + expect(container.textContent).not.toContain("This page hit an error"); + + act(() => { + root.unmount(); + }); + }); +}); diff --git a/ui/src/components/RouteErrorBoundary.tsx b/ui/src/components/RouteErrorBoundary.tsx new file mode 100644 index 00000000..69ab4884 --- /dev/null +++ b/ui/src/components/RouteErrorBoundary.tsx @@ -0,0 +1,73 @@ +import { Component, type ErrorInfo, type ReactNode } from "react"; +import { useLocation, useNavigate } from "@/lib/router"; +import { Button } from "@/components/ui/button"; + +type RouteErrorBoundaryInnerProps = { + resetKey: string; + onReset: () => void; + children: ReactNode; +}; + +type RouteErrorBoundaryState = { + error: Error | null; +}; + +class RouteErrorBoundaryInner extends Component { + override state: RouteErrorBoundaryState = { error: null }; + + static getDerivedStateFromError(error: unknown): RouteErrorBoundaryState { + return { error: error instanceof Error ? error : new Error(String(error)) }; + } + + override componentDidCatch(error: unknown, info: ErrorInfo): void { + console.error("Page render failed", { error, componentStack: info.componentStack }); + } + + override componentDidUpdate(prevProps: RouteErrorBoundaryInnerProps): void { + // A render throw with no boundary unmounts the whole app, so navigating + // away (back button included) can't recover without a hard refresh. We sit + // above the routed , so reset whenever the route changes. + if (this.state.error && prevProps.resetKey !== this.props.resetKey) { + this.setState({ error: null }); + } + } + + override render() { + const { error } = this.state; + if (!error) return this.props.children; + + return ( +
+
+

This page hit an error

+

+ Something went wrong while rendering this page. You can go back and try again, or reload. +

+
+
+          {error.message}
+        
+
+ + +
+
+ ); + } +} + +export function RouteErrorBoundary({ children }: { children: ReactNode }) { + const location = useLocation(); + const navigate = useNavigate(); + const resetKey = `${location.pathname}${location.search}`; + + return ( + navigate(-1)}> + {children} + + ); +}