mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
fix(terminal): repaint the take-back release the presence lock was cancelling
The release gate was widened to shouldSuppressDesktopPtyResize(), which is `getFitOverrideForPty(ptyId) || isPtyLocked(ptyId)`. That pulls the presence lock into a decision the lock cannot answer. reclaimTerminalForDesktop's active-subscriber branch awaits applyMobileDisplayMode — which publishes the desktop-fit release — and only then calls releaseDesktopTakeBack, which flips the driver. Neither of the first two steps touches the driver and the second is awaited, so the renderer deterministically handles the release while its driver mirror still reads mobile: shouldSuppressDesktopPtyResize() is true, safeFitAndThen cancels via shouldContinue, and the desktop "Take back" gesture gets no fit, no SIGWINCH and no repaint. Cancellation is terminal, and that branch emits no second desktop-fit, so the later driver flip cannot recover it. The phone-disconnect path escapes only because it sets the driver synchronously first. A lock-only takeover cannot reach this listener anyway: it fires only on priorMode === 'mobile-fit', which requires a hold that is being released. Gate on the hold alone. It is also the live staleness check — a phone that retakes the grid before a parked refit lands reinstates the override and drops the repaint. The 0x0 gate, the real content of the same commit, is untouched. Tests: make the harness production-faithful (the driver reads mobile for the duration of a hold, as handleMobileSubscribe leaves it) and drive releases in the emission order the gesture produces. Replace the "lock-only takeover" premise, which was really an ordinary take-back and cemented the bug, with the reachable one: a parked refit dropped because the phone retook the grid, plus its positive control.
This commit is contained in:
+98
-19
@@ -16,6 +16,7 @@ import {
|
||||
restoreTerminalTestGlobals
|
||||
} from './pty-connection-test-environment'
|
||||
import type { setFitOverride as SetFitOverride } from '@/lib/pane-manager/mobile-fit-overrides'
|
||||
import type { setDriverForPty as SetDriverForPty } from '@/lib/pane-manager/mobile-driver-state'
|
||||
|
||||
const {
|
||||
resetAndRefreshAllTerminalWebglAtlases,
|
||||
@@ -123,6 +124,7 @@ async function connectPaneHoldingMobileFit(ptyId: string = PTY_ID): Promise<{
|
||||
pane: MockPane
|
||||
signalPty: ReturnType<typeof vi.fn>
|
||||
setFitOverride: typeof SetFitOverride
|
||||
setDriverForPty: typeof SetDriverForPty
|
||||
dispose: () => void
|
||||
}> {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
@@ -138,7 +140,10 @@ async function connectPaneHoldingMobileFit(ptyId: string = PTY_ID): Promise<{
|
||||
const disposable = connectPanePty(pane as never, manager as never, deps as never)
|
||||
await flushAsyncTicks(6)
|
||||
|
||||
// The phone takes the floor: the host parks the PTY and xterm at phone dims.
|
||||
// The phone takes the floor. Production order (handleMobileSubscribe): the
|
||||
// driver flips to mobile first, then the phone layout writes the hold.
|
||||
const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state')
|
||||
setDriverForPty(ptyId, { kind: 'mobile', clientId: 'phone-1' })
|
||||
setFitOverride(ptyId, 'mobile-fit', PHONE.cols, PHONE.rows)
|
||||
pane.terminal.cols = PHONE.cols
|
||||
pane.terminal.rows = PHONE.rows
|
||||
@@ -146,7 +151,7 @@ async function connectPaneHoldingMobileFit(ptyId: string = PTY_ID): Promise<{
|
||||
|
||||
const signalPty = window.api.pty.signal as unknown as ReturnType<typeof vi.fn>
|
||||
signalPty.mockClear()
|
||||
return { pane, signalPty, setFitOverride, dispose: () => disposable.dispose() }
|
||||
return { pane, signalPty, setFitOverride, setDriverForPty, dispose: () => disposable.dispose() }
|
||||
}
|
||||
|
||||
/** The phone already holds the grid before this pane mounts — an app reload
|
||||
@@ -155,10 +160,13 @@ async function connectPaneHoldingMobileFit(ptyId: string = PTY_ID): Promise<{
|
||||
async function connectPaneIntoExistingMobileFitHold(): Promise<{
|
||||
signalPty: ReturnType<typeof vi.fn>
|
||||
setFitOverride: typeof SetFitOverride
|
||||
setDriverForPty: typeof SetDriverForPty
|
||||
dispose: () => void
|
||||
}> {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const { setFitOverride } = await import('@/lib/pane-manager/mobile-fit-overrides')
|
||||
const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state')
|
||||
setDriverForPty(PTY_ID, { kind: 'mobile', clientId: 'phone-1' })
|
||||
setFitOverride(PTY_ID, 'mobile-fit', PHONE.cols, PHONE.rows)
|
||||
|
||||
const transport = createMockTransport(PTY_ID)
|
||||
@@ -174,7 +182,22 @@ async function connectPaneIntoExistingMobileFitHold(): Promise<{
|
||||
|
||||
const signalPty = window.api.pty.signal as unknown as ReturnType<typeof vi.fn>
|
||||
signalPty.mockClear()
|
||||
return { signalPty, setFitOverride, dispose: () => disposable.dispose() }
|
||||
return { signalPty, setFitOverride, setDriverForPty, dispose: () => disposable.dispose() }
|
||||
}
|
||||
|
||||
/** The desktop "Take back" gesture exactly as reclaimTerminalForDesktop's
|
||||
* active-subscriber branch emits it: applyMobileDisplayMode publishes the
|
||||
* desktop-fit release *while currentDriver is still mobile*, and only the
|
||||
* following releaseDesktopTakeBack flips the driver. The order is awaited,
|
||||
* not raced, and webContents.send preserves it across the two channels. */
|
||||
async function takeBackFromPhone(
|
||||
setFitOverride: typeof SetFitOverride,
|
||||
setDriverForPty: typeof SetDriverForPty,
|
||||
ptyId: string = PTY_ID
|
||||
): Promise<void> {
|
||||
setFitOverride(ptyId, 'desktop-fit', DESKTOP.cols, DESKTOP.rows)
|
||||
setDriverForPty(ptyId, { kind: 'desktop' })
|
||||
await flushAsyncTicks(6)
|
||||
}
|
||||
|
||||
describe('mobile-fit release repaint', () => {
|
||||
@@ -192,7 +215,8 @@ describe('mobile-fit release repaint', () => {
|
||||
})
|
||||
|
||||
it('signals SIGWINCH once the restored desktop grid lands so the TUI repaints', async () => {
|
||||
const { pane, signalPty, setFitOverride, dispose } = await connectPaneHoldingMobileFit()
|
||||
const { pane, signalPty, setFitOverride, setDriverForPty, dispose } =
|
||||
await connectPaneHoldingMobileFit()
|
||||
// Why record inside the mock: asserting the final grid plus the call count
|
||||
// passes just as well when the signal fires *before* the refit, which is
|
||||
// the bug. Only the grid at signal time distinguishes the two.
|
||||
@@ -201,8 +225,7 @@ describe('mobile-fit release repaint', () => {
|
||||
gridAtSignal.push({ cols: pane.terminal.cols, rows: pane.terminal.rows })
|
||||
})
|
||||
|
||||
setFitOverride(PTY_ID, 'desktop-fit', DESKTOP.cols, DESKTOP.rows)
|
||||
await flushAsyncTicks(6)
|
||||
await takeBackFromPhone(setFitOverride, setDriverForPty)
|
||||
|
||||
expect({ signals: signalPty.mock.calls, gridAtSignal }).toEqual({
|
||||
signals: [[PTY_ID, 'SIGWINCH']],
|
||||
@@ -211,6 +234,27 @@ describe('mobile-fit release repaint', () => {
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('repaints a take-back whose release lands before the driver flips to desktop', async () => {
|
||||
const { signalPty, setFitOverride, setDriverForPty, dispose } =
|
||||
await connectPaneHoldingMobileFit()
|
||||
// Pin the emission order the gesture actually produces: the release is
|
||||
// observed while the presence lock still reads mobile. Gating the repaint
|
||||
// on the lock (rather than on the hold it released) silently drops every
|
||||
// desktop "Take back" — the exact gesture #14321 is about.
|
||||
const signalsBeforeDriverFlip: unknown[][] = []
|
||||
setFitOverride(PTY_ID, 'desktop-fit', DESKTOP.cols, DESKTOP.rows)
|
||||
await flushAsyncTicks(6)
|
||||
signalsBeforeDriverFlip.push(...signalPty.mock.calls)
|
||||
setDriverForPty(PTY_ID, { kind: 'desktop' })
|
||||
await flushAsyncTicks(6)
|
||||
|
||||
expect({ signalsBeforeDriverFlip, allSignals: signalPty.mock.calls }).toEqual({
|
||||
signalsBeforeDriverFlip: [[PTY_ID, 'SIGWINCH']],
|
||||
allSignals: [[PTY_ID, 'SIGWINCH']]
|
||||
})
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('does not repaint-signal when another desktop takes the grid instead', async () => {
|
||||
const { signalPty, setFitOverride, dispose } = await connectPaneHoldingMobileFit()
|
||||
|
||||
@@ -223,9 +267,10 @@ describe('mobile-fit release repaint', () => {
|
||||
})
|
||||
|
||||
it('repaint-signals only the hold it released, not every desktop-fit', async () => {
|
||||
const { signalPty, setFitOverride, dispose } = await connectPaneHoldingMobileFit()
|
||||
const { signalPty, setFitOverride, setDriverForPty, dispose } =
|
||||
await connectPaneHoldingMobileFit()
|
||||
|
||||
setFitOverride(PTY_ID, 'desktop-fit', DESKTOP.cols, DESKTOP.rows)
|
||||
await takeBackFromPhone(setFitOverride, setDriverForPty)
|
||||
// A pty exit re-emits desktop-fit for an id that holds nothing (0x0).
|
||||
setFitOverride(PTY_ID, 'desktop-fit', 0, 0)
|
||||
await flushAsyncTicks(6)
|
||||
@@ -246,10 +291,10 @@ describe('mobile-fit release repaint', () => {
|
||||
})
|
||||
|
||||
it('repaints a pane that connected while the phone already held the grid', async () => {
|
||||
const { signalPty, setFitOverride, dispose } = await connectPaneIntoExistingMobileFitHold()
|
||||
const { signalPty, setFitOverride, setDriverForPty, dispose } =
|
||||
await connectPaneIntoExistingMobileFitHold()
|
||||
|
||||
setFitOverride(PTY_ID, 'desktop-fit', DESKTOP.cols, DESKTOP.rows)
|
||||
await flushAsyncTicks(6)
|
||||
await takeBackFromPhone(setFitOverride, setDriverForPty)
|
||||
|
||||
expect(signalPty.mock.calls).toEqual([[PTY_ID, 'SIGWINCH']])
|
||||
dispose()
|
||||
@@ -267,17 +312,51 @@ describe('mobile-fit release repaint', () => {
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('does not repaint while the phone still drives the pty without a fit hold', async () => {
|
||||
const { signalPty, setFitOverride, dispose } = await connectPaneHoldingMobileFit()
|
||||
const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state')
|
||||
// A lock-only takeover keeps the phone authoritative with no fit override.
|
||||
setDriverForPty(PTY_ID, { kind: 'mobile', clientId: 'phone-1' })
|
||||
// Why not "the driver still reads mobile": that is true of every take-back at
|
||||
// release time, so it cannot be the discriminator. The hold itself is — the
|
||||
// phone owns the grid only while an override is held.
|
||||
it('drops a parked repaint once the phone has retaken the grid', async () => {
|
||||
const { pane, signalPty, setFitOverride, setDriverForPty, dispose } =
|
||||
await connectPaneHoldingMobileFit()
|
||||
// The pane cannot measure yet (tab mid-layout), so the release parks the
|
||||
// refit for retry instead of running it.
|
||||
pane.fitAddon.proposeDimensions.mockReturnValue(undefined as never)
|
||||
vi.useFakeTimers()
|
||||
await takeBackFromPhone(setFitOverride, setDriverForPty)
|
||||
const signalsWhileParked = [...signalPty.mock.calls]
|
||||
|
||||
setFitOverride(PTY_ID, 'desktop-fit', DESKTOP.cols, DESKTOP.rows)
|
||||
await flushAsyncTicks(6)
|
||||
// The phone takes the floor again before the pane measures, so the PTY is
|
||||
// back at phone dims. Replaying the parked repaint would redraw there.
|
||||
setDriverForPty(PTY_ID, { kind: 'mobile', clientId: 'phone-2' })
|
||||
setFitOverride(PTY_ID, 'mobile-fit', PHONE.cols, PHONE.rows)
|
||||
pane.fitAddon.proposeDimensions.mockReturnValue(DESKTOP)
|
||||
await vi.advanceTimersByTimeAsync(200)
|
||||
vi.useRealTimers()
|
||||
|
||||
expect(signalPty.mock.calls).toEqual([])
|
||||
expect({ signalsWhileParked, allSignals: signalPty.mock.calls }).toEqual({
|
||||
signalsWhileParked: [],
|
||||
allSignals: []
|
||||
})
|
||||
setDriverForPty(PTY_ID, { kind: 'desktop' })
|
||||
dispose()
|
||||
})
|
||||
|
||||
it('replays a parked repaint when the phone did not retake the grid', async () => {
|
||||
const { pane, signalPty, setFitOverride, setDriverForPty, dispose } =
|
||||
await connectPaneHoldingMobileFit()
|
||||
pane.fitAddon.proposeDimensions.mockReturnValue(undefined as never)
|
||||
vi.useFakeTimers()
|
||||
await takeBackFromPhone(setFitOverride, setDriverForPty)
|
||||
const signalsWhileParked = [...signalPty.mock.calls]
|
||||
|
||||
pane.fitAddon.proposeDimensions.mockReturnValue(DESKTOP)
|
||||
await vi.advanceTimersByTimeAsync(200)
|
||||
vi.useRealTimers()
|
||||
|
||||
expect({ signalsWhileParked, allSignals: signalPty.mock.calls }).toEqual({
|
||||
signalsWhileParked: [],
|
||||
allSignals: [[PTY_ID, 'SIGWINCH']]
|
||||
})
|
||||
dispose()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -4372,11 +4372,16 @@ export function connectPanePty(
|
||||
return
|
||||
}
|
||||
const releasedPtyId = event.ptyId
|
||||
// Why the shared guard: a lock-only takeover holds the grid with no
|
||||
// override, so testing the override alone would repaint a PTY the phone
|
||||
// still drives.
|
||||
// Why the hold and not shouldSuppressDesktopPtyResize: the presence lock is
|
||||
// still mobile when this event arrives. reclaimTerminalForDesktop awaits
|
||||
// applyMobileDisplayMode (which publishes this release) before
|
||||
// releaseDesktopTakeBack flips the driver, so gating on the lock cancels
|
||||
// every desktop "Take back" — the gesture this repaint exists for. The hold
|
||||
// is the real question: once it is gone the PTY is at desktop dims, and if
|
||||
// the phone retakes the grid before a parked refit lands, a fresh
|
||||
// mobile-fit reinstates it and this goes false again.
|
||||
const isCurrentRelease = (): boolean =>
|
||||
!disposed && transport.getPtyId() === releasedPtyId && !shouldSuppressDesktopPtyResize()
|
||||
!disposed && transport.getPtyId() === releasedPtyId && !getFitOverrideForPty(releasedPtyId)
|
||||
mobileFitReleaseRepaintFit?.cancel()
|
||||
mobileFitReleaseRepaintFit = safeFitAndThen(
|
||||
pane,
|
||||
|
||||
Reference in New Issue
Block a user