fix(codex-native): refresh the probe home's credential, not just its config

[dhruv0811]

Related issue

Closes #6252

Summary

Follow-up to #6249, addressing a review note on it. That PR consolidated the
model-probe home onto the shared _populate_codex_home_config bridge, which is
the right call, but the bridge skips any entry that already exists and the probe
home is a persistent cache keyed only on the -c overrides. config.toml was
unlinked before the bridge ran; the credential symlink was not.

• Clear every entry the bridge materializes, not just config.toml.
• Two failure modes this closes, both with byte-identical overrides so the cache
  key never changes: a source home that moves leaves the probe serving
  config.toml from the new source with the credential still naming the old one;
  a source home that is removed leaves the link dangling. The second is
  permanent, because a dangling symlink returns False from exists() but
  True from is_symlink(), and the bridge's skip test is
  if link_path.exists() or link_path.is_symlink(). So the home never
  self-heals and every later probe answers for a logged-out account (login-gated
  entries missing, wrong account default) with no error to point at.
• This restores a property the pre-fix(codex-native): bridge provider config into the model-probe home #6249 hand-rolled code had: it re-created the
  auth.json symlink unconditionally on each call. Consolidating onto the bridge
  traded that for skip-if-present, and this puts it back for the whole bridged
  set rather than one file.

                        config.toml     credential symlink
before this PR          unlinked, so    kept, so a moved source is
                        always fresh    stale and a removed one dangles
                                        forever (cache never self-heals)

after                   unlinked        unlinked
                             \             /
                              both re-materialized from the
                              source home this probe resolved

Test Plan

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. The new assertions are confirmed to catch the regression: narrowing the
refresh back to config.toml alone fails with the credential still resolving into
the old source home while the config came from the moved one:

AssertionError: assert PosixPath('.../test_probe_codex_home_bridges_0/.codex/.credentials.json')
                    == PosixPath('.../test_probe_codex_home_bridges_0/moved-codex/.credentials.json')

The removed-source case is covered too, and asserted independently of the
moved-source case (isolating it against a narrowed fix fails with
AssertionError: a dangling credential link must self-heal). It removes the
source home, requires the link to actually dangle, then re-probes against a
fresh source and requires both the credential and the config to track it.

The dangling-symlink mechanism this all turns on was verified directly rather
than assumed:

$ python -c "...; l.symlink_to(d/'gone'); print(l.exists(), l.is_symlink())"
exists(): False  is_symlink(): True  -> bridge skips: True

Live, against a real ~/.codex/config.toml on a Databricks gateway provider,
cache cleared first, confirming the added unlink does not break the warm-cache
path it runs on every probe:

probe 1: 5 models | probe 2 (warm home): 5 models

with the credential correctly relinked afterwards:

~/.omnigent/cache/codex-model-probe/<key>/.credentials.json -> ~/.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 extends the existing probe-home test with both failure modes. The
moved-source case renames the source home, repoints CODEX_HOME, re-probes under
the same overrides, and asserts the credential resolves into the new source. The
removed-source case then deletes it, asserts the link actually dangles (this is
what the bridge silently accepts), and requires a re-probe against a fresh source
to self-heal. Both assertions were checked to fail against a fix narrowed back to
config.toml alone, the second one in isolation so it is not merely riding on
the first.

Manual verification covers the warm-cache path against the real codex binary.
The added unlink runs on every probe including cache hits, so the thing worth
checking by hand is that a second probe against an unchanged source still returns
the full catalog rather than paying a re-link that breaks it.

Changelog

The codex model picker no longer falls back to a logged-out model list after your Codex config home moves or is recreated.

Issues

Resolves OMNI-6010

[dhruv0811]

/review

[omnigent-ci]

Polly AI Review

Review: fix(codex-native): refresh the probe home's credential, not just its config

1. Blocking issues

None. The change is correct and well-scoped.

Verified against the source: in _populate_codex_home_config, the skip guard is if link_path.exists() or link_path.is_symlink(): continue, and a dangling symlink (moved/removed source) satisfies is_symlink() even though exists() is False — so a stale/dangling credential link would indeed never self-heal. The old code only unlinked config.toml, leaving that hole. Unlinking the full materialized set before calling the bridge closes it.

The cleared set is also complete for the probe's call shape. The probe invokes the bridge with minimal_config=True, so the only entries materialized are _CODEX_HOME_SYMLINK_FILES (auth.json, .credentials.json, memories_1.sqlite) plus the copied config.toml. The global-instruction files and symlink dirs are gated behind not minimal_config and are never created here, so not clearing them is correct, not an omission. unlink(missing_ok=True) correctly removes a dangling symlink (it operates on the link, not the target), and each unlink is individually wrapped in contextlib.suppress(OSError) so one failure doesn't abort the rest.

2. Security vulnerabilities

None. No change to auth handling, path construction, or trust boundaries — credential resolution still symlinks into the resolved source home only. No lockfile or dependency/extras changes in this diff.

3. Non-blocking notes

• The loop hardcodes "config.toml" alongside _CODEX_HOME_SYMLINK_FILES rather than referencing _CODEX_HOME_COPY_FILES (currently ("config.toml",)). If copy-files ever expands, the refresh set here won't track it automatically. Minor coupling; current behavior is exact.
• Re-materializing auth.json/.credentials.json on every warm probe adds a couple of unlink+symlink syscalls per call. Negligible, and the PR's live warm-cache check confirms it doesn't regress the hot path.

4. Approach

Sound and idiomatic. Reusing the existing _CODEX_HOME_SYMLINK_FILES constant to drive the refresh (rather than re-listing filenames) keeps the clear set aligned with what the bridge writes, and restores the pre-#6249 unconditional-relink property for the whole bridged set instead of just one file. No materially simpler alternative given the bridge's skip-if-present contract.

5. Summary

A tight, correctly-reasoned follow-up fix. It closes a real, permanent failure mode (dangling/stale credential symlink in a persistent, override-keyed probe cache that never self-heals), the mechanism is verified against the actual skip guard in _populate_codex_home_config, and the added test exercises the moved-source regression directly with an assertion that catches a narrowed fix. No blocking or security concerns. Ready to merge.

---

_{Automated review by Polly · workflow run}

[dhruv0811]

/review

[omnigent-ci]

Polly AI Review

Review: fix(codex-native): refresh the probe home's credential, not just its config

1. Blocking issues

None.

The fix is correct. _probe_codex_home caches the home keyed only on the -c overrides, and _populate_codex_home_config skips any entry where link_path.exists() or link_path.is_symlink(). The old code unlinked only config.toml, so a symlinked .credentials.json (and the other symlink files) survived across probes and kept pointing at a stale/moved/removed source. The dangling-symlink trap is real and correctly diagnosed: exists() returns False for a broken link but is_symlink() returns True, so the bridge's skip test would keep it forever. Iterating (*_CODEX_HOME_SYMLINK_FILES, *_CODEX_HOME_COPY_FILES) and unlinking each before re-populating closes both failure modes.

I verified the surrounding facts:

• unlink(missing_ok=True) on a dangling symlink removes the link itself (not the absent target), so the self-heal path works.
• The probe runs with minimal_config=True, so _CODEX_HOME_SYMLINK_DIRS (memories/, rules/) and the global instruction files are never materialized into the probe home — they don't need clearing, and the loop correctly omits them. No stale-dir gap is introduced.
• The imported constants (_CODEX_HOME_SYMLINK_FILES, _CODEX_HOME_COPY_FILES) exist and are the exact tuples the bridge iterates, so the clear set stays in sync with what the bridge materializes.

2. Security vulnerabilities

None. The change operates on a per-user, 0o700 cache directory under ~/.omnigent/, unlinks only the specific bridged filenames, and re-materializes from the env-resolved source home. No new external input, no path traversal (filenames are fixed constants), no secret exposure. If anything it tightens correctness around a credential symlink that was previously serving a logged-out account.

3. Non-blocking notes

• The clear loop and the bridge's populate loop now both derive from the same two module constants, which is exactly the coupling you want — but there is no test asserting the clear set equals the bridge's materialized set. If a future symlink entry (e.g. a new dir) is added to the bridge but not covered by the clear loop, the same stale-cache class of bug returns silently. Not worth blocking, but a brief comment at the constants' definition noting "the probe clears these on every call" would help the next editor keep them aligned.
• The test only exercises the moved source case. The removed/dangling case (the permanent one described in the PR) is argued in the comment and verified manually, but not asserted in CI. Adding a case that deletes the source home, confirms the link dangles, then re-probes against a fresh source would lock in the self-heal behavior the PR is primarily defending. Optional.

4. Approach

Sound and minimal. Deriving the clear set from the same constants the bridge consumes is the right call — it avoids a hand-maintained second list drifting out of sync, and it restores the pre-#6249 property (unconditional credential relink) without reverting the consolidation onto the shared bridge. No materially simpler alternative; narrowing back to config.toml alone is exactly the regression this fixes.

5. Summary

A tight, well-reasoned follow-up that fixes a real correctness bug: the persistent probe home kept a stale (or permanently dangling) credential symlink because the bridge skips existing entries and only config.toml was being cleared. Clearing the full bridged set from the shared constants is the correct, minimal fix; the diagnosis of the exists()/is_symlink() dangling-link trap is accurate, and the added test catches the moved-source regression. No blocking or security concerns. Ship it.

---

_{Automated review by Polly · workflow run}

[dhruv0811]

/review

[omnigent-ci]

Polly AI Review

Review: fix(codex-native): refresh the probe home's credential, not just its config

1. Blocking issues

None. The core change is correct. The bridge's skip test at the symlink loop is exactly if link_path.exists() or link_path.is_symlink(): continue, and a dangling symlink returns False/True from that pair — so a persistent, override-keyed probe home would indeed serve a stale or dangling credential forever unless the entry is cleared first. Widening the pre-bridge unlink from config.toml alone to the full (*_CODEX_HOME_SYMLINK_FILES, *_CODEX_HOME_COPY_FILES) set restores the unconditional-recreate property the pre-#6249 code had. Each unlink stays wrapped in contextlib.suppress(OSError) with missing_ok=True, so a missing or partially-populated home is handled.

2. Security vulnerabilities

None. No secrets, no network, no auth-boundary changes. If anything this tightens correctness of an auth-related path (a stale credential link previously caused silent logged-out probes). No lockfile pins, dependencies, or extras changed.

3. Non-blocking notes

• The loop now also unlinks memories_1.sqlite on every probe. For a probe home this is harmless (it's re-symlinked immediately from the resolved source), just marginally more work — worth nothing beyond a mention, and arguably more correct since a moved/removed source would leave that link stale too.
• Clearing is scoped to the file tuples, not _CODEX_HOME_SYMLINK_DIRS. That's correct here: probes run with minimal_config=True, so the dir symlinks are never bridged into a probe home in the first place. No action needed.
• The unlink→repopulate sequence isn't atomic, so two concurrent probes on the same key could momentarily race on an entry. This is pre-existing (it already applied to config.toml) and not introduced by this PR — flagging only for completeness.

4. Approach

Sound and consistent with the repo's direction. #6249 deliberately consolidated onto the shared bridge; rather than reintroduce hand-rolled per-file relinking, this PR reuses the bridge's own _CODEX_HOME_SYMLINK_FILES/_CODEX_HOME_COPY_FILES constants as the source of truth for "what the bridge materializes," so the clear set can't drift out of sync with what gets populated. That's the right seam — a hardcoded second list would be the inferior alternative. No materially simpler option exists.

5. Summary

A small, well-targeted follow-up that fixes a real latent bug: the model-probe cache home is keyed only on -c overrides and outlives a single probe, so a credential symlink pointing at a moved (stale) or removed (dangling) source home would never self-heal and every later probe would answer as a logged-out account with no visible error. Clearing the full bridged file set before repopulating restores the unconditional-refresh behavior, and the constants are reused so the clear set tracks the populate set. The added tests cover both the moved-source and removed/dangling-source cases independently. I couldn't run pytest in this image, but the tests read as coherent and the described assertions match the code paths. Approve.

---

_{Automated review by Polly · workflow run}

[omni-resolve-agent]

Resolve-agent reviewed this contributor PR as the candidate fix. Future actionable maintainer review feedback may be remediated automatically.

[omni-resolve-agent]

Independent verification of this fix against the live reproduction for OMNI-6010 (mirrors #6252): the reproduction fails before this PR and passes on its head — recommend approval.

Fail→pass evidence (repro e2e test driving the real probe_codex_model_options → codex app-server 0.139.0 boot under an isolated $HOME):

• On unfixed main (ac99ec2): after mv codex-a codex-b + CODEX_HOME switch with byte-identical overrides (single cache key e3b0c44298fc), the probe home's auth.json is a dangling symlink to the old codex-a/auth.json (exists() False, is_symlink() True) — every later probe boots codex logged out, silently, forever.
• On this PR's head (f6b0763): the same journey passes — auth.json exists and resolves into the moved codex-b home.
• The second facet (source home removed and recreated at a new path — the permanent case, since a dangling link satisfies the bridge's exists() or is_symlink() skip) was verified independently the same way: dangles forever on unfixed code, self-heals on this head. The PR's own unit-test extension catches both regressions (confirmed failing against the pre-fix module).

Diff review: the approach is the right seam. Rather than reintroducing hand-rolled per-file relinking, it reuses the bridge's own _CODEX_HOME_SYMLINK_FILES / _CODEX_HOME_COPY_FILES constants as the source of truth for what to clear, so the clear set cannot drift from the populate set, and it restores the unconditional-refresh property the pre-#6249 code had. Each unlink is suppressed/missing_ok, so partially-populated homes are safe. Full tests/test_codex_native_app_server.py passes on this head (76 passed), and the touched tests also pass under a hostile ambient env (CODEX_HOME/OPENAI_API_KEY/DATABRICKS_HOST exported).

Polly: a real review exists for this exact head — no blocking/security findings. Its three non-blocking notes were re-assessed: (1) memories_1.sqlite re-linked per probe is harmless and more correct for a moved source; (2) dir symlinks are correctly out of scope — probes run minimal_config=True, and the dir loop is gated on if not minimal_config; (3) the unlink→repopulate race is pre-existing (already true of config.toml alone) and not introduced here.

CI: all checks green on f6b0763; branch is MERGEABLE/CLEAN against main.

Validate the fix live (this fix runs in the runner/host process, so check out the PR — a deployed preview's local runner would run unfixed code):

gh pr checkout 6253
# with the codex CLI on PATH, in an isolated HOME:
#  seed $HOME/codex-a with auth.json + config.toml, export CODEX_HOME=$HOME/codex-a, probe once:
python -c 'import asyncio; from omnigent.codex_native_app_server import probe_codex_model_options as p; print(len(asyncio.run(p())))'
#  mv $HOME/codex-a $HOME/codex-b; export CODEX_HOME=$HOME/codex-b; probe again, then check:
#  $HOME/.omnigent/cache/codex-model-probe/<key>/auth.json must exist and resolve into codex-b

This is an automated reviewer's verification; a maintainer's approval is still required to merge.