Fix final update review edge cases

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

src/main/updater.test.ts

@@ -786,6 +786,69 @@ describe('updater', () => {
     expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
   })
 
+  it('does not launch a manual check while an automatic download is active before progress arrives', 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, 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)
+    })
+
+    sendMock.mockClear()
+    checkForUpdatesFromMenu()
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled()
+    expect(sendMock).toHaveBeenCalledWith('updater:status', {
+      state: 'checking',
+      userInitiated: true
+    })
+
+    autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' })
+
+    expect(sendMock).toHaveBeenCalledWith('updater:status', {
+      state: 'downloaded',
+      version: '1.0.61',
+      releaseUrl: undefined,
+      userInitiated: true
+    })
+  })
+
+  it('does not launch a focus-triggered check while an automatic download is active before progress arrives', async () => {
+    let lastUpdateCheckAt = Date.now()
+    autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
+
+    const mainWindow = { webContents: { send: vi.fn() } }
+
+    const { setupAutoUpdater } = await import('./updater')
+
+    setupAutoUpdater(mainWindow as never, {
+      getLastUpdateCheckAt: () => lastUpdateCheckAt
+    })
+    autoUpdaterMock.emit('checking-for-update')
+    autoUpdaterMock.emit('update-available', { version: '1.0.61' })
+
+    await vi.waitFor(() => {
+      expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
+    })
+
+    lastUpdateCheckAt = Date.now() - 25 * 60 * 60 * 1000
+    appMock.emit('browser-window-focus')
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled()
+  })
+
   it('does not start the same active background download again after progress arrives', async () => {
     autoUpdaterMock.checkForUpdates.mockImplementation(() => {
       autoUpdaterMock.emit('checking-for-update')
@@ -824,6 +887,7 @@ describe('updater', () => {
       })
       return Promise.resolve(undefined)
     })
+    autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
 
     const sendMock = vi.fn()
     const mainWindow = { webContents: { send: sendMock } }
@@ -1179,6 +1243,46 @@ describe('updater', () => {
     })
   })
 
+  it('keeps a queued replacement across a later checking event before the older download finishes', async () => {
+    vi.stubGlobal('process', { ...process, platform: 'linux' })
+    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 })
+    autoUpdaterMock.emit('checking-for-update')
+    autoUpdaterMock.emit('update-available', { version: '1.0.62' })
+
+    await vi.waitFor(() => {
+      expect(sendMock).toHaveBeenCalledWith('updater:status', {
+        state: 'available',
+        version: '1.0.62',
+        changelog: null
+      })
+    })
+
+    sendMock.mockClear()
+    autoUpdaterMock.emit('checking-for-update')
+    autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' })
+
+    expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(2)
+    expect(sendMock).not.toHaveBeenCalledWith(
+      'updater:status',
+      expect.objectContaining({ state: 'downloaded', version: '1.0.61' })
+    )
+  })
+
   it('queues a newer replacement while an older download is still active', async () => {
     vi.stubGlobal('process', { ...process, platform: 'linux' })
     autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))

src/main/updater.ts

@@ -104,7 +104,6 @@ function getAutoUpdater(): ElectronAutoUpdater {
 function clearAvailableUpdateContext(): void {
   availableVersion = null
   availableReleaseUrl = null
-  pendingReplacementDownloadVersion = null
 }
 
 function clearStagedUpdateContext(): void {
@@ -306,6 +305,14 @@ function getPendingDownloadVersion(): string | null {
   ) {
     return currentStatus.version
   }
+  // Why: a follow-up freshness check can clear availableVersion while an older
+  // artifact is still finishing; the queued replacement remains the target.
+  if (
+    pendingReplacementDownloadVersion &&
+    compareVersions(pendingReplacementDownloadVersion, app.getVersion()) > 0
+  ) {
+    return pendingReplacementDownloadVersion
+  }
   return null
 }
 
@@ -706,7 +713,7 @@ function retryPrereleaseFallbackAfterMissingManifest(
 function runBackgroundUpdateCheck(
   nudgeId: string | null = getPersistedPendingUpdateNudgeId()
 ): void {
-  if (backgroundCheckLaunchPending || currentStatus.state === 'checking') {
+  if (backgroundCheckLaunchPending || currentStatus.state === 'checking' || activeDownloadVersion) {
     return
   }
   if (!app.isPackaged || is.dev) {
@@ -785,7 +792,8 @@ export function checkForUpdatesFromMenu(options?: { includePrerelease?: boolean
     enableIncludePrerelease()
   }
 
-  const checkAlreadyInFlight = backgroundCheckLaunchPending || currentStatus.state === 'checking'
+  const checkAlreadyInFlight =
+    backgroundCheckLaunchPending || currentStatus.state === 'checking' || activeDownloadVersion
   userInitiatedCheck = true
   // Why: a manual check is independent of any active nudge campaign. Reset the
   // nudge marker so the resulting status is not decorated with activeNudgeId,
@@ -859,7 +867,11 @@ async function checkForUpdateNudge(): Promise<void> {
       return
     }
 
-    if (currentStatus.state === 'checking' || currentStatus.state === 'downloading') {
+    if (
+      currentStatus.state === 'checking' ||
+      currentStatus.state === 'downloading' ||
+      activeDownloadVersion
+    ) {
       return
     }
 
@@ -1034,7 +1046,8 @@ export function setupAutoUpdater(
     if (
       backgroundCheckLaunchPending ||
       currentStatus.state === 'checking' ||
-      currentStatus.state === 'downloading'
+      currentStatus.state === 'downloading' ||
+      activeDownloadVersion
     ) {
       return
     }
@@ -1070,7 +1083,11 @@ function startAvailableUpdateDownload(): void {
     return
   }
   if (activeDownloadVersion) {
-    if (compareVersions(targetVersion, activeDownloadVersion) > 0) {
+    if (
+      compareVersions(targetVersion, activeDownloadVersion) > 0 &&
+      (!pendingReplacementDownloadVersion ||
+        compareVersions(targetVersion, pendingReplacementDownloadVersion) >= 0)
+    ) {
       pendingReplacementDownloadVersion = targetVersion
     }
     return
@@ -1094,6 +1111,10 @@ function startAvailableUpdateDownload(): void {
   beginMacUpdateDownload()
   getAutoUpdater()
     .downloadUpdate()
+    .then(() => {
+      clearActiveUpdateDownload(targetVersion)
+      startPendingReplacementDownload()
+    })
     .catch((err) => {
       const message = String(err?.message ?? err)
       const existingMatchingErrorUserInitiated =

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

@@ -355,6 +355,8 @@ function computeVisibility(input: VisibilityInput): VisibilityResult {
   const isNudgeDriven = 'activeNudgeId' in status && Boolean(status.activeNudgeId)
   const hasExplicitDownloadIntent =
     cachedVersion !== null && downloadIntentVersion === cachedVersion
+  const isLocallyDismissedVersion =
+    cachedVersion !== null && locallyDismissedVersion === cachedVersion
   const shouldShowDetailedErrorCard =
     status.state === 'error' &&
     (isUserInitiated || isNudgeDriven || hasStartedDownload || hasExplicitDownloadIntent)
@@ -381,17 +383,26 @@ function computeVisibility(input: VisibilityInput): VisibilityResult {
   ) {
     return 'hidden'
   }
+  if (
+    status.state === 'downloading' &&
+    isLocallyDismissedVersion &&
+    !hasStartedDownload &&
+    !hasExplicitDownloadIntent &&
+    !isNudgeDriven
+  ) {
+    return 'hidden'
+  }
   if (status.state === 'error' && !shouldShowDetailedErrorCard && !isUserInitiated) {
     return 'hidden'
   }
 
   const effectiveVersion = 'version' in status ? status.version : cachedVersion
-  const isLocallyDismissedVersion =
+  const isEffectiveLocallyDismissedVersion =
     effectiveVersion !== null && locallyDismissedVersion === effectiveVersion
   if (
     effectiveVersion &&
-    (dismissedVersion === effectiveVersion || isLocallyDismissedVersion) &&
-    (isLocallyDismissedVersion ||
+    (dismissedVersion === effectiveVersion || isEffectiveLocallyDismissedVersion) &&
+    (isEffectiveLocallyDismissedVersion ||
       (!userInitiatedCycle && !isUserInitiated && !hasExplicitDownloadIntent && !isNudgeDriven))
   ) {
     if (
@@ -592,6 +603,18 @@ describe('UpdateCard visibility gates', () => {
     ).toBe('visible')
   })
 
+  it('hides user-initiated download progress after the visible available card is dismissed', () => {
+    expect(
+      computeVisibility({
+        status: { state: 'downloading', percent: 42, version: '1.2.0', userInitiated: true },
+        dismissedVersion: '1.2.0',
+        cachedVersion: '1.2.0',
+        hasStartedDownload: false,
+        locallyDismissedVersion: '1.2.0'
+      })
+    ).toBe('hidden')
+  })
+
   it('shows settings-initiated download progress even when the version was dismissed', () => {
     expect(
       computeVisibility({
@@ -599,7 +622,8 @@ describe('UpdateCard visibility gates', () => {
         dismissedVersion: '1.2.0',
         cachedVersion: '1.2.0',
         hasStartedDownload: false,
-        downloadIntentVersion: '1.2.0'
+        downloadIntentVersion: '1.2.0',
+        locallyDismissedVersion: '1.2.0'
       })
     ).toBe('visible')
   })

src/renderer/src/components/UpdateCard.tsx

@@ -217,6 +217,8 @@ export function UpdateCard() {
   const cachedVersion = versionRef.current
   const hasExplicitDownloadIntent =
     cachedVersion !== null && downloadIntentVersion === cachedVersion
+  const isLocallyDismissedVersion =
+    cachedVersion !== null && locallyDismissedVersionRef.current === cachedVersion
   const shouldShowDetailedErrorCard =
     status.state === 'error' &&
     (isUserInitiated || isNudgeDriven || hasStartedDownload.current || hasExplicitDownloadIntent)
@@ -271,6 +273,17 @@ export function UpdateCard() {
   ) {
     return null
   }
+  // Why: closing the available card is also a request to keep the automatic
+  // pre-download quiet; explicit downloads and nudge campaigns still surface.
+  if (
+    status.state === 'downloading' &&
+    isLocallyDismissedVersion &&
+    !hasStartedDownload.current &&
+    !hasExplicitDownloadIntent &&
+    !isNudgeDriven
+  ) {
+    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 +304,6 @@ export function UpdateCard() {
   // 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.
-  const isLocallyDismissedVersion =
-    versionRef.current !== null && locallyDismissedVersionRef.current === versionRef.current
   if (
     versionRef.current &&
     (dismissedVersion === versionRef.current || isLocallyDismissedVersion) &&