Skip to content

Commit e1e2165

Browse files
Brian KrafftCopilot
andcommitted
feat: Epic 6.4 - DX polish (CLI help, preflight checks, error UX)
- Extract _build_arg_parser() with rich --help: epilog with examples, env var docs, --version flag, RawDescriptionHelpFormatter - Add openspace/deploy/preflight.py: PreflightIssue, check_python_version, check_bearer_token, check_skill_store, check_port_available, format_report - Wire preflight_check() into run_mcp_server() before server start - Preflight catches: missing bearer token, bad skill store, port in use - All errors include actionable suggestions (commands to run) - 28 new DX tests, 1953 total passed - Bump server.py size guard from 310 to 410 (CLI help + preflight growth) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c10818e commit e1e2165

4 files changed

Lines changed: 545 additions & 12 deletions

File tree

openspace/deploy/preflight.py

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
"""Pre-flight environment checks for OpenSpace.
2+
3+
Validates the runtime environment before server startup and provides
4+
actionable error messages with suggested fixes. Designed for DX — a
5+
developer who misconfigures something should know exactly what to do.
6+
7+
Usage::
8+
9+
from openspace.deploy.preflight import preflight_check
10+
11+
issues = preflight_check(transport="streamable-http")
12+
if issues:
13+
for issue in issues:
14+
print(f" ✗ {issue}")
15+
sys.exit(1)
16+
"""
17+
18+
from __future__ import annotations
19+
20+
import logging
21+
import os
22+
import sys
23+
from dataclasses import dataclass, field
24+
from pathlib import Path
25+
from typing import List, Optional
26+
27+
logger = logging.getLogger(__name__)
28+
29+
30+
@dataclass
31+
class PreflightIssue:
32+
"""A single pre-flight check failure with actionable fix."""
33+
34+
check: str
35+
message: str
36+
suggestion: str
37+
severity: str = "error" # error | warning
38+
39+
def __str__(self) -> str:
40+
icon = "✗" if self.severity == "error" else "⚠"
41+
return f"{icon} [{self.check}] {self.message}\n{self.suggestion}"
42+
43+
44+
def check_python_version(minimum: tuple = (3, 11)) -> Optional[PreflightIssue]:
45+
"""Verify Python version meets minimum requirement."""
46+
if sys.version_info < minimum:
47+
return PreflightIssue(
48+
check="python-version",
49+
message=f"Python {minimum[0]}.{minimum[1]}+ required, got {sys.version_info.major}.{sys.version_info.minor}",
50+
suggestion=f"Install Python {minimum[0]}.{minimum[1]}+ from https://python.org/downloads/",
51+
)
52+
return None
53+
54+
55+
def check_bearer_token(transport: str) -> Optional[PreflightIssue]:
56+
"""Verify bearer token is set for HTTP transports."""
57+
if transport == "stdio":
58+
return None # No auth needed for stdio
59+
60+
token_env = "OPENSPACE_MCP_BEARER_TOKEN"
61+
token = os.environ.get(token_env, "").strip()
62+
if not token:
63+
return PreflightIssue(
64+
check="bearer-token",
65+
message=f"{token_env} not set (required for {transport} transport)",
66+
suggestion=(
67+
f"Set {token_env} to a strong random token:\n"
68+
f" export {token_env}=$(python -c \"import secrets; print(secrets.token_urlsafe(32))\")\n"
69+
f" Or use --transport stdio for local-only access (no auth required)"
70+
),
71+
)
72+
return None
73+
74+
75+
def check_skill_store(path: str) -> Optional[PreflightIssue]:
76+
"""Verify skill store directory exists or can be created."""
77+
p = Path(path)
78+
if p.exists() and not p.is_dir():
79+
return PreflightIssue(
80+
check="skill-store",
81+
message=f"Skill store path exists but is not a directory: {path}",
82+
suggestion=f"Remove the file and create directory:\n rm {path} && mkdir -p {path}",
83+
)
84+
return None
85+
86+
87+
def check_port_available(port: int, host: str = "0.0.0.0") -> Optional[PreflightIssue]:
88+
"""Check if the target port is likely available (best-effort)."""
89+
import socket
90+
91+
try:
92+
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
93+
s.settimeout(1)
94+
s.bind((host if host != "0.0.0.0" else "127.0.0.1", port))
95+
except OSError:
96+
return PreflightIssue(
97+
check="port-available",
98+
message=f"Port {port} appears to be in use on {host}",
99+
suggestion=(
100+
f"Either stop the other process or use a different port:\n"
101+
f" openspace-mcp --port {port + 1}\n"
102+
f" Or set OPENSPACE_MCP_PORT={port + 1}"
103+
),
104+
severity="warning",
105+
)
106+
return None
107+
108+
109+
def preflight_check(
110+
transport: str = "stdio",
111+
port: int = 8000,
112+
host: str = "0.0.0.0",
113+
skill_store_path: str = "skills/",
114+
check_port: bool = True,
115+
) -> List[PreflightIssue]:
116+
"""Run all pre-flight checks and return any issues found.
117+
118+
Returns an empty list if all checks pass.
119+
120+
Args:
121+
transport: MCP transport type (stdio, sse, streamable-http)
122+
port: Server port (only checked for HTTP transports)
123+
host: Server bind address
124+
skill_store_path: Path to skill store directory
125+
check_port: Whether to check port availability
126+
"""
127+
issues: List[PreflightIssue] = []
128+
129+
result = check_python_version()
130+
if result:
131+
issues.append(result)
132+
133+
result = check_bearer_token(transport)
134+
if result:
135+
issues.append(result)
136+
137+
result = check_skill_store(skill_store_path)
138+
if result:
139+
issues.append(result)
140+
141+
if transport != "stdio" and check_port:
142+
result = check_port_available(port, host)
143+
if result:
144+
issues.append(result)
145+
146+
return issues
147+
148+
149+
def format_preflight_report(issues: List[PreflightIssue]) -> str:
150+
"""Format preflight issues into a readable report."""
151+
errors = [i for i in issues if i.severity == "error"]
152+
warnings = [i for i in issues if i.severity == "warning"]
153+
154+
lines = ["\n╔══ OpenSpace Pre-flight Check ══╗\n"]
155+
156+
if errors:
157+
lines.append(f" {len(errors)} error(s) found:\n")
158+
for issue in errors:
159+
lines.append(f" {issue}\n")
160+
161+
if warnings:
162+
lines.append(f" {len(warnings)} warning(s):\n")
163+
for issue in warnings:
164+
lines.append(f" {issue}\n")
165+
166+
if not errors:
167+
lines.append(" ✓ All checks passed\n")
168+
169+
lines.append("╚═══════════════════════════════╝")
170+
return "\n".join(lines)

openspace/mcp/server.py

Lines changed: 99 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,84 @@ def _probe_mcp_tools() -> HealthProbe:
183183

184184

185185
# ---------------------------------------------------------------------------
186-
# 4. Server entry point
186+
# 4. CLI argument parser (extracted for testability)
187+
# ---------------------------------------------------------------------------
188+
_VERSION = "0.1.0" # TODO: read from pyproject.toml or importlib.metadata
189+
190+
_EPILOG = """\
191+
examples:
192+
openspace-mcp # stdio transport (local, no auth)
193+
openspace-mcp --transport streamable-http # HTTP (requires bearer token)
194+
openspace-mcp --transport sse --port 9000 # SSE on custom port
195+
196+
environment variables:
197+
OPENSPACE_MCP_HOST Bind address (default: 0.0.0.0)
198+
OPENSPACE_MCP_PORT Port number (default: 8000)
199+
OPENSPACE_MCP_TRANSPORT Transport: stdio|sse|streamable-http
200+
OPENSPACE_MCP_BEARER_TOKEN Auth token (REQUIRED for HTTP transports)
201+
OPENSPACE_LOG_LEVEL Logging: DEBUG|INFO|WARNING|ERROR
202+
OPENSPACE_SHUTDOWN_TIMEOUT Graceful shutdown seconds (default: 30)
203+
OPENSPACE_METRICS_ENABLED Prometheus metrics: true|false
204+
205+
notes:
206+
HTTP transports (sse, streamable-http) require OPENSPACE_MCP_BEARER_TOKEN.
207+
Use --transport stdio for local development without authentication.
208+
"""
209+
210+
211+
def _build_arg_parser(deploy_cfg=None):
212+
"""Build the CLI argument parser with rich help text.
213+
214+
Extracted from run_mcp_server for testability. Returns an
215+
argparse.ArgumentParser ready for parse_args().
216+
"""
217+
import argparse
218+
219+
defaults = {
220+
"transport": "stdio",
221+
"port": 8000,
222+
"host": "0.0.0.0",
223+
}
224+
if deploy_cfg is not None:
225+
defaults["transport"] = deploy_cfg.mcp_transport
226+
defaults["port"] = deploy_cfg.mcp_port
227+
defaults["host"] = deploy_cfg.mcp_host
228+
229+
parser = argparse.ArgumentParser(
230+
prog="openspace-mcp",
231+
description="OpenSpace MCP Server — AI agent framework with tool execution",
232+
epilog=_EPILOG,
233+
formatter_class=argparse.RawDescriptionHelpFormatter,
234+
)
235+
parser.add_argument(
236+
"--version",
237+
action="version",
238+
version=f"openspace-mcp {_VERSION}",
239+
)
240+
parser.add_argument(
241+
"--transport",
242+
choices=["stdio", "sse", "streamable-http"],
243+
default=defaults["transport"],
244+
help="MCP transport protocol (default: %(default)s). "
245+
"HTTP transports require OPENSPACE_MCP_BEARER_TOKEN.",
246+
)
247+
parser.add_argument(
248+
"--port",
249+
type=int,
250+
default=defaults["port"],
251+
help="Server port for HTTP transports (default: %(default)s)",
252+
)
253+
parser.add_argument(
254+
"--host",
255+
type=str,
256+
default=defaults["host"],
257+
help="Bind address for HTTP transports (default: %(default)s)",
258+
)
259+
return parser
260+
261+
262+
# ---------------------------------------------------------------------------
263+
# 5. Server entry point
187264
# ---------------------------------------------------------------------------
188265
def run_mcp_server(mcp=None) -> None:
189266
"""Console-script entry point for ``openspace-mcp``.
@@ -195,8 +272,6 @@ def run_mcp_server(mcp=None) -> None:
195272
mcp: Optional pre-created FastMCP instance. If None, creates one
196273
via ``create_mcp_app()``.
197274
"""
198-
import argparse
199-
200275
import uvicorn
201276

202277
from openspace.auth.bearer import (
@@ -214,16 +289,29 @@ def run_mcp_server(mcp=None) -> None:
214289
# Load config from environment, then allow CLI overrides
215290
deploy_cfg = DeployConfig.from_env()
216291

217-
parser = argparse.ArgumentParser(description="OpenSpace MCP Server")
218-
parser.add_argument(
219-
"--transport",
220-
choices=["stdio", "sse", "streamable-http"],
221-
default=deploy_cfg.mcp_transport,
222-
)
223-
parser.add_argument("--port", type=int, default=deploy_cfg.mcp_port)
224-
parser.add_argument("--host", type=str, default=deploy_cfg.mcp_host)
292+
parser = _build_arg_parser(deploy_cfg)
225293
args = parser.parse_args()
226294

295+
# Pre-flight checks: validate environment before starting
296+
from openspace.deploy.preflight import (
297+
format_preflight_report,
298+
preflight_check,
299+
)
300+
301+
issues = preflight_check(
302+
transport=args.transport,
303+
port=args.port,
304+
host=args.host,
305+
skill_store_path=deploy_cfg.skill_store_path,
306+
check_port=(args.transport != "stdio"),
307+
)
308+
if issues:
309+
errors = [i for i in issues if i.severity == "error"]
310+
report = format_preflight_report(issues)
311+
logger.info(report)
312+
if errors:
313+
sys.exit(1)
314+
227315
if args.transport == "stdio":
228316
mcp.run(transport="stdio")
229317
return

0 commit comments

Comments
 (0)