mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(quit): degrade and surface checkpoint-vetoed app quits (#15352)
Cmd+Q walked the same shutdown checkpoint as the updater: a persist() throw preventDefault()ed the synthetic beforeunload and confirmNativeWindowClose returned silently — quit accepted, nothing logged, SIGKILL the only exit. - run the quit checkpoint inside a window-close scope so full-session staging failures degrade to the durable tier for app-level closes too (dirty editor drafts still hard-block) - when the checkpoint still vetoes the quit, toast the published failure reason instead of dying silently
This commit is contained in:
@@ -14,7 +14,7 @@ function makeDeps(
|
||||
buildSessionSnapshots: () => [{ state: { activeTabId: 't1' } }] as never,
|
||||
buildUiPatch: () => ({ activeView: 'workspace' }) as never,
|
||||
hasDirtyOpenFiles: () => false,
|
||||
isIntentionalAppRestartInProgress: () => true,
|
||||
isDegradableShutdownInProgress: () => true,
|
||||
stageBeforeUnloadSync: vi.fn(),
|
||||
...overrides
|
||||
}
|
||||
@@ -68,9 +68,9 @@ describe('runShutdownCheckpointPersist', () => {
|
||||
expect(() => runShutdownCheckpointPersist(deps)).toThrow('sync IPC staging failed')
|
||||
})
|
||||
|
||||
it('still fails the checkpoint when staging throws outside an intentional restart', () => {
|
||||
it('still fails the checkpoint when staging throws outside a degradable shutdown', () => {
|
||||
const deps = makeDeps({
|
||||
isIntentionalAppRestartInProgress: () => false,
|
||||
isDegradableShutdownInProgress: () => false,
|
||||
stageBeforeUnloadSync: vi.fn(() => {
|
||||
throw new Error('sync IPC staging failed')
|
||||
})
|
||||
|
||||
@@ -14,7 +14,9 @@ export type ShutdownCheckpointPersistDeps = {
|
||||
buildSessionSnapshots: () => WorkspaceSessionHostSnapshot[]
|
||||
buildUiPatch: () => Partial<PersistedUIState>
|
||||
hasDirtyOpenFiles: () => boolean
|
||||
isIntentionalAppRestartInProgress: () => boolean
|
||||
/** True during an intentional restart or an app-level quit/close — the unloads
|
||||
* where losing the full snapshot beats blocking the shutdown outright. */
|
||||
isDegradableShutdownInProgress: () => boolean
|
||||
stageBeforeUnloadSync: (args: ShutdownCheckpointStageArgs) => void
|
||||
}
|
||||
|
||||
@@ -36,7 +38,7 @@ export function runShutdownCheckpointPersist(deps: ShutdownCheckpointPersistDeps
|
||||
// Why: dirty drafts exist only in the full session snapshot, so their loss is the
|
||||
// one thing this checkpoint may never trade away for an update.
|
||||
const canDegradeToDurableSession = (): boolean =>
|
||||
deps.isIntentionalAppRestartInProgress() && !deps.hasDirtyOpenFiles()
|
||||
deps.isDegradableShutdownInProgress() && !deps.hasDirtyOpenFiles()
|
||||
let sessionSnapshots: WorkspaceSessionHostSnapshot[] = []
|
||||
let degraded = false
|
||||
try {
|
||||
|
||||
@@ -22,7 +22,10 @@ import {
|
||||
} from '../lib/shutdown-checkpoint-guard'
|
||||
import { runShutdownCheckpointPersist } from './shutdown-checkpoint-persist'
|
||||
import { shutdownBufferCaptures } from '../components/terminal-pane/shutdown-buffer-captures'
|
||||
import { dispatchWindowCloseRequest } from '../components/window-close-request-coordinator'
|
||||
import {
|
||||
dispatchWindowCloseRequest,
|
||||
isWindowCloseCheckpointInProgress
|
||||
} from '../components/window-close-request-coordinator'
|
||||
import {
|
||||
ORCA_APP_RESTART_ABORTED_EVENT,
|
||||
ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT
|
||||
@@ -148,7 +151,10 @@ export function useAppSessionPersistence(): void {
|
||||
},
|
||||
buildUiPatch: () => buildActiveViewUnloadPatch(useAppStore.getState()),
|
||||
hasDirtyOpenFiles: () => useAppStore.getState().openFiles.some((file) => file.isDirty),
|
||||
isIntentionalAppRestartInProgress,
|
||||
// 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)
|
||||
})
|
||||
)
|
||||
|
||||
@@ -143,7 +143,11 @@ import {
|
||||
} from './terminal-pane/terminal-parked-tab-watchers'
|
||||
import { isMainTerminalSideEffectAuthorityForPty } from './terminal-pane/terminal-side-effect-facts-handler'
|
||||
import { appendUniqueOpenFileIds } from './terminal/unsaved-close-queue'
|
||||
import { setWindowCloseRequestHandler } from './window-close-request-coordinator'
|
||||
import {
|
||||
runWithWindowCloseCheckpointScope,
|
||||
setWindowCloseRequestHandler
|
||||
} from './window-close-request-coordinator'
|
||||
import { consumeShutdownCheckpointFailureReason } from '../../../shared/renderer-shutdown-events'
|
||||
import {
|
||||
findActivityTerminalPortal,
|
||||
useActivityTerminalPortals,
|
||||
@@ -526,8 +530,17 @@ function Terminal(): React.JSX.Element | null {
|
||||
const confirmNativeWindowClose = useCallback(() => {
|
||||
// Why: capture only after every close guard has committed. A canceled child-
|
||||
// process prompt must not consume App's synthetic/native unload guard.
|
||||
const accepted = window.dispatchEvent(new Event('beforeunload', { cancelable: true }))
|
||||
const accepted = runWithWindowCloseCheckpointScope(() =>
|
||||
window.dispatchEvent(new Event('beforeunload', { cancelable: true }))
|
||||
)
|
||||
if (!accepted) {
|
||||
// Why: a checkpoint-vetoed quit used to die here with no dialog and no log,
|
||||
// leaving SIGKILL as the only exit (#15352). The dirty-file veto publishes
|
||||
// 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}).`)
|
||||
}
|
||||
return
|
||||
}
|
||||
window.api.ui.confirmWindowClose()
|
||||
|
||||
@@ -2,7 +2,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
dispatchWindowCloseRequest,
|
||||
getWindowCloseRequestHandler,
|
||||
isWindowCloseCheckpointInProgress,
|
||||
registerWindowCloseGuard,
|
||||
runWithWindowCloseCheckpointScope,
|
||||
setWindowCloseRequestHandler
|
||||
} from './window-close-request-coordinator'
|
||||
|
||||
@@ -127,4 +129,20 @@ describe('window-close-request-coordinator', () => {
|
||||
expect(guard).not.toHaveBeenCalled()
|
||||
expect(confirmWindowClose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('scopes the window-close checkpoint flag to the wrapped dispatch (STA-5505)', () => {
|
||||
expect(isWindowCloseCheckpointInProgress()).toBe(false)
|
||||
const seen = runWithWindowCloseCheckpointScope(() => isWindowCloseCheckpointInProgress())
|
||||
expect(seen).toBe(true)
|
||||
expect(isWindowCloseCheckpointInProgress()).toBe(false)
|
||||
})
|
||||
|
||||
it('clears the window-close checkpoint flag when the wrapped dispatch throws', () => {
|
||||
expect(() =>
|
||||
runWithWindowCloseCheckpointScope(() => {
|
||||
throw new Error('listener exploded')
|
||||
})
|
||||
).toThrow('listener exploded')
|
||||
expect(isWindowCloseCheckpointInProgress()).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -16,6 +16,23 @@ export type WindowCloseRequestHandler = (data: { isQuitting: boolean }) => void
|
||||
export type WindowCloseGuard = () => boolean | Promise<boolean>
|
||||
|
||||
let activeHandler: WindowCloseRequestHandler | null = null
|
||||
// Why: lets the shutdown checkpoint tell an app-level quit/close (durable-session
|
||||
// degradation is acceptable — the alternative is a quit the user can only complete
|
||||
// with SIGKILL, #15352) from an arbitrary unload, where it must stay strict.
|
||||
let windowCloseCheckpointInProgress = false
|
||||
|
||||
export function isWindowCloseCheckpointInProgress(): boolean {
|
||||
return windowCloseCheckpointInProgress
|
||||
}
|
||||
|
||||
export function runWithWindowCloseCheckpointScope<T>(fn: () => T): T {
|
||||
windowCloseCheckpointInProgress = true
|
||||
try {
|
||||
return fn()
|
||||
} finally {
|
||||
windowCloseCheckpointInProgress = false
|
||||
}
|
||||
}
|
||||
const closeGuards = new Set<WindowCloseGuard>()
|
||||
// Why: a guard can await a dialog; ignore re-entrant close requests (main resends
|
||||
// 'window:close-requested' on each attempt) so we don't stack duplicate prompts.
|
||||
|
||||
@@ -148,6 +148,23 @@ describe('createShutdownCheckpointGuard', () => {
|
||||
expect(persist).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('runs the quit checkpoint inside the window-close scope and surfaces a vetoed quit (STA-5505/#15352)', () => {
|
||||
const source = readFileSync(
|
||||
join(process.cwd(), 'src/renderer/src/components/Terminal.tsx'),
|
||||
'utf8'
|
||||
)
|
||||
const closeStart = source.indexOf('const confirmNativeWindowClose = useCallback(() => {')
|
||||
const closeEnd = source.indexOf('window.api.ui.confirmWindowClose()', closeStart)
|
||||
expect(closeStart).toBeGreaterThanOrEqual(0)
|
||||
expect(closeEnd).toBeGreaterThan(closeStart)
|
||||
const closeBlock = source.slice(closeStart, closeEnd)
|
||||
// Why pin: without the scope, a persist failure blocks quit with no degradable
|
||||
// tier; without the toast, the vetoed quit is silent and SIGKILL-only (#15352).
|
||||
expect(closeBlock).toContain('runWithWindowCloseCheckpointScope(() =>')
|
||||
expect(closeBlock).toContain('consumeShutdownCheckpointFailureReason()')
|
||||
expect(closeBlock).toContain('toast.error(')
|
||||
})
|
||||
|
||||
it('wires dirty editor unload vetoes to the paired-web checkpoint reset', () => {
|
||||
const source = readFileSync(
|
||||
join(process.cwd(), 'src/renderer/src/components/Terminal.tsx'),
|
||||
|
||||
Reference in New Issue
Block a user