fix: resolve corpus testing bugs in generator and converter#317
Merged
fix: resolve corpus testing bugs in generator and converter#317
Conversation
Fix three bugs discovered during MCP plugin corpus testing: **Generator: server wrapper type collision** - Schema names ending in "Request" (e.g., CreatePetRequest) collided with generated server wrapper structs, causing Go compilation failures - Added resolveWrapperName with suffix cascade: Request → Input → Req → numeric - Added defensive iteration limit (1000) on numeric fallback **Converter: formData parameters not converted (2.0→3.x)** - formData parameters were passed through unconverted during OAS 2.0→3.x - Added convertOAS2FormDataToRequestBody: builds requestBody with object schema - Handles file uploads (multipart/form-data), url-encoded, and mixed params - Transfers validation properties (enum, min/max, pattern, etc.) to schema - Converts array-type Items to Schema via convertOAS2ItemsToSchema - Sets Required on RequestBody when any formData param is required - Respects operation-level consumes override **Converter: incomplete 3.0→2.0 downconversion** - Parameters with composite schemas (allOf/oneOf/anyOf) lost their type field - Added inferTypeFromSchema to walk composite schemas for concrete types - Header $refs (#/components/headers/*) were left unresolved in OAS 2.0 output - Added resolveHeaderRef to inline component headers during downconversion - Uses DeepCopy for all header assignments to prevent source document mutation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request adds converter fixes (OAS2 formData→requestBody conversion, OAS3→OAS2 parameter type inference fallback, and inlining component header refs) and generator changes to avoid wrapper name collisions in generated server request types. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #317 +/- ##
==========================================
- Coverage 84.50% 84.42% -0.09%
==========================================
Files 182 182
Lines 26226 26410 +184
==========================================
+ Hits 22163 22296 +133
- Misses 2755 2797 +42
- Partials 1308 1317 +9
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@converter/helpers.go`:
- Around line 159-178: In resolveHeaderRef, add a defensive nil check for
c.sourceHeaders at the top (e.g., if c.sourceHeaders == nil) and handle it the
same way unresolved names are handled: call c.addIssueWithContext(result, path,
fmt.Sprintf("Unresolved header ref: %s", ref), "Header not found in
components.headers") and return nil; this prevents a nil-map panic if the method
is called without the upstream guard while keeping existing behavior around
addIssueWithContext, inlined := header.DeepCopy(), and clearing inlined.Ref
intact.
In `@converter/oas2_to_oas3.go`:
- Around line 278-316: The code sets propSchema.Format only in the default
switch branch, dropping param.Format for array and file cases; update the logic
in the parameter-to-schema conversion (in the block building propSchema for each
param in convertOAS2ItemsToSchema usage) to copy param.Format into propSchema
unconditionally (e.g., set propSchema.Format = param.Format after the switch) or
at minimum add propSchema.Format = param.Format into the "array" branch so
schema.Properties[param.Name] preserves any provided format; keep existing
special-case settings for "file" (Format = "binary") intact by overriding after
the unconditional copy if needed.
- Around line 256-341: The convertOAS2FormDataToRequestBody function currently
returns a RequestBody without Description and only treats "multipart/form-data"
as an override; update it to set RequestBody.Description (e.g., "Form data
parameters") and change the consumes fallback logic to detect any multipart
media type (e.g., use a prefix check like strings.HasPrefix(ct, "multipart/"))
when iterating the slice returned by c.getConsumes(src, doc), so multipart/mixed
and other multipart types are honored; keep existing behavior for
application/x-www-form-urlencoded when no multipart is found.
In `@converter/oas3_to_oas2_test.go`:
- Around line 711-768: Add a test case to
TestConvertOAS3ParameterToOAS2_TypeFallback exercising the AnyOf branch used by
inferTypeFromSchema: create a parameter whose Schema has AnyOf with at least one
concrete Type (e.g., "string" and "integer"), call convertOAS3ParameterToOAS2,
assert the converted.Type matches the expected concrete type (consistent with
the OneOf test), and assert result.Issues is non-empty to maintain existing
expectations.
In `@generator/security_gen_shared.go`:
- Around line 60-80: The final fallback in resolveWrapperName can return a name
that was already detected as colliding; update resolveWrapperName to handle
exhaustion explicitly by emitting a clear failure instead of silently returning
methodName+"Request": after exhausting wrapperSuffixes and numeric attempts
(maxAttempts), either panic or return an error/log with context (including
methodName and the attempted suffixes/maxAttempts) so generation fails loudly;
modify the function around resolveWrapperName, wrapperSuffixes, schemaTypes and
maxAttempts to replace the silent return with an explicit panic or
processLogger/error return that includes methodName and that no unique wrapper
name could be found.
- Add defensive nil check for sourceHeaders in resolveHeaderRef - Transfer Format field in array-type formData parameter conversion - Detect any multipart/* content type in consumes (not just multipart/form-data) - Add anyOf test case for type inference branch coverage - Return novel numeric suffix on exhaustion instead of known-colliding name Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CreatePetRequest) collided with generated server wrapper structs. AddedresolveWrapperNamewith suffix cascade (Request → Input → Req → numeric fallback) to disambiguate.in: "formData"parameters were passed through unconverted. AddedconvertOAS2FormDataToRequestBodythat builds a properrequestBodywith object schema, handles file uploads, transfers validation properties, and respects operation-levelconsumes.allOf/oneOf/anyOf) lost theirtypefield, and#/components/headers/*refs were left unresolved. Added type inference from composite schemas and header ref inlining with deep copy.All three bugs were discovered during MCP plugin corpus testing (discord-openapi.json, petstore 2.0, nws 3.0).
Test plan
TestServerWrapperTypeCollision— OAS3 spec with colliding schema nameTestResolveWrapperName— 6 table-driven subtests (no collision, nil map, cascade, numeric)TestConvertOAS2FormDataToRequestBody— 4 subtests (url-encoded, multipart, mixed, consumes override)TestConvertOAS3ParameterToOAS2_TypeFallback— 3 subtests (allOf, oneOf, default to string)TestConvertOAS3ToOAS2_InlineHeaderRefs— end-to-end with component header resolutionmake checkpasses (8078 tests)🤖 Generated with Claude Code