Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/desktop-update-overlay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@pymodel/pythinker-code": minor
---

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.
1 change: 1 addition & 0 deletions apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"@pymodel/pythinker-telemetry": "workspace:*",
"@types/node": "^26.1.2",
"@types/semver": "^7.7.0",
"builder-util-runtime": "9.7.0",
"electron": "43.4.0",
"electron-builder": "26.15.3",
"electron-updater": "6.8.9",
Expand Down
5 changes: 5 additions & 0 deletions apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
import { createSplashWindow } from './splash'
import {
acknowledgeCompletedUpdate,
cancelUpdateDownload,
checkForUpdatesNow,
getUpdateState,
initUpdater,
Expand Down Expand Up @@ -274,6 +275,10 @@ ipcMain.handle('pythinker:update:download', (event) => {
assertTrustedSender(event)
return startUpdateDownload()
})
ipcMain.handle('pythinker:update:cancel', (event) => {
assertTrustedSender(event)
return cancelUpdateDownload()
})
ipcMain.handle('pythinker:update:skip', (event, version: unknown) => {
assertTrustedSender(event)
return skipUpdate(updateVersion(version))
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ contextBridge.exposeInMainWorld('pythinkerDesktop', {
setAutoUpdate: (enabled: boolean) => ipcRenderer.invoke('pythinker:update:set-auto', enabled),
checkForUpdates: () => ipcRenderer.invoke('pythinker:update:check'),
downloadUpdate: () => ipcRenderer.invoke('pythinker:update:download'),
cancelUpdateDownload: () => ipcRenderer.invoke('pythinker:update:cancel'),
skipUpdate: (version: string) => ipcRenderer.invoke('pythinker:update:skip', version),
undoSkippedUpdate: () => ipcRenderer.invoke('pythinker:update:undo-skip'),
markUpdateNotified: (version: string) => ipcRenderer.invoke('pythinker:update:notified', version),
Expand Down
68 changes: 67 additions & 1 deletion apps/desktop/src/updater.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { existsSync, readFileSync, writeFileSync } from 'node:fs'
import { join } from 'node:path'
import { app, type BrowserWindow } from 'electron'
import { CancellationToken } from 'builder-util-runtime'
import electronUpdater, { type ProgressInfo, type UpdateInfo } from 'electron-updater'
import { gt, valid } from 'semver'

Expand Down Expand Up @@ -130,6 +131,8 @@ let initialCheckTimer: ReturnType<typeof setTimeout> | undefined
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 = () => {}
Expand Down Expand Up @@ -207,6 +210,20 @@ function stateError(error: unknown): void {
})
}

/**
* electron-updater reports a cancelled download through the same `error` path
* 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.
*
* That alone is not enough to make a retry safe — see `beginDownload`.
*/
function downloadError(token: CancellationToken, error: unknown): void {
if (token.cancelled) return
stateError(error)
}

function clearTimers(): void {
if (initialCheckTimer !== undefined) clearTimeout(initialCheckTimer)
if (checkInterval !== undefined) clearInterval(checkInterval)
Expand Down Expand Up @@ -443,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)
Expand All @@ -457,13 +497,39 @@ export function startUpdateDownload(): UpdateState {
bytesPerSecond: undefined,
message: undefined,
})
void autoUpdater.downloadUpdate().catch(stateError)
if (activeDownloadToken !== undefined) {
restartDownloadWhenSettled = true
return state
}
beginDownload()
} catch (error) {
activeDownloadToken = undefined
restartDownloadWhenSettled = false
stateError(error)
}
return state
}

/**
* Aborts an in-flight download and returns the update to the state it had
* before the user consented, so the same version can be downloaded again.
*/
export function cancelUpdateDownload(): UpdateState {
const token = activeDownloadToken
if (state.status !== 'downloading' || token === undefined) return state
restartDownloadWhenSettled = false
token.cancel()
updateState({
status: 'available',
percent: undefined,
transferred: undefined,
total: undefined,
bytesPerSecond: undefined,
message: undefined,
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return state
}

export function installDownloadedUpdateNow(): UpdateState {
const version = state.availableVersion
if (!app.isPackaged || !hasUpdateConfig() || state.status !== 'downloaded' || version === undefined) {
Expand Down
206 changes: 206 additions & 0 deletions apps/desktop/tests/updater.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,212 @@ describe('strict update consent', () => {
expect(localAutoUpdater.downloadUpdate).toHaveBeenCalledTimes(2)
expect(localAutoUpdater.autoDownload).toBe(false)
})

it('cancelling an in-flight download aborts the transfer and restores the available state', 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 })

initLocalUpdater(() => undefined)
const available = vi.mocked(localAutoUpdater.on).mock.calls.find(
([event]) => event === 'update-available',
)?.[1] as ((info: { version: string }) => void) | undefined
const progress = vi.mocked(localAutoUpdater.on).mock.calls.find(
([event]) => event === 'download-progress',
)?.[1] as ((info: {
percent: number
transferred: number
total: number
bytesPerSecond: number
}) => void) | undefined
available?.({ version: '1.2.3' })

startLocalUpdateDownload()
progress?.({ percent: 40, transferred: 400, total: 1_000, bytesPerSecond: 80 })
expect(getLocalUpdateState()).toMatchObject({ status: 'downloading', percent: 40 })

const token = vi.mocked(localAutoUpdater.downloadUpdate).mock.calls[0]?.[0] as
| { cancelled: boolean }
| undefined
expect(token).toBeDefined()
expect(token?.cancelled).toBe(false)

expect(cancelLocalUpdateDownload()).toMatchObject({
status: 'available',
availableVersion: '1.2.3',
percent: undefined,
transferred: undefined,
total: undefined,
bytesPerSecond: undefined,
})
expect(token?.cancelled).toBe(true)
expect(localAutoUpdater.autoDownload).toBe(false)
expect(localAutoUpdater.quitAndInstall).not.toHaveBeenCalled()
})

it('does not report an error when the cancelled download rejects', 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
vi.mocked(localAutoUpdater.downloadUpdate).mockReturnValueOnce(
new Promise<string[]>((_resolve, reject) => {
rejectDownload = reject
}),
)

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()
rejectDownload?.(new Error('cancelled'))
await Promise.resolve()
await Promise.resolve()

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', () => {
Expand Down
4 changes: 2 additions & 2 deletions apps/pythinker-code/dist-web/.web-bundle-manifest.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"sourceHash": "a99e6d017b3b0411cf2f7f4573234f80e7085afb05ad65a58d20642184cf0c38",
"sourceFileCount": 390
"sourceHash": "61f52b7dee70da78fd65a438890724971dc3bc50b828618503942a9370c1a583",
"sourceFileCount": 391
}
Loading
Loading