feat(codex): add account-qualified model namespaces#426
Conversation
📝 WalkthroughWalkthroughAdds opt-in Codex account-qualified model namespaces such as ChangesCodex account namespace foundation
Catalog and routing
Settings and documentation
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: 374313200f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/catalog/sync.ts (1)
87-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExclusion-reason mismatch for bare model IDs when namespace clones fall outside the display window.
effectiveSubagentRoster's advertised/ordered.somecheck (Line 116, Line 122) now uses the newconfiguredSubagentModelMatchesEntry, which treats a bare configured slug as matching any of its account-bound clones. But the fallback lookup for exclusion diagnostics still uses the oldconfiguredCatalogEntry(Line 123), which only does direct/slugsEquivalentmatching — it will resolve a bare configured id straight to the bare native entry.When
codexAccountNamespacesis enabled, that bare native entry hasvisibility: "hide"(set inbuildCatalogEntries, Lines 269-271). So if a model's account-bound clones exist but none made the top-MAX_SPAWN_AGENT_MODEL_OVERRIDESorderedslice (easily reachable once several native models × several namespaces produce more than 5 clones), the exclusion reason reported is"picker_hidden"(Line 127) instead of the more accurate"outside_display_limit"— even though the model is genuinely visible via its namespaced variants, just not within the priority window. This is untested:tests/multi-agent-compat.test.ts(Lines 161-176) only covers the case where a clone is inside the window.🩹 Suggested direction
const excluded = configured.flatMap((model): SubagentRosterExclusion[] => { if (ordered.some(({ entry }) => configuredSubagentModelMatchesEntry(model, entry))) return []; - const entry = configuredCatalogEntry(entries, model); + const entry = entries.find(candidate => configuredSubagentModelMatchesEntry(model, candidate)) + ?? configuredCatalogEntry(entries, model); if (!entry) return [{ configured: model, reason: "missing_catalog_entry" }];Based on the newly-added test coverage in
tests/multi-agent-compat.test.tsonly exercising the in-window case, this gap would benefit from a companion regression test simulating >5 competing clones.🤖 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 `@src/codex/catalog/sync.ts` around lines 87 - 138, Update effectiveSubagentRoster and its fallback catalog lookup so bare configured model IDs resolve against account-bound namespace clones using the same configuredSubagentModelMatchesEntry semantics as the advertised/ordered checks. When matching clones exist but fall outside MAX_SPAWN_AGENT_MODEL_OVERRIDES, report outside_display_limit rather than picker_hidden. Add a regression test in the multi-agent compatibility coverage with more than five competing clones.
🤖 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/codex-app-models.md`:
- Around line 27-54: The translated codex-app-models pages in ja, ko, ru, and
zh-cn are missing the English page’s account-specific model behavior. Update
each locale page to include the codexAccountNamespaces configuration,
exact-account model naming and selection, fail-closed behavior for missing or
unavailable credentials, bare-model Pool/Direct behavior, sync/catalog details,
and the corresponding configuration-reference link while preserving the existing
localized structure and translations.
In `@gui/src/pages/CodexAuth.tsx`:
- Around line 20-28: Update loadMode so it validates the fetch response before
applying codexAccountModeState or setNamespacesEnabled. Treat non-OK HTTP
responses as errors, preserving null or the last confirmed namespace state
instead of interpreting missing codexAccountNamespacesEnabled as disabled;
retain the existing namespaceMutationInFlightRef guard for successful responses.
In `@src/codex/auth-api.ts`:
- Around line 482-497: Add focused regression tests in
tests/codex-auth-api.test.ts for the account creation and deletion handlers in
the /api/codex-auth/accounts endpoint. Verify manual-import/OAuth creation
creates the expected namespace and triggers exactly one
refreshAccountNamespaceCatalog call, and deletion removes the namespace and
likewise triggers exactly one refresh. Cover both the POST flow around
appendDefaultCodexAccountNamespace and the DELETE flow around deleteCodexAccount
without changing unrelated /api/settings tests.
In `@src/codex/auth-context.ts`:
- Around line 100-101: Update the auth-context resolution flow so accountId is
validated before the Direct-mode shortcut, preventing Direct mode from bypassing
explicit account selection. Either resolve the requested account before Direct
handling or reject the accountId/Direct combination, preserving fail-closed
isolation; add a regression test covering this combination.
In `@src/config.ts`:
- Around line 623-634: Add regression tests for account-qualified combo aliases
at both affected sites: in src/config.ts lines 623-634, extend loadConfig()
coverage with a configured account namespace and assert a combo alias such as
work/gpt-5.6-sol invalidates the persisted configuration; in
src/server/management/combo-routes.ts lines 127-130, add a PUT /api/combos test
asserting the same alias returns 400 and config.combos remains unchanged.
In `@src/router.ts`:
- Around line 243-264: Update the namespace routing block around
codexAccountNamespaceEntries and routeResult to require the same canonical
OpenAI forward-provider check used for persisted codexAccountMode. Only inject
pool mode and codexAccountId when isCanonicalOpenAiForwardProvider confirms the
configured OPENAI_CODEX_PROVIDER_ID provider; otherwise fall through to normal
routing without Codex pool headers.
In `@structure/08_openai-provider-tiers.md`:
- Around line 67-76: Clarify the overview bullets in
structure/08_openai-provider-tiers.md to distinguish exact qualified account
binding for Responses/compact requests from global Pool/Direct behavior for
standalone image and alpha-search relays that do not carry the selected chat
model. Align the wording with the configuration reference and avoid implying
that all image, search, or vision surfaces preserve the selected account
namespace.
In `@tests/multi-agent-compat.test.ts`:
- Around line 161-176: Add a companion test for effectiveSubagentRoster covering
a bare configured model whose visible account-qualified replacements all fall
outside the top MAX_SPAWN_AGENT_MODEL_OVERRIDES priority window. Assert the
replacements are not advertised and that the excluded entries receive the
correct account-bound equivalence exclusion reason, using the existing
catalogFixture and V2 setup patterns.
---
Outside diff comments:
In `@src/codex/catalog/sync.ts`:
- Around line 87-138: Update effectiveSubagentRoster and its fallback catalog
lookup so bare configured model IDs resolve against account-bound namespace
clones using the same configuredSubagentModelMatchesEntry semantics as the
advertised/ordered checks. When matching clones exist but fall outside
MAX_SPAWN_AGENT_MODEL_OVERRIDES, report outside_display_limit rather than
picker_hidden. Add a regression test in the multi-agent compatibility coverage
with more than five competing clones.
🪄 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: 6356e7a9-04a8-4b65-b641-96e11ba990c7
📒 Files selected for processing (46)
docs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/model-routing.mddocs-site/src/content/docs/reference/configuration.mdgui/src/components/CodexAccountPickerSetting.tsxgui/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/pages/CodexAuth.tsxgui/tests/codex-account-picker-setting.test.tsxsrc/codex/account-lifecycle.tssrc/codex/account-namespace-match.tssrc/codex/account-namespaces.tssrc/codex/auth-api.tssrc/codex/auth-context.tssrc/codex/catalog/metadata.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/codex/routing.tssrc/config.tssrc/router.tssrc/server/auth-cors.tssrc/server/index.tssrc/server/management/combo-routes.tssrc/server/management/config-routes.tssrc/server/management/provider-routes.tssrc/server/responses/compact.tssrc/server/responses/core.tssrc/types.tsstructure/08_openai-provider-tiers.mdtests/claude-models-discovery.test.tstests/codex-auth-api.test.tstests/codex-auth-context.test.tstests/codex-catalog.test.tstests/codex-routing.test.tstests/codex-v2-gate.test.tstests/combo-management-api.test.tstests/config.test.tstests/management-provider-validation.test.tstests/multi-agent-compat.test.tstests/native-model-toggle.test.tstests/router.test.tstests/server-auth.test.tstests/settings-stream-mode.test.ts
🔒 Under maintainer review — detailed feedback incoming@lidge-jun (maintainer) has this PR in an active review pass. Please do not merge or close This PR is not part of the current integration batch. It is held back deliberately — Heads-up on rebasing: No action needed from you until then. Thanks for the contribution and for your patience. Review tracker: |
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 `@gui/src/i18n/ru.ts`:
- Around line 884-889: Update the Codex namespace visibility translations in
gui/src/i18n/ru.ts lines 884-889 and gui/src/i18n/zh.ts lines 602-607: describe
visibility for each configured Codex login rather than only added accounts, and
revise the save confirmation to state that account display settings were updated
rather than implying the selected or primary account changed.
🪄 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: b16b6e3c-a687-4f49-acb2-15709ae3202c
📒 Files selected for processing (7)
gui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/tests/codex-account-picker-setting.test.tsx
chrisae9
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/codex/auth-api.ts (1)
427-434: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not silently discard catalog-refresh failures.
Lines 429-434 convert every
refreshCodexModelCatalogfailure into a successful account mutation. A failed refresh leaves the injected picker catalog stale: new account-qualified models may never appear, and deleted-account selectors can remain visible. Ignore only the expected “catalog not injected” case; otherwise log and enqueue/retry a refresh (or return a recoverable error state to the management client).🤖 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 `@src/codex/auth-api.ts` around lines 427 - 434, Update refreshAccountNamespaceCatalog so it does not silently swallow refreshCodexModelCatalog failures: ignore only the expected missing-catalog-injection case, while logging other failures and enqueueing or retrying a catalog refresh, or returning a recoverable error state to the management client. Preserve successful account persistence and ensure the injected picker catalog is eventually refreshed after account changes.gui/src/pages/CodexAuth.tsx (1)
40-45: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse the in-flight ref to actually serialize saves.
namespaceSavingdoes not update synchronously, so rapid clicks can start multiple PUTs and catalog refreshes before rerender. IncludenamespaceMutationInFlightRef.currentin the guard.Proposed fix
- if (namespaceSaving || namespacesEnabled === null || namespacesEnabled === next) return; + if (namespaceMutationInFlightRef.current || namespaceSaving + || namespacesEnabled === null || namespacesEnabled === next) return;As per path instructions, “GUI state changes stay consistent with the management API responses.”
🤖 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/pages/CodexAuth.tsx` around lines 40 - 45, Update the saveNamespacesEnabled guard to also return when namespaceMutationInFlightRef.current is true, before setting the ref or starting the PUT/refresh flow. Keep the existing namespaceSaving, null-state, and unchanged-value checks so rapid clicks are serialized and GUI state remains consistent with management API responses.Source: Path instructions
src/codex/catalog/sync.ts (1)
567-583: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve routed entries when provider discovery fails with namespaces enabled.
mergeCatalogEntriesForSynctreatsroutedEntries.length === 0as the fetch-failure preservation signal (Line 449), but Line 567 appends generated account-bound native rows before passing them as that parameter. Once namespaces are enabled,goEntriesis non-empty even whengatherRoutedModels()returned no routed models, so existing entries for configured providers are removed instead of preserved during a transient discovery failure.Keep routed-discovery results separate from account-bound entries when calling the merge function, and append account-bound rows only after the preservation decision. Add a regression test with an empty routed fetch plus an existing configured-provider catalog row.
🤖 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 `@src/codex/catalog/sync.ts` around lines 567 - 583, Preserve the distinction between routed discovery results and generated account-bound rows in the sync flow around goEntries and mergeCatalogEntriesForSync. Pass the actual routed discovery result to the merge function so an empty routed fetch triggers configured-provider preservation, then append account-bound entries after that decision. Add a regression test covering an empty routed fetch with an existing configured-provider catalog row.
🤖 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/reference/configuration.md`:
- Around line 55-56: Update the codexAccountNamespaces description in the
configuration reference to say account-qualified native rows replace plain GPT
rows when the namespace map is non-empty, not merely when the setting is
present. Preserve the existing behavior and clarify that an empty/default {} map
does not enable account-qualified rows or hide bare rows.
In `@docs-site/src/content/docs/zh-cn/guides/codex-app-models.md`:
- Around line 26-28: Restore the paragraph flow in the Chinese documentation by
completing the deletion and re-addition behavior text before the “手动配置与 `ocx
sync` 详见[配置参考]” reference link. Remove the interrupted and duplicate reference
sentence, preserving the intended account-binding behavior description and
keeping the translated paragraph readable.
In `@src/server/management/config-routes.ts`:
- Around line 199-210: Update the request-body parsing in the settings route
before the existing field checks: parse the JSON result as unknown, require
isPlainRecord(raw), and return the existing 400 invalid-body response for null
or any non-object body. Only access codexAutoStart, streamMode, and
codexAccountPickerEnabled after this validation, preserving the current
supported-setting and type checks for valid records.
---
Outside diff comments:
In `@gui/src/pages/CodexAuth.tsx`:
- Around line 40-45: Update the saveNamespacesEnabled guard to also return when
namespaceMutationInFlightRef.current is true, before setting the ref or starting
the PUT/refresh flow. Keep the existing namespaceSaving, null-state, and
unchanged-value checks so rapid clicks are serialized and GUI state remains
consistent with management API responses.
In `@src/codex/auth-api.ts`:
- Around line 427-434: Update refreshAccountNamespaceCatalog so it does not
silently swallow refreshCodexModelCatalog failures: ignore only the expected
missing-catalog-injection case, while logging other failures and enqueueing or
retrying a catalog refresh, or returning a recoverable error state to the
management client. Preserve successful account persistence and ensure the
injected picker catalog is eventually refreshed after account changes.
In `@src/codex/catalog/sync.ts`:
- Around line 567-583: Preserve the distinction between routed discovery results
and generated account-bound rows in the sync flow around goEntries and
mergeCatalogEntriesForSync. Pass the actual routed discovery result to the merge
function so an empty routed fetch triggers configured-provider preservation,
then append account-bound entries after that decision. Add a regression test
covering an empty routed fetch with an existing configured-provider catalog row.
🪄 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: 32144e30-0643-4cc7-a46b-5b63ec75f575
📒 Files selected for processing (40)
docs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/model-routing.mddocs-site/src/content/docs/ja/guides/codex-app-models.mddocs-site/src/content/docs/ko/guides/codex-app-models.mddocs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/guides/codex-app-models.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mdgui/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/pages/CodexAuth.tsxgui/tests/codex-account-picker-setting.test.tsxsrc/codex/account-lifecycle.tssrc/codex/account-namespaces.tssrc/codex/auth-api.tssrc/codex/auth-context.tssrc/codex/catalog/sync.tssrc/config.tssrc/router.tssrc/server/auth-cors.tssrc/server/index.tssrc/server/management/config-routes.tssrc/types.tsstructure/08_openai-provider-tiers.mdtests/claude-models-discovery.test.tstests/codex-auth-api.test.tstests/codex-auth-context.test.tstests/codex-catalog.test.tstests/codex-v2-gate.test.tstests/combo-management-api.test.tstests/config.test.tstests/management-provider-validation.test.tstests/multi-agent-compat.test.tstests/native-model-toggle.test.tstests/router.test.tstests/server-auth.test.tstests/settings-stream-mode.test.ts
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/reference/configuration.md (1)
120-123: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the canonical
openairoute for qualified selectors.These lines explain account pinning and the bypassed Pool behaviors, but do not state that selectors such as
main/<model>andside/<model>still route through the canonicalopenaiprovider. Without that clarification, readers may interpret them as generic provider-qualified routes or as selectors subject to Pool/Direct switching.Add that qualified native selectors use
openai, pin the exact Codex account, and never fall through to Pool/Direct selection.As per path instructions, account-qualified native selectors must be documented as canonical
openairoutes with fixed account binding.Proposed wording
These selectors do not change `activeCodexAccountId`. They bypass quota auto-switch, transient- failure failover, affinity rebinding, and unsupported-model retry. If the exact account is missing, cooling down, or needs reauthentication, the request fails instead of using another account. Bare `gpt-*` models keep the configured Pool/Direct behavior. +Qualified native selectors still route through the canonical `openai` provider and pin the request +to the selected Codex account; they never fall through to Pool/Direct account selection.🤖 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/reference/configuration.md` around lines 120 - 123, Update the qualified-selector documentation near the account-pinning behavior to state that selectors such as main/<model> and side/<model> use the canonical openai route, bind to the exact Codex account, and never fall through to Pool/Direct selection. Preserve the existing descriptions of bypassed behaviors and failure handling.Source: Path instructions
src/server/management/config-routes.ts (2)
227-240: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRefresh the catalog when initializing bindings on an already-enabled picker.
pickerWasEnabledis captured beforedefaultCodexAccountNamespaces(config)populates an empty map. If persisted configuration hascodexAccountPickerEnabled: truebut no namespaces, this request adds bindings whilepickerWasEnabled === pickerIsEnabled, sorefreshCodexCatalogBestEffort()is skipped and the picker remains stale until a later sync.Track whether bindings were initialized/changed and include that in the refresh condition. Add a regression case to
tests/settings-stream-mode.test.tsfor an already-enabled picker with an empty namespace map.Proposed fix
+ const namespacesWereEmpty = Object.keys(config.codexAccountNamespaces ?? {}).length === 0; const pickerWasEnabled = codexAccountPickerIsEnabled(config); if (body.codexAccountPickerEnabled === true) { - if (Object.keys(config.codexAccountNamespaces ?? {}).length === 0) { + if (namespacesWereEmpty) { config.codexAccountNamespaces = defaultCodexAccountNamespaces(config); } config.codexAccountPickerEnabled = true; @@ } const pickerIsEnabled = codexAccountPickerIsEnabled(config); + const namespacesWereInitialized = + namespacesWereEmpty && Object.keys(config.codexAccountNamespaces ?? {}).length > 0; saveConfig(config); - if (pickerWasEnabled !== pickerIsEnabled) { + if (pickerWasEnabled !== pickerIsEnabled || namespacesWereInitialized) { await refreshCodexCatalogBestEffort(); }🤖 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 `@src/server/management/config-routes.ts` around lines 227 - 240, Update the picker configuration flow around codexAccountPickerIsEnabled and defaultCodexAccountNamespaces to track whether empty namespace bindings were initialized. Include that binding-change state in the condition that calls refreshCodexCatalogBestEffort, while preserving the existing enabled-state transition behavior. Add a regression case in settings-stream-mode.test.ts covering an already-enabled picker with an empty namespace map.
242-246: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExpose
codexAccountPickerEnabledinGET /api/settings. The PUT payload already returns this field, but the GET DTO still omits it, so clients hydrating settings can seeundefinedand render a stale toggle after the persisted value changes. Add the effective value to the GET response and a regression test for the GET shape intests/settings-stream-mode.test.ts.🤖 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 `@src/server/management/config-routes.ts` around lines 242 - 246, Update the GET settings response in the relevant config-routes handler to include the effective codexAccountPickerEnabled value, matching the PUT response and persisted configuration behavior. Add a regression test in settings-stream-mode.test.ts that verifies the GET /api/settings DTO contains this field and its current value.
🤖 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/reference/configuration.md`:
- Around line 120-123: Update the qualified-selector documentation near the
account-pinning behavior to state that selectors such as main/<model> and
side/<model> use the canonical openai route, bind to the exact Codex account,
and never fall through to Pool/Direct selection. Preserve the existing
descriptions of bypassed behaviors and failure handling.
In `@src/server/management/config-routes.ts`:
- Around line 227-240: Update the picker configuration flow around
codexAccountPickerIsEnabled and defaultCodexAccountNamespaces to track whether
empty namespace bindings were initialized. Include that binding-change state in
the condition that calls refreshCodexCatalogBestEffort, while preserving the
existing enabled-state transition behavior. Add a regression case in
settings-stream-mode.test.ts covering an already-enabled picker with an empty
namespace map.
- Around line 242-246: Update the GET settings response in the relevant
config-routes handler to include the effective codexAccountPickerEnabled value,
matching the PUT response and persisted configuration behavior. Add a regression
test in settings-stream-mode.test.ts that verifies the GET /api/settings DTO
contains this field and its current value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 98cb4229-e2a7-4e27-ac1a-3b296db8c8f1
📒 Files selected for processing (4)
docs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mdsrc/server/management/config-routes.tstests/settings-stream-mode.test.ts
Review deferred — head moved during the passThanks for your patience. This PR was queued for a detailed review, but the head advanced from One finding worth flagging now, since it appears to survive the newer commits: Also note: Because this changes account selection, OAuth-created credential lifecycle, and credential-to-model routing, it remains a C4 authentication boundary requiring the explicit maintainer security review that |
# Conflicts: # src/codex/auth-api.ts # src/codex/auth-context.ts # src/codex/routing.ts # src/router.ts # src/server/responses/compact.ts # src/server/responses/core.ts
|
Updated the branch after merging current The maintainer privacy finding is fixed at the boundary: picker selectors now come from an explicit alias or persisted random I also addressed the remaining review findings:
Validation on the final head: 4,280 tests across 327 files, 0 failures; typecheck, GUI lint/build, privacy scan, docs build, and focused GUI/catalog regressions pass. All currently open inline review threads are resolved. The PR remains draft and the C4 maintainer security-review requirement remains in place. |
|
Thank you for your patience — a review of this was prepared yesterday but not posted because the head moved mid-pass, so this is a full re-audit at Verdict: BLOCKED_SECURITY. The change routes credentials to models at an authentication boundary — an account-qualified selector overrides Direct/Pool selection and injects one stored credential at 1. High — the rebase must preserve current 2. High — the collaboration conflict drops the current fallback guidance contract. 3. Medium — account aliases can become persistent, privacy-sensitive log identifiers. Note that 4. Medium — picker-toggle refresh failures are reported as success. Conflicts against git fetch origin
git rebase bef0dfbed1863ccc1403112546e84abda5e9deb7
# resolve both files by preserving current dev behavior, then layering the namespace changes
git add src/server/responses/collaboration.ts src/server/responses/core.ts
GIT_EDITOR=true git rebase --continue
git push --force-with-leasePlease split this before the security review. At 61 files and
Verification at Full re-audit as part of a maintainer review pass, against |
2 participants
Closes #425.
What this is for
OpenCodex can keep multiple Codex accounts authenticated, but a bare GPT entry does not say which account will serve the conversation. Pool mode may intentionally rotate or fail over, and Direct mode uses the caller/main login.
The missing workflow is: keep several accounts logged in, then explicitly choose one account for a conversation from the model picker without logging out or changing the active Pool account.
This is not a built-in “personal account / work account” feature. Those are possible user labels, nothing more. The feature works with arbitrary account records and user-owned selector names.
Behavior
This is opt-in from Codex Auth → Show each Codex account separately in the model picker.
When it is off:
gpt-5.6-solkeep normal Pool or Direct behavior;When it is on:
main/<model>selects the built-in Codex Desktop/main login;pXXXXXXlabel;Turning the setting off only hides the generated rows. It preserves custom selector mappings and saved qualified selections. Deleting an account hides its rows but retains the fail-closed binding; re-adding the same stored account restores it. Newly supported native GPT models receive the existing account selectors on the next catalog sync.
Qualified models still use the canonical
openaiResponses path, including HTTP/SSE, WebSocket, compact, native reasoning, tools, and service-tier metadata. Bare child/helper models keep Pool/Direct behavior; explicitly qualified helper models keep the exact account binding.Configuration and compatibility
The UI manages these fields:
{ "codexAccountPickerEnabled": true, "codexAccountNamespaces": { "main": "main", "side": "stored-account-id" } }The map keys are public selectors. The values are private local routing targets. Existing non-empty hand-written maps remain an explicit opt-in when the visibility override is omitted, and custom maps are preserved byte-for-byte across off/on toggles.
Fresh and ordinary existing configurations remain unchanged until the user enables the setting. Plain GPT IDs are not removed from routing, so existing threads and saved configuration retain their Pool/Direct semantics.
Account add/delete persists even if an immediate catalog refresh fails. OpenCodex retries once, reports a recoverable pending state, and shows an actionable
ocx syncwarning in both account-management surfaces.Integration work
This branch merges current
devand preserves the completed account lifecycle, router, quota-probe, catalog, and response contracts that landed there.Review fixes include:
Validation
Security review
This changes credential-to-model routing at a C4 authentication boundary. It still requires the explicit maintainer security review described in
MAINTAINERS.mdbefore merge.