Skip to content

fix(core): generalize MCP compliance fix for tool results #27045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cocosheng-g merged 3 commits into from
May 20, 2026
Merged

fix(core): generalize MCP compliance fix for tool results #27045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a32cf56
fix(core): generalize MCP compliance fix for tool results
cocosheng-g May 14, 2026
7183a57
fix(core): fix missing Transport import in tests
cocosheng-g May 14, 2026
2d10ac9
test(core): fix mcp-client test failure and add compliance transport …
cocosheng-g May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 49 additions & 37 deletions packages/core/src/tools/mcp-client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
discoverPrompts,
type McpContext,
} from './mcp-client.js';
import { McpComplianceTransport } from './mcp-compliance-transport.js';
import type { ToolRegistry } from './tool-registry.js';
import type { ResourceRegistry } from '../resources/resource-registry.js';
import * as fs from 'node:fs';
Expand Down Expand Up @@ -71,6 +72,9 @@ const MOCK_CONTEXT_DEFAULT = {

let MOCK_CONTEXT: McpContext = MOCK_CONTEXT_DEFAULT;

const unwrap = (t: any) =>
t instanceof McpComplianceTransport ? t.transport : t;

vi.mock('@modelcontextprotocol/sdk/client/stdio.js');
vi.mock('@modelcontextprotocol/sdk/client/index.js');
vi.mock('@google/genai');
Expand Down Expand Up @@ -1937,7 +1941,7 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

const testableTransport = transport as unknown as {
const testableTransport = unwrap(transport) as unknown as {
_authProvider?: {
tokens: () => Promise<{ access_token: string } | undefined>;
};
Expand Down Expand Up @@ -1983,7 +1987,7 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

const testableTransport = transport as unknown as {
const testableTransport = unwrap(transport) as unknown as {
_authProvider?: {
tokens: () => Promise<
{ access_token: string; expires_in?: number } | undefined
Expand Down Expand Up @@ -2032,7 +2036,7 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

const testableTransport = transport as unknown as {
const testableTransport = unwrap(transport) as unknown as {
_authProvider?: {
tokens: () => Promise<{ access_token: string } | undefined>;
};
Expand Down Expand Up @@ -2075,7 +2079,7 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

const testableTransport = transport as unknown as {
const testableTransport = unwrap(transport) as unknown as {
_authProvider?: {
tokens: () => Promise<{ access_token: string } | undefined>;
};
Expand Down Expand Up @@ -2128,7 +2132,7 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

const testableTransport = transport as unknown as {
const testableTransport = unwrap(transport) as unknown as {
_authProvider?: {
tokens: () => Promise<
{ access_token: string; expires_in?: number } | undefined
Expand Down Expand Up @@ -2167,7 +2171,7 @@ describe('mcp-client', () => {
);

const wrappedFetch = (
transport as unknown as {
unwrap(transport) as unknown as {
_fetch: (
url: URL | string,
init?: RequestInit,
Expand Down Expand Up @@ -2209,7 +2213,7 @@ describe('mcp-client', () => {

// For SSEClientTransport, the fetch is private or passed to the SDK.
// We can check if it creates the transport successfully.
expect(transport).toBeInstanceOf(SSEClientTransport);
expect(unwrap(transport)).toBeInstanceOf(SSEClientTransport);
} finally {
vi.unstubAllEnvs();
vi.unstubAllGlobals();
Expand All @@ -2227,8 +2231,8 @@ describe('mcp-client', () => {
false,
MOCK_CONTEXT,
);
expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: { headers: {} },
});
Expand All @@ -2245,8 +2249,8 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: {
headers: { Authorization: 'derp' },
Expand All @@ -2265,8 +2269,8 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: { headers: {} },
});
Expand All @@ -2283,8 +2287,8 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(SSEClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(SSEClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: { headers: {} },
});
Expand All @@ -2300,8 +2304,8 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: { headers: {} },
});
Expand All @@ -2319,8 +2323,8 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: {
headers: { Authorization: 'Bearer token' },
Expand All @@ -2340,8 +2344,8 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(SSEClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(SSEClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server'),
_requestInit: {
headers: { 'X-API-Key': 'key123' },
Expand All @@ -2361,8 +2365,8 @@ describe('mcp-client', () => {
);

// httpUrl should take priority and create HTTP transport
expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(transport).toMatchObject({
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toMatchObject({
_url: new URL('http://test-server-http'),
_requestInit: { headers: {} },
});
Expand Down Expand Up @@ -2587,8 +2591,10 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
const testableTransport = transport as unknown as TestableTransport;
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
const testableTransport = unwrap(
transport,
) as unknown as TestableTransport;
const authProvider = testableTransport._authProvider;
expect(authProvider).toBeInstanceOf(GoogleCredentialProvider);
const googUserProject =
Expand Down Expand Up @@ -2618,9 +2624,11 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
expect(mockGetRequestHeaders).toHaveBeenCalled();
const testableTransport = transport as unknown as TestableTransport;
const testableTransport = unwrap(
transport,
) as unknown as TestableTransport;
const headers = testableTransport._requestInit?.headers;
expect(headers?.['X-Goog-User-Project']).toBe('provider-project');
});
Expand Down Expand Up @@ -2650,8 +2658,10 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(StreamableHTTPClientTransport);
const testableTransport = transport as unknown as TestableTransport;
expect(unwrap(transport)).toBeInstanceOf(StreamableHTTPClientTransport);
const testableTransport = unwrap(
transport,
) as unknown as TestableTransport;
const headers = testableTransport._requestInit?.headers;
expect(headers?.['X-Goog-User-Project']).toBe('provider-project');
});
Expand All @@ -2671,8 +2681,10 @@ describe('mcp-client', () => {
MOCK_CONTEXT,
);

expect(transport).toBeInstanceOf(SSEClientTransport);
const testableTransport = transport as unknown as TestableTransport;
expect(unwrap(transport)).toBeInstanceOf(SSEClientTransport);
const testableTransport = unwrap(
transport,
) as unknown as TestableTransport;
const authProvider = testableTransport._authProvider;
expect(authProvider).toBeInstanceOf(GoogleCredentialProvider);
});
Expand Down Expand Up @@ -2842,7 +2854,7 @@ describe('connectToMcpServer with OAuth', () => {
let capturedTransport: TestableTransport | undefined;
vi.mocked(mockedClient.connect).mockImplementationOnce(
async (transport) => {
capturedTransport = transport as unknown as TestableTransport;
capturedTransport = unwrap(transport) as unknown as TestableTransport;
return Promise.resolve();
},
);
Expand All @@ -2860,8 +2872,8 @@ describe('connectToMcpServer with OAuth', () => {
expect(mockedClient.connect).toHaveBeenCalledTimes(2);
expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce();

const authHeader = (capturedTransport as TestableTransport)._requestInit
?.headers?.['Authorization'];
const authHeader = (unwrap(capturedTransport) as TestableTransport)
._requestInit?.headers?.['Authorization'];
expect(authHeader).toBe('Bearer test-access-token');
});

Expand All @@ -2887,7 +2899,7 @@ describe('connectToMcpServer with OAuth', () => {
let capturedTransport: TestableTransport | undefined;
vi.mocked(mockedClient.connect).mockImplementationOnce(
async (transport) => {
capturedTransport = transport as unknown as TestableTransport;
capturedTransport = unwrap(transport) as unknown as TestableTransport;
return Promise.resolve();
},
);
Expand All @@ -2906,8 +2918,8 @@ describe('connectToMcpServer with OAuth', () => {
expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce();
expect(OAuthUtils.discoverOAuthConfig).toHaveBeenCalledWith(serverUrl);

const authHeader = (capturedTransport as TestableTransport)._requestInit
?.headers?.['Authorization'];
const authHeader = (unwrap(capturedTransport) as TestableTransport)
._requestInit?.headers?.['Authorization'];
expect(authHeader).toBe('Bearer test-access-token-from-discovery');
});

Expand Down
48 changes: 23 additions & 25 deletions packages/core/src/tools/mcp-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import {
import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js';
import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js';
import { DiscoveredMCPTool } from './mcp-tool.js';
import { XcodeMcpBridgeFixTransport } from './xcode-mcp-fix-transport.js';
import { McpComplianceTransport } from './mcp-compliance-transport.js';

import type { CallableTool, FunctionCall, Part, Tool } from '@google/genai';
import { basename } from 'node:path';
Expand Down Expand Up @@ -1055,7 +1055,7 @@ async function createTransportWithOAuth(
mcpServerConfig: MCPServerConfig,
accessToken: string,
cliConfig: McpContext,
): Promise<StreamableHTTPClientTransport | SSEClientTransport | null> {
): Promise<Transport | null> {
try {
const headers: Record<string, string> = {
Authorization: `Bearer ${accessToken}`,
Expand All @@ -1070,7 +1070,12 @@ async function createTransportWithOAuth(
),
};

return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions);
const transport = createUrlTransport(
mcpServerName,
mcpServerConfig,
transportOptions,
);
return transport ? new McpComplianceTransport(transport) : null;
} catch (error) {
cliConfig.emitMcpDiagnostic(
'error',
Expand Down Expand Up @@ -2318,7 +2323,9 @@ export async function createTransport(
authProvider,
};

return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions);
return new McpComplianceTransport(
createUrlTransport(mcpServerName, mcpServerConfig, transportOptions),
);
}

if (mcpServerConfig.command) {
Expand Down Expand Up @@ -2354,32 +2361,23 @@ export async function createTransport(
}
}

let transport: Transport = new StdioClientTransport({
command: mcpServerConfig.command,
args: mcpServerConfig.args || [],
env: finalEnv,
cwd: mcpServerConfig.cwd,
stderr: 'pipe',
});

// Fix for Xcode 26.3 mcpbridge non-compliant responses
// It returns JSON in `content` instead of `structuredContent`
if (
mcpServerConfig.command === 'xcrun' &&
mcpServerConfig.args?.includes('mcpbridge')
) {
transport = new XcodeMcpBridgeFixTransport(transport);
}
const transport: Transport = new McpComplianceTransport(
new StdioClientTransport({
command: mcpServerConfig.command,
args: mcpServerConfig.args || [],
env: finalEnv,
cwd: mcpServerConfig.cwd,
stderr: 'pipe',
}),
);

if (debugMode) {
// The `XcodeMcpBridgeFixTransport` wrapper hides the underlying `StdioClientTransport`,
// The `McpComplianceTransport` wrapper hides the underlying `StdioClientTransport`,
// which exposes `stderr` for debug logging. We need to unwrap it to attach the listener.

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const underlyingTransport =
transport instanceof XcodeMcpBridgeFixTransport
? // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-type-assertion
(transport as any).transport
transport instanceof McpComplianceTransport
? transport.transport
: transport;

if (
Expand Down
Loading
Loading