mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(mcp): answer a failed tool with a code, not the exception text
A tool that threw was answered by the SDK with error.message, so a database failure reached the model and the chat as Prisma's prose: the query, the column names, sometimes a value from the record. The tool call now returns a fixed sentence, the code mcp.tool.failed and the request id; the exception goes to the wide event, through its redaction, or to stderr on the stdio entrypoint.
This commit is contained in:
1 parent
c402e23656
commit
d78bcf1937
3 files changed
+138
-1
No files matched your search
@@ -0,0 +1,79 @@
|
||||
/**
|
||||
* A tool that throws answers with a fixed sentence and a code, never with the
|
||||
* exception's own text.
|
||||
*
|
||||
* Without the wrap the SDK hands the model `error.message`, which for a
|
||||
* database failure is Prisma's prose: the query, the columns, sometimes a value
|
||||
* from the record. The detail belongs in the operator's log.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
const event = vi.hoisted(() => ({
|
||||
setError: vi.fn(),
|
||||
getRequestId: () => "req-123",
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
ecgRecording: {
|
||||
findMany: vi.fn(async () => {
|
||||
throw new Error(
|
||||
"Invalid `prisma.ecgRecording.findMany()` invocation: column ecg_recordings.waveform_encrypted does not exist (value: 4711)",
|
||||
);
|
||||
}),
|
||||
},
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/logging/context", () => ({
|
||||
annotate: vi.fn(),
|
||||
getEvent: () => event,
|
||||
}));
|
||||
|
||||
process.env.APP_URL = "http://localhost";
|
||||
|
||||
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
|
||||
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
|
||||
import { createMcpServer } from "../server";
|
||||
import { MCP_TOOL_FAILED } from "../tool-failure";
|
||||
import type { McpAuthContext } from "../auth";
|
||||
|
||||
const ctx: McpAuthContext = {
|
||||
userId: "u-1",
|
||||
tokenId: "t-1",
|
||||
scopes: ["health:read"],
|
||||
binding: "u-1:t-1",
|
||||
canRead: true,
|
||||
canWrite: false,
|
||||
};
|
||||
|
||||
describe("a failing tool", () => {
|
||||
it("tells the model a fixed sentence, a code and the request id, and logs the cause", async () => {
|
||||
const server = createMcpServer(ctx);
|
||||
const client = new Client({ name: "failure-test", version: "1.0.0" });
|
||||
const [clientSide, serverSide] = InMemoryTransport.createLinkedPair();
|
||||
await Promise.all([server.connect(serverSide), client.connect(clientSide)]);
|
||||
|
||||
const result = (await client.callTool({
|
||||
name: "get_ecg_recordings",
|
||||
arguments: {},
|
||||
})) as { isError?: boolean; content: Array<{ text: string }> };
|
||||
await client.close();
|
||||
await server.close();
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
const text = result.content.map((c) => c.text).join("\n");
|
||||
expect(text).not.toMatch(/prisma|invocation|column|waveform|4711/i);
|
||||
const body = JSON.parse(text) as {
|
||||
error: { code: string; message: string; requestId?: string };
|
||||
};
|
||||
expect(body.error.code).toBe(MCP_TOOL_FAILED);
|
||||
expect(body.error.requestId).toBe("req-123");
|
||||
expect(body.error.message).toContain("get_ecg_recordings");
|
||||
|
||||
// The cause went to the event, which redacts it on the way to the log.
|
||||
expect(event.setError).toHaveBeenCalledWith(expect.any(Error));
|
||||
expect((event.setError.mock.calls[0][0] as Error).message).toContain(
|
||||
"findMany",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -27,6 +27,7 @@ import { MCP_RESOURCES, MCP_RESOURCE_TEMPLATES } from "./resources";
|
||||
import { MCP_PROMPTS } from "./prompts";
|
||||
import { resolveBaseOrigin } from "./oauth/config";
|
||||
import { advertiseToolSchemasAs2020 } from "./schema-dialect";
|
||||
import { toolFailure } from "./tool-failure";
|
||||
|
||||
export const MCP_SERVER_NAME = "healthlog";
|
||||
export const MCP_SERVER_VERSION = "1.0.0";
|
||||
@@ -102,7 +103,12 @@ export function createMcpServer(ctx: McpAuthContext): McpServer {
|
||||
...(tool.outputShape ? { outputSchema: tool.outputShape } : {}),
|
||||
},
|
||||
async (args: Record<string, unknown>) => {
|
||||
const result = await tool.run(ctx, args ?? {});
|
||||
let result: unknown;
|
||||
try {
|
||||
result = await tool.run(ctx, args ?? {});
|
||||
} catch (error) {
|
||||
return toolFailure(tool.name, error);
|
||||
}
|
||||
const content = [
|
||||
{ type: "text" as const, text: JSON.stringify(result) },
|
||||
];
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
/**
|
||||
* What a model is told when a tool fails, and what the operator is told.
|
||||
*
|
||||
* A tool that threw was answered by the MCP SDK with the exception's own
|
||||
* message as the result text, so a database failure reached the model, and
|
||||
* through it the person's chat, as Prisma's prose: the query shape, the
|
||||
* column names, sometimes a value from the record. That text is for the
|
||||
* operator. The model gets a fixed sentence it can relay, a stable code to
|
||||
* branch on, and the request id that finds the event in the log; the
|
||||
* exception goes to the wide event, through the same redaction every other
|
||||
* error takes.
|
||||
*/
|
||||
import { annotate, getEvent } from "@/lib/logging/context";
|
||||
|
||||
/** The code a failed tool call carries in its MCP result (not an HTTP `meta.errorCode`). */
|
||||
export const MCP_TOOL_FAILED = "mcp.tool.failed";
|
||||
|
||||
export function toolFailure(
|
||||
toolName: string,
|
||||
error: unknown,
|
||||
): {
|
||||
isError: true;
|
||||
content: Array<{ type: "text"; text: string }>;
|
||||
} {
|
||||
const evt = getEvent();
|
||||
const requestId = evt?.getRequestId() ?? null;
|
||||
if (evt) {
|
||||
evt.setError(error);
|
||||
annotate({
|
||||
action: { name: "mcp.tool.failed" },
|
||||
meta: { tool: toolName },
|
||||
});
|
||||
} else {
|
||||
// The stdio entrypoint runs outside a request, and stderr is its log.
|
||||
console.error(`HealthLog MCP: tool ${toolName} failed`, error);
|
||||
}
|
||||
return {
|
||||
isError: true,
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: JSON.stringify({
|
||||
error: {
|
||||
code: MCP_TOOL_FAILED,
|
||||
message: `The ${toolName} tool could not complete this request. Try again later; if it keeps failing, the operator can find the failure by its request id.`,
|
||||
...(requestId ? { requestId } : {}),
|
||||
},
|
||||
}),
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user