fix(cli): preserve output and comparison input validation (#981)
This commit is contained in:
parent
a45fe86a39
commit
d5c43d65f0
|
|
@ -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));
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>;
|
|||
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")
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue