fix(nightwatch-devtools): give captured command rows a stable id for replaceCommand

.changeset/nightwatch-row-ids.md

@@ -0,0 +1,5 @@
+---
+'@wdio/nightwatch-devtools': patch
+---
+
+Stamp a stable numeric `id` on Nightwatch command rows in `captureCommand` and preserve it across `replaceCommand`, and skip late screenshot replacements for rows already superseded in `commandsLog`. This prevents a late screenshot update on a command like `getText` from overwriting a `node:assert` row stamped in the same millisecond.

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

@@ -666,7 +666,10 @@ export class BrowserProxy {
     this.sessionCapturer
       .takeScreenshotViaHttp(browser)
       .then((screenshot) => {
-        if (screenshot) {
+        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

@@ -138,6 +138,7 @@ export class SessionCapturer extends SessionCapturerBase {
     const commandId = this.commandCounter++
     const commandLogEntry: CommandLog & { _id?: number } = {
       _id: commandId,
+      id: commandId,
       command,
       args,
       result,
@@ -263,6 +264,8 @@ export class SessionCapturer extends SessionCapturerBase {
     )
     const oldTimestamp: number =
       idx !== -1 ? (this.commandsLog[idx]?.timestamp ?? 0) : 0
+    const preservedId =
+      idx !== -1 ? (this.commandsLog[idx]?.id ?? oldId) : undefined
     if (idx !== -1) {
       this.commandsLog.splice(idx, 1)
     }
@@ -273,6 +276,7 @@ export class SessionCapturer extends SessionCapturerBase {
     const commandId = this.commandCounter++
     const entry: CommandLog & { _id?: number } = {
       _id: commandId,
+      id: preservedId ?? commandId,
       command,
       args,
       result,

packages/nightwatch-devtools/tests/browserProxy.test.ts

@@ -34,8 +34,10 @@ function makeCapturer() {
       error: Error | undefined,
       meta: CommandCaptureMeta = {}
     ) => {
+      const id = counter++
       commandsLog.push({
-        _id: counter++,
+        _id: id,
+        id,
         command,
         args,
         result,
@@ -839,4 +841,30 @@ describe('BrowserProxy capture ordering after a DOM-mutating command', () => {
     const { capturer } = run('click')
     expect(capturer.nativeNetwork.forward).not.toHaveBeenCalled()
   })
+
+  it('drops a late screenshot if a retry already replaced the row in commandsLog', async () => {
+    const { capturer, commandsLog } = makeCapturer()
+    let resolveScreenshot!: (val: string | null) => void
+    ;(
+      capturer.takeScreenshotViaHttp as unknown as ReturnType<typeof vi.fn>
+    ).mockImplementation(
+      () =>
+        new Promise<string | null>((resolve) => {
+          resolveScreenshot = resolve
+        })
+    )
+    const proxy = new BrowserProxy(capturer, makeTestManager(), () => ({
+      uid: 'uid-a'
+    }))
+    const browser = clickBrowser()
+    proxy.wrapBrowserCommands(browser)
+    ;(browser as unknown as Record<string, (...a: unknown[]) => unknown>).click(
+      '#a'
+    )
+    // Simulate a retry splicing out the first attempt before its screenshot resolves
+    commandsLog.splice(0, 1)
+    resolveScreenshot('stale-base64')
+    await new Promise((r) => setImmediate(r))
+    expect(capturer.sendReplaceCommand).not.toHaveBeenCalled()
+  })
 })

packages/nightwatch-devtools/tests/session.test.ts

@@ -129,6 +129,56 @@ describe('SessionCapturer.replaceCommand', () => {
     )
     expect(oldTimestamp).toBe(0)
   })
+
+  it('stamps a stable id on captured rows and preserves it across replaceCommand', async () => {
+    const { replaceCommand } = await import('@wdio/devtools-shared')
+    const cap = makeCapturer()
+    await cap.captureCommand('getText', ['#flash'], 'ok', undefined, {
+      timestamp: 1000
+    })
+    await cap.captureCommand(
+      'strictEqual',
+      ['ok', 'ok'],
+      undefined,
+      undefined,
+      {
+        timestamp: 1000
+      }
+    )
+    const getTextRow = cap.commandsLog[0]
+    const assertRow = cap.commandsLog[1]
+    expect(typeof getTextRow.id).toBe('number')
+    expect(typeof assertRow.id).toBe('number')
+    expect(getTextRow.id).not.toBe(assertRow.id)
+
+    // A late screenshot update on getText shares timestamp 1000 with the assertion
+    // after it; carrying `id` makes shared replaceCommand update getText in place
+    // instead of overwriting the assertion row via lastIndexOf(timestamp).
+    const updatedGetText = { ...getTextRow, screenshot: 'base64-shot' }
+    const afterScreenshot = replaceCommand(
+      [getTextRow, assertRow],
+      1000,
+      updatedGetText
+    )
+    expect(afterScreenshot.map((r) => r.command)).toEqual([
+      'getText',
+      'strictEqual'
+    ])
+    expect(afterScreenshot[0].screenshot).toBe('base64-shot')
+
+    // And a retry replacement preserves the public `id` while allocating a new `_id`.
+    const oldInternalId = (getTextRow as CommandLogWithId)._id
+    const { entry } = cap.replaceCommand(
+      oldInternalId,
+      'getText',
+      ['#flash'],
+      'retried',
+      undefined,
+      { timestamp: 1001 }
+    )
+    expect(entry.id).toBe(getTextRow.id)
+    expect((entry as CommandLogWithId)._id).not.toBe(oldInternalId)
+  })
 })
 
 describe('SessionCapturer.captureBrowserLogs', () => {