Skip to content

Commit caa3164

Browse files
committed
merge commit
2 parents a0a89bb + 00249ce commit caa3164

8 files changed

Lines changed: 247 additions & 5 deletions

File tree

.changeset/oauth-error-http200.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
---
4+
5+
Fix OAuth error handling for servers returning errors with HTTP 200 status
6+
7+
Some OAuth servers (e.g., GitHub) return error responses with HTTP 200 status instead of 4xx. The SDK now checks for an `error` field in the JSON response before attempting to parse it as tokens, providing users with meaningful error messages.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@modelcontextprotocol/client': patch
3+
---
4+
5+
Respect capability negotiation in list methods by returning empty lists when server lacks capability
6+
7+
The Client now returns empty lists instead of sending requests to servers that don't advertise the corresponding capability:
8+
- `listPrompts()` returns `{ prompts: [] }` if server lacks prompts capability
9+
- `listResources()` returns `{ resources: [] }` if server lacks resources capability
10+
- `listResourceTemplates()` returns `{ resourceTemplates: [] }` if server lacks resources capability
11+
- `listTools()` returns `{ tools: [] }` if server lacks tools capability
12+
13+
This respects the MCP spec requirement that "Both parties SHOULD respect capability negotiation" and avoids unnecessary server warnings and traffic. The existing `enforceStrictCapabilities` option continues to throw errors when set to `true`.

packages/client/src/client/auth.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1088,7 +1088,18 @@ async function executeTokenRequest(
10881088
throw await parseErrorResponse(response);
10891089
}
10901090

1091-
return OAuthTokensSchema.parse(await response.json());
1091+
const json: unknown = await response.json();
1092+
1093+
try {
1094+
return OAuthTokensSchema.parse(json);
1095+
} catch (parseError) {
1096+
// Some OAuth servers (e.g., GitHub) return error responses with HTTP 200 status.
1097+
// Check for error field only if token parsing failed.
1098+
if (typeof json === 'object' && json !== null && 'error' in json) {
1099+
throw await parseErrorResponse(JSON.stringify(json));
1100+
}
1101+
throw parseError;
1102+
}
10921103
}
10931104

10941105
/**

packages/client/src/client/client.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ export class Client<
289289
// Error handlers (single callback pattern, matching McpServer)
290290
private _onErrorHandler?: OnErrorHandler;
291291
private _onProtocolErrorHandler?: OnProtocolErrorHandler;
292+
private _enforceStrictCapabilities: boolean;
292293

293294
/**
294295
* Initializes this client with the given name and version information.
@@ -301,6 +302,7 @@ export class Client<
301302
this._capabilities = options?.capabilities ?? {};
302303
this._jsonSchemaValidator = options?.jsonSchemaValidator ?? new AjvJsonSchemaValidator();
303304
this._middleware = new ClientMiddlewareManager();
305+
this._enforceStrictCapabilities = options?.enforceStrictCapabilities ?? false;
304306

305307
// Store list changed config for setup after connection (when we know server capabilities)
306308
if (options?.listChanged) {
@@ -970,14 +972,31 @@ export class Client<
970972
}
971973

972974
async listPrompts(params?: ListPromptsRequest['params'], options?: RequestOptions) {
975+
if (!this._serverCapabilities?.prompts && !this._enforceStrictCapabilities) {
976+
// Respect capability negotiation: server does not support prompts
977+
console.debug('Client.listPrompts() called but server does not advertise prompts capability - returning empty list');
978+
return { prompts: [] };
979+
}
973980
return this.request({ method: 'prompts/list', params }, ListPromptsResultSchema, options);
974981
}
975982

976983
async listResources(params?: ListResourcesRequest['params'], options?: RequestOptions) {
984+
if (!this._serverCapabilities?.resources && !this._enforceStrictCapabilities) {
985+
// Respect capability negotiation: server does not support resources
986+
console.debug('Client.listResources() called but server does not advertise resources capability - returning empty list');
987+
return { resources: [] };
988+
}
977989
return this.request({ method: 'resources/list', params }, ListResourcesResultSchema, options);
978990
}
979991

980992
async listResourceTemplates(params?: ListResourceTemplatesRequest['params'], options?: RequestOptions) {
993+
if (!this._serverCapabilities?.resources && !this._enforceStrictCapabilities) {
994+
// Respect capability negotiation: server does not support resources
995+
console.debug(
996+
'Client.listResourceTemplates() called but server does not advertise resources capability - returning empty list'
997+
);
998+
return { resourceTemplates: [] };
999+
}
9811000
return this.request({ method: 'resources/templates/list', params }, ListResourceTemplatesResultSchema, options);
9821001
}
9831002

@@ -1096,6 +1115,11 @@ export class Client<
10961115
}
10971116

10981117
async listTools(params?: ListToolsRequest['params'], options?: RequestOptions) {
1118+
if (!this._serverCapabilities?.tools && !this._enforceStrictCapabilities) {
1119+
// Respect capability negotiation: server does not support tools
1120+
console.debug('Client.listTools() called but server does not advertise tools capability - returning empty list');
1121+
return { tools: [] };
1122+
}
10991123
const result = await this.request({ method: 'tools/list', params }, ListToolsResultSchema, options);
11001124

11011125
// Cache the tools and their output schemas for future validation

packages/server/src/server/mcp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ export class McpServer {
474474
// If that fails, use the schema directly (for union/intersection/etc)
475475
const inputObj = normalizeObjectSchema(tool.inputSchema);
476476
const schemaToParse = inputObj ?? (tool.inputSchema as AnySchema);
477-
const parseResult = await safeParseAsync(schemaToParse, args);
477+
const parseResult = await safeParseAsync(schemaToParse, args ?? {});
478478
if (!parseResult.success) {
479479
const error = 'error' in parseResult ? parseResult.error : 'Unknown error';
480480
const errorMessage = getParseErrorMessage(error);

test/integration/test/client/client.test.ts

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,79 @@ test('should respect server capabilities', async () => {
595595
).rejects.toThrow('Server does not support completions');
596596
});
597597

598+
/***
599+
* Test: Return empty lists for missing capabilities (default behavior)
600+
* When enforceStrictCapabilities is not set (default), list methods should
601+
* return empty lists instead of sending requests to servers that don't
602+
* advertise those capabilities.
603+
*/
604+
test('should return empty lists for missing capabilities by default', async () => {
605+
const server = new Server(
606+
{
607+
name: 'test server',
608+
version: '1.0'
609+
},
610+
{
611+
capabilities: {
612+
// Server only supports tools - no prompts or resources
613+
tools: {}
614+
}
615+
}
616+
);
617+
618+
server.setRequestHandler(InitializeRequestSchema, _request => ({
619+
protocolVersion: LATEST_PROTOCOL_VERSION,
620+
capabilities: {
621+
tools: {}
622+
},
623+
serverInfo: {
624+
name: 'test',
625+
version: '1.0'
626+
}
627+
}));
628+
629+
server.setRequestHandler(ListToolsRequestSchema, () => ({
630+
tools: [{ name: 'test-tool', inputSchema: { type: 'object' } }]
631+
}));
632+
633+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
634+
635+
// Client with default settings (enforceStrictCapabilities not set)
636+
const client = new Client(
637+
{
638+
name: 'test client',
639+
version: '1.0'
640+
},
641+
{
642+
capabilities: {}
643+
}
644+
);
645+
646+
await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]);
647+
648+
// Server only supports tools
649+
expect(client.getServerCapabilities()).toEqual({
650+
tools: {}
651+
});
652+
653+
// listTools should work and return actual tools
654+
const toolsResult = await client.listTools();
655+
expect(toolsResult.tools).toHaveLength(1);
656+
expect(toolsResult.tools[0].name).toBe('test-tool');
657+
658+
// listPrompts should return empty list without sending request
659+
const promptsResult = await client.listPrompts();
660+
expect(promptsResult.prompts).toEqual([]);
661+
662+
// listResources should return empty list without sending request
663+
const resourcesResult = await client.listResources();
664+
expect(resourcesResult.resources).toEqual([]);
665+
666+
// listResourceTemplates should return empty list without sending request
667+
const templatesResult = await client.listResourceTemplates();
668+
expect(templatesResult.resourceTemplates).toEqual([]);
669+
});
670+
598671
/***
599672
* Test: Respect Client Notification Capabilities
600673
*/
@@ -1885,7 +1958,7 @@ describe('outputSchema validation', () => {
18851958
// Set up server handlers
18861959
server.setRequestHandler(InitializeRequestSchema, async request => ({
18871960
protocolVersion: request.params.protocolVersion,
1888-
capabilities: {},
1961+
capabilities: { tools: {} },
18891962
serverInfo: {
18901963
name: 'test-server',
18911964
version: '1.0.0'
@@ -1977,7 +2050,7 @@ describe('outputSchema validation', () => {
19772050
// Set up server handlers
19782051
server.setRequestHandler(InitializeRequestSchema, async request => ({
19792052
protocolVersion: request.params.protocolVersion,
1980-
capabilities: {},
2053+
capabilities: { tools: {} },
19812054
serverInfo: {
19822055
name: 'test-server',
19832056
version: '1.0.0'
@@ -2270,7 +2343,7 @@ describe('outputSchema validation', () => {
22702343
// Set up server handlers
22712344
server.setRequestHandler(InitializeRequestSchema, async request => ({
22722345
protocolVersion: request.params.protocolVersion,
2273-
capabilities: {},
2346+
capabilities: { tools: {} },
22742347
serverInfo: {
22752348
name: 'test-server',
22762349
version: '1.0.0'
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* Regression test for https://github.com/modelcontextprotocol/typescript-sdk/issues/400
3+
*
4+
* When a tool has all optional parameters, some LLM models call the tool without
5+
* providing an `arguments` field. This test verifies that undefined arguments are
6+
* handled correctly by defaulting to an empty object.
7+
*/
8+
9+
import { Client } from '@modelcontextprotocol/client';
10+
import { CallToolResultSchema, InMemoryTransport } from '@modelcontextprotocol/core';
11+
import { McpServer } from '@modelcontextprotocol/server';
12+
import type { ZodMatrixEntry } from '@modelcontextprotocol/test-helpers';
13+
import { zodTestMatrix } from '@modelcontextprotocol/test-helpers';
14+
15+
describe.each(zodTestMatrix)('Issue #400: $zodVersionLabel', (entry: ZodMatrixEntry) => {
16+
const { z } = entry;
17+
18+
test('should accept undefined arguments when all tool params are optional', async () => {
19+
const mcpServer = new McpServer({
20+
name: 'test server',
21+
version: '1.0'
22+
});
23+
const client = new Client({
24+
name: 'test client',
25+
version: '1.0'
26+
});
27+
28+
mcpServer.registerTool(
29+
'optional-params-tool',
30+
{
31+
inputSchema: {
32+
limit: z.number().optional(),
33+
offset: z.number().optional()
34+
}
35+
},
36+
async ({ limit, offset }) => ({
37+
content: [
38+
{
39+
type: 'text',
40+
text: `limit: ${limit ?? 'default'}, offset: ${offset ?? 'default'}`
41+
}
42+
]
43+
})
44+
);
45+
46+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
47+
48+
await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]);
49+
50+
// Call tool without arguments (arguments is undefined)
51+
const result = await client.request(
52+
{
53+
method: 'tools/call',
54+
params: {
55+
name: 'optional-params-tool'
56+
// arguments is intentionally omitted (undefined)
57+
}
58+
},
59+
CallToolResultSchema
60+
);
61+
62+
expect(result.isError).toBeUndefined();
63+
expect(result.content).toEqual([
64+
{
65+
type: 'text',
66+
text: 'limit: default, offset: default'
67+
}
68+
]);
69+
});
70+
});
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Regression test for https://github.com/modelcontextprotocol/typescript-sdk/issues/1342
3+
*
4+
* Some OAuth servers (e.g., GitHub) return error responses with HTTP 200 status
5+
* instead of 4xx. Previously, the SDK would try to parse these as tokens and fail
6+
* with a confusing Zod validation error. This test verifies that the SDK properly
7+
* detects the error field and surfaces the actual OAuth error message.
8+
*/
9+
10+
import { exchangeAuthorization } from '@modelcontextprotocol/client';
11+
import { describe, expect, it, vi } from 'vitest';
12+
13+
const mockFetch = vi.fn();
14+
vi.stubGlobal('fetch', mockFetch);
15+
16+
describe('Issue #1342: OAuth error response with HTTP 200 status', () => {
17+
const validClientInfo = {
18+
client_id: 'test-client',
19+
client_secret: 'test-secret',
20+
redirect_uris: ['http://localhost:3000/callback'],
21+
token_endpoint_auth_method: 'client_secret_post' as const
22+
};
23+
24+
it('should throw OAuth error when server returns error with HTTP 200', async () => {
25+
// GitHub returns errors with HTTP 200 instead of 4xx
26+
mockFetch.mockResolvedValueOnce({
27+
ok: true,
28+
status: 200,
29+
json: async () => ({
30+
error: 'invalid_client',
31+
error_description: 'The client_id and/or client_secret passed are incorrect.'
32+
})
33+
});
34+
35+
await expect(
36+
exchangeAuthorization('https://auth.example.com', {
37+
clientInformation: validClientInfo,
38+
authorizationCode: 'code123',
39+
codeVerifier: 'verifier123',
40+
redirectUri: 'http://localhost:3000/callback'
41+
})
42+
).rejects.toThrow('The client_id and/or client_secret passed are incorrect.');
43+
});
44+
});

0 commit comments

Comments
 (0)