Support for v2 schemas - #82
Conversation
SummaryThis PR adds Swagger 2.0 (OpenAPI 2.0) support to Zenko by normalizing v2 specs into an OpenAPI 3 shape before code generation. It introduces a Changes
DiagramsOAS2 spec flow through generation (sequence)sequenceDiagram
participant User
participant Zenko
participant Normalizer as normalize-oas2.ts
participant Generator as existing OAS3 generator
User->>Zenko: generate(spec, { schemaVersion: "auto" })
Zenko->>Zenko: resolveSchemaVersion(spec)
alt spec is Swagger 2.0
Zenko->>Normalizer: normalizeOas2ToOas3(spec)
Normalizer-->>Zenko: OpenAPI 3 shaped spec
else spec is OpenAPI 3
Zenko->>Zenko: use spec as-is
end
Zenko->>Generator: generate from normalized spec
Generator-->>User: generated Zod schemas & types
Walkthrough by kanri |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Reviewed by Cursor Bugbot for commit ead7e48. Configure here.
![kanri-san[bot]](https://avatars.githubusercontent.com/in/4254147?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issues
1. The single-file CLI path (`zenko `) does not pass `schemaVers...
Location: packages/zenko/src/cli.ts:109
Severity: warning
The single-file CLI path (zenko <input> <output>) does not pass schemaVersion to generateSingle, even though generateSingle accepts it and the config-file path forwards it (cli.ts:239). There is also no --schema-version flag in parseArgs/printHelp, unlike the existing --strict-dates/--strict-numeric flags. Auto-detection still covers the default case, but users invoking Zenko directly cannot force oas2/oas3 normalization. Add --schema-version <auto|oas2|oas3> to parseArgs and printHelp, and forward parsed.schemaVersion in the single-file generateSingle call.
Verdict
Status: COMMENTED
Solid, well-tested PR: the normalization design (structuredClone isolation, circular-ref guard, correct path-level override semantics, reference-preserving passthrough for OAS3) is sound. No merge blockers, but fix the warnings before or soon after merge: verbatim-copied OAS2 securityDefinitions break basic/oauth2 schemes (C1), unresolved #/parameters|#/responses refs and non-JSON request bodies are silently dropped (C3, C4), the single-file CLI can't set schemaVersion (C2), and the two highest-value test gaps are the CLI config-entry wiring and the parameter merge-override semantics (C5, C6). Info items C7-C9 are optional hardening and doc wording.
3 findings below min severity (warning) were omitted from posting.
Inline comments: 5
Reviewed by kanri | 5 new inline comments
| components.schemas = { | ||
| ...(cloned.definitions as JsonObject | undefined), | ||
| ...(components.schemas as JsonObject | undefined), | ||
| } |
There was a problem hiding this comment.
WARNING
OAS2 securityDefinitions are copied verbatim into components.securitySchemes, but the OAS2 and OAS3 scheme shapes differ. Downstream (zenko.ts:412-423) only emits scheme when type === "http" and flows when type === "oauth2", so: (1) an OAS2 type: "basic" scheme is emitted as type: "basic" with no scheme instead of OAS3's type: "http", scheme: "basic"; (2) an OAS2 oauth2 scheme's flow/authorizationUrl/tokenUrl/scopes fields are never mapped to OAS3's flows object, so the generated output contains flows: undefined. The blockscout fixture only uses apiKey, so tests don't catch this. Convert basic → { type: "http", scheme: "basic" } and map OAS2 oauth2 flows (implicit/password/application/accessCode → implicit/password/clientCredentials/authorizationCode with authorizationUrl/tokenUrl/scopes) before assigning components.securitySchemes.
Fix with AI
Verify the issue still exists before applying this fix.
OAS2 `securityDefinitions` are copied verbatim into `components.securitySchemes`, but the OAS2 and OAS3 scheme shapes differ. Downstream (`zenko.ts:412-423`) only emits `scheme` when `type === "http"` and `flows` when `type === "oauth2"`, so: (1) an OAS2 `type: "basic"` scheme is emitted as `type: "basic"` with no scheme instead of OAS3's `type: "http", scheme: "basic"`; (2) an OAS2 `oauth2` scheme's `flow`/`authorizationUrl`/`tokenUrl`/`scopes` fields are never mapped to OAS3's `flows` object, so the generated output contains `flows: undefined`. The blockscout fixture only uses `apiKey`, so tests don't catch this. Convert `basic` → `{ type: "http", scheme: "basic" }` and map OAS2 oauth2 flows (`implicit`/`password`/`application`/`accessCode` → `implicit`/`password`/`clientCredentials`/`authorizationCode` with `authorizationUrl`/`tokenUrl`/`scopes`) before assigning `components.securitySchemes`.
In packages/zenko/src/utils/normalize-oas2.ts, before assigning components.securitySchemes in normalizeOas2ToOas3, convert OAS2 security scheme shapes to OAS3: map { type: "basic" } to { type: "http", scheme: "basic" }, and for { type: "oauth2", flow, authorizationUrl, tokenUrl, scopes } build the OAS3 flows object (implicit→implicit, password→password, application→clientCredentials, accessCode→authorizationCode, each with the relevant urls and scopes). Add tests in schema-version-oas2.test.ts covering basic and oauth2 conversion.
| seen = new Set<string>() | ||
| ): unknown { | ||
| if (!isPlainObject(value) || typeof value.$ref !== "string") return value | ||
| if (!value.$ref.startsWith(prefix) || seen.has(value.$ref)) return value |
There was a problem hiding this comment.
WARNING
Unresolved #/parameters/X and #/responses/X refs are returned unchanged and then silently dropped downstream: a leftover { $ref: "#/parameters/Foo" } is looked up in spec.components?.parameters by resolveParameter, but the normalizer deleted top-level parameters without populating components.parameters, so the lookup returns undefined and the parameter is skipped; a leftover #/responses/Foo ref yields a contentless response. A typo'd or missing ref in a real-world Swagger 2.0 spec produces a generated client that silently omits a required query/header/path parameter or a response type, with no error or warning. Per AGENTS.md's descriptive-errors guidance, throw (or collect and report) when a ref matches the prefix but has no target, e.g. throw new Error(\Swagger 2.0 normalization: unresolved reference "${value.$ref}"`)`.
Fix with AI
Verify the issue still exists before applying this fix.
Unresolved `#/parameters/X` and `#/responses/X` refs are returned unchanged and then silently dropped downstream: a leftover `{ $ref: "#/parameters/Foo" }` is looked up in `spec.components?.parameters` by `resolveParameter`, but the normalizer deleted top-level `parameters` without populating `components.parameters`, so the lookup returns undefined and the parameter is skipped; a leftover `#/responses/Foo` ref yields a contentless response. A typo'd or missing ref in a real-world Swagger 2.0 spec produces a generated client that silently omits a required query/header/path parameter or a response type, with no error or warning. Per AGENTS.md's descriptive-errors guidance, throw (or collect and report) when a ref matches the prefix but has no target, e.g. `throw new Error(\`Swagger 2.0 normalization: unresolved reference "${value.$ref}"\`)`.
In packages/zenko/src/utils/normalize-oas2.ts resolveOas2Reference: when value.$ref starts with the given prefix but definitions[name] is undefined, throw a descriptive error naming the unresolved reference and whether a top-level parameters or responses entry was expected. Add a test asserting the throw for a missing #/parameters ref.
| operation.requestBody = { | ||
| description: bodyParam.description, | ||
| required: bodyParam.required ?? false, | ||
| content: { |
There was a problem hiding this comment.
WARNING
A converted body param is attached under pickMediaType(consumes), which can be a non-JSON media type (application/xml, text/csv, application/json; charset=utf-8, …). Downstream getRequestBodySchema (operation-parser.ts) only checks application/json, multipart/form-data, and application/x-www-form-urlencoded, so for a Swagger 2.0 operation that consumes only XML/CSV/text the generated operation silently gets no request type at all — the entire request body vanishes with no warning. The response side is handled acceptably downstream, so only the request-body side needs fixing: attach the schema under application/json when the picked media type isn't one the generator can consume, and/or console.warn about the fallback.
Fix with AI
Verify the issue still exists before applying this fix.
A converted body param is attached under `pickMediaType(consumes)`, which can be a non-JSON media type (`application/xml`, `text/csv`, `application/json; charset=utf-8`, …). Downstream `getRequestBodySchema` (operation-parser.ts) only checks `application/json`, `multipart/form-data`, and `application/x-www-form-urlencoded`, so for a Swagger 2.0 operation that consumes only XML/CSV/text the generated operation silently gets no request type at all — the entire request body vanishes with no warning. The response side is handled acceptably downstream, so only the request-body side needs fixing: attach the schema under `application/json` when the picked media type isn't one the generator can consume, and/or `console.warn` about the fallback.
In packages/zenko/src/utils/normalize-oas2.ts normalizeOperation, when building operation.requestBody from a body param: if pickMediaType(consumes) is not one of application/json, multipart/form-data, or application/x-www-form-urlencoded, warn (console.warn with the operationId and media type) and attach the schema under application/json instead so downstream parsing finds it. Add a test with consumes: ["application/xml"] asserting the request body survives generation.
| typesConfig, | ||
| operationIds: entry.operationIds, | ||
| openEnums: entry.openEnums, | ||
| schemaVersion: entry.schemaVersion, |
There was a problem hiding this comment.
WARNING
The schemaVersion config-entry plumbing has no end-to-end CLI test. Every other per-entry option in CliConfigEntry is exercised via cli.test.ts with a JSON config file (operationIds, types.operationTypeSuffix, openEnums in all three forms, dateTimeOffset), but schemaVersion has zero mentions in cli.test.ts, so a regression in the runFromConfig → generateSingle → generate wiring would be invisible. Add a CLI test with a config entry using schemaVersion: "oas3" against the blockscout fixture asserting no OAS2 normalization artifacts, and one with "auto" asserting OAS2-specific output (e.g. v1Counters).
Fix with AI
Verify the issue still exists before applying this fix.
The `schemaVersion` config-entry plumbing has no end-to-end CLI test. Every other per-entry option in `CliConfigEntry` is exercised via `cli.test.ts` with a JSON config file (`operationIds`, `types.operationTypeSuffix`, `openEnums` in all three forms, `dateTimeOffset`), but `schemaVersion` has zero mentions in `cli.test.ts`, so a regression in the `runFromConfig` → `generateSingle` → `generate` wiring would be invisible. Add a CLI test with a config entry using `schemaVersion: "oas3"` against the blockscout fixture asserting no OAS2 normalization artifacts, and one with `"auto"` asserting OAS2-specific output (e.g. `v1Counters`).
In packages/zenko/src/__tests__/cli.test.ts add a test that writes a JSON config with a schema entry { input: blockscoutYamlPath, output, schemaVersion: "auto" } (and a second with "oas3"), runs the CLI with --config, and asserts the generated file contains (or, for oas3, does not contain) the OAS2 normalization artifacts such as `v1Counters` / `// Generated Zod Schemas`.
| function mergeOas2Parameters( | ||
| inherited: unknown[], | ||
| operation: unknown[], | ||
| definitions: JsonObject |
There was a problem hiding this comment.
WARNING
The path-level vs operation-level parameter merge order is load-bearing but untested: mergeOas2Parameters iterates [...inherited, ...operation] into a Map keyed by in:name, so operation entries override path-level ones (the correct Swagger 2.0 rule). Every existing test has parameters only at one level, so a refactor reversing the spread would silently flip override behavior with no failing test. Add a test with a path-level { name: "X-Token", in: "header", required: false } overridden by an operation-level required: true entry (and a path-level body overridden by an operation-level body) asserting a single merged parameter with the operation's values.
Fix with AI
Verify the issue still exists before applying this fix.
The path-level vs operation-level parameter merge order is load-bearing but untested: `mergeOas2Parameters` iterates `[...inherited, ...operation]` into a Map keyed by `in:name`, so operation entries override path-level ones (the correct Swagger 2.0 rule). Every existing test has parameters only at one level, so a refactor reversing the spread would silently flip override behavior with no failing test. Add a test with a path-level `{ name: "X-Token", in: "header", required: false }` overridden by an operation-level `required: true` entry (and a path-level body overridden by an operation-level body) asserting a single merged parameter with the operation's values.
In packages/zenko/src/__tests__/schema-version-oas2.test.ts add a test where a path item declares a header parameter (and/or body parameter) and the operation re-declares the same name+in with different required/schema values; assert normalizeOas2ToOas3 output contains exactly one entry with the operation-level values.
|
Status: COMMENTED All prior findings (C1, C3, C4, C5, C6) verified as fixed with tests; recent changes are in-scope follow-ups with no new issues. The effective status was recalculated from current findings and prior Kanri threads: COMMENTED. No inline comments on the diff. Walkthrough summary/changes preserved from an earlier completed run and may not reflect the latest push. Verdict by kanri |
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughZenko adds automatic and explicit Swagger 2/OpenAPI 3 schema-version handling, converts Swagger 2 documents into OpenAPI 3-shaped specs, exposes CLI configuration, and adds Blockscout specification generation and tests. ChangesSwagger 2 support and Blockscout generation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Config
participant CLI
participant generateWithMetadata
participant normalizeSpecForSchemaVersion
participant BlockscoutSpec
Config->>CLI: provide schemaVersion
CLI->>generateWithMetadata: forward schemaVersion and spec
generateWithMetadata->>normalizeSpecForSchemaVersion: normalize Swagger 2 input
normalizeSpecForSchemaVersion->>BlockscoutSpec: convert definitions and operations
BlockscoutSpec-->>generateWithMetadata: return OAS3-shaped specification
generateWithMetadata-->>CLI: generate schemas and types
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Convert securityDefinitions to OAS3 shapes, throw on unresolved refs, fall back unsupported request body media types to JSON, add --schema-version CLI flag, and expand test coverage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/zenko/src/__tests__/schema-version-oas2.test.ts (2)
206-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
as anyin the new tests.Use an explicit operation shape or narrow an
unknownvalue so strict TypeScript checking can catch malformedrequestBodyandparametersstructures.As per coding guidelines, use
unknowninstead ofanywhere possible.Also applies to: 244-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zenko/src/__tests__/schema-version-oas2.test.ts` at line 206, Replace the `as any` casts in the test’s operation lookups with an explicit operation type or by narrowing the value from `unknown`, including the corresponding occurrence near the other referenced line. Ensure strict TypeScript checking validates the `requestBody` and `parameters` structures without introducing `any`.Source: Coding guidelines
177-218: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the complete formData conversion contract.
These tests should also assert
operation.parametersis empty after conversion. The operation-level override test should additionally assert thatmultipart/form-datais absent; otherwise stale OAS2 parameters or the global media type could remain while the test still passes.Based on the normalization contract, formData should be moved into
requestBodycontent and removed from operation parameters.Suggested assertions
expect(operation.requestBody.content["multipart/form-data"]).toBeUndefined() + expect(operation.parameters).toEqual([]) ... + expect( + operation.requestBody.content["multipart/form-data"] + ).toBeUndefined() + expect(operation.parameters).toEqual([])Also applies to: 220-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zenko/src/__tests__/schema-version-oas2.test.ts` around lines 177 - 218, Update the formData conversion tests around the current consumes override and operation-level override cases to assert that the normalized operation’s parameters array is empty. In the operation-level override test, also assert multipart/form-data is absent from requestBody.content, ensuring formData is fully moved into requestBody content and removed from operation parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/zenko/src/__tests__/schema-version-oas2.test.ts`:
- Line 206: Replace the `as any` casts in the test’s operation lookups with an
explicit operation type or by narrowing the value from `unknown`, including the
corresponding occurrence near the other referenced line. Ensure strict
TypeScript checking validates the `requestBody` and `parameters` structures
without introducing `any`.
- Around line 177-218: Update the formData conversion tests around the current
consumes override and operation-level override cases to assert that the
normalized operation’s parameters array is empty. In the operation-level
override test, also assert multipart/form-data is absent from
requestBody.content, ensuring formData is fully moved into requestBody content
and removed from operation parameters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85cc2224-2445-4f87-93e0-352e2681e958
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
packages/examples/package.jsonpackages/zenko/src/__tests__/schema-version-oas2.test.tspackages/zenko/src/utils/normalize-oas2.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zenko/src/utils/normalize-oas2.ts
1 participant
What this PR does
This PR adds Swagger 2.0 (OpenAPI 2.0) support to Zenko by normalizing v2 specs into an OpenAPI 3 shape before code generation. It introduces a
schemaVersionoption (auto/oas2/oas3) across the public API, CLI, and config schema, adds a dedicated normalization utility, and validates the behavior with a Blockscout Swagger 2.0 fixture and tests.Summary by CodeRabbit
New Features
schemaVersionsetting (auto,oas2, oroas3) to generation configurations.Tests
Note
Medium Risk
Touches core spec ingestion and generation paths; incorrect normalization could change generated types for v2 APIs, though OAS3 specs are unchanged when
schemaVersionisautooroas3.Overview
Adds Swagger 2.0 (OpenAPI v2) support by normalizing specs to an OpenAPI 3-shaped document before the existing generator runs.
A new
schemaVersionoption (auto|oas2|oas3) is wired throughgenerate, the CLI, andzenko-config.schema.json, andSchemaVersionis exported from the package.normalize-oas2handles definitions →components.schemas, responseschema→content, body/formData parameters →requestBody, shared parameter/response$refs, path-level parameters, and#/definitions/→#/components/schemas/refs.OpenAPISpecis extended to accept v2 fields (swagger,definitions,securityDefinitions, etc.). Validation uses a Blockscout Stats API Swagger 2.0 fixture in@zenko/specs, with tests and examples codegen forblockscout.gen.ts. Examples also bump axios / undici and zenko to0.3.0-beta.4.Reviewed by Cursor Bugbot for commit ead7e48. Configure here.