Fix #2804: bound tool result size to prevent SDK buffer-overflow crashes

orchestrator/consensus_wrapper.py

@@ -119,6 +119,23 @@
 TRANSIENT_BACKOFF_MAX=30
 STARTUP_FAILURE_WINDOW_SECONDS={startup_failure_window_seconds}
 
+# Capture agent stdout+stderr so the wrapper can post-mortem the run.
+# Used by is_buffer_overflow() to detect the Claude Agent SDK
+# message-reader 1MB JSON buffer crash (issue #2804) which is
+# deterministic — retrying just hits the same overflow and burns
+# the restart budget for no gain.
+#
+# Use ``mktemp`` for the default path so a co-tenant on the same host
+# cannot pre-create a symlink at a predictable ``/tmp/agent-output-$$``
+# location (``tee -a`` follows symlinks). The container is single-tenant
+# in normal operation; mktemp is defense-in-depth for multi-tenant
+# sandbox setups. The fallback to ``/tmp/agent-output-$$.log`` only
+# fires if ``mktemp`` itself isn't on PATH, which would be a much
+# bigger problem.
+if [ -z "${{AGENT_OUTPUT_LOG:-}}" ]; then
+    AGENT_OUTPUT_LOG="$(mktemp -t agent-output.XXXXXX 2>/dev/null || echo "/tmp/agent-output-$$.log")"
+fi
+
 # Log wrapper messages to stderr so they never leak into agent SDK context.
 cw_log() {{
     echo "[consensus-wrapper] $*" >&2
@@ -127,12 +144,50 @@
 run_agent() {{
     local prompt="$1"
     local system_prompt="${{2:-}}"
+    : > "$AGENT_OUTPUT_LOG"  # truncate per run so old crashes don't bleed forward
+    # Pipe stdout+stderr through tee so the post-mortem grep
+    # (is_buffer_overflow) sees the agent's full output. We use a
+    # single pipeline rather than per-stream process substitution
+    # because bash waits on pipelines synchronously; process
+    # substitution (> >(tee ...)) backgrounds the tee subshell and
+    # doesn't wait, which races with the immediate is_buffer_overflow
+    # grep that follows on agent exit.
+    #
+    # Note: ``2>&1 | tee -a`` interleaves stdout and stderr in the
+    # captured log. This is intentional — the SDK overflow marker
+    # is emitted on stderr (``logger.error`` in
+    # ``claude_agent_sdk.query``), and the grep that triggers
+    # is_buffer_overflow needs to see it in the same file as
+    # stdout. Side-effect: any future log analysis that depends on
+    # stdout/stderr separation will need to capture them separately
+    # upstream (e.g. via ``script`` or a wrapper process), not from
+    # ``$AGENT_OUTPUT_LOG``.
     if [ -n "$system_prompt" ]; then
-        {agent_command_prefix} --system-prompt "$system_prompt" "$prompt"
+        {agent_command_prefix} --system-prompt "$system_prompt" "$prompt" 2>&1 | tee -a "$AGENT_OUTPUT_LOG"
     else
-        {agent_command_prefix} "$prompt"
+        {agent_command_prefix} "$prompt" 2>&1 | tee -a "$AGENT_OUTPUT_LOG"
     fi
-    return $?
+    return ${{PIPESTATUS[0]}}
+}}
+
+# Detect the Claude Agent SDK 1 MB JSON message-reader overflow
+# signature in the most recent agent run. Issue #2804. The overflow
+# is deterministic: re-running the agent against the same codebase
+# hits the same oversized tool result, so the wrapper must NOT
+# consume retry budget on this failure class. Returns 0 (true) if
+# the marker was logged, 1 otherwise.
+#
+# The substring matches CLI output from claude_agent_sdk emitted on
+# the buffer overflow path. If a future SDK bump changes the
+# wording, this grep silently falls through and the wrapper burns
+# its retry budget again — the marker_stability_test in
+# orchestrator/tests/test_consensus_wrapper.py exercises a synthetic
+# log to keep this honest, but does not pin against the installed
+# SDK. The real fix is tool-layer truncation (#2805); this is the
+# fail-fast path until that lands.
+is_buffer_overflow() {{
+    [ -f "$AGENT_OUTPUT_LOG" ] || return 1
+    grep -q "exceeded maximum buffer size" "$AGENT_OUTPUT_LOG" 2>/dev/null
 }}
 
 # Helper: extract BRC agent state from pipeline status JSON
@@ -286,7 +341,10 @@
         fi
     fi
 
-    if is_transient_crash "$AGENT_EXIT"; then
+    if is_buffer_overflow; then
+        cw_log "Agent crashed on Claude Agent SDK buffer overflow (issue #2804). Deterministic failure; retry budget would be wasted. NOT restarting."
+        exit $AGENT_EXIT
+    elif is_transient_crash "$AGENT_EXIT"; then
         cw_log "Transient crash (code $AGENT_EXIT). Will restart with backoff."
         CRASH_BACKOFF=$TRANSIENT_BACKOFF_INITIAL
     elif is_startup_failure "$AGENT_EXIT" "$AGENT_DURATION"; then
@@ -558,6 +616,10 @@
                 exit 0
             fi
         fi
+        if is_buffer_overflow; then
+            cw_log "Agent crashed on Claude Agent SDK buffer overflow (issue #2804) on restart $RESTART_COUNT. Deterministic failure; further retries would waste budget. Stopping."
+            exit $AGENT_EXIT
+        fi
         if is_transient_crash "$AGENT_EXIT"; then
             cw_log "Transient crash on restart $RESTART_COUNT (code $AGENT_EXIT). Will retry."
             if [ "$CRASH_BACKOFF" -eq 0 ]; then

orchestrator/tests/test_consensus_wrapper.py

@@ -1340,6 +1340,237 @@ def test_non_transient_exit_code_42_does_not_restart(self):
             assert "Transient crash" not in result.stderr
 
 
+class TestBufferOverflowDetection:
+    """Issue #2804: the Claude Agent SDK 1MB JSON buffer crash is
+    deterministic — retrying just hits the same overflow on the same
+    codebase. The wrapper must short-circuit retry budget when it
+    sees the overflow signature in the agent's output.
+    """
+
+    def test_script_defines_is_buffer_overflow(self):
+        cmd = build_consensus_wrapped_command("Prompt")
+        script = cmd[2]
+        assert "is_buffer_overflow()" in script
+        assert "exceeded maximum buffer size" in script
+
+    def test_script_marker_matches_client_constant(self):
+        """The bash grep substring must match ``_BUFFER_OVERFLOW_MARKER``
+        in ``shared/egg_agent/client.py``. Renaming the constant without
+        updating the wrapper script silently regresses the short-circuit
+        — this test pins them together. Issue #2804.
+        """
+        sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "shared"))
+        try:
+            from egg_agent.client import _BUFFER_OVERFLOW_MARKER
+        finally:
+            sys.path.pop(0)
+
+        cmd = build_consensus_wrapped_command("Prompt")
+        script = cmd[2]
+        assert _BUFFER_OVERFLOW_MARKER in script, (
+            f"consensus_wrapper script must grep for {_BUFFER_OVERFLOW_MARKER!r} "
+            "to match the marker emitted by run_agent_async on SDK overflow"
+        )
+
+    def test_script_captures_agent_output(self):
+        """Agent output must be tee'd to a log file the wrapper can grep."""
+        cmd = build_consensus_wrapped_command("Prompt")
+        script = cmd[2]
+        assert "AGENT_OUTPUT_LOG" in script
+        assert "tee -a" in script
+
+    def test_buffer_overflow_check_runs_before_transient_check(self):
+        """The overflow check must precede is_transient_crash so a
+        signal-255 agent that crashed on overflow doesn't get retried.
+        """
+        cmd = build_consensus_wrapped_command("Prompt")
+        script = cmd[2]
+        # Both functions exist
+        assert "is_buffer_overflow" in script
+        assert "is_transient_crash" in script
+        # Find the first occurrence of each in the initial-exit handler
+        # (the section after the consensus/confirmed checks).
+        idx_buffer = script.find("if is_buffer_overflow")
+        idx_transient = script.find('is_transient_crash "$AGENT_EXIT"')
+        assert idx_buffer > 0 and idx_transient > 0
+        assert idx_buffer < idx_transient, (
+            "is_buffer_overflow must be checked BEFORE is_transient_crash"
+        )
+
+    def test_buffer_overflow_log_message_cites_issue(self):
+        """When the wrapper aborts on overflow, log must mention #2804."""
+        cmd = build_consensus_wrapped_command("Prompt")
+        script = cmd[2]
+        assert "#2804" in script
+
+    def test_buffer_overflow_check_in_restart_loop(self):
+        """The restart-loop must ALSO check for buffer overflow — even if
+        the initial run survived, a recovery attempt that hits the
+        overflow must abort without consuming the rest of the budget.
+        """
+        cmd = build_consensus_wrapped_command("Prompt")
+        script = cmd[2]
+        # is_buffer_overflow appears at least twice: initial handler + restart loop
+        assert script.count("is_buffer_overflow") >= 2
+
+    @staticmethod
+    def _make_buffer_overflow_agent(tmpdir: str) -> None:
+        """Create a mock python3 that simulates the SDK buffer-overflow crash.
+
+        Writes the signature marker to stderr (matching the real SDK's
+        ``logger.error`` from ``query.py:221``) and exits 255 — the same
+        shape produced by the real crash in the issue-2777 #2804 incident.
+        """
+        mock_python = os.path.join(tmpdir, "python3")
+        real_python = sys.executable
+        with open(mock_python, "w") as f:
+            f.write("#!/bin/bash\n")
+            f.write('if [ "$1" = "-m" ] && [ "$2" = "egg_agent" ]; then\n')
+            f.write(
+                '  echo "Fatal error in message reader: Failed to decode '
+                "JSON: JSON message exceeded maximum buffer size of "
+                '1048576 bytes..." >&2\n'
+            )
+            f.write("  exit 255\n")
+            f.write("else\n")
+            f.write(f'  exec {shlex.quote(real_python)} "$@"\n')
+            f.write("fi\n")
+        os.chmod(mock_python, 0o755)  # nosec B103
+
+    def test_buffer_overflow_aborts_without_retry(self):
+        """Agent crashing with the SDK buffer-overflow signature must
+        exit immediately, NOT consume the restart budget.
+        """
+        with tempfile.TemporaryDirectory() as tmpdir:
+            log_file = os.path.join(tmpdir, "egg-orch.log")
+            # No consensus reached
+            TestConsensusWrapperBehavior._make_mock_orch_no_consensus(tmpdir, log_file)
+            self._make_buffer_overflow_agent(tmpdir)
+
+            cmd = build_consensus_wrapped_command("Prompt", max_restarts=2)
+            result = TestConsensusWrapperBehavior._run_wrapper_command(cmd, tmpdir)
+
+            # Wrapper must propagate the agent's exit code (255)
+            assert result.returncode == 255, result.stderr
+            # Must log the overflow diagnosis
+            assert "buffer overflow" in result.stderr.lower()
+            assert "#2804" in result.stderr
+            # Must NOT have attempted a restart
+            assert "Restarting" not in result.stderr
+            assert "Transient crash" not in result.stderr
+
+    def test_signal_255_without_overflow_marker_still_retries(self):
+        """Other 255 crashes (genuine SIGSEGV, bun segfault) keep the
+        existing transient-retry behavior — the wrapper must only
+        short-circuit when the overflow marker is present.
+        """
+        with tempfile.TemporaryDirectory() as tmpdir:
+            log_file = os.path.join(tmpdir, "egg-orch.log")
+            TestConsensusWrapperBehavior._make_mock_orch_no_consensus(tmpdir, log_file)
+            # Plain exit 255 with no marker → should still be classified
+            # as transient and trigger a restart.
+            _make_mock_agent(tmpdir, exit_code=255)
+
+            cmd = build_consensus_wrapped_command(
+                "Prompt",
+                max_restarts=1,
+                transient_backoff_initial=1,
+            )
+            result = TestConsensusWrapperBehavior._run_wrapper_command(cmd, tmpdir, timeout=20)
+
+            # Without the marker, the existing transient-crash classification
+            # still kicks in; should log Transient crash.
+            assert "Transient crash" in result.stderr
+
+    def test_buffer_overflow_in_restart_loop_aborts_without_further_retries(self):
+        """Restart-loop overflow path: clean initial exit triggers a
+        restart, recovery run crashes with the overflow marker, wrapper
+        aborts immediately instead of consuming the remaining budget.
+
+        Distinct from ``test_buffer_overflow_aborts_without_retry``,
+        which only exercises the initial-exit handler. Both code paths
+        need the buffer-overflow short-circuit; this regression-guards
+        the restart-loop branch (#2804 review feedback).
+        """
+        with tempfile.TemporaryDirectory() as tmpdir:
+            log_file = os.path.join(tmpdir, "egg-orch.log")
+            claude_log = os.path.join(tmpdir, "claude.log")
+            call_counter = os.path.join(tmpdir, "agent_call_count")
+
+            # Mock egg-orch: always returns is_complete=false so the
+            # wrapper enters the restart loop after each clean exit.
+            mock_orch = os.path.join(tmpdir, "egg-orch")
+            with open(mock_orch, "w") as f:
+                f.write("#!/bin/bash\n")
+                f.write(f'echo "$@" >> {shlex.quote(log_file)}\n')
+                f.write('if echo "$@" | grep -q "pipeline status"; then\n')
+                f.write(
+                    '  echo \'{"data": {"concurrent": {"consensus": '
+                    '{"is_complete": false, "agents": {}}}}}\'\n'
+                )
+                f.write('elif echo "$@" | grep -q "message poll"; then\n')
+                f.write('  echo "[]"\n')
+                f.write("else\n")
+                f.write('  echo "{}"\n')
+                f.write("fi\n")
+            os.chmod(mock_orch, 0o755)  # nosec B103
+
+            # Mock agent: clean exit on call 1 (triggers restart), then
+            # emits the SDK overflow signature and exits 255 on call 2.
+            mock_python = os.path.join(tmpdir, "python3")
+            real_python = sys.executable
+            with open(mock_python, "w") as f:
+                f.write("#!/bin/bash\n")
+                f.write('if [ "$1" = "-m" ] && [ "$2" = "egg_agent" ]; then\n')
+                f.write("  CALL_COUNT=0\n")
+                f.write(f"  if [ -f {shlex.quote(call_counter)} ]; then\n")
+                f.write(f"    CALL_COUNT=$(cat {shlex.quote(call_counter)})\n")
+                f.write("  fi\n")
+                f.write("  CALL_COUNT=$((CALL_COUNT + 1))\n")
+                f.write(f'  echo "$CALL_COUNT" > {shlex.quote(call_counter)}\n')
+                f.write(f'  echo "---CLAUDE_CALL_START---" >> {shlex.quote(claude_log)}\n')
+                f.write('  if [ "$CALL_COUNT" -eq 1 ]; then\n')
+                f.write("    exit 0\n")  # clean exit → restart triggered
+                f.write("  fi\n")
+                # call 2: emit the overflow signature and exit 255
+                f.write(
+                    '  echo "Fatal error in message reader: Failed to decode '
+                    "JSON: JSON message exceeded maximum buffer size of "
+                    '1048576 bytes..." >&2\n'
+                )
+                f.write("  exit 255\n")
+                f.write("else\n")
+                f.write(f'  exec {shlex.quote(real_python)} "$@"\n')
+                f.write("fi\n")
+            os.chmod(mock_python, 0o755)  # nosec B103
+
+            cmd = build_consensus_wrapped_command(
+                "Prompt", max_restarts=3, transient_backoff_initial=1
+            )
+            result = TestConsensusWrapperBehavior._run_wrapper_command(cmd, tmpdir, timeout=30)
+
+            # Wrapper must propagate the overflow crash's exit code from
+            # the restart loop, NOT continue retrying.
+            assert result.returncode == 255, result.stderr
+            # Diagnostic message must indicate the restart-loop path
+            # (the initial-exit handler says "Agent crashed on Claude
+            # Agent SDK buffer overflow ..."; the restart-loop handler
+            # adds "on restart N").
+            assert "buffer overflow" in result.stderr.lower()
+            assert "on restart" in result.stderr
+            assert "#2804" in result.stderr
+            # Agent was called exactly twice: initial clean exit + one
+            # restart that crashed on overflow. Should NOT be called a
+            # third time.
+            with open(claude_log) as f:
+                call_count = f.read().count("---CLAUDE_CALL_START---")
+            assert call_count == 2, (
+                f"Expected exactly 2 agent calls (initial + 1 restart with "
+                f"overflow), got {call_count}. The restart-loop buffer-overflow "
+                f"check must abort before consuming further retry budget."
+            )
+
+
 class TestEventDrivenWait:
     """Issue #1897 Phase 5 / TASK-5-1 (plan rev 4, reviewer_plan blocker 4):
     ``check_confirmed_and_wait`` is an SSE-primary hybrid.

sandbox/entrypoint.py

@@ -1054,7 +1054,7 @@ def setup_claude(config: Config, logger: Logger) -> None:
             logger.success(f"Skills installed: {', '.join(installed)}")
 
     # Create settings.json
-    settings = {
+    settings: dict[str, Any] = {
         "defaultPermissionMode": "bypassPermissions",
         "skipDangerousModePermissionPrompt": True,
         "autoApproveEdits": True,

shared/egg_agent/client.py

@@ -61,6 +61,16 @@ def error(self, msg: str, **kwargs: Any) -> None:
 # Default model for sandbox agents
 DEFAULT_MODEL = "opus[1m]"
 
+# Substring of the SDK's CLIJSONDecodeError message identifying the
+# 1 MB JSON message-reader buffer overflow (issue #2804). The overflow
+# is deterministic — the same tool call against the same codebase
+# produces the same oversized payload — so the consensus-wrapper greps
+# for this marker on agent exit to short-circuit retry instead of
+# burning the restart budget on a doomed re-run. The real fix is
+# tool-layer truncation (see #2805); this PR only makes the failure
+# mode observable and terminal.
+_BUFFER_OVERFLOW_MARKER = "exceeded maximum buffer size"
+
 
 async def run_agent_async(
     prompt: str,
@@ -118,6 +128,19 @@ async def run_agent_async(
             UserMessage,
             query,
         )
+        # CLIJSONDecodeError is a subclass of ClaudeSDKError, so it's
+        # caught by the existing handler below — issue #2804 relies on
+        # its error message preserving the ``exceeded maximum buffer
+        # size`` marker so the consensus-wrapper can short-circuit
+        # retry on this failure class. Tests pin that the marker
+        # propagates into ``result.error`` once raised, and that the
+        # consensus-wrapper grep matches ``_BUFFER_OVERFLOW_MARKER``;
+        # stability of the marker against future ``claude-agent-sdk``
+        # releases is NOT verified (the SDK could change the wording
+        # at any minor bump and the wrapper would silently fall back
+        # to burning the transient-crash retry budget). See #2823 for
+        # the follow-up to pin or smoke-test the marker against the
+        # installed SDK.
     except ImportError:
         return AgentResult(
             success=False,