Skip to content
Closed
13 changes: 11 additions & 2 deletions src/main/ipc/pty-dead-owner-respawn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { withDurableRuntimeStore } from '../runtime/runtime-durable-store-fixtur
import { setupPtyIpcSuite } from './pty-ipc-test-harness'
import { SessionNotFoundError } from '../daemon/daemon-errors'
import { makePaneKey } from '../../shared/stable-pane-id'
import { releaseTabRowPty } from '../persistence/loading-store/replaced-pane-binding'
import { registerPtyHandlers, setLocalPtyProvider } from './pty'

vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock()))
Expand Down Expand Up @@ -109,6 +110,7 @@ describe('registerPtyHandlers', () => {
session = next
}),
flushOrThrow: vi.fn(),
retirePtyBinding: vi.fn(async () => true),
persistPtyBinding: vi.fn(),
getFolderWorkspace: vi.fn(() => undefined),
getFolderWorkspaces: vi.fn(() => []),
Expand Down Expand Up @@ -171,8 +173,13 @@ describe('registerPtyHandlers', () => {
'inc-proven-absent-owner',
{ hostExitConfirmed: true }
)
expect(store.setWorkspaceSession).toHaveBeenCalledOnce()
expect(store.flushOrThrow).toHaveBeenCalledOnce()
// The pane and its binding stay for the fresh spawn's bind to swap; only the row lets go.
expect(store.setWorkspaceSession).not.toHaveBeenCalled()
expect(store.retirePtyBinding).toHaveBeenCalledWith(
expect.objectContaining({ worktreeId, tabId, leafId, ptyId: 'pty-proven-absent-owner' }),
undefined,
releaseTabRowPty
)
})
it('does not poll after the routed provider confirms absence', async () => {
const worktreeId = 'repo-1::/tmp/probe-blip-owner'
Expand Down Expand Up @@ -233,6 +240,7 @@ describe('registerPtyHandlers', () => {
session = next
}),
flushOrThrow: vi.fn(),
retirePtyBinding: vi.fn(async () => true),
persistPtyBinding: vi.fn(),
getFolderWorkspace: vi.fn(() => undefined),
getFolderWorkspaces: vi.fn(() => []),
Expand Down Expand Up @@ -357,6 +365,7 @@ describe('registerPtyHandlers', () => {
session = next
}),
flushOrThrow: vi.fn(),
retirePtyBinding: vi.fn(async () => true),
persistPtyBinding: vi.fn(),
getFolderWorkspace: vi.fn(() => undefined),
getFolderWorkspaces: vi.fn(() => []),
Expand Down
21 changes: 11 additions & 10 deletions src/main/ipc/pty-pane-reservation-settlement.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ describe('registerPtyHandlers', () => {
session = next
}),
flushOrThrow: vi.fn(),
retirePtyBinding: vi.fn(async () => true),
persistPtyBinding: vi.fn(),
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
Expand Down Expand Up @@ -184,16 +185,16 @@ describe('registerPtyHandlers', () => {
expect(remoteSpawn.mock.calls[1]?.[0]).toMatchObject({
command: 'codex resume exact-dead-ssh-provider-session'
})
expect(store.setWorkspaceSession).toHaveBeenCalledWith(expect.anything(), hostId)
expect(store.persistPtyBinding).toHaveBeenCalledWith(
expect.objectContaining({
worktreeId,
tabId,
leafId,
ptyId: freshPtyId
}),
hostId
)
expect(store.setWorkspaceSession).not.toHaveBeenCalled()
// The fresh spawn replaces the absent owner, so its bind swaps the binding naming it.
const [input, persistHostId] = store.persistPtyBinding.mock.calls[0] ?? []
expect(persistHostId).toBe(hostId)
expect(typeof input === 'function' ? input() : input).toMatchObject({
worktreeId,
tabId,
leafId,
ptyId: freshPtyId
})
} finally {
unregisterSshPtyProvider(connectionId)
}
Expand Down
8 changes: 5 additions & 3 deletions src/main/ipc/pty-persisted-incarnation-repair.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ describe('registerPtyHandlers', () => {
session = next
}),
flushOrThrow: vi.fn(),
retirePtyBinding: vi.fn(async () => true),
persistPtyBinding: vi.fn(),
getFolderWorkspace: vi.fn(() => ({
id: 'dead-persisted-owner',
Expand Down Expand Up @@ -350,7 +351,7 @@ describe('registerPtyHandlers', () => {
sessionId: 'pty-dead-persisted-owner',
command: undefined
})
expect(store.setWorkspaceSession).toHaveBeenCalledOnce()
expect(store.setWorkspaceSession).not.toHaveBeenCalled()
expect(runtime.onPtyExit).toHaveBeenCalledWith(
'pty-dead-persisted-owner',
-1,
Expand All @@ -374,8 +375,8 @@ describe('registerPtyHandlers', () => {
expect(providerSpawn.mock.calls[1]?.[0]).toMatchObject({
command: 'codex resume exact-dead-provider-session'
})
expect(store.setWorkspaceSession).toHaveBeenCalledOnce()
expect(store.flushOrThrow).toHaveBeenCalledOnce()
// The pane and its binding stay for the fresh spawn's bind to swap.
expect(store.setWorkspaceSession).not.toHaveBeenCalled()
expect(runtime.onPtyExit).toHaveBeenCalledWith(
'pty-dead-persisted-owner',
-1,
Expand Down Expand Up @@ -441,6 +442,7 @@ describe('registerPtyHandlers', () => {
session = next
}),
flushOrThrow: vi.fn(),
retirePtyBinding: vi.fn(async () => true),
persistPtyBinding: vi.fn(),
getFolderWorkspace: vi.fn(() => undefined),
getFolderWorkspaces: vi.fn(() => []),
Expand Down
2 changes: 2 additions & 0 deletions src/main/ipc/pty/ipc/spawn-preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export async function preparePtyIpcSpawnPreflight(ctx: PtyIpcSpawnState): Promis
: null
ctx.spawnTiming.mark('stable_adoption')
if (ctx.earlyStablePaneOwner && !ctx.preAdoptedStablePane) {
// The owner's process is absent; this spawn replaces it, so it is not reattached again.
ctx.replacedPaneOwner ??= { ptyId: ctx.earlyStablePaneOwner.ptyId, pane: null }
const pathUsable = ctx.deps.assertFolderWorkspacePtyPathUsable(args.worktreeId)
if (pathUsable) {
await pathUsable
Expand Down
20 changes: 11 additions & 9 deletions src/main/ipc/pty/pane/stable-owner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { ptyIncarnationById, ptyOwnership } from '../provider/ownership-state'
import { isHostReportedPtyAbsenceError, isObservedPtyExitEvidence } from '../provider/liveness'
import { clearProviderPtyState } from '../provider/state-cleanup'
import { spawnCommitBindingOrigin } from '../../../persistence/loading-store/pty-binding-span'
import { releaseTabRowPty } from '../../../persistence/loading-store/replaced-pane-binding'

export type StablePaneOwner = {
handle?: string
Expand Down Expand Up @@ -225,7 +226,7 @@ export async function persistAdmittedStablePaneBinding(args: {
export async function attachStablePaneOwner(
args: StablePaneSpawnContext & { owner: StablePaneOwner }
): Promise<{ result: PtySpawnResult; owner: StablePaneOwner } | null> {
const { owner, provider, runtime, spawnOptions } = args
const { owner, provider, runtime, spawnOptions, store, worktreeId, connectionId } = args
let result: PtySpawnResult
try {
result = await provider.spawn({
Expand Down Expand Up @@ -265,6 +266,10 @@ export async function attachStablePaneOwner(
) {
throw new Error('terminal_pane_owner_changed')
}
// Why 'replaced': the fresh spawn that follows takes the pane, so main keeps the pane and its
// binding until that spawn's bind swaps it, as for a restart.
const stops = runtime?.intentionalPtyStops
const settleReplacement = stops?.mark(owner.ptyId, 'replaced', owner.incarnationId ?? null)
// `pty.attach` answers absent both for a pid the relay probed and found gone and for an id its
// session map never had — every id minted before a relay restart, checked against nothing. Only
// the marked half observed the process, so only it may certify a death; the rest publishes the
Expand All @@ -276,16 +281,13 @@ export async function attachStablePaneOwner(
owner.incarnationId,
isObservedPtyExitEvidence(error) ? { hostExitConfirmed: true } : {}
)
settleReplacement?.(true)
clearProviderPtyState(owner.ptyId)
ptyOwnership.delete(owner.ptyId)
if (
args.worktreeId &&
!(await retirePersistedStablePaneOwner(args.store, owner, args.worktreeId, args.connectionId))
) {
throw new Error('terminal_pane_owner_changed')
}
if (args.resolveOwner?.()) {
throw new Error('terminal_pane_owner_changed')
if (worktreeId) {
// The first pane to bind takes the row, as after the window's hydration.
const hostId = connectionId ? toSshExecutionHostId(connectionId) : undefined
await store?.retirePtyBinding({ ...owner, worktreeId }, hostId, releaseTabRowPty)
}
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import { describe, expect, it, vi } from 'vitest'
import { withDurableRuntimeStore } from '../../../runtime/runtime-durable-store-fixture'
import { getDefaultWorkspaceSession } from '../../../../shared/constants'
import { makePaneKey } from '../../../../shared/stable-pane-id'
import { SSH_EXIT_UNCONFIRMED_REASON } from '../../../../shared/pty-liveness-verdict'
import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types'
import { SessionNotFoundError } from '../../../daemon/daemon-errors'
Expand All @@ -23,7 +22,7 @@ import {
} from '../../../providers/ssh-pty-errors'
import type { IPtyProvider } from '../../../providers/types'
import { OrcaRuntimeService } from '../../../runtime/orca-runtime'
import { resolvePersistedStablePaneOwner, spawnForStablePane } from './stable-owner'
import { spawnForStablePane } from './stable-owner'

const CONNECTION = 'conn-1'
const WORKTREE = 'repo-1::/tmp/pane-absence'
Expand Down Expand Up @@ -62,6 +61,7 @@ function paneStore(): { store: Store; read: () => WorkspaceSessionState } {
session = next
},
flushOrThrow: () => {},
retirePtyBinding: async () => true,
getRepos: () => [
{
id: 'repo-1',
Expand Down Expand Up @@ -137,16 +137,12 @@ describe('a stable pane whose reattach was refused', () => {
})

it('still certifies the death the relay proved with a pid probe', async () => {
const { runtime, store, spawn } = await adoptAfterAttachRefusal(
const { runtime, spawn } = await adoptAfterAttachRefusal(
new SshPtyProvenExitedOnRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty2:epoch-a:1`)
)

expect(spawn).toHaveBeenCalledTimes(2)
expect(runtime.getPtyLivenessVerdict(PTY_ID)).toEqual({ status: 'exited' })
// If nothing ever retired, a proven-dead pane would reattach to a corpse on every adoption.
expect(
resolvePersistedStablePaneOwner(store, makePaneKey(TAB, LEAF), WORKTREE, CONNECTION)
).toBeNull()
})

it('certifies an absence reported by the process registry that owns the PTY', async () => {
Expand Down
141 changes: 141 additions & 0 deletions src/main/ipc/pty/pane/stable-pane-absent-respawn-keeps-pane.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// Waking a slept split after a relaunch reattaches each pane to a process the sleep stopped. The
// host answers absent and the pane spawns fresh, so main's copy must keep the pane and its binding
// until the fresh spawn's bind swaps it: dropping the pane lets that spawn re-mint the tab and
// reorder the split, and a pane shown unbound meanwhile reads as gone to the agent resume.
import { describe, expect, it, vi } from 'vitest'
import { withDurableRuntimeStore } from '../../../runtime/runtime-durable-store-fixture'
import { getDefaultWorkspaceSession } from '../../../../shared/constants'
import type { TerminalPaneLayoutNode, TerminalTab } from '../../../../shared/terminal-tab-types'
import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types'
import { SessionNotFoundError } from '../../../daemon/daemon-errors'
import type { Store } from '../../../persistence'
import { clearReplacedPaneBinding } from '../../../persistence/loading-store/replaced-pane-binding'
import type { IPtyProvider } from '../../../providers/types'
import { OrcaRuntimeService } from '../../../runtime/orca-runtime'
import { spawnForStablePane } from './stable-owner'

const WORKTREE = 'repo-1::/tmp/pane-respawn'
const TAB = 'tab-1'
const SHELL_LEAF = '1b3f2c4d-5e6a-4b7c-8d9e-0f1a2b3c4d5e'
const AGENT_LEAF = '2c4d3e5f-6a7b-4c8d-9e0f-1a2b3c4d5e6f'
const SHELL_PTY = `${WORKTREE}@@aaaa0001`
const AGENT_PTY = `${WORKTREE}@@aaaa0002`
const SPLIT: TerminalPaneLayoutNode = {
type: 'split',
direction: 'vertical',
first: { type: 'leaf', leafId: SHELL_LEAF },
second: { type: 'leaf', leafId: AGENT_LEAF }
}

const ROW: TerminalTab = {
id: TAB,
worktreeId: WORKTREE,
ptyId: SHELL_PTY,
title: 'Terminal',
customTitle: null,
color: null,
sortOrder: 0,
createdAt: 0
}

function sleptSplit(): { store: Store; read: () => WorkspaceSessionState } {
let session: WorkspaceSessionState = {
...getDefaultWorkspaceSession(),
tabsByWorktree: { [WORKTREE]: [ROW] },
terminalLayoutsByTabId: {
[TAB]: {
root: SPLIT,
activeLeafId: AGENT_LEAF,
expandedLeafId: null,
ptyIdsByLeafId: { [SHELL_LEAF]: SHELL_PTY, [AGENT_LEAF]: AGENT_PTY }
}
}
}
return {
read: () => session,
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: This fixture supplies the persistence and catalog reads stable-pane adoption and exit bookkeeping use.
store: withDurableRuntimeStore({
getWorkspaceSession: () => session,
setWorkspaceSession: (next: WorkspaceSessionState) => {
session = next
},
// The store's host-side binding retirement, applying the change the caller names.
retirePtyBinding: async (
...[binding, , retire = clearReplacedPaneBinding]: Parameters<Store['retirePtyBinding']>
) => {
session = retire(session, { ...binding, parentTabId: binding.tabId })
return true
},
flushOrThrow: () => {},
getRepos: () => [
{
id: 'repo-1',
path: '/tmp/pane-respawn',
displayName: 'r',
badgeColor: '#000',
addedAt: 0
}
],
getAllWorktreeMeta: () => ({}),
getWorktreeMeta: () => undefined,
setWorktreeMeta: () => {},
removeWorktreeMeta: () => {},
getSettings: () => ({ workspaceDir: '/tmp/workspaces' }),
getProjects: () => []
}) as unknown as Store
}
}

/** The waking window names each pane's slept PTY, as its graph does before the reattach. */
function wakingRuntime(store: Store): OrcaRuntimeService {
const runtime = new OrcaRuntimeService(store as never)
runtime.setPtyController({
write: () => true,
kill: () => true,
hasPty: () => null,
listProcesses: async () => [],
getForegroundProcess: async () => null
} as never)
runtime.attachWindow(1)
runtime.registerPty(SHELL_PTY, WORKTREE, null, { tabId: TAB, leafId: SHELL_LEAF })
runtime.registerPty(AGENT_PTY, WORKTREE, null, { tabId: TAB, leafId: AGENT_LEAF })
return runtime
}

async function respawnOverAbsentProcess(
runtime: OrcaRuntimeService,
store: Store,
leafId: string,
ptyId: string
): Promise<void> {
const spawn = vi
.fn()
.mockRejectedValueOnce(new SessionNotFoundError(ptyId))
.mockResolvedValueOnce({ id: `${ptyId}-fresh`, isReattach: false })
const result = await spawnForStablePane({
runtime,
store,
provider: { spawn } as unknown as IPtyProvider,
spawnOptions: { cols: 80, rows: 24 },
owner: { tabId: TAB, leafId, ptyId, hasPersistedBinding: true },
worktreeId: WORKTREE,
resolveOwner: () => null
})
expect(spawn).toHaveBeenCalledTimes(2)
expect(result.owner).toBeNull()
}

describe('a stable pane respawned over an absent process', () => {
it('keeps the panes and their bindings, releasing only the row of the gone process', async () => {
const { store, read } = sleptSplit()
const before = structuredClone(read())
const runtime = wakingRuntime(store)

await respawnOverAbsentProcess(runtime, store, SHELL_LEAF, SHELL_PTY)
await respawnOverAbsentProcess(runtime, store, AGENT_LEAF, AGENT_PTY)

expect(read().terminalLayoutsByTabId).toEqual(before.terminalLayoutsByTabId)
// The row named the shell's gone process; the first pane to bind takes it.
expect(read().tabsByWorktree[WORKTREE]).toEqual([{ ...ROW, ptyId: null }])
})
})
Loading
Loading