Skip to content

Commit 0f67756

Browse files
authored
Merge pull request #902 from vincent0426/fix/resolve-ref-pre-validation
Fix $ref resolution in request schema properties before validation
2 parents f93afb0 + 156e1dc commit 0f67756

File tree

3 files changed

+178
-2
lines changed

3 files changed

+178
-2
lines changed

client/src/lib/hooks/__tests__/useConnection.test.tsx

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { renderHook, act } from "@testing-library/react";
22
import { useConnection } from "../useConnection";
33
import { z } from "zod";
4-
import { ClientRequest } from "@modelcontextprotocol/sdk/types.js";
4+
import {
5+
ClientRequest,
6+
JSONRPCMessage,
7+
} from "@modelcontextprotocol/sdk/types.js";
58
import { DEFAULT_INSPECTOR_CONFIG, CLIENT_IDENTITY } from "../../constants";
69
import {
710
SSEClientTransportOptions,
@@ -42,10 +45,12 @@ const mockSSETransport: {
4245
start: jest.Mock;
4346
url: URL | undefined;
4447
options: SSEClientTransportOptions | undefined;
48+
onmessage?: (message: JSONRPCMessage) => void;
4549
} = {
4650
start: jest.fn(),
4751
url: undefined,
4852
options: undefined,
53+
onmessage: undefined,
4954
};
5055

5156
const mockStreamableHTTPTransport: {
@@ -482,6 +487,129 @@ describe("useConnection", () => {
482487
});
483488
});
484489

490+
describe("Ref Resolution", () => {
491+
beforeEach(() => {
492+
jest.clearAllMocks();
493+
});
494+
495+
test("resolves $ref references in requestedSchema properties before validation", async () => {
496+
const mockProtocolOnMessage = jest.fn();
497+
498+
mockSSETransport.onmessage = mockProtocolOnMessage;
499+
500+
const { result } = renderHook(() => useConnection(defaultProps));
501+
502+
await act(async () => {
503+
await result.current.connect();
504+
});
505+
506+
const mockRequestWithRef: JSONRPCMessage = {
507+
jsonrpc: "2.0",
508+
id: 1,
509+
method: "elicitation/create",
510+
params: {
511+
message: "Please provide your information",
512+
requestedSchema: {
513+
type: "object",
514+
properties: {
515+
source: {
516+
type: "string",
517+
minLength: 1,
518+
title: "A Connectable Node",
519+
},
520+
target: {
521+
$ref: "#/properties/source",
522+
},
523+
},
524+
},
525+
},
526+
};
527+
528+
await act(async () => {
529+
mockSSETransport.onmessage!(mockRequestWithRef);
530+
});
531+
532+
expect(mockProtocolOnMessage).toHaveBeenCalledTimes(1);
533+
534+
const message = mockProtocolOnMessage.mock.calls[0][0];
535+
expect(message.params.requestedSchema.properties.target).toEqual({
536+
type: "string",
537+
minLength: 1,
538+
title: "A Connectable Node",
539+
});
540+
});
541+
542+
test("resolves $ref references to $defs in requestedSchema", async () => {
543+
const mockProtocolOnMessage = jest.fn();
544+
545+
mockSSETransport.onmessage = mockProtocolOnMessage;
546+
547+
const { result } = renderHook(() => useConnection(defaultProps));
548+
549+
await act(async () => {
550+
await result.current.connect();
551+
});
552+
553+
const mockRequestWithDefs: JSONRPCMessage = {
554+
jsonrpc: "2.0",
555+
id: 1,
556+
method: "elicitation/create",
557+
params: {
558+
message: "Please provide your information",
559+
requestedSchema: {
560+
type: "object",
561+
properties: {
562+
user: {
563+
$ref: "#/$defs/UserInput",
564+
},
565+
},
566+
$defs: {
567+
UserInput: {
568+
type: "object",
569+
properties: {
570+
name: {
571+
type: "string",
572+
title: "Name",
573+
},
574+
age: {
575+
type: "integer",
576+
title: "Age",
577+
minimum: 0,
578+
},
579+
},
580+
required: ["name"],
581+
},
582+
},
583+
},
584+
},
585+
};
586+
587+
await act(async () => {
588+
mockSSETransport.onmessage!(mockRequestWithDefs);
589+
});
590+
591+
expect(mockProtocolOnMessage).toHaveBeenCalledTimes(1);
592+
593+
const message = mockProtocolOnMessage.mock.calls[0][0];
594+
// The $ref should be resolved to the actual UserInput definition
595+
expect(message.params.requestedSchema.properties.user).toEqual({
596+
type: "object",
597+
properties: {
598+
name: {
599+
type: "string",
600+
title: "Name",
601+
},
602+
age: {
603+
type: "integer",
604+
title: "Age",
605+
minimum: 0,
606+
},
607+
},
608+
required: ["name"],
609+
});
610+
});
611+
});
612+
485613
describe("URL Port Handling", () => {
486614
const SSEClientTransport = jest.requireMock(
487615
"@modelcontextprotocol/sdk/client/sse.js",

client/src/lib/hooks/useConnection.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import { getMCPServerRequestTimeout } from "@/utils/configUtils";
5959
import { InspectorConfig } from "../configurationTypes";
6060
import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js";
6161
import { CustomHeaders } from "../types/customHeaders";
62+
import { resolveRefsInMessage } from "@/utils/schemaUtils";
6263

6364
interface UseConnectionOptions {
6465
transportType: "stdio" | "sse" | "streamable-http";
@@ -691,6 +692,14 @@ export function useConnection({
691692

692693
await client.connect(transport as Transport);
693694

695+
const protocolOnMessage = transport.onmessage;
696+
if (protocolOnMessage) {
697+
transport.onmessage = (message) => {
698+
const resolvedMessage = resolveRefsInMessage(message);
699+
protocolOnMessage(resolvedMessage);
700+
};
701+
}
702+
694703
setClientTransport(transport);
695704

696705
capabilities = client.getServerCapabilities();

client/src/utils/schemaUtils.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { JsonValue, JsonSchemaType, JsonObject } from "./jsonUtils";
22
import Ajv from "ajv";
33
import type { ValidateFunction } from "ajv";
4-
import type { Tool } from "@modelcontextprotocol/sdk/types.js";
4+
import type { Tool, JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js";
5+
import { isJSONRPCRequest } from "@modelcontextprotocol/sdk/types.js";
56

67
const ajv = new Ajv();
78

@@ -299,3 +300,41 @@ export function formatFieldLabel(key: string): string {
299300
.replace(/_/g, " ") // Replace underscores with spaces
300301
.replace(/^\w/, (c) => c.toUpperCase()); // Capitalize first letter
301302
}
303+
304+
/**
305+
* Resolves `$ref` references in a JSON-RPC "elicitation/create" message's `requestedSchema` field
306+
* @param message The JSON-RPC message that may contain $ref references
307+
* @returns A new message with resolved $ref references, or the original message if no resolution is needed
308+
*/
309+
export function resolveRefsInMessage(message: JSONRPCMessage): JSONRPCMessage {
310+
if (!isJSONRPCRequest(message) || !message.params?.requestedSchema) {
311+
return message;
312+
}
313+
314+
const requestedSchema = message.params.requestedSchema as JsonSchemaType;
315+
316+
if (!requestedSchema?.properties) {
317+
return message;
318+
}
319+
320+
const resolvedMessage = {
321+
...message,
322+
params: {
323+
...message.params,
324+
requestedSchema: {
325+
...requestedSchema,
326+
properties: Object.fromEntries(
327+
Object.entries(requestedSchema.properties).map(
328+
([key, propSchema]) => {
329+
const resolved = resolveRef(propSchema, requestedSchema);
330+
const normalized = normalizeUnionType(resolved);
331+
return [key, normalized];
332+
},
333+
),
334+
),
335+
},
336+
},
337+
};
338+
339+
return resolvedMessage;
340+
}

0 commit comments

Comments
 (0)