diff --git a/AGENT_HANDOFF.md b/AGENT_HANDOFF.md index a33c7d2..928e5db 100644 --- a/AGENT_HANDOFF.md +++ b/AGENT_HANDOFF.md @@ -53,6 +53,8 @@ Total: 139 orchestration-only tests, all passing. ## Completed (Phase 5) +**All five MCP tools registered:** ask_chatgpt, review_plan, review_code, debug_issue, architecture_review. + ### Task 5.1 - MCP server skeleton ✅ Minimal MCP stdio server in `src/server.js`. MCP initialize handshake succeeds. No tools registered yet. @@ -78,9 +80,34 @@ Minimal MCP stdio server in `src/server.js`. MCP initialize handshake succeeds. **Production code (`src/server.js`):** ~39 lines, single `ask_chatgpt` tool registered with MCP via Stdio transport. +### Task 5.3 - Register remaining MCP tools ✅ + +Four additional MCP tools registered on the server (`src/server.js`): + +| Tool | Handler | +|------|---------| +| `review_plan` | handleReviewPlan | +| `review_code` | handleReviewCode | +| `debug_issue` | handleDebugIssue | +| `architecture_review` | handleArchitectureReview | + +**All five MCP tools now registered:** ask_chatgpt, review_plan, review_code, debug_issue, architecture_review. + +**Smoke test results (all passing):** +- initialize → server returns `chatgpt-mcp` v0.1.0 ✅ +- tools/list → 5 tools total ✅ +- tools/call reaches handlers for all 5 tools ✅ +- missing OPENAI_API_KEY → structured tool errors: `"Error: Configuration error: OPENAI_API_KEY is missing."` ✅ +- npm test → 523 tests pass across 18 test files, no regressions ✅ + +**Implementation notes:** +- Each tool registered explicitly with its own `registerTool()` call — no registry abstraction. +- All handlers use existing modules only (no new imports or files). +- SDK quirk: `isError: true` wraps results in JSON-RPC error envelope (`code: -32603`). + ## Next Pending -### Task 5.3 - Register remaining MCP tools (review_plan, review_code, debug_issue, architecture_review) +### Task 5.4 - Normalize MCP tool error formatting ## General Rules diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 7cac035..2fbee2a 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -7,7 +7,7 @@ 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. Task 4.5 complete. Phase 4 complete. Task 5.1 complete. Task 5.2 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. Task 5.1 complete. Task 5.2 complete. Task 5.3 complete. ## Current Phase @@ -54,7 +54,24 @@ Phase 5 - MCP Server and Tool Registration - tools/call (invalid API key) → structured MCP error `"OpenAI API error (OpenAIAuthError): 401"` ✅ - All 523 unit tests pass across 18 test files ✅ -- Task 5.3 — Register remaining MCP tools (NEXT) +- Task 5.3 — Register remaining MCP tools ✅ + + **Registered tools:** + - `review_plan` → handleReviewPlan + - `review_code` → handleReviewCode + - `debug_issue` → handleDebugIssue + - `architecture_review` → handleArchitectureReview + + **All five MCP tools now registered:** ask_chatgpt, review_plan, review_code, debug_issue, architecture_review. + + **Smoke test results (all passing):** + - initialize → server returns `chatgpt-mcp` v0.1.0 ✅ + - tools/list → 5 tools total ✅ + - tools/call reaches handlers for all 5 tools ✅ + - missing OPENAI_API_KEY → structured tool errors (`"Error: Configuration error: OPENAI_API_KEY is missing."`) ✅ + - npm test → 523 tests pass across 18 test files, no regressions ✅ + +- Task 5.4 — Normalize MCP tool error formatting (NEXT) ## Phase 3 Completion Summary @@ -107,4 +124,4 @@ All five tool handlers are implemented and tested: ## Next Pending -### Task 5.3 - Register remaining MCP tools (review_plan, review_code, debug_issue, architecture_review) +### Task 5.4 - Normalize MCP tool error formatting diff --git a/TASKS.md b/TASKS.md index ae30eea..85a7560 100644 --- a/TASKS.md +++ b/TASKS.md @@ -491,4 +491,39 @@ await server.connect(new StdioServerTransport()); Status: ✅ Complete -### Task 5.3 - Register remaining MCP tools (NEXT) +### Task 5.3 - Register remaining MCP tools + +Register `review_plan`, `review_code`, `debug_issue`, and `architecture_review` as MCP tools on the server in `src/server.js`. + +**Requirements:** +- Import four existing handlers: `handleReviewPlan`, `handleReviewCode`, `handleDebugIssue`, `handleArchitectureReview`. +- Use `baseInputSchema` for all tool input schemas. +- Explicitly register each tool with its own `registerTool()` call — no registry abstraction, no dynamic loop. +- Pass all three deps (`loadConfig`, `createOpenAIClient`, `sendOpenAIResponse`) into each handler. +- Return structured MCP tool result: `{ content: [{ type: "text", text }], isError, warnings }`. + +**Smoke test results (all passing):** + +| Test | Result | +|------|--------| +| initialize handshake | ✅ Server returns `chatgpt-mcp` v0.1.0 | +| tools/list count | ✅ Returns 5 tools total | +| all tool names present | ✅ ask_chatgpt, review_plan, review_code, debug_issue, architecture_review | +| tools/call each tool (no API key) | ✅ Structured MCP error for all 5: `"Error: Configuration error: OPENAI_API_KEY is missing."` | +| npm test | ✅ 523 tests pass across 18 test files — no regressions | + +**Production code (`src/server.js`) changes:** + +- Added imports: `handleReviewPlan`, `handleReviewCode`, `handleDebugIssue`, `handleArchitectureReview` +- Added four `registerTool()` calls: `review_plan`, `review_code`, `debug_issue`, `architecture_review` +- Existing `ask_chatgpt` registration unchanged +- No new files created + +**SDK quirks noted:** + +1. `notifications/initialized` is not handled by the SDK — produces `-32601 Method not found`. Harmless. +2. When `isError: true`, the SDK wraps results in a JSON-RPC error envelope (`{"error":{"code":-32603,...}}`) rather than a success result. + +Status: ✅ Complete + +### Task 5.4 - Normalize MCP tool error formatting (NEXT) diff --git a/src/server.js b/src/server.js index ba6847c..0dadd54 100644 --- a/src/server.js +++ b/src/server.js @@ -2,6 +2,10 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { baseInputSchema } from "./tools/schemas.js"; import { handleAskChatGpt } from "./tools/ask-chatgpt.js"; +import { handleReviewPlan } from "./tools/review-plan.js"; +import { handleReviewCode } from "./tools/review-code.js"; +import { handleDebugIssue } from "./tools/debug-issue.js"; +import { handleArchitectureReview } from "./tools/architecture-review.js"; import { loadConfig } from "./config/env.js"; import { createOpenAIClient } from "./openai/client.js"; import { sendOpenAIResponse } from "./openai/responses.js"; @@ -35,4 +39,100 @@ server.registerTool( } ); +server.registerTool( + "review_plan", + { + description: + "Review a proposed implementation plan before acting. Looks for missing steps, unsafe assumptions, scope creep, sequencing, test gaps.", + inputSchema: baseInputSchema, + }, + async (input) => { + const result = await handleReviewPlan(input, { + loadConfig, + createOpenAIClient, + sendOpenAIResponse, + }); + + return { + content: [ + { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, + ], + isError: !result.ok, + warnings: result.warnings?.length ? result.warnings : undefined, + }; + } +); + +server.registerTool( + "review_code", + { + description: + "Review focused code snippets, patches, or diffs. Looks for correctness, bugs, maintainability, security issues, tests, simpler approaches.", + inputSchema: baseInputSchema, + }, + async (input) => { + const result = await handleReviewCode(input, { + loadConfig, + createOpenAIClient, + sendOpenAIResponse, + }); + + return { + content: [ + { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, + ], + isError: !result.ok, + warnings: result.warnings?.length ? result.warnings : undefined, + }; + } +); + +server.registerTool( + "debug_issue", + { + description: + "Analyse errors, logs, failed tests, or stack traces. Looks for root causes, fast checks, minimal safe experiments, what not to change yet.", + inputSchema: baseInputSchema, + }, + async (input) => { + const result = await handleDebugIssue(input, { + loadConfig, + createOpenAIClient, + sendOpenAIResponse, + }); + + return { + content: [ + { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, + ], + isError: !result.ok, + warnings: result.warnings?.length ? result.warnings : undefined, + }; + } +); + +server.registerTool( + "architecture_review", + { + description: + "Review architecture decisions and trade-offs. Looks at local vs cloud simplicity, maintainability, operational risk, vendor lock-in, future extension paths.", + inputSchema: baseInputSchema, + }, + async (input) => { + const result = await handleArchitectureReview(input, { + loadConfig, + createOpenAIClient, + sendOpenAIResponse, + }); + + return { + content: [ + { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, + ], + isError: !result.ok, + warnings: result.warnings?.length ? result.warnings : undefined, + }; + } +); + await server.connect(new StdioServerTransport());