diff --git a/gateway/run.py b/gateway/run.py index 657f706fd1..9970c20780 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -29620,6 +29620,7 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop cleanup_screenshot_cache, cleanup_video_cache, ) + from tools.tool_result_storage import cleanup_spillover_cache from hermes_cli.debug import _sweep_expired_pastes IMAGE_CACHE_EVERY = 60 # ticks — once per hour at default 60s interval @@ -29638,6 +29639,7 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop ("Audio", cleanup_audio_cache), ("Video", cleanup_video_cache), ("Screenshot", cleanup_screenshot_cache), + ("Spillover", cleanup_spillover_cache), ) logger.info("Gateway housekeeping started (interval=%ds)", interval) diff --git a/tests/tools/test_tool_result_storage.py b/tests/tools/test_tool_result_storage.py index 44198ca661..4cca1a315b 100644 --- a/tests/tools/test_tool_result_storage.py +++ b/tests/tools/test_tool_result_storage.py @@ -18,8 +18,10 @@ from tools.tool_result_storage import ( _resolve_storage_dir, _safe_result_filename, _write_to_sandbox, + cleanup_spillover_cache, enforce_turn_budget, generate_preview, + get_spillover_dir, maybe_persist_tool_result, ) @@ -320,3 +322,126 @@ class TestPerToolThresholds: assert val == 100_000 except ImportError: pytest.skip("file_tools not importable in test env") + + +# ── Host-side spillover ($HERMES_HOME/cache/spillover) ──────────────── + +class TestSpillover: + @pytest.fixture(autouse=True) + def _isolated_home(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + # Reset the once-per-process prune flag so each test is independent. + import tools.tool_result_storage as trs + monkeypatch.setattr(trs, "_spillover_pruned_once", False) + yield + + def test_env_none_persists_to_spillover(self): + """No active sandbox env (MCP-only / cron session) must persist + host-side instead of inline-truncating — the guglielmo bundle bug.""" + content = "x" * 60_000 + result = maybe_persist_tool_result( + content=content, + tool_name="tool_call", + tool_use_id="tc_mcp_1", + env=None, + threshold=30_000, + ) + assert PERSISTED_OUTPUT_TAG in result + assert "could not be saved" not in result + spill_file = get_spillover_dir() / "tc_mcp_1.txt" + assert spill_file.exists() + assert spill_file.read_text(encoding="utf-8") == content + assert str(spill_file) in result + + def test_local_env_persists_to_spillover_not_sandbox(self): + """LocalEnvironment routes host-side: no env.execute() shell-out.""" + from tools.environments.local import LocalEnvironment + + env = MagicMock(spec=LocalEnvironment) + content = "y" * 60_000 + result = maybe_persist_tool_result( + content=content, + tool_name="terminal", + tool_use_id="tc_local_1", + env=env, + threshold=30_000, + ) + assert PERSISTED_OUTPUT_TAG in result + assert (get_spillover_dir() / "tc_local_1.txt").exists() + env.execute.assert_not_called() + + def test_remote_env_still_uses_sandbox_write(self): + """Non-local envs keep the in-sandbox write path.""" + env = MagicMock() # not a LocalEnvironment + env.execute.return_value = {"output": "", "returncode": 0} + env.get_temp_dir.return_value = "/tmp" + content = "z" * 60_000 + result = maybe_persist_tool_result( + content=content, + tool_name="terminal", + tool_use_id="tc_remote_1", + env=env, + threshold=30_000, + ) + assert PERSISTED_OUTPUT_TAG in result + env.execute.assert_called_once() + assert not (get_spillover_dir() / "tc_remote_1.txt").exists() + + def test_spillover_write_failure_falls_back_to_inline(self, monkeypatch): + import tools.tool_result_storage as trs + monkeypatch.setattr(trs, "_write_to_spillover", lambda *a, **k: None) + content = "w" * 60_000 + result = maybe_persist_tool_result( + content=content, + tool_name="tool_call", + tool_use_id="tc_fail_1", + env=None, + threshold=30_000, + ) + assert "could not be saved" in result + assert PERSISTED_OUTPUT_TAG not in result + + def test_cleanup_spillover_cache_removes_old_keeps_new(self): + import os + import time as _time + + spill_dir = get_spillover_dir() + spill_dir.mkdir(parents=True, exist_ok=True) + old = spill_dir / "old.txt" + new = spill_dir / "new.txt" + old.write_text("old") + new.write_text("new") + stale = _time.time() - (48 * 3600) + os.utime(old, (stale, stale)) + + removed = cleanup_spillover_cache(max_age_hours=24) + + assert removed == 1 + assert not old.exists() + assert new.exists() + + def test_cleanup_missing_dir_returns_zero(self): + assert cleanup_spillover_cache() == 0 + + def test_first_spill_prunes_expired_files(self): + """The once-per-process prune fires on the first host-side spill.""" + import os + import time as _time + + spill_dir = get_spillover_dir() + spill_dir.mkdir(parents=True, exist_ok=True) + old = spill_dir / "ancient.txt" + old.write_text("ancient") + stale = _time.time() - (48 * 3600) + os.utime(old, (stale, stale)) + + maybe_persist_tool_result( + content="v" * 60_000, + tool_name="tool_call", + tool_use_id="tc_prune_1", + env=None, + threshold=30_000, + ) + + assert not old.exists() + assert (spill_dir / "tc_prune_1.txt").exists() diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py index b9ceccf75b..acb7b4c29d 100644 --- a/tools/tool_result_storage.py +++ b/tools/tool_result_storage.py @@ -8,12 +8,28 @@ Defense against context-window overflow operates at three levels: 2. **Per-result persistence** (maybe_persist_tool_result): After a tool returns, if its output exceeds the tool's registered threshold - (registry.get_max_result_size), the full output is written INTO THE - SANDBOX temp dir (for example /tmp/hermes-results/{tool_use_id}.txt on - standard Linux, or $TMPDIR/hermes-results/{tool_use_id}.txt on Termux) - via env.execute(). The in-context content is replaced with a preview + - file path reference. The model can read_file to access the full output - on any backend. + (registry.get_max_result_size), the full output is persisted and the + in-context content is replaced with a preview + file path reference. + + Where it lands depends on the backend: + + - **Local backend (or no active env):** written host-side to + ``$HERMES_HOME/cache/spillover/{tool_use_id}.txt`` — alongside the + other Hermes-owned caches (images, audio, documents, ...) instead of + littering the OS temp dir. This path needs no sandbox environment, + so it also works for sessions that never ran a terminal command + (MCP-only, cron, gateway) — previously those hit the inline-truncate + fallback because ``get_active_env()`` returned None until the first + terminal call created an environment. + - **Remote backends (docker/ssh/modal/daytona):** written INTO THE + SANDBOX temp dir (for example /tmp/hermes-results/{id}.txt) via + env.execute(), because HERMES_HOME does not exist inside the + container and read_file resolves in-sandbox there. + + The spillover dir is pruned two ways: the gateway housekeeping loop + sweeps it hourly with the other media caches, and a once-per-process + best-effort prune runs on the first spill so CLI-only installs (which + never run gateway housekeeping) self-clean too. 3. **Per-turn aggregate budget** (enforce_turn_budget): After all tool results in a single assistant turn are collected, if the total exceeds @@ -27,6 +43,8 @@ import logging import os import re import shlex +import threading +import time import uuid from tools.budget_config import ( @@ -39,11 +57,103 @@ logger = logging.getLogger(__name__) PERSISTED_OUTPUT_TAG = "" PERSISTED_OUTPUT_CLOSING_TAG = "" STORAGE_DIR = "/tmp/hermes-results" +SPILLOVER_SUBDIR = "cache/spillover" +SPILLOVER_MAX_AGE_HOURS = 24 HEREDOC_MARKER = "HERMES_PERSIST_EOF" _BUDGET_TOOL_NAME = "__budget_enforcement__" _UNSAFE_RESULT_FILENAME_CHARS = re.compile(r"[^A-Za-z0-9_.-]+") _MAX_RESULT_FILENAME_STEM = 120 +_spillover_prune_lock = threading.Lock() +_spillover_pruned_once = False + + +def get_spillover_dir(): + """Return $HERMES_HOME/cache/spillover as a Path (not created).""" + from hermes_constants import get_hermes_home + + return get_hermes_home() / SPILLOVER_SUBDIR + + +def cleanup_spillover_cache(max_age_hours: int = SPILLOVER_MAX_AGE_HOURS) -> int: + """Delete spillover files older than *max_age_hours*. + + Same contract as the ``cleanup_*_cache`` helpers in + ``gateway.platforms.base`` — returns the number of files removed — + so the gateway housekeeping loop can prune this dir on the same + hourly cadence as the media caches. + """ + cutoff = time.time() - (max_age_hours * 3600) + removed = 0 + try: + entries = list(get_spillover_dir().iterdir()) + except OSError: + return 0 + for f in entries: + try: + if f.is_file() and f.stat().st_mtime < cutoff: + f.unlink() + removed += 1 + except OSError: + continue + return removed + + +def _prune_spillover_once() -> None: + """Best-effort prune, at most once per process. + + The gateway housekeeping loop prunes hourly, but CLI-only installs + never run it — without this, spillover files would accumulate + forever on pure-CLI setups. + """ + global _spillover_pruned_once + with _spillover_prune_lock: + if _spillover_pruned_once: + return + _spillover_pruned_once = True + try: + removed = cleanup_spillover_cache() + if removed: + logger.debug("Pruned %d expired spillover file(s)", removed) + except Exception as exc: + logger.debug("Spillover prune failed: %s", exc) + + +def _is_host_side_env(env) -> bool: + """True when the spill file should be written by this process directly. + + Covers ``env=None`` (no sandbox environment active — e.g. a session + that has not run a terminal command yet) and the local backend + (where env.execute() runs on this same host anyway). Remote backends + (docker/ssh/modal/daytona) return False: their read_file resolves + inside the sandbox, so the spill must be written there. + """ + if env is None: + return True + try: + from tools.environments.local import LocalEnvironment + + return isinstance(env, LocalEnvironment) + except Exception: + return False + + +def _write_to_spillover(content: str, filename: str): + """Write content host-side to $HERMES_HOME/cache/spillover. + + Returns the absolute path string on success, None on failure. + """ + try: + spill_dir = get_spillover_dir() + spill_dir.mkdir(parents=True, exist_ok=True) + path = spill_dir / filename + path.write_text(content, encoding="utf-8", errors="replace") + except OSError as exc: + logger.warning("Spillover write failed for %s: %s", filename, exc) + return None + _prune_spillover_once() + return str(path) + def _resolve_storage_dir(env) -> str: """Return the best temp-backed storage dir for this environment.""" @@ -174,11 +284,24 @@ def maybe_persist_tool_result( if len(content) <= effective_threshold: return content - storage_dir = _resolve_storage_dir(env) - remote_path = f"{storage_dir}/{_safe_result_filename(tool_use_id)}" + filename = _safe_result_filename(tool_use_id) preview, has_more = generate_preview(content, max_chars=config.preview_size) - if env is not None: + # Host-side path: no active sandbox env (MCP-only / cron / gateway + # sessions that never ran a terminal command) or the local backend. + # Write into $HERMES_HOME/cache/spillover with the other Hermes-owned + # caches instead of shelling out to /tmp. + if _is_host_side_env(env): + host_path = _write_to_spillover(content, filename) + if host_path is not None: + logger.info( + "Persisted large tool result: %s (%s, %d chars -> %s)", + tool_name, tool_use_id, len(content), host_path, + ) + return _build_persisted_message(preview, has_more, len(content), host_path) + elif env is not None: + storage_dir = _resolve_storage_dir(env) + remote_path = f"{storage_dir}/{filename}" try: if _write_to_sandbox(content, remote_path, env): logger.info(