diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7b900bc..f5981ab1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,9 @@ env: jobs: checks: name: Typecheck, lint, unit tests - runs-on: ubuntu-latest + # Pinned: `ubuntu-latest` moves to Ubuntu 26 from 2026-10-19. The e2e job's apt packages + # (xvfb, libasound2t64) are only known to work on 24.04. + runs-on: ubuntu-24.04 timeout-minutes: 15 steps: - uses: actions/checkout@v4 @@ -42,7 +44,7 @@ jobs: e2e-linux: name: E2E (Linux, Xvfb) - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 timeout-minutes: 25 steps: - uses: actions/checkout@v4 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 52aa9b01..758b9cfc 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). **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), 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`. - **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 fb2cff53..a70b9e20 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -17,6 +17,10 @@ import { WindowManager } from './windows' const perf = new Perf() 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)) + // Tests and perf runs isolate their profile data. if (process.env.ORBIT_USER_DATA) app.setPath('userData', process.env.ORBIT_USER_DATA) const e2e = process.env.ORBIT_E2E === '1' diff --git a/src/main/split/strip.ts b/src/main/split/strip.ts index 876f2603..8fb50210 100644 --- a/src/main/split/strip.ts +++ b/src/main/split/strip.ts @@ -30,6 +30,10 @@ interface StripState { /** The strip fades out over this long before its view hides (SV-12). */ const FADE_OUT_MS = 120 +/** The view still has its page. Once the page is destroyed, `view.webContents` is undefined. */ +const live = (view: WebContentsView | undefined): view is WebContentsView => + !!view?.webContents && !view.webContents.isDestroyed() + /** * The floating pane strip (SV-12): a small transparent view centred at the top of a split * pane with the pane's close button and its split-controls menu. Pages are native views @@ -46,7 +50,7 @@ export class PaneStrip { // The strip follows its pane when the panes move (window resize, divider drag; review F7). ctx.events.on('layout.panes', ({ panes }, { windowId }) => { const s = windowId ? this.windows.get(windowId) : undefined - if (!windowId || !s?.tabId || !s.view || s.view.webContents.isDestroyed()) return + if (!windowId || !s?.tabId || !live(s.view)) return const pane = panes.find((p) => p.tabId === s.tabId) if (pane) s.view.setBounds(this.boundsOver(pane.rect)) else this.hide(windowId, true) @@ -75,7 +79,7 @@ export class PaneStrip { const view = s?.view return { tabId: s?.tabId ?? null, - visible: !!view && !view.webContents.isDestroyed() && view.getVisible(), + visible: live(view) && view.getVisible(), bounds: view?.getBounds(), } } @@ -158,14 +162,14 @@ export class PaneStrip { s.hovered = false s.dragging = false const view = s.view - if (!view || view.webContents.isDestroyed()) return + if (!live(view)) return if (now || !wasShown) { view.setVisible(false) return } view.webContents.send('orbit:strip:hide', {}) s.fadeTimer = setTimeout(() => { - if (!s.tabId && !view.webContents.isDestroyed()) view.setVisible(false) + if (!s.tabId && live(view)) view.setVisible(false) }, FADE_OUT_MS) } @@ -208,6 +212,9 @@ export class PaneStrip { loadRenderer(contents, 'strip') const win = this.ctx.windows.get(windowId) win?.once('closed', () => { + // A timer left running would act on a view that is gone: a throw there kept Orbit from quitting (D28). + const s = this.windows.get(windowId) + if (s) for (const timer of [s.showTimer, s.hideTimer, s.fadeTimer]) clearTimeout(timer) this.windows.delete(windowId) if (!contents.isDestroyed()) contents.close() }) diff --git a/tests/e2e/core.spec.ts b/tests/e2e/core.spec.ts index ca38aa48..180f5339 100644 --- a/tests/e2e/core.spec.ts +++ b/tests/e2e/core.spec.ts @@ -303,6 +303,33 @@ test('[TB-22] closing the last window keeps it for the next launch (review F1)', expect(win?.bounds).toMatchObject({ width: 900, height: 640 }) }) +test('[TB-22] an exception nothing caught in the main process is logged; Orbit still quits and restores (D28)', async () => { + const userData = orbit.userData + const tabId = (await run(orbit.app, 'tabs.create', { url: fx.url('/page/Kept') })) as string + // Electron's default shows the exception in a modal box that holds the main thread, so Orbit + // couldn't quit: app.close() waited until the harness killed it. + await orbit.app.evaluate(() => { + setTimeout(() => { + throw new Error('a bug nothing caught') + }, 0) + }) + await expect.poll(() => orbit.takeUncaught()).toContain('a bug nothing caught') + await orbit.app.close() + orbit = await launchOrbit({ userData }) + expect((await state(orbit.app)).tabs[tabId]).toBeDefined() +}) + +test('[TB-22] the quit check also catches an exception whose log is long (D28, PR #11 review M1)', async () => { + // Logged in one write of over 5,000 characters. The check used to keep only the last 4,000 + // characters before looking for the marker, which cut the marker off. + await orbit.app.evaluate(() => { + setTimeout(() => { + throw new Error(`long ${'z'.repeat(5000)}`) + }, 0) + }) + await expect.poll(() => orbit.takeUncaught()).toContain('long zzz') +}) + 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 12802e53..2aaa283c 100644 --- a/tests/e2e/helpers.ts +++ b/tests/e2e/helpers.ts @@ -1,4 +1,4 @@ -import { mkdtempSync } from 'node:fs' +import { mkdtempSync, readFileSync } from 'node:fs' import { createServer, type Server } from 'node:http' import type { AddressInfo } from 'node:net' import { tmpdir } from 'node:os' @@ -20,6 +20,11 @@ export interface Orbit { app: ElectronApplication shell: Page userData: string + /** + * Takes what the main process has logged about exceptions nothing caught. A test that throws one on + * purpose checks it here; `app.close()` then doesn't fail the test for it (D28). + */ + takeUncaught: () => string } /** @@ -40,10 +45,11 @@ export async function launchOrbit(opts: { userData?: string; env?: Record, }) evaluateBetweenTasks(app) + const takeUncaught = checkQuit(app) const shell = await viewPage(app, 'shell') await shell.getByTestId('sidebar').waitFor() await waitOffline(app) - return { app, shell, userData } + return { app, shell, userData, takeUncaught } } /** @@ -86,6 +92,69 @@ function evaluateBetweenTasks(app: ElectronApplication): void { }) as ElectronApplication['evaluate'] } +/** How Orbit's main process logs an exception nothing caught (src/main/index.ts). */ +const UNCAUGHT = '[orbit] uncaught exception:' + +/** + * `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). + */ +function checkQuit(app: ElectronApplication): () => string { + const proc = app.process() + let recent = '' + let thrown = '' + proc.stderr?.on('data', (chunk) => { + if (thrown) { + if (thrown.length < 4000) thrown += String(chunk) + 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 checked = false + app.close = async () => { + let timer: NodeJS.Timeout | undefined + const late = new Promise((resolve) => { + timer = setTimeout(() => resolve(true), 15_000) + }) + const closed = close() + const stuck = await Promise.race([closed.then(() => false), late]) + clearTimeout(timer) + 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') + } + await Promise.race([outputDone, new Promise((resolve) => setTimeout(resolve, 1000))]) + expect.soft(thrown, "Orbit's main process threw an exception nothing caught").toBe('') + } + return () => { + const taken = thrown + thrown = '' + recent = '' + return taken + } +} + +/** Where a process's main thread is waiting in the kernel (Linux only). */ +function mainThreadWait(pid: number | undefined): string { + try { + return readFileSync(`/proc/${pid}/wchan`, 'utf8') + } catch { + return 'unknown' + } +} + /** 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}`) diff --git a/tests/e2e/mini-window.spec.ts b/tests/e2e/mini-window.spec.ts index 3569b208..23c07fe8 100644 --- a/tests/e2e/mini-window.spec.ts +++ b/tests/e2e/mini-window.spec.ts @@ -268,15 +268,23 @@ function isMacKeys(mac: string, other: string): string { return process.platform === 'darwin' ? mac : other } +type UiBridge = { orbit: { invoke: (n: string, a: unknown) => Promise } } + /** Runs a command from a Mini Window's own UI (as its buttons and menus do), without waiting on it. */ async function fromMini(app: ElectronApplication, windowId: string, name: string, args: unknown): Promise { const ui = await miniUi(app, windowId) - await ui.evaluate( - ([n, a]) => { - void (window as unknown as { orbit: { invoke: (n: string, a: unknown) => Promise } }).orbit.invoke(n, a) - }, - [name, args] as const, - ) + await ui + .evaluate( + ([n, a]) => { + void (window as unknown as UiBridge).orbit.invoke(n, a) + }, + [name, args] as const, + ) + .catch(async (err: unknown) => { + // A command that closes the Mini Window can close it before the evaluate's reply gets back. + if (!ui.isClosed()) await ui.waitForEvent('close', { timeout: 2000 }).catch(() => {}) + if (!ui.isClosed()) throw err + }) } test('[MW-05] "Open in…" moves the page into the chosen Space of the main window', async () => { diff --git a/tests/e2e/split.spec.ts b/tests/e2e/split.spec.ts index 44a26671..85e19062 100644 --- a/tests/e2e/split.spec.ts +++ b/tests/e2e/split.spec.ts @@ -75,7 +75,8 @@ async function expectViewsOverPanes(o: Orbit, tabIds: string[]): Promise { .poll( async () => { const view = await tabView(o.app, tabId) - const pane = await paneBox(o.shell, tabId) + // A pane that has only just been added grows in from nothing (SV-26): no box yet, poll on. + const pane = await paneBox(o.shell, tabId).catch(() => null) return view?.visible === true && near(view.bounds, pane) }, { message: `view of ${tabId} over its pane` }, @@ -1369,6 +1370,34 @@ test('[SV-12][SV-22] the strip follows its pane when the panes move; Duplicate k expect((await state(orbit.app)).splits[copy]?.name).toBe('Pair') }) +test('[SV-12] a window that closes while its pane strip fades out closes cleanly (D28)', async () => { + const b = await openTab(orbit.app, 'Second') + const a = await openTab(orbit.app, 'First') + await run(orbit.app, 'split.create', { tabIds: [a, b] }) + await expectViewsOverPanes(orbit, [a, b]) + const windowIds = () => + orbit.app.evaluate(() => (globalThis as unknown as { __orbit: { windowIds: () => string[] } }).__orbit.windowIds()) + const [first] = await windowIds() + // A second window keeps Orbit running once the first one closes. + await action(orbit.app, 'window.new') + await expect.poll(async () => (await windowIds()).length).toBe(2) + await showStrip(orbit.app, a) + // The strip starts fading out, and its window closes before the fade ends. The fade's timer used to + // fire on a view that was gone and throw, and a throw nothing caught kept Orbit from quitting. + await orbit.app.evaluate((_e, id) => { + const g = globalThis as unknown as { + __orbitSplit: { strip: { hide: (w: string) => void } } + __orbit: { ctx: { windows: { get: (w: string) => { close: () => void } | undefined } } } + } + g.__orbitSplit.strip.hide(id) + g.__orbit.ctx.windows.get(id)?.close() + }, first as string) + await expect.poll(async () => (await windowIds()).includes(first as string)).toBe(false) + // Main-process timers fire in order: by the end of this one, the fade's has run. + await orbit.app.evaluate(() => new Promise((resolve) => setTimeout(resolve, 300))) + expect(orbit.takeUncaught()).toBe('') +}) + test('[SV-08] a link opens in a new pane beside the page that holds it', async () => { const a = await openTab(orbit.app, 'Source') await expect.poll(async () => (await tabView(orbit.app, a))?.visible).toBe(true)