feat(reliability): bound + fail over + surface LLM-optional ops
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m6s
CI/CD / lint (pull_request) Successful in 2m11s
CI/CD / test (pull_request) Successful in 1m21s
CI/CD / smoke (pull_request) Failing after 2m43s
CI/CD / build (pull_request) Failing after 3h11m43s
CI/CD / publish (pull_request) Has been cancelled
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m6s
CI/CD / lint (pull_request) Successful in 2m11s
CI/CD / test (pull_request) Successful in 1m21s
CI/CD / smoke (pull_request) Failing after 2m43s
CI/CD / build (pull_request) Failing after 3h11m43s
CI/CD / publish (pull_request) Has been cancelled
mcpctl hung on prompt-reading whenever the LLM misbehaved (thinking model = minutes; drifted model = silent failure). Root cause: the gate's begin_session prompt-selection called the LLM with no timeout, its fallback only fired on error and was silent, and it forced the project's vLLM model onto the anthropic heavy provider (so selection failed silently every time). - New withTimeout(run, ms, label): Promise.race + AbortSignal (fetch-based providers cancel). CompletionOptions.signal threaded into anthropic/openai. - Gate begin_session: LLM selection is time-bounded (MCPCTL_GATE_LLM_TIMEOUT_MS, 8s); on timeout/error it falls back to deterministic tag matching, logs [gate] loudly, prepends a ⚠ degraded note to the response, and sets degraded/degradedReason on the audit gate_decision. - Gate selector no longer forces the project vLLM model — uses the heavy provider's own model (fixes the always-silent-fail bug). - Pagination smart-index is time-bounded too (falls back to byte-range pages). - Chat (LLM-essential) surfaces the upstream status+body (names model+reason) instead of "Adapter returned no choice". - docs/reliability.md documents the principle. Tests: with-timeout unit tests; gate degradation test (error → visible ⚠ + deterministic prompts, no hang). mcplocal 737 + mcpd 945 green; tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -254,7 +254,11 @@ export class ChatService {
|
||||
const result = await this.runOneInference(ctx);
|
||||
const choice = extractChoice(result.body);
|
||||
if (choice === null) {
|
||||
throw new Error(`Adapter returned no choice (status ${String(result.status)})`);
|
||||
// Surface the upstream body (LiteLLM/vLLM names the model + reason,
|
||||
// e.g. "model deepseek-v4-flash not found") instead of a vague
|
||||
// "no choice" — the caller needs to know WHICH model failed and why.
|
||||
const detail = typeof result.body === 'string' ? result.body : JSON.stringify(result.body);
|
||||
throw new Error(`LLM returned no completion (HTTP ${String(result.status)}): ${detail.slice(0, 400)}`);
|
||||
}
|
||||
if (choice.tool_calls !== undefined && choice.tool_calls.length > 0) {
|
||||
// Tool turns: keep `content` literal — even if empty — because the
|
||||
|
||||
@@ -23,13 +23,13 @@ export interface LlmSelectionResult {
|
||||
export class LlmPromptSelector {
|
||||
constructor(
|
||||
private readonly providerRegistry: ProviderRegistry,
|
||||
private readonly modelOverride?: string,
|
||||
) {}
|
||||
|
||||
async selectPrompts(
|
||||
tags: string[],
|
||||
promptIndex: PromptIndexForLlm[],
|
||||
getSystemPromptFn?: SystemPromptFetcher,
|
||||
signal?: AbortSignal,
|
||||
): Promise<LlmSelectionResult> {
|
||||
const DEFAULT_SYSTEM_PROMPT = `You are a context selection assistant. Given a developer's task keywords and a list of available project prompts, select which prompts are relevant to their work. Return a JSON object with "selectedNames" (array of prompt names) and "reasoning" (brief explanation). Priority 10 prompts must always be included.`;
|
||||
const systemPrompt = getSystemPromptFn
|
||||
@@ -48,6 +48,9 @@ Select the relevant prompts. Return JSON: { "selectedNames": [...], "reasoning":
|
||||
throw new Error('No heavy LLM provider available');
|
||||
}
|
||||
|
||||
// Deliberately NOT setting `model` here. Prompt-ranking only needs a
|
||||
// working LLM; forcing the project's vLLM model onto the (anthropic) heavy
|
||||
// provider made every selection fail silently. Use the provider's own model.
|
||||
const completionOptions: import('../providers/types.js').CompletionOptions = {
|
||||
messages: [
|
||||
{ role: 'system', content: systemPrompt },
|
||||
@@ -55,10 +58,8 @@ Select the relevant prompts. Return JSON: { "selectedNames": [...], "reasoning":
|
||||
],
|
||||
temperature: 0,
|
||||
maxTokens: 1024,
|
||||
...(signal ? { signal } : {}),
|
||||
};
|
||||
if (this.modelOverride) {
|
||||
completionOptions.model = this.modelOverride;
|
||||
}
|
||||
|
||||
const result = await provider.complete(completionOptions);
|
||||
|
||||
|
||||
@@ -2,6 +2,10 @@ import { randomUUID } from 'node:crypto';
|
||||
import type { ProviderRegistry } from '../providers/registry.js';
|
||||
import { estimateTokens } from './token-counter.js';
|
||||
import type { SystemPromptFetcher } from '../proxymodel/types.js';
|
||||
import { withTimeout } from '../util/with-timeout.js';
|
||||
|
||||
/** Cap on the LLM smart-index call; on timeout, fall back to byte-range pages. */
|
||||
const PAGINATION_LLM_TIMEOUT_MS = Number(process.env['MCPCTL_PAGINATION_LLM_TIMEOUT_MS'] ?? '10000');
|
||||
|
||||
// --- Configuration ---
|
||||
|
||||
@@ -260,15 +264,22 @@ export class ResponsePaginator {
|
||||
? await this.getSystemPrompt('llm-pagination-index', PAGINATION_INDEX_SYSTEM_PROMPT)
|
||||
: PAGINATION_INDEX_SYSTEM_PROMPT;
|
||||
|
||||
const result = await provider.complete({
|
||||
messages: [
|
||||
{ role: 'system', content: systemPrompt },
|
||||
{ role: 'user', content: `Tool: ${toolName}\nTotal size: ${String(raw.length)} chars, ${String(pages.length)} pages\n\n${previews}` },
|
||||
],
|
||||
maxTokens: this.config.indexMaxTokens,
|
||||
temperature: 0,
|
||||
...(this.modelOverride ? { model: this.modelOverride } : {}),
|
||||
});
|
||||
// Time-bounded: a slow LLM must not stall pagination — the caller's catch
|
||||
// falls back to the simple byte-range index (visibly, via console.error).
|
||||
const result = await withTimeout(
|
||||
(signal) => provider.complete({
|
||||
messages: [
|
||||
{ role: 'system', content: systemPrompt },
|
||||
{ role: 'user', content: `Tool: ${toolName}\nTotal size: ${String(raw.length)} chars, ${String(pages.length)} pages\n\n${previews}` },
|
||||
],
|
||||
maxTokens: this.config.indexMaxTokens,
|
||||
temperature: 0,
|
||||
signal,
|
||||
...(this.modelOverride ? { model: this.modelOverride } : {}),
|
||||
}),
|
||||
PAGINATION_LLM_TIMEOUT_MS,
|
||||
'pagination smart-index',
|
||||
);
|
||||
|
||||
// LLMs often wrap JSON in ```json ... ``` fences — strip them
|
||||
const cleaned = result.content.replace(/^```(?:json)?\s*\n?/i, '').replace(/\n?```\s*$/i, '').trim();
|
||||
|
||||
@@ -45,7 +45,7 @@ export class AnthropicProvider implements LlmProvider {
|
||||
}));
|
||||
}
|
||||
|
||||
const response = await this.request(body);
|
||||
const response = await this.request(body, options.signal);
|
||||
return parseAnthropicResponse(response);
|
||||
}
|
||||
|
||||
@@ -72,8 +72,12 @@ export class AnthropicProvider implements LlmProvider {
|
||||
}
|
||||
}
|
||||
|
||||
private request(body: unknown): Promise<unknown> {
|
||||
private request(body: unknown, signal?: AbortSignal): Promise<unknown> {
|
||||
return new Promise((resolve, reject) => {
|
||||
if (signal?.aborted === true) {
|
||||
reject(new Error('request aborted'));
|
||||
return;
|
||||
}
|
||||
const payload = JSON.stringify(body);
|
||||
const isOAuth = this.apiKey.startsWith('sk-ant-oat');
|
||||
const opts = {
|
||||
@@ -109,6 +113,10 @@ export class AnthropicProvider implements LlmProvider {
|
||||
req.destroy();
|
||||
reject(new Error('Request timed out'));
|
||||
});
|
||||
signal?.addEventListener('abort', () => {
|
||||
req.destroy();
|
||||
reject(new Error('request aborted'));
|
||||
}, { once: true });
|
||||
req.write(payload);
|
||||
req.end();
|
||||
});
|
||||
|
||||
@@ -56,7 +56,7 @@ export class OpenAiProvider implements LlmProvider {
|
||||
}));
|
||||
}
|
||||
|
||||
const response = await this.request('/v1/chat/completions', body);
|
||||
const response = await this.request('/v1/chat/completions', body, 'POST', options.signal);
|
||||
return parseResponse(response);
|
||||
}
|
||||
|
||||
@@ -75,8 +75,12 @@ export class OpenAiProvider implements LlmProvider {
|
||||
}
|
||||
}
|
||||
|
||||
private request(path: string, body: unknown, method = 'POST'): Promise<unknown> {
|
||||
private request(path: string, body: unknown, method = 'POST', signal?: AbortSignal): Promise<unknown> {
|
||||
return new Promise((resolve, reject) => {
|
||||
if (signal?.aborted === true) {
|
||||
reject(new Error('request aborted'));
|
||||
return;
|
||||
}
|
||||
const url = new URL(path, this.baseUrl);
|
||||
const isHttps = url.protocol === 'https:';
|
||||
const transport = isHttps ? https : http;
|
||||
@@ -112,6 +116,10 @@ export class OpenAiProvider implements LlmProvider {
|
||||
req.destroy();
|
||||
reject(new Error('Request timed out'));
|
||||
});
|
||||
signal?.addEventListener('abort', () => {
|
||||
req.destroy();
|
||||
reject(new Error('request aborted'));
|
||||
}, { once: true });
|
||||
if (payload) req.write(payload);
|
||||
req.end();
|
||||
});
|
||||
|
||||
@@ -42,6 +42,8 @@ export interface CompletionOptions {
|
||||
temperature?: number;
|
||||
maxTokens?: number;
|
||||
model?: string;
|
||||
/** Cancel the in-flight request (fetch-based providers forward it to fetch). */
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
/** LLM provider tier. 'fast' = local inference, 'heavy' = cloud reasoning. */
|
||||
|
||||
@@ -15,6 +15,11 @@ import { TagMatcher, extractKeywordsFromToolCall, tokenizeDescription } from '..
|
||||
import type { TagMatchResult } from '../../gate/tag-matcher.js';
|
||||
import { LlmPromptSelector } from '../../gate/llm-selector.js';
|
||||
import type { ProviderRegistry } from '../../providers/registry.js';
|
||||
import { withTimeout, TimeoutError } from '../../util/with-timeout.js';
|
||||
|
||||
/** Cap on the gate's LLM prompt-selection. A slow/thinking LLM must never block
|
||||
* begin_session — on timeout we fall back to deterministic tag matching. */
|
||||
const GATE_LLM_TIMEOUT_MS = Number(process.env['MCPCTL_GATE_LLM_TIMEOUT_MS'] ?? '8000');
|
||||
|
||||
export interface GatePluginConfig {
|
||||
gated?: boolean;
|
||||
@@ -29,7 +34,7 @@ export function createGatePlugin(config: GatePluginConfig = {}): ProxyModelPlugi
|
||||
const isGated = config.gated !== false;
|
||||
const tagMatcher = new TagMatcher(config.byteBudget);
|
||||
const llmSelector = config.providerRegistry
|
||||
? new LlmPromptSelector(config.providerRegistry, config.modelOverride)
|
||||
? new LlmPromptSelector(config.providerRegistry)
|
||||
: null;
|
||||
|
||||
// Per-session state tracking (plugin-scoped, not global SessionGate)
|
||||
@@ -266,9 +271,12 @@ async function handleBeginSession(
|
||||
|
||||
const promptIndex = await ctx.fetchPromptIndex();
|
||||
|
||||
// Primary: LLM selection. Fallback: deterministic tag matching.
|
||||
// Primary: LLM selection (time-bounded). Fallback: deterministic tag matching.
|
||||
// A slow/thinking/down LLM must never block begin_session, and if we fall
|
||||
// back the user is told (degradedReason) instead of silently degrading.
|
||||
let matchResult: TagMatchResult;
|
||||
let reasoning = '';
|
||||
let degradedReason: string | null = null;
|
||||
|
||||
if (llmSelector) {
|
||||
try {
|
||||
@@ -279,7 +287,11 @@ async function handleBeginSession(
|
||||
chapters: p.chapters,
|
||||
}));
|
||||
const getSystemPromptFn = ctx.getSystemPrompt.bind(ctx);
|
||||
const llmResult = await llmSelector.selectPrompts(tags, llmIndex, getSystemPromptFn);
|
||||
const llmResult = await withTimeout(
|
||||
(signal) => llmSelector.selectPrompts(tags, llmIndex, getSystemPromptFn, signal),
|
||||
GATE_LLM_TIMEOUT_MS,
|
||||
'gate LLM prompt-selection',
|
||||
);
|
||||
reasoning = llmResult.reasoning;
|
||||
|
||||
const selectedSet = new Set(llmResult.selectedNames);
|
||||
@@ -291,7 +303,12 @@ async function handleBeginSession(
|
||||
selected,
|
||||
);
|
||||
matchResult.remaining = [...matchResult.remaining, ...remaining];
|
||||
} catch {
|
||||
} catch (err) {
|
||||
degradedReason = err instanceof TimeoutError
|
||||
? `LLM prompt-selection timed out after ${String(GATE_LLM_TIMEOUT_MS)}ms`
|
||||
: `LLM prompt-selection failed: ${(err as Error).message}`;
|
||||
// Loud, not silent — visible in mcplocal logs.
|
||||
console.error(`[gate] ${degradedReason} — falling back to priority-ordered prompts`);
|
||||
matchResult = tagMatcher.match(tags, promptIndex);
|
||||
}
|
||||
} else {
|
||||
@@ -313,12 +330,20 @@ async function handleBeginSession(
|
||||
clientIntent: { tags, description: description ?? null },
|
||||
matchedPrompts: matchResult.fullContent.map((p) => p.name),
|
||||
reasoning: reasoning || null,
|
||||
degraded: degradedReason !== null,
|
||||
degradedReason,
|
||||
},
|
||||
});
|
||||
|
||||
// Build response
|
||||
const responseParts: string[] = [];
|
||||
|
||||
if (degradedReason !== null) {
|
||||
responseParts.push(
|
||||
`⚠ Smart prompt-selection unavailable (${degradedReason}). `
|
||||
+ `Showing priority-ordered prompts — still relevant, just not LLM-ranked.\n`,
|
||||
);
|
||||
}
|
||||
if (reasoning) {
|
||||
responseParts.push(`Selection reasoning: ${reasoning}\n`);
|
||||
}
|
||||
|
||||
40
src/mcplocal/src/util/with-timeout.ts
Normal file
40
src/mcplocal/src/util/with-timeout.ts
Normal file
@@ -0,0 +1,40 @@
|
||||
/**
|
||||
* Bound an async operation with a timeout.
|
||||
*
|
||||
* Reliability principle: any LLM-*optional* operation (gate prompt-selection,
|
||||
* pagination summaries) must be time-bounded so a slow/hung LLM (e.g. a
|
||||
* thinking model that streams for minutes) can never block the caller. On
|
||||
* timeout the returned promise rejects with a {@link TimeoutError}; callers are
|
||||
* expected to fall back deterministically and report the degradation.
|
||||
*
|
||||
* The passed `AbortSignal` is aborted on timeout so fetch-based providers can
|
||||
* cancel the in-flight request. Providers that ignore the signal are still
|
||||
* unblocked immediately by the race (their orphaned request settles harmlessly
|
||||
* in the background).
|
||||
*/
|
||||
export class TimeoutError extends Error {
|
||||
constructor(label: string, ms: number) {
|
||||
super(`${label} timed out after ${ms}ms`);
|
||||
this.name = 'TimeoutError';
|
||||
}
|
||||
}
|
||||
|
||||
export async function withTimeout<T>(
|
||||
run: (signal: AbortSignal) => Promise<T>,
|
||||
ms: number,
|
||||
label: string,
|
||||
): Promise<T> {
|
||||
const controller = new AbortController();
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeout = new Promise<never>((_, reject) => {
|
||||
timer = setTimeout(() => {
|
||||
controller.abort();
|
||||
reject(new TimeoutError(label, ms));
|
||||
}, ms);
|
||||
});
|
||||
try {
|
||||
return await Promise.race([run(controller.signal), timeout]);
|
||||
} finally {
|
||||
if (timer !== undefined) clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
@@ -89,17 +89,30 @@ describe('LlmPromptSelector', () => {
|
||||
expect(call.temperature).toBe(0);
|
||||
});
|
||||
|
||||
it('passes model override to complete options', async () => {
|
||||
const provider = makeMockProvider(
|
||||
'{ "selectedNames": [], "reasoning": "" }',
|
||||
);
|
||||
it('does not force a model override — prompt-ranking uses the heavy provider\'s own model', async () => {
|
||||
// Regression: forcing the project's vLLM model (e.g. deepseek-v4-*) onto the
|
||||
// anthropic heavy provider made every selection fail silently. The selector
|
||||
// must leave `model` unset so it uses whatever the heavy provider serves.
|
||||
const provider = makeMockProvider('{ "selectedNames": [], "reasoning": "" }');
|
||||
const registry = makeRegistry(provider);
|
||||
const selector = new LlmPromptSelector(registry, 'gemini-pro');
|
||||
const selector = new LlmPromptSelector(registry);
|
||||
|
||||
await selector.selectPrompts(['test'], sampleIndex);
|
||||
|
||||
const call = (provider.complete as ReturnType<typeof vi.fn>).mock.calls[0]![0] as CompletionOptions;
|
||||
expect(call.model).toBe('gemini-pro');
|
||||
expect(call.model).toBeUndefined();
|
||||
});
|
||||
|
||||
it('forwards an abort signal into complete (for the gate timeout)', async () => {
|
||||
const provider = makeMockProvider('{ "selectedNames": [], "reasoning": "" }');
|
||||
const registry = makeRegistry(provider);
|
||||
const selector = new LlmPromptSelector(registry);
|
||||
const ac = new AbortController();
|
||||
|
||||
await selector.selectPrompts(['test'], sampleIndex, undefined, ac.signal);
|
||||
|
||||
const call = (provider.complete as ReturnType<typeof vi.fn>).mock.calls[0]![0] as CompletionOptions;
|
||||
expect(call.signal).toBe(ac.signal);
|
||||
});
|
||||
|
||||
it('throws when no heavy provider is available', async () => {
|
||||
|
||||
@@ -71,6 +71,8 @@ function setupPluginRouter(opts: {
|
||||
prompts?: typeof samplePrompts;
|
||||
withLlm?: boolean;
|
||||
llmResponse?: string;
|
||||
/** When set, the heavy provider's complete() rejects — exercises the loud fallback. */
|
||||
llmError?: boolean;
|
||||
byteBudget?: number;
|
||||
} = {}): { router: McpRouter; mcpdClient: McpdClient } {
|
||||
const router = new McpRouter();
|
||||
@@ -83,12 +85,14 @@ function setupPluginRouter(opts: {
|
||||
providerRegistry = new ProviderRegistry();
|
||||
const mockProvider: LlmProvider = {
|
||||
name: 'mock-heavy',
|
||||
complete: vi.fn().mockResolvedValue({
|
||||
content: opts.llmResponse ?? '{ "selectedNames": ["zigbee-pairing"], "reasoning": "User is working with zigbee" }',
|
||||
toolCalls: [],
|
||||
usage: { promptTokens: 100, completionTokens: 50, totalTokens: 150 },
|
||||
finishReason: 'stop',
|
||||
} satisfies CompletionResult),
|
||||
complete: opts.llmError
|
||||
? vi.fn().mockRejectedValue(new Error('upstream 400: model not found'))
|
||||
: vi.fn().mockResolvedValue({
|
||||
content: opts.llmResponse ?? '{ "selectedNames": ["zigbee-pairing"], "reasoning": "User is working with zigbee" }',
|
||||
toolCalls: [],
|
||||
usage: { promptTokens: 100, completionTokens: 50, totalTokens: 150 },
|
||||
finishReason: 'stop',
|
||||
} satisfies CompletionResult),
|
||||
listModels: vi.fn().mockResolvedValue([]),
|
||||
isAvailable: vi.fn().mockResolvedValue(true),
|
||||
};
|
||||
@@ -195,6 +199,25 @@ describe('Gate Plugin via setPlugin()', () => {
|
||||
});
|
||||
|
||||
describe('begin_session', () => {
|
||||
it('falls back with a visible warning when the LLM selector errors (no silent degrade)', async () => {
|
||||
const { router } = setupPluginRouter({ withLlm: true, llmError: true });
|
||||
await router.route({ jsonrpc: '2.0', id: 1, method: 'initialize' }, { sessionId: 's1' });
|
||||
|
||||
const res = await router.route(
|
||||
{ jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'begin_session', arguments: { tags: ['zigbee', 'pairing'] } } },
|
||||
{ sessionId: 's1' },
|
||||
);
|
||||
|
||||
// Fell back — the call succeeded rather than failing/hanging …
|
||||
expect(res.error).toBeUndefined();
|
||||
const text = ((res.result as { content: Array<{ text: string }> }).content[0]!.text);
|
||||
// … and the degradation is visible, not silent …
|
||||
expect(text).toContain('Smart prompt-selection unavailable');
|
||||
// … while deterministic tag matching still returned relevant prompts.
|
||||
expect(text).toContain('common-mistakes');
|
||||
expect(text).toContain('zigbee-pairing');
|
||||
});
|
||||
|
||||
it('returns matched prompts with keyword matching', async () => {
|
||||
const { router } = setupPluginRouter();
|
||||
await router.route({ jsonrpc: '2.0', id: 1, method: 'initialize' }, { sessionId: 's1' });
|
||||
|
||||
35
src/mcplocal/tests/with-timeout.test.ts
Normal file
35
src/mcplocal/tests/with-timeout.test.ts
Normal file
@@ -0,0 +1,35 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { withTimeout, TimeoutError } from '../src/util/with-timeout.js';
|
||||
|
||||
describe('withTimeout', () => {
|
||||
it('resolves when the operation finishes in time', async () => {
|
||||
const out = await withTimeout(async () => 'ok', 100, 'fast op');
|
||||
expect(out).toBe('ok');
|
||||
});
|
||||
|
||||
it('rejects with TimeoutError when the operation hangs', async () => {
|
||||
const start = Date.now();
|
||||
await expect(
|
||||
withTimeout(() => new Promise<never>(() => { /* never resolves */ }), 50, 'hang op'),
|
||||
).rejects.toBeInstanceOf(TimeoutError);
|
||||
// Returned promptly, not after the (never) inner promise.
|
||||
expect(Date.now() - start).toBeLessThan(1000);
|
||||
});
|
||||
|
||||
it('aborts the signal on timeout (fetch-based providers can cancel)', async () => {
|
||||
let aborted = false;
|
||||
await expect(
|
||||
withTimeout((signal) => {
|
||||
signal.addEventListener('abort', () => { aborted = true; });
|
||||
return new Promise<never>(() => { /* never */ });
|
||||
}, 50, 'abort op'),
|
||||
).rejects.toBeInstanceOf(TimeoutError);
|
||||
expect(aborted).toBe(true);
|
||||
});
|
||||
|
||||
it('propagates the operation error (not a timeout) when it rejects fast', async () => {
|
||||
await expect(
|
||||
withTimeout(async () => { throw new Error('boom'); }, 100, 'err op'),
|
||||
).rejects.toThrow('boom');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user