Skip to content

perf(client-runtime): batch terminal input writes - #9456

Closed
burnmandont wants to merge 1 commit into
pingdotgg:mainfrom
burnmandont:perf/batch-terminal-input-writes
Closed

burnmandont wants to merge 1 commit into
pingdotgg:mainfrom
burnmandont:perf/batch-terminal-input-writes

Conversation

@burnmandont

@burnmandont burnmandont commented Sep 3, 2026 •

Copy link
Copy Markdown

Interactive terminal input currently sends every terminal-emulator callback through a separate unary request/response lifecycle. Rapid typing, escape sequences, and paste bursts can create many small RPCs, adding avoidable transport and bookkeeping overhead—especially for remote environments.

This adds a bounded, ordered input lane per environment/thread/terminal. Input queued in the same JavaScript task is coalesced into batches of up to 65,536 characters, batches are sent serially without waiting for response frames, and each lane rejects new input after 512 KiB is buffered. Web/Desktop and Mobile interactive terminal surfaces use the batched command; the existing direct write command remains available for one-off launch input.

Verification:

  • vp test run packages/client-runtime/src/state/terminalInput.test.ts — 4 passed
  • client-runtime, web, and mobile typechecks passed
  • targeted lint passed with pre-existing warnings in the two terminal components
  • targeted format check passed

Model: GPT-5.6 Sol
Harness: Sovereign / Codex


Note

Medium Risk
Changes how terminal input reaches remote PTYs (ordering, batching, backpressure failures, discard RPCs); mistakes could drop or reorder keystrokes under load.

Overview
Interactive terminal keystrokes and pastes no longer trigger one unary RPC per emulator callback. The PR adds terminalEnvironment.input, which batches and serializes writes per environment/thread/terminal while sending them in discard mode so the client does not wait on response frames.

Client runtime: createTerminalInputCommand coalesces input queued in the same microtask, splits payloads over 65 KiB (UTF-16 surrogate-safe), enforces a 512 KiB per-lane buffer with TerminalInputBackpressureError when exceeded, and chains sends on a per-terminal promise tail. requestDiscard is the new RPC helper used for these fire-and-forget terminalWrite calls. terminalEnvironment.write stays for one-off writes (e.g. mobile launch initialInput, chat-driven writes).

UI: Web ThreadTerminalDrawer and mobile ThreadTerminalRouteScreen route live keyboard/toolbar input through input instead of write.

Reviewed by Cursor Bugbot for commit 7bab8fd. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Batch terminal input writes through terminalEnvironment.input

  • Adds a per-terminal input scheduler in terminalInput.ts that buffers concurrent input, coalesces compatible entries up to 65,536 characters, splits oversized payloads into ordered chunks (without breaking surrogate pairs), and sends them serially within each lane.
  • Adds requestDiscard in client.ts — a unary RPC helper that fires the request in discard mode without waiting for a response frame.
  • Wires terminalEnvironment.input into terminal.ts and switches the mobile (ThreadTerminalRouteScreen.tsx) and web (ThreadTerminalDrawer.tsx) terminal writers from write to input.
  • Lanes exceeding 512 KiB of buffered input reject new requests with TerminalInputBackpressureError.
  • Risk: terminal input now uses discard-mode RPCs, so failed sends no longer surface a response frame error to the UI in the same way as the previous write path; failure reporting in the web drawer was already disabled and remains so.

Macroscope summarized 7bab8fd.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 3, 2026
Comment thread packages/client-runtime/src/state/terminalInput.ts
@macroscopeapp

macroscopeapp Bot commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — Interactive terminal writes on web and mobile are rerouted through a new stateful batching, backpressure, and discard-mode RPC path, changing existing runtime behavior rather than adding an opt-in capability. The implementation is substantial and carries an unresolved high-severity edge case in the review record.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@burnmandont
burnmandont force-pushed the perf/batch-terminal-input-writes branch from e6749f3 to fb0f72a Compare September 3, 2026 20:18
Comment thread apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx Outdated
@burnmandont
burnmandont force-pushed the perf/batch-terminal-input-writes branch from fb0f72a to 7bab8fd Compare September 4, 2026 23:00
Comment on lines +56 to +57
const maxBatchChars = options.maxBatchChars ?? TERMINAL_INPUT_MAX_BATCH_CHARS;
const maxBufferedChars = options.maxBufferedChars ?? TERMINAL_INPUT_MAX_BUFFERED_CHARS;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High state/terminalInput.ts:56

With maxBatchChars: 0, any nonempty input enters the oversized path, sends an empty data payload, and never advances offset, so the command promise never settles. Validate that maxBatchChars is positive before scheduling chunks.

 const maxBatchChars = options.maxBatchChars ?? TERMINAL_INPUT_MAX_BATCH_CHARS;
+  if (maxBatchChars <= 0) throw new RangeError("maxBatchChars must be positive");
 const maxBufferedChars = options.maxBufferedChars ?? TERMINAL_INPUT_MAX_BUFFERED_CHARS;
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/terminalInput.ts around lines 56-57:

With `maxBatchChars: 0`, any nonempty input enters the oversized path, sends an empty `data` payload, and never advances `offset`, so the command promise never settles. Validate that `maxBatchChars` is positive before scheduling chunks.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

Closing for missing prior scope approval. Interactive terminal writes now use a new buffered lane, discard response frames, and reject input at the buffer limit. Those transport and failure-behavior choices need maintainer agreement; no approval is linked or recorded here. Establish the performance problem and agree on that scope in an Ideas discussion before requesting reconsideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants