From 6c88e2b96d49bb703c058561870c75b65bbb9a9b Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 5 Feb 2026 16:21:17 -0600 Subject: [PATCH] fix(#338): Don't instantiate OpenAI client with missing API key - Skip client initialization when OPENAI_API_KEY not configured - Set openai property to null instead of creating with dummy key - Methods return gracefully when embeddings not available - Updated tests to verify client is not instantiated without key Refs #338 Co-Authored-By: Claude Opus 4.5 --- .../services/embedding.service.spec.ts | 103 +++++++++++++----- .../knowledge/services/embedding.service.ts | 13 +-- 2 files changed, 82 insertions(+), 34 deletions(-) diff --git a/apps/api/src/knowledge/services/embedding.service.spec.ts b/apps/api/src/knowledge/services/embedding.service.spec.ts index 8d552d0..786aa6e 100644 --- a/apps/api/src/knowledge/services/embedding.service.spec.ts +++ b/apps/api/src/knowledge/services/embedding.service.spec.ts @@ -1,12 +1,28 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; import { EmbeddingService } from "./embedding.service"; import { PrismaService } from "../../prisma/prisma.service"; +// Mock OpenAI with a proper class +const mockEmbeddingsCreate = vi.fn(); +vi.mock("openai", () => { + return { + default: class MockOpenAI { + embeddings = { + create: mockEmbeddingsCreate, + }; + }, + }; +}); + describe("EmbeddingService", () => { let service: EmbeddingService; let prismaService: PrismaService; + let originalEnv: string | undefined; beforeEach(() => { + // Store original env + originalEnv = process.env.OPENAI_API_KEY; + prismaService = { $executeRaw: vi.fn(), knowledgeEmbedding: { @@ -14,36 +30,65 @@ describe("EmbeddingService", () => { }, } as unknown as PrismaService; - service = new EmbeddingService(prismaService); + // Clear mock call history + vi.clearAllMocks(); }); + afterEach(() => { + // Restore original env + if (originalEnv) { + process.env.OPENAI_API_KEY = originalEnv; + } else { + delete process.env.OPENAI_API_KEY; + } + }); + + describe("constructor", () => { + it("should not instantiate OpenAI client when API key is missing", () => { + delete process.env.OPENAI_API_KEY; + + service = new EmbeddingService(prismaService); + + // Verify service is not configured (client is null) + expect(service.isConfigured()).toBe(false); + }); + + it("should instantiate OpenAI client when API key is provided", () => { + process.env.OPENAI_API_KEY = "test-api-key"; + + service = new EmbeddingService(prismaService); + + // Verify service is configured (client is not null) + expect(service.isConfigured()).toBe(true); + }); + }); + + // Default service setup (without API key) for remaining tests + function createServiceWithoutKey(): EmbeddingService { + delete process.env.OPENAI_API_KEY; + return new EmbeddingService(prismaService); + } + describe("isConfigured", () => { it("should return false when OPENAI_API_KEY is not set", () => { - const originalEnv = process.env["OPENAI_API_KEY"]; - delete process.env["OPENAI_API_KEY"]; + service = createServiceWithoutKey(); expect(service.isConfigured()).toBe(false); - - if (originalEnv) { - process.env["OPENAI_API_KEY"] = originalEnv; - } }); it("should return true when OPENAI_API_KEY is set", () => { - const originalEnv = process.env["OPENAI_API_KEY"]; - process.env["OPENAI_API_KEY"] = "test-key"; + process.env.OPENAI_API_KEY = "test-key"; + service = new EmbeddingService(prismaService); expect(service.isConfigured()).toBe(true); - - if (originalEnv) { - process.env["OPENAI_API_KEY"] = originalEnv; - } else { - delete process.env["OPENAI_API_KEY"]; - } }); }); describe("prepareContentForEmbedding", () => { + beforeEach(() => { + service = createServiceWithoutKey(); + }); + it("should combine title and content with title weighting", () => { const title = "Test Title"; const content = "Test content goes here"; @@ -68,20 +113,19 @@ describe("EmbeddingService", () => { describe("generateAndStoreEmbedding", () => { it("should skip generation when not configured", async () => { - const originalEnv = process.env["OPENAI_API_KEY"]; - delete process.env["OPENAI_API_KEY"]; + service = createServiceWithoutKey(); await service.generateAndStoreEmbedding("test-id", "test content"); expect(prismaService.$executeRaw).not.toHaveBeenCalled(); - - if (originalEnv) { - process.env["OPENAI_API_KEY"] = originalEnv; - } }); }); describe("deleteEmbedding", () => { + beforeEach(() => { + service = createServiceWithoutKey(); + }); + it("should delete embedding for entry", async () => { const entryId = "test-entry-id"; @@ -95,8 +139,7 @@ describe("EmbeddingService", () => { describe("batchGenerateEmbeddings", () => { it("should return 0 when not configured", async () => { - const originalEnv = process.env["OPENAI_API_KEY"]; - delete process.env["OPENAI_API_KEY"]; + service = createServiceWithoutKey(); const entries = [ { id: "1", content: "content 1" }, @@ -106,10 +149,16 @@ describe("EmbeddingService", () => { const result = await service.batchGenerateEmbeddings(entries); expect(result).toBe(0); + }); + }); - if (originalEnv) { - process.env["OPENAI_API_KEY"] = originalEnv; - } + describe("generateEmbedding", () => { + it("should throw error when not configured", async () => { + service = createServiceWithoutKey(); + + await expect(service.generateEmbedding("test text")).rejects.toThrow( + "OPENAI_API_KEY not configured" + ); }); }); }); diff --git a/apps/api/src/knowledge/services/embedding.service.ts b/apps/api/src/knowledge/services/embedding.service.ts index f1f653b..7211408 100644 --- a/apps/api/src/knowledge/services/embedding.service.ts +++ b/apps/api/src/knowledge/services/embedding.service.ts @@ -20,7 +20,7 @@ export interface EmbeddingOptions { @Injectable() export class EmbeddingService { private readonly logger = new Logger(EmbeddingService.name); - private readonly openai: OpenAI; + private readonly openai: OpenAI | null; private readonly defaultModel = "text-embedding-3-small"; constructor(private readonly prisma: PrismaService) { @@ -28,18 +28,17 @@ export class EmbeddingService { if (!apiKey) { this.logger.warn("OPENAI_API_KEY not configured - embedding generation will be disabled"); + this.openai = null; + } else { + this.openai = new OpenAI({ apiKey }); } - - this.openai = new OpenAI({ - apiKey: apiKey ?? "dummy-key", // Provide dummy key to allow instantiation - }); } /** * Check if the service is properly configured */ isConfigured(): boolean { - return !!process.env.OPENAI_API_KEY; + return this.openai !== null; } /** @@ -51,7 +50,7 @@ export class EmbeddingService { * @throws Error if OpenAI API key is not configured */ async generateEmbedding(text: string, options: EmbeddingOptions = {}): Promise { - if (!this.isConfigured()) { + if (!this.openai) { throw new Error("OPENAI_API_KEY not configured"); }