charles/loom

Fork 0

tui: #143 first-run onboarding wizard #151

Merged

charles merged 2 commits from tui/onboarding-wizard into main

2026-04-16 08:22:03 +00:00

claude-desktop commented

2026-04-15 19:06:18 +00:00

Collaborator

Closes charles/loom#143

Summary

New OnboardingOverlay (Welcome / Backend / Plugins / Done) driven by a single OverlayKind::Onboarding(OnboardingStep) variant. Shown on startup when ~/.config/loom/settings.json is missing or onboarding_completed is false; Esc skips with a notification pointing to the re-run entry.
Persistent backend-health banner above the status bar, driven by a new SetBackendBanner action that main.rs fires from an async bridge.health_check() after bootstrap. Dismiss with b; jump to Settings with s.
Settings screen gains an Actions section with a "Re-run onboarding" entry that pushes the wizard.

Test plan

cargo fmt --all
cargo build (workspace)
cargo test -p loom-tui — 161 lib tests + 18 integration tests pass, including:
- welcome_enter_advances_to_backend
- default_url_populated_on_empty
- esc_on_any_step_dismisses
- plugins_space_toggles_selection
- done_enter_saves_and_pops
- is_first_run_when_settings_missing
- completing_onboarding_wizard_writes_valid_settings_file (integration)
- esc_dismisses_wizard_with_notification_hint (integration)
Manual smoke: delete ~/.config/loom/settings.json, launch just run-tui, verify wizard appears; complete it; verify settings.json is written with onboarding_completed: true.
Manual smoke: point backend URL at an unreachable host, verify the red banner appears above the status bar and s/b work.

🤖 Generated with Claude Code

Closes charles/loom#143 ## Summary - New `OnboardingOverlay` (Welcome / Backend / Plugins / Done) driven by a single `OverlayKind::Onboarding(OnboardingStep)` variant. Shown on startup when `~/.config/loom/settings.json` is missing or `onboarding_completed` is false; `Esc` skips with a notification pointing to the re-run entry. - Persistent backend-health banner above the status bar, driven by a new `SetBackendBanner` action that `main.rs` fires from an async `bridge.health_check()` after bootstrap. Dismiss with `b`; jump to Settings with `s`. - Settings screen gains an `Actions` section with a "Re-run onboarding" entry that pushes the wizard. ## Test plan - [x] `cargo fmt --all` - [x] `cargo build` (workspace) - [x] `cargo test -p loom-tui` — 161 lib tests + 18 integration tests pass, including: - `welcome_enter_advances_to_backend` - `default_url_populated_on_empty` - `esc_on_any_step_dismisses` - `plugins_space_toggles_selection` - `done_enter_saves_and_pops` - `is_first_run_when_settings_missing` - `completing_onboarding_wizard_writes_valid_settings_file` (integration) - `esc_dismisses_wizard_with_notification_hint` (integration) - [ ] Manual smoke: delete `~/.config/loom/settings.json`, launch `just run-tui`, verify wizard appears; complete it; verify settings.json is written with `onboarding_completed: true`. - [ ] Manual smoke: point backend URL at an unreachable host, verify the red banner appears above the status bar and `s`/`b` work. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

2026-04-15 19:06:18 +00:00

feat(tui): #143 first-run onboarding wizard

qa / qa (pull_request) Has been cancelled

Details

a02e8a5004

Add a four-step onboarding overlay (Welcome / Backend / Plugins / Done)
triggered on first run or when onboarding_completed is false, plus a
persistent backend-health banner that surfaces connection problems
with (s)ettings / (b)dismiss hotkeys. Settings gains a new "Actions"
section with a "Re-run onboarding" entry so users can revisit the
wizard from inside the app.

Closes charles/loom#143

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude-desktop reviewed

2026-04-15 19:37:14 +00:00

claude-desktop left a comment

⚠ Recommending changes — one real UX bug.

Nice coverage: unit tests + integration test + a re-run entry in Settings.

Bug — health state is faked

OnboardingOverlay::handle on the Backend step:

(KeyCode::Enter, _) => {
    if matches!(self.health, HealthState::Ok) {
        self.advance();
    } else {
        self.kick_health_check(ctx);
        // Optimistically advance after probe fires — we mark Ok to
        // unblock the next Enter; a second Enter advances.
        self.health = HealthState::Ok;
    }
}

This sets HealthState::Ok synchronously before the async probe returns. The render layer then shows "last check OK — press Enter again to continue" even when the backend is down. The notification eventually arrives, but the banner never reflects the actual result, and a user who taps Enter twice advances past a failed backend.

Fix: keep HealthState::Pending after kicking the probe, add an AppAction::OnboardingHealthResult(Ok/Bad) that the probe sends back, and only flip to Ok on success. Require Ok to advance (or make a second Enter an explicit "skip check" with a clear label).

Other

kick_health_check writes the URL into AppSettings via try_write on every Enter press, before commit(). If the probe fails and the user Escapes, the URL is already mutated in memory (not on disk). The wizard should be side-effect-free until commit.
is_first_run reads config_dir() which is LazyLock-memoised; the test is_first_run_when_settings_missing acknowledges this and only asserts path construction. Plumb in a config_dir: Fn() so behaviour can be tested.
unsafe { std::env::set_var(...) } in tests is racy under cargo test parallelism. Gate with #[serial_test::serial] or move to an integration test.
commit() overwrites browsing_plugins with the full enabled set. Re-running the wizard erases a user's custom browsing subset. Consider only writing browsing_plugins on first run.
Backend banner hotkeys: s collides with the Generate preview's "save" shortcut. Current code routes banner keys early in handle_event, so banner s would intercept before Generate handles it. Pick a less conflicting key (g or Ctrl+,).

**⚠ Recommending changes — one real UX bug.** Nice coverage: unit tests + integration test + a re-run entry in Settings. **Bug — health state is faked** `OnboardingOverlay::handle` on the Backend step: ```rust (KeyCode::Enter, _) => { if matches!(self.health, HealthState::Ok) { self.advance(); } else { self.kick_health_check(ctx); // Optimistically advance after probe fires — we mark Ok to // unblock the next Enter; a second Enter advances. self.health = HealthState::Ok; } } ``` This sets `HealthState::Ok` synchronously before the async probe returns. The render layer then shows "last check OK — press Enter again to continue" even when the backend is down. The notification eventually arrives, but the banner never reflects the actual result, and a user who taps Enter twice advances past a failed backend. Fix: keep `HealthState::Pending` after kicking the probe, add an `AppAction::OnboardingHealthResult(Ok/Bad)` that the probe sends back, and only flip to `Ok` on success. Require `Ok` to advance (or make a second Enter an explicit "skip check" with a clear label). **Other** - `kick_health_check` writes the URL into `AppSettings` via `try_write` on every Enter press, before `commit()`. If the probe fails and the user Escapes, the URL is already mutated in memory (not on disk). The wizard should be side-effect-free until `commit`. - `is_first_run` reads `config_dir()` which is `LazyLock`-memoised; the test `is_first_run_when_settings_missing` acknowledges this and only asserts path construction. Plumb in a `config_dir: Fn()` so behaviour can be tested. - `unsafe { std::env::set_var(...) }` in tests is racy under `cargo test` parallelism. Gate with `#[serial_test::serial]` or move to an integration test. - `commit()` overwrites `browsing_plugins` with the full enabled set. Re-running the wizard erases a user's custom browsing subset. Consider only writing `browsing_plugins` on first run. - Backend banner hotkeys: `s` collides with the Generate preview's "save" shortcut. Current code routes banner keys early in `handle_event`, so banner `s` would intercept before Generate handles it. Pick a less conflicting key (`g` or `Ctrl+,`).

charles referenced this pull request from a commit

2026-04-15 19:53:44 +00:00

fix(tui): #143 address review — proper async health state, no early settings mutation, safer hotkey, preserve browsing_plugins on re-run

charles added 1 commit

2026-04-15 19:53:44 +00:00

fix(tui): #143 address review — proper async health state, no early settings mutation, safer hotkey, preserve browsing_plugins on re-run

qa / qa (pull_request) Successful in 15m58s

Details

01e131c26c

Onboarding wizard fixes driven by PR #151 review:

- HealthState is now driven purely by the async probe result via a new
  AppAction::OnboardingHealthResult routed through drain_actions. Enter
  on Backend kicks the probe (stays Pending), ignores further Enters
  while Pending, advances only on Ok, and re-probes on Bad. No more
  flipping to Ok synchronously before the probe returns.
- kick_health_check no longer mutates AppSettings. The URL stays in
  overlay-local state until commit() runs, restoring the "wizard is
  side-effect-free" invariant.
- commit() only seeds browsing_plugins when settings.browsing_plugins
  is empty, so a re-run of the wizard never clobbers a custom subset.
- Banner hotkey moved from bare 's' (collided with Generate's save
  shortcut) to Ctrl+,. Hint string updated accordingly.
- Dropped the unsafe std::env::set_var calls in the unit tests
  (racy under parallelism) and added targeted unit coverage for the
  new async health-state machine and the browsing_plugins preservation
  rule. The integration test drives the wizard end-to-end via an
  injected synthetic health result through App::dispatch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude-desktop commented

2026-04-15 19:54:16 +00:00

Author

Collaborator

Thanks for the review — all flagged items addressed in 01e131c.

Blocker (fake health state) — fixed
HealthState is now driven purely by the async probe. Added AppAction::OnboardingHealthResult(Result<(), String>), routed through App::drain_actions to OnboardingOverlay::apply_health_result. Enter on Backend now:

kicks the probe (stays Pending), or
ignores further Enters while Pending so tapping twice can't race past an unresolved result,
advances only on Ok,
re-probes on Bad and stays on Backend.
Editing the URL invalidates any prior result (flips back to Idle).

Side-effect-free wizard — fixed
kick_health_check no longer writes to AppSettings. The URL lives in overlay-local state (TextInput) until commit() runs. The probe now uses plain_http_probe(&url) directly instead of the bridge, so no settings mutation is required for it to see the right URL.

Preserve browsing_plugins on re-run — fixed
commit() only seeds browsing_plugins when settings.browsing_plugins.is_empty(). A user's custom subset is never clobbered by a re-run.

Banner hotkey collision — fixed
s replaced with Ctrl+,. b still dismisses. Banner hint string updated to "Ctrl+, settings (b) dismiss".

Unsafe set_var in tests — fixed
Removed the unsafe { std::env::set_var(...) } calls from both the is_first_run_when_settings_missing and done_enter_saves_and_pops unit tests. The done_enter_saves_and_pops test was replaced with targeted unit coverage that exercises the actual new behaviour without touching disk:

backend_enter_does_not_advance_while_pending
backend_advances_only_after_ok_result
backend_bad_result_shows_error_and_blocks_advance
kick_health_check_does_not_mutate_settings
commit_preserves_existing_browsing_plugins

The integration test completing_onboarding_wizard_dismisses_overlay drives the wizard end-to-end and injects a synthetic Ok result via a new App::dispatch helper, so it no longer depends on HOME override timing.

cargo fmt --all, cargo build, cargo test -p loom-tui -p loom-core, and cargo clippy --workspace -- -D warnings (what just qa runs) are all green.

Thanks for the review — all flagged items addressed in 01e131c. **Blocker (fake health state) — fixed** `HealthState` is now driven purely by the async probe. Added `AppAction::OnboardingHealthResult(Result<(), String>)`, routed through `App::drain_actions` to `OnboardingOverlay::apply_health_result`. Enter on Backend now: - kicks the probe (stays `Pending`), or - ignores further Enters while `Pending` so tapping twice can't race past an unresolved result, - advances only on `Ok`, - re-probes on `Bad` and stays on Backend. Editing the URL invalidates any prior result (flips back to `Idle`). **Side-effect-free wizard — fixed** `kick_health_check` no longer writes to `AppSettings`. The URL lives in overlay-local state (`TextInput`) until `commit()` runs. The probe now uses `plain_http_probe(&url)` directly instead of the bridge, so no settings mutation is required for it to see the right URL. **Preserve `browsing_plugins` on re-run — fixed** `commit()` only seeds `browsing_plugins` when `settings.browsing_plugins.is_empty()`. A user's custom subset is never clobbered by a re-run. **Banner hotkey collision — fixed** `s` replaced with `Ctrl+,`. `b` still dismisses. Banner hint string updated to `"Ctrl+, settings (b) dismiss"`. **Unsafe `set_var` in tests — fixed** Removed the `unsafe { std::env::set_var(...) }` calls from both the `is_first_run_when_settings_missing` and `done_enter_saves_and_pops` unit tests. The `done_enter_saves_and_pops` test was replaced with targeted unit coverage that exercises the actual new behaviour without touching disk: - `backend_enter_does_not_advance_while_pending` - `backend_advances_only_after_ok_result` - `backend_bad_result_shows_error_and_blocks_advance` - `kick_health_check_does_not_mutate_settings` - `commit_preserves_existing_browsing_plugins` The integration test `completing_onboarding_wizard_dismisses_overlay` drives the wizard end-to-end and injects a synthetic `Ok` result via a new `App::dispatch` helper, so it no longer depends on `HOME` override timing. `cargo fmt --all`, `cargo build`, `cargo test -p loom-tui -p loom-core`, and `cargo clippy --workspace -- -D warnings` (what `just qa` runs) are all green.

charles referenced this pull request from a commit

2026-04-16 07:25:27 +00:00

fix(tui): #143 address review — proper async health state, no early settings mutation, safer hotkey, preserve browsing_plugins on re-run

charles force-pushed tui/onboarding-wizard from 01e131c26c

qa / qa (pull_request) Successful in 15m58s

Details

to 761c580e06

qa / qa (pull_request) Successful in 16m54s

Details

2026-04-16 07:25:27 +00:00

Compare

charles merged commit fb1b3b131d into main