Bound the process output a model receives: exec_poll returns the whole capture buffer every time #394

Merged

toasterson merged 2 commits from claude/wi-01a0c901-bound-the-process-output-a-model-receive into main 2026-10-06 19:20:58 +00:00

Conversation 1 Commits 2 Files changed 1 +208 -10

toasterson commented 2026-09-22 12:05:23 +00:00

Owner

Copy link

MAX_OUTPUT (1 MiB per stream) bounds what the executor captures. It was also, in effect, what it returned: both exec_poll paths embedded the whole captured buffer for stdout and stderr, so one receipt could carry 2 MiB of text into a model conversation.

Polling compounds it: a poll of a still-running process returns the live snapshot from byte zero every time, so polling a chatty command N times re-sends the entire accumulated buffer N times.

Evidence

Reviewer session 01a0c8bc-b7c3-7d93-9963-3e8a1d5f379f (sekhmet, 2026-09-22, 44 min), read read-only from the session-relay API:

• 15 run_command/poll_command pairs, every call succeeded — no failure, no missing capability
• submit_review_verdict was advertised to the session and never reached
• two timed-out commands returned receipts of 227 KB and 172 KB
• total tool execution time ~5 s; model generation between calls 1440 s, largest single gap 490 s
• closed with no verdict, leaving review_rounds: 0

One of 7/23 reviewer sessions that day that bound a runtime, burned a full ~30-minute slot, and produced nothing.

Fix

Capture is unchanged — the receipt still records true byte counts and the existing truncation flag. Only the model-facing text is bounded: 24 KiB per stream (16 KiB head + 8 KiB tail) with an explicit marker naming bytes dropped and bytes captured. Head and tail are kept because a reviewer needs how a build started and the error that ended it. The split lands on a UTF-8 boundary — a naive cut emits replacement characters mid-codepoint.

Adds stdout_bytes, stderr_bytes, elided so a caller can distinguish an elision from a command that genuinely printed little; previously indistinguishable.

Deliberately not done

Making exec_poll incremental is the deeper fix but would silently change what existing callers receive. Out of scope here.

Verification

6 new tests: at-budget verbatim passthrough; the observed 227 KB stream bounded with its true size still reported; head and tail both surviving; UTF-8 boundary safety; empty output; and the model budget staying well under the capture ceiling. Full crate suite 69 passed. Clippy on executor.rs is unchanged from main (5 pre-existing lines before and after, verified by stashing); the acp.rs warnings are pre-existing and untouched.

Gates not pre-cleared; fresh exact-head review and CI required.

🤖 Generated with Claude Code

`MAX_OUTPUT` (1 MiB per stream) bounds what the executor **captures**. It was also, in effect, what it **returned**: both `exec_poll` paths embedded the whole captured buffer for stdout and stderr, so one receipt could carry **2 MiB** of text into a model conversation. Polling compounds it: a poll of a still-running process returns the live snapshot **from byte zero every time**, so polling a chatty command N times re-sends the entire accumulated buffer N times. ## Evidence Reviewer session `01a0c8bc-b7c3-7d93-9963-3e8a1d5f379f` (sekhmet, 2026-09-22, 44 min), read read-only from the session-relay API: - 15 `run_command`/`poll_command` pairs, **every call succeeded** — no failure, no missing capability - `submit_review_verdict` **was advertised** to the session and never reached - two timed-out commands returned receipts of **227 KB** and **172 KB** - total tool execution time **~5 s**; model generation between calls **1440 s**, largest single gap **490 s** - closed with no verdict, leaving `review_rounds: 0` One of 7/23 reviewer sessions that day that bound a runtime, burned a full ~30-minute slot, and produced nothing. ## Fix Capture is unchanged — the receipt still records true byte counts and the existing truncation flag. Only the model-facing text is bounded: **24 KiB per stream** (16 KiB head + 8 KiB tail) with an explicit marker naming bytes dropped and bytes captured. Head *and* tail are kept because a reviewer needs how a build started and the error that ended it. The split lands on a UTF-8 boundary — a naive cut emits replacement characters mid-codepoint. Adds `stdout_bytes`, `stderr_bytes`, `elided` so a caller can distinguish an elision from a command that genuinely printed little; previously indistinguishable. ## Deliberately not done Making `exec_poll` incremental is the deeper fix but would silently change what existing callers receive. Out of scope here. ## Verification 6 new tests: at-budget verbatim passthrough; the observed 227 KB stream bounded with its true size still reported; head and tail both surviving; UTF-8 boundary safety; empty output; and the model budget staying well under the capture ceiling. **Full crate suite 69 passed.** Clippy on `executor.rs` is unchanged from main (5 pre-existing lines before and after, verified by stashing); the `acp.rs` warnings are pre-existing and untouched. Gates not pre-cleared; fresh exact-head review and CI required. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

toasterson added 1 commit 2026-09-22 12:05:23 +00:00

exec: bound the process output a model actually receives ... a29230a1d2

`MAX_OUTPUT` (1 MiB per stream) bounds what the executor CAPTURES. It was
also, in effect, what it RETURNED: both `exec_poll` paths embedded the whole
captured buffer for stdout and stderr, so a single receipt could carry 2 MiB
of text into a conversation.

Polling made it worse. A poll of a still-running process returns the live
snapshot from byte zero every time, so polling a chatty command N times
re-sent the entire accumulated buffer N times.

Evidence from production, reviewer session 01a0c8bc-b7c3 (2026-09-22,
sekhmet, 44 minutes): 15 run_command/poll_command pairs, every call
succeeding, two of them timed-out commands returning receipts of 227 KB and
172 KB. Total tool execution time in that session was about 5 seconds; 1440
seconds went to model generation between calls, with a single gap of 490
seconds. The session never reached submit_review_verdict, even though the
tool was advertised to it and no call ever failed.

Capture is unchanged. The receipt still records the true byte counts and the
existing truncation flag. Only the text placed in front of the model is now
bounded: 24 KiB per stream, as a 16 KiB head plus an 8 KiB tail, with an
explicit marker naming how many bytes were dropped and how many were
captured. Head and tail are both kept because the two things a reviewer needs
from a build log are how it started and the error that ended it. The split
lands on a UTF-8 boundary; a naive cut would emit replacement characters in
the middle of a multi-byte character.

The response also now carries `stdout_bytes`, `stderr_bytes` and `elided`, so
a caller can tell an elision from a command that genuinely printed little —
the previous shape made those indistinguishable.

This does not change the tool contract: `exec_poll` still returns the full
receipt shape and still replays from the beginning. Making it incremental
would be the deeper fix but would silently change what existing callers
receive, so it is deliberately not done here.

Tests: 6 new, covering at-budget verbatim passthrough, a 227 KB stream (the
observed regression) bounded with its true size still reported, head and tail
both surviving, UTF-8 boundary safety, empty output, and that the model
budget stays well under the capture ceiling. Full crate suite 69 passed.
Clippy on executor.rs is unchanged from main (5 pre-existing lines before and
after); the acp.rs warnings are pre-existing and untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

toasterson added 1 commit 2026-09-22 12:15:47 +00:00

exec: cap the whole poll response, and state that snapshots are cumulative ... 1eb1c8cbb2

Follow-up to the per-stream budget. Two gaps in that first cut.

The live snapshot was fetched regardless of the receipt's status. The worker
removes a process from the map only AFTER persisting its terminal receipt, so
a poll landing inside that window returned `result` AND `progress` — four
bounded streams instead of two. A finished process has no progress to report,
so `progress` is now None for a terminal receipt, which both halves the
worst case and closes the race.

Worth recording why that matters more than it looks: `ToolResult` stores the
payload twice, `text` being `data.to_string()` alongside `data` itself, so a
terminal receipt already carries the bounded output twice over. With a
snapshot on top that was four copies.

A new regression pins the ceiling on the whole response shape a caller
actually receives, not merely on each stream, so a later change cannot
quietly reintroduce an unbounded response.

Also documents, at the snapshot site, that the buffer returned is CUMULATIVE
and not incremental: successive polls re-send the same head. The previous
commit narrowed what each poll costs but did not change that, and nothing
here should be read as making the feed a delta.

Tests: 7 in the module, full crate suite green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

toasterson commented 2026-10-01 16:23:26 +00:00

Author

Owner

Copy link

Full-lane babysit 2026-10-01 — WI-472.

Exact head 1eb1c8cbb2ff5704969201f89a2d068f05db3553, current main ad441fe9e2359a1bf315eda28a4b072cc56cee2d. Real merge-tree clean; branch is not already on main.

Bounded head/tail model output and terminal-progress deduplication are implemented with UTF-8/count tests. Merge-tree clean. Hosted test-akh-medu is red; current tracker has no CI/review exit bars. Needs canonical exact-head validation and review.

Individual disposition also recorded in the Anima WI thread. No approval or CI gate was overridden.

Full-lane babysit 2026-10-01 — WI-472. Exact head `1eb1c8cbb2ff5704969201f89a2d068f05db3553`, current main `ad441fe9e2359a1bf315eda28a4b072cc56cee2d`. Real merge-tree clean; branch is not already on main. Bounded head/tail model output and terminal-progress deduplication are implemented with UTF-8/count tests. Merge-tree clean. Hosted test-akh-medu is red; current tracker has no CI/review exit bars. Needs canonical exact-head validation and review. Individual disposition also recorded in the Anima WI thread. No approval or CI gate was overridden.

toasterson merged commit 96e9423e91 into main 2026-10-06 19:20:57 +00:00

toasterson referenced this pull request from a commit 2026-10-06 19:20:58 +00:00

Merge pull request 'Bound the process output a model receives: exec_poll returns the whole capture buffer every time' (#394) from claude/wi-01a0c901-bound-the-process-output-a-model-receive into main

toasterson referenced this pull request 2026-10-19 08:27:13 +00:00

Bound model tool envelopes with ranged artifacts and process cursors #676

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

follow-up

Documented operational follow-up

No labels

follow-up

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

toasterson/akh-medu!394

No description provided.