fix(editor): clean up failed lazy chunk reload requests

src/preload/index.ts

@@ -236,10 +236,8 @@ import type { AgentType } from '../shared/native-chat-types'
 import {
   ORCA_APP_RESTART_ABORTED_EVENT,
   ORCA_APP_RESTART_STARTED_EVENT,
-  ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT,
-  ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT
+  ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT
 } from '../shared/updater-renderer-events'
-import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events'
 import {
   ORCA_INTERNAL_FILE_DRAG_TYPE,
   createNativeFileDropPayload,
@@ -279,6 +277,11 @@ import {
   createUpdaterQuitAbortRelay,
   prepareRendererForAppRestart
 } from '../shared/renderer-restart-preparation'
+import {
+  prepareAndInvokeUpdaterInstall,
+  relayRendererUnloadPrevented,
+  relayUpdaterStatus
+} from './renderer-restart-wiring'
 
 type NativeFileDropCallback = (data: NativeFileDropPayload) => void
 
@@ -290,10 +293,10 @@ const updaterQuitAbortRelay = createUpdaterQuitAbortRelay(
 )
 
 ipcRenderer.on('updater:status', (_event, status: UpdateStatus) => {
-  updaterQuitAbortRelay.handleStatus(status)
+  relayUpdaterStatus(updaterQuitAbortRelay, status)
 })
 ipcRenderer.on('window:unload-prevented', () => {
-  window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))
+  relayRendererUnloadPrevented(window)
 })
 
 function getLinuxDisplayServer(): 'wayland' | 'x11' | null {
@@ -2958,19 +2961,10 @@ const api = {
     dismissNudge: () => ipcRenderer.invoke('updater:dismissNudge'),
     dismissAvailableUpdate: () => ipcRenderer.invoke('updater:dismissAvailableUpdate'),
     listBuilds: (channel) => ipcRenderer.invoke('updater:listBuilds', channel),
-    quitAndInstall: async (): Promise<void> => {
-      await prepareRendererForAppRestart(window, {
-        startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT,
-        abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT
-      })
-      updaterQuitAbortRelay.markPrepared()
-      try {
-        return await ipcRenderer.invoke('updater:quitAndInstall')
-      } catch (error) {
-        updaterQuitAbortRelay.abort()
-        throw error
-      }
-    },
+    quitAndInstall: (): Promise<void> =>
+      prepareAndInvokeUpdaterInstall(window, updaterQuitAbortRelay, () =>
+        ipcRenderer.invoke('updater:quitAndInstall')
+      ),
     onStatus: (callback) => {
       const listener = (_event: Electron.IpcRendererEvent, status: UpdateStatus) => callback(status)
       ipcRenderer.on('updater:status', listener)

src/preload/renderer-restart-wiring.test.ts

@@ -1,36 +1,45 @@
-import { readFileSync } from 'node:fs'
-import { join } from 'node:path'
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it, vi } from 'vitest'
+import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events'
+import { ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT } from '../shared/updater-renderer-events'
+import {
+  prepareAndInvokeUpdaterInstall,
+  relayRendererUnloadPrevented,
+  relayUpdaterStatus
+} from './renderer-restart-wiring'
 
-describe('preload restart wiring', () => {
-  const source = readFileSync(join(process.cwd(), 'src/preload/index.ts'), 'utf8')
+describe('renderer restart wiring', () => {
+  it('relays updater status and prevented unload events', () => {
+    const eventTarget = new EventTarget()
+    const unloadPrevented = vi.fn()
+    const handleStatus = vi.fn()
+    eventTarget.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, unloadPrevented)
 
-  it('relays prevented unload and async updater failure IPC into renderer lifecycle events', () => {
-    expect(source).toContain("ipcRenderer.on('updater:status'")
-    expect(source).toContain('updaterQuitAbortRelay.handleStatus(status)')
-    expect(source).toContain("ipcRenderer.on('window:unload-prevented'")
-    expect(source).toContain(
-      'window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))'
-    )
+    relayUpdaterStatus({ handleStatus }, { state: 'error', message: 'install failed' })
+    relayRendererUnloadPrevented(eventTarget)
+
+    expect(handleStatus).toHaveBeenCalledWith({ state: 'error', message: 'install failed' })
+    expect(unloadPrevented).toHaveBeenCalledTimes(1)
   })
 
-  it('marks updater preparation before invoking main and aborts it on immediate IPC failure', () => {
-    const start = source.indexOf('quitAndInstall: async (): Promise<void> => {')
-    const end = source.indexOf('onStatus: (callback) => {', start)
-    expect(start).toBeGreaterThanOrEqual(0)
-    expect(end).toBeGreaterThan(start)
-    const block = source.slice(start, end)
-    const prepare = block.indexOf('await prepareRendererForAppRestart(window, {')
-    const markPrepared = block.indexOf('updaterQuitAbortRelay.markPrepared()')
-    const invoke = block.indexOf("ipcRenderer.invoke('updater:quitAndInstall')")
-    const abort = block.indexOf('updaterQuitAbortRelay.abort()')
+  it('marks preparation before invoking main and aborts on IPC failure', async () => {
+    const eventTarget = new EventTarget()
+    const calls: string[] = []
+    eventTarget.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, () => {
+      calls.push('prepared')
+    })
+    const relay = {
+      markPrepared: () => calls.push('marked'),
+      abort: () => calls.push('aborted')
+    }
+    const invoke = vi.fn(async () => {
+      calls.push('invoked')
+      throw new Error('IPC failed')
+    })
 
-    expect(prepare).toBeGreaterThanOrEqual(0)
-    expect(markPrepared).toBeGreaterThan(prepare)
-    expect(invoke).toBeGreaterThan(markPrepared)
-    expect(abort).toBeGreaterThan(invoke)
-    expect(block).toMatch(
-      /try \{\s*return await ipcRenderer\.invoke\('updater:quitAndInstall'\)\s*\} catch \(error\) \{\s*updaterQuitAbortRelay\.abort\(\)\s*throw error\s*\}/
+    await expect(prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke)).rejects.toThrow(
+      'IPC failed'
     )
+
+    expect(calls).toEqual(['prepared', 'marked', 'invoked', 'aborted'])
   })
 })

src/preload/renderer-restart-wiring.ts

@@ -0,0 +1,39 @@
+import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events'
+import {
+  prepareRendererForAppRestart,
+  type UpdaterQuitAbortRelay
+} from '../shared/renderer-restart-preparation'
+import type { UpdateStatus } from '../shared/types'
+import {
+  ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT,
+  ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT
+} from '../shared/updater-renderer-events'
+
+export function relayUpdaterStatus(
+  relay: Pick<UpdaterQuitAbortRelay, 'handleStatus'>,
+  status: UpdateStatus
+): void {
+  relay.handleStatus(status)
+}
+
+export function relayRendererUnloadPrevented(eventTarget: EventTarget): void {
+  eventTarget.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))
+}
+
+export async function prepareAndInvokeUpdaterInstall(
+  eventTarget: EventTarget,
+  relay: Pick<UpdaterQuitAbortRelay, 'markPrepared' | 'abort'>,
+  invoke: () => Promise<void>
+): Promise<void> {
+  await prepareRendererForAppRestart(eventTarget, {
+    startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT,
+    abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT
+  })
+  relay.markPrepared()
+  try {
+    await invoke()
+  } catch (error) {
+    relay.abort()
+    throw error
+  }
+}

src/renderer/src/components/editor/EditorContent.tsx

@@ -33,9 +33,6 @@ import { ExternalFileChangeBanner } from './ExternalFileChangeBanner'
 const MonacoEditor = lazy(() => import('./MonacoEditor'))
 const DiffViewer = lazy(() => import('./DiffViewer'))
 const CombinedDiffViewer = lazy(() => import('./CombinedDiffViewer'))
-// Why keyed: `reloadKey` rides on LazyChunkLoadError and on the vetoed-reload
-// breadcrumb, so the report names this chunk instead of one of the eight lazy sites
-// below. b860def2's own pre-reload crumb had already been evicted from the ring.
 const RichMarkdownEditor = lazy(() => import('./RichMarkdownEditor'), {
   reloadKey: 'rich-markdown-editor'
 })

src/renderer/src/components/editor/RichMarkdownErrorBoundary.lazy-chunk.test.tsx

@@ -1,7 +1,5 @@
 // @vitest-environment happy-dom
 
-import { readFileSync } from 'node:fs'
-import { join } from 'node:path'
 import { Suspense, act, type ReactElement, type ReactNode } from 'react'
 import { createRoot, type Root } from 'react-dom/client'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@@ -24,8 +22,6 @@ vi.mock('@/lib/crash-breadcrumb-recorder', () => ({
 }))
 
 const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
-// A guard left by a *different* document is what proves the reload landed; only then
-// does lazy-with-retry produce the sentinel this boundary suppresses.
 const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload'
 
 globalThis.IS_REACT_ACT_ENVIRONMENT = true
@@ -36,8 +32,6 @@ function createContainer(): { container: HTMLDivElement; root: Root } {
   return { container, root: createRoot(container) }
 }
 
-// Mirrors EditorContent: the Suspense sits above the boundary, which EditorContent
-// re-keys per open file (`key={viewStateScopeId}`), and which wraps the lazy editor.
 function BoundaryHarness({
   children,
   boundaryKey = 'pane-a'
@@ -60,8 +54,6 @@ async function flushReactWork(): Promise<void> {
   })
 }
 
-// The parse error a corrupt rich-editor chunk rejects with; reused across tests to
-// prove the module-scoped dedupe is reset rather than dodged by unique messages.
 const CORRUPT_CHUNK_PARSE_ERROR = "Unexpected token ':'"
 
 describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
@@ -72,7 +64,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
   beforeEach(() => {
     reportCrashMock.mockReset()
     recordBreadcrumbMock.mockReset()
-    // Dedupe is module-scoped so it can outlive boundary remounts; reset it per test.
     clearLazyChunkBreadcrumbDedupeForTest()
     window.sessionStorage.clear()
     consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined)
@@ -89,9 +80,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
     consoleError.mockRestore()
   })
 
-  // The only honest sentinel case: a landed reload already refetched the chunk and it
-  // still fails to parse, so the report is not actionable. (b860def2 itself is NOT this
-  // case — its reload was vetoed, so lazy-with-retry now re-throws the real error.)
   it('renders the fallback without reporting after guarded dynamic import exhaustion', async () => {
     window.sessionStorage.setItem(RELOAD_GUARD_KEY, LANDED_RELOAD_GUARD_VALUE)
     const LazyRejectingImport = lazyWithRetry(
@@ -112,7 +100,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
 
     expect(container?.textContent).toContain('rich markdown editor')
     expect(reportCrashMock).not.toHaveBeenCalled()
-    // The suppressed report was the only artifact naming this surface; keep evidence.
     expect(recordBreadcrumbMock).toHaveBeenCalledWith('lazy_chunk_boundary_degraded', {
       boundaryId: 'editor.rich-markdown',
       reloadKey: 'rich-markdown-editor',
@@ -120,9 +107,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
     })
   })
 
-  // The breadcrumb ring holds 30 entries and does not coalesce this name, so an
-  // impatient Retry streak on a permanently rejected chunk would erase the trail.
-  // Reuses the previous test's error to pin that the dedupe set is actually cleared.
   it('records the degraded breadcrumb once across repeated Retry clicks', async () => {
     window.sessionStorage.setItem(RELOAD_GUARD_KEY, LANDED_RELOAD_GUARD_VALUE)
     const LazyRejectingImport = lazyWithRetry(
@@ -154,8 +138,6 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
     expect(recordBreadcrumbMock).toHaveBeenCalledTimes(1)
   })
 
-  // EditorContent re-keys the boundary per open file, so instance-scoped dedupe would
-  // re-record on every markdown tab switch — the same ring flush the Retry streak causes.
   it('records the degraded breadcrumb once across boundary remounts', async () => {
     window.sessionStorage.setItem(RELOAD_GUARD_KEY, LANDED_RELOAD_GUARD_VALUE)
     const LazyRejectingImport = lazyWithRetry(
@@ -206,14 +188,4 @@ describe('RichMarkdownErrorBoundary lazy chunk containment', () => {
       })
     )
   })
-
-  // b860def2's pre-crash breadcrumb read `reloadKey: "unknown"`, which cannot tell
-  // the rich editor chunk apart from the seven other lazy chunks in EditorContent.
-  it('names the rich editor chunk at the EditorContent lazy call site', () => {
-    const source = readFileSync(
-      join(process.cwd(), 'src/renderer/src/components/editor/EditorContent.tsx'),
-      'utf8'
-    )
-    expect(source).toContain("reloadKey: 'rich-markdown-editor'")
-  })
 })

src/renderer/src/components/editor/RichMarkdownErrorBoundary.tsx

@@ -14,12 +14,7 @@ type State = {
   fileId: string
 }
 
-// Module-scoped, not per instance: EditorContent re-keys this boundary per open file,
-// so an instance field would re-record on every markdown tab switch and flush the
-// 30-entry breadcrumb ring that this very evidence lives in. The key is the cause's
-// name+message, which embeds per-chunk URLs for fetch failures, so a session that
-// exhausts recovery across many lazy sites can accumulate dozens of entries — small
-// and bounded by distinct failures, not by one.
+// Module scope deduplicates breadcrumbs across file-keyed boundary remounts.
 const recordedLazyChunkCauses = new Set<string>()
 
 export function clearLazyChunkBreadcrumbDedupeForTest(): void {
@@ -60,17 +55,12 @@ export class RichMarkdownErrorBoundary extends React.Component<Props, State> {
 
   componentDidCatch(error: Error, info: React.ErrorInfo): void {
     console.error('[RichMarkdownEditor] render crash contained by boundary', error, info)
-    // Retries and the one guarded reload are already spent, so a crash report is not
-    // actionable — but it was the only artifact naming this surface, so leave a
-    // breadcrumb. Fallback still renders (matches RecoverableRenderErrorBoundary).
     if (isLazyChunkLoadError(error)) {
       const cause = describeLazyChunkCause(error)
       if (!recordedLazyChunkCauses.has(cause)) {
         recordedLazyChunkCauses.add(cause)
         recordRendererCrashBreadcrumb('lazy_chunk_boundary_degraded', {
           boundaryId: 'editor.rich-markdown',
-          // Carried on the error itself: the pre-reload crumb naming the chunk may
-          // already have been evicted from the 30-entry ring (crash b860def2).
           reloadKey: error.reloadKey,
           cause
         })

src/renderer/src/components/error-boundaries/RecoverableRenderErrorBoundary.lazy-chunk.test.tsx

@@ -14,8 +14,6 @@ vi.mock('@/lib/react-error-boundary-reporting', () => ({
 }))
 
 const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
-// Only a guard left by a *different* document proves the reload landed, which is
-// what makes lazy-with-retry emit the sentinel this boundary suppresses.
 const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload'
 
 globalThis.IS_REACT_ACT_ENVIRONMENT = true

src/renderer/src/lib/lazy-chunk-recovery-reload.ts

@@ -5,16 +5,7 @@ import {
   ORCA_APP_RESTART_STARTED_EVENT
 } from '../../../shared/updater-renderer-events'
 
-/**
- * Why: `window.location.reload()` alone cannot recover a corrupt lazy chunk in an
- * editor app. Terminal's beforeunload handler preventDefault()s whenever any open
- * file is dirty, and Electron's will-prevent-unload cancels the navigation with no
- * dialog — so the recovery reload for crash b860def2 was requested and silently
- * dropped, leaving the pane dead for 44 minutes. The updater already solved this:
- * back up hot-exit buffers, take the one synchronous session checkpoint, and latch
- * `isIntentionalAppRestartInProgress()` so the dirty-tab guard stands down. This
- * runs that same sequence for chunk recovery.
- */
+// Bare reloads are vetoed by dirty tabs; restart preparation backs them up first.
 
 export type LazyChunkRecoveryReloadOutcome =
   /** Hot-exit backup or the session checkpoint refused; unsaved work stays put. */
@@ -23,36 +14,40 @@ export type LazyChunkRecoveryReloadOutcome =
   | 'unload-vetoed'
   /** No veto signal, but the document outlived the grace window. */
   | 'never-landed'
+  /** The host rejected the reload request before navigation could begin. */
+  | 'request-failed'
 
-// Backstop only. The veto event is authoritative, but a headless/paired-web host may
-// cancel navigation without one, so never suspend the boundary indefinitely.
+// Paired-web hosts may veto navigation without emitting the Electron signal.
 const RELOAD_SETTLE_GRACE_MS = 10_000
 
-function waitForRefusedNavigation(
-  win: Window
-): Promise<Exclude<LazyChunkRecoveryReloadOutcome, 'checkpoint-refused'>> {
-  return new Promise((resolve) => {
-    let graceTimer: ReturnType<typeof setTimeout> | undefined
-    const settle = (
-      outcome: Exclude<LazyChunkRecoveryReloadOutcome, 'checkpoint-refused'>
-    ): void => {
-      if (graceTimer !== undefined) {
-        clearTimeout(graceTimer)
-      }
-      win.removeEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, onUnloadPrevented)
-      resolve(outcome)
-    }
-    const onUnloadPrevented = (): void => settle('unload-vetoed')
+type RefusedNavigationWait = {
+  outcome: Promise<'unload-vetoed' | 'never-landed'>
+  cancel: () => void
+}
 
+function waitForRefusedNavigation(win: Window): RefusedNavigationWait {
+  let graceTimer: ReturnType<typeof setTimeout> | undefined
+  let onUnloadPrevented: () => void = () => undefined
+  const cancel = (): void => {
+    if (graceTimer !== undefined) {
+      clearTimeout(graceTimer)
+      graceTimer = undefined
+    }
+    win.removeEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, onUnloadPrevented)
+  }
+  const outcome = new Promise<'unload-vetoed' | 'never-landed'>((resolve) => {
+    const settle = (result: 'unload-vetoed' | 'never-landed'): void => {
+      cancel()
+      resolve(result)
+    }
+    onUnloadPrevented = () => settle('unload-vetoed')
     win.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, onUnloadPrevented)
     graceTimer = setTimeout(() => settle('never-landed'), RELOAD_SETTLE_GRACE_MS)
   })
+  return { outcome, cancel }
 }
 
-/**
- * Requests the one recovery reload. Resolves ONLY when the navigation was refused —
- * a landed reload tears the document down instead, so the caller stays suspended.
- */
+/** Resolves only if the navigation is refused; a landed reload destroys this document. */
 export async function requestLazyChunkRecoveryReload(
   win: Window
 ): Promise<LazyChunkRecoveryReloadOutcome> {
@@ -62,16 +57,21 @@ export async function requestLazyChunkRecoveryReload(
       abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT
     })
   } catch {
-    // prepareRendererForAppRestart already dispatched the abort; never reload over
-    // editor buffers that could not be backed up.
+    // Never reload over editor buffers that could not be backed up.
     return 'checkpoint-refused'
   }
 
-  const refused = waitForRefusedNavigation(win)
-  win.location.reload()
-  const outcome = await refused
-  // Why: the latch suppresses the unsaved-changes prompt, so a document that stayed
-  // alive must never keep it armed.
-  win.dispatchEvent(new Event(ORCA_APP_RESTART_ABORTED_EVENT))
-  return outcome
+  let cancelRefusalWait = (): void => undefined
+  try {
+    const refused = waitForRefusedNavigation(win)
+    cancelRefusalWait = refused.cancel
+    win.location.reload()
+    return await refused.outcome
+  } catch {
+    return 'request-failed'
+  } finally {
+    cancelRefusalWait()
+    // A surviving document must not retain the restart latch.
+    win.dispatchEvent(new Event(ORCA_APP_RESTART_ABORTED_EVENT))
+  }
 }

src/renderer/src/lib/lazy-with-retry.right-sidebar-syntax-error.test.ts

@@ -25,10 +25,7 @@ import { isLazyChunkLoadError, loadLazyWithRetry } from './lazy-with-retry'
 // corrupt-chunk failure; these tests pin that behavior.
 
 const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
-// Only a token left by a *different* document proves the reload landed; a token
-// matching this document means the reload was requested and vetoed.
 const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload'
-const VETOED_RELOAD_GUARD_VALUE = (): string => String(performance.timeOrigin)
 
 // The exact error the renderer received from the corrupt right-sidebar chunk.
 const reportedCrashError = (): SyntaxError => new SyntaxError("Unexpected token ')'")
@@ -113,31 +110,4 @@ describe('right-sidebar lazy chunk SyntaxError crash (regression)', () => {
     // The parse error from the very same corrupt chunk must be recovered too.
     expect(isLazyChunkLoadError(parseOutcome)).toBe(true)
   })
-
-  // Deliberate narrowing of the recovery above: a guard this document wrote itself
-  // means its reload() was vetoed, so no refetch ever happened and the sentinel
-  // would hide a live failure from the boundary.
-  it('surfaces the raw SyntaxError when this document is the one whose reload was vetoed', async () => {
-    const reload = spyOnReload()
-    window.sessionStorage.setItem(RELOAD_GUARD_KEY, VETOED_RELOAD_GUARD_VALUE())
-    const error = reportedCrashError()
-    const factory = vi.fn(() => Promise.reject(error))
-
-    const loaded = loadLazyWithRetry(factory, { retries: 0, reloadKey: 'right-sidebar' })
-    // Track settlement rather than awaiting: a regression leaves this pending forever.
-    let caught: unknown = 'pending'
-    void loaded.then(
-      (value) => {
-        caught = value
-      },
-      (rejection: unknown) => {
-        caught = rejection
-      }
-    )
-    await vi.advanceTimersByTimeAsync(5000)
-
-    expect(caught).toBe(error)
-    expect(isLazyChunkLoadError(caught)).toBe(false)
-    expect(reload).not.toHaveBeenCalled() // re-arming would loop against the veto
-  })
 })

src/renderer/src/lib/lazy-with-retry.test.ts

@@ -20,8 +20,6 @@ import {
 } from '../../../shared/editor-save-events'
 
 const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
-// A guard holding a *different* document's token is the only proof the reload
-// landed; a token matching this document means the reload was vetoed.
 const LANDED_RELOAD_GUARD_VALUE = 'doc-before-the-reload'
 const Comp: ComponentType = () => null
 const chunkParseError = (): SyntaxError => new SyntaxError("Unexpected token ']'")
@@ -59,7 +57,6 @@ function makeSessionStorageThrow(): void {
 beforeEach(() => {
   vi.useFakeTimers()
   window.sessionStorage.clear()
-  // The per-document reload cap is module state; a fresh document per test.
   resetLazyChunkReloadRequestsForTest()
 })
 
@@ -123,15 +120,10 @@ describe('loadLazyWithRetry', () => {
 
     expect(factory).toHaveBeenCalledTimes(3)
     expect(reload).toHaveBeenCalledTimes(1)
-    // The guard stores the requesting document's identity, not a bare flag.
     expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBe(String(performance.timeOrigin))
-    // The load promise must stay pending across the reload window, so the error
-    // boundary never flashes ahead of the navigation.
     expect(settled).toBe(false)
   })
 
-  // Crash b860def2: reload() was called at 19:05 and the document was still alive at
-  // 19:49 (no renderer bootstrap in between), so the requesting pane suspended forever.
   it('surfaces the original error when the guarded reload never tears the document down', async () => {
     const reload = spyOnReload()
     stubCrashReportsBreadcrumb()
@@ -151,7 +143,6 @@ describe('loadLazyWithRetry', () => {
 
     await vi.advanceTimersByTimeAsync(5000)
     expect(reload).toHaveBeenCalledTimes(1)
-    // Still suspended inside the grace window, so a landed reload shows no fallback.
     expect(settled).toBe('pending')
 
     await vi.advanceTimersByTimeAsync(10_000)
@@ -178,9 +169,6 @@ describe('loadLazyWithRetry', () => {
     expect(isLazyChunkLoadError(caught)).toBe(true)
   })
 
-  // Crash b860def2: the guarded reload() was vetoed, the document lived on for 44
-  // minutes, and the still-set guard turned the next failure into the sentinel that
-  // error boundaries suppress — hiding a recovery that never ran.
   it('reports the original error when the guard belongs to this same document (reload vetoed)', async () => {
     const reload = spyOnReload()
     window.sessionStorage.setItem(RELOAD_GUARD_KEY, String(performance.timeOrigin))
@@ -192,15 +180,11 @@ describe('loadLazyWithRetry', () => {
     await vi.advanceTimersByTimeAsync(5000)
     await assertion
 
-    // Re-arming the reload here would loop against whatever vetoed the first one.
     expect(reload).not.toHaveBeenCalled()
     const caught = await loaded.catch((rejection) => rejection)
     expect(isLazyChunkLoadError(caught)).toBe(false)
   })
 
-  // b860def2's only reload evidence was a 19:05 breadcrumb, evicted from the 30-entry
-  // ring long before the 19:49 report. This path now files the report, so it must leave
-  // its own proof in the same tick.
   it('records a lazy_chunk_reload_vetoed breadcrumb in the tick that reports the crash', async () => {
     spyOnReload()
     const recordBreadcrumb = stubCrashReportsBreadcrumb()
@@ -374,26 +358,20 @@ describe('loadLazyWithRetry', () => {
   })
 })
 
-// Crash b860def2: `window.location.reload()` was requested at 19:05:00.984Z and never
-// landed — Terminal's beforeunload handler preventDefault()s whenever any open editor
-// file is dirty and Electron's will-prevent-unload cancels the navigation with no
-// dialog. In an editor app with one unsaved tab, chunk recovery could never run.
 describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto', () => {
   type Harness = {
     navigations: string[]
     hotExitBackups: number
     restartLatchAtNavigation: boolean
-    cleanup: () => void
   }
 
-  // Mirrors production: Terminal.tsx's dirty-tab guard + Electron's will-prevent-unload
-  // relay (preload dispatches ORCA_RENDERER_UNLOAD_PREVENTED_EVENT on cancel).
+  let cleanupHarness: (() => void) | undefined
+
   function installDirtyEditorTab(options: { hotExitBackupFails?: boolean } = {}): Harness {
     const harness: Harness = {
       navigations: [],
       hotExitBackups: 0,
-      restartLatchAtNavigation: false,
-      cleanup: () => {}
+      restartLatchAtNavigation: false
     }
     const cleanupBypass = registerUpdaterBeforeUnloadBypass()
 
@@ -425,7 +403,7 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
       }
     })
 
-    harness.cleanup = () => {
+    cleanupHarness = () => {
       window.removeEventListener('beforeunload', dirtyTabGuard)
       window.removeEventListener(ORCA_EDITOR_PREPARE_HOT_EXIT_EVENT, hotExitBackup)
       cleanupBypass()
@@ -440,6 +418,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
   })
 
   afterEach(() => {
+    cleanupHarness?.()
+    cleanupHarness = undefined
     vi.restoreAllMocks()
     vi.useRealTimers()
     delete (window as unknown as { api?: unknown }).api
@@ -464,11 +444,8 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
 
     expect(harness.navigations).toEqual(['landed'])
     expect(harness.restartLatchAtNavigation).toBe(true)
-    // Unsaved buffers are backed up before the document is thrown away.
     expect(harness.hotExitBackups).toBe(1)
-    // A landed navigation kills the document, so the boundary never sees the error.
     expect(settled).toBe('pending')
-    harness.cleanup()
   })
 
   it('refuses to reload when unsaved buffers cannot be backed up', async () => {
@@ -495,7 +472,6 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
 
     expect(harness.navigations).toEqual([])
     expect(settled).toBe(error)
-    // The latch suppresses the unsaved-changes prompt; never leave it armed.
     expect(restartAborted).toHaveBeenCalled()
     expect(isIntentionalAppRestartInProgress()).toBe(false)
     expect(recordBreadcrumb).toHaveBeenCalledWith({
@@ -507,12 +483,37 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
       }
     })
     window.removeEventListener(ORCA_APP_RESTART_ABORTED_EVENT, restartAborted)
-    harness.cleanup()
+  })
+
+  it('clears recovery state when the host rejects the reload request', async () => {
+    const harness = installDirtyEditorTab()
+    const recordBreadcrumb = stubCrashReportsBreadcrumb()
+    vi.mocked(window.location.reload).mockImplementation(() => {
+      throw new Error('reload unavailable')
+    })
+    const error = chunkParseError()
+
+    const settled = await loadLazyWithRetry(() => Promise.reject(error), {
+      retries: 0,
+      reloadKey: 'rich-markdown-editor'
+    }).catch((rejection: unknown) => rejection)
+
+    expect(settled).toBe(error)
+    expect(harness.hotExitBackups).toBe(1)
+    expect(isIntentionalAppRestartInProgress()).toBe(false)
+    expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull()
+    expect(vi.getTimerCount()).toBe(0)
+    expect(recordBreadcrumb).toHaveBeenCalledWith({
+      name: 'lazy_chunk_reload_vetoed',
+      data: {
+        reloadKey: 'rich-markdown-editor',
+        message: "Unexpected token ']'",
+        outcome: 'request-failed'
+      }
+    })
   })
 
   it('settles on the unload-prevented signal instead of waiting out the blind grace window', async () => {
-    // Chromium throttles background-tab timers, so a 10s blind wait can stretch far past
-    // a real veto — and the old timer was never cleared for a navigation that did land.
     vi.spyOn(window.location, 'reload').mockImplementation(() => {
       window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))
     })
@@ -555,14 +556,11 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
     }
 
     expect(await attempt()).toBe(error)
-    // A guard left by a reload that never happened denies this document recovery for
-    // the rest of the session, even once the user saves the tab that blocked it.
     expect(window.sessionStorage.getItem(RELOAD_GUARD_KEY)).toBeNull()
 
     expect(await attempt()).toBe(error)
     expect(reload).toHaveBeenCalledTimes(2)
 
-    // ...but clearing the guard must not let a permanently vetoing window loop.
     expect(await attempt()).toBe(error)
     expect(reload).toHaveBeenCalledTimes(2)
   })
@@ -576,7 +574,6 @@ describe('loadLazyWithRetry recovery reload vs the dirty-editor-tab unload veto'
       () => undefined
     )
     await vi.advanceTimersByTimeAsync(50)
-    // A second lazy site fails while the document is already tearing down.
     void loadLazyWithRetry(() => Promise.reject(error), { retries: 0 }).then(
       () => undefined,
       () => undefined

src/renderer/src/lib/lazy-with-retry.ts

@@ -13,10 +13,8 @@ import {
  * ']'"). React.lazy permanently caches that rejection, so the error boundary's
  * "Retry" — which just re-renders the same Lazy — can never recover it; the
  * surface stays dead and reports a react-error-boundary crash. This wrapper first
- * retries transient fetch failures, then performs ONE guarded full reload — routed
- * through the intentional-restart path so Orca's dirty-tab beforeunload guard cannot
- * silently veto it — to refetch fresh chunk bytes and rebuild the ES module map,
- * before finally falling through to the error boundary.
+ * retries transient failures, then requests one restart-prepared reload before
+ * falling through to the error boundary.
  */
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any -- mirror React.lazy's own ComponentType<any> constraint so every existing call site type-checks unchanged.
@@ -49,23 +47,15 @@ export function isLazyChunkLoadError(error: unknown): error is LazyChunkLoadErro
   return error instanceof LazyChunkLoadError
 }
 
-// One recovery reload per session. The guard survives the reload itself (so we
-// never loop) but resets when the window/app closes, so a later launch — e.g.
-// after an update ships fresh chunks — can earn another reload. sessionStorage
-// (not localStorage) gives exactly that lifetime; a healthy sibling load never
-// clears it, otherwise an auto-mounted corrupt chunk would loop. The one exception
-// is a reload this document proved never landed (see clearChunkReloadGuard).
+// The session guard survives a landed reload to prevent loops, but a surviving
+// document clears its own token so a later failure can retry recovery.
 const RELOAD_GUARD_KEY = 'orca:lazy-chunk-reload-attempted'
-// Stand-in document identity for hosts without a usable performance.timeOrigin; a
-// reload re-evaluates this module, so a fresh value still means the reload landed.
+// Reloading reevaluates this fallback, giving the new document a different token.
 const FALLBACK_RELOAD_TOKEN = `doc-${Math.random().toString(36).slice(2)}`
 const DEFAULT_RETRIES = 2
 const DEFAULT_BASE_DELAY_MS = 250
 
-// The guard stores the requesting document's identity, not a bare flag: reload() can be
-// vetoed (beforeunload preventDefault → Electron cancels the navigation) and a bare flag
-// would then read as "recovery ran" forever inside a document that never reloaded (crash
-// b860def2). timeOrigin changes per navigation, so a value still ours means no reload.
+// A matching token proves this document requested a reload that never landed.
 function currentDocumentReloadToken(): string {
   const timeOrigin = typeof performance === 'undefined' ? Number.NaN : performance.timeOrigin
   return Number.isFinite(timeOrigin) && timeOrigin > 0 ? String(timeOrigin) : FALLBACK_RELOAD_TOKEN
@@ -98,9 +88,6 @@ function markChunkReloadAttempted(): boolean {
   }
 }
 
-// Why: a token this document wrote itself proves no reload happened, so leaving it in
-// place would deny the surface recovery for the whole session even after whatever
-// vetoed the navigation is gone (e.g. the user saved the dirty tab).
 function clearChunkReloadGuard(): void {
   try {
     window.sessionStorage.removeItem(RELOAD_GUARD_KEY)
@@ -109,12 +96,9 @@ function clearChunkReloadGuard(): void {
   }
 }
 
-// sessionStorage cannot bound a document that keeps re-arming its own guard, so cap
-// recovery requests in memory; the counter dies with the document a reload replaces.
+// A per-document cap prevents repeated vetoes from creating a reload loop.
 const MAX_RELOAD_REQUESTS_PER_DOCUMENT = 2
 let reloadRequestsThisDocument = 0
-// A sibling chunk failing mid-navigation reads the same 'reload-not-landed' guard;
-// clearing it there would re-arm a document that is already tearing down.
 let reloadRequestInFlight = false
 
 export function resetLazyChunkReloadRequestsForTest(): void {
@@ -211,11 +195,15 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
     reloadRequestsThisDocument += 1
     reloadRequestInFlight = true
     recordReloadBreadcrumb('lazy_chunk_reload', reloadKey, failureMessage)
-    // Resolves only if the navigation was refused; a landed reload kills this document.
-    const outcome: LazyChunkRecoveryReloadOutcome = await requestLazyChunkRecoveryReload(window)
-    reloadRequestInFlight = false
-    clearChunkReloadGuard()
-    recordReloadBreadcrumb('lazy_chunk_reload_vetoed', reloadKey, failureMessage, outcome)
+    let outcome: LazyChunkRecoveryReloadOutcome = 'request-failed'
+    try {
+      // A landed reload tears down this document before the promise settles.
+      outcome = await requestLazyChunkRecoveryReload(window)
+    } finally {
+      reloadRequestInFlight = false
+      clearChunkReloadGuard()
+      recordReloadBreadcrumb('lazy_chunk_reload_vetoed', reloadKey, failureMessage, outcome)
+    }
     throw lastError
   }
 
@@ -228,8 +216,7 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
     !reloadRequestInFlight &&
     isKnownDynamicImportFailure(lastError)
   ) {
-    // Why here and not only at request time: this crumb lands in the same tick as the
-    // crash report, so the 30-entry ring cannot evict it the way it evicted b860def2's.
+    // Record the veto beside the resulting crash report before the ring can evict it.
     clearChunkReloadGuard()
     recordReloadBreadcrumb(
       'lazy_chunk_reload_vetoed',
@@ -239,10 +226,7 @@ export async function loadLazyWithRetry<T extends AnyComponent>(
     )
   }
 
-  // No proven reload (SSR / node / blocked storage / a reload this document asked
-  // for but never got) or unknown failure: re-throw the original error so normal
-  // error reporting semantics stay intact. Boundaries suppress the sentinel, so
-  // wrapping an unproven recovery here would hide the failure entirely.
+  // Without a proven reload, preserve normal error-reporting behavior.
   throw lastError
 }