Skip to content

fix(codex-runtime): re-running /codex-runtime codex_app_server when already enabled now triggers migration (salvage of #26532 by @simpolism, closes #26529) - #468

Merged
hashbender merged 1 commit into
mainfrom
mirror/pr-56563
Jul 1, 2026
Merged

fix(codex-runtime): re-running /codex-runtime codex_app_server when already enabled now triggers migration (salvage of #26532 by @simpolism, closes #26529)#468
hashbender merged 1 commit into
mainfrom
mirror/pr-56563

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Summary

/codex-runtime codex_app_server now re-runs the migration when the runtime is already set to codex_app_server, instead of silently no-op'ing with "already set".

Root cause: apply() short-circuits on new_value == current before the migration block. That conflates "config value is correct" with "world state is converged" — fine for auto (disabling has no side effects) but wrong for codex_app_server, where the config value can be correct while the ~/.codex/config.toml managed block, the hermes-tools MCP callback, and plugin discovery are missing. Users hit this by pre-setting openai_runtime: codex_app_server in config.yaml (which hermes profile create --clone does automatically), then running the slash command expecting migration — silent no-op, bot falls back to codex_responses.

Changes

  • hermes_cli/codex_runtime_switch.py: on codex_app_server re-apply, fall through to the (idempotent) migration block while skipping set_runtime/persist — no config-value change, so config.yaml is not rewritten and prompt cache is untouched. User-visible message becomes "already set to codex_app_server — re-applying migration". auto re-apply keeps the early-return short-circuit.
  • tests/hermes_cli/test_codex_runtime_switch.py: test_reapply_codex_app_server_runs_migration — asserts migrate() ran, persist_callback did NOT fire, migration output surfaces, requires_new_session=True.

Validation

scenario migration runs config rewritten
first enable (autocodex_app_server) yes yes
re-apply (codex_app_servercodex_app_server) yes (was: no) no
re-apply autoauto no no
  • RED→GREEN proven: the new test fails on origin/main ("migration must run on reapply") and passes with this fix.
  • 31/31 tests in tests/hermes_cli/test_codex_runtime_switch.py pass.

Salvage of NousResearch#26532 by @simpolism (also reporter of NousResearch#26529). Cherry-picked cleanly onto current main, authorship preserved. Duplicate NousResearch#26535 (@aqilaziz) already closed; this is the more thorough implementation.

Closes NousResearch#26529


Review + follow-up (hermes-pr-review Phase 2)

Ran the full Phase-2 review (targeted + broader test sweep, real unmocked apply() smoke, 3-angle + Hermes-specific review): no Critical findings. Prompt-cache invariant verified (reapply does NOT call set_runtime/persist and does not touch system prompt/toolsets), auto-disable early-return preserved, migrate() idempotency (strip-before-write, atomic replace) confirmed.

Follow-up commit 6e761547d (mine) folds in the two non-blocking clarity findings: collapse the reapplying_enable predicate to a chained comparison, and dedupe the msg_lines literals. No behavior change; 31/31 tests green.

Also caught + corrected a stale-base artifact before pushing: an earlier working-tree state showed a phantom deletion of an unrelated scripts/release.py AUTHOR_MAP entry (branch was cut before that chore PR merged). Rebased onto current main so the PR shows only the two intended files.


Mirror-of: NousResearch#56563
NousResearch#56563

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete
No issues found!

Risk: 🟡 Medium (38/100) — no findings · 97 LOC across 2 files


PR adds a /codex-runtime slash command to toggle between Hermes' native runtime and Codex app-server mode, with MCP server migration. One medium-severity bug found: the binary check gate incorrectly blocks re-apply migration when codex is already enabled but the binary is missing.

Files Reviewed (2 files)
hermes_cli/codex_runtime_switch.py
tests/hermes_cli/test_codex_runtime_switch.py

@hashbender
hashbender merged commit f47e4cc into main Jul 1, 2026
22 of 31 checks passed
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.

/codex-runtime codex_app_server silently skips migration when openai_runtime already set in config

1 participant