fix(runtime): spill oversized shell_exec output instead of truncating (#6242)#6246
Merged
Conversation
…#6242) `shell_exec` capped stdout/stderr at `max_output_bytes` (default 100 KB) with `safe_truncate_str` plus an `[truncated, N total bytes]` note, so the dropped middle of a long run (e.g. a 25k-line test log) was unrecoverable — the universal artifact spill (#3347) never saw those bytes because the tool truncated them first. Oversized streams now route through `artifact_store::maybe_spill` (via the existing `spill::spill_or_passthrough` helper), yielding a compact stub the agent can page back in full via `read_artifact`. Streams at or below `max_output` pass through inline, and the universal post-tool spill still applies to the assembled result, so small streams are unaffected. Adds unit tests for `spill_or_passthrough`: passthrough below threshold, and a recoverable artifact written above it (isolated via `LIBREFANG_HOME` + tempdir).
github-actions
Bot
added
size/M
…move multi-line comment The fix(runtime) bullet was filed under ### Added, which is reserved for new features — a behaviour change to an existing tool belongs under ### Changed. Removes the 7-line comment block from shell.rs: it restated what spill_or_passthrough's name already communicates, referenced issue numbers that will rot, and exceeded the repo's one-short-line-max comment rule.
github-actions
Bot
added
ready-for-review
houko
enabled auto-merge (squash)
June 20, 2026 01:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs #6242 — implements the core information-loss fix from @pavver's proposal (the gap in Option 1).
The artifact store (#3347) already spills oversized tool results to disk and hands the agent a
read_artifactstub. Butshell_exectruncated its stdout/stderr lossily (safe_truncate_stratmax_output_bytes, default 100 KB) before the universal spill ever saw the bytes — so a long run's dropped middle was gone, not recoverable. That is the one place the store leaked information, and it's exactly the issue's motivating example (a 25k-linecargo testlosing its middle).Change
shell_execnow routes oversized stdout/stderr throughartifact_store::maybe_spill(via the existingspill::spill_or_passthroughhelper) instead ofsafe_truncate_str. Streams ≤max_outputpass through inline; over it, the agent gets a compact stub recoverable viaread_artifact. Reuses the existing store — GC, dedup, atomic writes, theread_artifactretrieval tool — so there's no new subsystem; the change is confined toshell.rsplus a unit test inspill.rs.Verification
Adds
spill.rsunit tests: below-threshold passthrough returns the body unchanged, and above-threshold spill writes a recoverable artifact (isolated viaLIBREFANG_HOME+ a tempdir, so the real home is untouched). The underlying spill machinery (maybe_spill/write/build_spill_stub) is already covered byartifact_store.rstests.Follow-ups (not in this PR)
Options 2/3 (exit-code-aware line filtering) and Option 4 (a prompt quiet-flag bullet) from the proposal are optional enhancements that should build on this spill — filter the context view, never the stored bytes. Please close #6242 if this core fix is sufficient, or keep it open to track those.