From e7281c30f4750b271ea7e99e371265242fc6492d Mon Sep 17 00:00:00 2001 From: Dave Date: Wed, 30 Sep 2026 23:40:41 +0200 Subject: [PATCH] fix(giellie): guard non-JSON MCP responses and surface error.data Co-Authored-By: Claude Sonnet 5.5 --- src/lib/server/giellie-mcp-bridge.test.ts | 36 +++++++++++++++++++++++ src/lib/server/giellie-mcp-bridge.ts | 15 ++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/lib/server/giellie-mcp-bridge.test.ts b/src/lib/server/giellie-mcp-bridge.test.ts index 7c664d8..16f7c71 100644 --- a/src/lib/server/giellie-mcp-bridge.test.ts +++ b/src/lib/server/giellie-mcp-bridge.test.ts @@ -33,6 +33,18 @@ describe("listMcpTools", () => { }); }); +describe("listMcpTools non-JSON", () => { + it("rejects on a non-JSON response and does not poison the cache", async () => { + mcpPost.mockResolvedValueOnce(new Response("bad gateway", { status: 502 })); + await expect(listMcpTools(ORIGIN)).rejects.toThrow(/tools\/list failed/); + mcpPost.mockResolvedValueOnce( + NextResponse.json({ jsonrpc: "2.0", id: 1, result: { tools: [{ name: "orders.list" }] } }), + ); + expect(await listMcpTools(ORIGIN)).toEqual([{ name: "orders.list" }]); + expect(mcpPost).toHaveBeenCalledTimes(2); + }); +}); + describe("callMcpTool", () => { it("sends name + args + actor and unwraps the text envelope", async () => { mcpPost.mockResolvedValue( @@ -68,6 +80,30 @@ describe("callMcpTool", () => { expect(out.text).toContain("Order not found"); }); + it("returns isError for a non-JSON response", async () => { + mcpPost.mockResolvedValue(new Response("bad gateway", { status: 502 })); + const out = await callMcpTool(ORIGIN, "orders.get", {}, { id: "u1", name: "Dave" }); + expect(out.isError).toBe(true); + expect(out.text).toContain("502"); + }); + + it("includes JSON-RPC error.data (hints) in the error text", async () => { + mcpPost.mockResolvedValue( + NextResponse.json( + { + jsonrpc: "2.0", + id: 2, + error: { code: -32000, message: "Upstream 404", data: { hint: "use orders.find_by_order_number" } }, + }, + { status: 500 }, + ), + ); + const out = await callMcpTool(ORIGIN, "orders.get", { id: "x" }, { id: "u1", name: "Dave" }); + expect(out.isError).toBe(true); + expect(out.text).toContain("Upstream 404"); + expect(out.text).toContain("use orders.find_by_order_number"); + }); + it("throws a clear error when AGENT_API_KEY is missing", async () => { delete process.env.AGENT_API_KEY; await expect(callMcpTool(ORIGIN, "orders.get", {}, { id: "u1", name: "Dave" })).rejects.toThrow( diff --git a/src/lib/server/giellie-mcp-bridge.ts b/src/lib/server/giellie-mcp-bridge.ts index 85e8260..9411a16 100644 --- a/src/lib/server/giellie-mcp-bridge.ts +++ b/src/lib/server/giellie-mcp-bridge.ts @@ -32,7 +32,7 @@ async function rpc( params: Record, id: number, ): Promise<{ - error?: { code?: number; message?: string }; + error?: { code?: number; message?: string; data?: unknown }; result?: { tools?: McpTool[]; content?: Array<{ type: string; text?: string }> }; }> { const key = process.env.AGENT_API_KEY; @@ -43,7 +43,11 @@ async function rpc( body: JSON.stringify({ jsonrpc: "2.0", id, method, params }), }); const res = await mcpPost(req); - return (await res.json()) as Awaited>; + try { + return (await res.json()) as Awaited>; + } catch { + return { error: { message: `MCP ${res.status} non-JSON response` } }; + } } export async function listMcpTools(origin: string): Promise { @@ -64,7 +68,12 @@ export async function callMcpTool( // `actor` goes last so the model cannot spoof another user. const json = await rpc(origin, "tools/call", { name, arguments: { ...args, actor } }, 2); if (json.error) { - return { text: `Error: ${json.error.message ?? "tool call failed"}`, isError: true }; + const base = `Error: ${json.error.message ?? "tool call failed"}`; + const data = + json.error.data !== undefined && json.error.data !== null + ? `\n${(JSON.stringify(json.error.data) ?? "").slice(0, 4000)}` + : ""; + return { text: base + data, isError: true }; } const text = json.result?.content?.find((c) => c.type === "text")?.text ?? ""; return { text, isError: false };