fix: address R1 review findings for Epic 6.4 DX polish - preflight visibility, platform-aware suggestions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Brian Krafft

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

@@ -60,12 +60,20 @@ def check_bearer_token(transport: str) -> Optional[PreflightIssue]:
     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"    export {token_env}=$(python -c \"import secrets; print(secrets.token_urlsafe(32))\")\n"
+                f"    {set_cmd}\n"
                 f"  Or use --transport stdio for local-only access (no auth required)"
             ),
         )
@@ -76,10 +84,14 @@ 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    rm {path} && mkdir -p {path}",
+            suggestion=f"Remove the file and create directory:\n    {fix_cmd}",
         )
     return None
 

openspace/mcp/server.py

@@ -308,7 +308,10 @@ def run_mcp_server(mcp=None) -> None:
     if issues:
         errors = [i for i in issues if i.severity == "error"]
         report = format_preflight_report(issues)
-        logger.info(report)
+        # 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)
 
@@ -318,19 +321,25 @@ def run_mcp_server(mcp=None) -> None:
 
     # --- 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":

tests/test_dx_polish.py

@@ -105,15 +105,16 @@ def test_preflight_catches_missing_bearer_token(self):
         assert "OPENSPACE_MCP_BEARER_TOKEN" in token_issues[0].message
 
     def test_preflight_suggestion_is_actionable(self):
-        """Suggestions must include a command the user can run."""
+        """Suggestions must include a platform-appropriate set command."""
         from openspace.deploy.preflight import preflight_check
 
         with patch.dict(os.environ, {}, clear=True):
             issues = preflight_check(transport="streamable-http", check_port=False)
         token_issues = [i for i in issues if i.check == "bearer-token"]
         assert token_issues
         suggestion = token_issues[0].suggestion
-        assert "export" in suggestion or "set" in suggestion.lower()
+        # Must contain a real shell command, not just descriptive text
+        assert "$env:" in suggestion or "export " in suggestion
 
     def test_preflight_no_token_check_for_stdio(self):
         from openspace.deploy.preflight import check_bearer_token
@@ -201,6 +202,60 @@ def test_preflight_report_all_clear(self):
         report = format_preflight_report([])
         assert "All checks passed" in report
 
+    def test_preflight_bearer_suggestion_platform_aware(self):
+        """Suggestion must use platform-appropriate shell syntax."""
+        from openspace.deploy.preflight import check_bearer_token
+
+        with patch.dict(os.environ, {}, clear=True):
+            result = check_bearer_token("streamable-http")
+        assert result is not None
+        if os.name == "nt":
+            assert "$env:" in result.suggestion
+        else:
+            assert "export " in result.suggestion
+
+    def test_preflight_skill_store_suggestion_platform_aware(self, tmp_path):
+        """Skill store fix suggestion must use platform-appropriate commands."""
+        from openspace.deploy.preflight import check_skill_store
+
+        fake_path = tmp_path / "skills"
+        fake_path.write_text("not a directory")
+        result = check_skill_store(str(fake_path))
+        assert result is not None
+        if os.name == "nt":
+            assert "Remove-Item" in result.suggestion
+        else:
+            assert "rm " in result.suggestion
+
+
+# ======================================================================
+# Preflight visibility (P0 fix)
+# ======================================================================
+class TestPreflightVisibility:
+    """Preflight output must reach the user, not just the log file."""
+
+    def test_preflight_writes_to_console_not_just_logger(self):
+        """run_mcp_server must write preflight to sys.__stderr__, not just logger."""
+        import inspect
+        from openspace.mcp.server import run_mcp_server
+
+        source = inspect.getsource(run_mcp_server)
+        assert "__stderr__" in source, (
+            "Preflight output must use sys.__stderr__ to reach console"
+        )
+
+    def test_critical_errors_write_to_console(self):
+        """Bearer token errors must also reach console."""
+        import inspect
+        from openspace.mcp.server import run_mcp_server
+
+        source = inspect.getsource(run_mcp_server)
+        # Count __stderr__ occurrences — should be used for both preflight AND auth errors
+        count = source.count("__stderr__")
+        assert count >= 2, (
+            f"Expected __stderr__ used for preflight AND auth errors, found {count} uses"
+        )
+
 
 # ======================================================================
 # Error message UX