diff --git a/tests/tools/test_tool_result_storage.py b/tests/tools/test_tool_result_storage.py
index 6d9b228ce3..f7b8c812da 100644
--- a/tests/tools/test_tool_result_storage.py
+++ b/tests/tools/test_tool_result_storage.py
@@ -1,5 +1,6 @@
"""Tests for tools/tool_result_storage.py -- 3-layer tool result persistence."""
+import json
import os
import subprocess
import sys
@@ -16,6 +17,7 @@ from tools.tool_result_storage import (
PERSISTED_OUTPUT_CLOSING_TAG,
STORAGE_DIR,
_build_persisted_message,
+ _pageable_text,
_resolve_storage_dir,
_safe_result_filename,
_write_to_sandbox,
@@ -515,3 +517,115 @@ class TestSpillover:
assert (spill_dir / "tc_prune_1.txt").exists()
# ── recovery hint in the persisted preview ────────────────────────────
+
+# ── MCP envelope unwrapping (#90426) ──────────────────────────────────
+
+class TestPageableText:
+ """Only the MCP handler's own envelope shape is unwrapped; every other JSON stays opaque."""
+
+ def test_single_result_envelope_is_unwrapped(self):
+ assert _pageable_text(json.dumps({"result": "line one\nline two"})) == "line one\nline two"
+
+ def test_envelope_metadata_is_kept_after_the_text(self):
+ out = _pageable_text(json.dumps({"result": "text\n", "structuredContent": {"answer": 42}}))
+ text, marker, tail = out.partition("\n\n\n")
+ assert text == "text\n"
+ assert marker
+ assert json.loads(tail.split("\n")[0]) == {"structuredContent": {"answer": 42}}
+
+ @pytest.mark.parametrize(
+ "content",
+ [
+ json.dumps({"output": "line\n", "exit_code": 0}), # ordinary tool JSON
+ json.dumps({"result": "line\n", "exit_code": 0}), # unknown sibling key
+ json.dumps([{"result": "line\n"}]), # not an object
+ json.dumps({"result": {"blob": "line\n"}}), # structuredContent-only style
+ json.dumps({"structuredContent": {"a": 1}}), # no model-facing text at all
+ json.dumps({"result": ""}), # empty text
+ "not json at all",
+ "plain text result",
+ ],
+ )
+ def test_unrecognized_content_is_verbatim(self, content):
+ assert _pageable_text(content) == content
+
+class TestMcpEnvelopeSpillover:
+ """An oversized MCP result spills its pageable text, not one escaped JSON line (#90426)."""
+
+ @pytest.fixture(autouse=True)
+ def _isolated_home(self, tmp_path, monkeypatch):
+ monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
+ import tools.tool_result_storage as trs
+ monkeypatch.setattr(trs, "_spillover_pruned_homes", set())
+ yield
+
+ def test_envelope_spills_real_newlines(self):
+ markdown = "# Research guide\n\n" + "pageable content\n" * 4_000
+ envelope = json.dumps({"result": markdown}, ensure_ascii=False)
+ assert len(envelope) > 30_000
+
+ result = maybe_persist_tool_result(
+ content=envelope, tool_name="mcp__archive__read_guide", tool_use_id="tc_mcp_text",
+ env=None, threshold=30_000)
+
+ assert PERSISTED_OUTPUT_TAG in result
+ spill_file = get_spillover_dir() / "tc_mcp_text.txt"
+ assert spill_file.read_text(encoding="utf-8") == markdown
+ # read_file offset/limit is only usable if the preview is the text too.
+ preview = result.split("Preview (first", 1)[1]
+ assert "# Research guide" in preview
+ assert "\\n" not in preview
+
+ def test_metadata_survives_after_the_text(self):
+ markdown = "body line\n" * 5_000
+ envelope = json.dumps({"result": markdown, "structuredContent": {"count": 5_000}})
+ maybe_persist_tool_result(
+ content=envelope, tool_name="mcp__archive__structured", tool_use_id="tc_mcp_meta",
+ env=None, threshold=30_000)
+
+ spill = (get_spillover_dir() / "tc_mcp_meta.txt").read_text(encoding="utf-8")
+ text, _, tail = spill.partition("\n\n\n")
+ assert text == markdown
+ assert json.loads(tail.split("\n")[0]) == {"structuredContent": {"count": 5_000}}
+
+ def test_non_envelope_json_still_verbatim(self):
+ content = json.dumps({"output": "line\n" * 8_000, "exit_code": 0})
+ maybe_persist_tool_result(
+ content=content, tool_name="terminal", tool_use_id="tc_json_verbatim",
+ env=None, threshold=30_000)
+ assert (get_spillover_dir() / "tc_json_verbatim.txt").read_text(encoding="utf-8") == content
+
+ def test_structured_content_only_result_still_verbatim(self):
+ content = json.dumps({"result": {"blob": "z" * 40_000}})
+ maybe_persist_tool_result(
+ content=content, tool_name="mcp__archive__opaque", tool_use_id="tc_mcp_opaque",
+ env=None, threshold=30_000)
+ assert (get_spillover_dir() / "tc_mcp_opaque.txt").read_text(encoding="utf-8") == content
+
+ def test_sandbox_write_carries_pageable_text(self):
+ env = MagicMock() # not a LocalEnvironment -> remote path
+ env.execute.side_effect = [
+ {"output": "", "returncode": 1}, # mounted-path probe: not readable
+ {"output": "", "returncode": 0}, # cat > sandbox path
+ {"output": "", "returncode": 1}, # wc -c probe: no answer -> best effort
+ ]
+ env.get_temp_dir.return_value = ""
+ markdown = "remote line\n" * 5_000
+ maybe_persist_tool_result(
+ content=json.dumps({"result": markdown}), tool_name="mcp__archive__remote",
+ tool_use_id="tc_mcp_remote", env=env, threshold=30_000)
+ assert env.execute.call_args_list[1][1]["stdin_data"] == markdown
+
+ def test_turn_budget_spill_unwraps_envelope(self):
+ """The aggregate layer persists under __budget_enforcement__, so a tool-name gate would
+ miss the very results it must fix; the shape gate still applies."""
+ markdown = "budgeted line\n" * 3_000
+ msgs = [{
+ "role": "tool", "name": "mcp__archive__read", "tool_call_id": "tc_budget_mcp",
+ "content": json.dumps({"result": markdown}),
+ }]
+
+ enforce_turn_budget(msgs, env=None, config=BudgetConfig(turn_budget=10_000))
+
+ assert PERSISTED_OUTPUT_TAG in msgs[0]["content"]
+ assert (get_spillover_dir() / "tc_budget_mcp.txt").read_text(encoding="utf-8") == markdown
diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py
index 958d22a7bd..b35166a39b 100644
--- a/tools/tool_result_storage.py
+++ b/tools/tool_result_storage.py
@@ -6,6 +6,7 @@ ran a terminal), remote backends get the translated in-sandbox path (probed for
else a copy in the sandbox temp dir; (3) ``enforce_turn_budget``."""
import hashlib
+import json
import logging
import os
import re
@@ -23,6 +24,11 @@ STORAGE_DIR = os.path.join(tempfile.gettempdir(), "hermes-results")
SPILLOVER_SUBDIR = "cache/spillover"
SPILLOVER_MAX_AGE_HOURS = 24
_BUDGET_TOOL_NAME = "__budget_enforcement__"
+# The exact key set tools/mcp_tool_handlers.py::_render_call_tool_result emits. A JSON object whose
+# keys stay inside this set is that handler's own envelope, never an arbitrary tool's JSON payload.
+_MCP_ENVELOPE_KEYS = frozenset({"result", "structuredContent", "_meta"})
+_ENVELOPE_METADATA_TAG = ""
+_ENVELOPE_METADATA_CLOSING_TAG = ""
_UNSAFE_RESULT_FILENAME_CHARS = re.compile(r"[^A-Za-z0-9_.-]+")
_MAX_RESULT_FILENAME_STEM = 120
@@ -173,6 +179,47 @@ def generate_preview(content: str, max_chars: int = DEFAULT_PREVIEW_SIZE_CHARS)
return content[:last_nl + 1 if last_nl > max_chars // 2 else max_chars], True
+def _pageable_text(content: str) -> str:
+ """Rewrite an MCP handler result envelope into the text it wraps, else return *content*.
+
+ ``tools/mcp_tool_handlers.py::_render_call_tool_result`` hands the model a JSON string
+ (``{"result": , ...}``) so structured metadata survives inline delivery. Persisting
+ that string verbatim put a multi-hundred-KB document on ONE line with escaped newlines, so
+ the ``read_file`` offset/limit pagination the ```` block recommends could
+ not be used at all (#90426).
+
+ Recognized by SHAPE, not by tool name: the envelope is the only JSON object in the codebase
+ whose keys are a subset of ``{"result", "structuredContent", "_meta"}`` with a non-empty
+ string ``result``. That keeps opaque JSON from any other tool verbatim, and it also covers
+ the aggregate path (``enforce_turn_budget`` persists under ``_BUDGET_TOOL_NAME``, so a
+ ``mcp__``-prefix test would miss exactly the results it has to fix).
+
+ Sibling members are appended after the text in a delimited metadata block instead of being
+ dropped: they are the structured payloads ``_render_call_tool_result`` deliberately keeps
+ for the model (#115430), and the spill file is the only copy left once the envelope is
+ replaced by the preview. Anything unrecognized — unparseable JSON, a non-object, an unknown
+ key, a missing/empty/non-string ``result`` (e.g. a structuredContent-only result, which has
+ no pageable text) — is persisted verbatim, exactly as before.
+ """
+ try:
+ payload = json.loads(content)
+ except (TypeError, ValueError):
+ return content
+ if not isinstance(payload, dict) or not payload or not set(payload) <= _MCP_ENVELOPE_KEYS:
+ return content
+ text = payload.get("result")
+ if not isinstance(text, str) or not text:
+ return content
+ extras = {key: value for key, value in payload.items() if key != "result"}
+ if not extras:
+ return text
+ try:
+ metadata = json.dumps(extras, ensure_ascii=False, indent=1, sort_keys=True, default=str)
+ except (TypeError, ValueError):
+ return text
+ return f"{text}\n\n{_ENVELOPE_METADATA_TAG}\n{metadata}\n{_ENVELOPE_METADATA_CLOSING_TAG}\n"
+
+
def _write_to_sandbox(content: str, remote_path: str, env) -> bool:
"""Write content into the sandbox via env.execute(); True on success. Content goes through
stdin, not the command string: Linux ``MAX_ARG_STRLEN`` caps one argv element at 128 KB,
@@ -253,16 +300,19 @@ def maybe_persist_tool_result(content: str, tool_name: str, tool_use_id: str, en
threshold = config.resolve_threshold(tool_name)
if threshold == float("inf") or len(content) <= threshold:
return content
+ # The size decision above stays on the raw inline result (that is what cost context); the file
+ # and the preview carry the pageable text inside an MCP envelope (#90426).
+ persisted_content = _pageable_text(content)
filename = _safe_result_filename(tool_use_id)
- preview, has_more = generate_preview(content, max_chars=config.preview_size)
+ preview, has_more = generate_preview(persisted_content, max_chars=config.preview_size)
def _persisted(path: str, host_suffix: str = "") -> str:
logger.info("Persisted large tool result: %s (%s, %d chars -> %s%s)",
- tool_name, tool_use_id, len(content), path, host_suffix)
- return _build_persisted_message(preview, has_more, len(content), path)
+ tool_name, tool_use_id, len(persisted_content), path, host_suffix)
+ return _build_persisted_message(preview, has_more, len(persisted_content), path)
# Always persist host-side first: cache/spillover is the single canonical home.
- host_path = _write_to_spillover(content, filename)
+ host_path = _write_to_spillover(persisted_content, filename)
host_side = _is_host_side_env(env)
if host_side and host_path is not None:
return _persisted(host_path)
@@ -274,7 +324,7 @@ def maybe_persist_tool_result(content: str, tool_name: str, tool_use_id: str, en
return _persisted(visible, f" [host: {host_path}]")
remote_path = f"{_resolve_storage_dir(env)}/{filename}"
try:
- if _write_to_sandbox(content, remote_path, env):
+ if _write_to_sandbox(persisted_content, remote_path, env):
return _persisted(remote_path)
except Exception as exc:
logger.warning("Sandbox write failed for %s: %s", tool_use_id, exc)