From c3b79cebe62f19f9aade36333772c45512314bc2 Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Sun, 25 Oct 2026 09:58:02 +0000 Subject: [PATCH] feat(browser): validate browser scenarios with --dry-run before launching Checking a scenario file meant running it, which discovers, launches or attaches to a browser and can change page state before a typo surfaces. `rea capture-browser-scenario --dry-run` reuses the same JSON and path resolution, the canonical scenario schema and its semantic rules, and the secret-availability rule capture applies before acquiring a browser, now shared from one domain owner. It returns the same projected input errors, or a compact plan of steps, secret availability and the stages it did not check, and never creates a provider or reads secret values. Fixes #5488 Co-Authored-By: Claude Opus 5.5 --- docs/browser-scenario-contract.md | 44 ++++ .../BrowserScenarioCaptureService.ts | 64 +++++- src/browser/BrowserScenarioSecrets.ts | 39 +--- src/cliBrowserScenarioCommands.ts | 28 ++- src/domain/browserScenario.ts | 38 ++++ .../cli/browserScenarioDryRun.test.ts | 201 ++++++++++++++++++ 6 files changed, 380 insertions(+), 34 deletions(-) create mode 100644 tests/boundary/cli/browserScenarioDryRun.test.ts diff --git a/docs/browser-scenario-contract.md b/docs/browser-scenario-contract.md index 1a3a0d703a..6ad83a6c98 100644 --- a/docs/browser-scenario-contract.md +++ b/docs/browser-scenario-contract.md @@ -79,6 +79,50 @@ profile for this request; it does not start your fixture server or install a browser. A schema-valid scenario does not prove that the executable or target is available on the host. +### Validate without a browser + +Add `--dry-run` to check a scenario before it launches or changes anything: + +```bash +rea capture-browser-scenario ./browser-scenario.json --dry-run --json +``` + +The dry run reads the same JSON argument and resolves the same paths as a +capture, then applies the same schema and semantic rules: duplicate or reserved +step IDs, duplicate secret IDs or variables, and undeclared secret references. +It also requires each declared secret's environment variable to be set, which +is the rule a capture applies before acquiring a browser. Errors are the same +`invalid_request` projection with field paths, under +`"error": "Browser scenario validation failed"`. A valid scenario exits 0 +with a compact plan and creates no provider, profile, file or connection: + +```json +{ + "operation": "capture_browser_scenario", + "dry_run": true, + "browser_mode": "launch", + "start_origin": "http://127.0.0.1:3000", + "steps": [ + { "step_id": "scenario_start", "action": "goto_start" }, + { "step_id": "settle", "action": "wait_for_timeout" } + ], + "secrets": [], + "not_checked": [ + "browser executable, CDP endpoint and target discovery", + "browser launch or attachment", + "navigation, network reachability and page state", + "locator resolution, action outcomes and capture limits" + ] +} +``` + +Steps are listed in capture order, including the initial navigation. Secrets +report only `environment_value: "present"`; values are never read into the +plan or printed. `not_checked` names the stages a dry run cannot establish, so +a valid plan is not a prediction that the capture will succeed. The dry run is +CLI-only. Over MCP, `capture_browser_scenario` already rejects invalid input +before acquiring a browser. + ## Interaction example: capture the visible result The default final URL answers "where did the page end up?", not "what did the diff --git a/src/application/BrowserScenarioCaptureService.ts b/src/application/BrowserScenarioCaptureService.ts index 24e8c0ca4f..0e1ef2a96c 100644 --- a/src/application/BrowserScenarioCaptureService.ts +++ b/src/application/BrowserScenarioCaptureService.ts @@ -8,8 +8,13 @@ import { } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import type { Evidence } from "../domain/evidence.js"; -import type { BrowserScenario } from "../domain/browserScenario.js"; -import { err, type Result } from "../domain/result.js"; +import { + browserScenarioSecretAvailability, + type BrowserScenario, +} from "../domain/browserScenario.js"; +import { BROWSER_SCENARIO_START_STEP_ID } from "../domain/browserScenarioValues.js"; +import type { AnalysisInputError } from "../domain/analysisErrorCore.js"; +import { err, ok, type Result } from "../domain/result.js"; import { browserScenarioCaptureSchema } from "../domain/browserScenarioCapture.js"; import { completeProviderEvidence, @@ -82,3 +87,58 @@ export const captureBrowserScenario = async ( options.signal, ); }; + +/** What a dry run checked, and the explicit stages it stopped before. */ +export interface BrowserScenarioPlan { + readonly operation: typeof OPERATION; + readonly dry_run: true; + readonly browser_mode: BrowserScenario["browser"]["mode"]; + readonly start_origin: string; + /** Steps in capture order, including the initial navigation step. */ + readonly steps: readonly { + readonly step_id: string; + readonly action: string; + }[]; + readonly secrets: readonly { + readonly secret_id: string; + readonly environment_variable: string; + readonly environment_value: "present"; + }[]; + readonly not_checked: readonly string[]; +} + +const NOT_CHECKED = [ + "browser executable, CDP endpoint and target discovery", + "browser launch or attachment", + "navigation, network reachability and page state", + "locator resolution, action outcomes and capture limits", +] as const; + +/** + * Apply the execution rules that need no browser to an already parsed + * scenario, then stop before any provider is created. Secret values are + * never read. + */ +export const planBrowserScenario = ( + scenario: BrowserScenario, + environment: Readonly>, +): Result => { + const available = browserScenarioSecretAvailability(scenario, environment); + if (!available.ok) return available; + return ok({ + operation: OPERATION, + dry_run: true, + browser_mode: scenario.browser.mode, + start_origin: new URL(scenario.start_url.url).origin, + steps: [ + { step_id: BROWSER_SCENARIO_START_STEP_ID, action: "goto_start" }, + ...scenario.actions.map(({ step_id, action }) => ({ step_id, action })), + ], + secrets: scenario.secrets.map(({ secret_id, environment_variable }) => ({ + secret_id, + environment_variable, + environment_value: "present" as const, + })), + not_checked: NOT_CHECKED, + }); +}; diff --git a/src/browser/BrowserScenarioSecrets.ts b/src/browser/BrowserScenarioSecrets.ts index 18c238342c..d0630f51ba 100644 --- a/src/browser/BrowserScenarioSecrets.ts +++ b/src/browser/BrowserScenarioSecrets.ts @@ -2,7 +2,10 @@ import { AnalysisInputError } from "../domain/analysisErrorCore.js"; import { err, ok, type Result } from "../domain/result.js"; import { createHash } from "node:crypto"; -import type { BrowserScenario } from "../domain/browserScenario.js"; +import { + browserScenarioSecretAvailability, + type BrowserScenario, +} from "../domain/browserScenario.js"; import type { BrowserScenarioUrl, BrowserScenarioValue, @@ -287,34 +290,14 @@ export class BrowserScenarioSecrets { scenario: BrowserScenario, environment: Readonly>, ): Result { + const available = browserScenarioSecretAvailability(scenario, environment); + if (!available.ok) return err(available.error); const values = new Map(); - for (const [index, declaration] of scenario.secrets.entries()) { - const value = environment[declaration.environment_variable]; - if ( - !Object.hasOwn(environment, declaration.environment_variable) || - typeof value !== "string" - ) - return err( - new AnalysisInputError( - "capture_browser_scenario", - { - userMessage: `Set environment variable ${declaration.environment_variable} for browser scenario secret ${declaration.secret_id} in the environment that starts REA, then retry.`, - }, - [ - { - path: ["secrets", index, "environment_variable"], - reason: "missing_argument", - message: `Environment variable ${declaration.environment_variable} is not set for declared secret ${declaration.secret_id}.`, - expected: { - secret_id: declaration.secret_id, - environment_variable: declaration.environment_variable, - }, - }, - ], - ), - ); - values.set(declaration.secret_id, value); - } + for (const declaration of scenario.secrets) + values.set( + declaration.secret_id, + environment[declaration.environment_variable]!, + ); return ok(new BrowserScenarioSecrets(values)); } diff --git a/src/cliBrowserScenarioCommands.ts b/src/cliBrowserScenarioCommands.ts index 20e8923203..b37969f2e9 100644 --- a/src/cliBrowserScenarioCommands.ts +++ b/src/cliBrowserScenarioCommands.ts @@ -1,7 +1,10 @@ import { Cli } from "incur"; import { z } from "zod"; -import { captureBrowserScenario } from "./application/BrowserScenarioCaptureService.js"; +import { + captureBrowserScenario, + planBrowserScenario, +} from "./application/BrowserScenarioCaptureService.js"; import { withCommandCancellation } from "./cli/commandCancellation.js"; import { CLI_COMMANDS } from "./cliCommandNames.js"; import { parseCliJsonInput, resolveCliJsonPaths } from "./cliJsonInput.js"; @@ -38,9 +41,20 @@ export const registerBrowserScenarioCommands = ( .string() .describe("Inline browser scenario JSON or JSON file path"), }), - run: ({ args }) => + options: z.object({ + dryRun: z + .boolean() + .default(false) + .describe( + "Validate the scenario and its secret environment variables, then stop before discovering, launching or attaching to a browser", + ), + }), + run: ({ args, options: { dryRun } }) => withCommandCancellation((signal) => logCliCommand(logger, CLI_COMMANDS.captureBrowserScenario, async () => { + const failure = dryRun + ? "Browser scenario validation failed" + : "Browser scenario capture failed"; const input = await parseCliJsonInput( args.inputJson, OPERATION, @@ -58,7 +72,12 @@ export const registerBrowserScenarioCommands = ( input.value, { cause: scenario.error }, ), + failure, ); + if (dryRun) { + const plan = planBrowserScenario(scenario.data, environment); + return plan.ok ? plan.value : cliError(plan.error, failure); + } // Load Playwright only for this command, not during CLI startup. const provider = options.createProvider === undefined @@ -74,7 +93,7 @@ export const registerBrowserScenarioCommands = ( result = await captureBrowserScenario(provider, scenario.data, { signal, }); - return result.ok ? result.value : cliError(result.error); + return result.ok ? result.value : cliError(result.error, failure); } finally { try { await provider?.close?.(); @@ -96,7 +115,8 @@ export const registerBrowserScenarioCommands = ( const cliError = ( error: Parameters[0], + label: string, ): JsonValue => ({ - error: "Browser scenario capture failed", + error: label, ...projectAnalysisError(error), }); diff --git a/src/domain/browserScenario.ts b/src/domain/browserScenario.ts index 2e27c445cd..05b926b36e 100644 --- a/src/domain/browserScenario.ts +++ b/src/domain/browserScenario.ts @@ -1,5 +1,8 @@ import { z } from "zod"; +import { AnalysisInputError } from "./analysisErrorCore.js"; +import { err, ok, type Result } from "./result.js"; + import { webRuntimeCommandTimeoutSchema } from "./browserDeadline.js"; import { browserScenarioActionSchema, @@ -209,3 +212,38 @@ export const browserScenarioSchema = browserScenarioInputSchema.superRefine( validateBrowserScenario, ); export type BrowserScenario = z.infer; + +/** + * Refuse a declared secret whose environment variable is absent. Execution and + * dry runs share this rule. It checks presence only and never reads a value. + */ +export const browserScenarioSecretAvailability = ( + scenario: BrowserScenario, + environment: Readonly>, +): Result => { + for (const [index, declaration] of scenario.secrets.entries()) + if ( + !Object.hasOwn(environment, declaration.environment_variable) || + typeof environment[declaration.environment_variable] !== "string" + ) + return err( + new AnalysisInputError( + "capture_browser_scenario", + { + userMessage: `Set environment variable ${declaration.environment_variable} for browser scenario secret ${declaration.secret_id} in the environment that starts REA, then retry.`, + }, + [ + { + path: ["secrets", index, "environment_variable"], + reason: "missing_argument", + message: `Environment variable ${declaration.environment_variable} is not set for declared secret ${declaration.secret_id}.`, + expected: { + secret_id: declaration.secret_id, + environment_variable: declaration.environment_variable, + }, + }, + ], + ), + ); + return ok(undefined); +}; diff --git a/tests/boundary/cli/browserScenarioDryRun.test.ts b/tests/boundary/cli/browserScenarioDryRun.test.ts new file mode 100644 index 0000000000..6d46aa27bd --- /dev/null +++ b/tests/boundary/cli/browserScenarioDryRun.test.ts @@ -0,0 +1,201 @@ +import { Cli } from "incur"; +import pino from "pino"; +import { describe, expect, it, vi } from "vitest"; + +import type { BrowserScenarioCapturePort } from "../../../src/application/BrowserScenarioCapturePort.js"; +import { BrowserScenarioSecrets } from "../../../src/browser/BrowserScenarioSecrets.js"; +import { registerBrowserScenarioCommands } from "../../../src/cliBrowserScenarioCommands.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { browserScenarioSchema } from "../../../src/domain/browserScenario.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +const SECRET_VALUE = "dry-run-secret-value-1234"; + +const scenario = { + browser: { mode: "launch", executable_path: "/rea-missing/browser" }, + start_url: { + url: "https://example.test/login", + query: [ + { + name: "token", + value: { source: "secret", secret_id: "api_token" }, + }, + ], + }, + actions: [ + { step_id: "settle", action: "wait_for_timeout", duration_ms: 1 }, + { + step_id: "account", + action: "goto", + destination: { url: "https://example.test/account" }, + wait_until: "load", + }, + ], + secrets: [{ secret_id: "api_token", environment_variable: "REA_DRY_TOKEN" }], +}; + +/** One in-process CLI whose provider factory records every invocation. */ +const scenarioCli = (environment: Readonly>) => { + const createProvider = vi.fn((): BrowserScenarioCapturePort => { + throw new Error("A dry run must not create a browser provider"); + }); + const cli = Cli.create("rea-scenario-dry-run-test", { sync: false }); + registerBrowserScenarioCommands(cli, pino({ level: "silent" }), environment, { + createProvider, + }); + const serve = async (input: unknown, ...flags: string[]) => { + let stdout = ""; + await cli.serve( + ["capture-browser-scenario", JSON.stringify(input), ...flags, "--json"], + { + env: {}, + stdout: (text) => { + stdout += text; + }, + exit: () => undefined, + }, + ); + return { stdout, json: JSON.parse(stdout) as Record }; + }; + return { createProvider, serve }; +}; + +describe("capture-browser-scenario --dry-run", () => { + it("returns the plan without creating a provider or reading secret values into output", async () => { + const { createProvider, serve } = scenarioCli({ + REA_DRY_TOKEN: SECRET_VALUE, + }); + const { stdout, json } = await serve(scenario, "--dry-run"); + expect(json).toEqual({ + operation: "capture_browser_scenario", + dry_run: true, + browser_mode: "launch", + start_origin: "https://example.test", + steps: [ + { step_id: "scenario_start", action: "goto_start" }, + { step_id: "settle", action: "wait_for_timeout" }, + { step_id: "account", action: "goto" }, + ], + secrets: [ + { + secret_id: "api_token", + environment_variable: "REA_DRY_TOKEN", + environment_value: "present", + }, + ], + not_checked: expect.arrayContaining(["browser launch or attachment"]), + }); + expect(stdout).not.toContain(SECRET_VALUE); + expect(createProvider).not.toHaveBeenCalled(); + }); + + it("refuses a missing secret environment variable with the execution rule's error", async () => { + const { createProvider, serve } = scenarioCli({}); + const { json } = await serve(scenario, "--dry-run"); + const parsed = browserScenarioSchema.parse(scenario); + const execution = BrowserScenarioSecrets.resolve(parsed, {}); + if (execution.ok) throw new Error("Missing secret resolved"); + expect(json).toEqual({ + error: "Browser scenario validation failed", + ...projectAnalysisError(execution.error), + }); + expect(json).toMatchObject({ + code: "invalid_request", + details: { + issues: [ + { + path: ["secrets", 0, "environment_variable"], + reason: "missing_argument", + }, + ], + }, + }); + // An inherited or non-string value is not an available secret. + const inherited = Object.create({ REA_DRY_TOKEN: SECRET_VALUE }) as Record< + string, + string + >; + expect( + (await scenarioCli(inherited).serve(scenario, "--dry-run")).json, + ).toMatchObject({ code: "invalid_request" }); + expect(createProvider).not.toHaveBeenCalled(); + }); + + it("reports schema and semantic errors exactly as execution does", async () => { + const invalid = { + ...scenario, + actions: [...scenario.actions, scenario.actions[0]], + secrets: [], + }; + const dry = scenarioCli({ REA_DRY_TOKEN: SECRET_VALUE }); + const executed = scenarioCli({ REA_DRY_TOKEN: SECRET_VALUE }); + const { json: dryJson } = await dry.serve(invalid, "--dry-run"); + const { json: executedJson } = await executed.serve(invalid); + expect(dryJson).toMatchObject({ + error: "Browser scenario validation failed", + code: "invalid_request", + details: { + issues: expect.arrayContaining([ + expect.objectContaining({ + path: ["actions"], + message: "Action step IDs must be unique", + }), + expect.objectContaining({ + path: ["secrets"], + message: "Secret reference api_token is not declared", + }), + ]), + }, + }); + const { error: dryLabel, ...dryProjection } = dryJson; + const { error: executedLabel, ...executedProjection } = executedJson; + expect([dryLabel, executedLabel]).toEqual([ + "Browser scenario validation failed", + "Browser scenario capture failed", + ]); + expect(dryProjection).toEqual(executedProjection); + expect(dry.createProvider).not.toHaveBeenCalled(); + expect(executed.createProvider).not.toHaveBeenCalled(); + }); + + it("still runs execution when the flag is omitted", async () => { + const { createProvider, serve } = scenarioCli({ + REA_DRY_TOKEN: SECRET_VALUE, + }); + const { json } = await serve(scenario); + expect(json).not.toHaveProperty("dry_run"); + expect(createProvider).toHaveBeenCalledOnce(); + }); +}); + +cliTest( + "validates a scenario through the compiled CLI without a browser", + async ({ cli }) => { + const valid = await cli.run({ + arguments: [ + "capture-browser-scenario", + JSON.stringify(scenario), + "--dry-run", + "--json", + ], + environment: { REA_LOG_LEVEL: "silent", REA_DRY_TOKEN: SECRET_VALUE }, + }); + expect(valid.exitCode).toBe(0); + expect(valid.json).toMatchObject({ dry_run: true, browser_mode: "launch" }); + expect(valid.stdout).not.toContain(SECRET_VALUE); + const missing = await cli.run({ + arguments: [ + "capture-browser-scenario", + JSON.stringify(scenario), + "--dry-run", + "--json", + ], + environment: { REA_LOG_LEVEL: "silent" }, + }); + expect(missing.exitCode).not.toBe(0); + expect(missing.json).toMatchObject({ + code: "invalid_request", + details: { issues: [{ path: ["secrets", 0, "environment_variable"] }] }, + }); + }, +);