From ec06a345cccb493fa1bc4072c2e58d201281cc87 Mon Sep 17 00:00:00 2001 From: Randi <55005611+rdself@users.noreply.github.com> Date: Sun, 29 Mar 2026 03:30:06 -0400 Subject: [PATCH] Require live responses for combo tests (#735) --- src/app/(dashboard)/dashboard/combos/page.tsx | 24 +-- src/app/api/combos/test/route.ts | 90 +++++------ src/lib/combos/testHealth.ts | 106 ++++++------- tests/unit/combo-test-health.test.mjs | 77 +++++---- tests/unit/combo-test-route.test.mjs | 148 ++++++++++++++++++ 5 files changed, 293 insertions(+), 152 deletions(-) create mode 100644 tests/unit/combo-test-route.test.mjs diff --git a/src/app/(dashboard)/dashboard/combos/page.tsx b/src/app/(dashboard)/dashboard/combos/page.tsx index f5e4cd21bc..5a5c71d3bf 100644 --- a/src/app/(dashboard)/dashboard/combos/page.tsx +++ b/src/app/(dashboard)/dashboard/combos/page.tsx @@ -1129,20 +1129,12 @@ function TestResultsView({ results }) { className={`material-symbols-outlined text-[14px] ${ r.status === "ok" ? "text-emerald-500" - : r.status === "reachable" - ? "text-amber-500" - : r.status === "skipped" - ? "text-text-muted" - : "text-red-500" + : r.status === "skipped" + ? "text-text-muted" + : "text-red-500" }`} > - {r.status === "ok" - ? "check_circle" - : r.status === "reachable" - ? "network_check" - : r.status === "skipped" - ? "skip_next" - : "error"} + {r.status === "ok" ? "check_circle" : r.status === "skipped" ? "skip_next" : "error"} {r.model} {r.latencyMs !== undefined && {r.latencyMs}ms} @@ -1150,11 +1142,9 @@ function TestResultsView({ results }) { className={`text-[10px] uppercase font-medium ${ r.status === "ok" ? "text-emerald-500" - : r.status === "reachable" - ? "text-amber-500" - : r.status === "skipped" - ? "text-text-muted" - : "text-red-500" + : r.status === "skipped" + ? "text-text-muted" + : "text-red-500" }`} > {r.status} diff --git a/src/app/api/combos/test/route.ts b/src/app/api/combos/test/route.ts index 726f8fa6b9..af49e6d1c1 100644 --- a/src/app/api/combos/test/route.ts +++ b/src/app/api/combos/test/route.ts @@ -1,16 +1,13 @@ import { NextResponse } from "next/server"; -import { - buildComboTestRequestBody, - probeComboModelReachability, - shouldProbeComboTestReachability, -} from "@/lib/combos/testHealth"; +import { buildComboTestRequestBody, extractComboTestResponseText } from "@/lib/combos/testHealth"; import { getComboByName } from "@/lib/localDb"; import { testComboSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; /** * POST /api/combos/test - Quick test a combo - * Sends a minimal request through each model in the combo to verify availability + * Sends a real chat completion request through each model in the combo + * and only reports success when the model returns usable text content. */ export async function POST(request) { let rawBody; @@ -53,34 +50,55 @@ export async function POST(request) { for (const modelStr of models) { const startTime = Date.now(); try { - // Send a minimal chat request to the internal SSE handler - // Use a tiny but realistic request body so gateway-routed models do not - // get flagged as dead just because the probe payload is too synthetic. + // Send a minimal but real chat request through the same internal + // endpoint an external OpenAI-compatible client would use. const testBody = buildComboTestRequestBody(modelStr); const internalUrl = `${getBaseUrl(request)}/v1/chat/completions`; const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 20000); // 20s timeout (was 15s, slow providers need more) + const timeout = setTimeout(() => controller.abort(), 20000); - const res = await fetch(internalUrl, { - method: "POST", - headers: { - "Content-Type": "application/json", - // Fix #350: bypass REQUIRE_API_KEY for internal admin combo tests - "X-Internal-Test": "combo-health-check", - }, - body: JSON.stringify(testBody), - signal: controller.signal, - }); + let res; + try { + res = await fetch(internalUrl, { + method: "POST", + headers: { + "Content-Type": "application/json", + // Internal dashboard tests still use the normal /v1 pipeline but + // bypass REQUIRE_API_KEY so admins can test with local session auth. + "X-Internal-Test": "combo-health-check", + }, + body: JSON.stringify(testBody), + signal: controller.signal, + }); + } finally { + clearTimeout(timeout); + } - clearTimeout(timeout); const latencyMs = Date.now() - startTime; if (res.ok) { - results.push({ model: modelStr, status: "ok", latencyMs }); + let responseBody = null; + try { + responseBody = await res.json(); + } catch { + responseBody = null; + } + + const responseText = extractComboTestResponseText(responseBody); + if (!responseText) { + results.push({ + model: modelStr, + status: "error", + statusCode: res.status, + error: "Provider returned HTTP 200 but no text content.", + latencyMs, + }); + continue; + } + + results.push({ model: modelStr, status: "ok", latencyMs, responseText }); if (!resolvedBy) resolvedBy = modelStr; - // For test, we can stop after first success (like a real combo would) - // But let's test all models to show full health } else { let errorMsg = ""; try { @@ -90,28 +108,6 @@ export async function POST(request) { errorMsg = res.statusText; } - let reachability = null; - if (shouldProbeComboTestReachability(res.status)) { - try { - reachability = await probeComboModelReachability(modelStr); - } catch { - reachability = null; - } - } - - if (reachability?.reachable) { - results.push({ - model: modelStr, - status: "reachable", - statusCode: res.status, - error: errorMsg, - latencyMs, - provider: reachability.provider, - probeMethod: reachability.method, - }); - continue; - } - results.push({ model: modelStr, status: "error", @@ -125,7 +121,7 @@ export async function POST(request) { results.push({ model: modelStr, status: "error", - error: error.name === "AbortError" ? "Timeout (15s)" : error.message, + error: error.name === "AbortError" ? "Timeout (20s)" : error.message, latencyMs, }); } diff --git a/src/lib/combos/testHealth.ts b/src/lib/combos/testHealth.ts index 997de666da..15959e6913 100644 --- a/src/lib/combos/testHealth.ts +++ b/src/lib/combos/testHealth.ts @@ -1,74 +1,70 @@ -import { validateProviderApiKey } from "@/lib/providers/validation"; -import { getProviderCredentials } from "@/sse/services/auth"; -import { getModelInfo } from "@/sse/services/model"; +type JsonRecord = Record; -const SOFT_REACHABILITY_STATUSES = new Set([400, 405, 406, 409, 422]); +function asRecord(value: unknown): JsonRecord { + return value && typeof value === "object" && !Array.isArray(value) ? (value as JsonRecord) : {}; +} + +function extractTextFromContent(content: unknown): string { + if (typeof content === "string") return content.trim(); + + if (!Array.isArray(content)) return ""; + + return content + .map((part) => { + if (typeof part === "string") return part.trim(); + + const block = asRecord(part); + const blockType = typeof block.type === "string" ? block.type : ""; + const blockText = typeof block.text === "string" ? block.text.trim() : ""; + + if (blockText && (blockType === "" || blockType === "text" || blockType === "output_text")) { + return blockText; + } + + return ""; + }) + .filter(Boolean) + .join("\n") + .trim(); +} export function buildComboTestRequestBody(modelStr: string) { return { model: modelStr, messages: [{ role: "user", content: "Reply with OK only." }], - // Some gateway-routed models reject ultra-tiny budgets during smoke tests. + // Keep this close to a real client request without inflating cost. max_tokens: 16, stream: false, }; } -export function shouldProbeComboTestReachability(statusCode: number) { - return SOFT_REACHABILITY_STATUSES.has(Number(statusCode)); -} +export function extractComboTestResponseText(responseBody: unknown): string { + const body = asRecord(responseBody); -type ProbeDeps = { - getModelInfo?: typeof getModelInfo; - getProviderCredentials?: typeof getProviderCredentials; - validateProviderApiKey?: typeof validateProviderApiKey; -}; - -export async function probeComboModelReachability(modelStr: string, deps: ProbeDeps = {}) { - const resolveModel = deps.getModelInfo || getModelInfo; - const loadCredentials = deps.getProviderCredentials || getProviderCredentials; - const validateKey = deps.validateProviderApiKey || validateProviderApiKey; - - const modelInfo = await resolveModel(modelStr); - if (!modelInfo?.provider) { - return { reachable: false, reason: "unresolved_model" }; - } - - const credentials = await loadCredentials( - modelInfo.provider, - null, - null, - modelInfo.model || modelStr - ); - if (!credentials || credentials.allRateLimited) { - return { reachable: false, reason: "credentials_unavailable" }; + if (typeof body.output_text === "string" && body.output_text.trim()) { + return body.output_text.trim(); } - const apiKey = credentials.apiKey || credentials.accessToken; - if (typeof apiKey !== "string" || apiKey.trim().length === 0) { - return { reachable: false, reason: "missing_auth_material" }; + if (Array.isArray(body.choices)) { + for (const choice of body.choices) { + const choiceRecord = asRecord(choice); + const message = asRecord(choiceRecord.message); + const messageText = extractTextFromContent(message.content); + if (messageText) return messageText; + + if (typeof choiceRecord.text === "string" && choiceRecord.text.trim()) { + return choiceRecord.text.trim(); + } + } } - - const providerSpecificData = - credentials.providerSpecificData && typeof credentials.providerSpecificData === "object" - ? { ...credentials.providerSpecificData } - : {}; - if (!providerSpecificData.validationModelId && modelInfo.model) { - providerSpecificData.validationModelId = modelInfo.model; + if (Array.isArray(body.output)) { + for (const item of body.output) { + const itemRecord = asRecord(item); + const contentText = extractTextFromContent(itemRecord.content); + if (contentText) return contentText; + } } - - const validation = await validateKey({ - provider: modelInfo.provider, - apiKey, - providerSpecificData, - }); - return { - reachable: Boolean(validation?.valid), - provider: modelInfo.provider, - model: modelInfo.model || null, - method: validation?.method || null, - warning: validation?.warning || null, - }; + return extractTextFromContent(body.content); } diff --git a/tests/unit/combo-test-health.test.mjs b/tests/unit/combo-test-health.test.mjs index 80bed2fd81..2471656d46 100644 --- a/tests/unit/combo-test-health.test.mjs +++ b/tests/unit/combo-test-health.test.mjs @@ -1,7 +1,7 @@ import test from "node:test"; import assert from "node:assert/strict"; -const { buildComboTestRequestBody, shouldProbeComboTestReachability, probeComboModelReachability } = +const { buildComboTestRequestBody, extractComboTestResponseText } = await import("../../src/lib/combos/testHealth.ts"); test("combo test helper builds a realistic smoke payload", () => { @@ -13,39 +13,50 @@ test("combo test helper builds a realistic smoke payload", () => { assert.equal(body.stream, false); }); -test("combo test helper probes only soft 4xx responses", () => { - assert.equal(shouldProbeComboTestReachability(400), true); - assert.equal(shouldProbeComboTestReachability(422), true); - assert.equal(shouldProbeComboTestReachability(401), false); - assert.equal(shouldProbeComboTestReachability(404), false); - assert.equal(shouldProbeComboTestReachability(429), false); -}); - -test("combo reachability probe reuses resolved provider credentials and model id", async () => { - let validationInput = null; - - const result = await probeComboModelReachability("openrouter/openai/gpt-5.4", { - getModelInfo: async () => ({ provider: "openrouter", model: "openai/gpt-5.4" }), - getProviderCredentials: async () => ({ - apiKey: "test-key", - providerSpecificData: { baseUrl: "https://openrouter.ai/api/v1" }, - }), - validateProviderApiKey: async (input) => { - validationInput = input; - return { valid: true, method: "models_endpoint" }; - }, +test("combo test helper extracts text from chat-completions responses", () => { + const text = extractComboTestResponseText({ + choices: [ + { + message: { + role: "assistant", + content: "OK", + }, + }, + ], }); - assert.equal(result.reachable, true); - assert.equal(result.provider, "openrouter"); - assert.equal(result.model, "openai/gpt-5.4"); - assert.equal(result.method, "models_endpoint"); - assert.deepEqual(validationInput, { - provider: "openrouter", - apiKey: "test-key", - providerSpecificData: { - baseUrl: "https://openrouter.ai/api/v1", - validationModelId: "openai/gpt-5.4", - }, + assert.equal(text, "OK"); +}); + +test("combo test helper extracts text from block-based responses", () => { + const text = extractComboTestResponseText({ + choices: [ + { + message: { + role: "assistant", + content: [ + { type: "text", text: "OK" }, + { type: "output_text", text: "Confirmed." }, + ], + }, + }, + ], }); + + assert.equal(text, "OK\nConfirmed."); +}); + +test("combo test helper returns empty string when no text content exists", () => { + const text = extractComboTestResponseText({ + choices: [ + { + message: { + role: "assistant", + content: [{ type: "tool_call", id: "call_1" }], + }, + }, + ], + }); + + assert.equal(text, ""); }); diff --git a/tests/unit/combo-test-route.test.mjs b/tests/unit/combo-test-route.test.mjs new file mode 100644 index 0000000000..2e2ad6a325 --- /dev/null +++ b/tests/unit/combo-test-route.test.mjs @@ -0,0 +1,148 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-combo-test-route-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const combosDb = await import("../../src/lib/db/combos.ts"); +const route = await import("../../src/app/api/combos/test/route.ts"); + +const originalFetch = globalThis.fetch; + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +async function createTestCombo(models = ["openrouter/openai/gpt-5.4"]) { + return combosDb.createCombo({ + name: "strict-live-test", + models, + strategy: "priority", + }); +} + +function makeRequest(comboName = "strict-live-test") { + return new Request("http://localhost/api/combos/test", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ comboName }), + }); +} + +test.beforeEach(async () => { + globalThis.fetch = originalFetch; + await resetStorage(); +}); + +test.afterEach(() => { + globalThis.fetch = originalFetch; +}); + +test.after(() => { + globalThis.fetch = originalFetch; + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("combo test route marks a model healthy only when it returns assistant text", async () => { + await createTestCombo(); + + const fetchCalls = []; + globalThis.fetch = async (url, init = {}) => { + fetchCalls.push({ url: String(url), init }); + return new Response( + JSON.stringify({ + choices: [ + { + message: { + role: "assistant", + content: "OK", + }, + }, + ], + }), + { + status: 200, + headers: { "content-type": "application/json" }, + } + ); + }; + + const response = await route.POST(makeRequest()); + const body = await response.json(); + const forwardedBody = JSON.parse(fetchCalls[0].init.body); + + assert.equal(response.status, 200); + assert.equal(fetchCalls.length, 1); + assert.equal(fetchCalls[0].url, "http://localhost/v1/chat/completions"); + assert.equal(fetchCalls[0].init.headers["X-Internal-Test"], "combo-health-check"); + assert.equal(forwardedBody.model, "openrouter/openai/gpt-5.4"); + assert.equal(forwardedBody.messages[0].content, "Reply with OK only."); + assert.equal(body.resolvedBy, "openrouter/openai/gpt-5.4"); + assert.equal(body.results[0].status, "ok"); + assert.equal(body.results[0].responseText, "OK"); +}); + +test("combo test route treats empty successful responses as failures", async () => { + await createTestCombo(); + + globalThis.fetch = async () => + new Response( + JSON.stringify({ + choices: [ + { + message: { + role: "assistant", + content: "", + }, + }, + ], + }), + { + status: 200, + headers: { "content-type": "application/json" }, + } + ); + + const response = await route.POST(makeRequest()); + const body = await response.json(); + + assert.equal(response.status, 200); + assert.equal(body.resolvedBy, null); + assert.equal(body.results[0].status, "error"); + assert.equal(body.results[0].statusCode, 200); + assert.match(body.results[0].error, /no text content/i); +}); + +test("combo test route surfaces provider errors instead of downgrading them to reachability", async () => { + await createTestCombo(); + + globalThis.fetch = async () => + new Response( + JSON.stringify({ + error: { + message: "Upstream rejected this request shape", + }, + }), + { + status: 422, + headers: { "content-type": "application/json" }, + } + ); + + const response = await route.POST(makeRequest()); + const body = await response.json(); + + assert.equal(response.status, 200); + assert.equal(body.resolvedBy, null); + assert.equal(body.results[0].status, "error"); + assert.equal(body.results[0].statusCode, 422); + assert.equal(body.results[0].error, "Upstream rejected this request shape"); + assert.equal("probeMethod" in body.results[0], false); +});