fix(terminal): cap per-session output buffers so massive process outp…

[edgarsskore]

A process emitting more output than V8's max string length (~536M chars) made string concatenation throw "RangeError: Invalid string length" inside a stdout 'data' handler in terminal-manager.ts. That uncaught exception hits the handler in index.ts which exits the process — the shared MCP server died silently ~4s into the flood and every command from every chat failed until restart. Reported as the "Invalid string length" / "MCP freezing in multiple chats" bugs. Reproduced with a finite ~600MB single-line flood.

Fix:

• Cap the per-session line buffer at 50MB chars, evicting oldest lines first; lastReadIndex is adjusted so pagination stays correct.
• Force-split single lines past 1MB so a process printing without newlines can't grow one unevictable line.
• Stop growing the start_process wait buffer after the call resolves and keep only a bounded sliding tail. This also bounds the periodic analyzeProcessState scan, which ran regex over the full accumulated output every 100ms.
• Track evicted chars/lines so interact_with_process snapshot offsets remain absolute and join() costs stay bounded.

Trade-offs: only the most recent ~50MB of a session's output remains readable (redirect to a file for full logs), and single lines longer than 1MB are split into multiple buffer lines.

Adds test/integration/terminal-output-buffer-leak.js, which drives the built TerminalManager in-process with a ~600MB flood that continues after executeCommand returns: asserts the buffer stays capped, the event loop never stalls, the output tail stays readable, and snapshot reads survive eviction. Crashes with "Invalid string length" on unfixed code; passes in ~1s with the fix.

Summary by CodeRabbit

• Bug Fixes

  • Terminal output buffering is now bounded per session with automatic eviction of oldest lines to prevent unbounded memory growth and crashes.
  • Snapshot-based reads and output history remain consistent even when older output has been evicted.
  • Completed session records now include eviction metadata so stored output lengths and snapshots are accurate.

• New Features

  • Status messages now indicate if older output was evicted and show the retained-buffer cap.

• Tests

  • Added an integration test validating buffer bounds, eviction behavior, and snapshot reads during heavy streaming.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a045fdca-6e42-4bac-92ed-074d8eff1cae

📥 Commits

Reviewing files that changed from the base of the PR and between e3efb52 and 3ca856d.

📒 Files selected for processing (3)

• src/terminal-manager.ts
• src/tools/improved-process-tools.ts
• test/integration/terminal-output-buffer-leak.js

✅ Files skipped from review due to trivial changes (1)

• src/tools/improved-process-tools.ts

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

• src/terminal-manager.ts
• test/integration/terminal-output-buffer-leak.js

---

📝 Walkthrough

Walkthrough

This PR hardens per-session output buffering in TerminalManager to prevent V8 "Invalid string length" crashes. Transient stdout/stderr buffers are now capped to recent characters, the line buffer evicts oldest lines when total characters exceed a per-session limit, and snapshot reads are updated to remain valid across eviction by tracking absolute character offsets and evicted character counts.

Changes

Per-Session Output Buffering with Eviction

Layer / File(s)	Summary
Buffer bounds constants and TerminalSession accounting fields src/terminal-manager.ts, src/types.ts	New constants (MAX_BUFFERED_OUTPUT_SIZE, MAX_LINE_SIZE) define per-session output limits. TerminalSession gains bufferedChars, evictedLines, and evictedChars fields to track current and historical buffer state.
Intermediate output stream capping in handlers src/terminal-manager.ts	Stdout and stderr handlers cap their transient output buffers to a bounded tail while the command is unresolved, preventing intermediate accumulation during streaming.
Line eviction logic in appendToLineBuffer src/terminal-manager.ts	Core buffering now tracks total character count, force-splits overlong lines, and evicts oldest lines when the per-session cap is exceeded, updating eviction metadata and read indices. Completed sessions persist evictedLines and evictedChars.
Snapshot accounting and output-since-snapshot reads with eviction src/terminal-manager.ts	Snapshot offsets are treated as absolute character counts since process start, incorporating evictedChars so reads remain valid across eviction. Output-since-snapshot returns only the retained tail when earlier data was evicted.
Integration test for bounded buffer retention and eviction test/integration/terminal-output-buffer-leak.js	Regression test floods stdout with ~600MB, verifies retained output stays within cap and eviction occurs, confirms event-loop responsiveness, and validates snapshot-based reads work correctly after heavy eviction.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• wonderwhy-er/DesktopCommanderMCP#210: Touches completed-session output reading paths that interact with improved completed-session output metadata.

Poem

A terminal once bloated beyond bound,
Now wears a cap and trims the ground—
Old lines are nudged out, the tail stays bright,
Snapshots still find the final light,
Rabbit hops, the buffer sleeps safe tonight. 🐰

🚥 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: capping per-session output buffers to prevent massive process output from causing crashes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/process-output-buffer-cap

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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