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

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",