feat: add architecture_review tool handler
This commit is contained in:
+14
-10
@@ -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
|
||||
|
||||
|
||||
+23
-39
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<any>
|
||||
* }} 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 };
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user