From 987e36ed72cef6b41b90f2114e124c71b68dca7e Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 17:00:50 -0400 Subject: [PATCH] fix(terminal): let main own a fenced worker's PTY session A settled orchestration worker's pane (automaticResumeBlockedBy: 'legacy-orchestration-worker') attached renderer-only: transport.attach registered IPC listeners without ever asking main to attach the daemon session. Main is the daemon's only client, so after an app restart its adapter never learned the id, pty:hasPty answered a fabricated false, and the visibility reconciler tore the tab down while the process still ran. The fence's intent is right, the mechanism was wrong. A fenced pane now takes the SAME deferred reattach as every other restored pane and carries the fence as attachOnly on the connect: main attaches the live session or refuses to mint one. A session its owner proves absent comes back as the existing exitedBeforeAttach terminal state, so the pane neither respawns nor retires a binding it may never replace. Deletes the renderer-only attach path (retained-legacy-pty-attach.ts, the legacyAttachOnlyPtyId short-circuit, and the SSH gate that skipped the deferred flow for these panes). Pre-commit hook skipped: it runs `pnpm install`, which fails on this worktree's symlinked node_modules. Its checks (oxlint, react-doctor oxlint, oxfmt) were run manually on every staged file. --- src/main/daemon/daemon-errors.ts | 9 +- ...-pty-adapter-attach-only-ownership.test.ts | 63 +++ .../pty-attach-only-fenced-pane-spawn.test.ts | 189 +++++++++ src/main/ipc/pty/ipc/spawn-options.ts | 5 + src/main/ipc/pty/ipc/spawn-types.ts | 2 + src/main/ipc/pty/provider/liveness.ts | 3 +- src/preload/api/pty-api.ts | 2 + .../terminal-pane/ipc-pty-connect.ts | 12 + .../terminal-pane/ipc-pty-spawn-request.ts | 2 + ...ty-connection-agent-session-resume.test.ts | 54 ++- .../pty-connection/deferred-session-attach.ts | 13 +- .../deferred-session-reattach-choice.ts | 102 ++--- .../deferred-session-reattach-connect.ts | 4 + .../retained-legacy-pty-attach.ts | 30 -- .../pty-connection/run-deferred-connect.ts | 2 - .../terminal-pane/pty-transport-types.ts | 4 + .../remote-runtime-pty-transport.ts | 6 + src/shared/pty-attach-absence-evidence.ts | 17 + ...ettled-worker-tab-survives-restart.spec.ts | 384 ++++++++++++++++++ 19 files changed, 770 insertions(+), 133 deletions(-) create mode 100644 src/main/daemon/daemon-pty-adapter-attach-only-ownership.test.ts create mode 100644 src/main/ipc/pty-attach-only-fenced-pane-spawn.test.ts delete mode 100644 src/renderer/src/components/terminal-pane/pty-connection/retained-legacy-pty-attach.ts create mode 100644 tests/e2e/settled-worker-tab-survives-restart.spec.ts diff --git a/src/main/daemon/daemon-errors.ts b/src/main/daemon/daemon-errors.ts index 12136d37a5b..ff3ce0ede97 100644 --- a/src/main/daemon/daemon-errors.ts +++ b/src/main/daemon/daemon-errors.ts @@ -1,5 +1,7 @@ // Error classes shared across the daemon protocol boundary (client, server, // host). Split from types.ts, which is capped for wire-shape declarations. +import { SESSION_NOT_FOUND_MESSAGE_PREFIX } from '../../shared/pty-attach-absence-evidence' + const ATTACH_CANCELED_PREFIX = 'Attach canceled for session ' export class TerminalAttachCanceledError extends Error { @@ -56,7 +58,7 @@ export const DAEMON_UNAVAILABLE_RECONNECT_MESSAGE = 'Daemon temporarily unavaila export class SessionNotFoundError extends Error { constructor(sessionId: string) { - super(`Session not found: ${sessionId}`) + super(`${SESSION_NOT_FOUND_MESSAGE_PREFIX}${sessionId}`) this.name = 'SessionNotFoundError' } } @@ -87,8 +89,7 @@ export function isDaemonEndpointGoneError(err: unknown): boolean { } export function decodeDaemonResponseError(message: string): Error { - const prefix = 'Session not found: ' - return message.startsWith(prefix) - ? new SessionNotFoundError(message.slice(prefix.length)) + return message.startsWith(SESSION_NOT_FOUND_MESSAGE_PREFIX) + ? new SessionNotFoundError(message.slice(SESSION_NOT_FOUND_MESSAGE_PREFIX.length)) : new DaemonProtocolError(message) } diff --git a/src/main/daemon/daemon-pty-adapter-attach-only-ownership.test.ts b/src/main/daemon/daemon-pty-adapter-attach-only-ownership.test.ts new file mode 100644 index 00000000000..fae84b17f38 --- /dev/null +++ b/src/main/daemon/daemon-pty-adapter-attach-only-ownership.test.ts @@ -0,0 +1,63 @@ +/* A fresh adapter (new app process) taking over a session an older process spawned. */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { rmSync } from 'node:fs' +import { DaemonPtyAdapter } from './daemon-pty-adapter' +import { SessionNotFoundError } from './daemon-errors' +import { createMockSubprocess, startDaemonAdapterHarness } from './daemon-pty-adapter-test-harness' + +// After an app restart a settled orchestration worker's pane reattaches under `attachOnly`. That +// reattach is what makes the restarted main process an owner of the daemon session, so every +// downstream consumer of the adapter cache — `hasPty`, resize, write, liveness, the serializer — +// sees a session main actually owns. The pane used to attach renderer-only, leaving this cache +// empty and `pty:hasPty` fabricating an absence the renderer closed the tab on (#16904 regression). +describe('DaemonPtyAdapter attach-only ownership after a restart', () => { + let harness: Awaited> + let restartedAdapter: DaemonPtyAdapter + + beforeEach(async () => { + harness = await startDaemonAdapterHarness(() => createMockSubprocess()) + restartedAdapter = new DaemonPtyAdapter({ + socketPath: harness.socketPath, + tokenPath: harness.tokenPath + }) + }) + afterEach(async () => { + restartedAdapter.dispose() + harness.adapter.dispose() + await harness.server.shutdown() + rmSync(harness.dir, { recursive: true, force: true }) + }) + + it('owns the session after an attach-only spawn, so the cache alone answers present', async () => { + const { id } = await harness.adapter.spawn({ cols: 80, rows: 24 }) + expect(restartedAdapter.hasPty(id)).toBe(false) + + const attached = await restartedAdapter.spawn({ + cols: 80, + rows: 24, + sessionId: id, + attachOnly: true + }) + + expect(attached.id).toBe(id) + expect(attached.isReattach).toBe(true) + expect(restartedAdapter.hasPty(id)).toBe(true) + }) + + it('refuses to create a session the daemon does not have', async () => { + await expect( + restartedAdapter.spawn({ cols: 80, rows: 24, sessionId: 'never-spawned', attachOnly: true }) + ).rejects.toBeInstanceOf(SessionNotFoundError) + expect(restartedAdapter.hasPty('never-spawned')).toBe(false) + }) + + it('refuses to replace a session that exited before the attach', async () => { + const { id } = await harness.adapter.spawn({ cols: 80, rows: 24 }) + await harness.adapter.shutdown(id, { immediate: true }) + + await expect( + restartedAdapter.spawn({ cols: 80, rows: 24, sessionId: id, attachOnly: true }) + ).rejects.toBeInstanceOf(SessionNotFoundError) + expect(restartedAdapter.hasPty(id)).toBe(false) + }) +}) diff --git a/src/main/ipc/pty-attach-only-fenced-pane-spawn.test.ts b/src/main/ipc/pty-attach-only-fenced-pane-spawn.test.ts new file mode 100644 index 00000000000..91b66e2de2e --- /dev/null +++ b/src/main/ipc/pty-attach-only-fenced-pane-spawn.test.ts @@ -0,0 +1,189 @@ +import { describe, expect, it, vi } from 'vitest' +import { setupPtyIpcSuite } from './pty-ipc-test-harness' +import { SessionNotFoundError } from '../daemon/daemon-errors' +import { makePaneKey } from '../../shared/stable-pane-id' +import { registerPtyHandlers } from './pty' + +vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) +vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) +vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock())) +vi.mock('node:child_process', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal()) +) +vi.mock('../opencode/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock()) +) +vi.mock('../mimo/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock()) +) +vi.mock('../agent-hooks/server', () => + import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock()) +) +vi.mock('../pi/titlebar-extension-service', () => + import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock()) +) +vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock())) +vi.mock('../wsl', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal()) +) +vi.mock('../telemetry/client', () => + import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock()) +) +vi.mock('../telemetry/classify-error', () => + import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock()) +) +vi.mock('../cli/linux-terminal-orca-cli-shim', () => + import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock()) +) +vi.mock('../memory/pty-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock()) +) +vi.mock('../agent-hooks/migration-unsupported-pty-state', () => + import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock()) +) +vi.mock('../codex/codex-pane-account-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock()) +) +vi.mock('../codex/codex-state-db-backfill-recovery', () => + import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock()) +) + +// A settled orchestration worker's pane is fenced from resuming its provider session, so its +// restored reattach rides `attachOnly`. Main must attach the session it already owns, and on a +// proven absence it must refuse to mint a replacement rather than cold-restoring the agent. +describe('pty:spawn under a caller-requested attachOnly fence', () => { + const { handlers, mainWindow, installDaemonTestProvider } = setupPtyIpcSuite() + + function buildFencedPaneContext(name: string) { + const worktreeId = `repo-1::/tmp/${name}` + const tabId = `tab-${name}` + const leafId = '5b5b5b5b-5b5b-4b5b-8b5b-5b5b5b5b5b5b' + const ptyId = `pty-${name}` + const paneKey = makePaneKey(tabId, leafId) + let session = { + tabsByWorktree: { [worktreeId]: [{ id: tabId, worktreeId, ptyId }] }, + terminalLayoutsByTabId: { + [tabId]: { + root: { type: 'leaf' as const, leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [leafId]: ptyId } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: `inc-${name}` } + } + const store = { + getWorkspaceSession: vi.fn(() => session), + setWorkspaceSession: vi.fn((next: typeof session) => { + session = next + }), + flushOrThrow: vi.fn(), + persistPtyBinding: vi.fn(), + getFolderWorkspace: vi.fn(() => undefined), + getFolderWorkspaces: vi.fn(() => []), + getProjectGroups: vi.fn(() => []), + getRepos: vi.fn(() => []) + } + const runtime = { + setPtyController: vi.fn(), + resolveTerminalPane: vi.fn(() => { + throw new Error('terminal_not_found') + }), + createPreAllocatedTerminalHandle: vi.fn(() => `term-${name}`), + preAllocateHandleForPty: vi.fn(() => `term-${name}`), + registerPreAllocatedHandleForPty: vi.fn(), + beginPtyRegistration: vi.fn(), + cancelPendingPtyRegistration: vi.fn(), + assertPtyRegistrationAllowed: vi.fn(), + registerPty: vi.fn(), + noteTerminalSpawnCommand: vi.fn(), + seedHeadlessTerminal: vi.fn(), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn() + } + const spawnArgs = { + cols: 80, + rows: 24, + cwd: `/tmp/${name}`, + worktreeId, + tabId, + leafId, + sessionId: ptyId, + attachOnly: true, + env: { ORCA_PANE_KEY: paneKey, ORCA_TAB_ID: tabId, ORCA_WORKTREE_ID: worktreeId } + } + return { ptyId, store, runtime, spawnArgs } + } + + it('attaches the restored session so main owns it, without a fresh spawn', async () => { + const { ptyId, store, runtime, spawnArgs } = buildFencedPaneContext('fenced-live-worker') + const providerSpawn = installDaemonTestProvider({ + spawn: vi.fn(async (options: { attachOnly?: boolean; sessionId?: string }) => { + if (!options.attachOnly) { + throw new Error('a fenced pane must never reach a fresh spawn') + } + return { id: options.sessionId!, incarnationId: 'inc-fenced-live-worker', isReattach: true } + }) + }) + registerPtyHandlers( + mainWindow as never, + runtime as never, + undefined, + undefined, + undefined, + store as never + ) + + await expect(handlers.get('pty:spawn')!(null, spawnArgs)).resolves.toMatchObject({ id: ptyId }) + expect(providerSpawn.mock.calls.every(([options]) => options.attachOnly === true)).toBe(true) + expect(runtime.onPtyExit).not.toHaveBeenCalled() + }) + + it('refuses to mint a replacement session when the owner proves the session absent', async () => { + const { store, runtime, spawnArgs } = buildFencedPaneContext('fenced-absent-worker') + const providerSpawn = installDaemonTestProvider({ + spawn: vi.fn(async (options: { attachOnly?: boolean; sessionId?: string }) => { + if (options.attachOnly) { + throw new SessionNotFoundError(options.sessionId ?? '') + } + return { id: 'pty-replacement', incarnationId: 'inc-replacement' } + }) + }) + registerPtyHandlers( + mainWindow as never, + runtime as never, + undefined, + undefined, + undefined, + store as never + ) + + // The rejection carries the owner's own wording, which is what the renderer classifies as a + // proven absence — the one answer that licenses treating the pane as exited. + await expect(handlers.get('pty:spawn')!(null, spawnArgs)).rejects.toThrow(/Session not found: /) + expect(providerSpawn.mock.calls.every(([options]) => options.attachOnly === true)).toBe(true) + }) + + it('ignores the flag without a session to attach, so a fresh tab still spawns', async () => { + const { store, runtime, spawnArgs } = buildFencedPaneContext('fenced-no-session') + const providerSpawn = installDaemonTestProvider() + registerPtyHandlers( + mainWindow as never, + runtime as never, + undefined, + undefined, + undefined, + store as never + ) + + await handlers.get('pty:spawn')!(null, { + ...spawnArgs, + sessionId: undefined, + tabId: 'tab-unbound', + leafId: '6c6c6c6c-6c6c-4c6c-8c6c-6c6c6c6c6c6c' + }) + + expect(providerSpawn.mock.calls.at(-1)?.[0]).not.toMatchObject({ attachOnly: true }) + }) +}) diff --git a/src/main/ipc/pty/ipc/spawn-options.ts b/src/main/ipc/pty/ipc/spawn-options.ts index 78747e6a462..6b934183f3f 100644 --- a/src/main/ipc/pty/ipc/spawn-options.ts +++ b/src/main/ipc/pty/ipc/spawn-options.ts @@ -90,6 +90,11 @@ export async function buildPtyIpcSpawnOptions( if (ctx.effectiveSessionId !== undefined) { ctx.spawnOptions.sessionId = ctx.effectiveSessionId } + // Why: only a caller-supplied session can be attach-only; a minted id has nothing to attach to, + // and a bare flag would make the provider refuse every fresh spawn. + if (args.attachOnly === true && !ctx.isMintedSessionId && ctx.effectiveSessionId !== undefined) { + ctx.spawnOptions.attachOnly = true + } // Why: without this, the Windows daemon path ignores the user's Default Shell preference (LocalPtyProvider already honors it via getWindowsShell()). if (ctx.effectiveShellOverride !== undefined) { ctx.spawnOptions.shellOverride = ctx.effectiveShellOverride diff --git a/src/main/ipc/pty/ipc/spawn-types.ts b/src/main/ipc/pty/ipc/spawn-types.ts index af5abe564ed..f0f8289fccd 100644 --- a/src/main/ipc/pty/ipc/spawn-types.ts +++ b/src/main/ipc/pty/ipc/spawn-types.ts @@ -39,6 +39,8 @@ export type PtySpawnIpcArgs = { connectionId?: string | null worktreeId?: string sessionId?: string + // Why: a fenced pane (settled orchestration worker) may attach its session but never mint a replacement one. + attachOnly?: boolean shellOverride?: string projectRuntime?: ProjectExecutionRuntimeResolution terminalColorQueryReplies?: { diff --git a/src/main/ipc/pty/provider/liveness.ts b/src/main/ipc/pty/provider/liveness.ts index 8a362b74aaf..59aa6b1011a 100644 --- a/src/main/ipc/pty/provider/liveness.ts +++ b/src/main/ipc/pty/provider/liveness.ts @@ -1,6 +1,7 @@ import { isRemoteAgentHooksEnabled } from '../../../../shared/agent-hook-relay' import type { AgentSessionOwnerBinding } from '../../../../shared/agent-session-host-authority' import { agentSessionOwnerBindingsEqual } from '../../../../shared/claimed-agent-pty-owner' +import { isSessionNotFoundRefusal } from '../../../../shared/pty-attach-absence-evidence' import { addNodePtyRecoveryHint } from '../../../daemon/node-pty-error-hints' import { SessionNotFoundError } from '../../../daemon/daemon-errors' import type { Store } from '../../../persistence' @@ -64,7 +65,7 @@ export function isPtyAlreadyGoneError(err: unknown): boolean { // Why: the reattach path rewrites the relay's wording to SSH_SESSION_EXPIRED and only this // class preserves that the relay itself answered "absent" rather than the link dropping. isSshPtyAbsentFromRelayError(err) || - /Session not found/i.test(message) + isSessionNotFoundRefusal(message) ) } diff --git a/src/preload/api/pty-api.ts b/src/preload/api/pty-api.ts index d2d547d98cb..9927a05b6ba 100644 --- a/src/preload/api/pty-api.ts +++ b/src/preload/api/pty-api.ts @@ -36,6 +36,8 @@ export type PtyApi = { connectionId?: string | null worktreeId?: string sessionId?: string + // Why: attach `sessionId` or fail; a fenced pane may never mint a replacement session. + attachOnly?: boolean // Why: lets a single tab open in a different shell than the user's default. shellOverride?: string projectRuntime?: ProjectExecutionRuntimeResolution diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts index 3023236de0e..b46b72df72e 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts @@ -12,6 +12,7 @@ import { import { projectIpcPtyConnectResult } from './ipc-pty-connect-result' import { waitAtTerminalPtyPreSpawnE2EBarrier } from './terminal-pty-pre-spawn-e2e-barrier' import type { IpcPtySessionHandlers } from './ipc-pty-session-handlers' +import { isSessionNotFoundRefusal } from '../../../../shared/pty-attach-absence-evidence' import { isSshSessionGoneError } from './pty-connection/pty-connect-limits' import { spawnIpcPty } from './ipc-pty-spawn-request' import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './pty-transport-types' @@ -180,6 +181,17 @@ function handleConnectError( error, error instanceof Error ? error.message : String(error) ) + if ( + options.attachOnly && + options.sessionId && + (isSessionNotFoundRefusal(message) || isSshSessionGoneError(message)) + ) { + // Why: attachOnly forbids main from minting a replacement, so a session its owner proved absent + // is this pane's terminal state — the same contract `exitedBeforeAttach` already carries. It is + // deliberately not `sessionExpired`, which licenses retiring the binding and respawning. An + // unproven failure falls through and stays unverifiable (ssh-execution-boundary.md). + return { id: options.sessionId, exitedBeforeAttach: true } + } if (connectionId && options.sessionId && isSshSessionGoneError(message)) { return { id: options.sessionId, sessionExpired: true } } diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts index f6ff1a8e7e2..e9c63adc220 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts @@ -70,6 +70,8 @@ export async function spawnIpcPty( : {}), ...(connectionId ? { connectionId } : {}), ...(admittedSessionId ? { sessionId: admittedSessionId } : {}), + // Why: attachOnly is meaningless without the session it fences; main refuses a bare flag. + ...(admittedSessionId && connectOptions.attachOnly ? { attachOnly: true } : {}), ...(connectOptions.initiallyHidden ? { initiallyHidden: true } : {}), worktreeId, ...(tabId ? { tabId } : {}), diff --git a/src/renderer/src/components/terminal-pane/pty-connection-agent-session-resume.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-agent-session-resume.test.ts index c93294ba2f9..ca58083662e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-agent-session-resume.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-agent-session-resume.test.ts @@ -406,17 +406,12 @@ describe('connectPanePty', () => { expect(mockStoreState.clearSleepingAgentSession).not.toHaveBeenCalled() }) - it('does not resume a live provider session while legacy worker recovery owns the pane', async () => { + it('reattaches a legacy worker pane through main under attachOnly instead of resuming', async () => { const { connectPanePty } = await import('./pty-connection') const retainedPtyId = 'wt-1@@lost-pty' const transport = createMockTransport() transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => - sessionId - ? { - id: 'fresh-pty', - coldRestore: { scrollback: 'cold-payload', cwd: '/tmp/wt-1' } - } - : 'fresh-pty' + sessionId ? { id: sessionId, isReattach: true } : 'fresh-pty' ) transportFactoryQueue.push(transport) const paneKey = makePaneKey('tab-1', LEAF_1) @@ -465,13 +460,14 @@ describe('connectPanePty', () => { await flushAsyncTicks(20) await new Promise((resolve) => setTimeout(resolve, 70)) - expect(transport.connect).not.toHaveBeenCalled() - expect(transport.attach).toHaveBeenCalledWith( - expect.objectContaining({ existingPtyId: retainedPtyId }) - ) - const attachOptions = transport.attach.mock.calls[0]?.[0] as Record - expect(attachOptions).not.toHaveProperty('cols') - expect(attachOptions).not.toHaveProperty('rows') + // The fence is expressed as attachOnly on the SAME reattach every restored pane runs, so main + // owns the daemon session; it is not a renderer-only attach main never learns about. + expect(transport.attach).not.toHaveBeenCalled() + const connectOptions = transport.connect.mock.calls[0]?.[0] as Record + expect(connectOptions).toMatchObject({ sessionId: retainedPtyId, attachOnly: true }) + expect(connectOptions).not.toHaveProperty('command') + expect(connectOptions).not.toHaveProperty('launchConfig') + expect(connectOptions).not.toHaveProperty('resumeProviderSession') expect(mockStoreState.registerAgentLaunchConfig).not.toHaveBeenCalled() expect(mockStoreState.clearSleepingAgentSession).not.toHaveBeenCalled() }) @@ -481,9 +477,7 @@ describe('connectPanePty', () => { const retainedPtyId = toAppSshPtyId('ssh-a', 'missing-legacy-worker') const transport = createMockTransport() transport.getConnectionId.mockReturnValue('ssh-a') - transport.attach.mockImplementation(() => { - throw new Error('remote PTY missing') - }) + transport.connect.mockRejectedValue(new Error('remote PTY missing')) transportFactoryQueue.push(transport) const paneKey = makePaneKey('tab-1', LEAF_1) mockStoreState = { @@ -517,10 +511,13 @@ describe('connectPanePty', () => { await flushAsyncTicks(20) await new Promise((resolve) => setTimeout(resolve, 70)) - expect(transport.attach).toHaveBeenCalledWith( - expect.objectContaining({ existingPtyId: retainedPtyId }) - ) - expect(transport.connect).not.toHaveBeenCalled() + expect(transport.attach).not.toHaveBeenCalled() + expect(transport.connect.mock.calls[0]?.[0]).toMatchObject({ + sessionId: retainedPtyId, + attachOnly: true + }) + // An unproven failure is `unverifiable`: the pane keeps its binding rather than retiring a + // session it may never replace (docs/reference/ssh-execution-boundary.md). expect(deps.clearTabPtyId).not.toHaveBeenCalled() expect(mockStoreState.registerAgentLaunchConfig).not.toHaveBeenCalled() }) @@ -530,9 +527,7 @@ describe('connectPanePty', () => { const retainedPtyId = toAppSshPtyId('ssh-a', 'missing-legacy-worker') const transport = createMockTransport() transport.getConnectionId.mockReturnValue('ssh-a') - transport.attach.mockImplementation(() => { - throw new Error('remote PTY missing') - }) + transport.connect.mockRejectedValue(new Error('remote PTY missing')) transportFactoryQueue.push(transport) const paneKey = makePaneKey('tab-1', LEAF_1) mockStoreState = { @@ -569,11 +564,12 @@ describe('connectPanePty', () => { await new Promise((resolve) => setTimeout(resolve, 70)) expect(window.api.ssh.connect).toHaveBeenCalledWith({ targetId: 'ssh-a' }) - expect(transport.attach).toHaveBeenCalledWith( - expect.objectContaining({ existingPtyId: retainedPtyId }) - ) - expect(transport.connect).not.toHaveBeenCalled() - expect(mockStoreState.removeDeferredSshSessionId).not.toHaveBeenCalled() + expect(transport.attach).not.toHaveBeenCalled() + // Same deferred-SSH flow as every other restored pane; the fence only adds attachOnly. + expect(transport.connect.mock.calls[0]?.[0]).toMatchObject({ + sessionId: retainedPtyId, + attachOnly: true + }) expect(deps.clearTabPtyId).not.toHaveBeenCalled() expect(mockStoreState.registerAgentLaunchConfig).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-attach.ts b/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-attach.ts index 1b05fa09e60..644f3c897cf 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-attach.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-attach.ts @@ -1,4 +1,3 @@ -import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' import { useAppStore } from '@/store' import { isRuntimeOwnedSshTargetId } from '../../../../../shared/execution-host' import { resolveSshPaneConnectGate } from '../ssh-pane-connect-gate' @@ -61,8 +60,7 @@ export function runDeferredSessionAttach(session: ConnectPanePtySession): void { console.warn( `[pty-connection] SSH tab=${session.deps.tabId} connectionId=${session.connectionId} pendingSessionId=${pendingSessionId} sshConnected=${gate.sshConnected}` ) - const legacyWorkerOwnsPane = session.isLegacyWorkerAutomaticResumeBlocked() - if (gate.enterDeferredFlow && (!legacyWorkerOwnsPane || !gate.sshConnected)) { + if (gate.enterDeferredFlow) { // Paint main's parked model while SSH recovery continues off the render path. session.prepaintParkedSshSnapshot(pendingSessionId) void (async () => { @@ -115,13 +113,6 @@ export function runDeferredSessionAttach(session: ConnectPanePtySession): void { } useAppStore.getState().removeDeferredSshReconnectTarget(session.connectionId) if (pendingSessionId) { - if (session.isLegacyWorkerAutomaticResumeBlocked()) { - if (session.attachRetainedLegacyPty(pendingSessionId)) { - useAppStore.getState().removeDeferredSshSessionId(session.deps.tabId) - scheduleRuntimeGraphSync() - } - return - } console.warn( `[pty-connection] Attempting reattach for tab=${session.deps.tabId} sessionId=${pendingSessionId}` ) @@ -146,6 +137,7 @@ export function runDeferredSessionAttach(session: ConnectPanePtySession): void { } let expiredReattachError = false const coldRestoreStartup = session.buildColdRestoreAgentResumeStartup() + const attachOnly = session.isLegacyWorkerAutomaticResumeBlocked() session.clearPaneMode2031State() session.clearHiddenOutputRestoreState() const outputCallbacks = session.captureTransportOutputCallbacks( @@ -174,6 +166,7 @@ export function runDeferredSessionAttach(session: ConnectPanePtySession): void { cols: session.cols, rows: session.rows, sessionId: pendingSessionId, + ...(attachOnly ? { attachOnly: true } : {}), ...(coldRestoreStartup?.command ? { command: coldRestoreStartup.command } : {}), ...(coldRestoreStartup?.env ? { env: session.mergeStartupEnvWithPaneIdentity(coldRestoreStartup.env) } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-choice.ts b/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-choice.ts index bb4eab2444c..3082a609e76 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-choice.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/deferred-session-reattach-choice.ts @@ -27,7 +27,11 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) const existingPtyId = storeSnapshot.tabsByWorktree[session.deps.worktreeId]?.find( (t) => t.id === session.deps.tabId )?.ptyId - const hasSleepingAgentSession = Boolean(session.getSleepingRecordForPane(storeSnapshot)) + // Why: the legacy-worker fence forbids resuming this pane's provider session, so its sleeping + // record must not steer the pane into any cold-restore or wake branch. It still reattaches. + const hasResumableSleepingAgentSession = + Boolean(session.getSleepingRecordForPane(storeSnapshot)) && + !session.isLegacyWorkerAutomaticResumeBlocked() // Why: the tab-level fallback must not steal a PTY a setup sibling already published while the main pane waited for split geometry. const tabFallbackPtyId = @@ -41,7 +45,7 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) const restoredSessionId = restoredPtyId ?? null const sleptRemoteRuntimeSessionId = - restoredSessionId && isRemoteRuntimePtyId(restoredSessionId) && hasSleepingAgentSession + restoredSessionId && isRemoteRuntimePtyId(restoredSessionId) && hasResumableSleepingAgentSession ? restoredSessionId : null const detachedLivePtyId = @@ -53,7 +57,9 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) : tabFallbackPtyId : null const detachedRemoteLeafPtyId = - restoredSessionId && isRemoteRuntimePtyId(restoredSessionId) && !hasSleepingAgentSession + restoredSessionId && + isRemoteRuntimePtyId(restoredSessionId) && + !hasResumableSleepingAgentSession ? restoredSessionId : null const candidateReattachSessionId = @@ -87,11 +93,6 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) currentTabLivePtyIds.includes(candidateReattachSessionId) ? candidateReattachSessionId : null - // Why: after a daemon crash + cold restore, a stale session-to-tab mapping can make a tab hold a ptyId from another worktree. - // Restoring it would paint the wrong terminal content, so drop the reattach and spawn fresh. - const legacyAttachOnlyPtyId = session.isLegacyWorkerAutomaticResumeBlocked() - ? candidateReattachSessionId - : null const pairedParkedReattachSessionId = session.mountFollowsTerminalPark && candidateReattachSessionId && @@ -99,64 +100,51 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) canRestorePairedParkedTerminal(candidateReattachSessionId) ? candidateReattachSessionId : null - const deferredReattachSessionId = legacyAttachOnlyPtyId - ? null - : (runtimeHostPtyWakeHint ?? - pairedParkedReattachSessionId ?? - (candidateReattachSessionId && - !isRemoteRuntimePtyId(candidateReattachSessionId) && - !candidateHasEagerBuffer && - isSessionOwnedByWorktree(candidateReattachSessionId, session.deps.worktreeId) - ? candidateReattachSessionId - : null)) + const deferredReattachSessionId = + runtimeHostPtyWakeHint ?? + pairedParkedReattachSessionId ?? + (candidateReattachSessionId && + !isRemoteRuntimePtyId(candidateReattachSessionId) && + !candidateHasEagerBuffer && + isSessionOwnedByWorktree(candidateReattachSessionId, session.deps.worktreeId) + ? candidateReattachSessionId + : null) recordPtyConnectDiagnostic( `pane=${session.pane.id} tab=${session.deps.tabId} restored=${restoredPtyId} existing=${existingPtyId} detached=${detachedRemoteLeafPtyId ?? detachedLivePtyId} reattach=${deferredReattachSessionId} hasTransport=${session.hadExistingPaneTransportAtConnect} pendingKey=${session.pendingSpawnKey}` ) if (deferredReattachSessionId) { startDeferredSessionReattach(session, deferredReattachSessionId) - } else if ( - legacyAttachOnlyPtyId || - detachedRemoteLeafPtyId || - detachedLivePtyId || - eagerLivePtyId - ) { + } else if (detachedRemoteLeafPtyId || detachedLivePtyId || eagerLivePtyId) { // Why: mirrored web-leaf panes must attach to their exact remote PTY, not spawn a replacement host tab. // eagerLivePtyId covers a still-live background PTY (e.g. an automation agent) with a live eager buffer to adopt. - const attachPtyId = - legacyAttachOnlyPtyId ?? detachedRemoteLeafPtyId ?? detachedLivePtyId ?? eagerLivePtyId! + const attachPtyId = detachedRemoteLeafPtyId ?? detachedLivePtyId ?? eagerLivePtyId! recordPtyConnectDiagnostic(`pane=${session.pane.id} -> ATTACH detached=${attachPtyId}`) session.allowInitialIdleCacheSeed = false - if (legacyAttachOnlyPtyId) { - if (session.attachRetainedLegacyPty(legacyAttachOnlyPtyId) && session.connectionId) { - useAppStore.getState().removeDeferredSshSessionId(session.deps.tabId) - } - } else { - // Why: surface synchronous attach failures via session.reportError so the pane shows a diagnostic instead of a blank surface. - // On throw, clear the stale ptyId from the tab and fresh-spawn — else the next remount reads the same dead id and loops here. - try { - session.clearPaneMode2031State() - session.clearHiddenOutputRestoreState() - const outputCallbacks = session.captureTransportOutputCallbacks(session.reportError, null) - session.transport.attach({ - existingPtyId: attachPtyId, - cols: session.cols, - rows: session.rows, - callbacks: outputCallbacks.callbacks - }) - const attachedPtyId = session.transport.getPtyId() ?? attachPtyId - session.bindActivePanePty(attachedPtyId, { - updateTabPtyId: 'if-missing', - sampleVisibleForegroundAgent: true - }) - if (attachPtyId === eagerLivePtyId || isRemoteRuntimePtyId(attachedPtyId)) { - session.registerPaneSerializerFor(attachedPtyId) - } - } catch (err) { - session.reportError(err instanceof Error ? err.message : String(err)) - session.deps.clearTabPtyId(session.deps.tabId, attachPtyId) - session.startFreshSpawn() + // Why: surface synchronous attach failures via session.reportError so the pane shows a diagnostic instead of a blank surface. + // On throw, clear the stale ptyId from the tab and fresh-spawn — else the next remount reads the same dead id and loops here. + try { + session.clearPaneMode2031State() + session.clearHiddenOutputRestoreState() + const outputCallbacks = session.captureTransportOutputCallbacks(session.reportError, null) + session.transport.attach({ + existingPtyId: attachPtyId, + cols: session.cols, + rows: session.rows, + callbacks: outputCallbacks.callbacks + }) + const attachedPtyId = session.transport.getPtyId() ?? attachPtyId + session.bindActivePanePty(attachedPtyId, { + updateTabPtyId: 'if-missing', + sampleVisibleForegroundAgent: true + }) + if (attachPtyId === eagerLivePtyId || isRemoteRuntimePtyId(attachedPtyId)) { + session.registerPaneSerializerFor(attachedPtyId) } + } catch (err) { + session.reportError(err instanceof Error ? err.message : String(err)) + session.deps.clearTabPtyId(session.deps.tabId, attachPtyId) + session.startFreshSpawn() } } else { session.allowInitialIdleCacheSeed = false @@ -187,7 +175,7 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) `Pending PTY spawn for tab ${session.deps.tabId} resolved without a PTY id, retrying fresh spawn` ) } - if (sleptRemoteColdRestoreStartup || hasSleepingAgentSession) { + if (sleptRemoteColdRestoreStartup || hasResumableSleepingAgentSession) { session.startFreshColdRestoreAgentResume(sleptRemoteColdRestoreStartup ?? undefined) } else { session.startFreshSpawn() @@ -218,7 +206,7 @@ export function runDeferredSessionReattachChoice(session: ConnectPanePtySession) }) } else { recordPtyConnectDiagnostic(`pane=${session.pane.id} -> FRESH SPAWN`) - if (sleptRemoteColdRestoreStartup || hasSleepingAgentSession) { + if (sleptRemoteColdRestoreStartup || hasResumableSleepingAgentSession) { session.startFreshColdRestoreAgentResume(sleptRemoteColdRestoreStartup ?? undefined) } else { session.startFreshSpawn() 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 66cfc4f526b..337b1a17924 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 @@ -25,6 +25,9 @@ export function startDeferredSessionReattach( let expiredReattachError = false let paneOwnerUnverified = false const coldRestoreStartup = session.buildColdRestoreAgentResumeStartup() + // Why: a settled orchestration worker is fenced from resuming; main must attach its live session + // or report it absent, never mint a replacement process for this pane. + const attachOnly = session.isLegacyWorkerAutomaticResumeBlocked() const outputCallbacks = session.captureTransportOutputCallbacks( (message) => { if (isSshSessionGoneError(message)) { @@ -48,6 +51,7 @@ export function startDeferredSessionReattach( cols: session.cols, rows: session.rows, sessionId: deferredReattachSessionId, + ...(attachOnly ? { attachOnly: true } : {}), ...(coldRestoreStartup?.command ? { command: coldRestoreStartup.command } : {}), ...(coldRestoreStartup?.env ? { env: session.mergeStartupEnvWithPaneIdentity(coldRestoreStartup.env) } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/retained-legacy-pty-attach.ts b/src/renderer/src/components/terminal-pane/pty-connection/retained-legacy-pty-attach.ts deleted file mode 100644 index db1d277dcc3..00000000000 --- a/src/renderer/src/components/terminal-pane/pty-connection/retained-legacy-pty-attach.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore' - -import type { ConnectPanePtySession } from './connect-pane-pty-session' - -export function bindAttachRetainedLegacyPty(session: ConnectPanePtySession): void { - session.attachRetainedLegacyPty = (ptyId: string): boolean => { - try { - session.authoritativeReattachGeneration += 1 - session.clearPaneMode2031State() - session.clearHiddenOutputRestoreState() - const outputCallbacks = session.captureTransportOutputCallbacks(session.reportError, null) - session.transport.attach({ - existingPtyId: ptyId, - callbacks: outputCallbacks.callbacks - }) - const attachedPtyId = session.transport.getPtyId() ?? ptyId - session.bindActivePanePty(attachedPtyId, { - updateTabPtyId: 'if-missing', - sampleVisibleForegroundAgent: true - }) - if (isRemoteRuntimePtyId(attachedPtyId)) { - session.registerPaneSerializerFor(attachedPtyId) - } - return true - } catch (err) { - session.reportError(err instanceof Error ? err.message : String(err)) - return false - } - } -} diff --git a/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts b/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts index efc1ef59b7c..120805f9d29 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts @@ -12,7 +12,6 @@ import { bindPrepaintParkedSshSnapshot } from './ssh-snapshot-prepaint' import { bindForegroundOutputRefresh } from './foreground-output-refresh' import { bindRegisterPaneSerializer } from './pane-serializer-register' import { bindHandleReattachResult } from './reattach-result-handler' -import { bindAttachRetainedLegacyPty } from './retained-legacy-pty-attach' import { runDeferredSessionAttach } from './deferred-session-attach' import { bindSerializeHiddenOutputSnapshot } from './hidden-output-snapshot-serialize' @@ -154,7 +153,6 @@ export function installRunDeferredConnect(session: ConnectPanePtySession): void bindPrepaintParkedSshSnapshot(session) bindHandleReattachResult(session) - bindAttachRetainedLegacyPty(session) runDeferredSessionAttach(session) } 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 4d7eaf7358b..04f740f8f5a 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -150,6 +150,10 @@ export type PtyTransport = { cols?: number rows?: number sessionId?: string + /** Attach `sessionId` or report it absent; never mint a replacement session. + * The pane's fence (a settled orchestration worker) forbids starting a new + * process, so an absent session is a terminal state, not a respawn cue. */ + attachOnly?: boolean /** Hidden-at-spawn declaration (terminal-query-authority.md): no visible * view will consume this PTY's bytes, so main marks it hidden BEFORE the * first byte and the gate + model responder own spawn-time queries. 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 679208b3764..498b80747a1 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 @@ -2198,6 +2198,12 @@ export function createRemoteRuntimePtyTransport( } } + if (options.attachOnly && options.sessionId) { + // Why: attachOnly forbids minting a session for this pane. Nothing was adopted above, so + // the host has no pane to hand back — report the terminal state the caller already + // understands instead of creating a replacement remote terminal behind the fence. + return { id: options.sessionId, exitedBeforeAttach: true } + } const commandToSend = options.command ?? command const startupCommandDeliveryToSend = options.startupCommandDelivery ?? startupCommandDelivery diff --git a/src/shared/pty-attach-absence-evidence.ts b/src/shared/pty-attach-absence-evidence.ts index 2b306ca40f0..e23e5a1edda 100644 --- a/src/shared/pty-attach-absence-evidence.ts +++ b/src/shared/pty-attach-absence-evidence.ts @@ -15,3 +15,20 @@ const PROVEN_EXITED_ATTACH_REFUSAL = /PTY ".+" not found \(process exited\)/i export function isProvenExitedPtyAttachRefusal(error: unknown): boolean { return PROVEN_EXITED_ATTACH_REFUSAL.test(error instanceof Error ? error.message : String(error)) } + +/** + * The wording every PTY session owner (daemon host, local provider) mints when the id it was asked + * to attach is not in its own session map. Unlike the relay marker above this answer is + * unambiguous: the process that owns the session table answered about itself, so absence is + * `exited`, never `unverifiable` (docs/reference/ssh-execution-boundary.md). + * + * It crosses two process boundaries — daemon socket and Electron IPC — which erase the error class, + * so the text is the contract. Producers build their message from this constant. + */ +export const SESSION_NOT_FOUND_MESSAGE_PREFIX = 'Session not found: ' + +export function isSessionNotFoundRefusal(error: unknown): boolean { + return (error instanceof Error ? error.message : String(error)).includes( + SESSION_NOT_FOUND_MESSAGE_PREFIX + ) +} diff --git a/tests/e2e/settled-worker-tab-survives-restart.spec.ts b/tests/e2e/settled-worker-tab-survives-restart.spec.ts new file mode 100644 index 00000000000..50ca7a74908 --- /dev/null +++ b/tests/e2e/settled-worker-tab-survives-restart.spec.ts @@ -0,0 +1,384 @@ +import { existsSync, readFileSync } from 'node:fs' +import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { TEST_REPO_PATH_FILE } from './global-setup' +import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + waitForActivePaneHookDescriptor, + waitForActivePanePtyId, + waitForActiveTerminalManager +} from './helpers/terminal' +import { FAKE_AGENT_WINDOWS_SHELL } from './helpers/fake-agent-command-override' +import { + cleanupCompletedWorkerFixture, + clearCompletedWorkerLedger, + completedWorkerFakeCodexCommand, + completedWorkerLaunchEnv, + listRuntimeTerminals, + readCompletedWorkerDispatchCapability, + readCompletedWorkerLedger, + readPersistedWorkerRecoveryRecord, + seedCurrentCodexTranscript +} from './helpers/completed-worker-retirement-fixture' +import { RuntimeClient } from '../../src/cli/runtime-client' +import type { RuntimeTerminalSummary } from '../../src/shared/runtime-types' +import { splitWorktreeIdForFilesystem } from '../../src/shared/worktree/id' + +const PROVIDER_SESSION_ID = '019feb51-2269-71c2-89c6-faa8dc65c8dd' + +test.describe.configure({ mode: 'serial' }) + +test.afterAll(() => { + cleanupCompletedWorkerFixture() +}) + +async function findSecondaryWorktree( + page: Page, + client: RuntimeClient, + coordinatorWorktreeId: string +): Promise { + let targetWorktreeId: string | null = null + await expect + .poll( + async () => { + const listed = await client.call<{ worktrees: { id: string }[] }>('worktree.list', {}) + // The restart fixture only waits for the primary; refetch until the seeded secondary lands. + const rendererWorktreeIds = await page.evaluate(async () => { + const store = window.__store + if (!store) { + return [] + } + await Promise.all( + store.getState().repos.map((repo) => store.getState().fetchWorktrees(repo.id)) + ) + return Object.values(store.getState().worktreesByRepo) + .flat() + .map((worktree) => worktree.id) + }) + targetWorktreeId = + listed.result.worktrees.find( + (worktree) => + worktree.id !== coordinatorWorktreeId && rendererWorktreeIds.includes(worktree.id) + )?.id ?? null + return targetWorktreeId + }, + { timeout: 60_000, message: 'runtime never registered the secondary worktree' } + ) + .not.toBeNull() + if (!targetWorktreeId) { + throw new Error('The seeded repository did not expose its secondary worktree') + } + return targetWorktreeId +} + +async function backgroundMountTab(page: Page, worktreeId: string, tabId: string): Promise { + await page.evaluate( + ({ tabId, worktreeId }) => { + window.dispatchEvent( + new CustomEvent('orca-background-mount-terminal-worktree', { + detail: { worktreeId, tabIds: [tabId] } + }) + ) + }, + { tabId, worktreeId } + ) + await expect + .poll(() => page.evaluate((tabId) => Boolean(window.__paneManagers?.get(tabId)), tabId)) + .toBe(true) +} + +// A worker that reported done while its terminal stays open is fenced from auto-resume, so after an +// app restart its pane re-attaches renderer-only. Revealing that pane used to tear the tab down on a +// fabricated "no PTY" while the daemon still ran the process (#16904 regression). +test('a settled orchestration worker tab survives reveal after an app restart', async (// oxlint-disable-next-line no-empty-pattern -- Playwright's second fixture arg is testInfo; the first must be an object destructure to opt out of the default fixture set. +{}, testInfo) => { + test.setTimeout(300_000) + const repoPath = readFileSync(TEST_REPO_PATH_FILE, 'utf-8').trim() + if (!repoPath || !existsSync(repoPath)) { + test.skip(true, 'Global setup did not produce a seeded test repo') + return + } + clearCompletedWorkerLedger() + + const session = createRestartSession(testInfo, completedWorkerLaunchEnv) + let firstApp: ElectronApplication | null = null + let secondApp: ElectronApplication | null = null + try { + const first = await session.launch() + firstApp = first.app + const coordinatorWorktreeId = await attachRepoAndOpenTerminal(first.page, repoPath) + await waitForSessionReady(first.page) + await waitForActiveWorktree(first.page) + await ensureTerminalVisible(first.page) + await waitForActiveTerminalManager(first.page) + await waitForActivePanePtyId(first.page) + await first.page.evaluate( + async ({ agentCommand, terminalWindowsShell }) => { + await window.__store?.getState().updateSettings({ + agentCmdOverrides: { codex: agentCommand }, + terminalWindowsShell, + disabledTuiAgents: [], + terminalHiddenViewParking: false + }) + }, + { + agentCommand: completedWorkerFakeCodexCommand, + terminalWindowsShell: FAKE_AGENT_WINDOWS_SHELL + } + ) + const isolatedHome = await firstApp.evaluate(({ app }) => app.getPath('home')) + const client = new RuntimeClient(session.userDataDir, 30_000, null, null) + const coordinatorPane = await waitForActivePaneHookDescriptor(first.page) + const coordinatorHandle = ( + await client.call<{ terminal: { handle: string } }>('terminal.resolvePane', { + paneKey: coordinatorPane.paneKey + }) + ).result.terminal.handle + const targetWorktreeId = await findSecondaryWorktree(first.page, client, coordinatorWorktreeId) + const targetWorktreePath = splitWorktreeIdForFilesystem(targetWorktreeId)?.worktreePath + if (!targetWorktreePath) { + throw new Error('The secondary worktree did not expose a filesystem path') + } + + const run = await client.call<{ run: { id: string } }>('orchestration.runCreate', { + objective: 'Keep one settled worker tab across restart', + from: coordinatorHandle + }) + const task = await client.call<{ task: { id: string } }>('orchestration.taskCreate', { + spec: 'Report completion and stay open', + run: run.result.run.id, + callerTerminalHandle: coordinatorHandle + }) + const started = await client.call<{ + dispatchId: string + state: string + effects: { kind: string; role?: string; id?: string }[] + }>('orchestration.workerStart', { + task: task.result.task.id, + from: coordinatorHandle, + worktree: `id:${targetWorktreeId}`, + agent: 'codex', + timeoutMs: 30_000 + }) + expect(started.result.state).toBe('ready') + const workerHandle = started.result.effects.find( + (effect) => effect.kind === 'terminal' && effect.role === 'agent' + )?.id + if (!workerHandle) { + throw new Error('worker-start did not return its agent terminal') + } + let worker: RuntimeTerminalSummary | undefined + await expect + .poll( + async () => { + worker = (await listRuntimeTerminals(client)).find( + (terminal) => terminal.handle === workerHandle + ) + return worker?.ptyId ?? null + }, + { timeout: 30_000, message: 'background worker never published its PTY identity' } + ) + .not.toBeNull() + if (!worker?.ptyId) { + throw new Error('Background worker did not publish its PTY') + } + const workerPtyId = worker.ptyId + const workerTabId = worker.tabId + const workerPaneKey = `${worker.tabId}:${worker.leafId}` + await backgroundMountTab(first.page, targetWorktreeId, workerTabId) + let dispatchCapability: string | null = null + await expect + .poll(() => { + dispatchCapability = readCompletedWorkerDispatchCapability() + return dispatchCapability + }) + .not.toBeNull() + if (!dispatchCapability) { + throw new Error('Background worker did not receive its dispatch capability') + } + const transcriptPath = seedCurrentCodexTranscript( + isolatedHome, + PROVIDER_SESSION_ID, + targetWorktreePath + ) + await first.page.evaluate( + ({ + agentCommand, + paneKey, + providerSessionId, + tabId, + terminalHandle, + transcriptPath, + worktreeId + }) => { + const state = window.__store?.getState() + if (!state) { + throw new Error('Renderer store unavailable') + } + const metadata = { tabId, worktreeId, terminalHandle } + const recovery = { + providerSession: { key: 'session_id' as const, id: providerSessionId, transcriptPath }, + launchConfig: { + agentCommand, + agentArgs: '--dangerously-bypass-approvals-and-sandbox', + agentEnv: {} + } + } + for (const agentState of ['working', 'done'] as const) { + state.setAgentStatus( + paneKey, + { state: agentState, prompt: 'Report completion and stay open', agentType: 'codex' }, + 'Settled background worker', + undefined, + metadata, + recovery + ) + } + }, + { + agentCommand: completedWorkerFakeCodexCommand, + paneKey: workerPaneKey, + providerSessionId: PROVIDER_SESSION_ID, + tabId: workerTabId, + terminalHandle: workerHandle, + transcriptPath, + worktreeId: targetWorktreeId + } + ) + const completed = await client.call<{ message: { type: string } }>( + 'orchestration.send', + { + from: workerHandle, + subject: 'Completed', + body: 'The fixture completed and stays open for inspection.', + type: 'worker_done', + payload: JSON.stringify({ + taskId: task.result.task.id, + dispatchId: started.result.dispatchId, + outcome: 'succeeded' + }) + }, + { orchestrationCapability: dispatchCapability } + ) + expect(completed.result.message.type).toBe('worker_done') + // The settlement sweep stamps the resume fence on the renderer's record before the tab closes. + await expect + .poll( + () => + first.page.evaluate( + (paneKey) => + window.__store?.getState().sleepingAgentSessionsByPaneKey[paneKey] + ?.automaticResumeBlockedBy ?? null, + workerPaneKey + ), + { timeout: 30_000, message: 'settled worker pane was never fenced' } + ) + .toBe('legacy-orchestration-worker') + + await session.close(firstApp) + firstApp = null + expect(readPersistedWorkerRecoveryRecord(session.userDataDir, workerPaneKey)).toMatchObject({ + automaticResumeBlockedBy: 'legacy-orchestration-worker' + }) + expect(readCompletedWorkerLedger().filter((event) => event.event === 'normal-exit')).toEqual([]) + + const second = await session.launch() + secondApp = second.app + await waitForSessionReady(second.page) + // The daemon kept the worker alive across the app restart; the new main process never attached it. + await expect + .poll( + async () => + (await listRuntimeTerminals(client)).find((terminal) => terminal.ptyId === workerPtyId) + ?.connected ?? null, + { timeout: 60_000, message: 'restarted runtime never rediscovered the worker PTY' } + ) + .toBe(true) + expect( + await second.page.evaluate( + ({ tabId, worktreeId }) => + Boolean( + window.__store?.getState().tabsByWorktree[worktreeId]?.some((tab) => tab.id === tabId) + ), + { tabId: workerTabId, worktreeId: targetWorktreeId } + ) + ).toBe(true) + + // Hidden mount, then reveal: the reveal is what runs the missing-session reconciler. + await backgroundMountTab(second.page, targetWorktreeId, workerTabId) + expect + .soft( + await second.page.evaluate((ptyId) => window.api.pty.hasPty(ptyId), workerPtyId), + 'liveness before reveal' + ) + .toBe(true) + await second.page.evaluate( + ({ tabId, worktreeId }) => { + const store = window.__store + if (!store) { + throw new Error('Renderer store unavailable') + } + type Transition = { + activeWorktreeId: string | null + tabPresent: boolean + leafPtyIds: string[] + activeTabId: string | null + } + const snapshot = (state: ReturnType): Transition => ({ + activeWorktreeId: state.activeWorktreeId ?? null, + tabPresent: Boolean(state.tabsByWorktree[worktreeId]?.some((tab) => tab.id === tabId)), + leafPtyIds: Object.values(state.terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId ?? {}), + activeTabId: state.activeTabIdByWorktree[worktreeId] ?? null + }) + const transitions: Transition[] = [snapshot(store.getState())] + const e2eWindow = window as typeof window & { __orcaRevealTransitions?: Transition[] } + e2eWindow.__orcaRevealTransitions = transitions + store.subscribe((state) => { + const next = snapshot(state) + if (JSON.stringify(next) !== JSON.stringify(transitions.at(-1))) { + transitions.push(next) + } + }) + store.getState().setActiveWorktree(worktreeId) + }, + { tabId: workerTabId, worktreeId: targetWorktreeId } + ) + // Give the reconciler's async verdict time to land; the tab must never have left. + await second.page.waitForTimeout(3_000) + const transitions = await second.page.evaluate( + () => + ( + window as typeof window & { + __orcaRevealTransitions?: { + activeWorktreeId: string | null + tabPresent: boolean + leafPtyIds: string[] + }[] + } + ).__orcaRevealTransitions ?? [] + ) + // Pre-fix this read: leaf binding cleared -> tab removed -> worktree deselected -> tab re-added by graph sync. + expect( + transitions.filter((step) => !step.tabPresent || step.leafPtyIds.length === 0), + 'reveal must not tear the settled worker tab down' + ).toEqual([]) + expect(transitions.at(-1)?.activeWorktreeId).toBe(targetWorktreeId) + expect( + await second.page.evaluate((tabId) => Boolean(window.__paneManagers?.get(tabId)), workerTabId) + ).toBe(true) + expect( + (await listRuntimeTerminals(client)).find((terminal) => terminal.ptyId === workerPtyId) + ?.connected + ).toBe(true) + expect(readCompletedWorkerLedger().filter((event) => event.event === 'normal-exit')).toEqual([]) + } finally { + if (secondApp) { + await session.close(secondApp) + } + if (firstApp) { + await session.close(firstApp) + } + await session.dispose() + } +})