diff --git a/devlog/_plan/260912_release_regression_train/025_sweep_results.md b/devlog/_plan/260912_release_regression_train/025_sweep_results.md new file mode 100644 index 0000000000..f35f4f98a5 --- /dev/null +++ b/devlog/_plan/260912_release_regression_train/025_sweep_results.md @@ -0,0 +1,74 @@ +# wp3 — sweep results + +Eight read-only lanes covered every non-merge commit in `e432cf565a..c27a4831a9`. Six +returned CLEAN, one returned MINOR, and one returned BLOCKING. Every finding below was +re-derived against source by a second reviewer before it was accepted. + +| Lane | Scope | Verdict | +|---|---|---| +| 1 | codex routing, combo resolve, provider quota | BLOCKING | +| 2 | responses/chat/claude-messages/bridge/vision/search | CLEAN | +| 3 | Claude inbound cache prefix | CLEAN | +| 4 | Codex inject, restore, history | CLEAN | +| 5 | OAuth, adapters, Devin CLI credentials | CLEAN | +| 6 | remote-control, update job | CLEAN | +| 7 | integrations, client export, CLI | MINOR | +| 8 | GUI catalog, Combo, Cline dialog | CLEAN | + +## Release-blocking: the quota-avoid window skips the main account + +#4368 (`d42a1363dc`) split a quota refusal into two durations. The cooldown still caps at +fifteen minutes, and a new `quotaAvoidUntil` records the window the refusal actually +announced, bounded at six hours. Pool candidates honour it at +`src/codex/routing.ts:1409`. The main account does not. + +`isSelectableCodexPoolAccount` rejects `__main__`, so the main login reaches the +candidate list only through the re-insertion block at `src/codex/routing.ts:1414`, and +that block checks `isCodexAccountSoftAvoided` but never `isCodexQuotaAvoided`. +`getCodexQuotaHealthSnapshot` reads `cooldownUntil` alone, so once the fifteen-minute +cooldown lapses the main account is a first-class candidate again while its announced +window still has hours left. + +A user with the main login plus a pool sees exactly the failure #4368 was written to +stop: pool accounts stay avoided for up to six hours, the main account returns after +fifteen minutes, and the quota strategy ranks it coolest because it ranks on a weekly bar +a burst limit never touches. A bound thread drops its affinity and is re-pinned to the +same exhausted account. With no pool the symptom is unchanged, because the last-resort +branch would hand back the only account anyway. + +## Same commit, same omission, two more paths + +The commit states that "an operator clearing the cooldown or naming the account overrules +it". Neither does. + +`resetCodexRoutingForManualSelection` (`src/codex/routing.ts:988`) drops +`quotaAvoidUntil` from `upstreamHealth` and returns early when that map has no entry. A +reset-derived 429 writes to `quotaScopedHealth` instead and returns at +`src/codex/routing.ts:2763`, so naming the account clears nothing in the case the commit +actually introduced. + +`clearCodexAccountCooldown` (`src/codex/routing.ts:1066`) destructures the cooldown and +probe-lease fields and carries `quotaAvoidUntil` through in `...rest`. Probe recovery at +`src/codex/routing.ts:863` deliberately drops it, with a comment saying that leaving it +would make the escape hatch stop escaping. The operator's escape hatch has the defect the +automatic one avoids. + +An exhaustive scan found `quotaAvoidUntil` and `isCodexQuotaAvoided` used only in +`src/codex/routing.ts`, and no fourth omission. + +## Non-blocking + +Lane 7 found only the top-level help text advertising `(14 clients)` against a registry +of fifteen, which #4390 already corrected. The count never reaches dispatch; +`ocx export --client cline` and the dashboard switch both read the registry. + +Lane 6 noted that `RemoteControlHost` defaults to the full capability set when a caller +omits `allowedCapabilities`. No runtime file imports `src/remote-control`, so nothing +reaches it; the documented `OCX_REMOTE_WORKSPACE_ENABLED` flag does not exist in `src/` +yet. That is a default to settle when the activation layer lands, not a regression here. + +## What the lanes did not do + +No lane executed the product. Every verdict is a source and diff reading, cross-checked +against the tests each commit shipped. Local product tests, builds, typecheck and install +were NOT RUN. diff --git a/src/codex/routing.ts b/src/codex/routing.ts index e26c902b4d..deb7a13f19 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -985,14 +985,27 @@ export function resetCodexRoutingForManualSelection(accountId: string): void { seedPoolRotationAccount(codexPoolKeyForScope(scope), accountId); } } - const current = upstreamHealth.get(accountId); - if (!current) return; - const preserved = preservedCooldownFields(current); // Quota avoidance is a preference, like the soft avoid dropped above, and an operator naming // this account has overruled it. The hard cooldown is the part that survives. - const { quotaAvoidUntil: _avoid, ...retained } = preserved; - if (Object.keys(retained).length === 0) upstreamHealth.delete(accountId); - else upstreamHealth.set(accountId, { consecutiveFailures: 0, ...retained }); + const overrule = (health: CodexUpstreamHealth) => { + const { quotaAvoidUntil: _avoid, ...retained } = preservedCooldownFields(health); + return retained; + }; + const current = upstreamHealth.get(accountId); + if (current) { + const retained = overrule(current); + if (Object.keys(retained).length === 0) upstreamHealth.delete(accountId); + else upstreamHealth.set(accountId, { consecutiveFailures: 0, ...retained }); + } + // A reset-derived refusal records its avoidance on the SCOPED map and returns before the + // account-wide entry is written, so naming the account has to reach that map too. Stopping + // at `upstreamHealth` — and returning early when it holds nothing — overruled nothing in + // the case that produces the avoidance this function exists to overrule. + for (const [scope, health] of [...(quotaScopedHealth.get(accountId) ?? [])]) { + const retained = overrule(health); + if (Object.keys(retained).length === 0) deleteScopedHealth(accountId, scope); + else setScopedHealth(accountId, scope, { consecutiveFailures: 0, ...retained }); + } } export function getCodexAccountCooldownUntil(accountId: string, now = Date.now()): number | null { @@ -1073,6 +1086,11 @@ export function clearCodexAccountCooldown(accountId: string, now = Date.now()): cooldownSource: _source, probeLeaseId: _leaseId, probeLeaseGeneration: _leaseGeneration, + // Same reasoning as the probe recovery above: "the quota window moved" is a statement + // about the whole refusal, so the avoidance it announced goes with the block it + // produced. Keeping it would leave this escape hatch not escaping, because selection + // would still pass over the account for as long as the announced window runs. + quotaAvoidUntil: _avoid, ...rest } = health; return { @@ -1417,6 +1435,12 @@ function getEligiblePoolAccounts( && (!isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID) || hasMainAccountRefreshGrant()) && getCodexQuotaHealthSnapshot(MAIN_CODEX_ACCOUNT_ID, quotaScope, now) === null && !isCodexAccountSoftAvoided(MAIN_CODEX_ACCOUNT_ID, now) + // The main login is not in `config.codexAccounts`, so it never passes through the + // filters above and this is the only place an avoidance window can exclude it. Without + // this the window a refusal announced applies to the pool but not to the account that + // earned it: the cooldown caps at fifteen minutes, the window runs up to six hours, and + // in between the main account returns as a first-class candidate. + && !isCodexQuotaAvoided(MAIN_CODEX_ACCOUNT_ID, quotaScope, now) && (!skipFailoverReadyCandidates || !shouldFailover(config, MAIN_CODEX_ACCOUNT_ID, now)) && isCodexAccountUsable(config, MAIN_CODEX_ACCOUNT_ID, selectionOptions) ) { diff --git a/structure/providers/openai-tiers.md b/structure/providers/openai-tiers.md index a44edff557..92737b5301 100644 --- a/structure/providers/openai-tiers.md +++ b/structure/providers/openai-tiers.md @@ -92,6 +92,18 @@ requests keep their captured credential. An all-paused pool fails closed. The dashboard's bulk pause action refreshes all account quotas and mutates only accounts whose plan-relevant window is freshly confirmed at exactly 100%; unknown and failed refreshes are skipped. +A quota refusal that announces a reset carries two durations, not one. The hard cooldown governs +blocking and keeps its cap, and a separate avoidance window records the period the refusal actually +announced, bounded at six hours so a reset days out cannot take an account out of rotation for that +long. Selection and affinity reuse both pass over an account while its window is live. The window +binds the stable `__main__` alias on the same terms as an added account: the main login is not in +the configured pool and enters candidacy through its own re-insertion path, which applies the same +avoidance check the pool filters apply. Avoidance stays soft. Last-resort selection still reaches +the account when nothing else can serve, a successful recovery probe drops the window with the +cooldown it belonged to, and both operator escapes remove it: clearing a cooldown and naming an +account each clear the window from the account-wide entry and from every scoped entry, because a +reset-derived refusal records only the scoped one. + A confirmed manual reset-credit consumption may immediately reconcile that account's eligible pre-existing ordinary reset-derived cooldown after a complete, non-exhausted usage observation started after the reset. Paused or reauthentication-required accounts and diff --git a/tests/codex-integration/codex-routing.test.ts b/tests/codex-integration/codex-routing.test.ts index 5a5d0159a7..92375ef2fc 100644 --- a/tests/codex-integration/codex-routing.test.ts +++ b/tests/codex-integration/codex-routing.test.ts @@ -971,6 +971,81 @@ describe("codex routing", () => { expect(resolveCodexAccountForThread("spark-refused", config, now + 16 * 60_000, "shared")).toBe("a"); }); + test("the main login is passed over by the window its own refusal announced", () => { + // The main account is not in `config.codexAccounts`, so it reaches selection through a + // separate re-insertion branch. That branch is the only place an avoidance window can + // exclude it, and it is the case the pool filters above cannot cover. + writeFileSync(join(TEST_DIR, "auth.json"), JSON.stringify({ + tokens: { access_token: "main-access", account_id: "main-chatgpt-id" }, + })); + const config = makeConfig({ + codexAccounts: [{ id: "a", email: "a@test", isMain: false }], + activeCodexAccountId: MAIN_CODEX_ACCOUNT_ID, + }); + const now = 1_800_000_000_000; + updateAccountQuota(MAIN_CODEX_ACCOUNT_ID, 10); + updateAccountQuota("a", 20); + expect(resolveCodexAccountForThread("main-spark-first", config, now, "spark")).toBe(MAIN_CODEX_ACCOUNT_ID); + + recordCodexUpstreamOutcome(config, MAIN_CODEX_ACCOUNT_ID, 429, { + now, + threadId: "main-spark-first", + modelId: "gpt-5.3-codex-spark", + resetAt: Math.floor((now + 4 * 60 * 60_000) / 1_000), + }); + + // Selection has to actually reach the re-insertion branch for this to prove anything. + // Leaving the main login active would exclude it by id on the fallback instead, so move + // the active account and put it over the switch threshold: now a cooler candidate is + // wanted, and the only thing that can keep the main login out is its avoidance window. + config.activeCodexAccountId = "a"; + updateAccountQuota("a", 85); + + // The capped cooldown has lapsed and the weekly bar still reads coolest, so without the + // window this request goes straight back to the account that just refused one. + expect(resolveCodexAccountForThread("main-spark-next", config, now + 16 * 60_000, "spark")).toBe("a"); + }); + + test("naming the account overrules an avoidance the refusal recorded on the scoped lane", () => { + const config = makeConfig(); + const now = 1_800_000_000_000; + updateAccountQuota("a", 10); + updateAccountQuota("b", 20); + recordCodexUpstreamOutcome(config, "a", 429, { + now, + threadId: "scoped-manual", + modelId: "gpt-5.3-codex-spark", + resetAt: Math.floor((now + 4 * 60 * 60_000) / 1_000), + }); + expect(resolveCodexAccountForThread("scoped-avoided", config, now + 16 * 60_000, "spark")).toBe("b"); + + // A reset-derived refusal writes only the scoped entry, so an operator naming the account + // has to reach that map. Stopping at the account-wide entry leaves the pick refused. + config.activeCodexAccountId = "a"; + resetCodexRoutingForManualSelection("a"); + + expect(resolveCodexAccountForThread("scoped-named", config, now + 17 * 60_000, "spark")).toBe("a"); + }); + + test("clearing the cooldown also lifts the avoidance that refusal announced", () => { + const config = makeConfig(); + const now = 1_800_000_000_000; + updateAccountQuota("a", 10); + updateAccountQuota("b", 20); + recordCodexUpstreamOutcome(config, "a", 429, { + now, + modelId: "gpt-5.3-codex-spark", + resetAt: Math.floor((now + 4 * 60 * 60_000) / 1_000), + }); + expect(resolveCodexAccountForThread("cleared-before", config, now + 16 * 60_000, "spark")).toBe("b"); + + // The escape hatch has to escape. Automatic probe recovery already drops the window; an + // operator lifting the same cooldown by hand was leaving it in place for up to six hours. + expect(clearCodexAccountCooldown("a", now + 60_000)).toBe(true); + + expect(resolveCodexAccountForThread("cleared-after", config, now + 61_000, "spark")).toBe("a"); + }); + test("a request the account serves releases the threads its quota refusal moved", () => { const config = makeConfig(); const now = 1_800_000_000_000;