feat(onboard): uplift first-run wizard flow#690
feat(onboard): uplift first-run wizard flow#690gh-xj wants to merge 53 commits intoeastreams:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a stepwise onboarding wizard with provenance-tracked draft state, a flow controller and guided-step runner, rich/plain interaction-mode detection with back-navigation, workspace/protocol derivation and validation, grouped preflight outcome classification (including post-write verification), and outcome-aware success summaries. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as Onboard CLI
participant UI as OnboardUi
participant Flow as OnboardFlowController
participant Runner as GuidedOnboardFlowStepRunner
participant Draft as OnboardDraft
User->>CLI: run onboarding
CLI->>UI: set_interaction_mode(mode)
CLI->>Draft: OnboardDraft::from_config(...)
CLI->>Flow: OnboardFlowController::new(draft)
loop per wizard step (until EnvironmentCheck)
Flow->>Runner: run_step(current_step, &mut draft)
Runner->>UI: render step UI
UI->>User: prompt / select
alt user selects value
User->>UI: choose
UI->>Runner: SelectAction::Selected(index)
Runner->>Draft: apply selection
Runner->>Flow: Next
else user presses back
User->>UI: "back"
UI->>Runner: SelectAction::Back
Runner->>Flow: Back
else user skips
Runner->>Flow: Skip
end
Flow->>Flow: advance/back/skip cursor
end
CLI->>CLI: run preflight checks
CLI->>CLI: derive OnboardOutcome (green/warnings/blocked)
CLI->>UI: render review & write confirmation
UI->>User: show diff and ask confirm
User->>CLI: confirm
CLI->>Draft: write config
CLI->>UI: build success summary with outcome
UI->>User: display final readiness
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Refactors and expands loongclaw onboard into an explicit 8-stage first-run wizard with shared draft/state handling, clearer readiness gating (ready, ready with warnings, blocked), new workspace/protocol stages, and updated docs/tests to match the shipped flow contract.
Changes:
- Introduces
OnboardDraft+OnboardFlowControllerto coordinate an 8-step guided wizard with origins tracking and back/skip semantics. - Adds explicit workspace and protocol (ACP/MCP) stages plus grouped preflight/readiness outcomes and post-write verification reporting.
- Updates integration tests and documentation/specs (README + product specs) to reflect the new onboarding contract.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/product-specs/onboarding.md | Marks acceptance criteria as shipped and documents the 8-stage wizard + readiness outcomes. |
| docs/product-specs/doctor.md | Aligns doctor acceptance criteria with onboarding readiness/repair handoff expectations. |
| README.md | Documents the new wizard stages, rerun semantics, readiness outcomes, and prompt fallback behavior. |
| crates/daemon/src/lib.rs | Exposes new onboarding modules (onboard_flow, onboard_state). |
| crates/daemon/src/onboard_cli.rs | Main orchestration refactor: flow controller integration, interaction-mode selection, workspace/protocol steps, readiness gating, write/rollback/verification behavior. |
| crates/daemon/src/onboard_flow.rs | Adds ordered-step controller and runner interface for guided flow execution. |
| crates/daemon/src/onboard_state.rs | Adds shared onboarding draft model, value origins, interaction modes, and outcome types. |
| crates/daemon/src/onboard_workspace.rs | Implements workspace step value derivation, persistence, and path validation. |
| crates/daemon/src/onboard_protocols.rs | Implements protocol step value derivation + ACP backend discovery/defaulting helpers. |
| crates/daemon/src/onboard_preflight.rs | Adds protocol checks and grouped preflight presentation + outcome computation helpers. |
| crates/daemon/src/onboard_finalize.rs | Extends success summary to include outcome + verification status and adjusts final screen copy. |
| crates/app/src/config/tools.rs | Adds explicit_file_root() to distinguish blank/implicit values from explicit configuration. |
| crates/daemon/tests/integration/onboard_cli.rs | Expands coverage for step order, back/skip, file_root blank semantics, readiness grouping, and post-write verification rollback/reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async fn run_authentication_step(&mut self, draft: &mut OnboardDraft) -> CliResult<()> { | ||
| let guided_prompt_path = resolve_guided_prompt_path(self.options, &draft.config); | ||
| let selected_provider = resolve_provider_selection( | ||
| self.options, | ||
| &draft.config, | ||
| &self.starting_selection.provider_selection, | ||
| guided_prompt_path, | ||
| self.ui, | ||
| self.context, | ||
| )?; | ||
| draft.set_provider_config(selected_provider); | ||
|
|
||
| let available_models = load_onboarding_model_catalog(self.options, &draft.config).await; | ||
| let selected_model = resolve_model_selection( | ||
| self.options, | ||
| &draft.config, | ||
| guided_prompt_path, | ||
| &available_models, | ||
| self.ui, | ||
| self.context, | ||
| )?; | ||
| draft.set_provider_model(selected_model); | ||
|
|
||
| let default_api_key_env = preferred_api_key_env_default(&draft.config); | ||
| let selected_api_key_env = resolve_api_key_env_selection( | ||
| self.options, | ||
| &draft.config, | ||
| default_api_key_env, | ||
| guided_prompt_path, | ||
| self.ui, | ||
| self.context, | ||
| )?; | ||
| draft.set_provider_credential_env(selected_api_key_env); | ||
| Ok(()) |
There was a problem hiding this comment.
Back navigation from the authentication step isn’t handled: resolve_provider_selection / resolve_model_selection / resolve_api_key_env_selection can now return SelectAction::Back (surfaced as the ONBOARD_BACK_NAVIGATION_SIGNAL error via select_one_selected_index), but run_authentication_step currently propagates that error and aborts the flow with an internal sentinel string instead of returning OnboardFlowStepAction::Back to the controller. Handle the back signal in this step (similar to run_runtime_defaults_step) so selecting “Back” navigates to the previous wizard step rather than failing the command.
| async fn run_authentication_step(&mut self, draft: &mut OnboardDraft) -> CliResult<()> { | |
| let guided_prompt_path = resolve_guided_prompt_path(self.options, &draft.config); | |
| let selected_provider = resolve_provider_selection( | |
| self.options, | |
| &draft.config, | |
| &self.starting_selection.provider_selection, | |
| guided_prompt_path, | |
| self.ui, | |
| self.context, | |
| )?; | |
| draft.set_provider_config(selected_provider); | |
| let available_models = load_onboarding_model_catalog(self.options, &draft.config).await; | |
| let selected_model = resolve_model_selection( | |
| self.options, | |
| &draft.config, | |
| guided_prompt_path, | |
| &available_models, | |
| self.ui, | |
| self.context, | |
| )?; | |
| draft.set_provider_model(selected_model); | |
| let default_api_key_env = preferred_api_key_env_default(&draft.config); | |
| let selected_api_key_env = resolve_api_key_env_selection( | |
| self.options, | |
| &draft.config, | |
| default_api_key_env, | |
| guided_prompt_path, | |
| self.ui, | |
| self.context, | |
| )?; | |
| draft.set_provider_credential_env(selected_api_key_env); | |
| Ok(()) | |
| async fn run_authentication_step( | |
| &mut self, | |
| draft: &mut OnboardDraft, | |
| ) -> CliResult<OnboardFlowStepAction> { | |
| let guided_prompt_path = resolve_guided_prompt_path(self.options, &draft.config); | |
| let selected_provider = match resolve_provider_selection( | |
| self.options, | |
| &draft.config, | |
| &self.starting_selection.provider_selection, | |
| guided_prompt_path, | |
| self.ui, | |
| self.context, | |
| ) { | |
| Ok(provider) => provider, | |
| Err(err) => { | |
| if err.to_string() == ONBOARD_BACK_NAVIGATION_SIGNAL { | |
| return Ok(OnboardFlowStepAction::Back); | |
| } else { | |
| return Err(err.into()); | |
| } | |
| } | |
| }; | |
| draft.set_provider_config(selected_provider); | |
| let available_models = load_onboarding_model_catalog(self.options, &draft.config).await; | |
| let selected_model = match resolve_model_selection( | |
| self.options, | |
| &draft.config, | |
| guided_prompt_path, | |
| &available_models, | |
| self.ui, | |
| self.context, | |
| ) { | |
| Ok(model) => model, | |
| Err(err) => { | |
| if err.to_string() == ONBOARD_BACK_NAVIGATION_SIGNAL { | |
| return Ok(OnboardFlowStepAction::Back); | |
| } else { | |
| return Err(err.into()); | |
| } | |
| } | |
| }; | |
| draft.set_provider_model(selected_model); | |
| let default_api_key_env = preferred_api_key_env_default(&draft.config); | |
| let selected_api_key_env = match resolve_api_key_env_selection( | |
| self.options, | |
| &draft.config, | |
| default_api_key_env, | |
| guided_prompt_path, | |
| self.ui, | |
| self.context, | |
| ) { | |
| Ok(api_key_env) => api_key_env, | |
| Err(err) => { | |
| if err.to_string() == ONBOARD_BACK_NAVIGATION_SIGNAL { | |
| return Ok(OnboardFlowStepAction::Back); | |
| } else { | |
| return Err(err.into()); | |
| } | |
| } | |
| }; | |
| draft.set_provider_credential_env(selected_api_key_env); | |
| Ok(OnboardFlowStepAction::Next) |
| @@ -132,6 +155,8 @@ impl OnboardRuntimeContext { | |||
| render_width: detect_render_width(), | |||
| workspace_root: env::current_dir().ok(), | |||
| codex_config_paths: default_codex_config_paths(), | |||
| attended_terminal: user_attended(), | |||
| rich_prompt_ui_supported: rich_prompt_ui_available(), | |||
| } | |||
| } | |||
|
|
|||
| @@ -144,10 +169,35 @@ impl OnboardRuntimeContext { | |||
| render_width, | |||
| workspace_root, | |||
| codex_config_paths: codex_config_paths.into_iter().collect(), | |||
| attended_terminal: true, | |||
| rich_prompt_ui_supported: true, | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| fn resolve_onboard_interaction_mode( | |||
| non_interactive: bool, | |||
| attended_terminal: bool, | |||
| rich_prompt_ui_supported: bool, | |||
| ) -> OnboardInteractionMode { | |||
| if non_interactive { | |||
| return OnboardInteractionMode::NonInteractive; | |||
| } | |||
| if attended_terminal && rich_prompt_ui_supported { | |||
| return OnboardInteractionMode::RichInteractive; | |||
| } | |||
| OnboardInteractionMode::PlainInteractive | |||
| } | |||
There was a problem hiding this comment.
OnboardRuntimeContext::capture sets both attended_terminal and rich_prompt_ui_supported from user_attended() (rich_prompt_ui_available() is just user_attended()), so resolve_onboard_interaction_mode will never choose PlainInteractive for an attended terminal in real runs. This makes the documented “plain-prompt fallback for degraded terminals” effectively unreachable. Consider implementing a real rich-prompt capability check (or dynamically falling back to plain prompts when dialoguer returns an I/O error) so rich_prompt_ui_supported can differ from attended_terminal.
| let explicit_file_root = draft.config.tools.explicit_file_root(); | ||
| let persist_displayed_file_root = | ||
| explicit_file_root.is_some() || context.workspace_root.is_some(); | ||
| let (file_root, file_root_origin) = | ||
| if explicit_file_root.is_none() && context.workspace_root.is_some() { | ||
| ( | ||
| context | ||
| .workspace_root | ||
| .clone() | ||
| .unwrap_or_else(|| draft.workspace.file_root.clone()), | ||
| Some(OnboardValueOrigin::DetectedStartingPoint), | ||
| ) | ||
| } else { | ||
| ( | ||
| draft.workspace.file_root.clone(), | ||
| draft.origin_for(OnboardDraft::WORKSPACE_FILE_ROOT_KEY), | ||
| ) |
There was a problem hiding this comment.
persist_displayed_file_root becomes true whenever context.workspace_root is Some(_). In normal CLI runs workspace_root is populated from env::current_dir(), so rerunning onboarding against a config with an implicit/blank tools.file_root will materialize and persist the current directory as an explicit tools.file_root even if the user just hits Enter. This conflicts with the stated goal that blank file roots remain implicit unless explicitly chosen. Consider making persistence depend on an explicit existing value or an explicit user override (e.g., persist only when explicit_file_root.is_some() or when the user edits the prompt), and avoid forcing DetectedStartingPoint origin purely because workspace_root exists.
…din infra Replace the trait-based UI abstraction with simple free functions that read/write directly to stdin/stdout. The guided flow now runs through RatatuiOnboardRunner; pre-flow and post-flow screens use the new prompt_stdin_* / print_stdout_* helpers. Removed: - OnboardUi trait and PlainOnboardUi struct - StdioOnboardLineReader (background thread + paste-drain machinery) - OnboardPromptCapture, OnboardPromptRead, OnboardPromptLineReader - run_onboard_cli_with_ui (merged into run_onboard_cli_inner) - dialoguer dependency (already gone from Cargo.toml) - onboard_presentation.rs (moved to onboard_cli::presentation earlier) Added: - read_stdin_line, prompt_stdin_with_default, prompt_stdin_required, prompt_stdin_confirm, prompt_stdin_select - print_stdout_lines, print_stdout_message All resolver functions retain their signatures (minus the ui param) as dead code covered by #![allow(dead_code)] -- they will be reconnected in Tasks 8-10. Unit and integration tests that depend on OnboardUi are disabled with #[cfg(any())] and will be rewritten in Tasks 8-9.
Re-enable the test module (#[cfg(test)] mod tests) that was disabled with #[cfg(any())] after the OnboardUi trait removal. Removed all test infrastructure that depended on the deleted OnboardUi trait: TestOnboardUi, SelectOnlyTestUi, AllowEmptyOnlyTestUi, TestPromptLineReader, and all tests that scripted interactive flows through the old UI parameter. Those interactive paths are now covered by the RatatuiOnboardRunner tests in onboard_tui::runner::tests. Kept 66 pure function tests covering: preflight checks, probe failure diagnostics, credential validation, config resolution, rendering screens, backup/rollback, web search provider recommendations, and select/option formatting.
Fix the non-interactive TUI bug: guard RatatuiOnboardRunner creation with !options.non_interactive so headless/CI runs never attempt to initialize a terminal. Add apply_non_interactive_overrides() that applies CLI flags (--provider, --model, --api-key-env, --personality, --memory-profile, --system-prompt, --web-search-provider, --web-search-api-key-env) and workspace defaults to the draft for non-interactive mode, replacing the old resolver function call chain. Add run_onboard_cli_with_context() test-only entrypoint so integration tests can supply a custom OnboardRuntimeContext without touching the real terminal. Rewrite integration tests: - Remove ScriptedOnboardUi, WriteConfirmationGuardUi, and OnboardUi trait impls (no longer exist) - Remove run_scripted_onboard_flow* helpers - Convert 20 non-interactive async tests to use run_onboard_cli_with_context directly - Delete 33 interactive flow tests (covered by TUI runner unit tests) - Keep 133 pure sync tests unchanged (rendering, validation, etc.) - Re-enable the onboard_cli test module (remove #[cfg(any())] guard) 153 integration tests pass, 733 total integration tests pass, 3856 workspace tests pass.
…lver code Remove ~1200 lines of dead interactive resolver functions that were only called by the removed GuidedOnboardUiRunner (resolve_provider_selection, resolve_model_selection, resolve_api_key_env_selection, etc.). Extract ~2100 lines of screen rendering, review display, starting point detail, and utility functions into onboard_cli/screens.rs submodule. Gate test-only items (SystemPromptSelection, WebSearchCredentialSelection, apply_selected_api_key_env, etc.) with #[cfg(test)]. Result: onboard_cli.rs reduced from 5990 to 2694 lines (55% reduction).
Move TUI initialization to the start of the interactive onboard flow so all screens (risk acknowledgement, entry choice, import selection, shortcut choice, guided wizard, preflight, review, write confirmation, success) render inside the ratatui alternate screen. Key changes: - Create RatatuiOnboardRunner at the top of run_onboard_cli_inner, before the risk screen, so it owns the terminal for the entire interactive session. - Add pre-flow screen methods: run_risk_screen, run_entry_choice_screen, run_import_candidate_screen, run_shortcut_choice_screen. These use full-width content (no progress spine) with the same branded header. - Add post-flow screen methods: run_preflight_screen (colored pass/warn/fail indicators), run_review_screen (scrollable), run_write_confirmation_screen, run_success_screen. - Add generic building blocks: run_confirm_screen (yes/no), run_info_screen (scrollable, Enter to continue), run_standalone_selection_loop (no spine, for pre/post flow). - Add load_import_starting_config_with_tui and select_interactive_import_starting_config_with_tui to route the entry/import choice through the TUI runner. - Replace all print_stdout_lines + prompt_stdin_confirm calls in the interactive path with runner method calls. - Non-interactive path remains unchanged. - 17 new headless tests cover all new runner methods.
… screens Add centered rounded-border dialog boxes to every onboard TUI screen (welcome, selection, input, confirm, info, risk, preflight, review, write-confirmation, success). Replace arrow indicators with radio button symbols and use Cyan accent for selected items. Dialog boxes are max 60 chars wide, horizontally and vertically centered, with configurable border colors per screen type.
Replace EnterAlternateScreen/LeaveAlternateScreen with Viewport::Inline(20) so the onboard TUI renders at the current cursor position instead of taking over the full terminal. Remove the OnboardTuiMode enum and all FullScreen vs Inline branching, including the progress spine sidebar that only applied in fullscreen mode.
- Defer backup-path message until after TUI runner is dropped so it does not corrupt the terminal by printing in raw mode (C1). - Return Ok(()) on Esc in run_info_screen; the info screen is informational, not a decision point, so both Enter and Esc should dismiss it cleanly (C2). - Remove dead _body_lines code in run_shortcut_choice_screen that was built but never rendered (C3). - Fix pre-existing clippy issues: replace unreachable!() with proper error returns, remove unused import, collapse nested if.
- Show cursor before placeholder text when default is active, so it does not look like typed text (M3). - Remove dead WelcomeScreen widget and its module declaration; the runner builds welcome content inline (M4).
… cleanup - Gracefully fall back to non-interactive path when TUI init fails (dumb terminal, piped stdin) instead of hard-erroring - Render text cursor at actual position instead of always at end, now that Left/Right navigation is supported - Remove stale #[allow(dead_code)] annotations from widgets, layout, event source, and runner that are no longer needed
Add 14 new tests covering the interactive TUI runner via ScriptedEventSource: - Full guided flow through all screens (ACP disabled and enabled) - Back-navigation across steps (Auth -> Welcome replay) - Sub-step back-navigation within Authentication - Ctrl-C interruption on every step type (7 tests) - Resize event handling on welcome and selection loops - Zero-item SelectionCardState/Widget guard
|
The integrated fullscreen first-run TUI handoff now also lives on #691 so the app-owned chat shell and the missing-config onboarding path can be reviewed together as one delivery branch. Leaving #690 open for traceability for now. Until we decide the final landing path, please treat #691 as the combined review branch for the fullscreen chat shell plus first-run TUI handoff. |
|
hi @gh-xj, thanks for carrying this forward. i rechecked the branch lineage and the review context here. this pr is now effectively a traceability branch rather than the landing branch: the integrated fullscreen onboarding/chat path moved to #691, and the current top branch has continued on #866. because of that overlap, and because this branch is still conflicting, i don't want to merge #690 directly and create another reconciliation pass on top of the later work. i'm leaving this open for traceability for now while the active landing path is settled on the later branch. |
|
closing this as outdated. we are redesigning the onboard flow and the tui ui/ux, so this is no longer the direction we want to keep open. we can reopen the work in a fresh pr once the new design is settled. |
Summary
Guided onboarding had grown into a monolithic flow with no explicit workspace or protocol stages, weak back-navigation, and readiness handling that could surface review and write too early or persist values the user never explicitly chose.
First-run setup is the main path to a working CLI. When reruns blur detected versus current values or blocked preflight results do not stop before write, operators lose trust in
loongclaw onboardand can end up with confusing config outcomes.Extracted a shared
OnboardDraftplusOnboardFlowController, made the wizard an explicit eight-stage flow, added interactive workspace and ACP protocol stages, grouped readiness outcomes (ready,ready with warnings,blocked), added plain-prompt fallback for degraded terminals, tightened rerun/file-root/review-write semantics, and aligned README plus product specs with the shipped contract.This PR does not add a permanent dashboard or full TUI shell, does not change the broader install flow, and does not introduce a new config schema or provider catalog surface outside onboarding orchestration and presentation.
Linked Issues
Change Type
Touched Areas
Risk Track
If Track B, fill these in:
Onboarding now owns more of the first-run decision path through a shared controller and explicit readiness gate, so regressions would show up as navigation drift, rerun-state mistakes, or write/rollback mistakes rather than isolated copy issues.
Land on
devonly, rely on the expandedonboard_cliintegration suite plus full workspace gates, and keep existing backup/restore plus post-write verification paths active. Blocked outcomes stop before write; degraded terminals fall back to plain prompts.Revert this PR to restore the prior onboarding path. If a regression is discovered after merge, operators can rerun
loongclaw onboardorloongclaw doctor --fixfrom preserved configs/backups.Validation
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --lockedcargo test --workspace --all-features --lockedCommands and evidence:
Before/after behavior and regression coverage:
tools.file_rootas an explicit persisted value.ready,ready with warnings, orblockedbefore review/write, reruns preserve current-versus-detected labeling, and blank file roots stay implicit unless the user explicitly selects them.crates/daemon/tests/integration/onboard_cli.rsnow covers back-navigation, skip preservation, workspace/protocol steps, readiness grouping, blocked-after-verification reporting, degraded-terminal fallback, and rerun semantics around blank file roots.User-visible / Operator-visible Changes
ready with warnings, or blocked. Warn-only runs require explicit confirmation before write; blocked runs stop before write and point users toloongclaw doctororloongclaw doctor --fix.Failure Recovery
Revert this PR to restore the previous wizard semantics. Runtime/operator recovery stays the same: overwrite paths keep backups and blocked post-write verification preserves the old config.
Guided flow jumping from protocols straight to review, blocked preflight still allowing write confirmation, reruns persisting an explicit blank
tools.file_root, or plain interactive terminals aborting instead of falling back.Adjacent Context
upstream/devbefore the PR was mergeable again.Reviewer Focus
crates/daemon/src/onboard_cli.rs,crates/daemon/src/onboard_flow.rs, andcrates/daemon/src/onboard_state.rsfor step ordering, shared draft ownership, and back/skip semantics.crates/daemon/src/onboard_workspace.rs,crates/daemon/src/onboard_protocols.rs,crates/daemon/src/onboard_preflight.rs, andcrates/daemon/src/onboard_finalize.rsfor persistence, readiness grouping, outcome reporting, and rollback behavior.crates/daemon/tests/integration/onboard_cli.rs,README.md,docs/product-specs/onboarding.md, anddocs/product-specs/doctor.mdfor contract coverage and user-facing alignment.Summary by CodeRabbit
New Features
Improvements
Documentation