diff --git a/__tests__/mcp.test.ts b/__tests__/mcp.test.ts index c5c023b..04cd56c 100644 --- a/__tests__/mcp.test.ts +++ b/__tests__/mcp.test.ts @@ -5,6 +5,7 @@ import { parseTunnelRegistry, summarizeToolSurface, HEAVY_TOOL_SURFACE, + buildHttpMcpServers, } from '../src/mcp.js'; import { TEAM } from '../src/team.js'; @@ -166,4 +167,32 @@ describe('mcp', () => { expect(s.heavyRoles).toBeGreaterThan(0); }); }); + + describe('buildHttpMcpServers', () => { + it('builds http configs for direct (non-gateway) servers, no bearer', () => { + const { servers, skipped } = buildHttpMcpServers(['github', 'linear'], {}); + expect(skipped).toEqual([]); + expect(Object.keys(servers)).toEqual(['github', 'linear']); + expect(servers.github.type).toBe('http'); + expect(servers.github.url).toBeTruthy(); + expect(servers.github.headers).toBeUndefined(); + }); + + it('injects the gateway bearer for gateway-hosted servers when the token is set', () => { + const { servers, skipped } = buildHttpMcpServers(['stripe'], { MCP_GATEWAY_TOKEN: 'tok123' }); + expect(skipped).toEqual([]); + expect(servers.stripe.headers).toEqual({ Authorization: 'Bearer tok123' }); + }); + + it('skips a gateway server (non-strict) when the token is missing, keeps direct servers', () => { + const { servers, skipped } = buildHttpMcpServers(['stripe', 'github'], {}); + expect(skipped).toEqual(['stripe']); + expect(servers.stripe).toBeUndefined(); + expect(servers.github).toBeTruthy(); + }); + + it('throws under FAB_MCP_STRICT when a gateway token is missing', () => { + expect(() => buildHttpMcpServers(['stripe'], { FAB_MCP_STRICT: '1' })).toThrow(/MCP_GATEWAY_TOKEN is not set/); + }); + }); }); diff --git a/src/mcp.ts b/src/mcp.ts index 0e5cc9a..03eaa28 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -223,6 +223,61 @@ export function resolveMcpServers(serverNames: string[]): { servers: McpServer[] return { servers, tools }; } +// ── Shared HTTP MCP config (claude-cli --mcp-config + sdk mcpServers) ── +// +// Both the claude-cli runtime (Claude Code's `--mcp-config` JSON) and the sdk +// runtime (the Agent SDK's `mcpServers` query option) need the same +// {type:'http', url, headers} map. Built once here so the gateway-bearer +// injection can't drift between the two transports. + +/** Servers that route through the mcp-gateway and need the shared bearer token. */ +const GATEWAY_HOSTED: ReadonlySet = new Set(['hubspot', 'gdrive', 'analytics', 'gcalendar', 'gcse', 'stripe']); + +/** An HTTP MCP server config — accepted by both Claude Code's `--mcp-config` and the Agent SDK's `mcpServers`. */ +export interface HttpMcpServer { + type: 'http'; + url: string; + headers?: Record; +} + +/** + * Resolve a role's MCP server names into the HTTP-config map shared by the + * claude-cli and sdk runtimes. Injects the gateway bearer for gateway-hosted + * servers; on a missing MCP_GATEWAY_TOKEN it throws under FAB_MCP_STRICT, else + * drops the server (returned in `skipped` so the caller can warn). + */ +export function buildHttpMcpServers( + serverNames: string[], + env: NodeJS.ProcessEnv, +): { servers: Record; skipped: string[] } { + const { servers: resolved } = resolveMcpServers(serverNames); + const gatewayToken = env.MCP_GATEWAY_TOKEN ?? ''; + const strict = env.FAB_MCP_STRICT === '1'; + const servers: Record = {}; + const skipped: string[] = []; + + for (const s of resolved) { + const entry: HttpMcpServer = { type: 'http', url: s.url }; + if (GATEWAY_HOSTED.has(s.name)) { + if (!gatewayToken) { + if (strict) { + throw new Error( + `MCP server "${s.name}" routes through the gateway but MCP_GATEWAY_TOKEN is not set. ` + + `Set the token, remove the server from the role's mcpServers list, or unset FAB_MCP_STRICT.`, + ); + } + skipped.push(s.name); + continue; + } + entry.headers = { Authorization: `Bearer ${gatewayToken}` }; + } + if (s.headers) entry.headers = { ...entry.headers, ...s.headers }; + servers[s.name] = entry; + } + + return { servers, skipped }; +} + /** * Get the full registry for display (e.g., help text, config commands). */ diff --git a/src/runtimes/claude-cli.ts b/src/runtimes/claude-cli.ts index 55e1e8a..ce7d4e7 100644 --- a/src/runtimes/claude-cli.ts +++ b/src/runtimes/claude-cli.ts @@ -10,7 +10,7 @@ import type { AgentEvent, FabState, TeamMember, TeamRole, UserEvent } from '../t import { TEAM } from '../team.js'; import { buildSystemPrompt } from '../prompts.js'; import { loadState, getPrimaryRepo } from '../state.js'; -import { resolveMcpServers } from '../mcp.js'; +import { buildHttpMcpServers } from '../mcp.js'; import { isTerminal, translateSdkMessage } from './sdk-events.js'; /** @@ -389,34 +389,16 @@ export function buildClaudeArgs(opts: BuildClaudeArgsOptions): string[] { return args; } -interface McpHttpEntry { - type: 'http'; - url: string; - headers?: Record; -} - -interface McpConfigShape { - mcpServers: Record; -} - -/** - * Servers that route through the mcp-gateway and need the shared bearer - * token. Matches `src/mcp.ts`'s `switchboardService()` set. Listed - * explicitly here so authentication injection doesn't - * depend on URL-prefix matching (which races with env-var resolution). - */ -const GATEWAY_HOSTED: ReadonlySet = new Set(['hubspot', 'gdrive', 'analytics', 'gcalendar', 'gcse', 'stripe']); - /** * Render the role's MCP server list into Claude Code's `--mcp-config` JSON * shape. Returns null when the role declares no servers or every server * was dropped (caller skips the `--mcp-config` flag entirely). * - * Gateway-routed servers (see {@link GATEWAY_HOSTED}) get - * `Authorization: Bearer ` injected. Third-party direct - * servers (github, linear, slack, notion, sentry, figma, hunter) pass - * through without fab-side auth headers — Claude Code handles those - * via its own credential store. + * Thin wrapper over {@link buildHttpMcpServers} (the shared resolver in + * `src/mcp.ts`): gateway-routed servers get `Authorization: Bearer + * ` injected; third-party direct servers (github, linear, + * slack, notion, sentry, figma, hunter) pass through without fab-side auth + * headers — Claude Code handles those via its own credential store. * * **Missing gateway token behaviour:** * - Default (`FAB_MCP_STRICT` unset): gateway servers are silently @@ -429,38 +411,7 @@ const GATEWAY_HOSTED: ReadonlySet = new Set(['hubspot', 'gdrive', 'analy export function buildMcpConfigJson(serverNames: string[], env: NodeJS.ProcessEnv): string | null { if (serverNames.length === 0) return null; - const { servers } = resolveMcpServers(serverNames); - if (servers.length === 0) return null; - - const gatewayToken = env.MCP_GATEWAY_TOKEN ?? ''; - const strict = env.FAB_MCP_STRICT === '1'; - const skipped: string[] = []; - - const mcpServers: Record = {}; - for (const server of servers) { - const entry: McpHttpEntry = { type: 'http', url: server.url }; - - if (GATEWAY_HOSTED.has(server.name)) { - if (!gatewayToken) { - if (strict) { - throw new Error( - `MCP server "${server.name}" routes through the gateway but MCP_GATEWAY_TOKEN is not set. ` + - `Set the token, remove the server from the role's mcpServers list, or unset FAB_MCP_STRICT to fall back to skip-with-warning.`, - ); - } - skipped.push(server.name); - continue; - } - entry.headers = { Authorization: `Bearer ${gatewayToken}` }; - } - - // Static headers declared in the registry (rare) flow through. - if (server.headers) { - entry.headers = { ...entry.headers, ...server.headers }; - } - - mcpServers[server.name] = entry; - } + const { servers, skipped } = buildHttpMcpServers(serverNames, env); if (skipped.length > 0) { process.stderr.write( @@ -469,10 +420,9 @@ export function buildMcpConfigJson(serverNames: string[], env: NodeJS.ProcessEnv ); } - if (Object.keys(mcpServers).length === 0) return null; + if (Object.keys(servers).length === 0) return null; - const shape: McpConfigShape = { mcpServers }; - return JSON.stringify(shape, null, 2); + return JSON.stringify({ mcpServers: servers }, null, 2); } // ── Internal helpers ─────────────────────────────────────────────────── diff --git a/src/runtimes/sdk.ts b/src/runtimes/sdk.ts index 9f74dcc..a58acc2 100644 --- a/src/runtimes/sdk.ts +++ b/src/runtimes/sdk.ts @@ -5,6 +5,7 @@ import { buildSystemPrompt } from '../prompts.js'; import { loadState, getBudgetLimit } from '../state.js'; import { inferenceEnv, resolveInferenceBackend, resolveModelId, type InferenceBackend } from '../inference.js'; import { isTerminal, textOf, translateSdkMessage } from './sdk-events.js'; +import { buildHttpMcpServers, type HttpMcpServer } from '../mcp.js'; /** * SDK agent runtime backed by `@anthropic-ai/claude-agent-sdk`. @@ -52,9 +53,17 @@ export class SdkRuntime implements AgentRuntime { // accumulation + interrupt in streamSessionWithAdvisor, which never fires // here (these transports emit no cost spans). const budgetUsd = await getBudgetLimit(); + // Wire the role's MCP servers into the in-process loop. Without this the sdk + // transport ran with NO MCP tools — roles lost github/linear/etc. and could + // not push commits — making it a degraded transport. Mirrors claude-cli's + // --mcp-config; the same gateway-bearer logic lives in buildHttpMcpServers. + const { servers: mcpServers, skipped } = buildHttpMcpServers(member.mcpServers, process.env); + if (skipped.length > 0) { + process.stderr.write(`[sdk] MCP_GATEWAY_TOKEN not set — dropping gateway server(s): ${skipped.join(', ')}.\n`); + } const sdk = await loadSdk(); - const session = new SdkAgentSession(sdk, model, systemPrompt, options, backend, budgetUsd); + const session = new SdkAgentSession(sdk, model, systemPrompt, options, backend, budgetUsd, mcpServers); await session.start(message); return session; } @@ -103,6 +112,7 @@ class SdkAgentSession implements AgentSession { private readonly options?: RunRoleOptions, private readonly backend: InferenceBackend = 'api', private readonly budgetUsd: number | null = null, + private readonly mcpServers: Record = {}, ) {} get id(): string { @@ -135,6 +145,9 @@ class SdkAgentSession implements AgentSession { systemPrompt: this.systemPrompt, permissionMode: 'bypassPermissions', ...(this.budgetUsd != null && { maxBudgetUsd: this.budgetUsd }), + // Role's MCP servers, scoped strictly to fab's set (not the user's + // ambient ~/.claude MCP config) — matches claude-cli's --strict-mcp-config. + ...(Object.keys(this.mcpServers).length > 0 && { mcpServers: this.mcpServers, strictMcpConfig: true }), ...(backendEnv && { env: { ...process.env, ...backendEnv } }), // Resources hint: the SDK uses cwd for filesystem-bound tools; // workflows.ts pre-creates branches on the cloud-mounted repos