From f946d93d4ec6f0296d109e495683981e2c3b4a0f Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 09:33:18 +0900 Subject: [PATCH 01/12] docs(triage): adjudicate the nine overnight PRs #967 found two real defects in my own #955 code and both verify at runtime: a Team account with a monthly window could never recover because the predicate picked its window by plan name while the parser picks by window duration, and the probe's own token refresh was mistaken for an external credential replacement. #963 and #965 both claim #962; #965 wins because it inherits from the row it actually replaces rather than recomputing config hints, and because #963 rewrites an existing regression contract to justify a broader change. #966 is a fifth design for #914 that survives two of the four prior falsifications but not all: mixed 5xx-then-rejection still loses the attributable failure, and five newly-classified sidecar paths keep default redirects, so a credential-visible 307 to a dead host still reads as neutral. --- .../000_dispositions.md | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 devlog/_plan/260804_overnight_triage/000_dispositions.md diff --git a/devlog/_plan/260804_overnight_triage/000_dispositions.md b/devlog/_plan/260804_overnight_triage/000_dispositions.md new file mode 100644 index 000000000..bcb81ea6d --- /dev/null +++ b/devlog/_plan/260804_overnight_triage/000_dispositions.md @@ -0,0 +1,160 @@ +# 000 — Overnight PR triage: nine PRs, four verdicts + +Nine pull requests arrived overnight while the #951–#955 stack was in review. +The question for each: does it duplicate work already on the stack, is it a real +independent defect, or does it belong somewhere else entirely. + +Measured 2026-08-04 against `origin/dev` and the stack head +`codex/915-cooldown-recovery-probe` at `493329df0`. + +## Verdicts + +| PR | Author | Verdict | Action | +|---|---|---|---| +| #967 | @Yuxin-Qiao | **two real defects in OUR #955** | carry onto layer 6 | +| #965 | @Yuxin-Qiao | correct fix for #962 | carry onto layer 6 | +| #963 | @MarcTCruz | duplicate of #965, broader and weaker | close, name #965 | +| #968 | @DevMello | real independent defect | carry onto layer 6 | +| #964 | @Yuxin-Qiao | real independent defect (#956) | leave open, own review | +| #966 | @Yuxin-Qiao | partial fix, fifth falsified design | leave open, request changes | +| #970 | @stephen-drew | real, out of stack scope | leave open | +| #961 | @Yuxin-Qiao | feature, not a bug | leave open | +| #969 | @Wibias | CI governance policy | out of scope | + +## #967 — the one that matters most + +@Yuxin-Qiao reviewed #955 and found two defects **in my own code**. Both verify. + +**P1: monthly-classified snapshots were rejected.** My +`isCompleteCodexQuotaRecoverySnapshot()` picked the required window from the +plan NAME. The parser picks it from the window DURATION +(`isExplicitMonthlyWindow()`, `src/codex/quota.ts:104-109`), and a Team response +whose primary window is explicitly monthly parses to `monthlyPercent` only. So +the probe rejected every successful fresh read for those accounts: + +```console +$ bun run .tmp/probe_967.ts # on the stack head, BEFORE the fix +parsed = {"monthlyPercent":12,"monthlyResetAt":1900000000} +recoverable? = false <-- Team monthly account can never recover +weekly parsed = {"weeklyPercent":12,"weeklyResetAt":1900000000} +recoverable? = true +``` + +That is the same "cooled forever" failure #915 exists to fix, reintroduced for +monthly-window plans. It is also the *third* time this predicate has been wrong +in the same direction — first the plan allowlist, then `prolite`, now the +window-classification mismatch. The lesson has been consistent and I kept +missing it: this predicate must read what the parser actually wrote, never +re-derive the classification itself. + +**P2: the probe's own token refresh looked like a replacement.** +`getValidCodexToken()` refreshes a near-expiry token inside the probe fetch and +advances the credential generation by exactly one +(`src/codex/account-store.ts:415-421`). My settle required the claim-time +generation to match exactly, so a successful fresh read under the refreshed +generation was thrown away and recovery waited another five minutes. + +The fix fences the +1 transition on `replacedAt`, which is the right +discriminator: +`saveCodexAccountCredentialIfGeneration()` **preserves** `replacedAt` +(`:195-217`) while `saveCodexAccountCredential()` **stamps a fresh one** +(`:131-146`). So a probe-owned refresh and an external replacement are +distinguishable even though both bump the generation. + +Verified after the fix — every fail-closed guard still holds: + +```console +credits-only -> false windowless {} -> false null -> false +exhausted 100 -> false go+weeklyOnly -> false +team monthly -> true team weekly -> true +``` + +Red-green: ablating P1 fails 2 tests, ablating P2 fails 1 different test. + +## #963 vs #965 — the duplicate pair + +Both claim "Fixes #962", both edit `src/codex/catalog/provider-fetch.ts`. +**#965 wins.** + +#962 is specifically about a custom row *replacing* a same-slug provider row. +#965 models exactly that: it indexes the rows deduplication will replace and +fills only undefined capability fields from the replaced row, so it also +inherits live `/models` metadata such as normalized `capabilities`. + +#963 instead recomputes `catalogHintsFromProviderConfig()` for **every** custom +row, including custom-only rows with no provider counterpart — broader than +#962 requires. It cannot retain discovered metadata, since it rebuilds from +config rather than inheriting. And it rewrites an existing regression contract +to fit: `tests/catalog-vision-sidecar-modalities.test.ts` changes from "no +registry reasoning metadata leaks onto an unmatched custom override" to +expecting that leak, and drops three `fetch should not be called` guards. + +Changing a test that encodes a deliberate prior decision, in order to make a +broader change pass, is the part that decides this. #965's ablation fails +exactly one test — the #962 regression — which is what a focused fix looks like. + +## #966 — a fifth design, still falsified + +#966 targets #914, which four prior designs already failed at an audit gate +(`devlog/_plan/260803_transport_attribution/000_plan.md`). It is a genuine +advance: it uses the real Bun 1.3.14 error labels including both alternating +ones, it does **not** repeat the hostname-resolution design, and it closes the +redirect counterexample on the pool Responses and Compact paths with manual +redirects. TLS/fake-IP codes correctly stay account-scoped. + +But two falsifications survive, both reproduced live: + +**Mixed 5xx → rejection still loses the attributable failure.** +`fetchWithTransientRetry()` discards prior transient responses when a later +attempt rejects (`src/lib/upstream-retry.ts:220-236`), so a genuine 503 followed +by a connection refusal is recorded as account-neutral. That is precisely the +hole the earlier audit documented. + +**Falsification 3 survives on five expanded surfaces.** Manual redirects were +added only to Responses and Compact; the five sidecar paths #966 newly +classifies still use default-follow fetch, so a credential-bearing sidecar that +receives a 307 to a dead host is misclassified as neutral — after the origin +already read the `Authorization` header: + +```text +redirect:"follow" serverSawAuthorization:"Bearer credential-follow" resolved:false +redirect:"manual" serverSawAuthorization:"Bearer credential-manual" resolved:true 307 +``` + +So #966 is not mergeable as-is, and its sidecar expansion carries the unresolved +hole *beyond* #914's original sites. It does supersede #922 (which misses one +Bun label entirely, keeps default redirects, and bundles unrelated probe-lease +work while sitting at `CHANGES_REQUESTED`). + +Neither lands on our stack. #914 remains open with a fifth design on record. + +## The rest + +**#968** (@DevMello) — the google adapter dropped `tool_choice` entirely: `none`, +`required`, and a forced tool all produced a wire body identical to `auto`, with +only a prose nudge in the system prompt. The wire compiler already validated +`toolConfig.functionCallingConfig`; the adapter simply never built it. Carries +cleanly onto the stack; same author as the already-carried #943. + +**#964** (@Yuxin-Qiao) — the `nvidia` registry entry lacks `noVisionModels`, so +the vision sidecar never activates for NIM text-only models and raw image parts +reach a text-only upstream. Real, but it is a registry/provider change with no +relationship to this stack's theme; it deserves its own review rather than a +ride on a bug stack. + +**#970** (@stephen-drew) — service re-registration during self-update. Real, but +30 files across CLI, GUI, and five docs locales, touching a +permission-sensitive install path. Out of scope here. + +**#961** — provider custom headers via PATCH. A feature, not a bug. + +**#969** (@Wibias, collaborator) — CI policy that auto-drafts contributor PRs +until a checklist is complete. Out of scope: it is a workflow change requiring +security review per `AGENTS.md`, and it encodes a contribution policy that is +the maintainer's call, not a bug fix. Worth noting the history — the #900–#905 +stack was closed on exactly this kind of policy question, not on its mechanics. + +## Layer 6 contents + +Carry, in order: #967 (fixes our own defects), #965 (catalog), #968 (google +tool_choice). Everything else stays where it is, with a reason on record. From 9c110d87a6753ffa29f7967acba40802ca423527 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com> Date: Tue, 4 Aug 2026 04:45:46 +0800 Subject: [PATCH 02/12] fix(codex): accept monthly-classified recovery snapshots and probe-owned refresh generations Addresses the two unresolved Codex review threads on #955: - isCompleteCodexQuotaRecoverySnapshot() required weeklyPercent for every non-Go/Free plan by plan name, but the parser classifies windows by duration: a Team response with an explicitly monthly primary window parses to monthlyPercent only, so those accounts could never recover early and stayed cooled until their predicted expiry. - settleCodexQuotaRecoveryProbe() required the claim-time credential generation to match exactly. A probe-owned token refresh inside getValidCodexToken() advances the generation by one before WHAM completes, so a successful fresh reading was rejected and the account waited another probe interval. replacedAt is preserved by refresh and stamped by external replacement, so it fences the +1 transition. (cherry picked from commit 79d2164e261a24c750df86b6a650cd8824dbd233) --- src/codex/quota.ts | 15 +++++-- src/codex/routing.ts | 22 ++++++++- tests/codex-cooldown-recovery.test.ts | 64 ++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src/codex/quota.ts b/src/codex/quota.ts index 739baf986..2d397a51e 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -103,10 +103,17 @@ export function isCompleteCodexQuotaRecoverySnapshot( // Recovery still fails closed on MISSING EVIDENCE — a credits-only or windowless payload // carries no usage reading at all and must never clear a cooldown. What it does not do is // fail closed on an unfamiliar plan NAME, which only ever meant "cooled forever". - const required = codexQuotaWindowForPlan(plan) === "monthly" - ? quota.monthlyPercent - : quota.weeklyPercent; - return typeof required === "number" && Number.isFinite(required); + // + // The parser classifies windows by DURATION, not by plan name: a Team response whose + // primary window is explicitly monthly parses to monthlyPercent only (no secondary + // window exists), so requiring weeklyPercent because the plan is not go/free would + // strand exactly those accounts until their predicted expiry. Accept whichever window(s) + // the parser actually wrote; Go/Free never carry a weekly value, so monthly-only is + // required there. + if (codexQuotaWindowForPlan(plan) === "monthly") { + return typeof quota.monthlyPercent === "number" && Number.isFinite(quota.monthlyPercent); + } + return hasKnownQuotaValue(quota); } export function normalizeUsagePercent(value: unknown): number | undefined { diff --git a/src/codex/routing.ts b/src/codex/routing.ts index 3331bc7d0..3d62c9048 100644 --- a/src/codex/routing.ts +++ b/src/codex/routing.ts @@ -139,6 +139,8 @@ export type CodexQuotaRecoveryProbeClaim = { leaseId: string; cooldownGeneration: number; credentialGeneration: number; + /** Claim-time `replacedAt`; unchanged after a probe-owned refresh, stamped on external replacement. */ + credentialReplacedAt?: number; }; export type CodexQuotaRecoveryProbeProof = { @@ -439,6 +441,7 @@ export function claimDueCodexQuotaRecoveryProbes( scope?: CodexQuotaScope; health: CodexUpstreamHealth; credentialGeneration: number; + credentialReplacedAt?: number; order: number; }> = []; for (const [order, account] of (config.codexAccounts ?? []).entries()) { @@ -467,6 +470,7 @@ export function claimDueCodexQuotaRecoveryProbes( ...(candidate.scope ? { scope: candidate.scope } : {}), health: candidate.health, credentialGeneration: record.generation, + ...(record.replacedAt !== undefined ? { credentialReplacedAt: record.replacedAt } : {}), order, }); } @@ -491,6 +495,9 @@ export function claimDueCodexQuotaRecoveryProbes( leaseId, cooldownGeneration: candidate.health.cooldownGeneration ?? 0, credentialGeneration: candidate.credentialGeneration, + ...(candidate.credentialReplacedAt !== undefined + ? { credentialReplacedAt: candidate.credentialReplacedAt } + : {}), }; }); } @@ -506,10 +513,21 @@ export function settleCodexQuotaRecoveryProbe( ? scopedHealthFor(claim.accountId, claim.scope) : upstreamHealth.get(claim.accountId); if (!health || health.probeLeaseId !== claim.leaseId) return false; + const currentRecord = readCodexAccountRecord(claim.accountId); + const proofGeneration = proof.credentialGeneration; + // A probe-owned token refresh (getValidCodexToken) advances the credential generation by + // exactly one while preserving `replacedAt`; an external credential replacement bumps the + // generation too but stamps a fresh `replacedAt`. Accept the +1 transition only when the + // claim-time lineage is intact AND the generation the fresh quota was proven under is live. + const generationFenced = proofGeneration !== undefined + && (proofGeneration === claim.credentialGeneration + ? isCodexAccountGenerationLive(claim.accountId, proofGeneration) + : proofGeneration === claim.credentialGeneration + 1 + && currentRecord?.replacedAt === claim.credentialReplacedAt + && isCodexAccountGenerationLive(claim.accountId, proofGeneration)); const fenced = (health.cooldownGeneration ?? 0) === claim.cooldownGeneration && (health.probeLeaseGeneration ?? 0) === claim.cooldownGeneration - && claim.credentialGeneration === proof.credentialGeneration - && isCodexAccountGenerationLive(claim.accountId, claim.credentialGeneration); + && generationFenced; if (!recovered || !fenced) { const released = withProbeLeaseReleased(health, now); if (claim.scope) setScopedHealth(claim.accountId, claim.scope, released); diff --git a/tests/codex-cooldown-recovery.test.ts b/tests/codex-cooldown-recovery.test.ts index 7ce26fd19..1c71975e2 100644 --- a/tests/codex-cooldown-recovery.test.ts +++ b/tests/codex-cooldown-recovery.test.ts @@ -117,6 +117,55 @@ describe("Codex cooldown recovery worker", () => { expect(routed).toEqual(["b", "b"]); }); + test("recovers a Team account from a duration-classified monthly snapshot", async () => { + // WHAM can legitimately return only an explicitly monthly primary window for a Team + // plan (30.4-day window, no secondary). parseUsageQuota then writes monthlyPercent only, + // so recovery must accept the window the parser actually classified instead of demanding + // a weekly reading because the plan name is not "go"/"free". + const config = makeConfig(["a"]); + saveCredential("a"); + cool(config, "a"); + globalThis.fetch = async () => usageResponse(0, { + plan_type: "team", + rate_limit: { + primary_window: { used_percent: 6, reset_at: 1_900_000_000, limit_window_seconds: 2_628_000 }, + secondary_window: null, + tertiary_window: null, + }, + rate_limit_reset_credits: { available_count: 0 }, + }); + await runCodexCooldownRecoveryProbes(config, due()); + expect(getCodexQuotaHealthSnapshot("a", "shared", due() + 1)).toBeNull(); + }); + + test("recovers when the probe's own token refresh advances the credential generation", async () => { + // A near-expiry access token makes getValidCodexToken() refresh it inside the probe + // fetch, bumping the credential generation from 1 to 2 before WHAM completes. The fresh + // quota is proven under the new live generation, so settling against the claim-time + // generation must accept this probe's own refresh, not treat it as a replacement. + const config = makeConfig(["a"]); + saveCodexAccountCredential("a", { + accessToken: "access-a", + refreshToken: "refresh-a", + expiresAt: Date.now() + 30_000, + chatgptAccountId: "acct-a", + }); + cool(config, "a"); + globalThis.fetch = async input => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + if (url.includes("/oauth/token")) { + return new Response(JSON.stringify({ + access_token: "access-a-2", + refresh_token: "refresh-a-2", + expires_in: 3600, + }), { status: 200, headers: { "Content-Type": "application/json" } }); + } + return usageResponse(12); + }; + await runCodexCooldownRecoveryProbes(config, due()); + expect(getCodexQuotaHealthSnapshot("a", "shared", due() + 1)).toBeNull(); + }); + test.each([ ["still exhausted", () => usageResponse(100)], ["credits only", () => usageResponse(0, { plan_type: "team", rate_limit_reset_credits: { available_count: 1 } })], @@ -290,13 +339,18 @@ describe("Codex cooldown recovery worker", () => { for (const plan of snapshotPlans) { const monthly = codexQuotaWindowForPlan(plan) === "monthly"; - const filled = monthly ? { monthlyPercent: 12 } : { weeklyPercent: 12 }; - const empty = monthly ? { weeklyPercent: 12 } : { monthlyPercent: 12 }; - expect(isCompleteCodexQuotaRecoverySnapshot(filled, plan)).toBe(true); - // The other window is not evidence for this plan, in either direction. - expect(isCompleteCodexQuotaRecoverySnapshot(empty, plan)).toBe(false); + // The parser classifies windows by duration, so a weekly-billed plan can carry a + // monthly-only reading (30-day primary, no secondary). Any window the parser actually + // wrote is evidence; only Go/Free never carry a weekly value. + expect(isCompleteCodexQuotaRecoverySnapshot({ weeklyPercent: 12 }, plan)).toBe(!monthly); + expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12 }, plan)).toBe(true); } + // Monthly-billed Go/Free parse to monthlyPercent only; a weekly-only reading is not + // evidence for them. + expect(isCompleteCodexQuotaRecoverySnapshot({ weeklyPercent: 12 }, "go")).toBe(false); + expect(isCompleteCodexQuotaRecoverySnapshot({ weeklyPercent: 12 }, "free")).toBe(false); + // Absent plan follows the parser's weekly default. expect(isCompleteCodexQuotaRecoverySnapshot({ weeklyPercent: 12 }, undefined)).toBe(true); // Missing EVIDENCE still fails closed — that is the guard that matters. From 833406628cfc1ec662ea35155b873e09a5a4b154 Mon Sep 17 00:00:00 2001 From: Yuxin Qiao Date: Tue, 4 Aug 2026 04:13:58 +0800 Subject: [PATCH 03/12] fix(catalog): custom model rows inherit provider reasoning metadata (cherry picked from commit c5565d055cff7342f92c21e565addb513b18bc25) --- src/codex/catalog/provider-fetch.ts | 30 +++++++++++++--- tests/codex-catalog.test.ts | 56 +++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 5422e1db5..71bc27227 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -782,6 +782,9 @@ async function gatherRoutedModelsUncached( // Enriched (registry-hydrated) provider clones, keyed by name — the same view used above so // custom rows get the same noVisionModels / inputModalities treatment as discovered rows. const enrichedByName = new Map(activeProviders); + // Provider-derived rows keyed by their Codex-facing slug: a custom override replaces the row + // with the same slug below, so that row's provider capability metadata is the inheritance source. + const replacedByRoutedSlug = new Map(all.map(model => [routedSlug(model.provider, model.id), model])); const customModels = (config.customModels ?? []).map(cm => { const rawProvider = config.providers[cm.provider]; const supportsReasoningSummaries = configuredReasoningSummarySupport(rawProvider, cm.modelId); @@ -794,19 +797,38 @@ async function gatherRoutedModelsUncached( ...(cm.inputModalities ? { inputModalities: cm.inputModalities } : {}), ...(typeof supportsReasoningSummaries === "boolean" ? { supportsReasoningSummaries } : {}), }; + // #962: the dedupe below drops the provider-derived row this custom row replaces. Inherit that + // row's provider capability metadata (reasoning ladder, default effort, parallel tool calls, + // context, ...) so the generated catalog keeps advertising what the router actually provides. + // Explicit custom fields win by construction; this only fills gaps. Without it a + // noReasoningModels model loses its empty ladder and the catalog synthesizes the generic one, + // which Codex then rejects for spawn_agent with effort "none". + const replaced = replacedByRoutedSlug.get(routedSlug(cm.provider, cm.modelId)); + const merged: CatalogModel = replaced ? { + ...base, + ...(base.contextWindow === undefined && replaced.contextWindow !== undefined ? { contextWindow: replaced.contextWindow } : {}), + ...(base.maxInputTokens === undefined && replaced.maxInputTokens !== undefined ? { maxInputTokens: replaced.maxInputTokens } : {}), + ...(base.inputModalities === undefined && replaced.inputModalities !== undefined ? { inputModalities: replaced.inputModalities } : {}), + ...(base.reasoningEfforts === undefined && replaced.reasoningEfforts !== undefined ? { reasoningEfforts: replaced.reasoningEfforts } : {}), + ...(base.defaultReasoningEffort === undefined && replaced.defaultReasoningEffort !== undefined ? { defaultReasoningEffort: replaced.defaultReasoningEffort } : {}), + ...(base.parallelToolCalls === undefined && replaced.parallelToolCalls !== undefined ? { parallelToolCalls: replaced.parallelToolCalls } : {}), + ...(base.supportsVerbosity === undefined && replaced.supportsVerbosity !== undefined ? { supportsVerbosity: replaced.supportsVerbosity } : {}), + ...(base.supportsReasoningSummaries === undefined && replaced.supportsReasoningSummaries !== undefined ? { supportsReasoningSummaries: replaced.supportsReasoningSummaries } : {}), + ...(base.capabilities === undefined && replaced.capabilities !== undefined ? { capabilities: replaced.capabilities } : {}), + } : base; // Vision-sidecar coverage ONLY: if the custom model is in the enriched provider's // noVisionModels, advertise image input so the Codex app lets images reach the sidecar // (#349/#344). Deliberately NOT the full applyProviderConfigHints pass — custom rows are a // user override, so their explicit contextWindow / inputModalities / reasoning fields must be // preserved verbatim (the hint pass would cap context and overwrite modalities from registry). const enrichedProvider = enrichedByName.get(cm.provider) ?? rawProvider; - if (enrichedProvider && modelInList(enrichedProvider.noVisionModels, base.id)) { - const current = base.inputModalities ?? ["text"]; + if (enrichedProvider && modelInList(enrichedProvider.noVisionModels, merged.id)) { + const current = merged.inputModalities ?? ["text"]; if (!current.includes("image")) { - return { ...base, inputModalities: [...current, "image"] }; + return { ...merged, inputModalities: [...current, "image"] }; } } - return base; + return merged; }); // Custom rows override discovered rows that encode to the same Codex-facing slug. const customKeys = new Set(customModels.map(c => routedSlug(c.provider, c.id))); diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 3a00105a6..2debabd4d 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -848,6 +848,62 @@ describe("configured CatalogModel displayName -> catalog display_name", () => { }); }); +test("a custom row inherits provider reasoning metadata from the provider-derived row it replaces (#962)", async () => { + clearModelCache("ollama"); + const originalFetch = globalThis.fetch; + globalThis.fetch = (() => { throw new Error("fetch should not be called"); }) as typeof fetch; + try { + const models = await gatherRoutedModels({ + port: 10100, + defaultProvider: "ollama", + providers: { + ollama: { + baseUrl: "http://localhost:11434/v1", + adapter: "openai-chat", + authMode: "key", + liveModels: false, + models: ["qwen-coder-3b"], + selectedModels: ["qwen-coder-3b"], + noReasoningModels: ["qwen-coder-3b"], + modelReasoningEfforts: { "qwen-coder-3b": [] }, + }, + }, + customModels: [ + { + id: "cm-962", + provider: "ollama", + modelId: "qwen-coder-3b", + displayName: "Qwen Coder 3B (local)", + contextWindow: 32768, + inputModalities: ["text"], + addedAt: "2026-01-01T00:00:00.000Z", + }, + ], + }); + + // Explicit custom fields stay verbatim; provider capability metadata is inherited from the + // replaced provider-derived row (noReasoningModels -> empty reasoning ladder, openai-chat + // adapter -> parallel tool calls). + const custom = models.find(m => m.provider === "ollama" && m.id === "qwen-coder-3b"); + expect(custom?.displayName).toBe("Qwen Coder 3B (local)"); + expect(custom?.contextWindow).toBe(32768); + expect(custom?.inputModalities).toEqual(["text"]); + expect(custom?.reasoningEfforts).toEqual([]); + expect(custom?.parallelToolCalls).toBe(true); + + const entries = buildCatalogEntries(nativeTemplate(), [], models); + const row = entries.find(e => e.slug === "ollama/qwen-coder-3b"); + expect(row?.display_name).toBe("Qwen Coder 3B (local)"); + // The catalog must expose no reasoning levels and no default reasoning level for this model; + // the generic low..ultra ladder and the medium default must not be synthesized. + expect(row?.supported_reasoning_levels).toEqual([]); + expect(row?.default_reasoning_level).toBeUndefined(); + } finally { + globalThis.fetch = originalFetch; + clearModelCache("ollama"); + } +}); + function openAiApiCatalogConfig(overrides: Record = {}): OcxConfig { return { port: 10100, From 76423caa73aefd8d6405703080330a7eb5b254a8 Mon Sep 17 00:00:00 2001 From: devmello Date: Mon, 3 Aug 2026 13:11:02 -0700 Subject: [PATCH 04/12] fix(google): map tool_choice onto functionCallingConfig (cherry picked from commit 3f7e4cf3ffbc7da661bd8f9aa497eb7fcca9eaa1) --- src/adapters/google.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/adapters/google.ts b/src/adapters/google.ts index 58374ddcd..6c2f7251d 100644 --- a/src/adapters/google.ts +++ b/src/adapters/google.ts @@ -12,7 +12,7 @@ import type { OcxToolCall, OcxUsage, } from "../types"; -import { isAllowedToolChoice, namespacedToolName, toolAllowedByChoice } from "../types"; +import { isAllowedToolChoice, namespacedToolName, resolveToolChoiceWireName, toolAllowedByChoice } from "../types"; import { contentPartsToText, parseDataUrl } from "./image"; import { getVertexAccessToken } from "../lib/gcp-adc"; import { fetchAntigravityWithRetry, fetchVertexWithRetry } from "./google-http"; @@ -232,6 +232,28 @@ function toolsToGeminiFormat(parsed: OcxParsedRequest): unknown[] | undefined { }]; } +/** + * Client tool_choice enforcement on the wire. The catalog nudge states the same contract in + * prose, but without functionCallingConfig the model is free to ignore it. "auto" stays absent + * so the common case is byte-identical. The allowedTools variant already filters the + * declarations in toolsToGeminiFormat; only its "required" half needs a wire mode. + */ +function toolChoiceToGeminiToolConfig(parsed: OcxParsedRequest): Record | undefined { + const choice = parsed.options.toolChoice; + if (!choice || choice === "auto") return undefined; + if (choice === "none") return { functionCallingConfig: { mode: "NONE" } }; + if (choice === "required") return { functionCallingConfig: { mode: "ANY" } }; + if (isAllowedToolChoice(choice)) { + return choice.mode === "required" ? { functionCallingConfig: { mode: "ANY" } } : undefined; + } + return { + functionCallingConfig: { + mode: "ANY", + allowedFunctionNames: [resolveToolChoiceWireName(parsed.context.tools, choice.name)], + }, + }; +} + function usageFromGemini(usage: Record | undefined): OcxUsage | undefined { if (!usage) return undefined; return { @@ -307,6 +329,10 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte const body: Record = { contents }; if (systemInstruction) body.systemInstruction = systemInstruction; if (tools) body.tools = tools; + // Only meaningful with declarations on the wire: mode ANY with an empty + // catalog is a guaranteed upstream 400. + const toolConfig = tools ? toolChoiceToGeminiToolConfig(parsed) : undefined; + if (toolConfig) body.toolConfig = toolConfig; const generationConfig: Record = {}; if (parsed.options.maxOutputTokens) generationConfig.maxOutputTokens = parsed.options.maxOutputTokens; From f754a63d12afd132fd1fabe1cceab8dc152dcc22 Mon Sep 17 00:00:00 2001 From: devmello Date: Mon, 3 Aug 2026 13:11:02 -0700 Subject: [PATCH 05/12] test(google): cover tool_choice wire enforcement (cherry picked from commit 46756f5e5004da56cde8dd8b08f1a58e7365b30b) --- tests/google-adapter.test.ts | 66 ++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/google-adapter.test.ts b/tests/google-adapter.test.ts index 4eecd1d48..d9dbe428e 100644 --- a/tests/google-adapter.test.ts +++ b/tests/google-adapter.test.ts @@ -189,3 +189,69 @@ describe("google adapter — tool-call ids on the wire", () => { expect(fc).toBe("call_xyz"); }); }); + +describe("google adapter — tool_choice on the wire", () => { + const TOOLS = [ + { name: "get_weather", parameters: { type: "object", properties: {} } }, + { name: "shot", namespace: "mcp__chrome", parameters: { type: "object", properties: {} } }, + ]; + + function parsedWithChoice(toolChoice: unknown, tools: unknown[] | null = TOOLS): OcxParsedRequest { + return { + modelId: "gemini-3-pro", + stream: false, + options: toolChoice === undefined ? {} : { toolChoice }, + context: { messages: [{ role: "user", content: "hi" }], tools: tools ?? undefined }, + } as unknown as OcxParsedRequest; + } + + test('"none" and "required" map to NONE and ANY', async () => { + expect((await geminiBody(parsedWithChoice("none"))).toolConfig) + .toEqual({ functionCallingConfig: { mode: "NONE" } }); + expect((await geminiBody(parsedWithChoice("required"))).toolConfig) + .toEqual({ functionCallingConfig: { mode: "ANY" } }); + }); + + test("a forced tool maps to ANY with its wire name allowed", async () => { + expect((await geminiBody(parsedWithChoice({ name: "get_weather" }))).toolConfig) + .toEqual({ functionCallingConfig: { mode: "ANY", allowedFunctionNames: ["get_weather"] } }); + // Dotted alias resolves to the namespaced declaration name. + expect((await geminiBody(parsedWithChoice({ name: "mcp__chrome.shot" }))).toolConfig) + .toEqual({ functionCallingConfig: { mode: "ANY", allowedFunctionNames: ["mcp__chrome__shot"] } }); + }); + + test('"auto", absent, and allowedTools+auto stay byte-identical (no toolConfig)', async () => { + expect((await geminiBody(parsedWithChoice("auto"))).toolConfig).toBeUndefined(); + expect((await geminiBody(parsedWithChoice(undefined))).toolConfig).toBeUndefined(); + expect((await geminiBody(parsedWithChoice({ allowedTools: ["get_weather"], mode: "auto" }))).toolConfig).toBeUndefined(); + }); + + test("allowedTools with mode required keeps the filtered catalog and adds ANY", async () => { + const body = await geminiBody(parsedWithChoice({ allowedTools: ["get_weather"], mode: "required" })); + const declared = (body.tools as { functionDeclarations: { name: string }[] }[])[0].functionDeclarations.map(d => d.name); + expect(declared).toEqual(["get_weather"]); + expect(body.toolConfig).toEqual({ functionCallingConfig: { mode: "ANY" } }); + }); + + test("no declared tools means no toolConfig even with a choice", async () => { + expect((await geminiBody(parsedWithChoice("none", null))).toolConfig).toBeUndefined(); + expect((await geminiBody(parsedWithChoice({ name: "get_weather" }, []))).toolConfig).toBeUndefined(); + }); + + test("claude-on-antigravity keeps VALIDATED mode over a client choice, allowed names survive", async () => { + const ccaProvider = { + adapter: "google", + googleMode: "cloud-code-assist", + baseUrl: "https://daily-cloudcode-pa.googleapis.com", + apiKey: "key", + project: "proj-123", + }; + const parsed = parsedWithChoice({ name: "get_weather" }); + (parsed as unknown as { modelId: string }).modelId = "claude-opus-4.8"; + const { body } = await createGoogleAdapter(ccaProvider).buildRequest(parsed); + const request = JSON.parse(body).request as Record; + expect(request.toolConfig).toEqual({ + functionCallingConfig: { mode: "VALIDATED", allowedFunctionNames: ["get_weather"] }, + }); + }); +}); From 9f6042919c1158e2331c0c94be71e9503741510a Mon Sep 17 00:00:00 2001 From: devmello Date: Mon, 3 Aug 2026 14:20:28 -0700 Subject: [PATCH 06/12] fix(google): honor tool_choice none for claude on antigravity (cherry picked from commit a366934d8ec1ce94918e5eea05327866021faf25) --- src/adapters/google.ts | 6 ++++++ tests/google-adapter.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/adapters/google.ts b/src/adapters/google.ts index 6c2f7251d..70ee585ac 100644 --- a/src/adapters/google.ts +++ b/src/adapters/google.ts @@ -384,6 +384,12 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte const draftRequest: Record = { ...body, sessionId }; // Claude-on-Antigravity forces VALIDATED function calling (the real client always sets it). if (/claude/i.test(wireModelId)) { + // VALIDATED would defeat a client's tool_choice "none": honor it by dropping the + // declarations instead, the wire shape of a tool-less Claude turn. + if (parsed.options.toolChoice === "none") { + delete draftRequest.tools; + delete draftRequest.toolConfig; + } const existing = (draftRequest.toolConfig ?? {}) as Record; const fcc = (existing.functionCallingConfig ?? {}) as Record; draftRequest.toolConfig = { ...existing, functionCallingConfig: { ...fcc, mode: "VALIDATED" } }; diff --git a/tests/google-adapter.test.ts b/tests/google-adapter.test.ts index d9dbe428e..4fe84806c 100644 --- a/tests/google-adapter.test.ts +++ b/tests/google-adapter.test.ts @@ -238,6 +238,28 @@ describe("google adapter — tool_choice on the wire", () => { expect((await geminiBody(parsedWithChoice({ name: "get_weather" }, []))).toolConfig).toBeUndefined(); }); + test('claude-on-antigravity honors "none" by dropping the declarations', async () => { + const ccaProvider = { + adapter: "google", + googleMode: "cloud-code-assist", + baseUrl: "https://daily-cloudcode-pa.googleapis.com", + apiKey: "key", + project: "proj-123", + }; + const claudeParsed = parsedWithChoice("none"); + (claudeParsed as unknown as { modelId: string }).modelId = "claude-opus-4.8"; + const claudeRequest = JSON.parse((await createGoogleAdapter(ccaProvider).buildRequest(claudeParsed)).body).request as Record; + // VALIDATED would defeat NONE, so the declarations go instead; the config matches a tool-less turn. + expect(claudeRequest.tools).toBeUndefined(); + expect(claudeRequest.toolConfig).toEqual({ functionCallingConfig: { mode: "VALIDATED" } }); + + // Gemini on the same route has no VALIDATED override, so NONE rides with the catalog intact. + const geminiParsed = parsedWithChoice("none"); + const geminiRequest = JSON.parse((await createGoogleAdapter(ccaProvider).buildRequest(geminiParsed)).body).request as Record; + expect(geminiRequest.tools).toBeDefined(); + expect(geminiRequest.toolConfig).toEqual({ functionCallingConfig: { mode: "NONE" } }); + }); + test("claude-on-antigravity keeps VALIDATED mode over a client choice, allowed names survive", async () => { const ccaProvider = { adapter: "google", From 7fb05002a65e23b704a93d11029ffdc494fdc81f Mon Sep 17 00:00:00 2001 From: devmello Date: Mon, 3 Aug 2026 14:41:35 -0700 Subject: [PATCH 07/12] test(google): assert the exact catalog in the none case (cherry picked from commit 9ba66a18164baf4e79b2e89fc13f42217fc7a36e) --- tests/google-adapter.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/google-adapter.test.ts b/tests/google-adapter.test.ts index 4fe84806c..467cf19c3 100644 --- a/tests/google-adapter.test.ts +++ b/tests/google-adapter.test.ts @@ -256,7 +256,8 @@ describe("google adapter — tool_choice on the wire", () => { // Gemini on the same route has no VALIDATED override, so NONE rides with the catalog intact. const geminiParsed = parsedWithChoice("none"); const geminiRequest = JSON.parse((await createGoogleAdapter(ccaProvider).buildRequest(geminiParsed)).body).request as Record; - expect(geminiRequest.tools).toBeDefined(); + const declared = (geminiRequest.tools as { functionDeclarations: { name: string }[] }[])[0].functionDeclarations.map(d => d.name); + expect(declared).toEqual(["get_weather", "mcp__chrome__shot"]); expect(geminiRequest.toolConfig).toEqual({ functionCallingConfig: { mode: "NONE" } }); }); From bb02e97ddea50b8035a407e6d53a644a0f7ccdeb Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 09:42:41 +0900 Subject: [PATCH 08/12] fix(codex): require primary-window provenance before monthly evidence recovers a weekly plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #967 correctly found that requiring weeklyPercent by plan name stranded Team accounts whose primary window is explicitly monthly. Its remedy — accept any window the parser wrote — was too permissive in the other direction: a tertiary-only response also writes monthlyPercent, describes a different period, and says nothing about the weekly quota that actually gates the account, so it could clear a cooldown on a reading of the wrong window. parseUsageQuota() now records monthlyIsPrimaryWindow when the monthly value came from an explicitly-monthly PRIMARY window, and recovery requires that provenance before accepting monthly-only evidence for a weekly-quota plan. Go/Free are unaffected: the monthly window governs them either way. The two shapes were previously indistinguishable — both parsed to {monthlyPercent} with no way to tell which window produced it. --- src/codex/quota.ts | 29 +++++++++++++++++--- tests/codex-cooldown-recovery.test.ts | 39 ++++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/codex/quota.ts b/src/codex/quota.ts index 2d397a51e..598f8b509 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -9,6 +9,14 @@ export type StoredAccountQuota = { weeklyResetAt?: number; monthlyResetAt?: number; resetCredits?: number; + /** + * True when `monthlyPercent` came from an explicitly-monthly PRIMARY window — + * i.e. it is the account's governing quota reading, not a supplementary + * tertiary window. Tertiary-only monthly data lands in the same field but says + * nothing about the weekly quota that actually gates a non-Go/Free account, + * so recovery must be able to tell the two apart (#967 audit). + */ + monthlyIsPrimaryWindow?: boolean; updatedAt: number; }; @@ -96,7 +104,7 @@ export function codexQuotaWindowForPlan(plan?: string | null): "monthly" | "week } export function isCompleteCodexQuotaRecoverySnapshot( - quota: Pick | null, + quota: Pick | null, plan?: string | null, ): boolean { if (!quota || isCodexQuotaExhausted(quota, plan)) return false; @@ -110,10 +118,21 @@ export function isCompleteCodexQuotaRecoverySnapshot( // strand exactly those accounts until their predicted expiry. Accept whichever window(s) // the parser actually wrote; Go/Free never carry a weekly value, so monthly-only is // required there. + // + // Audit correction: "the parser wrote monthlyPercent" is NOT by itself evidence for a + // weekly-quota plan. A tertiary-only response also writes monthlyPercent, and it says + // nothing about the weekly quota that actually gates a Team/Plus account — accepting it + // would clear the cooldown on a reading of a different window. Only an explicitly-monthly + // PRIMARY window is the governing reading, which is what `monthlyIsPrimaryWindow` records. if (codexQuotaWindowForPlan(plan) === "monthly") { - return typeof quota.monthlyPercent === "number" && Number.isFinite(quota.monthlyPercent); + return finitePercent(quota.monthlyPercent); } - return hasKnownQuotaValue(quota); + if (finitePercent(quota.weeklyPercent)) return true; + return quota.monthlyIsPrimaryWindow === true && finitePercent(quota.monthlyPercent); +} + +function finitePercent(value: number | undefined): boolean { + return typeof value === "number" && Number.isFinite(value); } export function normalizeUsagePercent(value: unknown): number | undefined { @@ -444,6 +463,10 @@ export function parseUsageQuota(data: WhamUsageResponse): Omit { expect(getCodexQuotaHealthSnapshot("a", "shared", due() + 1)).toBeNull(); }); + test("does NOT recover a Team account from a tertiary-only monthly snapshot", async () => { + // The mirror image of the case above, and the reason accepting "whatever the parser + // wrote" is too permissive. A tertiary window also lands in monthlyPercent, but it + // describes a different period and says nothing about the WEEKLY quota that actually + // gates a Team account — clearing the cooldown on it would restore traffic to an + // account whose governing window was never read. Only an explicitly-monthly PRIMARY + // window is that reading, which is what monthlyIsPrimaryWindow records. + const config = makeConfig(["a"]); + saveCredential("a"); + cool(config, "a"); + globalThis.fetch = async () => usageResponse(0, { + plan_type: "team", + rate_limit: { + primary_window: null, + secondary_window: null, + tertiary_window: { used_percent: 7, reset_at: 1_900_000_000 }, + }, + rate_limit_reset_credits: { available_count: 0 }, + }); + await runCodexCooldownRecoveryProbes(config, due()); + expect(getCodexQuotaHealthSnapshot("a", "shared", due() + 1)).not.toBeNull(); + }); + test("recovers when the probe's own token refresh advances the credential generation", async () => { // A near-expiry access token makes getValidCodexToken() refresh it inside the probe // fetch, bumping the credential generation from 1 to 2 before WHAM completes. The fresh @@ -339,11 +362,13 @@ describe("Codex cooldown recovery worker", () => { for (const plan of snapshotPlans) { const monthly = codexQuotaWindowForPlan(plan) === "monthly"; - // The parser classifies windows by duration, so a weekly-billed plan can carry a - // monthly-only reading (30-day primary, no secondary). Any window the parser actually - // wrote is evidence; only Go/Free never carry a weekly value. + // The parser classifies windows by duration, so a weekly-billed plan CAN carry a + // monthly-only reading (30-day primary, no secondary) — but only when that reading is + // the primary window. A bare monthlyPercent could equally be a tertiary window, which + // is a different period and no evidence for the weekly quota that gates the account. expect(isCompleteCodexQuotaRecoverySnapshot({ weeklyPercent: 12 }, plan)).toBe(!monthly); - expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12 }, plan)).toBe(true); + expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12 }, plan)).toBe(monthly); + expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12, monthlyIsPrimaryWindow: true }, plan)).toBe(true); } // Monthly-billed Go/Free parse to monthlyPercent only; a weekly-only reading is not @@ -353,6 +378,12 @@ describe("Codex cooldown recovery worker", () => { // Absent plan follows the parser's weekly default. expect(isCompleteCodexQuotaRecoverySnapshot({ weeklyPercent: 12 }, undefined)).toBe(true); + // Provenance, not just presence: monthlyPercent alone is evidence for a weekly-quota plan + // ONLY when it came from an explicitly-monthly primary window. + expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12 }, "team")).toBe(false); + expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12, monthlyIsPrimaryWindow: true }, "team")).toBe(true); + // Go/Free are governed by the monthly window either way, so the flag is not required. + expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12 }, "go")).toBe(true); // Missing EVIDENCE still fails closed — that is the guard that matters. expect(isCompleteCodexQuotaRecoverySnapshot({}, "plus")).toBe(false); expect(isCompleteCodexQuotaRecoverySnapshot(null, "plus")).toBe(false); From fede67f20aec8ba1e2632d16df44252022515fe3 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 09:45:58 +0900 Subject: [PATCH 09/12] fix(codex): carry monthly-window provenance through the quota cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recovery reads freshQuota directly, so this is not on its path today — but setAccountQuotaFromParsed() copies fields one by one, and a cached snapshot that kept monthlyPercent while dropping monthlyIsPrimaryWindow would look exactly like tertiary-only data to any future reader. A flag that silently fails to persist makes the guard decorative, and that failure would be invisible rather than loud. --- src/codex/quota.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/codex/quota.ts b/src/codex/quota.ts index 598f8b509..eae6a31f9 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -206,6 +206,7 @@ export function setAccountQuotaFromParsed( if (existing?.weeklyResetAt !== undefined) next.weeklyResetAt = existing.weeklyResetAt; if (existing?.monthlyPercent !== undefined) next.monthlyPercent = existing.monthlyPercent; if (existing?.monthlyResetAt !== undefined) next.monthlyResetAt = existing.monthlyResetAt; + if (existing?.monthlyIsPrimaryWindow === true) next.monthlyIsPrimaryWindow = true; next.resetCredits = quota.resetCredits; accountQuota.set(accountId, next); schedulePersistAccountQuotas(); @@ -225,9 +226,15 @@ export function setAccountQuotaFromParsed( if (snapshotHasMonthly(quota)) { if (quota.monthlyPercent !== undefined) next.monthlyPercent = quota.monthlyPercent; if (quota.monthlyResetAt !== undefined) next.monthlyResetAt = quota.monthlyResetAt; + // Carry the provenance with the value it describes. Recovery reads `freshQuota` directly, + // so this is not on its path today — but a cached snapshot that kept `monthlyPercent` + // while silently dropping `monthlyIsPrimaryWindow` would look like tertiary-only data to + // any future reader, and that failure would be invisible. + if (quota.monthlyIsPrimaryWindow === true) next.monthlyIsPrimaryWindow = true; } else if (snapshotHasWeekly(quota) && existing?.monthlyPercent !== undefined) { next.monthlyPercent = existing.monthlyPercent; if (existing.monthlyResetAt !== undefined) next.monthlyResetAt = existing.monthlyResetAt; + if (existing.monthlyIsPrimaryWindow === true) next.monthlyIsPrimaryWindow = true; } if (quota.resetCredits !== undefined) next.resetCredits = quota.resetCredits; From 01fbf34c99d563a338f4958cd2b2687b5b60155a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 09:50:11 +0900 Subject: [PATCH 10/12] test(codex): pin provenance through the cache, and set it in the header parser too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit noted the cache round trip was untested — so the guard could have been silently reduced to decoration by a later refactor. The test now asserts the flag survives setAccountQuotaFromParsed(), and ablating that copy fails it. parseUpstreamQuotaHeaders() recognizes the same explicitly-monthly primary window and now records the same provenance. It is not on the recovery path today, but two parsers disagreeing about what a bare monthlyPercent means is the kind of divergence that surfaces later as an unexplainable bug. --- src/codex/quota.ts | 4 ++++ tests/codex-cooldown-recovery.test.ts | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/codex/quota.ts b/src/codex/quota.ts index eae6a31f9..c8ae4543a 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -267,6 +267,10 @@ export function parseUpstreamQuotaHeaders(headers: Headers): Omit { expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12, monthlyIsPrimaryWindow: true }, "team")).toBe(true); // Go/Free are governed by the monthly window either way, so the flag is not required. expect(isCompleteCodexQuotaRecoverySnapshot({ monthlyPercent: 12 }, "go")).toBe(true); + + // The flag has to survive the cache, or the guard is decorative: a snapshot that keeps + // monthlyPercent while dropping its provenance looks exactly like tertiary-only data. + // setAccountQuotaFromParsed() rebuilds the record field by field, so this is a real + // drop risk rather than a theoretical one. + const parsedMonthly = parseUsageQuota({ + plan_type: "team", + rate_limit: { + primary_window: { used_percent: 12, limit_window_seconds: 2_628_000, reset_at: 1_900_000_000 }, + }, + } as never); + expect(parsedMonthly?.monthlyIsPrimaryWindow).toBe(true); + setAccountQuotaFromParsed("provenance-probe", parsedMonthly); + expect(getAccountQuota("provenance-probe")?.monthlyIsPrimaryWindow).toBe(true); // Missing EVIDENCE still fails closed — that is the guard that matters. expect(isCompleteCodexQuotaRecoverySnapshot({}, "plus")).toBe(false); expect(isCompleteCodexQuotaRecoverySnapshot(null, "plus")).toBe(false); From eb5064271dad8f8a1d65769312ce3ec5b22396ea Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 10:13:27 +0900 Subject: [PATCH 11/12] test(codex): teach the exact-shape quota assertions about window provenance Nine tests asserted parseUsageQuota()/getAccountQuota() output with toEqual, so the new monthlyIsPrimaryWindow field failed them on shape while every value was unchanged. Each expectation now states which side of the distinction it is on, which is the thing those tests were already about: - explicit-monthly PRIMARY windows carry the flag - the Go/Free thirtyDayOnly branch does not (recovery never consults it there) - a tertiary-sourced monthly value does not, which is the case that made the guard necessary - a credits-only refresh preserving prior usage does not The cached monthly-A snapshot now carries it too, proving propagation through setAccountQuotaFromParsed() rather than only asserting the parse. --- tests/codex-routing.test.ts | 5 ++++- tests/rate-limit-reset-credits.test.ts | 13 ++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/codex-routing.test.ts b/tests/codex-routing.test.ts index be0e8d23f..8b889cb81 100644 --- a/tests/codex-routing.test.ts +++ b/tests/codex-routing.test.ts @@ -1160,7 +1160,9 @@ describe("codex routing", () => { rate_limit: { primary_window: { used_percent: 39, reset_at: 3, limit_window_seconds: 2_628_000 }, }, - })).toEqual({ monthlyPercent: 39, monthlyResetAt: 3 }); + // The provenance flag rides with the value: this monthly reading IS the primary window, + // which is what lets recovery tell it apart from a tertiary-only monthly figure (#967). + })).toEqual({ monthlyPercent: 39, monthlyResetAt: 3, monthlyIsPrimaryWindow: true }); }); test("WHAM monthly primary preserves a legacy secondary weekly window", () => { @@ -1175,6 +1177,7 @@ describe("codex routing", () => { weeklyResetAt: 7, monthlyPercent: 39, monthlyResetAt: 30, + monthlyIsPrimaryWindow: true, }); }); diff --git a/tests/rate-limit-reset-credits.test.ts b/tests/rate-limit-reset-credits.test.ts index 9d66dc1f6..122d78c01 100644 --- a/tests/rate-limit-reset-credits.test.ts +++ b/tests/rate-limit-reset-credits.test.ts @@ -128,7 +128,7 @@ describe("rate-limit reset credits", () => { }, rate_limit_reset_credits: { available_count: 0 }, }); - expect(quota).toEqual({ monthlyPercent: 6, monthlyResetAt: 1787336442, resetCredits: 0 }); + expect(quota).toEqual({ monthlyPercent: 6, monthlyResetAt: 1787336442, resetCredits: 0, monthlyIsPrimaryWindow: true }); expect(quota!.weeklyPercent).toBeUndefined(); }); @@ -143,6 +143,7 @@ describe("rate-limit reset credits", () => { expect(quota).toEqual({ monthlyPercent: 39, monthlyResetAt: 1787401330, + monthlyIsPrimaryWindow: true, weeklyPercent: 20, weeklyResetAt: 1787000000, }); @@ -156,7 +157,7 @@ describe("rate-limit reset credits", () => { tertiary_window: { used_percent: 50, reset_at: 1788000000 }, }, }); - expect(quota).toEqual({ monthlyPercent: 39, monthlyResetAt: 1787401330 }); + expect(quota).toEqual({ monthlyPercent: 39, monthlyResetAt: 1787401330, monthlyIsPrimaryWindow: true }); }); it("falls back to tertiary wholesale when a monthly primary has no percent", () => { @@ -179,6 +180,8 @@ describe("rate-limit reset credits", () => { tertiary_window: { used_percent: 50, reset_at: 1788000000 }, }, }); + // No provenance flag on the Go/Free branch: the monthly window governs those plans + // regardless of which window produced the reading, so recovery never consults it. expect(quota).toEqual({ monthlyPercent: 30, monthlyResetAt: 1787401330 }); }); @@ -310,11 +313,12 @@ describe("rate-limit reset credits", () => { secondary_window: null, }, }); - expect(quota).toEqual({ monthlyPercent: 100, monthlyResetAt: 1787401330 }); + expect(quota).toEqual({ monthlyPercent: 100, monthlyResetAt: 1787401330, monthlyIsPrimaryWindow: true }); setAccountQuotaFromParsed("monthly-A", quota!); expect(getAccountQuota("monthly-A")).toEqual({ monthlyPercent: 100, monthlyResetAt: 1787401330, + monthlyIsPrimaryWindow: true, updatedAt: expect.any(Number), }); }); @@ -331,6 +335,7 @@ describe("rate-limit reset credits", () => { expect(getAccountQuota("monthly-A")).toEqual({ monthlyPercent: 100, monthlyResetAt: 1787401330, + monthlyIsPrimaryWindow: true, updatedAt: expect.any(Number), }); }); @@ -381,6 +386,7 @@ describe("rate-limit reset credits", () => { expect(getAccountQuota("team-tertiary")).toEqual({ monthlyPercent: 39, monthlyResetAt: 1787401330, + monthlyIsPrimaryWindow: true, updatedAt: expect.any(Number), }); }); @@ -431,6 +437,7 @@ describe("rate-limit reset credits", () => { expect(getAccountQuota("team-A")).toEqual({ monthlyPercent: 39, monthlyResetAt: 1787401330, + monthlyIsPrimaryWindow: true, weeklyPercent: 20, weeklyResetAt: 1787000000, updatedAt: expect.any(Number), From 74c4f765e6b6afddfaeadd4fb6de944442f76075 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 4 Aug 2026 10:14:31 +0900 Subject: [PATCH 12/12] fix(codex): keep window provenance across updateAccountQuota too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last copy site of the same class: an unrelated weekly update rebuilt the record and carried monthlyPercent forward without its provenance, silently downgrading a proven explicit-primary reading to unproven. The mirror case matters as much — a caller-supplied monthly value arrives with no window information, so it must REPLACE the proof rather than inherit it. Both directions are now pinned, and ablating the carry fails the test. --- src/codex/quota.ts | 9 +++++++++ tests/codex-cooldown-recovery.test.ts | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/src/codex/quota.ts b/src/codex/quota.ts index c8ae4543a..562e7082e 100644 --- a/src/codex/quota.ts +++ b/src/codex/quota.ts @@ -323,6 +323,11 @@ export function updateAccountQuota( const quota: StoredAccountQuota = { ...(existing?.weeklyPercent !== undefined ? { weeklyPercent: existing.weeklyPercent } : {}), ...(existing?.monthlyPercent !== undefined ? { monthlyPercent: existing.monthlyPercent } : {}), + // Carry provenance with the value it describes. Dropping it here would downgrade a proven + // explicit-primary reading to "unproven" on the next unrelated weekly update. + ...(existing?.monthlyPercent !== undefined && existing.monthlyIsPrimaryWindow === true + ? { monthlyIsPrimaryWindow: true } + : {}), ...(existing?.weeklyResetAt !== undefined ? { weeklyResetAt: existing.weeklyResetAt } : {}), ...(existing?.monthlyResetAt !== undefined ? { monthlyResetAt: existing.monthlyResetAt } : {}), ...(existing?.resetCredits !== undefined ? { resetCredits: existing.resetCredits } : {}), @@ -338,6 +343,10 @@ export function updateAccountQuota( if (nextMonthly !== undefined) { quota.monthlyPercent = nextMonthly; if (nextMonthlyResetAt !== undefined) quota.monthlyResetAt = nextMonthlyResetAt; + // A caller-supplied monthly value arrives without window provenance, so it REPLACES the + // proven reading and must not inherit its flag — otherwise an unproven number would be + // treated as governing evidence. + delete quota.monthlyIsPrimaryWindow; } if (resetCredits !== undefined) quota.resetCredits = resetCredits; diff --git a/tests/codex-cooldown-recovery.test.ts b/tests/codex-cooldown-recovery.test.ts index aff2f6c6a..c34eff6c6 100644 --- a/tests/codex-cooldown-recovery.test.ts +++ b/tests/codex-cooldown-recovery.test.ts @@ -14,6 +14,7 @@ import { isCompleteCodexQuotaRecoverySnapshot, parseUsageQuota, setAccountQuotaFromParsed, + updateAccountQuota, } from "../src/codex/quota"; import upstreamModels from "../src/codex/data/upstream-models.json"; import { @@ -404,6 +405,14 @@ describe("Codex cooldown recovery worker", () => { expect(parsedMonthly?.monthlyIsPrimaryWindow).toBe(true); setAccountQuotaFromParsed("provenance-probe", parsedMonthly); expect(getAccountQuota("provenance-probe")?.monthlyIsPrimaryWindow).toBe(true); + + // updateAccountQuota() rebuilds the record too. An unrelated weekly update must not + // downgrade a proven reading to unproven, and a caller-supplied monthly value — which + // arrives with no window information at all — must not inherit the proof. + updateAccountQuota("provenance-probe", 20, 111); + expect(getAccountQuota("provenance-probe")?.monthlyIsPrimaryWindow).toBe(true); + updateAccountQuota("provenance-probe", undefined, undefined, 44, 222); + expect(getAccountQuota("provenance-probe")?.monthlyIsPrimaryWindow).toBeUndefined(); // Missing EVIDENCE still fails closed — that is the guard that matters. expect(isCompleteCodexQuotaRecoverySnapshot({}, "plus")).toBe(false); expect(isCompleteCodexQuotaRecoverySnapshot(null, "plus")).toBe(false);