diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 5a185591c30..260cca1c830 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -8122,7 +8122,7 @@ describe('registerPtyHandlers', () => { clearProviderPtyState('pty-claimed-admission') }) - it('reuses runtime materialization when renderer focuses the same pane during spawn', async () => { + it('reuses a restored runtime reattach when the paired renderer focuses the same pane', async () => { type RuntimeSpawnController = { spawn(args: { cols: number @@ -8132,13 +8132,14 @@ describe('registerPtyHandlers', () => { env?: Record tabId?: string leafId?: string + sessionId?: string persistHostSessionBinding?: boolean }): Promise<{ id: string }> } - let resolveSpawn!: (result: { id: string }) => void + let resolveSpawn!: (result: { id: string; isReattach?: boolean }) => void const providerSpawn = vi.fn( () => - new Promise<{ id: string }>((resolve) => { + new Promise<{ id: string; isReattach?: boolean }>((resolve) => { resolveSpawn = resolve }) ) @@ -8200,6 +8201,7 @@ describe('registerPtyHandlers', () => { worktreeId: 'repo-1::/tmp', tabId: 'tab-race', leafId, + sessionId: 'restored-paired-session', env: { ORCA_PANE_KEY: paneKey }, persistHostSessionBinding: true }) @@ -8213,6 +8215,7 @@ describe('registerPtyHandlers', () => { worktreeId: 'repo-1::/tmp', tabId: 'tab-race', leafId, + sessionId: 'restored-paired-session', env: { ORCA_TAB_ID: 'tab-race', ORCA_WORKTREE_ID: 'repo-1::/tmp' @@ -8221,11 +8224,13 @@ describe('registerPtyHandlers', () => { await Promise.resolve() expect(providerSpawn).toHaveBeenCalledTimes(1) - resolveSpawn({ id: 'pty-shared' }) - await expect(Promise.all([runtimeSpawn, rendererSpawn])).resolves.toEqual([ - { id: 'pty-shared' }, - { id: 'pty-shared' } - ]) + expect(providerSpawn).toHaveBeenCalledWith( + expect.objectContaining({ sessionId: 'restored-paired-session' }) + ) + 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(providerSpawn).toHaveBeenCalledTimes(1) expect(store.persistPtyBinding).toHaveBeenCalledWith({ worktreeId: 'repo-1::/tmp', 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 53d6f9b27b0..1d719e4828d 100644 --- a/src/main/runtime/mobile-folder-session-mutation-race.test.ts +++ b/src/main/runtime/mobile-folder-session-mutation-race.test.ts @@ -142,7 +142,12 @@ describe('mobile folder session mutation races', () => { folderWorkspace: folder }) let attached = false + let releaseAttach!: () => void + const attach = new Promise((resolve) => { + releaseAttach = resolve + }) const spawn = vi.fn(async () => { + await attach attached = true return { id: ptyId, incarnationId } }) @@ -172,21 +177,120 @@ describe('mobile folder session mutation races', () => { }) ).rejects.toThrow('terminal_not_writable') - const activated = await runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) - const activatedTab = activated.tabs[0] + const activations = Promise.all([ + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID), + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) + ]) + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) + releaseAttach() + const activated = await activations + const activatedTab = activated[0].tabs[0] + expect(activated[1].tabs[0]).toMatchObject({ status: 'ready' }) await expect( runtime.sendTerminal(activatedTab?.type === 'terminal' ? activatedTab.terminal! : 'missing', { text: 'folder input' }) ).resolves.toMatchObject({ accepted: true, bytesWritten: 12 }) - await runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) - expect(spawn).toHaveBeenCalledOnce() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ sessionId: ptyId, worktreeId: folderKey }) ) }) + it('fences shared SSH reattachment when the reconnect generation changes', async () => { + const connectionId = 'reattach-generation' + const ptyId = `ssh:${connectionId}@@restored-pty` + const folder: FolderWorkspace = { + id: 'reconnect-folder', + projectGroupId: 'reconnect-group', + name: 'Reconnect folder', + folderPath: '/workspace/reconnect-folder', + connectionId, + 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 runtime = createRuntime([folder], group, ptyId) + vi.spyOn( + runtime as unknown as { + resolveTerminalWorkspaceLaunchScope: () => Promise<{ + id: string + path: string + connectionId: string + repo: null + folderWorkspace: FolderWorkspace + }> + }, + 'resolveTerminalWorkspaceLaunchScope' + ).mockResolvedValue({ + id: folderKey, + path: folder.folderPath, + connectionId, + repo: null, + folderWorkspace: folder + }) + let releaseSpawn!: () => void + const pendingSpawn = new Promise((resolve) => { + releaseSpawn = resolve + }) + const spawn = vi.fn(async () => { + await pendingSpawn + return { id: ptyId } + }) + const kill = vi.fn(() => true) + runtime.setPtyController({ + spawn, + write: () => true, + kill, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + + const activations = Promise.allSettled([ + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID), + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) + ]) + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) + runtime.notifySshStateChanged(connectionId, { + targetId: connectionId, + status: 'disconnected', + error: null, + reconnectAttempt: 0 + }) + releaseSpawn() + + const results = await activations + expect(results).toHaveLength(2) + results.forEach((result) => { + expect(result).toMatchObject({ status: 'rejected' }) + expect(result.status === 'rejected' ? result.reason : null).toMatchObject({ + message: 'tab_not_found' + }) + }) + expect(spawn).toHaveBeenCalledOnce() + expect(kill).toHaveBeenCalledWith(ptyId) + }) + it.each(['activate', 'close'] as const)( 'rejects a folder tab %s when deletion races PTY inventory', async (operation) => { @@ -283,7 +387,8 @@ describe('mobile folder session mutation races', () => { updatedAt: 1 } const folderKey = `folder:${folder.id}` - const runtime = createRuntime([folder], group) + const folders = [folder] + const runtime = createRuntime(folders, group) vi.spyOn( runtime as unknown as { resolveTerminalWorkspaceLaunchScope: () => Promise<{ @@ -318,12 +423,22 @@ describe('mobile folder session mutation races', () => { spawn }) - const activation = runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) + const activations = Promise.allSettled([ + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID), + runtime.activateMobileSessionTab(`id:${folderKey}`, TAB_ID) + ]) await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) - folder.folderPath = '/workspace/materialize-race-moved' + folders.length = 0 + folders.push({ ...folder, folderPath: '/workspace/materialize-race-moved' }) releaseSpawn({ id: 'stale-materialized-pty' }) - await expect(activation).rejects.toThrow('tab_not_found') + const results = await activations + results.forEach((result) => { + expect(result.status === 'rejected' ? result.reason : null).toMatchObject({ + message: 'tab_not_found' + }) + }) + expect(spawn).toHaveBeenCalledOnce() expect(kill).toHaveBeenCalledWith('stale-materialized-pty') }) }) diff --git a/src/main/runtime/mobile-terminal-materialization-key.test.ts b/src/main/runtime/mobile-terminal-materialization-key.test.ts new file mode 100644 index 00000000000..81e913015b6 --- /dev/null +++ b/src/main/runtime/mobile-terminal-materialization-key.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' +import { toRuntimeExecutionHostId, toSshExecutionHostId } from '../../shared/execution-host' +import { mobileTerminalMaterializationKey } from './mobile-terminal-materialization-key' + +const BASE_IDENTITY = { + executionHostId: toSshExecutionHostId('host-a'), + connectionId: 'host-a', + worktreeId: 'repo::/worktree', + parentTabId: 'tab-a', + leafId: 'leaf-a', + sessionId: 'ssh:host-a@@session-a' +} + +describe('mobile terminal materialization key', () => { + it.each([ + ['execution host', { executionHostId: toRuntimeExecutionHostId('host-b') }], + ['connection', { connectionId: 'host-b' }], + ['worktree', { worktreeId: 'repo::/other-worktree' }], + ['parent tab', { parentTabId: 'tab-b' }], + ['leaf', { leafId: 'leaf-b' }], + ['persisted session', { sessionId: 'ssh:host-a@@session-b' }] + ])('does not coalesce a different %s', (_label, difference) => { + expect(mobileTerminalMaterializationKey({ ...BASE_IDENTITY, ...difference })).not.toBe( + mobileTerminalMaterializationKey(BASE_IDENTITY) + ) + }) +}) diff --git a/src/main/runtime/mobile-terminal-materialization-key.ts b/src/main/runtime/mobile-terminal-materialization-key.ts new file mode 100644 index 00000000000..e6b2e613492 --- /dev/null +++ b/src/main/runtime/mobile-terminal-materialization-key.ts @@ -0,0 +1,19 @@ +import type { ExecutionHostId } from '../../shared/execution-host' + +export function mobileTerminalMaterializationKey(identity: { + executionHostId: ExecutionHostId + connectionId: string | null + worktreeId: string + parentTabId: string + leafId: string + sessionId?: string +}): string { + return JSON.stringify([ + identity.executionHostId, + identity.connectionId, + identity.worktreeId, + identity.parentTabId, + identity.leafId, + identity.sessionId ?? null + ]) +} diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index d4aa86c0152..a76d144a569 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -28287,7 +28287,12 @@ describe('OrcaRuntimeService', () => { }) ) let attached = false + let releaseAttach!: () => void + const attach = new Promise((resolve) => { + releaseAttach = resolve + }) const spawn = vi.fn(async () => { + await attach attached = true return { id: ptyId, incarnationId } }) @@ -28319,15 +28324,20 @@ describe('OrcaRuntimeService', () => { }) ).rejects.toThrow('terminal_not_writable') - const activated = await runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') - const activatedTab = activated.tabs[0] + const activations = Promise.all([ + runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab'), + runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') + ]) + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) + releaseAttach() + const activated = await activations + const activatedTab = activated[0].tabs[0] + expect(activated[1].tabs[0]).toMatchObject({ status: 'ready' }) await expect( runtime.sendTerminal(activatedTab?.type === 'terminal' ? activatedTab.terminal! : 'missing', { text: 'after attach' }) ).resolves.toMatchObject({ accepted: true, bytesWritten: 12 }) - await runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') - expect(spawn).toHaveBeenCalledOnce() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ @@ -28373,7 +28383,20 @@ describe('OrcaRuntimeService', () => { getRepos: () => [remoteRepo], getRepo: (id: string) => (id === TEST_REPO_ID ? remoteRepo : undefined) } - const spawn = vi.fn().mockResolvedValue({ id: 'ssh:ssh-1@@relay-pty', isReattach: true }) + let releaseAttach!: () => void + const attachResult = new Promise((resolve) => { + releaseAttach = resolve + }) + const ptyAttach = vi.fn(async () => { + if (ptyAttach.mock.calls.length > 1) { + throw new Error('Unknown or stale SSH PTY source token') + } + await attachResult + }) + const spawn = vi.fn(async () => { + await ptyAttach() + return { id: 'ssh:ssh-1@@relay-pty', isReattach: true } + }) const runtime = new OrcaRuntimeService(remoteStore as never) runtime.setPtyController({ spawn, @@ -28393,10 +28416,16 @@ describe('OrcaRuntimeService', () => { }) runtime.syncWindowGraph(0, { tabs: [], leaves: [] }) - await runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') - await runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') + const activations = Promise.all([ + runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab'), + runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') + ]) + await vi.waitFor(() => expect(ptyAttach).toHaveBeenCalledOnce()) + releaseAttach() + await expect(activations).resolves.toHaveLength(2) expect(spawn).toHaveBeenCalledOnce() + expect(ptyAttach).toHaveBeenCalledOnce() expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ connectionId: 'ssh-1', @@ -28437,9 +28466,14 @@ describe('OrcaRuntimeService', () => { getRepos: () => [remoteRepo], getRepo: (id: string) => (id === TEST_REPO_ID ? remoteRepo : undefined) } + let releaseFirstAttach!: () => void + const firstAttach = new Promise((resolve) => { + releaseFirstAttach = resolve + }) const spawn = vi .fn() .mockImplementationOnce(async () => { + await firstAttach const session = getSession() ;(runtimeStore.setWorkspaceSession as unknown as (next: WorkspaceSessionState) => void)({ ...session, @@ -28470,9 +28504,16 @@ describe('OrcaRuntimeService', () => { }) runtime.syncWindowGraph(0, { tabs: [], leaves: [] }) - await expect( + const firstActivations = Promise.allSettled([ + runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab'), runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') - ).rejects.toThrow('SSH session expired') + ]) + await vi.waitFor(() => expect(spawn).toHaveBeenCalledOnce()) + releaseFirstAttach() + await expect(firstActivations).resolves.toEqual([ + expect.objectContaining({ status: 'rejected', reason: expect.any(Error) }), + expect.objectContaining({ status: 'rejected', reason: expect.any(Error) }) + ]) await runtime.activateMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab') expect(spawn).toHaveBeenNthCalledWith( diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 4602cef5c53..6e455e3867a 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -243,6 +243,7 @@ import { navigationTargetsHost, type RuntimeNavigationTarget } from '../../shared/runtime-navigation' +import { mobileTerminalMaterializationKey } from './mobile-terminal-materialization-key' import type { SshConnectionState } from '../../shared/ssh-types' import { getPublicSshState } from './public-ssh-state' import { closeTerminalTabInWorkspaceSession } from '../../shared/workspace-session-terminal-tab-close' @@ -2813,6 +2814,7 @@ export class OrcaRuntimeService { fence: (() => boolean) | null } >() + private pendingMobileTerminalMaterializations = new Map>() private leaves = new Map() // Why: PTY output is a per-keystroke hot path. Looking up affected leaves by // ptyId keeps active TUI redraws independent of the total open terminal count. @@ -8101,44 +8103,100 @@ export class OrcaRuntimeService { this.shouldMaterializeHeadlessMobileSessionTab(snapshot!, tab)) if (shouldMaterializePendingTerminal) { const sessionId = tab.ptyId ?? tab.parentLayout?.ptyIdsByLeafId?.[tab.leafId] ?? undefined + const sshConnectionId = sessionId ? parseAppSshPtyId(sessionId)?.connectionId : undefined + const reconnectGeneration = sshConnectionId + ? (this.sshRelayRecoveryGenerationByTargetId.get(sshConnectionId) ?? 0) + : null + const acceptMaterialization = () => + acceptWorkspaceInventory?.() !== false && + (sshConnectionId === undefined || + (this.sshRelayRecoveryGenerationByTargetId.get(sshConnectionId) ?? 0) === + reconnectGeneration) const targetGroupId = snapshot?.tabGroups?.find((group) => group.tabOrder.includes(tab.parentTabId) )?.id - // Why: a pending agent tab may exist without its startup command ever - // having been delivered (the create's renderer stalled, #7587), so a - // bare materialize would put a plain shell under the agent icon. - // Re-resolve the launch like the create path; providers skip startup - // commands when attaching to live sessions, so this cannot double-launch. - let agentStartup: Awaited< - ReturnType - > = {} - if (tab.launchAgent) { - try { - const workspace = await this.resolveTerminalWorkspaceLaunchScope(`id:${worktreeId}`) - agentStartup = await this.resolveMobileSessionTerminalCommand(workspace, { - agent: tab.launchAgent + const executionHostId = this.getWorkspaceSessionHostIdForWorktree(worktreeId) + const parsedExecutionHost = parseExecutionHostId(executionHostId) + const connectionId = + sshConnectionId ?? + (parsedExecutionHost?.kind === 'ssh' ? parsedExecutionHost.targetId : null) + const materializationKey = mobileTerminalMaterializationKey({ + executionHostId, + connectionId, + worktreeId, + parentTabId: tab.parentTabId, + leafId: tab.leafId, + sessionId + }) + let materialization = this.pendingMobileTerminalMaterializations.get(materializationKey) + if (!materialization) { + materialization = (async () => { + // Why: a stalled renderer may leave a pending agent tab without its startup command. + let agentStartup: Awaited< + ReturnType + > = {} + if (tab.launchAgent) { + try { + const workspace = await this.resolveTerminalWorkspaceLaunchScope(`id:${worktreeId}`) + agentStartup = await this.resolveMobileSessionTerminalCommand(workspace, { + agent: tab.launchAgent + }) + } catch { + // Why: an unavailable agent must not make its restored tab untappable. + } + } + if (this.hasLiveRuntimeSessionOwnedPtyBinding(worktreeId, tab)) { + return + } + try { + await this.createRuntimeOwnedMobileSessionTerminal( + worktreeId, + targetsHost, + undefined, + { + identity: { + tabId: tab.parentTabId, + leafId: tab.leafId, + sessionId + }, + cwd: tab.startupCwd, + command: agentStartup.command, + env: agentStartup.env, + startupCommandDelivery: agentStartup.startupCommandDelivery, + launchConfig: agentStartup.launchConfig, + launchAgent: tab.launchAgent, + targetGroupId, + acceptWorkspaceInventory: () => + acceptMaterialization() && + !this.hasLiveRuntimeSessionOwnedPtyBinding(worktreeId, tab) + } + ) + } catch (error) { + if ( + error instanceof Error && + error.message === 'tab_not_found' && + acceptMaterialization() && + this.hasLiveRuntimeSessionOwnedPtyBinding(worktreeId, tab) + ) { + return + } + throw error + } + })() + this.pendingMobileTerminalMaterializations.set(materializationKey, materialization) + void materialization + .finally(() => { + if ( + this.pendingMobileTerminalMaterializations.get(materializationKey) === + materialization + ) { + this.pendingMobileTerminalMaterializations.delete(materializationKey) + } }) - } catch { - // Why: a disabled or unresolvable agent must not make the tab - // untappable; fall back to the plain-shell materialize. - } + .catch(() => undefined) } try { - await this.createRuntimeOwnedMobileSessionTerminal(worktreeId, targetsHost, undefined, { - identity: { - tabId: tab.parentTabId, - leafId: tab.leafId, - sessionId - }, - cwd: tab.startupCwd, - command: agentStartup.command, - env: agentStartup.env, - startupCommandDelivery: agentStartup.startupCommandDelivery, - launchConfig: agentStartup.launchConfig, - launchAgent: tab.launchAgent, - targetGroupId, - acceptWorkspaceInventory - }) + await materialization } catch (err) { if (sessionId && parseAppSshPtyId(sessionId)) { // Why: an expired SSH reattach clears durable bindings in the store, @@ -8147,6 +8205,9 @@ export class OrcaRuntimeService { } throw err } + if (!acceptMaterialization()) { + throw new Error('tab_not_found') + } return this.applyMobileSessionTabNavigation( this.getMobileSessionTabsForWorktree(worktreeId), tab.id,