Skip to content

Respect explicit Codex model provider config#191

Merged
friuns2 merged 1 commit into
mainfrom
codex/fix-issue-190-provider-config
May 24, 2026
Merged

Respect explicit Codex model provider config#191
friuns2 merged 1 commit into
mainfrom
codex/fix-issue-190-provider-config

Conversation

@friuns2
Copy link
Copy Markdown
Owner

@friuns2 friuns2 commented May 23, 2026

Summary

  • Skip the unauthenticated OpenCode Zen fallback when CODEX_HOME/config.toml explicitly sets a top-level model_provider
  • Add regression coverage for explicit custom providers and nested/commented model_provider entries
  • Update provider manual test notes for the custom-provider no-auth startup case

Fixes #190

Verification

  • pnpm exec vitest run src/server/codexAppServerBridge.archive.test.ts src/server/freeMode.test.ts -t "ensureDefaultFreeModeStateForMissingAuthSync|freeMode"
  • pnpm run build

Performance

  • The new config.toml read only happens on the startup/config path when no provider-state file exists and no usable Codex OAuth token is present. No duplicate requests, unbounded fanout, cache invalidation, or large payload risk.

Summary by CodeRabbit

  • Bug Fixes

    • Respect a top-level model_provider in config.toml: do not create the default free-mode state or generate provider state when a provider is explicitly configured; commented, profile-scoped, or string-contained occurrences are ignored so fallback still applies.
  • Tests

    • Added tests covering explicit, commented-out, profile-scoped, and multiline-string appearances of model_provider and verifying no leftover state files.
  • Documentation

    • Updated test instructions to exercise custom-provider startup, expected config/read results, and cleanup steps.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 84da3858-2106-4f5b-95cc-e915b5645484

📥 Commits

Reviewing files that changed from the base of the PR and between f108186 and 1917880.

📒 Files selected for processing (3)
  • src/server/codexAppServerBridge.archive.test.ts
  • src/server/codexAppServerBridge.ts
  • tests/providers-models/android-published-cli-loads-codex-app-server-models-through-local-proxy.md

📝 Walkthrough

Walkthrough

Prevents the unauthenticated OpenCode Zen fallback from overriding explicit model_provider settings in config.toml. Adds TOML parsing to detect top-level provider configuration, updates the free-mode initialization logic to skip synthesis when a provider is configured, includes scenario tests covering both cases, and updates end-to-end test documentation.

Changes

Free-mode fallback respects explicit provider configuration

Layer / File(s) Summary
TOML parsing helpers and fs import
src/server/codexAppServerBridge.ts
Adds statSync import, stripTomlComment to remove # comments while respecting quoted and multiline strings, and hasExplicitCodexModelProviderConfigSync to detect top-level model_provider assignments (memoized by path + mtime/size).
Free-mode initialization with explicit provider check
src/server/codexAppServerBridge.ts
ensureDefaultFreeModeStateForMissingAuthSync now checks explicit model_provider in config.toml only when a default free-mode state would otherwise be created and returns the existing state instead of synthesizing the OpenCode Zen fallback when explicit config exists.
Scenario tests for explicit vs. absent/commented provider config
src/server/codexAppServerBridge.archive.test.ts
Adds tests asserting the default free-mode state is not created when config.toml explicitly sets model_provider (including quoted key), and that the fallback is synthesized when entries are commented out, nested under [profiles.*], or inside multiline TOML strings; tests verify no stray state file is left behind.
End-to-end test scenario with custom-provider configuration
tests/providers-models/android-published-cli-loads-codex-app-server-models-through-local-proxy.md
Updates instructions to prepare a temporary ~/.codex/config.toml with model_provider="azure", restart under that config, add a config/read RPC check for zen-proxy wiring when appropriate, ensure no webui-custom-providers.json is created by no-auth startup, and restore cleanup steps.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A rabbit hops through TOML files with glee,
Stripping comments, guarding quotes with care,
Letting config speak so providers run free,
No Zen sneaks in where users chose to declare,
Hooray for tidy parses everywhere!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main objective: preventing the forced OpenCode Zen fallback when config.toml explicitly sets a model_provider.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #190 by detecting explicit model_provider in config.toml and skipping the OpenCode Zen fallback when present, plus adding regression tests.
Out of Scope Changes check ✅ Passed All changes are directly related to the stated objective: config.toml parsing logic, regression tests, and documentation updates for the custom-provider no-auth startup case.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-issue-190-provider-config

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Respect explicit Codex model provider config and skip Zen fallback

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Respect explicit top-level model_provider in config.toml during startup
• Skip OpenCode Zen fallback when custom provider is configured
• Add TOML comment stripping and top-level config detection logic
• Expand test coverage for explicit and nested provider configurations
• Update manual test documentation with custom-provider verification steps
Diagram
flowchart LR
  startup["Startup: ensureDefaultFreeModeStateForMissingAuthSync"]
  checkAuth{"Has usable Codex auth?"}
  checkDefault{"Should create default state?"}
  checkConfig{"Has explicit model_provider in config.toml?"}
  returnNull["Return null"]
  returnCurrent["Return current state"]
  returnZen["Return OpenCode Zen state"]
  
  startup --> checkAuth
  checkAuth -->|Yes| returnNull
  checkAuth -->|No| checkDefault
  checkDefault -->|No| returnCurrent
  checkDefault -->|Yes| checkConfig
  checkConfig -->|Yes| returnCurrent
  checkConfig -->|No| returnZen

Loading

File Changes

1. src/server/codexAppServerBridge.archive.test.ts 🧪 Tests +43/-0

Add regression tests for explicit provider config

• Add test for explicit model_provider in config.toml preventing OpenCode Zen synthesis
• Add test for commented and nested model_provider keys being ignored as fallback triggers
• Both tests verify that state file is not created when provider is explicitly configured

src/server/codexAppServerBridge.archive.test.ts


2. src/server/codexAppServerBridge.ts 🐞 Bug fix +55/-1

Implement explicit provider config detection logic

• Add stripTomlComment() function to parse TOML lines while respecting quoted strings and escape
 sequences
• Add hasExplicitCodexModelProviderConfigSync() function to detect top-level model_provider in
 config.toml
• Modify ensureDefaultFreeModeStateForMissingAuthSync() to skip OpenCode Zen fallback when
 explicit provider config exists
• Logic only checks top-level table entries, ignoring commented lines and nested table sections

src/server/codexAppServerBridge.ts


3. tests/providers-models/android-published-cli-loads-codex-app-server-models-through-local-proxy.md 📝 Documentation +5/-0

Update manual test documentation for custom provider

• Add prerequisite step for custom-provider test case setup with config.toml and provider state
 cleanup
• Add test steps 9-10 for verifying explicit custom-provider behavior without auth token
• Add expected result documenting that explicit model_provider in config.toml is not overridden
 by fallback
• Add cleanup instruction to restore original config.toml after manual testing

tests/providers-models/android-published-cli-loads-codex-app-server-models-through-local-proxy.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 23, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Grey Divider


Remediation recommended

1. Caches config scan failures 🐞 Bug ☼ Reliability ⭐ New
Description
hasExplicitCodexModelProviderConfigSync() caches value=false even when reading/parsing config.toml
fails, so a temporary read error (e.g., permissions/IO) can cause codexapp to keep treating the
config as having no explicit model_provider until the file’s mtime/size changes. This can
incorrectly re-enable the OpenCode Zen fallback despite an explicit top-level model_provider being
present.
Code

src/server/codexAppServerBridge.ts[R3795-3847]

Evidence
The function caches a false value even when the read/scan path throws, and the cache re-use
condition is based only on mtimeMs and size; this cached boolean is then used to decide whether
to synthesize the Zen fallback.

src/server/codexAppServerBridge.ts[3795-3847]
src/server/codexAppServerBridge.ts[3855-3867]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`hasExplicitCodexModelProviderConfigSync()` caches the computed boolean using only `mtimeMs` and `size`. When `readFileSync()`/scan throws, it sets `value=false` and still writes that into the cache. If the failure was transient (permission/IO) and the file’s `mtimeMs`/`size` do not change, subsequent calls will return the cached `false` without re-reading, potentially re-enabling the Zen fallback incorrectly.

## Issue Context
This boolean feeds `ensureDefaultFreeModeStateForMissingAuthSync()`; a stale `false` can cause `createDefaultOpenCodeZenFreeModeState()` to be used when it should be suppressed due to an explicit top-level `model_provider` in `config.toml`.

## Fix Focus Areas
- src/server/codexAppServerBridge.ts[3795-3847]
- src/server/codexAppServerBridge.ts[3855-3867]

## Suggested fix
- If `readFileSync`/scan throws, **do not update** the cache (or update it with a sentinel that forces a retry next call).
- Alternatively, include `ctimeMs` (and/or `mode`) in the cache key so chmod/metadata changes cause re-scan, but still avoid caching when the content wasn’t successfully read.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Sync config read hotpath 🐞 Bug ➹ Performance
Description
ensureDefaultFreeModeStateForMissingAuthSync() now synchronously reads and scans
CODEX_HOME/config.toml whenever there is no provider-state file and no usable auth, and this
function is invoked from multiple request handlers. Because the Zen fallback does not create a state
file, this can cause repeated readFileSync() calls per request in no-auth/no-state flows, adding
avoidable blocking I/O latency.
Code

src/server/codexAppServerBridge.ts[R3783-3786]

Evidence
The new hasExplicitCodexModelProviderConfigSync() performs readFileSync() of config.toml, and
ensureDefaultFreeModeStateForMissingAuthSync() calls it whenever there is no state file and no
usable auth. Since the Zen fallback returns an in-memory default (no file created) and
ensureDefaultFreeModeStateForMissingAuthSync() is called from request handlers, the sync file read
can repeat per request in no-auth/no-state scenarios.

src/server/codexAppServerBridge.ts[3749-3770]
src/server/codexAppServerBridge.ts[3777-3790]
src/server/codexAppServerBridge.ts[5320-5335]
src/server/codexAppServerBridge.ts[6439-6492]
src/server/freeMode.ts[201-206]
src/server/codexAppServerBridge.archive.test.ts[418-431]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureDefaultFreeModeStateForMissingAuthSync()` now calls `hasExplicitCodexModelProviderConfigSync()`, which does a synchronous `readFileSync()`+scan of `config.toml`. Since `ensureDefaultFreeModeStateForMissingAuthSync()` is used in request handlers and the Zen fallback intentionally does not persist a state file, this check can run repeatedly (and synchronously) on hot paths.
## Issue Context
- `shouldCreateDefaultFreeModeStateForMissingAuth()` stays `true` as long as the state file is absent and there is no usable auth.
- The Zen fallback path returns an in-memory state and does not write a file, so the “missing state file” condition persists.
- The bridge calls `ensureDefaultFreeModeStateForMissingAuthSync()` in startup config building and in multiple HTTP routes.
## Fix Focus Areas
- Cache the computed boolean result of `hasExplicitCodexModelProviderConfigSync()` (and/or cache the file-missing case) for the process lifetime, or cache with lightweight invalidation (e.g., `statSync` mtime check).
- Ensure caching is only used for the `shouldCreateDefault` path (so existing state/auth flows remain unchanged).
### References
- src/server/codexAppServerBridge.ts[3720-3790]
- src/server/codexAppServerBridge.ts[5320-5335]
- src/server/codexAppServerBridge.ts[6439-6492]
- src/server/freeMode.ts[201-206]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 1917880

Results up to commit f108186


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Remediation recommended
1. Sync config read hotpath 🐞 Bug ➹ Performance
Description
ensureDefaultFreeModeStateForMissingAuthSync() now synchronously reads and scans
CODEX_HOME/config.toml whenever there is no provider-state file and no usable auth, and this
function is invoked from multiple request handlers. Because the Zen fallback does not create a state
file, this can cause repeated readFileSync() calls per request in no-auth/no-state flows, adding
avoidable blocking I/O latency.
Code

src/server/codexAppServerBridge.ts[R3783-3786]

Evidence
The new hasExplicitCodexModelProviderConfigSync() performs readFileSync() of config.toml, and
ensureDefaultFreeModeStateForMissingAuthSync() calls it whenever there is no state file and no
usable auth. Since the Zen fallback returns an in-memory default (no file created) and
ensureDefaultFreeModeStateForMissingAuthSync() is called from request handlers, the sync file read
can repeat per request in no-auth/no-state scenarios.

src/server/codexAppServerBridge.ts[3749-3770]
src/server/codexAppServerBridge.ts[3777-3790]
src/server/codexAppServerBridge.ts[5320-5335]
src/server/codexAppServerBridge.ts[6439-6492]
src/server/freeMode.ts[201-206]
src/server/codexAppServerBridge.archive.test.ts[418-431]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureDefaultFreeModeStateForMissingAuthSync()` now calls `hasExplicitCodexModelProviderConfigSync()`, which does a synchronous `readFileSync()`+scan of `config.toml`. Since `ensureDefaultFreeModeStateForMissingAuthSync()` is used in request handlers and the Zen fallback intentionally does not persist a state file, this check can run repeatedly (and synchronously) on hot paths.
## Issue Context
- `shouldCreateDefaultFreeModeStateForMissingAuth()` stays `true` as long as the state file is absent and there is no usable auth.
- The Zen fallback path returns an in-memory state and does not write a file, so the “missing state file” condition persists.
- The bridge calls `ensureDefaultFreeModeStateForMissingAuthSync()` in startup config building and in multiple HTTP routes.
## Fix Focus Areas
- Cache the computed boolean result of `hasExplicitCodexModelProviderConfigSync()` (and/or cache the file-missing case) for the process lifetime, or cache with lightweight invalidation (e.g., `statSync` mtime check).
- Ensure caching is only used for the `shouldCreateDefault` path (so existing state/auth flows remain unchanged).
### References
- src/server/codexAppServerBridge.ts[3720-3790]
- src/server/codexAppServerBridge.ts[5320-5335]
- src/server/codexAppServerBridge.ts[6439-6492]
- src/server/freeMode.ts[201-206]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 4f6fa76


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Remediation recommended
1. Sync config read hotpath 🐞 Bug ➹ Performance
Description
ensureDefaultFreeModeStateForMissingAuthSync() now synchronously reads and scans
CODEX_HOME/config.toml whenever there is no provider-state file and no usable auth, and this
function is invoked from multiple request handlers. Because the Zen fallback does not create a state
file, this can cause repeated readFileSync() calls per request in no-auth/no-state flows, adding
avoidable blocking I/O latency.
Code

src/server/codexAppServerBridge.ts[R3783-3786]

Evidence
The new hasExplicitCodexModelProviderConfigSync() performs readFileSync() of config.toml, and
ensureDefaultFreeModeStateForMissingAuthSync() calls it whenever there is no state file and no
usable auth. Since the Zen fallback returns an in-memory default (no file created) and
ensureDefaultFreeModeStateForMissingAuthSync() is called from request handlers, the sync file read
can repeat per request in no-auth/no-state scenarios.

src/server/codexAppServerBridge.ts[3749-3770]
src/server/codexAppServerBridge.ts[3777-3790]
src/server/codexAppServerBridge.ts[5320-5335]
src/server/codexAppServerBridge.ts[6439-6492]
src/server/freeMode.ts[201-206]
src/server/codexAppServerBridge.archive.test.ts[418-431]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureDefaultFreeModeStateForMissingAuthSync()` now calls `hasExplicitCodexModelProviderConfigSync()`, which does a synchronous `readFileSync()`+scan of `config.toml`. Since `ensureDefaultFreeModeStateForMissingAuthSync()` is used in request handlers and the Zen fallback intentionally does not persist a state file, this check can run repeatedly (and synchronously) on hot paths.

## Issue Context
- `shouldCreateDefaultFreeModeStateForMissingAuth()` stays `true` as long as the state file is absent and there is no usable auth.
- The Zen fallback path returns an in-memory state and does not write a file, so the “missing state file” condition persists.
- The bridge calls `ensureDefaultFreeModeStateForMissingAuthSync()` in startup config building and in multiple HTTP routes.

## Fix Focus Areas
- Cache the computed boolean result of `hasExplicitCodexModelProviderConfigSync()` (and/or cache the file-missing case) for the process lifetime, or cache with lightweight invalidation (e.g., `statSync` mtime check).
- Ensure caching is only used for the `shouldCreateDefault` path (so existing state/auth flows remain unchanged).

### References
- src/server/codexAppServerBridge.ts[3720-3790]
- src/server/codexAppServerBridge.ts[5320-5335]
- src/server/codexAppServerBridge.ts[6439-6492]
- src/server/freeMode.ts[201-206]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@Hummer12007
Copy link
Copy Markdown

All good. Free mode doesn't get enabled with this fix. Thanks!

@friuns2 friuns2 force-pushed the codex/fix-issue-190-provider-config branch from 4f6fa76 to f108186 Compare May 23, 2026 22:56
@friuns2
Copy link
Copy Markdown
Owner Author

friuns2 commented May 23, 2026

/review

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 23, 2026

Persistent review updated to latest commit f108186

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/server/codexAppServerBridge.ts`:
- Around line 3720-3805: The TOML scanner in
hasExplicitCodexModelProviderConfigSync (and helper stripTomlComment) fails to
detect quoted keys and mis-parses multiline strings because state is reset per
line; update stripTomlComment or replace it with a small TOML-aware line scanner
that preserves string state across lines (track inSingleQuote, inDoubleQuote,
inMultilineBasic, inMultilineLiteral, and escaped across lines) so comments
inside multiline strings are ignored, and update the key-detection in
hasExplicitCodexModelProviderConfigSync to accept quoted keys (e.g., handle
"model_provider" and 'model_provider' in addition to unquoted) when matching the
left-hand side before '='; ensure explicitCodexModelProviderConfigCache usage
remains the same and add regression tests for a quoted key `"model_provider" =
"azure"` (should return true) and for a multiline string containing
`model_provider = "azure"` (should return false).
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e50847e4-507c-4514-82b2-3d94b3d108f6

📥 Commits

Reviewing files that changed from the base of the PR and between 4f6fa76 and f108186.

📒 Files selected for processing (3)
  • src/server/codexAppServerBridge.archive.test.ts
  • src/server/codexAppServerBridge.ts
  • tests/providers-models/android-published-cli-loads-codex-app-server-models-through-local-proxy.md
✅ Files skipped from review due to trivial changes (2)
  • src/server/codexAppServerBridge.archive.test.ts
  • tests/providers-models/android-published-cli-loads-codex-app-server-models-through-local-proxy.md

Comment thread src/server/codexAppServerBridge.ts Outdated
@friuns2 friuns2 force-pushed the codex/fix-issue-190-provider-config branch from f108186 to 1917880 Compare May 24, 2026 23:16
@friuns2
Copy link
Copy Markdown
Owner Author

friuns2 commented May 24, 2026

/review

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 24, 2026

Persistent review updated to latest commit 1917880

@friuns2 friuns2 merged commit fa66611 into main May 24, 2026
1 check was pending
@friuns2 friuns2 deleted the codex/fix-issue-190-provider-config branch May 24, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not force OpenCode Zen fallback when config.toml selects a custom provider

3 participants