Release per-turn stream state and prove the memory plateau (Fixes #3114)

[acoliver]

TLDR

Long-running sessions grew memory with uptime, not activity — a 128 GiB machine was driven to 64 MB free and 98.5% swap exhaustion, with the two worst offenders being the two most idle sessions.

Most of umbrella issue #3114 has already landed as separate PRs. This lands the one attributed retention defect still outstanding, plus the measurement that proves the combined result actually holds.

The bug: Turn.cleanupStreamResources aborted the timeout controller before calling closeIteratorBounded. That helper early-returns when the signal it is handed is already aborted — so the abort raced ahead of its own cleanup and the branch that awaits iterator.return() was unreachable on every single turn. Cooperative provider iterators never finished unwinding, so the generator scope capturing each turn's stream state stayed alive.

Reviewers should look at: the ordering change in turn.ts (it is three lines and the whole fix), and whether the reasoning benchmark is a fair proxy for the real leak.

Dive Deeper

Already merged elsewhere — deliberately not re-implemented here

Sub-issue	PR	What it covered
#3111	#3117	Thinking-block coalescing in StreamOutputAccumulator — the primary retention site
#3112	#3123	Honest Bun memory footer + runtime-aware --max-old-space-size suppression
#3109	#3125	Read-only history view instead of deep cloning
#3113	#3124	Bounded and rotated error reports

This branch was rebased onto those. An earlier revision of this PR independently implemented the #3112 footer/bootstrap work before #3123 merged; that work was dropped in favour of the merged version rather than re-landed.

The iterator cleanup fix

Closure is now awaited first; the controller is aborted after.

The cleanup signal is omitted deliberately — passing the turn-owned signal is precisely what defeated the wait. A noncooperative iterator is still bounded by closeIteratorBounded's own one-second timeout, so this cannot hang.

There is no added cancellation latency. onParentAbort already aborts the timeout controller from the parent signal, so on user cancel the controller is aborted before cleanup runs. The reordering only affects normal completion and early consumer exit.

This is a fail-fast fix at the attributed site, not a defensive guard: nothing was wrapped, no error is swallowed that was not already warning-only, and the existing bound is preserved rather than duplicated.

Why a multi-metric plateau gate

The existing harness gated only on post-GC JSC heap. That would have declared victory here while the strings were still resident: the vmmap investigation attributed 32.9 GB to external against 0.5 GB of ArrayBuffers, and under Bun external tracks JS string backing stores at roughly a byte per character.

The gate now evaluates JSC heap, process.memoryUsage().external, and dirty WebKit Malloc independently and passes only if all three settle. evaluateMultiMetricPlateau reuses the existing per-metric evaluator and warm-up handling rather than introducing a second notion of "plateau".

Why the reasoning workload looks the way it does

reasoning mode drives the real StreamOutputAccumulator — not a stand-in — with 200 full-so-far thinking deltas per turn against a 30 KB final thought, which is the shape Anthropic actually streams.

Two details that matter for it being a real measurement:

• Each prefix is copied through a Buffer rather than produced by String.prototype.slice, so the workload allocates distinct string backing stores instead of engine-dependent substring views that would understate retention.
• Every turn asserts the span collapses to exactly one block carrying the full final text, stream id, status and signature — so the target fails loudly if coalescing ever regresses, rather than silently measuring a workload that no longer reproduces the pressure.

This extends scripts/issue-2852-memory-runner.ts; no parallel harness was added.

No silent data loss

The reasoning span is retained in full. Only the duplicate partial copies of it are not. No new length, count, or disk bound is introduced, so no truncation label is required.

Documentation

docs/sandbox.md still described the Node heap limit as automatically derived regardless of runtime, which #3123 made untrue when it stopped passing --max-old-space-size under Bun. Corrected so the docs match shipped behaviour.

Reviewer Test Plan

1. Confirm the cleanup tests are real (RED/GREEN), not mock theater.

cd packages/agents
bun test src/core/turn.cooperative-cleanup.bun.test.ts    # 3 pass

Then revert just the fix and re-run:

git stash push -- src/core/turn.ts   # or hand-restore the old ordering
bun test src/core/turn.cooperative-cleanup.bun.test.ts

Expect the two cooperative cases to fail with Expected: false, Received: true — the turn finishing before cleanup released. The bounded-timeout case still passes, confirming the existing timeout behaviour is untouched. Restore afterwards.

These tests drive the public Turn.run() with a provider iterator whose return() settles only on a controlled release; they assert observable turn completion ordering, not that a mock was called.

2. Run the real memory benchmark (macOS; uses vmmap / footprint).

bun scripts/issue-2852-memory-runner.ts /tmp/mem-3114 reasoning 4
cat /tmp/mem-3114/os-checkpoints.json

Exit 0 and overallWithinTolerance: true. Observed on this branch:

metric	growth
JSC heap	+1.25%
external	+5.47%
dirty WebKit Malloc	+0.00%

To see the gate actually bite, hand-edit one post-GC external value upward in the target JSONL and re-derive — the run fails naming the offending metric.

3. Confirm the pre-existing text/media modes still work.

bun scripts/issue-2852-memory-runner.ts /tmp/mem-text text 4

4. Long-session sanity. Run an interactive session against a reasoning-capable model (Anthropic or an OpenAI Responses model), leave it idle across many turns, and watch the footer RSS/External. Growth should track activity, not uptime.

Testing Matrix

	🍏	🪟	🐧
npm run	✅	❓	❓
npx	❓	❓	❓
Docker	❓	❓	❓
Podman	❓	-	-
Seatbelt	❓	-	-

Verified on macOS 26.4 arm64 / Bun 1.3.14:

• npm run test — exit 0, zero failures across all 17 workspaces (agents 350/350, CLI 681/681, core 546/546)
• npm run lint — exit 0
• npm run typecheck — exit 0
• npm run format — clean
• npm run build — exit 0
• Smoke: bun scripts/start.ts --profile-load stepfun-37 "write me a haiku and nothing else" — exit 0
• TUI: bun scripts/tmux-harness.ts — exit 0

The benchmark is macOS-specific by design (it shells out to vmmap and footprint), as was the pre-existing issue-2852 harness it extends. It is a developer tool, not part of CI.

A note on agents-suite flakiness

While validating, the agents suite intermittently failed 1–4 files on timeouts only, with a different set each run. To rule out this change as the cause, the suite was run against the unmodified turn.ts: that baseline failed more files (displayCallbacks, memoryControl), which reproduces the flake independently of this PR. Every implicated file passes in isolation. The final clean run on this head is fully green.

Linked issues / bugs

Fixes #3114

Depends on and builds atop #3117, #3123, #3125 and #3124, all merged.

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability when stopping or completing streamed responses, ensuring resources are cleaned up without unnecessary delays.
  • Added safeguards so unresponsive streams remain bounded and do not hang indefinitely.

• Documentation

  • Clarified how macOS sandbox memory limits affect Node.js and Bun runtime configurations.

• Diagnostics

  • Expanded memory benchmarking to evaluate multiple memory categories and reasoning-stream behavior.
  • Added clearer checkpoint validation and metric-specific reporting for memory plateau results.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 31 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 50940016-165d-4d0c-817c-393d11114dd0

📥 Commits

Reviewing files that changed from the base of the PR and between 18108c6 and 962e4e9.

⛔ Files ignored due to path filters (1)

• project-plans/issue3114-memory-plateau.md is excluded by !project-plans/**

📒 Files selected for processing (7)

• docs/sandbox.md
• packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts
• packages/agents/src/core/turn.ts
• scripts/issue-2852-memory-benchmark.ts
• scripts/issue-2852-memory-runner.ts
• scripts/issue-2852-memory-target.ts
• scripts/tests/issue-2852-memory-plateau.bun.test.ts

📝 Walkthrough

Walkthrough

The PR fixes iterator cleanup ordering and adds Bun tests. It extends the memory benchmark with reasoning mode, multi-metric post-GC plateau evaluation, checkpoint validation, accumulator integration tests, and updated sandbox memory-limit documentation.

Changes

Stream cleanup lifecycle

Layer / File(s)	Summary
Cooperative iterator cleanup packages/agents/src/core/turn.ts, packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts	Turn.run() now completes bounded iterator cleanup before aborting its timeout controller. Tests cover early exit, normal completion, and unresolved cleanup.

Reasoning memory benchmark

Layer / File(s)	Summary
Checkpoint and plateau evaluation scripts/issue-2852-memory-benchmark.ts, scripts/tests/issue-2852-memory-plateau.bun.test.ts	The benchmark parses post-GC records with source line numbers and evaluates JSC heap, external memory, and dirty WebKit Malloc readings independently and collectively.
Reasoning benchmark stream scripts/issue-2852-memory-target.ts, scripts/tests/issue-2852-memory-plateau.bun.test.ts	The target supports reasoning mode and validates streamed thinking content, completion, stream metadata, and signature.
Checkpoint metric integration scripts/issue-2852-memory-runner.ts	Reasoning checkpoints capture and validate all three metrics. Other modes retain JSC-only evaluation.
Sandbox memory-limit documentation docs/sandbox.md	The documentation limits derived --max-old-space-size behavior to Node.js sandbox CLI processes.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• vybestack/llxprt-code#3014: Related Bun-native testing and runner behavior.
• vybestack/llxprt-code#3120: Related Turn.run() stream-iterator handling.
• vybestack/llxprt-code#3123: Related Bun --max-old-space-size documentation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the per-turn stream cleanup fix and memory plateau validation for issue #3114.
Description check	✅ Passed	The description fills the required sections with clear scope, implementation details, testing steps, results, and linked issue information.
Linked Issues check	✅ Passed	The PR addresses [#3114] by fixing iterator cleanup, adding behavioral tests, and validating the required multi-metric memory plateau without truncation.
Out of Scope Changes check	✅ Passed	The documentation, benchmark, runner, target, and tests directly support the cleanup fix and memory-validation objectives for [#3114].

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue3114

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Walkthrough

Before this PR, when a turn exited early or a stream timed out, Turn.run() aborted the timeout controller before awaiting the provider iterator's close(). This ordering could interrupt cooperative iterators before they finished cleanup, leaving per-turn stream state unreleased and making it harder to prove that memory plateaus across repeated equivalent turns. After this PR, stream resource cleanup awaits closeIteratorBounded() on the stream iterator before aborting the timeout controller, giving cooperative iterators a chance to complete their cleanup. Behavioral tests now verify this cooperative cleanup path and prove that post-GC memory reaches a stable plateau across equivalent turns, with extended benchmark tooling measuring JSC heap, external memory, and dirty WebKit Malloc.

New Features

• Added memory benchmark tooling with reasoning mode to reproduce per-turn stream pressure and evaluate multi-metric post-GC memory plateau across JSC heap, external memory, and dirty WebKit Malloc.

Bug Fixes

• Reordered stream resource cleanup in Turn.run() to await provider iterator closure before aborting the timeout controller, ensuring cooperative iterators complete cleanup even when consumers exit early.

Tests

• Added behavioral tests verifying cooperative stream cleanup behavior.
• Added tests for multi-metric post-GC memory plateau logic, including reasoning-mode stream accumulation.

Documentation

• Updated sandbox documentation as part of the broader issue Long-running sessions grow memory without bound until the host is exhausted (84.8 GB across sessions, 64 MB free observed) #3114 memory plateau work.

Changes

Layer	File(s)	Summary
core	packages/agents/src/core/turn.ts	Reorders stream resource cleanup to await provider iterator closure before aborting the timeout controller, ensuring cooperative iterators complete cleanup even when consumers exit early.
tests	packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts, scripts/tests/issue-2852-memory-plateau.bun.test.ts	Adds behavioral tests verifying cooperative stream cleanup behavior and multi-metric post-GC memory plateau logic, including reasoning-mode stream accumulation.
benchmark	scripts/issue-2852-memory-benchmark.ts, scripts/issue-2852-memory-target.ts, scripts/issue-2852-memory-runner.ts	Extends memory benchmark tooling with a reasoning mode to reproduce per-turn stream pressure and adds multi-metric post-GC plateau evaluation across JSC heap, external memory, and dirty WebKit Malloc.
docs	docs/sandbox.md	Updates sandbox documentation as part of the broader issue #3114 memory plateau work.
plan	project-plans/issue3114-memory-plateau.md	New plan document defining requirements, test-first implementation sequence, and phased RED/GREEN approach for bounding long-running session memory and proving the memory plateau.

Magnitude

🎯 1 (S)
912 additions, 34 deletions, 8 changed files across 1 package, 1 acceptance criterion

Related

• Long-running sessions grow memory without bound until the host is exhausted (84.8 GB across sessions, 64 MB free observed) #3114: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 explicitly fixes this umbrella issue and is titled "Release per-turn stream state and prove the memory plateau (Fixes Long-running sessions grow memory without bound until the host is exhausted (84.8 GB across sessions, 64 MB free observed) #3114)".
• Coalesce thinking blocks by streamId to stop unbounded history growth (Fixes #3111) #3117: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 explicitly depends on and builds atop the thinking-block coalescing fix for unbounded history growth.
• Stop reporting Bun's nonexistent heap ceiling (Fixes #3112) #3123: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 explicitly depends on and builds atop the Bun heap-reporting and sandbox memory-argument fix.
• Return conversation history as a read-only view instead of deep-cloning it on every call (Fixes #3109) #3125: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 explicitly depends on and builds atop the read-only history-view fix that removed per-call deep cloning.
• Bound and rotate error reports (Fixes #3113) #3124: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 explicitly depends on and builds atop the bounded and rotated error-report fix.
• getHistory() deep-clones the entire conversation on every call (performance, not a retention cause) #3109: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 builds atop Return conversation history as a read-only view instead of deep-cloning it on every call (Fixes #3109) #3125 and the review context repeatedly discusses getHistory() deep-clones the entire conversation on every call (performance, not a retention cause) #3109 as a complementary performance fix that was cleared as a retention cause.
• Memory leak: thinking/reasoning blocks accumulate unboundedly in content.blocks (the real 11.8GB leak) #3111: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 builds atop Coalesce thinking blocks by streamId to stop unbounded history growth (Fixes #3111) #3117 and the review context identifies Memory leak: thinking/reasoning blocks accumulate unboundedly in content.blocks (the real 11.8GB leak) #3111 as the primary steady-state leak whose fix is required for the memory plateau.
• Footer reports a fabricated heap limit and startup relaunches for a --max-old-space-size that Bun ignores #3112: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 builds atop Stop reporting Bun's nonexistent heap ceiling (Fixes #3112) #3123 and the review context identifies Footer reports a fabricated heap limit and startup relaunches for a --max-old-space-size that Bun ignores #3112 as a compounding defect that worsened diagnosis of the memory exhaustion.
• Error reports write the entire conversation to temp files with no rotation (3,565 files / 62 MB observed) #3113: related because PR Release per-turn stream state and prove the memory plateau (Fixes #3114) #3127 builds atop Bound and rotate error reports (Fixes #3113) #3124 and the review context identifies Error reports write the entire conversation to temp files with no rotation (3,565 files / 62 MB observed) #3113 as a compounding defect surfaced by the same investigation.

---

Walkthrough generated by LLxprt PR Review. Planner issue: #2256

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

scripts/issue-2852-memory-benchmark.ts (1)

172-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the metric name to failures raised by evaluatePostGcPlateau.

evaluatePostGcPlateau throws 'Post-GC heap readings must be positive' when the settled baseline is not positive. externalBytes and webkitMallocDirtyBytes can read as 0 in a valid checkpoint, and requireMetric in scripts/issue-2852-memory-runner.ts accepts 0 as a finite value. The runner then fails with a heap-specific message for a non-heap metric. Wrap the call so the error names the failing metric.

♻️ Proposed refactor to attach the metric name

   const metricResults = extractors.map(({ name, read }) => {
     const series = samples.map(read);
-    const result = evaluatePostGcPlateau(series, tolerance);
-    return { name, ...result };
+    try {
+      return { name, ...evaluatePostGcPlateau(series, tolerance) };
+    } catch (error) {
+      throw new Error(`Metric ${name}: ${(error as Error).message}`);
+    }
   });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/issue-2852-memory-benchmark.ts` around lines 172 - 176, Update the
metric evaluation in the extractor map around evaluatePostGcPlateau so thrown
errors are caught and rethrown with the current metric name included, while
preserving the original error details and successful results unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts`:
- Around line 178-208: In the noncooperative iterator test, tighten the
elapsed-time assertion after turn.run completes to a CI-tolerant threshold close
to the documented one-second cleanup timeout, rather than allowing up to five
seconds. Keep the existing event and returnCalled assertions unchanged.

In `@scripts/tests/issue-2852-memory-plateau.bun.test.ts`:
- Around line 114-126: Update the test around evaluateMultiMetricPlateau to
assert the heap metric’s growthRatio in addition to settledBaselineBytes and
withinTolerance. Keep the existing scenario and assertions, and use the expected
ratio for the 40M-to-55M input.

---

Nitpick comments:
In `@scripts/issue-2852-memory-benchmark.ts`:
- Around line 172-176: Update the metric evaluation in the extractor map around
evaluatePostGcPlateau so thrown errors are caught and rethrown with the current
metric name included, while preserving the original error details and successful
results unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9e866e27-9da9-4251-a650-b70faa36d609

📥 Commits

Reviewing files that changed from the base of the PR and between a805a21 and 7257ad1.

⛔ Files ignored due to path filters (1)

• project-plans/issue3114-memory-plateau.md is excluded by !project-plans/**

📒 Files selected for processing (7)

• docs/sandbox.md
• packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts
• packages/agents/src/core/turn.ts
• scripts/issue-2852-memory-benchmark.ts
• scripts/issue-2852-memory-runner.ts
• scripts/issue-2852-memory-target.ts
• scripts/tests/issue-2852-memory-plateau.bun.test.ts

[github-actions]

OpenCodeReview — automatic reviews suspended

Automatic OCR reviews are suspended for this PR after 2 of 2 automatic reviews.

To get more reviews you can:

• Check the box below to re-enable automatic reviews (resets the counter), or

• Comment /review, /ocr, or /open-code-review to request a single review on demand.

• Re-enable automatic reviews

[acoliver]

Fixed in e9c923a.

Agreed — a 5s allowance against a documented 1s bound would have let a regression that adds another whole second to every turn pass unnoticed, which is exactly the class of regression this test exists to catch. Tightened to 2.5s: still enough slack for scheduling jitter on a loaded CI runner, but it now fails on any additional second of cleanup latency.

The event and returnCalled assertions are unchanged.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/issue-2852-memory-benchmark.ts`:
- Line 119: Treat whitespace-only JSONL lines as blank by updating the line
filter in scripts/issue-2852-memory-benchmark.ts (119-119) to trim before
checking length. Add a whitespace-only input and assertion that it is ignored in
scripts/tests/issue-2852-memory-plateau.bun.test.ts (133-146).

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 140e032f-546f-4f98-8822-19bbd4f3e0f0

📥 Commits

Reviewing files that changed from the base of the PR and between 7257ad1 and 37765cb.

📒 Files selected for processing (4)

• packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts
• scripts/issue-2852-memory-benchmark.ts
• scripts/issue-2852-memory-runner.ts
• scripts/tests/issue-2852-memory-plateau.bun.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/agents/src/core/turn.cooperative-cleanup.bun.test.ts
• scripts/issue-2852-memory-runner.ts