mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-16 12:30:49 +00:00
CLI/completion: fix generator OOM and harden plugin registries (#45537)
* fix: avoid OOM during completion script generation * CLI/completion: fix PowerShell nested command paths * CLI/completion: cover generated shell scripts * Changelog: note completion generator follow-up * Plugins: reserve shared registry names --------- Co-authored-by: Xiaoyi <xiaoyi@example.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
52
src/cli/completion-cli.test.ts
Normal file
52
src/cli/completion-cli.test.ts
Normal file
@@ -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'",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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("");
|
||||
}
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user