From 28788cb9aff6315b62086a96ce56d552dd8923c2 Mon Sep 17 00:00:00 2001 From: Nathan <370788475@qq.com> Date: Fri, 21 Aug 2026 09:41:27 +0800 Subject: [PATCH] fix(cli): route provider tests through connection API (#10572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns provider-test CLI paths with the server's connection-owned management API: `omniroute test` now resolves a connection and calls `POST /api/providers/{id}/test` instead of the missing `/api/v1/providers/test` route; `--all-providers` carries exact connection ids into both non-interactive and TUI runs. Fixes #10570. Validated in an isolated worktree boarded onto origin/release/v3.8.50 (0 conflicts, 4 files): - 42/42 focused tests pass (cli-provider-test-routes-10570, cli-providers-command, cli-providers-rotate, cli-route-unavailable-fallback-10081, cli-expanded-commands). - One pre-existing test in cli-expanded-commands.test.ts (not touched by the PR) mocked the old route and the old `success` response field, exposed only after merging with the current release tip — fixed the mock to match the new per-connection route and the `valid` field the real route actually returns, pushed fix-in-place to the PR branch (owner-authorized rule: fix-in-place over reimplementation, credit preserved). - check-file-size, check-changelog-integrity: OK. - typecheck:core: clean. - check-complexity / check-cognitive-complexity: OK, both under baseline. Co-authored-by: hydraxman --- CHANGELOG.md | 1 + bin/cli/commands/providers.mjs | 35 +++- bin/cli/commands/test-provider.mjs | 87 ++++++--- bin/cli/tui/ProvidersTestAll.jsx | 24 +-- tests/unit/cli-expanded-commands.test.ts | 4 +- .../cli-provider-test-routes-10570.test.ts | 177 ++++++++++++++++++ 6 files changed, 290 insertions(+), 38 deletions(-) create mode 100644 tests/unit/cli-provider-test-routes-10570.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 94438a0922..9c01b39a7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -169,6 +169,7 @@ _Living section — regenerated 2026-08-12 from all cycle commits (cycle open `e - **security(search)**: block SSRF via `/v1/search` `provider_options.baseUrl` for the Firecrawl search provider — the client-controlled override is now validated as a public URL before it is used to build the server-side fetch target, so a caller with a valid API key can no longer redirect search requests at loopback, RFC1918, or cloud-metadata hosts — thanks @zmf963 - **providers**: honor `PATCH /api/providers/[id]` so `omniroute providers rotate` stops 405ing (the OpenAPI spec and CLI already use PATCH) (PR #10366) +- **cli**: route provider test commands through configured connection test endpoints (#10570) - **executors**: fix internal timeout misclassified as client disconnect (499) for 7 niche executors — pass TimeoutError reason to controller.abort() (#8197 side-finding) - test(combo): guard auto/best-free never leaks the combo name as a model (#7754) - fix(vision-bridge): describe-model no longer returns unreachable "openai/gpt-4o-mini" when every vision-capable provider is unreachable on the instance — returns null instead and surfaces a clear error (#8430) diff --git a/bin/cli/commands/providers.mjs b/bin/cli/commands/providers.mjs index 91d60cead8..eb872241bf 100644 --- a/bin/cli/commands/providers.mjs +++ b/bin/cli/commands/providers.mjs @@ -129,7 +129,34 @@ function buildTestInput(connection, apiKey) { }; } -async function runProviderTest(db, connection) { +async function testProviderConnectionThroughServer(connection) { + try { + const res = await apiFetch(`/api/providers/${encodeURIComponent(connection.id)}/test`, { + method: "POST", + body: {}, + retry: false, + timeout: 30000, + acceptNotOk: true, + }); + const data = res.ok ? await res.json() : { valid: false, error: `HTTP ${res.status}` }; + return { + connection: publicConnection(connection), + ...data, + valid: data.valid === true, + skipped: false, + }; + } catch (error) { + return { + connection: publicConnection(connection), + valid: false, + skipped: false, + error: error instanceof Error ? error.message : String(error), + statusCode: null, + }; + } +} + +async function runProviderTest(db, connection, { serverUp = false } = {}) { // Only API-key connections can be probed with a stored credential. OAuth / // no-auth connections have nothing for testProviderApiKey() to send, and // getProviderApiKey() throws for them by design — reporting that as a FAILED @@ -151,6 +178,9 @@ async function runProviderTest(db, connection) { // means the CLI has no probe recipe, not that the provider is unhealthy. // Persisting it would overwrite a good test_status with a failure. if (result.unsupported) { + if (serverUp) { + return testProviderConnectionThroughServer(connection); + } return { connection: publicConnection(connection), ...result, @@ -266,6 +296,7 @@ export async function runTestCommand(selector, opts = {}) { } export async function runTestAllCommand(opts = {}) { + const serverUp = await isServerUp(); const { db } = await openOmniRouteDb(); try { const connections = listProviderConnections(db); @@ -280,7 +311,7 @@ export async function runTestAllCommand(opts = {}) { }); continue; } - results.push(await runProviderTest(db, connection)); + results.push(await runProviderTest(db, connection, { serverUp })); } if (opts.json) { diff --git a/bin/cli/commands/test-provider.mjs b/bin/cli/commands/test-provider.mjs index 8802f75cd1..ec10c24649 100644 --- a/bin/cli/commands/test-provider.mjs +++ b/bin/cli/commands/test-provider.mjs @@ -38,12 +38,19 @@ export async function runTestProviderCommand(provider, model, opts = {}) { } const targetProvider = provider || "anthropic"; - const targetModel = model || "claude-haiku-4-5-20251001"; + const connections = await _loadConnections(); + if (!connections) return 1; + const connection = _resolveConnection(connections, targetProvider, model); + if (!connection) { + console.error(`Provider connection not found: ${targetProvider}`); + return 1; + } + const targetModel = model || connection.defaultModel; const repeat = opts.repeat && opts.repeat > 0 ? opts.repeat : 1; const results = []; for (let i = 0; i < repeat; i++) { - const result = await _runSingleTest(targetProvider, targetModel); + const result = await _runSingleTest(connection, targetModel); results.push(result); } @@ -70,18 +77,10 @@ export async function runTestProviderCommand(provider, model, opts = {}) { } async function _runAllProviders(opts) { - const res = await apiFetch("/api/providers?limit=200", { - retry: false, - timeout: 5000, - acceptNotOk: true, - }); - if (!res.ok) { - console.error(t("test.noServer")); - return 1; - } - const data = await res.json(); - const connections = (data.connections ?? data.providers ?? data.items ?? data).filter( - (c) => c.authType === "apikey" || c.testStatus !== "unavailable" + const loaded = await _loadConnections(); + if (!loaded) return 1; + const connections = loaded.filter( + (c) => c.isActive !== false && (c.authType === "apikey" || c.testStatus !== "unavailable") ); if (connections.length === 0) { console.log(t("test.noProviders")); @@ -89,6 +88,7 @@ async function _runAllProviders(opts) { } const providers = connections.map((c) => ({ + connectionId: c.id, provider: c.provider ?? c.id, model: c.defaultModel ?? c.model, })); @@ -102,8 +102,8 @@ async function _runAllProviders(opts) { } const results = await Promise.all( - providers.map(async ({ provider, model }) => { - const r = await _runSingleTest(provider, model); + providers.map(async ({ connectionId, provider, model }) => { + const r = await _runSingleTest({ id: connectionId }, model); return { provider, model, ...r }; }) ); @@ -123,6 +123,13 @@ async function _runAllProviders(opts) { async function _runCompare(provider, opts) { const targetProvider = provider || "anthropic"; + const connections = await _loadConnections(); + if (!connections) return 1; + const connection = _resolveConnection(connections, targetProvider); + if (!connection) { + console.error(`Provider connection not found: ${targetProvider}`); + return 1; + } const models = opts.compare .split(",") .map((m) => m.trim()) @@ -138,7 +145,7 @@ async function _runCompare(provider, opts) { for (const model of models) { const results = []; for (let i = 0; i < repeat; i++) { - const result = await _runSingleTest(targetProvider, model); + const result = await _runSingleTest(connection, model); results.push(result); } rows.push({ model, ..._aggregate(results, true) }); @@ -180,19 +187,55 @@ async function _runCompare(provider, opts) { return rows.every((r) => r.success) ? 0 : 1; } -async function _runSingleTest(provider, model) { +async function _loadConnections() { + const res = await apiFetch("/api/providers?limit=200", { + retry: false, + timeout: 5000, + acceptNotOk: true, + }); + if (!res.ok) { + console.error(t("test.noServer")); + return null; + } + const data = await res.json(); + const connections = data.connections ?? data.providers ?? data.items ?? data; + if (!Array.isArray(connections)) { + console.error(t("test.noServer")); + return null; + } + return connections; +} + +function _resolveConnection(connections, selector, model) { + const normalized = String(selector || "") + .trim() + .toLowerCase(); + const active = connections.filter((connection) => connection.isActive !== false); + return ( + active.find((connection) => String(connection.id || "").toLowerCase() === normalized) ?? + active.find((connection) => String(connection.name || "").toLowerCase() === normalized) ?? + active.find( + (connection) => + String(connection.provider || "").toLowerCase() === normalized && + (!model || connection.defaultModel === model || connection.model === model) + ) ?? + active.find((connection) => String(connection.provider || "").toLowerCase() === normalized) + ); +} + +async function _runSingleTest(connection, model) { const startMs = Date.now(); try { - const res = await apiFetch("/api/v1/providers/test", { + const res = await apiFetch(`/api/providers/${encodeURIComponent(connection.id)}/test`, { method: "POST", - body: { provider, model }, + body: model ? { validationModelId: model } : {}, retry: false, timeout: 30000, acceptNotOk: true, }); const durationMs = Date.now() - startMs; - const data = res.ok ? await res.json() : { success: false, error: `HTTP ${res.status}` }; - return { ...data, durationMs }; + const data = res.ok ? await res.json() : { valid: false, error: `HTTP ${res.status}` }; + return { ...data, success: data.valid === true, durationMs }; } catch (err) { const msg = err instanceof Error ? err.message : String(err); return { diff --git a/bin/cli/tui/ProvidersTestAll.jsx b/bin/cli/tui/ProvidersTestAll.jsx index 73fc78614a..c1911888ac 100644 --- a/bin/cli/tui/ProvidersTestAll.jsx +++ b/bin/cli/tui/ProvidersTestAll.jsx @@ -1,6 +1,7 @@ import React, { useState, useEffect, useCallback } from "react"; import { render, Box, Text, useInput } from "ink"; import Spinner from "ink-spinner"; +import { apiFetch } from "../api.mjs"; import { DataTable } from "../tui-components/DataTable.jsx"; import { ProgressBar } from "../tui-components/ProgressBar.jsx"; @@ -31,22 +32,20 @@ const TABLE_SCHEMA = [ { key: "error", header: "Error", width: 28, formatter: (v) => (v ? v.slice(0, 26) : "") }, ]; -async function testOne(provider, model, baseUrl, apiKey) { - const headers = { - "Content-Type": "application/json", - ...(apiKey ? { Authorization: `Bearer ${apiKey}` } : {}), - }; +async function testOne(connectionId, model, baseUrl, apiKey) { const start = Date.now(); try { - const res = await fetch(`${baseUrl}/api/v1/providers/test`, { + const res = await apiFetch(`/api/providers/${encodeURIComponent(connectionId)}/test`, { method: "POST", - headers, - body: JSON.stringify({ provider, model }), - signal: AbortSignal.timeout(30000), + body: model ? { validationModelId: model } : {}, + baseUrl, + token: apiKey, + timeout: 30000, + acceptNotOk: true, }); const latencyMs = Date.now() - start; - const data = res.ok ? await res.json() : { success: false, error: `HTTP ${res.status}` }; - return { status: data.success ? STATUS.PASS : STATUS.FAIL, latencyMs, error: data.error }; + const data = res.ok ? await res.json() : { valid: false, error: `HTTP ${res.status}` }; + return { status: data.valid ? STATUS.PASS : STATUS.FAIL, latencyMs, error: data.error }; } catch (err) { const msg = err instanceof Error ? err.message : String(err); return { @@ -63,6 +62,7 @@ function ProvidersTestAllApp({ providers, baseUrl, apiKey, concurrency = 4, onEx const [rows, setRows] = useState(() => providers.map((p, i) => ({ id: i, + connectionId: p.connectionId ?? p.id, provider: p.provider ?? p.id ?? String(p), model: p.model ?? p.defaultModel ?? "", status: STATUS.PENDING, @@ -91,7 +91,7 @@ function ProvidersTestAllApp({ providers, baseUrl, apiKey, concurrency = 4, onEx const row = queue[cursor++]; running++; update(row.id, { status: STATUS.RUNNING }); - testOne(row.provider, row.model, resolved, apiKey).then((result) => { + testOne(row.connectionId, row.model, resolved, apiKey).then((result) => { update(row.id, result); running--; nextSlot(); diff --git a/tests/unit/cli-expanded-commands.test.ts b/tests/unit/cli-expanded-commands.test.ts index 1327351555..e631a084c6 100644 --- a/tests/unit/cli-expanded-commands.test.ts +++ b/tests/unit/cli-expanded-commands.test.ts @@ -319,8 +319,8 @@ test("test-provider --all-providers consumes the connections envelope", async () if (url.includes("/api/providers?limit=200")) { return Promise.resolve(new Response(JSON.stringify({ connections }), { status: 200 })); } - if (url.includes("/api/v1/providers/test")) { - return Promise.resolve(new Response(JSON.stringify({ success: true }), { status: 201 })); + if (url.includes("/api/providers/") && url.includes("/test")) { + return Promise.resolve(new Response(JSON.stringify({ valid: true }), { status: 200 })); } throw new Error(`unexpected URL: ${url}`); }) as typeof fetch; diff --git a/tests/unit/cli-provider-test-routes-10570.test.ts b/tests/unit/cli-provider-test-routes-10570.test.ts new file mode 100644 index 0000000000..386a3e448f --- /dev/null +++ b/tests/unit/cli-provider-test-routes-10570.test.ts @@ -0,0 +1,177 @@ +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"; +import Database from "better-sqlite3"; + +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +const ORIGINAL_FETCH = globalThis.fetch; +const ORIGINAL_API_KEY = process.env.OMNIROUTE_API_KEY; + +function jsonResponse(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +async function withCliEnv(fn: (dataDir: string) => Promise) { + const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cli-routes-10570-")); + process.env.DATA_DIR = dataDir; + process.env.OMNIROUTE_API_KEY = "test-management-key"; + delete process.env.STORAGE_ENCRYPTION_KEY; + try { + await fn(dataDir); + } finally { + globalThis.fetch = ORIGINAL_FETCH; + fs.rmSync(dataDir, { recursive: true, force: true }); + if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = ORIGINAL_DATA_DIR; + if (ORIGINAL_API_KEY === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = ORIGINAL_API_KEY; + } +} + +async function createConnection( + dataDir: string, + input: { provider?: string; name?: string; apiKey?: string } = {} +) { + const { ensureProviderSchema, upsertApiKeyProviderConnection } = + await import("../../bin/cli/provider-store.mjs"); + const db = new Database(path.join(dataDir, "storage.sqlite")); + ensureProviderSchema(db); + const connection = upsertApiKeyProviderConnection(db, { + provider: input.provider ?? "custom-openai-compatible", + name: input.name ?? "Custom Connection", + apiKey: input.apiKey ?? "test-key", + }); + db.close(); + return connection; +} + +test("omniroute test resolves a connection and calls its server-owned test route", async () => { + await withCliEnv(async () => { + const requests: Array<{ path: string; method: string }> = []; + globalThis.fetch = (async (input, init) => { + const url = new URL(String(input)); + const method = String(init?.method ?? "GET").toUpperCase(); + requests.push({ path: `${url.pathname}${url.search}`, method }); + if (url.pathname === "/api/health") return jsonResponse({ status: "ok" }); + if (url.pathname === "/api/providers") { + return jsonResponse({ + connections: [ + { + id: "conn/custom 1", + provider: "custom-openai-compatible", + name: "Custom Connection", + authType: "apikey", + isActive: true, + defaultModel: "custom-model", + }, + ], + total: 1, + }); + } + if (url.pathname === "/api/providers/conn%2Fcustom%201/test" && method === "POST") { + return jsonResponse({ valid: true, error: null, latencyMs: 7 }); + } + return jsonResponse({ error: "unexpected route" }, 404); + }) as typeof fetch; + + const { runTestProviderCommand } = await import("../../bin/cli/commands/test-provider.mjs"); + const exitCode = await runTestProviderCommand("custom-openai-compatible", undefined, { + json: true, + }); + + assert.equal(exitCode, 0); + assert.deepEqual(requests, [ + { path: "/api/health", method: "GET" }, + { path: "/api/providers?limit=200", method: "GET" }, + { path: "/api/providers/conn%2Fcustom%201/test", method: "POST" }, + ]); + }); +}); + +test("omniroute test --all-providers consumes the current connections response shape", async () => { + await withCliEnv(async () => { + const requests: Array<{ path: string; method: string }> = []; + globalThis.fetch = (async (input, init) => { + const url = new URL(String(input)); + const method = String(init?.method ?? "GET").toUpperCase(); + requests.push({ path: `${url.pathname}${url.search}`, method }); + if (url.pathname === "/api/health") return jsonResponse({ status: "ok" }); + if (url.pathname === "/api/providers") { + return jsonResponse({ + connections: [ + { + id: "conn-all-1", + provider: "custom-openai-compatible", + name: "Custom Connection", + authType: "apikey", + isActive: true, + defaultModel: "custom-model", + }, + ], + total: 1, + }); + } + if (url.pathname === "/api/providers/conn-all-1/test" && method === "POST") { + return jsonResponse({ valid: true, error: null }); + } + return jsonResponse({ error: "unexpected route" }, 404); + }) as typeof fetch; + + const { runTestProviderCommand } = await import("../../bin/cli/commands/test-provider.mjs"); + const exitCode = await runTestProviderCommand(undefined, undefined, { + allProviders: true, + json: true, + }); + + assert.equal(exitCode, 0); + assert.deepEqual(requests, [ + { path: "/api/health", method: "GET" }, + { path: "/api/providers?limit=200", method: "GET" }, + { path: "/api/providers/conn-all-1/test", method: "POST" }, + ]); + }); +}); + +test("the interactive all-provider view uses the same connection-owned test route", async () => { + const source = await fs.promises.readFile( + new URL("../../bin/cli/tui/ProvidersTestAll.jsx", import.meta.url), + "utf8" + ); + assert.match(source, /import \{ apiFetch \} from "\.\.\/api\.mjs"/); + assert.match(source, /connectionId: p\.connectionId \?\? p\.id/); + assert.match(source, /apiFetch\(/); + assert.match(source, /\/api\/providers\/\$\{encodeURIComponent\(connectionId\)\}\/test/); + assert.match(source, /data\.valid/); + assert.doesNotMatch(source, /api\/v1\/providers\/test/); +}); + +test("providers test-all falls back to the server for unsupported custom API-key providers", async () => { + await withCliEnv(async (dataDir) => { + const connection = await createConnection(dataDir); + const requests: Array<{ path: string; method: string }> = []; + globalThis.fetch = (async (input, init) => { + const url = new URL(String(input)); + const method = String(init?.method ?? "GET").toUpperCase(); + requests.push({ path: url.pathname, method }); + if (url.pathname === "/api/health") return jsonResponse({ status: "ok" }); + if (url.pathname === `/api/providers/${connection.id}/test` && method === "POST") { + return jsonResponse({ valid: true, error: null, latencyMs: 9 }); + } + return jsonResponse({ error: "unexpected route" }, 404); + }) as typeof fetch; + + const { runTestAllCommand } = await import("../../bin/cli/commands/providers.mjs"); + const exitCode = await runTestAllCommand({ json: true }); + + assert.equal(exitCode, 0); + assert.deepEqual(requests, [ + { path: "/api/health", method: "GET" }, + { path: `/api/providers/${connection.id}/test`, method: "POST" }, + ]); + }); +});