From eda96f9f80f0af4e1a1b0fe97f971c7cb95fb845 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 19 Jun 2026 14:14:22 -0400 Subject: [PATCH] fix(cli): preserve timeout validation on routed commands --- src/cli/argv.test.ts | 36 +++++++++++++++++++++++++++--- src/cli/argv.ts | 21 ++++++++++++----- src/cli/program/route-args.test.ts | 22 ++++++++++++++++++ src/cli/program/route-args.ts | 25 +++++---------------- 4 files changed, 76 insertions(+), 28 deletions(-) diff --git a/src/cli/argv.test.ts b/src/cli/argv.test.ts index 4a8d9fd29eb9..e5c863d340a7 100644 --- a/src/cli/argv.test.ts +++ b/src/cli/argv.test.ts @@ -561,6 +561,16 @@ describe("argv helpers", () => { argv: ["node", "openclaw", "--", "--timeout=99"], expected: undefined, }, + { + name: "repeated flag uses final value", + argv: ["node", "openclaw", "status", "--timeout", "100", "--timeout=200"], + expected: "200", + }, + { + name: "missing repeated value remains invalid", + argv: ["node", "openclaw", "status", "--timeout", "--timeout", "200"], + expected: null, + }, ])("extracts flag values: $name", ({ argv, expected }) => { expect(getFlagValue(argv, "--timeout")).toBe(expected); }); @@ -597,17 +607,37 @@ describe("argv helpers", () => { { name: "invalid integer", argv: ["node", "openclaw", "status", "--timeout", "nope"], - expected: undefined, + expected: null, }, { name: "non-decimal integer", argv: ["node", "openclaw", "status", "--timeout", "0x10"], - expected: undefined, + expected: null, }, { name: "partial integer", argv: ["node", "openclaw", "status", "--timeout", "5s"], - expected: undefined, + expected: null, + }, + { + name: "zero", + argv: ["node", "openclaw", "status", "--timeout", "0"], + expected: null, + }, + { + name: "negative integer", + argv: ["node", "openclaw", "status", "--timeout", "-5"], + expected: null, + }, + { + name: "repeated value uses final valid integer", + argv: ["node", "openclaw", "status", "--timeout", "nope", "--timeout", "5000"], + expected: 5000, + }, + { + name: "repeated value rejects final invalid integer", + argv: ["node", "openclaw", "status", "--timeout", "5000", "--timeout", "nope"], + expected: null, }, ])("parses positive integer flag values: $name", ({ argv, expected }) => { expect(getPositiveIntFlagValue(argv, "--timeout")).toBe(expected); diff --git a/src/cli/argv.ts b/src/cli/argv.ts index 50d00ed0ff03..81c309a92a63 100644 --- a/src/cli/argv.ts +++ b/src/cli/argv.ts @@ -403,6 +403,7 @@ export function normalizeRootLogLevelArgv( export function getFlagValue(argv: string[], name: string): string | null | undefined { const args = argv.slice(2); + let value: string | undefined; for (let i = 0; i < args.length; i += 1) { const arg = args[i]; if (arg === FLAG_TERMINATOR) { @@ -410,14 +411,22 @@ export function getFlagValue(argv: string[], name: string): string | null | unde } if (arg === name) { const next = args[i + 1]; - return isValueToken(next) ? next : null; + if (!isValueToken(next)) { + return null; + } + value = next; + i += 1; + continue; } if (arg.startsWith(`${name}=`)) { - const value = arg.slice(name.length + 1); - return value ? value : null; + const assigned = arg.slice(name.length + 1); + if (!assigned) { + return null; + } + value = assigned; } } - return undefined; + return value; } export function getVerboseFlag(argv: string[], options?: { includeDebug?: boolean }): boolean { @@ -435,7 +444,9 @@ export function getPositiveIntFlagValue(argv: string[], name: string): number | if (raw === null || raw === undefined) { return raw; } - return parsePositiveInt(raw); + // Keep absent distinct from present-but-invalid so route-first callers can + // defer invalid input to Commander instead of silently applying defaults. + return parsePositiveInt(raw) ?? null; } export function getCommandPathWithRootOptions(argv: string[], depth = 2): string[] { diff --git a/src/cli/program/route-args.test.ts b/src/cli/program/route-args.test.ts index f24626b970bd..7960b60ac7ce 100644 --- a/src/cli/program/route-args.test.ts +++ b/src/cli/program/route-args.test.ts @@ -54,6 +54,28 @@ describe("route-args", () => { expect(parseStatusRouteArgs(["node", "openclaw", "status", "--timeout", bad])).toBeNull(); expect(parseHealthRouteArgs(["node", "openclaw", "health", "--timeout", bad])).toBeNull(); } + expect( + parseStatusRouteArgs([ + "node", + "openclaw", + "status", + "--timeout", + "5000", + "--timeout", + "nope", + ]), + ).toBeNull(); + expect( + parseHealthRouteArgs([ + "node", + "openclaw", + "health", + "--timeout", + "nope", + "--timeout", + "5000", + ]), + ).toMatchObject({ timeoutMs: 5000 }); // A valid positive integer still parses on the fast path. expect(parseStatusRouteArgs(["node", "openclaw", "status", "--timeout", "5000"])).toMatchObject( { timeoutMs: 5000 }, diff --git a/src/cli/program/route-args.ts b/src/cli/program/route-args.ts index 043931b7a263..6da5bc0c7405 100644 --- a/src/cli/program/route-args.ts +++ b/src/cli/program/route-args.ts @@ -3,6 +3,7 @@ import { isValueToken } from "../../infra/cli-root-options.js"; import { getCommandPositionalsWithRootOptions, getFlagValue, + getPositiveIntFlagValue, getVerboseFlag, hasFlag, } from "../argv.js"; @@ -66,16 +67,8 @@ function parseSinglePositional( /** Parse `openclaw health` flags for the route-first status family. */ export function parseHealthRouteArgs(argv: string[]) { - // A present-but-invalid --timeout (0, negative, non-numeric) must defer to - // Commander so it surfaces the same validation error as the full command - // path, instead of the fast path silently falling back to the default. - const rawTimeout = getFlagValue(argv, "--timeout"); - if (rawTimeout === null) { - return null; - } - const timeoutMs = - rawTimeout === undefined ? undefined : parseStrictPositiveIntOrUndefined(rawTimeout); - if (rawTimeout !== undefined && timeoutMs === undefined) { + const timeoutMs = getPositiveIntFlagValue(argv, "--timeout"); + if (timeoutMs === null) { return null; } return { @@ -87,16 +80,8 @@ export function parseHealthRouteArgs(argv: string[]) { /** Parse `openclaw status` flags without registering the full command tree. */ export function parseStatusRouteArgs(argv: string[]) { - // A present-but-invalid --timeout (0, negative, non-numeric) must defer to - // Commander so it surfaces the same validation error as the full command - // path, instead of the fast path silently falling back to the default. - const rawTimeout = getFlagValue(argv, "--timeout"); - if (rawTimeout === null) { - return null; - } - const timeoutMs = - rawTimeout === undefined ? undefined : parseStrictPositiveIntOrUndefined(rawTimeout); - if (rawTimeout !== undefined && timeoutMs === undefined) { + const timeoutMs = getPositiveIntFlagValue(argv, "--timeout"); + if (timeoutMs === null) { return null; } return {