fix(config): improve validation error messages with line numbers, bracket paths, and received values by ekinnee · Pull Request #106526 · openclaw/openclaw

ekinnee · 2026-07-13T15:51:54Z

What Problem This Solves

openclaw config validate reported lossy dot paths without source locations, making nested and array validation failures difficult to find. It also omitted the rejected scalar value even when that value was safe to display.

Why This Change Was Made

This improves human-readable validation errors without changing the config schema, parser, or machine-readable issue shape:

array indices use bracket notation, such as agents.list[3].tools.profile
root-file values include openclaw.json:<line> when the exact JSON5 value can be located
safe scalar values add a got: hint
structured Zod paths are retained internally so dotted record keys and numeric record keys cannot be confused
the locator follows canonical JSON5 key parsing and last-duplicate-key semantics

Received values are omitted for sensitive paths, SecretRefs, plugin-owned config, channel config, objects, environment-substituted values, included-only values, and migrated values. Plugin IDs containing dots remain inside the same redaction boundary.

User Impact

A safe root-file failure can now render as:

openclaw.json:247 — agents.list[3].tools.profile: Invalid input (allowed: "minimal", "coding"), got: "none"

When source ownership or value safety cannot be proven, output degrades to the existing path and message without a line or received value.

Evidence

node scripts/run-vitest.mjs src/config/issue-location.test.ts src/config/issue-format.test.ts src/config/validation.allowed-values.test.ts src/cli/config-cli.test.ts — 199 tests passed on rebased head 2490aa0fc37e
node scripts/check-changed.mjs -- <changed config files> — guards, formatting, core and core-test typechecks, full core lint, and database/runtime guards passed in hosted Testbox
exact-head CI passed, including dependency deadcode, in CI run 29553384563
fresh full-branch autoreview on rebased head: no actionable findings

AI-assisted: implementation by Luci (OpenClaw agent), improved and reviewed by OpenClaw maintainers.

Closes #104854

clawsweeper · 2026-07-13T17:49:14Z

Codex review: found issues before merge. Reviewed July 16, 2026, 9:54 PM ET / July 17, 2026, 01:54 UTC.

Summary
The PR adds source line numbers, bracket notation for array indices, and safety-filtered received-value hints to human-facing config validation errors.

PR surface: Source +529, Tests +449. Total +978 across 6 files.

Reproducibility: yes. Current main formats dotted validation paths directly, and an invalid enum under an array provides a concrete source-level reproduction; the contributor also supplies convincing after-fix CLI output.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #104854
Summary: The issue is the canonical report; this PR is its broad candidate fix, while the competing PR covers only bracket-path formatting.

Members:

canonical: Config validation errors should show line number, bad value, and bracket notation for array indices #104854 - It defines the three requested diagnostics improvements and is explicitly linked for closure by this PR.
partial_overlap: fix(config): render validation array indices with brackets #104935 - It implements bracket notation only and intentionally excludes line numbers and received-value rendering.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Derive received-value hints from the effective resolved source config while retaining raw root text for line lookup.
[P1] Obtain config/security owner agreement on a fail-closed disclosure policy and add focused environment-substitution and plugin-sensitive-path coverage.

Risk before merge

[P1] Environment-substituted values can be reported as their authored ${ENV} template instead of the effective invalid value, while values originating in included files are omitted entirely.
[P1] Echoing received values creates a credential-disclosure boundary; path-based sensitivity classification must fail closed for core, plugin-owned, and future credential fields before merge.

Maintainer options:

Separate location and value sources (recommended)
Before merge, retain raw and parsed for root-file navigation but read received values from sourceConfig under a fail-closed disclosure policy.
Land diagnostics without values
Remove received-value output and retain only bracket paths and source locations if maintainers do not want to establish the disclosure contract in this PR.
Pause for security ownership
Keep the PR open without further implementation churn until a config/security owner chooses the permanent disclosure boundary.

Next step before merge

[P2] A config/security owner should choose the received-value disclosure contract; the contributor can then make the bounded effective-value repair, add security coverage, rebase onto current main, and refresh proof.

Maintainer decision needed

Question: What received-value disclosure contract should openclaw config validate enforce for human-facing errors?
Rationale: The PR intentionally prints user-authored configuration values, and repository-wide path sensitivity plus plugin-owned credential coverage is a security policy choice that cannot be inferred safely from the patch alone.
Likely owner: Sid-Qin — The available history connects this contributor to source-aware config-state semantics central to the decision, though an official security/config owner may be preferable.
Options:
- Fail-closed scalar hints (recommended): Show bounded scalar values only when the effective resolved path is affirmatively non-sensitive, otherwise omit the hint.
- No received values: Land line numbers and bracket paths but omit received-value hints until a stronger metadata-backed disclosure contract exists.
- Accept path denylisting: Keep the current path-based suppression model and explicitly own the possibility that newly introduced sensitive fields require classifier updates.

Security
Needs attention: The patch introduces received-value terminal disclosure without a confirmed fail-closed contract covering every core and plugin-owned sensitive field.

Review findings

[P2] Use the effective validated value for received-value hints — src/cli/config-cli.ts:956

Review details

Best possible solution:

Keep raw root-file text only for source-location lookup, derive received scalar values from the resolved source config or validation issue, and expose them only under a maintainer-approved fail-closed sensitivity policy while preserving unchanged JSON output.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main formats dotted validation paths directly, and an invalid enum under an array provides a concrete source-level reproduction; the contributor also supplies convincing after-fix CLI output.

Is this the best way to solve the issue?

No, not yet. Display-layer enrichment is the correct boundary, but received values must come from the effective validated config rather than pre-resolution input, and their disclosure policy must fail closed.

Full review comments:

[P2] Use the effective validated value for received-value hints — src/cli/config-cli.ts:956
snapshot.parsed is the authored root value before $include and ${ENV} resolution, while schema validation operates on the resolved source config. For example, a field authored as ${PROFILE} and resolved to none will report the template instead of the rejected value, and included values cannot be reported. Pass the effective sourceConfig separately for value lookup while keeping raw/parsed for root-file location mapping. This is the still-unfixed finding from the prior review cycle.
Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1f5c8239d71c.

Label changes

Label justifications:

P2: This is a useful, bounded CLI diagnostics bug fix with limited runtime blast radius, but it is not an urgent workflow outage.
merge-risk: 🚨 security-boundary: Merging enables configuration values to be printed to terminal output, so incomplete sensitivity classification could disclose credentials.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body contains after-fix live terminal output from head 18137223b43 covering the visible CLI behavior and safety/degradation cases, with private paths redacted.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains after-fix live terminal output from head 18137223b43 covering the visible CLI behavior and safety/degradation cases, with private paths redacted.

Evidence reviewed

PR surface:

Source +529, Tests +449. Total +978 across 6 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	532	3	+529
Tests	3	453	4	+449
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	6	985	7	+978

Security concerns:

[medium] Confirm a fail-closed received-value disclosure policy
The new diagnostic path relies on sensitivity classification and SecretRef detection before echoing configuration values. A newly added or plugin-owned credential path that is not classified could be printed, so maintainers should approve an affirmative safe-value policy or remove value hints.
Confidence: 0.82

What I checked:

Current-head value source: Both changed CLI paths pass snapshot.parsed into diagnostic enrichment; that field represents the authored value before include and environment resolution, so it can differ from the value rejected by validation. (src/cli/config-cli.ts:956, 18137223b436)
Snapshot contract: Current main defines parsed as the pre-normalization parsed file and sourceConfig as config after $include and ${ENV} resolution, before runtime defaults. (github.com) (src/config/types.openclaw.ts:1474, 1f5c8239d71c)
Validation producer: Current main maps issues from the values passed through schema validation and discards the underlying received value when constructing ConfigValidationIssue. (github.com) (src/config/validation.ts:3673, 1f5c8239d71c)
Current main still lacks the change: The current main CLI continues to format snapshot.issues directly and does not contain attachConfigIssueDiagnostics, so this PR remains necessary. (github.com) (src/cli/config-cli.ts:3961, 1f5c8239d71c)
Prior finding continuity: The latest completed ClawSweeper cycle raised the same effective-value finding at head 7443eb0f48d2d587d85a125f40c68cd98a10418f; the current caller still supplies snapshot.parsed, so the blocker has not been resolved. (src/cli/config-cli.ts:956, 18137223b436)
Real CLI proof: The PR body provides current-head terminal output for bracket formatting, root-file line resolution, safe value display, sensitive-path suppression, include degradation, and numeric record keys; the dedicated real-behavior-proof checks also succeeded. (18137223b436)

Likely related people:

Sid-Qin: Recent source-aware config work is directly relevant to distinguishing authored parsed data from resolved sourceConfig; the available history trail is limited because local git inspection was unavailable. (role: recent source-config contributor; confidence: medium; commits: 8bd3067e694d; files: src/config/io.ts, src/config/types.openclaw.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (15 earlier review cycles; latest 8 shown)

reviewed 2026-07-14T06:56:55.242Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective value for received-value hints
reviewed 2026-07-14T07:48:48.388Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective value for received-value hints
reviewed 2026-07-14T08:38:39.102Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective value for received-value hints
reviewed 2026-07-14T09:21:04.436Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective value for received-value hints
reviewed 2026-07-14T10:06:37.932Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective value for received-value hints
reviewed 2026-07-14T10:36:14.731Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective value for received-value hints
reviewed 2026-07-14T11:03:27.378Z sha 7443eb0 :: found issues before merge. :: [P2] Use the effective validated value for received-value hints
reviewed 2026-07-17T01:11:54.545Z sha 1813722 :: found issues before merge. :: [P2] Use the effective rejected value for got: hints

…cket 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 openclaw#104854

- Add sourceFile to ConfigIssueFormatOptions (used in resolveIssueLocationPrefix) - Fix readKey regex test for undefined raw[c.pos]

- Add braces to all single-line if/while/for statements - Run oxfmt for consistent formatting

- 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

- 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

steipete · 2026-07-17T04:01:34Z

Merged via squash.

Prepared head SHA: 2490aa0fc37e9418e8dd1577d736df554688ec98
Landed commit: 135aa7a0025dbd4615db08ee1df3a85eec86f910

…cket paths, and received values (openclaw#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 openclaw#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]>

openclaw-barnacle Bot added cli CLI command changes size: L scripts Repository scripts size: XL and removed size: L labels Jul 13, 2026

mushuiyu886 mentioned this pull request Jul 14, 2026

fix(plugins): preserve state on npm metadata failures #107063

Merged

clawsweeper Bot mentioned this pull request Jul 16, 2026

fix(config): render validation array indices with brackets #104935

Merged

ekinnee force-pushed the im-104969 branch 2 times, most recently from 525bfe5 to 1813722 Compare July 17, 2026 00:59

openclaw-barnacle Bot added size: L and removed scripts Repository scripts size: XL labels Jul 17, 2026

steipete self-assigned this Jul 17, 2026

steipete force-pushed the im-104969 branch from 1813722 to 011409a Compare July 17, 2026 03:40

openclaw-barnacle Bot added size: XL and removed size: L labels Jul 17, 2026

Erick Kinnee added 4 commits July 17, 2026 04:46

fix(config): type fixes from tsgo:core validation

4e90efa

- 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

d453bb0

- 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

072c695

Erick Kinnee and others added 11 commits July 17, 2026 04:46

fix(config): fix formatConfigIssuePath ternary bug from trimming

1235704

fix(config): trim issue-location.ts to 469 lines for LOC ratchet

7f020ff

fix(config): format issue-location.test.ts

6c561df

fix(config): constrain received value diagnostics

12e6ebd

fix(config): preserve JSON5 diagnostic locations

1ecf3b7

fix(config): preserve validation path ownership

619fa58

fix(config): redact dotted plugin paths

1b4ec3c

test(config): avoid diagnostic fixture shadowing

06b5a62

refactor(config): privatize diagnostic helpers

2490aa0

steipete force-pushed the im-104969 branch from 011409a to 2490aa0 Compare July 17, 2026 03:50

steipete merged commit 135aa7a into openclaw:main Jul 17, 2026
125 checks passed

github-actions Bot mentioned this pull request Jul 17, 2026

📡 Upstream Digest — 2026-07-17 04:57 UTC curtismercier/openclaw-mods#1319

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

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#106526
steipete merged 15 commits into
openclaw:mainfrom
ekinnee:im-104969

ekinnee commented Jul 13, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

ekinnee commented Jul 13, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ekinnee commented Jul 13, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading