From a2a1a691d5a2f0353f0d3f4cd557a90376691b0a Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 20:36:00 +0900 Subject: [PATCH 1/3] docs(devlog): open the account pool unification unit --- .../000_plan.md | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 devlog/_plan/260911_account_pool_unification/000_plan.md diff --git a/devlog/_plan/260911_account_pool_unification/000_plan.md b/devlog/_plan/260911_account_pool_unification/000_plan.md new file mode 100644 index 0000000000..b2730d4895 --- /dev/null +++ b/devlog/_plan/260911_account_pool_unification/000_plan.md @@ -0,0 +1,113 @@ +# Account pool unification + +Unit opened 2026-09-11. Base: `dev` at `dd9a2906b` (2.52.0). + +## Objective + +Collapse the three independent account-pool implementations into one shared +selection kernel with per-kind policy, and make an operator's manual account +selection actually win over the pool cursor. + +## Why this unit exists + +An audit of `dev` on 2026-09-11 found pooling is not one feature but three, +plus a fourth path for API keys: + +| Kind | Owner | What it actually does | +|---|---|---| +| Codex | `src/codex/routing.ts`, `src/codex/pool-rotation.ts` | full: strategy, sticky, priority tiers, auto-switch threshold | +| Anthropic | `src/oauth/anthropic-routing.ts` | full: strategy, session affinity, manual preference | +| generic OAuth (10 providers) | `src/oauth/generic-account-failover.ts` | reactive 429 rotation only; `strategy` and `autoSwitchThreshold` are persisted but inert | +| API keys | `src/providers/key-failover.ts` | reactive 429/401 index walk; no strategy at all | + +The generic kind already has a settings DTO and a capability enum +(`src/oauth/pool-settings-capability.ts` returns `"codex" | "anthropic" | "generic"`), +so the seam for a shared layer was designed and then left hollow. This unit fills +it rather than inventing a new abstraction. + +## The defect that motivates work-phase 1 + +Reported by the maintainer and confirmed in code: the pool moves the active +account to B, the operator then selects A through the dashboard or +`ocx account use`, and the runtime keeps serving B. + +The cause is a single expression. `getEffectiveActiveCodexAccountId` +(`src/codex/routing.ts:1626`) resolves +`runtimeActiveCodexAccountId ?? config.activeCodexAccountId` and the pin is not +part of it, while `rememberActiveCodexAccount` (`:1645`) writes the pool's pick +into `runtimeActiveCodexAccountId` without releasing the pin. The runtime cursor +therefore outranks the stored operator choice. `applyQuotaAutoSwitch` has no pin +check at all. GUI and CLI are not the divergence: both issue the same +`PUT /api/codex-auth/active`. + +## Settled semantics + +Recorded during the 2026-09-11 interview (session tracker rounds 1-5): + +- **Manual selection is a one-shot preference that commits on success.** The next + dispatch uses the operator's account; if that dispatch succeeds the account is + committed as the stored active one. The pool may move again only for a real + reason such as 429, cooldown or quota exhaustion. This is the shape Anthropic + already implements through `manualPreference`; Codex and the generic kind lack it. +- **One shared layer, different policy per kind.** Selection order, cooldown and + account state are shared. Policy is not: API keys are a rate-limit scheduling + problem and rotate cheaply, while subscription accounts lose their prompt cache + on every move, so cache affinity must be consulted before quota for them. + +## Constraints + +- `dev` is the only integration branch; every layer targets the branch below it. +- Bun-native TypeScript. No Node-only APIs, no compile step. +- Touching OAuth account selection and credential resolution puts this unit inside + the AGENTS.md security boundary, so each layer needs explicit security review and + must not log tokens or account identifiers. +- `privacy:scan` must stay green. +- Existing Codex and Anthropic pool behavior must not regress; they migrate onto + the shared layer rather than being rewritten in place. + +## Work-phase map + +Dependency order, not effort order. Each layer stands alone with its own tests. + +| Phase | Doc | Thesis | Depends on | +|---|---|---|---| +| 0 | this unit | roadmap written to diff level | — | +| 1 | `010_phase1_manual_selection.md` | an operator pick beats the pool cursor | 0 | +| 2 | `020_phase2_shared_kernel.md` | the generic kind stops being inert | 1 | +| 3 | `030_phase3_cache_affinity.md` | cache affinity ranks ahead of quota | 2 | +| 4 | `040_phase4_key_pool_strategy.md` | API keys gain proactive selection | 2 | +| 5 | `050_phase5_surface_consolidation.md` | three contracts and two GUIs become one | 2 | + +Phases 3, 4 and 5 all depend on 2 and are independent of each other, so they are +parallel branches off the phase-2 layer rather than a deeper chain. + +## Delivery + +A manual branch chain, each layer a PR based on the layer below +(`gh pr create --base`). GitHub native stacks are not used: per +DEV-STACK-OPT-IN-01 a generic request to stack is not native opt-in. + +Stack depth is held at three for the first train (phases 1, 2, 3). Phases 4 and 5 +open after phase 2 lands, because DEV-STACK-01 warns that chains past four layers +cost more in cascading than they return. + +## Open assumptions + +Carried out of the interview unresolved. Each is a question the roadmap answers in +its own phase doc, not a blocker on this plan. + +1. **Affinity key composition.** Codex keys on thread id, Anthropic on a session + key. A shared key shape is not yet chosen. Phase 3 decides it. +2. **Shared-cohort handling.** `promptCacheKeyIsSharedCohort` currently discards + affinity entirely when a `prompt_cache_key` looks shared. Whether to fall back + to another identifier instead of discarding is open. +3. **Cache minimum threshold.** There is no minimum-token gate before applying + `cache_control`, and Anthropic's own 1024/2048 breakpoint minimum is not + implemented locally. Whether to add one is open. + +## Evidence + +Audit conducted 2026-09-11 against `origin/dev`. Interview record: +`.codexclaw/interviews/01a08fce-634e-7531-b383-26f2251d9dae.jsonl`, tracker +`.codexclaw/sessions/01a08fce-634e-7531-b383-26f2251d9dae.json` (five scan rounds, +no unresolved contradictions). From df53e8539857e6445693d57da24626c32f4ea0a5 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 20:50:57 +0900 Subject: [PATCH 2/3] docs(devlog): fold two audit rounds into the account pool unification plan --- .../000_plan.md | 79 +++++++++++++++---- 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/devlog/_plan/260911_account_pool_unification/000_plan.md b/devlog/_plan/260911_account_pool_unification/000_plan.md index b2730d4895..203c540c42 100644 --- a/devlog/_plan/260911_account_pool_unification/000_plan.md +++ b/devlog/_plan/260911_account_pool_unification/000_plan.md @@ -17,7 +17,7 @@ plus a fourth path for API keys: |---|---|---| | Codex | `src/codex/routing.ts`, `src/codex/pool-rotation.ts` | full: strategy, sticky, priority tiers, auto-switch threshold | | Anthropic | `src/oauth/anthropic-routing.ts` | full: strategy, session affinity, manual preference | -| generic OAuth (10 providers) | `src/oauth/generic-account-failover.ts` | reactive 429 rotation only; `strategy` and `autoSwitchThreshold` are persisted but inert | +| generic OAuth (10 providers) | `src/oauth/generic-account-failover.ts` | 429 rotation plus a proactive headroom preference when `enabled`; only `strategy` and `autoSwitchThreshold` are persisted-but-inert | | API keys | `src/providers/key-failover.ts` | reactive 429/401 index walk; no strategy at all | The generic kind already has a settings DTO and a capability enum @@ -31,15 +31,19 @@ Reported by the maintainer and confirmed in code: the pool moves the active account to B, the operator then selects A through the dashboard or `ocx account use`, and the runtime keeps serving B. -The cause is a single expression. `getEffectiveActiveCodexAccountId` -(`src/codex/routing.ts:1626`) resolves -`runtimeActiveCodexAccountId ?? config.activeCodexAccountId` and the pin is not -part of it, while `rememberActiveCodexAccount` (`:1645`) writes the pool's pick -into `runtimeActiveCodexAccountId` without releasing the pin. The runtime cursor -therefore outranks the stored operator choice. `applyQuotaAutoSwitch` has no pin -check at all. GUI and CLI are not the divergence: both issue the same +The shape of the defect, not its patch: the Codex pin is a priority-tier ceiling +rather than a selection input, so the strategy picker and the preemption path can +return a different account and record it as the runtime choice. Anthropic solves +the same problem with a one-shot `manualPreference` that Codex and the generic +kind do not have. GUI and CLI are not the divergence: both issue the same `PUT /api/codex-auth/active`. +This is a pin-semantics change, not a one-expression bug. An earlier draft named +`applyQuotaAutoSwitch` as the cause; the A-phase audit rejected that, because that +path only moves at `autoSwitchThreshold`, which the drain handler already treats +as the end of a pin. Exact call sites, line anchors and the before/after contract +belong to `010_phase1_manual_selection.md`, not here. + ## Settled semantics Recorded during the 2026-09-11 interview (session tracker rounds 1-5): @@ -56,7 +60,9 @@ Recorded during the 2026-09-11 interview (session tracker rounds 1-5): ## Constraints -- `dev` is the only integration branch; every layer targets the branch below it. +- `dev` is the only integration branch. Layers that sit in a chain target the layer + below them; layers that are not in a chain target `dev` directly. The Delivery + section names which is which. - Bun-native TypeScript. No Node-only APIs, no compile step. - Touching OAuth account selection and credential resolution puts this unit inside the AGENTS.md security boundary, so each layer needs explicit security review and @@ -64,6 +70,20 @@ Recorded during the 2026-09-11 interview (session tracker rounds 1-5): - `privacy:scan` must stay green. - Existing Codex and Anthropic pool behavior must not regress; they migrate onto the shared layer rather than being rewritten in place. +- **Lane ownership.** `devlog/_plan/260911_lane_dispatch_round/010_lane_partition.md` + is the authoritative ownership list for the multi-lane round in flight on `dev`, + and lane L3 owns `src/codex/auth-api.ts`, `src/codex/routing.ts` and + `src/types/config.ts`. Work-phases 1 and 2 need those files, so no implementation + cycle may open against them until that lane releases them or the maintainer + reassigns ownership. This roadmap cycle writes documents only and takes no owned path. +- **Reversibility is a precondition, not a nicety.** Because this unit changes + credential selection, every migrating phase ships behind a flag that defaults to + the existing pools, dual-reads the already-persisted keys + (`accountPoolStrategy`, `accountPoolStickyLimit`, `autoSwitchThreshold`, + `anthropicAccountPool`, `providers..oauthAccountFailover`, + `activeCodexAccountPinned`), and proves parity with before/after selection traces + for Codex and Anthropic across manual, affinity, quota, round-robin and fill-first. + Flag-off is the rollback. ## Work-phase map @@ -73,13 +93,24 @@ Dependency order, not effort order. Each layer stands alone with its own tests. |---|---|---|---| | 0 | this unit | roadmap written to diff level | — | | 1 | `010_phase1_manual_selection.md` | an operator pick beats the pool cursor | 0 | -| 2 | `020_phase2_shared_kernel.md` | the generic kind stops being inert | 1 | -| 3 | `030_phase3_cache_affinity.md` | cache affinity ranks ahead of quota | 2 | -| 4 | `040_phase4_key_pool_strategy.md` | API keys gain proactive selection | 2 | +| 2 | `020_phase2_shared_kernel.md` | one kernel, and the generic kind consumes its persisted strategy and threshold | 1 | +| 3 | `030_phase3_cache_affinity.md` | cache affinity ranks ahead of quota | 2, plus the three open assumptions closed | +| 4 | `040_phase4_key_pool_strategy.md` | API keys gain proactive selection | none (parallel off trunk) | | 5 | `050_phase5_surface_consolidation.md` | three contracts and two GUIs become one | 2 | -Phases 3, 4 and 5 all depend on 2 and are independent of each other, so they are -parallel branches off the phase-2 layer rather than a deeper chain. +Phase 4 was reparented during the A-phase audit. It does not depend on phase 2: +`src/providers/key-failover.ts` shares no module with the OAuth kernel, and an API +key is a different identity from an OAuth account set. It runs parallel off trunk, +and would gain a dependency only if phase 2 chose to export a credential-kind-agnostic +kernel that `key-failover` imports, which phase 2 does not promise. + +Phase 3 is the speculative layer: all three open assumptions below live in it, so it +does not ride the first train. + +Phase 5 and phase 4 must not both edit the pool management routes and the shared GUI +controls. Phase 5 owns `src/server/management/oauth-account-routes.ts`, the route +registry entries and the GUI pool surfaces; phase 4 keeps key-strategy fields out of +those files and exposes nothing operator-visible until phase 5 gives it a home. ## Delivery @@ -87,9 +118,10 @@ A manual branch chain, each layer a PR based on the layer below (`gh pr create --base`). GitHub native stacks are not used: per DEV-STACK-OPT-IN-01 a generic request to stack is not native opt-in. -Stack depth is held at three for the first train (phases 1, 2, 3). Phases 4 and 5 -open after phase 2 lands, because DEV-STACK-01 warns that chains past four layers -cost more in cascading than they return. +The first chain is two layers, phase 1 then phase 2. Phase 5 opens off the phase-2 +layer once the kernel lands. Phase 3 waits for its assumptions to close. Phase 4 is +an ordinary PR off `dev` and joins no chain. This replaces an earlier 1-2-3 chain +that the audit rejected for carrying the speculative layer. ## Open assumptions @@ -105,6 +137,19 @@ its own phase doc, not a blocker on this plan. `cache_control`, and Anthropic's own 1024/2048 breakpoint minimum is not implemented locally. Whether to add one is open. +## Audit record + +Two independent reviewers audited this plan at A and both returned FAIL. Folded +findings: the phase-1 implementation recipe moved out of this 000 document +(LEXICO-SPLIT-01); the causal story corrected away from `applyQuotaAutoSwitch`; the +generic-OAuth description corrected from "reactive only"; phase 4 reparented off +trunk; rollback, feature flag, persisted-config dual-read and parity proof added as +constraints; the lane-ownership collision with `260911_l3_account_pool` recorded as +a hard precondition on phases 1 and 2. + +One finding is passed to a phase doc rather than folded here: `key-failover` already +logs `failedId` and `candidateId`, so `040` must forbid inheriting that logging shape. + ## Evidence Audit conducted 2026-09-11 against `origin/dev`. Interview record: From c69f46b656bca3bbb177d78b4e79ecbf91a5218c Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 11 Sep 2026 20:54:24 +0900 Subject: [PATCH 3/3] docs(devlog): write the five phase documents for the account pool unit --- .../010_phase1_manual_selection.md | 100 ++++++++++++++++++ .../020_phase2_shared_kernel.md | 96 +++++++++++++++++ .../030_phase3_cache_affinity.md | 71 +++++++++++++ .../040_phase4_key_pool_strategy.md | 66 ++++++++++++ .../050_phase5_surface_consolidation.md | 66 ++++++++++++ 5 files changed, 399 insertions(+) create mode 100644 devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md create mode 100644 devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md create mode 100644 devlog/_plan/260911_account_pool_unification/030_phase3_cache_affinity.md create mode 100644 devlog/_plan/260911_account_pool_unification/040_phase4_key_pool_strategy.md create mode 100644 devlog/_plan/260911_account_pool_unification/050_phase5_surface_consolidation.md 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 new file mode 100644 index 0000000000..d687bdb9c5 --- /dev/null +++ b/devlog/_plan/260911_account_pool_unification/010_phase1_manual_selection.md @@ -0,0 +1,100 @@ +# Phase 1 — an operator pick beats the pool cursor (Codex) + +Base: `origin/dev` `dd9a2906b`. Branch: `codex/pool-manual-selection` off `dev`. +Precondition: lane L3 owns `src/codex/routing.ts` and `src/codex/auth-api.ts` +(000_plan.md constraints). Do not open this layer until that ownership clears. + +## Thesis + +A manual selection from the dashboard or `ocx account use` wins the next dispatch, +and commits as the stored active account when that dispatch succeeds. + +## Current behaviour (verified on dd9a2906b) + +``` +src/codex/routing.ts + 56 let runtimeActiveCodexAccountId: string | undefined; + 1625 export function getEffectiveActiveCodexAccountId(config: OcxConfig): string | undefined { + 1626 return runtimeActiveCodexAccountId ?? config.activeCodexAccountId; + 1644 function rememberActiveCodexAccount(_config: OcxConfig, accountId: string): void { + 1645 runtimeActiveCodexAccountId = accountId; +``` + +`rememberActiveCodexAccount` is called at `:1470` (round-robin commit), `:1481` +(fill-first commit), `:1678` (`promoteActiveCodexAccount`) and `:2286` +(preemption). None of the four consults the pin. The pin itself +(`config.activeCodexAccountPinned`, written only by `auth-api.ts:2441`) is read as +a priority-tier ceiling in `getEligiblePoolAccounts` `:1318-1322` and nowhere else +in the selection path. + +The path to copy is Anthropic's: + +``` +src/oauth/anthropic-routing.ts + 94 let manualPreference: OAuthAccountSelection | null | undefined; + 575 if (manualPreference === undefined) { ...seed from set.activeAccountId + selectionRevision } + 588 if (manualPreference.accountId !== set.activeAccountId || revision mismatch) manualPreference = null; + 597 return { accountId: chosen, reason: "manual" }; + 799 // consumed only after the admission commit + 808 export function resetAnthropicRoutingForManualSelection(accountId: string) +``` + +## Change surface + +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. +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` (`:1466-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 + `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`. +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. + +MODIFY `src/codex/auth-api.ts` PUT `/api/codex-auth/active` (`:2412-2444`): +no contract change. It keeps `setCodexAccountPin` and +`resetCodexRoutingForManualSelection`; the pin stays the tier ceiling and the new +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. + +## Tests + +Extend, do not add files. `codex-` is not in the `layout.json` domain regex, so a +new `codex-*.test.ts` would need entries in both `scripts/test-layout/layout.json` +`explicit` and `tests/fixtures/test-layout-expected.json`. + +- `tests/codex-integration/codex-pool-rotation.test.ts` — the operator pick wins the + next round-robin and fill-first dispatch (manual seed cases at `:524-541`); the + existing pin-holds-RR case at `:791-803` stays green for the ceiling after the + preference is consumed. +- `tests/codex-integration/codex-routing.test.ts` — a second unbound session follows + the pool cursor again once the preference is spent; a failed admission leaves the + preference unspent (pin cases at `:3139-3242`). +- `tests/codex-integration/codex-auth-api.test.ts` — PUT then next-dispatch identity + (`:3956-3989`). + +Semantic oracle: `tests/adapters/anthropic/anthropic-account-pool.test.ts` `:144`, +`:209`, `:234`. + +## Out of scope + +The generic OAuth kind gets no preference in this layer; that arrives with the +kernel in phase 2. No management or GUI change. 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 new file mode 100644 index 0000000000..c86c01a9ec --- /dev/null +++ b/devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md @@ -0,0 +1,96 @@ +# Phase 2 — one kernel, and the generic kind consumes its persisted settings + +Base: the phase-1 layer. Branch: `codex/pool-shared-kernel`, PR base +`codex/pool-manual-selection`. Same lane-L3 precondition as phase 1. + +## Thesis + +Extract the rotation primitives into a credential-neutral kernel, then make the +generic OAuth kind actually consume the `strategy` and `autoSwitchThreshold` it +already persists. + +## Current behaviour (verified on dd9a2906b) + +The primitives already take an opaque `poolKey`, so a third key is addable: + +``` +src/codex/pool-rotation.ts + 4-5 POOL_KEY_CODEX = "codex"; POOL_KEY_ANTHROPIC = "anthropic"; + 13 const selectionState = new Map(); + 86 selectPriorityTier(ids, priorityOf, hasHeadroom, pinnedId?) + 189 pickRoundRobinAccount(poolKey: string, eligibleIds, stickyLimit) + 201 peekRoundRobinAccount(...) + 213 notePoolRotationSuccess(poolKey, accountId, stickyLimit) + 232 notePoolRotationFailure(poolKey, accountId) + 245 seedPoolRotationAccount(poolKey, accountId) + 270 reconcilePoolRotationState // only sweeps "anthropic", "codex", "codex:*" +``` + +Fill-first is duplicated rather than shared: `pickFillFirstCodexAccount` +(`routing.ts:1370`) and `pickFillFirstAnthropicAccount` +(`anthropic-routing.ts:513`). + +`src/oauth/generic-account-failover.ts` imports nothing from `pool-rotation.ts`. +It keeps its own cooldown `health` map (`:64-70`, keyed `provider\0accountId`), +rotates on 429 through `rankAccountsByHeadroom` (`:178-218`) and steers the first +attempt through `preferredInitialAccount` (`:246-292`) when +`oauthAccountFailover.enabled`. It never reads `failover.strategy` or +`autoSwitchThreshold`. + +`src/oauth/pool-settings-capability.ts` returns `"codex" | "anthropic" | "generic"` +and stamps `inert: true` on the generic DTO (`:40-54`, `:57-67`). +`src/server/management/oauth-account-routes.ts:395-396` still rejects +`stickyLimit` and `quotaWindow` for the generic kind. + +## Change surface + +NEW `src/oauth/pool-kernel.ts` +- move `SelectionState`, `pickRoundRobinAccount`, `peekRoundRobinAccount`, + `seedPoolRotationAccount`, `notePoolRotationSuccess`, `notePoolRotationFailure`, + `selectPriorityTier`, and the strategy/sticky normalizers +- 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 + +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 +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. + +## Reversibility (audit blocker, mandatory) + +1. **Flag.** `pool.kernel` defaults to `false`. With it off, Codex and Anthropic + take the pre-kernel code path and the generic kind keeps reporting `inert`. +2. **Dual-read.** The kernel reads the already-persisted keys without rewriting + them: `accountPoolStrategy`, `accountPoolStickyLimit`, `autoSwitchThreshold`, + `anthropicAccountPool.*`, `providers..oauthAccountFailover`, + `activeCodexAccountPinned`. No migration writes on upgrade. +3. **Rollback.** Flag off. No config is rewritten, so downgrade is a restart. +4. **Parity proof.** Golden selection traces recorded before and after for Codex + and Anthropic across manual, affinity, quota, round-robin and fill-first, plus + the `__main__` and independent-quota-scope callers. Identical picks are the + gate; a differing pick is a blocker, not a note. + +## Tests + +- `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 + selected account, which is the criterion that closes "no longer inert" +- `tests/server/account-pool-management-api.test.ts` `:435`, `:449` and + `tests/cli/cli-account-pool-verbs.test.ts` `:315` — update the inert assertions +- `tests/adapters/anthropic/anthropic-account-pool.test.ts` — parity +- `tests/providers/kiro/kiro-pool-rank.test.ts` — the kiro exhaustion special case + in `account-quota-rank.ts:84-108` survives diff --git a/devlog/_plan/260911_account_pool_unification/030_phase3_cache_affinity.md b/devlog/_plan/260911_account_pool_unification/030_phase3_cache_affinity.md new file mode 100644 index 0000000000..bee0f768df --- /dev/null +++ b/devlog/_plan/260911_account_pool_unification/030_phase3_cache_affinity.md @@ -0,0 +1,71 @@ +# Phase 3 — cache affinity ranks ahead of quota + +Base: the phase-2 layer, and all three open assumptions in 000_plan.md closed +first. This is the speculative layer and does not ride the first train. + +## Thesis + +For subscription accounts, moving account destroys the prompt cache, so affinity +is consulted before quota. For API keys it is not, which is why phase 4 keeps a +different policy. + +## Current behaviour (verified on dd9a2906b) + +Stickiness exists but is not cache-driven. + +Codex binds on thread identity: codexPoolAffinityKey (src/codex/auth-context.ts) +from x-codex-parent-thread-id or an HMAC of session and thread id, bound by +bindThreadAffinity (routing.ts:1262), read at :1090. LRU cap +CODEX_THREAD_AFFINITY_MAX_ENTRIES = 2048 (:135), pruned oldest-first at +:1211-1234, idle TTL 24h (:134). + +Anthropic binds on a session key: anthropicSessionKeyFromParts +(anthropic-routing.ts:877) prefers client, session and thread id and treats +promptCacheKey as a last resort, discarding it entirely when +promptCacheKeyIsSharedCohort (:894). Cap MAX_AFFINITY_ENTRIES = 2000 (:48), +evict oldest by lastUsedAt (:468-471). + +Generic OAuth has no affinity at all (module comment :1-15). + +reevaluateAffinityQuota (routing.ts:1942) may rebind a live thread when the quota +strategy is active and usage passes autoSwitchThreshold (:2164-2170); round-robin +and fill-first stay sticky (:2157-2160). + +accountPoolStickyLimit is not a binding-count cap. It is the number of successful +binds retained on one round-robin selection, default 1 (src/types/config.ts:841, +pool-rotation.ts:167-171 and :204-216), so at the default it never even sets +activeKey. The real caps are the two LRU limits above. + +No minimum-token cache gate exists anywhere: there is no cacheThreshold or +minCacheTokens, and applyPromptCaching (src/adapters/anthropic.ts:100) places +cache_control without a size check. MAX_CACHE_BREAKPOINTS = 4 (:60) is the only +real cache numeric. + +## Open assumptions this phase must close first + +1. Affinity key shape. Codex keys on thread, Anthropic on session. Proposed + shared shape, to confirm before implementation: a composite of tenant, + conversation, provider and model, which is what cache-affine proxy practice + recommends over hashing the request body. +2. Shared cohort. Today a shared-looking prompt_cache_key discards affinity + entirely. Decide whether to fall back to another identifier instead. +3. Minimum cache size. Decide whether to implement a minimum-token gate and the + Anthropic 1024 and 2048 breakpoint minimum locally. + +## Change surface (provisional, re-verify at P) + +NEW src/oauth/affinity-key.ts - one composite key builder used by Codex, +Anthropic and the generic kind through the phase-2 kernel. + +MODIFY the kernel selection order so that, for pools marked cache-sensitive, a +live affinity binding outranks a higher-headroom candidate unless the affine +account is exhausted. Key pools are not marked cache-sensitive. + +MODIFY reevaluateAffinityQuota so a rebind requires exhaustion rather than merely +passing the threshold, because a threshold rebind throws away a warm cache. + +## Tests + +A cache-affine account is chosen over a higher-headroom one; an exhausted affine +account still yields; concurrent distinct sessions keep distinct accounts; a +shared-cohort cache key does not collapse every session onto one account. diff --git a/devlog/_plan/260911_account_pool_unification/040_phase4_key_pool_strategy.md b/devlog/_plan/260911_account_pool_unification/040_phase4_key_pool_strategy.md new file mode 100644 index 0000000000..f5d42333ff --- /dev/null +++ b/devlog/_plan/260911_account_pool_unification/040_phase4_key_pool_strategy.md @@ -0,0 +1,66 @@ +# Phase 4 — API keys gain proactive selection + +Base: dev directly. This layer is NOT in the chain: key-failover shares no module +with the OAuth kernel, and an API key is a different identity from an OAuth +account set. The A-phase audit reparented it here. + +## Thesis + +API-key pools get a proactive strategy before the first attempt, while keeping +the existing reactive 429 and 401 rotation as the fallback. + +## Current behaviour (verified on dd9a2906b) + +src/providers/key-failover.ts is reactive only. hasKeyPoolFailover (:98-101) +requires authMode not oauth or forward and apiKeyPool length at least 2. +Selection is a circular index walk in rotateKeyAfterFailure (:220-233) starting +from the failed entry, skipping cooled keys. Cooldown state is a local map +(:19-53) keyed by provider and key id. Wrappers: rotateKeyOn429 (:269-278), +rotateKeyOn401 (:288-296), rotateProviderTransportOn429 (:322-338). + +src/providers/api-keys.ts listProviderApiKeys (:62-80) returns the pool and an +activeId with no strategy. src/types/provider.ts:384-389 defines apiKeyPool as +id, key, label and addedAt only. + +The pre-dispatch hook points are in src/server/responses/core.ts: :4188-4190 +(refreshDispatchAdapter calling resolveCurrentProviderApiKeyTransport) and +:4437-4444 (resolveProviderTransport after OAuth resolution). The OAuth side has +preferredInitialAccount at :4335-4340 with the comment that it prefers a known +headroom account before the first attempt; API keys have no analogue. + +## Change surface + +MODIFY src/types/provider.ts - add an optional per-provider key-pool strategy +field. Do not reuse the OAuth account-pool field names; these are different +identities and phase 5 owns the operator surface. + +MODIFY src/providers/key-failover.ts - add a proactive selector invoked from the +pre-dispatch sites, supporting round-robin and a rate-limit-aware order. Keep +:220-233 exactly as the 429 and 401 fallback. + +MODIFY src/server/responses/core.ts at :4188 and :4437 to consult the selector +before the first attempt. The mid-retry resolveProviderTransport calls at :4119, +:5399, :5503 and :7346 stay recovery paths and are not touched. + +## Policy difference from OAuth pools, stated deliberately + +Key rotation is a rate-limit scheduling problem: keys usually share an account or +organization, so moving key costs little cache. Subscription accounts lose their +prompt cache on every move. That is why phase 3 puts affinity ahead of quota for +accounts and this phase does not for keys. + +## Security + +key-failover already logs failedId and candidateId. The new selector must not +inherit that shape, and must log no key identity. privacy:scan stays green. + +## Tests + +A configured round-robin strategy changes the first-attempt key; the reactive 429 +and 401 walk still works when the strategy is unset; a cooled key is skipped by +both paths; a single-key pool is a no-op. + +## Out of scope + +No operator-visible surface. Phase 5 owns the management route and GUI; adding +fields there from this layer would collide with it. diff --git a/devlog/_plan/260911_account_pool_unification/050_phase5_surface_consolidation.md b/devlog/_plan/260911_account_pool_unification/050_phase5_surface_consolidation.md new file mode 100644 index 0000000000..0144d41f82 --- /dev/null +++ b/devlog/_plan/260911_account_pool_unification/050_phase5_surface_consolidation.md @@ -0,0 +1,66 @@ +# Phase 5 — three contracts and two GUIs become one + +Base: the phase-2 layer. Opens once the kernel lands. + +## Thesis + +One pool-settings contract and one operator surface, so a new pooled provider +needs configuration rather than another name branch. + +## Current behaviour (verified on dd9a2906b) + +Three management contracts: + +1. Codex only. src/codex/auth-api.ts handleCodexAuthAPI :2477-2515 handles PUT + and PATCH /api/codex-auth/pool-strategy, writing accountPoolStrategy and + accountPoolStickyLimit. There is no GET on this path. +2. Anthropic versus generic. src/server/management/oauth-account-routes.ts + handleOauthAccountRoutes branches on provider !== "anthropic": GET :348-361, + PUT and PATCH :373-423 with stickyLimit and quotaWindow rejected at :395-396, + and the anthropic write at :424-483. +3. Registry. src/server/management/route-registry.ts :95 and :110 for the Codex + path, :263, :270 and :283 for the oauth pool path. + +Two GUI surfaces, one shared control: + +- shared gui/src/components/AccountPoolStrategyControls.tsx :42 and + gui/src/account-pool-strategy.ts, whose putCodexPoolStrategy :58-65 posts to the + Codex-only route +- Codex gui/src/components/CodexPoolStrategySetting.tsx :33 and :174 +- Anthropic gui/src/components/provider-workspace/AnthropicAccountPoolSettings.tsx + :63 GET and :117 PUT, hardcoded to provider=anthropic +- mounted by a name branch in + gui/src/components/provider-workspace/ProviderAuthPanel.tsx :387-389, + item.name === "anthropic" only, so the generic kind has an API and no UI + +i18n: 36 accountPool.* keys in gui/src/i18n/en.ts :1981-2023, and every catalog in +gui/src/i18n/catalogs.ts :24-33 already carries 36. All nine stay in sync. + +## Change surface + +NEW one pool-settings DTO covering every kind, served from a single route pair +under the oauth-account-routes module, with the Codex path kept as a deprecated +alias that forwards rather than duplicating the write. + +MODIFY ProviderAuthPanel to mount the pool panel from the capability returned by +poolSettingsCapability instead of item.name === "anthropic". + +MODIFY AnthropicAccountPoolSettings into a kind-driven component; keep +AccountPoolStrategyControls as the shared control it already is. + +MODIFY the i18n catalogs together. Any new key lands in all nine files in the same +commit, per the docs-sync rule in AGENTS.md. + +## Boundary with phase 4 + +This layer owns oauth-account-routes.ts, the route registry entries and the GUI +pool surfaces. Phase 4 keeps key-strategy fields out of those files. If the key +pool needs an operator surface, it arrives here after both have landed, not in +parallel. + +## Tests + +tests/server/account-pool-management-api.test.ts for the unified DTO and the +deprecated alias; tests/cli/cli-account-pool-verbs.test.ts for CLI parity; a GUI +test that the panel mounts for a generic OAuth provider. A gui-labelled PR needs a +screenshot in its description per AGENTS.md.