diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 223e98018..6252d3fcb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -354,6 +354,18 @@ jobs: BSK_VIDEO_BSK: ../../target/debug/bsk run: pnpm exec vitest run src/video/*.browser.test.ts --reporter=default --reporter=json --outputFile.json="$BSK_VIDEO_ARTIFACTS/results.json" + - name: Build extension for the isolated cooperation daemon + env: + BSK_DAEMON_WS_URL: ws://127.0.0.1:52917 + run: pnpm --filter @browser-skill/extension build + + - name: Validate failed-frame synchronization and CLI recovery + env: + BSK_COOP_CHROME: ${{ env.BSK_VIDEO_CHROME }} + BSK_COOP_FRAMES_ONLY: "1" + BSK_COOP_ARTIFACTS: ${{ env.BSK_VIDEO_ARTIFACTS }}/cooperation + run: node evals/browser/cases/regression/cooperation/run.mjs + - name: Upload video regression evidence if: always() uses: actions/upload-artifact@v7 diff --git a/CHANGELOG.md b/CHANGELOG.md index 70772fa2b..ddd78fafd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). Starting from 0.2.0, CLI / Extension / DSH Plugin share the same version number. +## [Unreleased] + +### Fixed + +- Shared-control synchronization skips failed child frames and inactive documents, + reporting them in `frame_gaps` with only HTTP(S) origins and paths or other URL schemes, + and repairs missing interaction collectors immediately once per current + document. Active frames without coverage still block actions with + `frame_unobservable`; a failed main page reports `page_load_failed`. +- `cooperate sync --settle` continues bounded retries when a wait reports unavailable + coverage, returning the last sync error if all attempts fail instead of a + successful `not_ready` reply or an invented permanent frame failure. Temporary + human control retains the existing `not_ready` behavior, including explicit + holds during navigation. Status and wait report + `control_policy: null` until the first successful policy negotiation. + ## [0.4.1] - 2026-11-02 ### Changed diff --git a/apps/extension/src/content/__tests__/cooperation-capture.browser.test.ts b/apps/extension/src/content/__tests__/cooperation-capture.browser.test.ts index 116477ad7..10fb062af 100644 --- a/apps/extension/src/content/__tests__/cooperation-capture.browser.test.ts +++ b/apps/extension/src/content/__tests__/cooperation-capture.browser.test.ts @@ -1,12 +1,13 @@ // @vitest-environment node -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { createServer } from "node:http"; import { tmpdir } from "node:os"; import path from "node:path"; -import { build } from "esbuild"; import { describe, expect, it, vi } from "vitest"; import { collectCooperationFrames } from "@/cooperation/browser-bridge"; -import { COOPERATION_ARM, COOPERATION_READ } from "@/cooperation/types"; +import { CooperationController } from "@/cooperation/controller"; +import { COOPERATION_ARM, COOPERATION_READ, type FrameState } from "@/cooperation/types"; +import { SessionManager } from "@/session-manager/manager"; type Send = >( method: string, @@ -14,38 +15,49 @@ type Send = >( sessionId?: string, ) => Promise; -describe.skipIf(!process.env.BSK_COOP_CHROME)("HTTP interaction capture", () => { - it("collects top-level and child frames on secure and insecure HTTP origins", async () => { +describe.skipIf(!process.env.BSK_COOP_CHROME)("frame interaction coverage", () => { + it("collects HTTP frames, skips load failures and repairs old tabs while preserving human control", async () => { const directory = await mkdtemp(path.join(tmpdir(), "bsk-http-cooperation-")); + const brokenFrame = await readFile( + new URL( + "../../../../../evals/browser/cases/regression/cooperation/broken-frame.html", + import.meta.url, + ), + ); const server = createServer((request, response) => { response.setHeader("Content-Type", "text/html"); response.end( - request.url === "/frame" - ? '' - : '', + request.url === "/broken" + ? brokenFrame + : request.url === "/late-frame" + ? '' + : request.url === "/frame" || request.url === "/delayed" + ? '' + : '', ); }); try { - const entry = path.join(directory, "content.js"); + // Use the shipped WXT entrypoint, including its invalidation/cleanup. + // Build the extension before running this browser regression. + const cooperationScript = await readFile( + path.resolve("dist/chrome-mv3/content-scripts/cooperation.js"), + "utf8", + ); await writeFile( - entry, - `import { attachCooperationCapture } from '@/content/cooperation-capture'; -chrome.runtime.onMessage.addListener((message, _sender, reply) => { + path.join(directory, "content.bundle.js"), + `if (location.pathname !== '/delayed') chrome.runtime.onMessage.addListener((message, _sender, reply) => { if (message.kind === 'fixture.environment') reply({ secure: isSecureContext, randomUUID: typeof crypto.randomUUID, getRandomValues: typeof crypto.getRandomValues, }); }); -attachCooperationCapture();`, +const start = () => { ${cooperationScript} }; +if (location.pathname === '/delayed') globalThis.fixtureStartCapture = start; +else start();`, ); - await build({ - entryPoints: [entry], - outfile: path.join(directory, "content.bundle.js"), - bundle: true, - format: "iife", - alias: { "@": path.resolve("src") }, - }); + await mkdir(path.join(directory, "content-scripts")); + await writeFile(path.join(directory, "content-scripts/cooperation.js"), cooperationScript); const matches = ["http://127.0.0.1/*", "http://intranet.test/*"]; await writeFile( path.join(directory, "manifest.json"), @@ -54,7 +66,7 @@ attachCooperationCapture();`, name: "HTTP interaction regression", version: "1.0", background: { service_worker: "http-cooperation-worker.js" }, - permissions: ["tabs", "webNavigation"], + permissions: ["tabs", "webNavigation", "scripting"], host_permissions: matches, content_scripts: [ { matches, js: ["content.bundle.js"], run_at: "document_start", all_frames: true }, @@ -63,7 +75,12 @@ attachCooperationCapture();`, ); await writeFile( path.join(directory, "http-cooperation-worker.js"), - "chrome.runtime.onInstalled.addListener(() => {}); globalThis.httpCooperationReady = true;", + `chrome.runtime.onInstalled.addListener(() => {}); +globalThis.frameUpdates = []; +chrome.runtime.onMessage.addListener(message => { + if (message.kind === 'cooperation.frame') globalThis.frameUpdates.push(message.state); +}); +globalThis.httpCooperationReady = true;`, ); await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); const port = (server.address() as { port: number }).port; @@ -77,6 +94,7 @@ attachCooperationCapture();`, { executable: process.env.BSK_COOP_CHROME, extensionPath: directory, + extensionViaCdp: true, deviceScale: 1, zoom: 1, // Loopback URLs are secure contexts. Map a non-localhost name to our @@ -105,22 +123,30 @@ attachCooperationCapture();`, { timeout: 10000 }, ) .toBeDefined(); - const { sessionId } = await send<{ sessionId: string }>("Target.attachToTarget", { + let { sessionId } = await send<{ sessionId: string }>("Target.attachToTarget", { targetId: worker!.targetId, flatten: true, }); const evaluate = async (expression: string): Promise => { - const reply = await send<{ result: { value: T }; exceptionDetails?: unknown }>( + const reply = await send<{ + result: { value: T }; + exceptionDetails?: { exception?: { description?: string }; text: string }; + }>( "Runtime.evaluate", { expression, returnByValue: true, awaitPromise: true }, sessionId, ); - expect(reply.exceptionDetails).toBeUndefined(); + if (reply.exceptionDetails) + throw new Error( + reply.exceptionDetails.exception?.description ?? reply.exceptionDetails.text, + ); return reply.result.value; }; - await expect - .poll(() => evaluate("globalThis.httpCooperationReady"), { timeout: 10000 }) - .toBe(true); + const waitForWorker = () => + expect + .poll(() => evaluate("globalThis.httpCooperationReady"), { timeout: 10000 }) + .toBe(true); + await waitForWorker(); expect(await evaluate("chrome.runtime.getManifest().name")).toBe( "HTTP interaction regression", ); @@ -134,7 +160,18 @@ attachCooperationCapture();`, evaluate( `chrome.tabs.sendMessage(${tabId},${JSON.stringify(message)},${JSON.stringify(options)})`, ); - vi.stubGlobal("chrome", { webNavigation: { getAllFrames }, tabs: { sendMessage } }); + const executeScript = vi.fn((options: object) => + evaluate(`chrome.scripting.executeScript(${JSON.stringify(options)})`), + ); + vi.stubGlobal("chrome", { + webNavigation: { getAllFrames }, + tabs: { + sendMessage, + get: (tabId: number) => evaluate(`chrome.tabs.get(${tabId})`), + query: (query: object) => evaluate(`chrome.tabs.query(${JSON.stringify(query)})`), + }, + scripting: { executeScript }, + }); for (const host of ["127.0.0.1", "intranet.test"]) { const url = `http://${host}:${port}/`; const { id: tabId } = await evaluate<{ id: number }>( @@ -160,7 +197,11 @@ attachCooperationCapture();`, getRandomValues: "function", }); } - const states = await collectCooperationFrames(tabId, COOPERATION_READ, "lease"); + const { frames: states } = await collectCooperationFrames( + tabId, + COOPERATION_READ, + "lease", + ); expect(states.size).toBe(2); expect(states.get(0)?.fields).toEqual([ expect.objectContaining({ label: "Page", value: "top" }), @@ -170,11 +211,16 @@ attachCooperationCapture();`, expect.objectContaining({ label: "Frame", value: "child" }), ]); expect(new Set([...states.values()].map((state) => state.document)).size).toBe(2); - const armed = await collectCooperationFrames(tabId, COOPERATION_ARM, "lease", { - type: "pointerdown", - x: 10, - y: 20, - }); + const { frames: armed } = await collectCooperationFrames( + tabId, + COOPERATION_ARM, + "lease", + { + type: "pointerdown", + x: 10, + y: 20, + }, + ); for (const [frameId, state] of states) { expect(armed.get(frameId)).toMatchObject({ document: state.document, @@ -183,6 +229,179 @@ attachCooperationCapture();`, } await evaluate(`chrome.tabs.remove(${tabId})`); } + + const createTab = async (route: string) => { + const tab = await evaluate<{ id: number; windowId: number }>( + `chrome.tabs.create({url:${JSON.stringify(`http://127.0.0.1:${port}${route}`)}})`, + ); + await expect + .poll(() => evaluate(`chrome.tabs.get(${tab.id})`), { timeout: 10000 }) + .toMatchObject({ status: "complete" }); + return tab; + }; + const clickFrame = async (route: string) => { + const { targetInfos } = await send<{ + targetInfos: { targetId: string; url: string }[]; + }>("Target.getTargets"); + const target = targetInfos.find( + (target) => target.url === `http://127.0.0.1:${port}${route}`, + )!; + const { sessionId: human } = await send<{ sessionId: string }>( + "Target.attachToTarget", + { targetId: target.targetId, flatten: true }, + ); + try { + await send("Page.bringToFront", {}, human); + const point = await send<{ result: { value: { x: number; y: number } } }>( + "Runtime.evaluate", + { + expression: `(() => { const f = document.querySelector('iframe'); const a = f.getBoundingClientRect(); const b = f.contentDocument.querySelector('input').getBoundingClientRect(); return { x: a.x + f.clientLeft + b.x + 10, y: a.y + f.clientTop + b.y + 10 }; })()`, + returnByValue: true, + }, + human, + ); + for (const type of ["mousePressed", "mouseReleased"]) + await send( + "Input.dispatchMouseEvent", + { type, button: "left", clickCount: 1, ...point.result.value }, + human, + ); + } finally { + await send("Target.detachFromTarget", { sessionId: human }); + } + }; + const broken = await createTab("/broken"); + const browserFrames = await getAllFrames({ tabId: broken.id }); + const failed = browserFrames.find((frame) => frame.frameId !== 0)!; + // Confirm Chrome's real metadata, not just a mocked error flag. + expect(failed.errorOccurred).toBe(true); + const collected = await collectCooperationFrames(broken.id, COOPERATION_READ, "broken"); + expect([...collected.frames.keys()]).toEqual([0]); + expect(collected.gaps).toEqual([ + { frame_id: failed.frameId, url: failed.url, reason: "load_error" }, + ]); + expect(executeScript).not.toHaveBeenCalled(); + await evaluate(`chrome.tabs.remove(${broken.id})`); + + const delayed = await createTab("/late-frame"); + const child = (await getAllFrames({ tabId: delayed.id })).find( + (frame) => frame.frameId !== 0, + )!; + await expect( + sendMessage( + delayed.id, + { kind: COOPERATION_READ, lease: "late" }, + { documentId: child.documentId }, + ), + ).rejects.toThrow("Receiving end does not exist"); + const repaired = await collectCooperationFrames(delayed.id, COOPERATION_READ, "late"); + expect(executeScript).toHaveBeenCalledOnce(); + const repairedDocument = repaired.frames.get(child.frameId)!.document; + // Let the delayed declared script run after the repair. WXT must + // dispose the previous collector rather than leave both listening. + await evaluate( + `chrome.scripting.executeScript({target:{tabId:${delayed.id},documentIds:[${JSON.stringify(child.documentId)}]},func:()=>globalThis.fixtureStartCapture()})`, + ); + const declared = await collectCooperationFrames(delayed.id, COOPERATION_READ, "late"); + const declaredDocument = declared.frames.get(child.frameId)!.document; + expect(declaredDocument).not.toBe(repairedDocument); + await evaluate("globalThis.frameUpdates = []"); + await clickFrame("/late-frame"); + const afterInput = await collectCooperationFrames(delayed.id, COOPERATION_READ, "late"); + expect(afterInput.frames.get(child.frameId)).toMatchObject({ + document: declaredDocument, + human: 1, + }); + const updates = await evaluate( + "globalThis.frameUpdates.filter(state => state.human > 0)", + ); + expect(updates.length).toBeGreaterThan(0); + expect(new Set(updates.map((state) => state.document))).toEqual( + new Set([declaredDocument]), + ); + expect(updates.every((state) => state.human === 1)).toBe(true); + expect(executeScript).toHaveBeenCalledOnce(); + await evaluate(`chrome.tabs.remove(${delayed.id})`); + executeScript.mockClear(); + + const oldTab = await createTab("/"); + const beforeReload = await getAllFrames({ tabId: oldTab.id }); + const oldWorker = worker!.targetId; + await send("Extensions.loadUnpacked", { path: directory }); + await expect + .poll( + async () => { + const { targetInfos } = await send<{ + targetInfos: { targetId: string; type: string; url: string }[]; + }>("Target.getTargets"); + worker = targetInfos.find( + (target) => + target.type === "service_worker" && + target.targetId !== oldWorker && + target.url.endsWith("/http-cooperation-worker.js"), + ); + return worker; + }, + { timeout: 10000 }, + ) + .toBeDefined(); + ({ sessionId } = await send<{ sessionId: string }>("Target.attachToTarget", { + targetId: worker!.targetId, + flatten: true, + })); + await waitForWorker(); + expect(await getAllFrames({ tabId: oldTab.id })).toEqual(beforeReload); + for (const frame of beforeReload) + await expect( + sendMessage( + oldTab.id, + { kind: COOPERATION_READ, lease: "old" }, + { documentId: frame.documentId }, + ), + ).rejects.toThrow("Receiving end does not exist"); + + const sessions = new SessionManager({ + agentWindow: { + create: async () => ({ windowId: oldTab.windowId, initialTabIds: [oldTab.id] }), + remove: async () => {}, + ensureActiveTab: async () => oldTab.id, + }, + }); + await sessions.start("repair"); + const controller = new CooperationController({ + sessions, + readFrames: (tabId, input, lease, repairs) => + collectCooperationFrames( + tabId, + input ? COOPERATION_ARM : COOPERATION_READ, + lease, + input, + repairs, + ), + resumeFrames: async () => {}, + changed: () => {}, + }); + controller.startSession("repair"); + await controller.observeTab(oldTab.id); + expect(controller.view(oldTab.id)?.mode).toBe("syncing"); + expect(executeScript).toHaveBeenCalledTimes(2); + expect( + executeScript.mock.calls + .map( + ([options]) => + (options as { target: { documentIds: string[] } }).target.documentIds[0], + ) + .sort(), + ).toEqual(beforeReload.map((frame) => frame.documentId).sort()); + const guard = controller.controlGuard("repair", oldTab.id); + expect(guard).not.toThrow(); + + await clickFrame("/"); + await controller.observeTab(oldTab.id); + expect(controller.view(oldTab.id)?.mode).toBe("human"); + expect(guard).toThrow(); + expect(executeScript).toHaveBeenCalledTimes(2); + await evaluate(`chrome.tabs.remove(${oldTab.id})`); }, ); } finally { diff --git a/apps/extension/src/cooperation/browser-bridge.test.ts b/apps/extension/src/cooperation/browser-bridge.test.ts index 1b4ba55d5..d45e63ffb 100644 --- a/apps/extension/src/cooperation/browser-bridge.test.ts +++ b/apps/extension/src/cooperation/browser-bridge.test.ts @@ -1,6 +1,14 @@ import { afterEach, expect, it, vi } from "vitest"; import { collectCooperationFrames } from "./browser-bridge"; -import { COOPERATION_ARM, COOPERATION_READ, COOPERATION_RESUME, type FrameState } from "./types"; +import { FrameCollectionError } from "./frame-error"; +import { + COOPERATION_ARM, + COOPERATION_DISABLE, + COOPERATION_READ, + COOPERATION_RESUME, + type FrameRepairs, + type FrameState, +} from "./types"; const frame = (human = 0): FrameState => ({ document: "document-a", @@ -13,12 +21,22 @@ const frame = (human = 0): FrameState => ({ fields: [], events: [], }); -function bridge(sendMessage: ReturnType) { +const main = { + frameId: 0, + documentId: "chrome-document-a", + url: "https://page.test/", + documentLifecycle: "active", + errorOccurred: false, +}; +function bridge(sendMessage: ReturnType, frames = [main]) { vi.useFakeTimers(); + const executeScript = vi.fn().mockResolvedValue([]); vi.stubGlobal("chrome", { - webNavigation: { getAllFrames: async () => [{ frameId: 0, documentId: "chrome-document-a" }] }, + webNavigation: { getAllFrames: async () => frames }, tabs: { sendMessage }, + scripting: { executeScript }, }); + return executeScript; } afterEach(() => { vi.useRealTimers(); @@ -31,7 +49,7 @@ it("retries an empty collection reply on the same document and retains interveni const input = { type: "pointerdown" as const, timestamp: 123, x: 4, y: 5 }; const result = collectCooperationFrames(7, COOPERATION_ARM, "lease", input); await vi.advanceTimersByTimeAsync(25); - expect((await result).get(0)?.human).toBe(1); + expect((await result).frames.get(0)?.human).toBe(1); expect(send).toHaveBeenCalledTimes(2); for (const call of send.mock.calls) expect(call).toEqual([ @@ -44,7 +62,7 @@ it("retries an empty collection reply on the same document and retains interveni it("does not retry a successful state read", async () => { const send = vi.fn().mockResolvedValue(frame()); bridge(send); - expect((await collectCooperationFrames(7, COOPERATION_READ, "lease")).size).toBe(1); + expect((await collectCooperationFrames(7, COOPERATION_READ, "lease")).frames.size).toBe(1); expect(send).toHaveBeenCalledOnce(); }); @@ -103,3 +121,213 @@ it("cannot arm a late retry after the collection deadline", async () => { await vi.advanceTimersByTimeAsync(25); expect(send).toHaveBeenCalledOnce(); }); + +it.each([ + COOPERATION_READ, + COOPERATION_RESUME, + COOPERATION_DISABLE, +])("skips only failed child frames and inactive documents for %s", async (kind) => { + const send = vi.fn().mockResolvedValue(frame()); + const execute = bridge(send, [ + main, + { ...main, frameId: 2, documentId: "failed", errorOccurred: true }, + ...["prerender", "cached", "pending_deletion"].map((documentLifecycle, i) => ({ + ...main, + frameId: i + 3, + documentId: documentLifecycle, + documentLifecycle, + })), + ]); + const result = await collectCooperationFrames(7, kind, "lease"); + expect([...result.frames.keys()]).toEqual([0]); + expect(result.gaps).toEqual([ + { frame_id: 2, url: main.url, reason: "load_error" }, + ...[3, 4, 5].map((frame_id) => ({ frame_id, url: main.url, reason: "inactive_document" })), + ]); + expect(send).toHaveBeenCalledOnce(); + expect(execute).not.toHaveBeenCalled(); +}); + +it("rejects a failed main page before messaging any frames", async () => { + const send = vi.fn(); + const execute = bridge(send, [{ ...main, errorOccurred: true }]); + await expect(collectCooperationFrames(7, COOPERATION_READ, "lease")).rejects.toMatchObject({ + reason: "page_load_failed", + frames: [{ frame_id: 0, url: main.url }], + }); + expect(send).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); +}); + +const missingReceiver = () => + new Error("Could not establish connection. Receiving end does not exist."); + +it("repairs a missing listener on the same document and lease", async () => { + const send = vi.fn().mockRejectedValueOnce(missingReceiver()).mockResolvedValue(frame(1)); + const execute = bridge(send); + const result = await collectCooperationFrames(7, COOPERATION_READ, "lease"); + expect(result.frames.get(0)?.human).toBe(1); + expect(execute).toHaveBeenCalledExactlyOnceWith({ + target: { tabId: 7, documentIds: [main.documentId] }, + files: ["content-scripts/cooperation.js"], + injectImmediately: true, + }); + expect(send).toHaveBeenCalledTimes(2); + expect(send.mock.calls[1]).toEqual(send.mock.calls[0]); +}); + +it("reports every unreachable active frame and never repeats a failed document repair", async () => { + const send = vi.fn().mockRejectedValue(missingReceiver()); + const execute = bridge(send, [main, { ...main, frameId: 3, documentId: "child" }]); + const repairs: FrameRepairs = new Map(); + for (let i = 0; i < 2; i++) { + const error = await collectCooperationFrames( + 7, + COOPERATION_READ, + "lease", + undefined, + repairs, + ).catch((error) => error as FrameCollectionError); + expect(error).toBeInstanceOf(FrameCollectionError); + expect((error as FrameCollectionError).toRpcError()).toMatchObject({ + code: "permission_denied", + data: { + reason: "frame_unobservable", + effect_state: "none", + frames: [ + { frame_id: 0, url: main.url }, + { frame_id: 3, url: main.url }, + ], + }, + }); + } + expect(execute).toHaveBeenCalledTimes(2); +}); + +it("prunes repair attempts for documents that left the tab", async () => { + const send = vi.fn().mockRejectedValue(missingReceiver()); + const frames = [main, { ...main, frameId: 3, documentId: "child" }]; + const execute = bridge(send, frames); + const repairs: FrameRepairs = new Map(); + const collect = () => collectCooperationFrames(7, COOPERATION_READ, "lease", undefined, repairs); + await expect(collect()).rejects.toMatchObject({ reason: "frame_unobservable" }); + expect([...repairs.keys()]).toEqual([main.documentId, "child"]); + frames.pop(); + await expect(collect()).rejects.toMatchObject({ reason: "frame_unobservable" }); + expect([...repairs.keys()]).toEqual([main.documentId]); + expect(execute).toHaveBeenCalledTimes(2); + frames.length = 0; + await expect(collect()).rejects.toThrow("No observable document"); + expect(repairs.size).toBe(0); +}); + +it.each([ + "gap", + "unobservable", + "main", +])("omits URL credentials, query and fragment from %s diagnostics", async (kind) => { + const url = "https://user:password@pay.test/checkout?token=secret#callback-token"; + const target = { + ...main, + frameId: kind === "main" ? 0 : 3, + documentId: "sensitive", + url, + errorOccurred: kind !== "unobservable", + }; + const send = vi.fn().mockImplementation(async (_tab, _message, { documentId }) => { + if (documentId === "sensitive") throw missingReceiver(); + return frame(); + }); + bridge(send, kind === "main" ? [target] : [main, target]); + const result = await collectCooperationFrames(7, COOPERATION_READ, "lease").then( + (collection) => collection.gaps, + (error: FrameCollectionError) => error.toRpcError(), + ); + const diagnostic = JSON.stringify(result); + expect(diagnostic).toContain("https://pay.test/checkout"); + expect(diagnostic).not.toMatch(/user|password|token|secret|callback|\?|#/); +}); + +it("shares an in-flight repair across concurrent reads", async () => { + let repaired = false; + let finish!: () => void; + const send = vi.fn(async () => { + if (!repaired) throw missingReceiver(); + return frame(); + }); + const execute = bridge(send); + execute.mockImplementation( + () => + new Promise((resolve) => { + finish = () => { + repaired = true; + resolve(); + }; + }), + ); + const repairs: FrameRepairs = new Map(); + const pending = [1, 2].map(() => + collectCooperationFrames(7, COOPERATION_READ, "lease", undefined, repairs), + ); + await vi.advanceTimersByTimeAsync(0); + expect(execute).toHaveBeenCalledOnce(); + finish(); + expect((await Promise.all(pending)).every((result) => result.frames.has(0))).toBe(true); +}); + +it.each([ + ["data:text/html,", "data:"], + ["blob:https://page.test/private-content", "blob:"], + ["https://bad host/private-content", "[unavailable]"], +])("limits opaque or malformed URL diagnostics without failing collection: %s", async (url, expected) => { + const send = vi.fn().mockResolvedValue(frame()); + bridge(send, [main, { ...main, frameId: 3, documentId: "child", errorOccurred: true, url }]); + const result = await collectCooperationFrames(7, COOPERATION_READ, "lease"); + expect(result.frames.has(0)).toBe(true); + expect(result.gaps).toEqual([{ frame_id: 3, url: expected, reason: "load_error" }]); +}); + +it("does not retry a denied injection or install collectors during release", async () => { + const send = vi.fn().mockRejectedValue(missingReceiver()); + const execute = bridge(send); + execute.mockRejectedValue(new Error("Cannot access contents of the page")); + const repairs: FrameRepairs = new Map(); + for (const kind of [COOPERATION_READ, COOPERATION_READ, COOPERATION_DISABLE]) + await expect( + collectCooperationFrames(7, kind, "lease", undefined, repairs), + ).rejects.toMatchObject({ + reason: "frame_unobservable", + }); + expect(execute).toHaveBeenCalledOnce(); + expect(send).toHaveBeenCalledTimes(3); +}); + +it.each([ + "Frame was removed", + "The message port closed before a response was received.", +])("does not inject for unrelated messaging errors: %s", async (message) => { + const send = vi.fn().mockRejectedValue(new Error(message)); + const execute = bridge(send); + await expect(collectCooperationFrames(7, COOPERATION_READ, "lease")).rejects.toThrow(message); + expect(execute).not.toHaveBeenCalled(); +}); + +it("cannot arm a repaired collector after the deadline", async () => { + let finish!: () => void; + const send = vi.fn().mockRejectedValue(missingReceiver()); + const execute = bridge(send); + execute.mockImplementation( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + const result = expect(collectCooperationFrames(7, COOPERATION_ARM, "lease")).rejects.toThrow( + "not responding", + ); + await vi.advanceTimersByTimeAsync(1200); + await result; + finish(); + await vi.advanceTimersByTimeAsync(25); + expect(send).toHaveBeenCalledOnce(); +}); diff --git a/apps/extension/src/cooperation/browser-bridge.ts b/apps/extension/src/cooperation/browser-bridge.ts index 5c13e2988..089c8ad87 100644 --- a/apps/extension/src/cooperation/browser-bridge.ts +++ b/apps/extension/src/cooperation/browser-bridge.ts @@ -1,5 +1,6 @@ import type { SessionManager } from "@/session-manager/manager"; import { CooperationController } from "./controller"; +import { FrameCollectionError } from "./frame-error"; import { COOPERATION_ARM, COOPERATION_CONTROL, @@ -8,6 +9,10 @@ import { COOPERATION_READ, COOPERATION_READY, COOPERATION_RESUME, + type CooperationFrames, + type FrameGap, + type FrameIdentity, + type FrameRepairs, type FrameState, type InputExpectation, } from "./types"; @@ -30,47 +35,137 @@ function validFrame(value: unknown): value is FrameState { ); } +function missingReceiver(error: unknown): boolean { + return error instanceof Error && error.message.includes("Receiving end does not exist"); +} + +function frameIdentity(frame: chrome.webNavigation.GetAllFrameResultDetails): FrameIdentity { + let url = "[unavailable]"; + try { + const parsed = new URL(frame.url); + url = ["http:", "https:"].includes(parsed.protocol) + ? `${parsed.origin}${parsed.pathname}` + : parsed.protocol; + } catch { + // Diagnostic formatting must not fail collection or expose the original value. + } + return { frame_id: frame.frameId, url }; +} + +async function readFrame( + tabId: number, + frame: chrome.webNavigation.GetAllFrameResultDetails, + message: { kind: string; lease: string; input?: InputExpectation }, + repairs: FrameRepairs, +): Promise { + const { kind } = message; + let timer: ReturnType | undefined; + let active = true; + try { + const request = () => chrome.tabs.sendMessage(tabId, message, { documentId: frame.documentId }); + const value = await Promise.race([ + (async () => { + let first: unknown; + try { + first = await request(); + } catch (error) { + if (!active || !missingReceiver(error)) throw error; + // Repair only a missing listener, never timeouts or navigation + // failures. Cleanup must not install a new collector after release. + if (kind === COOPERATION_DISABLE) + throw new FrameCollectionError("frame_unobservable", [frameIdentity(frame)]); + let repair = repairs.get(frame.documentId); + if (!repair) { + repair = chrome.scripting + .executeScript({ + target: { tabId, documentIds: [frame.documentId] }, + files: ["content-scripts/cooperation.js"], + injectImmediately: true, + }) + .then(() => {}); + repairs.set(frame.documentId, repair); + } + try { + await repair; + } catch { + throw new FrameCollectionError("frame_unobservable", [frameIdentity(frame)]); + } + if (!active) return undefined; + try { + first = await request(); + } catch (error) { + if (!missingReceiver(error)) throw error; + throw new FrameCollectionError("frame_unobservable", [frameIdentity(frame)]); + } + } + if (first !== undefined || ![COOPERATION_READ, COOPERATION_ARM].includes(kind)) + return first; + // A renderer can transiently return an empty reply. Retry only this + // state read, on the same document and lease, within the same deadline. + await new Promise((resolve) => setTimeout(resolve, 25)); + return active ? request() : undefined; + })(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error("A page frame is not responding")), 1200); + }), + ]); + if (!validFrame(value)) + throw new Error( + `Interaction collection is unavailable in frame ${frame.frameId} (${value === undefined ? "no reply" : "invalid state"})`, + ); + return value; + } finally { + active = false; + clearTimeout(timer); + } +} + export async function collectCooperationFrames( tabId: number, kind: string, lease: string, input?: InputExpectation, -): Promise> { + repairs: FrameRepairs = new Map(), +): Promise { const frames = await chrome.webNavigation.getAllFrames({ tabId }); + const documents = new Set(frames?.map((frame) => frame.documentId)); + for (const documentId of repairs.keys()) + if (!documents.has(documentId)) repairs.delete(documentId); if (!frames?.length) throw new Error("No observable document in this tab"); - const results = await Promise.all( - frames.map(async (frame) => { - let timer: ReturnType | undefined; - let active = true; - try { - const request = () => - chrome.tabs.sendMessage(tabId, { kind, input, lease }, { documentId: frame.documentId }); - const value = await Promise.race([ - (async () => { - const first = await request(); - if (first !== undefined || ![COOPERATION_READ, COOPERATION_ARM].includes(kind)) - return first; - // A renderer can transiently return an empty reply. Retry only this - // state read, on the same document and lease, within the same deadline. - await new Promise((resolve) => setTimeout(resolve, 25)); - return active ? request() : undefined; - })(), - new Promise((_, reject) => { - timer = setTimeout(() => reject(new Error("A page frame is not responding")), 1200); - }), - ]); - if (!validFrame(value)) - throw new Error( - `Interaction collection is unavailable in frame ${frame.frameId} (${value === undefined ? "no reply" : "invalid state"})`, - ); - return [frame.frameId, value] as const; - } finally { - active = false; - clearTimeout(timer); - } + const gaps: FrameGap[] = []; + const live = frames.filter((frame) => { + if (frame.errorOccurred) { + if (frame.frameId === 0) + throw new FrameCollectionError("page_load_failed", [frameIdentity(frame)]); + gaps.push({ ...frameIdentity(frame), reason: "load_error" }); + return false; + } + if (frame.documentLifecycle && frame.documentLifecycle !== "active") { + gaps.push({ ...frameIdentity(frame), reason: "inactive_document" }); + return false; + } + return true; + }); + const results = await Promise.allSettled( + live.map(async (frame) => { + return [ + frame.frameId, + await readFrame(tabId, frame, { kind, input, lease }, repairs), + ] as const; }), ); - return new Map(results); + const failures = results.filter((result) => result.status === "rejected"); + const unobservable = failures.flatMap((result) => + result.reason instanceof FrameCollectionError ? result.reason.frames : [], + ); + if (unobservable.length) throw new FrameCollectionError("frame_unobservable", unobservable); + if (failures.length) throw failures[0].reason; + return { + frames: new Map( + results.flatMap((result) => (result.status === "fulfilled" ? [result.value] : [])), + ), + gaps, + }; } export function attachCooperation( @@ -82,10 +177,16 @@ export function attachCooperation( sessions, changed, sessionActivity, - readFrames: (tabId, input, lease) => - collectCooperationFrames(tabId, input ? COOPERATION_ARM : COOPERATION_READ, lease, input), - resumeFrames: async (tabId, lease) => { - await collectCooperationFrames(tabId, COOPERATION_RESUME, lease); + readFrames: (tabId, input, lease, repairs) => + collectCooperationFrames( + tabId, + input ? COOPERATION_ARM : COOPERATION_READ, + lease, + input, + repairs, + ), + resumeFrames: async (tabId, lease, repairs) => { + await collectCooperationFrames(tabId, COOPERATION_RESUME, lease, undefined, repairs); }, releaseFrames: async (tabId, lease) => { await collectCooperationFrames(tabId, COOPERATION_DISABLE, lease); diff --git a/apps/extension/src/cooperation/controller.test.ts b/apps/extension/src/cooperation/controller.test.ts index 37258586a..019cb1099 100644 --- a/apps/extension/src/cooperation/controller.test.ts +++ b/apps/extension/src/cooperation/controller.test.ts @@ -8,7 +8,8 @@ import { captureVomObservation, handleSnapshot } from "@/tools/observation"; import type { CdpRunner } from "@/tools/shared"; import { handleTabBorrow, handleTabCreate, handleTabReturn, handleTabSelect } from "@/tools/tabs"; import { CooperationController } from "./controller"; -import type { FrameState } from "./types"; +import { FrameCollectionError } from "./frame-error"; +import type { CooperationFrames, FrameState } from "./types"; vi.mock("@/tools/observation", () => ({ captureVomObservation: vi.fn(), @@ -75,7 +76,12 @@ async function fixture(options: { url?: string; sync?: boolean } = {}) { return native(); }, } as CdpRunner; - const readFrames = vi.fn(async () => new Map([[0, structuredClone(frame)]])); + const readFrames = vi.fn( + async (): Promise => ({ + frames: new Map([[0, structuredClone(frame)]]), + gaps: [], + }), + ); const resumeFrames = vi.fn(async () => { frame = { ...frame, editing: false, revision: frame.revision + 1 }; }); @@ -1317,6 +1323,53 @@ describe("shared-tab dispatch fences", () => { expect(await f.command("wait")).toMatchObject({ ready: true }); expect(await f.act()).toMatchObject({ code: "permission_denied" }); }); + it("keeps an explicit human hold visible while navigation makes collection unavailable", async () => { + const f = await fixture(); + await f.controller.control(7, f.controller.view(7)!.revision, "hold"); + f.controller.invalidate(7); + f.readFrames.mockRejectedValueOnce(new Error("frame is loading")); + expect(await f.command("wait")).toMatchObject({ ready: false, state: { mode: "held" } }); + expect(await f.act()).toMatchObject({ code: "permission_denied" }); + expect(f.native).not.toHaveBeenCalled(); + }); + it.each([ + "frame_unobservable", + "page_load_failed", + ] as const)("returns %s immediately from sync, wait and actions without granting control", async (reason) => { + const f = await fixture(); + f.readFrames.mockRejectedValue( + new FrameCollectionError(reason, [{ frame_id: 3, url: "https://frame.test" }]), + ); + for (const action of ["sync", "wait"] as const) + expect(await f.command(action)).toMatchObject({ + code: "permission_denied", + data: { + reason, + effect_state: "none", + frames: [{ frame_id: 3, url: "https://frame.test" }], + }, + }); + expect(await f.act()).toMatchObject({ data: { reason, effect_state: "none" } }); + expect(f.controller.view(7)?.mode).toBe("unavailable"); + expect(f.native).not.toHaveBeenCalled(); + }); + it("exposes skipped frame gaps and leaves the policy unset until a successful sync", async () => { + const f = await fixture({ sync: false }); + const collection = await f.readFrames(); + collection.gaps = [{ frame_id: 3, url: "http://127.0.0.1:9/ad", reason: "load_error" }]; + f.readFrames.mockImplementation(async () => structuredClone(collection)); + expect(await f.command("status")).toMatchObject({ + control_policy: null, + frame_gaps: collection.gaps, + }); + expect(await f.command("wait")).toMatchObject({ control_policy: null, ready: true }); + expect(await f.command("sync")).toMatchObject({ + control_policy: "strict", + frame_gaps: collection.gaps, + context_id: expect.any(String), + }); + expect(await f.command("status")).toMatchObject({ control_policy: "strict" }); + }); it("fails closed when any frame is inaccessible and preserves a page the person touched", async () => { const f = await fixture(); f.change({ human: 1 }); @@ -2857,7 +2910,7 @@ it("revokes actions on media source changes even without DOM revision changes", const synced = await f.command("sync"); // A browser source selection can change currentSrc without a DOM mutation. const rev = f.readFrames.mock.results.at(-1)!.value; - const frames = await rev; + const { frames } = await rev; f.change( { media: [{ ...media, currentSrc: "two.webm" }], revision: frames.get(0)!.revision }, false, diff --git a/apps/extension/src/cooperation/controller.ts b/apps/extension/src/cooperation/controller.ts index bb3bbf48b..3d35d707d 100644 --- a/apps/extension/src/cooperation/controller.ts +++ b/apps/extension/src/cooperation/controller.ts @@ -17,6 +17,7 @@ import { } from "./action-checks"; import { guardedRunner } from "./action-guard"; import { type ControlAuthority, ControlAuthorityError } from "./control-authority"; +import { FrameCollectionError } from "./frame-error"; import { contentChanges, type ObservationEvidence, @@ -26,7 +27,15 @@ import { sameObservationEvidence, } from "./observation-evidence"; import { pointerTargetValidator } from "./pointer-target"; -import type { CooperationParams, CooperationView, FrameState, InputExpectation } from "./types"; +import type { + CooperationFrames, + CooperationParams, + CooperationView, + FrameGap, + FrameRepairs, + FrameState, + InputExpectation, +} from "./types"; type Frames = Map; interface Context { @@ -64,6 +73,8 @@ interface TabState { unavailable: boolean; frames: Frames; context?: Context; + frameGaps: FrameGap[]; + frameRepairs: FrameRepairs; baseline?: Context; activity: Activity[]; activitySequence: number; @@ -86,8 +97,13 @@ interface Volatile { } export interface CooperationDeps { sessions: SessionManager; - readFrames(tabId: number, input: InputExpectation | undefined, lease: string): Promise; - resumeFrames(tabId: number, lease: string): Promise; + readFrames( + tabId: number, + input: InputExpectation | undefined, + lease: string, + repairs: FrameRepairs, + ): Promise; + resumeFrames(tabId: number, lease: string, repairs: FrameRepairs): Promise; releaseFrames?(tabId: number, lease: string): Promise; changed(tabId: number): void; sessionActivity?(sessionId: string, idleProtected: boolean): void; @@ -306,6 +322,8 @@ export class CooperationController implements ControlAuthority { working: false, unavailable: true, frames: new Map(), + frameGaps: [], + frameRepairs: new Map(), activity: [], activitySequence: 0, activitySeen: new Map(), @@ -388,19 +406,21 @@ export class CooperationController implements ControlAuthority { return { target: !state.held && !this.humanActive(state) ? state.target : undefined, revision: state.revision, - mode: state.unavailable - ? "unavailable" - : state.working && (state.held || this.humanActive(state)) - ? "yielding" - : state.held - ? "held" - : this.humanActive(state) - ? "human" - : state.working - ? "acting" - : state.context - ? "ready" - : "syncing", + // Explicit human ownership survives navigation and unavailable collection. + mode: + state.unavailable && !state.held + ? "unavailable" + : state.working && (state.held || this.humanActive(state)) + ? "yielding" + : state.held + ? "held" + : this.humanActive(state) + ? "human" + : state.working + ? "acting" + : state.context + ? "ready" + : "syncing", editing: [...state.frames.values()].some((frame) => frame.editing), }; } @@ -482,7 +502,7 @@ export class CooperationController implements ControlAuthority { const epoch = state.epoch; this.changed(state); if (action === "resume") { - await this.deps.resumeFrames(tabId, state.lease); + await this.deps.resumeFrames(tabId, state.lease, state.frameRepairs); if (state.epoch === epoch) await this.refresh(state); } return { ok: true, state: this.view(tabId) }; @@ -517,15 +537,23 @@ export class CooperationController implements ControlAuthority { if (!live()) throw new Error("Shared-tab control has ended"); let frames: Frames; try { - frames = await this.deps.readFrames(state.tabId, input, state.lease); + const collection = await this.deps.readFrames( + state.tabId, + input, + state.lease, + state.frameRepairs, + ); + frames = collection.frames; + if (!live()) throw new Error("Shared-tab control ended during collection"); + if (!frames.size || !frames.has(0)) + throw new Error("Page interaction coverage is unavailable"); + state.frameGaps = collection.gaps; } catch (error) { state.unavailable = true; state.context = undefined; this.changed(state); throw error; } - if (!live()) throw new Error("Shared-tab control ended during collection"); - if (!frames.size || !frames.has(0)) throw new Error("Page interaction coverage is unavailable"); for (const [id, next] of frames) { const old = state.frames.get(id); if (old?.document === next.document) { @@ -600,7 +628,8 @@ export class CooperationController implements ControlAuthority { try { await this.refresh(state); } catch (error) { - if (params.action !== "wait" || signal.aborted) throw error; + if (params.action !== "wait" || signal.aborted || error instanceof FrameCollectionError) + throw error; state.unavailable = true; } }; @@ -638,7 +667,8 @@ export class CooperationController implements ControlAuthority { ), state: this.view(state.tabId), ready: !state.unavailable && !state.held && !state.working && !this.humanActive(state), - control_policy: session.controlPolicy ?? "strict", + control_policy: session.controlPolicy ?? null, + ...(state.frameGaps.length ? { frame_gaps: state.frameGaps } : {}), }; if (state.held || state.working || this.humanActive(state)) return failure("The user is still operating this tab; wait for hand-back"); @@ -750,6 +780,7 @@ export class CooperationController implements ControlAuthority { tab_id: state.tabId, context_id: context.id, control_policy: session.controlPolicy ?? "strict", + ...(state.frameGaps.length ? { frame_gaps: state.frameGaps } : {}), ...(targetPolicy ? { observation_id: context.observationId } : {}), kind: delta ? sameFrames(baseline.frames, after, "revision") @@ -805,6 +836,7 @@ export class CooperationController implements ControlAuthority { state.unavailable = true; state.context = undefined; this.changed(state); + if (error instanceof FrameCollectionError) return error.toRpcError(); return failure(error instanceof Error ? error.message : String(error)); } } @@ -1050,7 +1082,7 @@ export class CooperationController implements ControlAuthority { if (outcome === "cancelled") this.stopSession(state.sessionId); if (outcome === "continued" || outcome === "completed") { this.preserve(state); - await this.deps.resumeFrames(state.tabId, state.lease); + await this.deps.resumeFrames(state.tabId, state.lease, state.frameRepairs); } return result; } finally { @@ -1659,6 +1691,7 @@ export class CooperationController implements ControlAuthority { }; } if (error instanceof ActionCheckError) checkFailure = error.rpcError; + if (error instanceof FrameCollectionError) return error.toRpcError(sent ? "unknown" : "none"); return reportFailure( error instanceof Error ? error.message : String(error), sent ? "unknown" : "none", diff --git a/apps/extension/src/cooperation/frame-error.ts b/apps/extension/src/cooperation/frame-error.ts new file mode 100644 index 000000000..0b2b11843 --- /dev/null +++ b/apps/extension/src/cooperation/frame-error.ts @@ -0,0 +1,34 @@ +import type { RpcError } from "@/transport/types"; +import type { FrameIdentity } from "./types"; + +/** Permanent collection failures must not suggest another wait/sync loop. */ +export class FrameCollectionError extends Error { + constructor( + readonly reason: "frame_unobservable" | "page_load_failed", + readonly frames: FrameIdentity[], + ) { + super( + reason === "page_load_failed" + ? `The main page failed to load (${frames[0]?.url})` + : `Page frames cannot be observed: ${frames.map((frame) => `${frame.frame_id} (${frame.url})`).join(", ")}`, + ); + } + + toRpcError(effect: "none" | "unknown" = "none"): RpcError { + return { + code: "permission_denied", + message: this.message, + data: { + reason: this.reason, + effect_state: effect, + frames: this.frames, + recovery: + effect === "unknown" + ? "Inspect the application result before reloading or choosing another action. Never replay an uncertain submission." + : this.reason === "page_load_failed" + ? "Check the page URL and network connection, then reload the tab and synchronize." + : "Reload the tab once. If it persists, use read-only commands such as get-html or snapshot.", + }, + }; + } +} diff --git a/apps/extension/src/cooperation/live-text.browser.test.ts b/apps/extension/src/cooperation/live-text.browser.test.ts index 3acff8fc5..4c82c409d 100644 --- a/apps/extension/src/cooperation/live-text.browser.test.ts +++ b/apps/extension/src/cooperation/live-text.browser.test.ts @@ -212,7 +212,10 @@ async function withCooperation( const states = await evaluate>( "[window, ...Array.from(document.querySelectorAll('iframe'), f => f.contentWindow)].map(w => w.coop)", ); - return new Map(states.map((state, id) => [id, frameState(state)])); + return { + frames: new Map(states.map((state, id) => [id, frameState(state)])), + gaps: [], + }; }, resumeFrames: async () => {}, changed: () => {}, diff --git a/apps/extension/src/cooperation/policy-negotiation.test.ts b/apps/extension/src/cooperation/policy-negotiation.test.ts index 7fd0666de..1e8dd4714 100644 --- a/apps/extension/src/cooperation/policy-negotiation.test.ts +++ b/apps/extension/src/cooperation/policy-negotiation.test.ts @@ -40,7 +40,10 @@ async function fixture() { fields: [], events: [], }; - const readFrames = vi.fn(async () => new Map([[0, structuredClone(frame)]])); + const readFrames = vi.fn(async () => ({ + frames: new Map([[0, structuredClone(frame)]]), + gaps: [], + })); const controller = new CooperationController({ sessions, readFrames, diff --git a/apps/extension/src/cooperation/target-policy.test.ts b/apps/extension/src/cooperation/target-policy.test.ts index 027918cf5..4f50d1fb4 100644 --- a/apps/extension/src/cooperation/target-policy.test.ts +++ b/apps/extension/src/cooperation/target-policy.test.ts @@ -52,9 +52,10 @@ async function fixture(policy: "strict" | "target" = "target") { }); const send = vi.fn(async () => ({})); const cdp = { send } as CdpRunner; - const readFrames = vi.fn( - async () => new Map([[0, structuredClone(frame)]]), - ); + const readFrames = vi.fn(async () => ({ + frames: new Map([[0, structuredClone(frame)]]), + gaps: [], + })); const controller = new CooperationController({ sessions, readFrames, @@ -183,7 +184,7 @@ describe.each(["strict", "target"] as const)("completed action receipts (%s)", ( await runner.send(7, "Runtime.evaluate", { expression: "save()" }); if (coverage === "loading") f.readFrames.mockRejectedValueOnce(new Error("frame is loading")); - else f.readFrames.mockResolvedValueOnce(new Map()); + else f.readFrames.mockResolvedValueOnce({ frames: new Map(), gaps: [] }); return receipt; }), ).toEqual(receipt); @@ -258,10 +259,10 @@ describe.each(["strict", "target"] as const)("completed action receipts (%s)", ( ["navigated", "push"], ])("preserves human work in a %s iframe discovered by %s, even after quiet", async (kind, delivery) => { const f = await fixture(policy); - const frames = await f.readFrames.mock.results.at(-1)!.value; + const { frames } = await f.readFrames.mock.results.at(-1)!.value; const child = { ...frames.get(0)!, document: "child" }; if (kind === "navigated") frames.set(2, child); - f.readFrames.mockImplementation(async () => structuredClone(frames)); + f.readFrames.mockImplementation(async () => ({ frames: structuredClone(frames), gaps: [] })); const observation = await f.sync(); const lease = f.readFrames.mock.calls.at(-1)![2]; const result = await f.run( @@ -294,13 +295,16 @@ describe.each(["strict", "target"] as const)("completed action receipts (%s)", ( { human: 1, lastInput: 0 }, ])("does not preserve new frame history without recent human input: %j", async (history) => { const f = await fixture(policy); - const frames = await f.readFrames.mock.results.at(-1)!.value; + const { frames } = await f.readFrames.mock.results.at(-1)!.value; const receipt = { ok: true, tab_id: 7 }; expect( await f.run({ observation_id: f.first.observation_id }, "tool.evaluate", async (runner) => { await runner.send(7, "Runtime.evaluate", { expression: "openFrame()" }); frames.set(2, { ...frames.get(0)!, document: "new-child", ...history }); - f.readFrames.mockImplementation(async () => structuredClone(frames)); + f.readFrames.mockImplementation(async () => ({ + frames: structuredClone(frames), + gaps: [], + })); return receipt; }), ).toEqual(receipt); @@ -309,7 +313,7 @@ describe.each(["strict", "target"] as const)("completed action receipts (%s)", ( it("does not let an unknown document replace collected frames or repeatedly revoke control", async () => { const f = await fixture(policy); - const frame = (await f.readFrames.mock.results.at(-1)!.value).get(0)!; + const frame = (await f.readFrames.mock.results.at(-1)!.value).frames.get(0)!; const lease = f.readFrames.mock.calls.at(-1)![2]; const human = { ...frame, document: "uncollected", human: 1, lastInput: 10_000 }; const guard = f.controller.controlGuard("s", 7); @@ -810,10 +814,10 @@ describe("negotiated target control", () => { true, ])("checks unreported human input in an iframe at dispatch (existing: %s)", async (existing) => { const f = await fixture(); - const frames = await f.readFrames.mock.results.at(-1)!.value; + const { frames } = await f.readFrames.mock.results.at(-1)!.value; const child = { ...frames.get(0)!, document: "child" }; if (existing) frames.set(2, child); - f.readFrames.mockImplementation(async () => structuredClone(frames)); + f.readFrames.mockImplementation(async () => ({ frames: structuredClone(frames), gaps: [] })); const observation = await f.sync(); f.readFrames.mockClear(); vi.mocked(checkActionTarget).mockImplementationOnce(async () => { diff --git a/apps/extension/src/cooperation/types.ts b/apps/extension/src/cooperation/types.ts index ba91a3de2..9ffc1c6d2 100644 --- a/apps/extension/src/cooperation/types.ts +++ b/apps/extension/src/cooperation/types.ts @@ -60,6 +60,24 @@ export interface FrameState { events: HumanActivity[]; } +export interface FrameIdentity { + frame_id: number; + url: string; +} + +/** Documents that cannot receive page input and need no interaction collector. */ +export interface FrameGap extends FrameIdentity { + reason: "load_error" | "inactive_document"; +} + +export interface CooperationFrames { + frames: Map; + gaps: FrameGap[]; +} + +/** Shared by concurrent collections; pruned against the tab's document inventory. */ +export type FrameRepairs = Map>; + export interface CooperationView { target?: { x: number; y: number; expires: number }; revision: number; diff --git a/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts b/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts index c5cb965b3..079cb1258 100644 --- a/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts +++ b/apps/extension/src/session-manager/__tests__/tab-detachment-cdp.test.ts @@ -69,8 +69,9 @@ async function fixture() { sessions: manager, changed: () => {}, resumeFrames: async () => {}, - readFrames: async () => - new Map([ + readFrames: async () => ({ + gaps: [], + frames: new Map([ [ 0, { @@ -86,6 +87,7 @@ async function fixture() { }, ], ]), + }), }); cooperation.startSession("one"); await cooperation.observeTab(7); diff --git a/apps/extension/src/tools/__tests__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index 006c1e632..1f3e4dc74 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -401,7 +401,7 @@ describe("ToolDispatcher", () => { await sessions.start("aa11"); const cooperation = new CooperationController({ sessions, - readFrames: async () => new Map(), + readFrames: async () => ({ frames: new Map(), gaps: [] }), resumeFrames: async () => {}, changed: vi.fn(), }); @@ -933,7 +933,7 @@ describe("ToolDispatcher", () => { }; const cooperation = new CooperationController({ sessions, - readFrames: async () => new Map(), + readFrames: async () => ({ frames: new Map(), gaps: [] }), resumeFrames: async () => {}, changed: vi.fn(), }); diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index 802de108c..b9d644edf 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -26,6 +26,8 @@ export type ErrorCode = export type RpcErrorReason = | "user_moved_tab" | "cooperation_sync_required" + | "frame_unobservable" + | "page_load_failed" | "control_authority_required" | "control_context_invalid" | "human_intervention" diff --git a/crates/bsk-cli/skill/references/interaction-details.md b/crates/bsk-cli/skill/references/interaction-details.md index d52fd1d0e..496a578df 100644 --- a/crates/bsk-cli/skill/references/interaction-details.md +++ b/crates/bsk-cli/skill/references/interaction-details.md @@ -207,12 +207,28 @@ For an action followed by a transient synchronization refusal, use sync attempts, each retry preceded by `cooperate wait` (default 1s per wait; `--timeout-ms` overrides it). It retries only `cooperation_sync_required` with `effect_state: none`. Unknown effects, cancellation, user stop and other errors -are returned without retries. A wait that is not ready returns `settle.outcome: -not_ready` and no action context; continue the cooperation hand-back workflow. +are returned without retries. A wait reporting `state.mode: unavailable` uses the +remaining sync attempts, allowing navigation or slow frames to finish. If all +three attempts fail, the last sync error is returned with a non-zero exit code; +the CLI does not invent a frame failure or suggest reloading a loading page. Human control +or a stopped wait still returns `settle.outcome: not_ready` and no action context; +continue the cooperation hand-back workflow unless the user stopped the task. +An explicit human hold takes priority over unavailable coverage in the returned state. Inspect `settle`, ownership and context, not just exit code. This does not prove application readiness: use an observed loading-complete signal where available. Never replay the preceding click, drag or submission because sync was refused. +`frame_unobservable` means an active document still lacks interaction coverage +after one targeted collector repair. Reload once; if it persists, use read-only +commands such as `get-html` or `snapshot`. `page_load_failed` means the main page +failed to load: check its URL and network, then reload. Neither reason is retried +by `--settle`. Failed child frames and inactive documents are listed in +`frame_gaps` without blocking sync; other active frames must remain observable. +Frame diagnostic URLs retain only HTTP(S) origins and paths. Other URLs show +only their protocol, and malformed URLs show `[unavailable]`. +Before the first successful sync, status/wait reports `control_policy: null` +because the session has not yet selected a policy. + Use the ordinary full view for discovery when you do not yet know a useful label. Keep enough evidence to choose the next action; do not dump all markup when a targeted observation suffices. Stop at verified success. When measuring diff --git a/crates/bsk-cli/src/cli/cooperate/settle.rs b/crates/bsk-cli/src/cli/cooperate/settle.rs index d3cc1ecae..c98d34db9 100644 --- a/crates/bsk-cli/src/cli/cooperate/settle.rs +++ b/crates/bsk-cli/src/cli/cooperate/settle.rs @@ -20,6 +20,11 @@ pub(crate) fn run( } Err(error) if attempt < 3 && recoverable(&error) => { let mut wait = call(CooperateAction::Wait)?; + if wait["state"]["mode"] == "unavailable" && wait["stopped"] != true { + // Loading and slow frames can outlast this bounded wait. + // Let the next sync recover or return the actual failure. + continue; + } if wait["ready"] != true || wait["stopped"] == true { // No action context is issued while ownership is unavailable. if let Some(map) = wait.as_object_mut() { @@ -104,7 +109,9 @@ mod tests { fn human_hold_and_stopped_wait_return_without_an_action_context() { for wait in [ json!({"ready":false,"state":{"mode":"human"},"activity":["typing"]}), + json!({"ready":false,"state":{"mode":"held"}}), json!({"ready":true,"stopped":true}), + json!({"ready":false,"stopped":true,"state":{"mode":"unavailable"}}), ] { let mut calls = 0; let result = run(|_| { @@ -121,4 +128,65 @@ mod tests { assert_eq!(result["settle"]["outcome"], "not_ready"); } } + + #[test] + fn unobservable_frames_and_failed_pages_are_not_retried() { + for reason in ["frame_unobservable", "page_load_failed"] { + let mut calls = Vec::new(); + let result = run(|action| { + calls.push(format!("{action:?}")); + Err(CliError::from_rpc(RpcError { + code: ErrorCode::PermissionDenied, + message: "page unavailable".into(), + data: Some(json!({"reason": reason, "effect_state": "none"})), + })) + }); + assert_eq!(calls, ["Sync"]); + assert_eq!(result.unwrap_err().data().unwrap()["reason"], reason); + } + } + + #[test] + fn unavailable_wait_can_recover_on_the_next_sync() { + let mut calls = Vec::new(); + let result = run(|action| { + calls.push(format!("{action:?}")); + match calls.len() { + 1 => Err(error(ErrorCode::PermissionDenied, "none")), + 2 => Ok(json!({"ready":false,"state":{"mode":"unavailable"}})), + _ => Ok(json!({"context_id":"loaded"})), + } + }) + .unwrap(); + assert_eq!(calls, ["Sync", "Wait", "Sync"]); + assert_eq!(result["context_id"], "loaded"); + assert_eq!(result["settle"]["sync_attempts"], 2); + assert_eq!(result["settle"]["outcome"], "synchronized"); + } + + #[test] + fn unavailable_wait_exhausts_the_budget_and_preserves_the_last_sync_error() { + let mut calls = Vec::new(); + let result = run(|action| { + calls.push(format!("{action:?}")); + match action { + CooperateAction::Sync if calls.len() == 5 => Err(CliError::from_rpc(RpcError { + code: ErrorCode::PermissionDenied, + message: "frame is still loading".into(), + data: Some( + json!({"reason":"cooperation_sync_required","effect_state":"none","detail":"last attempt"}), + ), + })), + CooperateAction::Sync => Err(error(ErrorCode::PermissionDenied, "none")), + CooperateAction::Wait => Ok(json!({"ready":false,"state":{"mode":"unavailable"}})), + _ => panic!(), + } + }); + assert_eq!(calls, ["Sync", "Wait", "Sync", "Wait", "Sync"]); + let error = result.unwrap_err(); + assert_eq!(error.code(), Some(ErrorCode::PermissionDenied)); + assert_eq!(error.data().unwrap()["reason"], "cooperation_sync_required"); + assert_eq!(error.data().unwrap()["detail"], "last attempt"); + assert!(error.to_string().contains("frame is still loading")); + } } diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index 7188ece38..8506fd9a0 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -331,6 +331,18 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend ), ..base }, + (_, "frame_unobservable") => RenderInfo { + summary: "page interaction coverage is unavailable", + hint: Some( + "reload the tab once; if it persists, use read-only commands such as `bsk get-html` or `bsk snapshot`", + ), + ..base + }, + (_, "page_load_failed") => RenderInfo { + summary: "the main page failed to load", + hint: Some("check the page URL and network connection, then reload and synchronize"), + ..base + }, (_, "precondition_failed") => RenderInfo { summary: "a required business fact no longer matches", hint: Some( @@ -1249,4 +1261,21 @@ mod tests { assert!(info.summary.contains(phrase)); } } + + #[test] + fn collection_errors_explain_recovery_without_waiting_again() { + for (reason, summary, hint) in [ + ("frame_unobservable", "coverage is unavailable", "read-only"), + ("page_load_failed", "main page failed to load", "network"), + ] { + let info = info_for_error( + ErrorCode::PermissionDenied, + Some(&serde_json::json!({"reason":reason})), + ); + assert!(info.summary.contains(summary)); + assert!(info.hint.unwrap().contains(hint)); + assert!(!info.hint.unwrap().contains("cooperate wait")); + assert_ne!(info.exit_code, 0); + } + } } diff --git a/evals/browser/cases/regression/cooperation/broken-frame.html b/evals/browser/cases/regression/cooperation/broken-frame.html new file mode 100644 index 000000000..c7b72370f --- /dev/null +++ b/evals/browser/cases/regression/cooperation/broken-frame.html @@ -0,0 +1,9 @@ + + + + Cooperation with a failed child frame +

Product page

+ + 0 + + diff --git a/evals/browser/cases/regression/cooperation/frame-observability.mjs b/evals/browser/cases/regression/cooperation/frame-observability.mjs new file mode 100644 index 000000000..dad30149d --- /dev/null +++ b/evals/browser/cases/regression/cooperation/frame-observability.mjs @@ -0,0 +1,80 @@ +import assert from "node:assert/strict"; +import { attachExtensionWorker } from "./compatibility.mjs"; + +/** Exercise the shipped CLI and extension against a deterministic failed iframe. */ +export async function runFrameObservabilityScenarios({ send, bsk, base }) { + const worker = await attachExtensionWorker(send); + const results = []; + try { + for (const policy of ["target", "strict"]) { + const { session_id: session } = await bsk(["session", "start"]); + const scope = ["--session", session]; + try { + await bsk(["navigate", `${base}/broken-frame`, ...scope]); + const sync = () => bsk(["cooperate", "sync", "--settle", "--policy", policy, ...scope]); + const first = await sync(); + assert.equal(first.settle.outcome, "synchronized"); + assert.equal(first.control_policy, policy); + assert.equal(first.frame_gaps.length, 1); + assert.equal(first.frame_gaps[0].reason, "load_error"); + assert.notEqual(first.frame_gaps[0].frame_id, 0); + await bsk(["click", "#apply", "--context", first.context_id, ...scope]); + const current = await sync(); + const value = await bsk([ + "evaluate", + "document.querySelector('#count').textContent", + "--context", + current.context_id, + ...(current.observation_id ? ["--observation", current.observation_id] : []), + ...scope, + ]); + assert.equal(value.value, "1"); + + await send( + "Runtime.evaluate", + { + expression: `chrome.tabs.update(${first.tab_id}, {url:'http://127.0.0.1:9/main'})`, + awaitPromise: true, + }, + worker, + ); + const deadline = performance.now() + 10000; + while (true) { + const reply = await send( + "Runtime.evaluate", + { + expression: `chrome.webNavigation.getFrame({tabId:${first.tab_id},frameId:0})`, + awaitPromise: true, + returnByValue: true, + }, + worker, + ); + if (reply.result.value?.errorOccurred) break; + assert(performance.now() < deadline, "main-page load failure was not reported"); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + for (const args of [ + ["sync", "--settle"], + ["wait", "--timeout-ms", "20000"], + ]) { + const failure = await bsk(["cooperate", ...args, ...scope], "page_load_failed"); + assert.equal(failure.data.effect_state, "none"); + assert.equal(failure.data.frames[0].frame_id, 0); + assert.notEqual(failure.exit_code, 0); + } + results.push({ + policy, + failedChildSkipped: true, + click: true, + evaluate: true, + failedMainRejected: true, + }); + } finally { + await bsk(["session", "stop", session]); + } + } + } finally { + await send("Target.detachFromTarget", { sessionId: worker }).catch(() => {}); + } + return results; +} diff --git a/evals/browser/cases/regression/cooperation/run.mjs b/evals/browser/cases/regression/cooperation/run.mjs index 84ec640d3..bf04a8e37 100644 --- a/evals/browser/cases/regression/cooperation/run.mjs +++ b/evals/browser/cases/regression/cooperation/run.mjs @@ -9,6 +9,7 @@ import { promisify } from "node:util"; import { withChrome } from "../snapshot-coordinates/chrome.mjs"; import { attachExtensionWorker, runCompatibilityScenarios } from "./compatibility.mjs"; import { runLegacyStrictDshScenario } from "./dsh-client.mjs"; +import { runFrameObservabilityScenarios } from "./frame-observability.mjs"; import { runLifetimeScenario } from "./lifetime.mjs"; import { runGenericScenarios } from "./scenarios.mjs"; import { runTargetPolicyScenarios } from "./target-policy.mjs"; @@ -21,6 +22,7 @@ const artifacts = process.env.BSK_COOP_ARTIFACTS; const loadExtensionOverCdp = process.env.BSK_COOP_LOAD_CDP === "1"; const compatibilityOnly = process.env.BSK_COOP_COMPAT_ONLY === "1"; const targetOnly = process.env.BSK_COOP_TARGET_ONLY === "1"; +const framesOnly = process.env.BSK_COOP_FRAMES_ONLY === "1"; const fullIdle = process.env.BSK_COOP_FULL_IDLE === "1"; const headful = process.env.BSK_COOP_HEADFUL === "1"; const deviceScale = Number(process.env.BSK_COOP_DPR ?? 1); @@ -28,7 +30,10 @@ const deviceScale = Number(process.env.BSK_COOP_DPR ?? 1); const idleWaitMs = fullIdle ? 360500 : 12500; if (artifacts) await mkdir(artifacts, { recursive: true }); assert(executable, "Set BSK_COOP_CHROME to an isolated Chromium executable"); -assert(!(compatibilityOnly && targetOnly), "Choose compatibility-only or target-only coverage"); +assert( + [compatibilityOnly, targetOnly, framesOnly].filter(Boolean).length <= 1, + "Choose one coverage subset", +); assert(port !== 52800 && Number.isInteger(port)); assert( (await readFile(join(extensionPath, "background.js"), "utf8")).includes(`ws://127.0.0.1:${port}`), @@ -104,6 +109,7 @@ const poll = async (read, label = "setup") => { }; const html = await readFile(new URL("fixture.html", import.meta.url)); const targetHtml = await readFile(new URL("target-policy.html", import.meta.url)); +const brokenFrameHtml = await readFile(new URL("broken-frame.html", import.meta.url)); const submissions = []; const server = createServer(async (req, res) => { if (req.url === "/submit") { @@ -118,15 +124,18 @@ const server = createServer(async (req, res) => { res.end( req.url === "/frame" ? '' - : req.url?.startsWith("/target-policy?") - ? targetHtml - : html, + : req.url === "/broken-frame" + ? brokenFrameHtml + : req.url?.startsWith("/target-policy?") + ? targetHtml + : html, ); } }); let browserVersion; let compatibility; let targetPolicy; +let frameObservability; let daemon; let daemonOutput = ""; const evidence = []; @@ -143,7 +152,9 @@ try { "--port", String(port), // Capability-only runs must not inherit the accelerated idle-test clock. - ...(fullIdle || compatibilityOnly || targetOnly ? [] : ["--session-idle", "8s"]), + ...(fullIdle || compatibilityOnly || targetOnly || framesOnly + ? [] + : ["--session-idle", "8s"]), ], { env, @@ -202,6 +213,9 @@ try { }; browserVersion = (await send("Browser.getVersion")).product; await poll(async () => (await bsk(["browsers"])).length, "extension connection"); + if (!compatibilityOnly) + frameObservability = await runFrameObservabilityScenarios({ send, bsk: runBsk, base }); + if (framesOnly) return; if (!compatibilityOnly) targetPolicy = await runTargetPolicyScenarios({ send, bsk: runBsk, cli, env, base }); if (targetOnly) return; @@ -866,14 +880,17 @@ try { ); const report = { passed: true, - suite: targetOnly - ? "target-policy" - : compatibilityOnly - ? "legacy-strict-compatibility" - : "legacy-strict-with-target-smoke", + suite: framesOnly + ? "frame-observability" + : targetOnly + ? "target-policy" + : compatibilityOnly + ? "legacy-strict-compatibility" + : "legacy-strict-with-target-smoke", browserVersion, headful, - ...(!targetOnly ? { closedTaskTabFailsClosed: true, legacyStrict: true } : {}), + ...(!targetOnly && !framesOnly ? { closedTaskTabFailsClosed: true, legacyStrict: true } : {}), + frameObservability, targetPolicy, compatibility, submissions,