mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
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.
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user