diff --git a/CHANGELOG.md b/CHANGELOG.md index 80bcd534a81..3cc6263f245 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Docs: https://docs.openclaw.ai - OpenAI/Responses WebSocket tool-call id hygiene: normalize blank/whitespace streamed tool-call ids before persistence, and block empty `function_call_output.call_id` payloads in the WS conversion path to avoid OpenAI 400 errors (`Invalid 'input[n].call_id': empty string`), with regression coverage for both inbound stream normalization and outbound payload guards. - Gateway/Control UI basePath webhook passthrough: let non-read methods under configured `controlUiBasePath` fall through to plugin routes (instead of returning Control UI 405), restoring webhook handlers behind basePath mounts. (#32311) Thanks @ademczuk. +- CLI/Config validation and routing hardening: dedupe `openclaw config validate` failures to a single authoritative report, expose allowed-values metadata/hints across core Zod and plugin AJV validation (including `--json` fields), sanitize terminal-rendered validation text, and make command-path parsing root-option-aware across preaction/route/lazy registration (including routed `config get/unset` with split root options). Thanks @gumadeiras. - Models/config env propagation: apply `config.env.vars` before implicit provider discovery in models bootstrap so config-scoped credentials are visible to implicit provider resolution paths. (#32295) Thanks @hsiaoa. - Hooks/runtime stability: keep the internal hook handler registry on a `globalThis` singleton so hook registration/dispatch remains consistent when bundling emits duplicate module copies. (#32292) Thanks @Drickon. - Restart sentinel formatting: avoid duplicate `Reason:` lines when restart message text already matches `stats.reason`, keeping restart notifications concise for users and downstream parsers. (#32083) Thanks @velamints2. diff --git a/src/agents/context.lookup.test.ts b/src/agents/context.lookup.test.ts new file mode 100644 index 00000000000..5870be401a4 --- /dev/null +++ b/src/agents/context.lookup.test.ts @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +describe("lookupContextTokens", () => { + beforeEach(() => { + vi.resetModules(); + }); + + it("returns configured model context window on first lookup", async () => { + vi.doMock("../config/config.js", () => ({ + loadConfig: () => ({ + models: { + providers: { + openrouter: { + models: [{ id: "openrouter/claude-sonnet", contextWindow: 321_000 }], + }, + }, + }, + }), + })); + vi.doMock("./models-config.js", () => ({ + ensureOpenClawModelsJson: vi.fn(async () => {}), + })); + vi.doMock("./agent-paths.js", () => ({ + resolveOpenClawAgentDir: () => "/tmp/openclaw-agent", + })); + vi.doMock("./pi-model-discovery.js", () => ({ + discoverAuthStorage: vi.fn(() => ({})), + discoverModels: vi.fn(() => ({ + getAll: () => [], + })), + })); + + const { lookupContextTokens } = await import("./context.js"); + expect(lookupContextTokens("openrouter/claude-sonnet")).toBe(321_000); + }); + + it("does not skip eager warmup when --profile is followed by -- terminator", async () => { + const loadConfigMock = vi.fn(() => ({ models: {} })); + vi.doMock("../config/config.js", () => ({ + loadConfig: loadConfigMock, + })); + vi.doMock("./models-config.js", () => ({ + ensureOpenClawModelsJson: vi.fn(async () => {}), + })); + vi.doMock("./agent-paths.js", () => ({ + resolveOpenClawAgentDir: () => "/tmp/openclaw-agent", + })); + vi.doMock("./pi-model-discovery.js", () => ({ + discoverAuthStorage: vi.fn(() => ({})), + discoverModels: vi.fn(() => ({ + getAll: () => [], + })), + })); + + const argvSnapshot = process.argv; + process.argv = ["node", "openclaw", "--profile", "--", "config", "validate"]; + try { + await import("./context.js"); + expect(loadConfigMock).toHaveBeenCalledTimes(1); + } finally { + process.argv = argvSnapshot; + } + }); +}); diff --git a/src/agents/context.ts b/src/agents/context.ts index 2cb0f5296fa..50e549877ea 100644 --- a/src/agents/context.ts +++ b/src/agents/context.ts @@ -3,6 +3,7 @@ import { loadConfig } from "../config/config.js"; import type { OpenClawConfig } from "../config/config.js"; +import { consumeRootOptionToken, FLAG_TERMINATOR } from "../infra/cli-root-options.js"; import { resolveOpenClawAgentDir } from "./agent-paths.js"; import { ensureOpenClawModelsJson } from "./models-config.js"; @@ -66,55 +67,114 @@ export function applyConfiguredContextWindows(params: { } const MODEL_CACHE = new Map(); -const loadPromise = (async () => { - let cfg: ReturnType | undefined; +let loadPromise: Promise | null = null; +let configuredWindowsPrimed = false; + +function getCommandPathFromArgv(argv: string[]): string[] { + const args = argv.slice(2); + const tokens: string[] = []; + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; + if (!arg || arg === FLAG_TERMINATOR) { + break; + } + const consumed = consumeRootOptionToken(args, i); + if (consumed > 0) { + i += consumed - 1; + continue; + } + if (arg.startsWith("-")) { + continue; + } + tokens.push(arg); + if (tokens.length >= 2) { + break; + } + } + return tokens; +} + +function shouldSkipEagerContextWindowWarmup(argv: string[] = process.argv): boolean { + const [primary, secondary] = getCommandPathFromArgv(argv); + return primary === "config" && secondary === "validate"; +} + +function primeConfiguredContextWindows(): OpenClawConfig | undefined { + if (configuredWindowsPrimed) { + return undefined; + } + configuredWindowsPrimed = true; try { - cfg = loadConfig(); + const cfg = loadConfig(); + applyConfiguredContextWindows({ + cache: MODEL_CACHE, + modelsConfig: cfg.models as ModelsConfig | undefined, + }); + return cfg; } catch { // If config can't be loaded, leave cache empty. - return; + return undefined; } +} - try { - await ensureOpenClawModelsJson(cfg); - } catch { - // Continue with best-effort discovery/overrides. +function ensureContextWindowCacheLoaded(): Promise { + const cfg = primeConfiguredContextWindows(); + if (loadPromise) { + return loadPromise; } + loadPromise = (async () => { + if (!cfg) { + return; + } - try { - const { discoverAuthStorage, discoverModels } = await import("./pi-model-discovery.js"); - const agentDir = resolveOpenClawAgentDir(); - const authStorage = discoverAuthStorage(agentDir); - const modelRegistry = discoverModels(authStorage, agentDir) as unknown as ModelRegistryLike; - const models = - typeof modelRegistry.getAvailable === "function" - ? modelRegistry.getAvailable() - : modelRegistry.getAll(); - applyDiscoveredContextWindows({ + try { + await ensureOpenClawModelsJson(cfg); + } catch { + // Continue with best-effort discovery/overrides. + } + + try { + const { discoverAuthStorage, discoverModels } = await import("./pi-model-discovery.js"); + const agentDir = resolveOpenClawAgentDir(); + const authStorage = discoverAuthStorage(agentDir); + const modelRegistry = discoverModels(authStorage, agentDir) as unknown as ModelRegistryLike; + const models = + typeof modelRegistry.getAvailable === "function" + ? modelRegistry.getAvailable() + : modelRegistry.getAll(); + applyDiscoveredContextWindows({ + cache: MODEL_CACHE, + models, + }); + } catch { + // If model discovery fails, continue with config overrides only. + } + + applyConfiguredContextWindows({ cache: MODEL_CACHE, - models, + modelsConfig: cfg.models as ModelsConfig | undefined, }); - } catch { - // If model discovery fails, continue with config overrides only. - } - - applyConfiguredContextWindows({ - cache: MODEL_CACHE, - modelsConfig: cfg.models as ModelsConfig | undefined, + })().catch(() => { + // Keep lookup best-effort. }); -})().catch(() => { - // Keep lookup best-effort. -}); + return loadPromise; +} export function lookupContextTokens(modelId?: string): number | undefined { if (!modelId) { return undefined; } // Best-effort: kick off loading, but don't block. - void loadPromise; + void ensureContextWindowCacheLoaded(); return MODEL_CACHE.get(modelId); } +if (!shouldSkipEagerContextWindowWarmup()) { + // Keep prior behavior where model limits begin loading during startup. + // This avoids a cold-start miss on the first context token lookup. + void ensureContextWindowCacheLoaded(); +} + function resolveConfiguredModelParams( cfg: OpenClawConfig | undefined, provider: string, diff --git a/src/cli/argv.test.ts b/src/cli/argv.test.ts index fd7ed71d529..665b47c4f60 100644 --- a/src/cli/argv.test.ts +++ b/src/cli/argv.test.ts @@ -3,6 +3,7 @@ import { buildParseArgv, getFlagValue, getCommandPath, + getCommandPathWithRootOptions, getPrimaryCommand, getPositiveIntFlagValue, getVerboseFlag, @@ -160,6 +161,15 @@ describe("argv helpers", () => { expect(getCommandPath(argv, 2)).toEqual(expected); }); + it("extracts command path while skipping known root option values", () => { + expect( + getCommandPathWithRootOptions( + ["node", "openclaw", "--profile", "work", "--no-color", "config", "validate"], + 2, + ), + ).toEqual(["config", "validate"]); + }); + it.each([ { name: "returns first command token", @@ -171,6 +181,11 @@ describe("argv helpers", () => { argv: ["node", "openclaw"], expected: null, }, + { + name: "skips known root option values", + argv: ["node", "openclaw", "--log-level", "debug", "status"], + expected: "status", + }, ])("returns primary command: $name", ({ argv, expected }) => { expect(getPrimaryCommand(argv)).toBe(expected); }); diff --git a/src/cli/argv.ts b/src/cli/argv.ts index ecc33d689e5..ca989dc4a4b 100644 --- a/src/cli/argv.ts +++ b/src/cli/argv.ts @@ -1,11 +1,13 @@ import { isBunRuntime, isNodeRuntime } from "../daemon/runtime-binary.js"; +import { + consumeRootOptionToken, + FLAG_TERMINATOR, + isValueToken, +} from "../infra/cli-root-options.js"; const HELP_FLAGS = new Set(["-h", "--help"]); const VERSION_FLAGS = new Set(["-V", "--version"]); const ROOT_VERSION_ALIAS_FLAG = "-v"; -const ROOT_BOOLEAN_FLAGS = new Set(["--dev", "--no-color"]); -const ROOT_VALUE_FLAGS = new Set(["--profile", "--log-level"]); -const FLAG_TERMINATOR = "--"; export function hasHelpOrVersion(argv: string[]): boolean { return ( @@ -13,19 +15,6 @@ export function hasHelpOrVersion(argv: string[]): boolean { ); } -function isValueToken(arg: string | undefined): boolean { - if (!arg) { - return false; - } - if (arg === FLAG_TERMINATOR) { - return false; - } - if (!arg.startsWith("-")) { - return true; - } - return /^-\d+(?:\.\d+)?$/.test(arg); -} - function parsePositiveInt(value: string): number | undefined { const parsed = Number.parseInt(value, 10); if (Number.isNaN(parsed) || parsed <= 0) { @@ -62,17 +51,9 @@ export function hasRootVersionAlias(argv: string[]): boolean { hasAlias = true; continue; } - 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; - } + const consumed = consumeRootOptionToken(args, i); + if (consumed > 0) { + i += consumed - 1; continue; } if (arg.startsWith("-")) { @@ -109,17 +90,9 @@ function isRootInvocationForFlags( hasTarget = true; continue; } - if (ROOT_BOOLEAN_FLAGS.has(arg)) { - continue; - } - if (arg.startsWith("--profile=") || arg.startsWith("--log-level=")) { - continue; - } - if (ROOT_VALUE_FLAGS.has(arg)) { - const next = args[i + 1]; - if (isValueToken(next)) { - i += 1; - } + const consumed = consumeRootOptionToken(args, i); + if (consumed > 0) { + i += consumed - 1; continue; } // Unknown flags and subcommand-scoped help/version should fall back to Commander. @@ -170,6 +143,18 @@ export function getPositiveIntFlagValue(argv: string[], name: string): number | } export function getCommandPath(argv: string[], depth = 2): string[] { + return getCommandPathInternal(argv, depth, { skipRootOptions: false }); +} + +export function getCommandPathWithRootOptions(argv: string[], depth = 2): string[] { + return getCommandPathInternal(argv, depth, { skipRootOptions: true }); +} + +function getCommandPathInternal( + argv: string[], + depth: number, + opts: { skipRootOptions: boolean }, +): string[] { const args = argv.slice(2); const path: string[] = []; for (let i = 0; i < args.length; i += 1) { @@ -180,6 +165,13 @@ export function getCommandPath(argv: string[], depth = 2): string[] { if (arg === "--") { break; } + if (opts.skipRootOptions) { + const consumed = consumeRootOptionToken(args, i); + if (consumed > 0) { + i += consumed - 1; + continue; + } + } if (arg.startsWith("-")) { continue; } @@ -192,7 +184,7 @@ export function getCommandPath(argv: string[], depth = 2): string[] { } export function getPrimaryCommand(argv: string[]): string | null { - const [primary] = getCommandPath(argv, 1); + const [primary] = getCommandPathWithRootOptions(argv, 1); return primary ?? null; } diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index 0e2ee488500..c2130dc2df4 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -252,6 +252,55 @@ describe("config cli", () => { expect(mockError).not.toHaveBeenCalled(); }); + it("preserves allowed-values metadata in --json output", async () => { + setSnapshotOnce({ + path: "/tmp/custom-openclaw.json", + exists: true, + raw: "{}", + parsed: {}, + resolved: {}, + valid: false, + config: {}, + issues: [ + { + path: "update.channel", + message: 'Invalid input (allowed: "stable", "beta", "dev")', + allowedValues: ["stable", "beta", "dev"], + allowedValuesHiddenCount: 0, + }, + ], + warnings: [], + legacyIssues: [], + }); + + await expect(runConfigCommand(["config", "validate", "--json"])).rejects.toThrow( + "__exit__:1", + ); + + const raw = mockLog.mock.calls.at(0)?.[0]; + expect(typeof raw).toBe("string"); + const payload = JSON.parse(String(raw)) as { + valid: boolean; + path: string; + issues: Array<{ + path: string; + message: string; + allowedValues?: string[]; + allowedValuesHiddenCount?: number; + }>; + }; + expect(payload.valid).toBe(false); + expect(payload.path).toBe("/tmp/custom-openclaw.json"); + expect(payload.issues).toEqual([ + { + path: "update.channel", + message: 'Invalid input (allowed: "stable", "beta", "dev")', + allowedValues: ["stable", "beta", "dev"], + }, + ]); + expect(mockError).not.toHaveBeenCalled(); + }); + it("prints file-not-found and exits 1 when config file is missing", async () => { setSnapshotOnce({ path: "/tmp/openclaw.json", diff --git a/src/cli/config-cli.ts b/src/cli/config-cli.ts index d73d340b7c3..4793ff6bea6 100644 --- a/src/cli/config-cli.ts +++ b/src/cli/config-cli.ts @@ -1,6 +1,7 @@ import type { Command } from "commander"; import JSON5 from "json5"; import { readConfigFileSnapshot, writeConfigFile } from "../config/config.js"; +import { formatConfigIssueLines, normalizeConfigIssues } from "../config/issue-format.js"; import { CONFIG_PATH } from "../config/paths.js"; import { isBlockedObjectKey } from "../config/prototype-keys.js"; import { redactConfigObject } from "../config/redact-snapshot.js"; @@ -16,10 +17,6 @@ type PathSegment = string; type ConfigSetParseOpts = { strictJson?: boolean; }; -type ConfigIssue = { - path: string; - message: string; -}; const OLLAMA_API_KEY_PATH: PathSegment[] = ["models", "providers", "ollama", "apiKey"]; const OLLAMA_PROVIDER_PATH: PathSegment[] = ["models", "providers", "ollama"]; @@ -102,17 +99,6 @@ function hasOwnPathKey(value: Record, key: string): boolean { return Object.prototype.hasOwnProperty.call(value, key); } -function normalizeConfigIssues(issues: ReadonlyArray): ConfigIssue[] { - return issues.map((issue) => ({ - path: issue.path || "", - message: issue.message, - })); -} - -function formatConfigIssueLines(issues: ReadonlyArray, marker: string): string[] { - return normalizeConfigIssues(issues).map((issue) => `${marker} ${issue.path}: ${issue.message}`); -} - function formatDoctorHint(message: string): string { return `Run \`${formatCliCommand("openclaw doctor")}\` ${message}`; } @@ -249,7 +235,7 @@ async function loadValidConfig(runtime: RuntimeEnv = defaultRuntime) { return snapshot; } runtime.error(`Config invalid at ${shortenHomePath(snapshot.path)}.`); - for (const line of formatConfigIssueLines(snapshot.issues, "-")) { + for (const line of formatConfigIssueLines(snapshot.issues, "-", { normalizeRoot: true })) { runtime.error(line); } runtime.error(formatDoctorHint("to repair, then retry.")); @@ -381,7 +367,7 @@ export async function runConfigValidate(opts: { json?: boolean; runtime?: Runtim runtime.log(JSON.stringify({ valid: false, path: outputPath, issues }, null, 2)); } else { runtime.error(danger(`Config invalid at ${shortPath}:`)); - for (const line of formatConfigIssueLines(issues, danger("×"))) { + for (const line of formatConfigIssueLines(issues, danger("×"), { normalizeRoot: true })) { runtime.error(` ${line}`); } runtime.error(""); diff --git a/src/cli/daemon-cli/status.print.ts b/src/cli/daemon-cli/status.print.ts index 27787550c90..ce9934f7ed4 100644 --- a/src/cli/daemon-cli/status.print.ts +++ b/src/cli/daemon-cli/status.print.ts @@ -1,4 +1,5 @@ import { resolveControlUiLinks } from "../../commands/onboard-helpers.js"; +import { formatConfigIssueLine } from "../../config/issue-format.js"; import { resolveGatewayLaunchAgentLabel, resolveGatewaySystemdServiceName, @@ -110,7 +111,7 @@ export function printDaemonStatus(status: DaemonStatus, opts: { json: boolean }) if (!status.config.cli.valid && status.config.cli.issues?.length) { for (const issue of status.config.cli.issues.slice(0, 5)) { defaultRuntime.error( - `${errorText("Config issue:")} ${issue.path || ""}: ${issue.message}`, + `${errorText("Config issue:")} ${formatConfigIssueLine(issue, "", { normalizeRoot: true })}`, ); } } @@ -120,7 +121,7 @@ export function printDaemonStatus(status: DaemonStatus, opts: { json: boolean }) if (!status.config.daemon.valid && status.config.daemon.issues?.length) { for (const issue of status.config.daemon.issues.slice(0, 5)) { defaultRuntime.error( - `${errorText("Service config issue:")} ${issue.path || ""}: ${issue.message}`, + `${errorText("Service config issue:")} ${formatConfigIssueLine(issue, "", { normalizeRoot: true })}`, ); } } diff --git a/src/cli/program/config-guard.ts b/src/cli/program/config-guard.ts index 06c45a8ea58..48ca6c26e88 100644 --- a/src/cli/program/config-guard.ts +++ b/src/cli/program/config-guard.ts @@ -1,5 +1,6 @@ import { loadAndMaybeMigrateDoctorConfig } from "../../commands/doctor-config-flow.js"; import { readConfigFileSnapshot } from "../../config/config.js"; +import { formatConfigIssueLines } from "../../config/issue-format.js"; import type { RuntimeEnv } from "../../runtime.js"; import { colorize, isRich, theme } from "../../terminal/theme.js"; import { shortenHomePath } from "../../utils.js"; @@ -28,10 +29,6 @@ function resetConfigGuardStateForTests() { configSnapshotPromise = null; } -function formatConfigIssues(issues: Array<{ path: string; message: string }>): string[] { - return issues.map((issue) => `- ${issue.path || ""}: ${issue.message}`); -} - async function getConfigSnapshot() { // Tests often mutate config fixtures; caching can make those flaky. if (process.env.VITEST === "true") { @@ -83,11 +80,12 @@ export async function ensureConfigReady(params: { subcommandName && ALLOWED_INVALID_GATEWAY_SUBCOMMANDS.has(subcommandName)) : false; - const issues = snapshot.exists && !snapshot.valid ? formatConfigIssues(snapshot.issues) : []; - const legacyIssues = - snapshot.legacyIssues.length > 0 - ? snapshot.legacyIssues.map((issue) => `- ${issue.path}: ${issue.message}`) + const issues = + snapshot.exists && !snapshot.valid + ? formatConfigIssueLines(snapshot.issues, "-", { normalizeRoot: true }) : []; + const legacyIssues = + snapshot.legacyIssues.length > 0 ? formatConfigIssueLines(snapshot.legacyIssues, "-") : []; const invalid = snapshot.exists && !snapshot.valid; if (!invalid) { diff --git a/src/cli/program/preaction.test.ts b/src/cli/program/preaction.test.ts index b2fded5ed71..065abb3bbf7 100644 --- a/src/cli/program/preaction.test.ts +++ b/src/cli/program/preaction.test.ts @@ -103,6 +103,10 @@ describe("registerPreActionHooks", () => { .argument("") .option("--json") .action(() => {}); + config + .command("validate") + .option("--json") + .action(() => {}); registerPreActionHooks(program, "9.9.9-test"); return program; } @@ -204,6 +208,24 @@ describe("registerPreActionHooks", () => { }); }); + it("bypasses config guard for config validate", async () => { + await runPreAction({ + parseArgv: ["config", "validate"], + processArgv: ["node", "openclaw", "config", "validate"], + }); + + expect(ensureConfigReadyMock).not.toHaveBeenCalled(); + }); + + it("bypasses config guard for config validate when root option values are present", async () => { + await runPreAction({ + parseArgv: ["config", "validate"], + processArgv: ["node", "openclaw", "--profile", "work", "config", "validate"], + }); + + expect(ensureConfigReadyMock).not.toHaveBeenCalled(); + }); + beforeAll(() => { program = buildProgram(); const hooks = ( diff --git a/src/cli/program/preaction.ts b/src/cli/program/preaction.ts index c3adce61e19..5984df6e4f4 100644 --- a/src/cli/program/preaction.ts +++ b/src/cli/program/preaction.ts @@ -3,7 +3,12 @@ import { setVerbose } from "../../globals.js"; import { isTruthyEnvValue } from "../../infra/env.js"; import type { LogLevel } from "../../logging/levels.js"; import { defaultRuntime } from "../../runtime.js"; -import { getCommandPath, getVerboseFlag, hasFlag, hasHelpOrVersion } from "../argv.js"; +import { + getCommandPathWithRootOptions, + getVerboseFlag, + hasFlag, + hasHelpOrVersion, +} from "../argv.js"; import { emitCliBanner } from "../banner.js"; import { resolveCliName } from "../cli-name.js"; @@ -34,6 +39,22 @@ const JSON_PARSE_ONLY_COMMANDS = new Set(["config set"]); let configGuardModulePromise: Promise | undefined; let pluginRegistryModulePromise: Promise | undefined; +function shouldBypassConfigGuard(commandPath: string[]): boolean { + const [primary, secondary] = commandPath; + if (!primary) { + return false; + } + if (CONFIG_GUARD_BYPASS_COMMANDS.has(primary)) { + return true; + } + // config validate is the explicit validation command; let it render + // validation failures directly without preflight guard output duplication. + if (primary === "config" && secondary === "validate") { + return true; + } + return false; +} + function loadConfigGuardModule() { configGuardModulePromise ??= import("./config-guard.js"); return configGuardModulePromise; @@ -82,7 +103,7 @@ export function registerPreActionHooks(program: Command, programVersion: string) if (hasHelpOrVersion(argv)) { return; } - const commandPath = getCommandPath(argv, 2); + const commandPath = getCommandPathWithRootOptions(argv, 2); const hideBanner = isTruthyEnvValue(process.env.OPENCLAW_HIDE_BANNER) || commandPath[0] === "update" || @@ -100,7 +121,7 @@ export function registerPreActionHooks(program: Command, programVersion: string) if (!verbose) { process.env.NODE_NO_WARNINGS ??= "1"; } - if (CONFIG_GUARD_BYPASS_COMMANDS.has(commandPath[0])) { + if (shouldBypassConfigGuard(commandPath)) { return; } const suppressDoctorStdout = isJsonOutputMode(commandPath, argv); diff --git a/src/cli/program/routes.test.ts b/src/cli/program/routes.test.ts index eb4b7351c59..0ce9dde4310 100644 --- a/src/cli/program/routes.test.ts +++ b/src/cli/program/routes.test.ts @@ -1,7 +1,26 @@ -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { findRoutedCommand } from "./routes.js"; +const runConfigGetMock = vi.hoisted(() => vi.fn(async () => {})); +const runConfigUnsetMock = vi.hoisted(() => vi.fn(async () => {})); +const modelsListCommandMock = vi.hoisted(() => vi.fn(async () => {})); +const modelsStatusCommandMock = vi.hoisted(() => vi.fn(async () => {})); + +vi.mock("../config-cli.js", () => ({ + runConfigGet: runConfigGetMock, + runConfigUnset: runConfigUnsetMock, +})); + +vi.mock("../../commands/models.js", () => ({ + modelsListCommand: modelsListCommandMock, + modelsStatusCommand: modelsStatusCommandMock, +})); + describe("program routes", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + function expectRoute(path: string[]) { const route = findRoutedCommand(path); expect(route).not.toBeNull(); @@ -58,6 +77,31 @@ describe("program routes", () => { await expectRunFalse(["config", "unset"], ["node", "openclaw", "config", "unset"]); }); + it("passes config get path correctly when root option values precede command", async () => { + const route = expectRoute(["config", "get"]); + await expect( + route?.run([ + "node", + "openclaw", + "--log-level", + "debug", + "config", + "get", + "update.channel", + "--json", + ]), + ).resolves.toBe(true); + expect(runConfigGetMock).toHaveBeenCalledWith({ path: "update.channel", json: true }); + }); + + it("passes config unset path correctly when root option values precede command", async () => { + const route = expectRoute(["config", "unset"]); + await expect( + route?.run(["node", "openclaw", "--profile", "work", "config", "unset", "update.channel"]), + ).resolves.toBe(true); + expect(runConfigUnsetMock).toHaveBeenCalledWith({ path: "update.channel" }); + }); + it("returns false for memory status route when --agent value is missing", async () => { await expectRunFalse(["memory", "status"], ["node", "openclaw", "memory", "status", "--agent"]); }); @@ -95,4 +139,39 @@ describe("program routes", () => { ["node", "openclaw", "models", "status", "--probe-profile"], ); }); + + it("accepts negative-number probe profile values", async () => { + const route = expectRoute(["models", "status"]); + await expect( + route?.run([ + "node", + "openclaw", + "models", + "status", + "--probe-provider", + "openai", + "--probe-timeout", + "5000", + "--probe-concurrency", + "2", + "--probe-max-tokens", + "64", + "--probe-profile", + "-1", + "--agent", + "default", + ]), + ).resolves.toBe(true); + expect(modelsStatusCommandMock).toHaveBeenCalledWith( + expect.objectContaining({ + probeProvider: "openai", + probeTimeout: "5000", + probeConcurrency: "2", + probeMaxTokens: "64", + probeProfile: "-1", + agent: "default", + }), + expect.any(Object), + ); + }); }); diff --git a/src/cli/program/routes.ts b/src/cli/program/routes.ts index ccecd8548f5..943d2aecad4 100644 --- a/src/cli/program/routes.ts +++ b/src/cli/program/routes.ts @@ -1,3 +1,4 @@ +import { consumeRootOptionToken, isValueToken } from "../../infra/cli-root-options.js"; import { defaultRuntime } from "../../runtime.js"; import { getFlagValue, getPositiveIntFlagValue, getVerboseFlag, hasFlag } from "../argv.js"; @@ -102,13 +103,23 @@ const routeMemoryStatus: RouteSpec = { function getCommandPositionals(argv: string[]): string[] { const out: string[] = []; const args = argv.slice(2); - for (const arg of args) { + let commandStarted = false; + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; if (!arg || arg === "--") { break; } + if (!commandStarted) { + const consumed = consumeRootOptionToken(args, i); + if (consumed > 0) { + i += consumed - 1; + continue; + } + } if (arg.startsWith("-")) { continue; } + commandStarted = true; out.push(arg); } return out; @@ -124,7 +135,7 @@ function getFlagValues(argv: string[], name: string): string[] | null { } if (arg === name) { const next = args[i + 1]; - if (!next || next === "--" || next.startsWith("-")) { + if (!isValueToken(next)) { return null; } values.push(next); diff --git a/src/cli/route.test.ts b/src/cli/route.test.ts index 6979c4d58ea..c2b2270fd0a 100644 --- a/src/cli/route.test.ts +++ b/src/cli/route.test.ts @@ -69,4 +69,16 @@ describe("tryRouteCli", () => { commandPath: ["status"], }); }); + + it("routes status when root options precede the command", async () => { + await expect(tryRouteCli(["node", "openclaw", "--log-level", "debug", "status"])).resolves.toBe( + true, + ); + + expect(findRoutedCommandMock).toHaveBeenCalledWith(["status"]); + expect(ensureConfigReadyMock).toHaveBeenCalledWith({ + runtime: expect.any(Object), + commandPath: ["status"], + }); + }); }); diff --git a/src/cli/route.ts b/src/cli/route.ts index 2d86eeb036c..b1d7b2851e1 100644 --- a/src/cli/route.ts +++ b/src/cli/route.ts @@ -1,7 +1,7 @@ import { isTruthyEnvValue } from "../infra/env.js"; import { defaultRuntime } from "../runtime.js"; import { VERSION } from "../version.js"; -import { getCommandPath, hasFlag, hasHelpOrVersion } from "./argv.js"; +import { getCommandPathWithRootOptions, hasFlag, hasHelpOrVersion } from "./argv.js"; import { emitCliBanner } from "./banner.js"; import { ensurePluginRegistryLoaded } from "./plugin-registry.js"; import { ensureConfigReady } from "./program/config-guard.js"; @@ -34,7 +34,7 @@ export async function tryRouteCli(argv: string[]): Promise { return false; } - const path = getCommandPath(argv, 2); + const path = getCommandPathWithRootOptions(argv, 2); if (!path[0]) { return false; } diff --git a/src/cli/run-main.test.ts b/src/cli/run-main.test.ts index 0884d05b65e..495a23684d1 100644 --- a/src/cli/run-main.test.ts +++ b/src/cli/run-main.test.ts @@ -114,6 +114,7 @@ describe("shouldEnsureCliPath", () => { it("skips path bootstrap for read-only fast paths", () => { expect(shouldEnsureCliPath(["node", "openclaw", "status"])).toBe(false); + expect(shouldEnsureCliPath(["node", "openclaw", "--log-level", "debug", "status"])).toBe(false); expect(shouldEnsureCliPath(["node", "openclaw", "sessions", "--json"])).toBe(false); expect(shouldEnsureCliPath(["node", "openclaw", "config", "get", "update"])).toBe(false); expect(shouldEnsureCliPath(["node", "openclaw", "models", "status", "--json"])).toBe(false); diff --git a/src/cli/run-main.ts b/src/cli/run-main.ts index 4f78c82bd4d..b304f213bfb 100644 --- a/src/cli/run-main.ts +++ b/src/cli/run-main.ts @@ -8,7 +8,7 @@ import { ensureOpenClawCliOnPath } from "../infra/path-env.js"; import { assertSupportedRuntime } from "../infra/runtime-guard.js"; import { installUnhandledRejectionHandler } from "../infra/unhandled-rejections.js"; import { enableConsoleCapture } from "../logging.js"; -import { getCommandPath, getPrimaryCommand, hasHelpOrVersion } from "./argv.js"; +import { getCommandPathWithRootOptions, getPrimaryCommand, hasHelpOrVersion } from "./argv.js"; import { applyCliProfileEnv, parseCliProfileArgs } from "./profile.js"; import { tryRouteCli } from "./route.js"; import { normalizeWindowsArgv } from "./windows-argv.js"; @@ -46,7 +46,7 @@ export function shouldEnsureCliPath(argv: string[]): boolean { if (hasHelpOrVersion(argv)) { return false; } - const [primary, secondary] = getCommandPath(argv, 2); + const [primary, secondary] = getCommandPathWithRootOptions(argv, 2); if (!primary) { return true; } diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 52f68732ca0..7422d43f984 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -10,6 +10,7 @@ import { resolveGatewayPort, writeConfigFile, } from "../../config/config.js"; +import { formatConfigIssueLines } from "../../config/issue-format.js"; import { resolveGatewayService } from "../../daemon/service.js"; import { channelToNpmTag, @@ -655,7 +656,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { return; } if (opts.channel && !configSnapshot.valid) { - const issues = configSnapshot.issues.map((issue) => `- ${issue.path}: ${issue.message}`); + const issues = formatConfigIssueLines(configSnapshot.issues, "-"); defaultRuntime.error(["Config is invalid; cannot set update channel.", ...issues].join("\n")); defaultRuntime.exit(1); return; diff --git a/src/commands/config-validation.ts b/src/commands/config-validation.ts index e8c7cef84c2..707c6e87eff 100644 --- a/src/commands/config-validation.ts +++ b/src/commands/config-validation.ts @@ -1,5 +1,6 @@ import { formatCliCommand } from "../cli/command-format.js"; import { type OpenClawConfig, readConfigFileSnapshot } from "../config/config.js"; +import { formatConfigIssueLines } from "../config/issue-format.js"; import type { RuntimeEnv } from "../runtime.js"; export async function requireValidConfigSnapshot( @@ -9,7 +10,7 @@ export async function requireValidConfigSnapshot( if (snapshot.exists && !snapshot.valid) { const issues = snapshot.issues.length > 0 - ? snapshot.issues.map((issue) => `- ${issue.path}: ${issue.message}`).join("\n") + ? formatConfigIssueLines(snapshot.issues, "-").join("\n") : "Unknown validation issue."; runtime.error(`Config invalid:\n${issues}`); runtime.error(`Fix the config or run ${formatCliCommand("openclaw doctor")}.`); diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 84d2f9e4307..b61b7c06908 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -11,6 +11,7 @@ import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig } from "../config/config.js"; import { CONFIG_PATH, migrateLegacyConfig, readConfigFileSnapshot } from "../config/config.js"; import { collectProviderDangerousNameMatchingScopes } from "../config/dangerous-name-matching.js"; +import { formatConfigIssueLines } from "../config/issue-format.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; import { parseToolsBySenderTypedKey } from "../config/types.tools.js"; import { OpenClawSchema } from "../config/zod-schema.js"; @@ -1753,13 +1754,13 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { } const warnings = snapshot.warnings ?? []; if (warnings.length > 0) { - const lines = warnings.map((issue) => `- ${issue.path}: ${issue.message}`).join("\n"); + const lines = formatConfigIssueLines(warnings, "-").join("\n"); note(lines, "Config warnings"); } if (snapshot.legacyIssues.length > 0) { note( - snapshot.legacyIssues.map((issue) => `- ${issue.path}: ${issue.message}`).join("\n"), + formatConfigIssueLines(snapshot.legacyIssues, "-").join("\n"), "Compatibility config keys detected", ); const { config: migrated, changes } = migrateLegacyConfig(snapshot.parsed); diff --git a/src/commands/models/shared.ts b/src/commands/models/shared.ts index 925558aad11..793e7e4b8e3 100644 --- a/src/commands/models/shared.ts +++ b/src/commands/models/shared.ts @@ -12,6 +12,7 @@ import { readConfigFileSnapshot, writeConfigFile, } from "../../config/config.js"; +import { formatConfigIssueLines } from "../../config/issue-format.js"; import { toAgentModelListLike } from "../../config/model-input.js"; import type { AgentModelConfig } from "../../config/types.agents-shared.js"; import { normalizeAgentId } from "../../routing/session-key.js"; @@ -64,7 +65,7 @@ export const isLocalBaseUrl = (baseUrl: string) => { export async function loadValidConfigOrThrow(): Promise { const snapshot = await readConfigFileSnapshot(); if (!snapshot.valid) { - const issues = snapshot.issues.map((issue) => `- ${issue.path}: ${issue.message}`).join("\n"); + const issues = formatConfigIssueLines(snapshot.issues, "-").join("\n"); throw new Error(`Invalid config at ${snapshot.path}\n${issues}`); } return snapshot.config; diff --git a/src/commands/status-all/diagnosis.ts b/src/commands/status-all/diagnosis.ts index 35da8ab97e9..59140e49b44 100644 --- a/src/commands/status-all/diagnosis.ts +++ b/src/commands/status-all/diagnosis.ts @@ -1,4 +1,5 @@ import type { ProgressReporter } from "../../cli/progress.js"; +import { formatConfigIssueLine } from "../../config/issue-format.js"; import { resolveGatewayLogPaths } from "../../daemon/launchd.js"; import { formatPortDiagnostics } from "../../infra/ports.js"; import { @@ -88,7 +89,7 @@ export async function appendStatusAllDiagnosis(params: { issues.findIndex((x) => x.path === issue.path && x.message === issue.message) === index, ); for (const issue of uniqueIssues.slice(0, 12)) { - lines.push(` - ${issue.path}: ${issue.message}`); + lines.push(` ${formatConfigIssueLine(issue, "-")}`); } if (uniqueIssues.length > 12) { lines.push(` ${muted(`… +${uniqueIssues.length - 12} more`)}`); diff --git a/src/config/allowed-values.test.ts b/src/config/allowed-values.test.ts new file mode 100644 index 00000000000..f62b95dae9b --- /dev/null +++ b/src/config/allowed-values.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { summarizeAllowedValues } from "./allowed-values.js"; + +describe("summarizeAllowedValues", () => { + it("does not collapse mixed-type entries that stringify similarly", () => { + const summary = summarizeAllowedValues([1, "1", 1, "1"]); + expect(summary).not.toBeNull(); + if (!summary) { + return; + } + expect(summary.hiddenCount).toBe(0); + expect(summary.formatted).toContain('1, "1"'); + expect(summary.values).toHaveLength(2); + }); + + it("keeps distinct long values even when labels truncate the same way", () => { + const prefix = "a".repeat(200); + const summary = summarizeAllowedValues([`${prefix}x`, `${prefix}y`]); + expect(summary).not.toBeNull(); + if (!summary) { + return; + } + expect(summary.hiddenCount).toBe(0); + expect(summary.values).toHaveLength(2); + expect(summary.values[0]).not.toBe(summary.values[1]); + }); +}); diff --git a/src/config/allowed-values.ts b/src/config/allowed-values.ts new file mode 100644 index 00000000000..f85b04df9a0 --- /dev/null +++ b/src/config/allowed-values.ts @@ -0,0 +1,98 @@ +const MAX_ALLOWED_VALUES_HINT = 12; +const MAX_ALLOWED_VALUE_CHARS = 160; + +export type AllowedValuesSummary = { + values: string[]; + hiddenCount: number; + formatted: string; +}; + +function truncateHintText(text: string, limit: number): string { + if (text.length <= limit) { + return text; + } + return `${text.slice(0, limit)}... (+${text.length - limit} chars)`; +} + +function safeStringify(value: unknown): string { + try { + const serialized = JSON.stringify(value); + if (serialized !== undefined) { + return serialized; + } + } catch { + // Fall back to string coercion when value is not JSON-serializable. + } + return String(value); +} + +function toAllowedValueLabel(value: unknown): string { + if (typeof value === "string") { + return JSON.stringify(truncateHintText(value, MAX_ALLOWED_VALUE_CHARS)); + } + return truncateHintText(safeStringify(value), MAX_ALLOWED_VALUE_CHARS); +} + +function toAllowedValueValue(value: unknown): string { + if (typeof value === "string") { + return value; + } + return safeStringify(value); +} + +function toAllowedValueDedupKey(value: unknown): string { + if (value === null) { + return "null:null"; + } + const kind = typeof value; + if (kind === "string") { + return `string:${value as string}`; + } + return `${kind}:${safeStringify(value)}`; +} + +export function summarizeAllowedValues( + values: ReadonlyArray, +): AllowedValuesSummary | null { + if (values.length === 0) { + return null; + } + + const deduped: Array<{ value: string; label: string }> = []; + const seenValues = new Set(); + for (const item of values) { + const dedupeKey = toAllowedValueDedupKey(item); + if (seenValues.has(dedupeKey)) { + continue; + } + seenValues.add(dedupeKey); + deduped.push({ + value: toAllowedValueValue(item), + label: toAllowedValueLabel(item), + }); + } + + const shown = deduped.slice(0, MAX_ALLOWED_VALUES_HINT); + const hiddenCount = deduped.length - shown.length; + const formattedCore = shown.map((entry) => entry.label).join(", "); + const formatted = + hiddenCount > 0 ? `${formattedCore}, ... (+${hiddenCount} more)` : formattedCore; + + return { + values: shown.map((entry) => entry.value), + hiddenCount, + formatted, + }; +} + +function messageAlreadyIncludesAllowedValues(message: string): boolean { + const lower = message.toLowerCase(); + return lower.includes("(allowed:") || lower.includes("expected one of"); +} + +export function appendAllowedValuesHint(message: string, summary: AllowedValuesSummary): string { + if (messageAlreadyIncludesAllowedValues(message)) { + return message; + } + return `${message} (allowed: ${summary.formatted})`; +} diff --git a/src/config/config.plugin-validation.test.ts b/src/config/config.plugin-validation.test.ts index 488d4ab7eac..6c0b9e56587 100644 --- a/src/config/config.plugin-validation.test.ts +++ b/src/config/config.plugin-validation.test.ts @@ -35,6 +35,7 @@ describe("config plugin validation", () => { let fixtureRoot = ""; let suiteHome = ""; let badPluginDir = ""; + let enumPluginDir = ""; let bluebubblesPluginDir = ""; const envSnapshot = { OPENCLAW_STATE_DIR: process.env.OPENCLAW_STATE_DIR, @@ -48,6 +49,7 @@ describe("config plugin validation", () => { suiteHome = path.join(fixtureRoot, "home"); await fs.mkdir(suiteHome, { recursive: true }); badPluginDir = path.join(suiteHome, "bad-plugin"); + enumPluginDir = path.join(suiteHome, "enum-plugin"); bluebubblesPluginDir = path.join(suiteHome, "bluebubbles-plugin"); await writePluginFixture({ dir: badPluginDir, @@ -61,6 +63,20 @@ describe("config plugin validation", () => { required: ["value"], }, }); + await writePluginFixture({ + dir: enumPluginDir, + id: "enum-plugin", + schema: { + type: "object", + properties: { + fileFormat: { + type: "string", + enum: ["markdown", "html"], + }, + }, + required: ["fileFormat"], + }, + }); await writePluginFixture({ dir: bluebubblesPluginDir, id: "bluebubbles-plugin", @@ -185,13 +201,34 @@ describe("config plugin validation", () => { if (!res.ok) { const hasIssue = res.issues.some( (issue) => - issue.path === "plugins.entries.bad-plugin.config" && + issue.path.startsWith("plugins.entries.bad-plugin.config") && issue.message.includes("invalid config"), ); expect(hasIssue).toBe(true); } }); + it("surfaces allowed enum values for plugin config diagnostics", async () => { + const res = validateInSuite({ + agents: { list: [{ id: "pi" }] }, + plugins: { + enabled: true, + load: { paths: [enumPluginDir] }, + entries: { "enum-plugin": { config: { fileFormat: "txt" } } }, + }, + }); + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.issues.find( + (entry) => entry.path === "plugins.entries.enum-plugin.config.fileFormat", + ); + expect(issue).toBeDefined(); + expect(issue?.message).toContain('allowed: "markdown", "html"'); + expect(issue?.allowedValues).toEqual(["markdown", "html"]); + expect(issue?.allowedValuesHiddenCount).toBe(0); + } + }); + it("accepts known plugin ids and valid channel/heartbeat enums", async () => { const res = validateInSuite({ agents: { diff --git a/src/config/issue-format.test.ts b/src/config/issue-format.test.ts new file mode 100644 index 00000000000..fed82f99588 --- /dev/null +++ b/src/config/issue-format.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from "vitest"; +import { + formatConfigIssueLine, + formatConfigIssueLines, + normalizeConfigIssue, + normalizeConfigIssuePath, + normalizeConfigIssues, +} from "./issue-format.js"; + +describe("config issue format", () => { + it("normalizes empty paths to ", () => { + expect(normalizeConfigIssuePath("")).toBe(""); + expect(normalizeConfigIssuePath(" ")).toBe(""); + expect(normalizeConfigIssuePath(null)).toBe(""); + expect(normalizeConfigIssuePath(undefined)).toBe(""); + }); + + it("formats issue lines with and without markers", () => { + expect(formatConfigIssueLine({ path: "", message: "broken" }, "-")).toBe("- : broken"); + expect( + formatConfigIssueLine({ path: "", message: "broken" }, "-", { normalizeRoot: true }), + ).toBe("- : broken"); + expect(formatConfigIssueLine({ path: "gateway.bind", message: "invalid" }, "")).toBe( + "gateway.bind: invalid", + ); + expect( + formatConfigIssueLines( + [ + { path: "", message: "first" }, + { path: "channels.signal.dmPolicy", message: "second" }, + ], + "×", + { normalizeRoot: true }, + ), + ).toEqual(["× : first", "× channels.signal.dmPolicy: second"]); + }); + + it("sanitizes control characters and ANSI sequences in formatted lines", () => { + expect( + formatConfigIssueLine( + { + path: "gateway.\nbind\x1b[31m", + message: "bad\r\n\tvalue\x1b[0m\u0007", + }, + "-", + ), + ).toBe("- gateway.\\nbind: bad\\r\\n\\tvalue"); + }); + + it("normalizes issue metadata for machine output", () => { + expect( + normalizeConfigIssue({ + path: "", + message: "invalid", + allowedValues: ["stable", "beta"], + allowedValuesHiddenCount: 0, + }), + ).toEqual({ + path: "", + message: "invalid", + allowedValues: ["stable", "beta"], + }); + + expect( + normalizeConfigIssues([ + { + path: "update.channel", + message: "invalid", + allowedValues: [], + allowedValuesHiddenCount: 2, + }, + ]), + ).toEqual([ + { + path: "update.channel", + message: "invalid", + }, + ]); + + expect( + normalizeConfigIssue({ + path: "update.channel", + message: "invalid", + allowedValues: ["stable"], + allowedValuesHiddenCount: 2, + }), + ).toEqual({ + path: "update.channel", + message: "invalid", + allowedValues: ["stable"], + allowedValuesHiddenCount: 2, + }); + }); +}); diff --git a/src/config/issue-format.ts b/src/config/issue-format.ts new file mode 100644 index 00000000000..599e93986a2 --- /dev/null +++ b/src/config/issue-format.ts @@ -0,0 +1,68 @@ +import { sanitizeTerminalText } from "../terminal/safe-text.js"; +import type { ConfigValidationIssue } from "./types.js"; + +type ConfigIssueLineInput = { + path?: string | null; + message: string; +}; + +type ConfigIssueFormatOptions = { + normalizeRoot?: boolean; +}; + +export function normalizeConfigIssuePath(path: string | null | undefined): string { + if (typeof path !== "string") { + return ""; + } + const trimmed = path.trim(); + return trimmed ? trimmed : ""; +} + +export function normalizeConfigIssue(issue: ConfigValidationIssue): ConfigValidationIssue { + const hasAllowedValues = Array.isArray(issue.allowedValues) && issue.allowedValues.length > 0; + return { + path: normalizeConfigIssuePath(issue.path), + message: issue.message, + ...(hasAllowedValues ? { allowedValues: issue.allowedValues } : {}), + ...(hasAllowedValues && + typeof issue.allowedValuesHiddenCount === "number" && + issue.allowedValuesHiddenCount > 0 + ? { allowedValuesHiddenCount: issue.allowedValuesHiddenCount } + : {}), + }; +} + +export function normalizeConfigIssues( + issues: ReadonlyArray, +): ConfigValidationIssue[] { + return issues.map((issue) => normalizeConfigIssue(issue)); +} + +function resolveIssuePathForLine( + path: string | null | undefined, + opts?: ConfigIssueFormatOptions, +): string { + if (opts?.normalizeRoot) { + return normalizeConfigIssuePath(path); + } + return typeof path === "string" ? path : ""; +} + +export function formatConfigIssueLine( + issue: ConfigIssueLineInput, + marker = "-", + opts?: ConfigIssueFormatOptions, +): string { + const prefix = marker ? `${marker} ` : ""; + const path = sanitizeTerminalText(resolveIssuePathForLine(issue.path, opts)); + const message = sanitizeTerminalText(issue.message); + return `${prefix}${path}: ${message}`; +} + +export function formatConfigIssueLines( + issues: ReadonlyArray, + marker = "-", + opts?: ConfigIssueFormatOptions, +): string[] { + return issues.map((issue) => formatConfigIssueLine(issue, marker, opts)); +} diff --git a/src/config/types.openclaw.ts b/src/config/types.openclaw.ts index e83b105dff3..0a818419557 100644 --- a/src/config/types.openclaw.ts +++ b/src/config/types.openclaw.ts @@ -119,6 +119,8 @@ export type OpenClawConfig = { export type ConfigValidationIssue = { path: string; message: string; + allowedValues?: string[]; + allowedValuesHiddenCount?: number; }; export type LegacyConfigIssue = { diff --git a/src/config/validation.allowed-values.test.ts b/src/config/validation.allowed-values.test.ts new file mode 100644 index 00000000000..d586246ff87 --- /dev/null +++ b/src/config/validation.allowed-values.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import { validateConfigObjectRaw } from "./validation.js"; + +describe("config validation allowed-values metadata", () => { + it("adds allowed values for invalid union paths", () => { + const result = validateConfigObjectRaw({ + update: { channel: "nightly" }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + const issue = result.issues.find((entry) => entry.path === "update.channel"); + expect(issue).toBeDefined(); + expect(issue?.message).toContain('(allowed: "stable", "beta", "dev")'); + expect(issue?.allowedValues).toEqual(["stable", "beta", "dev"]); + expect(issue?.allowedValuesHiddenCount).toBe(0); + } + }); + + it("keeps native enum messages while attaching allowed values metadata", () => { + const result = validateConfigObjectRaw({ + channels: { signal: { dmPolicy: "maybe" } }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + const issue = result.issues.find((entry) => entry.path === "channels.signal.dmPolicy"); + expect(issue).toBeDefined(); + expect(issue?.message).toContain("expected one of"); + expect(issue?.message).not.toContain("(allowed:"); + expect(issue?.allowedValues).toEqual(["pairing", "allowlist", "open", "disabled"]); + expect(issue?.allowedValuesHiddenCount).toBe(0); + } + }); + + it("includes boolean variants for boolean-or-enum unions", () => { + const result = validateConfigObjectRaw({ + channels: { + telegram: { + botToken: "x", + allowFrom: ["*"], + dmPolicy: "allowlist", + streaming: "maybe", + }, + }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + const issue = result.issues.find((entry) => entry.path === "channels.telegram.streaming"); + expect(issue).toBeDefined(); + expect(issue?.allowedValues).toEqual([ + "true", + "false", + "off", + "partial", + "block", + "progress", + ]); + } + }); + + it("skips allowed-values hints for unions with open-ended branches", () => { + const result = validateConfigObjectRaw({ + cron: { sessionRetention: true }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + const issue = result.issues.find((entry) => entry.path === "cron.sessionRetention"); + expect(issue).toBeDefined(); + expect(issue?.allowedValues).toBeUndefined(); + expect(issue?.allowedValuesHiddenCount).toBeUndefined(); + expect(issue?.message).not.toContain("(allowed:"); + } + }); +}); diff --git a/src/config/validation.ts b/src/config/validation.ts index 96dd1c7e6fb..f6687e172bb 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -18,6 +18,7 @@ import { import { isCanonicalDottedDecimalIPv4, isLoopbackIpAddress } from "../shared/net/ip.js"; import { isRecord } from "../utils.js"; import { findDuplicateAgentDirs, formatDuplicateAgentDirError } from "./agent-dirs.js"; +import { appendAllowedValuesHint, summarizeAllowedValues } from "./allowed-values.js"; import { applyAgentDefaults, applyModelDefaults, applySessionDefaults } from "./defaults.js"; import { findLegacyConfigIssues } from "./legacy.js"; import type { OpenClawConfig, ConfigValidationIssue } from "./types.js"; @@ -25,6 +26,119 @@ import { OpenClawSchema } from "./zod-schema.js"; const LEGACY_REMOVED_PLUGIN_IDS = new Set(["google-antigravity-auth"]); +type UnknownIssueRecord = Record; +type AllowedValuesCollection = { + values: unknown[]; + incomplete: boolean; + hasValues: boolean; +}; + +function toIssueRecord(value: unknown): UnknownIssueRecord | null { + if (!value || typeof value !== "object") { + return null; + } + return value as UnknownIssueRecord; +} + +function collectAllowedValuesFromIssue(issue: unknown): AllowedValuesCollection { + const record = toIssueRecord(issue); + if (!record) { + return { values: [], incomplete: false, hasValues: false }; + } + const code = typeof record.code === "string" ? record.code : ""; + + if (code === "invalid_value") { + const values = record.values; + if (!Array.isArray(values)) { + return { values: [], incomplete: true, hasValues: false }; + } + return { values, incomplete: false, hasValues: values.length > 0 }; + } + + if (code === "invalid_type") { + const expected = typeof record.expected === "string" ? record.expected : ""; + if (expected === "boolean") { + return { values: [true, false], incomplete: false, hasValues: true }; + } + return { values: [], incomplete: true, hasValues: false }; + } + + if (code !== "invalid_union") { + return { values: [], incomplete: false, hasValues: false }; + } + + const nested = record.errors; + if (!Array.isArray(nested) || nested.length === 0) { + return { values: [], incomplete: true, hasValues: false }; + } + + const collected: unknown[] = []; + for (const branch of nested) { + if (!Array.isArray(branch) || branch.length === 0) { + return { values: [], incomplete: true, hasValues: false }; + } + const branchCollected = collectAllowedValuesFromIssueList(branch); + if (branchCollected.incomplete || !branchCollected.hasValues) { + return { values: [], incomplete: true, hasValues: false }; + } + collected.push(...branchCollected.values); + } + + return { values: collected, incomplete: false, hasValues: collected.length > 0 }; +} + +function collectAllowedValuesFromIssueList( + issues: ReadonlyArray, +): AllowedValuesCollection { + const collected: unknown[] = []; + let hasValues = false; + for (const issue of issues) { + const branch = collectAllowedValuesFromIssue(issue); + if (branch.incomplete) { + return { values: [], incomplete: true, hasValues: false }; + } + if (!branch.hasValues) { + continue; + } + hasValues = true; + collected.push(...branch.values); + } + return { values: collected, incomplete: false, hasValues }; +} + +function collectAllowedValuesFromUnknownIssue(issue: unknown): unknown[] { + const collection = collectAllowedValuesFromIssue(issue); + if (collection.incomplete || !collection.hasValues) { + return []; + } + return collection.values; +} + +function mapZodIssueToConfigIssue(issue: unknown): ConfigValidationIssue { + const record = toIssueRecord(issue); + const path = Array.isArray(record?.path) + ? record.path + .filter((segment): segment is string | number => { + const segmentType = typeof segment; + return segmentType === "string" || segmentType === "number"; + }) + .join(".") + : ""; + const message = typeof record?.message === "string" ? record.message : "Invalid input"; + const allowedValuesSummary = summarizeAllowedValues(collectAllowedValuesFromUnknownIssue(issue)); + + if (!allowedValuesSummary) { + return { path, message }; + } + + return { + path, + message: appendAllowedValuesHint(message, allowedValuesSummary), + allowedValues: allowedValuesSummary.values, + allowedValuesHiddenCount: allowedValuesSummary.hiddenCount, + }; +} + function isWorkspaceAvatarPath(value: string, workspaceDir: string): boolean { const workspaceRoot = path.resolve(workspaceDir); const resolved = path.resolve(workspaceRoot, value); @@ -129,10 +243,7 @@ export function validateConfigObjectRaw( if (!validated.success) { return { ok: false, - issues: validated.error.issues.map((iss) => ({ - path: iss.path.join("."), - message: iss.message, - })), + issues: validated.error.issues.map((issue) => mapZodIssueToConfigIssue(issue)), }; } const duplicates = findDuplicateAgentDirs(validated.data as OpenClawConfig); @@ -227,6 +338,14 @@ function validateConfigObjectWithPluginsBase( const hasExplicitPluginsConfig = isRecord(raw) && Object.prototype.hasOwnProperty.call(raw, "plugins"); + const resolvePluginConfigIssuePath = (pluginId: string, errorPath: string): string => { + const base = `plugins.entries.${pluginId}.config`; + if (!errorPath || errorPath === "") { + return base; + } + return `${base}.${errorPath}`; + }; + type RegistryInfo = { registry: ReturnType; knownIds?: Set; @@ -472,8 +591,10 @@ function validateConfigObjectWithPluginsBase( if (!res.ok) { for (const error of res.errors) { issues.push({ - path: `plugins.entries.${pluginId}.config`, - message: `invalid config: ${error}`, + path: resolvePluginConfigIssuePath(pluginId, error.path), + message: `invalid config: ${error.message}`, + allowedValues: error.allowedValues, + allowedValuesHiddenCount: error.allowedValuesHiddenCount, }); } } diff --git a/src/gateway/config-reload.ts b/src/gateway/config-reload.ts index dfd9ebdaf93..38fe786a667 100644 --- a/src/gateway/config-reload.ts +++ b/src/gateway/config-reload.ts @@ -1,6 +1,7 @@ import { isDeepStrictEqual } from "node:util"; import chokidar from "chokidar"; import type { OpenClawConfig, ConfigFileSnapshot, GatewayReloadMode } from "../config/config.js"; +import { formatConfigIssueLines } from "../config/issue-format.js"; import { isPlainObject } from "../utils.js"; import { buildGatewayReloadPlan, type GatewayReloadPlan } from "./config-reload-plan.js"; @@ -141,7 +142,7 @@ export function startGatewayConfigReloader(opts: { if (snapshot.valid) { return false; } - const issues = snapshot.issues.map((issue) => `${issue.path}: ${issue.message}`).join(", "); + const issues = formatConfigIssueLines(snapshot.issues, "").join(", "); opts.log.warn(`config reload skipped (invalid config): ${issues}`); return true; }; diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index ceb41029951..65c84593202 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -18,6 +18,7 @@ import { readConfigFileSnapshot, writeConfigFile, } from "../config/config.js"; +import { formatConfigIssueLines } from "../config/issue-format.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; import { resolveMainSessionKey } from "../config/sessions.js"; import { clearAgentRunContext, onAgentEvent } from "../infra/agent-events.js"; @@ -237,9 +238,7 @@ export async function startGatewayServer( if (configSnapshot.exists && !configSnapshot.valid) { const issues = configSnapshot.issues.length > 0 - ? configSnapshot.issues - .map((issue) => `${issue.path || ""}: ${issue.message}`) - .join("\n") + ? formatConfigIssueLines(configSnapshot.issues, "", { normalizeRoot: true }).join("\n") : "Unknown validation issue."; throw new Error( `Invalid config at ${configSnapshot.path}.\n${issues}\nRun "${formatCliCommand("openclaw doctor")}" to repair, then retry.`, @@ -332,9 +331,7 @@ export async function startGatewayServer( if (!freshSnapshot.valid) { const issues = freshSnapshot.issues.length > 0 - ? freshSnapshot.issues - .map((issue) => `${issue.path || ""}: ${issue.message}`) - .join("\n") + ? formatConfigIssueLines(freshSnapshot.issues, "", { normalizeRoot: true }).join("\n") : "Unknown validation issue."; throw new Error(`Invalid config at ${freshSnapshot.path}.\n${issues}`); } diff --git a/src/infra/cli-root-options.test.ts b/src/infra/cli-root-options.test.ts new file mode 100644 index 00000000000..514548586f7 --- /dev/null +++ b/src/infra/cli-root-options.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { consumeRootOptionToken } from "./cli-root-options.js"; + +describe("consumeRootOptionToken", () => { + it("consumes boolean and inline root options", () => { + expect(consumeRootOptionToken(["--dev"], 0)).toBe(1); + expect(consumeRootOptionToken(["--profile=work"], 0)).toBe(1); + expect(consumeRootOptionToken(["--log-level=debug"], 0)).toBe(1); + }); + + it("consumes split root value option only when next token is a value", () => { + expect(consumeRootOptionToken(["--profile", "work"], 0)).toBe(2); + expect(consumeRootOptionToken(["--profile", "--no-color"], 0)).toBe(1); + expect(consumeRootOptionToken(["--profile", "--"], 0)).toBe(1); + }); +}); diff --git a/src/infra/cli-root-options.ts b/src/infra/cli-root-options.ts new file mode 100644 index 00000000000..9522e114966 --- /dev/null +++ b/src/infra/cli-root-options.ts @@ -0,0 +1,31 @@ +export const FLAG_TERMINATOR = "--"; + +const ROOT_BOOLEAN_FLAGS = new Set(["--dev", "--no-color"]); +const ROOT_VALUE_FLAGS = new Set(["--profile", "--log-level"]); + +export function isValueToken(arg: string | undefined): boolean { + if (!arg || arg === FLAG_TERMINATOR) { + return false; + } + if (!arg.startsWith("-")) { + return true; + } + return /^-\d+(?:\.\d+)?$/.test(arg); +} + +export function consumeRootOptionToken(args: ReadonlyArray, index: number): number { + const arg = args[index]; + if (!arg) { + return 0; + } + if (ROOT_BOOLEAN_FLAGS.has(arg)) { + return 1; + } + if (arg.startsWith("--profile=") || arg.startsWith("--log-level=")) { + return 1; + } + if (ROOT_VALUE_FLAGS.has(arg)) { + return isValueToken(args[index + 1]) ? 2 : 1; + } + return 0; +} diff --git a/src/logging/logger.settings.test.ts b/src/logging/logger.settings.test.ts new file mode 100644 index 00000000000..39cc3f3d73c --- /dev/null +++ b/src/logging/logger.settings.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { __test__ } from "./logger.js"; + +describe("shouldSkipLoadConfigFallback", () => { + it("matches config validate invocations", () => { + expect(__test__.shouldSkipLoadConfigFallback(["node", "openclaw", "config", "validate"])).toBe( + true, + ); + }); + + it("handles root flags before config validate", () => { + expect( + __test__.shouldSkipLoadConfigFallback([ + "node", + "openclaw", + "--profile", + "work", + "--no-color", + "config", + "validate", + "--json", + ]), + ).toBe(true); + }); + + it("does not match other commands", () => { + expect( + __test__.shouldSkipLoadConfigFallback(["node", "openclaw", "config", "get", "foo"]), + ).toBe(false); + expect(__test__.shouldSkipLoadConfigFallback(["node", "openclaw", "status"])).toBe(false); + }); +}); diff --git a/src/logging/logger.ts b/src/logging/logger.ts index dcb89de9a43..47e5624dc20 100644 --- a/src/logging/logger.ts +++ b/src/logging/logger.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import { Logger as TsLogger } from "tslog"; +import { getCommandPathWithRootOptions } from "../cli/argv.js"; import type { OpenClawConfig } from "../config/types.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { readLoggingConfig } from "./config.js"; @@ -42,6 +43,11 @@ export type LogTransport = (logObj: LogTransportRecord) => void; const externalTransports = new Set(); +function shouldSkipLoadConfigFallback(argv: string[] = process.argv): boolean { + const [primary, secondary] = getCommandPathWithRootOptions(argv, 2); + return primary === "config" && secondary === "validate"; +} + function attachExternalTransport(logger: TsLogger, transport: LogTransport): void { logger.attachTransport((logObj: LogObj) => { if (!externalTransports.has(transport)) { @@ -78,7 +84,7 @@ function resolveSettings(): ResolvedSettings { let cfg: OpenClawConfig["logging"] | undefined = (loggingState.overrideSettings as LoggerSettings | null) ?? readLoggingConfig(); - if (!cfg) { + if (!cfg && !shouldSkipLoadConfigFallback()) { try { const loaded = requireConfig?.("../config/config.js") as | { @@ -289,6 +295,10 @@ export function registerLogTransport(transport: LogTransport): () => void { }; } +export const __test__ = { + shouldSkipLoadConfigFallback, +}; + function formatLocalDate(date: Date): string { const year = date.getFullYear(); const month = String(date.getMonth() + 1).padStart(2, "0"); diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index a2d2f50b6ac..c0ac9751a3d 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -121,7 +121,7 @@ function validatePluginConfig(params: { if (result.ok) { return { ok: true, value: params.value as Record | undefined }; } - return { ok: false, errors: result.errors }; + return { ok: false, errors: result.errors.map((error) => error.text) }; } function resolvePluginModuleExport(moduleExport: unknown): { diff --git a/src/plugins/schema-validator.test.ts b/src/plugins/schema-validator.test.ts new file mode 100644 index 00000000000..7f2b849d774 --- /dev/null +++ b/src/plugins/schema-validator.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it } from "vitest"; +import { validateJsonSchemaValue } from "./schema-validator.js"; + +describe("schema validator", () => { + it("includes allowed values in enum validation errors", () => { + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.enum", + schema: { + type: "object", + properties: { + fileFormat: { + type: "string", + enum: ["markdown", "html", "json"], + }, + }, + required: ["fileFormat"], + }, + value: { fileFormat: "txt" }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors.find((entry) => entry.path === "fileFormat"); + expect(issue?.message).toContain("(allowed:"); + expect(issue?.allowedValues).toEqual(["markdown", "html", "json"]); + expect(issue?.allowedValuesHiddenCount).toBe(0); + } + }); + + it("includes allowed value in const validation errors", () => { + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.const", + schema: { + type: "object", + properties: { + mode: { + const: "strict", + }, + }, + required: ["mode"], + }, + value: { mode: "relaxed" }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors.find((entry) => entry.path === "mode"); + expect(issue?.message).toContain("(allowed:"); + expect(issue?.allowedValues).toEqual(["strict"]); + expect(issue?.allowedValuesHiddenCount).toBe(0); + } + }); + + it("truncates long allowed-value hints", () => { + const values = [ + "v1", + "v2", + "v3", + "v4", + "v5", + "v6", + "v7", + "v8", + "v9", + "v10", + "v11", + "v12", + "v13", + ]; + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.enum.truncate", + schema: { + type: "object", + properties: { + mode: { + type: "string", + enum: values, + }, + }, + required: ["mode"], + }, + value: { mode: "not-listed" }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors.find((entry) => entry.path === "mode"); + expect(issue?.message).toContain("(allowed:"); + expect(issue?.message).toContain("... (+1 more)"); + expect(issue?.allowedValues).toEqual([ + "v1", + "v2", + "v3", + "v4", + "v5", + "v6", + "v7", + "v8", + "v9", + "v10", + "v11", + "v12", + ]); + expect(issue?.allowedValuesHiddenCount).toBe(1); + } + }); + + it("appends missing required property to the structured path", () => { + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.required.path", + schema: { + type: "object", + properties: { + settings: { + type: "object", + properties: { + mode: { type: "string" }, + }, + required: ["mode"], + }, + }, + required: ["settings"], + }, + value: { settings: {} }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors.find((entry) => entry.path === "settings.mode"); + expect(issue).toBeDefined(); + expect(issue?.allowedValues).toBeUndefined(); + } + }); + + it("appends missing dependency property to the structured path", () => { + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.dependencies.path", + schema: { + type: "object", + properties: { + settings: { + type: "object", + dependencies: { + mode: ["format"], + }, + }, + }, + }, + value: { settings: { mode: "strict" } }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors.find((entry) => entry.path === "settings.format"); + expect(issue).toBeDefined(); + expect(issue?.allowedValues).toBeUndefined(); + } + }); + + it("truncates oversized allowed value entries", () => { + const oversizedAllowed = "a".repeat(300); + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.enum.long-value", + schema: { + type: "object", + properties: { + mode: { + type: "string", + enum: [oversizedAllowed], + }, + }, + required: ["mode"], + }, + value: { mode: "not-listed" }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors.find((entry) => entry.path === "mode"); + expect(issue).toBeDefined(); + expect(issue?.message).toContain("(allowed:"); + expect(issue?.message).toContain("... (+"); + } + }); + + it("sanitizes terminal text while preserving structured fields", () => { + const maliciousProperty = "evil\nkey\t\x1b[31mred\x1b[0m"; + const res = validateJsonSchemaValue({ + cacheKey: "schema-validator.test.terminal-sanitize", + schema: { + type: "object", + properties: {}, + required: [maliciousProperty], + }, + value: {}, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + const issue = res.errors[0]; + expect(issue).toBeDefined(); + expect(issue?.path).toContain("\n"); + expect(issue?.message).toContain("\n"); + expect(issue?.text).toContain("\\n"); + expect(issue?.text).toContain("\\t"); + expect(issue?.text).not.toContain("\n"); + expect(issue?.text).not.toContain("\t"); + expect(issue?.text).not.toContain("\x1b"); + } + }); +}); diff --git a/src/plugins/schema-validator.ts b/src/plugins/schema-validator.ts index 19af4d63cf8..af64be10147 100644 --- a/src/plugins/schema-validator.ts +++ b/src/plugins/schema-validator.ts @@ -1,5 +1,7 @@ import { createRequire } from "node:module"; import type { ErrorObject, ValidateFunction } from "ajv"; +import { appendAllowedValuesHint, summarizeAllowedValues } from "../config/allowed-values.js"; +import { sanitizeTerminalText } from "../terminal/safe-text.js"; const require = createRequire(import.meta.url); type AjvLike = { @@ -31,14 +33,100 @@ type CachedValidator = { const schemaCache = new Map(); -function formatAjvErrors(errors: ErrorObject[] | null | undefined): string[] { +export type JsonSchemaValidationError = { + path: string; + message: string; + text: string; + allowedValues?: string[]; + allowedValuesHiddenCount?: number; +}; + +function normalizeAjvPath(instancePath: string | undefined): string { + const path = instancePath?.replace(/^\//, "").replace(/\//g, "."); + return path && path.length > 0 ? path : ""; +} + +function appendPathSegment(path: string, segment: string): string { + const trimmed = segment.trim(); + if (!trimmed) { + return path; + } + if (path === "") { + return trimmed; + } + return `${path}.${trimmed}`; +} + +function resolveMissingProperty(error: ErrorObject): string | null { + if ( + error.keyword !== "required" && + error.keyword !== "dependentRequired" && + error.keyword !== "dependencies" + ) { + return null; + } + const missingProperty = (error.params as { missingProperty?: unknown }).missingProperty; + return typeof missingProperty === "string" && missingProperty.trim() ? missingProperty : null; +} + +function resolveAjvErrorPath(error: ErrorObject): string { + const basePath = normalizeAjvPath(error.instancePath); + const missingProperty = resolveMissingProperty(error); + if (!missingProperty) { + return basePath; + } + return appendPathSegment(basePath, missingProperty); +} + +function extractAllowedValues(error: ErrorObject): unknown[] | null { + if (error.keyword === "enum") { + const allowedValues = (error.params as { allowedValues?: unknown }).allowedValues; + return Array.isArray(allowedValues) ? allowedValues : null; + } + + if (error.keyword === "const") { + const params = error.params as { allowedValue?: unknown }; + if (!Object.prototype.hasOwnProperty.call(params, "allowedValue")) { + return null; + } + return [params.allowedValue]; + } + + return null; +} + +function getAjvAllowedValuesSummary(error: ErrorObject): ReturnType { + const allowedValues = extractAllowedValues(error); + if (!allowedValues) { + return null; + } + return summarizeAllowedValues(allowedValues); +} + +function formatAjvErrors(errors: ErrorObject[] | null | undefined): JsonSchemaValidationError[] { if (!errors || errors.length === 0) { - return ["invalid config"]; + return [{ path: "", message: "invalid config", text: ": invalid config" }]; } return errors.map((error) => { - const path = error.instancePath?.replace(/^\//, "").replace(/\//g, ".") || ""; - const message = error.message ?? "invalid"; - return `${path}: ${message}`; + const path = resolveAjvErrorPath(error); + const baseMessage = error.message ?? "invalid"; + const allowedValuesSummary = getAjvAllowedValuesSummary(error); + const message = allowedValuesSummary + ? appendAllowedValuesHint(baseMessage, allowedValuesSummary) + : baseMessage; + const safePath = sanitizeTerminalText(path); + const safeMessage = sanitizeTerminalText(message); + return { + path, + message, + text: `${safePath}: ${safeMessage}`, + ...(allowedValuesSummary + ? { + allowedValues: allowedValuesSummary.values, + allowedValuesHiddenCount: allowedValuesSummary.hiddenCount, + } + : {}), + }; }); } @@ -46,7 +134,7 @@ export function validateJsonSchemaValue(params: { schema: Record; cacheKey: string; value: unknown; -}): { ok: true } | { ok: false; errors: string[] } { +}): { ok: true } | { ok: false; errors: JsonSchemaValidationError[] } { let cached = schemaCache.get(params.cacheKey); if (!cached || cached.schema !== params.schema) { const validate = getAjv().compile(params.schema); diff --git a/src/terminal/safe-text.test.ts b/src/terminal/safe-text.test.ts new file mode 100644 index 00000000000..cbed2a7b06f --- /dev/null +++ b/src/terminal/safe-text.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from "vitest"; +import { sanitizeTerminalText } from "./safe-text.js"; + +describe("sanitizeTerminalText", () => { + it("removes C1 control characters", () => { + expect(sanitizeTerminalText("a\u009bb\u0085c")).toBe("abc"); + }); + + it("escapes line controls while preserving printable text", () => { + expect(sanitizeTerminalText("a\tb\nc\rd")).toBe("a\\tb\\nc\\rd"); + }); +}); diff --git a/src/terminal/safe-text.ts b/src/terminal/safe-text.ts new file mode 100644 index 00000000000..f6754da5aef --- /dev/null +++ b/src/terminal/safe-text.ts @@ -0,0 +1,20 @@ +import { stripAnsi } from "./ansi.js"; + +/** + * Normalize untrusted text for single-line terminal/log rendering. + */ +export function sanitizeTerminalText(input: string): string { + const normalized = stripAnsi(input) + .replace(/\r/g, "\\r") + .replace(/\n/g, "\\n") + .replace(/\t/g, "\\t"); + let sanitized = ""; + for (const char of normalized) { + const code = char.charCodeAt(0); + const isControl = (code >= 0x00 && code <= 0x1f) || (code >= 0x7f && code <= 0x9f); + if (!isControl) { + sanitized += char; + } + } + return sanitized; +}