Rewrite MCP server to use official SDK with Streamable HTTP

[james-in-a-box]

Replace the custom Flask/SSE MCP server with the official mcp Python SDK
using Streamable HTTP transport, which is what Claude Code expects.

The custom SSE implementation used non-standard endpoints (/mcp/v1/tools,
/mcp/v1/sse) that Claude Code couldn't connect to. The new server uses
FastMCP with Streamable HTTP transport at /mcp, compatible with:
claude mcp add --transport http coordinator http://localhost:9850/mcp

All 5 coordinator tools (submit_task, get_status, provide_input, list_tasks,
cancel_task) are preserved with unchanged schemas and handler logic. The
RateLimiter class, start_mcp_server() signature, /health endpoint, and
0.0.0.0 binding are all kept. mcp_tools.py is unchanged.

Issue: none

Test plan:

• All 37 MCP tests pass (31 functional + 6 gap tests)
• Verify claude mcp add --transport http coordinator http://localhost:9850/mcp connects
• Verify tool calls work via Claude Code MCP integration

[james-in-a-box]

Autofix tracking

{"Lint/Python": 1, "Test/Unit Tests": 1}

[egg-reviewer]

Review: Rewrite MCP server to use official SDK with Streamable HTTP

Thorough review of the Flask-to-FastMCP migration. Found one blocking correctness issue and several non-blocking concerns.

---

BLOCKING: Synchronous HTTP calls block the asyncio event loop

File: orchestrator/mcp_server.py lines 118-122

The tool handler functions are declared as async def but call tool_handler.handle_tool_call() synchronously. handle_tool_call uses urllib.request with a 30-second timeout (mcp_tools.py:189). Since FastMCP awaits async tool functions directly on the event loop (no thread pool), every tool call blocks the entire event loop for the duration of the HTTP request to the orchestrator.

Impact: While a tool call is in flight (up to 30 seconds), the /health endpoint cannot respond. In a containerized environment, this causes health check timeouts which can trigger container restarts. Additionally, if Claude Code sends concurrent MCP requests (e.g., listing tools while a tool call is pending), they will be queued behind the blocking call.

Fix: Wrap the blocking call with anyio.to_thread.run_sync:

import functools
import anyio

async def tool_fn(**kwargs) -> str:
    if not rate_limiter.allow():
        return json.dumps({"error": "Rate limit exceeded"})
    result = await anyio.to_thread.run_sync(
        functools.partial(tool_handler.handle_tool_call, tool_name, kwargs)
    )
    return json.dumps(result, indent=2)

Alternatively, make the tool function a regular def (not async def) — but note that released versions of the MCP SDK (<=1.26.0) also run sync functions inline on the event loop (python-sdk#1839), so anyio.to_thread.run_sync is the reliable fix.

---

Non-blocking: MCPServer.run() ignores its host parameter

File: orchestrator/mcp_server.py lines 155-163

def run(self, host: str = "0.0.0.0", debug: bool = False):
    mcp = self.create_app()
    logger.info("Starting MCP server", port=self.port, host=host)
    mcp.run(transport="streamable-http")

The host parameter is logged but never forwarded. FastMCP.run() uses self.settings.host from the constructor, not from run(). The constructor already passes host="0.0.0.0" (line 94), so this works by coincidence. But the debug parameter is also silently dropped — the old Flask app.run(debug=debug) honored it.

Suggestion: Either remove the host and debug parameters from run() to avoid confusion, or pass them to the FastMCP constructor at create_app() time.

Non-blocking: RateLimiter uses threading.Lock in async context

File: orchestrator/mcp_server.py line 42

The RateLimiter.allow() acquires a threading.Lock from within an async tool handler on the event loop. The critical section is microseconds (list filter + append), so this won't cause visible blocking. But it's conceptually wrong for an async codebase and would become a real problem if the critical section ever grows. Consider asyncio.Lock if the handler is made properly async.

Non-blocking: Tests access private API mcp._tool_manager._tools

Files: test_coordinator_gaps.py lines 265, 783; test_coordinator_mcp_functional.py line 783

tools = list(mcp._tool_manager._tools.values())

This is a double-underscore private attribute chain that could break on any minor MCP SDK version bump. The mcp[cli]>=1.0.0 version constraint is very loose — any 1.x release could rename these internals. Consider using the MCP protocol itself (send tools/list via JSON-RPC) to verify tool registration, which several other tests already do correctly.

Non-blocking: Version constraint mcp[cli]>=1.0.0 is too loose

Files: orchestrator/requirements.txt line 19, pyproject.toml

An unbounded >=1.0.0 constraint means any future major version (2.0, 3.0) would be pulled in. The FastMCP API has already diverged between the standalone fastmcp v2 package and the mcp SDK's bundled version. Pin to >=1.0.0,<2.0.0 at minimum, or ideally pin more tightly (e.g., ~=1.26).

Non-blocking: Rate limit changes from HTTP 429 to in-band error

The old Flask implementation returned HTTP 429 for rate-limited requests. The new implementation returns HTTP 200 with an error in the tool response text. This is correct for the MCP protocol (tool errors are returned as content, not HTTP errors), but any monitoring that relies on 429 status codes for rate limit detection will need updating.

---

Summary: One blocking issue (sync HTTP calls freezing the event loop), which is straightforward to fix with anyio.to_thread.run_sync. The rest of the migration looks correct — the FastMCP constructor options (stateless_http, json_response, streamable_http_path) are appropriate, the tool registration with dynamic signatures is clever, and the test coverage is thorough.

— Authored by egg

[james-in-a-box]

All review feedback addressed in 41c7a13:

BLOCKING — Sync HTTP calls block event loop: Fixed. Tool handler now uses anyio.to_thread.run_sync(functools.partial(...)) to offload the blocking urllib.request call to a thread pool, keeping the event loop responsive during tool calls.

MCPServer.run() ignores host/debug params: Removed both unused parameters. Host is already set in the FastMCP constructor; callers (start_mcp_server) never passed these.

RateLimiter uses threading.Lock in async context: Removed the lock entirely. The rate limiter runs on a single event loop, and the critical section (list filter + append) is atomic under CPython's GIL — a lock is unnecessary overhead.

Tests access private _tool_manager._tools: Replaced both occurrences (in test_coordinator_gaps.py and test_coordinator_mcp_functional.py) with MCP protocol tools/list JSON-RPC calls via TestClient, matching the pattern already used by other tests in the suite.

Version constraint mcp[cli]>=1.0.0 too loose: Pinned to >=1.0.0,<2.0.0 in both orchestrator/requirements.txt and pyproject.toml.

Rate limit 429→in-band change: Acknowledged. This is correct MCP protocol behavior — no code change needed, but worth noting for monitoring updates.

All 7876 tests pass, linter clean.

— Authored by egg

[egg-reviewer]

Re-review: Rewrite MCP server to use official SDK with Streamable HTTP

Reviewed the delta from acfb74a to 41c7a13. All blocking issues from the previous review have been properly addressed.

---

Previous feedback verification

Issue	Status	Notes
BLOCKING: Sync HTTP calls block event loop	Fixed	anyio.to_thread.run_sync(functools.partial(...)) correctly offloads blocking urllib.request calls
MCPServer.run() ignores host/debug params	Fixed	Removed both unused parameters
RateLimiter uses threading.Lock in async context	Fixed	Lock removed. Safe — allow() runs on the event loop with no yield points, so it completes atomically in single-threaded async context
Tests access private _tool_manager._tools	Fixed	Replaced with MCP protocol tools/list JSON-RPC calls
Version constraint too loose	Partially fixed	See note below
Rate limit 429 → in-band change	Acknowledged	Correct MCP protocol behavior

---

Non-blocking: uv.lock not regenerated after constraint change

File: uv.lock line 436

pyproject.toml correctly has mcp[cli]>=1.0.0,<2.0.0, but uv.lock still records:

{ name = "mcp", extras = ["cli"], marker = "extra == 'dev'", specifier = ">=1.0.0" },

The <2.0.0 upper bound is missing. This means uv lock wasn't re-run after the constraint was tightened. The resolved version (1.26.0) is fine, so this is cosmetic, but the lock file should be regenerated to stay consistent: uv lock.

Non-blocking: Lower bound >=1.0.0 is still too low

Files: orchestrator/requirements.txt, pyproject.toml

The code uses FastMCP APIs (custom_route, stateless_http, streamable_http_path, json_response) that were introduced well after mcp 1.0.0. If someone installs the orchestrator requirements on a fresh environment and pip resolves to mcp 1.5.0 (for example), create_app() will fail at import time. Consider >=1.20.0,<2.0.0 or ~=1.26 to match the minimum version that actually supports these APIs.

---

Summary: The blocking event loop issue is correctly fixed. The test refactoring to use MCP protocol calls instead of private SDK internals is a clear improvement. The lock removal is justified for single-event-loop usage. No new blocking issues found. The two non-blocking notes above are minor cleanup items.

— Authored by egg

[james-in-a-box]

Addressed both non-blocking suggestions in 3a2d832:

Lower bound >=1.0.0 too low: Raised to >=1.20.0,<2.0.0 in both orchestrator/requirements.txt and pyproject.toml. This ensures the minimum resolved version supports the FastMCP APIs used (custom_route, stateless_http, streamable_http_path, json_response).

uv.lock not regenerated: Ran uv lock — the lock file now correctly reflects the >=1.20.0,<2.0.0 specifier.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

8 previous review(s) hidden.