openapi3: aggregate independent validation errors via EnableMultiError#1185
Conversation
fenollp
left a comment
There was a problem hiding this comment.
Leaf validators (Schema, Info, Server, individual Parameter, ...) still fail fast. Continuing past a leaf error can hit a nil deref because leaves often guard then dereference, so the change is deliberately scoped to the nil-guarded "for each independent sibling" sites where collect-and-continue is provably safe.
I find this very dubious. I don't see any proof? It is true some validation code expects previous code to be valid and this PR does not make any note of this principle except in this short passage, and that requires hard work. But eg. license.go is definitely safe to add to this PR. Do you plan on converting more files in the future or does this stop here?
cmd/validate flag? This PR does not expose the new option through cmd/validate. Happy to add --multi as a follow-up commit on this PR (it would mirror the existing --defaults / --examples / --patterns flags), or leave it for a separate change. Preference?
Yes please add a flag there.
I've added a few comments. Notably on some of the continues introduced (all are implied).
I understand these are intended so that all the MultiErrors returned actually are errors that make sense (and are not just the result of diverging logic from previous errors).
In fact I am wondering if this should be part of the expressed documentation, or if this is just a temporary goal:
Aggregation happens at container fan-out points (the document root, Paths,
PathItem, Operation, Components, Responses, Webhooks). Validation of a
single leaf element (for example, a single Schema) still stops at its first
error.
Because to me, and I guess to others, this sentence is hard to make sense of. Plus, I don't see why we'd not return multiple errors for a single schema?
| require.GreaterOrEqual(t, len(me), 2, | ||
| "expected at least one error per affected section") |
There was a problem hiding this comment.
Why not check "exactly equal 2" ?
There was a problem hiding this comment.
Tightened from require.GreaterOrEqual to require.Equal.
Today Validate returns the first error it encounters and stops, so a spec with several independent defects only reports one at a time: fix, re-run, see the next, repeat. With the new EnableMultiError ValidationOption, container validators aggregate independent problems and return them together as a flat MultiError. Aggregation happens at the container fan-out points where it is safe to continue past an error: T, Paths, PathItem, Operation, Components, Responses. Leaf validators (Schema, Info, Server, etc.) still fail fast, since continuing past a leaf error can hit a nil deref. The result shape is flat: each top-level MultiError entry is one fully-wrapped chain (SectionValidationError -> PathValidationError -> OperationValidationError -> typed leaf). Consumers iterating the MultiError get one entry per problem; consumers using errors.As / errors.Is work unchanged thanks to MultiError.As / MultiError.Is. By default Validate returns the first error encountered, byte-identical to today: the existing test suite passes through the same code path it always did.
…ators CI greps for "return .*validateExtensions" in openapi3/ and asserts the count equals the number of Extensions fields. The previous commit moved those calls to "me.emit(validateExtensions(...))" + "return me.result()", breaking the count. Add a finalize helper on errCollector that emits its argument and returns the accumulated result, so each container's trailing line is a single "return me.finalize(validateExtensions(ctx, X.Extensions))" that both reads cleanly and matches the CI fence regex.
759dff0 to
b4f96c6
Compare
|
Thanks for the thorough review. A. Leaf validators staying fail-fast. Fair pushback. "Leaves are unsafe" overstates it; what's actually unsafe is the Converted in this PR:
Skipped Concrete result on a spec with empty B. C. Docstring scope. Rewrote: dropped the explicit fan-out list (would have gone stale), named the transition explicitly, explained why some leaves are not yet converted, and replaced the generic Inline replies on each thread below. |
Top-level: - cmd/validate: add --multi flag (mirrors --defaults / --examples / --patterns) - Convert simple leaves to aggregate independent errors: License, Info, Server (singular + plural), Tag (singular + plural), ExternalDocs. Each is sequential independent checks with no guard-then-deref pattern, structurally identical to the existing container validators. Schema, Parameter, MediaType have internal cross-field dependencies and are left for follow-up PRs after per-method analysis. - Rewrite EnableMultiError docstring: drop the explicit fan-out list (would go stale), name the transition explicitly, explain why some leaves are not yet converted, and replace the generic errors.As / errors.Is note with a concrete how-to. Inline: - validate_multi_error_test.go: use require.ErrorContains for chain checks; tighten GreaterOrEqual to Equal where the count is exact. - response.go: route Responses.Validate's empty-responses error through me.emit for consistency with the rest of the multi-error path. - components.go, paths.go, openapi3.go: add comments at the error-path continue statements explaining why we skip descending past a malformed parent key (component name, path key, nil webhook). - error_collector.go (new): move the errCollector type and methods out of validation_options.go so the options file stays focused on options. - openapi3.go: revert to the wrap-variable reassignment pattern per section block; keep wrapSection as the factory.
d7b6285 to
15dc18e
Compare
Parameter.Validate and MediaType.Validate already use newSchemaValueError
for the singular `example:` field, but the plural `examples:` map loop
returned a bare fmt.Errorf wrapping the underlying *SchemaError. That
meant errors.As(err, &SchemaValueError) didn't match for plural examples,
so downstream consumers fell back to a generic catch-all instead of
recognising the example-violates-schema cluster.
Wrap the plural-loop result in newSchemaValueError("example", ...) too,
preserving the example key inside the wrap so the rendered message still
names the offending example. Aligns the plural path with the singular
path; both now surface as *SchemaValueError to errors.As consumers.
Existing example_validation_test.go assertions and the apis_guru golden
fixtures gain the "invalid example: " prefix the wrap adds.
Bump kin to oasdiff/kin-openapi @ e14b38a (the open getkin/kin-openapi#1185 branch with multi-error + simple-leaf conversions + plural-examples typing fix). Lets oasdiff exercise the changes end-to-end before the kin PR merges; the replace will drop and the dep will return to upstream master once #1185 lands. Wire-up: - internal/validate.go: pass openapi3.EnableMultiError() to Validate so simple-leaf and container errors aggregate rather than fail-fast. - internal/validate.go (fieldLoc): when the dotted Field name on a cluster error (e.g. "info.version") doesn't match a Fields key, fall back to the suffix after the last dot ("version"). Kin's Origin.Fields is keyed by the leaf name as it appears in the YAML mapping, while cluster errors use a dotted form for rule-ID disambiguation; without this fallback findings under aggregated leaves resolved to the parent object's Origin.Key instead of the precise field. Concrete result on /tmp/multi-problem.yaml (empty info.title, empty info.version, missing operation.responses): three findings at the exact field lines and columns where the value is missing, instead of one finding at the info section start. Tests updated: - missing-required-info.yaml fixture: title now set, only version left empty so single-finding tests stay single. - Test_ValidateCmd_LineColumnFromOrigin: now asserts line 4 col 3 (the version line in the fixture). - Test_ValidateCmd_DocRootFieldHasLineColumn (renamed from ...HasNoLineColumn): asserts doc-root findings carry line:1 col:1 now that T.Origin is populated (kin #1184). - Test_ValidateCmd_TextFormatLocation: location string updated to 4:3.
…t the parent For plural Examples entries on Parameter/MediaType, the Validate failure used the parent struct's Origin, which deep-links to the parameter or media-type start (e.g. the `in: query` line) rather than the offending example value. Build the SchemaValueError's Origin from the Example struct's per-field origin (`Origin.Fields["value"]`) when available, falling back to the example's struct origin, then to the parent. No message-format change: existing string assertions continue to hold, only the resolved line/column tightens to the example's `value:` field.
|
Two follow-up commits since the review, both surfaced by end-to-end testing in oasdiff:
The plural loop now also wraps in
No message format change in this one. The Cause string still matches what existed; only the line/column on the resolved Source tightens to the example's |
|
Heads-up on the next typing step. We tested the coverage and ~78% of findings now resolve to typed clusters with line/column; the remaining ~22% fall through to a catchall and trace back to five untyped kin sites:
Plan: ship those as a separate "type the remaining bare-error sites" PR after this one merges, to keep this PR focused on aggregation. If anything in the list jumps out as needing a different cluster shape or name, easier to discuss now than re-litigate post-merge. |
The empty-responses path used 'return me.result()' which short-circuited before validateExtensions could run, so extension errors would never aggregate with the empty-responses finding under multi mode. Drop the early return and let control fall through to the trailing finalize() call; the for loop is a no-op on an empty Responses, so the only behaviour change is that validateExtensions now runs. Spotted by @fenollp in PR review.
Adds TestResponses_Validate_EmptyAndExtensionAggregate which constructs an empty Responses with a non-x- sibling on Extensions and asserts both findings surface under EnableMultiError. Reverting the previous commit's fall-through edit causes this test to fail (one leaf instead of two).
kin getkin/kin-openapi#1185 (multi-error aggregation + simple-leaf conversions + plural-examples typing + precise example-value Origin) merged today as e56c2c7. Drop the temporary replace directive that pointed at the oasdiff fork branch and bump to a pseudo-version of upstream master that includes the merge.
Five fmt.Errorf call sites in the openapi3 validators returned bare errors that consumers could only inspect by substring matching. Wrap each in a typed cluster so errors.As / errors.Is consumers can dispatch on the error kind and pull structured fields out: - PathParameterRequiredError (parameter.go) Param + Origin - DuplicateOperationIDError (paths.go) OperationID + Endpoint1/Endpoint2 - ExtraSiblingFieldsError (extension.go, refs) Fields []string - SchemaTypeError (schema.go) Type + Origin Error() strings are unchanged byte-for-byte (existing string-comparison consumers, apis_guru golden fixtures, validation_error_test assertions all pass without edits). The benefit is purely additive: callers gain the typed-cluster dispatch they already use for the RequiredFieldError / FieldVersionMismatchError / etc. clusters added in getkin#1183. Promised in getkin#1185's heads-up comment after running oasdiff validate across ~500 fixtures and finding these as the remaining 22% of findings that fell through to a downstream catchall.
* feat(validate): new subcommand wrapping kin-openapi's Validate() Adds 'oasdiff validate <spec>' that flags per-RFC OpenAPI / JSON Schema spec violations. Wraps kin-openapi's openapi3.T.Validate() walker and dispatches each typed error to a stable kebab-case rule ID via errors.As against kin's RequiredFieldError / FieldVersionMismatchError clusters (introduced in kin-openapi#1166). Output format follows the changelog command's shape (id/text/level/ source) so a single CI script can parse both. Two formats: text (default, with a header summary line + per-finding block matching ApiChange.MultiLineError style) and yaml (-f yaml). Exit code 0 on no findings, 1 if any. 10 tests cover: happy path, typed dispatch (info-version-required, openapi-required, identifier-field-for-3-1-plus, webhooks-field-for- 3-1-plus, const-field-for-3-1-plus through 3 levels of %w wrapping), untyped fallback (kin-validation-error for sites kin hasn't migrated to a typed cluster yet), text + yaml format shapes, load-failure exit code distinct from validation-finding exit code. DO NOT MERGE - depends on kin-openapi#1166 landing and a kin release shipping. The go.mod replace directive pins kin to the fork's rfc branch. Cleanup before merge: 1. Remove the replace directive 2. Bump kin-openapi to the released version 3. go mod tidy * fix(validate): use validateCmd const to silence unused-const lint * feat(validate): surface line + column from kin's typed Origin Loader now starts with IncludeOrigin=true. The Finding struct gains Line/Column fields, populated from the kin cluster errors' Origin.Key when available (info, license, server, schema). Document-root fields (openapi, webhooks, jsonSchemaDialect) have nil Origin and emit findings without line/column (yaml omitempty). Text format renders <file>:<line>:<column> when origin is set, plain <file> otherwise — matches the changelog command's location shape. Three new tests pin: line/column populated for info-version-required, both fields absent for openapi-required (doc-root), text format includes :line:column when available. * chore: drop accidentally-committed .DS_Store * fix(validate): indent every line of multi-line finding messages kin errors like *SchemaError embed newlines in their Error() output (Schema and Value dumps). Without indenting continuation lines, those broke the finding's visual grouping in text format. Every \n in the message now becomes \n\t so the whole finding stays under the same tab indent. * chore: gitignore .DS_Store * chore: gitignore .DS_Store * fix(validate): leave blank lines blank when indenting multi-line text Previous indent logic prefixed every \n with \t, including blank lines — leaving stray tabs on otherwise-empty separator lines and a trailing \t at the end of the message. Switch to a line-by-line walk that skips blanks and trims trailing whitespace. * feat(validate): dispatch SchemaValueError cluster (example/default-violates-schema) kin#1166 added a *SchemaValueError cluster wrapping the *SchemaError returned by VisitJSON when a schema's example or default doesn't satisfy its own constraints. Add a third dispatch arm so findings of that shape get a specific rule ID derived from the cluster's ValueKind: 'example' → 'example-violates-schema', 'default' → 'default-violates-schema'. The cluster also carries the parameter/media-type/schema's Origin, so Line+Column now populate for these findings as well. * deps: bump kin-openapi to include long-tail RequiredFieldError leaves * deps: switch kin-openapi from oasdiff fork to upstream master All four typed-validation-error PRs now merged into upstream getkin/kin-openapi: - #1162 (openapi3conv 3.0→3.x canonicalization) - #1166 (ValidationError framework) - #1170 (long-tail RequiredFieldError leaves) - #1180 (combined long-tail PR collapsing #1171-#1179) Drop the replace directive; pin to upstream-master pseudo-version e8145f8f4d2b (the #1180 merge commit). When getkin tags a release containing this work, the require line will move from pseudo-version to a stable v0.138.x. * feat(validate): dispatch the 8 remaining kin clusters to typed rule IDs After bumping kin to upstream master post-#1180, eight cluster types became available that the validate dispatch was still routing to the 'kin-validation-error' catchall. Wire each: - *PathParametersError -> path-parameters-mismatch (static, since the cluster carries Path/Method/Missing not a single derivable field) - *MutuallyExclusiveFieldsError -> <f1>-<f2>-mutually-exclusive - *ForbiddenFieldError -> <field>-forbidden - *ServerURLTemplateError -> server-url-template-invalid (static, the cluster carries only the offending URL) - *EitherFieldRequiredError -> <f1>-or-<f2>-required - *SchemaBothFormsExclusive -> <field>-both-forms-exclusive - *ExactlyOneFieldError -> <f1>-or-<f2>-exactly-one - *SingleEntryContentError -> <subject>-content-single-entry - *WebhookNilError -> webhook-nil keyOriginForKinError extended in the same way so line/column flow through for clusters that carry Origin (all except WebhookNilError, whose offending key sits on the document root that the loader doesn't track per-key). Updated the previously-flipping Test_ValidateCmd_UntypedKinErrorFallback (server-url-mismatched-braces is now typed) and added a test for the user-reported PathParametersError case. * deps: temporarily point kin-openapi at oasdiff fork with DisableTimestamps Switches the kin-openapi dep to the oasdiff fork's feat/yaml-disable-timestamps branch via a replace directive. This is the same branch underlying getkin/kin-openapi#1181 (still in review). It includes the typed validation errors from #1166 and #1180 plus the DisableTimestamps integration with oasdiff/yaml v0.1.0, which prevents YAML 1.1 implicit-timestamp resolution from mangling date-shaped map keys in real-world specs. Verified with the canonical case: unicourt.com/1.0.0/openapi.yaml (originally cited in invopop/yaml#10 four years ago) now loads cleanly and produces actual schema-validation findings rather than the time.Time map-key explosion. REMOVE BEFORE MERGE: replace this directive with a bump to the released kin version once #1181 lands and a kin tag is cut. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(validate): align Finding shape with locked JSON schema Restructures the validate output to match the design's locked JSON schema (mirrors oasdiff changelog --format json with a few additions). Finding struct now exposes: - source as an object {file, line, column} (was: string source + top-level line/column) - section: which top-level doc section the finding belongs to ("info", "paths", "components", "webhooks", "servers", "security", "tags", or "" for unscoped doc-root findings). Determined per-cluster with a light Field-prefix check. - fingerprint: stable 12-char sha256-prefix derived from "{id}:{operation}:{path}:{args}" — mirrors the existing formatters/changes.go:computeFingerprint scheme so the Pro PR-comment can partition findings into new/pre-existing/fixed via set membership on fingerprint across the base/revision spec pair. - comment, operation, path: present but omitempty (Phase 1 leaves operation/path empty; extracting them from kin Origins requires walking the spec structure, deferred to Phase 2). - All fields have both yaml: and json: tags. Adds --format json support (encoder + enum value); existing yaml and text paths unchanged in behaviour. Tests: - Existing yaml-format + origin-tracking tests updated to the new source-object shape. - New JSON-format test pins the locked shape. - New fingerprint-stability test pins determinism across runs (the property Pro PR-comment partitioning depends on). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(validate): colorize text output via --color flag Adds the locked --color auto|always|never flag to oasdiff validate, matching the convention used by changelog and breaking. Text output now renders the severity in red/purple/cyan (error/warning/info, via Level.StringCond) and the rule ID in yellow, matching the established color scheme. Source path stays uncolored. Auto mode disables color when stdout is piped or redirected; the auto-detect helper lives in checker/piped_output.go and is shared across subcommands so behaviour is consistent. Exports checker.IsColorEnabled as a thin wrapper around the package-internal isColorEnabled. Lets oasdiff packages outside checker (validate, future subcommands) gate their own color logic without duplicating the auto-detect convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(validate): pin findings to the offending field line, not the enclosing object kin's Origin model carries two location handles: - Origin.Key — the start of the enclosing collection (for a license-identifier error, the `license:` key) - Origin.Fields[X] — the specific scalar field X inside that collection (for a license-identifier error, the `identifier:` line) The previous locator returned Origin.Key for every cluster, so a finding in `data/validate/license-identifier-in-3-0.yaml` was pinned to line 5 (`license:`) instead of line 7 (`identifier: MIT`). Reviewers reading the output had to scan from the parent key to find the actual offender. Reworks locationForKinError (renamed from keyOriginForKinError) to prefer Origin.Fields[Field] when the cluster carries a Field, falling back to Origin.Key when the per-field entry is missing (e.g. empty values, which kin doesn't track per-field). New fieldLoc helper centralises the lookup. Per-cluster strategy: - RequiredFieldError, FieldVersionMismatchError, ForbiddenFieldError, SchemaBothFormsExclusive — use Fields[Field] - MutuallyExclusiveFieldsError — use Fields[Field1] (either field pins to the right object) - SchemaValueError — use Fields[ValueKind] (e.g. "example", "default") - SingleEntryContentError — use Fields[Subject] - EitherFieldRequiredError, ExactlyOneFieldError — use Key (cluster fires when NONE of the fields are present, so per-field lookup wouldn't match) - PathParametersError, ServerURLTemplateError — use Key (no Field metadata) - WebhookNilError — no Origin (doc root) Adds a regression test asserting the License-identifier fixture pins to line 7:5 (the identifier line) rather than 5:3 (the license: line). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(validate): extract path + operation from kin's error chain kin wraps validation errors in nested fmt.Errorf("...: %w") layers that carry path/operation context as plain text rather than typed fields: invalid paths: invalid path /thing: invalid operation GET: <inner> These three layers (or any subset) are now stripped off the rendered message and surfaced as Finding.Operation and Finding.Path, matching the changelog command's convention of presenting them as discrete fields. Finding.Text holds the cleaned inner message. Text output gains a second header line "in API <op> <path>" rendered in green when color is enabled, mirroring changelog / breaking. Doc- root findings (info, openapi, license — no path/operation) skip the line entirely. Before: error [const-field-for-3-1-plus] at spec.yaml:18:21 invalid paths: invalid path /thing: invalid operation GET: field const is for OpenAPI >=3.1 After: error [const-field-for-3-1-plus] at spec.yaml:18:21 in API GET /thing field const is for OpenAPI >=3.1 Side benefit: Finding.Operation / Finding.Path are now populated for operation-scoped findings, which feeds into the fingerprint (per formatters/changes.go scheme: sha256(id:op:path:args)) and makes Pro PR-comment partitioning stable when the same finding moves to a new path between base and revision. Component-scoped findings (kin's "invalid components: schema X:" wrapper) are still inline in Text. Section is set correctly via sectionForKinError, but extracting the schema name into a discrete field is a separate enhancement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "feat(validate): extract path + operation from kin's error chain" This reverts commit f9bfe39. The reverted commit used regex to strip kin's "invalid paths: invalid path X: invalid operation Y:" prefixes off the rendered error message. That was always meant to be interim — message-text parsing is exactly the brittleness kin's typed-error work eliminates. kin PR getkin/kin-openapi#1183 adds typed SectionContextError / PathContextError / OperationContextError wrappers that carry the context as structured fields. Once that merges and a kin release is cut, the extraction becomes three errors.As calls with no string parsing. Carrying the regex in the interim isn't worth it: the whole feat/validate-command branch is gated on a kin release anyway, so there's no window where the regex would ship to users. Reverting now keeps the branch honest and avoids a "delete the regex" follow-up commit later. The Finding struct keeps its Operation / Path fields (added in the schema-alignment commit, not here) — they're simply unpopulated until the typed-error extraction lands. omitempty elides them from output in the meantime. Validate findings temporarily render the full wrapped message inline again, as they did before f9bfe39. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(validate): typed path/operation/section extraction from kin's error chain Now that getkin/kin-openapi #1183 is merged, validate lifts the structural scope of each finding out of the message text and into the typed fields: - Operation / Path come from PathValidationError + OperationValidationError via errors.As (replacing the reverted regex approach). - sectionForKinError now prefers SectionValidationError.Section, kin's authoritative section name, falling back to the pre-#1183 cluster heuristics only for doc-root errors that aren't section-wrapped. Adds data/validate/operation-missing-responses.yaml and a test asserting an in-operation error surfaces operation=GET, path=/things, section=paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(validate): strip the redundant context prefix from finding text Section/path/operation scope now lives in the Finding's typed fields, so the "invalid paths: invalid path X: invalid operation Y:" prefix that kin's context wrappers add to Error() was pure duplication in Text. unwrapContext peels those typed wrappers off the front of the chain so Text carries only the underlying leaf message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * validate: pin kin to PR #1185 branch for multi-error + precise locations Bump kin to oasdiff/kin-openapi @ e14b38a (the open getkin/kin-openapi#1185 branch with multi-error + simple-leaf conversions + plural-examples typing fix). Lets oasdiff exercise the changes end-to-end before the kin PR merges; the replace will drop and the dep will return to upstream master once #1185 lands. Wire-up: - internal/validate.go: pass openapi3.EnableMultiError() to Validate so simple-leaf and container errors aggregate rather than fail-fast. - internal/validate.go (fieldLoc): when the dotted Field name on a cluster error (e.g. "info.version") doesn't match a Fields key, fall back to the suffix after the last dot ("version"). Kin's Origin.Fields is keyed by the leaf name as it appears in the YAML mapping, while cluster errors use a dotted form for rule-ID disambiguation; without this fallback findings under aggregated leaves resolved to the parent object's Origin.Key instead of the precise field. Concrete result on /tmp/multi-problem.yaml (empty info.title, empty info.version, missing operation.responses): three findings at the exact field lines and columns where the value is missing, instead of one finding at the info section start. Tests updated: - missing-required-info.yaml fixture: title now set, only version left empty so single-finding tests stay single. - Test_ValidateCmd_LineColumnFromOrigin: now asserts line 4 col 3 (the version line in the fixture). - Test_ValidateCmd_DocRootFieldHasLineColumn (renamed from ...HasNoLineColumn): asserts doc-root findings carry line:1 col:1 now that T.Origin is populated (kin #1184). - Test_ValidateCmd_TextFormatLocation: location string updated to 4:3. * validate: bump kin pin to 9b85457 (example value origin fix) * validate: drop the kin fork replace; bump to kin master with #1185 kin getkin/kin-openapi#1185 (multi-error aggregation + simple-leaf conversions + plural-examples typing + precise example-value Origin) merged today as e56c2c7. Drop the temporary replace directive that pointed at the oasdiff fork branch and bump to a pseudo-version of upstream master that includes the merge. * validate: dispatch typed clusters from kin #1187 Wires oasdiff validate against the four new typed validation clusters added in getkin/kin-openapi #1187 plus the Origin tracking this branch pushed back to that PR for the duplicate-operation-id and extra-sibling-fields sites. Before this change, the four corresponding error sites all fell through to the kin-validation-error catchall and lost line:column precision. After this change each surfaces under its own stable rule ID with file:line:col when the loader tracks origins. Dispatcher updates in internal/validate.go: * ruleIDForKinError — four new errors.As cases returning the stable rule IDs path-parameter-required, duplicate-operation-id, extra-sibling-fields, schema-type-unsupported. * locationForKinError — four new cases: - PathParameterRequiredError → parameter object's Key. - SchemaTypeError → the offending type field via fieldLoc. - DuplicateOperationIDError → fieldLoc(Origin, "operationId") so the finding pins to the duplicate operationId scalar inside the second operation, not the operation block start. - ExtraSiblingFieldsError → parent object's Key (parser does not track Origin for unknown sibling field names). * sectionForKinError — PathParameterRequiredError and DuplicateOperationIDError both resolve to "paths"; the other two fall through to the existing schema/generic logic which already handles them correctly. * argsForKinError — fingerprint disambiguation for all four (Param / OperationID / joined Fields / Type). Tests + fixtures: * data/validate/{path-parameter-not-required, duplicate-operation-id, extra-sibling-fields, schema-type-unsupported}.yaml — minimal specs that each trigger exactly one of the four clusters. * internal/validate_test.go — one test per fixture asserting the stable rule ID, the kin-side error message, and that Origin resolves to a file:line:col suffix. go.mod pins to the oasdiff fork's branch via pseudo-version. Swap to the upstream tag once getkin/kin-openapi cuts a release that includes #1187. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * validate: dispatch InvalidParameterInError and SchemaPatternRegexError Wires oasdiff validate against the two additional typed clusters added to getkin/kin-openapi #1187 after corpus testing surfaced them as the dominant remaining catchall sites. Corpus result on testdata/APIs-guru-openapi-directory (~4,000 specs): catchall hits dropped from 385 to 10 after this change. The remaining 10 cluster into three untyped sites (two in security_scheme.go, one in ref.go) that are out of #1187's scope. Dispatcher updates in internal/validate.go: * ruleIDForKinError — two new errors.As cases returning the stable rule IDs parameter-in-invalid and schema-pattern-regex-invalid. * locationForKinError — two new cases pinning to the offending field via fieldLoc: parameter.in for InvalidParameterIn, schema.pattern for SchemaPatternRegex. * sectionForKinError — InvalidParameterIn → "paths". The other falls through to the existing schema-deep logic. * argsForKinError — fingerprint disambiguation by the structured field (Value / Pattern). Tests + fixtures: data/validate/{parameter-in-invalid, schema-pattern-regex-invalid}.yaml minimal specs and corresponding tests in internal/validate_test.go asserting typed rule ID, the kin-side error message, and Origin file:line:col resolution. go.mod bumped to the latest oasdiff/kin-openapi fork pseudo-version v0.0.0-20260517110407-8e7311f2c94f (corresponds to #1187 HEAD with the InvalidParameterIn + SchemaPatternRegex additions and the SchemaPatternRegex message revert that preserves Error() string). * validate: dispatch remaining 8 typed clusters from kin #1187; rename catchall to spec-validation-error Adds errors.As cases (rule ID, section, args, location) for the typed clusters in kin #1187 that didn't yet have oasdiff dispatchers: InvalidSecuritySchemeTypeError, InvalidHTTPSchemeError, UnresolvedRefError, APIKeyInInvalidError, PathMustStartWithSlashError, ConflictingPathsError, DuplicateParameterError, InvalidSerializationMethodError. Also extends unwrapContext to strip the 10 new typed context wrappers kin #1187 added (ComponentValidationError, ExternalDocsURLValidationError, HeaderFieldValidationError, MediaTypeExampleValidationError, WebhookValidationError, ParameterFieldValidationError, ParameterExampleValidationError, SecuritySchemeFlowValidationError, OAuthFlowValidationError, OAuthFlowFieldValidationError) so Finding.Text carries just the underlying typed message without the wrapper prefixes. Corpus result on testdata/APIs-guru-openapi-directory (~4,000 specs): zero catchall hits across the entire corpus. Renames the catchall rule ID from 'kin-validation-error' to 'spec-validation-error'. The old name leaked an implementation detail (kin-openapi is an oasdiff dependency users don't know about); the new name is meaningful in the user's domain (their OpenAPI spec failed validation). The constant is renamed in lockstep (kinUnknownID -> unknownValidationID). go.mod bumped to kin fork pseudo-version with the full PR. * validate: dedupe defects reported once per $ref site A defect in a shared components definition (e.g. a bad example in components/schemas/Status) is reported once by kin's components walk AND once per operation that $refs it. From the user's perspective that's one thing to fix; from the Pro PR-comment workflow's perspective it should be one approve/reject decision. dedupePreferringComponents groups findings by their defect identity (Id + Source location + Text — Text carries the args discriminator) and keeps one representative per group. When a group includes a components-rooted finding (Section == "components"), prefer it: the components-rooted version points at the definition site and has empty Operation/Path, giving a stable fingerprint across reference- graph changes (adding a 7th endpoint that $refs the schema doesn't churn the fingerprint). Covers all components/* sub-sections (schemas, parameters, headers, requestBodies, responses, examples, links, callbacks, securitySchemes, and 3.1+ pathItems) because the dedup looks at Section, not the specific bucket. Path-level shared parameters don't need handling here because kin only validates them once at the PathItem level (no per-operation re-validation). Regression test pins a fixture with components/schemas/Status that has a bad example referenced from 3 operations: produces one finding located at the components-schema definition's line:col, not three finding-sized copies pointing at $ref sites. * validate: rewrite Long help text to drop implementation leaks Dropped 'kin-openapi', 'Phase 1', 'errors.As / RequiredFieldError / FieldVersionMismatchError clusters' — internal implementation details users don't care about. Replaced with user-facing content: what the command catches, the output format options, and a clear exit-code table including 102 for load failure. * text formatter: drop trailing-tab artifacts; context-aware indent in validate MultiLineError for ApiChange, SecurityChange, ComponentChange carried a trailing "\t" on the header line (in security/component, a space-tab where "at source" never went). Visible as stray whitespace. Removed. validate's text formatter now mirrors the changelog shape when an operation context exists ("in API METHOD /path" plus a two-tab message body) and stays single-indent when it does not (doc-root findings, components-rooted findings after dedup). Continuation lines (Schema:, Value:, JSON blocks in nested-schema messages) inherit the same indent so multi-line messages stay visually grouped under the operation line. indentContinuation now takes the prefix as a parameter so the first line and continuation lines always agree. Tests updated to match the corrected strings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump kin-openapi pseudo-version Pick up the latest HEAD of the upstream fork branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: track kin-openapi upstream master directly Drop the oasdiff/kin-openapi fork replace. The fork's master is 109 commits behind upstream and all the typed-cluster work (#1166, #1180, #1187) landed upstream directly via Pierre's merges, so the fork is no longer carrying anything we need. Bumps the kin pseudo-version to 8381bfc (the #1187 merge commit, 2026-05-21 14:53 UTC). Phase 1 of the validate subcommand can now errors.As against the full typed-cluster surface (clusters from #1166 + #1180 + #1187 + the 24 new clusters/wrappers from #1187). Switch back to a tagged kin release version before merging this branch to main, once Pierre cuts one containing all three PRs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * formatters: update text-changelog golden after trailing-tab removal Commit f04519e (2026-05-17) dropped the trailing "\t" artifact from ComponentChange.MultiLineError (and the parallel ApiChange and SecurityChange formatters), but didn't update the only golden test that exercises this path. The branch has been failing TestTextFormatter_RenderChangelog ever since. Update the expected string to match the now-clean output — `error\t[change_id]\n` instead of `error\t[change_id] \t\n`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * build: pin kin-openapi to released v0.139.0 #894 was wired against a master pseudo-version while the typed-validation PRs were unreleased. Pierre tagged v0.139.0 (2026-05-23), which includes them, so pin the real release. Build + full test suite green. * docs: document the validate command Add docs/VALIDATE.md (usage, output formats, flags, exit codes, rule IDs) and list `validate` in the README Commands section (eight commands now). Mirrors the existing per-command doc pattern. * validate: document git-ref as a supported input; tidy help and comment Note git refs (e.g. main:openapi.yaml) alongside file/URL/stdin in the validate Long help and docs/VALIDATE.md, since the loader supports them. Also tidies the Long help and the catchall-ID comment. * validate: render output through the formatters package; add githubactions Move validate's output off the inline yaml/json/text marshaling and onto the shared formatters, the same path changelog/breaking/flatten use: a new Formatter.RenderValidate method renders a plain formatters.Finding, looked up by format via formatters.Lookup. text, yaml, and json implement it; the command's -f options come from SupportedFormatsByContentType(OutputValidate) so the advertised and implemented format sets stay in sync. Add githubactions support so the upcoming oasdiff-action validate wrapper can emit one CI annotation per finding (anchored to file/line/column) inline on the pull request, plus error_count/warning_count/info_count step outputs. Validate and changelog now share formatters.ComputeFingerprint so a downstream tool can match findings across spec versions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * validate: classify finding severities and add --fail-on Findings were all reported as errors. Classify them by impact via severityForKinError (errors.As dispatch, default ERR so any unrecognised or newly-typed kin cluster stays an error): WARN version-portability (3.1 field in an older doc), $ref siblings that are silently ignored, conflicting paths, duplicate parameters, and a default value that violates its schema INFO an example that violates its schema (the contract itself is valid) duplicate-operation-id stays ERR: it violates the spec's uniqueness MUST and breaks code generators. Add -o, --fail-on (default ERR), mirroring changelog: the command exits 1 only when a finding is at or above the threshold, so warnings and info print but don't fail CI by default. Severity flows through to githubactions annotations (::warning / ::notice / ::error) and the text summary counts. The mapping is a hardcoded dispatch; a future per-rule override (like changelog's --severity-levels) can sit on top of it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: escape GitHub Actions annotation messages and properties The githubactions formatter only escaped newlines, so a message containing '%' (decoded by GitHub) or a property value containing ':' or ',' (e.g. a git-ref source path like main:openapi.yaml) would produce a malformed annotation. Add escapeData (% / CR / LF) and escapeProperty (+ ':' / ',') per GitHub's workflow-command rules, escaping '%' first so the introduced %0A/%0D sequences are not double-escaped. Applied to both the changelog (RenderChangelog) and validate (RenderValidate) output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(validate): add Feedback section Invite users to report issues with a [validate] title prefix, mirroring the OpenAPI 3.1 page. Pairs with the catchall spec-validation-error rule ID, which we want users to report. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * validate: reject --color with a non-text format; clarify docs wording --color only affects the text output (yaml/json/githubactions ignore it), so pairing it with a non-text format now errors instead of being silently dropped, matching the changelog command. Also soften the VALIDATE.md intro: findings are spec-defined violations classified by severity, not "hard violations only", now that some are warnings/info. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: cover validate formatter rendering and more kin clusters Codecov patch coverage was low because CI runs `go test ./...` without -coverpkg, so the new formatters code (RenderValidate, Findings helpers) got no credit from the internal tests that exercise it, and validate.go's typed dispatch had several kin clusters with no fixture. - formatters-package tests for RenderValidate (text/yaml/json), the not-implemented fallback, and the Findings helpers (GetLevelCount, HasLevelOrHigher, indentContinuation). - fixtures + tests for previously-untested clusters: conflicting-paths, duplicate-parameter, path-must-start-with-slash, and the three invalid security-scheme clusters; plus a multi-cluster test over openapi-test3 (EitherFieldRequired / ExactlyOneField via joinFieldsForRuleID, OAuth-flow required field). - a checker test for the exported IsColorEnabled. No production code change. validate.go coverage 82.8% -> 88.6%; the new formatters validate rendering is now fully covered in-package. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Why
Today
Validatereturns the first error it encounters and stops. A spec with several independent defects only surfaces one at a time: fix it, re-run, see the next, repeat. That serializes work that is fundamentally parallel and hides the true shape of "is my spec valid?" until the last fix lands.What
A new
ValidationOption,EnableMultiError(), makes container validators aggregate independent problems and return them together as a flatMultiError. With the option off, behavior is unchanged.Aggregation happens at the container fan-out points where continuing past an error is safe:
T.Validate(across the doc sections: info, paths, components, security, servers, tags, externalDocs, webhooks, jsonSchemaDialect)Paths.Validate(across paths)PathItem.Validate(across HTTP methods)Operation.Validate(across parameters, requestBody, responses, externalDocs)Components.Validate(across schemas, parameters, request bodies, etc.)Responses.Validate(across status codes)Leaf validators (
Schema,Info,Server, individualParameter, ...) still fail fast. Continuing past a leaf error can hit a nil deref because leaves often guard then dereference, so the change is deliberately scoped to the nil-guarded "for each independent sibling" sites where collect-and-continue is provably safe.Result shape: flat
Each top-level
MultiErrorentry is one fully-wrapped chain, e.g.Consumers iterating the
MultiErrorget one entry per problem; the section/path/operation context travels with each leaf. Consumers usingerrors.As/errors.Iswork unchanged in both modes, thanks toMultiError.As/MultiError.Iswalking into contained errors.This is achieved by an
emitWrapped(wrap, err)helper that distributes the wrap over each leaf when the input is itself aMultiError, so per-section context is attached per-problem rather than wrapping a whole nestedMultiError.Backward compatibility
return err-on-first-error code path they always did. The full existing test suite passes through that path unchanged..github/docs/openapi3.txtdiff is purely additive (EnableMultiErroradded, nothing removed); thedocs.shbreaking-change check returns 0.errors.As/errors.Iskeep working for any consumer that uses them. Only direct type assertions on the whole returned error (e.g.err.(*RequiredFieldError)) would break in multi-error mode on a multi-defect spec — and that idiom is fragile regardless;errors.Asis the documented alternative.Tests
New
validate_multi_error_test.gocovers:MultiError, first-error semantics preservedMultiError(of one),errors.Aswalks throughFull suite green.
Open questions
cmd/validateflag? This PR does not expose the new option throughcmd/validate. Happy to add--multias a follow-up commit on this PR (it would mirror the existing--defaults/--examples/--patternsflags), or leave it for a separate change. Preference?