Skip to content

feat(quota): per-account Claude rate limits, canonical 5-hour window#493

Draft
wonsh42 wants to merge 2 commits into
lidge-jun:devfrom
wonsh42:fix/anthropic-per-account-rate-limits
Draft

feat(quota): per-account Claude rate limits, canonical 5-hour window#493
wonsh42 wants to merge 2 commits into
lidge-jun:devfrom
wonsh42:fix/anthropic-per-account-rate-limits

Conversation

@wonsh42

@wonsh42 wonsh42 commented Jul 26, 2026

Copy link
Copy Markdown

Summary

Anthropic reports OAuth usage per credential, so every logged-in Claude account can be probed with its own bearer token. Three related gaps came out of that:

  1. Only the active account's limits were ever fetched. With two Claude logins there was no way to see a background account's headroom before switching to it.
  2. The 5-hour window was a generic extra window. It was pushed into customWindows with the label 5h, so it rendered differently from the Codex login rows (which use the canonical Weekly limit styling and ordering).
  3. Switching the active account left stale numbers on screen. The workspace shell fetched /api/provider-quotas once on mount, so the Usage tab kept the previous account's percentages until a full page reload.

Before / after

Per-account rate limits on the provider Overview — before (no limits per account) and after:

In the after shot the two accounts show genuinely different upstream numbers (one at 100% and rate-limited, the active one at 10%), which is the point of probing per credential. The threshold/limit-reached styling is the existing QuotaBars behaviour, reused as-is.

Canonical 5-hour limit label on the providers overview, alongside Weekly limit:

Usage tab, now following the active account without a reload:

All e-mail addresses in these captures were replaced in the DOM before the screenshot was taken, so no real address is present in the image data.

Changes

Server:

  • fetchProviderAccountQuotas(provider, forceRefresh) in src/providers/quota.ts, mirroring the Codex pool's per-account probe (codex/auth-api.ts:fetchPoolAccountQuota), with a per-account cache + single-flight. Account-scoped tokens come from getValidAccessTokenForAccount, so refreshing a background account never touches the active selection.
  • GET /api/oauth/accounts?quota=1 returns each account's quota (and quotaUnavailable when the probe failed). The plain list stays a cheap local read; ?refresh=1 bypasses the TTL.
  • Claude's 5-hour bucket moves to the canonical fiveHourPercent/fiveHourResetAt fields. Opus/Sonnet stay as custom windows.
  • Logout and account-removal also clear the per-account cache.

GUI:

  • Account rows render that account's QuotaBars; the row is wrapped in a pwi-auth-acct container so the bars sit under the row without disturbing the existing layout.
  • The shell's quota fetch is keyed on account state, so an active-account switch re-reads the rows.

Design notes

The per-account TTL (10 min) is deliberately longer than the provider-level one (5 min): this path multiplies by account count, and the usage endpoint rate-limits under repeated probing — I hit a 429 on one account while testing. A failed probe is flagged unavailable and keeps the last good row rather than blanking a sibling account.

supportsPerAccountQuota() gates the feature to anthropic for now; other OAuth providers return an empty list and make no upstream calls.

Testing

New tests/provider-account-quota.test.ts (5 cases):

  • each account probed with its own bearer token, distinct upstream numbers per account
  • 5-hour lands in canonical fields, not customWindows
  • cached rows reused; forceRefresh bypasses the TTL
  • a 429 on one account is flagged unavailable without affecting the sibling
  • non-supported providers and empty account sets make no upstream calls

Updated tests/provider-quota.test.ts for the moved 5-hour fields.

Ran on this branch:

  • bun scripts/test.ts — 4157 pass, 0 fail
  • bun x tsc --noEmit — clean
  • cd gui && bun run build — clean
  • cd gui && bun run lint — clean
  • bun scripts/privacy-scan.ts — passed

Notes

Found while looking into multi-account Claude usage. Related to #491 (independent branch off main; no shared commits).

Summary by CodeRabbit

  • New Features

    • OAuth account lists now display per-account quota usage and availability when supported.
    • Quota information includes usage windows and reset times for supported providers.
    • Quota data is refreshed and cached to improve responsiveness.
  • Bug Fixes

    • Quota displays now update correctly when switching active accounts.
    • Logging out or removing an account clears its cached quota information.
    • Failed quota checks no longer discard previously available results.
  • Chores

    • Updated the application version to 2.7.40.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Anthropic per-account quota probing now uses account-scoped tokens, caches results, and exposes quota overlays through the OAuth accounts API. Provider workspace account rows display returned quota bars, with updated types, styles, cache invalidation, tests, and package version metadata.

Changes

OAuth account quota flow

Layer / File(s) Summary
Account quota probing and normalization
src/providers/quota.ts
Anthropic usage parsing now produces canonical five-hour fields, while per-account probing adds token-scoped fetches, caching, inflight request coalescing, and unavailable-account handling.
OAuth account response integration
src/server/management/oauth-account-routes.ts
The accounts endpoint optionally adds quota data per account, and logout or deletion clears account-scoped quota caches.
Account quota model and visualization
gui/src/components/provider-workspace/..., gui/src/hooks/useProviderAccountPools.ts, gui/src/styles/provider-workspace-settings.css, package.json
GUI account types and fetches include quota fields, account rows render quota bars, related styles are added, and the package version changes to 2.7.40.
Quota behavior validation
tests/provider-account-quota.test.ts, tests/provider-quota.test.ts
Tests cover per-account probing, caching, refresh, failures, unsupported providers, empty account sets, and canonical Anthropic five-hour quota fields.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProviderWorkspace
  participant useProviderAccountPools
  participant OAuthAccountsAPI
  participant fetchProviderAccountQuotas
  participant AnthropicUsageAPI
  ProviderWorkspace->>useProviderAccountPools: load provider account sets
  useProviderAccountPools->>OAuthAccountsAPI: request accounts with quota=1
  OAuthAccountsAPI->>fetchProviderAccountQuotas: fetch per-account quota rows
  fetchProviderAccountQuotas->>AnthropicUsageAPI: probe usage with each account token
  AnthropicUsageAPI-->>fetchProviderAccountQuotas: return account usage data
  fetchProviderAccountQuotas-->>OAuthAccountsAPI: return quota rows
  OAuthAccountsAPI-->>ProviderWorkspace: return accounts with quota fields
  ProviderWorkspace-->>ProviderWorkspace: render account quota bars
Loading

Possibly related PRs

  • lidge-jun/opencodex#479: Modifies the same OAuth account route and GUI account-row data path with a different account metadata overlay.

Suggested reviewers: lidge-jun, wibias

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: per-account Claude quota support and the canonical 5-hour window mapping.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

Target branch corrected

This pull request now targets dev.

The [WRONG BRANCH] title prefix has been removed. Its existing draft status has been preserved.

@github-actions github-actions Bot changed the title feat(quota): per-account Claude rate limits, canonical 5-hour window [WRONG BRANCH] feat(quota): per-account Claude rate limits, canonical 5-hour window Jul 26, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Jul 26, 2026
Anthropic reports OAuth usage per CREDENTIAL, so every logged-in account can
be probed with its own bearer token. Three related gaps:

1. Only the ACTIVE account's rate limits were ever fetched, so a multiauth
   Claude setup could not see a background account's headroom before switching
   to it. Adds fetchProviderAccountQuotas() (mirroring the Codex pool's
   per-account probe, including a per-account TTL) and exposes it on
   GET /api/oauth/accounts?quota=1, rendered as bars on each account row.
2. The 5-hour window was reported as a generic `customWindows` entry labelled
   "5h", so it rendered differently from the Codex login rows. It now uses the
   canonical fiveHourPercent/fiveHourResetAt fields and picks up the standard
   "5-hour limit" label and ordering. Opus/Sonnet stay custom windows.
3. The workspace shell fetched provider quotas once on mount, so switching the
   active account left the previous account's numbers on the Usage tab until a
   full page reload. The fetch is now keyed on account state.

The per-account TTL is longer than the provider-level one: this path multiplies
by account count and the usage endpoint rate-limits under repeated probing
(observed 429). A failing probe is flagged unavailable and keeps the last good
row instead of blanking a sibling account.
@wonsh42
wonsh42 force-pushed the fix/anthropic-per-account-rate-limits branch from e4b0b9e to 5e466a7 Compare July 26, 2026 08:47
@wonsh42 wonsh42 changed the title [WRONG BRANCH] feat(quota): per-account Claude rate limits, canonical 5-hour window feat(quota): per-account Claude rate limits, canonical 5-hour window Jul 26, 2026
@wonsh42
wonsh42 changed the base branch from main to dev July 26, 2026 08:48
@wonsh42
wonsh42 marked this pull request as ready for review July 26, 2026 08:51

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx`:
- Around line 247-251: Update the quota rendering in ProviderAuthPanel to read
account.quotaUnavailable: preserve and render stale quota bars when quota data
exists, while adding a translated pws.quotaUnavailable hint alongside them; when
quota is null but unavailable is true, render the same unavailable hint instead
of omitting the section. Add the pws.quotaUnavailable translation entry to the
locale files using the existing i18n conventions.

In `@src/providers/quota.ts`:
- Around line 283-326: Update fetchAccountQuota and fetchProviderAccountQuotas
to preserve quota data together with an explicit unavailable status, so failed
refreshes remain marked unavailable even when prior quota exists. Record failed
probes in the account cache with a refreshed timestamp and failure state to
enforce ACCOUNT_QUOTA_TTL_MS backoff, while retaining the last successful quota
for display; ensure successful probes clear the failure state and continue
returning normal rows.

In `@tests/provider-account-quota.test.ts`:
- Around line 86-100: Add a regression test alongside the existing provider
quota tests that first returns a successful probe to populate the account cache,
then force-refreshes with a failing response. Assert the refreshed result
preserves the cached quota while setting unavailable to true, using
fetchProviderAccountQuotas and the existing seed/setup helpers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2ba29eed-9518-4927-9a6e-361fa3ce192a

📥 Commits

Reviewing files that changed from the base of the PR and between b448570 and 5e466a7.

⛔ Files ignored due to path filters (4)
  • docs/assets/anthropic-account-quota-after.png is excluded by !**/*.png
  • docs/assets/anthropic-account-quota-before.png is excluded by !**/*.png
  • docs/assets/anthropic-five-hour-limit-overview.png is excluded by !**/*.png
  • docs/assets/anthropic-usage-tab-active-account.png is excluded by !**/*.png
📒 Files selected for processing (10)
  • gui/src/components/provider-workspace/ProviderAuthPanel.tsx
  • gui/src/components/provider-workspace/ProviderWorkspaceShell.tsx
  • gui/src/components/provider-workspace/types.ts
  • gui/src/hooks/useProviderAccountPools.ts
  • gui/src/styles/provider-workspace-settings.css
  • package.json
  • src/providers/quota.ts
  • src/server/management/oauth-account-routes.ts
  • tests/provider-account-quota.test.ts
  • tests/provider-quota.test.ts

Comment on lines +247 to +251
{account.quota && (
<div className="pwi-auth-acct-quota">
<QuotaBars quota={account.quota} plan={null} threshold={80} t={t} layout="stacked" />
</div>
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

account.quotaUnavailable is never surfaced to the user.

The row/hook types carry quotaUnavailable specifically to flag a failed probe while preserving stale data (per the PR's own stated behavior), but this component only checks account.quota and never reads account.quotaUnavailable. Two consequences:

  • When a probe fails on an account that previously succeeded, the bars render exactly as if the data were fresh — users have no way to know the shown percentages may be out of date.
  • When a probe fails on an account that never succeeded (quota: null, unavailable: true), nothing renders at all — no bars, no "unavailable" message.
💡 Suggested fix: render a stale/unavailable hint alongside the bars
                     </div>
-                    {account.quota && (
+                    {(account.quota || account.quotaUnavailable) && (
                       <div className="pwi-auth-acct-quota">
-                        <QuotaBars quota={account.quota} plan={null} threshold={80} t={t} layout="stacked" />
+                        {account.quotaUnavailable && (
+                          <span className="pwi-auth-quota-stale">{t("pws.quotaUnavailable")}</span>
+                        )}
+                        {account.quota && (
+                          <QuotaBars quota={account.quota} plan={null} threshold={80} t={t} layout="stacked" />
+                        )}
                       </div>
                     )}

New copy would need a pws.quotaUnavailable entry added to the locale files, per the i18n path guideline.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{account.quota && (
<div className="pwi-auth-acct-quota">
<QuotaBars quota={account.quota} plan={null} threshold={80} t={t} layout="stacked" />
</div>
)}
{(account.quota || account.quotaUnavailable) && (
<div className="pwi-auth-acct-quota">
{account.quotaUnavailable && (
<span className="pwi-auth-quota-stale">{t("pws.quotaUnavailable")}</span>
)}
{account.quota && (
<QuotaBars quota={account.quota} plan={null} threshold={80} t={t} layout="stacked" />
)}
</div>
)}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx` around lines 247
- 251, Update the quota rendering in ProviderAuthPanel to read
account.quotaUnavailable: preserve and render stale quota bars when quota data
exists, while adding a translated pws.quotaUnavailable hint alongside them; when
quota is null but unavailable is true, render the same unavailable hint instead
of omitting the section. Add the pws.quotaUnavailable translation entry to the
locale files using the existing i18n conventions.

Source: Path instructions

Comment thread src/providers/quota.ts
Comment on lines +283 to +326
async function fetchAccountQuota(provider: string, accountId: string, forceRefresh: boolean): Promise<ProviderQuota | null> {
const key = accountCacheKey(provider, accountId);
const cached = accountQuotaCache.get(key);
if (!forceRefresh && cached && Date.now() - cached.ts < ACCOUNT_QUOTA_TTL_MS) return cached.quota;
const joinable = accountQuotaInflight.get(key);
if (joinable) return joinable;

const probe = (async (): Promise<ProviderQuota | null> => {
try {
// Account-scoped token: refreshes (and persists) only THIS account, never the active one.
const token = await getValidAccessTokenForAccount(provider, accountId);
const quota = await fetchAnthropicUsageQuota(token);
// A failed probe keeps the previous good row rather than flashing an empty bar.
if (!quota) return cached?.quota ?? null;
accountQuotaCache.set(key, { ts: Date.now(), quota });
return quota;
} catch {
return cached?.quota ?? null;
}
})().finally(() => {
if (accountQuotaInflight.get(key) === probe) accountQuotaInflight.delete(key);
});
accountQuotaInflight.set(key, probe);
return probe;
}

/**
* Per-account quota rows for a provider's logged-in accounts. Probes run in parallel; a
* single failing account never blocks the others.
*/
export async function fetchProviderAccountQuotas(
provider: string,
forceRefresh = false,
): Promise<ProviderAccountQuota[]> {
if (!supportsPerAccountQuota(provider)) return [];
const set = getAccountSet(provider);
if (!set) return [];
return await Promise.all(set.accounts.map(async account => {
const quota = await fetchAccountQuota(provider, account.id, forceRefresh);
return quota
? { accountId: account.id, quota }
: { accountId: account.id, quota: null, unavailable: true as const };
}));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Failed probes silently lose the unavailable signal once a prior good value exists — and there's no backoff, risking a 429 retry storm.

Two issues in fetchAccountQuota / fetchProviderAccountQuotas:

  1. Lost unavailable flag on preserved data. When a probe fails but cached?.quota exists, fetchAccountQuota returns that truthy quota value (Line 296/300: return cached?.quota ?? null;). Back in fetchProviderAccountQuotas (Line 322-324), a truthy return is treated as success: quota ? { accountId, quota } : { accountId, quota: null, unavailable: true }. So once an account has ever succeeded, every subsequent failure (expired login, 429, network) is indistinguishable from a fresh success — the PR's own stated behavior ("Failed probes are marked unavailable while preserving the last successful data") is only honored the very first time a probe fails (before any cache entry exists). This is confirmed by tests/provider-account-quota.test.ts Lines 86-100, which only exercises the "never succeeded" path.
  2. No negative caching on failure. Since a failed probe never updates cached.ts (Lines 296, 300), the TTL guard (Date.now() - cached.ts < ACCOUNT_QUOTA_TTL_MS) stays permanently expired for that account after any failure, so every subsequent accounts-list fetch (e.g. GUI polling) re-probes upstream immediately with no backoff — exactly the "observed 429 under repeated probing" scenario the file's own comment (Lines 248-249) warns about.
🐛 Proposed fix: track unavailable state explicitly and negative-cache failures
-async function fetchAccountQuota(provider: string, accountId: string, forceRefresh: boolean): Promise<ProviderQuota | null> {
+async function fetchAccountQuota(provider: string, accountId: string, forceRefresh: boolean): Promise<{ quota: ProviderQuota | null; unavailable: boolean }> {
   const key = accountCacheKey(provider, accountId);
   const cached = accountQuotaCache.get(key);
-  if (!forceRefresh && cached && Date.now() - cached.ts < ACCOUNT_QUOTA_TTL_MS) return cached.quota;
+  if (!forceRefresh && cached && Date.now() - cached.ts < ACCOUNT_QUOTA_TTL_MS) return { quota: cached.quota, unavailable: false };
   const joinable = accountQuotaInflight.get(key);
   if (joinable) return joinable;

-  const probe = (async (): Promise<ProviderQuota | null> => {
+  const probe = (async (): Promise<{ quota: ProviderQuota | null; unavailable: boolean }> => {
     try {
       // Account-scoped token: refreshes (and persists) only THIS account, never the active one.
       const token = await getValidAccessTokenForAccount(provider, accountId);
       const quota = await fetchAnthropicUsageQuota(token);
-      // A failed probe keeps the previous good row rather than flashing an empty bar.
-      if (!quota) return cached?.quota ?? null;
-      accountQuotaCache.set(key, { ts: Date.now(), quota });
-      return quota;
+      // A failed probe keeps the previous good row rather than flashing an empty bar, but
+      // still bumps `ts` so we don't hammer upstream on every subsequent list refresh.
+      if (!quota) {
+        accountQuotaCache.set(key, { ts: Date.now(), quota: cached?.quota ?? null });
+        return { quota: cached?.quota ?? null, unavailable: true };
+      }
+      accountQuotaCache.set(key, { ts: Date.now(), quota });
+      return { quota, unavailable: false };
     } catch {
-      return cached?.quota ?? null;
+      accountQuotaCache.set(key, { ts: Date.now(), quota: cached?.quota ?? null });
+      return { quota: cached?.quota ?? null, unavailable: true };
     }
   })().finally(() => {
     if (accountQuotaInflight.get(key) === probe) accountQuotaInflight.delete(key);
   });
   accountQuotaInflight.set(key, probe);
   return probe;
 }

 export async function fetchProviderAccountQuotas(
   provider: string,
   forceRefresh = false,
 ): Promise<ProviderAccountQuota[]> {
   if (!supportsPerAccountQuota(provider)) return [];
   const set = getAccountSet(provider);
   if (!set) return [];
   return await Promise.all(set.accounts.map(async account => {
-    const quota = await fetchAccountQuota(provider, account.id, forceRefresh);
-    return quota
-      ? { accountId: account.id, quota }
-      : { accountId: account.id, quota: null, unavailable: true as const };
+    const { quota, unavailable } = await fetchAccountQuota(provider, account.id, forceRefresh);
+    return unavailable
+      ? { accountId: account.id, quota, unavailable: true as const }
+      : { accountId: account.id, quota };
   }));
 }

Note: this bug propagates downstream to GET /api/oauth/accounts?quota=1 in src/server/management/oauth-account-routes.ts (Lines 190-196), which forwards row.unavailable as-is, and ultimately to the GUI's quotaUnavailable field — no code change is needed there once this root cause is fixed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/providers/quota.ts` around lines 283 - 326, Update fetchAccountQuota and
fetchProviderAccountQuotas to preserve quota data together with an explicit
unavailable status, so failed refreshes remain marked unavailable even when
prior quota exists. Record failed probes in the account cache with a refreshed
timestamp and failure state to enforce ACCOUNT_QUOTA_TTL_MS backoff, while
retaining the last successful quota for display; ensure successful probes clear
the failure state and continue returning normal rows.

Comment on lines +86 to +100
test("a failing probe is flagged unavailable without dropping the other account", async () => {
await seedTwoAccounts();
globalThis.fetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
const auth = new Headers(init?.headers).get("authorization") ?? "";
// Anthropic rate-limits this endpoint; one 429 must not blank the sibling account.
if (auth.endsWith("token-first")) return new Response("rate limited", { status: 429 });
return new Response(usageBody(3, 21), { status: 200 });
}) as typeof fetch;

const rows = await fetchProviderAccountQuotas("anthropic");
const failed = rows.find(row => row.quota === null);
const ok = rows.find(row => row.quota !== null);
expect(failed?.unavailable).toBe(true);
expect(ok?.quota?.fiveHourPercent).toBe(3);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Missing regression test: probe fails after a prior success.

This test only covers accounts that never had a successful probe (fresh beforeEach cache). It doesn't cover the scenario where an account has a cached good value and a subsequent probe then fails — which is exactly the case where unavailable currently gets dropped in src/providers/quota.ts (see the comment there on Lines 283-326). Add a case that: (1) succeeds once to populate the cache, (2) force-refreshes with a failing response, and (3) asserts both that quota is preserved AND unavailable === true.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/provider-account-quota.test.ts` around lines 86 - 100, Add a regression
test alongside the existing provider quota tests that first returns a successful
probe to populate the account cache, then force-refreshes with a failing
response. Assert the refreshed result preserves the cached quota while setting
unavailable to true, using fetchProviderAccountQuotas and the existing
seed/setup helpers.

Source: Path instructions

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e466a7946

ℹ️ 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".

Comment thread src/providers/quota.ts
const token = await getValidAccessTokenForAccount(provider, accountId);
const quota = await fetchAnthropicUsageQuota(token);
// A failed probe keeps the previous good row rather than flashing an empty bar.
if (!quota) return cached?.quota ?? null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve failure status when returning cached quota

When a cached account is force-refreshed and Anthropic returns a non-OK response such as 429, this fallback returns the cached quota as though the probe succeeded. fetchProviderAccountQuotas consequently omits unavailable, so ?refresh=1 silently returns stale data and clients cannot report that the requested refresh failed. Preserve the cached quota and the probe-failure status separately.

Useful? React with 👍 / 👎.

<IconTrash style={{ width: 13, height: 13 }} aria-hidden="true" />
</button>
</div>
{account.quota && (

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface unavailable account quotas

When an account's initial probe fails because of an expired login, network error, or 429, the API returns quota: null with quotaUnavailable: true, but this component renders only truthy account.quota and never consumes the failure flag. The account therefore looks as if quota reporting does not exist, with no error or retry indication. Render a localized unavailable state when the flag is set.

AGENTS.md reference: gui/AGENTS.md:L5-L9

Useful? React with 👍 / 👎.

const wantQuota = url.searchParams.get("quota") === "1" && supportsPerAccountQuota(provider);
if (!wantQuota) return jsonResponse({ activeAccountId: status.activeAccountId ?? null, accounts });
const forceRefresh = url.searchParams.get("refresh") === "1";
const rows = await fetchProviderAccountQuotas(provider, forceRefresh);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Rebuild health after quota-triggered token refresh

For an expired Anthropic credential whose refresh fails terminally, this call marks the stored account needsReauth, but status, set, and the projected accounts array were all captured before the call. The response still reports that account as healthy, and the GUI has no periodic account-list refresh, so the reauthentication action remains absent until another action or page reload. Probe before projecting account health, or rebuild the status and health fields afterward.

Useful? React with 👍 / 👎.

</div>
{account.quota && (
<div className="pwi-auth-acct-quota">
<QuotaBars quota={account.quota} plan={null} threshold={80} t={t} layout="stacked" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Document Anthropic account quota behavior

This adds a user-visible per-account Anthropic quota surface and changes the displayed 5-hour window, but the commit does not update docs-site/; the four new docs/assets screenshots are also unreferenced anywhere in the repository. Update the public dashboard/provider documentation, keeping localized pages consistent with the English source.

AGENTS.md reference: AGENTS.md:L78-L79

Useful? React with 👍 / 👎.

Comment thread src/providers/quota.ts
} catch {
return null;
}
const quota = await fetchAnthropicUsageQuota(accessToken);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Share Anthropic quota probes across report paths

On every Providers-page load with Anthropic OAuth, the workspace requests both /api/provider-quotas and /api/oauth/accounts?...&quota=1. This provider-report path probes the active credential here, while the account path independently probes the same credential through a separate cache and inflight map, so the active account receives two near-simultaneous calls to Anthropic's rate-limited usage endpoint. Reuse the active per-account probe for the provider report, or share a single cache and inflight operation between both paths.

Useful? React with 👍 / 👎.

Comment thread package.json
{
"name": "@bitkyc08/opencodex",
"version": "2.7.35",
"version": "2.7.40",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Leave the package version to the release workflow

This feature commit advances the package version from 2.7.35 to 2.7.40, even though version bumps are owned by the repository's release authority and normally live in dedicated release: v... commits. Applied before that release commit, this advertises an unreleased version and takes the bump outside the guarded release flow; applied afterward, it is unrelated version churn. Revert this line from the feature change and let scripts/release.ts perform the bump.

AGENTS.md reference: AGENTS.md:L21-L22

Useful? React with 👍 / 👎.

Comment thread src/providers/quota.ts
const probe = (async (): Promise<ProviderQuota | null> => {
try {
// Account-scoped token: refreshes (and persists) only THIS account, never the active one.
const token = await getValidAccessTokenForAccount(provider, accountId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid adopting the global CLI token into background slots

When a stored Anthropic account originated from the local Claude CLI, is now a background account, and its token enters the refresh window, this new all-account probe calls getValidAccessTokenForAccount for it automatically. Anthropic's refresh path then checks the single global Claude CLI credential and persists any newer generation into the requested slot without verifying that its accountId or email matches; if the external CLI has since logged into another identity, merely opening Providers silently replaces the background slot's credential and later routes that slot's requests through the wrong account. Skip global-CLI adoption for background probes or require an identity match before persisting it.

AGENTS.md reference: AGENTS.md:L65-L71

Useful? React with 👍 / 👎.

@Wibias
Wibias marked this pull request as draft July 26, 2026 20:11

@Wibias Wibias left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Request changes ÔÇö do not merge until this branch is rebased onto current dev (resolve the package.json conflict without downgrading the version), and the per-account probe failure / credential-integrity defects below are fixed in this PR. Optional follow-ups are not accepted; every quality item below must ship here.

Summary

Base is correctly dev. Direction is right: Anthropic usage is per credential, so per-account probing with account-scoped tokens, a longer TTL, canonical fiveHourPercent / fiveHourResetAt, opt-in ?quota=1, and focused tests match the stated gaps. Tip 5e466a79 is still the current head. GitHub reports mergeable: CONFLICTING, mergeStateStatus: DIRTY, and the branch is 124 commits behind dev.

Blocking findings

  1. Merge conflict against current dev (mergeable=CONFLICTING, mergeStateStatus=DIRTY).

    • Verified via git merge-tree of 5e466a79 into dev @ 58f0fab3.
    • Conflicted file: package.json only (src/server/management/oauth-account-routes.ts auto-merges).
    • Conflict hunk:
      • dev: "version": "2.7.41"
      • this PR: "version": "2.7.40"
    • Required: rebase or merge latest dev into this branch and resolve in this PR. Keep 2.7.41 (current dev) ÔÇö do not downgrade. Prefer dropping the release: v2.7.40 / version bump from this feature PR entirely; version bumps belong to maintainer release promotion (scripts/release.ts), not a contributor quota UI fix.
  2. Failed probes drop unavailable once a prior good row exists, and do not advance the TTL  retry storm  src/providers/quota.ts ~L283324

    • Failure mode: On non-OK / thrown probe, fetchAccountQuota returns cached?.quota ?? null without writing a new cache entry (L296, L300). fetchProviderAccountQuotas then treats any truthy quota as success (L322ÔÇô324) and omits unavailable. After a first success, every later 429 / network / expired-login failure looks like a fresh success. Because a failed probe does not refresh ts, once the old TTL expires the next ?quota=1 load immediately re-probes again ÔÇö the author already observed Anthropic 429s; this path amplifies them. ?refresh=1 has the same silent-stale outcome.
    • Fix: Cache failure state separately from last-good quota (e.g. { ts, quota, unavailable? }). On failure: keep last-good quota for display, set unavailable: true, and advance ts so ACCOUNT_QUOTA_TTL_MS backs off. On success: clear unavailable. Propagate { quota, unavailable } from fetchAccountQuota instead of collapsing to ProviderQuota | null. Add the fail-after-success regression test CodeRabbit called for in tests/provider-account-quota.test.ts.
  3. Background Claude local-cli slots can adopt the global CLI token with no identity check  triggered from src/providers/quota.ts ~L293 via getValidAccessTokenForAccount  refreshAnthropicAccountWithLock / newerClaudeCredential in src/oauth/index.ts ~L304338

    • Failure mode: This PR probes every logged-in Anthropic account on Providers load (quota=1). For any background slot with source: "local-cli" whose stored generation differs from detectClaudeCodeToken(), newerClaudeCredential returns the disk credential and mergeAccountCredential persists it into that slot with no accountId / email match and no active-only guard (unlike KiroÔÇÖs active-only CLI import at ~L425ÔÇô426 and GrokÔÇÖs identity-gated authoritative). If the user later logged the Claude CLI into another identity, merely opening Providers can silently overwrite a background account and later route that slot through the wrong credential.
    • Fix in this PR (smallest safe options): (a) for per-account quota probes, use a non-adopting token path for non-active accounts (read stored access if still valid; on refresh need, skip CLI adoption / mark unavailable), and/or (b) gate Claude CLI adoption to the active account and/or same-identity checks, matching Kiro/Grok. Add a regression that proves probing a background local-cli account does not persist a mismatched disk credential.

Additional required quality changes (fix in this PR before merge)

  1. Drop / neutralize the version bump. Three-dot diff still changes package.json 2.7.35  2.7.40 via f47726a0 (release: v2.7.40). Out of scope; it is what conflicts with devs 2.7.41. Resolve by taking devs version (no version change in this PR).

  2. Surface quotaUnavailable in the GUI ÔÇö gui/src/components/provider-workspace/ProviderAuthPanel.tsx ~L247ÔÇô249

    • Types/API carry quotaUnavailable, but the row only renders {account.quota && <QuotaBars .../>}. Failed first probe ÔåÆ blank (looks like ÔÇ£no limitsÔÇØ); failed later probe ÔåÆ bars look fresh. Render a localized stale/unavailable hint (and still show last-good bars when present).
  3. Rebuild account health after quota-triggered token work ÔÇö src/server/management/oauth-account-routes.ts ~L164ÔÇô195

    • Health fields are projected from status before fetchProviderAccountQuotas. A probe that terminally fails refresh / marks needsReauth still returns pre-probe healthy rows; the GUI has no periodic account-list refresh, so reauth UI can stay missing until reload. Probe first, or rebuild status/health after probing.
  4. Share Anthropic usage probes across provider-report and per-account paths ÔÇö src/providers/quota.ts (fetchAnthropicQuota vs fetchProviderAccountQuotas)

    • Providers load hits both /api/provider-quotas and /api/oauth/accounts?...&quota=1, so the active credential is probed twice through separate caches/inflight maps. Reuse the active per-account probe (or one shared cache/inflight) for the provider report.
  5. clearAccountQuotaCache must also clear accountQuotaInflight ÔÇö src/providers/quota.ts ~L272ÔÇô280 / L253

    • Logout/remove clears the cache map only. An in-flight probe can repopulate stale rows after clear. Clear matching inflight entries too.
  6. Key Usage/overview quota refetch on active account identity, not only activeAccountNeedsReauth ÔÇö gui/src/components/provider-workspace/ProviderWorkspaceShell.tsx ~L180ÔÇô207

    • switchAccount already calls fetchProviderQuotas(true) for the Providers hook, but the shell keeps a separate /api/provider-quotas effect. Depending on object-identity of a boolean map is brittle (two healthy accounts ÔåÆ empty maps that only refetch because useMemo allocates a new object). Key on anthropic/active account id (or a dedicated refresh token bumped by switchAccount) so the stated ÔÇ£no reload after switchÔÇØ fix is explicit.
  7. Docs sync for user-visible behavior ÔÇö missing docs-site/ updates; docs/assets/anthropic-*.png are unreferenced.

    • AGENTS.md docs-sync: document per-account Claude rate limits / canonical 5-hour window on the dashboard/providers surface (English + keep locales from contradicting). Either reference the screenshots from docs or drop unused assets from the PR.
  8. Re-run full CI on the rebased tip. Tip CI: ubuntu/macOS Cross-platform green; windows-latest was cancelled (not a clean green matrix). After rebase/conflict resolution, require Cross-platform CI (incl. Windows), typecheck, tests, privacy:scan, and GUI lint/build green on the mergeable SHA.

Optional / non-blocking

None. Follow-up PRs are rejected for this review; every item above is required in this PR before merge.

Standards / Spec / Security

  • Standards: Bun-native + focused tests are directionally good (tests/provider-account-quota.test.ts, updated tests/provider-quota.test.ts). Gaps: release-owned package.json bump, docs-site sync, incomplete failure-state / cache-clear invariants, fragile GUI refresh coupling.
  • Spec: Matches the PRÔÇÖs three stated gaps (per-account bars, canonical 5h fields, refresh after active switch) at the happy path. Diverges on stated ÔÇ£failed probe ÔåÆ unavailable while keeping last-goodÔÇØ (API flag dropped after first success; GUI never consumes the flag) and amplifies credential side effects not called out in the design notes.
  • Security / privacy: No request-body / token logging observed in the diff. Credential integrity issue in blocking finding #3 (background local-cli adoption) is the security gate for this PR. Screenshots claim emails were redacted before capture ÔÇö good; keep real addresses out of committed assets.

CI / process gates

Gate Status
Base branch dev (correct)
mergeable / mergeStateStatus CONFLICTING / DIRTY ÔÇö block
Divergence ahead 2 / behind 124 vs dev
Conflicted files package.json (2.7.41 vs 2.7.40)
Cross-platform CI @ 5e466a79 ubuntu/macOS pass; windows-latest cancelled
react-doctor / enforce-target / privacy (author-reported) pass / pass / author claims pass
Review event CHANGES REQUESTED

Reviewed tip: 5e466a79461ad1934ff88a1eb960dcc2824f2f26.

After fixes

Once all blocking + required quality changes above are addressed (and conflicts/rebase done), mark this PR Ready for review (leave draft only until then). Then ping for re-review / CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants