fix(browser): distinguish duplicate screenshot frame events (#612)

* fix(browser): distinguish duplicate screenshot frame events

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>

* test(browser): preserve unknown with-scope loader bindings

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>

---------

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>

src/browser/CdpScreenshot.ts

@@ -56,7 +56,15 @@ export const captureCdpScreenshot = async (
   const mainFrame = captureFrames(before, origins, 1).items[0];
   if (authorized === undefined || mainFrame === undefined)
     throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
-  const navigation = { changed: false, leftScope: false };
+  const initialFrame = recordValue(
+    recordValue(recordValue(before)?.frameTree)?.frame,
+  );
+  const navigation = {
+    changed: false,
+    leftScope: false,
+    url: beforeUrl,
+    loaderId: stringValue(initialFrame?.loaderId),
+  };
   const removeListener = context.connection.onEvent((event) => {
     if (event.sessionId !== context.sessionId) return;
     observeScreenshotNavigation(event, mainFrame.frame_id, origins, navigation);
@@ -125,13 +133,27 @@ const observeScreenshotNavigation = (
   event: CdpEvent,
   mainFrameId: string,
   origins: ReadonlySet<string>,
-  state: { changed: boolean; leftScope: boolean },
+  state: {
+    changed: boolean;
+    leftScope: boolean;
+    readonly url: string | undefined;
+    readonly loaderId: string | undefined;
+  },
 ): void => {
   if (!isMainFrameNavigation(event, mainFrameId)) return;
   const params = recordValue(event.params);
   if (params === undefined) return;
   const frame =
     event.method === "Page.frameNavigated" ? recordValue(params.frame) : params;
+  if (
+    event.method === "Page.frameNavigated" &&
+    state.loaderId !== undefined &&
+    state.loaderId !== "" &&
+    stringValue(frame?.loaderId) === state.loaderId &&
+    stringValue(frame?.url) === state.url
+  )
+    return;
+
   state.changed = true;
   if (allowedSanitizedUrl(frame?.url, origins) === undefined)
     state.leftScope = true;

tests/boundary/browser/cdpScreenshotDocumentIdentity.test.ts

@@ -0,0 +1,29 @@
+import { describe, expect, it, onTestFinished } from "vitest";
+import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js";
+import { captureWebScreenshotInputSchema } from "../../../src/domain/webScreenshot.js";
+import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js";
+describe("screenshot committed document identity", () => {
+  it.each(["loader-main", "loader-reloaded"])(
+    "distinguishes %s for the same URL",
+    async (screenshotDocumentLoader) => {
+      const browser = await startFakeCdpBrowser({ screenshotDocumentLoader });
+      onTestFinished(async () => {
+        await browser.close();
+      });
+      const result = await new CdpBrowserProvider().captureScreenshot(
+        captureWebScreenshotInputSchema.parse({
+          cdp_endpoint: browser.endpoint,
+          allowed_origins: [browser.allowedOrigin],
+          target_id: "allowed-page",
+        }),
+      );
+      if (screenshotDocumentLoader === "loader-main")
+        expect(result.ok).toBe(true);
+      else
+        expect(result).toMatchObject({
+          ok: false,
+          error: { reason: "target_changed" },
+        });
+    },
+  );
+});

tests/fixtures/fakeCdpBrowserEvents.ts

@@ -13,7 +13,7 @@ export const emitEvents = (
   emitRuntimeEvents(socket, command, port, options);
   emitWebMcpEvents(socket, command, port, options);
   emitSessionTimeline(socket, command, port, options);
-  emitCaptureNavigation(socket, command, options);
+  emitCaptureNavigation(socket, command, port, options);
 };
 
 const emitWebMcpEvents = (
@@ -462,8 +462,20 @@ const emitRuntimeEvents = (
 const emitCaptureNavigation = (
   socket: WebSocket,
   command: FakeCdpCommand,
+  port: number,
   options: FakeOptions,
 ): void => {
+  if (
+    command.method === "Page.captureScreenshot" &&
+    options.screenshotDocumentLoader !== undefined
+  )
+    event(socket, "Page.frameNavigated", command.sessionId, {
+      frame: {
+        id: "frame-main",
+        url: `http://127.0.0.1:${String(port)}/app?token=frame-secret`,
+        loaderId: options.screenshotDocumentLoader,
+      },
+    });
   const navigationUrl =
     command.method === "DOMSnapshot.captureSnapshot"
       ? options.navigateDuringCaptureUrl

tests/fixtures/fakeCdpBrowserTypes.ts

@@ -42,6 +42,7 @@ export interface FakeOptions {
   readonly navigateDuringObservationUrl?: string;
   readonly navigateDuringCaptureUrl?: string;
   readonly navigateDuringScreenshotUrl?: string;
+  readonly screenshotDocumentLoader?: string;
   readonly extraCollections?: boolean;
   readonly foreignSessionEvents?: boolean;
   readonly redirectToDisallowedOrigin?: boolean;