mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(updater): capture install commitment synchronously at document load
Third-pass review found the popout guard was too narrow: View -> Reload, force reload, the app:reload IPC, renderer-crash recovery, and dock/tray activation can all create a fresh renderer document during the Linux revalidation window. Each one misses the broadcast, and its async seed can never be answered because the package install blocks main in spawnSync. Guarding those five call sites would be whack-a-mole. Instead preload reads the state synchronously before any document script runs, so every document - however it was created - knows before its first lazy import. The renderer reads it lazily, so it is available before React mounts rather than in an effect. Also fixes a defect mutation testing exposed in my own logic: a stale false broadcast from an earlier aborted install masked the local arming, so the initiating window lost its early coverage on the next restart. Adds a cross-layer test that kills both wiring survivors review 3 listed: dropping the install-path mark, and dropping the clear on recovery.
This commit is contained in:
@@ -1742,6 +1742,32 @@ describe('updater', () => {
|
||||
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledWith(false, true)
|
||||
})
|
||||
|
||||
it('commits install state before the native installer, and clears it on recovery', async () => {
|
||||
// Cross-layer wiring: renderers read this to stop loading lazy chunks out of an
|
||||
// archive the installer is replacing. If this call is ever dropped, every
|
||||
// renderer silently loses that protection while the tests stay green.
|
||||
vi.useFakeTimers()
|
||||
const commitment = await import('./updater-install-commitment')
|
||||
commitment.resetUpdaterInstallCommitmentForTest()
|
||||
|
||||
const mainWindow = { webContents: { send: vi.fn() } }
|
||||
const { setupAutoUpdater, quitAndInstall } = await import('./updater')
|
||||
setupAutoUpdater(mainWindow as never)
|
||||
|
||||
let committedWhenInstallerRan: boolean | null = null
|
||||
autoUpdaterMock.quitAndInstall.mockImplementation(() => {
|
||||
committedWhenInstallerRan = commitment.isUpdaterInstallCommitted()
|
||||
autoUpdaterMock.emit('error', new Error('installer refused'))
|
||||
})
|
||||
|
||||
quitAndInstall()
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
|
||||
expect(committedWhenInstallerRan).toBe(true)
|
||||
// A failed install leaves the archive intact, so renderers must recover normally.
|
||||
expect(commitment.isUpdaterInstallCommitted()).toBe(false)
|
||||
})
|
||||
|
||||
it('runs pre-quit cleanup before local PTY cleanup during update install', async () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
/* 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 { UPDATER_IS_INSTALL_COMMITTED_CHANNEL } from '../../shared/updater-install-events'
|
||||
import {
|
||||
UPDATER_IS_INSTALL_COMMITTED_CHANNEL,
|
||||
UPDATER_IS_INSTALL_COMMITTED_SYNC_CHANNEL
|
||||
} from '../../shared/updater-install-events'
|
||||
|
||||
import { app, ipcMain } from 'electron'
|
||||
import type { BrowserWindow, IpcMainInvokeEvent } from 'electron'
|
||||
@@ -549,6 +552,12 @@ export function registerUpdaterHandlers(_store: Store): void {
|
||||
ipcMain.removeHandler(UPDATER_IS_INSTALL_COMMITTED_CHANNEL)
|
||||
// Why: a window created mid-install misses the broadcast, so it seeds itself here.
|
||||
ipcMain.handle(UPDATER_IS_INSTALL_COMMITTED_CHANNEL, () => isUpdaterInstallCommitted())
|
||||
ipcMain.removeAllListeners(UPDATER_IS_INSTALL_COMMITTED_SYNC_CHANNEL)
|
||||
// Why: preload reads this before any document script, so a window created or
|
||||
// reloaded mid-install knows before its first lazy import.
|
||||
ipcMain.on(UPDATER_IS_INSTALL_COMMITTED_SYNC_CHANNEL, (event) => {
|
||||
event.returnValue = isUpdaterInstallCommitted()
|
||||
})
|
||||
ipcMain.handle('updater:getStatus', () => getUpdateStatus())
|
||||
ipcMain.handle('updater:getVersion', () => app.getVersion())
|
||||
ipcMain.handle('updater:check', (_event, options?: UpdateCheckOptions) => {
|
||||
|
||||
@@ -2740,6 +2740,8 @@ export type PreloadApi = {
|
||||
onChanged: (callback: (event: RemoteWorkspaceChangedEvent) => void) => () => void
|
||||
}
|
||||
updater: {
|
||||
/** Captured synchronously in preload, before any document script runs. */
|
||||
installCommittedAtLoad: boolean
|
||||
isInstallCommitted: () => Promise<boolean>
|
||||
onInstallCommitted: (callback: (committed: boolean) => void) => () => void
|
||||
getVersion: () => Promise<string>
|
||||
|
||||
+16
-1
@@ -2,8 +2,19 @@
|
||||
import { contextBridge, ipcRenderer, webFrame, webUtils } from 'electron'
|
||||
import {
|
||||
UPDATER_INSTALL_COMMITTED_CHANNEL,
|
||||
UPDATER_IS_INSTALL_COMMITTED_CHANNEL
|
||||
UPDATER_IS_INSTALL_COMMITTED_CHANNEL,
|
||||
UPDATER_IS_INSTALL_COMMITTED_SYNC_CHANNEL
|
||||
} from '../shared/updater-install-events'
|
||||
|
||||
function readInstallCommittedAtLoad(): boolean {
|
||||
try {
|
||||
return ipcRenderer.sendSync(UPDATER_IS_INSTALL_COMMITTED_SYNC_CHANNEL) === true
|
||||
} catch {
|
||||
// An unanswered probe must never stop a window loading; the broadcast and the
|
||||
// async seed still cover every case except a document born mid-install.
|
||||
return false
|
||||
}
|
||||
}
|
||||
import { electronAPI } from '@electron-toolkit/preload'
|
||||
import { preloadE2EConfig } from './e2e-config'
|
||||
import { glApi } from './gitlab'
|
||||
@@ -3041,6 +3052,10 @@ const api = {
|
||||
awaitBeforeUnloadCheckpoint
|
||||
),
|
||||
|
||||
// Why: captured before any document script runs, so a window created or
|
||||
// reloaded during an install knows before its first lazy import. The async
|
||||
// seed below cannot be relied on — the Linux package install blocks main.
|
||||
installCommittedAtLoad: readInstallCommittedAtLoad(),
|
||||
isInstallCommitted: () => ipcRenderer.invoke(UPDATER_IS_INSTALL_COMMITTED_CHANNEL),
|
||||
onInstallCommitted: (callback) => {
|
||||
const listener = (_event: Electron.IpcRendererEvent, committed: boolean) => callback(committed)
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
loadLazyWithRetry,
|
||||
resetLazyChunkReloadRequestsForTest
|
||||
} from './lazy-with-retry'
|
||||
import { resetUpdaterInstallCommitmentForTest } from './updater-install-commitment'
|
||||
|
||||
const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
|
||||
|
||||
@@ -79,6 +80,7 @@ describe('loadLazyWithRetry during a committed update install', () => {
|
||||
afterEach(() => {
|
||||
unregister?.()
|
||||
unregister = null
|
||||
resetUpdaterInstallCommitmentForTest()
|
||||
vi.restoreAllMocks()
|
||||
window.sessionStorage.clear()
|
||||
resetLazyChunkReloadRequestsForTest()
|
||||
@@ -275,6 +277,33 @@ describe('loadLazyWithRetry during a committed update install', () => {
|
||||
expect(getItem).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('skips recovery in a document born mid-install, before any registration', async () => {
|
||||
// Every reload/reopen path lands here: View -> Reload, force reload, the
|
||||
// app:reload IPC, renderer-crash recovery, dock activation, a new popout. The
|
||||
// fresh document never saw a broadcast, and its async seed can never be
|
||||
// answered while the Linux package install blocks main inside spawnSync. The
|
||||
// synchronous preload capture is the only thing that can be true this early —
|
||||
// note this test deliberately does NOT call registerUpdaterInstallCommitment.
|
||||
unregister?.()
|
||||
unregister = null
|
||||
resetUpdaterInstallCommitmentForTest()
|
||||
;(window as unknown as { api: Record<string, unknown> }).api = {
|
||||
crashReports: { recordBreadcrumb: () => undefined },
|
||||
updater: {
|
||||
installCommittedAtLoad: true,
|
||||
isInstallCommitted: () => new Promise<boolean>(() => undefined),
|
||||
onInstallCommitted: () => () => undefined
|
||||
}
|
||||
} as never
|
||||
|
||||
await loadLazyWithRetry(() => Promise.reject(CORRUPT_CHUNK_ERROR()), {
|
||||
retries: 0,
|
||||
reloadKey: 'app.root'
|
||||
}).catch(() => undefined)
|
||||
|
||||
expect(window.location.reload).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still requests recovery when no install is committed', async () => {
|
||||
installBreadcrumbSink()
|
||||
|
||||
|
||||
@@ -110,6 +110,20 @@ describe('renderer updater install commitment', () => {
|
||||
expect(isUpdaterInstallCommitted()).toBe(false)
|
||||
})
|
||||
|
||||
it('arms again after an earlier install aborted', () => {
|
||||
// The user cancels one update, then restarts for the next. A stale false from
|
||||
// the first must not swallow the second window's early arming.
|
||||
installBridge()
|
||||
unregister = registerUpdaterInstallCommitment()
|
||||
broadcast(true)
|
||||
broadcast(false)
|
||||
expect(isUpdaterInstallCommitted()).toBe(false)
|
||||
|
||||
window.dispatchEvent(new Event(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT))
|
||||
|
||||
expect(isUpdaterInstallCommitted()).toBe(true)
|
||||
})
|
||||
|
||||
it('detaches cleanly so a stale listener cannot re-arm a new document', () => {
|
||||
installBridge()
|
||||
const stop = registerUpdaterInstallCommitment()
|
||||
|
||||
@@ -4,11 +4,34 @@ import { ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT } from '../../../shared/upd
|
||||
// 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
|
||||
// null until main has said something authoritative for this document.
|
||||
let broadcastCommitted: boolean | null = null
|
||||
let locallyStarted = false
|
||||
let loadTimeCommitted: boolean | null = null
|
||||
|
||||
// Why: a document created or reloaded mid-install — View → Reload, crash recovery,
|
||||
// dock activation, a popout — misses the broadcast, and its async seed can lose the
|
||||
// race outright because the Linux package install blocks main inside spawnSync.
|
||||
// Preload captures this synchronously before any document script, so the very first
|
||||
// lazy import already knows. Read lazily so it is available before React mounts.
|
||||
function committedAtDocumentLoad(): boolean {
|
||||
if (loadTimeCommitted === null) {
|
||||
loadTimeCommitted = window.api?.updater?.installCommittedAtLoad === true
|
||||
}
|
||||
return loadTimeCommitted
|
||||
}
|
||||
|
||||
export function isUpdaterInstallCommitted(): boolean {
|
||||
return broadcastCommitted || locallyStarted
|
||||
// This window just asked to install, which is fresher than any earlier verdict —
|
||||
// a stale `false` from a previous aborted install must not mask it.
|
||||
if (locallyStarted) {
|
||||
return true
|
||||
}
|
||||
// Otherwise main is authoritative: only it can stand the archive back up.
|
||||
if (broadcastCommitted !== null) {
|
||||
return broadcastCommitted
|
||||
}
|
||||
return committedAtDocumentLoad()
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -28,6 +51,7 @@ export function registerUpdaterInstallCommitment(): () => void {
|
||||
broadcastCommitted = committed
|
||||
if (!committed) {
|
||||
locallyStarted = false
|
||||
loadTimeCommitted = false
|
||||
}
|
||||
}
|
||||
const unsubscribe = window.api?.updater?.onInstallCommitted?.(applyBroadcast)
|
||||
@@ -47,12 +71,14 @@ export function registerUpdaterInstallCommitment(): () => void {
|
||||
return () => {
|
||||
window.removeEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, markLocal)
|
||||
unsubscribe?.()
|
||||
broadcastCommitted = false
|
||||
broadcastCommitted = null
|
||||
locallyStarted = false
|
||||
loadTimeCommitted = null
|
||||
}
|
||||
}
|
||||
|
||||
export function resetUpdaterInstallCommitmentForTest(): void {
|
||||
broadcastCommitted = false
|
||||
broadcastCommitted = null
|
||||
locallyStarted = false
|
||||
loadTimeCommitted = null
|
||||
}
|
||||
|
||||
@@ -3106,6 +3106,7 @@ function createUpdaterApi(): NonNullable<Partial<PreloadApi>['updater']> {
|
||||
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.
|
||||
installCommittedAtLoad: false,
|
||||
isInstallCommitted: () => Promise.resolve(false),
|
||||
onInstallCommitted: () => () => undefined,
|
||||
check: () => Promise.resolve(),
|
||||
|
||||
@@ -1,3 +1,13 @@
|
||||
/** Single source of truth: main broadcasts on this channel, preload subscribes to it. */
|
||||
export const UPDATER_INSTALL_COMMITTED_CHANNEL = 'updater:installCommitted'
|
||||
export const UPDATER_IS_INSTALL_COMMITTED_CHANNEL = 'updater:isInstallCommitted'
|
||||
/**
|
||||
* Read synchronously in preload, before any document script runs.
|
||||
*
|
||||
* Why sync: a renderer document created or reloaded mid-install (View → Reload,
|
||||
* crash recovery, dock activation, …) misses the broadcast, and an async seed can
|
||||
* lose the race — on Linux main is blocked inside the package manager's spawnSync
|
||||
* and cannot answer at all. A document that starts during an install must know it
|
||||
* before its first lazy import, or it reads a swapped archive.
|
||||
*/
|
||||
export const UPDATER_IS_INSTALL_COMMITTED_SYNC_CHANNEL = 'updater:isInstallCommittedSync'
|
||||
|
||||
Reference in New Issue
Block a user