feat: verify AI access during macOS onboarding before the first chat#100288
Conversation
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9833cc070a
ℹ️ 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".
| var isAISetupBlocking: Bool { | ||
| self.activePageIndex == self.aiPageIndex && | ||
| self.state.connectionMode != .unconfigured && | ||
| !self.aiSetup.connected |
There was a problem hiding this comment.
When a user successfully verifies AI for one target, then goes Back and changes from remote to local (or to another remote gateway), this gate still trusts the same aiSetup.connected flag. Since OnboardingAISetupModel is a single unkeyed state object and startIfNeeded() will no-op after the first run, the new target can skip its own live inference check and reach the first chat with an unverified/broken backend. Reset or key the AI setup state whenever the connection mode/preferred gateway changes.
Useful? React with 👍 / 👎.
| await updateConfig((current) => | ||
| applyAuthProfileConfig(current, { profileId, provider, mode: "api_key" }), | ||
| ); |
There was a problem hiding this comment.
For manual API keys, the profile is staged before the live test and only restored when the test itself returns ok: false; once this config update succeeds or any following applySetup call throws (for example due to an invalid config, base-hash conflict, or disk error), the RPC fails while the newly staged ${provider}:manual key remains and may have overwritten a previous working manual profile. Wrap the post-test persistence phase in a catch/finally that restores stagedProfile on failure.
Useful? React with 👍 / 👎.
| { name: "plugins.sessionAction", scope: "dynamic" }, | ||
| { name: "crestodian.chat", scope: "operator.admin" }, | ||
| { name: "crestodian.setup.detect", scope: "operator.admin" }, | ||
| { name: "crestodian.setup.activate", scope: "operator.admin" }, |
There was a problem hiding this comment.
crestodian.setup.activate can run a long live model test and then mutate gateway-wide config/auth state via activateSetupInference, but this descriptor is not marked controlPlaneWrite. handleGatewayRequest only applies the shared write budget to methods with that flag (src/gateway/server-methods.ts:709), so an admin client can fire unthrottled activation attempts instead of being limited like config.patch/config.apply; add controlPlaneWrite: true to the activate method.
Useful? React with 👍 / 👎.
|
Codex review: found issues before merge. Reviewed July 5, 2026, 9:57 AM ET / 13:57 UTC. Summary Reproducibility: yes. for the review blockers: source inspection shows the stale setup state, manual auth staging, Codex ensure discard, and missing control-plane write flag paths. I did not run live reproduction because this review was read-only. 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:
Mantis proof suggestion Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Land a narrowed PR that keeps the verified AI onboarding gate/RPCs, preserves auth and plugin install records atomically, marks activation as a throttled control-plane write, and splits unrelated feature and release-note changes. Do we have a high-confidence way to reproduce the issue? Yes for the review blockers: source inspection shows the stale setup state, manual auth staging, Codex ensure discard, and missing control-plane write flag paths. I did not run live reproduction because this review was read-only. Is this the best way to solve the issue? No: the verified AI gate is the right end state, but this branch is not the best mergeable shape until auth/profile persistence is atomic, setup activation is throttled, and unrelated changes are split or explicitly approved. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8a698108e096. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity concerns:
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 (3 earlier review cycles)
|
clawsweeper
Bot
added
proof: sufficient
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b475dde6
ℹ️ 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".
| let key = self.manualKey.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !key.isEmpty, !self.manualTesting else { return } | ||
| self.manualError = nil | ||
| self.manualTesting = true |
There was a problem hiding this comment.
When the manual API-key path is used, this guard only blocks another manual submit and leaves the setup model in .ready while manualTesting runs. If no candidates were found, the window can be closed without the busy confirmation while crestodian.setup.activate is still staging/auth-writing; if the manual form is visible after failed candidates, the user can start a candidate retry and then press Connect because the manual button/guard do not honor phase == .testing, causing two activation RPCs to race and whichever finishes last can overwrite the persisted model/auth state. Treat manual activation as the same busy phase/gate as candidate activation before sending the RPC.
Useful? React with 👍 / 👎.
clawsweeper
Bot
removed
proof: sufficient
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eddc84f6ff
ℹ️ 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 updateConfig = | ||
| deps.updateConfig ?? (await import("../commands/models/shared.js")).updateConfig; | ||
| const { enablePluginInConfig } = await import("../plugins/enable.js"); | ||
| await updateConfig((current) => enablePluginInConfig(current, "codex").config); |
There was a problem hiding this comment.
When the Codex option is the first working candidate on a packaged install where the Codex plugin is not already recorded, ensureCodexRuntimePluginForModelSelection returns a config containing the install record/load metadata that other callers persist via codexInstall.cfg, but this branch discards it and only writes plugins.entries.codex.enabled. That leaves the setup default model routed through Codex while the gateway has no durable record for where the newly installed @openclaw/codex plugin lives, so the first post-onboarding run can fail to load the harness after this promised auto-install path.
Useful? React with 👍 / 👎.
| return { ok: false, status: "unavailable", error: plan.error }; | ||
| } | ||
|
|
||
| const agentDir = resolveAgentDir(cfg, resolveDefaultAgentId(cfg)); |
There was a problem hiding this comment.
When an existing config has a non-main default agent, the manual API key is staged under that default agent's auth store here, but the live test later runs as agentId: "crestodian" without passing the same agentDir; Crestodian's auth loading inherits the main store, not an arbitrary default-agent store. In that setup, a valid pasted key is invisible to the verification run, so manual activation fails even though the key was just staged.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
…en empty-state claim
eddc84f to
5095bf9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5095bf943b
ℹ️ 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 upserted = await upsertAuthProfileWithLock({ | ||
| profileId: params.profileId, | ||
| credential: { type: "api_key", provider: params.provider, key: params.apiKey }, | ||
| agentDir: params.agentDir, |
There was a problem hiding this comment.
When a user already has a <provider>:manual profile selected, this upsert replaces that live profile before the manual key has passed crestodian.setup.activate verification. While the test is running, any existing or new agent run that resolves the same profile reads the unverified pasted key, so a bad key can break unrelated sessions even if rollback later restores the prior value; stage the candidate under a unique temporary profile and only promote it after success.
Useful? React with 👍 / 👎.
openclaw-barnacle
Bot
added
channel: voice-call
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Merged via squash.
|
…l restart (#100488) * fix(macos): follow gateway endpoint changes in the open Dashboard window In remote SSH mode the Dashboard window baked the forwarded tunnel's local port into the WebView URL and injected auth at open time; when RemoteTunnelManager recreated the tunnel on a new ephemeral port the open window reconnected to the dead port forever. DashboardManager now subscribes to GatewayEndpointStore and, while the window is open, re-injects auth for the new origin and reloads the WebView without stealing focus. Dashboard log lines now strip the #token= fragment so endpoint changes never write credentials to unified logs. Fixes #100476 * test(macos): drop stale hasCrestodianSetupAuth assertions OnboardingView.hasCrestodianSetupAuth was removed with the replaced OnboardingView+CrestodianSetup source in #100288, leaving the macOS test target failing to compile on main. Delete the test for the removed internal helper.
…penclaw#100288) * feat(crestodian): add live-tested structured inference setup (detect/activate gateway RPCs) * feat(macos): redesign onboarding around a verified Connect-your-AI step * docs: describe the verified AI onboarding step and gemini setup ladder entry * chore(macos): drop replaced OnboardingView+CrestodianSetup source * fix(macos): keep the AI-detect error card from pairing with an unproven empty-state claim * chore(protocol): regenerate Swift gateway models for crestodian.setup methods * test(crestodian): give setup-inference mocks explicit params for test-types lane * chore(i18n): sync native app string inventory for onboarding redesign * chore(i18n): sync native app string inventory for onboarding redesign
…l restart (openclaw#100488) * fix(macos): follow gateway endpoint changes in the open Dashboard window In remote SSH mode the Dashboard window baked the forwarded tunnel's local port into the WebView URL and injected auth at open time; when RemoteTunnelManager recreated the tunnel on a new ephemeral port the open window reconnected to the dead port forever. DashboardManager now subscribes to GatewayEndpointStore and, while the window is open, re-injects auth for the new origin and reloads the WebView without stealing focus. Dashboard log lines now strip the #token= fragment so endpoint changes never write credentials to unified logs. Fixes openclaw#100476 * test(macos): drop stale hasCrestodianSetupAuth assertions OnboardingView.hasCrestodianSetupAuth was removed with the replaced OnboardingView+CrestodianSetup source in openclaw#100288, leaving the macOS test target failing to compile on main. Delete the test for the removed internal helper.
1 participant
Closes #100286
What Problem This Solves
Fixes an issue where new macOS users could finish onboarding without any working AI backend and meet their agent with a raw error ("No API key found for provider "openai"…" with an auth-store path) in the very first chat. Also fixes the Local/Remote page reading as perpetually loading (always-on discovery spinner), the install page running two step spinners at once, and the lack of any structured choice when multiple AI credentials exist on the machine.
Why This Change Was Made
Inference is the one thing onboarding must actually secure — everything else is optional. The previous flow gated on "config was authored", not "a model answers", so broken-but-present logins (expired Codex subscription, logged-out CLI, bad key) surfaced only at first chat, in the ugliest possible place. Setup now proves inference with a real completion before persisting anything, walking the documented ladder automatically and ending in a verified manual API-key step, so the first conversation can never start broken.
User Impact
OPENAI_API_KEY/ANTHROPIC_API_KEY; shows all of them with a Recommended badge; auto-tests the best one with a live "reply with OK" completion; on failure marks the card with a friendly reason and automatically tries the next; if nothing works, a manual API-key form (Anthropic/OpenAI/Google) is verified the same way. Config is written only after a test passes (crestodian.setup.activateis test-first; a failed candidate never mutates config). Codex selection auto-installs/enables the Codex plugin.google-gemini-cli/gemini-3.1-pro-preview) after Codex;openclaw crestodiansetup and the app now share the exact same detection ladder.Gateway protocol: adds
crestodian.setup.detect/crestodian.setup.activate(additive, operator.admin scope).Evidence
node scripts/run-vitest.mjs src/crestodian/setup-inference.test.ts src/commands/onboard-inference.test.ts src/crestodian/onboarding-welcome.test.ts— 15 passed;src/crestodian/ + gateway crestodian + server-methods-list— 26 passed;packages/gateway-protocol— 232 passed.pnpm tsgoclean,scripts/run-oxlint.mjsclean on touched files,pnpm buildclean (noINEFFECTIVE_DYNAMIC_IMPORT),swift build+scripts/package-mac-app.shclean,pnpm format:swiftclean.openclaw gateway call crestodian.setup.detect→{"candidates":[],"workspace":"/Users/steipete/.openclaw/workspace","setupComplete":false};crestodian.setup.activate {"kind":"claude-cli"}on a machine without Claude →{"ok":false,"status":"unknown","error":"spawn claude ENOENT"}with zero config diff (verified).OPENCLAW_STATE_DIR): detect returned claude-cli (recommended) + codex-cli + gemini-cli in ladder order;activate {kind:"claude-cli"}ran a real Claude Code completion (turn 8.4s, output "OK") and only then authored config: default modelclaude-cli/claude-opus-4-8, workspace + bootstrap files,gateway.auth.mode: token, wizard metadata.Connect-your-AI page (error card + manual key form, captured live in the VM): https://github.com/steipete/openclaw-pr-assets/releases/download/onboarding-ai-setup-2291e9/onboarding-connect-your-ai-error-state.png