diff --git a/docs/guides/web/browser-observation.md b/docs/guides/web/browser-observation.md index 5c939be338..1e2909a74b 100644 --- a/docs/guides/web/browser-observation.md +++ b/docs/guides/web/browser-observation.md @@ -320,6 +320,17 @@ rea discover-webmcp-tools http://127.0.0.1:9222 TARGET_ID \ --json ``` +The `observe-web-session` CLI writes JSON progress lines to stderr, leaving +stdout for the final Evidence or failure. Wait for `rea_progress` with +`phase: "browser_observation"`, `completed: 1` and `total: 2` before performing +the external action. After the window ends successfully, `completed: 2` and +`terminal: true` report its end. Failed preparation does not announce arming; +a failed or cancelled observation does not fabricate a successful end event. +MCP exposes the same provider progress through its progress notifications. +If navigation leaves the selected scope, or the target detaches or terminates +during preparation, the ended window can be reported without an armed event. +Only the explicit armed event invites the caller to perform its action. + ### WebMCP discovery prerequisites `discover_webmcp_tools` reports only what the attached browser's experimental diff --git a/src/browser/CdpObservationSession.ts b/src/browser/CdpObservationSession.ts index ffbfd5cfbb..af6e1a9764 100644 --- a/src/browser/CdpObservationSession.ts +++ b/src/browser/CdpObservationSession.ts @@ -135,13 +135,14 @@ export const observeCdpSession = async ( context.sessionId, context.signal, ); - await context.progress?.report({ - phase: "browser_observation", - completed: 1, - total: 2, - message: - "Browser observation armed; perform the requested user action now", - }); + if (!capture.stopped) + await context.progress?.report({ + phase: "browser_observation", + completed: 1, + total: 2, + message: + "Browser observation armed; perform the requested user action now", + }); let endReason = await waitForWindow( context.input.observation_ms, capture, @@ -226,7 +227,7 @@ class TimelineCapture { } ingest(event: CdpTransportEvent): void { - if (this.#endReason !== undefined || this.#scopeExcluded) return; + if (this.stopped) return; if (!isTimelineEvent(event.method)) return; if (this.#armTimestamp === undefined) { this.#bufferBeforeArm(event); @@ -236,7 +237,7 @@ class TimelineCapture { } #ingestArmed(event: CdpTransportEvent, bufferedBeforeArm = false): void { - if (this.#endReason !== undefined || this.#scopeExcluded) return; + if (this.stopped) return; const params = recordValue(event.params); if (params === undefined) { if ( @@ -322,6 +323,11 @@ class TimelineCapture { this.#eventsBeforeArmBytes += bytes; } + /** One lifecycle fact controls event retention and readiness delivery. */ + get stopped(): boolean { + return this.#endReason !== undefined || this.#scopeExcluded; + } + onEnd(listener: (reason: EndReason) => void): () => void { if (this.#endReason !== undefined) listener(this.#endReason); else this.#listeners.add(listener); diff --git a/src/cli/browserCommands.ts b/src/cli/browserCommands.ts index 4c8c7298ee..a15d3aa750 100644 --- a/src/cli/browserCommands.ts +++ b/src/cli/browserCommands.ts @@ -42,6 +42,7 @@ import { } from "../cliObservationOptions.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; +import { cliProgressReporter } from "./cliProgress.js"; /** Register CLI equivalents of the passive browser MCP tools. */ export const registerBrowserCommands = ( @@ -318,7 +319,12 @@ const registerObservationSession = ( ), ); return await finishAnalysisCommand( - () => observeWebSession(provider, parsed.data, { signal }), + () => + observeWebSession(provider, parsed.data, { + signal, + // Report the armed observation on stderr, as MCP does. + progress: cliProgressReporter, + }), { close: async () => { await provider.close?.(); diff --git a/src/cli/cliProgress.ts b/src/cli/cliProgress.ts new file mode 100644 index 0000000000..b731312e00 --- /dev/null +++ b/src/cli/cliProgress.ts @@ -0,0 +1,13 @@ +import type { ProgressReporter } from "../application/ProgressReporter.js"; + +/** Write one progress update as a stderr line, keeping stdout results parseable. */ +export const writeCliProgressLine = (update: unknown): void => { + process.stderr.write(`${JSON.stringify({ rea_progress: update })}\n`); +}; + +/** CLI counterpart of MCP progress notifications: each update is one stderr line. */ +export const cliProgressReporter: ProgressReporter = { + report: async (update) => { + writeCliProgressLine(update); + }, +}; diff --git a/src/cli/desktopCaptureCommands.ts b/src/cli/desktopCaptureCommands.ts index bd6408dbd6..9d8f0564e2 100644 --- a/src/cli/desktopCaptureCommands.ts +++ b/src/cli/desktopCaptureCommands.ts @@ -29,6 +29,7 @@ import { import { SessionRequestQueue } from "../desktop/DesktopSessionRequests.js"; import { withCommandCancellation } from "./commandCancellation.js"; import type { CliInstance } from "./types.js"; +import { writeCliProgressLine } from "./cliProgress.js"; /** One coherent view policy; invalid JSON and unknown controls fail at admission. */ const viewPolicyOption = z @@ -562,7 +563,7 @@ export const registerDesktopCaptureCommands = ( ...(event === undefined ? {} : { event }), }, }; - process.stderr.write(`${JSON.stringify(progress)}\n`); + writeCliProgressLine(progress.rea_progress); }, }, signal, diff --git a/src/cli/javascriptApplicationAnalysis.ts b/src/cli/javascriptApplicationAnalysis.ts index bc2522a757..25aed25f1e 100644 --- a/src/cli/javascriptApplicationAnalysis.ts +++ b/src/cli/javascriptApplicationAnalysis.ts @@ -12,6 +12,7 @@ import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { analysisInputErrorFromIssues } from "../domain/inputIssueProjection.js"; import { javaScriptSourcePathsSchema } from "../domain/javascript/javascriptSourceSelection.js"; import { parseCliJsonInput } from "../cliJsonInput.js"; +import { writeCliProgressLine } from "./cliProgress.js"; import { ownCliProcessResource } from "./cliResources.js"; const OPERATION = "analyze_javascript_application"; @@ -115,7 +116,7 @@ export const runCliJavaScriptApplicationAnalysis = async ( await import("../application/javascript/JavaScriptApplicationService.js"); const progress = createProgressReporter( async (update) => { - process.stderr.write(`${JSON.stringify({ rea_progress: update })}\n`); + writeCliProgressLine(update); }, { minimumIntervalMs: 0 }, ); diff --git a/src/cli/webRuntimeCommands.ts b/src/cli/webRuntimeCommands.ts index d40a70d8da..c88b9f2474 100644 --- a/src/cli/webRuntimeCommands.ts +++ b/src/cli/webRuntimeCommands.ts @@ -12,16 +12,9 @@ import { logCliCommand } from "../cliLogging.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { cliDecimalField } from "./options.js"; import type { Logger } from "pino"; -import type { ProgressReporter } from "../application/ProgressReporter.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; - -/** CLI progress exposes the actual armed point on stderr without corrupting JSON results. */ -const runtimeProgress: ProgressReporter = { - report: async (update) => { - process.stderr.write(`${JSON.stringify({ rea_progress: update })}\n`); - }, -}; +import { cliProgressReporter } from "./cliProgress.js"; /** Register public equivalents of the two modular website runtime tools. */ export const registerWebRuntimeCommands = ( @@ -59,7 +52,7 @@ export const registerWebRuntimeCommands = ( observation_ms: options.observationMs, ...selectedCommandTimeout(options.commandTimeoutMs), }, - { progress: runtimeProgress, signal }, + { progress: cliProgressReporter, signal }, ), { close: async () => { diff --git a/tests/acceptance/webSessionCliProgress.test.ts b/tests/acceptance/webSessionCliProgress.test.ts new file mode 100644 index 0000000000..e672528569 --- /dev/null +++ b/tests/acceptance/webSessionCliProgress.test.ts @@ -0,0 +1,242 @@ +import { once } from "node:events"; +import { fileURLToPath } from "node:url"; +import { expect, onTestFinished } from "vitest"; +import { z } from "zod"; + +import { parseEvidence } from "../../src/domain/evidence.js"; +import { webObservationSessionSchema } from "../../src/domain/browserSession.js"; +import { + startFakeCdpBrowser, + type FakeCdpBrowser, +} from "../fixtures/fakeCdpBrowser.js"; +import { + processTest, + type TestProcesses, +} from "../support/process/processFixture.js"; + +const entrypoint = fileURLToPath( + new URL("../../scripts/rea.mjs", import.meta.url), +); +const progressSchema = z.object({ + rea_progress: z.object({ + phase: z.literal("browser_observation"), + completed: z.union([z.literal(1), z.literal(2)]), + total: z.literal(2), + message: z.string(), + terminal: z.boolean().optional(), + }), +}); +type Progress = z.infer["rea_progress"]; + +const observe = async ( + processes: TestProcesses, + browser: FakeCdpBrowser, + onProgress: (update: Progress) => void = () => {}, +) => { + const child = processes.spawn( + process.execPath, + [ + entrypoint, + "observe-web-session", + browser.endpoint, + "allowed-page", + "--allowed-origins", + browser.allowedOrigin, + "--observation-ms", + "1000", + "--json", + ], + { env: { ...process.env, REA_LOG_LEVEL: "silent" } }, + ); + child.stdin.end(); + let stdout = ""; + let stderr = ""; + let pending = ""; + const updates: Progress[] = []; + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + child.stdout.on("data", (chunk: string) => { + stdout += chunk; + }); + child.stderr.on("data", (chunk: string) => { + stderr += chunk; + const lines = (pending + chunk).split("\n"); + pending = lines.pop() ?? ""; + for (const line of lines) { + let value: unknown; + try { + value = JSON.parse(line); + } catch { + continue; + } + const update = progressSchema.safeParse(value); + if (!update.success) continue; + updates.push(update.data.rea_progress); + onProgress(update.data.rea_progress); + } + }); + let timedOut = false; + const timer = setTimeout(() => { + timedOut = true; + child.kill("SIGKILL"); + }, 20_000); + try { + const [code, signal] = await once(child, "close"); + expect(timedOut, stderr).toBe(false); + expect(signal, stderr).toBeNull(); + expect(stdout).not.toContain("rea_progress"); + return { code, stdout, stderr, updates }; + } finally { + clearTimeout(timer); + } +}; + +processTest( + "arms the CLI caller before accepting its action and keeps final Evidence on stdout", + async ({ processes }) => { + const browser = await startFakeCdpBrowser({ commandEvents: () => [] }); + onTestFinished(() => browser.close()); + const url = `${browser.allowedOrigin}/action-after-ready`; + let actions = 0; + const result = await observe(processes, browser, (update) => { + if (update.completed !== 1) return; + actions += 1; + const sessionId = browser.commands.find( + ({ method }) => method === "Page.enable", + )?.sessionId; + browser.emitEvent({ + method: "Page.navigatedWithinDocument", + ...(sessionId === undefined ? {} : { sessionId }), + params: { frameId: "frame-main", url, navigationType: "historyApi" }, + }); + }); + expect(result.code, result.stderr).toBe(0); + expect(actions).toBe(1); + expect( + result.updates.map(({ message: _message, ...facts }) => facts), + ).toEqual([ + { + phase: "browser_observation", + completed: 1, + total: 2, + }, + { + phase: "browser_observation", + completed: 2, + total: 2, + terminal: true, + }, + ]); + const capture = webObservationSessionSchema.parse( + parseEvidence(JSON.parse(result.stdout)).normalized_result, + ); + expect(capture.window.end_reason).toBe("window_elapsed"); + expect(capture.timeline).toContainEqual( + expect.objectContaining({ type: "same_document_navigation", url }), + ); + expect(browser.commands).toContainEqual( + expect.objectContaining({ method: "Target.detachFromTarget" }), + ); + }, +); + +processTest( + "does not announce arming or completion when CDP preparation fails", + async ({ processes }) => { + const browser = await startFakeCdpBrowser({ + commandError: ({ method }) => + method === "Performance.enable" + ? { code: -32000, message: "authored preparation failure" } + : undefined, + }); + onTestFinished(() => browser.close()); + const result = await observe(processes, browser); + expect(result.code).not.toBe(0); + expect( + result.updates.map(({ message: _message, ...facts }) => facts), + ).toEqual([]); + expect(JSON.parse(result.stdout)).toMatchObject({ + code: expect.any(String), + category: expect.any(String), + }); + expect(browser.commands).toContainEqual( + expect.objectContaining({ method: "Target.detachFromTarget" }), + ); + }, +); + +processTest( + "keeps a failure after arming distinct from a successfully ended window", + async ({ processes }) => { + const browser = await startFakeCdpBrowser({ commandEvents: () => [] }); + onTestFinished(() => browser.close()); + const result = await observe(processes, browser, (update) => { + if (update.completed === 1) browser.emitRawMessage("{not-json"); + }); + expect(result.code).not.toBe(0); + expect(result.updates.map(({ completed }) => completed)).toEqual([1]); + expect(result.updates.every(({ terminal }) => terminal !== true)).toBe( + true, + ); + expect(JSON.parse(result.stdout)).toMatchObject({ + code: expect.any(String), + category: expect.any(String), + }); + }, +); + +const stoppedPreparations = [ + { event: "outside-origin", endReason: "target_left_scope" }, + { event: "session-detached", endReason: "session_detached" }, + { event: "target-crashed", endReason: "target_terminated" }, +] as const; + +processTest.for(stoppedPreparations)( + "does not arm when preparation already stopped collection: $event", + async ({ event, endReason }, { processes }) => { + const browser = await startFakeCdpBrowser({ + eventsBeforeReply: ["Page.setLifecycleEventsEnabled"], + commandEvents: (command) => { + if (command.method !== "Page.setLifecycleEventsEnabled") return []; + const sessionId = z.string().parse(command.sessionId); + if (event === "outside-origin") + return [ + { + method: "Page.navigatedWithinDocument", + sessionId, + params: { + frameId: "frame-main", + url: "https://outside.example/withheld", + navigationType: "historyApi", + }, + }, + ]; + if (event === "session-detached") + return [ + { + method: "Target.detachedFromTarget", + params: { sessionId, targetId: "allowed-page" }, + }, + ]; + return [{ method: "Inspector.targetCrashed", sessionId, params: {} }]; + }, + }); + onTestFinished(() => browser.close()); + let actions = 0; + const result = await observe(processes, browser, ({ completed }) => { + if (completed === 1) actions += 1; + }); + expect(result.code, result.stderr).toBe(0); + expect(actions).toBe(0); + expect( + result.updates.map(({ message: _message, ...facts }) => facts), + ).toEqual([ + { phase: "browser_observation", completed: 2, total: 2, terminal: true }, + ]); + const capture = webObservationSessionSchema.parse( + parseEvidence(JSON.parse(result.stdout)).normalized_result, + ); + expect(capture.window.end_reason).toBe(endReason); + expect(result.stdout).not.toContain("https://outside.example/withheld"); + }, +);