feat: implement format resolution for JSON Schema with built-in and custom resolvers#6
Merged
frontegg-david merged 5 commits intomainfrom Apr 8, 2026
Merged
Conversation
WalkthroughAdds format-resolution feature (built-in and custom resolvers) with integration into tool generation, exports/types for resolvers, extensive documentation and tests, minor coverage annotations, CI/script tweaks, and removal of CHANGELOG contents. No runtime API breaking changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Generator as OpenAPIToolGenerator
participant Schema as SchemaBuilder
participant Resolver as FormatResolverService
User->>Generator: generateTools(options)
activate Generator
Generator->>Schema: build input/output schemas for operation
activate Schema
Schema-->>Generator: { inputSchema, outputSchema }
deactivate Schema
alt options.resolveFormats == true or custom resolvers provided
Generator->>Resolver: resolveSchemaFormats(inputSchema, resolvers)
activate Resolver
Resolver->>Resolver: traverse properties, items, allOf/anyOf/oneOf, not, additionalProperties
Resolver-->>Generator: enriched inputSchema
deactivate Resolver
Generator->>Resolver: resolveSchemaFormats(outputSchema, resolvers)
activate Resolver
Resolver->>Resolver: traverse and enrich output schema
Resolver-->>Generator: enriched outputSchema
deactivate Resolver
end
Generator-->>User: return tools (schemas resolved when applied)
deactivate Generator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
CLAUDE.md (1)
9-29: Add language specifiers to fenced code blocks.The ASCII diagram blocks at lines 9-29 and 93-106 are missing language specifiers. Adding
textorplaintextwould satisfy markdown linting and improve editor syntax highlighting.📝 Proposed fix
-``` +```text OpenAPI Spec (JSON/YAML/URL/File)-``` +```text LoadOptions (factory methods)Also applies to: 93-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 9 - 29, The fenced ASCII diagram blocks in CLAUDE.md are missing language specifiers; update each triple-backtick fence that wraps the OpenAPI diagram and the LoadOptions diagram to include a language like "text" or "plaintext" (e.g., change ``` to ```text) so Markdown linters and editors correctly highlight them; ensure you modify both diagram blocks (the one containing "OpenAPI Spec (JSON/YAML/URL/File) ... McpOpenAPITool" and the one containing "LoadOptions (factory methods)") to use the same specifier.src/__tests__/generator.spec.ts (1)
431-441: Test name and comment are inconsistent.The test is named
'should load from relative file path'but the comment on line 435 says "Use absolute path directly". The test actually uses an absolute path fromos.tmpdir(). Consider renaming to clarify what's being tested, or adjust the test to use an actual relative path.📝 Suggested rename
- it('should load from relative file path', async () => { + it('should load from absolute file path in tmpdir', async () => { const tmpFile = path.join(os.tmpdir(), `test-spec-${Date.now()}.json`); fs.writeFileSync(tmpFile, JSON.stringify(minimalSpec)); try { - // Use absolute path directly (tests both branches of path.isAbsolute ternary) + // Uses absolute path from os.tmpdir() const generator = await OpenAPIToolGenerator.fromFile(tmpFile);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/generator.spec.ts` around lines 431 - 441, The test title and inline comment disagree: the spec creates an absolute tmpFile and calls OpenAPIToolGenerator.fromFile(tmpFile) (exercising the path.isAbsolute branch), so either rename the test from 'should load from relative file path' to 'should load from absolute file path' or change tmpFile to a true relative path (e.g., write the JSON into a file under process.cwd() and pass a relative path such as path.relative(process.cwd(), tmpFile)) and update the comment accordingly; modify the test name or tmpFile creation and comment to keep them consistent with the behavior around OpenAPIToolGenerator.fromFile and the path.isAbsolute branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@CLAUDE.md`:
- Around line 9-29: The fenced ASCII diagram blocks in CLAUDE.md are missing
language specifiers; update each triple-backtick fence that wraps the OpenAPI
diagram and the LoadOptions diagram to include a language like "text" or
"plaintext" (e.g., change ``` to ```text) so Markdown linters and editors
correctly highlight them; ensure you modify both diagram blocks (the one
containing "OpenAPI Spec (JSON/YAML/URL/File) ... McpOpenAPITool" and the one
containing "LoadOptions (factory methods)") to use the same specifier.
In `@src/__tests__/generator.spec.ts`:
- Around line 431-441: The test title and inline comment disagree: the spec
creates an absolute tmpFile and calls OpenAPIToolGenerator.fromFile(tmpFile)
(exercising the path.isAbsolute branch), so either rename the test from 'should
load from relative file path' to 'should load from absolute file path' or change
tmpFile to a true relative path (e.g., write the JSON into a file under
process.cwd() and pass a relative path such as path.relative(process.cwd(),
tmpFile)) and update the comment accordingly; modify the test name or tmpFile
creation and comment to keep them consistent with the behavior around
OpenAPIToolGenerator.fromFile and the path.isAbsolute branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e04039dd-05ed-47e4-bf79-561dbc7f87bb
📒 Files selected for processing (15)
CLAUDE.mddocs/FORMAT_RESOLUTION.mdpackage.jsonsrc/__tests__/format-resolver.spec.tssrc/__tests__/generator.spec.tssrc/__tests__/integration.spec.tssrc/__tests__/schema-builder.spec.tssrc/__tests__/security-resolver.spec.tssrc/format-resolver.tssrc/generator.tssrc/index.tssrc/parameter-resolver.tssrc/schema-builder.tssrc/security-resolver.tssrc/types.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/push.yml (1)
48-62: Coverage summary step is well-structured.The
if: always()condition appropriately ensures coverage reporting even on test failures, and the file existence check handles edge cases where the coverage file wasn't generated.One minor observation: if the coverage file is missing, the step silently completes. Consider adding an
elsebranch to note when coverage isn't available, which could help diagnose CI issues:💡 Optional: Add feedback when coverage is unavailable
" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY + else + echo "⚠️ Coverage summary not available" >> $GITHUB_STEP_SUMMARY fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push.yml around lines 48 - 62, The Coverage summary step currently skips output when coverage/coverage-summary.json is missing; add an else branch to the file existence check inside the "Coverage summary" run block to append a clear message to $GITHUB_STEP_SUMMARY (including the current matrix.node-version context) indicating coverage was not generated or is unavailable, so CI logs show explicit feedback when the coverage file is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/push.yml:
- Around line 48-62: The Coverage summary step currently skips output when
coverage/coverage-summary.json is missing; add an else branch to the file
existence check inside the "Coverage summary" run block to append a clear
message to $GITHUB_STEP_SUMMARY (including the current matrix.node-version
context) indicating coverage was not generated or is unavailable, so CI logs
show explicit feedback when the coverage file is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 615130e7-a8bb-4743-ad33-f5db3c6481a4
📒 Files selected for processing (5)
.github/workflows/push.ymlCHANGELOG.mdCLAUDE.mdREADME.mdpackage.json
💤 Files with no reviewable changes (1)
- CHANGELOG.md
✅ Files skipped from review due to trivial changes (1)
- package.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 1-130: CLAUDE.md is placed at the repository root which violates
the project's "All docs live in `docs/` folder" convention; move the file into
the docs directory (rename/move CLAUDE.md → docs/CLAUDE.md), then update any
references to it (e.g., README.md or other docs) to point to docs/CLAUDE.md and
ensure README links use the required absolute GitHub URL format described in the
README Links section.
- Around line 126-128: The fenced code block containing the URL in CLAUDE.md is
missing a language specifier; update that fenced block (the triple-backtick
block that wraps
"https://github.com/agentfront/mcp-from-openapi/blob/main/docs/<file>.md") to
include a language tag such as text (i.e., replace the opening ``` with ```text)
so markdown linters recognize the language.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf2c554c-8b79-49ea-a799-b93dd051120d
📒 Files selected for processing (6)
.github/workflows/cherry-pick-prompt.yml.github/workflows/create-release-branch.yml.github/workflows/publish-release.yml.github/workflows/push.ymlCLAUDE.mdsrc/__tests__/generator.spec.ts
✅ Files skipped from review due to trivial changes (4)
- .github/workflows/create-release-branch.yml
- .github/workflows/cherry-pick-prompt.yml
- .github/workflows/publish-release.yml
- src/tests/generator.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/push.yml
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 by CodeRabbit
New Features
Documentation
Tests
Chores