fix(remote): guard terminal geometry RPCs with the live-leaf resolver

[Jinwoo-H]

Summary

The terminal geometry RPC family mutated PTY state through the unguarded runtime.resolveLeafForHandle, while the keystroke path (terminal.send) was already hardened in #7827 to use the guarded runtime.resolveLiveLeafForHandle. The guarded resolver throws terminal_handle_stale when a pane's PTY was replaced under a handle (restart / re-spawn bumps ptyId / ptyGeneration); the unguarded one silently returns the pane's current PTY.

So a remote client holding a stale handle would resize / refit / viewport the wrong (new) PTY — surfacing as mysterious geometry corruption on the freshly-spawned session. Fixed by routing the four mutating methods through the guarded resolver:

• terminal.resizeForClient
• terminal.setDisplayMode
• terminal.restoreFit
• terminal.updateViewport

The return shape is identical ({ ptyId }), so no follow-on handler logic changes. Deliberately left untouched: the read-only terminal.getDisplayMode, the already-guarded terminal.multiplex subscribe path, and the legacy subscribe-time resolution.

Screenshots

No visual change.

Testing

• pnpm typecheck
• pnpm test — new terminal-geometry-stale-handle.test.ts (6) + terminal-send, terminal-multiplex, fit-override-integration, mobile-presence-lock, mobile-subscribe-integration, runtime-rpc, remote-runtime-pty-transport, terminal-fit-restore, and the runtime terminal_handle_stale resolver test — all green
• oxlint (changed files) + pnpm check:max-lines-ratchet
• Repro-first, deterministic: terminal-geometry-stale-handle.test.ts dispatches each of the four methods with a stale handle (unguarded resolver → replacement pty-b; guarded → throws). Pre-fix (source stashed): all 6 fail — the stale cases return ok:true and the mutator spies (resizeForClient / setMobileDisplayMode / reclaimTerminalForDesktop / updateMobileViewport …) were called against the wrong PTY. Post-fix: all 6 pass — each returns terminal_handle_stale and the wrong PTY is never mutated; two live-handle cases confirm a fresh handle still resizes/reclaims the resolved PTY.

AI Review Report

Reviewed each call site for follow-on assumptions: both resolvers return { ptyId: string | null }, and every handler already guards !leaf?.ptyId → no_connected_pty, so switching resolvers is drop-in. Confirmed the read-only getDisplayMode, multiplex, and legacy subscribe paths are intentionally excluded. Verified the client-side stale-handling story end-to-end (no red banners): terminal.updateViewport .catch(() => {}); terminal.restoreFit .catch(restoreFailedResult) → {restored:false}; mobile terminal.setDisplayMode wrapped in a swallowing try/catch; and the remote-runtime PTY transport already lists terminal_handle_stale in isRemoteTerminalGoneMessage, retiring the mirror and re-deriving the handle from the next session-tabs snapshot (#7718). Change is platform-agnostic control-plane TypeScript — no shortcuts, labels, paths, shell, or Electron surfaces — identical on macOS, Linux, and Windows.

Security Audit

No new input handling, command execution, path handling, auth, secrets, dependency, or IPC surface. The change makes handle resolution stricter (rejects stale/misrouted handles), which reduces the chance of one client mutating another session's PTY. No follow-up needed.

Notes

Core SSH / remote-server territory: both the SSH remote-runtime and remote Orca Server topologies route these control-plane RPCs through the same runtime, so both get the fix. Behavior change: these four methods now return terminal_handle_stale for a stale handle instead of silently succeeding against the wrong PTY — all known callers already treat that as benign (silent retry / re-derive), so no user-visible errors are introduced.

Made with Orca 🐋

[coderabbitai]

📝 Walkthrough

Walkthrough

Four terminal geometry RPC handlers now use live-handle resolution, causing stale PTY handles to return terminal_handle_stale instead of mutating replacement PTYs. A new test suite covers stale-handle rejection for resizing, display mode changes, fit restoration, and viewport updates, plus successful live-handle dispatch. An existing restore-fit test stub now matches the live-handle resolver contract.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately names the key change: guarding terminal geometry RPCs with the live-leaf resolver.
Description check	✅ Passed	All required sections are present, including summary, screenshots, testing, AI review, security audit, and notes, with cross-platform coverage explicitly called out.
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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main/runtime/rpc/terminal-geometry-stale-handle.test.ts (1)

103-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add live-handle tests for setDisplayMode and updateViewport.

The stale tests cover all four geometry RPCs, but the live-handle tests only cover restoreFit and resizeForClient. setDisplayMode and updateViewport have the most complex post-resolution logic (viewport binding, actor marking, updateViewportForClient delegation), so verifying the resolved ptyId flows through those paths would close a meaningful gap.

♻️ Suggested live-handle tests for the remaining two handlers

   it('terminal.resizeForClient resizes the resolved PTY when the handle is live', async () => {
     // ... existing test unchanged ...
   })

+  it('terminal.setDisplayMode mutates the resolved PTY when the handle is live', async () => {
+    const setMobileDisplayMode = vi.fn()
+    const applyMobileDisplayMode = vi.fn().mockResolvedValue(undefined)
+    const updateMobileSubscriberViewport = vi.fn()
+    const markMobileActor = vi.fn()
+    const getLayout = vi.fn().mockReturnValue({ seq: 42 })
+    const runtime = {
+      getRuntimeId: () => 'test-runtime',
+      resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-a' }),
+      setMobileDisplayMode,
+      applyMobileDisplayMode,
+      updateMobileSubscriberViewport,
+      markMobileActor,
+      getLayout
+    } as unknown as OrcaRuntimeService
+    const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS })
+
+    const response = await dispatcher.dispatch(
+      makeRequest('terminal.setDisplayMode', {
+        terminal: 'live-terminal',
+        mode: 'auto',
+        client: { id: 'client-1', type: 'mobile' },
+        viewport: { cols: 80, rows: 24 }
+      })
+    )
+
+    expect(response.ok).toBe(true)
+    if (!response.ok) {
+      throw new Error(response.error.message)
+    }
+    expect(setMobileDisplayMode).toHaveBeenCalledWith('pty-a', 'auto')
+    expect(applyMobileDisplayMode).toHaveBeenCalledWith('pty-a')
+    expect(updateMobileSubscriberViewport).toHaveBeenCalledWith('pty-a', 'client-1', { cols: 80, rows: 24 })
+    expect(markMobileActor).toHaveBeenCalledWith('pty-a', 'client-1')
+    expect(response.result).toEqual({ mode: 'auto', seq: 42 })
+  })
+
+  it('terminal.updateViewport updates the resolved PTY when the handle is live', async () => {
+    const updateMobileViewport = vi.fn().mockResolvedValue({ cols: 80, rows: 24 })
+    const getLayout = vi.fn().mockReturnValue({ seq: 7 })
+    const runtime = {
+      getRuntimeId: () => 'test-runtime',
+      resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-a' }),
+      updateMobileViewport,
+      updateDesktopViewport: vi.fn(),
+      getLayout
+    } as unknown as OrcaRuntimeService
+    const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS })
+
+    const response = await dispatcher.dispatch(
+      makeRequest('terminal.updateViewport', {
+        terminal: 'live-terminal',
+        client: { id: 'client-1', type: 'mobile' },
+        viewport: { cols: 80, rows: 24 }
+      })
+    )
+
+    expect(response.ok).toBe(true)
+    if (!response.ok) {
+      throw new Error(response.error.message)
+    }
+    expect(updateMobileViewport).toHaveBeenCalledWith('pty-a', 'client-1', { cols: 80, rows: 24 })
+    expect(response.result).toEqual({ cols: 80, rows: 24, seq: 7 })
+  })
 })

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c7f9e32c-3c8b-4584-abe3-5a371fcf15a0

📥 Commits

Reviewing files that changed from the base of the PR and between e4c7aab and 3aec590.

📒 Files selected for processing (3)

• src/main/runtime/rpc/methods/terminal.ts
• src/main/runtime/rpc/terminal-geometry-stale-handle.test.ts
• src/main/runtime/rpc/terminal-send.test.ts

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