diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index 557f0e4510b0..d664c9997925 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -310,14 +310,18 @@ function makeInvalidSnapshot(params: { issues: ConfigFileSnapshot["issues"]; warnings?: ConfigFileSnapshot["warnings"]; path?: string; + raw?: string; + parsed?: unknown; + sourceConfig?: OpenClawConfig; }): ConfigFileSnapshot { + const parsed = params.parsed ?? {}; return { path: params.path ?? "/tmp/custom-openclaw.json", exists: true, - raw: "{}", - parsed: {}, - sourceConfig: {}, - resolved: {}, + raw: params.raw ?? "{}", + parsed, + sourceConfig: params.sourceConfig ?? (parsed as OpenClawConfig), + resolved: parsed as OpenClawConfig, valid: false, runtimeConfig: {}, config: {}, @@ -1165,6 +1169,47 @@ describe("config cli", () => { expect(mockLog).not.toHaveBeenCalled(); }); + it("prints line numbers, bracket array paths, and safe received values", async () => { + const parsed = { + agents: { + list: [{ id: "a" }, { id: "b" }, { id: "c" }, { id: "d", tools: { profile: "none" } }], + }, + }; + const raw = [ + "{", + ' "agents": {', + ' "list": [', + ' { "id": "a" },', + ' { "id": "b" },', + ' { "id": "c" },', + ' { "id": "d", "tools": { "profile": "none" } }', + " ]", + " }", + "}", + ].join("\n"); + setSnapshotOnce( + makeInvalidSnapshot({ + raw, + parsed, + path: "/tmp/openclaw.json", + issues: [ + { + path: "agents.list.3.tools.profile", + pathSegments: ["agents", "list", 3, "tools", "profile"], + message: 'Invalid input (allowed: "minimal", "coding", "messaging", "full")', + allowedValues: ["minimal", "coding", "messaging", "full"], + }, + ], + }), + ); + + await expect(runConfigCommand(["config", "validate"])).rejects.toThrow("__exit__:1"); + + expectErrorIncludes( + 'openclaw.json:7 — agents.list[3].tools.profile: Invalid input (allowed: "minimal", "coding", "messaging", "full"), got: "none"', + ); + }); + it("returns machine-readable JSON with --json for invalid config", async () => { setSnapshotOnce( makeInvalidSnapshot({ diff --git a/src/cli/config-cli.ts b/src/cli/config-cli.ts index 39063bf8e959..0f68c854778e 100644 --- a/src/cli/config-cli.ts +++ b/src/cli/config-cli.ts @@ -19,6 +19,7 @@ import { } from "../config/config.js"; import { AUTO_MANAGED_CONFIG_META_PATHS } from "../config/io.meta.js"; import { formatConfigIssueLines, normalizeConfigIssues } from "../config/issue-format.js"; +import { attachConfigIssueDiagnostics } from "../config/issue-location.js"; import { normalizeAgentModelMapForConfig, normalizeAgentModelRefForConfig, @@ -952,7 +953,15 @@ async function loadValidConfig(runtime: RuntimeEnv = defaultRuntime) { return snapshot; } runtime.error(`OpenClaw config is invalid: ${shortenHomePath(snapshot.path)}`); - for (const line of formatConfigIssueLines(snapshot.issues, "-", { normalizeRoot: true })) { + const displayIssues = attachConfigIssueDiagnostics(snapshot.issues, { + raw: snapshot.raw, + parsed: snapshot.parsed, + effective: snapshot.sourceConfig, + configPath: snapshot.path, + formatPathForDisplay: true, + includeReceivedValueHint: true, + }); + for (const line of formatConfigIssueLines(displayIssues, "-", { normalizeRoot: true })) { runtime.error(line); } runtime.error(formatInvalidConfigRepairHint(snapshot, "to repair, then retry.")); @@ -2626,8 +2635,18 @@ async function runConfigValidate(opts: { json?: boolean; runtime?: RuntimeEnv } if (opts.json) { writeRuntimeJson(runtime, { valid: false, path: outputPath, issues }); } else { + const displayIssues = attachConfigIssueDiagnostics(issues, { + raw: snapshot.raw, + parsed: snapshot.parsed, + effective: snapshot.sourceConfig, + configPath: snapshot.path, + formatPathForDisplay: true, + includeReceivedValueHint: true, + }); runtime.error(danger(`OpenClaw config is invalid: ${shortPath}`)); - for (const line of formatConfigIssueLines(issues, danger("×"), { normalizeRoot: true })) { + for (const line of formatConfigIssueLines(displayIssues, danger("×"), { + normalizeRoot: true, + })) { runtime.error(` ${line}`); } runtime.error(""); diff --git a/src/config/issue-format.test.ts b/src/config/issue-format.test.ts index d46ee8ab3be0..9f45e184805f 100644 --- a/src/config/issue-format.test.ts +++ b/src/config/issue-format.test.ts @@ -8,6 +8,21 @@ import { } from "./issue-format.js"; describe("config issue format", () => { + it("formats issue lines with source locations", () => { + expect( + formatConfigIssueLine( + { + path: "agents.list[3].tools.profile", + message: 'Invalid input, got: "none"', + line: 247, + sourceFile: "openclaw.json", + }, + "×", + { normalizeRoot: true }, + ), + ).toBe('× openclaw.json:247 — agents.list[3].tools.profile: Invalid input, got: "none"'); + }); + it("formats issue lines with and without markers", () => { expect(formatConfigIssueLine({ path: "", message: "broken" }, "-")).toBe("- : broken"); expect( @@ -55,15 +70,18 @@ describe("config issue format", () => { }); it("normalizes issue collections for machine output", () => { - expect( - normalizeConfigIssues([ - { - path: "update.channel", - message: "invalid", - allowedValues: [], - allowedValuesHiddenCount: 2, - }, - ]), - ).toEqual([{ path: "update.channel", message: "invalid" }]); + const issues = normalizeConfigIssues([ + { + path: "update.channel", + pathSegments: ["update", "channel"], + message: "invalid", + allowedValues: [], + allowedValuesHiddenCount: 2, + }, + ]); + + expect(issues).toEqual([{ path: "update.channel", message: "invalid" }]); + expect(issues[0]?.pathSegments).toEqual(["update", "channel"]); + expect(JSON.stringify(issues)).not.toContain("pathSegments"); }); }); diff --git a/src/config/issue-format.ts b/src/config/issue-format.ts index 719af82bb24b..c0e4df9a6a7e 100644 --- a/src/config/issue-format.ts +++ b/src/config/issue-format.ts @@ -5,10 +5,13 @@ import type { ConfigValidationIssue } from "./types.js"; type ConfigIssueLineInput = { path?: string | null; message: string; + line?: number; + sourceFile?: string; }; type ConfigIssueFormatOptions = { normalizeRoot?: boolean; + sourceFile?: string; }; type ConfigIssueSummaryOptions = ConfigIssueFormatOptions & { @@ -27,7 +30,7 @@ function normalizeConfigIssuePath(path: string | null | undefined): string { /** Return the public config issue shape with a normalized path and non-empty allowed values. */ function normalizeConfigIssue(issue: ConfigValidationIssue): ConfigValidationIssue { const hasAllowedValues = Array.isArray(issue.allowedValues) && issue.allowedValues.length > 0; - return { + const normalized: ConfigValidationIssue = { path: normalizeConfigIssuePath(issue.path), message: issue.message, ...(hasAllowedValues ? { allowedValues: issue.allowedValues } : {}), @@ -37,6 +40,13 @@ function normalizeConfigIssue(issue: ConfigValidationIssue): ConfigValidationIss ? { allowedValuesHiddenCount: issue.allowedValuesHiddenCount } : {}), }; + if (issue.pathSegments) { + Object.defineProperty(normalized, "pathSegments", { + value: issue.pathSegments, + enumerable: false, + }); + } + return normalized; } /** Normalize a batch of config validation issues for display or JSON output. */ @@ -46,6 +56,22 @@ export function normalizeConfigIssues( return issues.map((issue) => normalizeConfigIssue(issue)); } +function resolveIssueLocationPrefix( + issue: ConfigIssueLineInput, + opts?: ConfigIssueFormatOptions, +): string { + const sourceFile = + typeof issue.sourceFile === "string" && issue.sourceFile.trim() + ? issue.sourceFile.trim() + : typeof opts?.sourceFile === "string" && opts.sourceFile.trim() + ? opts.sourceFile.trim() + : ""; + if (!sourceFile || typeof issue.line !== "number" || issue.line <= 0) { + return ""; + } + return `${sanitizeTerminalText(sourceFile)}:${issue.line} — `; +} + function resolveIssuePathForLine( path: string | null | undefined, opts?: ConfigIssueFormatOptions, @@ -66,9 +92,10 @@ export function formatConfigIssueLine( opts?: ConfigIssueFormatOptions, ): string { const prefix = marker ? `${marker} ` : ""; + const locationPrefix = resolveIssueLocationPrefix(issue, opts); const path = sanitizeTerminalText(resolveIssuePathForLine(issue.path, opts)); const message = sanitizeTerminalText(issue.message); - return `${prefix}${path}: ${message}`; + return `${prefix}${locationPrefix}${path}: ${message}`; } /** Format config issues as terminal-safe lines with a shared marker prefix. */ diff --git a/src/config/issue-location.test.ts b/src/config/issue-location.test.ts new file mode 100644 index 000000000000..1d077f8990fd --- /dev/null +++ b/src/config/issue-location.test.ts @@ -0,0 +1,558 @@ +import JSON5 from "json5"; +import { describe, expect, it } from "vitest"; +import { attachConfigIssueDiagnostics } from "./issue-location.js"; + +type PathSegment = string | number; + +function formatConfigIssuePath(pathSegments: PathSegment[]): string { + return ( + attachConfigIssueDiagnostics( + [{ path: pathSegments.join("."), pathSegments, message: "Invalid input" }], + { raw: null, parsed: {}, effective: {}, formatPathForDisplay: true }, + )[0]?.path ?? "" + ); +} + +function resolveConfigIssueLineInRaw(raw: string, pathSegments: PathSegment[]): number | undefined { + let parsed: unknown = {}; + try { + parsed = JSON5.parse(raw); + } catch { + // Malformed or empty text cannot own a source location. + } + return attachConfigIssueDiagnostics( + [{ path: pathSegments.join("."), pathSegments, message: "Invalid input" }], + { raw, parsed, effective: parsed }, + )[0]?.line; +} + +function appendReceivedValueHint(message: string, pathValue: string, value: unknown): string { + const pathSegments = pathValue.split("."); + const root: Record = {}; + let current = root; + for (const segment of pathSegments.slice(0, -1)) { + const child: Record = {}; + current[segment] = child; + current = child; + } + current[pathSegments.at(-1) ?? ""] = value; + return ( + attachConfigIssueDiagnostics([{ path: pathValue, pathSegments, message }], { + raw: JSON5.stringify(root), + parsed: root, + effective: root, + includeReceivedValueHint: true, + })[0]?.message ?? message + ); +} + +describe("formatConfigIssuePath", () => { + it("formats numeric segments with bracket notation", () => { + expect(formatConfigIssuePath(["agents", "list", 3, "tools", "profile"])).toBe( + "agents.list[3].tools.profile", + ); + }); + + it("handles consecutive numeric indices", () => { + expect(formatConfigIssuePath(["a", 0, "b", 1])).toBe("a[0].b[1]"); + }); + + it("returns empty string for empty path", () => { + expect(formatConfigIssuePath([])).toBe(""); + }); + + it("handles all-string path", () => { + expect(formatConfigIssuePath(["foo", "bar", "baz"])).toBe("foo.bar.baz"); + }); +}); + +describe("resolveConfigIssueLineInRaw", () => { + it("resolves line number for nested array object values", () => { + const raw = [ + "{", + ' "agents": {', + ' "list": [', + " {", + ' "id": "main"', + " },", + " {", + ' "tools": {', + ' "profile": "none"', + " }", + " }", + " ]", + " }", + "}", + ].join("\n"); + + expect(resolveConfigIssueLineInRaw(raw, ["agents", "list", 1, "tools", "profile"])).toBe(9); + }); + + it("resolves line number for top-level key", () => { + const raw = ["{", ' "update": {', ' "channel": "nightly"', " }", "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["update", "channel"])).toBe(3); + }); + + it("returns undefined for path not in raw text", () => { + const raw = ["{", "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["nonexistent"])).toBeUndefined(); + }); + + it("handles JSON5 comments", () => { + const raw = ["{", " // comment", ' "key": "value"', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["key"])).toBe(3); + }); + + it("handles comments between unquoted keys and colons", () => { + const raw = ["{", " key // comment", ' : "value"', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["key"])).toBe(3); + }); + + it("handles comments directly after scalar values", () => { + const raw = ["{", " ignored: 1 // comment", ' , target: "bad"', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["target"])).toBe(3); + }); + + it("uses the active value when an object repeats a key", () => { + const raw = ["{", ' key: "old",', ' key: "bad"', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["key"])).toBe(3); + }); + + it("handles single-quoted strings", () => { + const raw = ["{", " 'key': 'value'", "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["key"])).toBe(2); + }); + + it("handles hex numbers as values", () => { + const raw = ["{", ' "a": 0x1A,', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(3); + }); + + it("handles leading decimal numbers", () => { + const raw = ["{", ' "a": .5,', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(3); + }); + + it("handles Infinity value", () => { + const raw = ["{", ' "a": Infinity,', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(3); + }); + + it("handles NaN value", () => { + const raw = ["{", ' "a": NaN,', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(3); + }); + + it("handles null and boolean values", () => { + const raw = ["{", ' "a": null, "b": true, "c": false', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["a"])).toBe(2); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(2); + expect(resolveConfigIssueLineInRaw(raw, ["c"])).toBe(2); + }); + + it("handles trailing commas in objects", () => { + const raw = ["{", ' "a": 1,', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["a"])).toBe(2); + }); + + it("handles trailing commas in arrays", () => { + const raw = ["{", ' "a": [1, 2,]', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["a"])).toBe(2); + }); + + it("handles deeply nested arrays", () => { + const raw = ["{", ' "a": { "b": { "c": [1, [2, [3]]] } } }', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["a", "b", "c", 1, 0])).toBe(2); + }); + + it("handles unicode escape sequences in strings", () => { + const raw = ["{", ' "a": "hello \\u0041",', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(3); + }); + + it("handles multi-line string continuation", () => { + const raw = ["{", ' "a": "hello \\', 'world",', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(4); + }); + + it("handles unicode keys", () => { + const raw = ["{", ' "café": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["café"])).toBe(2); + }); + + it("handles escaped quotes in strings", () => { + const raw = ["{", ' "a": "hello \\"world\\"",', ' "b": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["b"])).toBe(3); + }); + + it("handles block comments before keys", () => { + const raw = ["{", " /* comment */", ' "key": "value"', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["key"])).toBe(3); + }); + + it("handles mixed single/double quotes", () => { + const raw = ["{", " 'key': \"value\"", "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["key"])).toBe(2); + }); + + it("handles empty object value", () => { + const raw = ["{", ' "a": {}', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["a"])).toBe(2); + }); + + it("handles empty array value", () => { + const raw = ["{", ' "a": []', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["a"])).toBe(2); + }); + + it("handles array index navigation with nested objects", () => { + const raw = [ + "{", + ' "agents": {', + ' "list": [', + " {", + ' "id": "main"', + " },", + " {", + ' "tools": {', + ' "profile": "none"', + " }", + " }", + " ]", + " }", + "}", + ].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["agents", "list", 1, "tools", "profile"])).toBe(9); + }); + + it("gracefully degrades for unresolvable paths", () => { + const raw = ["{", ' "a": 1', "}"].join("\n"); + expect(resolveConfigIssueLineInRaw(raw, ["nonexistent"])).toBeUndefined(); + expect(resolveConfigIssueLineInRaw(raw, ["a", "b"])).toBeUndefined(); + expect(resolveConfigIssueLineInRaw(raw, ["a", 0])).toBeUndefined(); + }); + + it("handles empty raw text", () => { + expect(resolveConfigIssueLineInRaw("", ["a"])).toBeUndefined(); + expect(resolveConfigIssueLineInRaw(" ", ["a"])).toBeUndefined(); + }); +}); + +describe("appendReceivedValueHint", () => { + it("appends got: for simple values", () => { + expect( + appendReceivedValueHint( + 'Invalid input (allowed: "minimal", "coding")', + "agents.list[0].tools.profile", + "none", + ), + ).toBe('Invalid input (allowed: "minimal", "coding"), got: "none"'); + }); + + it("skips when message already mentions received", () => { + expect(appendReceivedValueHint("expected string, received number", "gateway.port", 18789)).toBe( + "expected string, received number", + ); + }); + + it("skips sensitive paths", () => { + expect(appendReceivedValueHint("invalid token", "channels.telegram.botToken", "abc123")).toBe( + "invalid token", + ); + }); + + it("skips secret ref objects", () => { + expect( + appendReceivedValueHint("invalid input", "models.providers.openai.apiKey", { + source: "env", + provider: "default", + id: "OPENAI_API_KEY", + }), + ).toBe("invalid input"); + }); + + it("skips object values", () => { + expect(appendReceivedValueHint("invalid input", "some.path", { nested: true })).toBe( + "invalid input", + ); + }); + + it("skips undefined values", () => { + expect(appendReceivedValueHint("invalid input", "some.path", undefined)).toBe("invalid input"); + }); + + it("skips when message already has got:", () => { + expect(appendReceivedValueHint("already got: something", "some.path", "value")).toBe( + "already got: something", + ); + }); + + it.each([ + [Number.NaN, "NaN"], + [Number.POSITIVE_INFINITY, "Infinity"], + [Number.NEGATIVE_INFINITY, "-Infinity"], + [-0, "-0"], + ])("renders JSON5 number %s without coercing it to null", (value, label) => { + expect(appendReceivedValueHint("invalid input", "some.path", value)).toBe( + `invalid input, got: ${label}`, + ); + }); +}); + +describe("attachConfigIssueDiagnostics", () => { + const issue = ( + pathSegments: Array, + message: string, + extra: { allowedValues?: string[] } = {}, + ) => ({ path: pathSegments.join("."), pathSegments, message, ...extra }); + const raw = [ + "{", + ' "agents": {', + ' "list": [', + ' { "tools": { "profile": "none" } }', + " ]", + " }", + "}", + ].join("\n"); + + const parsed = { + agents: { list: [{ tools: { profile: "none" } }] }, + }; + + it("preserves internal path by default", () => { + const issues = attachConfigIssueDiagnostics( + [ + issue( + ["agents", "list", 0, "tools", "profile"], + 'Invalid input (allowed: "minimal", "coding")', + { + allowedValues: ["minimal", "coding"], + }, + ), + ], + { raw, parsed, effective: parsed, configPath: "/tmp/openclaw.json" }, + ); + + expect(issues[0]?.path).toBe("agents.list.0.tools.profile"); + expect(issues[0]?.message).toBe('Invalid input (allowed: "minimal", "coding")'); + expect(issues[0]?.line).toBe(4); + expect(issues[0]?.sourceFile).toBe("openclaw.json"); + }); + + it("formats display path when requested", () => { + const issues = attachConfigIssueDiagnostics( + [ + issue( + ["agents", "list", 0, "tools", "profile"], + 'Invalid input (allowed: "minimal", "coding")', + { + allowedValues: ["minimal", "coding"], + }, + ), + ], + { + raw, + parsed, + effective: parsed, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + expect(issues[0]?.path).toBe("agents.list[0].tools.profile"); + expect(issues[0]?.message).toContain('got: "none"'); + expect(issues[0]?.line).toBe(4); + }); + + it("handles empty raw gracefully", () => { + const issues = attachConfigIssueDiagnostics([issue(["foo"], "error")], { + raw: null, + parsed: {}, + effective: {}, + configPath: "/tmp/openclaw.json", + }); + + expect(issues[0]?.line).toBeUndefined(); + expect(issues[0]?.sourceFile).toBeUndefined(); + }); + + it("handles $include'd paths gracefully (navigator returns undefined)", () => { + const issues = attachConfigIssueDiagnostics( + [ + issue( + ["models", "providers", "openai", "api"], + 'Invalid input (allowed: "openai-chatgpt")', + ), + ], + { + raw: ["{", ' "$include": "./models.json"', "}"].join("\n"), + parsed: { models: { providers: { openai: { api: "bad" } } } }, + effective: { models: { providers: { openai: { api: "bad" } } } }, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + // Path exists in parsed but not in raw — no line number, no received value + expect(issues[0]?.line).toBeUndefined(); + expect(issues[0]?.sourceFile).toBeUndefined(); + expect(issues[0]?.message).toBe('Invalid input (allowed: "openai-chatgpt")'); + }); + + it("preserves numeric record keys (not array indices)", () => { + const issues = attachConfigIssueDiagnostics( + [issue(["plugins", "entries", "123", "config", "mode"], 'Invalid input (allowed: "good")')], + { + raw: [ + "{", + ' "plugins": {', + ' "entries": {', + ' "123": { "config": { "mode": "bad" } }', + " }", + " }", + "}", + ].join("\n"), + parsed: { plugins: { entries: { "123": { config: { mode: "bad" } } } } }, + effective: { plugins: { entries: { "123": { config: { mode: "bad" } } } } }, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + expect(issues[0]?.path).toBe("plugins.entries.123.config.mode"); + expect(issues[0]?.message).toBe('Invalid input (allowed: "good")'); + expect(issues[0]?.line).toBe(4); + }); + + it("preserves non-canonical numeric record keys", () => { + const recordConfig = { records: { "01": { mode: "bad" }, "1": { mode: "good" } } }; + const issues = attachConfigIssueDiagnostics( + [issue(["records", "01", "mode"], "Invalid input")], + { + raw: '{ records: { "01": { mode: "bad" }, "1": { mode: "good" } } }', + parsed: recordConfig, + effective: recordConfig, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + expect(issues[0]).toMatchObject({ + path: "records.01.mode", + message: 'Invalid input, got: "bad"', + line: 1, + }); + }); + + it("uses structured paths to distinguish dotted keys from nested keys", () => { + const dottedConfig = { "foo.bar": "literal", foo: { bar: "nested" } }; + const params = { + raw: ['{ "foo.bar": "literal",', ' foo: { bar: "nested" } }'].join("\n"), + parsed: dottedConfig, + effective: dottedConfig, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }; + + expect( + attachConfigIssueDiagnostics([issue(["foo.bar"], "Invalid input")], params)[0], + ).toMatchObject({ + message: 'Invalid input, got: "literal"', + line: 1, + }); + expect( + attachConfigIssueDiagnostics([issue(["foo", "bar"], "Invalid input")], params)[0], + ).toMatchObject({ + message: 'Invalid input, got: "nested"', + line: 2, + }); + }); + + it("does not let existing values steal missing dotted or nested paths", () => { + const dottedConfig = { "foo.bar": "literal", foo: {} }; + const params = { + raw: ['{ "foo.bar": "literal",', " foo: {} }"].join("\n"), + parsed: dottedConfig, + effective: dottedConfig, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }; + + expect(attachConfigIssueDiagnostics([issue(["foo", "bar"], "Required")], params)[0]).toEqual( + issue(["foo", "bar"], "Required"), + ); + const nestedOnly = { foo: { bar: "nested" } }; + expect( + attachConfigIssueDiagnostics([issue(["foo.bar"], "Required")], { + ...params, + raw: '{ foo: { bar: "nested" } }', + parsed: nestedOnly, + effective: nestedOnly, + })[0], + ).toEqual(issue(["foo.bar"], "Required")); + }); + + it("omits values changed by environment substitution", () => { + const envRaw = raw.replace('"none"', '"${PROFILE}"'); + const issues = attachConfigIssueDiagnostics( + [issue(["agents", "list", 0, "tools", "profile"], "Invalid input")], + { + raw: envRaw, + parsed: { agents: { list: [{ tools: { profile: "${PROFILE}" } }] } }, + effective: { agents: { list: [{ tools: { profile: "none" } }] } }, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + expect(issues[0]?.message).toBe("Invalid input"); + expect(issues[0]?.line).toBe(4); + }); + + it("omits arbitrary plugin-owned values", () => { + const pluginConfig = { + plugins: { entries: { custom: { config: { accessCode: "private" } } } }, + }; + const issues = attachConfigIssueDiagnostics( + [issue(["plugins", "entries", "custom", "config", "accessCode"], "Invalid input")], + { + raw: '{ plugins: { entries: { custom: { config: { accessCode: "private" } } } } }', + parsed: pluginConfig, + effective: pluginConfig, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + expect(issues[0]?.message).toBe("Invalid input"); + expect(issues[0]?.line).toBe(1); + }); + + it("omits plugin-owned values when the plugin id contains dots", () => { + const pluginConfig = { + plugins: { entries: { "vendor.plugin": { config: { sessionValue: "private" } } } }, + }; + const issues = attachConfigIssueDiagnostics( + [issue(["plugins", "entries", "vendor.plugin", "config", "sessionValue"], "Invalid input")], + { + raw: '{ plugins: { entries: { "vendor.plugin": { config: { sessionValue: "private" } } } } }', + parsed: pluginConfig, + effective: pluginConfig, + configPath: "/tmp/openclaw.json", + formatPathForDisplay: true, + includeReceivedValueHint: true, + }, + ); + + expect(issues[0]?.message).toBe("Invalid input"); + expect(issues[0]?.line).toBe(1); + }); +}); diff --git a/src/config/issue-location.ts b/src/config/issue-location.ts new file mode 100644 index 000000000000..68a81d679d70 --- /dev/null +++ b/src/config/issue-location.ts @@ -0,0 +1,369 @@ +import path from "node:path"; +import JSON5 from "json5"; +import { isSensitiveConfigPath } from "./sensitive-paths.js"; +import type { ConfigValidationIssue } from "./types.js"; +import { isSecretRef } from "./types.secrets.js"; + +type ConfigIssuePathSegment = string | number; + +type Cursor = { pos: number }; + +function skipTrivia(raw: string, cursor: Cursor): void { + while (cursor.pos < raw.length) { + const char = raw[cursor.pos]; + if (/\s/u.test(char ?? "")) { + cursor.pos++; + continue; + } + if (char === "/" && raw[cursor.pos + 1] === "/") { + cursor.pos += 2; + while (cursor.pos < raw.length && !/[\n\r\u2028\u2029]/u.test(raw[cursor.pos] ?? "")) { + cursor.pos++; + } + continue; + } + if (char === "/" && raw[cursor.pos + 1] === "*") { + const close = raw.indexOf("*/", cursor.pos + 2); + cursor.pos = close === -1 ? raw.length : close + 2; + continue; + } + return; + } +} + +function scanQuoted(raw: string, cursor: Cursor): void { + const quote = raw[cursor.pos++]; + while (cursor.pos < raw.length) { + const char = raw[cursor.pos++]; + if (char === "\\") { + cursor.pos++; + } else if (char === quote) { + return; + } + } +} + +function consume(raw: string, cursor: Cursor, expected: string): boolean { + skipTrivia(raw, cursor); + if (raw[cursor.pos] !== expected) { + return false; + } + cursor.pos++; + return true; +} + +function readObjectKey(raw: string, cursor: Cursor): string | null { + skipTrivia(raw, cursor); + const start = cursor.pos; + const char = raw[cursor.pos]; + if (char === '"' || char === "'") { + scanQuoted(raw, cursor); + } else { + while ( + cursor.pos < raw.length && + !/[\s:]/u.test(raw[cursor.pos] ?? "") && + !(raw[cursor.pos] === "/" && /[/*]/u.test(raw[cursor.pos + 1] ?? "")) + ) { + cursor.pos++; + } + } + if (cursor.pos === start) { + return null; + } + const token = raw.slice(start, cursor.pos); + try { + if (char === '"' || char === "'") { + const parsed = JSON5.parse(token); + return typeof parsed === "string" ? parsed : null; + } + const parsed = JSON5.parse(`{${token}:null}`) as Record; + return Object.keys(parsed)[0] ?? null; + } catch { + return null; + } +} + +function skipValue(raw: string, cursor: Cursor): void { + skipTrivia(raw, cursor); + const char = raw[cursor.pos]; + if (char === '"' || char === "'") { + scanQuoted(raw, cursor); + return; + } + if (char === "{") { + cursor.pos++; + while (cursor.pos < raw.length) { + skipTrivia(raw, cursor); + if (raw[cursor.pos] === "}") { + cursor.pos++; + return; + } + if (readObjectKey(raw, cursor) === null || !consume(raw, cursor, ":")) { + return; + } + skipValue(raw, cursor); + skipTrivia(raw, cursor); + if (raw[cursor.pos] === ",") { + cursor.pos++; + } + } + return; + } + if (char === "[") { + cursor.pos++; + while (cursor.pos < raw.length) { + skipTrivia(raw, cursor); + if (raw[cursor.pos] === "]") { + cursor.pos++; + return; + } + skipValue(raw, cursor); + skipTrivia(raw, cursor); + if (raw[cursor.pos] === ",") { + cursor.pos++; + } + } + return; + } + while ( + cursor.pos < raw.length && + !/[,}\]\s]/u.test(raw[cursor.pos] ?? "") && + !(raw[cursor.pos] === "/" && /[/*]/u.test(raw[cursor.pos + 1] ?? "")) + ) { + cursor.pos++; + } +} + +function locateValueOffset( + raw: string, + cursor: Cursor, + segments: readonly ConfigIssuePathSegment[], + depth: number, +): number | undefined { + const segment = segments[depth]; + const isLeaf = depth === segments.length - 1; + if (typeof segment === "number") { + if (!consume(raw, cursor, "[")) { + return undefined; + } + for (let index = 0; cursor.pos < raw.length; index++) { + skipTrivia(raw, cursor); + if (raw[cursor.pos] === "]") { + return undefined; + } + if (index === segment) { + return isLeaf ? cursor.pos : locateValueOffset(raw, cursor, segments, depth + 1); + } + skipValue(raw, cursor); + if (!consume(raw, cursor, ",")) { + return undefined; + } + } + return undefined; + } + + if (!consume(raw, cursor, "{")) { + return undefined; + } + let lastMatch: number | undefined; + while (cursor.pos < raw.length) { + skipTrivia(raw, cursor); + if (raw[cursor.pos] === "}") { + return lastMatch; + } + const key = readObjectKey(raw, cursor); + if (key === null || !consume(raw, cursor, ":")) { + return undefined; + } + skipTrivia(raw, cursor); + const valueStart = cursor.pos; + if (key === segment) { + lastMatch = isLeaf + ? valueStart + : locateValueOffset(raw, { pos: valueStart }, segments, depth + 1); + } + cursor.pos = valueStart; + skipValue(raw, cursor); + skipTrivia(raw, cursor); + if (raw[cursor.pos] === ",") { + cursor.pos++; + continue; + } + return lastMatch; + } + return lastMatch; +} + +function lineAtOffset(raw: string, offset: number): number { + let line = 1; + for (let index = 0; index < offset; index++) { + const char = raw[index]; + if (char === "\r") { + line++; + if (raw[index + 1] === "\n") { + index++; + } + } else if (char === "\n" || char === "\u2028" || char === "\u2029") { + line++; + } + } + return line; +} + +function formatConfigIssuePath(segments: readonly ConfigIssuePathSegment[]): string { + return segments.reduce( + (result, segment) => + typeof segment === "number" + ? `${result}[${segment}]` + : result + ? `${result}.${segment}` + : segment, + "", + ); +} + +function resolveConfigValueAtPath( + root: unknown, + segments: readonly ConfigIssuePathSegment[], +): unknown { + let current = root; + for (const segment of segments) { + if (typeof segment === "number") { + if (!Array.isArray(current) || segment >= current.length) { + return undefined; + } + current = current[segment]; + continue; + } + if (!current || typeof current !== "object" || Array.isArray(current)) { + return undefined; + } + current = (current as Record)[segment]; + } + return current; +} + +function stringifyReceivedValue(value: unknown): string | null { + if (value === undefined) { + return null; + } + if (typeof value === "number" && (!Number.isFinite(value) || Object.is(value, -0))) { + return Object.is(value, -0) ? "-0" : String(value); + } + try { + const serialized = JSON.stringify(value); + if (serialized === undefined) { + return null; + } + return serialized.length > 160 ? `${serialized.slice(0, 157)}...` : serialized; + } catch { + return null; + } +} + +function isPluginOwnedConfigPath( + pathValue: string, + pathSegments?: readonly ConfigIssuePathSegment[], +): boolean { + if (pathSegments) { + return ( + pathSegments[0] === "channels" || + (pathSegments[0] === "plugins" && + pathSegments[1] === "entries" && + pathSegments[3] === "config") + ); + } + return ( + pathValue.startsWith("channels.") || /^plugins\.entries\.[^.]+\.config(?:\.|$)/.test(pathValue) + ); +} + +function shouldOmitReceivedValue( + pathValue: string, + value: unknown, + pathSegments?: readonly ConfigIssuePathSegment[], +): boolean { + return ( + value === undefined || + isSecretRef(value) || + isSensitiveConfigPath(pathValue) || + isPluginOwnedConfigPath(pathValue, pathSegments) || + (typeof value === "object" && value !== null) || + stringifyReceivedValue(value) === null + ); +} + +function appendReceivedValueHint( + message: string, + pathValue: string, + value: unknown, + pathSegments?: readonly ConfigIssuePathSegment[], +): string { + if ( + shouldOmitReceivedValue(pathValue, value, pathSegments) || + message.toLowerCase().includes("got:") || + /\breceived\b/i.test(message) + ) { + return message; + } + const label = stringifyReceivedValue(value); + return label ? `${message}, got: ${label}` : message; +} + +function resolveConfigIssueLineInRaw( + raw: string, + segments: readonly ConfigIssuePathSegment[], +): number | undefined { + if (segments.length === 0 || raw.trim().length === 0) { + return undefined; + } + const offset = locateValueOffset(raw, { pos: 0 }, segments, 0); + return offset === undefined ? undefined : lineAtOffset(raw, offset); +} + +type AttachConfigIssueDiagnosticsParams = { + raw: string | null | undefined; + parsed: unknown; + effective: unknown; + configPath?: string | null; + formatPathForDisplay?: boolean; + includeReceivedValueHint?: boolean; +}; + +type ConfigIssueDiagnostics = ConfigValidationIssue & { + line?: number; + sourceFile?: string; +}; + +export function attachConfigIssueDiagnostics( + issues: readonly ConfigValidationIssue[], + params: AttachConfigIssueDiagnosticsParams, +): ConfigIssueDiagnostics[] { + const raw = typeof params.raw === "string" ? params.raw : null; + const sourceFile = + typeof params.configPath === "string" && params.configPath.trim() + ? path.basename(params.configPath) + : "openclaw.json"; + return issues.map((issue) => { + const segments = issue.pathSegments; + if (!segments || segments.length === 0) { + return issue; + } + const literalValue = resolveConfigValueAtPath(params.parsed, segments); + const effectiveValue = resolveConfigValueAtPath(params.effective, segments); + const line = raw === null ? undefined : resolveConfigIssueLineInRaw(raw, segments); + // Validation follows includes, env substitution, and migrations. Only a + // matching root-file literal is both accurate and safe to echo here. + const canShowReceivedValue = line !== undefined && Object.is(literalValue, effectiveValue); + const message = + params.includeReceivedValueHint && canShowReceivedValue + ? appendReceivedValueHint(issue.message, issue.path, effectiveValue, segments) + : issue.message; + return { + ...issue, + path: params.formatPathForDisplay ? formatConfigIssuePath(segments) : issue.path, + message, + ...(line === undefined ? {} : { line, sourceFile }), + }; + }); +} diff --git a/src/config/types.openclaw.ts b/src/config/types.openclaw.ts index 75897c32bd16..9d91409d52f6 100644 --- a/src/config/types.openclaw.ts +++ b/src/config/types.openclaw.ts @@ -278,6 +278,8 @@ export type RuntimeConfig = BrandedConfigState<"runtime">; export type ConfigValidationIssue = { /** Dot-path to the invalid or legacy config value. */ path: string; + /** Structured validator path used internally for lossless source diagnostics. */ + pathSegments?: Array; /** Human-readable validation message. */ message: string; /** Optional allowed values shown to the operator. */ diff --git a/src/config/validation.allowed-values.test.ts b/src/config/validation.allowed-values.test.ts index d2d6430e1d18..aa0cc34e34a5 100644 --- a/src/config/validation.allowed-values.test.ts +++ b/src/config/validation.allowed-values.test.ts @@ -27,6 +27,8 @@ describe("config validation allowed-values metadata", () => { expect(result.ok).toBe(false); if (!result.ok) { const issue = requireIssue(result.issues, "update.channel"); + expect(issue.pathSegments).toEqual(["update", "channel"]); + expect(JSON.stringify(issue)).not.toContain("pathSegments"); expect(issue.message).toContain('(allowed: "stable", "extended-stable", "beta", "dev")'); expect(issue.allowedValues).toEqual(["stable", "extended-stable", "beta", "dev"]); expect(issue.allowedValuesHiddenCount).toBe(0); diff --git a/src/config/validation.ts b/src/config/validation.ts index 4c64ee3f7961..2084f7321024 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -207,6 +207,17 @@ function formatConfigPath(segments: readonly ConfigPathSegment[]): string { return segments.join("."); } +function withConfigIssuePath( + issue: ConfigValidationIssue, + pathSegments: readonly ConfigPathSegment[], +): ConfigValidationIssue { + Object.defineProperty(issue, "pathSegments", { + value: [...pathSegments], + enumerable: false, + }); + return issue; +} + function formatMissingOfficialExternalPluginWarning( pluginId: string, opts?: { selectedMissingMemorySlot?: boolean }, @@ -631,7 +642,7 @@ function isRouteTypeMismatchIssue(issue: UnknownIssueRecord): boolean { function extractBindingsSpecificUnionIssue( record: UnknownIssueRecord, - parentPath: string, + parentPathSegments: readonly ConfigPathSegment[], ): ConfigValidationIssue | null { if (!isBindingsIssuePath(toConfigPathSegments(record.path)) || !Array.isArray(record.errors)) { return null; @@ -700,11 +711,16 @@ function extractBindingsSpecificUnionIssue( return null; } - const subPath = formatConfigPath(toConfigPathSegments(matchingBranchIssue.path)); - const fullPath = parentPath && subPath ? `${parentPath}.${subPath}` : parentPath || subPath; + const fullPathSegments = [ + ...parentPathSegments, + ...toConfigPathSegments(matchingBranchIssue.path), + ]; const subMessage = typeof matchingBranchIssue.message === "string" ? matchingBranchIssue.message : "Invalid input"; - return { path: fullPath, message: subMessage }; + return withConfigIssuePath( + { path: formatConfigPath(fullPathSegments), message: subMessage }, + fullPathSegments, + ); } function isObjectSecretRefCandidate(value: unknown): boolean { @@ -824,7 +840,8 @@ export function collectUnsupportedSecretRefPolicyIssues(raw: unknown): ConfigVal function mapZodIssueToConfigIssue(issue: unknown): ConfigValidationIssue { const record = toIssueRecord(issue); - const pathItem = formatConfigPath(toConfigPathSegments(record?.path)); + const pathSegments = toConfigPathSegments(record?.path); + const pathItem = formatConfigPath(pathSegments); const message = typeof record?.message === "string" ? record.message : "Invalid input"; // Numeric ceiling/floor hints (too_big / too_small with numeric origin). @@ -843,22 +860,25 @@ function mapZodIssueToConfigIssue(issue: unknown): ConfigValidationIssue { record.code === "invalid_union" && !allowedValuesSummary ) { - const betterIssue = extractBindingsSpecificUnionIssue(record, pathItem); + const betterIssue = extractBindingsSpecificUnionIssue(record, pathSegments); if (betterIssue) { return betterIssue; } } if (!allowedValuesSummary) { - return { path: pathItem, message: enrichedMessage }; + return withConfigIssuePath({ path: pathItem, message: enrichedMessage }, pathSegments); } - return { - path: pathItem, - message: appendAllowedValuesHint(enrichedMessage, allowedValuesSummary), - allowedValues: allowedValuesSummary.values, - allowedValuesHiddenCount: allowedValuesSummary.hiddenCount, - }; + return withConfigIssuePath( + { + path: pathItem, + message: appendAllowedValuesHint(enrichedMessage, allowedValuesSummary), + allowedValues: allowedValuesSummary.values, + allowedValuesHiddenCount: allowedValuesSummary.hiddenCount, + }, + pathSegments, + ); } function collectExplicitPluginReferences(raw: unknown): ExplicitPluginReferences {