diff --git a/apps/desktop/src/main/__tests__/app-quit-coordinator.test.ts b/apps/desktop/src/main/__tests__/app-quit-coordinator.test.ts index 190516dd4e..db1a26f7f8 100644 --- a/apps/desktop/src/main/__tests__/app-quit-coordinator.test.ts +++ b/apps/desktop/src/main/__tests__/app-quit-coordinator.test.ts @@ -29,6 +29,7 @@ describe('app quit coordinator', () => { prepareToQuit: async () => 'ready' as const, cleanup: async () => {}, focusOrCreateWindow: () => {}, + forceExit: () => {}, onPreparationError: () => {}, onCleanupError: () => {}, onWindowCreationError: () => {}, @@ -72,6 +73,7 @@ describe('app quit coordinator', () => { focusOrCreateWindow: () => { focusOrCreateCount += 1; }, + forceExit: () => {}, onPreparationError: () => {}, onCleanupError: () => {}, onWindowCreationError: () => {}, @@ -119,6 +121,7 @@ describe('app quit coordinator', () => { focusOrCreateCount += 1; windowCreationSignal = signal; }, + forceExit: () => {}, onPreparationError: () => {}, onCleanupError: () => {}, onWindowCreationError: () => {}, @@ -145,6 +148,7 @@ describe('app quit coordinator', () => { focusOrCreateWindow: () => { focusOrCreateCount += 1; }, + forceExit: () => {}, onPreparationError: () => {}, onCleanupError: () => {}, onWindowCreationError: () => {}, @@ -170,6 +174,7 @@ describe('app quit coordinator', () => { focusOrCreateWindow: async () => { throw failure; }, + forceExit: () => {}, onPreparationError: () => {}, onCleanupError: () => {}, onWindowCreationError: (error) => reportedErrors.push(error), @@ -202,6 +207,7 @@ describe('app quit coordinator', () => { focusOrCreateWindow: () => { focusOrCreateCount += 1; }, + forceExit: () => {}, onPreparationError: (error) => reportedErrors.push(error), onCleanupError: () => {}, onWindowCreationError: () => {}, @@ -239,6 +245,7 @@ describe('app quit coordinator', () => { focusOrCreateWindow: () => { focusOrCreateCount += 1; }, + forceExit: () => {}, onPreparationError: () => {}, onCleanupError: (error: unknown) => { reportedErrors.push(error); @@ -265,8 +272,213 @@ describe('app quit coordinator', () => { assert.equal(resumeQuitCount, 1); assert.equal(secondQuitPrevented, false); }); + + it('forces exit when cleanup never settles', async () => { + const timeouts: Array<{ stage: string; timeoutMs: number }> = []; + let forceExitCount = 0; + let resumeQuitCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: async () => 'ready', + cleanup: () => new Promise(() => {}), + focusOrCreateWindow: () => {}, + forceExit: () => { + forceExitCount += 1; + }, + onPreparationError: () => {}, + onCleanupError: () => {}, + onWindowCreationError: () => {}, + onQuitTimeout: (stage, timeoutMs) => { + timeouts.push({ stage, timeoutMs }); + }, + resumeQuit: () => { + resumeQuitCount += 1; + }, + cleanupTimeoutMs: 10, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await delay(60); + + assert.deepEqual(timeouts, [{ stage: 'cleaning', timeoutMs: 10 }]); + assert.equal(forceExitCount, 1); + assert.equal(resumeQuitCount, 0); + }); + + it('still quits when quit preparation never settles', async () => { + const timeouts: string[] = []; + let cleanupCount = 0; + let forceExitCount = 0; + let resumeQuitCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: () => new Promise<'ready' | 'cancelled'>(() => {}), + cleanup: async () => { + cleanupCount += 1; + }, + focusOrCreateWindow: () => {}, + forceExit: () => { + forceExitCount += 1; + }, + onPreparationError: () => {}, + onCleanupError: () => {}, + onWindowCreationError: () => {}, + onQuitTimeout: (stage) => { + timeouts.push(stage); + }, + resumeQuit: () => { + resumeQuitCount += 1; + }, + prepareTimeoutMs: 10, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await delay(60); + await flushQuitCoordinator(); + + assert.deepEqual(timeouts, ['preparing']); + assert.equal(cleanupCount, 1); + assert.equal(forceExitCount, 0); + assert.equal(resumeQuitCount, 1); + }); + + it('unwinds staged work when quit preparation is cancelled', async () => { + let abandonedCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: async () => 'cancelled', + cleanup: async () => {}, + focusOrCreateWindow: () => {}, + forceExit: () => {}, + onPreparationError: () => {}, + onCleanupError: () => {}, + onWindowCreationError: () => {}, + onQuitAbandoned: () => { + abandonedCount += 1; + }, + resumeQuit: () => {}, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await flushQuitCoordinator(); + + assert.equal(abandonedCount, 1); + }); + + it('unwinds staged work when quit preparation fails', async () => { + let abandonedCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: async () => { + throw new Error('retirement failed'); + }, + cleanup: async () => {}, + focusOrCreateWindow: () => {}, + forceExit: () => {}, + onPreparationError: () => {}, + onCleanupError: () => {}, + onWindowCreationError: () => {}, + onQuitAbandoned: () => { + abandonedCount += 1; + }, + resumeQuit: () => {}, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await flushQuitCoordinator(); + + assert.equal(abandonedCount, 1); + }); + + it('leaves staged work alone once the quit is committed', async () => { + // Unwinding here would resume a Host the process is about to drop, and + // an install handoff would be rolled back out from under the updater. + let abandonedCount = 0; + let resumeQuitCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: async () => 'ready', + cleanup: async () => {}, + focusOrCreateWindow: () => {}, + forceExit: () => {}, + onPreparationError: () => {}, + onCleanupError: () => {}, + onWindowCreationError: () => {}, + onQuitAbandoned: () => { + abandonedCount += 1; + }, + resumeQuit: () => { + resumeQuitCount += 1; + }, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await flushQuitCoordinator(); + + assert.equal(resumeQuitCount, 1); + assert.equal(abandonedCount, 0); + }); + + it('leaves staged work alone when a stuck quit is forced through', async () => { + let abandonedCount = 0; + let forceExitCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: async () => 'ready', + cleanup: () => new Promise(() => {}), + focusOrCreateWindow: () => {}, + forceExit: () => { + forceExitCount += 1; + }, + onPreparationError: () => {}, + onCleanupError: () => {}, + onWindowCreationError: () => {}, + onQuitAbandoned: () => { + abandonedCount += 1; + }, + resumeQuit: () => {}, + cleanupTimeoutMs: 10, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await delay(60); + + assert.equal(forceExitCount, 1); + assert.equal(abandonedCount, 0); + }); + + it('does not report an abandoned stage as a failure', async () => { + // A stage that stopped answering is not a Host that refused: routing it to + // the error sinks would invent a failure the product then has to explain. + const preparationErrors: unknown[] = []; + const cleanupErrors: unknown[] = []; + let forceExitCount = 0; + const coordinator = createAppQuitCoordinator({ + prepareToQuit: () => new Promise<'ready' | 'cancelled'>(() => {}), + cleanup: () => new Promise(() => {}), + focusOrCreateWindow: () => {}, + forceExit: () => { + forceExitCount += 1; + }, + onPreparationError: (error) => { + preparationErrors.push(error); + }, + onCleanupError: (error) => { + cleanupErrors.push(error); + }, + onWindowCreationError: () => {}, + resumeQuit: () => {}, + prepareTimeoutMs: 5, + cleanupTimeoutMs: 5, + }); + + coordinator.handleBeforeQuit({ preventDefault: () => {} }); + await delay(60); + + assert.deepEqual(preparationErrors, []); + assert.deepEqual(cleanupErrors, []); + assert.equal(forceExitCount, 1); + }); }); +async function delay(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + async function flushQuitCoordinator(): Promise { await new Promise((resolve) => setImmediate(resolve)); await new Promise((resolve) => setImmediate(resolve)); diff --git a/apps/desktop/src/main/__tests__/app-update-service.test.ts b/apps/desktop/src/main/__tests__/app-update-service.test.ts index 805f0b3c54..bc0ed9af94 100644 --- a/apps/desktop/src/main/__tests__/app-update-service.test.ts +++ b/apps/desktop/src/main/__tests__/app-update-service.test.ts @@ -587,6 +587,40 @@ describe('AppUpdateService', () => { assert.deepEqual(order, ['host-prepared', 'install']); }); + test('releases the Host handoff when the dispatched quit never happens', async () => { + let rollbacks = 0; + const updater = new FakeUpdater(); + const { service } = createHarness({ + updater, + prepareInstall: async () => ({ + kind: 'prepared', + rollback: () => { + rollbacks += 1; + }, + }), + }); + updater.emit('update-downloaded', { + ...updateInfo('1.1.0'), + downloadedFile: '/tmp/maka-update.zip', + }); + await settleUpdateVerification(); + + assert.deepEqual(await service.installUpdate({ allowInterruptActiveTasks: false }), { + ok: true, + }); + assert.equal(rollbacks, 0); + + // The installer was dispatched but the process stayed up. Nothing else + // observes a quit that did not happen, so the Host would stay retired + // with nothing left to restart it. + service.abandonPendingInstall(); + assert.equal(rollbacks, 1); + + // Releasing twice must not resume a Host a later install already owns. + service.abandonPendingInstall(); + assert.equal(rollbacks, 1); + }); + test('reports synchronous and asynchronous installer failures through status', async () => { let synchronousRollbacks = 0; const synchronous = createHarness({ diff --git a/apps/desktop/src/main/app-quit-coordinator.ts b/apps/desktop/src/main/app-quit-coordinator.ts index 565f450262..ff3624dcbe 100644 --- a/apps/desktop/src/main/app-quit-coordinator.ts +++ b/apps/desktop/src/main/app-quit-coordinator.ts @@ -26,6 +26,24 @@ export interface AppQuitCoordinator { handleBeforeQuit(event: AppQuitEvent): void; } +/** The two awaited stages of an orderly quit, in the order they run. */ +export type AppQuitStage = 'preparing' | 'cleaning'; + +/** + * Quitting waits on Host retirement, MCP child processes, peer mesh teardown + * and native resources. Any of those can stop settling, and an unbounded wait + * is indistinguishable from a hang: the process keeps running with its windows + * already committed to closing, and every later quit request is swallowed + * because the sequence still owns the phase. + * + * An updater install makes that failure permanent rather than merely annoying. + * Squirrel's ShipIt waits for this process to exit before it replaces the + * bundle, so a quit that never finishes silently blocks the update instead of + * reporting anything. + */ +const DEFAULT_PREPARE_TIMEOUT_MS = 15_000; +const DEFAULT_CLEANUP_TIMEOUT_MS = 10_000; + export interface AppQuitCoordinatorDeps { prepareToQuit(): Promise<'ready' | 'cancelled'>; cleanup(): Promise; @@ -34,13 +52,63 @@ export interface AppQuitCoordinatorDeps { onCleanupError(error: unknown): void; onWindowCreationError(error: unknown): void; resumeQuit(): void; + /** + * Terminate now, abandoning whatever never settled. Reached only after a + * stage exceeds its bound, so it is the difference between a stuck quit and + * a process that always goes away. + */ + forceExit(): void; + onQuitTimeout?(stage: AppQuitStage, timeoutMs: number): void; + /** + * The app is staying up after all. Anything staged for the quit that was + * expected to follow has to be unwound here, because nothing else observes + * a quit that simply did not happen. + */ + onQuitAbandoned?(): void; + prepareTimeoutMs?: number; + cleanupTimeoutMs?: number; } type AppQuitPhase = 'running' | 'preparing' | 'cleaning' | 'ready-to-exit'; +type StageOutcome = + | { kind: 'settled'; value: T } + | { kind: 'failed'; error: unknown } + | { kind: 'timeout' }; + +/** + * Run one quit stage under an upper bound. + * + * A late result is dropped rather than raced back in: once the bound elapses + * the sequence has already moved on, and its rejection is still observed here + * so abandoning the stage cannot surface as an unhandled rejection. + */ +function runStage(run: () => Promise, timeoutMs: number): Promise> { + return new Promise>((resolve) => { + let done = false; + const settle = (outcome: StageOutcome) => { + if (done) return; + done = true; + clearTimeout(timer); + resolve(outcome); + }; + const timer = setTimeout(() => settle({ kind: 'timeout' }), timeoutMs); + // A pending bound must never be the reason the process stays alive. + timer.unref?.(); + Promise.resolve() + .then(run) + .then( + (value) => settle({ kind: 'settled', value }), + (error) => settle({ kind: 'failed', error }), + ); + }); +} + export function createAppQuitCoordinator(deps: AppQuitCoordinatorDeps): AppQuitCoordinator { let phase: AppQuitPhase = 'running'; let windowCreationAbort = new AbortController(); + const prepareTimeoutMs = deps.prepareTimeoutMs ?? DEFAULT_PREPARE_TIMEOUT_MS; + const cleanupTimeoutMs = deps.cleanupTimeoutMs ?? DEFAULT_CLEANUP_TIMEOUT_MS; const focusOrCreateWindow = (): Promise => { if (phase !== 'running') return Promise.resolve(); @@ -54,6 +122,58 @@ export function createAppQuitCoordinator(deps: AppQuitCoordinatorDeps): AppQuitC } }; + const abandonQuit = (): void => { + phase = 'running'; + windowCreationAbort = new AbortController(); + deps.onQuitAbandoned?.(); + void focusOrCreateWindow(); + }; + + const finishCleanup = (): void => { + // `before-quit` was cancelled inside Electron's native quit transaction. + // Resuming from the cleanup Promise's microtask re-enters that transaction: + // Electron closes the windows but emits `window-all-closed` instead of + // `will-quit`, leaving the macOS process alive. Start a fresh transaction + // only after the current event-loop turn has unwound. + setImmediate(() => { + phase = 'ready-to-exit'; + deps.resumeQuit(); + }); + }; + + const runQuitSequence = async (): Promise => { + const preparation = await runStage(() => deps.prepareToQuit(), prepareTimeoutMs); + + if (preparation.kind === 'failed') { + deps.onPreparationError(preparation.error); + abandonQuit(); + return; + } + if (preparation.kind === 'settled' && preparation.value === 'cancelled') { + abandonQuit(); + return; + } + if (preparation.kind === 'timeout') { + // The user asked to quit and preparation is only a courtesy to the Host. + // Report it and keep going: refusing to quit is the worse outcome. + deps.onQuitTimeout?.('preparing', prepareTimeoutMs); + } + + phase = 'cleaning'; + const cleanup = await runStage(() => deps.cleanup(), cleanupTimeoutMs); + + if (cleanup.kind === 'failed') deps.onCleanupError(cleanup.error); + if (cleanup.kind === 'timeout') { + deps.onQuitTimeout?.('cleaning', cleanupTimeoutMs); + // Nothing left to wait for and no orderly exit available. Leaving the + // process up would strand an in-flight updater install indefinitely. + phase = 'ready-to-exit'; + deps.forceExit(); + return; + } + finishCleanup(); + }; + return { focusOrCreateWindow, handleBeforeQuit(event): void { @@ -62,42 +182,7 @@ export function createAppQuitCoordinator(deps: AppQuitCoordinatorDeps): AppQuitC if (phase !== 'running') return; phase = 'preparing'; windowCreationAbort.abort(); - const finishCleanup = () => { - // `before-quit` was cancelled inside Electron's native quit transaction. - // Resuming from the cleanup Promise's microtask re-enters that transaction: - // Electron closes the windows but emits `window-all-closed` instead of - // `will-quit`, leaving the macOS process alive. Start a fresh transaction - // only after the current event-loop turn has unwound. - setImmediate(() => { - phase = 'ready-to-exit'; - deps.resumeQuit(); - }); - }; - void Promise.resolve() - .then(() => deps.prepareToQuit()) - .then( - (preparation) => { - if (preparation === 'cancelled') { - phase = 'running'; - windowCreationAbort = new AbortController(); - focusOrCreateWindow(); - return; - } - phase = 'cleaning'; - return Promise.resolve() - .then(() => deps.cleanup()) - .then(finishCleanup, (error) => { - deps.onCleanupError(error); - finishCleanup(); - }); - }, - (error) => { - phase = 'running'; - windowCreationAbort = new AbortController(); - deps.onPreparationError(error); - focusOrCreateWindow(); - }, - ); + void runQuitSequence(); }, }; } diff --git a/apps/desktop/src/main/app-update-service.ts b/apps/desktop/src/main/app-update-service.ts index 1433308b1d..261734c912 100644 --- a/apps/desktop/src/main/app-update-service.ts +++ b/apps/desktop/src/main/app-update-service.ts @@ -43,6 +43,15 @@ export interface AppUpdateService { getStatus(): AppUpdateStatus; retryUpdateDownload(): Promise; installUpdate(input: AppUpdateInstallRequest): Promise; + /** + * Release an install handoff whose quit never happened. + * + * `installUpdate` retires the Host before handing off to the updater, and + * that retirement only unwinds through its rollback. A quit that is + * cancelled or abandoned would otherwise leave the Host retired with + * nothing left to restart it, so the quit path reports back here. + */ + abandonPendingInstall(): void; /** Check because the window regained focus, subject to a shared throttle. */ checkForUpdatesOnFocus(): Promise; /** @@ -524,6 +533,7 @@ export function createAppUpdateService(deps: AppUpdateServiceDeps): AppUpdateSer getStatus: currentStatus, retryUpdateDownload, installUpdate, + abandonPendingInstall: rollbackInstallHandoff, checkForUpdatesOnFocus, checkForUpdatesNow, }; diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index 958ffbc26a..7f0109334c 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -1447,7 +1447,17 @@ const quitCoordinator = createAppQuitCoordinator({ console.error("[runtime-host] shutdown failed:", error), onWindowCreationError: (error) => console.error("[window] creation failed:", error), + onQuitTimeout: (stage, timeoutMs) => { + // Squirrel's ShipIt waits for this process before it replaces the bundle, + // so a quit that never finishes blocks the update with nothing to read. + console.error( + `[runtime-host] quit ${stage} did not settle within ${timeoutMs}ms; ` + + (stage === "cleaning" ? "exiting without it" : "continuing shutdown"), + ); + }, + onQuitAbandoned: () => updateService.abandonPendingInstall(), resumeQuit: () => app.quit(), + forceExit: () => app.exit(0), }); app.on("before-quit", quitCoordinator.handleBeforeQuit); updateDesktopStartupProgress('connect');