mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +00:00
fix(terminal): re-bind a host-exit-settled remote pane on the host's next snapshot (#33062)
* fix(terminal): hand a host-exit-settled remote pane back when the host binds a new process After #31968 settles a host-session pane on the host's exit record, a host restart can bind the new process just after the probe read the old one's exit. The pane then stayed ended until the tab was reopened. Keep watching the host's accepted snapshots after that settle; once one publishes a provably different live process for the pane, remount the tab so it attaches to it. * fix(terminal): let a new host process reopen recovery after a failed rebind * fix(terminal): watch for a host rebind only on a leaf-addressed pane * fix(terminal): catch a host rebind published while the exit probe confirms * fix(terminal): keep the host-rebind remount under the recovery window cap
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { useAppStore } from '@/store'
|
||||
import { shouldRetainDisposedPaneSpawn } from './disposed-spawn-retention'
|
||||
import { requestTerminalPaneRecovery } from '../terminal-pane-recovery'
|
||||
import type { IpcPtyTransportOptions } from '../pty-transport-types'
|
||||
import type { ConnectPanePtySession } from './connect-pane-pty-session'
|
||||
|
||||
@@ -70,6 +71,17 @@ export function buildPaneTransportOptions(session: ConnectPanePtySession): IpcPt
|
||||
onPtyExit: session.onExit,
|
||||
onPtySpawn: session.onPtySpawn,
|
||||
onPtyRebind: session.onPtyRebind,
|
||||
// Why a remount: the exit already tore down this mount's PTY state, so a fresh mount attaches.
|
||||
onHostReboundExitedPty: (ptyId) =>
|
||||
void requestTerminalPaneRecovery({
|
||||
tabId: session.deps.tabId,
|
||||
ptyId,
|
||||
reason: 'host-rebound-exited-pane',
|
||||
// Why: a new host process is fresh evidence a failed attempt can't refuse; the cap still holds.
|
||||
trigger: 'host-evidence',
|
||||
terminalRecoveryGeneration: session.terminalRecoveryGeneration,
|
||||
terminalRecoveryInstanceId: session.terminalRecoveryInstance.id
|
||||
}),
|
||||
retainDisposedSpawn: () =>
|
||||
shouldRetainDisposedPaneSpawn(
|
||||
useAppStore.getState(),
|
||||
|
||||
@@ -323,6 +323,8 @@ export type IpcPtyTransportOptions = {
|
||||
retainDisposedSpawn?: () => boolean
|
||||
/** Rebind an existing pane after its provider replaces the PTY identity. */
|
||||
onPtyRebind?: (ptyId: string, replacedPtyId: string, incarnationId?: string | null) => void
|
||||
/** The host bound a new live process to a pane this transport already ended on its exit record. */
|
||||
onHostReboundExitedPty?: (ptyId: string) => void
|
||||
onBell?: () => void
|
||||
onAgentBecameIdle?: (title: string, meta?: AgentIdleEdgeMeta) => void
|
||||
onAgentBecameWorking?: () => void
|
||||
|
||||
+111
-2
@@ -63,14 +63,18 @@ function answerShow(terminal: Record<string, unknown>): string[] {
|
||||
return shown
|
||||
}
|
||||
|
||||
async function attachRestoredHostTab(onPtyExit: (ptyId: string, exitCode?: number) => void) {
|
||||
async function attachRestoredHostTab(
|
||||
onPtyExit: (ptyId: string, exitCode?: number) => void,
|
||||
onHostReboundExitedPty?: (ptyId: string) => void
|
||||
) {
|
||||
resolvedPaneHandle = 'terminal-ended'
|
||||
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
|
||||
const transport = createRemoteRuntimePtyTransport('hub-env', {
|
||||
worktreeId: 'wt-1',
|
||||
tabId: 'web-terminal-host-tab-1',
|
||||
leafId: 'pane:1',
|
||||
onPtyExit
|
||||
onPtyExit,
|
||||
...(onHostReboundExitedPty ? { onHostReboundExitedPty } : {})
|
||||
})
|
||||
transport.attach({ existingPtyId: HOST_TAB_PTY_ID, callbacks: {} })
|
||||
await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled())
|
||||
@@ -78,6 +82,40 @@ async function attachRestoredHostTab(onPtyExit: (ptyId: string, exitCode?: numbe
|
||||
return transport
|
||||
}
|
||||
|
||||
async function publishHostSurface(
|
||||
surface: { terminal: string; incarnationId?: string } | null,
|
||||
snapshotVersion: number
|
||||
): Promise<void> {
|
||||
const tab = {
|
||||
type: 'terminal' as const,
|
||||
id: 'host-tab-1::pane:1',
|
||||
parentTabId: 'host-tab-1',
|
||||
leafId: 'pane:1',
|
||||
title: 'Terminal',
|
||||
isActive: true
|
||||
}
|
||||
// Why dynamic: the harness resets modules, so the transport's channel is the freshly loaded one.
|
||||
const { queueAcceptedWebSessionTerminalSnapshot } =
|
||||
await import('../../runtime/web-session-terminal-handle-events')
|
||||
queueAcceptedWebSessionTerminalSnapshot(
|
||||
{
|
||||
worktree: 'wt-1',
|
||||
publicationEpoch: 'epoch-ready',
|
||||
snapshotVersion,
|
||||
activeGroupId: null,
|
||||
activeTabId: tab.id,
|
||||
activeTabType: 'terminal',
|
||||
tabs: [
|
||||
surface
|
||||
? { ...tab, status: 'ready', ...surface }
|
||||
: { ...tab, status: 'pending-handle', terminal: null }
|
||||
]
|
||||
},
|
||||
'hub-env'
|
||||
)
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
}
|
||||
|
||||
describe('remote runtime pane whose host session already ended (#31539)', () => {
|
||||
beforeEach(() => {
|
||||
resetRemoteRuntimeTransport()
|
||||
@@ -232,6 +270,77 @@ describe('remote runtime pane whose host session already ended (#31539)', () =>
|
||||
transport.destroy?.()
|
||||
})
|
||||
|
||||
it('hands a settled pane back once a host snapshot publishes its new process', async () => {
|
||||
const onHostReboundExitedPty = vi.fn()
|
||||
const transport = await attachRestoredHostTab(vi.fn(), onHostReboundExitedPty)
|
||||
answerShow({ connected: false, verdict: 'exited', incarnationId: 'inc-1' })
|
||||
refuseStream('terminal_handle_stale')
|
||||
await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('ended'))
|
||||
|
||||
await publishHostSurface({ terminal: 'terminal-ended', incarnationId: 'inc-1' }, 4)
|
||||
await publishHostSurface({ terminal: 'terminal-ended' }, 5)
|
||||
await publishHostSurface(null, 6)
|
||||
expect(onHostReboundExitedPty).not.toHaveBeenCalled()
|
||||
|
||||
await publishHostSurface({ terminal: 'terminal-ended', incarnationId: 'inc-2' }, 7)
|
||||
await publishHostSurface({ terminal: 'terminal-later' }, 8)
|
||||
expect(onHostReboundExitedPty.mock.calls).toEqual([['remote:hub-env@@terminal-ended']])
|
||||
transport.destroy?.()
|
||||
})
|
||||
|
||||
it('hands a settled pane back to a new handle the host binds to it', async () => {
|
||||
const onHostReboundExitedPty = vi.fn()
|
||||
const transport = await attachRestoredHostTab(vi.fn(), onHostReboundExitedPty)
|
||||
answerShow({ connected: false, exitCause: { kind: 'operator_close' } })
|
||||
refuseStream('no_connected_pty')
|
||||
await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('ended'))
|
||||
|
||||
await publishHostSurface({ terminal: 'terminal-restarted' }, 4)
|
||||
|
||||
expect(onHostReboundExitedPty.mock.calls).toEqual([['remote:hub-env@@terminal-restarted']])
|
||||
transport.destroy?.()
|
||||
})
|
||||
|
||||
it('hands a settled pane back to a process the host published while the exit was confirming', async () => {
|
||||
const onHostReboundExitedPty = vi.fn()
|
||||
const transport = await attachRestoredHostTab(vi.fn(), onHostReboundExitedPty)
|
||||
answerShow({ connected: false, verdict: 'exited', incarnationId: 'inc-1' })
|
||||
let answerPane: (value: unknown) => void = () => {}
|
||||
const answer = runtimeCall.getMockImplementation()
|
||||
runtimeCall.mockImplementation(async (args: { method: string; params?: unknown }) =>
|
||||
args.method === 'terminal.resolvePane'
|
||||
? new Promise((resolve) => {
|
||||
answerPane = resolve
|
||||
})
|
||||
: answer?.(args)
|
||||
)
|
||||
refuseStream('terminal_handle_stale')
|
||||
await vi.waitFor(() =>
|
||||
expect(runtimeCall).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ method: 'terminal.resolvePane' })
|
||||
)
|
||||
)
|
||||
|
||||
await publishHostSurface({ terminal: 'terminal-ended', incarnationId: 'inc-2' }, 4)
|
||||
answerPane({
|
||||
ok: true,
|
||||
result: {
|
||||
terminal: {
|
||||
handle: 'terminal-ended',
|
||||
tabId: 'host-tab-1',
|
||||
leafId: 'pane:1',
|
||||
worktreeId: 'wt-1',
|
||||
incarnationId: 'inc-1'
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(onHostReboundExitedPty.mock.calls).toEqual([['remote:hub-env@@terminal-ended']])
|
||||
)
|
||||
transport.destroy?.()
|
||||
})
|
||||
|
||||
it('settles a pane whose restore attach is refused as stale before it binds', async () => {
|
||||
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
|
||||
const onPtyExit = vi.fn()
|
||||
|
||||
@@ -60,6 +60,10 @@ import { getRemoteRuntimeTerminalMultiplexer } from '../../runtime/remote-runtim
|
||||
import { REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE } from '../../runtime/remote-runtime-terminal-snapshot-state'
|
||||
import { restoredSnapshotPaintsPrintableContent } from './restored-snapshot-coverage'
|
||||
import { hostRecordsTerminalExit, sameWhenBothKnown } from './remote-terminal-host-exit-record'
|
||||
import {
|
||||
watchHostRebindAfterExit,
|
||||
type HostRebindAfterExitWatch
|
||||
} from './remote-terminal-host-rebind-after-exit'
|
||||
import type {
|
||||
RemoteRuntimeMultiplexedTerminal,
|
||||
RemoteRuntimeSnapshotOutcome
|
||||
@@ -226,6 +230,7 @@ export function createRemoteRuntimePtyTransport(
|
||||
onPtyExit,
|
||||
onPtySpawn,
|
||||
onPtyRebind,
|
||||
onHostReboundExitedPty,
|
||||
onTitleChange,
|
||||
onBell,
|
||||
onAgentBecameIdle,
|
||||
@@ -271,6 +276,7 @@ export function createRemoteRuntimePtyTransport(
|
||||
let recoveryReplacementPolicy: HostHandleReplacementPolicy = 'reuse'
|
||||
let recoveryReplacementPolicyHandle: string | null = null
|
||||
let stopWaitingForPublishedHandle: (() => void) | null = null
|
||||
let stopWatchingHostRebind: (() => void) | null = null
|
||||
let publishedHandleWaitEpoch: number | null = null
|
||||
// Why: a spent auto-recovery window is the evidence that licenses reattaching the fenced handle; explicit retries must not erase it.
|
||||
let autoRecoveryWindowSpent = false
|
||||
@@ -2209,6 +2215,8 @@ export function createRemoteRuntimePtyTransport(
|
||||
stopWaitingForPublishedHandle?.()
|
||||
stopWaitingForPublishedHandle = null
|
||||
publishedHandleWaitEpoch = null
|
||||
stopWatchingHostRebind?.()
|
||||
stopWatchingHostRebind = null
|
||||
}
|
||||
|
||||
function isCurrentRemoteTerminal(targetHandle: string, targetPtyId: string | null): boolean {
|
||||
@@ -2262,6 +2270,7 @@ export function createRemoteRuntimePtyTransport(
|
||||
probeLifecycle === lifecycleEpoch &&
|
||||
handle === targetHandle &&
|
||||
sameWhenBothKnown(authoritativePtyIncarnationId, exited.incarnationId)
|
||||
let rebindWatch = startHostRebindWatch()
|
||||
void (async () => {
|
||||
const { terminal } = await callRuntime<{ terminal?: RuntimeTerminalShow }>('terminal.show', {
|
||||
terminal: targetHandle
|
||||
@@ -2276,17 +2285,46 @@ export function createRemoteRuntimePtyTransport(
|
||||
return
|
||||
}
|
||||
remotePtyId ??= exitedPtyId
|
||||
const exitedIncarnationId = terminal.incarnationId ?? authoritativePtyIncarnationId
|
||||
const cause = terminal.exitCause
|
||||
retireRemoteTerminalId(cause?.kind === 'exited' ? cause.exitCode : undefined)
|
||||
if (rebindWatch && !destroyed) {
|
||||
stopWatchingHostRebind = rebindWatch.dispose
|
||||
const armed = rebindWatch
|
||||
rebindWatch = null
|
||||
armed.arm(targetHandle, exitedIncarnationId)
|
||||
}
|
||||
})()
|
||||
.catch(() => {})
|
||||
.finally(() => {
|
||||
rebindWatch?.dispose()
|
||||
if (hostExitProbeHandle === targetHandle) {
|
||||
hostExitProbeHandle = null
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Why: a restart's new process can land just after the probe read the old one's exit (#31968).
|
||||
function startHostRebindWatch(): HostRebindAfterExitWatch | null {
|
||||
if (destroyed || !onHostReboundExitedPty || !worktreeId || !tabId || !leafId) {
|
||||
return null
|
||||
}
|
||||
if (!isWebTerminalSurfaceTabId(tabId)) {
|
||||
return null
|
||||
}
|
||||
const watchLifecycle = lifecycleEpoch
|
||||
const environmentId = currentRuntimeEnvironmentId
|
||||
return watchHostRebindAfterExit(
|
||||
{ environmentId, worktreeId, hostTabId: toHostSessionTabId(tabId), leafId },
|
||||
(nextHandle) => {
|
||||
stopWatchingHostRebind = null
|
||||
if (!destroyed && terminalEnded && watchLifecycle === lifecycleEpoch) {
|
||||
onHostReboundExitedPty(toRemoteRuntimePtyId(nextHandle, environmentId))
|
||||
}
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
async function hostPaneStillHoldsExitedTerminal(
|
||||
targetHandle: string,
|
||||
exited: Pick<RuntimeTerminalShow, 'ptyId' | 'incarnationId'>
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import {
|
||||
subscribeAcceptedWebSessionTerminalHandle,
|
||||
type WebSessionTerminalHandleUpdate
|
||||
} from '../../runtime/web-session-terminal-handle-events'
|
||||
import { sameWhenBothKnown } from './remote-terminal-host-exit-record'
|
||||
|
||||
export type HostRebindAfterExitWatch = {
|
||||
/** Starts judging updates against the settled exit, the one seen while probing included. */
|
||||
arm: (exitedHandle: string, exitedIncarnationId: string | null) => void
|
||||
dispose: () => void
|
||||
}
|
||||
|
||||
/** Fires once the host publishes a provably different live process for an exit-settled pane. */
|
||||
export function watchHostRebindAfterExit(
|
||||
args: { environmentId: string; worktreeId: string; hostTabId: string; leafId: string },
|
||||
onRebound: (handle: string) => void
|
||||
): HostRebindAfterExitWatch {
|
||||
let stopped = false
|
||||
let exited: { handle: string; incarnationId: string | null } | null = null
|
||||
// Why buffered: the replacement can be published while the exit probe is still confirming.
|
||||
let latest: WebSessionTerminalHandleUpdate | null = null
|
||||
const judge = (update: WebSessionTerminalHandleUpdate): void => {
|
||||
if (stopped || !exited) {
|
||||
return
|
||||
}
|
||||
if (!update.surfacePresent) {
|
||||
dispose()
|
||||
return
|
||||
}
|
||||
const next = update.terminalHandle
|
||||
if (
|
||||
!next ||
|
||||
(next === exited.handle && sameWhenBothKnown(update.incarnationId, exited.incarnationId))
|
||||
) {
|
||||
return
|
||||
}
|
||||
dispose()
|
||||
onRebound(next)
|
||||
}
|
||||
const stop = subscribeAcceptedWebSessionTerminalHandle(args, (update) => {
|
||||
latest = update
|
||||
judge(update)
|
||||
})
|
||||
function dispose(): void {
|
||||
stopped = true
|
||||
stop()
|
||||
}
|
||||
return {
|
||||
arm(exitedHandle, exitedIncarnationId) {
|
||||
exited = { handle: exitedHandle, incarnationId: exitedIncarnationId }
|
||||
if (latest) {
|
||||
judge(latest)
|
||||
}
|
||||
},
|
||||
dispose
|
||||
}
|
||||
}
|
||||
+57
@@ -0,0 +1,57 @@
|
||||
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
_resetTerminalPaneRecoveryForTests,
|
||||
captureTerminalPaneRecoveryGeneration
|
||||
} from './terminal-pane-recovery'
|
||||
import { requestAndSettle } from './terminal-recovery-ledger-test-driver'
|
||||
import {
|
||||
recoveryLedgerMocks as mocks,
|
||||
resetRecoveryLedgerStore,
|
||||
setTerminalTabs
|
||||
} from './terminal-recovery-ledger-test-store'
|
||||
|
||||
vi.mock('@/store', async () => {
|
||||
const store = await import('./terminal-recovery-ledger-test-store')
|
||||
return { useAppStore: { getState: () => store.recoveryLedgerStoreState() } }
|
||||
})
|
||||
|
||||
vi.mock('@/lib/crash-breadcrumb-recorder', async () => {
|
||||
const store = await import('./terminal-recovery-ledger-test-store')
|
||||
return { recordRendererCrashBreadcrumb: store.recoveryLedgerMocks.recordRendererCrashBreadcrumb }
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
_resetTerminalPaneRecoveryForTests()
|
||||
resetRecoveryLedgerStore()
|
||||
setTerminalTabs([{ id: 'tab-1' }])
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('stops re-binding at the window cap when the host keeps contradicting its exit record', async () => {
|
||||
vi.useFakeTimers()
|
||||
const rebind = () =>
|
||||
requestAndSettle(
|
||||
{
|
||||
tabId: 'tab-1',
|
||||
ptyId: 'remote:env@@terminal-1',
|
||||
reason: 'host-rebound-exited-pane',
|
||||
trigger: 'host-evidence',
|
||||
terminalRecoveryGeneration: captureTerminalPaneRecoveryGeneration('tab-1')
|
||||
},
|
||||
// Each remounted pane is settled as exited again by the same contradicting host.
|
||||
'failed'
|
||||
)
|
||||
|
||||
const outcomes: boolean[] = []
|
||||
for (let attempt = 0; attempt < 5; attempt += 1) {
|
||||
vi.setSystemTime(attempt * 20_000)
|
||||
outcomes.push(await rebind())
|
||||
}
|
||||
|
||||
expect(outcomes).toEqual([true, true, true, false, false])
|
||||
await vi.advanceTimersByTimeAsync(10 * 60_000)
|
||||
expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
@@ -101,7 +101,10 @@ function shouldScheduleRecoveryRetry(
|
||||
return request.terminalRecoveryGeneration !== undefined
|
||||
}
|
||||
if (decline.declinedBy === 'window-cap') {
|
||||
return request.reason !== 'reattach-unverifiable'
|
||||
// Why: a host that keeps contradicting itself must stop at the cap and leave the pane ended.
|
||||
return (
|
||||
request.reason !== 'reattach-unverifiable' && request.reason !== 'host-rebound-exited-pane'
|
||||
)
|
||||
}
|
||||
// 'settled-failure' deliberately schedules nothing: a retry timer would be
|
||||
// the counting loop again. Only a new trigger reopens that reason.
|
||||
|
||||
@@ -7,6 +7,8 @@ import { hostSnapshotAffirmsWorktreeContents } from './host-session-snapshot-aut
|
||||
export type WebSessionTerminalHandleUpdate = {
|
||||
surfacePresent: boolean
|
||||
terminalHandle: string | null
|
||||
/** The published handle's process lifetime, when the host stamps one. */
|
||||
incarnationId?: string | null
|
||||
}
|
||||
|
||||
type TerminalHandleSubscriber = {
|
||||
@@ -49,7 +51,8 @@ function resolveSubscriberUpdate(
|
||||
mirroredSurfaces.find((surface) => surface.status === 'ready')
|
||||
return {
|
||||
surfacePresent: true,
|
||||
terminalHandle: readySurface?.terminal ?? null
|
||||
terminalHandle: readySurface?.terminal ?? null,
|
||||
...(readySurface?.incarnationId ? { incarnationId: readySurface.incarnationId } : {})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -31,9 +31,10 @@ export const RECOVERY_COOLDOWN_MS = 15_000
|
||||
export const RECOVERY_SETTLEMENT_TIMEOUT_MS = 31_000
|
||||
|
||||
/** Why a request exists at all. Only 'automatic' is subject to the
|
||||
* settled-failure refusal: a user pressing Retry, or an external lifecycle
|
||||
* remount, IS the new trigger the refusal is waiting for. */
|
||||
export type TerminalRecoveryTrigger = 'automatic' | 'user' | 'external'
|
||||
* settled-failure refusal: a user pressing Retry, an external lifecycle
|
||||
* remount, or a host publishing a new process IS the new trigger the refusal
|
||||
* is waiting for. 'host-evidence' still counts against the window cap. */
|
||||
export type TerminalRecoveryTrigger = 'automatic' | 'user' | 'external' | 'host-evidence'
|
||||
|
||||
export type TerminalRecoveryRemountRequest = {
|
||||
reason: TerminalPaneRecoveryReason
|
||||
@@ -133,7 +134,7 @@ export function admitTerminalRecoveryRemount(
|
||||
retryInMs: recent[0] + RECOVERY_WINDOW_MS - request.now
|
||||
}
|
||||
}
|
||||
if (request.trigger === 'user') {
|
||||
if (request.trigger === 'user' || request.trigger === 'host-evidence') {
|
||||
// The user asking again IS the new evidence. Only the window cap — the
|
||||
// backstop against a loop neither side can see — survives it.
|
||||
return ADMITTED
|
||||
|
||||
@@ -22,6 +22,8 @@ export type TerminalPaneRecoveryReason =
|
||||
// binding. pty:data for the old id then lands in the pre-handler buffer, which
|
||||
// ACKs it — main's delivery health stays green while the pane shows nothing.
|
||||
| 'spawn-left-pane-unbound'
|
||||
// A pane ended on its host's exit record, then the host bound a new process to it.
|
||||
| 'host-rebound-exited-pane'
|
||||
|
||||
export type TerminalPaneRecoveryOutcome =
|
||||
| 'pending'
|
||||
|
||||
Reference in New Issue
Block a user