feat(admin): group the accounts and usage table by provider - #242
Conversation
The "Accounts and usage" table rendered one row per observed provider client, while managed pool accounts lived in a separate table collapsed behind "Manage pool accounts (advanced)". A two-account Claude pool therefore showed neither account under Claude. Group the table by provider — named once, with its accounts nested beneath — and fold managed pool accounts in alongside the read-only observations. Where a managed account holds the same subscription as the local client login, coalesce by account uuid so one subscription renders as one row: the managed name labels it, and the client's quota fills the bars because the pool only learns a window from a response header it has actually received. Emitting that uuid needs an identity for the observed credential, which parse_claude cannot derive from the credential blob; attach it at the already environment-dependent discovery site instead. Guarded, because the credential sources are profile-agnostic (fixed Keychain service name, hardcoded $HOME/.claude/.credentials.json) while oauthAccount.accountUuid moves with CLAUDE_CONFIG_DIR. With that set the two can name different accounts — verified against a live pool, where it labelled an exhausted account's usage with a second account's uuid — so identity resolves to None there and the rows stay separate rather than merging wrongly.
CONTRIBUTING requires the spec to move with a deviating change. Three statements had drifted: m9 said GET /admin/observed masks account identity, the endpoints reference described its payload without the new uuid, and the admin guide described the usage table as one row per observed identity with managed accounts reachable only from the advanced section. Record the grouped-by-provider layout, the uuid-keyed coalescing of an observation and a managed account holding the same subscription, and why identity resolves to None under CLAUDE_CONFIG_DIR. Note that the uuid adds no disclosure GET /admin/accounts did not already make to the same authenticated caller, and that closing the profile gap needs a token-scoped identity source that does not exist today.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![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 introduces grouping by provider and coalescing of observed logins and managed pool accounts sharing the same subscription into a single row on the admin dashboard. It adds the Claude account UUID to observed credentials and implements conservative merging logic that declines to merge when CLAUDE_CONFIG_DIR is set. The review feedback suggests making the current_account_uuid_from helper fully environment-free to improve test isolation and making the frontend Promise.all call resilient to individual endpoint fetch failures.
| pub(crate) fn current_account_uuid() -> Option<String> { | ||
| current_account_uuid_from(std::env::var_os("CLAUDE_CONFIG_DIR").as_deref()) | ||
| } | ||
|
|
||
| /// Env-free half of [`current_account_uuid`], split for the same reason as | ||
| /// [`claude_global_config_path_from`]: the guard is testable without mutating | ||
| /// process-global environment state. | ||
| fn current_account_uuid_from(config_dir: Option<&std::ffi::OsStr>) -> Option<String> { | ||
| if config_dir.is_some_and(|dir| !dir.is_empty()) { | ||
| return None; | ||
| } | ||
| read_current_account_uuid(&claude_global_config_path()).ok() | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Make current_account_uuid_from fully environment-free
Symptom — current_account_uuid_from is intended to be an env-free helper for testing, but it calls claude_global_config_path() which internally reads from the global process environment (std::env::var_os).
Source — Feathers — Working Effectively with Legacy Code (Ch. 3: Sensing and Separation / Seam Model).
Consequence — Tests calling current_account_uuid_from are not fully isolated from the process's actual environment variables (like HOME or CLAUDE_CONFIG_DIR), which can lead to flaky or non-deterministic test results if those environment variables are mutated or present during test execution.
Remedy — Refactor current_account_uuid_from to accept both config_dir and home parameters, and call claude_global_config_path_from(config_dir, home) to make it completely env-free.
Note: You will also need to update the test current_account_uuid_declines_under_a_relocated_config_dir at line 489 to pass None as the second argument to current_account_uuid_from.
pub(crate) fn current_account_uuid() -> Option<String> {
let config_dir = std::env::var_os("CLAUDE_CONFIG_DIR");
let home = std::env::var_os("HOME")
.filter(|path| !path.is_empty())
.or_else(|| std::env::var_os("USERPROFILE").filter(|path| !path.is_empty()));
current_account_uuid_from(config_dir.as_deref(), home.as_deref())
}
/// Env-free half of current_account_uuid, split for the same reason as
/// claude_global_config_path_from: the guard is testable without mutating
/// process-global environment state.
fn current_account_uuid_from(config_dir: Option<&std::ffi::OsStr>, home: Option<&std::ffi::OsStr>) -> Option<String> {
if config_dir.is_some_and(|dir| !dir.is_empty()) {
return None;
}
read_current_account_uuid(&claude_global_config_path_from(config_dir, home)).ok()
}References
- Brooks-Lint shared framework for decay-risk diagnosis, specifically regarding testability and environment isolation. (link)
- When resolving the default user home directory path on multiple platforms (including Windows), do not rely solely on the
HOMEenvironment variable. Fall back toUSERPROFILEon Windows to ensure credentials or configuration files are stored in the correct user directory regardless of the operating system. - Avoid replacing production once-only startup invariants (such as
OnceLock) with resettable locks (such asRwLock) solely for testing purposes, as this weakens production invariants. Instead, test the underlying logic using pure helper functions that do not rely on the global state.
| current_account_uuid_from(Some(std::ffi::OsStr::new("/tmp/some-other-profile"))), | ||
| None | ||
| ); |
There was a problem hiding this comment.
[MEDIUM] Update test call to match new current_account_uuid_from signature
Symptom — The test current_account_uuid_declines_under_a_relocated_config_dir calls current_account_uuid_from with only one argument, which will fail to compile if the signature is updated to accept the home parameter.
Source — xUnit Test Patterns — Test Brittleness.
Consequence — Compilation failure of the test suite.
Remedy — Update the test call to pass None as the second argument (home) to current_account_uuid_from.
assert_eq!(
current_account_uuid_from(Some(std::ffi::OsStr::new("/tmp/some-other-profile")), None),
None
);| try {{ | ||
| const [poolRes, accountsRes] = await Promise.all([fetch("/admin/pool"), fetch("/admin/accounts")]); | ||
| if (poolRes.ok) pool = await poolRes.json(); | ||
| if (accountsRes.ok) accounts = await accountsRes.json(); | ||
| }} catch (e) {{ /* observation-only render */ }} |
There was a problem hiding this comment.
[MEDIUM] Make Promise.all fetches resilient to individual failures
Symptom — The Promise.all call for fetching /admin/pool and /admin/accounts will reject entirely if either fetch fails with a network error, causing both to be treated as null even if one of them succeeded.
Source — Hunt & Thomas — Pragmatic Programmer (Orthogonality / Robustness).
Consequence — A transient network failure or timeout on one endpoint (e.g., /admin/accounts) will unnecessarily prevent the other successfully fetched data (e.g., /admin/pool) from being rendered on the dashboard.
Remedy — Append .catch(() => null) to each individual fetch promise inside Promise.all so they resolve gracefully to null on network failure, and check for nullness before accessing .ok.
| try {{ | |
| const [poolRes, accountsRes] = await Promise.all([fetch("/admin/pool"), fetch("/admin/accounts")]); | |
| if (poolRes.ok) pool = await poolRes.json(); | |
| if (accountsRes.ok) accounts = await accountsRes.json(); | |
| }} catch (e) {{ /* observation-only render */ }} | |
| try { | |
| const [poolRes, accountsRes] = await Promise.all([ | |
| fetch("/admin/pool").catch(() => null), | |
| fetch("/admin/accounts").catch(() => null) | |
| ]); | |
| if (poolRes && poolRes.ok) pool = await poolRes.json(); | |
| if (accountsRes && accountsRes.ok) accounts = await accountsRes.json(); | |
| } catch (e) { /* observation-only render */ } |
References
- Brooks-Lint shared framework for decay-risk diagnosis, specifically regarding orthogonality and error handling. (link)
Greptile SummaryGroups the admin usage dashboard by provider and combines observed usage with managed pool health.
Confidence Score: 3/5The PR is not yet safe to merge because cross-provider account coalescing can misattribute quota data and coalesced abnormal states still render contradictory health guidance. The global Claude name-to-UUID map is still applied to every pool provider, so same-named Claude and Codex accounts can merge an observation into the wrong provider row. Coalescing also leaves the managed state unchanged while the label follows the observed state, causing styling and remediation to disagree with statuses such as “Needs login.” Files Needing Attention: src/admin/script.rs
|
| Filename | Overview |
|---|---|
| src/admin/script.rs | Implements provider grouping and observed/managed account coalescing, while the two previously reported identity and status-reconciliation defects remain. |
| src/admin/mod.rs | Adds the resolved Claude account UUID to successful and unavailable observed-account response rows. |
| src/auth/observation.rs | Associates default-profile Claude observations with the current account UUID while declining identity under a relocated config directory. |
| src/auth/claude/login.rs | Supplies the Claude account identity lookup used by observation discovery. |
| src/admin/html.rs | Extracts the dashboard JavaScript into a separate module and substitutes the escaped CSRF token into it. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
O["GET /admin/observed"] --> G["Group accounts by provider"]
P["GET /admin/pool"] --> G
A["GET /admin/accounts"] --> I["Resolve managed Claude UUIDs"]
I --> G
G --> C["Coalesce matching account UUIDs"]
C --> T["Accounts and usage table"]
Reviews (3): Last reviewed commit: "fix(admin): address review — env-free id..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Adding the grouped-accounts renderer pushed src/admin/html.rs to 581
lines, past the contribution checklist's 500-line guidance.
Split the inline dashboard script into src/admin/script.rs: html.rs
drops to 262 lines and script.rs is 330. Holding it as a plain const
rather than a format! argument also lets the JavaScript read as
JavaScript — inside a format string every brace has to be doubled — so
the single {csrf} placeholder is substituted with replace() instead.
Behaviour-preserving: dashboard_page() renders byte-for-byte identical
output before and after (33348 bytes, verified by diffing the rendered
page across the change).
current_account_uuid_from was documented as the env-free half but still called claude_global_config_path(), which reads HOME/USERPROFILE, so its test stayed coupled to the ambient environment the split was meant to escape. Take home as a parameter too and resolve through claude_global_config_path_from, matching that helper's existing split. The dashboard's pool/accounts enrichment wrapped Promise.all in a single catch, so a transient failure on either endpoint discarded both results. Catch per fetch instead: one endpoint failing now degrades only its own contribution.
2 participants
Summary
With a multi-account Claude pool configured,
Accounts and usageshowed a singleClauderow:GET /admin/observedreturns one row per provider client on the host, and the per-account view lived in the collapsedManage pool accounts (advanced)disclosure. On a live two-account pool the top table readWeek 100%forClaude Code · Maxwhile one account was exhausted and cooling and a second sat healthy atWeek 12%— nothing above the fold said half the pool was gone.This groups the table by provider (named once, accounts nested beneath) and folds managed pool accounts in alongside the read-only observations. Provisioning and raw store metadata stay in the advanced section.
An observation and a managed account are often the same subscription, so they are coalesced by account uuid: one subscription renders as one row, labelled with the managed account name, with the observation’s windows preferred because the pool only learns a window from a response header it has actually received.
Closes #241.
Milestone / spec
M9 —
docs/m9-admin-surface.md§"Phase 2 — observed usage and managed-pool health", updated in this PR.Checklist
cargo buildpassescargo testpasses (new behavior is covered; tests run without network/loopback where possible)cargo clippy --all-targets -- -D warningscleancargo fmt --all --checkcleandocs/updated if this change deviates from itNotes for reviewers
Identity handling is the part to scrutinise.
parse_claudecannot derive an account id from the credential blob, so it is attached at the already environment-dependent discovery site instead, and guarded:current_account_uuid()returnsNonewheneverCLAUDE_CONFIG_DIRis set. Both Claude credential sources are profile-agnostic (fixed Keychain service name; hardcoded$HOME/.claude/.credentials.json) whileoauthAccount.accountUuidmoves with that variable, so under a selected profile the config can name a different account than the credential actually read.That is not hypothetical — an earlier build of this branch resolved a live observation carrying account A’s usage (
7d = 1.0) to account B’s uuid, which would have painted an exhausted account’s quota onto the healthy account’s row. Declining to merge costs a combined row; guessing costs a silently mis-attributed quota bar. Covered bycurrent_account_uuid_declines_under_a_relocated_config_dir.Known limitation, and the design question in #241: coalescing therefore only engages in the default-profile case. Closing that gap needs a token-scoped Claude identity source, which does not exist today —
/api/oauth/usageis the only Claude endpoint read here andshuntAccountUuidis captured only at login/import. Happy to rework this if you would rather add that first.Privacy posture change.
GET /admin/observedpreviously masked account identity; the Claude row now carriesuuid.GET /admin/accountsalready returns those uuids unmasked to the same authenticated caller, so this adds no disclosure the admin surface did not already make — but it is a deliberate deviation and the m9 spec was updated to say so. If you would prefer a hashed identity, equality is all the coalescing needs.File sizes. The renderer initially pushed
src/admin/html.rsto 581 lines, over the checklist guideline, so the third commit splits the inline dashboard script intosrc/admin/script.rs:html.rsis now 262 lines andscript.rs330. Holding the script as a plain const rather than aformat!argument also un-doubles every brace, so the JavaScript reads as JavaScript. The split is behaviour-preserving —dashboard_page()renders byte-for-byte identical output across it (33348 bytes, verified by diffing the rendered page).Test plan
cargo fmt --all --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo test --all-features --workspace— 958 passed, 0 failed (+1 new)claude_oauthpool on a scratch port:CLAUDE_CONFIG_DIRunset → observed row resolves to the pooled account’s uuid,7dagrees between the two sources, rows coalesceCLAUDE_CONFIG_DIRset →uuidisnull, no merge, rows render separatelySummary by cubic
Groups the admin “Accounts and usage” table by provider and merges observed logins with managed pool accounts so one subscription renders as one row. Adds Claude account UUIDs to
/admin/observed(guarded byCLAUDE_CONFIG_DIR) to match safely. Closes #241.New Features
uuid; managed name labels it, observed windows fill the bars./admin/observedincludes the Claude accountuuid(ornullwhen unknown). WhenCLAUDE_CONFIG_DIRselects a profile, identity is withheld to avoid misattribution, so rows aren’t merged.Refactors
src/admin/script.rswith{csrf}replacement; output unchanged and keepshtml.rswithin size guidance.current_account_uuid_from) and made the dashboard resilient to partial admin API failures (pool/accounts fetches are independent so observations still render).Written for commit e8c4c4d. Summary will update on new commits.