feat(onboard): guided CLI onboarding with live AI verification and classic fallback#101880
Conversation
|
Codex review: found issues before merge. Reviewed July 9, 2026, 5:03 AM ET / 09:03 UTC. Summary PR surface: Source +1046, Tests +933, Docs +117. Total +2096 across 39 files. Reproducibility: yes. source-reproducible: PR-head code shows the detected-env credential durability gap, selected-workspace verifier gap, and Codex install metadata loss. I did not run mutating onboarding tests in this read-only review. Review metrics: 3 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Land the guided default only after detected credentials remain usable for first chat, classic verification uses the selected workspace, Codex ensure metadata is persisted, conflicts are refreshed, and the onboarding owner accepts the default-flow compatibility change. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: PR-head code shows the detected-env credential durability gap, selected-workspace verifier gap, and Codex install metadata loss. I did not run mutating onboarding tests in this read-only review. Is this the best way to solve the issue? No, not yet: reusing setup-inference is the right owner boundary, but this implementation still has auth/provider persistence blockers and needs maintainer acceptance of the default-flow change. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 1ad346e46509. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1046, Tests +933, Docs +117. Total +2096 across 39 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (4 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0342c4525
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| deps.launchTui ?? | ||
| (async () => { | ||
| const { launchTuiCli } = await import("../tui/tui-launch.js"); | ||
| await launchTuiCli({ deliver: false }); |
There was a problem hiding this comment.
Keep shell-env API keys available for first chat
When guided setup succeeds via a detected OPENAI_API_KEY/ANTHROPIC_API_KEY that exists only in the invoking shell, the live test runs in this CLI process and applyCrestodianSetup persists only the model/workspace/gateway. This branch then opens the normal Gateway-backed TUI instead of local mode; the managed Gateway service environment is rebuilt from durable .env/config/auth-profile refs, not arbitrary shell env, so the just-verified key is absent and the first chat fails. Manual pasted keys and keys already in the state .env are unaffected, but the common shell-export path needs either a durable credential/ref or a local TUI handoff.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d46b2ad748
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const verify = async () => { | ||
| const progress = params.prompter.progress(t("wizard.setup.testAiProgress")); | ||
| const result = await withConsoleSubsystemsSuppressed(() => | ||
| verifySetupInference({ runtime: params.runtime }), |
There was a problem hiding this comment.
Verify classic setup in the selected workspace
When the classic wizard configures or keeps a model provided by an enabled workspace plugin, this call runs verifySetupInference without the selected workspaceDir. The preceding model/auth flow resolves provider choices with the real workspace, but the verifier then builds and runs its probe from a temporary workspace, so the provider plugin can be absent during the live check and the user gets a false failure with a prompt to “fix” an otherwise working config. Pass the selected workspace through to the verifier while keeping only session/auth scratch state temporary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
openclaw/src/crestodian/setup-inference.ts
Line 627 in 459b88a
When Codex is the selected candidate on a packaged install that does not already have a codex install record, ensureCodexRuntimePluginForModelSelection installs @openclaw/codex and returns a config containing the new install metadata. This update discards that returned config and writes only enablePluginInConfig(current, "codex"), so the durable plugins.installs.codex record is never committed; after setup persists an openai/* default and the Gateway/process restarts, plugin discovery can no longer resolve the external Codex runtime even though onboarding reported success. Merge or persist ensured.cfg (at least its install record) before applying the model.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…assic fallback Interactive `openclaw onboard` (and bare `openclaw` on a fresh install) now runs a guided flow with macOS-app parity: detect existing AI access, live-test candidates with a real completion before persisting anything, walk down the ladder on failure with mapped reasons, and offer verified manual API-key entry from installed provider manifests (masked input). In-flow escapes: classic wizard, Crestodian chat, skip-AI. Classic wizard gains an optional post-auth live verification step. `--classic`, `--modern`, and `--non-interactive` contracts unchanged. Docs corrected for post-#99935 routing. Closes #101851
…ck into classic escape Candidate live-tests during guided setup are probes: rename their run id and lane to the existing probe conventions (logging/subsystem.ts console suppression, command-queue quiet probe lanes) so expected failures stop leaking raw diagnostics into the Clack UI; file diagnostics unchanged. The classic-wizard escape now passes the already-collected risk acknowledgement through instead of re-prompting in the same session.
The live-test run enqueues on two lanes: the explicit probe lane and one derived from its temp session key. Extend the shared quiet-probe predicate to cover the derived lane so a failing candidate cannot leak lane-task diagnostics into the wizard TTY.
…e tests Provider-transport subsystem loggers (model-fetch start/response, transport errors) carry no run id, so probe suppression cannot catch them and a failing candidate printed raw log lines into the Clack TTY. Reuse the TUI console subsystem-filter seam via a finally-safe scoped helper around guided activation and the classic live-verify; file logging is unchanged and the gateway (macOS app) surface is unaffected.
…ck fails The re-run verification probe executes outside the configured workspace (setup never runs workspace plugins), so a workspace-backed current model can fail the check while working fine in the agent. Stop the auto ladder on an existing-model failure and hand the decision to the manual stage instead of silently persisting a different candidate as the default. Docs note the fail-safe and the workspace caveat.
… wizards From the chat, `open setup wizard`, `open classic wizard`, and `open channel wizard for <channel>` hand off to the guided flow, the classic wizard, or the masked `channels add` wizard after the chat TUI tears down (mirrors the open-tui handoff; gateway surface gets a text pointer instead). The hosted channel wizard no longer dead-ends at sensitive steps — it offers the switch and remembers the channel. New read-only `channel info <channel>` operation and ring-zero action surface label, blurb, configured state, and the real docs URL from channel-setup discovery so the assistant can explain Slack or Telegram prerequisites instead of guessing; both prompts instruct it to use them. `channels add --channel <id>` now preselects the channel. Docs cover the interchangeable flows.
…mpt guidance Live test showed the agent detouring an explicit connect request through channel_info because the guidance said to consult it first. Both prompts now distinguish asking about a channel (channel info + docs link) from asking to connect (connect right away).
The shared single-token prompt lacked sensitive:true, so terminal wizards echoed pasted channel tokens and the Crestodian chat bridge (which refuses plain-text secrets based on this flag) hosted the Telegram token step in visible chat. Found live-testing the chat-to-wizard switch; pre-existing on main but load-bearing for the masked-wizard contract this PR documents.
Mirror the classic finalize handoff so the chat TUI never inherits the wizard prompter's raw/paused terminal state on the default first-run path.
Engine-side surface gating already prevents open-setup replies on the gateway surface; this keeps the client-visible action enum stable even if that gate ever regresses. (Reviewed with the switching round; missed in its commit.)
459b88a to
59100ae
Compare
There was a problem hiding this comment.
💡 Codex Review
openclaw/src/crestodian/setup-inference.ts
Line 627 in 59100ae
When guided onboarding reaches the Codex CLI path and ensureCodexRuntimePluginForModelSelection has to install or repair the runtime plugin, the returned ensured.cfg is the config that carries the plugin enablement plus durable install metadata produced by recordPluginInstall; this mutator discards that result and only enables codex on the current config. Unlike the sibling model setup paths that continue with codexInstall.cfg, a fresh Codex setup can save the openai/* default without the plugin install/trust/integrity record (or any non-recoverable install path), so the next process may need doctor/reinstall work before the selected runtime is actually available. Persist or merge ensured.cfg here instead of recomputing only enablePluginInConfig.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Merged via squash.
|
…assic fallback (openclaw#101880) * feat(onboard): guided CLI onboarding with live AI verification and classic fallback Interactive `openclaw onboard` (and bare `openclaw` on a fresh install) now runs a guided flow with macOS-app parity: detect existing AI access, live-test candidates with a real completion before persisting anything, walk down the ladder on failure with mapped reasons, and offer verified manual API-key entry from installed provider manifests (masked input). In-flow escapes: classic wizard, Crestodian chat, skip-AI. Classic wizard gains an optional post-auth live verification step. `--classic`, `--modern`, and `--non-interactive` contracts unchanged. Docs corrected for post-openclaw#99935 routing. Closes openclaw#101851 * improve(onboard): quiet probe diagnostics in wizard TTY, carry risk ack into classic escape Candidate live-tests during guided setup are probes: rename their run id and lane to the existing probe conventions (logging/subsystem.ts console suppression, command-queue quiet probe lanes) so expected failures stop leaking raw diagnostics into the Clack UI; file diagnostics unchanged. The classic-wizard escape now passes the already-collected risk acknowledgement through instead of re-prompting in the same session. * fix(onboard): quiet the session-derived setup-inference probe lane too The live-test run enqueues on two lanes: the explicit probe lane and one derived from its temp session key. Extend the shared quiet-probe predicate to cover the derived lane so a failing candidate cannot leak lane-task diagnostics into the wizard TTY. * improve(onboard): suppress subsystem console output during wizard live tests Provider-transport subsystem loggers (model-fetch start/response, transport errors) carry no run id, so probe suppression cannot catch them and a failing candidate printed raw log lines into the Clack TTY. Reuse the TUI console subsystem-filter seam via a finally-safe scoped helper around guided activation and the classic live-verify; file logging is unchanged and the gateway (macOS app) surface is unaffected. * fix(onboard): never auto-replace a configured model when its live check fails The re-run verification probe executes outside the configured workspace (setup never runs workspace plugins), so a workspace-backed current model can fail the check while working fine in the agent. Stop the auto ladder on an existing-model failure and hand the decision to the manual stage instead of silently persisting a different candidate as the default. Docs note the fail-safe and the workspace caveat. * feat(onboard): two-way switching between Crestodian chat and the menu wizards From the chat, `open setup wizard`, `open classic wizard`, and `open channel wizard for <channel>` hand off to the guided flow, the classic wizard, or the masked `channels add` wizard after the chat TUI tears down (mirrors the open-tui handoff; gateway surface gets a text pointer instead). The hosted channel wizard no longer dead-ends at sensitive steps — it offers the switch and remembers the channel. New read-only `channel info <channel>` operation and ring-zero action surface label, blurb, configured state, and the real docs URL from channel-setup discovery so the assistant can explain Slack or Telegram prerequisites instead of guessing; both prompts instruct it to use them. `channels add --channel <id>` now preselects the channel. Docs cover the interchangeable flows. * fix(onboard): avoid param reassignment in open-setup handoff * improve(onboard): separate ask-about vs connect intent in channel prompt guidance Live test showed the agent detouring an explicit connect request through channel_info because the guidance said to consult it first. Both prompts now distinguish asking about a channel (channel info + docs link) from asking to connect (connect right away). * fix(channels): mark channel token entry as sensitive input The shared single-token prompt lacked sensitive:true, so terminal wizards echoed pasted channel tokens and the Crestodian chat bridge (which refuses plain-text secrets based on this flag) hosted the Telegram token step in visible chat. Found live-testing the chat-to-wizard switch; pre-existing on main but load-bearing for the masked-wizard contract this PR documents. * fix(onboard): restore terminal state around the guided flow's TUI launch Mirror the classic finalize handoff so the chat TUI never inherits the wizard prompter's raw/paused terminal state on the default first-run path. * fix(channels): type the token prompter mock for the sensitive-flag assertion * fix(gateway): map the TUI-only open-setup action to none for app clients Engine-side surface gating already prevents open-setup replies on the gateway surface; this keeps the client-visible action enum stable even if that gate ever regresses. (Reviewed with the switching round; missed in its commit.) * docs: regenerate docs map for onboarding page changes
Closes #101851
What Problem This Solves
The macOS app's first-run "Connect your AI" step live-tests every AI backend with a real completion before saving anything, falls through automatically when a candidate fails, and verifies manually entered keys before storing them. The CLI had none of that: interactive onboarding (Crestodian chat since #99935) persisted the first detected credential without any live test — a stale
OPENAI_API_KEY, an expired CLI login, or a billing-dead account became the configured default model, and the failure only surfaced on the first real agent turn. A fresh machine with no CLI logins dead-ended entirely: the chat TUI takes no masked input, so there was no way to enter and verify an API key during CLI onboarding. Docs still described pre-#99935 routing (classic as the fresh-install default).Why This Change Was Made
Interactive
openclaw onboard(and bareopenclawon a fresh install) now runs a guided flow driving the same engine the macOS app uses (detectSetupInference/activateSetupInference,surface: "cli"): risk acknowledgement → workspace → detect existing AI access → live-test the recommended candidate with a real "reply OK" completion → on failure show the mapped reason (auth/rate limit/billing/timeout) and automatically try the next usable candidate → when detection is exhausted, offer every remaining/failed candidate for retry, a masked and live-verified manual API-key step built from installed provider manifests, Crestodian chat, the classic step-by-step wizard, or skip-AI. Model + workspace + quickstart gateway are persisted only after a passing test; a failing candidate never touches config. The classic wizard keeps its full surface and gains an optional post-auth live verification (never blocks completion). Candidate live-tests are classified as probes so expected failures stay out of the wizard TTY (file diagnostics unchanged), and the classic escape carries the already-collected risk acknowledgement instead of re-prompting. No new flags, config keys, or protocol changes;--classic,--modern,--non-interactive, and all scripted flags behave exactly as before.User Impact
Fresh installs get a first chat that actually works: onboarding proves inference before claiming success, explains provider failures in plain language as it walks the ladder, and accepts a manually pasted key with masked input and the same verification — on every platform, not just macOS. Re-running
openclaw onboardon a configured install now doubles as a verify/repair pass for the current model. Classic-wizard users can confirm their model answers before finishing setup.Evidence
Live E2E on a clean Linux box (AWS Crabbox
cbx_114157c63e3d, packaged tarball viascripts/package-openclaw-for-docker.mjs+npm i -g, real API keys, driven over tmux):openai/gpt-5.5, workspace, quickstart gateway (token auth), bootstrap files seeded, clean exit 0.anthropic/claude-opus-4-8persisted; the failing candidate never reached config; no diagnostics leak into the TTY (re-verified after the probe-lane fix: leak count 0).--classic --flow quickstart): env-key detection, Codex runtime plugin auto-install from the packaged tarball, model configured, then "Test AI access now with a live completion?" → "AI access works. Replied in 3.0s."Screenshots (full set + manifest):
Gates:
pnpm check:changedexit 0 on the final head (leasetbx_01kwz4d92mmtswkykjbg7wm2pe, Testbox-through-Crabbox).pnpm test:changedon the same lease: 3 failures, all pre-existing onmainand unrelated — the affectedsrc/plugins/**files are byte-identical toorigin/main; they encode the host's/tmp/openclawstate inos.tmpdir()assertions and fail on any machine where/tmp/openclawis ownable (two-way proof + mechanism in #101876, follow-up filed). Everything else passed (e.g. gateway+agents projects: 270 files / 3296 tests green; an intermittent vitest "Worker exited unexpectedly" crash-at-shutdown on the Testbox can poison exit codes while all tests pass — observed at varying positions across runs, also unrelated to this diff). Autoreview (branch mode vs origin/main) ran to completion; its one finding (existing-model verification runs outside the configured workspace and could let the ladder silently replace a workspace-backed model) was addressed in f0342c4 by failing safe: a failed current-model check stops the auto-ladder and asks, never auto-replacing the configured default.Focused suites locally green throughout (
node scripts/run-vitest.mjson onboard-guided, onboard, onboard-interactive, setup-inference, wizard setup, register.onboard, i18n, command-queue, logging subsystem). Known proof gap: gateway service install on the E2E box fails (systemctl --userunavailable on the headless host) — the flow reports it honestly and continues; daemon install itself is untouched by this PR.Update: two-way switching + channel-setup guidance
The chat and the menu wizards are now fully interchangeable, and the assistant can guide channel setup with real metadata instead of guesses:
open setup wizard(guided),open classic wizard, andopen channel wizard for <channel>are new typed operations plus a ring-zeroopen_setupaction; the chat TUI tears down first and the wizard launches in its place (mirrors the existingtalk to agenthandoff; the gateway/macOS chat surface returns a text pointer instead). Workspace and the already-collected risk acknowledgement carry across the switch.open channel wizard, remembers the channel, and hands off into the maskedchannels addwizard (which now preselects that channel).channel info <channel>operation andchannel_inforing-zero action surface the channel's label, blurb, configured/installed state, and its realdocs.openclaw.ai/channels/...URL from channel-setup discovery. Both prompts instruct the assistant: asking about a channel →channel info+ docs link; asking to connect → start the wizard directly (the distinction came from a live-test failure where the agent detoured a connect request through info).sensitive: true, so terminal wizards echoed pasted tokens and the chat bridge hosted a Telegram token step in visible plain text. Fixed with a regression test asserting every token prompt is sensitive; pre-existing onmainbut load-bearing for the masked-wizard contract this PR documents.cli/crestodian,cli/onboard,start/wizard, andstart/onboarding-overviewto describe the interchangeable flows and new commands.Live proof (fresh Linux box, packaged tarball, real OpenAI key, tmux-driven):
channel info slack, cites real docs URLconnect telegramreaches the token step → guard fires with the switch offeropen channel wizard→ chat exits, masked channel wizard opensopen setup wizard→ chat hands off to guided onboardingGates:
check:changedgreen on the update head (Testbox-through-Crabbox);test:changedgreen except five failures pre-existing onmainwith unchanged files, all filed: #101876 (plugins/tmp/openclawenv-coupled assertions), #101965 (browser navigation-guard vs SSRF tightening from #100835), #101966 (Slack thread-session-key ACP routing). Focused suites for every touched surface pass locally. Branch autoreview surfaced one finding (the guided flow could launch the chat TUI while the wizard prompter still owned the terminal) — fixed in 422e1b4 by mirroring the classic finalize pre/post terminal restore.