diff --git a/src/main/ipc/renderer-shutdown-checkpoint.ts b/src/main/ipc/renderer-shutdown-checkpoint.ts index 82fcbee0d4e..4a79b62adc5 100644 --- a/src/main/ipc/renderer-shutdown-checkpoint.ts +++ b/src/main/ipc/renderer-shutdown-checkpoint.ts @@ -4,6 +4,8 @@ import type { PersistedUIState } from '../../shared/persisted-ui-state-types' import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' import type { Store } from '../persistence' import { isFrozenOrcadSourceSessionPartition } from '../ssh/orcad-retained-source' +import { windowSaveForMain } from '../persistence/window-save-pane-trees' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../shared/layout-owner-switch' type StageBeforeUnloadSyncArgs = { sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[] @@ -51,7 +53,13 @@ export function registerRendererShutdownCheckpointHandler(store: Store): void { try { for (const { state, hostId } of args.sessions) { if (!isFrozenOrcadSourceSessionPartition(store, hostId)) { - store.stageWorkspaceSessionBeforeUnload(state, hostId) + store.stageWorkspaceSessionBeforeUnload( + // Switch point: main keeps its own pane trees over the window's. + ORCA_LAYOUT_OWNER_IS_MAIN + ? windowSaveForMain(store.getWorkspaceSession(hostId), state, hostId) + : state, + hostId + ) } } store.updateUI(args.ui) diff --git a/src/main/ipc/session.ts b/src/main/ipc/session.ts index 9fe987bd622..790ca07dc05 100644 --- a/src/main/ipc/session.ts +++ b/src/main/ipc/session.ts @@ -4,6 +4,8 @@ import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { parseTerminalSurfaceCloseTarget } from '../../shared/terminal-surface-close-target' import { isFrozenOrcadSourceSessionPartition } from '../ssh/orcad-retained-source' import { markAgentLaunchesClosedByUser } from '../agent-launch/agent-launch-pane-attachment' +import { windowSaveForMain } from '../persistence/window-save-pane-trees' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../shared/layout-owner-switch' import type { WorkspaceSessionPatch, WorkspaceSessionState @@ -30,13 +32,24 @@ export function registerSessionHandlers(store: Store, runtime: OrcaRuntimeServic ipcMain.handle('session:set', (_event, args: WorkspaceSessionState, hostId?: string | null) => { if (!isFenced(hostId)) { - store.setWorkspaceSession(args, hostId) + // Switch point: main keeps its own pane trees over the window's. + store.setWorkspaceSession( + ORCA_LAYOUT_OWNER_IS_MAIN + ? windowSaveForMain(store.getWorkspaceSession(hostId), args, hostId) + : args, + hostId + ) } }) ipcMain.handle('session:patch', (_event, args: WorkspaceSessionPatch, hostId?: string | null) => { if (!isFenced(hostId)) { - store.patchWorkspaceSession(args, hostId) + store.patchWorkspaceSession( + ORCA_LAYOUT_OWNER_IS_MAIN + ? windowSaveForMain(store.getWorkspaceSession(hostId), args, hostId) + : args, + hostId + ) } }) 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 index 678baf87e45..cfaec407e8a 100644 --- 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 @@ -82,7 +82,7 @@ describe('a runtime pane in a layout-owner build', () => { ) }) - it('comes back when the window records it after main took it back, and only once', async () => { + it('comes back when the window records it after it was taken back, and only once', async () => { const store = await storeWithLeftovers() const { newLeafId: _newLeafId, ...split } = admission const record = { ...split, newLeafId: 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 0e999b7655c..856fe66238b 100644 --- a/src/main/persistence/terminal-topology/terminal-pane-admission.ts +++ b/src/main/persistence/terminal-topology/terminal-pane-admission.ts @@ -73,12 +73,17 @@ 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 + return ORCA_LAYOUT_OWNER_IS_MAIN ? (after) => touchedRecords(session, loaded, after) : undefined +} + +/** The stored partition with only the records the change touched rewritten; null when none. */ +function touchedRecords( + session: WorkspaceSessionState, + loaded: LoadedWorkspaceLayout, + after: WorkspaceSessionState +): WorkspaceSessionState | null { + const patched = patchStoredSession(session, saveWorkspaceLayout(loaded), after) + return patched === session ? null : patched } /** @@ -101,13 +106,13 @@ export function applyCommandToSession( return { value: taken, session: null } } return applyToSession(loaded, command, (after) => { - const patched = patchStoredSession(session, saveWorkspaceLayout(loaded), after) - return patched === session - ? null - : followRemoteSessionAfterPaneDetach( + const patched = touchedRecords(session, loaded, after) + return patched + ? followRemoteSessionAfterPaneDetach( command, followRemoteSessionAfterPaneClose(command, session, prunedOnRecord(command, patched)) ) + : null }) } diff --git a/src/main/persistence/window-save-pane-trees.test.ts b/src/main/persistence/window-save-pane-trees.test.ts new file mode 100644 index 00000000000..f5dac3bbdbb --- /dev/null +++ b/src/main/persistence/window-save-pane-trees.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from 'vitest' +import type { TerminalLayoutSnapshot } from '../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { windowSaveForMain } from './window-save-pane-trees' + +const A = '11111111-1111-4111-8111-111111111111' +const B = '22222222-2222-4222-8222-222222222222' + +const mains: TerminalLayoutSnapshot = { + root: { + type: 'split', + direction: 'vertical', + ratio: 0.3, + first: { type: 'leaf', leafId: A }, + second: { type: 'leaf', leafId: B } + }, + activeLeafId: A, + expandedLeafId: null +} + +function stored(layouts: Record): WorkspaceSessionState { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: windowSaveForMain reads only terminalLayoutsByTabId. + return { terminalLayoutsByTabId: layouts } as WorkspaceSessionState +} + +describe('windowSaveForMain', () => { + it('keeps main’s pane tree, and the window’s focus, zoom and terminals', () => { + const windows: TerminalLayoutSnapshot = { + root: { type: 'leaf', leafId: A }, + activeLeafId: B, + expandedLeafId: B, + ptyIdsByLeafId: { [A]: 'pty-a' } + } + + const save = windowSaveForMain( + stored({ 'tab-1': mains }), + { terminalLayoutsByTabId: { 'tab-1': windows } }, + 'local' + ) + + expect(save.terminalLayoutsByTabId?.['tab-1']).toEqual({ ...windows, root: mains.root }) + }) + + it('keeps the window’s tree for a tab main holds no tree of', () => { + const windows: TerminalLayoutSnapshot = { + root: { type: 'leaf', leafId: A }, + activeLeafId: A, + expandedLeafId: null + } + const save = { terminalLayoutsByTabId: { 'tab-new': windows } } + + expect(windowSaveForMain(stored({ 'tab-1': mains }), save, 'local')).toBe(save) + }) + + it('passes a save with no pane layouts, or matching trees, through unchanged', () => { + const noLayouts = { activeWorktreeId: 'wt-1' } + expect(windowSaveForMain(stored({ 'tab-1': mains }), noLayouts, 'local')).toBe(noLayouts) + + const same = { terminalLayoutsByTabId: { 'tab-1': { ...mains, activeLeafId: B } } } + expect(windowSaveForMain(stored({ 'tab-1': mains }), same, 'local')).toBe(same) + }) + + it('keeps the window’s trees in a paired server’s partition, where main applies no pane command', () => { + const save = { + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf' as const, leafId: A }, + activeLeafId: A, + expandedLeafId: null + } + } + } + + expect(windowSaveForMain(stored({ 'tab-1': mains }), save, 'runtime:env-1')).toBe(save) + expect(windowSaveForMain(stored({ 'tab-1': mains }), save, 'ssh:target-1')).not.toBe(save) + }) +}) diff --git a/src/main/persistence/window-save-pane-trees.ts b/src/main/persistence/window-save-pane-trees.ts new file mode 100644 index 00000000000..69e35d92688 --- /dev/null +++ b/src/main/persistence/window-save-pane-trees.ts @@ -0,0 +1,42 @@ +// A window's session save in a layout-owner build: every pane change reaches main as a layout +// command, so the save's pane trees (which panes a tab has, their shape and sizes) are dropped for +// tabs main holds, and main keeps its own. The rest of each pane layout stays the window's: which +// pane is focused or zoomed, its terminals and scrollback. A tab main does not hold yet keeps the +// window's tree, as does a paired server's partition: main applies no pane command there. + +import { parseExecutionHostId } from '../../shared/execution-host' +import type { TerminalLayoutSnapshot } from '../../shared/terminal-tab-types' +import type { + WorkspaceSessionPatch, + WorkspaceSessionState +} from '../../shared/workspace-session-state-types' + +function withStoredPaneTrees( + stored: Readonly> | undefined, + incoming: Record +): Record { + let result = incoming + for (const [tabId, layout] of Object.entries(incoming)) { + const root = stored?.[tabId]?.root + if (root && layout.root !== root) { + result = result === incoming ? { ...incoming } : result + result[tabId] = { ...layout, root } + } + } + return result +} + +/** The save main applies, in a layout-owner build (the IPC entries call it behind the switch). */ +export function windowSaveForMain( + stored: WorkspaceSessionState, + save: T, + hostId: string | null | undefined +): T { + if (!save.terminalLayoutsByTabId || parseExecutionHostId(hostId)?.kind === 'runtime') { + return save + } + const layouts = withStoredPaneTrees(stored.terminalLayoutsByTabId, save.terminalLayoutsByTabId) + return layouts === save.terminalLayoutsByTabId + ? save + : { ...save, terminalLayoutsByTabId: layouts } +} diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 000f6124475..0b66f9ae498 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -172,9 +172,6 @@ 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 index 06c4ef988e1..2934df01494 100644 --- a/src/main/runtime/runtime-pane-admission.test.ts +++ b/src/main/runtime/runtime-pane-admission.test.ts @@ -87,8 +87,7 @@ describe('startInAdmittedPane', () => { }) }) - it('starts without the pane after a failed write only when asked to', async () => { - vi.spyOn(console, 'warn').mockImplementation(() => {}) + it('starts nothing after a failed write', async () => { const failing = storeAnswering(new Error('disk full')) const start = vi.fn(async () => 'as-today') @@ -96,11 +95,5 @@ describe('startInAdmittedPane', () => { 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 8c5125dbfaf..64522e48a7d 100644 --- a/src/main/runtime/runtime-pane-admission.ts +++ b/src/main/runtime/runtime-pane-admission.ts @@ -76,21 +76,9 @@ export async function startInAdmittedPane( 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 - } = {} + options: { shown?: () => boolean } = {} ): 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) - } + const outcome = await store?.admitTerminalPane?.(admission, hostId) try { return { outcome, result: await start(outcome) } } catch (error) { 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 66e2d14355d..71de55cdaf1 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 @@ -36,27 +36,64 @@ function graph(leafIds: string[], unboundLeafIds: string[] = []) { } 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() - ) + function setup() { + const store = makeRuntimeStoreWithWorkspaceSession(makeWorkspaceSessionWithHeadlessTerminal()) const notifier = createMobileCreateTestNotifier(vi.fn()) - const splitTerminal = vi.mocked(notifier.splitTerminal) - const runtime = new OrcaRuntimeService(runtimeStore) + const runtime = new OrcaRuntimeService(store.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!) + collectLayoutLeafIdsInOrder(store.getSession().terminalLayoutsByTabId[TAB_ID]!.root!) + return { + ...store, + runtime, + handle: handle!, + splitTerminal: vi.mocked(notifier.splitTerminal), + leafIds + } + } + + it('writes the new pane in main before the window mounts it, and keeps it however the start goes', async () => { + const { runtime, runtimeStore, handle, splitTerminal, leafIds } = setup() + const withdraw = vi.fn(async () => null) + Reflect.set(runtimeStore, 'withdrawTerminalPane', withdraw) vi.useFakeTimers() try { - const split = runtime.splitTerminal(handle!, { direction: 'vertical' }) + const split = runtime.splitTerminal(handle, { direction: 'vertical', command: 'make test' }) + const rejected = expect(split).rejects.toThrow('Timed out waiting for split pane handle') + await vi.waitFor(() => expect(splitTerminal).toHaveBeenCalledTimes(1)) + const notice = splitTerminal.mock.calls[0]![2]! + expect(notice).toMatchObject({ command: 'make test', sourceLeafId: HEADLESS_LEAF_ID }) + expect(leafIds()).toEqual([HEADLESS_LEAF_ID, notice.newLeafId]) + + // Its terminal never starts: main keeps a dead pane, as any, and takes nothing back. + await vi.advanceTimersByTimeAsync(11_000) + await rejected + expect(leafIds()).toEqual([HEADLESS_LEAF_ID, notice.newLeafId]) + expect(withdraw).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it('closes a CLI split before its terminal starts, and nothing brings the pane back', async () => { + const { runtime, handle, splitTerminal, leafIds } = setup() + + 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]) + + await runtime.closeTerminalSurfaceFromRenderer({ + worktreeId: TEST_WORKTREE_ID, + target: { kind: 'pane', tabId: TAB_ID, leafId: newLeafId }, + reason: 'user' + }) + expect(leafIds()).toEqual([HEADLESS_LEAF_ID]) await vi.advanceTimersByTimeAsync(11_000) await rejected @@ -66,51 +103,18 @@ describe('a split of a pane the window draws, in a layout-owner build', () => { } }) - it.each([ - ['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() + it('splits nothing when main cannot write the pane', async () => { + const { runtime, runtimeStore, handle, splitTerminal, leafIds } = setup() + Reflect.set( + runtimeStore, + 'admitTerminalPane', + vi.fn(async () => 'pane_not_found') ) - 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)) - 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(() => {}) - } - - await vi.advanceTimersByTimeAsync(11_000) - await rejected - expect( - collectLayoutLeafIdsInOrder(getSession().terminalLayoutsByTabId[TAB_ID]!.root!) - ).toEqual([HEADLESS_LEAF_ID, newLeafId]) - } finally { - vi.useRealTimers() - } + await expect(runtime.splitTerminal(handle, { direction: 'vertical' })).rejects.toThrow( + 'split_not_written: pane_not_found' + ) + expect(splitTerminal).not.toHaveBeenCalled() + expect(leafIds()).toEqual([HEADLESS_LEAF_ID]) }) }) diff --git a/src/main/runtime/terminal/launch/terminal-split.ts b/src/main/runtime/terminal/launch/terminal-split.ts index f59741f7d8c..6903d19927e 100644 --- a/src/main/runtime/terminal/launch/terminal-split.ts +++ b/src/main/runtime/terminal/launch/terminal-split.ts @@ -10,16 +10,6 @@ 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, @@ -56,48 +46,46 @@ 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. - 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, { + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + const [hostId] = this.getWorkspaceSessionHostIdsForTab(leaf.worktreeId, leaf.tabId) + if (hostId && parseExecutionHostId(hostId)?.kind !== 'runtime') { + const newLeafId = randomUUID() + const { result: newHandle } = await startInAdmittedPane( + this.store, + { + type: 'splitPane', + workspace: leaf.worktreeId, + tabId: leaf.tabId, + leafId: leaf.leafId, direction, - command: opts.command, - worktreeId: leaf.worktreeId, - sourceLeafId: leaf.leafId, - telemetrySource: opts.telemetrySource, newLeafId - }) - return this.waitForLeafInTab(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 + }, + hostId, + // No pane written, no split: nothing else would put it in main's copy. + (outcome) => { + if (outcome !== 'admitted' && outcome !== 'exists') { + throw new Error(`split_not_written: ${outcome ?? 'unknown'}`) + } + this.notifier?.splitTerminal(leaf.tabId, leaf.paneRuntimeId, { + direction, + command: opts.command, + worktreeId: leaf.worktreeId, + sourceLeafId: leaf.leafId, + telemetrySource: opts.telemetrySource, + newLeafId + }) + return this.waitForLeafInTab(leaf.tabId, newLeafId) + }, + // Main keeps the pane however its start goes. One whose terminal never starts is a dead + // pane, as any. + { shown: () => true } + ) + return { + handle: newHandle, + tabId: leaf.tabId, + paneRuntimeId: leaf.paneRuntimeId, + leafId: newLeafId } - ).finally(() => this.awaitedSplitLeafIds.delete(newLeafId)) - return { - handle: newHandle, - tabId: leaf.tabId, - paneRuntimeId: leaf.paneRuntimeId, - leafId: newLeafId } } diff --git a/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts b/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts index a664ea69c74..1eb3a0040ec 100644 --- a/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts +++ b/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts @@ -1,20 +1,28 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { panesLeftAfterPendingCloses } from '@/runtime/layout-owner-pane-intents' /** * Main refused a pane close as its last pane. An earlier close in flight took the sibling: once - * it lands here too, the tab closes. More panes here than main has: main's copy lags, so close as - * today. An unmounted tab is left alone. + * it lands here too, or is about to, the tab closes. More panes here than main has: main's copy + * lags, so close as today. An unmounted tab is left alone. */ export function closeAfterLastPaneRefusal( + tabId: string, manager: PaneManager | null, closeTab: () => void, closePaneAsToday: (paneId: number) => void, paneId: number ): void { - const panes = manager?.getPanes().length - if (panes === 1) { + if (!manager) { + return + } + const panes = panesLeftAfterPendingCloses( + tabId, + manager.getPanes().map((pane) => pane.leafId) + ) + if (panes <= 1) { closeTab() - } else if (panes) { + } else { closePaneAsToday(paneId) } } diff --git a/src/renderer/src/components/terminal-pane/layout-owner-pane-close-command.ts b/src/renderer/src/components/terminal-pane/layout-owner-pane-close-command.ts new file mode 100644 index 00000000000..26fda417b03 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/layout-owner-pane-close-command.ts @@ -0,0 +1,78 @@ +import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' +import { layoutFramesFlowing } from '@/runtime/layout-frame-position' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { expectPaneToClose, type PaneCloseKind } from '@/runtime/layout-owner-pane-intents' +import { closeAfterLastPaneRefusal } from './layout-owner-last-pane-refusal' + +/** + * A pane close in a layout-owner build: main closes the pane first, and the window removes it when + * main's frame drops it (the frame reconciler runs the close `kind` names). False when a paired + * server owns the workspace; the caller closes as today. + */ +export function sendPaneCloseToLayoutOwner( + pane: { worktreeId: string; tabId: string; leafId: string; kind: PaneCloseKind }, + on: { + /** Main cannot close it: close as today. */ + asToday: () => void + /** Main refused the pane as its last. */ + lastPane: () => void + /** Main closed it but no frame is coming (the stream is down): close it here. */ + closeHere: () => void + } +): boolean { + const { tabId, leafId } = pane + const forget = expectPaneToClose(tabId, leafId, pane.kind) + const sent = sendLayoutCommandToOwner( + pane.worktreeId, + { type: 'closePane', tabId, leafId, ...(pane.kind === 'exit' && { automatic: true }) }, + { + apply: () => { + if (!layoutFramesFlowing()) { + forget() + on.closeHere() + } + }, + asToday: () => { + forget() + on.asToday() + }, + refused: (code) => { + forget() + if (code === 'last_pane') { + on.lastPane() + } + } + } + ) + if (!sent) { + forget() + } + return sent +} + +/** The user's close of a pane, through main. */ +export function sendUserPaneCloseToLayoutOwner( + pane: { worktreeId: string; tabId: string; leafId: string; paneId: number }, + tab: { + getManager: () => PaneManager | null + closeTab: () => void + closePaneLocally: (paneId: number, committedLeafId?: string) => void + } +): boolean { + const { tabId, leafId, paneId } = pane + return sendPaneCloseToLayoutOwner( + { worktreeId: pane.worktreeId, tabId, leafId, kind: 'user' }, + { + asToday: () => tab.closePaneLocally(paneId), + lastPane: () => + closeAfterLastPaneRefusal( + tabId, + tab.getManager(), + tab.closeTab, + tab.closePaneLocally, + paneId + ), + closeHere: () => tab.closePaneLocally(paneId, leafId) + } + ) +} diff --git a/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts b/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts index 82e4f825cd8..f728e50c679 100644 --- a/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts +++ b/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts @@ -43,32 +43,55 @@ it('a window-owned build closes the pane at once and reports it to main', async expect(p.transports.has(1)).toBe(false) }) -it('a layout-owner build removes the pane only once main has closed it', async () => { +it('a layout-owner build removes the pane only once main’s frame drops it', async () => { mocks.layoutOwnerIsMain = true const p = await preparePendingSplitClose() + const intents = await import('@/runtime/layout-owner-pane-intents') + ;(await import('@/runtime/layout-frame-position')).noteLayoutFramePosition(1) p.actions.executeClosePane(1) expect(mocks.sendLayoutCommandToOwner).toHaveBeenCalledWith( 'workspace', { type: 'closePane', tabId: 'tab-parent', leafId: LEAF }, - { apply: expect.any(Function), asToday: expect.any(Function), refused: expect.any(Function) } + { + apply: expect.any(Function), + asToday: expect.any(Function), + refused: expect.any(Function) + } ) + mocks.sendLayoutCommandToOwner.mock.calls[0]?.[2].apply({}) expect(p.transports.has(1)).toBe(true) + // The frame without the pane finds the window's close and runs it. + expect(intents.takePaneCloseIntent('tab-parent', LEAF)).toBe('user') + p.actions.closePaneLocally(1, LEAF) + + expect(p.transports.has(1)).toBe(false) + expect(mocks.commitTerminalSurfaceClose).not.toHaveBeenCalled() +}) + +it('a layout-owner build closes the pane itself when main closed it and no frame is coming', async () => { + mocks.layoutOwnerIsMain = true + const p = await preparePendingSplitClose() + ;(await import('@/runtime/layout-frame-position')).endLayoutFramePositions() + + p.actions.executeClosePane(1) mocks.sendLayoutCommandToOwner.mock.calls[0]?.[2].apply({}) expect(p.transports.has(1)).toBe(false) expect(mocks.commitTerminalSurfaceClose).not.toHaveBeenCalled() }) -it('a layout-owner build closes as today when a paired server holds the tab', async () => { +it('a layout-owner build closes as today, forgetting its close, when main lacks the pane', async () => { mocks.layoutOwnerIsMain = true const p = await preparePendingSplitClose() p.actions.executeClosePane(1) mocks.sendLayoutCommandToOwner.mock.calls[0]?.[2].asToday() + const intents = await import('@/runtime/layout-owner-pane-intents') + expect(intents.takePaneCloseIntent('tab-parent', LEAF)).toBeNull() expect(mocks.commitTerminalSurfaceClose).toHaveBeenCalledOnce() expect(p.transports.has(1)).toBe(false) }) @@ -80,26 +103,14 @@ it('a layout-owner build closes the tab when main refuses a second close as the p.actions.executeClosePane(2) const [first, second] = mocks.sendLayoutCommandToOwner.mock.calls.map(([, , on]) => on) + // Main took pane 1; its frame is still on the way when main refuses pane 2 as its last. + ;(await import('@/runtime/layout-frame-position')).noteLayoutFramePosition(1) first?.apply({}) second?.refused?.('last_pane') expect(p.controller.onCloseTab).toHaveBeenCalledOnce() }) -it('a layout-owner build closes the tab when main’s answer finds the pane alone', async () => { - mocks.layoutOwnerIsMain = true - const p = await preparePendingSplitClose() - p.actions.executeClosePane(1) - p.actions.executeClosePane(2) - const [first, second] = mocks.sendLayoutCommandToOwner.mock.calls.map(([, , on]) => on) - - first?.apply({}) - second?.apply({}) - - expect(p.controller.onCloseTab).toHaveBeenCalledOnce() - expect(p.controller.managerRef.current?.getPanes()).toHaveLength(1) -}) - it('a layout-owner build leaves an unmounted tab alone when main refuses its last pane', async () => { mocks.layoutOwnerIsMain = true const p = await preparePendingSplitClose() diff --git a/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts b/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts index 596a06bda8c..1325213367c 100644 --- a/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts +++ b/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts @@ -121,7 +121,7 @@ export async function preparePendingSplitClose( ] const managerFixture = { getPanes: () => panes, - getLeafId: () => leafId, + getLeafId: (id: number) => panes.find((pane) => pane.id === id)?.leafId ?? null, getActivePane: () => null, closePane(id: number) { panes = panes.filter((pane) => pane.id !== id) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.test.ts b/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.test.ts index 99580ee010e..f13a9da5201 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.test.ts @@ -2,6 +2,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { LayoutApplyHandlers } from '@/runtime/layout-apply-send' import type { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' +import { endLayoutFramePositions, noteLayoutFramePosition } from '@/runtime/layout-frame-position' +import { + _resetLayoutOwnerPaneIntentsForTests, + expectPaneToClose, + takePaneCloseIntent +} from '@/runtime/layout-owner-pane-intents' import { closeExitedSplitPaneThroughLayoutOwner } from './exited-split-pane-close' const mocks = vi.hoisted(() => ({ @@ -16,13 +22,17 @@ const LEAF = '11111111-1111-4111-8111-111111111111' function fakeManager(paneCount: number) { const leaves = new Map([[1, LEAF]]) let panes = paneCount + const leafIds = (): { leafId: string }[] => + Array.from({ length: panes }, (_, index) => ({ + leafId: index === 0 ? LEAF : `sibling-${index}` + })) const closePane = vi.fn((paneId: number) => { leaves.delete(paneId) panes -= 1 }) const manager = { getLeafId: (paneId: number) => leaves.get(paneId) ?? null, - getPanes: () => Array.from({ length: panes }), + getPanes: leafIds, closePane } return { @@ -52,11 +62,12 @@ function close(paneCount = 2) { describe('closeExitedSplitPaneThroughLayoutOwner', () => { beforeEach(() => { + _resetLayoutOwnerPaneIntentsForTests() mocks.sendLayoutCommandToOwner.mockReset() mocks.sendLayoutCommandToOwner.mockReturnValue(true) }) - it('asks main to close the exited pane, and removes it only once main answers', () => { + it('asks main to close the exited pane, leaving its removal to main’s frame', () => { const c = close() expect(c.sent).toBe(true) @@ -65,28 +76,48 @@ describe('closeExitedSplitPaneThroughLayoutOwner', () => { { type: 'closePane', tabId: 'tab-1', leafId: LEAF, automatic: true }, expect.any(Object) ) - expect(c.closePane).not.toHaveBeenCalled() - + noteLayoutFramePosition(1) c.handlers().apply({}) - expect(c.closePane).toHaveBeenCalledWith(1) - expect(c.closeTab).not.toHaveBeenCalled() + endLayoutFramePositions() + + expect(c.closePane).not.toHaveBeenCalled() + expect(takePaneCloseIntent('tab-1', LEAF)).toBe('exit') }) - it('closes the tab instead when a sibling went while main answered', () => { + it('closes the pane itself when main closed it and no frame is coming', () => { const c = close() - c.loseSibling() c.handlers().apply({}) + expect(c.closePane).toHaveBeenCalledWith(1) + expect(takePaneCloseIntent('tab-1', LEAF)).toBeNull() + }) + + it('closes the tab when main calls the pane its last while a sibling’s exit close is pending', () => { + const c = close() + expectPaneToClose('tab-1', 'sibling-1', 'exit') + + c.handlers().refused?.('last_pane') + expect(c.closePane).not.toHaveBeenCalled() expect(c.closeTab).toHaveBeenCalledOnce() }) + it('closes the tab instead when main refuses the pane as its last and the window agrees', () => { + const c = close() + c.loseSibling() + + c.handlers().refused?.('last_pane') + + expect(c.closePane).not.toHaveBeenCalled() + expect(c.closeTab).toHaveBeenCalledOnce() + expect(takePaneCloseIntent('tab-1', LEAF)).toBeNull() + }) + it('does nothing when the pane already went', () => { const c = close() c.lose() - c.handlers().apply({}) c.handlers().asToday() expect(c.closePane).not.toHaveBeenCalled() diff --git a/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.ts b/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.ts index e30e3344d71..fc453cc1aba 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/exited-split-pane-close.ts @@ -1,9 +1,11 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' -import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' +import { panesLeftAfterPendingCloses } from '@/runtime/layout-owner-pane-intents' +import { sendPaneCloseToLayoutOwner } from '../layout-owner-pane-close-command' /** * A split pane whose terminal ended, in a layout-owner build: main closes it first, then the - * window removes it. False when a paired server owns the workspace; the caller closes as today. + * window removes it when main's frame does. False when a paired server owns the workspace; the + * caller closes as today. */ export function closeExitedSplitPaneThroughLayoutOwner(args: { worktreeId: string @@ -23,21 +25,21 @@ export function closeExitedSplitPaneThroughLayoutOwner(args: { if (manager.getLeafId(paneId) !== leafId) { return } - // A sibling went while main answered: this pane is now the tab's last. - if (manager.getPanes().length <= 1) { + // A sibling went, or is going, while main answered: this pane is now the tab's last. + if ( + panesLeftAfterPendingCloses( + args.tabId, + manager.getPanes().map((pane) => pane.leafId) + ) <= 1 + ) { args.closeTab() return } manager.closePane(paneId) } - return sendLayoutCommandToOwner( - args.worktreeId, - { type: 'closePane', tabId: args.tabId, leafId, automatic: true }, - { - apply: close, - asToday: close, - // Main holds this pane alone while the window has more: its copy lags, so close as today. - refused: (code) => code === 'last_pane' && close() - } + // Main holds this pane alone while the window has more: its copy lags, so close as today. + return sendPaneCloseToLayoutOwner( + { worktreeId: args.worktreeId, tabId: args.tabId, leafId, kind: 'exit' }, + { asToday: close, lastPane: close, closeHere: close } ) } 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 751c8c65be3..232b8ad8b77 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 @@ -31,10 +31,7 @@ export function recordingBootstrapSplits( } } -/** - * 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). - */ +/** Records a split the window drew under its pane id; main writes it, or finds it already held. */ export function recordDrawnPaneSplit( worktreeId: string, tabId: string, diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-frame-reconciliation.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-frame-reconciliation.test.ts new file mode 100644 index 00000000000..59e8bce8dd7 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-frame-reconciliation.test.ts @@ -0,0 +1,163 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { PaneManagerOptions } from '@/lib/pane-manager/pane-manager-types' +import type { TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types' +import { + _resetLayoutOwnerPaneIntentsForTests, + expectPaneToAppear, + expectPaneToClose, + takePaneAppearIntent +} from '@/runtime/layout-owner-pane-intents' +import { serializePaneTree } from './layout-serialization' + +// Why: happy-dom has no canvas, so xterm cannot open; the reconciler never touches the terminal. +vi.mock('@/lib/pane-manager/pane-lifecycle', async (importOriginal) => { + const actual: object = await importOriginal() + return { ...actual, openTerminal: vi.fn() } +}) +const recordCreatedTerminalPaneSplit = vi.hoisted(() => vi.fn()) +vi.mock('./terminal-pane-split-completion', () => ({ recordCreatedTerminalPaneSplit })) + +const { PaneManager } = await import('@/lib/pane-manager/pane-manager') +const { reconcileMountedPanesToLayout } = await import('./terminal-pane-frame-reconciliation') + +const A = '11111111-1111-4111-8111-111111111111' +const B = '22222222-2222-4222-8222-222222222222' +const C = '33333333-3333-4333-8333-333333333333' +const TAB = 'tab-1' + +const leaf = (leafId: string): TerminalPaneLayoutNode => ({ type: 'leaf', leafId }) +const split = ( + direction: 'vertical' | 'horizontal', + first: TerminalPaneLayoutNode, + second: TerminalPaneLayoutNode, + ratio = 0.5 +): TerminalPaneLayoutNode => ({ type: 'split', direction, first, second, ratio }) + +describe('reconcileMountedPanesToLayout', () => { + let root: HTMLDivElement + let manager: InstanceType + let onPaneCreated: Mock> + const closePane = vi.fn() + + const reconcile = (layout: TerminalPaneLayoutNode, isExpanded = false): boolean => + reconcileMountedPanesToLayout({ + tabId: TAB, + manager, + layout: { root: layout, activeLeafId: null, expandedLeafId: null }, + closePane, + isExpanded + }) + const serialized = (): TerminalPaneLayoutNode | null => { + const top = root.firstElementChild + return top instanceof HTMLElement ? serializePaneTree(top) : null + } + const leaves = (): string[] => manager.getPanes().map((pane) => pane.leafId) + + beforeEach(() => { + _resetLayoutOwnerPaneIntentsForTests() + closePane.mockReset() + recordCreatedTerminalPaneSplit.mockReset() + vi.stubGlobal('requestAnimationFrame', () => 1) + vi.stubGlobal('cancelAnimationFrame', () => {}) + root = document.createElement('div') + document.body.appendChild(root) + onPaneCreated = vi.fn() + manager = new PaneManager(root, { linkOpenHint: () => '', onPaneCreated }) + const a = manager.createInitialPane({ leafId: A }) + manager.splitPane(a.id, 'vertical', { leafId: B }) + onPaneCreated.mockClear() + }) + + afterEach(() => { + manager.destroy() + root.remove() + vi.unstubAllGlobals() + }) + + it('mounts the window’s own split where main placed it, with the cwd it asked for', () => { + expectPaneToAppear(C, { + tabId: TAB, + cwd: '/repo', + telemetry: { source: 'keyboard', direction: 'horizontal' } + }) + const layout = split('vertical', leaf(A), split('horizontal', leaf(B), leaf(C), 0.4), 0.3) + + expect(reconcile(layout)).toBe(true) + + expect(serialized()).toEqual(layout) + expect(onPaneCreated).toHaveBeenCalledWith( + expect.objectContaining({ leafId: C }), + expect.objectContaining({ cwd: '/repo' }) + ) + expect(recordCreatedTerminalPaneSplit).toHaveBeenCalledWith( + expect.objectContaining({ leafId: C }), + { source: 'keyboard', direction: 'horizontal' } + ) + expect(takePaneAppearIntent(TAB, C)).toBeNull() + }) + + it('leaves a pane main added without the window’s ask to main’s notice', () => { + const layout = split('vertical', leaf(A), split('horizontal', leaf(B), leaf(C))) + + expect(reconcile(layout)).toBe(false) + + expect(leaves()).toEqual([A, B]) + expect(onPaneCreated).not.toHaveBeenCalled() + }) + + it('closes a pane main dropped only when the window asked, with the close it asked for', () => { + expect(reconcile(leaf(A))).toBe(false) + expect(closePane).not.toHaveBeenCalled() + + expectPaneToClose(TAB, B, 'exit') + expect(reconcile(leaf(A))).toBe(true) + + expect(closePane).toHaveBeenCalledWith(manager.getNumericIdForLeaf(B), B, 'exit') + }) + + it('keeps the last pane when main’s tree holds a pane the window has not drawn', () => { + expectPaneToClose(TAB, A, 'user') + manager.closePane(manager.getNumericIdForLeaf(B)!) + + expect(reconcile(leaf(C))).toBe(false) + + expect(closePane).not.toHaveBeenCalled() + expect(leaves()).toEqual([A]) + }) + + it('ignores an intent for another tab', () => { + expectPaneToAppear(C, { + tabId: 'tab-2', + telemetry: { source: 'keyboard', direction: 'vertical' } + }) + + reconcile(split('vertical', leaf(A), split('vertical', leaf(B), leaf(C)))) + + expect(leaves()).toEqual([A, B]) + }) + + it('takes main’s ratios, then another client’s move, without recreating a pane', () => { + const before = manager.getPanes().map((pane) => pane.container) + + // Neither adds or closes a pane: the store already holds main's tree. + expect(reconcile(split('vertical', leaf(A), leaf(B), 0.7))).toBe(false) + expect(serialized()).toEqual(split('vertical', leaf(A), leaf(B), 0.7)) + + expect(reconcile(split('horizontal', leaf(B), leaf(A), 0.2))).toBe(false) + expect(serialized()).toEqual(split('horizontal', leaf(B), leaf(A), 0.2)) + expect(manager.getPanes().map((pane) => pane.container)).toEqual(before) + expect(onPaneCreated).not.toHaveBeenCalled() + }) + + it('leaves a zoomed tab’s shape for later, still mounting its own split', () => { + expectPaneToAppear(C, { + tabId: TAB, + telemetry: { source: 'keyboard', direction: 'vertical' } + }) + + reconcile(split('horizontal', leaf(B), split('vertical', leaf(A), leaf(C))), true) + + expect(leaves()).toEqual([A, B, C]) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-frame-reconciliation.ts b/src/renderer/src/components/terminal-pane/terminal-pane-frame-reconciliation.ts new file mode 100644 index 00000000000..bf4ab268d47 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-frame-reconciliation.ts @@ -0,0 +1,88 @@ +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import { + expectPaneToAppear, + type PaneCloseKind, + takePaneAppearIntent, + takePaneCloseIntent +} from '@/runtime/layout-owner-pane-intents' +import { planTerminalLiveLayoutInsertions } from './terminal-live-layout-reconciliation' +import { collectLeafIds } from './terminal-pane-layout-tree' +import { recordCreatedTerminalPaneSplit } from './terminal-pane-split-completion' + +/** + * A layout-owner build's mounted tab drawn from main's tree (the store's layout, which frames set). + * The window's own split and close show here, with the inputs and close it asked for; a pane main + * added or dropped without such an intent is left to main's notice. Then the panes take main's + * shape (by moving, never recreating) and ratios. True when panes were added or closed. + */ +export function reconcileMountedPanesToLayout(args: { + tabId: string + manager: PaneManager + layout: TerminalLayoutSnapshot + /** The window's own close of a pane main closed: its terminal, status and pane go. */ + closePane: (paneId: number, leafId: string, kind: PaneCloseKind) => void + /** A zoomed pane hides its siblings; their shape and ratios wait until it collapses. */ + isExpanded: boolean +}): boolean { + const { tabId, manager, layout } = args + const root = layout.root + if (!root) { + return false + } + const target = new Set(collectLeafIds(root)) + let changed = false + + for (const pane of manager.getPanes()) { + const kind = target.has(pane.leafId) ? null : takePaneCloseIntent(tabId, pane.leafId) + // The window's last pane stays: main's tree then holds a pane the window lacks (a split whose + // source went), which main's notice brings; never a tab with no pane. + if (kind && manager.getPanes().length > 1) { + args.closePane(pane.id, pane.leafId, kind) + changed = true + } + } + + const mounted = manager.getPanes().map((pane) => pane.leafId) + for (const insertion of planTerminalLiveLayoutInsertions(root, mounted)) { + const sourcePaneId = manager.getNumericIdForLeaf(insertion.sourceLeafId) + if (sourcePaneId === null || manager.getNumericIdForLeaf(insertion.newLeafId) !== null) { + continue + } + const intent = takePaneAppearIntent(tabId, insertion.newLeafId) + if (!intent) { + continue + } + const ratio = + insertion.ratio === undefined + ? undefined + : insertion.placement === 'before' + ? 1 - insertion.ratio + : insertion.ratio + const created = manager.splitPaneAroundLeafIds( + insertion.sourceLeafIds, + sourcePaneId, + insertion.direction, + { + leafId: insertion.newLeafId, + placement: insertion.placement, + ...(ratio !== undefined && { ratio }), + ...(intent.cwd ? { cwd: intent.cwd } : {}), + ...(intent.cwdPromise ? { cwdPromise: intent.cwdPromise } : {}) + } + ) + if (!created) { + expectPaneToAppear(insertion.newLeafId, intent) + continue + } + recordCreatedTerminalPaneSplit(created, intent.telemetry) + changed = true + } + + if (args.isExpanded) { + return changed + } + // Ratios, or another client's move: the same panes move into main's shape, none recreated. + manager.applyLayoutGeometry(root, true) + return changed +} diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts index 12f796b3c37..62acf79e2b2 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts @@ -1,6 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { equalizeTerminalPanes } from './terminal-pane-layout-owner-commands' +import { + _resetLayoutFramePositionForTests, + beginLayoutFramePositions, + endLayoutFramePositions +} from '@/runtime/layout-frame-position' const mocks = vi.hoisted( (): { @@ -31,6 +36,7 @@ describe('equalizeTerminalPanes', () => { mocks.callRuntimeRpc.mockReset() mocks.ownerEnvironmentId = null mocks.layoutOwnerIsMain = false + _resetLayoutFramePositionForTests() }) function manager() { @@ -55,20 +61,39 @@ describe('equalizeTerminalPanes', () => { expect(mocks.callRuntimeRpc).not.toHaveBeenCalled() }) - it('a layout-owner build equalizes once main has', async () => { + it('a layout-owner build leaves the sizes to main’s frame, then focuses', async () => { mocks.layoutOwnerIsMain = true + beginLayoutFramePositions() mocks.callRuntimeRpc.mockResolvedValue({ ok: true, result: {} }) const current = manager() + const then = vi.fn() equalizeTerminalPanes( { worktreeId: 'worktree-1', tabId: 'tab-1', getManager: () => current }, - vi.fn() + then ) expect(mocks.callRuntimeRpc).toHaveBeenCalledWith({ kind: 'local' }, 'layout.apply', { command: { type: 'equalizePanes', tabId: 'tab-1', workspace: 'worktree-1' } }) + await vi.waitFor(() => expect(then).toHaveBeenCalledWith(current)) expect(current.equalizePaneSizes).not.toHaveBeenCalled() - await vi.waitFor(() => expect(current.equalizePaneSizes).toHaveBeenCalledOnce()) + }) + + it('a layout-owner build equalizes itself when no frame is coming', async () => { + mocks.layoutOwnerIsMain = true + beginLayoutFramePositions() + endLayoutFramePositions() + mocks.callRuntimeRpc.mockResolvedValue({ ok: true, result: {} }) + const current = manager() + const then = vi.fn() + + equalizeTerminalPanes( + { worktreeId: 'worktree-1', tabId: 'tab-1', getManager: () => current }, + then + ) + + await vi.waitFor(() => expect(then).toHaveBeenCalledWith(current)) + expect(current.equalizePaneSizes).toHaveBeenCalledOnce() }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts index 76fa2e47df5..33c063cdc4a 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts @@ -1,8 +1,9 @@ import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' +import { layoutFramesFlowing } from '@/runtime/layout-frame-position' -/** Equalizes a tab's panes, then `then` runs (focus); in a layout-owner build main equalizes first. */ +/** Equalizes a tab's panes, then `then` runs (focus); in a layout-owner build main equalizes. */ export function equalizeTerminalPanes( pane: { worktreeId: string; tabId: string; getManager: () => PaneManager | null }, then: (manager: PaneManager) => void @@ -14,13 +15,25 @@ export function equalizeTerminalPanes( then(manager) } } - // Switch point: main equalizes the tab first; the window then redraws it. + // Switch point: main equalizes the tab; the window takes the sizes from main's frame. if ( ORCA_LAYOUT_OWNER_IS_MAIN && sendLayoutCommandToOwner( pane.worktreeId, { type: 'equalizePanes', tabId: pane.tabId }, - { apply: equalize, asToday: equalize } + { + // No frame is coming (the stream is down): the window equalizes itself, as main did. + apply: () => { + const manager = pane.getManager() + if (!layoutFramesFlowing()) { + equalize() + } else if (manager) { + then(manager) + } + }, + asToday: equalize, + unapplied: () => {} + } ) ) { return diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts index 1823a72054c..50a456e6273 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts @@ -55,6 +55,7 @@ describe('layoutOwnerPaneGestures', () => { { apply: expect.any(Function), asToday: expect.any(Function), + unapplied: expect.any(Function), refused: expect.any(Function) } ) @@ -64,8 +65,17 @@ describe('layoutOwnerPaneGestures', () => { expect(move).toHaveBeenCalledOnce() }) - it.each(['asToday', 'refused'] as const)( - 'moves the pane as today when main answers %s', + it('moves the pane as today when main answers as today', () => { + const move = vi.fn() + gestures().takePaneDrop?.({ sourcePaneId: 2, targetPaneId: 1, zone: 'left' }, move) + + mocks.send.mock.calls[0]?.[2].asToday() + + expect(move).toHaveBeenCalledOnce() + }) + + it.each(['refused', 'unapplied'] as const)( + 'keeps the window’s shape when main answers %s, as main’s tree did not change', (outcome) => { const move = vi.fn() gestures().takePaneDrop?.({ sourcePaneId: 2, targetPaneId: 1, zone: 'left' }, move) @@ -74,10 +84,10 @@ describe('layoutOwnerPaneGestures', () => { if (outcome === 'refused') { handlers?.refused?.('same_pane') } else { - handlers?.asToday() + handlers?.unapplied?.() } - expect(move).toHaveBeenCalledOnce() + expect(move).not.toHaveBeenCalled() } ) @@ -91,7 +101,7 @@ describe('layoutOwnerPaneGestures', () => { view.onDividerRatiosCommitted?.() const [first, second] = mocks.send.mock.calls.map(([, , on]) => on) - expect(() => first?.refused?.('pane_structure_changed')).toThrow('pane gone') + expect(() => first?.apply({})).toThrow('pane gone') expect(mocks.send).toHaveBeenCalledTimes(2) second?.apply({}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts index 8bdd641745e..6fb0eb9aa94 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts @@ -3,8 +3,8 @@ import { useAppStore } from '@/store' import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' /** - * A layout-owner build's pane gestures: an in-tab drop moves the pane once main has answered, as - * today when main refuses; a divider's new ratios, already drawn while dragging, are saved by main + * A layout-owner build's pane gestures: an in-tab drop moves the pane once main has answered, and + * not when main refuses; a divider's new ratios, already drawn while dragging, are saved by main * when the drag ends. */ export function layoutOwnerPaneGestures(pane: { @@ -44,9 +44,11 @@ export function layoutOwnerPaneGestures(pane: { if (!leafId || !targetLeafId) { return false } - const settle = (): void => { + const settle = (moveHere: boolean): void => { try { - move() + if (moveHere) { + move() + } } finally { movesInFlight -= 1 if (movesInFlight === 0 && ratiosWaiting) { @@ -57,7 +59,13 @@ export function layoutOwnerPaneGestures(pane: { const taken = sendLayoutCommandToOwner( worktreeId, { type: 'movePane', tabId, leafId, targetLeafId, side: zone }, - { apply: settle, asToday: settle, refused: settle } + { + apply: () => settle(true), + asToday: () => settle(true), + // Main's tree did not change, and frames draw main's tree: the window keeps its shape. + unapplied: () => settle(false), + refused: () => settle(false) + } ) if (taken) { movesInFlight += 1 diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-restore.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-restore.ts index e3634481502..15a040725b6 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-restore.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-restore.ts @@ -8,6 +8,8 @@ import type { TerminalPaneLifecycleRefs } from './use-terminal-pane-lifecycle-re import type { UseTerminalPaneLifecycleDeps } from './terminal-pane-lifecycle-types' import type { PtyConnectionDeps } from './pty-connection-types' import { resolveParkRevealLiveTuiLeafIds } from './park-reveal-live-tui-leaves' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' +import { withoutQueuedMainSplits } from './terminal-pane-split-request-routing' import { mapRestoredPaneTitlesByPaneId, replayLayoutWithOneShotParkIntent @@ -28,7 +30,15 @@ export function restoreTerminalPaneLayout(args: { initialLayoutRef.current.ptyIdsByLeafId ) const restoredPaneByLeafId = replayLayoutWithOneShotParkIntent(ptyDeps, () => - replayTerminalLayout(manager, initialLayoutRef.current, isActive) + replayTerminalLayout( + manager, + // Switch point: main's frames already hold a split main asked for; its queued request mounts + // it, with its startup command. + ORCA_LAYOUT_OWNER_IS_MAIN + ? withoutQueuedMainSplits(initialLayoutRef.current, tabId, worktreeId) + : initialLayoutRef.current, + isActive + ) ) const localOnlyBuffers = useAppStore.getState().localOnlyScrollbackByTabId[tabId] const restoredBuffers = resolveLeafScrollbackBuffers({ 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 index b9c086015b1..5682aaa6da4 100644 --- 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 @@ -6,12 +6,23 @@ import type { PtyConnectionDeps } from './pty-connection-types' import { installTerminalPaneMountEvents } from './terminal-pane-mount-events' import { _resetTerminalPaneSplitRequestRoutingForTests, - dispatchTerminalPaneSplitRequest + dispatchTerminalPaneSplitRequest, + queueTerminalPaneSplitRequest, + withoutQueuedMainSplits } from './terminal-pane-split-request-routing' +import { + _resetLayoutFramePositionForTests, + beginLayoutFramePositions, + endLayoutFramePositions, + layoutFramesFlowing +} from '@/runtime/layout-frame-position' 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 sendLayoutApply = vi.hoisted(() => vi.fn()) +vi.mock('@/runtime/layout-apply-send', () => ({ + sendLayoutApply, + recordLayoutApply: sendLayoutApply +})) const storeState = vi.hoisted(() => ({ recordFeatureInteraction: () => {} })) vi.mock('@/store', () => ({ useAppStore: { getState: () => storeState } })) vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync: vi.fn() })) @@ -19,9 +30,16 @@ 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 } { +function mountedTab(ptyDeps: { startup?: unknown } = {}): { + manager: PaneManager + splitPane: ReturnType + startups: unknown[] +} { const leafById = new Map([[1, SOURCE]]) + const startups: unknown[] = [] const splitPane = vi.fn((_paneId: number, _direction: string, opts?: { leafId?: string }) => { + // What the new pane's terminal starts with: read while the pane mounts. + startups.push(ptyDeps.startup ?? null) const leafId = opts?.leafId ?? 'window-minted' leafById.set(2, leafId) return { id: 2, leafId } @@ -33,10 +51,10 @@ function mountedTab(): { manager: PaneManager; splitPane: ReturnType 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 } + return { manager: manager as unknown as PaneManager, splitPane, startups } } -function install(manager: PaneManager): () => void { +function install(manager: PaneManager, ptyDeps: { startup?: unknown } = {}): () => void { return installTerminalPaneMountEvents({ manager, deps: { @@ -48,18 +66,19 @@ function install(manager: PaneManager): () => void { 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 + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a split reads only `startup` from pty deps. + ptyDeps: ptyDeps as PtyConnectionDeps }) } describe('a split main asked for, mounted in a layout-owner build', () => { afterEach(() => { - recordLayoutApply.mockReset() + sendLayoutApply.mockReset() _resetTerminalPaneSplitRequestRoutingForTests() + _resetLayoutFramePositionForTests() }) - it('records the mounted pane in main, so a pane main took back meanwhile returns', () => { + it('mounts the pane under main’s id and records nothing: main already holds it', () => { const { manager, splitPane } = mountedTab() const uninstall = install(manager) @@ -73,18 +92,15 @@ describe('a split main asked for, mounted in a layout-owner build', () => { }) expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { leafId: NEW }) - expect(recordLayoutApply).toHaveBeenCalledWith(storeState, 'wt-1', { - type: 'splitPane', - tabId: 'tab-1', - leafId: SOURCE, - direction: 'vertical', - newLeafId: NEW - }) + expect(sendLayoutApply).not.toHaveBeenCalled() uninstall() }) - it('records nothing for a split that names no pane id', () => { - const { manager } = mountedTab() + it('mounts it while the stream is down, the same way', () => { + beginLayoutFramePositions() + endLayoutFramePositions() + expect(layoutFramesFlowing()).toBe(false) + const { manager, splitPane } = mountedTab() const uninstall = install(manager) dispatchTerminalPaneSplitRequest({ @@ -92,10 +108,52 @@ describe('a split main asked for, mounted in a layout-owner build', () => { worktreeId: 'wt-1', paneRuntimeId: 1, sourceLeafId: SOURCE, - direction: 'horizontal' + direction: 'horizontal', + newLeafId: NEW }) - expect(recordLayoutApply).not.toHaveBeenCalled() + expect(splitPane).toHaveBeenCalledWith(1, 'horizontal', { leafId: NEW }) + expect(sendLayoutApply).not.toHaveBeenCalled() + uninstall() + }) + + it('mounts a split into an unmounted tab with its command, not as a fresh shell', () => { + // The split lands while the tab is not mounted: its request waits; main's frame already + // holds the pane. + queueTerminalPaneSplitRequest({ + tabId: 'tab-1', + worktreeId: 'wt-1', + paneRuntimeId: 1, + sourceLeafId: SOURCE, + direction: 'vertical', + newLeafId: NEW, + command: 'make test' + }) + const framed = { + root: { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: SOURCE }, + second: { type: 'leaf' as const, leafId: NEW } + }, + activeLeafId: SOURCE, + expandedLeafId: null + } + + // The tab's first mount replays its layout without the pane... + expect(withoutQueuedMainSplits(framed, 'tab-1', 'wt-1').root).toEqual({ + type: 'leaf', + leafId: SOURCE + }) + // ...and the waiting request mounts it, starting its command. + const ptyDeps: { startup?: unknown } = {} + const { manager, splitPane, startups } = mountedTab(ptyDeps) + const uninstall = install(manager, ptyDeps) + + expect(splitPane).toHaveBeenCalledTimes(1) + expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { leafId: NEW }) + expect(startups).toEqual([{ command: 'make test' }]) + expect(sendLayoutApply).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 f6e4ec66d45..71d68cd9db6 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,8 +15,6 @@ 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 @@ -49,12 +47,6 @@ 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 } : {}) @@ -63,19 +55,12 @@ 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 diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.test.ts index 6fc9fdda38b..3cf5fcac463 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.test.ts @@ -12,7 +12,8 @@ import { registerTerminalPaneSplitRequestHandler, resolveTerminalPaneSplitSourceId, takeQueuedTerminalPaneSplitRequests, - TERMINAL_PANE_SPLIT_QUEUE_CAPACITY + TERMINAL_PANE_SPLIT_QUEUE_CAPACITY, + withoutQueuedMainSplits } from './terminal-pane-split-request-routing' const SOURCE_LEAF_ID = '11111111-1111-4111-8111-111111111111' @@ -136,3 +137,43 @@ describe('parked terminal split request routing', () => { unregisterSecond() }) }) + +describe('withoutQueuedMainSplits', () => { + const SOURCE = '11111111-1111-4111-8111-111111111111' + const NEW = '22222222-2222-4222-8222-222222222222' + const layout = { + root: { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: SOURCE }, + second: { type: 'leaf' as const, leafId: NEW } + }, + activeLeafId: SOURCE, + expandedLeafId: null + } + + it('leaves out a pane whose queued split main asked for, so that request mounts it with its command', () => { + _resetTerminalPaneSplitRequestRoutingForTests() + queueTerminalPaneSplitRequest({ + tabId: 'tab-1', + worktreeId: 'wt-1', + paneRuntimeId: 1, + sourceLeafId: SOURCE, + direction: 'vertical', + newLeafId: NEW, + command: 'make test' + }) + + expect(withoutQueuedMainSplits(layout, 'tab-1', 'wt-1').root).toEqual({ + type: 'leaf', + leafId: SOURCE + }) + expect(withoutQueuedMainSplits(layout, 'tab-2', 'wt-1')).toBe(layout) + expect(takeQueuedTerminalPaneSplitRequests('tab-1', 'wt-1')[0]).toMatchObject({ + newLeafId: NEW, + command: 'make test' + }) + expect(withoutQueuedMainSplits(layout, 'tab-1', 'wt-1')).toBe(layout) + _resetTerminalPaneSplitRequestRoutingForTests() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.ts index 26b03981309..9efd212a080 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-request-routing.ts @@ -1,5 +1,7 @@ import { SPLIT_TERMINAL_PANE_EVENT, type SplitTerminalPaneDetail } from '@/constants/terminal' import { BACKGROUND_WORKTREE_MEASURE_WINDOW_MS } from '../terminal/background-terminal-worktree-visibility' +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import { removeLayoutLeaf } from '../../../../shared/workspace-layout/terminal-pane-tree' // Why 32: ordinary use has one request; the cap tolerates automation bursts without letting a stalled renderer retain unbounded commands or mount leases. export const TERMINAL_PANE_SPLIT_QUEUE_CAPACITY = 32 @@ -123,6 +125,29 @@ export function takeQueuedTerminalPaneSplitRequests( return requests } +/** The tab's layout without the panes of splits main asked for that wait here, which mount them. */ +export function withoutQueuedMainSplits( + layout: TerminalLayoutSnapshot, + tabId: string, + worktreeId: string +): TerminalLayoutSnapshot { + let root = layout.root + for (const leafId of queuedMainSplitLeafIds(tabId, worktreeId)) { + root = removeLayoutLeaf(root, leafId) ?? root + } + return root === layout.root ? layout : { ...layout, root } +} + +function queuedMainSplitLeafIds(tabId: string, worktreeId: string): string[] { + return queuedRequests.flatMap((request) => + request.tabId === tabId && + request.newLeafId && + (request.worktreeId === undefined || request.worktreeId === worktreeId) + ? [request.newLeafId] + : [] + ) +} + export function cancelQueuedTerminalPaneSplitRequests(tabId: string, worktreeId?: string): void { removeQueuedRequestsForTarget(tabId, worktreeId) releaseSplitMountLease(tabId, worktreeId) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts index 9439da3c934..0e3cefb27a8 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts @@ -5,6 +5,11 @@ import type { PaneCwdMap } from './resolve-split-cwd' import { splitTerminalPaneWithInheritedCwd } from './terminal-pane-split-with-inherited-cwd' import { createDeferred } from './pty-connection-test-async' import type { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' +import { endLayoutFramePositions, noteLayoutFramePosition } from '@/runtime/layout-frame-position' +import { + _resetLayoutOwnerPaneIntentsForTests, + takePaneAppearIntent +} from '@/runtime/layout-owner-pane-intents' const mocks = vi.hoisted(() => ({ layoutOwnerIsMain: false, @@ -50,6 +55,7 @@ function makeManager(splitPane: ReturnType): PaneManager { describe('splitTerminalPaneWithInheritedCwd', () => { beforeEach(() => { + _resetLayoutOwnerPaneIntentsForTests() mocks.layoutOwnerIsMain = false mocks.sendLayoutCommandToOwner.mockReset() mocks.recordCreatedTerminalPaneSplit.mockReset() @@ -252,11 +258,10 @@ describe('splitTerminalPaneWithInheritedCwd', () => { expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { cwd: '/cached' }) }) - it('in a layout-owner build, mounts the pane main minted only once main answers', () => { + it('in a layout-owner build, asks main for the split and leaves drawing it to main’s frame', () => { mocks.layoutOwnerIsMain = true mocks.sendLayoutCommandToOwner.mockReturnValue(true) - const createdPane = { id: 2 } - const splitPane = vi.fn(() => createdPane) + const splitPane = vi.fn(() => ({ id: 2 })) splitTerminalPaneWithInheritedCwd({ worktreeId: 'worktree-1', @@ -270,27 +275,63 @@ describe('splitTerminalPaneWithInheritedCwd', () => { source: 'keyboard' }) - expect(mocks.sendLayoutCommandToOwner).toHaveBeenCalledWith( - 'worktree-1', - { type: 'splitPane', tabId: 'tab-1', leafId: 'leaf-1', direction: 'vertical' }, - { - apply: expect.any(Function), - asToday: expect.any(Function) - } - ) + const [, command, handlers] = mocks.sendLayoutCommandToOwner.mock.calls[0] ?? [] + expect(command).toEqual({ + type: 'splitPane', + tabId: 'tab-1', + leafId: 'leaf-1', + direction: 'vertical', + newLeafId: expect.any(String) + }) + noteLayoutFramePosition(1) + handlers?.apply({}) + endLayoutFramePositions() expect(splitPane).not.toHaveBeenCalled() - mocks.sendLayoutCommandToOwner.mock.calls[0]?.[2].apply({ leafId: 'leaf-minted' }) + const newLeafId = command?.type === 'splitPane' ? (command.newLeafId ?? '') : '' + expect(takePaneAppearIntent('tab-1', newLeafId)).toEqual({ + tabId: 'tab-1', + cwd: '/cached', + telemetry: { source: 'keyboard', direction: 'vertical' } + }) + }) - expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { - leafId: 'leaf-minted', - cwd: '/cached' - }) - expect(mocks.recordCreatedTerminalPaneSplit).toHaveBeenCalledWith(createdPane, { - source: 'keyboard', - direction: 'vertical' - }) - expect(mocks.sendLayoutCommandToOwner).toHaveBeenCalledOnce() + it('in a layout-owner build, draws main’s pane itself when no frame is coming', () => { + const splitPane = vi.fn(() => ({ id: 2 })) + splitInLayoutOwnerBuild(makeManager(splitPane)) + const [, command, handlers] = mocks.sendLayoutCommandToOwner.mock.calls[0] ?? [] + const newLeafId = command?.type === 'splitPane' ? (command.newLeafId ?? '') : '' + + handlers?.apply({}) + + expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { cwd: '/cached', leafId: newLeafId }) + expect(takePaneAppearIntent('tab-1', newLeafId)).toBeNull() + }) + + it.each(['unapplied', 'refused'] as const)( + 'in a layout-owner build, forgets the pane when main answers %s', + (outcome) => { + splitInLayoutOwnerBuild(makeManager(vi.fn())) + const [, command, handlers] = mocks.sendLayoutCommandToOwner.mock.calls[0] ?? [] + + if (outcome === 'refused') { + handlers?.refused?.('pane_not_found') + } else { + handlers?.unapplied?.() + } + + const newLeafId = command?.type === 'splitPane' ? (command.newLeafId ?? '') : '' + expect(takePaneAppearIntent('tab-1', newLeafId)).toBeNull() + } + ) + + it('in a layout-owner build, splits here when main answers as today', () => { + const splitPane = vi.fn(() => ({ id: 2 })) + splitInLayoutOwnerBuild(makeManager(splitPane)) + + mocks.sendLayoutCommandToOwner.mock.calls[0]?.[2].asToday() + + expect(splitPane).toHaveBeenCalledWith(1, 'vertical', { cwd: '/cached' }) }) it('in a layout-owner build, splits as today where a paired server owns the layout', () => { @@ -313,14 +354,13 @@ describe('splitTerminalPaneWithInheritedCwd', () => { expect(splitPane).toHaveBeenCalledWith(1, 'horizontal', { cwd: '/cached' }) }) - function splitInLayoutOwnerBuild(manager: PaneManager | null): void { + function splitInLayoutOwnerBuild(manager: PaneManager): void { mocks.layoutOwnerIsMain = true mocks.sendLayoutCommandToOwner.mockReturnValue(true) splitTerminalPaneWithInheritedCwd({ worktreeId: 'worktree-1', tabId: 'tab-1', - manager: manager ?? makeManager(vi.fn()), - getManager: () => manager, + manager, paneTransports: new Map(), paneCwdMap: new Map([[1, { cwd: '/cached', confirmed: true }]]), fallbackCwd: '/fallback', @@ -329,19 +369,4 @@ describe('splitTerminalPaneWithInheritedCwd', () => { source: 'keyboard' }) } - - it.each([ - ['the manager is gone', null], - ['the source pane is gone', makeManager(vi.fn(() => null))] - ])('withdraws the pane main saved when %s by the time main answers', (_case, manager) => { - splitInLayoutOwnerBuild(manager) - - mocks.sendLayoutCommandToOwner.mock.calls[0]?.[2].apply({ leafId: 'leaf-minted' }) - - expect(mocks.sendLayoutCommandToOwner).toHaveBeenLastCalledWith( - 'worktree-1', - { type: 'closePane', tabId: 'tab-1', leafId: 'leaf-minted' }, - { apply: expect.any(Function), asToday: expect.any(Function) } - ) - }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts index 325111756b7..358b7dbc7e4 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts @@ -6,6 +6,9 @@ import { resolveSplitCwd, type PaneCwdMap } from './resolve-split-cwd' import { recordCreatedTerminalPaneSplit } from './terminal-pane-split-completion' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' +import { expectPaneToAppear } from '@/runtime/layout-owner-pane-intents' +import { layoutFramesFlowing } from '@/runtime/layout-frame-position' +import { mintStablePaneId } from '@/lib/pane-manager/mint-stable-pane-id' type SplitArgs = { worktreeId: string @@ -31,65 +34,79 @@ export function splitTerminalPaneWithInheritedCwd(args: SplitArgs): void { ) { return } - // Switch point: main writes the split and mints the new pane's id; the window then mounts it. - if ( - ORCA_LAYOUT_OWNER_IS_MAIN && - sendLayoutCommandToOwner( - args.worktreeId, - { type: 'splitPane', tabId: args.tabId, leafId: args.pane.leafId, direction: args.direction }, - { - apply: ({ leafId }) => leafId && mountMainSplit(args, ptyId, leafId), - asToday: () => splitLocally(args, ptyId) - } - ) - ) { - return + // Switch point: main writes the split; the window draws it from main's frame, with its cwd. + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + const newLeafId = mintStablePaneId() + const cwd = splitCwd(args, ptyId) + const forget = expectPaneToAppear(newLeafId, { + tabId: args.tabId, + ...cwd, + telemetry: { source: args.source, direction: args.direction } + }) + if ( + sendLayoutCommandToOwner( + args.worktreeId, + { + type: 'splitPane', + tabId: args.tabId, + leafId: args.pane.leafId, + direction: args.direction, + newLeafId + }, + { + // No frame is coming (the stream is down): the window draws main's pane itself. + apply: () => { + if (!layoutFramesFlowing()) { + forget() + splitLocally(args, () => cwd, newLeafId) + } + }, + asToday: () => { + forget() + splitLocally(args, () => cwd) + }, + unapplied: forget, + refused: forget + } + ) + ) { + return + } + forget() } - splitLocally(args, ptyId) + splitLocally(args, () => splitCwd(args, ptyId)) } -/** Mounts main's new pane, or withdraws it when its source pane or manager went meanwhile. */ -function mountMainSplit(args: SplitArgs, ptyId: string | null, leafId: string): void { - splitLocally(args, ptyId, leafId) - const manager = args.getManager ? args.getManager() : args.manager - if (manager?.getNumericIdForLeaf(leafId) == null) { - const keep = (): void => {} - sendLayoutCommandToOwner( - args.worktreeId, - { type: 'closePane', tabId: args.tabId, leafId }, - { apply: keep, asToday: keep } - ) +/** The new pane's cwd: the source pane's when known, else resolved from its terminal. */ +type SplitCwd = { cwd: string } | { cwdPromise: Promise } + +function splitCwd(args: SplitArgs, ptyId: string | null): SplitCwd { + const cached = args.paneCwdMap.get(args.pane.id) + if (cached?.confirmed && cached.cwd) { + return { cwd: cached.cwd } + } + return { + cwdPromise: + cached?.pendingCwd ?? + resolveSplitCwd({ + paneCwdMap: args.paneCwdMap, + sourcePaneId: args.pane.id, + sourcePtyId: ptyId, + fallbackCwd: args.fallbackCwd + }) } } -function splitLocally(args: SplitArgs, ptyId: string | null, leafId?: string): void { +/** `cwd` is read only once a manager is there to split. */ +function splitLocally(args: SplitArgs, cwd: () => SplitCwd, leafId?: string): void { const manager = args.getManager ? args.getManager() : args.manager if (!manager) { return } - const placement = leafId ? { leafId } : {} - const cached = args.paneCwdMap.get(args.pane.id) - if (cached?.confirmed && cached.cwd) { - const createdPane = manager.splitPane(args.pane.id, args.direction, { - ...placement, - cwd: cached.cwd - }) - recordCreatedTerminalPaneSplit(createdPane, { - source: args.source, - direction: args.direction - }) - return - } - const paneId = args.pane.id - const cwdPromise = - cached?.pendingCwd ?? - resolveSplitCwd({ - paneCwdMap: args.paneCwdMap, - sourcePaneId: paneId, - sourcePtyId: ptyId, - fallbackCwd: args.fallbackCwd - }) - const createdPane = manager.splitPane(paneId, args.direction, { ...placement, cwdPromise }) + const createdPane = manager.splitPane(args.pane.id, args.direction, { + ...cwd(), + ...(leafId ? { leafId } : {}) + }) recordCreatedTerminalPaneSplit(createdPane, { source: args.source, direction: args.direction diff --git a/src/renderer/src/components/terminal-pane/use-layout-owner-pane-tree.test.ts b/src/renderer/src/components/terminal-pane/use-layout-owner-pane-tree.test.ts new file mode 100644 index 00000000000..0f2ccdb2682 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-layout-owner-pane-tree.test.ts @@ -0,0 +1,72 @@ +// @vitest-environment happy-dom +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import { + _resetLayoutOwnerPaneIntentsForTests, + holdTabPaneTree, + onTabPaneTreeRedraw +} from '@/runtime/layout-owner-pane-intents' +import type { TerminalPaneCloseController } from './use-terminal-pane-close-actions' + +const reconcile = vi.hoisted(() => vi.fn(() => false)) +vi.mock('./terminal-pane-frame-reconciliation', () => ({ + reconcileMountedPanesToLayout: reconcile +})) + +const { useLayoutOwnerPaneTree } = await import('./use-layout-owner-pane-tree') + +const layout = (expandedLeafId: string | null): TerminalLayoutSnapshot => ({ + root: { type: 'leaf', leafId: 'leaf-1' }, + activeLeafId: 'leaf-1', + expandedLeafId +}) + +function controllerWith(restoredLayout: TerminalLayoutSnapshot): TerminalPaneCloseController { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the hook reads only these fields. + return { + tabId: 'tab-1', + restoredLayout, + isolatedPaneKey: null, + expandedPaneIdRef: { current: null }, + managerRef: { current: {} }, + closePaneLocally: vi.fn(), + onCloseTab: vi.fn(), + persistLayoutSnapshot: vi.fn() + } as unknown as TerminalPaneCloseController +} + +describe('useLayoutOwnerPaneTree', () => { + beforeEach(() => { + _resetLayoutOwnerPaneIntentsForTests() + reconcile.mockClear() + }) + + it('waits while the window’s own pane command holds the tab, then draws main’s tree', () => { + const release = holdTabPaneTree('tab-1') + renderHook(() => useLayoutOwnerPaneTree(controllerWith(layout(null)))) + expect(reconcile).not.toHaveBeenCalled() + + act(() => release()) + + expect(reconcile).toHaveBeenCalledOnce() + }) + + it('asks for main’s tree again when a zoomed pane collapses', () => { + const redraws = vi.fn() + onTabPaneTreeRedraw(redraws) + const { rerender } = renderHook( + ({ snapshot }) => useLayoutOwnerPaneTree(controllerWith(snapshot)), + { + initialProps: { snapshot: layout('leaf-1') } + } + ) + expect(reconcile).toHaveBeenLastCalledWith(expect.objectContaining({ isExpanded: true })) + expect(redraws).not.toHaveBeenCalled() + + rerender({ snapshot: layout(null) }) + + expect(redraws).toHaveBeenCalledWith('tab-1') + expect(reconcile).toHaveBeenLastCalledWith(expect.objectContaining({ isExpanded: false })) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/use-layout-owner-pane-tree.ts b/src/renderer/src/components/terminal-pane/use-layout-owner-pane-tree.ts new file mode 100644 index 00000000000..cca7634b226 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-layout-owner-pane-tree.ts @@ -0,0 +1,77 @@ +import { useEffect, useRef, useState } from 'react' +import { isWebClientLocation } from '@/lib/web-client-location' +import { + isTabPaneTreeHeld, + onTabPaneTreeRedraw, + requestTabPaneTreeRedraw +} from '@/runtime/layout-owner-pane-intents' +import { isHostAuthoritativeLayout } from './terminal-live-layout-reconciliation' +import { reconcileMountedPanesToLayout } from './terminal-pane-frame-reconciliation' +import type { TerminalPaneCloseController } from './use-terminal-pane-close-actions' + +/** + * A layout-owner build's mounted local tab follows main's pane tree, which frames write to the + * store. A paired server's tab keeps the host-authoritative reconciler; a tab the window's own + * command holds waits for that command's answer. + */ +export function useLayoutOwnerPaneTree(controller: TerminalPaneCloseController): void { + const { + closePaneLocally, + expandedPaneIdRef, + isolatedPaneKey, + managerRef, + persistLayoutSnapshot, + restoredLayout, + tabId + } = controller + const [redraws, setRedraws] = useState(0) + useEffect( + () => + onTabPaneTreeRedraw((redrawnTabId) => { + if (redrawnTabId === tabId) { + setRedraws((count) => count + 1) + } + }), + [tabId] + ) + const zoomed = restoredLayout.expandedLeafId != null || isolatedPaneKey !== null + const wasZoomed = useRef(zoomed) + useEffect(() => { + // Collapsing writes the window's pre-zoom tree to the store; main's comes back from frames. + if (wasZoomed.current && !zoomed) { + requestTabPaneTreeRedraw(tabId) + } + wasZoomed.current = zoomed + }, [zoomed, tabId]) + useEffect(() => { + const manager = managerRef.current + if ( + !manager || + isTabPaneTreeHeld(tabId) || + isHostAuthoritativeLayout({ + isWebClient: isWebClientLocation(), + ptyIdsByLeafId: restoredLayout.ptyIdsByLeafId + }) + ) { + return + } + const changed = reconcileMountedPanesToLayout({ + tabId, + manager, + layout: restoredLayout, + closePane: (paneId, leafId, kind) => { + if (kind === 'user') { + closePaneLocally(paneId, leafId) + } else { + // An exited pane closes as the window's exit path closes it: no user-close clean-up. + manager.closePane(paneId) + } + }, + isExpanded: expandedPaneIdRef.current !== null || zoomed + }) + if (changed) { + persistLayoutSnapshot() + } + // oxlint-disable-next-line react-hooks/exhaustive-deps -- Refs and stable callbacks. + }, [restoredLayout, redraws, zoomed, tabId]) +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts index dcf02a8c3a6..267cd62177e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts @@ -20,8 +20,7 @@ import { capturePendingTerminalPaneClose } from './terminal-pane-close-admission import { commitTerminalSurfaceClose } from '@/store/terminals/terminal-surface-close-intent' import { noteAgentLaunchPaneClosedByUser } from '@/lib/agent-launch-pane-closes' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' -import { sendLayoutCommandToOwner } from '@/runtime/layout-owner-command' -import { closeAfterLastPaneRefusal } from './layout-owner-last-pane-refusal' +import { sendUserPaneCloseToLayoutOwner } from './layout-owner-pane-close-command' export function useTerminalPaneCloseActions(controller: TerminalPaneBindingController) { const confirmedCloseRef = useRef<(() => void) | null>(null) @@ -115,20 +114,13 @@ export function useTerminalPaneCloseActions(controller: TerminalPaneBindingContr return } const leafId = manager.getLeafId(paneId) - // Switch point: main closes the pane first; the window then removes it. + // Switch point: main closes the pane first; the window removes it when main's frame does. if ( ORCA_LAYOUT_OWNER_IS_MAIN && leafId && - sendLayoutCommandToOwner( - worktreeId, - { type: 'closePane', tabId, leafId }, - { - apply: () => closePaneLocally(paneId, leafId), - asToday: () => closePaneLocally(paneId), - refused: (code) => - code === 'last_pane' && - closeAfterLastPaneRefusal(managerRef.current, onCloseTab, closePaneLocally, paneId) - } + sendUserPaneCloseToLayoutOwner( + { worktreeId, tabId, leafId, paneId }, + { getManager: () => managerRef.current, closeTab: onCloseTab, closePaneLocally } ) ) { return @@ -304,6 +296,7 @@ export function useTerminalPaneCloseActions(controller: TerminalPaneBindingContr ) return { + closePaneLocally, executeClosePane, getCloseDialogCopyKind, handleRequestClosePane, diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-controller.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-controller.ts index 7f49388a402..35441e9d31e 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-controller.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-controller.ts @@ -15,6 +15,8 @@ import { useTerminalPaneTitleEffects } from './use-terminal-pane-title-effects' import { useTerminalPaneContextActions } from './use-terminal-pane-context-actions' import { useTerminalPaneMobileActions } from './use-terminal-pane-mobile-actions' import { useTerminalPaneProjection } from './use-terminal-pane-projection' +import { useLayoutOwnerPaneTree } from './use-layout-owner-pane-tree' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' export function useTerminalPaneController(props: TerminalPaneProps) { const foundation = useTerminalPaneFoundation(props) @@ -27,6 +29,11 @@ export function useTerminalPaneController(props: TerminalPaneProps) { const close = Object.assign(bindings, useTerminalPaneCloseActions(bindings)) useTerminalPaneLifecycleStage(close) const reconciliation = Object.assign(close, useTerminalPaneReconciliation(close)) + // Switch point: the mounted tab follows main's pane tree. A build constant, so the hook order is fixed. + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + // oxlint-disable-next-line react-hooks/rules-of-hooks -- SAFETY: the condition is a compile-time constant. + useLayoutOwnerPaneTree(close) + } useTerminalPaneGlobalListeners(reconciliation) useTerminalPanePasteListeners(reconciliation) useTerminalPaneTitleEffects(reconciliation) diff --git a/src/renderer/src/lib/pane-manager/pane-layout-geometry-apply.ts b/src/renderer/src/lib/pane-manager/pane-layout-geometry-apply.ts index 8fe8e23de6f..3aa2c083f71 100644 --- a/src/renderer/src/lib/pane-manager/pane-layout-geometry-apply.ts +++ b/src/renderer/src/lib/pane-manager/pane-layout-geometry-apply.ts @@ -1,5 +1,6 @@ import type { TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types' import type { PaneManagerHost } from './pane-manager-host' +import { movePanesToMatchTree } from './pane-tree-match-moves' import { applyDividerStyles, disposeDivider } from './pane-divider' import { findPaneChildren } from './pane-tree-equalization' import { refitPanesUnder } from './pane-tree-ops' @@ -89,17 +90,26 @@ function flipSplitOrientation( * Applies split orientation and ratios from `layout` to the mounted pane tree * in place. Only applies when the DOM already holds exactly the layout's * leaves in the same tree shape, unzoomed; returns whether anything changed. + * With `followFrameTree`, the same leaves in another shape are moved into it first. */ export function applyPaneLayoutGeometry( host: PaneManagerHost, - layout: TerminalPaneLayoutNode + layout: TerminalPaneLayoutNode, + followFrameTree = false ): boolean { const top = host.root.firstElementChild if (!(top instanceof HTMLElement)) { return false } const writes: GeometryWrite[] = [] - if (!planGeometryWrites(layout, top, writes) || writes.length === 0) { + if (!planGeometryWrites(layout, top, writes)) { + if (followFrameTree && movePanesToMatchTree(host, layout)) { + applyPaneLayoutGeometry(host, layout) + return true + } + return false + } + if (writes.length === 0) { return false } for (const write of writes) { diff --git a/src/renderer/src/lib/pane-manager/pane-manager.ts b/src/renderer/src/lib/pane-manager/pane-manager.ts index 4c9b94ec9dd..e1febd2d0df 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager.ts @@ -185,10 +185,11 @@ export class PaneManager { equalizeManagedPaneSizes(this.panes, this.root, this.options.onLayoutChanged) } - /** Applies a host layout's split orientation and ratios to mounted panes in place. - * Why no onLayoutChanged: an applied host geometry must never persist or push back. */ - applyLayoutGeometry(layout: TerminalPaneLayoutNode): boolean { - return applyPaneLayoutGeometry(this.host, layout) + /** Applies a host layout's split orientation and ratios to mounted panes in place; `followFrameTree` + * also moves the same panes into another shape. Why no onLayoutChanged: an applied host + * geometry must never persist or push back. */ + applyLayoutGeometry(layout: TerminalPaneLayoutNode, followFrameTree = false): boolean { + return applyPaneLayoutGeometry(this.host, layout, followFrameTree) } getActivePane(): ManagedPane | null { diff --git a/src/renderer/src/lib/pane-manager/pane-subtree-split.ts b/src/renderer/src/lib/pane-manager/pane-subtree-split.ts index e6be21fae50..95dec4945f7 100644 --- a/src/renderer/src/lib/pane-manager/pane-subtree-split.ts +++ b/src/renderer/src/lib/pane-manager/pane-subtree-split.ts @@ -10,6 +10,7 @@ import { splitManagedPane } from './pane-split-close' export type SplitPaneAroundLeafIdsOptions = { ratio?: number cwd?: string + cwdPromise?: Promise leafId?: string ptyId?: string placement?: 'before' | 'after' diff --git a/src/renderer/src/lib/pane-manager/pane-tree-match-moves.test.ts b/src/renderer/src/lib/pane-manager/pane-tree-match-moves.test.ts new file mode 100644 index 00000000000..8b0206c9de7 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-tree-match-moves.test.ts @@ -0,0 +1,148 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { PaneManagerOptions } from './pane-manager-types' +import type { TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types' +import { serializePaneTree } from '@/components/terminal-pane/layout-serialization' + +// Why: happy-dom has no canvas, so xterm cannot open; reshaping never touches the terminal. +vi.mock('./pane-lifecycle', async (importOriginal) => { + const actual: object = await importOriginal() + return { ...actual, openTerminal: vi.fn() } +}) + +const { PaneManager } = await import('./pane-manager') + +const A = '11111111-1111-4111-8111-111111111111' +const B = '22222222-2222-4222-8222-222222222222' +const C = '33333333-3333-4333-8333-333333333333' +const D = '44444444-4444-4444-8444-444444444444' + +const leaf = (leafId: string): TerminalPaneLayoutNode => ({ type: 'leaf', leafId }) +type Side = string | TerminalPaneLayoutNode +const node = (side: Side): TerminalPaneLayoutNode => (typeof side === 'string' ? leaf(side) : side) +const split = ( + direction: 'vertical' | 'horizontal', + first: Side, + second: Side +): TerminalPaneLayoutNode => ({ + type: 'split', + direction, + first: node(first), + second: node(second), + ratio: 0.5 +}) + +function withoutRatios(node: TerminalPaneLayoutNode | null): unknown { + if (!node || node.type === 'leaf') { + return node + } + return { + direction: node.direction, + first: withoutRatios(node.first), + second: withoutRatios(node.second) + } +} + +describe('PaneManager.applyLayoutGeometry following the frame tree', () => { + let root: HTMLDivElement + let onLayoutChanged: Mock> + let onPaneCreated: Mock> + let onPaneClosed: Mock> + let manager: InstanceType + + // A | (B / C), and with D: A | (B / (C | D)). + function mount(withD = false): void { + const a = manager.createInitialPane({ leafId: A }) + const b = manager.splitPane(a.id, 'vertical', { leafId: B }) + const c = manager.splitPane(b!.id, 'horizontal', { leafId: C }) + if (withD) { + manager.splitPane(c!.id, 'vertical', { leafId: D }) + } + onLayoutChanged.mockClear() + onPaneCreated.mockClear() + onPaneClosed.mockClear() + } + + const serialized = (): TerminalPaneLayoutNode | null => { + const top = root.firstElementChild + return top instanceof HTMLElement ? serializePaneTree(top) : null + } + const identity = (): unknown[] => + manager + .getPanes() + .map((pane) => [pane.leafId, pane.container, pane.terminal]) + .sort() + + beforeEach(() => { + vi.stubGlobal('requestAnimationFrame', () => 1) + vi.stubGlobal('cancelAnimationFrame', () => {}) + root = document.createElement('div') + document.body.appendChild(root) + onLayoutChanged = vi.fn() + onPaneCreated = vi.fn() + onPaneClosed = vi.fn() + manager = new PaneManager(root, { + linkOpenHint: () => '', + onLayoutChanged, + onPaneCreated, + onPaneClosed + }) + }) + + afterEach(() => { + manager.destroy() + root.remove() + vi.unstubAllGlobals() + }) + + it.each([ + ['a pane moved to the other side', split('vertical', split('horizontal', B, C), A)], + ['a regrouped tree', split('horizontal', split('vertical', A, B), C)], + ['a flat order change', split('vertical', C, split('vertical', A, B))] + ])('takes %s by moving the mounted panes', (_name, layout) => { + mount() + const before = identity() + + expect(manager.applyLayoutGeometry(layout, true)).toBe(true) + + expect(withoutRatios(serialized())).toEqual(withoutRatios(layout)) + expect(identity()).toEqual(before) + expect(onPaneCreated).not.toHaveBeenCalled() + expect(onPaneClosed).not.toHaveBeenCalled() + expect(onLayoutChanged).not.toHaveBeenCalled() + }) + + it('takes a deeper tree, splits on both sides', () => { + mount(true) + const layout = split('horizontal', split('vertical', D, A), split('vertical', C, B)) + const before = identity() + + expect(manager.applyLayoutGeometry(layout, true)).toBe(true) + + expect(withoutRatios(serialized())).toEqual(withoutRatios(layout)) + expect(identity()).toEqual(before) + }) + + it('moves nothing for the shape already mounted', () => { + mount() + const layout = serialized()! + const observer = new MutationObserver(() => {}) + observer.observe(root, { subtree: true, childList: true }) + + expect(manager.applyLayoutGeometry(layout, true)).toBe(false) + + expect(observer.takeRecords()).toEqual([]) + }) + + it('changes nothing when the layout names other panes', () => { + mount() + const before = serialized() + + expect(manager.applyLayoutGeometry(split('vertical', A, B), true)).toBe(false) + expect(manager.applyLayoutGeometry(split('vertical', A, split('vertical', B, D)), true)).toBe( + false + ) + + expect(serialized()).toEqual(before) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-tree-match-moves.ts b/src/renderer/src/lib/pane-manager/pane-tree-match-moves.ts new file mode 100644 index 00000000000..05dffae4a46 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-tree-match-moves.ts @@ -0,0 +1,64 @@ +import type { TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types' +import type { PaneManagerHost } from './pane-manager-host' +import type { ManagedPaneInternal } from './pane-manager-types' +import { updateMultiPaneState } from './pane-drag-reorder' +import { detachPaneFromTree, insertPaneNextTo } from './pane-tree-ops' + +function leftmostLeafId(node: TerminalPaneLayoutNode): string { + return node.type === 'leaf' ? node.leafId : leftmostLeafId(node.first) +} + +function leafIdsOf(node: TerminalPaneLayoutNode, into: string[] = []): string[] { + if (node.type === 'leaf') { + into.push(node.leafId) + } else { + leafIdsOf(node.first, into) + leafIdsOf(node.second, into) + } + return into +} + +/** + * Rearranges the mounted panes into `layout`'s shape by moving their containers, so no terminal is + * recreated; ratios are left to `applyPaneLayoutGeometry`. False, changing nothing, when the + * mounted panes are not exactly the layout's leaves. Never reports a layout change: the shape is + * the host's, not an edit to push back. + */ +export function movePanesToMatchTree( + host: PaneManagerHost, + layout: TerminalPaneLayoutNode +): boolean { + const callbacks = host.getDragCallbacks() + const paneByLeafId = new Map() + for (const pane of host.panes.values()) { + paneByLeafId.set(pane.leafId, pane) + } + const leafIds = leafIdsOf(layout) + if (leafIds.length !== paneByLeafId.size || leafIds.some((id) => !paneByLeafId.has(id))) { + return false + } + const pane = (leafId: string): ManagedPaneInternal => paneByLeafId.get(leafId)! + for (const leafId of leafIds.slice(1)) { + detachPaneFromTree(pane(leafId), callbacks) + } + // Each split's region is held by its leftmost pane; its second side is placed before either side + // is divided, so the new split wraps the whole region. + const place = (node: TerminalPaneLayoutNode): void => { + if (node.type === 'leaf') { + return + } + insertPaneNextTo( + pane(leftmostLeafId(node.second)), + pane(leftmostLeafId(node.first)), + node.direction === 'vertical' ? 'right' : 'bottom', + callbacks + ) + place(node.first) + place(node.second) + } + place(layout) + callbacks.applyPaneOpacity() + callbacks.applyDividerStyles() + updateMultiPaneState(callbacks) + return true +} diff --git a/src/renderer/src/lib/window-session-write-activity.ts b/src/renderer/src/lib/window-session-write-activity.ts index 4f9ecdc1bf7..519d19e4388 100644 --- a/src/renderer/src/lib/window-session-write-activity.ts +++ b/src/renderer/src/lib/window-session-write-activity.ts @@ -1,10 +1,8 @@ // Whether this window owes main a session write: one is scheduled or not yet acknowledged. Fed by // the session write subscriber in a layout-owner build only; layout frames wait while it is busy, -// so a frame never replaces a change main has not received yet. Frames also wait while a layout -// command whose answer the window draws itself is in flight. +// so a frame never replaces a change main has not received yet. let busy = false -let held = 0 let idleListener: (() => void) | null = null export function reportWindowSessionWriteActivity(nextBusy: boolean): void { @@ -12,29 +10,13 @@ export function reportWindowSessionWriteActivity(nextBusy: boolean): void { return } busy = nextBusy - if (!isWindowSessionWriteBusy()) { + if (!busy) { idleListener?.() } } -/** Holds layout frames until the returned release runs; releasing twice is a no-op. */ -export function holdLayoutFramesForWindowCommand(): () => void { - held += 1 - let released = false - return () => { - if (released) { - return - } - released = true - held -= 1 - if (!isWindowSessionWriteBusy()) { - idleListener?.() - } - } -} - export function isWindowSessionWriteBusy(): boolean { - return busy || held > 0 + return busy } /** The one listener: the window's layout frame sync. */ diff --git a/src/renderer/src/lib/worktree-agent-live-surface-adoption-layout-owner.test.ts b/src/renderer/src/lib/worktree-agent-live-surface-adoption-layout-owner.test.ts index 089b998c37a..221103c3f76 100644 --- a/src/renderer/src/lib/worktree-agent-live-surface-adoption-layout-owner.test.ts +++ b/src/renderer/src/lib/worktree-agent-live-surface-adoption-layout-owner.test.ts @@ -186,4 +186,45 @@ describe('adopting live terminals in a layout-owner build', () => { expect(adoption).toEqual({ surfaced: false, declinedPtyIds: [LEFT_PTY] }) expect(createTab).not.toHaveBeenCalled() }) + + it('declines a terminal, then the snapshot arriving after the sweep draws its tab with no new sweep', async () => { + const { window: windowSession, main } = sessions() + const store = createTestStore() + await bootWindowFromPartitions(store, [{ hostId: 'local', session: windowSession }]) + sync = openWorkspaceLayoutFrameSync(store) + provideLayoutFrameTerminalPanes(sync.holdsTerminalPane) + sync.start() + const createTab = vi.spyOn(store.getState(), 'createTab') + const readOwners = vi.fn(async () => owners) + + await expect( + adoptLiveWorkspacePtySurfaces(store.getState, GIT_KEY, [LEFT_PTY, RIGHT_PTY], readOwners) + ).resolves.toEqual({ surfaced: false, declinedPtyIds: [LEFT_PTY, RIGHT_PTY] }) + + frames.onFrame!(snapshotOf(main)) + await flushMicrotasks() + + expect(store.getState().terminalLayoutsByTabId['tab-split']?.ptyIdsByLeafId).toEqual({ + [leaf(2)]: LEFT_PTY, + [leaf(3)]: RIGHT_PTY + }) + expect(createTab).not.toHaveBeenCalled() + expect(readOwners).toHaveBeenCalledOnce() + }) + + it('declines a terminal in a workspace the window does not show yet, which frames do not draw', async () => { + const { main } = sessions() + const store = createTestStore() + await bootWindowFromPartitions(store, [{ hostId: 'local', session: emptySession() }]) + sync = openWorkspaceLayoutFrameSync(store) + provideLayoutFrameTerminalPanes(sync.holdsTerminalPane) + sync.start() + frames.onFrame!(snapshotOf(main)) + await flushMicrotasks() + + expect( + sync.holdsTerminalPane(GIT_KEY, { tabId: 'tab-split', leafId: leaf(2), ptyId: LEFT_PTY }) + ).toBe(false) + expect(store.getState().terminalLayoutsByTabId['tab-split']).toBeUndefined() + }) }) diff --git a/src/renderer/src/runtime/layout-apply-send.ts b/src/renderer/src/runtime/layout-apply-send.ts index 1148f5206f0..a0a76a7349b 100644 --- a/src/renderer/src/runtime/layout-apply-send.ts +++ b/src/renderer/src/runtime/layout-apply-send.ts @@ -2,6 +2,7 @@ import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { afterLayoutFrame } from '@/runtime/layout-frame-position' +import { holdTabPaneTree } from '@/runtime/layout-owner-pane-intents' import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' import type { TabGroupLayoutNode } from '../../../shared/tab-types' import type { TerminalPaneLayoutNode } from '../../../shared/terminal-tab-types' @@ -90,8 +91,23 @@ export type LayoutApplyHandlers = { apply: (result: LayoutCommandResult) => void asToday: () => void refused?: (code: LayoutRefusalCode) => void + /** Main's copy lacks the target, or the call threw; `asToday` when absent. A pane split or move + * passes one: main keeps its pane trees over the window's saves, so that change would not last. + * A close keeps `asToday`: a pane main lacks has nothing to keep it. */ + unapplied?: () => void } +/** Commands that change one tab's panes: frames wait to redraw that tab until the answer is in. */ +const PANE_COMMANDS: ReadonlySet = new Set([ + 'splitPane', + 'closePane', + 'renamePane', + 'equalizePanes', + 'movePane', + 'setPaneRatios', + 'movePaneToNewTab' +]) + /** * A layout command in a layout-owner build: main applies it first, then, once the window has the * frame with main's change, `apply` shows main's result. `asToday` runs instead when main cannot @@ -110,16 +126,36 @@ export function sendLayoutApply( if (getExplicitRuntimeEnvironmentIdForWorktree(state, worktreeId)) { return false } + const hold = + PANE_COMMANDS.has(command.type) && 'tabId' in command ? holdTabPaneTree(command.tabId) : null + // Why a task later: a handler that resolves a promise (drag-out) acts in its awaiting caller, + // microtasks after the handler returns. + const release = (): void => { + if (hold) { + setTimeout(hold, 0) + } + } void callRuntimeRpc({ kind: 'local' }, 'layout.apply', { command: { ...command, workspace: worktreeId } }) .then( - (reply) => afterLayoutFrame(reply.frameSeq, () => handleReply(command, reply, handlers)), + (reply) => + afterLayoutFrame(reply.frameSeq, () => { + try { + handleReply(command, reply, handlers) + } finally { + release() + } + }), (error: unknown) => { - // Why as today: a thrown write is rolled back. An indeterminate disk write is not, and may - // leave a pane main minted; accepted until main is the only writer. + // Why: a thrown write is rolled back. An indeterminate disk write is not, and may leave a + // pane main minted; accepted until main is the only writer. console.warn(`[layout-owner] ${command.type} failed:`, errorMessage(error)) - handlers.asToday() + try { + unapplied(handlers) + } finally { + release() + } } ) // A throwing handler is not a failed call; it must not run the fallback too. @@ -136,22 +172,29 @@ function handleReply( ): void { if (reply.ok) { handlers.apply(reply.result) - } else if ( - reply.code === 'host_not_owned' || - reply.code === 'other_partition' || - mainLacksTarget(reply.code) - ) { - // Why lacking: main's copy lags the window's here (the window still saves its own). + } else if (reply.code === 'host_not_owned' || reply.code === 'other_partition') { handlers.asToday() + } else if (mainLacksTarget(reply.code)) { + // Why: main's copy lags the window's here (the window still saves its tabs). + console.warn(`[layout-owner] ${command.type} found no target:`, reply.code) + unapplied(handlers) } else { console.warn(`[layout-owner] ${command.type} refused:`, reply.code) handlers.refused?.(reply.code) } } +function unapplied(handlers: LayoutApplyHandlers): void { + if (handlers.unapplied) { + handlers.unapplied() + } else { + handlers.asToday() + } +} + const keepWindowCopy = (): void => {} -/** Records a change the window already made; any answer leaves it, since the window's save carries it. */ +/** Records a change the window already made; any answer leaves the window's copy as it is. */ export function recordLayoutApply( state: Parameters[0], worktreeId: string, diff --git a/src/renderer/src/runtime/layout-frame-position.test.ts b/src/renderer/src/runtime/layout-frame-position.test.ts index b7fb79ba704..fda99ba5b27 100644 --- a/src/renderer/src/runtime/layout-frame-position.test.ts +++ b/src/renderer/src/runtime/layout-frame-position.test.ts @@ -1,14 +1,18 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { + _resetLayoutFramePositionForTests, afterLayoutFrame, + beginLayoutFramePositions, endLayoutFramePositions, + layoutFramesFlowing, noteLayoutFramePosition } from './layout-frame-position' describe('afterLayoutFrame', () => { afterEach(() => endLayoutFramePositions()) - it('runs at once while no sequenced stream is open, or for a reply that names no frame', () => { + it('runs at once once the stream ended, or for a reply that names no frame', () => { + endLayoutFramePositions() const run = vi.fn() afterLayoutFrame(9, run) expect(run).toHaveBeenCalledTimes(1) @@ -18,6 +22,33 @@ describe('afterLayoutFrame', () => { expect(run).toHaveBeenCalledTimes(2) }) + it('runs at once while the window has opened no stream', () => { + _resetLayoutFramePositionForTests() + const run = vi.fn() + afterLayoutFrame(2, run) + expect(run).toHaveBeenCalledOnce() + expect(layoutFramesFlowing()).toBe(false) + }) + + it('waits for an opened stream before its first frame: that frame, or its end', () => { + _resetLayoutFramePositionForTests() + beginLayoutFramePositions() + const framed = vi.fn() + afterLayoutFrame(2, framed) + expect(framed).not.toHaveBeenCalled() + noteLayoutFramePosition(2) + expect(framed).toHaveBeenCalledOnce() + + _resetLayoutFramePositionForTests() + beginLayoutFramePositions() + const ended = vi.fn() + afterLayoutFrame(2, ended) + expect(layoutFramesFlowing()).toBe(true) + endLayoutFramePositions() + expect(ended).toHaveBeenCalledOnce() + expect(layoutFramesFlowing()).toBe(false) + }) + it('runs once the window holds the named frame, in the order frames come in', () => { noteLayoutFramePosition(3) const order: string[] = [] diff --git a/src/renderer/src/runtime/layout-frame-position.ts b/src/renderer/src/runtime/layout-frame-position.ts index c905da60bfe..06379b4755e 100644 --- a/src/renderer/src/runtime/layout-frame-position.ts +++ b/src/renderer/src/runtime/layout-frame-position.ts @@ -6,6 +6,8 @@ type Waiter = { seq: number; run: () => void } /** Null while no sequenced stream is open: nothing is coming to wait for. */ let position: number | null = null +/** 'none' before the window opens its stream; 'coming' from then, even before its first frame. */ +let stream: 'none' | 'coming' | 'ended' = 'none' let waiters: Waiter[] = [] function release(ready: (waiter: Waiter) => boolean): void { @@ -28,25 +30,44 @@ export function noteLayoutFramePosition(seq: number | undefined): void { if (seq === undefined) { return } + stream = 'coming' position = position === null ? seq : Math.max(position, seq) const reached = position release((waiter) => waiter.seq <= reached) } +/** Whether main's frames will come: from when the window opens its stream until it ends. */ +export function layoutFramesFlowing(): boolean { + return stream === 'coming' +} + +/** The window opened its stream: replies wait for its frames, even before the first. */ +export function beginLayoutFramePositions(): void { + stream = 'coming' +} + /** The stream ended: no frame will come, so every waiter runs now. */ export function endLayoutFramePositions(): void { position = null + stream = 'ended' release(() => true) } /** * Runs `run` once this window holds the frame at `seq`: at once when it already does, when the - * reply names no position (an older host), or when no sequenced stream is open. + * reply names no position (an older host), or while no stream is coming. An opened stream is + * coming before its first frame too, so the wait holds until that frame or its end. */ export function afterLayoutFrame(seq: number | undefined, run: () => void): void { - if (seq === undefined || position === null || position >= seq) { + if (seq === undefined || stream !== 'coming' || (position !== null && position >= seq)) { run() return } waiters.push({ seq, run }) } + +export function _resetLayoutFramePositionForTests(): void { + position = null + stream = 'none' + waiters = [] +} diff --git a/src/renderer/src/runtime/layout-owner-command.test.ts b/src/renderer/src/runtime/layout-owner-command.test.ts index e1c6090d05a..39b505cabf9 100644 --- a/src/renderer/src/runtime/layout-owner-command.test.ts +++ b/src/renderer/src/runtime/layout-owner-command.test.ts @@ -1,6 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { endLayoutFramePositions, noteLayoutFramePosition } from './layout-frame-position' import { sendLayoutCommandToOwner } from './layout-owner-command' +import { + _resetLayoutOwnerPaneIntentsForTests, + isTabPaneTreeHeld +} from './layout-owner-pane-intents' const mocks = vi.hoisted( (): { @@ -30,7 +34,10 @@ function handlers() { } describe('sendLayoutCommandToOwner', () => { - afterEach(() => endLayoutFramePositions()) + afterEach(() => { + endLayoutFramePositions() + _resetLayoutOwnerPaneIntentsForTests() + }) beforeEach(() => { mocks.callRuntimeRpc.mockReset() @@ -105,6 +112,60 @@ describe('sendLayoutCommandToOwner', () => { expect(on.refused).not.toHaveBeenCalled() }) + it.each([ + ['main lacks the pane', { ok: false, code: 'pane_not_found' }], + ['the call fails', new Error('rpc_failed')] + ])('draws nothing for a pane command passing `unapplied` when %s', async (_case, answer) => { + if (answer instanceof Error) { + mocks.callRuntimeRpc.mockRejectedValue(answer) + } else { + mocks.callRuntimeRpc.mockResolvedValue(answer) + } + const on = { ...handlers(), unapplied: vi.fn() } + + sendLayoutCommandToOwner('worktree-1', SPLIT, on) + + await vi.waitFor(() => expect(on.unapplied).toHaveBeenCalledOnce()) + expect(on.asToday).not.toHaveBeenCalled() + await vi.waitFor(() => expect(isTabPaneTreeHeld('tab-1')).toBe(false)) + }) + + it('keeps the tab’s drawn tree from frames until its answer, and what awaits it, has run', async () => { + noteLayoutFramePosition(4) + mocks.callRuntimeRpc.mockResolvedValue({ ok: true, result: {}, frameSeq: 5 }) + const on = handlers() + const heldInCaller = Promise.withResolvers() + on.apply.mockImplementation(() => { + void Promise.resolve() + .then(() => undefined) + .then(() => heldInCaller.resolve(isTabPaneTreeHeld('tab-1'))) + throw new Error('handler failed') + }) + + sendLayoutCommandToOwner('worktree-1', SPLIT, on) + expect(isTabPaneTreeHeld('tab-1')).toBe(true) + await vi.waitFor(() => expect(mocks.callRuntimeRpc).toHaveBeenCalled()) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(isTabPaneTreeHeld('tab-1')).toBe(true) + + noteLayoutFramePosition(5) + expect(on.apply).toHaveBeenCalledOnce() + await expect(heldInCaller.promise).resolves.toBe(true) + await vi.waitFor(() => expect(isTabPaneTreeHeld('tab-1')).toBe(false)) + }) + + it('holds no tab for a command that changes no pane tree', async () => { + mocks.callRuntimeRpc.mockResolvedValue({ ok: true, result: {} }) + + sendLayoutCommandToOwner( + 'worktree-1', + { type: 'renameTab', tabId: 'tab-1', kind: 'custom', title: 'logs' }, + handlers() + ) + + expect(isTabPaneTreeHeld('tab-1')).toBe(false) + }) + it('does not run the fallback when the caller’s own handler throws', async () => { mocks.callRuntimeRpc.mockResolvedValue({ ok: true, result: { leafId: 'leaf-minted' } }) const on = handlers() diff --git a/src/renderer/src/runtime/layout-owner-pane-intents.test.ts b/src/renderer/src/runtime/layout-owner-pane-intents.test.ts new file mode 100644 index 00000000000..567d44a5ec0 --- /dev/null +++ b/src/renderer/src/runtime/layout-owner-pane-intents.test.ts @@ -0,0 +1,87 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + _resetLayoutOwnerPaneIntentsForTests, + expectPaneToAppear, + expectPaneToClose, + expectTabRowFields, + holdTabPaneTree, + isTabPaneTreeHeld, + onTabPaneTreeRedraw, + panesLeftAfterPendingCloses, + requestTabPaneTreeRedraw, + takePaneAppearIntent, + takePaneCloseIntent, + takeTabRowFields +} from './layout-owner-pane-intents' + +const telemetry = { source: 'keyboard', direction: 'vertical' } as const + +describe('layout-owner pane intents', () => { + beforeEach(() => _resetLayoutOwnerPaneIntentsForTests()) + + it('hands an intent out once, and only for its own tab', () => { + expectPaneToAppear('leaf-new', { tabId: 'tab-1', cwd: '/repo', telemetry }) + expectPaneToClose('tab-1', 'leaf-old', 'user') + + expect(takePaneAppearIntent('tab-2', 'leaf-new')).toBeNull() + expect(takePaneAppearIntent('tab-1', 'leaf-new')).toMatchObject({ cwd: '/repo' }) + expect(takePaneAppearIntent('tab-1', 'leaf-new')).toBeNull() + expect(takePaneCloseIntent('tab-2', 'leaf-old')).toBeNull() + expect(takePaneCloseIntent('tab-1', 'leaf-old')).toBe('user') + expect(takePaneCloseIntent('tab-1', 'leaf-old')).toBeNull() + }) + + it('forgets only its own intent, not a newer one for the same pane', () => { + const forgetFirst = expectPaneToClose('tab-1', 'leaf-1', 'user') + expectPaneToClose('tab-1', 'leaf-1', 'exit') + + forgetFirst() + + expect(takePaneCloseIntent('tab-1', 'leaf-1')).toBe('exit') + }) + + it('keeps only the newest intents', () => { + for (let index = 0; index < 65; index += 1) { + expectPaneToAppear(`leaf-${index}`, { tabId: 'tab-1', telemetry }) + } + + expect(takePaneAppearIntent('tab-1', 'leaf-0')).toBeNull() + expect(takePaneAppearIntent('tab-1', 'leaf-64')).not.toBeNull() + }) + + it('hands out a new row’s fields once, until released', () => { + const release = expectTabRowFields('tab-1', { pendingActivationSpawn: true }) + expect(takeTabRowFields('tab-1')).toEqual({ pendingActivationSpawn: true }) + expect(takeTabRowFields('tab-1')).toBeNull() + + expectTabRowFields('tab-2', { pendingActivationSpawn: true }) + release() + expect(takeTabRowFields('tab-2')).toEqual({ pendingActivationSpawn: true }) + }) + + it('holds a tab until its last hold is released, then tells the listener once', () => { + const listener = vi.fn() + onTabPaneTreeRedraw(listener) + const first = holdTabPaneTree('tab-1') + const second = holdTabPaneTree('tab-1') + + first() + first() + expect(isTabPaneTreeHeld('tab-1')).toBe(true) + expect(listener).not.toHaveBeenCalled() + + second() + expect(isTabPaneTreeHeld('tab-1')).toBe(false) + expect(listener).toHaveBeenCalledExactlyOnceWith('tab-1') + + requestTabPaneTreeRedraw('tab-2') + expect(listener).toHaveBeenLastCalledWith('tab-2') + }) + + it('counts the panes no pending close of this tab takes', () => { + expectPaneToClose('tab-1', 'a', 'exit') + expectPaneToClose('tab-2', 'b', 'user') + + expect(panesLeftAfterPendingCloses('tab-1', ['a', 'b', 'c'])).toBe(2) + }) +}) diff --git a/src/renderer/src/runtime/layout-owner-pane-intents.ts b/src/renderer/src/runtime/layout-owner-pane-intents.ts new file mode 100644 index 00000000000..e433ac0966b --- /dev/null +++ b/src/renderer/src/runtime/layout-owner-pane-intents.ts @@ -0,0 +1,139 @@ +// What this window asked main to do to a tab's panes, in a layout-owner build: main's frames say +// which panes a tab has, these say what the window wants done when one appears or goes (its +// spawn inputs, its close) and keep a tab's drawn tree while the window's own command is in +// flight. A pane a frame adds or drops with no intent here is left to main's notice to the window. + +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { TerminalPaneSplitSource } from '../../../shared/feature-education-telemetry' + +export type PaneAppearIntent = { + tabId: string + cwd?: string + cwdPromise?: Promise + telemetry: { source: TerminalPaneSplitSource; direction: 'vertical' | 'horizontal' } +} + +/** Intents outlive their frame only when a tab never mounts again; keep the newest few. */ +const MAX_INTENTS = 64 + +const appearing = new Map() +/** 'user': the user's close, with its status and launch clean-up. 'exit': its terminal ended. */ +export type PaneCloseKind = 'user' | 'exit' + +const closing = new Map() + +function remember(map: Map, key: string, value: T): void { + map.delete(key) + map.set(key, value) + if (map.size > MAX_INTENTS) { + map.delete(map.keys().next().value!) + } +} + +export function expectPaneToAppear(leafId: string, intent: PaneAppearIntent): () => void { + remember(appearing, leafId, intent) + return () => { + if (appearing.get(leafId) === intent) { + appearing.delete(leafId) + } + } +} + +export function takePaneAppearIntent(tabId: string, leafId: string): PaneAppearIntent | null { + const intent = appearing.get(leafId) + if (intent?.tabId !== tabId) { + return null + } + appearing.delete(leafId) + return intent +} + +export function expectPaneToClose(tabId: string, leafId: string, kind: PaneCloseKind): () => void { + const intent = { tabId, kind } + remember(closing, leafId, intent) + return () => { + if (closing.get(leafId) === intent) { + closing.delete(leafId) + } + } +} + +/** Of these panes, how many no close of this window's is about to take. */ +export function panesLeftAfterPendingCloses(tabId: string, leafIds: readonly string[]): number { + return leafIds.filter((leafId) => closing.get(leafId)?.tabId !== tabId).length +} + +export function takePaneCloseIntent(tabId: string, leafId: string): PaneCloseKind | null { + const intent = closing.get(leafId) + if (intent?.tabId !== tabId) { + return null + } + closing.delete(leafId) + return intent.kind +} + +/** Row fields only this window knows, for a tab main's frame will create. */ +export type TabRowFields = Pick + +const rowFields = new Map() + +export function expectTabRowFields(tabId: string, fields: TabRowFields): () => void { + remember(rowFields, tabId, fields) + return () => { + if (rowFields.get(tabId) === fields) { + rowFields.delete(tabId) + } + } +} + +export function takeTabRowFields(tabId: string): TabRowFields | null { + const fields = rowFields.get(tabId) ?? null + rowFields.delete(tabId) + return fields +} + +const heldTabs = new Map() +const redrawListeners = new Set<(tabId: string) => void>() + +/** Keeps the tab's drawn tree from frames until the release; releasing twice is a no-op. */ +export function holdTabPaneTree(tabId: string): () => void { + heldTabs.set(tabId, (heldTabs.get(tabId) ?? 0) + 1) + let released = false + return () => { + if (released) { + return + } + released = true + const count = (heldTabs.get(tabId) ?? 1) - 1 + if (count > 0) { + heldTabs.set(tabId, count) + return + } + heldTabs.delete(tabId) + requestTabPaneTreeRedraw(tabId) + } +} + +/** Shows main's tree for the tab again, as the frames hold it: after a hold, or a zoom ends. */ +export function requestTabPaneTreeRedraw(tabId: string): void { + for (const listener of redrawListeners) { + listener(tabId) + } +} + +export function isTabPaneTreeHeld(tabId: string): boolean { + return heldTabs.has(tabId) +} + +export function onTabPaneTreeRedraw(listener: (tabId: string) => void): () => void { + redrawListeners.add(listener) + return () => redrawListeners.delete(listener) +} + +export function _resetLayoutOwnerPaneIntentsForTests(): void { + appearing.clear() + closing.clear() + rowFields.clear() + heldTabs.clear() + redrawListeners.clear() +} diff --git a/src/renderer/src/runtime/workspace-layout-frame-sync.test.ts b/src/renderer/src/runtime/workspace-layout-frame-sync.test.ts index 9b48738f7da..6f86a47ce8a 100644 --- a/src/renderer/src/runtime/workspace-layout-frame-sync.test.ts +++ b/src/renderer/src/runtime/workspace-layout-frame-sync.test.ts @@ -21,6 +21,7 @@ import { createStoreSessionMockApi } from '../store/slices/store-session-test-ha import { createTestStore } from '../store/slices/store-test-helpers' import type { LayoutFrameSlices } from '../store/workspace-layout-frame-slices' import { afterLayoutFrame, endLayoutFramePositions } from './layout-frame-position' +import { holdTabPaneTree, requestTabPaneTreeRedraw } from './layout-owner-pane-intents' import { openWorkspaceLayoutFrameSync } from './workspace-layout-frame-sync' vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) @@ -236,6 +237,88 @@ describe('the window drawn from layout frames', () => { sync.close() }) + it('shows main’s pane tree at once even while a write is owed, except a tab the window’s own pane command holds', async () => { + const { store, sync, layouts } = await bootWithFrames('local', localDesktopSession()) + sync.start() + reportWindowSessionWriteActivity(true) + const shellRoot = () => store.getState().terminalLayoutsByTabId['tab-shell']!.root + const withRatio = (ratio: number) => { + const next = structuredClone(layouts[GIT_KEY]!) + const root = next.tabs.find((tab) => tab.id === 'tab-shell')!.terminal!.root! + if (root.type === 'split') { + root.ratio = ratio + } + return next + } + expect(shellRoot()?.type).toBe('split') + + stream.onFrame!({ type: 'workspace', key: GIT_KEY, layout: withRatio(0.2) }) + await flushMicrotasks() + expect(shellRoot()).toMatchObject({ ratio: 0.2 }) + + const release = holdTabPaneTree('tab-shell') + stream.onFrame!({ type: 'workspace', key: GIT_KEY, layout: withRatio(0.7) }) + await flushMicrotasks() + expect(shellRoot()).toMatchObject({ ratio: 0.2 }) + + release() + await flushMicrotasks() + expect(shellRoot()).toMatchObject({ ratio: 0.7 }) + reportWindowSessionWriteActivity(false) + sync.close() + }) + + it('shows main’s pane tree going back to one the window last showed before an owed write', async () => { + const { store, sync, layouts } = await bootWithFrames('local', localDesktopSession()) + sync.start() + const shellRoot = () => store.getState().terminalLayoutsByTabId['tab-shell']!.root + const original = shellRoot() + reportWindowSessionWriteActivity(true) + const changed = structuredClone(layouts[GIT_KEY]!) + const root = changed.tabs.find((tab) => tab.id === 'tab-shell')!.terminal!.root! + if (root.type === 'split') { + root.ratio = 0.15 + } + stream.onFrame!({ type: 'workspace', key: GIT_KEY, layout: changed }) + await flushMicrotasks() + expect(shellRoot()).toMatchObject({ ratio: 0.15 }) + + // Same structure as the frame shown before the write: its pane tree still shows. + stream.onFrame!({ type: 'workspace', key: GIT_KEY, layout: structuredClone(layouts[GIT_KEY]!) }) + await flushMicrotasks() + expect(shellRoot()).toEqual(original) + reportWindowSessionWriteActivity(false) + sync.close() + }) + + it('keeps the window’s own pane trees once the stream has ended', async () => { + const { store, sync, layouts } = await bootWithFrames('local', localDesktopSession()) + sync.start() + await flushMicrotasks() + // A frame queued behind an owed write when the stream ends. + reportWindowSessionWriteActivity(true) + stream.onFrame!({ type: 'workspace', key: GIT_KEY, layout: structuredClone(layouts[GIT_KEY]!) }) + stream.onStatus!({ state: 'lost' }) + const drawn = { type: 'leaf' as const, leafId: 'drawn-by-the-window' } + store.setState({ + terminalLayoutsByTabId: { + ...store.getState().terminalLayoutsByTabId, + 'tab-shell': { ...store.getState().terminalLayoutsByTabId['tab-shell']!, root: drawn } + } + }) + + requestTabPaneTreeRedraw('tab-shell') + await flushMicrotasks() + expect(store.getState().terminalLayoutsByTabId['tab-shell']!.root).toEqual(drawn) + + // Nor does a later flush, such as an owed write ending. + reportWindowSessionWriteActivity(true) + reportWindowSessionWriteActivity(false) + await flushMicrotasks() + expect(store.getState().terminalLayoutsByTabId['tab-shell']!.root).toEqual(drawn) + sync.close() + }) + it('shows a live title change of the focused pane at once, even while a write is owed', async () => { const { store, sync, layouts } = await bootWithFrames('local', localDesktopSession()) sync.start() diff --git a/src/renderer/src/runtime/workspace-layout-frame-sync.ts b/src/renderer/src/runtime/workspace-layout-frame-sync.ts index e8e9179129c..5266493835b 100644 --- a/src/renderer/src/runtime/workspace-layout-frame-sync.ts +++ b/src/renderer/src/runtime/workspace-layout-frame-sync.ts @@ -13,7 +13,10 @@ import type { AppState } from '@/store/types' import { applyLayoutFrame, EMPTY_WORKSPACE_LAYOUT_CACHE } from '@/store/workspace-layout-cache' import type { WorkspaceLayoutCache } from '@/store/workspace-layout-cache' import { buildLayoutFrameSlices, shownLiveTitle } from '@/store/workspace-layout-frame-slices' -import { buildLayoutFrameStorePatch } from '@/store/workspace-layout-frame-store-patch' +import { + buildLayoutFrameStorePatch, + terminalLayoutsPatch +} from '@/store/workspace-layout-frame-store-patch' import { buildWorkspaceSessionPayload } from '@/lib/workspace-session' import { buildHostIdByWorktreeId } from '@/lib/workspace-session-host-persistence' import { @@ -34,8 +37,17 @@ import { loadWorkspaceLayoutWithoutReport } from '../../../shared/workspace-layo import { stableJson } from '../../../shared/workspace-layout/workspace-layout-load-report' import { nameBasedLoadContext } from '../../../shared/workspace-layout/workspace-layout-minted-ids' import type { PublishedWorkspaceLayout } from '../../../shared/workspace-layout/workspace-layout-published' -import { endLayoutFramePositions, noteLayoutFramePosition } from './layout-frame-position' +import { + beginLayoutFramePositions, + endLayoutFramePositions, + noteLayoutFramePosition +} from './layout-frame-position' import { provideLayoutFrameTerminalPanes } from './layout-frame-terminal-panes' +import { + isTabPaneTreeHeld, + onTabPaneTreeRedraw, + takeTabRowFields +} from './layout-owner-pane-intents' import { subscribeRuntimeLayout } from './runtime-layout-client' type FrameSyncStore = { @@ -109,8 +121,16 @@ function liveTitleUpdates( return updates } -function holdsWorkspace(state: AppState, key: string): boolean { - return key in state.groupsByWorktree || key in state.unifiedTabsByWorktree +/** Which workspaces frames draw: ones the window holds that no paired server mirrors. */ +function framesDrawWorkspace(state: AppState): (key: string) => boolean { + let hostIdOf: HostIdByWorktreeId | null = null + return (key) => { + if (!(key in state.groupsByWorktree || key in state.unifiedTabsByWorktree)) { + return false + } + hostIdOf ??= buildHostIdByWorktreeId(state) + return parseExecutionHostId(hostIdOf(key))?.kind !== 'runtime' + } } export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLayoutFrameSync { @@ -122,6 +142,9 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa let snapshotReceived = false let flushQueued = false let streamOpen = true + /** Workspaces whose pane trees the store may still lack: a structure frame came in, or a tab + * the window's own command held skipped one. */ + let paneTreeKeys = new Set() const rebuild = (state: AppState, keys: string[]): void => { // The view and content facts are this window's: read them back out of what it saves. @@ -139,7 +162,43 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa facts, view: desktopView }) - const patch = buildLayoutFrameStorePatch(state, built, keys) + const patch = buildLayoutFrameStorePatch( + state, + built, + keys, + isTabPaneTreeHeld, + takeTabRowFields + ) + if (Object.keys(patch).length > 0) { + store.setState(patch) + } + } + + // Main keeps its own pane trees over the window's saves, so they show at once, even while the + // window owes main a write; only a tab the window's own pane command holds waits. + const showPaneTrees = (state: AppState, drawn: (key: string) => boolean): void => { + let layouts = state.terminalLayoutsByTabId + const waiting = new Set() + for (const key of paneTreeKeys) { + const layout = cache[key] + if (!layout || !drawn(key)) { + continue + } + for (const tab of layout.tabs) { + const root = tab.terminal?.root + const current = root ? layouts[tab.entityId] : undefined + if (!root || !current) { + continue + } + if (isTabPaneTreeHeld(tab.entityId)) { + waiting.add(key) + } else if (stableJson(current.root) !== stableJson(root)) { + layouts = { ...layouts, [tab.entityId]: { ...current, root } } + } + } + } + paneTreeKeys = waiting + const patch = terminalLayoutsPatch(state, layouts) if (Object.keys(patch).length > 0) { store.setState(patch) } @@ -151,23 +210,21 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa return } const state = store.getState() - let hostIdOf: HostIdByWorktreeId | null = null - const ownedByPairedServer = (key: string): boolean => { - hostIdOf ??= buildHostIdByWorktreeId(state) - return parseExecutionHostId(hostIdOf(key))?.kind === 'runtime' - } + const drawn = framesDrawWorkspace(state) const busy = isWindowSessionWriteBusy() const nextShown: Record = {} const structural: string[] = [] const titleUpdates: TerminalTabTitleUpdate[] = [] for (const [key, layout] of Object.entries(cache)) { const previous = shown[key] - if (layout === previous || !holdsWorkspace(state, key) || ownedByPairedServer(key)) { + if (layout === previous || !drawn(key)) { if (previous) { nextShown[key] = previous } continue } + // Against the store, not `shown`: a held frame leaves `shown` behind the pane trees drawn since. + paneTreeKeys.add(key) if (previous && structureJson(previous) === structureJson(layout)) { titleUpdates.push( ...liveTitleUpdates( @@ -194,6 +251,10 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa if (titleUpdates.length > 0) { store.getState().updateTabTitles(titleUpdates) } + // A closed stream's cache is frozen: the window draws its own answers then. + if (streamOpen && paneTreeKeys.size > 0) { + showPaneTrees(store.getState(), framesDrawWorkspace(store.getState())) + } } const scheduleFlush = (): void => { @@ -204,6 +265,7 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa } } + beginLayoutFramePositions() const stream = subscribeRuntimeLayout( { kind: 'local' }, {}, @@ -224,10 +286,24 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa endLayoutFramePositions() } }, - // After the frame's flush: a waiting reply runs once the store shows the frame, unless held. + // After the frame's flush. A busy write may still hold the frame's tabs (never its pane + // trees), so a waiting reply checks the store rather than assume the frame shows. (seq) => queueMicrotask(() => streamOpen && noteLayoutFramePosition(seq)) ) const stopIdleListener = onWindowSessionWriteIdle(scheduleFlush) + // Against the store: the window may have drawn over main's tree for this tab since. + const stopReleaseListener = onTabPaneTreeRedraw((tabId) => { + // A closed stream's cache is frozen: the window draws its own answers then. + if (!streamOpen) { + return + } + for (const [key, layout] of Object.entries(cache)) { + if (layout.tabs.some((tab) => tab.entityId === tabId)) { + paneTreeKeys.add(key) + } + } + scheduleFlush() + }) return { start: () => { @@ -241,15 +317,19 @@ export function openWorkspaceLayoutFrameSync(store: FrameSyncStore): WorkspaceLa close: () => { live = false stopIdleListener() + stopReleaseListener() stream.close() }, + // Only a pane flush draws: the same workspaces it shows. holdsTerminalPane: (worktreeId, { tabId, leafId, ptyId }) => - cache[worktreeId]?.tabs.some( + framesDrawWorkspace(store.getState())(worktreeId) && + (cache[worktreeId]?.tabs.some( (tab) => tab.entityId === tabId && (tab.terminal?.panes.some((pane) => pane.leafId === leafId && pane.ptyId === ptyId) ?? false) - ) ?? false + ) ?? + false) } } diff --git a/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.test.ts b/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.test.ts index d18096116f2..e128b5fa83f 100644 --- a/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.test.ts +++ b/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.test.ts @@ -1,7 +1,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AppState } from '../types' import type { LayoutApplyReply } from '../../../../shared/workspace-layout/workspace-layout-command-types' -import { isWindowSessionWriteBusy } from '@/lib/window-session-write-activity' +import { + _resetLayoutFramePositionForTests, + beginLayoutFramePositions, + endLayoutFramePositions, + noteLayoutFramePosition +} from '@/runtime/layout-frame-position' +import { + _resetLayoutOwnerPaneIntentsForTests, + takeTabRowFields +} from '@/runtime/layout-owner-pane-intents' import { createTestStore, makeTab } from '../slices/store-test-helpers' import { createInitialTerminalThroughLayoutOwner } from './terminal-tab-create-through-layout-owner' @@ -40,6 +49,8 @@ function start(state: () => AppState = () => stateWith()) { const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) beforeEach(() => { + _resetLayoutOwnerPaneIntentsForTests() + endLayoutFramePositions() mocks.pairedServer = null mocks.callRuntimeRpc.mockReset() mocks.callRuntimeRpc.mockImplementation( @@ -52,7 +63,7 @@ beforeEach(() => { }) describe('createInitialTerminalThroughLayoutOwner', () => { - it('asks main for the first tab and draws main’s answer, holding frames meanwhile', async () => { + it('asks main for the first tab and leaves drawing it, with the window’s row fields, to main’s frame', async () => { const { create, routed, command } = start() expect(routed).toBe(true) @@ -61,25 +72,43 @@ describe('createInitialTerminalThroughLayoutOwner', () => { workspace: WORKTREE, initial: 'activation' }) - expect(isWindowSessionWriteBusy()).toBe(true) + noteLayoutFramePosition(1) + mocks.reply?.({ ok: true, result: { tabId: command.tabId, leafId: command.leafId } }) + await settle() + + expect(create).not.toHaveBeenCalled() + // Kept for the frame, however late an owed write lets it in. + expect(takeTabRowFields(command.tabId)).toEqual({ pendingActivationSpawn: true }) + }) + + it('draws main’s tab itself, under main’s ids, when no frame is coming', async () => { + const { create, command } = start() + mocks.reply?.({ ok: true, result: { tabId: command.tabId, leafId: command.leafId } }) await settle() expect(create).toHaveBeenCalledWith({ tabId: command.tabId, leafId: command.leafId }) - expect(isWindowSessionWriteBusy()).toBe(false) + expect(takeTabRowFields(command.tabId)).toBeNull() }) - it('draws nothing when main’s copy already has a tab there', async () => { - const { create } = start() + it('waits for a stream not yet in, and draws main’s tab itself if it ends first', async () => { + _resetLayoutFramePositionForTests() + beginLayoutFramePositions() + const { create, command } = start() - mocks.reply?.({ ok: true, result: {} }) + mocks.reply?.({ + ok: true, + result: { tabId: command.tabId, leafId: command.leafId }, + frameSeq: 1 + }) await settle() - expect(create).not.toHaveBeenCalled() - expect(isWindowSessionWriteBusy()).toBe(false) + + endLayoutFramePositions() + expect(create).toHaveBeenCalledWith({ tabId: command.tabId, leafId: command.leafId }) }) - it('does not draw a tab twice when the window already shows it', async () => { + it('draws nothing itself when a frame showed the tab before the stream ended', async () => { let state = stateWith() const { create, command } = start(() => state) @@ -90,6 +119,16 @@ describe('createInitialTerminalThroughLayoutOwner', () => { expect(create).not.toHaveBeenCalled() }) + it('draws nothing when main’s copy already has a tab there', async () => { + const { create, command } = start() + + mocks.reply?.({ ok: true, result: {} }) + await settle() + + expect(create).not.toHaveBeenCalled() + expect(takeTabRowFields(command.tabId)).toBeNull() + }) + it('opens the terminal as today when main holds no partition for the workspace, or fails', async () => { const unowned = start() mocks.reply?.({ ok: false, code: 'host_not_owned' }) @@ -101,7 +140,7 @@ describe('createInitialTerminalThroughLayoutOwner', () => { mocks.fail?.(new Error('layout_store_unavailable')) await settle() expect(failed.create).toHaveBeenCalledWith() - expect(isWindowSessionWriteBusy()).toBe(false) + expect(takeTabRowFields(failed.command.tabId)).toBeNull() }) it('opens nothing as today once the user has left the workspace', async () => { diff --git a/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts b/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts index ed85b535ccf..284d2857a2c 100644 --- a/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts +++ b/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts @@ -4,7 +4,8 @@ // (select, retitle, anchor, mount, launch). Main's copy reaches the window through layout frames. import { createBrowserUuid } from '@/lib/browser-uuid' -import { holdLayoutFramesForWindowCommand } from '@/lib/window-session-write-activity' +import { afterLayoutFrame, layoutFramesFlowing } from '@/runtime/layout-frame-position' +import { expectTabRowFields } from '@/runtime/layout-owner-pane-intents' import { getExplicitRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { newGroupSplitOf } from '@/runtime/layout-owner-new-group-split' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' @@ -80,11 +81,11 @@ export function recordCreatedTerminalTabInLayoutOwner( /** * A workspace's first terminal on activation (F23). Main creates it only into a workspace its copy - * holds no tab of, so this window and the runtime asking at once still make one tab. The window - * draws main's answer itself, under main's ids; frames wait meanwhile so they cannot draw it first - * without the window's own facts (main gets its shell and cwd with the window's next save). False - * when a paired server owns the workspace's layout. `create` with no ids opens the terminal as - * today: main holds no partition for the workspace. + * holds no tab of, so this window and the runtime asking at once still make one tab. Main's frame + * draws it, with the window's own row fields; the window draws main's tab itself, under main's + * ids, only when no frame is coming (the stream is down). False when a paired server owns the + * workspace's layout. `create` with no ids opens the terminal as today: main holds no partition + * for the workspace. */ export function createInitialTerminalThroughLayoutOwner( getState: () => AppState, @@ -103,7 +104,8 @@ export function createInitialTerminalThroughLayoutOwner( create() } } - const release = holdLayoutFramesForWindowCommand() + // Kept until the frame bringing main's tab takes it, even one an owed write holds back. + const release = expectTabRowFields(tabId, ACTIVATION_ROW_FIELDS) void callRuntimeRpc({ kind: 'local' }, 'layout.apply', { command: { type: 'createTerminalTab', @@ -114,19 +116,29 @@ export function createInitialTerminalThroughLayoutOwner( } }) .then( - (reply) => { - if (reply.ok) { - // No tab id: main's copy already shows a tab here, which its frames bring. - if (reply.result.tabId === tabId && !holdsTerminalTab(getState(), tabId)) { - create({ tabId, leafId }) + (reply) => + // After the frame with main's tab: the stream may end first, and the window draws it then. + afterLayoutFrame(reply.frameSeq, () => { + if (reply.ok && reply.result.tabId === tabId) { + // Why the store check: a frame may have drawn it before the stream ended. + if (!layoutFramesFlowing()) { + release() + if (!holdsTerminalTab(getState(), tabId)) { + create({ tabId, leafId }) + } + } + return } - } else if (reply.code === 'host_not_owned') { - createAsToday() - } else { - console.warn('[layout-owner] initial createTerminalTab refused:', reply.code) - } - }, + // No tab id: main's copy already shows a tab here, which its frames bring. + release() + if (!reply.ok && reply.code === 'host_not_owned') { + createAsToday() + } else if (!reply.ok) { + console.warn('[layout-owner] initial createTerminalTab refused:', reply.code) + } + }), (error: unknown) => { + release() // Why as today: a thrown write is rolled back, so main holds no such tab. console.warn( '[layout-owner] initial createTerminalTab failed:', @@ -141,10 +153,12 @@ export function createInitialTerminalThroughLayoutOwner( error instanceof Error ? error.message : error ) }) - .finally(release) return true } +/** The window's own mark on a first terminal: its spawn does not count as workspace activity. */ +const ACTIVATION_ROW_FIELDS = { pendingActivationSpawn: true } as const + function holdsTerminalTab(state: AppState, tabId: string): boolean { return Object.values(state.tabsByWorktree).some((tabs) => tabs.some((tab) => tab.id === tabId)) } diff --git a/src/renderer/src/store/workspace-layout-frame-store-patch.test.ts b/src/renderer/src/store/workspace-layout-frame-store-patch.test.ts index b82de545ea6..72c124edd70 100644 --- a/src/renderer/src/store/workspace-layout-frame-store-patch.test.ts +++ b/src/renderer/src/store/workspace-layout-frame-store-patch.test.ts @@ -39,7 +39,15 @@ async function restored(): Promise<{ function patchFor( state: AppState, session: WorkspaceSessionState, - frame: PublishedWorkspaceLayout + frame: PublishedWorkspaceLayout, + ...windowSide: Parameters extends [ + unknown, + unknown, + unknown, + ...infer Rest + ] + ? Rest + : never ) { const { facts, desktopView } = loadWorkspaceLayoutWithoutReport( 'local', @@ -47,7 +55,7 @@ function patchFor( nameBasedLoadContext() ) const built = buildLayoutFrameSlices({ layouts: { [GIT_KEY]: frame }, facts, view: desktopView }) - return buildLayoutFrameStorePatch(state, built, [GIT_KEY]) + return buildLayoutFrameStorePatch(state, built, [GIT_KEY], ...windowSide) } function withoutTab(frame: PublishedWorkspaceLayout, tabId: string): PublishedWorkspaceLayout { @@ -75,6 +83,40 @@ describe('laying frame slices over the store', () => { expect(patch.unifiedTabsByWorktree?.[GIT_KEY]?.map((tab) => tab.id)).not.toContain('tab-shell') }) + it('keeps the pane layout of a tab the window’s own pane command holds, until main drops the tab', async () => { + const { state, session, frame } = await restored() + const newRatios: PublishedWorkspaceLayout = { + ...frame, + tabs: frame.tabs.map((tab) => + tab.id === 'tab-shell' && tab.terminal?.root?.type === 'split' + ? { ...tab, terminal: { ...tab.terminal, root: { ...tab.terminal.root, ratio: 0.2 } } } + : tab + ) + } + const held = (id: string): boolean => id === 'tab-shell' + + expect(patchFor(state, session, newRatios).terminalLayoutsByTabId?.['tab-shell']).toBeDefined() + expect(patchFor(state, session, newRatios, held).terminalLayoutsByTabId).toBeUndefined() + expect( + patchFor(state, session, withoutTab(frame, 'tab-shell'), held).terminalLayoutsByTabId + ).not.toHaveProperty('tab-shell') + }) + + it('gives a row the frame creates the window’s own fields for it', async () => { + const { state, session, frame } = await restored() + const shown = patchFor(state, session, withoutTab(frame, 'tab-shell')) + const before = { ...state, ...shown } + const fields = vi.fn((id: string) => + id === 'tab-shell' ? { pendingActivationSpawn: true } : null + ) + + const patch = patchFor(before, session, frame, undefined, fields) + + const row = patch.tabsByWorktree?.[GIT_KEY]?.find((tab) => tab.id === 'tab-shell') + expect(row?.pendingActivationSpawn).toBe(true) + expect(fields).toHaveBeenCalledTimes(1) + }) + it('keeps an open file’s editor state and place, and drops a file main closed', async () => { const { state, session, frame } = await restored() const file = state.openFiles.find((entry) => entry.worktreeId === GIT_KEY)! diff --git a/src/renderer/src/store/workspace-layout-frame-store-patch.ts b/src/renderer/src/store/workspace-layout-frame-store-patch.ts index 10ece56b42d..c0855cd4e65 100644 --- a/src/renderer/src/store/workspace-layout-frame-store-patch.ts +++ b/src/renderer/src/store/workspace-layout-frame-store-patch.ts @@ -49,9 +49,14 @@ function withoutWindowFields(tab: TerminalTab): Partial { return fields } -function mergeTerminalRow(existing: TerminalTab | undefined, built: TerminalTab): TerminalTab { +function mergeTerminalRow( + existing: TerminalTab | undefined, + built: TerminalTab, + newRowFields: (tabId: string) => Partial | null +): TerminalTab { if (!existing) { - return built + const fields = newRowFields(built.id) + return fields ? { ...built, ...fields } : built } if (same(withoutWindowFields(existing), withoutWindowFields(built))) { return existing @@ -121,7 +126,11 @@ function setKey( export function buildLayoutFrameStorePatch( state: LayoutFrameStoreState, built: LayoutFrameSlices, - keys: readonly string[] + keys: readonly string[], + /** A tab whose pane layout stays as the store has it: the window's own pane command holds it. */ + keepsTerminalLayout: (tabId: string) => boolean = () => false, + /** This window's own fields for a row the frame creates. */ + newRowFields: (tabId: string) => Partial | null = () => null ): Partial { const patch: Partial = {} const keyed = (slice: KeyedSlice, value: (key: string) => unknown): void => { @@ -135,7 +144,9 @@ export function buildLayoutFrameStorePatch( } keyed('tabsByWorktree', (key) => - mergeById(state.tabsByWorktree[key], built.tabsByWorktree[key], mergeTerminalRow) + mergeById(state.tabsByWorktree[key], built.tabsByWorktree[key], (existing, next) => + mergeTerminalRow(existing, next, newRowFields) + ) ) keyed('unifiedTabsByWorktree', (key) => mergeById(state.unifiedTabsByWorktree[key], built.unifiedTabsByWorktree[key]) @@ -181,12 +192,17 @@ export function buildLayoutFrameStorePatch( ...tabIdsOf(state.tabsByWorktree), ...tabIdsOf(built.tabsByWorktree) ])) { + if ( + keepsTerminalLayout(tabId) && + tabId in terminalLayouts && + tabId in built.terminalLayoutsByTabId + ) { + continue + } terminalLayouts = setKey(terminalLayouts, tabId, built.terminalLayoutsByTabId[tabId]) ?? terminalLayouts } - if (terminalLayouts !== state.terminalLayoutsByTabId) { - patch.terminalLayoutsByTabId = terminalLayouts - } + Object.assign(patch, terminalLayoutsPatch(state, terminalLayouts)) const keySet = new Set(keys) // An open file's editor state is this window's; a frame decides only which files are open. @@ -236,3 +252,11 @@ export function buildLayoutFrameStorePatch( } return patch } + +/** The patch that shows `layouts` as the store's pane trees; empty when they are the store's. */ +export function terminalLayoutsPatch( + state: Pick, + layouts: LayoutFrameStoreState['terminalLayoutsByTabId'] +): Partial { + return layouts === state.terminalLayoutsByTabId ? {} : { terminalLayoutsByTabId: layouts } +}