Repository navigation
fix(pty-daemon): pause flooding PTYs on backpressure instead of destroying the shared socket - #5747
Conversation
…oying the shared socket
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit cc9b321 on Jul 17, 2026 9:20pm UTC. |
📝 WalkthroughWalkthroughThe PTY daemon adds guarded pause/resume operations, applies outbound congestion thresholds per connection, resumes paused sessions after draining or disconnects, and adds integration coverage for slow consumers and interrupted connections. ChangesPTY flow control
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Socket as Client socket
participant Server as Server
participant PTY as Pty
Client->>Socket: Pause reading
PTY->>Server: Produce output
Server->>Socket: Write output
Socket-->>Server: Buffer exceeds threshold
Server->>PTY: pause()
Client->>Socket: Resume reading
Socket-->>Server: drain
Server->>PTY: resume()
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@packages/pty-daemon/src/Server/Server.ts`:
- Around line 541-554: Update resumePausedSessions to iterate over a snapshot of
conn.pausedSessions rather than the live Set, while preserving deletion,
ownership checks, and session resume behavior. Ensure synchronous pty.resume
callbacks that re-add IDs do not affect the current iteration.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 710fa3b4-4199-446a-89f9-7b19b20944e4
📒 Files selected for processing (8)
packages/pty-daemon/package.jsonpackages/pty-daemon/src/Pty/Pty.tspackages/pty-daemon/src/Server/Server.tspackages/pty-daemon/src/SessionStore/SessionStore.test.tspackages/pty-daemon/src/SessionStore/snapshot.test.tspackages/pty-daemon/src/handlers/handlers.test.tspackages/pty-daemon/test/byte-fidelity.test.tspackages/pty-daemon/test/flow-control.test.ts
| private resumePausedSessions(conn: ConnState): void { | ||
| for (const id of conn.pausedSessions) { | ||
| conn.pausedSessions.delete(id); | ||
| let heldElsewhere = false; | ||
| for (const other of this.conns) { | ||
| if (other !== conn && other.pausedSessions.has(id)) { | ||
| heldElsewhere = true; | ||
| break; | ||
| } | ||
| } | ||
| if (heldElsewhere) continue; | ||
| const session = this.store.get(id); | ||
| if (session && !session.exited) session.pty.resume(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Fix potential infinite loop during Set iteration.
Iterating over conn.pausedSessions while synchronously deleting and calling an external callback (pty.resume()) can cause an infinite loop. If session.pty.resume() causes the PTY to emit cached data synchronously, the onData handler in wireSession will run. If the connection becomes congested again, session.id is added back to conn.pausedSessions. Because JavaScript Set iterators visit elements appended during iteration (even if they were previously deleted), the re-added id will be visited again, locking the event loop indefinitely.
Clone the Set before iterating to safely resume sessions without modifying the collection being traversed.
🔒️ Proposed fix to isolate the iteration state
private resumePausedSessions(conn: ConnState): void {
- for (const id of conn.pausedSessions) {
- conn.pausedSessions.delete(id);
+ const toResume = Array.from(conn.pausedSessions);
+ conn.pausedSessions.clear();
+ for (const id of toResume) {
let heldElsewhere = false;
for (const other of this.conns) {
if (other !== conn && other.pausedSessions.has(id)) {
heldElsewhere = true;
break;
}
}
if (heldElsewhere) continue;
const session = this.store.get(id);
if (session && !session.exited) session.pty.resume();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private resumePausedSessions(conn: ConnState): void { | |
| for (const id of conn.pausedSessions) { | |
| conn.pausedSessions.delete(id); | |
| let heldElsewhere = false; | |
| for (const other of this.conns) { | |
| if (other !== conn && other.pausedSessions.has(id)) { | |
| heldElsewhere = true; | |
| break; | |
| } | |
| } | |
| if (heldElsewhere) continue; | |
| const session = this.store.get(id); | |
| if (session && !session.exited) session.pty.resume(); | |
| } | |
| private resumePausedSessions(conn: ConnState): void { | |
| const toResume = Array.from(conn.pausedSessions); | |
| conn.pausedSessions.clear(); | |
| for (const id of toResume) { | |
| let heldElsewhere = false; | |
| for (const other of this.conns) { | |
| if (other !== conn && other.pausedSessions.has(id)) { | |
| heldElsewhere = true; | |
| break; | |
| } | |
| } | |
| if (heldElsewhere) continue; | |
| const session = this.store.get(id); | |
| if (session && !session.exited) session.pty.resume(); | |
| } | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import * as childProcess from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@packages/pty-daemon/src/Server/Server.ts` around lines 541 - 554, Update
resumePausedSessions to iterate over a snapshot of conn.pausedSessions rather
than the live Set, while preserving deletion, ownership checks, and session
resume behavior. Ensure synchronous pty.resume callbacks that re-add IDs do not
affect the current iteration.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 8 files
You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/pty-daemon/src/Server/Server.ts">
<violation number="1" location="packages/pty-daemon/src/Server/Server.ts:481">
P2: Lowering `outboundBufferCap` below the default pause threshold still disconnects the shared socket before flow control runs. Derive or validate a pause threshold below the configured cap so custom buffer limits retain the new isolation behavior.</violation>
<violation number="2" location="packages/pty-daemon/src/Server/Server.ts:492">
P2: Replay bursts bypass the new per-session backpressure check, so subscribing a slow socket to enough buffered sessions can still hit `outboundBufferCap` and disconnect it. Account for replay writes and pause their source sessions when that socket crosses the threshold.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Flood as Flooding Process (e.g., yes)
participant Pty as Session PTY
participant Daemon as Daemon Server
participant Sub as Subscriber Conn (slow)
participant Socket as Sub TCP Socket
Note over Flood,Socket: NEW: Per-session flow control at daemon→host-service hop
Flood->>Pty: writes flood data
Pty->>Daemon: onData(chunk)
Daemon->>Daemon: for each subscriber conn
Daemon->>Sub: send(output, chunk)
Sub->>Socket: write() (buffers growing)
Daemon->>Daemon: check writableLength > pauseThreshold (1 MiB)
alt writableLength > threshold
Daemon->>Daemon: record session in conn.pausedSessions
Daemon->>Pty: NEW: pty.pause()
Pty->>Flood: kernel PTY buffer fills → flood process blocks on write
Note over Pty,Flood: Self-throttling at source – no more onData until resume
end
Note over Daemon,Sub: Resume path 1: socket drains
Socket->>Sub: drain event (buffer fully flushed)
Sub->>Daemon: 'drain' event
Daemon->>Daemon: resumePausedSessions(conn)
Daemon->>Daemon: for each session in conn.pausedSessions
alt no other conn holds session paused
Daemon->>Pty: NEW: pty.resume()
Pty->>Flood: allows writes again
Flood->>Pty: writes continue
else another conn still congested
Daemon->>Daemon: skip resume – session stays paused
end
Note over Daemon,Sub: Resume path 2: subscriber disconnects (socket destroyed)
Sub->>Socket: close/destroy
Daemon->>Daemon: dropConn(conn)
Daemon->>Daemon: resumePausedSessions(conn) – same logic as above
Daemon->>Pty: pty.resume() (if no other conn holds it)
Pty->>Flood: allows writes again
Note over Daemon,Socket: Destroy cap (8 MiB) unchanged – last-resort fallback
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (!c.subscriptions.has(session.id)) continue; | ||
| c.send(out, chunk); | ||
| if (!c.socket.destroyed && c.socket.writableLength > pauseThreshold) { | ||
| c.pausedSessions.add(session.id); |
There was a problem hiding this comment.
P2: Replay bursts bypass the new per-session backpressure check, so subscribing a slow socket to enough buffered sessions can still hit outboundBufferCap and disconnect it. Account for replay writes and pause their source sessions when that socket crosses the threshold.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pty-daemon/src/Server/Server.ts, line 492:
<comment>Replay bursts bypass the new per-session backpressure check, so subscribing a slow socket to enough buffered sessions can still hit `outboundBufferCap` and disconnect it. Account for replay writes and pause their source sessions when that socket crosses the threshold.</comment>
<file context>
@@ -460,13 +478,24 @@ export class Server {
if (!c.subscriptions.has(session.id)) continue;
c.send(out, chunk);
+ if (!c.socket.destroyed && c.socket.writableLength > pauseThreshold) {
+ c.pausedSessions.add(session.id);
+ congested = true;
+ }
</file context>
| * subscribed to this session id receives the output / exit frames. | ||
| */ | ||
| private wireSession(session: Session): void { | ||
| const pauseThreshold = |
There was a problem hiding this comment.
P2: Lowering outboundBufferCap below the default pause threshold still disconnects the shared socket before flow control runs. Derive or validate a pause threshold below the configured cap so custom buffer limits retain the new isolation behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pty-daemon/src/Server/Server.ts, line 481:
<comment>Lowering `outboundBufferCap` below the default pause threshold still disconnects the shared socket before flow control runs. Derive or validate a pause threshold below the configured cap so custom buffer limits retain the new isolation behavior.</comment>
<file context>
@@ -460,13 +478,24 @@ export class Server {
* subscribed to this session id receives the output / exit frames.
*/
private wireSession(session: Session): void {
+ const pauseThreshold =
+ this.opts.outboundPauseThreshold ??
+ DEFAULT_OUTBOUND_PAUSE_THRESHOLD_BYTES;
</file context>
Greptile SummaryThis PR introduces per-session flow control to the pty-daemon to prevent a flooding PTY (e.g.,
Confidence Score: 4/5The core flow-control mechanism is logically sound and well-tested for the happy path; the main concerns are an unvalidated configuration invariant and a timing-dependent test. The drain-guarantee invariant is correct for all current defaults and tests, but the ServerOptions interface allows arbitrary values with no validation. The 800ms sleep in test 2 may not hold on resource-constrained runners, making the conn-drop resume path potentially unexercised. packages/pty-daemon/src/Server/Server.ts (drain-guarantee invariant, Set iteration pattern) and packages/pty-daemon/test/flow-control.test.ts (timing-dependent pause-before-disconnect assertion).
|
| Filename | Overview |
|---|---|
| packages/pty-daemon/src/Server/Server.ts | Core flow control logic: adds pausedSessions tracking per connection, pause on writableLength > threshold, and resume on socket drain and conn drop; the drain guarantee relies on an implicit invariant that threshold > socket highWaterMark. |
| packages/pty-daemon/src/Pty/Pty.ts | Adds pause()/resume() to the Pty interface and implements them in both NodePtyAdapter and AdoptedPty with exited guards; straightforward delegation. |
| packages/pty-daemon/test/flow-control.test.ts | Two integration tests covering the slow-consumer and conn-drop-while-paused paths; test 2 relies on timing assumptions (PTY must be paused within 800ms) that could flake on slow CI. |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/pty-daemon/src/Server/Server.ts:541-555
**Set mutated while being iterated**
`conn.pausedSessions.delete(id)` removes the element that was just yielded by the outer `for...of` iterator over that same Set. This is technically safe in JavaScript — the spec guarantees the remaining elements are still visited — but it's an unusual pattern that relies on this subtlety. A harder-to-misread form would be to snapshot the set first and clear it up front:
```ts
const ids = [...conn.pausedSessions];
conn.pausedSessions.clear();
for (const id of ids) { … }
```
This also makes it clearer that the set is meant to be fully drained on every call.
### Issue 2 of 3
packages/pty-daemon/src/Server/Server.ts:338-341
**`drain` guarantee depends on an unvalidated invariant**
The comment correctly explains why `drain` is guaranteed: `writableLength > outboundPauseThreshold` implies `writableLength > highWaterMark` (default 16 KB for a Node.js `net.Socket`), which means `socket.write()` has already returned `false`, guaranteeing a future `drain`. This holds for the defaults (1 MiB pause threshold >> 16 KB HWM) and the test value (64 KB), but the `ServerOptions` interface accepts any value. If a caller passes `outboundPauseThreshold` below the socket's `highWaterMark` (e.g., 4 KB), the PTY could be paused with no `drain` guaranteed to fire, silently stalling the session until conn drop. An assertion or clamp at construction time would prevent this.
### Issue 3 of 3
packages/pty-daemon/test/flow-control.test.ts:89-92
**Fixed 800 ms sleep to guarantee PTY is paused**
The test pauses `c1`'s socket and sleeps for 800 ms, expecting the PTY to have been paused within that window. On a busy CI runner (or a system where `yes` is slow to start / the socket send buffer is large), 800 ms may not be enough time for the server's `writableLength` to exceed the 64 KB `PAUSE_THRESHOLD`, so `conn.pausedSessions` would be empty when `c1.socket.destroy()` fires and `resumePausedSessions` becomes a no-op — the PTY was never paused. `c2`'s `waitFor` would then succeed trivially (the PTY was always running), meaning the conn-drop resume path was never exercised and the test is a false positive.
Reviews (1): Last reviewed commit: "fix(pty-daemon): pause flooding PTYs on ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private resumePausedSessions(conn: ConnState): void { | ||
| for (const id of conn.pausedSessions) { | ||
| conn.pausedSessions.delete(id); | ||
| let heldElsewhere = false; | ||
| for (const other of this.conns) { | ||
| if (other !== conn && other.pausedSessions.has(id)) { | ||
| heldElsewhere = true; | ||
| break; | ||
| } | ||
| } | ||
| if (heldElsewhere) continue; | ||
| const session = this.store.get(id); | ||
| if (session && !session.exited) session.pty.resume(); | ||
| } | ||
| } |
There was a problem hiding this comment.
conn.pausedSessions.delete(id) removes the element that was just yielded by the outer for...of iterator over that same Set. This is technically safe in JavaScript — the spec guarantees the remaining elements are still visited — but it's an unusual pattern that relies on this subtlety. A harder-to-misread form would be to snapshot the set first and clear it up front:
const ids = [...conn.pausedSessions];
conn.pausedSessions.clear();
for (const id of ids) { … }This also makes it clearer that the set is meant to be fully drained on every call.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pty-daemon/src/Server/Server.ts
Line: 541-555
Comment:
**Set mutated while being iterated**
`conn.pausedSessions.delete(id)` removes the element that was just yielded by the outer `for...of` iterator over that same Set. This is technically safe in JavaScript — the spec guarantees the remaining elements are still visited — but it's an unusual pattern that relies on this subtlety. A harder-to-misread form would be to snapshot the set first and clear it up front:
```ts
const ids = [...conn.pausedSessions];
conn.pausedSessions.clear();
for (const id of ids) { … }
```
This also makes it clearer that the set is meant to be fully drained on every call.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // Flow control: the buffer only reaches the pause threshold after | ||
| // write() has returned false, so a 'drain' (buffer fully flushed) is | ||
| // guaranteed to follow and is our resume signal. | ||
| socket.on("drain", () => this.resumePausedSessions(conn)); |
There was a problem hiding this comment.
drain guarantee depends on an unvalidated invariant
The comment correctly explains why drain is guaranteed: writableLength > outboundPauseThreshold implies writableLength > highWaterMark (default 16 KB for a Node.js net.Socket), which means socket.write() has already returned false, guaranteeing a future drain. This holds for the defaults (1 MiB pause threshold >> 16 KB HWM) and the test value (64 KB), but the ServerOptions interface accepts any value. If a caller passes outboundPauseThreshold below the socket's highWaterMark (e.g., 4 KB), the PTY could be paused with no drain guaranteed to fire, silently stalling the session until conn drop. An assertion or clamp at construction time would prevent this.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pty-daemon/src/Server/Server.ts
Line: 338-341
Comment:
**`drain` guarantee depends on an unvalidated invariant**
The comment correctly explains why `drain` is guaranteed: `writableLength > outboundPauseThreshold` implies `writableLength > highWaterMark` (default 16 KB for a Node.js `net.Socket`), which means `socket.write()` has already returned `false`, guaranteeing a future `drain`. This holds for the defaults (1 MiB pause threshold >> 16 KB HWM) and the test value (64 KB), but the `ServerOptions` interface accepts any value. If a caller passes `outboundPauseThreshold` below the socket's `highWaterMark` (e.g., 4 KB), the PTY could be paused with no `drain` guaranteed to fire, silently stalling the session until conn drop. An assertion or clamp at construction time would prevent this.
How can I resolve this? If you propose a fix, please make it concise.| // never emits 'drain'). | ||
| c1.socket.pause(); | ||
| await new Promise((r) => setTimeout(r, 800)); | ||
| c1.socket.destroy(); |
There was a problem hiding this comment.
The test pauses c1's socket and sleeps for 800 ms, expecting the PTY to have been paused within that window. On a busy CI runner (or a system where yes is slow to start / the socket send buffer is large), 800 ms may not be enough time for the server's writableLength to exceed the 64 KB PAUSE_THRESHOLD, so conn.pausedSessions would be empty when c1.socket.destroy() fires and resumePausedSessions becomes a no-op — the PTY was never paused. c2's waitFor would then succeed trivially (the PTY was always running), meaning the conn-drop resume path was never exercised and the test is a false positive.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pty-daemon/test/flow-control.test.ts
Line: 89-92
Comment:
**Fixed 800 ms sleep to guarantee PTY is paused**
The test pauses `c1`'s socket and sleeps for 800 ms, expecting the PTY to have been paused within that window. On a busy CI runner (or a system where `yes` is slow to start / the socket send buffer is large), 800 ms may not be enough time for the server's `writableLength` to exceed the 64 KB `PAUSE_THRESHOLD`, so `conn.pausedSessions` would be empty when `c1.socket.destroy()` fires and `resumePausedSessions` becomes a no-op — the PTY was never paused. `c2`'s `waitFor` would then succeed trivially (the PTY was always running), meaning the conn-drop resume path was never exercised and the test is a false positive.
How can I resolve this? If you propose a fix, please make it concise.
E2E verification: CDP flood stress re-run against this branch (dev app)Same harness that reproduced the bug (5 real sessions on the Main workspace, WS clients attached,
The flood is now bounded by consumer throughput instead of killing the shared socket, and the quiet terminal keeps working throughout. One observation for the follow-up work (not this PR): with the transport surviving, host-service now visibly pays the per-chunk relay cost — ~93% CPU at 44–50 MB/s (ModeTracker parse + WS broadcast on the event loop). That's the measurement the loop-offload plan ( |
…#5751) * perf(desktop): LRU-evict parked terminal runtimes past a cap Parked xterm instances were kept alive forever (5,000-line buffer + WebGL context each, no cap). Track recency on mount/detach and release the least-recently-used parked runtimes past the cap: buffer serialized to localStorage first, PTY untouched, rebuilt on next mount. Eviction is deferred one tick so workspace switches re-adopt before the sweep counts. * feat(desktop): user-configurable background-terminal cap Settings > Terminal > "Background terminal memory": number input (2-64, default 12, blur-commit like FontSettingSection). Stored on the local-db settings row (migration 0042), seeded into the registry at boot, applied live on change so lowering the cap reclaims memory immediately. * docs(desktop): memory profile & worker offload review CDP-verified findings across renderer/main/host-service/pty-daemon, controlled A/Bs for the parked-terminal eviction, and the ranked recommendation backlog (SUPER-1544..1550). * docs(desktop): note daemon backpressure fix landed (#5747) * test(desktop): parked-runtime LRU eviction unit tests Covers recency ordering (not insertion order), attached-runtime protection, deferred-sweep rescue on remount, live cap lowering, invalid cap rejection, buffer persistence on evict, and runtime-less entries. Each assertion verified to fail under a targeted impl mutation (sort removed, attached included, sweep made synchronous, setter sweep dropped, persist flag flipped). * test(desktop): extract pure eviction policy; drop process-global module mocks The first test cut used bun mock.module, which is process-wide and replaced the real terminal-ws-transport for its own suite (15 downstream failures in CI). Restructure along a real seam instead: the LRU selection + cap clamp move to dependency-free terminal-runtime-eviction.ts, unit-tested without mocks (mutation-verified: sort removal, attached inclusion, bare-entry counting, clamp boundary all caught; one equivalent mutant on the <=cap guard documented). Timer deferral and dispose wiring remain covered by the CDP verification in the PR. * style(host-service): fix biome formatting in ai-workspace-names origin/main tip fails biome 2.4.2 on this block, which fails lint on every PR's merge ref; carrying the fix here.
Links
plans/20260717-host-service-git-worker-pool.md(perf(host-service): bound large git watcher batches #5746)Summary
yes, a huge build log) used to trip the daemon's 8 MiB outbound cap andsocket.destroy()the daemon↔host-service connection — severing every terminal in the org (host-service logspty-daemon disconnected; closing N terminal WS socket(s); users see all terminals blip). Reproduced deterministically in the 2026-07-17 stress test.pty.pause()); resume on socketdrain. The kernel PTY buffer fills and the flooding process blocks on its own write — it throttles itself at the source. The 8 MiB destroy stays as a last-resort for pathological control-message floods.How It Works
Ptygainspause()/resume(): node-pty's public pause/resume (added upstream for VS Code's ptyHost flow control, same actuator) for spawned sessions,ReadStream.pause()for adopted-fd sessions.Server.wireSession: after fanning out a chunk, any subscribed conn withwritableLength > outboundPauseThresholdrecords the session inconn.pausedSessionsand the PTY is paused. Paused PTYs emit no further data, so this fires once per congestion episode.drain(guaranteed to fire — the buffer only passes the threshold afterwrite()returned false), and conn drop (a destroyed socket never drains). A session held paused by another still-congested conn stays paused.Manual QA Checklist
yes) with other terminals open — no org-wide disconnect, other terminals stay live (covered by integration test with real shells + real unix socket; app-level re-run of the CDP flood stress is a follow-up)Testing
test/flow-control.test.ts(node --test, real/bin/sh+yesflood + real unix socket):bun run test:integration(51/51) andbun testunit suite (57/57) passbun run typecheck(pty-daemon + host-service),bun run lintcleanRisks / Rollout
Summary by cubic
Prevent org-wide terminal disconnects by pausing flooding PTYs on backpressure instead of destroying the shared daemon socket. Adds per-session flow control to
pty-daemonso heavy output (e.g., yes/build logs) doesn’t take down other terminals.socket.writableLengthexceeds a threshold (default 1 MiB viaoutboundPauseThreshold), pause the session’s PTY; resume on socketdrainor connection drop. The 8 MiBoutboundBufferCapremains a last-resort.Ptywithpause()/resume()and wired bothnode-ptysessions and adopted-fd sessions.pausedSessionsper connection to avoid resuming while another subscriber is still congested.test/flow-control.test.tsand included it intest:integrationto cover slow consumer and disconnect-resume paths.Written for commit cc9b321. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests