From 34f132afae8e2ea2ba05c6fc7854c2f604d8b719 Mon Sep 17 00:00:00 2001 From: shady Date: Wed, 8 Jul 2026 12:51:36 +0300 Subject: [PATCH] fix(terminal): let remote desktop viewers own the shared PTY width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A remote (relay/shared-control) desktop viewer resizes the host source PTY to its own width, but was never registered as a width owner. So the host's own fit cascade (window resize, split drag, tab reveal, "+"-new-tab re-render) freely resized the viewed PTY back to the host-local width with no signal to the viewer. The viewer kept its narrower grid while the host streamed wider alt-screen frames -> cell-layout garble ("porridge") until a manual resize. The mobile "presence lock" already solves this shape for phones by suppressing the host's pty:resize while the phone drives. This does the same for remote desktop viewers WITHOUT joining the mobile driver state machine (a viewer needs only resize suppression, not input lock / phone-fit / driver banners), and it routes every PTY geometry change through the existing enqueueLayout/applyLayout serialization path rather than resizing the PTY ad hoc. Design: - Registry `remoteDesktopViewers: Map>`, keyed per SUBSCRIPTION so duplicate streams of one client cannot release each other. isPtyResizeDrivenRemotely() = mobile driver OR any viewer present; pty:resize (host fit cascade) bails on it. INPUT is never locked (shared control: host and viewers can both type), unlike mobile. - New internal layout target { kind: 'remote-desktop' } in applyLayout: clears terminalFitOverrides like 'desktop', resizes only when dims changed, emits no fit-override notifications, and does not call onExternalPtyResize (so a viewer's width never pollutes desktop restore state). enqueueLayout remains the sole serialized writer of PTY dimensions. - Smallest client wins: applyRemoteDesktopLayout() sizes the PTY to the SMALLEST attached viewer, so viewers with different screen sizes never overflow the narrowest grid. - No snapshot/replay race: a viewport that arrives while the initial scrollback snapshot is being serialized is buffered and applied as ONE serialized resize before serialization, so a mid-repaint alt screen is never baked into the snapshot. - Reclaim on detach: when the last viewer leaves, applyRemoteDesktopLayout resizes the PTY back to the host's own width via enqueueLayout({kind:'desktop'}), so the host reflows to full width without a manual window jiggle. The host width is captured at the pty:resize suppression point (the host's own resize attempts) — a source a remote viewport never pollutes. - Mobile coexistence: a phone outranks a viewer; when the phone leaves, the mobile-release paths call applyRemoteDesktopLayout so a surviving viewer keeps the PTY, else the host reclaims. Verified live (two local desktop instances): correct render on connect, no garble on viewer-window resize (synchronous), and host reclaims full width on disconnect. Unit tests cover the registry lifecycle, smallest-client-wins, reclaim-via-layout, and mobile coexistence. Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Codex --- src/main/ipc/pty.test.ts | 48 ++++ src/main/ipc/pty.ts | 20 +- src/main/runtime/orca-runtime.ts | 215 +++++++++++++++--- .../runtime/remote-desktop-driver.test.ts | 209 +++++++++++++++++ src/main/runtime/rpc/methods/terminal.ts | 135 ++++++++++- .../runtime/rpc/terminal-multiplex.test.ts | 28 ++- 6 files changed, 600 insertions(+), 55 deletions(-) create mode 100644 src/main/runtime/remote-desktop-driver.test.ts diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 2b405685c45..b1cdef23da3 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -4170,10 +4170,12 @@ describe('registerPtyHandlers', () => { createPreAllocatedTerminalHandle: vi.fn(() => null), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), isResizeSuppressed: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn(), + recordRemoteDesktopHostReclaimTarget: vi.fn(), onExternalPtyResize: vi.fn() } handlers.clear() @@ -4202,10 +4204,12 @@ describe('registerPtyHandlers', () => { createPreAllocatedTerminalHandle: vi.fn(() => null), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), isResizeSuppressed: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn(), + recordRemoteDesktopHostReclaimTarget: vi.fn(), onExternalPtyResize: vi.fn() } handlers.clear() @@ -4217,6 +4221,39 @@ describe('registerPtyHandlers', () => { expect(runtime.onExternalPtyResize).not.toHaveBeenCalled() }) + + it('suppresses the host fit cascade while a remote viewer drives the width', async () => { + const resizeSpy = vi.fn() + setupProviderWithAppliedSize({ applied: { cols: 80, rows: 24 }, resize: resizeSpy }) + const runtime = { + setPtyController: vi.fn(), + createPreAllocatedTerminalHandle: vi.fn(() => null), + registerPty: vi.fn(), + getDriver: vi.fn(() => ({ kind: 'idle' })), + // The whole point of the fix: a PTY with a remote viewer reports true, + // even though the presence-lock driver state stays idle/desktop. + isPtyResizeDrivenRemotely: vi.fn(() => true), + isResizeSuppressed: vi.fn(() => false), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn(), + recordRemoteDesktopHostReclaimTarget: vi.fn(), + onExternalPtyResize: vi.fn() + } + handlers.clear() + registerPtyHandlers(mainWindow as never, runtime as never) + const spawn = await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, env: {} }) + const id = (spawn as { id: string }).id + resizeSpy.mockClear() + + // Host's own safeFit tries to widen the viewed PTY back to its window. + resizeListener()(mainWindowIpcEvent, { id, cols: 125, rows: 48 }) + + // It must not reach the PTY while the viewer owns the width. + expect(resizeSpy).not.toHaveBeenCalled() + expect(runtime.recordRemoteDesktopHostReclaimTarget).toHaveBeenCalledWith(id, 125, 48) + expect(runtime.onExternalPtyResize).not.toHaveBeenCalled() + }) }) it('injects ORCA_TERMINAL_HANDLE for non-local PTY providers', async () => { @@ -4289,6 +4326,7 @@ describe('registerPtyHandlers', () => { registerPreAllocatedHandleForPty: vi.fn(), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -4368,6 +4406,7 @@ describe('registerPtyHandlers', () => { registerPreAllocatedHandleForPty: vi.fn(), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -4399,6 +4438,7 @@ describe('registerPtyHandlers', () => { registerPreAllocatedHandleForPty: vi.fn(), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -4436,6 +4476,7 @@ describe('registerPtyHandlers', () => { registerPreAllocatedHandleForPty: vi.fn(), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -4533,6 +4574,7 @@ describe('registerPtyHandlers', () => { registerPty: vi.fn(), noteTerminalSpawnCommand: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -4585,6 +4627,7 @@ describe('registerPtyHandlers', () => { registerPty: vi.fn(), noteTerminalSpawnCommand: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -5073,6 +5116,7 @@ describe('registerPtyHandlers', () => { registerPty: vi.fn(), noteTerminalSpawnCommand: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -5330,6 +5374,7 @@ describe('registerPtyHandlers', () => { registerPty: vi.fn(), noteTerminalSpawnCommand: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -5439,6 +5484,7 @@ describe('registerPtyHandlers', () => { registerPty: vi.fn(), noteTerminalSpawnCommand: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -5533,6 +5579,7 @@ describe('registerPtyHandlers', () => { registerPty: vi.fn(), noteTerminalSpawnCommand: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() @@ -5637,6 +5684,7 @@ describe('registerPtyHandlers', () => { registerPreAllocatedHandleForPty: vi.fn(), registerPty: vi.fn(), getDriver: vi.fn(() => ({ kind: 'host' })), + isPtyResizeDrivenRemotely: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 893a65e4c65..154e62cfd96 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4538,14 +4538,18 @@ export function registerPtyHandlers( if (runtime?.isResizeSuppressed()) { return } - // Why: presence-lock defense-in-depth. While mobile is driving, - // desktop-side resizes (auto-fit on window resize, split drag) must - // not reach the PTY. The renderer guard checks the driver state too, - // but this is the load-bearing layer because the renderer mirror lags - // by one IPC hop. Note: BOTH guards apply — isResizeSuppressed handles - // the safeFit cascade after take-back; this driver check handles the - // ongoing locked state. See docs/mobile-presence-lock.md. - if (runtime?.getDriver(args.id).kind === 'mobile') { + // Why: presence-lock defense-in-depth. While a phone OR a remote desktop + // viewer drives the PTY width, the host's own desktop-side resizes + // (auto-fit on window resize, split drag, tab reveal, "+"-new-tab + // re-render) must not reach the PTY — otherwise they overwrite the remote + // viewer's grid and its alt-screen TUI garbles ("porridge"). The renderer + // guard checks the driver state too, but this is the load-bearing layer + // because the renderer mirror lags by one IPC hop. Note: BOTH guards apply + // — isResizeSuppressed handles the safeFit cascade after take-back; this + // driver check handles the ongoing locked state. See + // docs/mobile-presence-lock.md. + if (runtime?.isPtyResizeDrivenRemotely(args.id)) { + runtime.recordRemoteDesktopHostReclaimTarget(args.id, args.cols, args.rows) return } const provider = tryGetProviderForPty(args.id) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index e183aff463e..9aae907563b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2062,6 +2062,7 @@ export type DriverState = RuntimeTerminalDriverState export type PtyLayoutTarget = | { kind: 'desktop'; cols: number; rows: number } | { kind: 'phone'; cols: number; rows: number; ownerClientId: string } + | { kind: 'remote-desktop'; cols: number; rows: number } // Why: authoritative layout state with monotonic seq. Bumped on every // applyLayout success; emitted on mobile subscribe-stream events so clients @@ -2329,6 +2330,15 @@ export class OrcaRuntimeService { private currentDriver = new Map() private currentBrowserDriver = new Map() + // Why: remote (relay/shared-control) desktop viewers of a PTY are keyed by + // subscription, not client, because one client can open duplicate streams and + // each stream must release only the width floor it registered. + private remoteDesktopViewers = new Map< + string, + Map + >() + private remoteDesktopHostReclaimTargets = new Map() + // Why: resubscribe-grace window. When the last mobile subscriber for a // PTY unsubscribes, we hold the driver=mobile{clientId} state and the // inner-map record open for ~250ms. If the same (ptyId, clientId) @@ -7851,7 +7861,11 @@ export class OrcaRuntimeService { // at phone dims after the phone disconnects; the desktop banner's // Restore button is the explicit return path. See // docs/mobile-fit-hold.md. - if (cur?.kind === 'phone' && this.getAutoRestoreFitMs() != null) { + if (this.hasRemoteDesktopViewers(ptyId)) { + this.setDriver(ptyId, { kind: 'idle' }) + void this.applyRemoteDesktopLayout(ptyId) + continue + } else if (cur?.kind === 'phone' && this.getAutoRestoreFitMs() != null) { // Use the soft-leaver's snapshot baseline as a hint, falling // through to resolveDesktopRestoreTarget for missing values. const fallback = this.resolveDesktopRestoreTarget(ptyId) @@ -7890,7 +7904,11 @@ export class OrcaRuntimeService { for (const { ptyId, baseline } of ptysToRestore) { const cur = this.layouts.get(ptyId) // Why: Indefinite hold gate — see soft-leaver branch above. - if (cur?.kind === 'phone' && this.getAutoRestoreFitMs() != null) { + if (this.hasRemoteDesktopViewers(ptyId)) { + this.setDriver(ptyId, { kind: 'idle' }) + void this.applyRemoteDesktopLayout(ptyId) + continue + } else if (cur?.kind === 'phone' && this.getAutoRestoreFitMs() != null) { const fallback = this.resolveDesktopRestoreTarget(ptyId) const cols = baseline?.cols ?? fallback.cols const rows = baseline?.rows ?? fallback.rows @@ -8019,6 +8037,8 @@ export class OrcaRuntimeService { this.currentDriver.delete(ptyId) this.notifier?.terminalDriverChanged(ptyId, { kind: 'idle' }) } + this.remoteDesktopViewers.delete(ptyId) + this.remoteDesktopHostReclaimTargets.delete(ptyId) this.disposeHeadlessTerminal(ptyId) this.agentDetector?.onExit(ptyId) const pty = this.ptysById.get(ptyId) @@ -8074,6 +8094,157 @@ export class OrcaRuntimeService { } } + // Why: the host's own fit cascade (window resize, split drag, tab reveal, + // "+"-new-tab re-render) must not resize a PTY whose width a remote client + // owns — that is the remote "porridge" bug. True while a phone (mobile driver) + // OR a remote desktop viewer holds the PTY. Input is deliberately NOT gated + // here (see the `writePtyInput` mobile-only checks): shared-control desktop + // viewers may still type alongside the host. + // Note: this is intentionally NOT a driver kind. A remote desktop viewer needs + // only resize suppression, not the mobile driver machinery (input lock, + // phone-fit, driver-change banners), so it lives in its own registry and does + // not perturb the presence-lock state machine. It also coexists with mobile: + // while a phone drives, the registry still suppresses host resize, and when + // the phone leaves the surviving viewer keeps the PTY suppressed. + isPtyResizeDrivenRemotely(ptyId: string): boolean { + if (this.getDriver(ptyId).kind === 'mobile') { + return true + } + return this.hasRemoteDesktopViewers(ptyId) + } + + private hasRemoteDesktopViewers(ptyId: string): boolean { + const viewers = this.remoteDesktopViewers.get(ptyId) + return viewers !== undefined && viewers.size > 0 + } + + // Why: the smallest attached viewer wins (tmux model). A wider viewer showing + // a few unused columns is fine; a narrower viewer receiving host-width frames + // garbles, so the PTY must never exceed the narrowest grid. + private minRemoteDesktopViewport( + viewers: Map + ): { cols: number; rows: number } | null { + let cols = Infinity + let rows = Infinity + for (const viewport of viewers.values()) { + cols = Math.min(cols, viewport.cols) + rows = Math.min(rows, viewport.rows) + } + return Number.isFinite(cols) && Number.isFinite(rows) ? { cols, rows } : null + } + + private resolveRemoteDesktopHostReclaimTarget(ptyId: string): { cols: number; rows: number } { + const target = this.remoteDesktopHostReclaimTargets.get(ptyId) + if (target) { + return target + } + const renderer = this.lastRendererSizes.get(ptyId) + if (renderer) { + return { cols: renderer.cols, rows: renderer.rows } + } + const size = this.getTerminalSize(ptyId) + if (size) { + return { cols: size.cols, rows: size.rows } + } + return { cols: 80, rows: 24 } + } + + private ensureRemoteDesktopHostReclaimTarget(ptyId: string): void { + if (!this.remoteDesktopHostReclaimTargets.has(ptyId)) { + this.remoteDesktopHostReclaimTargets.set( + ptyId, + this.resolveRemoteDesktopHostReclaimTarget(ptyId) + ) + } + } + + recordRemoteDesktopHostReclaimTarget(ptyId: string, cols: number, rows: number): void { + if (cols <= 0 || rows <= 0) { + return + } + this.remoteDesktopHostReclaimTargets.set(ptyId, { cols, rows }) + } + + async applyRemoteDesktopLayout(ptyId: string): Promise { + if (this.getDriver(ptyId).kind === 'mobile') { + return true + } + const viewers = this.remoteDesktopViewers.get(ptyId) + const target = viewers ? this.minRemoteDesktopViewport(viewers) : null + const reclaimingHost = !target + const layoutTarget: PtyLayoutTarget = target + ? { kind: 'remote-desktop', cols: target.cols, rows: target.rows } + : { kind: 'desktop', ...this.resolveRemoteDesktopHostReclaimTarget(ptyId) } + this.freshSubscribeGuard.add(ptyId) + try { + const result = await this.enqueueLayout(ptyId, layoutTarget) + if (reclaimingHost) { + this.remoteDesktopHostReclaimTargets.delete(ptyId) + } + return result.ok + } finally { + this.freshSubscribeGuard.delete(ptyId) + } + } + + // Why: a remote (relay/shared-control) desktop viewer reporting a viewport + // takes/refreshes the width floor so the host's fit cascade stops fighting it. + // The source PTY is sized to the SMALLEST attached viewer so no viewer ever + // receives frames wider than its grid. + async updateRemoteDesktopViewer( + ptyId: string, + subscriptionKey: string, + clientId: string, + cols: number, + rows: number + ): Promise { + const viewport = clampTerminalViewport(cols, rows) + this.ensureRemoteDesktopHostReclaimTarget(ptyId) + let viewers = this.remoteDesktopViewers.get(ptyId) + if (!viewers) { + viewers = new Map() + this.remoteDesktopViewers.set(ptyId, viewers) + } + viewers.set(subscriptionKey, { clientId, cols: viewport.cols, rows: viewport.rows }) + return this.applyRemoteDesktopLayout(ptyId) + } + + unregisterRemoteDesktopViewer(ptyId: string, subscriptionKey: string): Promise { + const viewers = this.remoteDesktopViewers.get(ptyId) + if (!viewers) { + return Promise.resolve(false) + } + viewers.delete(subscriptionKey) + if (viewers.size === 0) { + this.remoteDesktopViewers.delete(ptyId) + } + return this.applyRemoteDesktopLayout(ptyId) + } + + async updateDesktopViewport( + ptyId: string, + viewport: { cols: number; rows: number } + ): Promise { + const { cols, rows } = clampTerminalViewport(viewport.cols, viewport.rows) + if (this.terminalFitOverrides.has(ptyId) || this.getDriver(ptyId).kind === 'mobile') { + this.recordRendererGeometry(ptyId, cols, rows) + return true + } + if (this.isResizeSuppressed()) { + return false + } + this.freshSubscribeGuard.add(ptyId) + try { + const result = await this.enqueueLayout(ptyId, { kind: 'desktop', cols, rows }) + if (result.ok) { + this.refreshRendererGeometry(ptyId, cols, rows) + } + return result.ok + } finally { + this.freshSubscribeGuard.delete(ptyId) + } + } + markMobileActor(ptyId: string, clientId: string): void { const inner = this.mobileSubscribers.get(ptyId) const sub = inner?.get(clientId) @@ -8174,37 +8345,6 @@ export class OrcaRuntimeService { return { updated: true, applied: result.ok } } - // Why: remote desktop clients do not have the local `pty:resize` IPC path. - // Their measured xterm size still has to resize the source PTY so TUIs - // reflow to the visible client dimensions. - async updateDesktopViewport( - ptyId: string, - viewport: { cols: number; rows: number } - ): Promise { - const { cols, rows } = clampTerminalViewport(viewport.cols, viewport.rows) - if (this.terminalFitOverrides.has(ptyId)) { - // Why: remote desktop panes do not have the local pty:reportGeometry - // IPC. While phone-fit holds the PTY, treat their viewport RPC as a - // measurement-only restore target, not a resize intent. - this.recordRendererGeometry(ptyId, cols, rows) - return true - } - if (this.isResizeSuppressed() || this.getDriver(ptyId).kind === 'mobile') { - return false - } - let resized = false - try { - resized = this.ptyController?.resize?.(ptyId, cols, rows) ?? false - } catch { - return false - } - if (!resized) { - return false - } - this.onExternalPtyResize(ptyId, cols, rows) - return true - } - // Why: invoked from `runtime:restoreTerminalFit` IPC (the desktop "Take // back" / "Restore" button). Forces the PTY back to desktop dims and flips // the driver to `desktop`, suppressing further mobile-driven dim changes @@ -8602,7 +8742,7 @@ export class OrcaRuntimeService { // only if that invariant is ever violated, repairing the renderer instead // of stranding the held modal. const overrideChanged = (prevFitOverride != null) !== (target.kind === 'phone') - if (modeChanged || overrideChanged) { + if (target.kind !== 'remote-desktop' && (modeChanged || overrideChanged)) { // Why: phone→desktop arms the renderer-cascade suppress window // before the collateral safeFit IPCs arrive. See "Renderer cascade // suppression". @@ -8938,6 +9078,9 @@ export class OrcaRuntimeService { this.pendingSoftLeavers.delete(ptyId) if (!this.mobileSubscribers.has(ptyId)) { this.setDriver(ptyId, { kind: 'idle' }) + if (this.hasRemoteDesktopViewers(ptyId)) { + void this.applyRemoteDesktopLayout(ptyId) + } } }, SOFT_LEAVE_GRACE_MS) if (typeof softTimer.unref === 'function') { @@ -8990,6 +9133,10 @@ export class OrcaRuntimeService { if (this.isMobileSubscriberActive(ptyId)) { return } + if (this.hasRemoteDesktopViewers(ptyId)) { + void this.applyRemoteDesktopLayout(ptyId) + return + } void this.enqueueLayout(ptyId, { kind: 'desktop', cols: restoreCols, diff --git a/src/main/runtime/remote-desktop-driver.test.ts b/src/main/runtime/remote-desktop-driver.test.ts new file mode 100644 index 00000000000..d3c7a5f242f --- /dev/null +++ b/src/main/runtime/remote-desktop-driver.test.ts @@ -0,0 +1,209 @@ +/** + * Tests for the remote-desktop viewer width driver. + * + * A remote (relay/shared-control) desktop viewer takes the PTY width floor so + * the host's own fit cascade stops resizing the viewed PTY out from under it + * (the remote alt-screen "porridge"). Mirrors the mobile presence lock but + * suppresses only RESIZE, never input. Covers: + * - idle → remote-desktop on register; release to idle on last unregister + * - multi-viewer: driver survives until the last viewer detaches + * - a live mobile driver outranks a remote-desktop viewer + * - isPtyResizeDrivenRemotely gates host resize for mobile AND remote-desktop + * - PTY exit clears the remote-desktop registry + */ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' +import type * as GitUsernameModule from '../git/git-username' +import { OrcaRuntimeService } from './orca-runtime' + +vi.mock('../git/worktree', () => ({ + listWorktrees: vi.fn().mockResolvedValue([]), + listWorktreesStrict: vi.fn().mockResolvedValue([]) +})) +vi.mock('../hooks', () => ({ + createSetupRunnerScript: vi.fn(), + getEffectiveHooks: vi.fn().mockReturnValue(null), + runHook: vi.fn().mockResolvedValue({ success: true, output: '' }) +})) +vi.mock('../ipc/worktree-logic', async (importOriginal) => { + const actual = (await importOriginal()) as Record + return { ...actual, computeWorktreePath: vi.fn(), ensurePathWithinWorkspace: vi.fn() } +}) +vi.mock('../ipc/filesystem-auth', () => ({ invalidateAuthorizedRootsCache: vi.fn() })) +vi.mock('../git/repo', async (importOriginal) => { + const actual = (await importOriginal()) as Record + return { + ...actual, + getDefaultBaseRef: vi.fn().mockReturnValue('origin/main'), + getBranchConflictKind: vi.fn().mockResolvedValue(null) + } +}) +vi.mock('../git/git-username', async () => { + const actual = await vi.importActual('../git/git-username') + return { ...actual, resolveLocalGitUsername: vi.fn(async () => '') } +}) + +const store = { + getRepo: () => ({ + id: 'repo-1', + path: '/tmp/repo', + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 + }), + getRepos: () => [store.getRepo()], + addRepo: () => {}, + updateRepo: () => undefined as never, + getAllWorktreeMeta: () => ({}), + getWorktreeMeta: () => undefined, + getGitHubCache: () => ({ pr: {}, issue: {} }), + setWorktreeMeta: () => undefined as never, + removeWorktreeMeta: () => {}, + getSettings: () => ({ + workspaceDir: '/tmp/workspaces', + nestWorkspaces: false, + refreshLocalBaseRefOnWorktreeCreate: false, + branchPrefix: 'none', + branchPrefixCustom: '', + mobileAutoRestoreFitMs: 5_000 + }) +} + +function createRuntime() { + const runtime = new OrcaRuntimeService(store) + const ptySizes = new Map([ + ['pty-1', { cols: 150, rows: 40 }] + ]) + const driverEvents: { ptyId: string; driver: { kind: string; clientId?: string } }[] = [] + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + resize: (ptyId, cols, rows) => { + ptySizes.set(ptyId, { cols, rows }) + return true + }, + getSize: (ptyId) => ptySizes.get(ptyId) ?? null + }) + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree: vi.fn(), + createTerminal: vi.fn(), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: (ptyId, driver) => { + driverEvents.push({ ptyId, driver: { ...driver } }) + } + }) + return { runtime, driverEvents } +} + +describe('remote desktop viewer width driver', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => vi.useRealTimers()) + + it('applying a viewport suppresses host resize without touching driver state', async () => { + const { runtime, driverEvents } = createRuntime() + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) + + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 100, rows: 40 }) + // It is deliberately NOT a driver kind: the presence-lock state machine and + // its cross-layer driver-change notifications stay untouched. + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'idle' }) + expect(driverEvents).toHaveLength(0) + }) + + it('sizes the PTY to the SMALLEST attached viewer (smallest client wins)', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 100, rows: 40 }) + + // A narrower viewer joins — the PTY shrinks so its wider frames never + // overflow the narrow grid. + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 80, 30) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 30 }) + + // The wide viewer growing does not widen the PTY past the narrow viewer. + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 140, 50) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 30 }) + + // The narrow viewer leaves — the PTY re-fits to the survivor's width. + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-B') + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 140, rows: 50 }) + }) + + it('stops suppressing host resize only when the LAST viewer detaches', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 80, 40) + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + + // First viewer leaves — another remote viewer still holds the floor. + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + + // Last viewer leaves — the host reclaims its own width (next pty:resize applies). + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-B') + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + }) + + it('coexists with a mobile driver and outlives it (host stays suppressed)', async () => { + const { runtime } = createRuntime() + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) + + // A desktop viewer registering must NOT disturb the mobile driver. + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + + // When the phone leaves, the surviving viewer keeps host resize suppressed + // (the registry is independent of the mobile driver state). + runtime.onClientDisconnected('phone-A') + vi.advanceTimersByTime(10_000) + expect(runtime.getDriver('pty-1').kind).not.toBe('mobile') + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + }) + + it('isPtyResizeDrivenRemotely is false for idle and desktop drivers', async () => { + const { runtime } = createRuntime() + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + }) + + it('reclaims the host width when the last viewer detaches', async () => { + const { runtime } = createRuntime() + // The host renderer reports its own 120-wide geometry (pty:reportGeometry). + runtime.recordRemoteDesktopHostReclaimTarget('pty-1', 120, 40) + // The viewer drives the source PTY to its own 80-wide viewport. + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 40) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 40 }) + + // Detaching the last viewer must actively resize the PTY back to the host's + // OWN width (120), not the departed viewer's polluted 80. + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 120, rows: 40 }) + }) + + it('PTY exit clears the remote-desktop registry', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + + runtime.onPtyExit('pty-1', 0) + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + + // A fresh viewer on the same id re-establishes suppression cleanly (no stale set). + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 100, 40) + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + }) +}) diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 73c77701c97..09993f6a151 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -88,6 +88,12 @@ type TerminalMultiplexStream = { isMobile: boolean ackOutput: boolean ackInFlightBytes: number + // Why: whether THIS stream registered a remote-desktop width driver, so + // detach only unregisters what it registered — a passive (viewport-less) + // stream sharing a client id must not release another stream's width floor. + registeredRemoteDesktopDriver: boolean + remoteDesktopSubscriptionKey: string + pendingRemoteDesktopViewport: { cols: number; rows: number } | null buffering: boolean ackPendingOutput: TerminalOutputFrameChunk[] ackPendingOutputBytes: number @@ -631,6 +637,7 @@ async function sendMobileResizeRestream( async function updateViewportForClient( runtime: OrcaRuntimeService, ptyId: string, + subscriptionKey: string, client: TerminalViewportClient, viewport: { cols: number; rows: number }, defaultType: 'mobile' | 'desktop' @@ -639,7 +646,16 @@ async function updateViewportForClient( if (type === 'mobile') { return runtime.updateMobileViewport(ptyId, client.id, viewport) } - const updated = await runtime.updateDesktopViewport(ptyId, viewport) + // Why: a remote desktop viewer's viewport drives the source PTY to the + // smallest attached viewer (tmux "smallest client wins") and registers it as + // a width owner so the host's fit cascade stops fighting it. + const updated = await runtime.updateRemoteDesktopViewer( + ptyId, + subscriptionKey, + client.id, + viewport.cols, + viewport.rows + ) return { updated, applied: updated } } @@ -1280,6 +1296,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ const viewportUpdate = await updateViewportForClient( runtime, leaf.ptyId, + `viewport:${params.client.id}`, params.client, params.viewport, 'mobile' @@ -1506,6 +1523,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ stream.exitWaiterAbort.abort() if (stream.isMobile && stream.client?.id) { runtime.handleMobileUnsubscribe(stream.ptyId, stream.client.id) + } else if (stream.registeredRemoteDesktopDriver && stream.client?.id) { + // Why: release the remote-desktop width floor so the host can reclaim + // its own width once the last remote viewer leaves — but only if THIS + // stream took it (a passive stream must not release a peer's floor). + runtime.unregisterRemoteDesktopViewer(stream.ptyId, stream.remoteDesktopSubscriptionKey) } if (emitEnd) { emit({ type: 'end', streamId }) @@ -1567,9 +1589,20 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (!viewport || typeof viewport.cols !== 'number' || typeof viewport.rows !== 'number') { return } + // Why: a desktop viewport update registers this stream as a width + // owner (even if it had no initial viewport at subscribe), so detach + // must later release it. + if (!stream.isMobile && stream.client?.id) { + stream.registeredRemoteDesktopDriver = true + if (stream.buffering) { + stream.pendingRemoteDesktopViewport = { cols: viewport.cols, rows: viewport.rows } + return + } + } void updateViewportForClient( runtime, stream.ptyId, + stream.remoteDesktopSubscriptionKey, stream.client, { cols: viewport.cols, rows: viewport.rows }, stream.isMobile ? 'mobile' : 'desktop' @@ -1730,6 +1763,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ isMobile, ackOutput: request.capabilities?.ackOutput === 1, ackInFlightBytes: 0, + registeredRemoteDesktopDriver: false, + remoteDesktopSubscriptionKey: `multiplex:${request.streamId}`, + pendingRemoteDesktopViewport: null, buffering: true, ackPendingOutput: [], ackPendingOutputBytes: 0, @@ -1788,12 +1824,29 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (isMobile && request.client?.id) { await runtime.handleMobileSubscribe(ptyId, request.client.id, request.viewport) - } else if (request.viewport && request.client) { + } else if (request.client?.id && request.viewport) { + // Why: a remote desktop viewer that reports a viewport is driving + // the PTY width, so the runtime registers it as a + // width owner (host fit cascade suppressed) and sizes the PTY to the + // smallest attached viewer. A viewport-less subscriber is a passive + // watcher and must NOT lock host resize. + stream.registeredRemoteDesktopDriver = true + stream.pendingRemoteDesktopViewport = request.viewport + } + if ( + !isMobile && + request.client?.id && + stream.registeredRemoteDesktopDriver && + stream.pendingRemoteDesktopViewport + ) { + const viewport = stream.pendingRemoteDesktopViewport + stream.pendingRemoteDesktopViewport = null await updateViewportForClient( runtime, ptyId, + stream.remoteDesktopSubscriptionKey, request.client, - request.viewport, + viewport, 'desktop' ) } @@ -1906,6 +1959,23 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ stream.pendingOutputBytes = 0 stream.pendingOutputOverflowed = false stream.outputBatcher.flush() + if ( + !stream.isMobile && + stream.client?.id && + stream.registeredRemoteDesktopDriver && + stream.pendingRemoteDesktopViewport + ) { + const viewport = stream.pendingRemoteDesktopViewport + stream.pendingRemoteDesktopViewport = null + void updateViewportForClient( + runtime, + ptyId, + stream.remoteDesktopSubscriptionKey, + stream.client, + viewport, + 'desktop' + ).catch(() => {}) + } stream.unsubscribeResize = runtime.subscribeToTerminalResize(ptyId, (event) => { stream.outputBatcher.flush() @@ -2047,6 +2117,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ const ptyId = leaf.ptyId const clientId = params.client?.id + // Why: only unregister the width floor this subscription took (see the + // multiplex stream's registeredRemoteDesktopDriver note). + let registeredRemoteDesktopDriver = false if (!useBinaryStream) { const read = await runtime.readTerminal(params.terminal) const serialized = await serializeBudgetedMobileSnapshot(runtime, ptyId, false) @@ -2124,9 +2197,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } const streamId = nextTerminalStreamId++ + const remoteDesktopSubscriptionKey = `stream:${streamId}` let cursor = 0 let closed = false let buffering = true + let pendingRemoteDesktopViewport: { cols: number; rows: number } | null = null // Why: the cols the mobile client last rewrapped to; gate the // resize re-stream so it only fires on an actual width change. let lastResizeCols: number | undefined @@ -2159,6 +2234,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ unregisterBinaryHandler() if (isMobile && clientId) { runtime.handleMobileUnsubscribe(ptyId, clientId) + } else if (registeredRemoteDesktopDriver && clientId) { + runtime.unregisterRemoteDesktopViewer(ptyId, remoteDesktopSubscriptionKey) } emit({ type: 'end' }) resolveStream() @@ -2227,9 +2304,17 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ ) { return } + if (clientId) { + registeredRemoteDesktopDriver = true + if (buffering) { + pendingRemoteDesktopViewport = { cols: viewport.cols, rows: viewport.rows } + return + } + } void updateViewportForClient( runtime, ptyId, + remoteDesktopSubscriptionKey, params.client, { cols: viewport.cols, rows: viewport.rows }, 'desktop' @@ -2240,6 +2325,12 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ try { if (isMobile && clientId) { await runtime.handleMobileSubscribe(ptyId, clientId, params.viewport) + } else if (clientId && params.viewport) { + // Why: legacy remote desktop viewers that report a viewport also take + // the width floor (host fit cascade suppressed) and size the PTY to + // the smallest attached viewer. Viewport-less watchers do not lock. + registeredRemoteDesktopDriver = true + pendingRemoteDesktopViewport = params.viewport } if (closed) { return @@ -2275,6 +2366,27 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ unsubscribeStreamData() } + if ( + clientId && + params.client && + registeredRemoteDesktopDriver && + pendingRemoteDesktopViewport + ) { + const viewport = pendingRemoteDesktopViewport + pendingRemoteDesktopViewport = null + await updateViewportForClient( + runtime, + ptyId, + remoteDesktopSubscriptionKey, + params.client, + viewport, + 'desktop' + ) + } + if (closed) { + return + } + let read = await runtime.readTerminal(params.terminal) let serialized = await serializeBudgetedMobileSnapshot(runtime, ptyId, isMobile) if (closed) { @@ -2405,6 +2517,23 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } pendingOutputBytes = 0 outputBatcher.flush() + if ( + clientId && + params.client && + registeredRemoteDesktopDriver && + pendingRemoteDesktopViewport + ) { + const viewport = pendingRemoteDesktopViewport + pendingRemoteDesktopViewport = null + void updateViewportForClient( + runtime, + ptyId, + remoteDesktopSubscriptionKey, + params.client, + viewport, + 'desktop' + ).catch(() => {}) + } const sendResizedFrame = (event: { cols: number diff --git a/src/main/runtime/rpc/terminal-multiplex.test.ts b/src/main/runtime/rpc/terminal-multiplex.test.ts index 62dcfddc5ab..3f2f056fac6 100644 --- a/src/main/runtime/rpc/terminal-multiplex.test.ts +++ b/src/main/runtime/rpc/terminal-multiplex.test.ts @@ -26,6 +26,9 @@ function stubRuntime(overrides: Partial = {}): OrcaRuntimeSe // only stub the legacy resolveLeafForHandle still bind; tests that need a // null/stale leaf override this explicitly. resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + updateRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + unregisterRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + isPtyResizeDrivenRemotely: vi.fn().mockReturnValue(false), ...overrides } as OrcaRuntimeService } @@ -82,8 +85,7 @@ describe('terminal multiplex RPC', () => { cleanup?.() }), waitForTerminal: vi.fn(() => new Promise(() => {})), - sendTerminal: vi.fn().mockResolvedValue({ accepted: true }), - updateDesktopViewport: vi.fn().mockResolvedValue(true) + sendTerminal: vi.fn().mockResolvedValue({ accepted: true }) }) const dispatcher = new RpcDispatcher({ runtime, @@ -143,10 +145,13 @@ describe('terminal multiplex RPC', () => { }) ]) ) - expect(runtime.updateDesktopViewport).toHaveBeenCalledWith('pty-1', { - cols: 300, - rows: 150 - }) + expect(runtime.updateRemoteDesktopViewer).toHaveBeenCalledWith( + 'pty-1', + 'multiplex:5', + 'desktop-1', + 300, + 150 + ) expect(handlers.has(5)).toBe(true) dataListenerRef.current?.('a') @@ -189,10 +194,13 @@ describe('terminal multiplex RPC', () => { )! ) await vi.waitFor(() => - expect(runtime.updateDesktopViewport).toHaveBeenLastCalledWith('pty-1', { - cols: 100, - rows: 30 - }) + expect(runtime.updateRemoteDesktopViewer).toHaveBeenLastCalledWith( + 'pty-1', + 'multiplex:5', + 'desktop-1', + 100, + 30 + ) ) const snapshotStartFrame = binaryFrames