Harden update replacement state

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-05-28 22:09:21 -07:00
co-authored by Orca
parent a00a94eaa2
commit cd33c6d0d1
7 changed files with 389 additions and 33 deletions
+54 -15
View File
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import { app, autoUpdater as nativeUpdater } from 'electron'
import type { UpdateStatus } from '../shared/types'
import {
@@ -44,6 +45,7 @@ type UpdaterHandlerContext = {
sendErrorStatus: (message: string, userInitiated?: boolean) => void
sendStatus: (status: UpdateStatus) => void
scheduleAutomaticUpdateCheck: (delayMs: number) => void
startPendingReplacementDownload: () => boolean
startAvailableUpdateDownload: () => void
shouldAcceptDownloadedUpdate: (version: string) => boolean
shouldSuppressMissingManifestPrereleaseFallbackEvent: (message: string, error: unknown) => boolean
@@ -65,6 +67,24 @@ function getActionableUpdateUserInitiated(status: UpdateStatus): boolean | undef
return undefined
}
function shouldShowDownloadedForStagedUpdate(): boolean {
return process.platform !== 'darwin' || isMacInstallerReady()
}
function shouldApplyAvailableResultAfterChangelog(status: UpdateStatus, version: string): boolean {
if (status.state === 'checking' || status.state === 'idle') {
return true
}
if (
status.state === 'available' ||
status.state === 'downloading' ||
status.state === 'downloaded'
) {
return compareVersions(status.version, version) < 0
}
return false
}
export function registerAutoUpdaterHandlers({
autoUpdater,
clearBackgroundCheckLaunchPending,
@@ -89,6 +109,7 @@ export function registerAutoUpdaterHandlers({
sendErrorStatus,
sendStatus,
scheduleAutomaticUpdateCheck,
startPendingReplacementDownload,
startAvailableUpdateDownload,
shouldAcceptDownloadedUpdate,
shouldSuppressMissingManifestPrereleaseFallbackEvent,
@@ -99,6 +120,26 @@ export function registerAutoUpdaterHandlers({
}: UpdaterHandlerContext): void {
let macNativeReadinessPendingVersion: string | null = null
const sendStagedUpdateStatus = (version: string, userInitiated: boolean): void => {
if (!shouldShowDownloadedForStagedUpdate()) {
// Why: on macOS the electron-updater artifact can be staged before ShipIt
// has finished pulling it from the localhost proxy.
sendStatus({
state: 'downloading',
percent: 100,
version,
...(userInitiated ? { userInitiated: true } : {})
})
return
}
sendStatus({
state: 'downloaded',
version,
releaseUrl: getKnownReleaseUrl(),
...(userInitiated ? { userInitiated: true } : {})
})
}
// On macOS, electron-updater's MacUpdater downloads the ZIP from GitHub,
// then serves it to Squirrel.Mac via a localhost proxy. The electron-updater
// 'update-downloaded' event fires BEFORE Squirrel finishes its download.
@@ -197,12 +238,7 @@ export function registerAutoUpdaterHandlers({
scheduleAutomaticUpdateCheck(AUTO_UPDATE_CHECK_INTERVAL_MS)
}
}
sendStatus({
state: 'downloaded',
version: stagedVersion,
releaseUrl: getKnownReleaseUrl(),
userInitiated: wasUserInitiated || undefined
})
sendStagedUpdateStatus(stagedVersion, wasUserInitiated)
return
}
@@ -217,7 +253,7 @@ export function registerAutoUpdaterHandlers({
// currentStatus during that window, broadcasting 'available' here would
// overwrite a more recent status. Guard against this by checking that the
// state hasn't advanced past the point where 'available' makes sense.
if (getCurrentStatus().state !== 'checking' && getCurrentStatus().state !== 'idle') {
if (!shouldApplyAvailableResultAfterChangelog(getCurrentStatus(), info.version)) {
return
}
@@ -279,12 +315,7 @@ export function registerAutoUpdaterHandlers({
if (stagedVersion && compareVersions(stagedVersion, app.getVersion()) > 0) {
// Why: "not available" means no newer feed result, not that a previously
// staged update disappeared. Keep the restart action available.
sendStatus({
state: 'downloaded',
version: stagedVersion,
releaseUrl: getKnownReleaseUrl(),
userInitiated: wasUserInitiated || undefined
})
sendStagedUpdateStatus(stagedVersion, wasUserInitiated)
return
}
sendStatus({ state: 'not-available', userInitiated: wasUserInitiated || undefined })
@@ -304,6 +335,8 @@ export function registerAutoUpdaterHandlers({
autoUpdater.on('update-downloaded', (info) => {
clearBackgroundCheckLaunchPending()
if (!shouldAcceptDownloadedUpdate(info.version)) {
clearActiveUpdateDownload(info.version)
startPendingReplacementDownload()
return
}
// Don't show the banner if the downloaded version isn't actually newer
@@ -349,8 +382,11 @@ export function registerAutoUpdaterHandlers({
return
}
const statusAtError = getCurrentStatus()
const activeDownloadVersionAtError = getActiveDownloadVersion()
clearBackgroundCheckLaunchPending()
if (statusAtError.state !== 'checking') {
if (activeDownloadVersionAtError) {
clearActiveUpdateDownload(activeDownloadVersionAtError)
} else if (statusAtError.state !== 'checking') {
clearActiveUpdateDownload()
}
resetMacInstallState()
@@ -360,10 +396,13 @@ export function registerAutoUpdaterHandlers({
(missingManifestFallback?.userInitiated ?? getUserInitiatedCheck()) ||
getActionableUpdateUserInitiated(getCurrentStatus())
setUserInitiatedCheck(false)
if (statusAtError.state === 'checking') {
if (statusAtError.state === 'checking' && !activeDownloadVersionAtError) {
void sendCheckFailureStatus(message, wasUserInitiated || undefined, 'event', err)
return
}
if (startPendingReplacementDownload()) {
return
}
sendErrorStatus(message, wasUserInitiated || undefined)
})
}
+58
View File
@@ -208,6 +208,64 @@ describe('updater mac install handoff', () => {
}
)
it('keeps staged mac updates pending through same-version fast paths until native readiness', async () => {
vi.stubGlobal('process', { ...process, platform: 'darwin' })
const sendMock = vi.fn()
const mainWindow = { webContents: { send: sendMock } }
const { setupAutoUpdater } = await import('./updater')
setupAutoUpdater(mainWindow as never, { getLastUpdateCheckAt: () => Date.now() })
const nativeDownloadedHandler = nativeUpdaterMock.on.mock.calls.find(
([eventName]) => eventName === 'update-downloaded'
)?.[1] as (() => void) | undefined
expect(nativeDownloadedHandler).toBeTypeOf('function')
autoUpdaterMock.emit('checking-for-update')
autoUpdaterMock.emit('update-available', { version: '1.0.61' })
await new Promise((resolve) => setTimeout(resolve, 0))
autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' })
sendMock.mockClear()
autoUpdaterMock.emit('checking-for-update')
sendMock.mockClear()
autoUpdaterMock.emit('update-not-available')
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloading',
percent: 100,
version: '1.0.61'
})
expect(sendMock).not.toHaveBeenCalledWith(
'updater:status',
expect.objectContaining({ state: 'downloaded' })
)
sendMock.mockClear()
autoUpdaterMock.emit('checking-for-update')
sendMock.mockClear()
autoUpdaterMock.emit('update-available', { version: '1.0.61' })
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloading',
percent: 100,
version: '1.0.61'
})
expect(sendMock).not.toHaveBeenCalledWith(
'updater:status',
expect.objectContaining({ state: 'downloaded' })
)
nativeDownloadedHandler?.()
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloaded',
version: '1.0.61',
releaseUrl: undefined
})
})
it('does not let stale native macOS readiness mark a replacement update ready', async () => {
vi.stubGlobal('process', { ...process, platform: 'darwin' })
autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
+182
View File
@@ -975,6 +975,46 @@ describe('updater', () => {
})
})
it('keeps active download errors retryable when a manual check is visible', async () => {
autoUpdaterMock.checkForUpdates.mockResolvedValue(undefined)
autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
const sendMock = vi.fn()
const persistLastUpdateCheckAt = vi.fn()
const mainWindow = { webContents: { send: sendMock } }
const { setupAutoUpdater, checkForUpdatesFromMenu, downloadUpdate } = await import('./updater')
setupAutoUpdater(mainWindow as never, {
getLastUpdateCheckAt: () => Date.now(),
setLastUpdateCheckAt: persistLastUpdateCheckAt
})
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 })
sendMock.mockClear()
persistLastUpdateCheckAt.mockClear()
checkForUpdatesFromMenu()
autoUpdaterMock.emit('error', new Error('disk full'))
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'error',
message: 'disk full',
userInitiated: true
})
expect(persistLastUpdateCheckAt).not.toHaveBeenCalled()
downloadUpdate()
expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(2)
})
it('surfaces automatic download failures against the cached update version', async () => {
autoUpdaterMock.checkForUpdates.mockImplementation(() => {
autoUpdaterMock.emit('checking-for-update')
@@ -1139,6 +1179,148 @@ describe('updater', () => {
})
})
it('queues a newer replacement while an older download is still active', async () => {
vi.stubGlobal('process', { ...process, platform: 'linux' })
autoUpdaterMock.downloadUpdate.mockImplementation(() => new Promise(() => {}))
const { fetchChangelog } = await import('./updater-changelog')
vi.mocked(fetchChangelog).mockResolvedValue(null)
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 })
sendMock.mockClear()
let resolveNewerChangelog: (
value: Awaited<ReturnType<typeof fetchChangelog>>
) => void = () => {}
vi.mocked(fetchChangelog).mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveNewerChangelog = resolve
})
)
autoUpdaterMock.emit('checking-for-update')
autoUpdaterMock.emit('update-available', { version: '1.0.62' })
await vi.waitFor(() => {
expect(fetchChangelog).toHaveBeenCalledWith('1.0.62', '1.0.51')
})
autoUpdaterMock.emit('download-progress', { percent: 42 })
resolveNewerChangelog(null)
await vi.waitFor(() => {
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'available',
version: '1.0.62',
changelog: null
})
})
expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1)
autoUpdaterMock.emit('download-progress', { percent: 67 })
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloading',
percent: 67,
version: '1.0.61'
})
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' })
)
autoUpdaterMock.emit('download-progress', { percent: 80 })
autoUpdaterMock.emit('update-downloaded', { version: '1.0.62' })
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloading',
percent: 80,
version: '1.0.62'
})
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloaded',
version: '1.0.62',
releaseUrl: undefined
})
})
it('starts a queued replacement when the older active download fails', async () => {
vi.stubGlobal('process', { ...process, platform: 'linux' })
let rejectFirstDownload: (reason: Error) => void = () => {}
autoUpdaterMock.downloadUpdate
.mockImplementationOnce(
() =>
new Promise<string[]>((_resolve, reject) => {
rejectFirstDownload = reject
})
)
.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()
rejectFirstDownload(new Error('disk full'))
await vi.waitFor(() => {
expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(2)
})
expect(sendMock).not.toHaveBeenCalledWith(
'updater:status',
expect.objectContaining({ state: 'error', message: 'disk full' })
)
autoUpdaterMock.emit('download-progress', { percent: 55 })
expect(sendMock).toHaveBeenCalledWith('updater:status', {
state: 'downloading',
percent: 55,
version: '1.0.62'
})
})
it('does not leak a nudge marker into a later ordinary update cycle', async () => {
const sendMock = vi.fn()
const mainWindow = { webContents: { send: sendMock } }
+44 -1
View File
@@ -88,6 +88,7 @@ let _setDismissedUpdateNudgeId: ((id: string | null) => void) | null = null
// background download is still active, so the duplicate-download guard must be
// version-scoped instead of tied to the visible status.
let activeDownloadVersion: string | null = null
let pendingReplacementDownloadVersion: string | null = null
/** Guards against the macOS `activate` handler re-opening the old version
* while Squirrel's ShipIt is replacing the .app bundle. */
let quittingForUpdate = false
@@ -103,6 +104,7 @@ function getAutoUpdater(): ElectronAutoUpdater {
function clearAvailableUpdateContext(): void {
availableVersion = null
availableReleaseUrl = null
pendingReplacementDownloadVersion = null
}
function clearStagedUpdateContext(): void {
@@ -129,6 +131,20 @@ function clearActiveUpdateDownload(version?: string): void {
}
}
function startPendingReplacementDownload(): boolean {
const pendingVersion = pendingReplacementDownloadVersion
if (!pendingVersion) {
return false
}
const targetVersion = getPendingDownloadVersion()
if (!targetVersion || compareVersions(targetVersion, pendingVersion) < 0) {
pendingReplacementDownloadVersion = null
return false
}
startAvailableUpdateDownload()
return activeDownloadVersion === targetVersion
}
function clearPrereleaseFallbackContext(): void {
pendingPrereleaseFallback = null
}
@@ -261,6 +277,12 @@ function hasNewerDownloadedVersion(): boolean {
if (activeDownloadVersion && compareVersions(activeDownloadVersion, stagedUpdateVersion) > 0) {
return false
}
if (
pendingReplacementDownloadVersion &&
compareVersions(pendingReplacementDownloadVersion, stagedUpdateVersion) > 0
) {
return false
}
return true
}
@@ -295,6 +317,12 @@ function shouldAcceptDownloadedUpdate(version: string): boolean {
if (activeDownloadVersion && compareVersions(version, activeDownloadVersion) < 0) {
return false
}
if (
pendingReplacementDownloadVersion &&
compareVersions(version, pendingReplacementDownloadVersion) < 0
) {
return false
}
if (availableVersion && compareVersions(version, availableVersion) < 0) {
return false
}
@@ -983,6 +1011,7 @@ export function setupAutoUpdater(
recordCompletedUpdateCheck,
sendStatus,
scheduleAutomaticUpdateCheck,
startPendingReplacementDownload,
startAvailableUpdateDownload,
shouldAcceptDownloadedUpdate,
clearBackgroundCheckLaunchPending,
@@ -1040,16 +1069,24 @@ function startAvailableUpdateDownload(): void {
if (activeDownloadVersion === targetVersion) {
return
}
if (activeDownloadVersion) {
if (compareVersions(targetVersion, activeDownloadVersion) > 0) {
pendingReplacementDownloadVersion = targetVersion
}
return
}
// Why: permit retry from 'error' when we still have a cached availableVersion —
// a failed download leaves the status at 'error' but availableVersion intact,
// and the error card's "Retry Download" button must be able to restart the
// download. Without this, the button would appear to do nothing.
const canStart =
currentStatus.state === 'available' ||
(currentStatus.state === 'error' && hasNewerAvailableVersion())
(currentStatus.state === 'error' && hasNewerAvailableVersion()) ||
pendingReplacementDownloadVersion === targetVersion
if (!canStart) {
return
}
pendingReplacementDownloadVersion = null
activeDownloadVersion = targetVersion
// Why: electron-updater can emit 'error' before downloadUpdate() rejects;
// capture launch intent, then merge any manual promotion observed by catch.
@@ -1073,6 +1110,12 @@ function startAvailableUpdateDownload(): void {
backgroundCheckPromotedToUserInitiated ||
undefined
clearActiveUpdateDownload(targetVersion)
if (startPendingReplacementDownload()) {
return
}
if (activeDownloadVersion && compareVersions(activeDownloadVersion, targetVersion) > 0) {
return
}
sendErrorStatus(message, wasUserInitiated)
})
}
+29 -6
View File
@@ -260,6 +260,15 @@ describe('updateDownloadIntentVersion', () => {
expect(store.getState().updateDownloadIntentVersion).toBeNull()
})
it('clears explicit download intent when a different available version becomes current', () => {
const store = createTestStore()
store.getState().markUpdateDownloadIntent('1.2.0')
setState(store, { state: 'available', version: '1.3.0', changelog: null })
expect(store.getState().updateDownloadIntentVersion).toBeNull()
})
})
// ── markUpdateReassuranceSeen ────────────────────────────────────────
@@ -326,6 +335,7 @@ type VisibilityInput = {
hasStartedDownload: boolean
userInitiatedCycle?: boolean
downloadIntentVersion?: string | null
locallyDismissedVersion?: string | null
}
type VisibilityResult = 'hidden' | 'visible'
@@ -338,7 +348,8 @@ function computeVisibility(input: VisibilityInput): VisibilityResult {
cachedVersion,
hasStartedDownload,
userInitiatedCycle = false,
downloadIntentVersion = null
downloadIntentVersion = null,
locallyDismissedVersion = null
} = input
const isUserInitiated = 'userInitiated' in status && status.userInitiated
const isNudgeDriven = 'activeNudgeId' in status && Boolean(status.activeNudgeId)
@@ -375,13 +386,13 @@ function computeVisibility(input: VisibilityInput): VisibilityResult {
}
const effectiveVersion = 'version' in status ? status.version : cachedVersion
const isLocallyDismissedVersion =
effectiveVersion !== null && locallyDismissedVersion === effectiveVersion
if (
effectiveVersion &&
dismissedVersion === effectiveVersion &&
!userInitiatedCycle &&
!isUserInitiated &&
!hasExplicitDownloadIntent &&
!isNudgeDriven
(dismissedVersion === effectiveVersion || isLocallyDismissedVersion) &&
(isLocallyDismissedVersion ||
(!userInitiatedCycle && !isUserInitiated && !hasExplicitDownloadIntent && !isNudgeDriven))
) {
if (
status.state !== 'downloading' &&
@@ -536,6 +547,18 @@ describe('UpdateCard visibility gates', () => {
).toBe('visible')
})
it('hides hydrated user-initiated available after the visible card is explicitly dismissed', () => {
expect(
computeVisibility({
status: { state: 'available', version: '1.2.0', changelog: null, userInitiated: true },
dismissedVersion: '1.2.0',
cachedVersion: '1.2.0',
hasStartedDownload: false,
locallyDismissedVersion: '1.2.0'
})
).toBe('hidden')
})
it('shows downloading even when version is dismissed (user clicked Update after dismiss)', () => {
expect(
computeVisibility({
+15 -5
View File
@@ -120,6 +120,9 @@ export function UpdateCard() {
// dismissed. This ref tracks whether the current check cycle was user-initiated
// so the dismiss gate can let the result through.
const userInitiatedCycleRef = useRef(false)
// Why: status.userInitiated can outlive an explicit close click. Track that
// local close so dismissing a visible card wins until a new check/version.
const locallyDismissedVersionRef = useRef<string | null>(null)
const changelog: ChangelogData | null = storeChangelog
@@ -149,6 +152,7 @@ export function UpdateCard() {
if (status.state === 'available' && status.version !== prevVersionRef.current) {
prevVersionRef.current = status.version
hasStartedDownload.current = false
locallyDismissedVersionRef.current = null
setMediaFailed(false)
setMediaLoaded(false)
setInstallError(null)
@@ -225,8 +229,10 @@ export function UpdateCard() {
// even though the user explicitly asked to see the result.
if (status.state === 'checking' && isUserInitiated) {
userInitiatedCycleRef.current = true
locallyDismissedVersionRef.current = null
} else if (status.state === 'idle' || (status.state === 'checking' && !isUserInitiated)) {
userInitiatedCycleRef.current = false
locallyDismissedVersionRef.current = null
}
// Compact transient states: only show for user-initiated checks.
@@ -285,13 +291,16 @@ 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 &&
!userInitiatedCycleRef.current &&
!isUserInitiated &&
!hasExplicitDownloadIntent &&
!isNudgeDriven
(dismissedVersion === versionRef.current || isLocallyDismissedVersion) &&
(isLocallyDismissedVersion ||
(!userInitiatedCycleRef.current &&
!isUserInitiated &&
!hasExplicitDownloadIntent &&
!isNudgeDriven))
) {
if (
status.state !== 'downloading' &&
@@ -333,6 +342,7 @@ export function UpdateCard() {
// immediately — otherwise the card would reappear on the next render
// because the bypass ref still overrides the persisted dismissal.
userInitiatedCycleRef.current = false
locallyDismissedVersionRef.current = cachedVersion
if (status.state === 'error') {
setErrorDismissed(true)
if (cachedVersion) {
+7 -6
View File
@@ -1337,7 +1337,9 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
updateStatus: { state: 'idle' },
setUpdateStatus: (status) => {
const prevState = get().updateStatus.state
const state = get()
const prevState = state.updateStatus.state
const intentVersion = state.updateDownloadIntentVersion
const update: Partial<
Pick<
UISlice,
@@ -1361,11 +1363,10 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (set, get)
// previous update cycle cannot resurface.
update.updateChangelog = null
update.updateDownloadIntentVersion = null
} else if (
'version' in status &&
get().updateDownloadIntentVersion !== null &&
get().updateDownloadIntentVersion !== status.version
) {
}
// Why: explicit download intent is version-scoped; keeping it after a new
// version appears makes passive failures look user-initiated.
if ('version' in status && intentVersion !== null && intentVersion !== status.version) {
update.updateDownloadIntentVersion = null
}
// For 'downloading', 'downloaded', 'error': leave updateChangelog untouched