Skip to content

test(config): add unit tests for mapZodIssueToConfigIssue#97736

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
solodmd:test/validation-unit-tests
Jun 29, 2026
Merged

test(config): add unit tests for mapZodIssueToConfigIssue#97736
vincentkoc merged 2 commits into
openclaw:mainfrom
solodmd:test/validation-unit-tests

Conversation

@solodmd

@solodmd solodmd commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

The validation.ts module provides config validation for OpenClaw runtime settings, including the mapZodIssueToConfigIssue function that transforms raw Zod validation issues into user-facing ConfigValidationIssue objects. Despite being a core path in config validation, this function had no direct unit tests, with only 2 indirect tests through higher-level validation flows.

Why This Change Was Made

Add focused unit tests for mapZodIssueToConfigIssue to verify:

  • Basic path and message mapping (string/numeric paths, missing messages)
  • Numeric bound hints (too_big/too_small with inclusive/exclusive boundaries)
  • Allowed values collection (invalid_value, invalid_type→boolean, custom expected one of patterns, invalid_union branches)
  • Allowed values hidden count when values exceed 12
  • Bindings-specific union extraction with route-type-mismatch filtering
  • Edge cases (null/undefined input, non-array paths, malformed errors)

This improves test coverage and prevents regressions when the function is modified.

User Impact

Config validation issue formatting now has 34 dedicated unit tests covering the main branching paths, reducing the risk of regressions.

Evidence

Direct behavior probe:

$ node --import tsx -e "import('./src/config/validation.js').then(({ testing }) => console.log(JSON.stringify([{ desc: 'simple path+message', result: testing.mapZodIssueToConfigIssue({ code: 'custom', path: ['channels', 'test'], message: 'invalid' }) }, { desc: 'too_big hint', result: testing.mapZodIssueToConfigIssue({ code: 'too_big', path: ['max'], message: 'too big', origin: 'number', maximum: 100, inclusive: true }) }, { desc: 'invalid_value allowed', result: testing.mapZodIssueToConfigIssue({ code: 'invalid_value', path: ['mode'], message: 'bad', values: ['a', 'b', 'c'] }) }, { desc: 'null input', result: testing.mapZodIssueToConfigIssue(null) }], null, 2)))"
[
  {
    "desc": "simple path+message",
    "result": {
      "path": "channels.test",
      "message": "invalid"
    }
  },
  {
    "desc": "too_big hint",
    "result": {
      "path": "max",
      "message": "too big (maximum: 100)"
    }
  },
  {
    "desc": "invalid_value allowed",
    "result": {
      "path": "mode",
      "message": "bad (allowed: \"a\", \"b\", \"c\")",
      "allowedValues": [
        "a",
        "b",
        "c"
      ],
      "allowedValuesHiddenCount": 0
    }
  },
  {
    "desc": "null input",
    "result": {
      "path": "",
      "message": "Invalid input"
    }
  }
]

Targeted test:

$ pnpm test src/config/validation.test.ts

 RUN  v4.1.8

 Test Files  1 passed (1)
      Tests  34 passed (34)
   Start at  17:15:21
   Duration  2.96s (transform 1.02s, setup 76ms, import 2.54s, tests 32ms, environment 0ms)

[test] passed 1 Vitest shard in 14.61s

Formatting:

$ oxfmt --check src/config/validation.test.ts
Checking formatting...
All matched files use the correct format.

Whitespace:

$ git diff --check

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 5:41 AM ET / 09:41 UTC.

Summary
Adds src/config/validation.test.ts with direct Vitest coverage for testing.mapZodIssueToConfigIssue path formatting, messages, numeric bounds, allowed values, bindings union extraction, and malformed inputs.

PR surface: Tests +436. Total +436 across 1 file.

Reproducibility: not applicable. This PR adds test coverage rather than reporting a runtime bug. The relevant check is source review plus the contributor's terminal proof and CI for the current head.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Next step before merge

  • [P2] No repair lane is needed because no review finding or contributor-facing blocker remains.

Security
Cleared: The diff only adds a Vitest test file and does not alter runtime code, dependencies, workflows, secrets handling, lockfiles, or package metadata.

Review details

Best possible solution:

Merge the additive test coverage after ordinary maintainer review, leaving the current config validation runtime unchanged.

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

Not applicable; this PR adds test coverage rather than reporting a runtime bug. The relevant check is source review plus the contributor's terminal proof and CI for the current head.

Is this the best way to solve the issue?

Yes; direct Vitest coverage around the existing exported testing helper is a narrow maintainable solution and avoids changing config runtime behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 389c355bcf3f.

Label changes

Label justifications:

  • P3: This is a low-blast-radius test-only config coverage PR with no runtime, dependency, workflow, package, or config-surface changes.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal proof for a direct mapper behavior probe plus targeted test, formatting, and whitespace checks.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof for a direct mapper behavior probe plus targeted test, formatting, and whitespace checks.
Evidence reviewed

PR surface:

Tests +436. Total +436 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 436 0 +436
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 436 0 +436

What I checked:

  • Repository policy read: Root AGENTS.md was read fully in two ranges and applied; no scoped AGENTS.md owns src/config, and the only maintainer note found was Telegram-specific. (AGENTS.md:1, 389c355bcf3f)
  • PR surface: Live PR metadata shows one added file, src/config/validation.test.ts, with 436 additions, no deletions, head b2cd0534c3bb..., and clean mergeability. (src/config/validation.test.ts:1, b2cd0534c3bb)
  • Added direct tests: The new test file imports the existing testing helper and covers the mapper directly across path/message mapping, numeric hints, allowed values, bindings-specific unions, and malformed null/undefined inputs. (src/config/validation.test.ts:9, b2cd0534c3bb)
  • Current mapper contract: Current main maps unknown Zod-like issues through toIssueRecord, path formatting, numeric-bound enrichment, allowed-value summaries, and bindings-specific union extraction before returning ConfigValidationIssue. (src/config/validation.ts:823, 389c355bcf3f)
  • Runtime caller checked: validateConfigObjectRaw maps every Zod parse issue through mapZodIssueToConfigIssue, so direct mapper coverage protects the config validation path without changing runtime behavior. (src/config/validation.ts:1057, 389c355bcf3f)
  • Adjacent coverage checked: Current main already has integration-style allowed-values and numeric-bound coverage in validation.allowed-values.test.ts; this PR adds broader direct unit coverage rather than duplicating a production path. (src/config/validation.allowed-values.test.ts:30, 389c355bcf3f)

Likely related people:

  • amknight: GitHub path history for src/config/validation.ts shows several recent June 2026 config validation warning changes by this author. (role: recent config validation contributor; confidence: medium; commits: e68db3a1b8cd, 57b66b2ec8e8, 90e72a67a328; files: src/config/validation.ts)
  • tanshanshan: Merged numeric-bound hint work touched src/config/validation.ts and src/config/validation.allowed-values.test.ts, which this PR now tests directly. (role: feature-history author; confidence: high; commits: 8a8f9dc8cb77; files: src/config/validation.ts, src/config/validation.allowed-values.test.ts)
  • Hollychou924: Merged union sub-issue behavior touched the mapper and adjacent allowed-values tests related to this PR's bindings-union cases. (role: feature-history author; confidence: high; commits: ab49afcd2760; files: src/config/validation.ts, src/config/validation.allowed-values.test.ts)
  • gumadeiras: GitHub path history shows the allowed-values display and issue-formatting baseline in config validation history, which this PR covers with direct unit cases. (role: feature-history author; confidence: medium; commits: f26853f14c9b; files: src/config/validation.ts, src/config/validation.allowed-values.test.ts, src/config/issue-format.ts)
  • vincentkoc: GitHub PR metadata shows this person merged the numeric-bound hint PR that contributes to the tested mapper behavior. (role: merger/reviewer of related config work; confidence: medium; commits: 8a8f9dc8cb77; files: src/config/validation.ts, src/config/validation.allowed-values.test.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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 29, 2026
@solodmd

solodmd commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 29, 2026
@vincentkoc
vincentkoc merged commit d21ddd3 into openclaw:main Jun 29, 2026
142 of 153 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 30, 2026
…7736)

* test(config): add unit tests for mapZodIssueToConfigIssue

* fix: remove unused ts-expect-error directive
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
…7736)

* test(config): add unit tests for mapZodIssueToConfigIssue

* fix: remove unused ts-expect-error directive
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
…7736)

* test(config): add unit tests for mapZodIssueToConfigIssue

* fix: remove unused ts-expect-error directive
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
…7736)

* test(config): add unit tests for mapZodIssueToConfigIssue

* fix: remove unused ts-expect-error directive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants