From 3e95d22dd4bfd3274e613e76ededd899381fd078 Mon Sep 17 00:00:00 2001 From: wunianze666-netizen Date: Sat, 14 Nov 2026 10:14:39 +0000 Subject: [PATCH] fix(cli): report observe-web-session progress on stderr MCP passes progress to observeWebSession, but the CLI did not, so a caller of `rea observe-web-session` never saw the armed observation window it must wait for before acting. Three CLI commands also each wrote their own copy of the same stderr progress line. One cliProgress module now owns the stderr progress line and reporter. observe-web-session uses it, as do web runtime, JavaScript application and desktop capture progress. The CDP session also stops reporting readiness once collection is no longer active, so a stopped capture does not invite the external action. Fixes #8141 Supersedes #8149 Based on #8149; the desktop capture copy now shares the line writer too. Co-Authored-By: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/guides/web/browser-observation.md | 11 + src/browser/CdpObservationSession.ts | 24 +- src/cli/browserCommands.ts | 8 +- src/cli/cliProgress.ts | 13 + src/cli/desktopCaptureCommands.ts | 3 +- src/cli/javascriptApplicationAnalysis.ts | 3 +- src/cli/webRuntimeCommands.ts | 11 +- .../acceptance/webSessionCliProgress.test.ts | 242 ++++++++++++++++++ 8 files changed, 294 insertions(+), 21 deletions(-) create mode 100644 src/cli/cliProgress.ts create mode 100644 tests/acceptance/webSessionCliProgress.test.ts 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"); + }, +);