fix(onboarding): skip setup for configured gateways and require inference first#102883
Conversation
openclaw-barnacle
Bot
added
app: macos
|
Codex review: needs real behavior proof before merge. Reviewed July 11, 2026, 1:15 PM ET / 17:15 UTC. Summary Reproducibility: yes. Hold a detect, verify, or chat request inside the shared keyed Gateway queue, start activation with a shorter client timeout, and then release the held request; current source shows the activation callback can still execute afterward. Review metrics: 1 noteworthy metric.
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:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Reject setup activation before it enters an occupied shared serialization path, or move it to a cancellation-aware exclusive lane that cannot execute after its RPC is gone; add held detect/verify/chat regressions and repeat exact-head live onboarding, upgrade, credential-isolation, and dependency/security proof. Do we have a high-confidence way to reproduce the issue? Yes. Hold a detect, verify, or chat request inside the shared keyed Gateway queue, start activation with a shorter client timeout, and then release the held request; current source shows the activation callback can still execute afterward. Is this the best way to solve the issue? No. The branch contains appropriate fixes for several reported failures, but activation admission still violates its fail-fast safety contract and is not the best mergeable solution until that race is removed. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 58ef98ec1311. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity 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)
|
clawsweeper
Bot
added
proof: sufficient
239d926 to
7216f59
Compare
openclaw-barnacle
Bot
added
docs
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
the
status: 📣 needs proof
![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: 5caf7089be
ℹ️ 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".
| ...(params.workspace !== undefined ? { workspace: params.workspace } : {}), | ||
| surface: "gateway", | ||
| runtime, | ||
| await runCrestodianGatewayTask(async () => { |
There was a problem hiding this comment.
Because runCrestodianGatewayTask serializes all Crestodian RPCs with KeyedAsyncQueue.enqueue, wrapping activation here means an activation accepted while a long crestodian.chat/detect/verify request is already running sits in the global queue after runExclusiveCrestodianSetupActivation has marked it in progress. If the macOS/CLI caller times out or cancels before the earlier task finishes, this queued callback can still later call activateSetupInference and persist model/credential config after the user has stopped or retried, defeating the intended fail-fast/ambiguous-activation guard. Keep activation out of this queue or fail fast when the global Crestodian lane is busy.
Useful? React with 👍 / 👎.
clawsweeper
Bot
removed
the
rating: 🦐 gold shrimp
clawsweeper
Bot
added
the
rating: 🦪 silver shellfish
clawsweeper
Bot
removed
rating: 🦪 silver shellfish
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65d111f9dc
ℹ️ 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 GATEWAY_SETUP_DETECT_TIMEOUT_MS = 20_000; | ||
| const GATEWAY_SETUP_ACTIVATE_TIMEOUT_MS = 150_000; | ||
| const GATEWAY_CODEX_SETUP_ACTIVATE_TIMEOUT_MS = 480_000; | ||
| const GATEWAY_SETUP_VERIFY_TIMEOUT_MS = 30_000; |
There was a problem hiding this comment.
When remote onboarding activates a candidate successfully, the follow-up crestodian.setup.verify runs the gateway's normal verifySetupInference path, whose inference probe defaults to 90s (src/crestodian/setup-inference.ts:94) and the gateway handler does not pass a shorter override. Capping only this client call at 30s means a slow-but-valid first remote model turn can time out locally after the gateway has already persisted the model, aborting onboarding instead of handing off to Crestodian/TUI; use a timeout at least as long as the server probe window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 049a931de8
ℹ️ 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".
| if (options.hasConfiguredGateway && !configuredGateway) { | ||
| configuredGateway = toReachableGateway(target, auth); |
There was a problem hiding this comment.
When a local Gateway is configured with gateway.bind: "tailnet" or "custom", resolveLocalGatewayProbeTargets() returns the loopback target first and the configured target second (src/cli/run-main.ts:629-647). This fallback records configuredGateway before probing, so if both probes fail while the configured Gateway is cold-starting, bare openclaw launches the TUI with OPENCLAW_GATEWAY_URL=ws://127.0.0.1:... instead of the configured tailnet/custom URL. For a Gateway that only binds to that configured address, the TUI will keep retrying the wrong endpoint and never attach when the Gateway finishes starting.
Useful? React with 👍 / 👎.
|
Land-ready at
Known proof gaps: none. |
…onboarding-safety # Conflicts: # docs/.generated/plugin-sdk-api-baseline.sha256 # docs/cli/crestodian.md # extensions/codex/src/app-server/attempt-startup.test.ts # extensions/codex/src/app-server/client.ts # extensions/codex/src/app-server/computer-use.test.ts # extensions/codex/src/app-server/computer-use.ts # extensions/codex/src/app-server/config.ts # extensions/codex/src/app-server/request.ts # scripts/plugin-sdk-surface-report.mjs # src/agents/agent-tools.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 739c3ebda3
ℹ️ 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".
| runtimeArtifact: { | ||
| kind: "bundled-package-tree", | ||
| packageName: "@google/gemini-cli", | ||
| entrypoint: "command", | ||
| }, |
There was a problem hiding this comment.
This declares Gemini as a bundled-package-tree, but the new resolver rejects any package whose package.json has a non-empty dependencies or peerDependencies map (src/agents/cli-executable-identity.ts). I checked the published @google/gemini-cli package and it currently declares many dependencies, so resolveCliExecutableIdentity() returns undefined before a Gemini setup probe can launch whenever runSetupInferenceTest() requires an execution owner. That blocks otherwise working Gemini CLI users from completing inference-gated setup/Crestodian; either fingerprint the resolved dependency tree or avoid this runtime-artifact contract for Gemini.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
2 participants
Closes #102844
Closes #102845
Closes #102846
What Problem This Solves
Clients connecting to an already configured Gateway were sent through local AI-provider onboarding. That could mutate the wrong machine, expose stale credentials across endpoint changes, and start Crestodian before usable inference existed.
Why This Change Was Made
The Gateway is the authority for its model, agent, credentials, and runtime. Configured Gateways now go directly to the normal UI. Unconfigured remote Gateways perform detection, activation, exact live verification, and Crestodian chat on that same pinned Gateway. Crestodian remains unavailable until inference succeeds.
User Impact
Evidence
5caf7089be5: frozen install,pnpm tsgo:all, and 224/224 focused tests passed; expanded CLI/Gateway/TUI coverage passed 1,022/1,022.739c3ebda38: route-edit race regressions passed 78/78; total focused macOS setup/endpoint/Dashboard/CLI/device-token coverage exceeds 300 tests.