fix(agent-core): cap foreground shell output to prevent OOM crash - #1285
Conversation
A foreground command that streams a very large or unbounded amount of output (e.g. `b3sum --length 18446744073709551615`) grew the live-output buffer until Node aborted with a JavaScript heap out-of-memory error (exit 134). The per-command output is now capped at 16 MiB: on breach the command is gracefully terminated (SIGTERM -> grace -> SIGKILL) and the result carries a message pointing at redirecting large output to a file. The per-task output ring buffer is also made O(1) per chunk (was O(n^2)), which previously starved the event loop and the foreground timeout. Background/detached tasks are exempt.
🦋 Changeset detectedLatest commit: 40362e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0199bd6f7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `appendOutput` above may synchronously abort the signal. Stop forwarding | ||
| // live output from that point so the unbounded forward buffer cannot keep | ||
| // growing while the process is being killed. | ||
| if (sink.signal.aborted) return; |
There was a problem hiding this comment.
When a foreground command crosses the 16 MiB cap but keeps writing during the SIGTERM grace period, this check only suppresses onOutput; every later data event still runs sink.appendOutput(chunk) first. In normal persisted sessions that continues to enqueue each chunk into the task output write chain, so a producer that ignores SIGTERM can still accumulate large strings/promises until SIGKILL and hit the same OOM class this change is meant to prevent. Stop appending/reading once the cap has tripped, or destroy/pause the streams before queuing more output.
Useful? React with 👍 / 👎.
After the 16 MiB foreground ceiling tripped, appendOutput still enqueued every subsequent chunk into the per-task disk write chain during the SIGTERM grace window. A producer that ignores SIGTERM could keep that chain — and the chunk strings each pending write retains — growing until SIGKILL, re-introducing the same out-of-memory risk the cap prevents. Once the cap has tripped, keep only the bounded in-memory ring buffer and stop feeding the disk write chain. Interrupt/timeout capture behaviour is unchanged (they do not set outputLimitTripped).
1 participant
Problem
A foreground
Bashcommand that produces a very large or unbounded amount of output crashes the whole agent process with a JavaScript heap out-of-memory error (FATAL ERROR: Reached heap limit ... JavaScript heap out of memory), i.e.SIGABRT/ exit code134.Reproduced in a headless (
-p) run where the model probed ab3sumbinary with--length 18446744073709551615(2^64-1), asking it to stream ~18 EB of hash output. The live tool-output was forwarded chunk-by-chunk to the transcript with no size cap, growing the process heap to the ~4 GB V8 limit until Node aborted. A single 685 MB stdout capture and the OOM stack trace confirmed it.Root cause
BackgroundManager.appendOutputrecomputed the ring-buffer total withreduceon every chunk —O(n^2)over a command's lifetime. Under a high-rate stream this starved the event loop, so the foreground timeout (a timer) never fired and the safety net that should have killed the command was defeated.The final tool result was already capped (50 KB) and the in-memory ring buffer was already capped (1 MiB); only the live-forward path and the per-chunk bookkeeping were unbounded/pathological.
Fix
Enforced at the single point where child output enters the process:
appendOutputis nowO(1)per chunk via an incrementally maintained running total, so the event loop (and the foreground timeout) is no longer starved.No public interface change; the fix lives in
agent-core/src/agent/background/{index,process-task}.ts.Testing
test/agent/background/output-limit.test.ts: a foreground command streaming past the ceiling is terminated (killed, SIGTERM, forward capped); a detached task streaming the same output is not terminated. (Fails before the fix, passes after.)@moonshot-ai/agent-coresuite green (3404 passed, 4 expected-fail, 1 skipped).tsc --noEmitandoxlint --type-awareclean on the changed files.