From d5c43d65f0f6a1de8fb2eb5f2c1940a6ea2af20c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 7 Oct 2026 23:04:54 +0800 Subject: [PATCH] fix(cli): preserve output and comparison input validation (#981) --- src/cli/browserAdvancedCommands.ts | 45 ++++++++++++++---- src/cliOutput.ts | 31 ++++++++----- tests/boundary/cli/browserCli.test.ts | 51 +++++++++++++++++++++ tests/boundary/cli/cliOutput.test.ts | 66 +++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 21 deletions(-) diff --git a/src/cli/browserAdvancedCommands.ts b/src/cli/browserAdvancedCommands.ts index 2d299503f..0fd49960e 100644 --- a/src/cli/browserAdvancedCommands.ts +++ b/src/cli/browserAdvancedCommands.ts @@ -10,7 +10,10 @@ import { } from "../application/BrowserObservationService.js"; import { CdpBrowserProvider } from "../browser/CdpBrowserProvider.js"; import { logCliCommand } from "../cliLogging.js"; -import { AnalysisInputError } from "../domain/analysisErrorCore.js"; +import { + AnalysisInputError, + type AnalysisInputIssue, +} from "../domain/analysisErrorCore.js"; import { browserCaptureComparisonInputSchema } from "../domain/browserCaptureComparison.js"; import { discoverWebMcpToolsInputSchema } from "../domain/webMcpDiscovery.js"; import { @@ -89,19 +92,32 @@ const registerCaptureDiff = ( }), run: ({ args, options }) => logCliCommand(logger, "compare-web-captures", async () => { - const before = parseJson(args.beforeJson); - const after = parseJson(args.afterJson); + const before = safeParseJson(args.beforeJson); + const after = safeParseJson(args.afterJson); + const normalization = safeParseJson(options.normalizationJson); + if (!before.ok) + return inputError("compare_web_captures", [ + invalidJsonIssue("before"), + ]); + if (!after.ok) + return inputError("compare_web_captures", [ + invalidJsonIssue("after"), + ]); + if (!normalization.ok) + return inputError("compare_web_captures", [ + invalidJsonIssue("normalization"), + ]); const scenarioComparison = browserCaptureComparisonInputSchema.safeParse({ - before_scenario: before, - after_scenario: after, - normalization: parseJson(options.normalizationJson), + before_scenario: before.value, + after_scenario: after.value, + normalization: normalization.value, }); const parsed = scenarioComparison.success ? scenarioComparison : browserCaptureComparisonInputSchema.safeParse({ - before, - after, + before: before.value, + after: after.value, }); if (!parsed.success) return inputError("compare_web_captures"); const result = await compareWebCaptureEvidence( @@ -186,5 +202,14 @@ const parseJson = (value: string): unknown => { return parsed.ok ? parsed.value : undefined; }; -const inputError = (operation: string): JsonValue => - browserCliError(new AnalysisInputError(operation)); +const invalidJsonIssue = (field: string): AnalysisInputIssue => ({ + path: [field], + reason: "invalid_format", + expected: "JSON", +}); + +const inputError = ( + operation: string, + issues: readonly AnalysisInputIssue[] = [], +): JsonValue => + browserCliError(new AnalysisInputError(operation, undefined, issues)); diff --git a/src/cliOutput.ts b/src/cliOutput.ts index 96f65a7e7..063a6d452 100644 --- a/src/cliOutput.ts +++ b/src/cliOutput.ts @@ -6,6 +6,7 @@ const UNSUPPORTED_OUTPUT_COMBINATION_MESSAGE = "Token windows cannot preserve structured output. Remove --token-limit/--token-offset and use --filter-output or command pagination."; type StructuredOutputFormat = "json" | "jsonl" | "yaml"; +const INCUR_OUTPUT_FORMATS = new Set(["toon", "json", "yaml", "md", "jsonl"]); export type CliOutputArgumentValidation = | { readonly ok: true } @@ -21,6 +22,7 @@ type JsonRecord = Record; interface ParsedCliOutputArguments { readonly filterOutput: boolean; readonly format: string; + readonly parseError: boolean; readonly tokenWindow: boolean; } @@ -50,40 +52,45 @@ const parseCliOutputArguments = ( } if (argument === "--format") { const value = arguments_[index + 1]; - if (value !== undefined) { + if (value) { + // Incur stops parsing at an invalid format before applying any later + // output controls. Let it report that malformed flag itself. + if (!INCUR_OUTPUT_FORMATS.has(value)) + return { filterOutput, format, parseError: true, tokenWindow: false }; format = value; index += 1; } continue; } if (argument === "--filter-output") { - if (arguments_[index + 1] !== undefined) { + if (arguments_[index + 1]) { filterOutput = true; index += 1; } continue; } if (argument === "--token-limit" || argument === "--token-offset") { - if (arguments_[index + 1] !== undefined) { + const value = arguments_[index + 1]; + if (value) { + const numericValue = Number(value); + if (!Number.isFinite(numericValue) || value.trim() === "") + return { filterOutput, format, parseError: true, tokenWindow: false }; tokenWindow = true; index += 1; } continue; } - if ( - argument?.startsWith("--token-limit=") || - argument?.startsWith("--token-offset=") - ) - tokenWindow = true; } - return { filterOutput, format, tokenWindow }; + return { filterOutput, format, parseError: false, tokenWindow }; }; /** Reject text-window controls that would corrupt a structured document. */ export const validateCliOutputArguments = ( arguments_: readonly string[], ): CliOutputArgumentValidation => { - const { format, tokenWindow } = parseCliOutputArguments(arguments_); + const { format, parseError, tokenWindow } = + parseCliOutputArguments(arguments_); + if (parseError) return { ok: true }; if ( tokenWindow && (format === "json" || format === "jsonl" || format === "yaml") @@ -101,7 +108,9 @@ export const validateCliOutputArguments = ( export const renderEmptyFilteredCliOutput = ( arguments_: readonly string[], ): string | undefined => { - const { filterOutput, format } = parseCliOutputArguments(arguments_); + const { filterOutput, format, parseError } = + parseCliOutputArguments(arguments_); + if (parseError) return undefined; if ( filterOutput && (format === "json" || format === "jsonl" || format === "yaml") diff --git a/tests/boundary/cli/browserCli.test.ts b/tests/boundary/cli/browserCli.test.ts index 6aab7fe3e..9ddd0ac9c 100644 --- a/tests/boundary/cli/browserCli.test.ts +++ b/tests/boundary/cli/browserCli.test.ts @@ -108,6 +108,57 @@ describe("browser CLI parity", () => { alignment: { status: "aligned", aligned_steps: 2 }, }, }); + const intentionalPolicy = await runCli( + [ + "compare-web-captures", + JSON.stringify(scenarioCapture), + JSON.stringify(scenarioCapture), + "--normalization-json", + JSON.stringify({ + rules: [ + { + rule_id: "domain", + artifacts: ["url"], + match: "app.example.test", + replacement: "normalized.test", + }, + ], + }), + "--json", + ], + process.env, + ); + expect(intentionalPolicy).toMatchObject({ + normalized_result: { + normalization: { rules: [{ rule_id: "domain" }] }, + }, + }); + + const malformedPolicy = await runCli( + [ + "compare-web-captures", + JSON.stringify(scenarioCapture), + JSON.stringify(scenarioCapture), + "--normalization-json", + "{invalid-policy", + "--json", + ], + process.env, + ); + expect(malformedPolicy).toMatchObject({ + error: "Browser observation failed", + code: "invalid_request", + details: { + operation: "compare_web_captures", + issues: [ + { + path: ["normalization"], + reason: "invalid_format", + expected: "JSON", + }, + ], + }, + }); }, INTEGRATION_TEST_TIMEOUT_MS, ); diff --git a/tests/boundary/cli/cliOutput.test.ts b/tests/boundary/cli/cliOutput.test.ts index 22fa1b0a6..c31ebf47a 100644 --- a/tests/boundary/cli/cliOutput.test.ts +++ b/tests/boundary/cli/cliOutput.test.ts @@ -57,7 +57,9 @@ describe("CLI output argument and sanitization boundary", () => { validateCliOutputArguments(["providers", "--token-count", "--json"]), ).toEqual({ ok: true }); }); +}); +describe("CLI output valued-flag parsing", () => { workspaceCliTest( "preserves complete JSON errors when native builtin flags follow unusual arguments", async ({ cli }) => { @@ -92,6 +94,70 @@ describe("CLI output argument and sanitization boundary", () => { CLI_INTEGRATION_TIMEOUT_MS, ); + workspaceCliTest( + "does not treat empty or equals-style output values as active builtins", + async ({ cli }) => { + for (const arguments_ of [ + ["capabilities", "--json", "--format", ""], + ["capabilities", "--json", "--filter-output", ""], + ["capabilities", "--json", "--token-limit", ""], + ["capabilities", "--json", "--token-limit=5"], + ]) { + const result = await cli.run({ arguments: arguments_ }); + expect(result.exitCode).toBe(1); + expect(result.json).toBeDefined(); + expect(result.stdout).not.toContain("[truncated:"); + expect(JSON.stringify(result.json)).not.toContain( + "UNSUPPORTED_OUTPUT_COMBINATION", + ); + } + const validTokenWindow = await cli.run({ + arguments: ["capabilities", "--json", "--token-limit", "5"], + }); + expect(validTokenWindow.exitCode).toBe(1); + expect(validTokenWindow.json).toMatchObject({ + ok: false, + error: { code: "UNSUPPORTED_OUTPUT_COMBINATION" }, + }); + const emptyFormatWithTokenWindow = await cli.run({ + arguments: [ + "capabilities", + "--json", + "--format", + "", + "--token-limit", + "5", + ], + }); + expect(emptyFormatWithTokenWindow.exitCode).toBe(1); + expect(emptyFormatWithTokenWindow.stdout).not.toContain("[truncated:"); + expect(emptyFormatWithTokenWindow.json).toMatchObject({ + ok: false, + error: { code: "UNSUPPORTED_OUTPUT_COMBINATION" }, + }); + + for (const arguments_ of [ + ["capabilities", "--json", "--token-limit", "not-a-number"], + [ + "capabilities", + "--format", + "unsupported-format", + "--json", + "--token-limit", + "5", + ], + ]) { + const result = await cli.run({ arguments: arguments_ }); + expect(result.exitCode).toBe(1); + expect(result.stdout).not.toContain("UNSUPPORTED_OUTPUT_COMBINATION"); + expect(result.stdout).not.toContain("[truncated:"); + } + }, + CLI_INTEGRATION_TIMEOUT_MS, + ); +}); + +describe("CLI output argument and sanitization boundary", () => { it("preserves normal output and sanitizes text and JSON validation errors", () => { expect(sanitizeCliOutput("result: ok\n")).toBe("result: ok\n"); expect(sanitizeCliOutput("result: VALIDATION_ERROR\n")).toBe(