Skip to content

Stop keystrokes stalling on PTY input-stream open after re-attach#40

Merged
khaliqgant merged 2 commits into
mainfrom
fix/pty-input-stream-reattach
May 25, 2026
Merged

Stop keystrokes stalling on PTY input-stream open after re-attach#40
khaliqgant merged 2 commits into
mainfrom
fix/pty-input-stream-reattach

Conversation

@khaliqgant

@khaliqgant khaliqgant commented May 25, 2026

Copy link
Copy Markdown
Member

Problem

Typing into an agent (e.g. claude-1) could appear completely dead after a window reload, app restart, or broker churn — with no error in the UI. Reproduced live: after a dev-server restart, keystrokes to a healthy agent (PTY process confirmed alive) went nowhere; a Cmd+R "fixed" it only because by then the broker was fully up.

Root cause

Every keystroke went use-terminal → sendInputFast → broker.sendInput → getOrOpenInputStream → await stream.send(data). The SDK's PtyInputStream.send() queues and flush() awaits the open handshake (pty_input_ready) with a 10s open timeout. So when the WS stream couldn't open promptly — common right after a re-attach while the broker is still coming up — each keystroke blocked up to 10s before falling back to HTTP, and sendInputFireAndForget swallowed the failure with a console.warn.

Fix

The WS input stream stays the primary, fast path whenever it is open — i.e. the entire steady-state typing case is unchanged, and there is no latency regression. The only change is to the brief window where the stream is not yet open: instead of blocking the keystroke on the up-to-10s handshake, send it over HTTP (the broker's other, always-available input endpoint) and switch back to the WS stream the instant it's ready.

  • ensureInputStream (replaces getOrOpenInputStream) opens the WS in the background, never awaits the handshake, and reports a ready flag.
  • sendInput uses the WS stream when ready (the normal case); only the not-yet-open window falls through to HTTP — so a keystroke never waits on the open timeout.
  • Tracks readiness + consecutive open failures; after MAX_INPUT_STREAM_OPEN_FAILURES (3) it rides HTTP for that agent and logs it (a persistently dead fast path is surfaced, not hidden) until the terminal re-attaches.
  • attachTerminal clears the fallback so a re-attach gives the WS a fresh chance.

Net: WS remains primary; the sub-second "stream still opening" window no longer freezes typing.

Testing

  • npm test → 10/10 pass
  • tsc -p tsconfig.node.json → no new errors (broker.ts clean; 36 pre-existing elsewhere, identical on main)
  • Found by live debugging of a real session where claude-1 input died after a restart while its PTY was confirmed alive.

Note

Independent of #38/#39 (renderer memory/CPU); this is the main-process input path.

🤖 Generated with Claude Code

Typing into an agent could appear completely dead after a window reload,
app restart, or broker churn. Every keystroke went through the WS PTY input
stream via getOrOpenInputStream + `await stream.send(data)`. The SDK's
PtyInputStream.send() queues until the broker acks `pty_input_ready`, with a
10s open timeout — so when the stream couldn't open promptly (common right
after a re-attach while the broker is still coming up), each keystroke blocked
up to 10s before falling back to HTTP. sendInputFireAndForget swallowed the
failures with only a console.warn, so the UI showed nothing. And because
non-404 open failures never set the permanent fallback, a doomed WS was
re-opened over and over.

Make HTTP the reliable default and the WS stream a latency optimization that
only carries input once it is confirmed ready:

- ensureInputStream (replaces getOrOpenInputStream) opens the WS in the
  background and never awaits the handshake; it reports whether the stream is
  ready. sendInput only awaits stream.send when ready, otherwise sends over
  HTTP immediately — no keystroke ever waits on the open timeout.
- Track readiness (inputStreamReady) and consecutive open failures
  (inputStreamOpenFailures). After MAX_INPUT_STREAM_OPEN_FAILURES, stop
  retrying the WS for that agent and ride HTTP until it re-attaches.
- attachTerminal clears the HTTP-only fallback so a re-attach gives the WS a
  fresh chance instead of being stuck on HTTP for the agent's lifetime.
- Clean up all three per-agent maps/sets on close and per project.

Net effect: typing works instantly on re-attach (over HTTP), then transparently
upgrades to the low-latency WS stream once it opens.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 3 reviews of capacity. Refill in 8 minutes and 41 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

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

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 073d5fa2-17bd-45ce-9e50-24653825256a

📥 Commits

Reviewing files that changed from the base of the PR and between bc89eff and a6cda52.

📒 Files selected for processing (1)
  • src/main/broker.ts
📝 Walkthrough

Walkthrough

BrokerManager's PTY input-stream handling is refactored to decouple keystroke acceptance from WebSocket stream readiness. Stream opening is now asynchronous with an immediate ready flag; input routes to HTTP fallback until the broker acknowledges pty_input_ready. Repeated WS failures persistently enable HTTP-only mode until terminal reattach.

Changes

PTY Input Stream Non-Blocking Handling

Layer / File(s) Summary
Input stream readiness and failure tracking
src/main/broker.ts
MAX_INPUT_STREAM_OPEN_FAILURES constant introduced to guard repeated WS failures. Per-agent-name tracking added: inputStreamReady set for broker-acknowledged readiness and inputStreamOpenFailures map for consecutive open failure counts.
Stream lifecycle and readiness management
src/main/broker.ts
ensureInputStream refactored to return { ready: boolean } immediately with WS open handled asynchronously in the background. closeInputStream now clears readiness and failure state; resetInputStreamFallback resets fallback and failure counters; closeInputStreamsForProject extended to clear project readiness.
Terminal reattach and send path integration
src/main/broker.ts
On terminal reattach, WS fallback state is cleared so WS fast path can be retried. sendInput now calls ensureInputStream, checks the ready flag, and only sends over WS when ready; otherwise routes to HTTP fallback. On send errors, the stream is closed and HTTP fallback is conditionally enabled.

Sequence Diagram

sequenceDiagram
  participant Client
  participant sendInput
  participant ensureInputStream
  participant WS as WS Stream
  participant HTTP as HTTP Fallback
  Client->>sendInput: input to send
  sendInput->>ensureInputStream: get stream and ready status
  ensureInputStream-->>sendInput: stream, ready=false or true
  alt ready is true
    sendInput->>WS: stream.send(input)
    WS-->>sendInput: ack or error
    alt send error
      sendInput->>sendInput: close stream, mark fallback for unsupported
    end
  else ready is false
    sendInput->>HTTP: POST input (fallback path)
    HTTP-->>sendInput: ack
  end
Loading

🎯 4 (Complex) | ⏱️ ~45 minutes

🐰 A stream that wouldn't block,
Async opens tick-tick-tock,
Ready flags now light the way,
HTTP steps in till WS says yay,
Reattach clears the fallback gray!


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

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

Addresses review feedback: when an agent's WS input stream repeatedly fails
to open and we permanently fall back to HTTP, log it once. Transient
not-ready (the normal startup/re-attach window) stays silent and is handled
by HTTP; only a genuinely stuck stream — where the latency fast path is off
for the agent's lifetime — is surfaced.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@khaliqgant khaliqgant merged commit 9ca2c78 into main May 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant