diff --git a/openspace/mcp/server.py b/openspace/mcp/server.py new file mode 100644 index 00000000..e4235a53 --- /dev/null +++ b/openspace/mcp/server.py @@ -0,0 +1,236 @@ +"""OpenSpace MCP Server — bootstrap, FastMCP creation, and entry point. + +Extracted from ``mcp_server.py`` (Epic 4.9) to consolidate all MCP server +concerns under ``openspace/mcp/``. + +Sections: + 1. _MCPSafeStdout — stdout wrapper for MCP stdio transport + 2. Logging/stderr bootstrap — Windows pipe deadlock prevention + 3. create_mcp_app() — FastMCP factory with tool handler registration + 4. run_mcp_server() — CLI entry point with transport + auth setup +""" + +from __future__ import annotations + +import atexit +import inspect +import logging +import os +import sys +from pathlib import Path + + +# --------------------------------------------------------------------------- +# 1. Stdout wrapper for MCP stdio transport +# --------------------------------------------------------------------------- +class _MCPSafeStdout: + """Stdout wrapper: binary (.buffer) → real stdout, text (.write) → stderr. + + MCP stdio transport requires stdout for JSON-RPC messages only. + Any text written by application code (print, warnings) must go to stderr. + """ + + def __init__(self, real_stdout, stderr): + self._real = real_stdout + self._stderr = stderr + + @property + def buffer(self): + return self._real.buffer + + def fileno(self): + return self._real.fileno() + + def write(self, s): + return self._stderr.write(s) + + def writelines(self, lines): + return self._stderr.writelines(lines) + + def flush(self): + self._stderr.flush() + self._real.flush() + + def isatty(self): + return self._stderr.isatty() + + @property + def encoding(self): + return self._stderr.encoding + + @property + def errors(self): + return self._stderr.errors + + @property + def closed(self): + return self._stderr.closed + + def readable(self): + return False + + def writable(self): + return True + + def seekable(self): + return False + + def __getattr__(self, name): + return getattr(self._stderr, name) + + +# --------------------------------------------------------------------------- +# 2. Logging / stderr bootstrap +# --------------------------------------------------------------------------- +_LOG_DIR = Path(__file__).resolve().parent.parent.parent / "logs" +_LOG_DIR.mkdir(parents=True, exist_ok=True) + +_real_stdout = sys.stdout + +# Windows pipe buffers are small. When using stdio MCP transport, +# the parent process only reads stdout for MCP messages and does NOT +# drain stderr. Heavy log/print output during execute_task fills the stderr +# pipe buffer, blocking this process on write() → deadlock → timeout. +# Redirect stderr to a log file on Windows to prevent this. +_stderr_file = None +if os.name == "nt": + _stderr_file = open(_LOG_DIR / "mcp_stderr.log", "a", encoding="utf-8", buffering=1) + sys.stderr = _stderr_file + +sys.stdout = _MCPSafeStdout(_real_stdout, sys.stderr) + +_log_file_handler = logging.FileHandler(_LOG_DIR / "mcp_server.log") +logging.basicConfig( + level=logging.INFO, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + handlers=[_log_file_handler], +) +logger = logging.getLogger("openspace.mcp_server") + + +def _cleanup_file_handles(): + """Clean up file handles on exit.""" + global _stderr_file, _log_file_handler + if _stderr_file: + _stderr_file.close() + _stderr_file = None + if _log_file_handler: + _log_file_handler.close() + + +atexit.register(_cleanup_file_handles) + + +# --------------------------------------------------------------------------- +# 3. FastMCP app factory +# --------------------------------------------------------------------------- +def create_mcp_app(): + """Create and return a configured FastMCP instance with all tools registered. + + This is the single factory for the MCP application. Tool handlers are + wired via ``register_handlers()`` from the tool_handlers module. + """ + from mcp.server.fastmcp import FastMCP + + from openspace.mcp.tool_handlers import register_handlers + + kwargs: dict = {} + try: + if "description" in inspect.signature(FastMCP.__init__).parameters: + kwargs["description"] = "OpenSpace: Unite the Agents. Evolve the Mind. Rebuild the World." + except (TypeError, ValueError): + pass + + app = FastMCP("OpenSpace", **kwargs) + register_handlers(app) + return app + + +# --------------------------------------------------------------------------- +# 4. Server entry point +# --------------------------------------------------------------------------- +def run_mcp_server(mcp=None) -> None: + """Console-script entry point for ``openspace-mcp``. + + For HTTP transports (SSE, streamable-http), bearer token auth is + REQUIRED. Set OPENSPACE_MCP_BEARER_TOKEN in the environment. + The server refuses to start without it (fail-closed). + + For stdio transport, auth is not applicable (local process IPC). + + Args: + mcp: Optional pre-created FastMCP instance. If None, creates one + via ``create_mcp_app()``. + """ + import argparse + + import uvicorn + + from openspace.auth.bearer import ( + BEARER_TOKEN_ENV, + BearerTokenMiddleware, + get_bearer_token, + validate_token_strength, + ) + from openspace.auth.rate_limit import RateLimitMiddleware + + if mcp is None: + mcp = create_mcp_app() + + parser = argparse.ArgumentParser(description="OpenSpace MCP Server") + parser.add_argument( + "--transport", + choices=["stdio", "sse", "streamable-http"], + default="stdio", + ) + parser.add_argument("--port", type=int, default=8080) + parser.add_argument("--host", type=str, default="127.0.0.1") + args = parser.parse_args() + + if args.transport == "stdio": + mcp.run(transport="stdio") + return + + # --- HTTP transports: enforce bearer token auth (fail-closed) --- + token = get_bearer_token() + if not token: + logger.critical( + "FAIL-CLOSED: %s not set. Refusing to start %s transport " + "without authentication. Set the environment variable or " + "use --transport stdio for local-only access.", + BEARER_TOKEN_ENV, + args.transport, + ) + sys.exit(1) + + token_ok, reason = validate_token_strength(token) + if not token_ok: + logger.critical("FAIL-CLOSED: %s — %s", BEARER_TOKEN_ENV, reason) + sys.exit(1) + + if args.transport == "sse": + starlette_app = mcp.sse_app() + else: + starlette_app = mcp.streamable_http_app() + + # Middleware chain: request → BearerAuth → RateLimit → MCP app + rate_limited_app = RateLimitMiddleware(starlette_app) + protected_app = BearerTokenMiddleware(rate_limited_app, token) + + logger.info( + "Starting MCP server with bearer auth + rate limiting on %s:%d (%s transport)", + args.host, + args.port, + args.transport, + ) + + config = uvicorn.Config( + protected_app, + host=args.host, + port=args.port, + log_level="info", + ) + server = uvicorn.Server(config) + import anyio + + anyio.run(server.serve) diff --git a/openspace/mcp/tool_handlers.py b/openspace/mcp/tool_handlers.py index 93d9f586..db8b31aa 100644 --- a/openspace/mcp/tool_handlers.py +++ b/openspace/mcp/tool_handlers.py @@ -791,7 +791,7 @@ async def upload_skill( def register_handlers(mcp) -> None: """Wire all tool handlers to the given FastMCP instance. - Called by ``mcp_server.py`` after creating the FastMCP app. + Called by ``create_mcp_app()`` in ``openspace.mcp.server``. """ mcp.tool()(execute_task) mcp.tool()(search_skills) diff --git a/openspace/mcp_server.py b/openspace/mcp_server.py index 0291f04f..4044da7c 100644 --- a/openspace/mcp_server.py +++ b/openspace/mcp_server.py @@ -1,220 +1,51 @@ -"""OpenSpace MCP Server +"""OpenSpace MCP Server — thin entry point. -Exposes the following tools to MCP clients: - execute_task — Delegate a task (auto-registers skills, auto-searches, auto-evolves) - search_skills — Standalone search across local & cloud skills - fix_skill — Manually fix a broken skill (FIX only; DERIVED/CAPTURED via execute_task) - upload_skill — Upload a local skill to cloud (pre-saved metadata, bot decides visibility) +All MCP concerns live under ``openspace/mcp/``: + - ``openspace.mcp.server`` — bootstrap, FastMCP, run_mcp_server() + - ``openspace.mcp.tool_handlers`` — execute_task, search_skills, fix_skill, upload_skill + +This module exists for backward compatibility: + - ``python -m openspace.mcp_server`` still works + - ``from openspace.mcp_server import run_mcp_server`` still works + - Existing test imports that reference this module still work Usage: python -m openspace.mcp_server # stdio (default) python -m openspace.mcp_server --transport sse # SSE on port 8080 python -m openspace.mcp_server --port 9090 # SSE on custom port - -Environment variables: see ``openspace/host_detection/`` and ``openspace/cloud/auth.py``. """ from __future__ import annotations -import atexit -import inspect -import logging -import os -import sys -from pathlib import Path - - -class _MCPSafeStdout: - """Stdout wrapper: binary (.buffer) → real stdout, text (.write) → stderr.""" - - def __init__(self, real_stdout, stderr): - self._real = real_stdout - self._stderr = stderr - - @property - def buffer(self): - return self._real.buffer - - def fileno(self): - return self._real.fileno() - - def write(self, s): - return self._stderr.write(s) - - def writelines(self, lines): - return self._stderr.writelines(lines) - - def flush(self): - self._stderr.flush() - self._real.flush() - - def isatty(self): - return self._stderr.isatty() - - @property - def encoding(self): - return self._stderr.encoding - - @property - def errors(self): - return self._stderr.errors - - @property - def closed(self): - return self._stderr.closed - - def readable(self): - return False - - def writable(self): - return True - - def seekable(self): - return False - - def __getattr__(self, name): - return getattr(self._stderr, name) - - -_LOG_DIR = Path(__file__).resolve().parent.parent / "logs" -_LOG_DIR.mkdir(parents=True, exist_ok=True) - -_real_stdout = sys.stdout - -# Windows pipe buffers are small. When using stdio MCP transport, -# the parent process only reads stdout for MCP messages and does NOT -# drain stderr. Heavy log/print output during execute_task fills the stderr -# pipe buffer, blocking this process on write() → deadlock → timeout. -# Redirect stderr to a log file on Windows to prevent this. -_stderr_file = None -if os.name == "nt": - _stderr_file = open(_LOG_DIR / "mcp_stderr.log", "a", encoding="utf-8", buffering=1) - sys.stderr = _stderr_file - -sys.stdout = _MCPSafeStdout(_real_stdout, sys.stderr) - -# Set up file handler for logging -_log_file_handler = logging.FileHandler(_LOG_DIR / "mcp_server.log") -logging.basicConfig( - level=logging.INFO, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - handlers=[_log_file_handler], +# Re-export for backward compatibility +from openspace.mcp.server import ( # noqa: F401 + _MCPSafeStdout, + _cleanup_file_handles, + create_mcp_app, + run_mcp_server, ) -logger = logging.getLogger("openspace.mcp_server") - - -def _cleanup_file_handles(): - """Clean up file handles on exit.""" - global _stderr_file, _log_file_handler - if _stderr_file: - _stderr_file.close() - _stderr_file = None - if _log_file_handler: - _log_file_handler.close() - - -# Register cleanup handler -atexit.register(_cleanup_file_handles) - -from mcp.server.fastmcp import FastMCP - -_fastmcp_kwargs: dict = {} -try: - if "description" in inspect.signature(FastMCP.__init__).parameters: - _fastmcp_kwargs["description"] = "OpenSpace: Unite the Agents. Evolve the Mind. Rebuild the World." -except (TypeError, ValueError): - pass - -mcp = FastMCP("OpenSpace", **_fastmcp_kwargs) - -# Register tool handlers from extracted module (Epic 4.7) -from openspace.mcp.tool_handlers import register_handlers -register_handlers(mcp) +# Lazy module-level mcp instance for backward compat. +# Some code imports mcp_server.mcp directly. Created on first access +# to avoid wasting a FastMCP instance when only run_mcp_server() is needed. +_mcp = None -def run_mcp_server() -> None: - """Console-script entry point for ``openspace-mcp``. +def _get_mcp(): + global _mcp + if _mcp is None: + _mcp = create_mcp_app() + return _mcp - For HTTP transports (SSE, streamable-http), bearer token auth is - REQUIRED. Set OPENSPACE_MCP_BEARER_TOKEN in the environment. - The server refuses to start without it (fail-closed). - For stdio transport, auth is not applicable (local process IPC). - """ - import argparse +class _LazyMcpProxy: + """Proxy that creates the FastMCP instance on first attribute access.""" - import uvicorn - - from openspace.auth.bearer import ( - BEARER_TOKEN_ENV, - BearerTokenMiddleware, - get_bearer_token, - validate_token_strength, - ) - from openspace.auth.rate_limit import RateLimitMiddleware - - parser = argparse.ArgumentParser(description="OpenSpace MCP Server") - parser.add_argument( - "--transport", - choices=["stdio", "sse", "streamable-http"], - default="stdio", - ) - parser.add_argument("--port", type=int, default=8080) - parser.add_argument("--host", type=str, default="127.0.0.1") - args = parser.parse_args() - - if args.transport == "stdio": - mcp.run(transport="stdio") - return - - # --- HTTP transports: enforce bearer token auth (fail-closed) --- - token = get_bearer_token() - if not token: - logger.critical( - "FAIL-CLOSED: %s not set. Refusing to start %s transport " - "without authentication. Set the environment variable or " - "use --transport stdio for local-only access.", - BEARER_TOKEN_ENV, - args.transport, - ) - sys.exit(1) - - token_ok, reason = validate_token_strength(token) - if not token_ok: - logger.critical("FAIL-CLOSED: %s — %s", BEARER_TOKEN_ENV, reason) - sys.exit(1) - - if args.transport == "sse": - starlette_app = mcp.sse_app() - else: - starlette_app = mcp.streamable_http_app() - - # Middleware chain: request → BearerAuth → RateLimit → MCP app - # Auth is outermost: unauthenticated floods are rejected immediately - # (cheap hmac check) before any rate-limit state is created. - # This prevents memory DoS via fake tokens from unauthenticated requests. - rate_limited_app = RateLimitMiddleware(starlette_app) - protected_app = BearerTokenMiddleware(rate_limited_app, token) - - logger.info( - "Starting MCP server with bearer auth + rate limiting on %s:%d (%s transport)", - args.host, - args.port, - args.transport, - ) - - config = uvicorn.Config( - protected_app, - host=args.host, - port=args.port, - log_level="info", - ) - server = uvicorn.Server(config) - import anyio + def __getattr__(self, name): + return getattr(_get_mcp(), name) - anyio.run(server.serve) +mcp = _LazyMcpProxy() if __name__ == "__main__": - run_mcp_server() + run_mcp_server(_get_mcp()) diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py new file mode 100644 index 00000000..c59507ad --- /dev/null +++ b/tests/test_mcp_server.py @@ -0,0 +1,150 @@ +"""Tests for openspace.mcp.server — Epic 4.9 extraction.""" +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +try: + from openspace.mcp.server import ( + _MCPSafeStdout, + _cleanup_file_handles, + create_mcp_app, + ) + + _HAS_MODULE = True +except ImportError: + _HAS_MODULE = False + +pytestmark = pytest.mark.skipif(not _HAS_MODULE, reason="openspace.mcp.server not importable") + + +# --------------------------------------------------------------------------- +# _MCPSafeStdout +# --------------------------------------------------------------------------- +class TestMCPSafeStdout: + """Verify stdout wrapper routes text→stderr, binary→real stdout.""" + + def test_write_goes_to_stderr(self): + real = MagicMock() + err = MagicMock() + wrapper = _MCPSafeStdout(real, err) + wrapper.write("hello") + err.write.assert_called_once_with("hello") + real.write.assert_not_called() + + def test_buffer_returns_real_stdout_buffer(self): + real = MagicMock() + err = MagicMock() + wrapper = _MCPSafeStdout(real, err) + assert wrapper.buffer is real.buffer + + def test_fileno_returns_real_fileno(self): + real = MagicMock() + real.fileno.return_value = 1 + err = MagicMock() + wrapper = _MCPSafeStdout(real, err) + assert wrapper.fileno() == 1 + + def test_flush_flushes_both(self): + real = MagicMock() + err = MagicMock() + wrapper = _MCPSafeStdout(real, err) + wrapper.flush() + err.flush.assert_called_once() + real.flush.assert_called_once() + + def test_writelines_goes_to_stderr(self): + real = MagicMock() + err = MagicMock() + wrapper = _MCPSafeStdout(real, err) + wrapper.writelines(["a", "b"]) + err.writelines.assert_called_once_with(["a", "b"]) + + def test_not_readable(self): + wrapper = _MCPSafeStdout(MagicMock(), MagicMock()) + assert wrapper.readable() is False + + def test_is_writable(self): + wrapper = _MCPSafeStdout(MagicMock(), MagicMock()) + assert wrapper.writable() is True + + def test_not_seekable(self): + wrapper = _MCPSafeStdout(MagicMock(), MagicMock()) + assert wrapper.seekable() is False + + def test_encoding_from_stderr(self): + err = MagicMock() + err.encoding = "utf-8" + wrapper = _MCPSafeStdout(MagicMock(), err) + assert wrapper.encoding == "utf-8" + + def test_getattr_delegates_to_stderr(self): + err = MagicMock() + err.some_method.return_value = 42 + wrapper = _MCPSafeStdout(MagicMock(), err) + assert wrapper.some_method() == 42 + + +# --------------------------------------------------------------------------- +# create_mcp_app +# --------------------------------------------------------------------------- +class TestCreateMcpApp: + """Verify factory returns a FastMCP instance with handlers wired.""" + + def test_returns_fastmcp_instance(self): + app = create_mcp_app() + # FastMCP should have a name attribute + assert hasattr(app, "name") or hasattr(app, "_tool_manager") + + def test_tools_registered(self): + app = create_mcp_app() + # Verify all 4 MCP tools are actually wired + if hasattr(app, "_tool_manager") and hasattr(app._tool_manager, "_tools"): + tool_names = set(app._tool_manager._tools.keys()) + assert tool_names == {"execute_task", "search_skills", "fix_skill", "upload_skill"}, ( + f"Expected 4 tools, got: {tool_names}" + ) + else: + # Fallback: at least verify create_mcp_app didn't silently fail + assert app is not None + + def test_idempotent_calls(self): + """Multiple calls should each return independent instances.""" + app1 = create_mcp_app() + app2 = create_mcp_app() + assert app1 is not app2 + + +# --------------------------------------------------------------------------- +# _cleanup_file_handles +# --------------------------------------------------------------------------- +class TestCleanupFileHandles: + """Verify cleanup properly closes handles.""" + + def test_cleanup_closes_stderr_file(self): + import openspace.mcp.server as srv + mock_file = MagicMock() + original = srv._stderr_file + try: + srv._stderr_file = mock_file + _cleanup_file_handles() + mock_file.close.assert_called_once() + assert srv._stderr_file is None + finally: + srv._stderr_file = original + + def test_cleanup_noop_when_none(self): + """Should not raise when handles are already None.""" + import openspace.mcp.server as srv + original_stderr = srv._stderr_file + original_handler = srv._log_file_handler + try: + srv._stderr_file = None + srv._log_file_handler = None + _cleanup_file_handles() # should not raise + finally: + srv._stderr_file = original_stderr + srv._log_file_handler = original_handler