From b53c66a08407560fe12c87e8bbc78e8252c0c8be Mon Sep 17 00:00:00 2001 From: David Antoon Date: Wed, 8 Apr 2026 01:58:31 +0300 Subject: [PATCH 1/4] test: enhance error handling and loading tests for OpenAPIToolGenerator --- jest.config.js | 5 + package.json | 3 +- src/__tests__/errors.spec.ts | 7 + src/__tests__/generator.spec.ts | 243 +++++++++++++++++++++++- src/__tests__/security-resolver.spec.ts | 39 ++++ src/generator.ts | 2 +- yarn.lock | 19 +- 7 files changed, 301 insertions(+), 17 deletions(-) diff --git a/jest.config.js b/jest.config.js index 04fd0fe..1857695 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,6 +1,7 @@ module.exports = { displayName: 'mcp-from-openapi', testEnvironment: 'node', + coverageProvider: 'v8', transform: { '^.+\\.[tj]s$': [ '@swc/jest', @@ -28,11 +29,15 @@ module.exports = { }, ], }, + transformIgnorePatterns: [ + 'node_modules/(?!@apidevtools/json-schema-ref-parser)', + ], moduleFileExtensions: ['ts', 'js'], testMatch: ['**/__tests__/**/*.spec.ts', '**/*.spec.ts', '**/*.test.ts'], coverageDirectory: 'coverage', collectCoverageFrom: [ 'src/**/*.ts', + '!src/index.ts', '!src/**/*.spec.ts', '!src/**/*.test.ts', '!src/**/__tests__/**', diff --git a/package.json b/package.json index d4f093d..e3fec65 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "prepublishOnly": "yarn clean && yarn build && yarn test" }, "dependencies": { - "@apidevtools/json-schema-ref-parser": "^11.9.3", + "@apidevtools/json-schema-ref-parser": "^15.3.5", "openapi-types": "^12.1.3", "yaml": "^2.8.3" }, @@ -70,6 +70,7 @@ "@swc/helpers": "^0.5.18", "@swc/jest": "~0.2.38", "@types/jest": "^29.5.0", + "@types/json-schema": "^7.0.15", "@types/node": "^24.0.0", "esbuild": "^0.27.2", "jest": "^29.7.0", diff --git a/src/__tests__/errors.spec.ts b/src/__tests__/errors.spec.ts index 75f4036..0509577 100644 --- a/src/__tests__/errors.spec.ts +++ b/src/__tests__/errors.spec.ts @@ -97,6 +97,13 @@ describe('ValidationError', () => { expect(error.errors).toEqual(validationErrors); }); + it('should handle context without errors key', () => { + const error = new ValidationError('Validation failed', { someOther: 'data' }); + + expect(error.errors).toBeUndefined(); + expect(error.context).toEqual({ someOther: 'data' }); + }); + it('should be instance of OpenAPIToolError', () => { const error = new ValidationError('Validation failed'); diff --git a/src/__tests__/generator.spec.ts b/src/__tests__/generator.spec.ts index b5a4737..4ea8158 100644 --- a/src/__tests__/generator.spec.ts +++ b/src/__tests__/generator.spec.ts @@ -192,6 +192,36 @@ paths: await expect(OpenAPIToolGenerator.fromURL('https://example.com/api.json')).rejects.toThrow(LoadError); }); + it('should throw LoadError when YAML parse fails from URL', async () => { + mockFetch.mockResolvedValue({ + ok: true, + text: () => Promise.resolve('{ invalid yaml: [[['), + headers: new Map([['content-type', 'application/yaml']]), + }); + + await expect(OpenAPIToolGenerator.fromURL('https://example.com/api.yaml')).rejects.toThrow(LoadError); + }); + + it('should throw LoadError when JSON parse fails from URL', async () => { + mockFetch.mockResolvedValue({ + ok: true, + text: () => Promise.resolve('not valid json at all'), + headers: new Map([['content-type', 'application/json']]), + }); + + await expect(OpenAPIToolGenerator.fromURL('https://example.com/api.json')).rejects.toThrow(LoadError); + }); + + it('should wrap non-Error thrown values as LoadError', async () => { + mockFetch.mockRejectedValue('string error'); + + const promise = OpenAPIToolGenerator.fromURL('https://example.com/api.json'); + await expect(promise).rejects.toThrow(LoadError); + await expect(promise).rejects.toMatchObject({ + message: expect.stringContaining('string error'), + }); + }); + it('should handle timeout option', async () => { const jsonSpec = JSON.stringify({ openapi: '3.0.0', @@ -308,8 +338,22 @@ paths: }); describe('File Loading', () => { - // Note: File loading tests are skipped by default since they require actual fs mocking - // The fromFile method is tested through the error case which doesn't need mocking + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + + const minimalSpec = { + openapi: '3.0.0', + info: { title: 'File Test', version: '1.0.0' }, + paths: { + '/test': { + get: { + operationId: 'test', + responses: { '200': { description: 'OK' } }, + }, + }, + }, + }; it('should throw LoadError on file read error', async () => { await expect(OpenAPIToolGenerator.fromFile('/nonexistent/path/api.json')).rejects.toThrow(LoadError); @@ -320,6 +364,52 @@ paths: message: expect.stringContaining('Failed to load OpenAPI spec from file'), }); }); + + it('should load from JSON file', async () => { + const tmpFile = path.join(os.tmpdir(), `test-spec-${Date.now()}.json`); + fs.writeFileSync(tmpFile, JSON.stringify(minimalSpec)); + try { + const generator = await OpenAPIToolGenerator.fromFile(tmpFile); + expect(generator.getDocument().info.title).toBe('File Test'); + } finally { + fs.unlinkSync(tmpFile); + } + }); + + it('should load from YAML file', async () => { + const yaml = require('yaml'); + const tmpFile = path.join(os.tmpdir(), `test-spec-${Date.now()}.yaml`); + fs.writeFileSync(tmpFile, yaml.stringify(minimalSpec)); + try { + const generator = await OpenAPIToolGenerator.fromFile(tmpFile); + expect(generator.getDocument().info.title).toBe('File Test'); + } finally { + fs.unlinkSync(tmpFile); + } + }); + + it('should auto-detect JSON for unknown extension', async () => { + const tmpFile = path.join(os.tmpdir(), `test-spec-${Date.now()}.txt`); + fs.writeFileSync(tmpFile, JSON.stringify(minimalSpec)); + try { + const generator = await OpenAPIToolGenerator.fromFile(tmpFile); + expect(generator.getDocument().info.title).toBe('File Test'); + } finally { + fs.unlinkSync(tmpFile); + } + }); + + it('should fallback to YAML for unknown extension when JSON parse fails', async () => { + const yaml = require('yaml'); + const tmpFile = path.join(os.tmpdir(), `test-spec-${Date.now()}.txt`); + fs.writeFileSync(tmpFile, yaml.stringify(minimalSpec)); + try { + const generator = await OpenAPIToolGenerator.fromFile(tmpFile); + expect(generator.getDocument().info.title).toBe('File Test'); + } finally { + fs.unlinkSync(tmpFile); + } + }); }); describe('Custom Tool Naming', () => { @@ -852,6 +942,48 @@ describe('ParameterResolver', () => { expect(mapper.find((m) => m.type === 'body')).toBeDefined(); }); + it('should include body parameter description', () => { + const resolver = new ParameterResolver(); + const { inputSchema } = resolver.resolve({ + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + name: { type: 'string', description: 'The user name' }, + }, + }, + }, + }, + }, + }); + + const nameSchema = inputSchema.properties?.['name'] as any; + expect(nameSchema.description).toBe('The user name'); + }); + + it('should handle object body without properties', () => { + const resolver = new ParameterResolver(); + const { inputSchema, mapper } = resolver.resolve({ + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + }, + }, + }, + }, + }); + + // Object without properties falls through to the non-object body path + expect(inputSchema.properties).toHaveProperty('body'); + expect(mapper.find((m) => m.type === 'body')).toBeDefined(); + }); + it('should handle deprecated parameters', () => { const resolver = new ParameterResolver(); const { inputSchema, mapper } = resolver.resolve({ @@ -989,6 +1121,16 @@ describe('ParameterResolver', () => { expect(oidcSchema.description).toContain('openid, profile'); }); + it('should throw when request body has empty content', () => { + const resolver = new ParameterResolver(); + expect(() => resolver.resolve({ + requestBody: { + required: true, + content: {}, + }, + })).toThrow('No content type available in request body'); + }); + it('should use fallback content type when none match preferences', () => { const resolver = new ParameterResolver(); const { inputSchema, mapper } = resolver.resolve({ @@ -1098,6 +1240,34 @@ describe('OpenAPIToolGenerator - Additional Coverage', () => { expect(tools).toHaveLength(1); expect(tools[0].name).toBe('getUsers'); }); + + it('should include operation when filterFn returns true', async () => { + const openapi = { + openapi: '3.0.0', + info: { title: 'Test', version: '1.0.0' }, + paths: { + '/users': { + get: { + operationId: 'getUsers', + responses: { '200': { description: 'OK' } }, + }, + }, + '/admin': { + get: { + operationId: 'getAdmin', + responses: { '200': { description: 'OK' } }, + }, + }, + }, + }; + + const generator = await OpenAPIToolGenerator.fromJSON(openapi); + const tools = await generator.generateTools({ + filterFn: () => true, + }); + + expect(tools).toHaveLength(2); + }); }); describe('Tool Name Generation', () => { @@ -1817,4 +1987,73 @@ describe('SSRF Prevention - $ref resolution security', () => { expect(canRead({ url: 'not-a-valid-url' })).toBe(false); expect(canRead({ url: '' })).toBe(false); }); + + it('should disable http resolver when only file protocol is allowed', async () => { + const generator = await OpenAPIToolGenerator.fromJSON(minimalSpec, { + refResolution: { allowedProtocols: ['file'] }, + }); + await generator.generateTools(); + + const options = derefSpy.mock.calls[0][1]; + expect(options?.resolve?.http).toBe(false); + expect(options?.resolve?.external).toBe(true); + }); + + it('should throw ParseError when dereference fails', async () => { + derefSpy.mockRejectedValueOnce(new Error('Circular $ref detected')); + + const generator = await OpenAPIToolGenerator.fromJSON(minimalSpec); + await expect(generator.generateTools()).rejects.toThrow('Failed to dereference OpenAPI document'); + }); + + it('should handle security with reference object and dereference disabled', async () => { + const openapi = { + openapi: '3.0.0', + info: { title: 'Test', version: '1.0.0' }, + components: { + securitySchemes: { + refAuth: { + $ref: '#/components/securitySchemes/otherAuth', + }, + }, + }, + paths: { + '/users': { + get: { + operationId: 'getUsers', + security: [{ refAuth: [] }], + responses: { '200': { description: 'OK' } }, + }, + }, + }, + }; + + const generator = await OpenAPIToolGenerator.fromJSON(openapi, { dereference: false }); + const tool = await generator.generateTool('/users', 'get'); + + // With dereference disabled, the $ref is not resolved, so it hits the reference fallback + expect(tool.metadata.security).toBeDefined(); + expect(tool.metadata.security![0].type).toBe('http'); + }); + + it('should return empty security when no securitySchemes defined', async () => { + const openapi = { + openapi: '3.0.0', + info: { title: 'Test', version: '1.0.0' }, + paths: { + '/users': { + get: { + operationId: 'getUsers', + security: [{ apiKey: [] }], + responses: { '200': { description: 'OK' } }, + }, + }, + }, + }; + + const generator = await OpenAPIToolGenerator.fromJSON(openapi); + const tool = await generator.generateTool('/users', 'get'); + + expect(tool.metadata.security).toEqual([]); + }); }); diff --git a/src/__tests__/security-resolver.spec.ts b/src/__tests__/security-resolver.spec.ts index 764d8bf..ad9dc13 100644 --- a/src/__tests__/security-resolver.spec.ts +++ b/src/__tests__/security-resolver.spec.ts @@ -398,6 +398,45 @@ describe('SecurityResolver', () => { expect(result.headers['Auth']).toBe('vapid-auth-value'); }); + it('should return undefined for unknown HTTP scheme in default case', async () => { + const mappers: ParameterMapper[] = [ + { + inputKey: 'Auth', + key: 'Auth', + type: 'header', + security: { + scheme: 'customScheme', + type: 'http', + httpScheme: 'totally-custom-xyz', + }, + }, + ]; + + const result = await resolver.resolve(mappers, {}); + + expect(result.headers['Auth']).toBeUndefined(); + }); + + it('should return undefined when custom HTTP scheme has no matching header', async () => { + const mappers: ParameterMapper[] = [ + { + inputKey: 'Auth', + key: 'Auth', + type: 'header', + security: { + scheme: 'customScheme', + type: 'http', + httpScheme: 'vapid', + }, + }, + ]; + + // No customHeaders provided — resolveCustomHttpScheme returns undefined + const result = await resolver.resolve(mappers, {}); + + expect(result.headers['Auth']).toBeUndefined(); + }); + it('should skip when auth value is not available', async () => { const mappers: ParameterMapper[] = [ { diff --git a/src/generator.ts b/src/generator.ts index 4cb1224..b3fb050 100644 --- a/src/generator.ts +++ b/src/generator.ts @@ -1,7 +1,6 @@ import * as yaml from 'yaml'; import * as path from 'path'; import * as fs from 'fs/promises'; -import $RefParser from '@apidevtools/json-schema-ref-parser'; import type { OpenAPIDocument, LoadOptions, @@ -293,6 +292,7 @@ export class OpenAPIToolGenerator { if (this.options.dereference && !this.dereferencedDocument) { try { + const { default: $RefParser } = await import('@apidevtools/json-schema-ref-parser'); const refParserOptions = this.buildRefParserOptions(); this.dereferencedDocument = (await $RefParser.dereference( JSON.parse(JSON.stringify(this.document)), diff --git a/yarn.lock b/yarn.lock index 0987b7c..600aa96 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,14 +2,12 @@ # yarn lockfile v1 -"@apidevtools/json-schema-ref-parser@^11.9.3": - version "11.9.3" - resolved "https://registry.yarnpkg.com/@apidevtools/json-schema-ref-parser/-/json-schema-ref-parser-11.9.3.tgz#0e0c9061fc41cf03737d499a4e6a8299fdd2bfa7" - integrity sha512-60vepv88RwcJtSHrD6MjIL6Ta3SOYbgfnkHb+ppAVK+o9mXprRtulx7VlRl3lN3bbvysAfCS7WMVfhUYemB0IQ== +"@apidevtools/json-schema-ref-parser@^15.3.5": + version "15.3.5" + resolved "https://registry.yarnpkg.com/@apidevtools/json-schema-ref-parser/-/json-schema-ref-parser-15.3.5.tgz#503726178d8d792eea7b195566272aaee6837e2f" + integrity sha512-orNOYXw3hYXxxisXMldjzjBzqqTLBPbwOtHg7ovBPvfBHDue1qM9YJENZ3W2BQuS+7z4ThogMbEzEsov57Itkg== dependencies: - "@jsdevtools/ono" "^7.1.3" - "@types/json-schema" "^7.0.15" - js-yaml "^4.1.0" + js-yaml "^4.1.1" "@babel/code-frame@^7.0.0", "@babel/code-frame@^7.12.13", "@babel/code-frame@^7.27.1": version "7.27.1" @@ -686,11 +684,6 @@ "@jridgewell/resolve-uri" "^3.1.0" "@jridgewell/sourcemap-codec" "^1.4.14" -"@jsdevtools/ono@^7.1.3": - version "7.1.3" - resolved "https://registry.yarnpkg.com/@jsdevtools/ono/-/ono-7.1.3.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" - integrity sha512-4JQNk+3mVzK3xh2rqd6RB4J46qUR19azEHBneZyTZM+c456qOrbbM/5xcR8huNCCcbVt7+UmizG6GuUvPvKUYg== - "@sinclair/typebox@^0.27.8": version "0.27.8" resolved "https://registry.yarnpkg.com/@sinclair/typebox/-/typebox-0.27.8.tgz#6667fac16c436b5434a387a34dedb013198f6e6e" @@ -1896,7 +1889,7 @@ js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" -js-yaml@^4.1.0: +js-yaml@^4.1.1: version "4.1.1" resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.1.1.tgz#854c292467705b699476e1a2decc0c8a3458806b" integrity sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA== From e1a6fa6d80e1cb91f34836882a37de927c9184be Mon Sep 17 00:00:00 2001 From: David Antoon Date: Wed, 8 Apr 2026 02:08:14 +0300 Subject: [PATCH 2/4] feat: add validation for dereferenced OpenAPI documents in generator --- src/__tests__/generator.spec.ts | 33 +++++++++++++++++++++++++++++++++ src/generator.ts | 18 ++++++++++-------- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/__tests__/generator.spec.ts b/src/__tests__/generator.spec.ts index 4ea8158..2ff3132 100644 --- a/src/__tests__/generator.spec.ts +++ b/src/__tests__/generator.spec.ts @@ -1682,6 +1682,39 @@ describe('OpenAPIToolGenerator - Additional Coverage', () => { await expect(generator.generateTools()).rejects.toThrow(ParseError); }); + + it('should validate dereferenced document so $ref parameters pass validation', async () => { + const openapi = { + openapi: '3.0.0', + info: { title: 'Test', version: '1.0.0' }, + components: { + parameters: { + UserId: { + name: 'userId', + in: 'path', + required: true, + schema: { type: 'string' }, + }, + }, + }, + paths: { + '/users/{userId}': { + get: { + operationId: 'getUser', + parameters: [{ $ref: '#/components/parameters/UserId' }], + responses: { '200': { description: 'OK' } }, + }, + }, + }, + }; + + // With validate: true (default), this should pass because + // dereferencing resolves the $ref before validation runs + const generator = await OpenAPIToolGenerator.fromJSON(openapi); + const tools = await generator.generateTools(); + expect(tools).toHaveLength(1); + expect(tools[0].name).toBe('getUser'); + }); }); }); diff --git a/src/generator.ts b/src/generator.ts index b3fb050..be5d885 100644 --- a/src/generator.ts +++ b/src/generator.ts @@ -280,16 +280,9 @@ export class OpenAPIToolGenerator { } /** - * Initialize the generator (dereference if needed) + * Initialize the generator (dereference if needed, then validate) */ private async initialize(): Promise { - if (this.options.validate) { - const result = await this.validate(); - if (!result.valid) { - throw new ParseError('Invalid OpenAPI document', { errors: result.errors }); - } - } - if (this.options.dereference && !this.dereferencedDocument) { try { const { default: $RefParser } = await import('@apidevtools/json-schema-ref-parser'); @@ -305,6 +298,15 @@ export class OpenAPIToolGenerator { }); } } + + if (this.options.validate) { + const validator = new Validator(); + const documentToValidate = this.dereferencedDocument ?? this.document; + const result = await validator.validate(documentToValidate); + if (!result.valid) { + throw new ParseError('Invalid OpenAPI document', { errors: result.errors }); + } + } } /** From ab554064c01eaa70a765228c3937dbe296660e4a Mon Sep 17 00:00:00 2001 From: David Antoon Date: Wed, 8 Apr 2026 02:20:29 +0300 Subject: [PATCH 3/4] chore: update Node.js version requirement to 20.0.0 in documentation and package files --- README.md | 4 ++-- docs/getting-started.md | 2 +- package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 90be98c..0caf803 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ [![npm version](https://badge.fury.io/js/mcp-from-openapi.svg)](https://www.npmjs.com/package/mcp-from-openapi) [![License: Apache 2.0](https://img.shields.io/badge/License-Apache_2.0-yellow.svg)](https://opensource.org/license/apache-2-0) [![TypeScript](https://img.shields.io/badge/TypeScript-5.0+-blue.svg)](https://www.typescriptlang.org/) -[![Node.js](https://img.shields.io/badge/Node.js-18+-green.svg)](https://nodejs.org/) +[![Node.js](https://img.shields.io/badge/Node.js-20+-green.svg)](https://nodejs.org/) ## What This Solves @@ -154,7 +154,7 @@ function buildRequest(tool: McpOpenAPITool, input: Record) { ## Requirements -- Node.js >= 18.0.0 +- Node.js >= 20.0.0 - TypeScript >= 5.0 (for TypeScript users) - Peer dependency: `zod@^4.0.0` diff --git a/docs/getting-started.md b/docs/getting-started.md index 2e2b401..c062fac 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -15,7 +15,7 @@ pnpm add mcp-from-openapi ``` **Requirements:** -- Node.js >= 18.0.0 +- Node.js >= 20.0.0 - Peer dependency: `zod@^4.0.0` --- diff --git a/package.json b/package.json index e3fec65..d55918c 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ }, "homepage": "https://github.com/agentfront/mcp-from-openapi#readme", "engines": { - "node": ">=18.0.0" + "node": ">=20.0.0" }, "type": "commonjs", "main": "./dist/index.js", From 4aa05229e13f114304cee3e1ec1ae63a3760aed7 Mon Sep 17 00:00:00 2001 From: David Antoon Date: Wed, 8 Apr 2026 02:23:55 +0300 Subject: [PATCH 4/4] chore: update Node.js version matrix in CI configuration to remove 18.x --- .github/workflows/push.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index fee6a37..4111c85 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -26,7 +26,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [18.x, 20.x, 22.x, 24.x] + node-version: [20.x, 22.x, 24.x] steps: - uses: actions/checkout@v4