Skip to content

Commit 4b1b476

Browse files
committed
fix(desktop): replace a stale staged update with the newest release
1 parent 420c0df commit 4b1b476

3 files changed

Lines changed: 230 additions & 19 deletions

File tree

‎apps/desktop/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ Raw local file bytes are never exposed through the preload bridge and cannot be
172172

173173
## Auto-update, channels, rollout, rollback
174174

175-
- `electron-updater` reads the deployment's `/api/desktop/update` feed; production resolves stable releases from `simstudioai/sim`, while dev/staging resolve prereleases from `simstudioai/sim-desktop-releases`. Artifact downloads go directly to GitHub and deltas use `.zip.blockmap`. Sim validates every candidate before starting its download. Developer ID builds installed under `/Applications` use a prompt (Restart and update / Later; Later installs on quit); other packaged builds offer a validated installer download — never forced mid-session.
175+
- `electron-updater` reads the deployment's `/api/desktop/update` feed; production resolves stable releases from `simstudioai/sim`, while dev/staging resolve prereleases from `simstudioai/sim-desktop-releases`. Artifact downloads go directly to GitHub and deltas use `.zip.blockmap`. Sim validates every candidate before starting its download. Developer ID builds installed under `/Applications` use a prompt (Restart and update / Later; Later installs on quit); other packaged builds offer a validated installer download — never forced mid-session. A staged or offered update keeps being re-checked on the normal cadence, and a newer release replaces it, so a shell left running across several releases installs the latest build in one restart instead of the stale one followed by another prompt.
176176
- Streams: production follows stable `X.Y.Z` releases, dev follows `-dev.N`, and staging follows `-staging.N`. The feed still recognizes legacy `-alpha.N`/`-beta.N` releases during migration.
177177
- Staged rollout: after publishing, edit `stagingPercentage: 10` into the release's `latest-mac.yml`, then raise as crash metrics stay clean.
178178
- Rollback: a pulled release must be superseded by a **higher** version — users on the broken build will not reinstall an equal one. (A blocked-versions kill-switch was removed as unwired dead code; reintroduce it in `updater.ts` if a remote config source ever exists to feed it.)

‎apps/desktop/src/main/updater.test.ts‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,118 @@ describe('initUpdater state machine', () => {
445445
})
446446
})
447447

448+
it('replaces a staged update with a newer release instead of installing the stale build', async () => {
449+
const { handle, states } = await createUpdater()
450+
handle.check()
451+
await vi.advanceTimersByTimeAsync(0)
452+
emit('update-available', { version: '2.0.0' })
453+
emit('update-downloaded', { version: '2.0.0' })
454+
expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' })
455+
states.length = 0
456+
457+
await vi.advanceTimersByTimeAsync(10_000)
458+
expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(2)
459+
emit('checking-for-update')
460+
emit('update-available', { version: '2.1.0' })
461+
emit('download-progress', { percent: 50 })
462+
expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(2)
463+
expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' })
464+
465+
await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
466+
expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(2)
467+
468+
emit('update-downloaded', { version: '2.1.0' })
469+
expect(states).toEqual([{ status: 'ready', version: '2.1.0' }])
470+
expect(autoUpdaterMock.autoInstallOnAppQuit).toBe(true)
471+
expect(events.record).toHaveBeenCalledWith('update_downloaded', { version: '2.1.0' })
472+
})
473+
474+
it('keeps a staged update when a background re-check finds nothing newer or fails', async () => {
475+
const { handle, states } = await createUpdater()
476+
handle.check()
477+
await vi.advanceTimersByTimeAsync(0)
478+
emit('update-available', { version: '2.0.0' })
479+
emit('update-downloaded', { version: '2.0.0' })
480+
states.length = 0
481+
482+
await vi.advanceTimersByTimeAsync(10_000)
483+
emit('update-available', { version: '2.0.0' })
484+
await vi.advanceTimersByTimeAsync(30 * 60 * 1000 - 10_000)
485+
emit('update-not-available')
486+
await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
487+
emit('error', new Error('net::ERR_NETWORK_CHANGED'))
488+
await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
489+
emit('update-available', { version: '2.1.0' })
490+
emit('error', new Error('download interrupted'))
491+
await vi.advanceTimersByTimeAsync(0)
492+
493+
expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(5)
494+
expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(2)
495+
expect(states).toEqual([])
496+
expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' })
497+
expect(autoUpdaterMock.autoInstallOnAppQuit).toBe(true)
498+
})
499+
500+
it('does not replace a staged update when background downloads are disabled', async () => {
501+
const { handle } = await createUpdater()
502+
handle.check()
503+
await vi.advanceTimersByTimeAsync(0)
504+
emit('update-available', { version: '2.0.0' })
505+
emit('update-downloaded', { version: '2.0.0' })
506+
handle.setAutoDownload(false)
507+
508+
await vi.advanceTimersByTimeAsync(10_000)
509+
emit('update-available', { version: '2.1.0' })
510+
511+
expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
512+
expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' })
513+
})
514+
515+
it('installs a replacement that finished staging while the restart prompt was open', async () => {
516+
let resolveConfirmation: (result: { response: number; checkboxChecked: boolean }) => void =
517+
() => {
518+
throw new Error('Restart confirmation did not initialize')
519+
}
520+
const { handle } = await createUpdater()
521+
handle.check()
522+
await vi.advanceTimersByTimeAsync(0)
523+
emit('update-available', { version: '2.0.0' })
524+
emit('update-downloaded', { version: '2.0.0' })
525+
await vi.advanceTimersByTimeAsync(10_000)
526+
emit('update-available', { version: '2.1.0' })
527+
528+
vi.mocked(dialog.showMessageBox).mockImplementationOnce(
529+
() =>
530+
new Promise((resolve) => {
531+
resolveConfirmation = resolve
532+
})
533+
)
534+
handle.install()
535+
emit('update-downloaded', { version: '2.1.0' })
536+
resolveConfirmation({ response: 1, checkboxChecked: false })
537+
await vi.advanceTimersByTimeAsync(0)
538+
539+
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
540+
})
541+
542+
it('refreshes an offered update to a newer release before it is downloaded', async () => {
543+
const { handle } = await createUpdater({ autoDownload: false })
544+
handle.check()
545+
await vi.advanceTimersByTimeAsync(0)
546+
emit('update-available', { version: '2.0.0' })
547+
expect(handle.getState()).toEqual({ status: 'available', version: '2.0.0' })
548+
549+
await vi.advanceTimersByTimeAsync(10_000)
550+
emit('checking-for-update')
551+
emit('error', new Error('net::ERR_INTERNET_DISCONNECTED'))
552+
expect(handle.getState()).toEqual({ status: 'available', version: '2.0.0' })
553+
554+
await vi.advanceTimersByTimeAsync(30 * 60 * 1000 - 10_000)
555+
emit('update-available', { version: '2.1.0' })
556+
expect(handle.getState()).toEqual({ status: 'available', version: '2.1.0' })
557+
expect(autoUpdaterMock.downloadUpdate).not.toHaveBeenCalled()
558+
})
559+
448560
it('checks from idle and ignores re-entrant checks while busy', async () => {
449561
const { handle } = await createUpdater()
450562
handle.check()
@@ -1035,6 +1147,31 @@ describe('initUpdater manual mode (no Developer ID signature)', () => {
10351147
expect(handle.getState()).toEqual({ status: 'error', manual: true })
10361148
})
10371149

1150+
it('replaces an offered manual download with a newer release', async () => {
1151+
let feedVersion: string | null = '2.0.0'
1152+
const fetchManifest = vi.fn(async () => {
1153+
if (feedVersion === null) throw new Error('network down')
1154+
return manifest(feedVersion)
1155+
})
1156+
const { handle } = await createManualUpdater(fetchManifest)
1157+
handle.check()
1158+
await vi.advanceTimersByTimeAsync(0)
1159+
expect(handle.getState()).toEqual({ status: 'available', version: '2.0.0', manual: true })
1160+
1161+
feedVersion = null
1162+
await vi.advanceTimersByTimeAsync(10_000)
1163+
expect(handle.getState()).toEqual({ status: 'available', version: '2.0.0', manual: true })
1164+
1165+
feedVersion = '2.1.0'
1166+
await vi.advanceTimersByTimeAsync(30 * 60 * 1000)
1167+
expect(handle.getState()).toEqual({ status: 'available', version: '2.1.0', manual: true })
1168+
1169+
handle.install()
1170+
expect(shell.openExternal).toHaveBeenCalledWith(
1171+
'https://github.com/simstudioai/sim/releases/download/v2.1.0/Sim-2.1.0-universal.dmg'
1172+
)
1173+
})
1174+
10381175
it('checks on the scheduled interval', async () => {
10391176
const fetchManifest = vi.fn(async () => manifest('9.9.9'))
10401177
await createManualUpdater(fetchManifest)

‎apps/desktop/src/main/updater.ts‎

Lines changed: 92 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -395,13 +395,17 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
395395
let installInFlight = false
396396
let installConfirmationInFlight = false
397397

398-
const quitAndInstall = (version: string | undefined) => {
398+
/**
399+
* Squirrel installs whatever it has staged when the process exits, so a
400+
* newer build that replaced the staged one mid-confirmation still installs.
401+
*/
402+
const quitAndInstall = () => {
399403
if (installInFlight) return
400404
installInFlight = true
401405
void Promise.resolve()
402406
.then(() => deps.beforeInstall?.())
403407
.then(() => {
404-
if (state.status !== 'ready' || state.version !== version) {
408+
if (state.status !== 'ready') {
405409
autoUpdater.autoInstallOnAppQuit = false
406410
installInFlight = false
407411
return
@@ -438,8 +442,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
438442
const confirmation = win ? showShellDialog(win, options) : showShellDialog(options)
439443
void confirmation
440444
.then(({ response }) => {
441-
if (response === 1 && state.status === 'ready' && state.version === version) {
442-
quitAndInstall(version)
445+
if (response === 1 && state.status === 'ready') {
446+
quitAndInstall()
443447
}
444448
})
445449
.catch((error) => {
@@ -459,8 +463,20 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
459463
let nextUpdaterCheckId = 0
460464
let updaterCheckTimeout: ReturnType<typeof setTimeout> | null = null
461465
let updaterRequestId: number | null = null
466+
/**
467+
* The validated version whose download is in flight. While `ready`, a
468+
* non-null value means a newer build is replacing the staged one.
469+
*/
462470
let acceptedUpdateVersion: string | null = null
463471

472+
/**
473+
* A staged (`ready`) or offered (`available`) update keeps being re-checked
474+
* in the background so a newer release replaces it. Without this, a shell
475+
* left running across several releases installs the stale build on
476+
* restart and immediately offers the next one.
477+
*/
478+
const isRefreshingOffer = () => state.status === 'ready' || state.status === 'available'
479+
464480
const finishProbe = (probeId: number) => {
465481
if (activeProbeId !== probeId) return
466482
activeProbeId = null
@@ -479,7 +495,7 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
479495
}
480496

481497
autoUpdater.on('checking-for-update', () => {
482-
if (activeUpdaterCheckId === null) return
498+
if (activeUpdaterCheckId === null || isRefreshingOffer()) return
483499
setState({ status: 'checking' })
484500
})
485501

@@ -488,6 +504,8 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
488504
if (checkId === null) return
489505
finishUpdaterCheck(checkId)
490506
if (updaterRequestId === checkId) updaterRequestId = null
507+
// A staged bundle is already armed in Squirrel and cannot be withdrawn.
508+
if (state.status === 'ready') return
491509
setState({ status: 'idle' })
492510
})
493511

@@ -501,6 +519,28 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
501519
!originFeedConfigured ||
502520
(info.files.length > 0 &&
503521
info.files.every((file) => isReleaseAssetUrl(file.url, info.version, channel)))
522+
if (state.status === 'ready' || state.status === 'available') {
523+
const offeredVersion = state.version ?? currentVersion
524+
if (
525+
!validOriginAssets ||
526+
!isValidUpdateCandidate(info.version, currentVersion) ||
527+
!isNewerVersion(info.version, offeredVersion)
528+
) {
529+
return
530+
}
531+
if (state.status === 'ready') {
532+
if (!autoDownloadEnabled) return
533+
acceptedUpdateVersion = info.version
534+
deps.events.record('update_check', { available: info.version, replacing: offeredVersion })
535+
void autoUpdater.downloadUpdate().catch((error) => {
536+
if (acceptedUpdateVersion === info.version) acceptedUpdateVersion = null
537+
logger.warn('Replacement update download failed; keeping the staged update', {
538+
message: getErrorMessage(error, 'unknown'),
539+
})
540+
})
541+
return
542+
}
543+
}
504544
if (!isValidUpdateCandidate(info.version, currentVersion) || !validOriginAssets) {
505545
acceptedUpdateVersion = null
506546
autoUpdater.autoInstallOnAppQuit = false
@@ -536,7 +576,11 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
536576
})
537577

538578
autoUpdater.on('update-downloaded', (info) => {
539-
if (state.status !== 'downloading') return
579+
if (state.status === 'ready') {
580+
if (acceptedUpdateVersion !== info.version) return
581+
} else if (state.status !== 'downloading') {
582+
return
583+
}
540584
if (
541585
acceptedUpdateVersion !== info.version ||
542586
!isValidUpdateCandidate(info.version, currentVersion)
@@ -558,6 +602,16 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
558602
if (checkId !== null) {
559603
finishUpdaterCheck(checkId)
560604
if (updaterRequestId === checkId) updaterRequestId = null
605+
if (isRefreshingOffer() && !installInFlight) {
606+
logger.warn('Background update re-check failed; keeping the current update', {
607+
message: getErrorMessage(error, 'unknown'),
608+
})
609+
return
610+
}
611+
} else if (state.status === 'ready' && acceptedUpdateVersion !== null && !installInFlight) {
612+
acceptedUpdateVersion = null
613+
deps.events.record('update_error', { message: getErrorMessage(error, 'unknown') })
614+
return
561615
} else if (state.status !== 'downloading' && state.status !== 'ready' && !installInFlight) {
562616
return
563617
}
@@ -660,12 +714,21 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
660714
if (
661715
activeProbeId !== null ||
662716
activeUpdaterCheckId !== null ||
663-
state.status === 'available' ||
664-
state.status === 'downloading' ||
665-
state.status === 'ready'
717+
state.status === 'downloading'
666718
) {
667719
return
668720
}
721+
if (isRefreshingOffer()) {
722+
const replacementInFlight = state.status === 'ready' && acceptedUpdateVersion !== null
723+
if (
724+
interactive ||
725+
installInFlight ||
726+
installConfirmationInFlight ||
727+
replacementInFlight
728+
) {
729+
return
730+
}
731+
}
669732
if (interactive) {
670733
setState({ status: 'checking' })
671734
}
@@ -726,28 +789,38 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
726789
let nextCheckId = 0
727790
let checkTimeout: ReturnType<typeof setTimeout> | null = null
728791

792+
/**
793+
* An offered download keeps being re-checked in the background, and only a
794+
* strictly newer release with a usable asset replaces it; failures and
795+
* equal versions leave the current offer untouched.
796+
*/
729797
const doCheck = async () => {
730-
if (activeCheckId !== null || state.status === 'available') return
798+
if (activeCheckId !== null) return
799+
const offeredVersion = state.status === 'available' ? state.version : undefined
800+
const refreshing = offeredVersion !== undefined
731801
const checkId = ++nextCheckId
732802
activeCheckId = checkId
733-
downloadUrl = null
734-
setState({ status: 'checking', manual: true })
803+
if (!refreshing) {
804+
downloadUrl = null
805+
setState({ status: 'checking', manual: true })
806+
}
735807
checkTimeout = setTimeout(() => {
736808
if (activeCheckId !== checkId) return
737809
activeCheckId = null
738810
checkTimeout = null
739811
deps.events.record('update_error', { message: 'Manual update check timed out' })
740-
setState({ status: 'error', manual: true })
812+
if (!refreshing) setState({ status: 'error', manual: true })
741813
}, UPDATE_CHECK_TIMEOUT_MS)
742814
try {
743815
const feedUrl = feedUrlForOrigin(deps.appOrigin())
744816
const manifest = feedUrl ? await fetchManifest(`${feedUrl}/latest-mac.yml`) : null
745817
if (activeCheckId !== checkId) return
746818
const version = manifest ? (/^version:\s*(\S+)\s*$/m.exec(manifest)?.[1] ?? null) : null
747819
if (!manifest || !version || !isValidUpdateCandidate(version, currentVersion)) {
748-
setState({ status: 'idle', manual: true })
820+
if (!refreshing) setState({ status: 'idle', manual: true })
749821
return
750822
}
823+
if (refreshing && !isNewerVersion(version, offeredVersion)) return
751824
// The feed rewrites manifest urls to absolute GitHub asset URLs;
752825
// prefer the dmg for a human download.
753826
//
@@ -759,12 +832,12 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
759832
manifest.matchAll(/^\s*(?:-\s*)?url:\s*(\S+)\s*$/gm),
760833
(m) => m[1]
761834
).filter((url) => isReleaseAssetUrl(url, version, resolveUpdateChannel(currentVersion)))
762-
downloadUrl =
835+
const nextDownloadUrl =
763836
urls.find((url) => url.endsWith('.dmg')) ??
764837
urls.find((url) => url.endsWith('.zip')) ??
765838
urls[0] ??
766839
null
767-
if (!downloadUrl) {
840+
if (!nextDownloadUrl) {
768841
// 'error', not 'idle': a newer version demonstrably exists and cannot
769842
// be offered, so "Sim is up to date" would strand a user whose shell
770843
// the server's minimum-version gate is already blocking.
@@ -773,15 +846,16 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle {
773846
candidates: urls.length,
774847
})
775848
deps.events.record('update_blocked_version', { version, reason: 'unusable-url' })
776-
setState({ status: 'error', version: state.version, manual: true })
849+
if (!refreshing) setState({ status: 'error', version: state.version, manual: true })
777850
return
778851
}
852+
downloadUrl = nextDownloadUrl
779853
deps.events.record('update_check', { available: version, manual: true })
780854
setState({ status: 'available', version, manual: true })
781855
} catch (error) {
782856
if (activeCheckId !== checkId) return
783857
logger.warn('Manual update check failed', { message: getErrorMessage(error, 'unknown') })
784-
setState({ status: 'error', version: state.version, manual: true })
858+
if (!refreshing) setState({ status: 'error', version: state.version, manual: true })
785859
} finally {
786860
if (activeCheckId === checkId) {
787861
activeCheckId = null

0 commit comments

Comments
 (0)