feat: register remaining MCP tools

2026-06-12 12:38:52 +01:00
parent d3aefa5bb8
commit b65ee3a598
4 changed files with 184 additions and 5 deletions
@@ -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

@@ -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
@@ -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)
@@ -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());