From d94f5b97c154b61600897eb6f4ff9e5fd2d6a052 Mon Sep 17 00:00:00 2001 From: robbond Date: Fri, 12 Jun 2026 13:03:47 +0100 Subject: [PATCH] fix: normalize MCP tool error formatting --- AGENT_HANDOFF.md | 13 ++++++++++++- PROJECT_STATE.md | 8 +++++--- TASKS.md | 41 ++++++++++++++++++++++++++++++++++++++- src/server.js | 50 +++++++++++++++++++++++++++++++++--------------- 4 files changed, 92 insertions(+), 20 deletions(-) diff --git a/AGENT_HANDOFF.md b/AGENT_HANDOFF.md index 928e5db..8b2a668 100644 --- a/AGENT_HANDOFF.md +++ b/AGENT_HANDOFF.md @@ -105,9 +105,20 @@ Four additional MCP tools registered on the server (`src/server.js`): - All handlers use existing modules only (no new imports or files). - SDK quirk: `isError: true` wraps results in JSON-RPC error envelope (`code: -32603`). +### Task 5.4 - Normalize MCP tool error formatting ✅ + +MCP tool error formatting normalized in `src/server.js`. All 5 tool registrations now produce a single "Error:" prefix — no more duplicate `"Error: Error:"` strings. + +**Changes:** Each tool callback normalizes the error text before returning: +- If `result.error` already starts with `"Error:"`, it is used as-is. +- Otherwise, `"Error: "` is prepended. +- `result.ok` responses are unchanged. + +Smoke tests: npm test 523 passed ✅ · tools/list 5 tools ✅ · ask_chatgpt single prefix ✅ · review_plan single prefix ✅ + ## Next Pending -### Task 5.4 - Normalize MCP tool error formatting +### Task 5.5 - Claude Code MCP configuration and local end-to-end setup ## General Rules diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 2fbee2a..768bba3 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. Task 5.3 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. Task 5.4 complete. ## Current Phase @@ -71,7 +71,9 @@ Phase 5 - MCP Server and Tool Registration - 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) +- Task 5.4 — Normalize MCP tool error formatting ✅ + +MCP tool error formatting normalized in `src/server.js`. All 5 tools now produce a single "Error:" prefix with no duplicates. ## Phase 3 Completion Summary @@ -124,4 +126,4 @@ All five tool handlers are implemented and tested: ## Next Pending -### Task 5.4 - Normalize MCP tool error formatting +### Task 5.5 - Claude Code MCP configuration and local end-to-end setup diff --git a/TASKS.md b/TASKS.md index 85a7560..56cdfc5 100644 --- a/TASKS.md +++ b/TASKS.md @@ -526,4 +526,43 @@ Register `review_plan`, `review_code`, `debug_issue`, and `architecture_review` Status: ✅ Complete -### Task 5.4 - Normalize MCP tool error formatting (NEXT) +### Task 5.4 - Normalize MCP tool error formatting ✅ + +Ensure all MCP tool error messages have a single, consistent "Error:" prefix. + +Before this change, handlers returned errors prefixed with `"Error: "` and server.js prepended another `"Error: "`, producing duplicates like: + +``` +Error: Error: Configuration error: OPENAI_API_KEY is missing. +``` + +**Changes made (`src/server.js`):** + +Each of the 5 tool registration callbacks now normalizes error text: + +```js +const text = result.ok + ? result.answer + : String(result.error || "Unknown error").startsWith("Error:") + ? result.error + : `Error: ${result.error}`; +``` + +After fix: + +- `"Error: Configuration error: OPENAI_API_KEY is missing."` — single prefix ✅ +- Non-"Error:"-prefixed errors receive one prefix added by the server ✅ +- Success responses unchanged ✅ + +Smoke tests: + +| Test | Result | +|------|--------| +| npm test | ✅ 523 passed, no regressions | +| tools/list | ✅ 5 tools, unchanged | +| ask_chatgpt (no API key) | ✅ Single "Error:" prefix | +| review_plan (no API key) | ✅ Single "Error:" prefix | + +Status: ✅ Complete + +### Task 5.5 - Claude Code MCP configuration and local end-to-end setup (NEXT) diff --git a/src/server.js b/src/server.js index 0dadd54..c93005e 100644 --- a/src/server.js +++ b/src/server.js @@ -29,10 +29,14 @@ server.registerTool( sendOpenAIResponse, }); + const text = result.ok + ? result.answer + : String(result.error || "Unknown error").startsWith("Error:") + ? result.error + : `Error: ${result.error}`; + return { - content: [ - { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, - ], + content: [{ type: "text", text }], isError: !result.ok, warnings: result.warnings?.length ? result.warnings : undefined, }; @@ -53,10 +57,14 @@ server.registerTool( sendOpenAIResponse, }); + const text = result.ok + ? result.answer + : String(result.error || "Unknown error").startsWith("Error:") + ? result.error + : `Error: ${result.error}`; + return { - content: [ - { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, - ], + content: [{ type: "text", text }], isError: !result.ok, warnings: result.warnings?.length ? result.warnings : undefined, }; @@ -77,10 +85,14 @@ server.registerTool( sendOpenAIResponse, }); + const text = result.ok + ? result.answer + : String(result.error || "Unknown error").startsWith("Error:") + ? result.error + : `Error: ${result.error}`; + return { - content: [ - { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, - ], + content: [{ type: "text", text }], isError: !result.ok, warnings: result.warnings?.length ? result.warnings : undefined, }; @@ -101,10 +113,14 @@ server.registerTool( sendOpenAIResponse, }); + const text = result.ok + ? result.answer + : String(result.error || "Unknown error").startsWith("Error:") + ? result.error + : `Error: ${result.error}`; + return { - content: [ - { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, - ], + content: [{ type: "text", text }], isError: !result.ok, warnings: result.warnings?.length ? result.warnings : undefined, }; @@ -125,10 +141,14 @@ server.registerTool( sendOpenAIResponse, }); + const text = result.ok + ? result.answer + : String(result.error || "Unknown error").startsWith("Error:") + ? result.error + : `Error: ${result.error}`; + return { - content: [ - { type: "text", text: result.ok ? result.answer : `Error: ${result.error}` }, - ], + content: [{ type: "text", text }], isError: !result.ok, warnings: result.warnings?.length ? result.warnings : undefined, };