From ac32224ce25dded76b5b286d3894711055c240fa Mon Sep 17 00:00:00 2001 From: Ning Date: Wed, 27 May 2026 22:02:54 -0700 Subject: [PATCH] fix(openai-embeddings): honor embedding-specific API key in factory and auto-detect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #503. PR #503 added OPENAI_EMBEDDING_API_KEY / OPENAI_EMBEDDING_BASE_URL to OpenAIEmbeddingProvider but two call sites still bypass them, so the env vars do not take effect when set alongside OPENAI_API_KEY: 1. src/providers/embedding/index.ts:38 — factory passes getEnvVar("OPENAI_API_KEY")! into the constructor's apiKey arg, beating the constructor's documented precedence (apiKey > OPENAI_EMBEDDING_API_KEY > OPENAI_API_KEY). 2. src/config.ts — detectEmbeddingProvider() does not consider OPENAI_EMBEDDING_API_KEY, so an embedding-only OpenAI config paired with a different chat provider (e.g. Gemini) misroutes embeddings to whichever generic provider key is also set. Reproduces against real OpenAI: OPENAI_API_KEY=sk-deepseek-... # chat on a self-hosted/proxy OPENAI_EMBEDDING_API_KEY=sk-openai-real-... OPENAI_EMBEDDING_BASE_URL=https://api.openai.com Embedding requests 401 because the LLM key is sent. The bug is invisible when the embedding endpoint ignores Authorization (local vLLM / Ollama / LM Studio), which is why #503's verification missed it. Fix: - index.ts: drop the explicit OPENAI_API_KEY argument; the constructor already documents and implements the correct precedence. - config.ts: check OPENAI_EMBEDDING_API_KEY ahead of generic provider keys so "chat on X, embeddings on OpenAI" auto-detects without forcing EMBEDDING_PROVIDER=openai. - .env.example, README.md, openai.ts JSDoc: sync docs to match the now-functional behavior. - test/embedding-provider.test.ts: 3 regression tests (both-keys factory path, embedding-only detect, gemini+embedding precedence); expand beforeEach to strip embedding-specific env vars. Compatibility: zero behavior change for users not setting OPENAI_EMBEDDING_*; only the two currently-broken configurations above shift, which is the intent. Signed-off-by: Ning --- .env.example | 12 +++++++--- README.md | 2 ++ src/config.ts | 6 +++++ src/providers/embedding/index.ts | 2 +- src/providers/embedding/openai.ts | 2 +- test/embedding-provider.test.ts | 40 +++++++++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index 77ca0f3a3..995928ed7 100644 --- a/.env.example +++ b/.env.example @@ -63,8 +63,9 @@ # ----------------------------------------------------------------------------- # # Without an embedding key, agentmemory runs in BM25-only mode for hybrid -# search. Detection order: EMBEDDING_PROVIDER override → GEMINI_API_KEY → -# OPENAI_API_KEY → VOYAGE_API_KEY → COHERE_API_KEY → OPENROUTER_API_KEY → +# search. Detection order: EMBEDDING_PROVIDER override → +# OPENAI_EMBEDDING_API_KEY → GEMINI_API_KEY → OPENAI_API_KEY → +# VOYAGE_API_KEY → COHERE_API_KEY → OPENROUTER_API_KEY → # local (Xenova/all-MiniLM-L6-v2, 384-dim). # EMBEDDING_PROVIDER=local # local | openai | voyage | cohere | gemini | openrouter @@ -73,7 +74,12 @@ # COHERE_API_KEY=... # General-purpose embeddings -# Reuses OPENAI_API_KEY / OPENAI_BASE_URL above when EMBEDDING_PROVIDER=openai. +# When EMBEDDING_PROVIDER=openai, embeddings reuse OPENAI_API_KEY / +# OPENAI_BASE_URL above by default. Set the two vars below to point +# embeddings at a different endpoint / key than the chat LLM — e.g. chat +# on a self-hosted OpenAI-compatible proxy, embeddings on OpenAI proper. +# OPENAI_EMBEDDING_API_KEY=sk-... # Embedding-specific key; takes precedence over OPENAI_API_KEY +# OPENAI_EMBEDDING_BASE_URL=https://api.openai.com # Embedding-specific base URL; takes precedence over OPENAI_BASE_URL # OPENAI_EMBEDDING_MODEL=text-embedding-3-small # Embedding model when EMBEDDING_PROVIDER=openai # OPENAI_EMBEDDING_DIMENSIONS=1536 # Required when the model is not in the known-models table diff --git a/README.md b/README.md index da5a2dc1a..37329544f 100644 --- a/README.md +++ b/README.md @@ -1261,6 +1261,8 @@ Create `~/.agentmemory/.env`: # VOYAGE_API_KEY=... # OPENAI_API_KEY=sk-... # OPENAI_BASE_URL=https://api.openai.com # Override for Azure / vLLM / LM Studio / proxies +# OPENAI_EMBEDDING_API_KEY=sk-... # Optional: embedding-specific key; takes precedence over OPENAI_API_KEY +# OPENAI_EMBEDDING_BASE_URL=https://api.openai.com # Optional: embedding-specific base URL; takes precedence over OPENAI_BASE_URL # OPENAI_EMBEDDING_MODEL=text-embedding-3-small # OPENAI_EMBEDDING_DIMENSIONS=1536 # Required when the model is not in the known-models table diff --git a/src/config.ts b/src/config.ts index 4162eefa0..943d9aa6e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -226,6 +226,12 @@ export function detectEmbeddingProvider( const forced = source["EMBEDDING_PROVIDER"]; if (forced) return forced; + // Embedding-specific key beats generic provider auto-detection: it's a + // stronger signal of intent than a chat-LLM key that merely shares the + // OpenAI wire shape. Without this, "chat on Gemini, embeddings on OpenAI" + // silently routes embeddings through the Gemini branch. + if (source["OPENAI_EMBEDDING_API_KEY"]) return "openai"; + if (source["GEMINI_API_KEY"]) return "gemini"; if (source["OPENAI_API_KEY"]) return "openai"; if (source["VOYAGE_API_KEY"]) return "voyage"; diff --git a/src/providers/embedding/index.ts b/src/providers/embedding/index.ts index d18de2328..58f48a9fe 100644 --- a/src/providers/embedding/index.ts +++ b/src/providers/embedding/index.ts @@ -35,7 +35,7 @@ export function createEmbeddingProvider(): EmbeddingProvider | null { case "gemini": return withDimensionGuard(new GeminiEmbeddingProvider(getEnvVar("GEMINI_API_KEY")!)); case "openai": - return withDimensionGuard(new OpenAIEmbeddingProvider(getEnvVar("OPENAI_API_KEY")!)); + return withDimensionGuard(new OpenAIEmbeddingProvider()); case "voyage": return withDimensionGuard(new VoyageEmbeddingProvider(getEnvVar("VOYAGE_API_KEY")!)); case "cohere": diff --git a/src/providers/embedding/openai.ts b/src/providers/embedding/openai.ts index 7384d5137..a256aba2b 100644 --- a/src/providers/embedding/openai.ts +++ b/src/providers/embedding/openai.ts @@ -48,7 +48,7 @@ function resolveDimensions(model: string, override: string | undefined): number * `api-key` header instead of `Authorization: Bearer`. * * Required env vars: - * OPENAI_API_KEY — API key (fallback for OPENAI_EMBEDDING_API_KEY) + * OPENAI_API_KEY — Fallback API key when OPENAI_EMBEDDING_API_KEY is unset * * Optional: * OPENAI_BASE_URL — base URL without path (default: https://api.openai.com). diff --git a/test/embedding-provider.test.ts b/test/embedding-provider.test.ts index 6c2d263ec..fa19cd6a9 100644 --- a/test/embedding-provider.test.ts +++ b/test/embedding-provider.test.ts @@ -18,6 +18,11 @@ describe("createEmbeddingProvider", () => { delete process.env["COHERE_API_KEY"]; delete process.env["OPENROUTER_API_KEY"]; delete process.env["EMBEDDING_PROVIDER"]; + delete process.env["OPENAI_BASE_URL"]; + delete process.env["OPENAI_EMBEDDING_API_KEY"]; + delete process.env["OPENAI_EMBEDDING_BASE_URL"]; + delete process.env["OPENAI_EMBEDDING_MODEL"]; + delete process.env["OPENAI_EMBEDDING_DIMENSIONS"]; }); afterEach(() => { @@ -50,6 +55,41 @@ describe("createEmbeddingProvider", () => { const provider = createEmbeddingProvider(); expect(provider).toBeInstanceOf(OpenAIEmbeddingProvider); }); + + it("uses OPENAI_EMBEDDING_API_KEY in Authorization when both keys are set", async () => { + process.env["OPENAI_API_KEY"] = "llm-key"; + process.env["OPENAI_EMBEDDING_API_KEY"] = "embedding-key"; + const provider = createEmbeddingProvider(); + expect(provider).toBeInstanceOf(OpenAIEmbeddingProvider); + + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify({ data: [{ embedding: new Array(1536).fill(0.1) }] }), + { status: 200 }, + ), + ); + + await provider!.embed("hi"); + const headers = (fetchSpy.mock.calls[0]![1] as RequestInit).headers as Record; + expect(headers["Authorization"]).toBe("Bearer embedding-key"); + + fetchSpy.mockRestore(); + }); + + it("detects openai when only OPENAI_EMBEDDING_API_KEY is set", () => { + process.env["OPENAI_EMBEDDING_API_KEY"] = "embedding-only-key"; + const provider = createEmbeddingProvider(); + expect(provider).toBeInstanceOf(OpenAIEmbeddingProvider); + expect(provider!.name).toBe("openai"); + }); + + it("OPENAI_EMBEDDING_API_KEY takes precedence over GEMINI_API_KEY for embedding detection", () => { + process.env["GEMINI_API_KEY"] = "gemini-chat-key"; + process.env["OPENAI_EMBEDDING_API_KEY"] = "embedding-key"; + const provider = createEmbeddingProvider(); + expect(provider).toBeInstanceOf(OpenAIEmbeddingProvider); + expect(provider!.name).toBe("openai"); + }); }); describe("OpenAIEmbeddingProvider", () => {