From 55cce6fc11a4edc32b4a6d4c2d8fd4f2ed2a4f18 Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:26:57 +0800 Subject: [PATCH 1/2] test(cli): retain recovery guidance in filtered failures (#5536) Preserve typed input-contract recovery and source diagnostics through filtered CLI output in JSON, TOON, YAML and Markdown. Update legacy remediation-text assertions introduced by the concurrent main update. Refs #5431 and #5470. --- .../cli/filteredFailureOutput.test.ts | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/boundary/cli/filteredFailureOutput.test.ts b/tests/boundary/cli/filteredFailureOutput.test.ts index 36d4b1699c..684b7b518d 100644 --- a/tests/boundary/cli/filteredFailureOutput.test.ts +++ b/tests/boundary/cli/filteredFailureOutput.test.ts @@ -50,15 +50,21 @@ const runCli = async (arguments_: readonly string[]) => { }; const expectMissingInputFailureText = (text: string, path: string): void => { + const normalizedText = text.replace(/\s+/g, " "); for (const value of [ "invalid_request", "invalid_input", - "Correct the listed arguments and retry.", + "Read the declared input schema or CLI usage", + "trace-application-feature", + "tools/list", + "Correct the listed fields, then retry.", + "Do not repeat the unchanged call.", + "inspect_input_contract", path, "read-failed", "ENOENT", ]) - expect(text).toContain(value); + expect(normalizedText).toContain(value); }; it.each(["json", "toon", "yaml", "md"] as const)( @@ -81,13 +87,22 @@ it.each(["json", "toon", "yaml", "md"] as const)( expect(result.output.failed).toBe(false); expect(result.fallback).toBe(""); expect(result.output.handled).toBe(true); - if (format === "json") { - expect(JSON.parse(result.text)).toMatchObject({ + expectMissingInputFailureText(result.text, missingPath); + if (format === "json" || format === "yaml") { + const document = + format === "json" + ? JSON.parse(result.text) + : parseYaml(result.text); + expect(document).toMatchObject({ code: "invalid_request", category: "invalid_input", retryable: true, remediation: { - action: "Correct the listed arguments and retry.", + action: expect.stringContaining("tools/list"), + }, + recovery: { + kind: "inspect_input_contract", + operation: "trace-application-feature", }, details: { input_path: missingPath, @@ -99,7 +114,7 @@ it.each(["json", "toon", "yaml", "md"] as const)( ], }, }); - } else expectMissingInputFailureText(result.text, missingPath); + } } finally { process.exitCode = previousExitCode; } @@ -127,6 +142,13 @@ it.each(["json", "toon", "yaml", "md"] as const)( code: "invalid_request", category: "invalid_input", retryable: true, + remediation: { + action: expect.stringContaining("tools/list"), + }, + recovery: { + kind: "inspect_input_contract", + operation: "trace-application-feature", + }, details: { input_path: missingPath, input_reason: "read-failed", From eac296a7da3356f1c5342155d635975ddd89cba6 Mon Sep 17 00:00:00 2001 From: Sagar Kewat Date: Tue, 20 Oct 2026 20:02:54 +0530 Subject: [PATCH 2/2] fix(browser): reject overflowing scenario timers (#3515) Reject browser scenario waits and action deadlines beyond the underlying signed 32-bit timer interval in the shared input contract. Preserve maximum legal values and omitted deadlines; prevent unsupported input from opening a browser. Strengthen the rejection oracle and verify public CLI/MCP canonical validation in both schema profiles, actual Chromium timing/cancellation/cleanup, Evidence export and compatibility with current main. Fixes #3392. --- docs/browser-scenario-contract.md | 5 + src/domain/browserScenario.test.ts | 57 ++++++++++++ src/domain/browserScenarioValues.ts | 5 + ...browserScenarioInputDiagnosticsMcp.test.ts | 91 +++++++++++++++---- 4 files changed, 141 insertions(+), 17 deletions(-) diff --git a/docs/browser-scenario-contract.md b/docs/browser-scenario-contract.md index 067fb94091..787f91bcfa 100644 --- a/docs/browser-scenario-contract.md +++ b/docs/browser-scenario-contract.md @@ -107,6 +107,11 @@ Through MCP, pass the same object as the `capture_browser_scenario` arguments. The final step's DOM and accessibility artifacts retain the page state at that capture time inline. REA still launches and cleans up the browser profile. +Action `timeout_ms` and `wait_for_timeout.duration_ms` accept integers from 1 +through 2,147,483,647 milliseconds, matching the underlying timer representation. +Larger values receive an input error before browser launch. Omit `timeout_ms` +to keep an action pending until completion or request cancellation. + The click's `timeout_ms` covers the click action. A non-navigating `fetch` or delayed render can finish later, so use `wait_for` to observe a page-specific ready condition before final capture. A page that updates synchronously can diff --git a/src/domain/browserScenario.test.ts b/src/domain/browserScenario.test.ts index 96cf35590c..5d393a6924 100644 --- a/src/domain/browserScenario.test.ts +++ b/src/domain/browserScenario.test.ts @@ -196,3 +196,60 @@ it("accepts caller-selected viewport sizes and click counts without ceilings", ( }); expect(parsed.actions[0]).toMatchObject({ click_count: 4 }); }); + +it.each([ + { + field: "duration_ms", + action: { action: "wait_for_timeout", duration_ms: 2_147_483_648 }, + }, + { + field: "timeout_ms", + action: { + action: "wait_for", + locator: { kind: "css", selector: "#btn" }, + state: "visible", + timeout_ms: 2_147_483_648, + }, + }, +])("rejects $field at its precise owning field", ({ field, action }) => { + const parsed = browserScenarioSchema.safeParse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [{ step_id: "overflow", ...action }], + }); + if (parsed.success) + throw new Error("Expected unrepresentable timer rejection"); + expect(parsed.error.issues).toMatchObject([ + { code: "too_big", maximum: 2_147_483_647, path: ["actions", 0, field] }, + ]); +}); + +it("preserves the largest representable timers and an omitted action deadline", () => { + const parsed = browserScenarioSchema.parse({ + browser: { mode: "launch", executable_path: "/opt/chromium" }, + start_url: { url: "https://app.example.test/" }, + actions: [ + { + step_id: "max-wait", + action: "wait_for_timeout", + duration_ms: 2_147_483_647, + }, + { + step_id: "max-deadline", + action: "wait_for", + locator: { kind: "css", selector: "#btn" }, + state: "visible", + timeout_ms: 2_147_483_647, + }, + { + step_id: "unbounded", + action: "wait_for", + locator: { kind: "css", selector: "#btn" }, + state: "visible", + }, + ], + }); + expect(parsed.actions[0]).toMatchObject({ duration_ms: 2_147_483_647 }); + expect(parsed.actions[1]).toMatchObject({ timeout_ms: 2_147_483_647 }); + expect(parsed.actions[2]).not.toHaveProperty("timeout_ms"); +}); diff --git a/src/domain/browserScenarioValues.ts b/src/domain/browserScenarioValues.ts index 6491a664e4..4095793312 100644 --- a/src/domain/browserScenarioValues.ts +++ b/src/domain/browserScenarioValues.ts @@ -157,12 +157,16 @@ const locatorSchema = z.discriminatedUnion("kind", [ }), ]); +// Playwright forwards these intervals to Node timers. +const MAX_BROWSER_SCENARIO_TIMEOUT_MS = 2_147_483_647; + const stepBase = { step_id: scenarioIdentifierSchema, timeout_ms: z .number() .int() .min(1) + .max(MAX_BROWSER_SCENARIO_TIMEOUT_MS) .optional() .describe( "Optional action deadline in milliseconds; omitted means wait until completion or request cancellation.", @@ -219,6 +223,7 @@ export const browserScenarioActionSchema = z.discriminatedUnion("action", [ .number() .int() .min(1) + .max(MAX_BROWSER_SCENARIO_TIMEOUT_MS) .describe( "Wait this many milliseconds; the wait ends early on cancellation.", ), diff --git a/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts b/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts index 42fc7020cb..e39bbf7a61 100644 --- a/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts +++ b/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts @@ -12,8 +12,12 @@ import { cliTest } from "../../support/cli/cliFixture.js"; const scenarios = [ { name: "undeclared secret references", - issuePath: ["secrets"], - issueMessage: "Secret reference pw is not declared", + fullSchemaValid: true, + issue: { + path: ["secrets"], + reason: "invalid_value", + message: "Secret reference pw is not declared", + }, actions: [ { step_id: "pw", @@ -25,18 +29,57 @@ const scenarios = [ }, { name: "duplicate action step IDs", - issuePath: ["actions"], - issueMessage: "Action step IDs must be unique", + fullSchemaValid: true, + issue: { + path: ["actions"], + reason: "invalid_value", + message: "Action step IDs must be unique", + }, actions: [ { step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }, { step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }, ], }, + { + name: "overflowing wait duration", + fullSchemaValid: false, + issue: { + path: ["actions", 0, "duration_ms"], + reason: "out_of_range", + maximum: 2_147_483_647, + }, + actions: [ + { + step_id: "wait", + action: "wait_for_timeout", + duration_ms: 2_147_483_648, + }, + ], + }, + { + name: "overflowing action deadline", + fullSchemaValid: false, + issue: { + path: ["actions", 0, "timeout_ms"], + reason: "out_of_range", + maximum: 2_147_483_647, + }, + actions: [ + { + step_id: "visible", + action: "wait_for", + locator: { kind: "css", selector: "#btn" }, + state: "visible", + timeout_ms: 2_147_483_648, + }, + ], + }, ] as const; -cliTest( - "projects refined browser scenario validation into CLI-equivalent diagnostics over stdio MCP", - async ({ cli }) => { +cliTest.for(["full", "compact"] as const)( + "projects browser scenario input diagnostics over stdio MCP (%s)", + { timeout: 60_000 }, + async (profile, { cli }) => { const home = await createTestTempDirectory( "rea-browser-scenario-mcp-home-", ); @@ -52,6 +95,7 @@ cliTest( PATH: process.env.PATH ?? "", HOME: home, REA_LOG_LEVEL: "silent", + REA_MCP_INPUT_SCHEMA_PROFILE: profile, }, stderr: "pipe", }); @@ -81,7 +125,7 @@ cliTest( expect( validate(input), `${scenario.name}: ${JSON.stringify(validate.errors)}`, - ).toBe(true); + ).toBe(scenario.fullSchemaValid || profile === "compact"); const cliResult = await cli.run({ arguments: [ @@ -111,13 +155,7 @@ cliTest( }, details: { operation: "capture_browser_scenario", - issues: [ - { - path: scenario.issuePath, - reason: "invalid_value", - message: scenario.issueMessage, - }, - ], + issues: [scenario.issue], }, }); @@ -126,12 +164,31 @@ cliTest( arguments: input, }); expect(mcpResult.structuredContent).toBeUndefined(); - expect(parseMcpToolError(mcpResult).error).toEqual(cliDiagnostic); + const mcpError = parseMcpToolError(mcpResult).error; + if (scenario.fullSchemaValid) expect(mcpError).toEqual(cliDiagnostic); + else { + // The SDK validates the canonical schema before invoking the handler. + expect(mcpError).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: true, + recovery: { kind: "inspect_input_contract", operation: tool.name }, + details: { + operation: tool.name, + issues: [ + { + message: expect.stringContaining( + `${scenario.issue.path.join(".")}: Too big: expected number to be <=2147483647`, + ), + }, + ], + }, + }); + } await expect(client.ping()).resolves.toEqual({}); } } finally { await Promise.all([client.close(), transport.close()]); } }, - 60_000, );