fix(remote): harden terminal, relay, and SSH reliability#8141
Merged
Conversation
Large git diff/exec responses (up to MAX_GIT_BUFFER=10MB) were sent as one JSON-RPC frame on the shared SSH channel. encodeJsonRpcFrame's synchronous JSON.stringify + Buffer.from of the whole payload, plus the multi-MB write, head-of-line-blocked interactive pty.data echo: opening a big diff (e.g. a lockfile) over SSH while typing in another pane stalled the echo for seconds. The 2026-07 fs.readFileStream fix added a per-client serialized bulk lane (RelayDispatcher.notifyBulk) honoring write()===false + waitWriteDrain, plus an ack credit window — but only fs streams used it; git responses never did. Stream the diff family (git.diff/branchDiff/commitDiff) and git.exec through the bulk lane in chunks with the same ack-window backpressure. The relay returns a small sentinel result and pumps git.responseChunk frames; the client reassembles and JSON.parses. Small responses stay single-frame. Cross-version safe via an opt-in capability gate (no handshake change): - client sends __streamResponse:true; old relays ignore it → single-frame. - new relays only stream when the flag is present → old clients get single-frame. - new client checks for the stream sentinel; a plain result (old relay) is returned directly. STREAM_CHUNK_SIZE is untouched; git chunking uses its own GIT_RESPONSE_CHUNK_SIZE and the client reassembles by concatenation, so chunk size is not baked into any cross-version offset math. Co-authored-by: Orca <help@stably.ai>
… for git response streams Address self-review findings on the git response streaming reader: - MEDIUM: mux.request's timeout only bounded the sentinel; the reassembly phase had no deadline, so a relay pump that broke on staleness (sends no responseEnd) while the SSH channel stayed up would hang the client forever. Add a client-side inactivity timeout that resets on each chunk and rejects if no stream frame arrives within the window (default 30s, matching the mux request timeout). New regression test drives a post-sentinel stall. - LOW: widen the options type to declare timeoutMs (sentinel) and a distinct inactivityTimeoutMs (reassembly); forward only mux-request options to the sentinel request. - LOW: cap the pre-sentinel pending-frame buffer (MAX_PENDING_FRAMES) so concurrent streams cannot transiently buffer each other's chunks unbounded; foreign frames are dropped on drain anyway. Co-authored-by: Orca <help@stably.ai>
The multiplex output path silently drops Output frames when the websocket buffer exceeds MAX_BINARY_BUFFERED_AMOUNT (encryptedBinaryReply returns false). The client never noticed the gap, so a congested link (fast PTY over slow/SSH transport) permanently corrupted the remote terminal with missing/garbled output and no error. Output frame seq is a byte high-water, so a drop is self-describing on the wire. The client now tracks the expected next offset and, on a gap, discards the corrupt tail and pulls a fresh authoritative snapshot (self-healing, no user-facing error). Server-side, seq-less Output chunks now carry the sentinel 0 instead of the control-frame cursor so they don't poison the client's gap detector, and sendFrame propagates the send/drop result end-to-end. Adds an end-to-end repro test driving the real client multiplexer through a fake backpressured transport. Co-authored-by: Orca <help@stably.ai>
A shared-control keepalive frame refreshed the timeout of every pending request on the shared socket. Those pending requests are ordinary short RPCs (git/repo/session-tabs), while the keepalive is armed server-side only for an unrelated long-poll on the same socket. So while any long-poll kept keepalives flowing, a genuinely-stuck short RPC never hit its deadline: the caller hung forever, the timeout-driven teardown/reconnect/ replay never fired, and eight such hangs exhausted the per-environment call queue (concurrency 8), wedging all further control-plane calls. Tag each pending request with `refreshTimeoutOnKeepalive` (default false). Short RPCs now keep an absolute deadline so a stuck server call times out, tears the socket down, and reconnects/replays as designed. Only requests that explicitly opt in (long-polls routed through the short-RPC path) may have their deadline extended by keepalives — preserving that escape hatch without changing any current caller, since today's long-polls run on the subscription map, not pendingRequests. Applies to both SSH remote-runtime and remote Orca Server topologies, which share this control connection.
terminal.send was already hardened (#7827) to resolve handles through the guarded runtime.resolveLiveLeafForHandle, which throws terminal_handle_stale when a pane's PTY was replaced under a handle (restart/re-spawn bumps ptyId/generation). The geometry family still used the UNGUARDED resolveLeafForHandle and then mutated PTY state: - terminal.resizeForClient - terminal.setDisplayMode - terminal.restoreFit - terminal.updateViewport The unguarded resolver returns the pane's CURRENT pty, so a remote client holding a stale handle resized/refit/viewported the WRONG (new) pty — mysterious geometry corruption on the fresh session. Switch all four to resolveLiveLeafForHandle. The return shape is identical ({ ptyId }), so no follow-on logic changes. The read-only terminal.getDisplayMode, the already-guarded multiplex path, and the legacy subscribe-time resolution are left untouched. Clients already handle terminal_handle_stale gracefully (no red banners): updateViewport/restoreFit/setDisplayMode callers swallow the error, and the remote-runtime transport classifies terminal_handle_stale as "terminal gone" → retires the mirror and re-derives the handle from the next session-tabs snapshot. Applies to both SSH remote-runtime and remote Orca Server, which share these control-plane RPCs.
Two remote-terminal ws paths wrote to the socket with no backpressure handling, so a fast producer over a slow/congested link ballooned ws.bufferedAmount and main-process RSS without bound: - Server text/JSON reply path (e2ee-channel encryptedReply): had NO bufferedAmount gate (only the binary path did). A legacy client without the terminalBinaryStream capability streams terminal output as JSON, and that path has no seq/resync — so silently dropping frames there would recreate the corruption bug. Instead we now hold replies in order and drain on recovery. - Client binary send path (remote-runtime-client sendBinary): carries keystrokes and must never drop. Same held-and-drained treatment. Adds a shared, generic createWsOutboundBackpressureQueue: while bufferedAmount is over a soft cap (8 MiB) it parks frames in FIFO order and drains as the socket clears — no drop, no reorder. Only when a hard byte cap (64 MiB) is exceeded (link effectively dead) does it fire onOverflow, which closes the socket so the client reconnects and replays a fresh authoritative snapshot instead of growing memory unbounded. The existing binary drop+resync path (#8059) is untouched. Repro-first: deterministic queue unit tests (injected bufferedAmount + timer) and an e2ee text-path repro that fails on main (replies shoved onto a congested socket) and passes after the fix. Co-authored-by: Orca <help@stably.ai>
… leaked subscription Co-authored-by: Orca <help@stably.ai>
…minal error Co-authored-by: Orca <help@stably.ai>
… resize Co-authored-by: Orca <help@stably.ai>
…us churn Spinner-in-title agents (e.g. the Claude Code braille spinner) flip a PTY title several times per second. Each flip called touchMobileSessionSnapshotsForPty, which bumped every PTY-backed snapshot's version and immediately fanned a fresh session.tabs emit out to every subscriber — where the ws layer JSON.stringifies it per client. That is O(clients × snapshot size) of churn work with no debounce, while files.watch batches at 150ms and native-chat debounces at 40ms. Route the title/status churn path through a small per-worktree trailing-edge coalescer (50ms window, 250ms max-wait cap). Clients gate on snapshotVersion freshness, so collapsing intermediate versions is safe — the emit re-reads the latest snapshot, so only the freshest version ships. Structural changes (tab add/remove/activate) still emit immediately and cancel any pending coalesced notify. Pending notifies flush on subscription close and are dropped on worktree removal so no final state is lost or lands stale.
writeFrame caught a write throw and, for the PRIMARY client, only logged and set closed=true WITHOUT notifying detach — unlike non-primary clients which are detached. The framing carries seq/ack but has no retransmit buffer, so the frame (possibly pty.data or pty.exit) was lost, and because detach never fired the owning Orca kept treating the primary as alive until the ~20s keepalive timeout, silently missing output or a pane death in the meantime. Every other primary-death path (stdin end/error, stdout error) already calls invalidateClient() -> notifyClientDetached. Make the socket-write-throw path consistent: fire notifyClientDetached so the reconnect + PTY-reattach machinery runs promptly. Reattach replays the PTY output buffer and surfaces pane death via reattach-not-found, so pty.exit is never lost. This fail-fast approach reuses the tested reconnect path instead of adding a seq-keyed resend buffer to the hot path. Regression test asserts the primary is detached on write throw (fails pre-fix) and recovers via setWrite. Co-authored-by: Orca <help@stably.ai>
relay.log grows unbounded on long-lived relays: the detached launch redirects > relay.log 2>&1, which truncates only at relaunch, so per-stream stderr lines and reconnect flaps accumulate forever and can fill small remote hosts. Add an in-process RotatingLogWriter (the relay owns its own stderr; a shell redirect can't cap size). installRelayLogRotation wraps process.stdout/stderr so all logging (42 call sites) flows through a writer that appends to relay.log, tracks size, and at 10MB renames relay.log -> relay.log.1 (one archived generation) and reopens a fresh relay.log. Wired only for the detached daemon; deploy passes --log-file on the POSIX and Windows launch paths. Constraints: portable node:fs sync calls (works with pipe/redirect on Linux/macOS/Windows); the current log is always relay.log so the tail-diagnostics workflow keeps working; writes and rotation are guarded so a filesystem error degrades to uncapped logging rather than crashing, and rename (not delete) means no window without a log file. Tests cover rotation at the cap, tail-ability, boot-output preservation, ~2x cap footprint bound, and stderr wrapping/restore. Co-authored-by: Orca <help@stably.ai>
Self-review found a HIGH cross-platform bug: on Windows the launch shell holds
its own `1>relay.log` handle without FILE_SHARE_DELETE, so renameSync of the
live file fails (EPERM/EBUSY). The previous rotate() swallowed that, reopened the
un-renamed file, and re-read its full over-cap size — so the cap never held on
Windows AND every subsequent log line churned a failing close/rename/open.
On rename failure, fall back to truncating relay.log in place ('w') so the size
cap still holds and there is no per-line churn. The shell's fd keeps appending at
its own offset but truncation bounds total growth; trade-off is no archived
generation on that path. New test models the rename-blocked case (archive path is
a directory) and asserts the cap still holds (fails pre-fix: 20100 > 4096).
Also fixed a test nit: spy on process.stderr so the post-restore assertion write
does not leak to the real test-runner stderr.
Co-authored-by: Orca <help@stably.ai>
…agents (aider/vibe) Co-authored-by: Orca <help@stably.ai>
… is dropped Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe changes add streamed Git response handling across SSH and relay layers, including chunking, acknowledgements, cancellation, and reassembly. WebSocket backpressure queues protect encrypted replies and subscription traffic. Mobile session notifications are coalesced, terminal streams recover from dropped frames, stale terminal handles are rejected, relay logs rotate, shared-control replay and timeout state is refined, and agent startup failures surface toast feedback. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/runtime/orca-runtime.ts (1)
5232-5237: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
flushAll()on every unsubscribe fans a settled emit out to all remaining listeners.Flushing before removing the listener does let a subscriber closing mid-window receive the latest state, but
flushAll()also force-emits pending coalesced notifications for every worktree to every other still-subscribed listener, collapsing the coalescing window for them too. If the coalescer supports per-listener or targeted flushing this could be narrowed; otherwise it's an acceptable, minor eager-emit. Not blocking.src/relay/dispatcher.ts (1)
540-552: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider aborting in-flight requests in the write-failure catch path for full consistency.
The comment states the write-throw path is now "consistent with"
invalidateClient()anddetachClient(), but both of those also callthis.requestAborts.abortClient(client.id)to cancel in-flight handlers. Without the abort, long-running handlers for the failing client continue executing unnecessarily —isStale()prevents stale responses but doesn't cancel work or signalAbortControllers.♻️ Add abort call to match invalidateClient/detachClient
} catch (err) { client.closed = true client.generation++ + this.requestAborts.abortClient(client.id) this.flushDrainWaiters(client)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5c2dad34-11fe-4a75-b701-efd14e2f4bbf
📒 Files selected for processing (51)
src/main/providers/ssh-git-provider.test.tssrc/main/providers/ssh-git-provider.tssrc/main/runtime/mobile-session-tabs-notify-coalescer.test.tssrc/main/runtime/mobile-session-tabs-notify-coalescer.tssrc/main/runtime/mobile-subscribe-integration.test.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/e2ee-channel-text-backpressure.test.tssrc/main/runtime/rpc/e2ee-channel.tssrc/main/runtime/rpc/methods/terminal.tssrc/main/runtime/rpc/terminal-geometry-stale-handle.test.tssrc/main/runtime/rpc/terminal-multiplex-escape-tail.test.tssrc/main/runtime/rpc/terminal-multiplex.test.tssrc/main/runtime/rpc/terminal-output-batching.test.tssrc/main/runtime/rpc/terminal-send.test.tssrc/main/runtime/rpc/terminal-subscribe-buffer.test.tssrc/main/ssh/relay-protocol.test.tssrc/main/ssh/relay-protocol.tssrc/main/ssh/ssh-git-response-stream-reader.tssrc/main/ssh/ssh-relay-deploy.tssrc/relay/dispatcher.test.tssrc/relay/dispatcher.tssrc/relay/git-handler.tssrc/relay/git-response-pty-echo-backpressure.integration.test.tssrc/relay/git-response-stream-ownership.test.tssrc/relay/git-response-stream.tssrc/relay/protocol.tssrc/relay/relay.tssrc/relay/rotating-log-writer.test.tssrc/relay/rotating-log-writer.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-batching.test.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-batching.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.tssrc/renderer/src/lib/agent-followup-delivery.test.tssrc/renderer/src/lib/agent-followup-delivery.tssrc/renderer/src/lib/new-workspace.test.tssrc/renderer/src/lib/new-workspace.tssrc/renderer/src/runtime/remote-runtime-terminal-frame-drop-resync.test.tssrc/renderer/src/runtime/remote-runtime-terminal-multiplexer.tssrc/shared/remote-runtime-client.tssrc/shared/remote-runtime-shared-control-connection.test.tssrc/shared/remote-runtime-shared-control-keepalive-refresh.test.tssrc/shared/remote-runtime-shared-control-requests.tssrc/shared/remote-runtime-shared-control-state.tssrc/shared/remote-runtime-shared-control-subscriptions.test.tssrc/shared/remote-runtime-shared-control-subscriptions.tssrc/shared/remote-runtime-shared-control-types.tssrc/shared/ws-outbound-backpressure-queue.test.tssrc/shared/ws-outbound-backpressure-queue.ts
Co-authored-by: Orca <help@stably.ai>
Contributor
Author
|
Addressed the integrated review in 4d49b18:
I kept the non-blocking |
This was referenced Jul 10, 2026
This was referenced Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Combines and supersedes the ten remote-reliability audit PRs into one reviewed integration:
Supersedes #8059, #8060, #8067, #8070, #8073, #8091, #8092, #8094, #8096, and #8099.
Integration review
Reviewed the ten changes together for ordering, lifecycle, ownership, and cross-feature interactions. Follow-up fixes in this branch:
These changes address the substantive CodeRabbit findings across the source PRs. The suggested UTF-8 byte offset change was not applied because the producer's sequence values intentionally use JavaScript string lengths; a multibyte regression test now locks that wire contract down.
Automated validation
Full lint reaches an unrelated current-main localization mismatch: ja.json key
components.native-chat.composer.uploadingAttachmentslacks{{value1}}. This branch has no locale diff.Live Electron + SSH validation
Tested the exact branch in an isolated Electron profile against a throwaway Linux arm64 Docker SSH host and a real git repo. GitHub clone auth was unavailable, so the documented real-repo bundle fallback was used.
fix/remote-reliability-auditand this worktreeORCA_REMOTE_PTY_OK, with the backing marker present over SSHstty sizereturned35 100WRAPPED_PROMPT_OKto a Python-wrapped agent process over the real SSH PTY; the visible terminal renderedWRAPPER_CAPTUREDfalseafter readiness polling and did not write or execute the marker--log-file, produced a healthy bounded log, and had no transport/runtime errorsPerformance and bounded-resource review
Remaining platform scope
The real transport test covered macOS host to Linux arm64 SSH target. Windows relay artifacts build successfully and the Windows rename-lock fallback is unit-tested, but no physical Windows host was used in this session.
Made with Orca 🐋