Skip to content

Commit f94de88

Browse files
authored
Merge pull request #249 from vystartasv/fix/onboarding-focus-trap-and-platform-paths
fix: onboarding usability + platform-aware MCP paths
2 parents 168bd13 + fa49484 commit f94de88

4 files changed

Lines changed: 97 additions & 22 deletions

File tree

app/onboarding/interfaces/steps.py

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -522,28 +522,35 @@ def get_options(self) -> List[StepOption]:
522522
try:
523523
from app.tui.mcp_settings import list_mcp_servers
524524
servers = list_mcp_servers()
525-
526-
# Create a lookup by name
527-
server_lookup = {s["name"]: s for s in servers}
528-
529-
# Return only recommended servers that exist in config
530-
options = []
531-
for name, (icon, requires_setup) in self.RECOMMENDED_SERVERS.items():
532-
if name in server_lookup:
533-
server = server_lookup[name]
534-
label = server["name"].replace("-", " ").replace(" mcp", "").title()
535-
options.append(StepOption(
536-
value=server["name"],
537-
label=label,
538-
description=server.get("description", f"MCP server: {server['name']}"),
539-
default=server.get("enabled", False),
540-
icon=icon,
541-
requires_setup=requires_setup
542-
))
543-
return options
544-
except ImportError:
525+
except Exception:
526+
# If MCP config is completely broken, show nothing rather than
527+
# crashing the wizard — the user can configure later in Settings.
545528
return []
546529

530+
# Create a lookup by name
531+
server_lookup = {s["name"]: s for s in servers}
532+
533+
# Return only recommended servers that exist in config
534+
options = []
535+
for name, (icon, requires_setup) in self.RECOMMENDED_SERVERS.items():
536+
if name in server_lookup:
537+
server = server_lookup[name]
538+
label = server["name"].replace("-", " ").replace(" mcp", "").title()
539+
# Append platform warning to description when server paths
540+
# are incompatible with the current OS
541+
desc = server.get("description", f"MCP server: {server['name']}")
542+
if server.get("platform_blocked"):
543+
label += " (⚠ Windows-only — requires setup on this OS)"
544+
options.append(StepOption(
545+
value=server["name"],
546+
label=label,
547+
description=desc,
548+
default=server.get("enabled", False),
549+
icon=icon,
550+
requires_setup=requires_setup,
551+
))
552+
return options
553+
547554
def validate(self, value: Any) -> tuple[bool, Optional[str]]:
548555
# Value should be a list of server names
549556
if not isinstance(value, list):

app/tui/mcp_settings.py

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from __future__ import annotations
33

44
import json
5+
import sys
56
from pathlib import Path
67
from typing import Dict, List, Optional, Any
78

@@ -13,6 +14,23 @@
1314
MCP_CONFIG_PATH = APP_CONFIG_PATH / "mcp_config.json"
1415

1516

17+
def _is_windows_path(path: str) -> bool:
18+
"""Check if a path uses Windows drive-letter syntax (e.g. C:/...)."""
19+
return bool(path) and len(path) >= 2 and path[0].isalpha() and path[1] == ":"
20+
21+
22+
def _path_usable_on_current_platform(command: str, args: list) -> bool:
23+
"""Return False if command/args reference paths not valid on this OS."""
24+
if sys.platform == "win32":
25+
return True
26+
if _is_windows_path(command):
27+
return False
28+
for arg in args or []:
29+
if _is_windows_path(arg):
30+
return False
31+
return True
32+
33+
1634
def load_mcp_config() -> MCPConfig:
1735
"""Load MCP configuration from file."""
1836
try:
@@ -34,10 +52,27 @@ def save_mcp_config(config: MCPConfig) -> bool:
3452

3553

3654
def list_mcp_servers() -> List[Dict[str, Any]]:
37-
"""Get list of configured MCP servers with their status."""
38-
config = load_mcp_config()
55+
"""Get list of configured MCP servers with their status.
56+
57+
Servers with platform-incompatible paths (e.g. Windows paths on macOS)
58+
are annotated with a ``platform_blocked`` flag so the UI can explain why
59+
they cannot be started.
60+
"""
61+
try:
62+
config = load_mcp_config()
63+
except Exception as exc:
64+
logger.error(f"Failed to load MCP config: {exc}")
65+
return []
3966
servers = []
4067
for server in config.mcp_servers:
68+
platform_blocked = not _path_usable_on_current_platform(
69+
server.command or "", getattr(server, "args", []) or []
70+
)
71+
if platform_blocked:
72+
logger.debug(
73+
"MCP server %s has platform-specific paths — skipping on %s",
74+
server.name, sys.platform,
75+
)
4176
servers.append({
4277
"name": server.name,
4378
"description": server.description,
@@ -46,6 +81,7 @@ def list_mcp_servers() -> List[Dict[str, Any]]:
4681
"command": server.command,
4782
"action_set": server.resolved_action_set_name,
4883
"env": server.env,
84+
"platform_blocked": platform_blocked,
4985
})
5086
return servers
5187

app/tui/onboarding/widgets.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,11 @@ class OnboardingWizardScreen(Screen):
286286

287287
CSS = ONBOARDING_CSS
288288

289+
BINDINGS = [
290+
("ctrl+s", "skip_step", "Skip"),
291+
("escape", "cancel", "Cancel"),
292+
]
293+
289294
def __init__(self, handler: "TUIHardOnboarding"):
290295
super().__init__()
291296
self._handler = handler
@@ -695,7 +700,19 @@ def _complete(self) -> None:
695700
self._handler.on_complete(cancelled=False)
696701
self.app.pop_screen()
697702

703+
def action_skip_step(self) -> None:
704+
"""Skip the current optional step (Ctrl+S)."""
705+
step = self._handler.get_step(self._current_step)
706+
if not step.required:
707+
self._skip_step()
708+
698709
def action_cancel(self) -> None:
699710
"""Handle Escape key to cancel wizard."""
700711
self._handler.on_complete(cancelled=True)
701712
self.app.pop_screen()
713+
714+
def action_focus_nav(self) -> None:
715+
"""Focus the navigation bar (Tab)."""
716+
nav = self.query_one("#nav-actions")
717+
if hasattr(nav, 'focus'):
718+
nav.focus()

app/ui_layer/browser/frontend/src/pages/Onboarding/OnboardingPage.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,21 @@ export function OnboardingPage() {
509509
}, [onboardingStep, selectedValue, textValue, orModel, proxiedVia, ollamaUrl, formValues, submitOnboardingStep])
510510

511511
const handleSkip = useCallback(() => skipOnboardingStep(), [skipOnboardingStep])
512+
513+
// Ctrl+S to skip optional steps (matches TUI behavior)
514+
useEffect(() => {
515+
const handler = (e: KeyboardEvent) => {
516+
if ((e.ctrlKey || e.metaKey) && e.key === 's') {
517+
if (onboardingStep && !onboardingStep.required) {
518+
e.preventDefault()
519+
skipOnboardingStep()
520+
}
521+
}
522+
}
523+
window.addEventListener('keydown', handler)
524+
return () => window.removeEventListener('keydown', handler)
525+
}, [onboardingStep, skipOnboardingStep])
526+
512527
const handleBack = useCallback(() => goBackOnboardingStep(), [goBackOnboardingStep])
513528

514529
const isMultiSelect = onboardingStep?.name === 'mcp' || onboardingStep?.name === 'skills'

0 commit comments

Comments
 (0)