fix(updater): make install commitment a process-wide fact

Closes the three blockers from independent review of the renderer-only guard:

1. The dashboard popout has its own JS context and its own lazy chunks read
   from the same app.asar, but the updater event only ever reached the window
   that pressed Restart. Main now owns the state and broadcasts it to every
   window, and a window opened mid-install seeds itself over IPC.
2. Only main clears the state, so an ordinary recovery reload's abort event can
   no longer stand the archive back up mid-install.
3. Commitment is marked before the Linux package revalidation await, so an
   unrelated check error during that window cannot clear it while the install
   proceeds.

Also adds the two invariants review found untested: a chunk already in the
module map still resolves during an install (guard sits after the retry loop
on purpose), and the skipped path touches sessionStorage not at all.

The broadcast is fully defensive - a window that throws must never be able to
fail an update install. The existing updater suite caught that; regression test
added.
This commit is contained in:
Jinwoo-H
2026-08-08 10:50:06 -04:00
parent 6040760ad4
commit 87ce12028e
13 changed files with 382 additions and 32 deletions
@@ -0,0 +1,98 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const windows: { destroyed: boolean; sent: [string, boolean][] }[] = []
vi.mock('electron', () => ({
BrowserWindow: {
getAllWindows: () =>
windows.map((w) => ({
isDestroyed: () => w.destroyed,
webContents: {
send: (channel: string, committed: boolean) => {
if (w.destroyed) {
throw new Error('window destroyed')
}
w.sent.push([channel, committed])
}
}
}))
}
}))
const {
UPDATER_INSTALL_COMMITTED_CHANNEL,
clearUpdaterInstallCommitted,
isUpdaterInstallCommitted,
markUpdaterInstallCommitted,
resetUpdaterInstallCommitmentForTest
} = await import('./updater-install-commitment')
function addWindow(destroyed = false): { destroyed: boolean; sent: [string, boolean][] } {
const w = { destroyed, sent: [] as [string, boolean][] }
windows.push(w)
return w
}
describe('updater install commitment', () => {
beforeEach(() => {
windows.length = 0
resetUpdaterInstallCommitmentForTest()
})
it('reaches every live window, not only the one that pressed Restart', () => {
const main = addWindow()
const popout = addWindow()
markUpdaterInstallCommitted()
// The popout reads its lazy chunks from the same archive the installer swaps.
expect(main.sent).toEqual([[UPDATER_INSTALL_COMMITTED_CHANNEL, true]])
expect(popout.sent).toEqual([[UPDATER_INSTALL_COMMITTED_CHANNEL, true]])
expect(isUpdaterInstallCommitted()).toBe(true)
})
it('keeps installing when one window is already gone', () => {
addWindow(true)
const alive = addWindow()
expect(() => markUpdaterInstallCommitted()).not.toThrow()
expect(alive.sent).toEqual([[UPDATER_INSTALL_COMMITTED_CHANNEL, true]])
})
it('stands the archive back up only on a real abort', () => {
const win = addWindow()
markUpdaterInstallCommitted()
win.sent.length = 0
clearUpdaterInstallCommitted()
expect(isUpdaterInstallCommitted()).toBe(false)
expect(win.sent).toEqual([[UPDATER_INSTALL_COMMITTED_CHANNEL, false]])
})
it('never lets a hostile window break the install', () => {
// Regression: an earlier revision called isDestroyed() outside the try, so a
// window lacking it threw straight out of the install path and aborted the
// update. Notifying renderers is best effort; installing is not.
windows.push({
get destroyed(): boolean {
throw new Error('not a real window')
},
sent: []
} as never)
const alive = addWindow()
expect(() => markUpdaterInstallCommitted()).not.toThrow()
expect(alive.sent).toEqual([[UPDATER_INSTALL_COMMITTED_CHANNEL, true]])
expect(isUpdaterInstallCommitted()).toBe(true)
})
it('does not broadcast a clear that changes nothing', () => {
const win = addWindow()
clearUpdaterInstallCommitted()
// Otherwise an unrelated updater error would churn every renderer.
expect(win.sent).toEqual([])
})
})
+70
View File
@@ -0,0 +1,70 @@
import { BrowserWindow } from 'electron'
export const UPDATER_INSTALL_COMMITTED_CHANNEL = 'updater:installCommitted'
// Why: the installer replaces app.asar while renderers are still alive, so every
// renderer needs to know — not just the one that pressed Restart. Renderer-local
// state cannot cross into the dashboard popout, which has its own JS context and
// its own lazy chunks. Main owns the fact and broadcasts it.
let installCommitted = false
export function isUpdaterInstallCommitted(): boolean {
return installCommitted
}
function broadcast(committed: boolean): void {
let windows: BrowserWindow[] = []
try {
windows = BrowserWindow.getAllWindows()
} catch {
return
}
for (const win of windows) {
// Why: notifying renderers is best effort, but this runs inside the install
// path — a window that is mid-teardown, or otherwise not what we expect, must
// never be able to throw an update install into failure. Guard the whole
// interaction, not just the send.
try {
if (win.isDestroyed()) {
continue
}
win.webContents.send(UPDATER_INSTALL_COMMITTED_CHANNEL, committed)
} catch {
// Ignored deliberately; the renderer also seeds itself over IPC.
}
}
}
/**
* Marked before the Linux package revalidation await, so an unrelated updater
* check failing during that window cannot leave renderers believing no install
* is under way while one proceeds.
*/
export function markUpdaterInstallCommitted(): void {
installCommitted = true
broadcast(true)
}
export function clearUpdaterInstallCommitted(): void {
if (!installCommitted) {
return
}
installCommitted = false
broadcast(false)
}
/** Seeds a window created mid-install, which would otherwise never see the broadcast. */
export function syncUpdaterInstallCommitmentTo(win: BrowserWindow): void {
if (!installCommitted || win.isDestroyed()) {
return
}
try {
win.webContents.send(UPDATER_INSTALL_COMMITTED_CHANNEL, true)
} catch {
// Best effort: the renderer also seeds itself over IPC when it registers.
}
}
export function resetUpdaterInstallCommitmentForTest(): void {
installCommitted = false
}
+11
View File
@@ -1,6 +1,10 @@
/* eslint-disable max-lines */
import { app, BrowserWindow, powerMonitor } from 'electron'
import { is } from '@electron-toolkit/utils'
import {
clearUpdaterInstallCommitted,
markUpdaterInstallCommitted
} from './updater-install-commitment'
import type {
LinuxPackageInstallInstructions,
LinuxPackageInstallRecovery,
@@ -715,6 +719,11 @@ async function performQuitAndInstall(): Promise<void> {
if (deferHeadlessServeInstall('install', pendingVersion)) {
return
}
// Why: main owns this from here on. It precedes the package revalidation await
// below so an unrelated check error during that window cannot leave renderers
// believing no install is under way, and it reaches every renderer including
// the popout, whose chunks are read from the same archive.
markUpdaterInstallCommitted()
// Why: the retained .deb/.rpm sits on a user-writable path that a root package manager is about
// to read, and nothing re-checks it after download. Re-prove it here — before any teardown — so a
// swapped or vanished package aborts instead of being installed as root. The synchronous guard
@@ -723,6 +732,7 @@ async function performQuitAndInstall(): Promise<void> {
// Why: the renderer armed its restart before invoking, and it infers the abort from the error
// status — which a stale-cycle verdict deliberately withholds. Signal the abandon here, where
// it cannot depend on that decision, or the window keeps skipping its unsaved-work prompt.
clearUpdaterInstallCommitted()
mainWindowRef?.webContents.send('updater:quitAndInstallAborted')
return
}
@@ -874,6 +884,7 @@ async function performQuitAndInstall(): Promise<void> {
}
function resetQuitForUpdateState(): void {
clearUpdaterInstallCommitted()
quitAndInstallInProgress = false
quittingForUpdate = false
updateInstallCommitted = false
@@ -1,5 +1,6 @@
/* eslint-disable max-lines -- Why: this file is the central main-window IPC wiring point; splitting it during the mobile release compatibility rebase would increase release risk. */
import { randomUUID } from 'node:crypto'
import { isUpdaterInstallCommitted } from '../updater-install-commitment'
import { app, ipcMain } from 'electron'
import type { BrowserWindow, IpcMainInvokeEvent } from 'electron'
@@ -544,6 +545,9 @@ export function registerUpdaterHandlers(_store: Store): void {
ipcMain.removeHandler('updater:showLinuxPackage')
ipcMain.removeHandler('updater:listBuilds')
ipcMain.removeHandler('updater:isInstallCommitted')
// Why: a window created mid-install misses the broadcast, so it seeds itself here.
ipcMain.handle('updater:isInstallCommitted', () => isUpdaterInstallCommitted())
ipcMain.handle('updater:getStatus', () => getUpdateStatus())
ipcMain.handle('updater:getVersion', () => app.getVersion())
ipcMain.handle('updater:check', (_event, options?: UpdateCheckOptions) => {
+2
View File
@@ -2740,6 +2740,8 @@ export type PreloadApi = {
onChanged: (callback: (event: RemoteWorkspaceChangedEvent) => void) => () => void
}
updater: {
isInstallCommitted: () => Promise<boolean>
onInstallCommitted: (callback: (committed: boolean) => void) => () => void
getVersion: () => Promise<string>
getStatus: () => Promise<UpdateStatus>
check: (options?: UpdateCheckOptions) => Promise<void>
+6
View File
@@ -3037,6 +3037,12 @@ const api = {
awaitBeforeUnloadCheckpoint
),
isInstallCommitted: () => ipcRenderer.invoke('updater:isInstallCommitted'),
onInstallCommitted: (callback) => {
const listener = (_event: Electron.IpcRendererEvent, committed: boolean) => callback(committed)
ipcRenderer.on('updater:installCommitted', listener)
return () => ipcRenderer.removeListener('updater:installCommitted', listener)
},
onStatus: (callback) => {
const listener = (_event: Electron.IpcRendererEvent, status: UpdateStatus) => callback(status)
ipcRenderer.on('updater:status', listener)
+2
View File
@@ -117,6 +117,7 @@ import { useGlobalFileDrop } from './hooks/useGlobalFileDrop'
import { MacosTccPromptNoticeHost } from './hooks/MacosTccPromptNoticeHost'
import { useRadixBodyPointerEventsRecovery } from './hooks/useRadixBodyPointerEventsRecovery'
import { registerUpdaterBeforeUnloadBypass } from './lib/updater-beforeunload'
import { registerUpdaterInstallCommitment } from './lib/updater-install-commitment'
import {
ORCA_APP_RESTART_ABORTED_EVENT,
ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT
@@ -1257,6 +1258,7 @@ function App(): React.JSX.Element {
}, [])
useEffect(() => registerUpdaterBeforeUnloadBypass(), [])
useEffect(() => registerUpdaterInstallCommitment(), [])
useEffect(() => {
setRuntimeGraphSyncEnabled(workspaceSessionReady)
+1 -1
View File
@@ -4,7 +4,7 @@ import {
requestLazyChunkRecoveryReload,
type LazyChunkRecoveryReloadOutcome
} from './lazy-chunk-recovery-reload'
import { isUpdaterInstallCommitted } from './updater-beforeunload'
import { isUpdaterInstallCommitted } from './updater-install-commitment'
/**
* Resilient replacement for React.lazy.
@@ -21,7 +21,7 @@ import {
ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT,
ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT
} from '../../../shared/updater-renderer-events'
import { registerUpdaterBeforeUnloadBypass } from './updater-beforeunload'
import { registerUpdaterInstallCommitment } from './updater-install-commitment'
import {
isLazyChunkLoadError,
loadLazyWithRetry,
@@ -35,18 +35,26 @@ const CORRUPT_CHUNK_ERROR = (): SyntaxError => new SyntaxError("Unexpected token
type Breadcrumb = { name: string; data: Record<string, unknown> }
const broadcastListeners: ((committed: boolean) => void)[] = []
function installBreadcrumbSink(): Breadcrumb[] {
const breadcrumbs: Breadcrumb[] = []
;(window as unknown as { api: unknown }).api = {
crashReports: {
recordBreadcrumb: (crumb: Breadcrumb) => {
breadcrumbs.push(crumb)
}
const api = (window as unknown as { api: Record<string, unknown> }).api
api.crashReports = {
recordBreadcrumb: (crumb: Breadcrumb) => {
breadcrumbs.push(crumb)
}
}
return breadcrumbs
}
/** Main is the authority: this is what every renderer, popout included, receives. */
function broadcastInstallCommitted(committed: boolean): void {
for (const listener of broadcastListeners) {
listener(committed)
}
}
describe('loadLazyWithRetry during a committed update install', () => {
let unregister: (() => void) | null = null
@@ -54,7 +62,18 @@ describe('loadLazyWithRetry during a committed update install', () => {
window.sessionStorage.clear()
resetLazyChunkReloadRequestsForTest()
vi.spyOn(window.location, 'reload').mockImplementation(() => undefined)
unregister = registerUpdaterBeforeUnloadBypass()
broadcastListeners.length = 0
;(window as unknown as { api: { updater: unknown } }).api = {
...(window as unknown as { api?: Record<string, unknown> }).api,
updater: {
isInstallCommitted: () => Promise.resolve(false),
onInstallCommitted: (cb: (c: boolean) => void) => {
broadcastListeners.push(cb)
return () => undefined
}
}
} as never
unregister = registerUpdaterInstallCommitment()
})
afterEach(() => {
@@ -155,7 +174,8 @@ describe('loadLazyWithRetry during a committed update install', () => {
// rest of the session — the bundle was never swapped.
installBreadcrumbSink()
commitUpdateInstall()
window.dispatchEvent(new Event(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT))
// Only main can stand the archive down again.
broadcastInstallCommitted(false)
await loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
retries: 0,
@@ -165,6 +185,96 @@ describe('loadLazyWithRetry during a committed update install', () => {
expect(window.location.reload).toHaveBeenCalled()
})
it('skips recovery in a renderer that never dispatched the local event (popout)', async () => {
// The dashboard popout has its own JS context and never invokes quitAndInstall,
// so only main's broadcast can reach it. Its chunks come from the same archive.
installBreadcrumbSink()
broadcastInstallCommitted(true)
await loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
retries: 0,
reloadKey: 'agent-kanban-board'
}).catch(() => undefined)
expect(window.location.reload).not.toHaveBeenCalled()
})
it('seeds a window opened mid-install, which never saw the broadcast', async () => {
unregister?.()
;(window as unknown as { api: Record<string, unknown> }).api = {
...(window as unknown as { api: Record<string, unknown> }).api,
updater: {
isInstallCommitted: () => Promise.resolve(true),
onInstallCommitted: () => () => undefined
}
} as never
unregister = registerUpdaterInstallCommitment()
await Promise.resolve()
await Promise.resolve()
installBreadcrumbSink()
await loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
retries: 0,
reloadKey: 'agent-map'
}).catch(() => undefined)
expect(window.location.reload).not.toHaveBeenCalled()
})
it('keeps skipping when an unrelated updater error fires mid-install', async () => {
// Reviewer blocker 3: the preload relay treats any updater error status as an
// abort. Main owns the committed state, so an unrelated failed check during the
// Linux revalidation window must not stand the archive back up.
installBreadcrumbSink()
broadcastInstallCommitted(true)
window.dispatchEvent(new Event(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT))
await loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
retries: 0,
reloadKey: 'overlay.update-card'
}).catch(() => undefined)
expect(window.location.reload).not.toHaveBeenCalled()
})
it('still serves a chunk already cached in the module map during an install', async () => {
// The guard sits after the retry loop on purpose: an already-evaluated module
// resolves from Chromium's module map with no disk read, so a panel that would
// have worked must keep working. Placing the guard earlier breaks this.
installBreadcrumbSink()
broadcastInstallCommitted(true)
let calls = 0
const factory = (): Promise<{ default: () => null }> => {
calls += 1
return calls === 1
? Promise.reject(CORRUPT_CHUNK_ERROR())
: Promise.resolve({ default: () => null })
}
const loaded = await loadLazyWithRetry(factory, { retries: 1, baseDelayMs: 0 })
expect(calls).toBe(2)
expect(loaded.default).toBeTypeOf('function')
})
it('touches sessionStorage not at all on the skipped path', async () => {
installBreadcrumbSink()
broadcastInstallCommitted(true)
const getItem = vi.spyOn(window.sessionStorage, 'getItem')
const setItem = vi.spyOn(window.sessionStorage, 'setItem')
const removeItem = vi.spyOn(window.sessionStorage, 'removeItem')
await loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
retries: 0,
reloadKey: 'overlay.update-card'
}).catch(() => undefined)
// A write-then-remove leaves the key null too, so assert the path, not the key.
expect(setItem).not.toHaveBeenCalled()
expect(removeItem).not.toHaveBeenCalled()
expect(getItem).not.toHaveBeenCalled()
})
it('still requests recovery when no install is committed', async () => {
installBreadcrumbSink()
+4 -23
View File
@@ -6,20 +6,11 @@ import {
} from '../../../shared/updater-renderer-events'
let intentionalAppRestartInProgress = false
// Tracked separately from the restart flag above: an ordinary lazy-chunk recovery
// reload also announces itself as an app restart, but only an updater install
// replaces app.asar underneath the running renderer.
let updaterInstallCommitted = false
export function isUpdaterQuitAndInstallInProgress(): boolean {
return isIntentionalAppRestartInProgress()
}
/** True only while the updater is installing, so chunk reads may hit a swapped archive. */
export function isUpdaterInstallCommitted(): boolean {
return updaterInstallCommitted
}
export function isIntentionalAppRestartInProgress(): boolean {
return intentionalAppRestartInProgress
}
@@ -32,29 +23,19 @@ export function registerUpdaterBeforeUnloadBypass(): () => void {
intentionalAppRestartInProgress = false
}
const markInstallCommitted = (): void => {
updaterInstallCommitted = true
markInProgress()
}
const clearInstallCommitted = (): void => {
updaterInstallCommitted = false
clearInProgress()
}
window.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markInstallCommitted)
window.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, clearInstallCommitted)
window.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markInProgress)
window.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, clearInProgress)
window.addEventListener(ORCA_APP_RESTART_STARTED_EVENT, markInProgress)
window.addEventListener(ORCA_APP_RESTART_ABORTED_EVENT, clearInProgress)
return () => {
window.removeEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markInstallCommitted)
window.removeEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, clearInstallCommitted)
window.removeEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markInProgress)
window.removeEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, clearInProgress)
window.removeEventListener(ORCA_APP_RESTART_STARTED_EVENT, markInProgress)
window.removeEventListener(ORCA_APP_RESTART_ABORTED_EVENT, clearInProgress)
// Why: hot reloads can re-register this listener inside the same renderer.
// Reset the module flag on cleanup so a failed earlier restart attempt
// cannot silently suppress future unsaved-change prompts.
intentionalAppRestartInProgress = false
updaterInstallCommitted = false
}
}
@@ -0,0 +1,58 @@
import { ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT } from '../../../shared/updater-renderer-events'
// Why: the installer replaces app.asar underneath every live renderer, so this has
// to be true in the dashboard popout too — it has its own JS context and its own
// lazy chunks read from the same archive. Main broadcasts the authoritative value;
// the local event only arms the initiating window a few milliseconds earlier.
let broadcastCommitted = false
let locallyStarted = false
export function isUpdaterInstallCommitted(): boolean {
return broadcastCommitted || locallyStarted
}
/**
* Registered by every renderer entry point, not just the main app root.
*
* The local start event is deliberately one-way: only main clears the committed
* state, so an unrelated updater check failing mid-install cannot make a renderer
* believe the archive underneath it is stable again.
*/
export function registerUpdaterInstallCommitment(): () => void {
const markLocal = (): void => {
locallyStarted = true
}
window.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markLocal)
const applyBroadcast = (committed: boolean): void => {
broadcastCommitted = committed
if (!committed) {
locallyStarted = false
}
}
const unsubscribe = window.api?.updater?.onInstallCommitted?.(applyBroadcast)
// A window opened mid-install never saw the broadcast, so seed it.
void window.api?.updater
?.isInstallCommitted?.()
.then((committed) => {
if (committed) {
broadcastCommitted = true
}
})
.catch(() => {
// Best effort: the broadcast still arrives if an install starts later.
})
return () => {
window.removeEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markLocal)
unsubscribe?.()
broadcastCommitted = false
locallyStarted = false
}
}
export function resetUpdaterInstallCommitmentForTest(): void {
broadcastCommitted = false
locallyStarted = false
}
+4
View File
@@ -9,6 +9,7 @@ import {
recordRendererCrashBreadcrumb
} from './lib/crash-diagnostics'
import { applyDocumentTheme } from './lib/document-theme'
import { registerUpdaterInstallCommitment } from './lib/updater-install-commitment'
import { buildAppFontFamily } from './lib/app-font-family'
import { I18nProvider } from './i18n/I18nProvider'
import { translate } from './i18n/i18n'
@@ -99,6 +100,9 @@ function PopoutSettingsSync(): null {
function PopoutRoot(): React.JSX.Element {
useTranslation()
// Why: this window has its own JS context and its own lazy chunks read from the
// same app.asar, so it needs the install signal independently of the app root.
useEffect(() => registerUpdaterInstallCommitment(), [])
return (
<RecoverableRenderErrorBoundary
boundaryId="dashboard-popout.root"
+4
View File
@@ -3104,6 +3104,10 @@ function createUpdaterApi(): NonNullable<Partial<PreloadApi>['updater']> {
return {
getVersion: () => Promise.resolve('web'),
getStatus: () => Promise.resolve({ state: 'idle' } as never),
// Why: the web build serves chunks over HTTP from a running server, so no
// installer ever swaps the archive underneath it.
isInstallCommitted: () => Promise.resolve(false),
onInstallCommitted: () => () => undefined,
check: () => Promise.resolve(),
download: () => Promise.resolve(),
quitAndInstall: () => Promise.resolve(),