fix(ui): fix blank page when creating an agent (#8336)
## 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 `<Outlet />` in
`<RouteErrorBoundary>` 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
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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}
|
||||
/>
|
||||
) : (
|
||||
<Outlet />
|
||||
<RouteErrorBoundary>
|
||||
<Outlet />
|
||||
</RouteErrorBoundary>
|
||||
)}
|
||||
</main>
|
||||
<PropertiesPanel />
|
||||
|
||||
@@ -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<typeof vi.spyOn>;
|
||||
|
||||
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(
|
||||
<RouteErrorBoundary>
|
||||
<Boom />
|
||||
</RouteErrorBoundary>,
|
||||
);
|
||||
});
|
||||
|
||||
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(
|
||||
<RouteErrorBoundary>
|
||||
<Boom />
|
||||
</RouteErrorBoundary>,
|
||||
);
|
||||
});
|
||||
|
||||
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(
|
||||
<RouteErrorBoundary>
|
||||
<Boom />
|
||||
</RouteErrorBoundary>,
|
||||
);
|
||||
});
|
||||
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(
|
||||
<RouteErrorBoundary>
|
||||
<div>Agents list</div>
|
||||
</RouteErrorBoundary>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(container.textContent).toContain("Agents list");
|
||||
expect(container.textContent).not.toContain("This page hit an error");
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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<RouteErrorBoundaryInnerProps, RouteErrorBoundaryState> {
|
||||
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 <Outlet />, 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 (
|
||||
<div className="mx-auto max-w-2xl space-y-4 px-4 py-10">
|
||||
<div>
|
||||
<h1 className="text-lg font-semibold">This page hit an error</h1>
|
||||
<p className="mt-1 text-sm text-muted-foreground">
|
||||
Something went wrong while rendering this page. You can go back and try again, or reload.
|
||||
</p>
|
||||
</div>
|
||||
<pre className="overflow-auto rounded-md border border-destructive/30 bg-destructive/10 px-3 py-2 text-xs text-destructive whitespace-pre-wrap">
|
||||
{error.message}
|
||||
</pre>
|
||||
<div className="flex items-center gap-2">
|
||||
<Button variant="outline" size="sm" onClick={this.props.onReset}>
|
||||
Go back
|
||||
</Button>
|
||||
<Button size="sm" onClick={() => window.location.reload()}>
|
||||
Reload page
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export function RouteErrorBoundary({ children }: { children: ReactNode }) {
|
||||
const location = useLocation();
|
||||
const navigate = useNavigate();
|
||||
const resetKey = `${location.pathname}${location.search}`;
|
||||
|
||||
return (
|
||||
<RouteErrorBoundaryInner resetKey={resetKey} onReset={() => navigate(-1)}>
|
||||
{children}
|
||||
</RouteErrorBoundaryInner>
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user