diff --git a/src/main/agent-launch/agent-launch-token.ts b/src/main/agent-launch/agent-launch-token.ts index 78655bfe538..3469fd9aca8 100644 --- a/src/main/agent-launch/agent-launch-token.ts +++ b/src/main/agent-launch/agent-launch-token.ts @@ -5,3 +5,11 @@ export function agentLaunchTokenFor( ): string | undefined { return launch.launchConfig ? (launch.launchToken ?? mint()) : undefined } + +/** The launch token travels with the same environment the provider will execute. */ +export function withAgentLaunchToken( + env: Record | undefined, + launchToken: string | undefined +): Record { + return { ...env, ...(launchToken ? { ORCA_AGENT_LAUNCH_TOKEN: launchToken } : {}) } +} diff --git a/src/main/daemon/daemon-pty-adapter-replacement-exit-race.test.ts b/src/main/daemon/daemon-pty-adapter-replacement-exit-race.test.ts index e1ee89f95e0..8f030185c97 100644 --- a/src/main/daemon/daemon-pty-adapter-replacement-exit-race.test.ts +++ b/src/main/daemon/daemon-pty-adapter-replacement-exit-race.test.ts @@ -1,3 +1,7 @@ +import { fixture } from '../persistence/loading-store/profile-state-delayed-authority-fixture' +import { OrcaRuntimeService } from '../runtime/orca-runtime' +import { adoptStablePane } from '../ipc/pty/pane/adopt-stable' +import { setLocalPtyProvider } from '../ipc/pty/provider/registry' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { rmSync } from 'node:fs' import type { DaemonPtyAdapter } from './daemon-pty-adapter' @@ -31,43 +35,73 @@ describe('DaemonPtyAdapter replacement exit races', () => { } }) - it('captures a replacement exit while the crashed daemon incarnation is still cached', async () => { - const sessionId = 'replacement-exit-with-stale-incarnation-cache' - const internals = adapter as unknown as { - activeSessionIds: Set - sessionIncarnations: Map - pendingSpawnOperationsBySessionId: Map< - string, - Set<{ exitsBySessionId: Map }> - > - client: { request: (type: string, payload?: unknown) => Promise } - } - internals.activeSessionIds.add(sessionId) - internals.sessionIncarnations.set(sessionId, 'incarnation-from-crashed-daemon') - const originalRequest = internals.client.request.bind(internals.client) - vi.spyOn(internals.client, 'request').mockImplementation( - async (type: string, payload?: unknown) => { - const response = await originalRequest(type, payload) - if (type === 'createOrAttach') { - lastSubprocess._simulateExit(19) - await waitFor(() => - [...(internals.pendingSpawnOperationsBySessionId.get(sessionId) ?? [])].some( - (operation) => (operation.exitsBySessionId.get(sessionId)?.length ?? 0) > 0 - ) - ) - } - return response + it.each([false, true])( + 'captures a replacement exit with stale cache (production adoption: %s)', + async (adoption) => { + const sessionId = 'replacement-exit-with-stale-incarnation-cache' + const internals = adapter as unknown as { + activeSessionIds: Set + sessionIncarnations: Map + pendingSpawnOperationsBySessionId: Map< + string, + Set<{ exitsBySessionId: Map }> + > + client: { request: (type: string, payload?: unknown) => Promise } } - ) + const { store } = await fixture() + const runtime = new OrcaRuntimeService(store) + const pane = { + worktreeId: 'repo-local::/fixture/local', + tabId: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', + leafId: 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb' + } + if (adoption) { + await adapter.spawn({ cols: 80, rows: 24, sessionId }) + await store.persistPtyBinding({ + ...pane, + ptyId: sessionId, + incarnationId: 'incarnation-from-crashed-daemon' + }) + setLocalPtyProvider(adapter) + } + const spawn = vi.spyOn(adapter, 'spawn') + internals.activeSessionIds.add(sessionId) + internals.sessionIncarnations.set(sessionId, 'incarnation-from-crashed-daemon') + const originalRequest = internals.client.request.bind(internals.client) + vi.spyOn(internals.client, 'request').mockImplementation( + async (type: string, payload?: unknown) => { + const response = await originalRequest(type, payload) + if (type === 'createOrAttach') { + lastSubprocess._simulateExit(19) + await waitFor(() => + [...(internals.pendingSpawnOperationsBySessionId.get(sessionId) ?? [])].some( + (operation) => (operation.exitsBySessionId.get(sessionId)?.length ?? 0) > 0 + ) + ) + } + return response + } + ) - const result = await adapter.spawn({ cols: 80, rows: 24, sessionId }) + if (adoption) { + await expect( + adoptStablePane(runtime, store, { ...pane, cols: 80, rows: 24 }) + ).rejects.toThrow('agent_session_exited_during_start') + expect(() => + runtime.resolveTerminalPane(`${pane.tabId}:${pane.leafId}`, pane.worktreeId) + ).toThrow('terminal_not_found') + } else { + await adapter.spawn({ cols: 80, rows: 24, sessionId }) + } + const result = await spawn.mock.results[0].value - expect(result).toMatchObject({ - incarnationId: expect.any(String), - exitedBeforeSpawnReply: true - }) - expect(internals.activeSessionIds.has(sessionId)).toBe(false) - expect(internals.sessionIncarnations.has(sessionId)).toBe(false) - expect(internals.pendingSpawnOperationsBySessionId.has(sessionId)).toBe(false) - }) + expect(result).toMatchObject({ + incarnationId: expect.any(String), + exitedBeforeSpawnReply: true + }) + expect(internals.activeSessionIds.has(sessionId)).toBe(false) + expect(internals.sessionIncarnations.has(sessionId)).toBe(false) + expect(internals.pendingSpawnOperationsBySessionId.has(sessionId)).toBe(false) + } + ) }) diff --git a/src/main/ipc/pty-agent-pane-launch.test.ts b/src/main/ipc/pty-agent-pane-launch.test.ts index ed6f5a54ca1..984aabb3f31 100644 --- a/src/main/ipc/pty-agent-pane-launch.test.ts +++ b/src/main/ipc/pty-agent-pane-launch.test.ts @@ -1,3 +1,5 @@ +import { ptyIncarnationById, ptyOwnership } from './pty/provider/ownership-state' +import { createLaunchRaceRuntime } from './pty-launch-race-test-fixture' import { afterEach, describe, expect, it, vi } from 'vitest' import type { AgentPaneLaunchRequest } from '../../shared/agent-pane-launch-request' import { setupPtyIpcSuite } from './pty-ipc-test-harness' @@ -67,6 +69,8 @@ describe('a pane that starts its own agent launch', () => { afterEach(() => { resetAgentLaunchPanesForTests() + ptyIncarnationById.delete('pty-agent') + ptyOwnership.delete('pty-agent') }) function register( @@ -75,10 +79,10 @@ describe('a pane that starts its own agent launch', () => { ) { installDaemonTestProvider({ spawn: providerSpawn }) const runtime = { + ...createLaunchRaceRuntime(), setPtyController: vi.fn(), createPreAllocatedTerminalHandle: vi.fn(() => 'term_pane'), registerPreAllocatedHandleForPty: vi.fn(), - registerPty: vi.fn(), onPtySpawned: vi.fn(), onPtyExit: vi.fn(), onPtyData: vi.fn(), diff --git a/src/main/ipc/pty-closed-pane-spawn-refusal.test.ts b/src/main/ipc/pty-closed-pane-spawn-refusal.test.ts index 75965d120bb..d21d823b4d5 100644 --- a/src/main/ipc/pty-closed-pane-spawn-refusal.test.ts +++ b/src/main/ipc/pty-closed-pane-spawn-refusal.test.ts @@ -109,5 +109,9 @@ describe('a window spawn for a pane main took back while it waited', () => { }) ).rejects.toThrow('terminal_pane_closed') expect(spawnMock).not.toHaveBeenCalled() + expect(current.value.terminalLayoutsByTabId?.[TAB]?.root).toEqual({ + type: 'leaf', + leafId: KEPT + }) }) }) diff --git a/src/main/ipc/pty-controller-ownership-routing.test.ts b/src/main/ipc/pty-controller-ownership-routing.test.ts index e8bb5aa854a..ff88379b501 100644 --- a/src/main/ipc/pty-controller-ownership-routing.test.ts +++ b/src/main/ipc/pty-controller-ownership-routing.test.ts @@ -257,7 +257,8 @@ describe('registerPtyHandlers', () => { expect(runtime.registerPreAllocatedHandleForPty).not.toHaveBeenCalled() expect(runtime.cancelPendingPtyRegistration).toHaveBeenCalledWith( 'different-requested-id', - 'incarnation-canonical-exited' + 'incarnation-canonical-exited', + undefined ) }) }) diff --git a/src/main/ipc/pty-controller-spawn-admission.test.ts b/src/main/ipc/pty-controller-spawn-admission.test.ts index ed4356f6c6d..87bc2fba4a4 100644 --- a/src/main/ipc/pty-controller-spawn-admission.test.ts +++ b/src/main/ipc/pty-controller-spawn-admission.test.ts @@ -75,7 +75,7 @@ describe('registerPtyHandlers', () => { spawn: vi.fn(async () => { runtime.onPtySpawned(ptyId, incarnationId) runtime.onPtyExit(ptyId, 0, incarnationId) - return { id: ptyId, incarnationId } + return { id: ptyId, incarnationId, exitedBeforeSpawnReply: true } }), authoritativeOwnerListings: false }) @@ -105,10 +105,8 @@ describe('registerPtyHandlers', () => { expect(store.persistPtyBinding).not.toHaveBeenCalled() expect(registerRuntimePty).not.toHaveBeenCalled() const internals = runtime as unknown as { - earlyExitedPtyIncarnations: Map pendingPtyRegistrationIncarnations: Map } - expect(internals.earlyExitedPtyIncarnations.size).toBe(0) expect(internals.pendingPtyRegistrationIncarnations.size).toBe(0) clearProviderPtyState(ptyId) }) @@ -392,9 +390,9 @@ describe('registerPtyHandlers', () => { ).rejects.toThrow('agent_session_exited_during_start') const internals = runtime as unknown as { - earlyExitedPtyIncarnations: Map + pendingPtyRegistrationIncarnations: Map } - expect(internals.earlyExitedPtyIncarnations.has(owner.ptyId)).toBe(false) + expect(internals.pendingPtyRegistrationIncarnations.has(owner.ptyId)).toBe(false) }) it('rejects stale exits immediately after SSH reconnect restores an incarnation', () => { const ptyId = 'ssh:target-1@@pty-reconnected' diff --git a/src/main/ipc/pty-launch-parity.test-fixture.ts b/src/main/ipc/pty-launch-parity.test-fixture.ts index bfea858cebc..23ae861c610 100644 --- a/src/main/ipc/pty-launch-parity.test-fixture.ts +++ b/src/main/ipc/pty-launch-parity.test-fixture.ts @@ -1,3 +1,4 @@ +import { createPaneBindingStore } from './pty-launch-race-test-fixture' // Test-only: one real OrcaRuntimeService wired into the real pty handlers, so both launch lanes // reach the same fake provider. Window lane: pty:spawn. Host lane: createTerminal / createAgentSession // through the runtime controller registerPtyHandlers installs. Suites still declare the IPC vi.mock block. @@ -127,6 +128,7 @@ type ScopeResolver = { } export type ParityLanes = { + store: ReturnType runtime: OrcaRuntimeService /** The local daemon provider's spawn mock. */ provider: Mock @@ -145,7 +147,8 @@ export function startParityLanes( } ): ParityLanes { const settings = parityMainSettings(args.settings) - const persistPtyBinding = vi.fn(async () => true) + const bindingStore = createPaneBindingStore() + const persistPtyBinding = bindingStore.bindingWrites const { repo: scopeRepo, folderWorkspace } = launchScope(args.workspace) // A folder's contained repo is saved too, so the host can borrow its runtime if it would. const repo = @@ -154,6 +157,7 @@ export function startParityLanes( ? launchScope({ kind: 'repo', path: args.workspace.repoPath }).repo : null) const store = { + ...bindingStore, getSettings: () => settings, getProjects: () => [ { @@ -168,11 +172,9 @@ export function startParityLanes( getRepos: () => (repo ? [repo] : []), getAllWorktreeMeta: () => ({}), getWorktreeMeta: () => undefined, - getWorkspaceSession: () => ({ tabsByWorktree: {}, terminalLayoutsByTabId: {} }), getFolderWorkspace: () => folderWorkspace ?? undefined, getFolderWorkspaces: () => (folderWorkspace ? [folderWorkspace] : []), - getProjectGroups: () => [{ id: 'pg-1', name: 'group', parentPath: null, connectionId: null }], - persistPtyBinding + getProjectGroups: () => [{ id: 'pg-1', name: 'group', parentPath: null, connectionId: null }] } // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: launches read only the store members defined above; the rest of the persistence store is unreachable here. const runtime = new OrcaRuntimeService(store as never) @@ -215,6 +217,7 @@ export function startParityLanes( terminalDriverChanged: vi.fn() }) return { + store, runtime, provider, persistPtyBinding, diff --git a/src/main/ipc/pty-launch-parity.test.ts b/src/main/ipc/pty-launch-parity.test.ts index ffe76011540..c87a1bb940e 100644 --- a/src/main/ipc/pty-launch-parity.test.ts +++ b/src/main/ipc/pty-launch-parity.test.ts @@ -1,4 +1,9 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import * as spawnRouting from './pty/host-env/fresh-spawn-routing' +import * as teamsShim from '../runtime/claude-agent-teams-shim-env' +import { WorktreeTerminalMutationLock } from '../runtime/worktree-terminal-mutation-lock' +import * as teamsPlan from '../runtime/terminal/launch/agent-teams-launch-plan' +import { SessionNotFoundError } from '../daemon/daemon-errors' +import { afterEach, beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest' import { setupPtyIpcSuite } from './pty-ipc-test-harness' import { classifyErrorMock, statSyncMock, trackMock } from './pty-ipc-mock-registry' import { @@ -287,6 +292,270 @@ describe('launch facts outside the tables', () => { const suite = setupPtyIpcSuite() afterEach(() => restoreMainPlatform()) + it.each(['ipc', 'runtime', 'ipc-replay', 'ipc-reentry', 'ipc-reentry-refused'] as const)( + '%s settles a contested recovery on one owner with balanced preparation resources', + async (lane) => { + const workspace = + lane === 'ipc' || lane === 'runtime' + ? POSIX_REPO + : { kind: 'folder' as const, path: POSIX_PATH } + const lanes = startParityLanes(suite, { workspace }) + const pane = { + worktreeId: launchWorkspaceId(workspace), + tabId: HOST_TAB_ID, + leafId: HOST_LEAF_ID + } + if (lane === 'ipc-reentry' || lane === 'ipc-reentry-refused') { + const refused = lane === 'ipc-reentry-refused' + classifyErrorMock.mockReturnValue({ error_class: 'unknown' }) + await lanes.store.admitTerminalPane({ + type: 'createTerminalTab', + workspace: pane.worktreeId, + ...pane + }) + const paneKey = makePaneKey(pane.tabId, pane.leafId) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: exposes the existing selector; empty-inventory Sleep needs only this local workspace identity. + const selecting = lanes.runtime as unknown as { + resolveWorktreeSelector: () => Promise<{ id: string }> + } + vi.spyOn(selecting, 'resolveWorktreeSelector').mockResolvedValue({ id: pane.worktreeId }) + const events: string[] = [] + const releases: ReturnType void>>[] = [] + const finishes: ReturnType void>>[] = [] + const existingSizes = new Set(ptySizes.keys()) + let firstId: string | undefined + let queuedSleep: Promise | undefined + let attempts = 0 + vi.spyOn(teamsShim, 'ensureClaudeAgentTeamsShimDir').mockResolvedValue('/fixture/shim') + vi.spyOn(teamsShim, 'resolveClaudeAgentTeamsShimBin').mockReturnValue(null) + const prepareTeam = vi.spyOn(lanes.runtime, 'prepareClaudeAgentTeamsLeaderForHandle') + const releaseTeam = vi.spyOn(lanes.runtime, 'releaseClaudeAgentTeamsLeaderForHandle') + const registeredHandle = vi.spyOn(lanes.runtime, 'registerPreAllocatedHandleForPty') + const beginRegistration = vi.spyOn(lanes.runtime, 'beginPtyRegistration') + const begin = spawnRouting.beginPtySpawnForWorktree + const installing = vi + .spyOn(spawnRouting, 'beginPtySpawnForWorktree') + .mockImplementation((...a) => { + const finish = vi.fn(begin(...a)) + finishes.push(finish) + return finish + }) + const acquire = WorktreeTerminalMutationLock.prototype.acquire + vi.spyOn(WorktreeTerminalMutationLock.prototype, 'acquire').mockImplementation( + async function (this: WorktreeTerminalMutationLock, key, kind, deadline) { + if (kind !== 'shared') { + return acquire.call(this, key, kind, deadline) + } + const attempt = ++attempts + if (attempt === 2 && refused) { + throw new Error('second_preparation_refused') + } + const release = await acquire.call(this, key, kind, deadline) + events.push(`shared-${attempt}`) + const receipt = vi.fn(() => { + events.push(`release-${attempt}`) + release() + }) + releases.push(receipt) + if (attempt === 1) { + firstId = [...ptySizes.keys()].find((id) => !existingSizes.has(id)) + await lanes.store.persistPtyBinding({ + ...pane, + ptyId: 'winner', + incarnationId: 'winner-inc' + }) + queuedSleep = acquire.call(this, key, 'exclusive').then((finishSleep) => { + events.push('exclusive') + finishSleep() + }) + expect(events).toEqual(['shared-1']) + } + return receipt + } + ) + onTestFinished(async () => { + for (const release of [...releases, ...finishes]) { + if (release.mock.calls.length === 0) { + release() + } + } + await queuedSleep + vi.restoreAllMocks() + }) + lanes.provider.mockImplementation(async (args) => { + if (args.attachOnly) { + throw new SessionNotFoundError(args.sessionId) + } + return { id: 'fresh-reentry', incarnationId: 'fresh-inc' } + }) + const operation = lanes.spawnWindow({ + ...pane, + cols: 80, + rows: 24, + cwd: POSIX_PATH, + command: 'claude --teammate-mode auto', + initiallyHidden: true + }) + if (refused) { + await expect(operation).rejects.toThrow('second_preparation_refused') + } else { + await expect(operation).resolves.toMatchObject({ id: 'fresh-reentry' }) + expect( + lanes.runtime.resolveTerminalPane(`${pane.tabId}:${pane.leafId}`, pane.worktreeId) + ).toMatchObject({ ptyId: 'fresh-reentry', connected: true }) + expect( + lanes.store.getWorkspaceSession().terminalLayoutsByTabId[pane.tabId]?.ptyIdsByLeafId?.[ + pane.leafId + ] + ).toBe('fresh-reentry') + } + await queuedSleep + const handles = prepareTeam.mock.calls.map(([args]) => args.handle) + expect(new Set(handles).size).toBe(2) + expect(releaseTeam.mock.calls).toEqual( + (refused ? handles : handles.slice(0, 1)).map((handle) => [handle]) + ) + expect(registeredHandle.mock.calls).toEqual( + (refused ? [] : [1, 2]).map(() => ['fresh-reentry', handles[1]]) + ) + expect(releaseTeam.mock.invocationCallOrder[0]).toBeLessThan( + prepareTeam.mock.invocationCallOrder[1]! + ) + expect(attempts).toBe(2) + expect(releases).toHaveLength(refused ? 1 : 2) + expect(finishes).toHaveLength(2) + for (const receipt of [...releases, ...finishes]) { + expect(receipt).toHaveBeenCalledOnce() + } + expect(finishes[0]!.mock.invocationCallOrder[0]).toBeLessThan( + installing.mock.invocationCallOrder[1]! + ) + expect(events).toEqual( + refused + ? ['shared-1', 'release-1', 'exclusive'] + : ['shared-1', 'release-1', 'exclusive', 'shared-2', 'release-2'] + ) + expect( + lanes.provider.mock.calls.map(([args]) => [args.attachOnly === true, args.command]) + ).toEqual( + refused + ? [[true, undefined]] + : [ + [true, undefined], + [false, 'claude --teammate-mode auto'] + ] + ) + expect(firstId).toBeDefined() + expect(isHiddenRendererPty(firstId!)).toBe(false) + expect(ptySizes.has(firstId!)).toBe(false) + expect(beginRegistration.mock.calls.some(([id]) => id === firstId)).toBe(false) + expect( + lanes.store.getWorkspaceSession().terminalLayoutsByTabId[pane.tabId]?.root + ).toMatchObject({ type: 'leaf', leafId: pane.leafId }) + expect( + paneSpawnReservationsByOwnerKey.has( + makePaneSpawnReservationKey(pane.worktreeId, null, paneKey)! + ) + ).toBe(false) + if (!refused) { + await lanes.runtime.onPtyExit('fresh-reentry', 0, 'fresh-inc', { + providerExitObserved: true + }) + } + await expect( + lanes.runtime.sleepTerminalsForWorktree(`id:${pane.worktreeId}`) + ).resolves.toMatchObject({ stopped: 0, postStopVerified: true }) + return + } + // Replay: a stale window save re-adds the binding the host just proved dead before the commit. + const replay = lane === 'ipc-replay' + const owner = replay ? 'fresh' : 'winner' + await lanes.store.persistPtyBinding({ ...pane, ptyId: 'old' }) + lanes.provider.mockImplementation(async (args) => { + if (!args.attachOnly) { + await lanes.store.persistPtyBinding({ ...pane, ptyId: 'old' }) + return { id: 'fresh', incarnationId: 'fresh-inc' } + } + if (args.sessionId !== 'old') { + return { id: 'winner', incarnationId: 'winner-inc', isReattach: true } + } + if (!replay) { + await lanes.store.persistPtyBinding({ + ...pane, + ptyId: 'winner', + incarnationId: 'winner-inc' + }) + } + throw new SessionNotFoundError('old') + }) + await (lane.startsWith('ipc') + ? expect(lanes.spawnWindow({ ...pane, cols: 80, rows: 24 })).resolves.toMatchObject({ + id: owner + }) + : lanes.runtime.createTerminal(`id:${pane.worktreeId}`, { + tabId: pane.tabId, + leafId: pane.leafId + })) + expect( + lanes.runtime.resolveTerminalPane(`${pane.tabId}:${pane.leafId}`, pane.worktreeId) + ).toMatchObject({ ptyId: owner, connected: true }) + expect(lanes.provider.mock.calls.map(([args]) => args.attachOnly === true)).toEqual([ + true, + !replay + ]) + expect( + lanes.store.getWorkspaceSession().terminalLayoutsByTabId[pane.tabId]?.ptyIdsByLeafId?.[ + pane.leafId + ] + ).toBe(owner) + } + ) + + it('does not readmit a recovered pane closed during lazy launch preparation', async () => { + const lanes = startParityLanes(suite, { workspace: POSIX_REPO }) + const binding = { + worktreeId: launchWorkspaceId(POSIX_REPO), + tabId: HOST_TAB_ID, + leafId: HOST_LEAF_ID, + ptyId: 'absent-before-preparation' + } + await lanes.store.persistPtyBinding(binding) + lanes.provider.mockRejectedValueOnce(new SessionNotFoundError(binding.ptyId)) + let release!: () => void + const gate = new Promise((resolve) => { + release = resolve + }) + const plan = teamsPlan.buildRuntimeAgentTeamsLaunchPlan + const preparing = vi + .spyOn(teamsPlan, 'buildRuntimeAgentTeamsLaunchPlan') + .mockImplementation(async (args) => { + await gate + return plan(args) + }) + try { + const created = lanes.runtime.createTerminal(`id:${binding.worktreeId}`, { + tabId: HOST_TAB_ID, + leafId: HOST_LEAF_ID + }) + const refused = expect(created).rejects.toThrow('terminal_pane_owner_changed') + await vi.waitFor(() => expect(preparing).toHaveBeenCalledOnce()) + await lanes.store.withdrawTerminalPane({ + type: 'closePane', + workspace: binding.worktreeId, + tabId: HOST_TAB_ID, + leafId: HOST_LEAF_ID + }) + release() + await refused + expect(lanes.provider).toHaveBeenCalledOnce() + expect(lanes.store.getWorkspaceSession().terminalLayoutsByTabId[HOST_TAB_ID]).toBeUndefined() + expect(lanes.reveal).not.toHaveBeenCalled() + } finally { + release() + preparing.mockRestore() + } + }) + // C5a (one agent_error place): main today reports a failed window-lane spawn. it.each([ { @@ -340,6 +609,7 @@ describe('launch facts outside the tables', () => { release() } }) + const beginRegistration = vi.spyOn(lanes.runtime, 'beginPtyRegistration') const cancelRegistration = vi.spyOn(lanes.runtime, 'cancelPendingPtyRegistration') const worktreeId = launchWorkspaceId(POSIX_REPO) const launch = @@ -362,7 +632,12 @@ describe('launch facts outside the tables', () => { expect(trackMock.mock.calls).toEqual(FAILED_START_AGENT_ERROR[lane]) expect(hiddenWhileSpawning).toBe(true) expect(isHiddenRendererPty(sessionId)).toBe(false) - expect(cancelRegistration.mock.calls).toEqual([[sessionId, undefined]]) + const claim = + beginRegistration.mock.results[ + beginRegistration.mock.calls.findIndex(([id]) => id === sessionId) + ]?.value + expect(claim).toBeDefined() + expect(cancelRegistration.mock.calls).toEqual([[sessionId, undefined, claim]]) expect(gateReleased).toHaveBeenCalledOnce() expect(ptySizes.has(sessionId)).toBe(false) const paneKey = makePaneKey(HOST_TAB_ID, HOST_LEAF_ID) diff --git a/src/main/ipc/pty-launch-race-parity.test.ts b/src/main/ipc/pty-launch-race-parity.test.ts index f181fea3142..93865d80fe7 100644 --- a/src/main/ipc/pty-launch-race-parity.test.ts +++ b/src/main/ipc/pty-launch-race-parity.test.ts @@ -77,12 +77,6 @@ describe('launch race parity, both spawn lanes', () => { const { handlers, mainWindow, installDaemonTestProvider } = setupPtyIpcSuite() describe('who owns the pane', () => { - // C6 picks one: the window joins before its setup and checks nothing; the host joins after and verifies. - const JOINED_UNVERIFIED_WINNER: Record = { - ipc: 'reattach', - runtime: 'refused' - } - const JOINER_PREPARES_ITS_OWN_LAUNCH: Record = { ipc: 0, runtime: 1 } it.each(LAUNCH_RACE_LANES)( '%s lane: a spawn arriving while a window spawn is starting the pane joins it', async (lane) => { @@ -114,17 +108,15 @@ describe('launch race parity, both spawn lanes', () => { onTestFinished(() => joinPaneSpawn.mockRestore()) const joiner = lanes.spawn[lane]({ ...pane.args, command: 'claude' }) if (lane === 'runtime') { - // Runtime preflight does filesystem work before it can join the reserved pane. + // Both lanes join before any fresh launch preparation. await vi.waitFor(() => expect(joinPaneSpawn).toHaveBeenCalledOnce()) } finishSpawn() await expect(winner).resolves.toMatchObject({ id: 'pty-join-winner' }) - await (JOINED_UNVERIFIED_WINNER[lane] === 'reattach' - ? expect(joiner).resolves.toMatchObject({ id: 'pty-join-winner', isReattach: true }) - : expect(joiner).rejects.toThrow('terminal_pane_owner_unknown')) + await expect(joiner).resolves.toMatchObject({ id: 'pty-join-winner' }) expect(spawn).toHaveBeenCalledOnce() - expect(auth.prepareClaudeAuth).toHaveBeenCalledTimes(JOINER_PREPARES_ITS_OWN_LAUNCH[lane]) + expect(auth.prepareClaudeAuth).not.toHaveBeenCalled() } ) @@ -402,7 +394,7 @@ describe('launch race parity, both spawn lanes', () => { immediate: true, expectedIncarnationId: 'inc-1' }) - expect(runtime.cancelPendingPtyRegistration).toHaveBeenCalledWith(ptyId, 'inc-1') + expect(runtime.cancelPendingPtyRegistration).toHaveBeenCalledWith(ptyId, 'inc-1', undefined) } ) @@ -433,7 +425,11 @@ describe('launch race parity, both spawn lanes', () => { await (EXITED_DURING_REGISTRATION[lane] === 'resolves' ? expect(spawn).resolves.toMatchObject({ id: ptyId, incarnationId: 'inc-exited' }) : expect(spawn).rejects.toThrow('agent_session_exited_during_start')) - expect(runtime.cancelPendingPtyRegistration).toHaveBeenCalledWith(ptyId, 'inc-exited') + expect(runtime.cancelPendingPtyRegistration).toHaveBeenCalledWith( + ptyId, + 'inc-exited', + undefined + ) expect(runtime.noteTerminalSpawnCommit).not.toHaveBeenCalled() expect(runtime.reflowHeadlessTerminalToPtyGrid).not.toHaveBeenCalled() } diff --git a/src/main/ipc/pty-launch-race-test-fixture.ts b/src/main/ipc/pty-launch-race-test-fixture.ts index d5efb9298b9..607fe6a5f97 100644 --- a/src/main/ipc/pty-launch-race-test-fixture.ts +++ b/src/main/ipc/pty-launch-race-test-fixture.ts @@ -1,3 +1,11 @@ +import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { PersistPtyBindingArgs } from '../persistence/loading-store/pty-binding-persistence' +import { ptyBindingIsRefused } from '../persistence/loading-store/pty-binding-refusals' +import { applyPtyBinding } from '../persistence/loading-store/pty-binding-session-update' +import { + withRuntimePaneAdmission, + withDurableRuntimeStore +} from '../runtime/runtime-durable-store-fixture' import { vi, type Mock } from 'vitest' import { makePaneKey } from '../../shared/stable-pane-id' import { WORKTREE_TERMINALS_SLEEPING_ERROR } from '../runtime/worktree-terminals-sleeping-error' @@ -69,9 +77,21 @@ export function launchRacePane(tag: string, leafId: string) { export function createLaunchRaceRuntime>( overrides?: T ): LaunchRaceRuntimeBase & T { + let owner: + | { + ptyId: string + worktreeId: string + tabId: string + leafId: string + handle: string + } + | undefined const base: LaunchRaceRuntimeBase = { setPtyController: vi.fn(), resolveTerminalPane: vi.fn((): unknown => { + if (owner) { + return owner + } throw new Error('terminal_not_found') }), createPreAllocatedTerminalHandle: vi.fn(() => 'term-launch-race'), @@ -80,7 +100,16 @@ export function createLaunchRaceRuntime> beginPtyRegistration: vi.fn(), cancelPendingPtyRegistration: vi.fn(), assertPtyRegistrationAllowed: vi.fn(), - registerPty: vi.fn(), + registerPty: vi.fn( + ( + ptyId: string, + worktreeId: string, + _connection: unknown, + binding: { tabId: string; leafId: string } + ) => { + owner = { ptyId, worktreeId, ...binding, handle: 'term-launch-race' } + } + ), noteTerminalSpawnCommand: vi.fn(), noteTerminalSpawnCommit: vi.fn(), reflowHeadlessTerminalToPtyGrid: vi.fn(), @@ -151,3 +180,35 @@ export function sleptWorktreeLock() { ) return { acquireWorktreeTerminalSpawn, refuse } } + +/** Evaluates the same queued binding input against mutable topology, recording admitted arguments. */ +export function createPaneBindingStore() { + let session = getDefaultWorkspaceSession() + const bindingWrites = vi.fn((_binding: PersistPtyBindingArgs) => true) + const store = withDurableRuntimeStore( + withRuntimePaneAdmission({ + flushOrThrow: () => {}, + getWorkspaceSession: () => session, + setWorkspaceSession: (next: typeof session) => { + session = next + }, + persistPtyBinding: vi.fn( + async (input: PersistPtyBindingArgs | (() => PersistPtyBindingArgs | null)) => { + const binding = typeof input === 'function' ? input() : input + if (!binding) { + return false + } + const accepted = bindingWrites(binding) + const paneKey = makePaneKey(binding.tabId, binding.leafId) + const workspace = binding.expectedSourceBinding?.worktreeId ?? binding.worktreeId + if (!accepted || ptyBindingIsRefused(binding, session, workspace, paneKey, [])) { + return false + } + applyPtyBinding(binding, session, workspace, paneKey) + return true + } + ) + }) + ) + return { ...store, bindingWrites } +} diff --git a/src/main/ipc/pty-pane-claim-arbitration.test.ts b/src/main/ipc/pty-pane-claim-arbitration.test.ts index 127b310e2d8..ae43bfbc709 100644 --- a/src/main/ipc/pty-pane-claim-arbitration.test.ts +++ b/src/main/ipc/pty-pane-claim-arbitration.test.ts @@ -1,3 +1,4 @@ +import { createPaneBindingStore } from './pty-launch-race-test-fixture' import { describe, expect, it, vi } from 'vitest' import { spawnMock } from './pty-ipc-mock-registry' import { setupPtyIpcSuite } from './pty-ipc-test-harness' @@ -113,9 +114,7 @@ describe('registerPtyHandlers', () => { persistHostSessionBinding?: boolean }): Promise<{ id: string }> } - const store = { - persistPtyBinding: vi.fn() - } + const store = createPaneBindingStore() let controller: RuntimeSpawnController | null = null const runtime = { setPtyController: vi.fn((value) => { @@ -141,6 +140,12 @@ describe('registerPtyHandlers', () => { ) const spawnController = controller as unknown as RuntimeSpawnController const leafId = '11111111-1111-4111-8111-111111111111' + await store.admitTerminalPane({ + type: 'createTerminalTab', + workspace: 'wt-1', + tabId: 'tab-headless', + leafId + }) await spawnController.spawn({ cols: 80, rows: 24, @@ -151,7 +156,7 @@ describe('registerPtyHandlers', () => { persistHostSessionBinding: true }) - expect(store.persistPtyBinding).toHaveBeenCalledWith({ + expect(store.bindingWrites).toHaveBeenCalledWith({ worktreeId: 'wt-1', tabId: 'tab-headless', leafId, @@ -195,7 +200,7 @@ describe('registerPtyHandlers', () => { pid: 12345 } spawnMock.mockReturnValue(proc) - const store = { persistPtyBinding: vi.fn(() => false) } + const store = createPaneBindingStore() let controller: RuntimeSpawnController | null = null const runtime = { setPtyController: vi.fn((value) => { @@ -242,7 +247,7 @@ describe('registerPtyHandlers', () => { error.mockRestore() } - expect(store.persistPtyBinding).toHaveBeenCalledWith( + expect(store.bindingWrites).toHaveBeenCalledWith( expect.objectContaining({ expectedSourceBinding }) ) expect(killSpy).toHaveBeenCalledOnce() @@ -277,6 +282,7 @@ describe('registerPtyHandlers', () => { return { id: 'pty-early-exit', incarnationId: 'incarnation-early-exit', + exitedBeforeSpawnReply: true, providerSequence: { value: 17, generation: 'reset' as const }, wslDistro: 'Ubuntu' } @@ -312,20 +318,21 @@ describe('registerPtyHandlers', () => { expect(onPtySpawnCommitted).toHaveBeenCalledOnce() expect(persistPtyBinding).not.toHaveBeenCalled() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this test reads existing private maps only to verify rejected admission leaves no state. const internals = runtime as unknown as { handleByPtyId: Map providerSequenceInitializedPtys: Set ptyOutputSequenceById: Map ptysById: Map wslDistroByPtyId: Map - earlyExitedPtyIncarnations: Map + pendingPtyRegistrationIncarnations: Map } expect(internals.handleByPtyId.has('pty-early-exit')).toBe(false) expect(internals.providerSequenceInitializedPtys.has('pty-early-exit')).toBe(false) expect(internals.ptyOutputSequenceById.has('pty-early-exit')).toBe(false) expect(internals.ptysById.get('pty-early-exit')?.connected).not.toBe(true) expect(internals.wslDistroByPtyId.has('pty-early-exit')).toBe(false) - expect(internals.earlyExitedPtyIncarnations.has('pty-early-exit')).toBe(false) + expect(internals.pendingPtyRegistrationIncarnations.has('pty-early-exit')).toBe(false) clearProviderPtyState('pty-early-exit') }) it('does not retain a claimed owner when its PTY exits before controller admission', async () => { @@ -451,9 +458,7 @@ describe('registerPtyHandlers', () => { getDefaultShell: vi.fn(), getProfiles: vi.fn() } as never) - const store = { - persistPtyBinding: vi.fn() - } + const store = createPaneBindingStore() let controller: RuntimeSpawnController | null = null const runtime = { setPtyController: vi.fn((value) => { @@ -479,6 +484,12 @@ describe('registerPtyHandlers', () => { const spawnController = controller as unknown as RuntimeSpawnController const leafId = '22222222-2222-4222-8222-222222222222' const paneKey = makePaneKey('tab-race', leafId) + await store.admitTerminalPane({ + type: 'createTerminalTab', + workspace: 'repo-1::/tmp', + tabId: 'tab-race', + leafId + }) const runtimeSpawn = spawnController.spawn({ cols: 80, rows: 24, @@ -511,7 +522,7 @@ describe('registerPtyHandlers', () => { { id: 'pty-shared', isReattach: true } ]) expect(providerSpawn).toHaveBeenCalledTimes(1) - expect(store.persistPtyBinding).toHaveBeenCalledWith({ + expect(store.bindingWrites).toHaveBeenCalledWith({ worktreeId: 'repo-1::/tmp', tabId: 'tab-race', leafId, diff --git a/src/main/ipc/pty-pane-materialization-race.test.ts b/src/main/ipc/pty-pane-materialization-race.test.ts index 0ddb8574429..3f2e633fc64 100644 --- a/src/main/ipc/pty-pane-materialization-race.test.ts +++ b/src/main/ipc/pty-pane-materialization-race.test.ts @@ -397,14 +397,7 @@ describe('registerPtyHandlers', () => { } }) expect(providerSpawn).toHaveBeenCalledTimes(1) - expect(store.persistPtyBinding).toHaveBeenCalledWith({ - worktreeId: 'repo-1::/tmp', - tabId: 'tab-race', - leafId, - ptyId: 'pty-renderer', - startupCwd: '/tmp', - origin: 'spawn' - }) + expect(store.persistPtyBinding).toHaveBeenCalledWith(expect.any(Function), undefined) }) it.each([ { @@ -448,6 +441,7 @@ describe('registerPtyHandlers', () => { const leafId = '66666666-6666-4666-8666-666666666666' const paneKey = makePaneKey(tabId, leafId) let ownerPublished = false + const ownerId = 'pty-live-owner' let releaseAttach!: () => void let attachBarrier: Promise const resetAttachBarrier = (): void => { @@ -463,7 +457,7 @@ describe('registerPtyHandlers', () => { if (options.attachOnly) { await attachBarrier return { - id: 'pty-live-owner', + id: options.sessionId ?? ownerId, incarnationId: 'inc-live-owner', isReattach: true, snapshot: 'original-live-output', @@ -527,7 +521,7 @@ describe('registerPtyHandlers', () => { handle: 'term-live-owner', tabId, leafId, - ptyId: 'pty-live-owner', + ptyId: ownerId, worktreeId } }), diff --git a/src/main/ipc/pty-persisted-incarnation-repair.test.ts b/src/main/ipc/pty-persisted-incarnation-repair.test.ts index 2f4910addda..2fc824a83a9 100644 --- a/src/main/ipc/pty-persisted-incarnation-repair.test.ts +++ b/src/main/ipc/pty-persisted-incarnation-repair.test.ts @@ -1,3 +1,4 @@ +import { createPaneBindingStore } from './pty-launch-race-test-fixture' import { withDurableRuntimeStore } from '../runtime/runtime-durable-store-fixture' import { describe, expect, it, vi } from 'vitest' import { statSyncMock } from './pty-ipc-mock-registry' @@ -106,20 +107,15 @@ describe('registerPtyHandlers', () => { seedHeadlessTerminal: vi.fn(), onPtyExit: vi.fn() } - const store = { - getWorkspaceSession: vi.fn(() => ({ - tabsByWorktree: { - [worktreeId]: [{ id: tabId, worktreeId, ptyId: 'pty-persisted-owner' }] - }, - terminalLayoutsByTabId: { - [tabId]: { ptyIdsByLeafId: { [leafId]: 'pty-persisted-owner' } } - }, - terminalPtyIncarnationsByPaneKey: { - [paneKey]: 'inc-stale-owner' - } - })), - persistPtyBinding: vi.fn(() => true) - } + const store = createPaneBindingStore() + await store.persistPtyBinding({ + worktreeId, + tabId, + leafId, + ptyId: 'pty-persisted-owner', + incarnationId: 'inc-stale-owner' + }) + store.persistPtyBinding.mockClear() registerPtyHandlers( mainWindow as never, @@ -161,9 +157,15 @@ describe('registerPtyHandlers', () => { command: undefined }) ) - expect(runtime.registerPreAllocatedHandleForPty).toHaveBeenCalledWith( + expect(runtime.registerPty).toHaveBeenCalledWith( 'pty-persisted-owner', - 'term-rebuilt-owner' + worktreeId, + null, + expect.objectContaining({ + tabId, + leafId, + incarnationId: 'inc-live-owner' + }) ) expect(runtime.noteTerminalSpawnCommand).not.toHaveBeenCalled() expect(store.persistPtyBinding).toHaveBeenCalledOnce() @@ -178,7 +180,8 @@ describe('registerPtyHandlers', () => { ptyId: 'pty-persisted-owner', incarnationId: 'inc-stale-owner' } - }) + }), + undefined ) expect( mainWindow.webContents.send.mock.calls.filter(([channel]) => channel === 'pty:spawned') diff --git a/src/main/ipc/pty-spawn-placement-threading.test.ts b/src/main/ipc/pty-spawn-placement-threading.test.ts index d54ef47711d..31db1f4733a 100644 --- a/src/main/ipc/pty-spawn-placement-threading.test.ts +++ b/src/main/ipc/pty-spawn-placement-threading.test.ts @@ -1,3 +1,4 @@ +import { createPaneBindingStore } from './pty-launch-race-test-fixture' import { describe, expect, it, vi } from 'vitest' import { setupPtyIpcSuite } from './pty-ipc-test-harness' import { makePaneKey } from '../../shared/stable-pane-id' @@ -65,10 +66,10 @@ describe('pty spawn placement threading', () => { const { handlers, mainWindow } = setupPtyIpcSuite() function register(): { - store: { persistPtyBinding: ReturnType } + store: ReturnType controller: () => RuntimeSpawnController } { - const store = { persistPtyBinding: vi.fn(async () => true) } + const store = createPaneBindingStore() let controller: RuntimeSpawnController | null = null const runtime = { setPtyController: vi.fn((value) => { @@ -120,16 +121,20 @@ describe('pty spawn placement threading', () => { it('passes a valid renderer placement to the binding write', async () => { const { store } = register() await ipcSpawn({ placement: SPLIT }) - expect(store.persistPtyBinding).toHaveBeenCalledWith( - expect.objectContaining({ tabId: 'tab-placement', leafId: LEAF, placement: SPLIT }) + expect(store.bindingWrites).toHaveBeenCalledWith( + expect.objectContaining({ + tabId: 'tab-placement', + leafId: LEAF, + placement: SPLIT + }) ) }) it('keeps an old client request (no placement) byte-for-byte as before', async () => { const { store } = register() await ipcSpawn({}) - expect(store.persistPtyBinding).toHaveBeenCalledOnce() - expect(store.persistPtyBinding.mock.calls[0]).toEqual([ + expect(store.bindingWrites).toHaveBeenCalledOnce() + expect(store.bindingWrites.mock.calls[0]).toEqual([ { worktreeId: WORKTREE, tabId: 'tab-placement', @@ -145,15 +150,28 @@ describe('pty spawn placement threading', () => { it('drops a malformed or future-kind placement instead of failing the spawn', async () => { const { store } = register() await ipcSpawn({ placement: { kind: 'floating-window', anchor: 'x' } }) - await ipcSpawn({ placement: { kind: 'split', parentLeafId: 'pane-1', direction: 'vertical' } }) - expect(store.persistPtyBinding).toHaveBeenCalledTimes(2) - for (const [binding] of store.persistPtyBinding.mock.calls) { + await ipcSpawn({ + tabId: 'tab-malformed-second', + placement: { + kind: 'split', + parentLeafId: 'pane-1', + direction: 'vertical' + } + }) + expect(store.bindingWrites).toHaveBeenCalledTimes(2) + for (const [binding] of store.bindingWrites.mock.calls) { expect(binding).not.toHaveProperty('placement') } }) it('passes a main runtime spawn placement through the host-admitted binding', async () => { const { store, controller } = register() + await store.admitTerminalPane({ + type: 'createTerminalTab', + workspace: WORKTREE, + tabId: 'tab-runtime', + leafId: LEAF + }) await controller().spawn({ cols: 80, rows: 24, @@ -164,7 +182,7 @@ describe('pty spawn placement threading', () => { persistHostSessionBinding: true, placement: SPLIT }) - expect(store.persistPtyBinding).toHaveBeenCalledWith({ + expect(store.bindingWrites).toHaveBeenCalledWith({ worktreeId: WORKTREE, tabId: 'tab-runtime', leafId: LEAF, diff --git a/src/main/ipc/pty/ipc/spawn-absent-binding-swap.ts b/src/main/ipc/pty/ipc/spawn-absent-binding-swap.ts index 69ab266e18f..c157facf894 100644 --- a/src/main/ipc/pty/ipc/spawn-absent-binding-swap.ts +++ b/src/main/ipc/pty/ipc/spawn-absent-binding-swap.ts @@ -4,18 +4,14 @@ import type { PtyIpcSpawnState } from './spawn-state' /** * Switch point: a gone process's binding stays until this spawn's bind swaps it, one write, so the - * pane is never published unbound. The held reservation keeps any other spawn off the pane meanwhile. + * pane is never published unbound. Stable-pane adoption holds the pane's reservation through its fresh + * launch, which keeps any other spawn off the pane meanwhile. * A pane on an SSH host keeps the early retire: its dead owner lives on that host. */ export function keepAbsentBindingForSwap( ctx: PtyIpcSpawnState ): ((dead: ReplacedPaneOwner) => void) | undefined { - if ( - !ORCA_LAYOUT_OWNER_IS_MAIN || - !ctx.paneSpawnReservation || - ctx.replacedPaneOwner || - ctx.args.connectionId - ) { + if (!ORCA_LAYOUT_OWNER_IS_MAIN || ctx.replacedPaneOwner || ctx.args.connectionId) { return undefined } return (dead) => { diff --git a/src/main/ipc/pty/ipc/spawn-begin.ts b/src/main/ipc/pty/ipc/spawn-begin.ts index 716ef585215..37f5f660297 100644 --- a/src/main/ipc/pty/ipc/spawn-begin.ts +++ b/src/main/ipc/pty/ipc/spawn-begin.ts @@ -11,8 +11,6 @@ import { snapshotCodexPaneHomeRoutes } from '../host-env/codex-home' import { getAppPtyId } from '../provider/registry' import { makePaneSpawnReservationKey, - reservePaneSpawn, - paneSpawnReservationsByOwnerKey, pendingRuntimePaneCreatesByOwnerKey } from '../pane/spawn-reservation' import { resolveStablePaneOwner } from '../pane/stable-owner' @@ -111,12 +109,6 @@ export async function beginPtyIpcSpawn( if (pendingRuntimeCreate) { await pendingRuntimeCreate.promise } - const existingPaneSpawn = earlyReservationKey - ? paneSpawnReservationsByOwnerKey.get(earlyReservationKey) - : undefined - if (existingPaneSpawn) { - return { ...(await existingPaneSpawn.promise), isReattach: true } - } } ctx.earlyStablePaneOwner = initialPaneKey && args.worktreeId @@ -134,9 +126,7 @@ export async function beginPtyIpcSpawn( ctx.earlyWorktreeId = args.worktreeId // Reserve early so renderer/runtime materialization cannot start duplicate provider spawns. ctx.paneSpawnReservationKey = earlyReservationKey - ctx.paneSpawnReservation = - heldReservation ?? - (ctx.paneSpawnReservationKey ? reservePaneSpawn(ctx.paneSpawnReservationKey) : null) + ctx.paneSpawnReservation = heldReservation ?? null ctx.finishTerminalInstall = (): void => {} ctx.stablePaneOwner = null ctx.stablePaneBindingPersisted = false diff --git a/src/main/ipc/pty/ipc/spawn-commit-persist.ts b/src/main/ipc/pty/ipc/spawn-commit-persist.ts index 226a55b11a4..883cf0583a0 100644 --- a/src/main/ipc/pty/ipc/spawn-commit-persist.ts +++ b/src/main/ipc/pty/ipc/spawn-commit-persist.ts @@ -1,8 +1,9 @@ import { closeStartupQueryAuthorityForPty } from '../provider/registry' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' import { recordCodexPaneAccountForSpawn } from '../host-env/codex-home' -import { persistAdmittedStablePaneBinding } from '../pane/stable-owner' import { swapReplacedPaneBinding } from '../pane/pane-owner-replacement' +import { PaneOwnerContentionError } from '../pane/pane-spawn-operation' +import { persistFreshPaneBinding } from '../runtime/spawn-commit' import { pendingByPaneKey, pendingPtyIdBySerializerGeneration, @@ -17,23 +18,7 @@ import { parseTerminalPanePlacement } from '../../../../shared/terminal-pane-pla export async function persistPtyIpcSpawnCommit(ctx: PtyIpcSpawnState): Promise { const args = ctx.args - try { - ctx.stablePaneBindingPersisted = await persistAdmittedStablePaneBinding({ - store: ctx.deps.store, - owner: ctx.stablePaneOwner, - result: ctx.result, - worktreeId: args.worktreeId, - startupCwd: ctx.cwd - }) - } catch (error) { - if (error instanceof Error && error.message === 'terminal_pane_owner_changed') { - throw error - } - console.error('[pty] failed to persist PTY binding after attach:', error) - throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage(error)), { - agentSessionOperationOutcome: 'unknown' as const - }) - } + const committedSize = resolveCommittedPtySize({ result: ctx.result, requested: { cols: args.cols, rows: args.rows }, @@ -44,8 +29,7 @@ export async function persistPtyIpcSpawnCommit(ctx: PtyIpcSpawnState): Promise

{ expect(outcome).toBe('discarded') expect(persistPtyBinding).toHaveBeenCalledOnce() expect(facts.read(PTY_ID, INCARNATION_ID).freshSpawn).toBe(false) - expect(stops.claimExit(PTY_ID, INCARNATION_ID)).toEqual(['reversible']) + expect(stops.claimExit(PTY_ID, INCARNATION_ID)).toEqual(['replaced']) }) }) diff --git a/src/main/ipc/pty/ipc/spawn-commit.ts b/src/main/ipc/pty/ipc/spawn-commit.ts index 666fcac2c73..22a34dc7dc3 100644 --- a/src/main/ipc/pty/ipc/spawn-commit.ts +++ b/src/main/ipc/pty/ipc/spawn-commit.ts @@ -60,6 +60,7 @@ async function commitReservedPtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { const args = ctx.args @@ -27,11 +30,25 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { ), ctx.replacedPaneOwner ) - const stablePaneOwnerCandidate = resolveOwner() - const expectedPtyId = - stablePaneOwnerCandidate?.ptyId ?? ctx.effectiveSessionAppId ?? ctx.effectiveSessionId + const stablePaneOwnerCandidate = excludeAbsentOwnerReplay( + resolveOwner(), + ctx.paneSpawnOperation?.absentOwner + ) + if (stablePaneOwnerCandidate) { + throw new PaneOwnerContentionError() + } + if (ctx.paneSpawnOperation?.mustExist && args.worktreeId && args.tabId && args.leafId) { + assertPaneStillExists( + ctx.deps.store, + args.worktreeId, + args.tabId, + args.leafId, + args.connectionId + ) + } + const expectedPtyId = ctx.effectiveSessionAppId ?? ctx.effectiveSessionId if (expectedPtyId) { - ctx.deps.runtime?.beginPtyRegistration?.(expectedPtyId) + ctx.pendingRegistrationClaim = ctx.deps.runtime?.beginPtyRegistration?.(expectedPtyId) ctx.pendingRegistrationPtyId = expectedPtyId } if (ctx.isDaemonHostSpawn && expectedPtyId) { @@ -47,38 +64,34 @@ export async function executePtyIpcSpawn(ctx: PtyIpcSpawnState): Promise { if (!ctx.preAdoptedStablePane && ctx.paneAgentLaunch) { ctx.paneAgentLaunch.dispatched = true } - const stablePaneSpawn = ctx.preAdoptedStablePane - ? ctx.preAdoptedStablePane - : await spawnForStablePane({ - runtime: ctx.deps.runtime, - store: ctx.deps.store, - provider: ctx.provider, - spawnOptions: ctx.spawnOptions, - owner: stablePaneOwnerCandidate, - worktreeId: args.worktreeId, - connectionId: args.connectionId, - resolveOwner, - keepAbsentBinding: keepAbsentBindingForSwap(ctx) - }) - ctx.result = stablePaneSpawn.result - ctx.stablePaneOwner = stablePaneSpawn.owner - if ( - ctx.stablePaneOwner && - ctx.isMintedSessionId && - ctx.effectiveSessionAppId && - ctx.effectiveSessionAppId !== ctx.result.id - ) { - clearProviderPtyState(ctx.effectiveSessionAppId) - } + ctx.result = await ctx.provider.spawn(ctx.spawnOptions) + ctx.stablePaneOwner = null ctx.rejectedRegistrationCandidate = ctx.result if (ctx.pendingRegistrationPtyId !== ctx.result.id) { if (ctx.pendingRegistrationPtyId) { - ctx.deps.runtime?.cancelPendingPtyRegistration?.(ctx.pendingRegistrationPtyId) + ctx.deps.runtime?.cancelPendingPtyRegistration?.( + ctx.pendingRegistrationPtyId, + undefined, + ctx.pendingRegistrationClaim + ) } - ctx.deps.runtime?.beginPtyRegistration?.(ctx.result.id, ctx.result.incarnationId) + ctx.pendingRegistrationClaim = ctx.deps.runtime?.beginPtyRegistration?.( + ctx.result.id, + ctx.result.incarnationId + ) ctx.pendingRegistrationPtyId = ctx.result.id } assertSpawnReplyWasLive(ctx.result) + if ( + ctx.pendingRegistrationClaim && + !ctx.deps.runtime?.narrowPtyRegistration( + ctx.result.id, + ctx.pendingRegistrationClaim, + ctx.result.incarnationId + ) + ) { + throw new PaneOwnerContentionError() + } ctx.deps.runtime?.assertPtyRegistrationAllowed?.(ctx.result.id, ctx.result.incarnationId) if (ctx.result.providerSequence) { const runtimeSequenceBeforeReconcile = diff --git a/src/main/ipc/pty/ipc/spawn-options.ts b/src/main/ipc/pty/ipc/spawn-options.ts index 31dfdfc6c17..e4408d760ab 100644 --- a/src/main/ipc/pty/ipc/spawn-options.ts +++ b/src/main/ipc/pty/ipc/spawn-options.ts @@ -1,24 +1,16 @@ import { isRestoreSpawnIntent } from '../../../../shared/host-spawn-admission' import { spawnWorktreeTarget } from '../../../../shared/worktree/host-qualified-identity' import { beginPtySpawnForWorktree } from '../host-env/fresh-spawn-routing' -import { - makePaneSpawnReservationKey, - reservePaneSpawn, - paneSpawnReservationsByOwnerKey, - pendingRuntimePaneCreatesByOwnerKey -} from '../pane/spawn-reservation' +import { makePaneSpawnReservationKey } from '../pane/spawn-reservation' import { assemblePtySpawnEnvDeletions, assemblePtySpawnLaunchOptions, - restoreProvisionalPtySize, seedPtySpawnSize } from '../spawn-options-assembly' import type { PtyIpcSpawnState } from './spawn-state' -/** Window lane: the shared spawn options plus this lane's pane reservation. */ -export async function buildPtyIpcSpawnOptions( - ctx: PtyIpcSpawnState -): Promise<{ isReattach: true } | null> { +/** Carries deletions to provider-owned environments, including persistent older daemons. */ +export async function buildPtyIpcSpawnOptions(ctx: PtyIpcSpawnState): Promise { const args = ctx.args const settings = ctx.deps.getSettings?.() const deletions = assemblePtySpawnEnvDeletions({ @@ -82,35 +74,7 @@ export async function buildPtyIpcSpawnOptions( ) { throw new Error('terminal_pane_identity_changed') } - if (!ctx.paneSpawnReservationKey) { - ctx.paneSpawnReservationKey = resolvedPaneSpawnReservationKey - const pendingRuntimeCreateAfterPreflight = ctx.paneSpawnReservationKey - ? pendingRuntimePaneCreatesByOwnerKey.get(ctx.paneSpawnReservationKey) - : undefined - if (pendingRuntimeCreateAfterPreflight) { - await pendingRuntimeCreateAfterPreflight.promise - } - const existingPaneSpawnAfterPreflight = ctx.paneSpawnReservationKey - ? paneSpawnReservationsByOwnerKey.get(ctx.paneSpawnReservationKey) - : undefined - if (existingPaneSpawnAfterPreflight) { - const reattach = { - ...(await existingPaneSpawnAfterPreflight.promise), - isReattach: true as const - } - restoreProvisionalPtySize({ - sessionId: ctx.effectiveSessionId, - sessionAppId: ctx.effectiveSessionAppId, - reattachId: reattach.id, - hadSessionSizeBeforeAttach: ctx.hadSessionSizeBeforeAttach, - sessionSizeBeforeAttach: ctx.sessionSizeBeforeAttach - }) - return reattach - } - ctx.paneSpawnReservation = ctx.paneSpawnReservationKey - ? reservePaneSpawn(ctx.paneSpawnReservationKey) - : null - } + ctx.paneSpawnReservationKey = resolvedPaneSpawnReservationKey ctx.finishTerminalInstall = beginPtySpawnForWorktree(args.worktreeId, ctx.cwd, args.connectionId) ctx.initiallyHidden = args.initiallyHidden === true // Why: daemon PTYs can emit before spawn() resolves, so the hidden mark must beat byte zero (terminal-query-authority.md §races); other providers are safe with the post-spawn mark below. @@ -130,5 +94,4 @@ export async function buildPtyIpcSpawnOptions( spawnWorktreeTarget(args.worktreeId, args.connectionId) ) : undefined - return null } diff --git a/src/main/ipc/pty/ipc/spawn-preflight.ts b/src/main/ipc/pty/ipc/spawn-preflight.ts index d318d175ec7..15d464f1931 100644 --- a/src/main/ipc/pty/ipc/spawn-preflight.ts +++ b/src/main/ipc/pty/ipc/spawn-preflight.ts @@ -9,7 +9,6 @@ import { import { getProvider } from '../provider/registry' import { resolvePtySpawnStartupTarget } from '../spawn-startup-target' import type { PtyIpcSpawnState } from './spawn-state' -import { keepAbsentBindingForSwap } from './spawn-absent-binding-swap' import { getConnectionExecutionHostId } from '../../../../shared/execution-host' export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promise { @@ -38,20 +37,7 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis } } ctx.spawnTiming.mark('stable_adoption_setup') - ctx.preAdoptedStablePane = - ctx.earlyStablePaneOwner && ctx.earlyWorktreeId - ? await ctx.deps.adoptStablePane({ - cols: args.cols, - rows: args.rows, - cwd: ctx.cwd, - connectionId: args.connectionId, - worktreeId: ctx.earlyWorktreeId, - tabId: ctx.earlyStablePaneOwner.tabId, - leafId: ctx.earlyStablePaneOwner.leafId, - ownsPaneSpawnReservation: true, - keepAbsentBinding: keepAbsentBindingForSwap(ctx) - }) - : null + ctx.preAdoptedStablePane = null ctx.spawnTiming.mark('stable_adoption') if (ctx.earlyStablePaneOwner && !ctx.preAdoptedStablePane) { const admission = ctx.deps.assertWorkspaceAcceptsNewPty(args.worktreeId, args.sessionId) diff --git a/src/main/ipc/pty/ipc/spawn-run.ts b/src/main/ipc/pty/ipc/spawn-run.ts index fd25968b02d..0405fef3e65 100644 --- a/src/main/ipc/pty/ipc/spawn-run.ts +++ b/src/main/ipc/pty/ipc/spawn-run.ts @@ -1,4 +1,11 @@ -import { rejectPaneSpawnReservation, reserveIdlePaneSpawn } from '../pane/spawn-reservation' +import { parsePaneKey } from '../../../../shared/stable-pane-id' +import { codexReattachedHomeRouteField } from '../host-env/codex-home' +import type { PaneSpawnOperation } from '../pane/pane-spawn-operation' +import { + rejectPaneSpawnReservation, + reserveIdlePaneSpawn, + pendingRuntimePaneCreatesByOwnerKey +} from '../pane/spawn-reservation' import { restoreProvisionalPtySize } from '../spawn-options-assembly' import { beginPtyIpcSpawn, @@ -6,6 +13,7 @@ import { resolveEarlyPaneSpawnReservationKey } from './spawn-begin' import { releaseStoppedPaneBinding, stopReplacedPaneOwner } from '../pane/pane-owner-replacement' +import { keepAbsentBindingForSwap } from './spawn-absent-binding-swap' import { planPaneAgentLaunch } from './spawn-agent-launch' import { endPaneAgentLaunch } from './spawn-agent-launch-end' import { preparePtyIpcSpawnPreflight } from './spawn-preflight' @@ -38,6 +46,90 @@ export async function runPtyIpcSpawn(deps: PtySpawnIpcDeps, args: PtySpawnIpcArg ctx.paneSpawnReservation = await reserveIdlePaneSpawn(replacedPaneKey) ctx.paneSpawnReservationKey = replacedPaneKey } + const fresh = async (operation: PaneSpawnOperation) => { + // Retire the previous preparation before replacing its resource slots or reacquiring its lease. + releaseAbandonedAgentTeamsLeader(ctx) + ctx.finishTerminalInstall() + ctx.finishTerminalInstall = () => {} + ctx.releaseWorktreeSpawn?.() + ctx.releaseWorktreeSpawn = undefined + ctx.paneSpawnOperation = operation + ctx.paneSpawnReservation = null + ctx.earlyStablePaneOwner = null + // Awaited only for a launch: an ordinary spawn reaches its provider in the same turn as before. + if (ctx.args.agentLaunch !== undefined) { + await planPaneAgentLaunch(ctx) + } + await preparePtyIpcSpawnPreflight(ctx) + await assemblePtyIpcSpawnEnv(ctx) + await preparePtyIpcQoderCommand(ctx) + await buildPtyIpcSpawnOptions(ctx).catch((error: unknown) => { + restoreProvisionalPtySize({ + sessionId: ctx.effectiveSessionId, + sessionAppId: ctx.effectiveSessionAppId, + hadSessionSizeBeforeAttach: ctx.hadSessionSizeBeforeAttach, + sessionSizeBeforeAttach: ctx.sessionSizeBeforeAttach + }) + throw error + }) + const complete = async (scope: PaneSpawnOperation) => { + ctx.paneSpawnOperation = scope + // Right before the provider spawn, with nothing awaited in between: a closed pane starts nothing. + ctx.paneAgentLaunch?.refuseIfClosed() + // Switch point: a pane main closed or took back while this spawn waited starts nothing. + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + refuseClosedPaneSpawn(deps.store, args.tabId, args.leafId) + } + try { + await executePtyIpcSpawn(ctx) + const committed = await commitPtyIpcSpawn(ctx) + endPaneAgentLaunch( + ctx, + ctx.exitedBeforeRegistration ? { exitedDuringStart: true } : { started: true } + ) + return committed + } finally { + if (ctx.pendingRegistrationPtyId) { + deps.runtime?.cancelPendingPtyRegistration?.( + ctx.pendingRegistrationPtyId, + ctx.rejectedRegistrationCandidate?.incarnationId, + ctx.pendingRegistrationClaim + ) + ctx.pendingRegistrationPtyId = null + } + } + } + const latePane = + !resolveEarlyPaneKey(args) && ctx.reservationPaneKey + ? parsePaneKey(ctx.reservationPaneKey) + : null + if (latePane) { + const pendingCreate = ctx.paneSpawnReservationKey + ? pendingRuntimePaneCreatesByOwnerKey.get(ctx.paneSpawnReservationKey) + : undefined + if (pendingCreate) { + await pendingCreate.promise + } + ctx.args = { ...ctx.args, ...latePane } + let completed: Awaited> | undefined + const adopted = await deps.adoptStablePane({ + cols: args.cols, + rows: args.rows, + cwd: ctx.cwd, + worktreeId: args.worktreeId, + ...latePane, + connectionId: args.connectionId, + renderer: true, + keepAbsentBinding: keepAbsentBindingForSwap(ctx), + spawnFresh: async (scope) => (completed = await complete(scope)) + }) + if (adopted) { + releaseAbandonedAgentTeamsLeader(ctx) + } + return adopted?.result ?? completed! + } + return complete(operation) + } try { if (args.replacesPtyId !== undefined) { ctx.replacedPaneOwner = await stopReplacedPaneOwner(deps, { @@ -52,43 +144,42 @@ export async function runPtyIpcSpawn(deps: PtySpawnIpcDeps, args: PtySpawnIpcArg if (early) { return early } - // Awaited only for a launch: an ordinary spawn reaches its provider in the same turn as before. - if (ctx.args.agentLaunch !== undefined) { - await planPaneAgentLaunch(ctx) + const paneKey = resolveEarlyPaneKey(args) + if (!paneKey || !args.worktreeId || !args.tabId || !args.leafId) { + return await fresh({ mustExist: false }) } - await preparePtyIpcSpawnPreflight(ctx) - await assemblePtyIpcSpawnEnv(ctx) - await preparePtyIpcQoderCommand(ctx) - const earlyReserved = await buildPtyIpcSpawnOptions(ctx).catch((error: unknown) => { - restoreProvisionalPtySize({ - sessionId: ctx.effectiveSessionId, - sessionAppId: ctx.effectiveSessionAppId, - hadSessionSizeBeforeAttach: ctx.hadSessionSizeBeforeAttach, - sessionSizeBeforeAttach: ctx.sessionSizeBeforeAttach - }) - throw error + let freshResult: Awaited> | undefined + const adopted = await deps.adoptStablePane({ + cols: args.cols, + rows: args.rows, + cwd: ctx.cwd, + worktreeId: args.worktreeId, + tabId: args.tabId, + leafId: args.leafId, + connectionId: args.connectionId, + renderer: true, + reservation: ctx.paneSpawnReservation ?? undefined, + stoppedPtyId: ctx.replacedPaneOwner?.ptyId, + keepAbsentBinding: keepAbsentBindingForSwap(ctx), + spawnFresh: async (operation) => (freshResult = await fresh(operation)) }) - if (earlyReserved) { - // Why: this request lost the pane to the reservation winner, so its - // pre-allocated leader handle never binds to a PTY. Nothing else can - // evict the team env assembly created for it — exit/close cleanup keys - // off handleByPtyId — so every lost race would leak one team forever. + if (adopted) { releaseAbandonedAgentTeamsLeader(ctx) - return earlyReserved + if (ctx.preSpawnHiddenMarkId !== null) { + deps.transitionSpawnHiddenRendererPtyDeliveryState(ctx.preSpawnHiddenMarkId, false) + } + deps.options?.onCodexHomePtySpawned?.({ + id: adopted.result.id, + codexHomePath: null, + reattached: true, + startedAt: ctx.codexHomeLaunchStartedAt, + startedSequence: ctx.codexHomeLaunchStartedSequence, + ...codexReattachedHomeRouteField(ctx.reattachedCodexHomeRoutes, adopted.result.id, true) + }) + endPaneAgentLaunch(ctx, { started: true }) + return adopted.result } - // Right before the provider spawn, with nothing awaited in between: a closed pane starts nothing. - ctx.paneAgentLaunch?.refuseIfClosed() - // Switch point: a pane main closed or took back while this spawn waited starts nothing. - if (ORCA_LAYOUT_OWNER_IS_MAIN) { - refuseClosedPaneSpawn(deps.store, args.tabId, args.leafId) - } - await executePtyIpcSpawn(ctx) - const committed = await commitPtyIpcSpawn(ctx) - endPaneAgentLaunch( - ctx, - ctx.exitedBeforeRegistration ? { exitedDuringStart: true } : { started: true } - ) - return committed + return freshResult! } catch (caught) { const err = endPaneAgentLaunch(ctx, { error: caught }) ?? caught releaseAbandonedAgentTeamsLeader(ctx) @@ -98,7 +189,8 @@ export async function runPtyIpcSpawn(deps: PtySpawnIpcDeps, args: PtySpawnIpcArg if (ctx.pendingRegistrationPtyId) { deps.runtime?.cancelPendingPtyRegistration?.( ctx.pendingRegistrationPtyId, - ctx.rejectedRegistrationCandidate?.incarnationId + ctx.rejectedRegistrationCandidate?.incarnationId, + ctx.pendingRegistrationClaim ) ctx.pendingRegistrationPtyId = null } diff --git a/src/main/ipc/pty/ipc/spawn-state.ts b/src/main/ipc/pty/ipc/spawn-state.ts index b6abdf5b980..e0d7d2ffe01 100644 --- a/src/main/ipc/pty/ipc/spawn-state.ts +++ b/src/main/ipc/pty/ipc/spawn-state.ts @@ -1,3 +1,5 @@ +import type { PtyRegistrationClaim } from '../../../runtime/runtime-terminal-state-records' +import type { PaneSpawnOperation } from '../pane/pane-spawn-operation' import type { IPtyProvider, PtySpawnOptions, @@ -34,10 +36,12 @@ export type PtyIpcSpawnState = { replacedPaneOwner: ReplacedPaneOwner | null finishTerminalInstall: () => void result: PtySpawnResult + paneSpawnOperation?: PaneSpawnOperation stablePaneOwner: StablePaneOwner | null stablePaneBindingPersisted: boolean rejectedRegistrationCandidate: PtySpawnResult | null pendingRegistrationPtyId: string | null + pendingRegistrationClaim?: PtyRegistrationClaim reconciledSnapshotSeq: number | null snapshotKittyFlagsCoverReconciledSeq: boolean preparedProvisionalExecutionContext: boolean diff --git a/src/main/ipc/pty/ipc/spawn-types.ts b/src/main/ipc/pty/ipc/spawn-types.ts index 2185d2a00d2..175ed5ac2f3 100644 --- a/src/main/ipc/pty/ipc/spawn-types.ts +++ b/src/main/ipc/pty/ipc/spawn-types.ts @@ -1,3 +1,5 @@ +import type { PaneSpawnReservation } from '../pane/spawn-reservation' +import type { PaneSpawnOperation } from '../pane/pane-spawn-operation' import type { TuiAgent } from '../../../../shared/tui-agent' import type { AgentProviderSessionMetadata, @@ -72,12 +74,19 @@ export type AdoptStablePaneArgs = { rows: number cwd?: string connectionId?: string | null - worktreeId: string + worktreeId?: string preAllocatedHandle?: string tabId: string leafId: string + onAdopted?: (result: PtySpawnResult) => void + completed?: AdoptStablePaneResult ownsPaneSpawnReservation?: true - /** See StablePaneSpawnContext; only with `ownsPaneSpawnReservation`. */ + reservation?: PaneSpawnReservation + stoppedPtyId?: string + renderer?: boolean + signal?: AbortSignal + spawnFresh?: (operation: PaneSpawnOperation) => Promise + /** See StablePaneSpawnContext; adoption holds the pane's reservation through its fresh launch. */ keepAbsentBinding?: (dead: ReplacedPaneOwner) => void } diff --git a/src/main/ipc/pty/pane/absent-pane-owner.ts b/src/main/ipc/pty/pane/absent-pane-owner.ts index 262dd14d953..b8705283c96 100644 --- a/src/main/ipc/pty/pane/absent-pane-owner.ts +++ b/src/main/ipc/pty/pane/absent-pane-owner.ts @@ -2,7 +2,7 @@ import { toSshExecutionHostId } from '../../../../shared/execution-host' import { UNVERIFIED_PROCESS_EXIT_CODE } from '../../../../shared/terminal-exit-cause' import type { Store } from '../../../persistence' import type { OrcaRuntimeService } from '../../../runtime/orca-runtime' -import { ptyOwnership } from '../provider/ownership-state' +import { ptyIncarnationById, ptyOwnership } from '../provider/ownership-state' import { clearProviderPtyState } from '../provider/state-cleanup' import { retireReplacedPaneBinding } from './pane-owner-replacement' import type { ReplacedPaneOwner } from './replaced-pane-owner' @@ -13,7 +13,20 @@ type AbsentPaneContext = { worktreeId?: string connectionId?: string | null } -type AbsentPaneOwner = { tabId: string; leafId: string; ptyId: string; incarnationId?: string } +type AbsentPaneOwner = { + tabId: string + leafId: string + ptyId: string + incarnationId?: string + runtimeIncarnationId?: string + persistedIncarnationId?: string +} + +// A newer incarnation of this id committed after the host's answer; it is not the absent process. +function isSupersededAbsentOwner(owner: AbsentPaneOwner): boolean { + const liveIncarnationId = ptyIncarnationById.get(owner.ptyId) + return liveIncarnationId !== undefined && liveIncarnationId !== owner.runtimeIncarnationId +} /** Ends the dead process's in-memory state; its durable binding is the caller's to clear. */ function releaseAbsentPaneProcess( @@ -37,9 +50,13 @@ function absentPaneReplacement( worktreeId: string, connectionId: string | null | undefined, owner: AbsentPaneOwner -): ReplacedPaneOwner & { pane: NonNullable } { +): ReplacedPaneOwner & { + pane: NonNullable + absenceFence: { incarnationId: string | undefined } +} { return { ptyId: owner.ptyId, + absenceFence: { incarnationId: owner.persistedIncarnationId }, pane: { worktreeId, tabId: owner.tabId, @@ -59,6 +76,9 @@ export async function releaseAbsentPaneOwner( owner: AbsentPaneOwner ): Promise { const { runtime, store, worktreeId, connectionId } = context + if (isSupersededAbsentOwner(owner)) { + return false + } releaseAbsentPaneProcess(runtime, owner) if (!worktreeId) { return true @@ -79,7 +99,7 @@ export function keepAbsentPaneBindingForSwap( owner: AbsentPaneOwner ): ReplacedPaneOwner | null { const { runtime, store, worktreeId, connectionId } = context - if (!worktreeId || !store) { + if (!worktreeId || !store || isSupersededAbsentOwner(owner)) { return null } releaseAbsentPaneProcess(runtime, owner) diff --git a/src/main/ipc/pty/pane/adopt-stable.ts b/src/main/ipc/pty/pane/adopt-stable.ts index b1fa6b83285..a572071ae22 100644 --- a/src/main/ipc/pty/pane/adopt-stable.ts +++ b/src/main/ipc/pty/pane/adopt-stable.ts @@ -1,3 +1,5 @@ +import { ptyIncarnationById } from '../provider/ownership-state' +import { reconcilePaneOwner } from './stable-pane-attach' import type { OrcaRuntimeService } from '../../../runtime/orca-runtime' import type { Store } from '../../../persistence' import { makePaneKey } from '../../../../shared/stable-pane-id' @@ -6,32 +8,28 @@ import { getConnectionExecutionHostId } from '../../../../shared/execution-host' import { joinPaneSpawn, makePaneSpawnReservationKey, - paneSpawnReservationsByOwnerKey + paneSpawnReservationsByOwnerKey, + reservePaneSpawn, + resolvePaneSpawnReservation, + rejectPaneSpawnReservation } from './spawn-reservation' -import { - attachStablePaneOwner, - resolveStablePaneOwner, - stablePaneAdoptionsByOwnerKey -} from './stable-owner' +import { resolveStablePaneOwner } from './stable-owner' +import { completePaneAdoption } from './complete-pane-adoption' +import { assertPaneStillExists, type AbsentPaneBinding } from './pane-spawn-operation' import type { AdoptStablePaneArgs, AdoptStablePaneResult } from '../ipc/spawn-types' +/** Owns attachment through publication, including the lazy fresh path, under one reservation. */ export async function adoptStablePane( runtime: OrcaRuntimeService | undefined, store: Store | undefined, args: AdoptStablePaneArgs ): Promise { - const paneKey = makePaneKey(args.tabId, args.leafId) - const ownerKey = makePaneSpawnReservationKey(args.worktreeId, args.connectionId, paneKey) - const pendingAdoption = ownerKey ? stablePaneAdoptionsByOwnerKey.get(ownerKey) : undefined - if (pendingAdoption) { - return await pendingAdoption + if (args.signal?.aborted) { + throw new Error('client_disconnected') } - const activePaneSpawn = - ownerKey && !args.ownsPaneSpawnReservation - ? paneSpawnReservationsByOwnerKey.get(ownerKey) - : undefined - const result = activePaneSpawn ? await joinPaneSpawn(activePaneSpawn) : null - if (result) { + const paneKey = makePaneKey(args.tabId, args.leafId) + const key = makePaneSpawnReservationKey(args.worktreeId, args.connectionId, paneKey)! + const resolveOwner = () => { const owner = resolveStablePaneOwner( runtime, store, @@ -39,54 +37,141 @@ export async function adoptStablePane( args.worktreeId, args.connectionId ) - if ( - !owner || - owner.ptyId !== result.id || - (owner.incarnationId !== undefined && - result.incarnationId !== undefined && - owner.incarnationId !== result.incarnationId) - ) { + return owner?.ptyId === args.stoppedPtyId ? null : owner + } + let remaining = 1 + if (args.completed?.materialized) { + const owner = resolveOwner() + if (!owner) { throw new Error('terminal_pane_owner_changed') } - return { - result: { - ...result, - isReattach: true, - ...(owner.incarnationId ? { incarnationId: owner.incarnationId } : {}) - }, - owner, - materialized: true as const + if (args.worktreeId && typeof store?.getWorkspaceSession === 'function') { + assertPaneStillExists(store, args.worktreeId, args.tabId, args.leafId, args.connectionId) + } + if ( + owner.ptyId === args.completed.result.id && + owner.incarnationId === args.completed.result.incarnationId + ) { + return args.completed + } + remaining = 0 + } + let reservation = args.reservation + if (!reservation) { + for (;;) { + const pending = paneSpawnReservationsByOwnerKey.get(key) + if (!pending) { + reservation = reservePaneSpawn(key) + break + } + const joined = await joinPaneSpawn(pending) + if (args.signal?.aborted) { + throw new Error('client_disconnected') + } + if (!joined) { + continue + } + if (!args.worktreeId) { + const successor = paneSpawnReservationsByOwnerKey.get(key) + if (successor && successor !== pending) { + if (remaining-- <= 0) { + throw new Error('terminal_pane_owner_changed') + } + continue + } + const incarnation = ptyIncarnationById.get(joined.id) + if ( + joined.incarnationId !== undefined && + incarnation !== undefined && + incarnation !== joined.incarnationId + ) { + throw new Error('terminal_pane_owner_changed') + } + return { + result: { ...joined, isReattach: true }, + owner: { ptyId: joined.id, tabId: args.tabId, leafId: args.leafId }, + materialized: true + } + } + const owner = resolveOwner() + if (!owner) { + throw new Error('terminal_pane_owner_changed') + } + if (args.worktreeId && typeof store?.getWorkspaceSession === 'function') { + assertPaneStillExists(store, args.worktreeId, args.tabId, args.leafId, args.connectionId) + } + if (owner.ptyId === joined.id && owner.incarnationId === joined.incarnationId) { + return { result: { ...joined, isReattach: true }, owner, materialized: true } + } + if (remaining-- <= 0) { + throw new Error('terminal_pane_owner_changed') + } + // Recheck after the await; a new holder may already have reserved the pane. } } - const owner = resolveStablePaneOwner(runtime, store, paneKey, args.worktreeId, args.connectionId) - if (!owner) { - return null - } - const adoption = attachStablePaneOwner({ - runtime, - store, - provider: getProvider(getConnectionExecutionHostId(args.connectionId)), - spawnOptions: { - cols: args.cols, - rows: args.rows, - cwd: args.cwd - }, - owner, - worktreeId: args.worktreeId, - connectionId: args.connectionId, - resolveOwner: () => - resolveStablePaneOwner(runtime, store, paneKey, args.worktreeId, args.connectionId), - keepAbsentBinding: args.ownsPaneSpawnReservation ? args.keepAbsentBinding : undefined - }) - if (!ownerKey) { - return await adoption - } - stablePaneAdoptionsByOwnerKey.set(ownerKey, adoption) + let mustExist = false + let absentOwner: AbsentPaneBinding | undefined try { - return await adoption - } finally { - if (stablePaneAdoptionsByOwnerKey.get(ownerKey) === adoption) { - stablePaneAdoptionsByOwnerKey.delete(ownerKey) + if (args.signal?.aborted) { + throw new Error('client_disconnected') } + return await reconcilePaneOwner({ + owner: resolveOwner(), + resolveOwner, + losses: 1 - remaining, + attach: async (owner) => { + if (!args.worktreeId) { + throw new Error('terminal_pane_owner_changed') + } + mustExist = true + absentOwner = undefined + if (args.worktreeId && typeof store?.getWorkspaceSession === 'function') { + assertPaneStillExists(store, args.worktreeId, args.tabId, args.leafId, args.connectionId) + } + const adopted = await completePaneAdoption({ + runtime, + store, + owner, + worktreeId: args.worktreeId, + connectionId: args.connectionId, + provider: getProvider(getConnectionExecutionHostId(args.connectionId)), + spawnOptions: { cols: args.cols, rows: args.rows, cwd: args.cwd, signal: args.signal }, + resolveOwner, + preAllocatedHandle: args.preAllocatedHandle, + renderer: args.renderer, + keepAbsentBinding: args.keepAbsentBinding + }) + if (adopted) { + args.onAdopted?.(adopted.result) + resolvePaneSpawnReservation(key, reservation, adopted.result) + } else { + absentOwner = { + ptyId: owner.ptyId, + incarnationIds: [owner.runtimeIncarnationId, owner.persistedIncarnationId].filter( + (id): id is string => id !== undefined + ) + } + } + return adopted + }, + fresh: async () => { + if (args.signal?.aborted) { + throw new Error('client_disconnected') + } + if (mustExist && args.worktreeId) { + assertPaneStillExists(store, args.worktreeId, args.tabId, args.leafId, args.connectionId) + } + if (!args.spawnFresh) { + rejectPaneSpawnReservation(key, reservation, new Error('terminal_pane_unbound')) + return null + } + const result = await args.spawnFresh({ mustExist, ...(absentOwner ? { absentOwner } : {}) }) + resolvePaneSpawnReservation(key, reservation, result) + return null + } + }) + } catch (error) { + rejectPaneSpawnReservation(key, reservation, error) + throw error } } diff --git a/src/main/ipc/pty/pane/complete-pane-adoption.ts b/src/main/ipc/pty/pane/complete-pane-adoption.ts new file mode 100644 index 00000000000..7f5f032f38b --- /dev/null +++ b/src/main/ipc/pty/pane/complete-pane-adoption.ts @@ -0,0 +1,191 @@ +import { + isNativeWindowsLocalPtySpawn, + markNativeWindowsConptyPty +} from '../../../runtime/terminal-model-query-authority' +import { registerPty } from '../../../memory/pty-registry' +import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import type { StablePaneSpawnContext, StablePaneOwner } from './stable-owner' +import { persistAdmittedStablePaneBinding } from './stable-owner' +import { attemptStablePaneAttach } from './stable-pane-attach' +import { PaneOwnerContentionError } from './pane-spawn-operation' +import { admitPtySpawnOwnership, registerPersistedPtySpawn } from './spawn-registration' +import { + assertSpawnReplyWasLive, + seedTerminalRestoreRecordsFromSpawnResult +} from './agent-session-owners' +import { seedHeadlessTerminalFromSpawnResult } from './terminal-spawn-restore' +import { admitProviderReattachLaunchIdentity } from './launch-authority' +import { ensureWslHookRelayForReattach } from '../../../agent-hooks/wsl-hook-relay-reattach' +import { ptySizes } from '../delivery/visibility-state' +import { + resolveCommittedPtySize, + reflowHeadlessTerminalToCommittedGrid +} from '../delivery/attached-pty-size' +import { rememberPaneKeyForPty } from './key-state' +import { + pendingByPaneKey, + pendingPtyIdBySerializerGeneration, + rendererSerializerReadiness +} from './serializer-state' + +export async function completePaneAdoption( + args: StablePaneSpawnContext & { + owner: StablePaneOwner + worktreeId: string + preAllocatedHandle?: string + renderer?: boolean + } +) { + const { runtime, owner } = args + const paneKey = makePaneKey(owner.tabId, owner.leafId) + const cachedBeforeAttach = ptySizes.get(owner.ptyId) + const claim = runtime?.beginPtyRegistration?.(owner.ptyId, owner.runtimeIncarnationId) + runtime?.preparePtyExecutionContext?.(owner.ptyId, null, { + preserveExisting: true + }) + const beforeAttach = runtime?.getPtyOutputSequence?.(owner.ptyId) ?? 0 + let incarnationId = owner.incarnationId + try { + const attempt = await attemptStablePaneAttach(args) + if ('lostTo' in attempt) { + throw new PaneOwnerContentionError() + } + if ('absent' in attempt) { + return null + } + const result = attempt.attached.result + if (args.spawnOptions.signal?.aborted) { + throw new Error('client_disconnected') + } + incarnationId = result.incarnationId + const assertClaim = () => { + if (claim && !runtime?.narrowPtyRegistration(result.id, claim, result.incarnationId)) { + throw new PaneOwnerContentionError() + } + } + assertClaim() + assertSpawnReplyWasLive(result) + runtime?.assertPtyRegistrationAllowed?.(result.id, result.incarnationId) + const beforeReconcile = runtime?.getPtyOutputSequence?.(result.id) ?? 0 + const snapshotSeq = result.providerSequence + ? runtime?.synchronizePtyOutputSequenceFromProvider?.( + result.id, + result.providerSequence, + beforeAttach + ) + : undefined + ensureWslHookRelayForReattach(result, args.connectionId) + runtime?.preparePtyExecutionContext?.(result.id, result.wslDistro ?? null, { + preserveExisting: result.wslDistro === undefined + }) + admitPtySpawnOwnership(runtime, result) + if ( + !result.wslDistro && + isNativeWindowsLocalPtySpawn({ + connectionId: args.connectionId, + cwd: args.spawnOptions.cwd, + shellOverride: args.spawnOptions.shellOverride + }) + ) { + markNativeWindowsConptyPty(result.id) + } + seedHeadlessTerminalFromSpawnResult(runtime, result, paneKey) + seedTerminalRestoreRecordsFromSpawnResult(runtime, result) + try { + await persistAdmittedStablePaneBinding({ + store: args.store, + owner, + result, + worktreeId: args.worktreeId, + startupCwd: args.spawnOptions.cwd, + connectionId: args.connectionId + }) + } catch (error) { + if ( + error instanceof PaneOwnerContentionError || + (error instanceof Error && error.message === 'terminal_pane_owner_changed') + ) { + throw new PaneOwnerContentionError() + } + throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage(error)), { + agentSessionOperationOutcome: 'unknown' as const + }) + } + assertClaim() + const terminalHandle = owner.handle ?? args.preAllocatedHandle + const rejected = registerPersistedPtySpawn( + runtime, + args.store, + claim, + result.id, + args.worktreeId, + args.connectionId ?? null, + { + tabId: owner.tabId, + leafId: owner.leafId, + ...(terminalHandle ? { terminalHandle } : {}), + ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}), + providerReattachLaunchIdentity: admitProviderReattachLaunchIdentity(result) ?? undefined + } + ) + if (rejected) { + try { + await rejected + } catch (error) { + if ( + !args.renderer || + !(error instanceof Error) || + error.message !== 'agent_session_exited_during_start' + ) { + throw error + } + return { result, owner, materialized: true as const } + } + } + runtime?.noteTerminalSpawnCommit?.(result) + if (!owner.handle && terminalHandle) { + runtime?.registerPreAllocatedHandleForPty?.(result.id, terminalHandle) + } + const committedSize = resolveCommittedPtySize({ + result, + requested: args.spawnOptions, + cachedBeforeAttach + }) + ptySizes.set(result.id, committedSize) + reflowHeadlessTerminalToCommittedGrid({ + result, + committedSize, + reflowHeadlessTerminalToPtyGrid: runtime?.reflowHeadlessTerminalToPtyGrid?.bind(runtime) + }) + seedTerminalRestoreRecordsFromSpawnResult(runtime, result) + rememberPaneKeyForPty(result.id, paneKey) + registerPty({ + ptyId: result.id, + worktreeId: args.worktreeId, + sessionId: result.id, + paneKey, + pid: result.pid ?? null + }) + const pending = pendingByPaneKey.get(paneKey) + rendererSerializerReadiness.beginIncarnation( + result.id, + !pending && rendererSerializerReadiness.has(result.id) + ) + if (pending) { + pendingPtyIdBySerializerGeneration.set(pending.gen, result.id) + } + return { + result: { + ...result, + ...(typeof snapshotSeq === 'number' && beforeReconcile <= beforeAttach + ? { snapshotSeq } + : { snapshotKittyKeyboardFlags: undefined }) + }, + owner: { ...owner, ...(terminalHandle ? { handle: terminalHandle } : {}) }, + materialized: true as const + } + } finally { + runtime?.cancelPendingPtyRegistration?.(owner.ptyId, incarnationId, claim) + } +} diff --git a/src/main/ipc/pty/pane/pane-owner-replacement.ts b/src/main/ipc/pty/pane/pane-owner-replacement.ts index 45b4079a722..46f022ff0e1 100644 --- a/src/main/ipc/pty/pane/pane-owner-replacement.ts +++ b/src/main/ipc/pty/pane/pane-owner-replacement.ts @@ -71,10 +71,14 @@ export function swapReplacedPaneBinding( /** Clears a stopped id's binding and keeps the pane; false when another owner took the pane. */ export async function retireReplacedPaneBinding( store: Store, - replaced: ReplacedPaneOwner & { pane: NonNullable } + replaced: ReplacedPaneOwner & { + pane: NonNullable + /** Also fence on the saved incarnation, for an id the host reported absent rather than stopped. */ + absenceFence?: { incarnationId: string | undefined } + } ): Promise { const { hostId, ...pane } = replaced.pane - return store.retirePtyBinding({ ...pane, ptyId: replaced.ptyId }, hostId) + return store.retirePtyBinding({ ...pane, ptyId: replaced.ptyId }, hostId, replaced.absenceFence) } /** Best effort: a failed replacement must not leave the stopped id bound for the remount to reattach. */ diff --git a/src/main/ipc/pty/pane/pane-spawn-operation.ts b/src/main/ipc/pty/pane/pane-spawn-operation.ts new file mode 100644 index 00000000000..d2c9d2447fa --- /dev/null +++ b/src/main/ipc/pty/pane/pane-spawn-operation.ts @@ -0,0 +1,65 @@ +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { toSshExecutionHostId } from '../../../../shared/execution-host' +import type { Store } from '../../../persistence' +import { ptyBindingIsRefused } from '../../../persistence/loading-store/pty-binding-refusals' + +export class PaneOwnerContentionError extends Error { + constructor() { + super('terminal_pane_owner_changed') + } +} + +/** A binding the operation's host answer proved dead, with every incarnation it was known by. */ +export type AbsentPaneBinding = { ptyId: string; incarnationIds: readonly string[] } + +/** In-process authority of the enclosing operation; never sent over IPC/RPC or saved. */ +export type PaneSpawnOperation = { mustExist: boolean; absentOwner?: AbsentPaneBinding } + +// Why: a window save can replay the binding the operation just proved dead; that is not an owner. +export function isAbsentBindingReplay( + absent: AbsentPaneBinding | undefined, + ptyId: string, + incarnationId: string | undefined +): boolean { + return ( + absent?.ptyId === ptyId && + (incarnationId === undefined || absent.incarnationIds.includes(incarnationId)) + ) +} + +/** Null when the resolved owner is only that replay. */ +export function excludeAbsentOwnerReplay< + T extends { ptyId: string; persistedIncarnationId?: string; runtimeIncarnationId?: string } +>(owner: T | null, absent: AbsentPaneBinding | undefined): T | null { + return owner && + isAbsentBindingReplay(absent, owner.ptyId, owner.persistedIncarnationId) && + isAbsentBindingReplay(absent, owner.ptyId, owner.runtimeIncarnationId) + ? null + : owner +} + +export function assertPaneStillExists( + store: Store | undefined, + worktreeId: string, + tabId: string, + leafId: string, + connectionId?: string | null +): void { + if (!store) { + throw new Error('terminal_pane_owner_changed') + } + const session = store.getWorkspaceSession( + connectionId ? toSshExecutionHostId(connectionId) : undefined + ) + if ( + ptyBindingIsRefused( + { tabId, leafId, mayCreate: false }, + session, + worktreeId, + makePaneKey(tabId, leafId), + [] + ) + ) { + throw new Error('terminal_pane_owner_changed') + } +} diff --git a/src/main/ipc/pty/pane/spawn-registration.test.ts b/src/main/ipc/pty/pane/spawn-registration.test.ts index 0481252f2d7..527ee25a36b 100644 --- a/src/main/ipc/pty/pane/spawn-registration.test.ts +++ b/src/main/ipc/pty/pane/spawn-registration.test.ts @@ -1,9 +1,11 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi, type Mock } from 'vitest' import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' import type { IPtyProvider, PtySpawnResult } from '../../../providers/pty-provider-contract' import { ptyIncarnationById, ptyOwnership } from '../provider/ownership-state' +import { TerminalIntentionalStops } from '../../../runtime/terminal-intentional-stops' import { admitPtySpawnOwnership, discardUnpersistedPtySpawn } from './spawn-registration' +type MarkUnverifiable = (ptyId: string, reason: string) => void const PTY_ID = 'unpersisted-spawn' const result: PtySpawnResult = { id: PTY_ID, incarnationId: 'inc-1' } @@ -21,7 +23,10 @@ describe('discarding a spawn whose binding failed to save', () => { function discard( shutdown: () => Promise, - runtime = { markPtyLivenessUnverifiable: vi.fn() } + runtime: { + markPtyLivenessUnverifiable: Mock + intentionalPtyStops?: TerminalIntentionalStops + } = { markPtyLivenessUnverifiable: vi.fn() } ): Promise { admitPtySpawnOwnership(undefined, result) return discardUnpersistedPtySpawn(providerWhoseShutdown(shutdown), result, runtime).then( @@ -40,11 +45,17 @@ describe('discarding a spawn whose binding failed to save', () => { ) }) - it('retires the PTY once the host acknowledged the shutdown', async () => { - const runtime = await discard(() => Promise.resolve()) + // QA687: an unmarked discard's exit read as the agent ending and removed the user's tab. + it('retires the PTY once the host acknowledged the shutdown, as a stop that keeps the tab', async () => { + const intentionalPtyStops = new TerminalIntentionalStops() + const runtime = await discard(() => Promise.resolve(), { + markPtyLivenessUnverifiable: vi.fn(), + intentionalPtyStops + }) expect(ptyOwnership.has(PTY_ID)).toBe(false) expect(runtime.markPtyLivenessUnverifiable).not.toHaveBeenCalled() + expect(intentionalPtyStops.claimExit(PTY_ID, 'inc-1')).toEqual(['replaced']) }) it('leaves a same-id replacement that arrived during the shutdown alone', async () => { diff --git a/src/main/ipc/pty/pane/spawn-registration.ts b/src/main/ipc/pty/pane/spawn-registration.ts index 712e40387bc..f62eb188819 100644 --- a/src/main/ipc/pty/pane/spawn-registration.ts +++ b/src/main/ipc/pty/pane/spawn-registration.ts @@ -1,3 +1,5 @@ +import type { PtyRegistrationClaim } from '../../../runtime/runtime-terminal-state-records' +import { PaneOwnerContentionError } from './pane-spawn-operation' import type { Store } from '../../../persistence' import type { OrcaRuntimeService } from '../../../runtime/orca-runtime' import type { IPtyProvider, PtySpawnResult } from '../../../providers/pty-provider-contract' @@ -55,7 +57,10 @@ function exitedBeforeAdmission( export async function discardUnpersistedPtySpawn( provider: IPtyProvider, result: PtySpawnResult, - runtime: Pick | undefined + runtime: + | (Pick & + Partial>) + | undefined ): Promise { if ( result.isReattach || @@ -64,44 +69,60 @@ export async function discardUnpersistedPtySpawn( ) { return } - let proof: PtyExitProof + // Why: this candidate's exit is our cleanup, not the pane's process ending; it must keep the tab. + const settleStop = runtime?.intentionalPtyStops?.mark( + result.id, + 'replaced', + result.incarnationId ?? null + ) + let stopped = false try { - await provider.shutdown(result.id, { - immediate: true, - ...(result.incarnationId ? { expectedIncarnationId: result.incarnationId } : {}) - }) - proof = PtyExitProof.from('shutdown-acknowledged', result.incarnationId) - } catch (error) { - if (!isPtyAlreadyGoneError(error)) { - console.warn('[pty] failed to clean up PTY after persistence failure:', error) - // An observed exit, or a same-id replacement, outranks a lost control reply. - if ( - !ptyOwnership.has(result.id) || - ptyIncarnationById.get(result.id) !== result.incarnationId - ) { + let proof: PtyExitProof + try { + await provider.shutdown(result.id, { + immediate: true, + ...(result.incarnationId ? { expectedIncarnationId: result.incarnationId } : {}) + }) + proof = PtyExitProof.from('shutdown-acknowledged', result.incarnationId) + } catch (error) { + if (!isPtyAlreadyGoneError(error)) { + console.warn('[pty] failed to clean up PTY after persistence failure:', error) + // An observed exit, or a same-id replacement, outranks a lost control reply. + if ( + !ptyOwnership.has(result.id) || + ptyIncarnationById.get(result.id) !== result.incarnationId + ) { + return + } + // Keep ownership: the process may still run, so it reads unverifiable until it exits or an + // orphan sweep reaps it, never forgotten. + runtime?.markPtyLivenessUnverifiable?.( + result.id, + `discard shutdown failed: ${errorMessage(error)}` + ) return } - // Keep ownership: the process may still run, so it reads unverifiable until it exits or an - // orphan sweep reaps it, never forgotten. - runtime?.markPtyLivenessUnverifiable?.( - result.id, - `discard shutdown failed: ${errorMessage(error)}` - ) - return + proof = PtyExitProof.from('absent', result.incarnationId) } - proof = PtyExitProof.from('absent', result.incarnationId) + stopped = true + // The proof names this incarnation, so a replacement that arrived meanwhile keeps its own. + finishPtyShutdown(result.id, null, undefined, proof) + } finally { + settleStop?.(stopped) } - // The proof names this incarnation, so a replacement that arrived meanwhile keeps its own. - finishPtyShutdown(result.id, null, undefined, proof) } // Successful registration must not yield before the remaining spawn publication. export function registerPersistedPtySpawn( runtime: OrcaRuntimeService | undefined, store: Store | undefined, + claim: PtyRegistrationClaim | undefined, ...args: Parameters ): Promise | undefined { try { + if (claim && !runtime?.narrowPtyRegistration(args[0], claim, args[3]?.incarnationId)) { + throw new PaneOwnerContentionError() + } runtime?.registerPty(...args) } catch (error) { const [ptyId, worktreeId, connectionId, binding] = args diff --git a/src/main/ipc/pty/pane/spawn-reservation.ts b/src/main/ipc/pty/pane/spawn-reservation.ts index 8f8c7a0bca7..4fb51d0a690 100644 --- a/src/main/ipc/pty/pane/spawn-reservation.ts +++ b/src/main/ipc/pty/pane/spawn-reservation.ts @@ -49,7 +49,10 @@ export async function joinPaneSpawn( try { return await reservation.promise } catch (error) { - if (isWorktreeTerminalsSleepingError(error)) { + if ( + isWorktreeTerminalsSleepingError(error) || + (error instanceof Error && error.message === 'terminal_pane_unbound') + ) { return null } throw error diff --git a/src/main/ipc/pty/pane/stable-owner.ts b/src/main/ipc/pty/pane/stable-owner.ts index ff8997181a3..8552b9254c7 100644 --- a/src/main/ipc/pty/pane/stable-owner.ts +++ b/src/main/ipc/pty/pane/stable-owner.ts @@ -1,3 +1,4 @@ +import { persistFreshPaneBinding } from '../runtime/spawn-commit' import { rollbackWorkspaceSessionAfterFailedAsyncWrite } from '../../../persistence/restoring-sessions/workspace-session-write-rollback' import { cloneWorkspaceSessionState } from '../../../persistence/restoring-sessions/session-owner-fields' import { toSshExecutionHostId } from '../../../../shared/execution-host' @@ -11,15 +12,8 @@ import type { PtySpawnResult } from '../../../providers/pty-provider-contract' import { getPtySshConnectionId } from '../provider/registry' -import { - isDaemonEndpointGoneError, - SessionNotFoundError, - TerminalHostGoneError, - TerminalSessionOwnerUnverifiedError -} from '../../../daemon/daemon-errors' import { ptyIncarnationById } from '../provider/ownership-state' -import { keepAbsentPaneBindingForSwap, releaseAbsentPaneOwner } from './absent-pane-owner' -import { excludeReplacedPaneOwner, type ReplacedPaneOwner } from './replaced-pane-owner' +import type { ReplacedPaneOwner } from './replaced-pane-owner' import { spawnCommitBindingOrigin } from '../../../persistence/loading-store/pty-binding-span' export type StablePaneOwner = { @@ -180,6 +174,8 @@ export type StablePaneSpawnContext = { connectionId?: string | null resolveOwner?: () => StablePaneOwner | null onFreshSpawn?: (result: PtySpawnResult) => void + /** Called before attaching an owner that took the pane during the first attach. */ + onOwnerReresolved?: (owner: StablePaneOwner) => void /** Only a spawn holding the pane's reservation: it binds next, so its bind swaps the dead id. */ keepAbsentBinding?: (dead: ReplacedPaneOwner) => void } @@ -201,12 +197,13 @@ export async function persistAdmittedStablePaneBinding(args: { result: PtySpawnResult worktreeId: string | undefined startupCwd: string | undefined + connectionId?: string | null }): Promise { const expectedBinding = stablePanePersistenceFence(args.owner) - if (!args.store || !args.owner || !args.worktreeId || !expectedBinding) { + if (!args.store || !args.owner || !args.worktreeId) { return false } - const persisted = await args.store.persistPtyBinding({ + const binding = { worktreeId: args.worktreeId, tabId: args.owner.tabId, leafId: args.owner.leafId, @@ -214,91 +211,18 @@ export async function persistAdmittedStablePaneBinding(args: { ...(args.result.incarnationId ? { incarnationId: args.result.incarnationId } : {}), ...(args.startupCwd ? { startupCwd: args.startupCwd } : {}), expectedBinding, + mayCreate: false, + mayReviveRetiredSurface: false, origin: spawnCommitBindingOrigin(args.result) - }) + } + const persisted = expectedBinding + ? await args.store.persistPtyBinding( + binding, + args.connectionId ? toSshExecutionHostId(args.connectionId) : undefined + ) + : await persistFreshPaneBinding(args.store, binding, args.connectionId) if (persisted === false) { throw new Error('terminal_pane_owner_changed') } return true } - -export async function attachStablePaneOwner( - args: StablePaneSpawnContext & { owner: StablePaneOwner } -): Promise<{ result: PtySpawnResult; owner: StablePaneOwner } | null> { - const { owner, provider, spawnOptions } = args - let result: PtySpawnResult - try { - result = await provider.spawn({ - ...spawnOptions, - sessionId: owner.ptyId, - attachOnly: true, - expectedIncarnationId: owner.runtimeIncarnationId ?? owner.persistedIncarnationId, - expectedIncarnationIsAuthoritative: owner.runtimeIncarnationId !== undefined, - isNewSession: undefined, - command: undefined, - commandDelivery: undefined, - startupCommandDelivery: undefined, - launchAgent: undefined, - startupIngress: undefined, - agentSessionEnsure: undefined, - agentSessionCreateOperationId: undefined, - onPtySpawnCommitted: undefined - }) - } catch (error) { - if (error instanceof TerminalSessionOwnerUnverifiedError) { - throw new Error('terminal_pane_owner_unverified') - } - // Why: translate before paired-runtime RPC strips the socket error's code and syscall. - if (isDaemonEndpointGoneError(error)) { - throw new TerminalHostGoneError() - } - // Why: only the process that owns the PTY answers this, so its absence is an observed exit. - if (!(error instanceof SessionNotFoundError)) { - throw error - } - const ownerBeforeRetire = args.resolveOwner?.() - if ( - ownerBeforeRetire && - (ownerBeforeRetire.ptyId !== owner.ptyId || - ownerBeforeRetire.runtimeIncarnationId !== owner.runtimeIncarnationId || - ownerBeforeRetire.hasPersistedBinding !== owner.hasPersistedBinding || - ownerBeforeRetire.persistedIncarnationId !== owner.persistedIncarnationId) - ) { - throw new Error('terminal_pane_owner_changed') - } - const kept = args.keepAbsentBinding ? keepAbsentPaneBindingForSwap(args, owner) : null - if (kept) { - args.keepAbsentBinding?.(kept) - } else if (!(await releaseAbsentPaneOwner(args, owner))) { - throw new Error('terminal_pane_owner_changed') - } - if (excludeReplacedPaneOwner(args.resolveOwner?.() ?? null, kept)) { - throw new Error('terminal_pane_owner_changed') - } - return null - } - if ( - result.id !== owner.ptyId || - result.isReattach !== true || - (owner.runtimeIncarnationId !== undefined && - result.incarnationId !== owner.runtimeIncarnationId) || - (result.incarnationId === undefined && owner.incarnationId !== undefined) - ) { - throw new Error('terminal_pane_owner_changed') - } - return { result, owner } -} - -export async function spawnForStablePane( - args: StablePaneSpawnContext -): Promise<{ result: PtySpawnResult; owner: StablePaneOwner | null }> { - if (args.owner) { - const attached = await attachStablePaneOwner({ ...args, owner: args.owner }) - if (attached) { - return attached - } - } - const result = await args.provider.spawn(args.spawnOptions) - args.onFreshSpawn?.(result) - return { result, owner: null } -} diff --git a/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts b/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts index bdb134f68c2..b0b988cd6f6 100644 --- a/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts +++ b/src/main/ipc/pty/pane/stable-pane-absence-death-certificate.test.ts @@ -9,7 +9,8 @@ import { SessionNotFoundError } from '../../../daemon/daemon-errors' import type { Store } from '../../../persistence' import type { IPtyProvider } from '../../../providers/pty-provider-contract' import { OrcaRuntimeService } from '../../../runtime/orca-runtime' -import { spawnForStablePane } from './stable-owner' +import { adoptStablePane } from './adopt-stable' +import * as providers from '../provider/registry' const CONNECTION = 'conn-1' const WORKTREE = 'repo-1::/tmp/pane-absence' @@ -17,7 +18,6 @@ const TAB = 'tab-1' const LEAF = '1b3f2c4d-5e6a-4b7c-8d9e-0f1a2b3c4d5e' const SIBLING_LEAF = '2c4d3e5f-6a7b-4c8d-9e0f-1a2b3c4d5e6f' const PTY_ID = 'ssh:conn-1@@pty2:epoch-a:1' -const OWNER = { tabId: TAB, leafId: LEAF, ptyId: PTY_ID, hasPersistedBinding: true as const } function paneStore(): { store: Store; read: () => WorkspaceSessionState } { let session = { @@ -93,15 +93,16 @@ async function adoptAfterAttachRefusal(error: unknown): Promise<{ .fn() .mockRejectedValueOnce(error) .mockResolvedValueOnce({ id: 'ssh:conn-1@@pty2:epoch-b:1', isReattach: false }) - await spawnForStablePane({ - runtime, - store, - provider: { spawn } as unknown as IPtyProvider, - spawnOptions: { cols: 80, rows: 24 }, - owner: OWNER, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: refusal exits attachment before any provider method other than spawn is used. + vi.spyOn(providers, 'getProvider').mockReturnValue({ spawn } as unknown as IPtyProvider) + await adoptStablePane(runtime, store, { + cols: 80, + rows: 24, + tabId: TAB, + leafId: LEAF, worktreeId: WORKTREE, connectionId: CONNECTION, - resolveOwner: () => null + spawnFresh: () => spawn({ cols: 80, rows: 24 }) }) return { runtime, store, read, spawn } } @@ -122,16 +123,16 @@ describe('a stable pane whose reattach was refused', () => { const runtime = runtimeOwning(store) const spawn = vi.fn().mockRejectedValue(new Error(`PTY "pty2:epoch-a:1" not found`)) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the rejected attachment calls only spawn and never admits this provider. + vi.spyOn(providers, 'getProvider').mockReturnValue({ spawn } as unknown as IPtyProvider) await expect( - spawnForStablePane({ - runtime, - store, - provider: { spawn } as unknown as IPtyProvider, - spawnOptions: { cols: 80, rows: 24 }, - owner: OWNER, + adoptStablePane(runtime, store, { + cols: 80, + rows: 24, + tabId: TAB, + leafId: LEAF, worktreeId: WORKTREE, - connectionId: CONNECTION, - resolveOwner: () => null + connectionId: CONNECTION }) ).rejects.toThrow('not found') diff --git a/src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts b/src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts index d6b8ecc1142..1d95c2d460b 100644 --- a/src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts +++ b/src/main/ipc/pty/pane/stable-pane-absence-respawn.test.ts @@ -1,13 +1,18 @@ -import { describe, expect, it, vi } from 'vitest' -import { withDurableRuntimeStore } from '../../../runtime/runtime-durable-store-fixture' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createPaneBindingStore } from '../../pty-launch-race-test-fixture' import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' import { SessionNotFoundError, TerminalSessionOwnerUnverifiedError } from '../../../daemon/daemon-errors' import type { Store } from '../../../persistence' -import type { IPtyProvider } from '../../../providers/pty-provider-contract' -import { spawnForStablePane, type StablePaneOwner } from './stable-owner' +import { LocalPtyProvider } from '../../../providers/local-pty-provider' +import { OrcaRuntimeService } from '../../../runtime/orca-runtime' +import * as providers from '../provider/registry' +import { adoptStablePane } from './adopt-stable' +import type { StablePaneOwner } from './stable-owner' + +import { ptyIncarnationById, ptyOwnership } from '../provider/ownership-state' const LEAF = '1b3f2c4d-5e6a-4b7c-8d9e-0f1a2b3c4d5e' const SIBLING_LEAF = '2c4d3e5f-6a7b-4c8d-9e0f-1a2b3c4d5e6f' @@ -23,31 +28,37 @@ function spawnAfterAttachRejection( error: unknown, persistence?: { store: Store; worktreeId: string } ): { - run: () => ReturnType + run: () => ReturnType spawn: ReturnType } { const spawn = vi .fn() .mockRejectedValueOnce(error) .mockResolvedValueOnce({ id: 'ssh:conn-1@@pty-2', isReattach: false }) + const provider = new LocalPtyProvider() + vi.spyOn(provider, 'spawn').mockImplementation(spawn) + vi.spyOn(providers, 'getProvider').mockReturnValue(provider) + const store = persistence?.store ?? sessionStore([LEAF]).store return { spawn, run: () => - spawnForStablePane({ - runtime: undefined, - provider: { spawn } as unknown as IPtyProvider, - spawnOptions: { cols: 80, rows: 24 }, - owner: OWNER, + adoptStablePane(undefined, store, { + ...OWNER, + worktreeId: WORKTREE, connectionId: 'conn-1', - resolveOwner: () => null, - ...(persistence ? { store: persistence.store, worktreeId: persistence.worktreeId } : {}) + cols: 80, + rows: 24, + spawnFresh: () => spawn({ cols: 80, rows: 24 }) }) } } /** Real binding retirement against an in-memory session. */ function sessionStore(leaves: string[]): { store: Store; read: () => WorkspaceSessionState } { - let session = { + const store = createPaneBindingStore() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the legacy split fixture supplies the topology and binding fields read by adoption and retirement. + const session = { + ...store.getWorkspaceSession(), tabsByWorktree: { [WORKTREE]: [{ id: 'tab-1', worktreeId: WORKTREE, ptyId: 'ssh:conn-1@@pty-1' }] }, @@ -68,20 +79,27 @@ function sessionStore(leaves: string[]): { store: Store; read: () => WorkspaceSe }, terminalPtyIncarnationsByPaneKey: {} } as unknown as WorkspaceSessionState + store.setWorkspaceSession(session) return { - read: () => session, - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This fixture supplies every persistence method used by stable-pane retirement. - store: withDurableRuntimeStore({ - getWorkspaceSession: () => session, - setWorkspaceSession: (next: WorkspaceSessionState) => { - session = next - }, - flushOrThrow: () => {} - }) as unknown as Store + read: store.getWorkspaceSession, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the shared fixture implements durable pane admission, binding and retirement. + store: store as unknown as Store } } describe('stable pane adoption after the owning host reports the PTY absent', () => { + beforeEach(() => { + for (const id of [OWNER.ptyId, 'ssh:conn-1@@pty-2']) { + ptyOwnership.set(id, 'ssh:conn-1') + } + }) + afterEach(() => { + for (const id of [OWNER.ptyId, 'ssh:conn-1@@pty-2']) { + ptyOwnership.delete(id) + ptyIncarnationById.delete(id) + } + }) + it.each([false, true])( 'reattaches a live pane without launching a provider process (settled worker: %s)', async (settledWorker) => { @@ -105,18 +123,18 @@ describe('stable pane adoption after the owning host reports the PTY absent', () }) const spawn = vi.fn().mockResolvedValue({ id: OWNER.ptyId, isReattach: true }) const onFreshSpawn = vi.fn() - const result = await spawnForStablePane({ - runtime: undefined, - store, + const provider = new LocalPtyProvider() + vi.spyOn(provider, 'spawn').mockImplementation(spawn) + vi.spyOn(providers, 'getProvider').mockReturnValue(provider) + const result = await adoptStablePane(undefined, store, { + ...OWNER, worktreeId: WORKTREE, - provider: { spawn } as unknown as IPtyProvider, - spawnOptions: { cols: 80, rows: 24, command: 'claude --resume provider-session' }, - owner: OWNER, connectionId: 'conn-1', - resolveOwner: () => OWNER, - onFreshSpawn + cols: 80, + rows: 24, + spawnFresh: onFreshSpawn }) - expect(result.owner).toBe(OWNER) + expect(result?.owner).toMatchObject(OWNER) expect(spawn).toHaveBeenCalledExactlyOnceWith( expect.objectContaining({ sessionId: OWNER.ptyId, attachOnly: true, command: undefined }) ) @@ -133,24 +151,27 @@ describe('stable pane adoption after the owning host reports the PTY absent', () expect(spawn).toHaveBeenCalledTimes(2) expect(spawn.mock.calls[0]?.[0]).toMatchObject({ attachOnly: true }) expect(spawn.mock.calls[1]?.[0]).not.toHaveProperty('sessionId') - expect(result.owner).toBeNull() + expect(result).toBeNull() }) it('refuses to respawn a workspace pane when no Store can clear its binding', async () => { - const spawn = vi.fn().mockRejectedValueOnce(new SessionNotFoundError(OWNER.ptyId)) - + const spawn = vi.fn() + const provider = new LocalPtyProvider() + vi.spyOn(provider, 'spawn').mockRejectedValueOnce(new SessionNotFoundError(OWNER.ptyId)) + vi.spyOn(providers, 'getProvider').mockReturnValue(provider) + const runtime = new OrcaRuntimeService() + runtime.registerPty(OWNER.ptyId, WORKTREE, 'conn-1', OWNER) await expect( - spawnForStablePane({ - runtime: undefined, - provider: { spawn } as unknown as IPtyProvider, - spawnOptions: { cols: 80, rows: 24 }, - owner: OWNER, + adoptStablePane(runtime, undefined, { + ...OWNER, worktreeId: WORKTREE, connectionId: 'conn-1', - resolveOwner: () => null + cols: 80, + rows: 24, + spawnFresh: spawn }) ).rejects.toThrow('terminal_pane_owner_changed') - expect(spawn).toHaveBeenCalledTimes(1) + expect(spawn).not.toHaveBeenCalled() }) // The message alone cannot distinguish absence from a lost link; only the class may authorise @@ -180,7 +201,7 @@ describe('stable pane adoption after the owning host reports the PTY absent', () const result = await run() expect(spawn).toHaveBeenCalledTimes(2) - expect(result.owner).toBeNull() + expect(result).toBeNull() const session = read() expect(session.tabsByWorktree[WORKTREE]?.map((tab) => tab.id)).toEqual(['tab-1']) const layout = session.terminalLayoutsByTabId['tab-1'] @@ -201,14 +222,14 @@ describe('stable pane adoption after the owning host reports the PTY absent', () const result = await run() expect(spawn).toHaveBeenCalledTimes(2) - expect(result.owner).toBeNull() + expect(result).toBeNull() const session = read() expect(session.tabsByWorktree[WORKTREE]?.map((tab) => tab.id)).toEqual(['tab-1']) expect(session.terminalLayoutsByTabId['tab-1']?.root).toEqual({ type: 'leaf', leafId: LEAF }) expect(session.terminalLayoutsByTabId['tab-1']?.ptyIdsByLeafId?.[LEAF]).toBeUndefined() }) - // A spawn holding the pane's reservation binds next: its bind swaps the dead id in one write. + // Adoption holds the pane's reservation through its fresh launch, so that bind swaps the dead id. it('keeps the absent binding for the swap when the spawn takes it', async () => { const { store, read } = sessionStore([LEAF, SIBLING_LEAF]) const before = JSON.stringify(read()) @@ -216,28 +237,30 @@ describe('stable pane adoption after the owning host reports the PTY absent', () .fn() .mockRejectedValueOnce(new SessionNotFoundError(OWNER.ptyId)) .mockResolvedValueOnce({ id: 'ssh:conn-1@@pty-3', isReattach: false }) + const provider = new LocalPtyProvider() + vi.spyOn(provider, 'spawn').mockImplementation(spawn) + vi.spyOn(providers, 'getProvider').mockReturnValue(provider) const keepAbsentBinding = vi.fn() - const result = await spawnForStablePane({ - runtime: undefined, - store, + const result = await adoptStablePane(undefined, store, { + ...OWNER, worktreeId: WORKTREE, - provider: { spawn } as unknown as IPtyProvider, - spawnOptions: { cols: 80, rows: 24 }, - owner: OWNER, connectionId: 'conn-1', - // The persisted owner is still the dead id until the bind swaps it. - resolveOwner: () => OWNER, - keepAbsentBinding + cols: 80, + rows: 24, + keepAbsentBinding, + spawnFresh: () => spawn({ cols: 80, rows: 24 }) }) - expect(result.owner).toBeNull() + expect(result).toBeNull() expect(spawn).toHaveBeenCalledTimes(2) - expect(keepAbsentBinding).toHaveBeenCalledExactlyOnceWith({ - ptyId: OWNER.ptyId, - pane: { worktreeId: WORKTREE, tabId: 'tab-1', leafId: LEAF, hostId: 'ssh:conn-1' }, - dropsIncarnation: true - }) + expect(keepAbsentBinding).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + ptyId: OWNER.ptyId, + pane: { worktreeId: WORKTREE, tabId: 'tab-1', leafId: LEAF, hostId: 'ssh:conn-1' }, + dropsIncarnation: true + }) + ) expect(JSON.stringify(read())).toBe(before) }) diff --git a/src/main/ipc/pty/pane/stable-pane-attach.ts b/src/main/ipc/pty/pane/stable-pane-attach.ts new file mode 100644 index 00000000000..1d2fd22ff94 --- /dev/null +++ b/src/main/ipc/pty/pane/stable-pane-attach.ts @@ -0,0 +1,115 @@ +import { PaneOwnerContentionError } from './pane-spawn-operation' +import { + isDaemonEndpointGoneError, + SessionNotFoundError, + TerminalHostGoneError, + TerminalSessionOwnerUnverifiedError +} from '../../../daemon/daemon-errors' +import type { PtySpawnResult } from '../../../providers/pty-provider-contract' +import { keepAbsentPaneBindingForSwap, releaseAbsentPaneOwner } from './absent-pane-owner' +import { excludeReplacedPaneOwner } from './replaced-pane-owner' +import type { StablePaneOwner, StablePaneSpawnContext } from './stable-owner' + +type StablePaneAttachAttempt = + | { attached: { result: PtySpawnResult; owner: StablePaneOwner } } + | { absent: true } + | { lostTo: StablePaneOwner | null } + +function isSameStablePaneOwner(a: StablePaneOwner, b: StablePaneOwner): boolean { + return ( + a.ptyId === b.ptyId && + a.runtimeIncarnationId === b.runtimeIncarnationId && + a.hasPersistedBinding === b.hasPersistedBinding && + a.persistedIncarnationId === b.persistedIncarnationId + ) +} + +export async function attemptStablePaneAttach( + args: StablePaneSpawnContext & { owner: StablePaneOwner } +): Promise { + const { owner, provider, spawnOptions } = args + let result: PtySpawnResult + try { + result = await provider.spawn({ + ...spawnOptions, + sessionId: owner.ptyId, + attachOnly: true, + expectedIncarnationId: owner.runtimeIncarnationId ?? owner.persistedIncarnationId, + expectedIncarnationIsAuthoritative: owner.runtimeIncarnationId !== undefined, + isNewSession: undefined, + command: undefined, + commandDelivery: undefined, + startupCommandDelivery: undefined, + launchAgent: undefined, + startupIngress: undefined, + agentSessionEnsure: undefined, + agentSessionCreateOperationId: undefined, + onPtySpawnCommitted: undefined + }) + } catch (error) { + if (error instanceof TerminalSessionOwnerUnverifiedError) { + throw new Error('terminal_pane_owner_unverified') + } + // Why: translate before paired-runtime RPC strips the socket error's code and syscall. + if (isDaemonEndpointGoneError(error)) { + throw new TerminalHostGoneError() + } + // Why: only the process that owns the PTY answers this, so its absence is an observed exit. + if (!(error instanceof SessionNotFoundError)) { + throw error + } + const ownerBeforeRetire = args.resolveOwner?.() + if (ownerBeforeRetire && !isSameStablePaneOwner(ownerBeforeRetire, owner)) { + return { lostTo: ownerBeforeRetire } + } + // The adoption holds the pane's reservation, so its fresh bind can swap the dead binding. + const kept = args.keepAbsentBinding ? keepAbsentPaneBindingForSwap(args, owner) : null + if (kept) { + args.keepAbsentBinding?.(kept) + } else if (!(await releaseAbsentPaneOwner(args, owner))) { + return { lostTo: args.resolveOwner?.() ?? null } + } + const ownerAfterRetire = excludeReplacedPaneOwner(args.resolveOwner?.() ?? null, kept) + return ownerAfterRetire ? { lostTo: ownerAfterRetire } : { absent: true } + } + if ( + result.id !== owner.ptyId || + result.isReattach !== true || + (owner.runtimeIncarnationId !== undefined && + result.incarnationId !== owner.runtimeIncarnationId) || + (result.incarnationId === undefined && owner.incarnationId !== undefined) + ) { + throw new Error('terminal_pane_owner_changed') + } + return { attached: { result, owner } } +} + +export async function reconcilePaneOwner(args: { + owner: StablePaneOwner | null + resolveOwner: () => StablePaneOwner | null + attach: (owner: StablePaneOwner) => Promise + fresh: () => Promise + losses?: number +}): Promise { + let owner = args.owner + let losses = args.losses ?? 0 + for (;;) { + try { + if (owner) { + const attached = await args.attach(owner) + if (attached !== null) { + return attached + } + } + return await args.fresh() + } catch (error) { + if (!(error instanceof PaneOwnerContentionError) || losses++ >= 1) { + throw error + } + owner = args.resolveOwner() + if (!owner) { + throw error + } + } + } +} diff --git a/src/main/ipc/pty/register-handlers.ts b/src/main/ipc/pty/register-handlers.ts index 239c466014e..84d649a2ac2 100644 --- a/src/main/ipc/pty/register-handlers.ts +++ b/src/main/ipc/pty/register-handlers.ts @@ -230,7 +230,10 @@ export function registerPtyHandlers( ): ReturnType => prepareCodexResumeHome(options?.prepareCodexSessionResume, args) const adoptStablePaneBound = (args: Parameters[2]) => - adoptStablePane(runtime, store, args) + adoptStablePane(runtime, store, { + ...args, + onAdopted: (result) => session.sendPtySpawnedToRenderer(result.id) + }) // Why: route through getProviderForPty() so CLI commands work for remote PTYs too; localProvider would silently fail for them. installPtyRuntimeController({ diff --git a/src/main/ipc/pty/runtime/controller.ts b/src/main/ipc/pty/runtime/controller.ts index f80e78897bf..1bf389cfeb0 100644 --- a/src/main/ipc/pty/runtime/controller.ts +++ b/src/main/ipc/pty/runtime/controller.ts @@ -1,3 +1,5 @@ +import { adoptMaterializedRuntimePtySpawn } from './spawn-early' +import { createRuntimePtySpawnState } from './spawn-state' import { localProvider } from '../provider/registry' import { getLocalAutomaticWorkStatus, @@ -47,7 +49,15 @@ export function installPtyRuntimeController(deps: PtyRuntimeControllerDeps): voi const ownerKey = makePaneSpawnReservationKey(args.worktreeId, args.connectionId, paneKey) return ownerKey ? claimRuntimePaneCreate(ownerKey) : () => {} }, - adoptStablePane, + adoptStablePane: async (args) => { + const adopted = await adoptStablePane(args) + if (adopted) { + await adoptMaterializedRuntimePtySpawn( + createRuntimePtySpawnState(deps, { ...args, adoptedStablePane: adopted }) + ) + } + return adopted + }, spawn: async (args) => spawnPtyFromRuntimeController(deps, args), // Why the local owner only: an SSH host's gate belongs to that host's own runtime. getAutomaticWorkStatus: () => getLocalAutomaticWorkStatus(localProvider), diff --git a/src/main/ipc/pty/runtime/spawn-commit-response.ts b/src/main/ipc/pty/runtime/spawn-commit-response.ts new file mode 100644 index 00000000000..d7e677425b8 --- /dev/null +++ b/src/main/ipc/pty/runtime/spawn-commit-response.ts @@ -0,0 +1,30 @@ +import type { RuntimePtySpawnState } from './spawn-state' +import { resolvePaneSpawnReservation } from '../pane/spawn-reservation' + +export function completeRuntimeSpawnResponse(ctx: RuntimePtySpawnState) { + const args = ctx.args + const response = { + id: ctx.result.id, + ...(ctx.result.incarnationId ? { incarnationId: ctx.result.incarnationId } : {}), + ...(ctx.stablePaneOwner && (ctx.stablePaneOwner.handle || args.preAllocatedHandle) + ? { + stablePaneOwner: { + handle: ctx.stablePaneOwner.handle ?? args.preAllocatedHandle!, + tabId: ctx.stablePaneOwner.tabId, + leafId: ctx.stablePaneOwner.leafId + } + } + : {}), + ...(ctx.result.agentSessionEnsure ? { agentSessionEnsure: ctx.result.agentSessionEnsure } : {}) + } + resolvePaneSpawnReservation(ctx.paneSpawnReservationKey, ctx.paneSpawnReservation, { + ...ctx.result, + ...(typeof ctx.result.snapshotKittyKeyboardFlags === 'number' && + ctx.reconciledSnapshotSeq !== null && + ctx.snapshotKittyFlagsCoverReconciledSeq + ? { snapshotSeq: ctx.reconciledSnapshotSeq } + : { snapshotKittyKeyboardFlags: undefined }), + isReattach: true + }) + return { ...ctx.result, ...response, wslDistro: ctx.result.wslDistro ?? undefined } +} diff --git a/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts b/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts index b35dbd4593c..e808cfa57eb 100644 --- a/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts +++ b/src/main/ipc/pty/runtime/spawn-commit-run-facts.test.ts @@ -121,7 +121,7 @@ describe('runtime spawn commit: run facts', () => { expect(persistPtyBinding).toHaveBeenCalledOnce() expect(facts.read(PTY_ID, INCARNATION_ID).freshSpawn).toBe(false) - expect(stops.claimExit(PTY_ID, INCARNATION_ID)).toEqual(['reversible']) + expect(stops.claimExit(PTY_ID, INCARNATION_ID)).toEqual(['replaced']) }) it.each([ diff --git a/src/main/ipc/pty/runtime/spawn-commit.ts b/src/main/ipc/pty/runtime/spawn-commit.ts index 6a6bd59ee14..335578b511c 100644 --- a/src/main/ipc/pty/runtime/spawn-commit.ts +++ b/src/main/ipc/pty/runtime/spawn-commit.ts @@ -1,3 +1,13 @@ +import { completeRuntimeSpawnResponse } from './spawn-commit-response' +import type { Store } from '../../../persistence' +import type { PersistPtyBindingArgs } from '../../../persistence/loading-store/pty-binding-persistence' +import { ptyBindingIsRefused } from '../../../persistence/loading-store/pty-binding-refusals' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { + isAbsentBindingReplay, + PaneOwnerContentionError, + type AbsentPaneBinding +} from '../pane/pane-spawn-operation' import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id' import { ptySizes } from '../delivery/visibility-state' import { commitRuntimePtySize } from './spawn-commit-pty-size' @@ -17,11 +27,11 @@ import { seedTerminalRestoreRecordsFromSpawnResult } from '../pane/agent-session import { seedResumeLaunchIdentityFromSpawn } from '../pane/resume-launch-identity' import { seedHeadlessTerminalFromSpawnResult } from '../pane/terminal-spawn-restore' import { recordPtySpawnTelemetry } from '../pane/spawn-telemetry' -import { persistAdmittedStablePaneBinding } from '../pane/stable-owner' import { isNativeWindowsLocalPtySpawn, markNativeWindowsConptyPty } from '../../../runtime/terminal-model-query-authority' +import { toSshExecutionHostId } from '../../../../shared/execution-host' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' import { resolvePaneSpawnReservation } from '../pane/spawn-reservation' import { admitProviderReattachLaunchIdentity } from '../pane/launch-authority' @@ -54,23 +64,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { // Seed before the first disk await so live output appends to the restored history. seedHeadlessTerminalFromSpawnResult(ctx.deps.runtime, ctx.result, ctx.spawnIdentityPaneKey) seedTerminalRestoreRecordsFromSpawnResult(ctx.deps.runtime, ctx.result) - try { - ctx.stablePaneBindingPersisted = await persistAdmittedStablePaneBinding({ - store: ctx.hostSessionBinding?.store, - owner: ctx.stablePaneOwner, - result: ctx.result, - worktreeId: ctx.hostSessionBinding?.worktreeId, - startupCwd: ctx.cwd - }) - } catch (error) { - if (error instanceof Error && error.message === 'terminal_pane_owner_changed') { - throw error - } - console.error('[pty] failed to persist runtime PTY binding after attach:', error) - throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage(error)), { - agentSessionOperationOutcome: 'unknown' as const - }) - } + if (ctx.result.agentSessionEnsure?.disposition === 'adopted') { // Why: an adoption is an attach to a live owner by definition, but an adopted reply can omit // isReattach; derive it once so the size commit and the reservation agree. @@ -79,6 +73,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { const rejectedRegistration = registerPersistedPtySpawn( ctx.deps.runtime, ctx.hostSessionBinding?.store ?? ctx.deps.store, + ctx.pendingRegistrationClaim, ctx.result.id, owner.surface.worktreeId, null, @@ -125,7 +120,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { agentSessionEnsure: ctx.result.agentSessionEnsure } } - if (ctx.hostSessionBinding && !ctx.stablePaneBindingPersisted) { + if (ctx.hostSessionBinding) { try { const { store, worktreeId, tabId, leafId, expectedSourceBinding, placement } = ctx.hostSessionBinding @@ -142,14 +137,22 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ...(placement ? { placement } : {}), origin: spawnCommitBindingOrigin(ctx.result, expectedSourceBinding) } - const persisted = await store.persistPtyBinding(binding) + const persisted = await persistFreshPaneBinding( + store, + binding, + args.connectionId, + args.paneSpawnOperation?.absentOwner + ) if (persisted === false) { throw new Error('terminal_split_source_not_found') } } catch (err) { console.error('[pty] failed to persist runtime PTY binding after spawn:', err) await discardUnpersistedPtySpawn(ctx.provider, ctx.result, ctx.deps.runtime) - if (err instanceof Error && err.message === 'terminal_split_source_not_found') { + if ( + err instanceof PaneOwnerContentionError || + (err instanceof Error && err.message === 'terminal_split_source_not_found') + ) { throw err } throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage(err)), { @@ -161,6 +164,7 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { const rejectedRegistration = registerPersistedPtySpawn( ctx.deps.runtime, ctx.hostSessionBinding?.store ?? ctx.deps.store, + ctx.pendingRegistrationClaim, ctx.result.id, args.worktreeId, null, @@ -184,7 +188,11 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { } } else { // Why: non-worktree PTYs have no later surface-registration phase to clear admission intent. - ctx.deps.runtime?.cancelPendingPtyRegistration?.(ctx.result.id, ctx.result.incarnationId) + ctx.deps.runtime?.cancelPendingPtyRegistration?.( + ctx.result.id, + ctx.result.incarnationId, + ctx.pendingRegistrationClaim + ) } // Why after registration: a spawn discarded for a failed save or rejected for exiting during // start must not record facts or end a stop. @@ -268,28 +276,38 @@ async function commitReservedRuntimePtySpawn(ctx: RuntimePtySpawnState) { ? { launchEnv: ctx.env } : {}) }) - const response = { - id: ctx.result.id, - ...(ctx.result.incarnationId ? { incarnationId: ctx.result.incarnationId } : {}), - ...(ctx.stablePaneOwner && (ctx.stablePaneOwner.handle || args.preAllocatedHandle) - ? { - stablePaneOwner: { - handle: ctx.stablePaneOwner.handle ?? args.preAllocatedHandle!, - tabId: ctx.stablePaneOwner.tabId, - leafId: ctx.stablePaneOwner.leafId - } - } - : {}), - ...(ctx.result.agentSessionEnsure ? { agentSessionEnsure: ctx.result.agentSessionEnsure } : {}) + return completeRuntimeSpawnResponse(ctx) +} + +/** Runs inside the existing durable mutation, after source/topology checks and before any bind. */ +export async function persistFreshPaneBinding( + store: Store, + binding: PersistPtyBindingArgs, + connectionId?: string | null, + absentOwner?: AbsentPaneBinding +): Promise { + let targetChanged = false + const hostId = connectionId ? toSshExecutionHostId(connectionId) : undefined + const persisted = await store.persistPtyBinding(() => { + const session = store.getWorkspaceSession(hostId) + const paneKey = makePaneKey(binding.tabId, binding.leafId) + if (ptyBindingIsRefused(binding, session, binding.worktreeId, paneKey, [])) { + return binding + } + const bound = session.terminalLayoutsByTabId[binding.tabId]?.ptyIdsByLeafId?.[binding.leafId] + const stamp = session.terminalPtyIncarnationsByPaneKey?.[paneKey] + if ( + bound && + (bound !== binding.ptyId || stamp !== binding.incarnationId) && + !isAbsentBindingReplay(absentOwner, bound, stamp) + ) { + targetChanged = true + return null + } + return binding + }, hostId) + if (targetChanged) { + throw new PaneOwnerContentionError() } - resolvePaneSpawnReservation(ctx.paneSpawnReservationKey, ctx.paneSpawnReservation, { - ...ctx.result, - ...(typeof ctx.result.snapshotKittyKeyboardFlags === 'number' && - ctx.reconciledSnapshotSeq !== null && - ctx.snapshotKittyFlagsCoverReconciledSeq - ? { snapshotSeq: ctx.reconciledSnapshotSeq } - : { snapshotKittyKeyboardFlags: undefined }), - isReattach: true - }) - return response + return persisted } diff --git a/src/main/ipc/pty/runtime/spawn-execute.ts b/src/main/ipc/pty/runtime/spawn-execute.ts index 91c2c58ec67..cede5ace25f 100644 --- a/src/main/ipc/pty/runtime/spawn-execute.ts +++ b/src/main/ipc/pty/runtime/spawn-execute.ts @@ -9,8 +9,12 @@ import { assertSpawnReplyWasLive, reconcileAgentSessionOwnerListings } from '../pane/agent-session-owners' -import { spawnForStablePane, resolveStablePaneOwner } from '../pane/stable-owner' -import { clearProviderPtyState } from '../provider/state-cleanup' +import { resolveStablePaneOwner } from '../pane/stable-owner' +import { + assertPaneStillExists, + excludeAbsentOwnerReplay, + PaneOwnerContentionError +} from '../pane/pane-spawn-operation' import { isProviderAgentSessionOwnerLive } from '../provider/liveness' import { failPtySpawnExecute } from '../spawn-execute-failure' import type { RuntimePtySpawnState } from './spawn-state' @@ -36,20 +40,34 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise ? ctx.preAdoptedStablePane.owner : args.agentSessionEnsure ? null - : resolveStablePaneOwner( - ctx.deps.runtime, - ctx.deps.store, - ctx.spawnIdentityPaneKey, - args.worktreeId, - args.connectionId + : excludeAbsentOwnerReplay( + resolveStablePaneOwner( + ctx.deps.runtime, + ctx.deps.store, + ctx.spawnIdentityPaneKey, + args.worktreeId, + args.connectionId + ), + args.paneSpawnOperation?.absentOwner ) - const expectedPtyId = - stablePaneOwnerCandidate?.ptyId ?? ctx.effectiveSessionAppId ?? ctx.sessionId + if (stablePaneOwnerCandidate) { + throw new PaneOwnerContentionError() + } + if (args.paneSpawnOperation?.mustExist && args.worktreeId && args.tabId && args.leafId) { + assertPaneStillExists( + ctx.deps.store, + args.worktreeId, + args.tabId, + args.leafId, + args.connectionId + ) + } + const expectedPtyId = ctx.effectiveSessionAppId ?? ctx.sessionId if (!stablePaneOwnerCandidate) { markRuntimeSpawnHiddenBeforeSpawn(ctx) } if (expectedPtyId) { - ctx.deps.runtime?.beginPtyRegistration?.(expectedPtyId) + ctx.pendingRegistrationClaim = ctx.deps.runtime?.beginPtyRegistration?.(expectedPtyId) ctx.pendingRegistrationPtyId = expectedPtyId } if (ctx.isDaemonHostSpawn && expectedPtyId) { @@ -74,9 +92,13 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise const recoveredOwner = agentSessionOwners.find(args.agentSessionEnsure.claim) if (recoveredOwner && ctx.pendingRegistrationPtyId !== recoveredOwner.ptyId) { if (ctx.pendingRegistrationPtyId) { - ctx.deps.runtime?.cancelPendingPtyRegistration?.(ctx.pendingRegistrationPtyId) + ctx.deps.runtime?.cancelPendingPtyRegistration?.( + ctx.pendingRegistrationPtyId, + undefined, + ctx.pendingRegistrationClaim + ) } - ctx.deps.runtime?.beginPtyRegistration?.( + ctx.pendingRegistrationClaim = ctx.deps.runtime?.beginPtyRegistration?.( recoveredOwner.ptyId, ptyIncarnationById.get(recoveredOwner.ptyId) ) @@ -136,48 +158,38 @@ export async function executeRuntimePtySpawn(ctx: RuntimePtySpawnState): Promise ctx.result.agentSessionEnsure = ensured } else { assertClientStillConnected() - const stablePaneSpawn = ctx.preAdoptedStablePane - ? ctx.preAdoptedStablePane - : await spawnForStablePane({ - runtime: ctx.deps.runtime, - store: ctx.deps.store, - provider: ctx.provider, - spawnOptions: ctx.spawnOptions, - owner: stablePaneOwnerCandidate, - worktreeId: args.worktreeId, - connectionId: args.connectionId, - resolveOwner: () => - resolveStablePaneOwner( - ctx.deps.runtime, - ctx.deps.store, - ctx.spawnIdentityPaneKey, - args.worktreeId, - args.connectionId - ), - onFreshSpawn: ctx.reportPtySpawnCommitted - }) - ctx.result = stablePaneSpawn.result - ctx.stablePaneOwner = stablePaneSpawn.owner - if ( - ctx.stablePaneOwner && - ctx.isNewDaemonSession && - ctx.effectiveSessionAppId && - ctx.effectiveSessionAppId !== ctx.result.id - ) { - clearProviderPtyState(ctx.effectiveSessionAppId) - } + ctx.result = await ctx.provider.spawn(ctx.spawnOptions) + ctx.reportPtySpawnCommitted() + ctx.stablePaneOwner = null ctx.rejectedRegistrationCandidate = ctx.result - assertSpawnReplyWasLive(ctx.result) } ctx.rejectedRegistrationCandidate ??= ctx.result if (ctx.pendingRegistrationPtyId !== ctx.result.id) { if (ctx.pendingRegistrationPtyId) { - ctx.deps.runtime?.cancelPendingPtyRegistration?.(ctx.pendingRegistrationPtyId) + ctx.deps.runtime?.cancelPendingPtyRegistration?.( + ctx.pendingRegistrationPtyId, + undefined, + ctx.pendingRegistrationClaim + ) } - ctx.deps.runtime?.beginPtyRegistration?.(ctx.result.id, ctx.result.incarnationId) + ctx.pendingRegistrationClaim = ctx.deps.runtime?.beginPtyRegistration?.( + ctx.result.id, + ctx.result.incarnationId + ) ctx.pendingRegistrationPtyId = ctx.result.id } + assertSpawnReplyWasLive(ctx.result) // Why: admission precedes sequence/context state and every durable publication below. + if ( + ctx.pendingRegistrationClaim && + !ctx.deps.runtime?.narrowPtyRegistration( + ctx.result.id, + ctx.pendingRegistrationClaim, + ctx.result.incarnationId + ) + ) { + throw new PaneOwnerContentionError() + } ctx.deps.runtime?.assertPtyRegistrationAllowed?.(ctx.result.id, ctx.result.incarnationId) if (ctx.result.providerSequence) { const runtimeSequenceBeforeReconcile = diff --git a/src/main/ipc/pty/runtime/spawn-options.ts b/src/main/ipc/pty/runtime/spawn-options.ts index 50509b3a61a..dcc1282f594 100644 --- a/src/main/ipc/pty/runtime/spawn-options.ts +++ b/src/main/ipc/pty/runtime/spawn-options.ts @@ -6,17 +6,10 @@ import { routesFreshSpawnsToLocalProvider, beginPtySpawnForWorktree } from '../host-env/fresh-spawn-routing' -import { resolveStablePaneOwner } from '../pane/stable-owner' -import { - joinPaneSpawn, - makePaneSpawnReservationKey, - reservePaneSpawn, - paneSpawnReservationsByOwnerKey -} from '../pane/spawn-reservation' +import { makePaneSpawnReservationKey } from '../pane/spawn-reservation' import { assemblePtySpawnEnvDeletions, assemblePtySpawnLaunchOptions, - restoreProvisionalPtySize, seedPtySpawnSize } from '../spawn-options-assembly' import type { RuntimePtySpawnState } from './spawn-state' @@ -153,52 +146,8 @@ export async function buildRuntimePtySpawnOptions( ) { throw new Error('terminal_pane_identity_changed') } - if (!ctx.paneSpawnReservationKey) { - ctx.paneSpawnReservationKey = resolvedPaneSpawnReservationKey - const existingPaneSpawn = ctx.spawnIdentityPaneKey - ? paneSpawnReservationsByOwnerKey.get(resolvedPaneSpawnReservationKey!) - : undefined - const concurrentResult = existingPaneSpawn ? await joinPaneSpawn(existingPaneSpawn) : null - if (concurrentResult) { - const concurrentOwner = resolveStablePaneOwner( - ctx.deps.runtime, - ctx.deps.store, - ctx.spawnIdentityPaneKey, - args.worktreeId, - args.connectionId - ) - if ( - !concurrentOwner?.handle || - concurrentOwner.ptyId !== concurrentResult.id || - (concurrentOwner.incarnationId !== undefined && - concurrentResult.incarnationId !== undefined && - concurrentOwner.incarnationId !== concurrentResult.incarnationId) - ) { - throw new Error('terminal_pane_owner_unknown') - } - const reattach = { - id: concurrentOwner.ptyId, - ...(concurrentOwner.incarnationId ? { incarnationId: concurrentOwner.incarnationId } : {}), - stablePaneOwner: { - handle: concurrentOwner.handle, - tabId: concurrentOwner.tabId, - leafId: concurrentOwner.leafId - } - } - restoreProvisionalPtySize({ - sessionId: ctx.sessionId, - sessionAppId: ctx.effectiveSessionAppId, - reattachId: reattach.id, - hadSessionSizeBeforeAttach: ctx.hadSessionSizeBeforeAttach, - sessionSizeBeforeAttach: ctx.sessionSizeBeforeAttach - }) - return reattach - } - } + ctx.paneSpawnReservationKey = resolvedPaneSpawnReservationKey ctx.finishTerminalInstall = beginPtySpawnForWorktree(args.worktreeId, ctx.cwd, args.connectionId) - ctx.paneSpawnReservation ??= ctx.paneSpawnReservationKey - ? reservePaneSpawn(ctx.paneSpawnReservationKey) - : null ctx.stablePaneOwner = null ctx.stablePaneBindingPersisted = false ctx.rejectedRegistrationCandidate = null diff --git a/src/main/ipc/pty/runtime/spawn-state.ts b/src/main/ipc/pty/runtime/spawn-state.ts index 57a3ef2394a..36516055476 100644 --- a/src/main/ipc/pty/runtime/spawn-state.ts +++ b/src/main/ipc/pty/runtime/spawn-state.ts @@ -1,3 +1,5 @@ +import type { PtyRegistrationClaim } from '../../../runtime/runtime-terminal-state-records' +import type { PaneSpawnOperation } from '../pane/pane-spawn-operation' import type { AgentLaunchRuntime } from '../../../../shared/agent-launch-runtime' import type { SpawnIntent } from '../../../../shared/host-spawn-admission' import type { @@ -81,6 +83,7 @@ export type RuntimePtySpawnState = { stablePaneBindingPersisted: boolean rejectedRegistrationCandidate: PtySpawnResult | null pendingRegistrationPtyId: string | null + pendingRegistrationClaim?: PtyRegistrationClaim reconciledSnapshotSeq: number | null snapshotKittyFlagsCoverReconciledSeq: boolean preparedProvisionalExecutionContext: boolean @@ -131,6 +134,7 @@ export type RuntimePtySpawnArgs = { signal?: AbortSignal refuseSleptWorktree?: boolean onPtySpawnCommitted?: () => void + paneSpawnOperation?: PaneSpawnOperation adoptedStablePane?: { result: PtySpawnResult owner: { diff --git a/src/main/ipc/pty/runtime/spawn.ts b/src/main/ipc/pty/runtime/spawn.ts index 42e8a2764e5..b9bc0ddbeb5 100644 --- a/src/main/ipc/pty/runtime/spawn.ts +++ b/src/main/ipc/pty/runtime/spawn.ts @@ -1,13 +1,7 @@ import type { AgentSessionClaimedSpawnResult } from '../../../../shared/agent-session-host-authority' -import { isTerminalLeafId, makePaneKey } from '../../../../shared/stable-pane-id' +import { isTerminalLeafId, parsePaneKey } from '../../../../shared/stable-pane-id' import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id' -import { - makePaneSpawnReservationKey, - paneSpawnReservationsByOwnerKey, - rejectPaneSpawnReservation, - reservePaneSpawn -} from '../pane/spawn-reservation' -import { resolveStablePaneOwner } from '../pane/stable-owner' +import { rejectPaneSpawnReservation } from '../pane/spawn-reservation' import { restoreProvisionalPtySize } from '../spawn-options-assembly' import type { PtyRuntimeControllerDeps } from './controller-deps' import { adoptMaterializedRuntimePtySpawn } from './spawn-early' @@ -41,33 +35,53 @@ export async function spawnPtyFromRuntimeController( deps: PtyRuntimeControllerDeps, args: RuntimePtySpawnArgs ) { - const ctx = createRuntimePtySpawnState(deps, args) - if (!args.adoptedStablePane) { - const leafId = - typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null - const paneKey = - typeof args.worktreeId === 'string' && - typeof args.tabId === 'string' && - isValidTerminalTabId(args.tabId) && - args.tabId.length <= 512 && - leafId - ? makePaneKey(args.tabId, leafId) - : null - const ownerKey = makePaneSpawnReservationKey(args.worktreeId, args.connectionId, paneKey) - const existingOwner = paneKey - ? resolveStablePaneOwner( - deps.runtime, - deps.store, - paneKey, - args.worktreeId, - args.connectionId - ) - : null - if (ownerKey && !existingOwner && !paneSpawnReservationsByOwnerKey.has(ownerKey)) { - ctx.paneSpawnReservationKey = ownerKey - ctx.paneSpawnReservation = reservePaneSpawn(ownerKey) - } + if ( + args.paneSpawnOperation || + !args.tabId || + !args.leafId || + !isValidTerminalTabId(args.tabId) || + !isTerminalLeafId(args.leafId) + ) { + return spawnFreshPtyFromRuntimeController(deps, args) } + let freshResult: Awaited> | undefined + const adopted = await deps.adoptStablePane({ + cols: args.cols, + rows: args.rows, + cwd: args.cwd, + worktreeId: args.worktreeId, + tabId: args.tabId, + leafId: args.leafId, + connectionId: args.connectionId, + preAllocatedHandle: args.preAllocatedHandle, + signal: args.signal, + completed: args.adoptedStablePane, + spawnFresh: async (operation) => { + freshResult = await spawnFreshPtyFromRuntimeController(deps, { + ...args, + paneSpawnOperation: operation + }) + return freshResult + } + }) + if (!adopted) { + return toRuntimeSpawnReply(freshResult!) + } + if (!args.worktreeId) { + return toRuntimeSpawnReply(adopted.result) + } + const completed = await adoptMaterializedRuntimePtySpawn( + createRuntimePtySpawnState(deps, { ...args, adoptedStablePane: adopted }) + ) + return toRuntimeSpawnReply(completed ?? adopted.result) +} + +async function spawnFreshPtyFromRuntimeController( + deps: PtyRuntimeControllerDeps, + args: RuntimePtySpawnArgs +) { + const ctx = createRuntimePtySpawnState(deps, args) + try { const materializedOrPromise = adoptMaterializedRuntimePtySpawn(ctx) const materialized = @@ -91,16 +105,41 @@ export async function spawnPtyFromRuntimeController( if (earlyReserved) { return toRuntimeSpawnReply(earlyReserved) } - await executeRuntimePtySpawn(ctx) - const committed = await commitRuntimePtySpawn(ctx) - commitRuntimeSpawnHiddenDelivery(ctx) - return toRuntimeSpawnReply(committed) + const complete = async (operation = args.paneSpawnOperation) => { + ctx.args.paneSpawnOperation = operation + await executeRuntimePtySpawn(ctx) + const committed = await commitRuntimePtySpawn(ctx) + commitRuntimeSpawnHiddenDelivery(ctx) + return committed + } + const latePane = + !args.paneSpawnOperation && ctx.spawnIdentityPaneKey + ? parsePaneKey(ctx.spawnIdentityPaneKey) + : null + if (latePane) { + ctx.args = { ...ctx.args, ...latePane } + let result: Awaited> | undefined + const adopted = await deps.adoptStablePane({ + cols: args.cols, + rows: args.rows, + cwd: args.cwd, + worktreeId: args.worktreeId, + connectionId: args.connectionId, + ...latePane, + signal: args.signal, + preAllocatedHandle: args.preAllocatedHandle, + spawnFresh: async (operation) => (result = await complete(operation)) + }) + return toRuntimeSpawnReply(adopted?.result ?? result!) + } + return await complete() } catch (err) { releaseRuntimeSpawnPreSpawnHiddenMark(ctx) if (ctx.pendingRegistrationPtyId) { deps.runtime?.cancelPendingPtyRegistration?.( ctx.pendingRegistrationPtyId, - ctx.rejectedRegistrationCandidate?.incarnationId + ctx.rejectedRegistrationCandidate?.incarnationId, + ctx.pendingRegistrationClaim ) ctx.pendingRegistrationPtyId = null } diff --git a/src/main/ipc/pty/spawn-execute-failure.ts b/src/main/ipc/pty/spawn-execute-failure.ts index 04b678ff139..ffb349d4c19 100644 --- a/src/main/ipc/pty/spawn-execute-failure.ts +++ b/src/main/ipc/pty/spawn-execute-failure.ts @@ -1,3 +1,4 @@ +import type { PtyRegistrationClaim } from '../../runtime/runtime-terminal-state-records' import type { PtySpawnResult } from '../../providers/pty-provider-contract' import type { OrcaRuntimeService } from '../../runtime/orca-runtime' import { classifyError } from '../../telemetry/classify-error' @@ -17,6 +18,7 @@ type PtySpawnExecuteState = { sessionSizeBeforeAttach: { cols: number; rows: number } | undefined preparedProvisionalExecutionContext: boolean pendingRegistrationPtyId: string | null + pendingRegistrationClaim?: PtyRegistrationClaim rejectedRegistrationCandidate: PtySpawnResult | null } @@ -42,17 +44,11 @@ export function failPtySpawnExecute( resetIncarnation: true }) } - const rawMessage = err instanceof Error ? err.message : String(err) - if (rawMessage === 'agent_session_exited_during_start' && ctx.rejectedRegistrationCandidate) { - runtime?.releaseRejectedPtyRegistrationFence?.( - ctx.rejectedRegistrationCandidate.id, - ctx.rejectedRegistrationCandidate.incarnationId - ) - } if (ctx.pendingRegistrationPtyId) { runtime?.cancelPendingPtyRegistration?.( ctx.pendingRegistrationPtyId, - ctx.rejectedRegistrationCandidate?.incarnationId + ctx.rejectedRegistrationCandidate?.incarnationId, + ctx.pendingRegistrationClaim ) ctx.pendingRegistrationPtyId = null } diff --git a/src/main/persistence/loading-store/pty-binding-persistence.ts b/src/main/persistence/loading-store/pty-binding-persistence.ts index 7ac6bd2ec14..075249d8382 100644 --- a/src/main/persistence/loading-store/pty-binding-persistence.ts +++ b/src/main/persistence/loading-store/pty-binding-persistence.ts @@ -1,4 +1,5 @@ import { LOCAL_EXECUTION_HOST_ID, parseExecutionHostId } from '../../../shared/execution-host' +import { makePaneKey } from '../../../shared/stable-pane-id' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { rollbackFailedPtyBinding } from './pty-binding-write-rollback' import { cloneWorkspaceSessionState } from '../restoring-sessions/session-owner-fields' @@ -110,10 +111,14 @@ export class PtyBindingPersistenceOperations { this[ptyBindingPersistenceOperationsContext] = { runtime, sessions } } - /** Clears a stopped process's binding, keeping the pane; fenced on the id, which is dead in any incarnation. */ + /** + * Clears a stopped process's binding, keeping the pane; fenced on the id, which is dead in any + * incarnation. `fence.incarnationId` narrows that to one process, for an absence answer. + */ async retirePtyBinding( binding: Pick, - hostId?: string | null + hostId?: string | null, + fence?: { incarnationId: string | undefined } ): Promise { const { runtime, sessions } = this[ptyBindingPersistenceOperationsContext] const resolved = resolveHostId(hostId) @@ -138,6 +143,13 @@ export class PtyBindingPersistenceOperations { if (currentId !== binding.ptyId) { return { value: false, persist: false } } + if ( + fence && + session.terminalPtyIncarnationsByPaneKey?.[makePaneKey(binding.tabId, binding.leafId)] !== + fence.incarnationId + ) { + return { value: false, persist: false } + } if (!session.tabsByWorktree[binding.worktreeId]?.some((tab) => tab.id === binding.tabId)) { return { value: false, persist: false } } diff --git a/src/main/persistence/loading-store/pty-binding-restart-retirement.spec.ts b/src/main/persistence/loading-store/pty-binding-restart-retirement.spec.ts index d5f19ae114f..305f46e290e 100644 --- a/src/main/persistence/loading-store/pty-binding-restart-retirement.spec.ts +++ b/src/main/persistence/loading-store/pty-binding-restart-retirement.spec.ts @@ -55,6 +55,24 @@ export function registerPtyBindingRestartRetirementTests(): void { ).toEqual({}) }) + it('keeps a same-id incarnation published while an unstamped retirement waits in the Store queue', async () => { + const { store, authority } = await fixture() + await store.persistPtyBinding({ ...binding, incarnationId: undefined }) + const gate = authority.pause() + const publish = store.persistPtyBinding(binding) + await gate.started.promise + const retire = store.retirePtyBinding(binding, undefined, { incarnationId: undefined }) + gate.finish.resolve() + await publish + expect(await retire).toBe(false) + expect( + store.getWorkspaceSession().terminalLayoutsByTabId[binding.tabId].ptyIdsByLeafId?.[ + binding.leafId + ] + ).toBe(binding.ptyId) + expect(await store.retirePtyBinding(binding)).toBe(true) + }) + it('restores the old binding after a known save failure and allows another attempt', async () => { const { store, authority } = await fixture() vi.spyOn(console, 'error').mockImplementation(() => {}) diff --git a/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts b/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts index afaf6023ad2..f42dbe046c4 100644 --- a/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts +++ b/src/main/persistence/loading-store/pty-spawn-exit-durability.test.ts @@ -1,3 +1,11 @@ +import { SessionNotFoundError } from '../../daemon/daemon-errors' +import { completePaneAdoption } from '../../ipc/pty/pane/complete-pane-adoption' +import { LocalPtyProvider } from '../../providers/local-pty-provider' +import { adoptStablePane } from '../../ipc/pty/pane/adopt-stable' +import * as providerRegistry from '../../ipc/pty/provider/registry' +import { bindProviderListeners } from '../../ipc/pty/provider/bind-listeners' +import { createPtyIpcSession } from '../../ipc/pty/session' +import { DaemonPtyAdapter } from '../../daemon/daemon-pty-adapter' import { expect, it, vi } from 'vitest' import { fixture, admitRuntimeSpawnPane } from './profile-state-delayed-authority-fixture' import { OrcaRuntimeService } from '../../runtime/orca-runtime' @@ -59,13 +67,33 @@ it.each( if (owner) { await store.persistPtyBinding({ ...binding, incarnationId: owner.persistedIncarnationId }) } - runtime.onPtySpawned(binding.ptyId, binding.incarnationId) - runtime.beginPtyRegistration(binding.ptyId, binding.incarnationId) - runtime.assertPtyRegistrationAllowed(binding.ptyId, binding.incarnationId) + if (!stableOwner) { + runtime.onPtySpawned(binding.ptyId, binding.incarnationId) + runtime.beginPtyRegistration(binding.ptyId, binding.incarnationId) + } let commit: () => Promise const reservationKey = JSON.stringify([connectionId, binding.worktreeId, binding.leafId]) let reservation: PaneSpawnReservation | undefined - if (controller === 'runtime') { + if (owner) { + const provider = new LocalPtyProvider() + vi.spyOn(provider, 'spawn').mockResolvedValue({ + id: binding.ptyId, + incarnationId: binding.incarnationId, + isReattach: true + }) + commit = async () => + ( + await completePaneAdoption({ + runtime, + store, + owner, + provider, + worktreeId: binding.worktreeId, + renderer: controller === 'ipc', + spawnOptions: { cols: 80, rows: 24 } + }) + )?.result + } else if (controller === 'runtime') { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only commit runs; its runtime/store are real and preflight-only dependencies are unreachable. const deps = { runtime, store, options: {} } as PtyRuntimeControllerDeps const ctx = createRuntimePtySpawnState(deps, { ...binding, connectionId, cols: 80, rows: 24 }) @@ -146,7 +174,15 @@ it.each(['replacement-pty', binding.ptyId])( }) await gate.started.promise const registration = expect( - registerPersistedPtySpawn(runtime, store, binding.ptyId, binding.worktreeId, null, binding) + registerPersistedPtySpawn( + runtime, + store, + undefined, + binding.ptyId, + binding.worktreeId, + null, + binding + ) ).rejects.toThrow('agent_session_exited_during_start') gate.finish.resolve() await Promise.all([replacement, registration]) @@ -165,7 +201,15 @@ it('retains the binding when loss of contact supplies no process-exit proof', as await store.persistPtyBinding(binding) await runtime.onPtyExit(binding.ptyId, -1, binding.incarnationId) expect(() => - registerPersistedPtySpawn(runtime, store, binding.ptyId, binding.worktreeId, null, binding) + registerPersistedPtySpawn( + runtime, + store, + undefined, + binding.ptyId, + binding.worktreeId, + null, + binding + ) ).toThrow('agent_session_exited_during_start') expect( readState().workspaceSession.terminalLayoutsByTabId[binding.tabId].ptyIdsByLeafId[ @@ -185,6 +229,7 @@ it('does not settle rejected registration until its exit cleanup reaches SQLite' const cleanup = registerPersistedPtySpawn( runtime, store, + undefined, binding.ptyId, binding.worktreeId, null, @@ -219,6 +264,190 @@ it('keeps successful registration synchronous through the remaining spawn public const runtime = new OrcaRuntimeService(store) await store.persistPtyBinding(binding) expect( - registerPersistedPtySpawn(runtime, store, binding.ptyId, binding.worktreeId, null, binding) + registerPersistedPtySpawn( + runtime, + store, + undefined, + binding.ptyId, + binding.worktreeId, + null, + binding + ) ).toBeUndefined() }) + +it.each(['success', 'new-exit', 'old-exit'] as const)( + 'retains candidate authority over a disconnected old record: %s', + async (event) => { + const { store, authority, readState } = await fixture() + const runtime = new OrcaRuntimeService(store) + await store.persistPtyBinding(binding) + runtime.registerPty(binding.ptyId, binding.worktreeId, null, binding) + runtime.intentionalPtyStops.mark(binding.ptyId, 'replaced', binding.incarnationId) + await runtime.onPtyExit(binding.ptyId, 0, binding.incarnationId, { providerExitObserved: true }) + clearProviderPtyState(binding.ptyId) + ptyOwnership.delete(binding.ptyId) + expect( + runtime.resolveTerminalPane(`${binding.tabId}:${binding.leafId}`, binding.worktreeId) + ).toMatchObject({ connected: false, incarnationId: binding.incarnationId }) + const begin = vi.spyOn(runtime, 'beginPtyRegistration') + const provider = new DaemonPtyAdapter({ + socketPath: '/unused-round3', + tokenPath: '/unused-round3-token' + }) + const spawn = vi.spyOn(provider, 'spawn').mockResolvedValue({ + id: binding.ptyId, + incarnationId: 'new-incarnation', + isReattach: true, + snapshot: 'history before attach\r\n', + providerSequence: { value: 10, generation: 'continued' } + }) + let emitExit: Parameters[0] = () => {} + vi.spyOn(provider, 'onExit').mockImplementation((listener) => { + emitExit = listener + return () => {} + }) + providerRegistry.setLocalPtyProvider(provider) + const session = createPtyIpcSession({ runtime, store }) + session.sendPtyExitToRenderer = vi.fn() + session.consumeSyntheticKillExit = () => false + bindProviderListeners(session) + const gate = authority.pause() + const operation = adoptStablePane(runtime, store, { ...binding, cols: 80, rows: 24 }) + const settled = operation.then( + (value) => ({ value, error: null }), + (error) => ({ value: null, error }) + ) + await gate.started.promise + runtime.onPtyData(binding.ptyId, 'live during save\r\n', 11) + if (event === 'new-exit' || event === 'old-exit') { + emitExit({ + id: binding.ptyId, + code: 0, + incarnationId: event === 'new-exit' ? 'new-incarnation' : binding.incarnationId + }) + } + gate.finish.resolve() + const result = await settled + if (event === 'new-exit') { + expect.soft(result.error?.message).toBe('agent_session_exited_during_start') + expect + .soft( + readState().workspaceSession.terminalLayoutsByTabId[binding.tabId]?.ptyIdsByLeafId?.[ + binding.leafId + ] + ) + .toBeUndefined() + expect + .soft( + runtime.resolveTerminalPane(`${binding.tabId}:${binding.leafId}`, binding.worktreeId) + .connected + ) + .toBe(false) + } else { + expect(result.error).toBeNull() + expect(result.value?.result).toMatchObject({ + incarnationId: 'new-incarnation', + snapshotSeq: 10 + }) + const history = await runtime.serializeMainTerminalBuffer(binding.ptyId) + expect(history?.data).toContain('history before attach') + expect(history?.data).toContain('live during save') + expect(runtime.getPtyOutputSequence(binding.ptyId)).toBe(28) + } + expect( + runtime.narrowPtyRegistration(binding.ptyId, begin.mock.results[0].value, 'new-incarnation') + ).toBe(false) + expect(spawn.mock.calls.every(([args]) => args.attachOnly && args.command === undefined)).toBe( + true + ) + provider.dispose() + } +) + +it.each(['joined', 'fresh'] as const)( + 'completes %s owner loss through B with final history and fenced fresh cleanup', + async (boundary) => { + const { store, authority, readState } = await fixture() + const runtime = new OrcaRuntimeService(store) + await store.persistPtyBinding(binding) + const provider = new LocalPtyProvider() + vi.spyOn(providerRegistry, 'getProvider').mockReturnValue(provider) + const shutdown = vi.spyOn(provider, 'shutdown').mockResolvedValue() + const spawn = vi.spyOn(provider, 'spawn').mockImplementation(async (args) => { + if (args.attachOnly && args.sessionId === binding.ptyId) { + throw new SessionNotFoundError(binding.ptyId) + } + return args.attachOnly + ? { + id: 'winner-B', + incarnationId: 'inc-B', + isReattach: true, + snapshot: 'B history\r\n', + providerSequence: { value: 20, generation: 'continued' } + } + : { id: 'fresh-A', incarnationId: 'inc-A' } + }) + const key = JSON.stringify([null, binding.worktreeId, `${binding.tabId}:${binding.leafId}`]) + const reservation = boundary === 'joined' ? reservePaneSpawn(key) : undefined + const operation = adoptStablePane(runtime, store, { + ...binding, + cols: 80, + rows: 24, + spawnFresh: async (paneSpawnOperation) => { + const result = await provider.spawn({ cols: 80, rows: 24 }) + ptyIncarnationById.set(result.id, result.incarnationId!) + await store.persistPtyBinding({ ...binding, ptyId: 'winner-B', incarnationId: 'inc-B' }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: actual commit uses the real runtime/store; launch preparation is outside this candidate-loss flow. + const deps = { runtime, store, options: {} } as PtyRuntimeControllerDeps + const ctx = createRuntimePtySpawnState(deps, { + ...binding, + cols: 80, + rows: 24, + paneSpawnOperation + }) + ctx.provider = provider + ctx.result = result + ctx.hostSessionBinding = { store, ...binding } + ctx.metadataLeafId = binding.leafId + return commitRuntimePtySpawn(ctx) + } + }) + if (reservation) { + await store.persistPtyBinding({ ...binding, ptyId: 'winner-B', incarnationId: 'inc-B' }) + resolvePaneSpawnReservation(key, reservation, { id: 'joined-A', incarnationId: 'inc-A' }) + } + const result = await operation + expect(result?.result).toMatchObject({ + id: 'winner-B', + incarnationId: 'inc-B', + snapshotSeq: 20 + }) + expect( + runtime.resolveTerminalPane(`${binding.tabId}:${binding.leafId}`, binding.worktreeId) + ).toMatchObject({ ptyId: 'winner-B', connected: true, incarnationId: 'inc-B' }) + expect((await runtime.serializeMainTerminalBuffer('winner-B'))?.data).toContain('B history') + expect( + store.getWorkspaceSession().terminalLayoutsByTabId[binding.tabId]?.ptyIdsByLeafId?.[ + binding.leafId + ] + ).toBe('winner-B') + store.updateSettings({ theme: 'dark' }) + await store.flushPendingOrThrowAsync() + expect(readState().settings.theme).toBe('dark') + expect(authority.close).not.toHaveBeenCalled() + if (boundary === 'fresh') { + expect(shutdown).toHaveBeenCalledExactlyOnceWith('fresh-A', { + immediate: true, + expectedIncarnationId: 'inc-A' + }) + } else { + expect(shutdown).not.toHaveBeenCalled() + } + expect( + spawn.mock.calls + .filter(([args]) => args.attachOnly) + .every(([args]) => args.command === undefined) + ).toBe(true) + } +) diff --git a/src/main/persistence/loading-store/pty-spawn-replacement-durability.test.ts b/src/main/persistence/loading-store/pty-spawn-replacement-durability.test.ts index 96043f9084d..c4ecda048b9 100644 --- a/src/main/persistence/loading-store/pty-spawn-replacement-durability.test.ts +++ b/src/main/persistence/loading-store/pty-spawn-replacement-durability.test.ts @@ -5,10 +5,8 @@ import { deferred, fixture, admitRuntimeSpawnPane } from './profile-state-delaye import { OrcaRuntimeService } from '../../runtime/orca-runtime' import { commitPtyIpcSpawn } from '../../ipc/pty/ipc/spawn-commit' import { createPtyIpcSpawnState } from '../../ipc/pty/ipc/spawn-state' -import type { PtySpawnIpcDeps } from '../../ipc/pty/ipc/spawn-types' import { commitRuntimePtySpawn } from '../../ipc/pty/runtime/spawn-commit' import { createRuntimePtySpawnState } from '../../ipc/pty/runtime/spawn-state' -import type { PtyRuntimeControllerDeps } from '../../ipc/pty/runtime/controller-deps' import { ptyIncarnationById, ptyOwnership } from '../../ipc/pty/provider/ownership-state' import { clearProviderPtyState } from '../../ipc/pty/provider/state-cleanup' import { createPtySpawnCommitDependencies } from './pty-spawn-commit-dependencies-fixture' @@ -124,19 +122,19 @@ it.each(['ipc', 'runtime'])( const { store, authority } = await fixture() const runtime = new OrcaRuntimeService(store) runtime.onPtySpawned(binding.ptyId, binding.incarnationId) + const deps = createPtySpawnCommitDependencies(runtime, store) + const claim = runtime.beginPtyRegistration(binding.ptyId, binding.incarnationId) let commit: () => Promise if (controller === 'ipc') { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only commit runs; the runtime/store are real and preflight-only dependencies are unreachable. - const deps = { runtime, store } as PtySpawnIpcDeps const ctx = createPtyIpcSpawnState(deps, { ...binding, cols: 80, rows: 24 }) + ctx.pendingRegistrationClaim = claim ctx.result = { id: binding.ptyId, incarnationId: binding.incarnationId } ctx.metadataLeafId = binding.leafId ctx.validatedLeafId = binding.leafId commit = () => commitPtyIpcSpawn(ctx) } else { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only commit runs; the runtime/store are real and preflight-only dependencies are unreachable. - const deps = { runtime, store } as PtyRuntimeControllerDeps const ctx = createRuntimePtySpawnState(deps, { ...binding, cols: 80, rows: 24 }) + ctx.pendingRegistrationClaim = claim ctx.result = { id: binding.ptyId, incarnationId: binding.incarnationId } ctx.metadataLeafId = binding.leafId await admitRuntimeSpawnPane(store, binding) @@ -144,7 +142,7 @@ it.each(['ipc', 'runtime'])( commit = () => commitRuntimePtySpawn(ctx) } const gate = authority.pause() - const pending = expect(commit()).rejects.toThrow('agent_session_exited_during_start') + const pending = expect(commit()).rejects.toThrow('terminal_pane_owner_changed') await gate.started.promise await runtime.onPtyExit(binding.ptyId, 0, binding.incarnationId, { providerExitObserved: true }) runtime.onPtySpawned(binding.ptyId, replacementIncarnation) diff --git a/src/main/runtime/construction/runtime-identity-state.ts b/src/main/runtime/construction/runtime-identity-state.ts index 88afd99ff72..9e39d2e0c11 100644 --- a/src/main/runtime/construction/runtime-identity-state.ts +++ b/src/main/runtime/construction/runtime-identity-state.ts @@ -36,7 +36,7 @@ import type { PtyControllerInventory, RuntimePtyController } from '../runtime-pty-controller-contract' -import type { RuntimeLeafRecord } from '../runtime-terminal-state-records' +import type { RuntimeLeafRecord, PtyRegistrationClaim } from '../runtime-terminal-state-records' import type { TerminalHandleRecord } from '../runtime-terminal-contracts' import type { PtyIncarnationHandleRecord } from '../orca-runtime-core' import { MailPointerRepointScheduler } from '../orchestration/mail-pointer-repoint-scheduler' @@ -236,10 +236,8 @@ export abstract class OrcaRuntimeWithRuntimeId extends RuntimeConstructionServic // Why: one watermark per repo replaces per-closed-pane fences while preserving stale-write safety. protected terminalTopologyRevisionByRepoId = new Map() - // Why: provider exit can beat surface registration; that exact dead incarnation must never publish. - protected earlyExitedPtyIncarnations = new Map() - - protected pendingPtyRegistrationIncarnations = new Map() + // The candidate receipt owns its exit evidence until publication or cancellation. + protected pendingPtyRegistrationIncarnations = new Map() // Why public: the PTY IPC layer's stop paths write it and its exit delivery reads it. readonly intentionalPtyStops = new TerminalIntentionalStops() diff --git a/src/main/runtime/orca-runtime-create-terminal-dependencies.ts b/src/main/runtime/orca-runtime-create-terminal-dependencies.ts index 6bcc07ba0a6..600b5cb965a 100644 --- a/src/main/runtime/orca-runtime-create-terminal-dependencies.ts +++ b/src/main/runtime/orca-runtime-create-terminal-dependencies.ts @@ -14,6 +14,7 @@ export { makePaneKey } from '../../shared/stable-pane-id' export { randomUUID } from 'node:crypto' export { admitStablePaneAdoption, + materializedStablePaneReply, allocateTerminalPaneIdentity } from './runtime-terminal-pane-identity' export { diff --git a/src/main/runtime/orca-runtime-terminal-retirement.test.ts b/src/main/runtime/orca-runtime-terminal-retirement.test.ts index d6f70fd4c6e..d7f0ef96d38 100644 --- a/src/main/runtime/orca-runtime-terminal-retirement.test.ts +++ b/src/main/runtime/orca-runtime-terminal-retirement.test.ts @@ -152,7 +152,7 @@ describe('OrcaRuntimeService terminal surface retirement', () => { it('releases each early-exit fence after its matching registration is rejected', async () => { const runtime = new OrcaRuntimeService() const internals = runtime as unknown as { - earlyExitedPtyIncarnations: Map + pendingPtyRegistrationIncarnations: Map } for (let index = 0; index < 1_000; index += 1) { @@ -166,13 +166,12 @@ describe('OrcaRuntimeService terminal surface retirement', () => { runtime.releaseRejectedPtyRegistrationFence(ptyId, incarnationId) } - expect(internals.earlyExitedPtyIncarnations.size).toBe(0) + expect(internals.pendingPtyRegistrationIncarnations.size).toBe(0) }) it('does not retain fences for completed surface-less lifecycles', async () => { const runtime = new OrcaRuntimeService() const internals = runtime as unknown as { - earlyExitedPtyIncarnations: Map pendingPtyRegistrationIncarnations: Map } @@ -183,7 +182,6 @@ describe('OrcaRuntimeService terminal surface retirement', () => { await runtime.onPtyExit(`pty-headless-${index}`, 0, `incarnation-${index}`) } - expect(internals.earlyExitedPtyIncarnations.size).toBe(0) expect(internals.pendingPtyRegistrationIncarnations.size).toBe(0) }) @@ -205,10 +203,8 @@ describe('OrcaRuntimeService terminal surface retirement', () => { ).toThrow('agent_session_exited_during_start') runtime.releaseRejectedPtyRegistrationFence('pty-left', 'incarnation-replacement') const internals = runtime as unknown as { - earlyExitedPtyIncarnations: Map pendingPtyRegistrationIncarnations: Map } - expect(internals.earlyExitedPtyIncarnations.size).toBe(0) expect(internals.pendingPtyRegistrationIncarnations.size).toBe(0) }) diff --git a/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts b/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts index 7da878ca2f7..f5d454a609f 100644 --- a/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts +++ b/src/main/runtime/orca-runtime-tests/exit-retirement-activation.spec.ts @@ -1,3 +1,4 @@ +import type { AdoptStablePaneArgs } from '../../ipc/pty/ipc/spawn-types' import { describe, expect, it, vi } from 'vitest' import { OrcaRuntimeService } from '../orca-runtime-test-mocks.spec' import { @@ -38,7 +39,10 @@ async function startSplitHost() { const spawn = vi.fn(async (options: { sessionId?: string }) => ({ id: options.sessionId ?? 'fresh-pty' })) - const adoptStablePane = vi.fn(async () => null) + const adoptStablePane = vi.fn(async (args: AdoptStablePaneArgs) => { + await args.spawnFresh?.({ mustExist: false }) + return null + }) const runtime = new OrcaRuntimeService(runtimeStore) runtime.setPtyController({ spawn, diff --git a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-12.spec.ts b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-12.spec.ts index da4cdbf6376..c9d8d94cf0b 100644 --- a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-12.spec.ts +++ b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-12.spec.ts @@ -360,6 +360,10 @@ describe('OrcaRuntimeService', () => { it('still attaches to a live pane when the caller did not require a fresh one', async () => { // `terminal.create` keeps its reattach; only agent.launch opts into the refusal. const { runtime, revealTerminalSession } = runtimeWithDesktopWindow(LIVE_PANE_SPAWN) + runtime.registerPty('pty-live', TEST_WORKTREE_ID, null, { + ...LIVE_PANE_SPAWN.stablePaneOwner, + terminalHandle: 'term_live' + }) const created = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { startupAgent: 'claude', diff --git a/src/main/runtime/orca-runtime-tests/terminal-handles-and-agent-status-part-04.spec.ts b/src/main/runtime/orca-runtime-tests/terminal-handles-and-agent-status-part-04.spec.ts index 24f39579698..b7b21d26f98 100644 --- a/src/main/runtime/orca-runtime-tests/terminal-handles-and-agent-status-part-04.spec.ts +++ b/src/main/runtime/orca-runtime-tests/terminal-handles-and-agent-status-part-04.spec.ts @@ -1,3 +1,4 @@ +import type { AdoptStablePaneArgs, AdoptStablePaneResult } from '../../ipc/pty/ipc/spawn-types' import { describe, expect, it, vi } from 'vitest' import { OrcaRuntimeService } from '../orca-runtime-test-mocks.spec' import { @@ -490,7 +491,12 @@ describe('OrcaRuntimeService', () => { }) it('preserves host metadata when terminal.create adopts a stable pane owner', async () => { - const adoptStablePane = vi.fn().mockResolvedValue(null) + const adoptStablePane = vi.fn( + async (args: AdoptStablePaneArgs): Promise => { + await args.spawnFresh?.({ mustExist: false }) + return null + } + ) const spawn = vi.fn(async (opts: { adoptedStablePane?: { owner: { handle?: string } } }) => opts.adoptedStablePane ? { @@ -526,6 +532,7 @@ describe('OrcaRuntimeService', () => { launchAgent: 'claude' }) adoptStablePane.mockResolvedValueOnce({ + materialized: true, result: { id: 'pty-stable-owner', isReattach: true }, owner: { handle: first.handle, @@ -557,13 +564,7 @@ describe('OrcaRuntimeService', () => { launchAgent: 'claude' }) ]) - expect(spawn.mock.calls[1]?.[0]).toMatchObject({ - command: "claude 'replacement'", - adoptedStablePane: expect.anything() - }) - expect(spawn.mock.calls[1]?.[0]).not.toMatchObject({ - command: expect.stringContaining('--teammate-mode') - }) + expect(spawn).toHaveBeenCalledOnce() }) it('releases a stable-pane claim when creation aborts before provider spawn', async () => { @@ -602,8 +603,9 @@ describe('OrcaRuntimeService', () => { const spawn = vi.fn() const runtime = new OrcaRuntimeService(store) runtime.setPtyController({ - adoptStablePane: vi.fn(async () => { + adoptStablePane: vi.fn(async (args: AdoptStablePaneArgs) => { abort.abort() + await args.spawnFresh?.({ mustExist: false }) return null }), spawn, @@ -630,7 +632,10 @@ describe('OrcaRuntimeService', () => { }) const runtime = new OrcaRuntimeService(store) runtime.setPtyController({ - adoptStablePane: vi.fn(async () => null), + adoptStablePane: vi.fn(async (args: AdoptStablePaneArgs) => { + await args.spawnFresh?.({ mustExist: false }) + return null + }), spawn, write: () => true, kill: () => true, diff --git a/src/main/runtime/runtime-durable-store-fixture.ts b/src/main/runtime/runtime-durable-store-fixture.ts index c65c3a86a82..26cd7bcdb4e 100644 --- a/src/main/runtime/runtime-durable-store-fixture.ts +++ b/src/main/runtime/runtime-durable-store-fixture.ts @@ -52,7 +52,8 @@ export function withDurableRuntimeStore< /** The Store's binding-only retirement, over the fake's own session accessors. */ retirePtyBinding( binding: { worktreeId: string; tabId: string; leafId: string; ptyId: string }, - hostId?: string | null + hostId?: string | null, + fence?: { incarnationId: string | undefined } ): Promise { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: fakes that reach retirement are Store-shaped sessions; the accessors are optional and checked before use. const sessions = store as { @@ -69,6 +70,9 @@ export function withDurableRuntimeStore< } if ( bound !== binding.ptyId || + (fence && + session.terminalPtyIncarnationsByPaneKey?.[`${binding.tabId}:${binding.leafId}`] !== + fence.incarnationId) || !session.tabsByWorktree?.[binding.worktreeId]?.some((tab) => tab.id === binding.tabId) ) { return { value: false, persist: false } diff --git a/src/main/runtime/runtime-pane-admission.ts b/src/main/runtime/runtime-pane-admission.ts index 774500ddf4b..93732cd9152 100644 --- a/src/main/runtime/runtime-pane-admission.ts +++ b/src/main/runtime/runtime-pane-admission.ts @@ -1,3 +1,4 @@ +import { PaneOwnerContentionError } from '../ipc/pty/pane/pane-spawn-operation' import { toSshExecutionHostId } from '../../shared/execution-host' import type { TerminalPaneAdmission, @@ -118,7 +119,7 @@ export async function startInAdmittedPane( try { return { outcome, result: await start(outcome) } } catch (error) { - if (outcome === 'admitted' && !keepOnFailure) { + if (outcome === 'admitted' && !keepOnFailure && !(error instanceof PaneOwnerContentionError)) { await withdrawAdmittedPane(store, admission, hostId) } throw error @@ -136,6 +137,9 @@ export async function spawnInAdmittedPane( { startStatus, ...args }: AdmittedSpawnArgs & PaneStartArgs ): ReturnType { const { groupId: _groupId, afterTabId: _afterTabId, ...spawnArgs } = args + if (args.paneSpawnOperation?.mustExist) { + return controller.spawn!(spawnArgs) + } const { outcome, result } = await startInAdmittedPane( store, admissionFor(args), diff --git a/src/main/runtime/runtime-pty-controller-contract.ts b/src/main/runtime/runtime-pty-controller-contract.ts index 0b06d5e4326..14fcdf300d5 100644 --- a/src/main/runtime/runtime-pty-controller-contract.ts +++ b/src/main/runtime/runtime-pty-controller-contract.ts @@ -1,3 +1,4 @@ +import type { PaneSpawnOperation } from '../ipc/pty/pane/pane-spawn-operation' import type { AgentLaunchRuntime } from '../../shared/agent-launch-runtime' import type { AutomaticWorkResumeOutcome, @@ -38,6 +39,8 @@ export type RuntimePtyController = { leafId: string }): () => void adoptStablePane?(opts: { + signal?: AbortSignal + spawnFresh?: (operation: PaneSpawnOperation) => Promise cols: number rows: number cwd?: string @@ -96,6 +99,7 @@ export type RuntimePtyController = { signal?: AbortSignal refuseSleptWorktree?: boolean onPtySpawnCommitted?: () => void + paneSpawnOperation?: PaneSpawnOperation adoptedStablePane?: { result: PtySpawnResult owner: { diff --git a/src/main/runtime/runtime-terminal-pane-identity.ts b/src/main/runtime/runtime-terminal-pane-identity.ts index 225ac564262..4833c485e00 100644 --- a/src/main/runtime/runtime-terminal-pane-identity.ts +++ b/src/main/runtime/runtime-terminal-pane-identity.ts @@ -1,3 +1,4 @@ +import type { RuntimePtyController } from './runtime-pty-controller-contract' import { AgentLaunchPaneAlreadyLiveError } from '../../shared/agent-launch-pane-already-live' import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../shared/stable-pane-id' import { isValidHostTerminalTabId } from '../../shared/terminal-tab-id' @@ -37,3 +38,19 @@ export function admitStablePaneAdoption( } return Boolean(result.stablePaneOwner) } + +export function materializedStablePaneReply( + adoption: NonNullable>>>, + fallbackHandle: string +) { + const { owner, result } = adoption + return { + ...result, + wslDistro: result.wslDistro ?? undefined, + stablePaneOwner: { + handle: owner.handle ?? fallbackHandle, + tabId: owner.tabId, + leafId: owner.leafId + } + } +} diff --git a/src/main/runtime/runtime-terminal-state-records.ts b/src/main/runtime/runtime-terminal-state-records.ts index ec81bed136c..e0e989d0527 100644 --- a/src/main/runtime/runtime-terminal-state-records.ts +++ b/src/main/runtime/runtime-terminal-state-records.ts @@ -33,6 +33,11 @@ type RuntimeTerminalTailState = { tailWaitState?: TerminalTailWaitState } +export type PtyRegistrationClaim = { + incarnationId: PtyIncarnationId | null + exitedIncarnations: Set +} + export type RuntimeLeafRecord = RuntimeSyncedLeaf & RuntimeTerminalTailState & { ptyGeneration: number diff --git a/src/main/runtime/terminal/launch/terminal-create-reveal.ts b/src/main/runtime/terminal/launch/terminal-create-reveal.ts new file mode 100644 index 00000000000..825e62dbb99 --- /dev/null +++ b/src/main/runtime/terminal/launch/terminal-create-reveal.ts @@ -0,0 +1,69 @@ +import * as dependencies from '../../orca-runtime-create-terminal-dependencies' +import type { RuntimeNotifier } from '../../runtime-notifier-contract' +import type { TerminalWorkspaceLaunchScope } from '../../runtime-legacy-worker-terminal-recovery-types' +import type { RuntimeTerminalPresentation } from '../../../../shared/runtime-types' + +export function revealCreatedTerminal(args: { + notifier: RuntimeNotifier | null + workspace: Pick + ptyId: string + handle: string + cwd: string + launchOpts: dependencies.TerminalCreateOptions + effectiveLaunchConfig: dependencies.TerminalCreateOptions['launchConfig'] + launchToken: string | undefined + presentation: RuntimeTerminalPresentation | undefined + opts: dependencies.TerminalCreateOptions + tabId: string + leafId: string +}): + | Pick + | Promise> { + const { + notifier, + workspace, + ptyId, + handle, + cwd, + launchOpts, + effectiveLaunchConfig, + launchToken, + presentation, + opts, + tabId, + leafId + } = args + const reveal = notifier?.revealTerminalSession?.bind(notifier) + if (presentation !== 'background' && reveal) { + return (async () => { + try { + await reveal(workspace.id, { + ptyId, + title: launchOpts.title ?? null, + ...(cwd !== workspace.path ? { cwd } : {}), + ...(effectiveLaunchConfig ? { launchConfig: effectiveLaunchConfig } : {}), + ...(launchToken ? { launchToken } : {}), + ...(launchOpts.launchAgent ? { launchAgent: launchOpts.launchAgent } : {}), + ...(launchOpts.viewMode ? { viewMode: launchOpts.viewMode } : {}), + activate: presentation === 'focused', + ...(presentation ? { presentation } : {}), + ...dependencies.terminalRevealViewerOptions(opts), + tabId, + leafId + }) + return { surface: 'visible' as const } + } catch (err) { + console.warn(`[terminal-create] failed to create inactive tab for ${ptyId}:`, err) + return { + surface: 'background' as const, + warning: dependencies.createTerminalRevealWarning(handle, err) + } + } + })() + } + return { + surface: 'background', + warning: + presentation !== 'background' ? dependencies.createTerminalRevealWarning(handle) : undefined + } +} diff --git a/src/main/runtime/terminal/launch/terminal-create.ts b/src/main/runtime/terminal/launch/terminal-create.ts index 1921769f99e..16b7df5546a 100644 --- a/src/main/runtime/terminal/launch/terminal-create.ts +++ b/src/main/runtime/terminal/launch/terminal-create.ts @@ -1,10 +1,12 @@ +import { revealCreatedTerminal } from './terminal-create-reveal' +import type { PaneSpawnOperation } from '../../../ipc/pty/pane/pane-spawn-operation' import { OrcaRuntimeWithTerminalCreateDeduplication } from './terminal-create-deduplication' import * as dependencies from '../../orca-runtime-create-terminal-dependencies' import { createDesktopTerminal } from '../../orca-runtime-create-terminal-desktop' import { buildRuntimeAgentTeamsLaunchPlan } from './agent-teams-launch-plan' import { createIdempotentCallback } from '../../orca-runtime-report-pty-spawn-commit' import { recordPtySurface, spawnSurfaceClaimSequence } from '../../pty-recorded-surface-topology' -import { agentLaunchTokenFor } from '../../../agent-launch/agent-launch-token' +import { agentLaunchTokenFor, withAgentLaunchToken } from '../../../agent-launch/agent-launch-token' import { migratedPaneRestoreHint } from '../../migrated-pane-restore-cwd' import { paneSize, resolvePtyFirstSize } from '../../../ipc/pty/pty-first-size' @@ -29,7 +31,8 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi // Why: this process starts every terminal it can place, so a slow window only delays showing // it; a window-made create that timed out here still ran later, and a retry duplicated it. if (worktreeSelector !== undefined) { - if (!this.ptyController?.spawn) { + const { ptyController } = this + if (!ptyController?.spawn) { throw new Error('runtime_unavailable') } const workspace = await this.resolveTerminalWorkspaceLaunchScope(worktreeSelector, created) @@ -57,36 +60,20 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi restoreHint.consumeThen(launchOpts.onPtySpawnCommitted) ) // Claimed before any await, so a concurrent create of this pane cannot slip in between. - const claimedStablePaneCreate = this.ptyController.claimStablePaneCreate?.({ + const claimedStablePaneCreate = ptyController.claimStablePaneCreate?.({ worktreeId: workspace.id, connectionId: workspace.connectionId, tabId, leafId }) const releaseStablePaneCreate = createIdempotentCallback(claimedStablePaneCreate) - try { - // A pane moved from another host reopens where it last was, until a spawn commits. - cwd = (await restoreHint.peekCwd(launchOpts.cwd)) ?? cwd - if (launchOpts.signal?.aborted) { - throw new Error('client_disconnected') - } - const adoptedBeforeLaunch = await this.ptyController.adoptStablePane?.({ - ...initialSize, - cwd, - connectionId: workspace.connectionId, - worktreeId: workspace.id, - preAllocatedHandle, - tabId, - leafId - }) - const launchToken = agentLaunchTokenFor(launchOpts, dependencies.randomUUID) - const baseEnv = { - ...launchOpts.env, - ...(launchToken ? { ORCA_AGENT_LAUNCH_TOKEN: launchToken } : {}) - } + const launchToken = agentLaunchTokenFor(launchOpts, dependencies.randomUUID) + let effectiveLaunchConfig = launchOpts.launchConfig + let result!: Awaited>> + const spawnFresh = async (operation: PaneSpawnOperation) => { + const baseEnv = withAgentLaunchToken(launchOpts.env, launchToken) let agentTeamsPlan: Awaited>['plan'] let sequencedStartupCommand: string | undefined - let effectiveLaunchConfig = launchOpts.launchConfig try { const agentTeams = await buildRuntimeAgentTeamsLaunchPlan({ launchConfig: launchOpts.launchConfig, @@ -94,7 +81,7 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi claudeAgentTeamsSourceCommand: launchOpts.claudeAgentTeamsSourceCommand, claudeAgentTeamsMode: this.store?.getSettings?.().claudeAgentTeamsMode, baseEnv: { ...process.env, ...baseEnv }, - adoptedBeforeLaunch, + adoptedBeforeLaunch: null, createTeamEnv: (shimDir, shimBin) => this.claudeAgentTeams.createLaunchEnv({ leaderHandle: preAllocatedHandle, @@ -126,14 +113,12 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi if (launchOpts.signal?.aborted) { throw new Error('client_disconnected') } - let result: Awaited>> try { - const { launchAgent } = launchOpts launchOpts.onPtySpawnDispatched?.({ launchConfig: effectiveLaunchConfig, - launchAgent + launchAgent: launchOpts.launchAgent }) - result = await dependencies.spawnInAdmittedPane(this.store, this.ptyController, { + result = await dependencies.spawnInAdmittedPane(this.store, ptyController, { ...initialSize, cwd, command: sequencedStartupCommand @@ -179,17 +164,41 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi ...(launchOpts.onPtySpawnCommitted ? { onPtySpawnCommitted: reportPtySpawnCommitted } : {}), - ...(adoptedBeforeLaunch ? { adoptedStablePane: adoptedBeforeLaunch } : {}), + paneSpawnOperation: operation, ...dependencies.newTabIn(launchOpts), ...(launchOpts.afterTabId ? { afterTabId: launchOpts.afterTabId } : {}), ...(launchOpts.sessionId ? { sessionId: launchOpts.sessionId } : {}), - ...(!adoptedBeforeLaunch && launchOpts.isNewSession ? { isNewSession: true } : {}), + ...(launchOpts.isNewSession ? { isNewSession: true } : {}), ...dependencies.BACKGROUND_TERMINAL_SPAWN_FLAGS, ...this.paneStartStatusArgs(launchOpts) }) } finally { releaseStablePaneCreate?.() } + return result + } + try { + // A pane moved from another host reopens where it last was, until a spawn commits. + cwd = (await restoreHint.peekCwd(launchOpts.cwd)) ?? cwd + if (launchOpts.signal?.aborted) { + throw new Error('client_disconnected') + } + const adoptedBeforeLaunch = await ptyController.adoptStablePane?.({ + ...initialSize, + cwd, + connectionId: workspace.connectionId, + worktreeId: workspace.id, + preAllocatedHandle, + tabId, + leafId, + spawnFresh, + signal: launchOpts.signal + }) + if (adoptedBeforeLaunch) { + result = dependencies.materializedStablePaneReply(adoptedBeforeLaunch, preAllocatedHandle) + } else if (!ptyController.adoptStablePane) { + result = await spawnFresh({ mustExist: false }) + } if (!result.stablePaneOwner) { reportPtySpawnCommitted() } @@ -216,16 +225,18 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi } throw error } - this.registerPreAllocatedHandleForPty(result.id, preAllocatedHandle) - if (result.wslDistro) { - this.preparePtyExecutionContext(result.id, result.wslDistro) + if (!adoptedStablePane) { + this.registerPreAllocatedHandleForPty(result.id, preAllocatedHandle) + if (result.wslDistro) { + this.preparePtyExecutionContext(result.id, result.wslDistro) + } + this.registerPty(result.id, workspace.id, workspace.connectionId, { + tabId, + leafId, + terminalHandle: preAllocatedHandle, + ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}) + }) } - this.registerPty(result.id, workspace.id, workspace.connectionId, { - tabId, - leafId, - terminalHandle: preAllocatedHandle, - ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}) - }) const pty = this.getOrCreatePtyWorktreeRecord(result.id) if (pty) { pty.runtimeSessionOwned = true @@ -260,32 +271,21 @@ export abstract class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTermi ...(cwd !== workspace.path ? { startupCwd: cwd } : {}) }) } - let surface: dependencies.RuntimeTerminalCreate['surface'] = 'background' - let warning: string | undefined - if (presentation !== 'background' && this.notifier?.revealTerminalSession) { - try { - await this.notifier.revealTerminalSession(workspace.id, { - ptyId: result.id, - title: launchOpts.title ?? null, - ...(cwd !== workspace.path ? { cwd } : {}), - ...(effectiveLaunchConfig ? { launchConfig: effectiveLaunchConfig } : {}), - ...(launchToken ? { launchToken } : {}), - ...(launchOpts.launchAgent ? { launchAgent: launchOpts.launchAgent } : {}), - ...(launchOpts.viewMode ? { viewMode: launchOpts.viewMode } : {}), - activate: presentation === 'focused', - ...(presentation ? { presentation } : {}), - ...dependencies.terminalRevealViewerOptions(opts), - tabId, - leafId - }) - surface = 'visible' - } catch (err) { - console.warn(`[terminal-create] failed to create inactive tab for ${result.id}:`, err) - warning = dependencies.createTerminalRevealWarning(handle, err) - } - } else if (presentation !== 'background') { - warning = dependencies.createTerminalRevealWarning(handle) - } + const reveal = revealCreatedTerminal({ + notifier: this.notifier, + workspace, + ptyId: result.id, + handle, + cwd, + launchOpts, + effectiveLaunchConfig, + launchToken, + presentation, + opts, + tabId, + leafId + }) + const { surface, warning } = reveal instanceof Promise ? await reveal : reveal return { handle, tabId, diff --git a/src/main/runtime/terminal/launch/terminal-execution-context.ts b/src/main/runtime/terminal/launch/terminal-execution-context.ts index a6dd2a9223f..28a4f5a61b2 100644 --- a/src/main/runtime/terminal/launch/terminal-execution-context.ts +++ b/src/main/runtime/terminal/launch/terminal-execution-context.ts @@ -20,7 +20,7 @@ export abstract class OrcaRuntimeWithPreparePtyExecutionContext extends OrcaRunt if (options.resetIncarnation) { // Why: an explicit new lifecycle supersedes an unidentifiable exit from the reused PTY id. - this.earlyExitedPtyIncarnations.delete(ptyId) + this.pendingPtyRegistrationIncarnations.get(ptyId)?.exitedIncarnations.clear() this.disposeHeadlessTerminal(ptyId) this.osc7ScanTailByPtyId.delete(ptyId) this.terminalCwdByPtyId.delete(ptyId) diff --git a/src/main/runtime/terminal/lifecycle/pty-exit-lifecycle.ts b/src/main/runtime/terminal/lifecycle/pty-exit-lifecycle.ts index 5db5b732335..546920d5cd4 100644 --- a/src/main/runtime/terminal/lifecycle/pty-exit-lifecycle.ts +++ b/src/main/runtime/terminal/lifecycle/pty-exit-lifecycle.ts @@ -53,9 +53,12 @@ export abstract class OrcaRuntimeWithOnPtyExit extends OrcaRuntimeWithOnClientDi } = {} ): void | Promise { const pty = this.ptysById.get(ptyId) - if (exitIncarnationId && pty?.incarnationId && exitIncarnationId !== pty.incarnationId) { + const pending = this.pendingPtyRegistrationIncarnations.get(ptyId) + const candidateIncarnation = pending ? pending.incarnationId : pty?.incarnationId + if (exitIncarnationId && candidateIncarnation && exitIncarnationId !== candidateIncarnation) { return } + pending?.exitedIncarnations.add(exitIncarnationId ?? candidateIncarnation ?? null) this.invalidatePtyControllerInventoryForLifecycle(ptyId, pty?.connectionId) // A bare exit code is not enough to establish why a process ended: older // daemons and SSH relays can report 0 for crashes and wrapper exits. @@ -139,20 +142,6 @@ export abstract class OrcaRuntimeWithOnPtyExit extends OrcaRuntimeWithOnClientDi }) } const exactSurfaces = [...exactSurfaceByKey.values()] - const pendingIncarnation = this.pendingPtyRegistrationIncarnations.get(ptyId) - const exitMatchesPendingRegistration = - this.pendingPtyRegistrationIncarnations.has(ptyId) && - (pendingIncarnation === null || - exitIncarnationId === null || - exitIncarnationId === undefined || - pendingIncarnation === exitIncarnationId) - if (exitMatchesPendingRegistration) { - // Why: reused surfaces can look registered while their replacement incarnation still awaits admission. - this.earlyExitedPtyIncarnations.set( - ptyId, - exitIncarnationId ?? pendingIncarnation ?? pty?.incarnationId ?? null - ) - } // Why both kinds: a sleep keeps its wake hint, and a restart's replacement takes the pane. const preservesIntentionallyStoppedSurface = this.intentionalPtyStops.claimExit(ptyId, exitIncarnationId ?? pty?.incarnationId).length > diff --git a/src/main/runtime/terminal/lifecycle/pty-handle-invalidation.ts b/src/main/runtime/terminal/lifecycle/pty-handle-invalidation.ts index 04d2f457c8c..1739f31f815 100644 --- a/src/main/runtime/terminal/lifecycle/pty-handle-invalidation.ts +++ b/src/main/runtime/terminal/lifecycle/pty-handle-invalidation.ts @@ -163,7 +163,15 @@ export abstract class OrcaRuntimeWithInvalidateAllHandlesForPty extends OrcaRunt this.stopRequestedPtyIds.delete(ptyId) if (options.awaitsRegistration !== false) { // Why: surface absence cannot distinguish an in-flight admission from a completed headless lifecycle. - this.pendingPtyRegistrationIncarnations.set(ptyId, incarnationId ?? null) + const pending = this.pendingPtyRegistrationIncarnations.get(ptyId) + if (pending && (pending.incarnationId === null || pending.incarnationId === incarnationId)) { + pending.incarnationId = incarnationId ?? null + } else { + this.pendingPtyRegistrationIncarnations.set(ptyId, { + incarnationId: incarnationId ?? null, + exitedIncarnations: new Set() + }) + } } this.terminalViewSubscribers.markSpawnPublished(ptyId) const pty = this.getOrCreatePtyWorktreeRecord(ptyId) diff --git a/src/main/runtime/terminal/lifecycle/pty-registration.ts b/src/main/runtime/terminal/lifecycle/pty-registration.ts index c1473862304..6ad704b4b4d 100644 --- a/src/main/runtime/terminal/lifecycle/pty-registration.ts +++ b/src/main/runtime/terminal/lifecycle/pty-registration.ts @@ -5,7 +5,10 @@ import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id' import { isTerminalLeafId, makePaneKey } from '../../../../shared/stable-pane-id' import { isTuiAgent } from '../../../../shared/tui-agent-config' import { spawnSurfaceClaimSequence } from '../../pty-recorded-surface-topology' -import type { RuntimePtyWorktreeRecord } from '../../runtime-terminal-state-records' +import type { + PtyRegistrationClaim, + RuntimePtyWorktreeRecord +} from '../../runtime-terminal-state-records' import type { OrchestrationCompatibilityTerminalAuthority } from '../../runtime-terminal-contracts' import type { RuntimeMobileSessionCreateTerminalResult } from '../../../../shared/runtime-session-contracts' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' @@ -147,7 +150,7 @@ export abstract class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalida // Why: daemon metadata owns the surviving process; its incarnation fence restores identity without minting renderer launch authority. pty.launchAgent = providerReattachLaunchIdentity.launchAgent } - const pendingIncarnation = this.pendingPtyRegistrationIncarnations.get(ptyId) + const pendingIncarnation = this.pendingPtyRegistrationIncarnations.get(ptyId)?.incarnationId if ( pendingIncarnation === null || pendingIncarnation === undefined || @@ -190,23 +193,31 @@ export abstract class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalida ptyId: string, candidateIncarnation?: PtyIncarnationId ): void { - if (!this.earlyExitedPtyIncarnations.has(ptyId)) { - return - } - const exitedIncarnation = this.earlyExitedPtyIncarnations.get(ptyId) ?? null - if ( - exitedIncarnation === null || - candidateIncarnation === undefined || - exitedIncarnation === candidateIncarnation - ) { - // Why: the rejected spawn call was the fence's sole late publisher; retaining it leaks fresh PTY ids. - this.earlyExitedPtyIncarnations.delete(ptyId) - this.pendingPtyRegistrationIncarnations.delete(ptyId) + const pending = this.pendingPtyRegistrationIncarnations.get(ptyId) + if (pending?.exitedIncarnations.size) { + this.cancelPendingPtyRegistration(ptyId, candidateIncarnation, pending) } } - beginPtyRegistration(ptyId: string, incarnationId?: PtyIncarnationId): void { - this.pendingPtyRegistrationIncarnations.set(ptyId, incarnationId ?? null) + beginPtyRegistration(ptyId: string, incarnationId?: PtyIncarnationId) { + const claim: PtyRegistrationClaim = { + incarnationId: incarnationId ?? null, + exitedIncarnations: new Set() + } + this.pendingPtyRegistrationIncarnations.set(ptyId, claim) + return claim + } + + narrowPtyRegistration( + ptyId: string, + claim: PtyRegistrationClaim, + incarnationId?: PtyIncarnationId + ): boolean { + if (this.pendingPtyRegistrationIncarnations.get(ptyId) !== claim) { + return false + } + claim.incarnationId = incarnationId ?? null + return true } acceptPtyIncarnationForExit(ptyId: string, incarnationId: PtyIncarnationId): void { @@ -217,41 +228,34 @@ export abstract class OrcaRuntimeWithRegisterPty extends OrcaRuntimeWithInvalida } } - cancelPendingPtyRegistration(ptyId: string, incarnationId?: PtyIncarnationId): void { + cancelPendingPtyRegistration( + ptyId: string, + incarnationId?: PtyIncarnationId, + claim?: PtyRegistrationClaim + ): void { const pending = this.pendingPtyRegistrationIncarnations.get(ptyId) if ( - !this.pendingPtyRegistrationIncarnations.has(ptyId) || - (pending !== null && incarnationId !== undefined && pending !== incarnationId) + !pending || + (claim !== undefined && pending !== claim) || + (pending.incarnationId !== null && + incarnationId !== undefined && + pending.incarnationId !== incarnationId) ) { return } this.pendingPtyRegistrationIncarnations.delete(ptyId) - const exited = this.earlyExitedPtyIncarnations.get(ptyId) - if ( - exited === null || - exited === undefined || - incarnationId === undefined || - exited === incarnationId - ) { - this.earlyExitedPtyIncarnations.delete(ptyId) - } } protected assertPtyDidNotExitBeforeRegistration( ptyId: string, candidateIncarnation?: PtyIncarnationId ): void { - if (this.earlyExitedPtyIncarnations.has(ptyId)) { - const exitedIncarnation = this.earlyExitedPtyIncarnations.get(ptyId) ?? null - const nextIncarnation = candidateIncarnation ?? null - if ( - exitedIncarnation === null || - nextIncarnation === null || - exitedIncarnation === nextIncarnation - ) { - throw new Error('agent_session_exited_during_start') - } - this.earlyExitedPtyIncarnations.delete(ptyId) + const exits = this.pendingPtyRegistrationIncarnations.get(ptyId)?.exitedIncarnations + if ( + exits?.size && + (candidateIncarnation === undefined || exits.has(null) || exits.has(candidateIncarnation)) + ) { + throw new Error('agent_session_exited_during_start') } } } diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index 5d0a5f313b5..24eb75794a8 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -34,6 +34,8 @@ const STALE_DAEMON_CWD_MARKERS = [ ] // Thrown by ipc/pty.ts when a persisted pane owner can't be proven alive or dead (STA-3536). const PANE_OWNER_UNVERIFIED_MARKER = 'terminal_pane_owner_unverified' +// Main lost the pane to another connection twice in one attach, or an older host never retries. +const PANE_OWNER_CHANGED_MARKER = 'terminal_pane_owner_changed' // remote-runtime-pty-transport.ts surfaces this English literal as a wire-level marker, so it is // translated here rather than at the source -- otherwise the banner mixes English with the // localized chrome around it (#9194). @@ -108,6 +110,18 @@ export function isPaneOwnerUnverifiedError(error: string): boolean { return lines.length > 0 && lines.every((line) => line.includes(PANE_OWNER_UNVERIFIED_MARKER)) } +/** Owner errors a reattach can recover from: the saved session is kept, and Retry asks again. */ +export function isPaneOwnerRecoveryError(error: string): boolean { + const lines = error.split('\n').filter((line) => line.length > 0) + return ( + lines.length > 0 && + lines.every( + (line) => + line.includes(PANE_OWNER_UNVERIFIED_MARKER) || line.includes(PANE_OWNER_CHANGED_MARKER) + ) + ) +} + function humanizeUnreattachableSession(error: string): string { const explanation = translate( 'auto.components.terminal.pane.TerminalErrorToast.sessionUnavailable', @@ -156,6 +170,12 @@ export function humanizeTerminalError(error: string): string { ) humanized = humanized.replaceAll(PANE_OWNER_UNVERIFIED_MARKER, () => explanation) } + humanized = humanized.replaceAll(PANE_OWNER_CHANGED_MARKER, () => + translate( + 'auto.components.terminal.pane.TerminalErrorToast.ownerUnknown', + "Orca couldn't verify this terminal's owner." + ) + ) humanized = humanized.replace(SOURCE_RESTORE_REQUIRED.replaceAll, () => translate( 'auto.components.terminal.pane.TerminalErrorToast.sourceRestoring', @@ -218,7 +238,7 @@ export function TerminalErrorToast({ const ssh = isSshHostStateTerminalError(error) // Why: the client's OS and shell describe neither the host nor its shell, and the renderer knows neither. const showClientEnvironment = paneOnClient && !ssh - const paneOwnerUnverified = isPaneOwnerUnverifiedError(error) + const paneOwnerUnverified = isPaneOwnerRecoveryError(error) const showDaemonRestart = !ssh && onRestartDaemon && shouldOfferDaemonRestart(error) // Restart cannot recover a session after its owning daemon exits. const showIssueLink = diff --git a/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx b/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx index fb51aa04631..52348d8e3c8 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx @@ -179,7 +179,7 @@ export function TerminalPaneRecoveryPortals({ <> {managedPanes.map((pane) => { const recoveryState = ptyRecoveryStatesByPaneId[pane.id] - if (!recoveryState) { + if (!recoveryState || recoveryState.reason === 'ownership') { return null } return createPortal( diff --git a/src/renderer/src/components/terminal-pane/TerminalPaneSurface.tsx b/src/renderer/src/components/terminal-pane/TerminalPaneSurface.tsx index c4d00c4b741..603b8bdd6b5 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPaneSurface.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPaneSurface.tsx @@ -8,10 +8,9 @@ import { WORKSPACE_FILE_PATH_MIME, WORKSPACE_FILE_PATHS_MIME } from '@/lib/works import CloseTerminalDialog from './CloseTerminalDialog' import TerminalContextMenu from './TerminalContextMenu' import TerminalPaneHeaderOverlay from './TerminalPaneHeaderOverlay' -import { isPaneOwnerUnverifiedError, TerminalErrorToast } from './TerminalErrorToast' +import { isPaneOwnerRecoveryError, TerminalErrorToast } from './TerminalErrorToast' import { openTerminalBesideTab } from './open-terminal-beside-tab' import { AgentLaunchPaneNoticePortal } from './AgentLaunchPaneNotice' -import { requestTerminalPaneRecovery } from './terminal-pane-recovery' import { TerminalSessionStateSaveFailureDialog } from './TerminalSessionStateSaveFailureDialog' import { LinkActionPopover } from '@/components/link-actions/LinkActionPopover' import { TerminalAgentSessionForkDialog } from './TerminalAgentSessionForkDialog' @@ -203,25 +202,11 @@ export function TerminalPaneSurface({ onDismiss={dismissTerminalError} onRestartDaemon={() => daemonActions.setPending('restart')} onRetry={ - isPaneOwnerUnverifiedError(visibleTerminalError) - ? () => { - const ptyId = activePane - ? (paneTransportsRef.current.get(activePane.id)?.getPtyId() ?? null) - : null - return requestTerminalPaneRecovery({ - tabId, - ptyId, - reason: 'reattach-unverifiable', - // The user asking again is the new trigger that reopens - // a reason an observed failure has closed. - trigger: 'user' - }).then((recovered) => { - if (recovered) { - dismissTerminalError() - } - return recovered - }) - } + isPaneOwnerRecoveryError(visibleTerminalError) + ? () => + Promise.resolve( + paneTransportsRef.current.get(activePane.id)?.retryRecovery?.() ?? false + ) : undefined } onOpenNewTerminal={() => { diff --git a/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts b/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts index a5d1aa39164..12182fc2aeb 100644 --- a/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts +++ b/src/renderer/src/components/terminal-pane/pending-split-close-test-fixture.ts @@ -12,6 +12,8 @@ import type { PtyConnectResult, PtyTransport } from './pty-transport-types' import type { TerminalPaneBindingController } from './use-terminal-pane-layout-bindings' import { installIpcPtyWindow, restorePtySpecWindow } from './pty-transport-test-harness' +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) + const store = vi.hoisted((): { current: AppState | null } => ({ current: null })) vi.mock('../../store', () => ({ useAppStore: { getState: () => store.current } })) vi.mock('react', () => ({ @@ -20,7 +22,6 @@ vi.mock('react', () => ({ useImperativeHandle: () => {}, useRef: (current: unknown) => ({ current }) })) -vi.mock('../../runtime/web-runtime-terminal-actions', () => ({ closeWebRuntimeTerminal: vi.fn() })) vi.mock('../../runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync: vi.fn() })) vi.mock('../terminal/terminal-close-copy-kind', () => ({ resolveLeafCloseCopyKind: vi.fn() })) vi.mock('../terminal/running-terminal-close-guard', () => ({ RUNNING_CLOSE_PROBE_TIMEOUT_MS: 100 })) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-session-liveness.spec.ts b/src/renderer/src/components/terminal-pane/pty-connection-session-liveness.spec.ts index 82571cc00c7..c65ebffb5c9 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-session-liveness.spec.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-session-liveness.spec.ts @@ -469,6 +469,55 @@ export function registerPtyConnectionSessionLivenessTests(): void { expect(window.api.pty.clearPendingPaneSerializer).toHaveBeenCalledWith(expect.any(String), 1) }) + it.each([false, true])( + 'preserves an exhausted pane until explicit Retry and clears only the current owner error (fresh: %s)', + async (fresh) => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async (opts: { callbacks?: ConnectCallbacks }) => { + opts.callbacks?.onError?.('terminal_pane_owner_changed') + return undefined + }) + transportFactoryQueue.push(transport) + const deps = fresh ? createDeps() : seedUnverifiableRestoredPane() + const cleared = vi.fn() + deps.onPtyErrorClearedRef = { current: cleared } + + connectPanePty(createPane(2) as never, createManager(2) as never, deps as never) + await flushAsyncTicks() + + expect(transport.connect).toHaveBeenCalledTimes(1) + expect(deps.onPtyErrorRef.current).toHaveBeenCalledExactlyOnceWith( + 2, + 'terminal_pane_owner_changed' + ) + expect(deps.clearExitedPanePtyLayoutBinding).not.toHaveBeenCalled() + expect(deps.clearTabPtyId).not.toHaveBeenCalled() + if (!fresh) { + expect(mockStoreState.terminalLayoutsByTabId?.['tab-1']?.ptyIdsByLeafId).toEqual({ + [LEAF_2]: 'unverifiable-pty' + }) + } + const stale = transport.connect.mock.calls[0]?.[0].callbacks + transport.connect.mockImplementation(async ({ callbacks }) => { + callbacks?.onError?.('unrelated diagnostic') + callbacks?.onConnect?.() + return { id: 'unverifiable-pty', isReattach: true } + }) + expect(transport.retryRecovery?.()).toBe(true) + await flushAsyncTicks() + expect(transport.connect).toHaveBeenCalledTimes(2) + expect(transport.disconnect).not.toHaveBeenCalled() + expect(cleared).toHaveBeenCalledExactlyOnceWith(2, 'terminal_pane_owner_changed') + stale?.onError?.('terminal_pane_owner_changed') + expect( + deps.onPtyErrorRef.current.mock.calls.filter( + ([, message]) => message === 'terminal_pane_owner_changed' + ) + ).toHaveLength(1) + } + ) + describe('terminal input liveness IPC gating (perf)', () => { // Why (perf regression guard): listSessions() is a renderer→main→daemon round-trip; terminal input must never trigger it. async function connectActivePaneWithInput(): Promise<{ diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts index 1e868695d03..aa077b43822 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-pane-fixtures.ts @@ -26,6 +26,7 @@ export type ConnectCallbacks = { } export type MockTransport = { + retryRecovery?: () => boolean attach: ReturnType connect: ReturnType & { mockImplementation: ( diff --git a/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-connect.ts b/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-connect.ts index c16085dba04..6414ccb1031 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-connect.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-connect.ts @@ -1,3 +1,4 @@ +import { armPaneOwnerRetry } from './pane-owner-recovery' import { isRemoteRuntimePtyId } from '../../../../../shared/remote-runtime-pty-id' import { warnTerminalLifecycleAnomaly } from '../terminal-lifecycle-diagnostics' import { isSshSessionGoneError, recordPtyConnectDiagnostic } from './pty-connect-limits' @@ -15,6 +16,7 @@ import { } from '@/hooks/ipc-events/agent-status-startup-snapshot' const PANE_OWNER_UNVERIFIED_ERROR = 'terminal_pane_owner_unverified' +const PANE_OWNER_CHANGED_ERROR = 'terminal_pane_owner_changed' export function startDeferredSessionReattach( session: ConnectPanePtySession, @@ -73,6 +75,7 @@ function connectDeferredSessionReattach( let expiredReattachError = false let paneOwnerUnverified = false + let paneOwnerChanged = false let heldByNewerDaemon = false const coldRestoreStartup = session.buildColdRestoreAgentResumeStartup() const outputCallbacks = session.captureTransportOutputCallbacks( @@ -89,6 +92,9 @@ function connectDeferredSessionReattach( if (message.includes(PANE_OWNER_UNVERIFIED_ERROR)) { paneOwnerUnverified = true } + if (message.includes(PANE_OWNER_CHANGED_ERROR)) { + paneOwnerChanged = true + } session.reportError(message) }, toProcessExitStartup(coldRestoreStartup ?? session.paneStartup) @@ -156,7 +162,22 @@ function connectDeferredSessionReattach( } return NOT_REOPENED } + if (!result && paneOwnerChanged) { + session.finishReattachLiveDataDeferral(false, outputCallbacks.generation) + const gen = await preSignalPromise + if (typeof gen === 'number') { + void window.api.pty.clearPendingPaneSerializer(session.cacheKey, gen).catch(() => {}) + } + armPaneOwnerRetry(session, outputCallbacks.generation, () => + connectDeferredSessionReattach(session, deferredReattachSessionId) + ) + session.settlePaneAttachAttempt('failed') + return NOT_REOPENED + } if (!result && paneOwnerUnverified) { + armPaneOwnerRetry(session, outputCallbacks.generation, () => + connectDeferredSessionReattach(session, deferredReattachSessionId) + ) session.finishReattachLiveDataDeferral(false, outputCallbacks.generation) const gen = await preSignalPromise if (typeof gen === 'number') { @@ -221,8 +242,14 @@ function connectDeferredSessionReattach( if (!isCurrentReattach()) { return NOT_REOPENED } - if (message.includes(PANE_OWNER_UNVERIFIED_ERROR)) { - session.reportError(message) + if ( + message.includes(PANE_OWNER_UNVERIFIED_ERROR) || + message.includes(PANE_OWNER_CHANGED_ERROR) + ) { + outputCallbacks.callbacks.onError(message) + armPaneOwnerRetry(session, outputCallbacks.generation, () => + connectDeferredSessionReattach(session, deferredReattachSessionId) + ) session.settlePaneAttachAttempt('failed') return NOT_REOPENED } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts b/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts index db1a271d351..d22eb56b6ea 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.ts @@ -1,3 +1,4 @@ +import { armPaneOwnerRetry, isPaneOwnerRefusal } from './pane-owner-recovery' import type { SpawnDeferralReason } from '../../../../../shared/host-spawn-admission' import { isRemoteRuntimePtyId } from '../../../../../shared/remote-runtime-pty-id' import { useAppStore } from '@/store' @@ -91,8 +92,12 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void { session.pendingSpawnDeferral = null const visibleAtSpawn = session.deps.isVisibleRef.current === true const effectiveStartup = startupOverride === undefined ? session.paneStartup : startupOverride + let ownerRefused = false const outputCallbacks = session.captureTransportOutputCallbacks( - session.reportError, + (message: string) => { + ownerRefused ||= isPaneOwnerRefusal(message) + session.reportError(message) + }, toProcessExitStartup(coldRestoreOverride ?? effectiveStartup) ) const spawnedRaw = session.transport.connect({ @@ -255,7 +260,11 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void { session.finishReattachLiveDataDeferral(Boolean(resolvedPtyId), outputCallbacks.generation) return resolvedPtyId }) - .catch(async () => { + .catch(async (error: unknown) => { + const message = error instanceof Error ? error.message : String(error) + if (!ownerRefused && isPaneOwnerRefusal(message)) { + outputCallbacks.callbacks.onError(message) + } releaseDeferredCwdFence() session.finishReattachLiveDataDeferral(false, outputCallbacks.generation) if ( @@ -288,11 +297,19 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void { queueMicrotask(() => { if ( session.disposed || + outputCallbacks.generation !== session.transportStreamGeneration || session.transport.getPtyId() || pendingSpawnByPaneKey.has(session.pendingSpawnKey) ) { return } + if (ownerRefused) { + armPaneOwnerRetry(session, outputCallbacks.generation, () => + session.startFreshSpawn(startupOverride, options) + ) + session.settlePaneAttachAttempt?.('failed') + return + } if (session.deferredSpawn) { // Why no remount: the host would defer the remount's restore again, in a loop. session.settlePaneAttachAttempt?.('failed') diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-owner-recovery.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-owner-recovery.ts new file mode 100644 index 00000000000..b123d5439a1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-owner-recovery.ts @@ -0,0 +1,29 @@ +import type { ConnectPanePtySession } from './connect-pane-pty-session' + +export function isPaneOwnerRefusal(message: string): boolean { + return ( + message.includes('terminal_pane_owner_changed') || + message.includes('terminal_pane_owner_unverified') + ) +} + +export function armPaneOwnerRetry( + session: ConnectPanePtySession, + generation: number, + retry: () => unknown +): void { + if (session.runtimeEnvironmentId) { + return + } + session.transport.retryRecovery = () => { + if ( + session.disposed || + generation !== session.transportStreamGeneration || + session.deps.paneTransportsRef.current.get(session.pane.id) !== session.transport + ) { + return false + } + void retry() + return true + } +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts b/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts index a217b2b66c4..2a121bdbc07 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/transport-output-callbacks.ts @@ -1,3 +1,4 @@ +import { isPaneOwnerRefusal } from './pane-owner-recovery' import type { SpawnDeferralReason } from '../../../../../shared/host-spawn-admission' import { createTerminalStartupTiming } from '../terminal-startup-timing' import type { PtyReplayDataMeta, PtyTransportRecoveryState } from '../pty-transport-types' @@ -8,6 +9,7 @@ import type { ConnectPanePtySession } from './connect-pane-pty-session' /** Per-generation transport output callbacks and the hidden-output restore state they reset. */ export function bindCaptureTransportOutputCallbacks(session: ConnectPanePtySession): void { + const ownerErrors = new Set() session.captureTransportOutputCallbacks = ( onError: (message: string) => void, startup: PtyPaneStartup @@ -47,6 +49,10 @@ export function bindCaptureTransportOutputCallbacks(session: ConnectPanePtySessi }, onConnect: (): void => { if (isCurrent()) { + for (const message of ownerErrors) { + session.deps.onPtyErrorClearedRef?.current?.(session.pane.id, message) + } + ownerErrors.clear() session.startupTiming?.mark('connected') session.reportRemoteRendererSerializerReady() // Re-derive the pause bit after a rebind; visibility can change while no PTY is bound. @@ -74,6 +80,11 @@ export function bindCaptureTransportOutputCallbacks(session: ConnectPanePtySessi }, onError: (message: string): void => { if (isCurrent()) { + for (const line of message.split('\n')) { + if (isPaneOwnerRefusal(line)) { + ownerErrors.add(line) + } + } session.startupTiming?.finish('error') onError(message) } 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 a58a0465e30..9a144cdba80 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -154,6 +154,7 @@ type PtyCallbacks = { } export type PtyTransportRecoveryState = { + reason?: 'ownership' phase: | 'connecting' | 'connected' diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-limits.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-limits.ts new file mode 100644 index 00000000000..2b9b57ed098 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-limits.ts @@ -0,0 +1,14 @@ +export const REMOTE_RUNTIME_RECOVERY_DELAYS_MS = [ + 250, 500, 1000, 2000, 4000, 8000, 15_000, 30_000 +] as const + +// Why: mirrors DEFAULT_REMOTE_RUNTIME_TIMEOUT_MS in the main-process runtime router; a silently +// dropped link burns the whole RPC timeout on the attempt each backoff step leads into. +export const REMOTE_RUNTIME_RECOVERY_ATTEMPT_BUDGET_MS = 15_000 + +// Why derived, not hand-tuned: a literal deadline drifted below the ladder it arms, making the last +// backoff steps unreachable dead code (#11305). Loss of contact is never evidence of exit, so the +// window must outlast the schedule it advertises rather than the schedule being trimmed to fit. +export const REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS = + REMOTE_RUNTIME_RECOVERY_DELAYS_MS.reduce((total, delayMs) => total + delayMs, 0) + + REMOTE_RUNTIME_RECOVERY_DELAYS_MS.length * REMOTE_RUNTIME_RECOVERY_ATTEMPT_BUDGET_MS diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.test.ts index b9aa0b6c425..1c7c0ab8c43 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.test.ts @@ -160,22 +160,45 @@ describe('RemoteRuntimePtyRecoveryState', () => { state.dispose() }) - it.each([ - ['healthy', (state: RemoteRuntimePtyRecoveryState) => state.markHealthy()], - ['cancelled', (state: RemoteRuntimePtyRecoveryState) => state.cancel()], - ['disposed', (state: RemoteRuntimePtyRecoveryState) => state.dispose()] - ])('removes %s panes from the scheduled recovery registry', (_label, finish) => { - vi.useFakeTimers() - const state = new RemoteRuntimePtyRecoveryState() - const epoch = state.begin() - state.schedule(epoch, vi.fn()) - const retryNow = vi.spyOn(state, 'retryNow') - - finish(state) - retryAllRemoteRuntimePtyRecoveriesNow() - - expect(retryNow).not.toHaveBeenCalled() - }) + it.each(['healthy', 'cancelled', 'disposed', 'deadline', 'exhausted', 'contact loss'] as const)( + 'settles the owner caller once and retains only the allowance owned by %s', + async (finish) => { + vi.useFakeTimers() + const state = new RemoteRuntimePtyRecoveryState() + const epoch = state.begin() + const retry = vi.fn() + const claim = state.claimOwnerRefresh(epoch, retry) + const settled = vi.fn() + void claim?.promise.then(settled) + expect(state.claimOwnerRefresh(epoch, retry)).toEqual({ + started: false, + promise: claim?.promise + }) + const result = { id: 'owner', replay: '', isReattach: true } + await { + healthy: () => state.markHealthy(result), + cancelled: () => state.cancel(), + disposed: () => state.dispose(), + exhausted: () => state.markDisconnected(), + 'contact loss': () => state.parkOwnerRefresh(epoch), + deadline: () => vi.advanceTimersByTimeAsync(REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS) + }[finish]() + expect(await claim?.promise).toEqual(finish === 'healthy' ? result : undefined) + if (finish === 'deadline' || finish === 'exhausted' || finish === 'contact loss') { + expect(state.hasOwnerRefresh).toBe(true) + expect(state.retryNow()).toBe(true) + const nextEpoch = finish === 'contact loss' ? epoch : epoch + 1 + expect(retry).toHaveBeenCalledExactlyOnceWith(nextEpoch) + expect(state.claimOwnerRefresh(nextEpoch, retry)?.started).toBe(finish !== 'contact loss') + } else { + expect(state.hasOwnerRefresh).toBe(false) + expect(retryAllRemoteRuntimePtyRecoveriesNow()).toBe(0) + } + state.dispose() + await Promise.resolve() + expect(settled).toHaveBeenCalledOnce() + } + ) it('keeps a timed-out pane revivable through the scheduled recovery registry', async () => { vi.useFakeTimers() diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.ts index bd40493e3b7..788687d76fa 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-recovery-state.ts @@ -1,17 +1,10 @@ -export const REMOTE_RUNTIME_RECOVERY_DELAYS_MS = [ - 250, 500, 1000, 2000, 4000, 8000, 15_000, 30_000 -] as const +import type { PtyConnectResult } from './pty-transport-types' -// Why: mirrors DEFAULT_REMOTE_RUNTIME_TIMEOUT_MS in the main-process runtime router; a silently -// dropped link burns the whole RPC timeout on the attempt each backoff step leads into. -export const REMOTE_RUNTIME_RECOVERY_ATTEMPT_BUDGET_MS = 15_000 - -// Why derived, not hand-tuned: a literal deadline drifted below the ladder it arms, making the last -// backoff steps unreachable dead code (#11305). Loss of contact is never evidence of exit, so the -// window must outlast the schedule it advertises rather than the schedule being trimmed to fit. -export const REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS = - REMOTE_RUNTIME_RECOVERY_DELAYS_MS.reduce((total, delayMs) => total + delayMs, 0) + - REMOTE_RUNTIME_RECOVERY_DELAYS_MS.length * REMOTE_RUNTIME_RECOVERY_ATTEMPT_BUDGET_MS +import { + REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS, + REMOTE_RUNTIME_RECOVERY_DELAYS_MS +} from './remote-runtime-pty-recovery-limits' +export * from './remote-runtime-pty-recovery-limits' export type RemoteRuntimePtyRecoveryPhase = | 'idle' @@ -77,6 +70,12 @@ export function reviveRemoteRuntimePtyRecoveriesForEnvironment(environmentId: st } export class RemoteRuntimePtyRecoveryState { + // Caller settlement does not restore this epoch's spent owner-refresh allowance. + private ownerRefresh: { + promise: Promise + settle: ((result?: PtyConnectResult) => void) | null + retry: (epoch: number) => void + } | null = null private phase: RemoteRuntimePtyRecoveryPhase = 'idle' private epoch = 0 private attempt = 0 @@ -110,11 +109,54 @@ export class RemoteRuntimePtyRecoveryState { return this.deadlineExpired } + get hasOwnerRefresh(): boolean { + return this.ownerRefresh !== null + } + + claimOwnerRefresh(epoch: number, retry: (epoch: number) => void) { + if (!this.isCurrent(epoch)) { + return null + } + if (this.ownerRefresh) { + return { started: false, promise: this.ownerRefresh.promise } + } + const { promise, resolve } = Promise.withResolvers() + this.ownerRefresh = { promise, settle: resolve, retry } + return { started: true, promise } + } + + parkOwnerRefresh(epoch: number): void { + if (!this.ownsEpoch(epoch) || !this.ownerRefresh) { + return + } + this.settleOwnerRefresh() + if (this.isActive) { + this.parkRetryForExternalTrigger(epoch, this.ownerRefresh.retry) + } else { + this.parkRetryAfterDeadline(this.ownerRefresh.retry) + } + } + + private settleOwnerRefresh(result?: PtyConnectResult): void { + const refresh = this.ownerRefresh + if (refresh?.settle) { + const settle = refresh.settle + refresh.settle = null + settle(result) + } + } + + private clearOwnerRefresh(result?: PtyConnectResult): void { + this.settleOwnerRefresh(result) + this.ownerRefresh = null + } + begin(): number { if (this.phase === 'disposed') { return this.epoch } if (!this.isActive) { + this.clearOwnerRefresh() this.epoch += 1 this.attempt = 0 this.armDeadline(this.epoch) @@ -211,6 +253,7 @@ export class RemoteRuntimePtyRecoveryState { this.clearRetryTimer() if (latched) { // Why: the deadline only stops auto-retry; an explicit trigger opens a fresh recovery window. + this.clearOwnerRefresh() this.epoch += 1 this.attempt = 0 this.armDeadline(this.epoch) @@ -222,10 +265,11 @@ export class RemoteRuntimePtyRecoveryState { return true } - markHealthy(): void { + markHealthy(result?: PtyConnectResult): void { if (this.phase === 'disposed') { return } + this.clearOwnerRefresh(result) this.deadlineExpired = false this.clearTimers() this.phase = 'idle' @@ -243,6 +287,11 @@ export class RemoteRuntimePtyRecoveryState { this.stopRetryTimer() this.clearDeadlineTimer() this.phase = 'disconnected' + if (this.ownerRefresh) { + this.settleOwnerRefresh() + this.clearRetryTimer() + this.parkRetry(this.ownerRefresh.retry) + } this.onChange?.() } @@ -250,6 +299,7 @@ export class RemoteRuntimePtyRecoveryState { if (this.phase === 'disposed') { return } + this.clearOwnerRefresh() this.deadlineExpired = false this.epoch += 1 this.clearTimers() @@ -259,6 +309,7 @@ export class RemoteRuntimePtyRecoveryState { } dispose(): void { + this.clearOwnerRefresh() this.epoch += 1 this.clearTimers() this.phase = 'disposed' @@ -275,9 +326,7 @@ export class RemoteRuntimePtyRecoveryState { this.deadlineTimer = null this.deadlineExpired = true // Why: the cutoff stops self-initiated retries but must keep the pane revivable by online/resume/reconnect. - this.stopRetryTimer() - this.phase = 'disconnected' - this.onChange?.() + this.markDisconnected() }, REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS) timer.unref?.() this.deadlineTimer = timer diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-pane-binding-ownership.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-pane-binding-ownership.test.ts index 70d550ca461..b75e15c9ada 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-pane-binding-ownership.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-pane-binding-ownership.test.ts @@ -1,3 +1,8 @@ +import { LEAF_1, LEAF_2, unboundSplitLayout } from './terminal-pane-tab-detach-fixture' +import type { AppState } from '@/store/types' +import type { TerminalPaneController } from './use-terminal-pane-controller' +import type { ConnectPanePtySession } from './pty-connection/connect-pane-pty-session' +import { REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS } from './remote-runtime-pty-recovery-state' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createRemoteRuntimeTransportMocks, @@ -7,15 +12,22 @@ import { // These transport fixtures never read abandoned-launch ownership from the app store. const appStore = vi.hoisted(() => ({ - getState: vi.fn(() => { + getState: vi.fn<() => AppState>(() => { throw new Error('These transport fixtures must not read app-store ownership') }) })) vi.mock('@/store', () => ({ useAppStore: { getState: appStore.getState } })) +// Why: the recovery portals read a pairing-refusal hook this pane-binding test doesn't exercise. +vi.mock('./use-managed-pairing-refused', () => ({ + useManagedPairingRefusedEnvironmentId: () => null +})) +let composedStore = false afterEach(() => { - expect(appStore.getState).not.toHaveBeenCalled() + if (!composedStore) { + expect(appStore.getState).not.toHaveBeenCalled() + } }) let subscriptionCallbacks: MultiplexSubscriptionCallbacks = null @@ -25,6 +37,7 @@ const { runtimeCall, runtimeSubscribe, latestSubscribePayload, + subscribeFrameCount, inputFrameTexts, emitSnapshot, resetRemoteRuntimeTransport @@ -45,12 +58,308 @@ function unreachable(): Error { }) } +const callsTo = (method: string) => runtimeCall.mock.calls.filter(([r]) => r.method === method) + // Every exit from binding a remote pane must leave one owner: the host's answer, or the transport's own retry. describe('remote runtime pane binding ownership', () => { beforeEach(() => { resetRemoteRuntimeTransport() + composedStore = false }) + it.each([ + ['terminal-1', 'old-incarnation', 'direct'], + ['terminal-1', 'new-incarnation', 'close'], + ['terminal-2', 'new-incarnation', 'end'] + ])( + 'retains one refresh through %s delayed acceptance (%s, %s)', + async (replacement, nextIncarnation, interruption) => { + if (interruption === 'close') { + vi.useFakeTimers() + } + composedStore = true + vi.doMock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync: vi.fn() })) + vi.doMock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep: vi.fn() + })) + const { createTestStore, makeTab, seedStore } = + await import('@/store/slices/store-test-helpers') + const { installPanePtyVisibilityBind } = + await import('./pty-connection/pane-pty-visibility-bind') + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const store = createTestStore() + const old = 'remote:env-1@@saved-predecessor' + const sibling = 'remote:env-1@@sibling' + const initial = 'remote:env-1@@terminal-1' + seedStore(store, { + tabsByWorktree: { 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: old })] }, + ptyIdsByTabId: { 'tab-1': [old, sibling] }, + terminalLayoutsByTabId: { + 'tab-1': { + ...unboundSplitLayout(), + activeLeafId: LEAF_1, + ptyIdsByLeafId: { [LEAF_1]: old, [LEAF_2]: sibling } + } + } + }) + appStore.getState.mockImplementation(store.getState) + let incarnationId = 'old-incarnation' + let refused = false + let delayResolution = false + const delayed = Promise.withResolvers() + const defaultCall = runtimeCall.getMockImplementation() + runtimeCall.mockImplementation(async (args: { method: string }) => { + if (args.method !== 'terminal.resolvePane') { + return defaultCall?.(args) + } + if (delayResolution) { + delayResolution = false + await delayed.promise + } + if (!refused) { + refused = true + return { + ok: false, + error: { code: 'runtime_error', message: 'terminal_pane_owner_changed' } + } + } + return { + ok: true, + result: { + terminal: { + handle: resolvedPaneHandle, + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: LEAF_1, + incarnationId + } + } + } + }) + const onPtyRebind = vi.fn((id: string, prior: string, incarnation?: string | null) => + session.onPtyRebind(id, prior, incarnation) + ) + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: LEAF_1, + onPtyRebind + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only the visibility installer runs; its pane/store/transport dependencies are supplied below. + const session = { + pane: { id: 1, leafId: LEAF_1 }, + cacheKey: `tab-1:${LEAF_1}`, + transport, + deps: { + tabId: 'tab-1', + worktreeId: 'wt-1', + paneTransportsRef: { current: new Map([[1, transport]]) }, + isVisibleRef: { current: true }, + updateTabPtyId: store.getState().updateTabPtyId + }, + activePanePtyBinding: old, + bindProcessExitState: vi.fn(), + setPanePtyFitBinding: vi.fn(), + registerSideEffectFactConsumerForPty: vi.fn(), + syncHiddenRendererPtyDelivery: vi.fn(), + syncPanePtyLayoutBinding: vi.fn(), + agentCompletionCoordinator: { startProcessTracking: vi.fn() } + } as unknown as ConnectPanePtySession + installPanePtyVisibilityBind(session) + const onError = vi.fn() + try { + transport.attach({ existingPtyId: old, callbacks: { onError } }) + await vi.waitFor(() => expect(latestSubscribePayload().terminal).toBe('terminal-1')) + expect(onPtyRebind).toHaveBeenCalledWith(initial, old, incarnationId) + emitSnapshot(latestSubscribePayload().streamId, 'prompt$ ') + expect(store.getState().terminalLayoutsByTabId['tab-1'].ptyIdsByLeafId).toEqual({ + [LEAF_1]: initial, + [LEAF_2]: sibling + }) + expect(store.getState().tabsByWorktree['wt-1'][0].ptyId).toBe(initial) + expect(store.getState().ptyIdsByTabId['tab-1']).toEqual([initial, sibling]) + expect(session.activePanePtyBinding).toBe(initial) + expect(session.bindProcessExitState).toHaveBeenCalledWith(initial, old) + expect(session.agentCompletionCoordinator.startProcessTracking).toHaveBeenCalledOnce() + expect(onError).not.toHaveBeenCalled() + const refuse = (streamId: number, callbacks = subscriptionCallbacks) => + callbacks?.onResponse({ + ok: true, + result: { + type: 'error', + streamId, + message: 'terminal_pane_owner_changed' + } + }) + const { setDriverForPty, getDriverForPty } = + await import('@/lib/pane-manager/mobile-driver-state') + const { setFitOverride, getFitOverrideForPty } = + await import('@/lib/pane-manager/mobile-fit-overrides') + const phone = { kind: 'mobile' as const, clientId: 'phone-1' } + setDriverForPty(initial, phone) + setFitOverride(initial, 'mobile-fit', 49, 20) + const firstSubscription = subscriptionCallbacks + const firstFrameCount = subscribeFrameCount() + resolvedPaneHandle = replacement + incarnationId = nextIncarnation + refuse(latestSubscribePayload().streamId) + expect(transport.sendInput('old process input', 'driving')).toBe(true) + await vi.waitFor(() => expect(subscriptionCallbacks).not.toBe(firstSubscription)) + await vi.waitFor(() => expect(subscribeFrameCount()).toBeGreaterThan(firstFrameCount)) + await vi.waitFor(() => expect(latestSubscribePayload().terminal).toBe(replacement)) + const refreshedSubscription = subscriptionCallbacks + const epoch = transport.getRecoveryState?.().epoch + const next = `remote:env-1@@${replacement}` + expect(onPtyRebind).toHaveBeenLastCalledWith(next, initial, incarnationId) + expect.soft(getDriverForPty(next)).toEqual(phone) + expect.soft(getFitOverrideForPty(next)).toEqual({ mode: 'mobile-fit', cols: 49, rows: 20 }) + // A real multiplexer interruption must not restore the allowance before acceptance. + if (interruption !== 'direct') { + const frameCount = subscribeFrameCount() + if (interruption === 'close') { + subscriptionCallbacks?.onClose?.() + } else { + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'end', + streamId: latestSubscribePayload().streamId, + verdict: 'unverifiable' + } + }) + } + await vi.waitFor(() => expect(subscriptionCallbacks).not.toBe(refreshedSubscription)) + await vi.waitFor(() => expect(subscribeFrameCount()).toBeGreaterThan(frameCount)) + await vi.waitFor(() => expect(latestSubscribePayload().terminal).toBe(replacement)) + expect(transport.getRecoveryState?.().epoch).toBe(epoch) + } + const unacceptedSubscription = subscriptionCallbacks + const unacceptedStream = latestSubscribePayload().streamId + const resolves = callsTo('terminal.resolvePane').length + refuse(unacceptedStream) + expect(callsTo('terminal.resolvePane')).toHaveLength(resolves) + expect(transport.getRecoveryState?.()).toMatchObject({ + phase: 'disconnected', + reason: 'ownership' + }) + expect(transport.getPtyId()).toBe(next) + const frameCount = subscribeFrameCount() + expect(transport.retryRecovery?.()).toBe(true) + expect(transport.getRecoveryState?.().epoch).toBe((epoch ?? 0) + 1) + await vi.waitFor(() => expect(subscriptionCallbacks).not.toBe(unacceptedSubscription)) + await vi.waitFor(() => expect(subscribeFrameCount()).toBeGreaterThan(frameCount)) + await vi.waitFor(() => expect(latestSubscribePayload().terminal).toBe(replacement)) + emitSnapshot(latestSubscribePayload().streamId, 'replacement$ ') + await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('connected')) + refuse(unacceptedStream, unacceptedSubscription) + expect(transport.getRecoveryState?.().phase).toBe('connected') + if (nextIncarnation !== 'old-incarnation') { + expect(inputFrameTexts().join('')).not.toContain('old process input') + } + setDriverForPty(next, { kind: 'idle' }) + setFitOverride(next, 'desktop-fit', 80, 24) + if (interruption === 'close') { + delayResolution = true + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(delayResolution).toBe(false)) + await vi.advanceTimersByTimeAsync(REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS + 1) + const frameCount = subscribeFrameCount() + expect(transport.getRecoveryState?.().phase).toBe('disconnected') + expect(transport.retryRecovery?.()).toBe(true) + await vi.waitFor(() => expect(subscribeFrameCount()).toBeGreaterThan(frameCount)) + emitSnapshot(latestSubscribePayload().streamId, 'successor$ ') + await vi.advanceTimersByTimeAsync(20) + expect(transport.getRecoveryState?.().phase).toBe('connected') + const rebinds = onPtyRebind.mock.calls.length + resolvedPaneHandle = 'late-owner' + delayed.resolve() + await vi.advanceTimersByTimeAsync(20) + expect(onPtyRebind).toHaveBeenCalledTimes(rebinds) + expect(transport.getPtyId()).toBe(next) + expect(store.getState().terminalLayoutsByTabId['tab-1'].ptyIdsByLeafId?.[LEAF_1]).toBe( + next + ) + expect(transport.getRecoveryState?.().phase).toBe('connected') + expect(subscribeFrameCount()).toBe(frameCount + 1) + } + expect(callsTo('terminal.create')).toHaveLength(0) + } finally { + transport.destroy?.() + vi.useRealTimers() + } + } + ) + + it.each([false, true])( + 'resolves create refusal without replay (contact loss: %s)', + async (contactLoss) => { + vi.useFakeTimers() + const defaultCall = runtimeCall.getMockImplementation() + let reachable = !contactLoss + runtimeCall.mockImplementation(async (args: { method: string }) => { + if (args.method === 'terminal.create') { + return { + ok: false, + error: { code: 'runtime_error', message: 'terminal_pane_owner_changed' } + } + } + if (!reachable) { + throw unreachable() + } + return defaultCall?.(args) + }) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + const onError = vi.fn() + try { + const connecting = transport.connect({ url: '', callbacks: { onError } }) + if (contactLoss) { + expect(await connecting).toBeUndefined() + await vi.advanceTimersByTimeAsync(REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS + 1) + vi.doMock('react-dom', () => ({ createPortal: (element: unknown) => element })) + vi.doMock('./use-wsl-host-unavailable-reason', () => ({ + useWslHostUnavailableReason: () => null + })) + const { TerminalPaneRecoveryPortals } = await import('./TerminalPaneRuntimePortals') + const portals = TerminalPaneRecoveryPortals({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only recovery fields are read; the portal and WSL hook are inert rendering dependencies. + controller: { + managedPanes: [{ id: 1 }], + ptyRecoveryStatesByPaneId: { 1: transport.getRecoveryState?.() }, + paneTransportsRef: { current: new Map([[1, transport]]) } + } as unknown as TerminalPaneController + }) + const banner = portals?.props.children[0] + expect(banner?.props.phase).toBe('disconnected') + reachable = true + const retryRecovery = transport.retryRecovery + let accepted: boolean | undefined + transport.retryRecovery = () => (accepted = retryRecovery?.() ?? false) + banner?.props.onReconnect() + expect(accepted).toBe(true) + } + await vi.waitFor(() => expect(latestSubscribePayload().terminal).toBe('terminal-1')) + emitSnapshot(latestSubscribePayload().streamId, 'recovered$ ') + if (!contactLoss) { + expect(await connecting).toMatchObject({ id: 'remote:env-1@@terminal-1' }) + } + await vi.advanceTimersByTimeAsync(20) + expect(transport.isConnected()).toBe(true) + expect(transport.getRecoveryState?.().phase).toBe('connected') + expect(onError).not.toHaveBeenCalled() + expect(callsTo('terminal.create')).toHaveLength(1) + expect(callsTo('terminal.resolvePane')).toHaveLength(contactLoss ? 2 : 1) + } finally { + transport.destroy?.() + vi.useRealTimers() + } + } + ) + it('settles an attach whose pane the host no longer knows instead of latching in connecting (#21344)', async () => { const defaultCall = runtimeCall.getMockImplementation() runtimeCall.mockImplementation(async (args: { method: string }) => { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index d8b164f696f..9169c577d96 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -259,6 +259,7 @@ export function createRemoteRuntimePtyTransport( let desiredViewport: { cols: number; rows: number } | null = null let storedCallbacks: Parameters[0]['callbacks'] = {} const surfacedErrorMessages = new Set() + const ownerChanged = 'terminal_pane_owner_changed' let resubscribeEpoch: number | null = null let resubscribeRequestedHandle: string | null = null let resubscribeRequestedReplacementPolicy: HostHandleReplacementPolicy = 'reuse' @@ -605,13 +606,16 @@ export function createRemoteRuntimePtyTransport( return { phase, epoch: recovery.currentEpoch, - attempt: recovery.attemptCount + attempt: recovery.attemptCount, + ...((recovery.isActive && recovery.hasOwnerRefresh) || surfacedErrorMessages.has(ownerChanged) + ? { reason: 'ownership' as const } + : {}) } } function emitRecoveryState(force = false): void { const state = getRecoveryState() - const key = `${state.phase}:${state.epoch}:${state.attempt}` + const key = `${state.phase}:${state.epoch}:${state.attempt}:${state.reason ?? ''}` if (!force && key === lastRecoveryStateKey) { return } @@ -634,10 +638,10 @@ export function createRemoteRuntimePtyTransport( storedCallbacks.onError?.(message) } - function markRecoveryHealthy(): void { + function markRecoveryHealthy(result?: PtyConnectResult): void { const recoveredErrors = [...surfacedErrorMessages] surfacedErrorMessages.clear() - recovery.markHealthy() + recovery.markHealthy(result) for (const message of recoveredErrors) { storedCallbacks.onErrorCleared?.(message) } @@ -1451,9 +1455,10 @@ export function createRemoteRuntimePtyTransport( async function adoptResolvedHostPane( terminal: RuntimeTerminalResolvePane, - options: { cols?: number; rows?: number }, + options: { cols?: number; rows?: number; existingPtyId?: string }, notifySpawn = true, - expectedAttachGeneration?: number + expectedAttachGeneration?: number, + expectedRecoveryEpoch?: number ): Promise { if ( destroyed || @@ -1461,12 +1466,23 @@ export function createRemoteRuntimePtyTransport( ) { return undefined } + if (!notifySpawn) { + rebindRemoteTerminalHandle( + terminal.handle, + terminal.incarnationId ?? null, + remotePtyId ?? options.existingPtyId ?? null + ) + } else { + const previousPtyId = remotePtyId + handle = terminal.handle + remotePtyId = toRemoteRuntimePtyId(handle, currentRuntimeEnvironmentId) + unregisterShutdownHandlers(previousPtyId) + registerShutdownHandlers(remotePtyId) + } adoptExecutionMetadata(terminal) - const previousPtyId = remotePtyId - handle = terminal.handle - remotePtyId = toRemoteRuntimePtyId(handle, currentRuntimeEnvironmentId) - unregisterShutdownHandlers(previousPtyId) - registerShutdownHandlers(remotePtyId) + if (!remotePtyId) { + return undefined + } connected = true desiredViewport = { cols: options.cols ?? 80, @@ -1477,9 +1493,12 @@ export function createRemoteRuntimePtyTransport( } emitRecoveryState() try { - await subscribeToHandle() + await subscribeToHandle(expectedRecoveryEpoch) } catch (error) { - if (!recoverAfterSubscribeFailure(error, handle, remotePtyId)) { + if (recovery.hasOwnerRefresh) { + throw error + } + if (!recoverAfterSubscribeFailure(error, terminal.handle, remotePtyId)) { throw error } } @@ -1499,6 +1518,91 @@ export function createRemoteRuntimePtyTransport( } } + function reconcilePaneOwner( + options: { cols?: number; rows?: number; existingPtyId?: string }, + expectedLifecycle = lifecycleEpoch + ): Promise { + if ( + destroyed || + expectedLifecycle !== lifecycleEpoch || + recovery.currentPhase === 'disconnected' + ) { + return Promise.resolve(undefined) + } + const generation = attachGeneration + const epoch = recovery.begin() + const current = () => + !destroyed && + expectedLifecycle === lifecycleEpoch && + generation === attachGeneration && + recovery.isCurrent(epoch) + const claim = recovery.claimOwnerRefresh(epoch, (nextEpoch) => { + if (destroyed || expectedLifecycle !== lifecycleEpoch || generation !== attachGeneration) { + return + } + if (nextEpoch === epoch) { + void resolveOwner() + } else { + void reconcilePaneOwner(options, expectedLifecycle) + } + }) + if (!claim) { + return Promise.resolve(undefined) + } + function exhaust(): void { + if (!current()) { + return + } + subscriptionGeneration += 1 + closeMultiplexedStream() + connecting = false + connected = false + recovery.markDisconnected() + surfaceErrorMessage(ownerChanged) + emitRecoveryState(true) + } + async function resolveOwner(): Promise { + if (!current()) { + return + } + holdUnsentPendingInput() + closeMultiplexedStream() + connected = false + connecting = true + try { + const resolution = await resolvePersistedHostPane() + if (!current()) { + return + } + if (resolution.kind !== 'resolved') { + exhaust() + return + } + await adoptResolvedHostPane(resolution.terminal, options, false, generation, epoch) + } catch (error) { + if (!current()) { + return + } + if (runtimeTerminalErrorMessage(error).includes(ownerChanged)) { + exhaust() + } else if (isRecoverablePaneBindingError(error)) { + recovery.parkOwnerRefresh(epoch) + connecting = false + emitRecoveryState(true) + } else { + handleRemoteTerminalError(error) + } + } + } + if (claim.started) { + void resolveOwner() + emitRecoveryState(true) + } else { + exhaust() + } + return claim.promise + } + function recoverExpiredHostPane(): void { const expiredHandle = handle if (!expiredHandle || !tabId || !leafId || !worktreeId || recoveringPaneHandle) { @@ -2139,12 +2243,20 @@ export function createRemoteRuntimePtyTransport( function rebindRemoteTerminalHandle( nextHandle: string, - nextIncarnationId: string | null = null + nextIncarnationId: string | null = null, + replacedPtyId: string | null = remotePtyId ): void { clearPublishedHandleWait() - // Why: keys typed at the replaced shell must not run in its successor (#10065). - discardPendingInput() - const replacedPtyId = remotePtyId + const nextPtyId = toRemoteRuntimePtyId(nextHandle, currentRuntimeEnvironmentId) + const changed = + replacedPtyId !== nextPtyId || + (authoritativePtyIncarnationId !== null && + nextIncarnationId !== null && + authoritativePtyIncarnationId !== nextIncarnationId) + // A newly learned stamp is not evidence that held typing belongs to another process. + if (changed) { + discardPendingInput() + } unregisterShutdownHandlers(replacedPtyId) handle = nextHandle remotePtyId = toRemoteRuntimePtyId(nextHandle, currentRuntimeEnvironmentId) @@ -2154,9 +2266,11 @@ export function createRemoteRuntimePtyTransport( registerShutdownHandlers(remotePtyId) setAttachmentReady(false) // Why: host handle rotation preserves the pane generation; only the store identity changes, not spawn/exit semantics. - if (replacedPtyId) { - replaceFitOverridePtyId(replacedPtyId, remotePtyId) - replaceDriverPtyId(replacedPtyId, remotePtyId) + if (replacedPtyId && (changed || nextIncarnationId !== null)) { + if (replacedPtyId !== remotePtyId) { + replaceFitOverridePtyId(replacedPtyId, remotePtyId) + replaceDriverPtyId(replacedPtyId, remotePtyId) + } if (nextIncarnationId) { onPtyRebind?.(remotePtyId, replacedPtyId, nextIncarnationId) } else { @@ -2236,6 +2350,10 @@ export function createRemoteRuntimePtyTransport( function handleRemoteTerminalError(error: unknown): void { const message = runtimeTerminalErrorMessage(error) + if (message.includes(ownerChanged)) { + void reconcilePaneOwner(desiredViewport ?? {}) + return + } if (message === REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE) { // Why: an oversized initial snapshot is skipped but live output keeps flowing — informational, not fatal. return @@ -2284,6 +2402,9 @@ export function createRemoteRuntimePtyTransport( return } connecting = false + if (recovery.hasOwnerRefresh) { + recovery.markDisconnected() + } discardPendingInput() emitRecoveryState() surfaceErrorMessage(message) @@ -2395,7 +2516,12 @@ export function createRemoteRuntimePtyTransport( } else if (tabId && leafId && worktreeId) { const resolution = await resolvePersistedHostPane() const resolved = resolution.kind === 'resolved' ? resolution.terminal : null - if (destroyed || !connected || handle !== previousHandle) { + if ( + destroyed || + !connected || + handle !== previousHandle || + !recovery.isCurrent(recoveryEpoch) + ) { return } const effectivePolicy = stricterReplacementPolicy( @@ -2499,7 +2625,9 @@ export function createRemoteRuntimePtyTransport( scheduleResubscribeAfterTransportClose(currentReplacementPolicy, nextEpoch) }) } else { - recovery.markDisconnected() + if (!runtimeTerminalErrorMessage(error).includes(ownerChanged)) { + recovery.markDisconnected() + } // Why: stale/gone/SSH-expired handling lives in handleRemoteTerminalError; its // fallthrough surfaces the message, so routing here keeps those recoveries alive. handleRemoteTerminalError(error) @@ -2561,6 +2689,8 @@ export function createRemoteRuntimePtyTransport( const subscribedHandle = handle const subscribedPtyId = remotePtyId const generation = ++subscriptionGeneration + const recoveryEpoch = + expectedRecoveryEpoch ?? (recovery.isActive ? recovery.currentEpoch : undefined) setAttachmentReady(false) let transportClosed = false let subscriptionAttached = false @@ -2572,7 +2702,9 @@ export function createRemoteRuntimePtyTransport( const isCurrentSubscription = (): boolean => !transportClosed && generation === subscriptionGeneration && - (expectedRecoveryEpoch === undefined || recovery.ownsEpoch(expectedRecoveryEpoch)) && + (recoveryEpoch === undefined || + recovery.isCurrent(recoveryEpoch) || + (subscriptionAttached && recovery.ownsEpoch(recoveryEpoch))) && isCurrentRemoteTerminal(subscribedHandle, subscribedPtyId) if (currentPaneRevision() !== getRuntimeEnvironmentRevision(currentRuntimeEnvironmentId)) { // Why local: the shared stream would subscribe on a pairing not proven to be this pane's machine. @@ -2685,8 +2817,15 @@ export function createRemoteRuntimePtyTransport( subscriptionAttached = true setAttachmentReady(true) connecting = false + markRecoveryHealthy({ + id: subscribedPtyId!, + replay: '', + isReattach: true, + ...(authoritativePtyIncarnationId + ? { incarnationId: authoritativePtyIncarnationId } + : {}) + }) resetRecoveryReplacementPolicy() - markRecoveryHealthy() emitRecoveryState() storedCallbacks.onConnect?.() // Why: a subscribe that replayed no visible image (empty, escape-only, or over the client's @@ -2707,6 +2846,8 @@ export function createRemoteRuntimePtyTransport( } outputProcessor.clearAccumulatedState() if (verdict === 'unverifiable' || (tabId && isWebTerminalSurfaceTabId(tabId))) { + subscriptionGeneration += 1 + closeMultiplexedStream() setAttachmentReady(false) multiplexedStream = null multiplexedStreamHandle = null @@ -2722,6 +2863,7 @@ export function createRemoteRuntimePtyTransport( } return } + recovery.cancel() unregisterShutdownHandlers(subscribedPtyId) discardPendingInput() connected = false @@ -2777,16 +2919,11 @@ export function createRemoteRuntimePtyTransport( } }, onTransportClose: ({ recoverable, retryWithBackoff }) => { + const current = isCurrentSubscription() transportClosed = true - if (generation !== subscriptionGeneration) { + if (!current) { return } - if (!isCurrentSubscription()) { - // isCurrentSubscription excludes the just-closed stream by design. - if (!isCurrentRemoteTerminal(subscribedHandle, subscribedPtyId)) { - return - } - } multiplexedStream = null multiplexedStreamHandle = null setAttachmentReady(false) @@ -2810,7 +2947,10 @@ export function createRemoteRuntimePtyTransport( if ( transportClosed || generation !== subscriptionGeneration || - (expectedRecoveryEpoch !== undefined && !recovery.ownsEpoch(expectedRecoveryEpoch)) || + (recoveryEpoch !== undefined && + !(subscriptionAttached + ? recovery.ownsEpoch(recoveryEpoch) + : recovery.isCurrent(recoveryEpoch))) || destroyed || !connected || handle !== subscribedHandle || @@ -2824,7 +2964,7 @@ export function createRemoteRuntimePtyTransport( connected && handle === subscribedHandle && recovery.currentPhase === 'disconnected' && - (expectedRecoveryEpoch === undefined || recovery.ownsEpoch(expectedRecoveryEpoch)) + (recoveryEpoch === undefined || recovery.ownsEpoch(recoveryEpoch)) ) { parkResubscribeAfterDeadline(subscribedHandle) } @@ -2948,10 +3088,14 @@ export function createRemoteRuntimePtyTransport( // Why: a host that cannot resolve this pane gives no later answer to wait for; settle offline. connecting = false emitRecoveryState() - })().catch((error) => { + })().catch(async (error) => { if (generation !== attachGeneration || attachLifecycleEpoch !== lifecycleEpoch || destroyed) { return } + if (runtimeTerminalErrorMessage(error).includes(ownerChanged)) { + await reconcilePaneOwner(options, attachLifecycleEpoch) + return + } clearPendingViewportClaim() if (!connected && isRetryableUnboundAttachError(error)) { // Why: no handle is bound yet, so the resubscribe path cannot own this retry; replay attach @@ -3030,6 +3174,9 @@ export function createRemoteRuntimePtyTransport( options: Parameters[0] ): Promise { cancelTerminalCreateRetryWait() + if (recovery.hasOwnerRefresh) { + recovery.cancel() + } const connectLifecycleEpoch = ++lifecycleEpoch const createEnvironmentId = currentRuntimeEnvironmentId lastConnectOptions = options @@ -3272,6 +3419,9 @@ export function createRemoteRuntimePtyTransport( if (!destroyed && lifecycleEpoch === connectLifecycleEpoch) { connecting = false const message = runtimeTerminalErrorMessage(error) + if (message.includes(ownerChanged)) { + return await reconcilePaneOwner(options, connectLifecycleEpoch) + } const deferral = readTerminalSpawnDeferral(error) if (deferral) { recovery.cancel() @@ -3475,6 +3625,9 @@ export function createRemoteRuntimePtyTransport( }, retryRecovery() { + if (recovery.hasOwnerRefresh || surfacedErrorMessages.has(ownerChanged)) { + return recovery.retryNow() + } if ( !destroyed && !terminalEnded &&