Skip to content

Commit 9260f31

Browse files
DeepfreezechillBrian KrafftCopilot
authored
feat(4.9): Extract MCPServer into openspace/mcp/server.py (#37)
* feat(4.9): extract MCPServer into openspace/mcp/server.py Extract all MCP server bootstrap, FastMCP creation, and run_mcp_server() from mcp_server.py into openspace/mcp/server.py. Changes: - Create openspace/mcp/server.py (236 lines) with: - _MCPSafeStdout class (stdio transport wrapper) - Logging/stderr bootstrap (Windows pipe deadlock prevention) - create_mcp_app() factory (FastMCP + tool handler registration) - run_mcp_server() entry point (CLI, transport, auth middleware) - Slim mcp_server.py to 34-line thin entry point (re-exports + __main__) - Add 15 new tests in test_mcp_server.py - Skip Epic 4.8 (auth middleware already extracted in P3) openspace/mcp/ package now owns all MCP concerns: server.py — bootstrap, FastMCP, entry point (236 lines) tool_handlers.py — 4 MCP tools + helpers (799 lines) __init__.py — package marker Original mcp_server.py: 992 lines → 34-line backward-compat shim. All 1,391 tests pass, 117 skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review findings for Epic 4.9 - C1: test_tools_registered now asserts all 4 tool names (was no-op) - M1: mcp_server.py uses lazy proxy — no wasted FastMCP on console script - L1: register_handlers docstring updated to reference create_mcp_app() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Brian Krafft <bkrafft@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ded8183 commit 9260f31

4 files changed

Lines changed: 417 additions & 200 deletions

File tree

openspace/mcp/server.py

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
"""OpenSpace MCP Server — bootstrap, FastMCP creation, and entry point.
2+
3+
Extracted from ``mcp_server.py`` (Epic 4.9) to consolidate all MCP server
4+
concerns under ``openspace/mcp/``.
5+
6+
Sections:
7+
1. _MCPSafeStdout — stdout wrapper for MCP stdio transport
8+
2. Logging/stderr bootstrap — Windows pipe deadlock prevention
9+
3. create_mcp_app() — FastMCP factory with tool handler registration
10+
4. run_mcp_server() — CLI entry point with transport + auth setup
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import atexit
16+
import inspect
17+
import logging
18+
import os
19+
import sys
20+
from pathlib import Path
21+
22+
23+
# ---------------------------------------------------------------------------
24+
# 1. Stdout wrapper for MCP stdio transport
25+
# ---------------------------------------------------------------------------
26+
class _MCPSafeStdout:
27+
"""Stdout wrapper: binary (.buffer) → real stdout, text (.write) → stderr.
28+
29+
MCP stdio transport requires stdout for JSON-RPC messages only.
30+
Any text written by application code (print, warnings) must go to stderr.
31+
"""
32+
33+
def __init__(self, real_stdout, stderr):
34+
self._real = real_stdout
35+
self._stderr = stderr
36+
37+
@property
38+
def buffer(self):
39+
return self._real.buffer
40+
41+
def fileno(self):
42+
return self._real.fileno()
43+
44+
def write(self, s):
45+
return self._stderr.write(s)
46+
47+
def writelines(self, lines):
48+
return self._stderr.writelines(lines)
49+
50+
def flush(self):
51+
self._stderr.flush()
52+
self._real.flush()
53+
54+
def isatty(self):
55+
return self._stderr.isatty()
56+
57+
@property
58+
def encoding(self):
59+
return self._stderr.encoding
60+
61+
@property
62+
def errors(self):
63+
return self._stderr.errors
64+
65+
@property
66+
def closed(self):
67+
return self._stderr.closed
68+
69+
def readable(self):
70+
return False
71+
72+
def writable(self):
73+
return True
74+
75+
def seekable(self):
76+
return False
77+
78+
def __getattr__(self, name):
79+
return getattr(self._stderr, name)
80+
81+
82+
# ---------------------------------------------------------------------------
83+
# 2. Logging / stderr bootstrap
84+
# ---------------------------------------------------------------------------
85+
_LOG_DIR = Path(__file__).resolve().parent.parent.parent / "logs"
86+
_LOG_DIR.mkdir(parents=True, exist_ok=True)
87+
88+
_real_stdout = sys.stdout
89+
90+
# Windows pipe buffers are small. When using stdio MCP transport,
91+
# the parent process only reads stdout for MCP messages and does NOT
92+
# drain stderr. Heavy log/print output during execute_task fills the stderr
93+
# pipe buffer, blocking this process on write() → deadlock → timeout.
94+
# Redirect stderr to a log file on Windows to prevent this.
95+
_stderr_file = None
96+
if os.name == "nt":
97+
_stderr_file = open(_LOG_DIR / "mcp_stderr.log", "a", encoding="utf-8", buffering=1)
98+
sys.stderr = _stderr_file
99+
100+
sys.stdout = _MCPSafeStdout(_real_stdout, sys.stderr)
101+
102+
_log_file_handler = logging.FileHandler(_LOG_DIR / "mcp_server.log")
103+
logging.basicConfig(
104+
level=logging.INFO,
105+
format="%(asctime)s - %(name)s - %(levelname)s - %(message)s",
106+
handlers=[_log_file_handler],
107+
)
108+
logger = logging.getLogger("openspace.mcp_server")
109+
110+
111+
def _cleanup_file_handles():
112+
"""Clean up file handles on exit."""
113+
global _stderr_file, _log_file_handler
114+
if _stderr_file:
115+
_stderr_file.close()
116+
_stderr_file = None
117+
if _log_file_handler:
118+
_log_file_handler.close()
119+
120+
121+
atexit.register(_cleanup_file_handles)
122+
123+
124+
# ---------------------------------------------------------------------------
125+
# 3. FastMCP app factory
126+
# ---------------------------------------------------------------------------
127+
def create_mcp_app():
128+
"""Create and return a configured FastMCP instance with all tools registered.
129+
130+
This is the single factory for the MCP application. Tool handlers are
131+
wired via ``register_handlers()`` from the tool_handlers module.
132+
"""
133+
from mcp.server.fastmcp import FastMCP
134+
135+
from openspace.mcp.tool_handlers import register_handlers
136+
137+
kwargs: dict = {}
138+
try:
139+
if "description" in inspect.signature(FastMCP.__init__).parameters:
140+
kwargs["description"] = "OpenSpace: Unite the Agents. Evolve the Mind. Rebuild the World."
141+
except (TypeError, ValueError):
142+
pass
143+
144+
app = FastMCP("OpenSpace", **kwargs)
145+
register_handlers(app)
146+
return app
147+
148+
149+
# ---------------------------------------------------------------------------
150+
# 4. Server entry point
151+
# ---------------------------------------------------------------------------
152+
def run_mcp_server(mcp=None) -> None:
153+
"""Console-script entry point for ``openspace-mcp``.
154+
155+
For HTTP transports (SSE, streamable-http), bearer token auth is
156+
REQUIRED. Set OPENSPACE_MCP_BEARER_TOKEN in the environment.
157+
The server refuses to start without it (fail-closed).
158+
159+
For stdio transport, auth is not applicable (local process IPC).
160+
161+
Args:
162+
mcp: Optional pre-created FastMCP instance. If None, creates one
163+
via ``create_mcp_app()``.
164+
"""
165+
import argparse
166+
167+
import uvicorn
168+
169+
from openspace.auth.bearer import (
170+
BEARER_TOKEN_ENV,
171+
BearerTokenMiddleware,
172+
get_bearer_token,
173+
validate_token_strength,
174+
)
175+
from openspace.auth.rate_limit import RateLimitMiddleware
176+
177+
if mcp is None:
178+
mcp = create_mcp_app()
179+
180+
parser = argparse.ArgumentParser(description="OpenSpace MCP Server")
181+
parser.add_argument(
182+
"--transport",
183+
choices=["stdio", "sse", "streamable-http"],
184+
default="stdio",
185+
)
186+
parser.add_argument("--port", type=int, default=8080)
187+
parser.add_argument("--host", type=str, default="127.0.0.1")
188+
args = parser.parse_args()
189+
190+
if args.transport == "stdio":
191+
mcp.run(transport="stdio")
192+
return
193+
194+
# --- HTTP transports: enforce bearer token auth (fail-closed) ---
195+
token = get_bearer_token()
196+
if not token:
197+
logger.critical(
198+
"FAIL-CLOSED: %s not set. Refusing to start %s transport "
199+
"without authentication. Set the environment variable or "
200+
"use --transport stdio for local-only access.",
201+
BEARER_TOKEN_ENV,
202+
args.transport,
203+
)
204+
sys.exit(1)
205+
206+
token_ok, reason = validate_token_strength(token)
207+
if not token_ok:
208+
logger.critical("FAIL-CLOSED: %s — %s", BEARER_TOKEN_ENV, reason)
209+
sys.exit(1)
210+
211+
if args.transport == "sse":
212+
starlette_app = mcp.sse_app()
213+
else:
214+
starlette_app = mcp.streamable_http_app()
215+
216+
# Middleware chain: request → BearerAuth → RateLimit → MCP app
217+
rate_limited_app = RateLimitMiddleware(starlette_app)
218+
protected_app = BearerTokenMiddleware(rate_limited_app, token)
219+
220+
logger.info(
221+
"Starting MCP server with bearer auth + rate limiting on %s:%d (%s transport)",
222+
args.host,
223+
args.port,
224+
args.transport,
225+
)
226+
227+
config = uvicorn.Config(
228+
protected_app,
229+
host=args.host,
230+
port=args.port,
231+
log_level="info",
232+
)
233+
server = uvicorn.Server(config)
234+
import anyio
235+
236+
anyio.run(server.serve)

openspace/mcp/tool_handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,7 @@ async def upload_skill(
791791
def register_handlers(mcp) -> None:
792792
"""Wire all tool handlers to the given FastMCP instance.
793793
794-
Called by ``mcp_server.py`` after creating the FastMCP app.
794+
Called by ``create_mcp_app()`` in ``openspace.mcp.server``.
795795
"""
796796
mcp.tool()(execute_task)
797797
mcp.tool()(search_skills)

0 commit comments

Comments
 (0)