refactor: complete approval mode/timeout resolution migration to tools/approval.py core (TUI + codex surfaces)

TUI (tui_gateway/server.py _load_approval_mode): now delegates to
tools.approval._get_approval_mode instead of re-reading config raw via
_load_cfg + _deep_merge(DEFAULT_CONFIG, ...) and normalizing locally.
Behavior fix, not pure refactor: the canonical load_config path applies
managed-scope config overlays and ${VAR} env expansion, plus a legacy
max_turns lift, which the TUI's raw YAML read bypassed — under a managed
config that sets approvals.mode, the TUI previously reported/toggled a
different mode than the approval gate actually enforced. Both surfaces
now agree by construction. Name/signature and the mode-vocabulary clamp
are preserved.

Codex (agent/transports/codex_app_server_session.py): read confirmed the
_decide_exec_approval/_decide_apply_patch_approval paths carry NO
Hermes-side mode/timeout reads — the Hermes resolution already flows in
from agent/codex_runtime.py via tools.approval.is_approval_bypass_active()
(auto_approve_* routing) and via the shared approval-gate callback. So no
code extraction was needed; added docstrings pinning that invariant and a
cross-reference on the protocol-semantic choice mapping
(_approval_choice_to_codex_decision), which intentionally stays local.

Adds tests/tools/test_approval_mode_parity.py: cross-surface invariant
test asserting the core resolver, the TUI path, and the codex bypass
derivation agree for synthetic configs (unset defaults, global mode set,
YAML-bool off, malformed values, whitespace/case), plus a delegation-seam
test proving the TUI has no independent config read left.

Note: gateway/run.py has sibling raw reads but is intentionally untouched
(multiple in-flight PRs); flagged as follow-up.

agent/transports/codex_app_server_session.py

@@ -1054,6 +1054,18 @@ class CodexAppServerSession:
             )
 
     def _decide_exec_approval(self, params: dict) -> str:
+        """Decide a Codex exec approval request.
+
+        This is protocol-level routing only — it carries NO Hermes
+        approval-mode/timeout logic. The Hermes-side resolution happens
+        upstream: ``agent/codex_runtime.py`` derives
+        ``auto_approve_exec`` from the canonical
+        ``tools.approval.is_approval_bypass_active()`` (which reads
+        ``approvals.mode`` via ``tools.approval._get_approval_mode``),
+        and ``self._approval_callback`` itself runs the shared approval
+        gate (mode + ``approvals.timeout``) in ``tools/approval.py``.
+        Keep it that way — do not re-read approval config here.
+        """
         if self._routing.auto_approve_exec:
             return "accept"
         command = params.get("command") or ""
@@ -1077,6 +1089,12 @@ class CodexAppServerSession:
         return "decline"  # fail-closed when no callback wired
 
     def _decide_apply_patch_approval(self, params: dict) -> str:
+        """Decide a Codex apply_patch approval request.
+
+        Protocol-level routing only; Hermes approval-mode/timeout
+        resolution is delegated to ``tools/approval.py`` upstream — see
+        the docstring on ``_decide_exec_approval``.
+        """
         if self._routing.auto_approve_apply_patch:
             return "accept"
         if self._approval_callback is not None:
@@ -1231,6 +1249,11 @@ def _approval_choice_to_codex_decision(choice: str) -> str:
     Codex expects 'accept', 'acceptForSession', 'decline', or 'cancel'
     (verified against codex-rs/app-server-protocol/src/protocol/v2/item.rs
     on codex 0.130.0).
+
+    This mapping is Codex-protocol-semantic and intentionally lives here,
+    NOT in tools/approval.py: the Hermes approval mode/timeout resolution
+    and the choice itself come from the shared core (tools/approval.py);
+    only the wire-value translation is local.
     """
     if choice in {"once",}:
         return "accept"

tests/tools/test_approval_mode_parity.py

@@ -0,0 +1,156 @@
+"""Cross-surface approval mode/timeout parity invariant.
+
+The approval mode (``approvals.mode``) and timeout (``approvals.timeout``)
+must resolve identically on every surface that consults them:
+
+  - the canonical core: ``tools.approval._get_approval_mode`` /
+    ``tools.approval._get_approval_timeout``
+  - the TUI gateway: ``tui_gateway.server._load_approval_mode`` (delegates
+    to the core as of the decision-core migration)
+  - the codex app-server surface: ``agent/codex_runtime.py`` feeds
+    ``auto_approve_*`` from ``tools.approval.is_approval_bypass_active()``,
+    which itself reads the core resolver — so parity there reduces to
+    ``is_approval_bypass_active() == (mode == "off")`` when no yolo
+    source is active.
+
+Historic drift class: tui_gateway re-read config raw and normalized
+locally (see commits f9cd577915, 1e652cca7a, bd246db10d — repeated parity
+re-alignments). This test pins the invariant so drift regressions fail
+loudly instead of silently disagreeing per surface.
+
+There is no per-platform ``approvals.mode`` override in the config schema;
+mode/timeout are global, so the synthetic configs below cover global-set,
+unset (defaults), and malformed values.
+"""
+
+from __future__ import annotations
+
+import importlib
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+
+@pytest.fixture()
+def hermes_home(tmp_path, monkeypatch):
+    home = tmp_path / ".hermes"
+    home.mkdir()
+    monkeypatch.setenv("HERMES_HOME", str(home))
+    return home
+
+
+@pytest.fixture()
+def tui_server():
+    with patch.dict(
+        "sys.modules",
+        {
+            "hermes_cli.env_loader": MagicMock(),
+            "hermes_cli.banner": MagicMock(),
+        },
+    ):
+        yield importlib.import_module("tui_gateway.server")
+
+
+def _write_config(home, yaml_text: str | None) -> None:
+    cfg = home / "config.yaml"
+    if yaml_text is None:
+        if cfg.exists():
+            cfg.unlink()
+    else:
+        cfg.write_text(yaml_text, encoding="utf-8")
+
+
+# (config yaml, expected mode, expected timeout)
+CASES = [
+    pytest.param(None, "smart", 300, id="unset-defaults"),
+    pytest.param(
+        "approvals:\n  mode: manual\n", "manual", 300, id="global-manual"
+    ),
+    pytest.param(
+        "approvals:\n  mode: smart\n  timeout: 120\n",
+        "smart",
+        120,
+        id="global-smart-timeout",
+    ),
+    pytest.param(
+        # YAML 1.1 parses bare OFF as boolean False; the normalizer maps
+        # False -> "off". Both surfaces must agree on that quirk.
+        "approvals:\n  mode: OFF\n  timeout: 45\n",
+        "off",
+        45,
+        id="yaml-bool-off",
+    ),
+    pytest.param(
+        "approvals:\n  mode: bogus-value\n  timeout: not-a-number\n",
+        "manual",
+        300,
+        id="malformed-values",
+    ),
+    pytest.param(
+        "approvals:\n  mode: '  Smart  '\n", "smart", 300, id="whitespace-case"
+    ),
+]
+
+
+def _approval_module():
+    """Resolve tools.approval via sys.modules, not the package attribute.
+
+    The ``tui_server`` fixture's ``patch.dict("sys.modules", ...)`` purges
+    modules imported during its block at teardown; ``from tools import
+    approval`` can then hand back a stale attribute cached on the ``tools``
+    package while the server re-imports a fresh module object. Going
+    through ``importlib.import_module`` keeps the test and the server on
+    the same sys.modules entry.
+    """
+    return importlib.import_module("tools.approval")
+
+
+@pytest.mark.parametrize("yaml_text,expected_mode,expected_timeout", CASES)
+def test_mode_and_timeout_parity_across_surfaces(
+    hermes_home, tui_server, yaml_text, expected_mode, expected_timeout
+):
+    approval_mod = _approval_module()
+
+    _write_config(hermes_home, yaml_text)
+
+    core_mode = approval_mod._get_approval_mode()
+    core_timeout = approval_mod._get_approval_timeout()
+    tui_mode = tui_server._load_approval_mode()
+
+    # Canonical resolver matches expectations.
+    assert core_mode == expected_mode
+    assert core_timeout == expected_timeout
+
+    # TUI surface returns the identical mode (delegation invariant).
+    assert tui_mode == core_mode
+
+    # Codex surface: auto-approve routing is derived from
+    # is_approval_bypass_active(), which must equal (mode == "off")
+    # whenever no yolo source is active in this process.
+    if not approval_mod._YOLO_MODE_FROZEN:
+        with patch.object(
+            approval_mod, "is_current_session_yolo_enabled", return_value=False
+        ):
+            assert approval_mod.is_approval_bypass_active() == (
+                core_mode == "off"
+            )
+
+
+def test_tui_loader_delegates_to_core(hermes_home, tui_server):
+    """The TUI must not re-resolve mode itself — it delegates to the core.
+
+    Pin the delegation seam directly: patching the core resolver changes
+    what the TUI reports, proving there is no independent config read left.
+    """
+    approval_mod = _approval_module()
+
+    with patch.object(approval_mod, "_get_approval_mode", return_value="smart"):
+        assert tui_server._load_approval_mode() == "smart"
+    with patch.object(approval_mod, "_get_approval_mode", return_value="off"):
+        assert tui_server._load_approval_mode() == "off"
+    # Defensive clamp: an out-of-vocabulary value from the core is coerced
+    # to manual rather than leaking an unknown mode to the TUI client.
+    with patch.object(
+        approval_mod, "_get_approval_mode", return_value="weird"
+    ):
+        assert tui_server._load_approval_mode() == "manual"

tui_gateway/server.py

@@ -3575,14 +3575,23 @@ _APPROVAL_MODES = frozenset({"manual", "smart", "off"})
 
 
 def _load_approval_mode() -> str:
-    from hermes_cli.config import DEFAULT_CONFIG, _deep_merge
-    from tools.approval import _normalize_approval_mode
+    """Resolve the effective ``approvals.mode`` for the TUI surface.
 
-    raw_cfg = _load_cfg()
-    cfg = _deep_merge(DEFAULT_CONFIG, raw_cfg if isinstance(raw_cfg, dict) else {})
-    approvals = cfg.get("approvals")
-    raw = approvals.get("mode") if isinstance(approvals, dict) else None
-    mode = _normalize_approval_mode(raw)
+    Delegates to the canonical resolver in ``tools.approval``
+    (``_get_approval_mode``) so mode resolution cannot drift per surface —
+    the same normalization, defaults, and config precedence the approval
+    gate itself uses (see ``tools/approval.py``).
+
+    Previously this re-read the config raw via ``_load_cfg`` +
+    ``_deep_merge(DEFAULT_CONFIG, ...)`` and normalized locally, which
+    could disagree with the gate's own view of the mode (e.g. the
+    canonical ``hermes_cli.config.load_config`` path applies managed-scope
+    overlays and ``${VAR}`` env expansion that the TUI's raw YAML read did
+    not fully mirror).
+    """
+    from tools.approval import _get_approval_mode
+
+    mode = _get_approval_mode()
     return mode if mode in _APPROVAL_MODES else "manual"