mirror of
https://github.com/morluto/rea.git
synced 2026-11-14 18:52:48 +00:00
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>
This commit is contained in:
1 parent
fee267759d
commit
3e95d22dd4
8 files changed
+294
-21
No files matched your search
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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?.();
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
};
|
||||
@@ -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,
|
||||
|
||||
@@ -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 },
|
||||
);
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<typeof progressSchema>["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");
|
||||
},
|
||||
);
|
||||
Reference in new issue
Block a user