From 9b2745149be432d6661ef9a73c52d81f969888a9 Mon Sep 17 00:00:00 2001 From: DaoyuanLi2816 Date: Thu, 6 Aug 2026 22:57:27 -0700 Subject: [PATCH 1/2] test(tool-output): stop expressing "unwritable path" as a magic absolute path `test_returns_none_on_invalid_path` and `test_fallback_when_disk_write_fails` both need an `outputs_path` that `os.makedirs` refuses to create, so they can reach `_externalize`'s `except OSError: return None` branch. They spell that as the literal path `/dev/null/cannot-mkdir-here`, which only works where `/dev/null` is a character device. On Windows it is an ordinary relative path, so `os.makedirs` succeeds, both tests fail, and the suite writes real files to `C:\dev\null\cannot-mkdir-here\ .tool-results\` -- outside any temporary directory, at the drive root. Running the backend suite a few times leaves dozens of stray files behind. The comment above the first test records that this is the second time the same assumption has broken: `/nonexistent/...` was silently created by `mkdir -p` when CI ran as root in a container, and `/dev/null/...` was the fix. Both encode a guess about the environment rather than the condition under test. Use a regular file as the parent component instead. Creating a directory below a file fails with an `OSError` subclass on every platform -- `NotADirectoryError` (errno 20) on POSIX, `FileNotFoundError` (errno 2) on Windows -- so the branch is reached deterministically, and the path lives inside the test's own `TemporaryDirectory`, so nothing is written outside it. Verified both spellings on Linux (WSL Ubuntu, non-root) and Windows; only the file-as-parent form fails on both. The two tests still have teeth: dropping `_externalize`'s `except OSError` guard makes both fail rather than pass. Tests only -- no production code or documented behaviour changes. --- .../test_tool_output_budget_middleware.py | 51 +++++++++++++------ 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/backend/tests/test_tool_output_budget_middleware.py b/backend/tests/test_tool_output_budget_middleware.py index 82e940adf3..6c9b41441e 100644 --- a/backend/tests/test_tool_output_budget_middleware.py +++ b/backend/tests/test_tool_output_budget_middleware.py @@ -8,6 +8,7 @@ from __future__ import annotations +import contextlib import json import os import tempfile @@ -66,6 +67,28 @@ def _make_request(tool_name: str = "remote_executor", tool_call_id: str = "tc-1" ) +@contextlib.contextmanager +def _unwritable_outputs_path(): + """Yield an ``outputs_path`` that ``os.makedirs`` cannot create, on any platform. + + The parent component is a regular file, so creating a directory below it + fails with an ``OSError`` subclass everywhere (``NotADirectoryError`` on + POSIX, ``FileNotFoundError`` on Windows) and nothing is written outside the + temporary directory. + + This deliberately avoids expressing "unwritable" as a magic absolute path. + ``/nonexistent/...`` was creatable by root in the CI container, and its + replacement ``/dev/null/...`` relies on ``/dev/null`` being a character + device, which is only true on POSIX -- on Windows it is an ordinary + relative path that ``os.makedirs`` happily creates at the drive root. + """ + with tempfile.TemporaryDirectory() as tmpdir: + blocker = os.path.join(tmpdir, "not-a-directory") + with open(blocker, "w", encoding="utf-8") as f: + f.write("placeholder") + yield os.path.join(blocker, "outputs") + + def _tm(content: str = "ok", name: str = "tool", tool_call_id: str = "tc-1") -> ToolMessage: return ToolMessage(content=content, name=name, tool_call_id=tool_call_id) @@ -160,20 +183,15 @@ def test_writes_file_and_returns_virtual_path(self): assert f.read() == "full content here" def test_returns_none_on_invalid_path(self): - # ``/dev/null`` is a character device on both Linux and macOS, so - # ``os.makedirs`` cannot create any subdirectory under it for any - # user (including root). The previously-used ``/nonexistent/...`` - # path was silently created by ``mkdir -p`` when the test process - # ran as root inside the CI container, which made this test fail - # in CI independently of the externalization logic under test. - path = _externalize( - "data", - tool_name="test", - tool_call_id="tc-1", - outputs_path="/dev/null/cannot-mkdir-here", - storage_subdir=".tool-results", - ) - assert path is None + with _unwritable_outputs_path() as outputs_path: + path = _externalize( + "data", + tool_name="test", + tool_call_id="tc-1", + outputs_path=outputs_path, + storage_subdir=".tool-results", + ) + assert path is None def test_txt_extension_for_unknown_tool(self): with tempfile.TemporaryDirectory() as tmpdir: @@ -710,9 +728,10 @@ def test_fallback_when_disk_write_fails(self): mw = ToolOutputBudgetMiddleware(config=config) content = "x" * 500 msg = _tm(content, name="tool") - req = _make_request(outputs_path="/dev/null/cannot-mkdir-here") - result = mw.wrap_tool_call(req, lambda _: msg) + with _unwritable_outputs_path() as outputs_path: + req = _make_request(outputs_path=outputs_path) + result = mw.wrap_tool_call(req, lambda _: msg) assert isinstance(result, ToolMessage) assert "omitted from tool output" in result.content From 39d00825d9c6d3cfb57eac187b48ca6e2ac34f8b Mon Sep 17 00:00:00 2001 From: DaoyuanLi2816 Date: Sun, 9 Aug 2026 22:19:41 -0700 Subject: [PATCH 2/2] test(tool-output): touch the blocker file instead of writing content Only its existence as a regular file matters for os.makedirs to fail below it, so touch() states that directly. --- backend/tests/test_tool_output_budget_middleware.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/tests/test_tool_output_budget_middleware.py b/backend/tests/test_tool_output_budget_middleware.py index 6c9b41441e..da64761395 100644 --- a/backend/tests/test_tool_output_budget_middleware.py +++ b/backend/tests/test_tool_output_budget_middleware.py @@ -11,6 +11,7 @@ import contextlib import json import os +import pathlib import tempfile from types import SimpleNamespace @@ -83,9 +84,8 @@ def _unwritable_outputs_path(): relative path that ``os.makedirs`` happily creates at the drive root. """ with tempfile.TemporaryDirectory() as tmpdir: - blocker = os.path.join(tmpdir, "not-a-directory") - with open(blocker, "w", encoding="utf-8") as f: - f.write("placeholder") + blocker = pathlib.Path(tmpdir) / "not-a-directory" + blocker.touch() yield os.path.join(blocker, "outputs")