From 8f4b491d9aff0c2b38ce457a55a1919d6335a481 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 19 Jun 2026 13:06:41 -0700 Subject: [PATCH] fix(ui): fix blank page when creating an agent (#8336) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Creating an agent starts at **New Agent → "manually" → pick an adapter**, which routes to `/{company}/agents/new?adapterType=claude_local` and renders the `NewAgent` page with the `AgentConfigForm` > - `AgentConfigForm` hands the parent a `triggerTestEnvironment` callback via an `onTestActionChange` effect so the page can wire up its "Test"/"Save + Test" button > - That trigger was rebuilt on every render: it depended on `runEnvironmentTest`, which is derived from a react-query `useMutation` result, and `useMutation` returns a **brand-new result object identity on every render** > - So the `onTestActionChange` effect re-fired every render and pushed a new function into the parent's state, producing an infinite `setState` loop ("Maximum update depth exceeded") that threw during render > - The app's custom router updates location **without remounting**, and there was no error boundary around the routed outlet, so the throw left a dead render tree — a fully **blank page** that stayed blank on back-navigation until a hard refresh > - This pull request stabilizes the trigger with a latest-ref pattern so the effect no longer re-fires, and adds a route-keyed error boundary so any future render throw degrades to a recoverable error card instead of a blank screen > - The benefit is that creating an agent works again, and render-time failures anywhere in the routed UI are contained and recoverable rather than silently blanking the app ## Linked Issues or Issue Description No public GitHub issue exists, so the underlying bug is described inline following the bug report template (`.github/ISSUE_TEMPLATE/bug_report.yml`): ### What happened? In the UI, choosing **New Agent → "manually" → (any adapter, e.g. Claude)** navigates to the agent-config page and renders a **completely blank page**. The browser console shows React's `Maximum update depth exceeded`. Using the back button changes the URL but the page stays blank until a full hard refresh. ### Expected behavior Selecting an adapter shows the agent configuration form so the agent can be created. ### Steps to reproduce 1. Open the app and click **New Agent**. 2. Choose **manually**. 3. Pick an adapter (e.g. Claude / `claude_local`). 4. Observe the blank page (URL becomes `/{company}/agents/new?adapterType=claude_local`). ### Paperclip version or commit Reproduces on `master` (base of this PR). ### Deployment mode Reproduces regardless of deployment mode — it is a client-side render loop. ### Root cause `useMutation` returns a new result object identity each render, so the `runEnvironmentTest`-derived `triggerTestEnvironment` callback was unstable, which made the `onTestActionChange` effect push a new function into parent state every render → infinite update loop → render throw → no boundary → blank tree. ## What Changed - **`ui/src/components/AgentConfigForm.tsx`** — Stabilize the environment-test trigger handed to the parent using a latest-ref pattern: the churny behavior (`runEnvironmentTest`, `testEnvironmentDisabled`) lives in a `useRef` updated by an effect, and the exposed `triggerTestEnvironment` is a `useCallback(() => triggerRef.current(), [])` with an empty dep array, so its identity is stable across renders and the `onTestActionChange` effect no longer re-fires every render. - **`ui/src/components/RouteErrorBoundary.tsx`** (new) — A route-keyed React error boundary that catches render throws and renders a recoverable error card (showing the error message, with "Go back" and "Reload page" actions). It resets automatically when the route (`pathname + search`) changes. - **`ui/src/components/Layout.tsx`** — Wrap the routed `` in `` so a render throw degrades to the error card instead of a blank page. - **`ui/src/components/RouteErrorBoundary.test.tsx`** (new) — Regression test: a throwing child is contained as a recoverable error card (showing the message), "Go back" calls `navigate(-1)`, and the boundary resets to render children again after the route changes. ## Verification - `npx vitest run ui/src/components/RouteErrorBoundary.test.tsx` → 3 passed (regression test for this fix). - `npx vitest run ui/src/components/AgentConfigForm.test.ts` → 9 passed. - `npx tsc -b` in `ui` → 0 errors. - Manual: ran the dev server, clicked **New Agent → manually → Claude**. - **Before:** blank page; console logs `Maximum update depth exceeded`; back button leaves the page blank until hard refresh. - **After:** the agent configuration form renders normally and the agent can be created; navigating away and back works without a hard refresh. - Boundary check: with the loop still in place (pre-fix), the new boundary catches the throw and shows a recoverable error card instead of a blank screen; "Go back" / route change resets it. _Screenshots: the before-state is the React `Maximum update depth exceeded` error and a blank `/agents/new` page; the after-state is the rendered agent-config form. Both were observed locally; rendered images can be attached on request._ ## Risks - **Low risk.** Changes are confined to three UI files with no API, schema, or behavioral change to agent creation beyond fixing the loop. - The latest-ref pattern preserves identical runtime behavior of the test trigger (same guard, same `runEnvironmentTest()` call) — it only stabilizes the callback identity. - The error boundary is additive; on the happy path it renders its children unchanged. Its only behavior is to catch render throws that previously blanked the app. ## Model Used Claude (Anthropic), model `claude-opus-4-8` — extended-thinking-capable, tool-use (file edit, shell, tests). Used to diagnose the infinite render loop, implement the latest-ref fix and route error boundary, and verify via local typecheck/tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (described textually in Verification — see note) - [ ] I have updated relevant documentation to reflect my changes (N/A — bug fix, no docs affected) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ui/src/components/AgentConfigForm.tsx | 17 ++- ui/src/components/Layout.tsx | 5 +- ui/src/components/RouteErrorBoundary.test.tsx | 112 ++++++++++++++++++ ui/src/components/RouteErrorBoundary.tsx | 73 ++++++++++++ 4 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 ui/src/components/RouteErrorBoundary.test.tsx create mode 100644 ui/src/components/RouteErrorBoundary.tsx 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} + + ); +}