fix(cli): route provider tests through connection API (#10572)

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 <hydraxman@users.noreply.github.com>
2026-08-21 22:52:19 +03:00 · 2026-08-21 09:41:27 +08:00
parent 00aba8ef16
commit 28788cb9af
6 changed files with 290 additions and 38 deletions
--- 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)
--- 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) {
--- 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 {
--- 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();
--- 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;
--- a/tests/unit/cli-provider-test-routes-10570.test.ts
+++ 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<void>) {
+  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" },
+    ]);
+  });
+});