diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 99408bd9dc5..ded057125a9 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -1594,7 +1594,7 @@ jobs: with: ref: ${{ github.sha }} # LAYOUT_OWNER_SWITCH_ON_E2E_SPECS in config/scripts/pr-layout-owner-switch-on-route.mjs. - test_files: '["tests/e2e/workspace-layout-oracle-headless.spec.ts","tests/e2e/workspace-layout-oracle.spec.ts","tests/e2e/cli-file-open-position.spec.ts","tests/e2e/relaunch-first-terminal.spec.ts","tests/e2e/relaunch-sleeping-agents-resume-on-view.spec.ts","tests/e2e/terminal-cold-activation-deferral.spec.ts","tests/e2e/cli-first-worktree-single-terminal.spec.ts","tests/e2e/unopened-workspace-agent-single-resume.spec.ts","tests/e2e/terminal-pane-close-layout-consistency.spec.ts","tests/e2e/golden-file-open-edit-save.spec.ts","tests/e2e/golden-source-control-open-diff.spec.ts","tests/e2e/paired-cli-terminal-graph-sync-tab-retention.spec.ts","tests/e2e/floating-workspace-pane-height.spec.ts","tests/e2e/editor-model-host-isolation.spec.ts","tests/e2e/editor-tab-selection-restore.spec.ts","tests/e2e/finished-agent-ghost-resume.spec.ts","tests/e2e/live-background-terminal-mount-authority.spec.ts","tests/e2e/workspace-back-forward-navigation.spec.ts","tests/e2e/worktree-lineage-toggle-shortcut.spec.ts","tests/e2e/local-cli-terminal-retention.spec.ts","tests/e2e/default-tabs-mark-crash-relaunch.spec.ts","tests/e2e/terminal-cli-split-closed-before-mount.spec.ts","tests/e2e/layout-stream-loss-reopen.spec.ts","tests/e2e/artificial-opencode-terminal-load.spec.ts","tests/e2e/ssh-orcad-"]' + test_files: '["tests/e2e/workspace-layout-oracle-headless.spec.ts","tests/e2e/workspace-layout-oracle.spec.ts","tests/e2e/cli-file-open-position.spec.ts","tests/e2e/relaunch-first-terminal.spec.ts","tests/e2e/relaunch-sleeping-agents-resume-on-view.spec.ts","tests/e2e/terminal-cold-activation-deferral.spec.ts","tests/e2e/cli-first-worktree-single-terminal.spec.ts","tests/e2e/unopened-workspace-agent-single-resume.spec.ts","tests/e2e/terminal-pane-close-layout-consistency.spec.ts","tests/e2e/golden-file-open-edit-save.spec.ts","tests/e2e/golden-source-control-open-diff.spec.ts","tests/e2e/paired-cli-terminal-graph-sync-tab-retention.spec.ts","tests/e2e/floating-workspace-pane-height.spec.ts","tests/e2e/editor-model-host-isolation.spec.ts","tests/e2e/editor-tab-selection-restore.spec.ts","tests/e2e/finished-agent-ghost-resume.spec.ts","tests/e2e/live-background-terminal-mount-authority.spec.ts","tests/e2e/workspace-back-forward-navigation.spec.ts","tests/e2e/worktree-lineage-toggle-shortcut.spec.ts","tests/e2e/local-cli-terminal-retention.spec.ts","tests/e2e/default-tabs-mark-crash-relaunch.spec.ts","tests/e2e/terminal-cli-split-closed-before-mount.spec.ts","tests/e2e/layout-stream-loss-reopen.spec.ts","tests/e2e/terminal-tab-close-restart-persistence.spec.ts","tests/e2e/artificial-opencode-terminal-load.spec.ts","tests/e2e/ssh-orcad-"]' run_changed_e2e: true needs_build: true artifact_suffix: -layout-owner-switch-on diff --git a/config/scripts/pr-layout-owner-switch-on-route.mjs b/config/scripts/pr-layout-owner-switch-on-route.mjs index 1b329223028..fc9fbc4fcc5 100644 --- a/config/scripts/pr-layout-owner-switch-on-route.mjs +++ b/config/scripts/pr-layout-owner-switch-on-route.mjs @@ -32,6 +32,8 @@ export const LAYOUT_OWNER_SWITCH_ON_E2E_SPECS = [ 'tests/e2e/default-tabs-mark-crash-relaunch.spec.ts', 'tests/e2e/terminal-cli-split-closed-before-mount.spec.ts', 'tests/e2e/layout-stream-loss-reopen.spec.ts', + // A tab close by the handle a split returned. + 'tests/e2e/terminal-tab-close-restart-persistence.spec.ts', // A split main first sees at its terminal binding keeps its direction (fails only at CI's window size). 'tests/e2e/artificial-opencode-terminal-load.spec.ts', // Every managed-SSH spec, in e2e.yml's Docker lane; its orcad template stays switch-off. diff --git a/src/main/runtime/session-lifecycle/close/close-by-split-handle-layout-owner.test.ts b/src/main/runtime/session-lifecycle/close/close-by-split-handle-layout-owner.test.ts new file mode 100644 index 00000000000..5725250eca2 --- /dev/null +++ b/src/main/runtime/session-lifecycle/close/close-by-split-handle-layout-owner.test.ts @@ -0,0 +1,255 @@ +import { describe, expect, it, vi } from 'vitest' +import { + withDurableRuntimeStore, + withRuntimePaneAdmission +} from '../../runtime-durable-store-fixture' +import { getDefaultWorkspaceSession } from '../../../../shared/constants' +import type { + RuntimeMobileSessionTabsSnapshot, + RuntimeMobileSessionTerminalTab +} from '../../../../shared/runtime-types' +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' +import { OrcaRuntimeService } from '../../orca-runtime' + +vi.mock('../../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) + +const REPO_ID = 'repo-1' +const WORKTREE_ID = `${REPO_ID}::/workspace` +const TAB_ID = 'tab-1' +const SOURCE_LEAF_ID = '11111111-1111-4111-8111-111111111111' +const SOURCE_PTY_ID = 'pty-source' +const SPLIT_PTY_ID = 'pty-split' + +function sourceLayout(): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId: SOURCE_LEAF_ID }, + activeLeafId: SOURCE_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [SOURCE_LEAF_ID]: SOURCE_PTY_ID } + } +} + +function persistedSession(): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [WORKTREE_ID]: [ + { + id: TAB_ID, + ptyId: SOURCE_PTY_ID, + worktreeId: WORKTREE_ID, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { [TAB_ID]: sourceLayout() } + } +} + +function surface( + leafId: string, + ptyId: string | null, + parentLayout: TerminalLayoutSnapshot +): RuntimeMobileSessionTerminalTab { + return { + type: 'terminal', + id: `${TAB_ID}::${leafId}`, + parentTabId: TAB_ID, + leafId, + ptyId, + title: 'Terminal', + parentLayout, + isActive: leafId === SOURCE_LEAF_ID + } +} + +/** The window's own publication: one surface per pane it has mounted. */ +function windowSnapshot( + version: number, + surfaces: RuntimeMobileSessionTerminalTab[] +): RuntimeMobileSessionTabsSnapshot { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'window', + snapshotVersion: version, + activeGroupId: 'group-1', + activeTabId: `${TAB_ID}::${SOURCE_LEAF_ID}`, + activeTabType: 'terminal', + tabGroups: [{ id: 'group-1', activeTabId: TAB_ID, tabOrder: [TAB_ID] }], + tabs: surfaces + } +} + +function windowGraph( + leaves: { leafId: string; ptyId: string | null }[], + snapshot: RuntimeMobileSessionTabsSnapshot +) { + return { + tabs: [ + { + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + title: 'Terminal', + activeLeafId: SOURCE_LEAF_ID, + layout: null + } + ], + leaves: leaves.map(({ leafId, ptyId }, index) => ({ + tabId: TAB_ID, + worktreeId: WORKTREE_ID, + leafId, + paneRuntimeId: index + 1, + ptyId + })), + mobileSessionTabs: [snapshot] + } +} + +/** What the window's list holds for main's new pane when it lands, after main's own. */ +type WindowListing = 'pane-without-terminal' | 'no-new-pane' + +function setup(listing: WindowListing) { + let session = persistedSession() + const repo = { + id: REPO_ID, + path: '/workspace', + displayName: 'repo', + badgeColor: 'blue', + addedAt: 1 + } + const store = withDurableRuntimeStore( + withRuntimePaneAdmission({ + getRepos: () => [repo], + getRepo: (id: string) => (id === REPO_ID ? repo : undefined), + getWorkspaceSession: () => session, + setWorkspaceSession: (next: WorkspaceSessionState) => { + session = next + }, + persistPtyBinding: () => true, + flushOrThrow: () => undefined + }) + ) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fake carries every store method a split and a tab close reach. + const runtime = new OrcaRuntimeService(store as never) + Object.assign(runtime, { + resolveTerminalWorkspaceLaunchScope: vi.fn(async () => ({ + id: WORKTREE_ID, + path: '/workspace', + connectionId: null, + repo, + folderWorkspace: null + })) + }) + runtime.setPtyController({ + // The real spawn writes the new pane's binding into main's session; the fake does the same. + spawn: vi.fn(async (args: { tabId?: string; leafId?: string }) => { + const layout = session.terminalLayoutsByTabId[args.tabId!]! + session = { + ...session, + terminalLayoutsByTabId: { + ...session.terminalLayoutsByTabId, + [args.tabId!]: { + ...layout, + ptyIdsByLeafId: { ...layout.ptyIdsByLeafId, [args.leafId!]: SPLIT_PTY_ID } + } + } + } + return { id: SPLIT_PTY_ID } + }), + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + // The window's list lags main: built before main's first frame (no new pane), or from it (the + // pane, but not yet its terminal). Either lands after main's own listing. + const revealTerminalSession = vi.fn(async (_worktreeId: string, args: { leafId?: string }) => { + const newLeafId = args.leafId! + const twoPanes = { + ...sourceLayout(), + root: { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: SOURCE_LEAF_ID }, + second: { type: 'leaf' as const, leafId: newLeafId } + } + } + runtime.syncWindowGraph( + 1, + windowGraph( + [{ leafId: SOURCE_LEAF_ID, ptyId: SOURCE_PTY_ID }], + windowSnapshot( + 2, + listing === 'no-new-pane' + ? [surface(SOURCE_LEAF_ID, SOURCE_PTY_ID, sourceLayout())] + : [surface(SOURCE_LEAF_ID, SOURCE_PTY_ID, twoPanes), surface(newLeafId, null, twoPanes)] + ) + ) + ) + return { tabId: TAB_ID } + }) + const notifier = { + revealTerminalSession, + splitTerminal: vi.fn(), + closeTerminalTab: vi.fn(async () => undefined) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the split and the tab close call only these notices. + runtime.setNotifier(notifier as never) + runtime.syncWindowGraph( + 1, + windowGraph( + [{ leafId: SOURCE_LEAF_ID, ptyId: SOURCE_PTY_ID }], + windowSnapshot(1, [surface(SOURCE_LEAF_ID, SOURCE_PTY_ID, sourceLayout())]) + ) + ) + runtime.registerPty(SOURCE_PTY_ID, WORKTREE_ID, null, { tabId: TAB_ID, leafId: SOURCE_LEAF_ID }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: both members are declared on OrcaRuntimeService's inheritance chain. + const internals = runtime as unknown as { + issuePtyHandle: (pty: unknown) => string + ptysById: Map + } + // A terminal listing can name the source by its terminal, so the split runs main-side. + const handle = internals.issuePtyHandle(internals.ptysById.get(SOURCE_PTY_ID)) + // Main's layout drops the pane, as a close of it would. + const dropPane = (leafId: string): void => { + const layout = session.terminalLayoutsByTabId[TAB_ID]! + const { [leafId]: _dropped, ...ptyIdsByLeafId } = layout.ptyIdsByLeafId ?? {} + session = { + ...session, + terminalLayoutsByTabId: { + ...session.terminalLayoutsByTabId, + [TAB_ID]: { ...layout, root: sourceLayout().root, ptyIdsByLeafId } + } + } + } + return { runtime, handle, dropPane } +} + +describe("a tab close by a main-side split's handle while the window's list lags, in a layout-owner build", () => { + it.each(['pane-without-terminal', 'no-new-pane'])( + "closes the tab on main's own record of the pane (%s)", + async (listing) => { + const { runtime, handle } = setup(listing) + + const split = await runtime.splitTerminal(handle, { direction: 'vertical' }) + + await expect(runtime.closeTerminalTab(split.handle)).resolves.toMatchObject({ + tabId: TAB_ID, + closeMode: 'tab' + }) + } + ) + + it('still refuses once main no longer holds the pane', async () => { + const { runtime, handle, dropPane } = setup('no-new-pane') + + const split = await runtime.splitTerminal(handle, { direction: 'vertical' }) + dropPane(split.leafId!) + + await expect(runtime.closeTerminalTab(split.handle)).rejects.toThrow('terminal_handle_stale') + }) +}) diff --git a/src/main/runtime/terminal/view/layout-owner-close-relisting.ts b/src/main/runtime/terminal/view/layout-owner-close-relisting.ts new file mode 100644 index 00000000000..7a409d6f5a4 --- /dev/null +++ b/src/main/runtime/terminal/view/layout-owner-close-relisting.ts @@ -0,0 +1,66 @@ +import type { + RuntimePtyTabCloseAuthority, + RuntimePtyWorktreeRecord +} from '../../runtime-terminal-state-records' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' +import { makePaneKey, parsePaneKey } from '../../../../shared/stable-pane-id' +import { terminalLayoutContainsLeaf } from '../../../../shared/workspace-session-pane-ownership' +import type { ExecutionHostId } from '../../../../shared/execution-host' + +type RelistedPane = { + tabId: string + leafId: string + title: null + activate: false + selectIfNoActiveTab: false + /** The close's own record: clients keep the window's list, which catches up on its own. */ + notify: false +} + +/** + * Layout-owner builds: the pane main's own layout binds this close's terminal to, when the window's + * list lacks it. That list lags main (a split written, not yet drawn or bound), and a close by the + * terminal's handle must not depend on it. + */ +export function relistPaneMainBinds( + authority: RuntimePtyTabCloseAuthority, + pty: RuntimePtyWorktreeRecord | undefined, + host: { + hostId: (tabId: string) => ExecutionHostId | undefined + session: (hostId: ExecutionHostId) => WorkspaceSessionState | null | undefined + relist: (pty: RuntimePtyWorktreeRecord, pane: RelistedPane) => void + } +): void { + if ( + !pty?.connected || + pty.ptyId !== authority.ptyId || + pty.incarnationId !== authority.incarnationId || + pty.worktreeId !== authority.worktreeId + ) { + return + } + const pane = parsePaneKey(pty.paneKey ?? '') + const hostId = pane && pane.tabId === pty.tabId ? host.hostId(pane.tabId) : undefined + if (!pane || !hostId) { + return + } + const session = host.session(hostId) + const layout = session?.terminalLayoutsByTabId?.[pane.tabId] + const incarnationId = + session?.terminalPtyIncarnationsByPaneKey?.[makePaneKey(pane.tabId, pane.leafId)] + if ( + layout?.ptyIdsByLeafId?.[pane.leafId] !== pty.ptyId || + !terminalLayoutContainsLeaf(layout.root, pane.leafId) || + (incarnationId && incarnationId !== pty.incarnationId) + ) { + return + } + host.relist(pty, { + tabId: pane.tabId, + leafId: pane.leafId, + title: null, + activate: false, + selectIfNoActiveTab: false, + notify: false + }) +} diff --git a/src/main/runtime/terminal/view/mobile-terminal-surface.ts b/src/main/runtime/terminal/view/mobile-terminal-surface.ts index de9633613e9..6338637cb15 100644 --- a/src/main/runtime/terminal/view/mobile-terminal-surface.ts +++ b/src/main/runtime/terminal/view/mobile-terminal-surface.ts @@ -1,4 +1,6 @@ /* eslint-disable unicorn/no-useless-spread */ +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' +import { relistPaneMainBinds } from './layout-owner-close-relisting' import { OrcaRuntimeWithCreateRuntimeOwnedMobileSessionTerminal } from '../launch/mobile-terminal-owned-create' import type { OrcaRuntimeWithResolveTerminalSplitSourceAuthority } from '../launch/terminal-split-source-authority' import type { @@ -135,6 +137,18 @@ export abstract class OrcaRuntimeWithWaitForMobileTerminalSurface extends OrcaRu ) { return null } + // Switch point: main binds the pane, so its own record stands while the window's list lags. + if ( + ORCA_LAYOUT_OWNER_IS_MAIN && + !this.findMobileTerminalSurfaceForPty(authority.worktreeId, authority.ptyId) + ) { + relistPaneMainBinds(authority, live.pty, { + hostId: (tabId) => this.getWorkspaceSessionHostIdsForTab(authority.worktreeId, tabId)[0], + session: (hostId) => this.store?.getWorkspaceSession?.(hostId), + relist: (pty, pane) => + this.publishPtyBackedMobileSessionTerminal(authority.worktreeId, pty, pane) + }) + } const surface = this.findMobileTerminalSurfaceForPty(authority.worktreeId, authority.ptyId) if (!surface) { return null diff --git a/tests/e2e/terminal-tab-close-restart-persistence.spec.ts b/tests/e2e/terminal-tab-close-restart-persistence.spec.ts index 20421dd1730..287317ec8cf 100644 --- a/tests/e2e/terminal-tab-close-restart-persistence.spec.ts +++ b/tests/e2e/terminal-tab-close-restart-persistence.spec.ts @@ -12,6 +12,7 @@ import { } from './helpers/store' import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart' import { worktreeRowSurface } from './worktree-row-locators' +import { readFreshTerminalInventory } from './helpers/terminal-inventory-observation' import { RuntimeClient } from '../../src/cli/runtime/client' import { RuntimeRpcFailureError } from '../../src/cli/runtime/types' import type { @@ -81,10 +82,17 @@ test('durable whole-tab close removes a split tab across restart', async (// oxl await expect .poll( async () => { - const listed = await client.call('terminal.list', { - worktree: `id:${worktreeId}` + // The pane's terminal can still be binding; the host then says it can't answer yet. + const listed = await readFreshTerminalInventory(async () => { + const response = await client.call('terminal.list', { + worktree: `id:${worktreeId}` + }) + return response.result }) - const matching = listed.result.terminals.filter( + if (!listed) { + return null + } + const matching = listed.terminals.filter( (terminal) => terminal.worktreeId === worktreeId && terminal.tabId === closedTabId ) activeHandle = matching.length === 1 ? (matching[0]?.handle ?? null) : null @@ -120,10 +128,13 @@ test('durable whole-tab close removes a split tab across restart', async (// oxl await expect .poll( async () => { - const afterClose = await client.call('terminal.list', { - worktree: `id:${worktreeId}` + const afterClose = await readFreshTerminalInventory(async () => { + const response = await client.call('terminal.list', { + worktree: `id:${worktreeId}` + }) + return response.result }) - return afterClose.result.terminals + return afterClose?.terminals .filter((terminal) => terminal.tabId === closedTabId) .map((terminal) => terminal.handle) },