fix(ai): bound the provider test and explain why it failed (#3319)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Amruth Pillai <im.amruth@gmail.com>

packages/api/src/features/ai-providers/e2e.test.ts

@@ -0,0 +1,203 @@
+/**
+ * End-to-end coverage for the provider connection test.
+ *
+ * Everything from the oRPC procedure down is the real thing: the router (including its auth
+ * middleware and error mapping), the service, `testConnection`, the failure classifier, and the
+ * AES-GCM credential encryption. Only the database and the outbound HTTP call are substituted —
+ * the database with an in-memory row store, the provider with a stubbed `fetch`.
+ */
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { createRouterClient } from "@orpc/server";
+
+const { dbMock, dbState } = vi.hoisted(() => {
+	const state = { rows: [] as Record<string, unknown>[] };
+
+	const selectChain = {
+		from: () => selectChain,
+		where: () => selectChain,
+		orderBy: async () => state.rows,
+		limit: async () => state.rows,
+	};
+
+	const db = {
+		select: () => selectChain,
+		update: () => ({
+			set: (values: Record<string, unknown>) => ({
+				where: () => {
+					const applied = state.rows.map((row) => Object.assign(row, values));
+					return Object.assign(Promise.resolve(applied), { returning: async () => applied });
+				},
+			}),
+		}),
+	};
+
+	return { dbMock: db, dbState: state };
+});
+
+vi.mock("@reactive-resume/db/client", () => ({ db: dbMock }));
+vi.mock("@reactive-resume/db/schema", () => ({
+	aiProvider: { id: "ai_provider.id", userId: "ai_provider.user_id" },
+	user: { id: "user.id" },
+}));
+vi.mock("drizzle-orm", () => ({
+	and: (...conditions: unknown[]) => ({ conditions }),
+	asc: (value: unknown) => ({ value }),
+	desc: (value: unknown) => ({ value }),
+	eq: (left: unknown, right: unknown) => ({ left, right }),
+	sql: () => ({}),
+}));
+
+// Real AES-GCM credential encryption runs; it only needs a secret.
+vi.mock("@reactive-resume/env/server", () => ({
+	env: { ENCRYPTION_SECRET: "e2e-encryption-secret", FLAG_ALLOW_UNSAFE_AI_BASE_URL: true },
+}));
+
+vi.mock("@reactive-resume/auth/config", () => ({
+	auth: {
+		api: {
+			getSession: async () => ({ user: { id: "user-1", email: "kaushik@example.test" } }),
+			verifyApiKey: async () => ({ valid: false, key: null }),
+		},
+	},
+	verifyOAuthToken: async () => null,
+}));
+
+const { aiProvidersRouter } = await import("./router");
+const { encryptCredential } = await import("../ai/credentials");
+
+const client = createRouterClient(aiProvidersRouter, {
+	context: { locale: "en-US" as const, reqHeaders: new Headers() },
+});
+
+function seedProvider(overrides: Record<string, unknown> = {}) {
+	const credential = encryptCredential("sk-live-demo-key-1234");
+
+	dbState.rows = [
+		{
+			id: "provider-1",
+			userId: "user-1",
+			label: "My provider",
+			provider: "openai",
+			model: "gpt-4.1",
+			baseUrl: "https://api.openai.test/v1",
+			enabled: false,
+			testStatus: "untested",
+			testError: null,
+			lastTestedAt: null,
+			lastUsedAt: null,
+			createdAt: new Date("2026-08-01T00:00:00Z"),
+			updatedAt: new Date("2026-08-01T00:00:00Z"),
+			...credential,
+			...overrides,
+		},
+	];
+}
+
+function stubProvider(status: number, body: unknown) {
+	const fetchMock = vi.fn(
+		() =>
+			new Response(typeof body === "string" ? body : JSON.stringify(body), {
+				status,
+				headers: { "Content-Type": "application/json" },
+			}),
+	);
+	vi.stubGlobal("fetch", fetchMock);
+	return fetchMock;
+}
+
+function chatCompletion(content: string) {
+	return {
+		id: "chatcmpl-1",
+		object: "chat.completion",
+		created: 1,
+		model: "gpt-4.1",
+		choices: [{ index: 0, message: { role: "assistant", content }, finish_reason: "stop" }],
+		usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 },
+	};
+}
+
+afterEach(() => {
+	vi.unstubAllGlobals();
+});
+
+describe("POST /ai-providers/{id}/test — end to end", () => {
+	beforeEach(() => {
+		seedProvider();
+	});
+
+	it("resolves with a usable provider and enables it", async () => {
+		stubProvider(200, chatCompletion("1"));
+
+		const response = await client.test({ id: "provider-1" });
+
+		expect(response.testStatus).toBe("success");
+		expect(response.testError).toBeNull();
+		expect(response.enabled).toBe(true);
+		// The persisted row is what the dashboard reads on the next page load.
+		expect(dbState.rows[0]).toMatchObject({ testStatus: "success", testError: null, enabled: true });
+	});
+
+	it("resolves — not rejects — when the provider rejects the key, and explains why", async () => {
+		stubProvider(401, { error: { message: "Incorrect API key provided." } });
+
+		// Before this change the procedure threw BAD_GATEWAY, so this call would reject.
+		const response = await client.test({ id: "provider-1" });
+
+		expect(response.testStatus).toBe("failure");
+		expect(response.testError).toBe("OpenAI rejected the API key.");
+		expect(response.enabled).toBe(false);
+		expect(dbState.rows[0]?.testError).toBe("OpenAI rejected the API key.");
+	});
+
+	it("names the model when the provider returns not-found", async () => {
+		stubProvider(404, {});
+
+		const response = await client.test({ id: "provider-1" });
+
+		expect(response.testError).toBe('OpenAI has no model named "gpt-4.1", or the base URL is wrong.');
+	});
+
+	it("attributes an outage to the provider and does not retry", async () => {
+		const fetchMock = stubProvider(503, {});
+
+		const response = await client.test({ id: "provider-1" });
+
+		expect(response.testError).toBe("OpenAI reported a server error (503). This is a problem on the provider's side.");
+		expect(fetchMock).toHaveBeenCalledTimes(1);
+	});
+
+	it("separates a reachable provider from a usable one", async () => {
+		stubProvider(200, chatCompletion("Of course! I am connected."));
+
+		const response = await client.test({ id: "provider-1" });
+
+		expect(response.testStatus).toBe("failure");
+		expect(response.testError).toContain("reachable, but the model replied with unexpected output");
+	});
+
+	it("never leaks the decrypted API key into the persisted error", async () => {
+		stubProvider(400, { error: { message: "Bad request for key sk-live-demo-key-1234" } });
+
+		const response = await client.test({ id: "provider-1" });
+
+		expect(response.testError).not.toContain("sk-live-demo-key-1234");
+		expect(response.testError).toContain("***");
+	});
+
+	it("still rejects with BAD_REQUEST when the base URL is not permitted", async () => {
+		// A blocked address must remain a configuration error, not a provider failure.
+		seedProvider({ baseUrl: "ftp://api.openai.test/v1" });
+		stubProvider(200, chatCompletion("1"));
+
+		await expect(client.test({ id: "provider-1" })).rejects.toMatchObject({
+			code: "BAD_REQUEST",
+			message: "Invalid AI provider configuration.",
+		});
+	});
+
+	it("rejects with NOT_FOUND when the provider belongs to nobody", async () => {
+		dbState.rows = [];
+
+		await expect(client.test({ id: "provider-1" })).rejects.toMatchObject({ code: "NOT_FOUND" });
+	});
+});

packages/api/src/features/ai-providers/service.ts

@@ -228,19 +228,21 @@ export const aiProvidersService = {
 		const apiKey = decryptCredential(provider.encryptedApiKey);
 
 		try {
-			const ok = await testConnection({
+			const result = await testConnection({
 				provider: parsedProvider,
 				model: provider.model,
 				apiKey,
 				baseURL: provider.baseUrl ?? "",
 			});
 
+			// A provider that answers "no" is a completed test, not a failed request: it comes back as
+			// data so the client can show why, instead of a generic transport error.
 			const [updated] = await db
 				.update(schema.aiProvider)
 				.set({
-					enabled: ok,
-					testStatus: ok ? "success" : "failure",
-					testError: ok ? null : "The provider test returned an unexpected response.",
+					enabled: result.ok,
+					testStatus: result.ok ? "success" : "failure",
+					testError: result.ok ? null : result.message,
 					lastTestedAt: new Date(),
 				})
 				.where(and(eq(schema.aiProvider.id, input.id), eq(schema.aiProvider.userId, input.userId)))
@@ -249,6 +251,7 @@ export const aiProvidersService = {
 			if (!updated) throw new ORPCError("NOT_FOUND");
 			return toResponse(updated);
 		} catch (error) {
+			// Only unexpected failures reach here now: provider-side outcomes come back as data above.
 			await db
 				.update(schema.aiProvider)
 				.set({

packages/api/src/features/ai/service.test.ts

@@ -44,8 +44,130 @@ function stubOpenAICompatibleResponse(response?: { content?: string; finishReaso
 	return { fetchMock, getRequestBody: () => requestBody };
 }
 
+function testInput(overrides?: { model?: string; apiKey?: string; baseURL?: string }) {
+	return {
+		provider: "openai-compatible" as const,
+		model: overrides?.model ?? "test-model",
+		apiKey: overrides?.apiKey ?? "test-key",
+		baseURL: overrides?.baseURL ?? "https://example.test/v1",
+	};
+}
+
+function stubFailedResponse(status: number, body = "{}") {
+	const fetchMock = vi.fn(() => new Response(body, { status, headers: { "Content-Type": "application/json" } }));
+	vi.stubGlobal("fetch", fetchMock);
+
+	return fetchMock;
+}
+
+function stubRejectedFetch(error: unknown) {
+	const fetchMock = vi.fn(() => Promise.reject(error));
+	vi.stubGlobal("fetch", fetchMock);
+
+	return fetchMock;
+}
+
 const { testConnection } = await import("./service");
 
+describe("AI provider connection test", () => {
+	it("names the rejected key instead of reporting a transport failure", async () => {
+		stubFailedResponse(401);
+
+		await expect(testConnection(testInput())).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("rejected the API key"),
+		});
+	});
+
+	it("points at the model when the provider returns a not-found status", async () => {
+		stubFailedResponse(404);
+
+		await expect(testConnection(testInput({ model: "missing-model" }))).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining('no model named "missing-model"'),
+		});
+	});
+
+	it("distinguishes rate limiting from an outage", async () => {
+		stubFailedResponse(429);
+
+		await expect(testConnection(testInput())).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("rate-limited"),
+		});
+	});
+
+	it("attributes a server error to the provider", async () => {
+		stubFailedResponse(503);
+
+		await expect(testConnection(testInput())).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("server error (503)"),
+		});
+	});
+
+	it("reports an unreachable base URL rather than the socket error", async () => {
+		stubRejectedFetch(
+			Object.assign(new TypeError("fetch failed"), {
+				cause: Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:11434"), { code: "ECONNREFUSED" }),
+			}),
+		);
+
+		await expect(testConnection(testInput({ baseURL: "https://example.test/v1" }))).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("Could not reach https://example.test/v1"),
+		});
+	});
+
+	it("explains a timeout in terms of the wait the user just sat through", async () => {
+		stubRejectedFetch(new DOMException("The operation was aborted due to timeout", "TimeoutError"));
+
+		await expect(testConnection(testInput())).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("did not respond within 30 seconds"),
+		});
+	});
+
+	it("does not retry, so the test cannot silently multiply its own wait", async () => {
+		const fetchMock = stubFailedResponse(503);
+
+		await testConnection(testInput());
+
+		expect(fetchMock).toHaveBeenCalledTimes(1);
+	});
+
+	it("separates a reachable provider from a usable one", async () => {
+		stubOpenAICompatibleResponse({ content: "Sure! The connection works.", finishReason: "stop" });
+
+		await expect(testConnection(testInput())).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("reachable"),
+		});
+	});
+
+	it("keeps the API key out of a passed-through provider message", async () => {
+		stubFailedResponse(400, JSON.stringify({ error: { message: "Bad key sk-secret-value-123" } }));
+
+		const result = await testConnection(testInput({ apiKey: "sk-secret-value-123" }));
+
+		expect(result.ok).toBe(false);
+		if (!result.ok) expect(result.message).not.toContain("sk-secret-value-123");
+	});
+
+	// The credentials schema accepts a single character, so short keys must be redacted too.
+	it("redacts a short API key as well", async () => {
+		stubFailedResponse(400, JSON.stringify({ error: { message: "Bad key tok123" } }));
+
+		const result = await testConnection(testInput({ apiKey: "tok123" }));
+
+		expect(result.ok).toBe(false);
+		if (!result.ok) {
+			expect(result.message).not.toContain("tok123");
+			expect(result.message).toContain("***");
+		}
+	});
+});
+
 describe("AI chat service", () => {
 	it("tests OpenAI-compatible providers without requiring structured output", async () => {
 		const openAiCompatible = stubOpenAICompatibleResponse();
@@ -57,7 +179,7 @@ describe("AI chat service", () => {
 				apiKey: "test-key",
 				baseURL: "https://example.test/v1",
 			}),
-		).resolves.toBe(true);
+		).resolves.toEqual({ ok: true });
 
 		expect(openAiCompatible.fetchMock).toHaveBeenCalledTimes(1);
 		expect(openAiCompatible.getRequestBody()).not.toHaveProperty("response_format");
@@ -67,14 +189,10 @@ describe("AI chat service", () => {
 	it("explains when the provider test hits the output limit", async () => {
 		stubOpenAICompatibleResponse({ content: "1. The connection works.", finishReason: "length" });
 
-		await expect(
-			testConnection({
-				provider: "openai-compatible",
-				model: "test-model",
-				apiKey: "test-key",
-				baseURL: "https://example.test/v1",
-			}),
-		).rejects.toThrow("The model returned too much text during the provider test.");
+		await expect(testConnection(testInput())).resolves.toMatchObject({
+			ok: false,
+			message: expect.stringContaining("returned too much text"),
+		});
 	});
 
 	it("keeps proposal tool history valid for follow-up chat messages", async () => {

packages/api/src/features/ai/service.ts

@@ -17,7 +17,17 @@ import { createPerplexity } from "@ai-sdk/perplexity";
 import { createTogetherAI } from "@ai-sdk/togetherai";
 import { createXai } from "@ai-sdk/xai";
 import { streamToEventIterator } from "@orpc/server";
-import { convertToModelMessages, createGateway, generateText, stepCountIs, streamText, tool } from "ai";
+import {
+	APICallError,
+	convertToModelMessages,
+	createGateway,
+	generateText,
+	LoadAPIKeyError,
+	NoSuchModelError,
+	stepCountIs,
+	streamText,
+	tool,
+} from "ai";
 import { createOllama } from "ollama-ai-provider-v2";
 import { match } from "ts-pattern";
 import { z } from "zod";
@@ -36,7 +46,7 @@ import {
 	resumePatchProposalToolInputSchema,
 	resumePatchProposalToolOutputSchema,
 } from "@reactive-resume/ai/tools/patch-proposal";
-import { aiProviderSchema } from "@reactive-resume/ai/types";
+import { AI_PROVIDER_DEFAULT_BASE_URLS, AI_PROVIDER_DISPLAY_NAMES, aiProviderSchema } from "@reactive-resume/ai/types";
 import { applyResumePatches } from "@reactive-resume/resume/patch";
 import { resumeAnalysisSchema } from "@reactive-resume/schema/resume/analysis";
 import { supportsProviderNativeWebSearch } from "./capabilities";
@@ -83,6 +93,8 @@ type GetModelInput = {
 const MAX_AI_FILE_BYTES = 10 * 1024 * 1024; // 10MB
 const MAX_AI_FILE_BASE64_CHARS = Math.ceil((MAX_AI_FILE_BYTES * 4) / 3) + 4;
 const TEST_CONNECTION_MAX_OUTPUT_TOKENS = 128;
+// Long enough for a cold local model to load, short enough that the UI does not look frozen.
+const TEST_CONNECTION_TIMEOUT_MS = 30_000;
 const DOCX_DOCUMENT_XML_PATH = "word/document.xml";
 const ZIP_LOCAL_FILE_HEADER_SIGNATURE = 0x04034b50;
 const ZIP_CENTRAL_DIRECTORY_SIGNATURE = 0x02014b50;
@@ -144,20 +156,135 @@ export const fileInputSchema = z.object({
 
 type TestConnectionInput = z.infer<typeof aiCredentialsSchema>;
 
-export async function testConnection(input: TestConnectionInput): Promise<boolean> {
-	const RESPONSE_OK = "1";
+type TestConnectionResult = { ok: true } | { ok: false; message: string };
 
-	const result = await generateText({
-		model: getModel(input),
-		maxOutputTokens: TEST_CONNECTION_MAX_OUTPUT_TOKENS,
-		temperature: 0,
-		messages: [{ role: "user", content: `Respond only with the single character: ${RESPONSE_OK}` }],
+const NETWORK_ERROR_CODES = new Set([
+	"ECONNREFUSED",
+	"ECONNRESET",
+	"EAI_AGAIN",
+	"ENOTFOUND",
+	"ETIMEDOUT",
+	"UND_ERR_CONNECT_TIMEOUT",
+	"UND_ERR_SOCKET",
+]);
+
+// Provider SDKs wrap the useful error (a socket failure, an abort) inside a generic one, so the
+// signal we need is usually a few `cause` hops down rather than on the error we are handed.
+function findInCauseChain<T>(error: unknown, pick: (candidate: unknown) => T | undefined): T | undefined {
+	let current = error;
+
+	for (let depth = 0; depth < 8 && current !== null && current !== undefined; depth++) {
+		const picked = pick(current);
+		if (picked !== undefined) return picked;
+		current = (current as { cause?: unknown }).cause;
+	}
+
+	return undefined;
+}
+
+function isTimeout(error: unknown): boolean {
+	return (
+		findInCauseChain(error, (candidate) =>
+			candidate instanceof Error && (candidate.name === "TimeoutError" || candidate.name === "AbortError")
+				? true
+				: undefined,
+		) ?? false
+	);
+}
+
+function findNetworkErrorCode(error: unknown): string | undefined {
+	return findInCauseChain(error, (candidate) => {
+		const code = (candidate as { code?: unknown }).code;
+
+		return typeof code === "string" && NETWORK_ERROR_CODES.has(code) ? code : undefined;
 	});
+}
 
-	if (result.text.trim() === RESPONSE_OK) return true;
-	if (result.finishReason === "length") throw new Error("The model returned too much text during the provider test.");
+// The key is never part of a response body, but a provider echoing the request would leak it into
+// the message we persist, so scrub it from anything we did not write ourselves. The schema allows
+// keys as short as one character, and a mangled message costs less than a leaked credential, so
+// every non-empty key is redacted. The guard only keeps `replaceAll` from splicing "***" between
+// every character of the message.
+function redactApiKey(message: string, apiKey: string): string {
+	if (!apiKey) return message;
 
-	return false;
+	return message.replaceAll(apiKey, "***");
+}
+
+function describeTestConnectionFailure(input: TestConnectionInput, error: unknown): string {
+	const provider = AI_PROVIDER_DISPLAY_NAMES[input.provider];
+	const endpoint = input.baseURL.trim() || AI_PROVIDER_DEFAULT_BASE_URLS[input.provider];
+
+	if (isTimeout(error)) {
+		return `${provider} did not respond within ${TEST_CONNECTION_TIMEOUT_MS / 1000} seconds. The service may be unreachable, or the model may be too slow to load.`;
+	}
+
+	if (findNetworkErrorCode(error)) {
+		return endpoint
+			? `Could not reach ${endpoint}. Check that the base URL is correct and the service is running.`
+			: `${provider} could not be reached. Check that the base URL is correct and the service is running.`;
+	}
+
+	if (LoadAPIKeyError.isInstance(error)) return `${provider} was configured without an API key.`;
+	if (NoSuchModelError.isInstance(error)) return `${provider} has no model named "${input.model}".`;
+
+	if (APICallError.isInstance(error)) {
+		const status = error.statusCode;
+
+		if (status === 401 || status === 403) return `${provider} rejected the API key.`;
+		if (status === 404) return `${provider} has no model named "${input.model}", or the base URL is wrong.`;
+		if (status === 429) return `${provider} rate-limited the test. Wait a moment and try again.`;
+		if (status !== undefined && status >= 500) {
+			return `${provider} reported a server error (${status}). This is a problem on the provider's side.`;
+		}
+
+		return `${provider} rejected the test request${status === undefined ? "" : ` (${status})`}: ${redactApiKey(error.message, input.apiKey)}`;
+	}
+
+	if (error instanceof Error && error.message) {
+		return `${provider} could not be tested: ${redactApiKey(error.message, input.apiKey)}`;
+	}
+
+	return `${provider} could not be tested, and reported no reason.`;
+}
+
+export async function testConnection(input: TestConnectionInput): Promise<TestConnectionResult> {
+	const RESPONSE_OK = "1";
+	const provider = AI_PROVIDER_DISPLAY_NAMES[input.provider];
+
+	// Resolved outside the try so the base-URL policy error still reaches the router, which turns it
+	// into an invalid-configuration response rather than a provider failure.
+	const model = getModel(input);
+
+	let result: Awaited<ReturnType<typeof generateText>>;
+
+	try {
+		result = await generateText({
+			model,
+			maxOutputTokens: TEST_CONNECTION_MAX_OUTPUT_TOKENS,
+			temperature: 0,
+			// A connection test must not silently multiply its own wait by retrying behind the user.
+			maxRetries: 0,
+			abortSignal: AbortSignal.timeout(TEST_CONNECTION_TIMEOUT_MS),
+			messages: [{ role: "user", content: `Respond only with the single character: ${RESPONSE_OK}` }],
+		});
+	} catch (error) {
+		return { ok: false, message: describeTestConnectionFailure(input, error) };
+	}
+
+	if (result.text.trim() === RESPONSE_OK) return { ok: true };
+
+	if (result.finishReason === "length") {
+		return {
+			ok: false,
+			message: `${provider} is reachable, but the model returned too much text during the test. Try a model that follows short instructions.`,
+		};
+	}
+
+	return {
+		ok: false,
+		message: `${provider} is reachable, but the model replied with unexpected output instead of a simple confirmation.`,
+	};
 }
 
 type ParsePdfInput = z.infer<typeof aiCredentialsSchema> & {