starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-28 00:02:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: fence teardown from replacement sessions

Browse Source
This commit is contained in:
Brennan Benson
2026-07-13 11:43:21 -07:00
parent 971b1d9b0c
commit 2384370d25
8 changed files with 262 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
config/reliability-gates.jsonc
+32 -2
View File
@@ -3686,7 +3686,8 @@
"file": "src/main/daemon/terminal-host.test.ts",
"assertions": [
"a reused live daemon session rejects attach and kill from a replacement pane identity",
"native immediate force-kill failure preserves the live daemon session for retry"
"native immediate force-kill failure preserves the live daemon session for retry",
"same-id create waits for physical exit proof without disposing the terminating session owner"
]
},
{
@@ -3786,6 +3787,7 @@
"disconnected SSH kill and stop retain the lease and reject without a synthetic exit",
"provider reconnect retries durable shutdown intent with its generation and pane identity",
"persisted local shutdown waits for daemon startup instead of accepting unknown-id success from the fallback provider",
"renderer and runtime same-id restore reject while startup teardown replay retains that exact PTY owner",
"an in-flight replaced local provider cannot complete or delete the newer shutdown owner",
"local and SSH stale-provider not-found rejections preserve replacement ownership and coalesced callers fail closed",
"direct local and SSH kill requests share the retained owner and re-drive after in-flight provider replacement",
@@ -3827,7 +3829,8 @@
{
"file": "src/main/ipc/runtime-terminal-close-retry-owner.test.ts",
"assertions": [
"a late runtime close completion cannot delete a replacement-generation retry owner"
"a late runtime close completion cannot delete a replacement-generation retry owner",
"same-generation terminal_handle_stale releases durable ownership and cancels the last retry timer"
]
},
{
@@ -4163,6 +4166,33 @@
"result": "passed",
"durationSeconds": 1.43,
"summary": "219 tests passed including packaged relay patch parity, vanilla pre-ready red control, one-shot accepted close ownership, and delayed OS-death inventory reaping."
},
{
"date": "2026-07-12",
"runner": "local",
"platform": "macos",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts",
"result": "passed",
"durationSeconds": 9.17,
"summary": "1442 tests passed: terminating same-id daemon sessions deferred creation until physical exit, renderer/runtime startup restore rejected retained teardown identities, and same-generation stale runtime handles released persistence plus the last timer."
},
{
"date": "2026-07-12",
"runner": "local",
"platform": "macos",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts",
"result": "passed",
"durationSeconds": 9.63,
"summary": "1891 tests passed across app-start teardown replay, local/SSH/runtime identity ownership, selected-provider liveness, and durable close settlement."
},
{
"date": "2026-07-12",
"runner": "local",
"platform": "macos",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts",
"result": "passed",
"durationSeconds": 10.49,
"summary": "1800 tests passed with exact same-id startup replay admission, retained pre-handler/liveness bounds, and cross-provider retry identity preserved."
}
],
"runtimeBudget": {
src/main/daemon/session.ts
+10
View File
@@ -139,6 +139,8 @@ export class Session {
private producerPaused = false
private producerPauseFailsafeTimer: ReturnType<typeof setTimeout> | null = null
private readonly _historySeeded: boolean | undefined
private readonly exitProofPromise: Promise<void>
private resolveExitProof!: () => void
constructor(opts: SessionOptions) {
this.sessionId = opts.sessionId
@@ -148,6 +150,9 @@ export class Session {
this.launchAgent = opts.launchAgent ?? null
this.subprocess = opts.subprocess
this.onSessionExit = opts.onExit
this.exitProofPromise = new Promise((resolve) => {
this.resolveExitProof = resolve
})
const size = normalizePtySize(opts.cols, opts.rows)
this.emulator = new HeadlessEmulator({
cols: size.cols,
@@ -202,6 +207,10 @@ export class Session {
return this._isTerminating
}
waitForExitProof(): Promise<void> {
return this._state === 'exited' ? Promise.resolve() : this.exitProofPromise
}
get pid(): number {
return this.subprocess.pid
}
@@ -653,6 +662,7 @@ export class Session {
this._exitCode = code
this._state = 'exited'
this.resolveExitProof()
// Why resume:false — the child is reaped, so there is nothing to unblock;
// only the failsafe timer must not outlive the session.
this.releaseProducerPause({ resume: false })
src/main/daemon/terminal-host.test.ts
+28
View File
@@ -128,6 +128,34 @@ describe('TerminalHost', () => {
expect(spawnFn).toHaveBeenCalledOnce()
})
it('retains a terminating same-id session until physical exit proof', async () => {
await host.createOrAttach({
sessionId: 'session-1',
cols: 80,
rows: 24,
streamClient: { onData: vi.fn(), onExit: vi.fn() }
})
const terminatingSubprocess = lastSubprocess
host.kill('session-1', { immediate: true })
const replacement = host.createOrAttach({
sessionId: 'session-1',
cols: 80,
rows: 24,
streamClient: { onData: vi.fn(), onExit: vi.fn() }
})
await Promise.resolve()
expect(spawnFn).toHaveBeenCalledOnce()
expect(terminatingSubprocess.forceKill).toHaveBeenCalledOnce()
expect(terminatingSubprocess.dispose).not.toHaveBeenCalled()
expect(host.listSessions()).toHaveLength(1)
terminatingSubprocess._onExitCb?.(0)
await expect(replacement).resolves.toMatchObject({ isNew: true })
expect(spawnFn).toHaveBeenCalledTimes(2)
})
it('returns snapshot when attaching to existing session', async () => {
await host.createOrAttach({
sessionId: 'session-1',
src/main/daemon/terminal-host.ts
+9 -8
View File
@@ -61,14 +61,15 @@ export class TerminalHost {
* already deliver them through shell launch arguments.
*/
async createOrAttach(opts: CreateOrAttachOptions): Promise<CreateOrAttachResult> {
const existing = this.sessions.get(opts.sessionId)
let existing = this.sessions.get(opts.sessionId)
// Why: a session that has been asked to terminate (kill() called but the
// subprocess hasn't exited yet) must not be reattached. Reattaching would
// hand the caller a handle that races with the in-flight exit, and any
// subsequent operation (write/kill/resize) would fail once the subprocess
// finally exits. Treat terminating sessions the same as fully-exited ones.
if (existing && existing.isAlive && !existing.isTerminating) {
// Why: replacing a terminating session would dispose its exit listener and
// retry timer before physical exit proof, orphaning the old native process.
while (existing?.isAlive && existing.isTerminating) {
await existing.waitForExitProof()
existing = this.sessions.get(opts.sessionId)
}
if (existing?.isAlive) {
assertPtyPaneIdentity(
opts.sessionId,
{ paneKey: existing.paneKey, tabId: existing.tabId },
@@ -89,7 +90,7 @@ export class TerminalHost {
}
}
// Clean up dead session if present
// Clean up only a physically exited session if its exit reaper has not run yet.
if (existing) {
existing.dispose()
this.sessions.delete(opts.sessionId)
src/main/ipc/pty.test.ts
+89
View File
@@ -3923,6 +3923,95 @@ describe('registerPtyHandlers', () => {
vi.useRealTimers()
})
it('blocks renderer same-id restore until startup teardown replay completes', async () => {
const startup = makeDeferred()
const shutdownGate = makeDeferred()
const spawn = vi.fn().mockResolvedValue({ id: 'daemon-pty' })
const shutdown = vi.fn(() => shutdownGate.promise)
const removePendingLocalPtyShutdown = vi.fn()
setLocalPtyProvider({
spawn,
shutdown,
onData: vi.fn(() => () => {}),
onReplay: vi.fn(() => () => {}),
onExit: vi.fn(() => () => {})
} as never)
handlers.clear()
registerPtyHandlers(
mainWindow as never,
undefined,
undefined,
undefined,
undefined,
{
getPendingLocalPtyShutdowns: () => [{ ptyId: 'daemon-pty', requestedAt: 1 }],
upsertPendingLocalPtyShutdown: vi.fn(),
removePendingLocalPtyShutdown
} as never,
{ awaitLocalPtyStartup: () => startup.promise }
)
const restore = handlers.get('pty:spawn')!(null, {
sessionId: 'daemon-pty',
cols: 80,
rows: 24
}) as Promise<unknown>
startup.resolve()
await expect(restore).rejects.toThrow('shutdown is still pending')
expect(shutdown).toHaveBeenCalledOnce()
expect(spawn).not.toHaveBeenCalled()
shutdownGate.resolve()
await vi.waitFor(() => expect(removePendingLocalPtyShutdown).toHaveBeenCalledOnce())
await expect(
handlers.get('pty:spawn')!(null, { sessionId: 'daemon-pty', cols: 80, rows: 24 })
).resolves.toMatchObject({ id: 'daemon-pty' })
expect(spawn).toHaveBeenCalledOnce()
})
it('blocks runtime same-id restore while startup teardown replay owns the PTY', async () => {
const startup = makeDeferred()
const shutdownGate = makeDeferred()
const spawn = vi.fn().mockResolvedValue({ id: 'daemon-pty' })
const shutdown = vi.fn(() => shutdownGate.promise)
const removePendingLocalPtyShutdown = vi.fn()
setLocalPtyProvider({
spawn,
shutdown,
onData: vi.fn(() => () => {}),
onReplay: vi.fn(() => () => {}),
onExit: vi.fn(() => () => {})
} as never)
const runtime = { setPtyController: vi.fn(), onPtyExit: vi.fn() }
handlers.clear()
registerPtyHandlers(
mainWindow as never,
runtime as never,
undefined,
undefined,
undefined,
{
getPendingLocalPtyShutdowns: () => [{ ptyId: 'daemon-pty', requestedAt: 1 }],
upsertPendingLocalPtyShutdown: vi.fn(),
removePendingLocalPtyShutdown
} as never,
{ awaitLocalPtyStartup: () => startup.promise }
)
const controller = runtime.setPtyController.mock.calls[0]?.[0] as {
spawn: (args: { sessionId: string; cols: number; rows: number }) => Promise<unknown>
}
const restore = controller.spawn({ sessionId: 'daemon-pty', cols: 80, rows: 24 })
startup.resolve()
await expect(restore).rejects.toThrow('shutdown is still pending')
expect(shutdown).toHaveBeenCalledOnce()
expect(spawn).not.toHaveBeenCalled()
shutdownGate.resolve()
await vi.waitFor(() => expect(removePendingLocalPtyShutdown).toHaveBeenCalledOnce())
})
it('does not let an in-flight old provider complete persisted shutdown ownership', async () => {
vi.useFakeTimers()
let rejectOldShutdown!: (error: Error) => void
src/main/ipc/pty.ts
+11
View File
@@ -195,6 +195,15 @@ const providerProcessListsInFlight = new WeakMap<IPtyProvider, Promise<PtyProces
const sshRetainedExitUnsubscribers = new Map<string, () => void>()
let retainedShutdownRetryTimer: ReturnType<typeof setTimeout> | null = null
const MAX_RETAINED_SHUTDOWN_BACKOFF_MS = 30_000
function assertPtySpawnHasNoRetainedShutdown(id: string | undefined): void {
if (id && (pendingLocalShutdownRetries.has(id) || pendingSshShutdownRetries.has(id))) {
// Why: replacing or attaching this exact id would strand the retained
// teardown owner on the older native process/provider generation.
throw new Error(`PTY "${id}" shutdown is still pending`)
}
}
const SYNTHETIC_KILL_EXIT_DUPLICATE_WINDOW_MS = 30_000
// Why: producer flow control changes terminal physics — a flooding shell now
// blocks on write instead of buffering in main. Kill switch: flip this one
@@ -3695,6 +3704,7 @@ export function registerPtyHandlers(
: undefined
}
assertPtySpawnHasNoRetainedShutdown(effectiveSessionAppId)
const existingPaneSpawn = materializedPaneKey
? paneSpawnReservationsByPaneKey.get(materializedPaneKey)
: undefined
@@ -4592,6 +4602,7 @@ export function registerPtyHandlers(
? (getSettings()?.terminalWindowsPowerShellImplementation ?? 'auto')
: undefined
}
assertPtySpawnHasNoRetainedShutdown(effectiveSessionAppId)
const existingPaneSpawn = reservationPaneKey
? paneSpawnReservationsByPaneKey.get(reservationPaneKey)
: undefined
src/main/ipc/runtime-terminal-close-retry-owner.test.ts
+56 -1
View File
@@ -1,4 +1,4 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { callRuntimeEnvironmentMock, getRuntimeEnvironmentStatusMock } = vi.hoisted(() => ({
callRuntimeEnvironmentMock: vi.fn(),
@@ -24,6 +24,11 @@ function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
}
describe('runtime terminal close retry ownership', () => {
beforeEach(() => {
callRuntimeEnvironmentMock.mockReset()
getRuntimeEnvironmentStatusMock.mockReset()
})
afterEach(() => {
initializeRuntimeTerminalCloseRetryOwner(
{ getPendingRuntimeTerminalCloses: () => [] } as never,
@@ -115,4 +120,54 @@ describe('runtime terminal close retry ownership', () => {
expect(remove).toHaveBeenCalledOnce()
expect(vi.getTimerCount()).toBe(0)
})
it('releases a same-runtime stale handle without another retry', async () => {
vi.useFakeTimers()
const remove = vi.fn()
initializeRuntimeTerminalCloseRetryOwner(
{
getPendingRuntimeTerminalCloses: () => [],
upsertPendingRuntimeTerminalClose: vi.fn(),
removePendingRuntimeTerminalClose: remove
} as never,
'/tmp/orca-runtime-owner-test'
)
getRuntimeEnvironmentStatusMock.mockResolvedValue({
id: 'status-a',
ok: true,
result: { runtimeId: 'runtime-a' },
_meta: { runtimeId: 'runtime-a' }
})
callRuntimeEnvironmentMock
.mockResolvedValueOnce({
id: 'close-transient',
ok: false,
error: { code: 'runtime_unavailable', message: 'runtime unavailable' },
_meta: { runtimeId: 'runtime-a' }
})
.mockResolvedValueOnce({
id: 'close-a',
ok: false,
error: { code: 'terminal_handle_stale', message: 'terminal_handle_stale' },
_meta: { runtimeId: 'runtime-a' }
})
await expect(
closeRuntimeTerminalWithRetryOwnership('environment-1', 'terminal-1', 'runtime-a')
).resolves.toMatchObject({ ok: false, error: { code: 'runtime_unavailable' } })
expect(vi.getTimerCount()).toBe(1)
await expect(
closeRuntimeTerminalWithRetryOwnership('environment-1', 'terminal-1', 'runtime-a')
).resolves.toMatchObject({
ok: true,
result: { close: false, reason: 'terminal_handle_stale' }
})
expect(vi.getTimerCount()).toBe(0)
await vi.advanceTimersByTimeAsync(60_000)
expect(callRuntimeEnvironmentMock).toHaveBeenCalledTimes(2)
expect(remove).toHaveBeenCalledWith('environment-1', 'terminal-1')
expect(vi.getTimerCount()).toBe(0)
})
})
src/main/ipc/runtime-terminal-close-retry-owner.ts
+27 -12
View File
@@ -34,6 +34,15 @@ function isCurrentOwner(close: RetainedTerminalClose): boolean {
)
}
function release(close: RetainedTerminalClose): void {
if (!isCurrentOwner(close)) {
return
}
retainedCloses.delete(key(close.environmentId, close.handle))
store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle)
schedule()
}
function schedule(): void {
if (retryTimer) {
clearTimeout(retryTimer)
@@ -67,10 +76,12 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
if (close.inFlight) {
return close.inFlight
}
const closeKey = key(close.environmentId, close.handle)
const noOpResponse = (
runtimeId?: string | null,
reason: 'retry_owner_replaced' | 'environment_removed' = 'retry_owner_replaced'
reason:
| 'retry_owner_replaced'
| 'environment_removed'
| 'terminal_handle_stale' = 'retry_owner_replaced'
) => ({
id: 'terminal.close',
ok: true as const,
@@ -87,8 +98,7 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
}
const currentRuntimeId = status._meta.runtimeId
if (close.runtimeId && close.runtimeId !== currentRuntimeId) {
retainedCloses.delete(closeKey)
store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle)
release(close)
return {
id: 'terminal.close',
ok: true as const,
@@ -117,8 +127,7 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
if (!response.ok) {
const responseRuntimeId = response._meta?.runtimeId
if (close.runtimeId && responseRuntimeId && responseRuntimeId !== close.runtimeId) {
retainedCloses.delete(closeKey)
store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle)
release(close)
return {
id: 'terminal.close',
ok: true as const,
@@ -126,12 +135,19 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
_meta: { runtimeId: responseRuntimeId }
}
}
if (
response.error.code === 'terminal_handle_stale' &&
close.runtimeId &&
responseRuntimeId === close.runtimeId
) {
// Why: the same runtime generation is authoritative for its handle.
// A stale handle proves this exact retained resource no longer exists.
release(close)
return noOpResponse(responseRuntimeId, 'terminal_handle_stale')
}
throw Object.assign(new Error(response.error.message), { response })
}
retainedCloses.delete(closeKey)
if (typeof store?.removePendingRuntimeTerminalClose === 'function') {
store.removePendingRuntimeTerminalClose(close.environmentId, close.handle)
}
release(close)
return response
})
.catch((error: unknown) => {
@@ -140,8 +156,7 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
}
if (error instanceof RuntimeEnvironmentStoreError && error.code === 'invalid_argument') {
// Why: persisted ownership for a removed environment can never become reachable again.
retainedCloses.delete(closeKey)
store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle)
release(close)
return noOpResponse(undefined, 'environment_removed')
}
close.attempts += 1