feat(schema): validate value constraints in --validate#71
Conversation
Client-side --validate checked required fields and enums but silently ignored minimum, maximum, and multipleOf. The Node struct never modeled those keywords, so encoding/json dropped them on unmarshal and out-of-range values like width=-1 passed local validation and only failed server-side. Add the three keywords to Node and a recursive ValidateNumericConstraints that checks them against payload values, descending into nested objects and array items the same way ValidateEnums does. Wire it into the validation pipeline after ValidateEnums.
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
This PR extends the CLI’s optional --validate behavior to enforce JSON Schema numeric constraints (minimum, maximum, multipleOf) client-side, preventing out-of-range numeric payloads from reaching the API.
Changes:
- Add
Minimum,Maximum, andMultipleOffields tointernal/schema.Nodeso schema numeric bounds survive JSON unmarshalling. - Implement recursive numeric validation (
ValidateNumericConstraints) for nested objects and array items, mirroring existing enum validation traversal. - Invoke numeric validation in the request validation pipeline (
internal/api/client.go) when--validateis enabled, and add unit tests covering common bound violations and nested/array cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/schema/schema.go |
Models numeric bounds in Node and adds recursive numeric constraint validation helpers. |
internal/schema/schema_test.go |
Adds test coverage for min/max/multipleOf enforcement across scalar, nested, and array-item values. |
internal/api/client.go |
Wires numeric constraint validation into the --validate pipeline after enum validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extend --validate to the rest of the JSON Schema value vocabulary the API uses: exclusiveMinimum and exclusiveMaximum, minLength, maxLength, pattern, and format (uri, uuid, date-time, email, matching the SDK ajv-formats), plus minItems, maxItems, and uniqueItems. Rename ValidateNumericConstraints to ValidateConstraints and dispatch by value type. Non-finite numbers (NaN and Inf) on a bounded field are now rejected explicitly, which the prior numeric check would silently pass. This addresses the Copilot review note on checkNumericBounds. All checks recurse through nested objects and array items, reporting the first violation with its field path. Still opt-in via --validate.
felixsanz
changed the title
What
--validatewas a partial validator: it enforcedrequiredand enums but silently dropped every value constraint, because theNodestruct never modeled those keywords. This PR makes it validate the full JSON Schema value vocabulary the API uses, matching the SDK's AJV-based validation.Coverage
minimum,maximum,multipleOf,exclusiveMinimum,exclusiveMaximum. Non-finite values (NaN,±Inf) on a bounded field are rejected up front, since they cannot satisfy any finite bound.minLength,maxLength(counted in Unicode code points),pattern(RE2, and a pattern Go cannot compile is left to the server), andformatfor the named formats the schemas actually use (uri,uuid,date-time,email), matching the SDK'sajv-formats. Unrecognised formats pass.minItems,maxItems,uniqueItems.All checks recurse through nested objects and array items, reporting the first violation with its field path. Still opt-in via
--validate.Notes
ValidateNumericConstraintsis renamed toValidateConstraintsand dispatches by value type.checkNumericBounds:NaNandInfare now handled explicitly rather than slipping through the comparisons.Tests
24 cases covering every keyword: violations, valid values, nested objects, array items, float multiples, NaN/Inf, exclusive bounds, each format, and the unknown-format pass-through.