fix(remote): preserve structured terminal stream failures

[bbingz]

Summary

Remote terminal subscription failures now reach the renderer with their machine code intact instead of collapsing into an English message, and a stream that closes while its handle is still resolving no longer leaves a dead subscription retained in the main process.

• runtimeEnvironments:subscribe returns { ok: true, subscriptionId, requestId } | { ok: false, error: { code, message } } instead of rejecting, so contextBridge can clone the failure
• the preload rethrows plain { code, message } data rather than a custom Error subclass, whose extra properties contextBridge drops
• an onClose that lands before the awaited subscribe handle resolves is tombstoned, so the late handle is closed and released rather than stored

Refs #12667

Why

Upstream #12667 (15ef69a81) converted runtimeEnvironments:call rejections into the structured { ok: false, error: { code, message } } union, and kept the message-fragment fallback on purpose because "untyped main-handler rejections, subscription-start failures, and older code-less paths still rely on it". This PR is the subscribe-side counterpart: it takes subscription-start failures out of that fallback set by giving them the same coded union.

The late-close half is a separate, narrower gap. #12384 guarantees the renderer receives exactly one close when a transport is retired; it does not address the bookkeeping race inside the subscribe handler itself. On origin/main, onClose calls remoteRuntimeSubscriptions.delete(subscriptionId) (src/main/ipc/runtime-environments.ts:178-182). If that fires before subscribeRuntimeEnvironment resolves there is nothing in the map to delete, and the later remoteRuntimeSubscriptions.set(...) at line 207 then stores a handle whose stream has already closed. Nothing else catches it: a plain stream close neither advances the transport generation nor destroys the sender, so the guards at lines 197-206 all pass. The fix latches a subscriptionClosed flag in onClose and checks it before the set.

Screenshots

No visual change.

Testing

• src/main/ipc/runtime-environments.test.ts — 51/51 pass (re-run after rebase onto current main)
• src/preload/runtime-environment-subscriptions.test.ts — 9/9 pass
• Full suite / typecheck / build — not re-run in this pass

AI Review Report

Independent reviews checked contextBridge serialisation, startup-versus-established ordering, error-then-close deduplication, late subscription handles, and recoverable/fatal classification. Review pruning removed an unconsumed failure subscription id while preserving successful subscription identity. Verdict: spec PASS, code APPROVED.

Security Audit

Reviewed contextBridge data shapes, IPC cleanup, structured authentication/protocol errors, and late-handle disposal. Only cloneable code/message failure data crosses the bridge; no secrets, commands, paths, dependencies, or privileged IPC methods are introduced.

ELI5

Remote terminal stream failures were easy to lose across the IPC boundary: the error code was stripped and only English text survived. Startup results and close signals are now structured, cloneable data, so a recoverable transport close does not look like a hard pane death.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds structured runtime subscription results and cleanup handling across IPC and preload layers. Updates terminal multiplexing and shared-control connections to classify recoverable failures, manage reconnects, preserve subscriptions, and fence teardown races. Revises client and server socket liveness to re-probe after delayed ticks. Adds tests covering subscription races, transport recovery, reconnect behavior, callback ordering, and pause-aware heartbeat handling. Includes a design document describing the broader remote terminal recovery architecture.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly describes the primary change: preserving structured failures for remote terminal streams.
Description check	✅ Passed	The description covers the main change, testing status, AI review, security audit, and visual impact; the missing Notes section is non-critical.

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main/ipc/runtime-environments.ts (1)

46-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting the duplicated RuntimeEnvironmentSubscriptionStartResult type to a shared module.

This type is defined identically in src/preload/runtime-environment-subscriptions.ts (lines 28-33). Since the preload already imports from ../shared/, extracting it to a shared module would keep the IPC contract in one place and prevent drift.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 2aed13c2-859a-455f-ba32-3fecd2c63cea

📥 Commits

Reviewing files that changed from the base of the PR and between 20ebe85 and 49e2f39.

📒 Files selected for processing (25)

• docs/reference/2026-07-11-remote-runtime-terminal-recovery-design.md
• src/main/ipc/runtime-environments.test.ts
• src/main/ipc/runtime-environments.ts
• src/main/runtime/rpc/ws-transport.test.ts
• src/main/runtime/rpc/ws-transport.ts
• src/preload/runtime-environment-subscriptions.test.ts
• src/preload/runtime-environment-subscriptions.ts
• src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts
• src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts
• src/shared/remote-runtime-client-error-classification.test.ts
• src/shared/remote-runtime-client-error-classification.ts
• src/shared/remote-runtime-client.test.ts
• src/shared/remote-runtime-shared-control-connection.test.ts
• src/shared/remote-runtime-shared-control-connection.ts
• src/shared/remote-runtime-shared-control-frame-handler.ts
• src/shared/remote-runtime-shared-control-keepalive-refresh.test.ts
• src/shared/remote-runtime-shared-control-open.ts
• src/shared/remote-runtime-shared-control-reconnect-delay.test.ts
• src/shared/remote-runtime-shared-control-reconnect.ts
• src/shared/remote-runtime-shared-control-requests.ts
• src/shared/remote-runtime-shared-control-state.ts
• src/shared/remote-runtime-shared-control-subscription-start.ts
• src/shared/remote-runtime-shared-control-types.ts
• src/shared/remote-runtime-socket-liveness.test.ts
• src/shared/remote-runtime-socket-liveness.ts

[bbingz]

CodeRabbit disposition: shared subscription-start result type

No change in this stack. The result shape currently has exactly two private consumers: the main IPC handler that produces it and the preload decoder that consumes it. Moving the alias into src/shared would add another cross-process type surface, but it would not add runtime validation at the dynamic IPC boundary. The main and preload tests already pin both the success and serialized-failure shapes.

This is worth extracting if the contract gains another consumer or starts evolving independently; for the current two-site, stable union, keeping the recovery PR focused has the lower maintenance cost.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: bfb8eae0-6690-4485-930e-3cac13e9f0a4

📥 Commits

Reviewing files that changed from the base of the PR and between 49e2f39 and 9a65961.

📒 Files selected for processing (25)

• docs/reference/2026-07-11-remote-runtime-terminal-recovery-design.md
• src/main/ipc/runtime-environments.test.ts
• src/main/ipc/runtime-environments.ts
• src/main/runtime/rpc/ws-transport.test.ts
• src/main/runtime/rpc/ws-transport.ts
• src/preload/runtime-environment-subscriptions.test.ts
• src/preload/runtime-environment-subscriptions.ts
• src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts
• src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts
• src/shared/remote-runtime-client-error-classification.test.ts
• src/shared/remote-runtime-client-error-classification.ts
• src/shared/remote-runtime-client.test.ts
• src/shared/remote-runtime-shared-control-connection.test.ts
• src/shared/remote-runtime-shared-control-connection.ts
• src/shared/remote-runtime-shared-control-frame-handler.ts
• src/shared/remote-runtime-shared-control-keepalive-refresh.test.ts
• src/shared/remote-runtime-shared-control-open.ts
• src/shared/remote-runtime-shared-control-reconnect-delay.test.ts
• src/shared/remote-runtime-shared-control-reconnect.ts
• src/shared/remote-runtime-shared-control-requests.ts
• src/shared/remote-runtime-shared-control-state.ts
• src/shared/remote-runtime-shared-control-subscription-start.ts
• src/shared/remote-runtime-shared-control-types.ts
• src/shared/remote-runtime-socket-liveness.test.ts
• src/shared/remote-runtime-socket-liveness.ts

🚧 Files skipped from review as they are similar to previous changes (24)

• src/shared/remote-runtime-client-error-classification.test.ts
• src/shared/remote-runtime-shared-control-reconnect-delay.test.ts
• src/shared/remote-runtime-shared-control-open.ts
• src/shared/remote-runtime-client-error-classification.ts
• src/shared/remote-runtime-shared-control-frame-handler.ts
• src/shared/remote-runtime-socket-liveness.test.ts
• src/shared/remote-runtime-shared-control-types.ts
• src/shared/remote-runtime-shared-control-subscription-start.ts
• src/main/runtime/rpc/ws-transport.ts
• src/shared/remote-runtime-client.test.ts
• src/shared/remote-runtime-shared-control-state.ts
• src/main/runtime/rpc/ws-transport.test.ts
• src/main/ipc/runtime-environments.ts
• src/preload/runtime-environment-subscriptions.ts
• src/shared/remote-runtime-shared-control-keepalive-refresh.test.ts
• src/main/ipc/runtime-environments.test.ts
• src/shared/remote-runtime-shared-control-requests.ts
• src/shared/remote-runtime-socket-liveness.ts
• src/preload/runtime-environment-subscriptions.test.ts
• src/shared/remote-runtime-shared-control-reconnect.ts
• src/shared/remote-runtime-shared-control-connection.test.ts
• src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts
• src/shared/remote-runtime-shared-control-connection.ts
• src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts

[bbingz]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fa62913d-db44-400a-9b22-04699ec18642

📥 Commits

Reviewing files that changed from the base of the PR and between f68be37 and 06d82bc.

📒 Files selected for processing (1)

• docs/reference/2026-07-11-remote-runtime-terminal-recovery-design.md

[bbingz]

Rebased onto rebased #8253 + latest main (clean cherry-pick of the two unique commits).

[bbingz]

Stack status vs merged #9774

#9774 covers the recoverable transport-close → pane recovery path, but does not fully replace this PR’s IPC/bridge pieces:

1. Serializable runtimeEnvironments:subscribe start result (ok union instead of throw)
2. Preload cloneable {code,message} across contextBridge
3. Late-close tombstone before handle resolves

Plan: keep this PR open but narrow to those follow-ups on current main (drop transport-foundation rewrites that #9774 already owns). Full stack rebase of the pre-#9774 branch is not the path forward.

Will follow with a slimmed branch or force-push once the narrowed patch is ready.

[bbingz]

Narrow rewrite complete (post-#9774)

Force-pushed a single-scope branch onto current main (head c26405cd4c). Dropped the transport-foundation stack that #9774 already owns.

Kept only:

1. runtimeEnvironments:subscribe returns { ok: true, … } | { ok: false, error: { code, message } } (no throw across IPC)
2. Preload reconstructs plain cloneable { code, message } (contextBridge-safe)
3. Early onClose tombstone so a late handle is release-only (never retained)

Focused tests: runtime-environments.test.ts + runtime-environment-subscriptions.test.ts (41 passed).

Ready for re-review as a small follow-up to #9774.

[AmethystLiang]

Associating this PR with related issue(s): #8180

[AmethystLiang]

Associating this PR with related issue(s): #8180

[bbingz]

Rebased onto current origin/main (0326594d52).

Merged main's pairing-revision/transport-generation guards with this PR's serializable ok-union subscribe results + late-close tombstone (no throw across contextBridge; no double-close on re-pair).

Focused tests: runtime-environments + preload subscriptions 46/46 green. ahead=2 behind=0.

[bbingz]

Restack + review guide (community, awaiting maintainer)

Rebased onto latest main (force-with-lease). 2 commits, behind=0. Narrow residual only.

What this PR still uniquely does

Recoverable transport-close → pane recovery largely lives on main (see #9774-era recovery). This PR is not that stack.

Residual:

1. Serializable subscribe start result as ok-union data (not throw-only)
2. contextBridge-safe error serialization for dedicated subscribe failures
3. Late-close tombstones so async handles are release-only after early close

Files (small)

• src/main/ipc/runtime-environments.ts (+ tests)
• src/preload/runtime-environment-subscriptions.ts (+ tests)

Depends on

• Independent of fix(orchestration): deliver replyable messages to background PTYs #8057. Complements fix(remote): accelerate terminal recovery on resume/online #8255 (resume kick) but does not require it.

Tests (local)

• runtime-environments + preload subscription suites — 51 passed

Ready for upstream review; community author cannot merge.

[greptile-apps]

Greptile Summary

This PR replaces thrown IPC errors and bare success shapes in the terminal stream subscribe handler with a shared ok-union type (RuntimeEnvironmentSubscriptionStartResult), making all outcomes safely cloneable across Electron's contextBridge. It also introduces a tombstone flag so an onClose that beats the async handle continuation is forwarded to the renderer before the handle promise settles, avoiding a silent dead-handle situation.

• Ok-union serialization: runtimeEnvironments:subscribe now returns { ok: true, subscriptionId, requestId } or { ok: false, error: { code, message } } in all paths; the main process never throws across IPC, and the preload re-throws plain objects rather than Error instances to survive contextBridge cloning.
• Tombstone (early-close) path: a subscriptionClosed flag set inside onClose lets the post-await continuation detect that the remote already closed, forward the close event before the handle resolves, then call subscription.close() once (idempotent because ws is nulled out by cleanupSocketListeners).
• Test coverage: new tests validate startup failures, duplicate ids, pairing-change detection, tombstone ordering on both main and preload sides, and idempotent post-remote-close behaviour at the transport layer.

Confidence Score: 5/5

Safe to merge; all changed code paths are covered by dedicated regression tests and the transport-layer close is already idempotent.

The subscribe handler now catches every error path and returns structured data instead of throwing, removing the contextBridge serialization hazard. The tombstone flag correctly gates the post-await continuation — no genuine race in Node.js single-threaded event loop. Idempotent close and tombstone ordering are both covered by new dedicated tests. No previously passing assertions were silently weakened.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/shared/runtime-environment-subscription-start-result.ts	New shared ok-union type for the subscribe IPC return value; simple and correct discriminated union.
src/main/ipc/runtime-environments.ts	Subscribe handler restructured to catch all errors and return ok-union data; tombstone flag added to track early remote close before the async handle resolves.
src/preload/runtime-environment-subscriptions.ts	Consumer updated to parse ok-union result, release dispatcher on failure paths, and throw plain cloneable error objects; tombstone case correctly handled.
src/shared/remote-runtime-client.test.ts	Adds test verifying that subscription.close() is a no-op after the remote side closes, plus closeClient() helper on createSubscriptionServer.
src/main/ipc/runtime-environments.test.ts	Comprehensive new tests for structured startup failures, duplicate subscription ids, tombstone, and pairing-changed ok:false; existing tests updated to the new ok-union return shape.
src/preload/runtime-environment-subscriptions.test.ts	New tests for preload tombstone ordering, structured failure rejection with plain objects, and unexpected invoke rejection normalization.

_{Reviews (2): Last reviewed commit: "test(remote): cover early subscription c..." | Re-trigger Greptile}