fix(relay): detach primary client on write failure to trigger reconnect

[Jinwoo-H]

Summary

The relay dispatcher silently drops frames on a primary-client write failure and never triggers reconnect, so the owning Orca can miss pty.data output or a pty.exit pane-death.

writeFrame (src/relay/dispatcher.ts) catches a write throw. For non-primary clients it deletes and detaches them; for the primary client it only logged to stderr and set closed = true without notifying detach. The framing carries seq/ack but there is no retransmit buffer, so the frame is gone. Because detach never fired, none of the primary-death consumers ran and the owning Orca kept treating the primary as alive until the ~20s keepalive timeout — during that window every frame written to the throwing sink was silently lost.

Every other primary-death path (process.stdin end/error, process.stdout error in relay.ts) already calls dispatcher.invalidateClient(), which fires notifyClientDetached. The socket-write-throw path in writeFrame was the one inconsistency.

Fix & design rationale

Fail-fast (reuse existing reconnect + reattach), not a new resend buffer. On a primary write throw, writeFrame now fires notifyClientDetached (the primary client object stays in the map because it is reused across setWrite reconnects, but listeners must still fire — exactly as invalidateClient() does for stdin/stdout death). This makes the socket-write-throw path consistent with every other primary-death path and engages the reconnect machinery promptly instead of after the keepalive timeout.

Why fail-fast over a seq-keyed resend buffer:

• The existing reconnect path already regenerates the two things that matter. reattachKnownPtys (ssh-relay-session.ts) replays each PTY's buffered output (the relay keeps the last ~100 KB in managed.buffered), and a PTY that exited during the failure window is torn out of the relay's map, so reattach throws not-found and the session emits pty:exit to the renderer. Pane death is therefore never lost.
• A general seq-keyed retransmit buffer would touch the hot outbound path for every frame and duplicate reconnection logic the client already owns — much larger surface, higher risk, for a transient-write-blip edge.

Known tradeoff (documented, acceptable): the reattach-not-found path surfaces a pane exit as code: -1 rather than the process's real exit code. The renderer treats any exit as pane death; the exact code is informational. The guarantee the task required — pty.exit is never lost — holds. Preserving the exact code would require the resend buffer, which is out of proportion to the benefit; noted as possible follow-up.

Repro / evidence

src/relay/dispatcher.test.ts → detaches the primary client when its write throws:

• Primary sink throws on write; dispatcher notify('pty.exit', …).
• Before: onClientDetached listener is never called (verified: the test fails on pre-fix dispatcher.ts — expected "vi.fn()" to be called with arguments: [ 1 ]).
• After: the listener fires with the primary client id, and a setWrite reconnect makes the client usable again (subsequent pty.data reaches the new sink).

Screenshots

No visual change.

Testing

• pnpm typecheck
• oxlint on changed files clean
• src/relay/dispatcher.test.ts (23), pty-handler.test.ts (63), integration.test.ts (27) pass in isolation. New regression test fails on pre-fix, passes on post-fix.

AI Review Report

Reviewed the re-entrancy of firing notifyClientDetached mid-notify() loop: the primary is not deleted from the client map, so iteration stays valid; non-primary deletion behavior is unchanged. Confirmed the fs-handler detach listener (releaseClientWatches + wakeAllAckWaiters) already runs on primary death via invalidateClient(), so this path introduces no new listener behavior — only makes the write-throw case consistent. Cross-platform: pure JS control-flow, no path/shell/OS assumptions; the relay runs on Linux/macOS/Windows remote hosts. Git operations are unaffected.

Security Audit

No input handling, command execution, path, auth, secrets, or IPC surface added or changed. The change only alters when an existing detach notification fires on a failed write; it cannot be driven by remote input beyond causing the write to fail, which already triggers the same recovery for non-primary clients.

Notes

Fail-fast reuses the reconnect + PTY-reattach machinery in ssh-relay-session.ts. If exact exit-code fidelity across a primary write blip is ever required, that would need a bounded seq-keyed retransmit buffer (deliberately out of scope here).

Made with Orca 🐋

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3a1d53e2-bc91-414e-ad8e-3ff4ec74dc54

📥 Commits

Reviewing files that changed from the base of the PR and between 2622419 and 763cc3e.

📒 Files selected for processing (2)

• src/relay/dispatcher.test.ts
• src/relay/dispatcher.ts

---

📝 Walkthrough

Walkthrough

Updated RelayDispatcher.writeFrame to notify detach listeners whenever a client write throws, including for the primary client, while retaining the primary client in the client map. Added a regression test covering the detach notification and recovery through setWrite, followed by successful delivery of later notification frames.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: detaching the primary client on write failure to trigger reconnect.
Description check	✅ Passed	The description matches the required template and includes summary, screenshots, testing, AI review, security audit, and notes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Jinwoo-H]

Superseded by #8141, which combines all ten remote-reliability audit fixes, incorporates the valid CodeRabbit follow-ups, and has full CI plus live Electron/SSH validation. Closing this standalone PR so #8141 is the single review target; this fix remains included there.