diff --git a/docs/cli-reference.md b/docs/cli-reference.md index ff51da75e..6f8790201 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -627,7 +627,7 @@ ouroboros mcp serve [OPTIONS] |--------|-------------| | `-h, --host TEXT` | Host to bind to (default: localhost) | | `-p, --port INTEGER` | Port to bind to (default: 8080) | -| `-t, --transport TEXT` | Transport type: `stdio` or `sse` (default: stdio). Note: `http` and `streamable-http` are supported as *client* transports for bridging upstream MCP servers via `mcp_servers.yaml`, not as serve transports. | +| `-t, --transport TEXT` | Transport type: `stdio`, `sse`, or `streamable-http` (default: stdio). Note: `http` is only a client config alias for outbound MCP connections and is NOT a valid serve transport. | | `--db TEXT` | Path to the EventStore database file | | `--runtime TEXT` | Agent runtime backend for orchestrator-driven tools (`claude`, `codex`, `opencode`). Affects which tool variants are instantiated | | `--llm-backend TEXT` | LLM backend for interview/seed/evaluation tools (`claude_code`, `litellm`, `codex`, `opencode`). Affects which tool variants are instantiated | @@ -641,6 +641,9 @@ ouroboros mcp serve # Start with SSE transport on custom port ouroboros mcp serve --transport sse --port 9000 +# Start with streamable HTTP transport on custom port +ouroboros mcp serve --transport streamable-http --port 9000 + # Start with Codex-backed orchestrator tools ouroboros mcp serve --runtime codex --llm-backend codex @@ -648,9 +651,13 @@ ouroboros mcp serve --runtime codex --llm-backend codex ouroboros mcp serve --host 0.0.0.0 --port 8080 --transport sse ``` +For serving with streamable HTTP, use `streamable-http`, not `http`. `http` is accepted only in MCP client configuration as a compatibility alias for dialing another server's streamable HTTP endpoint; `mcp serve` uses the precise protocol name so users do not confuse it with a generic HTTP API. Streamable HTTP clients should connect to `http://:/mcp`. + +FastMCP caveats: Network serving uses the MCP SDK's FastMCP server. The streamable HTTP path is FastMCP's default `/mcp`. Authentication and rate limiting configured on `MCPServerAdapter` are rejected for FastMCP transports because FastMCP does not pass credentials or stable client identity to handlers; protect `0.0.0.0` binds with normal network controls. + **Startup behavior:** -On startup, `mcp serve` automatically cancels any sessions left in `RUNNING` or `PAUSED` state for more than 1 hour. These are treated as orphaned from a previous crash. Cancelled sessions are reported on stderr (or console when using SSE transport). This cleanup is best-effort and does not prevent the server from starting if it fails. +On startup, `mcp serve` automatically cancels any sessions left in `RUNNING` or `PAUSED` state for more than 1 hour. These are treated as orphaned from a previous crash. Cancelled sessions are reported on stderr for `stdio` and on the console for network transports (`sse`, `streamable-http`). This cleanup is best-effort and does not prevent the server from starting if it fails. **Claude Desktop / Claude Code CLI Integration:** diff --git a/src/ouroboros/cli/commands/mcp.py b/src/ouroboros/cli/commands/mcp.py index b2c546b69..02e28adfd 100644 --- a/src/ouroboros/cli/commands/mcp.py +++ b/src/ouroboros/cli/commands/mcp.py @@ -185,7 +185,7 @@ async def _run_mcp_server( Args: host: Host to bind to. port: Port to bind to. - transport: Transport type (stdio or sse). + transport: Transport type (stdio, sse, or streamable-http). db_path: Optional path to EventStore database. runtime_backend: Optional orchestrator runtime backend override. llm_backend: Optional LLM-only backend override. @@ -202,10 +202,13 @@ async def _run_mcp_server( transport = validate_transport(transport) except ValueError: _stderr_console.print( - f"[red]Invalid transport {transport!r}. Must be 'stdio' or 'sse'.[/red]" + "[red]Invalid transport " + f"{transport!r}. Must be 'stdio', 'sse', or 'streamable-http'.[/red]" ) raise typer.Exit(code=1) + _console_out = _stderr_console if transport == "stdio" else Console() + # Create EventStore with custom path if provided if db_path: event_store = EventStore(f"sqlite+aiosqlite:///{db_path}") @@ -222,7 +225,7 @@ async def _run_mcp_server( await event_store.initialize() except Exception as e: # Auto-cleanup is best-effort — don't prevent server from starting - _stderr_console.print(f"[yellow]Warning: auto-cleanup failed: {e}[/yellow]") + _console_out.print(f"[yellow]Warning: auto-cleanup failed: {e}[/yellow]") else: repo = SessionRepository(event_store) @@ -230,12 +233,12 @@ async def _run_startup_cleanup() -> None: try: cancelled = await repo.cancel_orphaned_sessions() if cancelled: - _stderr_console.print( + _console_out.print( f"[yellow]Auto-cancelled {len(cancelled)} orphaned session(s)[/yellow]" ) except Exception as e: # Auto-cleanup is best-effort — don't prevent server startup - _stderr_console.print(f"[yellow]Warning: auto-cleanup failed: {e}[/yellow]") + _console_out.print(f"[yellow]Warning: auto-cleanup failed: {e}[/yellow]") cleanup_task = asyncio.create_task( _run_startup_cleanup(), @@ -250,11 +253,11 @@ async def _run_startup_cleanup() -> None: try: results = await mcp_bridge.connect() connected = sum(1 for r in results.values() if r.is_ok) - _stderr_console.print( + _console_out.print( f"[blue]MCP Bridge: {connected}/{len(results)} upstream server(s) connected[/blue]" ) except Exception as e: - _stderr_console.print(f"[yellow]MCP Bridge connection failed: {e}[/yellow]") + _console_out.print(f"[yellow]MCP Bridge connection failed: {e}[/yellow]") mcp_bridge = None # Create server with all tools pre-registered via dependency injection. @@ -273,7 +276,6 @@ async def _run_startup_cleanup() -> None: # Detect Codex seatbelt sandbox and warn about network restrictions. _sandbox_network_disabled = os.environ.get("CODEX_SANDBOX_NETWORK_DISABLED") == "1" - _console_out = _stderr_console if transport == "stdio" else Console() if transport == "stdio": # In stdio mode, stdout is the JSON-RPC channel. @@ -285,7 +287,10 @@ async def _run_startup_cleanup() -> None: else: print_success(f"MCP Server starting on {transport}...") print_info(f"Registered {tool_count} tools") - print_info(f"Listening on {host}:{port}") + if transport == "streamable-http": + print_info(f"Listening on http://{host}:{port}/mcp") + else: + print_info(f"Listening on {host}:{port}") print_info("Press Ctrl+C to stop") if _sandbox_network_disabled: @@ -341,7 +346,7 @@ def serve( typer.Option( "--transport", "-t", - help="Transport type: stdio or sse.", + help="Transport type: stdio, sse, or streamable-http.", ), ] = "stdio", db: Annotated[ @@ -389,6 +394,9 @@ def serve( # Start with SSE transport on custom port ouroboros mcp serve --transport sse --port 9000 + # Start with streamable HTTP transport for Codex CLI --url clients + ouroboros mcp serve --transport streamable-http --port 9000 + # Start with OpenCode runtime ouroboros mcp serve --runtime opencode diff --git a/src/ouroboros/mcp/server/adapter.py b/src/ouroboros/mcp/server/adapter.py index 11f3fc3a4..f4635b2dc 100644 --- a/src/ouroboros/mcp/server/adapter.py +++ b/src/ouroboros/mcp/server/adapter.py @@ -38,7 +38,7 @@ log = structlog.get_logger(__name__) -VALID_TRANSPORTS: frozenset[str] = frozenset({"stdio", "sse"}) +VALID_TRANSPORTS: frozenset[str] = frozenset({"stdio", "sse", "streamable-http"}) def _safe_cwd() -> Path: @@ -66,7 +66,7 @@ def validate_transport(transport: str) -> str: Returns the lowercased transport if valid, raises ValueError otherwise. """ - transport = transport.lower() + transport = transport.lower().replace("_", "-") if transport not in VALID_TRANSPORTS: msg = f"Invalid transport {transport!r}. Must be one of: {', '.join(sorted(VALID_TRANSPORTS))}" raise ValueError(msg) @@ -554,9 +554,10 @@ async def serve( Uses the MCP SDK's FastMCP server implementation. Args: - transport: Transport type - "stdio" or "sse" (case-insensitive). - host: Host to bind to (SSE only). Defaults to "localhost". - port: Port to bind to (SSE only). Defaults to 8080. + transport: Transport type - "stdio", "sse", or "streamable-http" + (case-insensitive). + host: Host to bind to for network transports. Defaults to "localhost". + port: Port to bind to for network transports. Defaults to 8080. Raises: ValueError: If transport is invalid or incompatible with security config. @@ -586,9 +587,10 @@ async def serve( msg = "mcp package not installed. Install with: pip install 'ouroboros-ai[mcp]'" raise ImportError(msg) from e - # Pass host/port at construction time — FastMCP reads these from - # its internal settings, so run_sse_async() alone won't pick them up. - if transport == "sse": + # Pass host/port at construction time for network transports — FastMCP + # reads these from its internal settings, so the run_* method alone + # won't pick them up. + if transport in {"sse", "streamable-http"}: self._mcp_server = FastMCP( self._name, host=host, @@ -690,6 +692,8 @@ async def resource_wrapper() -> str: # Run the server with the appropriate transport if transport == "sse": await self._mcp_server.run_sse_async() + elif transport == "streamable-http": + await self._mcp_server.run_streamable_http_async() else: await self._mcp_server.run_stdio_async() diff --git a/tests/unit/cli/test_mcp_nested_guard.py b/tests/unit/cli/test_mcp_nested_guard.py index be4d8848c..e7d6d05cb 100644 --- a/tests/unit/cli/test_mcp_nested_guard.py +++ b/tests/unit/cli/test_mcp_nested_guard.py @@ -53,3 +53,26 @@ async def mock_run_mcp_server(*args, **kwargs): # _OUROBOROS_NESTED should have been set to "1" before asyncio.run was called assert captured_env.get("_OUROBOROS_NESTED") == "1" + + +def test_serve_defaults_to_port_8080_when_port_omitted(monkeypatch): + """mcp serve should pass port 8080 when --port is omitted.""" + monkeypatch.delenv("_OUROBOROS_NESTED", raising=False) + + mock_run_mcp_server = AsyncMock() + + with patch( + "ouroboros.cli.commands.mcp._run_mcp_server", + new=mock_run_mcp_server, + ): + result = runner.invoke(app, ["serve", "--transport", "streamable-http"]) + + assert result.exit_code == 0 + mock_run_mcp_server.assert_awaited_once_with( + "localhost", + 8080, + "streamable-http", + None, + None, + None, + ) diff --git a/tests/unit/cli/test_mcp_startup_cleanup.py b/tests/unit/cli/test_mcp_startup_cleanup.py index fd10a55a4..3c26e6f32 100644 --- a/tests/unit/cli/test_mcp_startup_cleanup.py +++ b/tests/unit/cli/test_mcp_startup_cleanup.py @@ -196,6 +196,67 @@ async def serve_side_effect(*args, **kwargs) -> None: mock_console.print.assert_any_call("[yellow]Auto-cancelled 2 orphaned session(s)[/yellow]") mock_server.serve.assert_called_once() + @pytest.mark.asyncio + async def test_streamable_http_advertises_endpoint_and_uses_stdout_startup(self, capfd) -> None: + """Streamable HTTP startup output advertises the client endpoint.""" + orphaned_tracker = _make_tracker( + session_id="orch_orphan_http", + execution_id="exec_orphan_http", + status=SessionStatus.RUNNING, + ) + cleanup_called = asyncio.Event() + + mock_es = AsyncMock() + mock_es.initialize = AsyncMock() + mock_repo = AsyncMock() + + async def cancel_orphans() -> list[SessionTracker]: + cleanup_called.set() + return [orphaned_tracker] + + mock_repo.cancel_orphaned_sessions = AsyncMock(side_effect=cancel_orphans) + + mock_server = MagicMock() + mock_server.info.tools = [] + + async def serve_side_effect(*args, **kwargs) -> None: + await cleanup_called.wait() + await asyncio.sleep(0.01) + + mock_server.serve = AsyncMock(side_effect=serve_side_effect) + + with ( + patch("ouroboros.cli.commands.mcp._ensure_shell_env", lambda **_: None), + patch( + "ouroboros.persistence.event_store.EventStore", + return_value=mock_es, + ), + patch( + "ouroboros.orchestrator.session.SessionRepository", + return_value=mock_repo, + ), + patch( + "ouroboros.mcp.server.adapter.create_ouroboros_server", + return_value=mock_server, + ), + patch("ouroboros.mcp.bridge.create_bridge_from_env", return_value=None), + ): + from ouroboros.cli.commands.mcp import _run_mcp_server + + capfd.readouterr() + await _run_mcp_server("127.0.0.1", 9100, "streamable-http") + + captured = capfd.readouterr() + assert "http://127.0.0.1:9100/mcp" in captured.out + assert "Auto-cancelled 1 orphaned session(s)" in captured.out + assert "http://127.0.0.1:9100/mcp" not in captured.err + assert "Auto-cancelled 1 orphaned session(s)" not in captured.err + mock_server.serve.assert_awaited_once_with( + transport="streamable-http", + host="127.0.0.1", + port=9100, + ) + @pytest.mark.asyncio async def test_pending_background_cleanup_is_cancelled_on_shutdown(self) -> None: """Server shutdown should cancel an unfinished startup cleanup task.""" diff --git a/tests/unit/cli/test_mcp_validate_transport_stderr.py b/tests/unit/cli/test_mcp_validate_transport_stderr.py index c0f6bfc06..3171b31a1 100644 --- a/tests/unit/cli/test_mcp_validate_transport_stderr.py +++ b/tests/unit/cli/test_mcp_validate_transport_stderr.py @@ -32,7 +32,15 @@ def test_validate_transport_rejects_invalid(bad_transport: str) -> None: @pytest.mark.parametrize( "good_transport,expected", - [("stdio", "stdio"), ("sse", "sse"), ("STDIO", "stdio"), ("SSE", "sse")], + [ + ("stdio", "stdio"), + ("sse", "sse"), + ("streamable-http", "streamable-http"), + ("streamable_http", "streamable-http"), + ("STDIO", "stdio"), + ("SSE", "sse"), + ("STREAMABLE-HTTP", "streamable-http"), + ], ) def test_validate_transport_accepts_valid(good_transport: str, expected: str) -> None: """validate_transport must accept and lowercase known transports.""" diff --git a/tests/unit/mcp/server/test_adapter.py b/tests/unit/mcp/server/test_adapter.py index 4191a4fe1..a85e80261 100644 --- a/tests/unit/mcp/server/test_adapter.py +++ b/tests/unit/mcp/server/test_adapter.py @@ -315,11 +315,14 @@ class TestValidateTransport: def test_valid_lowercase(self): assert validate_transport("stdio") == "stdio" assert validate_transport("sse") == "sse" + assert validate_transport("streamable-http") == "streamable-http" def test_case_insensitive(self): assert validate_transport("SSE") == "sse" assert validate_transport("Stdio") == "stdio" assert validate_transport("sSe") == "sse" + assert validate_transport("STREAMABLE-HTTP") == "streamable-http" + assert validate_transport("streamable_http") == "streamable-http" def test_invalid_raises(self): with pytest.raises(ValueError, match="Invalid transport"): @@ -332,6 +335,7 @@ def test_empty_raises(self): def test_valid_transports_constant(self): assert "stdio" in VALID_TRANSPORTS assert "sse" in VALID_TRANSPORTS + assert "streamable-http" in VALID_TRANSPORTS class TestServeTransport: @@ -404,6 +408,67 @@ async def test_sse_ephemeral_port_zero(self): assert mock_fastmcp_cls.call_args.kwargs["port"] == 0 + @pytest.mark.asyncio + async def test_streamable_http_passes_host_port_to_fastmcp(self): + """Verify host/port are forwarded to FastMCP for streamable HTTP.""" + from unittest.mock import MagicMock, patch + + mock_fastmcp_cls = MagicMock() + mock_instance = MagicMock() + mock_instance.tool = MagicMock(return_value=lambda f: f) + mock_instance.resource = MagicMock(return_value=lambda f: f) + mock_instance.run_streamable_http_async = AsyncMock() + mock_fastmcp_cls.return_value = mock_instance + + adapter = MCPServerAdapter() + + with ( + patch( + "ouroboros.mcp.server.adapter.FastMCP", + mock_fastmcp_cls, + create=True, + ), + patch.dict( + "sys.modules", + {"mcp.server.fastmcp": MagicMock(FastMCP=mock_fastmcp_cls)}, + ), + ): + await adapter.serve(transport="streamable-http", host="127.0.0.1", port=9100) + + mock_fastmcp_cls.assert_called_once() + call_kwargs = mock_fastmcp_cls.call_args + assert call_kwargs.kwargs["host"] == "127.0.0.1" + assert call_kwargs.kwargs["port"] == 9100 + mock_instance.run_streamable_http_async.assert_awaited_once() + + @pytest.mark.asyncio + async def test_streamable_http_real_fastmcp_exposes_mcp_path(self) -> None: + """Real FastMCP streamable HTTP serving exposes the advertised /mcp path.""" + from unittest.mock import patch + + pytest.importorskip("mcp.server.fastmcp") + pytest.importorskip("uvicorn") + + served = SimpleNamespace(config=None) + + async def capture_serve(server, *args, **kwargs) -> None: + served.config = server.config + + adapter = MCPServerAdapter() + + with patch("uvicorn.Server.serve", new=capture_serve): + await adapter.serve(transport="streamable-http", host="127.0.0.1", port=9100) + + assert served.config is not None + assert served.config.host == "127.0.0.1" + assert served.config.port == 9100 + + fastmcp = adapter._mcp_server + assert fastmcp.settings.streamable_http_path == "/mcp" + + route_paths = {getattr(route, "path", None) for route in served.config.app.routes} + assert "/mcp" in route_paths + @pytest.mark.asyncio async def test_fastmcp_path_enforces_security(self): """FastMCP tool wrapper routes through call_tool to enforce security checks."""