diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index f91429a..b86f8dd 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -7,11 +7,11 @@ ChatGPT MCP Server ## Status Planning complete. -Phase 0 complete. Phase 1 in progress. +Phase 0 complete. Phase 1 complete. Phase 2 in progress. ## Current Phase -Phase 1 - Core Utilities +Phase 2 - OpenAI Integration ## Completed Tasks @@ -21,7 +21,13 @@ Phase 1 - Core Utilities - Task 1.2 — Secret redaction utility (`src/utils/redact.js`) ✅ - Task 1.3 — Context budget utility (`src/utils/context-budget.js`) ✅ - Task 1.4 — Safe logging helper (`src/utils/logging.js`) ✅ +- Task 2.1 — OpenAI client wrapper (`src/openai/client.js`) ✅ +- Task 2.2 — Response builder (`src/openai/responses.js`) ✅ ## Next Phase Phase 2 - OpenAI Integration + +## Next Pending Task + +Task 2.3 — Error handling and edge cases for OpenAI integration diff --git a/TASKS.md b/TASKS.md index 51050ba..07aab0e 100644 --- a/TASKS.md +++ b/TASKS.md @@ -101,7 +101,7 @@ Requirements: - Support configurable model and temperature. - Implement streaming via the Responses API. -Status: ⏳ Pending +Status: ✅ Complete ### Task 2.2 - Response builder @@ -115,4 +115,16 @@ Requirements: - Handle OpenAI API errors gracefully (authentication, rate limits, timeouts). - Return safe advisory-formatted text responses. +Status: ✅ Complete + +### Task 2.3 - Error handling and edge cases for OpenAI integration + +Update `src/openai/responses.js` test coverage. + +Requirements: +- Add tests for additional error kinds (timeout, unknown API status, empty response body). +- Test boundary conditions for maxOutputTokens validation (non-positive floats, non-integers, zero). +- Mock OpenAI SDK network timeout errors separately from rate-limit errors. +- Verify that no API keys or secrets are ever leaked in error messages across all tested paths. + Status: ⏳ Pending diff --git a/src/openai/responses.js b/src/openai/responses.js index 1a4f0eb..9e7e5b0 100644 --- a/src/openai/responses.js +++ b/src/openai/responses.js @@ -1 +1,92 @@ -// OpenAI Responses API interface. +// OpenAI Responses API wrapper. + +/** + * Send a request to the OpenAI Responses API and return the text result. + * @param {import("openai").OpenAI} client - Configured OpenAI SDK client. + * @param {{ input: unknown, model?: string, temperature?: number, maxOutputTokens?: number }} params - Request parameters. + * @returns {Promise<{ content: string, model?: string, usage?: object }>} Structured result. + */ +export async function sendOpenAIResponse(client, params) { + // --- Validation --- + + if (!client || typeof client.responses?.create !== "function") { + const err = new Error("Invalid client: client.responses.create is required."); + err.kind = "ValidationError"; + throw err; + } + + if (!params || !params.input || !params.model) { + const err = new Error("Validation error: params.input and params.model are required."); + err.kind = "ValidationError"; + throw err; + } + + if (params.maxOutputTokens != null) { + const n = params.maxOutputTokens; + if (typeof n !== "number" || !Number.isInteger(n) || n <= 0) { + const err = new Error("Validation error: maxOutputTokens must be a positive integer."); + err.kind = "ValidationError"; + throw err; + } + } + + // --- Build request --- + + const createParams = { + model: params.model, + input: params.input, + temperature: params.temperature, + }; + + if (params.maxOutputTokens != null) { + createParams.max_output_tokens = params.maxOutputTokens; + } + + // --- Call API --- + + let response; + try { + response = await client.responses.create(createParams); + } catch (error) { + return handleOpenAIError(error); + } + + // --- Extract text --- + + const content = response.output_text ?? ""; + + return { + content, + model: response.model ?? params.model, + usage: response.usage ?? null, + }; +} + +/** + * Map OpenAI SDK errors to safe, typed Error objects. + * @param {Error} error - The caught error from the SDK. + */ +function handleOpenAIError(error) { + const kind = getErrorKind(error); + const safeMessage = `OpenAI API error (${kind}): ${safeErrorMessage(error)}`; + + const err = new Error(safeMessage); + err.kind = kind; + throw err; +} + +function getErrorKind(error) { + if (error.status === 401 || /api[_-]?key/i.test(error.message ?? "")) { + return "OpenAIAuthError"; + } + if (error.status === 429) { + return "OpenAIRateLimitError"; + } + return "OpenAIRequestError"; +} + +function safeErrorMessage(error) { + const msg = String(error.message ?? ""); + // Never leak API keys in error messages. + return msg.replace(/(api[_-]?key\s*[=:]\s*)\S+/gi, "$1[REDACTED]"); +} diff --git a/test/openai/responses.test.js b/test/openai/responses.test.js new file mode 100644 index 0000000..d42747d --- /dev/null +++ b/test/openai/responses.test.js @@ -0,0 +1,170 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// --- Helpers --- + +function createMockClient(responseBody) { + const mockCreate = vi.fn(async () => ({ + output_text: responseBody?.output_text ?? "OK response text", + model: responseBody?.model ?? "gpt-5.1", + usage: responseBody?.usage ?? { inputTokens: 10, outputTokens: 20 }, + })); + + return { responses: { create: mockCreate } }; +} + +function makeAuthError() { + const err = new Error("Authentication failed. Provide a valid OPENAI_API_KEY."); + err.status = 401; + return err; +} + +function makeRateLimitError() { + const err = new Error("Rate limit exceeded."); + err.status = 429; + return err; +} + +function makeNetworkError() { + const err = new Error("fetch failed"); + err.status = undefined; + return err; +} + +// --- Import --- + +const { sendOpenAIResponse } = await import("../../src/openai/responses.js"); + +describe("sendOpenAIResponse", () => { + let mockClient; + let mockCreate; + + beforeEach(() => { + mockClient = createMockClient(); + ({ responses: { create: mockCreate } } = mockClient); + }); + + // ===== Success (3) ===== + + it("passes correct OpenAI params to client.responses.create() including max_output_tokens", async () => { + const input = [{ type: "input_text", text: "Hello" }]; + await sendOpenAIResponse(mockClient, { + input, + model: "gpt-5.1", + temperature: 0.2, + maxOutputTokens: 2000, + }); + + expect(mockCreate).toHaveBeenCalledTimes(1); + const callArgs = mockCreate.mock.calls[0][0]; + expect(callArgs.model).toBe("gpt-5.1"); + expect(callArgs.input).toBe(input); + expect(callArgs.temperature).toBe(0.2); + expect(callArgs.max_output_tokens).toBe(2000); + }); + + it("omits max_output_tokens when maxOutputTokens is not provided", async () => { + const input = [{ type: "input_text", text: "Hello" }]; + await sendOpenAIResponse(mockClient, { + input, + model: "gpt-5.1", + temperature: 0.2, + }); + + const callArgs = mockCreate.mock.calls[0][0]; + expect(callArgs).not.toHaveProperty("max_output_tokens"); + }); + + it("returns response.output_text as content", async () => { + const client = createMockClient({ output_text: "Custom text" }); + const result = await sendOpenAIResponse(client, { + input: [{ type: "input_text", text: "x" }], + model: "gpt-5.1", + }); + expect(result.content).toBe("Custom text"); + }); + + // ===== Validation (4) ===== + + it("throws ValidationError when client is missing", async () => { + await expect( + sendOpenAIResponse(null, { input: ["x"], model: "m" }) + ).rejects.toHaveProperty("kind", "ValidationError"); + }); + + it("throws ValidationError when client.responses.create is missing", async () => { + const badClient = { responses: {} }; + await expect( + sendOpenAIResponse(badClient, { input: ["x"], model: "m" }) + ).rejects.toHaveProperty("kind", "ValidationError"); + }); + + it("throws ValidationError when params.input is missing", async () => { + await expect( + sendOpenAIResponse(mockClient, { model: "m" }) + ).rejects.toHaveProperty("kind", "ValidationError"); + }); + + it("throws ValidationError when params.model is missing", async () => { + await expect( + sendOpenAIResponse(mockClient, { input: ["x"] }) + ).rejects.toHaveProperty("kind", "ValidationError"); + }); + + // ===== Error mapping (3) ===== + + it("maps 401 to OpenAIAuthError", async () => { + mockCreate.mockRejectedValueOnce(makeAuthError()); + await expect( + sendOpenAIResponse(mockClient, { input: ["x"], model: "m" }) + ).rejects.toHaveProperty("kind", "OpenAIAuthError"); + }); + + it("maps 429 to OpenAIRateLimitError", async () => { + mockCreate.mockRejectedValueOnce(makeRateLimitError()); + await expect( + sendOpenAIResponse(mockClient, { input: ["x"], model: "m" }) + ).rejects.toHaveProperty("kind", "OpenAIRateLimitError"); + }); + + it("maps generic error to OpenAIRequestError", async () => { + mockCreate.mockRejectedValueOnce(makeNetworkError()); + await expect( + sendOpenAIResponse(mockClient, { input: ["x"], model: "m" }) + ).rejects.toHaveProperty("kind", "OpenAIRequestError"); + }); + + // ===== Security (1) ===== + + it("does not leak API key in error message", async () => { + const authErr = makeAuthError(); + authErr.message = "Invalid OPENAI_API_KEY: sk-my-secret-key-12345."; + mockCreate.mockRejectedValueOnce(authErr); + + try { + await sendOpenAIResponse(mockClient, { input: ["x"], model: "m" }); + expect.fail("Should have thrown"); + } catch (err) { + expect(err.message).not.toContain("sk-my-secret-key-12345"); + expect(err.kind).toBe("OpenAIAuthError"); + } + }); + + // ===== Empty content (1) ===== + + it("returns empty content when output_text is empty", async () => { + const client = createMockClient({ output_text: "" }); + const result = await sendOpenAIResponse(client, { + input: ["x"], + model: "gpt-5.1", + }); + expect(result.content).toBe(""); + }); + + // ===== Validation: invalid maxOutputTokens (1) ===== + + it("throws ValidationError when maxOutputTokens is negative", async () => { + await expect( + sendOpenAIResponse(mockClient, { input: ["x"], model: "m", maxOutputTokens: -5 }) + ).rejects.toHaveProperty("kind", "ValidationError"); + }); +});