fix(selenium-devtools-py): anchor the document the drain reads

CLAUDE.md

@@ -265,6 +265,7 @@ Documented divergences from the conventions above. They exist today as debt to b
 - **A command row is stamped at COMPLETION, and the DOM anchor carries the document's own birth time.** These two together are what make the replay line up; both adapters got them wrong in the same way and the fix is symmetric. (a) `selenium-devtools/src/driverPatcher.ts` and `nightwatch-devtools/src/helpers/browserProxy.ts` both ran their capture at completion but stamped `timestamp` with the *invocation* clock, keeping the invocation time as `startTime` only after this fix. The page-side mutation stream is on real time, so an invocation-stamped row ended before its own effect landed and replayed the page from before it — the `#username` fill rendered an empty field, the `#password` fill rendered only the username, and a navigation row rendered the page it had just left. Rows also now span their real duration instead of a synthetic 1 ms. (b) `collector.captureCurrentDom` (the only producer of a mutation with a `url`) stamps `performance.timeOrigin`, not the drain clock. A drain is forced from Node whenever a collector might be fresh, which is always after the navigation — a round trip at best, a whole page load at worst — so drain-stamping put the anchor after several later actions (measured: 9/15 Selenium and 8/15 Nightwatch rows on the wrong DOM). With both in place a navigation row ends after its destination document was born, so the anchor needs no repositioning at all.
   - `core/trace-mutations.ts` `reattributeDomAnchors` remains as a narrow backstop for the one case the stamps can't cover: an anchor born *after* the last logged command, i.e. a click whose navigation commits once the click has already returned. It snaps such an anchor to the newest logged command, but **only when no logged command completed after it** — if one did, that command's row already resolves the anchor and pulling it earlier mis-credits it to a preceding action and steals the new page's DOM from rows still on the old one (measured: a 206 ms pull moved `/login` onto two rows that were on `/add_remove_elements`). Anchors are only pulled earlier, never past the newest timestamp already in the stream, or replay would apply the outgoing document's refs to the incoming tree.
   - Residual, accepted: Nightwatch's `click` resolves *before* its navigation commits (measured 5 ms), so a submit-click row can still show its pre-navigation page. Selenium is immune — its click waits for page load. Not worth another heuristic; every heuristic tried here regressed a different row.
+- **A drain must anchor the document it reads, and the flag for that has only ever had one value.** `core/script-loader.ts` `collectorDrainExpression(forceAnchor)` prepends `captureCurrentDom()` so a freshly injected collector's *async* initial anchor is not lost: the collector schedules it after `waitForBody`, so a drain issued right after a navigation beats it, reads an empty buffer, and the destination's buffer then dies with the page — leaving the navigating action with no DOM. Every production caller in both JS adapters passes `true` (selenium's `drainAfterLiveCommand`, its re-inject-after-navigation and teardown paths; nightwatch's five sites), so the `false` default is vestigial. Python's drain read `getTraceData()` with no anchor at all, which is the same missing backstop the preload does not cover; `selenium-devtools-py/src/selenium_devtools/snapshot.py` `_DRAIN_SCRIPT` now forces it **unconditionally and carries no flag** — one setting is not a knob. Forcing is free after the first anchor of a document because `packages/script` guards `captureCurrentDom` with an `#anchored` flag that deliberately survives its `reset()`, which is why selenium anchors on every live command and still emits ~3 anchors across a 16-row run rather than 16.
 - **Document-start injection is what removes the whole race class; everything else is reconstruction.** `<script>`-append injection only instruments the document loaded at the time it runs, and a `<script>` dies with its document — so a navigation always yields a document we learn about afterwards, and every question that follows (when to re-inject, when to drain, which action owns the new DOM) is guesswork. `core/bidi-preload.ts` `registerCollectorPreload` registers the collector via BiDi `script.addPreloadScript` with **no browsing-context id**, which scopes it globally so contexts created later are covered: every document then instruments itself before any of its own script runs and anchors its own DOM at its own `performance.timeOrigin`. Measured on the Nightwatch example: 5 of 5 documents anchored and **0 of 19 rows on the wrong DOM**, versus 4 of 5 and 1–5 wrong with the polling/attribution approach. The service has always done this (`browser.scriptAddPreloadScript`), which is why it never had this bug class.
   - **All three adapters now register it.** Selenium does so per driver in `session-lifecycle.ts` `registerPreload`, inside the `Promise.all` that `onDriverCreated` awaits — the patched `build()` thenable waits on that, so the preload is live before the first `get`; `ensureBidiCapability` already sets `webSocketUrl: true` on the Builder. Measured on a local two-page form, the appended-`<script>` path captured **21 of 29 input events** (all 8 username keystrokes lost — the collector came up ~1 s after `get`, behind `injectScript`'s ≥200 ms readiness poll and `capturePerformance`'s 500 ms settle) and **3 of 4 DOM anchors** (a destination that lived 150 ms never anchored, the recovery injection's poll never finishing); with the preload, **29 of 29** and **4 of 4**, 3/3 runs. On the cucumber example: **0** injections and **0** "collector missing" recoveries (was 6-9 per run), rows-on-wrong-document 0 of 15, trace zip 1.66-1.71 MB → 1.31-1.41 MB — the injected `<script>`'s own source is no longer part of the captured DOM.
   - The `<script>` branches are **gated on `SessionCapturer.preloadRegistered`, not deleted**: they are the only capture when BiDi is absent, and a *missing* collector is the fallback's only "the document was replaced" signal — which the preload makes permanently false. Verified by forcing the helper to return false: the example reproduces the pre-change numbers exactly (46 mutations, 4 anchors, 42 input events, 6 injection lines). Selenium's navigation hook also drains **before** `capturePerformance`'s 500 ms settle rather than after: the drain is what moves a destination's anchor out of the page, and a short-lived page was gone by the time the settle ended. Performance entries only get more complete with time; the anchor does not.

packages/selenium-devtools-py/README.md

@@ -82,7 +82,7 @@ is on `PATH`; otherwise keep it current (`brew upgrade chromedriver`).
 | Browser console + JS errors | Selenium **BiDi** (`driver.script` handlers) | `consoleLogs` | 2 |
 | Network requests | Selenium **BiDi** (`Network.add_event_handler`, observe-only — never an intercept, which would pause every request) | `networkRequests` | 2 |
 | Assertions | pytest hooks under pytest; line tracing for a plain script | `commands` | 2 |
-| DOM snapshot (preview iframe) | inject `packages/script`, re-inject per navigation, drain mutations | `mutations` | 2 |
+| DOM snapshot (preview iframe) | inject `packages/script`, re-inject per navigation, drain mutations with a forced document anchor | `mutations` | 2 |
 | Screencast video | screenshot polling → ffmpeg-encoded `.webm` | `screencast` | 2 |
 
 Element actions (`click`, `send_keys`, `text`, …) are captured for free: they

packages/selenium-devtools-py/src/selenium_devtools/snapshot.py

@@ -39,16 +39,32 @@
     "return true;"
 )
 
-#: Atomic check+read: the collector may vanish (navigation) between an
-#: existence check and the read, so both happen in one eval — mirrors the
+#: Whether the collector is present in THIS document. One definition, because
+#: the readiness probe and the drain's own guard must agree — mirrors core's
+#: `COLLECTOR_READY_EXPRESSION`.
+_COLLECTOR_READY = 'typeof window.wdioTraceCollector !== "undefined"'
+
+#: Atomic anchor+read: the collector may vanish (navigation) between an
+#: existence check and the read, so all of it happens in one eval — mirrors the
 #: TOCTOU fix in selenium-devtools/session.ts.
-_READ_TRACE_SCRIPT = (
-    'return typeof window.wdioTraceCollector !== "undefined"'
-    " ? window.wdioTraceCollector.getTraceData() : null;"
+#:
+#: The anchor is FORCED, and that is the point of this script. A collector's
+#: initial full-DOM anchor is scheduled asynchronously (after `waitForBody`), so
+#: a drain issued right after a navigation beats it and reads an empty buffer —
+#: and the destination's buffer then dies with the page, leaving the navigating
+#: action with no DOM at all. Forcing costs nothing after the first anchor of a
+#: document: `captureCurrentDom` is guarded by the collector's own `#anchored`
+#: flag, which deliberately survives its `reset()`. Every caller of core's
+#: `collectorDrainExpression` passes `forceAnchor: true` for the same reason, so
+#: this carries no flag to get wrong.
+_DRAIN_SCRIPT = (
+    f"if (!({_COLLECTOR_READY})) {{ return null; }} "
+    "window.wdioTraceCollector.captureCurrentDom(); "
+    "return window.wdioTraceCollector.getTraceData();"
 )
 
 #: Cheap readiness probe used after injection.
-_READY_SCRIPT = 'return typeof window.wdioTraceCollector !== "undefined";'
+_READY_SCRIPT = f"return {_COLLECTOR_READY};"
 
 #: Distinguishes "the read itself failed" from "the collector is absent here",
 #: which the page returns as a plain null and which is the recovery signal.
@@ -230,31 +246,38 @@ def _install_now(self) -> bool:
         self._injected = ready is True
         return self._injected
 
-    def _read_trace(self) -> Any:
-        """The page's trace payload, None when the collector is absent from this
-        document, or `_UNREADABLE` when the read itself failed."""
+    def _drain(self) -> Any:
+        """Anchor this document and take its trace payload. None when the
+        collector is absent from the document, `_UNREADABLE` when the eval
+        itself failed."""
         try:
-            return self._execute(_READ_TRACE_SCRIPT)
+            return self._execute(_DRAIN_SCRIPT)
         except BaseException as exc:  # noqa: BLE001
             if not _is_quiet_error(exc):
                 _warn(f"trace read failed: {exc}")
             return _UNREADABLE
 
     def pull_mutations(self) -> List[Any]:
-        """Read and drain the buffered mutations (``getTraceData()`` resets the
-        page-side buffer). Returns [] on any failure or when nothing's buffered.
+        """Anchor the current document and drain the buffered mutations
+        (``getTraceData()`` resets the page-side buffer). Returns [] on any
+        failure or when nothing's buffered.
 
         A null payload means the collector is not in THIS document, and that is
         the only signal which says so — a preload can still miss one (its script
         can throw, or a document can predate registration), and with the preload
         registered nothing else probes. Recovered once here, mirroring core's
         `drainCollectorWithRecovery`, which costs nothing on the happy path
-        because the drain happens either way."""
-        data = self._read_trace()
+        because the drain happens either way.
+
+        The retry after a recovery is the case the forced anchor exists for: a
+        collector installed a moment ago has not run its own async anchor yet,
+        so an unanchored read would return an empty buffer and the document
+        would never be anchored at all."""
+        data = self._drain()
         if data is _UNREADABLE:
             return []
         if data is None and self._install_now():
-            data = self._read_trace()
+            data = self._drain()
             if data is _UNREADABLE:
                 return []
         return normalize_mutations(data)

packages/selenium-devtools-py/tests/test_snapshot.py

@@ -159,6 +159,108 @@ def boom(script, *args):
         self.assertEqual(cap.pull_mutations(), [])  # no raise
 
 
+class FakeCollector:
+    """A page-side collector modelled closely enough to tell an anchored drain
+    from an unanchored one.
+
+    The real collector schedules its first full-DOM anchor asynchronously (after
+    `waitForBody`), so a drain arriving before that has run finds an empty
+    buffer unless it anchors the document itself. `captureCurrentDom` is guarded
+    by an `#anchored` flag that survives the buffer reset, which is what makes
+    forcing it on every drain free rather than a full DOM per command.
+    """
+
+    def __init__(self, present: bool = True, url: str = "https://x/secure"):
+        self.present = present
+        self.url = url
+        self.anchored = False
+        self.anchor_calls = 0
+        self.drains = 0
+        self._buffer: list = []
+
+    def __call__(self, script, *args):
+        if "captureCurrentDom" not in script and "getTraceData" not in script:
+            return None  # an injection or readiness probe, not our concern
+        if not self.present:
+            return None  # the collector is not in this document
+        if "captureCurrentDom" in script:
+            self.anchor_calls += 1
+            if not self.anchored:
+                self.anchored = True
+                self._buffer.append(
+                    {"type": "childList", "url": self.url, "addedNodes": ["<html>"]}
+                )
+        self.drains += 1
+        payload = {"mutations": list(self._buffer)}
+        self._buffer.clear()  # getTraceData resets the page-side buffer
+        return payload
+
+
+class TestTheDrainAnchorsTheDocument(unittest.TestCase):
+    """A navigation destination has to be anchored by the drain itself.
+
+    Without it a short-lived page is never anchored at all: its own async anchor
+    has not run when the drain arrives, and its buffer dies with the page — so
+    every action on that document replays the one before it.
+    """
+
+    def test_a_document_that_has_not_self_anchored_is_anchored_by_the_drain(self):
+        collector = FakeCollector()
+        cap = SnapshotCapturer(collector)
+
+        mutations = cap.pull_mutations()
+
+        self.assertEqual(len(mutations), 1)
+        self.assertEqual(mutations[0]["url"], "https://x/secure")
+        self.assertEqual(collector.anchor_calls, 1)
+
+    def test_anchoring_is_forced_on_every_drain_but_emits_the_document_once(self):
+        # Idempotence is what makes forcing unconditional affordable: the anchor
+        # is requested every time and the page answers with it only once.
+        collector = FakeCollector()
+        cap = SnapshotCapturer(collector)
+
+        first = cap.pull_mutations()
+        second = cap.pull_mutations()
+
+        self.assertEqual(len(first), 1)
+        self.assertEqual(second, [])
+        self.assertEqual(collector.anchor_calls, 2)
+
+    def test_the_guard_runs_before_the_collector_is_touched(self):
+        # Order matters in the one eval: calling captureCurrentDom on a document
+        # without a collector throws, and the drain must answer null instead.
+        script = snapshot._DRAIN_SCRIPT
+        guard = script.index("wdioTraceCollector !== ")
+        anchor = script.index("captureCurrentDom")
+        read = script.index("getTraceData")
+
+        self.assertLess(guard, anchor)
+        self.assertLess(anchor, read)
+        self.assertIn("return null", script[:anchor])
+
+    def test_an_absent_collector_still_answers_empty(self):
+        cap = SnapshotCapturer(FakeCollector(present=False))
+
+        self.assertEqual(cap.pull_mutations(), [])
+
+    def test_a_recovered_collector_is_anchored_by_the_retry(self):
+        # The collector was missing, an injection installed it, and the retry is
+        # the only chance to anchor a document whose own anchor has not run.
+        collector = FakeCollector(present=False)
+        cap = SnapshotCapturer(collector)
+
+        def install() -> bool:
+            collector.present = True
+            return True
+
+        with mock.patch.object(cap, "_install_now", side_effect=install):
+            mutations = cap.pull_mutations()
+
+        self.assertEqual(len(mutations), 1)
+        self.assertEqual(collector.anchor_calls, 1)
+
+
 class TestStartSnapshotCapture(unittest.TestCase):
     def _tmp_script(self):
         fh = tempfile.NamedTemporaryFile("w", suffix=".js", delete=False)