fix(nightwatch-devtools): key late row replaces by a per-capturer id

.changeset/nightwatch-row-ids.md

@@ -0,0 +1,6 @@
+---
+'@wdio/nightwatch-devtools': patch
+'@wdio/selenium-devtools': patch
+---
+
+A command's late screenshot now updates only that command's row. In Nightwatch, an assertion that finished in the same millisecond as the read before it could be overwritten by the read's screenshot, so the dashboard and the trace showed the read twice and lost the assertion. Every Nightwatch row now carries its own id, a retried command keeps the id of the row it replaces, and a screenshot that arrives after its row was retried is dropped. Row ids are also numbered apart per process in both adapters, so a rerun started from the dashboard, or a parallel worker, cannot update a row another process recorded.

CLAUDE.md

@@ -371,7 +371,6 @@ Documented divergences from the conventions above. They exist today as debt to b
 - **Two mechanisms resolve a command's target selector, and the WDIO one is wrong for interleaved handles.** Selenium keys a `WeakMap` on handle identity (`WebElement.id_` is a promise, so no id is readable when a command is invoked); the service uses a mutable last-selector in `service/src/command-selectors.ts`, which stamps the wrong locator for `const a = await $('#a'); const b = await $('#b'); await a.click()` (the Selenium side is covered in `selenium-devtools/tests/element-locators.test.ts`; the WDIO failure itself has **no** test — it is an unverified reading of `command-selectors.ts`). They cannot share a registry — WDIO's hook sees a *serialized* handle carrying an id string and never the live object, Selenium has the object and no readable id. Unifying would need the policy parameterized rather than the storage shared; until then the WDIO path is knowingly wrong in that case. Nightwatch is a third mechanism: it reads arg 0 through a per-kind allowlist (`assertTarget.ts`) rather than tracking handles at all, because its classic API takes selector strings. The allowlists are deliberately **not** derived from shared's `ACTION_MAP` — that table says how a command *renders* (its `Element` entries include WDIO commands called on a handle, with no selector argument), not whether arg 0 is an element definition.
 - Service renders expect-webdriverio matchers as single `expect.<matcher>` rows by **folding**, not stack/depth suppression (the old `#assertionDepth`/`#matcherStarted`/self-heal machinery is gone). The matcher's value-read (`toHaveText`→`getText`, `toExist`→`isExisting`, …) is captured as a normal command; `afterAssertion` then coalesces the synthesized `expect.*` row into that read in place — inheriting its callSource, screenshot, and timeline position — and the fold replaces **by timestamp, never a public `id`**: `id` is the per-worker `commandCounter`, which resets per spec, so stamping one lets the app's id-first `replaceCommand` swap a same-id row from another spec (duplicate rows + a fold from another spec vanishing, in multi-spec live mode). `beforeAssertion` arms the pending matcher (depth-counted so aliases like `toBeChecked`→`toBeSelected` fold once); a matcher that **hard-throws** — element never resolves, so expect-webdriverio's `waitUntil` rethrows and `afterAssertion` never fires — is synthesized at `afterTest`/`afterStep` from the throwing read, so a failing assertion renders as `expect.<matcher>` whether or not the element existed. Two limits: its error is then the read's (`Can't call getText on … element wasn't found`), not an assertion-phrased message; and `MATCHER_READ_COMMANDS` is a hand-maintained allowlist, so a matcher whose read isn't listed leaves its raw read visible alongside the `expect.*` row. Plain-value jest matchers (`expect(x).toBe(y)`) don't fire the ewdio hooks, so they aren't captured as rows.
 - **What the conformance scenario shows the adapters doing differently.** Each entry is a `KNOWN_GAPS` reason in `packages/conformance/src/allowlist.ts`, scoped to the differences it explains (one hunk, one moved step, or one test's reported state), and reported stale once they stop occurring, except an entry marked `timingDependent`, which is never reported stale and has to be removed by hand.
-  - **A same-millisecond replaceCommand overwrites the Nightwatch row after it** (#458). A read's late screenshot (`browserProxy.ts` `attachScreenshot`) re-sends its row with `sendReplaceCommand(timestamp, ...)`; Nightwatch rows carry no numeric `id`, so shared `replaceCommandIndex` falls back to the last row with that timestamp, and a node:assert row stamped in the same millisecond is the one replaced. The stream then lists the read twice and loses the assertion (3 of 9 runs).
   - **Nightwatch reports a passing test skipped** (#459). The scenario's two passing tests that call no assertion (the condition waits, and the caught refused navigation) run, their rows are in the stream, and the runner does not count them failed, but the suite tree reports both `skipped` in every run; the passing test that asserts is reported `passed`.
   - **A Selenium wait has no row when it succeeds, and a condition row when it fails** (#464). A successful `driver.wait(until.elementLocated(...))` is a folded `findElements` row, which ACTION_MAP leaves unmapped; a timed-out one is a `wait` row named after the condition and mapped to `Browser.waitForFunction` without a target, where the WDIO service and Nightwatch show `Element.waitForExist` on the selector.
   - **A condition wait shows its reads in some adapters and one wait row in others** (#464). A passing wait on a condition that reads the page (`waitUntil(() => getText ...)`, `driver.wait(async () => ...)`, `WebDriverWait(...).until(lambda d: ...)`) is one `Browser.waitForFunction` row in the WebdriverIO service, which logs only the command on top of its stack and so hides the reads inside it; its reads and no wait row in Selenium and Python; and both in Nightwatch. The reads are what show a fold bug (one read folded over another), so the scenario keeps them visible where the adapter shows them.

packages/conformance/src/allowlist.ts

@@ -16,10 +16,8 @@ import type {
   Violation
 } from './types.js'
 
-const [LOGIN, WAITS, HEADING, MISSING, REFUSED] = [
-  'logs in and reads the greeting',
+const [WAITS, MISSING, REFUSED] = [
   'waits on what the page shows',
-  'fails on the wrong heading',
   'times out waiting for a missing element',
   'survives a refused navigation'
 ].map(scenarioIndex)
@@ -71,48 +69,16 @@ export const KNOWN_GAPS: KnownGap[] = [
 ]
 
 /** Nightwatch's scenario gaps in one mode, each an adapter bug with its Known debt entry. */
-// eslint-disable-next-line max-lines-per-function -- a declarative list of gaps; splitting it would only scatter them
 function nightwatchGaps(bidi: BidiMode): KnownGap[] {
   const adapter = 'nightwatch-devtools'
   return [
-    {
-      adapter,
-      bidi,
-      rules: [RULES.failedTestExplained],
-      only: failureUnexplained(HEADING),
-      reason: KNOWN_DEBT.nightwatchSameMsReplace,
-      timingDependent: true
-    },
     {
       adapter,
       bidi,
       rules: [RULES.testsAsDesigned],
       only: anyOf(reportedAs(WAITS, 'skipped'), reportedAs(REFUSED, 'skipped')),
       reason: KNOWN_DEBT.nightwatchSkipped
     },
-    {
-      adapter,
-      bidi,
-      rules: [RULES.agreeOnActions],
-      only: anyOf(
-        hunk(
-          LOGIN,
-          'Element.getText #flash',
-          adapter,
-          'Element.getText #flash',
-          'Assert.equal #flash'
-        ),
-        hunk(
-          HEADING,
-          'Element.getText h2',
-          adapter,
-          'Element.getText h2',
-          'Assert.equal h2'
-        )
-      ),
-      reason: KNOWN_DEBT.nightwatchSameMsReplace,
-      timingDependent: true
-    },
     {
       adapter,
       bidi,

packages/conformance/src/constants.ts

@@ -184,7 +184,6 @@ const debt = (entry: string, issue?: string) =>
 
 /** Why each scenario gap exists: the limitation, and the Known debt entry that describes it. */
 export const KNOWN_DEBT = {
-  nightwatchSameMsReplace: `A Nightwatch row's late replaceCommand, keyed by timestamp, overwrites a later row stamped in the same millisecond, so an assertion row becomes a second copy of the read before it. ${debt('A same-millisecond replaceCommand overwrites the Nightwatch row after it', '#458')}`,
   nightwatchSkipped: `Nightwatch reports a passing test that ran no assertion as skipped, although it ran. ${debt('Nightwatch reports a passing test skipped', '#459')}`,
   conditionWait: `A passing condition wait shows as its reads in Selenium and Python, which have no row for the wait itself, as one Browser.waitForFunction row in the WebdriverIO service, which hides the reads inside it, and as both in Nightwatch. ${debt('A condition wait shows its reads in some adapters and one wait row in others', '#464')}`,
   seleniumWait: `A successful driver.wait() has no row of its own: its polls are findElements rows, which the trace vocabulary leaves unmapped. ${debt('A Selenium wait has no row when it succeeds, and a condition row when it fails', '#464')}`,

packages/conformance/src/features.ts

@@ -22,6 +22,7 @@ const SELENIUM = ADAPTER_TESTS['selenium-devtools']
 const NIGHTWATCH = ADAPTER_TESTS['nightwatch-devtools']
 const PYTHON = ADAPTER_TESTS['selenium-devtools-py']
 
+const FAILED_TEST_RULES = [...ERROR_RULES, RULES.failedTestExplained]
 const SAFARI_FOCUS_ONCE =
   'Brings Safari to the front once after the session starts (live mode, local macOS), so a click issued after another window took the front can still land nowhere.'
 const NO_SCREENSHOT_SWITCH = 'No option: every command row takes a screenshot.'
@@ -302,11 +303,11 @@ export const FEATURES = [
     id: 'failed-command-errors',
     name: "Failed command in the Errors tab, with the browser's message",
     tables: ['readme-dashboard', 'cross-framework'],
-    // Nightwatch and Python hold gaps on a failed test's error row (#458, #461), so only the others are tested on it.
+    // Python holds a gap on a failed test's error row (#461), so only the others are tested on it.
     cells: {
-      'devtools-service': tested([...ERROR_RULES, RULES.failedTestExplained]),
-      'selenium-devtools': tested([...ERROR_RULES, RULES.failedTestExplained]),
-      'nightwatch-devtools': tested(ERROR_RULES),
+      'devtools-service': tested(FAILED_TEST_RULES),
+      'selenium-devtools': tested(FAILED_TEST_RULES),
+      'nightwatch-devtools': tested(FAILED_TEST_RULES),
       'selenium-devtools-py': tested(ERROR_RULES)
     }
   },

packages/conformance/tests/agreement.test.ts

@@ -5,7 +5,7 @@ import type { RunnerOptions } from '@wdio/devtools-shared'
 import { compareRuns, judgeDivergences } from '../src/agreement.js'
 import { covers, declaresLacks, KNOWN_GAPS } from '../src/allowlist.js'
 import { KNOWN_DEBT, RULES, SCENARIO_TESTS } from '../src/constants.js'
-import { moved } from '../src/gap-matchers.js'
+import { hunk, moved } from '../src/gap-matchers.js'
 import { normalizeRun } from '../src/normalize.js'
 import { scenarioIndex } from '../src/scenario-tests.js'
 import { renderReport } from '../src/report-html.js'
@@ -656,6 +656,20 @@ const moveGap = (): KnownGap => ({
   reason: 'Test fixture: a step one adapter shows at another place.'
 })
 
+const overwriteGap = (): KnownGap => ({
+  adapter: 'nightwatch-devtools',
+  bidi: 'live',
+  rules: [RULES.agreeOnActions],
+  only: hunk(
+    scenarioIndex(LOGIN),
+    'Element.getText #flash',
+    'nightwatch-devtools',
+    'Element.getText #flash',
+    'Assert.equal #flash'
+  ),
+  reason: 'Test fixture: one hunk where a row shows in place of another.'
+})
+
 describe('a step one adapter shows at another place', () => {
   const flashWait = { command: 'waitForExist', selector: '#flash' }
   const read = { command: 'getText', selector: '#flash' }
@@ -677,10 +691,7 @@ describe('a step one adapter shows at another place', () => {
       )
     ])
     const waitOrder = moveGap()
-    const sameMs = gapOf(
-      'nightwatch-devtools',
-      KNOWN_DEBT.nightwatchSameMsReplace
-    )
+    const sameMs = overwriteGap()
     expect(covers(waitOrder, divergences[0])).toBe(false)
     expect(covers(sameMs, divergences[0])).toBe(false)
   })
@@ -735,12 +746,7 @@ describe('a step one adapter shows at another place', () => {
     }
     expect(covers(waitOrder, move)).toBe(true)
     expect(covers(waitOrder, overwrite)).toBe(false)
-    expect(
-      covers(
-        gapOf('nightwatch-devtools', KNOWN_DEBT.nightwatchSameMsReplace),
-        overwrite
-      )
-    ).toBe(true)
+    expect(covers(overwriteGap(), overwrite)).toBe(true)
   })
 })
 

packages/core/src/session-capturer.ts

@@ -1,3 +1,4 @@
+import { randomInt } from 'node:crypto'
 import fs from 'node:fs/promises'
 import { WebSocket } from 'ws'
 import type {
@@ -91,7 +92,8 @@ export abstract class SessionCapturerBase {
   // (WDIO owns the command lifecycle), so this state is harmless overhead.
   // `protected` (not `#`) so subclasses can override the send/replace flow
   // while still sharing the counter and de-dup set with base helpers.
-  protected commandCounter = 0
+  // A row's public `id` must not repeat across processes: a rerun child's rows join its parent's in the backend's accumulator, and test workers share one dashboard.
+  protected commandCounter = randomInt(2 ** 32) * 2 ** 20
   protected sentCommandIds = new Set<number>()
 
   // Map of file path → source text. Populated by `captureSource` (also

packages/core/tests/session-capturer-base.test.ts

@@ -310,7 +310,19 @@ describe('sendCommand', () => {
 
   it('auto-allocates _id when omitted', () => {
     const id = cap.sendCommand({ command: 'click', args: [], timestamp: 1 })
-    expect(id).toBe(0)
+    expect(id).toBeTypeOf('number')
+    expect(cap.sendCommand({ command: 'b', args: [], timestamp: 1 })).toBe(
+      id + 1
+    )
+  })
+
+  // A rerun child's rows join its parent's in the backend, and test workers share one dashboard.
+  it('numbers each capturer apart, so an id names one row across processes', () => {
+    const other = new TestSessionCapturer()
+    const first = cap.sendCommand({ command: 'a', args: [], timestamp: 1 })
+    const second = other.sendCommand({ command: 'a', args: [], timestamp: 1 })
+    expect(Number.isSafeInteger(first)).toBe(true)
+    expect(second).not.toBe(first)
   })
 
   it('de-dupes — second call with same _id is a no-op', () => {

packages/nightwatch-devtools/src/helpers/browserProxy.ts

@@ -666,7 +666,11 @@ export class BrowserProxy {
     this.sessionCapturer
       .takeScreenshotViaHttp(browser)
       .then((screenshot) => {
-        if (screenshot) {
+        // A retry has replaced the row meanwhile, and its replacement carries the same `id`.
+        if (
+          screenshot &&
+          this.sessionCapturer.commandsLog.includes(entry as CommandLog)
+        ) {
           entry.screenshot = screenshot
           this.sessionCapturer.sendReplaceCommand(ts, entry as CommandLog)
           log.info(`[screenshot] Attached to ${methodName}${suffix}`)

packages/nightwatch-devtools/src/session.ts

@@ -136,8 +136,10 @@ export class SessionCapturer extends SessionCapturerBase {
     const serializedError = serializeError(error)
 
     const commandId = this.commandCounter++
+    // A public `id`, so a late replace of this row cannot land on another row stamped in the same millisecond.
     const commandLogEntry: CommandLog & { _id?: number } = {
       _id: commandId,
+      id: commandId,
       command,
       args,
       result,
@@ -269,8 +271,8 @@ export class SessionCapturer extends SessionCapturerBase {
     const idx = this.commandsLog.findIndex(
       (c) => (c as CommandLog & { _id?: number })._id === oldId
     )
-    const oldTimestamp: number =
-      idx !== -1 ? (this.commandsLog[idx]?.timestamp ?? 0) : 0
+    const previous = idx !== -1 ? this.commandsLog[idx] : undefined
+    const oldTimestamp: number = previous?.timestamp ?? 0
     if (idx !== -1) {
       this.commandsLog.splice(idx, 1)
     }
@@ -279,8 +281,10 @@ export class SessionCapturer extends SessionCapturerBase {
 
     const serializedError = serializeError(error)
     const commandId = this.commandCounter++
+    // The retry keeps the row's public `id`, so the dashboard updates the row it already shows.
     const entry: CommandLog & { _id?: number } = {
       _id: commandId,
+      id: previous?.id ?? commandId,
       command,
       args,
       result,