From 2fa54fdeb69d203b66f74b01fe493c65dbb5fdf3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:14:58 -0700 Subject: [PATCH] fix(terminal): make remote-host take-back release the phone-fit lock The remote-desktop branch of reclaimTerminalForDesktop was the one path that rolled the presence lock back when its reclaim resize did not converge. On a remote/SSH host that left the phone-fit banner stranded and made every subsequent "Take back all terminals" click a no-op. Its sibling (active mobile subscriber) released the lock but still reported the layout's `ok`, so the desktop renderer skipped its post-take-back refit and focus. Both now follow the guarantee the method already documents: an explicit desktop take-back always drops the lock, and the trailing remote layout is best-effort. --- src/main/runtime/orca-runtime.ts | 19 ++++--- .../runtime/remote-desktop-driver.test.ts | 49 ++++++++++++++++++- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 83d3c138b97..c8a48430c57 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -15941,7 +15941,10 @@ export class OrcaRuntimeService { // in passive desktop-watch mode. this.setMobileDisplayMode(ptyId, 'auto') if (this.hasRemoteDesktopLayoutState(ptyId)) { - return this.applyRemoteDesktopLayout(ptyId) + // Why: the lock is already released above, so this re-layout is + // best-effort. Reporting its `ok` would tell the desktop "nothing was + // reclaimed" and cost the caller its post-take-back refit and focus. + await this.applyRemoteDesktopLayout(ptyId) } return true } @@ -15957,14 +15960,14 @@ export class OrcaRuntimeService { clearTimeout(softLeaver.timer) this.pendingSoftLeavers.delete(ptyId) } - const priorDriver = this.getDriver(ptyId) + // Why: applyRemoteDesktopLayout no-ops while the driver still reads mobile. this.setDriver(ptyId, { kind: 'idle' }) - const converged = await this.applyRemoteDesktopLayout(ptyId) - if (!converged) { - this.setDriver(ptyId, priorDriver) - return false - } - this.setDriver(ptyId, { kind: 'desktop' }) + // Why: best-effort, like the local held branch below. A host whose resize + // keeps failing (dropped SSH/WSL provider, exited PTY) would otherwise + // roll the lock back and leave the banner stranded, making every retry a + // no-op — the one branch that broke this method's release guarantee. + await this.applyRemoteDesktopLayout(ptyId) + this.releaseDesktopTakeBack(ptyId) this.setMobileDisplayMode(ptyId, 'auto') return true } diff --git a/src/main/runtime/remote-desktop-driver.test.ts b/src/main/runtime/remote-desktop-driver.test.ts index 06eea32df00..355a65c49b8 100644 --- a/src/main/runtime/remote-desktop-driver.test.ts +++ b/src/main/runtime/remote-desktop-driver.test.ts @@ -84,11 +84,16 @@ function createRuntime(mobileAutoRestoreFitMs: number | null = 5_000) { 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 }[] = [] + let resizeSucceeds = true runtime.setPtyController({ write: () => true, kill: () => true, getForegroundProcess: async () => null, resize: (ptyId, cols, rows) => { + // Why: the production controller returns false when the provider throws — a dropped SSH/WSL provider or an exited PTY. + if (!resizeSucceeds) { + return false + } resizeCalls.push({ ptyId, cols, rows }) ptySizes.set(ptyId, { cols, rows }) return true @@ -116,7 +121,10 @@ function createRuntime(mobileAutoRestoreFitMs: number | null = 5_000) { runtime, driverEvents, fitOverrideEvents, - resizeCalls + resizeCalls, + setResizeSucceeds: (next: boolean) => { + resizeSucceeds = next + } } } @@ -519,4 +527,43 @@ describe('remote desktop viewer width driver', () => { await runtime.updateRemoteDesktopViewer('pty-1', 'sub-B', 'viewer-B', 100, 40) expect(runtime.isPtyResizeDrivenRemotely('pty-1')).toBe(true) }) + + it('held phone-fit take-back releases the lock when the remote reclaim resize fails', async () => { + // Why: the local held branch releases unconditionally (mobile-presence-lock.test.ts scenario 4). + // The remote branch rolled the lock back instead, so the banner survived and every retry was a no-op. + const { runtime, fitOverrideEvents, setResizeSucceeds } = 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.getTerminalFitOverride('pty-1')).toMatchObject({ mode: 'mobile-fit' }) + + setResizeSucceeds(false) + const notifierBefore = fitOverrideEvents.length + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + expect(runtime.getDriver('pty-1').kind).not.toBe('mobile') + expect(fitOverrideEvents.slice(notifierBefore).some((e) => e.mode === 'desktop-fit')).toBe(true) + // A second click must not be needed, and must stay idempotent. + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(false) + }) + + it('driving take-back reports success when the trailing remote layout cannot converge', async () => { + // Why: the lock is already released here, so returning false told the desktop renderer + // "nothing was reclaimed" and it skipped the post-take-back refit and focus. + const { runtime, setResizeSucceeds } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 30) + await runtime.handleMobileSubscribe('pty-1', 'phone-A', { cols: 45, rows: 20 }) + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'mobile', clientId: 'phone-A' }) + + setResizeSucceeds(false) + + await expect(runtime.reclaimTerminalForDesktop('pty-1')).resolves.toBe(true) + + expect(runtime.getDriver('pty-1')).toEqual({ kind: 'desktop' }) + expect(runtime.getTerminalFitOverride('pty-1')).toBeNull() + }) })