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

fix terminal close retry persistence ownership

Browse Source
This commit is contained in:
Brennan Benson
2026-07-13 23:09:04 -07:00
parent 9659bd0ec9
commit 74239e7f56
4 changed files with 183 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/ipc/pty.test.ts
+43 -2
View File
@@ -3549,7 +3549,8 @@ describe('registerPtyHandlers', () => {
},
markSshRemotePtyShutdownRequested: () => {
throw new Error('SSH intent disk full')
}
},
markSshRemotePtyLease: vi.fn()
} as never)
try {
@@ -3575,6 +3576,42 @@ describe('registerPtyHandlers', () => {
}
})
it('retries durable local intent removal without repeating provider shutdown', async () => {
vi.useFakeTimers()
const shutdown = vi.fn().mockResolvedValue(undefined)
const remove = vi
.fn()
.mockImplementationOnce(() => {
throw new Error('intent removal disk full')
})
.mockReturnValue(undefined)
setLocalPtyProvider({
shutdown,
onData: vi.fn(() => () => {}),
onReplay: vi.fn(() => () => {}),
onExit: vi.fn(() => () => {})
} as never)
handlers.clear()
registerPtyHandlers(mainWindow as never, undefined, undefined, undefined, undefined, {
getPendingLocalPtyShutdowns: () => [],
upsertPendingLocalPtyShutdown: vi.fn(),
removePendingLocalPtyShutdown: remove
} as never)
await expect(handlers.get('pty:kill')!(null, { id: 'local-remove-fail' })).rejects.toThrow(
'intent removal disk full'
)
expect(shutdown).toHaveBeenCalledOnce()
expect(remove).toHaveBeenCalledOnce()
await vi.advanceTimersByTimeAsync(250)
expect(shutdown).toHaveBeenCalledOnce()
expect(remove).toHaveBeenCalledTimes(2)
await vi.advanceTimersByTimeAsync(30_000)
expect(shutdown).toHaveBeenCalledOnce()
expect(remove).toHaveBeenCalledTimes(2)
})
it('keeps accepted local kills until provider readback proves physical exit', async () => {
vi.useFakeTimers()
const shutdown = vi.fn().mockResolvedValue(undefined)
@@ -3994,6 +4031,9 @@ describe('registerPtyHandlers', () => {
const fallbackShutdown = vi.fn().mockResolvedValue(undefined)
const daemonShutdown = vi.fn().mockResolvedValue(undefined)
const removePendingLocalPtyShutdown = vi.fn()
const upsertPendingLocalPtyShutdown = vi.fn(() => {
throw new Error('startup replay must not rewrite durable intent')
})
setLocalPtyProvider(makeProvider(fallbackShutdown))
handlers.clear()
registerPtyHandlers(
@@ -4004,7 +4044,7 @@ describe('registerPtyHandlers', () => {
undefined,
{
getPendingLocalPtyShutdowns: () => [{ ptyId: 'daemon-pty', requestedAt: 1 }],
upsertPendingLocalPtyShutdown: vi.fn(),
upsertPendingLocalPtyShutdown,
removePendingLocalPtyShutdown
} as never,
{ awaitLocalPtyStartup: () => startup.promise }
@@ -4017,6 +4057,7 @@ describe('registerPtyHandlers', () => {
await vi.advanceTimersByTimeAsync(0)
expect(fallbackShutdown).not.toHaveBeenCalled()
expect(upsertPendingLocalPtyShutdown).not.toHaveBeenCalled()
expect(daemonShutdown).toHaveBeenCalledOnce()
expect(removePendingLocalPtyShutdown).toHaveBeenCalledWith('daemon-pty')
await vi.advanceTimersByTimeAsync(30_000)
src/main/ipc/pty.ts
+26 -6
View File
@@ -184,6 +184,8 @@ type RetainedPtyShutdown = {
durable: boolean
attempts: number
shutdownAccepted: boolean
providerShutdownComplete: boolean
providerExitObserved: boolean
requireExitProof: boolean
nextRetryAt: number
inFlight: Promise<void> | null
@@ -593,7 +595,7 @@ function finishPtyShutdown(
): void {
clearProviderPtyState(id)
if (connectionId) {
store?.markSshRemotePtyLease(connectionId, id, 'terminated')
store?.markSshRemotePtyLease?.(connectionId, id, 'terminated')
}
ptyOwnership.delete(id)
markClaudePtyExited(id)
@@ -643,11 +645,11 @@ function completeRetainedPtyShutdown(
if (ownerMap.get(retained.id) !== retained) {
return
}
ownerMap.delete(retained.id)
finishPtyShutdown(retained.id, retained.connectionId, retained.store ?? undefined)
if (!retained.connectionId) {
retained.store?.removePendingLocalPtyShutdown?.(retained.id)
}
finishPtyShutdown(retained.id, retained.connectionId, retained.store ?? undefined)
ownerMap.delete(retained.id)
if (!providerExitObserved) {
notifyRetainedPtyExit?.(retained.id)
}
@@ -688,6 +690,10 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
retained.provider = currentProvider
retained.lastError = null
const attempt = (async () => {
if (retained.providerShutdownComplete) {
completeRetainedPtyShutdown(retained, retained.providerExitObserved)
return
}
if (retained.shutdownAccepted) {
const stopped = await verifyPtyStopped(currentProvider, retained.id, retained.options)
if (retained.provider !== currentProvider) {
@@ -697,6 +703,8 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
deferRetainedPtyShutdownRetry(retained)
return
}
retained.providerShutdownComplete = true
retained.providerExitObserved = false
completeRetainedPtyShutdown(retained, false)
return
}
@@ -719,6 +727,8 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
return
}
if (providerAlreadyGone) {
retained.providerShutdownComplete = true
retained.providerExitObserved = false
completeRetainedPtyShutdown(retained, false)
return
}
@@ -740,6 +750,8 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
return
}
}
retained.providerShutdownComplete = true
retained.providerExitObserved = providerExitObserved
completeRetainedPtyShutdown(retained, providerExitObserved)
})()
.catch((error: unknown) => {
@@ -868,7 +880,8 @@ function retainLocalPtyShutdown(
id: string,
options: PtyShutdownOptions,
attempts = 0,
deferUntilProviderReady = false
deferUntilProviderReady = false,
persist = true
): RetainedPtyShutdown {
let retained = pendingLocalShutdownRetries.get(id)
if (!retained || ptyShutdownIdentityConflicts(retained.options, options)) {
@@ -883,6 +896,8 @@ function retainLocalPtyShutdown(
durable: true,
attempts,
shutdownAccepted: false,
providerShutdownComplete: false,
providerExitObserved: false,
requireExitProof: false,
nextRetryAt: deferUntilProviderReady
? Number.POSITIVE_INFINITY
@@ -899,7 +914,7 @@ function retainLocalPtyShutdown(
// Why: disk failure must not strand the live process merely because its
// crash-durable intent could not be refreshed.
scheduleRetainedShutdownRetries()
if (typeof activePtyStore?.upsertPendingLocalPtyShutdown === 'function') {
if (persist && typeof activePtyStore?.upsertPendingLocalPtyShutdown === 'function') {
activePtyStore.upsertPendingLocalPtyShutdown({
ptyId: id,
...(retained.options.expectedPaneKey
@@ -942,6 +957,8 @@ function retainSshPtyShutdown(
durable: persisted,
attempts: 0,
shutdownAccepted: false,
providerShutdownComplete: false,
providerExitObserved: false,
requireExitProof: false,
nextRetryAt: provider ? 0 : Number.POSITIVE_INFINITY,
inFlight: null,
@@ -1621,6 +1638,8 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider
durable: true,
attempts: 0,
shutdownAccepted: false,
providerShutdownComplete: false,
providerExitObserved: false,
requireExitProof: false,
nextRetryAt: 0,
inFlight: null,
@@ -2097,7 +2116,8 @@ export function registerPtyHandlers(
...(request.expectedTabId ? { expectedTabId: request.expectedTabId } : {})
},
0,
Boolean(startupReady)
Boolean(startupReady),
false
)
const retryWithReadyProvider = (): void => {
if (pendingLocalShutdownRetries.get(retained.id) !== retained) {
src/main/ipc/runtime-terminal-close-retry-owner.test.ts
+75
View File
@@ -206,4 +206,79 @@ describe('runtime terminal close retry ownership', () => {
expect(remove).toHaveBeenCalledWith('environment-1', 'terminal-1')
expect(vi.getTimerCount()).toBe(0)
})
it('hydrates persisted closes without rewriting their durable intent', async () => {
vi.useFakeTimers()
const upsert = vi.fn(() => {
throw new Error('startup replay must not rewrite durable intent')
})
initializeRuntimeTerminalCloseRetryOwner(
{
getPendingRuntimeTerminalCloses: () => [
{ environmentId: 'environment-1', handle: 'terminal-1', runtimeId: 'runtime-a' }
],
upsertPendingRuntimeTerminalClose: upsert,
removePendingRuntimeTerminalClose: vi.fn()
} as never,
'/tmp/orca-runtime-owner-test'
)
getRuntimeEnvironmentStatusMock.mockResolvedValue({
id: 'status-a',
ok: true,
result: { runtimeId: 'runtime-a' },
_meta: { runtimeId: 'runtime-a' }
})
callRuntimeEnvironmentMock.mockResolvedValue({
id: 'close-a',
ok: true,
result: { close: true },
_meta: { runtimeId: 'runtime-a' }
})
await vi.advanceTimersByTimeAsync(0)
expect(upsert).not.toHaveBeenCalled()
expect(callRuntimeEnvironmentMock).toHaveBeenCalledOnce()
})
it('retries durable removal without repeating an accepted runtime close', async () => {
vi.useFakeTimers()
const remove = vi
.fn()
.mockImplementationOnce(() => {
throw new Error('runtime removal disk full')
})
.mockReturnValue(undefined)
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.mockResolvedValue({
id: 'close-a',
ok: true,
result: { close: true },
_meta: { runtimeId: 'runtime-a' }
})
await expect(
closeRuntimeTerminalWithRetryOwnership('environment-1', 'terminal-1', 'runtime-a')
).rejects.toThrow('runtime removal disk full')
expect(callRuntimeEnvironmentMock).toHaveBeenCalledOnce()
expect(remove).toHaveBeenCalledOnce()
await vi.advanceTimersByTimeAsync(250)
expect(callRuntimeEnvironmentMock).toHaveBeenCalledOnce()
expect(remove).toHaveBeenCalledTimes(2)
expect(vi.getTimerCount()).toBe(0)
})
})
src/main/ipc/runtime-terminal-close-retry-owner.ts
+39 -33
View File
@@ -12,6 +12,7 @@ type RetainedTerminalClose = {
handle: string
runtimeId: string | null
attempts: number
closeComplete: boolean
nextRetryAt: number
inFlight: Promise<RuntimeRpcResponse<unknown>> | null
}
@@ -38,8 +39,9 @@ function release(close: RetainedTerminalClose): void {
if (!isCurrentOwner(close)) {
return
}
retainedCloses.delete(key(close.environmentId, close.handle))
close.closeComplete = true
store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle)
retainedCloses.delete(key(close.environmentId, close.handle))
schedule()
}
@@ -88,38 +90,40 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
result: { close: false, reason },
_meta: { runtimeId: runtimeId ?? close.runtimeId ?? 'unknown-runtime' }
})
const request = getRuntimeEnvironmentStatus(userDataPath, close.environmentId)
.then((status) => {
if (!isCurrentOwner(close)) {
return noOpResponse(status._meta?.runtimeId)
}
if (!status.ok) {
throw Object.assign(new Error(status.error.message), { response: status })
}
const currentRuntimeId = status._meta.runtimeId
if (close.runtimeId && close.runtimeId !== currentRuntimeId) {
release(close)
return {
id: 'terminal.close',
ok: true as const,
result: { close: false, reason: 'runtime_replaced' },
_meta: { runtimeId: currentRuntimeId }
const closeAttempt = close.closeComplete
? Promise.resolve(noOpResponse(close.runtimeId))
: getRuntimeEnvironmentStatus(userDataPath, close.environmentId).then((status) => {
if (!isCurrentOwner(close)) {
return noOpResponse(status._meta?.runtimeId)
}
}
if (!close.runtimeId) {
close.runtimeId = currentRuntimeId
store?.upsertPendingRuntimeTerminalClose?.({
environmentId: close.environmentId,
handle: close.handle,
runtimeId: currentRuntimeId,
requestedAt: Date.now()
if (!status.ok) {
throw Object.assign(new Error(status.error.message), { response: status })
}
const currentRuntimeId = status._meta.runtimeId
if (close.runtimeId && close.runtimeId !== currentRuntimeId) {
release(close)
return {
id: 'terminal.close',
ok: true as const,
result: { close: false, reason: 'runtime_replaced' },
_meta: { runtimeId: currentRuntimeId }
}
}
if (!close.runtimeId) {
close.runtimeId = currentRuntimeId
store?.upsertPendingRuntimeTerminalClose?.({
environmentId: close.environmentId,
handle: close.handle,
runtimeId: currentRuntimeId,
requestedAt: Date.now()
})
}
return callRuntimeEnvironment(userDataPath, close.environmentId, 'terminal.close', {
terminal: close.handle,
expectedRuntimeId: close.runtimeId
})
}
return callRuntimeEnvironment(userDataPath, close.environmentId, 'terminal.close', {
terminal: close.handle,
expectedRuntimeId: close.runtimeId
})
})
const request = closeAttempt
.then((response) => {
if (!isCurrentOwner(close)) {
return noOpResponse(response._meta?.runtimeId)
@@ -187,7 +191,8 @@ function attempt(close: RetainedTerminalClose): Promise<RuntimeRpcResponse<unkno
function retain(
environmentId: string,
handle: string,
runtimeId?: string | null
runtimeId?: string | null,
persist = true
): RetainedTerminalClose {
const closeKey = key(environmentId, handle)
const incomingRuntimeId = runtimeId ?? null
@@ -204,11 +209,12 @@ function retain(
handle,
runtimeId: incomingRuntimeId,
attempts: 0,
closeComplete: false,
nextRetryAt: 0,
inFlight: null
}
retainedCloses.set(closeKey, close)
if (typeof store?.upsertPendingRuntimeTerminalClose === 'function') {
if (persist && typeof store?.upsertPendingRuntimeTerminalClose === 'function') {
try {
store.upsertPendingRuntimeTerminalClose({
environmentId,
@@ -257,7 +263,7 @@ export function initializeRuntimeTerminalCloseRetryOwner(
? store.getPendingRuntimeTerminalCloses()
: []
for (const close of persisted) {
retain(close.environmentId, close.handle, close.runtimeId)
retain(close.environmentId, close.handle, close.runtimeId, false)
}
schedule()
}