Skip to content

Unify the driver observer drain, survive mid-step navigation, clean up on signal - #170

Merged
jantimon merged 1 commit into
mainfrom
fix/observer-drain-and-signal-cleanup
Jul 30, 2026
Merged

Unify the driver observer drain, survive mid-step navigation, clean up on signal#170
jantimon merged 1 commit into
mainfrom
fix/observer-drain-and-signal-cleanup

Conversation

@jantimon

Copy link
Copy Markdown
Owner

Three driver-reliability fixes from the concurrency/cleanup review. Patch-level (post-1.0): all three are fixes, no surface change.

1. Unified observer drain + INP delivery-race (top priority)

The end-of-step flush read INP, LoAF and layout-shift point-in-time after one rAF + one macrotask — only LCP and soft-nav had a takeRecords() drain. An entry queued-but-undispatched at that instant was silently lost, and a lost Event Timing entry reads INP lower, so assert --max-inp could pass a real regression.

  • All five observers now register a takeRecords() drain in one in-page registry (win.__wpdDrains); the flush drains every observer before it reads, from both the immediate-finish path and the bounded settleRead.
  • The bounded wait now also covers the Event Timing entry: a step that dispatched a trusted interaction (a capture-phase flag, win.__wpdSawInteraction, set only by trusted pointerup/click/keydown/keyup — a synthetic page.evaluate click is untrusted and arms nothing) waits up to INP_ENTRY_WAIT_MS (250 ms) for its entry, a tighter sibling of LCP_ENTRY_WAIT_MS.
  • All waiting stays after the step's end mark, so it never grows a measured number; a trusted interaction under the 16 ms spec floor produces no entry and its flush ends empty (absence stays absence).
  • Documented beside the LCP wait in docs/dev/navigation-and-lcp.md. E2e guard: a 45 ms handler's INP must survive the race (non-null, >=40 ms, processing recovered).

2. Disposer registry + signal handlers

src/ had zero signal handling, so SIGINT/SIGTERM/SIGHUP orphaned the Chrome process, the temp static server, and the gecko dump temp (field-confirmed: an orphaned Chrome after a SIGTERM'd run). New src/browser/disposers.ts: acquisition sites (browser launch, server listen, gecko temp creation) register a synchronous best-effort disposer and deregister on clean release; a handler runs them once, synchronously (browser.process()?.kill("SIGKILL"), unlinkSync), then re-raises the signal so exit codes stay conventional. No async in the handler, no new dependency. Unit test covers register/release/run-once/throwing-disposer.

3. Default settle survives a mid-step hard navigation

A hard navigation that commits during the default page.evaluate(SETTLE_SOURCE) threw "Execution context was destroyed" — not matched by isTransientNavError, so the record hard-failed. The settle now catches the destroyed-context family and re-attaches to the new document (mirroring waitForStable's re-attach), bounded. A single immediate re-attach still raced the mid-commit context, so the re-attach backs off briefly and retries up to SETTLE_REATTACH_LIMIT (3) before failing honestly — truer to waitForStable's bounded-loop re-attach than a literal single retry. E2e guard: a step whose action reloads mid-settle completes as a hard navigation instead of failing.

4. __cp* -> __wpd* (cosmetic)

Mechanical rename of the in-page globals to match the internal docs; all tests stay green.

Gates

build, lint, format:check, knip, unit (685), measurement (6), chrome e2e (51) — all green. Self-reviewed via a hostile two-pass (the review subagent is unavailable in this environment): pass 1 caught the single-retry commit race (-> bounded backoff loop) and a lint spread warning (-> direct Set iteration); pass 2 verified the INP wait stays after the end mark, disposer idempotency/deregistration across every launch/close/gecko path, and that the drain registry and interaction listeners do not accumulate across a navigation re-arm.

…p on signal

Close the Event Timing delivery race: the end-of-step flush drains every in-page observer's takeRecords() and waits bounded for a dispatched interaction's entry, so a lost entry can no longer read INP low past a --max-inp gate. The default settle re-attaches to the new document when a hard navigation commits mid-settle instead of failing the record. A module-level disposer registry SIGKILLs Chrome and unlinks temp files on SIGINT/SIGTERM/SIGHUP, then re-raises. Also renames the in-page __cp* globals to __wpd*.
@jantimon
jantimon merged commit 8ab7390 into main Jul 30, 2026
4 checks passed
@jantimon
jantimon deleted the fix/observer-drain-and-signal-cleanup branch July 30, 2026 05:29
@github-actions github-actions Bot mentioned this pull request Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant