fix(tui): fall back to checkpoints config when HERMES_TUI_CHECKPOINTS unset (#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. _make_agent now resolves enablement via the new topical sibling tui_gateway/checkpoints.py: the env var (explicit '--tui --checkpoints' opt-in) wins when present, otherwise fall back to the checkpoints config section with DEFAULT_CONFIG defaults, mirroring gateway/run.py _checkpoint_agent_kwargs. The pure _resolve_checkpoint_hash moves to the same sibling to offset the facade's line cap. Reimplements https://github.com/NousResearch/hermes-agent/pull/79743 by @thatssoheil (its branch pre-dates current main and no longer applies); test coverage carried over from that PR.
This commit is contained in:
145
tests/tui_gateway/test_checkpoints_config_fallback.py
Normal file
145
tests/tui_gateway/test_checkpoints_config_fallback.py
Normal file
@@ -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
|
||||
55
tui_gateway/checkpoints.py
Normal file
55
tui_gateway/checkpoints.py
Normal file
@@ -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)}.")
|
||||
@@ -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 ─────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user