fix(desktop): hold a re-requested download until the cancelled one settles

electron-updater hands the in-flight download promise to the next caller and
ignores the token it is given, so downloading again straight after a cancel
bound the new attempt to the cancelled promise and surfaced its rejection as
an error. Defer the restart until the previous promise settles, and drop the
deferred restart if the user cancels again.

Also assert the dialog close control exists before clicking it, so the test
cannot pass without it.

Author: elkaix

apps/desktop/src/updater.ts

@@ -132,6 +132,7 @@ let checkInterval: ReturnType<typeof setInterval> | undefined
 let checkPromise: Promise<UpdateState> | undefined
 let installRequestedVersion: string | undefined
 let activeDownloadToken: CancellationToken | undefined
+let restartDownloadWhenSettled = false
 let listenersWired = false
 let initialized = false
 let updateTelemetryTrack: UpdateTelemetryTrack = () => {}
@@ -214,8 +215,9 @@ function stateError(error: unknown): void {
  * as a genuine failure, and `CancellationError` carries no distinguishing
  * `name`, so the token itself is the discriminator: `cancel()` flips
  * `cancelled` synchronously, before the rejection reaches us. The guard reads
- * the token that owns this rejection rather than the current one, so a retry
- * started after a cancel cannot inherit the cancelled download's failure.
+ * the token that owns this rejection rather than the current one.
+ *
+ * That alone is not enough to make a retry safe — see `beginDownload`.
  */
 function downloadError(token: CancellationToken, error: unknown): void {
   if (token.cancelled) return
@@ -458,6 +460,29 @@ export function undoSkippedUpdate(): UpdateState {
   return state
 }
 
+/**
+ * `AppUpdater.downloadUpdate` returns the in-flight `downloadPromise` when one
+ * exists and ignores the token it is handed. A download started before the
+ * previous one has settled would therefore be bound to the older promise — so
+ * cancelling and immediately downloading again would surface the cancelled
+ * attempt's rejection as this attempt's error. Hold the new start until the
+ * previous promise settles, and only then ask for a fresh one.
+ */
+function beginDownload(): void {
+  const token = new CancellationToken()
+  activeDownloadToken = token
+  void autoUpdater
+    .downloadUpdate(token)
+    .catch((error: unknown) => downloadError(token, error))
+    .finally(() => {
+      if (activeDownloadToken === token) activeDownloadToken = undefined
+      token.dispose()
+      if (!restartDownloadWhenSettled) return
+      restartDownloadWhenSettled = false
+      if (state.status === 'downloading') beginDownload()
+    })
+}
+
 export function startUpdateDownload(): UpdateState {
   const canDownload = state.status === 'available'
     || (state.status === 'error' && state.availableVersion !== undefined)
@@ -472,17 +497,14 @@ export function startUpdateDownload(): UpdateState {
       bytesPerSecond: undefined,
       message: undefined,
     })
-    const token = new CancellationToken()
-    activeDownloadToken = token
-    void autoUpdater
-      .downloadUpdate(token)
-      .catch((error: unknown) => downloadError(token, error))
-      .finally(() => {
-        if (activeDownloadToken === token) activeDownloadToken = undefined
-        token.dispose()
-      })
+    if (activeDownloadToken !== undefined) {
+      restartDownloadWhenSettled = true
+      return state
+    }
+    beginDownload()
   } catch (error) {
     activeDownloadToken = undefined
+    restartDownloadWhenSettled = false
     stateError(error)
   }
   return state
@@ -495,6 +517,7 @@ export function startUpdateDownload(): UpdateState {
 export function cancelUpdateDownload(): UpdateState {
   const token = activeDownloadToken
   if (state.status !== 'downloading' || token === undefined) return state
+  restartDownloadWhenSettled = false
   token.cancel()
   updateState({
     status: 'available',

apps/desktop/tests/updater.spec.ts

@@ -484,6 +484,116 @@ describe('strict update consent', () => {
     expect(getLocalUpdateState()).toMatchObject({ status: 'available', availableVersion: '1.2.3' })
     expect(getLocalUpdateState().message).toBeUndefined()
   })
+
+  it('retries a download requested before the cancelled one settled', async () => {
+    vi.resetModules()
+    const directory = temporaryDirectory()
+    writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
+    writeFileSync(join(directory, 'update-settings.json'), '{"autoUpdate":false}\n', 'utf8')
+    const { app: localApp } = await import('electron')
+    const { default: localElectronUpdater } = await import('electron-updater')
+    const {
+      cancelUpdateDownload: cancelLocalUpdateDownload,
+      getUpdateState: getLocalUpdateState,
+      initUpdater: initLocalUpdater,
+      startUpdateDownload: startLocalUpdateDownload,
+    } = await import('../src/updater')
+    const localAutoUpdater = localElectronUpdater.autoUpdater
+    vi.mocked(localApp.getPath).mockReturnValue(directory)
+    Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
+    Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
+
+    // Mirrors AppUpdater.downloadUpdate: an in-flight download is handed back
+    // to the next caller and the token it passes is ignored. The promise that
+    // clears the slot is the one the caller receives, so the slot is already
+    // free by the time the caller's own handlers run.
+    let rejectDownload: ((error: Error) => void) | undefined
+    let downloadPromise: Promise<string[]> | null = null
+    vi.mocked(localAutoUpdater.downloadUpdate).mockImplementation(() => {
+      if (downloadPromise !== null) return downloadPromise
+      const inner = new Promise<string[]>((_resolve, reject) => {
+        rejectDownload = reject
+      })
+      downloadPromise = inner.finally(() => {
+        downloadPromise = null
+      })
+      return downloadPromise
+    })
+
+    initLocalUpdater(() => undefined)
+    const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
+      ([event]) => event === 'update-available',
+    )?.[1] as ((info: { version: string }) => void) | undefined
+    available?.({ version: '1.2.3' })
+
+    startLocalUpdateDownload()
+    cancelLocalUpdateDownload()
+    startLocalUpdateDownload()
+
+    // The retry must not reach electron-updater yet: it would be handed the
+    // cancelled download and inherit its rejection.
+    expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledOnce()
+
+    rejectDownload?.(new Error('cancelled'))
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledTimes(2)
+    const retryToken = vi.mocked(localAutoUpdater.downloadUpdate).mock.calls[1]?.[0] as
+      | { cancelled: boolean }
+      | undefined
+    expect(retryToken?.cancelled).toBe(false)
+    expect(getLocalUpdateState()).toMatchObject({ status: 'downloading' })
+    expect(getLocalUpdateState().message).toBeUndefined()
+  })
+
+  it('drops a deferred retry when the user cancels again', async () => {
+    vi.resetModules()
+    const directory = temporaryDirectory()
+    writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
+    writeFileSync(join(directory, 'update-settings.json'), '{"autoUpdate":false}\n', 'utf8')
+    const { app: localApp } = await import('electron')
+    const { default: localElectronUpdater } = await import('electron-updater')
+    const {
+      cancelUpdateDownload: cancelLocalUpdateDownload,
+      getUpdateState: getLocalUpdateState,
+      initUpdater: initLocalUpdater,
+      startUpdateDownload: startLocalUpdateDownload,
+    } = await import('../src/updater')
+    const localAutoUpdater = localElectronUpdater.autoUpdater
+    vi.mocked(localApp.getPath).mockReturnValue(directory)
+    Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
+    Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
+
+    let rejectDownload: ((error: Error) => void) | undefined
+    let downloadPromise: Promise<string[]> | null = null
+    vi.mocked(localAutoUpdater.downloadUpdate).mockImplementation(() => {
+      if (downloadPromise !== null) return downloadPromise
+      const inner = new Promise<string[]>((_resolve, reject) => {
+        rejectDownload = reject
+      })
+      downloadPromise = inner.finally(() => {
+        downloadPromise = null
+      })
+      return downloadPromise
+    })
+
+    initLocalUpdater(() => undefined)
+    const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
+      ([event]) => event === 'update-available',
+    )?.[1] as ((info: { version: string }) => void) | undefined
+    available?.({ version: '1.2.3' })
+
+    startLocalUpdateDownload()
+    cancelLocalUpdateDownload()
+    startLocalUpdateDownload()
+    cancelLocalUpdateDownload()
+
+    rejectDownload?.(new Error('cancelled'))
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledOnce()
+    expect(getLocalUpdateState()).toMatchObject({ status: 'available', availableVersion: '1.2.3' })
+  })
 })
 
 describe('update prompt receipts', () => {

apps/pythinker-web/test/update-dialog.test.ts

@@ -201,7 +201,9 @@ describe('UpdateDialog', () => {
 
     update.openDialog();
     await nextTick();
-    body().querySelector<HTMLElement>('.ui-dialog__close')?.click();
+    const close = body().querySelector<HTMLElement>('.ui-dialog__close');
+    expect(close).not.toBeNull();
+    close?.click();
     await nextTick();
 
     expect(update.dialogOpen.value).toBe(true);