Epic 6.4: DX Polish - CLI help, preflight checks, error UX

RUNBOOK.yaml

@@ -1,7 +1,7 @@
 schema_version: '1.0'
 lock:
-  active_claim: null
-  claimed_by: null
+  active_claim: '6.4'
+  claimed_by: 2026-04-07-e9b4572b
   claimed_at: '2026-04-07T01:20:00Z'
 review:
   max_rounds: 3
@@ -585,10 +585,10 @@ epics:
 - id: '6.4'
   phase: P6
   title: DX polish (CLI help, errors, onboarding)
-  status: pending
-  branch: null
-  pr: null
-  review_round: 0
+  status: claimed
+  branch: epic/6.4-dx-polish
+  pr: 53
+  review_round: 1
   description: 'Improve CLI help text, error messages with actionable suggestions,
     first-run onboarding flow, quickstart documentation.
 

openspace/deploy/preflight.py

@@ -0,0 +1,182 @@
+"""Pre-flight environment checks for OpenSpace.
+
+Validates the runtime environment before server startup and provides
+actionable error messages with suggested fixes. Designed for DX — a
+developer who misconfigures something should know exactly what to do.
+
+Usage::
+
+    from openspace.deploy.preflight import preflight_check
+
+    issues = preflight_check(transport="streamable-http")
+    if issues:
+        for issue in issues:
+            print(f"  ✗ {issue}")
+        sys.exit(1)
+"""
+
+from __future__ import annotations
+
+import logging
+import os
+import sys
+from dataclasses import dataclass, field
+from pathlib import Path
+from typing import List, Optional
+
+logger = logging.getLogger(__name__)
+
+
+@dataclass
+class PreflightIssue:
+    """A single pre-flight check failure with actionable fix."""
+
+    check: str
+    message: str
+    suggestion: str
+    severity: str = "error"  # error | warning
+
+    def __str__(self) -> str:
+        icon = "✗" if self.severity == "error" else "⚠"
+        return f"{icon} [{self.check}] {self.message}\n  → {self.suggestion}"
+
+
+def check_python_version(minimum: tuple = (3, 11)) -> Optional[PreflightIssue]:
+    """Verify Python version meets minimum requirement."""
+    if sys.version_info < minimum:
+        return PreflightIssue(
+            check="python-version",
+            message=f"Python {minimum[0]}.{minimum[1]}+ required, got {sys.version_info.major}.{sys.version_info.minor}",
+            suggestion=f"Install Python {minimum[0]}.{minimum[1]}+ from https://python.org/downloads/",
+        )
+    return None
+
+
+def check_bearer_token(transport: str) -> Optional[PreflightIssue]:
+    """Verify bearer token is set for HTTP transports."""
+    if transport == "stdio":
+        return None  # No auth needed for stdio
+
+    token_env = "OPENSPACE_MCP_BEARER_TOKEN"
+    token = os.environ.get(token_env, "").strip()
+    if not token:
+        if sys.platform == "win32":
+            set_cmd = (
+                f'$env:{token_env} = python -c "import secrets; print(secrets.token_urlsafe(32))"'
+            )
+        else:
+            set_cmd = (
+                f'export {token_env}=$(python -c "import secrets; print(secrets.token_urlsafe(32))")'
+            )
+        return PreflightIssue(
+            check="bearer-token",
+            message=f"{token_env} not set (required for {transport} transport)",
+            suggestion=(
+                f"Set {token_env} to a strong random token:\n"
+                f"    {set_cmd}\n"
+                f"  Or use --transport stdio for local-only access (no auth required)"
+            ),
+        )
+    return None
+
+
+def check_skill_store(path: str) -> Optional[PreflightIssue]:
+    """Verify skill store directory exists or can be created."""
+    p = Path(path)
+    if p.exists() and not p.is_dir():
+        if sys.platform == "win32":
+            fix_cmd = f"Remove-Item {path}; New-Item -ItemType Directory {path}"
+        else:
+            fix_cmd = f"rm {path} && mkdir -p {path}"
+        return PreflightIssue(
+            check="skill-store",
+            message=f"Skill store path exists but is not a directory: {path}",
+            suggestion=f"Remove the file and create directory:\n    {fix_cmd}",
+        )
+    return None
+
+
+def check_port_available(port: int, host: str = "0.0.0.0") -> Optional[PreflightIssue]:
+    """Check if the target port is likely available (best-effort)."""
+    import socket
+
+    try:
+        with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
+            s.settimeout(1)
+            s.bind((host if host != "0.0.0.0" else "127.0.0.1", port))
+    except OSError:
+        return PreflightIssue(
+            check="port-available",
+            message=f"Port {port} appears to be in use on {host}",
+            suggestion=(
+                f"Either stop the other process or use a different port:\n"
+                f"    openspace-mcp --port {port + 1}\n"
+                f"  Or set OPENSPACE_MCP_PORT={port + 1}"
+            ),
+            severity="warning",
+        )
+    return None
+
+
+def preflight_check(
+    transport: str = "stdio",
+    port: int = 8000,
+    host: str = "0.0.0.0",
+    skill_store_path: str = "skills/",
+    check_port: bool = True,
+) -> List[PreflightIssue]:
+    """Run all pre-flight checks and return any issues found.
+
+    Returns an empty list if all checks pass.
+
+    Args:
+        transport: MCP transport type (stdio, sse, streamable-http)
+        port: Server port (only checked for HTTP transports)
+        host: Server bind address
+        skill_store_path: Path to skill store directory
+        check_port: Whether to check port availability
+    """
+    issues: List[PreflightIssue] = []
+
+    result = check_python_version()
+    if result:
+        issues.append(result)
+
+    result = check_bearer_token(transport)
+    if result:
+        issues.append(result)
+
+    result = check_skill_store(skill_store_path)
+    if result:
+        issues.append(result)
+
+    if transport != "stdio" and check_port:
+        result = check_port_available(port, host)
+        if result:
+            issues.append(result)
+
+    return issues
+
+
+def format_preflight_report(issues: List[PreflightIssue]) -> str:
+    """Format preflight issues into a readable report."""
+    errors = [i for i in issues if i.severity == "error"]
+    warnings = [i for i in issues if i.severity == "warning"]
+
+    lines = ["\n╔══ OpenSpace Pre-flight Check ══╗\n"]
+
+    if errors:
+        lines.append(f"  {len(errors)} error(s) found:\n")
+        for issue in errors:
+            lines.append(f"  {issue}\n")
+
+    if warnings:
+        lines.append(f"  {len(warnings)} warning(s):\n")
+        for issue in warnings:
+            lines.append(f"  {issue}\n")
+
+    if not errors:
+        lines.append("  ✓ All checks passed\n")
+
+    lines.append("╚═══════════════════════════════╝")
+    return "\n".join(lines)

openspace/mcp/server.py

@@ -183,7 +183,84 @@ def _probe_mcp_tools() -> HealthProbe:
 
 
 # ---------------------------------------------------------------------------
-# 4. Server entry point
+# 4. CLI argument parser (extracted for testability)
+# ---------------------------------------------------------------------------
+_VERSION = "0.1.0"  # TODO: read from pyproject.toml or importlib.metadata
+
+_EPILOG = """\
+examples:
+  openspace-mcp                          # stdio transport (local, no auth)
+  openspace-mcp --transport streamable-http  # HTTP (requires bearer token)
+  openspace-mcp --transport sse --port 9000  # SSE on custom port
+
+environment variables:
+  OPENSPACE_MCP_HOST             Bind address (default: 0.0.0.0)
+  OPENSPACE_MCP_PORT             Port number (default: 8000)
+  OPENSPACE_MCP_TRANSPORT        Transport: stdio|sse|streamable-http
+  OPENSPACE_MCP_BEARER_TOKEN     Auth token (REQUIRED for HTTP transports)
+  OPENSPACE_LOG_LEVEL            Logging: DEBUG|INFO|WARNING|ERROR
+  OPENSPACE_SHUTDOWN_TIMEOUT     Graceful shutdown seconds (default: 30)
+  OPENSPACE_METRICS_ENABLED      Prometheus metrics: true|false
+
+notes:
+  HTTP transports (sse, streamable-http) require OPENSPACE_MCP_BEARER_TOKEN.
+  Use --transport stdio for local development without authentication.
+"""
+
+
+def _build_arg_parser(deploy_cfg=None):
+    """Build the CLI argument parser with rich help text.
+
+    Extracted from run_mcp_server for testability. Returns an
+    argparse.ArgumentParser ready for parse_args().
+    """
+    import argparse
+
+    defaults = {
+        "transport": "stdio",
+        "port": 8000,
+        "host": "0.0.0.0",
+    }
+    if deploy_cfg is not None:
+        defaults["transport"] = deploy_cfg.mcp_transport
+        defaults["port"] = deploy_cfg.mcp_port
+        defaults["host"] = deploy_cfg.mcp_host
+
+    parser = argparse.ArgumentParser(
+        prog="openspace-mcp",
+        description="OpenSpace MCP Server — AI agent framework with tool execution",
+        epilog=_EPILOG,
+        formatter_class=argparse.RawDescriptionHelpFormatter,
+    )
+    parser.add_argument(
+        "--version",
+        action="version",
+        version=f"openspace-mcp {_VERSION}",
+    )
+    parser.add_argument(
+        "--transport",
+        choices=["stdio", "sse", "streamable-http"],
+        default=defaults["transport"],
+        help="MCP transport protocol (default: %(default)s). "
+        "HTTP transports require OPENSPACE_MCP_BEARER_TOKEN.",
+    )
+    parser.add_argument(
+        "--port",
+        type=int,
+        default=defaults["port"],
+        help="Server port for HTTP transports (default: %(default)s)",
+    )
+    parser.add_argument(
+        "--host",
+        type=str,
+        default=defaults["host"],
+        help="Bind address for HTTP transports (default: %(default)s)",
+    )
+    return parser
+
+
+# ---------------------------------------------------------------------------
+# 5. Server entry point
 # ---------------------------------------------------------------------------
 def run_mcp_server(mcp=None) -> None:
     """Console-script entry point for ``openspace-mcp``.
@@ -195,8 +272,6 @@ def run_mcp_server(mcp=None) -> None:
         mcp: Optional pre-created FastMCP instance. If None, creates one
              via ``create_mcp_app()``.
     """
-    import argparse
-
     import uvicorn
 
     from openspace.auth.bearer import (
@@ -214,35 +289,57 @@ def run_mcp_server(mcp=None) -> None:
     # Load config from environment, then allow CLI overrides
     deploy_cfg = DeployConfig.from_env()
 
-    parser = argparse.ArgumentParser(description="OpenSpace MCP Server")
-    parser.add_argument(
-        "--transport",
-        choices=["stdio", "sse", "streamable-http"],
-        default=deploy_cfg.mcp_transport,
-    )
-    parser.add_argument("--port", type=int, default=deploy_cfg.mcp_port)
-    parser.add_argument("--host", type=str, default=deploy_cfg.mcp_host)
+    parser = _build_arg_parser(deploy_cfg)
     args = parser.parse_args()
 
+    # Pre-flight checks: validate environment before starting
+    from openspace.deploy.preflight import (
+        format_preflight_report,
+        preflight_check,
+    )
+
+    issues = preflight_check(
+        transport=args.transport,
+        port=args.port,
+        host=args.host,
+        skill_store_path=deploy_cfg.skill_store_path,
+        check_port=(args.transport != "stdio"),
+    )
+    if issues:
+        errors = [i for i in issues if i.severity == "error"]
+        report = format_preflight_report(issues)
+        # Write directly to console — logger goes to file only, user won't see it
+        _console = sys.__stderr__ or sys.stderr
+        _console.write(report + "\n")
+        _console.flush()
+        if errors:
+            sys.exit(1)
+
     if args.transport == "stdio":
         mcp.run(transport="stdio")
         return
 
     # --- HTTP transports: enforce bearer token auth (fail-closed) ---
     token = get_bearer_token()
+    _console = sys.__stderr__ or sys.stderr
     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,
+        msg = (
+            f"FAIL-CLOSED: {BEARER_TOKEN_ENV} not set. Refusing to start "
+            f"{args.transport} transport without authentication.\n"
+            f"  → Set the environment variable or use --transport stdio "
+            f"for local-only access.\n"
         )
+        _console.write(msg)
+        _console.flush()
+        logger.critical(msg.strip())
         sys.exit(1)
 
     token_ok, reason = validate_token_strength(token)
     if not token_ok:
-        logger.critical("FAIL-CLOSED: %s — %s", BEARER_TOKEN_ENV, reason)
+        msg = f"FAIL-CLOSED: {BEARER_TOKEN_ENV} — {reason}\n"
+        _console.write(msg)
+        _console.flush()
+        logger.critical(msg.strip())
         sys.exit(1)
 
     if args.transport == "sse":