Merge pull request 'feat(reliability): bound + fail over + surface LLM-optional ops' (#82) from feat/llm-resilience into main

docs/reliability.md

@@ -0,0 +1,37 @@
+# Reliability: don't let a bad LLM take mcpctl down
+
+The homelab model changes often (fast ↔ thinking, model swaps, backends that
+drift or go down). mcpctl must stay responsive and honest through all of it.
+
+## Principle
+
+**LLM-*optional* operations must be time-bounded, fall back deterministically,
+and report the degradation — never hang and never degrade silently.**
+
+- **Bounded:** every optional LLM call is wrapped in
+  [`withTimeout`](../src/mcplocal/src/util/with-timeout.ts) (Promise.race + an
+  `AbortSignal` so fetch-based providers actually cancel). A thinking model that
+  streams for minutes can never block the caller.
+- **Deterministic fallback:** when the LLM times out or errors, use the
+  non-LLM path (priority/keyword ordering, byte-range pages).
+- **Loud, not silent:** log the reason (`[gate] …`, `[pagination] …`) and tell
+  the user. `begin_session` prepends `⚠ Smart prompt-selection unavailable
+  (<reason>)…` and sets `degraded: true` + `degradedReason` on the audit
+  `gate_decision` event.
+
+Applied in: the gate's `begin_session` prompt selection
+(`proxymodel/plugins/gate.ts`, cap `MCPCTL_GATE_LLM_TIMEOUT_MS`, default 8s) and
+pagination's smart index (`llm/pagination.ts`, `MCPCTL_PAGINATION_LLM_TIMEOUT_MS`,
+default 10s). `read_prompts` is LLM-free by design.
+
+Note: the gate's prompt-ranking uses the **heavy client provider's own model** —
+it deliberately does *not* force the project's vLLM model onto it (doing so made
+every selection fail silently when the model wasn't anthropic-servable).
+
+## LLM-*essential* operations
+
+Chat needs the LLM — it can't fall back. It must **fail fast with a clear,
+actionable message** instead of a vague one. Chat surfaces the upstream status +
+body (which names the model + reason), e.g.
+`LLM returned no completion (HTTP 400): {"error":"model deepseek-v4-flash not found"}`,
+rather than "Adapter returned no choice".

src/mcpd/src/services/chat.service.ts

@@ -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

src/mcplocal/src/gate/llm-selector.ts

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

src/mcplocal/src/llm/pagination.ts

@@ -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({
+    // Time-bounded: a slow LLM must not stall pagination — the caller's catch
-      messages: [
+    // falls back to the simple byte-range index (visibly, via console.error).
-        { role: 'system', content: systemPrompt },
+    const result = await withTimeout(
-        { role: 'user', content: `Tool: ${toolName}\nTotal size: ${String(raw.length)} chars, ${String(pages.length)} pages\n\n${previews}` },
+      (signal) => provider.complete({
-      ],
+        messages: [
-      maxTokens: this.config.indexMaxTokens,
+          { role: 'system', content: systemPrompt },
-      temperature: 0,
+          { role: 'user', content: `Tool: ${toolName}\nTotal size: ${String(raw.length)} chars, ${String(pages.length)} pages\n\n${previews}` },
-      ...(this.modelOverride ? { model: this.modelOverride } : {}),
+        ],
-    });
+        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();

src/mcplocal/src/providers/anthropic.ts

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

src/mcplocal/src/providers/openai.ts

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

src/mcplocal/src/providers/types.ts

@@ -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. */

src/mcplocal/src/proxymodel/plugins/gate.ts

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

src/mcplocal/src/util/with-timeout.ts

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

src/mcplocal/tests/llm-selector.test.ts

@@ -89,17 +89,30 @@ describe('LlmPromptSelector', () => {
     expect(call.temperature).toBe(0);
   });
 
-  it('passes model override to complete options', async () => {
+  it('does not force a model override — prompt-ranking uses the heavy provider\'s own model', async () => {
-    const provider = makeMockProvider(
+    // Regression: forcing the project's vLLM model (e.g. deepseek-v4-*) onto the
-      '{ "selectedNames": [], "reasoning": "" }',
+    // 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 () => {

src/mcplocal/tests/plugin-gate.test.ts

@@ -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({
+      complete: opts.llmError
-        content: opts.llmResponse ?? '{ "selectedNames": ["zigbee-pairing"], "reasoning": "User is working with zigbee" }',
+        ? vi.fn().mockRejectedValue(new Error('upstream 400: model not found'))
-        toolCalls: [],
+        : vi.fn().mockResolvedValue({
-        usage: { promptTokens: 100, completionTokens: 50, totalTokens: 150 },
+          content: opts.llmResponse ?? '{ "selectedNames": ["zigbee-pairing"], "reasoning": "User is working with zigbee" }',
-        finishReason: 'stop',
+          toolCalls: [],
-      } satisfies CompletionResult),
+          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' });

src/mcplocal/tests/with-timeout.test.ts

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