fix(config): append numeric bound hints to ceiling/floor validation errors by tanshanshan · Pull Request #84852 · openclaw/openclaw

tanshanshan · 2026-05-21T06:21:35Z

Summary

Append numeric ceiling/floor bound hints to config validation error messages when Zod produces too_big / too_small issues with numeric origin
Inclusive bounds: (maximum: 20) / (minimum: 0)
Exclusive bounds: (must be less than 5) / (must be greater than 0)
Only numeric-origin issues are enriched; string, array, and file-size origins are left unchanged
Matches the clarity that enum/union rejections already get via (allowed: …) hints

Motivation

When a config value exceeds a schema-enforced ceiling, the error message omits the maximum allowed value. Operators are told their value was rejected but not what value would be accepted. Previous attempts (#62091, #73220, #83916) were all closed without merging (author capacity / queue noise / too-many-PRs bot), leaving the issue unfixed.

Change Type

fix

Scope

src/config/validation.ts — appendNumericBoundHint() + mapZodIssueToConfigIssue() enrichment
src/config/validation.allowed-values.test.ts — 5 new test cases
src/config/logging-max-file-bytes.test.ts — snapshot update for enriched message

Real behavior proof

Behavior addressed: Config validation error messages for numeric ceiling/floor rejections now include the bound value explicitly, matching the clarity that enum/union rejections get via (allowed: …) hints.

Real environment tested: Local checkout on main (cd019cf), Node 22.22, gateway build verified.

Exact steps or command run after this patch:

node --import tsx -e "import { validateConfigObjectRaw } from './src/config/validation.js'; import { formatConfigIssueLine } from './src/config/issue-format.js'; console.log(formatConfigIssueLine(validateConfigObjectRaw({ session: { agentToAgent: { maxPingPongTurns: 50 } } }).issues[0]));"

Evidence after fix: Terminal output from node --import tsx running validateConfigObjectRaw directly against real config schema objects.

Observed result after fix:

- session.agentToAgent.maxPingPongTurns: Too big: expected number to be <=20 (maximum: 20)
- session.agentToAgent.maxPingPongTurns: Too small: expected number to be >=0 (minimum: 0)
- logging.maxFileBytes: Too small: expected number to be >0 (must be greater than 0)
- update.channel: Invalid input (allowed: "stable", "beta", "dev")

The (maximum: 20), (minimum: 0), and (must be greater than 0) hints are the new enrichment. The enum (allowed: …) line shows existing behavior is unchanged.

What was not tested: Full pnpm check suite; only scoped config validation invocations and colocated unit tests.

Key design decisions

Not using summarizeAllowedValues chain: that path JSON-stringifies string values, producing (allowed: "maximum: 20") with unwanted quotes. Numeric bounds and enum values are different semantics.
origin !== "number" filter: avoids fix(config): include numeric bounds in schema validation errors #73220 P2 bug where string/array/file-size too_big issues would get misleading numeric range wording.
inclusive distinction: .max(20) → (maximum: 20) vs .lt(5) → (must be less than 5). Previous PR fix(config): include numeric bounds in schema validation errors #73220 incorrectly treated exclusive bounds as inclusive, saying "maximum: 5" when 5 itself is not allowed.
.positive() / .gt(0) correctness: these produce inclusive: false, minimum: 0. Output is (must be greater than 0), not the misleading (minimum: 0) that fix(config): include numeric bounds in schema validation errors #73220 produced.
Nullable record guard: toIssueRecord() returns UnknownIssueRecord | null. When null, the original message is preserved (generic "Invalid input" for malformed issues).

Security Impact

None. This only enriches error message text for config validation failures.

…rrors When a config value exceeds a schema-enforced ceiling or falls below a floor, the error message now includes the constraint explicitly: - Inclusive: `(maximum: 20)` / `(minimum: 0)` - Exclusive: `(must be less than 5)` / `(must be greater than 0)` This matches the clarity that enum/union rejections already get via `(allowed: …)` hints, and avoids the misleading "minimum: 0" wording that previous attempts produced for `.positive()` / `.gt(0)` rejections. Only numeric-origin `too_big`/`too_small` issues are enriched; string, array, and file-size origins are left unchanged. Fixes openclaw#52500 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

clawsweeper · 2026-05-21T06:23:00Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
This PR appends numeric Zod ceiling/floor bound hints to config validation messages and updates focused config validation tests.

Reproducibility: yes. source-reproducible: current main maps Zod issues to the raw message without numeric-bound enrichment, and the session schema has a capped numeric config value. I did not execute tests because this review was read-only.

PR rating
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Summary: Focused, likely mergeable PR with real terminal proof and targeted tests, with only broad-check coverage left to CI or maintainer validation.

Rank-up moves:

Run the scoped config tests or changed-check lane before merge if CI does not cover them.

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.

Real behavior proof
Sufficient (terminal): Sufficient: the PR body now includes after-fix copied terminal output from a config validation invocation showing the new maximum/minimum hints.

Risk before merge

The PR body reports scoped config validation invocations and colocated tests, but not a broad changed-check or full check run.

Maintainer options:

Decide the mitigation before merge
Land the narrow mapper and regression-test change after normal checks, then let the linked config ceiling issue close from the merge.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No automated repair is needed because the nullable-record defect was fixed and no blocking findings remain; this is ready for normal maintainer and CI handling.

Security
Cleared: Cleared: the diff only changes config validation error text and colocated tests, with no dependency, CI, credential, or code-execution surface changes.

Review details

Best possible solution:

Land the narrow mapper and regression-test change after normal checks, then let the linked config ceiling issue close from the merge.

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

Yes, source-reproducible: current main maps Zod issues to the raw message without numeric-bound enrichment, and the session schema has a capped numeric config value. I did not execute tests because this review was read-only.

Is this the best way to solve the issue?

Yes, the proposed shape is the narrowest maintainable fix: it uses Zod's numeric too_big/too_small metadata in the existing mapper while keeping allowed-values formatting separate.

Label changes:

add P2: This is a normal-priority operator-facing config validation fix with limited blast radius and focused tests.
add proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body now includes after-fix copied terminal output from a config validation invocation showing the new maximum/minimum hints.
add rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🐚 platinum hermit, patch quality is 🐚 platinum hermit, and Focused, likely mergeable PR with real terminal proof and targeted tests, with only broad-check coverage left to CI or maintainer validation.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Sufficient: the PR body now includes after-fix copied terminal output from a config validation invocation showing the new maximum/minimum hints.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a normal-priority operator-facing config validation fix with limited blast radius and focused tests.
rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🐚 platinum hermit, patch quality is 🐚 platinum hermit, and Focused, likely mergeable PR with real terminal proof and targeted tests, with only broad-check coverage left to CI or maintainer validation.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Sufficient: the PR body now includes after-fix copied terminal output from a config validation invocation showing the new maximum/minimum hints.
proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body now includes after-fix copied terminal output from a config validation invocation showing the new maximum/minimum hints.

What I checked:

Current main omits numeric bound hints: Current main maps a Zod issue to { path, message } when no allowed-values summary exists, so numeric too_big/too_small issues return Zod's native message without the proposed parenthesized bound hint. (src/config/validation.ts:601, 9ec9fbf58d86)
Schema source has the reported numeric ceiling: session.agentToAgent.maxPingPongTurns is defined as an integer with .min(0).max(20), which is the kind of numeric bound the PR enriches. (src/config/zod-schema.session.ts:69, 9ec9fbf58d86)
PR implementation is scoped to validation formatting: The PR adds appendNumericBoundHint() and uses the enriched message in both the no-allowed-values and allowed-values paths without changing schema acceptance or config loading. (src/config/validation.ts:317, 846ac0629bba)
Regression coverage covers inclusive, exclusive, and non-number cases: The PR adds tests for inclusive max/min, exclusive lt/positive bounds, and string max behavior, plus updates the logging maxFileBytes expectation. (src/config/validation.allowed-values.test.ts:146, 846ac0629bba)
Zod contract supports the metadata fields used: Zod 4.4.3 issue types expose too_big/too_small with origin, maximum/minimum, and inclusive; its numeric less-than/greater-than checks emit origin from the bound value type, so number min/max constraints provide the metadata the helper reads.
Related work remains centered on the same open bug: Search found the linked open issue plus prior closed unmerged PR attempts for the same config-bound diagnostic, so this PR is not superseded by a merged fix on current main.

Likely related people:

giodl73-repo: Recent merged commits touched src/config/validation.ts for config diagnostics, provider overlay validation, and metadata preservation. (role: recent config validation contributor; confidence: high; commits: 68c5a892d05f, ff871e162aaf, 19065e4a2fc8; files: src/config/validation.ts)
steipete: Recent history shows work on allowed-values helper shape, config validation tests, logging maxFileBytes tests, and the session ping-pong ceiling. (role: recent adjacent contributor; confidence: high; commits: 3f6b67fd4e86, d90ab9a13f3f, 8cc744ef1f14; files: src/config/allowed-values.ts, src/config/validation.allowed-values.test.ts, src/config/zod-schema.session.ts)
gumadeiras: The allowed-values helper history includes the commit that exposed allowed values in config validation, which is the pattern this PR mirrors for numeric bounds. (role: introduced allowed-values behavior; confidence: medium; commits: f26853f14c9b; files: src/config/allowed-values.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 9ec9fbf58d86.

The test snapshot for `logging.maxFileBytes: 0` rejection now includes the `(must be greater than 0)` hint appended by the numeric bound enrichment added in the previous commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

clawsweeper · 2026-05-21T06:28:26Z

ClawSweeper PR egg

✨ Hatched: 🌱 uncommon Sunspot Diff Drake

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🌱 uncommon.
Trait: watches the merge queue.
Image traits: location merge queue dock; accessory rollback rope; palette moonlit blue and soft silver; mood watchful; pose curling around a status light; shell translucent glimmer shell; lighting tiny status-light glow; background gentle dashboard dots.
Share on X: post this hatch
Copy: My PR egg hatched a 🌱 uncommon Sunspot Diff Drake in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

ClawSweeper P1: `record` from `toIssueRecord()` can be null, but `appendNumericBoundHint` expects a non-null `UnknownIssueRecord`. Guard with a ternary so the original message is returned when record is null (which only happens for malformed/empty issues that already produce generic "Invalid input" messages). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

clawsweeper · 2026-05-21T06:40:14Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26209953938
Updated: 2026-05-21T06:47:54.461Z

…rrors (openclaw#84852) * fix(config): append numeric bound hints to ceiling/floor validation errors When a config value exceeds a schema-enforced ceiling or falls below a floor, the error message now includes the constraint explicitly: - Inclusive: `(maximum: 20)` / `(minimum: 0)` - Exclusive: `(must be less than 5)` / `(must be greater than 0)` This matches the clarity that enum/union rejections already get via `(allowed: …)` hints, and avoids the misleading "minimum: 0" wording that previous attempts produced for `.positive()` / `.gt(0)` rejections. Only numeric-origin `too_big`/`too_small` issues are enriched; string, array, and file-size origins are left unchanged. Fixes openclaw#52500 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(config): update maxFileBytes test for numeric bound hint The test snapshot for `logging.maxFileBytes: 0` rejection now includes the `(must be greater than 0)` hint appended by the numeric bound enrichment added in the previous commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(config): guard nullable record in appendNumericBoundHint call ClawSweeper P1: `record` from `toIssueRecord()` can be null, but `appendNumericBoundHint` expects a non-null `UnknownIssueRecord`. Guard with a ternary so the original message is returned when record is null (which only happens for malformed/empty issues that already produce generic "Invalid input" messages). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: tanshanshan <tanshanshan@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 21, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 21, 2026

This comment was marked as low quality.

Sign in to view

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 21, 2026

vincentkoc merged commit 8a8f9dc into openclaw:main May 21, 2026
149 of 158 checks passed

github-actions Bot mentioned this pull request May 21, 2026

📡 Upstream Digest — 2026-05-21 10:59 UTC curtismercier/openclaw-mods#907

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(config): append numeric bound hints to ceiling/floor validation errors#84852

fix(config): append numeric bound hints to ceiling/floor validation errors#84852
vincentkoc merged 3 commits into
openclaw:mainfrom
tanshanshan:fix/config-numeric-bound-hints-52500

tanshanshan commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026 •

edited

Loading

Uh oh!

This comment was marked as low quality.

clawsweeper Bot commented May 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tanshanshan commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Change Type

Scope

Real behavior proof

Key design decisions

Security Impact

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Hatch command

Uh oh!

This comment was marked as low quality.

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tanshanshan commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading