Fix update state review findings

Co-authored-by: Orca <help@stably.ai>

src/main/updater-events.ts

@@ -55,6 +55,7 @@ type UpdaterHandlerContext = {
 
 function getActionableUpdateUserInitiated(status: UpdateStatus): boolean | undefined {
   if (
+    status.state === 'checking' ||
     status.state === 'available' ||
     status.state === 'downloading' ||
     status.state === 'downloaded'
@@ -96,16 +97,23 @@ export function registerAutoUpdaterHandlers({
   setAvailableVersion,
   setUserInitiatedCheck
 }: UpdaterHandlerContext): void {
+  let macNativeReadinessPendingVersion: string | null = null
+
   // On macOS, electron-updater's MacUpdater downloads the ZIP from GitHub,
   // then serves it to Squirrel.Mac via a localhost proxy. The electron-updater
   // 'update-downloaded' event fires BEFORE Squirrel finishes its download.
   // Track Squirrel readiness so we don't show "ready to install" prematurely.
   if (process.platform === 'darwin') {
     nativeUpdater.on('update-downloaded', () => {
-      if (!hasNewerDownloadedVersion()) {
+      const readyVersion = macNativeReadinessPendingVersion
+      macNativeReadinessPendingVersion = null
+
+      if (!readyVersion && !hasNewerDownloadedVersion()) {
         return
       }
-      handleMacInstallerReady(true, performQuitAndInstall, () => {
+      const shouldUseNativeReadiness =
+        !readyVersion || (getStagedUpdateVersion() === readyVersion && hasNewerDownloadedVersion())
+      handleMacInstallerReady(shouldUseNativeReadiness, performQuitAndInstall, () => {
         // If we were holding the 'downloaded' status, send it now — but only
         // when the staged version is actually newer than what's running.
         sendStatus({
@@ -115,6 +123,7 @@ export function registerAutoUpdaterHandlers({
           ...(getActionableUpdateUserInitiated(getCurrentStatus()) ? { userInitiated: true } : {})
         })
       })
+      startAvailableUpdateDownload()
     })
   }
 
@@ -238,6 +247,11 @@ export function registerAutoUpdaterHandlers({
         ...(wasUserInitiated ? { userInitiated: true } : {}),
         changelog
       })
+      if (process.platform === 'darwin' && macNativeReadinessPendingVersion) {
+        // Why: Squirrel.Mac's native ready event has no version payload. Wait
+        // for the prior handoff before starting a replacement download.
+        return
+      }
       startAvailableUpdateDownload()
     })()
   })
@@ -308,6 +322,7 @@ export function registerAutoUpdaterHandlers({
     // processing the update via the localhost proxy. On other platforms,
     // the update is ready immediately after electron-updater downloads it.
     if (process.platform === 'darwin' && !isMacInstallerReady()) {
+      macNativeReadinessPendingVersion = info.version
       // Squirrel is still processing. Keep the UI at 100% downloaded so the
       // user sees the handoff instead of a misleading "ready to install".
       sendStatus({

src/main/updater.mac-install.test.ts

@@ -210,6 +210,7 @@ describe('updater mac install handoff', () => {
 
   it('does not let stale native macOS readiness mark a replacement update ready', async () => {
     vi.stubGlobal('process', { ...process, platform: 'darwin' })
+    autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
 
     const sendMock = vi.fn()
     const mainWindow = { webContents: { send: sendMock } }
@@ -228,16 +229,21 @@ describe('updater mac install handoff', () => {
     autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' })
     sendMock.mockClear()
 
+    expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
+
     autoUpdaterMock.emit('checking-for-update')
     autoUpdaterMock.emit('update-available', { version: '1.0.62' })
     await new Promise((resolve) => setTimeout(resolve, 0))
 
+    expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
+
     nativeDownloadedHandler?.()
 
     expect(sendMock).not.toHaveBeenCalledWith(
       'updater:status',
       expect.objectContaining({ state: 'downloaded' })
     )
+    expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(2)
 
     autoUpdaterMock.emit('update-downloaded', { version: '1.0.62' })
 

src/main/updater.test.ts

@@ -860,6 +860,88 @@ describe('updater', () => {
     })
   })
 
+  it('preserves user initiation when a manual check races active background download events', async () => {
+    vi.stubGlobal('process', { ...process, platform: 'linux' })
+    autoUpdaterMock.checkForUpdates.mockResolvedValue(undefined)
+    autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
+
+    const sendMock = vi.fn()
+    const mainWindow = { webContents: { send: sendMock } }
+
+    const { setupAutoUpdater } = await import('./updater')
+
+    setupAutoUpdater(mainWindow as never, { getLastUpdateCheckAt: () => Date.now() })
+    autoUpdaterMock.emit('checking-for-update')
+    autoUpdaterMock.emit('update-available', { version: '1.0.61' })
+
+    await vi.waitFor(() => {
+      expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
+    })
+
+    autoUpdaterMock.emit('download-progress', { percent: 10 })
+    sendMock.mockClear()
+
+    const { checkForUpdatesFromMenu } = await import('./updater')
+    checkForUpdatesFromMenu()
+
+    autoUpdaterMock.emit('download-progress', { percent: 42 })
+    autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' })
+
+    expect(sendMock).toHaveBeenCalledWith('updater:status', {
+      state: 'checking',
+      userInitiated: true
+    })
+    expect(sendMock).toHaveBeenCalledWith('updater:status', {
+      state: 'downloading',
+      percent: 42,
+      version: '1.0.61',
+      userInitiated: true
+    })
+    expect(sendMock).toHaveBeenCalledWith('updater:status', {
+      state: 'downloaded',
+      version: '1.0.61',
+      releaseUrl: undefined,
+      userInitiated: true
+    })
+  })
+
+  it('preserves user initiation when a manual check races an active background download failure', async () => {
+    let rejectDownload: (reason: Error) => void = () => {}
+    autoUpdaterMock.checkForUpdates.mockResolvedValue(undefined)
+    autoUpdaterMock.downloadUpdate.mockImplementation(
+      () =>
+        new Promise<string[]>((_resolve, reject) => {
+          rejectDownload = reject
+        })
+    )
+
+    const sendMock = vi.fn()
+    const mainWindow = { webContents: { send: sendMock } }
+
+    const { setupAutoUpdater, checkForUpdatesFromMenu } = await import('./updater')
+
+    setupAutoUpdater(mainWindow as never, { getLastUpdateCheckAt: () => Date.now() })
+    autoUpdaterMock.emit('checking-for-update')
+    autoUpdaterMock.emit('update-available', { version: '1.0.61' })
+
+    await vi.waitFor(() => {
+      expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
+    })
+
+    autoUpdaterMock.emit('download-progress', { percent: 10 })
+    sendMock.mockClear()
+    checkForUpdatesFromMenu()
+    rejectDownload(new Error('disk full'))
+
+    await vi.waitFor(() => {
+      expect(sendMock).toHaveBeenCalledWith('updater:status', {
+        state: 'error',
+        message: 'disk full',
+        userInitiated: true
+      })
+    })
+  })
+
   it('preserves user initiation when a manual auto-download emits an error before rejecting', async () => {
     autoUpdaterMock.checkForUpdates.mockImplementation(() => {
       autoUpdaterMock.emit('checking-for-update')

src/main/updater.ts

@@ -306,6 +306,7 @@ function shouldAcceptDownloadedUpdate(version: string): boolean {
 
 function getCurrentActionableUpdateUserInitiated(): boolean | undefined {
   if (
+    currentStatus.state === 'checking' ||
     currentStatus.state === 'available' ||
     currentStatus.state === 'downloading' ||
     currentStatus.state === 'downloaded'
@@ -1050,15 +1051,29 @@ function startAvailableUpdateDownload(): void {
     return
   }
   activeDownloadVersion = targetVersion
-  // Why: electron-updater can emit 'error' before downloadUpdate() rejects; the
-  // catch must not recompute intent from the error status written by the event.
+  // Why: electron-updater can emit 'error' before downloadUpdate() rejects;
+  // capture launch intent, then merge any manual promotion observed by catch.
   const downloadUserInitiated = getCurrentActionableUpdateUserInitiated()
   beginMacUpdateDownload()
   getAutoUpdater()
     .downloadUpdate()
     .catch((err) => {
+      const message = String(err?.message ?? err)
+      const existingMatchingErrorUserInitiated =
+        currentStatus.state === 'error' && currentStatus.message === message
+          ? currentStatus.userInitiated
+          : undefined
+      // Why: a manual check can promote an active background download while
+      // the download promise is still pending; preserve that promotion.
+      const wasUserInitiated =
+        downloadUserInitiated ||
+        getCurrentActionableUpdateUserInitiated() ||
+        existingMatchingErrorUserInitiated ||
+        userInitiatedCheck ||
+        backgroundCheckPromotedToUserInitiated ||
+        undefined
       clearActiveUpdateDownload(targetVersion)
-      sendErrorStatus(String(err?.message ?? err), downloadUserInitiated)
+      sendErrorStatus(message, wasUserInitiated)
     })
 }
 

src/renderer/src/components/UpdateCard.test.ts

@@ -370,16 +370,6 @@ function computeVisibility(input: VisibilityInput): VisibilityResult {
   ) {
     return 'hidden'
   }
-  if (
-    status.state === 'downloaded' &&
-    !userInitiatedCycle &&
-    !hasStartedDownload &&
-    !hasExplicitDownloadIntent &&
-    !isNudgeDriven &&
-    !isUserInitiated
-  ) {
-    return 'hidden'
-  }
   if (status.state === 'error' && !shouldShowDetailedErrorCard && !isUserInitiated) {
     return 'hidden'
   }
@@ -393,7 +383,11 @@ function computeVisibility(input: VisibilityInput): VisibilityResult {
     !hasExplicitDownloadIntent &&
     !isNudgeDriven
   ) {
-    if (status.state !== 'downloading' && status.state !== 'error') {
+    if (
+      status.state !== 'downloading' &&
+      status.state !== 'downloaded' &&
+      status.state !== 'error'
+    ) {
       return 'hidden'
     }
   }
@@ -603,7 +597,7 @@ describe('UpdateCard visibility gates', () => {
     ).toBe('visible')
   })
 
-  it('hides downloaded when version is dismissed and no explicit download is active', () => {
+  it('shows downloaded when version is dismissed and no explicit download is active', () => {
     expect(
       computeVisibility({
         status: { state: 'downloaded', version: '1.2.0' },
@@ -611,7 +605,7 @@ describe('UpdateCard visibility gates', () => {
         cachedVersion: '1.2.0',
         hasStartedDownload: false
       })
-    ).toBe('hidden')
+    ).toBe('visible')
   })
 
   it('shows settings-initiated downloaded state even when the version was dismissed', () => {
@@ -626,7 +620,7 @@ describe('UpdateCard visibility gates', () => {
     ).toBe('visible')
   })
 
-  it('hides ordinary background downloaded updates until the user checks', () => {
+  it('shows ordinary background downloaded updates once the update is ready to install', () => {
     expect(
       computeVisibility({
         status: { state: 'downloaded', version: '1.2.0' },
@@ -634,7 +628,7 @@ describe('UpdateCard visibility gates', () => {
         cachedVersion: '1.2.0',
         hasStartedDownload: false
       })
-    ).toBe('hidden')
+    ).toBe('visible')
   })
 
   it('shows downloaded updates after a user-initiated check', () => {
@@ -753,12 +747,15 @@ describe('UpdateCard visibility gates', () => {
 // ── Status-bar update segment gates ─────────────────────────────────
 
 describe('UpdateStatusSegment visibility gates', () => {
-  it('hides suppressed passive background download states', () => {
+  it('hides passive background download progress', () => {
     expect(
       shouldShowUpdateStatusSegment({ state: 'downloading', percent: 50, version: '1.2.0' }, null)
     ).toBe(false)
+  })
+
+  it('shows passive downloaded updates so the install UI is discoverable', () => {
     expect(shouldShowUpdateStatusSegment({ state: 'downloaded', version: '1.2.0' }, null)).toBe(
-      false
+      true
     )
   })
 

src/renderer/src/components/UpdateCard.tsx

@@ -265,17 +265,6 @@ export function UpdateCard() {
   ) {
     return null
   }
-  if (
-    status.state === 'downloaded' &&
-    !userInitiatedCycleRef.current &&
-    !hasStartedDownload.current &&
-    !hasExplicitDownloadIntent &&
-    !isNudgeDriven &&
-    !isUserInitiated
-  ) {
-    return null
-  }
-
   // Error: show card for user-initiated check failures or for failures tied to
   // a concrete cached update version (card-initiated and Settings-initiated
   // download/install flows). Background check failures stay silent.
@@ -291,8 +280,8 @@ export function UpdateCard() {
   }
 
   // Dismiss gate: if the user previously dismissed this version, hide the card
-  // for passive reminder states. Keep active in-progress/error states visible so
-  // explicit install actions can still surface progress and failures.
+  // for passive reminder states. Keep in-progress/error and ready-to-install
+  // states visible so users can always finish or recover an update.
   // Why: bypass the gate when the current cycle was user-initiated — the user
   // explicitly asked to check, so they expect to see the result even if they
   // dismissed the same version earlier.
@@ -304,7 +293,11 @@ export function UpdateCard() {
     !hasExplicitDownloadIntent &&
     !isNudgeDriven
   ) {
-    if (status.state !== 'downloading' && status.state !== 'error') {
+    if (
+      status.state !== 'downloading' &&
+      status.state !== 'downloaded' &&
+      status.state !== 'error'
+    ) {
       return null
     }
   }

src/renderer/src/components/status-bar/update-status-segment-visibility.ts

@@ -11,9 +11,14 @@ export function shouldShowUpdateStatusSegment(
     downloadIntentVersion !== null &&
     status.version === downloadIntentVersion
 
-  if (status.state === 'downloading' || status.state === 'downloaded') {
+  if (status.state === 'downloading') {
     return isUserInitiated || isNudgeDriven || matchesExplicitDownload
   }
+  if (status.state === 'downloaded') {
+    // Why: passive background downloads are quiet while in progress, but once
+    // ready they need a persistent way back to the install UI.
+    return true
+  }
   if (status.state === 'error') {
     return isUserInitiated || isNudgeDriven || downloadIntentVersion !== null
   }