codex-runtime migration skipped when already enabled

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

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

  merged to main

  files: hermes_cli/codex_runtime_switch.py, tests/hermes_cli/test_codex_runtime_switch.py

  Show the diff

  • hermes_cli/codex_runtime_switch.py

    @@ -144,8 +144,17 @@ def _check_binary_cached() -> tuple[bool, Optional[str]]:
                 codex_version=ver if ok else None,
             )
     
    -    # No change requested
    -    if new_value == current:
    +    # No-config-change paths. For `auto` we return immediately — disabling
    +    # doesn't touch ~/.codex/. For `codex_app_server`, we fall through to
    +    # the migration block below: the config value is already correct, but
    +    # the world state (managed block in ~/.codex/config.toml, hermes-tools
    +    # MCP callback, plugin discovery) may be stale or missing — common
    +    # footgun when users pre-set `openai_runtime: codex_app_server` in
    +    # config.yaml without ever running the slash command. The migration is
    +    # idempotent by design (it replaces its own managed block in place), so
    +    # re-running is cheap and safe.
    +    reapplying_enable = new_value == current == "codex_app_server"
    +    if new_value == current and not reapplying_enable:
             return CodexRuntimeStatus(
                 success=True,
                 new_value=current,
    @@ -172,22 +181,26 @@ def _check_binary_cached() -> tuple[bool, Optional[str]]:
                     codex_version=None,
                 )
     
    -    set_runtime(config, new_value)
    -    if persist_callback is not None:
    -        try:
    -            persist_callback(config)
    -        except Exception as exc:
    -            logger.exception("failed to persist openai_runtime change")
    -            return CodexRuntimeStatus(
    -                success=False,
    -                new_value=new_value,
    -                old_value=current,
    -                message=f"updated config in memory but persist failed: {exc}",
    -            )
    +    if not reapplying_enable:
    +        set_runtime(config, new_value)
    +        if persist_callback is not None:
    +            try:
    +                persist_callback(config)
    +            except Exception as exc:
    +                logger.exception("failed to persist openai_runtime change")
    +                return CodexRuntimeStatus(
    +                    success=False,
    +                    new_value=new_value,
    +                    old_value=current,
    +                    message=f"updated config in memory but persist failed: {exc}",
    +                )
     
    -    msg_lines = [
    -        f"openai_runtime: {current} → {new_value}",
    -    ]
    +    if reapplying_enable:
    +        msg_lines = [
    +            f"openai_runtime already set to {current} — re-applying migration"
    +        ]
    +    else:
    +        msg_lines = [f"openai_runtime: {current} → {new_value}"]
         if new_value == "codex_app_server":
             ok, ver = _check_binary_cached()
             if ok:

  • tests/hermes_cli/test_codex_runtime_switch.py

    @@ -96,6 +96,56 @@ def test_no_change_when_already_set(self):
             assert r.success
             assert r.message == "openai_runtime already set to auto"
     
    +    def test_reapply_codex_app_server_runs_migration(self):
    +        """Re-applying codex_app_server when already enabled must still
    +        run the migration. Common footgun: user pre-sets
    +        `openai_runtime: codex_app_server` in config.yaml, then runs
    +        /codex-runtime codex_app_server expecting the migration. Without
    +        this, the slash command short-circuits with "already set" and
    +        ~/.codex/config.toml never gets the hermes-tools MCP callback
    +        or plugin migration — silent partial setup.
    +        """
    +        cfg = {
    +            "model": {"openai_runtime": "codex_app_server"},
    +            "mcp_servers": {
    +                "filesystem": {"command": "npx", "args": ["-y", "fs-server"]},
    +            },
    +        }
    +        persisted = {}
    +
    +        def persist(c):
    +            persisted.update(c)
    +
    +        with patch.object(crs, "check_codex_binary_ok",
    +                          return_value=(True, "0.130.0")), \
    +             patch("hermes_cli.codex_runtime_plugin_migration.migrate") as mig:
    +            mig.return_value.migrated = ["filesystem", "hermes-tools"]
    +            mig.return_value.migrated_plugins = []
    +            mig.return_value.plugin_query_error = None
    +            mig.return_value.wrote_permissions_default = ":workspace"
    +            mig.return_value.errors = []
    +            mig.return_value.target_path = "/fake/.codex/config.toml"
    +            r = crs.apply(cfg, "codex_app_server",
    +                          persist_callback=persist)
    +        assert r.success
    +        assert mig.called, "migration must run on reapply, not just first enable"
    +        # Re-apply should signal "already set" but still announce migration ran
    +        assert "already set" in r.message
    +        assert "re-applying migration" in r.message
    +        # Migration output still surfaces
    +        assert "Migrated 1 MCP server" in r.message
    +        assert "filesystem" in r.message
    +        assert "Default sandbox: :workspace" in r.message
    +        # No config write needed when value is unchanged — the persist
    +        # callback should NOT have fired (avoids spurious config.yaml mtimes
    +        # on every re-apply).
    +        assert persisted == {}, (
    +            "persist_callback fired despite no config-value change"
    +        )
    +        # Caller still needs a fresh session for the cached agent to pick
    +        # up any migration-driven changes.
    +        assert r.requires_new_session is True
    +
         def test_enable_blocked_when_codex_missing(self):
             cfg = {}
             with patch.object(crs, "check_codex_binary_ok",