perf(cli): speed up help/config paths and route config get/unset
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import type { Command } from "commander";
|
||||
import JSON5 from "json5";
|
||||
import type { RuntimeEnv } from "../runtime.js";
|
||||
import { readConfigFileSnapshot, writeConfigFile } from "../config/config.js";
|
||||
import { danger, info } from "../globals.js";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
@@ -201,20 +202,81 @@ function unsetAtPath(root: Record<string, unknown>, path: PathSegment[]): boolea
|
||||
return true;
|
||||
}
|
||||
|
||||
async function loadValidConfig() {
|
||||
async function loadValidConfig(runtime: RuntimeEnv = defaultRuntime) {
|
||||
const snapshot = await readConfigFileSnapshot();
|
||||
if (snapshot.valid) {
|
||||
return snapshot;
|
||||
}
|
||||
defaultRuntime.error(`Config invalid at ${shortenHomePath(snapshot.path)}.`);
|
||||
runtime.error(`Config invalid at ${shortenHomePath(snapshot.path)}.`);
|
||||
for (const issue of snapshot.issues) {
|
||||
defaultRuntime.error(`- ${issue.path || "<root>"}: ${issue.message}`);
|
||||
runtime.error(`- ${issue.path || "<root>"}: ${issue.message}`);
|
||||
}
|
||||
defaultRuntime.error(`Run \`${formatCliCommand("openclaw doctor")}\` to repair, then retry.`);
|
||||
defaultRuntime.exit(1);
|
||||
runtime.error(`Run \`${formatCliCommand("openclaw doctor")}\` to repair, then retry.`);
|
||||
runtime.exit(1);
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
function parseRequiredPath(path: string): PathSegment[] {
|
||||
const parsedPath = parsePath(path);
|
||||
if (parsedPath.length === 0) {
|
||||
throw new Error("Path is empty.");
|
||||
}
|
||||
return parsedPath;
|
||||
}
|
||||
|
||||
export async function runConfigGet(opts: { path: string; json?: boolean; runtime?: RuntimeEnv }) {
|
||||
const runtime = opts.runtime ?? defaultRuntime;
|
||||
try {
|
||||
const parsedPath = parseRequiredPath(opts.path);
|
||||
const snapshot = await loadValidConfig(runtime);
|
||||
const res = getAtPath(snapshot.config, parsedPath);
|
||||
if (!res.found) {
|
||||
runtime.error(danger(`Config path not found: ${opts.path}`));
|
||||
runtime.exit(1);
|
||||
return;
|
||||
}
|
||||
if (opts.json) {
|
||||
runtime.log(JSON.stringify(res.value ?? null, null, 2));
|
||||
return;
|
||||
}
|
||||
if (
|
||||
typeof res.value === "string" ||
|
||||
typeof res.value === "number" ||
|
||||
typeof res.value === "boolean"
|
||||
) {
|
||||
runtime.log(String(res.value));
|
||||
return;
|
||||
}
|
||||
runtime.log(JSON.stringify(res.value ?? null, null, 2));
|
||||
} catch (err) {
|
||||
runtime.error(danger(String(err)));
|
||||
runtime.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
export async function runConfigUnset(opts: { path: string; runtime?: RuntimeEnv }) {
|
||||
const runtime = opts.runtime ?? defaultRuntime;
|
||||
try {
|
||||
const parsedPath = parseRequiredPath(opts.path);
|
||||
const snapshot = await loadValidConfig(runtime);
|
||||
// Use snapshot.resolved (config after $include and ${ENV} resolution, but BEFORE runtime defaults)
|
||||
// instead of snapshot.config (runtime-merged with defaults).
|
||||
// This prevents runtime defaults from leaking into the written config file (issue #6070)
|
||||
const next = structuredClone(snapshot.resolved) as Record<string, unknown>;
|
||||
const removed = unsetAtPath(next, parsedPath);
|
||||
if (!removed) {
|
||||
runtime.error(danger(`Config path not found: ${opts.path}`));
|
||||
runtime.exit(1);
|
||||
return;
|
||||
}
|
||||
await writeConfigFile(next);
|
||||
runtime.log(info(`Removed ${opts.path}. Restart the gateway to apply.`));
|
||||
} catch (err) {
|
||||
runtime.error(danger(String(err)));
|
||||
runtime.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
export function registerConfigCli(program: Command) {
|
||||
const cmd = program
|
||||
.command("config")
|
||||
@@ -261,35 +323,7 @@ export function registerConfigCli(program: Command) {
|
||||
.argument("<path>", "Config path (dot or bracket notation)")
|
||||
.option("--json", "Output JSON", false)
|
||||
.action(async (path: string, opts) => {
|
||||
try {
|
||||
const parsedPath = parsePath(path);
|
||||
if (parsedPath.length === 0) {
|
||||
throw new Error("Path is empty.");
|
||||
}
|
||||
const snapshot = await loadValidConfig();
|
||||
const res = getAtPath(snapshot.config, parsedPath);
|
||||
if (!res.found) {
|
||||
defaultRuntime.error(danger(`Config path not found: ${path}`));
|
||||
defaultRuntime.exit(1);
|
||||
return;
|
||||
}
|
||||
if (opts.json) {
|
||||
defaultRuntime.log(JSON.stringify(res.value ?? null, null, 2));
|
||||
return;
|
||||
}
|
||||
if (
|
||||
typeof res.value === "string" ||
|
||||
typeof res.value === "number" ||
|
||||
typeof res.value === "boolean"
|
||||
) {
|
||||
defaultRuntime.log(String(res.value));
|
||||
return;
|
||||
}
|
||||
defaultRuntime.log(JSON.stringify(res.value ?? null, null, 2));
|
||||
} catch (err) {
|
||||
defaultRuntime.error(danger(String(err)));
|
||||
defaultRuntime.exit(1);
|
||||
}
|
||||
await runConfigGet({ path, json: Boolean(opts.json) });
|
||||
});
|
||||
|
||||
cmd
|
||||
@@ -324,27 +358,6 @@ export function registerConfigCli(program: Command) {
|
||||
.description("Remove a config value by dot path")
|
||||
.argument("<path>", "Config path (dot or bracket notation)")
|
||||
.action(async (path: string) => {
|
||||
try {
|
||||
const parsedPath = parsePath(path);
|
||||
if (parsedPath.length === 0) {
|
||||
throw new Error("Path is empty.");
|
||||
}
|
||||
const snapshot = await loadValidConfig();
|
||||
// Use snapshot.resolved (config after $include and ${ENV} resolution, but BEFORE runtime defaults)
|
||||
// instead of snapshot.config (runtime-merged with defaults).
|
||||
// This prevents runtime defaults from leaking into the written config file (issue #6070)
|
||||
const next = structuredClone(snapshot.resolved) as Record<string, unknown>;
|
||||
const removed = unsetAtPath(next, parsedPath);
|
||||
if (!removed) {
|
||||
defaultRuntime.error(danger(`Config path not found: ${path}`));
|
||||
defaultRuntime.exit(1);
|
||||
return;
|
||||
}
|
||||
await writeConfigFile(next);
|
||||
defaultRuntime.log(info(`Removed ${path}. Restart the gateway to apply.`));
|
||||
} catch (err) {
|
||||
defaultRuntime.error(danger(String(err)));
|
||||
defaultRuntime.exit(1);
|
||||
}
|
||||
await runConfigUnset({ path });
|
||||
});
|
||||
}
|
||||
|
||||
@@ -23,4 +23,16 @@ describe("program routes", () => {
|
||||
it("does not match unknown routes", () => {
|
||||
expect(findRoutedCommand(["definitely-not-real"])).toBeNull();
|
||||
});
|
||||
|
||||
it("returns false for config get route when path argument is missing", async () => {
|
||||
const route = findRoutedCommand(["config", "get"]);
|
||||
expect(route).not.toBeNull();
|
||||
await expect(route?.run(["node", "openclaw", "config", "get", "--json"])).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for config unset route when path argument is missing", async () => {
|
||||
const route = findRoutedCommand(["config", "unset"]);
|
||||
expect(route).not.toBeNull();
|
||||
await expect(route?.run(["node", "openclaw", "config", "unset"])).resolves.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -88,12 +88,58 @@ const routeMemoryStatus: RouteSpec = {
|
||||
},
|
||||
};
|
||||
|
||||
function getCommandPositionals(argv: string[]): string[] {
|
||||
const out: string[] = [];
|
||||
const args = argv.slice(2);
|
||||
for (const arg of args) {
|
||||
if (!arg || arg === "--") {
|
||||
break;
|
||||
}
|
||||
if (arg.startsWith("-")) {
|
||||
continue;
|
||||
}
|
||||
out.push(arg);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const routeConfigGet: RouteSpec = {
|
||||
match: (path) => path[0] === "config" && path[1] === "get",
|
||||
run: async (argv) => {
|
||||
const positionals = getCommandPositionals(argv);
|
||||
const pathArg = positionals[2];
|
||||
if (!pathArg) {
|
||||
return false;
|
||||
}
|
||||
const json = hasFlag(argv, "--json");
|
||||
const { runConfigGet } = await import("../config-cli.js");
|
||||
await runConfigGet({ path: pathArg, json });
|
||||
return true;
|
||||
},
|
||||
};
|
||||
|
||||
const routeConfigUnset: RouteSpec = {
|
||||
match: (path) => path[0] === "config" && path[1] === "unset",
|
||||
run: async (argv) => {
|
||||
const positionals = getCommandPositionals(argv);
|
||||
const pathArg = positionals[2];
|
||||
if (!pathArg) {
|
||||
return false;
|
||||
}
|
||||
const { runConfigUnset } = await import("../config-cli.js");
|
||||
await runConfigUnset({ path: pathArg });
|
||||
return true;
|
||||
},
|
||||
};
|
||||
|
||||
const routes: RouteSpec[] = [
|
||||
routeHealth,
|
||||
routeStatus,
|
||||
routeSessions,
|
||||
routeAgentsList,
|
||||
routeMemoryStatus,
|
||||
routeConfigGet,
|
||||
routeConfigUnset,
|
||||
];
|
||||
|
||||
export function findRoutedCommand(path: string[]): RouteSpec | null {
|
||||
|
||||
13
src/cli/respawn-policy.test.ts
Normal file
13
src/cli/respawn-policy.test.ts
Normal file
@@ -0,0 +1,13 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { shouldSkipRespawnForArgv } from "./respawn-policy.js";
|
||||
|
||||
describe("shouldSkipRespawnForArgv", () => {
|
||||
it("skips respawn for help/version calls", () => {
|
||||
expect(shouldSkipRespawnForArgv(["node", "openclaw", "--help"])).toBe(true);
|
||||
expect(shouldSkipRespawnForArgv(["node", "openclaw", "-V"])).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps respawn path for normal commands", () => {
|
||||
expect(shouldSkipRespawnForArgv(["node", "openclaw", "status"])).toBe(false);
|
||||
});
|
||||
});
|
||||
5
src/cli/respawn-policy.ts
Normal file
5
src/cli/respawn-policy.ts
Normal file
@@ -0,0 +1,5 @@
|
||||
import { hasHelpOrVersion } from "./argv.js";
|
||||
|
||||
export function shouldSkipRespawnForArgv(argv: string[]): boolean {
|
||||
return hasHelpOrVersion(argv);
|
||||
}
|
||||
@@ -1,5 +1,9 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { rewriteUpdateFlagArgv } from "./run-main.js";
|
||||
import {
|
||||
rewriteUpdateFlagArgv,
|
||||
shouldRegisterPrimarySubcommand,
|
||||
shouldSkipPluginCommandRegistration,
|
||||
} from "./run-main.js";
|
||||
|
||||
describe("rewriteUpdateFlagArgv", () => {
|
||||
it("leaves argv unchanged when --update is absent", () => {
|
||||
@@ -34,3 +38,46 @@ describe("rewriteUpdateFlagArgv", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldRegisterPrimarySubcommand", () => {
|
||||
it("skips eager primary registration for help/version invocations", () => {
|
||||
expect(shouldRegisterPrimarySubcommand(["node", "openclaw", "status", "--help"])).toBe(false);
|
||||
expect(shouldRegisterPrimarySubcommand(["node", "openclaw", "-V"])).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps eager primary registration for regular command runs", () => {
|
||||
expect(shouldRegisterPrimarySubcommand(["node", "openclaw", "status"])).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldSkipPluginCommandRegistration", () => {
|
||||
it("skips plugin registration for root help/version", () => {
|
||||
expect(
|
||||
shouldSkipPluginCommandRegistration({
|
||||
argv: ["node", "openclaw", "--help"],
|
||||
primary: null,
|
||||
hasBuiltinPrimary: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("skips plugin registration for builtin subcommand help", () => {
|
||||
expect(
|
||||
shouldSkipPluginCommandRegistration({
|
||||
argv: ["node", "openclaw", "config", "--help"],
|
||||
primary: "config",
|
||||
hasBuiltinPrimary: true,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps plugin registration for non-builtin help", () => {
|
||||
expect(
|
||||
shouldSkipPluginCommandRegistration({
|
||||
argv: ["node", "openclaw", "voicecall", "--help"],
|
||||
primary: "voicecall",
|
||||
hasBuiltinPrimary: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -24,6 +24,24 @@ export function rewriteUpdateFlagArgv(argv: string[]): string[] {
|
||||
return next;
|
||||
}
|
||||
|
||||
export function shouldRegisterPrimarySubcommand(argv: string[]): boolean {
|
||||
return !hasHelpOrVersion(argv);
|
||||
}
|
||||
|
||||
export function shouldSkipPluginCommandRegistration(params: {
|
||||
argv: string[];
|
||||
primary: string | null;
|
||||
hasBuiltinPrimary: boolean;
|
||||
}): boolean {
|
||||
if (!hasHelpOrVersion(params.argv)) {
|
||||
return false;
|
||||
}
|
||||
if (!params.primary) {
|
||||
return true;
|
||||
}
|
||||
return params.hasBuiltinPrimary;
|
||||
}
|
||||
|
||||
export async function runCli(argv: string[] = process.argv) {
|
||||
const normalizedArgv = stripWindowsNodeExec(argv);
|
||||
loadDotEnv({ quiet: true });
|
||||
@@ -55,12 +73,18 @@ export async function runCli(argv: string[] = process.argv) {
|
||||
const parseArgv = rewriteUpdateFlagArgv(normalizedArgv);
|
||||
// Register the primary subcommand if one exists (for lazy-loading)
|
||||
const primary = getPrimaryCommand(parseArgv);
|
||||
if (primary) {
|
||||
if (primary && shouldRegisterPrimarySubcommand(parseArgv)) {
|
||||
const { registerSubCliByName } = await import("./program/register.subclis.js");
|
||||
await registerSubCliByName(program, primary);
|
||||
}
|
||||
|
||||
const shouldSkipPluginRegistration = !primary && hasHelpOrVersion(parseArgv);
|
||||
const hasBuiltinPrimary =
|
||||
primary !== null && program.commands.some((command) => command.name() === primary);
|
||||
const shouldSkipPluginRegistration = shouldSkipPluginCommandRegistration({
|
||||
argv: parseArgv,
|
||||
primary,
|
||||
hasBuiltinPrimary,
|
||||
});
|
||||
if (!shouldSkipPluginRegistration) {
|
||||
// Register plugin CLI commands before parsing
|
||||
const { registerPluginCliCommands } = await import("../plugins/cli.js");
|
||||
|
||||
@@ -3,6 +3,7 @@ import { spawn } from "node:child_process";
|
||||
import path from "node:path";
|
||||
import process from "node:process";
|
||||
import { applyCliProfileEnv, parseCliProfileArgs } from "./cli/profile.js";
|
||||
import { shouldSkipRespawnForArgv } from "./cli/respawn-policy.js";
|
||||
import { isTruthyEnvValue, normalizeEnv } from "./infra/env.js";
|
||||
import { installProcessWarningFilter } from "./infra/warning-filter.js";
|
||||
import { attachChildProcessBridge } from "./process/child-process-bridge.js";
|
||||
@@ -32,6 +33,9 @@ function hasExperimentalWarningSuppressed(): boolean {
|
||||
}
|
||||
|
||||
function ensureExperimentalWarningSuppressed(): boolean {
|
||||
if (shouldSkipRespawnForArgv(process.argv)) {
|
||||
return false;
|
||||
}
|
||||
if (isTruthyEnvValue(process.env.OPENCLAW_NO_RESPAWN)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user