feat(codex): selection order for the account pool - #715
Open
XertroV wants to merge 46 commits into
Open
Conversation
* fix(ci): let the PR labeler actually write labels * test(ci): pin the labeler to the permissions the API demands
The .codexclaw/ goalplans and ledgers are per-machine agent state. They were committed with 'git add -f' despite the ignore rule, and once tracked the rule stopped applying, so they rode along into main and preview. Untrack them (files stay on disk), drop two .DS_Store files that got in the same way, and add tests/repo-hygiene.test.ts so a forced add fails CI instead of landing silently. Also drop a registry.ts comment pointing at a .codexclaw evidence file that was never committed and cannot be resolved by any reader.
Promotes the current dev line to main. Highlights since v2.7.42: - Security hardening batch 1 (lidge-jun#697): management/data credential split, blank-hostname rejection, pinned provider discovery transports, clickjacking and websocket-origin defenses, and safer update/uninstall paths. - 31 bug fixes and 18 features across catalog, Windows service, Cursor bridge, Codex account routing, and the GUI. - Account-pool settings card no longer prints its title twice. BREAKING CHANGES: - A blank "hostname" is rejected on write and degraded to 127.0.0.1 on read with a warning. - Data-plane credentials (OPENCODEX_API_AUTH_TOKEN, service-api-token, config.apiKeys) no longer reach /api/*. Operational scripts must move to OPENCODEX_ADMIN_AUTH_TOKEN. Model clients are unaffected. - A non-loopback dashboard no longer mints session tokens.
Adds the storage and pure-selection primitives for account priority tiers, with no routing behaviour change yet. - pool-rotation.ts: DEFAULT/MIN/MAX_ACCOUNT_PRIORITY, strict parseAccountPriority + lenient normalizeAccountPriority (mirroring the sticky-limit pair), and selectPriorityTier — the pure tier filter routing will hang off. Provider-agnostic so the Anthropic pool can adopt it. - account-priority.ts: config-sidecar storage (codexAccountPriorities) plus the manual-selection pin, mirroring account-pause.ts. - config schema: proto-guarded record wrapped in .catch(undefined) so a hand-edited typo disables ordering instead of tripping backup-and-defaults. The sidecar (rather than a codexAccounts row field) is what lets the Desktop login __main__ carry an order at all: it has no row.
Hangs tiering off a single choke point: getEligiblePoolAccounts now returns the highest-priority tier that still has quota headroom, so all three strategies, failover, auto-switch destinations, and the preview path inherit it without their own priority logic. An unordered pool takes the fast path and picks exactly what it picked before. - hasCodexQuotaHeadroom: isActiveUnderFillFirstThreshold renamed and reused as the tier-drain predicate; its two escape hatches (threshold disabled, unknown usage) are what stop an unprimed tier from being skipped. - pickPriorityPreemption: unbound requests move back up when a higher tier regains headroom, runtime-only so an automatic move never rewrites the operator's persisted selection. Bound threads return before it. - Pin lifecycle: PUT /active records the manual choice, which acts as a tier floor until the account is drained, excluded, or routing moves off it. - pickAlternateCodexAccount now passes excludeId into eligibility instead of post-filtering, or a same-request retry could dead-end on a tier whose only healthy member is the account being excluded. - subagent-model-fallback reads the effective active account; the raw field misses every in-memory cursor move (pre-existing under round-robin).
- PUT /api/codex-auth/accounts/priority, deliberately its own route rather than a field on the alias PATCH: aliases are display-only and reject __main__, but the Desktop account is exactly the one an operator wants to order last. null resets to 0 and drops an emptied map. - priority is a required field on every account DTO, like paused, so a dashboard never has to infer the default. - GET /active gains pinned, computed by a core predicate that compares the stored pin against the *effective* active account, so an automatic pick that has already moved past the pin does not read as operator intent.
- `ocx account priority <provider> <id|main> [value|preset|reset]`. Omitting the value is a read, and a read must not rewrite what it reports, so that path never issues a PUT. Values are validated before any HTTP call so a typo never reaches the proxy. - Preset words are first/earlier/normal/later/last rather than high/normal/low: ordering decides *when* the pool reaches an account, not how much traffic it gets, and rank vocabulary implies the latter. - PRIORITY column sits before STATUS, signed, with "-" on the families that have no ordering.
An independent native quota group (Spark) routes on its own ring and must not move the shared active-account cursor — every other automatic pick already guards on isIndependentCodexQuotaScope, and the preemption commit did not.
Every pool card and the main-account card gain a "Selection order" select backed by the new PUT /api/codex-auth/accounts/priority route, plus a muted badge whenever an account sits off the default tier. The presets read as sequence words rather than ranks - First (+2), Earlier (+1), Normal (0), Later (-1), Last (-2) - because the setting orders the pool, it does not grade the accounts. Values set through the API or CLI that fall outside the presets render as a selectable "Custom (n)" peer, so the control never silently rewrites a stored value it cannot name. Manual selection now shows a PINNED badge and explains that the choice holds until the account drains, which is what the new pin actually promises. The switch dialog copy moves to match. Cards are deliberately not re-ordered by priority: the main card renders outside the pool list so any sort would be partial, and the 30s poll would reshuffle rows under the pointer. The priority mutation keeps its own in-flight ref rather than sharing the pause one, so pausing one account does not block re-ordering another.
Adds the `codexAccountPriorities` and pin rows to the configuration reference, the `ocx account priority` verb and PRIORITY column to the CLI reference, and the dashboard control to the web guide, mirrored across ja, ko, ru, and zh-cn. The two structure notes carry the invariants that reviewers need rather than the usage: 08 records that selection order is an ordering boundary and never an eligibility one, so it cannot admit an account that pause, cooldown, health, or reauth already excluded, and cannot re-bind a bound thread. 05 records why this is a route of its own instead of a field on the alias family - ordering is routing metadata, aliases are display-only, and the alias route rejects the `__main__` id that ordering specifically needs.
setAccountPriority copied the pause mutation's handling of activeCodexAccountId, but the two writes are not alike. Pausing the active account reconciles routing, so the response reports a real transition. Re-ordering never does: the route only stores the value and reports the active account as a read. Adopting that read set pendingActiveIdRef, which exists to stop a background load from rolling back an active id the server has not yet committed. Armed against a value this write did not cause, it clears only when a later load happens to see the same id - so an auto-switch landing in that window would leave the dashboard showing a stale active account.
`word in PRIORITY_PRESETS` also matches names inherited from Object.prototype, so `ocx account priority openai main __proto__` resolved to Object.prototype and `constructor` to the Object function. Both slipped past the local validation that exists to keep a typo off the network, and reached the proxy as a body whose priority field JSON.stringify had dropped - a confusing 400 from the server instead of the usage error the user should see. Object.hasOwn restricts the lookup to the five real presets. The reject-before-HTTP test gains the three inherited names.
The no-behaviour-change guard compared an unset config against an all-zero one. Both take the same fast path through selectPriorityTier, so the test only proved the new code agrees with itself - it would have stayed green if tiering had reordered every pick. Recording the three pre-feature sequences as literals makes it a real regression guard, and drops the set-cardinality assertion that was standing in for them.
# Conflicts: # src/config.ts
The read path deliberately degrades codexAccountPriorities to undefined so a hand-edited typo cannot trip the backup-and-defaults repair that would wipe providers. validateConfigCandidate shared that schema, so a write inherited the degrade: `ocx config set codexAccountPriorities.side 200` stripped the whole map, saved the stripped config, and printed success. The two paths are not alike. Degrading on load leaves the raw entries in the file to be repaired by hand; degrading on a write erases every order the user had accumulated. blankHostnameError already draws that line for a field with the same read-time degrade, so selection order follows it - a live caller is told the value is wrong rather than silently losing the rest of the map. The pin field gets the same treatment, and its pattern moves to a named constant now that the schema and the write guard both use it.
A pin and an order are both the operator naming an account to use, but only one of them could win: every PUT /api/codex-auth/active writes a pin, and nothing operator-facing cleared one. So a pin made long before any order existed - an ordinary account switch - outranked every order set afterwards. It blocked preemption and capped each eligibility list at its own tier until that account drained, paused, or 429'd. Under round-robin it was sharper still: pre-feature a manual selection washed out after one pick, and the pin confined the ring to its tier indefinitely. Writing an order now releases the pin, so the newer statement wins and the stale-pin case cannot arise. This also gives the operator a way to clear a pin without immediately setting another, which nothing else offered. Documented as an invariant in structure/08, cross-referenced from 05, and added to the activeCodexAccountId row in all five configuration references.
Two strings still described the pre-feature behaviour of paths this branch changed. The CLI note under `ocx account priority` was copied from `ocx account use` and said the change applies to new Codex sessions. Re-ordering is not session-scoped: preemption moves unbound requests up on the next request, and it is bound threads, not new sessions, that keep their account. `codexAuth.switchBackDesc` kept its pre-pin wording while `switchDesc` was updated, but switching back to Main pins too - PUT /active pins its target unconditionally, including the `__main__` default - and the switch-back modal shows this string. It now carries the same drain-or-reselect promise as the forward switch, in all six locales.
…t ownership Three seams that the reviews flagged as promising more than they delivered. selectPriorityTier took a readonly array and returned string[], which needed a cast and was untrue: it returns the caller's own array in exactly the no-change cases, so a mutating caller would corrupt its input precisely where the sequence must not change. The return is readonly now, and that propagates cleanly through getEligiblePoolAccounts, listEligibleCodexAccountIds, pickRoundRobinAccount, and the fill-first picker - none of which mutated it. isCodexAccountPriorityKey moved from config.ts to account-priority.ts. Which ids may carry an order is domain logic, not schema detail, and the management API needs it too: it must reject a bad key before setCodexAccountPriority persists one as an own property that the load schema would then reject, degrading the whole map. The route now shares the predicate rather than open-coding it. The CLI hard-coded the -100..100 range next to a core parser that already owned it. It now calls parseAccountPriority and builds both the range and the preset list in its error message from the constants, so the two cannot drift.
The cli.md block introduced by "The shipped help surface is:" had drifted from src/cli/help.ts - the usage line and details omitted login, reauth, code, cancel, and reset-credits. The drift predates this branch, but this branch edited that exact line in all five locales to insert `priority`, which re-asserts the block as accurate. Now verified verbatim against the help entry rather than by eye.
getCodexAccountPriority reimplemented codexAccountPriorityLookup's closure body; it delegates now, so the hasOwn gating lives in one place. The CLI's `row.priority ?? 0` was a second default behind fetchCodexRows, which already coalesces. It reuses the shared normalizer instead, so a malformed value from a future payload lands on the default rather than rendering raw. A malformed activeCodexAccountPinned degraded silently while the priorities map next to it warned. Silence reads as the manual selection simply not surviving the restart, so it warns too.
Seven gaps a review named: an unreachable proxy on both the read and the write, the read path's JSON envelope, reading `main`, an unknown id, the explicit `+2` spelling, the trailing-argument fall-through to usage, and that the advisory note goes to stderr so `--json` stdout stays parseable. The note test drives a write because the read path returns before emitting it, which is the intended asymmetry - a read must not advise about a change it did not make.
…e PINNED badge The behaviour suite declared nextPauseResponseGate, wired it into the pause stub, and never used it: only "an in-flight order save does not block a pause" existed. The two mutations hold separate in-flight refs, so each direction needs its own proof - the gate now carries the missing converse. The PINNED badge had no rendered coverage, only a controller flag and literal JSX source-greps that a markup refactor breaks and a logic inversion can survive. The new file mounts the real component and checks the badge and its hint appear on the pinned card and nowhere else, for a pinned pool account and a pinned app login separately, since those are two different card components and a one-sided inversion would otherwise slip through.
…h their siblings `.card` carries no padding: `.card-row` and `.card-sub` each inset themselves, so a card assembled from plain children leaves its title and controls flush against the border while the hints sit 16px in. The selection-order row now uses the same box as the `.card-sub` hints it follows, and the pool-strategy card pads itself and flattens its hints, which is what the auto-switch card already does.
The control disabled every account's select while any one save was in flight, so re-ordering two accounts meant waiting between them. Pause already scopes its busy state by id; match it.
The five orders are declared in three places that cannot import one another. Driving the CLI from the dashboard's list, and comparing the two range definitions directly, turns a silent disagreement about what "First" means into a failure here.
Fills the gaps the slice reviews named: the priority map's reserved-key and degrade behaviour, the tier ceiling returning its tier whole rather than filtering it, fill-first holding and then descending past a pin, and a 429 releasing a pin through the runtime cursor while the persisted active account stays put.
The trigger is a button, which is labelable, so an `id` lets a sibling `<label htmlFor>` name it the way a native select was named. `describedBy` and `title` carry over the hint wiring that `aria-describedby` and `title` gave the native element, so a caller can migrate without losing either.
`select.input { appearance: none }` strips the native arrow and nothing replaces it,
so both pool dropdowns rendered as plain boxes with no sign they open a menu. Move
them to the app's own Select, which supplies the chevron, listbox semantics, and
keyboard navigation that 17 other call sites already use.
The strategy control's wrapping <label> becomes a div: a button nested inside its own
label can take the click twice, and the accessible name now comes from Select's
aria-label. Its tests moved off HTMLSelectElement value mutation to opening the menu
and clicking an option, which is what a user does and also proves the options render.
Unwrapping the <label> to avoid a button nested in its own label left the visible "Rotation strategy" text as a bare span, so clicking it no longer focused the control and the two were only related by position. A sibling label with htmlFor restores both — a button is labelable, and a non-ancestor label cannot double-fire the click. Most visible on the Anthropic pool page, which renders this label.
Four copy defects, all the same class: text that describes the pin or a switch in
terms the routing code does not implement.
**A manual switch does not preserve running threads.** Every surface said "running
threads keep their account". `resetCodexRoutingForManualSelection` calls
`clearThreadAccountMap()` as its first statement, so a switch wipes all thread
affinity and the route reports `appliesImmediately: true`. A running thread's
binding is deleted and it rebinds on its next request; only requests already in
flight keep the account they resolved. Corrected in the `ocx account use` note,
the shipped help block (`src/cli/help.ts`), the five `cli.md` copies that quote
that block verbatim, and both dashboard switch strings.
That clause predates this branch, but the test asserting it is what kept it alive,
so the assertion is now inverted to `not.toContain` — rewording back toward
"running threads keep their account" fails.
**"Next unbound request" was the wrong borrow.** That phrasing is load-bearing for
`account priority`, where it is true precisely because re-ordering never clears
affinity. Applied to `account use` it is only vacuously true — after the wipe every
request is unbound — and it implied affinity lapses rather than being force-cleared.
**The switch modal listed two of three pin-release conditions.** Rather than grow
a third clause, the list is gone: the full set lives in `pinnedHint`, which renders
on the pinned card itself. The modal now states only what the switch does, so there
is no incomplete list to be wrong.
**`ocx account priority` never mentioned releasing the pin.** structure/05 calls
this route the only way to clear a pin without immediately setting another, which
means a write storing the order an account already had still releases it — a silent
side effect of a command that reads as purely declarative. Now stated on stderr and
in all five `cli.md` locales.
Also gives ko and ru the "any account" sense in `pinnedHint` that en, de, ja and zh
already carried; both read as "until you change *this* account's order".
Includes the mutation-proven PRIORITY column fix: the column's sign was unasserted,
so the suite passed with priorityText's `+${n}` branch collapsed to `String(n)`.
The malformed-id 400 was the one rejection branch with no server-side test, and it is the load-bearing one: `isCodexAccountPriorityKey` is what keeps a prototype key out of the priorities map. Neither client can reach it — the CLI validates ids before issuing the PUT, and the dashboard can only name accounts it just listed — so only a direct request exercises it. Added as a test.each table matching the file's idiom for value tables, covering `__proto__`, `prototype`, `constructor`, a space, over-length, empty, a non-string and null, each asserting the 400, the error message, and that nothing was written. The priority-value and request-body branches already had test.each tables ~30 lines below; rather than restate them, this folds in what they were missing: an omitted priority key (which JSON cannot distinguish from explicit undefined, and which is a malformed request rather than the reset that `null` means), the two body shapes `7` and `null`, and exact error-message assertions so a status-only pass cannot hide a regression in validation order. Malformed JSON moves to its own test because it answers a different message than the wrong-type-body cases. No over-the-wire NaN case: JSON.stringify turns NaN into null, which this route legitimately reads as a reset, so a 400 assertion there would assert something unreachable. NaN is covered where it can occur, against parseAccountPriority in tests/codex-pool-rotation.test.ts. Also corrects structure/05, which claimed both `pinned` and `pinnedAccountId` were needed. No surface reads `pinned`; the doc now names the field a surface should render and states the boolean's narrower question.
Three comments described the pin as a tier "floor". `selectPriorityTier` names the variable `ceiling` and skips every tier where `tier > ceiling`, so the pin suppresses higher-ordered accounts rather than guaranteeing the pinned one -- the opposite of what "floor" suggests. One of the three even said "floor" and "capping" in the same sentence. Matters beyond wording: this invariant is what the Go port has to reproduce, and "floor" would send a porter looking for a lower bound that does not exist.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-account Codex selection priorities, persisted pinning, priority-aware routing tiers, management API and CLI commands, dashboard controls, localization, configuration validation, and comprehensive tests and documentation. ChangesCodex account priority and pinning
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf44228dac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `codexAccountPriorities?` | `Record<string,number>` | — | Per-account selection order for the Codex pool: account id → integer from `-100` to `100`, **higher is used earlier**, absent means `0`. This is an ordering boundary, not an eligibility one: selection narrows the already-eligible accounts to the highest tier that still has quota headroom, and `accountPoolStrategy` then picks within that tier. A tier is skipped only when every member is over `autoSwitchThreshold`, cooling down, soft-avoided, paused, or needs reauthentication — unknown quota never drains a tier. Ordering never makes an ineligible account selectable and never re-binds a thread that already has an account. The main `__main__` account participates on equal terms, which is how the Codex Desktop login can be set to drain last. With no entries the pool behaves exactly as before. A malformed map is ignored with a console warning (ordering off, no config repair). Managed by `ocx account priority` and the Codex Auth page. | | ||
| | `codexAccountNamespaces?` | `Record<string,string>` | — | Optional public model-selector namespace → stored Codex account target map. This foundation layer validates and persists the map but does not add picker rows or change routing. | | ||
| | `activeCodexAccountId?` | `string` | — | Manually selected Pool account. Selection clears existing thread affinity and applies to the next request; in-flight requests keep their captured account. | | ||
| | `activeCodexAccountId?` | `string` | — | Manually selected Pool account. Selection clears existing thread affinity and applies to the next request; in-flight requests keep their captured account. Choosing an account by hand also pins its tier ceiling (recorded in `activeCodexAccountPinned`): a higher `codexAccountPriorities` tier cannot preempt it until the pin is released by drain, exclusion, deletion, or an explicit failover/promotion away. Ordinary round-robin movement inside the capped tier does not release it. Writing any `codexAccountPriorities` entry also releases the pin, so a pin made before an order existed cannot outrank one set afterwards. `GET /api/codex-auth/active` reports both whether the effective account is pinned (`pinned`) and the account carrying the ceiling (`pinnedAccountId`). | |
There was a problem hiding this comment.
When a user runs the documented offline command ocx config set codexAccountPriorities '{"a":1}', handleConfigCommand validates and saves the new map without deleting activeCodexAccountPinned. After restart, selectPriorityTier therefore continues to cap selection at the old pinned tier, so the new higher order may have no effect despite this line claiming that any priorities write releases the pin. Either clear the pin in this generic config mutation path or limit the documentation claim to writes through ocx account priority and its management route.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
Comment on lines
+310
to
+312
| const setAccountPriority = useCallback(async (id: string, priority: number | null) => { | ||
| if (priorityMutationRef.current) return { ok: false, reason: "busy" } as const; | ||
| priorityMutationRef.current = { accountId: id }; |
There was a problem hiding this comment.
If an operator confirms “use this account” while a priority request is still in flight, the two independent mutation gates allow both requests even though they make opposite changes to the same pin: the active-account PUT sets it while this handler unconditionally clears it. Because each response updates activePinnedId without a shared revision, out-of-order responses can leave the dashboard showing the inverse of the server’s final pin state whenever the background reconciliation fails or is delayed. Gate priority writes against switchingRef (and disable the switch controls during them), or reconcile both operations through one serialized mutation/revision path.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
Comment on lines
+1549
to
+1553
| if ( | ||
| !isIndependentCodexQuotaScope(quotaScope) | ||
| && getEffectiveActiveCodexAccountId(config) === accountId | ||
| ) { | ||
| applyFailureFailover(config, accountId, now); |
There was a problem hiding this comment.
With the quota strategy, if a Spark request reaches upstreamFailoverThreshold, this guard prevents the immediate shared promotion but leaves the account-wide failure streak intact. Once the 30-second soft avoid expires but before the five-minute failure window closes, the next Spark resolution considers the account selectable and then calls the unscoped applyFailureFailover at line 1312, which promotes and persists a different shared active account. Thus the independent scope still changes shared routing state, only later than the added test observes; pass the quota scope through the failover step and suppress shared promotion there as well.
Useful? React with 👍 / 👎.
Suppressing the shared promotion at the moment of the failure only covered one of the two places it happens. The failure streak lives for five minutes but the soft avoid clears in thirty seconds, so there is a window where the account is selectable again while shouldFailover still trips. A scoped resolve reaches applyFailureFailover in that window, and it took no quotaScope at all -- it promoted and persisted a new shared active account, and picked its alternate from shared-scope eligibility rather than the scope's own. Thread the scope through and guard the promotion. The scope still routes away from the failing account, which is its own decision to make; it just no longer moves the cursor every other scope resolves from. Found by CodeRabbit on lidge-jun#715.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/guides/web-dashboard.md`:
- Around line 104-106: Update the pin lifecycle descriptions in
docs-site/src/content/docs/guides/web-dashboard.md lines 104-106 and
docs-site/src/content/docs/ja/guides/web-dashboard.md lines 84-91 to state that
changing an account’s selection order via an accepted priority update also
clears the pin, alongside account drainage and selecting another account.
In `@docs-site/src/content/docs/reference/cli.md`:
- Around line 274-279: Update the Codex pool timing documentation so it
distinguishes manual account selection via use, which applies only to new
sessions, from priority changes via priority, which apply starting with the next
unbound request. Apply the equivalent clarification in
docs-site/src/content/docs/reference/cli.md lines 274-279,
docs-site/src/content/docs/ja/reference/cli.md lines 214-219, and
docs-site/src/content/docs/ko/reference/cli.md lines 218-223, preserving
accurate localized wording and removing any blanket statement that all pool
switches take effect immediately.
In `@src/cli/account.ts`:
- Line 260: The wording in the account-switch documentation must consistently
describe immediate rebinding semantics. In
docs-site/src/content/docs/ru/reference/cli.md:293-298 and
docs-site/src/content/docs/zh-cn/reference/cli.md:264-268, update the ocx
account use descriptions to state that running threads rebind on their next
request while only in-flight requests retain the captured account, matching
src/cli/account.ts:260. In
docs-site/src/content/docs/zh-cn/guides/web-dashboard.md:78-79, state that
manually selected accounts rebind already-bound threads on their next request.
Make no change to src/cli/account.ts:260; also verify the canonical English CLI
and dashboard pages contain the corrected wording.
In `@structure/05_gui-and-management-api.md`:
- Line 67: Update the OpenAI account mode documentation paragraph to acknowledge
that clearing the active account also releases the pin, as described in the
active-account behavior in 08_openai-provider-tiers.md. Clarify that PUT
/api/codex-auth/accounts/priority is the only operator-facing route for clearing
a pin while changing ordering metadata, rather than the only way to clear a pin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d873e1e-d748-498a-9bd3-a15f7e8f5e80
📒 Files selected for processing (59)
.gitignoreREADME.mddocs-site/src/content/docs/guides/web-dashboard.mddocs-site/src/content/docs/ja/guides/web-dashboard.mddocs-site/src/content/docs/ja/reference/cli.mddocs-site/src/content/docs/ja/reference/configuration.mddocs-site/src/content/docs/ko/guides/web-dashboard.mddocs-site/src/content/docs/ko/reference/cli.mddocs-site/src/content/docs/ko/reference/configuration.mddocs-site/src/content/docs/reference/cli.mddocs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/guides/web-dashboard.mddocs-site/src/content/docs/ru/reference/cli.mddocs-site/src/content/docs/ru/reference/configuration.mddocs-site/src/content/docs/zh-cn/guides/web-dashboard.mddocs-site/src/content/docs/zh-cn/reference/cli.mddocs-site/src/content/docs/zh-cn/reference/configuration.mdgui/src/account-priority.tsgui/src/components/AccountPoolStrategyControls.tsxgui/src/components/AccountPriorityControl.tsxgui/src/components/CodexAccountPool.tsxgui/src/components/CodexPoolStrategySetting.tsxgui/src/components/codex-account-pool-cards.tsxgui/src/components/codex-account-pool-main-card.tsxgui/src/hooks/useCodexAccountPool.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/styles.cssgui/src/ui.tsxgui/tests/account-pool-strategy.test.tsxgui/tests/account-priority.test.tsxgui/tests/codex-account-pool-behaviour.test.tsxgui/tests/codex-account-pool-controller.test.tsgui/tests/codex-account-pool-pinned-badge.test.tsxgui/tests/codex-account-pool-toast-tone.test.tsxpackage.jsonsrc/cli/account-api.tssrc/cli/account-extended.tssrc/cli/account.tssrc/cli/help.tssrc/codex/account-lifecycle.tssrc/codex/account-priority.tssrc/codex/auth-api.tssrc/codex/pool-rotation.tssrc/codex/routing.tssrc/codex/subagent-model-fallback.tssrc/config.tssrc/types.tsstructure/05_gui-and-management-api.mdstructure/08_openai-provider-tiers.mdtests/cli-account.test.tstests/codex-auth-api.test.tstests/codex-pool-rotation.test.tstests/codex-routing.test.tstests/config.test.ts
`ocx config set` is fully generic -- it setPaths any dotted path, validates, and saves -- so writing codexAccountPriorities through it stored the new order and left activeCodexAccountPinned untouched. Nothing self-heals that: releaseDrainedCodexAccountPin only fires on a pin whose account is drained, so a pin on a healthy account kept capping the tier ceiling and the order just written had no visible effect. The documented rule that any priorities write releases the pin was true only through `ocx account priority` and the management route. `import` is deliberately not covered. That file supplies its own pin, so it is a statement rather than a leftover. Found by CodeRabbit on lidge-jun#715.
Both PUTs move the manual pin, in opposite directions -- the active route sets it, the order route clears it -- and each handler now applies its edge optimistically rather than waiting for the reload. They were gated on separate refs, so they could overlap, and response order is not request order: whichever reply landed last won the client's pin state regardless of which write the server processed last. The dashboard could sit on the inverse of the server's pin until some later reload happened to correct it. Cross-gate them in both directions, since either can be the newer statement. A refused mutation returns "busy", which both call sites drop without a toast, so the order control and the switch confirm are also disabled while the other is in flight -- otherwise the gate would only turn a visible race into an invisible no-op. Found by CodeRabbit on lidge-jun#715.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/codex-pool-rotation.test.ts`:
- Around line 923-947: Update the test around resolveCodexAccountForThread to
capture its returned account and assert that the scoped resolution selects the
expected account "a". Keep the existing shared-state assertions, so the test
verifies both that the intended failover path was exercised and that shared
routing remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1379aa66-8d06-4ea0-8fd4-066097e63012
📒 Files selected for processing (11)
gui/src/components/CodexAccountPool.tsxgui/src/components/codex-account-pool-cards.tsxgui/src/components/codex-account-pool-main-card.tsxgui/src/components/codex-account-switch-modal.tsxgui/src/hooks/useCodexAccountPool.tsgui/tests/codex-account-pool-behaviour.test.tsxgui/tests/codex-account-pool-controller.test.tssrc/cli/config-command.tssrc/codex/routing.tstests/cli-headless-parity.test.tstests/codex-pool-rotation.test.ts
A manual Codex switch and a selection-order write have different timings, and five docs surfaces still described the pre-PR guarantee for the first one. `resetCodexRoutingForManualSelection` calls `clearThreadAccountMap`, so a switch moves running threads on their next request; only requests already in flight keep the account they captured. An order write never touches affinity and applies from the next unbound request. - The `ocx account use` subsection in all five `reference/cli.md` locales still said Codex selections "apply only to new sessions; existing threads keep their account". CodeRabbit flagged ru and zh-cn and asked me to double-check the canonical English; English was wrong too, so all five are corrected against `account.ts:260`, which is the wording the command actually prints. - The blanket line in the `account` help block covered both verbs with the switch's timing. Split into two sentences, one per verb, and the switch half now says running threads move rather than only naming in-flight requests. The five cli.md help blocks mirror the block verbatim, so they carry the same split. - `guides/web-dashboard.md` in all five locales carried the same false bound-thread claim -- the file I missed when correcting seven other surfaces -- and listed only two of the three pin-clearing triggers. Both fixed, plus the order bullet now states the from-next-unbound-request timing. - `structure/05` claimed the priority route is the only operator-facing way to clear a pin without selecting another account. A null-id `PUT /active` clears one too; the priority route is the only one that clears it while leaving the selected account in place. The spark-scope regression test asserted only that shared state was untouched, which would also hold if the request never reached `applyFailureFailover`. It now asserts the routed account first: "a" is selectable again by then, so routing away from it is what proves the guarded branch ran.
XertroV
force-pushed
the
feat/priority-levels
branch
from
July 30, 2026 00:11
6b0d7d3 to
f5f474b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/ko/guides/web-dashboard.md`:
- Around line 84-87: Clarify the manual-account-selection wording so
already-bound threads switch to the selected account starting with their next
request, while in-flight requests continue using their captured account. Apply
this wording consistently in
docs-site/src/content/docs/ko/guides/web-dashboard.md lines 84-87,
docs-site/src/content/docs/ru/guides/web-dashboard.md lines 87-91, and
docs-site/src/content/docs/zh-cn/guides/web-dashboard.md lines 78-80.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 97ac0313-c5da-4d25-950e-ab4002380aca
📒 Files selected for processing (13)
docs-site/src/content/docs/guides/web-dashboard.mddocs-site/src/content/docs/ja/guides/web-dashboard.mddocs-site/src/content/docs/ja/reference/cli.mddocs-site/src/content/docs/ko/guides/web-dashboard.mddocs-site/src/content/docs/ko/reference/cli.mddocs-site/src/content/docs/reference/cli.mddocs-site/src/content/docs/ru/guides/web-dashboard.mddocs-site/src/content/docs/ru/reference/cli.mddocs-site/src/content/docs/zh-cn/guides/web-dashboard.mddocs-site/src/content/docs/zh-cn/reference/cli.mdsrc/cli/help.tsstructure/05_gui-and-management-api.mdtests/codex-pool-rotation.test.ts
"Applies immediately, to new and already-bound threads alike" is true of the routing decision but reads as though a request in progress could migrate. The switch clears affinity, so a bound thread picks up the new account on its *next* request; only requests already in flight keep what they captured. That is the distinction the CLI text already draws, and the dashboard guide now draws it too. The review named ko, ru, and zh-cn; English and Japanese shared the imprecision, so all five locales are corrected.
…writes Pausing the pinned account releases the pin too, so the obvious reading of the switch/order cross-gate is that pause was missed. It was not: that edge is conditional on the pin still naming the paused account, so it is a no-op whenever a concurrent write has already moved the pin elsewhere, and the client agrees with the server whichever response lands last. The gated pair state a pin outright, which is what makes response order decide the outcome. Comment only -- worth having at the line, because the safe-looking fix (share pauseMutationRef) would make a pause and an order change on two unrelated accounts reject each other, and because copying the unconditional pattern here would introduce the very race the gate exists to prevent.
XertroV
force-pushed
the
feat/priority-levels
branch
from
July 30, 2026 00:48
2705a34 to
2ebfe7f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs-site/src/content/docs/ru/reference/cli.md (2)
342-347: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify the affinity lifetime in both translated CLI references.
The provider contract allows 401/403 reauthentication failures and 429 cooldown responses to clear affinities or rotate selection before quota exhaustion. These paragraphs should describe “until exhaustion” as normal routing behavior, not an unconditional guarantee.
docs-site/src/content/docs/ru/reference/cli.md#L342-L347: qualifyПотоки, уже привязанные к аккаунту, сохраняют его до исчерпания.docs-site/src/content/docs/zh-cn/reference/cli.md#L306-L309: qualify已绑定账号的 thread 会保留该账号，直到其用尽with the same failure exceptions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs-site/src/content/docs/ru/reference/cli.md` around lines 342 - 347, Qualify the affinity-lifetime statements in both translated CLI references to state that bound threads normally retain their account until quota exhaustion, except when 401/403 reauthentication failures or 429 cooldown responses clear the affinity or rotate selection. Update the corresponding wording in docs-site/src/content/docs/ru/reference/cli.md lines 342-347 and docs-site/src/content/docs/zh-cn/reference/cli.md lines 306-309 with equivalent exceptions.Source: Path instructions
347-347: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the pin-clearing qualifier in both translated CLI references.
Both pages omit that clearing the active account with
nullalso releases the pin, while removing the selected account. State that the priority command is the only way to clear the pin while retaining the current selection.
docs-site/src/content/docs/ru/reference/cli.md#L347-L347: add the “while retaining the selected account” qualifier and mention the null-active API path.docs-site/src/content/docs/zh-cn/reference/cli.md#L309-L309: add the equivalent qualifier and API clarification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs-site/src/content/docs/ru/reference/cli.md` at line 347, Update the CLI account-pinning explanations in docs-site/src/content/docs/ru/reference/cli.md:347-347 and docs-site/src/content/docs/zh-cn/reference/cli.md:309-309 to state that setting the active account to null also releases the pin and removes the selected account; clarify that the priority command is the only way to clear the pin while retaining the currently selected account.Source: Path instructions
gui/src/hooks/useCodexAccountPool.ts (1)
349-352: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear stale active-switch reconciliation before reloading after a priority write.
switchAccountleavespendingActiveIdRefset until a matching/activeread arrives. A priority write can run after the switch response but before that background read completes; if clearing the pin changes the effective account, the nextload()seesserverActiveId !== pending.id, takes the stale-read branch at Lines 168-174, and never clears the pending marker. Subsequent loads then keep the dashboard’sactiveIdstale.Invalidate
pendingActiveIdRefafter the priority write succeeds and beforevoid load(), or version the pending switch separately. Add a regression test covering a switch followed by a priority update before the switch reconciliation completes.As per path instructions, GUI state must stay consistent with management API responses.
Proposed fix
// Unlike pause, re-ordering never reconciles the active account, so the response's // activeCodexAccountId is a read rather than a transition. Adopting it here would // arm pendingActiveIdRef against a value this write did not cause. + pendingActiveIdRef.current = null; void load();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gui/src/hooks/useCodexAccountPool.ts` around lines 349 - 352, Clear the stale pending active-account reconciliation after the priority write succeeds and before calling load() in the reordering flow. Update pendingActiveIdRef so the subsequent load() cannot take the stale-read branch in switchAccount reconciliation; add a regression test covering a switch followed by a priority update before reconciliation completes.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs-site/src/content/docs/ru/reference/cli.md`:
- Around line 342-347: Qualify the affinity-lifetime statements in both
translated CLI references to state that bound threads normally retain their
account until quota exhaustion, except when 401/403 reauthentication failures or
429 cooldown responses clear the affinity or rotate selection. Update the
corresponding wording in docs-site/src/content/docs/ru/reference/cli.md lines
342-347 and docs-site/src/content/docs/zh-cn/reference/cli.md lines 306-309 with
equivalent exceptions.
- Line 347: Update the CLI account-pinning explanations in
docs-site/src/content/docs/ru/reference/cli.md:347-347 and
docs-site/src/content/docs/zh-cn/reference/cli.md:309-309 to state that setting
the active account to null also releases the pin and removes the selected
account; clarify that the priority command is the only way to clear the pin
while retaining the currently selected account.
In `@gui/src/hooks/useCodexAccountPool.ts`:
- Around line 349-352: Clear the stale pending active-account reconciliation
after the priority write succeeds and before calling load() in the reordering
flow. Update pendingActiveIdRef so the subsequent load() cannot take the
stale-read branch in switchAccount reconciliation; add a regression test
covering a switch followed by a priority update before reconciliation completes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ae31cb6d-22f2-4384-954f-0d566439fb56
📒 Files selected for processing (14)
docs-site/src/content/docs/guides/web-dashboard.mddocs-site/src/content/docs/ja/guides/web-dashboard.mddocs-site/src/content/docs/ja/reference/cli.mddocs-site/src/content/docs/ko/guides/web-dashboard.mddocs-site/src/content/docs/ko/reference/cli.mddocs-site/src/content/docs/reference/cli.mddocs-site/src/content/docs/ru/guides/web-dashboard.mddocs-site/src/content/docs/ru/reference/cli.mddocs-site/src/content/docs/zh-cn/guides/web-dashboard.mddocs-site/src/content/docs/zh-cn/reference/cli.mdgui/src/hooks/useCodexAccountPool.tssrc/cli/help.tsstructure/05_gui-and-management-api.mdtests/codex-pool-rotation.test.ts
`switchAccount` arms `pendingActiveIdRef` so a reload already in flight cannot roll the active id back to a value the server had not committed yet. The only thing that retires the marker is an `/active` read that agrees with it; a read that disagrees is treated as stale, which deliberately does NOT clear it. The cross-gate between the switch and the order write does not cover this window: `switchingRef` clears when the switch response is processed, while its reconciliation reload is still in flight. An order write landing in that gap is the newer operator statement -- it releases the pin the switch set, so the account the switch named is no longer the one routing has to agree with -- and the marker stayed armed against it. Every later read then disagreed, so the dashboard's `activeId` was stranded on that account for the rest of the session, because nothing else retires the marker. Clearing it before the order write's reload is safe: `loadGenerationRef` already discards a response from any generation but the latest, and this write starts a newer one, so the switch's own reload cannot win the race afterwards. The regression test fails without the fix (1 of 26) and asserts the marker is gone rather than merely satisfied once, by loading again afterwards. Two docs corrections from the same review, each verified against routing and each wider than reported -- the English pages carried both: - "Threads already bound to an account keep it until that account is drained" is too absolute. `recordCodexUpstreamOutcome` clears that thread's affinity earlier on a 401/403 reauth quarantine, on a 429 cooldown in either scope, and once a transient failure streak trips the failover threshold. - "the only way to clear a pin without immediately setting another" is the overclaim already fixed in the maintainer invariant: a null-id active PUT clears one too. It is the only way to clear a pin while KEEPING the selected account.
Owner
|
LAND-AFTER #671 — the two are complementary, and that determines the order. First the thing worth clearing up, because on the surface this looks like it competes with #671 for the same subsystem. It does not. Your priority tiers filter accounts that are already eligible ( That makes the order: #671 lands first after its security review, then this rebases onto it. During the rebase, keep the invariant that a The design reads well. Defaulting to a flat tier at zero means existing deployments see no behaviour change until someone opts in. Degrading malformed preference data instead of rejecting the whole config ( Tests are behavioural, not decorative: tier invariants, fallback, pin ceiling, drained tiers, quota reset, pin expiry, failover, quota-scope isolation. The conflicts are the real work. Nine files, all GUI and GUI tests, because Note the Because nine files conflict, the green result on this branch no longer proves compatibility with current Selection determines which stored credential goes upstream, so this also needs explicit security review under What happens next: wait for #671, rebase onto it preserving both the exact-selector bypass and the current progressive-loading UX, add the combined regression, then request security review. |
Open
10 tasks
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Selection order for the Codex account pool
The Codex pool has no way to say "drain these accounts before that one". Today
the synthetic
__main__Codex Desktop login is actually biased first — it isunshifted to the head of the eligible list — which is the opposite of what most
multi-subscription setups want, since the Desktop login is usually the one worth
preserving.
This adds a per-account selection order: an integer from
-100to100,higher used earlier, absent meaning
0.How it works
One pure function,
selectPriorityTier, narrows the already-eligibility-filteredaccount list to the highest tier that still holds an account with quota headroom.
It is inserted at a single choke point,
getEligiblePoolAccounts, so all threerotation strategies (
quota,round-robin,fill-first), failover,alternate-picking, and the routing previews inherit tiering from that one hook
rather than each growing their own copy of the rule.
Selection order is an ordering boundary and never an eligibility one. It
cannot admit an account that pause, cooldown, health, or reauth already excluded,
and it never re-binds a thread that already has an account — an affinity-held
thread still moves only when its own account crosses
autoSwitchThreshold. Atier drains only when every member is over the threshold, cooling down,
soft-avoided, paused, or needs reauth; unknown quota never drains a tier, so
an unprimed account is not skipped.
With no entries configured — or with every account on the same tier — a fast path
returns the input list unchanged, so the pick sequence is identical to today's.
Two further pieces:
weekly reset, say), new sessions move back up immediately. This is a
runtime-only cursor, so there is no disk write on the request path, and it is
skipped for independent quota scopes so a
spark-scope request can never movethe shared active cursor.
activeCodexAccountPinnedand suppresses upward preemption until the accountdrains or routing otherwise moves off it. This is what makes the dashboard's
"use this one now" actually stick under round-robin and fill-first.
Two pre-existing bugs fixed along the way
Both are inert today and were found while tracing the new paths:
pickAlternateCodexAccountpost-filteredexcludeIdafter eligibility. Undertiering, a same-request retry could dead-end when the excluded account was the
sole healthy member of the top tier. It now passes
excludeIdinto eligibilityso the tier walk sees the exclusion.
subagent-model-fallbackreadconfig.activeCodexAccountIddirectly instead ofthe effective active account, so it missed the round-robin cursor entirely.
Two behaviour changes to be aware of. Both sit outside the "byte-identical
when unconfigured" invariant that governs the rest of the change, so they are
called out rather than buried.
The
subagent-model-fallbackfix above is the first: subagent-fallback quotadecisions now see the runtime cursor, so round-robin and fill-first users get
different (correct) answers where they previously got the persisted account's. It
is required for preemption's runtime-only cursor to be visible at all, and it
reads as a genuine latent-bug fix.
The second is quota-scope isolation on the failure paths. A
spark-scope requestthat took a 429 or tripped the transient-failover streak used to call
notePoolRotationFailureagainst the shared rotation ring and promote a newshared active account. The same function already refuses this 50 lines earlier for
the native scoped-quota path — "Spark remains isolated so a same-account
Terra/Luna combo fallback can run" — so the two branches simply disagreed. They
now agree. The dropped failure note is not redirected onto the scope's own ring,
matching that existing branch: the account is in cooldown either way, so
eligibility filtering removes it, and a stale ring weight self-corrects.
Four defects found reviewing the pin lifecycle
All four were found and fixed during the final review of this branch, each
confirmed with a probe before the fix and each with a regression test that fails
without it.
A no-op order pick released the pin. The app's
SelectcallsonChangefor the clicked option whether or not it was already selected, and commits the
highlighted option on Tab-out of an open menu. The order handler PUT
unconditionally, and the route releases the pin on every accepted write. So
opening the selection-order menu and clicking the value already set — or just
tabbing out of it — dropped the pin and reported success. The guard goes at the
widget (
if (priority === account.priority) return;, matching the two existingstrategy controls); the route stays unconditional on purpose, since an explicit
CLI write is a statement of intent even when the value matches.
A cleared selection left an invisible pin.
PUT /api/codex-auth/activewith
{accountId: null}clears the active account but was pinning the__main__fallback that the same handler uses for its paused check and routingreset. That produced a pin no effective active account matches:
pinnedcompares the two and so reported
false, whileselectPriorityTierstillhonoured the stored pin as a tier ceiling. With main ordered Last, the eligible
pool collapsed from the pool accounts onto
__main__alone — the exact inverseof ordering main last, with no surface that showed or cleared it. A null id
states no selection, so it now releases the pin.
The write-path guard coerced before testing.
codexAccountPrioritiesErrortested the pin as
CODEX_ACCOUNT_PIN_PATTERN.test(String(pin)), andString(123)matches the id pattern. A non-string pin passed the guard and wasthen dropped by the schema's
.catch(undefined)while the write reportedsuccess — the silent degrade that guard exists to prevent.
The PINNED badge could vanish while the pin still bound. The pin is a tier
ceiling, not a selection: inside the capped tier the strategy cursor still
moves. So under round-robin or fill-first,
pinnedgoes false the moment asame-tier sibling is served, even though the pin is still suppressing every
higher tier — leaving no indication of why a First-order account sits idle.
GET /api/codex-auth/activenow also reportspinnedAccountId(pinnedisunchanged), and the dashboard badges that account's own card rather than
whichever card routing landed on. The GUI controller tracks the id only; the
boolean answers a question no surface here asks.
Three more from a second review pass
after the thread-affinity branch, so it was reached only by unbound requests. A
workload of nothing but bound threads never retired a drained pin, and once the
pinned account's quota reset the pin was live again — capping the pool at that
account's tier and suppressing every higher-ordered account, indefinitely. The
release now runs before the affinity lookup. It cannot rebind the thread (the
affinity branch below still returns that thread's own account), and it stays a
single config write per pin lifetime.
behaviour-change note above.
state only from the follow-up
GET /active, so a slow or failed reconciliationcould leave the badge on the account the operator had just moved away from. The
accepted switch, order write, pause, and bulk-pause responses now each apply the
pin lifecycle edge they are known to have caused, with the reload as
reconciliation rather than as the source of truth.
Also from that pass: a malformed selection-order map or pin now surfaces in config
diagnostics as a warning, not only on the console, so
ocx doctorand thedashboard show it; and the pin's ceiling semantics were corrected in the
maintainer invariant and all five documentation locales, which still described
same-tier rotation as releasing the pin.
Three more from the Codex reviewer on this PR
Each verified against the code before fixing, and each has a test confirmed to fail
without its fix.
ocx config setleft a stale pin. That command is fully generic — it writesany dotted path, validates, saves — so writing
codexAccountPrioritiesthroughit stored the order and left
activeCodexAccountPinnedalone. Nothing self-healsit, because the pin's garbage collector only fires on a drained pin: a pin on a
healthy account kept capping the tier ceiling and the order just written had no
visible effect. The documented "any priorities write releases the pin" rule held
only for
ocx account priorityand the management route.importis deliberatelystill exempt — that file supplies its own pin, so it is a statement rather than a
leftover.
promotion at the moment of the failure covered only one of the two places it
happens. The failure streak lives five minutes; the soft avoid clears in thirty
seconds. In between, the account is selectable again while
shouldFailoverstilltrips, and a scoped resolve reaches
applyFailureFailover— which took noquotaScopeat all, so it promoted and persisted a new shared active account andpicked its alternate from shared-scope eligibility. The scope now still routes
away from the failing account, which is its own decision, without moving the
cursor every other scope resolves from.
directions, and each handler applies its edge optimistically. They were gated on
separate refs, and response order is not request order, so whichever reply landed
last won the client's pin state regardless of which write the server processed
last. They are now cross-gated in both directions; because a refused mutation
returns
busyand both call sites drop that without a toast, the order controland the switch confirm are also disabled while the other is in flight — otherwise
the gate would only convert a visible race into an invisible no-op. Pausing the
pinned account also releases the pin, but that path is deliberately left ungated
and now says so: its edge is conditional on the pin still naming that account,
which makes it order-robust. The two gated writes state a pin outright, which is
exactly what makes response order matter.
And a docs round from CodeRabbit, which found a fifth copy of a claim I had already fixed
A manual switch and an order write have different timings, and one blanket sentence
in the
accounthelp block asserted the switch's timing for both. Splitting itturned up the same false bound-thread claim I corrected in seven surfaces earlier —
resetCodexRoutingForManualSelectioncallsclearThreadAccountMap, so a switchmoves running threads on their next request — still sitting in two more places:
ocx account usesubsection of all fivereference/cli.mdlocales saidCodex selections "apply only to new sessions; existing threads keep their
account". The review flagged ru and zh-cn and asked me to check the canonical
English; English was wrong too. All five now mirror what the command prints.
guides/web-dashboard.mdin all five locales carried it as well, and listed onlytwo of the three pin-clearing triggers. Both fixed, and the order bullet now
states its own from-next-unbound-request timing.
way to clear a pin without selecting another account; a null-id
PUT /activeclears one too. The route is the only one that clears it while keeping the
selected account.
The spark-scope regression test above asserted only that shared state was
untouched, which would also hold if the request never reached the guarded branch.
It now asserts the routed account first: the failing account is selectable again by
then, so routing away from it is what proves the branch ran.
A third round, which found one the cross-gate did not cover
switchAccountarms amarker so a reload already in flight cannot roll the active id back to a value the
server had not committed yet, and the only thing that retires it is an
/activeread that agrees with it — a disagreeing read is treated as stale and
deliberately leaves it armed. The cross-gate above does not cover this window:
switchingRefclears when the switch response is processed, while its ownreconciliation reload is still in flight. An order write landing in that gap is the
newer statement — it releases the pin the switch set, so the account the switch
named is no longer the one routing has to agree with — and the marker stayed armed
against it. Every later read then disagreed, stranding the dashboard's active
account for the rest of the session, because nothing else retires the marker.
Clearing it before that write's reload is safe: the load generation counter already
discards responses from superseded generations.
recordCodexUpstreamOutcomeand both widerthan reported — the English pages carried them too. "Threads already bound to an
account keep it until that account is drained" is too absolute: a 401/403 reauth
quarantine, a 429 cooldown in either scope, and a tripped transient-failure streak
each release the binding earlier. And "the only way to clear a pin without
immediately setting another" is the overclaim already corrected in the maintainer
invariant — a null-id active PUT clears one too, so the priority write is the only
way to clear a pin while keeping the selected account.
Surfaces
PUT /api/codex-auth/accounts/priority— accepts__main__(which is why thisis not folded into the alias route, which rejects it),
nullresets to thedefault.
GET /api/codex-auth/activenow reportspinnedandpinnedAccountId.with presets that read as sequence words — First (+2), Earlier (+1), Normal (0),
Later (−1), Last (−2) — plus a PINNED badge. Cards are deliberately not
re-ordered; the main card renders outside the pool list, so a sort would be
partial, and the 30s poll would reshuffle rows under the pointer.
ocx account priority <provider> <id|main> [<value|first|earlier|normal|later|last|reset>]and a PRIORITY column in
ocx account list. Omitting the value reads thecurrent setting without issuing a write.
codexAccountPrioritiesdegrades to off with a console warning if themap is malformed, rather than tripping the backup-and-defaults repair path —
this is a preference, not a safety control, and a hand-edit typo must not be
able to wipe the provider list. The write path still rejects a malformed map,
so a bad
PUTcannot silently erase existing entries.One adjacent fix, outside the feature
select.input { appearance: none }strips the native dropdown arrow and nothingreplaces it, so every native
<select>in the dashboard renders as a plain boxwith no sign it opens a menu. Rather than add a fourteenth instance of that, both
Codex pool dropdowns now use the app's own
Select(gui/src/ui.tsx), whichalready supplies the chevron, listbox semantics, and keyboard navigation at 17
other call sites.
Selectgainedid,describedBy, andtitleprops so acaller can migrate without losing label association or its hint.
This touches
AccountPoolStrategyControls, which the Anthropic pool page alsorenders — that page gets the same improvement. The remaining native selects
elsewhere in the app are untouched and still have no arrow; that is a
pre-existing, app-wide issue worth its own change.
One layout note on that shared control. Going from a full-width
.inputto acontent-width pill needed a compensating inset on the Codex strategy card, whose
sibling rows are
.card-row(padding: 14px 16px). The Anthropic card gets noequivalent rule deliberately: it renders the control as a direct
.cardchildnext to its own threshold field, which has no horizontal inset either, so both
sit flush and the pill aligns with its sibling's left edge. Insetting only the
strategy block there would misalign it against the threshold. The flush/inset mix
on that card is pre-existing and out of scope here.
Notes for the merging maintainer
No Go port obligation applies:
22309149retired thedev2-goline and theneeds-go-portlabel while this branch was in review. An earlier revision of thisdescription named a port surface under
go/and a carry list — disregard it.Nothing here touches
go/.Rotation across multiple accounts does not protect against provider enforcement;
this changes the order of an existing behaviour rather than introducing one.
Verification
bun run typecheck,bun run privacy:scan,tsc -bfor the GUI, the GUI suite(417 pass),
bun run lint:gui,bun run lint:i18n,bun run build:gui, andReact Doctor are all green. Docs updated in all five locales, GUI strings in all
six.
bun run prepushrun serially against currentdev(merged in ascf44228d):6145 pass, 1 skip, 4 fail. The same failures predate that merge and the review
fixes, so neither introduced any of them. The fifth listed below passed on the
latest run — it is a timeout, not a defect. Every one is accounted for and none is
in this branch's diff:
combo management API > PUT renames atomically…combo management API > PUT rename migrates canonical references…provider outbound GET transport > proxy mode reaches one real proxy…(a 15stimeout)
Those three reproduce by name, with matching timings, on a clean
upstream/devworktree (
c0ad57ad).provider management validation > … accepts only canonical OpenAI option seeds— a per-test 60s timeout exceeded under load. Passes alone: 35/35 in 125s. The
test needs most of that budget even unloaded, so it has almost no headroom.
Codex autostart shim > Unix shim exports persisted service API token…— failsonly when a real
OPENCODEX_API_AUTH_TOKENis present in the environment. Thetest deletes the variable from
process.envbefore spawning the shim, but itsurvives into the child, so the shim's
[ -z "$OPENCODEX_API_AUTH_TOKEN" ]guard is false and it never reads the token file the test wrote. With the
variable unset it passes 3/3. Both the test and
src/codex/shim.tsarebyte-identical to
upstream/dev, andsrc/lib/service-secrets.tsandconfig-dir resolution are untouched here.
Two notes for anyone re-running this suite.
It is load-sensitive: the same commit produced 3, 5, 11, and 17 failures across
runs depending on what else was executing, the extras being port-binding,
proxy-startup, and process-teardown suites (
ports,provider API key pool,ocx launcher graceful shutdown) whose timeouts stretch past their limits undercontention. Only a serial run is meaningful.
Separately,
Codex autostart shim > multi-wrapper restore rolls back when a later sibling fingerprint changesis independently flaky — it failed 2 of 3 isolatedruns on this branch and did not fail in the serial prepush run at all. It asserts
on an mtime the test rewinds by one second, so it looks like a filesystem
timestamp race. Also byte-identical to
upstream/dev; noted here only so the nextperson does not spend time on it.
Summary by CodeRabbit
ocx account prioritycommand and extended management/API support for reading/updating active pin state and per-account priority.