From 00a00ad233f2272fa73112d92d20941d6121e548 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 1 Aug 2026 03:26:54 -0700 Subject: [PATCH] fix(mobile): preserve PTY cleanup ownership --- src/main/ipc/pty.test.ts | 19 +- src/main/ipc/pty.ts | 21 +- ...obile-folder-session-mutation-race.test.ts | 199 +++++++++--------- ...bile-session-pty-inventory-fencing.test.ts | 142 +++++++++++++ src/main/runtime/orca-runtime.ts | 26 ++- src/preload/api-types.ts | 2 + src/preload/index.ts | 2 + .../terminal-pane/pty-transport-types.ts | 2 + .../terminal-pane/pty-transport.test.ts | 70 +++++- .../components/terminal-pane/pty-transport.ts | 8 +- src/shared/pty-spawn-disposition.ts | 1 + 11 files changed, 367 insertions(+), 125 deletions(-) create mode 100644 src/shared/pty-spawn-disposition.ts diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 260cca1c830..4f38371f702 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -8229,8 +8229,16 @@ describe('registerPtyHandlers', () => { ) resolveSpawn({ id: 'pty-shared', isReattach: true }) const [runtimeResult, rendererResult] = await Promise.all([runtimeSpawn, rendererSpawn]) - expect(runtimeResult).toEqual({ id: 'pty-shared' }) - expect(rendererResult).toEqual({ id: 'pty-shared' }) + expect(runtimeResult).toEqual({ + id: 'pty-shared', + isReattach: true, + spawnDisposition: 'reattached' + }) + expect(rendererResult).toEqual({ + id: 'pty-shared', + isReattach: true, + spawnDisposition: 'awaited' + }) expect(providerSpawn).toHaveBeenCalledTimes(1) expect(store.persistPtyBinding).toHaveBeenCalledWith({ worktreeId: 'repo-1::/tmp', @@ -8342,8 +8350,8 @@ describe('registerPtyHandlers', () => { expect(providerSpawn).toHaveBeenCalledTimes(1) resolveSpawn({ id: 'pty-renderer' }) await expect(Promise.all([rendererSpawn, runtimeSpawn])).resolves.toEqual([ - { id: 'pty-renderer' }, - { id: 'pty-renderer' } + { id: 'pty-renderer', spawnDisposition: 'created' }, + { id: 'pty-renderer', spawnDisposition: 'awaited' } ]) expect(providerSpawn).toHaveBeenCalledTimes(1) expect(store.persistPtyBinding).toHaveBeenCalledWith({ @@ -14782,7 +14790,8 @@ describe('registerPtyHandlers', () => { expect(result).toEqual({ id: expect.any(String), pid: 12345, - incarnationId: expect.any(String) + incarnationId: expect.any(String), + spawnDisposition: 'created' }) expect(spawnMock).toHaveBeenCalledTimes(1) expect(spawnMock).toHaveBeenCalledWith( diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 9c01b47729d..964b199b39f 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -217,6 +217,7 @@ import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' import { resolveLocalProjectRuntimeForWorktreeId } from '../local-project-runtime-resolution' import { isPtyIncarnationId } from '../../shared/pty-incarnation' import type { PtyListedSession } from '../../shared/pty-listed-session' +import type { PtySpawnDisposition } from '../../shared/pty-spawn-disposition' // ─── Provider Registry ────────────────────────────────────────────── // Routes PTY operations by connectionId (null = local provider). @@ -311,6 +312,7 @@ type PaneSpawnReservation = { } type PaneSpawnReservationResult = { id: string + spawnDisposition: PtySpawnDisposition launchConfig?: SleepingAgentLaunchConfig } & Partial // Why: mobile materialization and a newly-focused pane can race to spawn the same leaf; key by paneKey so the loser adopts the winner's PTY. @@ -509,6 +511,12 @@ function resolvePaneSpawnReservation( return response } +async function awaitPaneSpawnReservation( + reservation: PaneSpawnReservation +): Promise { + return { ...(await reservation.promise), spawnDisposition: 'awaited' } +} + function settlePendingPaneSerializer(paneKey: string, gen: number): boolean { if (pendingByPaneKey.get(paneKey)?.gen !== gen) { return false @@ -4107,7 +4115,7 @@ export function registerPtyHandlers( ? paneSpawnReservationsByPaneKey.get(materializedPaneKey) : undefined if (existingPaneSpawn) { - return await existingPaneSpawn.promise + return await awaitPaneSpawnReservation(existingPaneSpawn) } const finishTerminalInstall = beginPtySpawnForWorktree( args.worktreeId, @@ -4334,11 +4342,13 @@ export function registerPtyHandlers( leafId: owner.surface.leafId, ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}) }) - return { + return resolvePaneSpawnReservation(materializedPaneKey, paneSpawnReservation, { id: result.id, + isReattach: true, + spawnDisposition: 'reattached', ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}), agentSessionEnsure: result.agentSessionEnsure - } + }) } ptyOwnership.set(result.id, args.connectionId ?? null) if (result.incarnationId) { @@ -4495,6 +4505,8 @@ export function registerPtyHandlers( sendPtySpawnedToRenderer(result.id) const response = { id: result.id, + ...(result.isReattach ? { isReattach: true as const } : {}), + spawnDisposition: result.isReattach ? ('reattached' as const) : ('created' as const), ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}), ...(result.agentSessionEnsure ? { agentSessionEnsure: result.agentSessionEnsure } : {}) } @@ -5311,7 +5323,7 @@ export function registerPtyHandlers( ? paneSpawnReservationsByPaneKey.get(reservationPaneKey) : undefined if (existingPaneSpawn) { - return await existingPaneSpawn.promise + return await awaitPaneSpawnReservation(existingPaneSpawn) } const finishTerminalInstall = beginPtySpawnForWorktree( args.worktreeId, @@ -5707,6 +5719,7 @@ export function registerPtyHandlers( } const response = { ...result, + spawnDisposition: result.isReattach ? ('reattached' as const) : ('created' as const), ...(!result.isReattach && effectiveLaunchConfig ? { launchConfig: effectiveLaunchConfig } : {}), diff --git a/src/main/runtime/mobile-folder-session-mutation-race.test.ts b/src/main/runtime/mobile-folder-session-mutation-race.test.ts index 387e102c3d2..e7eb7591889 100644 --- a/src/main/runtime/mobile-folder-session-mutation-race.test.ts +++ b/src/main/runtime/mobile-folder-session-mutation-race.test.ts @@ -308,7 +308,7 @@ describe('mobile folder session mutation races', () => { }) const spawn = vi.fn(async () => { await pendingSpawn - return { id: ptyId } + return { id: ptyId, spawnDisposition: 'reattached' as const } }) const kill = vi.fn(() => true) runtime.setPtyController({ @@ -341,7 +341,7 @@ describe('mobile folder session mutation races', () => { }) }) expect(spawn).toHaveBeenCalledOnce() - expect(kill).toHaveBeenCalledWith(ptyId) + expect(kill).not.toHaveBeenCalled() expect(focusTerminal).not.toHaveBeenCalled() expect(published).not.toContainEqual( expect.objectContaining({ activeTabId: `${TAB_ID}::${LEAF_ID}` }) @@ -413,97 +413,108 @@ describe('mobile folder session mutation races', () => { } ) - it('kills a pending materialization when folder routing changes during spawn', async () => { - const folder: FolderWorkspace = { - id: 'materialize-race-folder', - projectGroupId: 'materialize-race-group', - name: 'Materialize race', - folderPath: '/workspace/materialize-race', - connectionId: null, - linkedTask: null, - comment: '', - isArchived: false, - isUnread: false, - isPinned: false, - sortOrder: 0, - lastActivityAt: 1, - createdAt: 1, - updatedAt: 1 - } - const group: ProjectGroup = { - id: folder.projectGroupId, - name: folder.projectGroupId, - parentPath: folder.folderPath, - connectionId: null, - parentGroupId: null, - createdFrom: 'manual', - tabOrder: 0, - isCollapsed: false, - color: null, - createdAt: 1, - updatedAt: 1 - } - const folderKey = `folder:${folder.id}` - const folders = [folder] - const runtime = createRuntime(folders, group) - const focusTerminal = vi.fn() - const published: RuntimeMobileSessionTabsResult[] = [] - runtime.setNotifier({ focusTerminal } as never) - runtime.onMobileSessionTabsChanged((snapshot) => published.push(snapshot)) - vi.spyOn( - runtime as unknown as { - resolveTerminalWorkspaceLaunchScope: () => Promise<{ - id: string - path: string - connectionId: null - repo: null - folderWorkspace: FolderWorkspace - }> - }, - 'resolveTerminalWorkspaceLaunchScope' - ).mockImplementation(async () => ({ - id: folderKey, - path: folder.folderPath, - connectionId: null, - repo: null, - folderWorkspace: folder - })) - let releaseSpawn!: (value: { id: string }) => void - const spawn = vi.fn( - () => - new Promise<{ id: string }>((resolve) => { - releaseSpawn = resolve - }) - ) - const kill = vi.fn(() => true) - runtime.setPtyController({ - write: () => true, - kill, - getForegroundProcess: async () => null, - listProcesses: async () => [], - spawn - }) - - const activations = Promise.allSettled([ - runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID), - runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) - ]) - await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) - folders.length = 0 - folders.push({ ...folder, folderPath: '/workspace/materialize-race-moved' }) - releaseSpawn({ id: 'stale-materialized-pty' }) - - const results = await activations - results.forEach((result) => { - expect(result.status === 'rejected' ? result.reason : null).toMatchObject({ - message: 'tab_not_found' + it.each([ + ['creator', 'created', true], + ['waiter', 'awaited', false], + ['reattach consumer', 'reattached', false] + ] as const)( + 'lets only the %s retire a stale materialization', + async (_consumer, spawnDisposition, shouldKill) => { + const folder: FolderWorkspace = { + id: 'materialize-race-folder', + projectGroupId: 'materialize-race-group', + name: 'Materialize race', + folderPath: '/workspace/materialize-race', + connectionId: null, + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 1, + createdAt: 1, + updatedAt: 1 + } + const group: ProjectGroup = { + id: folder.projectGroupId, + name: folder.projectGroupId, + parentPath: folder.folderPath, + connectionId: null, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + const folderKey = `folder:${folder.id}` + const folders = [folder] + const runtime = createRuntime(folders, group) + const focusTerminal = vi.fn() + const published: RuntimeMobileSessionTabsResult[] = [] + runtime.setNotifier({ focusTerminal } as never) + runtime.onMobileSessionTabsChanged((snapshot) => published.push(snapshot)) + vi.spyOn( + runtime as unknown as { + resolveTerminalWorkspaceLaunchScope: () => Promise<{ + id: string + path: string + connectionId: null + repo: null + folderWorkspace: FolderWorkspace + }> + }, + 'resolveTerminalWorkspaceLaunchScope' + ).mockImplementation(async () => ({ + id: folderKey, + path: folder.folderPath, + connectionId: null, + repo: null, + folderWorkspace: folder + })) + let releaseSpawn!: (value: { id: string; spawnDisposition: typeof spawnDisposition }) => void + const spawn = vi.fn( + () => + new Promise<{ id: string; spawnDisposition: typeof spawnDisposition }>((resolve) => { + releaseSpawn = resolve + }) + ) + const kill = vi.fn(() => true) + runtime.setPtyController({ + write: () => true, + kill, + getForegroundProcess: async () => null, + listProcesses: async () => [], + spawn }) - }) - expect(spawn).toHaveBeenCalledOnce() - expect(kill).toHaveBeenCalledWith('stale-materialized-pty') - expect(focusTerminal).not.toHaveBeenCalled() - expect(published).not.toContainEqual( - expect.objectContaining({ activeTabId: `${TAB_ID}::${LEAF_ID}` }) - ) - }) + + const activations = Promise.allSettled([ + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID), + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) + ]) + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) + folders.length = 0 + folders.push({ ...folder, folderPath: '/workspace/materialize-race-moved' }) + releaseSpawn({ id: 'stale-materialized-pty', spawnDisposition }) + + const results = await activations + results.forEach((result) => { + expect(result.status === 'rejected' ? result.reason : null).toMatchObject({ + message: 'tab_not_found' + }) + }) + expect(spawn).toHaveBeenCalledOnce() + if (shouldKill) { + expect(kill).toHaveBeenCalledWith('stale-materialized-pty') + } else { + expect(kill).not.toHaveBeenCalled() + } + expect(focusTerminal).not.toHaveBeenCalled() + expect(published).not.toContainEqual( + expect.objectContaining({ activeTabId: `${TAB_ID}::${LEAF_ID}` }) + ) + } + ) }) diff --git a/src/main/runtime/mobile-session-pty-inventory-fencing.test.ts b/src/main/runtime/mobile-session-pty-inventory-fencing.test.ts index 96eb01969ee..36c20dd88c4 100644 --- a/src/main/runtime/mobile-session-pty-inventory-fencing.test.ts +++ b/src/main/runtime/mobile-session-pty-inventory-fencing.test.ts @@ -337,6 +337,148 @@ describe('mobile PTY inventory fencing', () => { }) }) + it('hydrates an attached explicit Git read from its scoped owner alias', async () => { + const worktreeId = `${REPO_ID}::/workspace/scoped-explicit` + const ownerKey = worktreeWorkspaceKey(worktreeId) + const tabId = 'scoped-explicit-tab' + const leafId = '12121212-1212-4212-8212-121212121212' + const session = makeSession({ + worktreeId: ownerKey, + ptyId: 'serve-scoped-explicit-pty', + tabId, + leafId + }) + const runtime = createRuntime({ + repos: [ + { + id: REPO_ID, + path: '/workspace/scoped-explicit', + displayName: 'Scoped explicit', + badgeColor: 'blue', + connectionId: null, + executionHostId: null, + addedAt: 1 + } + ], + sessions: new Map([['local', session]]) + }) + runtime.attachWindow(17) + + const snapshot = await runtime.listMobileSessionTabs(`id:${worktreeId}`) + + expect(snapshot).toMatchObject({ + worktree: worktreeId, + tabs: [{ parentTabId: tabId, leafId, ptyId: 'serve-scoped-explicit-pty' }] + }) + }) + + it('hydrates scoped serve and SSH tabs during isolated relay recovery', async () => { + const connectionId = 'scoped-owner-relay' + const worktreeId = `${REPO_ID}::/remote/scoped-owner` + const ownerKey = worktreeWorkspaceKey(worktreeId) + const sshTabId = 'scoped-relay-ssh-tab' + const sshLeafId = '23232323-2323-4232-8232-232323232323' + const serveTabId = 'scoped-relay-serve-tab' + const serveLeafId = '34343434-3434-4343-8343-343434343434' + const sshPtyId = `ssh:${connectionId}@@scoped-relay-pty` + const session = makeSession({ + worktreeId: ownerKey, + ptyId: sshPtyId, + tabId: sshTabId, + leafId: sshLeafId + }) + session.tabsByWorktree[ownerKey]!.push({ + id: serveTabId, + ptyId: 'serve-scoped-relay-pty', + worktreeId: ownerKey, + title: 'Scoped serve', + customTitle: null, + color: null, + sortOrder: 1, + createdAt: 1 + }) + session.terminalLayoutsByTabId[serveTabId] = { + root: { type: 'leaf', leafId: serveLeafId }, + activeLeafId: serveLeafId, + expandedLeafId: null, + ptyIdsByLeafId: { [serveLeafId]: 'serve-scoped-relay-pty' } + } + const unrelatedWorktreeId = `${REPO_ID}::/local/unrelated` + const unrelatedSession = makeSession({ + worktreeId: unrelatedWorktreeId, + ptyId: 'serve-unrelated-pty', + tabId: 'unrelated-tab', + leafId: '45454545-4545-4454-8454-454545454545' + }) + const runtime = createRuntime({ + repos: [ + { + id: REPO_ID, + path: '/local/unrelated', + displayName: 'Unrelated local', + badgeColor: 'blue', + connectionId: null, + executionHostId: null, + addedAt: 1 + }, + { + id: REPO_ID, + path: '/remote/scoped-owner', + displayName: 'Scoped relay', + badgeColor: 'blue', + connectionId, + executionHostId: null, + addedAt: 1 + } + ], + sessions: new Map([ + ['local', unrelatedSession], + [toSshExecutionHostId(connectionId), session] + ]) + }) + const listProcesses = vi.fn(async () => [ + { + id: sshPtyId, + cwd: '/remote/scoped-owner', + title: 'Scoped SSH', + worktreeId, + terminalHandle: 'term_scoped_relay', + incarnationId: INCARNATION_ID + } + ]) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses + }) + vi.spyOn( + runtime as unknown as { refreshRestoredOrchestrationAuthority: () => Promise }, + 'refreshRestoredOrchestrationAuthority' + ).mockResolvedValue() + vi.spyOn(runtime, 'reconcileLegacyWorkerTerminals').mockReturnValue( + new Promise(() => undefined) + ) + const events: Awaited>[] = [] + runtime.onMobileSessionTabsChanged((snapshot) => events.push(snapshot)) + + runtime.notifySshRelayReady(connectionId) + + await vi.waitFor(() => + expect( + events + .at(-1) + ?.tabs.filter((tab) => tab.type === 'terminal') + .map((tab) => tab.parentTabId) + ).toEqual(expect.arrayContaining([sshTabId, serveTabId])) + ) + expect(events.at(-1)?.worktree).toBe(worktreeId) + expect(events.flatMap((snapshot) => snapshot.tabs)).not.toContainEqual( + expect.objectContaining({ parentTabId: 'unrelated-tab' }) + ) + expect(listProcesses).toHaveBeenCalledWith(connectionId) + }) + it('fences PTY mutations when a folder path changes in place during inventory', async () => { const folder: FolderWorkspace = { id: 'moved-during-inventory', diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index af146e56ded..10b84847be9 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -378,6 +378,7 @@ import { type TerminalQuickCommandMutation } from '../../shared/terminal-quick-commands' import type { PtyIncarnationId } from '../../shared/pty-incarnation' +import type { PtySpawnDisposition } from '../../shared/pty-spawn-disposition' import { buildAgentDraftLaunchPlan, buildAgentResumeStartupPlan, @@ -1550,7 +1551,9 @@ type RuntimePtyController = { }): Promise<{ id: string incarnationId?: PtyIncarnationId - wslDistro?: string + wslDistro?: string | null + isReattach?: boolean + spawnDisposition?: PtySpawnDisposition agentSessionEnsure?: AgentSessionClaimedSpawnResult }> write(ptyId: string, data: string): boolean @@ -6418,6 +6421,9 @@ export class OrcaRuntimeService { // every worktree — skip the per-worktree rebuild entirely (hot on every // graph sync). Scoped to onlyRuntimeOwnedTerminals so full hydrates are // untouched. + const scopedOwnerKey = worktreeId + ? resolveWorkspaceSessionRecordOwnerKey(session.tabsByWorktree, worktreeId) + : null if ( options.onlyRuntimeOwnedTerminals === true && !this.offscreenBrowserBackend && @@ -6426,7 +6432,7 @@ export class OrcaRuntimeService { ? this.workspaceSessionWorktreeHasRuntimeOwnedPtyCandidate( session, worktreeId, - session.tabsByWorktree[worktreeId] ?? [] + scopedOwnerKey ? (session.tabsByWorktree[scopedOwnerKey] ?? []) : [] ) : this.workspaceSessionHasRuntimeOwnedPtyCandidate(session)) ) { @@ -6434,13 +6440,9 @@ export class OrcaRuntimeService { } const entries: readonly (readonly [string, string, readonly TerminalTab[]])[] = worktreeId !== undefined - ? (() => { - const ownerKey = resolveWorkspaceSessionRecordOwnerKey( - session.tabsByWorktree, - worktreeId - ) - return ownerKey ? [[worktreeId, ownerKey, session.tabsByWorktree[ownerKey] ?? []]] : [] - })() + ? scopedOwnerKey + ? [[worktreeId, scopedOwnerKey, session.tabsByWorktree[scopedOwnerKey] ?? []]] + : [] : Object.entries(session.tabsByWorktree ?? {}).flatMap(([ownerKey, tabs]) => { const normalizedOwnerKey = normalizeWorkspaceSessionOwnerKey(ownerKey) return resolveWorkspaceSessionRecordOwnerKey( @@ -25439,8 +25441,10 @@ export class OrcaRuntimeService { : {}) }) if (launchOpts.acceptWorkspaceInventory?.() === false) { - this.ptyController.kill(result.id) - this.releaseRejectedPtyRegistrationFence(result.id, result.incarnationId) + if (result.spawnDisposition === 'created') { + this.ptyController.kill(result.id) + this.releaseRejectedPtyRegistrationFence(result.id, result.incarnationId) + } throw new Error('tab_not_found') } reportPtySpawnCommitted() diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index cdb5000d6cc..6a852f76e45 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -8,6 +8,7 @@ import type { HostedReviewInfo, HostedReviewProvider } from '../shared/hosted-review' +import type { PtySpawnDisposition } from '../shared/pty-spawn-disposition' import type { NativeFileDropPayload } from '../shared/native-file-drop' import type { BrowserFindSource } from '../shared/browser-find-source' import type { DashboardSnapshot, DashboardRevealAgentArgs } from '../shared/dashboard-snapshot' @@ -1496,6 +1497,7 @@ export type PreloadApi = { telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }) => Promise<{ id: string + spawnDisposition: PtySpawnDisposition launchAgent?: TuiAgent launchConfig?: SleepingAgentLaunchConfig snapshot?: string diff --git a/src/preload/index.ts b/src/preload/index.ts index 9da862f485b..f38686d0776 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -23,6 +23,7 @@ import type { import type { MobileRelayStatus } from '../shared/mobile-relay-status' import type { MobilePairingConnectionMode } from '../shared/mobile-pairing-connection-mode' import type { MobileRelayMintFailure } from '../shared/mobile-relay-mint-failure' +import type { PtySpawnDisposition } from '../shared/pty-spawn-disposition' import type { VerifyAndAddRuntimeEnvironmentResult } from '../shared/remote-pairing-verification' import type { SshMutationExpectation, @@ -926,6 +927,7 @@ const api = { telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }): Promise<{ id: string + spawnDisposition: PtySpawnDisposition launchConfig?: SleepingAgentLaunchConfig snapshot?: string snapshotCols?: number 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 f56949a2c16..32029bf6e52 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -14,6 +14,7 @@ import type { TerminalOscColorQueryReplyColors } from '../../../../shared/termin import type { TuiAgent } from '../../../../shared/types' import type { ExecutionHostId } from '../../../../shared/execution-host' import type { PtyDataMeta } from './pty-dispatcher' +import type { PtySpawnDisposition } from '../../../../shared/pty-spawn-disposition' export type PtyBufferSnapshot = { data: string @@ -47,6 +48,7 @@ export type LocalPtySessionMetadata = { export type PtyConnectResult = { id: string + spawnDisposition?: PtySpawnDisposition /** The requested session exited while it had no primary pane handler. Its * buffered final data/exit were delivered, so callers must not fresh-spawn. */ exitedBeforeAttach?: boolean diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index ea7d9d043e1..3fbbfd1e4b2 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -322,7 +322,11 @@ describe('createIpcPtyTransport', () => { const onPtySpawn = vi.fn() const onDataCallback = vi.fn() const onExitCallback = vi.fn() - spawn.mockResolvedValueOnce({ id: 'pty-fresh-fallback', sessionExpired: true }) + spawn.mockResolvedValueOnce({ + id: 'pty-fresh-fallback', + sessionExpired: true, + spawnDisposition: 'created' + }) const transport = createIpcPtyTransport({ onPtySpawn }) const result = await transport.connect({ @@ -334,7 +338,11 @@ describe('createIpcPtyTransport', () => { onData?.({ id: 'pty-fresh-fallback', data: 'orphaned output' }) onExit?.({ id: 'pty-fresh-fallback', code: 0 }) - expect(result).toEqual({ id: 'pty-fresh-fallback', sessionExpired: true }) + expect(result).toEqual({ + id: 'pty-fresh-fallback', + sessionExpired: true, + spawnDisposition: 'created' + }) expect(kill).toHaveBeenCalledExactlyOnceWith('pty-fresh-fallback') expect(onPtySpawn).not.toHaveBeenCalled() expect(onDataCallback).not.toHaveBeenCalled() @@ -352,7 +360,11 @@ describe('createIpcPtyTransport', () => { const onExitCallback = vi.fn() const onErrorCallback = vi.fn() const retirementError = new Error('provider shutdown refused') - spawn.mockResolvedValueOnce({ id: 'pty-fresh-fallback', sessionExpired: true }) + spawn.mockResolvedValueOnce({ + id: 'pty-fresh-fallback', + sessionExpired: true, + spawnDisposition: 'created' + }) kill.mockRejectedValueOnce(retirementError) const transport = createIpcPtyTransport({ onPtySpawn }) @@ -1761,9 +1773,15 @@ describe('createIpcPtyTransport', () => { it('does not kill a pre-existing session when a reattach resolves after destroy', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawnControls: { - resolve: ((value: { id: string; isReattach: true }) => void) | null + resolve: + | ((value: { id: string; isReattach: true; spawnDisposition: 'reattached' }) => void) + | null } = { resolve: null } - const spawnPromise = new Promise<{ id: string; isReattach: true }>((resolve) => { + const spawnPromise = new Promise<{ + id: string + isReattach: true + spawnDisposition: 'reattached' + }>((resolve) => { spawnControls.resolve = resolve }) const spawnMock = vi.fn().mockReturnValue(spawnPromise) @@ -1798,18 +1816,48 @@ describe('createIpcPtyTransport', () => { if (!spawnControls.resolve) { throw new Error('Expected spawn resolver to be captured') } - spawnControls.resolve({ id: 'pty-preexisting', isReattach: true }) + spawnControls.resolve({ + id: 'pty-preexisting', + isReattach: true, + spawnDisposition: 'reattached' + }) await connectPromise expect(killMock).not.toHaveBeenCalledWith('pty-preexisting') }) + it('does not kill a shared fresh spawn when an awaited connect resolves after destroy', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + let resolveSpawn!: (value: { id: string; spawnDisposition: 'awaited' }) => void + const spawn = window.api.pty.spawn as unknown as ReturnType + const kill = window.api.pty.kill as unknown as ReturnType + spawn.mockReturnValueOnce( + new Promise((resolve) => { + resolveSpawn = resolve + }) + ) + const transport = createIpcPtyTransport({}) + const connect = transport.connect({ url: '', callbacks: {} }) + + transport.destroy?.() + resolveSpawn({ id: 'pty-shared-fresh', spawnDisposition: 'awaited' }) + await connect + + expect(kill).not.toHaveBeenCalledWith('pty-shared-fresh') + }) + it('kills a fresh session fallback that resolves after the transport was destroyed', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawnControls: { - resolve: ((value: { id: string; sessionExpired: true }) => void) | null + resolve: + | ((value: { id: string; sessionExpired: true; spawnDisposition: 'created' }) => void) + | null } = { resolve: null } - const spawnPromise = new Promise<{ id: string; sessionExpired: true }>((resolve) => { + const spawnPromise = new Promise<{ + id: string + sessionExpired: true + spawnDisposition: 'created' + }>((resolve) => { spawnControls.resolve = resolve }) const spawn = window.api.pty.spawn as unknown as ReturnType @@ -1826,7 +1874,11 @@ describe('createIpcPtyTransport', () => { if (!spawnControls.resolve) { throw new Error('Expected spawn resolver to be captured') } - spawnControls.resolve({ id: 'pty-fresh-fallback', sessionExpired: true }) + spawnControls.resolve({ + id: 'pty-fresh-fallback', + sessionExpired: true, + spawnDisposition: 'created' + }) await connectPromise expect(kill).toHaveBeenCalledExactlyOnceWith('pty-fresh-fallback') diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 6b777cbb21e..287885a07fd 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -804,8 +804,10 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra const resultLaunchAgent = isTuiAgent(spawnResult.launchAgent) ? spawnResult.launchAgent : undefined + const spawnDisposition = + spawnResult.spawnDisposition ?? (spawnResult.isReattach ? 'reattached' : 'created') const retireFreshSpawn = async (): Promise => { - if (!spawnResult.isReattach && !spawnResult.coldRestore) { + if (spawnDisposition === 'created' && !spawnResult.coldRestore) { await window.api.pty.kill(spawnResult.id) } } @@ -826,7 +828,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra connected = true // Why: skip onPtySpawn for reattach/coldRestore — it would reset lastActivityAt and destroy the recency sort order. - if (!spawnResult.isReattach && !spawnResult.coldRestore) { + if (spawnDisposition === 'created' && !spawnResult.coldRestore) { onPtySpawn?.(spawnResult.id) } @@ -845,6 +847,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra if (spawnResult.isReattach || spawnResult.coldRestore || spawnResult.sessionExpired) { return { id: spawnResult.id, + ...(spawnResult.spawnDisposition ? { spawnDisposition } : {}), // Why: recovery needs to distinguish an attach that ignored startup intent from a fresh spawn that ran it. ...(spawnResult.isReattach ? { isReattach: true } : {}), ...(resultLaunchAgent ? { launchAgent: resultLaunchAgent } : {}), @@ -870,6 +873,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra ) { return { id: spawnResult.id, + ...(spawnResult.spawnDisposition ? { spawnDisposition } : {}), ...(resultLaunchAgent ? { launchAgent: resultLaunchAgent } : {}), ...(spawnResult.launchConfig ? { launchConfig: spawnResult.launchConfig } : {}), ...(spawnResult.startupCwdFallback diff --git a/src/shared/pty-spawn-disposition.ts b/src/shared/pty-spawn-disposition.ts new file mode 100644 index 00000000000..1000ac99084 --- /dev/null +++ b/src/shared/pty-spawn-disposition.ts @@ -0,0 +1 @@ +export type PtySpawnDisposition = 'created' | 'reattached' | 'awaited'