From b358328b7fad35bffe1ca618f91683ca8c9b6c00 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:32:01 -0400 Subject: [PATCH] test(e2e): pane-close specs close panes the way the user does and check main's copy (#30745) The pane-close consistency spec closed panes with a bare PaneManager.closePane, the drawing-layer call that never asks main. In a layout-owner build that close reached main only because killing the pane's terminal made main retire the pane; right after a reveal the terminal has not attached yet, so main kept the pane. Every user close path already sends the close to main first. The helpers now close through the pane close request the menu and shortcut use, and every scenario also checks that main's stored panes match the window's. --- tests/e2e/helpers/main-terminal-layout.ts | 47 ++++++++++++++ tests/e2e/helpers/terminal-pane-operations.ts | 65 ++++++++++++++----- tests/e2e/helpers/terminal.ts | 2 + tests/e2e/layout-stream-loss-reopen.spec.ts | 46 +------------ ...inal-pane-close-layout-consistency.spec.ts | 61 +++++++++-------- 5 files changed, 132 insertions(+), 89 deletions(-) create mode 100644 tests/e2e/helpers/main-terminal-layout.ts diff --git a/tests/e2e/helpers/main-terminal-layout.ts b/tests/e2e/helpers/main-terminal-layout.ts new file mode 100644 index 00000000000..026d4792a55 --- /dev/null +++ b/tests/e2e/helpers/main-terminal-layout.ts @@ -0,0 +1,47 @@ +import type { Page } from '@stablyai/playwright-test' +import { leafIdsInOrder } from './workspace-layout-oracle-model' +import { readWorkspaceLayoutStreamFrame } from '../../../src/shared/workspace-layout/workspace-layout-stream-frames' + +/** Terminal tab id → its pane ids in tree order, in tab-bar order. */ +export type TerminalLayout = [tabId: string, paneIds: string[]][] + +/** What main holds for the workspace, read from a stream of the test's own. */ +export async function readMainLayout(page: Page, worktreeId: string): Promise { + const raw = await page.evaluate( + (id) => + new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('main sent no layout snapshot')), 10_000) + let handle: { unsubscribe: () => void } | null = null + let snapshot: unknown + const finish = (): void => { + if (handle && snapshot !== undefined) { + clearTimeout(timer) + handle.unsubscribe() + resolve(snapshot) + } + } + void window.api.runtime + .subscribe({ method: 'layout.subscribe', params: { workspaces: [id] } }, (response) => { + if (snapshot === undefined && response.ok) { + snapshot = response.result + finish() + } + }) + .then((opened) => { + handle = opened + finish() + }, reject) + }), + worktreeId + ) + const frame = readWorkspaceLayoutStreamFrame(raw) + if (frame?.type !== 'snapshot') { + throw new Error( + `layout.subscribe opened with no snapshot: ${JSON.stringify(raw).slice(0, 300)}` + ) + } + const layout = frame.workspaces.find((entry) => entry.key === worktreeId)?.layout + return (layout?.tabs ?? []) + .filter((tab) => tab.kind === 'terminal') + .map((tab) => [tab.entityId, leafIdsInOrder(tab.terminal?.root)]) +} diff --git a/tests/e2e/helpers/terminal-pane-operations.ts b/tests/e2e/helpers/terminal-pane-operations.ts index 81023dce669..460cdbe237a 100644 --- a/tests/e2e/helpers/terminal-pane-operations.ts +++ b/tests/e2e/helpers/terminal-pane-operations.ts @@ -4,6 +4,7 @@ import { readPaneIdentitySnapshot, resolveActiveTabId } from './terminal-pane-identity' +import { REQUEST_ACTIVE_TERMINAL_PANE_CLOSE_EVENT } from '../../../src/renderer/src/constants/terminal' export async function readTerminalPaneDomLeafOrder(page: Page): Promise { const snapshot = await readPaneIdentitySnapshot(page) @@ -125,25 +126,53 @@ export async function closeActiveTerminalPane(page: Page): Promise { if (!tabId) { throw new Error('closeActiveTerminalPane: no active terminal tab') } - await page.evaluate((tabId) => { - const paneManagers = window.__paneManagers - if (!paneManagers) { - throw new Error('closeActiveTerminalPane: terminal store/manager unavailable') - } + await requestTerminalPaneClose(page, tabId, 'active') +} - const manager = paneManagers.get(tabId) - const panes = manager?.getPanes?.() ?? [] - if (!manager?.closePane || panes.length < 2) { - return - } - - const activePane = manager.getActivePane?.() ?? panes[0] - if (!activePane) { - return - } - - manager.closePane(activePane.id) - }, tabId) +/** + * Closes one of a tab's split panes the way the user's close does (pane menu, shortcut), then waits + * for it to go. A bare `PaneManager.closePane` skips the close command main applies first in a + * layout-owner build, so main keeps a pane whose terminal had not attached yet. + */ +export async function requestTerminalPaneClose( + page: Page, + tabId: string, + pane: 'active' | 'last' +): Promise { + const leafId = await page.evaluate( + ({ tabId, pane, closeEvent }) => { + const manager = window.__paneManagers?.get(tabId) + if (!manager) { + throw new Error(`requestTerminalPaneClose: no mounted pane manager for tab ${tabId}`) + } + const panes = manager.getPanes() + const target = pane === 'last' ? panes.at(-1) : (manager.getActivePane() ?? panes[0]) + if (!target || panes.length < 2) { + return null + } + manager.setActivePane(target.id, { focus: false }) + window.dispatchEvent(new CustomEvent(closeEvent, { detail: { tabId } })) + return target.leafId + }, + { tabId, pane, closeEvent: REQUEST_ACTIVE_TERMINAL_PANE_CLOSE_EVENT } + ) + if (!leafId) { + return + } + await expect + .poll( + () => + page.evaluate( + ({ tabId, leafId }) => + window.__paneManagers + ?.get(tabId) + ?.getPanes() + .some((candidate) => candidate.leafId === leafId) ?? false, + { tabId, leafId } + ), + { timeout: 15_000, message: `pane ${leafId} did not close` } + ) + .toBe(false) } export async function focusLastTerminalPane(page: Page): Promise { diff --git a/tests/e2e/helpers/terminal.ts b/tests/e2e/helpers/terminal.ts index 7b7c83d2c87..5b37627c7c6 100644 --- a/tests/e2e/helpers/terminal.ts +++ b/tests/e2e/helpers/terminal.ts @@ -25,6 +25,8 @@ import { waitForTerminalOutput as waitForTerminalOutputImpl } from './terminal-pane-operations' +export { requestTerminalPaneClose } from './terminal-pane-operations' + export { expectTerminalAccessibilityText, readTerminalAccessibilityText diff --git a/tests/e2e/layout-stream-loss-reopen.spec.ts b/tests/e2e/layout-stream-loss-reopen.spec.ts index 4cedd5e29cb..10bb10414f6 100644 --- a/tests/e2e/layout-stream-loss-reopen.spec.ts +++ b/tests/e2e/layout-stream-loss-reopen.spec.ts @@ -12,15 +12,12 @@ import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca- import { ensureTerminalVisible, switchToWorktree, waitForSessionReady } from './helpers/store' import { splitActiveTerminalPane, waitForActiveTerminalManager } from './helpers/terminal' import { leafIdsInOrder } from './helpers/workspace-layout-oracle-model' -import { readWorkspaceLayoutStreamFrame } from '../../src/shared/workspace-layout/workspace-layout-stream-frames' +import { readMainLayout, type TerminalLayout } from './helpers/main-terminal-layout' const LAYOUT_OWNER_BUILD = existsSync( path.join(process.cwd(), 'out', 'main', 'ORCA_LAYOUT_OWNER_MAIN') ) -/** Terminal tab id → its pane ids in tree order, in tab-bar order. */ -type TerminalLayout = [tabId: string, paneIds: string[]][] - type Launched = Awaited['launch']>> /** What the window draws for the workspace's terminal tabs. */ @@ -48,47 +45,6 @@ async function readWindowLayout(page: Page, worktreeId: string): Promise [tabId, leafIdsInOrder(drawn.roots[tabId])]) } -/** What main holds for the workspace, read from a stream of the test's own. */ -async function readMainLayout(page: Page, worktreeId: string): Promise { - const raw = await page.evaluate( - (id) => - new Promise((resolve, reject) => { - const timer = setTimeout(() => reject(new Error('main sent no layout snapshot')), 10_000) - let handle: { unsubscribe: () => void } | null = null - let snapshot: unknown - const finish = (): void => { - if (handle && snapshot !== undefined) { - clearTimeout(timer) - handle.unsubscribe() - resolve(snapshot) - } - } - void window.api.runtime - .subscribe({ method: 'layout.subscribe', params: { workspaces: [id] } }, (response) => { - if (snapshot === undefined && response.ok) { - snapshot = response.result - finish() - } - }) - .then((opened) => { - handle = opened - finish() - }, reject) - }), - worktreeId - ) - const frame = readWorkspaceLayoutStreamFrame(raw) - if (frame?.type !== 'snapshot') { - throw new Error( - `layout.subscribe opened with no snapshot: ${JSON.stringify(raw).slice(0, 300)}` - ) - } - const layout = frame.workspaces.find((entry) => entry.key === worktreeId)?.layout - return (layout?.tabs ?? []) - .filter((tab) => tab.kind === 'terminal') - .map((tab) => [tab.entityId, leafIdsInOrder(tab.terminal?.root)]) -} - /** Tab ids in tab-bar order and each tab's pane count: what a lost or doubled tab or pane changes. */ type Summary = { tabs: string[]; panes: number[] } diff --git a/tests/e2e/terminal-pane-close-layout-consistency.spec.ts b/tests/e2e/terminal-pane-close-layout-consistency.spec.ts index cde78f85c57..eea50b2fff9 100644 --- a/tests/e2e/terminal-pane-close-layout-consistency.spec.ts +++ b/tests/e2e/terminal-pane-close-layout-consistency.spec.ts @@ -7,12 +7,15 @@ import { waitForSessionReady } from './helpers/store' import { + requestTerminalPaneClose, sendToTerminal, splitActiveTerminalPane, waitForActiveTerminalManager, waitForPaneIdentitySnapshot } from './helpers/terminal' import { parkHiddenTabBehindDecoy } from './helpers/terminal-hidden-parking' +import { readMainLayout } from './helpers/main-terminal-layout' +import { REQUEST_ACTIVE_TERMINAL_PANE_CLOSE_EVENT } from '../../src/renderer/src/constants/terminal' /** * Repro hunt for the "ghost blank pane" field incident: a split pane whose PTY @@ -159,18 +162,21 @@ async function expectLayoutConsistent( }) } -async function closeLastPaneOnTab(page: Page, tabId: string): Promise { - await page.evaluate((tabId) => { - const manager = window.__paneManagers?.get(tabId) - if (!manager) { - throw new Error(`closeLastPaneOnTab: no mounted pane manager for tab ${tabId}`) - } - const target = manager.getPanes().at(-1) - if (!target) { - throw new Error('closeLastPaneOnTab: tab has no panes') - } - manager.closePane(target.id) - }, tabId) +/** Main's stored panes for the tab are the window's: the close the window drew reached main. */ +async function expectMainHoldsWindowPanes( + page: Page, + worktreeId: string, + tabId: string, + phase: string +): Promise { + const { rootLeafIds } = await readLayoutConsistency(page, tabId) + await expect + .poll( + async () => + (await readMainLayout(page, worktreeId)).find(([id]) => id === tabId)?.[1] ?? null, + { timeout: 15_000, message: `[${phase}] main's stored panes differ from the window's` } + ) + .toEqual(rootLeafIds) } async function createActiveTerminalTab(page: Page, worktreeId: string): Promise { @@ -254,15 +260,16 @@ async function setUpSplitTab(page: Page): Promise { test.describe('terminal pane close vs hidden/park lifecycle keeps layout consistent', () => { test('control: close while visible', async ({ orcaPage }) => { - const { tabId } = await setUpSplitTab(orcaPage) - await closeLastPaneOnTab(orcaPage, tabId) + const { worktreeId, tabId } = await setUpSplitTab(orcaPage) + await requestTerminalPaneClose(orcaPage, tabId, 'last') await expectLayoutConsistent(orcaPage, tabId, 1, 'close-visible') + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'close-visible') }) test('close and hide the tab in the same tick', async ({ orcaPage }) => { const { worktreeId, tabId } = await setUpSplitTab(orcaPage) await orcaPage.evaluate( - ({ tabId, worktreeId }) => { + ({ tabId, worktreeId, closeEvent }) => { const store = window.__store const manager = window.__paneManagers?.get(tabId) if (!store || !manager) { @@ -272,31 +279,35 @@ test.describe('terminal pane close vs hidden/park lifecycle keeps layout consist if (!target) { throw new Error('close+hide: no split pane') } - manager.closePane(target.id) + // The user's close (see requestTerminalPaneClose), sent without waiting for it. + manager.setActivePane(target.id, { focus: false }) + window.dispatchEvent(new CustomEvent(closeEvent, { detail: { tabId } })) // Hide tab A before any deferred post-close work can run. const state = store.getState() const tab = state.createTab(worktreeId, undefined, undefined, { activate: true }) state.setActiveTab(tab.id) state.setActiveTabType('terminal', window.__store?.getState().activeWorktreeId ?? null) }, - { tabId, worktreeId } + { tabId, worktreeId, closeEvent: REQUEST_ACTIVE_TERMINAL_PANE_CLOSE_EVENT } ) await orcaPage.waitForTimeout(PARKING_DELAY_MS * 3) await activateTerminalTab(orcaPage, tabId) await waitForTabRemounted(orcaPage, tabId) await expectLayoutConsistent(orcaPage, tabId, 1, 'close-then-hide-same-tick') + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'close-then-hide-same-tick') }) test('close while hidden but still mounted (hot-retain window)', async ({ orcaPage }) => { const { worktreeId, tabId } = await setUpSplitTab(orcaPage) await createActiveTerminalTab(orcaPage, worktreeId) - await closeLastPaneOnTab(orcaPage, tabId) + await requestTerminalPaneClose(orcaPage, tabId, 'last') await parkHiddenTabBehindDecoy(orcaPage, worktreeId, tabId, { parkDelayMs: PARKING_DELAY_MS }) await activateTerminalTab(orcaPage, tabId) await waitForTabRemounted(orcaPage, tabId) await expectLayoutConsistent(orcaPage, tabId, 1, 'close-while-hidden-mounted') + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'close-while-hidden-mounted') }) test('close immediately after reveal remount, before panes settle', async ({ orcaPage }) => { @@ -308,19 +319,14 @@ test.describe('terminal pane close vs hidden/park lifecycle keeps layout consist await activateTerminalTab(orcaPage, tabId) await waitForTabRemounted(orcaPage, tabId) // Close as soon as the manager exists — panes may still be attaching. - await orcaPage.evaluate((tabId) => { - const manager = window.__paneManagers?.get(tabId) - const target = manager?.getPanes().at(-1) - if (manager && target) { - manager.closePane(target.id) - } - }, tabId) + await requestTerminalPaneClose(orcaPage, tabId, 'last') await expectLayoutConsistent(orcaPage, tabId, 1, 'close-mid-reveal') + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'close-mid-reveal') }) test('clean visible close survives a later park/reveal cycle', async ({ orcaPage }) => { const { worktreeId, tabId } = await setUpSplitTab(orcaPage) - await closeLastPaneOnTab(orcaPage, tabId) + await requestTerminalPaneClose(orcaPage, tabId, 'last') await expectLayoutConsistent(orcaPage, tabId, 1, 'pre-park close') await createActiveTerminalTab(orcaPage, worktreeId) await parkHiddenTabBehindDecoy(orcaPage, worktreeId, tabId, { @@ -329,6 +335,7 @@ test.describe('terminal pane close vs hidden/park lifecycle keeps layout consist await activateTerminalTab(orcaPage, tabId) await waitForTabRemounted(orcaPage, tabId) await expectLayoutConsistent(orcaPage, tabId, 1, 'post-park-reveal') + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'post-park-reveal') }) test('split pane shell exits while hidden but still mounted', async ({ orcaPage }) => { @@ -341,6 +348,7 @@ test.describe('terminal pane close vs hidden/park lifecycle keeps layout consist await activateTerminalTab(orcaPage, tabId) await waitForTabRemounted(orcaPage, tabId) await expectLayoutConsistent(orcaPage, tabId, 1, 'shell-exit-while-hidden-mounted', splitPtyId) + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'shell-exit-while-hidden-mounted') }) test('split pane shell exits while the tab is parked', async ({ orcaPage }) => { @@ -357,5 +365,6 @@ test.describe('terminal pane close vs hidden/park lifecycle keeps layout consist // siblings) — the reveal remount owns the per-leaf teardown. This asserts // that ownership actually resolves instead of leaving a ghost pane. await expectLayoutConsistent(orcaPage, tabId, 1, 'shell-exit-while-parked', splitPtyId) + await expectMainHoldsWindowPanes(orcaPage, worktreeId, tabId, 'shell-exit-while-parked') }) })