From 3a3759eabde8d5e592fe4c76aa74e5c904583500 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 21:00:04 +0900 Subject: [PATCH 1/6] docs(devlog): re-verify the phase 1 anchors against the current dev tip --- .../010_phase1_manual_selection.md | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md b/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md index d687bdb9c5..eec127e3fa 100644 --- a/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md +++ b/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md @@ -50,9 +50,9 @@ MODIFY `src/codex/routing.ts` `manualPreference` from `config.activeCodexAccountId`, mirroring `anthropic-routing.ts:810`. It keeps clearing thread affinity, clearing the runtime cursor and seeding round-robin, and keeps preserving cooldown. -3. `pickUnboundStrategyAccount` (`:1466-1481`) returns early while a preference is - live, so round-robin and fill-first cannot call `rememberActiveCodexAccount` - over the operator choice. +3. `pickUnboundStrategyAccount` (declared `:1446`, commit sites `:1470` and + `:1481`) returns early while a preference is live, so round-robin and + fill-first cannot call `rememberActiveCodexAccount` over the operator choice. 4. `getEffectiveActiveCodexAccountId` (`:1625`) returns the preference account while one is live, ahead of the runtime cursor. 5. `resolveCodexAccountForThreadDetailed` (`:2069`) checks the preference before @@ -98,3 +98,23 @@ Semantic oracle: `tests/adapters/anthropic/anthropic-account-pool.test.ts` `:144 The generic OAuth kind gets no preference in this layer; that arrives with the kernel in phase 2. No management or GUI change. + +## Staleness re-verification + +Re-verified at the wp1 P entry against `origin/dev` `16f18d654`, after lane L3 +landed `de1d88739`, `abec9ee51` and `7f91737c2` on the owned files. Every anchor +this document depends on is unchanged from the `dd9a2906b` reading: + +| Symbol | Line on 16f18d654 | +|---|---| +| `getEffectiveActiveCodexAccountId` | 1625 | +| `rememberActiveCodexAccount` | 1644 | +| `applyQuotaAutoSwitch` | 1784 | +| `resetCodexRoutingForManualSelection` | 870 | +| `pickUnboundStrategyAccount` | 1446 | +| `releaseDrainedCodexAccountPin` | 1757 | + +The design therefore survives the lane's landings. What does not change is the +coordination risk: L3 still owns these files for the dispatch round, so the B +phase of this work-phase must not open until that ownership clears. Re-run this +table at that point, because the guarantee above is a snapshot of `16f18d654`. From a11da688335414edf419a093211caf3e1aa7ff36 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 21:12:01 +0900 Subject: [PATCH 2/6] docs(devlog): fold three audit rounds into the phase 1 design --- .../010_phase1_manual_selection.md | 91 ++++++++++++++++--- 1 file changed, 78 insertions(+), 13 deletions(-) diff --git a/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md b/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md index eec127e3fa..bfb1a2b1b0 100644 --- a/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md +++ b/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md @@ -43,27 +43,70 @@ src/oauth/anthropic-routing.ts MODIFY `src/codex/routing.ts` -1. NEW module-local `manualPreference: { accountId: string } | null | undefined` - beside `runtimeActiveCodexAccountId` (`:56`). `undefined` means not yet seeded - from the persisted active account; `null` means consumed. +1. NEW `manualPreference`, keyed by pool scope rather than a singleton: + `Map` beside `runtimeActiveCodexAccountId` + (`:56`), keyed by `codexPoolKeyForScope` (`:225`). A singleton would let an + independent quota scope (spark, reserve) apply or consume the shared one-shot, + because `isIndependentCodexQuotaScope` deliberately isolates those from the + shared `remember` path. An absent entry means not yet seeded; `null` means + consumed. + + Seeding is explicit only. The entry is written by + `resetCodexRoutingForManualSelection` and nowhere else. There is no lazy seed + from `config.activeCodexAccountId` on first read, because an absent entry plus a + lazy seed would let an independent quota scope invent a preference it was never + given. + + Invalidation, since Codex has no account-side equivalent of Anthropic's + `selectionRevision` (`apiKeySelectionRevision` is for keys and the store + `generation` is credential lineage): the preference is dropped only by an + OPERATOR-driven change of the active account, meaning another + `resetCodexRoutingForManualSelection` naming a different account, or an explicit + clear. A POOL-driven move must not drop it. + + That distinction is load-bearing and was missed twice. An earlier draft said + "drop it whenever the accountId no longer equals the persisted active account", + which contradicts the guarantee below: `promoteActiveCodexAccount` (`:1677`) + calls `releaseCodexAccountPinFor` and then `setActiveCodexAccount` (`:1660`, + which clears `runtimeActiveCodexAccountId` at `:1661`) BEFORE it would reach the + guarded `remember`. Under the old rule a failover promote would move the + persisted active, look like a mismatch, and silently spend the operator's + one-shot. Keying invalidation to the operator path instead of to value equality + is what keeps F1 and F4 from cancelling each other. 2. `resetCodexRoutingForManualSelection` (`:870`) additionally seeds `manualPreference` from `config.activeCodexAccountId`, mirroring `anthropic-routing.ts:810`. It keeps clearing thread affinity, clearing the runtime cursor and seeding round-robin, and keeps preserving cooldown. -3. `pickUnboundStrategyAccount` (declared `:1446`, commit sites `:1470` and - `:1481`) returns early while a preference is live, so round-robin and - fill-first cannot call `rememberActiveCodexAccount` over the operator choice. +3. The guard sits on BOTH writers, not only on `remember`. + `rememberActiveCodexAccount` (`:1644`) becomes a no-op while a live preference + names a different account, which closes its four call sites `:1470`, `:1481`, + `:1678` and `:2286` at once. That alone is still insufficient, because + `promoteActiveCodexAccount` (`:1677`) releases the pin and calls + `setActiveCodexAccount` (`:1660`) before it ever reaches `remember`. So + `promoteActiveCodexAccount` and `setActiveCodexAccount` also check for a live + preference and leave the operator's account in place for the pool-driven paths + (failover `:1878`, model detour `:2213`, exclusion `:1704`, cooldown `:2534` + and `:2584`). An operator PUT still moves them, because that path seeds a new + preference first. 4. `getEffectiveActiveCodexAccountId` (`:1625`) returns the preference account while one is live, ahead of the runtime cursor. 5. `resolveCodexAccountForThreadDetailed` (`:2069`) checks the preference before - `pickUnboundStrategyAccount` (`:2194`). If it names the persisted active - account and that account is selectable and not exhausted, return it with a - `manual` reason and do not call `rememberActiveCodexAccount`. + `pickUnboundStrategyAccount` (`:2194`). If the preference account is selectable + and not exhausted, return it with a `manual` reason and do not call + `rememberActiveCodexAccount`. Honouring does NOT require the preference to still + equal `config.activeCodexAccountId`: a pool-driven promote may legitimately have + moved that value, and treating the difference as staleness is the mistake the + audit rejected twice. 6. `previewCodexAccountForRequest` (`:1987`) peeks the preference without consuming it. -7. NEW consume-on-success, mirroring `anthropic-routing.ts:799-800`: after a - successful token and admission, set `manualPreference = null` and confirm - `config.activeCodexAccountId`. A failed lookup must not spend the preference. +7. NEW consume-on-success, mirroring `anthropic-routing.ts:799-800`. Codex has no + equivalent of the Anthropic admission commit, so the hook must be named + explicitly: consume at the same point that already records a successful upstream + outcome for the resolved account, `recordCodexUpstreamOutcome`, and only for a + non-quota success. Consuming must call `setActiveCodexAccount` rather than only + nulling the entry, because nulling alone leaves `runtimeActiveCodexAccountId` + pointing at the pool's earlier pick and the next dispatch would silently return + to it. A failed lookup must not spend the preference. MODIFY `src/codex/auth-api.ts` PUT `/api/codex-auth/active` (`:2412-2444`): no contract change. It keeps `setCodexAccountPin` and @@ -73,7 +116,9 @@ preference carries the one-shot. A null body still clears the pin (`:2440`). Explicitly NOT changed: `applyQuotaAutoSwitch` (`:1784`). It only moves at `autoSwitchThreshold`, and `releaseDrainedCodexAccountPin` (`:1757`) already treats that drain as the end of a pin. An earlier draft named it as the cause and -the audit rejected that. +the audit rejected that. Goalplan criterion c-2 therefore already holds on `dev`; +what is missing is not behaviour but proof, so this layer adds the test rather +than the code. ## Tests @@ -94,8 +139,28 @@ new `codex-*.test.ts` would need entries in both `scripts/test-layout/layout.jso Semantic oracle: `tests/adapters/anthropic/anthropic-account-pool.test.ts` `:144`, `:209`, `:234`. +Added after the A-phase audit, because the three files above prove the ceiling and +the drain but not these: + +- a live preference survives `promoteActiveCodexAccount` reached through failover + and through a model detour, and survives a priority preemption +- an independent quota scope neither applies nor consumes the shared preference +- an operator selecting a different account replaces the previous preference, while + a pool-driven promote that moves the persisted active account does not spend it +- criterion c-2 directly: with a pinned account that is selectable and under + `autoSwitchThreshold`, auto-switch holds, under both the quota strategy and + round-robin or fill-first + ## Out of scope +## Audit record + +The A-phase reviewer returned FAIL with one blocker and four majors, all folded +above: the overwrite hole at `promoteActiveCodexAccount` and preemption, the +singleton-versus-scope-keyed state, the missing invalidation rule in the absence +of an account-side revision, the pin-versus-preference disagreement after a +released pin, and the test gap against criterion c-2. + The generic OAuth kind gets no preference in this layer; that arrives with the kernel in phase 2. No management or GUI change. From e395a87d8dce83d0eb38fdfcf90e74a7214fa1f8 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 21:32:16 +0900 Subject: [PATCH 3/6] feat(providers): pick a warm API key before the first attempt --- src/providers/key-failover.ts | 78 +++++++++++++++++++++++++++++ src/server/auth-cors.ts | 2 + src/types/provider.ts | 10 ++++ tests/adapters/key-failover.test.ts | 69 +++++++++++++++++++++++++ 4 files changed, 159 insertions(+) diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index ad4d61ba8d..bce8f3368d 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -100,6 +100,84 @@ export function hasKeyPoolFailover(provider: OcxProviderConfig): boolean { return (provider.apiKeyPool?.length ?? 0) >= 2; } +/** + * Process-local round-robin cursor per provider, deliberately parallel to `keyCooldowns` + * rather than borrowing the Codex pool-rotation state: an API key is not an OAuth account + * and must not share a quota scope key. Multi-process desync is the same accepted limit + * the cooldown map already carries. + */ +const keyRotationCursor = new Map(); + +/** Forget a provider's cursor so an operator's manual key selection is not second-guessed. */ +export function forgetApiKeyRotationCursor(providerName: string): void { + keyRotationCursor.delete(providerName); +} + +/** + * Pick a better key BEFORE the first attempt when the committed one is already cooling. + * + * This is intentionally narrow. It never overrides a healthy key: if the committed + * `apiKey` is not in cooldown it returns null, so an operator's manual selection stands + * and no config write happens. It only acts when the committed key is known-cooled (or + * missing from the pool), which is exactly the case where the first request would + * otherwise be spent earning a 429 the runtime could already predict. + * + * Returning null is the common path, so the persisted-selection transaction is not on + * the per-request hot path. + */ +export function selectProactiveApiKey( + config: OcxConfig, + providerName: string, + now = Date.now(), +): OcxProviderConfig | null { + const provider = config.providers?.[providerName]; + if (!provider) return null; + const strategy = provider.apiKeyPoolStrategy; + if (!strategy) return null; + if (!hasKeyPoolFailover(provider)) return null; + const pool = provider.apiKeyPool ?? []; + + const activeEntry = pool.find(entry => entry.key === provider.apiKey); + // A healthy committed key wins, whether the operator chose it or a previous rotation did. + if (activeEntry && !isKeyInCooldown(providerName, activeEntry.id, now)) return null; + + const eligible = pool.filter(entry => !isKeyInCooldown(providerName, entry.id, now)); + if (eligible.length === 0) return null; + + let chosen = eligible[0]!; + if (strategy === "round-robin") { + const lastId = keyRotationCursor.get(providerName); + const lastIndex = lastId ? pool.findIndex(entry => entry.id === lastId) : -1; + for (let offset = 1; offset <= pool.length; offset += 1) { + const candidate = pool[(lastIndex + offset) % pool.length]!; + if (isKeyInCooldown(providerName, candidate.id, now)) continue; + chosen = candidate; + break; + } + } + if (chosen.key === provider.apiKey) return null; + + const outcome = commitProviderApiKeySelection(config, providerName, freshProvider => { + const freshPool = freshProvider.apiKeyPool ?? []; + const target = freshPool.find(entry => entry.id === chosen.id); + if (!target) return { changed: false, value: null }; + if (freshProvider.apiKey === target.key) return { changed: false, value: null }; + const freshActive = freshPool.find(entry => entry.key === freshProvider.apiKey); + // Re-check under the lock: a concurrent manual selection may have landed a healthy key. + if (freshActive && !isKeyInCooldown(providerName, freshActive.id, now)) { + return { changed: false, value: null }; + } + freshProvider.apiKey = target.key; + return { changed: true, value: target.id }; + }); + if (outcome.status !== "committed" || outcome.value === null) return null; + + keyRotationCursor.set(providerName, outcome.value); + const committed = structuredClone(outcome.provider); + config.providers[providerName] = committed; + return structuredClone(committed); +} + /** * Normalize a provider's `retryOn429` policy, or return null when the knob is absent, * explicitly disabled, or the provider is not key-auth (OAuth/forward credentials must not be diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index cad0acbd33..3a93246cd0 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -817,6 +817,8 @@ const PROVIDER_CONFIG_FIELD_POLICY = { apiKey: "redacted", apiKeyTransport: "editor", apiKeyPool: "redacted", + // Ordering preference only; it names no key material, so an editor may read and set it. + apiKeyPoolStrategy: "editor", apiKeySelectionRevision: "runtime", _apiKeyAttempt: "runtime", defaultModel: "editor", diff --git a/src/types/provider.ts b/src/types/provider.ts index e14b89abe6..e65130a4fa 100644 --- a/src/types/provider.ts +++ b/src/types/provider.ts @@ -387,6 +387,16 @@ export interface OcxProviderConfig { * `apiKey` seeds a one-entry pool on first management touch. */ apiKeyPool?: Array<{ id: string; key: string; label?: string; addedAt?: number }>; + /** + * Optional proactive ordering for `apiKeyPool` when the committed key is already + * cooling. Deliberately NOT named like the OAuth `accountPoolStrategy`: an API key + * is a different identity from an OAuth account set, and key rotation is a + * rate-limit scheduling problem rather than a prompt-cache one. + * + * Absent means today's behaviour: no pre-dispatch pick at all, only the reactive + * 429/401 walk in `key-failover`. + */ + apiKeyPoolStrategy?: "round-robin" | "fill-first"; /** Changes on manual selection (including re-selection) and committed automatic allocation. */ apiKeySelectionRevision?: string; /** Runtime only. Never expose in management responses or persist a routed provider. */ diff --git a/tests/adapters/key-failover.test.ts b/tests/adapters/key-failover.test.ts index 8efb505aa6..1bd89af487 100644 --- a/tests/adapters/key-failover.test.ts +++ b/tests/adapters/key-failover.test.ts @@ -18,6 +18,10 @@ import { rotateProviderTransportOn429, rotateProviderTransportOn401, } from "../../src/providers/key-failover"; +import { + forgetApiKeyRotationCursor, + selectProactiveApiKey, +} from "../../src/providers/key-failover"; import { resolveOpenCodeGoTransport } from "../../src/providers/opencode-go-transport"; import { deriveXaiConvId } from "../../src/providers/xai-transport"; import { routeModel, routedProviderConfig } from "../../src/router"; @@ -449,4 +453,69 @@ describe("rotateKeyOn401", () => { expect(rotated?.apiKey).toBe("key-beta-444555666777"); expect(getKeyCooldownUntil("p", "k1", now)).toBe(now + 10 * 60_000); }); + + describe("proactive key selection", () => { + const now = 2_000_000; + + test("does nothing without a configured strategy", () => { + const config = makeConfig({ apiKey: "key-alpha-000111222333", apiKeyPool: pool3() }); + forgetApiKeyRotationCursor("p"); + rotateKeyOn429(config, "p", null, now); + expect(selectProactiveApiKey(config, "p", now)).toBeNull(); + }); + + test("keeps a healthy committed key instead of rotating off an operator choice", () => { + const config = makeConfig({ + apiKey: "key-alpha-000111222333", + apiKeyPool: pool3(), + apiKeyPoolStrategy: "round-robin", + }); + forgetApiKeyRotationCursor("p"); + // No cooldown recorded, so the committed key is healthy and must survive untouched. + expect(selectProactiveApiKey(config, "p", now)).toBeNull(); + expect(config.providers.p.apiKey).toBe("key-alpha-000111222333"); + }); + + test("moves off a committed key that is already cooling", () => { + const config = makeConfig({ + apiKey: "key-alpha-000111222333", + apiKeyPool: pool3(), + apiKeyPoolStrategy: "round-robin", + }); + forgetApiKeyRotationCursor("p"); + // Cool the committed key, then persist it back as active so the next request starts on + // it. Writing only the in-memory copy is not enough: the selector re-reads under the + // persistence lock, which is the guard that stops it clobbering a healthy choice. + rotateKeyOn429(config, "p", null, now); + setActiveProviderApiKey(config, "p", "k1"); + const picked = selectProactiveApiKey(config, "p", now); + expect(picked).not.toBeNull(); + expect(picked?.apiKey).not.toBe("key-alpha-000111222333"); + expect(getKeyCooldownUntil("p", "k1", now)).toBeGreaterThan(now); + }); + + test("returns null when every key is cooling", () => { + const config = makeConfig({ + apiKey: "key-alpha-000111222333", + apiKeyPool: pool3(), + apiKeyPoolStrategy: "round-robin", + }); + forgetApiKeyRotationCursor("p"); + rotateKeyOn429(config, "p", null, now); + rotateKeyOn429(config, "p", null, now); + rotateKeyOn429(config, "p", null, now); + config.providers.p.apiKey = "key-alpha-000111222333"; + expect(selectProactiveApiKey(config, "p", now)).toBeNull(); + }); + + test("a single-key pool is a no-op", () => { + const config = makeConfig({ + apiKey: "key-alpha-000111222333", + apiKeyPool: [{ id: "k1", key: "key-alpha-000111222333" }], + apiKeyPoolStrategy: "round-robin", + }); + forgetApiKeyRotationCursor("p"); + expect(selectProactiveApiKey(config, "p", now)).toBeNull(); + }); + }); }); From 2e6841be692695536353a45d7a5a305dd69e6fe2 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 21:34:36 +0900 Subject: [PATCH 4/6] fix(config): reject an unknown apiKeyPoolStrategy instead of loading it silently --- src/config.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/config.ts b/src/config.ts index a5a75565ea..106dadd2b6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -580,6 +580,10 @@ const modelPinnedEffortsSchema = z.unknown().superRefine((value, ctx) => { const providerConfigSchema = z.object({ pinnedReasoningEffort: pinnedReasoningEffortSchema.optional(), modelPinnedReasoningEfforts: modelPinnedEffortsSchema.optional(), + // Validated rather than left to passthrough: an unrecognized strategy would otherwise + // load silently and then be ignored at selection time, which reads as a broken feature + // rather than a rejected setting. + apiKeyPoolStrategy: z.enum(["round-robin", "fill-first"]).optional(), adapter: z.string().min(1), baseUrl: z.string().min(1), alias: z.string().optional(), From e1cc65548c91137df280ffc1b70f4b6d3d804f85 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 21:37:08 +0900 Subject: [PATCH 5/6] docs(devlog): scope phase 2 to the files no lane owns --- .../020_phase2_shared_kernel.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md b/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md index c86c01a9ec..e019583ad7 100644 --- a/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md +++ b/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md @@ -9,6 +9,30 @@ Extract the rotation primitives into a credential-neutral kernel, then make the generic OAuth kind actually consume the `strategy` and `autoSwitchThreshold` it already persists. +## Availability and the slice this cycle can actually take + +Re-verified at the wp2 P entry against `origin/dev`. The lane partition for the +round in flight does not list `src/oauth/generic-account-failover.ts`, +`src/oauth/pool-settings-capability.ts` or `src/codex/pool-rotation.ts`, so the +kernel extraction and the generic-kind strategy work are available now. Two things +are not: + +- `src/codex/routing.ts` is owned by lane L3, so the Codex-side import swap waits. +- `src/server/responses/core.ts` is owned by lane L1 and is the most contended + file in the round with four open PRs, which is also why the wp4b call-site + wiring could not follow #4277 immediately. + +This cycle therefore takes the kernel plus the generic consumer and leaves the +Codex and Anthropic import swaps to a later layer. That ordering is not a +concession: a kernel that nothing imports yet is still verifiable through the +generic kind, and it keeps the contended files out of this PR entirely. + +Anchors confirmed present on `origin/dev`: `selectPriorityTier` :86, +`pickRoundRobinAccount` :189, `notePoolRotationSuccess` :213, +`seedPoolRotationAccount` :245, `reconcilePoolRotationState` :260 in +`pool-rotation.ts`; `preferredInitialAccount` :246 and the +`rankAccountsByHeadroom` import :19 in `generic-account-failover.ts`. + ## Current behaviour (verified on dd9a2906b) The primitives already take an opaque `poolKey`, so a third key is addable: From fe2b76364ab93607cebae770a60468d9acbde00a Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 21:46:10 +0900 Subject: [PATCH 6/6] docs(devlog): fold two audit rounds into the phase 2 kernel plan --- .../020_phase2_shared_kernel.md | 103 ++++++++++++++---- 1 file changed, 83 insertions(+), 20 deletions(-) diff --git a/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md b/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md index e019583ad7..b9ead490fd 100644 --- a/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md +++ b/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md @@ -22,10 +22,22 @@ are not: file in the round with four open PRs, which is also why the wp4b call-site wiring could not follow #4277 immediately. -This cycle therefore takes the kernel plus the generic consumer and leaves the -Codex and Anthropic import swaps to a later layer. That ordering is not a -concession: a kernel that nothing imports yet is still verifiable through the -generic kind, and it keeps the contended files out of this PR entirely. +This cycle takes the kernel, the Anthropic import swap and the generic consumer. +Only the CODEX import swap is deferred, and it is deferred for free: once +`pool-rotation.ts` re-exports the kernel, `src/codex/` keeps its existing import +path and needs no edit at all. So the contended files stay out of this PR without +the kernel being an orphan. + +Two kinds of change are moving here and they carry different risk, which is why +only one of them is behind the flag: + +- **Relocation** is behaviour-preserving. Moving the state and primitives into + `pool-kernel.ts` and re-exporting them changes no selection outcome, so it is + not flagged. `git` history and a green existing suite are its proof. +- **Behaviour** is flagged. The generic kind consuming `strategy` and + `autoSwitchThreshold`, and the DTO reporting `inert: false`, only happen when + `pool.kernel` is on. Flag off restores today's outcomes exactly, because the + pre-kernel path is the same code reached through the shim. Anchors confirmed present on `origin/dev`: `selectPriorityTier` :86, `pickRoundRobinAccount` :189, `notePoolRotationSuccess` :213, @@ -69,29 +81,73 @@ and stamps `inert: true` on the generic DTO (`:40-54`, `:57-67`). ## Change surface NEW `src/oauth/pool-kernel.ts` -- move `SelectionState`, `pickRoundRobinAccount`, `peekRoundRobinAccount`, - `seedPoolRotationAccount`, `notePoolRotationSuccess`, `notePoolRotationFailure`, - `selectPriorityTier`, and the strategy/sticky normalizers +- move the WHOLE private `selectionState` map together with + `pickRoundRobinAccount`, `peekRoundRobinAccount`, `seedPoolRotationAccount`, + `notePoolRotationSuccess`, `notePoolRotationFailure`, `clearPoolRotationState`, + `selectPriorityTier`, the priority parsers, `POOL_KEY_*` and the strategy and + sticky normalizers. Moving a function subset while leaving the map behind would + split one piece of state across two modules. +- the move is safe: `pool-rotation.ts` imports only two TYPES, + `OcxAccountPoolRotationStrategy` from `../types` and `GenerationContext` from + `../lib/state-store-sweeper`. Neither creates a cycle into `src/oauth`. - add `genericPoolKey(provider) => \`generic:\${provider}\`` -- lift fill-first to `pickFillFirst(ids, afterId, hasHeadroom)` so both existing - copies call one implementation -- extend the reconcile sweep to `generic:*` keys, which `:270-276` currently skips +- add a fill-first helper with the signature + `pickFillFirst(ids, afterId, hasHeadroom, stableAll)`. The earlier three-argument + shape was rejected by the audit: both existing copies walk a STABLE FULL roster + and not the eligible subset, so dropping `stableAll` changes the wrap order + whenever an ineligible id sits between two eligible ones. +- extend the reconcile sweep to `generic:*`. `buildGenerationContext` already fills + `oauthAccountKeys` from `listLiveOAuthAccountKeys` as `provider\0id` for every + live OAuth provider, so the sweep needs no new field and no Codex dependency; + today those keys are simply skipped as `valid === null`. + +NOT moved, deliberately: the Codex fill-first copy in `src/codex/routing.ts` stays +where it is. Deleting it is the only thing that would force an edit to a file lane +L3 owns, and the audit flagged that as a blocker against this unit's own freeze. +Only `anthropic-routing.ts` and the generic kind switch to the kernel helper, and +the Anthropic caller keeps its weekly `exhausted5h` pre-filter rather than pushing +that rule into the shared helper. MODIFY `src/codex/pool-rotation.ts` — re-export the kernel so existing importers and `tests/codex-integration/codex-pool-rotation.test.ts` keep working unchanged. -MODIFY `src/oauth/generic-account-failover.ts` — route selection through the kernel -by strategy: `quota` keeps `rankAccountsByHeadroom`, `round-robin` calls -`pickRoundRobinAccount(genericPoolKey(name), ...)`, `fill-first` calls the lifted -helper; seed on manual selection; note success and failure. Keep the presence +MODIFY `src/oauth/generic-account-failover.ts` — branch BOTH paths on strategy, not +just the proactive one. `preferredInitialAccount` currently no-ops when the active +account is healthy and requires `hasHeadroomEvidence`, and the 429 path always ends +in `rankAccountsByHeadroom`; leaving either unbranched keeps the strategy inert in +practice even after the DTO says otherwise. `quota` keeps +`rankAccountsByHeadroom`, `round-robin` calls +`pickRoundRobinAccount(genericPoolKey(name), ...)`, and `fill-first` uses the +kernel helper with `autoSwitchThreshold` as its headroom test. Keep the presence quorum, the `EXCLUDED_PROVIDERS` guard and the per-provider `health` cooldown. -MODIFY `src/oauth/pool-settings-capability.ts` — drop `inert: true`, add -`stickyLimit`. MODIFY `src/types/provider.ts:512-518` comments and -`oauth-account-routes.ts:395` to accept `stickyLimit`. - -MODIFY `src/codex/routing.ts` and `src/oauth/anthropic-routing.ts` — import from -the kernel instead of holding their own copies. +MODIFY `src/server/management/oauth-account-routes.ts` — a manual account selection +must seed the cursor, or the operator's pick immediately loses to sticky +round-robin. Today that PUT calls only `forgetGenericFailoverRoster`, which clears +the presence cache and not the rotation state. Add +`seedPoolRotationAccount(genericPoolKey(provider), accountId)` beside it, mirroring +what `resetAnthropicRoutingForManualSelection` already does for Anthropic. +`clearGenericFailoverHealth` is the wrong map and `clearPoolRotationState` wipes +where seeding is wanted. + +MODIFY `src/oauth/pool-settings-capability.ts` — report `inert` from the flag rather +than as a type literal. While `pool.kernel` is off the generic DTO must keep saying +`inert: true`, because nothing consumes the strategy yet and the reversibility rule +below requires the old behaviour to be exactly restorable. The literal becomes a +computed field and only turns false once the kernel is on. + +Known readers of that field, all of which move in the same PR: +`src/cli/account-extended.ts` (forces generic auto-switch inactive), +`tests/server/account-pool-management-api.test.ts` and +`tests/cli/cli-account-pool-verbs.test.ts`. The GUI does not read it. +Also lift the `stickyLimit` rejection at `oauth-account-routes.ts:395` and update +`src/types/provider.ts:512-518` comments. + +MODIFY `src/oauth/anthropic-routing.ts` — import from the kernel. `src/codex/` +keeps importing `./pool-rotation`, which is now a re-export, so this layer needs +no edit inside lane L3's files at all. The audit confirmed the shim is sufficient: +`routing.ts`, `auth-api.ts`, `account-priority.ts` and +`state-store-registrations.ts` all keep their existing import path. ## Reversibility (audit blocker, mandatory) @@ -109,6 +165,13 @@ the kernel instead of holding their own copies. ## Tests +Audit record: the A-phase reviewer returned PASS-WITH-FINDINGS with two blockers, +both folded above. The first was that lifting fill-first out of its Codex copy +would have forced an edit inside lane L3's freeze. The second was that dropping +`inert: true` unconditionally contradicts this document's own reversibility rule, +which requires `pool.kernel` to default off and the old behaviour to be exactly +restorable. + - `tests/codex-integration/codex-pool-rotation.test.ts` — unchanged behaviour through the re-export (`pickRoundRobinAccount` `:270`, `selectPriorityTier` `:111`) - `tests/oauth/generic-oauth-failover.test.ts` — a configured strategy changes the