fix: refit terminals after bulk mobile restore (#5962)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong 2026-06-20 23:35:37 -07:00 committed by GitHub
parent 8226220466
commit 62f1394b44
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 237 additions and 6 deletions

View File

@ -98,6 +98,7 @@ import {
import type { TerminalQuickCommand, TerminalQuickCommandScope } from '../../../../shared/types'
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
import { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id'
import { refitAndRefreshAllTerminalPanes } from '@/lib/pane-manager/pane-manager-registry'
import {
getTerminalQuickCommandScope,
isTerminalQuickCommandComplete,
@ -2161,6 +2162,8 @@ export default function TerminalPane({
settingsRef.current ?? undefined
)
if (restored) {
requestAnimationFrame(refitAndRefreshAllTerminalPanes)
window.setTimeout(refitAndRefreshAllTerminalPanes, 100)
focusPane.terminal.focus()
}
},

View File

@ -1,5 +1,6 @@
import { afterEach, describe, expect, it, vi, type Mock } from 'vitest'
import {
refitAndRefreshAllTerminalPanes,
registerLivePaneManager,
resetAllTerminalWebglAtlases,
unregisterLivePaneManager
@ -57,4 +58,54 @@ describe('pane manager registry', () => {
expect(broken.resetWebglTextureAtlases).toHaveBeenCalledTimes(1)
expect(healthy.resetWebglTextureAtlases).toHaveBeenCalledTimes(1)
})
it('fits and refreshes every registered manager', () => {
const first = {
resetWebglTextureAtlases: vi.fn<() => void>(),
fitAllPanes: vi.fn<() => void>(),
refreshAllPanes: vi.fn<() => void>()
}
const second = {
resetWebglTextureAtlases: vi.fn<() => void>(),
fitAllPanes: vi.fn<() => void>(),
refreshAllPanes: vi.fn<() => void>()
}
registerLivePaneManager(first)
registeredManagers.push(first)
registerLivePaneManager(second)
registeredManagers.push(second)
refitAndRefreshAllTerminalPanes()
expect(first.fitAllPanes).toHaveBeenCalledTimes(1)
expect(first.refreshAllPanes).toHaveBeenCalledTimes(1)
expect(second.fitAllPanes).toHaveBeenCalledTimes(1)
expect(second.refreshAllPanes).toHaveBeenCalledTimes(1)
})
it('continues refitting later managers when one manager throws', () => {
const broken = {
resetWebglTextureAtlases: vi.fn<() => void>(),
fitAllPanes: vi.fn<() => void>(() => {
throw new Error('pane disposed')
}),
refreshAllPanes: vi.fn<() => void>()
}
registerLivePaneManager(broken)
registeredManagers.push(broken)
const healthy = {
resetWebglTextureAtlases: vi.fn<() => void>(),
fitAllPanes: vi.fn<() => void>(),
refreshAllPanes: vi.fn<() => void>()
}
registerLivePaneManager(healthy)
registeredManagers.push(healthy)
expect(() => refitAndRefreshAllTerminalPanes()).not.toThrow()
expect(broken.fitAllPanes).toHaveBeenCalledTimes(1)
expect(broken.refreshAllPanes).not.toHaveBeenCalled()
expect(healthy.fitAllPanes).toHaveBeenCalledTimes(1)
expect(healthy.refreshAllPanes).toHaveBeenCalledTimes(1)
})
})

View File

@ -1,14 +1,16 @@
type AtlasResettablePaneManager = {
type RegisteredPaneManager = {
resetWebglTextureAtlases(): void
fitAllPanes?: () => void
refreshAllPanes?: () => void
}
const liveManagers = new Set<AtlasResettablePaneManager>()
const liveManagers = new Set<RegisteredPaneManager>()
export function registerLivePaneManager(manager: AtlasResettablePaneManager): void {
export function registerLivePaneManager(manager: RegisteredPaneManager): void {
liveManagers.add(manager)
}
export function unregisterLivePaneManager(manager: AtlasResettablePaneManager): void {
export function unregisterLivePaneManager(manager: RegisteredPaneManager): void {
liveManagers.delete(manager)
}
@ -31,3 +33,16 @@ export function resetAllTerminalWebglAtlases(): void {
}
}
}
export function refitAndRefreshAllTerminalPanes(): void {
for (const manager of liveManagers) {
try {
// Why: after bulk desktop restore, background panes may have correct
// cols/rows but a stale xterm renderer until focus forces a repaint.
manager.fitAllPanes?.()
manager.refreshAllPanes?.()
} catch {
// Why: restore-all is best-effort across live managers during teardown.
}
}
}

View File

@ -165,6 +165,18 @@ export class PaneManager {
fitAllPanesInternal(this.panes)
}
refreshAllPanes(): void {
for (const pane of this.panes.values()) {
try {
if (pane.terminal.rows > 0) {
pane.terminal.refresh(0, pane.terminal.rows - 1)
}
} catch {
// Why: restore-all repaint is best-effort while panes are mounting or tearing down.
}
}
}
equalizePaneSizes(): void {
if (this.panes.size < 2) {
return

View File

@ -1,7 +1,11 @@
import type { ElectronApplication, Page, TestInfo } from '@stablyai/playwright-test'
import { test, expect } from './helpers/orca-app'
import { ensureTerminalVisible, waitForSessionReady, waitForActiveWorktree } from './helpers/store'
import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal'
import {
splitActiveTerminalPane,
waitForActivePanePtyId,
waitForActiveTerminalManager
} from './helpers/terminal'
// Why: regression coverage for the mobile-presence-lock UX (PR #1532). Strong
// DOM assertions guard the "doesn't mount / doesn't dismiss" regression class;
@ -69,7 +73,7 @@ test('mobile subscribe mounts overlay; collapse → chip; Take back dismisses',
await expect(overlay).not.toContainText(/your phone is in control/i)
// Take back from the chip dismisses the overlay. The button calls
// runtime.restoreTerminalFit via IPC; main responds with desktop-fit + idle
// runtime.restoreTerminalFit via IPC; main responds with desktop-fit + desktop
// driver events that we mirror here so the renderer state lands on the
// post-take-back terminal state.
await overlay.getByRole('button', { name: /take back/i }).click()
@ -115,6 +119,48 @@ test('held phone-fit state mounts restore overlay without collapse', async ({
await expect(overlay).toBeHidden({ timeout: 15_000 })
})
test('restore all refits non-focused restored terminal panes', async ({
orcaPage,
electronApp
}) => {
await waitForSessionReady(orcaPage)
await waitForActiveWorktree(orcaPage)
await ensureTerminalVisible(orcaPage)
await waitForActiveTerminalManager(orcaPage)
await splitActiveTerminalPane(orcaPage, 'vertical')
const ptyIds = await waitForVisiblePanePtyIds(orcaPage, 2)
const focusPtyId = await waitForActivePanePtyId(orcaPage)
const inactivePtyId = ptyIds.find((ptyId) => ptyId !== focusPtyId)
if (!inactivePtyId || !focusPtyId) {
throw new Error('Expected two visible terminal panes with PTY bindings')
}
await installRestoreTerminalFitAutoRestoreRecorder(electronApp)
await sendHeldPhoneFitIpc(electronApp, { ptyId: inactivePtyId, cols: 45, rows: 20 })
await sendHeldPhoneFitIpc(electronApp, { ptyId: focusPtyId, cols: 45, rows: 20 })
await expect(orcaPage.locator('.mobile-driver-banner')).toHaveCount(2, { timeout: 15_000 })
await forcePaneToOneColumn(orcaPage, inactivePtyId)
await expect
.poll(() => getPaneTerminalCols(orcaPage, inactivePtyId), {
message: 'test harness should force the non-focused pane into the bad narrow state'
})
.toBeLessThanOrEqual(2)
await orcaPage
.locator(`[data-pty-id="${focusPtyId}"] .mobile-driver-banner`)
.getByRole('button', { name: /restore all terminals/i })
.click()
await expectRestoreTerminalFitCallSet(electronApp, [inactivePtyId, focusPtyId])
await expect
.poll(() => getPaneTerminalCols(orcaPage, inactivePtyId), {
timeout: 5_000,
message: 'Restore all should refit the non-focused restored pane'
})
.toBeGreaterThan(20)
})
async function sendMobileSubscribeIpc(
electronApp: ElectronApplication,
args: { ptyId: string; cols: number; rows: number }
@ -196,6 +242,36 @@ async function installRestoreTerminalFitRecorder(electronApp: ElectronApplicatio
})
}
async function installRestoreTerminalFitAutoRestoreRecorder(
electronApp: ElectronApplication
): Promise<void> {
await electronApp.evaluate(({ BrowserWindow, ipcMain }) => {
const testGlobal = globalThis as typeof globalThis & {
__mobileBannerRestoreCalls?: string[]
}
testGlobal.__mobileBannerRestoreCalls = []
// Why: the production restore path sets desktop control after clearing the
// fit override, so this harness mirrors both renderer-facing events.
ipcMain.removeHandler('runtime:restoreTerminalFit')
ipcMain.handle('runtime:restoreTerminalFit', (_event, args: { ptyId: string }) => {
testGlobal.__mobileBannerRestoreCalls?.push(args.ptyId)
for (const win of BrowserWindow.getAllWindows()) {
win.webContents.send('runtime:terminalFitOverrideChanged', {
ptyId: args.ptyId,
mode: 'desktop-fit',
cols: 0,
rows: 0
})
win.webContents.send('runtime:terminalDriverChanged', {
ptyId: args.ptyId,
driver: { kind: 'desktop' }
})
}
return { restored: true }
})
})
}
async function expectRestoreTerminalFitCalls(
electronApp: ElectronApplication,
expected: string[]
@ -216,6 +292,80 @@ async function expectRestoreTerminalFitCalls(
.toEqual(expected)
}
async function expectRestoreTerminalFitCallSet(
electronApp: ElectronApplication,
expected: string[]
): Promise<void> {
await expect
.poll(
() =>
electronApp.evaluate(
() =>
(
globalThis as typeof globalThis & {
__mobileBannerRestoreCalls?: string[]
}
).__mobileBannerRestoreCalls ?? []
),
{ message: 'restore all should invoke the production restore channel for each PTY' }
)
.toEqual(expect.arrayContaining(expected))
}
async function waitForVisiblePanePtyIds(page: Page, expectedCount: number): Promise<string[]> {
let ptyIds: string[] = []
await expect
.poll(
async () => {
ptyIds = await page.evaluate(() => {
const state = window.__store?.getState()
const tabId = state?.activeTabId
const manager = tabId ? window.__paneManagers?.get(tabId) : null
return (manager?.getPanes?.() ?? [])
.map((pane) => pane.container?.dataset?.ptyId ?? null)
.filter((ptyId): ptyId is string => Boolean(ptyId))
})
return ptyIds.length
},
{
timeout: 15_000,
message: `Expected ${expectedCount} visible panes with PTY bindings`
}
)
.toBe(expectedCount)
return ptyIds
}
async function forcePaneToOneColumn(page: Page, ptyId: string): Promise<void> {
await page.evaluate((targetPtyId) => {
for (const manager of window.__paneManagers?.values?.() ?? []) {
const pane = manager
.getPanes?.()
.find((candidate) => candidate.container.dataset.ptyId === targetPtyId)
if (pane) {
pane.terminal.resize(1, Math.max(8, pane.terminal.rows))
pane.terminal.refresh(0, pane.terminal.rows - 1)
return
}
}
throw new Error(`No pane found for PTY ${targetPtyId}`)
}, ptyId)
}
async function getPaneTerminalCols(page: Page, ptyId: string): Promise<number> {
return page.evaluate((targetPtyId) => {
for (const manager of window.__paneManagers?.values?.() ?? []) {
const pane = manager
.getPanes?.()
.find((candidate) => candidate.container.dataset.ptyId === targetPtyId)
if (pane) {
return pane.terminal.cols
}
}
return 0
}, ptyId)
}
async function expectExpandedOverlayLeavesPaneReadable(page: Page, ptyId: string): Promise<void> {
await expect
.poll(