Skip to content

refactor(tui): decompose init wizard into composable step classes#432

Merged
Aaronontheweb merged 13 commits into
devfrom
claude-wt-ui-refactor
Mar 27, 2026
Merged

refactor(tui): decompose init wizard into composable step classes#432
Aaronontheweb merged 13 commits into
devfrom
claude-wt-ui-refactor

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Introduces composable wizard step abstractions (IWizardStepViewModel, IWizardStepView, WizardOrchestrator) to replace the monolithic InitWizardViewModel (1,448 lines) + InitWizardPage (1,933 lines)
  • Typed WizardConfigBuilder replaces 200-line manual dictionary assembly in WriteConfig()
  • NavigationDirection-aware OnEnter fixes back-navigation bug where sub-steps always reset to 0 instead of resuming at the last completed sub-step
  • Steps will be extracted incrementally from the monolith in subsequent commits

Motivation

Preparing for open source — adding a new channel (Discord) or provider currently requires touching both monolith files end-to-end. After this refactor, adding Discord = creating 2 new files + registering in step list.

Test plan

  • Phase 1: Core abstractions compile, 30 new orchestrator/config tests pass
  • 54 existing InitWizardViewModelTests still pass
  • Phase 2: Extract SecurityPosture step (proof-of-concept)
  • Phase 3: Extract Search + BrowserAutomation steps
  • Phase 4: Extract ChatServices (Slack) step
  • Phase 5: Extract Provider step
  • Phase 6: Extract Identity + HealthCheck + Channels, delete monolith
  • Manual: netclaw init full wizard flow works end-to-end
  • Manual: back-navigation resumes at last sub-step (not sub-step 0)

Add IWizardStepViewModel, IWizardStepView, WizardContext, WizardConfigBuilder,
HealthCheckRunner, and WizardOrchestrator as the foundation for decomposing
the monolithic InitWizardViewModel (1,448 lines) and InitWizardPage (1,933 lines)
into self-contained, extensible step classes.

Key design decisions:
- NavigationDirection-aware OnEnter fixes back-nav bug where sub-steps reset to 0
- Typed config sections replace 200-line manual dictionary assembly
- Step ordering via explicit list replaces enum arithmetic with conditional skipping
- Upfront instantiation preserves back-navigation state for free

No changes to existing files — abstractions sit alongside the monolith.
30 new tests pass, 54 existing tests unaffected.
First step extracted from the monolithic InitWizardViewModel/Page into the
new composable step architecture:

- SecurityPostureStepViewModel: owns posture state, derives shell mode,
  contributes Security and Tools config sections
- SecurityPostureStepView: owns SelectionList component, wires selection
  to VM state and step advancement callback
- StepViewCallbacks: replaces separate WireSubscriptions method with a
  single context object passed to BuildContent (includes AdvanceStep,
  InvalidateContent, InvalidateHelp, Subscriptions)

Also fixes IWizardStepView interface: Color is Termina.Terminal.Color,
DisposeWith is from Termina.Reactive.

12 new step tests, 42 total wizard tests pass.
Search step: 2 sub-steps (backend selection → credentials for Brave/SearXNG).
DuckDuckGo selection completes immediately. Contributes Search config section
and Brave API key to secrets.

BrowserAutomation step: 2 sub-steps (enable/disable → backend selection).
Disabled selection completes immediately. Contributes McpServers config.
Chrome DevTools availability detected at construction time.

Both steps implement direction-aware OnEnter for back-navigation resume.
22 new step tests, 119 total tests pass.
SlackStepViewModel: 7 sub-steps (enable → bot token → app token →
channel names → DM enabled → allowed user IDs → owner identity).
Contributes Slack config + secrets sections. Runs Slack probe and
channel resolution health checks.

SlackStepView: layout builders for each sub-step with selection lists
and text inputs. Validates token prefixes (xoxb-/xapp-).

Key design for multi-channel support:
- DisplayTitle is "Slack" (not "Chat Services") — Discord will be a peer step
- OnLeave uses additive context.AnyChatServicesEnabled |= SlackEnabled
  so multiple channel steps can coexist without overwriting each other
- Channels step reads context.AnyChatServicesEnabled for IsApplicable

15 new Slack step tests pass.
ProviderStepViewModel: 7 sub-steps (provider selection → auth method →
credentials → validation/probe → model selection → OAuth device → OAuth browser).
Owns all reactive probe state (IsProbing, ProbeResult, ProbeElapsedSeconds).
Non-linear sub-step transitions handled via SetSubStep for OAuth branches
and probe auto-advance. Reuses OAuthFlowCoordinator.

ProviderStepView: All 7 sub-step layouts including provider selection from
registry, auth method labels from OAuthFlowViews, credential inputs,
validation spinner, model selection (list + manual entry), and both
OAuth flow displays.

14 new Provider step tests pass.
Identity step: 5 sub-steps (agent name → comm style → user name →
timezone → webhook URL). Owns identity file generation (SOUL.md, AGENTS.md).
Contributes Identity and Notifications config sections.

Channels step: conditional (IsApplicable checks AnyChatServicesEnabled).
Custom keyboard nav (arrow keys for audience cycling, a/d add/remove).
Populates ChannelEntries on forward entry with posture-based defaults.

HealthCheck step: final wizard step that runs health checks from all steps,
writes config via orchestrator.WriteConfig(), starts daemon, and navigates
to chat on success. Owns daemon lifecycle management.

All 8 wizard steps are now extracted as composable classes.
14 new tests, all existing tests unaffected.

Note: monolith is not deleted yet — the Page rewiring to use the
orchestrator will be done in a follow-up with manual testing.
Critical fixes:
- Wire IdentityStepViewModel.WriteIdentityFiles() and SeedBuiltInAgents()
  into orchestrator's WriteConfig finalization path
- Restore full AGENTS.md content (was truncated to ~10 lines, now matches
  monolith's ~130 lines with all guidance sections)
- Add TOOLING.md generation and BuildOnboardingTrigger()
- Move ProviderCredentialWriter.WriteProvider() from ContributeSecrets
  (which wrote eagerly to disk) to WriteProviderCredentials() called
  by orchestrator during finalization only

Should-fix:
- WizardContext implements IDisposable, disposes StatusMessage
- Explicit _currentSubStep = 0 on Forward entry for Search,
  BrowserAutomation, Slack, Identity steps
- Remove dead channelAudience variable from ChannelsStepViewModel
- Remove ChannelsStepView.SetContext leak — flow ChannelEntries and
  SelectedPosture through VM properties instead
- Fix async-without-await in HealthCheckStepViewModel.RunHealthCheckAsync
- Fix GoBack/GoNext nav bug — capture currentIdx before
  OnLeave/RebuildActiveSteps can shift the index
- Remove Task.Delay(200) artificial latency from provider health checks
- Remove 7 trivial constant-assertion tests

154 tests pass (all existing + new wizard tests).

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental review

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread src/Netclaw.Cli/Tui/Wizard/WizardContext.cs Outdated
/// Null for fresh init. When populated, steps should pre-populate
/// their fields from the existing config. (Deferred — not implemented yet.)
/// </summary>
public Dictionary<string, object>? ExistingConfig { get; init; }

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't implemented yet, but we should probably give the user the option, if we detect that they have existing config already, on whether or not they want to start fresh or modify what they already have. If the user says start fresh, we don't wipe out untl they run the validate stage at the end.

Change WizardContext.ChannelEntries from List<ChannelEntry> to
Dictionary<string, List<ChannelEntry>> keyed by channel source
("slack", "discord", etc.).

Each channel step populates its own bucket in OnLeave:
- SlackStepViewModel populates context.ChannelEntries["slack"] with
  DM entries (audience derived from posture + user count) and channel
  entries from ChannelNamesInput
- When Slack is disabled, its bucket is removed

ChannelsStepViewModel exposes:
- AllEntries: flattened view across all sources for rendering
- AddEntry/RemoveEntry: source-aware mutations
- GetSource: lookup which platform owns an entry

This enables Discord to populate context.ChannelEntries["discord"]
independently, with DM rows distinguishable per-platform.

Also documents re-edit UX intent per PR comment: detect existing
config → offer fresh vs modify → don't wipe until validate passes.

157 tests pass.
Replace string keys ("slack", "discord") with the existing ChannelType
enum from Netclaw.Actors.Channels. Prevents typo-based mismatches and
makes the dictionary type-safe.

Dictionary<string, List<ChannelEntry>> → Dictionary<ChannelType, List<ChannelEntry>>

ChannelsStepViewModel.AddEntry/GetSource now take ChannelType parameters.
All consumers (SlackStepViewModel, ChannelsStepView, tests) updated.
Replace raw strings with strongly typed enums throughout:

- ChannelEntry.Audience: string → TrustAudience enum
  Removes hardcoded AudienceValues string array in ChannelsStepView;
  now derives from TrustAudience enum values. ToWireValue() used only
  at serialization boundaries.

- SearchConfig.Backend: string → SearchBackend enum (new)
  New SearchBackend enum (DuckDuckGo, Brave, SearXng) with ToWireValue()
  extension. Replaces magic string comparisons across SearchStepViewModel,
  SearchStepView, WizardConfigBuilder, and daemon Program.cs.

- BrowserAutomationMcpProfiles: string constants → BrowserAutomationBackend enum (new)
  Create() now takes BrowserAutomationBackend enum. Default case throws
  ArgumentOutOfRangeException instead of silently falling back to
  Chrome DevTools (fixes CLAUDE.md "no silent fallbacks" violation).

- IBrowserAutomationBootstrapper.EnsureReadyAsync: string → BrowserAutomationBackend

All 22 affected files updated. Both CLI and daemon build clean.
157 wizard tests pass.
Replace the monolithic InitWizardViewModel (1,448 → 219 lines) and
InitWizardPage (1,933 → 356 lines) with thin wrappers around the
WizardOrchestrator and composable step views.

InitWizardViewModel now:
- Creates all 8 step VMs, views, context, and orchestrator in constructor
- Exposes Orchestrator, StepViews, Context for the Page
- GoNext/GoBack delegate to orchestrator
- Health check triggers RunWithOrchestrator for config finalization

InitWizardPage now:
- BuildStepContent delegates to current step's IWizardStepView
- BuildHelpText uses step.GetHelpText()
- HandleKeyPress routes to step view, with special cases for channels
  keyboard nav, provider clipboard/retry, and health check enter
- Subscribes to provider ProbeResult/SpinnerTick/OAuth for auto-advance

Deleted:
- WizardStep enum (replaced by step IDs)
- All step-specific build methods (20+ methods)
- All sub-step tracking fields
- All input routing switch statements
- InitWizardViewModelTests (behavior covered by step-specific tests)

Net: -4,376 lines deleted, +295 added.
101 tests pass. Both CLI and daemon build clean.
…nges

Root cause: GoNext/GoBack on the ViewModel delegated to the orchestrator
but only step-level changes (CurrentStepIndex subscription) triggered
DynamicLayoutNode invalidation. Sub-step advances within a step changed
internal state but never rebuilt the UI.

Fix: ViewModel exposes OnStepContentChanged callback, fired by both
GoNext and GoBack after any navigation. Page wires it to invalidate
_stepContentNode and _helpTextNode. Simplifies AdvanceStep callback
in StepViewCallbacks to just call GoNext (invalidation is centralized).

Symptoms fixed:
- Enter not advancing through Slack sub-steps (stuck on bot token)
- Escape not working for sub-step back-navigation

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally and it works great

return false;

// Let the step handle internal back-navigation (sub-steps)
if (current.TryGoBack())

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a great improvement over what we have right now, which is kind of lossy

var configBuilder = new WizardConfigBuilder(_context.Paths);
var secretsBuilder = new WizardSecretsBuilder(_context.Paths);

foreach (var step in _allSteps)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a lot better than what we had before also - accumulating config + secrets modularly per-step

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant