Skip to content

Commit 4ce7861

Browse files
zhawkencursoragent
zhawken
and
cursoragent
authored
fix: allow MCP clients that double-serialize parameters to work correctly (#212)
* fix: allow string-encoded object params to pass schema validation Schema validation rejects tool call arguments before the handler runs, so the existing deserializeParams() fix in proxy.ts never gets a chance to convert JSON-encoded strings back to objects. Add withStringFallback() to OpenAPIToMCPConverter which wraps any complex (object/$ref/anyOf/oneOf/allOf) property schema with anyOf: [originalSchema, { type: 'string' }]. This lets schema validation accept both properly-typed objects AND JSON-encoded strings, so the request reaches the handler where deserializeParams() converts the string back to the expected object before the API call. Fully backward compatible — object inputs continue to work unchanged. Closes #208 Co-authored-by: Cursor <cursoragent@cursor.com> * fix: handle array items with JSON-encoded string elements Two follow-up fixes for the children parameter bug: 1. parser.ts: extend withStringFallback() to recurse into array items, wrapping them with anyOf: [original, string, object] so schema validation accepts both proper objects and JSON-encoded strings. 2. proxy.ts: extend deserializeParams() to iterate array items and deserialize any that are valid JSON strings back to objects before the API call, mirroring the existing top-level string conversion. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 50fb415 commit 4ce7861

5 files changed

Lines changed: 452 additions & 41 deletions

File tree

‎src/openapi-mcp-server/mcp/__tests__/proxy.test.ts‎

Lines changed: 318 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,160 @@ describe('MCPProxy', () => {
334334
})
335335
})
336336

337+
describe('string-encoded object params deserialized in handler (issue #208)', () => {
338+
let callToolHandler: Function
339+
340+
beforeEach(() => {
341+
const server = (proxy as any).server
342+
const handlers = server.setRequestHandler.mock.calls
343+
.flatMap((x: unknown[]) => x)
344+
.filter((x: unknown) => typeof x === 'function')
345+
callToolHandler = handlers[1]
346+
})
347+
348+
it('should handle notion-create-a-page parent provided as a JSON string', async () => {
349+
const mockResponse = {
350+
data: { id: 'new-page-id' },
351+
status: 200,
352+
headers: new Headers({ 'content-type': 'application/json' }),
353+
}
354+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
355+
356+
;(proxy as any).openApiLookup = {
357+
'notion-create-a-page': {
358+
operationId: 'notion-create-a-page',
359+
responses: { '200': { description: 'Success' } },
360+
method: 'post',
361+
path: '/pages',
362+
},
363+
}
364+
365+
// Claude Desktop ≥ v1.1.3189 sends object params as JSON strings
366+
const parentAsString = JSON.stringify({ database_id: 'abc123' })
367+
368+
// Should not throw in this handler-level test
369+
await expect(
370+
callToolHandler({
371+
params: {
372+
name: 'notion-create-a-page',
373+
arguments: { parent: parentAsString },
374+
},
375+
}),
376+
).resolves.toBeDefined()
377+
378+
// deserializeParams should have converted it back to an object
379+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
380+
expect.anything(),
381+
expect.objectContaining({
382+
parent: { database_id: 'abc123' },
383+
}),
384+
)
385+
})
386+
387+
it('should still work when notion-create-a-page parent is already an object (backward compatible)', async () => {
388+
const mockResponse = {
389+
data: { id: 'new-page-id' },
390+
status: 200,
391+
headers: new Headers({ 'content-type': 'application/json' }),
392+
}
393+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
394+
395+
;(proxy as any).openApiLookup = {
396+
'notion-create-a-page': {
397+
operationId: 'notion-create-a-page',
398+
responses: { '200': { description: 'Success' } },
399+
method: 'post',
400+
path: '/pages',
401+
},
402+
}
403+
404+
await expect(
405+
callToolHandler({
406+
params: {
407+
name: 'notion-create-a-page',
408+
arguments: { parent: { database_id: 'abc123' } },
409+
},
410+
}),
411+
).resolves.toBeDefined()
412+
413+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
414+
expect.anything(),
415+
expect.objectContaining({
416+
parent: { database_id: 'abc123' },
417+
}),
418+
)
419+
})
420+
421+
it('should handle notion-update-page data provided as a JSON string', async () => {
422+
const mockResponse = {
423+
data: { id: 'updated-page-id' },
424+
status: 200,
425+
headers: new Headers({ 'content-type': 'application/json' }),
426+
}
427+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
428+
429+
;(proxy as any).openApiLookup = {
430+
'notion-update-page': {
431+
operationId: 'notion-update-page',
432+
responses: { '200': { description: 'Success' } },
433+
method: 'patch',
434+
path: '/pages/{page_id}',
435+
},
436+
}
437+
438+
const dataAsString = JSON.stringify({ properties: { Status: { select: { name: 'Done' } } } })
439+
440+
await expect(
441+
callToolHandler({
442+
params: {
443+
name: 'notion-update-page',
444+
arguments: { data: dataAsString },
445+
},
446+
}),
447+
).resolves.toBeDefined()
448+
449+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
450+
expect.anything(),
451+
expect.objectContaining({
452+
data: { properties: { Status: { select: { name: 'Done' } } } },
453+
}),
454+
)
455+
})
456+
457+
it('should call deserializeParams and convert string to object before executeOperation', async () => {
458+
const mockResponse = {
459+
data: { success: true },
460+
status: 200,
461+
headers: new Headers({ 'content-type': 'application/json' }),
462+
}
463+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
464+
465+
;(proxy as any).openApiLookup = {
466+
'notion-move-pages': {
467+
operationId: 'notion-move-pages',
468+
responses: { '200': { description: 'Success' } },
469+
method: 'post',
470+
path: '/pages/move',
471+
},
472+
}
473+
474+
const newParentAsString = JSON.stringify({ page_id: 'parent-page-id' })
475+
476+
await callToolHandler({
477+
params: {
478+
name: 'notion-move-pages',
479+
arguments: { new_parent: newParentAsString },
480+
},
481+
})
482+
483+
// Verify executeOperation received the deserialized object, not the string
484+
const callArgs = (HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mock.calls[0]
485+
const passedParams = callArgs[1]
486+
expect(typeof passedParams.new_parent).not.toBe('string')
487+
expect(passedParams.new_parent).toEqual({ page_id: 'parent-page-id' })
488+
})
489+
})
490+
337491
describe('double-serialization fix (issue #176)', () => {
338492
it('should deserialize stringified JSON object parameters', async () => {
339493
// Mock HttpClient response
@@ -435,6 +589,170 @@ describe('MCPProxy', () => {
435589
)
436590
})
437591

592+
it('should deserialize JSON string items within an array parameter', async () => {
593+
const mockResponse = {
594+
data: { id: 'new-page-id' },
595+
status: 200,
596+
headers: new Headers({ 'content-type': 'application/json' }),
597+
}
598+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
599+
600+
;(proxy as any).openApiLookup = {
601+
'API-appendBlockChildren': {
602+
operationId: 'appendBlockChildren',
603+
responses: { '200': { description: 'Success' } },
604+
method: 'patch',
605+
path: '/blocks/{block_id}/children',
606+
},
607+
}
608+
609+
const server = (proxy as any).server
610+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
611+
const callToolHandler = handlers[1]
612+
613+
// Claude Desktop sends each array item as a JSON string
614+
const block1 = JSON.stringify({ object: 'block', type: 'paragraph', paragraph: { rich_text: [{ type: 'text', text: { content: 'Hello' } }] } })
615+
const block2 = JSON.stringify({ object: 'block', type: 'heading_1', heading_1: { rich_text: [{ type: 'text', text: { content: 'Title' } }] } })
616+
617+
await callToolHandler({
618+
params: {
619+
name: 'API-appendBlockChildren',
620+
arguments: {
621+
children: [block1, block2],
622+
},
623+
},
624+
})
625+
626+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
627+
expect.anything(),
628+
{
629+
children: [
630+
{ object: 'block', type: 'paragraph', paragraph: { rich_text: [{ type: 'text', text: { content: 'Hello' } }] } },
631+
{ object: 'block', type: 'heading_1', heading_1: { rich_text: [{ type: 'text', text: { content: 'Title' } }] } },
632+
],
633+
},
634+
)
635+
})
636+
637+
it('should pass through an array of proper objects unchanged', async () => {
638+
const mockResponse = {
639+
data: { id: 'new-page-id' },
640+
status: 200,
641+
headers: new Headers({ 'content-type': 'application/json' }),
642+
}
643+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
644+
645+
;(proxy as any).openApiLookup = {
646+
'API-appendBlockChildren': {
647+
operationId: 'appendBlockChildren',
648+
responses: { '200': { description: 'Success' } },
649+
method: 'patch',
650+
path: '/blocks/{block_id}/children',
651+
},
652+
}
653+
654+
const server = (proxy as any).server
655+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
656+
const callToolHandler = handlers[1]
657+
658+
const block1 = { object: 'block', type: 'paragraph' }
659+
const block2 = { object: 'block', type: 'heading_1' }
660+
661+
await callToolHandler({
662+
params: {
663+
name: 'API-appendBlockChildren',
664+
arguments: {
665+
children: [block1, block2],
666+
},
667+
},
668+
})
669+
670+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
671+
expect.anything(),
672+
{ children: [block1, block2] },
673+
)
674+
})
675+
676+
it('should handle a mixed array with both string items and object items', async () => {
677+
const mockResponse = {
678+
data: { success: true },
679+
status: 200,
680+
headers: new Headers({ 'content-type': 'application/json' }),
681+
}
682+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
683+
684+
;(proxy as any).openApiLookup = {
685+
'API-appendBlockChildren': {
686+
operationId: 'appendBlockChildren',
687+
responses: { '200': { description: 'Success' } },
688+
method: 'patch',
689+
path: '/blocks/{block_id}/children',
690+
},
691+
}
692+
693+
const server = (proxy as any).server
694+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
695+
const callToolHandler = handlers[1]
696+
697+
const blockAsString = JSON.stringify({ object: 'block', type: 'paragraph' })
698+
const blockAsObject = { object: 'block', type: 'heading_1' }
699+
700+
await callToolHandler({
701+
params: {
702+
name: 'API-appendBlockChildren',
703+
arguments: {
704+
children: [blockAsString, blockAsObject],
705+
},
706+
},
707+
})
708+
709+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
710+
expect.anything(),
711+
{
712+
children: [
713+
{ object: 'block', type: 'paragraph' },
714+
{ object: 'block', type: 'heading_1' },
715+
],
716+
},
717+
)
718+
})
719+
720+
it('should preserve non-JSON string items within arrays', async () => {
721+
const mockResponse = {
722+
data: { success: true },
723+
status: 200,
724+
headers: new Headers({ 'content-type': 'application/json' }),
725+
}
726+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
727+
728+
;(proxy as any).openApiLookup = {
729+
'API-search': {
730+
operationId: 'search',
731+
responses: { '200': { description: 'Success' } },
732+
method: 'post',
733+
path: '/search',
734+
},
735+
}
736+
737+
const server = (proxy as any).server
738+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
739+
const callToolHandler = handlers[1]
740+
741+
await callToolHandler({
742+
params: {
743+
name: 'API-search',
744+
arguments: {
745+
tags: ['hello', 'world', '{ not valid json }'],
746+
},
747+
},
748+
})
749+
750+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
751+
expect.anything(),
752+
{ tags: ['hello', 'world', '{ not valid json }'] },
753+
)
754+
})
755+
438756
it('should preserve non-JSON string parameters', async () => {
439757
const mockResponse = {
440758
data: { success: true },

‎src/openapi-mcp-server/mcp/proxy.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,29 @@ function deserializeParams(params: Record<string, unknown>): Record<string, unkn
5353
// If parsing fails, keep the original string value
5454
}
5555
}
56+
} else if (Array.isArray(value)) {
57+
// Deserialize any JSON-string items within the array
58+
result[key] = value.map((item) => {
59+
if (typeof item !== 'string') return item
60+
const trimmed = item.trim()
61+
if (
62+
(trimmed.startsWith('{') && trimmed.endsWith('}')) ||
63+
(trimmed.startsWith('[') && trimmed.endsWith(']'))
64+
) {
65+
try {
66+
const parsed = JSON.parse(item)
67+
if (typeof parsed === 'object' && parsed !== null) {
68+
return Array.isArray(parsed)
69+
? parsed
70+
: deserializeParams(parsed as Record<string, unknown>)
71+
}
72+
} catch {
73+
// If parsing fails, keep the original string item
74+
}
75+
}
76+
return item
77+
})
78+
continue
5679
}
5780
result[key] = value
5881
}

0 commit comments

Comments
 (0)