feat(crestodian): conversational agent-loop onboarding across CLI, web install, and macOS app#99935
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3ceef35b5
ℹ️ 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".
| workspace?: string; | ||
| }): Promise<string> { | ||
| const overview = await params.engine.loadOverview(); | ||
| const isConfigured = overview.config.exists && overview.config.valid && overview.defaultModel; |
There was a problem hiding this comment.
Require setup completion before skipping bootstrap
When a user has a valid minimal config that only sets a default model (for example from a previous config set or import) but has not completed onboarding, this condition treats the install as already configured and returns the “agent is ready” welcome instead of seeding the setup proposal. That skips the workspace, gateway defaults, and security acknowledgement that the first-run setup path is supposed to apply, leaving onboarding apparently complete while core setup is still missing.
Useful? React with 👍 / 👎.
| "set_default_model", | ||
| "config_set", | ||
| "config_set_ref", | ||
| "create_agent", |
There was a problem hiding this comment.
Map create_agent to the create-agent operation
The tool schema advertises create_agent to the Crestodian agent, but operationForAction has no matching case, so any approved tool call for agent creation falls through to the default “unknown action” error instead of executing the existing create-agent operation. This breaks the documented mutating action in the real agent-loop path even though typed chat commands can still parse create agent ....
Useful? React with 👍 / 👎.
| guard pageIndex == self.crestodianPageIndex else { return } | ||
| // Local mode reaches this page only after the CLI/gateway install page, | ||
| // so the gateway is up before the first RPC. | ||
| guard self.state.connectionMode != .local || self.cliInstalled else { return } |
There was a problem hiding this comment.
Respect paused local mode before starting chat
In local onboarding while OpenClaw is paused, the old wizard path deferred setup because the gateway is intentionally stopped, but this guard only checks cliInstalled before issuing the crestodian.chat RPC. Since GatewayConnection.request auto-recovers local failures by calling GatewayProcessManager.setActive(true), simply opening this page can restart a paused gateway instead of showing/respecting the paused setup state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4258c98245
ℹ️ 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".
| return ( | ||
| opts.classic === true || | ||
| opts.flow !== undefined || | ||
| opts.mode === "remote" || | ||
| opts.importFrom !== undefined || | ||
| opts.authChoice !== undefined |
There was a problem hiding this comment.
Route explicit onboard flags through the classic wizard
For interactive runs that pass existing setup flags but no --classic/--flow (for example openclaw onboard --gateway-port 19000 or --no-install-daemon), this predicate returns false and setupWizardCommand routes to runConversationalOnboarding, which only forwards workspace. The Crestodian setup path then calls ensureGatewayServiceForOnboarding with opts: {}, so gateway port/auth/daemon choices are silently replaced by quickstart defaults even though those public flags were honored by the previous wizard path. Please either treat any explicit setup flag as classic here or thread the flags through the conversational setup.
Useful? React with 👍 / 👎.
| var canAdvance: Bool { | ||
| !self.isCLIBlocking && !self.isWizardBlocking | ||
| !self.isCLIBlocking |
There was a problem hiding this comment.
Block advancement until Crestodian setup completes
In local onboarding, once the CLI page is done this makes the Next button enabled on the Crestodian setup page even before the user has accepted or completed the pending setup proposal. Because handleNext also no longer has a setup guard, a user can skip straight through permissions/Finish and markComplete() records onboarding as seen while no workspace, gateway defaults, or security acknowledgement were applied; the previous wizard path kept canAdvance false until the wizard completed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2a44829c4
ℹ️ 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 nextConfig = await setupChannels(baseConfig, defaultRuntime, prompter, { | ||
| initialSelection: [channel], | ||
| forceAllowFromChannels: [channel], | ||
| allowSignalInstall: true, | ||
| deferStatusUntilSelection: true, | ||
| quickstartDefaults: true, | ||
| skipDmPolicyPrompt: true, | ||
| skipConfirm: true, | ||
| }); |
There was a problem hiding this comment.
Run channel post-write hooks from chat setup
When the chat-hosted setup configures a channel whose adapter has afterConfigWritten (Matrix encrypted accounts currently do: extensions/matrix/src/onboarding.ts:754 runs verification bootstrap), this path calls setupChannels and immediately writes the config without passing onPostWriteHook or draining runCollectedChannelOnboardingPostWriteHooks like channels add does. The chat can then report the channel as configured while the required post-write bootstrap never runs, leaving encrypted Matrix accounts without their recovery/backup setup.
Useful? React with 👍 / 👎.
| let data = try await GatewayConnection.shared.request( | ||
| method: "crestodian.chat", | ||
| params: params, | ||
| timeoutMs: 120_000) |
There was a problem hiding this comment.
Keep macOS chat timeout above server turns
When Crestodian takes the real agent-loop path, the gateway can wait up to 180s for the turn (src/crestodian/chat-engine.ts:213), and the embedded run itself is allowed 120s (src/crestodian/agent-turn.ts:21). This macOS request times out at exactly 120,000ms, so a cold or slow local model turn can time out in the app before the gateway response arrives, showing an error while the server-side setup turn continues. Please either raise this client timeout above the server deadline or lower the server deadline for this RPC.
Useful? React with 👍 / 👎.
|
Codex review: found issues before merge. Reviewed July 4, 2026, 7:13 PM ET / 23:13 UTC. Summary PR surface: Source +2108, Tests +869, Docs +78, Other -282. Total +2773 across 76 files. Reproducibility: yes. source-level: current PR head still contains the patched paths that append sensitive replies, preserve stale proposalRef state on decline, skip post-write channel hooks, default away from existing workspace config, terminalize wizard validation errors, share one transcript file, and mismatch client/server deadlines. Review metrics: 3 noteworthy metrics.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land only after chat onboarding preserves the classic wizard contracts for secret handling, workspace defaults, channel hooks, validation, session reset, and deadline alignment, with refreshed macOS/CLI/channel proof. Do we have a high-confidence way to reproduce the issue? Yes, source-level: current PR head still contains the patched paths that append sensitive replies, preserve stale proposalRef state on decline, skip post-write channel hooks, default away from existing workspace config, terminalize wizard validation errors, share one transcript file, and mismatch client/server deadlines. Is this the best way to solve the issue? No: the direction is plausible for #99934, but this is not yet the best mergeable fix because it regresses existing setup contracts and security boundaries that the old wizard and channel setup paths already handled. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against dd6bc2ec32fa. Label changesLabel justifications:
Evidence reviewedPR surface: Source +2108, Tests +869, Docs +78, Other -282. Total +2773 across 76 files. View PR surface stats
Security concerns:
Acceptance criteria:
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 (15 earlier review cycles; latest 8 shown)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6760352c5
ℹ️ 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 workspace = resolveUserPath(params.workspace ?? DEFAULT_WORKSPACE); | ||
|
|
||
| params.engine.propose({ kind: "setup", workspace }); |
There was a problem hiding this comment.
Preserve the configured workspace in setup
When a user already has a valid config with agents.defaults.workspace but is not considered fully configured here (for example no default model yet), default conversational onboarding ignores that existing workspace and seeds the setup proposal with DEFAULT_WORKSPACE; accepting the proposal then calls the setup path that writes this workspace back into config. This regresses the classic wizard behavior, which used the existing configured workspace as the quickstart default, and can silently move an existing user back to ~/.openclaw/workspace unless they remember to pass --workspace.
Useful? React with 👍 / 👎.
| const detected: InferenceBackendCandidate | undefined = candidates.find( | ||
| (candidate) => candidate.kind !== "existing-model", | ||
| ); |
There was a problem hiding this comment.
Skip logged-out CLI candidates during setup
On Linux/Windows with only a claude or codex binary installed but no stored login and no API key/default model, detectInferenceBackends() returns that CLI candidate with credentials === false; this selection still accepts it and setup writes the corresponding default model. The welcome then says it will reuse an installed-but-not-logged-in backend, leaving a fresh onboarding flow configured to a model that cannot run instead of leaving the model unset and prompting the user to log in or add a key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
openclaw/src/crestodian/operations.ts
Lines 371 to 373 in c153965
When a Crestodian proposal uses a spec like npm:file:../pkg or npm:https://example/package.tgz, this guard only checks the outer string and lets it through because it starts with npm:. The plugin installer then strips that prefix (src/cli/plugins-command-helpers.ts:219-224) and passes the remaining local/URL spec to npm (src/cli/plugins-install-command.ts:1177-1193), bypassing the Crestodian restriction that plugin installs are npm/ClawHub package specs only. Please validate the inner npm: target as well before executing approved plugin-install actions.
ℹ️ 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".
…letion gate Review findings: the model-supplied approved flag alone could authorize ring-zero mutations (prompt injection / model error), and removing the macOS wizard gate let users Next past the Crestodian page with nothing configured. Mutating tool actions now also require host-verified consent (the engine arms approval only when the user's actual message is an explicit yes), and local macOS onboarding blocks advancing until setup authored the config, using the same signal the old step wizard checked.
…te dot navigation A generic yes no longer authorizes arbitrary mutations: denied mutating tool calls register a canonical operation fingerprint (host-owned, per session), and an armed turn executes only the identical call, once. The denial message is arming-aware so the approved turn self-heals in one roundtrip, and the agent protocol pre-registers proposals. macOS onboarding page dots now honor the same setup-completion gate as the Next button.
…s, gate programmatic advance Sensitive channel-wizard answers (tokens, passwords) are redacted from the AI-visible conversation history; setup and the onboarding welcome never pick or advertise a definitively logged-out CLI as the model; and macOS handleNext() honors the page gates for programmatic callers (chat handoff) just like the Next button.
…with the app gate A valid config carrying only a default model (partial/hand-written) now still gets the first-run proposal instead of the ready guide, so the macOS setup gate can always be satisfied from the conversation.
…osal An approval-mismatched call inside an armed turn no longer re-registers and invites a retry (which let the model swap the approved operation for another in the same turn); it voids the approval entirely and requires a fresh yes. Proposals register only in unarmed turns, which the agent protocol already does when proposing.
…tifacts for crestodian
afea355 to
bb658a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb658a5b48
ℹ️ 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 snapshot = await readSetupConfigFileSnapshot(); | ||
| const baseConfig: OpenClawConfig = | ||
| snapshot.valid && snapshot.exists ? (snapshot.sourceConfig ?? snapshot.config) : {}; |
There was a problem hiding this comment.
Refuse setup writes when the existing config is invalid
When openclaw.json exists but fails validation, this discards the parsed snapshot by using {} and then continues to write a fresh setup config. The classic wizard explicitly stops on the same invalid snapshot before writing, because otherwise a small malformed-but-user-authored config can be replaced with only workspace/gateway defaults, dropping agents/channels/settings instead of asking the user to run doctor/repair first.
Useful? React with 👍 / 👎.
| const { mutateConfigFile, readConfigFileSnapshot } = await loadConfigFileMutationHelpers(); | ||
| const { readConfigFileSnapshot } = await loadConfigModule(); | ||
| const before = await readConfigFileSnapshot(); | ||
| const workspace = resolveUserPath(operation.workspace ?? process.cwd()); |
There was a problem hiding this comment.
Use the onboarding default workspace for bare setup
When a user runs Crestodian setup without a workspace, this falls back to process.cwd(). In the CLI that is whatever directory they happened to launch from, and in the gateway/macOS path it can be the app/gateway working directory, so accepting the setup writes and bootstraps the wrong agents.defaults.workspace instead of the normal onboarding default or existing configured workspace.
Useful? React with 👍 / 👎.
What Problem This Solves
Closes #99934.
Onboarding was a different multi-step wizard on every surface, and the macOS onboarding chat dead-ended with
No API key found for provider "openai"on machines that already had usable inference (Claude Code/Codex logins, API keys). Setting up "an agent with intelligence" required clicking through menus instead of talking to it.Why This Change Was Made
Inference is the only required step; everything else can default or happen conversationally. Crestodian (the configless-safe setup custodian) becomes the default onboarding on all surfaces and now runs as a real agent on the production embedded agent loop, restricted to a single ring-zero
crestodiantool:openclaw onboard/ bareopenclaw(fresh install) opens the conversation. First-run welcome states the detected plan (login-aware Claude Code/Codex detection viasrc/commands/onboard-inference.ts, keychain-safe); yes applies model + workspace + bootstrap files + quickstart gateway + service install (src/crestodian/setup-apply.ts, audited). Classic wizard:--classicor any explicit step flag;--non-interactiveunchanged;--modern= deprecated alias foropenclaw crestodian.src/crestodian/agent-turn.ts,src/agents/tools/crestodian-tool.ts): same runner, session transcripts, and tool pipeline as regular agents; the tool is construction-gated (RunEmbeddedAgentParams.crestodianTool+toolsAllow) so normal agents can never receive it. Mutations requireapproved=trueasserted from explicit user consent, are audited, and post-write config validation feeds schema errors back into the loop for in-conversation self-repair. Fallback ladder: agent loop → single-turn persona planner → deterministic typed commands.config get(secret-redacting) andconfig schemaread ops ground the model in the real zod schema; writes are schema-validated with exact errors surfaced back into the chat; the engine re-validatesopenclaw.jsonafter every applied write.crestodian.chatgateway method (operator.adminscoped, works before any model auth exists — fixes the dead-end). Permissions page importance-sorted, fits without scrollbars, auto-refreshes. Onboarding version bumped.install.shalready ends inopenclaw onboard, so it inherits the conversation; docs synced.tools.exec.mode=denyfloor + tools hidden behind the tool-search index); Crestodian channel listing was empty in packaged installs; setup approval now seeds the crestodian-scoped exec approval that local model harnesses require.User Impact
Fresh install → talk to a custodian with personality that already found your AI access; say yes; connect channels by chatting; meet your agent. Existing installs are untouched (config-authored setups keep opening the TUI/app; old app versions still get the classic wizard RPC; no config shape changes, no doctor migration needed).
Evidence
pnpm tsgo,pnpm check:test-types,pnpm tsgo:extensionsgreen; 165+ tests acrosssrc/crestodian,src/agents/tools/crestodian-tool.test.ts,src/wizard,src/commands/onboard*,src/gateway/server-methods/crestodian.test.ts,src/cli/run-main.test.ts; 18/18 Swift onboarding tests (incl. new permission-order regression) +swift build.npm packtarball, isolated state):openclaw onboard→ conversation with detection → yes wroteopenclaw.json(model, gateway token) + workspace bootstrap files;channelslists 28 channels;connect telegramhosts the wizard in-chat.crestodiantool calls, reported real machine state, and proposed the next mutation behind approval.gateway.port = "banana"→ exact zod error into the chat; unknown key rejected;config get gateway.authredacts the token; invalid-after-write drives an approval-gated fix proposal (unit-tested).pnpm check:changedvia Testbox-through-Crabbox) started; Docker lanestest:docker:crestodian-{first-run,planner,rescue}should run in CI before merge. Known gap: claude-cli harness sessions use the planner fallback until the MCP tool bridge lands (follow-up filed).