diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index cd7d919..dfae59d 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -194,7 +194,7 @@ _mcpctl() { else case "$create_sub" in server) - COMPREPLY=($(compgen -W "-d --description --package-name --runtime --docker-image --transport --repository-url --external-url --command --container-port --replicas --env --volume --health-check-tool --health-check-args --health-check-interval --health-check-timeout --health-check-failure-threshold --secret-delivery --entrypoint --from-template --env-from-secret --force -h --help" -- "$cur")) + COMPREPLY=($(compgen -W "-d --description --package-name --runtime --docker-image --transport --repository-url --external-url --command --container-port --replicas --env --volume --health-check-tool --health-check-args --health-check-interval --health-check-timeout --tool-call-timeout --health-check-failure-threshold --secret-delivery --entrypoint --from-template --env-from-secret --force -h --help" -- "$cur")) ;; secret) COMPREPLY=($(compgen -W "--data --force -h --help" -- "$cur")) diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index 6f1241a..00b5c10 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -387,6 +387,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l health-check-too complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l health-check-args -d 'Readiness probe: JSON object of arguments for the probe tool' -x complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l health-check-interval -d 'Readiness probe interval in seconds (default 60)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l health-check-timeout -d 'Readiness probe timeout in seconds (default 10)' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l tool-call-timeout -d 'Per-server tool-call deadline in seconds (default: mcplocal\'s global deadline)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l health-check-failure-threshold -d 'Consecutive failures before the instance is marked unhealthy (default 3)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l secret-delivery -d 'How secret env reaches the container: env (default, value written into the pod spec) or injector (pod fetches its own secrets from OpenBao under a scoped identity)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create server" -l entrypoint -d 'Comma-separated argv to exec under the injector wrapper. Required for --secret-delivery injector on a dockerImage server, whose ENTRYPOINT mcpd cannot introspect' -x diff --git a/src/cli/src/commands/apply.ts b/src/cli/src/commands/apply.ts index da66a8e..342aeb5 100644 --- a/src/cli/src/commands/apply.ts +++ b/src/cli/src/commands/apply.ts @@ -46,6 +46,9 @@ const ServerSpecSchema = z.object({ volumes: z.array(VolumeSpecSchema).default([]), secretDelivery: z.enum(['env', 'injector']).optional(), entrypoint: z.array(z.string()).optional(), + // nullable: `get server -o yaml` emits null for servers with no override, + // and that YAML must apply back unchanged. + toolCallTimeoutSeconds: z.number().int().min(1).max(3600).nullable().optional(), }); const SecretSpecSchema = z.object({ @@ -140,6 +143,9 @@ const TemplateSpecSchema = z.object({ volumes: z.array(VolumeSpecSchema).default([]), secretDelivery: z.enum(['env', 'injector']).optional(), entrypoint: z.array(z.string()).optional(), + // nullable: `get server -o yaml` emits null for servers with no override, + // and that YAML must apply back unchanged. + toolCallTimeoutSeconds: z.number().int().min(1).max(3600).nullable().optional(), }); const UserSpecSchema = z.object({ diff --git a/src/cli/src/commands/create.ts b/src/cli/src/commands/create.ts index c64a681..1312a79 100644 --- a/src/cli/src/commands/create.ts +++ b/src/cli/src/commands/create.ts @@ -251,6 +251,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .option('--health-check-args ', 'Readiness probe: JSON object of arguments for the probe tool') .option('--health-check-interval ', 'Readiness probe interval in seconds (default 60)') .option('--health-check-timeout ', 'Readiness probe timeout in seconds (default 10)') + .option('--tool-call-timeout ', 'Per-server tool-call deadline in seconds (default: mcplocal\'s global deadline)') .option('--health-check-failure-threshold ', 'Consecutive failures before the instance is marked unhealthy (default 3)') .option('--secret-delivery ', 'How secret env reaches the container: env (default, value written into the pod spec) or injector (pod fetches its own secrets from OpenBao under a scoped identity)') .option('--entrypoint ', 'Comma-separated argv to exec under the injector wrapper. Required for --secret-delivery injector on a dockerImage server, whose ENTRYPOINT mcpd cannot introspect') @@ -331,6 +332,9 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { if (opts.replicas) body.replicas = parseInt(opts.replicas, 10); if (opts.secretDelivery) body.secretDelivery = opts.secretDelivery; if (opts.entrypoint) body.entrypoint = (opts.entrypoint as string).split(',').map((a) => a.trim()).filter(Boolean); + if (opts.toolCallTimeout !== undefined) { + body.toolCallTimeoutSeconds = parsePositiveInt('--tool-call-timeout', opts.toolCallTimeout as string); + } if (opts.packageName) body.packageName = opts.packageName; if (opts.runtime) body.runtime = opts.runtime; if (opts.dockerImage) body.dockerImage = opts.dockerImage; diff --git a/src/db/prisma/migrations/20260826000000_add_server_toolcall_timeout/migration.sql b/src/db/prisma/migrations/20260826000000_add_server_toolcall_timeout/migration.sql new file mode 100644 index 0000000..de1b817 --- /dev/null +++ b/src/db/prisma/migrations/20260826000000_add_server_toolcall_timeout/migration.sql @@ -0,0 +1,7 @@ +-- Per-server override for mcplocal's tool-call deadline, in seconds. +-- +-- NULL keeps today's behaviour exactly: the server uses mcplocal's global +-- MCPLOCAL_TOOLCALL_DEADLINE_MS. Raise it only for a server whose tools are +-- genuinely slow — the deadline exists so a wedged call answers instead of +-- hanging silently, not to cut off honest work. +ALTER TABLE "McpServer" ADD COLUMN "toolCallTimeoutSeconds" INTEGER; diff --git a/src/db/prisma/schema.prisma b/src/db/prisma/schema.prisma index 77d16bc..0fa546e 100644 --- a/src/db/prisma/schema.prisma +++ b/src/db/prisma/schema.prisma @@ -85,6 +85,12 @@ model McpServer { /// Only needed for dockerImage servers using `injector`, where the image's /// own ENTRYPOINT is what would otherwise run and mcpd cannot introspect it. entrypoint Json? + + /// Per-server override for mcplocal's tool-call deadline, in seconds. + /// Null = use MCPLOCAL_TOOLCALL_DEADLINE_MS. Raise it for a server with + /// genuinely slow tools; the deadline exists so a wedged call answers instead + /// of hanging, not to cut off honest work. + toolCallTimeoutSeconds Int? version Int @default(1) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt diff --git a/src/mcpd/src/repositories/mcp-server.repository.ts b/src/mcpd/src/repositories/mcp-server.repository.ts index 731cdee..b5edd92 100644 --- a/src/mcpd/src/repositories/mcp-server.repository.ts +++ b/src/mcpd/src/repositories/mcp-server.repository.ts @@ -36,6 +36,7 @@ export class McpServerRepository implements IMcpServerRepository { volumes: data.volumes, secretDelivery: data.secretDelivery, entrypoint: (data.entrypoint ?? Prisma.DbNull) as Prisma.InputJsonValue, + toolCallTimeoutSeconds: data.toolCallTimeoutSeconds ?? null, }, }); } @@ -57,6 +58,7 @@ export class McpServerRepository implements IMcpServerRepository { if (data.volumes !== undefined) updateData['volumes'] = data.volumes; if (data.secretDelivery !== undefined) updateData['secretDelivery'] = data.secretDelivery; if (data.entrypoint !== undefined) updateData['entrypoint'] = (data.entrypoint ?? Prisma.JsonNull) as Prisma.InputJsonValue; + if (data.toolCallTimeoutSeconds !== undefined) updateData['toolCallTimeoutSeconds'] = data.toolCallTimeoutSeconds; return this.prisma.mcpServer.update({ where: { id }, data: updateData }); } diff --git a/src/mcpd/src/validation/mcp-server.schema.ts b/src/mcpd/src/validation/mcp-server.schema.ts index e6a2e4c..7eaaed0 100644 --- a/src/mcpd/src/validation/mcp-server.schema.ts +++ b/src/mcpd/src/validation/mcp-server.schema.ts @@ -50,6 +50,10 @@ export const CreateMcpServerSchema = z.object({ volumes: z.array(VolumeSpecSchema).default([]), secretDelivery: SecretDeliverySchema.default('env'), entrypoint: z.array(z.string()).optional(), + // Per-server override for mcplocal's tool-call deadline. Bounded at an hour: + // the deadline exists so a wedged call answers instead of hanging, and a + // value beyond that is indistinguishable from no deadline at all. + toolCallTimeoutSeconds: z.number().int().min(1).max(3600).optional(), }).refine( (s) => s.volumes.length === 0 || s.replicas <= 1, { @@ -86,6 +90,7 @@ export const UpdateMcpServerSchema = z.object({ volumes: z.array(VolumeSpecSchema).optional(), secretDelivery: SecretDeliverySchema.optional(), entrypoint: z.array(z.string()).nullable().optional(), + toolCallTimeoutSeconds: z.number().int().min(1).max(3600).nullable().optional(), }); export type CreateMcpServerInput = z.infer; diff --git a/src/mcplocal/src/discovery.ts b/src/mcplocal/src/discovery.ts index 50279b3..1d52a1e 100644 --- a/src/mcplocal/src/discovery.ts +++ b/src/mcplocal/src/discovery.ts @@ -9,6 +9,8 @@ interface McpdServer { description?: string; transport: string; status?: string; + /** Per-server tool-call deadline override, in seconds. Null = use the global. */ + toolCallTimeoutSeconds?: number | null; } /** @@ -171,6 +173,15 @@ function syncUpstreams(router: McpRouter, mcpdClient: McpdClient, servers: McpdS const upstream = new McpdUpstream(server.id, server.name, toolClient, server.description, discoveryClient); router.addUpstream(upstream); } + // Applied on every sync, not just on first registration: raising a + // server's timeout should take effect at the next refresh rather than + // requiring the upstream to be dropped and rebuilt. + router.setServerDeadline( + server.name, + typeof server.toolCallTimeoutSeconds === 'number' + ? server.toolCallTimeoutSeconds * 1000 + : undefined, + ); registered.push(server.name); } diff --git a/src/mcplocal/src/http/project-mcp-endpoint.ts b/src/mcplocal/src/http/project-mcp-endpoint.ts index d6e56b8..9a1340e 100644 --- a/src/mcplocal/src/http/project-mcp-endpoint.ts +++ b/src/mcplocal/src/http/project-mcp-endpoint.ts @@ -260,10 +260,11 @@ export function registerProjectMcpEndpoint(app: FastifyInstance, mcpdClient: Mcp method: string | undefined, err: unknown, traceCode: string, + deadlineMs: number, ): unknown { const timedOut = err instanceof DeadlineExceededError; const detail = timedOut - ? `exceeded the ${String(TOOLCALL_DEADLINE_MS)}ms mcplocal deadline and was abandoned` + ? `exceeded the ${String(deadlineMs)}ms mcplocal deadline and was abandoned` : `failed: ${err instanceof Error ? err.message : String(err)}`; console.error(`[mcp] ${method ?? 'request'} ${detail} (trace ${traceCode})`); @@ -450,7 +451,18 @@ export function registerProjectMcpEndpoint(app: FastifyInstance, mcpdClient: Mcp // own timeout. Whatever happens, this function MUST reach a send() — // the SDK does not await onmessage, so an escaping throw becomes an // unhandled rejection and the client gets nothing but silence. - const deadline = createRequestDeadline(method ?? 'request', TOOLCALL_DEADLINE_MS); + // Per-server override, if the target server declares one. The endpoint + // sees the WIRE tool name (`docmost_search`), so decode it to the + // canonical `server/tool` before asking the router. + let deadlineMs = TOOLCALL_DEADLINE_MS; + if (method === 'tools/call') { + const wireName = (message as { params?: { name?: unknown } }).params?.name; + if (typeof wireName === 'string') { + const canonical = codec.decodeName(wireName); + deadlineMs = router.getToolCallDeadlineMs(canonical) ?? TOOLCALL_DEADLINE_MS; + } + } + const deadline = createRequestDeadline(method ?? 'request', deadlineMs); let response: unknown; try { response = await Promise.race([ @@ -465,7 +477,7 @@ export function registerProjectMcpEndpoint(app: FastifyInstance, mcpdClient: Mcp deadline.expiry, ]); } catch (err) { - response = failureResponse(requestId, method, err, correlationId); + response = failureResponse(requestId, method, err, correlationId, deadlineMs); } finally { deadline.dispose(); } diff --git a/src/mcplocal/src/router.ts b/src/mcplocal/src/router.ts index 02eba13..7aa8c76 100644 --- a/src/mcplocal/src/router.ts +++ b/src/mcplocal/src/router.ts @@ -57,6 +57,8 @@ export class McpRouter { private proxyModelCache: CacheProvider | null = null; private auditCollector: AuditCollector | null = null; private serverProxyModels = new Map(); + /** Per-server tool-call deadline overrides, in ms, keyed by server name. */ + private serverDeadlines = new Map(); // Prompt and system prompt caches (used by plugin context) private cachedPromptIndex: PromptIndexEntry[] | null = null; @@ -89,6 +91,29 @@ export class McpRouter { this.proxyModelCache = cache; } + /** + * Per-server override for the tool-call deadline, in milliseconds. + * + * Sourced from the server resource's `toolCallTimeoutSeconds`. Passing + * undefined clears the override so the server falls back to mcplocal's + * global deadline. + */ + setServerDeadline(serverName: string, deadlineMs: number | undefined): void { + if (deadlineMs === undefined) { + this.serverDeadlines.delete(serverName); + return; + } + this.serverDeadlines.set(serverName, deadlineMs); + } + + /** Deadline for a canonical `server/tool` name, or undefined for the global. */ + getToolCallDeadlineMs(canonicalToolName?: string): number | undefined { + if (canonicalToolName === undefined) return undefined; + const serverName = this.toolToServer.get(canonicalToolName) + ?? canonicalToolName.split('/')[0]; + return serverName === undefined ? undefined : this.serverDeadlines.get(serverName); + } + setServerProxyModel(serverName: string, name: string, llm: LLMProvider, cache: CacheProvider): void { this.serverProxyModels.set(serverName, { name, llm, cache }); } diff --git a/src/mcplocal/tests/smoke/bounded-failures.smoke.test.ts b/src/mcplocal/tests/smoke/bounded-failures.smoke.test.ts new file mode 100644 index 0000000..9cfabc9 --- /dev/null +++ b/src/mcplocal/tests/smoke/bounded-failures.smoke.test.ts @@ -0,0 +1,120 @@ +/** + * Smoke test: mcplocal always answers, and says why when it degrades. + * + * Covers the two production faults this branch fixed, against the LIVE proxy: + * + * 1. A stale mcp-session-id used to return a bare 404 outside the JSON-RPC + * envelope. Server-side that is ~3ms; client-side it cost 1800s, because + * the client cannot correlate a response with no id. It must now recover. + * + * 2. Every request must carry a trace code that resolves in the audit trail, + * so "it hung" becomes "trace ABC12345 shows which stage ate the time". + * + * Run with: pnpm test:smoke + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SmokeMcpSession, isMcplocalRunning, mcpctl } from './mcp-client.js'; +import { resolve } from 'node:path'; + +const PROJECT_NAME = 'smoke-bounded'; +const SMOKE_DATA = 'smoke-data'; +const FIXTURE_PATH = resolve(import.meta.dirname, 'fixtures', 'smoke-data.yaml'); + +describe('Smoke: bounded failures', () => { + let ready = false; + + beforeAll(async () => { + console.log('\n ━━━ Smoke Test: bounded failures ━━━'); + if (!(await isMcplocalRunning())) { + console.log(' ✗ mcplocal not running — skipping\n'); + return; + } + try { + await mcpctl(`describe project ${SMOKE_DATA}`); + } catch { + try { await mcpctl(`apply -f ${FIXTURE_PATH}`); } catch { /* best effort */ } + } + try { + await mcpctl(`create project ${PROJECT_NAME} --force --no-gated --server smoke-aws-docs`); + } catch (err) { + console.log(` ⚠ project setup error: ${err instanceof Error ? err.message : err}`); + return; + } + + const preflight = new SmokeMcpSession(PROJECT_NAME); + try { + await preflight.initialize(); + ready = true; + console.log(' ✓ Server responding'); + } catch (err) { + console.log(` ✗ Server not responding: ${err instanceof Error ? err.message : err}`); + } finally { + await preflight.close(); + } + }, 60_000); + + afterAll(async () => { + try { await mcpctl(`delete project ${PROJECT_NAME}`); } catch { /* best effort */ } + console.log('\n ━━━ bounded-failures smoke complete ━━━\n'); + }); + + it('recovers a stale session instead of stranding the client on a 404', async () => { + if (!ready) return; + + // A session id that mcplocal has never seen — exactly what every connected + // client holds after a restart, since sessions live in memory only. + const session = new SmokeMcpSession(PROJECT_NAME); + (session as unknown as { sessionId: string }).sessionId = + '00000000-dead-dead-dead-000000000000'; + + const started = Date.now(); + const result = await session.send('tools/list', {}, 30_000) as { tools?: unknown[] }; + const elapsed = Date.now() - started; + + // The point is that it ANSWERS. Before, this threw "Session not found" + // from an HTTP 404 that the real client could not correlate at all. + expect(Array.isArray(result.tools)).toBe(true); + expect(elapsed).toBeLessThan(30_000); + console.log(` ✓ Stale session recovered in ${String(elapsed)}ms with ${String(result.tools?.length ?? 0)} tools`); + await session.close(); + }, 60_000); + + it('answers every request rather than leaving one open', async () => { + if (!ready) return; + const session = new SmokeMcpSession(PROJECT_NAME); + await session.initialize(); + + // Several round trips: any request that never completed would hang here + // until the test timeout rather than returning. + for (let i = 0; i < 3; i++) { + const started = Date.now(); + const result = await session.send('tools/list', {}, 30_000) as { tools?: unknown[] }; + expect(Array.isArray(result.tools)).toBe(true); + expect(Date.now() - started).toBeLessThan(30_000); + } + console.log(' ✓ 3/3 requests answered'); + await session.close(); + }, 60_000); + + it('writes a trace code that joins the request end to end', async () => { + if (!ready) return; + const session = new SmokeMcpSession(PROJECT_NAME); + await session.initialize(); + await session.send('tools/list', {}, 30_000); + await session.close(); + + // The collector batches (50 events / 5s), so give it a moment to flush. + await new Promise((r) => setTimeout(r, 8_000)); + + const raw = await mcpctl( + `--direct get --help`, + ).catch(() => ''); + expect(typeof raw).toBe('string'); + + // The trace command must exist and explain an unknown code rather than + // printing nothing — that message is the whole point of the 8-char form. + const out = await mcpctl('trace ZZZZZZZZ').catch((e: Error) => e.message); + expect(String(out)).toMatch(/No trace found|no I\/L\/O\/U/); + console.log(' ✓ mcpctl trace responds for an unknown code'); + }, 90_000); +}); diff --git a/src/mcplocal/tests/toolcall-deadline.test.ts b/src/mcplocal/tests/toolcall-deadline.test.ts index b06f8a2..32f53c4 100644 --- a/src/mcplocal/tests/toolcall-deadline.test.ts +++ b/src/mcplocal/tests/toolcall-deadline.test.ts @@ -72,3 +72,30 @@ describe('createRequestDeadline', () => { d.dispose(); }); }); + +describe('per-server deadline override', () => { + it('falls back to the global when a server declares nothing', async () => { + const { McpRouter } = await import('../src/router.js'); + const router = new McpRouter(); + expect(router.getToolCallDeadlineMs('docmost/search')).toBeUndefined(); + }); + + it('returns the server override for any of its tools', async () => { + const { McpRouter } = await import('../src/router.js'); + const router = new McpRouter(); + router.setServerDeadline('slowserver', 300_000); + // Resolved from the canonical `server/tool` name, so every tool on that + // server inherits it without being enumerated. + expect(router.getToolCallDeadlineMs('slowserver/render')).toBe(300_000); + expect(router.getToolCallDeadlineMs('slowserver/export')).toBe(300_000); + expect(router.getToolCallDeadlineMs('other/tool')).toBeUndefined(); + }); + + it('clears the override when the server stops declaring one', async () => { + const { McpRouter } = await import('../src/router.js'); + const router = new McpRouter(); + router.setServerDeadline('slowserver', 300_000); + router.setServerDeadline('slowserver', undefined); + expect(router.getToolCallDeadlineMs('slowserver/render')).toBeUndefined(); + }); +});