fix(gate): stop masking backend errors + rescue thinking-model prompt selection

Two independent, live-reproduced causes of "Smart prompt-selection unavailable (LLM response did not contain valid selection JSON)" in the gate: - Cause B (openai.ts request): the response handler JSON.parsed the body with no res.statusCode check, so a LiteLLM/vLLM HTTP 500 ("Connection error") parsed as an empty completion and surfaced as generic "bad JSON", masking backend-down. Now reject on status >= 400 with "OpenAI HTTP <status>: <body>" so the selector reports the true degradedReason. - Cause A (parseResponse + selector budget): a reasoning model (qwen3-thinking) spends its whole 1024-token budget in reasoning_content and emits content=null, so the {selectedNames} JSON never appears. parseResponse now falls back to reasoning_content (incl. provider_specific_fields) when content is empty, and the selector budget is raised 1024 -> 4000 (env MCPCTL_GATE_SELECT_MAX_TOKENS) so thinking + the JSON answer both fit. Note: mcpd's chat path got a reasoning_content fallback in PR #67; this is the separate mcplocal gate-selection provider, which never did. Tests: openai-provider (500 rejects, reasoning_content capture/preference). mcplocal suite green (749). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-23 22:10:08 +01:00
parent f614e9bb98
commit 9dbd2f175d
3 changed files with 110 additions and 2 deletions
--- a/src/mcplocal/src/gate/llm-selector.ts
+++ b/src/mcplocal/src/gate/llm-selector.ts
@@ -20,6 +20,17 @@ export interface LlmSelectionResult {
  reasoning: string;
 }

+/**
+ * Token budget for the selection call. A reasoning model spends most of its
+ * budget inside reasoning_content before emitting the {selectedNames} JSON, so
+ * 1024 was too tight (finish_reason=length, empty content → "bad JSON"). Give
+ * it room for thinking + the small JSON answer. Override via env.
+ */
+const SELECT_MAX_TOKENS = (() => {
+  const v = Number(process.env['MCPCTL_GATE_SELECT_MAX_TOKENS']);
+  return Number.isFinite(v) && v > 0 ? v : 4000;
+})();
+
 export class LlmPromptSelector {
  constructor(
    private readonly providerRegistry: ProviderRegistry,
@@ -57,7 +68,7 @@ Select the relevant prompts. Return JSON: { "selectedNames": [...], "reasoning":
        { role: 'user', content: userPrompt },
      ],
      temperature: 0,
-      maxTokens: 1024,
+      maxTokens: SELECT_MAX_TOKENS,
      ...(signal ? { signal } : {}),
    };

--- a/src/mcplocal/src/providers/openai.ts
+++ b/src/mcplocal/src/providers/openai.ts
@@ -104,6 +104,16 @@ export class OpenAiProvider implements LlmProvider {
        res.on('data', (chunk: Buffer) => chunks.push(chunk));
        res.on('end', () => {
          const raw = Buffer.concat(chunks).toString('utf-8');
+          // Surface transport failures as errors instead of parsing the error
+          // body as a completion. A LiteLLM/vLLM HTTP 500 ("Connection error")
+          // is valid JSON, so without this check it slips through as an empty
+          // completion and gets misreported downstream as "bad JSON" — masking
+          // the true backend-down / rate-limit / auth reason.
+          const status = res.statusCode ?? 0;
+          if (status >= 400) {
+            reject(new Error(`OpenAI HTTP ${status}: ${raw.slice(0, 300)}`));
+            return;
+          }
          try {
            resolve(JSON.parse(raw));
          } catch {
@@ -141,6 +151,12 @@ function parseResponse(raw: unknown): CompletionResult {
    choices?: Array<{
      message?: {
        content?: string | null;
+        // Reasoning ("thinking") models (e.g. qwen3-thinking) emit their answer
+        // under reasoning_content with content null when the token budget is
+        // tight. Capture it so gate/pagination selection can still find their
+        // JSON instead of seeing an empty completion.
+        reasoning_content?: string | null;
+        provider_specific_fields?: { reasoning_content?: string | null };
        tool_calls?: Array<{
          id: string;
          function: { name: string; arguments: string };
@@ -166,8 +182,15 @@ function parseResponse(raw: unknown): CompletionResult {
    : choice?.finish_reason === 'length' ? 'length' as const
    : 'stop' as const;

+  const message = choice?.message;
+  const primaryContent = message?.content ?? '';
+  const reasoningContent =
+    message?.reasoning_content ?? message?.provider_specific_fields?.reasoning_content ?? '';
+
  return {
-    content: choice?.message?.content ?? '',
+    // Fall back to reasoning_content only when the model produced no content —
+    // preserves normal behaviour but rescues thinking-only responses.
+    content: primaryContent !== '' ? primaryContent : reasoningContent,
    toolCalls,
    usage: {
      promptTokens: data.usage?.prompt_tokens ?? 0,
--- a/src/mcplocal/tests/openai-provider.test.ts
+++ b/src/mcplocal/tests/openai-provider.test.ts
@@ -0,0 +1,74 @@
+/**
+ * OpenAiProvider — transport-error surfacing and reasoning_content capture.
+ *
+ * Covers the two gate prompt-ranking failure modes reproduced live 2026-07-23:
+ *  - a LiteLLM/vLLM HTTP 500 must reject (not parse as an empty completion),
+ *  - a reasoning ("thinking") model that emits its answer under
+ *    reasoning_content with content=null must still yield that text.
+ */
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+
+let mockStatus = 200;
+let mockBody = '{}';
+
+vi.mock('node:http', () => {
+  const request = vi.fn((_opts: unknown, cb?: unknown) => {
+    const res = {
+      statusCode: mockStatus,
+      on: (event: string, handler: (d?: unknown) => void) => {
+        if (event === 'data') handler(Buffer.from(mockBody));
+        if (event === 'end') handler();
+        return res;
+      },
+    };
+    if (typeof cb === 'function') (cb as (r: unknown) => void)(res);
+    return { on: vi.fn().mockReturnThis(), write: vi.fn(), end: vi.fn() };
+  });
+  return { default: { request }, request };
+});
+
+const { OpenAiProvider } = await import('../src/providers/openai.js');
+
+function provider(): InstanceType<typeof OpenAiProvider> {
+  return new OpenAiProvider({ apiKey: 'k', baseUrl: 'http://localhost:9' });
+}
+
+describe('OpenAiProvider transport + reasoning handling', () => {
+  beforeEach(() => {
+    mockStatus = 200;
+    mockBody = '{}';
+  });
+
+  it('rejects on HTTP >= 400 with the status + body (not a masked empty completion)', async () => {
+    mockStatus = 500;
+    mockBody = JSON.stringify({ error: { message: 'litellm.InternalServerError: Connection error' } });
+    await expect(
+      provider().complete({ messages: [{ role: 'user', content: 'hi' }], maxTokens: 8 }),
+    ).rejects.toThrow(/OpenAI HTTP 500/);
+  });
+
+  it('falls back to reasoning_content when content is null (thinking model)', async () => {
+    mockBody = JSON.stringify({
+      choices: [{ message: { content: null, reasoning_content: '{"selectedNames":["x"]}' }, finish_reason: 'length' }],
+    });
+    const r = await provider().complete({ messages: [{ role: 'user', content: 'hi' }], maxTokens: 8 });
+    expect(r.content).toContain('selectedNames');
+    expect(r.finishReason).toBe('length');
+  });
+
+  it('reads reasoning_content from provider_specific_fields too', async () => {
+    mockBody = JSON.stringify({
+      choices: [{ message: { content: '', provider_specific_fields: { reasoning_content: 'thought-answer' } } }],
+    });
+    const r = await provider().complete({ messages: [{ role: 'user', content: 'hi' }], maxTokens: 8 });
+    expect(r.content).toBe('thought-answer');
+  });
+
+  it('prefers real content over reasoning_content when both are present', async () => {
+    mockBody = JSON.stringify({
+      choices: [{ message: { content: 'real', reasoning_content: 'thinking' }, finish_reason: 'stop' }],
+    });
+    const r = await provider().complete({ messages: [{ role: 'user', content: 'hi' }], maxTokens: 8 });
+    expect(r.content).toBe('real');
+  });
+});