From 238bf39d521bc1127e0a04d622b44d11617432dd Mon Sep 17 00:00:00 2001 From: pufit Date: Fri, 4 Sep 2026 16:35:46 -0400 Subject: [PATCH] Raise the CLI stdout line cap so image reads stop aborting turns The Agent SDK caps one stream-json line at 1 MiB by default and kills the reader (and the turn) when a line exceeds it. A Read of a screenshot routinely does: the CLI ships the base64 twice per line and re-encodes anything over 2000 px. Expose agent.cli_max_message_bytes (64 MiB), pass it as max_buffer_size, refuse images that cannot fit under it in the Read hook, and name the cause in the error message. --- docs/config.md | 1 + nerve/agent/backends/claude.py | 14 ++++++- nerve/agent/backends/images.py | 38 ++++++++++++++++- nerve/agent/engine.py | 15 +++++++ nerve/config.py | 12 ++++++ tests/test_engine.py | 75 ++++++++++++++++++++++++++++++++- tests/test_image_validation.py | 76 ++++++++++++++++++++++++++++++++++ 7 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 tests/test_image_validation.py diff --git a/docs/config.md b/docs/config.md index af122d04..e94850cd 100644 --- a/docs/config.md +++ b/docs/config.md @@ -907,6 +907,7 @@ from any working directory: | `agent.max_concurrent` | int | `32` | Max concurrent agent sessions | | `agent.cache_ttl` | string | `"5m"` | Prompt-cache write TTL policy: `5m` (status quo), `1h` (always request the 1-hour TTL), or `auto` (per session at client-build time: sparse-cadence sessions — persistent crons, wakeup loops, spaced chats — get `1h`; dense sessions stay on `5m`). Per-cron-job override via `cache_ttl` in jobs.yaml. See `nerve/agent/cache_policy.py` | | `agent.cache_ttl_excluded_models` | list | `[]` | Model-name substrings that never request the 1h TTL | +| `agent.cli_max_message_bytes` | int | `67108864` (64 MiB) | Upper bound on one stream-json message read from the Claude CLI subprocess (the Agent SDK's `max_buffer_size`). The SDK's own default is 1 MiB, and a line over the bound aborts the whole turn — image tool results routinely exceed it: the Read tool ships the base64 image twice per line and re-encodes anything over 2000 px, so a 300 KB screenshot can become a 1.3 MB line. The Read image validator also refuses images whose encoded line cannot fit under this bound, so a too-big image fails as a tool error instead of killing the turn | | `agent.agent_teams` | bool | `true` | Set `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1` for the CLI subprocess, which registers the `SendMessage` tool. The Agent tool advertises `SendMessage` for resuming a sub-agent whether or not the flag is set, so with it off the model reaches for a tool that does not exist. Nerve loads no settings files (`setting_sources=[]`), so the env dict is the flag's only route in. Teammates stay opt-in per turn and cost a full context window each; the CLI cannot restore in-process teammates when a session's client is recycled (idle timeout, restart, crash retry) | | `agent.prompt_rewrite.enabled` | bool | `true` | Offer the first-prompt rewrite feature in the web UI (per-user toggle lives in the composer) | | `agent.prompt_rewrite.model` | string | `""` | Model for prompt rewriting (empty = `agent.model`, the chat model) | diff --git a/nerve/agent/backends/claude.py b/nerve/agent/backends/claude.py index 3880b441..99ff3caa 100644 --- a/nerve/agent/backends/claude.py +++ b/nerve/agent/backends/claude.py @@ -555,6 +555,10 @@ def _cli_stderr(line: str) -> None: hooks=hooks, stderr=_cli_stderr, extra_args=extra_args, + # Per-line cap on the CLI's stdout. The SDK default (1 MiB) is + # fatal to the turn the moment a Read returns a screenshot — see + # AgentConfig.cli_max_message_bytes. + max_buffer_size=config.agent.cli_max_message_bytes, # No allowed_tools — can_use_tool handles permissions. # External MCP server tools are discovered at connection time, # so we can't enumerate them upfront. @@ -759,6 +763,7 @@ def _build_hooks(self, spec: SessionSpec) -> dict: """ session_id = spec.session_id captured_files: set[str] = set() + max_message_bytes = self.config.agent.cli_max_message_bytes async def _snapshot_hook(hook_input, tool_use_id, context): """PreToolUse: capture file content before Edit/Write/NotebookEdit.""" @@ -785,12 +790,17 @@ async def _validate_image_hook(hook_input, tool_use_id, context): encodes them into image content blocks. If the file isn't a valid image, the API rejects it with 400 and the bad block persists in the CLI's history — an unrecoverable poison loop. - Check magic bytes and size *before* Read executes. + A valid image can still be too big for the SDK transport: the + result travels as one stream-json line, and a line over + ``max_buffer_size`` aborts the whole turn. Check magic bytes + and both size bounds *before* Read executes. """ tool_input = hook_input.get("tool_input", {}) file_path = tool_input.get("file_path", "") - error = validate_image_file(file_path) + error = validate_image_file( + file_path, max_message_bytes=max_message_bytes, + ) if error: logger.warning( "Blocked Read of invalid image for session %s: %s", diff --git a/nerve/agent/backends/images.py b/nerve/agent/backends/images.py index c9f1d56f..1cc1c9da 100644 --- a/nerve/agent/backends/images.py +++ b/nerve/agent/backends/images.py @@ -14,6 +14,14 @@ # Anthropic API image limit; a sane general ceiling for codex too. MAX_IMAGE_BYTES = 5 * 1024 * 1024 # 5 MB +# How the Claude CLI puts a Read image on its stdout: the base64 payload +# appears twice in one stream-json line (the tool_result content block and +# the top-level ``tool_use_result``), and the CLI never emits more than this +# much base64 for one image — it downsizes to fit. +CLI_MAX_IMAGE_BASE64 = 5 * 1024 * 1024 +IMAGE_WIRE_COPIES = 2 +IMAGE_WIRE_OVERHEAD = 4096 # JSON envelope, ids, timestamps + IMAGE_EXTENSIONS = {".png", ".jpg", ".jpeg", ".gif", ".webp"} # Magic byte signatures for supported image formats. @@ -29,13 +37,31 @@ } -def validate_image_file(file_path: str) -> str | None: +def estimate_image_wire_bytes(size: int) -> int: + """Lower bound on the stream-json line a Read of a ``size``-byte image makes. + + A lower bound, not an estimate of the typical case: the CLI re-encodes + images wider or taller than 2000 px, and the re-encoded PNG is often + larger than the file on disk (a 269 KB screenshot became a 1.3 MB line). + """ + b64 = min(4 * ((size + 2) // 3), CLI_MAX_IMAGE_BASE64) + return IMAGE_WIRE_COPIES * b64 + IMAGE_WIRE_OVERHEAD + + +def validate_image_file( + file_path: str, max_message_bytes: int | None = None, +) -> str | None: """Validate that a file with an image extension contains actual image data. Returns None if valid, or an error string describing the problem. This prevents the runtime from base64-encoding non-image files (e.g. HTML redirect pages saved with a .png extension) and poisoning the conversation context with an unprocessable image block. + + ``max_message_bytes`` is the transport's per-message cap (the Agent + SDK's ``max_buffer_size``): an image whose encoded line cannot fit + under it is refused up front, because a line over the cap aborts the + whole turn instead of failing the one tool call. """ from pathlib import Path @@ -58,6 +84,16 @@ def validate_image_file(file_path: str) -> str | None: f"The Anthropic API rejects images larger than 5 MB." ) + if max_message_bytes and estimate_image_wire_bytes(size) > max_message_bytes: + return ( + f"Image {file_path} ({size / 1024:.0f} KB on disk) would exceed the " + f"agent transport's {max_message_bytes / (1024 * 1024):.0f} MiB " + f"per-message limit once base64-encoded, and reading it would abort " + f"the whole turn. Downscale it first (e.g. " + f"`sips -Z 1600 in.png --out small.png`) or raise " + f"agent.cli_max_message_bytes." + ) + # Check magic bytes magic_specs = IMAGE_MAGIC.get(ext, []) if not magic_specs: diff --git a/nerve/agent/engine.py b/nerve/agent/engine.py index 48d41361..c23d3911 100644 --- a/nerve/agent/engine.py +++ b/nerve/agent/engine.py @@ -2821,6 +2821,11 @@ async def _run_inner( "Could not process image" in err_str or "Could not process document" in err_str ) + # One oversized stream-json line (a large image tool result is + # the routine case — see AgentConfig.cli_max_message_bytes) kills + # the SDK's reader and with it the turn. The CLI transcript is + # intact, so the session stays resumable; say what happened. + is_oversized = "exceeded maximum buffer size" in err_str # Preserve resumability on crashed turns (parity with the old # any-message early capture): the terminal event never arrived, # so pull the native id off the live client. _finalize_turn's @@ -2850,6 +2855,16 @@ async def _run_inner( session_id, {"sdk_session_id": None}, ) + if is_oversized: + limit_mib = self.config.agent.cli_max_message_bytes / (1024 * 1024) + error_msg = ( + f"Agent error: a tool result exceeded the CLI transport's " + f"{limit_mib:.0f} MiB per-message limit (usually a large " + "image read), so the turn was aborted. The session can " + "continue; downscale the image or raise " + "agent.cli_max_message_bytes." + ) + await broadcaster.broadcast_error(session_id, error_msg) # Schedule memorization BEFORE mark_error clears connected_at — # the frozen bound keeps coverage intact. Scheduled, not diff --git a/nerve/config.py b/nerve/config.py index dc192688..ef3004e2 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -860,6 +860,15 @@ class AgentConfig: # behaviour: turns can hang forever). 900s comfortably covers a 10-min # Bash tool call plus SDK round-trips while still catching real hangs. cli_idle_timeout_seconds: int = 900 + # Upper bound on ONE stream-json message read from the CLI subprocess (the + # Agent SDK's ``max_buffer_size``). The SDK's own default is 1 MiB and a + # line over it is fatal to the SDK's reader, i.e. it aborts the whole turn. + # Image tool results are the routine offender: the Read tool ships the + # base64 image TWICE per line (content block + ``tool_use_result``) and + # re-encodes anything over 2000 px, so a 300 KB screenshot can become a + # 1.3 MB line. The CLI caps one image at 5 MiB of base64 (~10.5 MB per + # line worst case); 64 MiB leaves headroom for document blocks. + cli_max_message_bytes: int = 64 * 1024 * 1024 # When True, background sub-agents (the Agent tool with run_in_background, or # background Bash) get the SAME auto-approved tool permissions as foreground # agents, via a PreToolUse hook that pre-approves all non-interactive tools. @@ -923,6 +932,9 @@ def from_dict(cls, d: dict) -> AgentConfig: d.get("cache_ttl_excluded_models") ), cli_idle_timeout_seconds=d.get("cli_idle_timeout_seconds", 900), + cli_max_message_bytes=int( + d.get("cli_max_message_bytes", 64 * 1024 * 1024) + ), background_agent_permissions=d.get("background_agent_permissions", True), agent_teams=d.get("agent_teams", True), prompt_rewrite=PromptRewriteConfig.from_dict(d.get("prompt_rewrite") or {}), diff --git a/tests/test_engine.py b/tests/test_engine.py index 724659e8..63b8f53e 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -312,6 +312,33 @@ def test_claude_options_pass_resume_session_at_only_for_forks(tmp_path): assert plain.resume_session_at is None +def test_claude_options_set_stdout_buffer_from_config(tmp_path): + """The SDK's 1 MiB per-line default aborts the turn on any large image + tool result; the configured cap must reach ClaudeAgentOptions.""" + def _opts(cfg): + backend = ClaudeBackend(SimpleNamespace( + config=lambda: cfg, + claude_plugins=lambda: [], + )) + spec = SessionSpec( + session_id="buf-opts", source="web", model=cfg.agent.model, + effort="high", system_prompt="p", cwd=str(tmp_path), + ) + with patch.object(backend, "_build_mcp_servers", return_value={}), \ + patch.object(backend, "_build_hooks", return_value={}): + return backend._build_options(spec) + + default = NerveConfig.from_dict({"workspace": str(tmp_path)}) + assert default.agent.cli_max_message_bytes == 64 * 1024 * 1024 + assert _opts(default).max_buffer_size == 64 * 1024 * 1024 + + custom = NerveConfig.from_dict({ + "workspace": str(tmp_path), + "agent": {"cli_max_message_bytes": 8 * 1024 * 1024}, + }) + assert _opts(custom).max_buffer_size == 8 * 1024 * 1024 + + @pytest.mark.asyncio async def test_receive_turn_disabled_when_timeout_zero(): """idle_timeout <= 0 disables the timeout (legacy behaviour).""" @@ -799,11 +826,15 @@ def test_falls_back_to_unresolved_path(self, tmp_path, monkeypatch): # ClaudeBackend._build_hooks — background-agent permission parity # --------------------------------------------------------------------------- -def _make_hook_backend(background_agent_permissions: bool) -> ClaudeBackend: +def _make_hook_backend( + background_agent_permissions: bool, + cli_max_message_bytes: int = 64 * 1024 * 1024, +) -> ClaudeBackend: """Minimal backend stub for exercising _build_hooks's PreToolUse wiring.""" config = SimpleNamespace( agent=SimpleNamespace( background_agent_permissions=background_agent_permissions, + cli_max_message_bytes=cli_max_message_bytes, ), ) return ClaudeBackend(SimpleNamespace(config=lambda: config)) @@ -824,6 +855,48 @@ def _catch_all_grant_hook(hooks: dict): return None +def _read_validator_hook(backend: ClaudeBackend): + for matcher in backend._build_hooks(_hook_spec("sess-img"))["PreToolUse"]: + if matcher.matcher == "Read": + return matcher.hooks[0] + raise AssertionError("Read image validator hook not registered") + + +def _fake_png(tmp_path: Path, size: int) -> str: + png = tmp_path / "shot.png" + png.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\0" * size) + return str(png) + + +class TestReadHookTransportBound: + """A valid image can still be too big for the SDK transport: its result + is one stream-json line, and a line over max_buffer_size aborts the + whole turn. The Read validator refuses it up front instead.""" + + @pytest.mark.asyncio + async def test_denies_image_that_cannot_fit_the_configured_cap(self, tmp_path): + # 600 KB → ~820 KB of base64, shipped twice per line → > 1 MiB. + path = _fake_png(tmp_path, 600 * 1024) + hook = _read_validator_hook( + _make_hook_backend(True, cli_max_message_bytes=1024 * 1024), + ) + out = await hook( + {"tool_name": "Read", "tool_input": {"file_path": path}}, "tid", None, + ) + spec = out["hookSpecificOutput"] + assert spec.get("permissionDecision") == "deny" + assert "cli_max_message_bytes" in spec["permissionDecisionReason"] + + @pytest.mark.asyncio + async def test_allows_the_same_image_under_the_default_cap(self, tmp_path): + path = _fake_png(tmp_path, 600 * 1024) + hook = _read_validator_hook(_make_hook_backend(True)) + out = await hook( + {"tool_name": "Read", "tool_input": {"file_path": path}}, "tid", None, + ) + assert "permissionDecision" not in out["hookSpecificOutput"] + + class TestBuildHooksBackgroundPermissions: """The catch-all PreToolUse hook gives background sub-agents (whose nested tool calls never reach can_use_tool) the same permissions as foreground.""" diff --git a/tests/test_image_validation.py b/tests/test_image_validation.py new file mode 100644 index 00000000..f4eb2c62 --- /dev/null +++ b/tests/test_image_validation.py @@ -0,0 +1,76 @@ +"""Read-image validation: magic bytes, the API size cap, and the transport bound. + +The transport bound is the Agent SDK's per-line ``max_buffer_size``: a Read +image result travels as ONE stream-json line carrying the base64 twice, and +a line over the bound aborts the whole turn rather than the one tool call. +""" + +from nerve.agent.backends.images import ( + CLI_MAX_IMAGE_BASE64, + IMAGE_WIRE_COPIES, + IMAGE_WIRE_OVERHEAD, + MAX_IMAGE_BYTES, + estimate_image_wire_bytes, + validate_image_file, +) + +PNG_MAGIC = b"\x89PNG\r\n\x1a\n" +ONE_MIB = 1024 * 1024 + + +def _png(tmp_path, size: int, name: str = "shot.png") -> str: + path = tmp_path / name + path.write_bytes(PNG_MAGIC + b"\0" * (size - len(PNG_MAGIC))) + return str(path) + + +def test_wire_estimate_counts_both_copies_of_the_base64(): + # A 420,301-byte PNG is 560,404 chars of base64 — shipped twice, the + # line is already over 1 MiB. + assert estimate_image_wire_bytes(420_301) == ( + IMAGE_WIRE_COPIES * 560_404 + IMAGE_WIRE_OVERHEAD + ) + assert estimate_image_wire_bytes(420_301) > ONE_MIB + # The CLI downsizes to at most 5 MiB of base64, so the bound saturates. + assert estimate_image_wire_bytes(MAX_IMAGE_BYTES) == ( + IMAGE_WIRE_COPIES * CLI_MAX_IMAGE_BASE64 + IMAGE_WIRE_OVERHEAD + ) + + +def test_image_over_transport_cap_is_refused(tmp_path): + err = validate_image_file(_png(tmp_path, 600 * 1024), max_message_bytes=ONE_MIB) + assert err is not None + assert "per-message limit" in err + assert "cli_max_message_bytes" in err + + +def test_same_image_passes_under_a_roomy_cap(tmp_path): + assert validate_image_file( + _png(tmp_path, 600 * 1024), max_message_bytes=64 * ONE_MIB, + ) is None + + +def test_no_cap_means_no_transport_check(tmp_path): + assert validate_image_file(_png(tmp_path, 600 * 1024)) is None + + +def test_api_limit_still_wins(tmp_path): + err = validate_image_file( + _png(tmp_path, MAX_IMAGE_BYTES + 1), max_message_bytes=64 * ONE_MIB, + ) + assert err is not None + assert "5 MB API limit" in err + + +def test_bad_magic_is_still_refused_as_poison(tmp_path): + path = tmp_path / "redirect.png" + path.write_bytes(b"not an image") + err = validate_image_file(str(path), max_message_bytes=64 * ONE_MIB) + assert err is not None + assert "HTML" in err + + +def test_non_image_extension_is_ignored(tmp_path): + path = tmp_path / "notes.txt" + path.write_bytes(b"\0" * (2 * ONE_MIB)) + assert validate_image_file(str(path), max_message_bytes=ONE_MIB) is None