diff --git a/src/main/agent-hooks/server-pane-authority.test.ts b/src/main/agent-hooks/server-pane-authority.test.ts index 7d622b5a8c9..57c3f8f8718 100644 --- a/src/main/agent-hooks/server-pane-authority.test.ts +++ b/src/main/agent-hooks/server-pane-authority.test.ts @@ -53,6 +53,23 @@ describe('AgentHookServer pane authority', () => { ]) }) + // A pane move is announced by main and again by the renderer; the repeat must change nothing. + it('ignores a transfer that is already in place', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setPaneKeyAliasPersistenceListener(listener) + + server.transferPaneAuthority(SOURCE, TARGET, 'pty-1', 10, { authorityVerified: true }) + server.transferPaneAuthority(SOURCE, TARGET, 'pty-1', 20) + server.transferPaneAuthority(SOURCE, TARGET, undefined, 30) + + expect(listener).toHaveBeenCalledOnce() + server.transferPaneAuthority(SOURCE, TARGET, 'pty-2', 40) + expect(listener).toHaveBeenLastCalledWith([ + { legacyPaneKey: SOURCE, stablePaneKey: TARGET, ptyId: 'pty-2', updatedAt: 40 } + ]) + }) + it('persists one physical alias while chained transfers advance its owner', () => { const server = new AgentHookServer() const listener = vi.fn() diff --git a/src/main/agent-hooks/server/server-authority-aliases.ts b/src/main/agent-hooks/server/server-authority-aliases.ts index fb4890e10a2..d5d3b0666d1 100644 --- a/src/main/agent-hooks/server/server-authority-aliases.ts +++ b/src/main/agent-hooks/server/server-authority-aliases.ts @@ -144,6 +144,14 @@ export abstract class AgentHookServerAuthorityAliases extends AgentHookServerAut } const previousOwnerPaneKey = this.resolvePaneKeyAlias(fromPaneKey) const physicalPaneKey = this.getPhysicalPaneKeyForAuthority(fromPaneKey, ptyId) + const requestedPtyId = ptyId?.trim() + // Why: a repeat would clear the owner's own polls; a pane move announces from main and renderer. + if ( + previousOwnerPaneKey === toPaneKey && + (!requestedPtyId || this.legacyPaneKeyAliases.get(physicalPaneKey)?.ptyId === requestedPtyId) + ) { + return + } for (const key of [fromPaneKey, previousOwnerPaneKey, physicalPaneKey, toPaneKey]) { this.takeRetiredPaneRestartId(key) } diff --git a/src/main/ipc/pty/ipc/leaf-move.test.ts b/src/main/ipc/pty/ipc/leaf-move.test.ts new file mode 100644 index 00000000000..724a706a56b --- /dev/null +++ b/src/main/ipc/pty/ipc/leaf-move.test.ts @@ -0,0 +1,70 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { agentHookServer } from '../../../agent-hooks/server' +import type { Store } from '../../../persistence' +import type { OrcaRuntimeService } from '../../../runtime/orca-runtime' +import type { TerminalLeafMoveResult } from '../../../../shared/terminal-leaf-move' +import { commitLeafMoveAndRekey } from './leaf-move' + +const LEAF = '22222222-2222-4222-8222-222222222222' +const request = { + worktreeId: 'repo-1::/tmp/wt', + sourceTabId: 'tab-source', + targetTabId: 'tab-target', + leafId: LEAF, + ptyId: 'pty-agent' +} + +function deps(result: TerminalLeafMoveResult) { + const rekeyWorkerTerminalResourcePaneKey = vi.fn(() => 1) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the move reads only this Store method. + const store = { moveTerminalLeafToNewTab: vi.fn(async () => result) } as unknown as Store + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the move reads only the orchestration DB accessor. + const runtime = { + getExistingOrchestrationDb: () => ({ rekeyWorkerTerminalResourcePaneKey }) + } as unknown as OrcaRuntimeService + return { store, runtime, rekeyWorkerTerminalResourcePaneKey } +} + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('pty:moveLeafToNewTab', () => { + it('aliases agent status and re-keys worker resources after a committed move', async () => { + const transfer = vi.spyOn(agentHookServer, 'transferPaneAuthority').mockImplementation(() => {}) + const { store, runtime, rekeyWorkerTerminalResourcePaneKey } = deps({ + status: 'moved', + ptyId: 'pty-agent' + }) + + await expect(commitLeafMoveAndRekey({ store, runtime }, request)).resolves.toEqual({ + status: 'moved', + ptyId: 'pty-agent' + }) + + expect(transfer).toHaveBeenCalledWith( + `tab-source:${LEAF}`, + `tab-target:${LEAF}`, + 'pty-agent', + expect.any(Number), + { authorityVerified: true } + ) + expect(rekeyWorkerTerminalResourcePaneKey).toHaveBeenCalledWith({ + fromPaneKey: `tab-source:${LEAF}`, + toPaneKey: `tab-target:${LEAF}` + }) + }) + + it.each([ + { status: 'not_held' }, + { status: 'refused', reason: 'pty_mismatch' } + ])('leaves every pane key alone when the move did not commit (%o)', async (result) => { + const transfer = vi.spyOn(agentHookServer, 'transferPaneAuthority').mockImplementation(() => {}) + const { store, runtime, rekeyWorkerTerminalResourcePaneKey } = deps(result) + + await expect(commitLeafMoveAndRekey({ store, runtime }, request)).resolves.toEqual(result) + + expect(transfer).not.toHaveBeenCalled() + expect(rekeyWorkerTerminalResourcePaneKey).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/pty/ipc/leaf-move.ts b/src/main/ipc/pty/ipc/leaf-move.ts new file mode 100644 index 00000000000..c8b0cb9f07a --- /dev/null +++ b/src/main/ipc/pty/ipc/leaf-move.ts @@ -0,0 +1,58 @@ +import { getPtyIpc } from '../../pty-host-bindings' +import type { Store } from '../../../persistence' +import type { OrcaRuntimeService } from '../../../runtime/orca-runtime' +import { agentHookServer } from '../../../agent-hooks/server' +import { + isTerminalLeafMoveRequest, + terminalLeafMovePaneKeys, + type TerminalLeafMoveRequest, + type TerminalLeafMoveResult +} from '../../../../shared/terminal-leaf-move' + +/** Durable move first; agent-status and orchestration keys follow only a committed move. */ +export async function commitLeafMoveAndRekey( + deps: { store?: Store; runtime?: OrcaRuntimeService }, + request: TerminalLeafMoveRequest +): Promise { + if (!deps.store) { + return { status: 'not_held' } + } + const result = await deps.store.moveTerminalLeafToNewTab(request) + if (result.status !== 'moved') { + return result + } + const { from: fromPaneKey, to: toPaneKey } = terminalLeafMovePaneKeys(request) + try { + // The process keeps the pane key baked into its env, so status must alias old to new. + agentHookServer.transferPaneAuthority( + fromPaneKey, + toPaneKey, + result.ptyId ?? undefined, + Date.now(), + { authorityVerified: true } + ) + } catch (error) { + console.warn('[pty] moved pane kept its old agent-status key:', error) + } + try { + deps.runtime?.getExistingOrchestrationDb()?.rekeyWorkerTerminalResourcePaneKey({ + fromPaneKey, + toPaneKey + }) + } catch (error) { + console.warn('[pty] moved pane kept its old orchestration resource key:', error) + } + return result +} + +export function installPtyLeafMoveIpcHandler(deps: { + store?: Store + runtime?: OrcaRuntimeService +}): void { + getPtyIpc().handle('pty:moveLeafToNewTab', async (_event, args: unknown) => { + if (!isTerminalLeafMoveRequest(args)) { + return { status: 'refused', reason: 'invalid_request' } satisfies TerminalLeafMoveResult + } + return commitLeafMoveAndRekey(deps, args) + }) +} diff --git a/src/main/ipc/pty/register-handlers.ts b/src/main/ipc/pty/register-handlers.ts index 9ce1d4082a4..7f29e895e0b 100644 --- a/src/main/ipc/pty/register-handlers.ts +++ b/src/main/ipc/pty/register-handlers.ts @@ -20,6 +20,7 @@ import { } from './ipc/renderer-kill' import { installPtyWriteIpcHandlers } from './ipc/write' import { installPtySpawnIpcHandler } from './ipc/spawn' +import { installPtyLeafMoveIpcHandler } from './ipc/leaf-move' import { installPtyRuntimeController } from './runtime/controller' import { installPtySnapshotIpcHandlers } from './ipc/snapshot' import { @@ -112,6 +113,7 @@ export function registerPtyHandlers( // Remove prior handlers so re-registration (e.g. macOS re-activate creating a new window) doesn't double-register. ipcMain.removeHandler('pty:spawn') + ipcMain.removeHandler('pty:moveLeafToNewTab') ipcMain.removeHandler('pty:kill') ipcMain.removeHandler('pty:listSessions') ipcMain.removeHandler('pty:hasPty') @@ -255,6 +257,7 @@ export function registerPtyHandlers( rememberSyntheticKillExit: session.rememberSyntheticKillExit, sendPtyExitToRenderer: session.sendPtyExitToRenderer } + installPtyLeafMoveIpcHandler({ store, runtime }) installPtySpawnIpcHandler({ runtime, store, diff --git a/src/main/persistence/loading-store/pty-binding-persistence.ts b/src/main/persistence/loading-store/pty-binding-persistence.ts index 5ad0acab94a..9e6041adb04 100644 --- a/src/main/persistence/loading-store/pty-binding-persistence.ts +++ b/src/main/persistence/loading-store/pty-binding-persistence.ts @@ -16,6 +16,11 @@ import { evaluatePtyBindingFastLane } from './pty-binding-fast-lane' import { ptyBindingIsRefused } from './pty-binding-refusals' import { startPtyBindingSpan, type PtyBindingOrigin, type PtyBindingSpan } from './pty-binding-span' import { applyPtyBinding } from './pty-binding-session-update' +import type { + TerminalLeafMoveRequest, + TerminalLeafMoveResult +} from '../../../shared/terminal-leaf-move' +import { moveLeaf } from '../terminal-topology/terminal-topology-commit' import { findTerminalBindingConflict } from '../terminal-topology/terminal-owner-invariants' type PtyBindingPersistenceOperationsRuntime = Pick< @@ -188,6 +193,22 @@ export class PtyBindingPersistenceOperations { throw error } } + + /** + * Detach-to-new-tab, committed before the renderer mounts the target tab (STA-9259). It lives on + * the binding domain only for its runtime and partition access; the commit module owns the write. + */ + moveTerminalLeafToNewTab(request: TerminalLeafMoveRequest): Promise { + const { runtime, sessions } = this[ptyBindingPersistenceOperationsContext] + return runtime.runDurableMutation( + moveLeaf(request, { + state: runtime.state, + hostIds: () => sessions.getWorkspaceSessionHostIds(), + getSession: (hostId) => sessions.getWorkspaceSession(hostId), + markDirty: (domain) => runtime.dirtyProfileStateDomains?.add(domain) + }) + ) + } } function writePtyBinding( diff --git a/src/main/persistence/terminal-topology/terminal-leaf-move-concurrent-close.test.ts b/src/main/persistence/terminal-topology/terminal-leaf-move-concurrent-close.test.ts new file mode 100644 index 00000000000..d5d0a89c4bc --- /dev/null +++ b/src/main/persistence/terminal-topology/terminal-leaf-move-concurrent-close.test.ts @@ -0,0 +1,120 @@ +import { tmpdir } from 'node:os' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { TerminalSurfaceCloseTarget } from '../../../shared/terminal-surface-close-target' +import { makeRepo } from '../../persistence-test-harness' +import { + closeMoveTestStores, + MOVED, + moveRequest, + newDataFile, + openStore, + seedSplitSource, + SOURCE, + TARGET, + tabsHoldingLeaf, + WT +} from './terminal-leaf-move-fixture' +import { closeLeafOrTab } from './terminal-topology-commit' + +vi.mock('electron', () => ({ + app: { + getPath: () => tmpdir(), + getName: () => 'orca-test', + getVersion: () => '0.0.0-test', + isPackaged: false, + on: () => {}, + whenReady: () => Promise.resolve() + }, + safeStorage: { + isEncryptionAvailable: () => false, + encryptString: (value: string) => Buffer.from(value), + decryptString: (value: Buffer) => value.toString() + }, + ipcMain: { on: () => {}, handle: () => {} }, + BrowserWindow: { getAllWindows: () => [] } +})) + +afterEach(closeMoveTestStores) + +const request = { ...moveRequest, ptyId: 'pty-agent' } + +/** What `session:close-terminal-surface` commits for a close the renderer already performed. */ +async function rendererClose( + store: ReturnType, + target: TerminalSurfaceCloseTarget, + reason: 'user' | 'cleanup' +): Promise { + await store.runDurableMutation( + closeLeafOrTab({ + worktreeId: WT, + target, + options: { allowMissing: true, force: true, closedByLayoutOwner: true, reason }, + requestedSession: store.getWorkspaceSession(), + ownerMatches: () => true, + hostId: () => 'local', + getSession: (hostId) => store.getWorkspaceSession(hostId), + setSession: (session, hostId) => store.setWorkspaceSession(session, hostId), + onClosed: () => {} + }) + ) +} + +async function openMovedStore() { + const dataFile = newDataFile() + const store = openStore(dataFile) + store.addRepo(makeRepo({ id: 'repo-1', path: '/tmp/move-worktree' })) + await seedSplitSource(store) + await expect(store.moveTerminalLeafToNewTab(request)).resolves.toMatchObject({ + status: 'moved' + }) + return { dataFile, store } +} + +function restartedTabs(store: ReturnType, dataFile: string) { + store.flush() + const session = openStore(dataFile).getWorkspaceSession() + return { tabIds: (session.tabsByWorktree[WT] ?? []).map((tab) => tab.id), session } +} + +// The renderer rolls a committed move forward: when the pane is gone it closes main's new tab. +describe('a close that lands while main commits the move', () => { + it('leaves no dead pane after restart when the user closed the moved pane', async () => { + const { dataFile, store } = await openMovedStore() + // The pane close reaches main after the move, so main no longer has SOURCE:leaf. + await rendererClose(store, { kind: 'pane', tabId: SOURCE, leafId: MOVED }, 'user') + expect(tabsHoldingLeaf(store.getWorkspaceSession(), MOVED)).toEqual([TARGET]) + + await rendererClose(store, { kind: 'tab', tabId: TARGET }, 'cleanup') + + const { tabIds, session } = restartedTabs(store, dataFile) + expect(tabsHoldingLeaf(session, MOVED)).toEqual([]) + expect(tabIds).toEqual([SOURCE]) + }) + + it('leaves no ghost tab after restart when the user closed the source tab', async () => { + const { dataFile, store } = await openMovedStore() + await rendererClose(store, { kind: 'tab', tabId: SOURCE }, 'user') + + await rendererClose(store, { kind: 'tab', tabId: TARGET }, 'cleanup') + + const { tabIds, session } = restartedTabs(store, dataFile) + expect(tabIds).toEqual([]) + expect(tabsHoldingLeaf(session, MOVED)).toEqual([]) + }) +}) + +// Why the renderer refuses to drag a pane whose spawn is in flight: a move with no PTY id lets the +// late spawn result bind SOURCE:leaf, and that bind grafts the leaf back into the source tab. +it('grafts a late spawn result for the moved leaf back into its source tab', async () => { + const store = openStore(newDataFile()) + await seedSplitSource(store) + await store.moveTerminalLeafToNewTab({ ...request, ptyId: null }) + await store.persistPtyBinding({ + worktreeId: WT, + tabId: SOURCE, + leafId: MOVED, + ptyId: 'pty-late', + incarnationId: 'inc-late' + }) + expect(tabsHoldingLeaf(store.getWorkspaceSession(), MOVED).sort()).toEqual([SOURCE, TARGET]) +}) diff --git a/src/main/persistence/terminal-topology/terminal-leaf-move-fixture.ts b/src/main/persistence/terminal-topology/terminal-leaf-move-fixture.ts new file mode 100644 index 00000000000..413d6cecc41 --- /dev/null +++ b/src/main/persistence/terminal-topology/terminal-leaf-move-fixture.ts @@ -0,0 +1,86 @@ +import { mkdtempSync, realpathSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { vi } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { closeTestStores, createSqliteTestStore } from '../../persistence-test-harness' +import { Store } from '../loading-store/store' +import { collectLayoutLeafIdsInOrder } from '../restoring-sessions/terminal-layout-normalization' + +/** Shared by the move tests; each test file mocks `electron` before importing this. */ + +export const WT = 'repo-1::/tmp/move-worktree' +export const SOURCE = 'tab-source' +export const TARGET = 'tab-target' +export const LEFT = '11111111-1111-4111-8111-111111111111' +export const MOVED = '22222222-2222-4222-8222-222222222222' +export const FROM = `${SOURCE}:${MOVED}` +export const TO = `${TARGET}:${MOVED}` + +const stores: Store[] = [] + +export async function closeMoveTestStores(): Promise { + for (const store of stores.splice(0)) { + store.freezeWrites() + } + await closeTestStores() + vi.restoreAllMocks() +} + +export function newDataFile(): string { + return join(realpathSync(mkdtempSync(join(tmpdir(), 'orca-leaf-move-'))), 'state.json') +} + +export function openStore(dataFile: string): Store { + const store = createSqliteTestStore(Store, { dataFile }) + stores.push(store) + return store +} + +export function sleeping(paneKey: string, tabId: string): SleepingAgentSessionRecord { + return { + paneKey, + tabId, + worktreeId: WT, + agent: 'codex', + providerSession: { key: 'session_id', id: 'codex-1' }, + prompt: 'go', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } +} + +/** A split source tab whose second leaf runs the agent that gets moved. */ +export async function seedSplitSource(store: Store, hostId?: string): Promise { + await store.persistPtyBinding( + { worktreeId: WT, tabId: SOURCE, leafId: LEFT, ptyId: 'pty-left', incarnationId: 'inc-left' }, + hostId + ) + await store.persistPtyBinding( + { worktreeId: WT, tabId: SOURCE, leafId: MOVED, ptyId: 'pty-agent', incarnationId: 'inc-1' }, + hostId + ) +} + +export function ownersOf(session: WorkspaceSessionState, ptyId: string): string[] { + return Object.entries(session.terminalLayoutsByTabId).flatMap(([tabId, layout]) => + collectLayoutLeafIdsInOrder(layout.root) + .filter((leafId) => layout.ptyIdsByLeafId?.[leafId] === ptyId) + .map((leafId) => `${tabId}:${leafId}`) + ) +} + +export function tabsHoldingLeaf(session: WorkspaceSessionState, leafId: string): string[] { + return Object.entries(session.terminalLayoutsByTabId) + .filter(([, layout]) => collectLayoutLeafIdsInOrder(layout.root).includes(leafId)) + .map(([tabId]) => tabId) +} + +export const moveRequest = { + worktreeId: WT, + sourceTabId: SOURCE, + targetTabId: TARGET, + leafId: MOVED +} diff --git a/src/main/persistence/terminal-topology/terminal-leaf-move.test.ts b/src/main/persistence/terminal-topology/terminal-leaf-move.test.ts new file mode 100644 index 00000000000..14ff2d7b764 --- /dev/null +++ b/src/main/persistence/terminal-topology/terminal-leaf-move.test.ts @@ -0,0 +1,363 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { tmpdir } from 'node:os' +import { toSshExecutionHostId } from '../../../shared/execution-host' +import { _resetTracerForTests, setActiveSink } from '../../observability/tracer' +import { makeRepo, makeTerminalTab } from '../../persistence-test-harness' +import { planTerminalLeafMove } from './terminal-leaf-move' +import { + closeMoveTestStores, + FROM, + LEFT, + MOVED, + moveRequest, + newDataFile, + openStore, + ownersOf, + seedSplitSource, + sleeping, + SOURCE, + TARGET, + tabsHoldingLeaf, + TO, + WT +} from './terminal-leaf-move-fixture' + +vi.mock('electron', () => ({ + app: { + getPath: () => tmpdir(), + getName: () => 'orca-test', + getVersion: () => '0.0.0-test', + isPackaged: false, + on: () => {}, + whenReady: () => Promise.resolve() + }, + safeStorage: { + isEncryptionAvailable: () => false, + encryptString: (value: string) => Buffer.from(value), + decryptString: (value: Buffer) => value.toString() + }, + ipcMain: { on: () => {}, handle: () => {} }, + BrowserWindow: { getAllWindows: () => [] } +})) + +afterEach(closeMoveTestStores) + +describe('moving a pane to a new tab', () => { + it('moves the leaf and its binding in one write and re-keys pane-keyed records', async () => { + const store = openStore(newDataFile()) + await seedSplitSource(store) + store.setWorkspaceSession({ + ...store.getWorkspaceSession(), + sleepingAgentSessionsByPaneKey: { [FROM]: sleeping(FROM, SOURCE) } + }) + store.updateUI({ + acknowledgedAgentsByPaneKey: { [FROM]: 10 }, + activityClearedAtByPaneKey: { [FROM]: 11 }, + manuallyUnreadTurnsByPaneKey: { [FROM]: 12 } + }) + + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + ).resolves.toEqual({ status: 'moved', ptyId: 'pty-agent' }) + + const session = store.getWorkspaceSession() + expect(session.tabsByWorktree[WT]?.map((tab) => tab.id)).toEqual([SOURCE, TARGET]) + expect(session.terminalLayoutsByTabId[SOURCE]).toMatchObject({ + root: { type: 'leaf', leafId: LEFT }, + ptyIdsByLeafId: { [LEFT]: 'pty-left' } + }) + expect(session.terminalLayoutsByTabId[TARGET]).toMatchObject({ + root: { type: 'leaf', leafId: MOVED }, + ptyIdsByLeafId: { [MOVED]: 'pty-agent' } + }) + expect(session.terminalPtyIncarnationsByPaneKey).toEqual({ + [`${SOURCE}:${LEFT}`]: 'inc-left', + [TO]: 'inc-1' + }) + expect(session.sleepingAgentSessionsByPaneKey).toEqual({ + [TO]: expect.objectContaining({ paneKey: TO, tabId: TARGET }) + }) + expect(store.getUI()).toMatchObject({ + acknowledgedAgentsByPaneKey: { [TO]: 10 }, + activityClearedAtByPaneKey: { [TO]: 11 }, + manuallyUnreadTurnsByPaneKey: { [TO]: 12 } + }) + expect(ownersOf(session, 'pty-agent')).toEqual([TO]) + }) + + it('re-keys the SSH lease and moves within the partition that holds the tab', async () => { + const store = openStore(newDataFile()) + const hostId = toSshExecutionHostId('ssh-1') + await seedSplitSource(store, hostId) + store.upsertSshRemotePtyLease({ + targetId: 'ssh-1', + ptyId: 'pty-agent', + worktreeId: WT, + tabId: SOURCE, + leafId: MOVED, + state: 'attached' + }) + + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + ).resolves.toMatchObject({ status: 'moved' }) + + expect(tabsHoldingLeaf(store.getWorkspaceSession(hostId), MOVED)).toEqual([TARGET]) + expect(store.getWorkspaceSession().tabsByWorktree[WT]).toBeUndefined() + expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([ + expect.objectContaining({ ptyId: 'pty-agent', tabId: TARGET, leafId: MOVED }) + ]) + }) + + it('reports a leaf main never held instead of inventing one', async () => { + const store = openStore(newDataFile()) + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + ).resolves.toEqual({ status: 'not_held' }) + expect(store.getWorkspaceSession().tabsByWorktree[WT]).toBeUndefined() + }) + + it('refuses a move that names another terminal or an existing tab', async () => { + const store = openStore(newDataFile()) + await seedSplitSource(store) + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-other' }) + ).resolves.toEqual({ status: 'refused', reason: 'pty_mismatch' }) + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, targetTabId: SOURCE, ptyId: 'pty-agent' }) + ).resolves.toEqual({ status: 'refused', reason: 'target_tab_exists' }) + expect(tabsHoldingLeaf(store.getWorkspaceSession(), MOVED)).toEqual([SOURCE]) + }) + + it('moves a leaf that a stray layout of a removed tab still names', () => { + const planned = planTerminalLeafMove( + [ + { + hostId: 'local', + session: { + activeRepoId: 'repo-1', + activeWorktreeId: WT, + activeTabId: SOURCE, + tabsByWorktree: { [WT]: [makeTerminalTab({ id: SOURCE, worktreeId: WT })] }, + terminalLayoutsByTabId: { + [SOURCE]: { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEFT }, + second: { type: 'leaf', leafId: MOVED } + }, + activeLeafId: LEFT, + expandedLeafId: null + }, + 'tab-removed': { + root: { type: 'leaf', leafId: MOVED }, + activeLeafId: MOVED, + expandedLeafId: null + } + } + } + } + ], + { ...moveRequest, ptyId: 'pty-agent' } + ) + expect(planned.result).toEqual({ status: 'moved', ptyId: 'pty-agent' }) + }) + + it('records a persistence.terminal-topology span without pane keys or PTY ids', async () => { + const records: { name: string; attributes: Record }[] = [] + setActiveSink({ + push: (record) => { + records.push(JSON.parse(JSON.stringify(record))) + }, + flush: () => {}, + close: () => {} + }) + try { + const store = openStore(newDataFile()) + await seedSplitSource(store) + records.length = 0 + const request = { ...moveRequest, ptyId: 'pty-agent' } + await store.moveTerminalLeafToNewTab(request) + await store.moveTerminalLeafToNewTab(request) + await store.moveTerminalLeafToNewTab({ ...request, targetTabId: 'tab-other' }) + + const spans = records.filter((record) => record.name === 'persistence.terminal-topology') + expect(spans.map((span) => span.attributes)).toEqual([ + { kind: 'persistence', 'topology.kind': 'move_leaf', 'topology.outcome': 'committed' }, + { + kind: 'persistence', + 'topology.kind': 'move_leaf', + 'topology.outcome': 'refused', + 'topology.refusal': 'target_tab_exists' + }, + { + kind: 'persistence', + 'topology.kind': 'move_leaf', + 'topology.outcome': 'refused', + 'topology.refusal': 'leaf_in_other_tab' + } + ]) + expect(JSON.stringify(spans)).not.toMatch(/pty-|tab-source|tab-target|2222/) + } finally { + _resetTracerForTests() + } + }) +}) + +// After a restart the relay reattach writes the SSH pane into `local` as well as `ssh:`; a move +// that left either copy behind refused the moved pane and the next relay reattach. +describe('moving an SSH pane held by both partitions', () => { + it('moves every copy, so the target reattach and the next relay reattach both bind', async () => { + const store = openStore(newDataFile()) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + const hostId = toSshExecutionHostId('ssh-1') + await seedSplitSource(store, hostId) + const relay = { worktreeId: WT, leafId: MOVED, ptyId: 'pty-agent', incarnationId: 'inc-1' } + expect( + await store.persistPtyBinding({ ...relay, tabId: SOURCE, origin: 'relay_reattach' }) + ).toBe(true) + + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + ).resolves.toMatchObject({ status: 'moved' }) + + expect(tabsHoldingLeaf(store.getWorkspaceSession(), MOVED)).toEqual([TARGET]) + expect(tabsHoldingLeaf(store.getWorkspaceSession(hostId), MOVED)).toEqual([TARGET]) + expect( + await store.persistPtyBinding({ ...relay, tabId: TARGET, origin: 'reattach' }, hostId) + ).toBe(true) + expect( + await store.persistPtyBinding({ + ...relay, + tabId: TARGET, + origin: 'relay_reattach', + mayReviveRetiredSurface: false + }) + ).toBe(true) + }) + + // Relay reattach into local, move, target reattach in ssh:, next-start relay reattach. + it('keeps one holder per partition across a restart, and the next relay reattach binds', async () => { + const dataFile = newDataFile() + const store = openStore(dataFile) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + store.addRepo(makeRepo({ id: 'repo-1', path: '/tmp/move-worktree' })) + const hostId = toSshExecutionHostId('ssh-1') + await seedSplitSource(store, hostId) + const relay = { worktreeId: WT, leafId: MOVED, ptyId: 'pty-agent', incarnationId: 'inc-1' } + await store.persistPtyBinding({ ...relay, tabId: SOURCE, origin: 'relay_reattach' }) + await store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + expect( + await store.persistPtyBinding({ ...relay, tabId: TARGET, origin: 'reattach' }, hostId) + ).toBe(true) + store.flush() + + const restarted = openStore(dataFile) + expect( + await restarted.persistPtyBinding({ + ...relay, + tabId: TARGET, + origin: 'relay_reattach', + mayReviveRetiredSurface: false + }) + ).toBe(true) + for (const session of [ + restarted.getWorkspaceSession(), + restarted.getWorkspaceSession(hostId) + ]) { + expect(tabsHoldingLeaf(session, MOVED)).toEqual([TARGET]) + expect(ownersOf(session, 'pty-agent')).toEqual([TO]) + } + }) + + it('follows the live PTY when an SSH respawn bound one partition before the other', async () => { + const store = openStore(newDataFile()) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + const hostId = toSshExecutionHostId('ssh-1') + await seedSplitSource(store, hostId) + const relay = { worktreeId: WT, tabId: SOURCE, leafId: MOVED, incarnationId: 'inc-1' } + await store.persistPtyBinding({ ...relay, ptyId: 'pty-agent', origin: 'relay_reattach' }) + await store.persistPtyBinding( + { ...relay, ptyId: 'pty-respawn', incarnationId: 'inc-2' }, + hostId + ) + + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-respawn' }) + ).resolves.toEqual({ status: 'moved', ptyId: 'pty-respawn' }) + + for (const session of [store.getWorkspaceSession(), store.getWorkspaceSession(hostId)]) { + expect(tabsHoldingLeaf(session, MOVED)).toEqual([TARGET]) + expect(session.terminalLayoutsByTabId[TARGET]?.ptyIdsByLeafId).toEqual({ + [MOVED]: 'pty-respawn' + }) + } + expect(store.getWorkspaceSession(hostId).terminalPtyIncarnationsByPaneKey?.[TO]).toBe('inc-2') + expect(store.getWorkspaceSession().terminalPtyIncarnationsByPaneKey?.[TO]).toBeUndefined() + }) + + it('refuses a move while another tab already holds the leaf', async () => { + const store = openStore(newDataFile()) + await seedSplitSource(store) + const session = store.getWorkspaceSession() + store.setWorkspaceSession({ + ...session, + tabsByWorktree: { + ...session.tabsByWorktree, + [WT]: [ + ...(session.tabsByWorktree[WT] ?? []), + { ...(session.tabsByWorktree[WT] ?? [])[0]!, id: 'tab-earlier-move' } + ] + }, + terminalLayoutsByTabId: { + ...session.terminalLayoutsByTabId, + 'tab-earlier-move': { + root: { type: 'leaf', leafId: MOVED }, + activeLeafId: MOVED, + expandedLeafId: null + } + } + }) + + await expect( + store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + ).resolves.toEqual({ status: 'refused', reason: 'leaf_in_other_tab' }) + }) +}) + +// STA-9259: detach, then the moved pane's reattach binding, then a renderer snapshot that still +// shows the pre-move layout, then a restart. Before the move transaction this left one leaf and +// one PTY in both tabs. +describe('STA-9259 move sequence', () => { + it('keeps one owner through reattach, a stale renderer save and a restart', async () => { + const dataFile = newDataFile() + const store = openStore(dataFile) + // Load sweeps sessions of unregistered repos, so the restart needs a real owner. + store.addRepo(makeRepo({ id: 'repo-1', path: '/tmp/move-worktree' })) + await seedSplitSource(store) + const preMoveRendererSnapshot = structuredClone(store.getWorkspaceSession()) + + await store.moveTerminalLeafToNewTab({ ...moveRequest, ptyId: 'pty-agent' }) + // The moved pane mounts in the target tab and reattaches with its stable-owner fence. + const reattached = await store.persistPtyBinding({ + worktreeId: WT, + tabId: TARGET, + leafId: MOVED, + ptyId: 'pty-agent', + incarnationId: 'inc-1', + expectedBinding: { ptyId: 'pty-agent', incarnationId: 'inc-1' }, + origin: 'reattach' + }) + expect(reattached).toBe(true) + // A debounced renderer save that predates the move must not resurrect the source copy. + store.setWorkspaceSession(preMoveRendererSnapshot) + expect(ownersOf(store.getWorkspaceSession(), 'pty-agent')).toEqual([TO]) + expect(tabsHoldingLeaf(store.getWorkspaceSession(), MOVED)).toEqual([TARGET]) + + store.flush() + const restarted = openStore(dataFile) + expect(ownersOf(restarted.getWorkspaceSession(), 'pty-agent')).toEqual([TO]) + expect(tabsHoldingLeaf(restarted.getWorkspaceSession(), MOVED)).toEqual([TARGET]) + }) +}) diff --git a/src/main/persistence/terminal-topology/terminal-leaf-move.ts b/src/main/persistence/terminal-topology/terminal-leaf-move.ts new file mode 100644 index 00000000000..627e4a705e7 --- /dev/null +++ b/src/main/persistence/terminal-topology/terminal-leaf-move.ts @@ -0,0 +1,236 @@ +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { SshRemotePtyLease } from '../../../shared/ssh-types' +import { + terminalLeafMovePaneKeys, + type TerminalLeafMoveRequest, + type TerminalLeafMoveResult +} from '../../../shared/terminal-leaf-move' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { retireLeavesFromTerminalLayout } from '../../runtime/mobile-session-terminal-retirement' +import { createMinimalPersistedTerminalTab } from '../restoring-sessions/session-owner-fields' +import { layoutContainsLeafId } from '../restoring-sessions/terminal-layout-normalization' +import { + advanceTerminalTopologyRevision, + isTerminalOwnerPartition, + type TerminalSessionPartition +} from './terminal-topology-membership' + +type PlannedTerminalLeafMove = { + result: TerminalLeafMoveResult + /** Every partition the plan rewrote; empty when nothing changes. */ + sessions: TerminalSessionPartition[] +} + +function moveRecordKey( + record: Record | undefined, + fromKey: string, + toKey: string | null, + remap: (value: T) => T = (value) => value +): Record | undefined { + if (!record || !Object.hasOwn(record, fromKey)) { + return record + } + const next = { ...record } + const value = next[fromKey] + delete next[fromKey] + if (toKey !== null && value !== undefined) { + next[toKey] = remap(value) + } + return next +} + +function liveTabIds(session: WorkspaceSessionState): Set { + return new Set( + Object.values(session.tabsByWorktree ?? {}).flatMap((tabs) => tabs.map((tab) => tab.id)) + ) +} + +function hasTabId(session: WorkspaceSessionState, tabId: string): boolean { + return ( + liveTabIds(session).has(tabId) || Object.hasOwn(session.terminalLayoutsByTabId ?? {}, tabId) + ) +} + +type SourceHolder = TerminalSessionPartition & { + sourceTab: TerminalTab + sourceLayout: TerminalLayoutSnapshot +} + +function moveLeafInPartition( + { session, sourceTab, sourceLayout }: SourceHolder, + request: TerminalLeafMoveRequest, + ptyId: string | null +): WorkspaceSessionState { + const { worktreeId, sourceTabId, targetTabId, leafId } = request + const tabs = session.tabsByWorktree?.[worktreeId] ?? [] + const { from: fromPaneKey, to: toPaneKey } = terminalLeafMovePaneKeys(request) + const boundHere = sourceLayout.ptyIdsByLeafId?.[leafId] + const remainingLayout = retireLeavesFromTerminalLayout(sourceLayout, new Set([leafId])) + const remainingPtyIds = Object.values(remainingLayout?.ptyIdsByLeafId ?? {}) + const { pendingActivationSpawn, ...row } = createMinimalPersistedTerminalTab({ + worktreeId, + tabId: targetTabId, + ptyId: ptyId ?? '', + existingTabCount: tabs.length, + ...(sourceTab.startupCwd ? { startupCwd: sourceTab.startupCwd } : {}) + }) + const targetTab = { + ...row, + ptyId, + // A moved live pane reattaches; only an unbound one still spawns on activation. + ...(ptyId ? {} : { pendingActivationSpawn }), + ...(sourceTab.shellOverride ? { shellOverride: sourceTab.shellOverride } : {}) + } + const nextTabs = tabs.map((tab) => + tab.id === sourceTabId && tab.ptyId === ptyId + ? { ...tab, ptyId: remainingPtyIds[0] ?? null } + : tab + ) + const movedTitle = sourceLayout.titlesByLeafId?.[leafId] + const terminalLayoutsByTabId = { + ...session.terminalLayoutsByTabId, + [targetTabId]: { + root: { type: 'leaf' as const, leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ...(ptyId ? { ptyIdsByLeafId: { [leafId]: ptyId } } : {}), + ...(movedTitle ? { titlesByLeafId: { [leafId]: movedTitle } } : {}), + ...(sourceLayout.chatLeafId === leafId ? { chatLeafId: leafId } : {}) + } + } + if (remainingLayout) { + terminalLayoutsByTabId[sourceTabId] = remainingLayout + } else { + // Its sibling was never bound here; the sibling's own binding mints the layout again. + delete terminalLayoutsByTabId[sourceTabId] + } + const remoteSessionIdsByTabId = { ...session.remoteSessionIdsByTabId } + if (ptyId && remoteSessionIdsByTabId[sourceTabId] === ptyId) { + remoteSessionIdsByTabId[targetTabId] = ptyId + if (remainingPtyIds[0]) { + remoteSessionIdsByTabId[sourceTabId] = remainingPtyIds[0] + } else { + delete remoteSessionIdsByTabId[sourceTabId] + } + } + return advanceTerminalTopologyRevision( + { + ...session, + tabsByWorktree: { ...session.tabsByWorktree, [worktreeId]: [...nextTabs, targetTab] }, + terminalLayoutsByTabId, + ...(session.remoteSessionIdsByTabId ? { remoteSessionIdsByTabId } : {}), + // A stale copy's incarnation belongs to the PTY it no longer names. + terminalPtyIncarnationsByPaneKey: moveRecordKey( + session.terminalPtyIncarnationsByPaneKey, + fromPaneKey, + boundHere && boundHere !== ptyId ? null : toPaneKey + ), + sleepingAgentSessionsByPaneKey: moveRecordKey( + session.sleepingAgentSessionsByPaneKey, + fromPaneKey, + toPaneKey, + (record) => ({ ...record, paneKey: toPaneKey, tabId: targetTabId }) + ) + }, + worktreeId + ) +} + +/** + * Moves one leaf and its binding into a new tab in a single session write (STA-9259). The leaf + * id and the PTY are kept; only the tab half of the pane key changes, so every pane-keyed record + * follows it here instead of being rebuilt later by a renderer save that main's membership rebase + * would discard. Every owner partition holding the pane moves together: a relay reattach writes an + * SSH pane into `local` as well as `ssh:`, and a copy left behind refuses the moved pane's bind. + */ +export function planTerminalLeafMove( + partitions: readonly TerminalSessionPartition[], + request: TerminalLeafMoveRequest +): PlannedTerminalLeafMove { + const { worktreeId, sourceTabId, leafId } = request + const refuse = ( + reason: Extract['reason'] + ): PlannedTerminalLeafMove => ({ result: { status: 'refused', reason }, sessions: [] }) + const owners = partitions.filter(({ hostId }) => isTerminalOwnerPartition(hostId)) + if (partitions.some(({ session }) => hasTabId(session, request.targetTabId))) { + return refuse('target_tab_exists') + } + // A layout left behind by a removed tab row owns nothing. + const leafElsewhere = owners.some(({ session }) => { + const tabIds = liveTabIds(session) + return Object.entries(session.terminalLayoutsByTabId ?? {}).some( + ([tabId, layout]) => + tabId !== sourceTabId && + tabIds.has(tabId) && + layoutContainsLeafId(layout?.root ?? null, leafId) + ) + }) + if (leafElsewhere) { + return refuse('leaf_in_other_tab') + } + const holders = owners.flatMap(({ hostId, session }) => { + const sourceTab = session.tabsByWorktree?.[worktreeId]?.find((tab) => tab.id === sourceTabId) + const sourceLayout = session.terminalLayoutsByTabId?.[sourceTabId] + return sourceTab && sourceLayout && layoutContainsLeafId(sourceLayout.root, leafId) + ? [{ hostId, session, sourceTab, sourceLayout }] + : [] + }) + // Main never saw this leaf, so no binding of it can be duplicated here. + if (holders.length === 0) { + return { result: { status: 'not_held' }, sessions: [] } + } + const boundPtyIds = new Set( + holders.flatMap(({ sourceLayout }) => sourceLayout.ptyIdsByLeafId?.[leafId] ?? []) + ) + // The renderer's live PTY id wins: after an SSH respawn one copy can still name the old PTY. + const ptyId = request.ptyId ?? (boundPtyIds.size === 1 ? [...boundPtyIds][0] : null) + if (boundPtyIds.size > 0 && !(ptyId && boundPtyIds.has(ptyId))) { + return refuse('pty_mismatch') + } + return { + result: { status: 'moved', ptyId }, + sessions: holders.map((holder) => ({ + hostId: holder.hostId, + session: moveLeafInPartition(holder, request, ptyId) + })) + } +} + +const PANE_KEYED_UI_RECORDS = [ + 'acknowledgedAgentsByPaneKey', + 'activityClearedAtByPaneKey', + 'manuallyUnreadTurnsByPaneKey' +] as const + +/** Pane-keyed UI marks and SSH lease leaf addresses that must follow a moved leaf. */ +export function rekeyMovedLeafProfileRecords( + state: Pick, + move: TerminalLeafMoveRequest +): { ui?: PersistedState['ui']; sshRemotePtyLeases?: SshRemotePtyLease[] } { + const { from: fromPaneKey, to: toPaneKey } = terminalLeafMovePaneKeys(move) + const ui = state.ui + let nextUi: PersistedState['ui'] | undefined + for (const key of PANE_KEYED_UI_RECORDS) { + const moved = moveRecordKey(ui?.[key], fromPaneKey, toPaneKey) + if (ui && moved !== ui[key]) { + nextUi = { ...(nextUi ?? ui), [key]: moved } + } + } + const leases = state.sshRemotePtyLeases ?? [] + const leasesChanged = leases.some( + (lease) => lease.tabId === move.sourceTabId && lease.leafId === move.leafId + ) + return { + ...(nextUi ? { ui: nextUi } : {}), + ...(leasesChanged + ? { + sshRemotePtyLeases: leases.map((lease) => + lease.tabId === move.sourceTabId && lease.leafId === move.leafId + ? { ...lease, tabId: move.targetTabId } + : lease + ) + } + : {}) + } +} diff --git a/src/main/persistence/terminal-topology/terminal-topology-commit.ts b/src/main/persistence/terminal-topology/terminal-topology-commit.ts index dee0f6283a9..dc1b562a51d 100644 --- a/src/main/persistence/terminal-topology/terminal-topology-commit.ts +++ b/src/main/persistence/terminal-topology/terminal-topology-commit.ts @@ -1,15 +1,24 @@ +import type { ExecutionHostId } from '../../../shared/execution-host' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { + TerminalLeafMoveRequest, + TerminalLeafMoveResult +} from '../../../shared/terminal-leaf-move' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { startSpan } from '../../observability/tracer' import { terminalSurfaceCloseMutation, type TerminalSurfaceCloseCommit } from '../../runtime/terminal-surface-close' import type { DurableProfileStateMutation } from '../loading-store/store-runtime-state' +import { planTerminalLeafMove, rekeyMovedLeafProfileRecords } from './terminal-leaf-move' +import { assignWorkspaceSessionPartition } from './terminal-topology-membership' -// The commit boundary for terminal layout (tabs, panes, pane-to-PTY bindings). Today it wraps only -// the close, whose transform still lives in runtime/; the other writers move here later. +// The commit boundary for terminal layout (tabs, panes, pane-to-PTY bindings). Wraps the close and +// the pane move; the close transform still lives in runtime/ and other writers move here later. /** Bindings are not listed: `persistPtyBinding` already records `persistence.pty-binding`. */ -type TerminalTopologyCommitKind = 'close_leaf' | 'close_tab' +type TerminalTopologyCommitKind = 'close_leaf' | 'close_tab' | 'move_leaf' export function closeLeafOrTab( commit: TerminalSurfaceCloseCommit @@ -22,9 +31,21 @@ export function closeLeafOrTab( ) } +/** Moves a leaf, its binding and its pane-keyed records into a new tab in one durable mutation. */ +export function moveLeaf( + request: TerminalLeafMoveRequest, + context: TerminalTopologyCommitContext +): () => DurableProfileStateMutation { + return traced( + 'move_leaf', + () => commitLeafMove(request, context), + (result) => (result.status === 'refused' ? result.reason : undefined) + ) +} + /** * One `persistence.terminal-topology` span per commit, from admission to the in-memory write. - * Attributes stay low-cardinality: no pane key, PTY id or path. + * Attributes stay low-cardinality: no pane key, PTY id or path; `refusalOf` returns a fixed code. */ function traced( kind: TerminalTopologyCommitKind, @@ -55,3 +76,74 @@ function traced( return result } } + +type TopologyState = Pick< + PersistedState, + 'workspaceSession' | 'workspaceSessionsByHostId' | 'ui' | 'sshRemotePtyLeases' +> + +type TerminalTopologyCommitContext = { + state: TopologyState + hostIds: () => ExecutionHostId[] + getSession: (hostId: ExecutionHostId) => WorkspaceSessionState + markDirty: ( + domain: 'workspaceSession' | 'workspaceSessionsByHostId' | 'ui' | 'sshRemotePtyLeases' + ) => void +} + +/** Writes `next`, returning a restore that puts the prior value back unless a later write replaced it. */ +function writeRestorable(read: () => V, write: (value: V) => void, next: V): () => void { + const prior = read() + write(next) + return () => { + if (read() === next) { + write(prior) + } + } +} + +function commitLeafMove( + request: TerminalLeafMoveRequest, + context: TerminalTopologyCommitContext +): DurableProfileStateMutation { + const { state } = context + const planned = planTerminalLeafMove( + context.hostIds().map((hostId) => ({ hostId, session: context.getSession(hostId) })), + request + ) + if (planned.sessions.length === 0) { + return { value: planned.result, persist: false } + } + const restores = planned.sessions.map(({ hostId, session }) => + writeRestorable( + () => context.getSession(hostId), + (value) => context.markDirty(assignWorkspaceSessionPartition(state, hostId, value)), + session + ) + ) + const rekeyed = rekeyMovedLeafProfileRecords(state, request) + if (rekeyed.ui) { + restores.push( + writeRestorable( + () => state.ui, + (ui) => (state.ui = ui), + rekeyed.ui + ) + ) + context.markDirty('ui') + } + if (rekeyed.sshRemotePtyLeases) { + restores.push( + writeRestorable( + () => state.sshRemotePtyLeases, + (leases) => (state.sshRemotePtyLeases = leases), + rekeyed.sshRemotePtyLeases + ) + ) + context.markDirty('sshRemotePtyLeases') + } + return { + value: planned.result, + rollback: () => restores.forEach((restore) => restore()) + } +} diff --git a/src/main/persistence/terminal-topology/terminal-topology-membership.ts b/src/main/persistence/terminal-topology/terminal-topology-membership.ts index 99437157437..358a1a34736 100644 --- a/src/main/persistence/terminal-topology/terminal-topology-membership.ts +++ b/src/main/persistence/terminal-topology/terminal-topology-membership.ts @@ -1,3 +1,9 @@ +import { + LOCAL_EXECUTION_HOST_ID, + parseExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' +import type { PersistedState } from '../../../shared/persisted-state-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' import { layoutContainsLeafId } from '../restoring-sessions/terminal-layout-normalization' @@ -54,3 +60,29 @@ export function hasHostAuthoritativeTerminalMembership( ) ) } + +export type TerminalSessionPartition = { + hostId: ExecutionHostId + session: WorkspaceSessionState +} + +/** Partitions that own terminal panes; `runtime:` partitions mirror another host's. */ +export function isTerminalOwnerPartition(hostId: ExecutionHostId): boolean { + const kind = parseExecutionHostId(hostId)?.kind + return kind === 'local' || kind === 'ssh' +} + +/** Puts one partition's session in place and names the profile domain that now needs a write. */ +export function assignWorkspaceSessionPartition( + state: Pick, + hostId: ExecutionHostId, + session: WorkspaceSessionState +): 'workspaceSession' | 'workspaceSessionsByHostId' { + // Why: 'local' always lives in workspaceSession, never workspaceSessionsByHostId.local. + if (hostId === LOCAL_EXECUTION_HOST_ID) { + state.workspaceSession = session + return 'workspaceSession' + } + state.workspaceSessionsByHostId = { ...state.workspaceSessionsByHostId, [hostId]: session } + return 'workspaceSessionsByHostId' +} diff --git a/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.ts b/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.ts index fe26380d539..b5061de5950 100644 --- a/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.ts +++ b/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.ts @@ -244,10 +244,27 @@ export function retainReplacedWorkerTerminalResources( ) } +/** A detached pane keeps its process, so the resources it owns follow its new pane key. */ +export function rekeyWorkerTerminalResourcePaneKey( + this: OrchestrationDb, + params: { fromPaneKey: string; toPaneKey: string } +): number { + return Number( + this.db + .prepare( + `UPDATE worker_terminal_resources + SET pane_key = ?, updated_at = datetime('now') + WHERE pane_key = ? AND release_state != 'released'` + ) + .run(params.toPaneKey, params.fromPaneKey).changes + ) +} + // Finds an owned, settled, exact-match resource for an explicitly reused terminal. export type WorkerTerminalResourceStoreMethods = { retainReplacedWorkerTerminalResources: typeof retainReplacedWorkerTerminalResources + rekeyWorkerTerminalResourcePaneKey: typeof rekeyWorkerTerminalResourcePaneKey backfillWorkerTerminalResources: typeof backfillWorkerTerminalResources createWorkerTerminalResourceStatement: typeof createWorkerTerminalResourceStatement getWorkerTerminalResource: typeof getWorkerTerminalResource @@ -262,6 +279,7 @@ export type WorkerTerminalResourceStoreMethods = { export function attachWorkerTerminalResourceStore(ctor: { prototype: object }): void { Object.assign(ctor.prototype, { retainReplacedWorkerTerminalResources, + rekeyWorkerTerminalResourcePaneKey, backfillWorkerTerminalResources, createWorkerTerminalResourceStatement, getWorkerTerminalResource, diff --git a/src/preload/api/pty-api.ts b/src/preload/api/pty-api.ts index 8a7604b4ac2..8a75d7fba5a 100644 --- a/src/preload/api/pty-api.ts +++ b/src/preload/api/pty-api.ts @@ -4,6 +4,10 @@ import type { } from '../../shared/agent-session-resume' import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' import type { TerminalInputKind } from '../../shared/terminal-input-kind' +import type { + TerminalLeafMoveRequest, + TerminalLeafMoveResult +} from '../../shared/terminal-leaf-move' import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' import type { PtyListedSession, PtySessionListScope } from '../../shared/pty-listed-session' import type { PtyMainDeliveryDiagnostics } from '../../shared/pty-delivery-diagnostics' @@ -145,6 +149,7 @@ export type PtyApi = { ids: string[] ) => Promise<{ id: string; authoritative: boolean | null }[]> hasPty: (id: string) => Promise + moveLeafToNewTab: (request: TerminalLeafMoveRequest) => Promise getMainBufferSnapshot: ( id: string, opts?: { scrollbackRows?: number } diff --git a/src/preload/api/pty-bridge-session-control.ts b/src/preload/api/pty-bridge-session-control.ts index e407163e5f6..91fdb9a5fb6 100644 --- a/src/preload/api/pty-bridge-session-control.ts +++ b/src/preload/api/pty-bridge-session-control.ts @@ -2,6 +2,10 @@ import { ipcRenderer } from 'electron' import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' import type { TerminalInputKind } from '../../shared/terminal-input-kind' +import type { + TerminalLeafMoveRequest, + TerminalLeafMoveResult +} from '../../shared/terminal-leaf-move' import type { AgentProviderSessionMetadata, SleepingAgentLaunchConfig @@ -160,6 +164,8 @@ export const ptySessionControlApi = { ): Promise<{ id: string; authoritative: boolean | null }[]> => ipcRenderer.invoke('pty:getAuthoritativeBufferSnapshotCapabilities', { ids }), hasPty: (id: string): Promise => ipcRenderer.invoke('pty:hasPty', { id }), + moveLeafToNewTab: (request: TerminalLeafMoveRequest): Promise => + ipcRenderer.invoke('pty:moveLeafToNewTab', request), getMainBufferSnapshot: ( id: string, opts?: { scrollbackRows?: number } 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 59502adcca6..42171d9368d 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 @@ -66,6 +66,28 @@ describe('createIpcPtyTransport', () => { transport.disconnect() }) + // A pane drag waits out a spawn whose result would bind the leaf to its old tab. + it('reports a connect as pending only until its PTY id arrives', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + let resolveSpawn!: (value: { id: string }) => void + vi.mocked(window.api.pty.spawn).mockReturnValue( + new Promise((resolve) => { + resolveSpawn = resolve + }) + ) + const transport = createIpcPtyTransport({}) + expect(transport.isConnectPending?.()).toBe(false) + + const connecting = transport.connect({ url: '', callbacks: {} }) + expect(transport.isConnectPending?.()).toBe(true) + resolveSpawn({ id: 'pty-late' }) + await connecting + + expect(transport.getPtyId()).toBe('pty-late') + expect(transport.isConnectPending?.()).toBe(false) + transport.disconnect() + }) + it('does not create a PTY when the pane generation is stale', 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 39f1c7962ef..4b2de188fce 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -223,6 +223,8 @@ export type PtyTransport = { /** The user dismissed the error surface; the next occurrence of the same message must surface again. */ notifyErrorSurfaceDismissed?: () => void getPtyId: () => string | null + /** A connect (spawn or reattach) is still awaiting its PTY id. */ + isConnectPending?: () => boolean getConnectionId?: () => string | null | undefined /** The runtime captured by this transport; legacy remote PTY ids do not * encode their owner, and current worktree settings may have changed. */ diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index d1b9242243e..9f9783d6c1d 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -49,6 +49,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra let onAbandonedConnect: ((ptyId: string) => boolean) | undefined let ptyId: string | null = null let lifecycleGeneration = 0 + let pendingConnectGeneration: number | null = null let lastExitGeneration: number | null = null let suppressAttentionEvents = false let storedCallbacks: Parameters[0]['callbacks'] = {} @@ -145,6 +146,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra getPendingEscapeTailAnsi: outputProcessor.getPendingEscapeTailAnsi, connect: async (options) => { const connectGeneration = advancePtyLifecycle() + pendingConnectGeneration = connectGeneration try { return await connectIpcPty(options, { transportOptions: opts, @@ -162,12 +164,18 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra getCallbacks: () => storedCallbacks }) } finally { + if (pendingConnectGeneration === connectGeneration) { + pendingConnectGeneration = null + } if (lifecycleGeneration === connectGeneration) { await flushPreconnectInput() } } }, + isConnectPending: () => + !destroyed && ptyId === null && pendingConnectGeneration === lifecycleGeneration, + attach: (options) => { const attachGeneration = advancePtyLifecycle() try { diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach-commit.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach-commit.test.ts new file mode 100644 index 00000000000..8b83383572d --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach-commit.test.ts @@ -0,0 +1,272 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { + TerminalLeafMoveRequest, + TerminalLeafMoveResult +} from '../../../../shared/terminal-leaf-move' +import { detachTerminalPaneToTab } from './terminal-pane-tab-detach' +import { + createStore, + LEAF_1, + LEAF_2, + SOURCE_TAB_ID, + splitLayout, + TARGET_GROUP_ID, + unboundSplitLayout, + WORKTREE_ID +} from './terminal-pane-tab-detach-fixture' + +const toastErrorMock = vi.hoisted(() => vi.fn()) +vi.mock('sonner', () => ({ toast: { error: toastErrorMock } })) +const closeTerminalSurface = vi.fn(async () => {}) +beforeEach(() => { + toastErrorMock.mockClear() + closeTerminalSurface.mockClear() + vi.spyOn(console, 'warn').mockImplementation(() => {}) +}) + +/** Answers each call with the next result; an Error throws. */ +function mainAnswering(results: (TerminalLeafMoveResult | Error)[]) { + return vi.fn((_request: TerminalLeafMoveRequest): Promise => { + const next = results.shift() ?? { status: 'not_held' } + return next instanceof Error ? Promise.reject(next) : Promise.resolve(next) + }) +} + +function managerWithPanes(paneIds: () => number[] = () => [1, 2]) { + return { + getPanes: vi.fn(() => paneIds().map((id) => ({ id }))), + getLeafId: vi.fn((paneId: number): string | null => (paneId === 2 ? LEAF_2 : LEAF_1)), + detachPaneForExternalMove: vi.fn(() => true) + } +} + +type CommitMove = (request: TerminalLeafMoveRequest) => Promise + +function detach( + overrides: Partial[0]> & { + commitMove?: CommitMove + store?: ReturnType + } +) { + const { commitMove = mainAnswering([]), store = createStore(), ...rest } = overrides + vi.stubGlobal('window', { + api: { pty: { moveLeafToNewTab: commitMove }, session: { closeTerminalSurface } } + }) + return detachTerminalPaneToTab({ + getStore: () => store, + manager: managerWithPanes(), + persistLayoutSnapshot: vi.fn(), + sourcePaneId: 2, + sourceTabId: SOURCE_TAB_ID, + targetGroupId: TARGET_GROUP_ID, + worktreeId: WORKTREE_ID, + ...rest + }) +} + +const moved: TerminalLeafMoveResult = { status: 'moved', ptyId: 'remote:env-1@@terminal-1' } + +describe('detachTerminalPaneToTab rolls a committed move forward', () => { + it('refuses, silently, a pane whose PTY spawn is still in flight', async () => { + const commitMove = mainAnswering([{ status: 'moved', ptyId: null }]) + const store = createStore(unboundSplitLayout()) + + await expect(detach({ commitMove, store, sourceConnectPending: true })).resolves.toBeNull() + + expect(commitMove).not.toHaveBeenCalled() + expect(store.createTab).not.toHaveBeenCalled() + expect(toastErrorMock).not.toHaveBeenCalled() + }) + + it('shows the failure toast and keeps the pane when main refuses or the commit throws', async () => { + for (const results of [ + [{ status: 'refused', reason: 'pty_mismatch' } as const], + [new Error('write failed')] + ]) { + toastErrorMock.mockClear() + const store = createStore() + const manager = managerWithPanes() + + await expect( + detach({ commitMove: mainAnswering(results), store, manager }) + ).resolves.toBeNull() + + expect(manager.detachPaneForExternalMove).not.toHaveBeenCalled() + expect(store.createTab).not.toHaveBeenCalled() + expect(toastErrorMock).toHaveBeenCalledOnce() + } + }) + + it('closes main’s new tab, without a toast, when the user closed the pane meanwhile', async () => { + let paneIds = [1, 2] + const manager = managerWithPanes(() => paneIds) + const commitMove = vi.fn( + async (_request: TerminalLeafMoveRequest): Promise => { + paneIds = [1] + manager.getLeafId.mockImplementation((paneId) => (paneId === 1 ? LEAF_1 : null)) + return moved + } + ) + const store = createStore() + + await expect(detach({ commitMove, manager, store })).resolves.toBeNull() + + const targetTabId = commitMove.mock.calls[0]?.[0]?.targetTabId + expect(closeTerminalSurface).toHaveBeenCalledWith({ + worktreeId: WORKTREE_ID, + target: { kind: 'tab', tabId: targetTabId }, + reason: 'cleanup' + }) + expect(store.createTab).not.toHaveBeenCalled() + expect(toastErrorMock).not.toHaveBeenCalled() + }) + + it('closes main’s new tab when the user closed the source tab meanwhile', async () => { + const store = createStore() + const commitMove = vi.fn( + async (_request: TerminalLeafMoveRequest): Promise => { + // A tab close drops its row and its layout. + store.tabsByWorktree[WORKTREE_ID] = [] + delete store.terminalLayoutsByTabId[SOURCE_TAB_ID] + return moved + } + ) + + await expect(detach({ commitMove, store })).resolves.toBeNull() + + expect(closeTerminalSurface).toHaveBeenCalledOnce() + expect(store.createTab).not.toHaveBeenCalled() + }) + + it('finds the pane by its leaf when its pane id changed meanwhile', async () => { + const manager = managerWithPanes(() => [1, 7]) + manager.getLeafId.mockImplementation((paneId) => (paneId === 1 ? LEAF_1 : LEAF_2)) + const commitMove = vi.fn( + async (_request: TerminalLeafMoveRequest): Promise => { + manager.getLeafId.mockImplementation((paneId) => + paneId === 1 ? LEAF_1 : paneId === 7 ? LEAF_2 : null + ) + return moved + } + ) + + await expect(detach({ commitMove, manager })).resolves.toMatchObject({ leafId: LEAF_2 }) + + expect(manager.detachPaneForExternalMove).toHaveBeenCalledWith(7) + }) + + it('closes main’s new tab when the pane manager refuses to detach the pane', async () => { + const manager = managerWithPanes() + manager.detachPaneForExternalMove.mockReturnValue(false) + const commitMove = mainAnswering([moved]) + const store = createStore() + + await expect(detach({ commitMove, manager, store })).resolves.toBeNull() + + expect(closeTerminalSurface).toHaveBeenCalledWith({ + worktreeId: WORKTREE_ID, + target: { kind: 'tab', tabId: commitMove.mock.calls[0]?.[0]?.targetTabId }, + reason: 'cleanup' + }) + expect(store.createTab).not.toHaveBeenCalled() + expect(store.setTabLayout).not.toHaveBeenCalled() + }) + + it('moves the last pane, after a sibling closed meanwhile, without killing its PTY', async () => { + const store = createStore() + let paneIds = [1, 2] + const manager = managerWithPanes(() => paneIds) + const commitMove = vi.fn(async (_request: TerminalLeafMoveRequest) => { + // Closing a pane persists the layout synchronously (onLayoutChanged), as the real close does. + paneIds = [2] + store.terminalLayoutsByTabId[SOURCE_TAB_ID] = { + root: { type: 'leaf', leafId: LEAF_2 }, + activeLeafId: LEAF_2, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_2]: 'pty-right' } + } + return { status: 'moved', ptyId: 'pty-right' } satisfies TerminalLeafMoveResult + }) + + await expect(detach({ commitMove, manager, store })).resolves.toMatchObject({ + leafId: LEAF_2, + ptyId: 'pty-right' + }) + + expect(closeTerminalSurface).not.toHaveBeenCalled() + expect(manager.detachPaneForExternalMove).not.toHaveBeenCalled() + expect(store.createTab).toHaveBeenCalledOnce() + expect(store.syncPaneDetachPtyOwnership).toHaveBeenCalledWith( + expect.objectContaining({ detachedLeafId: LEAF_2, detachedPtyId: 'pty-right' }) + ) + expect(store.closeTab).toHaveBeenCalledWith( + SOURCE_TAB_ID, + expect.objectContaining({ localPtyTeardownOwnedExternally: true }) + ) + }) + + it('commits the move in main before the target tab exists, using the same tab id', async () => { + const store = createStore() + let release!: () => void + const commitMove = vi.fn( + (_request: TerminalLeafMoveRequest) => + new Promise((resolve) => { + release = () => resolve(moved) + }) + ) + + const detaching = detach({ commitMove, store }) + await vi.waitFor(() => expect(commitMove).toHaveBeenCalledOnce()) + expect(store.createTab).not.toHaveBeenCalled() + release() + await detaching + + const request = commitMove.mock.calls[0]?.[0] + expect(request).toEqual({ + worktreeId: WORKTREE_ID, + sourceTabId: SOURCE_TAB_ID, + targetTabId: expect.any(String), + leafId: LEAF_2, + ptyId: expect.any(String) + }) + expect(store.createTab).toHaveBeenCalledWith( + WORKTREE_ID, + TARGET_GROUP_ID, + 'powershell.exe', + expect.objectContaining({ id: request?.targetTabId, initialLeafId: LEAF_2 }) + ) + }) + + it('ignores a repeat drop of a pane whose move is still committing', async () => { + const store = createStore() + let release!: () => void + const commitMove = vi.fn( + (_request: TerminalLeafMoveRequest) => + new Promise((resolve) => { + release = () => resolve(moved) + }) + ) + + const first = detach({ commitMove, store }) + await vi.waitFor(() => expect(commitMove).toHaveBeenCalledOnce()) + await expect(detach({ commitMove, store })).resolves.toBeNull() + release() + + await expect(first).resolves.toMatchObject({ leafId: LEAF_2 }) + expect(commitMove).toHaveBeenCalledOnce() + expect(toastErrorMock).not.toHaveBeenCalled() + }) + + it('binds the moved tab’s layout to the live PTY, not the saved one', async () => { + const store = createStore(splitLayout()) + const saved = store.terminalLayoutsByTabId[SOURCE_TAB_ID]?.ptyIdsByLeafId?.[LEAF_2] + expect(saved).toBeTruthy() + + const result = await detach({ livePtyId: 'pty-respawned', store }) + + expect(result?.ptyId).toBe('pty-respawned') + expect(store.terminalLayoutsByTabId['tab-detached']?.ptyIdsByLeafId?.[LEAF_2]).toBe( + 'pty-respawned' + ) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach-fixture.ts b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach-fixture.ts new file mode 100644 index 00000000000..b7d69404a44 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach-fixture.ts @@ -0,0 +1,122 @@ +import { vi } from 'vitest' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TerminalPaneTabDetachStore } from './terminal-pane-tab-detach' + +export const WORKTREE_ID = 'repo-1::/worktree' +export const SOURCE_TAB_ID = 'tab-source' +export const TARGET_GROUP_ID = 'group-target' +export const EXISTING_TAB_1 = 'tab-existing-1' +export const EXISTING_TAB_2 = 'tab-existing-2' +export const LEAF_1 = '11111111-1111-4111-8111-111111111111' +export const LEAF_2 = '22222222-2222-4222-8222-222222222222' + +export function splitLayout(): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_1 }, + second: { type: 'leaf', leafId: LEAF_2 } + }, + activeLeafId: LEAF_2, + expandedLeafId: null, + ptyIdsByLeafId: { + [LEAF_1]: 'pty-left', + [LEAF_2]: 'remote:env-1@@terminal-1' + }, + buffersByLeafId: { + [LEAF_2]: 'remote-buffer' + }, + titlesByLeafId: { + [LEAF_2]: 'remote shell' + } + } +} + +export function unboundSplitLayout(): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_1 }, + second: { type: 'leaf', leafId: LEAF_2 } + }, + activeLeafId: LEAF_2, + expandedLeafId: null + } +} + +export function createTerminalTab( + id: string, + ptyId: string | null, + shellOverride?: string +): TerminalTab { + return { + id, + ptyId, + worktreeId: WORKTREE_ID, + title: 'Terminal 2', + defaultTitle: 'Terminal 2', + customTitle: null, + color: null, + sortOrder: 1, + createdAt: 1, + ...(shellOverride !== undefined ? { shellOverride } : {}) + } +} + +export function createStore( + layout: TerminalLayoutSnapshot = splitLayout(), + targetTabOrder: string[] = [EXISTING_TAB_1, EXISTING_TAB_2], + sourceShellOverride = 'powershell.exe' +): TerminalPaneTabDetachStore { + const store = { + closeTab: vi.fn(), + createTab: vi.fn((_worktreeId, _targetGroupId, _shellOverride, options) => { + const tab = createTerminalTab('tab-detached', options?.initialPtyId ?? null) + const group = store.groupsByWorktree[WORKTREE_ID]?.find( + (candidate) => candidate.id === TARGET_GROUP_ID + ) + if (group && !group.tabOrder.includes(tab.id)) { + group.tabOrder = [...group.tabOrder, tab.id] + } + return tab + }), + groupsByWorktree: { + [WORKTREE_ID]: [ + { + id: TARGET_GROUP_ID, + worktreeId: WORKTREE_ID, + activeTabId: targetTabOrder[0] ?? null, + tabOrder: targetTabOrder, + recentTabIds: [] + } + ] + }, + reorderUnifiedTabs: vi.fn((groupId: string, tabIds: string[]) => { + const group = store.groupsByWorktree[WORKTREE_ID]?.find( + (candidate) => candidate.id === groupId + ) + if (group) { + group.tabOrder = tabIds + } + }), + setActiveTab: vi.fn(), + setActiveTabType: vi.fn(), + setTabLayout: vi.fn((tabId: string, nextLayout: TerminalLayoutSnapshot | null) => { + if (nextLayout) { + store.terminalLayoutsByTabId[tabId] = nextLayout + } else { + delete store.terminalLayoutsByTabId[tabId] + } + }), + syncPaneDetachPtyOwnership: vi.fn(), + tabsByWorktree: { + [WORKTREE_ID]: [createTerminalTab(SOURCE_TAB_ID, 'pty-left', sourceShellOverride)] + }, + terminalLayoutsByTabId: { + [SOURCE_TAB_ID]: layout + } + } + return store as unknown as TerminalPaneTabDetachStore +} diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts index 8586db21e41..75ef69692e8 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.test.ts @@ -1,19 +1,31 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AppState } from '@/store' -import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types' import { detachTerminalPaneToTab, - resolveTerminalTabStripDropTarget, - type TerminalPaneTabDetachStore + resolveTerminalTabStripDropTarget } from './terminal-pane-tab-detach' +import { + createStore, + EXISTING_TAB_1, + EXISTING_TAB_2, + LEAF_1, + LEAF_2, + SOURCE_TAB_ID, + splitLayout, + TARGET_GROUP_ID, + unboundSplitLayout, + WORKTREE_ID +} from './terminal-pane-tab-detach-fixture' -const WORKTREE_ID = 'repo-1::/worktree' -const SOURCE_TAB_ID = 'tab-source' -const TARGET_GROUP_ID = 'group-target' -const EXISTING_TAB_1 = 'tab-existing-1' -const EXISTING_TAB_2 = 'tab-existing-2' -const LEAF_1 = '11111111-1111-4111-8111-111111111111' -const LEAF_2 = '22222222-2222-4222-8222-222222222222' +const toastErrorMock = vi.hoisted(() => vi.fn()) +vi.mock('sonner', () => ({ toast: { error: toastErrorMock } })) +beforeEach(() => { + toastErrorMock.mockClear() + // No local main holds these sessions, so each move stays in the renderer. + vi.stubGlobal('window', { + api: { pty: { moveLeafToNewTab: () => Promise.resolve({ status: 'not_held' }) } } + }) +}) function rect(args: { left: number; top: number; width: number; height: number }): DOMRect { return { @@ -26,115 +38,9 @@ function rect(args: { left: number; top: number; width: number; height: number } } as DOMRect } -function splitLayout(): TerminalLayoutSnapshot { - return { - root: { - type: 'split', - direction: 'vertical', - first: { type: 'leaf', leafId: LEAF_1 }, - second: { type: 'leaf', leafId: LEAF_2 } - }, - activeLeafId: LEAF_2, - expandedLeafId: null, - ptyIdsByLeafId: { - [LEAF_1]: 'pty-left', - [LEAF_2]: 'remote:env-1@@terminal-1' - }, - buffersByLeafId: { - [LEAF_2]: 'remote-buffer' - }, - titlesByLeafId: { - [LEAF_2]: 'remote shell' - } - } -} - -function unboundSplitLayout(): TerminalLayoutSnapshot { - return { - root: { - type: 'split', - direction: 'vertical', - first: { type: 'leaf', leafId: LEAF_1 }, - second: { type: 'leaf', leafId: LEAF_2 } - }, - activeLeafId: LEAF_2, - expandedLeafId: null - } -} - -function createTerminalTab(id: string, ptyId: string | null, shellOverride?: string): TerminalTab { - return { - id, - ptyId, - worktreeId: WORKTREE_ID, - title: 'Terminal 2', - defaultTitle: 'Terminal 2', - customTitle: null, - color: null, - sortOrder: 1, - createdAt: 1, - ...(shellOverride !== undefined ? { shellOverride } : {}) - } -} - -function createStore( - layout: TerminalLayoutSnapshot = splitLayout(), - targetTabOrder: string[] = [EXISTING_TAB_1, EXISTING_TAB_2], - sourceShellOverride = 'powershell.exe' -): TerminalPaneTabDetachStore { - const store = { - createTab: vi.fn((_worktreeId, _targetGroupId, _shellOverride, options) => { - const tab = createTerminalTab('tab-detached', options?.initialPtyId ?? null) - const group = store.groupsByWorktree[WORKTREE_ID]?.find( - (candidate) => candidate.id === TARGET_GROUP_ID - ) - if (group && !group.tabOrder.includes(tab.id)) { - group.tabOrder = [...group.tabOrder, tab.id] - } - return tab - }), - groupsByWorktree: { - [WORKTREE_ID]: [ - { - id: TARGET_GROUP_ID, - worktreeId: WORKTREE_ID, - activeTabId: targetTabOrder[0] ?? null, - tabOrder: targetTabOrder, - recentTabIds: [] - } - ] - }, - reorderUnifiedTabs: vi.fn((groupId: string, tabIds: string[]) => { - const group = store.groupsByWorktree[WORKTREE_ID]?.find( - (candidate) => candidate.id === groupId - ) - if (group) { - group.tabOrder = tabIds - } - }), - setActiveTab: vi.fn(), - setActiveTabType: vi.fn(), - setTabLayout: vi.fn((tabId: string, nextLayout: TerminalLayoutSnapshot | null) => { - if (nextLayout) { - store.terminalLayoutsByTabId[tabId] = nextLayout - } else { - delete store.terminalLayoutsByTabId[tabId] - } - }), - syncPaneDetachPtyOwnership: vi.fn(), - tabsByWorktree: { - [WORKTREE_ID]: [createTerminalTab(SOURCE_TAB_ID, 'pty-left', sourceShellOverride)] - }, - terminalLayoutsByTabId: { - [SOURCE_TAB_ID]: layout - } - } - return store as unknown as TerminalPaneTabDetachStore -} - type SourcePaneCwd = NonNullable[0]['sourcePaneCwd']> -function expectDeferredSplitDetachRejected(sourcePaneCwd: SourcePaneCwd): void { +async function expectDeferredSplitDetachRejected(sourcePaneCwd: SourcePaneCwd): Promise { const store = createStore(unboundSplitLayout()) const manager = { getPanes: vi.fn(() => [{ id: 1 }, { id: 2 }]), @@ -143,7 +49,7 @@ function expectDeferredSplitDetachRejected(sourcePaneCwd: SourcePaneCwd): void { } const persistLayoutSnapshot = vi.fn() - const result = detachTerminalPaneToTab({ + const result = await detachTerminalPaneToTab({ getStore: () => store, manager, persistLayoutSnapshot, @@ -271,9 +177,9 @@ describe('resolveTerminalTabStripDropTarget', () => { }) describe('detachTerminalPaneToTab', () => { - it.each([LEAF_1, LEAF_2])('moves chat mode only with its owning leaf %s', (chatLeafId) => { + it.each([LEAF_1, LEAF_2])('moves chat mode only with its owning leaf %s', async (chatLeafId) => { const store = createStore({ ...splitLayout(), chatLeafId }) - detachTerminalPaneToTab({ + await detachTerminalPaneToTab({ getStore: () => store, manager: { getPanes: () => [{ id: 1 }, { id: 2 }], @@ -296,7 +202,7 @@ describe('detachTerminalPaneToTab', () => { ) }) - it('creates a new terminal tab with the detached leaf layout and PTY id', () => { + it('creates a new terminal tab with the detached leaf layout and PTY id', async () => { const store = createStore() const manager = { getPanes: vi.fn(() => [{ id: 1 }, { id: 2 }]), @@ -305,7 +211,7 @@ describe('detachTerminalPaneToTab', () => { } const persistLayoutSnapshot = vi.fn() - const result = detachTerminalPaneToTab({ + const result = await detachTerminalPaneToTab({ manager, getStore: () => store, persistLayoutSnapshot, @@ -324,8 +230,10 @@ describe('detachTerminalPaneToTab', () => { expect(result?.ptyId).toBe('remote:env-1@@terminal-1') expect(manager.detachPaneForExternalMove).toHaveBeenCalledWith(2) expect(store.createTab).toHaveBeenCalledWith(WORKTREE_ID, TARGET_GROUP_ID, 'powershell.exe', { + id: expect.any(String), activate: true, initialPtyId: 'remote:env-1@@terminal-1', + initialLeafId: LEAF_2, recordInteraction: true }) expect(store.setTabLayout).toHaveBeenCalledWith(SOURCE_TAB_ID, { @@ -361,7 +269,7 @@ describe('detachTerminalPaneToTab', () => { it.each(['powershell.exe', 'wsl.exe'])( 'preserves the moved PTY shell override when the source uses %s', - (shellOverride) => { + async (shellOverride) => { const store = createStore(splitLayout(), [EXISTING_TAB_1], shellOverride) const manager = { getPanes: vi.fn(() => [{ id: 1 }, { id: 2 }]), @@ -369,7 +277,7 @@ describe('detachTerminalPaneToTab', () => { detachPaneForExternalMove: vi.fn(() => true) } - detachTerminalPaneToTab({ + await detachTerminalPaneToTab({ getStore: () => store, manager, persistLayoutSnapshot: vi.fn(), @@ -388,7 +296,7 @@ describe('detachTerminalPaneToTab', () => { } ) - it('syncs PTY ownership when the primary source pane is detached', () => { + it('syncs PTY ownership when the primary source pane is detached', async () => { const store = createStore() const manager = { getPanes: vi.fn(() => [{ id: 1 }, { id: 2 }]), @@ -396,7 +304,7 @@ describe('detachTerminalPaneToTab', () => { detachPaneForExternalMove: vi.fn(() => true) } - const result = detachTerminalPaneToTab({ + const result = await detachTerminalPaneToTab({ getStore: () => store, manager, persistLayoutSnapshot: vi.fn(), @@ -431,7 +339,7 @@ describe('detachTerminalPaneToTab', () => { }) }) - it('moves the detached tab into the requested group slot', () => { + it('moves the detached tab into the requested group slot', async () => { const store = createStore(splitLayout(), [EXISTING_TAB_1, EXISTING_TAB_2]) const manager = { getPanes: vi.fn(() => [{ id: 1 }, { id: 2 }]), @@ -439,7 +347,7 @@ describe('detachTerminalPaneToTab', () => { detachPaneForExternalMove: vi.fn(() => true) } - detachTerminalPaneToTab({ + await detachTerminalPaneToTab({ getStore: () => store, manager, persistLayoutSnapshot: vi.fn(), @@ -457,7 +365,7 @@ describe('detachTerminalPaneToTab', () => { ) }) - it('uses the live transport PTY id when the snapshot has not persisted it yet', () => { + it('uses the live transport PTY id when the snapshot has not persisted it yet', async () => { const store = createStore({ root: { type: 'split', @@ -474,8 +382,8 @@ describe('detachTerminalPaneToTab', () => { detachPaneForExternalMove: vi.fn(() => true) } - detachTerminalPaneToTab({ - fallbackPtyId: 'remote:env-2@@terminal-9', + await detachTerminalPaneToTab({ + livePtyId: 'remote:env-2@@terminal-9', getStore: () => store, manager, persistLayoutSnapshot: vi.fn(), @@ -499,30 +407,30 @@ describe('detachTerminalPaneToTab', () => { }) }) - it('rejects a deferred split while inherited cwd is pending', () => { - expectDeferredSplitDetachRejected({ + it('rejects a deferred split while inherited cwd is pending', async () => { + await expectDeferredSplitDetachRejected({ cwd: '/remote/repo', deferredSplitSpawn: true, pendingCwd: new Promise(() => {}) }) }) - it('rejects a pending cwd even when the deferred marker is absent', () => { - expectDeferredSplitDetachRejected({ + it('rejects a pending cwd even when the deferred marker is absent', async () => { + await expectDeferredSplitDetachRejected({ cwd: '/remote/repo', pendingCwd: new Promise(() => {}) }) }) - it('still rejects a deferred split after cwd resolves but before PTY bind', () => { - expectDeferredSplitDetachRejected({ + it('still rejects a deferred split after cwd resolves but before PTY bind', async () => { + await expectDeferredSplitDetachRejected({ cwd: '/remote/repo/packages/app', confirmed: false, deferredSplitSpawn: true }) }) - it('carries resolved cwd when detaching an unbound non-deferred pane', () => { + it('carries resolved cwd when detaching an unbound non-deferred pane', async () => { const store = createStore(unboundSplitLayout()) const manager = { getPanes: vi.fn(() => [{ id: 1 }, { id: 2 }]), @@ -530,7 +438,7 @@ describe('detachTerminalPaneToTab', () => { detachPaneForExternalMove: vi.fn(() => true) } - const result = detachTerminalPaneToTab({ + const result = await detachTerminalPaneToTab({ getStore: () => store, manager, persistLayoutSnapshot: vi.fn(), @@ -546,6 +454,7 @@ describe('detachTerminalPaneToTab', () => { expect(result?.ptyId).toBeNull() expect(store.createTab).toHaveBeenCalledWith(WORKTREE_ID, TARGET_GROUP_ID, 'powershell.exe', { + id: expect.any(String), activate: true, pendingActivationSpawn: true, recordInteraction: true, diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.ts b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.ts index 27c97538d2d..b71d435af53 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-tab-detach.ts @@ -1,5 +1,10 @@ +import { toast } from 'sonner' import type { AppState } from '@/store' +import { translate } from '@/i18n/i18n' +import { createBrowserUuid } from '@/lib/browser-uuid' import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TerminalLeafMoveRequest } from '../../../../shared/terminal-leaf-move' +import { commitTerminalSurfaceClose } from '@/store/terminals/terminal-surface-close-intent' import type { PaneCwdEntry } from './resolve-split-cwd' import { detachTerminalLayoutLeaf } from './terminal-layout-leaf-detach' export { @@ -10,6 +15,7 @@ export type { TerminalTabStripDropTarget } from './terminal-tab-strip-drop-targe export type TerminalPaneTabDetachStore = Pick< AppState, + | 'closeTab' | 'createTab' | 'groupsByWorktree' | 'reorderUnifiedTabs' @@ -36,23 +42,6 @@ export type DetachedTerminalPaneTab = { ptyId: string | null } -function withDetachedPtyFallback(args: { - leafId: string - ptyId: string | null - detachedLayout: NonNullable>['detachedLayout'] -}): NonNullable>['detachedLayout'] { - if (!args.ptyId || args.detachedLayout.ptyIdsByLeafId?.[args.leafId]) { - return args.detachedLayout - } - return { - ...args.detachedLayout, - ptyIdsByLeafId: { - ...args.detachedLayout.ptyIdsByLeafId, - [args.leafId]: args.ptyId - } - } -} - function moveCreatedTabToIndex(args: { groupId: string store: TerminalPaneTabDetachStore @@ -76,18 +65,53 @@ function moveCreatedTabToIndex(args: { args.store.reorderUnifiedTabs(args.groupId, nextOrder, { recordInteraction: false }) } -export function detachTerminalPaneToTab(args: { - fallbackPtyId?: string | null +function reportMoveFailed(): void { + toast.error(translate('terminal.paneMove.failed', "Couldn't move the pane to a new tab.")) +} + +type DetachTerminalPaneToTabArgs = { + /** The PTY the source pane's transport is attached to now; it outranks the saved layout. */ + livePtyId?: string | null getStore: () => TerminalPaneTabDetachStore manager: TerminalPaneTabDetachManager | null persistLayoutSnapshot: () => void sourcePaneId: number sourcePaneCwd?: SourcePaneCwd + /** The pane's transport is still awaiting its PTY id from a spawn or reattach. */ + sourceConnectPending?: boolean sourceTabId: string targetGroupId: string targetIndex?: number worktreeId: string -}): DetachedTerminalPaneTab | null { +} + +// Leaves whose move main is committing; a repeat drop meanwhile is a no-op. +const leavesMovingToNewTab = new Set() + +/** + * Moves a pane into a new tab. Main commits the move (leaf, binding and pane-keyed records) before + * the target tab exists here, so the moved pane's reattach never races a second owner (STA-9259). + * Once main has moved it, this window only rolls forward: it never asks main to put it back. + */ +export async function detachTerminalPaneToTab( + args: DetachTerminalPaneToTabArgs +): Promise { + const leafId = args.manager?.getLeafId(args.sourcePaneId) + if (!leafId || leavesMovingToNewTab.has(leafId)) { + return null + } + leavesMovingToNewTab.add(leafId) + try { + return await moveLeafToNewTab(args, leafId) + } finally { + leavesMovingToNewTab.delete(leafId) + } +} + +async function moveLeafToNewTab( + args: DetachTerminalPaneToTabArgs, + leafId: string +): Promise { const initialStore = args.getStore() const targetGroupExists = initialStore.groupsByWorktree[args.worktreeId]?.some( @@ -96,49 +120,83 @@ export function detachTerminalPaneToTab(args: { if (!args.manager || !targetGroupExists || args.manager.getPanes().length <= 1) { return null } - - const sourceLeafId = args.manager.getLeafId(args.sourcePaneId) - if (!sourceLeafId) { - return null - } - const persistedPtyId = - initialStore.terminalLayoutsByTabId[args.sourceTabId]?.ptyIdsByLeafId?.[sourceLeafId] + initialStore.terminalLayoutsByTabId[args.sourceTabId]?.ptyIdsByLeafId?.[leafId] const cwdDeferred = Boolean( args.sourcePaneCwd?.pendingCwd || args.sourcePaneCwd?.deferredSplitSpawn ) - if (cwdDeferred && !persistedPtyId && !args.fallbackPtyId) { + // Why: a spawn result landing after the move binds SOURCE:leaf, and that bind grafts the leaf + // back into the source tab beside its moved copy. + if ((cwdDeferred || args.sourceConnectPending) && !persistedPtyId && !args.livePtyId) { return null } args.persistLayoutSnapshot() - const store = args.getStore() - const detached = detachTerminalLayoutLeaf( - store.terminalLayoutsByTabId[args.sourceTabId], - sourceLeafId - ) - if (!detached) { - return null + const request: TerminalLeafMoveRequest = { + worktreeId: args.worktreeId, + sourceTabId: args.sourceTabId, + targetTabId: createBrowserUuid(), + leafId, + ptyId: args.livePtyId ?? persistedPtyId ?? null } - - const ptyId = detached.ptyId ?? args.fallbackPtyId ?? null - const detachedLayout = withDetachedPtyFallback({ - leafId: sourceLeafId, - ptyId, - detachedLayout: detached.detachedLayout + const answer = await window.api.pty.moveLeafToNewTab(request).catch((error: unknown) => { + console.warn('[terminal-pane-detach] main did not answer the move', error) + return null }) + if (answer?.status === 'moved') { + return applyMove(args, request, true, answer.ptyId) + } + if (answer?.status === 'not_held') { + return applyMove(args, request, false, request.ptyId) + } + // A failed write rolls main back; one whose outcome is unknown faults persistence, and the next + // load converges on whichever side main kept. + console.warn('[terminal-pane-detach] main did not move the pane', answer) + reportMoveFailed() + return null +} - // Why: remove the renderer pane only after the layout/PTY handoff has been - // computed; the close callback detaches listeners but must not kill the PTY. - if (!args.manager.detachPaneForExternalMove(args.sourcePaneId)) { +/** Applies the move here; the pane is found by its leaf, since its pane id may have changed. */ +function applyMove( + args: DetachTerminalPaneToTabArgs, + request: TerminalLeafMoveRequest, + mainMoved: boolean, + ptyId: string | null +): DetachedTerminalPaneTab | null { + const { leafId, sourceTabId, targetTabId, worktreeId } = request + const { manager } = args + const panes = manager?.getPanes() ?? [] + const paneId = panes.find((pane) => manager?.getLeafId(pane.id) === leafId)?.id + const sourceLayout = args.getStore().terminalLayoutsByTabId[sourceTabId] + // A sibling closed meanwhile, so the pane takes the whole source layout and the source tab closes. + const lastPane = panes.length === 1 + const split = lastPane ? null : detachTerminalLayoutLeaf(sourceLayout, leafId) + const movedLayout = lastPane ? sourceLayout : split?.detachedLayout + // Why: remove the renderer pane only after the layout handoff is computed; the close callback + // detaches listeners but must not kill the PTY. + if ( + !manager || + paneId === undefined || + !movedLayout || + (!lastPane && !manager.detachPaneForExternalMove(paneId)) + ) { + // Why: the pane or its tab is gone here (or would not detach), so the tab main moved it into + // holds nothing; close it the way any tab close reaches main. + if (mainMoved) { + commitTerminalSurfaceClose(worktreeId, { kind: 'tab', tabId: targetTabId }, 'cleanup') + } return null } + const detachedLayout = ptyId + ? { ...movedLayout, ptyIdsByLeafId: { ...movedLayout.ptyIdsByLeafId, [leafId]: ptyId } } + : movedLayout const latestStore = args.getStore() - const sourceShellOverride = latestStore.tabsByWorktree[args.worktreeId]?.find( - (candidate) => candidate.id === args.sourceTabId + const sourceShellOverride = latestStore.tabsByWorktree[worktreeId]?.find( + (candidate) => candidate.id === sourceTabId )?.shellOverride - const tab = latestStore.createTab(args.worktreeId, args.targetGroupId, sourceShellOverride, { + const tab = latestStore.createTab(worktreeId, args.targetGroupId, sourceShellOverride, { + id: targetTabId, activate: true, ...(detachedLayout.chatLeafId ? { viewMode: 'chat' as const } : {}), initialPtyId: ptyId ?? undefined, @@ -147,7 +205,7 @@ export function detachTerminalPaneToTab(args: { pendingActivationSpawn: true, ...(args.sourcePaneCwd?.cwd ? { startupCwd: args.sourcePaneCwd.cwd } : {}) } - : {}), + : { initialLeafId: leafId }), recordInteraction: true }) const afterCreateStore = args.getStore() @@ -156,19 +214,30 @@ export function detachTerminalPaneToTab(args: { store: afterCreateStore, tabId: tab.id, targetIndex: args.targetIndex, - worktreeId: args.worktreeId + worktreeId }) - afterCreateStore.setTabLayout(args.sourceTabId, detached.sourceLayout) + if (split) { + afterCreateStore.setTabLayout(sourceTabId, split.sourceLayout) + } afterCreateStore.setTabLayout(tab.id, detachedLayout) afterCreateStore.syncPaneDetachPtyOwnership({ - detachedLeafId: sourceLeafId, + detachedLeafId: leafId, detachedPtyId: ptyId, - sourceLayout: detached.sourceLayout, - sourceTabId: args.sourceTabId, + sourceLayout: split?.sourceLayout ?? { root: null, activeLeafId: null, expandedLeafId: null }, + sourceTabId, targetTabId: tab.id }) + if (!split) { + // The new tab reattaches the PTY, so the source tab's close must not kill it. + afterCreateStore.closeTab(sourceTabId, { + reason: 'cleanup', + recordInteraction: false, + captureRecentlyClosed: false, + localPtyTeardownOwnedExternally: true + }) + } afterCreateStore.setActiveTab(tab.id) - afterCreateStore.setActiveTabType('terminal', args.worktreeId) + afterCreateStore.setActiveTabType('terminal', worktreeId) - return { tab, leafId: sourceLeafId, ptyId } + return { tab, leafId, ptyId } } 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 308711892ed..efad698fca4 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 @@ -239,22 +239,23 @@ export function useTerminalPaneCloseActions(controller: TerminalPaneBindingContr if (!isTerminalTabStripDropTarget(target)) { return false } - const fallbackPtyId = paneTransportsRef.current.get(sourcePaneId)?.getPtyId() ?? null + const sourceTransport = paneTransportsRef.current.get(sourcePaneId) + const livePtyId = sourceTransport?.getPtyId() ?? null const sourcePaneCwd = paneCwdRef.current.get(sourcePaneId) - return ( - detachTerminalPaneToTab({ - fallbackPtyId, - getStore: useAppStore.getState, - manager: managerRef.current, - persistLayoutSnapshot, - sourcePaneId, - ...(sourcePaneCwd ? { sourcePaneCwd } : {}), - sourceTabId: tabId, - targetGroupId: target.groupId, - targetIndex: target.insertionIndex, - worktreeId - }) !== null - ) + void detachTerminalPaneToTab({ + livePtyId, + getStore: useAppStore.getState, + manager: managerRef.current, + persistLayoutSnapshot, + sourcePaneId, + ...(sourcePaneCwd ? { sourcePaneCwd } : {}), + sourceConnectPending: sourceTransport?.isConnectPending?.() ?? false, + sourceTabId: tabId, + targetGroupId: target.groupId, + targetIndex: target.insertionIndex, + worktreeId + }).catch((error) => console.warn('[terminal-pane-detach] move failed', error)) + return true }, // oxlint-disable-next-line react-hooks/exhaustive-deps -- Preserve the pre-split dependency contract. [persistLayoutSnapshot, tabId, worktreeId] diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a8320eb8c96..92322d7db16 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -19105,6 +19105,9 @@ "confirmStopDescription": "This closes any open Codex sessions that share it, including ones outside Orca.", "cancel": "Cancel", "runs": "Runs" + }, + "paneMove": { + "failed": "Couldn't move the pane to a new tab." } }, "fileExplorer": { diff --git a/src/renderer/src/web/preload-api/web-terminal-api.ts b/src/renderer/src/web/preload-api/web-terminal-api.ts index 55c0ec364eb..fcd16fab175 100644 --- a/src/renderer/src/web/preload-api/web-terminal-api.ts +++ b/src/renderer/src/web/preload-api/web-terminal-api.ts @@ -50,6 +50,8 @@ export function createPtyApi(): NonNullable['pty']> { getAuthoritativeBufferSnapshotCapabilities: (ids) => Promise.resolve(ids.map((id) => ({ id, authoritative: false }))), hasPty: () => Promise.resolve(null), + // Why: no local main owns a paired client's session, so the move stays in this window. + moveLeafToNewTab: () => Promise.resolve({ status: 'not_held' }), getMainBufferSnapshot: () => Promise.resolve(null), // Why: remote-runtime PTYs skip local main (no side-effect source); renderer byte parsing stays authoritative. onSideEffect: () => noopUnsubscribe, diff --git a/src/shared/terminal-leaf-move.test.ts b/src/shared/terminal-leaf-move.test.ts new file mode 100644 index 00000000000..1319a70c6f9 --- /dev/null +++ b/src/shared/terminal-leaf-move.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' +import { isTerminalLeafMoveRequest, terminalLeafMovePaneKeys } from './terminal-leaf-move' + +const LEAF = '22222222-2222-4222-8222-222222222222' +const valid = { + worktreeId: 'repo-1::/tmp/wt', + sourceTabId: 'tab-source', + targetTabId: 'tab-target', + leafId: LEAF, + ptyId: null +} + +describe('isTerminalLeafMoveRequest', () => { + it('accepts a move between two tabs with a stable leaf id', () => { + expect(isTerminalLeafMoveRequest(valid)).toBe(true) + expect(isTerminalLeafMoveRequest({ ...valid, ptyId: 'pty-1' })).toBe(true) + }) + + it.each([ + ['the same tab', { targetTabId: 'tab-source' }], + ['a legacy numeric leaf id', { leafId: '2' }], + ['a source tab id that would split a pane key', { sourceTabId: 'tab:source' }], + ['a target tab id that would split a pane key', { targetTabId: 'tab:target' }], + ['an empty PTY id', { ptyId: '' }] + ])('rejects %s', (_name, patch) => { + expect(isTerminalLeafMoveRequest({ ...valid, ...patch })).toBe(false) + }) + + it('builds both pane keys of a valid move', () => { + expect(terminalLeafMovePaneKeys(valid)).toEqual({ + from: `tab-source:${LEAF}`, + to: `tab-target:${LEAF}` + }) + }) +}) diff --git a/src/shared/terminal-leaf-move.ts b/src/shared/terminal-leaf-move.ts new file mode 100644 index 00000000000..ddf3a823619 --- /dev/null +++ b/src/shared/terminal-leaf-move.ts @@ -0,0 +1,59 @@ +import { makePaneKey, type PaneKey } from './stable-pane-id' + +/** Detach one pane into a new tab; the leaf id and its terminal move with it. */ +export type TerminalLeafMoveRequest = { + worktreeId: string + sourceTabId: string + targetTabId: string + leafId: string + /** The PTY the renderer's pane is attached to now, when it has one. */ + ptyId: string | null +} + +export type TerminalLeafMoveResult = + /** Main moved the leaf and every pane-keyed record into the target tab in one durable write. */ + | { status: 'moved'; ptyId: string | null } + /** Main's session never held the leaf, so there is nothing to move there. */ + | { status: 'not_held' } + | { + status: 'refused' + reason: 'invalid_request' | 'target_tab_exists' | 'pty_mismatch' | 'leaf_in_other_tab' + } + +/** The moved pane's key before and after; only valid requests reach here. */ +export function terminalLeafMovePaneKeys(request: TerminalLeafMoveRequest): { + from: PaneKey + to: PaneKey +} { + return { + from: makePaneKey(request.sourceTabId, request.leafId), + to: makePaneKey(request.targetTabId, request.leafId) + } +} + +/** The one validation of a move request: shape, distinct tabs, and ids a pane key can carry. */ +export function isTerminalLeafMoveRequest(value: unknown): value is TerminalLeafMoveRequest { + if (!value || typeof value !== 'object') { + return false + } + const candidate: Record = { ...value } + const isId = (field: unknown): field is string => + typeof field === 'string' && field.length > 0 && field.length <= 512 + if ( + !isId(candidate.worktreeId) || + !isId(candidate.sourceTabId) || + !isId(candidate.targetTabId) || + !isId(candidate.leafId) || + !(candidate.ptyId === null || isId(candidate.ptyId)) || + candidate.sourceTabId === candidate.targetTabId + ) { + return false + } + try { + makePaneKey(candidate.sourceTabId, candidate.leafId) + makePaneKey(candidate.targetTabId, candidate.leafId) + return true + } catch { + return false + } +}