diff --git a/tests/tui_gateway/test_checkpoints_config_fallback.py b/tests/tui_gateway/test_checkpoints_config_fallback.py new file mode 100644 index 0000000000..8e4a0698e0 --- /dev/null +++ b/tests/tui_gateway/test_checkpoints_config_fallback.py @@ -0,0 +1,145 @@ +"""Tests for TUI/desktop checkpoint config fallback (#79625). + +Desktop sessions (via ``hermes serve`` → TUI gateway) never set +``HERMES_TUI_CHECKPOINTS``, so agent creation gated filesystem checkpoints +on that env var alone — ``checkpoints.enabled: true`` in config.yaml was +silently ignored on the desktop surface while the CLI and messaging gateway +honored it. The fix makes ``_make_agent`` fall back to the ``checkpoints`` +config section when the env var is unset, mirroring the messaging gateway's +``_checkpoint_agent_kwargs`` (gateway/run.py). +""" + +import os + +import pytest + +from tui_gateway import server + + +class TestLoadCheckpointsEnabled: + def test_defaults_to_false_when_no_config_section(self): + """No ``checkpoints`` section → DEFAULT_CONFIG default (disabled).""" + assert server._load_checkpoints_enabled({}) is False + + def test_honors_config_enabled_true(self): + """``checkpoints: {enabled: true}`` in config.yaml enables checkpoints.""" + assert server._load_checkpoints_enabled({"checkpoints": {"enabled": True}}) is True + + def test_honors_config_enabled_false(self): + assert server._load_checkpoints_enabled({"checkpoints": {"enabled": False}}) is False + + def test_legacy_bool_form(self): + """Legacy ``checkpoints: true`` (bare bool) still enables.""" + assert server._load_checkpoints_enabled({"checkpoints": True}) is True + assert server._load_checkpoints_enabled({"checkpoints": False}) is False + + def test_malformed_section_falls_back_to_default(self): + assert server._load_checkpoints_enabled({"checkpoints": "enabled"}) is False + + def test_none_cfg_falls_back(self): + assert server._load_checkpoints_enabled(None) is False + + +class TestResolveCheckpointsEnabled: + """The env-var-first precedence used by ``_make_agent``.""" + + def test_config_enabled_used_when_env_unset(self, monkeypatch): + """Desktop path: no HERMES_TUI_CHECKPOINTS → config ``enabled: true`` wins.""" + monkeypatch.delenv("HERMES_TUI_CHECKPOINTS", raising=False) + assert server._resolve_checkpoints_enabled( + {"checkpoints": {"enabled": True}} + ) is True + + def test_config_disabled_used_when_env_unset(self, monkeypatch): + monkeypatch.delenv("HERMES_TUI_CHECKPOINTS", raising=False) + assert server._resolve_checkpoints_enabled( + {"checkpoints": {"enabled": False}} + ) is False + + def test_env_var_overrides_config(self, monkeypatch): + """CLI ``--tui --checkpoints`` (env set) still wins over config.""" + monkeypatch.setenv("HERMES_TUI_CHECKPOINTS", "1") + assert server._resolve_checkpoints_enabled( + {"checkpoints": {"enabled": False}} + ) is True + + def test_env_var_false_overrides_config_true(self, monkeypatch): + monkeypatch.setenv("HERMES_TUI_CHECKPOINTS", "0") + assert server._resolve_checkpoints_enabled( + {"checkpoints": {"enabled": True}} + ) is False + + +class TestMakeAgentCheckpointsFallback: + """The env-var → config fallback inside ``_make_agent`` (the wiring). + + Addresses review: unit tests of the helper alone could false-pass if + ``_make_agent`` never actually passed the value through. These drive + ``_make_agent`` end-to-end with a fake ``AIAgent`` that captures + kwargs, proving the config-derived value reaches the agent. + """ + + def _make_agent_with_checkpoints(self, monkeypatch, cfg, env_val="__unset__"): + import types + + captured = {} + + def fake_agent(**kwargs): + captured.update(kwargs) + return types.SimpleNamespace(model=kwargs.get("model")) + + if env_val == "__unset__": + monkeypatch.delenv("HERMES_TUI_CHECKPOINTS", raising=False) + else: + monkeypatch.setenv("HERMES_TUI_CHECKPOINTS", env_val) + monkeypatch.delenv("HERMES_MODEL", raising=False) + monkeypatch.delenv("HERMES_INFERENCE_MODEL", raising=False) + monkeypatch.delenv("HERMES_TUI_PROVIDER", raising=False) + monkeypatch.delenv("HERMES_DESKTOP", raising=False) + monkeypatch.delenv("HERMES_DESKTOP_TERMINAL", raising=False) + monkeypatch.setattr( + server, "_load_cfg", + lambda: {**cfg, "model": {"default": "gpt-5.5", "provider": "openai-codex"}}, + ) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda requested=None, target_model=None: { + "provider": "openai-codex", + "base_url": "https://chatgpt.com/backend-api/codex", + "api_key": "token", + "api_mode": "codex_responses", + "credential_pool": None, + }, + ) + monkeypatch.setattr("run_agent.AIAgent", fake_agent) + monkeypatch.setattr(server, "_load_enabled_toolsets", lambda platform=None: ["file"]) + monkeypatch.setattr(server, "_get_db", lambda: None) + server._make_agent("sid", "session-key") + return captured + + def test_desktop_config_enabled_reaches_agent(self, monkeypatch): + """Issue #79625: config ``enabled: true`` (env unset) → agent has + checkpoints_enabled=True. On the old code this was always False.""" + captured = self._make_agent_with_checkpoints( + monkeypatch, {"checkpoints": {"enabled": True}} + ) + assert captured.get("checkpoints_enabled") is True + + def test_desktop_config_disabled_reaches_agent(self, monkeypatch): + captured = self._make_agent_with_checkpoints( + monkeypatch, {"checkpoints": {"enabled": False}} + ) + assert captured.get("checkpoints_enabled") is False + + def test_cli_env_override_still_wins(self, monkeypatch): + """Legacy ``--tui --checkpoints`` (env '1') overrides config False.""" + captured = self._make_agent_with_checkpoints( + monkeypatch, {"checkpoints": {"enabled": False}}, env_val="1" + ) + assert captured.get("checkpoints_enabled") is True + + def test_cli_env_off_still_wins(self, monkeypatch): + captured = self._make_agent_with_checkpoints( + monkeypatch, {"checkpoints": {"enabled": True}}, env_val="0" + ) + assert captured.get("checkpoints_enabled") is False diff --git a/tui_gateway/checkpoints.py b/tui_gateway/checkpoints.py new file mode 100644 index 0000000000..642d3665d6 --- /dev/null +++ b/tui_gateway/checkpoints.py @@ -0,0 +1,55 @@ +"""Checkpoint-enablement resolution for TUI-created agents (issue #79625). + +Desktop sessions (``hermes serve`` → TUI gateway) never set ``HERMES_TUI_CHECKPOINTS`` +(that env var is only set by the ``hermes --tui --checkpoints`` CLI path), so agent +creation previously gated filesystem checkpointing on the env var alone and silently +ignored ``checkpoints.enabled`` in config.yaml. These helpers resolve enablement the +way the messaging gateway does (``gateway/run.py`` ``_checkpoint_agent_kwargs``): +the env var (explicit CLI opt-in) wins when set, otherwise the ``checkpoints`` +config section applies with ``DEFAULT_CONFIG`` defaults. +""" + +import os + +from utils import is_truthy_value + + +def _load_checkpoints_enabled(cfg: dict | None = None) -> bool: + """Whether filesystem checkpoints are enabled for TUI-created agents. + + Keeps the legacy ``checkpoints: true`` bool form working and falls back to the + ``DEFAULT_CONFIG`` default when the section is absent or malformed. + """ + cp_cfg = (cfg or {}).get("checkpoints", {}) + if isinstance(cp_cfg, bool): + cp_cfg = {"enabled": cp_cfg} + elif not isinstance(cp_cfg, dict): + cp_cfg = {} + from hermes_cli.config import DEFAULT_CONFIG + return bool(cp_cfg.get("enabled", DEFAULT_CONFIG["checkpoints"]["enabled"])) + + +def resolve_checkpoints_enabled(cfg: dict | None = None) -> bool: + """Resolve checkpoint enablement with the env-var override first. + + ``HERMES_TUI_CHECKPOINTS`` (set by ``hermes --tui --checkpoints``) is the + explicit opt-in and wins when present; otherwise fall back to the + ``checkpoints`` config section (issue #79625 — the desktop backend never + sets the env var, so config was being silently ignored). + """ + env_val = os.environ.get("HERMES_TUI_CHECKPOINTS") + if env_val is not None: + return is_truthy_value(env_val) + return _load_checkpoints_enabled(cfg) + + +def _resolve_checkpoint_hash(mgr, cwd: str, ref: str) -> str: + """``/rollback N`` style number → the checkpoint's hash; a real hash passes through.""" + try: + checkpoints = mgr.list_checkpoints(cwd) + idx = int(ref) - 1 + except ValueError: + return ref + if 0 <= idx < len(checkpoints): + return checkpoints[idx].get("hash", ref) + raise ValueError(f"Invalid checkpoint number. Use 1-{len(checkpoints)}.") diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 2ec14e2952..641f2221af 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -37,6 +37,8 @@ from agent.compaction_display import project_compaction_message_for_display # n from agent.skill_commands import describe_skill_invocation # noqa: F401 from agent.conversation_loop import INTERRUPT_WAITING_FOR_MODEL_PREFIX # noqa: F401 from tui_gateway import git_probe +from tui_gateway.checkpoints import (_load_checkpoints_enabled, _resolve_checkpoint_hash, + resolve_checkpoints_enabled as _resolve_checkpoints_enabled) # noqa: F401 from tui_gateway._env import env_float, env_int from tui_gateway.turn_marker import clear_turn_marker, marker_writer_state, read_turn_marker, record_turn_start # noqa: F401 from tui_gateway.contracts import registry as _contracts @@ -2683,7 +2685,7 @@ def _make_agent( # Builds that run before the record exists (branch, eager resume, compute host) pass it explicitly. user_id=auth_user_id if auth_user_id is not None else _session_auth_user_id(session), session_db=session_db if session_db is not None else _get_db(), ephemeral_system_prompt=system_prompt or None, - checkpoints_enabled=is_truthy_value(os.environ.get("HERMES_TUI_CHECKPOINTS")), + checkpoints_enabled=_resolve_checkpoints_enabled(cfg), pass_session_id=is_truthy_value(os.environ.get("HERMES_TUI_PASS_SESSION_ID")), skip_context_files=ignore_rules, skip_memory=ignore_rules, fallback_model=_load_fallback_model(), # The resolved provider's request body (a custom entry's extra_body), as the CLI/cron/gateway pass it. @@ -2814,17 +2816,6 @@ def _with_checkpoints(session, fn): return fn(session["agent"]._checkpoint_mgr, _session_cwd(session)) -def _resolve_checkpoint_hash(mgr, cwd: str, ref: str) -> str: - try: - checkpoints = mgr.list_checkpoints(cwd) - idx = int(ref) - 1 - except ValueError: - return ref - if 0 <= idx < len(checkpoints): - return checkpoints[idx].get("hash", ref) - raise ValueError(f"Invalid checkpoint number. Use 1-{len(checkpoints)}.") - - # ── Methods: session ─────────────────────────────────────────────────