From 90d0dfb4f026326ae80c191f7283aaed7a6c0014 Mon Sep 17 00:00:00 2001 From: Ahmed Allam Date: Sat, 25 Jul 2026 22:41:42 +0000 Subject: [PATCH] fix(context): keep tool-output retrieval reliable and stream pages Exempt read_tool_output from result-bounding so paging a large stored output isn't re-spilled under a new id, byte-cap each page in place, and stream the requested window with islice instead of loading the whole file per page. --- strix/agents/factory.py | 7 ++++++- strix/tools/output_store.py | 22 ++++++++++++++++++---- tests/test_agent_factory_shell.py | 11 +++++++++++ tests/test_output_store.py | 19 +++++++++++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/strix/agents/factory.py b/strix/agents/factory.py index c411c2ea..b36f5360 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -528,7 +528,12 @@ def build_strix_agent( tools = [*_BASE_TOOLS, *agent_tools, agent_finish] _ensure_unique_tool_names(tools) tools = [ - _with_bounded_result(tool) if isinstance(tool, FunctionTool) else tool for tool in tools + # read_tool_output pages already-stored output; re-bounding it would + # re-spill a large page under a new id and make retrieval unusable. + _with_bounded_result(tool) + if isinstance(tool, FunctionTool) and tool is not read_tool_output + else tool + for tool in tools ] logger.info( diff --git a/strix/tools/output_store.py b/strix/tools/output_store.py index 51fffa9e..bbe814c1 100644 --- a/strix/tools/output_store.py +++ b/strix/tools/output_store.py @@ -10,6 +10,7 @@ truncated detail is bounded in history but never lost. from __future__ import annotations +import itertools import logging import re import uuid @@ -27,6 +28,12 @@ _SPILL_NOTICE = ( _OUTPUT_ID_RE = re.compile(r"^[0-9a-f]{32}$") _DEFAULT_STORE_DIR = Path.home() / ".strix" / "tool-output" +# Ceilings for a single retrieval page, so a page of very long lines can't +# itself overflow history. Applied without spilling (no new output_id), or +# paging would loop forever. +_PAGE_MAX_LINES = 2_000 +_PAGE_MAX_BYTES = 50 * 1024 + # Single-key holder so the configured path can be swapped per scan without a # module-level ``global`` rebind. _config: dict[str, Path] = {} @@ -183,11 +190,18 @@ def read_stored_output(output_id: str, *, offset: int = 0, limit: int = 2_000) - if not path.is_file(): return f"No stored output for output_id={output_id!r} (it may have expired)." - lines = path.read_text(encoding="utf-8").split("\n") start = max(0, offset) - window = lines[start : start + max(1, limit)] - shown = "\n".join(window) - remaining = len(lines) - (start + len(window)) + count = min(max(1, limit), _PAGE_MAX_LINES) + with path.open(encoding="utf-8") as handle: + # Stream to the window instead of materialising the whole file per page. + for _ in itertools.islice(handle, start): + pass + window = [line.rstrip("\n") for line in itertools.islice(handle, count)] + remaining = sum(1 for _ in handle) + + # Bound the page's byte size with a plain notice (never a spill id) so a + # page of very long lines stays within history without re-triggering spill. + shown = bound_text("\n".join(window), max_lines=_PAGE_MAX_LINES, max_bytes=_PAGE_MAX_BYTES) if remaining > 0: shown += f"\n\n[... {remaining} more lines; call read_tool_output(output_id=" shown += f'"{output_id}", offset={start + len(window)}) to continue ...]' diff --git a/tests/test_agent_factory_shell.py b/tests/test_agent_factory_shell.py index 22b662c7..b3b4f6a1 100644 --- a/tests/test_agent_factory_shell.py +++ b/tests/test_agent_factory_shell.py @@ -110,3 +110,14 @@ async def test_chat_completions_filesystem_custom_tool_becomes_function_tool() - factory._configure_filesystem_tools(toolset, chat_completions=True) assert isinstance(toolset.read_file, FunctionTool) + + +def test_read_tool_output_is_not_result_bounded() -> None: + # Every other FunctionTool gets the result-bounding wrapper, but the + # retrieval tool must be exempt or paging a large stored output would be + # re-spilled under a new id. + agent = factory.build_strix_agent(is_root=True) + by_name = {t.name: t for t in agent.tools} + + assert getattr(by_name["read_tool_output"], "_strix_bounded", False) is False + assert getattr(by_name["think"], "_strix_bounded", False) is True diff --git a/tests/test_output_store.py b/tests/test_output_store.py index 818572a3..78224e7c 100644 --- a/tests/test_output_store.py +++ b/tests/test_output_store.py @@ -114,6 +114,25 @@ def test_read_stored_output_paginates(tmp_path: Path) -> None: assert "offset=10" in page +def test_read_stored_output_page_is_bounded_without_new_spill(tmp_path: Path) -> None: + # A page of very long lines must be byte-capped in place, never re-spilled + # under a fresh output_id (which would make paging loop forever). + configure_output_store(tmp_path) + text = "\n".join("z" * 5_000 for _ in range(50)) + output_id = re.search( + r'output_id="([0-9a-f]{32})"', + bound_and_store(text, max_lines=4, max_bytes=1_000), + ) + assert output_id is not None + + page = read_stored_output(output_id.group(1), offset=0, limit=2_000) + + assert len(page.encode("utf-8")) <= 60 * 1024 + # No brand-new spill id in the returned page. + ids = re.findall(r'output_id="([0-9a-f]{32})"', page) + assert ids == [output_id.group(1)] or ids == [] + + def test_read_stored_output_rejects_traversal(tmp_path: Path) -> None: configure_output_store(tmp_path) assert "Invalid output_id" in read_stored_output("../../etc/passwd")