feat(admin): port the dashboard views onto the embedded SPA bundle - #508
Merged
Merged
Conversation
ADR-0003 item 4 in two steps; this is the first. The React bundle now carries the whole operator dashboard — upstream status, the grouped accounts-and-usage table, both add-account flows, both store tables, and managed pool health. Nothing is removed: `GET /admin` still serves the server-rendered page, so this change is additive and a default build is unaffected. Flipping that route and deleting the string literals is the follow-up. Two per-session values the server-rendered page interpolates into itself cannot reach a static bundle: the session's CSRF token and `claude::auth::EXPIRY_BUFFER`. The shell is one file embedded at compile time and served, unauthenticated, to every visitor alike, so it knows nothing about the request that fetched it. `GET /admin/api/session` serves both, authenticated like the rest of that namespace. Returning a CSRF token over a GET is safe here because there is no CORS layer on the admin router — a cross-origin page can send the request but cannot read the reply — and the integration test proves the served token is the one `check_csrf` accepts rather than a decorative copy. The refresh buffer is served rather than copied into TypeScript for a different reason: the dashboard calls a setup token expired once it is inside that buffer and routing refuses one on the same boundary, so a duplicated constant could drift. The views arrive with a behavioral suite (`ui/`, vitest + Testing Library) that renders components and asserts on what an operator sees. That replaces what the string-literal dashboard could support — substring matches against emitted JavaScript, which cannot tell a guard that runs from one that is merely present. Every property in it was checked by deleting the guard it covers and confirming the test fails; one was vacuous on the first pass, because a `disabled` attribute blocked the click before the guard behind it could run, and now drives two clicks inside one task instead. CI runs `npm test` in `ui/`, so the suite is a gate rather than a local convenience.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request ports the admin operator dashboard from server-rendered Rust string literals to an embedded Single Page Application (SPA) built with React, TypeScript, and Vite. It introduces a new GET /admin/api/session endpoint to bootstrap the SPA session with the CSRF token and session expiry buffer, and adds comprehensive frontend tests using Vitest and Testing Library. The review feedback highlights two key improvements: robustly handling non-JSON responses in readJson to preserve HTTP status codes (especially for 401 redirects), and preventing test flakiness in Rust integration tests by using unique environment variable names and a drop guard for process-global environment variables.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8369cb6759
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 43 files
Architecture diagram
sequenceDiagram
participant Browser
participant AdminRouter as Admin Router (src/admin/mod.rs)
participant Session as Session Bootstrap (GET /admin/api/session)
participant UI as SPA Bundle (ui/src/)
participant API as Data Endpoints (/admin/api/*)
participant Stores as Token Stores & Pool
Note over Browser,Stores: SPA Dashboard Runtime Flow
Browser->>AdminRouter: GET /admin/ui (shell, unauthenticated)
AdminRouter-->>Browser: 200 HTML shell + external JS/CSS
Browser->>UI: Load React bundle from /admin/assets/*
UI->>Session: GET /admin/api/session (with cookie)
Session->>Session: Authenticate session or header credential
alt Cookie session
Session-->>UI: 200 { csrf: token, expiry_buffer_ms: 300000 }
else Header credential
Session-->>UI: 200 { csrf: "", expiry_buffer_ms: 300000 }
else Unauthenticated
Session-->>UI: 401
UI->>Browser: Redirect to /admin/login
end
UI->>API: GET /admin/api/observed, GET /admin/api/accounts, GET /admin/api/pool, GET /admin/api/accounts/codex, GET /admin/api/status
API-->>UI: Provider observations, store metadata, pool health, upstream status
Note over UI: Coalesce managed pool + local observations by UUID<br/>into one row set (accounts.ts)
UI->>UI: Render dashboard (usage first, pool management collapsed)
Note over UI,Stores: Store Mutation Flow (ADD / REMOVE / REFRESH)
UI->>API: POST /admin/api/accounts/claude { name, mode } + x-csrf-token
API->>Stores: Persist store mutation
Stores-->>API: Acknowledge
API-->>UI: Mutation verdict
UI->>API: Re-read ALL tables (observed, store, pool) after any mutation
API-->>UI: Fresh data — grouped table reflects new needs_relogin / coalesced state
Note over UI: Sign-out (scripted POST for CSP form-action 'none')
UI->>API: POST /admin/api/logout (same-origin fetch, no CSRF needed)
API-->>UI: 303
UI->>Browser: Redirect to /admin/login
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…s failure
The port routed every mutation through one new `mutate()` helper, which wraps
the body parse in its own try/catch. `src/admin/script.rs` does not: its
`removeAccount`/`refreshAccount` read the body through `.catch(() => ({}))`,
while `completeClaude`/`completeCodex` use a bare `await res.json()` so an
unreadable answer falls to the handler's outer catch — which re-reads the
tables and says the account may still have been stored.
Giving every caller the tolerant form lost that. A completion whose answer the
page could not parse — a proxy's error page in front of the admin surface, a
truncated body — reported a definite "Failed to complete" and skipped the
refresh, on a single-use authorization code: an operator was sent to retry an
exchange that may already have stored the account. On a 200 the same body
reported "Account stored" for an outcome nothing had actually read.
`mutate()` now reports whether it could read the answer as a JSON object, and
only the completion path consumes it, so remove and refresh keep the tolerance
the original gave them on purpose. A start that cannot be read is reported too,
rather than leaving the authorization step silently unopened.
Also here:
- pin `PoolHealth`'s state precedence with tests. Its `poolState` is a second
implementation of the ordering `effectiveState` already has tests for, and a
comment marks the `needs_relogin`-before-cooldown step as deliberate; nothing
covered it, so inverting it stayed green.
- pass the dependency array `useImperativeHandle` in `AddClaudeAccount` was
missing, matching `AddCodexAccount`.
- `cleanup_reprovisioned_pool_health` is defined in `src/admin/mod.rs`;
`src/admin/codex.rs` only calls it. The wrong citation came over from
`script.rs:364` verbatim.
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
This was referenced Sep 10, 2026
- `readJson` keeps the HTTP status when the body is not JSON. `App` routes a 401 to `/admin/login` off that status, so a proxy answering with an HTML error page dropped the redirect and left the operator on the loading state. The transport failure keeps returning no status, because there is none. - The pool re-read in the mutation test asserted nothing. It compared `/admin/api/pool` against the `/admin/api/observed` baseline, which cubic caught; giving it its own baseline was still not enough, because `reloadObserved` fetches `/admin/api/pool` as well, so that endpoint's call count rises whether or not `reloadPool` ran. Only `reloadPool` writes the loadable the pool table renders from, so the assertion now reads the table: the fixture changes the plan between reads, and a row still showing the old one never re-read. Deleting `reloadPool` fails it, which neither earlier version did. - Docs: `useDashboard` has four sequenced reads plus a one-shot `[server.status]` read, not five guarded ones; and both `--features ui` catch-alls are registered with `get`, so non-GET answers 405 rather than falling through. Declined as parity with `src/admin/script.rs`, which `GET /admin` still serves, and filed instead: overlapping cooldown notes (#511), pool state precedence across the two tables (#512), and the add-account form staying interactive during an open flow (#513). Also filed #514: the repo's cubic default flags make the CLI exit 0 with an empty issue list without reviewing anything.
Contributor
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request ports the server-rendered admin dashboard to a React/TypeScript SPA under ui/, introducing a new GET /admin/api/session endpoint to bootstrap session-specific values. It also adds extensive frontend and backend test coverage. Feedback on these changes highlights a concurrency risk in tests/admin_surface.rs when modifying process-global environment variables, suggesting the use of a global Mutex and a drop guard for safe cleanup. Additionally, the accountGroups function in ui/src/accounts.ts violates the style guide's 50 LOC limit, and extracting helper functions is recommended to improve maintainability.
I answered the earlier review threads on this test by saying the file isolates env-setting tests by giving each its own variable name. That was wrong — it also has locks, and their doc comments say what they are for: `CLAUDE_ENV_LOCK` and `CODEX_ENV_LOCK` serialize tests that share `SHUNT_CLAUDE_*`/`SHUNT_CODEX_*` names, and the admin one serializes admin tests "because their config resolves process environment", which is what this test does through `token_env`. A distinct name only rules out a collision. It does not stop this test's `set_var` from landing while a concurrent test is reading the env to build its own config, and that read/write race is what the lock is there for. So the test now takes it. The lock's name said OIDC while its reason had nothing to do with OIDC, so it is `ADMIN_ENV_LOCK` now, with the reason spelled out. Its three existing users are unchanged apart from the name. Not adopted from the same threads, both of which cut against the file's own practice: a `std::process::id()` suffix (each integration test binary is already its own process, so the name is unique without it) and a drop guard in place of the final `remove_var` (93 manual removals here, no drop guards — and dropping the manual call without adding a guard would leave no cleanup at all).
|
amondnet
added a commit
that referenced
this pull request
Sep 11, 2026
…string literals (#526) * feat(admin)!: serve the dashboard from the SPA bundle and delete the string literals Completes ADR-0003 item 4. PR #508 ported the views onto the embedded bundle but left `GET /admin` on the server-rendered page so the two could be diffed against each other line for line; that comparison is done, so the literals go. Deleted: `src/admin/script.rs` (646 lines) and `html::dashboard_page` with its 15 source-text tests (`html.rs` 887 -> 150). Those tests matched emitted JavaScript source, so they could never distinguish a guard that runs from one merely present; `ui/`'s suite renders components and asserts on what an operator sees. `html.rs` keeps the login page, which stays server-rendered on purpose — a sign-in screen cannot depend on a bundle only a signed-in session is meant to reach (`admin-ui-delivery.md`, Resolution 6). BREAKING CHANGE: `GET /admin` answers `200` with the SPA shell instead of `303 /admin/login` when the caller is unauthenticated. The shell is one static file embedded at compile time and identical for every visitor, so it carries no operator data and needs no credential; the redirect did not disappear but moved into the bundle, which follows a `401` from `GET /admin/api/session` to the same page. A script that treated the `303` as "not signed in" must read the bootstrap endpoint instead. Without `--features ui` there is no bundle, and `/admin` answers `404` with a body naming the feature. Resolution 1 already accepts that a from-source build has no dashboard; what it does not license is an empty body, which is what dropping the route would leave — indistinguishable from an unconfigured `[server.admin]`, and the two have different fixes. Release binaries and the Homebrew formula build with the feature, so this gap is from-source only. Two test assertions moved to `/admin/api/session`, where they belong now that `/admin` is unauthenticated: "the OIDC callback minted a usable session" and "logout invalidated it" were both reading a status that no longer says anything about the caller's cookie. Both would have gone on passing while testing nothing. Also lands the three items #508 deferred into this PR: - the empty-usage copy test, the one of those 15 properties not carried forward. Each of its four branches names a different operator action, and nothing else on the page distinguishes them. - a settle-race in the `ui/` harness. `Upstream status` renders `null` — not a "Loading…" row — until its read resolves, so the existing wait was already satisfied while the section was still pending; every assertion about it would have raced the fetch. `renderDashboard` now waits for the section itself, and the new test holds that one read open so the wait is load-bearing. - `accountGroups` (106 lines -> 45) split into named helpers. It was left matching `script.rs`'s coalescing block line for line because the port review depended on that correspondence; with `script.rs` deleted there is nothing to correspond to. Closes #504: the README's admin row now says the dashboard needs a `--features ui` build, and the Install section notes that its own `cargo install` line does not produce one. Mirrored into the ko/ja/zh-CN READMEs and the four locale copies of the endpoints reference. `cargo test` both with and without `--all-features`, `cargo clippy --all-targets --all-features -- -D warnings`, `cargo fmt --all --check`, and `ui/` (50 tests, tsc, build) all pass. The two new `ui/` properties and the harness fix were each checked by deleting the guard and confirming the test fails. * fix(admin): correct the login-page rationale and close the docs the cutover falsified In-house review findings on the cutover commit. Four of the five are places where `--features ui` newly became load-bearing and a doc did not notice. The comment finding is the one worth stating plainly, because I got it backwards. `html.rs`'s module doc — and `ui/README.md` after it — said the login page stays server-rendered "because a sign-in page cannot depend on a bundle only a signed-in session is meant to reach". The bundle is not session-gated at all: the shell is served unauthenticated, which this very PR says three other times (`ui.rs`, `dashboard`, Decision 5). The real reason is availability, not authentication — the bundle exists only in a `--features ui` build, and an admin surface whose *sign-in* page vanished with the feature would be unusable rather than merely dashboard-less. Resolution 6 was also miscited: it keeps `/admin/login` and `/admin/oidc/callback` on their paths because they are pages rather than JSON with somewhere to move to, and says nothing about bundles. The rest follow from `GET /admin` now needing the feature, which it never did before — the server-rendered dashboard answered there in any build: - `site/.../guides/admin-remote-provisioning.mdx` (+ ko/ja/zh-cn) is the guide whose whole subject is this surface, and step 1 is "Open `/admin`". A reader on a from-source build got an unexplained 404. Adds the build note, linking each locale's own installation page. - The README's `[server.status]` row promised "Statuspage indicators in the dashboard and as a metric". On a default build only the metric half is now true. Reworded in all four READMEs. - `docs/desktop-app.md` justified pre-authenticating the webview with "`/admin` redirects to `/admin/login`", which it no longer does. The app's need is unchanged but its reason moved into the bundle, and the doc now also says the sidecar has to be a `--features ui` build. The test finding: `tests/router_surface.rs`'s three `#[cfg(not(feature = "ui"))]` arms — including the one this PR added for the default build's `/admin` — never ran in CI, because the only `cargo test` in `ci.yml` passes `--all-features`, which compiles them out. Adds a step running that one test binary with default features. Scoped to the binary rather than a second full suite: the marginal cost is this crate compiled once more, not every test binary rebuilt. Two findings were not acted on. The security review's note that the login page keeps `script-src 'unsafe-inline'` is already #525 and deliberately out of this PR. The silent-failure review's note that `renderDashboard`'s new wait would time out opaquely for a future test that supplies `fixtures.status` *and* forces the read to fail describes a test that does not exist, and the file already documents why that test is not written. Gates re-run: `cargo fmt --all --check`, `cargo clippy --all-targets --all-features -- -D warnings` (0), `cargo test --test router_surface` (14, the new default-feature path), the three admin test binaries under `--all-features` (45 + 11 + 13), and `ui/` (50). * docs(admin): correct the mount-fallback and unauthenticated-route claims Applies five cubic review findings on #526, plus two from the local gpt/ocr finders. - ui/README.md said every other path under the /admin mount serves the bundle. /admin/login, /admin/oidc/callback and /admin/api/* are routes of their own, and /admin/ is registered nowhere -- an axum wildcard must match at least one character, so it 404s. - The endpoints reference listed only the two wildcards as served without admin authentication, contradicting its own GET /admin row one screen above. Bare /admin joins the list, and the --features ui gating clause now names the two wildcard routes, since /admin is registered in both builds. All four locales. - Three .claude/agent-memory notes carried claims this PR falsified: a CI blind spot that ci.yml's default-build step closes, two doc gaps that 7fdaaf9 closed, and an overbroad no-leftover-references grep claim. * docs(memory): split the html_body hit count by directory The count covered `src/` and `tests/` together, which read as ambiguous. It is 4 in `src/` and 1 in `tests/admin_ui.rs`, now stated separately.
This was referenced Sep 11, 2026
amondnet
added a commit
that referenced
this pull request
Sep 11, 2026
…m misreporting a login (#530) * fix(admin): converge the dashboard state ladders and stop the add form misreporting a login Three defects split out of PR #508, where each was declined because that port had to keep the React page identical to the server-rendered original. That original is gone (#526), so the TypeScript under `ui/` is the only implementation left and the "must not diverge" constraint no longer applies. `statusNote` tested the account-wide and Fable cooldowns in sequence and returned on the first, so an account cooling both ways showed only the account-wide deadline while the pool table listed both. The active notes are now joined with ` · ` the way `cooldownText` already joins them; a single cooldown renders exactly as before, and neither renders as before. `poolState` and `managedState` disagreed about whether `near_quota` or the account-wide cooldown wins, so one account could read "near quota" in the pool table and "Cooling" in the accounts table. They converge on `managedState`'s ordering — cooldown first — because an account-wide cooldown is the fact that *all* of this account's traffic is gated right now, while `near_quota` is a threshold warning about what is coming; reporting the warning while hiding the active gate is the defect. Only `poolState` changes. `disabled` → `needs_relogin` → `!has_state` stay ahead of every cooldown/quota state in both, which is what keeps a permanently dead credential distinguishable from a pause. The add-account form left the login-method radios live after Start, so it could read "Setup token" over a pending OAuth login the server had already fixed; both mode inputs are now closed while `authorizeUrl` is non-null. `start` also left the previous authorization step on screen and clickable, with its Complete button posting to the name captured for that earlier flow; it now clears `authorizeUrl` and `currentName` up front. The epoch mechanism, the `complete` path, and the `completingNow` marker are untouched. Closes #511 Closes #512 Closes #513 * fix(admin): explain why the login-method radios lock, and document it Review follow-up on the #513 half of this branch. Disabling the mode radios when the authorization step opens is the right fix, but a `disabled` attribute states no reason: nothing in the DOM said why the choice went dead, and the form's own live region is empty at exactly that moment, because `start` clears it and a successful start never sets one. The radio group now carries `aria-describedby` — the mode help text it had never referenced, plus a `role="status"` note naming the lock, which announces itself as it appears. Both stylesheets gain a disabled-input rule so the dimming is not left to the browser default (`ui/src/index.css` mirrors `html::STYLE`). The lock was also undocumented. `docs/m9-admin-surface.md` records it beside the flow-epoch reasoning it belongs with, together with the start-clears-the- step half of #513, and the remote-provisioning guide gains a sentence in all four locales beside the existing "the server records the selected mode with the pending attempt" guarantee the lock enforces in the UI. * fix(admin): hold the login-method lock across the start request Two review findings, each raised independently by more than one reviewer. `start` cleared the authorization step but not the code that belonged to it. `complete` clears the code only when the exchange succeeds, so a failed one leaves it in the box, and the next flow submits it against a pending entry it was never issued for — which fails on a state mismatch, blaming the operator's fresh paste for a stale one. `start` clears it now, the way `prime` already did. (gemini-code-assist, cubic) The mode lock had the same shape of gap as the bug it was added for. `start` nulls `authorizeUrl` before it sends, so a lock keyed on that alone reopened the radios for exactly the length of the request that had already captured `mode`: the operator could switch method mid-request and be handed an authorize link for the other one, with the form then locking around the wrong answer. The flow exposes a `starting` flag instead, true from the moment the request is issued, and the form locks on `flow.starting || authorizeUrl`. A superseded start does not release it — `prime` and the newer start each set it as they take over, so the flag always belongs to whoever owns the epoch. (greptile P1, cubic P2, chatgpt-codex-connector P1) Both tests fail if their fix is reverted: dropping `flow.starting` from `locked` reds the in-flight test alone, and dropping `setCode('')` reds the stale-code test alone. * fix(admin): stop the lock note promising a release it cannot give Round two of review findings. The note said "Start another login to change it", and with the lock now spanning the whole flow that is not reachable: a replacement Start re-arms `starting` before any render, so there is no frame in which the radios are selectable. The note and the guide now name the two things that do release the choice — completing the flow, or reloading the page. Whether the form should offer an explicit way to abandon an open flow is a separate question, tracked in #531. (chatgpt-codex-connector P2) `start`'s catch reported a bare `'Request failed'`. Two reviewers asked for `copy.startFailure` instead; that string is `'Failed to start'`, the message for a rejection the server *answered*, and the catch is the case where it answered nothing. This hook already distinguishes the two on the completion path, so the literal becomes a documented module constant beside `UNKNOWN_COMPLETION` rather than folding into the caller's rejection wording. The other `'Request failed'` sites are in files this change does not touch. (gemini-code-assist MEDIUM, cubic P2 — applied as an alternative) `docs/m9-admin-surface.md` also records why the lock is keyed on `starting || authorizeUrl` and why a superseded start does not release it. * style(admin): drop an unneeded :has() from the disabled-choice rule `.choice:has(input:disabled) span` becomes `.choice input:disabled ~ span`. The span is a following sibling of the input inside `.choice`, so the two select the same elements and the sibling form needs no `:has()`. Applied for the simpler selector rather than for the reason given: the reviewer cited `:has()` browser support, but the rule two lines down (`.choice:has(input:focus-visible)`) predates this PR and already requires it, so a browser without `:has()` loses the focus ring either way. Both mirrored stylesheets change together, per the note on `html::STYLE`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ADR-0003 sequences the UI platform track in four items. Items 1–3 landed as #497, #499 and #503. Item 4 — "port the existing views and delete the dashboard string literals" — ships in two PRs, and this is the first: the port, with nothing deleted.
Why two PRs
The deletion is 1,525 lines of Rust string literal and the 15 tests that guarded it. Splitting means that deletion happens only after the React port is proven green, with the old implementation still in the tree to compare against. This PR is purely additive:
GET /adminstill serves the server-rendered dashboard,--features uiis still off by default, and a default build is byte-for-byte unaffected.The follow-up flips
GET /adminto the shell and removeshtml::dashboard_page,src/admin/script.rs, and the 15 source-text tests.What is here
The dashboard, ported. Upstream status, the grouped accounts-and-usage table, both add-account flows, both store tables, and managed pool health — all of
src/admin/script.rsand the dashboard half ofsrc/admin/html.rs, as React components underui/src/.GET /admin/api/session— the one new route. The server-rendered page interpolates two per-session values into itself; the SPA shell is a single file embedded at compile time and served, unauthenticated, to every visitor alike, so it can carry neither:csrf— the session's token. Returning it over aGETdoes not weaken the guard: there is no CORS layer anywhere on the admin router, so a cross-origin page can send the request with the ambient cookie but cannot read the reply. That is the same propertyGET /adminalready depends on. Recorded as Decision 5 indocs/admin-ui-delivery.md, including the review trigger it creates — adding a permissiveAccess-Control-Allow-Originto this surface would hand the token to any origin.expiry_buffer_ms—claude::auth::EXPIRY_BUFFER. Served rather than copied into TypeScript because the dashboard reports a setup token as expired once it is inside that buffer and routing refuses one on the same boundary (Tokens::is_valid_at); a duplicated constant could drift.A header-credential caller receives an empty
csrf, matching whatdashboardrenders for one.Tests
ui/gains vitest + Testing Library and 31 tests that render components and assert on what an operator sees. That is the point rather than an incidental choice: the string-literal dashboard could only be tested by matching substrings of its emitted JavaScript, which cannot distinguish a guard that runs from one that is merely present.All 15 behavioral properties the old
html.rstests pinned are carried across as real behavior tests, plus the bootstrap's own cases. Each was checked by deleting the guard it covers and confirming the test fails:One of these was vacuous on the first pass: the second-completion test drove two awaited clicks, and the
disabledattribute the first click sets blocked the second before the ref guard behind it could run — so deleting that guard stayed green. It now dispatches both clicks inside one task, which is the window the guard actually exists for (React has not re-rendered, so the attribute is not on the button yet). That mutation now fails..github/workflows/ci.ymlrunsnpm testinui/, so this is a gate rather than a local convenience.npm run buildonly typechecked it before.One behavior change
Sign-out is a scripted
POSTrather than a form submit. The shell is served underform-action 'none'(from #503), which is what lets that policy stay tight for a bundle that posts no forms;POST /admin/api/logoutis guarded bysame_origin, not a CSRF token, which a same-originfetchsatisfies.Trying it
The bundle is already reachable without flipping any route:
--features uiserves the shell at any unmatched path under the mount, so/admin/uirenders the ported dashboard while/adminstill serves the old one. That is also how the two can be compared side by side.Not here
--features uionce the ported views land #504 (a README row for--features ui) stays deferred to the follow-up. Adding it now would document a dashboard thatGET /admindoes not yet serve.Verification
cargo clippy --all-targets --all-features -- -D warnings— clean, 0 warningscargo test --all-features --workspace— 2569 passed (2568 onmain; +1 is the new bootstrap test)ui:npm run typecheck,npm test— 31 passedscript-src 'self'; style-src 'self'policy still holdsBooted once for real, rather than trusting the mocks: a debug binary built
--features ui, signed in through/admin/login, then loaded/admin/ui.GET /admin/api/session→200 application/jsonwith a live token andexpiry_buffer_ms: 300000; the same request without the cookie →401/admin/assets/*.js→200 text/javascript,*.css→200 text/cssGET /admin/api/nope→404, not the shellcodexone labelled GPT, each with its own state and remediation line, and a managed pool account coalesced with its matching local observation into a single row rather than listed twice. (No screenshot: the live page shows real account identities.)A pre-existing flake, found and not attributed here
cargo test --test multi_accountfails about two runs in three on a default build —terminal_invalid_grant_during_resolution_marks_the_account_as_needing_relogin,502where200is expected. It is not this branch's doing: a control run onorigin/mainin a fresh worktree fails at the same 2-of-3 rate, and the test passes 5 of 5 in isolation. Filed as #507 with both arms recorded. CI does not see it because CI runs--all-features, where this suite passed.Summary by cubic
Ports the admin dashboard from Rust string literals into React components in the embedded SPA bundle.
GET /adminstill serves the server-rendered page for now, while the ported dashboard is reachable at/admin/uiunder--features uiso both can be compared side by side; deleting the string literals and their tests is a follow-up once this port is proven green.New Features
GET /admin/api/session, serving the session's CSRF token andexpiry_buffer_msthat a static bundle cannot have interpolated into its source; returning the token over GET is safe because the admin surface has no CORS layer, and a header-credential caller receives an emptycsrf.ui/that render components and assert on what an operator sees, each checked by deleting the guard it covers; CI now runsnpm testso the suite gates merges.form-action 'none'policy.Bug Fixes
PoolHealth's state precedence with tests; theneeds_relogin-before-cooldown ordering was deliberate but uncovered.readJsonkeeps the HTTP status when the body is not JSON, so a proxy's HTML error page no longer drops the 401 redirect to/admin/loginand strands the operator on the loading state.ADMIN_OIDC_ENV_LOCKtoADMIN_ENV_LOCK), so itsset_varcannot race a concurrent test's env read.Written for commit eb25c35. Summary will update on new commits.