fix(updater): isolate shutdown checkpoint retries

Reset full-session staging retry state when a shutdown attempt is abandoned, and route Terminal-less closes through the same scoped synthetic checkpoint as mounted workspaces. Keep arbitrary thrown-value diagnostics non-throwing and localize the quit failure toast.
This commit is contained in:
Brennan Benson
2026-08-25 15:18:40 -07:00
parent 200c76fb62
commit 99ad116232
11 changed files with 167 additions and 62 deletions
@@ -29,7 +29,7 @@ describe('createShutdownCheckpointPersist', () => {
}
})
expect(createShutdownCheckpointPersist(deps)).not.toThrow()
expect(createShutdownCheckpointPersist(deps).run).not.toThrow()
// The full snapshot must still be staged — a weaker resume record for done
// panes is strictly better than blocking the update.
expect(deps.stageBeforeUnloadSync).toHaveBeenCalledWith({
@@ -39,11 +39,24 @@ describe('createShutdownCheckpointPersist', () => {
vi.restoreAllMocks()
})
it('keeps sleeping-capture diagnostics non-throwing for unstringifiable values', () => {
vi.spyOn(console, 'error').mockImplementation(() => {})
const deps = makeDeps({
captureSleepingAgentSessions: () => {
throw Object.create(null)
}
})
expect(createShutdownCheckpointPersist(deps).run).not.toThrow()
expect(deps.stageBeforeUnloadSync).toHaveBeenCalledTimes(1)
vi.restoreAllMocks()
})
it('keeps the first full-staging failure a visible, retryable error (STA-5505)', () => {
const stageBeforeUnloadSync = vi.fn(() => {
throw new Error('sync IPC staging failed')
})
const persist = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
const { run: persist } = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
// A transient failure must not silently cost the full snapshot on attempt one.
expect(persist).toThrow('sync IPC staging failed')
@@ -57,7 +70,7 @@ describe('createShutdownCheckpointPersist', () => {
throw new Error('sync IPC staging failed')
}
})
const persist = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
const { run: persist } = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
expect(persist).toThrow('sync IPC staging failed')
expect(persist).not.toThrow()
@@ -77,7 +90,7 @@ describe('createShutdownCheckpointPersist', () => {
throw new Error('transient staging failure')
}
})
const persist = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
const { run: persist } = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
expect(persist).toThrow('transient staging failure')
expect(persist).not.toThrow()
@@ -94,7 +107,7 @@ describe('createShutdownCheckpointPersist', () => {
throw new Error('sync IPC staging failed')
})
})
const persist = createShutdownCheckpointPersist(deps)
const { run: persist } = createShutdownCheckpointPersist(deps)
expect(persist).toThrow('sync IPC staging failed')
expect(persist).toThrow('sync IPC staging failed')
@@ -107,7 +120,7 @@ describe('createShutdownCheckpointPersist', () => {
throw new Error('sync IPC staging failed')
})
})
const persist = createShutdownCheckpointPersist(deps)
const { run: persist } = createShutdownCheckpointPersist(deps)
expect(persist).toThrow('sync IPC staging failed')
expect(persist).toThrow('sync IPC staging failed')
@@ -124,7 +137,7 @@ describe('createShutdownCheckpointPersist', () => {
})
})
expect(createShutdownCheckpointPersist(deps)).toThrow('durable staging failed')
expect(createShutdownCheckpointPersist(deps).run).toThrow('durable staging failed')
vi.restoreAllMocks()
})
@@ -136,7 +149,7 @@ describe('createShutdownCheckpointPersist', () => {
}
})
expect(createShutdownCheckpointPersist(deps)).not.toThrow()
expect(createShutdownCheckpointPersist(deps).run).not.toThrow()
expect(deps.stageBeforeUnloadSync).toHaveBeenCalledTimes(1)
expect(deps.stageBeforeUnloadSync).toHaveBeenCalledWith({
sessions: [],
@@ -153,14 +166,14 @@ describe('createShutdownCheckpointPersist', () => {
}
})
expect(createShutdownCheckpointPersist(deps)).toThrow('snapshot build failed')
expect(createShutdownCheckpointPersist(deps).run).toThrow('snapshot build failed')
expect(deps.stageBeforeUnloadSync).not.toHaveBeenCalled()
})
it('skips capture and stages empty sessions before hydration completes', () => {
const deps = makeDeps({ shouldCaptureSession: () => false })
createShutdownCheckpointPersist(deps)()
createShutdownCheckpointPersist(deps).run()
expect(deps.captureTerminalBuffers).not.toHaveBeenCalled()
expect(deps.captureSleepingAgentSessions).not.toHaveBeenCalled()
@@ -169,4 +182,16 @@ describe('createShutdownCheckpointPersist', () => {
ui: { activeView: 'workspace' }
})
})
it('forgets a failed attempt when an aborted shutdown resets the lifecycle', () => {
const stageBeforeUnloadSync = vi.fn(() => {
throw new Error('sync IPC staging failed')
})
const persist = createShutdownCheckpointPersist(makeDeps({ stageBeforeUnloadSync }))
expect(persist.run).toThrow('sync IPC staging failed')
persist.reset()
expect(persist.run).toThrow('sync IPC staging failed')
expect(stageBeforeUnloadSync).toHaveBeenCalledTimes(2)
})
})
@@ -1,4 +1,5 @@
import type { PersistedUIState } from '../../../shared/persisted-ui-state-types'
import { formatShutdownCheckpointFailureReason } from '../../../shared/renderer-shutdown-events'
import type { WorkspaceSessionHostSnapshot } from '../lib/workspace-session-host-persistence'
import { recordRendererCrashBreadcrumb } from '../lib/crash-breadcrumb-recorder'
@@ -21,16 +22,23 @@ export type ShutdownCheckpointPersistDeps = {
stageBeforeUnloadSync: (args: ShutdownCheckpointStageArgs) => void
}
/** Returns the synchronous body of the shutdown checkpoint: capture renderer-owned
* state, then stage everything durable through one main-process call. Throwing
* fails the checkpoint and blocks the restart, so only unstageable data may throw.
export type ShutdownCheckpointPersist = {
run: () => void
reset: () => void
}
/** Returns the shutdown checkpoint attempt lifecycle. Running it captures renderer-owned
* state, then stages everything durable through one main-process call; reset discards
* retry state when that shutdown attempt is abandoned. Only unstageable data may throw.
*
* A factory rather than a bare function so full-session staging failures can stay
* visible-and-retryable on the first attempt and only degrade on a repeat: a
* transient failure gets its retry, a deterministic one can't strand the user. */
export function createShutdownCheckpointPersist(deps: ShutdownCheckpointPersistDeps): () => void {
export function createShutdownCheckpointPersist(
deps: ShutdownCheckpointPersistDeps
): ShutdownCheckpointPersist {
let fullStagingFailedOnPriorAttempt = false
return (): void => {
const run = (): void => {
const shouldCaptureSession = deps.shouldCaptureSession()
if (shouldCaptureSession) {
deps.captureTerminalBuffers()
@@ -43,7 +51,7 @@ export function createShutdownCheckpointPersist(deps: ShutdownCheckpointPersistD
// alternative (no update, or a SIGKILL'd quit losing the whole snapshot).
console.error('[app] Sleeping-agent quit capture failed; continuing checkpoint', error)
recordRendererCrashBreadcrumb('renderer_shutdown_sleeping_capture_failed', {
message: error instanceof Error ? error.message : String(error)
message: formatShutdownCheckpointFailureReason(error)
})
}
}
@@ -92,4 +100,10 @@ export function createShutdownCheckpointPersist(deps: ShutdownCheckpointPersistD
}
fullStagingFailedOnPriorAttempt = false
}
return {
run,
reset: () => {
fullStagingFailedOnPriorAttempt = false
}
}
}
@@ -121,42 +121,44 @@ export function useAppSessionPersistence(): void {
// two firings, PTY exit events can arrive and unmount TerminalPanes,
// emptying shutdownBufferCaptures. The guard prevents the second call
// from overwriting the good session data with an empty snapshot.
const shutdownCheckpoint = createShutdownCheckpointGuard(
createShutdownCheckpointPersist({
shouldCaptureSession: () => shouldPersistWorkspaceSession(useAppStore.getState()),
captureTerminalBuffers: () => {
for (const capture of shutdownBufferCaptures.values()) {
try {
capture({ includeLocalBuffers: false })
} catch {
// Don't let one pane's failure block the rest.
}
const shutdownCheckpointPersist = createShutdownCheckpointPersist({
shouldCaptureSession: () => shouldPersistWorkspaceSession(useAppStore.getState()),
captureTerminalBuffers: () => {
for (const capture of shutdownBufferCaptures.values()) {
try {
capture({ includeLocalBuffers: false })
} catch {
// Don't let one pane's failure block the rest.
}
},
// Why: agent provider session ids live only in agentStatusByPaneKey,
// which is in-memory. Capture them into the persisted sleeping-session
// map so a daemon/session death while the app is closed can still
// cold-restore via the agent's resume command (#5232).
captureSleepingAgentSessions: () =>
useAppStore.getState().captureAllSleepingAgentSessions('quit'),
// Why: re-read state after capture() calls populated scrollback buffers
// into the store via Zustand setters. The shouldCaptureSession read is
// only for the gating flags and would miss those updates.
buildSessionSnapshots: () => {
const freshState = useAppStore.getState()
return buildWorkspaceSessionHostSnapshots(
buildWorkspaceSessionPayload(freshState),
freshState
)
},
buildUiPatch: () => buildActiveViewUnloadPatch(useAppStore.getState()),
hasDirtyOpenFiles: () => useAppStore.getState().openFiles.some((file) => file.isDirty),
// Why: an app-level quit degrades too — the pre-fix alternative was a quit
// the user could only complete with SIGKILL, which loses strictly more (#15352).
isDegradableShutdownInProgress: () =>
isIntentionalAppRestartInProgress() || isWindowCloseCheckpointInProgress(),
stageBeforeUnloadSync: (args) => window.api.app.stageBeforeUnloadSync(args)
})
}
},
// Why: agent provider session ids live only in agentStatusByPaneKey,
// which is in-memory. Capture them into the persisted sleeping-session
// map so a daemon/session death while the app is closed can still
// cold-restore via the agent's resume command (#5232).
captureSleepingAgentSessions: () =>
useAppStore.getState().captureAllSleepingAgentSessions('quit'),
// Why: re-read state after capture() calls populated scrollback buffers
// into the store via Zustand setters. The shouldCaptureSession read is
// only for the gating flags and would miss those updates.
buildSessionSnapshots: () => {
const freshState = useAppStore.getState()
return buildWorkspaceSessionHostSnapshots(
buildWorkspaceSessionPayload(freshState),
freshState
)
},
buildUiPatch: () => buildActiveViewUnloadPatch(useAppStore.getState()),
hasDirtyOpenFiles: () => useAppStore.getState().openFiles.some((file) => file.isDirty),
// Why: an app-level quit degrades too — the pre-fix alternative was a quit
// the user could only complete with SIGKILL, which loses strictly more (#15352).
isDegradableShutdownInProgress: () =>
isIntentionalAppRestartInProgress() || isWindowCloseCheckpointInProgress(),
stageBeforeUnloadSync: (args) => window.api.app.stageBeforeUnloadSync(args)
})
const shutdownCheckpoint = createShutdownCheckpointGuard(
shutdownCheckpointPersist.run,
shutdownCheckpointPersist.reset
)
const persistBeforeUnload = createShutdownCheckpointBeforeUnloadHandler(shutdownCheckpoint)
window.addEventListener('beforeunload', persistBeforeUnload)
+7 -3
View File
@@ -488,7 +488,7 @@ describe('renderer startup runtime routing', () => {
it('checkpoints activeView and all session snapshots through one beforeunload handler (#9002)', () => {
const source = readSource(SESSION_PERSISTENCE_PATH)
const checkpointStart = source.indexOf(
'const shutdownCheckpoint = createShutdownCheckpointGuard('
'const shutdownCheckpointPersist = createShutdownCheckpointPersist({'
)
const checkpointEnd = source.indexOf(
'const persistBeforeUnload = createShutdownCheckpointBeforeUnloadHandler(shutdownCheckpoint)',
@@ -498,9 +498,11 @@ describe('renderer startup runtime routing', () => {
expect(checkpointEnd).toBeGreaterThan(checkpointStart)
const checkpointBlock = source.slice(checkpointStart, checkpointEnd)
expect(checkpointBlock).toContain('createShutdownCheckpointPersist({')
expect(checkpointBlock).toContain(
'buildWorkspaceSessionHostSnapshots(\n buildWorkspaceSessionPayload(freshState),\n freshState\n )'
'const shutdownCheckpointPersist = createShutdownCheckpointPersist({'
)
expect(checkpointBlock).toContain(
'buildWorkspaceSessionHostSnapshots(\n buildWorkspaceSessionPayload(freshState),\n freshState\n )'
)
expect(checkpointBlock).toContain('buildUiPatch: () => buildActiveViewUnloadPatch(')
// Why pin the exact gate: the degrade tiers must arm only for intentional
@@ -514,6 +516,8 @@ describe('renderer startup runtime routing', () => {
expect(checkpointBlock).toContain(
'stageBeforeUnloadSync: (args) => window.api.app.stageBeforeUnloadSync(args)'
)
expect(checkpointBlock).toContain('shutdownCheckpointPersist.run')
expect(checkpointBlock).toContain('shutdownCheckpointPersist.reset')
expect(source).toContain(
'window.addEventListener(ORCA_APP_RESTART_ABORTED_EVENT, shutdownCheckpoint.reset)'
)
+7 -1
View File
@@ -539,7 +539,13 @@ function Terminal(): React.JSX.Element | null {
// no reason — its deferred dialog flow already gives the user a surface.
const reason = consumeShutdownCheckpointFailureReason()
if (reason) {
toast.error(`Quit canceled: the session snapshot could not be saved (${reason}).`)
toast.error(
translate(
'auto.components.Terminal.quitSnapshotSaveFailed',
'Quit canceled: the session snapshot could not be saved ({{value0}}).',
{ value0: reason }
)
)
}
return
}
@@ -20,9 +20,11 @@ describe('window-close-request-coordinator', () => {
confirmWindowClose.mockClear()
// Why: dispatch falls back to the preload bridge when no rich handler is
// registered; stub just the surface it touches.
;(
globalThis as unknown as { window: { api: { ui: { confirmWindowClose: () => void } } } }
).window = { api: { ui: { confirmWindowClose } } }
const windowTarget = new EventTarget() as EventTarget & {
api: { ui: { confirmWindowClose: () => void } }
}
windowTarget.api = { ui: { confirmWindowClose } }
;(globalThis as unknown as { window: typeof windowTarget }).window = windowTarget
})
afterEach(() => {
@@ -55,6 +57,20 @@ describe('window-close-request-coordinator', () => {
expect(confirmWindowClose).toHaveBeenCalledTimes(1)
})
it('runs the Terminal-less close checkpoint in the degradable close scope', async () => {
const beforeUnload = vi.fn((event: Event) => {
expect(isWindowCloseCheckpointInProgress()).toBe(true)
event.preventDefault()
})
window.addEventListener('beforeunload', beforeUnload, { once: true })
await dispatchWindowCloseRequest({ isQuitting: true })
expect(beforeUnload).toHaveBeenCalledTimes(1)
expect(confirmWindowClose).not.toHaveBeenCalled()
expect(isWindowCloseCheckpointInProgress()).toBe(false)
})
it('delegates to the rich handler and does NOT confirm directly when one is registered', async () => {
const handler = vi.fn()
setWindowCloseRequestHandler(handler)
@@ -86,5 +86,10 @@ export async function dispatchWindowCloseRequest(data: { isQuitting: boolean }):
activeHandler(data)
return
}
window.api.ui.confirmWindowClose()
const accepted = runWithWindowCloseCheckpointScope(() =>
window.dispatchEvent(new Event('beforeunload', { cancelable: true }))
)
if (accepted) {
window.api.ui.confirmWindowClose()
}
}
+2 -1
View File
@@ -2099,7 +2099,8 @@
"61ed600d29": "\"{{value0}}\" has unsaved changes. Do you want to save before closing?",
"cdc9ac4b2d": "editor",
"e57db40c11": "Could not build launch command for {{value0}}.",
"5b2c1a9e44": "No agent CLI detected — install one or pick a default agent in Settings."
"5b2c1a9e44": "No agent CLI detected — install one or pick a default agent in Settings.",
"quitSnapshotSaveFailed": "Quit canceled: the session snapshot could not be saved ({{value0}})."
},
"TerminalSearch": {
"db234b7519": "Close",
@@ -84,6 +84,25 @@ describe('createShutdownCheckpointGuard', () => {
expect(consumeShutdownCheckpointFailureReason()).toBeNull()
})
it('keeps failure reporting non-throwing for unstringifiable thrown values', () => {
vi.spyOn(console, 'error').mockImplementation(() => {})
const guard = createShutdownCheckpointGuard(() => {
throw Object.create(null)
})
expect(guard.persistOnce()).toBe(false)
expect(consumeShutdownCheckpointFailureReason()).toBe('Unknown shutdown checkpoint failure')
})
it('resets state owned by the persist attempt lifecycle', () => {
const resetPersistAttempt = vi.fn()
const guard = createShutdownCheckpointGuard(vi.fn(), resetPersistAttempt)
guard.reset()
expect(resetPersistAttempt).toHaveBeenCalledTimes(1)
})
it('reports checkpoint failure separately from the unload verdict', () => {
const eventTarget = new EventTarget()
const failed = vi.fn()
@@ -1,5 +1,6 @@
import {
clearShutdownCheckpointFailureReason,
formatShutdownCheckpointFailureReason,
ORCA_RENDERER_SHUTDOWN_CHECKPOINT_FAILED_EVENT,
ORCA_RENDERER_UNLOAD_PREVENTED_EVENT,
publishShutdownCheckpointFailureReason
@@ -15,12 +16,15 @@ export type ShutdownCheckpointGuard = {
// build behind an error that names the symptom while the cause is swallowed (STA-5505).
function reportShutdownCheckpointFailure(error: unknown): void {
console.error('[app] Shutdown checkpoint persist failed:', error)
const message = error instanceof Error ? error.message : String(error)
const message = formatShutdownCheckpointFailureReason(error)
publishShutdownCheckpointFailureReason(message)
recordRendererCrashBreadcrumb('renderer_shutdown_checkpoint_failed', { message })
}
export function createShutdownCheckpointGuard(persist: () => void): ShutdownCheckpointGuard {
export function createShutdownCheckpointGuard(
persist: () => void,
resetPersistAttempt?: () => void
): ShutdownCheckpointGuard {
let persisted = false
return {
persistOnce(): boolean {
@@ -41,6 +45,7 @@ export function createShutdownCheckpointGuard(persist: () => void): ShutdownChec
},
reset(): void {
persisted = false
resetPersistAttempt?.()
}
}
}
+8
View File
@@ -9,6 +9,14 @@ export const ORCA_RENDERER_SHUTDOWN_CHECKPOINT_FAILED_EVENT =
export const ORCA_SHUTDOWN_CHECKPOINT_FAILURE_REASON_ATTRIBUTE =
'data-orca-shutdown-checkpoint-failure'
export function formatShutdownCheckpointFailureReason(error: unknown): string {
try {
return String(error instanceof Error ? error.message : error)
} catch {
return 'Unknown shutdown checkpoint failure'
}
}
export function publishShutdownCheckpointFailureReason(reason: string): void {
try {
globalThis.document?.documentElement?.setAttribute(