Add automated tests for model discovery (POST /providers/list-models)
Task #25: the model-discovery capability (list available models, used by the guided provider setup) had no automated coverage. Added a new vitest suite that exercises the endpoint end-to-end against the in-process Express app. New file: - artifacts/api-server/src/routes/providers.listModels.test.ts Coverage (6 tests, all passing): - ok=false + clear German message when no token (empty token, no providerId), and the upstream provider is never called. - Falls back to the stored provider token when providerId is given and apiToken is empty (inserts a real provider row, asserts the Bearer header carries the stored token, cleans up afterward). - Normalizes the OpenAI-compatible response (data[].id) into a deduped, sorted model list; drops non-string ids. - Anthropic path: GET /models with x-api-key + anthropic-version headers (no Authorization), reads models[] with id/name fallback, dedupes. - Upstream failure returns ok=false (HTTP 200, not 500), empty models, and the token is redacted from the message ([REDACTED], never the raw token). - fetch throwing (network error) returns ok=false without leaking the token. Implementation note: the suite runs the app in-process and the test client also uses fetch, so global fetch is mocked with a passthrough — requests to the test server's baseUrl delegate to the captured real fetch; only upstream provider URLs are synthesized. Spy assertions filter out the localhost passthrough call. Saved this non-obvious testing lesson to memory. Deviation / note: pre-existing failures in relation.test.ts and compare.test.ts are unrelated to this task — the dev database's scans table is missing the fingerprint/relation/similarity/compared_scan_id columns (schema drift; needs a drizzle-kit push). Out of scope for this task; proposed as a follow-up. Replit-Task-Id: 7e8a3db2-0da7-40d9-b74d-132779a44d39
This commit is contained in:
parent
d54b3ace19
commit
2f7e58670a
3 changed files with 310 additions and 0 deletions
|
|
@ -4,3 +4,4 @@
|
|||
- [Skill fingerprint & relation matching](skill-fingerprint-matching.md) — don't put display name in fingerprint path; match modified by file-path Jaccard (hash-Jaccard misses single-file edits), report content-aware similarity.
|
||||
- [Testing api-server from shell](api-server-local-curl.md) — external `$REPLIT_DEV_DOMAIN/api` curl returns HTTP 000; curl `http://localhost:<PORT>/api` instead (port from workflow log).
|
||||
- [Stale codegen & unapplied migrations](skillguard-stale-codegen-and-migrations.md) — "field already in API" tasks: dev/test DB + lib `dist/*.d.ts` lag; run drizzle push + `tsc -b` the lib.
|
||||
- [Mocking fetch in api-server route tests](api-server-fetch-mocking-in-tests.md) — route tests run app in-process; delegate localhost requests to real fetch, only synthesize upstream; filter spy calls by URL.
|
||||
|
|
|
|||
24
.agents/memory/api-server-fetch-mocking-in-tests.md
Normal file
24
.agents/memory/api-server-fetch-mocking-in-tests.md
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
---
|
||||
name: Mocking fetch in api-server route tests
|
||||
description: How to mock upstream fetch without breaking the test client's own request to the in-process Express server
|
||||
---
|
||||
|
||||
When testing an api-server route that itself calls `fetch` (e.g. provider model
|
||||
discovery / connection tests), the route tests spin up the real Express app via
|
||||
`app.listen(0)` and hit it with `fetch(`${baseUrl}/api/...`)`. The app runs in the
|
||||
SAME process as the test, so `vi.spyOn(globalThis, "fetch")` intercepts BOTH the
|
||||
test client's request to the server AND the route's upstream provider call.
|
||||
|
||||
**Rule:** capture `const realFetch = globalThis.fetch.bind(globalThis)` BEFORE
|
||||
mocking, and in the mock implementation delegate any request whose URL starts with
|
||||
the test server's `baseUrl` back to `realFetch`. Only synthesize a response for the
|
||||
upstream (non-localhost) URL.
|
||||
|
||||
**Also:** the spy records the localhost passthrough call too, so don't assert on
|
||||
`mock.calls[0]` — filter to the call whose URL does NOT start with `baseUrl` to find
|
||||
the real upstream request. Likewise `not.toHaveBeenCalled()` / `toHaveBeenCalledTimes(1)`
|
||||
must be expressed in terms of the filtered upstream calls, not the raw spy.
|
||||
|
||||
**Why:** without the passthrough, the test client's request to the in-process server
|
||||
gets the synthetic upstream body (or the thrown error), so every test fails before the
|
||||
route logic runs.
|
||||
285
artifacts/api-server/src/routes/providers.listModels.test.ts
Normal file
285
artifacts/api-server/src/routes/providers.listModels.test.ts
Normal file
|
|
@ -0,0 +1,285 @@
|
|||
import {
|
||||
describe,
|
||||
it,
|
||||
expect,
|
||||
beforeAll,
|
||||
afterAll,
|
||||
afterEach,
|
||||
vi,
|
||||
} from "vitest";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import type { Server } from "node:http";
|
||||
import app from "../app";
|
||||
import { db, pool, aiProvidersTable } from "@workspace/db";
|
||||
import { inArray } from "drizzle-orm";
|
||||
|
||||
const createdProviderIds: number[] = [];
|
||||
let server: Server;
|
||||
let baseUrl: string;
|
||||
|
||||
// The test client itself uses fetch to reach the in-process server, so we must
|
||||
// only intercept upstream provider calls and let localhost requests pass through
|
||||
// to the real implementation.
|
||||
const realFetch = globalThis.fetch.bind(globalThis);
|
||||
|
||||
beforeAll(async () => {
|
||||
await new Promise<void>((resolve) => {
|
||||
server = app.listen(0, () => resolve());
|
||||
});
|
||||
const { port } = server.address() as AddressInfo;
|
||||
baseUrl = `http://127.0.0.1:${port}`;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
if (createdProviderIds.length > 0) {
|
||||
await db
|
||||
.delete(aiProvidersTable)
|
||||
.where(inArray(aiProvidersTable.id, createdProviderIds.splice(0)));
|
||||
}
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.close((err) => (err ? reject(err) : resolve()));
|
||||
});
|
||||
await pool.end();
|
||||
});
|
||||
|
||||
type FetchResponseInit = {
|
||||
ok: boolean;
|
||||
status?: number;
|
||||
jsonBody?: unknown;
|
||||
textBody?: string;
|
||||
};
|
||||
|
||||
function mockFetch(resp: FetchResponseInit): ReturnType<typeof vi.fn> {
|
||||
const fn = vi.fn(
|
||||
async (input: unknown, init?: RequestInit): Promise<Response> => {
|
||||
const url = typeof input === "string" ? input : String(input);
|
||||
// Let the test client's own request to the in-process server through.
|
||||
if (url.startsWith(baseUrl)) {
|
||||
return realFetch(input as never, init as never);
|
||||
}
|
||||
return {
|
||||
ok: resp.ok,
|
||||
status: resp.status ?? (resp.ok ? 200 : 500),
|
||||
json: async () => resp.jsonBody ?? {},
|
||||
text: async () => resp.textBody ?? "",
|
||||
} as unknown as Response;
|
||||
},
|
||||
);
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(
|
||||
fn as unknown as typeof fetch,
|
||||
);
|
||||
return fn;
|
||||
}
|
||||
|
||||
// The spy records both the test client's localhost request and the upstream
|
||||
// provider request. Pick the upstream one (anything not aimed at our server).
|
||||
function upstreamCall(
|
||||
fn: ReturnType<typeof vi.fn>,
|
||||
): [string, RequestInit] {
|
||||
const call = fn.mock.calls.find(
|
||||
(c) => !String(c[0]).startsWith(baseUrl),
|
||||
);
|
||||
if (!call) throw new Error("no upstream fetch call was recorded");
|
||||
return [String(call[0]), call[1] as RequestInit];
|
||||
}
|
||||
|
||||
async function listModels(body: Record<string, unknown>): Promise<{
|
||||
status: number;
|
||||
json: { ok: boolean; models: string[]; message?: string | null };
|
||||
}> {
|
||||
const res = await fetch(`${baseUrl}/api/providers/list-models`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
const json = (await res.json()) as {
|
||||
ok: boolean;
|
||||
models: string[];
|
||||
message?: string | null;
|
||||
};
|
||||
return { status: res.status, json };
|
||||
}
|
||||
|
||||
async function insertProvider(apiToken: string | null): Promise<number> {
|
||||
const [row] = await db
|
||||
.insert(aiProvidersTable)
|
||||
.values({
|
||||
name: `list-models-test ${Math.random().toString(36).slice(2)}`,
|
||||
apiType: "openai",
|
||||
baseUrl: "https://api.example.test/v1",
|
||||
model: "",
|
||||
apiToken,
|
||||
enabled: true,
|
||||
})
|
||||
.returning();
|
||||
createdProviderIds.push(row.id);
|
||||
return row.id;
|
||||
}
|
||||
|
||||
describe("POST /api/providers/list-models", () => {
|
||||
it("returns ok=false with a clear message when no token is supplied", async () => {
|
||||
const upstream = mockFetch({ ok: true, jsonBody: { data: [] } });
|
||||
|
||||
const { status, json } = await listModels({
|
||||
apiType: "openai",
|
||||
baseUrl: "https://api.example.test/v1",
|
||||
apiToken: "",
|
||||
});
|
||||
|
||||
expect(status).toBe(200);
|
||||
expect(json.ok).toBe(false);
|
||||
expect(json.models).toEqual([]);
|
||||
expect(json.message).toBe("Kein API-Token angegeben.");
|
||||
// It must short-circuit before ever calling the upstream provider.
|
||||
const upstreamCalls = upstream.mock.calls.filter(
|
||||
(c) => !String(c[0]).startsWith(baseUrl),
|
||||
);
|
||||
expect(upstreamCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("falls back to the stored token when providerId is given and apiToken is empty", async () => {
|
||||
const storedToken = "stored-secret-token-1234567890";
|
||||
const providerId = await insertProvider(storedToken);
|
||||
const upstream = mockFetch({
|
||||
ok: true,
|
||||
jsonBody: { data: [{ id: "gpt-4o" }] },
|
||||
});
|
||||
|
||||
const { json } = await listModels({
|
||||
apiType: "openai",
|
||||
baseUrl: "https://api.example.test/v1",
|
||||
apiToken: "",
|
||||
providerId,
|
||||
});
|
||||
|
||||
expect(json.ok).toBe(true);
|
||||
expect(json.models).toEqual(["gpt-4o"]);
|
||||
|
||||
// The stored token was used for the Bearer header.
|
||||
const [, init] = upstreamCall(upstream);
|
||||
const headers = init.headers as Record<string, string>;
|
||||
expect(headers.Authorization).toBe(`Bearer ${storedToken}`);
|
||||
});
|
||||
|
||||
it("normalizes the OpenAI-compatible response into a deduped, sorted list of model IDs", async () => {
|
||||
const upstream = mockFetch({
|
||||
ok: true,
|
||||
jsonBody: {
|
||||
data: [
|
||||
{ id: "gpt-4o" },
|
||||
{ id: "gpt-3.5-turbo" },
|
||||
{ id: "gpt-4o" }, // duplicate
|
||||
{ id: "claude-ignored-by-shape" }, // still a string id
|
||||
{ id: 12345 }, // non-string id is dropped
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
const { json } = await listModels({
|
||||
apiType: "openai",
|
||||
baseUrl: "https://api.example.test/v1",
|
||||
apiToken: "tok-abc",
|
||||
});
|
||||
|
||||
expect(json.ok).toBe(true);
|
||||
expect(json.models).toEqual([
|
||||
"claude-ignored-by-shape",
|
||||
"gpt-3.5-turbo",
|
||||
"gpt-4o",
|
||||
]);
|
||||
|
||||
// OpenAI-compatible path: GET /models with a Bearer header.
|
||||
const [url, init] = upstreamCall(upstream);
|
||||
expect(url).toBe("https://api.example.test/v1/models");
|
||||
expect(init.method).toBe("GET");
|
||||
const headers = init.headers as Record<string, string>;
|
||||
expect(headers.Authorization).toBe("Bearer tok-abc");
|
||||
expect(headers["x-api-key"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("uses Anthropic headers (x-api-key + anthropic-version) and reads the models array", async () => {
|
||||
const upstream = mockFetch({
|
||||
ok: true,
|
||||
jsonBody: {
|
||||
models: [
|
||||
{ id: "claude-3-5-sonnet" },
|
||||
{ name: "claude-3-haiku" }, // name fallback when id is absent
|
||||
{ id: "claude-3-5-sonnet" }, // duplicate
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
const { json } = await listModels({
|
||||
apiType: "anthropic",
|
||||
baseUrl: "https://api.anthropic.test/v1",
|
||||
apiToken: "anthropic-key-xyz",
|
||||
});
|
||||
|
||||
expect(json.ok).toBe(true);
|
||||
expect(json.models).toEqual(["claude-3-5-sonnet", "claude-3-haiku"]);
|
||||
|
||||
const [url, init] = upstreamCall(upstream);
|
||||
expect(url).toBe("https://api.anthropic.test/v1/models");
|
||||
expect(init.method).toBe("GET");
|
||||
const headers = init.headers as Record<string, string>;
|
||||
expect(headers["x-api-key"]).toBe("anthropic-key-xyz");
|
||||
expect(headers["anthropic-version"]).toBe("2023-06-01");
|
||||
expect(headers.Authorization).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns ok=false (not a 500) and never leaks the token when the upstream call fails", async () => {
|
||||
const token = "leaky-secret-token-abcdef123456";
|
||||
mockFetch({
|
||||
ok: false,
|
||||
status: 401,
|
||||
// The upstream error body echoes the token back to us.
|
||||
textBody: `{"error":"invalid api key ${token}, Bearer ${token}"}`,
|
||||
});
|
||||
|
||||
const { status, json } = await listModels({
|
||||
apiType: "openai",
|
||||
baseUrl: "https://api.example.test/v1",
|
||||
apiToken: token,
|
||||
});
|
||||
|
||||
// Errors are surfaced gracefully, not as a 500.
|
||||
expect(status).toBe(200);
|
||||
expect(json.ok).toBe(false);
|
||||
expect(json.models).toEqual([]);
|
||||
expect(typeof json.message).toBe("string");
|
||||
|
||||
// The token must be redacted from the error message.
|
||||
expect(json.message).not.toContain(token);
|
||||
expect(json.message).toContain("[REDACTED]");
|
||||
expect(json.message).toContain("HTTP 401");
|
||||
});
|
||||
|
||||
it("returns ok=false without leaking the token when fetch itself throws", async () => {
|
||||
const token = "throwy-secret-token-zzz999";
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation((async (
|
||||
input: unknown,
|
||||
init?: RequestInit,
|
||||
) => {
|
||||
const url = typeof input === "string" ? input : String(input);
|
||||
if (url.startsWith(baseUrl)) {
|
||||
return realFetch(input as never, init as never);
|
||||
}
|
||||
throw new Error("network down");
|
||||
}) as unknown as typeof fetch);
|
||||
|
||||
const { status, json } = await listModels({
|
||||
apiType: "openai",
|
||||
baseUrl: "https://api.example.test/v1",
|
||||
apiToken: token,
|
||||
});
|
||||
|
||||
expect(status).toBe(200);
|
||||
expect(json.ok).toBe(false);
|
||||
expect(json.models).toEqual([]);
|
||||
expect(json.message).not.toContain(token);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue