Conversation
`GET /admin/api/observed` reports every provider credential it finds on the gateway host, with no knowledge of the account pool. A login that is *also* a pooled account therefore appeared twice: once as its pool entry, carrying live quota and cooldown, and again as a bare observation of the same subscription. With two ChatGPT accounts pooled, the dashboard showed three rows. The SPA already coalesces this case, but only for `claude_oauth` (`ui/src/accounts.ts`): it matches on a uuid that the Claude observed row carries and the ChatGPT one does not, and `/admin/api/pool` exposes no uuids at all, so no client can do the same for Codex. Resolve it at the source instead, where both halves are already in hand: drop a discovered credential whose stable upstream identity is one the pool resolves. The pool row is strictly the more informative of the two. Suppression is keyed by store family *and* identity, so an id pooled under Claude cannot hide a ChatGPT credential that happens to share the string. Only the three pooled families can match at all; Grok, Gemini and Cursor have no pool representation, so their observed rows are the only view of those providers and always survive. A credential whose identity cannot be established is kept too -- an unlabelled row is a smaller problem than a silently hidden account -- and a provider whose scope fails to resolve contributes no identities, so its credentials stay visible rather than disappearing behind a read error.
There was a problem hiding this comment.
Code Review
This pull request introduces de-duplication logic to the observed_accounts admin endpoint by filtering out discovered credentials that are already present in the resolved accounts pool. It adds helper functions pooled_identities and is_already_pooled along with corresponding unit tests. Feedback on these changes suggests resolving the pool accounts concurrently using futures_util::future::join_all to avoid sequential backend latency bottlenecks.
| async fn pooled_identities(state: &AppState) -> HashSet<(crate::accounts::StoreFamily, String)> { | ||
| let mut pooled = HashSet::new(); | ||
| for (name, provider) in &state.config.providers { | ||
| let (label, family, dir, scan): ( | ||
| _, | ||
| _, | ||
| _, | ||
| fn() -> std::io::Result<Vec<crate::config::AccountConfig>>, | ||
| ) = match provider.auth { | ||
| AuthMode::ClaudeOauth => ( | ||
| "Claude", | ||
| crate::accounts::StoreFamily::Claude, | ||
| claude_store::default_accounts_dir(), | ||
| claude_store::scan_accounts, | ||
| ), | ||
| AuthMode::ChatgptOauth => ( | ||
| "codex", | ||
| crate::accounts::StoreFamily::Chatgpt, | ||
| crate::auth::codex::store::default_accounts_dir(), | ||
| crate::auth::codex::store::scan_accounts, | ||
| ), | ||
| AuthMode::KimiOauth => ( | ||
| "Kimi", | ||
| crate::accounts::StoreFamily::Kimi, | ||
| crate::auth::kimi::store::default_accounts_dir(), | ||
| crate::auth::kimi::store::scan_accounts, | ||
| ), | ||
| _ => continue, | ||
| }; | ||
| match crate::auth::shared::resolve_pool_accounts( | ||
| label, | ||
| &provider.accounts, | ||
| &provider.account_scope, | ||
| family, | ||
| dir, | ||
| scan, | ||
| ) | ||
| .await | ||
| { | ||
| Ok(resolved) => pooled.extend( | ||
| resolved | ||
| .iter() | ||
| .filter_map(|account| account.uuid.as_deref()) | ||
| .map(str::trim) | ||
| .filter(|uuid| !uuid.is_empty()) | ||
| .map(|uuid| (family, uuid.to_string())), | ||
| ), | ||
| Err(error) => { | ||
| tracing::debug!( | ||
| provider = %name, | ||
| %error, | ||
| "admin: could not resolve pool identities for observed-row de-duplication" | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| pooled | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Resolve pool accounts concurrently to reduce latency
Problem: The pooled_identities function sequentially awaits resolve_pool_accounts for each configured provider in a loop. If there are multiple providers, this sequential execution can unnecessarily increase the latency of the observed_accounts admin API endpoint.
Rationale: Section 6 (Performance — Backend) of the Org Style Guide emphasizes avoiding backend latency bottlenecks. Sequential asynchronous operations that can be run in parallel should be executed concurrently.
Suggestion: Use futures_util::future::join_all to resolve the pool accounts for all providers concurrently.
async fn pooled_identities(state: &AppState) -> HashSet<(crate::accounts::StoreFamily, String)> {
let futures = state.config.providers.iter().map(|(name, provider)| async move {
let (label, family, dir, scan): (
_,
_,
_,
fn() -> std::io::Result<Vec<crate::config::AccountConfig>>,
) = match provider.auth {
AuthMode::ClaudeOauth => (
"Claude",
crate::accounts::StoreFamily::Claude,
claude_store::default_accounts_dir(),
claude_store::scan_accounts,
),
AuthMode::ChatgptOauth => (
"codex",
crate::accounts::StoreFamily::Chatgpt,
crate::auth::codex::store::default_accounts_dir(),
crate::auth::codex::store::scan_accounts,
),
AuthMode::KimiOauth => (
"Kimi",
crate::accounts::StoreFamily::Kimi,
crate::auth::kimi::store::default_accounts_dir(),
crate::auth::kimi::store::scan_accounts,
),
_ => return None,
};
match crate::auth::shared::resolve_pool_accounts(
label,
&provider.accounts,
&provider.account_scope,
family,
dir,
scan,
)
.await
{
Ok(resolved) => {
let identities: Vec<_> = resolved
.iter()
.filter_map(|account| account.uuid.as_deref())
.map(str::trim)
.filter(|uuid| !uuid.is_empty())
.map(|uuid| (family, uuid.to_string()))
.collect();
Some(identities)
}
Err(error) => {
tracing::debug!(
provider = %name,
%error,
"admin: could not resolve pool identities for observed-row de-duplication"
);
None
}
}
});
let results = futures_util::future::join_all(futures).await;
let mut pooled = HashSet::new();
for res in results {
if let Some(identities) = res {
pooled.extend(identities);
}
}
pooled
}References
- Section 6 (Performance — Backend) of the Org Style Guide emphasizes avoiding backend latency bottlenecks. (link)
- Poll independent network requests concurrently (e.g., using
join_all) to prevent a slow or hanging provider from delaying others.
|
| let rows = futures_util::future::join_all( | ||
| discovered | ||
| .into_iter() | ||
| .filter(|observed| !is_already_pooled(observed, &pooled)) |
There was a problem hiding this comment.
When a Claude credential matches a pooled account, this filter removes it before build_claude_observed_row can fetch its local-client quota. The dashboard intentionally folds those observed quota windows into the managed row because the pool can have missing or stale windows until it receives relevant response headers. As a result, the account is shown only once, but its observed quota and status signal is lost. Preserve the observation data when coalescing the rows.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/admin/mod.rs
Line: 798
Comment:
**Matched quota data is lost**
When a Claude credential matches a pooled account, this filter removes it before `build_claude_observed_row` can fetch its local-client quota. The dashboard intentionally folds those observed quota windows into the managed row because the pool can have missing or stale windows until it receives relevant response headers. As a result, the account is shown only once, but its observed quota and status signal is lost. Preserve the observation data when coalescing the rows.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
An account that the pool already resolves also shows up as a separate observed
row, so a machine with two ChatGPT accounts renders three ChatGPT entries on the
admin surface.
discover()is machine-level credential discovery with no poolawareness, so it re-reports credentials the pool already owns.
observed_accountsnow resolves the pool's stable upstream identities(
shuntAccountUuidfor Claude,chatgpt_account_idfor ChatGPT/Codex) per storefamily and drops the observed copy when it matches. The pool row is the one kept:
it carries live quota, priority and cooldown that the observed row cannot.
Conservative in the safe direction throughout — hiding a row is the destructive
outcome, so a credential is only suppressed on a confident match:
representation, so their observed rows are the only view of them and always survive.
id string.
identities, leaving its credentials visible rather than failing the endpoint.
This is a display concern.
Test plan
observed_row_is_suppressed_only_for_its_own_pooled_identitycovers own-family suppression for Codex and Claude, cross-family non-collision,
an unpooled second account surviving, Grok always surviving, and
None/blank ids.cargo fmt --all --check,cargo clippy --all-targets -- -D warningsclean.cargo test --lib admin(96 passed) andcargo test --test admin_surface(48 passed) green on top of current
main.before, two after.
Summary by cubic
Pooled accounts were appearing twice on the admin observed surface; they now show only as the pool row, which carries live quota, priority, and cooldown.
Written for commit cfbac99. Summary will update on new commits.