diff --git a/src/lib/llm/utils/config.ts b/src/lib/llm/utils/config.ts index 661136af..9d5162d4 100644 --- a/src/lib/llm/utils/config.ts +++ b/src/lib/llm/utils/config.ts @@ -18,6 +18,8 @@ export const DEFAULT_MODELS: Record = { custom: "gpt-3.5-turbo", }; +const VALID_PROVIDERS = Object.keys(DEFAULT_MODELS) as LLMProviderType[]; + export const DEFAULT_API_URLS: Record = { ollama: "http://localhost:11434/v1", openai: "https://api.openai.com/v1", @@ -27,20 +29,14 @@ export const DEFAULT_API_URLS: Record = { // Environment Resolution // ============================================================================ -function getEnvVar(key: string): string | undefined { - return process.env[key]; -} - function resolveProvider(): LLMProviderType { - const provider = getEnvVar("LLM_PROVIDER")?.toLowerCase(); + const provider = process.env.LLM_PROVIDER?.toLowerCase(); if (!provider) { return DEFAULT_PROVIDER; } - const validProviders: LLMProviderType[] = ["gemini", "openai", "ollama", "custom"]; - - if (!validProviders.includes(provider as LLMProviderType)) { + if (!VALID_PROVIDERS.includes(provider as LLMProviderType)) { console.error(`[LLM] Invalid provider "${provider}", falling back to "${DEFAULT_PROVIDER}"`); return DEFAULT_PROVIDER; } @@ -49,7 +45,7 @@ function resolveProvider(): LLMProviderType { } function resolveApiKey(provider: LLMProviderType): string | undefined { - const apiKey = getEnvVar("LLM_API_KEY"); + const apiKey = process.env.LLM_API_KEY; // Ollama doesn't require API key if (!apiKey && provider === "ollama") { @@ -60,13 +56,13 @@ function resolveApiKey(provider: LLMProviderType): string | undefined { } function resolveModel(provider: LLMProviderType): string { - const model = getEnvVar("LLM_MODEL"); + const model = process.env.LLM_MODEL; return model || DEFAULT_MODELS[provider]; } function resolveApiUrl(provider: LLMProviderType): string | undefined { // Primary: LLM_API_URL - const apiUrl = getEnvVar("LLM_API_URL"); + const apiUrl = process.env.LLM_API_URL; if (apiUrl) { return apiUrl; } @@ -116,10 +112,9 @@ export function resolveConfig(overrides?: Partial): LLMConfig { */ export function validateConfig(config: LLMConfig): void { // Validate provider - const validProviders: LLMProviderType[] = ["gemini", "openai", "ollama", "custom"]; - if (!validProviders.includes(config.provider)) { + if (!VALID_PROVIDERS.includes(config.provider)) { throw new LLMConfigError( - `Invalid provider: ${config.provider}. Valid options: ${validProviders.join(", ")}`, + `Invalid provider: ${config.provider}. Valid options: ${VALID_PROVIDERS.join(", ")}`, config.provider, ); } diff --git a/tests/unit/env-documentation.test.ts b/tests/unit/env-documentation.test.ts index e84dbc70..30a08343 100644 --- a/tests/unit/env-documentation.test.ts +++ b/tests/unit/env-documentation.test.ts @@ -30,11 +30,10 @@ * * What stays out of scope: a name that reaches `process.env[...]` as a function * argument or parameter, because resolving it needs a call graph and a regex - * that faked one would report a name that is not a name. Two reads are in that - * position today -- `getEnvVar("LLM_PROVIDER")` in `src/lib/llm/utils/config.ts` - * and `process.env[envVar]` in `src/lib/seed/credential-resolver.ts` -- leaving - * `HOSTNAME`, `MY_DB_PASSWORD` and the four `LLM_*` names undiscovered. That is - * the whole remaining gap, not an aside. + * that faked one would report a name that is not a name. The only remaining + * read in that position is `process.env[envVar]` in + * `src/lib/seed/credential-resolver.ts`, leaving `HOSTNAME` and `MY_DB_PASSWORD` + * undiscovered. The four direct `LLM_*` reads are covered. */ import { describe, expect, test } from "bun:test"; import { readdirSync, readFileSync, statSync } from "node:fs"; @@ -175,16 +174,20 @@ describe("environment variable documentation", () => { }); test("#609 boundary: a name reached through a function argument stays undiscovered", () => { - // getEnvVar("LLM_PROVIDER") in src/lib/llm/utils/config.ts and - // process.env[envVar] in src/lib/seed/credential-resolver.ts need a call + // process.env[envVar] in src/lib/seed/credential-resolver.ts needs a call // graph. When that stops being true, this test is the reminder to widen the // extractor rather than a silent gain. const names = new Set(readNames()); // Control: a negative-only test passes on an empty set, so anchor it to a // name the extractor must always find before trusting the absences below. expect(names.has("JWT_SECRET")).toBe(true); - for (const name of ["LLM_PROVIDER", "LLM_API_KEY", "LLM_MODEL", "LLM_API_URL", "MY_DB_PASSWORD"]) { - expect(names.has(name)).toBe(false); + expect(names.has("MY_DB_PASSWORD")).toBe(false); + }); + + test("LLM configuration reads are visible to the documentation guard", () => { + const names = new Set(readNames()); + for (const name of ["LLM_PROVIDER", "LLM_API_KEY", "LLM_MODEL", "LLM_API_URL"]) { + expect(names.has(name), name).toBe(true); } }); diff --git a/tests/unit/llm/config.test.ts b/tests/unit/llm/config.test.ts index 29bd27ff..a1101897 100644 --- a/tests/unit/llm/config.test.ts +++ b/tests/unit/llm/config.test.ts @@ -8,7 +8,7 @@ import { DEFAULT_MODELS, DEFAULT_API_URLS, } from "@/lib/llm/utils/config"; -import { LLMConfigError } from "@/lib/llm/types"; +import { LLMConfigError, type LLMProviderType } from "@/lib/llm/types"; // ============================================================================ // Environment Variable Helpers @@ -129,6 +129,22 @@ describe("resolveConfig", () => { // ============================================================================ describe("validateConfig", () => { + test("accepts every provider in DEFAULT_MODELS and rejects an unknown provider", () => { + for (const provider of Object.keys(DEFAULT_MODELS) as LLMProviderType[]) { + expect(() => + validateConfig({ + provider, + model: DEFAULT_MODELS[provider], + apiKey: "test-key", + apiUrl: "https://example.test/v1", + }), + ).not.toThrow(); + } + expect(() => validateConfig({ provider: "unknown-provider" as LLMProviderType, model: "test-model" })).toThrow( + LLMConfigError, + ); + }); + test("valid gemini config with key passes", () => { expect(() => validateConfig({