diff --git a/CHANGELOG.md b/CHANGELOG.md index 052510b8628..ebbfb3f0924 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ Docs: https://docs.openclaw.ai - CLI/startup: lazy-load channel add and root help startup paths to trim avoidable RSS and help latency on constrained hosts. (#46784) Thanks @vincentkoc. - CLI/onboarding: import static provider definitions directly for onboarding model/config helpers so those paths no longer pull provider discovery just for built-in defaults. (#47467) Thanks @vincentkoc. - CLI/auth choice: lazy-load plugin/provider fallback resolution so mapped auth choices stay on the static path and only unknown choices pay the heavy provider load. (#47495) Thanks @vincentkoc. +- CLI/completion: reduce recursive completion-script string churn and fix nested PowerShell command-path matching so generated nested completions resolve on PowerShell too. (#45537) Thanks @yiShanXin and @vincentkoc. ## 2026.3.13 diff --git a/src/cli/completion-cli.test.ts b/src/cli/completion-cli.test.ts new file mode 100644 index 00000000000..d2f34b0e8cb --- /dev/null +++ b/src/cli/completion-cli.test.ts @@ -0,0 +1,52 @@ +import { Command } from "commander"; +import { describe, expect, it } from "vitest"; +import { getCompletionScript } from "./completion-cli.js"; + +function createCompletionProgram(): Command { + const program = new Command(); + program.name("openclaw"); + program.description("CLI root"); + program.option("-v, --verbose", "Verbose output"); + + const gateway = program.command("gateway").description("Gateway commands"); + gateway.option("--force", "Force the action"); + + gateway.command("status").description("Show gateway status").option("--json", "JSON output"); + gateway.command("restart").description("Restart gateway"); + + return program; +} + +describe("completion-cli", () => { + it("generates zsh functions for nested subcommands", () => { + const script = getCompletionScript("zsh", createCompletionProgram()); + + expect(script).toContain("_openclaw_gateway()"); + expect(script).toContain("(status) _openclaw_gateway_status ;;"); + expect(script).toContain("(restart) _openclaw_gateway_restart ;;"); + expect(script).toContain("--force[Force the action]"); + }); + + it("generates PowerShell command paths without the executable prefix", () => { + const script = getCompletionScript("powershell", createCompletionProgram()); + + expect(script).toContain("if ($commandPath -eq 'gateway') {"); + expect(script).toContain("if ($commandPath -eq 'gateway status') {"); + expect(script).not.toContain("if ($commandPath -eq 'openclaw gateway') {"); + expect(script).toContain("$completions = @('status','restart','--force')"); + }); + + it("generates fish completions for root and nested command contexts", () => { + const script = getCompletionScript("fish", createCompletionProgram()); + + expect(script).toContain( + 'complete -c openclaw -n "__fish_use_subcommand" -a "gateway" -d \'Gateway commands\'', + ); + expect(script).toContain( + 'complete -c openclaw -n "__fish_seen_subcommand_from gateway" -a "status" -d \'Show gateway status\'', + ); + expect(script).toContain( + "complete -c openclaw -n \"__fish_seen_subcommand_from gateway\" -l force -d 'Force the action'", + ); + }); +}); diff --git a/src/cli/completion-cli.ts b/src/cli/completion-cli.ts index 01cd02c018c..cbc235e41f9 100644 --- a/src/cli/completion-cli.ts +++ b/src/cli/completion-cli.ts @@ -69,7 +69,7 @@ export async function completionCacheExists( return pathExists(cachePath); } -function getCompletionScript(shell: CompletionShell, program: Command): string { +export function getCompletionScript(shell: CompletionShell, program: Command): string { if (shell === "zsh") { return generateZshCompletion(program); } @@ -442,17 +442,19 @@ function generateZshSubcmdList(cmd: Command): string { } function generateZshSubcommands(program: Command, prefix: string): string { - let script = ""; - for (const cmd of program.commands) { - const cmdName = cmd.name(); - const funcName = `_${prefix}_${cmdName.replace(/-/g, "_")}`; + const segments: string[] = []; - // Recurse first - script += generateZshSubcommands(cmd, `${prefix}_${cmdName.replace(/-/g, "_")}`); + const visit = (current: Command, currentPrefix: string) => { + for (const cmd of current.commands) { + const cmdName = cmd.name(); + const nextPrefix = `${currentPrefix}_${cmdName.replace(/-/g, "_")}`; + const funcName = `_${nextPrefix}`; - const subCommands = cmd.commands; - if (subCommands.length > 0) { - script += ` + visit(cmd, nextPrefix); + + const subCommands = cmd.commands; + if (subCommands.length > 0) { + segments.push(` ${funcName}() { local -a commands local -a options @@ -470,17 +472,21 @@ ${funcName}() { ;; esac } -`; - } else { - script += ` +`); + continue; + } + + segments.push(` ${funcName}() { _arguments -C \\ ${generateZshArgs(cmd)} } -`; +`); } - } - return script; + }; + + visit(program, prefix); + return segments.join(""); } function generateBashCompletion(program: Command): string { @@ -528,38 +534,34 @@ function generateBashSubcommand(cmd: Command): string { function generatePowerShellCompletion(program: Command): string { const rootCmd = program.name(); + const segments: string[] = []; - const visit = (cmd: Command, parents: string[]): string => { - const cmdName = cmd.name(); - const fullPath = [...parents, cmdName].join(" "); - - let script = ""; + const visit = (cmd: Command, pathSegments: string[]) => { + const fullPath = pathSegments.join(" "); // Command completion for this level const subCommands = cmd.commands.map((c) => c.name()); const options = cmd.options.map((o) => o.flags.split(/[ ,|]+/)[0]); // Take first flag const allCompletions = [...subCommands, ...options].map((s) => `'${s}'`).join(","); - if (allCompletions.length > 0) { - script += ` + if (fullPath.length > 0 && allCompletions.length > 0) { + segments.push(` if ($commandPath -eq '${fullPath}') { $completions = @(${allCompletions}) $completions | Where-Object { $_ -like "$wordToComplete*" } | ForEach-Object { [System.Management.Automation.CompletionResult]::new($_, $_, 'ParameterName', $_) } } -`; +`); } - // Recurse for (const sub of cmd.commands) { - script += visit(sub, [...parents, cmdName]); + visit(sub, [...pathSegments, sub.name()]); } - - return script; }; - const rootBody = visit(program, []); + visit(program, []); + const rootBody = segments.join(""); return ` Register-ArgumentCompleter -Native -CommandName ${rootCmd} -ScriptBlock { @@ -593,65 +595,57 @@ Register-ArgumentCompleter -Native -CommandName ${rootCmd} -ScriptBlock { function generateFishCompletion(program: Command): string { const rootCmd = program.name(); - let script = ""; + const segments: string[] = []; const visit = (cmd: Command, parents: string[]) => { const cmdName = cmd.name(); - const fullPath = [...parents]; - if (parents.length > 0) { - fullPath.push(cmdName); - } // Only push if not root, or consistent root handling - - // Fish uses 'seen_subcommand_from' to determine context. - // For root: complete -c openclaw -n "__fish_use_subcommand" -a "subcmd" -d "desc" // Root logic if (parents.length === 0) { // Subcommands of root for (const sub of cmd.commands) { - script += buildFishSubcommandCompletionLine({ - rootCmd, - condition: "__fish_use_subcommand", - name: sub.name(), - description: sub.description(), - }); + segments.push( + buildFishSubcommandCompletionLine({ + rootCmd, + condition: "__fish_use_subcommand", + name: sub.name(), + description: sub.description(), + }), + ); } // Options of root for (const opt of cmd.options) { - script += buildFishOptionCompletionLine({ - rootCmd, - condition: "__fish_use_subcommand", - flags: opt.flags, - description: opt.description, - }); + segments.push( + buildFishOptionCompletionLine({ + rootCmd, + condition: "__fish_use_subcommand", + flags: opt.flags, + description: opt.description, + }), + ); } } else { - // Nested commands - // Logic: if seen subcommand matches parents... - // But fish completion logic is simpler if we just say "if we haven't seen THIS command yet but seen parent" - // Actually, a robust fish completion often requires defining a function to check current line. - // For simplicity, we'll assume standard fish helper __fish_seen_subcommand_from. - - // To properly scope to 'openclaw gateway' and not 'openclaw other gateway', we need to check the sequence. - // A simplified approach: - // Subcommands for (const sub of cmd.commands) { - script += buildFishSubcommandCompletionLine({ - rootCmd, - condition: `__fish_seen_subcommand_from ${cmdName}`, - name: sub.name(), - description: sub.description(), - }); + segments.push( + buildFishSubcommandCompletionLine({ + rootCmd, + condition: `__fish_seen_subcommand_from ${cmdName}`, + name: sub.name(), + description: sub.description(), + }), + ); } // Options for (const opt of cmd.options) { - script += buildFishOptionCompletionLine({ - rootCmd, - condition: `__fish_seen_subcommand_from ${cmdName}`, - flags: opt.flags, - description: opt.description, - }); + segments.push( + buildFishOptionCompletionLine({ + rootCmd, + condition: `__fish_seen_subcommand_from ${cmdName}`, + flags: opt.flags, + description: opt.description, + }), + ); } } @@ -661,5 +655,5 @@ function generateFishCompletion(program: Command): string { }; visit(program, []); - return script; + return segments.join(""); } diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index c37cfbfd46c..ac6ff410268 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -986,6 +986,153 @@ describe("loadOpenClawPlugins", () => { expect(httpPlugin?.httpRoutes).toBe(1); }); + it("rejects duplicate plugin-visible hook names", () => { + useNoBundledPlugins(); + const first = writePlugin({ + id: "hook-owner-a", + filename: "hook-owner-a.cjs", + body: `module.exports = { id: "hook-owner-a", register(api) { + api.registerHook("gateway:startup", () => {}, { name: "shared-hook" }); +} };`, + }); + const second = writePlugin({ + id: "hook-owner-b", + filename: "hook-owner-b.cjs", + body: `module.exports = { id: "hook-owner-b", register(api) { + api.registerHook("gateway:startup", () => {}, { name: "shared-hook" }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [first.file, second.file] }, + allow: ["hook-owner-a", "hook-owner-b"], + }, + }, + }); + + expect(registry.hooks.filter((entry) => entry.entry.hook.name === "shared-hook")).toHaveLength( + 1, + ); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "hook-owner-b" && + diag.message === "hook already registered: shared-hook (hook-owner-a)", + ), + ).toBe(true); + }); + + it("rejects duplicate plugin service ids", () => { + useNoBundledPlugins(); + const first = writePlugin({ + id: "service-owner-a", + filename: "service-owner-a.cjs", + body: `module.exports = { id: "service-owner-a", register(api) { + api.registerService({ id: "shared-service", start() {} }); +} };`, + }); + const second = writePlugin({ + id: "service-owner-b", + filename: "service-owner-b.cjs", + body: `module.exports = { id: "service-owner-b", register(api) { + api.registerService({ id: "shared-service", start() {} }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [first.file, second.file] }, + allow: ["service-owner-a", "service-owner-b"], + }, + }, + }); + + expect(registry.services.filter((entry) => entry.service.id === "shared-service")).toHaveLength( + 1, + ); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "service-owner-b" && + diag.message === "service already registered: shared-service (service-owner-a)", + ), + ).toBe(true); + }); + + it("requires plugin CLI registrars to declare explicit command roots", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "cli-missing-metadata", + filename: "cli-missing-metadata.cjs", + body: `module.exports = { id: "cli-missing-metadata", register(api) { + api.registerCli(() => {}); +} };`, + }); + + const registry = loadRegistryFromSinglePlugin({ + plugin, + pluginConfig: { + allow: ["cli-missing-metadata"], + }, + }); + + expect(registry.cliRegistrars).toHaveLength(0); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "cli-missing-metadata" && + diag.message === "cli registration missing explicit commands metadata", + ), + ).toBe(true); + }); + + it("rejects duplicate plugin CLI command roots", () => { + useNoBundledPlugins(); + const first = writePlugin({ + id: "cli-owner-a", + filename: "cli-owner-a.cjs", + body: `module.exports = { id: "cli-owner-a", register(api) { + api.registerCli(() => {}, { commands: ["shared-cli"] }); +} };`, + }); + const second = writePlugin({ + id: "cli-owner-b", + filename: "cli-owner-b.cjs", + body: `module.exports = { id: "cli-owner-b", register(api) { + api.registerCli(() => {}, { commands: ["shared-cli"] }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [first.file, second.file] }, + allow: ["cli-owner-a", "cli-owner-b"], + }, + }, + }); + + expect(registry.cliRegistrars).toHaveLength(1); + expect(registry.cliRegistrars[0]?.pluginId).toBe("cli-owner-a"); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "cli-owner-b" && + diag.message === "cli command already registered: shared-cli (cli-owner-a)", + ), + ).toBe(true); + }); + it("registers http routes", () => { useNoBundledPlugins(); const plugin = writePlugin({ diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index ca987dc8e79..c1c63cc96cb 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -238,6 +238,16 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { }); return; } + const existingHook = registry.hooks.find((entry) => entry.entry.hook.name === name); + if (existingHook) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `hook already registered: ${name} (${existingHook.pluginId})`, + }); + return; + } const description = entry?.hook.description ?? opts?.description ?? ""; const hookEntry: HookEntry = entry @@ -473,6 +483,28 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { opts?: { commands?: string[] }, ) => { const commands = (opts?.commands ?? []).map((cmd) => cmd.trim()).filter(Boolean); + if (commands.length === 0) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: "cli registration missing explicit commands metadata", + }); + return; + } + const existing = registry.cliRegistrars.find((entry) => + entry.commands.some((command) => commands.includes(command)), + ); + if (existing) { + const overlap = commands.find((command) => existing.commands.includes(command)); + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `cli command already registered: ${overlap ?? commands[0]} (${existing.pluginId})`, + }); + return; + } record.cliCommands.push(...commands); registry.cliRegistrars.push({ pluginId: record.id, @@ -487,6 +519,16 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { if (!id) { return; } + const existing = registry.services.find((entry) => entry.service.id === id); + if (existing) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `service already registered: ${id} (${existing.pluginId})`, + }); + return; + } record.services.push(id); registry.services.push({ pluginId: record.id,