Skip to content

Commit 135aa7a

Browse files
ekinneeErick Kinneesteipete
authored
fix(config): improve validation error messages with line numbers, bracket paths, and received values (#106526)
* fix(config): improve validation error messages with line numbers, bracket paths, and received values - Add JSON5-aware path navigator (issue-location.ts) that resolves line numbers and received values from raw config text without a full parser - Add formatConfigIssuePath() for bracket notation (agents.list[3]) - Add appendReceivedValueHint() for safe value display (got: "none") - Add attachConfigIssueDiagnostics() to enrich issues with location info - Wire enrichment into runConfigValidate() and loadValidConfig() in config-cli.ts - Add resolveIssueLocationPrefix() to issue-format.ts for sourceFile:line prefix - Sensitive paths and secret refs are never leaked in received value hints - Paths in 'd files gracefully degrade (no line number shown) Closes #104854 * fix(config): type fixes from tsgo:core validation - Add sourceFile to ConfigIssueFormatOptions (used in resolveIssueLocationPrefix) - Fix readKey regex test for undefined raw[c.pos] * fix(config): satisfy eslint(curly) and formatting for issue-location.ts - Add braces to all single-line if/while/for statements - Run oxfmt for consistent formatting * fix(config): trim issue-location.ts to 500 lines for LOC ratchet * fix(config): fix formatConfigIssuePath ternary bug from trimming * fix(config): trim issue-location.ts to 469 lines for LOC ratchet * fix(config): format issue-location.test.ts * fix(config): document JSON5 subset and add 20 edge case tests - Add explicit supported-subset documentation to issue-location.ts header - Add 20 tests covering hex numbers, leading decimals, Infinity, NaN, null/bool, trailing commas, deep nesting, unicode escapes, multi-line strings, unicode keys, escaped quotes, numeric separators, block comments, mixed quotes, empty objects/arrays, and graceful degradation * fix(config): address autoreview findings on JSON5 docs and tests - Remove numeric separator claim (not valid JSON5) - Remove 'None known' unsupported-syntax claim, document known limitations - Restructure scalar tests to exercise skipVal (resolve sibling after exotic value) - Remove numeric separator test * fix(config): constrain received value diagnostics * fix(config): preserve JSON5 diagnostic locations * fix(config): preserve validation path ownership * fix(config): redact dotted plugin paths * test(config): avoid diagnostic fixture shadowing * refactor(config): privatize diagnostic helpers --------- Co-authored-by: Erick Kinnee <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
1 parent 8d5e39a commit 135aa7a

9 files changed

Lines changed: 1091 additions & 31 deletions

‎src/cli/config-cli.test.ts‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,14 +310,18 @@ function makeInvalidSnapshot(params: {
310310
issues: ConfigFileSnapshot["issues"];
311311
warnings?: ConfigFileSnapshot["warnings"];
312312
path?: string;
313+
raw?: string;
314+
parsed?: unknown;
315+
sourceConfig?: OpenClawConfig;
313316
}): ConfigFileSnapshot {
317+
const parsed = params.parsed ?? {};
314318
return {
315319
path: params.path ?? "/tmp/custom-openclaw.json",
316320
exists: true,
317-
raw: "{}",
318-
parsed: {},
319-
sourceConfig: {},
320-
resolved: {},
321+
raw: params.raw ?? "{}",
322+
parsed,
323+
sourceConfig: params.sourceConfig ?? (parsed as OpenClawConfig),
324+
resolved: parsed as OpenClawConfig,
321325
valid: false,
322326
runtimeConfig: {},
323327
config: {},
@@ -1165,6 +1169,47 @@ describe("config cli", () => {
11651169
expect(mockLog).not.toHaveBeenCalled();
11661170
});
11671171

1172+
it("prints line numbers, bracket array paths, and safe received values", async () => {
1173+
const parsed = {
1174+
agents: {
1175+
list: [{ id: "a" }, { id: "b" }, { id: "c" }, { id: "d", tools: { profile: "none" } }],
1176+
},
1177+
};
1178+
const raw = [
1179+
"{",
1180+
' "agents": {',
1181+
' "list": [',
1182+
' { "id": "a" },',
1183+
' { "id": "b" },',
1184+
' { "id": "c" },',
1185+
' { "id": "d", "tools": { "profile": "none" } }',
1186+
" ]",
1187+
" }",
1188+
"}",
1189+
].join("\n");
1190+
setSnapshotOnce(
1191+
makeInvalidSnapshot({
1192+
raw,
1193+
parsed,
1194+
path: "/tmp/openclaw.json",
1195+
issues: [
1196+
{
1197+
path: "agents.list.3.tools.profile",
1198+
pathSegments: ["agents", "list", 3, "tools", "profile"],
1199+
message: 'Invalid input (allowed: "minimal", "coding", "messaging", "full")',
1200+
allowedValues: ["minimal", "coding", "messaging", "full"],
1201+
},
1202+
],
1203+
}),
1204+
);
1205+
1206+
await expect(runConfigCommand(["config", "validate"])).rejects.toThrow("__exit__:1");
1207+
1208+
expectErrorIncludes(
1209+
'openclaw.json:7 — agents.list[3].tools.profile: Invalid input (allowed: "minimal", "coding", "messaging", "full"), got: "none"',
1210+
);
1211+
});
1212+
11681213
it("returns machine-readable JSON with --json for invalid config", async () => {
11691214
setSnapshotOnce(
11701215
makeInvalidSnapshot({

‎src/cli/config-cli.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from "../config/config.js";
2020
import { AUTO_MANAGED_CONFIG_META_PATHS } from "../config/io.meta.js";
2121
import { formatConfigIssueLines, normalizeConfigIssues } from "../config/issue-format.js";
22+
import { attachConfigIssueDiagnostics } from "../config/issue-location.js";
2223
import {
2324
normalizeAgentModelMapForConfig,
2425
normalizeAgentModelRefForConfig,
@@ -952,7 +953,15 @@ async function loadValidConfig(runtime: RuntimeEnv = defaultRuntime) {
952953
return snapshot;
953954
}
954955
runtime.error(`OpenClaw config is invalid: ${shortenHomePath(snapshot.path)}`);
955-
for (const line of formatConfigIssueLines(snapshot.issues, "-", { normalizeRoot: true })) {
956+
const displayIssues = attachConfigIssueDiagnostics(snapshot.issues, {
957+
raw: snapshot.raw,
958+
parsed: snapshot.parsed,
959+
effective: snapshot.sourceConfig,
960+
configPath: snapshot.path,
961+
formatPathForDisplay: true,
962+
includeReceivedValueHint: true,
963+
});
964+
for (const line of formatConfigIssueLines(displayIssues, "-", { normalizeRoot: true })) {
956965
runtime.error(line);
957966
}
958967
runtime.error(formatInvalidConfigRepairHint(snapshot, "to repair, then retry."));
@@ -2626,8 +2635,18 @@ async function runConfigValidate(opts: { json?: boolean; runtime?: RuntimeEnv }
26262635
if (opts.json) {
26272636
writeRuntimeJson(runtime, { valid: false, path: outputPath, issues });
26282637
} else {
2638+
const displayIssues = attachConfigIssueDiagnostics(issues, {
2639+
raw: snapshot.raw,
2640+
parsed: snapshot.parsed,
2641+
effective: snapshot.sourceConfig,
2642+
configPath: snapshot.path,
2643+
formatPathForDisplay: true,
2644+
includeReceivedValueHint: true,
2645+
});
26292646
runtime.error(danger(`OpenClaw config is invalid: ${shortPath}`));
2630-
for (const line of formatConfigIssueLines(issues, danger("×"), { normalizeRoot: true })) {
2647+
for (const line of formatConfigIssueLines(displayIssues, danger("×"), {
2648+
normalizeRoot: true,
2649+
})) {
26312650
runtime.error(` ${line}`);
26322651
}
26332652
runtime.error("");

‎src/config/issue-format.test.ts‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,21 @@ import {
88
} from "./issue-format.js";
99

1010
describe("config issue format", () => {
11+
it("formats issue lines with source locations", () => {
12+
expect(
13+
formatConfigIssueLine(
14+
{
15+
path: "agents.list[3].tools.profile",
16+
message: 'Invalid input, got: "none"',
17+
line: 247,
18+
sourceFile: "openclaw.json",
19+
},
20+
"×",
21+
{ normalizeRoot: true },
22+
),
23+
).toBe('× openclaw.json:247 — agents.list[3].tools.profile: Invalid input, got: "none"');
24+
});
25+
1126
it("formats issue lines with and without markers", () => {
1227
expect(formatConfigIssueLine({ path: "", message: "broken" }, "-")).toBe("- : broken");
1328
expect(
@@ -55,15 +70,18 @@ describe("config issue format", () => {
5570
});
5671

5772
it("normalizes issue collections for machine output", () => {
58-
expect(
59-
normalizeConfigIssues([
60-
{
61-
path: "update.channel",
62-
message: "invalid",
63-
allowedValues: [],
64-
allowedValuesHiddenCount: 2,
65-
},
66-
]),
67-
).toEqual([{ path: "update.channel", message: "invalid" }]);
73+
const issues = normalizeConfigIssues([
74+
{
75+
path: "update.channel",
76+
pathSegments: ["update", "channel"],
77+
message: "invalid",
78+
allowedValues: [],
79+
allowedValuesHiddenCount: 2,
80+
},
81+
]);
82+
83+
expect(issues).toEqual([{ path: "update.channel", message: "invalid" }]);
84+
expect(issues[0]?.pathSegments).toEqual(["update", "channel"]);
85+
expect(JSON.stringify(issues)).not.toContain("pathSegments");
6886
});
6987
});

‎src/config/issue-format.ts‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ import type { ConfigValidationIssue } from "./types.js";
55
type ConfigIssueLineInput = {
66
path?: string | null;
77
message: string;
8+
line?: number;
9+
sourceFile?: string;
810
};
911

1012
type ConfigIssueFormatOptions = {
1113
normalizeRoot?: boolean;
14+
sourceFile?: string;
1215
};
1316

1417
type ConfigIssueSummaryOptions = ConfigIssueFormatOptions & {
@@ -27,7 +30,7 @@ function normalizeConfigIssuePath(path: string | null | undefined): string {
2730
/** Return the public config issue shape with a normalized path and non-empty allowed values. */
2831
function normalizeConfigIssue(issue: ConfigValidationIssue): ConfigValidationIssue {
2932
const hasAllowedValues = Array.isArray(issue.allowedValues) && issue.allowedValues.length > 0;
30-
return {
33+
const normalized: ConfigValidationIssue = {
3134
path: normalizeConfigIssuePath(issue.path),
3235
message: issue.message,
3336
...(hasAllowedValues ? { allowedValues: issue.allowedValues } : {}),
@@ -37,6 +40,13 @@ function normalizeConfigIssue(issue: ConfigValidationIssue): ConfigValidationIss
3740
? { allowedValuesHiddenCount: issue.allowedValuesHiddenCount }
3841
: {}),
3942
};
43+
if (issue.pathSegments) {
44+
Object.defineProperty(normalized, "pathSegments", {
45+
value: issue.pathSegments,
46+
enumerable: false,
47+
});
48+
}
49+
return normalized;
4050
}
4151

4252
/** Normalize a batch of config validation issues for display or JSON output. */
@@ -46,6 +56,22 @@ export function normalizeConfigIssues(
4656
return issues.map((issue) => normalizeConfigIssue(issue));
4757
}
4858

59+
function resolveIssueLocationPrefix(
60+
issue: ConfigIssueLineInput,
61+
opts?: ConfigIssueFormatOptions,
62+
): string {
63+
const sourceFile =
64+
typeof issue.sourceFile === "string" && issue.sourceFile.trim()
65+
? issue.sourceFile.trim()
66+
: typeof opts?.sourceFile === "string" && opts.sourceFile.trim()
67+
? opts.sourceFile.trim()
68+
: "";
69+
if (!sourceFile || typeof issue.line !== "number" || issue.line <= 0) {
70+
return "";
71+
}
72+
return `${sanitizeTerminalText(sourceFile)}:${issue.line} — `;
73+
}
74+
4975
function resolveIssuePathForLine(
5076
path: string | null | undefined,
5177
opts?: ConfigIssueFormatOptions,
@@ -66,9 +92,10 @@ export function formatConfigIssueLine(
6692
opts?: ConfigIssueFormatOptions,
6793
): string {
6894
const prefix = marker ? `${marker} ` : "";
95+
const locationPrefix = resolveIssueLocationPrefix(issue, opts);
6996
const path = sanitizeTerminalText(resolveIssuePathForLine(issue.path, opts));
7097
const message = sanitizeTerminalText(issue.message);
71-
return `${prefix}${path}: ${message}`;
98+
return `${prefix}${locationPrefix}${path}: ${message}`;
7299
}
73100

74101
/** Format config issues as terminal-safe lines with a shared marker prefix. */

0 commit comments

Comments
 (0)