From c353ef4a34b7d996c5b32dd485cd45928befeafd Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Fri, 16 Oct 2026 08:19:05 -0700 Subject: [PATCH] One write path for main's split panes, late-shown splits reported again (dark, build switch off by default) (#29723) * One pane-commit path for main's splits, a late-mounted split recorded again, and one fresh-pane exit rule (dark, build switch off by default) * Runtime panes write only touched records in the layout-owner build; main keeps a split the window already recorded * Admission patch that finds no change writes nothing, as layout.apply --- ...rminal-pane-admission-layout-owner.test.ts | 106 +++++++++++ .../terminal-pane-admission.ts | 20 ++- src/main/runtime/orca-runtime.ts | 3 + .../runtime/runtime-pane-admission.test.ts | 106 +++++++++++ src/main/runtime/runtime-pane-admission.ts | 85 ++++++--- .../terminal-split-layout-owner.test.ts | 165 +++++++++++------- .../launch/terminal-split-layout-owner.ts | 52 ------ .../runtime/terminal/launch/terminal-split.ts | 55 ++++-- .../pty-connection/pty-exit-hibernate.ts | 16 +- .../terminal-pane-bootstrap-split-record.ts | 28 ++- ...nal-pane-mount-events-layout-owner.test.ts | 101 +++++++++++ .../terminal-pane-mount-events.ts | 15 ++ 12 files changed, 582 insertions(+), 170 deletions(-) create mode 100644 src/main/persistence/terminal-topology/terminal-pane-admission-layout-owner.test.ts create mode 100644 src/main/runtime/runtime-pane-admission.test.ts delete mode 100644 src/main/runtime/terminal/launch/terminal-split-layout-owner.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-pane-mount-events-layout-owner.test.ts diff --git a/src/main/persistence/terminal-topology/terminal-pane-admission-layout-owner.test.ts b/src/main/persistence/terminal-topology/terminal-pane-admission-layout-owner.test.ts new file mode 100644 index 00000000000..678baf87e45 --- /dev/null +++ b/src/main/persistence/terminal-topology/terminal-pane-admission-layout-owner.test.ts @@ -0,0 +1,106 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import type { Store } from '../loading-store/store' +import { leafIds, WindowSession } from './terminal-topology-window-session-fixture' +import { + emptyTerminalSessionProfile, + FIXTURE_GIT_WORKTREE_ID, + openTopologyStore +} from './terminal-topology-profile-fixture' + +vi.mock('../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) +vi.mock('../../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../../ssh/ssh-config-parser', () => ({ + loadUserSshConfig: () => ({ hosts: [] }), + sshConfigHostsToTargets: () => [] +})) + +const WORKTREE = FIXTURE_GIT_WORKTREE_ID +const TAB = 'tab-1' +const LEAF = '11111111-1111-4111-8111-111111111111' +const NEW_LEAF = '22222222-2222-4222-8222-222222222222' +const directories: string[] = [] + +afterEach(() => { + for (const directory of directories.splice(0)) { + rmSync(directory, { recursive: true, force: true }) + } +}) + +/** One tab with leftovers a full load-and-save rewrite would drop or renumber. */ +async function storeWithLeftovers(): Promise { + const directory = mkdtempSync(join(tmpdir(), 'orca-pane-admission-')) + directories.push(directory) + const store = await openTopologyStore(directory, emptyTerminalSessionProfile()) + const window = new WindowSession(getDefaultWorkspaceSession()) + window.addTab(WORKTREE, TAB, LEAF) + window.bind(TAB, LEAF, 'pty-1') + window.session.terminalPtyIncarnationsByPaneKey = { [`${TAB}:${LEAF}`]: 'incarnation-1' } + window.session.tabsByWorktree[WORKTREE]![0]!.sortOrder = 7 + store.setWorkspaceSession(window.snapshot()) + return store +} + +const admission = { + type: 'splitPane', + workspace: WORKTREE, + tabId: TAB, + leafId: LEAF, + direction: 'vertical', + newLeafId: NEW_LEAF +} as const + +const root = (store: Store) => store.getWorkspaceSession().terminalLayoutsByTabId[TAB]?.root + +describe('a runtime pane in a layout-owner build', () => { + it('is written and taken back touching no other record', async () => { + const store = await storeWithLeftovers() + const stored = store.getWorkspaceSession() + + await expect(store.admitTerminalPane(admission)).resolves.toBe('admitted') + const admitted = store.getWorkspaceSession() + expect(leafIds(root(store))).toEqual([LEAF, NEW_LEAF]) + expect(admitted.terminalPtyIncarnationsByPaneKey).toEqual( + stored.terminalPtyIncarnationsByPaneKey + ) + expect(admitted.tabsByWorktree).toEqual(stored.tabsByWorktree) + + await store.withdrawTerminalPane({ + type: 'closePane', + workspace: WORKTREE, + tabId: TAB, + leafId: NEW_LEAF + }) + const withdrawn = store.getWorkspaceSession() + expect(leafIds(root(store))).toEqual([LEAF]) + expect(withdrawn.tabsByWorktree).toEqual(stored.tabsByWorktree) + expect(withdrawn.terminalPtyIncarnationsByPaneKey).toEqual( + stored.terminalPtyIncarnationsByPaneKey + ) + }) + + it('comes back when the window records it after main took it back, and only once', async () => { + const store = await storeWithLeftovers() + const { newLeafId: _newLeafId, ...split } = admission + const record = { ...split, newLeafId: NEW_LEAF } + await store.admitTerminalPane(admission) + + // Main still holds it: the window's record changes nothing. + await store.applyLayoutCommand(record) + expect(leafIds(root(store))).toEqual([LEAF, NEW_LEAF]) + + await store.withdrawTerminalPane({ + type: 'closePane', + workspace: WORKTREE, + tabId: TAB, + leafId: NEW_LEAF + }) + await store.applyLayoutCommand(record) + expect(leafIds(root(store))).toEqual([LEAF, NEW_LEAF]) + await store.applyLayoutCommand(record) + expect(leafIds(root(store))).toEqual([LEAF, NEW_LEAF]) + }) +}) diff --git a/src/main/persistence/terminal-topology/terminal-pane-admission.ts b/src/main/persistence/terminal-topology/terminal-pane-admission.ts index e1e626a9a50..0e999b7655c 100644 --- a/src/main/persistence/terminal-topology/terminal-pane-admission.ts +++ b/src/main/persistence/terminal-topology/terminal-pane-admission.ts @@ -29,6 +29,7 @@ import { followRemoteSessionAfterPaneDetach } from './pane-remote-session-follow' import { pruneClosedTerminalTabTombstones } from '../../../shared/closed-terminal-tab-tombstones' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../shared/layout-owner-switch' /** A new tab, or a split of an existing pane, with the ids its terminal will run under. */ export type TerminalPaneAdmission = @@ -52,7 +53,7 @@ export function admitPaneToSession( if (workspace && locatePane(workspace, paneKeyOf(admission.tabId, leafId))) { return { value: 'exists', session: null } } - const change = applyToSession(loaded, admission) + const change = applyToSession(loaded, admission, touchedRecordsWrite(session, loaded)) return { ...change, value: change.value.ok ? 'admitted' : change.value.code } } @@ -61,10 +62,25 @@ export function withdrawPaneFromSession( session: WorkspaceSessionState, pane: CommandOf<'closePane'> ): PaneLayoutChange { - const change = applyToSession(loadWorkspaceLayout(hostId, session, nameBasedLoadContext()), pane) + const loaded = loadWorkspaceLayout(hostId, session, nameBasedLoadContext()) + const change = applyToSession(loaded, pane, touchedRecordsWrite(session, loaded)) return { ...change, value: change.value.ok ? null : change.value.code } } +/** Switch point: in a layout-owner build a runtime pane writes only the records it changes, as + * `layout.apply` does; window-owned builds rewrite the whole partition. */ +function touchedRecordsWrite( + session: WorkspaceSessionState, + loaded: LoadedWorkspaceLayout +): ((after: WorkspaceSessionState) => WorkspaceSessionState | null) | undefined { + return ORCA_LAYOUT_OWNER_IS_MAIN + ? (after) => { + const patched = patchStoredSession(session, saveWorkspaceLayout(loaded), after) + return patched === session ? null : patched + } + : undefined +} + /** * A client's layout command (`layout.apply`), with main minting every new id. Only the records the * command changed are written; the rest of the partition stays as stored, normalized or not. diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 0b66f9ae498..000f6124475 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -172,6 +172,9 @@ class OrcaRuntimeService extends OrcaRuntimeWithMigrationCatalog { /** A window's layout command (`layout.apply`), applied to the partitions that hold its tab. */ async applyLayoutCommand(command: WindowLayoutCommand): Promise { + if (command.type === 'splitPane' && command.newLeafId) { + this.noteWindowMountedSplit(command.newLeafId) + } const reply = await applyWindowLayoutCommand( this.requireStore(), (worktreeId, tabId) => { diff --git a/src/main/runtime/runtime-pane-admission.test.ts b/src/main/runtime/runtime-pane-admission.test.ts new file mode 100644 index 00000000000..06c4ef988e1 --- /dev/null +++ b/src/main/runtime/runtime-pane-admission.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it, vi } from 'vitest' +import type { TerminalPaneAdmissionOutcome } from '../persistence/terminal-topology/terminal-pane-admission' +import { startInAdmittedPane } from './runtime-pane-admission' +import type { RuntimeStore } from './runtime-store-contract' + +const SPLIT = { + type: 'splitPane', + workspace: 'wt-1', + tabId: 'tab-1', + leafId: 'leaf-1', + direction: 'vertical', + newLeafId: 'leaf-new' +} as const + +function storeAnswering(admit: TerminalPaneAdmissionOutcome | Error) { + const store = { + admitTerminalPane: vi.fn(async () => { + if (admit instanceof Error) { + throw admit + } + return admit + }), + withdrawTerminalPane: vi.fn(async () => null) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the admission path calls only these two store methods. + return { store, runtimeStore: store as unknown as RuntimeStore } +} + +describe('startInAdmittedPane', () => { + it('writes the pane, then starts what fills it', async () => { + const { store, runtimeStore } = storeAnswering('admitted') + const start = vi.fn(async () => 'term-1') + + await expect(startInAdmittedPane(runtimeStore, SPLIT, 'ssh:target-1', start)).resolves.toEqual({ + outcome: 'admitted', + result: 'term-1' + }) + expect(store.admitTerminalPane).toHaveBeenCalledWith(SPLIT, 'ssh:target-1') + expect(start).toHaveBeenCalledWith('admitted') + }) + + it('takes back the pane it wrote when the start fails and no client shows it', async () => { + const { store, runtimeStore } = storeAnswering('admitted') + + await expect( + startInAdmittedPane(runtimeStore, SPLIT, undefined, async () => { + throw new Error('mount timed out') + }) + ).rejects.toThrow('mount timed out') + expect(store.withdrawTerminalPane).toHaveBeenCalledWith( + { type: 'closePane', workspace: 'wt-1', tabId: 'tab-1', leafId: 'leaf-new' }, + undefined + ) + }) + + it('keeps a pane a client already shows, and one it did not write', async () => { + const shown = storeAnswering('admitted') + await expect( + startInAdmittedPane( + shown.runtimeStore, + SPLIT, + undefined, + async () => { + throw new Error('terminal still starting') + }, + { shown: () => true } + ) + ).rejects.toThrow() + expect(shown.store.withdrawTerminalPane).not.toHaveBeenCalled() + + const existing = storeAnswering('exists') + await expect( + startInAdmittedPane(existing.runtimeStore, SPLIT, undefined, async () => { + throw new Error('spawn failed') + }) + ).rejects.toThrow() + expect(existing.store.withdrawTerminalPane).not.toHaveBeenCalled() + }) + + it('hands a refusal to the start, which decides', async () => { + const { runtimeStore } = storeAnswering('pane_not_found') + const start = vi.fn(async () => 'as-today') + + await expect(startInAdmittedPane(runtimeStore, SPLIT, undefined, start)).resolves.toEqual({ + outcome: 'pane_not_found', + result: 'as-today' + }) + }) + + it('starts without the pane after a failed write only when asked to', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + const failing = storeAnswering(new Error('disk full')) + const start = vi.fn(async () => 'as-today') + + await expect( + startInAdmittedPane(failing.runtimeStore, SPLIT, undefined, start) + ).rejects.toThrow('disk full') + expect(start).not.toHaveBeenCalled() + + await expect( + startInAdmittedPane(failing.runtimeStore, SPLIT, undefined, start, { + startIfWriteFails: true + }) + ).resolves.toEqual({ outcome: undefined, result: 'as-today' }) + }) +}) diff --git a/src/main/runtime/runtime-pane-admission.ts b/src/main/runtime/runtime-pane-admission.ts index 1867b869e8d..8c5125dbfaf 100644 --- a/src/main/runtime/runtime-pane-admission.ts +++ b/src/main/runtime/runtime-pane-admission.ts @@ -1,5 +1,8 @@ import { toSshExecutionHostId } from '../../shared/execution-host' -import type { TerminalPaneAdmission } from '../persistence/terminal-topology/terminal-pane-admission' +import type { + TerminalPaneAdmission, + TerminalPaneAdmissionOutcome +} from '../persistence/terminal-topology/terminal-pane-admission' import type { RuntimePtyController } from './runtime-pty-controller-contract' import type { RuntimeStore } from './runtime-store-contract' @@ -48,38 +51,80 @@ export async function withdrawRuntimePane( store: RuntimeStore | null | undefined, args: AdmittedSpawnArgs ): Promise { + await withdrawAdmittedPane(store, admissionFor(args), hostIdOf(args)) +} + +/** Never the user's close: main stops no launch for it, unlike a window's closePane. */ +async function withdrawAdmittedPane( + store: RuntimeStore | null | undefined, + admission: TerminalPaneAdmission, + hostId: string | undefined +): Promise { + const leafId = admission.type === 'createTerminalTab' ? admission.leafId : admission.newLeafId await store?.withdrawTerminalPane?.( - { type: 'closePane', workspace: args.worktreeId, tabId: args.tabId, leafId: args.leafId }, - hostIdOf(args) + { type: 'closePane', workspace: admission.workspace, tabId: admission.tabId, leafId }, + hostId ) } /** - * Starts a runtime-originated terminal (design 4.1): its pane, tab-bar entry and group are written - * first, then the process starts and binds to that pane. A start that fails, or that attaches to a - * session already running in another pane, takes back the pane it wrote. + * Writes a pane before what fills it starts (design 4.1), then runs `start`. A start that fails + * takes back a pane this wrote, unless `shown` says a client already holds it. + */ +export async function startInAdmittedPane( + store: RuntimeStore | null | undefined, + admission: TerminalPaneAdmission, + hostId: string | undefined, + start: (outcome: TerminalPaneAdmissionOutcome | undefined) => Promise, + options: { + shown?: () => boolean + /** A thrown write is rolled back, so the start may go ahead without the pane. */ + startIfWriteFails?: true + } = {} +): Promise<{ outcome: TerminalPaneAdmissionOutcome | undefined; result: T }> { + let outcome: TerminalPaneAdmissionOutcome | undefined + try { + outcome = await store?.admitTerminalPane?.(admission, hostId) + } catch (error) { + if (!options.startIfWriteFails) { + throw error + } + console.warn('[layout-owner] pane not written before its start:', error) + } + try { + return { outcome, result: await start(outcome) } + } catch (error) { + if (outcome === 'admitted' && !options.shown?.()) { + await withdrawAdmittedPane(store, admission, hostId) + } + throw error + } +} + +/** + * Starts a runtime-originated terminal: its pane, tab-bar entry and group are written first, then + * the process starts and binds to that pane. A start that fails, or that attaches to a session + * already running in another pane, takes back the pane it wrote. */ export async function spawnInAdmittedPane( store: RuntimeStore | null | undefined, controller: Pick, args: AdmittedSpawnArgs ): ReturnType { - const outcome = await store?.admitTerminalPane?.(admissionFor(args), hostIdOf(args)) - if (outcome !== undefined && outcome !== 'admitted' && outcome !== 'exists') { - // Only a split is refused: its parent pane left the layout. - throw new Error('terminal_split_source_not_found') - } const { groupId: _groupId, ...spawnArgs } = args - let result: Awaited> - try { - // SAFETY: Callers require spawn; a lost capability during admission keeps the original failure. - result = await controller.spawn!(spawnArgs) - } catch (error) { - if (outcome === 'admitted') { - await withdrawRuntimePane(store, args) + const { outcome, result } = await startInAdmittedPane( + store, + admissionFor(args), + hostIdOf(args), + async (admitted) => { + if (admitted !== undefined && admitted !== 'admitted' && admitted !== 'exists') { + // Only a split is refused: its parent pane left the layout. + throw new Error('terminal_split_source_not_found') + } + // SAFETY: Callers require spawn; a lost capability during admission keeps the original failure. + return controller.spawn!(spawnArgs) } - throw error - } + ) const started = result.agentSessionEnsure?.owner.surface ?? result.stablePaneOwner const elsewhere = started && (started.tabId !== args.tabId || started.leafId !== args.leafId) if (outcome === 'admitted' && elsewhere) { diff --git a/src/main/runtime/terminal/launch/terminal-split-layout-owner.test.ts b/src/main/runtime/terminal/launch/terminal-split-layout-owner.test.ts index f6d1a02371a..66e2d14355d 100644 --- a/src/main/runtime/terminal/launch/terminal-split-layout-owner.test.ts +++ b/src/main/runtime/terminal/launch/terminal-split-layout-owner.test.ts @@ -1,85 +1,116 @@ import { describe, expect, it, vi } from 'vitest' -import type { LayoutApplyReply } from '../../../../shared/workspace-layout/workspace-layout-command-types' -import type { WindowLayoutCommand } from '../../runtime-layout-apply' -import { splitWindowLeafThroughLayoutOwner } from './terminal-split-layout-owner' +import { OrcaRuntimeService, makePaneKey } from '../../orca-runtime-test-mocks.spec' +import { + HEADLESS_LEAF_ID, + TEST_WORKTREE_ID, + makeRuntimeStoreWithWorkspaceSession, + makeWorkspaceSessionWithHeadlessTerminal +} from '../../orca-runtime-test-fixtures.spec' +import { createMobileCreateTestNotifier } from '../../orca-runtime-test-scenario-builders.spec' +import { collectLayoutLeafIdsInOrder } from '../../../../shared/workspace-layout/terminal-pane-tree' -const LEAF = { worktreeId: 'wt-1', tabId: 'tab-1', leafId: 'leaf-1' } +vi.mock('../../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) -function deps( - reply: LayoutApplyReply | Error, - mount: (leafId: string) => Promise, - mounted = false -) { +const TAB_ID = 'host-tab' + +function graph(leafIds: string[], unboundLeafIds: string[] = []) { return { - apply: vi.fn(async (_command: WindowLayoutCommand): Promise => { - if (reply instanceof Error) { - throw reply + tabs: [ + { + tabId: TAB_ID, + worktreeId: TEST_WORKTREE_ID, + title: 'shell', + activeLeafId: leafIds[0]!, + layout: null } - return reply - }), - mintLeafId: () => 'minted-leaf', - mount: vi.fn(mount), - isMounted: () => mounted + ], + leaves: [...leafIds, ...unboundLeafIds].map((leafId, index) => ({ + tabId: TAB_ID, + worktreeId: TEST_WORKTREE_ID, + leafId, + paneRuntimeId: index + 1, + ptyId: index < leafIds.length ? `pty-${index}` : null, + paneTitle: null + })) } } -describe('splitWindowLeafThroughLayoutOwner', () => { - it('writes the split in main first, then has the window mount main’s pane', async () => { - const d = deps({ ok: true, result: { leafId: 'main-leaf' } }, async () => 'term_new') +describe('a split of a pane the window draws, in a layout-owner build', () => { + it('writes the new pane in main before the window mounts it, and takes back one never mounted', async () => { + const { runtimeStore, getSession } = makeRuntimeStoreWithWorkspaceSession( + makeWorkspaceSessionWithHeadlessTerminal() + ) + const notifier = createMobileCreateTestNotifier(vi.fn()) + const splitTerminal = vi.mocked(notifier.splitTerminal) + const runtime = new OrcaRuntimeService(runtimeStore) + runtime.setNotifier(notifier) + runtime.syncWindowGraph(1, graph([HEADLESS_LEAF_ID])) + const handle = runtime.getTerminalHandleForPaneKey(makePaneKey(TAB_ID, HEADLESS_LEAF_ID)) + const leafIds = () => + collectLayoutLeafIdsInOrder(getSession().terminalLayoutsByTabId[TAB_ID]!.root!) - const split = await splitWindowLeafThroughLayoutOwner(LEAF, 'vertical', d) + vi.useFakeTimers() + try { + const split = runtime.splitTerminal(handle!, { direction: 'vertical' }) + const rejected = expect(split).rejects.toThrow('Timed out waiting for split pane handle') + await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1)) + const newLeafId = splitTerminal.mock.calls[0]![2]!.newLeafId! + // Main holds the pane before the window has mounted it. + expect(leafIds()).toEqual([HEADLESS_LEAF_ID, newLeafId]) - expect(d.apply).toHaveBeenCalledWith({ - type: 'splitPane', - workspace: 'wt-1', - tabId: 'tab-1', - leafId: 'leaf-1', - direction: 'vertical' - }) - expect(d.mount).toHaveBeenCalledWith('main-leaf') - expect(split).toEqual({ handle: 'term_new', leafId: 'main-leaf' }) + await vi.advanceTimersByTimeAsync(11_000) + await rejected + expect(leafIds()).toEqual([HEADLESS_LEAF_ID]) + } finally { + vi.useRealTimers() + } }) it.each([ - ['refuses', { ok: false, code: 'tab_not_found' } as const], - ['throws', new Error('layout_store_unavailable')] - ])('splits as today under a fresh id when main %s', async (_name, reply) => { - const d = deps(reply, async () => 'term_new') - - const split = await splitWindowLeafThroughLayoutOwner(LEAF, 'horizontal', d) - - expect(d.mount).toHaveBeenCalledWith('minted-leaf') - expect(split.leafId).toBe('minted-leaf') - expect(d.apply).toHaveBeenCalledOnce() - }) - - it('takes main’s pane back when the window never mounts it', async () => { - const d = deps({ ok: true, result: { leafId: 'main-leaf' } }, async () => { - throw new Error('timeout') - }) - - await expect(splitWindowLeafThroughLayoutOwner(LEAF, 'vertical', d)).rejects.toThrow('timeout') - - expect(d.apply).toHaveBeenLastCalledWith({ - type: 'closePane', - workspace: 'wt-1', - tabId: 'tab-1', - leafId: 'main-leaf', - automatic: true - }) - }) - - it('keeps main’s pane when the window mounted it but its terminal is still starting', async () => { - const d = deps( - { ok: true, result: { leafId: 'main-leaf' } }, - async () => { - throw new Error('timeout') - }, - true + ['holds the pane while its terminal is still starting', 'graph'], + ['recorded the pane before its graph says so', 'record'] + ] as const)('keeps a pane main awaited when the window %s', async (_, shownBy) => { + const { runtimeStore, getSession } = makeRuntimeStoreWithWorkspaceSession( + makeWorkspaceSessionWithHeadlessTerminal() ) + const notifier = createMobileCreateTestNotifier(vi.fn()) + const splitTerminal = vi.mocked(notifier.splitTerminal) + const runtime = new OrcaRuntimeService(runtimeStore) + runtime.setNotifier(notifier) + runtime.syncWindowGraph(1, graph([HEADLESS_LEAF_ID])) + const handle = runtime.getTerminalHandleForPaneKey(makePaneKey(TAB_ID, HEADLESS_LEAF_ID)) - await expect(splitWindowLeafThroughLayoutOwner(LEAF, 'vertical', d)).rejects.toThrow('timeout') + vi.useFakeTimers() + try { + const split = runtime.splitTerminal(handle!, { direction: 'vertical' }) + const rejected = expect(split).rejects.toThrow('Timed out waiting for split pane handle') + await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1)) + const newLeafId = splitTerminal.mock.calls[0]![2]!.newLeafId! + if (shownBy === 'graph') { + // The window's session publication lists the new pane; this graph carries none. + Reflect.set(runtime, 'isMobileSessionSurfaceMembershipAllowed', () => true) + runtime.syncWindowGraph(1, graph([HEADLESS_LEAF_ID], [newLeafId])) + } else { + // This fixture store has no layout.apply; main notes the record before applying it. + await runtime + .applyLayoutCommand({ + type: 'splitPane', + workspace: TEST_WORKTREE_ID, + tabId: TAB_ID, + leafId: HEADLESS_LEAF_ID, + direction: 'vertical', + newLeafId + }) + .catch(() => {}) + } - expect(d.apply).toHaveBeenCalledOnce() + await vi.advanceTimersByTimeAsync(11_000) + await rejected + expect( + collectLayoutLeafIdsInOrder(getSession().terminalLayoutsByTabId[TAB_ID]!.root!) + ).toEqual([HEADLESS_LEAF_ID, newLeafId]) + } finally { + vi.useRealTimers() + } }) }) diff --git a/src/main/runtime/terminal/launch/terminal-split-layout-owner.ts b/src/main/runtime/terminal/launch/terminal-split-layout-owner.ts deleted file mode 100644 index fd91ad00641..00000000000 --- a/src/main/runtime/terminal/launch/terminal-split-layout-owner.ts +++ /dev/null @@ -1,52 +0,0 @@ -import type { LayoutApplyReply } from '../../../../shared/workspace-layout/workspace-layout-command-types' -import type { WindowLayoutCommand } from '../../runtime-layout-apply' - -type SplitLeaf = { worktreeId: string; tabId: string; leafId: string } - -/** - * A split of a pane the window draws, in a layout-owner build: main writes the new pane first - * and the window mounts that pane. Main's copy lacking the tab, or refusing, splits as today - * under a fresh id. A pane the window never mounted is taken back; one still binding its - * terminal (a slow SSH start) stays. - */ -export async function splitWindowLeafThroughLayoutOwner( - leaf: SplitLeaf, - direction: 'horizontal' | 'vertical', - deps: { - apply: (command: WindowLayoutCommand) => Promise - mintLeafId: () => string - /** Asks the window to mount the pane and waits for its terminal; the new pane's handle. */ - mount: (newLeafId: string) => Promise - /** Whether the window's graph holds the pane, bound or not. */ - isMounted: (newLeafId: string) => boolean - } -): Promise<{ handle: string; leafId: string }> { - const { worktreeId: workspace, tabId, leafId } = leaf - let reply: LayoutApplyReply | null = null - try { - reply = await deps.apply({ - type: 'splitPane', - workspace, - tabId, - leafId, - direction - }) - } catch (error) { - // A thrown write is rolled back; the window splits as today. - console.warn('[layout-owner] runtime split failed:', error) - } - const mainLeafId = reply?.ok ? reply.result.leafId : undefined - const newLeafId = mainLeafId ?? deps.mintLeafId() - try { - return { handle: await deps.mount(newLeafId), leafId: newLeafId } - } catch (error) { - if (mainLeafId && !deps.isMounted(mainLeafId)) { - await deps - .apply({ type: 'closePane', workspace, tabId, leafId: mainLeafId, automatic: true }) - .catch((withdrawError: unknown) => - console.warn('[layout-owner] unmounted split not withdrawn:', withdrawError) - ) - } - throw error - } -} diff --git a/src/main/runtime/terminal/launch/terminal-split.ts b/src/main/runtime/terminal/launch/terminal-split.ts index fb7b3f9a03c..f59741f7d8c 100644 --- a/src/main/runtime/terminal/launch/terminal-split.ts +++ b/src/main/runtime/terminal/launch/terminal-split.ts @@ -4,21 +4,28 @@ import { OrcaRuntimeWithStopExplicitlyClosedTabPtys } from '../../session-tabs/e import type { TerminalPaneSplitSource } from '../../../../shared/feature-education-telemetry' import type { RuntimeTerminalSplit } from '../../../../shared/runtime-types' import { randomUUID } from 'node:crypto' -import type { LayoutApplyReply } from '../../../../shared/workspace-layout/workspace-layout-command-types' +import { parseExecutionHostId } from '../../../../shared/execution-host' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' -import type { WindowLayoutCommand } from '../../runtime-layout-apply' -import { splitWindowLeafThroughLayoutOwner } from './terminal-split-layout-owner' +import { startInAdmittedPane } from '../../runtime-pane-admission' import type { Worktree } from '../../../../shared/worktree/types' export abstract class OrcaRuntimeWithSplitTerminal extends OrcaRuntimeWithStopExplicitlyClosedTabPtys { + /** Pane id of each split main wrote and awaits → whether the window already recorded it. */ + private readonly awaitedSplitLeafIds = new Map() + + /** The window recorded a split it mounted: one main awaits counts as shown before its graph says so. */ + protected noteWindowMountedSplit(leafId: string): void { + if (this.awaitedSplitLeafIds.has(leafId)) { + this.awaitedSplitLeafIds.set(leafId, true) + } + } + protected abstract splitPtyBackedTerminal( pty: RuntimePtyWorktreeRecord, opts?: TerminalSplitOptions, createdWorktree?: Worktree ): Promise - abstract applyLayoutCommand(command: WindowLayoutCommand): Promise - protected abstract waitForLeafInTab( tabId: string, leafId: string, @@ -49,11 +56,25 @@ export abstract class OrcaRuntimeWithSplitTerminal extends OrcaRuntimeWithStopEx const { leaf } = this.getLiveLeafForHandle(handle) const direction = opts.direction ?? 'horizontal' // Switch point: main writes the split first; the window then mounts main's pane. - if (ORCA_LAYOUT_OWNER_IS_MAIN) { - const split = await splitWindowLeafThroughLayoutOwner(leaf, direction, { - apply: (command) => this.applyLayoutCommand(command), - mintLeafId: randomUUID, - mount: (newLeafId) => { + const [hostId] = ORCA_LAYOUT_OWNER_IS_MAIN + ? this.getWorkspaceSessionHostIdsForTab(leaf.worktreeId, leaf.tabId) + : [] + if (hostId && parseExecutionHostId(hostId)?.kind !== 'runtime') { + const newLeafId = randomUUID() + this.awaitedSplitLeafIds.set(newLeafId, false) + const { result: newHandle } = await startInAdmittedPane( + this.store, + { + type: 'splitPane', + workspace: leaf.worktreeId, + tabId: leaf.tabId, + leafId: leaf.leafId, + direction, + newLeafId + }, + hostId, + // A refused or failed write splits as today; the window records the pane it mounts. + () => { this.notifier?.splitTerminal(leaf.tabId, leaf.paneRuntimeId, { direction, command: opts.command, @@ -64,13 +85,19 @@ export abstract class OrcaRuntimeWithSplitTerminal extends OrcaRuntimeWithStopEx }) return this.waitForLeafInTab(leaf.tabId, newLeafId) }, - isMounted: (newLeafId) => this.leaves.has(this.getLeafKey(leaf.tabId, newLeafId)) - }) + { + // A pane the window holds but whose terminal is still starting (a slow SSH start) stays. + shown: () => + this.awaitedSplitLeafIds.get(newLeafId) === true || + this.leaves.has(this.getLeafKey(leaf.tabId, newLeafId)), + startIfWriteFails: true + } + ).finally(() => this.awaitedSplitLeafIds.delete(newLeafId)) return { - handle: split.handle, + handle: newHandle, tabId: leaf.tabId, paneRuntimeId: leaf.paneRuntimeId, - leafId: split.leafId + leafId: newLeafId } } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts b/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts index 03f79613c44..9d9ac21743d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pty-exit-hibernate.ts @@ -32,6 +32,11 @@ export function noteArmsHibernatedPaneWake(record: SleepingAgentSessionRecord): ) } +/** A tab's sole pane keeps a dead terminal it freshly spawned and the user never typed into. */ +function keepsUntouchedFreshPane(session: ConnectPanePtySession, ptyId: string): boolean { + return session.spawnedFreshPtyId === ptyId && !Number.isFinite(session.lastTerminalInputAt) +} + /** PTY exit handling, hibernated-pane wake targets, and post-exit focus transfer. */ export function installPtyExitHibernate(session: ConnectPanePtySession): void { type ProcessExitState = { @@ -323,7 +328,7 @@ export function installPtyExitHibernate(session: ConnectPanePtySession): void { // for this ptyId — reattach/coldRestore skip it) that the user never typed // into, so a reattached-dead session or an explicit `exit` still tears // down as before. - if (session.spawnedFreshPtyId === ptyId && !Number.isFinite(session.lastTerminalInputAt)) { + if (keepsUntouchedFreshPane(session, ptyId)) { return } session.deps.onPtyExitRef.current(ptyId, exitCode) @@ -354,14 +359,9 @@ export function installPtyExitHibernate(session: ConnectPanePtySession): void { manager: session.manager, paneId: session.pane.id, closeTab: () => { - // The sole-pane rule above: a fresh terminal the user never typed into keeps its pane. - if ( - session.spawnedFreshPtyId === ptyId && - !Number.isFinite(session.lastTerminalInputAt) - ) { - return + if (!keepsUntouchedFreshPane(session, ptyId)) { + session.deps.onPtyExitRef.current(ptyId, exitCode) } - session.deps.onPtyExitRef.current(ptyId, exitCode) } }) ) { diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-bootstrap-split-record.ts b/src/renderer/src/components/terminal-pane/terminal-pane-bootstrap-split-record.ts index 914eafea0a3..751c8c65be3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-bootstrap-split-record.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-bootstrap-split-record.ts @@ -24,15 +24,29 @@ export function recordingBootstrapSplits( const sourceLeafId = manager.getLeafId(paneId) const created = manager.splitPane(paneId, direction, options) if (created && sourceLeafId) { - recordLayoutApply(useAppStore.getState(), worktreeId, { - type: 'splitPane', - tabId, - leafId: sourceLeafId, - direction, - newLeafId: created.leafId - }) + recordDrawnPaneSplit(worktreeId, tabId, sourceLeafId, direction, created.leafId) } return created } } } + +/** + * Records a split the window drew under its pane id. Main finds the pane it already holds and + * changes nothing, or writes it again (one main took back while the window mounted it late). + */ +export function recordDrawnPaneSplit( + worktreeId: string, + tabId: string, + sourceLeafId: string, + direction: 'vertical' | 'horizontal', + newLeafId: string +): void { + recordLayoutApply(useAppStore.getState(), worktreeId, { + type: 'splitPane', + tabId, + leafId: sourceLeafId, + direction, + newLeafId + }) +} diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-mount-events-layout-owner.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-mount-events-layout-owner.test.ts new file mode 100644 index 00000000000..b9c086015b1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-mount-events-layout-owner.test.ts @@ -0,0 +1,101 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import type { PtyConnectionDeps } from './pty-connection-types' +import { installTerminalPaneMountEvents } from './terminal-pane-mount-events' +import { + _resetTerminalPaneSplitRequestRoutingForTests, + dispatchTerminalPaneSplitRequest +} from './terminal-pane-split-request-routing' + +vi.mock('../../../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) +const recordLayoutApply = vi.hoisted(() => vi.fn()) +vi.mock('@/runtime/layout-apply-send', () => ({ recordLayoutApply })) +const storeState = vi.hoisted(() => ({ recordFeatureInteraction: () => {} })) +vi.mock('@/store', () => ({ useAppStore: { getState: () => storeState } })) +vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync: vi.fn() })) + +const SOURCE = '11111111-1111-4111-8111-111111111111' +const NEW = '22222222-2222-4222-8222-222222222222' + +function mountedTab(): { manager: PaneManager; splitPane: ReturnType } { + const leafById = new Map([[1, SOURCE]]) + const splitPane = vi.fn((_paneId: number, _direction: string, opts?: { leafId?: string }) => { + const leafId = opts?.leafId ?? 'window-minted' + leafById.set(2, leafId) + return { id: 2, leafId } + }) + const manager = { + splitPane, + getLeafId: (paneId: number) => leafById.get(paneId) ?? null, + getNumericIdForLeaf: (leafId: string) => + [...leafById].find(([, id]) => id === leafId)?.[0] ?? null + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the split handler calls only these manager methods. + return { manager: manager as unknown as PaneManager, splitPane } +} + +function install(manager: PaneManager): () => void { + return installTerminalPaneMountEvents({ + manager, + deps: { + tabId: 'tab-1', + worktreeId: 'wt-1', + isActive: true, + managerRef: { current: manager }, + persistLayoutSnapshot: vi.fn(), + syncCanExpandState: vi.fn(), + queueResizeAll: vi.fn() + }, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a split with no startup command reads no pty deps. + ptyDeps: {} as PtyConnectionDeps + }) +} + +describe('a split main asked for, mounted in a layout-owner build', () => { + afterEach(() => { + recordLayoutApply.mockReset() + _resetTerminalPaneSplitRequestRoutingForTests() + }) + + it('records the mounted pane in main, so a pane main took back meanwhile returns', () => { + const { manager, splitPane } = mountedTab() + const uninstall = install(manager) + + dispatchTerminalPaneSplitRequest({ + tabId: 'tab-1', + worktreeId: 'wt-1', + paneRuntimeId: 1, + sourceLeafId: SOURCE, + direction: 'vertical', + newLeafId: NEW + }) + + expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { leafId: NEW }) + expect(recordLayoutApply).toHaveBeenCalledWith(storeState, 'wt-1', { + type: 'splitPane', + tabId: 'tab-1', + leafId: SOURCE, + direction: 'vertical', + newLeafId: NEW + }) + uninstall() + }) + + it('records nothing for a split that names no pane id', () => { + const { manager } = mountedTab() + const uninstall = install(manager) + + dispatchTerminalPaneSplitRequest({ + tabId: 'tab-1', + worktreeId: 'wt-1', + paneRuntimeId: 1, + sourceLeafId: SOURCE, + direction: 'horizontal' + }) + + expect(recordLayoutApply).not.toHaveBeenCalled() + uninstall() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-mount-events.ts b/src/renderer/src/components/terminal-pane/terminal-pane-mount-events.ts index 71d68cd9db6..f6e4ec66d45 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-mount-events.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-mount-events.ts @@ -15,6 +15,8 @@ import { resolveTerminalPaneSplitSourceId } from './terminal-pane-split-request-routing' import type { PtyConnectionDeps } from './pty-connection-types' +import { recordDrawnPaneSplit } from './terminal-pane-bootstrap-split-record' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' export function installTerminalPaneMountEvents(args: { manager: PaneManager @@ -47,6 +49,12 @@ export function installTerminalPaneMountEvents(args: { if (sourcePaneId < 0) { return } + const recordMainSplitMounted = (leafId: string | undefined): void => { + const sourceLeafId = mgr.getLeafId(sourcePaneId) + if (leafId && leafId === detail.newLeafId && sourceLeafId) { + recordDrawnPaneSplit(deps.worktreeId, deps.tabId, sourceLeafId, detail.direction, leafId) + } + } const splitOptions = { ...(detail.newLeafId ? { leafId: detail.newLeafId } : {}), ...(detail.ptyId ? { ptyId: detail.ptyId } : {}) @@ -55,12 +63,19 @@ export function installTerminalPaneMountEvents(args: { const createdPane = splitPaneWithOneShotStartup(ptyDeps, { command: detail.command }, () => mgr.splitPane(sourcePaneId, detail.direction, splitOptions) ) + // Switch point: main may have taken back a pane the window mounted late; record it again. + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + recordMainSplitMounted(createdPane?.leafId) + } recordRuntimeCreatedTerminalPaneSplit(createdPane, { source: detail.telemetrySource ?? 'command', direction: detail.direction }) } else { const createdPane = mgr.splitPane(sourcePaneId, detail.direction, splitOptions) + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + recordMainSplitMounted(createdPane?.leafId) + } const telemetrySuppressed = createdPane ? consumePendingWebRuntimeSplitMirrorTelemetry(detail.sourcePtyId, detail.direction) : false