Skip to content

Commit c4f78e5

Browse files
authored
feat(desktop): drive updates from a sidebar button and one overlay (#172)
## Related Issue No issue — requested directly: an Update button at the top of the sidebar that opens one centred overlay with download, progress, and cancel, then installs. ## Problem The desktop app could already find, download, and install an update, but the surface around it was wrong in three ways: 1. **No entry point.** Nothing in the main window said an update was waiting. Discovering one meant opening Settings. 2. **No way out of a download.** Once bytes were moving there was no cancel — only quitting the app, which leaves a partial file behind. 3. **Two places showed progress.** A bottom-right toast and the Settings panel both rendered the same feed, and the toast could surface over other UI while the user was working. ## What changed **Sidebar** — an Update button sits above New Chat. It renders only on desktop, only when a version is waiting, and only when that version has not been skipped. **One overlay** carries the whole flow: | state | shows | | --- | --- | | available | version, release date, **Download** / Skip / View notes | | downloading | transferred against total, progress bar, **Cancel download** | | downloaded | **Restart to update** / Later | | error | Retry / Dismiss | While a download is running, overlay-click and Esc are disabled — Cancel is the only exit, so the frame cannot be dismissed out from under an in-flight transfer. **Cancel** is the only genuinely new capability in the main process. Two things made it more than a one-liner: - `electron-updater` reports a cancelled download through the same `error` path as a real failure, and `CancellationError` carries no distinguishing `name`. The token itself is the discriminator, and the guard reads **the token that owns the rejection** rather than the current one — so a retry started after a cancel cannot inherit the cancelled download's error. - `builder-util-runtime` was a phantom dependency: used through `electron-updater` but never declared. It is now declared at `9.7.0`. The lockfile resolves exactly one copy, `electron-updater` pins the same one, and the package contains no `instanceof CancellationToken`, so class identity is not load-bearing. **Removed** `UpdateToast.vue` and its test. Download and install remain two deliberate actions — the main process refuses to combine them, and a test holds that line. The shared `useDesktopUpdate` composable keeps module-level state on purpose: the sidebar button and the overlay read one feed, so two components can never open two `onUpdateState` listeners, and the button must know whether to render before the overlay is ever mounted. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/PyModel/pythinker-code/blob/main/CONTRIBUTING.md) document. - [ ] I have linked a related issue (external PRs: the issue must have a maintainer's `/approve`). - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. ## Verification desktop 157/157 · web 939/939 · `pnpm run typecheck` 0 · `pnpm run lint` 0 errors · `check-nix-workspace.mjs` 0 · `nix build .#pythinker-code` exit 0 (the lockfile change is importer-only, so the `pnpmDeps` hash is unaffected) · `pnpm run build:web` 0 with the shipped bundle rebuilt. All six new tests are mutation-proven. Dropping the cancellation token, removing the cancelled-download guard, unguarding the dialog close, pointing Cancel at download, rendering the sidebar button unconditionally, and neutering its click each turn exactly their own test red — no test passes for a reason other than the one it claims. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a desktop sidebar update button with the available version displayed. * Added an update dialog for reviewing release notes, downloading updates, monitoring progress, cancelling downloads, retrying, skipping versions, and restarting to apply updates. * Added localized update messaging and responsive progress displays. * **Bug Fixes** * Cancelled downloads now safely return to the available state without showing errors. * Update retries are handled more reliably after interrupted downloads. * **Tests** * Added coverage for update actions, progress states, cancellation, retries, restart flows, and sidebar behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 5439d36 commit c4f78e5

112 files changed

Lines changed: 1379 additions & 876 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": minor
3+
---
4+
5+
Add an Update button to the top of the desktop sidebar that opens a single overlay for the whole update: download it, watch the progress, cancel it mid-transfer, then restart to apply it.

apps/desktop/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"@pymodel/pythinker-telemetry": "workspace:*",
2020
"@types/node": "^26.1.2",
2121
"@types/semver": "^7.7.0",
22+
"builder-util-runtime": "9.7.0",
2223
"electron": "43.4.0",
2324
"electron-builder": "26.15.3",
2425
"electron-updater": "6.8.9",

apps/desktop/src/main.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
import { createSplashWindow } from './splash'
3939
import {
4040
acknowledgeCompletedUpdate,
41+
cancelUpdateDownload,
4142
checkForUpdatesNow,
4243
getUpdateState,
4344
initUpdater,
@@ -274,6 +275,10 @@ ipcMain.handle('pythinker:update:download', (event) => {
274275
assertTrustedSender(event)
275276
return startUpdateDownload()
276277
})
278+
ipcMain.handle('pythinker:update:cancel', (event) => {
279+
assertTrustedSender(event)
280+
return cancelUpdateDownload()
281+
})
277282
ipcMain.handle('pythinker:update:skip', (event, version: unknown) => {
278283
assertTrustedSender(event)
279284
return skipUpdate(updateVersion(version))

apps/desktop/src/preload.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ contextBridge.exposeInMainWorld('pythinkerDesktop', {
66
setAutoUpdate: (enabled: boolean) => ipcRenderer.invoke('pythinker:update:set-auto', enabled),
77
checkForUpdates: () => ipcRenderer.invoke('pythinker:update:check'),
88
downloadUpdate: () => ipcRenderer.invoke('pythinker:update:download'),
9+
cancelUpdateDownload: () => ipcRenderer.invoke('pythinker:update:cancel'),
910
skipUpdate: (version: string) => ipcRenderer.invoke('pythinker:update:skip', version),
1011
undoSkippedUpdate: () => ipcRenderer.invoke('pythinker:update:undo-skip'),
1112
markUpdateNotified: (version: string) => ipcRenderer.invoke('pythinker:update:notified', version),

apps/desktop/src/updater.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { existsSync, readFileSync, writeFileSync } from 'node:fs'
22
import { join } from 'node:path'
33
import { app, type BrowserWindow } from 'electron'
4+
import { CancellationToken } from 'builder-util-runtime'
45
import electronUpdater, { type ProgressInfo, type UpdateInfo } from 'electron-updater'
56
import { gt, valid } from 'semver'
67

@@ -130,6 +131,8 @@ let initialCheckTimer: ReturnType<typeof setTimeout> | undefined
130131
let checkInterval: ReturnType<typeof setInterval> | undefined
131132
let checkPromise: Promise<UpdateState> | undefined
132133
let installRequestedVersion: string | undefined
134+
let activeDownloadToken: CancellationToken | undefined
135+
let restartDownloadWhenSettled = false
133136
let listenersWired = false
134137
let initialized = false
135138
let updateTelemetryTrack: UpdateTelemetryTrack = () => {}
@@ -207,6 +210,20 @@ function stateError(error: unknown): void {
207210
})
208211
}
209212

213+
/**
214+
* electron-updater reports a cancelled download through the same `error` path
215+
* as a genuine failure, and `CancellationError` carries no distinguishing
216+
* `name`, so the token itself is the discriminator: `cancel()` flips
217+
* `cancelled` synchronously, before the rejection reaches us. The guard reads
218+
* the token that owns this rejection rather than the current one.
219+
*
220+
* That alone is not enough to make a retry safe — see `beginDownload`.
221+
*/
222+
function downloadError(token: CancellationToken, error: unknown): void {
223+
if (token.cancelled) return
224+
stateError(error)
225+
}
226+
210227
function clearTimers(): void {
211228
if (initialCheckTimer !== undefined) clearTimeout(initialCheckTimer)
212229
if (checkInterval !== undefined) clearInterval(checkInterval)
@@ -443,6 +460,29 @@ export function undoSkippedUpdate(): UpdateState {
443460
return state
444461
}
445462

463+
/**
464+
* `AppUpdater.downloadUpdate` returns the in-flight `downloadPromise` when one
465+
* exists and ignores the token it is handed. A download started before the
466+
* previous one has settled would therefore be bound to the older promise — so
467+
* cancelling and immediately downloading again would surface the cancelled
468+
* attempt's rejection as this attempt's error. Hold the new start until the
469+
* previous promise settles, and only then ask for a fresh one.
470+
*/
471+
function beginDownload(): void {
472+
const token = new CancellationToken()
473+
activeDownloadToken = token
474+
void autoUpdater
475+
.downloadUpdate(token)
476+
.catch((error: unknown) => downloadError(token, error))
477+
.finally(() => {
478+
if (activeDownloadToken === token) activeDownloadToken = undefined
479+
token.dispose()
480+
if (!restartDownloadWhenSettled) return
481+
restartDownloadWhenSettled = false
482+
if (state.status === 'downloading') beginDownload()
483+
})
484+
}
485+
446486
export function startUpdateDownload(): UpdateState {
447487
const canDownload = state.status === 'available'
448488
|| (state.status === 'error' && state.availableVersion !== undefined)
@@ -457,13 +497,39 @@ export function startUpdateDownload(): UpdateState {
457497
bytesPerSecond: undefined,
458498
message: undefined,
459499
})
460-
void autoUpdater.downloadUpdate().catch(stateError)
500+
if (activeDownloadToken !== undefined) {
501+
restartDownloadWhenSettled = true
502+
return state
503+
}
504+
beginDownload()
461505
} catch (error) {
506+
activeDownloadToken = undefined
507+
restartDownloadWhenSettled = false
462508
stateError(error)
463509
}
464510
return state
465511
}
466512

513+
/**
514+
* Aborts an in-flight download and returns the update to the state it had
515+
* before the user consented, so the same version can be downloaded again.
516+
*/
517+
export function cancelUpdateDownload(): UpdateState {
518+
const token = activeDownloadToken
519+
if (state.status !== 'downloading' || token === undefined) return state
520+
restartDownloadWhenSettled = false
521+
token.cancel()
522+
updateState({
523+
status: 'available',
524+
percent: undefined,
525+
transferred: undefined,
526+
total: undefined,
527+
bytesPerSecond: undefined,
528+
message: undefined,
529+
})
530+
return state
531+
}
532+
467533
export function installDownloadedUpdateNow(): UpdateState {
468534
const version = state.availableVersion
469535
if (!app.isPackaged || !hasUpdateConfig() || state.status !== 'downloaded' || version === undefined) {

apps/desktop/tests/updater.spec.ts

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,212 @@ describe('strict update consent', () => {
388388
expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledTimes(2)
389389
expect(localAutoUpdater.autoDownload).toBe(false)
390390
})
391+
392+
it('cancelling an in-flight download aborts the transfer and restores the available state', async () => {
393+
vi.resetModules()
394+
const directory = temporaryDirectory()
395+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
396+
writeFileSync(join(directory, 'update-settings.json'), '{"autoUpdate":false}\n', 'utf8')
397+
const { app: localApp } = await import('electron')
398+
const { default: localElectronUpdater } = await import('electron-updater')
399+
const {
400+
cancelUpdateDownload: cancelLocalUpdateDownload,
401+
getUpdateState: getLocalUpdateState,
402+
initUpdater: initLocalUpdater,
403+
startUpdateDownload: startLocalUpdateDownload,
404+
} = await import('../src/updater')
405+
const localAutoUpdater = localElectronUpdater.autoUpdater
406+
vi.mocked(localApp.getPath).mockReturnValue(directory)
407+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
408+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
409+
410+
initLocalUpdater(() => undefined)
411+
const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
412+
([event]) => event === 'update-available',
413+
)?.[1] as ((info: { version: string }) => void) | undefined
414+
const progress = vi.mocked(localAutoUpdater.on).mock.calls.find(
415+
([event]) => event === 'download-progress',
416+
)?.[1] as ((info: {
417+
percent: number
418+
transferred: number
419+
total: number
420+
bytesPerSecond: number
421+
}) => void) | undefined
422+
available?.({ version: '1.2.3' })
423+
424+
startLocalUpdateDownload()
425+
progress?.({ percent: 40, transferred: 400, total: 1_000, bytesPerSecond: 80 })
426+
expect(getLocalUpdateState()).toMatchObject({ status: 'downloading', percent: 40 })
427+
428+
const token = vi.mocked(localAutoUpdater.downloadUpdate).mock.calls[0]?.[0] as
429+
| { cancelled: boolean }
430+
| undefined
431+
expect(token).toBeDefined()
432+
expect(token?.cancelled).toBe(false)
433+
434+
expect(cancelLocalUpdateDownload()).toMatchObject({
435+
status: 'available',
436+
availableVersion: '1.2.3',
437+
percent: undefined,
438+
transferred: undefined,
439+
total: undefined,
440+
bytesPerSecond: undefined,
441+
})
442+
expect(token?.cancelled).toBe(true)
443+
expect(localAutoUpdater.autoDownload).toBe(false)
444+
expect(localAutoUpdater.quitAndInstall).not.toHaveBeenCalled()
445+
})
446+
447+
it('does not report an error when the cancelled download rejects', async () => {
448+
vi.resetModules()
449+
const directory = temporaryDirectory()
450+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
451+
writeFileSync(join(directory, 'update-settings.json'), '{"autoUpdate":false}\n', 'utf8')
452+
const { app: localApp } = await import('electron')
453+
const { default: localElectronUpdater } = await import('electron-updater')
454+
const {
455+
cancelUpdateDownload: cancelLocalUpdateDownload,
456+
getUpdateState: getLocalUpdateState,
457+
initUpdater: initLocalUpdater,
458+
startUpdateDownload: startLocalUpdateDownload,
459+
} = await import('../src/updater')
460+
const localAutoUpdater = localElectronUpdater.autoUpdater
461+
vi.mocked(localApp.getPath).mockReturnValue(directory)
462+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
463+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
464+
465+
let rejectDownload: ((error: Error) => void) | undefined
466+
vi.mocked(localAutoUpdater.downloadUpdate).mockReturnValueOnce(
467+
new Promise<string[]>((_resolve, reject) => {
468+
rejectDownload = reject
469+
}),
470+
)
471+
472+
initLocalUpdater(() => undefined)
473+
const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
474+
([event]) => event === 'update-available',
475+
)?.[1] as ((info: { version: string }) => void) | undefined
476+
available?.({ version: '1.2.3' })
477+
478+
startLocalUpdateDownload()
479+
cancelLocalUpdateDownload()
480+
rejectDownload?.(new Error('cancelled'))
481+
await Promise.resolve()
482+
await Promise.resolve()
483+
484+
expect(getLocalUpdateState()).toMatchObject({ status: 'available', availableVersion: '1.2.3' })
485+
expect(getLocalUpdateState().message).toBeUndefined()
486+
})
487+
488+
it('retries a download requested before the cancelled one settled', async () => {
489+
vi.resetModules()
490+
const directory = temporaryDirectory()
491+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
492+
writeFileSync(join(directory, 'update-settings.json'), '{"autoUpdate":false}\n', 'utf8')
493+
const { app: localApp } = await import('electron')
494+
const { default: localElectronUpdater } = await import('electron-updater')
495+
const {
496+
cancelUpdateDownload: cancelLocalUpdateDownload,
497+
getUpdateState: getLocalUpdateState,
498+
initUpdater: initLocalUpdater,
499+
startUpdateDownload: startLocalUpdateDownload,
500+
} = await import('../src/updater')
501+
const localAutoUpdater = localElectronUpdater.autoUpdater
502+
vi.mocked(localApp.getPath).mockReturnValue(directory)
503+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
504+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
505+
506+
// Mirrors AppUpdater.downloadUpdate: an in-flight download is handed back
507+
// to the next caller and the token it passes is ignored. The promise that
508+
// clears the slot is the one the caller receives, so the slot is already
509+
// free by the time the caller's own handlers run.
510+
let rejectDownload: ((error: Error) => void) | undefined
511+
let downloadPromise: Promise<string[]> | null = null
512+
vi.mocked(localAutoUpdater.downloadUpdate).mockImplementation(() => {
513+
if (downloadPromise !== null) return downloadPromise
514+
const inner = new Promise<string[]>((_resolve, reject) => {
515+
rejectDownload = reject
516+
})
517+
downloadPromise = inner.finally(() => {
518+
downloadPromise = null
519+
})
520+
return downloadPromise
521+
})
522+
523+
initLocalUpdater(() => undefined)
524+
const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
525+
([event]) => event === 'update-available',
526+
)?.[1] as ((info: { version: string }) => void) | undefined
527+
available?.({ version: '1.2.3' })
528+
529+
startLocalUpdateDownload()
530+
cancelLocalUpdateDownload()
531+
startLocalUpdateDownload()
532+
533+
// The retry must not reach electron-updater yet: it would be handed the
534+
// cancelled download and inherit its rejection.
535+
expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledOnce()
536+
537+
rejectDownload?.(new Error('cancelled'))
538+
await new Promise((resolve) => setTimeout(resolve, 0))
539+
540+
expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledTimes(2)
541+
const retryToken = vi.mocked(localAutoUpdater.downloadUpdate).mock.calls[1]?.[0] as
542+
| { cancelled: boolean }
543+
| undefined
544+
expect(retryToken?.cancelled).toBe(false)
545+
expect(getLocalUpdateState()).toMatchObject({ status: 'downloading' })
546+
expect(getLocalUpdateState().message).toBeUndefined()
547+
})
548+
549+
it('drops a deferred retry when the user cancels again', async () => {
550+
vi.resetModules()
551+
const directory = temporaryDirectory()
552+
writeFileSync(join(directory, 'app-update.yml'), '', 'utf8')
553+
writeFileSync(join(directory, 'update-settings.json'), '{"autoUpdate":false}\n', 'utf8')
554+
const { app: localApp } = await import('electron')
555+
const { default: localElectronUpdater } = await import('electron-updater')
556+
const {
557+
cancelUpdateDownload: cancelLocalUpdateDownload,
558+
getUpdateState: getLocalUpdateState,
559+
initUpdater: initLocalUpdater,
560+
startUpdateDownload: startLocalUpdateDownload,
561+
} = await import('../src/updater')
562+
const localAutoUpdater = localElectronUpdater.autoUpdater
563+
vi.mocked(localApp.getPath).mockReturnValue(directory)
564+
Object.defineProperty(localApp, 'isPackaged', { configurable: true, value: true })
565+
Object.defineProperty(process, 'resourcesPath', { configurable: true, value: directory })
566+
567+
let rejectDownload: ((error: Error) => void) | undefined
568+
let downloadPromise: Promise<string[]> | null = null
569+
vi.mocked(localAutoUpdater.downloadUpdate).mockImplementation(() => {
570+
if (downloadPromise !== null) return downloadPromise
571+
const inner = new Promise<string[]>((_resolve, reject) => {
572+
rejectDownload = reject
573+
})
574+
downloadPromise = inner.finally(() => {
575+
downloadPromise = null
576+
})
577+
return downloadPromise
578+
})
579+
580+
initLocalUpdater(() => undefined)
581+
const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
582+
([event]) => event === 'update-available',
583+
)?.[1] as ((info: { version: string }) => void) | undefined
584+
available?.({ version: '1.2.3' })
585+
586+
startLocalUpdateDownload()
587+
cancelLocalUpdateDownload()
588+
startLocalUpdateDownload()
589+
cancelLocalUpdateDownload()
590+
591+
rejectDownload?.(new Error('cancelled'))
592+
await new Promise((resolve) => setTimeout(resolve, 0))
593+
594+
expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledOnce()
595+
expect(getLocalUpdateState()).toMatchObject({ status: 'available', availableVersion: '1.2.3' })
596+
})
391597
})
392598

393599
describe('update prompt receipts', () => {
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
{
2-
"sourceHash": "a99e6d017b3b0411cf2f7f4573234f80e7085afb05ad65a58d20642184cf0c38",
3-
"sourceFileCount": 390
2+
"sourceHash": "61f52b7dee70da78fd65a438890724971dc3bc50b828618503942a9370c1a583",
3+
"sourceFileCount": 391
44
}

0 commit comments

Comments
 (0)