fix(codex-runtime): pin CLI registration, honour CODEX_HOME, widen preserve-user

- The `hermes codex-runtime` subcommand registration in hermes_cli/main.py was unpinned (deleting build_codex_runtime_parser(subparsers) kept every test green). The CLI test now drives _build_cli_parser() and args.func. - migrate() defaulted codex_home to ~/.codex and ignored CODEX_HOME, unlike codex_models.py / auth_codex.py and the codex subprocess it spawns; the documented `hermes -p work codex-runtime migrate` inherited the mismatch. - _unmanaged_mcp_server_names derives user-owned names from tomllib when the user text parses (inline tables, dotted keys, `[ mcp_servers.x ]`), so the preserve-user policy applies instead of refusing to write; the header scan stays as the fallback for unparsable text. - When plugin discovery fails on a pre-broken config.toml, the report says the existing file was unloadable and to re-run to migrate plugins (#79023).
2026-09-19 01:17:01 -07:00
parent d09183d8d3
commit 0923589e7d
3 changed files with 59 additions and 7 deletions
--- a/hermes_cli/codex_runtime_plugin_migration.py
+++ b/hermes_cli/codex_runtime_plugin_migration.py
@@ -251,6 +251,12 @@ def _unmanaged_mcp_server_names(toml_text: str) -> set[str]:
    NOT re-emitted (the user's table wins and is preserved verbatim); emitting both would be a
    duplicate table header, which is invalid TOML that codex refuses to load (issue #79023).
    """
+    try:
+        parsed = tomllib.loads(toml_text).get("mcp_servers")
+    except tomllib.TOMLDecodeError:
+        parsed = None
+    if isinstance(parsed, dict):  # covers inline tables, dotted keys, `[ mcp_servers.x ]`
+        return {str(name) for name in parsed}
    names: set[str] = set()
    for line in toml_text.splitlines():
        stripped = line.lstrip()
@@ -427,7 +433,8 @@ def migrate(
    server so the codex subprocess can call back for tools it lacks.
    """
    report = MigrationReport(dry_run=dry_run)
-    codex_home = codex_home or Path.home() / ".codex"
+    codex_home = codex_home or Path(
+        os.getenv("CODEX_HOME", "").strip() or str(Path.home() / ".codex")).expanduser()
    target = codex_home / "config.toml"
    report.target_path = target
    hermes_servers = (hermes_config or {}).get("mcp_servers") or {}
@@ -469,6 +476,14 @@ def migrate(
        except Exception as exc:
            report.errors.append(f"could not read {target}: {exc}")
            return report
+        if report.plugin_query_error:
+            try:
+                tomllib.loads(existing)
+            except tomllib.TOMLDecodeError:
+                # codex could not load the pre-broken file, so plugin/list failed for that reason.
+                report.plugin_query_error += (
+                    "; existing config.toml was unloadable — re-run `hermes codex-runtime migrate` "
+                    "to migrate plugins")
        without_managed = _strip_existing_managed_block(existing)
        if plugin_query_succeeded:
            without_managed = _strip_unmanaged_plugin_tables(without_managed)
--- a/tests/hermes_cli/test_codex_runtime_plugin_migration.py
+++ b/tests/hermes_cli/test_codex_runtime_plugin_migration.py
@@ -448,20 +448,57 @@ class TestSameNameUserMcpTable:
        assert report.migrated == ["other"]
        assert "gbrain" in report.summary()

+    def test_inline_table_user_server_is_preserved(self, tmp_path):
+        """User declarations in other valid TOML shapes (`[mcp_servers]` + inline table) are
+        theirs too: skip the projection instead of refusing to write on a duplicate key."""
+        import tomllib
+
+        target = tmp_path / "config.toml"
+        target.write_text('[mcp_servers]\ngbrain = { command = "existing-gbrain" }\n', encoding="utf-8")
+        report = migrate(
+            {"mcp_servers": {"gbrain": {"command": "projected-gbrain"}, "other": {"command": "o"}}},
+            codex_home=tmp_path, discover_plugins=False, expose_hermes_tools=False,
+            default_permission_profile=None)
+        parsed = tomllib.loads(target.read_text(encoding="utf-8"))
+        assert report.written and report.errors == []
+        assert report.preserved_user_servers == ["gbrain"]
+        assert parsed["mcp_servers"]["gbrain"]["command"] == "existing-gbrain"
+        assert parsed["mcp_servers"]["other"]["command"] == "o"
+
+    def test_unloadable_existing_config_explains_plugin_rerun(self, tmp_path, monkeypatch):
+        """Repairing a pre-broken config.toml: codex cannot load it, so plugin/list fails on this
+        run. The report must say plugins need a re-run instead of a bare discovery error."""
+        from hermes_cli import codex_runtime_plugin_migration as crpm
+
+        target = tmp_path / "config.toml"
+        target.write_text('[mcp_servers.gbrain]\ncommand = "a"\n[mcp_servers.gbrain]\ncommand = "b"\n',
+                          encoding="utf-8")
+        monkeypatch.setattr(crpm, "_query_codex_plugins",
+                            lambda codex_home=None, timeout=8.0: ([], "plugin/list query failed"))
+        report = migrate({}, codex_home=tmp_path, discover_plugins=True, expose_hermes_tools=False)
+        assert "re-run `hermes codex-runtime migrate` to migrate plugins" in (report.plugin_query_error or "")
+        assert "existing config.toml was unloadable" in report.summary()
+
    def test_cli_migrate_dry_run_json_reports_without_writing(self, tmp_path, monkeypatch, capsys):
-        """`hermes codex-runtime migrate --dry-run --json` is the supported automation seam."""
+        """`hermes codex-runtime migrate --dry-run --json` is the supported automation seam:
+        drive it through the real ``hermes`` argparse tree so the subcommand registration in
+        hermes_cli/main.py stays pinned, and honour ``CODEX_HOME`` like every codex sibling."""
        import json

-        from hermes_cli.subcommands import codex_runtime as mod
+        import hermes_cli.main as main

-        (tmp_path / ".codex").mkdir()
-        target = tmp_path / ".codex" / "config.toml"
+        codex_home = tmp_path / "alt-codex"
+        codex_home.mkdir()
+        target = codex_home / "config.toml"
        target.write_text('[mcp_servers.gbrain]\ncommand = "existing-gbrain"\n', encoding="utf-8")
+        monkeypatch.setenv("CODEX_HOME", str(codex_home))
        monkeypatch.setattr("pathlib.Path.home", classmethod(lambda cls: tmp_path))
        monkeypatch.setattr(
            "hermes_cli.config.load_config",
            lambda: {"mcp_servers": {"gbrain": {"command": "projected"}, "other": {"command": "o"}}})
-        rc = mod.cmd_codex_runtime_migrate(argparse.Namespace(dry_run=True, json=True))
+        parser, _subparsers = main._build_cli_parser()
+        args = parser.parse_args(["codex-runtime", "migrate", "--dry-run", "--json"])
+        rc = args.func(args)
        payload = json.loads(capsys.readouterr().out)
        assert rc == 0
        assert payload["dry_run"] is True and payload["written"] is False
--- a/website/docs/user-guide/features/codex-app-server-runtime.md
+++ b/website/docs/user-guide/features/codex-app-server-runtime.md
@@ -319,7 +319,7 @@ hermes codex-runtime migrate --json     # machine-readable report (migrated, pre
 hermes -p work codex-runtime migrate    # a named profile's mcp_servers
 ```

-This is the same migration `/codex-runtime codex_app_server` runs; it is idempotent, writes atomically, and exits non-zero when the report contains errors.
+This is the same migration `/codex-runtime codex_app_server` runs; it is idempotent, writes atomically, and exits non-zero when the report contains errors. It writes `$CODEX_HOME/config.toml` when `CODEX_HOME` is set (see below), otherwise `~/.codex/config.toml`.

 ## Multi-profile / multi-tenant setups