mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-11 14:40:52 -07:00
fix(core): handle non-compliant mcpbridge responses from Xcode 26.3 (#18376)
This commit is contained in:
@@ -42,6 +42,7 @@ import { AuthProviderType } from '../config/config.js';
|
|||||||
import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js';
|
import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js';
|
||||||
import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js';
|
import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js';
|
||||||
import { DiscoveredMCPTool } from './mcp-tool.js';
|
import { DiscoveredMCPTool } from './mcp-tool.js';
|
||||||
|
import { XcodeMcpBridgeFixTransport } from './xcode-mcp-fix-transport.js';
|
||||||
|
|
||||||
import type { CallableTool, FunctionCall, Part, Tool } from '@google/genai';
|
import type { CallableTool, FunctionCall, Part, Tool } from '@google/genai';
|
||||||
import { basename } from 'node:path';
|
import { basename } from 'node:path';
|
||||||
@@ -1905,7 +1906,7 @@ export async function createTransport(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (mcpServerConfig.command) {
|
if (mcpServerConfig.command) {
|
||||||
const transport = new StdioClientTransport({
|
let transport: Transport = new StdioClientTransport({
|
||||||
command: mcpServerConfig.command,
|
command: mcpServerConfig.command,
|
||||||
args: mcpServerConfig.args || [],
|
args: mcpServerConfig.args || [],
|
||||||
env: sanitizeEnvironment(
|
env: sanitizeEnvironment(
|
||||||
@@ -1928,14 +1929,38 @@ export async function createTransport(
|
|||||||
cwd: mcpServerConfig.cwd,
|
cwd: mcpServerConfig.cwd,
|
||||||
stderr: 'pipe',
|
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);
|
||||||
|
}
|
||||||
|
|
||||||
if (debugMode) {
|
if (debugMode) {
|
||||||
transport.stderr!.on('data', (data) => {
|
// The `XcodeMcpBridgeFixTransport` wrapper hides the underlying `StdioClientTransport`,
|
||||||
const stderrStr = data.toString().trim();
|
// which exposes `stderr` for debug logging. We need to unwrap it to attach the listener.
|
||||||
debugLogger.debug(
|
|
||||||
`[DEBUG] [MCP STDERR (${mcpServerName})]: `,
|
const underlyingTransport =
|
||||||
stderrStr,
|
transport instanceof XcodeMcpBridgeFixTransport
|
||||||
);
|
? // eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
});
|
(transport as any).transport
|
||||||
|
: transport;
|
||||||
|
|
||||||
|
if (
|
||||||
|
underlyingTransport instanceof StdioClientTransport &&
|
||||||
|
underlyingTransport.stderr
|
||||||
|
) {
|
||||||
|
underlyingTransport.stderr.on('data', (data) => {
|
||||||
|
const stderrStr = data.toString().trim();
|
||||||
|
debugLogger.debug(
|
||||||
|
`[DEBUG] [MCP STDERR (${mcpServerName})]: `,
|
||||||
|
stderrStr,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return transport;
|
return transport;
|
||||||
}
|
}
|
||||||
|
|||||||
120
packages/core/src/tools/xcode-mcp-fix-transport.test.ts
Normal file
120
packages/core/src/tools/xcode-mcp-fix-transport.test.ts
Normal file
@@ -0,0 +1,120 @@
|
|||||||
|
/**
|
||||||
|
* @license
|
||||||
|
* Copyright 2026 Google LLC
|
||||||
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
|
*/
|
||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { EventEmitter } from 'node:events';
|
||||||
|
import { XcodeMcpBridgeFixTransport } from './xcode-mcp-fix-transport.js';
|
||||||
|
import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js';
|
||||||
|
import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types.js';
|
||||||
|
|
||||||
|
// Mock Transport that simulates the mcpbridge behavior
|
||||||
|
class MockBadMcpBridgeTransport extends EventEmitter implements Transport {
|
||||||
|
onclose?: () => void;
|
||||||
|
onerror?: (error: Error) => void;
|
||||||
|
onmessage?: (message: JSONRPCMessage) => void;
|
||||||
|
|
||||||
|
async start() {}
|
||||||
|
async close() {}
|
||||||
|
async send(_message: JSONRPCMessage) {}
|
||||||
|
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
emitMessage(msg: any) {
|
||||||
|
this.onmessage?.(msg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('Xcode MCP Bridge Fix', () => {
|
||||||
|
it('intercepts and fixes the non-compliant mcpbridge response', async () => {
|
||||||
|
const mockTransport = new MockBadMcpBridgeTransport();
|
||||||
|
const fixTransport = new XcodeMcpBridgeFixTransport(mockTransport);
|
||||||
|
|
||||||
|
// We need to capture what the fixTransport emits to its listeners
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
const messages: any[] = [];
|
||||||
|
fixTransport.onmessage = (msg) => {
|
||||||
|
messages.push(msg);
|
||||||
|
};
|
||||||
|
|
||||||
|
await fixTransport.start();
|
||||||
|
|
||||||
|
// SCENARIO 1: Bad response from Xcode
|
||||||
|
// It has `content` stringified JSON, but misses `structuredContent`
|
||||||
|
const badPayload = {
|
||||||
|
jsonrpc: '2.0',
|
||||||
|
id: 1,
|
||||||
|
result: {
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: 'text',
|
||||||
|
text: JSON.stringify({
|
||||||
|
windows: [{ title: 'HelloWorld', path: '/path/to/project' }],
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
// Missing: structuredContent
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
mockTransport.emitMessage(badPayload);
|
||||||
|
|
||||||
|
// Verify the message received by the client (listener of fixTransport)
|
||||||
|
const fixedMsg = messages.find((m) => m.id === 1);
|
||||||
|
expect(fixedMsg).toBeDefined();
|
||||||
|
expect(fixedMsg.result.structuredContent).toBeDefined();
|
||||||
|
expect(fixedMsg.result.structuredContent.windows[0].title).toBe(
|
||||||
|
'HelloWorld',
|
||||||
|
);
|
||||||
|
|
||||||
|
// SCENARIO 2: Good response (should be untouched)
|
||||||
|
const goodPayload = {
|
||||||
|
jsonrpc: '2.0',
|
||||||
|
id: 2,
|
||||||
|
result: {
|
||||||
|
content: [{ type: 'text', text: 'normal text' }],
|
||||||
|
structuredContent: { some: 'data' },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
mockTransport.emitMessage(goodPayload);
|
||||||
|
|
||||||
|
const goodMsg = messages.find((m) => m.id === 2);
|
||||||
|
expect(goodMsg).toBeDefined();
|
||||||
|
expect(goodMsg.result.structuredContent).toEqual({ some: 'data' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores responses that cannot be parsed as JSON', async () => {
|
||||||
|
const mockTransport = new MockBadMcpBridgeTransport();
|
||||||
|
const fixTransport = new XcodeMcpBridgeFixTransport(mockTransport);
|
||||||
|
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
const messages: any[] = [];
|
||||||
|
fixTransport.onmessage = (msg) => {
|
||||||
|
messages.push(msg);
|
||||||
|
};
|
||||||
|
|
||||||
|
await fixTransport.start();
|
||||||
|
|
||||||
|
const nonJsonPayload = {
|
||||||
|
jsonrpc: '2.0',
|
||||||
|
id: 3,
|
||||||
|
result: {
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: 'text',
|
||||||
|
text: "Just some plain text that isn't JSON",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
mockTransport.emitMessage(nonJsonPayload);
|
||||||
|
|
||||||
|
const msg = messages.find((m) => m.id === 3);
|
||||||
|
expect(msg).toBeDefined();
|
||||||
|
expect(msg.result.structuredContent).toBeUndefined();
|
||||||
|
expect(msg.result.content[0].text).toBe(
|
||||||
|
"Just some plain text that isn't JSON",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
101
packages/core/src/tools/xcode-mcp-fix-transport.ts
Normal file
101
packages/core/src/tools/xcode-mcp-fix-transport.ts
Normal file
@@ -0,0 +1,101 @@
|
|||||||
|
/**
|
||||||
|
* @license
|
||||||
|
* Copyright 2025 Google LLC
|
||||||
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
|
*/
|
||||||
|
|
||||||
|
import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js';
|
||||||
|
import type {
|
||||||
|
JSONRPCMessage,
|
||||||
|
JSONRPCResponse,
|
||||||
|
} from '@modelcontextprotocol/sdk/types.js';
|
||||||
|
import { EventEmitter } from 'node:events';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A wrapper transport that intercepts messages from Xcode's mcpbridge and fixes
|
||||||
|
* non-compliant responses.
|
||||||
|
*
|
||||||
|
* Issue: Xcode 26.3's mcpbridge returns tool results in `content` but misses
|
||||||
|
* `structuredContent` when the tool has an output schema.
|
||||||
|
*
|
||||||
|
* Fix: Parse the text content as JSON and populate `structuredContent`.
|
||||||
|
*/
|
||||||
|
export class XcodeMcpBridgeFixTransport
|
||||||
|
extends EventEmitter
|
||||||
|
implements Transport
|
||||||
|
{
|
||||||
|
constructor(private readonly transport: Transport) {
|
||||||
|
super();
|
||||||
|
|
||||||
|
// Forward messages from the underlying transport
|
||||||
|
this.transport.onmessage = (message) => {
|
||||||
|
this.handleMessage(message);
|
||||||
|
};
|
||||||
|
|
||||||
|
this.transport.onclose = () => {
|
||||||
|
this.onclose?.();
|
||||||
|
};
|
||||||
|
|
||||||
|
this.transport.onerror = (error) => {
|
||||||
|
this.onerror?.(error);
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// Transport interface implementation
|
||||||
|
onclose?: () => void;
|
||||||
|
onerror?: (error: Error) => void;
|
||||||
|
onmessage?: (message: JSONRPCMessage) => void;
|
||||||
|
|
||||||
|
async start(): Promise<void> {
|
||||||
|
await this.transport.start();
|
||||||
|
}
|
||||||
|
|
||||||
|
async close(): Promise<void> {
|
||||||
|
await this.transport.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
async send(message: JSONRPCMessage): Promise<void> {
|
||||||
|
await this.transport.send(message);
|
||||||
|
}
|
||||||
|
|
||||||
|
private handleMessage(message: JSONRPCMessage) {
|
||||||
|
if (this.isJsonResponse(message)) {
|
||||||
|
this.fixStructuredContent(message);
|
||||||
|
}
|
||||||
|
this.onmessage?.(message);
|
||||||
|
}
|
||||||
|
|
||||||
|
private isJsonResponse(message: JSONRPCMessage): message is JSONRPCResponse {
|
||||||
|
return 'result' in message || 'error' in message;
|
||||||
|
}
|
||||||
|
|
||||||
|
private fixStructuredContent(response: JSONRPCResponse) {
|
||||||
|
if (!('result' in response)) return;
|
||||||
|
|
||||||
|
// We can cast because we verified 'result' is in response,
|
||||||
|
// but TS might still be picky if the type is a strict union.
|
||||||
|
// Let's treat it safely.
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
const result = response.result as any;
|
||||||
|
|
||||||
|
// Check if we have content but missing structuredContent
|
||||||
|
if (
|
||||||
|
result.content &&
|
||||||
|
Array.isArray(result.content) &&
|
||||||
|
result.content.length > 0 &&
|
||||||
|
!result.structuredContent
|
||||||
|
) {
|
||||||
|
const firstItem = result.content[0];
|
||||||
|
if (firstItem.type === 'text' && typeof firstItem.text === 'string') {
|
||||||
|
try {
|
||||||
|
// Attempt to parse the text as JSON
|
||||||
|
const parsed = JSON.parse(firstItem.text);
|
||||||
|
// If successful, populate structuredContent
|
||||||
|
result.structuredContent = parsed;
|
||||||
|
} catch (_) {
|
||||||
|
// Ignored: Content is likely plain text, not JSON.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user