From 867bc4860cab8530f5e9e06400abe8110235ed9f Mon Sep 17 00:00:00 2001 From: robbond Date: Fri, 12 Jun 2026 10:06:30 +0100 Subject: [PATCH] feat: add architecture_review tool handler --- AGENT_HANDOFF.md | 24 +- PROJECT_STATE.md | 62 ++-- TASKS.md | 33 +- src/tools/architecture-review.js | 79 ++++- test/tools/architecture-review.test.js | 453 +++++++++++++++++++++++++ 5 files changed, 599 insertions(+), 52 deletions(-) create mode 100644 test/tools/architecture-review.test.js diff --git a/AGENT_HANDOFF.md b/AGENT_HANDOFF.md index 99b7c21..69178bd 100644 --- a/AGENT_HANDOFF.md +++ b/AGENT_HANDOFF.md @@ -39,20 +39,24 @@ Build the MCP tool handlers that: ## Completed (Phase 4) -- Task 4.1 — ask_chatgpt handler (`src/tools/ask-chatgpt.js`, `test/tools/ask-chatgpt.test.js`) ✅ -- Task 4.2 — review_plan handler (`src/tools/review-plan.js`, `test/tools/review-plan.test.js`) ✅ -- Task 4.3 — review_code handler (`src/tools/review-code.js`, `test/tools/review-code.test.js`) ✅ -- Task 4.4 — debug_issue tool handler (`src/tools/debug-issue.js`, `test/tools/debug-issue.test.js`) ✅ +All five MCP tool handlers are complete: + +| Handler | File | Tests | +|---------|------|-------| +| handleAskChatGpt | `src/tools/ask-chatgpt.js` | ✅ 27 | +| handleReviewPlan | `src/tools/review-plan.js` | ✅ 28 | +| handleReviewCode | `src/tools/review-code.js` | ✅ 28 | +| handleDebugIssue | `src/tools/debug-issue.js` | ✅ 28 | +| handleArchitectureReview | `src/tools/architecture-review.js` | ✅ 28 | + +Total: 139 orchestration-only tests, all passing. ## Next Pending -### Task 4.4 - debug_issue tool handler ✅ +### Phase 5 - MCP Server and Tool Registration -Built `src/tools/debug-issue.js` with dependency-injected `handleDebugIssue(input, deps)` and 28 orchestration-only tests in `test/tools/debug-issue.test.js`. Followed the same pattern as 4.1, 4.2, and 4.3. All 28 tests pass. - -### Next Pending - -- Task 4.5 — architecture_review tool handler (`src/tools/architecture-review.js`, `test/tools/architecture-review.test.js`) +- Register the five tools with the MCP server. +- Implement the stdio server. ## General Rules diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index e08ea74..21d045e 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -7,11 +7,11 @@ ChatGPT MCP Server ## Status Planning complete. -Phase 0 complete. Phase 1 complete. Phase 2 complete. Phase 3 complete. Task 4.1 complete. Task 4.2 complete. Task 4.3 complete. Task 4.4 complete. +Phase 0 complete. Phase 1 complete. Phase 2 complete. Phase 3 complete. Task 4.1 complete. Task 4.2 complete. Task 4.3 complete. Task 4.4 complete. Task 4.5 complete. Phase 4 complete. ## Current Phase -Phase 4 - Tool Handlers +Phase 5 - MCP Server and Tool Registration ## Completed Tasks @@ -35,6 +35,7 @@ Phase 4 - Tool Handlers - Task 4.2 — review_plan MCP tool handler (`src/tools/review-plan.js`, `test/tools/review-plan.test.js`) ✅ - Task 4.3 — review_code MCP tool handler (`src/tools/review-code.js`, `test/tools/review-code.test.js`) ✅ - Task 4.4 — debug_issue tool handler (`src/tools/debug-issue.js`, `test/tools/debug-issue.test.js`) ✅ +- Task 4.5 — architecture_review tool handler (`src/tools/architecture-review.js`, `test/tools/architecture-review.test.js`) ✅ ## Phase 3 Completion Summary @@ -58,46 +59,29 @@ Phase 3 — Tool Inputs and Prompts — is now complete. - No MCP tool registration. - No tool handlers. -## Completed Tasks (Phase 4) +## Phase 4 Completion Summary — Tool Handlers ✅ -### Task 4.1 — ask_chatgpt MCP Tool Handler ✅ +All five tool handlers are implemented and tested: -Implemented `src/tools/ask-chatgpt.js` with dependency-injected handler and 27 orchestration-only tests in `test/tools/ask-chatgpt.test.js`. +| # | Handler | File | Tests | +|---|---------|------|-------| +| 1 | `handleAskChatGpt` | `src/tools/ask-chatgpt.js` | ✅ 27 | +| 2 | `handleReviewPlan` | `src/tools/review-plan.js` | ✅ 28 | +| 3 | `handleReviewCode` | `src/tools/review-code.js` | ✅ 28 | +| 4 | `handleDebugIssue` | `src/tools/debug-issue.js` | ✅ 28 | +| 5 | `handleArchitectureReview` | `src/tools/architecture-review.js` | ✅ 28 | -**Key design decisions:** -- Execution order: validate → loadConfig → checkContextBudget → buildAskChatGptPrompt → createOpenAIClient → sendOpenAIResponse -- All external deps injected (loadConfig, createOpenAIClient, sendOpenAIResponse); internal utilities imported directly -- OpenAI errors pass through `String(err)` unchanged — no wrapping or reformatting -- Budget check short-circuits before prompt building or client creation -- Every path returns structured `{ ok, answer|error, warnings }` — never throws to caller -- 27 tests covering: success path, validation failure, config failure, budget failure, client creation failure, OpenAI failure, dependency call order, error handling for null/string throws, warning propagation, result shape, and short-circuit behavior +**What Phase 4 established:** -### Task 4.2 — review_plan MCP Tool Handler ✅ +- Five standalone, dependency-injected handlers following the same orchestration pattern: validate → config → budget → prompt → client → response. +- All external deps injected (loadConfig, createOpenAIClient, sendOpenAIResponse); internal utilities imported directly. +- Every handler returns structured `{ ok, answer|error, warnings }` — never throws to caller. +- Budget check short-circuits before prompt building or client creation. +- OpenAI errors pass through `String(err)` unchanged — no wrapping or reformatting. +- 139 orchestration-only tests covering success, validation failure, config failure, budget failure, client failure, OpenAI failure, call order, prompt integration, throws escaping, warnings, result shape, and short-circuit behavior. -Implemented `src/tools/review-plan.js` with dependency-injected handler and 28 orchestration-only tests in `test/tools/review-plan.test.js`. +**What Phase 4 did NOT do:** -**Key design decisions:** -- Execution order: validate → loadConfig → checkContextBudget → buildReviewPlanPrompt → createOpenAIClient → sendOpenAIResponse -- All external deps injected (loadConfig, createOpenAIClient, sendOpenAIResponse); internal utilities imported directly -- OpenAI errors pass through `String(err)` unchanged — no wrapping or reformatting -- Budget check short-circuits before prompt building or client creation -- Every path returns structured `{ ok, answer|error, warnings }` — never throws to caller -- 28 tests mirroring 4.1 structure with one additional test verifying buildReviewPlanPrompt is called with budget.input - -**Task 4.3 — review_code MCP Tool Handler ✅** - -Implemented `src/tools/review-code.js` with dependency-injected handler and 28 orchestration-only tests in `test/tools/review-code.test.js`. - -**Key design decisions:** -- Execution order: validate → loadConfig → checkContextBudget → buildReviewCodePrompt → createOpenAIClient → sendOpenAIResponse -- All external deps injected (loadConfig, createOpenAIClient, sendOpenAIResponse); internal utilities imported directly -- OpenAI errors pass through `String(err)` unchanged — no wrapping or reformatting -- Budget check short-circuits before prompt building or client creation -- Every path returns structured `{ ok, answer|error, warnings }` — never throws to caller -- 28 tests mirroring 4.1/4.2 structure with one test verifying buildReviewCodePrompt is called with budget.input - -**Next pending task:** -- Task 4.5 — architecture_review tool handler (`src/tools/architecture-review.js`, `test/tools/architecture-review.test.js`) - -**Not done yet (Phase 4):** -- No MCP tool registration +- No MCP tool registration yet. +- No server or router code yet. +- That belongs to Phase 5. diff --git a/TASKS.md b/TASKS.md index 45da4d7..0c1ab57 100644 --- a/TASKS.md +++ b/TASKS.md @@ -372,8 +372,37 @@ Create `src/tools/architecture-review.js` and `test/tools/architecture-review.te - OpenAI errors pass through `String(err)` unchanged (no wrapping/reformatting). - Mirror 4.1/4.2/4.3/4.4 structure: ~28 orchestration-only tests covering the same test categories. -Status: ⬜ Pending +Status: ✅ Complete + +### Task 4.6 — Phase 4 completion summary + +**Phase 4 — Tool Handlers** is now complete. + +All five tool handlers are implemented and tested: + +| # | Handler | File | Tests | +|---|---------|------|-------| +| 1 | `handleAskChatGpt` | `src/tools/ask-chatgpt.js` | ✅ 27 | +| 2 | `handleReviewPlan` | `src/tools/review-plan.js` | ✅ 28 | +| 3 | `handleReviewCode` | `src/tools/review-code.js` | ✅ 28 | +| 4 | `handleDebugIssue` | `src/tools/debug-issue.js` | ✅ 28 | +| 5 | `handleArchitectureReview` | `src/tools/architecture-review.js` | ✅ 28 | + +**What Phase 4 established:** + +- Five standalone, dependency-injected handlers following the same orchestration pattern: validate → config → budget → prompt → client → response. +- All external deps injected (loadConfig, createOpenAIClient, sendOpenAIResponse); internal utilities imported directly. +- Every handler returns structured `{ ok, answer|error, warnings }` — never throws to caller. +- Budget check short-circuits before prompt building or client creation. +- OpenAI errors pass through `String(err)` unchanged — no wrapping or reformatting. +- 139 orchestration-only tests covering success, validation failure, config failure, budget failure, client failure, OpenAI failure, call order, prompt integration, throws escaping, warnings, result shape, and short-circuit behavior. + +**What Phase 4 did NOT do:** + +- No MCP tool registration yet. +- No server or router code yet. +- That belongs to Phase 5. --- -Phase 2 complete. Phase 3 complete. Phase 4 next: Tool Handlers. +Phase 2 complete. Phase 3 complete. Phase 4 complete. Phase 5 next: MCP Server and Tool Registration. diff --git a/src/tools/architecture-review.js b/src/tools/architecture-review.js index 3c7d3bd..07961ff 100644 --- a/src/tools/architecture-review.js +++ b/src/tools/architecture-review.js @@ -1 +1,78 @@ -// architecture_review tool handler. +// Tool handler for the architecture_review MCP tool. + +import { validateToolInput } from "./schemas.js"; +import { checkContextBudget } from "../utils/context-budget.js"; +import { buildArchitectureReviewPrompt } from "../prompts/architecture-review.js"; + +/** + * Handle the architecture_review MCP tool. + * + * Orchestration order: validate -> config -> budget -> prompt -> client -> response. + * All external dependencies injected via deps. Internal utilities imported directly. + * No throws escape — all paths return structured results. + * + * @param {unknown} input + * Raw tool input per ARCHITECTURE.md §7 schema. + * @param {{ + * loadConfig: () => object, + * createOpenAIClient: (config: object) => any, + * sendOpenAIResponse: (client: any, params: object) => Promise + * }} deps + * Injected external dependencies. + * @returns {Promise<{ ok: true, answer: string, warnings: string[] } | { ok: false, error: string, warnings: string[] }>} + */ +export async function handleArchitectureReview(input, deps) { + // --- 1. Validate input (before anything else) --- + + const validation = validateToolInput(input); + if (!validation.ok) { + return { ok: false, error: validation.errors.join(" | "), warnings: [] }; + } + + // --- 2. Load config --- + + let config; + try { + config = deps.loadConfig(); + } catch (err) { + return { ok: false, error: String(err), warnings: [] }; + } + + // --- 3. Context budget check --- + + const budget = checkContextBudget(validation.data, config); + if (!budget.ok) { + return { ok: false, error: budget.error, warnings: budget.warnings }; + } + + // --- 4. Build prompt --- + + const promptMessages = buildArchitectureReviewPrompt(budget.input); + + // --- 5. Create OpenAI client --- + + let client; + try { + client = deps.createOpenAIClient(config); + } catch (err) { + return { ok: false, error: String(err), warnings: [] }; + } + + // --- 6. Send to OpenAI --- + + let aiResult; + try { + aiResult = await deps.sendOpenAIResponse(client, { + input: [{ role: "system", content: promptMessages }], + model: config.openaiModel, + temperature: config.temperature, + maxOutputTokens: config.maxOutputTokens, + }); + } catch (err) { + return { ok: false, error: String(err), warnings: [] }; + } + + // --- 7. Success --- + + return { ok: true, answer: aiResult.content, warnings: budget.warnings }; +} diff --git a/test/tools/architecture-review.test.js b/test/tools/architecture-review.test.js new file mode 100644 index 0000000..72ec45d --- /dev/null +++ b/test/tools/architecture-review.test.js @@ -0,0 +1,453 @@ +import { describe, it, expect, vi } from "vitest"; + +// Module-level mock capture for buildArchitectureReviewPrompt integration testing. +const buildArchitectureReviewPromptCalls = []; + +vi.mock("../../src/prompts/architecture-review.js", async () => ({ + buildArchitectureReviewPrompt: vi.fn((input) => { + buildArchitectureReviewPromptCalls.push(input); + return "mocked architecture review prompt"; + }), +})); + +// Import after the mock is registered (hoisted by vitest). +const { handleArchitectureReview } = await import("../../src/tools/architecture-review.js"); + +const mockConfig = { + openaiApiKey: "sk-test-key", + openaiModel: "gpt-5.1", + temperature: 0.2, + maxOutputTokens: 2000, + logLevel: "info", + enableFileContext: false, + contextDir: "./context", + maxInputChars: 30000, + maxFileChars: 12000, + maxFiles: 5, + maxLogChars: 10000, + redactSecrets: true, +}; + +function makeValidInput(question) { + return { question }; +} + +// --- Success path --- + +describe("success path", () => { + it("returns ok:true with answer on full happy flow", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn(async () => ({ content: "OK" })); + + const result = await handleArchitectureReview(makeValidInput("What architecture should we use?"), { + loadConfig, createOpenAIClient, sendOpenAIResponse, + }); + + expect(result.ok).toBe(true); + expect(result.answer).toBe("OK"); + expect(result.warnings).toEqual([]); + }); + + it("propagates budget warnings through to success result", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const trimmedBudget = { ...mockConfig, maxInputChars: 50 }; + const loadConfig = vi.fn(() => trimmedBudget); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn(async () => ({ content: "OK" })); + + const result = await handleArchitectureReview(makeValidInput("hi"), { + loadConfig, createOpenAIClient, sendOpenAIResponse, + }); + + expect(result.ok).toBe(true); + expect(Array.isArray(result.warnings)).toBe(true); + }); +}); + +// --- Validation failure (short-circuit before config) --- + +describe("validation failure", () => { + it("returns structured error when question is missing", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview({}, { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + expect(result.warnings).toEqual([]); + }); + + it("short-circuits — no other deps called", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + await handleArchitectureReview({ foo: "bar" }, { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(loadConfig).not.toHaveBeenCalled(); + expect(createOpenAIClient).not.toHaveBeenCalled(); + expect(sendOpenAIResponse).not.toHaveBeenCalled(); + }); + + it("returns structured error when question is empty string", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview({ question: "" }, { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + expect(result.warnings).toEqual([]); + }); + + it("returns structured error when question is wrong type", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview({ question: 123 }, { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + expect(result.warnings).toEqual([]); + }); +}); + +// --- Config failure --- + +describe("config failure", () => { + it("returns structured error when loadConfig throws", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => { throw new Error("OPENAI_API_KEY is missing."); }); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(result.error).toContain("OPENAI_API_KEY is missing"); + }); + + it("short-circuits — no client or response calls after config failure", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => { throw new Error("No key."); }); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(createOpenAIClient).not.toHaveBeenCalled(); + expect(sendOpenAIResponse).not.toHaveBeenCalled(); + }); + + it("passes original error message", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => { throw new Error("OPENAI_API_KEY is missing."); }); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.error).toBe("Error: OPENAI_API_KEY is missing."); + }); +}); + +// --- Budget failure (short-circuit before prompt/client) --- + +describe("budget failure", () => { + it("returns structured error with budget warnings when input exceeds budget", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const tinyBudget = { ...mockConfig, maxInputChars: 0 }; + const loadConfig = vi.fn(() => tinyBudget); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + expect(Array.isArray(result.warnings)).toBe(true); + }); + + it("short-circuits — no prompt built, no client created, no response sent", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const tinyBudget = { ...mockConfig, maxInputChars: 0 }; + const loadConfig = vi.fn(() => tinyBudget); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(createOpenAIClient).not.toHaveBeenCalled(); + expect(sendOpenAIResponse).not.toHaveBeenCalled(); + }); + + it("passes budget warnings through to the error result", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const tinyBudget = { ...mockConfig, maxInputChars: 0 }; + const loadConfig = vi.fn(() => tinyBudget); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview( + { question: "x", context: "a".repeat(20) }, + { loadConfig, createOpenAIClient, sendOpenAIResponse }, + ); + + expect(result.ok).toBe(false); + expect(Array.isArray(result.warnings)).toBe(true); + }); +}); + +// --- Client creation failure --- + +describe("client creation failure", () => { + it("returns structured error when createOpenAIClient throws", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => { throw new Error("Invalid config."); }); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + expect(result.warnings).toEqual([]); + }); + + it("short-circuits — no response sent after client failure", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => { throw new Error("Invalid config."); }); + const sendOpenAIResponse = vi.fn(); + + await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(sendOpenAIResponse).not.toHaveBeenCalled(); + }); + + it("passes original error message unchanged", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => { throw new Error("Invalid config."); }); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.error).toBe("Error: Invalid config."); + }); +}); + +// --- OpenAI failure (pass-through) --- + +describe("OpenAI failure", () => { + it("passes through err.message unchanged", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn().mockRejectedValue(new Error("API key invalid.")); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(result.error).toBe("Error: API key invalid."); + expect(result.warnings).toEqual([]); + }); + + it("short-circuits — no extra processing after API failure", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn().mockRejectedValue(new Error("rate limit")); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(result.error).toBe("Error: rate limit"); + }); + + it("does not wrap or reformat the error", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn().mockRejectedValue(new Error("429 Too Many Requests")); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.error).toBe("Error: 429 Too Many Requests"); + }); +}); + +// --- Dependency call order --- + +describe("dependency call order", () => { + it("calls deps in correct order: config -> client -> response", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const callLog = []; + const loadConfig = vi.fn(() => { callLog.push("config"); return mockConfig; }); + const createOpenAIClient = vi.fn(() => { callLog.push("client"); return { responses: { create: vi.fn() } }; }); + const sendOpenAIResponse = vi.fn(async () => { callLog.push("response"); return { content: "OK" }; }); + + await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(callLog).toEqual(["config", "client", "response"]); + }); +}); + +// --- buildArchitectureReviewPrompt integration --- + +describe("buildArchitectureReviewPrompt integration", () => { + it("calls buildArchitectureReviewPrompt and receives budget.input as argument", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn(async () => ({ content: "OK" })); + + await handleArchitectureReview(makeValidInput("test question"), { + loadConfig, createOpenAIClient, sendOpenAIResponse, + }); + + // Verify buildArchitectureReviewPrompt was called (once) with the trimmed input from checkContextBudget. + expect(buildArchitectureReviewPromptCalls.length).toBe(1); + const captured = buildArchitectureReviewPromptCalls[0]; + expect(typeof captured).toBe("object"); + expect(captured.question).toBe("test question"); + }); +}); + +// --- No throws escaping --- + +describe("no throws escaping", () => { + it("returns structured result when sendOpenAIResponse throws null", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn().mockRejectedValue(null); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + }); + + it("returns structured result when loadConfig throws non-Error", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => { throw "string error"; }); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + }); +}); + +// --- Warning propagation --- + +describe("warning propagation", () => { + it("includes budget warnings in success result when budget passes with warnings", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const trimmedBudget = { ...mockConfig, maxInputChars: 50 }; + const loadConfig = vi.fn(() => trimmedBudget); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn(async () => ({ content: "OK" })); + + const result = await handleArchitectureReview( + { question: "hi", context: "x".repeat(49) }, + { loadConfig, createOpenAIClient, sendOpenAIResponse }, + ); + + expect(result.ok).toBe(true); + expect(Array.isArray(result.warnings)).toBe(true); + }); + + it("includes budget warnings in failure result when budget fails", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const emptyBudget = { ...mockConfig, maxInputChars: 0 }; + const loadConfig = vi.fn(() => emptyBudget); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(result.ok).toBe(false); + expect(Array.isArray(result.warnings)).toBe(true); + }); +}); + +// --- Result shape --- + +describe("result shape", () => { + it("returns exactly { ok, answer, warnings } on success", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn(async () => ({ content: "OK" })); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(Object.keys(result).sort()).toEqual(["answer", "ok", "warnings"]); + expect(result.ok).toBe(true); + expect(typeof result.answer).toBe("string"); + expect(Array.isArray(result.warnings)).toBe(true); + }); + + it("returns exactly { ok, error, warnings } on failure", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => { throw new Error("fail"); }); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + const result = await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(Object.keys(result).sort()).toEqual(["error", "ok", "warnings"]); + expect(result.ok).toBe(false); + expect(typeof result.error).toBe("string"); + expect(Array.isArray(result.warnings)).toBe(true); + }); +}); + +// --- Short-circuit behavior --- + +describe("short-circuit behavior", () => { + it("stops at first failure without calling downstream deps", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => mockConfig); + const createOpenAIClient = vi.fn(() => ({ responses: { create: vi.fn() } })); + const sendOpenAIResponse = vi.fn().mockRejectedValue(new Error("boom")); + + await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(loadConfig).toHaveBeenCalledTimes(1); + expect(createOpenAIClient).toHaveBeenCalledTimes(1); + expect(sendOpenAIResponse).toHaveBeenCalledTimes(1); + }); + + it("stops at config failure without calling downstream deps", async () => { + buildArchitectureReviewPromptCalls.length = 0; + + const loadConfig = vi.fn(() => { throw new Error("fail"); }); + const createOpenAIClient = vi.fn(); + const sendOpenAIResponse = vi.fn(); + + await handleArchitectureReview(makeValidInput("hi"), { loadConfig, createOpenAIClient, sendOpenAIResponse }); + expect(loadConfig).toHaveBeenCalledTimes(1); + expect(createOpenAIClient).not.toHaveBeenCalled(); + expect(sendOpenAIResponse).not.toHaveBeenCalled(); + }); +});