E2E quit check: time a quit from when it reaches main; check apps that quit on their own

docs/ARCHITECTURE.md

@@ -112,7 +112,7 @@ Rules: no synchronous disk I/O on hot paths after boot, no full-state broadcasts
 ## 9. Testing and CI
 
 - **Unit** (`vitest`): shared logic (tree moves, URL parsing, accelerators, theme math, persistence normalization), main-process models with fakes. Fast; run on every commit.
-- **E2E** (`@playwright/test` `_electron`): `tests/e2e/helpers.ts` launches the built app with an isolated `userData` and `ORBIT_E2E=1`. That exposes `globalThis.__orbit` test hooks in main (state, view bounds, run command) and serves local fixture pages from a loopback HTTP server. The app runs behind a dead local proxy that loopback bypasses, and `launchOrbit` waits until every profile's session uses it, so a test that searches or follows a link to a real site gets the URL but loads nothing. Playwright reaches main through the inspector, which can interrupt main-process code part-way, even inside a store update. So `launchOrbit` makes each `app.evaluate` start in a task of its own, and the hooks' `run` and `action` defer the same way (D25). Main logs an exception nothing caught as `[orbit] uncaught exception:` rather than letting Electron show its modal box, which blocks quitting. `app.close()` fails a test whose main process logged one (`takeUncaught()` claims one thrown on purpose), and kills an app that hasn't quit 15 s after being asked (D28). **Every test title starts with the feature IDs it covers**, e.g. `test('[TB-06][TB-07] close and reopen a tab', …)`. `scripts/features.mjs` reads those IDs to mark rows tested in `FEATURES.md`.
+- **E2E** (`@playwright/test` `_electron`): `tests/e2e/helpers.ts` launches the built app with an isolated `userData` and `ORBIT_E2E=1`. That exposes `globalThis.__orbit` test hooks in main (state, view bounds, run command) and serves local fixture pages from a loopback HTTP server. The app runs behind a dead local proxy that loopback bypasses, and `launchOrbit` waits until every profile's session uses it, so a test that searches or follows a link to a real site gets the URL but loads nothing. Playwright reaches main through the inspector, which can interrupt main-process code part-way, even inside a store update. So `launchOrbit` makes each `app.evaluate` start in a task of its own, and the hooks' `run` and `action` defer the same way (D25). Main logs an exception nothing caught as `[orbit] uncaught exception:` rather than letting Electron show its modal box, which blocks quitting. `app.close()` fails a test whose main process logged one (`takeUncaught()` claims one thrown on purpose). It also kills an app that hasn't quit 15 s after the quit reached main (main logs `[orbit] quit requested` in e2e mode), or that the quit hasn't reached 35 s after `app.close()` (D28). An app that quit on its own, when its last window closed, is checked by the next `launchOrbit()`. On Linux the failure names the main thread's kernel wait; on macOS, the deepest frames of a one-second `sample`. **Every test title starts with the feature IDs it covers**, e.g. `test('[TB-06][TB-07] close and reopen a tab', …)`. `scripts/features.mjs` reads those IDs to mark rows tested in `FEATURES.md`.
 - **CI** (`.github/workflows/ci.yml`): typecheck + lint + unit (Linux), e2e under Xvfb (Linux), and build + unit + e2e + unsigned `.dmg`/`.zip` packaging (macOS). Green CI is required before merge.
 - **Feel checks and clips**: `scripts/capture/record.mjs <scenario>` drives the built app through `scripts/capture/scenarios/<scenario>.mjs` while ffmpeg (`ORBIT_FFMPEG`, needs x11grab) records the X display, so native tab views are captured exactly as composited. Clips are named by feature ID and stored in `clips/` in the Orca context directory, screenshots in `screenshots/`. "Feels right" in `FEATURES.md` is a lead sign-off against `feel.md`.
 

src/main/index.ts

@@ -20,6 +20,11 @@ perf.mark('main:start')
 // An exception nothing caught is logged, and Orbit carries on. Electron's default shows it in a modal
 // box that blocks the main thread until someone dismisses it: until then Orbit can't even quit (D28).
 process.on('uncaughtException', (err) => console.error('[orbit] uncaught exception:', err))
+// A promise rejection nothing handled is logged with the same marker, so the e2e quit check fails
+// the test that caused it. Electron's default only prints a warning (PR #11 review F3).
+process.on('unhandledRejection', (reason) =>
+  console.error('[orbit] uncaught exception: (a promise rejection nothing handled)', reason),
+)
 
 // Tests and perf runs isolate their profile data.
 if (process.env.ORBIT_USER_DATA) app.setPath('userData', process.env.ORBIT_USER_DATA)
@@ -90,6 +95,8 @@ function start(): void {
     if (windows.ids().length === 0) windows.open()
   })
   app.on('before-quit', () => {
+    // The e2e harness times a quit from here: Playwright does work of its own before it asks (D28).
+    if (e2e) console.error('[orbit] quit requested')
     windows.quitting = true
     persistence.flush()
   })

src/main/tabs/runtime.ts

@@ -418,7 +418,9 @@ export class TabRuntime {
   }
 
   private load(contents: WebContents, url: string): void {
-    if (!url || url === 'about:blank') return
+    // A tab closed while its history was restoring lands here from restore's catch, after its
+    // contents are gone: loadURL would throw, and the rejection would go unhandled.
+    if (!url || url === 'about:blank' || contents.isDestroyed()) return
     contents.loadURL(url).catch(() => {
       // Failures surface through did-fail-load; ERR_ABORTED is normal on redirects.
     })

tests/e2e/core.spec.ts

@@ -1,4 +1,5 @@
 import { mkdtempSync, writeFileSync } from 'node:fs'
+import { createServer } from 'node:http'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { pathToFileURL } from 'node:url'
@@ -10,6 +11,7 @@ import {
   action,
   type Fixtures,
   launchOrbit,
+  listenInRange,
   MOD,
   type Orbit,
   openViaCommandBar,
@@ -330,6 +332,44 @@ test('[TB-22] the quit check also catches an exception whose log is long (D28, P
   await expect.poll(() => orbit.takeUncaught()).toContain('long zzz')
 })
 
+test('[TB-22] the quit check also catches a promise rejection nothing handled (D28, PR #11 review F3)', async () => {
+  // Electron only prints a warning for these, so the check used to miss them.
+  await orbit.app.evaluate(() => {
+    void Promise.reject(new Error('a rejection nothing handled'))
+  })
+  await expect.poll(() => orbit.takeUncaught()).toContain('a rejection nothing handled')
+})
+
+test('[TB-22] a tab closed while its history is restoring leaves no rejection behind (D28, archive review F8)', async () => {
+  const { app } = orbit
+  // A page that stops answering once `hang` is set, so restoring a history that ends on it never finishes.
+  let hang = false
+  const slow = createServer((_req, res) => {
+    if (!hang) res.end('<title>Slow</title>')
+  })
+  const port = await listenInRange(slow)
+  try {
+    const tabId = (await run(app, 'tabs.create', { url: fx.url('/page/One') })) as string
+    await tabPage(app, '/page/One')
+    await run(app, 'tabs.navigate', { tabId, url: `http://127.0.0.1:${port}/slow` })
+    await tabPage(app, '/slow')
+    await run(app, 'tabs.create', { url: fx.url('/page/Other') })
+    await tabPage(app, '/page/Other')
+    // Asleep, the tab keeps its history; waking it restores that history, ending on the slow page.
+    await app.evaluate((_e, id) => (globalThis as unknown as OrbitGlobal).__orbit.ctx.tabs.sleep(id), tabId)
+    hang = true
+    await run(app, 'tabs.activate', { tabId })
+    await expect.poll(async () => (await state(app)).tabs[tabId]?.loading).toBe(true)
+    // Closing it mid-restore made restore's catch load the page into contents already destroyed.
+    await run(app, 'tabs.close', { tabId })
+    await new Promise((r) => setTimeout(r, 1500))
+    expect(orbit.takeUncaught()).toBe('')
+  } finally {
+    slow.closeAllConnections()
+    slow.close()
+  }
+})
+
 test('[TB-02] opening tabs keeps every UI mirror in step without resyncs (review M2, F4)', async () => {
   const errors: string[] = []
   orbit.shell.on('console', (m) => m.type() === 'error' && errors.push(m.text()))

tests/e2e/helpers.ts

@@ -1,3 +1,4 @@
+import { execFileSync } from 'node:child_process'
 import { mkdtempSync, readFileSync } from 'node:fs'
 import { createServer, type Server } from 'node:http'
 import type { AddressInfo } from 'node:net'
@@ -42,6 +43,8 @@ const OFFLINE = `--proxy-server=http://${DEAD_PROXY}`
 export async function launchOrbit(
   opts: { userData?: string; env?: Record<string, string>; reducedMotion?: boolean; args?: string[] } = {},
 ): Promise<Orbit> {
+  // An app that quit on its own (its last window closed) was never asked to close: check it now.
+  for (const old of quitAlone) await old.close()
   const userData = opts.userData ?? mkdtempSync(join(tmpdir(), 'orbit-e2e-'))
   const motion = opts.reducedMotion ? '--force-prefers-reduced-motion' : '--force-prefers-no-reduced-motion'
   const args = [OFFLINE, motion, ...(opts.args ?? []), resolve('out/main/index.js')]
@@ -99,48 +102,99 @@ function evaluateBetweenTasks(app: ElectronApplication): void {
   }) as ElectronApplication['evaluate']
 }
 
-/** How Orbit's main process logs an exception nothing caught (src/main/index.ts). */
+/**
+ * How Orbit's main process logs an exception nothing caught, or a promise rejection nothing handled
+ * (src/main/index.ts).
+ */
 const UNCAUGHT = '[orbit] uncaught exception:'
+/** What Orbit's main process logs in e2e mode when a quit reaches it (src/main/index.ts). */
+const QUITTING = '[orbit] quit requested'
+/** How long a quit may take once it has reached Orbit's main process. Healthy ones take under 0.5 s. */
+const QUIT_BUDGET = 15_000
+/**
+ * How long `app.close()` waits for the quit to reach main at all. Playwright does work of its own
+ * before it asks: about 5 s after a failed test here, and longer on a slower runner.
+ */
+const ASK_BUDGET = 35_000
+
+/** Apps that quit on their own, with no `app.close()` to check them yet. */
+const quitAlone = new Set<ElectronApplication>()
 
 /**
- * `app.close()` checks the quit too. The test fails if Orbit's main process logged an exception
- * nothing caught, even one thrown while quitting. If Orbit still hasn't quit 15 s after being asked,
- * the test fails and Orbit is killed, so one stuck quit can't hang the rest of the run (D28).
+ * `app.close()` checks the quit too (D28):
+ * - the test fails if Orbit's main process logged an exception nothing caught or a promise
+ *   rejection nothing handled, even while quitting;
+ * - if Orbit hasn't quit 15 s after the quit reached its main process, or the quit hasn't reached it
+ *   35 s after `app.close()`, the test fails and Orbit is killed, so one stuck quit can't hang the
+ *   rest of the run.
+ *
+ * An app that quits on its own is checked by the next `launchOrbit()`, or by its own `app.close()`.
  */
 function checkQuit(app: ElectronApplication): () => string {
   const proc = app.process()
   let recent = ''
   let thrown = ''
+  let quitting = false
+  let onQuitting: (() => void) | undefined
   proc.stderr?.on('data', (chunk) => {
+    const piece = String(chunk)
+    // Look for the markers before trimming: one write can be longer than what is kept. The tail
+    // kept is just long enough to find a marker split across two writes.
+    const text = recent + piece
+    recent = text.slice(-Math.max(UNCAUGHT.length, QUITTING.length))
+    if (!quitting && text.includes(QUITTING)) {
+      quitting = true
+      onQuitting?.()
+    }
     if (thrown) {
-      if (thrown.length < 4000) thrown += String(chunk)
+      if (thrown.length < 4000) thrown += piece
       return
     }
-    // Look for the marker before trimming: one write can be longer than what is kept. The tail
-    // kept is just long enough to find a marker split across two writes.
-    const text = recent + String(chunk)
     const at = text.indexOf(UNCAUGHT)
     if (at >= 0) thrown = text.slice(at, at + 4000)
-    else recent = text.slice(-UNCAUGHT.length)
   })
   const outputDone = new Promise((resolve) => (proc.stderr ? proc.stderr.once('close', resolve) : resolve(null)))
   const close = app.close.bind(app)
+  let asked = false
   let checked = false
+  app.once('close', () => {
+    if (!asked) quitAlone.add(app)
+  })
   app.close = async () => {
-    let timer: NodeJS.Timeout | undefined
-    const late = new Promise<true>((resolve) => {
-      timer = setTimeout(() => resolve(true), 15_000)
-    })
+    asked = true
+    quitAlone.delete(app)
     const closed = close()
-    const stuck = await Promise.race([closed.then(() => false), late])
-    clearTimeout(timer)
+    const timers: NodeJS.Timeout[] = []
+    const stuck = await new Promise<string>((resolve, reject) => {
+      const settle = () => {
+        for (const t of timers) clearTimeout(t)
+        onQuitting = undefined
+      }
+      const done = (why: string) => {
+        settle()
+        resolve(why)
+      }
+      closed.then(
+        () => done(''),
+        (err) => {
+          settle()
+          reject(err)
+        },
+      )
+      const quitClock = () => timers.push(setTimeout(() => done('it had the quit for 15 s'), QUIT_BUDGET))
+      if (quitting) quitClock()
+      else onQuitting = quitClock
+      timers.push(setTimeout(() => done('the quit never reached its main process'), ASK_BUDGET))
+    })
     if (checked) return
     checked = true
     if (stuck) {
       const waiting = mainThreadWait(proc.pid)
       proc.kill('SIGKILL')
       await closed
-      expect.soft(`still running (main thread: ${waiting})`, 'Orbit 15 s after app.close(); it was killed').toBe('quit')
+      expect
+        .soft(`still running: ${stuck} (main thread: ${waiting})`, 'Orbit after app.close(); it was killed')
+        .toBe('quit')
     }
     await Promise.race([outputDone, new Promise((resolve) => setTimeout(resolve, 1000))])
     expect.soft(thrown, "Orbit's main process threw an exception nothing caught").toBe('')
@@ -153,15 +207,32 @@ function checkQuit(app: ElectronApplication): () => string {
   }
 }
 
-/** Where a process's main thread is waiting in the kernel (Linux only). */
+/**
+ * Where a stuck process's main thread is: its wait in the kernel on Linux, the deepest frames of a
+ * one-second `sample` on macOS (no `/proc` there).
+ */
 function mainThreadWait(pid: number | undefined): string {
   try {
+    if (process.platform === 'darwin') return sampleMainThread(pid)
     return readFileSync(`/proc/${pid}/wchan`, 'utf8')
   } catch {
     return 'unknown'
   }
 }
 
+/** The deepest frames of the main thread in a one-second `sample` of the process (macOS). */
+function sampleMainThread(pid: number | undefined): string {
+  const file = join(mkdtempSync(join(tmpdir(), 'orbit-sample-')), 'sample.txt')
+  execFileSync('/usr/bin/sample', [String(pid), '1', '-file', file], { stdio: 'ignore', timeout: 20_000 })
+  const lines = readFileSync(file, 'utf8').split('\n')
+  const start = lines.findIndex((line) => line.includes('com.apple.main-thread'))
+  if (start < 0) return `unknown (no main thread in ${file})`
+  // The main thread's call tree runs until the next thread's header or the blank line after it.
+  const end = lines.findIndex((line, i) => i > start && (/^\s*\d+ Thread_/.test(line) || line.trim() === ''))
+  const frames = lines.slice(start + 1, end < 0 ? undefined : end).map((line) => line.replace(/^[\s+!:|]+/, ''))
+  return `${frames.slice(-40).join('\n')}\n(full sample: ${file})`
+}
+
 /** Finds an Orbit UI page (shell/overlay) by its ?view= query. */
 export async function viewPage(app: ElectronApplication, view: 'shell' | 'overlay'): Promise<Page> {
   const match = (p: Page) => p.url().includes(`view=${view}`)