fix: keep PTY output flow control bounded

[htazq]

Summary

• return exactly one PTY output credit for each WebSocket frame after xterm processes its RAF batch
• immediately return credits for frames buffered by the streaming UTF-8 decoder without producing text
• remove the cumulative 2 MiB reset() / clear() path and rely on the existing bounded xterm scrollback
• add regression coverage for initial credits, single and batched frames, split UTF-8, sustained output, and tab activation

Root cause

The backend consumes one semaphore permit per output frame, but the frontend replenished a fixed four permits after processing any low-water batch. creditsGranted was never decremented when frames arrived and was reset immediately after granting, so a low-rate stream could grow the backend credit window by roughly three permits per frame.

The separate 2 MiB guard counted all decoded output since the last clear, not retained terminal memory. Normal sustained output eventually called both term.reset() and term.clear() even though xterm scrollback is already bounded by its configured line limit.

After this change, the invariant is: available credits plus frames awaiting xterm completion remain equal to the initial window. An incomplete UTF-8 sequence retains only the decoder's pending bytes and returns that frame credit immediately, avoiding deadlock.

Related work

• Fixes Bug: terminal clears all scrollback after only 2 MiB of cumulative output #60.
• fix: preserve terminal scrollback history #34 already bounds configurable xterm scrollback; this change relies on that existing mechanism.
• Windows: r-shell.exe virtual memory grows unbounded during high PTY output #26 / fix: bound PTY websocket output buffering #27 addressed the downstream WebSocket queue and binary frames.
• fix: bound PTY output memory growth #30 targets possible russh-side accumulation when downstream reading stalls. It is related but distinct and is intentionally not included here.

Validation

Reproduced on current main (547e1bb) before the fix:

• one processed frame: 6 Resume messages instead of 3 total
• three batched frames: 6 instead of 5 total
• split UTF-8 across two frames: 6 instead of 4 total
• sustained output over 2 MiB: term.reset() called once

Passing after the fix:

• pnpm test -- src/__tests__/pty-terminal-activation.test.tsx src/__tests__/terminal-scrollback.test.ts src/__tests__/terminal-tab-portals.test.tsx - 14 passed
• pnpm test - 569 passed
• pnpm exec tsc --noEmit
• pnpm i18n:check - 950 keys in both locales
• pnpm build
• pnpm exec eslint src/components/pty-terminal.tsx --quiet
• git diff --check

Repository-wide pnpm lint remains blocked by the pre-existing src/components/settings-modal.tsx:308 no-unnecessary-type-assertion error; the changed source file passes strict lint. Local cargo test reached the unrelated openssl-sys build script and exceeded the 304-second validation limit; this PR changes no Rust files, and PR CI runs the Rust suite on Windows, macOS, and Ubuntu.

Risk / rollback

The change reduces the maximum in-flight output to the existing two-frame window. If xterm stops completing writes, output backpressures instead of accumulating permits. Rollback is the single commit in this PR.

[htazq]

Refreshed this PR onto current main (547e1bb) and re-ran the regression loop. The four focused tests fail on main with the original credit counts / 2 MiB reset, then pass with this one-commit branch. Full frontend suite is 569/569; cross-platform CI is running on the refreshed head.

[GOODBOY008]

Reviewed the rebased head (45890f0, which includes the maintainer rebase onto current main — conflict resolution in pty-terminal.tsx plus removal of the now-unused ptyTerminal.outputLimitReached locale keys).

Verdict: correct, and the invariant finally holds end-to-end. What I verified against the backend protocol:

1. Exact credit accounting. The backend reader consumes exactly one permit per sent frame (credits.acquire() + forget() before each flush_output — both branches in websocket_server.rs), and this change returns exactly one credit per received frame: frameCount is captured at flush time and granted only from the term.write() callback, i.e. after xterm has actually processed the batch. The old leak (creditsGranted zeroed right after every grant, so the creditsGranted < CREDIT_BATCH * 2 guard never bound anything) is gone along with the variable.
2. No deadlock paths. writeBuffer and bufferedFrameCount are only mutated together synchronously; frames arriving mid-write reschedule the flush; and the empty-decode immediate grant covers the streaming decoder holding an incomplete UTF-8 sequence. I couldn't construct a sequence that loses a permit or double-grants.
3. Bounded under stall. If rAF stops (hidden window), credits stop returning, the backend blocks on credits.acquire(), and buffering is capped at ~2 frames + the 256-message WS queue + the 16 KB accumulator — a few MB worst case, self-recovering on visibility.
4. 2 MiB reset removal is safe. Scrollback is hard-capped at MAX_TERMINAL_SCROLLBACK = 50000 lines with xterm trimming internally, so the removed guard bought nothing except destroyed history/alt-screen state (#60).
5. Tests are substantive — they hold the write callback to prove grants happen only after xterm processes, assert exact permit totals (2→3, 2+3, 2+1+1 for split UTF-8, 132 across >2 MiB), and verify decoder reassembly across frames.

CI is green on all three platforms. Two non-blocking notes inline.

[GOODBOY008]

The window of 2 is adequate today, but only because of a coupling with the backend: the reader flushes at most OUTPUT_FLUSH_BYTES = 16 KB per OUTPUT_FLUSH_INTERVAL_MS = 10, capping production at ~1.6 MB/s — just below what 2 frames per rAF cycle (~2 MB/s at 60 Hz) can drain. If either backend constant is ever raised (e.g. for high-throughput cat of large files), this window silently becomes the bottleneck and would need to scale with it. Worth a one-line note here so future tuning doesn't miss it. Non-blocking.

[GOODBOY008]

Each credit is its own WS message, so a burst of N frames flushed together (e.g. output queued while the window was hidden) sends N tiny JSON Resume frames back-to-back. Fine at current volumes, but a natural follow-up would be extending the protocol to Resume { connection_id, count } with the backend doing add_permits(count) — the frontend already knows the batch size here. Non-blocking.

[GOODBOY008]

Nice catch — without this immediate return, a multi-byte character split across frames would park a permit in the decoder forever (the frame is consumed but produces no text), eventually starving the backend reader after INITIAL_WINDOW such frames. The split-中 regression test covers exactly this.

[GOODBOY008]

Approving after full review (see previous COMMENTED review for the detailed verification): credit invariant holds end-to-end against the backend protocol, no permit-loss paths, bounded under rAF stall, 2 MiB reset removal safe under the 50k-line scrollback cap, substantive regression tests, CI green on all platforms. The two inline notes are non-blocking follow-ups (Resume batch count, INITIAL_WINDOW coupling note). CI: green.