test(config): add unit tests for mapZodIssueToConfigIssue#97736
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 5:41 AM ET / 09:41 UTC. Summary 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 follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest 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 changesLabel justifications:
Evidence reviewedPR surface: Tests +436. Total +436 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…7736) * test(config): add unit tests for mapZodIssueToConfigIssue * fix: remove unused ts-expect-error directive
…7736) * test(config): add unit tests for mapZodIssueToConfigIssue * fix: remove unused ts-expect-error directive
…7736) * test(config): add unit tests for mapZodIssueToConfigIssue * fix: remove unused ts-expect-error directive
…7736) * test(config): add unit tests for mapZodIssueToConfigIssue * fix: remove unused ts-expect-error directive
What Problem This Solves
The
validation.tsmodule provides config validation for OpenClaw runtime settings, including themapZodIssueToConfigIssuefunction that transforms raw Zod validation issues into user-facingConfigValidationIssueobjects. 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
mapZodIssueToConfigIssueto verify:expected one ofpatterns, invalid_union branches)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:
Targeted test:
Formatting:
Whitespace: