fix(scripts): reject short flag values in benchmark CLIs

This commit is contained in:
Vincent Koc 2026-06-21 22:11:13 +02:00
parent 409adfbe10
commit 4684bbba97
No known key found for this signature in database
6 changed files with 60 additions and 9 deletions

View file

@ -449,7 +449,7 @@ function parseFlagValue(flag: string): string | undefined {
return undefined;
}
const value = process.argv[idx + 1];
if (!value || value.startsWith("--")) {
if (!value || value.startsWith("-")) {
throw new Error(`${flag} requires a value`);
}
return value;
@ -462,7 +462,8 @@ function hasFlag(flag: string): boolean {
function parseRepeatableFlag(flag: string): string[] {
const values: string[] = [];
for (let i = 0; i < process.argv.length; i += 1) {
if (process.argv[i] === flag && process.argv[i + 1]) {
const value = process.argv[i + 1];
if (process.argv[i] === flag && value && !value.startsWith("-")) {
values.push(process.argv[i + 1]);
}
}
@ -474,7 +475,7 @@ function validateCliArgs(argv: readonly string[] = process.argv.slice(2)): void
const arg = argv[index];
if (VALUE_FLAGS.has(arg)) {
const value = argv[index + 1];
if (!value || value.startsWith("--")) {
if (!value || value.startsWith("-")) {
throw new Error(`${arg} requires a value`);
}
index += 1;
@ -1161,12 +1162,12 @@ function readBenchmarkComparisonForTesting(
}
async function main(): Promise<void> {
validateCliArgs();
if (hasFlag("--help")) {
printUsage();
return;
}
validateCliArgs();
const options = parseOptions();
if (options.compareBaseline || options.compareCandidate) {
if (!options.compareBaseline || !options.compareCandidate) {

View file

@ -33,7 +33,7 @@ class CliArgumentError extends Error {
function readValue(argv: string[], index: number, flag: string): string {
const value = argv[index + 1]?.trim() ?? "";
if (!value || value.startsWith("--")) {
if (!value || value.startsWith("-")) {
throw new CliArgumentError(`${flag} requires a value`);
}
return value;

View file

@ -53,12 +53,16 @@ Options:
function readOptionValue(argv: readonly string[], index: number, arg: string) {
const value = argv[index + 1] ?? "";
if (!value || value.startsWith("--")) {
if (!value || value.startsWith("-")) {
throw new Error(`${arg} requires a value`);
}
return value;
}
function isHelpRequest(argv: readonly string[]) {
return argv.length === 1 && (argv[0] === "--help" || argv[0] === "-h");
}
function parseOptions(argv: readonly string[]): ProducerOptions {
let artifactBase = "";
let repoRoot = process.cwd();
@ -698,7 +702,7 @@ export async function runUxMatrixEvidenceProducer(options: ProducerOptions) {
if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) {
(async () => {
const cliArgs = process.argv.slice(2);
if (cliArgs.includes("--help") || cliArgs.includes("-h")) {
if (isHelpRequest(cliArgs)) {
console.log(usage());
return;
}

View file

@ -36,6 +36,26 @@ describe("bench-cli-startup", () => {
expect(result.stderr).not.toContain("\n at ");
});
it("rejects short flag values before running benchmarks", () => {
expect(() => testing.validateCliArgs(["--output", "-h"])).toThrow("--output requires a value");
expect(() => testing.validateCliArgs(["--case", "-h"])).toThrow("--case requires a value");
const result = spawnSync(
process.execPath,
["--import", "tsx", "scripts/bench-cli-startup.ts", "--output", "-h"],
{
cwd: join(__dirname, "../.."),
encoding: "utf8",
},
);
expect(result.status).toBe(1);
expect(result.stdout).toBe("");
expect(result.stderr.trim()).toBe("--output requires a value");
expect(result.stderr).not.toContain("Node.js");
expect(result.stderr).not.toContain("\n at ");
});
it.runIf(process.platform !== "win32")(
"cleans timed-out benchmark process groups when the leader exits first",
() => {
@ -43,7 +63,7 @@ describe("bench-cli-startup", () => {
const tmpDir = tempDirs.make("openclaw-cli-startup-timeout-group-");
const entryPath = join(tmpDir, "entry.mjs");
const childPidPath = join(tmpDir, "child.pid");
let childPid = 0;
let childPid: number | undefined;
try {
writeFileSync(
entryPath,
@ -93,7 +113,7 @@ describe("bench-cli-startup", () => {
expect(result.stderr).toContain("version sample 1: timed out");
expect(isProcessAlive(childPid)).toBe(false);
} finally {
if (childPid && isProcessAlive(childPid)) {
if (childPid !== undefined && isProcessAlive(childPid)) {
process.kill(childPid, "SIGKILL");
}
tempDirs.cleanup();

View file

@ -47,6 +47,18 @@ describe("scripts/bench-model", () => {
expect(result.stderr).not.toContain("Missing ANTHROPIC_API_KEY");
});
it("rejects short flag values before checking provider credentials", () => {
expect(() => testing.parseArgs(["--prompt", "-h"])).toThrow("--prompt requires a value");
expect(() => testing.parseArgs(["--runs", "-h"])).toThrow("--runs requires a value");
const result = runBenchModel(["--prompt", "-h"]);
expect(result.status).toBe(1);
expect(result.stdout).toBe("");
expect(result.stderr.trim()).toBe("--prompt requires a value");
expect(result.stderr).not.toContain("Missing ANTHROPIC_API_KEY");
});
it("prints help without checking provider credentials", () => {
const result = runBenchModel(["--help"]);

View file

@ -52,6 +52,20 @@ describe("QA UX Matrix evidence producer CLI", () => {
expectNoNodeStack(result.stderr);
});
it("reports short flag values without treating them as help", () => {
const artifactBaseResult = runCli("--artifact-base", "-h");
const repoRootResult = runCli("--artifact-base", "/tmp/openclaw-ux-test", "--repo-root", "-h");
expect(artifactBaseResult.status).toBe(1);
expect(artifactBaseResult.stdout).toBe("");
expect(artifactBaseResult.stderr.trim()).toBe("--artifact-base requires a value");
expectNoNodeStack(artifactBaseResult.stderr);
expect(repoRootResult.status).toBe(1);
expect(repoRootResult.stdout).toBe("");
expect(repoRootResult.stderr.trim()).toBe("--repo-root requires a value");
expectNoNodeStack(repoRootResult.stderr);
});
it("sanitizes local checkout paths from generated evidence artifacts", () => {
const artifactBase = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-ux-evidence-test-"));
const fakeRepoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-ux-repo-test-"));