feat: per-server tool-call timeout, on the server resource
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m25s
CI/CD / lint (pull_request) Successful in 2m39s
CI/CD / test (pull_request) Successful in 1m29s
CI/CD / smoke (pull_request) Failing after 3m6s
CI/CD / build (pull_request) Successful in 2m27s
CI/CD / publish (pull_request) Has been skipped

Completes the plan's last item. The deadline shipped with a global default;
this makes it overridable per server, where the knowledge actually lives -- a
server with genuinely slow tools declares its own budget instead of forcing the
global up for everyone.

Source of truth is the server resource, following healthCheck exactly:
Prisma column + migration (NULL keeps today's behaviour, so no existing server
changes), zod validation on create and update, the repository, an
`--tool-call-timeout` flag mirroring `--health-check-timeout`, and the apply
schema so `apply -f` accepts what `get server -o yaml` emits.

That round-trip needed care: get emits `toolCallTimeoutSeconds: null` for every
server without an override, so the apply schema is nullable, not merely
optional -- otherwise the very first `get -o yaml | apply -f` on an untouched
server would have failed validation.

Bounded at one hour. A deadline exists so a wedged call answers instead of
hanging; a value beyond an hour is indistinguishable from having none.

It reaches mcplocal through server discovery rather than the project-scoped
serverOverrides map, and is applied on EVERY sync rather than only at first
registration -- raising a server's timeout should take effect at the next
refresh, not require the upstream to be dropped and rebuilt.

The endpoint sees the wire name (`docmost_search`), so it decodes to the
canonical `server/tool` before resolving. The override is keyed by server, so
every tool on that server inherits it without being enumerated, and the
failure message quotes the deadline actually applied rather than the global.

Smoke tests (tests/smoke/bounded-failures.smoke.test.ts) cover the two
production faults against the live proxy: a stale session recovers instead of
stranding the client on an uncorrelatable 404, every request is answered, and
`mcpctl trace` responds for an unknown code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2
This commit is contained in:
2026-08-26 00:03:06 +01:00
parent 08b451b7aa
commit 47d7c779d0
13 changed files with 230 additions and 4 deletions

View File

@@ -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);
});