fix(macos): harden fresh AI onboarding#102637
Conversation
![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: 5f0477e089
ℹ️ 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".
| runtime: params.runtime, | ||
| workspaceDir: tempDir, | ||
| }); | ||
| if (!ensured.installed) { |
There was a problem hiding this comment.
When the current config makes modelSelectionShouldEnsureCodexPlugin return false (for example an existing models.providers.openai.baseUrl custom endpoint), ensureRuntimePluginForModelSelection returns { required: false, installed: false }; this new check treats that legitimate “not required” result as an install failure and returns before the live Codex test. A user with a custom OpenAI-compatible provider config who explicitly selects the detected Codex CLI will now see a misleading plugin-install failure instead of exercising the candidate; gate this on ensured.required && !ensured.installed or ensure against the Codex test plan instead.
Useful? React with 👍 / 👎.
|
Codex review: found issues before merge. Reviewed July 9, 2026, 7:20 PM ET / 23:20 UTC. Summary PR surface: Source +186, Tests +772, Docs +31, Other +1113. Total +2102 across 41 files. Reproducibility: yes. The linked issue provides fresh macOS VM steps, and current-main source still runs the Codex live setup test before ensuring the runtime plugin; I did not rerun the Parallels flow in this read-only review. Review metrics: 2 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: Fix the decode/protocol-error reconciliation path, refresh against current main, then land this as the linked macOS Codex onboarding fix if maintainers accept the additive harness auth-bootstrap contract and success-gated Codex setup persistence. Do we have a high-confidence way to reproduce the issue? Yes. The linked issue provides fresh macOS VM steps, and current-main source still runs the Codex live setup test before ensuring the runtime plugin; I did not rerun the Parallels flow in this read-only review. Is this the best way to solve the issue? No as currently submitted. The owner-boundary direction is acceptable, but the decode/protocol-error reconciliation bug must be fixed and maintainers must accept the auth/provider compatibility change before this is the best landable fix. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 50cb0d66b8e6. Label changesLabel justifications:
Evidence reviewedPR surface: Source +186, Tests +772, Docs +31, Other +1113. Total +2102 across 41 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 (15 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
openclaw-barnacle
Bot
added
docs
steipete
changed the title
clawsweeper
Bot
added
the
merge-risk: 🚨 availability
…rding-plugin-bootstrap # Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c626095390
ℹ️ 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".
| } | ||
|
|
||
| func handleConnectionModeChange(updatePageMonitoring: ((Int) -> Void)? = nil) { | ||
| self.resetGatewayBoundAIState() |
There was a problem hiding this comment.
When connectionMode changes after the user has already advanced past the AI page (for example from an external config save or route change while they are on Permissions/Ready), this reset clears aiSetup.connected but reconcilePageForModeChange can keep the current later page if it still exists in the new pageOrder. Since canAdvance only blocks while the active page is the AI page, the user can click Next/Finish without retesting inference against the new gateway; force the cursor back to the AI page or make the gate position-based after resetting route-bound AI state.
Useful? React with 👍 / 👎.
…rding-plugin-bootstrap # Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: babcd5f850
ℹ️ 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".
| private func resetRemoteProbeFeedback() { | ||
| self.remoteProbeState = .idle | ||
| self.remoteAuthIssue = nil | ||
| self.resetGatewayBoundAIState() |
There was a problem hiding this comment.
When a remote-route field such as URL, token, identity, or transport changes via config sync while the user is already on the AI page, this reset clears aiSetup.started and returns the model to .idle, but this path never calls updateMonitoring/maybeStartAISetup like the connection-mode reset does. The page then stays in the “Looking for AI…” state without dispatching a new crestodian.setup.detect until the user navigates away and back; route this reset through the same monitoring restart path or explicitly restart AI setup when the active page is the AI page.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0981a9659a
ℹ️ 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".
| static func activationIsPersisted( | ||
| expectedModel: String, | ||
| setupComplete: Bool, | ||
| configuredModel: String?) -> Bool | ||
| { | ||
| setupComplete && configuredModel == expectedModel |
There was a problem hiding this comment.
When the selected candidate's model is already present in config (for example the existing-model candidate, or Codex selecting the same openai/gpt-5.5 default), a timed-out or dropped crestodian.setup.activate request can enter the reconciliation path and this predicate returns true solely from the pre-existing configuredModel/setupComplete reported by crestodian.setup.detect (those are derived from current config in src/crestodian/setup-inference.ts:200 and :214). That lets onboarding mark AI as connected without proving the live completion succeeded, and for Codex can also skip proving the runtime pin/install reached the final persisted state.
Useful? React with 👍 / 👎.
|
Land-ready proof for exact head
Known proof gaps: none for the changed behavior. Release-note text is in the PR body; the landing gate reports repository changelog not required for this changed-file set. |
|
Merged via squash.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6f742d398
ℹ️ 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".
| ...strippedCurrent, | ||
| plugins: { | ||
| ...strippedCurrent.plugins, | ||
| installs: { codex: pendingCodexInstall }, |
There was a problem hiding this comment.
When current.plugins.installs already contains another pending install record (for example, a provider/channel install that has not yet been committed into the installed-plugin index), this transform strips all pending records and then writes an installs map containing only Codex. Since transformConfigWithPendingPluginInstalls can only migrate records that remain in nextConfig, the other plugin's ownership record is silently dropped, leaving its enabled config without install metadata after this setup path; merge the Codex record with existing pending installs instead of replacing the map.
Useful? React with 👍 / 👎.
* fix(macos): bootstrap Codex before onboarding test * fix(plugins): activate deferred runtimes on demand * fix(macos): stage Codex runtime for onboarding probe * fix(macos): expose sanitized onboarding errors * fix(codex): reuse native CLI auth during onboarding * fix(macos): hide Crestodian without inference * fix(codex): honor explicit runtime policy during setup * fix(onboarding): redact verification errors * fix(macos): reset onboarding state with gateway route * fix(onboarding): persist Codex plugin install metadata * chore: refresh onboarding inventories * fix(macos): restart AI setup after gateway change * chore: refresh native onboarding inventory * fix(onboarding): defer gateway restart until setup persists * fix(macos): reset Crestodian on gateway changes * chore(macos): refresh native i18n inventory * fix(onboarding): harden inference setup state * docs(skills): reuse pristine Parallels snapshots * chore(macos): refresh onboarding i18n inventory * fix(onboarding): prevent stale gateway and install state * docs(skills): preserve saved Parallels sessions * fix(macos): balance AI onboarding layout * fix(parallels): parse npm workspace pack output * chore(macos): refresh onboarding i18n inventory * fix(parallels): bundle workspace runtime in package artifact * fix(parallels): isolate canonical package helper * fix(parallels): pack self-contained workspace artifact * fix(packaging): exclude macOS app bundle * fix(macos): restart inference checks after route changes * docs(changelog): defer onboarding note to release * fix(onboarding): enforce inference-first gateway setup
1 participant
Closes #102614
AI-assisted change; implementation and proof were reviewed by the submitting maintainer.
What Problem This Solves
Fresh macOS onboarding had four related failure modes:
The hosted macOS Swift job also used
swift test --show-codecov-path, which only printed a coverage path and exited instead of running the suite.Why This Change Was Made
Codex contract checked directly at
openai/codex@0d18d4f7535d93ce7fd0ffa9ca69937321352688: the CLI accepts API-key login through stdin and persists it (CLI dispatch); auth loading falls back to the configured persistent store when environment auth is disabled (auth manager); and app-server constructs its auth manager with API-key environment loading disabled (app-server bootstrap).User Impact
Connecting the Mac app to an existing configured Gateway now goes straight to the normal web UI. A fresh or incomplete Gateway stays on inference setup; only after a real model turn succeeds does Crestodian become available to configure the rest. Native Codex/Claude CLI credentials remain reusable without a second provider login.
Release note: macOS inference onboarding now opens configured Gateways directly, keeps Crestodian hidden until intelligence works, verifies the effective default-agent route, and discards stale setup replies after Gateway changes.
Evidence
f6f742d3982:2795entries, no drift), andgit diff --checkpassed.cf04edbb000f0768168a499233f4675c527625a9, using a Developer ID-signed app on a restored Parallels macOS 26.5 VM:vm codex ok 20260709; after killing and relaunching the app, the same session/model returnedvm relaunch ok.pnpm check:changed, type checks, core/plugin lint, package guards, macOS packaging, and focused runtime tests (tbx_01kx3epgvt59amzcwd9r62k3pe). Final-head hosted checks are the merge gate.