From a0026588e41ec323386ab94ce68ea46db35be171 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 7 Oct 2026 14:45:26 -0400 Subject: [PATCH] terminal: window says where each new terminal goes (no behavior change) (#26078) * refactor(terminal): renderer senders attach placement to pty spawn, report-only The window now says where each fresh terminal goes: the first pane of a tab sends new-tab with the tab's creation fields, a split sends the parent leaf, direction, ratio and the tree after the split, background launches send the same, and a Codex restart that rebuilds a rootless layout sends root. A reattach sends nothing. Main still only records whether placement names the tab its binding write picks; its 'absent' value now counts only new leaves without placement, so it reads the fallback mint and graft directly. Extends the shared placement type with optional row, ratio and proposedRoot, each dropped alone if malformed, so older and newer peers keep working. * fix(terminal): background agent placement row claims no launchAgent the tab lacks The adopted tab is created without launchAgent, so a row naming one would diverge from the renderer's tab once main applies placement. * fix(terminal): a before-split publishes its pane with the final tree order wrapInSplit now places the new pane first itself, so the pane-created handler's proposedRoot sees the real post-split tree instead of the order before the subtree split moved it. * test(terminal): type the background-terminal tab patch precisely --- .../pty-binding-placement-inertness.test.ts | 2 +- .../terminal-pane-placement-agreement.ts | 11 +- .../codex-detached-pane-restart.test.ts | 4 +- .../codex-detached-pane-restart.ts | 30 +++--- .../terminal-pane/ipc-pty-spawn-request.ts | 2 + .../pty-connection-fresh-spawn-guards.test.ts | 12 +++ .../terminal-pane/pty-connection-types.ts | 2 + .../pty-connection/pty-input-recovery.ts | 1 + .../pty-transport-connect-spawn.test.ts | 20 ++++ .../terminal-pane/pty-transport-types.ts | 3 + .../terminal-pane-pane-created.ts | 10 ++ .../terminal-pane-spawn-placement.test.ts | 101 ++++++++++++++++++ .../terminal-pane-spawn-placement.ts | 23 ++++ .../src/lib/background-pane-exit-output.ts | 46 ++++++++ .../launch-agent-background-session.test.ts | 3 +- .../lib/launch-agent-background-session.ts | 2 + ...unch-worktree-background-terminals.test.ts | 35 +++++- .../launch-worktree-background-terminals.ts | 82 ++++++-------- .../pane-manager-pane-creation.ts | 2 +- .../pane-manager-spawn-placement.test.ts | 95 ++++++++++++++++ .../lib/pane-manager/pane-manager-types.ts | 5 +- .../pane-manager/pane-overlay-focus.test.ts | 2 +- .../lib/pane-manager/pane-split-close.test.ts | 7 +- .../src/lib/pane-manager/pane-split-close.ts | 23 ++-- .../lib/pane-manager/pane-subtree-split.ts | 32 +----- .../src/lib/pane-manager/pane-tree-ops.ts | 12 ++- .../src/lib/terminal-pane-placement-row.ts | 29 +++++ src/shared/terminal-pane-placement.test.ts | 44 ++++++++ src/shared/terminal-pane-placement.ts | 30 +++++- src/shared/workspace-session-schema.ts | 4 +- 30 files changed, 549 insertions(+), 125 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.ts create mode 100644 src/renderer/src/lib/background-pane-exit-output.ts create mode 100644 src/renderer/src/lib/pane-manager/pane-manager-spawn-placement.test.ts create mode 100644 src/renderer/src/lib/terminal-pane-placement-row.ts diff --git a/src/main/persistence/loading-store/pty-binding-placement-inertness.test.ts b/src/main/persistence/loading-store/pty-binding-placement-inertness.test.ts index 2ad1c882f23..ce412cf4633 100644 --- a/src/main/persistence/loading-store/pty-binding-placement-inertness.test.ts +++ b/src/main/persistence/loading-store/pty-binding-placement-inertness.test.ts @@ -232,7 +232,7 @@ describe('placement on the binding write is inert', () => { it(`${scenario.name}: memory and saved state match a write without placement`, async () => { const baseline = await bindAndSave(scenario, undefined) expect(baseline.result).toBe(true) - expect(placementAttributes).toEqual(['absent']) + expect(placementAttributes).toEqual([scenario.prebind ? 'leaf_present' : 'absent']) const cases = scenario.placements('') for (const [index, [, agreement]] of cases.entries()) { const placed = await bindAndSave(scenario, (leaf) => scenario.placements(leaf)[index][0]) diff --git a/src/main/persistence/terminal-topology/terminal-pane-placement-agreement.ts b/src/main/persistence/terminal-topology/terminal-pane-placement-agreement.ts index 0a104592d94..3376859499e 100644 --- a/src/main/persistence/terminal-topology/terminal-pane-placement-agreement.ts +++ b/src/main/persistence/terminal-topology/terminal-pane-placement-agreement.ts @@ -2,7 +2,10 @@ import type { TerminalPanePlacement } from '../../../shared/terminal-pane-placem import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { layoutContainsLeafId } from '../restoring-sessions/terminal-layout-normalization' -/** Low-cardinality: it lands on the `persistence.pty-binding` span. */ +/** + * Low-cardinality: it lands on the `persistence.pty-binding` span. `absent` counts new leaves with + * no placement, which only the mint and graft can place; it should read zero before they go. + */ export type TerminalPanePlacementAgreement = | 'absent' | 'leaf_present' @@ -24,14 +27,14 @@ export function terminalPanePlacementAgreement( tabId: string, leafId: string ): TerminalPanePlacementAgreement { - if (!placement) { - return 'absent' - } const tabExists = session.tabsByWorktree?.[worktreeId]?.some((tab) => tab.id === tabId) === true const root = session.terminalLayoutsByTabId?.[tabId]?.root ?? null if (tabExists && layoutContainsLeafId(root, leafId)) { return 'leaf_present' } + if (!placement) { + return 'absent' + } switch (placement.kind) { case 'new-tab': return tabExists ? 'tab_exists' : 'agrees' diff --git a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts index 8aea1440364..1aa4adfcd9b 100644 --- a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts +++ b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.test.ts @@ -126,6 +126,8 @@ describe('codex detached pane restart executor', () => { initiallyHidden: true }) ) + // The leaf is in the root, so main already knows where it goes. + expect(vi.mocked(window.api.pty.spawn).mock.calls[0]?.[0]).not.toHaveProperty('placement') expect(window.api.pty.getSize).not.toHaveBeenCalled() expect(vi.mocked(window.api.pty.spawn).mock.calls[0]?.[0]?.env).toEqual( expect.objectContaining({ @@ -310,7 +312,7 @@ describe('codex detached pane restart executor', () => { }) ) expect(window.api.pty.spawn).toHaveBeenCalledExactlyOnceWith( - expect.objectContaining({ replacesPtyId: OLD_PTY }) + expect.objectContaining({ replacesPtyId: OLD_PTY, placement: { kind: 'root' } }) ) }) diff --git a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts index 6c3228ae10b..027e6ee156a 100644 --- a/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts +++ b/src/renderer/src/components/terminal-pane/codex-detached-pane-restart.ts @@ -10,7 +10,7 @@ */ import { isTerminalLeafId, makePaneKey } from '../../../../shared/stable-pane-id' import { parseWorkspaceKey } from '../../../../shared/workspace-scope' -import type { TerminalPaneLayoutNode, TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types' import type { AppState } from '@/store' import { useAppStore } from '@/store' import { getWorktreeMapFromState } from '@/store/selectors' @@ -25,6 +25,7 @@ import { hasCachedWindowsTerminalCapabilities } from '@/lib/windows-terminal-capabilities' import { ptyDataHandlers, unregisterPtyDataHandlers } from './pty-dispatcher' +import { collectLeafIds } from './terminal-pane-layout-tree' import { discardPreHandlerPtyState } from './pty-pre-handler-buffer' import { disposeParkedTerminalWatchersForPtyIds } from './terminal-parked-watcher-registry' @@ -224,6 +225,9 @@ async function executeDetachedCodexPaneRestart( tabId: tab.id, leafId, replacesPtyId: ptyId, + ...(restartReplacesLayoutRoot(currentState.terminalLayoutsByTabId[tab.id], leafId) + ? { placement: { kind: 'root' as const } } + : {}), ...(tab.shellOverride ? { shellOverride: tab.shellOverride } : {}), ...(projectRuntime ? { projectRuntime } : {}), initiallyHidden: true @@ -307,27 +311,23 @@ function reopenCurrentCodexRestartPrompt(located: LocatedCodexPane, replacedPtyI } } -function layoutRootContainsLeaf( - node: TerminalPaneLayoutNode | null | undefined, +// Why: mount replays panes from the root — a root that doesn't name this leaf +// mints a fresh one and silently orphans the replacement PTY. Rewriting is +// only safe when this is the tab's sole bound pane; a split keeps its root. +function restartReplacesLayoutRoot( + layout: TerminalLayoutSnapshot | undefined, leafId: string ): boolean { - if (!node) { - return false - } - if (node.type === 'leaf') { - return node.leafId === leafId - } - return layoutRootContainsLeaf(node.first, leafId) || layoutRootContainsLeaf(node.second, leafId) + return ( + !collectLeafIds(layout?.root).includes(leafId) && + Object.keys(layout?.ptyIdsByLeafId ?? {}).every((id) => id === leafId) + ) } function rebindCodexPaneLayoutLeaf(tabId: string, leafId: string, newPtyId: string): void { const store = useAppStore.getState() const layout = store.terminalLayoutsByTabId[tabId] - const boundLeafIds = Object.keys(layout?.ptyIdsByLeafId ?? {}) - // Why: mount replays panes from the root — a root that doesn't name this leaf - // mints a fresh one and silently orphans the replacement PTY. Rewriting is - // only safe when this is the tab's sole bound pane; a split keeps its root. - if (!layoutRootContainsLeaf(layout?.root, leafId) && boundLeafIds.every((id) => id === leafId)) { + if (restartReplacesLayoutRoot(layout, leafId)) { store.setTabLayout( tabId, singlePaneLayoutSnapshot(leafId, newPtyId, layout?.titlesByLeafId?.[leafId] ?? null) diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts index 667865deb7e..7961fed61ec 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts @@ -85,6 +85,7 @@ export async function spawnIpcPty( worktreeId, tabId, leafId, + placement, shellOverride, projectRuntime, terminalColorQueryReplies, @@ -131,6 +132,7 @@ export async function spawnIpcPty( worktreeId, ...(tabId ? { tabId } : {}), ...(leafId ? { leafId } : {}), + ...(placement && !admittedSessionId ? { placement } : {}), ...(replacesPtyId ? { replacesPtyId } : {}), ...(shellOverride ? { shellOverride } : {}), ...(projectRuntime ? { projectRuntime } : {}), diff --git a/src/renderer/src/components/terminal-pane/pty-connection-fresh-spawn-guards.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-fresh-spawn-guards.test.ts index 1f0935b8085..adf9f511a2c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-fresh-spawn-guards.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-fresh-spawn-guards.test.ts @@ -237,6 +237,18 @@ describe('connectPanePty', () => { expect(retain()).toBe(false) }) + it('carries the pane placement onto its transport', async () => { + const { connectPanePty } = await import('./pty-connection') + transportFactoryQueue.push(createMockTransport()) + const placement = { kind: 'new-tab' } as const + const deps = createDeps({ tabId: 'tab-placement', placement }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks() + + expect(createdTransportOptions[0]).toMatchObject({ placement }) + }) + it('fresh-spawns normally when the pane worktree is not being deleted', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index 6e8d340b3a3..b45b6478725 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -9,6 +9,7 @@ import type { EventProps } from '../../../../shared/telemetry-events' import type { TerminalColorSchemeMode } from '../../../../shared/terminal-color-scheme-protocol' import type { StartupCommandDelivery } from '../../../../shared/codex-startup-delivery' import type { TuiAgent } from '../../../../shared/tui-agent' +import type { TerminalPanePlacement } from '../../../../shared/terminal-pane-placement' import type { SetupSplitDirection } from '../../../../shared/worktree/launch-types' import type { AgentProviderSessionMetadata, @@ -60,6 +61,7 @@ export type PtyConnectionDeps = { cwd?: string /** Delays a fresh split's spawn without delaying its renderer pane. */ cwdPromise?: Promise + placement?: TerminalPanePlacement /** Input handed off from a predecessor mount of the same deferred split. */ preconnectInput?: readonly PtyPreconnectInputEntry[] /** Captures newly retained input for a remount-safe deferred split handoff. */ diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts b/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts index aa226a3a78e..d6f8a6de9c6 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts @@ -82,6 +82,7 @@ export function installPtyInputRecovery(session: ConnectPanePtySession): void { // and the main-side guard short-circuits. tabId: session.deps.tabId, leafId: session.pane.leafId, + ...(session.deps.placement ? { placement: session.deps.placement } : {}), activate: session.deps.isActiveRef.current && session.deps.isVisibleRef.current, ...(session.shellOverride ? { shellOverride: session.shellOverride } : {}), ...(session.projectRuntime ? { projectRuntime: session.projectRuntime } : {}), diff --git a/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts index 4d6b0aa911b..ca8d7a816b4 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts @@ -306,6 +306,26 @@ describe('createIpcPtyTransport', () => { transport.disconnect() }) + it('sends placement on a fresh spawn but not on a session reattach', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const spawn = window.api.pty.spawn as unknown as ReturnType + const placement = { + kind: 'split', + parentLeafId: '11111111-1111-4111-8111-111111111111', + direction: 'horizontal' + } as const + + const fresh = createIpcPtyTransport({ placement }) + await fresh.connect({ url: '', callbacks: {} }) + expect(spawn).toHaveBeenLastCalledWith(expect.objectContaining({ placement })) + fresh.disconnect() + + const reattach = createIpcPtyTransport({ placement }) + await reattach.connect({ url: '', callbacks: {}, sessionId: 'session-1' }) + expect(spawn).toHaveBeenLastCalledWith(expect.not.objectContaining({ placement })) + reattach.disconnect() + }) + it('returns startup cwd fallback metadata to the connection layer', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawn = window.api.pty.spawn as unknown as ReturnType diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 4b2de188fce..c45457dfbc1 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -13,6 +13,7 @@ import type { EventProps } from '../../../../shared/telemetry-events' import type { TerminalInputKind } from '../../../../shared/terminal-input-kind' import type { TerminalOscColorQueryReplyColors } from '../../../../shared/terminal-osc-color-reply' import type { TuiAgent } from '../../../../shared/tui-agent' +import type { TerminalPanePlacement } from '../../../../shared/terminal-pane-placement' import type { ExecutionHostId } from '../../../../shared/execution-host' import type { PtyDataMeta } from './pty-dispatcher' import type { RemoteRuntimeSnapshotOutcome } from '../../runtime/remote-runtime-terminal-multiplexer' @@ -280,6 +281,8 @@ export type IpcPtyTransportOptions = { worktreeId?: string tabId?: string leafId?: string + /** Sent on fresh spawns only; a reattach names a PTY whose leaf main already knows. */ + placement?: TerminalPanePlacement activate?: boolean shellOverride?: string projectRuntime?: ProjectExecutionRuntimeResolution diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-pane-created.ts b/src/renderer/src/components/terminal-pane/terminal-pane-pane-created.ts index f422f99fb53..4e0d7bd960c 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-pane-created.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-pane-created.ts @@ -21,6 +21,7 @@ import type { PtyPreconnectInputEntry } from './pty-preconnect-input-buffer' import { guardParserHandler } from './terminal-parser-handler-guard' import { isPaneReplaying } from './replay-guard' import { connectPanePty } from './pty-connection' +import { completePaneSpawnPlacement } from './terminal-pane-spawn-placement' import { createQueuedStartupConsumer, resolvePaneSeedCwd, @@ -202,6 +203,15 @@ export function createTerminalPaneCreatedHandler( : {}), ...(effectiveSpawnHints?.cwd ? { cwd: effectiveSpawnHints.cwd } : {}), ...(effectiveSpawnHints?.cwdPromise ? { cwdPromise: effectiveSpawnHints.cwdPromise } : {}), + ...(effectiveSpawnHints?.placement + ? { + placement: completePaneSpawnPlacement(effectiveSpawnHints.placement, { + worktreeId: deps.worktreeId, + tabId: deps.tabId, + container: deps.containerRef.current + }) + } + : {}), restoredPtyIdByLeafId: effectiveSpawnHints?.ptyId ? { ...ptyDeps.restoredPtyIdByLeafId, diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.test.ts new file mode 100644 index 00000000000..eec7df33470 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.test.ts @@ -0,0 +1,101 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { TerminalPanePlacement } from '../../../../shared/terminal-pane-placement' + +const TAB = { + id: 'tab-1', + ptyId: null, + worktreeId: 'wt-1', + title: 'Terminal 2', + defaultTitle: 'Terminal 2', + customTitle: 'Build', + color: '#f97316', + sortOrder: 1, + createdAt: 42, + shellOverride: 'wsl.exe', + isPinned: true +} + +vi.mock('@/store', () => ({ + useAppStore: { getState: () => ({ tabsByWorktree: { 'wt-1': [TAB] } }) } +})) +// Why: happy-dom has no canvas, so xterm cannot open; placement never touches the terminal. +vi.mock('@/lib/pane-manager/pane-lifecycle', async (importOriginal) => { + const actual: object = await importOriginal() + return { ...actual, openTerminal: vi.fn() } +}) + +const { PaneManager } = await import('@/lib/pane-manager/pane-manager') +const { completePaneSpawnPlacement } = await import('./terminal-pane-spawn-placement') + +const LEAF_A = '11111111-1111-4111-8111-111111111111' +const LEAF_B = '22222222-2222-4222-8222-222222222222' + +describe('completePaneSpawnPlacement', () => { + const root = document.createElement('div') + + afterEach(() => { + root.replaceChildren() + }) + + it('adds the tab creation fields to a new tab', () => { + const placement = completePaneSpawnPlacement( + { kind: 'new-tab' }, + { worktreeId: 'wt-1', tabId: 'tab-1', container: root } + ) + + expect(placement).toEqual({ + kind: 'new-tab', + row: { + title: 'Terminal 2', + defaultTitle: 'Terminal 2', + customTitle: 'Build', + color: '#f97316', + createdAt: 42, + shellOverride: 'wsl.exe' + } + }) + }) + + it('leaves a new tab bare when the store no longer has it', () => { + expect( + completePaneSpawnPlacement( + { kind: 'new-tab' }, + { worktreeId: 'wt-1', tabId: 'tab-gone', container: root } + ) + ).toEqual({ kind: 'new-tab' }) + }) + + it('adds the mounted tree after the split', () => { + let placement: TerminalPanePlacement | undefined + const manager = new PaneManager(root, { + linkOpenHint: () => '', + onPaneCreated: (_pane, hints) => { + placement = hints?.placement + ? completePaneSpawnPlacement(hints.placement, { + worktreeId: 'wt-1', + tabId: 'tab-1', + container: root + }) + : undefined + } + }) + const first = manager.createInitialPane({ leafId: LEAF_A }) + manager.splitPane(first.id, 'horizontal', { leafId: LEAF_B, ratio: 0.3 }) + + expect(placement).toEqual({ + kind: 'split', + parentLeafId: LEAF_A, + direction: 'horizontal', + ratio: 0.3, + proposedRoot: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: LEAF_A }, + second: { type: 'leaf', leafId: LEAF_B }, + ratio: 0.3 + } + }) + manager.destroy() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.ts b/src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.ts new file mode 100644 index 00000000000..920e2786546 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-spawn-placement.ts @@ -0,0 +1,23 @@ +import type { TerminalPanePlacement } from '../../../../shared/terminal-pane-placement' +import { useAppStore } from '@/store' +import { terminalPanePlacementRow } from '@/lib/terminal-pane-placement-row' +import { serializePaneTree } from './layout-serialization' + +/** Adds what only the mounted tab knows: the new tab's row, or the tree after the split. */ +export function completePaneSpawnPlacement( + placement: TerminalPanePlacement, + pane: { worktreeId: string; tabId: string; container: HTMLElement | null } +): TerminalPanePlacement { + if (placement.kind === 'new-tab') { + const tab = useAppStore + .getState() + .tabsByWorktree[pane.worktreeId]?.find((entry) => entry.id === pane.tabId) + return tab ? { ...placement, row: terminalPanePlacementRow(tab) } : placement + } + if (placement.kind === 'split') { + const top = pane.container?.firstElementChild + const proposedRoot = top instanceof HTMLElement ? serializePaneTree(top) : null + return proposedRoot ? { ...placement, proposedRoot } : placement + } + return placement +} diff --git a/src/renderer/src/lib/background-pane-exit-output.ts b/src/renderer/src/lib/background-pane-exit-output.ts new file mode 100644 index 00000000000..5d077313bf1 --- /dev/null +++ b/src/renderer/src/lib/background-pane-exit-output.ts @@ -0,0 +1,46 @@ +import { + registerEagerPtyBuffer, + type EagerPtyHandle +} from '@/components/terminal-pane/pty-dispatcher' +import { useAppStore } from '@/store' + +function persistExitedPaneOutput(tabId: string, leafId: string, output: string): void { + const store = useAppStore.getState() + const layout = store.terminalLayoutsByTabId[tabId] + if (!layout) { + return + } + const { ptyIdsByLeafId: existingPtyIds, buffersByLeafId: existingBuffers, ...rest } = layout + const nextPtyIds = { ...existingPtyIds } + delete nextPtyIds[leafId] + const trimmedOutput = output.trim() ? output : '' + store.setTabLayout(tabId, { + ...rest, + ...(Object.keys(nextPtyIds).length > 0 ? { ptyIdsByLeafId: nextPtyIds } : {}), + ...(trimmedOutput + ? { + buffersByLeafId: { + ...existingBuffers, + [leafId]: output + } + } + : existingBuffers + ? { buffersByLeafId: existingBuffers } + : {}) + }) +} + +// Why the incarnation: a relay-recycled id can hold the previous owner's exit, and draining that +// into this handler tears the pane down seconds after it launched. +export function registerBackgroundPaneBuffer( + tabId: string, + leafId: string, + pane: { ptyId: string; incarnationId?: string } +): void { + let eagerBuffer: EagerPtyHandle | null = null + const onExit = (exitPtyId: string): void => { + persistExitedPaneOutput(tabId, leafId, eagerBuffer?.flush() ?? '') + useAppStore.getState().clearTabPtyId(tabId, exitPtyId) + } + eagerBuffer = registerEagerPtyBuffer(pane.ptyId, onExit, pane.incarnationId) +} diff --git a/src/renderer/src/lib/launch-agent-background-session.test.ts b/src/renderer/src/lib/launch-agent-background-session.test.ts index 23606a83776..d665133b910 100644 --- a/src/renderer/src/lib/launch-agent-background-session.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session.test.ts @@ -129,7 +129,8 @@ describe('launchAgentBackgroundSession', () => { }), connectionId: null, worktreeId: 'wt-1', - tabId + tabId, + placement: { kind: 'new-tab', row: { customTitle: 'Nightly audit' } } }) ) const paneKey = expectStableAgentBackgroundPaneSpawn(mockSpawn) diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index 3e997d32e51..ed6c58d82bc 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -191,6 +191,8 @@ export async function launchAgentBackgroundSession( worktreeId, tabId: reservedTabId, leafId, + // Why no launchAgent: the adopted tab is created without one, and the row must match it. + placement: { kind: 'new-tab', ...(title ? { row: { customTitle: title } } : {}) }, telemetry: { agent_kind: tuiAgentToAgentKind(agent), launch_source: launchSource ?? 'unknown', diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.test.ts b/src/renderer/src/lib/launch-worktree-background-terminals.test.ts index ea463464751..88ab463e60d 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.test.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.test.ts @@ -87,6 +87,16 @@ describe('launchWorktreeBackgroundTerminals', () => { state.tabsByWorktree['wt-1'].push(tab) return tab }) + // Placement reads the row back from the store, so title and color writes must land there. + const patchTab = (tabId: string, patch: { customTitle?: string; color?: string }): void => { + state.tabsByWorktree['wt-1'] = state.tabsByWorktree['wt-1'].map((tab) => + tab.id === tabId ? { ...tab, ...patch } : tab + ) + } + mockSetTabCustomTitle.mockImplementation((tabId: string, customTitle: string) => + patchTab(tabId, { customTitle }) + ) + mockSetTabColor.mockImplementation((tabId: string, color: string) => patchTab(tabId, { color })) mockCloseTab.mockImplementation((tabId: string) => { state.tabsByWorktree['wt-1'] = state.tabsByWorktree['wt-1'].filter((tab) => tab.id !== tabId) }) @@ -136,7 +146,11 @@ describe('launchWorktreeBackgroundTerminals', () => { connectionId: null, worktreeId: 'wt-1', tabId: 'tab-1', - leafId: '00000000-0000-4000-8000-000000000001' + leafId: '00000000-0000-4000-8000-000000000001', + placement: { + kind: 'new-tab', + row: expect.objectContaining({ customTitle: 'Dev', color: '#f97316' }) + } }) ) expect(mockSpawn).toHaveBeenNthCalledWith( @@ -145,7 +159,8 @@ describe('launchWorktreeBackgroundTerminals', () => { command: 'bash /tmp/setup.sh', env: expect.objectContaining({ ORCA_WORKTREE_PATH: '/repo/worktree' }), tabId: 'tab-2', - leafId: '00000000-0000-4000-8000-000000000002' + leafId: '00000000-0000-4000-8000-000000000002', + placement: { kind: 'new-tab', row: expect.objectContaining({ customTitle: 'Setup' }) } }) ) expect(mockUpdateTabPtyId).toHaveBeenCalledWith('tab-1', 'pty-1') @@ -168,7 +183,8 @@ describe('launchWorktreeBackgroundTerminals', () => { 1, expect.objectContaining({ tabId: 'tab-1', - leafId: '00000000-0000-4000-8000-000000000001' + leafId: '00000000-0000-4000-8000-000000000001', + placement: { kind: 'new-tab', row: expect.any(Object) } }) ) expect(mockSpawn.mock.calls[0]?.[0]).not.toHaveProperty('command') @@ -177,7 +193,18 @@ describe('launchWorktreeBackgroundTerminals', () => { expect.objectContaining({ command: 'bash /tmp/setup.sh', tabId: 'tab-1', - leafId: '00000000-0000-4000-8000-000000000002' + leafId: '00000000-0000-4000-8000-000000000002', + placement: { + kind: 'split', + parentLeafId: '00000000-0000-4000-8000-000000000001', + direction: 'horizontal', + proposedRoot: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: '00000000-0000-4000-8000-000000000001' }, + second: { type: 'leaf', leafId: '00000000-0000-4000-8000-000000000002' } + } + } }) ) expect(mockSetTabLayout).toHaveBeenLastCalledWith( diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.ts b/src/renderer/src/lib/launch-worktree-background-terminals.ts index 791504d6948..b5b31b5f97c 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.ts @@ -1,12 +1,10 @@ -import { - registerEagerPtyBuffer, - type EagerPtyHandle -} from '@/components/terminal-pane/pty-dispatcher' import { createBrowserUuid } from '@/lib/browser-uuid' import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner' import { getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' import { retireUnownedTerminal } from '@/lib/retire-unowned-background-terminal' +import { registerBackgroundPaneBuffer } from '@/lib/background-pane-exit-output' +import { terminalPanePlacementRow } from '@/lib/terminal-pane-placement-row' import { useAppStore } from '@/store' import { translate } from '@/i18n/i18n' import { makePaneKey } from '../../../shared/stable-pane-id' @@ -14,7 +12,11 @@ import { buildSetupRunnerCommand, getSetupRunnerCommandPlatformForPath } from '../../../shared/setup-runner-command' -import type { TerminalLayoutSnapshot } from '../../../shared/terminal-tab-types' +import type { + TerminalLayoutSnapshot, + TerminalPaneLayoutNode +} from '../../../shared/terminal-tab-types' +import type { TerminalPanePlacement } from '../../../shared/terminal-pane-placement' import type { WorktreeDefaultTabsLaunch, WorktreeSetupLaunch @@ -62,6 +64,19 @@ function buildPaneEnv( } } +function buildSplitRoot( + firstLeafId: string, + secondLeafId: string, + direction: 'horizontal' | 'vertical' +): TerminalPaneLayoutNode { + return { + type: 'split', + direction, + first: { type: 'leaf', leafId: firstLeafId }, + second: { type: 'leaf', leafId: secondLeafId } + } +} + function buildSplitLayout( first: BackgroundPane, second: BackgroundPane, @@ -69,12 +84,7 @@ function buildSplitLayout( secondTitle: string ): TerminalLayoutSnapshot { return { - root: { - type: 'split', - direction, - first: { type: 'leaf', leafId: first.leafId }, - second: { type: 'leaf', leafId: second.leafId } - }, + root: buildSplitRoot(first.leafId, second.leafId, direction), activeLeafId: first.leafId, expandedLeafId: null, ptyIdsByLeafId: { @@ -87,43 +97,6 @@ function buildSplitLayout( } } -function persistExitedPaneOutput(tabId: string, leafId: string, output: string): void { - const store = useAppStore.getState() - const layout = store.terminalLayoutsByTabId[tabId] - if (!layout) { - return - } - const { ptyIdsByLeafId: existingPtyIds, buffersByLeafId: existingBuffers, ...rest } = layout - const nextPtyIds = { ...existingPtyIds } - delete nextPtyIds[leafId] - const trimmedOutput = output.trim() ? output : '' - store.setTabLayout(tabId, { - ...rest, - ...(Object.keys(nextPtyIds).length > 0 ? { ptyIdsByLeafId: nextPtyIds } : {}), - ...(trimmedOutput - ? { - buffersByLeafId: { - ...existingBuffers, - [leafId]: output - } - } - : existingBuffers - ? { buffersByLeafId: existingBuffers } - : {}) - }) -} - -// Why the incarnation: a relay-recycled id can hold the previous owner's exit, and draining that -// into this handler tears the pane down seconds after it launched. -function registerBackgroundPaneBuffer(tabId: string, leafId: string, pane: SpawnedPane): void { - let eagerBuffer: EagerPtyHandle | null = null - const onExit = (exitPtyId: string): void => { - persistExitedPaneOutput(tabId, leafId, eagerBuffer?.flush() ?? '') - useAppStore.getState().clearTabPtyId(tabId, exitPtyId) - } - eagerBuffer = registerEagerPtyBuffer(pane.ptyId, onExit, pane.incarnationId) -} - function buildSetupCommand(setup: WorktreeSetupLaunch): string { // Why: background setup tabs can launch later, so they must reuse the same shell chosen when the runner was written. return buildSetupRunnerCommand( @@ -141,6 +114,7 @@ async function spawnPane(args: { connectionId: string | null tabId: string leafId: string + placement: TerminalPanePlacement command?: string env?: Record }): Promise { @@ -153,7 +127,8 @@ async function spawnPane(args: { connectionId: args.connectionId, worktreeId: args.worktree.id, tabId: args.tabId, - leafId: args.leafId + leafId: args.leafId, + placement: args.placement }) return { ptyId: result.id, @@ -180,6 +155,8 @@ async function createBackgroundTab(args: { const leafId = createBrowserUuid() store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId)) + const created = + useAppStore.getState().tabsByWorktree[args.worktree.id]?.find(({ id }) => id === tab.id) ?? tab let pane: SpawnedPane try { pane = await spawnPane({ @@ -187,6 +164,7 @@ async function createBackgroundTab(args: { connectionId: args.connectionId, tabId: tab.id, leafId, + placement: { kind: 'new-tab', row: terminalPanePlacementRow(created) }, command: args.launch.command, env: args.launch.env }) @@ -223,6 +201,12 @@ async function addSetupSplit(args: { connectionId: args.connectionId, tabId: args.tab.tabId, leafId: setupLeafId, + placement: { + kind: 'split', + parentLeafId: args.tab.primary.leafId, + direction: args.direction, + proposedRoot: buildSplitRoot(args.tab.primary.leafId, setupLeafId, args.direction) + }, command: buildSetupCommand(args.setup), env: args.setup.envVars }) diff --git a/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts b/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts index 129685726da..b86a28fe054 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-pane-creation.ts @@ -30,7 +30,7 @@ export function createInitialManagedPane( focusPanePreservingOverlays(pane) } - host.publishPaneCreated(pane) + host.publishPaneCreated(pane, { placement: { kind: 'new-tab' } }) return toPublicPane(pane) } diff --git a/src/renderer/src/lib/pane-manager/pane-manager-spawn-placement.test.ts b/src/renderer/src/lib/pane-manager/pane-manager-spawn-placement.test.ts new file mode 100644 index 00000000000..348cbb1e5a9 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-manager-spawn-placement.test.ts @@ -0,0 +1,95 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import type { PaneManagerOptions } from './pane-manager-types' +import { serializePaneTree } from '@/components/terminal-pane/layout-serialization' + +// Why: happy-dom has no canvas, so xterm cannot open; placement 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 LEAF_A = '11111111-1111-4111-8111-111111111111' +const LEAF_B = '22222222-2222-4222-8222-222222222222' +const LEAF_C = '33333333-3333-4333-8333-333333333333' + +describe('PaneManager spawn placement hints', () => { + let root: HTMLDivElement + let onPaneCreated: Mock> + let manager: InstanceType + + beforeEach(() => { + vi.stubGlobal('requestAnimationFrame', () => 1) + vi.stubGlobal('cancelAnimationFrame', () => {}) + root = document.createElement('div') + document.body.appendChild(root) + onPaneCreated = vi.fn() + manager = new PaneManager(root, { linkOpenHint: () => '', onPaneCreated }) + }) + + afterEach(() => { + manager.destroy() + root.remove() + vi.unstubAllGlobals() + }) + + it('marks a tab first pane as a new tab', () => { + const pane = manager.createInitialPane({ leafId: LEAF_A }) + + expect(onPaneCreated).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ id: pane.id, leafId: LEAF_A }), + { placement: { kind: 'new-tab' } } + ) + }) + + it('names the split pane and direction alongside the other spawn hints', () => { + const first = manager.createInitialPane({ leafId: LEAF_A }) + onPaneCreated.mockClear() + + manager.splitPane(first.id, 'horizontal', { leafId: LEAF_B, cwd: '/repo/app' }) + + expect(onPaneCreated).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ leafId: LEAF_B }), + { + cwd: '/repo/app', + placement: { kind: 'split', parentLeafId: LEAF_A, direction: 'horizontal' } + } + ) + }) + + it('publishes a split placed before a mounted subtree with its final order', () => { + const first = manager.createInitialPane({ leafId: LEAF_A }) + manager.splitPane(first.id, 'vertical', { leafId: LEAF_B }) + const treesAtCreation: unknown[] = [] + onPaneCreated.mockReset().mockImplementation(() => { + const top = root.firstElementChild + treesAtCreation.push(top instanceof HTMLElement ? serializePaneTree(top) : null) + }) + + manager.splitPaneAroundLeafIds([LEAF_A, LEAF_B], first.id, 'horizontal', { + leafId: LEAF_C, + placement: 'before' + }) + + expect(onPaneCreated).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ leafId: LEAF_C }), + { placement: { kind: 'split', parentLeafId: LEAF_A, direction: 'horizontal' } } + ) + // The pane-created handler serializes proposedRoot from the DOM, so the order must be final then. + expect(treesAtCreation).toEqual([ + { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: LEAF_C }, + second: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_A }, + second: { type: 'leaf', leafId: LEAF_B } + } + } + ]) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index 80531b7fecf..30fe5010844 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -9,6 +9,7 @@ import type { WebglAddon } from '@xterm/addon-webgl' import type { SerializeAddon } from '@xterm/addon-serialize' import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { TerminalLeafId } from '../../../../shared/stable-pane-id' +import type { TerminalPanePlacement } from '../../../../shared/terminal-pane-placement' import type { PaneLayoutEditIntent } from '../../../../shared/rpc-contract/session-tabs-schemas-params' import type { TerminalWebglAutoDecision } from './terminal-webgl-auto-policy' @@ -26,9 +27,11 @@ export type PaneSpawnHints = { cwd?: string cwdPromise?: Promise ptyId?: string + /** Where the pane's leaf sits; main reads it only for a leaf it does not know yet. */ + placement?: TerminalPanePlacement } -export type PaneSplitOptions = PaneSpawnHints & { +export type PaneSplitOptions = Omit & { ratio?: number leafId?: string } diff --git a/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts b/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts index 93c4a67f97b..d335ce829a4 100644 --- a/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-overlay-focus.test.ts @@ -72,7 +72,7 @@ describe.each(['initial', 'active'] as const)('%s pane focus', (operation) => { function focus(f: ReturnType, requested = true) { if (operation === 'initial') { createInitialManagedPane(f.host, { focus: requested }) - expect(f.publishPaneCreated).toHaveBeenCalledWith(f.pane) + expect(f.publishPaneCreated).toHaveBeenCalledWith(f.pane, { placement: { kind: 'new-tab' } }) } else { f.root.append(f.container) f.manager.setActivePane(f.pane.id, { focus: requested }) diff --git a/src/renderer/src/lib/pane-manager/pane-split-close.test.ts b/src/renderer/src/lib/pane-manager/pane-split-close.test.ts index 098c5eb2c15..cd3b728cd24 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-close.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-close.test.ts @@ -156,7 +156,10 @@ describe('splitManagedPane', () => { expect(result?.id).toBe(newPane.id) expect(setActivePaneId).toHaveBeenCalledWith(newPane.id) expect(newPane.terminal.focus).toHaveBeenCalledOnce() - expect(publishPaneCreated).toHaveBeenCalledWith(newPane, { cwdPromise }) + expect(publishPaneCreated).toHaveBeenCalledWith(newPane, { + cwdPromise, + placement: { kind: 'split', parentLeafId: existingPane.leafId, direction: 'vertical' } + }) }) it('prepares every pane under a moved mounted subtree for split reparenting', () => { @@ -213,7 +216,7 @@ describe('splitManagedPane', () => { newPane.container, true, expect.anything(), - undefined + { ratio: undefined, newPaneFirst: undefined } ) expect(scheduleSplitScrollRestore).toHaveBeenCalledTimes(2) expect(scheduleSplitScrollRestore).toHaveBeenNthCalledWith( diff --git a/src/renderer/src/lib/pane-manager/pane-split-close.ts b/src/renderer/src/lib/pane-manager/pane-split-close.ts index 37ccf92b385..6cf53fe9a2d 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-close.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-close.ts @@ -34,6 +34,8 @@ type SplitManagedPaneArgs = { direction: 'vertical' | 'horizontal' opts?: PaneSplitOptions sourceContainer?: HTMLElement + /** Before publishing, so the pane-created tree already has the final order. */ + newPaneFirst?: boolean panes: Map root: HTMLElement styleOptions: PaneStyleOptions @@ -65,9 +67,12 @@ export function splitManagedPane(args: SplitManagedPaneArgs): ManagedPane | null const movedPaneStates = prepareMovedPanesForSplit(existingContainer, existing, args.panes) - wrapInSplit(existingContainer, newPane.container, isVertical, divider, args.opts) + wrapInSplit(existingContainer, newPane.container, isVertical, divider, { + ratio: args.opts?.ratio, + newPaneFirst: args.newPaneFirst + }) args.setActivePaneId(newPane.id) - openSplitPane(args, newPane, args.opts?.cwd) + openSplitPane(args, newPane, existing.leafId, args.opts?.cwd) for (const movedPaneState of movedPaneStates) { scheduleSplitScrollRestore( @@ -139,6 +144,7 @@ function findManagedPanesInContainer( function openSplitPane( args: SplitManagedPaneArgs, newPane: ManagedPaneInternal, + parentLeafId: string, cwd?: string ): void { openTerminal(newPane, { @@ -151,12 +157,17 @@ function openSplitPane( updateMultiPaneState(args.getDragCallbacks()) // Why: forward one-shot spawn/adoption hints so the new pane inherits the // source cwd for local splits or attaches a runtime-spawned PTY for web splits. - const spawnHints = { + args.publishPaneCreated(newPane, { ...(cwd ? { cwd } : {}), ...(args.opts?.cwdPromise ? { cwdPromise: args.opts.cwdPromise } : {}), - ...(args.opts?.ptyId ? { ptyId: args.opts.ptyId } : {}) - } - args.publishPaneCreated(newPane, Object.keys(spawnHints).length > 0 ? spawnHints : undefined) + ...(args.opts?.ptyId ? { ptyId: args.opts.ptyId } : {}), + placement: { + kind: 'split', + parentLeafId, + direction: args.direction, + ...(args.opts?.ratio !== undefined ? { ratio: args.opts.ratio } : {}) + } + }) args.managerOptions.onLayoutChanged?.() } 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 37e75cfd3e6..e6be21fae50 100644 --- a/src/renderer/src/lib/pane-manager/pane-subtree-split.ts +++ b/src/renderer/src/lib/pane-manager/pane-subtree-split.ts @@ -47,11 +47,12 @@ export function splitPaneAroundMountedSubtree( if (!sourceContainer) { return null } - const createdPane = splitManagedPane({ + return splitManagedPane({ paneId: args.fallbackPaneId, direction: args.direction, opts: args.opts, sourceContainer, + newPaneFirst: args.opts?.placement === 'before', panes: args.panes, root: args.root, styleOptions: args.styleOptions, @@ -63,15 +64,6 @@ export function splitPaneAroundMountedSubtree( setActivePaneId: args.setActivePaneId, isDestroyed: args.isDestroyed }) - if (!createdPane || args.opts?.placement !== 'before') { - return createdPane - } - - const createdInternal = args.panes.get(createdPane.id) - if (createdInternal) { - placeCreatedPaneBeforeSource(sourceContainer, createdInternal.container) - } - return createdPane } function findMountedSubtreeContainer( @@ -125,23 +117,3 @@ function setsEqual(left: ReadonlySet, right: ReadonlySet): boole } return true } - -function placeCreatedPaneBeforeSource( - sourceContainer: HTMLElement, - createdContainer: HTMLElement -): boolean { - const split = createdContainer.parentElement - if (!split || sourceContainer.parentElement !== split) { - return false - } - const divider = Array.from(split.children).find( - (child): child is HTMLElement => - child instanceof HTMLElement && child.classList.contains('pane-divider') - ) - if (!divider) { - return false - } - - split.replaceChildren(createdContainer, divider, sourceContainer) - return true -} diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts index 778adfb6c15..6b435177d2b 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts @@ -245,14 +245,14 @@ export function removeDividers(parent: HTMLElement): void { /** * Create a flex split wrapper that replaces `existingContainer` in the DOM, - * then places [existing] [divider] [new] inside it. + * then places [existing] [divider] [new] inside it ([new] first when `newPaneFirst`). */ export function wrapInSplit( existingContainer: HTMLElement, newContainer: HTMLElement, isVertical: boolean, divider: HTMLElement, - opts?: { ratio?: number } + opts?: { ratio?: number; newPaneFirst?: boolean } ): void { const parent = existingContainer.parentElement if (!parent) { @@ -292,7 +292,9 @@ export function wrapInSplit( // Replace existing with split in the DOM, then build children parent.replaceChild(split, existingContainer) - split.appendChild(existingContainer) - split.appendChild(divider) - split.appendChild(newContainer) + if (opts?.newPaneFirst) { + split.append(newContainer, divider, existingContainer) + } else { + split.append(existingContainer, divider, newContainer) + } } diff --git a/src/renderer/src/lib/terminal-pane-placement-row.ts b/src/renderer/src/lib/terminal-pane-placement-row.ts new file mode 100644 index 00000000000..53ae435a200 --- /dev/null +++ b/src/renderer/src/lib/terminal-pane-placement-row.ts @@ -0,0 +1,29 @@ +import type { TerminalPaneNewTabRow } from '../../../shared/terminal-pane-placement' +import type { TerminalTab } from '../../../shared/terminal-tab-types' + +export function terminalPanePlacementRow(tab: TerminalTab): TerminalPaneNewTabRow { + const { + title, + defaultTitle, + customTitle, + color, + createdAt, + startupCwd, + shellOverride, + quickCommandLabel, + launchAgent, + viewMode + } = tab + return { + title, + defaultTitle, + customTitle, + color, + createdAt, + startupCwd, + shellOverride, + quickCommandLabel, + launchAgent, + viewMode + } +} diff --git a/src/shared/terminal-pane-placement.test.ts b/src/shared/terminal-pane-placement.test.ts index 71e1c027ea4..1ff106cf846 100644 --- a/src/shared/terminal-pane-placement.test.ts +++ b/src/shared/terminal-pane-placement.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { parseTerminalPanePlacement } from './terminal-pane-placement' const LEAF = '11111111-1111-4111-8111-111111111111' +const OTHER_LEAF = '22222222-2222-4222-8222-222222222222' describe('parseTerminalPanePlacement', () => { it('accepts each kind', () => { @@ -11,6 +12,49 @@ describe('parseTerminalPanePlacement', () => { expect(parseTerminalPanePlacement({ kind: 'root' })).toEqual({ kind: 'root' }) }) + it('keeps a new tab row and a split ratio and proposed tree', () => { + const newTab = { + kind: 'new-tab', + row: { + title: 'Terminal 2', + customTitle: null, + color: '#f97316', + createdAt: 1, + viewMode: 'chat' + } + } + const split = { + kind: 'split', + parentLeafId: LEAF, + direction: 'horizontal', + ratio: 0.3, + proposedRoot: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: OTHER_LEAF }, + second: { type: 'leaf', leafId: LEAF }, + ratio: 0.3 + } + } + expect(parseTerminalPanePlacement(newTab)).toEqual(newTab) + expect(parseTerminalPanePlacement(split)).toEqual(split) + }) + + it('drops only a malformed optional field', () => { + expect(parseTerminalPanePlacement({ kind: 'new-tab', row: { createdAt: 'now' } })).toEqual({ + kind: 'new-tab' + }) + expect( + parseTerminalPanePlacement({ + kind: 'split', + parentLeafId: LEAF, + direction: 'vertical', + ratio: 'half', + proposedRoot: { type: 'tree' } + }) + ).toEqual({ kind: 'split', parentLeafId: LEAF, direction: 'vertical' }) + }) + it('strips fields a newer sender adds', () => { expect(parseTerminalPanePlacement({ kind: 'root', future: 1 })).toEqual({ kind: 'root' }) expect(parseTerminalPanePlacement({ kind: 'new-tab', size: { cols: 120, rows: 40 } })).toEqual({ diff --git a/src/shared/terminal-pane-placement.ts b/src/shared/terminal-pane-placement.ts index 361922446ff..9627f6d9edc 100644 --- a/src/shared/terminal-pane-placement.ts +++ b/src/shared/terminal-pane-placement.ts @@ -1,7 +1,28 @@ import { z } from 'zod' import { isTerminalLeafId } from './stable-pane-id' +import { terminalPaneLayoutNodeSchema, terminalTabSchema } from './workspace-session-schema' -const NewTabPlacement = z.object({ kind: z.literal('new-tab') }) +// The creation fields of the tab the renderer made, so main need not wait for its save. +const NewTabRow = terminalTabSchema + .pick({ + title: true, + defaultTitle: true, + customTitle: true, + color: true, + createdAt: true, + startupCwd: true, + quickCommandLabel: true, + launchAgent: true, + viewMode: true + }) + .extend({ shellOverride: z.string().optional() }) + .partial() + +// Optional fields degrade alone: a malformed one is dropped, not the whole placement. +const NewTabPlacement = z.object({ + kind: z.literal('new-tab'), + row: NewTabRow.optional().catch(undefined) +}) const SplitPlacement = z.object({ kind: z.literal('split'), @@ -9,7 +30,10 @@ const SplitPlacement = z.object({ .string() .max(128) .refine((value): boolean => isTerminalLeafId(value)), - direction: z.enum(['horizontal', 'vertical']) + direction: z.enum(['horizontal', 'vertical']), + ratio: z.number().optional().catch(undefined), + /** The tab's tree after the split, which `parentLeafId` alone can't express (subtrees, before/after). */ + proposedRoot: terminalPaneLayoutNodeSchema.optional().catch(undefined) }) // An existing tab whose layout is still empty. @@ -21,6 +45,8 @@ const TerminalPanePlacementSchema = z.discriminatedUnion('kind', [ RootPlacement ]) +export type TerminalPaneNewTabRow = z.infer + /** Which tab and leaf a new PTY joins; a binding reads it only for a leaf it does not know yet. */ export type TerminalPanePlacement = z.infer diff --git a/src/shared/workspace-session-schema.ts b/src/shared/workspace-session-schema.ts index 5274cb8be3e..bf67f580a24 100644 --- a/src/shared/workspace-session-schema.ts +++ b/src/shared/workspace-session-schema.ts @@ -51,7 +51,7 @@ const workspaceKeySchema = z.custom( // Why: z.lazy + type annotation keeps the recursive inference working without // forcing zod to resolve the whole tree at definition time. Discriminated on `type` because a // plain union re-tries the leaf branch for every split node of every restored terminal layout. -const terminalPaneLayoutNodeSchema: z.ZodType = z.lazy(() => +export const terminalPaneLayoutNodeSchema: z.ZodType = z.lazy(() => z.discriminatedUnion('type', [ z.object({ type: z.literal('leaf'), @@ -82,7 +82,7 @@ const terminalLayoutSnapshotSchema = z.object({ // ─── Terminal tab (legacy) ────────────────────────────────────────── -const terminalTabSchema = z.object({ +export const terminalTabSchema = z.object({ id: terminalTabIdSchema, ptyId: z.string().nullable(), worktreeId: z.string(),