From 8223aa517b9c08bbbca353940f1eb3217729ad8f Mon Sep 17 00:00:00 2001 From: Siya <57148521+siyabendoezdemir@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:24:43 +0000 Subject: [PATCH 1/3] test(e2e): the quit check times a quit from when it reaches main, checks apps that quit on their own, and samples a stuck main thread on macOS Follow-ups F1, F2 and F4 from PR #11's review (D28). - F1: Playwright does work of its own before it asks Orbit to quit (5 s after a failed test, measured by the reviewer), and the 15 s clock used to include it. In e2e mode main now logs "[orbit] quit requested" in before-quit; Orbit gets 15 s from that line, or 35 s from app.close() if the line never comes. The failure says which. - F2: an app that quits on its own (its last window closed) had no app.close() to check it. It is now remembered when it closes, and the next launchOrbit() checks it. - F4: macOS has no /proc, so a kill said "main thread: unknown". It now runs `sample 1` and reports the main thread's deepest frames, with the full sample's path. Not run on macOS yet; any error still falls back to "unknown". Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 2 +- src/main/index.ts | 2 + tests/e2e/helpers.ts | 100 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 87 insertions(+), 17 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 758b9cfc..83c273c7 100644 --- a/docs/ARCHITECTURE.md +++ b/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 ` drives the built app through `scripts/capture/scenarios/.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`. diff --git a/src/main/index.ts b/src/main/index.ts index a70b9e20..23dd2866 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -90,6 +90,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() }) diff --git a/tests/e2e/helpers.ts b/tests/e2e/helpers.ts index 2aaa283c..dc0448f0 100644 --- a/tests/e2e/helpers.ts +++ b/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' @@ -36,6 +37,8 @@ const OFFLINE = `--proxy-server=http://${DEAD_PROXY}` /** Launches the built app (out/) with an isolated profile directory. */ export async function launchOrbit(opts: { userData?: string; env?: Record } = {}): Promise { + // 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 args = [OFFLINE, resolve('out/main/index.js')] // Linux CI/dev boxes often lack the setuid sandbox helper; macOS runs fully sandboxed. @@ -94,46 +97,94 @@ function evaluateBetweenTasks(app: ElectronApplication): void { /** How Orbit's main process logs an exception nothing caught (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() /** - * `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, even one thrown + * 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((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((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('') @@ -146,15 +197,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 { const match = (p: Page) => p.url().includes(`view=${view}`) From 9329c88698b1fa13304ac0e6170b21d1baa3bf86 Mon Sep 17 00:00:00 2001 From: Siya <57148521+siyabendoezdemir@users.noreply.github.com> Date: Sat, 10 Oct 2026 03:05:21 +0000 Subject: [PATCH 2/3] test(e2e): the quit check also fails a test on a promise rejection nothing handled in main Electron only prints a warning for an unhandled rejection in the main process, so the D28 check never saw one. Main now logs them with the same marker as an uncaught exception, so the check fails the test that caused it. The new TB-22 test fails without the logging (received "") and passes with it. PR #11 review F3. Co-Authored-By: Claude Opus 5.5 --- src/main/index.ts | 5 +++++ tests/e2e/core.spec.ts | 8 ++++++++ tests/e2e/helpers.ts | 9 ++++++--- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 23dd2866..ea75f7c8 100644 --- a/src/main/index.ts +++ b/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) diff --git a/tests/e2e/core.spec.ts b/tests/e2e/core.spec.ts index 180f5339..b6c26a9a 100644 --- a/tests/e2e/core.spec.ts +++ b/tests/e2e/core.spec.ts @@ -330,6 +330,14 @@ 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-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())) diff --git a/tests/e2e/helpers.ts b/tests/e2e/helpers.ts index dc0448f0..e706337a 100644 --- a/tests/e2e/helpers.ts +++ b/tests/e2e/helpers.ts @@ -95,7 +95,10 @@ 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' @@ -112,8 +115,8 @@ const quitAlone = new Set() /** * `app.close()` checks the quit too (D28): - * - the test fails if Orbit's main process logged an exception nothing caught, even one thrown - * while quitting; + * - 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. From a151ced4f9789550b20e701f2cae3ddbfa37f270 Mon Sep 17 00:00:00 2001 From: Siya <57148521+siyabendoezdemir@users.noreply.github.com> Date: Sat, 10 Oct 2026 03:10:19 +0000 Subject: [PATCH 3/3] fix(tabs): a tab closed while its history restores no longer leaves a rejection behind Waking a tab restores its history. If the tab closed before the restore finished, restore's catch loaded the page into contents already destroyed: loadURL threw "Object has been destroyed", and the rejection went unhandled (archive review F8, seen after archive's restore tests at quit). load() now does nothing for destroyed contents. The new TB-22 test fails without the fix, now that the quit check sees rejections (Received: "[orbit] uncaught exception: (a promise rejection nothing handled) TypeError: Object has been destroyed"), and passes 3/3 with it. Co-Authored-By: Claude Opus 5.5 --- src/main/tabs/runtime.ts | 4 +++- tests/e2e/core.spec.ts | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/main/tabs/runtime.ts b/src/main/tabs/runtime.ts index 82c8da19..ab89f064 100644 --- a/src/main/tabs/runtime.ts +++ b/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. }) diff --git a/tests/e2e/core.spec.ts b/tests/e2e/core.spec.ts index b6c26a9a..604412a1 100644 --- a/tests/e2e/core.spec.ts +++ b/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, @@ -338,6 +340,36 @@ test('[TB-22] the quit check also catches a promise rejection nothing handled (D 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('Slow') + }) + 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()))