Skip to content

fix(codex-native): bridge provider config into the model-probe home - #6249

Merged
dhruv0811 merged 1 commit into
omnigent-ai:mainfrom
dhruv0811:fix/codex-probe-home-provider-config
Sep 3, 2026
Merged

fix(codex-native): bridge provider config into the model-probe home#6249
dhruv0811 merged 1 commit into
omnigent-ai:mainfrom
dhruv0811:fix/codex-probe-home-provider-config

Conversation

@dhruv0811

@dhruv0811 dhruv0811 commented Sep 3, 2026

Copy link
Copy Markdown
Member

Related issue

Closes #6248

Summary

The codex-native model catalog probe built its CODEX_HOME by hand and
materialized only auth.json, never config.toml. When a host's codex routing
comes from the user's own ~/.codex/config.toml, ambient detection pins just the
provider name (-c model_provider="Databricks") while the
[model_providers.Databricks] tables live in that config file, which the probe
home never received. Codex refuses to load a config naming a provider it cannot
see and exits 1 before binding its listener, so every probe raised
Codex model discovery exited early (1) and the pre-launch picker offered zero
models (host/connect.py deliberately keeps no curated fallback).

  • _probe_codex_home now calls the same bridge a session launch uses
    (_populate_codex_home_config(..., minimal_config=True)) instead of
    re-implementing a subset of it. minimal_config already exists for this shape:
    it copies only model_provider / model_providers / profiles and skips
    hooks, AGENTS.md and plugin dirs, so a probe still starts no MCPs. It is
    already used by runner/background_titles/codex_native.py for the same reason.
  • Fixes the credential as a side effect: the hand-rolled version linked only
    auth.json, while _CODEX_HOME_SYMLINK_FILES also covers
    .credentials.json. A home storing the credential under that spelling was
    probing as logged out, so login-gated entries and the account default were
    wrong even when the probe did boot.
  • Every entry the bridge materializes is unlinked first, because the bridge skips
    files that already exist and the probe home is deliberately persistent (codex's
    models_cache.json ETag reuse). Without that, the tables copied on first use
    would be pinned forever and later edits to ~/.codex/config.toml ignored. The
    same applies to the credential: the home is keyed only by the overrides, so a
    source home that moves under an unchanged override set would leave the symlink
    naming the old path, and a removed source would leave it dangling. A dangling
    link reads as present to the bridge's skip check, so such a home could never
    self-heal and every later probe would answer for a logged-out account.

ELI5: the probe was handed the name of a door but not the door. Now it gets the
same keyring the real session launch gets, minus the noisy extras.

before                                  after
------                                  -----
probe home:  auth.json only             probe home:  config.toml (provider
  (absent on homes using                  tables only) + credential symlink
   .credentials.json)                      (.credentials.json / auth.json)
     |                                        |
  -c model_provider="Databricks"          -c model_provider="Databricks"
     |                                        |
  codex: "Model provider                  codex: binds listener,
   `Databricks` not found" -> exit 1       model/list -> 5 models
     |                                        |
  RuntimeError: exited early (1)          catalog populated
  picker: 0 models                        picker: real models

Test Plan

Unit, on the failing path:

pytest tests/test_codex_native_app_server.py::test_probe_codex_home_bridges_provider_tables_and_credential
pytest tests/test_codex_native_app_server.py::test_probe_codex_model_options_uses_launch_config_and_marks_default
pytest tests/test_codex_native_app_server.py::test_probe_codex_model_options_probes_every_launch_shape

3 passed. Both halves of the new test are confirmed to catch their regression by
reverting only codex_native_app_server.py:

  • dropping the bridge call fails with
    FileNotFoundError: .../codex-model-probe/<key>/config.toml;
  • narrowing the refresh back to config.toml alone fails the credential
    assertion, with the symlink still resolving into the old .codex while the
    config came from moved-codex.

Live, against a real ~/.codex/config.toml using a Databricks gateway provider
(the configuration that reproduced the bug), with the probe cache cleared first:

$ rm -rf ~/.omnigent/cache/codex-model-probe
$ python -c 'import asyncio; from omnigent.codex_native_app_server import \
    probe_codex_model_options; print(asyncio.run(probe_codex_model_options()))'
PROBE OK: 5 models
   gpt-5.6-sol   | default: True
   gpt-5.6-terra | default: False
   gpt-5.6-luna  | default: False
   gpt-5.5       | default: False
   gpt-5.2       | default: False

A second, warm-home probe returns the same 5 models, with the credential
correctly relinked. The same command before the fix raised RuntimeError: Codex model discovery exited early (1). Inspected the materialized home to confirm the
minimal shape held:
model_providers present (3 hits), mcp_servers absent (0 hits), no
hooks.json, credential symlinked to the real ~/.codex/.credentials.json.

Demo

  • Visual demo attached below
  • Non-visual evidence provided below or in Test Plan
  • Not applicable — no behavioral change

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

The unit test drives _probe_codex_home against a fake source home and asserts
the three properties the bug turned on: the provider tables are present, the
credential is symlinked, and a persistent home re-reads an edited source config
rather than pinning the first copy. It also asserts the minimal shape (no
mcp_servers, no hooks.json) so a future widening to a full config copy does
not silently make probes start the user's MCP servers.

Manual verification covers what a unit test cannot: that the real codex
binary actually boots and returns a catalog under this materialization. The
failure mode was a subprocess exiting on its own config validation, so the fix
is only meaningful against a real CLI and a real provider config.

Changelog

The codex model picker now lists models when your codex routing comes from a custom provider in ~/.codex/config.toml, instead of coming up empty.

The codex catalog probe built its CODEX_HOME by hand and materialized only
auth.json, never config.toml. A launch shape resolved off the user's own
~/.codex/config.toml pins just the provider name (-c model_provider="X"),
while the [model_providers.X] tables live in that config file, so codex was
told to route through a provider the probe home could not see:

  Error: error loading default config after config error:
  Model provider `Databricks` not found

It exits 1 before binding the listener, so every probe raised "Codex model
discovery exited early (1)" and the pre-launch picker offered zero models
(host/connect.py keeps no curated fallback).

Call the same bridge a session launch uses, in its minimal shape, which
copies only model_provider/model_providers/profiles and skips hooks,
instructions and plugin dirs so no MCPs start for a probe. That also fixes
the credential: the hand-rolled version linked only auth.json, while the
bridge covers .credentials.json too, so a home using that spelling no
longer probes as logged out. The copied config is unlinked first because
the bridge skips existing files and the probe home is persistent.

Closes omnigent-ai#6248

Signed-off-by: Dhruv Gupta <dhruv0811@gmail.com>
@github-actions github-actions Bot added the size/M Pull request size: M label Sep 3, 2026
@dhruv0811

Copy link
Copy Markdown
Member Author

/review

@omnigent-ci

omnigent-ci Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Review: fix(codex-native): bridge provider config into the model-probe home

1. Blocking issues

None. The core change is correct and well-targeted. The probe now materializes its CODEX_HOME through the same _populate_codex_home_config(..., minimal_config=True) bridge a session launch uses, so the [model_providers.<name>] tables that ambient -c model_provider="…" overrides reference are actually present. That directly resolves the exited early (1) config-load failure. Verified against the source: minimal_config=True copies only model_provider / model_providers / profiles (strips mcp_servers), does not append _CODEX_HOME_GLOBAL_INSTRUCTION_FILES (so hooks.json/AGENTS.md aren't linked) and skips _CODEX_HOME_SYMLINK_DIRS, so the probe stays minimal and starts no MCPs — matching the test assertions. The credential fix is also real: _CODEX_HOME_SYMLINK_FILES covers both auth.json and .credentials.json, so a home storing the credential under the latter spelling now probes as logged in.

The unlink-before-bridge for config.toml is necessary and correct: the bridge early-returns on any file that already exists, and the probe home is persistent, so without the unlink(missing_ok=True) the first-probe tables would be pinned and later edits to ~/.codex/config.toml ignored. The test's second half exercises exactly this.

2. Security vulnerabilities

None. No new secret exposure — credentials are symlinked (not copied into a location with weaker perms), the home is created mode=0o700, and the minimal copy deliberately narrows what leaves the source config. No injection, traversal, or boundary weakening. No lockfile or dependency-extra changes in this diff.

3. Non-blocking notes

  • Stale credential symlink when only the source changes but overrides don't. The probe home is keyed solely by the config_overrides hash. config.toml is now force-refreshed each call (unlinked first), but the auth.json / .credentials.json symlinks are left alone whenever they already exist (the bridge skips existing links). The old hand-rolled code unconditionally re-created the auth.json symlink each probe. So if _codex_home_config_source_from_env() resolves to a different source directory across probes while the overrides are byte-identical (e.g. a CODEX_HOME env switch on the empty-override plain-login shape), the probe would serve config.toml from the new source but a credential symlink pointing at the old one. This is a narrow edge case, but it's a behavioral asymmetry worth a comment or a matching unlink for consistency.
  • Source-config parse failure surfaces as a raise. With minimal_config=True, a malformed ~/.codex/config.toml makes tomlkit.parse throw inside the bridge, propagating out of _probe_codex_home. That matches session-launch behavior, so it's acceptable, but the probe path had previously been resilient to config content (it ignored config.toml entirely). No action required — just noting the widened failure surface.

4. Approach

Sound, and the more consistent option. Replacing a hand-rolled subset of the materialization with the canonical bridge is exactly right — it removes drift risk (the .credentials.json gap is a concrete example of that drift), and minimal_config already existed for precisely this "provider routing without the noisy extras" shape and is already used by runner/background_titles/codex_native.py. No simpler alternative that wouldn't just re-introduce a divergent copy of the bridge logic.

5. Summary

A focused, correct bug fix that eliminates a class of probe failures by routing the probe home through the same config bridge as a real session launch, incidentally repairing the .credentials.json credential gap. The persistent-home refresh semantics are handled correctly for config.toml. The only follow-up worth considering is the minor asymmetry where the credential symlink isn't refreshed on a same-override source change. Recommend merge; the noted item is non-blocking.


Automated review by Polly · workflow run

@dhruv0811
dhruv0811 enabled auto-merge (squash) September 3, 2026 01:31
@dhruv0811
dhruv0811 merged commit a8418c7 into omnigent-ai:main Sep 3, 2026
61 checks passed
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

Internal change to how the Codex model-probe home is materialized (provider tables plus credential) with added tests; no user-facing surface, integration, or documented default changed.

Auto-classified on merge. Set the label manually before merging to override. · run

@dhruv0811

Copy link
Copy Markdown
Member Author

/review

@github-actions github-actions Bot added the P1-high Priority: major feature broken, no workaround label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review skipped

abb4cc4 already has a review, so there is nothing new to look at.

Push a commit or comment /review force to review it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-doc-update Merged PR does not need a docs update P1-high Priority: major feature broken, no workaround size/M Pull request size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codex-native model catalog probe fails when routing comes from ~/.codex/config.toml (empty model picker)

1 participant