From a8f0557c9de6cfd78e4f452fe9b826fea4a909d0 Mon Sep 17 00:00:00 2001 From: Catalin Lupuleti <105351510+lupuletic@users.noreply.github.com> Date: Thu, 26 Feb 2026 22:21:23 +0000 Subject: [PATCH] fix: address review feedback (#5871) --- src/cli/argv.test.ts | 52 ++++++++++++++++++++++++++++++++++++++ src/cli/argv.ts | 49 +++++++++++++++++++++++++++++++++++ src/cli/channel-options.ts | 18 +++++++------ src/entry.ts | 39 ++++++++++++++++++++-------- 4 files changed, 140 insertions(+), 18 deletions(-) diff --git a/src/cli/argv.test.ts b/src/cli/argv.test.ts index ef42e0579ac..63ecac5c4e9 100644 --- a/src/cli/argv.test.ts +++ b/src/cli/argv.test.ts @@ -8,6 +8,7 @@ import { getVerboseFlag, hasHelpOrVersion, hasFlag, + isRootHelpRequest, isRootVersionRequest, shouldMigrateState, shouldMigrateStateFromPath, @@ -332,4 +333,55 @@ describe("argv helpers", () => { ])("isRootVersionRequest: $name", ({ argv, expected }) => { expect(isRootVersionRequest(argv)).toBe(expected); }); + + // isRootHelpRequest: guards the entry.ts fast-path help exit. + it.each([ + { + name: "--help flag at root", + argv: ["node", "openclaw", "--help"], + expected: true, + }, + { + name: "-h flag at root", + argv: ["node", "openclaw", "-h"], + expected: true, + }, + { + name: "--help after a root boolean flag", + argv: ["node", "openclaw", "--dev", "--help"], + expected: true, + }, + { + name: "--help after a root value flag", + argv: ["node", "openclaw", "--profile", "dev", "--help"], + expected: true, + }, + { + name: "--help after -- terminator must NOT trigger (forwarded arg)", + argv: ["node", "openclaw", "nodes", "run", "--", "git", "--help"], + expected: false, + }, + { + name: "-h after -- terminator must NOT trigger", + argv: ["node", "openclaw", "--", "-h"], + expected: false, + }, + { + name: "--help with subcommand must NOT trigger (subcommand-scoped)", + argv: ["node", "openclaw", "status", "--help"], + expected: false, + }, + { + name: "-h with subcommand must NOT trigger", + argv: ["node", "openclaw", "models", "-h"], + expected: false, + }, + { + name: "normal command without help flag", + argv: ["node", "openclaw", "status"], + expected: false, + }, + ])("isRootHelpRequest: $name", ({ argv, expected }) => { + expect(isRootHelpRequest(argv)).toBe(expected); + }); }); diff --git a/src/cli/argv.ts b/src/cli/argv.ts index 2953d2572dc..631f4f02649 100644 --- a/src/cli/argv.ts +++ b/src/cli/argv.ts @@ -7,6 +7,13 @@ const ROOT_BOOLEAN_FLAGS = new Set(["--dev", "--no-color"]); const ROOT_VALUE_FLAGS = new Set(["--profile", "--log-level"]); const FLAG_TERMINATOR = "--"; +/** + * Returns true if argv contains any help or version flag. + * NOTE: uses `argv.some()` without `--` terminator awareness, so forwarded args + * like `nodes run -- git --help` will produce a false positive. Use + * `isRootHelpRequest`/`isRootVersionRequest` for fast-path guards where + * correctness around forwarded args matters. + */ export function hasHelpOrVersion(argv: string[]): boolean { return ( argv.some((arg) => HELP_FLAGS.has(arg) || VERSION_FLAGS.has(arg)) || hasRootVersionAlias(argv) @@ -24,6 +31,48 @@ export function isRootVersionRequest(argv: string[]): boolean { return hasFlag(argv, "--version") || hasFlag(argv, "-V") || hasRootVersionAlias(argv); } +/** + * Returns true only when the process is a root-level help request: + * - `-h` or `--help` appears before any `--` terminator, and + * - no subcommand (non-flag token) precedes the help flag. + * `openclaw status --help` returns false (subcommand-scoped); `openclaw --help` returns true. + * Used by the entry.ts fast path to display help without loading route.ts or run-main.js. + */ +export function isRootHelpRequest(argv: string[]): boolean { + const args = argv.slice(2); + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; + if (!arg) { + continue; + } + if (arg === FLAG_TERMINATOR) { + break; + } + if (arg === "-h" || arg === "--help") { + return true; + } + if (ROOT_BOOLEAN_FLAGS.has(arg)) { + continue; + } + if (arg.startsWith("--profile=")) { + continue; + } + if (ROOT_VALUE_FLAGS.has(arg)) { + const next = args[i + 1]; + if (isValueToken(next)) { + i += 1; + } + continue; + } + if (arg.startsWith("-")) { + continue; + } + // Non-flag token is a subcommand — this is a subcommand-scoped help request. + return false; + } + return false; +} + function isValueToken(arg: string | undefined): boolean { if (!arg) { return false; diff --git a/src/cli/channel-options.ts b/src/cli/channel-options.ts index 1b2a98d25ac..0df7e7bf66b 100644 --- a/src/cli/channel-options.ts +++ b/src/cli/channel-options.ts @@ -23,14 +23,16 @@ export function resolveCliChannelOptions(): string[] { const catalog = listChannelPluginCatalogEntries().map((entry) => entry.id); const base = dedupe([...CHAT_CHANNEL_ORDER, ...catalog]); if (isTruthyEnvValue(process.env.OPENCLAW_EAGER_CHANNEL_OPTIONS)) { - // CHANGED SEMANTIC: ensurePluginRegistryLoaded() is intentionally NOT called - // here to avoid pulling in plugins/loader.ts → jiti at startup (slow on - // low-powered devices). As a result, OPENCLAW_EAGER_CHANNEL_OPTIONS is - // effectively a no-op for its original purpose of force-loading all plugins - // into the option list before Commander parses args. Plugin IDs are only - // included here if the registry was already populated by some other means - // (e.g. the preaction hook for a real command). If this env var behaviour is - // needed, restore the ensurePluginRegistryLoaded() call explicitly. + // Emit a deprecation warning: ensurePluginRegistryLoaded() was removed to avoid + // pulling plugins/loader.ts → jiti at startup (slow on low-powered devices). + // OPENCLAW_EAGER_CHANNEL_OPTIONS no longer force-loads plugin channels; plugin IDs + // are only included if the registry was already populated by another code path. + process.emitWarning( + "OPENCLAW_EAGER_CHANNEL_OPTIONS no longer force-loads plugin channels at startup. " + + "Plugin IDs are only included if the registry was pre-loaded by another means. " + + "Remove this env var to silence this warning.", + { code: "OPENCLAW_EAGER_CHANNEL_OPTIONS_DEPRECATED" }, + ); const pluginIds = listChannelPlugins().map((plugin) => plugin.id); return dedupe([...base, ...pluginIds]); } diff --git a/src/entry.ts b/src/entry.ts index 633f378eece..2d48029271b 100644 --- a/src/entry.ts +++ b/src/entry.ts @@ -2,7 +2,7 @@ import { spawn } from "node:child_process"; import process from "node:process"; import { fileURLToPath } from "node:url"; -import { isRootVersionRequest } from "./cli/argv.js"; +import { isRootHelpRequest, isRootVersionRequest } from "./cli/argv.js"; import { applyCliProfileEnv, parseCliProfileArgs } from "./cli/profile.js"; import { shouldSkipRespawnForArgv } from "./cli/respawn-policy.js"; import { normalizeWindowsArgv } from "./cli/windows-argv.js"; @@ -143,14 +143,33 @@ if ( process.exit(0); } - import("./cli/run-main.js") - .then(({ runCli }) => runCli(process.argv)) - .catch((error) => { - console.error( - "[openclaw] Failed to start CLI:", - error instanceof Error ? (error.stack ?? error.message) : error, - ); - process.exitCode = 1; - }); + // Fast path: display root help without loading run-main.js or route.ts. + // isRootHelpRequest stops at -- and requires no subcommand before -h/--help, + // so `openclaw status --help` still falls through to the full CLI path. + // Importing program.js directly avoids the route.ts static import in run-main.ts. + if (isRootHelpRequest(argv)) { + import("./cli/program.js") + .then(({ buildProgram }) => { + buildProgram().outputHelp(); + process.exit(0); + }) + .catch((error) => { + console.error( + "[openclaw] Failed to display help:", + error instanceof Error ? (error.stack ?? error.message) : error, + ); + process.exit(1); + }); + } else { + import("./cli/run-main.js") + .then(({ runCli }) => runCli(process.argv)) + .catch((error) => { + console.error( + "[openclaw] Failed to start CLI:", + error instanceof Error ? (error.stack ?? error.message) : error, + ); + process.exitCode = 1; + }); + } } }