test(e2e): ask Orbit to quit from a task of its own

[siyabendoezdemir]

The e2e quit check (D28) caught an exception in archive's gate run that wasn't Orbit's. As Orbit quit, main logged TypeError: timer._onTimeout is not a function (media.spec.ts:1485, PR #12's gate run). This PR fixes the harness.

Cause

• Playwright's app.close() calls app.quit() through the inspector (electronHandle.evaluate(({ app }) => app.quit())).
  • Node's inspector can run that part-way through other main-process code.
  • PR Lead polish 2: full-split drop target, Favorite splits past 12, e2e evaluate race #10's evaluateBetweenTasks found the same for test evaluates. It doesn't cover Playwright's own call.
• Node's timer loop checks that a due timer is still set, then calls it.
  • Orbit's quit handlers clear pending timers: persistence.flush() clears the state file's next write, and history's flushSync() does the same.
  • Run between that check and the call, they make Node call a cleared timer.
• Reproduced outside Orbit with Electron's own Node (v24.21). A worker's inspector session kept evaluating "clear every timer" while 200,000 timers fired. It landed mid-dispatch in 10 of 10 runs, and threw this error in 3.
• Real quits come from a menu, a shortcut or the OS as ordinary tasks, so users can't hit this.

Fix (tests/e2e/helpers.ts)

• app.close() first asks Orbit to quit from a task of its own. The ask goes through the harness's deferred evaluate, and since f49d66a the evaluated function also waits for a task of its own (review F3).
• It then waits for Orbit to exit, and only then calls Playwright's close, which finds Orbit gone (review M1). Playwright's close always asks again through the inspector: called any earlier, it could finish a quit Orbit had cancelled, and it could run inside a timer again.
• An app that already quit on its own skips the ask.
• The kill clocks are unchanged and bound the wait: 35 s from app.close() for the quit to reach main, then 15 s for it to finish.

Tests (core.spec.ts)

• "[TB-22] app.close() asks Orbit to quit between tasks, never part-way through a timer (D28)". One timer keeps main busy for 1.5 s as the test closes it, and a before-quit listener logs whether it ran inside a timer.
  • With Playwright's raw close: 4 of 4 runs fail ("quit handler inside a timer: true").
  • With the fix: they pass.
  • The first version used ten 10 ms intervals; the review found it passed without the fix in 3 of 12 runs (F1).
• "[TB-22] a quit Orbit cancels fails the test" (test.fail(), review M1). Orbit cancels the first quit, as a "quit anyway?" question would.
  • Without the wait for exit: "Expected to fail, but passed", 2 of 2.
  • With it: the quit check kills Orbit after 15 s and fails the test, as D28 intends.
• With the fix, at load 46–51: both tests passed 3 of 3, and core.spec.ts passed 21 of 21. Lint and typecheck: exit 0.

Merge gate (D42)

Run locally on the Linux VPS on this PR's head, e4486e4 (main at 32b8894, with PR #14, merged in):

npm run build; npm run typecheck; npm run lint; ./node_modules/.bin/vitest run
xvfb-run -a -s "-screen 0 1440x900x24" ./node_modules/.bin/playwright test --workers 1 --retries 0 --reporter=line

• Build, typecheck and lint: exit 0. Unit: 375 of 375 in 39 files.
• E2E at 1440×900, one worker, no retries: 267 passed, 0 failed, 2 skipped, exit 0, in 14.7 min (06:28–06:43 UTC, load average 24–33). The two skipped are media's test.fixme frame tests (MD-01, MD-09).
• Main logged no uncaught exception, and no quit was killed.
• macOS: unverified. The macOS e2e run needs a Mac. The change is test code only.
• Update, 07:50 UTC: main moved to 60da889 (settings, PR Settings window, appearance, onboarding and updates #15). It was merged into this branch with no conflicts (new head 2019597), then up the stack into PR test(commandbar): judge CB-22's budgets on the best of up to three rounds #17 and PR test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19. Under D50 the gate reruns once, on PR test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19's head (42c9819), which contains this head; its result goes in PR test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19. Then test(e2e): ask Orbit to quit from a task of its own #16, test(commandbar): judge CB-22's budgets on the best of up to three rounds #17 and test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19 merge in that order.
• Update after the review: this branch's fixes (f49d66a) went up the stack, and the gate reran on PR test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19's new head 4345e72, which contains them: build, typecheck and lint exit 0; unit 435 of 435; e2e 306 passed, 0 failed, 2 skipped, exit 0, in 16.2 min (10:25–10:41 UTC, load average 9–34). Both TB-22 tests ran in it, and main logged no uncaught exception. Details are in PR test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19.

Review

• Independent review (reviews/lead-quit-task.md): approve with 1 must-fix.
  • M1: a quit Orbit cancels passed, because Playwright's own close finished it.
  • The lead fixed it in f49d66a, with F1 and F3.
• The fix check runs on PR test: harness follow-ups from PR #10's review (no DNS lookups, deferred evaluateHandle, SV-25/SV-26, perf) #19's head 4345e72, which contains this branch.

🤖 Generated with Claude Code

---

Note

Low Risk
Changes are limited to e2e harness teardown and new tests; no production app code.

Overview
Fixes the e2e D28 teardown so Orbit is asked to quit from the main process between Node tasks, instead of relying on Playwright’s inspector-driven app.quit() first—which could run before-quit handlers in the middle of timer dispatch and trigger spurious timer._onTimeout is not a function failures.

In tests/e2e/helpers.ts, wrapped app.close() now calls new askQuit() (deferred setImmediate + app.quit(), up to 10s) and only then Playwright’s native close, after the app has actually exited; apps that already quit skip the ask. Existing quit timeouts (35s / 15s) are unchanged.

tests/e2e/core.spec.ts adds [TB-22] coverage: one test asserts quit handlers never run inside a busy timer during close; another (test.fail) documents that a cancelled quit must not be “completed” by Playwright’s second quit attempt.

^{Reviewed by Cursor Bugbot for commit f49d66a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: a398f076-d089-475d-a958-a55cdcc511fc)

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: ec06eb5d-1ca9-4287-810a-ad6d940422bf)

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: f0cc9473-167b-4d64-af25-d7c5c7efeb13)

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 2236e6f9-9157-4db7-9dcc-6470d2e6bcbc)