Fix four bugs in MCP tool schema and result handling

common/src/mcp/__tests__/mcp-content-mapping.test.ts

@@ -0,0 +1,102 @@
+import { describe, test, expect } from 'bun:test'
+
+import { mcpContentToToolResultOutputs } from '../client'
+
+/**
+ * Regression tests for MCP tool-result content mapping.
+ *
+ * Tool results live in message history and are replayed into every later
+ * prompt build, and the AI SDK base64-decodes file-part data at prompt
+ * build. Text content therefore never travels as media: prose stored as
+ * media died with "The string contains invalid characters" on every
+ * subsequent turn, permanently, because the poisoned message replays from
+ * history.
+ */
+describe('mcpContentToToolResultOutputs resources', () => {
+  /**
+   * Given: an MCP resource whose contents are plain text.
+   * When: it is mapped.
+   * Then: the output is a json value carrying that text - never media.
+   */
+  test('maps text resource to json value not media', () => {
+    const outputs = mcpContentToToolResultOutputs([
+      {
+        type: 'resource',
+        resource: {
+          uri: 'file:///notes.txt',
+          mimeType: 'text/plain',
+          text: 'Resource 1: This is a plain text resource.',
+        },
+      },
+    ] as never)
+
+    expect(outputs).toEqual([
+      {
+        type: 'json',
+        value: 'Resource 1: This is a plain text resource.',
+      },
+    ])
+  })
+
+  /**
+   * Given: an MCP resource carrying binary image data.
+   * When: it is mapped.
+   * Then: the output stays media with the server's mime type, because
+   *   every provider path accepts image file parts.
+   */
+  test('keeps image resource as media with server mime type', () => {
+    const outputs = mcpContentToToolResultOutputs([
+      {
+        type: 'resource',
+        resource: {
+          uri: 'file:///logo.png',
+          mimeType: 'image/png',
+          blob: 'aGVsbG8=',
+        },
+      },
+    ] as never)
+
+    expect(outputs).toHaveLength(1)
+    expect(outputs[0].type).toBe('media')
+    expect((outputs[0] as { mediaType?: string }).mediaType).toBe('image/png')
+  })
+
+  /**
+   * Given: an MCP resource carrying non-image binary data.
+   * When: it is mapped.
+   * Then: the output is descriptive text, not media - media here killed
+   *   the OpenAI-compatible converter at prompt build (session death).
+   */
+  test('maps non-image binary resource to descriptive text not media', () => {
+    const outputs = mcpContentToToolResultOutputs([
+      {
+        type: 'resource',
+        resource: {
+          uri: 'file:///archive.gz',
+          mimeType: 'application/gzip',
+          blob: 'aGVsbG8=',
+        },
+      },
+    ] as never)
+
+    expect(outputs[0].type).toBe('json')
+
+    const value = (outputs[0] as { value: string }).value
+    expect(value).toContain('application/gzip')
+    expect(value).toContain('not displayable')
+  })
+
+  /**
+   * Given: an ordinary MCP text content block (no resource involved).
+   * When: it is mapped.
+   * Then: it stays a json value - the extraction must not alter the
+   *   pre-existing text mapping.
+   */
+  test('maps plain text content to json value', () => {
+    const outputs = mcpContentToToolResultOutputs([
+      { type: 'text', text: 'Echo: hello' },
+    ] as never)
+
+    expect(outputs).toEqual([{ type: 'json', value: 'Echo: hello' }])
+  })
+})

common/src/mcp/client.ts

@@ -181,18 +181,14 @@ function getResourceData(
   return ''
 }
 
-export async function callMCPTool(
-  clientId: string,
-  ...args: Parameters<typeof Client.prototype.callTool>
-): Promise<ToolResultOutput[]> {
-  const client = runningClients[clientId]
-  if (!client) {
-    throw new Error(`callTool: client not found with id: ${clientId}`)
-  }
-  const callResult = await client.callTool(...args)
-  const result = callResult as CallToolResult
-  const content = result.content
-
+/**
+ * Convert MCP tool-result content blocks into codebuff tool-result outputs.
+ * Pure function (no client access) so conversion rules are testable in
+ * isolation. No behavior change from the previous inline map.
+ */
+export function mcpContentToToolResultOutputs(
+  content: CallToolResult['content'],
+): ToolResultOutput[] {
   return content.map((c: (typeof content)[number]) => {
     if (c.type === 'text') {
       return {
@@ -215,10 +211,34 @@ export async function callMCPTool(
       } satisfies ToolResultOutput
     }
     if (c.type === 'resource') {
+      // A resource with text contents is text, not media. Wrapping prose as
+      // media makes the AI SDK base64-decode it when rebuilding the prompt on
+      // every later turn, which dies with "The string contains invalid
+      // characters" forever, since the poisoned message replays from history.
+      if ('text' in c.resource) {
+        return {
+          type: 'json',
+          value: c.resource.text,
+        } satisfies ToolResultOutput
+      }
+      const mimeType = c.resource.mimeType ?? 'application/octet-stream'
+      // Only images stay media: every provider path (including the
+      // OpenAI-compatible chat converter used by GLM) accepts image file
+      // parts but throws on anything else — and a thrown converter poisons
+      // the whole session, since the message replays on every later turn.
+      if (mimeType.startsWith('image/')) {
+        return {
+          type: 'media',
+          data: getResourceData(c.resource),
+          mediaType: mimeType,
+        } satisfies ToolResultOutput
+      }
+      // Other binary resources (gzip, PDF, ...): surface metadata instead of
+      // undecodable bytes.
+      const blobData = getResourceData(c.resource)
       return {
-        type: 'media',
-        data: getResourceData(c.resource),
-        mediaType: c.resource.mimeType ?? 'text/plain',
+        type: 'json',
+        value: `[Binary resource ${c.resource.uri}: ${mimeType}, ~${Math.round((blobData.length * 3) / 4)} bytes, not displayable]`,
       } satisfies ToolResultOutput
     }
     const fallbackValue =
@@ -231,3 +251,18 @@ export async function callMCPTool(
     } satisfies ToolResultOutput
   })
 }
+
+export async function callMCPTool(
+  clientId: string,
+  ...args: Parameters<typeof Client.prototype.callTool>
+): Promise<ToolResultOutput[]> {
+  const client = runningClients[clientId]
+  if (!client) {
+    throw new Error(`callTool: client not found with id: ${clientId}`)
+  }
+  const callResult = await client.callTool(...args)
+  const result = callResult as CallToolResult
+  const content = result.content
+
+  return mcpContentToToolResultOutputs(content)
+}

packages/agent-runtime/src/__tests__/mcp-schema-store.test.ts

@@ -0,0 +1,105 @@
+import { describe, test, expect } from 'bun:test'
+import { z } from 'zod/v4'
+
+import { getMCPToolData } from '../mcp'
+import { MCP_TOOL_SEPARATOR } from '../mcp-constants'
+
+/**
+ * Regression tests for MCP tool-schema storage.
+ *
+ * Tool definitions returned by getMCPToolData are persisted in run/session
+ * state, which is snapshotted and JSON-serialized on every turn. Schemas
+ * must be stored verbatim: storing converted live zod instances instead
+ * round-trips to def/shape internals and can carry cycles that detonate
+ * JSON.stringify over the whole run state ("cannot serialize cyclic
+ * structures", session death from turn 2 onward).
+ */
+describe('getMCPToolData schema storage', () => {
+  /**
+   * Given: one MCP server reporting one tool with a JSON Schema.
+   * When: getMCPToolData stores it.
+   * Then: the stored schema round-trips through JSON as the exact schema
+   *   the server sent - the persisted-state contract.
+   */
+  test('stores the server JSON Schema verbatim and JSON round-trips it', async () => {
+    const serverSchema = {
+      type: 'object',
+      properties: {
+        location: { type: 'string', enum: ['NYC', 'LA'] },
+        units: { type: 'string', description: 'metric or imperial' },
+      },
+      required: ['location'],
+    }
+    const writeTo: Record<string, any> = {}
+
+    await getMCPToolData({
+      toolNames: ['weather/get_forecast'],
+      mcpServers: {
+        weather: { command: 'echo', args: [] },
+      } as never,
+      writeTo: writeTo as never,
+      requestMcpToolData: async () => [
+        {
+          name: 'get_forecast',
+          description: 'Get the forecast',
+          inputSchema: serverSchema,
+        },
+      ],
+    })
+
+    const stored = writeTo[`weather${MCP_TOOL_SEPARATOR}get_forecast`]
+    const roundTripped = JSON.parse(JSON.stringify(stored.inputSchema))
+    expect(roundTripped).toEqual(serverSchema)
+  })
+
+  /**
+   * Given: two servers each reporting one tool with a distinct schema.
+   * When: getMCPToolData stores both.
+   * Then: each server's tool carries its own schema, namespaced with the
+   *   internal separator, verbatim and JSON-serializable.
+   */
+  test('stores distinct schemas per server without conversion', async () => {
+    const schemaA = { type: 'object', properties: { a: { type: 'number' } } }
+    const schemaB = { type: 'string' }
+    const writeTo: Record<string, any> = {}
+
+    await getMCPToolData({
+      toolNames: [],
+      mcpServers: {
+        alpha: { command: 'echo', args: [] },
+        beta: { command: 'echo', args: [] },
+      } as never,
+      writeTo: writeTo as never,
+      requestMcpToolData: async ({ toolNames }: { toolNames: unknown }) => {
+        void toolNames
+        return [
+          { name: 't1', description: 'A', inputSchema: schemaA },
+          { name: 't2', description: 'B', inputSchema: schemaB },
+        ]
+      },
+    })
+
+    const alphaStored = writeTo[`alpha${MCP_TOOL_SEPARATOR}t1`]
+    const betaStored = writeTo[`beta${MCP_TOOL_SEPARATOR}t2`]
+    expect(JSON.parse(JSON.stringify(alphaStored.inputSchema))).toEqual(schemaA)
+    expect(JSON.parse(JSON.stringify(betaStored.inputSchema))).toEqual(schemaB)
+    expect(betaStored.description).toBe('B')
+  })
+
+  /**
+   * Given: the old implementation stored convertJsonSchemaToZod output.
+   * When: a live zod instance is round-tripped through JSON.
+   * Then: the result is zod internals (def/shape), not the server schema -
+   *   the failure mode this contract guards against, kept here as a
+   *   characterization so a regression to zod storage cannot pass silently.
+   */
+  test('keeps the zod storage failure mode characterized as non passing', () => {
+    const serverSchema = { type: 'object', properties: { q: { type: 'string' } } }
+    const zodInstance = z.object({ q: z.string() })
+
+    const roundTripped = JSON.parse(JSON.stringify(zodInstance))
+
+    expect(roundTripped).not.toEqual(serverSchema)
+    expect(roundTripped.def).toBeDefined()
+  })
+})

packages/agent-runtime/src/mcp.ts

@@ -1,5 +1,4 @@
 import { getErrorObject } from '@codebuff/common/util/error'
-import { convertJsonSchemaToZod } from 'zod-from-json-schema'
 
 import { MCP_TOOL_SEPARATOR } from './mcp-constants'
 
@@ -55,12 +54,21 @@ export async function getMCPToolData(
           })
 
           for (const { name, description, inputSchema } of mcpData) {
+            // Store the raw JSON Schema from the server, NOT the converted Zod
+            // schema. Tool definitions are persisted in run state / session
+            // state and must stay JSON-serializable; Zod instances are cyclic
+            // and make any JSON.stringify over that state detonate. Consumers
+            // convert at point of use (ensureZodSchema / toTokenCountInputSchema).
             writeTo[mcpName + MCP_TOOL_SEPARATOR + name] = {
-              inputSchema: convertJsonSchemaToZod(inputSchema as any) as any,
+              inputSchema: inputSchema as {},
               endsAgentStep: true,
               description,
             }
           }
+          logger?.debug(
+            { mcpServer: mcpName, toolCount: mcpData.length },
+            `Loaded ${mcpData.length} tool(s) from MCP server "${mcpName}".`,
+          )
         } catch (error) {
           // A failed MCP server (e.g. a stdio server that can't be spawned)
           // should disable just its own tools, not abort the whole turn. The