feat(onboarding): add interactive skill ecosystem selection to wave init

[nextlevelshit]

Summary

• Adds Step 6 — Skill Selection to the wave init onboarding wizard
• Supports 4 skill ecosystems: Tessl (browse + multi-select), BMAD, OpenSpec, Spec-Kit (install-all)
• Checks CLI dependencies before installation with install instructions and retry flow
• Handles network failures gracefully with skip/retry options
• Supports reconfigure mode with skill deduplication against existing manifest
• Updates step numbering across all wizard steps (1-5 → 1-6)
• Writes installed skills to wave.yaml manifest under the skills key

Spec

specs/384-init-skill-selection/spec.md

Test Plan

• Unit tests for parseTesslOutput, findEcosystem, mergeSkills helpers (table-driven)
• Non-interactive mode returns empty skills (no TUI prompts)
• Manifest building: skills key present when skills installed, absent when empty
• defaultCommandRunner success/failure paths
• All ecosystem definitions verified (binary names, install instructions, flags)
• Full go test -race ./... passes

Known Limitations

• Interactive TUI flows (ecosystem selection, multi-select) are not unit-tested due to huh form dependencies — manual testing recommended
• Tessl search output parsing assumes whitespace-separated fields; may need adjustment if registry format changes

Closes #384

[nextlevelshit]

Code Review (Wave Pipeline)

Verdict: REQUEST_CHANGES

This PR adds a well-structured onboarding step for interactive skill ecosystem selection. Error handling is comprehensive, naming follows codebase conventions, and all 45 onboarding tests pass. However, two issues warrant changes before merge: a silent data loss bug in non-interactive reconfigure and missing timeouts on external subprocess calls.

---

Critical Issues

1. Non-interactive reconfigure silently drops all installed skills
internal/onboarding/skill_step.go:87-94

When --yes --reconfigure is used (CI, scripted workflows), the step returns an empty skills list, bypassing merge logic entirely. Previously installed skills vanish from the manifest with no warning. Fix: when cfg.Reconfigure && !cfg.Interactive && cfg.Existing != nil, return cfg.Existing.Skills instead of []string{}.

2. No timeout on external command execution
internal/onboarding/skill_step.go:168,578-585

context.Background() is used for tessl search and router.Install() calls. If the external registry is unresponsive, the wizard hangs indefinitely with no way to cancel except killing the terminal. The commandRunner signature already accepts a context — use context.WithTimeout(ctx, 30*time.Second) or align with the existing skill.CLITimeout constant.

---

Suggested Improvements

3. Extract shared select-and-install helper (~60 duplicated lines)
internal/onboarding/skill_step.go:288-362 and 364-464

handleSearchFailure is a verbatim copy of the multi-select form + install loop from handleTesslEcosystem. A shared selectAndInstallSkills() helper would eliminate this duplication and ensure future fixes apply to both paths.

4. parseTesslOutput unconditionally drops the second field
internal/onboarding/skill_step.go:517-539

The comment says "if the second field looks like a rating, skip it" but the code always skips fields[1] when there are 3+ fields — no rating detection occurs. If tessl output ever omits the rating column, the first description word is silently lost. This also duplicates parseTesslSearchOutput from cmd/wave/commands/skills.go:412-437; consider extracting a shared parser.

5. TestSkillSelectionStep_MissingCLI doesn't test missing CLI behavior
internal/onboarding/skill_step_test.go:228-243

The test sets Interactive: false, so Run() returns before lookPath is ever called (lookPath is initialized after the non-interactive early return at line 88-93). The test is functionally identical to TestSkillSelectionStep_NonInteractive and gives false confidence in CLI-check coverage. Rename it or make it exercise the actual CLI check path with Interactive: true.

6. Use filepath.Join for path construction
internal/onboarding/skill_step.go:157-158

cfg.WaveDir + "/skills" should be filepath.Join(cfg.WaveDir, "skills") for consistency with the rest of the codebase. This also addresses the defense-in-depth concern around path traversal if WaveDir ever becomes user-configurable.

7. Hardcoded "Step N of 6" strings across 7 locations
internal/onboarding/steps.go (6 sites) + skill_step.go:215

Adding another step requires touching all 7 locations. A const totalSteps = 6 with fmt.Sprintf would reduce this to a single-line change.

---

Positive Observations

• Safe command invocation — all external commands use exec.CommandContext with argument slices, never shell string concatenation, preventing command injection
• Strong downstream validation — skill.SourceRouter validates names via strict regex; skill.Store enforces path containment with symlink detection
• Testability — LookPath and RunCommand are injectable, enabling thorough unit testing without real external dependencies
• Graceful degradation — network failures and install errors present skip/retry options rather than crashing the wizard
• Comprehensive error handling — every error path (form abort, lookup failure, mkdir failure, install failure, search failure) is handled with appropriate user feedback
• Clean integration — the step slots into the existing wizard with minimal changes to onboarding.go and steps.go

---

Generated by Wave gh-pr-review pipeline