fix: update oasdiff to v1.14.0 and migrate from oasdiff/kin-openapi to getkin/kin-openapi#1228
Conversation
…o getkin/kin-openapi Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/c0e1cb22-e1cc-4ad1-a06b-e58f8deb479a Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
…oasdiff v1.14.0 compatibility Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/c0e1cb22-e1cc-4ad1-a06b-e58f8deb479a Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
| @@ -0,0 +1,127 @@ | |||
| cel.dev/expr v0.24.0 h1:56OvJKSH3hDGL0ml5uSxZmz3/3Pq4tJ+fb1unVLAFcY= | |||
Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/5fced335-cabe-4abd-8ea4-91b700fec381 Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/6b19d61d-16e1-427c-90be-23ea97d85a1f Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/6b19d61d-16e1-427c-90be-23ea97d85a1f Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/44ea7958-f26f-4959-a88e-d74966dfc9bc Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
…rmat changes Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/2dad8bfe-ecbd-467c-a9fc-186f6c06d9f3 Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
…incorrectly added in previous commit Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/521d59a0-16c2-4643-826c-cbe80bdca3f9 Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
| { | ||
| ID: "response-write-only-property-enum-value-added", | ||
| Text: "added the new 'DUBLIN_IRL' enum value to the '/items/dataProcessRegion/region' response write-only property", | ||
| Text: "added the new `DUBLIN_IRL` enum value to the `/items/dataProcessRegion/region` response write-only property for the response status `200`", |
There was a problem hiding this comment.
@copilot the lint test if failing. Could you fix it? logs: https://github.com/mongodb/openapi/actions/runs/24454623724/job/71452311418?pr=1228
There was a problem hiding this comment.
Fixed in cd3d20f. The lll linter was flagging 8 lines exceeding 150 characters:
enum.golines 9, 22, 48: wrapped long comment examples across two linesenum_test.golines 22, 29, 185, 241 andfield_test.goline 192: added//nolint:lll //Test stringannotations, matching the existing pattern already used in those files
…test.go Agent-Logs-Url: https://github.com/mongodb/openapi/sessions/33781a4c-0055-4b2d-83fe-351d96eea656 Co-authored-by: andreaangiolillo <5663078+andreaangiolillo@users.noreply.github.com>
| // squashPlaceholder is used internally during squash text construction to mark substituted values. | ||
| // It must not appear in any oasdiff message text. The final text replaces this with a single quote. | ||
| squashPlaceholder = "\x01" |
There was a problem hiding this comment.
@copilot Why did you introduced this placeholder? what are the advantages and disadvantages of using it? why this was not defined before?
There was a problem hiding this comment.
The placeholder (\x01, ASCII SOH) was introduced because replaceOnlyFirstOccurrence needs two-phase text construction:
- Phase 1: It replaces each backtick-quoted value in the template with
\x01<value>\x01(e.g.`DUBLIN_IRL`→\x01DUBLIN_IRL\x01) - Phase 2: After all values are substituted, a final
strings.ReplaceAllconverts every\x01→'to produce the output format (e.g.'DUBLIN_IRL')
Why not use "" (empty string)?
Before oasdiff v1.14.0, the input messages used single-quoted values (e.g. 'DUBLIN_IRL'). The old placeholder was "", which simply wrapped the replacement value in empty strings — harmless because single quotes couldn't appear inside another single-quoted value.
With v1.14.0, the input now uses backticks (e.g. `DUBLIN_IRL`). If we still used "" as the placeholder during Phase 1, any replacement value that itself contains a backtick (e.g. a path like `/items/region`) would accidentally match the identifierRegex again in subsequent iterations of ReplaceAllStringFunc, causing double-substitution.
\x01 (SOH) is safe because it's a control character that oasdiff never includes in its message text, so it can't create a false regex match during Phase 1.
Fixes the build failures caused by upgrading
github.com/oasdiff/oasdifffrom v1.13.1 to v1.14.0 (see dependabot PR #1208).oasdiff v1.14.0introduced two breaking changes:github.com/oasdiff/kin-openapifork to the upstreamgithub.com/getkin/kin-openapi, causing type incompatibilities across ~55 Go files intools/cliand 6 files intools/mcp-server.GetUncolorizedText()output from single-quoted values ('value') to backtick-quoted values (`value`), and added the HTTP status code as an extra argument to several response-related message templates.Changes:
tools/cli/go.mod: Replacegithub.com/oasdiff/kin-openapi v0.136.10withgithub.com/getkin/kin-openapi v0.135.0(the version used by oasdiff v1.14.0) and bump oasdiff to v1.14.0tools/mcp-server/go.mod: Replacegithub.com/oasdiff/kin-openapi v0.136.10withgithub.com/getkin/kin-openapi v0.135.0as a direct dependencygithub.com/oasdiff/kin-openapi/openapi3withgithub.com/getkin/kin-openapi/openapi3and applygciimport formatting across bothtools/cliandtools/mcp-servertools/cli/go.sumandtools/mcp-server/go.sum: Updated viago mod tidy.gitignore: Addedtools/go.work.sumto prevent the auto-generated workspace checksum file from being accidentally committedsquash.go: UpdatedidentifierRegexfrom'([^']*)'to`([^`]*)`to match backtick-quoted values; replaced the""placeholder with\x01(ASCII SOH) to avoid false regex matches during the two-phase text construction (phase 1 marks substitutions with\x01<value>\x01; phase 2 converts\x01→'for the final output format)enum.go: UpdatedexpectedNumberOfValuesforresponse-property-enum-value-added/removedandresponse-write-only-property-enum-value-added(2→3), andresponse-mediatype-enum-value-removed(1→2, squashIdx 0→1) to handle the new status code and newly-quoted media type arguments; wrapped long comment examples across two lines to satisfy thellllinter (150-character limit)field.go: Added the newexpectedNumberOfValuesparameter tosquashFieldChanged— set to1for request handlers (which have a single backtick-quoted value: the property name) and2for response handlers (which now include the HTTP status code as a second backtick-quoted value in oasdiff v1.14.0)squash_test.go,enum_test.go,field_test.go): Updated input texts to use backtick format; expected outputs continue to use single-quote format (squash output) with status codes now included; added//nolint:lll //Test stringannotations to test string literals that inherently exceed the 150-character line limit, matching the existing pattern used elsewhere in those filesnew-api-version,same-api-version,new-api-preview-version,new-upcoming-version,rename-api-version) using the updated CLI binary with--run-date 2025-06-15, matching the fixed date used by the e2e testJira ticket: CLOUDP-#
Checklist
Changes to Spectral
Further comments
Validation:
go build ./...✅ (bothtools/cliandtools/mcp-server)go test ./internal/...✅ (all 25 test packages pass)gciimport formatting applied to all changed Go files ✅llllint violations resolved inenum.go,enum_test.go, andfield_test.go✅--run-date 2025-06-15flag ✅TestChangelog/Generate_Changelog_with_Upcoming_API_Versione2e test passes ✅