fix(codex-runtime): re-running /codex-runtime codex_app_server when already enabled now triggers migration

[simpolism]

Fixes #26529

What

When model.openai_runtime is already set to codex_app_server, re-running /codex-runtime codex_app_server now triggers the migration (writes managed block to ~/.codex/config.toml, registers hermes-tools MCP callback, discovers native plugins) instead of short-circuiting with "already set".

Why

The current short-circuit at hermes_cli/codex_runtime_switch.py:148 returns early when new_value == current, skipping the entire migration block. This conflates "the config value is correct" with "the world state is converged" — which is fine for the auto → no-op case (disabling has no world-state side effects) but breaks for codex_app_server → no-op, where the user can have the config value correct but be missing the ~/.codex/config.toml managed block, the MCP callback, and the plugin discovery.

How users hit this: pre-set openai_runtime: codex_app_server in config.yaml (legitimate workflow — hermes profile create --clone does it automatically when cloning a profile that has codex runtime enabled), then run /codex-runtime codex_app_server to trigger migration. Silent no-op. The bot looks like it's on the app-server runtime but is actually on codex_responses fallback.

How

Fall through to the migration block when re-applying codex_app_server while skipping config persistence (no value-level change → no need to rewrite config.yaml). The migration is idempotent by design (uses _strip_existing_managed_block() before writing), so re-running is safe and cheap. The user-visible message changes to "openai_runtime already set to codex_app_server — re-applying migration" so re-runs surface what happened.

The auto re-apply path keeps the early-return short-circuit since disabling has no world-state side effects to converge.

Tests

New regression test test_reapply_codex_app_server_runs_migration asserts:

• migrate() was called when re-applying
• persist_callback was NOT called (no spurious config writes on no-op transitions)
• Migration output (MCP servers, sandbox default) surfaces in the user-visible message
• requires_new_session is True so callers know to /reset

Verified RED→GREEN locally: the test fails on origin/main with "migration must run on reapply, not just first enable" and passes with this fix.

$ python -m pytest tests/hermes_cli/test_codex_runtime_switch.py -o addopts="" -q
31 passed in 0.54s

Scope

Single-commit, two-file change. Production code change is ~15 lines (one new code path branching on reapplying_enable). Existing tests for the value-changing transitions remain green and unmodified.

[alt-glitch]

Competing fix: #26535 by @aqilaziz targets the same bug with the same approach. Both modify codex_runtime_switch.py identically in logic. Maintainer should pick one.

[austinpickett]

Please use .github/PULL_REQUEST_TEMPLATE.md

[Copilot]

Pull request overview

This PR fixes a /codex-runtime codex_app_server footgun where re-running the command while model.openai_runtime is already codex_app_server would previously short-circuit and skip the migration that converges Codex world-state (~/.codex/config.toml managed block, hermes-tools MCP callback registration, and plugin discovery).

Changes:

• Adjusted codex_runtime_switch.apply() to not early-return when reapplying codex_app_server, allowing the migration block to run while skipping config persistence.
• Updated the user-facing message to indicate the runtime is already set and that migration is being re-applied.
• Added a regression test ensuring migration runs on reapply and that persist_callback is not invoked on no-op value transitions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
hermes_cli/codex_runtime_switch.py	Allows “reapply codex_app_server” to run migration while avoiding unnecessary config writes; updates messaging accordingly.
tests/hermes_cli/test_codex_runtime_switch.py	Adds regression coverage for reapply behavior (migration runs, no persistence, messaging, requires new session).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kshitijk4poor]

Merged via #56563 (#56563) — your commit was cherry-picked with authorship preserved (rebase-merge, 35eb93c8d on main).

Clean salvage onto current main. Your reapplying_enable fix and the test_reapply_codex_app_server_runs_migration regression test (asserting migration runs, persist_callback does NOT fire, requires_new_session=True) carried over unchanged and RED→GREEN reproduced.

One follow-up on top (7322da487, mine): a small control-flow tidy — collapsed the reapplying_enable predicate to a chained comparison and deduped the msg_lines literals. No behavior change.

Nice writeup distinguishing "config value correct" from "world state converged" — that's exactly the right framing, and delegating idempotency to migrate()'s strip-before-write rather than adding a parallel check was the clean call. Thanks!