Repository navigation
fix(ui): work over plain http, and catch render crashes at the boundaries - #2865
Conversation
`crypto.randomUUID` is only defined in a secure context, so every page that minted an id with it threw when the app was served over plain http from anything but localhost — the new-schedule page did it at mount, which blanked the page. One `randomId()` helper now serves every call site; it returns a v4 UUID because some of these ids are validated as UUIDs server-side. Signed-off-by: Nicholas Bucher <[email protected]>
…sion slot A thrown component used to take the whole page with it. Three boundaries now contain one: react-router's `errorElement` on a pathless route nested inside the shell, so a page crash keeps the nav; a theme-free class boundary above the providers for anything that fails before the app can style itself; and one per extension contribution, so a broken extension drops out instead of the page. Signed-off-by: Nicholas Bucher <[email protected]>
|
Note Outdated. Superseded by the full reel. Schedules over plain http, and a crashed page keeping the shellreel.mp4🤖 written by Claude |
This comment has been minimized.
This comment has been minimized.
Nothing has rendered when the stylesheet, branding or mock-backend step throws, so the boundary above cannot see it and the user gets the blank page this branch exists to remove. The fallback moves out of the boundary so both can render it, and the route boundary gains the test for its own claim: a page that throws is replaced, the shell around it is not. Signed-off-by: Nicholas Bucher <[email protected]>
…t fails `navigator.clipboard` is undefined over plain http, so the optional call was a silent no-op under an alert reading "Copy this link now — it cannot be shown again": the button did nothing and the token was gone. `copyText` falls back to `execCommand`, and the dialog reports a failure instead of implying a copy. The textarea it borrows goes inside the open dialog, not on the body. A modal traps focus, so one outside it takes no selection and `execCommand` then returns true for having copied nothing. Signed-off-by: Nicholas Bucher <[email protected]>
The reload button said what the browser already offers and told the reader nothing. In its place the message and stack sit behind a disclosure, as raw text, so whoever hits this can read or copy what actually happened. The fallback renders above the theme provider, so it carries its own palette and reads the mode the user chose from storage, falling back to the system preference. Both themes. Signed-off-by: Nicholas Bucher <[email protected]>
|
Note Outdated. Superseded by the full reel, which covers this and the other three fixes. The root error boundary, dark and lightreel2.mp4🤖 written by Claude |
There was a problem hiding this comment.
This automated review is a first pass. A manual review will follow once the blockers are addressed.
Nothing blocks merge. Each fix was checked on a LAN IP against a live cluster — a non-secure context — rather than reasoned about.
Two findings are worth taking first, both verified in a browser. crypto.getRandomValues is not secure-context gated, so randomId can use real randomness. antd already ships the copy technique this reimplements, though its copyable UI reports success unconditionally and cannot be used as-is.
✅ What I checked
- ✅ On
http://<LAN-IP>:crypto.randomUUIDundefined,getRandomValuesreturns bytes. - ✅ New-schedule page mounts and takes input there; on
mainit throws. - ✅ Share link reaches the clipboard on both origins — clipboard poisoned first, read back from a secure page.
- ✅ No
crypto.randomUUIDor barenavigator.clipboardleft insrc/. - ✅ No
randomIdcaller needs unguessable randomness; share tokens are minted by the controller. - ✅
RouteErrorBoundaryhas its Emotion and antd context on every route. - ✅ Reverting either boundary fix fails its test.
- ✅ 165 e2e, 524 unit.
- ✅ The reported
TestRuntimeRevisionLifecyclefailure is a Go cluster test; this diff is allui/.
A module that throws while being evaluated is still uncovered: main.tsx never runs, so no boundary exists yet.
🤖 written by Claude
…tform `getRandomValues` carries no secure-context condition — only `randomUUID` and `subtle` do — so the ids are properly random for the same line count. The copy fallback now answers the document's own `copy` event rather than selecting a borrowed textarea, which is what antd does and what removes the need to know where the focus trap is. Success is the event having fired, not what `execCommand` returns, since it returns true for copying nothing. Also: a rejected copy reports failure rather than leaving the button silent, a missing root element no longer throws inside the handler that renders the failure, and the retry button's comment says what it does. Signed-off-by: Nicholas Bucher <[email protected]>
|
Note Outdated. Superseded by the current reel; the route boundary gained its disclosure after this was recorded. All four fixes, before the route boundary disclosurereel3.mp4🤖 written by Claude |
…IdNoCrypto Signed-off-by: Nicholas Bucher <[email protected]> # Conflicts: # ui/src/mocks/transport.ts
The page boundary showed one line and dropped the rest, so a crash mid-session told the reader less than the console did. It now carries the same disclosure the root fallback has, in theme tokens, with the formatter shared between them. `storedMode` moves to its own module so reading the theme costs no import of the provider the root fallback renders above. Signed-off-by: Nicholas Bucher <[email protected]>
`crypto.randomUUID` was fixed once in #1868 and came back when the interface was rewritten and the file holding the helper went with it. A note in a file is lost the same way twice, so the rule states it where a rewrite cannot drop it, and names the helper to use instead. `navigator.clipboard` is listed for the same reason: bare, it fails silently rather than loudly. Signed-off-by: Nicholas Bucher <[email protected]>
|
All four fixes at reel4.mp4🤖 written by Claude |
…ries (kagent-dev#2865) Signed-off-by: alexdarbyshire <[email protected]> --- *🤖 written by Claude (start)* ## Changelog The UI now works when served over plain http from anything but localhost, and one component throwing no longer blanks the page. ## Testing 1. `cd ui && yarn dev --host`, then open the printed **Network** URL — a LAN IP, not localhost, so not a secure context. 2. `/schedules/new` loads and takes input. On `main` it throws `crypto.randomUUID is not a function` at mount and stays blank. 3. Share a conversation and press **Copy link**. The button reports whether it worked; on `main` it silently does nothing here. 4. Put `crypto.randomUUID()` back in `ScheduledRunForm.tsx` and reload. The error renders inside the shell, nav usable, stack behind **Error details**. ## Additional Notes This reinstates kagent-dev#1868, which fixed the same crash in May. That fix lived in `ui/src/lib/utils.ts` and went with the interface rewrite. The lint rule is the part meant to outlast the next one. `randomId()` returns a v4 UUID because the control plane validates some of these ids as UUIDs. It uses `crypto.getRandomValues`, which carries no secure-context condition. The clipboard fallback answers the document's own `copy` event, as antd does. antd's `copyable` UI is not used: it reports a copy whatever `copy()` returns. Not covered: a module that throws while being evaluated. `main.tsx` never runs, so no boundary exists yet. --- *🤖 written by Claude (end)* --------- Signed-off-by: Nicholas Bucher <[email protected]>


🤖 written by Claude (start)
Changelog
The UI now works when served over plain http from anything but localhost, and one component throwing no longer blanks the page.
Testing
cd ui && yarn dev --host, then open the printed Network URL — a LAN IP, not localhost, so not a secure context./schedules/newloads and takes input. Onmainit throwscrypto.randomUUID is not a functionat mount and stays blank.mainit silently does nothing here.crypto.randomUUID()back inScheduledRunForm.tsxand reload. The error renders inside the shell, nav usable, stack behind Error details.Additional Notes
This reinstates #1868, which fixed the same crash in May. That fix lived in
ui/src/lib/utils.tsand went with the interface rewrite. The lint rule is the part meant to outlast the next one.randomId()returns a v4 UUID because the control plane validates some of these ids as UUIDs. It usescrypto.getRandomValues, which carries no secure-context condition.The clipboard fallback answers the document's own
copyevent, as antd does. antd'scopyableUI is not used: it reports a copy whatevercopy()returns.Not covered: a module that throws while being evaluated.
main.tsxnever runs, so no boundary exists yet.🤖 written by Claude (end)