Skip to content

fix: onboarding usability + platform-aware MCP paths#249

Open
vystartasv wants to merge 10 commits into
CraftOS-dev:devfrom
vystartasv:fix/onboarding-focus-trap-and-platform-paths
Open

fix: onboarding usability + platform-aware MCP paths#249
vystartasv wants to merge 10 commits into
CraftOS-dev:devfrom
vystartasv:fix/onboarding-focus-trap-and-platform-paths

Conversation

@vystartasv
Copy link
Copy Markdown

What

Three fixes from onboarding testing on macOS:

1. Can't skip optional wizard steps (#1)

The MCP and Skills onboarding steps use multi-select toggle buttons that trap keyboard focus — no way to reach the "next/skip" nav bar without a mouse. Fix: added action_skip_step() (Ctrl+S) to skip optional steps, and action_focus_nav() to jump to the navigation bar.

2. Platform-incompatible MCP paths (#2)

mcp_config.json contains ~70 Windows hardcoded paths (C:/path/to/...) that silently fail on macOS/Linux via subprocess. Fix: list_mcp_servers() now detects Windows drive-letter paths and annotates servers with a platform_blocked flag. The onboarding MCPStep renders blocked servers with "(Windows-only)" in the description.

3. Silenced configuration errors (#3)

MCPStep.get_options() swallowed ALL exceptions (ImportError, JSON decode, etc.) and returned an empty list — leaving users with a blank MCP screen and zero feedback. Fix: broadened the guard to catch all load failures (config corruption, file missing) and moved the import try/except to only guard the actual import.

Testing

  • Compiles clean on Python 3.13
  • list_mcp_servers() filters Windows paths on macOS
  • Ctrl+S skips optional steps in the wizard
  • platform_blocked flag visible in server metadata

Files changed

  • app/tui/onboarding/widgets.py — keyboard actions
  • app/tui/mcp_settings.py — platform helpers + list filtering
  • app/onboarding/interfaces/steps.py — error handling + platform warnings

@zfoong zfoong added feature New feature or request priority: medium labels May 13, 2026
@zfoong
Copy link
Copy Markdown
Collaborator

zfoong commented May 13, 2026

Thanks for the PR @vystartasv! @ahmad-ajmal you think we can include this in V1.3.1?

@zfoong
Copy link
Copy Markdown
Collaborator

zfoong commented May 13, 2026

@vystartasv By the way, we will rework on TUI entirely, changing stack to React + Ink.

@ahmad-ajmal
Copy link
Copy Markdown
Collaborator

Concerns

  1. The PR description says: "Multi-select toggle buttons trapped focus, preventing keyboard-only access to navigation controls. Solution adds Ctrl+S shortcut and focus-jumping action."

    However, OnboardingWizardScreen has no BINDINGS attribute, so Textual never routes ctrl+s to action_skip_step or tab to action_focus_nav. The action methods are defined but unreachable from the keyboard.

    Minimum fix:

    BINDINGS = [
        ("ctrl+s", "skip_step", "Skip"),
        ("escape", "cancel", "Cancel"),
    ]
    

    (Tab is risky at screen scope, it overrides Textual's default focus traversal. Better as a Binding on the multi-select widget itself, scoped to fire only when focus is inside the toggle list.)

  2. The suffix is appended to StepOption.description in steps.py, but _build_multi_select in widgets.py:452-456 only renders opt.label and never description. So on the very step the PR targets, TUI users won't see the platform warning.

    Two options:

    • Append the suffix to label instead of description (simplest)
    • Extend _build_multi_select to render a second line with description below label
  3. Browser onboarding has no Ctrl+S handler, pressing Ctrl+S during onboarding triggers the browser's "Save Page As" dialog. If keyboard-skip is a goal of this PR, the browser frontend at app/ui_layer/browser/frontend/src/pages/Onboarding/OnboardingPage.tsx needs an equivalent useEffect keydown handler that calls the skip-step action when the current step is optional.

To think about

Currently a user on macOS can still tick the checkbox on Gmail or Google Calendar, save the selection, and the server will silently fail when the agent later tries to spawn node C:/.... Is "informational warning only" the intent? If so, worth a comment in the code. If not, the toggle should probably be disabled when platform_blocked is true.

Infra

Please make the PR to be merged into dev

zfoong and others added 9 commits May 13, 2026 12:36
- Add Ctrl+S shortcut to skip optional wizard steps (MCP/Skills)
- Add 'focus_nav' action to reach nav bar from multi-select widgets
- Detect Windows-only paths in MCP servers and annotate as platform_blocked
- Surface platform warnings in onboarding MCP step descriptions
- Guard list_mcp_servers() against silent config-load failures
…Ctrl+S

1. Add BINDINGS to OnboardingWizardScreen so Ctrl+S/Escape route to
   action_skip_step/action_cancel (previously unreachable from keyboard).

2. Append platform-blocked warning to label instead of description
   so _build_multi_select renders it — it only shows opt.label, never
   opt.description.

3. Add useEffect keydown handler in OnboardingPage.tsx for Ctrl+S/Cmd+S
   to skip optional onboarding steps, matching TUI behavior. Prevents
   browser 'Save As' dialog interference.
@vystartasv vystartasv force-pushed the fix/onboarding-focus-trap-and-platform-paths branch from 125bc4c to 75533eb Compare May 13, 2026 11:36
@vystartasv vystartasv changed the base branch from main to dev May 13, 2026 11:36
@vystartasv
Copy link
Copy Markdown
Author

Addressed all three review concerns:

  1. BINDINGS — Added to OnboardingWizardScreen:

    BINDINGS = [("ctrl+s", "skip_step", "Skip"), ("escape", "cancel", "Cancel")]
  2. Platform warning — Moved from description → label so _build_multi_select renders it. Now shows (⚠ Windows-only — requires setup on this OS) inline.

  3. Browser Ctrl+S — Added useEffect keydown handler in OnboardingPage.tsx. Prevents browser Save dialog, calls skipOnboardingStep() for optional steps only. Matches TUI behavior.

Also retargeted to dev branch.

@zfoong zfoong requested a review from ahmad-ajmal May 14, 2026 01:14
@zfoong
Copy link
Copy Markdown
Collaborator

zfoong commented May 14, 2026

@vystartasv Good to go, before that, can you point the PR to branch V1.3.1? Sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request priority: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants