fix(cli): stop bare --frame-check from swallowing the next flag (#2966)

citty parses string options greedily, so a bare --frame-check consumed
the following flag as its value (--caption-zone silently dropped,
--json disabled) and failed with an error blaming --caption-zone.

Normalize rawArgs at the check command boundary (bare --frame-check
followed by a flag or at end becomes --frame-check=), attribute
frame-check spec errors to --frame-check, and detect dash-prefixed
values with corrective guidance.

Fixes #2965
This commit is contained in:
Xinyu YANG
2026-08-04 04:03:04 +02:00
committed by GitHub
parent 127eb19371
commit 70cc4f59a1
2 changed files with 194 additions and 97 deletions
+69
View File
@@ -359,6 +359,66 @@ it("parses the caption-zone grammar and enables the frame gate", async () => {
); );
}); });
it("preserves caption-zone after bare --frame-check", async () => {
const { report } = await runScenario(fakeDriver());
const runPipeline = vi.fn(async (_project: ProjectDir, _options: CheckOptions) => report);
vi.spyOn(console, "log").mockImplementation(() => undefined);
const command = createCheckCommand({
resolveProject: () => PROJECT,
runPipeline,
withMeta: (value) => value,
});
await runCommand(command, {
rawArgs: [
"--frame-check",
"--caption-zone",
"x0=0;y0=.82;x1=1;y1=1;severity=error;seek=.25,1",
"--json",
],
});
expect(runPipeline).toHaveBeenCalledWith(
PROJECT,
expect.objectContaining({
captionZone: {
x0: 0,
y0: 0.82,
x1: 1,
y1: 1,
severity: "error",
seek: [0.25, 1],
},
frameCheck: {},
}),
);
});
it("preserves --json after bare --frame-check", async () => {
const { report } = await runScenario(fakeDriver());
const runPipeline = vi.fn(async (_project: ProjectDir, _options: CheckOptions) => report);
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
const command = createCheckCommand({
resolveProject: () => PROJECT,
runPipeline,
withMeta: (value) => value,
});
await runCommand(command, {
rawArgs: ["--snapshots", "--samples", "15", "--frame-check", "--json"],
});
expect(runPipeline).toHaveBeenCalledWith(
PROJECT,
expect.objectContaining({
samples: 15,
snapshots: true,
frameCheck: {},
}),
);
expect(log).toHaveBeenCalledWith(expect.stringContaining('"ok"'));
});
it("threads --no-proxy into the browser check options", async () => { it("threads --no-proxy into the browser check options", async () => {
const { report } = await runScenario(fakeDriver()); const { report } = await runScenario(fakeDriver());
const runPipeline = vi.fn(async (_project: ProjectDir, _options: CheckOptions) => report); const runPipeline = vi.fn(async (_project: ProjectDir, _options: CheckOptions) => report);
@@ -1166,6 +1226,15 @@ describe("frame-check flag grammar", () => {
expect(() => parseFrameCheck("tol=4px")).toThrow("Invalid --frame-check"); expect(() => parseFrameCheck("tol=4px")).toThrow("Invalid --frame-check");
expect(() => parseFrameCheck("tol=2garbage")).toThrow("Invalid --frame-check"); expect(() => parseFrameCheck("tol=2garbage")).toThrow("Invalid --frame-check");
}); });
it("attributes swallowed option values to --frame-check", async () => {
const { parseFrameCheck } = await import("./check.js");
expect(() => parseFrameCheck("--json")).toThrow(
'Invalid --frame-check: value "--json" appears to have swallowed the next option; use --frame-check= or move --frame-check to the end',
);
expect(() => parseFrameCheck("severity")).toThrow("Invalid --frame-check");
});
}); });
describe("layout flag grammar", () => { describe("layout flag grammar", () => {
+49 -21
View File
@@ -1,4 +1,5 @@
import { defineCommand } from "citty"; import { defineCommand, parseArgs } from "citty";
import type { ArgsDef } from "citty";
import type { Example } from "./_examples.js"; import type { Example } from "./_examples.js";
import { parseAt } from "./layout.js"; import { parseAt } from "./layout.js";
import { c } from "../ui/colors.js"; import { c } from "../ui/colors.js";
@@ -38,16 +39,7 @@ const DEFAULT_COMMAND_DEPENDENCIES: CheckCommandDependencies = {
withMeta, withMeta,
}; };
export function createCheckCommand( const CHECK_COMMAND_ARGS = {
dependencies: CheckCommandDependencies = DEFAULT_COMMAND_DEPENDENCIES,
) {
return defineCommand({
meta: {
name: "check",
description:
"Run lint, runtime, layout, motion, and WCAG contrast verification in one browser session",
},
args: {
dir: { type: "positional", description: "Project directory", required: false }, dir: { type: "positional", description: "Project directory", required: false },
json: { type: "boolean", description: "Output agent-readable JSON", default: false }, json: { type: "boolean", description: "Output agent-readable JSON", default: false },
samples: { samples: {
@@ -132,8 +124,20 @@ export function createCheckCommand(
type: "string", type: "string",
description: 'Layout knobs: "proseCoverageFloor=0.05" (01; default 0.15).', description: 'Layout knobs: "proseCoverageFloor=0.05" (01; default 0.15).',
}, },
} satisfies ArgsDef;
export function createCheckCommand(
dependencies: CheckCommandDependencies = DEFAULT_COMMAND_DEPENDENCIES,
) {
return defineCommand({
meta: {
name: "check",
description:
"Run lint, runtime, layout, motion, and WCAG contrast verification in one browser session",
}, },
async run({ args }) { args: CHECK_COMMAND_ARGS,
async run({ rawArgs }) {
const args = parseArgs(normalizeFrameCheckRawArgs(rawArgs), CHECK_COMMAND_ARGS);
const asJson = args.json === true; const asJson = args.json === true;
try { try {
@@ -164,6 +168,14 @@ export function createCheckCommand(
}); });
} }
function normalizeFrameCheckRawArgs(rawArgs: string[]): string[] {
return rawArgs.map((arg, index) => {
if (arg !== "--frame-check") return arg;
const next = rawArgs[index + 1];
return next === undefined || next.startsWith("-") ? "--frame-check=" : arg;
});
}
function parseCheckOptions(args: Record<string, unknown>): CheckOptions { function parseCheckOptions(args: Record<string, unknown>): CheckOptions {
const maxTransitionSamples = positiveInteger(args["max-transition-samples"], 0); const maxTransitionSamples = positiveInteger(args["max-transition-samples"], 0);
return { return {
@@ -199,9 +211,10 @@ export function parseFrameCheck(value: unknown): FrameCheckOptions | undefined {
if (value === undefined || value === null || value === false) return undefined; if (value === undefined || value === null || value === false) return undefined;
if (value === true || value === "") return {}; if (value === true || value === "") return {};
if (typeof value !== "string") throw frameCheckError(); if (typeof value !== "string") throw frameCheckError();
if (value.startsWith("-")) throw swallowedOptionError("frame-check", value);
const fields = parseFrameCheckFields(value); const fields = parseFrameCheckFields(value);
const severity = captionSeverity(fields.get("severity")); const severity = captionSeverity(fields.get("severity"), frameCheckError);
const seek = captionSeeks(fields.get("seek")); const seek = captionSeeks(fields.get("seek"), frameCheckError);
const tol = parseFrameCheckTolerance(fields.get("tol")); const tol = parseFrameCheckTolerance(fields.get("tol"));
return { return {
...(severity ? { severity } : {}), ...(severity ? { severity } : {}),
@@ -213,7 +226,7 @@ export function parseFrameCheck(value: unknown): FrameCheckOptions | undefined {
function parseFrameCheckFields(value: string): Map<string, string> { function parseFrameCheckFields(value: string): Map<string, string> {
const fields = new Map<string, string>(); const fields = new Map<string, string>();
for (const part of value.split(";")) { for (const part of value.split(";")) {
const { key, entry } = parseCaptionField(part); const { key, entry } = parseCaptionField(part, frameCheckError);
if (!FRAME_CHECK_FIELDS.has(key) || fields.has(key)) throw frameCheckError(); if (!FRAME_CHECK_FIELDS.has(key) || fields.has(key)) throw frameCheckError();
fields.set(key, entry); fields.set(key, entry);
} }
@@ -233,6 +246,12 @@ function frameCheckError(): Error {
); );
} }
function swallowedOptionError(flag: string, value: string): Error {
return new Error(
`Invalid --${flag}: value "${value}" appears to have swallowed the next option; use --${flag}= or move --${flag} to the end`,
);
}
/** Parse `--layout "proseCoverageFloor=0.05"` (semicolon-separated key=value, like caption-zone). */ /** Parse `--layout "proseCoverageFloor=0.05"` (semicolon-separated key=value, like caption-zone). */
export function parseLayout(value: unknown): LayoutOptions | undefined { export function parseLayout(value: unknown): LayoutOptions | undefined {
if (value === undefined || value === null || value === false) return undefined; if (value === undefined || value === null || value === false) return undefined;
@@ -310,9 +329,12 @@ function parseCaptionFields(value: string): Map<string, string> {
return fields; return fields;
} }
function parseCaptionField(part: string): { key: string; entry: string } { function parseCaptionField(
part: string,
errorFactory: () => Error = captionZoneError,
): { key: string; entry: string } {
const separator = part.indexOf("="); const separator = part.indexOf("=");
if (separator <= 0) throw captionZoneError(); if (separator <= 0) throw errorFactory();
return { return {
key: part.slice(0, separator).trim(), key: part.slice(0, separator).trim(),
entry: part.slice(separator + 1).trim(), entry: part.slice(separator + 1).trim(),
@@ -345,17 +367,23 @@ function captionFraction(value: string | undefined): number | null {
return parsed !== null && parsed >= 0 && parsed <= 1 ? parsed : null; return parsed !== null && parsed >= 0 && parsed <= 1 ? parsed : null;
} }
function captionSeverity(value: string | undefined): "error" | "warning" | undefined { function captionSeverity(
value: string | undefined,
errorFactory: () => Error = captionZoneError,
): "error" | "warning" | undefined {
if (value === undefined) return undefined; if (value === undefined) return undefined;
if (value === "error" || value === "warning") return value; if (value === "error" || value === "warning") return value;
throw captionZoneError(); throw errorFactory();
} }
function captionSeeks(value: string | undefined): number[] | undefined { function captionSeeks(
value: string | undefined,
errorFactory: () => Error = captionZoneError,
): number[] | undefined {
if (value === undefined) return undefined; if (value === undefined) return undefined;
if (value === "") return []; if (value === "") return [];
const values = value.split(",").map(captionFraction); const values = value.split(",").map(captionFraction);
if (values.some((entry) => entry === null)) throw captionZoneError(); if (values.some((entry) => entry === null)) throw errorFactory();
return values.flatMap((entry) => (entry === null ? [] : entry)); return values.flatMap((entry) => (entry === null ? [] : entry));
} }