Skip to content

Commit 30ba34f

Browse files
ndycodeclaude
andcommitted
fix(rotation): make selection token-bucket-aware; stop persisting local limiter
Fixes a cross-process regression introduced in 6.4.0. The 6.4.0 fix for sticky/hybrid stalling on local token-bucket depletion wrote a synthetic window into account.rateLimitResetTimes — but that field is persisted to the shared accounts file and reloaded by every process, so one process draining its OWN in-memory proactive limiter could spuriously rate-limit a server-healthy account in OTHER concurrent processes (the multi-process / PID-offset mode this tool targets). Root-cause fix: - TokenBucketTracker gains hasToken() / msUntilToken(). - AccountRotation selection (sticky, hybrid, round-robin) and getMinWaitTimeForFamily are now token-bucket-aware in-memory via a shared isSelectable() helper: a locally-depleted account is skipped with NO persisted state, and an all-depleted pool waits for refill (no spurious 503). - The local-depletion branch in index.ts no longer calls recordRateLimit() (which applied a server-429-style health penalty to a purely-local throttle) and no longer writes a synthetic window. Removed the now-unused LOCAL_TOKEN_DEPLETION_COOLDOWN_MS constant. Also fixes codex-warm reporting a quota-exhausted account as "warmed": a 429 is now classified by reason (quota/usage_limit -> exhausted/failed; transient tokens/concurrent -> opened/warmed). Adds cross-process-leak regression tests + warm 429-reason tests. Full suite: 2613 passed, 1 skipped. Bumps to 6.4.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e1a8576 commit 30ba34f

14 files changed

Lines changed: 303 additions & 112 deletions

.codex-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "oc-codex-multi-auth",
3-
"version": "6.4.0",
3+
"version": "6.4.1",
44
"description": "Install and operate oc-codex-multi-auth for OpenCode with ChatGPT Plus/Pro OAuth, Codex/GPT-5 routing, multi-account rotation, account switching, health checks, diagnostics, quota status, and recovery tools.",
55
"skills": "./skills/",
66
"interface": {

.release-please-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
".": "6.4.0"
2+
".": "6.4.1"
33
}

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [6.4.1] - 2026-06-30
11+
12+
### Fixed
13+
- Local token-bucket depletion no longer leaks into persisted, cross-process state. 6.4.0 made a depleted account rotate by writing a short synthetic window into `rateLimitResetTimes` — but that field is saved to the shared accounts file and reloaded by every process, so one process exhausting its own in-memory proactive limiter could spuriously mark a server-healthy account as rate-limited in OTHER concurrent processes (the multi-process/PID-offset deployment this tool targets). Account selection (`sticky`, `hybrid`, `round-robin`) and `getMinWaitTimeForFamily` are now token-bucket-aware directly: a locally-depleted account is skipped in-memory with no persisted state, and an all-depleted pool waits for token refill instead of returning a spurious 503. The local skip also no longer records a server-429-style health penalty, so a busy-but-healthy account is not deprioritized in `hybrid` scoring. (#183)
14+
- `codex-warm` no longer reports a quota-exhausted account as "warmed". A `429` is now classified by reason: a `quota`/`usage_limit` 429 (the window is already spent) is surfaced as a distinct failure, while a transient `tokens`/`concurrent` 429 (window active) still counts as warmed. (#182)
15+
1016
## [6.4.0] - 2026-06-30
1117

1218
### Added

index.ts

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2139,33 +2139,28 @@ while (attempted.size < Math.max(1, accountCount)) {
21392139
// Consume a token before making the request for proactive rate limiting
21402140
const tokenConsumed = accountManager.consumeToken(account, modelFamily, model);
21412141
if (!tokenConsumed) {
2142-
accountManager.recordRateLimit(account, modelFamily, model);
2143-
// Mark a short, auto-expiring rate-limit window for this
2144-
// (family, model) so the depleted account becomes ineligible in
2145-
// `isRateLimitedForFamily`. Without this, drain-first `sticky`
2146-
// (and the `hybrid` fast-path) re-select the SAME depleted account
2147-
// on the next traversal iteration — `attempted` then trips the
2148-
// guard and the loop 503s while other accounts still have quota.
2149-
// The window also feeds `getMinWaitTimeForFamily`, so an all-
2150-
// depleted pool waits for refill instead of failing fast.
2151-
accountManager.markRateLimitedWithReason(
2152-
account,
2153-
ACCOUNT_LIMITS.LOCAL_TOKEN_DEPLETION_COOLDOWN_MS,
2154-
modelFamily,
2155-
"tokens",
2156-
model,
2157-
);
2142+
// Local (in-memory, per-process) proactive limiter is depleted for
2143+
// this account. The rotation selectors are token-bucket-aware, so
2144+
// they will not re-select this account until a token refills — no
2145+
// synthetic rate-limit window is written. Crucially we do NOT call
2146+
// recordRateLimit() here: that records a server-429-style health
2147+
// penalty and would mis-attribute a purely-local throttle as an
2148+
// upstream rejection. We also must not persist any local-limiter
2149+
// state (rateLimitResetTimes is written to the shared accounts file
2150+
// and would spuriously rate-limit healthy accounts in other
2151+
// processes). Just account the rotation and move on.
21582152
runtimeMetrics.accountRotations++;
21592153
runtimeMetrics.lastError =
21602154
`Local token bucket depleted for account ${account.index + 1} (${modelFamily}${model ? `:${model}` : ""})`;
21612155
runtimeMetrics.lastErrorCategory = "rate-limit-local";
21622156
logWarn(
21632157
`Skipping account ${account.index + 1}: local token bucket depleted for ${modelFamily}${model ? `:${model}` : ""}`,
21642158
);
2165-
// Skip THIS account and rotate to the next one. `account.index` is
2166-
// already in `attempted` (added above), so the traversal loop guard
2167-
// prevents reselecting it and terminates once all are exhausted.
2168-
// Using `break` here would abandon every other healthy account.
2159+
// Skip THIS account and rotate to the next one. The selector's
2160+
// token-bucket awareness guarantees it advances to an account with
2161+
// quota (or returns null so the wait/retry path engages via the
2162+
// token-refill wait in getMinWaitTimeForFamily). `break` would
2163+
// abandon every other healthy account.
21692164
continue;
21702165
}
21712166

lib/accounts/rotation.ts

Lines changed: 48 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,31 @@ import type { AccountState, ManagedAccount } from "./state.js";
2828
export class AccountRotation {
2929
constructor(private readonly state: AccountState) {}
3030

31+
/**
32+
* Whether an account can serve a request for this (family, model) right now:
33+
* enabled, not rate-limited (real server 429s tracked in rateLimitResetTimes),
34+
* not cooling down, AND its in-memory local token bucket has a token.
35+
*
36+
* The token-bucket check is what keeps a locally-depleted account out of
37+
* selection. It is intentionally evaluated here (in-memory, per-process)
38+
* rather than by writing a synthetic window into the persisted
39+
* rateLimitResetTimes — that would leak a per-process proactive-limiter
40+
* signal into the cross-process accounts file and spuriously rate-limit
41+
* server-healthy accounts in other processes.
42+
*/
43+
private isSelectable(
44+
account: ManagedAccount,
45+
family: ModelFamily,
46+
model?: string | null,
47+
): boolean {
48+
if (account.enabled === false) return false;
49+
clearExpiredRateLimits(account);
50+
if (isRateLimitedForFamily(account, family, model)) return false;
51+
if (this.state.isAccountCoolingDown(account)) return false;
52+
const quotaKey = model ? `${family}:${model}` : family;
53+
return getTokenTracker().hasToken(account.index, quotaKey);
54+
}
55+
3156
getCurrentOrNextForFamily(
3257
family: ModelFamily,
3358
model?: string | null,
@@ -41,15 +66,7 @@ export class AccountRotation {
4166
const idx = (cursor + i) % count;
4267
const account = this.state.accounts[idx];
4368
if (!account) continue;
44-
if (account.enabled === false) continue;
45-
46-
clearExpiredRateLimits(account);
47-
if (
48-
isRateLimitedForFamily(account, family, model) ||
49-
this.state.isAccountCoolingDown(account)
50-
) {
51-
continue;
52-
}
69+
if (!this.isSelectable(account, family, model)) continue;
5370

5471
this.state.cursorByFamily[family] = (idx + 1) % count;
5572
this.state.currentAccountIndexByFamily[family] = idx;
@@ -70,15 +87,7 @@ export class AccountRotation {
7087
const idx = (cursor + i) % count;
7188
const account = this.state.accounts[idx];
7289
if (!account) continue;
73-
if (account.enabled === false) continue;
74-
75-
clearExpiredRateLimits(account);
76-
if (
77-
isRateLimitedForFamily(account, family, model) ||
78-
this.state.isAccountCoolingDown(account)
79-
) {
80-
continue;
81-
}
90+
if (!this.isSelectable(account, family, model)) continue;
8291

8392
this.state.cursorByFamily[family] = (idx + 1) % count;
8493
account.lastUsed = nowMs();
@@ -102,15 +111,9 @@ export class AccountRotation {
102111
if (currentAccount) {
103112
if (currentAccount.enabled === false) {
104113
// Fall through to hybrid selection.
105-
} else {
106-
clearExpiredRateLimits(currentAccount);
107-
if (
108-
!isRateLimitedForFamily(currentAccount, family, model) &&
109-
!this.state.isAccountCoolingDown(currentAccount)
110-
) {
111-
currentAccount.lastUsed = nowMs();
112-
return currentAccount;
113-
}
114+
} else if (this.isSelectable(currentAccount, family, model)) {
115+
currentAccount.lastUsed = nowMs();
116+
return currentAccount;
114117
}
115118
}
116119
}
@@ -123,13 +126,9 @@ export class AccountRotation {
123126
.map((account): AccountWithMetrics | null => {
124127
if (!account) return null;
125128
if (account.enabled === false) return null;
126-
clearExpiredRateLimits(account);
127-
const isAvailable =
128-
!isRateLimitedForFamily(account, family, model) &&
129-
!this.state.isAccountCoolingDown(account);
130129
return {
131130
index: account.index,
132-
isAvailable,
131+
isAvailable: this.isSelectable(account, family, model),
133132
lastUsed: account.lastUsed,
134133
};
135134
})
@@ -177,20 +176,11 @@ export class AccountRotation {
177176
const count = this.state.accounts.length;
178177
if (count === 0) return null;
179178

180-
const isAvailable = (account: ManagedAccount): boolean => {
181-
if (account.enabled === false) return false;
182-
clearExpiredRateLimits(account);
183-
return (
184-
!isRateLimitedForFamily(account, family, model) &&
185-
!this.state.isAccountCoolingDown(account)
186-
);
187-
};
188-
189179
// Prefer the account we are already pinned to while it still has quota.
190180
const currentIndex = this.state.currentAccountIndexByFamily[family];
191181
if (currentIndex >= 0 && currentIndex < count) {
192182
const currentAccount = this.state.accounts[currentIndex];
193-
if (currentAccount && isAvailable(currentAccount)) {
183+
if (currentAccount && this.isSelectable(currentAccount, family, model)) {
194184
currentAccount.lastUsed = nowMs();
195185
return currentAccount;
196186
}
@@ -201,7 +191,7 @@ export class AccountRotation {
201191
for (let idx = 0; idx < count; idx++) {
202192
const account = this.state.accounts[idx];
203193
if (!account) continue;
204-
if (!isAvailable(account)) continue;
194+
if (!this.isSelectable(account, family, model)) continue;
205195

206196
this.state.currentAccountIndexByFamily[family] = idx;
207197
this.state.cursorByFamily[family] = (idx + 1) % count;
@@ -326,19 +316,17 @@ export class AccountRotation {
326316
const enabledAccounts = this.state.accounts.filter(
327317
(account) => account.enabled !== false,
328318
);
329-
const available = enabledAccounts.filter((account) => {
330-
clearExpiredRateLimits(account);
331-
return (
332-
!isRateLimitedForFamily(account, family, model) &&
333-
!this.state.isAccountCoolingDown(account)
334-
);
335-
});
319+
const available = enabledAccounts.filter((account) =>
320+
this.isSelectable(account, family, model),
321+
);
336322
if (available.length > 0) return 0;
337323
if (enabledAccounts.length === 0) return 0;
338324

339325
const waitTimes: number[] = [];
340326
const baseKey = getQuotaKey(family);
341327
const modelKey = model ? getQuotaKey(family, model) : null;
328+
const tokenQuotaKey = model ? `${family}:${model}` : family;
329+
const tokenTracker = getTokenTracker();
342330

343331
for (const account of enabledAccounts) {
344332
const baseResetAt = account.rateLimitResetTimes[baseKey];
@@ -356,6 +344,16 @@ export class AccountRotation {
356344
if (typeof account.coolingDownUntil === "number") {
357345
waitTimes.push(Math.max(0, account.coolingDownUntil - now));
358346
}
347+
348+
// An account blocked only by a depleted local token bucket becomes
349+
// available again after refill; include that wait so a fully
350+
// token-depleted pool waits for refill instead of returning 0 (503).
351+
if (account.enabled !== false) {
352+
const tokenWait = tokenTracker.msUntilToken(account.index, tokenQuotaKey);
353+
if (tokenWait > 0 && Number.isFinite(tokenWait)) {
354+
waitTimes.push(tokenWait);
355+
}
356+
}
359357
}
360358

361359
return waitTimes.length > 0 ? Math.min(...waitTimes) : 0;

lib/accounts/warm-request.ts

Lines changed: 73 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import { CODEX_BASE_URL } from "../constants.js";
1818
import { createCodexHeaders } from "../request/fetch-helpers.js";
1919
import { getCodexInstructions } from "../prompts/codex.js";
20+
import { parseRateLimitReason } from "./rate-limits.js";
2021
import type { RequestBody } from "../types.js";
2122
import { createLogger } from "../logger.js";
2223

@@ -62,16 +63,33 @@ export async function buildWarmRequestBody(model = WARM_MODEL): Promise<RequestB
6263
};
6364
}
6465

66+
/**
67+
* Outcome of a warm request.
68+
* - `opened`: the request started/confirmed the usage window (2xx, or a 429
69+
* whose reason is a transient token/concurrency limit — the window is ticking).
70+
* - `exhausted`: a 429 whose reason is quota/usage-limit — the account's window
71+
* is already spent, so warming it is meaningless. Reported distinctly so the
72+
* tool does not claim a quota-dead account was "warmed".
73+
*/
74+
export type WarmRequestStatus = "opened" | "exhausted";
75+
76+
export interface WarmRequestResult {
77+
status: WarmRequestStatus;
78+
detail?: string;
79+
}
80+
6581
/**
6682
* Send one warm request to open the account's usage window.
6783
*
68-
* Resolves `true` when the upstream accepted the request enough to start the
69-
* window. A 2xx clearly opens it; a 429 (rate-limited) means the window is
70-
* already open/active, which still satisfies the user intent ("the window is
71-
* ticking"). Any other non-2xx, or a network/timeout error, throws so the
72-
* caller records the account as failed.
84+
* Resolves with `{ status: "opened" }` when the upstream started/confirmed the
85+
* window (2xx, or a non-quota 429 meaning the window is already active), or
86+
* `{ status: "exhausted" }` for a quota/usage-limit 429 (window already spent).
87+
* Any other non-2xx, or a network/timeout error, throws so the caller records
88+
* the account as failed.
7389
*/
74-
export async function warmAccountWindow(params: WarmRequestParams): Promise<boolean> {
90+
export async function warmAccountWindow(
91+
params: WarmRequestParams,
92+
): Promise<WarmRequestResult> {
7593
const doFetch = params.fetchImpl ?? fetch;
7694
const body = await buildWarmRequestBody();
7795
const headers = createCodexHeaders(undefined, params.accountId, params.accessToken, {
@@ -92,27 +110,67 @@ export async function warmAccountWindow(params: WarmRequestParams): Promise<bool
92110
signal: controller.signal,
93111
});
94112

95-
// Drain/cancel the SSE stream immediately — we only needed to start the
96-
// window, not consume the response.
113+
if (response.ok) {
114+
// Drain/cancel the SSE stream — we only needed to start the window.
115+
try {
116+
await response.body?.cancel();
117+
} catch {
118+
// Ignore cancellation failures.
119+
}
120+
return { status: "opened" };
121+
}
122+
123+
// Read the error body (small) BEFORE classifying so a 429 quota-exhausted
124+
// account is not mis-reported as warmed.
125+
let bodyText = "";
97126
try {
98-
await response.body?.cancel();
127+
bodyText = (await response.text()).slice(0, 2048);
99128
} catch {
100-
// Ignore cancellation failures.
129+
// Ignore body-read failures; fall back to status-only classification.
101130
}
102131

103-
if (response.ok) return true;
104-
105-
// 429 = window already open and currently rate-limited. The user's goal
106-
// (window is ticking) is satisfied, so treat it as a successful warm.
107132
if (response.status === 429) {
133+
const reason = parseRateLimitReason(extractErrorCode(bodyText));
134+
if (reason === "quota") {
135+
log.debug("Warm ping hit 429 quota limit — account window already spent", {
136+
accountId: params.accountId,
137+
});
138+
return { status: "exhausted", detail: "quota/usage limit reached" };
139+
}
140+
// token/concurrent/unknown → the window is active and ticking.
108141
log.debug("Warm ping hit 429 — window already active", {
109142
accountId: params.accountId,
143+
reason,
110144
});
111-
return true;
145+
return { status: "opened" };
112146
}
113147

114148
throw new Error(`Warm request failed: HTTP ${response.status}`);
115149
} finally {
116150
clearTimeout(timeout);
117151
}
118152
}
153+
154+
/**
155+
* Pull a rate-limit error code/message out of a JSON or plain-text error body
156+
* so {@link parseRateLimitReason} can classify the 429. Best-effort: returns
157+
* the raw text when JSON parsing fails so substring matching still works.
158+
*/
159+
function extractErrorCode(bodyText: string): string | undefined {
160+
if (!bodyText) return undefined;
161+
try {
162+
const parsed = JSON.parse(bodyText) as {
163+
error?: { code?: string; type?: string; message?: string };
164+
code?: string;
165+
};
166+
return (
167+
parsed.error?.code ??
168+
parsed.error?.type ??
169+
parsed.error?.message ??
170+
parsed.code ??
171+
bodyText
172+
);
173+
} catch {
174+
return bodyText;
175+
}
176+
}

lib/constants.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,4 @@ export const ACCOUNT_LIMITS = {
105105
AUTH_FAILURE_COOLDOWN_MS: 30_000,
106106
/** Number of consecutive auth failures before auto-removing account */
107107
MAX_AUTH_FAILURES_BEFORE_REMOVAL: 3,
108-
/**
109-
* Transient rate-limit window (ms) applied to an account when its local
110-
* client-side token bucket is depleted (proactive limiter, not a server
111-
* 429). Sized to ~one token refill at the default 6 tokens/min so the
112-
* account becomes selectable again as soon as a token regenerates. This
113-
* window makes the depleted account ineligible in `isRateLimitedForFamily`,
114-
* so every rotation strategy (including drain-first `sticky`, which would
115-
* otherwise re-pin the same depleted account and stall the request) advances
116-
* to the next account with quota instead of returning a spurious 503.
117-
*/
118-
LOCAL_TOKEN_DEPLETION_COOLDOWN_MS: 10_000,
119108
} as const;

0 commit comments

Comments
 (0)