diff --git a/AGENT_HANDOFF.md b/AGENT_HANDOFF.md index 3e94fa1..e890390 100644 --- a/AGENT_HANDOFF.md +++ b/AGENT_HANDOFF.md @@ -1,9 +1,45 @@ # AGENT_HANDOFF.md -Read ARCHITECTURE.md before making changes. +## Completed -Rules: +### Phase 0 — Repository Setup +- Repository skeleton (TASK 0.1) +- package.json with dependencies (TASK 0.2) +### Phase 1 — Core Utilities +- Configuration loader (`src/config/env.js`) — TASK 1.1 +- Secret redaction utility (`src/utils/redact.js`) — TASK 1.2 +- Context budget utility (`src/utils/context-budget.js`) — TASK 1.3 +- Safe logging helper (`src/utils/logging.js`) — TASK 1.4 + +### Phase 2 — OpenAI Integration +- OpenAI client wrapper (`src/openai/client.js`) — TASK 2.1 +- Response builder (`src/openai/responses.js`) — TASK 2.2 +- Error handling and edge cases (tests) — TASK 2.3 + +### Phase 3 — Tool Inputs and Prompts +- Zod input validation schemas (`src/tools/schemas.js`, tests) — TASK 3.1 +- Base prompt template (`src/prompts/base.js`, tests) — TASK 3.2 +- ask_chatgpt prompt builder (`src/prompts/ask-chatgpt.js`, tests) — TASK 3.3 +- review_plan prompt builder (`src/prompts/review-plan.js`, tests) — TASK 3.4 +- review_code prompt builder (`src/prompts/review-code.js`, tests) — TASK 3.5 +- debug_issue prompt builder (`src/prompts/debug-issue.js`, tests) — TASK 3.6 +- architecture_review prompt builder (`src/prompts/architecture-review.js`, tests) — TASK 3.7 + +## Next Phase + +### Phase 4 - Tool Handlers + +Build the MCP tool handlers that: +1. Register each tool with the MCP server. +2. Validate input using `schemas.js`. +3. Call the appropriate prompt builder. +4. Send the prompt to OpenAI via `responses.js`. +5. Return structured advisory output to Claude Code. + +## General Rules + +- Read ARCHITECTURE.md before making changes. - Work incrementally. - Keep changes small. - Do not implement multiple phases at once. diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 9e72b34..16f2142 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -7,13 +7,13 @@ ChatGPT MCP Server ## Status Planning complete. -Phase 0 complete. Phase 1 complete. Phase 2 complete. Phase 3 in progress. +Phase 0 complete. Phase 1 complete. Phase 2 complete. Phase 3 complete. ## Current Phase -Phase 3 in progress. +Phase 4 - Tool Handlers -## Completed Tasks +## Completed Tasks (Phase 3 Complete) - Task 0.1 — Create repository skeleton ✅ - Task 0.2 — package.json with dependencies ✅ @@ -30,7 +30,26 @@ Phase 3 in progress. - Task 3.4 — review_plan prompt builder (`src/prompts/review-plan.js`, `test/prompts/review-plan.test.js`) ✅ - Task 3.5 — review_code prompt builder (`src/prompts/review-code.js`, `test/prompts/review-code.test.js`) ✅ - Task 3.6 — debug_issue prompt builder (`src/prompts/debug-issue.js`, `test/prompts/debug-issue.test.js`) ✅ +- Task 3.7 — architecture_review prompt builder (`src/prompts/architecture-review.js`, `test/prompts/architecture-review.test.js`) ✅ -## Next Phase +## Phase 3 Completion Summary -Phase 3 — in progress; next is Task 3.6 (debug_issue prompt builder). +Phase 3 — Tool Inputs and Prompts — is now complete. + +**Completed prompt builders:** +- `buildBasePrompt` — base system prompt (ARCHITECTURE.md §11) +- `buildAskChatGptPrompt` — general second-opinion advisor +- `buildReviewPlanPrompt` — plan review before implementation +- `buildReviewCodePrompt` — focused code/diff review +- `buildDebugIssuePrompt` — error/log/stack trace analysis +- `buildArchitectureReviewPrompt` — architecture decision trade-off review + +**What Phase 3 established:** +- Prompt layer complete with composition pattern established. +- All prompt builders tested (56 tests for architecture-review alone; 384 total). +- Each builder follows the same pattern: `buildBasePrompt()` → `\n\n---\n\n` → tool-specific section. +- Guard rails reinforced in every builder (Claude Code is executor; ChatGPT is advisory only). + +**Not done yet (belongs to Phase 4):** +- No MCP tool registration. +- No tool handlers. diff --git a/TASKS.md b/TASKS.md index 01e57cf..5df2e8a 100644 --- a/TASKS.md +++ b/TASKS.md @@ -267,8 +267,38 @@ Create `test/prompts/architecture-review.test.js`. Requirements: - Mirror structure of ask_chatgpt, review-plan, review-code, and debug-issue tests: base composition, architecture_review-specific instructions, optional fields, expectedOutput, input validation, tool guard, full integration. -Status: ⬜ Pending +Status: ✅ Complete + +### Task 3.8 - Phase 3 completion summary (NEXT) + +**Phase 3 — Tool Inputs and Prompts** is now complete. + +All six prompt builders are implemented and tested: + +| # | Prompt Builder | File | Tests | +|---|---|---|---| +| 0 | `buildBasePrompt` | `src/prompts/base.js` | ✅ | +| 1 | `buildAskChatGptPrompt` | `src/prompts/ask-chatgpt.js` | ✅ | +| 2 | `buildReviewPlanPrompt` | `src/prompts/review-plan.js` | ✅ | +| 3 | `buildReviewCodePrompt` | `src/prompts/review-code.js` | ✅ | +| 4 | `buildDebugIssuePrompt` | `src/prompts/debug-issue.js` | ✅ | +| 5 | `buildArchitectureReviewPrompt` | `src/prompts/architecture-review.js` | ✅ | + +**What Phase 3 established:** + +- Prompt composition pattern: `buildBasePrompt()` → explicit `\n\n---\n\n` separator → tool-specific section. +- All builders share the same input shape, validation rules, and optional field handling. +- Guard rails in every builder: Claude Code remains executor; ChatGPT is advisory only. +- `relevantFiles` inclusion with path, language (when present), and content — omitted when absent/empty. +- Each builder passes full integration tests covering all dimensions, response structure, optional fields, and negative guards. + +**What Phase 3 did NOT do:** + +- No MCP tool registration yet. +- No tool handlers yet. +- No OpenAI API calls from the builders. +- No file loading, logging, or context budget within the builders. --- -Phase 2 complete. Phase 3 in progress. +Phase 2 complete. Phase 3 complete. Phase 4 next: Tool Handlers. diff --git a/src/prompts/architecture-review.js b/src/prompts/architecture-review.js index c77e13e..b17c2bb 100644 --- a/src/prompts/architecture-review.js +++ b/src/prompts/architecture-review.js @@ -1 +1,112 @@ -// architecture_review prompt template. +// Tool-specific prompt builder for architecture_review. + +import { buildBasePrompt } from "./base.js"; + +/** + * Build a tool-specific prompt for the architecture_review MCP tool. + * + * @param {{ + * question: string, + * context?: string, + * constraints?: string[], + * expectedOutput?: string, + * projectSummary?: string, + * taskSummary?: string, + * relevantFiles?: Array<{path: string, content: string, language?: string}> + * }} input + * @returns {string} A two-part prompt: base system message + architecture_review user message. + */ +export function buildArchitectureReviewPrompt(input) { + if (!input || typeof input.question !== "string" || input.question.length === 0) { + const err = new TypeError("architecture_review requires a non-empty 'question' field."); + err.kind = "ValidationError"; + throw err; + } + + const base = buildBasePrompt(); + + const lines = [ + "", + "---", + "", + "You are acting as an architecture advisor for Claude Code. Claude Code will implement your suggestions — you do not implement anything yourself.", + "", + "Your task: review the proposed architecture decision and evaluate its trade-offs before Claude Code acts on it.", + "", + "Review the architecture systematically across these dimensions:", + "- Simplicity — is there a simpler approach that meets the requirements?", + "- Maintainability — who maintains this after implementation? What is the long-term maintenance burden?", + "- Security — are there data exposure, auth, or access control concerns?", + "- Scalability — does the design support expected growth in users, data volume, and requests?", + "- Operational burden — what monitoring, backup, and recovery responsibilities are introduced?", + "- Vendor lock-in — how hard would it be to switch providers or go self-hosted later?", + "- Local-first / self-hosted fit — can this work well without cloud dependency?", + "- Integration risk — how does this integrate with the existing stack? What new dependencies are introduced?", + "- Migration risk — what is the data migration path? Can it be done incrementally?", + "- Appropriateness for current project stage — is this decision premature, or does it match the team's size and maturity?", + "", + "Important rules:", + "- Claude Code remains the implementation agent. You are acting only as an architecture advisor.", + "- ChatGPT should not assume repository-wide context — use only the supplied context and files.", + "- Prefer simple, incremental, reviewable architecture.", + "- Avoid large rewrites unless clearly justified by the evidence provided.", + "", + "Respond in this exact structure:", + "- Summary: Brief overall assessment of the architectural approach.", + "- Recommendation: Clear recommendation with reasoning.", + "- Trade-offs: Key advantages and disadvantages.", + "- Risks: Potential risks ordered by severity.", + "- Future Extension Paths: How well does each option support future growth and change?", + "- Simpler Alternatives: Any simpler approaches that could meet the same goals.", + "- Implementation Notes: Practical guidance for Claude Code if this approach is accepted.", + "- Decision Confidence: Low / Medium / High — with explanation of what would increase confidence.", + "", + ]; + + lines.push(`Question: ${input.question}`); + + if (input.context) { + lines.push(""); + lines.push(`Context: ${input.context}`); + } + + if (input.constraints && input.constraints.length > 0) { + lines.push(""); + lines.push("Constraints:"); + for (const c of input.constraints) { + lines.push(`- ${c}`); + } + } + + if (input.expectedOutput) { + lines.push(""); + lines.push(`Expected output: ${input.expectedOutput}`); + } + + if (input.projectSummary) { + lines.push(""); + lines.push(`Project summary: ${input.projectSummary}`); + } + + if (input.taskSummary) { + lines.push(""); + lines.push(`Task summary: ${input.taskSummary}`); + } + + if (input.relevantFiles && input.relevantFiles.length > 0) { + lines.push(""); + lines.push("Relevant files:"); + + for (const file of input.relevantFiles) { + lines.push(""); + lines.push(`File: ${file.path}`); + if (file.language) { + lines.push(`Language: ${file.language}`); + } + lines.push(file.content); + } + } + + const toolPrompt = lines.join("\n"); + return `${base}\n\n---\n\n${toolPrompt}`; +} diff --git a/test/prompts/architecture-review.test.js b/test/prompts/architecture-review.test.js new file mode 100644 index 0000000..7ede1ed --- /dev/null +++ b/test/prompts/architecture-review.test.js @@ -0,0 +1,465 @@ +import { describe, it, expect } from "vitest"; +import { buildArchitectureReviewPrompt } from "../../src/prompts/architecture-review.js"; + +describe("buildArchitectureReviewPrompt", () => { + // --- Basic output contract --- + + it("returns a non-empty string given question-only input", () => { + const result = buildArchitectureReviewPrompt({ question: "Should we use a managed vector database?" }); + expect(typeof result).toBe("string"); + expect(result.length).toBeGreaterThan(0); + }); + + it("is deterministic — same input always produces the same output", () => { + const a = buildArchitectureReviewPrompt({ question: "Should we use a managed vector database?" }); + const b = buildArchitectureReviewPrompt({ question: "Should we use a managed vector database?" }); + expect(a).toBe(b); + }); + + // --- Explicit separator --- + + it("uses an explicit \\n---\\n separator between base and tool sections", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toContain("\n\n---\n\n"); + }); + + it("contains exactly one separator (two-part prompt)", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + const parts = result.split("\n\n---\n\n"); + expect(parts.length).toBe(2); + }); + + // --- Composition with base prompt --- + + it("includes the base system prompt content", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toContain("second-opinion assistant"); + expect(result).toContain("Claude Code"); + expect(result).toContain("not responsible for editing files"); + }); + + // --- Architecture advisor role instructions --- + + it("contains architecture advisor role instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toContain("architecture advisor"); + }); + + it("instructs Claude Code as the implementation agent", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toContain("Claude Code will implement your suggestions"); + }); + + it("instructs ChatGPT that you do not implement anything yourself", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toContain("you do not implement anything yourself"); + }); + + it("states ChatGPT is acting only as an architecture advisor", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/acting\s+only\s+as\s+an?\s+architecture\s+advisor/i); + }); + + // --- Guard rails --- + + it("states Claude Code remains the implementation agent", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toContain("Claude Code remains the implementation agent"); + }); + + it("instructs ChatGPT to use only supplied context and files", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/use\s+only\s+the?\s+supplied\s+context/i); + }); + + it("instructs ChatGPT not to assume repository-wide context", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/not\s+assume\s+repository-wide\s+context/i); + }); + + it("instructs to prefer simple, incremental architecture", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/prefer\s+simple.*incremental.*architecture/i); + }); + + it("instructs to avoid large rewrites unless justified", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/avoid\s+large\s+rewrites.*unless.*justified/i); + }); + + // --- Review dimensions (10) --- + + it("contains Simplicity dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("simplicity"); + }); + + it("contains Maintainability dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("maintainability"); + }); + + it("contains Security dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/security/i); + }); + + it("contains Scalability dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("scalability"); + }); + + it("contains Operational burden dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("operational burden"); + }); + + it("contains Vendor lock-in dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("vendor lock-in"); + }); + + it("contains Local-first / self-hosted fit dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toMatch(/local-?first.*self-?hosted\s+fit|self-?hosted.*local-?first/i); + }); + + it("contains Integration risk dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("integration risk"); + }); + + it("contains Migration risk dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("migration risk"); + }); + + it("contains Appropriateness for current project stage dimension instruction", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/appropriateness.*project\s+stage|current\s+project\s+stage/i); + }); + + // --- Response structure (8 sections) --- + + it("requests Summary section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/summary/i); + }); + + it("requests Recommendation section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/recommendation/i); + }); + + it("requests Trade-offs section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/trade-?offs?/i); + }); + + it("requests Risks section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/risks/i); + }); + + it("requests Future Extension Paths section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/future\s+extension\s+paths/i); + }); + + it("requests Simpler Alternatives section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).toContain("simpler alternatives"); + }); + + it("requests Implementation Notes section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/implementation\s+notes/i); + }); + + it("requests Decision Confidence section in response structure", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).toMatch(/decision\s+confidence/i); + }); + + // --- Question field --- + + it("appends the input question verbatim in the task section", () => { + const result = buildArchitectureReviewPrompt({ question: "Should we migrate to S3?" }); + expect(result).toContain("Question: Should we migrate to S3?"); + }); + + // --- Optional fields appended correctly (with question) --- + + it("appends Context when context is provided", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we use a managed vector DB?", + context: "Current stack uses PostgreSQL with pgvector for search.", + }); + expect(result).toContain("Context: Current stack uses PostgreSQL with pgvector for search."); + }); + + it("does NOT append Context section when context is empty", () => { + const result = buildArchitectureReviewPrompt({ question: "Test", context: "" }); + expect(result).not.toContain("Context:"); + }); + + it("appends Constraints list when constraints are provided", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we use a managed vector DB?", + constraints: ["Must be self-hostable", "Budget under $100/mo"], + }); + expect(result).toContain("Constraints:"); + expect(result).toContain("- Must be self-hostable"); + expect(result).toContain("- Budget under $100/mo"); + }); + + it("does NOT append Constraints section when constraints array is empty", () => { + const result = buildArchitectureReviewPrompt({ question: "Test", constraints: [] }); + expect(result).not.toContain("Constraints:"); + }); + + it("appends Expected output when expectedOutput is provided", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we use a managed vector DB?", + expectedOutput: "A trade-off analysis comparing options.", + }); + expect(result).toContain( + "Expected output: A trade-off analysis comparing options." + ); + }); + + it("does NOT append Expected output section when expectedOutput is omitted", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).not.toContain("Expected output:"); + }); + + it("does NOT append Expected output section when expectedOutput is empty", () => { + const result = buildArchitectureReviewPrompt({ question: "Test", expectedOutput: "" }); + expect(result).not.toContain("Expected output:"); + }); + + it("appends all optional fields alongside each other", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we migrate to S3?", + context: "Current stack uses local disk with Express.", + constraints: ["Must be deployable without Kubernetes", "Budget under $100/mo"], + expectedOutput: "A comparison of S3, MinIO, and shared NFS.", + projectSummary: "Internal document management system for a 50-person team.", + taskSummary: "Migrating from local filesystem to scalable object storage.", + }); + expect(result).toContain("Question: Should we migrate to S3?"); + expect(result).toContain("Context: Current stack uses local disk with Express."); + expect(result).toContain("Constraints:"); + expect(result).toContain("- Must be deployable without Kubernetes"); + expect(result).toContain("- Budget under $100/mo"); + expect(result).toContain("Expected output: A comparison of S3, MinIO, and shared NFS."); + expect(result).toContain( + "Project summary: Internal document management system for a 50-person team." + ); + expect(result).toContain( + "Task summary: Migrating from local filesystem to scalable object storage." + ); + }); + + it("does NOT append Project summary when projectSummary is omitted", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).not.toContain("Project summary:"); + }); + + it("does NOT append Task summary when taskSummary is omitted", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).not.toContain("Task summary:"); + }); + + // --- relevantFiles (present) --- + + it("includes file section header when relevantFiles is provided", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we migrate to S3?", + relevantFiles: [ + { path: "src/storage/local.js", content: "fs.writeFileSync(path, data);" }, + ], + }); + expect(result).toContain("Relevant files:"); + }); + + it("includes file path, language, and content when relevantFiles has language", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we migrate to S3?", + relevantFiles: [ + { path: "src/storage/local.js", language: "javascript", content: "fs.writeFileSync(path, data);" }, + ], + }); + expect(result).toContain("File: src/storage/local.js"); + expect(result).toContain("Language: javascript"); + expect(result).toContain("fs.writeFileSync(path, data);"); + }); + + it("includes file content without language header when language is omitted", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we migrate to S3?", + relevantFiles: [ + { path: "src/storage/local.js", content: "fs.writeFileSync(path, data);" }, + ], + }); + expect(result).toContain("File: src/storage/local.js"); + expect(result).not.toContain("Language:"); + expect(result).toContain("fs.writeFileSync(path, data);"); + }); + + it("includes multiple files when relevantFiles has multiple entries", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we migrate to S3?", + relevantFiles: [ + { path: "src/storage/local.js", language: "javascript", content: "fs.writeFileSync(path, data);" }, + { path: "src/middleware/upload.js", language: "javascript", content: "app.post('/upload', handler);" }, + ], + }); + expect(result).toContain("File: src/storage/local.js"); + expect(result).toContain("File: src/middleware/upload.js"); + expect(result).toContain("fs.writeFileSync(path, data);"); + expect(result).toContain("app.post('/upload', handler);"); + }); + + // --- relevantFiles (absent/empty) --- + + it("does NOT include Relevant files section when relevantFiles is omitted", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).not.toContain("Relevant files:"); + }); + + it("does NOT include Relevant files section when relevantFiles is empty array", () => { + const result = buildArchitectureReviewPrompt({ question: "Test", relevantFiles: [] }); + expect(result).not.toContain("Relevant files:"); + }); + + // --- Tool-specific guard --- + + it("does NOT contain debug_issue specific content", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).not.toContain("Likely Causes"); + expect(result).not.toContain("Fast Checks"); + }); + + it("does NOT contain review_plan specific content", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result).not.toContain("missing steps"); + expect(result).not.toContain("scope creep"); + }); + + it("does NOT contain review_code specific content", () => { + const result = buildArchitectureReviewPrompt({ question: "Test" }); + expect(result.toLowerCase()).not.toContain("issues by severity"); + expect(result.toLowerCase()).not.toContain("suggested fixes"); + }); + + // --- Input validation --- + + it("throws TypeError when input is missing, null, or undefined", () => { + expect(() => buildArchitectureReviewPrompt()).toThrow(TypeError); + expect(() => buildArchitectureReviewPrompt(null)).toThrow(TypeError); + expect(() => buildArchitectureReviewPrompt(undefined)).toThrow(TypeError); + }); + + it("throws TypeError when question is missing", () => { + expect(() => buildArchitectureReviewPrompt({ context: "something" })).toThrow(TypeError); + }); + + it("throws TypeError when question is empty", () => { + expect(() => buildArchitectureReviewPrompt({ question: "" })).toThrow(TypeError); + }); + + // --- Full integration test --- + + it("returns a well-formed two-part prompt with all fields and relevantFiles", () => { + const result = buildArchitectureReviewPrompt({ + question: "Should we migrate our file storage from local filesystem to an object store?", + context: "Current system stores uploads on the application server's disk. We're getting disk space alerts and need multi-server support.", + constraints: ["Must be deployable without Kubernetes", "Cost must not exceed current $50/mo by more than 50%"], + expectedOutput: "A comparison of S3, MinIO, and shared NFS with trade-off analysis.", + projectSummary: "Internal document management system for a mid-sized company.", + taskSummary: "Migrating from local disk storage to a scalable file storage solution.", + relevantFiles: [ + { + path: "src/storage/local-filesystem.js", + language: "javascript", + content: `const fs = require('fs');\n\nfunction saveFile(name, data) {\n fs.writeFileSync(\`./uploads/\${name}\`, data);\n}`, + }, + { + path: "src/middleware/upload.js", + content: `app.post('/upload', upload.single('file'), (req, res) => { ... });`, + }, + ], + }); + + // Base section present + expect(result).toContain("second-opinion assistant"); + expect(result).toContain("Rules:"); + + // Explicit separator — exactly one + const parts = result.split("\n\n---\n\n"); + expect(parts.length).toBe(2); + + // Tool-specific section present (second part) + const toolSection = parts[1]; + expect(toolSection).toContain("architecture advisor"); + expect(toolSection).toContain("Claude Code remains the implementation agent"); + expect(toolSection).toMatch(/use\s+only\s+the?\s+supplied\s+context/i); + + // Guard rails present + expect(toolSection).toMatch(/prefer\s+simple.*incremental.*architecture/i); + expect(toolSection).toMatch(/avoid\s+large\s+rewrites.*unless.*justified/i); + + // All 10 review dimensions + expect(toolSection.toLowerCase()).toContain("simplicity"); + expect(toolSection.toLowerCase()).toContain("maintainability"); + expect(toolSection).toMatch(/security/i); + expect(toolSection.toLowerCase()).toContain("scalability"); + expect(toolSection.toLowerCase()).toContain("operational burden"); + expect(toolSection.toLowerCase()).toContain("vendor lock-in"); + expect(toolSection.toLowerCase()).toMatch(/local-?first.*self-?hosted\s+fit|self-?hosted.*local-?first/i); + expect(toolSection.toLowerCase()).toContain("integration risk"); + expect(toolSection.toLowerCase()).toContain("migration risk"); + expect(toolSection).toMatch(/appropriateness.*project\s+stage|current\s+project\s+stage/i); + + // All 8 response structure sections + expect(toolSection).toMatch(/summary/i); + expect(toolSection).toMatch(/recommendation/i); + expect(toolSection).toMatch(/trade-?offs?/i); + expect(toolSection).toMatch(/risks/i); + expect(toolSection).toMatch(/future\s+extension\s+paths/i); + expect(toolSection.toLowerCase()).toContain("simpler alternatives"); + expect(toolSection).toMatch(/implementation\s+notes/i); + expect(toolSection).toMatch(/decision\s+confidence/i); + + // All fields present verbatim + expect(result).toContain("Question: Should we migrate our file storage from local filesystem to an object store?"); + expect(result).toContain("Context: Current system stores uploads on the application server's disk. We're getting disk space alerts and need multi-server support."); + expect(result).toContain("Constraints:"); + expect(result).toContain("- Must be deployable without Kubernetes"); + expect(result).toContain("- Cost must not exceed current $50/mo by more than 50%"); + expect(result).toContain( + "Expected output: A comparison of S3, MinIO, and shared NFS with trade-off analysis." + ); + expect(result).toContain( + "Project summary: Internal document management system for a mid-sized company." + ); + expect(result).toContain( + "Task summary: Migrating from local disk storage to a scalable file storage solution." + ); + + // Relevant files present + expect(toolSection).toContain("Relevant files:"); + expect(toolSection).toContain("File: src/storage/local-filesystem.js"); + expect(toolSection).toContain("Language: javascript"); + expect(toolSection).toContain("fs.writeFileSync"); + expect(toolSection).toContain("File: src/middleware/upload.js"); + + // Does not contain other tool content + expect(toolSection).not.toContain("Likely Causes"); + expect(toolSection).not.toContain("Fast Checks"); + expect(toolSection).not.toContain("missing steps"); + expect(toolSection).not.toContain("scope creep"); + expect(toolSection.toLowerCase()).not.toContain("issues by severity"); + }); +});