diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index b1cdef23da3..751245fb62d 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -4075,12 +4075,13 @@ describe('registerPtyHandlers', () => { applied: { cols: number; rows: number } | null resize?: (cols: number, rows: number) => void getAppliedSize?: (id: string) => Promise<{ cols: number; rows: number } | null> - }): void { + }): ReturnType { + const write = vi.fn() setLocalPtyProvider({ spawn: vi.fn(async (opts: { sessionId?: string }) => ({ id: opts.sessionId ?? 'daemon-pty' })), - write: vi.fn(), + write, resize: vi.fn(args.resize ?? (() => {})), getAppliedSize: vi.fn(args.getAppliedSize ?? (async () => args.applied)), kill: vi.fn(), @@ -4090,6 +4091,7 @@ describe('registerPtyHandlers', () => { listProcesses: vi.fn(async () => []), getForegroundProcess: vi.fn(async () => null) } as never) + return write } const resizeListener = (): ((event: unknown, args: unknown) => void) => { @@ -4170,12 +4172,10 @@ 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() @@ -4204,12 +4204,10 @@ 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() @@ -4232,7 +4230,7 @@ describe('registerPtyHandlers', () => { 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), + isRemoteDesktopResizeDriven: vi.fn(() => true), isResizeSuppressed: vi.fn(() => false), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), @@ -4254,6 +4252,80 @@ describe('registerPtyHandlers', () => { expect(runtime.recordRemoteDesktopHostReclaimTarget).toHaveBeenCalledWith(id, 125, 48) expect(runtime.onExternalPtyResize).not.toHaveBeenCalled() }) + + it('lets trusted host activity reclaim remote viewport ownership', () => { + const claimRemoteDesktopHost = vi.fn().mockResolvedValue(true) + const runtime = { + setPtyController: vi.fn(), + claimRemoteDesktopHost + } + handlers.clear() + registerPtyHandlers(mainWindow as never, runtime as never) + const call = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:claimViewport') + const claimListener = call?.[1] as + | ((event: unknown, args: { id: string; cols: number; rows: number }) => void) + | undefined + expect(claimListener).toBeTypeOf('function') + + claimListener?.(mainWindowIpcEvent, { id: 'pty-1', cols: 125, rows: 48 }) + + expect(claimRemoteDesktopHost).toHaveBeenCalledWith('pty-1', 125, 48) + }) + + it('does not forward host input when viewport reclaim fails', async () => { + const write = setupProviderWithAppliedSize({ applied: { cols: 80, rows: 24 } }) + const runtime = { + setPtyController: vi.fn(), + createPreAllocatedTerminalHandle: vi.fn(() => null), + registerPty: vi.fn(), + getDriver: vi.fn(() => ({ kind: 'idle' })), + claimRemoteDesktopHost: vi.fn().mockResolvedValue(false), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: 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 + const claim = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:claimViewport') + const writeEvent = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:write') + + claim?.[1](mainWindowIpcEvent, { id, cols: 125, rows: 48 }) + writeEvent?.[1](mainWindowIpcEvent, { id, data: 'x' }) + await Promise.resolve() + + expect(write).not.toHaveBeenCalled() + }) + + it('does not populate the remote reclaim cache when only a phone drives', 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: 'mobile', clientId: 'phone-A' })), + isRemoteDesktopResizeDriven: vi.fn(() => false), + 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() + + resizeListener()(mainWindowIpcEvent, { id, cols: 125, rows: 48 }) + + expect(resizeSpy).not.toHaveBeenCalled() + expect(runtime.recordRemoteDesktopHostReclaimTarget).not.toHaveBeenCalled() + expect(runtime.onExternalPtyResize).not.toHaveBeenCalled() + }) }) it('injects ORCA_TERMINAL_HANDLE for non-local PTY providers', async () => { @@ -4326,7 +4398,6 @@ 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() @@ -4406,7 +4477,6 @@ 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() @@ -4438,7 +4508,6 @@ 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() @@ -4476,7 +4545,6 @@ 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() @@ -4574,7 +4642,6 @@ 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() @@ -4627,7 +4694,6 @@ 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() @@ -5116,7 +5182,6 @@ 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() @@ -5374,7 +5439,6 @@ 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() @@ -5484,7 +5548,6 @@ 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() @@ -5579,7 +5642,6 @@ 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() @@ -5684,7 +5746,6 @@ 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 154e62cfd96..4dcc451324a 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4441,6 +4441,7 @@ export function registerPtyHandlers( } type PtyWritePayload = { id: string; data: string } + type PtyViewportClaimPayload = { id: string; cols: number; rows: number } const isPtyWritePayload = (value: unknown): value is PtyWritePayload => typeof value === 'object' && @@ -4449,6 +4450,18 @@ export function registerPtyHandlers( (value as { id: string }).id.length > 0 && typeof (value as { data?: unknown }).data === 'string' + const isPtyViewportClaimPayload = (value: unknown): value is PtyViewportClaimPayload => + typeof value === 'object' && + value !== null && + typeof (value as { id?: unknown }).id === 'string' && + (value as { id: string }).id.length > 0 && + typeof (value as { cols?: unknown }).cols === 'number' && + Number.isFinite((value as { cols: number }).cols) && + typeof (value as { rows?: unknown }).rows === 'number' && + Number.isFinite((value as { rows: number }).rows) && + (value as { cols: number }).cols > 0 && + (value as { rows: number }).rows > 0 + const isPtyWriteEventFromMainWindow = ( event: IpcMainEvent | IpcMainInvokeEvent, mainWebContents: WebContents @@ -4512,17 +4525,55 @@ export function registerPtyHandlers( } } + const hostViewportClaimTails = new Map>() + ipcMain.on('pty:write', (event, args: unknown) => { if (!isPtyWriteEventFromMainWindow(event, mainWindow.webContents) || !isPtyWritePayload(args)) { return } + const claimTail = hostViewportClaimTails.get(args.id) + if (claimTail) { + void claimTail.then((claimed) => (claimed ? writePtyInput(args) : false)) + return + } writePtyInput(args) }) ipcMain.handle('pty:writeAccepted', (event, args: unknown): boolean | Promise => { if (!isPtyWriteEventFromMainWindow(event, mainWindow.webContents) || !isPtyWritePayload(args)) { return false } - return writePtyInputAccepted(args) + const claimTail = hostViewportClaimTails.get(args.id) + return claimTail + ? claimTail.then((claimed) => (claimed ? writePtyInputAccepted(args) : false)) + : writePtyInputAccepted(args) + }) + + ipcMain.removeAllListeners('pty:claimViewport') + ipcMain.on('pty:claimViewport', (event, args: unknown) => { + if ( + !isPtyWriteEventFromMainWindow(event, mainWindow.webContents) || + !runtime || + !isPtyViewportClaimPayload(args) + ) { + return + } + const prior = hostViewportClaimTails.get(args.id) + // Why: two panes can mirror one PTY. Never let a later no-op claim replace + // the in-flight resize that the following host input must await. + const claim = ( + prior + ? prior.then( + () => runtime.claimRemoteDesktopHost(args.id, args.cols, args.rows), + () => runtime.claimRemoteDesktopHost(args.id, args.cols, args.rows) + ) + : runtime.claimRemoteDesktopHost(args.id, args.cols, args.rows) + ).catch(() => false) + hostViewportClaimTails.set(args.id, claim) + void claim.then(() => { + if (hostViewportClaimTails.get(args.id) === claim) { + hostViewportClaimTails.delete(args.id) + } + }) }) // Why: resize is fire-and-forget — the renderer doesn't need a reply. @@ -4548,8 +4599,12 @@ export function registerPtyHandlers( // — 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) + const mobileOwnsResize = runtime?.getDriver(args.id).kind === 'mobile' + const remoteDesktopOwnsResize = runtime?.isRemoteDesktopResizeDriven?.(args.id) === true + if (mobileOwnsResize || remoteDesktopOwnsResize) { + if (remoteDesktopOwnsResize) { + runtime?.recordRemoteDesktopHostReclaimTarget(args.id, args.cols, args.rows) + } return } const provider = tryGetProviderForPty(args.id) diff --git a/src/main/ipc/runtime.ts b/src/main/ipc/runtime.ts index e2805fd2862..3b9a8719afc 100644 --- a/src/main/ipc/runtime.ts +++ b/src/main/ipc/runtime.ts @@ -48,7 +48,12 @@ export function registerRuntimeHandlers(runtime: OrcaRuntimeService): void { ipcMain.removeHandler('runtime:getTerminalFitOverrides') ipcMain.handle( 'runtime:getTerminalFitOverrides', - (): { ptyId: string; mode: 'mobile-fit'; cols: number; rows: number }[] => { + (): { + ptyId: string + mode: 'mobile-fit' | 'remote-desktop-fit' + cols: number + rows: number + }[] => { const overrides = runtime.getAllTerminalFitOverrides() return Array.from(overrides.entries()).map(([ptyId, override]) => ({ ptyId, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 87bd81898e8..0f60418e721 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1378,7 +1378,7 @@ type RuntimeNotifier = { resumeSleepingAgents?(worktreeId: string): void terminalFitOverrideChanged( ptyId: string, - mode: 'mobile-fit' | 'desktop-fit', + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit', cols: number, rows: number ): void @@ -2062,7 +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 } + | { kind: 'remote-desktop'; cols: number; rows: number; ownerSubscriptionKey: string } // Why: authoritative layout state with monotonic seq. Bumped on every // applyLayout success; emitted on mobile subscribe-stream events so clients @@ -2181,7 +2181,13 @@ export class OrcaRuntimeService { // invoked from resizeForClient and onClientDisconnected/onPtyExit. private fitOverrideListeners = new Map< string, - Set<(event: { mode: 'mobile-fit' | 'desktop-fit'; cols: number; rows: number }) => void> + Set< + (event: { + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' + cols: number + rows: number + }) => void + > >() private driverListeners = new Map void>>() private subscriptionCleanups = new Map void>() @@ -2335,9 +2341,14 @@ export class OrcaRuntimeService { // each stream must release only the width floor it registered. private remoteDesktopViewers = new Map< string, - Map + Map >() + private remoteDesktopOwners = new Map() + private remoteDesktopActivity = 0 private remoteDesktopHostReclaimTargets = new Map() + // Why: a completed host reclaim must not consume the cache if a newer + // viewer mutation landed while that serialized layout was in flight. + private remoteDesktopViewerRevisions = new Map() // Why: resubscribe-grace window. When the last mobile subscriber for a // PTY unsubscribes, we hold the driver=mobile{clientId} state and the @@ -6488,7 +6499,11 @@ export class OrcaRuntimeService { subscribeToFitOverrideChanges( ptyId: string, - listener: (event: { mode: 'mobile-fit' | 'desktop-fit'; cols: number; rows: number }) => void + listener: (event: { + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' + cols: number + rows: number + }) => void ): () => void { return addListenerToMap(this.fitOverrideListeners, ptyId, listener) } @@ -6499,7 +6514,7 @@ export class OrcaRuntimeService { private notifyFitOverrideListeners( ptyId: string, - mode: 'mobile-fit' | 'desktop-fit', + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit', cols: number, rows: number ): void { @@ -7776,11 +7791,26 @@ export class OrcaRuntimeService { return this.terminalFitOverrides.get(ptyId) ?? null } - getAllTerminalFitOverrides(): Map { - const result = new Map() + getAllTerminalFitOverrides(): Map< + string, + { mode: 'mobile-fit' | 'remote-desktop-fit'; cols: number; rows: number } + > { + const result = new Map< + string, + { mode: 'mobile-fit' | 'remote-desktop-fit'; cols: number; rows: number } + >() for (const [ptyId, override] of this.terminalFitOverrides) { result.set(ptyId, { mode: override.mode, cols: override.cols, rows: override.rows }) } + for (const [ptyId] of this.remoteDesktopOwners) { + if (result.has(ptyId)) { + continue + } + const size = this.getTerminalSize(ptyId) + if (size) { + result.set(ptyId, { mode: 'remote-desktop-fit', ...size }) + } + } return result } @@ -7866,6 +7896,11 @@ export class OrcaRuntimeService { void this.applyRemoteDesktopLayout(ptyId) continue } else if (cur?.kind === 'phone' && this.getAutoRestoreFitMs() != null) { + if (this.remoteDesktopHostReclaimTargets.has(ptyId)) { + this.setDriver(ptyId, { kind: 'idle' }) + void this.applyRemoteDesktopLayout(ptyId) + continue + } // Use the soft-leaver's snapshot baseline as a hint, falling // through to resolveDesktopRestoreTarget for missing values. const fallback = this.resolveDesktopRestoreTarget(ptyId) @@ -7909,6 +7944,11 @@ export class OrcaRuntimeService { void this.applyRemoteDesktopLayout(ptyId) continue } else if (cur?.kind === 'phone' && this.getAutoRestoreFitMs() != null) { + if (this.remoteDesktopHostReclaimTargets.has(ptyId)) { + this.setDriver(ptyId, { kind: 'idle' }) + void this.applyRemoteDesktopLayout(ptyId) + continue + } const fallback = this.resolveDesktopRestoreTarget(ptyId) const cols = baseline?.cols ?? fallback.cols const rows = baseline?.rows ?? fallback.rows @@ -8038,7 +8078,9 @@ export class OrcaRuntimeService { this.notifier?.terminalDriverChanged(ptyId, { kind: 'idle' }) } this.remoteDesktopViewers.delete(ptyId) + this.remoteDesktopOwners.delete(ptyId) this.remoteDesktopHostReclaimTargets.delete(ptyId) + this.remoteDesktopViewerRevisions.delete(ptyId) this.disposeHeadlessTerminal(ptyId) this.agentDetector?.onExit(ptyId) const pty = this.ptysById.get(ptyId) @@ -8097,10 +8139,10 @@ 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 + // OR an active remote desktop viewer owns 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 + // Note: this is intentionally NOT a driver kind. An active remote 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: @@ -8110,7 +8152,28 @@ export class OrcaRuntimeService { if (this.getDriver(ptyId).kind === 'mobile') { return true } - return this.hasRemoteDesktopViewers(ptyId) + return this.isRemoteDesktopResizeDriven(ptyId) + } + + isRemoteDesktopResizeDriven(ptyId: string): boolean { + return this.remoteDesktopOwners.has(ptyId) + } + + isRemoteDesktopViewerOwner(ptyId: string, subscriptionKey: string): boolean { + return this.remoteDesktopOwners.get(ptyId) === subscriptionKey + } + + getRemoteDesktopFitHold( + ptyId: string, + subscriptionKey: string + ): { mode: 'remote-desktop-fit' | 'desktop-fit'; cols: number; rows: number } { + const size = this.getTerminalSize(ptyId) ?? { cols: 0, rows: 0 } + return { + mode: this.isRemoteDesktopViewerOwner(ptyId, subscriptionKey) + ? 'desktop-fit' + : 'remote-desktop-fit', + ...size + } } private hasRemoteDesktopViewers(ptyId: string): boolean { @@ -8118,19 +8181,9 @@ export class OrcaRuntimeService { 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 activeRemoteDesktopViewport(ptyId: string): { cols: number; rows: number } | null { + const owner = this.remoteDesktopOwners.get(ptyId) + return owner ? (this.remoteDesktopViewers.get(ptyId)?.get(owner) ?? null) : null } private resolveRemoteDesktopHostReclaimTarget(ptyId: string): { cols: number; rows: number } { @@ -8138,15 +8191,10 @@ export class OrcaRuntimeService { 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 } + // Why: a viewer can join while a phone owns the actual PTY size. The + // mobile restore chain retains the pre-phone desktop geometry; current + // PTY size alone would incorrectly capture the phone grid as host truth. + return this.resolveDesktopRestoreTarget(ptyId) } private ensureRemoteDesktopHostReclaimTarget(ptyId: string): void { @@ -8159,21 +8207,38 @@ export class OrcaRuntimeService { } recordRemoteDesktopHostReclaimTarget(ptyId: string, cols: number, rows: number): void { - if (cols <= 0 || rows <= 0) { + // Why: phone presence also suppresses host resize, but must not seed the + // separate remote-viewer cache when no desktop stream owns a width floor. + if (!this.remoteDesktopOwners.has(ptyId) || cols <= 0 || rows <= 0) { return } this.remoteDesktopHostReclaimTargets.set(ptyId, { cols, rows }) } + private hasRemoteDesktopLayoutState(ptyId: string): boolean { + return this.remoteDesktopOwners.has(ptyId) || this.remoteDesktopHostReclaimTargets.has(ptyId) + } + + private bumpRemoteDesktopViewerRevision(ptyId: string): number { + const revision = (this.remoteDesktopViewerRevisions.get(ptyId) ?? 0) + 1 + this.remoteDesktopViewerRevisions.set(ptyId, revision) + return revision + } + 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 target = this.activeRemoteDesktopViewport(ptyId) const reclaimingHost = !target + const viewerRevision = this.remoteDesktopViewerRevisions.get(ptyId) ?? 0 const layoutTarget: PtyLayoutTarget = target - ? { kind: 'remote-desktop', cols: target.cols, rows: target.rows } + ? { + kind: 'remote-desktop', + cols: target.cols, + rows: target.rows, + ownerSubscriptionKey: this.remoteDesktopOwners.get(ptyId)! + } : { kind: 'desktop', ...this.resolveRemoteDesktopHostReclaimTarget(ptyId) } this.freshSubscribeGuard.add(ptyId) try { @@ -8182,7 +8247,12 @@ export class OrcaRuntimeService { // landed. If it failed, the PTY is still at the remote-viewer width, so // keep the target for the next reclaim (otherwise it resolves via the // stale remote width and never restores true host geometry). - if (reclaimingHost && result.ok) { + if ( + reclaimingHost && + result.ok && + !this.remoteDesktopOwners.has(ptyId) && + this.remoteDesktopViewerRevisions.get(ptyId) === viewerRevision + ) { this.remoteDesktopHostReclaimTargets.delete(ptyId) } return result.ok @@ -8191,38 +8261,125 @@ export class OrcaRuntimeService { } } - // 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. + // Why: attachment only records geometry. Passive hydration/reconnect must not + // steal the shared PTY from the desktop where the user is actively working. async updateRemoteDesktopViewer( ptyId: string, subscriptionKey: string, clientId: string, cols: number, - rows: number + rows: number, + claim = true ): Promise { const viewport = clampTerminalViewport(cols, rows) - this.ensureRemoteDesktopHostReclaimTarget(ptyId) + if (claim) { + this.ensureRemoteDesktopHostReclaimTarget(ptyId) + } let viewers = this.remoteDesktopViewers.get(ptyId) if (!viewers) { - viewers = new Map() + viewers = new Map< + string, + { clientId: string; cols: number; rows: number; activity: number } + >() this.remoteDesktopViewers.set(ptyId, viewers) } - viewers.set(subscriptionKey, { clientId, cols: viewport.cols, rows: viewport.rows }) + const prior = viewers.get(subscriptionKey) + if ( + prior && + prior.cols === viewport.cols && + prior.rows === viewport.rows && + (!claim || this.remoteDesktopOwners.get(ptyId) === subscriptionKey) + ) { + if (claim && this.remoteDesktopOwners.get(ptyId) === subscriptionKey) { + const size = this.getTerminalSize(ptyId) + if (size?.cols !== viewport.cols || size.rows !== viewport.rows) { + return this.applyRemoteDesktopLayout(ptyId) + } + } + return true + } + const activity = claim ? ++this.remoteDesktopActivity : (prior?.activity ?? 0) + viewers.set(subscriptionKey, { clientId, cols: viewport.cols, rows: viewport.rows, activity }) + this.bumpRemoteDesktopViewerRevision(ptyId) + if (claim) { + this.remoteDesktopOwners.set(ptyId, subscriptionKey) + return this.applyRemoteDesktopLayout(ptyId) + } + return true + } + + claimRemoteDesktopViewer(ptyId: string, subscriptionKey: string): Promise { + const viewer = this.remoteDesktopViewers.get(ptyId)?.get(subscriptionKey) + if (!viewer) { + return Promise.resolve(false) + } + if (this.remoteDesktopOwners.get(ptyId) === subscriptionKey) { + const size = this.getTerminalSize(ptyId) + return size?.cols === viewer.cols && size.rows === viewer.rows + ? Promise.resolve(true) + : this.applyRemoteDesktopLayout(ptyId) + } + this.ensureRemoteDesktopHostReclaimTarget(ptyId) + viewer.activity = ++this.remoteDesktopActivity + this.remoteDesktopOwners.set(ptyId, subscriptionKey) + this.bumpRemoteDesktopViewerRevision(ptyId) + return this.applyRemoteDesktopLayout(ptyId) + } + + claimRemoteDesktopHost(ptyId: string, cols: number, rows: number): Promise { + if (!this.remoteDesktopOwners.has(ptyId)) { + // Why: disconnect can remove the owner before its queued host resize + // lands. A host input in that window must join the reclaim, not pass it. + return this.remoteDesktopHostReclaimTargets.has(ptyId) + ? this.applyRemoteDesktopLayout(ptyId) + : Promise.resolve(true) + } + const viewport = clampTerminalViewport(cols, rows) + this.remoteDesktopHostReclaimTargets.set(ptyId, viewport) + this.remoteDesktopOwners.delete(ptyId) + this.bumpRemoteDesktopViewerRevision(ptyId) return this.applyRemoteDesktopLayout(ptyId) } unregisterRemoteDesktopViewer(ptyId: string, subscriptionKey: string): Promise { + return this.unregisterRemoteDesktopViewers(ptyId, [subscriptionKey]) + } + + unregisterRemoteDesktopViewers( + ptyId: string, + subscriptionKeys: Iterable + ): Promise { const viewers = this.remoteDesktopViewers.get(ptyId) if (!viewers) { return Promise.resolve(false) } - viewers.delete(subscriptionKey) + let changed = false + let removedOwner = false + for (const subscriptionKey of subscriptionKeys) { + removedOwner = this.remoteDesktopOwners.get(ptyId) === subscriptionKey || removedOwner + changed = viewers.delete(subscriptionKey) || changed + } + if (!changed) { + return Promise.resolve(false) + } if (viewers.size === 0) { this.remoteDesktopViewers.delete(ptyId) } - return this.applyRemoteDesktopLayout(ptyId) + if (removedOwner) { + let fallback: { key: string; activity: number } | null = null + for (const [key, viewer] of viewers) { + if (viewer.activity > 0 && (!fallback || viewer.activity > fallback.activity)) { + fallback = { key, activity: viewer.activity } + } + } + if (fallback) { + this.remoteDesktopOwners.set(ptyId, fallback.key) + } else { + this.remoteDesktopOwners.delete(ptyId) + } + } + this.bumpRemoteDesktopViewerRevision(ptyId) + return removedOwner ? this.applyRemoteDesktopLayout(ptyId) : Promise.resolve(true) } // Why: the one-shot `terminal.updateViewport` RPC has no disconnect hook, so @@ -8237,24 +8394,42 @@ export class OrcaRuntimeService { ptyId: string, clientId: string, cols: number, - rows: number + rows: number, + claim = false ): Promise { const viewers = this.remoteDesktopViewers.get(ptyId) if (!viewers) { return Promise.resolve(false) } const viewport = clampTerminalViewport(cols, rows) + if (claim) { + // Why: terminal.send may be the first activity while the stream is only + // passively registered. Snapshot host truth before this refresh owns it. + this.ensureRemoteDesktopHostReclaimTarget(ptyId) + } let changed = false for (const [subscriptionKey, viewer] of viewers) { if (viewer.clientId === clientId) { - viewers.set(subscriptionKey, { clientId, cols: viewport.cols, rows: viewport.rows }) + const activity = claim ? ++this.remoteDesktopActivity : viewer.activity + viewers.set(subscriptionKey, { + ...viewer, + cols: viewport.cols, + rows: viewport.rows, + activity + }) + if (claim) { + this.remoteDesktopOwners.set(ptyId, subscriptionKey) + } changed = true } } if (!changed) { return Promise.resolve(false) } - return this.applyRemoteDesktopLayout(ptyId) + this.bumpRemoteDesktopViewerRevision(ptyId) + return this.remoteDesktopOwners.has(ptyId) + ? this.applyRemoteDesktopLayout(ptyId) + : Promise.resolve(true) } async updateDesktopViewport( @@ -8412,9 +8587,34 @@ export class OrcaRuntimeService { // the terminal tab on the phone) must default to phone-fit again, not stay // in passive desktop-watch mode. this.setMobileDisplayMode(ptyId, 'auto') + if (this.hasRemoteDesktopLayoutState(ptyId)) { + return this.applyRemoteDesktopLayout(ptyId) + } return true } const heldOverride = this.terminalFitOverrides.get(ptyId) + if (heldOverride && this.hasRemoteDesktopLayoutState(ptyId)) { + const pending = this.pendingRestoreTimers.get(ptyId) + if (pending) { + clearTimeout(pending.timer) + this.pendingRestoreTimers.delete(ptyId) + } + const softLeaver = this.pendingSoftLeavers.get(ptyId) + if (softLeaver) { + clearTimeout(softLeaver.timer) + this.pendingSoftLeavers.delete(ptyId) + } + const priorDriver = this.getDriver(ptyId) + this.setDriver(ptyId, { kind: 'idle' }) + const converged = await this.applyRemoteDesktopLayout(ptyId) + if (!converged) { + this.setDriver(ptyId, priorDriver) + return false + } + this.setDriver(ptyId, { kind: 'desktop' }) + this.setMobileDisplayMode(ptyId, 'auto') + return true + } if (heldOverride) { const pending = this.pendingRestoreTimers.get(ptyId) if (pending) { @@ -8631,6 +8831,11 @@ export class OrcaRuntimeService { if (prev.kind === 'phone' && next.kind === 'phone') { return prev.ownerClientId === next.ownerClientId } + if (prev.kind === 'remote-desktop' && next.kind === 'remote-desktop') { + // Why: each owner's claim promise gates its following input. Sharing a + // waiter across owners could release A's input only after B's grid lands. + return prev.ownerSubscriptionKey === next.ownerSubscriptionKey + } return true } @@ -8768,9 +8973,9 @@ export class OrcaRuntimeService { this.resizeHeadlessTerminal(ptyId, target.cols, target.rows) } - // Why: emit fit-override-changed when the *mode* flips. Layouts can - // change dims without flipping mode (keyboard show/hide while phone), - // and waking the renderer on every viewport tick is wasteful churn. + // Why: remote desktop ownership is a fit hold for the host and passive + // peer viewers. Emit every remote layout so owner changes at equal geometry + // still park/release the correct clients without relying on resize deltas. // Defense-in-depth (#7588): also emit when the override's presence // changed even without a kind flip. applyLayout is the sole writer and // keeps override presence in lockstep with layout kind, so overrideChanged @@ -8778,7 +8983,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 (target.kind !== 'remote-desktop' && (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". @@ -8788,13 +8993,21 @@ export class OrcaRuntimeService { } this.notifier?.terminalFitOverrideChanged( ptyId, - target.kind === 'phone' ? 'mobile-fit' : 'desktop-fit', + target.kind === 'phone' + ? 'mobile-fit' + : target.kind === 'remote-desktop' + ? 'remote-desktop-fit' + : 'desktop-fit', target.cols, target.rows ) this.notifyFitOverrideListeners( ptyId, - target.kind === 'phone' ? 'mobile-fit' : 'desktop-fit', + target.kind === 'phone' + ? 'mobile-fit' + : target.kind === 'remote-desktop' + ? 'remote-desktop-fit' + : 'desktop-fit', target.cols, target.rows ) @@ -9169,7 +9382,7 @@ export class OrcaRuntimeService { if (this.isMobileSubscriberActive(ptyId)) { return } - if (this.hasRemoteDesktopViewers(ptyId)) { + if (this.hasRemoteDesktopLayoutState(ptyId)) { void this.applyRemoteDesktopLayout(ptyId) return } @@ -9308,6 +9521,11 @@ export class OrcaRuntimeService { if (activeOverride && activeOverride.cols === cols && activeOverride.rows === rows) { return } + // Why: a successful host resize supersedes any target retained after a + // failed viewer reclaim; a later viewer cycle must capture this new truth. + if (!this.hasRemoteDesktopViewers(ptyId)) { + this.remoteDesktopHostReclaimTargets.delete(ptyId) + } this.resizeHeadlessTerminal(ptyId, cols, rows) this.refreshRendererGeometry(ptyId, cols, rows) } @@ -9328,6 +9546,11 @@ export class OrcaRuntimeService { if (cols <= 0 || rows <= 0) { return } + // Why: a viewer may leave while phone-fit still owns the PTY. Keep its + // deferred host reclaim cache aligned with later trusted pane measurements. + if (this.remoteDesktopHostReclaimTargets.has(ptyId)) { + this.remoteDesktopHostReclaimTargets.set(ptyId, { cols, rows }) + } this.refreshRendererGeometry(ptyId, cols, rows) } diff --git a/src/main/runtime/remote-desktop-driver.test.ts b/src/main/runtime/remote-desktop-driver.test.ts index 7d2bac6855a..2a75853b1be 100644 --- a/src/main/runtime/remote-desktop-driver.test.ts +++ b/src/main/runtime/remote-desktop-driver.test.ts @@ -68,17 +68,23 @@ const store = { }) } -function createRuntime() { - const runtime = new OrcaRuntimeService(store) +function createRuntime(mobileAutoRestoreFitMs: number | null = 5_000) { + const runtime = new OrcaRuntimeService({ + ...store, + getSettings: () => ({ ...store.getSettings(), mobileAutoRestoreFitMs }) + }) const ptySizes = new Map([ ['pty-1', { cols: 150, rows: 40 }] ]) + const resizeCalls: { ptyId: string; cols: number; rows: number }[] = [] const driverEvents: { ptyId: string; driver: { kind: string; clientId?: string } }[] = [] + const fitOverrideEvents: { ptyId: string; mode: string; cols: number; rows: number }[] = [] runtime.setPtyController({ write: () => true, kill: () => true, getForegroundProcess: async () => null, resize: (ptyId, cols, rows) => { + resizeCalls.push({ ptyId, cols, rows }) ptySizes.set(ptyId, { cols, rows }) return true }, @@ -94,12 +100,19 @@ function createRuntime() { focusTerminal: vi.fn(), closeTerminal: vi.fn(), sleepWorktree: vi.fn(), - terminalFitOverrideChanged: vi.fn(), + terminalFitOverrideChanged: (ptyId, mode, cols, rows) => { + fitOverrideEvents.push({ ptyId, mode, cols, rows }) + }, terminalDriverChanged: (ptyId, driver) => { driverEvents.push({ ptyId, driver: { ...driver } }) } }) - return { runtime, driverEvents } + return { + runtime, + driverEvents, + fitOverrideEvents, + resizeCalls + } } describe('remote desktop viewer width driver', () => { @@ -120,21 +133,20 @@ describe('remote desktop viewer width driver', () => { expect(driverEvents).toHaveLength(0) }) - it('sizes the PTY to the SMALLEST attached viewer (smallest client wins)', async () => { + it('sizes the PTY to the latest active desktop viewer', 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. + // Activity on the narrower viewer transfers ownership to it. 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. + // Later activity on the wide viewer transfers ownership back. await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 140, 50) - expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 30 }) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 140, rows: 50 }) - // The narrow viewer leaves — the PTY re-fits to the survivor's width. + // A passive peer leaving cannot disturb the active owner. await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-B') expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 140, rows: 50 }) }) @@ -155,7 +167,7 @@ describe('remote desktop viewer width driver', () => { }) it('coexists with a mobile driver and outlives it (host stays suppressed)', async () => { - const { runtime } = createRuntime() + const { runtime, fitOverrideEvents } = createRuntime() await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) @@ -170,6 +182,16 @@ describe('remote desktop viewer width driver', () => { vi.advanceTimersByTime(10_000) expect(runtime.getDriver('pty-1').kind).not.toBe('mobile') expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 100, rows: 40 }) + expect(fitOverrideEvents.at(-1)).toMatchObject({ + ptyId: 'pty-1', + mode: 'remote-desktop-fit', + cols: 100, + rows: 40 + }) + + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 150, rows: 40 }) }) it('isPtyResizeDrivenRemotely is false for idle and desktop drivers', async () => { @@ -180,13 +202,52 @@ describe('remote desktop viewer width driver', () => { expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) }) + it('does not let passive attachment or hydration steal host ownership', async () => { + const { runtime, resizeCalls } = createRuntime() + + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 24, false) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 90, 30, false) + + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 150, rows: 40 }) + expect(resizeCalls).toHaveLength(0) + }) + + it('lets host activity automatically reclaim from a remote owner', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 24) + + await runtime.claimRemoteDesktopHost('pty-1', 132, 42) + + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 132, rows: 42 }) + expect(runtime.getRemoteDesktopFitHold('pty-1', 'sub-A')).toMatchObject({ + mode: 'remote-desktop-fit', + cols: 132, + rows: 42 + }) + }) + + it('does not emit resize churn for repeated activity from the current owner', async () => { + const { runtime, resizeCalls, fitOverrideEvents } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + resizeCalls.splice(0) + fitOverrideEvents.splice(0) + + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + + expect(resizeCalls).toHaveLength(0) + expect(fitOverrideEvents).toHaveLength(0) + }) + 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 }) + // While the viewer owns width, the blocked host fit records the host's + // latest reclaim geometry. + runtime.recordRemoteDesktopHostReclaimTarget('pty-1', 120, 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. @@ -194,6 +255,137 @@ describe('remote desktop viewer width driver', () => { expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 120, rows: 40 }) }) + it('does not retain a remote reclaim target when only a phone suppresses host resize', async () => { + const { runtime } = createRuntime() + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + + // pty:resize is suppressed for mobile too, but this measurement must not + // seed the separate remote-viewer cache when no desktop viewer exists. + runtime.recordRemoteDesktopHostReclaimTarget('pty-1', 120, 35) + runtime.onClientDisconnected('phone-A') + await vi.advanceTimersByTimeAsync(10_000) + + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 24) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 150, rows: 40 }) + }) + + it('restores and consumes the host target when the last viewer leaves during phone-fit', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + // The host pane can change after remote ownership ends while phone-fit + // remains active; its trusted measurement becomes the deferred target. + runtime.recordRendererGeometry('pty-1', 140, 38) + + runtime.onClientDisconnected('phone-A') + await vi.advanceTimersByTimeAsync(10_000) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 140, rows: 38 }) + + // A later viewer must capture the new host geometry, not reuse the prior + // session's already-consumed 140-column reclaim target. + runtime.onExternalPtyResize('pty-1', 130, 36) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 80, 24) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-B') + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 130, rows: 36 }) + }) + + it('preserves indefinite phone-fit after the last desktop viewer leaves', async () => { + const { runtime } = createRuntime(null) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + + runtime.onClientDisconnected('phone-A') + await vi.advanceTimersByTimeAsync(10_000) + + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 45, rows: 20 }) + expect(runtime.getTerminalFitOverride('pty-1')).toMatchObject({ mode: 'mobile-fit' }) + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 150, rows: 40 }) + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + }) + + it('does not let an older host reclaim consume a newer viewer cycle target', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 24) + + // Do not await the reclaim before the next viewer joins. The serialized + // host resize can finish after sub-B has established a newer viewer cycle. + const firstReclaim = runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + const secondAttach = runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 90, 30) + await Promise.all([firstReclaim, secondAttach]) + + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-B') + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 150, rows: 40 }) + }) + + it('does not coalesce queued claims from different desktop owners', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 80, 24, false) + const layoutQueues = Reflect.get(runtime, 'layoutQueues') as Map< + string, + { running: Promise; pending: { target: { ownerSubscriptionKey?: string } }[] } + > + layoutQueues.set('pty-1', { running: new Promise(() => {}), pending: [] }) + + void runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 90, 28) + void runtime.claimRemoteDesktopViewer('pty-1', 'sub-B') + + expect( + layoutQueues.get('pty-1')?.pending.map(({ target }) => target.ownerSubscriptionKey) + ).toEqual(['sub-A', 'sub-B']) + layoutQueues.delete('pty-1') + }) + + it('makes a host claim join a pending disconnect reclaim', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 24) + const layoutQueues = Reflect.get(runtime, 'layoutQueues') as Map< + string, + { running: Promise; pending: { waiters: unknown[] }[] } + > + layoutQueues.set('pty-1', { running: new Promise(() => {}), pending: [] }) + + void runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + void runtime.claimRemoteDesktopHost('pty-1', 150, 40) + + expect(layoutQueues.get('pty-1')?.pending).toHaveLength(1) + expect(layoutQueues.get('pty-1')?.pending[0]?.waiters).toHaveLength(2) + layoutQueues.delete('pty-1') + }) + + it('removes same-PTY viewer floors with one bounded reclaim', async () => { + const { runtime, resizeCalls } = createRuntime() + const subscriptionKeys: string[] = [] + for (let index = 0; index < 100; index += 1) { + const key = `sub-${index}` + subscriptionKeys.push(key) + await runtime.updateRemoteDesktopViewer('pty-1', key, `viewer-${index}`, 100, 30) + } + resizeCalls.splice(0) + + await runtime.unregisterRemoteDesktopViewers('pty-1', subscriptionKeys) + + expect(resizeCalls).toEqual([{ ptyId: 'pty-1', cols: 150, rows: 40 }]) + }) + + it('applies the latest viewer width when the host takes back from a phone', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 24) + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 24 }) + expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) + }) + it('refresh never creates a floor (one-shot viewport cannot leak host suppression)', async () => { const { runtime } = createRuntime() // A one-shot terminal.updateViewport (refresh) from a viewer with no stream @@ -223,6 +415,17 @@ describe('remote desktop viewer width driver', () => { expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(false) }) + it('captures host geometry when a passive stream is first claimed by terminal.send', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'stream:1', 'viewer-A', 100, 30, false) + + await runtime.refreshRemoteDesktopViewer('pty-1', 'viewer-A', 80, 24, true) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 24 }) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'stream:1') + + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 150, rows: 40 }) + }) + it('keeps the host reclaim target when the reclaim resize fails', async () => { const { runtime } = createRuntime() const ptySizes = new Map([ @@ -243,9 +446,10 @@ describe('remote desktop viewer width driver', () => { getSize: (ptyId) => ptySizes.get(ptyId) ?? null }) - // Host reports its own 120-wide geometry; a viewer drives the PTY to 80. - runtime.recordRemoteDesktopHostReclaimTarget('pty-1', 120, 40) + // A viewer drives the PTY to 80; while it owns width, the host reports its + // own 120-wide reclaim geometry. await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 40) + runtime.recordRemoteDesktopHostReclaimTarget('pty-1', 120, 40) expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 80, rows: 40 }) // The last viewer leaves but the reclaim resize fails: the PTY is stuck at @@ -262,6 +466,42 @@ describe('remote desktop viewer width driver', () => { expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 120, rows: 40 }) }) + it('supersedes a failed reclaim target when the host later resizes successfully', async () => { + const { runtime } = createRuntime() + const ptySizes = new Map([ + ['pty-1', { cols: 150, rows: 40 }] + ]) + let resizeSucceeds = true + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + resize: (ptyId, cols, rows) => { + if (!resizeSucceeds) { + return false + } + ptySizes.set(ptyId, { cols, rows }) + return true + }, + getSize: (ptyId) => ptySizes.get(ptyId) ?? null + }) + + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 80, 40) + runtime.recordRemoteDesktopHostReclaimTarget('pty-1', 120, 40) + resizeSucceeds = false + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-A') + + // pty:resize has already resized the provider before this runtime mirror + // hook runs; that successful host geometry supersedes the retained 120. + resizeSucceeds = true + ptySizes.set('pty-1', { cols: 140, rows: 42 }) + runtime.onExternalPtyResize('pty-1', 140, 42) + + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 80, 30) + await runtime.unregisterRemoteDesktopViewer('pty-1', 'sub-B') + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 140, rows: 42 }) + }) + it('PTY exit clears the remote-desktop registry', async () => { const { runtime } = createRuntime() await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index ccabeb47e57..4a96c445ff4 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -88,6 +88,8 @@ type TerminalMultiplexStream = { isMobile: boolean ackOutput: boolean ackInFlightBytes: number + supportsDesktopViewportClaims: boolean + desktopClaimTail: Promise // 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. @@ -644,24 +646,31 @@ async function updateViewportForClient( // Why: the one-shot `terminal.updateViewport` RPC has no disconnect hook, so // it must only refresh a floor the client already owns via its stream (never // create a leak-prone standalone one). Stream paths that own cleanup register. - registration: 'register' | 'refresh' = 'register' + registration: 'register' | 'refresh' = 'register', + claim = false ): Promise<{ updated: boolean; applied: boolean }> { const type = client.type ?? defaultType if (type === 'mobile') { return runtime.updateMobileViewport(ptyId, client.id, 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. + // Why: stream attachment observes geometry without taking control. Only a + // later activity/claim frame may make this desktop authoritative. const updated = registration === 'refresh' - ? await runtime.refreshRemoteDesktopViewer(ptyId, client.id, viewport.cols, viewport.rows) + ? await runtime.refreshRemoteDesktopViewer( + ptyId, + client.id, + viewport.cols, + viewport.rows, + claim + ) : await runtime.updateRemoteDesktopViewer( ptyId, subscriptionKey, client.id, viewport.cols, - viewport.rows + viewport.rows, + claim ) return { updated, applied: updated } } @@ -732,7 +741,14 @@ const TerminalSend = TerminalHandle.extend({ id: requiredString('Missing client ID'), type: z.enum(['mobile', 'desktop']).default('desktop').optional() }) - .optional() + .optional(), + viewport: z + .object({ + cols: z.number().int().min(1).max(1000), + rows: z.number().int().min(1).max(500) + }) + .optional(), + claimViewport: z.literal(true).optional() }) const TerminalViewport = z.object({ @@ -829,7 +845,8 @@ const TerminalSubscribe = TerminalHandle.extend({ viewport: TerminalViewport.optional(), capabilities: z .object({ - terminalBinaryStream: z.literal(1).optional() + terminalBinaryStream: z.literal(1).optional(), + desktopViewportClaims: z.literal(1).optional() }) .optional() }) @@ -847,7 +864,8 @@ const TerminalMultiplexSubscribeFrame = TerminalHandle.extend({ viewport: TerminalViewport.optional(), capabilities: z .object({ - ackOutput: z.literal(1).optional() + ackOutput: z.literal(1).optional(), + desktopViewportClaims: z.literal(1).optional() }) .optional() }) @@ -918,7 +936,8 @@ const TerminalUpdateViewport = TerminalHandle.extend({ viewport: z.object({ cols: z.number().int().min(20).max(240), rows: z.number().int().min(8).max(120) - }) + }), + claim: z.boolean().optional() }) // Why: phone-fit auto-restore preference (docs/mobile-fit-hold.md). `null` @@ -1022,6 +1041,34 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } } } + if ( + leaf?.ptyId && + params.client?.type === 'desktop' && + params.claimViewport === true && + params.viewport + ) { + const claim = await updateViewportForClient( + runtime, + leaf.ptyId, + `send:${params.client.id}`, + params.client, + params.viewport, + 'desktop', + 'refresh', + true + ) + // Why: a stream-less request has no lifecycle cleanup and cannot safely + // create ownership. Never write at stale geometry if no stream exists. + if (!claim.updated || isTerminalInputLockedForClient(runtime, leaf.ptyId, params.client)) { + return { + send: { + handle: params.terminal, + accepted: false, + bytesWritten: 0 + } + } + } + } const hasText = typeof params.text === 'string' && params.text.length > 0 const hasSuffix = params.enter === true || params.interrupt === true if (params.requireAgentStatus === 'sendable' && hasText && hasSuffix) { @@ -1309,7 +1356,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ 'mobile', // Why: one-shot RPC with no disconnect hook — refresh the client's // existing stream-owned floor only, never create a leak-prone one. - 'refresh' + 'refresh', + params.claim === true ) return { ...viewportUpdate, seq: runtime.getLayout(leaf.ptyId)?.seq } } @@ -1508,7 +1556,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ ackTotalInFlightBytes = Math.max(0, ackTotalInFlightBytes - acknowledged) flushAllAckPendingOutput() } - const detachStream = (streamId: number, emitEnd: boolean): void => { + const detachStream = ( + streamId: number, + emitEnd: boolean, + releaseRemoteDesktopDriver = true + ): void => { const stream = streams.get(streamId) if (!stream) { return @@ -1533,7 +1585,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) { + } else if ( + releaseRemoteDesktopDriver && + 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). @@ -1548,8 +1604,20 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ return } closed = true + const remoteDesktopKeysByPty = new Map() for (const streamId of Array.from(streams.keys())) { - detachStream(streamId, false) + const stream = streams.get(streamId) + if (stream?.registeredRemoteDesktopDriver && !stream.isMobile && stream.client?.id) { + const keys = remoteDesktopKeysByPty.get(stream.ptyId) ?? [] + keys.push(stream.remoteDesktopSubscriptionKey) + remoteDesktopKeysByPty.set(stream.ptyId, keys) + } + detachStream(streamId, false, false) + } + // Why: one connection can own many panes backed by the same PTY. + // Remove those floors together so close scans each PTY registry once. + for (const [ptyId, subscriptionKeys] of remoteDesktopKeysByPty) { + void runtime.unregisterRemoteDesktopViewers(ptyId, subscriptionKeys) } unregisterControlHandler() resolveMultiplex() @@ -1582,8 +1650,16 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (isTerminalInputLockedForClient(runtime, stream.ptyId, stream.client)) { return } - void runtime - .sendTerminal(stream.terminal, { text, enter: false, interrupt: false }) + void stream.desktopClaimTail + .then((claimed) => + !claimed || isTerminalInputLockedForClient(runtime, stream.ptyId, stream.client) + ? null + : runtime.sendTerminal(stream.terminal, { + text, + enter: false, + interrupt: false + }) + ) .then(async () => { if (stream.isMobile && stream.client?.id) { await runtime.mobileTookFloor(stream.ptyId, stream.client.id) @@ -1599,9 +1675,10 @@ 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. + const cols = viewport.cols + const rows = viewport.rows + // Why: resize registers stream-scoped geometry so detach can release + // it. Older clients lack explicit claims, so Resize remains control. if (!stream.isMobile && stream.client?.id) { stream.registeredRemoteDesktopDriver = true if (stream.buffering) { @@ -1609,14 +1686,61 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ return } } - void updateViewportForClient( - runtime, - stream.ptyId, - stream.remoteDesktopSubscriptionKey, - stream.client, - { cols: viewport.cols, rows: viewport.rows }, - stream.isMobile ? 'mobile' : 'desktop' - ).catch(() => {}) + stream.desktopClaimTail = stream.desktopClaimTail + .then(async (priorClaimed) => { + const result = await updateViewportForClient( + runtime, + stream.ptyId, + stream.remoteDesktopSubscriptionKey, + stream.client!, + { cols, rows }, + stream.isMobile ? 'mobile' : 'desktop', + 'register', + !stream.supportsDesktopViewportClaims + ) + return stream.supportsDesktopViewportClaims + ? priorClaimed && result.applied + : result.applied + }) + .catch(() => false) + return + } + if ( + frame.opcode === TerminalStreamOpcode.ClaimViewport && + stream.client && + !stream.isMobile + ) { + const viewport = decodeTerminalStreamJson<{ cols?: unknown; rows?: unknown }>( + frame.payload + ) + if (!viewport || typeof viewport.cols !== 'number' || typeof viewport.rows !== 'number') { + return + } + const cols = viewport.cols + const rows = viewport.rows + stream.registeredRemoteDesktopDriver = true + stream.desktopClaimTail = stream.desktopClaimTail + .then( + () => + runtime.updateRemoteDesktopViewer( + stream.ptyId, + stream.remoteDesktopSubscriptionKey, + stream.client!.id, + cols, + rows, + true + ), + () => + runtime.updateRemoteDesktopViewer( + stream.ptyId, + stream.remoteDesktopSubscriptionKey, + stream.client!.id, + cols, + rows, + true + ) + ) + .catch(() => false) return } if (frame.opcode === TerminalStreamOpcode.SnapshotRequest) { @@ -1729,7 +1853,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ stream.remoteDesktopSubscriptionKey, stream.client, viewport, - 'desktop' + 'desktop', + 'register', + !stream.supportsDesktopViewportClaims ).catch(() => {}) } } @@ -1793,6 +1919,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ isMobile, ackOutput: request.capabilities?.ackOutput === 1, ackInFlightBytes: 0, + supportsDesktopViewportClaims: request.capabilities?.desktopViewportClaims === 1, + desktopClaimTail: Promise.resolve(true), registeredRemoteDesktopDriver: false, // Why: streamId is client-local, so two remote connections can both // use stream 1 for the same PTY. Scope the width-floor key by @@ -1859,11 +1987,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (isMobile && request.client?.id) { await runtime.handleMobileSubscribe(ptyId, request.client.id, request.viewport) } 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. + // Why: subscribe records this stream's geometry and cleanup key, + // but does not claim ownership. Activity frames claim later. stream.registeredRemoteDesktopDriver = true stream.pendingRemoteDesktopViewport = request.viewport } @@ -1881,7 +2006,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ stream.remoteDesktopSubscriptionKey, request.client, viewport, - 'desktop' + 'desktop', + 'register', + !stream.supportsDesktopViewportClaims ) } if (closed || streams.get(request.streamId) !== stream) { @@ -1890,10 +2017,15 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (!isMobile) { stream.unsubscribeFit = runtime.subscribeToFitOverrideChanges(ptyId, (event) => { + const mode = + event.mode === 'mobile-fit' + ? event.mode + : (runtime.getRemoteDesktopFitHold?.(ptyId, stream.remoteDesktopSubscriptionKey) + .mode ?? 'desktop-fit') emit({ type: 'fit-override-changed', streamId: request.streamId, - mode: event.mode, + mode, cols: event.cols, rows: event.rows }) @@ -1936,12 +2068,16 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ const snapshotOutputSeq = serialized?.seq if (!isMobile) { const fitOverride = runtime.getTerminalFitOverride(ptyId) + const desktopHold = runtime.getRemoteDesktopFitHold?.( + ptyId, + stream.remoteDesktopSubscriptionKey + ) ?? { mode: 'desktop-fit' as const, cols: size?.cols ?? 0, rows: size?.rows ?? 0 } emit({ type: 'fit-override-changed', streamId: request.streamId, - mode: fitOverride?.mode ?? 'desktop-fit', - cols: fitOverride?.cols ?? size?.cols ?? 0, - rows: fitOverride?.rows ?? size?.rows ?? 0 + mode: fitOverride?.mode ?? desktopHold.mode, + cols: fitOverride?.cols ?? desktopHold.cols, + rows: fitOverride?.rows ?? desktopHold.rows }) emit({ type: 'driver-changed', @@ -1993,24 +2129,6 @@ 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() const resizeGeneration = stream.resizeGeneration + 1 @@ -2059,6 +2177,27 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } sendResizedFrame(stream, event) }) + // Install the resize listener before draining the parked viewport; + // applyLayout emits synchronously and the stream must observe it. + 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', + 'register', + !stream.supportsDesktopViewportClaims + ).catch(() => {}) + } void runtime .waitForTerminal(request.terminal, { condition: 'exit', @@ -2151,43 +2290,86 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ const ptyId = leaf.ptyId const clientId = params.client?.id + const supportsDesktopViewportClaims = params.capabilities?.desktopViewportClaims === 1 // 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) - // Why: legacy JSON streams register cleanup after snapshot awaits; if - // the socket closed meanwhile, registering now would orphan listeners. - if (signal?.aborted) { - return - } - const size = runtime.getTerminalSize(ptyId) - const displayMode = runtime.getMobileDisplayMode(ptyId) - const seq = runtime.getLayout(ptyId)?.seq - emit({ - type: 'scrollback', - lines: read.tail, - truncated: isTerminalReadPayloadIncomplete(read), - serialized: serialized?.data, - oscLinks: serialized?.oscLinks, - cwd: serialized?.cwd, - cols: serialized?.cols ?? size?.cols, - rows: serialized?.rows ?? size?.rows, - displayMode, - seq - }) - // Why: desktop can have both a hidden automation watcher and a visible // pane subscribed to the same terminal. Key by client when provided so // one stream cannot evict the other. const subscriptionId = clientId ? `${params.terminal}:${clientId}` : params.terminal - await new Promise((resolve) => { - const outputBatcher = createTerminalOutputBatcher((chunk) => { + const remoteDesktopSubscriptionKey = `json:${nextTerminalStreamId++}` + let closed = false + let outputBatcher: ReturnType | null = null + let unsubscribeData = (): void => {} + let unsubscribeFit = (): void => {} + let resolveStream = (): void => {} + const streamClosed = new Promise((resolve) => { + resolveStream = resolve + }) + // Why: register before viewport/snapshot awaits so a socket close cannot + // orphan either the stream listeners or its remote-desktop width floor. + runtime.registerSubscriptionCleanup( + subscriptionId, + () => { + closed = true + outputBatcher?.flush() + outputBatcher?.dispose() + unsubscribeData() + unsubscribeFit() + if (registeredRemoteDesktopDriver && clientId) { + runtime.unregisterRemoteDesktopViewer(ptyId, remoteDesktopSubscriptionKey) + } + emit({ type: 'end' }) + resolveStream() + }, + connectionId + ) + try { + if (clientId && params.client && params.viewport) { + registeredRemoteDesktopDriver = true + await updateViewportForClient( + runtime, + ptyId, + remoteDesktopSubscriptionKey, + params.client, + params.viewport, + 'desktop', + 'register', + !supportsDesktopViewportClaims + ) + } + if (closed || signal?.aborted) { + runtime.cleanupSubscription(subscriptionId) + return + } + const read = await runtime.readTerminal(params.terminal) + const serialized = await serializeBudgetedMobileSnapshot(runtime, ptyId, false) + if (closed || signal?.aborted) { + runtime.cleanupSubscription(subscriptionId) + return + } + const size = runtime.getTerminalSize(ptyId) + const displayMode = runtime.getMobileDisplayMode(ptyId) + const seq = runtime.getLayout(ptyId)?.seq + emit({ + type: 'scrollback', + lines: read.tail, + truncated: isTerminalReadPayloadIncomplete(read), + serialized: serialized?.data, + oscLinks: serialized?.oscLinks, + cwd: serialized?.cwd, + cols: serialized?.cols ?? size?.cols, + rows: serialized?.rows ?? size?.rows, + displayMode, + seq + }) + outputBatcher = createTerminalOutputBatcher((chunk) => { emit({ type: 'data', chunk }) }) const unsubscribeStreamData = runtime.subscribeToTerminalData(ptyId, (data) => { - outputBatcher.push(data) + outputBatcher?.push(data) }) // Why: this legacy JSON stream can feed a live xterm view too // (older web/desktop subscribers), so it conservatively registers @@ -2195,38 +2377,35 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ // a withheld model reply — the pre-Phase-5 status quo — which is // strictly safer than a double reply under a view consumer. const releaseViewSubscriber = runtime.registerRemoteTerminalViewSubscriber(ptyId) - const unsubscribeData = (): void => { + unsubscribeData = () => { releaseViewSubscriber() unsubscribeStreamData() } - const unsubscribeFit = runtime.subscribeToFitOverrideChanges(ptyId, (event) => { - outputBatcher.flush() + unsubscribeFit = runtime.subscribeToFitOverrideChanges(ptyId, (event) => { + outputBatcher?.flush() + const mode = + event.mode === 'mobile-fit' + ? event.mode + : (runtime.getRemoteDesktopFitHold?.(ptyId, remoteDesktopSubscriptionKey).mode ?? + 'desktop-fit') emit({ type: 'fit-override-changed', - mode: event.mode, + mode, cols: event.cols, rows: event.rows }) }) - runtime.registerSubscriptionCleanup( - subscriptionId, - () => { - outputBatcher.flush() - outputBatcher.dispose() - unsubscribeData() - unsubscribeFit() - emit({ type: 'end' }) - resolve() - }, - connectionId - ) // Why: bind the exit-waiter to the connection dispatch signal so it is // removed on socket close/error instead of leaking until real exit. void runtime .waitForTerminal(params.terminal, { condition: 'exit', signal }) .then(() => runtime.cleanupSubscription(subscriptionId)) .catch(() => runtime.cleanupSubscription(subscriptionId)) - }) + await streamClosed + } catch (error) { + runtime.cleanupSubscription(subscriptionId) + throw error + } return } @@ -2241,6 +2420,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ let lastResizeCols: number | undefined let resizeGeneration = 0 let pendingOutput: TerminalOutputChunk[] = [] + let desktopClaimTail: Promise = Promise.resolve(true) let pendingOutputBytes = 0 let pendingOutputOverflowed = false let unsubscribeData = (): void => {} @@ -2317,8 +2497,16 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (isTerminalInputLockedForClient(runtime, ptyId, params.client)) { return } - void runtime - .sendTerminal(params.terminal, { text, enter: false, interrupt: false }) + void desktopClaimTail + .then((claimed) => + !claimed || isTerminalInputLockedForClient(runtime, ptyId, params.client) + ? null + : runtime.sendTerminal(params.terminal, { + text, + enter: false, + interrupt: false + }) + ) .then(async () => { if (isMobile && clientId) { await runtime.mobileTookFloor(ptyId, clientId) @@ -2338,6 +2526,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ ) { return } + const cols = viewport.cols + const rows = viewport.rows if (clientId) { registeredRemoteDesktopDriver = true if (buffering) { @@ -2345,14 +2535,66 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ return } } - void updateViewportForClient( - runtime, - ptyId, - remoteDesktopSubscriptionKey, - params.client, - { cols: viewport.cols, rows: viewport.rows }, - 'desktop' - ).catch(() => {}) + desktopClaimTail = desktopClaimTail + .then(async (priorClaimed) => { + const result = await updateViewportForClient( + runtime, + ptyId, + remoteDesktopSubscriptionKey, + params.client!, + { cols, rows }, + 'desktop', + 'register', + !supportsDesktopViewportClaims + ) + return supportsDesktopViewportClaims + ? priorClaimed && result.applied + : result.applied + }) + .catch(() => false) + return + } + if ( + frame.opcode === TerminalStreamOpcode.ClaimViewport && + params.client && + clientId && + !isMobile + ) { + const viewport = decodeTerminalStreamJson<{ cols?: unknown; rows?: unknown }>( + frame.payload + ) + if ( + !viewport || + typeof viewport.cols !== 'number' || + typeof viewport.rows !== 'number' + ) { + return + } + const cols = viewport.cols + const rows = viewport.rows + registeredRemoteDesktopDriver = true + desktopClaimTail = desktopClaimTail + .then( + () => + runtime.updateRemoteDesktopViewer( + ptyId, + remoteDesktopSubscriptionKey, + clientId, + cols, + rows, + true + ), + () => + runtime.updateRemoteDesktopViewer( + ptyId, + remoteDesktopSubscriptionKey, + clientId, + cols, + rows, + true + ) + ) + .catch(() => false) } }) ?? (() => {}) // Server-side auto-fit: resize PTY to phone dims before serializing scrollback @@ -2360,9 +2602,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ 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. + // Why: legacy subscribe records geometry without taking ownership; + // only an explicit activity/claim frame may suppress the host. registeredRemoteDesktopDriver = true pendingRemoteDesktopViewport = params.viewport } @@ -2414,7 +2655,9 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ remoteDesktopSubscriptionKey, params.client, viewport, - 'desktop' + 'desktop', + 'register', + !supportsDesktopViewportClaims ) } if (closed) { @@ -2551,24 +2794,6 @@ 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 rows: number @@ -2628,12 +2853,39 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ sendResizedFrame(event) }) + // Install the resize listener before draining the parked viewport; + // applyLayout emits synchronously and the stream must observe it. + if ( + clientId && + params.client && + registeredRemoteDesktopDriver && + pendingRemoteDesktopViewport + ) { + const viewport = pendingRemoteDesktopViewport + pendingRemoteDesktopViewport = null + void updateViewportForClient( + runtime, + ptyId, + remoteDesktopSubscriptionKey, + params.client, + viewport, + 'desktop', + 'register', + !supportsDesktopViewportClaims + ).catch(() => {}) + } + // Legacy fit-override-changed for non-mobile (desktop) subscribers unsubscribeFit = !isMobile ? runtime.subscribeToFitOverrideChanges(ptyId, (event) => { + const mode = + event.mode === 'mobile-fit' + ? event.mode + : (runtime.getRemoteDesktopFitHold?.(ptyId, remoteDesktopSubscriptionKey).mode ?? + 'desktop-fit') emit({ type: 'fit-override-changed', - mode: event.mode, + mode, cols: event.cols, rows: event.rows }) diff --git a/src/main/runtime/rpc/streaming.test.ts b/src/main/runtime/rpc/streaming.test.ts index 9cdc88a021b..77deacc420c 100644 --- a/src/main/runtime/rpc/streaming.test.ts +++ b/src/main/runtime/rpc/streaming.test.ts @@ -315,6 +315,9 @@ describe('RpcDispatcher streaming', () => { ) await vi.waitFor(() => expect(cleanups.has('terminal-1:desktop-1')).toBe(true)) + // Cleanup now registers before snapshot work so a disconnect cannot orphan + // a desktop width floor; wait for the actual exit waiter before resolving it. + await vi.waitFor(() => expect(runtime.waitForTerminal).toHaveBeenCalled()) resolveExit() await dispatchPromise diff --git a/src/main/runtime/rpc/terminal-multiplex.test.ts b/src/main/runtime/rpc/terminal-multiplex.test.ts index 7c702dba28f..474e90a4743 100644 --- a/src/main/runtime/rpc/terminal-multiplex.test.ts +++ b/src/main/runtime/rpc/terminal-multiplex.test.ts @@ -28,7 +28,10 @@ function stubRuntime(overrides: Partial = {}): OrcaRuntimeSe resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), updateRemoteDesktopViewer: vi.fn().mockResolvedValue(true), unregisterRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + unregisterRemoteDesktopViewers: vi.fn().mockResolvedValue(true), isPtyResizeDrivenRemotely: vi.fn().mockReturnValue(false), + getRemoteDesktopFitHold: vi.fn().mockReturnValue({ mode: 'desktop-fit', cols: 120, rows: 40 }), + isRemoteDesktopViewerOwner: vi.fn().mockReturnValue(false), ...overrides } as OrcaRuntimeService } @@ -122,7 +125,8 @@ describe('terminal multiplex RPC', () => { streamId: 5, terminal: 'terminal-1', client: { id: 'desktop-1', type: 'desktop' }, - viewport: { cols: 300, rows: 150 } + viewport: { cols: 300, rows: 150 }, + capabilities: { desktopViewportClaims: 1 } }) }) )! @@ -150,10 +154,129 @@ describe('terminal multiplex RPC', () => { 'multiplex:conn-1:5', 'desktop-1', 300, - 150 + 150, + false ) expect(handlers.has(5)).toBe(true) + let releaseClaim = (): void => {} + vi.mocked(runtime.updateRemoteDesktopViewer).mockImplementationOnce( + () => + new Promise((resolve) => { + releaseClaim = () => resolve(true) + }) + ) + + handlers.get(5)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.ClaimViewport, + streamId: 5, + seq: 0, + payload: encodeTerminalStreamJson({ cols: 96, rows: 32 }) + }) + )! + ) + await vi.waitFor(() => + expect(runtime.updateRemoteDesktopViewer).toHaveBeenLastCalledWith( + 'pty-1', + 'multiplex:conn-1:5', + 'desktop-1', + 96, + 32, + true + ) + ) + handlers.get(5)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Resize, + streamId: 5, + seq: 1, + payload: encodeTerminalStreamJson({ cols: 96, rows: 32 }) + }) + )! + ) + handlers.get(5)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Input, + streamId: 5, + seq: 2, + payload: encodeTerminalStreamText('ls\r') + }) + )! + ) + expect(runtime.sendTerminal).not.toHaveBeenCalled() + expect(runtime.updateRemoteDesktopViewer).not.toHaveBeenLastCalledWith( + 'pty-1', + 'multiplex:conn-1:5', + 'desktop-1', + 96, + 32, + false + ) + releaseClaim() + await vi.waitFor(() => + expect(runtime.updateRemoteDesktopViewer).toHaveBeenLastCalledWith( + 'pty-1', + 'multiplex:conn-1:5', + 'desktop-1', + 96, + 32, + false + ) + ) + await vi.waitFor(() => + expect(runtime.sendTerminal).toHaveBeenCalledWith('terminal-1', { + text: 'ls\r', + enter: false, + interrupt: false + }) + ) + const sentAfterSuccessfulClaim = vi.mocked(runtime.sendTerminal).mock.calls.length + vi.mocked(runtime.updateRemoteDesktopViewer).mockResolvedValueOnce(false) + for (const [opcode, seq, payload] of [ + [TerminalStreamOpcode.ClaimViewport, 3, encodeTerminalStreamJson({ cols: 88, rows: 28 })], + [TerminalStreamOpcode.Resize, 4, encodeTerminalStreamJson({ cols: 88, rows: 28 })], + [TerminalStreamOpcode.Input, 5, encodeTerminalStreamText('blocked')] + ] as const) { + handlers.get(5)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ opcode, streamId: 5, seq, payload }) + )! + ) + } + await vi.waitFor(() => + expect(runtime.updateRemoteDesktopViewer).toHaveBeenLastCalledWith( + 'pty-1', + 'multiplex:conn-1:5', + 'desktop-1', + 88, + 28, + false + ) + ) + expect(runtime.sendTerminal).toHaveBeenCalledTimes(sentAfterSuccessfulClaim) + for (const [opcode, seq, payload] of [ + [TerminalStreamOpcode.ClaimViewport, 6, encodeTerminalStreamJson({ cols: 88, rows: 28 })], + [TerminalStreamOpcode.Resize, 7, encodeTerminalStreamJson({ cols: 88, rows: 28 })], + [TerminalStreamOpcode.Input, 8, encodeTerminalStreamText('retry')] + ] as const) { + handlers.get(5)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ opcode, streamId: 5, seq, payload }) + )! + ) + } + await vi.waitFor(() => + expect(runtime.sendTerminal).toHaveBeenLastCalledWith('terminal-1', { + text: 'retry', + enter: false, + interrupt: false + }) + ) + dataListenerRef.current?.('a') dataListenerRef.current?.('b') await vi.runOnlyPendingTimersAsync() @@ -165,24 +288,6 @@ describe('terminal multiplex RPC', () => { expect(outputFrames[0]?.streamId).toBe(5) expect(outputFrames[0] ? decodeTerminalStreamText(outputFrames[0].payload) : '').toBe('ab') - handlers.get(5)?.( - decodeTerminalStreamFrame( - encodeTerminalStreamFrame({ - opcode: TerminalStreamOpcode.Input, - streamId: 5, - seq: 2, - payload: encodeTerminalStreamText('ls\r') - }) - )! - ) - await vi.waitFor(() => - expect(runtime.sendTerminal).toHaveBeenCalledWith('terminal-1', { - text: 'ls\r', - enter: false, - interrupt: false - }) - ) - handlers.get(5)?.( decodeTerminalStreamFrame( encodeTerminalStreamFrame({ @@ -199,7 +304,8 @@ describe('terminal multiplex RPC', () => { 'multiplex:conn-1:5', 'desktop-1', 100, - 30 + 30, + false ) ) @@ -256,8 +362,67 @@ describe('terminal multiplex RPC', () => { .join('') ).toBe('snapshot') + // A viewport-less stream is passive: it must neither register nor later + // release the active stream's width floor when the connection closes. + handlers.get(0)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Subscribe, + streamId: 0, + seq: 5, + payload: encodeTerminalStreamJson({ + streamId: 6, + terminal: 'terminal-1', + client: { id: 'desktop-1', type: 'desktop' } + }) + }) + )! + ) + await vi.waitFor(() => + expect( + messages.some( + (msg) => + JSON.parse(msg).result?.type === 'subscribed' && + JSON.parse(msg).result?.streamId === 6 + ) + ).toBe(true) + ) + + // A second active floor on the same PTY is released in the same batch, + // keeping connection teardown to one registry recomputation per PTY. + handlers.get(0)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Subscribe, + streamId: 0, + seq: 6, + payload: encodeTerminalStreamJson({ + streamId: 7, + terminal: 'terminal-1', + client: { id: 'desktop-2', type: 'desktop' }, + viewport: { cols: 90, rows: 30 } + }) + }) + )! + ) + await vi.waitFor(() => + expect( + messages.some( + (msg) => + JSON.parse(msg).result?.type === 'subscribed' && + JSON.parse(msg).result?.streamId === 7 + ) + ).toBe(true) + ) + runtime.cleanupSubscription('terminal-multiplex:conn-1') await dispatchPromise + expect(runtime.unregisterRemoteDesktopViewer).not.toHaveBeenCalled() + expect(runtime.unregisterRemoteDesktopViewers).toHaveBeenCalledTimes(1) + expect(runtime.unregisterRemoteDesktopViewers).toHaveBeenCalledWith('pty-1', [ + 'multiplex:conn-1:5', + 'multiplex:conn-1:7' + ]) } finally { vi.useRealTimers() } @@ -274,7 +439,7 @@ describe('terminal multiplex RPC', () => { >() const cleanups = new Map void>() const runtime = stubRuntime({ - resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), serializeTerminalBuffer: vi .fn() @@ -306,7 +471,9 @@ describe('terminal multiplex RPC', () => { (msg) => messages.push(msg), { connectionId: 'conn-snap', - sendBinary: (bytes) => binaryFrames.push(bytes), + sendBinary: (bytes) => { + binaryFrames.push(bytes) + }, registerBinaryStreamHandler: (streamId, handler) => { handlers.set(streamId, handler) return () => handlers.delete(streamId) @@ -327,7 +494,8 @@ describe('terminal multiplex RPC', () => { streamId: 9, terminal: 'terminal-1', client: { id: 'desktop-1', type: 'desktop' }, - viewport: { cols: 300, rows: 150 } + viewport: { cols: 300, rows: 150 }, + capabilities: { desktopViewportClaims: 1 } }) }) )! @@ -371,7 +539,8 @@ describe('terminal multiplex RPC', () => { 'multiplex:conn-snap:9', 'desktop-1', 88, - 33 + 33, + false ) ) @@ -382,6 +551,129 @@ describe('terminal multiplex RPC', () => { } }) + it('emits a resize drained after the initial snapshot', async () => { + const messages: string[] = [] + const binaryFrames: Uint8Array[] = [] + const handlers = new Map< + number, + (frame: NonNullable>) => void + >() + const cleanups = new Map void>() + let resolveSnapshot = (_value: { data: string; cols: number; rows: number }): void => {} + let resizeListener: + | ((event: { + cols: number + rows: number + displayMode: string + reason: string + seq?: number + }) => void) + | undefined + const updateRemoteDesktopViewer = vi.fn( + async (_ptyId: string, _key: string, _clientId: string, cols: number, rows: number) => { + if (updateRemoteDesktopViewer.mock.calls.length > 1) { + resizeListener?.({ cols, rows, displayMode: 'desktop', reason: 'apply-layout', seq: 2 }) + } + return true + } + ) + const runtime = stubRuntime({ + updateRemoteDesktopViewer, + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), + serializeTerminalBuffer: vi.fn( + () => + new Promise<{ data: string; cols: number; rows: number }>((resolve) => { + resolveSnapshot = resolve + }) + ), + getTerminalSize: vi.fn().mockReturnValue({ cols: 80, rows: 24 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + subscribeToTerminalData: vi.fn().mockReturnValue(vi.fn()), + subscribeToTerminalResize: vi.fn((_ptyId, listener) => { + resizeListener = listener + return vi.fn() + }), + subscribeToFitOverrideChanges: vi.fn().mockReturnValue(vi.fn()), + subscribeToDriverChanges: vi.fn().mockReturnValue(vi.fn()), + getTerminalFitOverride: vi.fn().mockReturnValue(null), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + registerSubscriptionCleanup: vi.fn((id: string, cleanup: () => void) => { + cleanups.set(id, cleanup) + }), + cleanupSubscription: vi.fn((id: string) => { + const cleanup = cleanups.get(id) + cleanups.delete(id) + cleanup?.() + }), + waitForTerminal: vi.fn(() => new Promise(() => {})), + sendTerminal: vi.fn().mockResolvedValue({ accepted: true }) + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + const dispatchPromise = dispatcher.dispatchStreaming( + makeRequest('terminal.multiplex', {}), + (msg) => messages.push(msg), + { + connectionId: 'conn-initial-resize', + sendBinary: (bytes) => { + binaryFrames.push(bytes) + }, + registerBinaryStreamHandler: (streamId, handler) => { + handlers.set(streamId, handler) + return () => handlers.delete(streamId) + } + } + ) + + await vi.waitFor(() => + expect(messages.some((msg) => JSON.parse(msg).result?.type === 'ready')).toBe(true) + ) + handlers.get(0)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Subscribe, + streamId: 0, + seq: 1, + payload: encodeTerminalStreamJson({ + streamId: 9, + terminal: 'terminal-1', + client: { id: 'desktop-1', type: 'desktop' }, + viewport: { cols: 80, rows: 24 } + }) + }) + )! + ) + await vi.waitFor(() => expect(runtime.serializeTerminalBuffer).toHaveBeenCalled()) + handlers.get(9)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Resize, + streamId: 9, + seq: 2, + payload: encodeTerminalStreamJson({ cols: 132, rows: 43 }) + }) + )! + ) + expect(updateRemoteDesktopViewer).toHaveBeenCalledTimes(1) + + resolveSnapshot({ data: 'snapshot', cols: 80, rows: 24 }) + await vi.waitFor(() => + expect( + binaryFrames.some( + (bytes) => decodeTerminalStreamFrame(bytes)?.opcode === TerminalStreamOpcode.Resized + ) + ).toBe(true) + ) + const opcodes = binaryFrames.map((bytes) => decodeTerminalStreamFrame(bytes)?.opcode) + expect(opcodes.indexOf(TerminalStreamOpcode.Resized)).toBeGreaterThan( + opcodes.indexOf(TerminalStreamOpcode.SnapshotEnd) + ) + + runtime.cleanupSubscription('terminal-multiplex:conn-initial-resize') + await dispatchPromise + }) + it('drops stale mobile resize re-stream completions for multiplex streams', async () => { const messages: string[] = [] const binaryFrames: Uint8Array[] = [] @@ -1839,6 +2131,58 @@ describe('terminal multiplex RPC', () => { await dispatchPromise }) + it('owns and releases a viewport floor for legacy JSON desktop streams', async () => { + const messages: string[] = [] + const cleanups = new Map void>() + const runtime = stubRuntime({ + resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), + serializeTerminalBuffer: vi.fn().mockResolvedValue(null), + getTerminalSize: vi.fn().mockReturnValue({ cols: 120, rows: 40 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + subscribeToTerminalData: vi.fn().mockReturnValue(vi.fn()), + subscribeToFitOverrideChanges: vi.fn().mockReturnValue(vi.fn()), + registerSubscriptionCleanup: vi.fn((id: string, cleanup: () => void) => { + cleanups.set(id, cleanup) + }), + cleanupSubscription: vi.fn((id: string) => { + const cleanup = cleanups.get(id) + cleanups.delete(id) + cleanup?.() + }), + waitForTerminal: vi.fn(() => new Promise(() => {})) + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const dispatchPromise = dispatcher.dispatchStreaming( + makeRequest('terminal.subscribe', { + terminal: 'terminal-1', + client: { id: 'desktop-json-1', type: 'desktop' }, + viewport: { cols: 88, rows: 30 } + }), + (msg) => messages.push(msg), + { connectionId: 'conn-json-1' } + ) + + await vi.waitFor(() => + expect(messages.some((msg) => JSON.parse(msg).result?.type === 'scrollback')).toBe(true) + ) + const subscriptionKey = vi.mocked(runtime.updateRemoteDesktopViewer).mock.calls[0]?.[1] + expect(runtime.updateRemoteDesktopViewer).toHaveBeenCalledWith( + 'pty-1', + expect.stringMatching(/^json:/), + 'desktop-json-1', + 88, + 30, + true + ) + + runtime.cleanupSubscription('terminal-1:desktop-json-1') + await dispatchPromise + expect(runtime.unregisterRemoteDesktopViewer).toHaveBeenCalledWith('pty-1', subscriptionKey) + }) + it('bounds oversized live output frames for subscribed binary streams', async () => { vi.useFakeTimers() try { diff --git a/src/main/runtime/rpc/terminal-send.test.ts b/src/main/runtime/rpc/terminal-send.test.ts index c426513c9f5..7443b95c7cd 100644 --- a/src/main/runtime/rpc/terminal-send.test.ts +++ b/src/main/runtime/rpc/terminal-send.test.ts @@ -132,6 +132,46 @@ describe('terminal send RPC', () => { expect(runtime.mobileTookFloor).not.toHaveBeenCalled() }) + it('awaits a desktop viewport claim before acknowledged input', async () => { + let releaseClaim = (): void => {} + const refreshRemoteDesktopViewer = vi.fn( + () => + new Promise((resolve) => { + releaseClaim = () => resolve(true) + }) + ) + const sendTerminal = vi.fn().mockResolvedValue({ + handle: 'terminal-1', + accepted: true, + bytesWritten: 1 + }) + const runtime = stubRuntime({ + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + refreshRemoteDesktopViewer, + sendTerminal + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const response = dispatcher.dispatch( + makeRequest('terminal.send', { + terminal: 'terminal-1', + text: '\x03', + client: { id: 'desktop-1', type: 'desktop' }, + viewport: { cols: 96, rows: 32 }, + claimViewport: true + }) + ) + + await vi.waitFor(() => + expect(refreshRemoteDesktopViewer).toHaveBeenCalledWith('pty-1', 'desktop-1', 96, 32, true) + ) + expect(sendTerminal).not.toHaveBeenCalled() + releaseClaim() + await expect(response).resolves.toMatchObject({ ok: true }) + expect(sendTerminal).toHaveBeenCalled() + }) + it('accepts legacy clientless mobile input when the current driver is mobile', async () => { const runtime = stubRuntime({ resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), diff --git a/src/main/runtime/rpc/terminal-subscribe-buffer.test.ts b/src/main/runtime/rpc/terminal-subscribe-buffer.test.ts index ba921c6b14e..1ca811754f3 100644 --- a/src/main/runtime/rpc/terminal-subscribe-buffer.test.ts +++ b/src/main/runtime/rpc/terminal-subscribe-buffer.test.ts @@ -214,7 +214,14 @@ describe('terminal subscribe buffering', () => { expect(await outcomePromise).toBe('settled') expect(runtime.subscribeToTerminalData).not.toHaveBeenCalled() expect(runtime.subscribeToFitOverrideChanges).not.toHaveBeenCalled() - expect(runtime.registerSubscriptionCleanup).not.toHaveBeenCalled() + // Cleanup must exist before snapshot work so an abort cannot orphan a + // desktop width floor, but the abort must consume it before listeners start. + expect(runtime.registerSubscriptionCleanup).toHaveBeenCalledWith( + 'terminal-1:desktop-1', + expect.any(Function), + 'conn-legacy-json' + ) + expect(runtime.cleanupSubscription).toHaveBeenCalledWith('terminal-1:desktop-1') expect(runtime.waitForTerminal).not.toHaveBeenCalled() expect(messages).toEqual([]) } finally { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 7c9f989be5a..9b91002fb78 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1253,6 +1253,7 @@ export type PreloadApi = { write: (id: string, data: string) => void writeAccepted: (id: string, data: string) => Promise resize: (id: string, cols: number, rows: number) => void + claimViewport: (id: string, cols: number, rows: number) => void reportGeometry: (id: string, cols: number, rows: number) => void signal: (id: string, signal: string) => void clearBuffer: (id: string) => void @@ -2907,7 +2908,7 @@ export type PreloadApi = { getStatus: () => Promise call: (args: { method: string; params?: unknown }) => Promise> getTerminalFitOverrides: () => Promise< - { ptyId: string; mode: 'mobile-fit'; cols: number; rows: number }[] + { ptyId: string; mode: 'mobile-fit' | 'remote-desktop-fit'; cols: number; rows: number }[] > getTerminalDrivers: () => Promise< { @@ -2926,7 +2927,7 @@ export type PreloadApi = { onTerminalFitOverrideChanged: ( callback: (event: { ptyId: string - mode: 'mobile-fit' | 'desktop-fit' + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' cols: number rows: number }) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 059e2aa4e90..837a87b9715 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -846,6 +846,9 @@ const api = { resize: (id: string, cols: number, rows: number): void => { ipcRenderer.send('pty:resize', { id, cols, rows }) }, + claimViewport: (id: string, cols: number, rows: number): void => { + ipcRenderer.send('pty:claimViewport', { id, cols, rows }) + }, /** Why: measurement-only sibling of resize. Fires when a desktop pane * container measures real geometry (e.g. previously hidden tab becomes @@ -3878,7 +3881,7 @@ const api = { call: (args: { method: string; params?: unknown }): Promise> => ipcRenderer.invoke('runtime:call', args), getTerminalFitOverrides: (): Promise< - { ptyId: string; mode: 'mobile-fit'; cols: number; rows: number }[] + { ptyId: string; mode: 'mobile-fit' | 'remote-desktop-fit'; cols: number; rows: number }[] > => ipcRenderer.invoke('runtime:getTerminalFitOverrides'), getTerminalDrivers: (): Promise< { @@ -3899,14 +3902,19 @@ const api = { onTerminalFitOverrideChanged: ( callback: (event: { ptyId: string - mode: 'mobile-fit' | 'desktop-fit' + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' cols: number rows: number }) => void ): (() => void) => { const listener = ( _event: Electron.IpcRendererEvent, - data: { ptyId: string; mode: 'mobile-fit' | 'desktop-fit'; cols: number; rows: number } + data: { + ptyId: string + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' + cols: number + rows: number + } ) => callback(data) ipcRenderer.on('runtime:terminalFitOverrideChanged', listener) return () => ipcRenderer.removeListener('runtime:terminalFitOverrideChanged', listener) diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 8c7f3cd1b03..a3d0286efac 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -88,10 +88,11 @@ import { connectPanePty } from './pty-connection' import { resolveTerminalLayoutActiveLeafId } from './terminal-layout-leaf-ids' import { shouldPreserveTerminalScrollbackBuffers } from '../../../../shared/workspace-session-terminal-buffers' import { - getAllOverrides, + getMobileFitOverridePtyIds, getFitOverrideForPty, onOverrideChange } from '@/lib/pane-manager/mobile-fit-overrides' +import { shouldShowMobileDriverOverlay } from './mobile-driver-overlay-visibility' import { getAllDrivers, getDriverForPty, @@ -441,7 +442,7 @@ export default function TerminalPane({ (paneId) => paneTransportsRef.current.get(paneId)?.getPtyId(), event.ptyId ) - if (event.mode === 'mobile-fit') { + if (event.mode === 'mobile-fit' || event.mode === 'remote-desktop-fit') { // Why: when mobile starts driving, the agent re-renders its output at // phone width and that phone-wrapped byte stream flows live into this // passive watcher's xterm. xterm must shrink to the phone dims now or @@ -2659,7 +2660,7 @@ export default function TerminalPane({ }, [getContextMenuLeafId, toggleNativeChatForLeaf]) const getMobileOwnedTerminalPtyIds = useCallback((): string[] => { - const ptyIds = new Set(getAllOverrides().keys()) + const ptyIds = new Set(getMobileFitOverridePtyIds()) for (const [ptyId, driver] of getAllDrivers()) { if (driver.kind === 'mobile') { ptyIds.add(ptyId) @@ -3183,9 +3184,9 @@ export default function TerminalPane({ // treatment and collapse-to-chip state; both branches share the // same local/remote desktop-restore route. const driver = getDriverForPty(ptyId) - const isMobileDriving = driver.kind === 'mobile' - const hasFitOverride = getFitOverrideForPty(ptyId) !== null - if (!isMobileDriving && !hasFitOverride) { + const fitMode = getFitOverrideForPty(ptyId)?.mode ?? null + const hasFitOverride = fitMode === 'mobile-fit' + if (!shouldShowMobileDriverOverlay(driver.kind, fitMode)) { return null } // Why: only the pane replaced by native chat should hide terminal-owned diff --git a/src/renderer/src/components/terminal-pane/mobile-driver-overlay-visibility.test.ts b/src/renderer/src/components/terminal-pane/mobile-driver-overlay-visibility.test.ts new file mode 100644 index 00000000000..36f8f0a4245 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/mobile-driver-overlay-visibility.test.ts @@ -0,0 +1,11 @@ +import { describe, expect, it } from 'vitest' +import { shouldShowMobileDriverOverlay } from './mobile-driver-overlay-visibility' + +describe('shouldShowMobileDriverOverlay', () => { + it('shows only mobile ownership and phone-fit holds', () => { + expect(shouldShowMobileDriverOverlay('mobile', null)).toBe(true) + expect(shouldShowMobileDriverOverlay('idle', 'mobile-fit')).toBe(true) + expect(shouldShowMobileDriverOverlay('idle', 'remote-desktop-fit')).toBe(false) + expect(shouldShowMobileDriverOverlay('desktop', 'desktop-fit')).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/mobile-driver-overlay-visibility.ts b/src/renderer/src/components/terminal-pane/mobile-driver-overlay-visibility.ts new file mode 100644 index 00000000000..82534030454 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/mobile-driver-overlay-visibility.ts @@ -0,0 +1,8 @@ +import type { FitHoldMode } from '@/lib/pane-manager/mobile-fit-overrides' + +export function shouldShowMobileDriverOverlay( + driverKind: 'idle' | 'desktop' | 'mobile', + fitMode: FitHoldMode | null +): boolean { + return driverKind === 'mobile' || fitMode === 'mobile-fit' +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 18fbf3e1f6a..90daadec930 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -242,6 +242,7 @@ type MockTransport = { sendInput: ReturnType sendInputImmediate: ReturnType sendInputAccepted?: ReturnType + claimViewport: ReturnType resize: ReturnType getPtyId: ReturnType getConnectionId: ReturnType @@ -383,6 +384,7 @@ function createMockTransport(initialPtyId: string | null = null): MockTransport return ptyId }), sendInput: vi.fn(() => true), + claimViewport: vi.fn(() => true), resize: vi.fn(() => true), getPtyId: vi.fn(() => ptyId), getConnectionId: vi.fn(() => null), @@ -4000,6 +4002,7 @@ describe('connectPanePty', () => { const manager = createManager(1) const replayingPanesRef = { current: new Map([[1, 1]]) } const deps = createDeps({ replayingPanesRef }) + const { setFitOverride } = await import('@/lib/pane-manager/mobile-fit-overrides') connectPanePty(pane as never, manager as never, deps as never) @@ -4010,11 +4013,19 @@ describe('connectPanePty', () => { // Simulate xterm emitting a DA1 auto-reply during replay parse. ;(onDataHandler as (data: string) => void)('\x1b[?1;2c') expect(transport.sendInput).not.toHaveBeenCalled() + expect(transport.claimViewport).not.toHaveBeenCalled() // Once replay completes (guard cleared), real keystrokes flow through. replayingPanesRef.current.delete(1) + setFitOverride('pty-live', 'remote-desktop-fit', 80, 24) ;(onDataHandler as (data: string) => void)('a') expect(transport.sendInput).toHaveBeenCalledWith('a') + expect(transport.claimViewport).toHaveBeenCalledTimes(1) + ;(onDataHandler as (data: string) => void)('b') + // The renderer stays parked until runtime convergence is acknowledged, so + // a second keystroke can retry a transient failed resize. + expect(transport.claimViewport).toHaveBeenCalledTimes(2) + setFitOverride('pty-live', 'desktop-fit', 0, 0) }) it('does not enumerate every worktree tab for ordinary input without Codex restart notices', async () => { @@ -11683,7 +11694,7 @@ describe('connectPanePty', () => { isVisibleRef.current = true resizeHandler({ cols: 122, rows: 42 }) - expect(transport.resize).toHaveBeenCalledWith(122, 42) + expect(transport.resize).toHaveBeenCalledWith(122, 42, { claim: true }) disposable.dispose() }) @@ -18228,7 +18239,7 @@ describe('connectPanePty', () => { await flushAsyncTicks() expect(window.api.pty.getSize).toHaveBeenCalledWith('pty-pane-2') - expect(transport.resize).toHaveBeenCalledWith(82, 40) + expect(transport.resize).toHaveBeenCalledWith(82, 40, { claim: true }) } finally { observer.restore() } @@ -18277,7 +18288,7 @@ describe('connectPanePty', () => { expect(pane.fitAddon.fit).toHaveBeenCalled() expect(window.api.pty.getSize).toHaveBeenCalledWith('pty-pane-2') - expect(transport.resize).toHaveBeenCalledWith(65, 63) + expect(transport.resize).toHaveBeenCalledWith(65, 63, { claim: true }) }) it('skips foreground grid drift repair while mobile owns the PTY without a fit override', async () => { @@ -18368,6 +18379,56 @@ describe('connectPanePty', () => { } }) + it('updates the claiming desktop xterm before forwarding an observed viewport claim', async () => { + const originalDocument = globalThis.document + ;(globalThis as { document?: Document }).document = { + visibilityState: 'visible', + hasFocus: vi.fn(() => true) + } as unknown as Document + globalThis.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => { + queueMicrotask(() => callback(0)) + return 1 + }) + const { setFitOverride } = await import('@/lib/pane-manager/mobile-fit-overrides') + const pane = createPane(2) + const observer = installObservedPane(pane) + try { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-pane-2') + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + let proposedGrid = { cols: 120, rows: 40 } + pane.fitAddon = { + ...pane.fitAddon, + proposeDimensions: vi.fn(() => proposedGrid) + } as never + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + observer.trigger() + await flushAsyncTicks() + setFitOverride('pty-pane-2', 'remote-desktop-fit', 80, 24) + proposedGrid = { cols: 70, rows: 30 } + vi.mocked(pane.terminal.resize).mockClear() + transport.resize.mockClear() + + observer.trigger() + await flushAsyncTicks() + + expect(pane.terminal.resize).toHaveBeenCalledWith(70, 30) + expect(transport.resize).toHaveBeenCalledTimes(1) + expect(transport.resize).toHaveBeenCalledWith(70, 30, { claim: true }) + } finally { + setFitOverride('pty-pane-2', 'desktop-fit', 0, 0) + observer.restore() + globalThis.document = originalDocument + } + }) + it('skips observed desktop reassertion while mobile owns the PTY without a fit override', async () => { const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state') const pane = createPane(2) @@ -18428,7 +18489,7 @@ describe('connectPanePty', () => { binding.noteVisibilityResume() await flushAsyncTicks() - expect(transport.resize).toHaveBeenCalledWith(120, 40) + expect(transport.resize).toHaveBeenCalledWith(120, 40, { claim: true }) }) it('queues re-asserted resizes while pane resize holds are active', async () => { @@ -18470,7 +18531,7 @@ describe('connectPanePty', () => { release.flush() expect(transport.resize).toHaveBeenCalledTimes(1) - expect(transport.resize).toHaveBeenCalledWith(120, 40) + expect(transport.resize).toHaveBeenCalledWith(120, 40, { claim: true }) } finally { globalThis.CustomEvent = originalCustomEvent } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 0578868f48b..ff7090ca53e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -46,6 +46,7 @@ import { requestStablePaneFit } from '@/lib/pane-manager/pane-fit-resize-observe import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' import { isPtyLocked } from '@/lib/pane-manager/mobile-driver-state' import { reconcilePtySizeAcrossFrames, type PtySizeReconcileHandle } from './pty-size-reconcile' +import { shouldClaimRemoteDesktopViewport } from './remote-desktop-viewport-claim' import { createPtySizeReassertion } from './pty-size-reassertion' import { isPaneReplaying, replayIntoTerminal, replayIntoTerminalAsync } from './replay-guard' import { @@ -981,6 +982,7 @@ export function connectPanePty( // flips, exit, dispose) run before/after it exists, so start with no-ops. let syncHiddenRendererPtyDelivery: () => void = () => {} let releaseHiddenRendererPtyDelivery: () => void = () => {} + let suppressViewportClaimTerminalResize = false // Why: idle callbacks are registered before the deferred PTY output plumbing // exists. Start with the shared scheduler, then switch to the PTY writer // below so hidden-tab resets keep backlog-recovery callbacks and byte order. @@ -3294,6 +3296,26 @@ export function connectPanePty( (data) => transport.sendInputImmediate(data) ) + const claimViewportForUserActivity = (): void => { + const currentPtyId = transport.getPtyId() + if (!currentPtyId || getFitOverrideForPty(currentPtyId)?.mode !== 'remote-desktop-fit') { + return + } + let proposed: { cols: number; rows: number } | undefined + try { + proposed = pane.fitAddon.proposeDimensions() + } catch { + proposed = undefined + } + const cols = proposed?.cols ?? pane.terminal.cols + const rows = proposed?.rows ?? pane.terminal.rows + if (cols > 0 && rows > 0) { + // Why: queuing a claim is not convergence. Keep the pane parked until the + // runtime confirms desktop-fit so a transient resize failure retries. + transport.claimViewport?.(cols, rows) + } + } + const onDataDisposable = pane.terminal.onData((data) => { // Why: xterm auto-replies to embedded query sequences (DA1, DECRQM, // OSC 10/11, focus, CPR) via onData. When we replay recorded PTY bytes @@ -3349,6 +3371,7 @@ export function connectPanePty( // excluded because those transports do not expose sendInputAccepted. const acknowledgedIntent = intent ?? inferIntentFromExactTerminalInput(data) if (acknowledgedIntent && transport.sendInputAccepted) { + claimViewportForUserActivity() if (acknowledgedIntent === 'ctrl-c') { // Why: the accepted-write callback is async; let the next command be // inferred if the user cancelled an oversized line and immediately typed. @@ -3374,6 +3397,7 @@ export function connectPanePty( return } if (intent) { + claimViewportForUserActivity() if (transport.sendInput(data)) { markAcceptedTerminalInputSent() observeAcceptedShellCommandInput(data) @@ -3382,6 +3406,7 @@ export function connectPanePty( clearPendingTerminalInputIntent() return } + claimViewportForUserActivity() if (transport.sendInput(data)) { markAcceptedTerminalInputSent() observeAcceptedShellCommandInput(data) @@ -3427,7 +3452,7 @@ export function connectPanePty( if (queuePanePtyResizeIfHeld(pane.container, cols, rows)) { return } - transport.resize(cols, rows) + transport.resize(cols, rows, { claim: true }) } const onHeldPtyResizeFlush = (event: Event): void => { @@ -3440,7 +3465,7 @@ export function connectPanePty( pane.container.addEventListener(PANE_PTY_RESIZE_HOLD_FLUSH_EVENT, onHeldPtyResizeFlush) const onResizeDisposable = pane.terminal.onResize(({ cols, rows }) => { - if (suppressSnapshotReplayPtyResize) { + if (suppressSnapshotReplayPtyResize || suppressViewportClaimTerminalResize) { return } forwardPtyResize(cols, rows) @@ -3526,14 +3551,41 @@ export function connectPanePty( // the PTY's applied size; mobile-fit panes only report desktop geometry so // the parked phone-sized PTY is not resized. See docs/mobile-fit-hold.md. let pendingGeometryReportRaf: number | null = null + let lastObservedDesktopGrid: { cols: number; rows: number } | null = null + const readPaneSize = (): { width: number; height: number } | null => { + if (typeof pane.container.getBoundingClientRect !== 'function') { + return null + } + const rect = pane.container.getBoundingClientRect() + return { width: rect.width, height: rect.height } + } + let lastObservedPaneSize = readPaneSize() + let pendingPaneGeometryChanged = false const handleObservedPaneGeometry = (): void => { pendingGeometryReportRaf = null + const paneGeometryChanged = pendingPaneGeometryChanged + pendingPaneGeometryChanged = false const currentPtyId = transport.getPtyId() if (!currentPtyId) { + // Why: ResizeObserver may deliver its initial measurement before the + // remote binding completes; retain that passive baseline for the first + // real focused resize instead of swallowing the user's first claim. + const proposed = readProposedTerminalGrid() + if (proposed) { + lastObservedDesktopGrid = proposed + } return } const fitOverride = getFitOverrideForPty(currentPtyId) if (!fitOverride) { + if (pane.terminal.cols > 0 && pane.terminal.rows > 0) { + // Why: record the local grid before a later remote hold parks xterm; + // the first real window/split resize can then claim immediately. + lastObservedDesktopGrid = { + cols: pane.terminal.cols, + rows: pane.terminal.rows + } + } if (shouldSuppressDesktopPtyResize()) { return } @@ -3551,6 +3603,33 @@ export function connectPanePty( if (!proposed || proposed.cols <= 0 || proposed.rows <= 0) { return } + const priorProposed = lastObservedDesktopGrid + lastObservedDesktopGrid = proposed + if (fitOverride.mode === 'remote-desktop-fit') { + if ( + shouldClaimRemoteDesktopViewport({ + holdMode: fitOverride.mode, + prior: priorProposed, + current: proposed, + paneGeometryChanged, + paneVisible: deps.isVisibleRef.current, + documentVisible: document.visibilityState !== 'hidden', + documentFocused: document.hasFocus() + }) + ) { + // Why: a focused, visible layout change is genuine activity; release + // the park and update xterm before claiming so the owner does not keep + // rendering the prior owner's stale grid. + suppressViewportClaimTerminalResize = true + try { + pane.terminal.resize(proposed.cols, proposed.rows) + } finally { + suppressViewportClaimTerminalResize = false + } + transport.resize(proposed.cols, proposed.rows, { claim: true }) + } + return + } if (isRemoteRuntimePtyId(currentPtyId)) { transport.resize(proposed.cols, proposed.rows) } else { @@ -3561,6 +3640,16 @@ export function connectPanePty( typeof ResizeObserver === 'undefined' ? null : new ResizeObserver(() => { + const paneSize = readPaneSize() + if ( + paneSize && + lastObservedPaneSize && + (paneSize.width !== lastObservedPaneSize.width || + paneSize.height !== lastObservedPaneSize.height) + ) { + pendingPaneGeometryChanged = true + } + lastObservedPaneSize = paneSize if (pendingGeometryReportRaf !== null) { return } diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 40892001963..7f0f4876eba 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -101,10 +101,17 @@ export type PtyTransport = { // (preserving order) and sends the reply immediately. sendInputImmediate: (data: string) => boolean sendInputAccepted?: (data: string) => Promise + claimViewport?: (cols: number, rows: number) => boolean resize: ( cols: number, rows: number, - meta?: { widthPx?: number; heightPx?: number; cellW?: number; cellH?: number } + meta?: { + widthPx?: number + heightPx?: number + cellW?: number + cellH?: number + claim?: boolean + } ) => boolean isConnected: () => boolean getPtyId: () => string | null diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 70a6071b0e6..60656f2907f 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -976,11 +976,24 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra } }), - resize(cols: number, rows: number): boolean { + claimViewport(cols: number, rows: number): boolean { if (!connected || !ptyId) { return false } - window.api.pty.resize(ptyId, cols, rows) + window.api.pty.claimViewport(ptyId, cols, rows) + return true + }, + + resize(cols: number, rows: number, meta): boolean { + if (!connected || !ptyId) { + return false + } + if (meta?.claim) { + window.api.pty.resize(ptyId, cols, rows) + window.api.pty.claimViewport(ptyId, cols, rows) + } else { + window.api.pty.resize(ptyId, cols, rows) + } return true }, diff --git a/src/renderer/src/components/terminal-pane/remote-desktop-viewport-claim.test.ts b/src/renderer/src/components/terminal-pane/remote-desktop-viewport-claim.test.ts new file mode 100644 index 00000000000..a135ca247e1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-desktop-viewport-claim.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest' +import { shouldClaimRemoteDesktopViewport } from './remote-desktop-viewport-claim' + +describe('shouldClaimRemoteDesktopViewport', () => { + it('requires a second, changed measurement from a focused visible pane', () => { + const current = { cols: 100, rows: 30 } + expect( + shouldClaimRemoteDesktopViewport({ + holdMode: 'remote-desktop-fit', + prior: null, + current, + paneGeometryChanged: false, + paneVisible: true, + documentVisible: true, + documentFocused: true + }) + ).toBe(false) + expect( + shouldClaimRemoteDesktopViewport({ + holdMode: 'remote-desktop-fit', + prior: { cols: 90, rows: 30 }, + current, + paneGeometryChanged: false, + paneVisible: true, + documentVisible: true, + documentFocused: true + }) + ).toBe(true) + }) + + it.each([ + { paneVisible: false, documentVisible: true, documentFocused: true }, + { paneVisible: true, documentVisible: false, documentFocused: true }, + { paneVisible: true, documentVisible: true, documentFocused: false } + ])('rejects passive background geometry: %o', (visibility) => { + expect( + shouldClaimRemoteDesktopViewport({ + holdMode: 'remote-desktop-fit', + prior: { cols: 90, rows: 30 }, + current: { cols: 100, rows: 30 }, + paneGeometryChanged: false, + ...visibility + }) + ).toBe(false) + }) + + it('accepts the first focused measurement after the observed pane box changed', () => { + expect( + shouldClaimRemoteDesktopViewport({ + holdMode: 'remote-desktop-fit', + prior: null, + current: { cols: 70, rows: 30 }, + paneGeometryChanged: true, + paneVisible: true, + documentVisible: true, + documentFocused: true + }) + ).toBe(true) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-desktop-viewport-claim.ts b/src/renderer/src/components/terminal-pane/remote-desktop-viewport-claim.ts new file mode 100644 index 00000000000..bb057d0b5b5 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-desktop-viewport-claim.ts @@ -0,0 +1,23 @@ +import type { FitHoldMode } from '@/lib/pane-manager/mobile-fit-overrides' + +type TerminalGrid = { cols: number; rows: number } + +export function shouldClaimRemoteDesktopViewport(args: { + holdMode: FitHoldMode + prior: TerminalGrid | null + current: TerminalGrid + paneGeometryChanged: boolean + paneVisible: boolean + documentVisible: boolean + documentFocused: boolean +}): boolean { + return Boolean( + args.holdMode === 'remote-desktop-fit' && + (args.paneGeometryChanged || + (args.prior && + (args.prior.cols !== args.current.cols || args.prior.rows !== args.current.rows))) && + args.paneVisible && + args.documentVisible && + args.documentFocused + ) +} diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 9e408822e65..aea5df9bb0b 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -38,6 +38,7 @@ describe('createRemoteRuntimePtyTransport', () => { terminal: string client: { id: string; type: string } viewport?: { cols: number; rows: number } + capabilities?: { desktopViewportClaims?: 1 } } { const frames = subscriptionSendBinary.mock.calls .map((call) => decodeTerminalStreamFrame(call[0])) @@ -51,6 +52,7 @@ describe('createRemoteRuntimePtyTransport', () => { terminal: string client: { id: string; type: string } viewport?: { cols: number; rows: number } + capabilities?: { desktopViewportClaims?: 1 } }>(frame.payload) if (!payload) { throw new Error('invalid terminal subscribe payload') @@ -166,6 +168,12 @@ describe('createRemoteRuntimePtyTransport', () => { expect(onError).not.toHaveBeenCalled() expect(transport.getPtyId()).toBe('remote:terminal-1') + await vi.waitFor(() => + expect(latestSubscribePayload().capabilities).toEqual({ + ackOutput: 1, + desktopViewportClaims: 1 + }) + ) expect(runtimeSubscribe).toHaveBeenCalledWith( expect.objectContaining({ selector: 'env-1', @@ -177,11 +185,78 @@ describe('createRemoteRuntimePtyTransport', () => { await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) expect(latestSubscribePayload()).toMatchObject({ terminal: 'terminal-1', - client: { id: 'desktop:tab-1:pane:1', type: 'desktop' }, + client: { id: expect.stringMatching(/^desktop:tab-1:pane:1:/), type: 'desktop' }, viewport: { cols: 120, rows: 40 } }) }) + it('parks passive peers when another remote desktop owns the grid', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { getFitOverrideForPty, setFitOverride } = + await import('@/lib/pane-manager/mobile-fit-overrides') + const transport = createRemoteRuntimePtyTransport('env-1', { worktreeId: 'wt-1' }) + await transport.connect({ url: '', cols: 120, rows: 40, callbacks: {} }) + const { streamId } = latestSubscribePayload() + const ptyId = transport.getPtyId() + expect(ptyId).not.toBeNull() + + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'fit-override-changed', + streamId, + mode: 'remote-desktop-fit', + cols: 96, + rows: 32 + } + }) + + expect(ptyId ? getFitOverrideForPty(ptyId) : null).toEqual({ + mode: 'remote-desktop-fit', + cols: 96, + rows: 32 + }) + if (ptyId) { + setFitOverride(ptyId, 'desktop-fit', 0, 0) + } + }) + + it('gives separate paired viewers of the same host pane distinct refresh identities', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const first = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + const second = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + first.attach({ existingPtyId: 'remote:terminal-1', cols: 80, rows: 24, callbacks: {} }) + second.attach({ existingPtyId: 'remote:terminal-1', cols: 120, rows: 40, callbacks: {} }) + + await vi.waitFor(() => { + const subscribeFrames = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + expect(subscribeFrames).toHaveLength(2) + const clientIds = subscribeFrames.map((frame) => { + const payload = frame + ? decodeTerminalStreamJson<{ client: { id: string } }>(frame.payload) + : null + return payload?.client.id + }) + expect(clientIds[0]).toMatch(/^desktop:tab-1:pane:1:/) + expect(clientIds[1]).toMatch(/^desktop:tab-1:pane:1:/) + expect(clientIds[0]).not.toBe(clientIds[1]) + }) + + first.destroy?.() + second.destroy?.() + }) + it('routes encoded restored terminal ids to their owning runtime environment', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const transport = createRemoteRuntimePtyTransport('env-2', { @@ -593,7 +668,9 @@ describe('createRemoteRuntimePtyTransport', () => { params: { terminal: 'terminal-new', text: 'x', - client: { id: 'desktop:tab-1:pane:1', type: 'desktop' } + client: { id: expect.stringMatching(/^desktop:tab-1:pane:1:/), type: 'desktop' }, + viewport: { cols: 80, rows: 24 }, + claimViewport: true }, timeoutMs: 15_000 }) @@ -1363,6 +1440,63 @@ describe('createRemoteRuntimePtyTransport', () => { await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(2)) }) + it('releases pending claimed input when reconnect subscription fails', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onError = vi.fn() + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + await transport.connect({ url: '', callbacks: { onError } }) + let rejectReconnect = (_error: Error): void => {} + runtimeSubscribe.mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectReconnect = reject + }) + ) + + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(2)) + expect(transport.claimViewport?.(101, 33)).toBe(true) + const accepted = transport.sendInputAccepted?.('\x03') + await Promise.resolve() + rejectReconnect(new Error('reconnect failed')) + + await expect(accepted).resolves.toBe(false) + expect(onError).toHaveBeenCalledWith('reconnect failed') + }) + + it('retries when a replacement transport closes before its stream installs', async () => { + const transportCallbacks: NonNullable[] = [] + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: NonNullable) => { + transportCallbacks.push(callbacks) + subscriptionCallbacks = callbacks + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + const connected = transport.connect({ url: '', callbacks: {} }) + await vi.waitFor(() => expect(transportCallbacks).toHaveLength(1)) + transportCallbacks[0].onResponse({ ok: true, result: { type: 'ready' } }) + await connected + + transportCallbacks[0].onClose?.() + await vi.waitFor(() => expect(transportCallbacks).toHaveLength(2)) + transportCallbacks[1].onResponse({ ok: true, result: { type: 'ready' } }) + transportCallbacks[1].onClose?.() + + await vi.waitFor(() => expect(transportCallbacks).toHaveLength(3)) + transport.destroy?.() + }) + it('resubscribes with the latest pane viewport after the remote stream closes', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const transport = createRemoteRuntimePtyTransport('env-1', { @@ -1440,6 +1574,54 @@ describe('createRemoteRuntimePtyTransport', () => { transport.destroy?.() }) + it('replays a claim before input typed during the subscribe round-trip', async () => { + vi.useFakeTimers() + try { + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + subscriptionCallbacks = callbacks + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + transport.attach({ + existingPtyId: 'remote:terminal-1', + cols: 80, + rows: 24, + callbacks: {} + }) + await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalled()) + expect(transport.claimViewport?.(101, 33)).toBe(true) + expect(transport.sendInput('x')).toBe(true) + await vi.advanceTimersByTimeAsync(8) + expect(runtimeCall).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.send' }) + ) + + emitMultiplexReady() + await vi.waitFor(() => { + const opcodes = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])?.opcode) + .filter((opcode) => opcode !== undefined) + expect(opcodes).toEqual([ + TerminalStreamOpcode.Subscribe, + TerminalStreamOpcode.ClaimViewport, + TerminalStreamOpcode.Resize, + TerminalStreamOpcode.Input + ]) + }) + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) + it('coalesces rapid remote terminal input before sending it to the runtime', async () => { vi.useFakeTimers() try { @@ -1568,7 +1750,9 @@ describe('createRemoteRuntimePtyTransport', () => { params: { terminal: 'terminal-1', text: '\x03', - client: { id: 'desktop:tab-1:pane:1', type: 'desktop' } + client: { id: expect.stringMatching(/^desktop:tab-1:pane:1:/), type: 'desktop' }, + viewport: { cols: 80, rows: 24 }, + claimViewport: true }, timeoutMs: 15_000 }) @@ -1609,7 +1793,9 @@ describe('createRemoteRuntimePtyTransport', () => { params: { terminal: 'terminal-1', text: 'a\x03', - client: { id: 'desktop:tab-1:pane:1', type: 'desktop' } + client: { id: expect.stringMatching(/^desktop:tab-1:pane:1:/), type: 'desktop' }, + viewport: { cols: 80, rows: 24 }, + claimViewport: true }, timeoutMs: 15_000 }) @@ -1857,6 +2043,42 @@ describe('createRemoteRuntimePtyTransport', () => { } }) + it('sends an activity claim before the user input it sizes', async () => { + vi.useFakeTimers() + try { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + + await transport.connect({ url: '', callbacks: {} }) + const { streamId } = latestSubscribePayload() + subscriptionSendBinary.mockClear() + + expect(transport.claimViewport?.(101, 33)).toBe(true) + expect(transport.sendInput('x')).toBe(true) + await vi.runOnlyPendingTimersAsync() + + const frames = subscriptionSendBinary.mock.calls.map((call) => + decodeTerminalStreamFrame(call[0]) + ) + expect(frames.map((frame) => frame?.opcode)).toEqual([ + TerminalStreamOpcode.ClaimViewport, + TerminalStreamOpcode.Resize, + TerminalStreamOpcode.Input + ]) + expect(frames[0]?.streamId).toBe(streamId) + expect(frames[0] ? decodeTerminalStreamJson(frames[0].payload) : null).toEqual({ + cols: 101, + rows: 33 + }) + } finally { + vi.useRealTimers() + } + }) + it('replays remote scrollback through the parser without firing stale attention events', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const onReplayData = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index a29eb395703..2219b40199a 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -32,6 +32,7 @@ import { createRemoteRuntimePtyTextBatcher, createRemoteRuntimeViewportBatcher } from './remote-runtime-pty-batching' +import { createBrowserUuid } from '@/lib/browser-uuid' import { setFitOverride } from '@/lib/pane-manager/mobile-fit-overrides' import { setDriverForPty } from '@/lib/pane-manager/mobile-driver-state' import { isWebTerminalSurfaceTabId, toHostSessionTabId } from '@/runtime/web-terminal-surface-id' @@ -89,7 +90,22 @@ export function createRemoteRuntimePtyTransport( let desiredViewport: { cols: number; rows: number } | null = null let storedCallbacks: Parameters[0]['callbacks'] = {} let resubscribing = false - const clientId = `desktop:${tabId ?? 'tab'}:${leafId ?? 'leaf'}` + let resubscribeRequested = false + let subscriptionGeneration = 0 + let pendingViewportClaim = false + let pendingClaimInput = '' + const viewportClaimReadyWaiters = new Set<(ready: boolean) => void>() + const clearPendingViewportClaim = (): void => { + pendingViewportClaim = false + pendingClaimInput = '' + for (const resolve of viewportClaimReadyWaiters) { + resolve(false) + } + viewportClaimReadyWaiters.clear() + } + // Why: tab/leaf ids identify the mirrored host pane, so every paired viewer + // shares them. The instance suffix keeps one viewer's refresh off peer records. + const clientId = `desktop:${tabId ?? 'tab'}:${leafId ?? 'leaf'}:${createBrowserUuid()}` const outputProcessor = createPtyOutputProcessor({ onTitleChange, onBell, @@ -262,6 +278,14 @@ export function createRemoteRuntimePtyTransport( if (!connected || handle !== targetHandle) { return false } + if (pendingViewportClaim && !getCurrentMultiplexedStream(targetHandle)) { + const ready = await new Promise((resolve) => { + viewportClaimReadyWaiters.add(resolve) + }) + if (!ready || !connected || handle !== targetHandle) { + return false + } + } // Why: normal remote sendInput may be waiting on yielded size validation; // drain it before acknowledged writes so terminal bytes stay ordered. const text = `${inputBatcher.takePending()}${data}` @@ -283,7 +307,8 @@ export function createRemoteRuntimePtyTransport( const result = await callRuntime<{ send: RuntimeTerminalSend }>('terminal.send', { terminal: targetHandle, text: chunk, - client: { id: clientId, type: 'desktop' } + client: { id: clientId, type: 'desktop' }, + ...(desiredViewport ? { viewport: desiredViewport, claimViewport: true as const } : {}) }) if (result.send.accepted !== true) { return false @@ -303,30 +328,46 @@ export function createRemoteRuntimePtyTransport( if (!connected || !targetHandle) { return } - if (getCurrentMultiplexedStream(targetHandle)?.sendInput(text)) { + const stream = getCurrentMultiplexedStream(targetHandle) + if (stream?.sendInput(text)) { + return + } + if (pendingViewportClaim) { + // Why: a claim during subscribe/reconnect has no stream record to own + // yet. Hold its input until the stream can emit claim+input in one order. + pendingClaimInput += text return } void callRuntime('terminal.send', { terminal: targetHandle, text, - client: { id: clientId, type: 'desktop' } + client: { id: clientId, type: 'desktop' }, + ...(desiredViewport ? { viewport: desiredViewport, claimViewport: true as const } : {}) }).catch((error) => { handleRemoteTerminalError(error) }) }) - function sendViewportUpdate(cols: number, rows: number): void { + function sendViewportUpdate(cols: number, rows: number, claim = false): void { const targetHandle = handle if (!connected || !targetHandle) { return } - if (getCurrentMultiplexedStream(targetHandle)?.resize(cols, rows)) { + const stream = getCurrentMultiplexedStream(targetHandle) + if (claim ? stream?.claimViewport(cols, rows) : stream?.resize(cols, rows)) { + if (claim) { + pendingViewportClaim = false + } return } + if (claim) { + pendingViewportClaim = true + } void callRuntime('terminal.updateViewport', { terminal: targetHandle, client: { id: clientId, type: 'desktop' }, - viewport: { cols, rows } + viewport: { cols, rows }, + ...(claim ? { claim: true } : {}) }).catch(() => {}) } @@ -363,6 +404,7 @@ export function createRemoteRuntimePtyTransport( function retireRemoteTerminalId(): void { connected = false + clearPendingViewportClaim() const stalePtyId = remotePtyId handle = null remotePtyId = null @@ -414,18 +456,48 @@ export function createRemoteRuntimePtyTransport( await subscribeToHandle() } + function scheduleResubscribeAfterTransportClose(): void { + if (destroyed || !connected || !handle) { + return + } + if (resubscribing) { + resubscribeRequested = true + return + } + resubscribing = true + const resubscribeHandle = handle + void resubscribeAfterTransportClose(resubscribeHandle) + .catch((error) => { + if (!destroyed && connected && handle) { + clearPendingViewportClaim() + handleRemoteTerminalError(error) + } + }) + .finally(() => { + resubscribing = false + if (resubscribeRequested) { + resubscribeRequested = false + scheduleResubscribeAfterTransportClose() + } + }) + } + async function subscribeToHandle(): Promise { if (!handle) { return } const subscribedHandle = handle const subscribedPtyId = remotePtyId + const generation = ++subscriptionGeneration + let transportClosed = false // Why: the viewport we hand the subscribe request. A resize landing during // the round-trip falls back to the one-shot RPC, which is refresh-only (no - // leak) and no-ops before the stream floor exists — so replay the latest + // leak) and no-ops before the stream record exists — so replay the latest // remembered viewport through the stream once it's current (below). const subscribedViewport = desiredViewport const isCurrentSubscription = (): boolean => + !transportClosed && + generation === subscriptionGeneration && isCurrentRemoteTerminal(subscribedHandle, subscribedPtyId) const nextStream = await getRemoteRuntimeTerminalMultiplexer( currentRuntimeEnvironmentId @@ -491,29 +563,30 @@ export function createRemoteRuntimePtyTransport( } }, onTransportClose: () => { - if (!isCurrentSubscription()) { + transportClosed = true + if (generation !== subscriptionGeneration) { return } + if (!isCurrentSubscription()) { + // isCurrentSubscription excludes the just-closed stream by design. + if (!isCurrentRemoteTerminal(subscribedHandle, subscribedPtyId)) { + return + } + } multiplexedStream = null multiplexedStreamHandle = null - if (destroyed || !connected || !handle || resubscribing) { - return - } - resubscribing = true - const resubscribeHandle = handle - void resubscribeAfterTransportClose(resubscribeHandle) - .catch((error) => { - if (!destroyed && connected && handle) { - handleRemoteTerminalError(error) - } - }) - .finally(() => { - resubscribing = false - }) + scheduleResubscribeAfterTransportClose() } } }) - if (destroyed || !connected || handle !== subscribedHandle || remotePtyId !== subscribedPtyId) { + if ( + transportClosed || + generation !== subscriptionGeneration || + destroyed || + !connected || + handle !== subscribedHandle || + remotePtyId !== subscribedPtyId + ) { nextStream.close() return } @@ -521,10 +594,22 @@ export function createRemoteRuntimePtyTransport( multiplexedStream = nextStream multiplexedStreamHandle = subscribedHandle // Why: a viewport change that landed during the subscribe round-trip took - // the now-no-op one-shot fallback, so the stream floor is still at the + // the now-no-op one-shot fallback, so the stream record is still at the // subscribe-time size. Replay the latest remembered viewport so the PTY // tracks the current width instead of stalling until the next resize. - if ( + if (pendingViewportClaim && desiredViewport) { + nextStream.claimViewport(desiredViewport.cols, desiredViewport.rows) + pendingViewportClaim = false + const queuedInput = pendingClaimInput + pendingClaimInput = '' + if (queuedInput) { + nextStream.sendInput(queuedInput) + } + for (const resolve of viewportClaimReadyWaiters) { + resolve(true) + } + viewportClaimReadyWaiters.clear() + } else if ( desiredViewport && (desiredViewport.cols !== subscribedViewport?.cols || desiredViewport.rows !== subscribedViewport?.rows) @@ -634,6 +719,7 @@ export function createRemoteRuntimePtyTransport( if (handle === targetHandle && multiplexedStreamHandle !== targetHandle) { closeMultiplexedStream() } + clearPendingViewportClaim() handleRemoteTerminalError(error) }) }, @@ -647,6 +733,7 @@ export function createRemoteRuntimePtyTransport( return } connected = false + clearPendingViewportClaim() const id = remotePtyId closeMultiplexedStream() handle = null @@ -663,6 +750,7 @@ export function createRemoteRuntimePtyTransport( viewportBatcher.flush() outputProcessor.clearAccumulatedState() connected = false + clearPendingViewportClaim() closeMultiplexedStream() storedCallbacks = {} }, @@ -712,10 +800,15 @@ export function createRemoteRuntimePtyTransport( if (stream?.sendInput(text)) { return true } + if (pendingViewportClaim) { + pendingClaimInput += text + return true + } void callRuntime('terminal.send', { terminal: targetHandle, text, - client: { id: clientId, type: 'desktop' } + client: { id: clientId, type: 'desktop' }, + ...(desiredViewport ? { viewport: desiredViewport, claimViewport: true as const } : {}) }).catch((error) => { handleRemoteTerminalError(error) }) @@ -724,11 +817,26 @@ export function createRemoteRuntimePtyTransport( sendInputAccepted: sendInputAcceptedToRuntime, - resize(cols: number, rows: number): boolean { + claimViewport(cols: number, rows: number): boolean { if (!connected || !handle) { return false } rememberViewport(cols, rows) + viewportBatcher.clear() + sendViewportUpdate(cols, rows, true) + return true + }, + + resize(cols: number, rows: number, meta): boolean { + if (!connected || !handle) { + return false + } + rememberViewport(cols, rows) + if (meta?.claim) { + viewportBatcher.clear() + sendViewportUpdate(cols, rows, true) + return true + } // Why: xterm fit can emit resize bursts while the user drags panes or // restores layouts. Remote runtimes only need the last viewport in a frame. viewportBatcher.queue(cols, rows) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index d65f73c1fab..aadcc09e230 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -3228,7 +3228,7 @@ export function useIpcEvents(): void { kind: 'fit' event: { ptyId: string - mode: 'mobile-fit' | 'desktop-fit' + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' cols: number rows: number } diff --git a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts index 69dee772077..9d264c4eacc 100644 --- a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts +++ b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.test.ts @@ -8,7 +8,8 @@ import { getPaneIdsForPty, onOverrideChange, hydrateOverrides, - getAllOverrides + getAllOverrides, + getMobileFitOverridePtyIds } from './mobile-fit-overrides' afterEach(() => { @@ -36,6 +37,18 @@ describe('setFitOverride / getFitOverrideForPty', () => { expect(override).toEqual({ mode: 'mobile-fit', cols: 49, rows: 20 }) }) + it('stores and releases a remote desktop fit hold', () => { + setFitOverride('pty-remote', 'remote-desktop-fit', 96, 32) + expect(getFitOverrideForPty('pty-remote')).toEqual({ + mode: 'remote-desktop-fit', + cols: 96, + rows: 32 + }) + + setFitOverride('pty-remote', 'desktop-fit', 120, 40) + expect(getFitOverrideForPty('pty-remote')).toBeNull() + }) + it('removes the override when mode is desktop-fit', () => { setFitOverride('pty-1', 'mobile-fit', 49, 20) setFitOverride('pty-1', 'desktop-fit', 120, 40) @@ -323,6 +336,13 @@ describe('getAllOverrides', () => { all.delete('pty-1') expect(getFitOverrideForPty('pty-1')).not.toBeNull() }) + + it('excludes remote desktop holds from mobile bulk restore', () => { + setFitOverride('pty-mobile', 'mobile-fit', 49, 20) + setFitOverride('pty-remote', 'remote-desktop-fit', 100, 30) + + expect(getMobileFitOverridePtyIds()).toEqual(['pty-mobile']) + }) }) // --------------------------------------------------------------------------- diff --git a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts index 3ab4937c4d2..fc287f45663 100644 --- a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts +++ b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts @@ -1,10 +1,10 @@ -// Why: mobile-fit overrides are runtime-owned state that the renderer must -// respect. When a mobile client resizes a PTY to phone dimensions, the desktop -// renderer must not auto-fit that PTY back to desktop size. This module stores -// the override state and provides lookup for safeFit() and transport.resize(). +// Why: a phone or another desktop may own the shared PTY grid. Non-owners park +// xterm at that authoritative size so passive fitting cannot start a resize war. + +export type FitHoldMode = 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' type FitOverride = { - mode: 'mobile-fit' + mode: 'mobile-fit' | 'remote-desktop-fit' cols: number rows: number } @@ -24,7 +24,7 @@ function fitBindingKey(tabId: string, paneId: number): string { // and trigger safeFit on affected panes. type OverrideChangeEvent = { ptyId: string - mode: 'mobile-fit' | 'desktop-fit' + mode: FitHoldMode cols: number rows: number // Why: the dimensions the PTY was at *before* this event fired. For a @@ -49,14 +49,9 @@ function notifyChange(event: OverrideChangeEvent): void { } } -export function setFitOverride( - ptyId: string, - mode: 'mobile-fit' | 'desktop-fit', - cols: number, - rows: number -): void { +export function setFitOverride(ptyId: string, mode: FitHoldMode, cols: number, rows: number): void { const prior = overridesByPtyId.get(ptyId) ?? null - if (mode === 'mobile-fit') { + if (mode === 'mobile-fit' || mode === 'remote-desktop-fit') { overridesByPtyId.set(ptyId, { mode, cols, rows }) } else { overridesByPtyId.delete(ptyId) @@ -117,7 +112,12 @@ export function unbindPane(paneId: number, tabId?: string): void { } export function hydrateOverrides( - overrides: { ptyId: string; mode: 'mobile-fit'; cols: number; rows: number }[] + overrides: { + ptyId: string + mode: 'mobile-fit' | 'remote-desktop-fit' + cols: number + rows: number + }[] ): void { const previous = new Map(overridesByPtyId) overridesByPtyId.clear() @@ -131,7 +131,7 @@ export function hydrateOverrides( const prior = previous.get(ptyId) ?? null notifyChange({ ptyId, - mode: 'mobile-fit', + mode: override.mode, cols: override.cols, rows: override.rows, priorCols: prior?.cols ?? null, @@ -155,3 +155,9 @@ export function hydrateOverrides( export function getAllOverrides(): Map { return new Map(overridesByPtyId) } + +export function getMobileFitOverridePtyIds(): string[] { + return [...overridesByPtyId].flatMap(([ptyId, override]) => + override.mode === 'mobile-fit' ? [ptyId] : [] + ) +} diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts index c5951458020..abe054b80c1 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts @@ -252,6 +252,22 @@ describe('safeFit', () => { expect(pane.terminal.resize).toHaveBeenCalledWith(49, 20) }) + it('parks xterm at a remote desktop owner grid', () => { + const pane = createPane({ + proposedCols: 120, + proposedRows: 40, + terminalCols: 120, + terminalRows: 40 + }) + pane.container.dataset.ptyId = 'pty-remote' + setFitOverride('pty-remote', 'remote-desktop-fit', 96, 32) + + safeFit(pane) + + expect(pane.terminal.resize).toHaveBeenCalledWith(96, 32) + expect(pane.fitAddon.fit).not.toHaveBeenCalled() + }) + it('skips resize when terminal already matches override dimensions', () => { const pane = createPane({ proposedCols: 120, diff --git a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts index e854a5e7efc..7adf6b5dcf2 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts @@ -25,7 +25,7 @@ type TerminalMultiplexEvent = | { type: 'fit-override-changed' streamId: number - mode: 'mobile-fit' | 'desktop-fit' + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' cols: number rows: number } @@ -43,7 +43,7 @@ export type RemoteRuntimeMultiplexedTerminalCallbacks = { onEnd?: () => void onError?: (message: string) => void onFitOverrideChanged?: (event: { - mode: 'mobile-fit' | 'desktop-fit' + mode: 'mobile-fit' | 'remote-desktop-fit' | 'desktop-fit' cols: number rows: number }) => void @@ -57,6 +57,7 @@ export type RemoteRuntimeMultiplexedTerminal = { streamId: number sendInput: (text: string) => boolean resize: (cols: number, rows: number) => boolean + claimViewport: (cols: number, rows: number) => boolean serializeBuffer: (opts?: { scrollbackRows?: number }) => Promise<{ data: string cols: number @@ -247,6 +248,22 @@ class RemoteRuntimeTerminalMultiplexer { TerminalStreamOpcode.Resize, encodeTerminalStreamJson({ cols, rows }) ), + claimViewport: (cols, rows) => { + const claimed = this.sendFrame( + streamId, + TerminalStreamOpcode.ClaimViewport, + encodeTerminalStreamJson({ cols, rows }) + ) + // Why: older runtimes ignore the claim opcode but still understand + // Resize. Claim first keeps new-runtime ownership precise and leaves a + // backwards-compatible resize immediately behind it. + const resized = this.sendFrame( + streamId, + TerminalStreamOpcode.Resize, + encodeTerminalStreamJson({ cols, rows }) + ) + return claimed && resized + }, serializeBuffer: (opts) => this.requestSnapshot(state, opts), close: () => { if (this.streams.get(streamId) === state) { @@ -271,7 +288,8 @@ class RemoteRuntimeTerminalMultiplexer { terminal: args.terminal, client: args.client, viewport: args.viewport, - capabilities: args.client.type === 'desktop' ? { ackOutput: 1 } : undefined + capabilities: + args.client.type === 'desktop' ? { ackOutput: 1, desktopViewportClaims: 1 } : undefined }) ) if (!sent) { @@ -389,7 +407,9 @@ class RemoteRuntimeTerminalMultiplexer { ) } else if (event.type === 'fit-override-changed') { if ( - (event.mode !== 'mobile-fit' && event.mode !== 'desktop-fit') || + (event.mode !== 'mobile-fit' && + event.mode !== 'remote-desktop-fit' && + event.mode !== 'desktop-fit') || typeof event.cols !== 'number' || typeof event.rows !== 'number' ) { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 321940a15c8..5e7c61a12ff 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -2713,6 +2713,7 @@ function createPtyApi(): NonNullable['pty']> { write: () => {}, writeAccepted: () => Promise.resolve(false), resize: () => {}, + claimViewport: () => {}, reportGeometry: () => {}, signal: () => {}, // Web panes clear the host buffer via the terminal.clearBuffer runtime RPC. diff --git a/src/shared/terminal-stream-protocol.ts b/src/shared/terminal-stream-protocol.ts index 0011463babe..02314fb39e4 100644 --- a/src/shared/terminal-stream-protocol.ts +++ b/src/shared/terminal-stream-protocol.ts @@ -17,7 +17,10 @@ export enum TerminalStreamOpcode { Metadata = 12, // Why 13: Metadata=12 shipped to mobile clients in v1.4.120; Ack (branch-only // remote-multiplex flow control) renumbers to stay wire-compatible. - Ack = 13 + Ack = 13, + // Why 14: Ack already occupies 13 on current clients; older runtimes ignore + // this opcode and still receive the compatibility Resize frame behind it. + ClaimViewport = 14 } export type TerminalStreamFrame = { @@ -98,6 +101,7 @@ function isTerminalStreamOpcode(value: number): value is TerminalStreamOpcode { value === TerminalStreamOpcode.Unsubscribe || value === TerminalStreamOpcode.SnapshotRequest || value === TerminalStreamOpcode.Metadata || - value === TerminalStreamOpcode.Ack + value === TerminalStreamOpcode.Ack || + value === TerminalStreamOpcode.ClaimViewport ) }