fix(tools): large tool results persist to HERMES_HOME/cache/spillover instead of truncating when no sandbox env is active
Sessions that never ran a terminal command (MCP-only, cron, gateway)
have no active sandbox environment, so maybe_persist_tool_result()
got env=None and fell through to the inline-truncate fallback --
a 467K MCP result was cut to a ~1.3K preview with no file written
('Full output could not be saved to sandbox').
Now the host-side cases (env=None or the local backend) write the
spill file directly to $HERMES_HOME/cache/spillover/<id>.txt,
alongside the other Hermes-owned caches instead of littering /tmp.
Remote backends (docker/ssh/modal/daytona) keep the in-sandbox
env.execute() write since read_file resolves in-sandbox there.
Cleanup: the gateway housekeeping loop prunes spillover hourly with
the other media caches, and a once-per-process best-effort prune on
first spill covers CLI-only installs.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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>"
|
||||
PERSISTED_OUTPUT_CLOSING_TAG = "</persisted-output>"
|
||||
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(
|
||||
|
||||
Reference in New Issue
Block a user