From 2d1e5c6d9d5529ebf72d2a7c3fb35156e4a5a2ff Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 9 Sep 2026 01:50:11 +0800 Subject: [PATCH] fix(observation): validate document consistency with bounded identity checks --- .../__tests__/chromium-cdp.test.ts | 18 ++ .../__tests__/frame-graph.test.ts | 18 ++ .../src/browser-driver/chromium-cdp.ts | 10 + .../src/browser-driver/frame-graph.ts | 3 + .../src/tools/__tests__/observation.test.ts | 9 +- apps/extension/src/tools/observation.ts | 12 + apps/extension/src/tools/shared.ts | 1 + .../vom/__tests__/capture-coordinator.test.ts | 304 +++++++++++++++++- .../src/tools/vom/capture-coordinator.ts | 124 ++++++- .../src/tools/vom/document-identity.ts | 71 ++++ apps/extension/src/tools/vom/facts.ts | 19 +- apps/extension/src/tools/vom/normalize.ts | 7 + 12 files changed, 585 insertions(+), 11 deletions(-) create mode 100644 apps/extension/src/tools/vom/document-identity.ts diff --git a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts index fccd4e2..41b6719 100644 --- a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts +++ b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts @@ -29,6 +29,24 @@ function fakeApi() { } describe("ChromiumCdp", () => { + it("changes attachment identity only across real debugger attachments", async () => { + const { api, onEvent, onDetach } = fakeApi(); + const cdp = new ChromiumCdp(api); + expect(cdp.getAttachmentId(4)).toBeUndefined(); + await cdp.ensureAttached(4); + const first = cdp.getAttachmentId(4); + expect(first).toBeTruthy(); + await cdp.ensureAttached(4); + onEvent.fire({ tabId: 4 }, "Target.attachedToTarget", { sessionId: "child" }); + expect(cdp.getAttachmentId(4)).toBe(first); + onDetach.fire({ tabId: 4 }, "target_closed"); + expect(cdp.getAttachmentId(4)).toBeUndefined(); + await cdp.ensureAttached(4); + expect(cdp.getAttachmentId(4)).not.toBe(first); + await cdp.detach(4); + expect(cdp.getAttachmentId(4)).toBeUndefined(); + }); + it("discovers multiple iframe targets and recursively routes nested OOPIF commands", async () => { const { api, onEvent } = fakeApi(); (api.sendCommand as ReturnType).mockImplementation( diff --git a/apps/extension/src/browser-driver/__tests__/frame-graph.test.ts b/apps/extension/src/browser-driver/__tests__/frame-graph.test.ts index b9d4b40..3838d2b 100644 --- a/apps/extension/src/browser-driver/__tests__/frame-graph.test.ts +++ b/apps/extension/src/browser-driver/__tests__/frame-graph.test.ts @@ -2,6 +2,24 @@ import { describe, expect, it } from "vitest"; import { buildFrameGraph, type CdpFrameTreeNode, type CdpFrameTreeSource } from "../frame-graph"; describe("buildFrameGraph", () => { + it("preserves the owning target's loader identity", () => { + const graph = buildFrameGraph([ + { + target: { tabId: 4 }, + tree: { + frame: { id: "root", loaderId: "root-loader" }, + childFrames: [{ frame: { id: "child", loaderId: "stale-placeholder" } }], + }, + }, + { + target: { tabId: 4, sessionId: "child-session" }, + tree: { frame: { id: "child", loaderId: "child-loader" } }, + }, + ]); + expect(graph?.frames.find((frame) => frame.frameId === "root")?.loaderId).toBe("root-loader"); + expect(graph?.frames.find((frame) => frame.frameId === "child")?.loaderId).toBe("child-loader"); + }); + it("keeps sibling frames distinct and routes nested OOPIFs to their child sessions", () => { const graph = buildFrameGraph([ { diff --git a/apps/extension/src/browser-driver/chromium-cdp.ts b/apps/extension/src/browser-driver/chromium-cdp.ts index 8af2abc..ddbf5ee 100644 --- a/apps/extension/src/browser-driver/chromium-cdp.ts +++ b/apps/extension/src/browser-driver/chromium-cdp.ts @@ -164,6 +164,7 @@ async function settleBeforeDeadline(promises: Promise[], deadline: number) export class ChromiumCdp { private readonly api: CdpDebuggerApi; private readonly attachedTabs = new Set(); + private readonly attachmentIds = new Map(); private readonly attachInFlight = new Map>(); private readonly detachInFlight = new Map>(); private readonly tabOwners = new Map>(); @@ -198,6 +199,11 @@ export class ChromiumCdp { this.bindFrameTargetHandler(); } + /** Identity of this actual debugger attachment, independent of frame topology. */ + getAttachmentId(tabId: number): string | undefined { + return this.attachmentIds.get(tabId); + } + /** Attach to `tabId` if we haven't already in this driver. */ async ensureAttached(tabId: number): Promise { // Returning a tab clears the cache before Chrome finishes detaching. @@ -217,6 +223,7 @@ export class ChromiumCdp { await this.enableConsoleDomains(tabId); await this.enableNetworkDomainBestEffort(tabId); this.attachedTabs.add(tabId); + this.attachmentIds.set(tabId, crypto.randomUUID()); await this.enableFrameDiscovery({ tabId }).catch((err) => { console.debug("[bsk cdp] frame discovery unavailable", { tabId, err }); }); @@ -444,6 +451,7 @@ export class ChromiumCdp { this.attachInFlight.delete(tabId); if (!this.attachedTabs.has(tabId)) return; this.attachedTabs.delete(tabId); + this.attachmentIds.delete(tabId); this.clearDialogState(tabId); this.clearConsoleState(tabId); this.clearNetworkState(tabId); @@ -502,6 +510,7 @@ export class ChromiumCdp { this.attachInFlight.clear(); this.tabOwners.clear(); this.attachedTabs.clear(); + this.attachmentIds.clear(); this.dialogBuffers.clear(); this.dialogSequences.clear(); this.consoleBuffers.clear(); @@ -848,6 +857,7 @@ export class ChromiumCdp { const listener = (source: chrome.debugger.Debuggee, _reason: string) => { if (typeof source.tabId === "number") { this.attachedTabs.delete(source.tabId); + this.attachmentIds.delete(source.tabId); this.attachInFlight.delete(source.tabId); this.tabOwners.delete(source.tabId); this.clearDialogState(source.tabId); diff --git a/apps/extension/src/browser-driver/frame-graph.ts b/apps/extension/src/browser-driver/frame-graph.ts index 1dc2ead..2367a5f 100644 --- a/apps/extension/src/browser-driver/frame-graph.ts +++ b/apps/extension/src/browser-driver/frame-graph.ts @@ -8,6 +8,7 @@ export interface CdpFrame { parentFrameId?: string; ownerBackendNodeId?: number; url?: string; + loaderId?: string; target: CdpTarget; } @@ -21,6 +22,7 @@ export interface CdpFrameTreeNode { id: string; parentId?: string; url?: string; + loaderId?: string; }; childFrames?: CdpFrameTreeNode[]; } @@ -48,6 +50,7 @@ function mergeFrame( frameId: node.frame.id, ...(parentFrameId ? { parentFrameId } : {}), ...(url ? { url } : {}), + ...(node.frame.loaderId ? { loaderId: node.frame.loaderId } : {}), target, }); } diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index 07547ca..95f8690 100644 --- a/apps/extension/src/tools/__tests__/observation.test.ts +++ b/apps/extension/src/tools/__tests__/observation.test.ts @@ -3695,7 +3695,8 @@ describe("handleSnapshot", () => { const result = await captureVomObservation(makeFrameAwareDeps().cdp, 4, "https://example.com", { maxTokens, }); - expect(result.text.match(/@warning/g)).toHaveLength(1); + expect(result.text.match(/@warning/g)).toHaveLength(2); + expect(result.text).toContain("document identity unverified"); expect(result.text).toContain("geometry incomplete"); expect(result.text.startsWith("@vom 1\n")).toBe(true); if (maxTokens === undefined) { @@ -3722,7 +3723,8 @@ describe("handleSnapshot", () => { for (const maxTokens of [undefined, 100, 0]) { const result = await captureVomObservation(cdp, 4, "https://example.com", { maxTokens }); expect(result.text).toContain(`@warning observation incomplete: some ${stage} data`); - expect(result.text.match(/@warning/g)).toHaveLength(1); + expect(result.text.match(/@warning/g)).toHaveLength(2); + expect(result.text).toContain("document identity unverified"); expect(result.text.startsWith("@vom 1\n")).toBe(true); if (maxTokens === undefined) { expect(result.frames.map((frame) => frame.frameId)).toEqual(["main", "child"]); @@ -3770,7 +3772,8 @@ describe("handleSnapshot", () => { 4, "https://example.com", ); - expect(result.text).not.toContain("@warning"); + expect(result.text).not.toContain("iframe geometry incomplete"); + expect(result.text).toContain("document identity unverified"); const node = result.matchNodes.find((item) => item.backendNodeId === 22); if (ownerGeometry === "clipped") expect(node?.rect).toBeNull(); else expect(node?.rect).toEqual({ x: 120, y: 130, w: 120, h: 40 }); diff --git a/apps/extension/src/tools/observation.ts b/apps/extension/src/tools/observation.ts index 230b267..ea99ddf 100644 --- a/apps/extension/src/tools/observation.ts +++ b/apps/extension/src/tools/observation.ts @@ -938,6 +938,18 @@ export async function captureVomObservation( notices.push( `@warning observation incomplete: some ${incompleteStages.join(", ")} data is unavailable or omitted.`, ); + if ( + facts.issues.some( + (issue) => issue.stage === "identity" && issue.reason !== "identity-unverified", + ) + ) + notices.push( + "@warning observation incomplete: some documents were omitted because their identity changed or could not be revalidated.", + ); + if (facts.issues.some((issue) => issue.reason === "identity-unverified")) + notices.push( + "@warning document identity unverified: some retained documents could not be checked for changes during capture.", + ); const captureNotice = notices.join("\n"); const rendered = renderVom(decoratedScene, { maxDepth: options.maxDepth, diff --git a/apps/extension/src/tools/shared.ts b/apps/extension/src/tools/shared.ts index 82aed79..7ae9fda 100644 --- a/apps/extension/src/tools/shared.ts +++ b/apps/extension/src/tools/shared.ts @@ -55,6 +55,7 @@ export interface CdpRunner { sendToTarget?(target: CdpTarget, method: string, params?: object): Promise; detach?(tabId: number): Promise; getFrameGraph?(tabId: number): Promise; + getAttachmentId?(tabId: number): string | undefined; ensureAttachedToUrl?(tabId: number, expectedUrl: string | undefined): Promise; trackSessionTab?(sessionId: string, tabId: number): void; releaseSessionTab?(sessionId: string, tabId: number): Promise; diff --git a/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts b/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts index a4aad0b..ad972b5 100644 --- a/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts +++ b/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts @@ -2,39 +2,58 @@ import { describe, expect, it, vi } from "vitest"; import type { CdpFrame, CdpTarget } from "@/browser-driver/frame-graph"; import { cdpTargetKey } from "@/browser-driver/frame-graph"; import { OVERLAY_HOST_MARKER_ATTR } from "@/lib/overlay-bridge"; +import { captureVomObservation } from "../../observation"; import type { CdpRunner } from "../../shared"; import { captureObservationFacts, semanticCapture } from "../capture-coordinator"; import { buildSemanticGraph } from "../semantic-graph/build"; import { REQUESTED_STYLES, type SnapshotReply } from "../snapshot"; function fixture( - options: { frames?: CdpFrame[]; fail?: string; omitDocument?: string; overlay?: string } = {}, + options: { + frames?: CdpFrame[]; + after?: Record< + string, + { loader?: string; element?: number; missing?: boolean; unreadable?: boolean } + >; + fail?: string; + missingIdentity?: boolean; + omitDocument?: string; + attachmentChanged?: boolean; + overlay?: string; + } = {}, ) { const frames = options.frames ?? [ - { frameId: "main", target: { tabId: 4 } }, + { frameId: "main", loaderId: "main-loader", target: { tabId: 4 } }, { frameId: "same", parentFrameId: "main", ownerBackendNodeId: 3, + loaderId: "same-loader", target: { tabId: 4 }, }, { frameId: "nested", parentFrameId: "same", ownerBackendNodeId: 13, + loaderId: "nested-loader", target: { tabId: 4 }, }, { frameId: "remote", parentFrameId: "main", ownerBackendNodeId: 4, + loaderId: "remote-loader", target: { tabId: 4, sessionId: "remote" }, }, ]; const elements = new Map( frames.map((frame, i) => [frame.frameId, frame.target.sessionId ? 1 : i * 10 + 1]), ); + const contexts = new Map(frames.map((frame, i) => [i + 1, frame.frameId])); + const reads = new Map(); const logs: Array<{ target: CdpTarget; method: string; params: Record }> = []; + let snapshots = 0; + const targetCount = new Set(frames.map((frame) => cdpTargetKey(frame.target))).size; const send = vi.fn( async (target: CdpTarget, method: string, params: object = {}): Promise => { logs.push({ target, method, params: params as Record }); @@ -42,6 +61,51 @@ function fixture( if (options.fail === `${target.sessionId ?? "main"}:${method}`) throw new Error("fixture failure"); let result: unknown = {}; + if (method === "Page.createIsolatedWorld") + result = { + executionContextId: frames.findIndex((frame) => frame.frameId === args.frameId) + 1, + }; + if (method === "Runtime.evaluate") + result = { result: { objectId: contexts.get(Number(args.contextId)) } }; + if (method === "DOM.describeNode") { + const id = String(args.objectId); + const count = reads.get(id) ?? 0; + reads.set(id, count + 1); + result = + options.missingIdentity || (count > 0 && options.after?.[id]?.unreadable) + ? {} + : { + node: { + backendNodeId: + count > 0 + ? (options.after?.[id]?.element ?? elements.get(id)) + : elements.get(id), + }, + }; + } + if (method === "Page.getFrameTree") { + const own = frames.filter( + (frame) => + cdpTargetKey(frame.target) === cdpTargetKey(target) && + !options.after?.[frame.frameId]?.missing, + ); + const trees = new Map( + own.map((frame) => [ + frame.frameId, + { + frame: { + id: frame.frameId, + loaderId: options.after?.[frame.frameId]?.loader ?? frame.loaderId, + }, + childFrames: [] as unknown[], + }, + ]), + ); + for (const frame of own) + if (frame.parentFrameId && trees.has(frame.parentFrameId)) + trees.get(frame.parentFrameId)!.childFrames.push(trees.get(frame.frameId)); + result = { frameTree: trees.get(own[0]?.frameId) }; + } if (method === "Page.getLayoutMetrics") result = { visualViewport: { clientWidth: 1000 }, @@ -49,6 +113,7 @@ function fixture( cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 }, }; if (method === "DOMSnapshot.captureSnapshot") { + snapshots++; expect((params as { computedStyles: unknown }).computedStyles).toEqual(REQUESTED_STYLES); result = { strings: [ @@ -137,6 +202,8 @@ function fixture( }, ); const cdp: CdpRunner = { + getAttachmentId: () => + options.attachmentChanged && snapshots === targetCount ? "new-attachment" : "attachment", getFrameGraph: async () => ({ rootFrameId: frames[0].frameId, frames }), send: (tabId, method, params) => (send as NonNullable)({ tabId }, method, params), @@ -181,6 +248,9 @@ describe("captureObservationFacts", () => { expect(logs.filter((call) => call.method === "DOMSnapshot.captureSnapshot")).toHaveLength(2); expect(logs.filter((call) => call.method === "Accessibility.enable")).toHaveLength(2); expect(logs.filter((call) => call.method === "Accessibility.getFullAXTree")).toHaveLength(4); + expect(facts.documents.every((doc) => doc.identity)).toBe(true); + expect(logs.filter((call) => call.method === "DOM.describeNode")).toHaveLength(8); + expect(logs.filter((call) => call.method === "Runtime.releaseObject")).toHaveLength(8); const main = facts.documents.find((doc) => doc.frame.frameId === "main")!; const remote = facts.documents.find((doc) => doc.frame.frameId === "remote")!; expect(main.index.nodes.get(2)).not.toBe(remote.index.nodes.get(2)); @@ -191,6 +261,230 @@ describe("captureObservationFacts", () => { expect(facts.finishedAt).toBeGreaterThanOrEqual(facts.startedAt); }); + it.each([ + { loader: "new-loader" }, + { element: 99 }, + { missing: true }, + ])("isolates changed child identity %j and dependent descendants", async (change) => { + const { cdp } = fixture({ after: { same: change } }); + const facts = await captureObservationFacts(cdp, 4); + expect(facts.documents.map((doc) => doc.frame.frameId)).toEqual(["main", "remote"]); + expect(facts.issues).toContainEqual( + expect.objectContaining({ frameId: "same", reason: "document-changed" }), + ); + }); + + it.each([ + { after: { main: { element: 99 } } }, + { attachmentChanged: true }, + ])("rejects a root replacement or reattachment %j", async (options) => { + await expect(captureObservationFacts(fixture(options).cdp, 4)).rejects.toThrow( + "document identity", + ); + }); + + it.each([ + "main", + "same", + ])("does not publish a previously identified %s document after identity reads fail", async (frameId) => { + const { cdp } = fixture({ after: { [frameId]: { unreadable: true } } }); + if (frameId === "main") + await expect(captureObservationFacts(cdp, 4)).rejects.toThrow("document identity"); + else { + const facts = await captureObservationFacts(cdp, 4); + expect(facts.documents.map((doc) => doc.frame.frameId)).toEqual(["main", "remote"]); + expect(facts.issues).toContainEqual( + expect.objectContaining({ frameId, reason: "identity-unavailable" }), + ); + } + }); + + it("keeps newly discovered snapshot documents unverified without reading a synthetic before identity", async () => { + const { cdp, logs } = fixture(); + const originalGraph = await cdp.getFrameGraph!(4); + cdp.getFrameGraph = async () => ({ + ...originalGraph, + frames: originalGraph.frames.slice(0, 1), + }); + const facts = await captureObservationFacts(cdp, 4); + expect(facts.documents.map((doc) => doc.frame.frameId)).toEqual(["main", "same", "nested"]); + expect(facts.documents[0].identity).toBeDefined(); + for (const doc of facts.documents.slice(1)) { + expect(doc.identity).toBeUndefined(); + expect(facts.issues).toContainEqual( + expect.objectContaining({ frameId: doc.frame.frameId, reason: "identity-unverified" }), + ); + } + expect( + logs.filter((call) => call.method === "DOM.describeNode").map((call) => call.params.objectId), + ).toEqual(["main", "main"]); + }); + + it.each([ + "frame-id", + "snapshot-element", + ])("rejects root replacement evidenced by %s even if the later graph is stale", async (mode) => { + const { cdp } = fixture(); + const original = cdp.sendToTarget!; + cdp.sendToTarget = async (target, method, params) => { + const result = await original(target, method, params); + if (method === "DOMSnapshot.captureSnapshot" && !target.sessionId) { + const snapshot = result as { + documents: { frameId: string; nodes: { backendNodeId: number[] } }[]; + }; + if (mode === "frame-id") snapshot.documents[0].frameId = "replacement"; + else snapshot.documents[0].nodes.backendNodeId[2] = 999; + } + return result as never; + }; + cdp.send = (tabId, method, params) => cdp.sendToTarget!({ tabId }, method, params); + await expect(captureObservationFacts(cdp, 4)).rejects.toThrow("document identity"); + }); + + it("starts after-identity reads only after every target finishes and propagates cancellation during identity cleanup", async () => { + const { cdp, logs } = fixture(); + const controller = new AbortController(); + const original = cdp.sendToTarget!; + let identityCleanup = 0; + cdp.sendToTarget = async (target, method, params) => { + if (method === "Page.getFrameTree") { + expect(logs.filter((call) => call.method === "Accessibility.getFullAXTree")).toHaveLength( + 4, + ); + } + const result = await original(target, method, params); + if ( + method === "Runtime.releaseObject" && + (params as { objectId?: string })?.objectId === "remote" + ) { + if (++identityCleanup === 2) controller.abort(); + } + return result as never; + }; + cdp.send = (tabId, method, params) => cdp.sendToTarget!({ tabId }, method, params); + await expect(captureObservationFacts(cdp, 4, controller.signal)).rejects.toMatchObject({ + name: "AbortError", + }); + expect(identityCleanup).toBe(2); + }); + + it.each([ + 1, 10, 100, + ])("bounds identity reads for %i same-target documents across both phases", async (count) => { + const frames: CdpFrame[] = Array.from({ length: count }, (_, i) => ({ + frameId: `frame-${i}`, + loaderId: `loader-${i}`, + target: { tabId: 4 }, + ...(i ? { parentFrameId: "frame-0", ownerBackendNodeId: 90000 + i } : {}), + })); + const { cdp, logs } = fixture({ frames }); + const original = cdp.sendToTarget!; + let active = 0; + let peak = 0; + let released = 0; + cdp.sendToTarget = async (target, method, params) => { + if (method === "Page.createIsolatedWorld") { + peak = Math.max(peak, ++active); + await new Promise((resolve) => setTimeout(resolve, 1)); + } + if (method === "DOMSnapshot.captureSnapshot") { + expect(active).toBe(0); + expect(released).toBe(count); + } + if (method === "Page.getFrameTree") { + expect(active).toBe(0); + expect(logs.filter((call) => call.method === "Accessibility.getFullAXTree")).toHaveLength( + count, + ); + } + const result = await original(target, method, params); + if (method === "Runtime.releaseObject") { + active--; + released++; + } + return result as never; + }; + cdp.send = (tabId, method, params) => cdp.sendToTarget!({ tabId }, method, params); + const facts = await captureObservationFacts(cdp, 4); + expect(facts.documents.every((doc) => doc.identity)).toBe(true); + expect(peak).toBe(Math.min(count, 4)); + expect(active).toBe(0); + expect(released).toBe(count * 2); + for (const method of [ + "Page.createIsolatedWorld", + "Runtime.evaluate", + "DOM.describeNode", + "Runtime.releaseObject", + ]) + expect(logs.filter((call) => call.method === method)).toHaveLength(count * 2); + expect(logs.filter((call) => call.method === "DOMSnapshot.captureSnapshot")).toHaveLength(1); + expect(logs.filter((call) => call.method === "Page.getFrameTree")).toHaveLength(1); + }); + + it.each([ + "before", + "after", + ])("joins identity cleanup on %s cancellation without starting queued frames", async (phase) => { + const frames: CdpFrame[] = Array.from({ length: 5 }, (_, i) => ({ + frameId: `frame-${i}`, + loaderId: `loader-${i}`, + target: { tabId: 4 }, + ...(i ? { parentFrameId: "frame-0", ownerBackendNodeId: 90000 + i } : {}), + })); + const { cdp, logs } = fixture({ frames }); + const original = cdp.sendToTarget!; + const controller = new AbortController(); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + let ready!: () => void; + const blocked = new Promise((resolve) => { + ready = resolve; + }); + let held = 0; + let after = false; + cdp.sendToTarget = async (target, method, params) => { + if (method === "Page.getFrameTree") after = true; + const result = await original(target, method, params); + if (method === "Runtime.releaseObject" && after === (phase === "after")) { + if (++held === 4) ready(); + await gate; + } + return result as never; + }; + cdp.send = (tabId, method, params) => cdp.sendToTarget!({ tabId }, method, params); + let settled = false; + const pending = captureObservationFacts(cdp, 4, controller.signal).finally(() => { + settled = true; + }); + const rejected = expect(pending).rejects.toMatchObject({ name: "AbortError" }); + await blocked; + controller.abort(); + await Promise.resolve(); + expect(settled).toBe(false); + release(); + await rejected; + expect(held).toBe(4); + expect(logs.filter((call) => call.method === "Page.createIsolatedWorld")).toHaveLength( + phase === "after" ? 9 : 4, + ); + }); + + it.each([ + [{ after: { same: { element: 99 } } }, "omitted", false], + [{ after: { same: { unreadable: true } } }, "omitted", false], + [{ missingIdentity: true }, "document identity unverified", true], + [{}, "", true], + ] as const)("publishes identity integrity without silently losing content %j", async (options, notice, retained) => { + const { cdp } = fixture(options); + const result = await captureVomObservation(cdp, 4, "https://example.com"); + expect(result.frames.some((frame) => frame.frameId === "same")).toBe(retained); + if (notice) expect(result.text).toContain(notice); + else expect(result.text).not.toContain("identity"); + if (!retained) expect(result.text).not.toContain("document identity unverified"); + }); + it("does not retry failed or missing documents and retains valid AX-only semantics", async () => { const { cdp, logs } = fixture({ fail: "remote:DOMSnapshot.captureSnapshot", @@ -209,6 +503,12 @@ describe("captureObservationFacts", () => { ); }); + it("keeps missing identity explicit without manufacturing a verified document", async () => { + const facts = await captureObservationFacts(fixture({ missingIdentity: true }).cdp, 4); + expect(facts.documents.every((doc) => !doc.identity)).toBe(true); + expect(facts.issues.filter((issue) => issue.reason === "identity-unverified")).toHaveLength(4); + }); + it("excludes overlay AX in its own document without excluding equal IDs elsewhere", async () => { const facts = await captureObservationFacts(fixture({ overlay: "remote" }).cdp, 4); const { documents } = semanticCapture(facts); diff --git a/apps/extension/src/tools/vom/capture-coordinator.ts b/apps/extension/src/tools/vom/capture-coordinator.ts index 7745ed2..c859c3f 100644 --- a/apps/extension/src/tools/vom/capture-coordinator.ts +++ b/apps/extension/src/tools/vom/capture-coordinator.ts @@ -1,17 +1,20 @@ import { + buildFrameGraph, type CdpFrame, type CdpFrameGraph, + type CdpFrameTreeNode, type CdpTarget, cdpTargetKey, } from "@/browser-driver/frame-graph"; import { cssViewport, GeometryContext } from "../geometry/frame-context"; -import { type CdpRunner, cdpRunnerForTarget } from "../shared"; +import { type CdpRunner, cdpRunnerForTarget, sendToCdpTarget } from "../shared"; import { collectOverlayExcludedBackendIds } from "./capture"; import { isAbortError as isCaptureAbort, throwIfAborted as throwCaptureAborted, } from "./capture-abort"; -import type { CapturedSceneInput } from "./facts"; +import { readDocumentIdentity, sameDocument } from "./document-identity"; +import type { CapturedSceneInput, DocumentIdentity } from "./facts"; import { buildDocumentIndex, type CaptureIssue, type ObservationFacts } from "./facts"; import { enrichFormControlStates } from "./form-capture"; import { @@ -26,6 +29,7 @@ import { describeSnapshotFrames, REQUESTED_STYLES, type SnapshotReply } from "./ interface TargetBatch { target: CdpTarget; frames: CdpFrame[]; + before: Map; documents: NormalizedFrameDocument[]; ax: FrameAxBatch[]; fallbackExcluded: Set; @@ -88,6 +92,7 @@ export async function captureObservationFacts( group = { target: frame.target, frames: [], + before: new Map(), documents: [], ax: [], fallbackExcluded: new Set(), @@ -104,6 +109,7 @@ export async function captureObservationFacts( groups.set(cdpTargetKey({ tabId }), { target: { tabId }, frames: [], + before: new Map(), documents: [], ax: [], fallbackExcluded: new Set(), @@ -111,6 +117,16 @@ export async function captureObservationFacts( const batches = [...groups.values()]; const unavailable = (batch: TargetBatch, stage: CaptureIssue["stage"], frameId?: string) => issues.push({ target: batch.target, frameId, stage, reason: "capture-unavailable" }); + // All before reads finish before any target starts collecting. The bound is + // shared across frames, including frames in the same target. + await collectTasks( + batches.flatMap((batch) => batch.frames.map((frame) => ({ batch, frame }))), + async ({ batch, frame }) => { + const identity = await readDocumentIdentity(cdp, frame, signal); + if (identity) batch.before.set(frame.frameId, identity); + }, + signal, + ); let firstFailure: unknown; const axReady = new Set>(); await collectTasks( @@ -246,6 +262,100 @@ export async function captureObservationFacts( const rootBatch = batches.find((batch) => !batch.target.sessionId); const rootFrameId = rootBatch?.rootFrameId ?? graph?.rootFrameId ?? rootBatch?.frames[0]?.frameId ?? "root"; + const ownershipInvalid = new Set(invalid); + const identities = new Map(); + const rootBefore = graph && rootBatch?.before.get(graph.rootFrameId); + if (rootBefore && rootFrameId !== rootBefore.frameId) { + invalid.add(rootFrameId); + issues.push({ + target: rootBefore.target, + frameId: rootFrameId, + stage: "identity", + reason: "document-changed", + }); + } + const selected = new Map(currentFrames.map((frame) => [frame.frameId, frame])); + const freshFrames = new Map>(); + await collectTasks( + batches.filter((batch) => batch.before.size > 0), + async (batch) => { + try { + throwCaptureAborted(signal); + const reply = await sendToCdpTarget<{ frameTree?: CdpFrameTreeNode }>( + cdp, + batch.target, + "Page.getFrameTree", + {}, + ); + const tree = reply.frameTree + ? buildFrameGraph([{ target: batch.target, tree: reply.frameTree }]) + : null; + if (tree) + freshFrames.set( + cdpTargetKey(batch.target), + new Map(tree.frames.map((frame) => [frame.frameId, frame])), + ); + } catch (error) { + throwCaptureAborted(signal); + if (isCaptureAbort(error)) throw error; + } + }, + signal, + ); + const checks = batches.flatMap((batch) => { + const docs = new Map(batch.documents.map((doc) => [doc.frame.frameId, doc])); + return batch.frames + .filter( + (frame) => + !invalid.has(frame.frameId) && + cdpTargetKey(selected.get(frame.frameId)!.target) === cdpTargetKey(batch.target), + ) + .map((frame) => ({ batch, frame, doc: docs.get(frame.frameId) })); + }); + await collectTasks( + checks, + async ({ batch, frame, doc }) => { + const fresh = freshFrames.get(cdpTargetKey(batch.target)); + throwCaptureAborted(signal); + const before = batch.before.get(frame.frameId); + const current = fresh?.get(frame.frameId); + const after = + before && current ? await readDocumentIdentity(cdp, current, signal) : undefined; + const snapshotMismatch = + before && + doc?.documentElementBackendNodeId !== undefined && + before.documentElementBackendNodeId !== doc.documentElementBackendNodeId; + const changed = + before && + (snapshotMismatch || + (fresh && !current) || + (after && !sameDocument(before, after)) || + cdp.getAttachmentId?.(tabId) !== before.attachmentId); + if (changed || (before && !after)) { + invalid.add(frame.frameId); + issues.push({ + target: frame.target, + frameId: frame.frameId, + stage: "identity", + reason: changed ? "document-changed" : "identity-unavailable", + }); + } else if ( + before && + after && + (!doc || doc.documentElementBackendNodeId === before.documentElementBackendNodeId) + ) { + identities.set(frame.frameId, after); + } else { + issues.push({ + target: frame.target, + frameId: frame.frameId, + stage: "identity", + reason: "identity-unverified", + }); + } + }, + signal, + ); const children = new Map(); for (const frame of currentFrames) { if (!frame.parentFrameId) continue; @@ -263,7 +373,7 @@ export async function captureObservationFacts( } } for (const frame of currentFrames) { - if (invalid.has(frame.frameId)) + if (ownershipInvalid.has(frame.frameId)) issues.push({ target: frame.target, frameId: frame.frameId, @@ -271,7 +381,12 @@ export async function captureObservationFacts( reason: "frame-ownership-unresolved", }); } - if (invalid.has(rootFrameId)) throw new Error("observation root document ownership is ambiguous"); + if (invalid.has(rootFrameId)) + throw new Error( + ownershipInvalid.has(rootFrameId) + ? "observation root document ownership is ambiguous" + : "observation document identity changed or could not be verified; observe again", + ); const frames = currentFrames.filter((frame) => !invalid.has(frame.frameId)); const frameById = new Map(frames.map((frame) => [frame.frameId, frame])); const belongs = (frame: CdpFrame) => { @@ -319,6 +434,7 @@ export async function captureObservationFacts( } = document; facts.push({ frame, + identity: identities.get(document.frameId), index: fallback?.size ? { ...index, excludedBackendNodeIds: fallback } : index, domNodes, axNodes, diff --git a/apps/extension/src/tools/vom/document-identity.ts b/apps/extension/src/tools/vom/document-identity.ts new file mode 100644 index 0000000..90b6391 --- /dev/null +++ b/apps/extension/src/tools/vom/document-identity.ts @@ -0,0 +1,71 @@ +import type { CdpFrame } from "@/browser-driver/frame-graph"; +import type { CdpRunner } from "../shared"; +import { sendToCdpTarget } from "../shared"; +import { + isAbortError as isCaptureAbort, + throwIfAborted as throwCaptureAborted, +} from "./capture-abort"; +import type { DocumentIdentity } from "./facts"; + +/** Fresh, frame-scoped identity read. An isolated world supplies the owning + * document without querying each node or installing a page observer. */ +export async function readDocumentIdentity( + cdp: CdpRunner, + frame: CdpFrame, + signal?: AbortSignal, +): Promise { + const attachmentId = cdp.getAttachmentId?.(frame.target.tabId); + if (!attachmentId || !frame.loaderId) return undefined; + let objectId: string | undefined; + const send = (method: string, params: object) => { + throwCaptureAborted(signal); + return sendToCdpTarget(cdp, frame.target, method, params); + }; + try { + const world = await send<{ executionContextId: number }>("Page.createIsolatedWorld", { + frameId: frame.frameId, + worldName: "bsk-document-identity", + }); + const element = await send<{ result?: { objectId?: string } }>("Runtime.evaluate", { + expression: "document.documentElement", + contextId: world.executionContextId, + }); + objectId = element.result?.objectId; + if (!objectId) return undefined; + const reply = await send<{ node?: { backendNodeId?: number } }>("DOM.describeNode", { + objectId, + depth: 0, + }); + const backendNodeId = reply.node?.backendNodeId; + if (!backendNodeId || cdp.getAttachmentId?.(frame.target.tabId) !== attachmentId) + return undefined; + return { + attachmentId, + target: frame.target, + frameId: frame.frameId, + loaderId: frame.loaderId, + documentElementBackendNodeId: backendNodeId, + }; + } catch (error) { + throwCaptureAborted(signal); + if (isCaptureAbort(error)) throw error; + return undefined; + } finally { + if (objectId) + await sendToCdpTarget(cdp, frame.target, "Runtime.releaseObject", { objectId }).catch( + () => {}, + ); + throwCaptureAborted(signal); + } +} + +export function sameDocument(a: DocumentIdentity, b: DocumentIdentity): boolean { + return ( + a.attachmentId === b.attachmentId && + a.target.tabId === b.target.tabId && + a.target.sessionId === b.target.sessionId && + a.frameId === b.frameId && + a.loaderId === b.loaderId && + a.documentElementBackendNodeId === b.documentElementBackendNodeId + ); +} diff --git a/apps/extension/src/tools/vom/facts.ts b/apps/extension/src/tools/vom/facts.ts index c6cb1ca..36e4a13 100644 --- a/apps/extension/src/tools/vom/facts.ts +++ b/apps/extension/src/tools/vom/facts.ts @@ -97,6 +97,14 @@ export async function buildDocumentIndex( return { nodes, excludedBackendNodeIds }; } +export interface DocumentIdentity { + attachmentId: string; + target: CdpTarget; + frameId: string; + loaderId: string; + documentElementBackendNodeId: number; +} + /** Narrow input shared by the existing semantic scene/hover consumers. */ export interface CapturedSceneInput { nodes: CapturedNode[]; @@ -110,12 +118,19 @@ export interface CaptureIssue { projectionIssue?: FrameProjectionIssue; target: CdpTarget; frameId?: string; - stage: "dom" | "ax" | "ownership" | "geometry" | "forms"; - reason: "capture-unavailable" | "frame-ownership-unresolved" | "geometry-unavailable"; + stage: "dom" | "ax" | "identity" | "ownership" | "geometry" | "forms"; + reason: + | "document-changed" + | "identity-unavailable" + | "identity-unverified" + | "capture-unavailable" + | "frame-ownership-unresolved" + | "geometry-unavailable"; } export interface DocumentFacts { readonly frame: CdpFrame; + readonly identity?: DocumentIdentity; readonly index: DocumentIndex; readonly domNodes: CapturedNode[]; readonly axNodes: T[]; diff --git a/apps/extension/src/tools/vom/normalize.ts b/apps/extension/src/tools/vom/normalize.ts index 445c5f5..6f162dc 100644 --- a/apps/extension/src/tools/vom/normalize.ts +++ b/apps/extension/src/tools/vom/normalize.ts @@ -38,6 +38,7 @@ export interface FrameContext { export interface NormalizedDocument { nodes: NodeFacts[]; index: DocumentIndex; + documentElementBackendNodeId?: number; } /** Interpret one document using a supplied projection. No live reads or frame scheduling. */ @@ -98,6 +99,12 @@ export async function normalizeDocument( (node) => !node.tag.startsWith("#") && !index.excludedBackendNodeIds.has(node.backendNodeId), ), index, + documentElementBackendNodeId: nodes.find( + (node) => + node.nodeType === 1 && + node.parentBackendNodeId !== null && + index.nodes.get(node.parentBackendNodeId)?.nodeType === 9, + )?.backendNodeId, }; }