From 647dd6042f7b240518acc009d5dceebac9993aaa Mon Sep 17 00:00:00 2001 From: Developing-Gamer Date: Tue, 9 Jun 2026 10:30:00 -0700 Subject: [PATCH] Add support for handling HEAD requests in MCP tool routes and improve error handling for malformed route names. Update tests to cover new functionality and reject invalid query parameters. --- apps/skills/src/app/[toolName]/route.ts | 6 +-- apps/skills/src/mcp-wrapper.test.ts | 52 ++++++++++++++++++++- apps/skills/src/mcp-wrapper.ts | 60 +++++++++++++++++++------ 3 files changed, 100 insertions(+), 18 deletions(-) diff --git a/apps/skills/src/app/[toolName]/route.ts b/apps/skills/src/app/[toolName]/route.ts index aacb3b4c7..53ae7463f 100644 --- a/apps/skills/src/app/[toolName]/route.ts +++ b/apps/skills/src/app/[toolName]/route.ts @@ -1,4 +1,4 @@ -import { handleMcpToolOptions, handleMcpToolRoute } from "@/mcp-wrapper"; +import { handleMcpToolHead, handleMcpToolOptions, handleMcpToolRoute } from "@/mcp-wrapper"; export const dynamic = "force-dynamic"; @@ -6,8 +6,8 @@ export async function GET(req: Request) { return await handleMcpToolRoute(req); } -export async function HEAD(req: Request) { - return await handleMcpToolRoute(req); +export function HEAD() { + return handleMcpToolHead(); } export function OPTIONS() { diff --git a/apps/skills/src/mcp-wrapper.test.ts b/apps/skills/src/mcp-wrapper.test.ts index 6f3f5817f..376710329 100644 --- a/apps/skills/src/mcp-wrapper.test.ts +++ b/apps/skills/src/mcp-wrapper.test.ts @@ -1,6 +1,6 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; -import { buildMcpToolArguments, getAvailableRouteNames, getMcpEndpointUrl, resolveMcpToolRoute } from "./mcp-wrapper"; +import { buildMcpToolArguments, getAvailableRouteNames, getMcpEndpointUrl, handleMcpToolRoute, resolveMcpToolRoute } from "./mcp-wrapper"; function restoreEnvVariable(name: string, value: string | undefined) { if (value == null) { @@ -111,6 +111,23 @@ describe("skill-site MCP wrapper", () => { `); }); + it("rejects arrays for object query parameters", () => { + const tool = { + name: "search", + inputSchema: { + type: "object", + properties: { + filters: { type: "object" }, + }, + }, + }; + const params = new URLSearchParams({ + filters: "[]", + }); + + expect(() => buildMcpToolArguments(tool, params)).toThrow("must be a JSON object"); + }); + it("infers the sibling MCP endpoint from local and production skill URLs", () => { const previousHexclaveMcpBaseUrl = process.env.HEXCLAVE_MCP_BASE_URL; const previousStackMcpBaseUrl = process.env.STACK_MCP_BASE_URL; @@ -120,9 +137,40 @@ describe("skill-site MCP wrapper", () => { try { expect(getMcpEndpointUrl(new Request("http://localhost:8145/ask")).toString()).toBe("http://localhost:8144/mcp"); expect(getMcpEndpointUrl(new Request("https://skill.hexclave.com/ask")).toString()).toBe("https://mcp.hexclave.com/mcp"); + expect(() => getMcpEndpointUrl(new Request("https://skill.evil.example/ask"))).toThrow("Unable to derive MCP endpoint URL"); } finally { restoreEnvVariable("HEXCLAVE_MCP_BASE_URL", previousHexclaveMcpBaseUrl); restoreEnvVariable("STACK_MCP_BASE_URL", previousStackMcpBaseUrl); } }); + + it("does not call MCP tools for HEAD requests", async () => { + const previousFetch = globalThis.fetch; + const previousHexclaveMcpBaseUrl = process.env.HEXCLAVE_MCP_BASE_URL; + process.env.HEXCLAVE_MCP_BASE_URL = "https://mcp.hexclave.com/mcp"; + + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + const body = typeof init?.body === "string" ? JSON.parse(init.body) : null; + expect(body?.method).toBe("tools/list"); + + return new Response(`data: ${JSON.stringify({ + result: { + tools: [askTool], + }, + jsonrpc: "2.0", + id: 1, + })}`); + }); + + globalThis.fetch = fetchMock; + + try { + const response = await handleMcpToolRoute(new Request("https://skill.hexclave.com/ask", { method: "HEAD" })); + expect(response.status).toBe(200); + expect(fetchMock).toHaveBeenCalledTimes(1); + } finally { + globalThis.fetch = previousFetch; + restoreEnvVariable("HEXCLAVE_MCP_BASE_URL", previousHexclaveMcpBaseUrl); + } + }); }); diff --git a/apps/skills/src/mcp-wrapper.ts b/apps/skills/src/mcp-wrapper.ts index 636f53dfe..a3c018b6e 100644 --- a/apps/skills/src/mcp-wrapper.ts +++ b/apps/skills/src/mcp-wrapper.ts @@ -10,6 +10,8 @@ const TOOL_ROUTE_HEADERS = { "Access-Control-Allow-Headers": "*", }; +const MCP_RPC_TIMEOUT_MS = 15_000; + type JsonRecord = Record; type McpTool = { @@ -45,7 +47,7 @@ class McpJsonRpcError extends Error { } function isRecord(value: unknown): value is JsonRecord { - return typeof value === "object" && value !== null; + return typeof value === "object" && value !== null && !Array.isArray(value); } function parseJsonFromMcpBody(body: string): unknown { @@ -83,16 +85,20 @@ function getConfiguredMcpEndpointUrl(): URL | null { return normalizeMcpEndpointUrl(new URL(configured)); } +function isLocalHostname(hostname: string): boolean { + return hostname === "localhost" || hostname === "127.0.0.1" || hostname === "::1" || hostname.endsWith(".localhost"); +} + function getSiblingMcpUrl(req: Request): URL { const url = new URL(req.url); const sibling = new URL(url); if (sibling.hostname === "skill.hexclave.com") { sibling.hostname = "mcp.hexclave.com"; - } else if (sibling.hostname.startsWith("skill.")) { - sibling.hostname = `mcp.${sibling.hostname.slice("skill.".length)}`; - } else if (sibling.port.endsWith("45")) { + } else if (isLocalHostname(sibling.hostname) && sibling.port.endsWith("45")) { sibling.port = `${sibling.port.slice(0, -2)}44`; + } else { + throw new QueryArgumentError("Unable to derive MCP endpoint URL for this skill host."); } sibling.pathname = "/mcp"; @@ -110,15 +116,28 @@ async function mcpJsonRpc(endpointUrl: URL, method: string, params?: unknown): P ? { jsonrpc: "2.0", id: 1, method } : { jsonrpc: "2.0", id: 1, method, params }; - const response = await fetch(endpointUrl, { - method: "POST", - headers: MCP_RPC_HEADERS, - body: JSON.stringify(body), - }); + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), MCP_RPC_TIMEOUT_MS); + let response: Response; + try { + response = await fetch(endpointUrl, { + method: "POST", + headers: MCP_RPC_HEADERS, + body: JSON.stringify(body), + signal: controller.signal, + }); + } catch (error) { + if (error instanceof DOMException && error.name === "AbortError") { + throw new McpHttpError(504, `MCP HTTP timeout after ${MCP_RPC_TIMEOUT_MS}ms`); + } + throw error; + } finally { + clearTimeout(timeout); + } const text = await response.text(); if (!response.ok) { - throw new McpHttpError(response.status, `MCP HTTP error ${response.status}: ${text}`); + throw new McpHttpError(response.status, `MCP HTTP error ${response.status}`); } const parsed = parseJsonFromMcpBody(text); @@ -318,7 +337,7 @@ function applyQuestionAlias(values: Map, properties: Map { return textResponse(`Unknown MCP tool route "/${routeName}". Available routes: ${getAvailableRouteNames(tools).join(", ")}`, 404); } + if (req.method === "HEAD") { + return textResponse(""); + } + const response = await callMcpTool(endpointUrl, tool, new URL(req.url).searchParams); return textResponse(response.text, response.isError ? 502 : 200); } catch (error) { @@ -407,7 +437,7 @@ export async function handleMcpToolRoute(req: Request): Promise { } if (error instanceof McpJsonRpcError) { - return textResponse(error.message, errorStatusForMcpError(error)); + return textResponse(`MCP JSON-RPC error ${error.code}`, errorStatusForMcpError(error)); } if (error instanceof McpHttpError) { @@ -424,3 +454,7 @@ export function handleMcpToolOptions(): Response { headers: TOOL_ROUTE_HEADERS, }); } + +export function handleMcpToolHead(): Response { + return textResponse(""); +}