From 41f47b54f02f82c468f7bd202fb332e8bc84eeb4 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:28:24 -0800 Subject: [PATCH] Reuse resource ownership and fence restart results before attachment --- .../ipc/pty/pane/account-restart-owner.ts | 2 +- src/main/ipc/pty/pty-account-restart.test.ts | 20 +++--- .../pty/pty-pane-owner-swap.test-fixture.ts | 4 +- .../profile-writing/write-scheduling.ts | 8 ++- .../recovery/account-restart-pane-snapshot.ts | 2 +- .../codex-restart-status-summary.ts | 4 -- .../codex-account-restart-execution.ts | 27 +++++++- .../codex-account-restart.test.ts | 62 +++++++++++++++++ .../codex-detached-account-continuation.ts | 3 +- .../spawn/fresh-pane-spawn-settlement.ts | 3 +- .../pty-connection/spawn/fresh-spawn-start.ts | 6 +- .../pty-transport/ipc/ipc-pty-connect.ts | 3 + ...transport-recycled-pty-incarnation.test.ts | 32 +++++++++ .../pty-transport/pty-transport-types.ts | 4 +- ...runtime-pty-transport-spawn-intent.test.ts | 37 +++++++++- .../remote-runtime-pty-transport.ts | 10 +++ .../codex-account-restart-actions.ts | 69 +++++++++---------- .../codex-account-restart-continuation.ts | 22 ++++-- .../terminals/pty/terminal-restart-state.ts | 2 +- .../pty/lifecycle/account-restart-contract.ts | 2 + 20 files changed, 249 insertions(+), 73 deletions(-) diff --git a/src/main/ipc/pty/pane/account-restart-owner.ts b/src/main/ipc/pty/pane/account-restart-owner.ts index 95b00a389ca..8f4a697240f 100644 --- a/src/main/ipc/pty/pane/account-restart-owner.ts +++ b/src/main/ipc/pty/pane/account-restart-owner.ts @@ -52,7 +52,7 @@ export function accountRestartSnapshot( } return { hostEpoch: store.getPtyBindingEpoch(), - bindingGeneration: store.trackPtyBindingGeneration(hostId, pane.leafId), + bindingGeneration: store.trackPtyBindingGeneration({ hostId, leafId: pane.leafId }), tabCreatedAt: tab.createdAt, owner: owner?.incarnationId ? { ptyId: owner.ptyId, incarnationId: owner.incarnationId } : null } diff --git a/src/main/ipc/pty/pty-account-restart.test.ts b/src/main/ipc/pty/pty-account-restart.test.ts index ac1be31f17c..7ec4e703c07 100644 --- a/src/main/ipc/pty/pty-account-restart.test.ts +++ b/src/main/ipc/pty/pty-account-restart.test.ts @@ -81,7 +81,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('replaces the exact old endpoint and commits its successor', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) registerWithFakes(mainWindow, h.runtime, h.store) await expect(invoke()).resolves.toMatchObject({ id: 'pty-new' }) expect(h.leafBinding()).toBe('pty-new') @@ -90,7 +90,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('refuses same-ID/new-incarnation ownership before stopping anything', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) h.store.getWorkspaceSession().terminalPtyIncarnationsByPaneKey[paneKey] = 'inc-successor' registerWithFakes(mainWindow, h.runtime, h.store) await expect(invoke()).rejects.toThrow('terminal_pane_owner_changed') @@ -101,7 +101,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('preserves the human diagnostic and unknown stop evidence on pre-stop failure', async () => { const h = installRestartHarness({ shutdownFails: true }) - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) registerWithFakes(mainWindow, h.runtime, h.store) const error = await invoke().catch((e: unknown) => e) expect(String(error)).toContain('daemon unreachable') @@ -116,7 +116,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('keeps an empty existing leaf after post-stop failure and retries there without a PTY', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) h.providerSpawn.mockRejectedValueOnce(new Error('launch refused')) registerWithFakes(mainWindow, h.runtime, h.store) const error = await invoke().catch((e: unknown) => e) @@ -144,7 +144,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('does not dispatch or clear a successor installed during stop', async () => { const stopped = Promise.withResolvers() const h = installRestartHarness({ shutdownGate: stopped.promise }) - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) registerWithFakes(mainWindow, h.runtime, h.store) const pending = invoke().catch((e: unknown) => e) await vi.waitFor(() => expect(h.shutdown).toHaveBeenCalledTimes(1)) @@ -157,7 +157,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { }) it('enforces the same condition on runtime creation and empty-leaf Retry', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) registerWithFakes(mainWindow, h.runtime, h.store) const controller = h.runtime.setPtyController.mock.lastCall?.[0] if (!controller?.spawn) { @@ -194,7 +194,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('refuses a late binding commit over a successor and preserves that successor', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) h.providerSpawn.mockImplementationOnce(async () => { h.store.getWorkspaceSession().terminalLayoutsByTabId[tabId].ptyIdsByLeafId[leafId] = 'successor' @@ -207,7 +207,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { }) it('preserves the Windows capacity diagnostic and stop proof through the final IPC formatter', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) h.providerSpawn.mockRejectedValueOnce( terminalStartError( 'provider stopped', @@ -230,7 +230,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { it('preserves the same proof in the RPC error data after Windows formatting', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) h.providerSpawn.mockRejectedValueOnce( terminalStartError( 'provider stopped', @@ -266,7 +266,7 @@ describe('conditional Codex account restart at the IPC boundary', () => { }) it('refuses an ensure adoption from another provider-session owner without touching that owner', async () => { const h = installRestartHarness() - h.store.trackPtyBindingGeneration('local', leafId) + h.store.trackPtyBindingGeneration({ hostId: 'local', leafId }) registerWithFakes(mainWindow, h.runtime, h.store) const controller = h.runtime.setPtyController.mock.lastCall?.[0] if (!controller?.spawn) { diff --git a/src/main/ipc/pty/pty-pane-owner-swap.test-fixture.ts b/src/main/ipc/pty/pty-pane-owner-swap.test-fixture.ts index 210cda2a9dc..2f498297312 100644 --- a/src/main/ipc/pty/pty-pane-owner-swap.test-fixture.ts +++ b/src/main/ipc/pty/pty-pane-owner-swap.test-fixture.ts @@ -218,8 +218,8 @@ export function installRestartHarness( persistPtyBinding: vi.fn(bindingOperations.persistPtyBinding.bind(bindingOperations)), retirePtyBinding: bindingOperations.retirePtyBinding.bind(bindingOperations), getPtyBindingEpoch: () => 'test-host', - trackPtyBindingGeneration: (hostId: ExecutionHostId, leafId: string) => - trackPtyBindingGeneration(bindingRuntime, hostId, leafId), + trackPtyBindingGeneration: (pane: { hostId: ExecutionHostId; leafId: string }) => + trackPtyBindingGeneration(bindingRuntime, pane.hostId, pane.leafId), getPtyBindingGeneration: (hostId: ExecutionHostId, leafId: string) => ptyBindingGenerationOf(bindingRuntime, hostId, leafId) }) diff --git a/src/main/persistence/loading-store/profile-writing/write-scheduling.ts b/src/main/persistence/loading-store/profile-writing/write-scheduling.ts index a482596ec91..114c8891b90 100644 --- a/src/main/persistence/loading-store/profile-writing/write-scheduling.ts +++ b/src/main/persistence/loading-store/profile-writing/write-scheduling.ts @@ -88,8 +88,12 @@ export class WriteSchedulingOperations { } } - trackPtyBindingGeneration(hostId: ExecutionHostId, leafId: string): number { - return trackPtyBindingGeneration(this[writeSchedulingOperationsContext].runtime, hostId, leafId) + trackPtyBindingGeneration(pane: { hostId: ExecutionHostId; leafId: string }): number { + return trackPtyBindingGeneration( + this[writeSchedulingOperationsContext].runtime, + pane.hostId, + pane.leafId + ) } getPtyBindingEpoch(): string { return this[writeSchedulingOperationsContext].runtime.workspaceMetadataCursor.epoch diff --git a/src/main/runtime/terminal/recovery/account-restart-pane-snapshot.ts b/src/main/runtime/terminal/recovery/account-restart-pane-snapshot.ts index 9554bee200a..94b9a99fe88 100644 --- a/src/main/runtime/terminal/recovery/account-restart-pane-snapshot.ts +++ b/src/main/runtime/terminal/recovery/account-restart-pane-snapshot.ts @@ -49,7 +49,7 @@ export function resolveAccountRestartPane( }), accountRestart: { hostEpoch: store.getPtyBindingEpoch(), - bindingGeneration: store.trackPtyBindingGeneration(hostId, pane.leafId), + bindingGeneration: store.trackPtyBindingGeneration({ hostId, leafId: pane.leafId }), tabCreatedAt: tab.createdAt, owner: bound && incarnationId ? { ptyId: bound, incarnationId } : null } diff --git a/src/renderer/src/components/status-bar/codex-restart-status-summary.ts b/src/renderer/src/components/status-bar/codex-restart-status-summary.ts index 9eadde0f80a..f61f6430236 100644 --- a/src/renderer/src/components/status-bar/codex-restart-status-summary.ts +++ b/src/renderer/src/components/status-bar/codex-restart-status-summary.ts @@ -55,7 +55,6 @@ export function selectCodexRestartStatus( previous.ptyIdsByTabId !== state.ptyIdsByTabId || previous.tabsByWorktree !== state.tabsByWorktree || previous.terminalLayoutsByTabId !== state.terminalLayoutsByTabId || - previous.settings?.activeRuntimeEnvironmentId !== state.settings?.activeRuntimeEnvironmentId || previous.repos !== state.repos || previous.worktreesByRepo !== state.worktreesByRepo || previous.detectedWorktreesByRepo !== state.detectedWorktreesByRepo || @@ -70,9 +69,6 @@ export function selectCodexRestartStatus( previous.removedRuntimeEnvironmentIds !== state.removedRuntimeEnvironmentIds ) { previous = { - settings: state.settings - ? { activeRuntimeEnvironmentId: state.settings.activeRuntimeEnvironmentId } - : null, repos: state.repos, worktreesByRepo: state.worktreesByRepo, detectedWorktreesByRepo: state.detectedWorktreesByRepo, diff --git a/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart-execution.ts b/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart-execution.ts index 43284bd1cfd..e5895741abc 100644 --- a/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart-execution.ts +++ b/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart-execution.ts @@ -2,7 +2,7 @@ import { getTerminalNoticeLifetime } from '../provider-notices/terminal-notice-l import { waitForPromiseWithSignal } from '../../../../../shared/abort-signal-reason' import { useAppStore } from '@/store' import { runtimeEnvironmentIdUnlessTwin } from '@/lib/workspace/ownership/resolve-owner' -import { getExecutionHostIdForWorktree } from '@/lib/workspace/ownership/worktree-runtime-owner' +import { getResourceExecutionHostIdForWorktree } from '@/lib/workspace/ownership/worktree-runtime-owner' import { callRuntimeRpc } from '@/runtime/rpc/calls/runtime-rpc-call' import { toHostSessionPaneKey } from '../../../../../shared/workspace-layout/terminal-surface-id' import { makePaneKey, parsePaneKey } from '../../../../../shared/workspace-layout/stable-pane-id' @@ -33,6 +33,26 @@ export function isObsoleteCodexRestartResult( ) } +/** Claim the returned endpoint before the transport admits its process lifetime. */ +export function continueCodexAccountRestart( + token: CodexAccountRestartToken, + result?: { id: string; incarnationId?: string | null } +): boolean { + const state = useAppStore.getState(), + request = currentCodexAccountRestart(state, token) + if (!request) { + return false + } + if (result && isObsoleteCodexRestartResult(request, result)) { + state.updateCodexAccountRestart(token, null) + return false + } + if (result) { + state.updateCodexAccountRestart(token, { resultPtyId: result.id }) + } + return true +} + export function prepareCodexAccountRestart( request: CodexAccountRestart ): Promise { @@ -51,7 +71,7 @@ async function prepareCodexAccountRestartOwner( if (!request.leafId || !currentCodexAccountRestart(state, token)) { return null } - if (getExecutionHostIdForWorktree(state, request.worktreeId) !== request.hostId) { + if (getResourceExecutionHostIdForWorktree(state, request.worktreeId) !== request.hostId) { state.updateCodexAccountRestart(token, null) return null } @@ -115,7 +135,8 @@ async function prepareCodexAccountRestartOwner( return null } if ( - getExecutionHostIdForWorktree(useAppStore.getState(), request.worktreeId) !== request.hostId || + getResourceExecutionHostIdForWorktree(useAppStore.getState(), request.worktreeId) !== + request.hostId || runtimeEnvironmentIdUnlessTwin(useAppStore.getState(), request.worktreeId) !== environmentId ) { throw new Error('terminal_pane_owner_changed') diff --git a/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart.test.ts b/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart.test.ts index adc8baebb60..5e1d4152ae1 100644 --- a/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart.test.ts +++ b/src/renderer/src/components/terminal-pane/account-restart/codex-account-restart.test.ts @@ -17,6 +17,7 @@ import { useAppStore } from '@/store' import { selectCodexStaleSessionCount } from '@/components/status-bar/codex-restart-status-summary' import { codexAccountRestartToken } from '@/store/terminals/codex-account-restart-continuation' import { + continueCodexAccountRestart, prepareCodexAccountRestart, failCodexAccountRestart, retryCodexAccountRestarts @@ -650,6 +651,67 @@ describe('Codex account continuation', () => { } ) + it.each(['not-started', 'confirmed-retired'] as const)( + 'ignores a late %s failure after a same-ID successor takes the pane', + async (stop) => { + const request = accepted() + await prepareCodexAccountRestart(request) + const lifetime = admitTerminalNoticeLifetime('old', { + incarnationId: 'inc-successor', + fresh: true + }) + try { + failCodexAccountRestart( + codexAccountRestartToken(request), + receipt(request, stop, stop === 'not-started' ? 'original' : 'empty') + ) + expect(useAppStore.getState().terminalLayoutsByTabId.tab.ptyIdsByLeafId?.[leaf]).toBe('old') + expect(selectCodexStaleSessionCount(useAppStore.getState())).toBe(0) + } finally { + retireTerminalNoticeLifetime(lifetime) + } + } + ) + + it('accepts its own same-ID replacement before the transport admits the new incarnation', async () => { + const request = accepted() + await prepareCodexAccountRestart(request) + expect( + continueCodexAccountRestart(codexAccountRestartToken(request), { + id: 'old', + incarnationId: 'inc-own-replacement' + }) + ).toBe(true) + const lifetime = admitTerminalNoticeLifetime('old', { + incarnationId: 'inc-own-replacement', + fresh: true + }) + try { + const scrollback = new Map([[leaf, 'old transcript']]) + const session = mountedSpawnSession( + request, + scrollback, + async () => undefined, + 'inc-own-replacement' + ) + session.transport.getPtyId = () => 'old' + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fixture supplies the settlement members consumed by this callback. + settleFreshPaneSpawn(session as never, { + spawnedPtyId: 'old', + accountError: null, + accountDeferred: false, + ownerRefused: false, + generation: 1, + startupOverride: null, + options: {} + }) + expect(scrollback.has(leaf)).toBe(false) + expect(selectCodexStaleSessionCount(useAppStore.getState())).toBe(0) + } finally { + retireTerminalNoticeLifetime(lifetime) + } + }) + it('reopens the live moved endpoint without carrying the old request into its new pane', async () => { const request = accepted() await prepareCodexAccountRestart(request) diff --git a/src/renderer/src/components/terminal-pane/account-restart/detached/codex-detached-account-continuation.ts b/src/renderer/src/components/terminal-pane/account-restart/detached/codex-detached-account-continuation.ts index 1e8cda4894d..f478f0431a8 100644 --- a/src/renderer/src/components/terminal-pane/account-restart/detached/codex-detached-account-continuation.ts +++ b/src/renderer/src/components/terminal-pane/account-restart/detached/codex-detached-account-continuation.ts @@ -1,3 +1,4 @@ +import { ACCOUNT_RESTART_OUTCOME_UNKNOWN } from '../../../../../../shared/terminal/pty/lifecycle/account-restart-contract' import { readTerminalSpawnDeferral } from '../../../../../../shared/terminal/pty/admission/spawn-admission-contract' import { setPaneSpawnDeferral, @@ -128,7 +129,7 @@ export async function restartDetachedAccountRequest( const release = holdDetachedRestartSlot(accountRestartSlotKey(key, token), () => { useAppStore.getState().updateCodexAccountRestart(token, { phase: 'unresolved', - error: 'terminal_account_restart_outcome_unknown' + error: ACCOUNT_RESTART_OUTCOME_UNKNOWN }) onSettled?.() }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-pane-spawn-settlement.ts b/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-pane-spawn-settlement.ts index bf1d80795ac..a4213ce8717 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-pane-spawn-settlement.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-pane-spawn-settlement.ts @@ -1,3 +1,4 @@ +import { ACCOUNT_RESTART_VIEW_CHANGED } from '../../../../../../shared/terminal/pty/lifecycle/account-restart-contract' import { isObsoleteCodexRestartResult, failCodexAccountRestart @@ -49,7 +50,7 @@ export function settleFreshPaneSpawn( if (session.disposed || generation !== session.transportStreamGeneration) { useAppStore.getState().updateCodexAccountRestart(accountToken, { phase: 'unresolved', - error: 'terminal_account_restart_view_changed' + error: ACCOUNT_RESTART_VIEW_CHANGED }) return } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-spawn-start.ts b/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-spawn-start.ts index 46211cc509c..6ac14b909c4 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-spawn-start.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/spawn/fresh-spawn-start.ts @@ -1,6 +1,7 @@ import { releaseDeferredCwdFence } from './deferred-cwd-spawn-fence' export { releaseDeferredCwdFence } from './deferred-cwd-spawn-fence' import { settleFreshPaneSpawn } from './fresh-pane-spawn-settlement' +import { continueCodexAccountRestart } from '../../account-restart/codex-account-restart-execution' import { currentCodexAccountRestart } from '@/store/terminals/codex-account-restart-continuation' import { applyTerminalLaunchResult } from '../startup/terminal-launch-result-policy' import { isPaneOwnerRefusal } from '../host-ownership/pane-owner-recovery' @@ -116,9 +117,8 @@ export function bindStartFreshSpawn(session: ConnectPanePtySession): void { } : {}), ...(session.userRestart ? { claimReplacedPtyId: session.claimPendingReplacedPtyId } : {}), - shouldContinue: () => - (!accountToken || - Boolean(currentCodexAccountRestart(useAppStore.getState(), accountToken))) && + shouldContinue: (result) => + (!accountToken || continueCodexAccountRestart(accountToken, result)) && !session.disposed && (findTerminalTabForPane(useAppStore.getState(), session.deps.worktreeId, session.deps.tabId) ?.generation ?? 0) === session.tabGeneration, diff --git a/src/renderer/src/components/terminal-pane/pty-transport/ipc/ipc-pty-connect.ts b/src/renderer/src/components/terminal-pane/pty-transport/ipc/ipc-pty-connect.ts index 59f21525d1f..456daee5922 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport/ipc/ipc-pty-connect.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport/ipc/ipc-pty-connect.ts @@ -87,6 +87,9 @@ export async function connectIpcPty( // Fence before spawning: redeployed relays recycle IDs, so earlier buffered state belongs to a previous PTY lifetime. const priorIncarnationFence = currentPreHandlerPtySequence() const spawnResult = await spawnIpcPty(transportOptions, options, admittedSessionId) + if (options.accountRestart && options.shouldContinue && !options.shouldContinue(spawnResult)) { + return + } settleAgentLaunchPaneStart(transportOptions.tabId, transportOptions.leafId, spawnResult.id) // Any resolved spawn or cold restore is live under this id, even if this transport was destroyed meanwhile. markTerminalProviderSessionBound(spawnResult.id) diff --git a/src/renderer/src/components/terminal-pane/pty-transport/ipc/pty-transport-recycled-pty-incarnation.test.ts b/src/renderer/src/components/terminal-pane/pty-transport/ipc/pty-transport-recycled-pty-incarnation.test.ts index dc851f9a887..3b8f9e67004 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport/ipc/pty-transport-recycled-pty-incarnation.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport/ipc/pty-transport-recycled-pty-incarnation.test.ts @@ -110,6 +110,38 @@ describe('createIpcPtyTransport against a relay-recycled PTY id', () => { expect(connected).not.toHaveBeenCalled() }) + it('does not bind or kill a late account-restart result after its pane owner changed', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const pending = Promise.withResolvers<{ id: string; incarnationId: string }>() + vi.mocked(window.api.pty.spawn).mockReturnValueOnce(pending.promise) + let current = true + const onPtySpawn = vi.fn() + const transport = createIpcPtyTransport({ onPtySpawn }) + const connecting = transport.connect({ + url: '', + accountRestart: { + requestId: 'request', + attemptId: 'attempt', + hostEpoch: 'host', + pane: { + worktreeId: 'wt', + tabId: 'tab', + leafId: '11111111-1111-4111-8111-111111111111', + tabCreatedAt: 1 + }, + expected: { kind: 'empty', bindingGeneration: 1 } + }, + shouldContinue: () => current, + callbacks: {} + }) + current = false + pending.resolve({ id: 'successor-id', incarnationId: 'obsolete-incarnation' }) + expect(await connecting).toBeUndefined() + expect(transport.getPtyId()).toBeNull() + expect(onPtySpawn).not.toHaveBeenCalled() + expect(window.api.pty.kill).not.toHaveBeenCalled() + }) + // Missing incarnation stays unknown for old-host/synthetic exits; remote-runtime exits use a different channel. it('never discards on incarnation when the host names none', async () => { const { result, paneExit } = await connectWithExitDuringSpawn( diff --git a/src/renderer/src/components/terminal-pane/pty-transport/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport/pty-transport-types.ts index 70bf8e78bc2..874de4d4116 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport/pty-transport-types.ts @@ -203,8 +203,8 @@ export type PtyTransport = { startupCommandDelivery?: StartupCommandDelivery /** Consume only at fresh spawn dispatch; main stops the returned predecessor before resolving ownership. Reattach leaves retirement with the caller. */ claimReplacedPtyId?: () => string | null - /** Reject a stale pane after any pre-spawn test gate but before creating a PTY. */ - shouldContinue?: () => boolean + /** Fence dispatch and, for account restarts, acceptance before binding a returned owner. */ + shouldContinue?: (result?: { id: string; incarnationId?: string | null }) => boolean callbacks: PtyCallbacks }) => void | Promise attach: (options: { diff --git a/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-spawn-intent.test.ts b/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-spawn-intent.test.ts index 486507e6c76..04538c8c141 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-spawn-intent.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-spawn-intent.test.ts @@ -81,6 +81,41 @@ describe('createRemoteRuntimePtyTransport: restore spawn intent', () => { return transport.connect({ url: '', resumeSpawnIntent: 'automatic-restore', callbacks }) } + it('does not bind or publish a late account-restart result whose pane owner changed', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onPtySpawn = vi.fn() + ensureResponse = okResponse({ + terminal: { + handle: 'replacement', + incarnationId: 'obsolete-incarnation', + worktreeId: 'repo1::/remote/wt', + title: 'Codex' + } + }) + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'repo1::/remote/wt', + launchAgent: 'codex', + tabId: 'tab-1', + leafId: LEAF_ID, + onPtySpawn + }) + const result = await transport.connect({ + url: '', + accountRestart: { + requestId: 'request', + attemptId: 'attempt', + hostEpoch: 'host', + pane: { worktreeId: 'repo1::/remote/wt', tabId: 'tab-1', leafId: LEAF_ID, tabCreatedAt: 1 }, + expected: { kind: 'empty', bindingGeneration: 1 } + }, + shouldContinue: (result) => result === undefined, + callbacks: {} + }) + expect(result).toBeUndefined() + expect(transport.getPtyId()).toBeNull() + expect(onPtySpawn).not.toHaveBeenCalled() + }) + it.each([false, true])( 'keeps saved-origin negotiation stable through an unknown outcome (capability %s)', async (modern) => { @@ -117,7 +152,7 @@ describe('createRemoteRuntimePtyTransport: restore spawn intent', () => { } else { expect(requests[0].params).not.toHaveProperty('resumeOrigin') } - expect(onSpawnDeferred).toHaveBeenCalledWith('paused') + expect(onSpawnDeferred).toHaveBeenCalledWith('paused', 'terminal_spawn_deferred:paused') } ) }) diff --git a/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport.ts index daf778432b7..993a33b6ef3 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport.ts @@ -3892,6 +3892,16 @@ export function createRemoteRuntimePtyTransport( return } const createdTerminal = created.terminal + if ( + options.accountRestart && + options.shouldContinue && + !options.shouldContinue({ + id: toRemoteRuntimePtyId(createdTerminal.handle, createEnvironmentId), + incarnationId: createdTerminal.incarnationId + }) + ) { + return + } adoptExecutionMetadata(createdTerminal) if (created.disposition !== undefined && tabId && createdTerminal.tabId) { recordWebAgentSessionHandoff({ diff --git a/src/renderer/src/store/terminals/codex-account-restart-actions.ts b/src/renderer/src/store/terminals/codex-account-restart-actions.ts index d7a3cbe9ee2..d02f1a96b46 100644 --- a/src/renderer/src/store/terminals/codex-account-restart-actions.ts +++ b/src/renderer/src/store/terminals/codex-account-restart-actions.ts @@ -9,7 +9,7 @@ import { captureAgentAccountRestartResumeInput, buildPaneAccountRestartStartup } from '@/lib/agent-session/accounts/agent-account-restart-startup' -import { getExecutionHostIdForWorktree } from '@/lib/workspace/ownership/worktree-runtime-owner' +import { getResourceExecutionHostIdForWorktree } from '@/lib/workspace/ownership/worktree-runtime-owner' import { currentCodexRestartPanes, eligibleCodexRestartPtyIds, @@ -17,10 +17,11 @@ import { currentCodexAccountRestart } from './codex-account-restart-continuation' import type { CodexAccountRestart } from './terminal-contracts' -import type { TerminalSlice, TerminalStoreSet } from './terminal-state' +import type { TerminalSlice, TerminalStoreGet, TerminalStoreSet } from './terminal-state' export function createCodexAccountRestartActions( - set: TerminalStoreSet + set: TerminalStoreSet, + get: TerminalStoreGet ): Pick< TerminalSlice, 'claimCodexAccountRestart' | 'updateCodexAccountRestart' | 'retryCodexAccountRestart' @@ -92,12 +93,12 @@ export function createCodexAccountRestartActions( tab?.startupCwd ?? s.getKnownWorktreeById( pane.worktreeId, - getExecutionHostIdForWorktree(s, pane.worktreeId) + getResourceExecutionHostIdForWorktree(s, pane.worktreeId) )?.path, connectionId: getConnectionIdFromState(s, pane.worktreeId), shellOverride: tab?.shellOverride, projectRuntime: getLocalProjectExecutionRuntimeContext(s, pane.worktreeId), - hostId: getExecutionHostIdForWorktree(s, pane.worktreeId), + hostId: getResourceExecutionHostIdForWorktree(s, pane.worktreeId), phase: startup ? 'accepted' : 'failed', startupInput: startup ? null : startupInput, startup, @@ -176,40 +177,34 @@ export function createCodexAccountRestartActions( paneAccountRestarts: next, ...(nextNotices !== notices ? { codexRestartNoticeByPtyId: nextNotices } : {}) } - if ( - (patch?.phase === 'failed' || - patch?.phase === 'deferred' || - patch?.phase === 'unresolved') && - patch.condition?.expected.kind === 'empty' && - request.leafId - ) { - const layout = s.terminalLayoutsByTabId[request.tabId] - if (layout?.ptyIdsByLeafId?.[request.leafId] === request.oldPtyId) { - const bindings = { ...layout.ptyIdsByLeafId } - delete bindings[request.leafId] - const ids = (s.ptyIdsByTabId[request.tabId] ?? []).filter( - (id) => id !== request.oldPtyId - ) - return { - ...common, - terminalLayoutsByTabId: { - ...s.terminalLayoutsByTabId, - [request.tabId]: { ...layout, ptyIdsByLeafId: bindings } - }, - ptyIdsByTabId: { ...s.ptyIdsByTabId, [request.tabId]: ids }, - tabsByWorktree: { - ...s.tabsByWorktree, - [request.worktreeId]: s.tabsByWorktree[request.worktreeId].map((tab) => - tab.id === request.tabId && tab.ptyId === request.oldPtyId - ? { ...tab, ptyId: ids[0] ?? null } - : tab - ) - } - } - } - } return common }) + if ( + !patch || + !['failed', 'deferred', 'unresolved'].includes(patch.phase ?? '') || + patch.condition?.expected.kind !== 'empty' + ) { + return + } + const state = get(), + request = currentCodexAccountRestart(state, token) + if (!request?.leafId) { + return + } + const layout = state.terminalLayoutsByTabId[request.tabId] + if (layout?.ptyIdsByLeafId?.[request.leafId] !== request.oldPtyId) { + return + } + const bindings = { ...layout.ptyIdsByLeafId } + delete bindings[request.leafId] + state.setTabLayout(request.tabId, { ...layout, ptyIdsByLeafId: bindings }) + const latest = get() + if ( + currentCodexAccountRestart(latest, token) && + !latest.terminalLayoutsByTabId[request.tabId]?.ptyIdsByLeafId?.[request.leafId] + ) { + latest.clearTabPtyId(request.tabId, request.oldPtyId) + } }, retryCodexAccountRestart: (token) => { set((s) => { diff --git a/src/renderer/src/store/terminals/codex-account-restart-continuation.ts b/src/renderer/src/store/terminals/codex-account-restart-continuation.ts index b2f601b9af0..b4c7b9bda38 100644 --- a/src/renderer/src/store/terminals/codex-account-restart-continuation.ts +++ b/src/renderer/src/store/terminals/codex-account-restart-continuation.ts @@ -1,7 +1,7 @@ import { getTerminalNoticeLifetime } from '@/components/terminal-pane/provider-notices/terminal-notice-lifetime' import { collectLeafIds } from '../../../../shared/workspace-layout/terminal-topology/terminal-pane-layout-tree' import { awaitsCodexRestartAnswer } from '@/components/codex-restart-notice-state' -import { getExecutionHostIdForWorktree } from '@/lib/workspace/ownership/worktree-runtime-owner' +import { getResourceExecutionHostIdForWorktree } from '@/lib/workspace/ownership/worktree-runtime-owner' import type { WorktreeRuntimeOwnerState } from '@/lib/workspace/ownership/worktree-runtime-owner-state' import { isTerminalLeafId, makePaneKey } from '../../../../shared/workspace-layout/stable-pane-id' import { layoutContainsLeafId } from '../../../../shared/workspace-layout/terminal-topology/terminal-pane-tree' @@ -83,7 +83,7 @@ export function isCurrentCodexAccountRestart( request: CodexAccountRestart, tabs?: ReadonlyMap ): boolean { - if (getExecutionHostIdForWorktree(state, request.worktreeId) !== request.hostId) { + if (getResourceExecutionHostIdForWorktree(state, request.worktreeId) !== request.hostId) { return false } if (request.environmentId && request.pairingRevision !== undefined) { @@ -105,7 +105,11 @@ export function isCurrentCodexAccountRestart( return false } if (!request.leafId) { - return (state.ptyIdsByTabId[request.tabId] ?? []).includes(request.oldPtyId) + const incarnation = getTerminalNoticeLifetime(request.oldPtyId)?.incarnationId + return ( + (state.ptyIdsByTabId[request.tabId] ?? []).includes(request.oldPtyId) && + (!request.oldIncarnationId || !incarnation || incarnation === request.oldIncarnationId) + ) } const layout = state.terminalLayoutsByTabId[request.tabId] if ( @@ -115,6 +119,16 @@ export function isCurrentCodexAccountRestart( return false } const current = layout.ptyIdsByLeafId?.[request.leafId] + const incarnation = current ? getTerminalNoticeLifetime(current)?.incarnationId : undefined + if ( + current === request.oldPtyId && + current !== request.resultPtyId && + request.oldIncarnationId && + incarnation && + incarnation !== request.oldIncarnationId + ) { + return false + } return !current || current === request.oldPtyId || current === request.resultPtyId } @@ -193,7 +207,7 @@ function reconcileRetiredCodexNotices( const incarnation = getTerminalNoticeLifetime(request.oldPtyId)?.incarnationId const originalStillCurrent = pane && - getExecutionHostIdForWorktree(next, pane.worktreeId) === request.hostId && + getResourceExecutionHostIdForWorktree(next, pane.worktreeId) === request.hostId && (!incarnation || !request.oldIncarnationId || incarnation === request.oldIncarnationId) if (notices === next.codexRestartNoticeByPtyId) { notices = { ...notices } diff --git a/src/renderer/src/store/terminals/pty/terminal-restart-state.ts b/src/renderer/src/store/terminals/pty/terminal-restart-state.ts index 5dab06fcc1c..223678fb77e 100644 --- a/src/renderer/src/store/terminals/pty/terminal-restart-state.ts +++ b/src/renderer/src/store/terminals/pty/terminal-restart-state.ts @@ -36,7 +36,7 @@ export function createTerminalRestartActions( | 'dismissCodexRestartNotices' | 'reopenCodexRestartPrompt' > { - const { queueCodexAccountRestarts, ...codexActions } = createCodexAccountRestartActions(set) + const { queueCodexAccountRestarts, ...codexActions } = createCodexAccountRestartActions(set, get) return { consumeSuppressedPtyExit: (ptyId) => { let wasSuppressed = false diff --git a/src/shared/terminal/pty/lifecycle/account-restart-contract.ts b/src/shared/terminal/pty/lifecycle/account-restart-contract.ts index c8d405ff5c9..c216dcbd22e 100644 --- a/src/shared/terminal/pty/lifecycle/account-restart-contract.ts +++ b/src/shared/terminal/pty/lifecycle/account-restart-contract.ts @@ -1,5 +1,7 @@ export const ACCOUNT_RESTART_CAPABILITY = 'terminal.account-restart.v1' const PREFIX = 'terminal_account_restart_v1:' +export const ACCOUNT_RESTART_OUTCOME_UNKNOWN = 'terminal_account_restart_outcome_unknown' +export const ACCOUNT_RESTART_VIEW_CHANGED = 'terminal_account_restart_view_changed' export type AccountRestartOwner = | { kind: 'endpoint'; ptyId: string; incarnationId: string; bindingGeneration: number }