From 73a61fff7fd90e010b94ef20b3715ed3ee52ed57 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:40:31 -0700 Subject: [PATCH] fix(updater): resume background checks after a local build session ends (#11223) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A local-build check (Option+click "Check for Updates" on macOS) pins activeUpdateSource to 'local' for the rest of the process. The 'update-available' success path never restores it, and runBackgroundUpdateCheck early-returns on it, so every wake-from-sleep check, window-focus daily check and nudge poll became a no-op once a local build reached 'available'. The one-shot automatic timer fired into that early return and nothing re-armed it, so the scheduling chain died too and lastUpdateCheckAt froze. Restoring the source when 'update-available' fires would break the flow the user just started — the pending download still needs the local feed and allowDowngrade. Instead the release source is restored when the user closes the offered card, which main previously never learned about, and only while status is exactly 'available': downloadUpdate() flips status to 'downloading' synchronously before it calls into electron-updater, so this cannot fire once a download is under way. The automatic timer now re-arms when a check is deferred rather than launched, so a deferral can no longer end automatic checks for the process lifetime. --- src/main/updater.test.ts | 176 ++++++++++++++++++ src/main/updater.ts | 33 +++- .../window/attach-main-window-services.ts | 3 + src/preload/api-types.ts | 1 + src/preload/index.ts | 1 + .../components/UpdateCard.error-card.test.tsx | 1 + .../src/components/UpdateCard.test.ts | 3 +- src/renderer/src/store/slices/ui.ts | 2 + src/renderer/src/web/web-preload-api.ts | 1 + 9 files changed, 215 insertions(+), 6 deletions(-) diff --git a/src/main/updater.test.ts b/src/main/updater.test.ts index 730dd5be96c..f2204c6c5ac 100644 --- a/src/main/updater.test.ts +++ b/src/main/updater.test.ts @@ -177,6 +177,9 @@ vi.mock('./local-builds/local-build-feed-server', () => ({ startLocalBuildFeed: startLocalBuildFeedMock })) +/** Mirrors AUTO_UPDATE_CHECK_INTERVAL_MS in updater.ts. */ +const AUTO_UPDATE_CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000 + describe('updater', () => { beforeEach(() => { vi.resetModules() @@ -400,6 +403,179 @@ describe('updater', () => { } ) + it.runIf(process.platform === 'darwin')( + 'keeps the automatic check chain alive across a local build session', + async () => { + vi.useFakeTimers() + chooseLocalBuildMock.mockResolvedValue({ + version: '0.9.0-local.1', + manifestContent: 'version: 0.9.0-local.1', + artifacts: new Map() + }) + autoUpdaterMock.checkForUpdates.mockImplementation(() => { + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-available', { version: '0.9.0-local.1' }) + return Promise.resolve(undefined) + }) + autoUpdaterMock.downloadUpdate.mockRejectedValue(new Error('local download failed')) + const send = vi.fn() + const { setupAutoUpdater, checkForUpdatesFromMenu, downloadUpdate } = + await import('./updater') + setupAutoUpdater({ webContents: { send } } as never, { + getLastUpdateCheckAt: () => Date.now() + }) + + checkForUpdatesFromMenu({ localBuild: true }) + await vi.advanceTimersByTimeAsync(0) + expect(send).toHaveBeenCalledWith( + 'updater:status', + expect.objectContaining({ state: 'available', source: 'local' }) + ) + + // Why: assert through this window rather than the shared updater mock — a status only lands here + // when this module instance owns the check attempt, so sibling tests' timers can't fake a pass. + autoUpdaterMock.checkForUpdates.mockReset().mockImplementation(() => { + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-not-available') + return Promise.resolve(undefined) + }) + + // The scheduled release check fires while the local session still owns the feed, so it defers. + send.mockClear() + await vi.advanceTimersByTimeAsync(AUTO_UPDATE_CHECK_INTERVAL_MS) + expect(send).not.toHaveBeenCalledWith('updater:status', { state: 'not-available' }) + + // A failed local download ends the session and restores the release source. + downloadUpdate() + await vi.advanceTimersByTimeAsync(0) + expect(autoUpdaterMock.allowDowngrade).toBe(false) + + send.mockClear() + await vi.advanceTimersByTimeAsync(AUTO_UPDATE_CHECK_INTERVAL_MS) + expect(send).toHaveBeenCalledWith('updater:status', { state: 'not-available' }) + expect(autoUpdaterMock.setFeedURL).toHaveBeenLastCalledWith({ + provider: 'generic', + url: 'https://github.com/stablyai/orca/releases/latest/download' + }) + } + ) + + it.runIf(process.platform === 'darwin')( + 'restores release checks when an offered local build is dismissed', + async () => { + chooseLocalBuildMock.mockResolvedValue({ + version: '0.9.0-local.1', + manifestContent: 'version: 0.9.0-local.1', + artifacts: new Map() + }) + autoUpdaterMock.checkForUpdates.mockImplementation(() => { + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-available', { version: '0.9.0-local.1' }) + return Promise.resolve(undefined) + }) + const send = vi.fn() + const { setupAutoUpdater, checkForUpdates, checkForUpdatesFromMenu, dismissAvailableUpdate } = + await import('./updater') + setupAutoUpdater({ webContents: { send } } as never, { + getLastUpdateCheckAt: () => Date.now() + }) + + checkForUpdatesFromMenu({ localBuild: true }) + await vi.waitFor(() => { + expect(send).toHaveBeenCalledWith( + 'updater:status', + expect.objectContaining({ state: 'available', source: 'local' }) + ) + }) + + dismissAvailableUpdate() + expect(closeLocalBuildFeedMock).toHaveBeenCalledTimes(1) + expect(autoUpdaterMock.allowDowngrade).toBe(false) + expect(autoUpdaterMock.disableDifferentialDownload).toBe(false) + expect(send).toHaveBeenCalledWith('updater:status', { state: 'idle' }) + + autoUpdaterMock.checkForUpdates.mockReset().mockResolvedValue(undefined) + checkForUpdates() + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) + expect(autoUpdaterMock.setFeedURL).toHaveBeenLastCalledWith({ + provider: 'generic', + url: 'https://github.com/stablyai/orca/releases/latest/download' + }) + } + ) + + it.runIf(process.platform === 'darwin')( + 'keeps the local feed in force when a dismiss lands during a local build download', + async () => { + chooseLocalBuildMock.mockResolvedValue({ + version: '0.9.0-local.1', + manifestContent: 'version: 0.9.0-local.1', + artifacts: new Map() + }) + autoUpdaterMock.checkForUpdates.mockImplementation(() => { + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-available', { version: '0.9.0-local.1' }) + return Promise.resolve(undefined) + }) + autoUpdaterMock.downloadUpdate.mockResolvedValue(undefined) + const send = vi.fn() + const { setupAutoUpdater, checkForUpdatesFromMenu, dismissAvailableUpdate, downloadUpdate } = + await import('./updater') + setupAutoUpdater({ webContents: { send } } as never, { + getLastUpdateCheckAt: () => Date.now() + }) + + checkForUpdatesFromMenu({ localBuild: true }) + await vi.waitFor(() => { + expect(send).toHaveBeenCalledWith( + 'updater:status', + expect.objectContaining({ state: 'available', source: 'local' }) + ) + }) + + downloadUpdate() + dismissAvailableUpdate() + + expect(closeLocalBuildFeedMock).not.toHaveBeenCalled() + expect(autoUpdaterMock.allowDowngrade).toBe(true) + expect(autoUpdaterMock.disableDifferentialDownload).toBe(true) + expect(autoUpdaterMock.setFeedURL).toHaveBeenLastCalledWith({ + provider: 'generic', + url: 'http://127.0.0.1:1234/token/' + }) + } + ) + + it('leaves a dismissed release update on the release source', async () => { + autoUpdaterMock.checkForUpdates.mockImplementation(() => { + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-available', { version: '2.0.0' }) + return Promise.resolve(undefined) + }) + const send = vi.fn() + const { setupAutoUpdater, checkForUpdatesFromMenu, dismissAvailableUpdate } = + await import('./updater') + setupAutoUpdater({ webContents: { send } } as never, { + getLastUpdateCheckAt: () => Date.now() + }) + + checkForUpdatesFromMenu() + await vi.waitFor(() => { + expect(send).toHaveBeenCalledWith( + 'updater:status', + expect.objectContaining({ state: 'available', version: '2.0.0' }) + ) + }) + + dismissAvailableUpdate() + + // Why: a release dismissal is renderer-only state; main must not clear the offer it can still install. + expect(closeLocalBuildFeedMock).not.toHaveBeenCalled() + expect(send).not.toHaveBeenCalledWith('updater:status', { state: 'idle' }) + }) + it('deduplicates identical check errors from the event and rejected promise', async () => { autoUpdaterMock.checkForUpdates.mockImplementation(() => { autoUpdaterMock.emit('checking-for-update') diff --git a/src/main/updater.ts b/src/main/updater.ts index 34067e03b50..b0df2d5b22e 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -978,7 +978,10 @@ function scheduleAutomaticUpdateCheck(delayMs: number): void { } autoUpdateCheckTimer = setTimeout(() => { // Why: Orca runs for days, so keep the next background check scheduled in the main process rather than tying it to relaunches or renderer lifetime. - runBackgroundUpdateCheck() + if (!runBackgroundUpdateCheck()) { + // Why: a deferred check reaches no outcome handler, so re-arm here or one deferral ends automatic checks for the process lifetime. + scheduleAutomaticUpdateCheck(AUTO_UPDATE_CHECK_INTERVAL_MS) + } }, effectiveDelayMs) } @@ -1212,18 +1215,19 @@ function retryPrereleaseFallbackAfterMissingManifest( return true } +/** Returns false when the check was deferred instead of launched, so timer-driven callers can re-arm. */ function runBackgroundUpdateCheck( nudgeId: string | null = getPersistedPendingUpdateNudgeId() -): void { +): boolean { if (activeUpdateSource === 'local' || localBuildSelectionInProgress) { - return + return false } if (backgroundCheckLaunchPending || currentStatus.state === 'checking') { - return + return false } if (!app.isPackaged || is.dev) { sendStatus({ state: 'not-available' }) - return + return false } // Why: set the nudge marker before any events arrive so later checks can't inherit a stale campaign id; persisted id keeps a nudge card dismissable after relaunch. activeUpdateNudgeId = nudgeId @@ -1255,6 +1259,7 @@ function runBackgroundUpdateCheck( } void sendCheckFailureStatus(String(err?.message ?? err), wasUserInitiated, 'promise', err) }) + return true } export function checkForUpdates(): void { @@ -1509,6 +1514,24 @@ export function dismissNudge(): void { } } +/** + * The user closed an offered update without taking it. For a local build that ends the session: + * nothing will consume the local feed now, so release checks must stop being deferred. + */ +export function dismissAvailableUpdate(): void { + if (activeUpdateSource !== 'local' || localBuildSelectionInProgress) { + return + } + // Why: only an un-acted 'available' card is abandoned — 'downloading'/'downloaded' still need the local feed and allowDowngrade. + if (currentStatus.state !== 'available') { + return + } + clearAvailableUpdateContext() + restoreReleaseUpdateSource() + // Why: leaving the card's 'available' status behind would let a retry download the local version off the restored release feed. + sendStatus({ state: 'idle' }) +} + export function setupAutoUpdater( mainWindow: BrowserWindow, opts?: { diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 5384d11c16a..80a8e693e45 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -30,6 +30,7 @@ import { getUpdateStatus, quitAndInstall, setupAutoUpdater, + dismissAvailableUpdate, dismissNudge, type UpdateInstallMode } from '../updater' @@ -426,6 +427,7 @@ export function registerUpdaterHandlers(_store: Store): void { ipcMain.removeHandler('updater:download') ipcMain.removeHandler('updater:quitAndInstall') ipcMain.removeHandler('updater:dismissNudge') + ipcMain.removeHandler('updater:dismissAvailableUpdate') ipcMain.handle('updater:getStatus', () => getUpdateStatus()) ipcMain.handle('updater:getVersion', () => app.getVersion()) @@ -436,4 +438,5 @@ export function registerUpdaterHandlers(_store: Store): void { ipcMain.handle('updater:download', () => downloadUpdate()) ipcMain.handle('updater:quitAndInstall', () => quitAndInstall()) ipcMain.handle('updater:dismissNudge', () => dismissNudge()) + ipcMain.handle('updater:dismissAvailableUpdate', () => dismissAvailableUpdate()) } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index d60947df694..dc68afc05f1 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2620,6 +2620,7 @@ export type PreloadApi = { download: () => Promise quitAndInstall: () => Promise dismissNudge: () => Promise + dismissAvailableUpdate: () => Promise onStatus: (callback: (status: UpdateStatus) => void) => () => void onClearDismissal: (callback: () => void) => () => void } diff --git a/src/preload/index.ts b/src/preload/index.ts index bb0a17220ed..804c1f87019 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2884,6 +2884,7 @@ const api = { check: (options) => ipcRenderer.invoke('updater:check', options), download: () => ipcRenderer.invoke('updater:download'), dismissNudge: () => ipcRenderer.invoke('updater:dismissNudge'), + dismissAvailableUpdate: () => ipcRenderer.invoke('updater:dismissAvailableUpdate'), quitAndInstall: async (): Promise => { await prepareRendererForAppRestart(window, { startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, diff --git a/src/renderer/src/components/UpdateCard.error-card.test.tsx b/src/renderer/src/components/UpdateCard.error-card.test.tsx index e440729f273..618e73d6bfd 100644 --- a/src/renderer/src/components/UpdateCard.error-card.test.tsx +++ b/src/renderer/src/components/UpdateCard.error-card.test.tsx @@ -38,6 +38,7 @@ beforeEach(() => { updater: { check, dismissNudge: vi.fn(), + dismissAvailableUpdate: vi.fn().mockResolvedValue(undefined), download, quitAndInstall: vi.fn().mockResolvedValue(undefined) } diff --git a/src/renderer/src/components/UpdateCard.test.ts b/src/renderer/src/components/UpdateCard.test.ts index 9d4b75094d2..6eed56fad3e 100644 --- a/src/renderer/src/components/UpdateCard.test.ts +++ b/src/renderer/src/components/UpdateCard.test.ts @@ -41,7 +41,8 @@ beforeEach(() => { updater: { download: vi.fn().mockResolvedValue(undefined), quitAndInstall: vi.fn().mockResolvedValue(undefined), - dismissNudge: vi.fn().mockResolvedValue(undefined) + dismissNudge: vi.fn().mockResolvedValue(undefined), + dismissAvailableUpdate: vi.fn().mockResolvedValue(undefined) } }, matchMedia: vi.fn().mockReturnValue({ diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index a39631edc79..691e8fa3c6c 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -2579,6 +2579,8 @@ export const createUISlice: StateCreator = (set, get) 'activeNudgeId' in s.updateStatus ? (s.updateStatus.activeNudgeId ?? null) : null // Why: persist dismissal so relaunch doesn't immediately re-show the same card until a newer release. void window.api.ui.set({ dismissedUpdateVersion }).catch(console.error) + // Why: main can't otherwise tell an offered update was abandoned, which keeps a local-build session pinned and stalls background checks. + void window.api.updater.dismissAvailableUpdate().catch(console.error) // Why: only consume the nudge campaign for cards from a nudge cycle, not ordinary dismissals. if (activeNudgeId) { void window.api.updater.dismissNudge().catch(console.error) diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index b01292b9bda..a1dc1fb96ab 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -2919,6 +2919,7 @@ function createUpdaterApi(): NonNullable['updater']> { download: () => Promise.resolve(), quitAndInstall: () => Promise.resolve(), dismissNudge: () => Promise.resolve(), + dismissAvailableUpdate: () => Promise.resolve(), onStatus: () => noopUnsubscribe, onClearDismissal: () => noopUnsubscribe }