mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
2e38649341815fa7f58195e2fa60e2e89e72e4ce
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
73526559b5 |
fix(ui): reload once when a lazy chunk or its CSS fails to load (#17349)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The board UI loads each page as a lazy chunk. Vite adds a preload
step that also loads the CSS of the chunk.
> - Staging error monitoring reported `Unable to preload CSS for
…/assets/issue-detail-CIbQo9zW.css`. The same file name comes from the
current build, so the CSS did not change. One request for the file
failed for a short time.
> - Vite rejects only the first import that asks for a failed file.
Later imports skip that file. On a task page, the first import is the
task-page prefetch, which ignores errors. The page then shows without
its CSS. On other pages that share the CSS, such as Agents, the lazy
route rejects and the route error boundary shows the error.
> - Neither case recovers without a manual reload. This is a routine
retry that the app can do safely, so the app must do it.
> - This pull request reloads the page one time when a chunk or its CSS
fails to load. A loop guard limits this to one reload in five minutes.
> - The benefit is that a stale tab after a deploy, or one failed asset
request, recovers without an error page, without missing styles, and
without error-monitoring noise. An asset that stays broken still shows
the error and still sends a report.
## Linked Issues or Issue Description
**What happened?**
A board page showed "This page hit an error" with `Unable to preload CSS
for <origin>/assets/issue-detail-<hash>.css`. On a task page, the same
failure gave no error message, but the page showed without its
stylesheet.
**Steps to reproduce**
1. Build the UI and start the server.
2. Make the first request for `/assets/issue-detail-*.css` fail. The new
Playwright spec does this with `page.route(...).abort("failed")`.
3. Open `/<prefix>/agents`. The route error boundary shows "Unable to
preload CSS".
4. Open `/<prefix>/issues/<id>`. The task shows, but the browser does
not request the task CSS again. The page has no task-page CSS.
**Expected behavior**
The app recovers from a chunk or CSS load failure without user action.
If the failure continues, the app shows the error one time, with no
reload loop.
**Environment**
Hosted staging browser. The local build of current `master` reproduces
it.
I searched open and closed PRs for `preload`, `preloadError`, and `stale
chunk`. I found no related PR.
## What Changed
- `ui/src/lib/chunk-load-recovery.ts` (new): `isChunkLoadError` matches
the Vite CSS preload error and the dynamic import errors from Chromium,
Firefox, and WebKit. `reloadForChunkLoadError` reloads one time. A
`sessionStorage` marker blocks a second reload for five minutes. After
the first reload, later chunk errors in the same document are part of
that reload, so they return "reloading" and do not show an error.
Without `sessionStorage`, the function does not reload, because it
cannot guard against a loop. `startChunkLoadRecovery` listens for
`vite:preloadError`. It does not cancel the event, so the import still
rejects as before.
- `ui/src/main.tsx`: Starts the listener before the first render. The
listener covers every import, including the task-page prefetch that
ignores errors.
- `RouteErrorBoundary` and `AppErrorBoundary`: While a recovery reload
runs, they show a loading state. They do not show the error page and do
not report to Sentry. Other errors work as before.
- Tests: Vitest unit tests for the helper and both boundaries. A new
Playwright spec, `tests/e2e/chunk-load-recovery.spec.ts`, runs against
the real server and the built UI.
## Verification
- `pnpm --filter @paperclipai/ui exec vitest run
src/lib/chunk-load-recovery.test.ts
src/components/RouteErrorBoundary.test.tsx
src/components/AppErrorBoundary.test.tsx`: 21 tests pass.
- Full UI Vitest suite: 793 files, 9,075 tests pass.
- `pnpm --filter @paperclipai/ui typecheck` passes. `node
scripts/check-token-gates.mjs` is clean. `node scripts/check-maps.mjs`
passes.
- `npx playwright test --config tests/e2e/playwright.config.ts
chunk-load-recovery.spec.ts`: 3 tests pass.
- Task page with one failed CSS request: one reload, the CSS loads, and
the task shows.
- Agents page with one failed CSS request: one reload, the CSS loads,
and the page shows with no error page.
- CSS that always fails: one reload, then the error page shows. There is
no second reload.
- Red proof: I ran the same spec with the `ui/src` changes removed
(current `master` UI code). Both recovery tests fail. On Agents, the
route error boundary shows `Unable to preload CSS for
http://127.0.0.1:3199/assets/issue-detail-CIbQo9zW.css`. On the task
page, the CSS is never requested again.
- A failure I found and fixed during the work: my first version changed
only the error boundaries. The task-page e2e case failed with that
version, because the prefetch received the error and the boundary never
saw it. The `vite:preloadError` listener fixes that case. All results
above are from the final commit.
- The spec blocks service workers, because Playwright page routes cannot
see requests that a service worker sends. With a worker, a failed
network request for an asset that is not cached also gives
`Response.error()`. Thus the preload error is the same.
## Risks
- A page can reload while a user is on it. This occurs only after a
chunk or CSS load failure, and at most one time in five minutes per tab
session. Without this change, that page already shows an error or shows
without styles. Composer drafts use browser storage, so a reload keeps
them.
- A failure that recovers no longer sends a Sentry event. A failure that
continues after the reload still sends one, so a broken deploy stays
visible.
- The error patterns match browser and Vite message prefixes. If a
browser changes its message, that browser goes back to the current
behavior: the error page and a report.
- No API, schema, or server changes.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) by Anthropic, run as a Paperclip
agent through Claude Code with tool use (shell, file edits, Playwright,
and Vitest).
## 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
- [ ] 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>
|
||
|
DottaandPaperclip
|
fa50cab568 |
perf(ui): speed task loading without shifting the conversation (#16849)
## Thinking Path > - Paperclip's board should open a task quickly while keeping its conversation and composer stable. > - The browser currently downloads unrelated screens and unopened dialog implementations before task content appears. > - Splitting routes alone creates many shared requests and did not improve measured cold loading. > - This change groups the startup and task dependency trees, preloads the task chunk, and defers optional routes and dialogs. > - Existing coordinated history reveal stays intact so late data does not insert content above the reader. > - Hidden routes must not overwrite saved scroll positions with zero-height geometry. > - Repeated browser measurements and delayed-response tests establish the result independently of bundle-file size. ## Linked Issues or Issue Description Refs #14758. Related prior work: #10463 and #14727. **What existing behavior does this improve?** Cold task-page opening downloads and evaluates code for other screens and globally mounted, unopened dialogs. A direct task link should reach its saved conversation sooner without moving already-visible messages or the composer. **Subsystem affected** Board UI startup, route boundaries, dialog loading, task scroll restoration, and the issue-detail performance harness. **Proposed behavior** Load optional screens and dialogs when needed, keep startup requests bounded, and preload the task route. Preserve the current history reveal policy. Keep the shell mounted during route loading and preserve the reader's position when a route is temporarily hidden. **Additional context** The older bundle-splitting attempt in #10463 is still open from August. This implementation targets current master and deliberately preserves today's editor, composer, and transcript reveal behavior. It does not implement the alternative transcript-row design discussed in #14758. ROADMAP.md does not describe overlapping startup-bundle work. ## What Changed - Lazy-load board routes, standalone screens, the legacy layout, onboarding, and unopened create dialogs. - Keep activated dialogs mounted across close/reopen to retain their existing draft lifecycle. - Group startup and issue dependencies and preload the emitted issue chunk from the production HTML. - Contain route loading within the shell and retain existing chunk-error recovery. - Contain dialog import failures locally, support cancellation, and keep setup dismissed after a late download. Warm the task module from task lists and keep the small chat redirect eager. - Wait for task-creation dialog fields in the browser fixture before filling an explicit title. - Ignore zero-height scroll geometry while a route is hidden. - Reuse the same persisted performance fixture between builds, record actual network spans, and distinguish concealed loading content from the ready thread in browser assertions. ## Verification - Production UI build and UI TypeScript check passed. - Recovery follow-up: 74 focused UI tests passed; the cancellation change passed 23 focused tests plus all 131 onboarding tests (the latter with a 15-second per-test timeout on the contended local host). - Setup cancellation was reproduced against the prior production build and verified fixed after releasing the held real wizard download. - 91 focused UI tests passed; the final route registrations were checked again after making standalone screens lazy (33 routing tests passed). - All eight real-browser layout scenarios passed: delayed comments, late media, properties changes, growing drafts, older history, Back navigation, deep links, desktop/mobile same-task links, request failure/retry, and stalled native/log reads. - Browser walkthrough: read a populated task, submit one reply, refresh, navigate to Projects, and return. The reply persisted once and the normal composer remained usable. - Token design gates passed. The separate repository-wide publish-token scan fails on pre-existing references to the local maintainer username in unchanged files; this change adds none. - Production-build measurements use the same persisted task, local API/database, fresh browser contexts for cold opens, and five samples per scenario/profile. These measurements do not establish deployed staging latency. - Final head `076466816a`, consecutive before/after production builds: five samples per scenario/profile, same saved task and server. Cold Fast 4G/4x CPU content paint: **5,775 → 4,248 ms (26%)**; issue API server timing was 23 ms in both sets. Warm Fast 4G: 989 → 971 ms. JavaScript transferred before content: **2,596 → 1,722 KiB (34%)**. - Unthrottled cold content paint: 1,306 → 676 ms; warm: 269 → 224 ms. These local timings vary with host load. Earlier measurements showed about 20% cold improvement, and a contended intermediate run had substantially higher API latency; those runs are retained in the local evidence. The final matched pair meets the proposed 25% target, but it does not establish a guaranteed 25% improvement across environments. - All four browser scenarios that failed on the first CI run passed locally after the readiness fixes; the subsequent CI run passed all eight browser shards. - Fault-injection browser checks: delayed route loading preserves shell geometry; a failed route recovers through Back; cancelled or failed dialog loading preserves an unsent reply; Back dismisses pending setup and contains its late failure. - Final head `076466816a`: all CI gates green (55 successful checks, no pending or failed checks); Greptile 5/5 with no actionable findings and all review threads resolved. ## Risks - Dynamic chunks add a loading state on first entry to optional screens and dialogs. Route failures use the existing error boundary. Optional dialog failures are dismissible without unmounting the board; retrying a rejected import requires a reload. - The task preload prioritizes the primary board journey, including when another screen is opened first. - Chunk grouping should be revisited when the dependency graph changes; bundle size alone does not establish a speedup. - No API, authorization, migration, editor semantics, or transcript-reveal policy changes. ## Model Used OpenAI Codex, GPT-6, with reasoning, repository tools, code execution, and browser verification. The runtime does not expose a more precise model identifier or context-window size. ## 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> |
||
|
DottaandPaperclip
|
ccbfefef5f |
test(ui): retire 33 duplicate executions and share render setup (#16072)
Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
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> |
||
|
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> |
||
|
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
|