Admin console: surface failures instead of swallowing them

[stephane-segning]

Closes #309, #310, #311, #312, #313, #314. Does not close #308 — see below; I got that one wrong and have retracted it.

Summary

Seven destructive actions had no error path at all: the mutation's error was never passed to the confirmation, and three of the confirmations had no slot to render one. A failed delete looked exactly like a successful one that hadn't refreshed.

Intent

This is reachable, not theoretical. App.delete is owner-only; OptOut.delete is narrower than OptOut.create, so a role that can record an opt-out cannot remove one. That user clicks Remove, the spinner stops, the row stays, and nothing ever explains why.

It also contradicts the codebase's own stated contract — providers-screen.tsx's module doc says errors are "never swallowed, never silently retried", and InlineConfirm already has an error prop that four sibling call sites use correctly. These seven were the exception.

Scope

issue	fix
#309	7 confirmations surface the real error; 3 components gained errorMessage
#310	Replay's dismissal moved from after mutate() into onSuccess; mutation reset on re-arm
#311	Receipts error checked before the empty state, rendered distinctly
#312	Apps/Users copy now matches the schema (owner/admin roles, not app:write/user:manage)
#313	Three Messages strings said "this app"; the banner above says every app
#314	Simulator: App id marked required, hint added, title on the disabled button

Two deliberate non-changes:

• No onError callbacks. TanStack Query populates mutation.error regardless; the codebase's own working examples (EndpointDeleteConfirm, RouteDeleteConfirm) rely on exactly that. No-op callbacks would be dead code.
• No mutationCache.onError backstop. It fires for every mutation regardless of local handling, so it would double-report on all seven now-fixed sites — an inline banner and a contextless toast for the same failure. It protects only against a future forgotten call site. Reasonable to disagree; flagging as a decision rather than an oversight.
• No Layer-2 permission gates for Admin console copy claims permissions that nothing enforces (app:write, user:manage) #312. Making app:write/user:manage real would change the security model — maintainer's call. It remains the better end state; the copy fix just stops the UI lying meanwhile.

Also fixed in passing: routes-screen's closeMore() reset createMutation/updateMutation but not deleteMutation, so a failed delete on one route showed its stale error when the confirm next opened for a different one. And four module-doc comments still claiming writes "cannot succeed until #194 lands" — both #194 and #211 landed.

#308 — retracted, not fixed

I filed #308 claiming 9 screens render permanently blank. That was wrong, and the full retraction with evidence is on the issue.

React 19's Suspense reveal is requestAnimationFrame-gated; Chromium suspends rAF for a page that never composites. My test browser reported document.hidden: true, rAF never fires, setTimeout unaffected — which is why the app looked alive (React hydrated, tRPC returned data) while one rAF-gated step silently never ran. A real user can't hit it: noticing a page is blank requires looking at it, which foregrounds the tab, which makes rAF fire.

What survives is a real UX weakness rather than a bug, and it's included here on those terms:

• 8 screens: fallback={null} → a real RouteSkeleton. A null fallback makes any slow boundary indistinguishable from a working page with no data — precisely why this cost so long to diagnose.
• dashboard/page.tsx: <Suspense> removed outright. Its own comment already said it "isn't strictly required here", and the screen uses no useSearchParams().

Verification

cargo xtask r6, pnpm --filter admin typecheck, pnpm --filter @vsms/ui typecheck, biome check (285 files) — all clean, on this branch rebased onto current main (which includes #316, touching two of the same files; both sides verified intact).

Honest limitation: none of the error paths was click-tested. These fixes live on /users, /apps, /opt-outs, /webhooks, /messages/[id] — and the only browser available here is the non-compositing one described above, so I can't drive them. Each was verified by tracing the wiring end to end (mutation → prop → the component that renders it), and I confirmed every added prop is actually read by its component rather than accepted and ignored. Nobody has yet watched a real 403 produce a real banner; that needs a human with a visible browser.

One caught during development, worth noting: the first skeleton draft passed a block-level Skeleton into ScreenHeader's <p>-wrapped description slot. Browsers auto-close <p> on a block child, silently splitting it into siblings — a real hydration-mismatch shape, reproduced before it was fixed.

Risk Assessment

Presentation-layer only; no backend, schema or business logic. The realistic failure is a prop accepted but never rendered — which would relocate the silent failure rather than fix it, so that's what I checked per call site. The skeleton change is visual and reversible.

AI Usage Declaration

• A human directed this change and is accountable for it.
• Claims verified against the repository and a running toolchain. Where verification wasn't possible (live click-testing), it's stated rather than implied.

Reviewer Focus

The mutationCache.onError decision — declining it means this class can recur silently, which is exactly how these seven arose. And #312's copy: I'd rather it be checked against schemas/vsms.cstack directly than trusted, since replacing one wrong claim with another is the specific risk there.

---

Checklist

• docs/roadmap.md checked — no edit needed; no milestone, gate, dependency or decision changes.
• No R1 exceptions touched.

🤖 Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: a29dae8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[lightbridge-assistant]

🅵 Fast automated pass — SAST + a quick, diff-scoped look (no repo-wide retrieval). For a deeper, repo-aware review, mention @lightbridge-assistant on this PR.

All 34 changed files opened and reviewed (none left unopened). This is a UI-safety/UX PR: RouteSkeleton Suspense fallbacks across eight pages (component verified against Skeleton/ScreenStack and its props at every call site), error display threaded through confirm dialogs (all feeding verified InlineConfirm error/pending and InlineBanner props), a correct webhooks replay refactor (#310) and routes deleteMutation.reset() stale-error fix, a safe dashboard Suspense removal (confirmed no useSearchParams), and comment-only reachability-doc edits in the gateway/api routers (no code or security surface change). One P2 stands: opt-outs-screen (and the same newly-added rotate/replay error displays) renders a shared mutation-hook error never reset on dialog close, so a failed action leaks its error onto the next, different target — the same stale-error class this PR fixes in routes-screen. No P0/P1, no security findings.

---

🤖 AI-generated review — treat it as untrusted, verify before acting; a human owns the final decision (AI governance).

[lightbridge-assistant]

Stale delete error shown on next target

RemoveConfirmDialog now renders deleteMutation.error?.message, but deleteMutation is never reset (checked closeRemoveConfirm/onOpenChange — they only clear urlState.remove). A failed remove leaves the error set; if the user cancels and re-opens Remove for a different opt-out, the stale error renders against a row it never applied to — the exact stale-error-on-shared-hook pattern this PR fixes in routes-screen.tsx via deleteMutation.reset() on close. Mirror that here (reset in closeRemoveConfirm or the dialog's onCancel/onOpenChange). Same latent pattern applies to the newly-added rotate/replay error displays in webhooks-screen.tsx (neither rotateMutation nor deleteMutation is reset on cancel/dismiss).

Was this useful? React 👍/👎 to give us feedback

[stephane-segning]

Reviewed by verifying the claims against the tree rather than reading the description. Recommend merge.

Verified

"Seven destructive actions had no error path" — exactly seven, confirmed by count, not approximately:

users/users-screen.tsx                           2
apps/apps-screen.tsx                             2
sender-ids/components/resubmit-confirm.tsx       1
webhooks/components/attempt-replay-confirm.tsx   1
webhooks/components/endpoint-rotate-confirm.tsx  1
                                          TOTAL  7

The reachability argument is real, not theoretical. Checked the schema directly:

@@allow("create", hasRole('owner') || hasRole('admin') || hasRole('operator') || hasRole('system'))
@@allow("delete", hasRole('owner') || hasRole('admin'))

OptOut.delete genuinely is narrower than create — an operator can record an opt-out and then cannot remove it. That is a user hitting a silent failure on a permission the same screen granted them a moment earlier.

Completeness: after the change every InlineConfirm call site carries an error path except gallery/components/nested-dialog-in-drawer-regression.tsx, which correctly has none — it is a static demo with no mutation. No site was missed.

All gates pass on the branch: pnpm biome ci . (393 files), typecheck (admin + @vsms/ui), 129 tests, 23/23 routes, cargo xtask r6.

The #310 fix is the best thing here

Moving the dismissal from synchronously-after-.mutate() into onSuccess is the obvious half. The part worth calling out is that it then followed the consequence: now that a failed replay's error survives, a stale error would appear the next time the confirm is armed — so it resets the mutation on re-arm, and the comment explains that this is a residual of its own fix rather than an unrelated tidy.

Fixing the second-order effect of your own fix is the thing that usually gets missed.

The #308 retraction — independently corroborated

Retracting your own filed issue with a mechanism rather than a hedge is the right call, and I can confirm the diagnosis from the other direction: I hit the identical artifact in this same repo today. My browser harness reported viewport 0x0, elementFromPoint returning null on an element with a real bounding rect, and screenshots rendering black — while JS, the DOM and the network were all completely healthy.

That is the same root cause: a browser that never composites suspends requestAnimationFrame, and anything rAF-gated silently never runs. Worth keeping in AGENTS.md — it will produce another confident, wrong bug report otherwise, and the tell (setTimeout fine, rAF dead, page otherwise alive) is not obvious.

One inaccuracy

InlineConfirm already has an error prop that four sibling call sites use correctly

It is three, not four: register-provider-confirm.tsx, endpoint-delete-confirm.tsx, route-delete-confirm.tsx. Immaterial to the argument — the point that a working pattern already existed and these seven were the exception stands — but the number is in the PR body and someone will check it.

On the deliberate non-changes

The mutationCache.onError reasoning is right and worth preserving: a global backstop fires regardless of local handling, so it would double-report on all seven now-fixed sites — an inline banner and a contextless toast for one failure. Flagging it as a decision rather than an oversight is the correct treatment.

Not adding Layer-2 gates for #312 is also right. Making app:write/user:manage real changes the security model; the copy fix stops the UI lying in the meantime without pre-empting that call.