developer__shell captures command output with no cap, and persists it two to three times
- Langage dominant
- Rust
- Étoiles
- 54.2k
- Forks
- 6.2k
- Merge moyen
- 3 j 2 h
- PR mergées (30 j)
- 262
Description
**Verified against:** `main` @ `6cf0bf122ea2` (2026-08-10) and release `v1.45.0`.
**Platform:** Linux (x86_64), `goose serve`.
**File:** `crates/goose/src/agents/platform_extensions/developer/shell.rs`.
## What happens
The shell tool caps what the model *sees* (`OUTPUT_LIMIT_LINES = 2000`,
`OUTPUT_LIMIT_BYTES = 50_000`, and since #10992 `OUTPUT_PREVIEW_BYTES = 10_000`)
but it does not cap what it *captures*.
On current `main`:
- `run_command()` creates a `tokio::sync::mpsc::unbounded_channel()` (L577).
`collect_tagged_lines()` (L788) sends every line of stdout and stderr into it,
and `ExecutionOutput.lines: Vec<(bool, String)>` (L532) accumulates all of
them. There is no line cap, no byte cap and no backpressure, and nothing is
capped until after the process has exited.
- `split_lines()` (L762) then materialises three more full copies in RAM
(`stdout`, `stderr`, `interleaved`), so peak footprint is roughly 2x the raw
output on top of the `Vec`.
- `save_full_output()` (L929) writes the **full** raw text -- not the truncated
preview -- to `output_dir/{label}`. It is called once via
`truncate_output(raw_stdout, "stdout-{slot}")` (L425) and again via
`render_output(interleaved, "output-{slot}")` (L451), plus `stderr-{slot}`
(L436). The same bytes are therefore persisted two to three times.
- `output_dir` is a `tempfile::TempDir` owned by `ShellTool` (L326, constructed
at L337), so it lives for the whole process lifetime and is reclaimed only on
a graceful `Drop`.
- `OUTPUT_SLOTS = 8` (L163) bounds the file *count*. It bounds the size at
nothing.
The display-side cap is loud and correct -- `truncate_output()` emits
`[Output exceeded 2000 line limit (N lines total). Full output saved to ...]`
-- which is exactly why this is easy to miss. The tool looks capped from every
angle the model can see, and is unbounded underneath. The most recent change to
this file (#10992, "byte-bound the shell truncation preview") again bounds the
preview, not the capture.
## Impact
A single agent turn that runs a full-screen or TUI program (`airodump-ng`,
`top`, `tail -f`, a chatty build) puts the entire redraw stream into the goosed
heap and onto `$TMPDIR`.
On our host `$TMPDIR` defaulted to `/tmp`, which is a 62 GiB RAM-backed tmpfs.
Eight such shell calls (slots 0-7) wrote 62 GiB there while goosed itself
reached 43.6 GiB RSS. The kernel then OOM-killed the local `llama-server` that
goose was talking to -- the agent destroyed the model it was running on. Two
spool directories from that incident:
| dir | size | window |
|---|---|---|
| `/tmp/.tmpARXvXp` | 50 452 MB | 01:43-02:03 |
| `/tmp/.tmp2UheZd` | 12 726 MB | 04:26-04:37 (filling into the OOM) |
`/tmp` at 100% also broke unrelated processes on the box, because many shells
materialise heredocs in `$TMPDIR`.
The only bound on a shell tool call today is the extension timeout (#10348)
multiplied by the command write rate. Every downstream consumer has to work
around this with prompt discipline, which is not a control. We ended up
shipping a wrapper installed as `GOOSE_SHELL` that copies each stream through a
per-stream byte budget, because there was no knob for this.
## Three asks, in priority order
1. **Cap the capture, not just the display.** A bounded channel, or a running
byte counter in `collect_tagged_lines()` that stops accumulating (and
optionally kills the child) past a configurable limit -- e.g.
`GOOSE_SHELL_CAPTURE_LIMIT_BYTES`, defaulting to a few MiB. If truncation
happens it should be announced in the model-visible output; a silently
truncated result that looks complete would be worse than the current bug.
2. **Stop persisting the same bytes twice.** `stdout-{slot}` and
`output-{slot}` differ only by stderr interleaving. Storing the tagged lines
once and rendering views on read would halve the on-disk cost for no loss of
function.
3. **Bound and clean the spool.** `output_dir` is a `TempDir` on `ShellTool`,
so it survives for the process lifetime and -- critically -- is *not* cleaned
when goosed is SIGKILLed, which is exactly what an OOM or a cgroup
`MemoryMax` produces. That makes the leak worst on precisely the failure path
this bug creates. A total-size budget across slots, plus a sweep of stale
`.tmpXXXXXX` spool dirs at startup, would both help.
## Note on `TMPDIR`
`$TMPDIR` being a tmpfs is common (systemd `PrivateTmp`, many container images,
our host). Since the spill exists to move data *out* of memory, defaulting it
to a location that may be RAM defeats the purpose.
This is the same reasoning already accepted upstream in #9308 ("keep large MCP
response files off tmpfs"), which moved the *large-response* spill from
`/tmp/goose_mcp_responses` into the goose data dir. The developer shell tool
spool was not covered by that change and still calls `tempfile::tempdir()`.
Worth aligning, or documenting at minimum.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.