Skip to content

docs: document buffer-overflow fail-fast in agent-recovery [doc-updater] - #2830

Merged
jwbron merged 1 commit into
mainfrom
egg/doc-update-buffer-overflow-recovery
May 27, 2026
Merged

docs: document buffer-overflow fail-fast in agent-recovery [doc-updater]#2830
jwbron merged 1 commit into
mainfrom
egg/doc-update-buffer-overflow-recovery

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

Update docs/reference/agent-recovery.md to reflect the buffer-overflow detection added in b3cb713 (#2810).

  • Add a new Buffer Overflow Detection section explaining is_buffer_overflow(), which is checked before is_transient_crash() and exits immediately without consuming restart budget when the SDK's 1 MB JSON buffer cap is exceeded.
  • Add a row to the Before/After table for the SDK buffer overflow case (exit 255 + marker) so readers understand this is now handled differently from a plain segfault.

The code change was purely defensive — no user-facing configuration was added — but the behavioral difference (exit 255 with buffer-overflow marker → no retry vs. exit 255 from segfault → retry with backoff) is important for operators debugging stuck pipelines.

Triggered by: b3cb713 (Fix #2804: bound tool result size to prevent SDK buffer-overflow crashes)

Authored-by: egg

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — doc-only change, technical claims verified against the implementation.

Verification

I cross-checked every technical claim in the new section against orchestrator/consensus_wrapper.py and shared/egg_agent/client.py:

Claim Evidence
is_buffer_overflow() is checked before is_transient_crash() consensus_wrapper.py:355-358 (if is_buffer_overflow; then … elif is_transient_crash)
Greps for "exceeded maximum buffer size" consensus_wrapper.py:201
Exits immediately without consuming restart budget exit $AGENT_EXIT at consensus_wrapper.py:357
Wrapper log message text consensus_wrapper.py:356 (exact-match string)
AGENT_OUTPUT_LOG created via mktemp consensus_wrapper.py:144-146
Truncated per run (: > "$AGENT_OUTPUT_LOG") consensus_wrapper.py:156
Capture via 2>&1 | tee -a consensus_wrapper.py:175-177
Marker synced with _BUFFER_OVERFLOW_MARKER in shared/egg_agent/client.py client.py:72
Drift-risk caveat about SDK wording change Matches the in-code comment at client.py:131-143
#2804 closed; #2805, #2823 open Verified via gh issue view

Also handled on the restart path (consensus_wrapper.py:647-650), with a slightly different log message — not mentioned in the docs, but that omission is fine; the behavior summary is the same (no further retries).

Non-blocking notes

None. The note block honestly flags the SDK-marker drift risk and points at the correct follow-up issues (#2823 for the marker pin, #2805 for the real fix at the tool layer), which is the right framing.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor Author
egg is addressing review feedback...

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough verification — every technical claim cross-checked against the implementation.

Per-item disposition

  • Approval verdict (no blocking concerns)disagree (no action required; review approved with "Non-blocking notes: None"). No code changes pushed.
  • Observation: restart path at consensus_wrapper.py:647-650 also handles this with a slightly different log message, not mentioned in the docsdisagree (reviewer explicitly stated "that omission is fine; the behavior summary is the same"). This was framed as an FYI, not a request to update the docs. The Buffer Overflow Detection section describes the canonical path; mirroring the restart-path log verbatim would add noise without changing the behavior summary readers need.

No commits pushed for this review round.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

1 previous review(s) hidden.

@jwbron
jwbron merged commit 3b83d50 into main May 27, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent SDK message reader has 1MB JSON buffer limit — large tool results crash agents with exit code 255

1 participant