From bf8303370eeb9ede220152f03424d51ac86d1c93 Mon Sep 17 00:00:00 2001 From: dhananjai1729 Date: Thu, 19 Mar 2026 17:15:10 +0530 Subject: [PATCH] fix: address review feedback - fix env JSDoc, warn on dropped headers, await server close --- src/agents/mcp-sse.ts | 13 ++++++++++--- src/agents/pi-bundle-mcp-tools.test.ts | 4 +++- src/agents/pi-bundle-mcp-tools.ts | 8 +++++++- src/config/types.mcp.ts | 2 +- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/agents/mcp-sse.ts b/src/agents/mcp-sse.ts index 3a3bc8f0b44..86c50759105 100644 --- a/src/agents/mcp-sse.ts +++ b/src/agents/mcp-sse.ts @@ -11,7 +11,10 @@ function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } -function toStringRecord(value: unknown): Record | undefined { +function toStringRecord( + value: unknown, + warnDropped?: (key: string, entry: unknown) => void, +): Record | undefined { if (!isRecord(value)) { return undefined; } @@ -23,13 +26,17 @@ function toStringRecord(value: unknown): Record | undefined { if (typeof entry === "number" || typeof entry === "boolean") { return [key, String(entry)] as const; } + warnDropped?.(key, entry); return null; }) .filter((entry): entry is readonly [string, string] => entry !== null); return entries.length > 0 ? Object.fromEntries(entries) : undefined; } -export function resolveSseMcpServerLaunchConfig(raw: unknown): SseMcpServerLaunchResult { +export function resolveSseMcpServerLaunchConfig( + raw: unknown, + options?: { onDroppedHeader?: (key: string, value: unknown) => void }, +): SseMcpServerLaunchResult { if (!isRecord(raw)) { return { ok: false, reason: "server config must be an object" }; } @@ -53,7 +60,7 @@ export function resolveSseMcpServerLaunchConfig(raw: unknown): SseMcpServerLaunc ok: true, config: { url, - headers: toStringRecord(raw.headers), + headers: toStringRecord(raw.headers, options?.onDroppedHeader), }, }; } diff --git a/src/agents/pi-bundle-mcp-tools.test.ts b/src/agents/pi-bundle-mcp-tools.test.ts index 34c20555dca..d3b6c4a9868 100644 --- a/src/agents/pi-bundle-mcp-tools.test.ts +++ b/src/agents/pi-bundle-mcp-tools.test.ts @@ -188,7 +188,9 @@ describe("createBundleMcpToolRuntime", () => { await runtime.dispose(); } } finally { - httpServer.close(); + await new Promise((resolve, reject) => + httpServer.close((err) => (err ? reject(err) : resolve())), + ); } }); }); diff --git a/src/agents/pi-bundle-mcp-tools.ts b/src/agents/pi-bundle-mcp-tools.ts index 71b6b617673..9d3699e0472 100644 --- a/src/agents/pi-bundle-mcp-tools.ts +++ b/src/agents/pi-bundle-mcp-tools.ts @@ -153,7 +153,13 @@ function resolveTransport( } // Try SSE (url-based servers). - const sseLaunch = resolveSseMcpServerLaunchConfig(rawServer); + const sseLaunch = resolveSseMcpServerLaunchConfig(rawServer, { + onDroppedHeader: (key) => { + logWarn( + `bundle-mcp: server "${serverName}": header "${key}" has an unsupported value type and was ignored.`, + ); + }, + }); if (sseLaunch.ok) { const headers: Record = { ...sseLaunch.config.headers, diff --git a/src/config/types.mcp.ts b/src/config/types.mcp.ts index d3f20a1db69..fcc5297434e 100644 --- a/src/config/types.mcp.ts +++ b/src/config/types.mcp.ts @@ -3,7 +3,7 @@ export type McpServerConfig = { command?: string; /** Stdio transport: arguments for the command. */ args?: string[]; - /** Environment variables passed to the server process (stdio) or as headers (SSE). */ + /** Environment variables passed to the server process (stdio only). */ env?: Record; /** Working directory for stdio server. */ cwd?: string;