feat(onboard): clean-slate rewrite — schema-driven, idempotent, DRY#5960
Conversation
github-actions
Bot
added
docs
|
@WareWolf-MoonWall requesting a review. It's still in Draft form, but it's very testable and seems to work decently well end-to-end in both CLI and TUI mode (they use the same source of truth). Tests need to be written, but check out that beautiful red line diff. |
PR review — agent build/compile verificationComprehension summary. PR #5960 replaces the 8,534-line Status note. PR is in draft and touches several high-risk paths per Validation battery run locally on
CI on the PR is green (all 12 Quality Gate checks including Build x86_64-linux, aarch64-darwin, x86_64-windows, Test, Security, Benchmarks Compile). Security & performance assessment.
Findings[question] R3 — 4 pre-existing test failures surfaced. All four call [suggestion] Model-id fallback hints are one release behind. [suggestion] Deprecation error messages should name the replacement. src/main.rs:1635-1640 ( [suggestion] Legacy [suggestion] Missing labels. PR body requests [question] CLI functionality removal vs. "temporarily disabled". Architectural observations
Verdict — Needs deeper maintainer reviewThis PR builds and passes every lint/build gate cleanly on this reviewer's hardware. No PR-caused test failures; the four provider test failures are pre-existing on master. Architecture is sound and consistent with the project's stated constraints. However:
@JordanTheJet — requesting maintainer review for merge readiness, label assignment ( Thanks @singlerider for the thorough PR body, validation evidence, and scoped-out rollback discipline. Generated by the ZeroClaw PR review agent. Build + test evidence captured locally at |
|
The CLI and TUI both have identical UX and sources of truth. TUIDetects and honors previous configurations
Fetches available models, if failed, allows user to enter themselves; nothing hard-coded
Allows configuring "advanced" settings, allows for going back in nav with Esc
Better help text for users to understand every single setting, auto-populate defaults
CLIRecognizes previous configs
Allows for
|
singlerider
requested review from
JordanTheJet and
theonlyhennygod
as code owners
singlerider
force-pushed
the
feat/onboard-rewrite
branch
3 times, most recently
from
6a906fc to
ecd71d1
Compare
JordanTheJet
left a comment
JordanTheJet
left a comment
There was a problem hiding this comment.
Review — feat(onboard): clean-slate rewrite
Net −10,626 lines, trait-driven, schema-fed, idempotent. This is the cleanest way out of the wizard-drift mess #5951 catalogued, and the execution lives up to the PR body.
Reviewed at HEAD 03b29ace. I read the full diff, cross-checked the new orchestrator against the local worktree, and ran validation on a fresh PR checkout. The entire protocol battery passes:
cargo fmt --all -- --check→ cleancargo clippy --workspace --all-targets -- -D warnings→ 0 warningscargo test -p zeroclaw-runtime --lib onboard::→ 13/13 passedcargo test -p zeroclaw-providers --lib flatten_system_messages→ 4/4 passed (the R3 finding from JordanTheJet's earlier agent pass is now fully RESOLVED ✅ via commit5eccff3e)
What's great
✅ The orchestrator is genuinely thin. crates/zeroclaw-runtime/src/onboard/mod.rs is 1,203 lines (including 250+ lines of tests) replacing an 8,534-line monolith, and every mutation funnels through persist(cfg, path, value) → cfg.set_prop(path, value) → cfg.save().await. No direct struct-field assignment anywhere in the flow. That's the DRY invariant the #5951 ADR demanded, and it's actually enforced by construction — not by convention.
✅ Nav / SkipNav state machine is the right abstraction. Two tiny enums propagate Esc upward — local rewind inside a section, section-level rewind inside run_all, program exit from the first prompt. Clean, trivially testable, and it eliminates the ad-hoc "go back" callback noise the old wizard accrued.
✅ Persist-after-every-write is the correct UX call. onboard/mod.rs:126-134 — Ctrl+C mid-flow leaves prior answers on disk and re-running onboard picks up where the user bailed. The idempotency tests (providers_second_run_no_flags_is_idempotent_on_disk, channels_done_selection_is_idempotent_on_disk, workspace_double_run_is_idempotent_on_disk) assert byte-identical config.toml across two runs, which is the strongest guarantee you can give against the drift class.
✅ BASELINE_* constants plus per-family default_*() trait overrides. crates/zeroclaw-api/src/provider.rs:298-362 gives every caller a typed, named fallback without magic numbers at the call site. The rule in CONTRIBUTING.md ("override ONLY where your family differs from baselines — DRY violation otherwise") is exactly the right norm to lay down in the same PR that establishes the shape.
✅ OnboardUi trait placement. Living in zeroclaw-config::traits next to ChannelConfig respects the existing dependency graph (channels/runtime/tui already depend on zeroclaw-config) and means no new crate. Async-by-default is the right forward bet given the gateway/WebSocket backend reasoning in crates/zeroclaw-config/src/traits.rs:142-157.
✅ Skip-gate design. onboard/mod.rs:321-377 — OR of marker AND signal, with --force bypass. The section_has_signal switch correctly handles the per-section nuances: memory/tunnel are marker-only because their defaults (sqlite/none) are valid user choices indistinguishable from untouched state, and the channels.cli: bool false-positive is explicitly documented and guarded by "require a nested channels.<x>.<y>" at onboard/mod.rs:362-370. The code comment names why the naïve starts_with("channels.") doesn't work — exactly the kind of reasoning that'll save the next contributor.
✅ secret(has_current=true) defaults to keep-current. onboard/ui/term.rs:55-65 and the idempotency contract in traits.rs:172-185. Strict improvement against accidental key wipe on reconfigure; --force is the explicit re-entry. Worth the PR by itself.
✅ Zero unwrap() / expect() in production onboard/* code. Verified by grep — every match is inside #[cfg(test)].
✅ Docstring rewrite discipline. The /// comments on ModelProviderConfig, WorkspaceConfig, HardwareConfig, MemoryConfig, TunnelConfig fields no longer restate the field name. Defaults are surfaced separately in the prompt label ("timeout-secs (default: 120)"), so the docstring says why the field exists not what it is. That's the contribution-culture norm (FND-005 §"specificity > generic") applied to config docs.
Findings
🔵 TermUi::editor comment and return disagree. crates/zeroclaw-runtime/src/onboard/ui/term.rs:104-108:
// Editor close-without-save returns None — treat as Back so users
// who bail out of $EDITOR can rewind instead of accepting the
// unchanged buffer silently.
match Editor::new().edit(&buffer)? {
Some(edited) => Ok(Answer::Value(edited)),
None => Ok(Answer::Value(fallback)), // ← returns Value, not Back
}The comment promises Answer::Back; the code returns Answer::Value(fallback) — exactly the "accept unchanged buffer silently" case the comment says it's preventing. Zero behavioral impact today because OnboardUi::editor has no callers in the current diff (ui.editor( grep is empty), so this is a "fix the comment or the code before the first caller lands" marker, not a block.
🟡 CLI deprecation surface is one release of muscle memory. The --*-only flag shim at src/main.rs:1089-1121 prints a clean warning: --<flag>-only is deprecated; use zeroclaw onboard <section> instead to stderr and still routes to the right section. The zeroclaw models bail now names the replacement ("use zeroclaw onboard providers for the live model picker, or check providers.models.<name>.model in your config.toml") — that's the R1 from JordanTheJet's earlier pass, RESOLVED ✅. Flagging only that the v0.7.4 release notes should call this out under CLI changes so users grep-updating scripts have a single place to discover the rename.
🟡 Provider trait signature break cascades are fully contained in-tree but change the Beta contract. temperature: f64 → Option<f64> affects every Provider impl in crates/zeroclaw-providers/src/**, every caller (agent loop, delegate, swarm, gateway, channels orchestrator, memory consolidation, live tests), and benches/agent_benchmarks.rs. All updated; workspace builds with -D warnings. For in-tree callers this is a non-event. Out-of-tree Provider implementors (none known to me) would need to migrate — and because zeroclaw-providers is Beta per AGENTS.md §Stability Tiers, "breaking changes permitted in MINOR with changelog notes" applies. Worth a line in the v0.7.4 changelog explicitly naming the trait break.
🔵 Minor — prompt_field label decoration. onboard/mod.rs:207-212 builds "<short> (default: <d>)" for the unset+has-default case but the "Default: <d>. Press Enter to accept." tail in the help text is redundant once the label carries the default inline. Cosmetic — users will understand either way, and the help text is only shown when ui.note(_) is called. Leave as-is unless the ratatui layout starts to feel cluttered; this is preference.
Architectural callouts
- Ratatui backend is drawing-only. crates/zeroclaw-tui/src/onboarding.rs implements
OnboardUiwith a six-region layout (banner, breadcrumb, log, help, input, hints) and every flow decision lives above the trait. That's the separation the ADR demanded, and it makes a future gateway-WebSocketOnboardUidrop-in — which is the stated design goal for the async signature. - Channels/tunnel section picker derives from schema via
init_defaultsprobe. onboard/mod.rs:710-722 — feature-gated channels drop out of the picker automatically because they never appear inprop_fields()after the probe. No hard-coded channel list to drift from reality. doctor modelswas restored in commitecd71d16. The earlier concern about therun_modelsdeletion is now moot;Commands::Doctor { doctor_command: Some(DoctorCommands::Models { … }) }still routes todoctor::run_models(&config, provider.as_deref(), use_cache)on the PR head.
Scope / risk
- 13 tests directly exercise acceptance criteria from #5951 (
section_has_signal_*,skip_gate_*,mark_completed_is_dedupe_safe, double-run idempotency across workspace/channels/providers, flags-driven providers path). - Config file format is 100% backward compatible — same TOML, same keys, no migration.
[onboard_state]is#[serde(default)], so rolling back to the old binary on a config written by the new binary is safe. - No new network calls, no new permissions, no new FS scope.
models.devfetch was already in-tree pre-PR.
Verdict — APPROVE ✅
This is the scorched-earth rewrite #5951 asked for, executed cleanly. Architecture matches the ADR, validation is green across fmt / clippy / the targeted test slices, idempotency is proven in tests (not just asserted in prose), and all four open findings from JordanTheJet's earlier agent pass are resolved on HEAD. The editor comment/code mismatch is the only real nit, and it's in unused code.
Per AGENTS.md §Risk Tiers this touches crates/zeroclaw-runtime/src/**, crates/zeroclaw-tools/src/**, and crates/zeroclaw-gateway/src/** — the high-risk triad — but the change is surgical at each boundary (temperature arg widened, onboard entry points swapped, doctor::run_models rewired), not a policy shift. Labels in the PR body request risk: medium, size: xl, scope: core/onboard/provider — please apply those before merge for queue hygiene.
Thanks for the PR body discipline, @singlerider — the linked-issue map, the explicit scope boundary, the acceptance-criteria test list, and the screenshot pair made review genuinely faster. This is the shape good high-risk PRs take.
— WareWolf-MoonWall
JordanTheJet
left a comment
JordanTheJet
left a comment
There was a problem hiding this comment.
Review — feat(onboard): clean-slate rewrite
Net −10,626 lines, trait-driven, schema-fed, idempotent. This is the cleanest way out of the wizard-drift mess #5951 catalogued, and the execution lives up to the PR body.
Reviewed at HEAD 03b29ace. I read the full diff, cross-checked the new orchestrator against the local worktree, and ran validation on a fresh PR checkout. The entire protocol battery passes:
cargo fmt --all -- --check→ cleancargo clippy --workspace --all-targets -- -D warnings→ 0 warningscargo test -p zeroclaw-runtime --lib onboard::→ 13/13 passedcargo test -p zeroclaw-providers --lib flatten_system_messages→ 4/4 passed (the R3 finding from JordanTheJet's earlier agent pass is now fully RESOLVED ✅ via commit5eccff3e)
What's great
✅ The orchestrator is genuinely thin. crates/zeroclaw-runtime/src/onboard/mod.rs is 1,203 lines (including 250+ lines of tests) replacing an 8,534-line monolith, and every mutation funnels through persist(cfg, path, value) → cfg.set_prop(path, value) → cfg.save().await. No direct struct-field assignment anywhere in the flow. That's the DRY invariant the #5951 ADR demanded, and it's actually enforced by construction — not by convention.
✅ Nav / SkipNav state machine is the right abstraction. Two tiny enums propagate Esc upward — local rewind inside a section, section-level rewind inside run_all, program exit from the first prompt. Clean, trivially testable, and it eliminates the ad-hoc "go back" callback noise the old wizard accrued.
✅ Persist-after-every-write is the correct UX call. onboard/mod.rs:126-134 — Ctrl+C mid-flow leaves prior answers on disk and re-running onboard picks up where the user bailed. The idempotency tests (providers_second_run_no_flags_is_idempotent_on_disk, channels_done_selection_is_idempotent_on_disk, workspace_double_run_is_idempotent_on_disk) assert byte-identical config.toml across two runs, which is the strongest guarantee you can give against the drift class.
✅ BASELINE_* constants plus per-family default_*() trait overrides. crates/zeroclaw-api/src/provider.rs:298-362 gives every caller a typed, named fallback without magic numbers at the call site. The rule in CONTRIBUTING.md ("override ONLY where your family differs from baselines — DRY violation otherwise") is exactly the right norm to lay down in the same PR that establishes the shape.
✅ OnboardUi trait placement. Living in zeroclaw-config::traits next to ChannelConfig respects the existing dependency graph (channels/runtime/tui already depend on zeroclaw-config) and means no new crate. Async-by-default is the right forward bet given the gateway/WebSocket backend reasoning in crates/zeroclaw-config/src/traits.rs:142-157.
✅ Skip-gate design. onboard/mod.rs:321-377 — OR of marker AND signal, with --force bypass. The section_has_signal switch correctly handles the per-section nuances: memory/tunnel are marker-only because their defaults (sqlite/none) are valid user choices indistinguishable from untouched state, and the channels.cli: bool false-positive is explicitly documented and guarded by "require a nested channels.<x>.<y>" at onboard/mod.rs:362-370. The code comment names why the naïve starts_with("channels.") doesn't work — exactly the kind of reasoning that'll save the next contributor.
✅ secret(has_current=true) defaults to keep-current. onboard/ui/term.rs:55-65 and the idempotency contract in traits.rs:172-185. Strict improvement against accidental key wipe on reconfigure; --force is the explicit re-entry. Worth the PR by itself.
✅ Zero unwrap() / expect() in production onboard/* code. Verified by grep — every match is inside #[cfg(test)].
✅ Docstring rewrite discipline. The /// comments on ModelProviderConfig, WorkspaceConfig, HardwareConfig, MemoryConfig, TunnelConfig fields no longer restate the field name. Defaults are surfaced separately in the prompt label ("timeout-secs (default: 120)"), so the docstring says why the field exists not what it is. That's the contribution-culture norm (FND-005 §"specificity > generic") applied to config docs.
Findings
🔵 TermUi::editor comment and return disagree. crates/zeroclaw-runtime/src/onboard/ui/term.rs:104-108:
// Editor close-without-save returns None — treat as Back so users
// who bail out of $EDITOR can rewind instead of accepting the
// unchanged buffer silently.
match Editor::new().edit(&buffer)? {
Some(edited) => Ok(Answer::Value(edited)),
None => Ok(Answer::Value(fallback)), // ← returns Value, not Back
}The comment promises Answer::Back; the code returns Answer::Value(fallback) — exactly the "accept unchanged buffer silently" case the comment says it's preventing. Zero behavioral impact today because OnboardUi::editor has no callers in the current diff (ui.editor( grep is empty), so this is a "fix the comment or the code before the first caller lands" marker, not a block.
🟡 CLI deprecation surface is one release of muscle memory. The --*-only flag shim at src/main.rs:1089-1121 prints a clean warning: --<flag>-only is deprecated; use zeroclaw onboard <section> instead to stderr and still routes to the right section. The zeroclaw models bail now names the replacement ("use zeroclaw onboard providers for the live model picker, or check providers.models.<name>.model in your config.toml") — that's the R1 from JordanTheJet's earlier pass, RESOLVED ✅. Flagging only that the v0.7.4 release notes should call this out under CLI changes so users grep-updating scripts have a single place to discover the rename.
🟡 Provider trait signature break cascades are fully contained in-tree but change the Beta contract. temperature: f64 → Option<f64> affects every Provider impl in crates/zeroclaw-providers/src/**, every caller (agent loop, delegate, swarm, gateway, channels orchestrator, memory consolidation, live tests), and benches/agent_benchmarks.rs. All updated; workspace builds with -D warnings. For in-tree callers this is a non-event. Out-of-tree Provider implementors (none known to me) would need to migrate — and because zeroclaw-providers is Beta per AGENTS.md §Stability Tiers, "breaking changes permitted in MINOR with changelog notes" applies. Worth a line in the v0.7.4 changelog explicitly naming the trait break.
🔵 Minor — prompt_field label decoration. onboard/mod.rs:207-212 builds "<short> (default: <d>)" for the unset+has-default case but the "Default: <d>. Press Enter to accept." tail in the help text is redundant once the label carries the default inline. Cosmetic — users will understand either way, and the help text is only shown when ui.note(_) is called. Leave as-is unless the ratatui layout starts to feel cluttered; this is preference.
Architectural callouts
- Ratatui backend is drawing-only. crates/zeroclaw-tui/src/onboarding.rs implements
OnboardUiwith a six-region layout (banner, breadcrumb, log, help, input, hints) and every flow decision lives above the trait. That's the separation the ADR demanded, and it makes a future gateway-WebSocketOnboardUidrop-in — which is the stated design goal for the async signature. - Channels/tunnel section picker derives from schema via
init_defaultsprobe. onboard/mod.rs:710-722 — feature-gated channels drop out of the picker automatically because they never appear inprop_fields()after the probe. No hard-coded channel list to drift from reality. doctor modelswas restored in commitecd71d16. The earlier concern about therun_modelsdeletion is now moot;Commands::Doctor { doctor_command: Some(DoctorCommands::Models { … }) }still routes todoctor::run_models(&config, provider.as_deref(), use_cache)on the PR head.
Scope / risk
- 13 tests directly exercise acceptance criteria from #5951 (
section_has_signal_*,skip_gate_*,mark_completed_is_dedupe_safe, double-run idempotency across workspace/channels/providers, flags-driven providers path). - Config file format is 100% backward compatible — same TOML, same keys, no migration.
[onboard_state]is#[serde(default)], so rolling back to the old binary on a config written by the new binary is safe. - No new network calls, no new permissions, no new FS scope.
models.devfetch was already in-tree pre-PR.
Verdict — APPROVE ✅
This is the scorched-earth rewrite #5951 asked for, executed cleanly. Architecture matches the ADR, validation is green across fmt / clippy / the targeted test slices, idempotency is proven in tests (not just asserted in prose), and all four open findings from JordanTheJet's earlier agent pass are resolved on HEAD. The editor comment/code mismatch is the only real nit, and it's in unused code.
Per AGENTS.md §Risk Tiers this touches crates/zeroclaw-runtime/src/**, crates/zeroclaw-tools/src/**, and crates/zeroclaw-gateway/src/** — the high-risk triad — but the change is surgical at each boundary (temperature arg widened, onboard entry points swapped, doctor::run_models rewired), not a policy shift. Labels in the PR body request risk: medium, size: xl, scope: core/onboard/provider — please apply those before merge for queue hygiene.
Thanks for the PR body discipline, @singlerider — the linked-issue map, the explicit scope boundary, the acceptance-criteria test list, and the screenshot pair made review genuinely faster. This is the shape good high-risk PRs take.
— WareWolf-MoonWall
JordanTheJet
dismissed stale reviews from themself
Dismissing — posted under wrong voice (WareWolf-MoonWall signature, but authored by JordanTheJet). Re-posting with correct attribution.
WareWolf-MoonWall
left a comment
WareWolf-MoonWall
left a comment
There was a problem hiding this comment.
Review — feat(onboard): clean-slate rewrite
Reviewed at HEAD 03b29ace. Full diff read, cross-checked against local source. Jordan's commendations are well-earned and I won't echo them in full — I'll name the ones that especially stand out, then get to my findings.
What landed well
✅ The thin-dispatcher principle is enforced by construction. Every mutation routes through persist(cfg, path, value) → cfg.set_prop() → cfg.save(). No direct struct-field assignment anywhere in the flow. The DRY invariant #5951 demanded isn't just documented — it's architecturally enforced.
✅ Nav / SkipNav as the navigation contract. Two tiny enums carry the Esc signal upward cleanly — no callbacks, no shared state, no implicit returns. The result is a flow graph you can reason about on a whiteboard. This abstraction is what makes 1,203 lines feel manageable.
✅ Idempotency proven in tests, not prose. providers_second_run_no_flags_is_idempotent_on_disk, channels_done_selection_is_idempotent_on_disk, workspace_double_run_is_idempotent_on_disk all assert byte-identical config.toml across two runs. That's the gold standard for this class of guarantee — property verified at the actual storage layer, not just asserted in the PR body.
✅ BASELINE_* constants and per-family default_*(). Named constants at crates/zeroclaw-api/src/provider.rs:298-316, each family overrides only where it genuinely differs (Anthropic → 1.0, Ollama → deterministic), and CONTRIBUTING.md now names the rule. No more magic numbers scattered through every caller.
✅ secret(has_current=true) defaults to keep-current. Silently wiping an API key on reconfigure was a real pain point. This is a strict UX improvement that stands alone — it ships as a side effect of a much larger change and that's fine.
✅ Zero unwrap()/expect() in production onboard/* code. Verified by grep. Every match is inside #[cfg(test)]. That's the discipline level this code needed.
Findings
🔴 TermUi::editor comment and return disagree — fix required.
crates/zeroclaw-runtime/src/onboard/ui/term.rs lines 103–110:
// Editor close-without-save returns None — treat as Back so users
// who bail out of $EDITOR can rewind instead of accepting the
// unchanged buffer silently.
match Editor::new().edit(&buffer)? {
Some(edited) => Ok(Answer::Value(edited)),
None => Ok(Answer::Value(fallback)), // ← returns Value, not Back
}The comment says "treat as Back." The code returns Answer::Value(fallback) — precisely the "accept unchanged buffer silently" case the comment explicitly says it's preventing.
Jordan rated this 🔵 on the grounds that ui.editor( has no callers in the current diff. That is true today. But a comment that promises the opposite of what the code delivers is a trap for the first contributor who calls this method. The Answer::Back contract is the backbone of the entire navigation model — every section function builds its flow graph around it. Having one method lie about whether it honors that contract is exactly the kind of subtle breakage that causes hard-to-trace UX regressions when the first caller lands, and the "editor" method is the most likely place for a caller to appear next (StringArray fields, system-prompt edit flows).
The fix is a single line:
None => Ok(Answer::Back),I want that in before this merges, not on a follow-up.
🟡 Per-channel smoke tests are an open acceptance criterion from #5951 — open a tracking ticket.
AC #6 from issue #5951: "Per-channel onboard smoke tests with a scripted mock OnboardUi green." The PR body is honest about this: "Deferred follow-up tests: per-channel field-walk smoke tests (picking a specific channel and asserting its fields land via set_prop) — the current channels_done_selection test only exercises the 'pick Done immediately' path."
"Deferred" is fine. But it needs a ticket, not just a sentence in PR prose. Please open a follow-up issue before merge — a one-liner referencing this PR and naming the missing paths is enough. Without a tracking issue this gap disappears into history and the next contributor has no way to know the channel section wasn't fully tested.
🔵 Milestone required — maintainer action.
Both active milestone scope boundaries explicitly exclude zeroclaw-runtime changes:
- v0.7.4: No changes to
zeroclaw-runtime,zeroclaw-gateway,zeroclaw-tools, or security paths. - v0.7.5: Pipeline and tooling only. No new feature work. No runtime changes.
This PR touches crates/zeroclaw-runtime/src/** throughout. It cannot land before v0.7.5 ships. A post-v0.7.5 milestone (v0.7.6, or whatever the team designates for the next runtime release) needs to be set before merge.
🔵 Risk label — make a conscious call (maintainer action).
Per AGENTS.md §Risk Tiers, crates/zeroclaw-runtime/src/** is classified high-risk. The PR body requests risk: medium, which Jordan endorsed, and the rationale is reasonable: the onboard path is not src/security/ (the "especially" sub-tier), and the security surface of this PR is actually narrowed — no new permissions, no new network calls, and secret behavior is strictly safer. A conscious team decision either way is fine. What isn't fine is leaving the PR unlabeled — please apply risk: medium or risk: high explicitly alongside size: xl before merge.
🔵 zeroclaw models removal — add a changelog line.
The bail! in Commands::Models { model_command: _ } is the right move and the message already names the replacement (zeroclaw onboard providers + config.toml path). This is a user-visible breaking change and should appear in CHANGELOG-next.md under a "CLI changes" heading so users and tool authors can find it during upgrades.
🔵 Team decision check-in from #5951 — one item needs a response.
My comment on #5951 raised three 🔵 team decisions as prerequisites before implementation:
- ✅
OnboardUiplacement: landed inzeroclaw-config::traitsalongsideChannelConfig— resolved. - ✅ Strangler Fig single-PR approach: Jordan's approval and the PR proceeding to HEAD constitute the team's implicit sign-off on the non-incremental approach — resolved.
⚠️ jokemanfire's #5940 overlap: I asked singlerider to respond to jokemanfire in the #5951 thread before #5940 gets more work. Please confirm that happened or close #5940 with a note explaining the supersede. jokemanfire put real effort into that PR; they deserve a direct response.
Scope and compatibility
The Provider trait signature change (temperature: f64 → Option<f64>) cascades through 18 provider files, every caller in the agent loop, delegate, swarm, gateway, channels orchestrator, memory consolidation, benchmarks, and live tests. All updated; workspace builds clean with -D warnings. For in-tree callers this is a non-event. Out-of-tree Provider implementors (none known) would need to migrate — zeroclaw-providers is Beta tier, so "breaking changes permitted in MINOR with changelog notes" applies. Worth a named entry in the v0.7.4 changelog alongside the CLI change above.
Config file format: 100% backward compatible. Same TOML, same keys, no migration. [onboard_state] is #[serde(default)] — a config written by the new binary is safe to read by the old binary.
Verdict
One 🔴 to resolve (single-line editor fix), one deferred-test ticket to open (channel smoke tests), two maintainer actions (milestone assignment, explicit risk label), one thread check-in (jokemanfire/#5940), and one changelog entry. The architecture is sound, the ADR acceptance criteria are covered in tests, and the provider default-ownership refactor is a genuine quality improvement that will pay dividends for every provider added going forward. After the editor fix this is ready for maintainer sign-off and a milestone.
— WareWolf-MoonWall
WareWolf-MoonWall
added
the
risk: medium
Hands-on testing notesBuilt the branch and walked through the onboard flow a few times on my machine — both CLI ( What feels good✅ The TUI is the polished story. It's genuinely beautiful — the six-region layout anchors the input at the bottom while the rolling log keeps context, and navigation feels intuitive without needing to read docs. This is the onboarding I'd point a new user at. ✅ Every entry point I tried works exactly as advertised.
✅ Gateway comes up clean. No onboarding side-effects on the HTTP surface. Issues I hit🔴 Daemon won't stay quiet — Mochat poll loop spins on a builder error. After The daemon itself keeps running (gateway responds), but the log is unusable and something downstream of If it's pre-existing on master, happy to file a separate issue — just want to make sure it's not getting missed because the CI gate and the unit tests don't exercise the 🟡 Live model fetch errors on some providers — fallback works but the UX is rough. Minimax (I think — going from memory, may have been a couple others too) fails the models.dev catalog fetch during 🟡 CLI advanced-settings walk is really long. I went through For someone setting up Anthropic, walking past 🟡 TUI drops advanced settings entirely. The advanced gate I'm complaining about in the CLI isn't in the TUI main flow at all, from what I could tell. That's a backend inconsistency — same Suggestion🔵 Drop the advanced walk from the main onboard flow entirely. My take after walking both backends: advanced users will edit This also cleanly resolves the TUI/CLI inconsistency above — if neither backend has an advanced walk, there's nothing to keep in sync. NetFirst-run UX is good up through the model picker, the TUI is genuinely delightful, and the 90% onboarding path is tight. The advanced-settings tail is where it frays, and the daemon warning is the one thing I'd want eyes on before this ships — the rest I can live with in a follow-up. |
Previously `prompt_model` silently swallowed `list_models()` failures (`.await.ok()`), and the user-visible fallback note was generic — users couldn't tell whether the catalog lookup failed for a transient reason worth retrying or because the provider genuinely doesn't expose a no-auth listing. Two changes: - Capture both the `create_provider` error and the `list_models` error with explicit `match` arms; log each at `tracing::debug!` with the provider name and underlying cause, so operators can diagnose without surfacing a reqwest dump to end users. - Rewrite the fallback note to name the provider: "Catalog lookup failed for <provider> — enter a model id manually (see the provider's docs for the exact format)." — gives users the context they need to decide between retrying, consulting the provider, or hand-entering. Addresses JordanTheJet's 🟡 on cryptic live-fetch errors during the onboard providers walk.
…iderConfig
A fresh provider entry created via `zeroclaw onboard` (or any path that
instantiates `ModelProviderConfig::default()` and serializes it) wrote
provider-family-irrelevant fields to TOML on disk:
[providers.models.anthropic]
requires_openai_auth = false
merge_system_into_user = false
Both fields are plain `bool` with struct-level default `false`, so they
serialized unconditionally. Same issue for `name`, `base_url`, and
`wire_api` — all `Option<String>` without `skip_serializing_if`,
which would write whenever a caller set them to `Some("")` or to
their common-default string.
Add `skip_serializing_if = "Option::is_none"` to the three Options and
`skip_serializing_if = "is_false"` (with a small local helper) to the
two bools, so a fresh-config entry for e.g. Anthropic only carries
fields that actually apply.
Existing configs round-trip unchanged: the `#[serde(default)]` markers
still populate the same default values on deserialize, so dropping the
redundant `= false` / `= ""` lines from disk doesn't alter runtime
behavior.
The advanced-settings walk previously prompted every non-excluded field
under `providers.models.<provider>.*`, regardless of which provider the
user selected. Walking `azure-openai-resource`/`deployment`/`api-version`
for someone picking Anthropic is pure noise; walking `wire-api` and
`requires-openai-auth` for a non-OpenAI-family provider is the same.
Add a small `provider_family_excludes()` helper that extends the
advanced-walk exclusion list based on the selected provider:
- `azure_openai` only: `azure-openai-resource`, `azure-openai-deployment`,
`azure-openai-api-version`
- `openai` / `openai_codex` only: `wire-api`, `requires-openai-auth`
Hardcoded for now; a `#[configurable(family = "...")]` macro attribute
would let the `Configurable` derive emit this automatically. Tracked as
a follow-up per the zeroclaw-labs#5960 scope boundary.
Addresses JordanTheJet's 🟡 on the long advanced walk surfacing fields
from unrelated provider families.
Previously the only test covering the Channels section was `channels_done_selection_is_idempotent_on_disk`, which picks "Done" immediately and never enters any channel's field walk. That left the per-channel init_defaults + set_prop path entirely untested — the "picked X, fields are actually writable" guarantee lived only in manual QA. Two new smoke tests: - `channels_telegram_selection_writes_entry` — picks Telegram via a sequenced "Channel" answer, scripts the required bot-token secret, then exits with "Done". Asserts the telegram subsection initialized and the token round-tripped through set_prop. - `channels_mochat_selection_persists_enabled_url_and_token` — picks Mochat, flips the enabled bool through confirm, scripts api-url + api-token, exits. Doubles as a regression guard for the orchestrator's new mochat enabled-gate: enabled=true must reach persistence. `QuickUi` gains a `with_sequence()` helper so a single prompt label (like "Channel") can return distinct answers across calls. The existing `with()` single-answer map is preserved as a fallback for exhausted sequences. Covers AC #6 from issue zeroclaw-labs#5951 (per-channel onboard smoke tests with a scripted mock OnboardUi).
Cover the changes landed in the preceding PR-zeroclaw-labs#5960 follow-up commits: - `zeroclaw onboard` defaults to TUI; `--cli` forces terminal prompts; `--tui` deprecated no-op (Breaking Changes + Agent & Runtime) - advanced-walk provider-family filtering - models.dev fetch error UX — named provider in fallback note, debug tracing of underlying error - `zeroclaw models` restored via doctor::run_models routing - TermUi::editor Answer::Back contract fix - channels.mochat orchestrator enabled-gate (matches WeCom/ClawdTalk) - ModelProviderConfig skip_serializing_if cleanup — fresh provider entries no longer write family-irrelevant default flags
…check, completions help Three Windows-unsafe patterns in src/main.rs: 1. zeroclaw-desktop locator (~line 1898) read `HOME` directly via `std::env::var_os`, which is unset on Windows. Replaced with `directories::UserDirs::new()` (workspace dep, already used in zeroclaw-config). Also try .exe-suffixed name on Windows since ~/.cargo/bin/zeroclaw-desktop on Windows is actually .cargo\bin\zeroclaw-desktop.exe. .local/bin (XDG) is Unix-only. 2. Daemon-from-home warning (~line 1477) used substring matches on '.cargo/bin' and '/home/' — fails on Windows due to backslash paths and absent /home prefix. Switched to a Path::starts_with check against the user's home dir, with a platform-appropriate install- location hint (/usr/local/bin on Linux, Homebrew prefix on macOS, Program Files on Windows). 3. Completions long_about showed only Unix shell examples. Added PowerShell examples ($PROFILE, Invoke-Expression). The CompletionShell::PowerShell variant already exists; just surface it in the help text. Onboarding wizard itself (crates/zeroclaw-runtime/src/onboard, the TUI) audited and clean — no hardcoded Unix paths or HOME reads.
$SHELL is a Unix convention and is not set on Windows. The doctor was emitting a warning every run on Windows even though the platform has a working default shell. Check %ComSpec% (path to cmd.exe) as a fallback so Windows users see 'shell: C:\\Windows\\system32\\cmd.exe' instead of a spurious '$SHELL not set'.
AutonomyConfig::default() hardcoded Unix forbidden_paths (/etc, /root, /home, /usr, /bin, /sbin, /lib, /opt, /boot, /dev, /proc, /sys, /var, /tmp + ~/.ssh, ~/.gnupg, ~/.aws, ~/.config) and a Unix-only allowed_commands list. The wizard wrote these to ~/.zeroclaw/config.toml regardless of target OS, leaving Windows users with a config full of paths that don't exist on their system and a command allowlist missing dir/type/where/ findstr/more. policy.rs already had cfg-guarded default_allowed_commands() and default_forbidden_paths() helpers used by SecurityPolicy::default(). Promote them to pub(crate) and reuse them in AutonomyConfig::default() so the on-disk defaults match the runtime policy and the host platform.
singlerider
force-pushed
the
feat/onboard-rewrite
branch
from
351149f to
a0f0eea
Compare
- Drop `providers.fallback` auto-persist and the empty-entry save; the entry is now seeded in memory only and dropped on back-out paths so config.toml never gains an empty `[providers.models.<name>]`. - Populate trait defaults on fresh provider entries via a shared `provider_trait_defaults()` helper (single source for both onboard pre-populate and the advanced-settings walk). - Add a "Done" exit to the provider picker so first-time users can skip the section without picking one (matches the channels UX). - StringArray prompts: bidirectional with config.toml — accept either `alice,bob` or `["alice", "bob"]`. Reject malformed brackets via `parses_as_string_array` (TOML round-trip, no JSON). Help text surfaces the format. Clear the note slot on success so error messages don't linger. - Bump ollama default_temperature 0.0 -> 0.8 to match the upstream Modelfile default.
|
I did some runs and tried to break it. I found some issues and fixed them in 5532018. Merging when CI passes. This should be considered experimental in 0.7.4. |
…t, DRY (#5960) - 0c622e6 feat(onboard): scorched-earth delete old wizard + TUI twin (#5951) - 3dabdb1 feat(config): add async OnboardUi trait in traits.rs (#5951) - 65e9809 feat(onboard): add TermUi — dialoguer OnboardUi backend (#5951) - 95b8db4 feat(onboard): add QuickUi — headless OnboardUi backend (#5951) - bcd3df1 feat(onboard): scaffold orchestrator — Section, Flags, run() dispatch… - d14cd46 feat(onboard): implement workspace section (#5951) - 043dd8a feat(onboard): implement memory section; drop Project variant (#5951) - 7b5a4b3 feat(onboard): DRY up memory + implement tunnel via existing sources… - 26251b7 feat(onboard): add generic prompt_field + prompt_fields_under helpers… - 1ada5fb feat(onboard): implement hardware section via prop_fields only (#5951) - fe836a7 feat(onboard): implement providers section via list_providers() + pro… - c22d8c5 feat(onboard): TermUi select uses FuzzySelect for searchable menus (#… - b397324 feat(providers): no-auth live model listing via models.dev + native p… - ff4183a feat(onboard): wire live model picker into providers section (#5951) - 415fe0b feat(onboard): implement channels section via prop_fields + free-text… - acb4658 feat(tui): thin RatatuiUi implementing OnboardUi (#5951) - b7a99db feat(cli): wire zeroclaw onboard to the new orchestrator (#5951) - 2710fbc chore(onboard): pass cargo fmt + clippy --workspace -D warnings gate… - e11191f fix(config): route get_prop/set_prop through HashMap<String, T> + Ter… - 3f8f30b feat(onboard): human-readable prompts from doc comments + consolidate… - 8fee0f7 fix(tui): wrap prompt text + grow prompt bar to fit (#5951) - 8979fdd fix(onboard): show field docstring as context above the prompt, not i… - 045388d docs(config): rewrite WorkspaceConfig + HardwareConfig doc comments f… - b27c4ea fix(onboard): channels menu derives full list from schema via probe c… - eb97efc feat(tui): restore ZeroClaw ASCII banner at top of RatatuiUi (#5951) - 514a77c fix(onboard): reset context panel between sections, note=replace (#5951) - c9c47d5 feat(onboard): persist config after every write so mid-flow aborts r… - fc5ad62 fix(tui): select list uses ratatui List + ListState for scroll-aware… - 59512f1 feat(onboard): section-level skip gate for already-configured section… - 8abab39 feat(onboard): track completed sections via onboard_state.completed_s… - 576ac39 fix(onboard): surface HashMap fields, Back nav, Markdown-style phase… - 38d55f4 refactor(providers): Option<f64> temperature + per-family default_* o… - 0a43a14 feat(onboard): six-region TUI layout, provider-defaults prefill, adva… - 1f50f68 fix(cli): wrap final_temperature in Some for Provider::simple_chat (#… - 1954cf1 test(onboard): skip-gate + idempotency coverage, fix channels signal… - 696e95d test(onboard): cover legacy flag shim and provider flag path (#5951) - f0a2923 fix(onboard): bool prompts and multi-line help text rendering (#5951) - e13d06e fix(onboard): flatten bool is_set so default annotations reach the l… - 2188306 feat(onboard): paint status/warn immediately so web-fetch wait is vi… - 5c2b3fd fix(onboard,doctor): restore zeroclaw doctor models; drop hardcoded m… - 651573f fix(onboard): rustfmt line-length in doctor summary counters - f3e0e48 fix(onboard): honor Answer::Back contract on editor close-without-save - 10c5b7e fix(channels): gate Mochat registration on channels.mochat.enabled - 53ab2a3 feat(cli): restore zeroclaw models via doctor::run_models routing - 73be86e feat(onboard): default to TUI; add --cli and TUI init fallback - 063b23a feat(onboard): improve models.dev fetch error UX - be4688a refactor(config): omit default-valued flags from serialized ModelProv… - e727afa feat(onboard): filter advanced-walk fields by provider family - bd12473 test(onboard): per-channel smoke tests for telegram + mochat - ba0bbbc docs(changelog): document onboard + provider config + CLI changes - 6abb3ce style: remove extra blank line in src/config/mod.rs (cargo fmt) - 50a9bab fix(main): handle Windows paths in desktop locator, daemon-from-home… - e14b30e fix(doctor): fall back to %ComSpec% on Windows when checking shell - 72447ac fix(config): make AutonomyConfig defaults Windows-aware - a0f0eea docs(extension-examples): update Provider example to Option<f64> temp… - 5532018 fix(onboard): no eager writes; complete provider flow before any persist 4112432
…t, DRY (zeroclaw-labs#5960) - 0c622e6 feat(onboard): scorched-earth delete old wizard + TUI twin (zeroclaw-labs#5951) - 3dabdb1 feat(config): add async OnboardUi trait in traits.rs (zeroclaw-labs#5951) - 65e9809 feat(onboard): add TermUi — dialoguer OnboardUi backend (zeroclaw-labs#5951) - 95b8db4 feat(onboard): add QuickUi — headless OnboardUi backend (zeroclaw-labs#5951) - bcd3df1 feat(onboard): scaffold orchestrator — Section, Flags, run() dispatch… - d14cd46 feat(onboard): implement workspace section (zeroclaw-labs#5951) - 043dd8a feat(onboard): implement memory section; drop Project variant (zeroclaw-labs#5951) - 7b5a4b3 feat(onboard): DRY up memory + implement tunnel via existing sources… - 26251b7 feat(onboard): add generic prompt_field + prompt_fields_under helpers… - 1ada5fb feat(onboard): implement hardware section via prop_fields only (zeroclaw-labs#5951) - fe836a7 feat(onboard): implement providers section via list_providers() + pro… - c22d8c5 feat(onboard): TermUi select uses FuzzySelect for searchable menus (#… - b397324 feat(providers): no-auth live model listing via models.dev + native p… - ff4183a feat(onboard): wire live model picker into providers section (zeroclaw-labs#5951) - 415fe0b feat(onboard): implement channels section via prop_fields + free-text… - acb4658 feat(tui): thin RatatuiUi implementing OnboardUi (zeroclaw-labs#5951) - b7a99db feat(cli): wire zeroclaw onboard to the new orchestrator (zeroclaw-labs#5951) - 2710fbc chore(onboard): pass cargo fmt + clippy --workspace -D warnings gate… - e11191f fix(config): route get_prop/set_prop through HashMap<String, T> + Ter… - 3f8f30b feat(onboard): human-readable prompts from doc comments + consolidate… - 8fee0f7 fix(tui): wrap prompt text + grow prompt bar to fit (zeroclaw-labs#5951) - 8979fdd fix(onboard): show field docstring as context above the prompt, not i… - 045388d docs(config): rewrite WorkspaceConfig + HardwareConfig doc comments f… - b27c4ea fix(onboard): channels menu derives full list from schema via probe c… - eb97efc feat(tui): restore ZeroClaw ASCII banner at top of RatatuiUi (zeroclaw-labs#5951) - 514a77c fix(onboard): reset context panel between sections, note=replace (zeroclaw-labs#5951) - c9c47d5 feat(onboard): persist config after every write so mid-flow aborts r… - fc5ad62 fix(tui): select list uses ratatui List + ListState for scroll-aware… - 59512f1 feat(onboard): section-level skip gate for already-configured section… - 8abab39 feat(onboard): track completed sections via onboard_state.completed_s… - 576ac39 fix(onboard): surface HashMap fields, Back nav, Markdown-style phase… - 38d55f4 refactor(providers): Option<f64> temperature + per-family default_* o… - 0a43a14 feat(onboard): six-region TUI layout, provider-defaults prefill, adva… - 1f50f68 fix(cli): wrap final_temperature in Some for Provider::simple_chat (#… - 1954cf1 test(onboard): skip-gate + idempotency coverage, fix channels signal… - 696e95d test(onboard): cover legacy flag shim and provider flag path (zeroclaw-labs#5951) - f0a2923 fix(onboard): bool prompts and multi-line help text rendering (zeroclaw-labs#5951) - e13d06e fix(onboard): flatten bool is_set so default annotations reach the l… - 2188306 feat(onboard): paint status/warn immediately so web-fetch wait is vi… - 5c2b3fd fix(onboard,doctor): restore zeroclaw doctor models; drop hardcoded m… - 651573f fix(onboard): rustfmt line-length in doctor summary counters - f3e0e48 fix(onboard): honor Answer::Back contract on editor close-without-save - 10c5b7e fix(channels): gate Mochat registration on channels.mochat.enabled - 53ab2a3 feat(cli): restore zeroclaw models via doctor::run_models routing - 73be86e feat(onboard): default to TUI; add --cli and TUI init fallback - 063b23a feat(onboard): improve models.dev fetch error UX - be4688a refactor(config): omit default-valued flags from serialized ModelProv… - e727afa feat(onboard): filter advanced-walk fields by provider family - bd12473 test(onboard): per-channel smoke tests for telegram + mochat - ba0bbbc docs(changelog): document onboard + provider config + CLI changes - 6abb3ce style: remove extra blank line in src/config/mod.rs (cargo fmt) - 50a9bab fix(main): handle Windows paths in desktop locator, daemon-from-home… - e14b30e fix(doctor): fall back to %ComSpec% on Windows when checking shell - 72447ac fix(config): make AutonomyConfig defaults Windows-aware - a0f0eea docs(extension-examples): update Provider example to Option<f64> temp… - 5532018 fix(onboard): no eager writes; complete provider flow before any persist 4112432
…t, DRY (zeroclaw-labs#5960) - 0c622e6 feat(onboard): scorched-earth delete old wizard + TUI twin (zeroclaw-labs#5951) - 3dabdb1 feat(config): add async OnboardUi trait in traits.rs (zeroclaw-labs#5951) - 65e9809 feat(onboard): add TermUi — dialoguer OnboardUi backend (zeroclaw-labs#5951) - 95b8db4 feat(onboard): add QuickUi — headless OnboardUi backend (zeroclaw-labs#5951) - bcd3df1 feat(onboard): scaffold orchestrator — Section, Flags, run() dispatch… - d14cd46 feat(onboard): implement workspace section (zeroclaw-labs#5951) - 043dd8a feat(onboard): implement memory section; drop Project variant (zeroclaw-labs#5951) - 7b5a4b3 feat(onboard): DRY up memory + implement tunnel via existing sources… - 26251b7 feat(onboard): add generic prompt_field + prompt_fields_under helpers… - 1ada5fb feat(onboard): implement hardware section via prop_fields only (zeroclaw-labs#5951) - fe836a7 feat(onboard): implement providers section via list_providers() + pro… - c22d8c5 feat(onboard): TermUi select uses FuzzySelect for searchable menus (#… - b397324 feat(providers): no-auth live model listing via models.dev + native p… - ff4183a feat(onboard): wire live model picker into providers section (zeroclaw-labs#5951) - 415fe0b feat(onboard): implement channels section via prop_fields + free-text… - acb4658 feat(tui): thin RatatuiUi implementing OnboardUi (zeroclaw-labs#5951) - b7a99db feat(cli): wire zeroclaw onboard to the new orchestrator (zeroclaw-labs#5951) - 2710fbc chore(onboard): pass cargo fmt + clippy --workspace -D warnings gate… - e11191f fix(config): route get_prop/set_prop through HashMap<String, T> + Ter… - 3f8f30b feat(onboard): human-readable prompts from doc comments + consolidate… - 8fee0f7 fix(tui): wrap prompt text + grow prompt bar to fit (zeroclaw-labs#5951) - 8979fdd fix(onboard): show field docstring as context above the prompt, not i… - 045388d docs(config): rewrite WorkspaceConfig + HardwareConfig doc comments f… - b27c4ea fix(onboard): channels menu derives full list from schema via probe c… - eb97efc feat(tui): restore ZeroClaw ASCII banner at top of RatatuiUi (zeroclaw-labs#5951) - 514a77c fix(onboard): reset context panel between sections, note=replace (zeroclaw-labs#5951) - c9c47d5 feat(onboard): persist config after every write so mid-flow aborts r… - fc5ad62 fix(tui): select list uses ratatui List + ListState for scroll-aware… - 59512f1 feat(onboard): section-level skip gate for already-configured section… - 8abab39 feat(onboard): track completed sections via onboard_state.completed_s… - 576ac39 fix(onboard): surface HashMap fields, Back nav, Markdown-style phase… - 38d55f4 refactor(providers): Option<f64> temperature + per-family default_* o… - 0a43a14 feat(onboard): six-region TUI layout, provider-defaults prefill, adva… - 1f50f68 fix(cli): wrap final_temperature in Some for Provider::simple_chat (#… - 1954cf1 test(onboard): skip-gate + idempotency coverage, fix channels signal… - 696e95d test(onboard): cover legacy flag shim and provider flag path (zeroclaw-labs#5951) - f0a2923 fix(onboard): bool prompts and multi-line help text rendering (zeroclaw-labs#5951) - e13d06e fix(onboard): flatten bool is_set so default annotations reach the l… - 2188306 feat(onboard): paint status/warn immediately so web-fetch wait is vi… - 5c2b3fd fix(onboard,doctor): restore zeroclaw doctor models; drop hardcoded m… - 651573f fix(onboard): rustfmt line-length in doctor summary counters - f3e0e48 fix(onboard): honor Answer::Back contract on editor close-without-save - 10c5b7e fix(channels): gate Mochat registration on channels.mochat.enabled - 53ab2a3 feat(cli): restore zeroclaw models via doctor::run_models routing - 73be86e feat(onboard): default to TUI; add --cli and TUI init fallback - 063b23a feat(onboard): improve models.dev fetch error UX - be4688a refactor(config): omit default-valued flags from serialized ModelProv… - e727afa feat(onboard): filter advanced-walk fields by provider family - bd12473 test(onboard): per-channel smoke tests for telegram + mochat - ba0bbbc docs(changelog): document onboard + provider config + CLI changes - 6abb3ce style: remove extra blank line in src/config/mod.rs (cargo fmt) - 50a9bab fix(main): handle Windows paths in desktop locator, daemon-from-home… - e14b30e fix(doctor): fall back to %ComSpec% on Windows when checking shell - 72447ac fix(config): make AutonomyConfig defaults Windows-aware - a0f0eea docs(extension-examples): update Provider example to Option<f64> temp… - 5532018 fix(onboard): no eager writes; complete provider flow before any persist 4112432
…t, DRY (zeroclaw-labs#5960) - 0c622e6 feat(onboard): scorched-earth delete old wizard + TUI twin (zeroclaw-labs#5951) - 3dabdb1 feat(config): add async OnboardUi trait in traits.rs (zeroclaw-labs#5951) - 65e9809 feat(onboard): add TermUi — dialoguer OnboardUi backend (zeroclaw-labs#5951) - 95b8db4 feat(onboard): add QuickUi — headless OnboardUi backend (zeroclaw-labs#5951) - bcd3df1 feat(onboard): scaffold orchestrator — Section, Flags, run() dispatch… - d14cd46 feat(onboard): implement workspace section (zeroclaw-labs#5951) - 043dd8a feat(onboard): implement memory section; drop Project variant (zeroclaw-labs#5951) - 7b5a4b3 feat(onboard): DRY up memory + implement tunnel via existing sources… - 26251b7 feat(onboard): add generic prompt_field + prompt_fields_under helpers… - 1ada5fb feat(onboard): implement hardware section via prop_fields only (zeroclaw-labs#5951) - fe836a7 feat(onboard): implement providers section via list_providers() + pro… - c22d8c5 feat(onboard): TermUi select uses FuzzySelect for searchable menus (#… - b397324 feat(providers): no-auth live model listing via models.dev + native p… - ff4183a feat(onboard): wire live model picker into providers section (zeroclaw-labs#5951) - 415fe0b feat(onboard): implement channels section via prop_fields + free-text… - acb4658 feat(tui): thin RatatuiUi implementing OnboardUi (zeroclaw-labs#5951) - b7a99db feat(cli): wire zeroclaw onboard to the new orchestrator (zeroclaw-labs#5951) - 2710fbc chore(onboard): pass cargo fmt + clippy --workspace -D warnings gate… - e11191f fix(config): route get_prop/set_prop through HashMap<String, T> + Ter… - 3f8f30b feat(onboard): human-readable prompts from doc comments + consolidate… - 8fee0f7 fix(tui): wrap prompt text + grow prompt bar to fit (zeroclaw-labs#5951) - 8979fdd fix(onboard): show field docstring as context above the prompt, not i… - 045388d docs(config): rewrite WorkspaceConfig + HardwareConfig doc comments f… - b27c4ea fix(onboard): channels menu derives full list from schema via probe c… - eb97efc feat(tui): restore ZeroClaw ASCII banner at top of RatatuiUi (zeroclaw-labs#5951) - 514a77c fix(onboard): reset context panel between sections, note=replace (zeroclaw-labs#5951) - c9c47d5 feat(onboard): persist config after every write so mid-flow aborts r… - fc5ad62 fix(tui): select list uses ratatui List + ListState for scroll-aware… - 59512f1 feat(onboard): section-level skip gate for already-configured section… - 8abab39 feat(onboard): track completed sections via onboard_state.completed_s… - 576ac39 fix(onboard): surface HashMap fields, Back nav, Markdown-style phase… - 38d55f4 refactor(providers): Option<f64> temperature + per-family default_* o… - 0a43a14 feat(onboard): six-region TUI layout, provider-defaults prefill, adva… - 1f50f68 fix(cli): wrap final_temperature in Some for Provider::simple_chat (#… - 1954cf1 test(onboard): skip-gate + idempotency coverage, fix channels signal… - 696e95d test(onboard): cover legacy flag shim and provider flag path (zeroclaw-labs#5951) - f0a2923 fix(onboard): bool prompts and multi-line help text rendering (zeroclaw-labs#5951) - e13d06e fix(onboard): flatten bool is_set so default annotations reach the l… - 2188306 feat(onboard): paint status/warn immediately so web-fetch wait is vi… - 5c2b3fd fix(onboard,doctor): restore zeroclaw doctor models; drop hardcoded m… - 651573f fix(onboard): rustfmt line-length in doctor summary counters - f3e0e48 fix(onboard): honor Answer::Back contract on editor close-without-save - 10c5b7e fix(channels): gate Mochat registration on channels.mochat.enabled - 53ab2a3 feat(cli): restore zeroclaw models via doctor::run_models routing - 73be86e feat(onboard): default to TUI; add --cli and TUI init fallback - 063b23a feat(onboard): improve models.dev fetch error UX - be4688a refactor(config): omit default-valued flags from serialized ModelProv… - e727afa feat(onboard): filter advanced-walk fields by provider family - bd12473 test(onboard): per-channel smoke tests for telegram + mochat - ba0bbbc docs(changelog): document onboard + provider config + CLI changes - 6abb3ce style: remove extra blank line in src/config/mod.rs (cargo fmt) - 50a9bab fix(main): handle Windows paths in desktop locator, daemon-from-home… - e14b30e fix(doctor): fall back to %ComSpec% on Windows when checking shell - 72447ac fix(config): make AutonomyConfig defaults Windows-aware - a0f0eea docs(extension-examples): update Provider example to Option<f64> temp… - 5532018 fix(onboard): no eager writes; complete provider flow before any persist 4112432
Resolve conflicts after PR zeroclaw-labs#5960 (schema-driven onboard rewrite) and PR zeroclaw-labs#5735 (decouple gateway/tui-onboarding from agent-runtime): - crates/zeroclaw-runtime/src/onboard/wizard.rs: deleted (whole file removed in master; the new onboard reads SlackConfig from the schema via traits::OnboardUi, so the manual field plumbing is obsolete). - crates/zeroclaw-tui/src/onboarding.rs: take master version (rewritten as a pure ratatui OnboardUi implementation; no longer holds channel struct literals, so the explicit `strict_mention_in_thread: false` initializer is no longer needed). Substantive Slack changes (schema.rs field, slack.rs struct/builder/ runtime checks/test, orchestrator/mod.rs wiring, CHANGELOG-next.md) auto-merged cleanly. Behavior unchanged: `#[serde(default)]` keeps the flag false on all non-configured paths, matching the old onboard default. Verified: cargo check --workspace --exclude zeroclaw-desktop, cargo clippy on affected crates with -D warnings, cargo fmt --check, and cargo test -p zeroclaw-channels --lib slack:: (96/96 pass).
…providers (ported from zeroclaw-labs#5960 conflict)
5 participants
Summary
Base branch:
masterWhat changed and why:
onboard/wizard.rsmonolith + 3,898-line ratatui twin with a thin schema-driven orchestrator that drivesconfig.set_propeverywhere, pre-populates from the loadedConfig, and only writes changed values (idempotent). Closes the drift-class bugs catalogued in [Feature]: Clean-slate rewrite of zeroclaw onboard (DRY, schema-driven, idempotent) #5951 (Sonnet 4.6 gap, etc.).OnboardUitrait inzeroclaw-config::traits(async, alongsideChannelConfig) — one prompt surface, three backends (dialoguer TermUi, headless QuickUi, ratatui RatatuiUi). No new workspace crates.Providertrait to own its defaults: fivedefault_*methods (default_temperature,default_max_tokens,default_timeout_secs,default_base_url,default_wire_api) with namedBASELINE_*constants inzeroclaw-api::provider. Signatures changetemperature: f64→temperature: Option<f64>end-to-end; callers stop computing defaults, providers unwrap once against their family default. Per-family overrides declared as namedconsts (no magic numbers).timeout-secs (default: 120)+ pre-filled input so Enter accepts). Advanced-settings gate in Providers ("Configure advanced settings? [y/N]"default N) so most users breeze through auth + model + done.CONTRIBUTING.md"How to Add a New Provider" section rewritten with correct import paths, the new trait surface, the DRY rule fordefault_*overrides,list_modelsguidance for onboard integration, and the right factory dispatch location.Scope boundary (what this PR does NOT change):
zeroclaw-configschema shape beyond the newOnboardUitrait, theonboard_state.completed_sectionsmeta-state, and docstring rewrites.ProviderInfostructure / factory-match consolidation inzeroclaw-providers/src/lib.rs(deferred per ADR [Feature]: Clean-slate rewrite of zeroclaw onboard (DRY, schema-driven, idempotent) #5951 to avoid touching everylist_providers()caller).const XXX_BASE_URLstrings inzeroclaw-providers/src/lib.rs:54-86— those are factory-routing constants (MINIMAX_CN vs INTL, QWEN's 3 regions, etc.), not per-family trait defaults. Left for the registry-consolidation follow-up.Blast radius:
zeroclaw-api::Providertrait signature change cascades to every provider impl (18 files inzeroclaw-providers/) and every caller (agent loop, delegate tool, swarm, gateway, channels/orchestrator, memory consolidation, main.rs, live tests). All updated; workspace compiles with-D warnings.zeroclaw onboardCLI: legacy--*-onlyflags still parse with deprecation warning; new positional subcommands (zeroclaw onboard providers,… channels, …) are the forward path.0.7/4096/120the callers used to compute inline.Linked issue(s):
zeroclaw_providers::list_providers(), which includescopilot; Copilot now appears in onboarding without the manual-TOML workaround.src/main.rsnow callsregister_cli_channel_fnon both interactive entry paths, sozeroclaw agentno longer panics after a freshonboard.[providers.models.llamacpp]object ignored because onboard wroteproviders.fallback = "custom:http://..."instead of a canonical provider name. The new onboard usesset_prop providers.fallback = "llamacpp"(the canonical key matchingproviders.models.llamacpp), so the fallback resolves correctly.llamacppprovider name.base_url. Does not provide a dedicated "Anthropic (Custom Endpoint)" menu option or theanthropic-custom:auto-prefix the issue proposed, but the underlying workflow (configure a custom endpoint during onboarding without hand-editing config.toml) is supported.Validation Evidence (required)
Commands run and tail output:
cargo fmt --all -- --check→ clean (exit 0).cargo clippy --workspace --all-targets -- -D warnings→Finished \dev` profile ... in 45.72s` with no warnings.cargo check --workspace --all-targets→Finished \dev` profile ... in 17.24s`.cargo test --workspace→ all green, 0 failures across every crate.Beyond CI — what did you manually verified:
zeroclaw onboardin ratatui mode through Providers → Anthropic: auth-phase heading fires, model picker pulls from models.dev (Sonnet 4.6, Opus 4.7 present), advanced-settings gate defaults to N, accepting it writesproviders.models.anthropic.*viaset_prop.zeroclaw onboardon the configured install: section skip gates fire ("Providers is already configured. Reconfigure?") viaonboard_state.completed_sections+ per-section meaningful-config heuristic.New test coverage landed on this branch (tied to the AC list in [Feature]: Clean-slate rewrite of zeroclaw onboard (DRY, schema-driven, idempotent) #5951):
onboard::tests(13 cases) —section_has_signalper-section correctness;skip_if_configureddecision matrix under marker / signal /--force;mark_completeddedupe; full double-run idempotency forworkspace,channels, andproviders(assertsconfig.tomlbyte-identical across two runs).src/main.rs::tests(4 cases) —resolve_onboard_target()covers the--*-onlylegacy flag deprecation shim: precedence of positional subcommand over legacy flag, per-flag target routing, and the warning pair emitted whenever a legacy flag is set.flatten_system_messagestests surfaced by the workspace test run.section_has_signal("channels")treated the default-truechannels.clibool as user-driven signal, which silently skipped the Channels section on every fresh install. Fixed to require a nestedchannels.<x>.<y>field.Deferred follow-up tests: per-channel field-walk smoke tests (picking a specific channel and asserting its fields land via
set_prop) — the currentchannels_done_selectiontest only exercises the "pick Done immediately" path.Security & Privacy Impact (required)
ui.secret(has_current=true)now defaults to "keep current" instead of silently re-prompting, preventing accidental key wipes.--forceis the explicit re-entry path.Compatibility (required)
Providertrait signature (internal Rust surface) changed fromf64→Option<f64>, but this only matters for out-of-tree provider impls (none known). Thezeroclaw onboardCLI gained positional subcommands; legacy--channels-only/--providers-only/ etc. still parse for one release with a deprecation warning pointing at the new subcommand.[onboard_state]opaque meta-state (empty on fresh installs, auto-populated as sections complete).git pull && cargo build --release. No action required from existing users.Rollback (required for
risk: mediumandrisk: high)git revert <merge-sha>. No data migration; config file format unchanged, so reverting does not strand any user data.[onboard_state]would remain in config files that were updated during the new-binary run but is#[serde(default)]-tolerant of absence on the old binary.wizard.rsis deleted in the same PR; no runtime switch to fall back to.zeroclaw onboardpanics orset_properrors in logs;cargo test -p zeroclaw-config -p zeroclaw-providers -p zeroclaw-runtimefailing in CI.Supersede Attribution
N/A.
i18n Follow-Through
N.A. — no user-facing documentation strings were added; rewritten docstrings are internal Rust
///comments surfaced only in the onboard UI (which is not i18n'd today) andrustdoc. CONTRIBUTING.md is English-only per repo convention.Labels: please set
risk: medium,size: xl,scope: core,scope: onboard,scope: provider.