feat(codex): add account-qualified catalog entries - #949
Conversation
|
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:
📝 WalkthroughWalkthroughThe change adds account-qualified native Codex models. It filters selectors, generates and reconciles account-bound catalog rows, updates discovery and parsing, applies provider-aware visibility, and scopes multi-agent guidance by account namespace. ChangesAccount-bound native catalog
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ModelDiscovery
participant CatalogSync
participant CatalogStore
participant MultiAgentGuidance
Client->>ModelDiscovery: Request model listings
ModelDiscovery->>CatalogSync: Provide provider state and account selectors
CatalogSync->>CatalogStore: Generate and reconcile account-bound rows
CatalogStore-->>ModelDiscovery: Return visible catalog entries
ModelDiscovery-->>Client: Return Codex and OpenAI model listings
Client->>MultiAgentGuidance: Request guidance with account namespace
MultiAgentGuidance-->>Client: Return namespace-filtered model guidance
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 |
2bf0a71 to
f9bbc72
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@src/codex/catalog/account-models.ts`:
- Around line 15-28: In src/codex/catalog/account-models.ts lines 15-28, ensure
visibleCodexAccountSelectors returns only non-empty keys without “/”; if
configuration validation already guarantees this, document that contract in the
account-models doc comment and make no filtering change. In
src/codex/catalog/account-models.ts lines 54-61, make no direct change because
filtering fixes the propagated values. In src/server/responses/collaboration.ts
lines 270-285, align both guards around codexAccountNamespace to use the same
empty-string predicate.
In `@src/codex/catalog/parsing.ts`:
- Around line 248-251: Add direct regression tests for
applyNativeOpenAiContextOverride covering trusted account-qualified slugs,
malformed account-qualified slugs, and unmarked slugs. Use selector/gpt-5.6-luna
to verify multi_agent_version restoration; do not use gpt-5.5 for that
assertion, and leave existing bare-native and routed-entry tests unchanged.
In `@src/codex/catalog/sync.ts`:
- Around line 542-553: Update the account-slug collision handling around
finalRoutedEntries and alignedAccountBoundEntries to issue a once-only warning
when a freshly gathered routed provider row is removed because its slug matches
a generated account slug, while preserving silent handling for stale disk rows.
Add the warning helper beside warnComboMasqueradeCollisionOnce in aggregation.ts
and invoke it with the colliding slug before filtering.
In `@src/server/index.ts`:
- Around line 621-628: Gate bare native catalog rows with
shouldIncludeNativeOpenAi(config) in the buildCatalogEntries flow, matching the
existing includeAccountRows policy and the on-disk catalog behavior; pass an
empty native-slug collection when native OpenAI is unavailable while preserving
current behavior when enabled. Add a regression assertion in the Claude model
discovery test that no bare gpt- model IDs are returned without a canonical
OpenAI provider.
In `@src/server/responses/collaboration.ts`:
- Around line 255-265: Cache the catalog used by multiAgentGuidanceText so
repeated resolveRoster calls reuse one request-scoped snapshot or valid
path/mtime-cached result instead of re-reading and reparsing it. Apply the cache
through readCatalog or the surrounding guidance flow while preserving the
separate roster projections for subagentEffective and preferredEffective.
In `@tests/claude-models-discovery.test.ts`:
- Around line 200-201: The privacy tests do not assert that stored account
aliases are excluded from serialized discovery output. In
tests/claude-models-discovery.test.ts lines 200-201, add alias-absence
assertions for both catalog and plain using the configured “Private Display
Name”; in tests/codex-catalog-sync-hardening.test.ts lines 294-295, add the
corresponding rows assertion. Keep the existing id and email checks unchanged.
In `@tests/codex-catalog-sync-hardening.test.ts`:
- Around line 252-264: Update the idempotency test around the captured firstRows
and subsequent rows to compare the complete sync snapshots directly, asserting
they are equal before the existing targeted context-window checks. Preserve the
current parity and duplicate assertions while ensuring any ordering, priority,
or display-name drift causes the test to fail.
🪄 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: 3d492bc3-66d1-4449-b8c1-d940a970e47f
📒 Files selected for processing (12)
src/codex/catalog.tssrc/codex/catalog/account-models.tssrc/codex/catalog/metadata.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/server/index.tssrc/server/responses/collaboration.tssrc/server/responses/core.tstests/claude-models-discovery.test.tstests/codex-catalog-sync-hardening.test.tstests/multi-agent-compat.test.tstests/native-model-toggle.test.ts
![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: f9bbc72efc
ℹ️ 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".
| const preferred = injectionModel?.includes("/") | ||
| ? preferredCandidates[0] | ||
| : codexAccountNamespace | ||
| ? preferredCandidates.find(candidate => | ||
| candidate.model.startsWith(`${codexAccountNamespace}/`) | ||
| ) | ||
| : preferredCandidates.length === 1 && !preferredCandidates[0]!.model.includes("/") | ||
| ? preferredCandidates[0] | ||
| : undefined; |
There was a problem hiding this comment.
When a request is routed through an account-qualified native model and injectionModel is configured as the bare native slug, this new projection can resolve a usable preferred such as team/gpt-5.6-sol, but the custom injectionPrompt branch still substitutes the original bare injectionModel via {{model}}. In catalogs with account selectors the bare native rows are hidden, so a custom prompt like Use {{model}} tells the model to spawn an override Codex will reject; pass the projected preferred.model (when present) into the placeholder instead of the raw bare id.
Useful? React with 👍 / 👎.
| : codexAccountNamespace | ||
| ? preferredCandidates.find(candidate => | ||
| candidate.model.startsWith(`${codexAccountNamespace}/`) | ||
| ) |
There was a problem hiding this comment.
When the parent request is account-qualified but injectionModel names a bare non-account catalog entry (for example a combo alias or user-native row like local-fast), this branch now only accepts preferred candidates under the current account namespace. That makes the configured bare injection model disappear from guidance, and with no subagentModels the guidance becomes entirely silent, even though the candidate is in Codex's spawn window; apply the namespace projection only for bare native OpenAI slugs and keep exact bare matches otherwise.
Useful? React with 👍 / 👎.
| const accountBoundEntries = includeNativeOpenAi && accountSelectors.length > 0 | ||
| ? buildCatalogEntries( | ||
| template ? JSON.parse(JSON.stringify(template)) : null, | ||
| nativeOpenAiSlugs(), |
There was a problem hiding this comment.
Building generated account rows from nativeOpenAiSlugs() misses supported native slugs that are currently hidden in the on-disk catalog. If a user disabled gpt-5.5 before adding account selectors, then later re-enables it, this call still won't create team/gpt-5.5, while the merge hides the bare row because account rows exist for other natives; the model remains absent from the picker. Use the merged native set (including hidden supported rows) or the static supported list as the source for account-bound clones.
Useful? React with 👍 / 👎.
| const hasPhysicalComboProvider = Object.hasOwn(config.providers, COMBO_NAMESPACE); | ||
| const goEntries = buildCatalogEntries(template ? JSON.parse(JSON.stringify(template)) : null, [], orderedGoModels, featured, websocketsEnabled(config), multiAgentMode, exactComboSlugs); | ||
| const includeNativeOpenAi = shouldIncludeNativeOpenAi(config); | ||
| const accountSelectors = includeNativeOpenAi ? visibleCodexAccountSelectors(config) : []; |
There was a problem hiding this comment.
With account selectors configured, this enables account-bound rows even when every provider is disabled, such as providers.openai.disabled = true with no other enabled providers, because shouldIncludeNativeOpenAi returns true for the bootstrap case. routeModel still rejects account-qualified models unless the canonical openai provider exists and is enabled, so sync/live discovery can advertise team/gpt-5.5 rows that cannot be served; gate account selectors on an enabled canonical OpenAI provider instead of the bare-native bootstrap predicate.
Useful? React with 👍 / 👎.
f9bbc72 to
2b84389
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/codex/catalog/account-models.ts (1)
8-29: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winConfirm that config validation really rejects empty or slash-containing selector keys.
The doc comment now asserts an upstream contract: "Selector keys have already passed config's nonempty, single-segment namespace validation." This function relies on that contract instead of filtering. The consequence of a broken contract is asymmetric:
src/codex/catalog/sync.tsLine 668 keeps only entries that satisfytrustedAccountBoundNativeCatalogSlug, so a key containing/producesa/b/gpt-5.5and is dropped from the on-disk catalog.accountBoundNativeModelSlugsat Line 55-62 has no such filter, sosrc/server/index.tsLine 669-671 still advertises the same malformed id in/v1/models, andid.slice(id.indexOf("/") + 1)then yieldsb/gpt-5.5, which loses the reasoning ladder.Verify the validation exists on every write path that can set
codexAccountNamespaces(management API, config load/migration, hand-edited config.json). If any path can persist an unvalidated key, filter here instead of documenting the assumption.#!/bin/bash # Find every read/write of codexAccountNamespaces and any key validation for it. rg -nP --type=ts -C4 'codexAccountNamespaces' src | head -200 # Look for namespace/selector key validators (single-segment, nonempty). rg -nP --type=ts -C4 '(namespace|selector)[A-Za-z]*\s*(=|:)?.*(includes\("/"\)|/\^\[|test\()' src/config.ts src/codex | head -120🤖 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/account-models.ts` around lines 8 - 29, Verify that every write path for codexAccountNamespaces, including management APIs, config loading or migration, and hand-edited config handling, rejects empty or slash-containing selector keys. If any path can persist invalid keys, update visibleCodexAccountSelectors to filter selectors to nonempty single-segment values so malformed entries cannot reach accountBoundNativeModelSlugs or /v1/models. Preserve valid main-account and stored-pool-account filtering behavior.
🤖 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 `@src/server/responses/collaboration.ts`:
- Around line 260-302: Update the preferred selection logic in the
response-building flow so the activeAccountNamespace branch first prefers a
matching namespaced candidate, then falls back to the sole bare candidate when
preferredCandidates contains exactly one bare model. Preserve existing behavior
for account-qualified injection models and add a focused regression test in the
multi-agent compatibility tests covering a real namespace with one unrelated
bare injectionModel and asserting the Preferred sub-agent text appears.
In `@tests/multi-agent-compat.test.ts`:
- Around line 205-286: The existing tests lack coverage for selecting a truly
bare preferred candidate when an active codexAccountNamespace is set. Add a
focused test near the current multiAgentGuidanceText cases with one bare
non-account catalog entry and one account-bound family, set
codexAccountNamespace and injectionModel to the bare entry, and assert the
result contains the “Preferred sub-agent” text.
---
Duplicate comments:
In `@src/codex/catalog/account-models.ts`:
- Around line 8-29: Verify that every write path for codexAccountNamespaces,
including management APIs, config loading or migration, and hand-edited config
handling, rejects empty or slash-containing selector keys. If any path can
persist invalid keys, update visibleCodexAccountSelectors to filter selectors to
nonempty single-segment values so malformed entries cannot reach
accountBoundNativeModelSlugs or /v1/models. Preserve valid main-account and
stored-pool-account filtering behavior.
🪄 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: f7bb71a5-6b2a-45dd-909a-26273884bd00
📒 Files selected for processing (14)
src/codex/catalog.tssrc/codex/catalog/account-models.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/metadata.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/server/index.tssrc/server/responses/collaboration.tssrc/server/responses/core.tstests/claude-models-discovery.test.tstests/codex-catalog-sync-hardening.test.tstests/grok-models-effort-list.test.tstests/multi-agent-compat.test.tstests/native-model-toggle.test.ts
2b84389 to
0f24517
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 `@src/codex/catalog/metadata.ts`:
- Around line 211-217: Expand the comment above the accountBound branch in
nativeOpenAiSlugs to document that it intentionally bypasses the visibility
filter: applyNativeVisibility may hide bare native rows when qualified
account-bound rows are active, so retaining trusted account-bound slugs prevents
the next sync from shrinking the native slug set. Keep the existing
documented-additions explanation as well.
🪄 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: e5bab1a1-5a9a-4af3-a4d2-63da0c1c9501
📒 Files selected for processing (14)
src/codex/catalog.tssrc/codex/catalog/account-models.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/metadata.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/server/index.tssrc/server/responses/collaboration.tssrc/server/responses/core.tstests/claude-models-discovery.test.tstests/codex-catalog-sync-hardening.test.tstests/grok-models-effort-list.test.tstests/multi-agent-compat.test.tstests/native-model-toggle.test.ts
2308180 to
18aedec
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/codex/catalog/sync.ts`:
- Around line 661-672: Validate account selectors in
visibleCodexAccountSelectors so empty keys and keys containing “/” are rejected
consistently before catalog generation. Update src/codex/catalog/sync.ts lines
661-672 to rely on that validated list (or emit a single warning before
filtering), and update src/server/index.ts lines 670-676 to derive the metadata
id from the same validated selectors rather than splitting the id at the first
slash.
In `@tests/codex-catalog-sync-hardening.test.ts`:
- Around line 409-445: Update the test around syncCatalogModels to configure
gpt-5.5 as disabled via the supported disabledModels input, then assert the
generated team/gpt-5.5 row is hidden while retaining the existing
account-selector metadata assertion. Keep the bare gpt-5.5 assertion only if it
still represents the intended persisted visibility behavior, and ensure the test
explicitly exercises applyNativeVisibility’s disabled-native handling rather
than relying on the on-disk hide flag.
🪄 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: 616c1b69-9956-478f-a622-cd84b735a40e
📒 Files selected for processing (14)
src/codex/catalog.tssrc/codex/catalog/account-models.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/metadata.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/server/index.tssrc/server/responses/collaboration.tssrc/server/responses/core.tstests/claude-models-discovery.test.tstests/codex-catalog-sync-hardening.test.tstests/grok-models-effort-list.test.tstests/multi-agent-compat.test.tstests/native-model-toggle.test.ts
18aedec to
1d737c2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d737c22b4
ℹ️ 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".
| const { accountBoundNativeModelSlugs, applyNativeVisibility, buildCatalogEntries, disabledNativeSlugs, exactComboCatalogSlugs, loadCatalogTemplate, nativeOpenAiSlugs, nativeReasoningEfforts, nativeDefaultReasoningEffort, orderForSubagents, filterCatalogVisibleModels, shouldIncludeAccountBoundNativeOpenAi, shouldIncludeNativeOpenAi, uniqueCatalogModelsForRawPublicList, visibleCodexAccountSelectors, visibleNativeSlugs, desktopVisibleNativeSlugs } = await import("../codex/catalog"); | ||
| const includeNativeOpenAi = shouldIncludeNativeOpenAi(config); | ||
| const includeAccountBoundNativeOpenAi = shouldIncludeAccountBoundNativeOpenAi(config); | ||
| const nativeSlugs = includeNativeOpenAi ? nativeOpenAiSlugs() : []; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier sync finding is that the live client_version discovery path still seeds account-qualified clones from nativeOpenAiSlugs(), which ignores supported bare rows whose on-disk visibility is hide whenever any other native is visible. In a custom/previously-synced catalog where gpt-5.5 was disabled before account selectors were enabled and is later re-enabled without running sync first, this endpoint never passes gpt-5.5 into buildCatalogEntries, so team/gpt-5.5 remains absent from the Codex picker even though exact routing can serve it; use the hidden-inclusive/static supported native set for account-clone generation here as well.
Useful? React with 👍 / 👎.
1d737c2 to
c6c9e7e
Compare
|
Please put your Pull-Request on Ready for Review, once you are finished. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6c9e7e76e
ℹ️ 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".
| const accountBoundEntries = includeAccountBoundNativeOpenAi && accountSelectors.length > 0 | ||
| ? buildCatalogEntries( | ||
| template ? JSON.parse(JSON.stringify(template)) : null, | ||
| NATIVE_OPENAI_MODELS, | ||
| [], |
There was a problem hiding this comment.
When these account-qualified rows are written into the Codex catalog, a spawned sub-agent can legitimately request a model like team/gpt-5.5. In installs with subagentModelFallback or role fallback enabled, isRoutableFallbackModel in src/codex/subagent-model-fallback.ts rejects any slash prefix that is not a configured/known provider before routeModel gets a chance to resolve codexAccountNamespaces, so the requested account model is treated as stale and skipped or rewritten even though exact routing supports it. Please teach the fallback routability gate about account selectors before advertising these rows.
Useful? React with 👍 / 👎.
| ...(includeAccountBoundNativeOpenAi | ||
| ? accountBoundNativeModelSlugs(config, visibleNatives) | ||
| : []).map(id => |
There was a problem hiding this comment.
The Codex-catalog branch above now uses the hidden-inclusive supported set, but the raw OpenAI /v1/models branch still builds account rows from visibleNatives, which comes from visibility-filtered bare rows. With a custom/cache-backed catalog where gpt-5.5 was hidden before account selectors were added or re-enabled, this list omits both gpt-5.5 and team/gpt-5.5 until a sync creates an account row, even though exact routing can serve team/gpt-5.5; use the same supported/hidden-inclusive native source for the account-qualified raw list.
Useful? React with 👍 / 👎.
| return applyMultiAgentMode( | ||
| applyNativeVisibility(mergedEntries, disabledNative, alignedAccountBoundEntries.length > 0), | ||
| multiAgentMode, |
There was a problem hiding this comment.
When account selectors are active this persists every bare native row with visibility: "hide", but restoreCodexCatalog() has fallback paths that simply remove slash rows and keep the remaining native objects as-is when the pristine backup is missing or lacks a later-added native. In that scenario, running restore removes the account-qualified rows but leaves the bare GPT rows hidden, so the picker stays empty for those models; restore needs to unhide supported natives (respecting disabled natives) or avoid relying on persisted hide flags as the only copy.
Useful? React with 👍 / 👎.
[GD] Verdict: changes-requestedTLDR
Full verdictSemantic propagation
UsefulnessDelivers the layer-3 catalog/discovery behavior for #426/#425: per-selector native rows, stale-row reconciliation independent of routed-provider discovery outages, hidden-native reseeding, privacy-safe selectors, preservation of Codex's five-model spawn window, and account-aware subagent projection. Meaningful, well-tested work. Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
Simplification (for the PR owner)Nothing was edited or pushed (foreign PR). Bounded candidates:
GateNone (not draft, no WIP markers, mergeable) — but merge-ready is blocked by the confirmed bug, the red required CI, and the open bot thread. Bottom lineUseful layer-3 catalog/discovery work with strong hardening and good coverage. One confirmed correctness regression (plain availability loses hidden bare natives and their qualified clones when selectors are active) must be fixed in-PR, required CI must go green, and the open Codex bot thread should be answered. Since this is a foreign PR, the owner applies the fix and the optional simplification candidates; this review changed nothing. |
c6c9e7e to
c2132f0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42f2782c33
ℹ️ 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".
| if (!injectionModel && roster === "" && fallbackGuidance === "") return null; | ||
| if (injectionPrompt) { | ||
| return `<multi_agent_mode>${applyInjectionPlaceholders(injectionPrompt, injectionModel, injectionEffort, roster, fallbackGuidance)}</multi_agent_mode>`; | ||
| return `<multi_agent_mode>${applyInjectionPlaceholders(injectionPrompt, preferred?.model ?? injectionModel, injectionEffort, roster, fallbackGuidance)}</multi_agent_mode>`; |
There was a problem hiding this comment.
When an account-qualified request projects a bare injectionModel, {{model}} now expands to the effective selector-qualified model (for example team/gpt-5.6-sol) instead of the configured value, but the public sub-agent docs and the OcxConfig comment still describe it as the configured injectionModel. Users with custom injectionPrompt templates will see different prompt text without documentation of why; update the affected docs/type comment to describe the effective/projected model behavior.
Useful? React with 👍 / 👎.
| const catalogNativeSlugs = accountSelectors.length > 0 | ||
| ? NATIVE_OPENAI_MODELS | ||
| : nativeSlugs; | ||
| const entries = buildCatalogEntries(loadCatalogTemplate(), catalogNativeSlugs, goOrdered, config.subagentModels, websocketsEnabled(config), maMode as "v1" | "default" | "v2", exactComboCatalogSlugs(config), accountSelectors); |
There was a problem hiding this comment.
Once this advertises selector-qualified models, Codex can spawn a child whose requested primary is team/gpt-5.5; however the response path only calls applySubagentModelFallback when the initial route has no codexAccountId, so an exhausted or paused team account fails auth before the configured role/global fallback chain is considered. If exact account rows are spawnable, thread-spawn fallback needs to run for those primaries too (still treating the fixed account candidate as unavailable when cooled).
Useful? React with 👍 / 👎.
42f2782 to
ee2c541
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4528215b
ℹ️ 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".
| const catalogNativeSlugs = accountSelectors.length > 0 | ||
| ? NATIVE_OPENAI_MODELS | ||
| : nativeSlugs; | ||
| const entries = buildCatalogEntries(loadCatalogTemplate(), catalogNativeSlugs, goOrdered, config.subagentModels, websocketsEnabled(config), maMode as "v1" | "default" | "v2", exactComboCatalogSlugs(config), accountSelectors); |
There was a problem hiding this comment.
Fresh evidence after the prior docs fix is that this path now makes Codex clients receive selector-qualified picker rows and hidden bare native rows, but docs-site/src/content/docs/guides/codex-app-models.md still describes the Codex-login picker ids as bare gpt-5.6-* rows only and says account selection does not change picker ids. Users using the App picker guide will miss that codexAccountNamespaces changes the displayed ids to <selector>/<native-openai-model>; please update that page and its translations alongside the configuration reference.
AGENTS.md reference: AGENTS.md:L224-L225
Useful? React with 👍 / 👎.
cc45282 to
a0d0957
Compare
a0d0957 to
bf7b132
Compare
bf7b132 to
72ed0ed
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf7b132469
ℹ️ 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".
| `spawn_agent` model overrides. Pick up to five bare native ids, account-qualified | ||
| `<selector>/<native-openai-model>` ids, or routed `provider/model` ids through `subagentModels` or | ||
| the dashboard Subagents page. opencodex assigns low catalog priorities in the selected order; when |
There was a problem hiding this comment.
With account selectors configured, this says the dashboard Subagents page can pick exact <selector>/<native-openai-model> ids, but the GET backing that page (src/server/management/agent-settings-routes.ts 497-511) builds available only from bare listCatalogNativeSlugs() plus routed provider slugs, and the GUI filters chosen values against that list before rendering/saving. As a result exact selector rows never appear in the dashboard and existing exact selections are dropped from the UI; either add account-bound slugs to that management API or limit this statement to hand-edited subagentModels.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
| `spawn_agent` model overrides. Pick up to five bare native ids, account-qualified | ||
| `<selector>/<native-openai-model>` ids, or routed `provider/model` ids through `subagentModels` or | ||
| the dashboard Subagents page. opencodex assigns low catalog priorities in the selected order; when |
There was a problem hiding this comment.
With account selectors configured, this says the dashboard Subagents page can pick exact <selector>/<native-openai-model> ids, but the GET backing that page (src/server/management/agent-settings-routes.ts 497-511) builds available only from bare listCatalogNativeSlugs() plus routed provider slugs, and the GUI filters chosen values against that list before rendering/saving. As a result exact selector rows never appear in the dashboard and existing exact selections are dropped from the UI; either add account-bound slugs to that management API or limit this statement to hand-edited subagentModels.
Useful? React with 👍 / 👎.
| - Account-qualified native ids use `<selector>/<native-openai-model>`. Disabling one hides only that | ||
| selector row. |
There was a problem hiding this comment.
This says the dashboard Models page can disable an account-qualified native id, but the page is fed by /api/models, whose row builder only returns bare nativeModelRows(config) plus routed models, and /api/model-visibility validates native targets against that same bare native set. A <selector>/gpt-... row therefore never appears in the Models page and is rejected by the toggle API, so users cannot hide just one selector row there without hand-editing disabledModels; either surface selector rows in the management model APIs or remove the dashboard claim.
Useful? React with 👍 / 👎.
72ed0ed to
3d67eb2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d67eb29a4
ℹ️ 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".
| if (!injectionModel && roster === "" && fallbackGuidance === "") return null; | ||
| if (injectionPrompt) { | ||
| return `<multi_agent_mode>${applyInjectionPlaceholders(injectionPrompt, injectionModel, injectionEffort, roster, fallbackGuidance)}</multi_agent_mode>`; | ||
| return `<multi_agent_mode>${applyInjectionPlaceholders(injectionPrompt, preferred?.model ?? injectionModel, injectionEffort, roster, fallbackGuidance)}</multi_agent_mode>`; |
There was a problem hiding this comment.
When account selectors are active but the parent request is still a bare native route, a bare injectionModel can resolve to multiple selector-qualified spawn candidates, leaving preferred undefined; this fallback then expands {{model}} to the raw bare id. Since the catalog hides bare native rows in that configuration, a custom prompt like Use {{model}} tells the agent to pass a model override that the v2 spawn surface will reject. The remaining case is the bare-parent ambiguity; only substitute a model when a unique/current-route preferred was found, or blank/suppress the placeholder as the built-in branch does.
Useful? React with 👍 / 👎.
93c8284 to
a88b3c7
Compare
2 participants
Summary
dev; this layer adds the catalog, discovery, and subagent projection that make those selectors usable from model pickers./v1/modelsavailability. Missing stored-account targets are not advertised, and stored ids, aliases, and email addresses never enter public model metadata.disabledModelsconsistent: a bare native id hides that model for every selector, while an account-qualified id hides only the matching selector row.{{model}}is blank unless the request resolves a unique/current-route candidate, while explicit routed or account-qualified ids retain their existing raw fallback.Verification
a88b3c70: TypeScript typecheck, privacy scan,git diff --check, and the 216-page documentation build passed.a88b3c70: the combined catalog/discovery/restore, exact-account routing, subagent fallback, and upstream retry integration matrix passed with 371 passed, 2 platform-only skipped, 0 failed, and 5,418 assertions across 15 files.devateb2ceb2f;git range-diffconfirms all nine patches are unchanged, and all nine commits have valid GPG signatures.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I fixed all correct Codex and CodeRabbit findings.
My PR is ready for review.