From f1eb8440553415dcd9ce4f670ea8eef9d0d42b9b Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Thu, 3 Sep 2026 12:51:39 -0700 Subject: [PATCH] fix(updater): avoid duplicate markers and stale quit relaunches --- src/main/updater-mac-install.ts | 14 ++++- src/main/updater.mac-install.test.ts | 52 ++++++++++++++++++- .../window-close-request-coordinator.test.ts | 3 +- .../window-close-request-coordinator.ts | 3 ++ 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/main/updater-mac-install.ts b/src/main/updater-mac-install.ts index a677f32a7d5..55046b23b36 100644 --- a/src/main/updater-mac-install.ts +++ b/src/main/updater-mac-install.ts @@ -1,6 +1,9 @@ import { app, autoUpdater as nativeUpdater } from 'electron' import type { UpdateStatus } from '../shared/update-status-types' -import { markMacUpdateInstallInFlight } from './mac-update-install-marker' +import { + isMacUpdateInstallInFlight, + markMacUpdateInstallInFlight +} from './mac-update-install-marker' import { recordUpdaterLifecycle } from './updater-lifecycle-diagnostics' const MAC_INSTALL_READY_TIMEOUT_MS = 15000 @@ -42,7 +45,14 @@ export function registerMacUpdaterEvents({ // Why will-quit rather than before-quit: vetoes have already been resolved here, so the app is // genuinely on its way out and the marker cannot strand a launch for a quit that got abandoned. app.on('will-quit', () => { - if (process.platform !== 'darwin' || !isMacInstallerReady()) { + if ( + process.platform !== 'darwin' || + !isMacInstallerReady() || + // Explicit quitAndInstall already wrote the marker immediately before invoking native + // handoff. Writing a second attempt here leaves an orphan marker that reports the same + // install again on a later startup and can keep the launch gate active after settlement. + isMacUpdateInstallInFlight() + ) { return } const pendingVersion = getPendingInstallVersion() diff --git a/src/main/updater.mac-install.test.ts b/src/main/updater.mac-install.test.ts index 563b8562fd6..adc8c2056db 100644 --- a/src/main/updater.mac-install.test.ts +++ b/src/main/updater.mac-install.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { loadUpdaterModule, warmUpdaterModule } from './updater-test-module-loader' +import type * as MacUpdateMarkerModule from './mac-update-install-marker' const { appMock, @@ -8,7 +9,9 @@ const { autoUpdaterMock, shellMock, isMock, - killAllPtyMock + killAllPtyMock, + isMacUpdateInstallInFlightMock, + markMacUpdateInstallInFlightMock } = vi.hoisted(() => { const appEventHandlers = new Map void)[]>() const eventHandlers = new Map void)[]>() @@ -80,7 +83,9 @@ const { openExternal: vi.fn() }, isMock: { dev: false }, - killAllPtyMock: vi.fn() + killAllPtyMock: vi.fn(), + isMacUpdateInstallInFlightMock: vi.fn(), + markMacUpdateInstallInFlightMock: vi.fn() } }) @@ -109,6 +114,12 @@ vi.mock('./ipc/pty', () => ({ killAllPty: killAllPtyMock })) +vi.mock('./mac-update-install-marker', async (importOriginal) => ({ + ...(await importOriginal()), + isMacUpdateInstallInFlight: isMacUpdateInstallInFlightMock, + markMacUpdateInstallInFlight: markMacUpdateInstallInFlightMock +})) + vi.mock('./updater-changelog', () => ({ fetchChangelog: vi.fn().mockResolvedValue(null) })) @@ -134,11 +145,48 @@ describe('updater mac install handoff', () => { appMock.isPackaged = true isMock.dev = false killAllPtyMock.mockReset() + isMacUpdateInstallInFlightMock.mockReset() + isMacUpdateInstallInFlightMock.mockReturnValue(false) + markMacUpdateInstallInFlightMock.mockReset() autoUpdaterMock.downloadUpdate.mockResolvedValue([]) vi.unstubAllGlobals() vi.useRealTimers() }) + it.runIf(process.platform === 'darwin')( + 'does not write a duplicate marker during explicit quitAndInstall', + async () => { + const mainWindow = { webContents: { send: vi.fn() } } + autoUpdaterMock.checkForUpdates.mockResolvedValue(undefined) + const { setupAutoUpdater, downloadUpdate, quitAndInstall } = await loadUpdaterModule() + + setupAutoUpdater(mainWindow as never) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-available', { version: '1.0.61' }) + await new Promise((r) => setTimeout(r, 0)) + downloadUpdate() + autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' }) + + const nativeDownloadedHandler = nativeUpdaterMock.on.mock.calls.find( + ([eventName]) => eventName === 'update-downloaded' + )?.[1] as (() => void) | undefined + nativeDownloadedHandler?.() + quitAndInstall() + await vi.waitFor(() => { + expect(markMacUpdateInstallInFlightMock).toHaveBeenCalledTimes(1) + }) + + // The native install path writes the marker before quit; its will-quit hook must not write + // another attempt for the same handoff. + isMacUpdateInstallInFlightMock.mockReturnValue(true) + appMock.emit('will-quit') + expect(markMacUpdateInstallInFlightMock).toHaveBeenCalledTimes(1) + } + ) + it.runIf(process.platform === 'darwin')( 'waits for Squirrel.Mac before honoring a manual quit that should install the update', async () => { diff --git a/src/renderer/src/components/window-close-request-coordinator.test.ts b/src/renderer/src/components/window-close-request-coordinator.test.ts index 5000fff605f..1b78f05830e 100644 --- a/src/renderer/src/components/window-close-request-coordinator.test.ts +++ b/src/renderer/src/components/window-close-request-coordinator.test.ts @@ -98,9 +98,10 @@ describe('window-close-request-coordinator', () => { { once: true } ) - await dispatchWindowCloseRequest({ isQuitting: true }) + await dispatchWindowCloseRequest({ isQuitting: true, requestId: 42 }) expect(confirmWindowClose).not.toHaveBeenCalled() + expect(cancelWindowClose).toHaveBeenCalledWith(42) expect(toast.error).toHaveBeenCalledWith( 'Quit canceled: the session snapshot could not be saved (sendSync payload rejected).' ) diff --git a/src/renderer/src/components/window-close-request-coordinator.ts b/src/renderer/src/components/window-close-request-coordinator.ts index b99146b15a2..2bd3d6a632e 100644 --- a/src/renderer/src/components/window-close-request-coordinator.ts +++ b/src/renderer/src/components/window-close-request-coordinator.ts @@ -101,5 +101,8 @@ export async function dispatchWindowCloseRequest(data: { window.api.ui.confirmWindowClose() return } + // A beforeunload veto is another abandoned close path. Without this signal, main keeps the + // request outstanding and any relaunch deferred to `quit` can fire on a later unrelated quit. + window.api.ui.cancelWindowClose(data.requestId) showShutdownCheckpointFailureToast() }