fix(browser): preserve frame tree root identity (#7915)

src/browser/CdpCaptureDocuments.ts

@@ -38,7 +38,7 @@ export type CapturedResource = Omit<
   "resource_key"
 > & { readonly rawUrl: string };
 
-/** Normalize every allowed frame, optionally selecting a leading frame. */
+/** Normalize allowed frames and retain the enclosing tree's root identity. */
 export const captureFrames = (
   result: unknown,
   allowedOrigins: ReadonlySet<string>,
@@ -46,13 +46,15 @@ export const captureFrames = (
   completeness?: CdpCaptureCompleteness,
 ): {
   readonly items: WebPageInspection["frames"];
+  readonly root: WebPageInspection["frames"][number] | undefined;
 } => {
   const root = recordValue(recordValue(result)?.frameTree);
   if (root === undefined) {
     completeness?.exclude("frames", "invalid_protocol_value");
-    return { items: [] };
+    return { items: [], root: undefined };
   }
   const items: WebPageInspection["frames"] = [];
+  let rootFrame: WebPageInspection["frames"][number] | undefined;
   for (const tree of walkFrameTrees(root, (omittedCount) =>
     completeness?.exclude("frames", "invalid_protocol_value", omittedCount),
   )) {
@@ -84,14 +86,17 @@ export const captureFrames = (
       (tree !== root && identity.parentRelation === "root")
     )
       completeness?.exclude("frames", "invalid_protocol_value");
-    items.push({
+    const captured = {
       frame_id: frameId,
       parent_frame_id: identity.parentId,
       url: facts.url,
       origin: facts.origin,
-    });
+    };
+    items.push(captured);
+    if (tree === root && identity.parentRelation === "root")
+      rootFrame = captured;
   }
-  return { items };
+  return { items, root: rootFrame };
 };
 
 export type CdpFrameUrlObservation =

src/browser/CdpPageCapture.ts

@@ -164,9 +164,9 @@ const captureAuthorizedPage = async (
     state.events.completeness,
   );
   const frames = frameCapture.items;
-  const captureFrame = frames[0];
+  const captureFrame = frameCapture.root;
   if (captureFrame === undefined)
-    throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
+    throw new BrowserObservationError("inspect_web_page", "protocol_error");
   state.events.beginFinalCapture(captureFrame.frame_id);
   const resourceCapture = captureResources(
     await connection.send("Page.getResourceTree", {}, sessionId, signal),
@@ -294,9 +294,9 @@ const authorizeObservationWindow = async (
     allowedOrigins,
     operation: context.operation,
   });
-  const mainFrame = captureFrames(initialFrameResult, allowedOrigins).items[0];
+  const mainFrame = captureFrames(initialFrameResult, allowedOrigins).root;
   if (mainFrame === undefined)
-    throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
+    throw new BrowserObservationError("inspect_web_page", "protocol_error");
   events.beginAuthorizedFrame(mainFrame.frame_id);
   await enableObservationDomains(state);
   await report(context.progress, 2, "Observing page events");

tests/boundary/browser/cdpCaptureNavigation.test.ts

@@ -2,6 +2,7 @@ import { expect, it, onTestFinished } from "vitest";
 
 import { inspectWebPageInputSchema } from "../../../src/domain/browserObservation.js";
 import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js";
+import { BrowserObservationError } from "../../../src/domain/browserObservationError.js";
 import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js";
 
 it("captures a page without aborting on a duplicate frame event", async () => {
@@ -19,3 +20,164 @@ it("captures a page without aborting on a duplicate frame event", async () => {
   );
   expect(result.ok).toBe(true);
 });
+
+it("accepts child navigation when the frame-tree root has an identity", async () => {
+  const browser = await startFakeCdpBrowser({
+    eventsBeforeReply: ["Network.enable"],
+    commandResult: (command, origin) =>
+      command.method === "Page.getFrameTree"
+        ? {
+            frameTree: {
+              frame: { id: "main", url: `${origin}/app` },
+              childFrames: [
+                {
+                  frame: {
+                    id: "child",
+                    parentId: "main",
+                    url: `${origin}/child`,
+                  },
+                },
+              ],
+            },
+          }
+        : undefined,
+    commandEvents: (command, origin) =>
+      command.method === "Network.enable"
+        ? [
+            {
+              method: "Page.frameNavigated",
+              sessionId: command.sessionId,
+              params: {
+                frame: {
+                  id: "child",
+                  parentId: "main",
+                  url: `${origin}/child`,
+                },
+              },
+            },
+          ]
+        : undefined,
+  });
+  const provider = new CdpBrowserProvider();
+  try {
+    const result = await provider.inspectPage(
+      inspectWebPageInputSchema.parse({
+        cdp_endpoint: browser.endpoint,
+        allowed_origins: [browser.allowedOrigin],
+        target_id: "allowed-page",
+        observation_ms: 0,
+      }),
+    );
+    expect(result.ok).toBe(true);
+  } finally {
+    await provider.close();
+    await browser.close();
+  }
+});
+
+it.each([false, true])(
+  "requires the actual frame-tree root identity before observing (child event: %s)",
+  async (navigateChild) => {
+    const commands: string[] = [];
+    const browser = await startFakeCdpBrowser({
+      eventsBeforeReply: ["Network.enable"],
+      commandResult: (command, origin) => {
+        commands.push(command.method);
+        if (command.method !== "Page.getFrameTree") return undefined;
+        return {
+          frameTree: {
+            frame: { url: `${origin}/app` },
+            childFrames: [
+              {
+                frame: {
+                  id: "child",
+                  parentId: "main",
+                  url: `${origin}/child`,
+                },
+              },
+            ],
+          },
+        };
+      },
+      commandEvents: (command, origin) =>
+        navigateChild && command.method === "Network.enable"
+          ? [
+              {
+                method: "Page.frameNavigated",
+                sessionId: command.sessionId,
+                params: {
+                  frame: {
+                    id: "child",
+                    parentId: "main",
+                    url: `${origin}/child`,
+                  },
+                },
+              },
+            ]
+          : undefined,
+    });
+    const provider = new CdpBrowserProvider();
+    try {
+      const result = await provider.inspectPage(
+        inspectWebPageInputSchema.parse({
+          cdp_endpoint: browser.endpoint,
+          allowed_origins: [browser.allowedOrigin],
+          target_id: "allowed-page",
+          observation_ms: 0,
+        }),
+      );
+      expect(result.ok).toBe(false);
+      if (!result.ok) {
+        expect(result.error).toBeInstanceOf(BrowserObservationError);
+        expect(result.error).toMatchObject({ reason: "protocol_error" });
+      }
+      expect(commands).not.toContain("Network.enable");
+      expect(commands).not.toContain("Runtime.enable");
+      expect(commands).not.toContain("Debugger.enable");
+    } finally {
+      await provider.close();
+      await browser.close();
+    }
+  },
+);
+
+it("does not promote a child when the final frame-tree root loses its identity", async () => {
+  let reads = 0;
+  const browser = await startFakeCdpBrowser({
+    commandResult: (command, origin) => {
+      if (command.method !== "Page.getFrameTree") return undefined;
+      reads += 1;
+      return {
+        frameTree: {
+          frame: {
+            ...(reads === 1 ? { id: "main" } : {}),
+            url: `${origin}/app`,
+          },
+          childFrames: [
+            {
+              frame: { id: "child", parentId: "main", url: `${origin}/child` },
+            },
+          ],
+        },
+      };
+    },
+  });
+  const provider = new CdpBrowserProvider();
+  try {
+    const result = await provider.inspectPage(
+      inspectWebPageInputSchema.parse({
+        cdp_endpoint: browser.endpoint,
+        allowed_origins: [browser.allowedOrigin],
+        target_id: "allowed-page",
+        observation_ms: 0,
+      }),
+    );
+    expect(reads).toBeGreaterThanOrEqual(2);
+    expect(result.ok).toBe(false);
+    if (!result.ok)
+      expect(result.error).toMatchObject({ reason: "protocol_error" });
+  } finally {
+    await provider.close();
+    await browser.close();
+  }
+});