fix: preserve PTY generation and observer authority

This commit is contained in:
Neil
2026-07-11 04:19:49 -07:00
parent 3440d8a346
commit 21ebcb4fa9
8 changed files with 183 additions and 40 deletions
+8 -7
View File
@@ -3223,7 +3223,8 @@
"PTY startup",
"background terminal launch",
"handler registration",
"failed shutdown recovery"
"failed shutdown recovery",
"daemon cold restore"
],
"platforms": [
"macos",
@@ -3247,8 +3248,8 @@
"motivatingLinks": [
"https://github.com/stablyai/orca/pull/7695"
],
"invariant": "Before a PTY handler mounts, the singleton dispatcher is already listening and retained data, exit, and warning identities remain capped; data drains before exit; a fresh provider-reused PTY id cannot consume an older lifecycle's events; explicit reattach keeps its current backlog; and failed shutdown restore drains detached data before later live output.",
"oracle": "Drive first-use background exit-before-spawn-resolution, more than 64 exit-before-handler identities, warning-producing data churn, duplicate/reused ids, fresh-spawn and explicit-reattach ordering, handler registration at cap, and failed-shutdown restoration; assert listener-first capture, bounded eviction, exact latest-code delivery once, fresh-generation filtering, data-before-exit order, and detached-before-live output.",
"invariant": "Before a PTY handler mounts, the singleton dispatcher is already listening and retained data, exit, and warning identities remain capped; pre-spawn bytes reach background side-effect observers exactly once before live-sidecar handoff; data drains before exit; a fresh provider-reused PTY id or new-generation cold restore cannot consume an older lifecycle's events; explicit live reattach keeps its current backlog; and failed shutdown restore drains detached data before later live output without overwriting a newer remount handler.",
"oracle": "Drive first-use background data/exit before spawn resolution, real observer-to-sidecar handoff and handler replacement, more than 64 exit-before-handler identities, warning-producing data churn, duplicate/reused ids, fresh spawn, failed-reattach fallback, new-generation cold restore, explicit live reattach, handler registration at cap, and failed-shutdown restoration after a newer remount; assert listener-first capture, exactly-once observer handoff, bounded eviction, exact latest-code delivery once, fresh-generation filtering with restore payload preservation, data-before-exit order, and detached-before-live output routed to current authority.",
"commands": [
"pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/launch-worktree-background-terminals.test.ts"
],
@@ -3272,15 +3273,15 @@
"file": "src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts",
"assertions": [
"pre-handler data drains before its exit",
"fresh PTY-id reuse and failed-reattach fresh fallback filter old lifecycle events while explicit daemon reattach preserves pre-connect backlog",
"first-use eager registration attaches the singleton dispatcher before a fast background PTY can emit data or exit",
"fresh PTY-id reuse, failed-reattach fallback, and new-generation cold restore filter old lifecycle events while explicit daemon reattach preserves pre-connect backlog",
"first-use eager registration attaches the singleton dispatcher before a fast background PTY emits data or exit, routes retained bytes through its side-effect observer exactly once, and preserves a replacement handler",
"a registered exit handler remains authoritative while 65 orphan exits churn through the bounded buffer"
]
},
{
"file": "src/renderer/src/components/terminal-pane/pty-transport.test.ts",
"assertions": [
"failed intentional shutdown restoration drains data received while detached before later live data"
"failed intentional shutdown restoration drains detached data before later live data to a newer remount handler without overwriting its data, replay, or teardown authority"
]
}
],
@@ -3292,7 +3293,7 @@
"command": "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/launch-worktree-background-terminals.test.ts",
"result": "passed",
"durationSeconds": 1,
"summary": "5 test files passed, 115 tests passed on main@20ebe858af plus the #7695 candidate. Main@20ebe858af retained the 65th oldest exit (received code 0 instead of eviction); the bounded candidate evicts it and preserves listener-first and ordering/reuse cases."
"summary": "5 test files and 120 tests passed on main@dcfa91919f plus the #7695 candidate. The red repro on main@20ebe858af retained the 65th oldest exit (received code 0 instead of eviction); the candidate evicts it and preserves listener-first, exactly-once side-effect handoff, ordering/reuse, and new-generation cold-restore cases."
}
],
"runtimeBudget": {
@@ -400,6 +400,49 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => {
expect(events).toEqual(['data:fresh-data', 'exit:2'])
})
it('filters an older lifecycle when cold restore starts a new reused-id generation', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const { ensurePtyDispatcher } = await import('./pty-dispatcher')
const events: string[] = []
let resolveSpawn: (value: {
id: string
coldRestore: { scrollback: string; cwd: string }
}) => void = () => {}
ensurePtyDispatcher()
dispatcherCallback?.({ id: 'pty-cold-restore', data: 'old-data' })
exitDispatcherCallback?.({ id: 'pty-cold-restore', code: 1 })
;(window.api.pty.spawn as ReturnType<typeof vi.fn>).mockReturnValueOnce(
new Promise((resolve) => {
resolveSpawn = resolve
})
)
const transport = createIpcPtyTransport()
const connectPromise = transport.connect({
url: '',
sessionId: 'pty-cold-restore',
callbacks: {
onData: (data) => events.push(`data:${data}`),
onExit: (code) => events.push(`exit:${code}`)
}
})
dispatcherCallback?.({ id: 'pty-cold-restore', data: 'restored-generation-data' })
resolveSpawn({
id: 'pty-cold-restore',
coldRestore: { scrollback: 'saved scrollback', cwd: '/saved' }
})
const result = await connectPromise
expect(events).toEqual(['data:restored-generation-data'])
expect(result).toEqual(
expect.objectContaining({ coldRestore: { scrollback: 'saved scrollback', cwd: '/saved' } })
)
exitDispatcherCallback?.({ id: 'pty-cold-restore', code: 2 })
expect(events).toEqual(['data:restored-generation-data', 'exit:2'])
})
it('preserves pre-connect backlog for an explicit daemon reattach', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const { ensurePtyDispatcher } = await import('./pty-dispatcher')
@@ -442,7 +485,7 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => {
})
it('attaches the dispatcher before first-use eager-buffer spawn can emit an exit', async () => {
const { captureEagerPtyBufferRegistration } = await import('./pty-dispatcher')
const { captureEagerPtyBufferRegistration } = await import('./pty-eager-buffer-registration')
const eagerExit = vi.fn()
expect(exitDispatcherCallback).toBeNull()
@@ -460,21 +503,42 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => {
})
it('filters old lifecycle events for delayed background eager-buffer registration', async () => {
const { captureEagerPtyBufferRegistration, ensurePtyDispatcher } =
await import('./pty-dispatcher')
const { captureEagerPtyBufferRegistration } = await import('./pty-eager-buffer-registration')
const { ensurePtyDispatcher, ptyDataHandlers } = await import('./pty-dispatcher')
const { subscribeToPtyData } = await import('./pty-data-sidecar-subscriptions')
const eagerExit = vi.fn()
const automationStatusObserver = vi.fn()
const replacementHandler = vi.fn()
ensurePtyDispatcher()
dispatcherCallback?.({ id: 'pty-background-reused', data: 'old-background-data' })
exitDispatcherCallback?.({ id: 'pty-background-reused', code: 1 })
const registerEagerPtyBuffer = captureEagerPtyBufferRegistration()
const registerEagerPtyBuffer = captureEagerPtyBufferRegistration(automationStatusObserver)
dispatcherCallback?.({ id: 'pty-background-reused', data: 'new-background-data' })
exitDispatcherCallback?.({ id: 'pty-background-reused', code: 2 })
const handle = registerEagerPtyBuffer('pty-background-reused', eagerExit)
expect(handle.flush()).toBe('new-background-data')
expect(automationStatusObserver).toHaveBeenCalledOnce()
expect(automationStatusObserver).toHaveBeenCalledWith('new-background-data')
handle.stopObservingData?.()
const unsubscribeSidecar = subscribeToPtyData('pty-background-reused', automationStatusObserver)
ptyDataHandlers.set('pty-background-reused', replacementHandler)
dispatcherCallback?.({ id: 'pty-background-reused', data: 'live-sidecar-era-data' })
await Promise.resolve()
handle.dispose()
dispatcherCallback?.({ id: 'pty-background-reused', data: 'replacement-owned-data' })
expect(eagerExit).toHaveBeenCalledOnce()
expect(eagerExit).toHaveBeenCalledWith('pty-background-reused', 2)
expect(automationStatusObserver.mock.calls).toEqual([
['new-background-data'],
['live-sidecar-era-data'],
['replacement-owned-data']
])
expect(replacementHandler.mock.calls).toEqual([
['live-sidecar-era-data', undefined],
['replacement-owned-data', undefined]
])
unsubscribeSidecar()
})
})
@@ -105,17 +105,19 @@ export function restorePtyDataHandlersAfterFailedShutdown(
snapshots: readonly PtyDataHandlerShutdownSnapshot[]
): void {
for (const snapshot of snapshots) {
if (snapshot.dataHandler) {
ptyDataHandlers.set(snapshot.ptyId, snapshot.dataHandler)
const authoritativeDataHandler = ptyDataHandlers.get(snapshot.ptyId) ?? snapshot.dataHandler
if (authoritativeDataHandler) {
if (!ptyDataHandlers.has(snapshot.ptyId)) {
ptyDataHandlers.set(snapshot.ptyId, authoritativeDataHandler)
}
// Why: a kill failure can restore this handler after main already sent
// a final data burst into the pre-handler buffer. Replay it before later
// live data so failure recovery neither leaks nor reorders output.
drainPreHandlerPtyData(snapshot.ptyId, snapshot.dataHandler)
// a final data burst. A newer remount owns delivery if it registered first.
drainPreHandlerPtyData(snapshot.ptyId, authoritativeDataHandler)
}
if (snapshot.replayHandler) {
if (snapshot.replayHandler && !ptyReplayHandlers.has(snapshot.ptyId)) {
ptyReplayHandlers.set(snapshot.ptyId, snapshot.replayHandler)
}
if (snapshot.teardownHandler) {
if (snapshot.teardownHandler && !ptyTeardownHandlers.has(snapshot.ptyId)) {
ptyTeardownHandlers.set(snapshot.ptyId, snapshot.teardownHandler)
}
}
@@ -298,7 +300,11 @@ export function subscribeToPtyExit(ptyId: string, watcher: (code: number) => voi
// (prompt, MOTD) arrives via pty:data before xterm exists. These helpers buffer
// that output so transport.attach() can replay it when the pane finally mounts.
export type EagerPtyHandle = { flush: () => string; dispose: () => void }
export type EagerPtyHandle = {
flush: () => string
dispose: () => void
stopObservingData?: () => void
}
const eagerPtyHandles = new Map<string, EagerPtyHandle>()
export function getEagerPtyBufferHandle(ptyId: string): EagerPtyHandle | undefined {
@@ -310,18 +316,11 @@ export function getEagerPtyBufferHandle(ptyId: string): EagerPtyHandle | undefin
// runs a long-lived command (e.g. tail -f) in a worktree the user never opens.
const EAGER_BUFFER_MAX_BYTES = TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT
export function captureEagerPtyBufferRegistration(): typeof registerEagerPtyBuffer {
// Why: pty:exit is not boot-gated like pty:data. Attach the singleton before
// spawn so a first-use background PTY cannot exit into a listener-less page.
ensurePtyDispatcher()
const afterCursor = capturePreHandlerPtyEventCursor()
return (ptyId, onExit) => registerEagerPtyBuffer(ptyId, onExit, afterCursor)
}
export function registerEagerPtyBuffer(
ptyId: string,
onExit: (ptyId: string, code: number) => void,
afterCursor?: number
afterCursor?: number,
observePreSubscriptionData?: (data: string) => void
): EagerPtyHandle {
ensurePtyDispatcher()
// Why: a head index instead of Array.shift() — shift() is O(n), making
@@ -336,6 +335,7 @@ export function registerEagerPtyBuffer(
const chunk = clampUtf8Tail(data, EAGER_BUFFER_MAX_BYTES)
chunks.push(chunk)
bufferBytes += chunk.bytes
observePreSubscriptionData?.(data)
// Drop whole leading chunks (keeping the prompt-bearing tail) until within cap.
while (bufferBytes > EAGER_BUFFER_MAX_BYTES && head < chunks.length - 1) {
bufferBytes -= chunks[head].bytes
@@ -376,6 +376,9 @@ export function registerEagerPtyBuffer(
bufferBytes = 0
return data
},
stopObservingData() {
observePreSubscriptionData = undefined
},
dispose() {
// Why: dispose runs at pane attach (mount completed) — the pane's own
// visibility sync now owns the hidden-delivery decision for this PTY.
@@ -0,0 +1,18 @@
import { ensurePtyDispatcher, registerEagerPtyBuffer, type EagerPtyHandle } from './pty-dispatcher'
import { capturePreHandlerPtyEventCursor } from './pty-pre-handler-buffer'
export type EagerPtyBufferRegistration = (
ptyId: string,
onExit: (ptyId: string, code: number) => void
) => EagerPtyHandle
export function captureEagerPtyBufferRegistration(
observePreSubscriptionData?: (data: string) => void
): EagerPtyBufferRegistration {
// Why: pty:exit is not boot-gated like pty:data. Attach before spawn and
// retain one cursor so reused ids cannot inherit an older lifecycle.
ensurePtyDispatcher()
const afterCursor = capturePreHandlerPtyEventCursor()
return (ptyId, onExit) =>
registerEagerPtyBuffer(ptyId, onExit, afterCursor, observePreSubscriptionData)
}
@@ -1444,17 +1444,31 @@ describe('createIpcPtyTransport', () => {
} = await import('./pty-transport')
const onDataCallback = vi.fn()
const transport = createIpcPtyTransport()
const replacementDataHandler = vi.fn()
const replacementReplayHandler = vi.fn()
const replacementTeardownHandler = vi.fn()
const { ptyDataHandlers, ptyReplayHandlers, ptyTeardownHandlers } =
await import('./pty-dispatcher')
await transport.connect({ url: '', callbacks: { onData: onDataCallback } })
const snapshots = unregisterPtyDataHandlers(['pty-1'])
onData?.({ id: 'pty-1', data: 'final burst while detached' })
expect(onDataCallback).not.toHaveBeenCalled()
ptyDataHandlers.set('pty-1', replacementDataHandler)
ptyReplayHandlers.set('pty-1', replacementReplayHandler)
ptyTeardownHandlers.set('pty-1', replacementTeardownHandler)
restorePtyDataHandlersAfterFailedShutdown(snapshots)
onData?.({ id: 'pty-1', data: 'live again' })
expect(onDataCallback.mock.calls).toEqual([['final burst while detached'], ['live again']])
expect(onDataCallback).not.toHaveBeenCalled()
expect(replacementDataHandler.mock.calls).toEqual([
['final burst while detached', undefined],
['live again', undefined]
])
expect(ptyReplayHandlers.get('pty-1')).toBe(replacementReplayHandler)
expect(ptyTeardownHandlers.get('pty-1')).toBe(replacementTeardownHandler)
})
it('unregisterPtyDataHandlers cancels staleTitleTimer so it cannot fire stale idle transition', async () => {
@@ -753,8 +753,9 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
onPtySpawn?.(spawnResult.id)
}
const afterCursor =
spawnResult.isReattach || spawnResult.coldRestore ? undefined : freshSpawnCursor
// Why: cold restore may start a new daemon generation with a reused id.
// Only explicit live reattach has authority to consume pre-spawn backlog.
const afterCursor = spawnResult.isReattach ? undefined : freshSpawnCursor
registerPtyDataHandler(spawnResult.id, afterCursor)
registerPtyExitHandler(spawnResult.id, afterCursor)
if (!connected || ptyId !== spawnResult.id) {
@@ -23,6 +23,7 @@ const mockPasteDraftWhenAgentReady = vi.fn()
const mockMarkTrusted = vi.fn()
const mockDispatchEvent = vi.fn()
const mockGetAgentLaunchPlatformForRepo = vi.fn<() => NodeJS.Platform>()
let capturedPreSubscriptionDataObserver: ((data: string) => void) | undefined
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/
function expectStablePaneSpawn(): string {
@@ -99,8 +100,14 @@ vi.mock('@/lib/agent-launch-platform', () => ({
getAgentLaunchPlatformForRepo: mockGetAgentLaunchPlatformForRepo
}))
vi.mock('@/components/terminal-pane/pty-eager-buffer-registration', () => ({
captureEagerPtyBufferRegistration: (observer?: (data: string) => void) => {
capturedPreSubscriptionDataObserver = observer
return mockRegisterEagerPtyBuffer
}
}))
vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({
captureEagerPtyBufferRegistration: () => mockRegisterEagerPtyBuffer,
subscribeToPtyExit: mockSubscribeToPtyExit
}))
@@ -113,6 +120,7 @@ describe('launchAgentBackgroundSession', () => {
resetRemoteRuntimeTerminalMultiplexersForTests()
clearRuntimeCompatibilityCacheForTests()
vi.clearAllMocks()
capturedPreSubscriptionDataObserver = undefined
mockGetAgentLaunchPlatformForRepo.mockReturnValue('linux')
mockRuntimeEnvironmentTransportCall.mockImplementation(
(args) =>
@@ -155,6 +163,13 @@ describe('launchAgentBackgroundSession', () => {
})
mockSubscribeToPtyData.mockReturnValue(vi.fn())
mockSubscribeToPtyExit.mockReturnValue(vi.fn())
mockRegisterEagerPtyBuffer.mockReturnValue({
flush: () => '',
dispose: vi.fn(),
stopObservingData: () => {
capturedPreSubscriptionDataObserver = undefined
}
})
vi.stubGlobal('window', {
dispatchEvent: mockDispatchEvent,
api: {
@@ -273,6 +288,34 @@ describe('launchAgentBackgroundSession', () => {
)
})
it('delivers pre-subscription background bytes to observers exactly once', async () => {
const onData = vi.fn()
mockRegisterEagerPtyBuffer.mockImplementationOnce(() => {
capturedPreSubscriptionDataObserver?.('early output')
return {
flush: () => '',
dispose: vi.fn(),
stopObservingData: () => {
capturedPreSubscriptionDataObserver = undefined
}
}
})
const { launchAgentBackgroundSession } = await import('./launch-agent-background-session')
await launchAgentBackgroundSession({
agent: 'claude',
worktreeId: 'wt-1',
prompt: 'run the automation',
onData
})
expect(onData).toHaveBeenCalledWith('early output')
const dataSidecar = mockSubscribeToPtyData.mock.calls[0]?.[1] as (data: string) => void
capturedPreSubscriptionDataObserver?.('live output')
dataSidecar('live output')
expect(onData.mock.calls).toEqual([['early output'], ['live output']])
})
it('records effective launch config returned by local PTY spawn', async () => {
const effectiveLaunchConfig = {
agentCommand: "claude '--dangerously-skip-permissions'",
@@ -18,11 +18,8 @@ import {
import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config'
import { repoIsRemote } from '../../../shared/agent-launch-remote'
import { makePaneKey } from '../../../shared/stable-pane-id'
import {
captureEagerPtyBufferRegistration,
subscribeToPtyExit,
type EagerPtyHandle
} from '@/components/terminal-pane/pty-dispatcher'
import { subscribeToPtyExit, type EagerPtyHandle } from '@/components/terminal-pane/pty-dispatcher'
import { captureEagerPtyBufferRegistration } from '@/components/terminal-pane/pty-eager-buffer-registration'
import { subscribeToPtyData } from '@/components/terminal-pane/pty-data-sidecar-subscriptions'
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner'
@@ -62,7 +59,6 @@ export async function launchAgentBackgroundSession(
// Best-effort: continue with launch. The user can still accept the trust menu.
}
}
const agentArgs = resolveTuiAgentLaunchArgs(agent, store.settings?.agentDefaultArgs)
const agentEnv = resolveTuiAgentLaunchEnv(agent, store.settings?.agentDefaultEnv)
const launchPlatform = repo
? getAgentLaunchPlatformForRepo(
@@ -87,7 +83,7 @@ export async function launchAgentBackgroundSession(
agent,
prompt: hasPrompt && !isFollowupPath ? trimmedPrompt : '',
cmdOverrides: store.settings?.agentCmdOverrides ?? {},
agentArgs,
agentArgs: resolveTuiAgentLaunchArgs(agent, store.settings?.agentDefaultArgs),
agentEnv,
platform: launchPlatform,
shell: startupShell,
@@ -212,7 +208,9 @@ export async function launchAgentBackgroundSession(
runtimeTerminalHandle = created.terminal.handle
ptyId = toRemoteRuntimePtyId(runtimeTerminalHandle, runtimeTarget.environmentId)
} else {
const registerEagerPtyBuffer = captureEagerPtyBufferRegistration()
// Why: bytes emitted before spawn resolves must reach automation/status
// observers as well as the pane buffer, without duplicating later sidecar data.
const registerEagerPtyBuffer = captureEagerPtyBufferRegistration(handleData)
const result = await window.api.pty.spawn({
cols: 120,
rows: 40,
@@ -284,6 +282,7 @@ export async function launchAgentBackgroundSession(
.then((result) => handleExit(ptyId, result.wait.exitCode ?? 0))
.catch(() => {})
} else {
eagerPtyBuffer?.stopObservingData?.()
unsubscribeData = subscribeToPtyData(ptyId, handleData)
// Why: opening the workspace attaches a real terminal transport and disposes
// the eager exit handler. This sidecar keeps automation completion tracking