From 09cffbdfbf6efeb19e66fbc66e0f92b34e67b71c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 10 May 2026 07:59:20 +0100 Subject: [PATCH] fix(cli): avoid plugin allowlist hints for unknown commands Co-authored-by: kagura-agent --- CHANGELOG.md | 1 + src/cli/run-main-policy.ts | 32 ++++++++++++ src/cli/run-main.exit.test.ts | 50 ++++++++++++++++++ src/cli/run-main.test.ts | 52 ++++++++++++++----- src/cli/run-main.ts | 48 +++++++++++++++++ .../manifest-command-aliases.runtime.ts | 26 ++++++++++ 6 files changed, 197 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff45005d72f..4cf01d20726 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai - Plugins/doctor: invalidate persisted plugin registry snapshots when plugin diagnostics point at deleted source paths, so `openclaw doctor` stops repeating stale warnings after a local extension is replaced by a managed npm plugin. Fixes #80087. (#80134) Thanks @hclsys. - Cron: let isolated self-cleanup runs inspect their own job run history while keeping other cron jobs and mutation actions blocked. Fixes #80019. Thanks @hclsys. - Cron: report isolated agent-turn setup and pre-model stalls with phase-specific timeout errors instead of waiting for the full job budget when no model call starts. Fixes #74803. Thanks @jeffsteinbok-openclaw and @dgkim311. +- CLI/plugins: treat arbitrary unknown subcommands outside plugin CLI metadata as normal unknown commands instead of suggesting `plugins.allow`, while preserving allowlist guidance for real plugin command roots. Fixes #80109. (#80123) Thanks @kagura-agent. - CLI/config: persist explicit `config set` and `config patch` values that equal runtime defaults instead of reporting success while dropping them. Fixes #79856. (#80106) Thanks @abodanty and @hclsys. - OpenAI/realtime voice: accept Codex-compatible legacy audio and transcript event aliases so provider protocol drift does not drop assistant audio or captions. - Discord/voice: keep default agent-proxy realtime sessions from auto-speaking filler before the forced OpenClaw consult answer, finish Discord playback on realtime response completion, and queue later exact-speech answers until playback idles to avoid mid-sentence replacement. diff --git a/src/cli/run-main-policy.ts b/src/cli/run-main-policy.ts index 011486279b5..7a96b9409ca 100644 --- a/src/cli/run-main-policy.ts +++ b/src/cli/run-main-policy.ts @@ -112,6 +112,11 @@ export function resolveMissingPluginCommandMessage( config?: OpenClawConfig; registry?: PluginManifestCommandAliasRegistry; }) => PluginManifestToolOwnerRecord | undefined; + resolveCliCommandSurfaceOwner?: (params: { + command: string | undefined; + config?: OpenClawConfig; + registry?: PluginManifestCommandAliasRegistry; + }) => string | undefined; }, ): string | null { const normalizedPluginId = normalizeLowercaseStringOrEmpty(pluginId); @@ -230,6 +235,33 @@ export function resolveMissingPluginCommandMessage( if (parentPluginId && allow.includes(parentPluginId)) { return null; } + const cliCommandSurfaceOwner = options?.resolveCliCommandSurfaceOwner + ? options.resolveCliCommandSurfaceOwner({ + command: normalizedPluginId, + config, + ...(options?.registry ? { registry: options.registry } : {}), + }) + : options?.registry + ? resolveManifestCommandAliasOwnerInRegistry({ + command: normalizedPluginId, + registry: options.registry, + })?.pluginId + : undefined; + const normalizedCliCommandSurfaceOwner = + normalizeOptionalLowercaseString(cliCommandSurfaceOwner); + if (!normalizedCliCommandSurfaceOwner) { + return null; + } + if (allow.includes(normalizedCliCommandSurfaceOwner)) { + return null; + } + if (normalizedCliCommandSurfaceOwner !== normalizedPluginId) { + return ( + `"${normalizedPluginId}" is not a plugin; it is a command provided by the ` + + `"${normalizedCliCommandSurfaceOwner}" plugin. Add "${normalizedCliCommandSurfaceOwner}" to ` + + `\`plugins.allow\` instead of "${normalizedPluginId}".` + ); + } return ( `The \`openclaw ${normalizedPluginId}\` command is unavailable because ` + `\`plugins.allow\` excludes "${normalizedPluginId}". Add "${normalizedPluginId}" to ` + diff --git a/src/cli/run-main.exit.test.ts b/src/cli/run-main.exit.test.ts index e6ce9f0c82e..0a1391791dc 100644 --- a/src/cli/run-main.exit.test.ts +++ b/src/cli/run-main.exit.test.ts @@ -25,6 +25,7 @@ const registerPluginCliCommandsFromValidatedConfigMock = vi.hoisted(() => vi.fn( const resolvePluginCliRootOwnerIdsMock = vi.hoisted(() => vi.fn()); const resolveManifestCommandAliasOwnerMock = vi.hoisted(() => vi.fn()); const resolveManifestToolOwnerMock = vi.hoisted(() => vi.fn()); +const resolveManifestCliCommandSurfaceOwnerMock = vi.hoisted(() => vi.fn()); const restoreTerminalStateMock = vi.hoisted(() => vi.fn()); const hasEnvHttpProxyAgentConfiguredMock = vi.hoisted(() => vi.fn(() => false)); const ensureGlobalUndiciEnvProxyDispatcherMock = vi.hoisted(() => vi.fn()); @@ -179,6 +180,7 @@ vi.mock("../plugins/cli-registry-loader.js", () => ({ })); vi.mock("../plugins/manifest-command-aliases.runtime.js", () => ({ + resolveManifestCliCommandSurfaceOwner: resolveManifestCliCommandSurfaceOwnerMock, resolveManifestCommandAliasOwner: resolveManifestCommandAliasOwnerMock, resolveManifestToolOwner: resolveManifestToolOwnerMock, })); @@ -252,6 +254,7 @@ describe("runCli exit behavior", () => { ); resolveManifestCommandAliasOwnerMock.mockReturnValue(undefined); resolveManifestToolOwnerMock.mockReturnValue(undefined); + resolveManifestCliCommandSurfaceOwnerMock.mockReturnValue(undefined); delete process.env.OPENCLAW_DISABLE_CLI_STARTUP_HELP_FAST_PATH; delete process.env.OPENCLAW_HIDE_BANNER; }); @@ -488,6 +491,53 @@ describe("runCli exit behavior", () => { expect(registerPluginCliCommandsFromValidatedConfigMock).not.toHaveBeenCalled(); }); + it("does not suggest plugins.allow for unknown command roots before proxy startup", async () => { + loadConfigMock.mockReturnValueOnce({ + plugins: { + allow: ["browser"], + }, + }); + + let error: unknown; + try { + await runCli(["node", "openclaw", "totally-unknown"]); + } catch (caught) { + error = caught; + } + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain( + 'No built-in command or plugin CLI metadata owns "totally-unknown"', + ); + expect((error as Error).message).not.toContain("plugins.allow"); + expect(startProxyMock).not.toHaveBeenCalled(); + expect(tryRouteCliMock).not.toHaveBeenCalled(); + expect(registerPluginCliCommandsFromValidatedConfigMock).not.toHaveBeenCalled(); + }); + + it("preserves plugins.allow diagnostics for roots owned only by CLI metadata", async () => { + loadConfigMock.mockReturnValueOnce({ + plugins: { + allow: ["browser"], + }, + }); + resolvePluginCliRootOwnerIdsMock.mockImplementation( + ({ + cfg, + primaryCommand, + }: { + cfg?: { plugins?: { allow?: string[] } }; + primaryCommand?: string; + }) => (primaryCommand === "qa" && cfg?.plugins?.allow?.length === 0 ? ["qa-lab"] : []), + ); + + await expect(runCli(["node", "openclaw", "qa"])).rejects.toThrow( + 'Add "qa-lab" to `plugins.allow` instead of "qa"', + ); + expect(startProxyMock).not.toHaveBeenCalled(); + expect(tryRouteCliMock).not.toHaveBeenCalled(); + expect(registerPluginCliCommandsFromValidatedConfigMock).not.toHaveBeenCalled(); + }); + it("reports plugin tool command mistakes before proxy startup", async () => { resolveManifestToolOwnerMock.mockReturnValueOnce({ toolName: "lcm_recent", diff --git a/src/cli/run-main.test.ts b/src/cli/run-main.test.ts index 98ce87916d4..0320c99ad34 100644 --- a/src/cli/run-main.test.ts +++ b/src/cli/run-main.test.ts @@ -40,6 +40,16 @@ const losslessClawToolRegistry: PluginManifestCommandAliasRegistry = { ], }; +const browserCommandAliasRegistry: PluginManifestCommandAliasRegistry = { + plugins: [ + { + id: "browser", + enabledByDefault: true, + commandAliases: [{ name: "browser" }], + }, + ], +}; + describe("isGatewayRunFastPathArgv", () => { it("matches only plain gateway foreground starts without root options or help", () => { expect(isGatewayRunFastPathArgv(["node", "openclaw", "gateway"])).toBe(true); @@ -191,11 +201,15 @@ describe("shouldUseBrowserHelpFastPath", () => { describe("resolveMissingPluginCommandMessage", () => { it("explains plugins.allow misses for a bundled plugin command", () => { expect( - resolveMissingPluginCommandMessage("browser", { - plugins: { - allow: ["quietchat"], + resolveMissingPluginCommandMessage( + "browser", + { + plugins: { + allow: ["quietchat"], + }, }, - }), + { registry: browserCommandAliasRegistry }, + ), ).toContain('`plugins.allow` excludes "browser"'); }); @@ -402,7 +416,7 @@ describe("resolveMissingPluginCommandMessage", () => { expect(message).toContain('"lossless-claw"'); }); - it("preserves the plugins.allow suggestion when the unknown name is not a plugin tool", () => { + it("returns null for unknown names excluded by plugins.allow", () => { const message = resolveMissingPluginCommandMessage( "totally-unknown", { @@ -412,14 +426,30 @@ describe("resolveMissingPluginCommandMessage", () => { }, { registry: losslessClawToolRegistry }, ); - expect(message).not.toBeNull(); - expect(message).toContain('`plugins.allow` excludes "totally-unknown"'); + expect(message).toBeNull(); + }); + + it("points metadata-only CLI roots in plugins.allow at their parent plugin", () => { + const message = resolveMissingPluginCommandMessage( + "qa", + { + plugins: { + allow: ["browser"], + }, + }, + { + resolveCliCommandSurfaceOwner: () => "qa-lab", + }, + ); + expect(message).toContain('"qa" is not a plugin'); + expect(message).toContain('"qa-lab"'); + expect(message).toContain('Add "qa-lab" to `plugins.allow` instead of "qa"'); }); it("does not attribute a tool to an owning plugin excluded by plugins.allow", () => { // The owning plugin is denied via plugins.allow, so the manifest-declared - // tool is not available through the owning plugin. Fall through to the - // standard plugins.allow message instead of falsely attributing it. + // tool is not available through the owning plugin. Tool names are not CLI + // command surfaces, so do not suggest adding the tool name to plugins.allow. const message = resolveMissingPluginCommandMessage( "lcm_recent", { @@ -429,9 +459,7 @@ describe("resolveMissingPluginCommandMessage", () => { }, { registry: losslessClawToolRegistry }, ); - expect(message).not.toBeNull(); - expect(message).not.toContain("agent tool available"); - expect(message).toContain('`plugins.allow` excludes "lcm_recent"'); + expect(message).toBeNull(); }); it("does not attribute a tool to an owning plugin disabled via plugins.entries", () => { diff --git a/src/cli/run-main.ts b/src/cli/run-main.ts index 566cb8e4f41..ebefe4bbc96 100644 --- a/src/cli/run-main.ts +++ b/src/cli/run-main.ts @@ -319,6 +319,47 @@ async function isPluginCliRoot(params: { } } +function createAllowlistAgnosticCliLookupConfig(config: OpenClawConfig): OpenClawConfig { + if (!Array.isArray(config.plugins?.allow) || config.plugins.allow.length === 0) { + return config; + } + return { + ...config, + plugins: { + ...config.plugins, + allow: [], + }, + }; +} + +async function resolveCliCommandSurfaceOwner(params: { + primary: string; + config: OpenClawConfig; +}): Promise { + const { resolveManifestCliCommandSurfaceOwner } = + await import("../plugins/manifest-command-aliases.runtime.js"); + const manifestOwner = resolveManifestCliCommandSurfaceOwner({ + command: params.primary, + config: params.config, + env: process.env, + }); + if (manifestOwner) { + return manifestOwner; + } + try { + const { resolvePluginCliRootOwnerIds } = await import("../plugins/cli-registry-loader.js"); + return ( + await resolvePluginCliRootOwnerIds({ + cfg: createAllowlistAgnosticCliLookupConfig(params.config), + env: process.env, + primaryCommand: params.primary, + }) + )?.[0]; + } catch { + return undefined; + } +} + async function resolveUnownedCliPrimary(params: { argv: string[]; config: OpenClawConfig; @@ -347,10 +388,12 @@ async function resolveUnownedCliPrimaryMessage(params: { }): Promise { const { resolveManifestCommandAliasOwner, resolveManifestToolOwner } = await import("../plugins/manifest-command-aliases.runtime.js"); + const cliCommandSurfaceOwner = await resolveCliCommandSurfaceOwner(params); return ( resolveMissingPluginCommandMessageFromPolicy(params.primary, params.config, { resolveCommandAliasOwner: resolveManifestCommandAliasOwner, resolveToolOwner: resolveManifestToolOwner, + resolveCliCommandSurfaceOwner: () => cliCommandSurfaceOwner, }) ?? `Unknown command: openclaw ${params.primary}. No built-in command or plugin CLI metadata owns "${params.primary}".` ); @@ -696,12 +739,17 @@ export async function runCli(argv: string[] = process.argv) { ) { const { resolveManifestCommandAliasOwner, resolveManifestToolOwner } = await import("../plugins/manifest-command-aliases.runtime.js"); + const cliCommandSurfaceOwner = await resolveCliCommandSurfaceOwner({ + primary, + config, + }); const missingPluginCommandMessage = resolveMissingPluginCommandMessageFromPolicy( primary, config, { resolveCommandAliasOwner: resolveManifestCommandAliasOwner, resolveToolOwner: resolveManifestToolOwner, + resolveCliCommandSurfaceOwner: () => cliCommandSurfaceOwner, }, ); if (missingPluginCommandMessage) { diff --git a/src/plugins/manifest-command-aliases.runtime.ts b/src/plugins/manifest-command-aliases.runtime.ts index c68b2c44ebb..0b0d1fc3d9e 100644 --- a/src/plugins/manifest-command-aliases.runtime.ts +++ b/src/plugins/manifest-command-aliases.runtime.ts @@ -1,5 +1,6 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; +import { resolveManifestActivationPluginIds } from "./activation-planner.js"; import { resolveManifestCommandAliasOwnerInRegistry, resolveManifestToolOwnerInRegistry, @@ -34,6 +35,31 @@ export function resolveManifestCommandAliasOwner(params: { }); } +export function resolveManifestCliCommandSurfaceOwner(params: { + command: string | undefined; + config?: OpenClawConfig; + workspaceDir?: string; + env?: NodeJS.ProcessEnv; + registry?: PluginManifestCommandAliasRegistry; +}): string | undefined { + const normalizedCommand = normalizeOptionalLowercaseString(params.command); + if (!normalizedCommand) { + return undefined; + } + if (params.registry) { + return resolveManifestCommandAliasOwnerInRegistry({ + command: normalizedCommand, + registry: params.registry, + })?.pluginId; + } + return resolveManifestActivationPluginIds({ + trigger: { kind: "command", command: normalizedCommand }, + config: params.config, + workspaceDir: params.workspaceDir, + env: params.env, + })[0]; +} + /** * Resolve which plugin owns an agent-tool name, applying control-plane * availability filters so disabled/denied plugins are not falsely attributed.