fix: parse qa cli integers strictly

This commit is contained in:
Peter Steinberger
2026-05-28 14:36:47 -04:00
parent 68ff0b9881
commit cd80b4efca
4 changed files with 91 additions and 16 deletions

View File

@@ -456,6 +456,17 @@ describe("qa cli runtime", () => {
});
});
it("rejects fractional suite concurrency from programmatic callers", async () => {
await expect(
runQaSuiteCommand({
repoRoot: "/tmp/openclaw-repo",
scenarioIds: ["channel-chat-baseline"],
concurrency: 1.5,
}),
).rejects.toThrow("--concurrency must be a positive integer");
expect(runQaSuiteFromRuntime).not.toHaveBeenCalled();
});
it("sets a failing exit code when host suite scenarios fail", async () => {
const priorExitCode = process.exitCode;
process.exitCode = undefined;

View File

@@ -158,10 +158,10 @@ function parseQaPositiveIntegerOption(label: string, value: number | undefined)
if (value === undefined) {
return undefined;
}
if (!Number.isFinite(value) || value < 1) {
if (!Number.isSafeInteger(value) || value < 1) {
throw new Error(`${label} must be a positive integer`);
}
return Math.floor(value);
return value;
}
function normalizeQaOptionalModelRef(input: string | undefined) {

View File

@@ -567,6 +567,33 @@ describe("qa cli registration", () => {
});
});
it.each([
[["qa", "suite", "--concurrency", "1.5"], "--concurrency must be a positive integer."],
[["qa", "suite", "--cpus", "0x4"], "--cpus must be a positive integer."],
[
["qa", "manual", "--message", "hi", "--timeout-ms", "1e3"],
"--timeout-ms must be a positive integer.",
],
[["qa", "credentials", "list", "--limit", "0x10"], "--limit must be a positive integer."],
[["qa", "ui", "--port", "1e4"], "--port must be a positive integer."],
[
["qa", "docker-scaffold", "--output-dir", "/tmp/qa", "--gateway-port", "1.5"],
"--gateway-port must be a positive integer.",
],
[["qa", "up", "--qa-lab-port", "0x43124"], "--qa-lab-port must be a positive integer."],
[["qa", "aimock", "--port", "1e4"], "--port must be a positive integer."],
])("rejects non-decimal QA numeric option %j", async (args, message) => {
const invalidProgram = new Command();
invalidProgram.exitOverride();
invalidProgram.configureOutput({
writeErr: () => {},
writeOut: () => {},
});
registerQaLabCli(invalidProgram);
await expect(invalidProgram.parseAsync(["node", "openclaw", ...args])).rejects.toThrow(message);
});
it("shows an enable hint when a discovered runner plugin is installed but blocked", async () => {
listQaRunnerCliContributions.mockReset().mockReturnValue([createBlockedQaRunnerContribution()]);
const blockedProgram = new Command();

View File

@@ -23,6 +23,25 @@ async function loadQaLabCliRuntime(): Promise<QaLabCliRuntime> {
return await qaLabCliRuntimePromise;
}
const DECIMAL_INTEGER_RE = /^\d+$/;
function invalidQaCliArgument(message: string): Error & { code: string; exitCode: number } {
const error = new Error(message) as Error & { code: string; exitCode: number };
error.name = "InvalidArgumentError";
error.code = "commander.invalidArgument";
error.exitCode = 1;
return error;
}
function parseQaCliPositiveIntegerOption(value: string, flag: string): number {
const trimmed = value.trim();
const parsed = DECIMAL_INTEGER_RE.test(trimmed) ? Number(trimmed) : Number.NaN;
if (!Number.isSafeInteger(parsed) || parsed < 1) {
throw invalidQaCliArgument(`${flag} must be a positive integer.`);
}
return parsed;
}
async function runQaSelfCheck(opts: { repoRoot?: string; output?: string }) {
const runtime = await loadQaLabCliRuntime();
await runtime.runQaLabSelfCheckCommand(opts);
@@ -302,7 +321,7 @@ export function registerQaLabCli(program: Command) {
[],
)
.option("--concurrency <count>", "Scenario worker concurrency", (value: string) =>
Number(value),
parseQaCliPositiveIntegerOption(value, "--concurrency"),
)
.option("--preflight", "Run a single-scenario bootstrap preflight and stop", false)
.option(
@@ -316,7 +335,9 @@ export function registerQaLabCli(program: Command) {
"Suite thinking default: off|minimal|low|medium|high|xhigh|adaptive|max",
)
.option("--image <alias>", "Multipass image alias")
.option("--cpus <count>", "Multipass vCPU count", (value: string) => Number(value))
.option("--cpus <count>", "Multipass vCPU count", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--cpus"),
)
.option("--memory <size>", "Multipass memory size")
.option("--disk <size>", "Multipass disk size")
.option("--runtime-pair <pair>", "Run each scenario under both runtimes, e.g. openclaw,codex")
@@ -534,17 +555,17 @@ export function registerQaLabCli(program: Command) {
[],
)
.option("--judge-timeout-ms <ms>", "Override judge wait timeout", (value: string) =>
Number(value),
parseQaCliPositiveIntegerOption(value, "--judge-timeout-ms"),
)
.option(
"--blind-judge-models",
"Hide candidate model refs from judge prompts; reports still map rankings back to real refs",
)
.option("--concurrency <count>", "Candidate model run concurrency", (value: string) =>
Number(value),
parseQaCliPositiveIntegerOption(value, "--concurrency"),
)
.option("--judge-concurrency <count>", "Judge model run concurrency", (value: string) =>
Number(value),
parseQaCliPositiveIntegerOption(value, "--judge-concurrency"),
)
.action(
async (opts: {
@@ -574,7 +595,9 @@ export function registerQaLabCli(program: Command) {
.option("--model <ref>", "Primary provider/model ref (defaults by provider mode)")
.option("--alt-model <ref>", "Alternate provider/model ref")
.option("--fast", "Enable provider fast mode where supported", false)
.option("--timeout-ms <ms>", "Override agent.wait timeout", (value: string) => Number(value))
.option("--timeout-ms <ms>", "Override agent.wait timeout", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--timeout-ms"),
)
.action(
async (opts: {
message: string;
@@ -672,7 +695,9 @@ export function registerQaLabCli(program: Command) {
.description("List credential rows in the shared Convex pool")
.option("--kind <kind>", "Filter by credential kind")
.option("--status <status>", 'Filter by row status: "active", "disabled", or "all"', "all")
.option("--limit <count>", "Max rows to return", (value: string) => Number(value))
.option("--limit <count>", "Max rows to return", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--limit"),
)
.option("--show-secrets", "Include credential payload JSON in output", false)
.option("--site-url <url>", "Override OPENCLAW_QA_CONVEX_SITE_URL")
.option("--endpoint-prefix <path>", "Override OPENCLAW_QA_CONVEX_ENDPOINT_PREFIX")
@@ -697,10 +722,12 @@ export function registerQaLabCli(program: Command) {
.description("Start the private QA debugger UI and local QA bus")
.option("--repo-root <path>", "Repository root to target when running from a neutral cwd")
.option("--host <host>", "Bind host", "127.0.0.1")
.option("--port <port>", "Bind port", (value: string) => Number(value))
.option("--port <port>", "Bind port", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--port"),
)
.option("--advertise-host <host>", "Optional public host to advertise in bootstrap payloads")
.option("--advertise-port <port>", "Optional public port to advertise", (value: string) =>
Number(value),
parseQaCliPositiveIntegerOption(value, "--advertise-port"),
)
.option("--control-ui-url <url>", "Optional Control UI URL to embed beside the QA panel")
.option(
@@ -737,8 +764,12 @@ export function registerQaLabCli(program: Command) {
.description("Write a prebaked Docker scaffold for the QA dashboard + gateway lane")
.option("--repo-root <path>", "Repository root to target when running from a neutral cwd")
.requiredOption("--output-dir <path>", "Output directory for docker-compose + state files")
.option("--gateway-port <port>", "Gateway host port", (value: string) => Number(value))
.option("--qa-lab-port <port>", "QA lab host port", (value: string) => Number(value))
.option("--gateway-port <port>", "Gateway host port", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--gateway-port"),
)
.option("--qa-lab-port <port>", "QA lab host port", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--qa-lab-port"),
)
.option("--provider-base-url <url>", "Provider base URL for the QA gateway")
.option("--image <name>", "Prebaked image name", "openclaw:qa-local-prebaked")
.option("--use-prebuilt-image", "Use image: instead of build: in docker-compose", false)
@@ -774,8 +805,12 @@ export function registerQaLabCli(program: Command) {
.description("Build the QA site, start the Docker-backed QA stack, and print the QA Lab URL")
.option("--repo-root <path>", "Repository root to target when running from a neutral cwd")
.option("--output-dir <path>", "Output directory for docker-compose + state files")
.option("--gateway-port <port>", "Gateway host port", (value: string) => Number(value))
.option("--qa-lab-port <port>", "QA lab host port", (value: string) => Number(value))
.option("--gateway-port <port>", "Gateway host port", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--gateway-port"),
)
.option("--qa-lab-port <port>", "QA lab host port", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--qa-lab-port"),
)
.option("--provider-base-url <url>", "Provider base URL for the QA gateway")
.option("--image <name>", "Image tag", "openclaw:qa-local-prebaked")
.option("--use-prebuilt-image", "Use image: instead of build: in docker-compose", false)
@@ -805,7 +840,9 @@ export function registerQaLabCli(program: Command) {
qa.command(providerCommand.name)
.description(providerCommand.description)
.option("--host <host>", "Bind host", "127.0.0.1")
.option("--port <port>", "Bind port", (value: string) => Number(value))
.option("--port <port>", "Bind port", (value: string) =>
parseQaCliPositiveIntegerOption(value, "--port"),
)
.action(async (opts: { host?: string; port?: number }) => {
await runQaProviderServer(providerCommand.providerMode, opts);
});