PaperClipAI

Jannik/PaperClipAI

0 Fork 0

mirror of https://github.com/paperclipai/paperclip.git synced 2026-10-28 12:04:41 +01:00

Commit Graph

Author	SHA1	Message	Date
Devin FoleyandPaperclip	fff410dfe7	fix(ui): retain bounded context for browser render errors (#13904 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Its browser error boundaries recover from failed renders and report the exception when error monitoring is enabled. > - The boundaries already receive the React component stack, but discard it before reporting the error. > - A minified DOM insertion error can therefore lack enough context to identify the affected component. > - Browser translation can replace text nodes that React still uses as insertion anchors. > - This pull request preserves bounded component names and browser state for the next failure. > - Maintainers can locate the failed component without collecting page content or customer URLs. ## Linked Issues or Issue Description Related: #13719 attributes browser errors to the loaded release. #13784 supplies the browser environment. This change adds context to error-boundary reports. What happened? A browser error boundary reports a DOM `NotFoundError` with only a minified JavaScript stack. The React component trace is logged to the console, which production builds remove. Browser translation is a plausible cause, but the report cannot identify the component or confirm the translation marker. Expected behavior An error-boundary report includes a bounded component trace and limited browser state. It excludes component props, text, HTML, element IDs, arbitrary CSS classes, page URLs, and query strings. Steps to reproduce 1. Render a component with conditional content before a text node. 2. Replace that text node with a translation element outside React. 3. Enable the preceding conditional content. React tries to insert before the detached text node and throws `NotFoundError`. 4. The boundary shows its recovery UI, but the old report loses the component trace. Paperclip version or commit Based on `6681c71b40`. The regression test reproduces the DOM mutation with the installed React version. Deployment mode Built browser UI with optional Sentry monitoring enabled for the signed-in session. ## What Changed - Pass the component stack and boundary kind from both error boundaries. - Keep at most 40 component names from at most 16 KiB of stack input. Drop locations and unrecognized lines. - Snapshot document readiness, visibility, and the browser translation root-class marker before the asynchronous reporting queue runs. - Attach diagnostics to that event only. Preserve the existing monitoring gate, sign-out behavior, and original exception if diagnostics fail. - Preserve function names in production bundles. Test the actual Vite production pipeline. - Document the fields, privacy limits, and translation-marker limitations. ## Verification - Focused diagnostics, boundary, real Sentry SDK, and production-build tests: 49 passed. - The translation DOM-mutation test reproduces `NotFoundError`, verifies the failed component trace, and keeps the recovery UI usable. - The real SDK test checks emitted events, private fixture exclusion, event isolation, and no capture after sign-out. Its transport stays in-process. - `pnpm check:token-gates`: passed. - [Greptile review](https://github.com/paperclipai/paperclip/pull/13904#issuecomment-5804220318): 5/5 on `92a2a23d9c`, with no review threads. - `pnpm -r typecheck` and `pnpm build`: passed. - Full CI unit and integration test matrix: passed, including all server, chat, workspace, runner, and serialized suites. - Local `pnpm test:run` was started, then stopped after the equivalent full CI matrix passed. The unsharded local run was not completed and is not claimed as a pass. - `pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/project-repositories.spec.ts --repeat-each=3 --trace=on --reporter=line`: six tests passed against the production build. - Initial CI browser shard 8 timed out after the repository form replaced an enabled Save button with a disabled one before the click. The retained page snapshot shows the original repository selection. No error-boundary fallback appeared. [CI attempt 2](https://github.com/paperclipai/paperclip/actions/runs/35929990870/attempts/2) passed the failed jobs on the same commit. The full PR check matrix is green. This confirms an intermittent failure, but does not establish the cause of the first failure. - Full browser builds with and without name preservation passed. The initial JavaScript chunk grows from 1,571,919 to 1,668,453 gzip bytes (+6.1%). Total JavaScript across all chunks grows by 219,610 gzip bytes (+5.3%). ## Risks - This adds diagnostics. It reproduces a translation failure mode but does not identify or repair the specific application component from a past report. - The root-class marker is a hint. Other translation tools may omit it, and its presence does not prove causation. - Name preservation increases bundle size as measured above. No source maps are published by this change. - The error is still reported. DOM operations, browser translation, and recovery behavior are unchanged. ## Model Used OpenAI GPT-6 through Codex, with reasoning, code editing, terminal tools, and test execution. The exact serving model identifier and context-window size are not exposed in this session. ## 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] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-09-23 16:23:42 -07:00
Nicky LeachandPaperclip	8f1e3cfe24	feat(observability): add opt-in Sentry error monitoring for the server and the browser (#12190 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server and the browser need clear error reports when an operator enables external monitoring. > - Paperclip already uses an opt-in OpenTelemetry pattern for server traces. > - Sentry can provide error reports for both runtime paths when the operator sets one data source name. > - This pull request adds one opt-in Sentry gate for the server and the browser. > - The benefit is faster diagnosis while the default setup sends no Sentry data. ## Linked Issues or Issue Description What is improved? Paperclip gains optional error monitoring for server and browser failures. Subsystem affected Cross-cutting (server, UI, and shared authentication data). Current behavior Paperclip has no built-in Sentry error capture for server failures or browser boundary failures. Operators must inspect local logs and browser tools. Proposed behavior When the operator sets `SENTRY_DSN`, the server and authenticated browser use the same Sentry project. When the variable is absent, both paths stay inactive. The server loads Sentry dynamically and fails open when the optional package is absent. Reason and benefit Operators can inspect runtime errors in one Sentry project. The default setup remains local and sends no monitoring data. Breaking changes None when `SENTRY_DSN` remains unset. Authenticated session responses add the optional `sentryDsn` field. Additional context The implementation uses built-in Sentry privacy options. It disables default HTTP context and breadcrumb integrations and keeps `sendDefaultPii` false. ## What Changed - Add an opt-in server Sentry gate with dynamic package loading and fail-open behavior. - Add the Sentry data source name to the authenticated session response. - Add an authenticated browser Sentry gate and React error boundary capture. - Add tests for server, browser, route, and application error paths. - Document activation, installation, privacy settings, capture behavior, and operator controls. ## Verification - Run `npx vitest run server/src/__tests__/sentry.test.ts`. - Run `npx vitest run ui/src/lib/sentry.test.ts`. - Run `npx vitest run server/src/__tests__/auth-routes.test.ts server/src/__tests__/shutdown.test.ts`. - Confirm that the full continuous integration suite passes on this pull request. - Leave `SENTRY_DSN` unset and confirm that the server and browser gates stay inactive. - Set `SENTRY_DSN` and install the optional Sentry packages before a manual capture check. ## Risks The operator controls the Sentry project and accepts the data risk when the operator enables the feature. Error objects can contain messages, stacks, or cause chains with private values. The default configuration sends no data because the feature stays off without `SENTRY_DSN`. A missing optional server package does not stop server boot. ## Model Used OpenAI Codex, GPT-5, with tool use, repository inspection, GitHub CLI operations, and code review support. ## 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] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-08-26 11:15:45 -07:00
Devin Foley	8f4b491d9a	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	2026-06-19 13:06:41 -07:00

Author

SHA1

Message

Date

Devin FoleyandPaperclip

fff410dfe7

fix(ui): retain bounded context for browser render errors (#13904 )

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Its browser error boundaries recover from failed renders and report
the exception when error monitoring is enabled.
> - The boundaries already receive the React component stack, but
discard it before reporting the error.
> - A minified DOM insertion error can therefore lack enough context to
identify the affected component.
> - Browser translation can replace text nodes that React still uses as
insertion anchors.
> - This pull request preserves bounded component names and browser
state for the next failure.
> - Maintainers can locate the failed component without collecting page
content or customer URLs.

## Linked Issues or Issue Description

Related: #13719 attributes browser errors to the loaded release. #13784
supplies the browser environment. This change adds context to
error-boundary reports.

**What happened?**

A browser error boundary reports a DOM `NotFoundError` with only a
minified JavaScript stack. The React component trace is logged to the
console, which production builds remove. Browser translation is a
plausible cause, but the report cannot identify the component or confirm
the translation marker.

**Expected behavior**

An error-boundary report includes a bounded component trace and limited
browser state. It excludes component props, text, HTML, element IDs,
arbitrary CSS classes, page URLs, and query strings.

**Steps to reproduce**

1. Render a component with conditional content before a text node.
2. Replace that text node with a translation element outside React.
3. Enable the preceding conditional content. React tries to insert
before the detached text node and throws `NotFoundError`.
4. The boundary shows its recovery UI, but the old report loses the
component trace.

**Paperclip version or commit**

Based on `6681c71b40`. The regression test reproduces the DOM mutation
with the installed React version.

**Deployment mode**

Built browser UI with optional Sentry monitoring enabled for the
signed-in session.

## What Changed

- Pass the component stack and boundary kind from both error boundaries.
- Keep at most 40 component names from at most 16 KiB of stack input.
Drop locations and unrecognized lines.
- Snapshot document readiness, visibility, and the browser translation
root-class marker before the asynchronous reporting queue runs.
- Attach diagnostics to that event only. Preserve the existing
monitoring gate, sign-out behavior, and original exception if
diagnostics fail.
- Preserve function names in production bundles. Test the actual Vite
production pipeline.
- Document the fields, privacy limits, and translation-marker
limitations.

## Verification

- Focused diagnostics, boundary, real Sentry SDK, and production-build
tests: 49 passed.
- The translation DOM-mutation test reproduces `NotFoundError`, verifies
the failed component trace, and keeps the recovery UI usable.
- The real SDK test checks emitted events, private fixture exclusion,
event isolation, and no capture after sign-out. Its transport stays
in-process.
- `pnpm check:token-gates`: passed.
- [Greptile
review](https://github.com/paperclipai/paperclip/pull/13904#issuecomment-5804220318):
5/5 on `92a2a23d9c`, with no review threads.
- `pnpm -r typecheck` and `pnpm build`: passed.
- Full CI unit and integration test matrix: passed, including all
server, chat, workspace, runner, and serialized suites.
- Local `pnpm test:run` was started, then stopped after the equivalent
full CI matrix passed. The unsharded local run was not completed and is
not claimed as a pass.
- `pnpm exec playwright test --config tests/e2e/playwright.config.ts
tests/e2e/project-repositories.spec.ts --repeat-each=3 --trace=on
--reporter=line`: six tests passed against the production build.
- Initial CI browser shard 8 timed out after the repository form
replaced an enabled Save button with a disabled one before the click.
The retained page snapshot shows the original repository selection. No
error-boundary fallback appeared. [CI attempt
2](https://github.com/paperclipai/paperclip/actions/runs/35929990870/attempts/2)
passed the failed jobs on the same commit. The full PR check matrix is
green. This confirms an intermittent failure, but does not establish the
cause of the first failure.
- Full browser builds with and without name preservation passed. The
initial JavaScript chunk grows from 1,571,919 to 1,668,453 gzip bytes
(+6.1%). Total JavaScript across all chunks grows by 219,610 gzip bytes
(+5.3%).

## Risks

- This adds diagnostics. It reproduces a translation failure mode but
does not identify or repair the specific application component from a
past report.
- The root-class marker is a hint. Other translation tools may omit it,
and its presence does not prove causation.
- Name preservation increases bundle size as measured above. No source
maps are published by this change.
- The error is still reported. DOM operations, browser translation, and
recovery behavior are unchanged.

## Model Used

OpenAI GPT-6 through Codex, with reasoning, code editing, terminal
tools, and test execution. The exact serving model identifier and
context-window size are not exposed in this session.

## 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] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

Co-authored-by: Paperclip <noreply@paperclip.ing>

2026-09-23 16:23:42 -07:00

Nicky LeachandPaperclip

8f1e3cfe24

feat(observability): add opt-in Sentry error monitoring for the server and the browser (#12190 )

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The server and the browser need clear error reports when an operator
enables external monitoring.
> - Paperclip already uses an opt-in OpenTelemetry pattern for server
traces.
> - Sentry can provide error reports for both runtime paths when the
operator sets one data source name.
> - This pull request adds one opt-in Sentry gate for the server and the
browser.
> - The benefit is faster diagnosis while the default setup sends no
Sentry data.

## Linked Issues or Issue Description

**What is improved?**

Paperclip gains optional error monitoring for server and browser
failures.

**Subsystem affected**

Cross-cutting (server, UI, and shared authentication data).

**Current behavior**

Paperclip has no built-in Sentry error capture for server failures or
browser boundary failures. Operators must inspect local logs and browser
tools.

**Proposed behavior**

When the operator sets `SENTRY_DSN`, the server and authenticated
browser use the same Sentry project. When the variable is absent, both
paths stay inactive. The server loads Sentry dynamically and fails open
when the optional package is absent.

**Reason and benefit**

Operators can inspect runtime errors in one Sentry project. The default
setup remains local and sends no monitoring data.

**Breaking changes**

None when `SENTRY_DSN` remains unset. Authenticated session responses
add the optional `sentryDsn` field.

**Additional context**

The implementation uses built-in Sentry privacy options. It disables
default HTTP context and breadcrumb integrations and keeps
`sendDefaultPii` false.

## What Changed

- Add an opt-in server Sentry gate with dynamic package loading and
fail-open behavior.
- Add the Sentry data source name to the authenticated session response.
- Add an authenticated browser Sentry gate and React error boundary
capture.
- Add tests for server, browser, route, and application error paths.
- Document activation, installation, privacy settings, capture behavior,
and operator controls.

## Verification

- Run `npx vitest run server/src/__tests__/sentry.test.ts`.
- Run `npx vitest run ui/src/lib/sentry.test.ts`.
- Run `npx vitest run server/src/__tests__/auth-routes.test.ts
server/src/__tests__/shutdown.test.ts`.
- Confirm that the full continuous integration suite passes on this pull
request.
- Leave `SENTRY_DSN` unset and confirm that the server and browser gates
stay inactive.
- Set `SENTRY_DSN` and install the optional Sentry packages before a
manual capture check.

## Risks

The operator controls the Sentry project and accepts the data risk when
the operator enables the feature. Error objects can contain messages,
stacks, or cause chains with private values. The default configuration
sends no data because the feature stays off without `SENTRY_DSN`. A
missing optional server package does not stop server boot.

## Model Used

OpenAI Codex, GPT-5, with tool use, repository inspection, GitHub CLI
operations, and code review support.

## 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] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>

2026-08-26 11:15:45 -07:00

Devin Foley

8f4b491d9a

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

2026-06-19 13:06:41 -07:00