From 09e657793eb9dd0b508ee25861a1d1271e67869b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:40:30 -0700 Subject: [PATCH] feat: MCP tool results spill at 50K and carry upstream-elision warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composio-style MCP servers return un-paginated 22-47K-char payloads that sail under the generic 100K per-result spillover threshold, bloating context and ballooning per-turn reasoning time on long conversations. Competitors cap harder (OpenCode/pi 50KB, Claude Code 30K, Codex ~10K tokens). Three changes: - mcp_* tools spill at a tighter 50K default (BudgetConfig.mcp_result_size, config-overridable via tool_budget.mcp_result_size_chars; pinned and per-tool overrides still win; capped by the context-scaled default). - The persisted-output preview now teaches recovery: page the saved file with read_file or process with execute_code instead of re-requesting the same data from the remote API. - Untrusted/MCP string results are scanned (bounded, first 64KB) for provider-side elision markers ('...N more items', "has_more": true, 'saved to sandbox', data_preview) and get ONE cache-safe incompleteness notice appended at result-construction time, before untrusted wrapping — so the model stops treating provider-elided enumerations as complete. - Hard 2M-char allocation cap in mcp_tool.py (text, error, and structuredContent paths) so a pathological multi-MB server payload is bounded before it propagates, while ordinary large results reach spillover intact. Distilled from #56060/#56072/#56511 (issue #56059); supersedes their 50K lossy truncation with spillover-friendly semantics. Docs: configuration.md spillover-budget section + cli-config.yaml.example. Co-authored-by: Stoltemberg <215755014+Stoltemberg@users.noreply.github.com> Co-authored-by: AlexFucuson9 <295703459+AlexFucuson9@users.noreply.github.com> Co-authored-by: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com> --- agent/tool_dispatch_helpers.py | 72 +++++++++++++++++- agent/tool_executor.py | 5 +- cli-config.yaml.example | 13 ++++ tests/agent/test_tool_dispatch_helpers.py | 90 +++++++++++++++++++++++ tests/tools/test_budget_config.py | 65 ++++++++++++++++ tests/tools/test_mcp_result_size_limit.py | 69 +++++++++++++++++ tests/tools/test_tool_result_storage.py | 19 +++++ tools/budget_config.py | 69 ++++++++++++++++- tools/mcp_tool.py | 55 +++++++++++++- tools/tool_result_storage.py | 7 +- website/docs/user-guide/configuration.md | 15 ++++ 11 files changed, 473 insertions(+), 6 deletions(-) create mode 100644 tests/tools/test_mcp_result_size_limit.py diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index af0970accb..0c4259b656 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -557,7 +557,11 @@ def make_tool_result_message( The outer list itself is rebuilt rather than returned by identity, so callers should compare by value, not by ``is``. """ - wrapped = _maybe_wrap_untrusted(name, content) + # Order matters: detect provider-side elision on the RAW content and + # append the notice first, THEN wrap — so the notice lives inside the + # untrusted block next to the data it describes, appended exactly once + # at construction time (cache-safe). + wrapped = _maybe_wrap_untrusted(name, _maybe_append_elision_notice(name, content)) message = stamp_message_timestamp({ "role": "tool", "name": name, @@ -608,6 +612,70 @@ def _is_untrusted_tool(name: Optional[str]) -> bool: return any(name.startswith(p) for p in _UNTRUSTED_TOOL_PREFIXES) +# --- Upstream-elision detection -------------------------------------------- +# +# Some MCP servers elide data SERVER-SIDE and mark the elision inside the +# payload itself (e.g. Composio: '...13 more items' inside a JSON array, +# '"has_more": true', 'Complete response was large (N tokens). Full data +# saved to sandbox in /mnt/files/...', 'data_preview' envelopes). Because the +# result looks structurally complete, models treat the visible slice as the +# whole dataset and falsely claim completeness. When one of these markers is +# present, we append ONE compact notice at result-construction time — before +# the message enters history, never mutated later, so prompt caching is safe. + +# Conservative patterns only: each one is an explicit provider-side "there is +# more data than what you can see" signal, not a generic truncation heuristic. +_UPSTREAM_ELISION_PATTERNS = ( + re.compile(r"\.\.\.\s*\d+\s+more\s+items?", re.IGNORECASE), + re.compile(r'"has_more"\s*:\s*true', re.IGNORECASE), + re.compile(r"saved to sandbox", re.IGNORECASE), + re.compile(r"data_preview", re.IGNORECASE), +) + +# Results smaller than this can't meaningfully hide an elided enumeration — +# skip the scan entirely so tiny results pay nothing. +_ELISION_SCAN_MIN_CHARS = 1_000 + +# Bound the regex scan: markers appear near the elided structure, which for +# the payload sizes that matter (20-50K) is always inside the first 64KB. +_ELISION_SCAN_MAX_CHARS = 65_536 + +_UPSTREAM_ELISION_NOTICE = ( + '\n[hermes note: this result contains provider-side elision markers ' + '(e.g. "...N more items" / has_more:true). The data shown is INCOMPLETE ' + '— page/fetch the remainder before treating any enumeration as complete.]' +) + + +def _detect_upstream_elision(content: Any) -> bool: + """True when a string tool result carries provider-side elision markers. + + Cheap and safe by construction: non-string content is never scanned, + results under ``_ELISION_SCAN_MIN_CHARS`` short-circuit, and the regex + scan is capped at the first ``_ELISION_SCAN_MAX_CHARS`` chars. + """ + if not isinstance(content, str): + return False + if len(content) < _ELISION_SCAN_MIN_CHARS: + return False + window = content[:_ELISION_SCAN_MAX_CHARS] + return any(p.search(window) for p in _UPSTREAM_ELISION_PATTERNS) + + +def _maybe_append_elision_notice(name: str, content: Any) -> Any: + """Append the incompleteness notice to untrusted string results that + embed upstream elision markers. Returns ``content`` unchanged otherwise. + + Runs on the RAW result before untrusted-wrapping so the notice sits with + the data it describes, and only at result-construction time (cache-safe). + """ + if not _is_untrusted_tool(name): + return content + if _detect_upstream_elision(content): + return content + _UPSTREAM_ELISION_NOTICE + return content + + def _tool_output_risk_metadata(name: str, content: Any) -> Optional[Dict[str, Any]]: """Classify textual attacker-controlled output without retaining a copy. @@ -729,5 +797,7 @@ __all__ = [ "_extract_landed_file_mutation_paths", "_extract_error_preview", "_trajectory_normalize_msg", + "_detect_upstream_elision", + "_maybe_append_elision_notice", "make_tool_result_message", ] diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 90226b02e1..2eb69c6124 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -87,7 +87,10 @@ def _budget_for_agent(agent) -> BudgetConfig: """ try: ctx = getattr(getattr(agent, "context_compressor", None), "context_length", None) - return budget_for_context_window(int(ctx)) if ctx else DEFAULT_BUDGET + # budget_for_context_window(None) (rather than DEFAULT_BUDGET) so the + # config-driven MCP threshold override still applies when the context + # length isn't resolvable. + return budget_for_context_window(int(ctx) if ctx else None) except Exception: return DEFAULT_BUDGET diff --git a/cli-config.yaml.example b/cli-config.yaml.example index d2228c76a7..87aad0ae94 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -671,6 +671,19 @@ compression: # To pin a specific model/provider for compression summaries, use the # auxiliary section below (auxiliary.compression.provider / model). +# ============================================================================= +# Tool-result budget (optional) +# ============================================================================= +# Controls when a large tool result is spilled to disk (full output saved to +# $HERMES_HOME/cache/spillover, preview + path kept in context). MCP tool +# results (tools named mcp_*) spill at a tighter 50,000-char threshold than +# the generic 100K default: MCP servers routinely return un-paginated 20-50K +# payloads that bloat context and slow every subsequent turn. Nothing is +# lost — the full result is on disk and readable with read_file. +# +# tool_budget: +# mcp_result_size_chars: 50000 # per-result spillover threshold for mcp_* tools + # ============================================================================= # Anthropic prompt caching TTL # ============================================================================= diff --git a/tests/agent/test_tool_dispatch_helpers.py b/tests/agent/test_tool_dispatch_helpers.py index 34c0a6dda6..c56c362d36 100644 --- a/tests/agent/test_tool_dispatch_helpers.py +++ b/tests/agent/test_tool_dispatch_helpers.py @@ -215,3 +215,93 @@ class TestFileMutationTargets: }, ) assert targets == ["old/name.py", "new/name.py"] + + +class TestUpstreamElisionDetection: + """Provider-side elision markers get a one-line incompleteness notice.""" + + def _payload(self, marker: str) -> str: + return '{"items": ["' + "x" * 1_200 + '"], ' + marker + "}" + + def test_more_items_marker_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision(self._payload('"note": "... 13 more items"')) + + def test_has_more_true_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision(self._payload('"has_more": true')) + + def test_saved_to_sandbox_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision( + "y" * 1_100 + " Complete response was large. Full data saved to sandbox in /mnt/files/x.json" + ) + + def test_data_preview_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision(self._payload('"data_preview": {}')) + + def test_has_more_false_not_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert not _detect_upstream_elision(self._payload('"has_more": false')) + + def test_plain_large_result_not_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert not _detect_upstream_elision("z" * 5_000) + + def test_non_string_content_skipped(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert not _detect_upstream_elision(None) + assert not _detect_upstream_elision({"has_more": True}) + assert not _detect_upstream_elision([{"type": "text", "text": "... 5 more items"}]) + + def test_short_results_short_circuit(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + # Marker present but under the 1K scan floor -> skipped. + assert not _detect_upstream_elision('"has_more": true') + + def test_marker_beyond_scan_cap_not_matched(self): + from agent.tool_dispatch_helpers import ( + _ELISION_SCAN_MAX_CHARS, + _detect_upstream_elision, + ) + content = "a" * (_ELISION_SCAN_MAX_CHARS + 10) + '"has_more": true' + assert not _detect_upstream_elision(content) + + +class TestElisionNoticeWiring: + """Notice appended once at construction time, before untrusted wrapping.""" + + def _elided(self) -> str: + return '{"items": ["' + "x" * 1_200 + '"], "has_more": true}' + + def test_notice_appended_for_mcp_tool(self): + from agent.tool_dispatch_helpers import ( + _UPSTREAM_ELISION_NOTICE, + _maybe_append_elision_notice, + ) + out = _maybe_append_elision_notice("mcp_composio_search", self._elided()) + assert out.endswith(_UPSTREAM_ELISION_NOTICE) + + def test_trusted_tool_never_annotated(self): + from agent.tool_dispatch_helpers import _maybe_append_elision_notice + content = self._elided() + assert _maybe_append_elision_notice("terminal", content) is content + + def test_untrusted_without_markers_unchanged(self): + from agent.tool_dispatch_helpers import _maybe_append_elision_notice + content = "y" * 2_000 + assert _maybe_append_elision_notice("mcp_x", content) is content + + def test_notice_inside_untrusted_wrapper(self): + """Order: detect on raw -> append notice -> wrap. The notice must sit + INSIDE the untrusted block, and the message is built once (cache-safe).""" + from agent.tool_dispatch_helpers import make_tool_result_message + msg = make_tool_result_message("mcp_composio_search", self._elided(), "call_1") + content = msg["content"] + assert content.startswith("") + assert "INCOMPLETE" in content + assert content.index("hermes note") < content.index("") + # Exactly one notice. + assert content.count("hermes note") == 1 diff --git a/tests/tools/test_budget_config.py b/tests/tools/test_budget_config.py index 118bca3ecb..2ef47369c3 100644 --- a/tests/tools/test_budget_config.py +++ b/tests/tools/test_budget_config.py @@ -173,3 +173,68 @@ class TestBudgetForContextWindow: threshold = cfg.resolve_threshold("mcp_firecrawl_firecrawl_search") assert threshold < huge_len assert cfg.default_result_size < huge_len + + +# --------------------------------------------------------------------------- +# MCP-prefix threshold (mcp_result_size) +# --------------------------------------------------------------------------- + + +class TestMcpPrefixThreshold: + """mcp_* tools get the tighter 50K default, config-overridable.""" + + def test_default_mcp_threshold_is_50k(self): + from tools.budget_config import DEFAULT_MCP_RESULT_SIZE_CHARS + assert DEFAULT_MCP_RESULT_SIZE_CHARS == 50_000 + assert DEFAULT_BUDGET.resolve_threshold("mcp_composio_search_tools") == 50_000 + + def test_non_mcp_tools_keep_generic_default(self): + assert DEFAULT_BUDGET.resolve_threshold("some_random_tool") == DEFAULT_RESULT_SIZE_CHARS + + def test_pinned_wins_over_mcp_prefix(self): + with patch.dict(PINNED_THRESHOLDS, {"mcp_pinned_tool": float("inf")}): + assert DEFAULT_BUDGET.resolve_threshold("mcp_pinned_tool") == float("inf") + + def test_tool_override_wins_over_mcp_prefix(self): + cfg = BudgetConfig(tool_overrides={"mcp_special": 75_000}) + assert cfg.resolve_threshold("mcp_special") == 75_000 + + def test_mcp_threshold_capped_by_scaled_default(self): + """On a small model the scaled default_result_size caps the MCP value.""" + cfg = BudgetConfig(default_result_size=20_000, mcp_result_size=50_000) + assert cfg.resolve_threshold("mcp_anything") == 20_000 + + def test_mcp_threshold_never_exceeds_default_result_size(self): + cfg = BudgetConfig(default_result_size=100_000, mcp_result_size=999_999) + assert cfg.resolve_threshold("mcp_anything") == 100_000 + + def test_config_override_via_hermes_home(self, tmp_path, monkeypatch): + (tmp_path / "config.yaml").write_text( + "tool_budget:\n mcp_result_size_chars: 30000\n" + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + cfg = budget_for_context_window(None) + assert cfg.resolve_threshold("mcp_composio_multi_execute") == 30_000 + # Generic tools are untouched by the MCP knob. + assert cfg.default_result_size == DEFAULT_RESULT_SIZE_CHARS + + def test_config_override_survives_window_scaling(self, tmp_path, monkeypatch): + (tmp_path / "config.yaml").write_text( + "tool_budget:\n mcp_result_size_chars: 30000\n" + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + cfg = budget_for_context_window(200_000) + assert cfg.mcp_result_size == 30_000 + + def test_malformed_config_falls_back_to_default(self, tmp_path, monkeypatch): + (tmp_path / "config.yaml").write_text("tool_budget: not-a-mapping\n") + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + cfg = budget_for_context_window(None) + assert cfg.resolve_threshold("mcp_x_y") == 50_000 + + def test_scaled_small_window_caps_mcp_threshold(self, tmp_path, monkeypatch): + """A tiny model's scaled default_result_size caps even the MCP value.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) # no config.yaml + cfg = budget_for_context_window(16_384) # scaled default < 50K + assert cfg.default_result_size < 50_000 + assert cfg.resolve_threshold("mcp_tool") == cfg.default_result_size diff --git a/tests/tools/test_mcp_result_size_limit.py b/tests/tools/test_mcp_result_size_limit.py new file mode 100644 index 0000000000..da9e11a308 --- /dev/null +++ b/tests/tools/test_mcp_result_size_limit.py @@ -0,0 +1,69 @@ +"""Regression tests for the MCP hard result cap (#56059). + +MCP tool results had no allocation bound — a buggy or malicious MCP server +could return multi-megabyte text that floods memory and context before the +budget/spillover layer sees it. The hard cap truncates only pathological +payloads (over 2M chars by default) with a 40% head / 60% tail split; +ordinary large results pass through untouched so the 50K MCP spillover +threshold (tools/budget_config.py) can preserve them in full on disk. + +Test shape adapted from PR #56511 (Tranquil-Flow); cap semantics differ — +see _MCP_HARD_RESULT_CAP_CHARS in tools/mcp_tool.py. +""" + +from __future__ import annotations + +from tools.mcp_tool import _MCP_HARD_RESULT_CAP_CHARS, _truncate_mcp_text_result + + +class TestTruncateMcpTextResult: + def test_short_result_unchanged(self): + text = "x" * 100 + assert _truncate_mcp_text_result(text) == text + + def test_exact_limit_unchanged(self): + text = "y" * 100 + assert _truncate_mcp_text_result(text, max_chars=100) == text + + def test_spillover_sized_result_passes_untouched(self): + """A 60K result (over the 50K spillover threshold) is NOT truncated + here — the budget layer must receive it intact so spillover can + preserve the full payload on disk.""" + text = "z" * 60_000 + assert _truncate_mcp_text_result(text) == text + + def test_pathological_result_is_truncated(self): + text = "z" * (_MCP_HARD_RESULT_CAP_CHARS + 500_000) + result = _truncate_mcp_text_result(text) + assert len(result) < len(text) + assert "TRUNCATED" in result + + def test_truncation_preserves_head_and_tail(self): + head_marker = "HEAD_MARKER_START" + tail_marker = "TAIL_MARKER_END" + text = head_marker + "x" * 5000 + tail_marker + result = _truncate_mcp_text_result(text, max_chars=200) + assert result.startswith(head_marker) + assert result.endswith(tail_marker) + + def test_truncation_includes_omitted_count(self): + text = "a" * 5000 + result = _truncate_mcp_text_result(text, max_chars=100) + assert "4,900" in result # 5000 - 100 omitted + assert "5,000" in result # total original length + + def test_truncation_uses_40_60_head_tail_split(self): + text = "H" * 40 + "M" * 5000 + "T" * 60 + result = _truncate_mcp_text_result(text, max_chars=100) + assert result[:40] == "H" * 40 + assert result[-60:] == "T" * 60 + + def test_empty_result_unchanged(self): + assert _truncate_mcp_text_result("") == "" + + def test_hard_cap_sits_above_spillover_threshold(self): + """The hard cap must stay far above the MCP spillover threshold so + spillover, not lossy truncation, handles ordinary large results.""" + from tools.budget_config import DEFAULT_MCP_RESULT_SIZE_CHARS + + assert _MCP_HARD_RESULT_CAP_CHARS > DEFAULT_MCP_RESULT_SIZE_CHARS * 10 diff --git a/tests/tools/test_tool_result_storage.py b/tests/tools/test_tool_result_storage.py index bdb50a7361..c72d8f933f 100644 --- a/tests/tools/test_tool_result_storage.py +++ b/tests/tools/test_tool_result_storage.py @@ -480,3 +480,22 @@ class TestSpillover: assert not old.exists() assert (spill_dir / "tc_prune_1.txt").exists() + + +# ── recovery hint in the persisted preview ──────────────────────────── + +class TestRecoveryHint: + def test_preview_teaches_recovery_not_refetch(self): + msg = _build_persisted_message( + preview="preview text", + has_more=True, + original_size=60_000, + file_path="/tmp/hermes-results/r.txt", + ) + assert "Recovery:" in msg + assert "execute_code" in msg + assert "re-request" in msg + # Structure preserved: tag, size, path, read_file guidance all intact. + assert msg.startswith(PERSISTED_OUTPUT_TAG) + assert msg.endswith(PERSISTED_OUTPUT_CLOSING_TAG) + assert "read_file" in msg diff --git a/tools/budget_config.py b/tools/budget_config.py index 8e47479446..c746bdb3b3 100644 --- a/tools/budget_config.py +++ b/tools/budget_config.py @@ -18,6 +18,55 @@ DEFAULT_RESULT_SIZE_CHARS: int = 100_000 DEFAULT_TURN_BUDGET_CHARS: int = 200_000 DEFAULT_PREVIEW_SIZE_CHARS: int = 1_500 +# Tighter default per-result threshold for MCP tools (name prefix ``mcp_``). +# +# MCP servers routinely return un-paginated 20-50K-char payloads (tool +# discovery catalogs, batched executions) that sail under the generic 100K +# threshold and silently bloat context — in agentic evals this measurably +# ballooned per-turn reasoning time on long conversations. Competitor +# harnesses cap harder (OpenCode 50KB, pi 50KB, Claude Code 30K chars, +# Codex ~10K tokens); 50K chars keeps parity with the strictest general- +# purpose caps while spillover (unlike truncation) preserves the full +# payload on disk. Overridable via ``tool_budget.mcp_result_size_chars`` +# in config.yaml. +DEFAULT_MCP_RESULT_SIZE_CHARS: int = 50_000 + +# Tool-name prefix that identifies MCP-served tools (same prefix the +# untrusted-content wrapper keys on in agent/tool_dispatch_helpers.py). +MCP_TOOL_PREFIX: str = "mcp_" + + +def _configured_mcp_result_size() -> int: + """Read ``tool_budget.mcp_result_size_chars`` from the active config.yaml. + + Reads ``$HERMES_HOME/config.yaml`` (falling back to ``~/.hermes/config.yaml`` + when HERMES_HOME is unset) so the value is hermetically testable. Fully + guarded: any error, missing file, missing key, or non-positive value + returns the built-in default. The ``tool_budget:`` block name is shared + with the wider configurable-caps proposal (#80508) so the two can merge + without a key rename. + """ + import os + + try: + import yaml + + home = os.environ.get("HERMES_HOME") or os.path.expanduser("~/.hermes") + path = os.path.join(home, "config.yaml") + if os.path.isfile(path): + with open(path, encoding="utf-8") as fh: + data = yaml.safe_load(fh) or {} + block = data.get("tool_budget") + if isinstance(block, dict): + raw = block.get("mcp_result_size_chars") + if raw is not None: + value = int(raw) + if value > 0: + return value + except Exception: + pass + return DEFAULT_MCP_RESULT_SIZE_CHARS + @dataclass(frozen=True) class BudgetConfig: @@ -32,12 +81,21 @@ class BudgetConfig: default_result_size: int = DEFAULT_RESULT_SIZE_CHARS turn_budget: int = DEFAULT_TURN_BUDGET_CHARS preview_size: int = DEFAULT_PREVIEW_SIZE_CHARS + mcp_result_size: int = DEFAULT_MCP_RESULT_SIZE_CHARS tool_overrides: Dict[str, int] = field(default_factory=dict) def resolve_threshold(self, tool_name: str) -> int | float: """Resolve the persistence threshold for a tool. - Priority: pinned -> tool_overrides -> registry per-tool -> default. + Priority: pinned -> tool_overrides -> mcp_ prefix -> registry + per-tool -> default. + + MCP tools (``mcp_`` prefix) get a tighter default threshold + (``mcp_result_size``, 50K chars) because MCP servers return + un-paginated payloads with no per-tool registry entry to constrain + them. The value is additionally capped at ``default_result_size`` + so a context-scaled budget for a small model still constrains MCP + results the same way it constrains registry values. The registry per-tool value is capped at ``default_result_size`` so a context-scaled budget (small model) actually constrains tools that @@ -50,6 +108,8 @@ class BudgetConfig: return PINNED_THRESHOLDS[tool_name] if tool_name in self.tool_overrides: return self.tool_overrides[tool_name] + if tool_name.startswith(MCP_TOOL_PREFIX): + return min(self.mcp_result_size, self.default_result_size) from tools.registry import registry registry_value = registry.get_max_result_size(tool_name, default=self.default_result_size) if registry_value == float("inf"): @@ -95,8 +155,12 @@ def budget_for_context_window(context_length: int | None) -> BudgetConfig: small models proportionally to their window, floored so a usable preview always survives. """ + mcp_result_size = _configured_mcp_result_size() + if not context_length or context_length <= 0: - return DEFAULT_BUDGET + if mcp_result_size == DEFAULT_MCP_RESULT_SIZE_CHARS: + return DEFAULT_BUDGET + return BudgetConfig(mcp_result_size=mcp_result_size) window_chars = context_length * _CHARS_PER_TOKEN per_result = int(window_chars * _PER_RESULT_WINDOW_FRACTION) @@ -111,4 +175,5 @@ def budget_for_context_window(context_length: int | None) -> BudgetConfig: default_result_size=per_result, turn_budget=per_turn, preview_size=DEFAULT_PREVIEW_SIZE_CHARS, + mcp_result_size=mcp_result_size, ) diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 93b4f2585c..08b15dcd42 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -121,6 +121,41 @@ from tools.ansi_strip import strip_unicode_tags logger = logging.getLogger(__name__) + +# Hard allocation ceiling for a single MCP text payload (chars). This is the +# FIRST line of defense against a buggy or malicious MCP server returning +# multi-megabyte text: without it the full payload is allocated, JSON-encoded +# and handed downstream before the budget/spillover layer ever sees it +# (#56059). It deliberately sits far ABOVE the budget layer's 50K MCP +# spillover threshold (tools/budget_config.py) so ordinary large results +# reach spillover INTACT — spilled to disk in full, preview in context — +# while only pathological multi-MB floods are lossy-truncated here. +# +# Distilled from #56060 (Stoltemberg), #56072 (AlexFucuson9) and #56511 +# (Tranquil-Flow), which capped at get_max_bytes() (50K) — correct +# protection, but at that level it would truncate before spillover could +# preserve the data. The 40% head / 60% tail split is #56511's shape. +_MCP_HARD_RESULT_CAP_CHARS = 2_000_000 + + +def _truncate_mcp_text_result(text: str, max_chars: int = _MCP_HARD_RESULT_CAP_CHARS) -> str: + """Bound pathological MCP text before it propagates (#56059). + + Results at or under ``max_chars`` pass through unchanged; oversized text + keeps a 40% head / 60% tail split with an omission notice in between. + """ + if len(text) <= max_chars: + return text + head_chars = int(max_chars * 0.4) + tail_chars = max_chars - head_chars + omitted = len(text) - head_chars - tail_chars + return ( + text[:head_chars] + + f"\n\n... [MCP RESULT TRUNCATED - {omitted:,} chars omitted " + f"out of {len(text):,} total] ...\n\n" + + text[-tail_chars:] + ) + # Upper bound for the OSV malware preflight during stdio MCP startup. The # check makes a blocking urllib HTTPS call whose own timeout can fail to # interrupt a stalled SSL handshake, which froze the asyncio event loop and @@ -5816,7 +5851,9 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): if res_text: error_text += str(res_text) return tool_error(_sanitize_error( - error_text or "MCP tool returned an error" + _truncate_mcp_text_result( + error_text or "MCP tool returned an error" + ) )) # Collect text from content blocks. MCP tool results can also @@ -5868,6 +5905,10 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): ) text_result = "\n".join(parts) if parts else "" + # Hard-cap pathological payloads before they propagate (#56059); + # ordinary large results pass untouched to the spillover layer. + text_result = _truncate_mcp_text_result(text_result) + # Combine content + structuredContent when both are present. # MCP spec: content is model-oriented (text), structuredContent # is machine-oriented (JSON metadata). For an AI agent, content @@ -5885,6 +5926,18 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): # vendor-namespaced keys (`com.example.mcp/...`) pass through — # their semantics belong to the server. structured = mcp_field(result, "structured_content", "structuredContent") + # Cap structuredContent too — a malicious server could flood + # context via a multi-MB JSON payload (#56059). When the + # serialized form exceeds the hard cap, replace it with the + # truncated string (head + tail preserved) so it degrades + # gracefully instead of flooding downstream. + if structured is not None: + try: + _structured_json = json.dumps(structured, ensure_ascii=False, default=str) + except (TypeError, ValueError): + _structured_json = None + if _structured_json is not None and len(_structured_json) > _MCP_HARD_RESULT_CAP_CHARS: + structured = _truncate_mcp_text_result(_structured_json) meta = _strip_reserved_meta_keys(mcp_field(result, "meta", "meta")) if structured is not None or meta is not None: payload: Dict[str, Any] = {} diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py index 47bf3799f8..ff731a5b8f 100644 --- a/tools/tool_result_storage.py +++ b/tools/tool_result_storage.py @@ -281,7 +281,12 @@ def _build_persisted_message( msg = f"{PERSISTED_OUTPUT_TAG}\n" msg += f"This tool result was too large ({original_size:,} characters, {size_str}).\n" msg += f"Full output saved to: {file_path}\n" - msg += "Use the read_file tool with offset and limit to access specific sections of this output.\n\n" + msg += "Use the read_file tool with offset and limit to access specific sections of this output.\n" + msg += ( + "Recovery: page through the saved file with read_file (offset/limit) or " + "process it with execute_code — do NOT re-request the same data from the " + "remote API; the full result is already on disk.\n\n" + ) msg += f"Preview (first {len(preview)} chars):\n" msg += preview if has_more: diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index e031cb639e..7b9b43e455 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -746,6 +746,21 @@ tool_output: max_lines: 500 ``` +### Tool-Result Spillover Budget + +Separately from truncation, oversized tool *results* are spilled to disk rather than cut: the full output is saved under `$HERMES_HOME/cache/spillover/` and the in-context content is replaced by a preview plus the saved file's path (readable with `read_file` using `offset`/`limit`, or processable with `execute_code`). The generic per-result spillover threshold is 100,000 chars, scaled down automatically for small-context models. + +MCP tool results (tools named `mcp_*`) spill at a tighter **50,000-char** default: MCP servers routinely return large un-paginated payloads (tool-discovery catalogs, batched executions) that would otherwise sit under the generic threshold and bloat context on every subsequent turn. Nothing is lost — the full result is preserved on disk. Override the threshold via: + +```yaml +tool_budget: + mcp_result_size_chars: 50000 # per-result spillover threshold for mcp_* tools +``` + +The MCP threshold is always capped at the (possibly context-scaled) generic per-result threshold, so raising it cannot exceed what the active model's window allows. + +Hermes also flags **provider-side elision**: when an MCP or web tool result embeds its own truncation markers (`...N more items`, `"has_more": true`, "saved to sandbox" notes), a one-line notice is appended to the result warning that the visible data is incomplete and should be paged/fetched before treating any enumeration as complete. + ## Global Toolset Disable To suppress specific toolsets across the CLI and every gateway platform in one