feat(crestodian): run CLI harnesses on the agent loop via a ring-zero MCP server#100029
Conversation
openclaw-barnacle
Bot
added
docs
|
Codex review: found issues before merge. Reviewed July 4, 2026, 7:22 PM ET / 23:22 UTC. Summary Reproducibility: yes. The PR defects are source-reproducible by tracing the ring-zero tool approval path, macOS chat host, plugin install parser/executor, channel setup hook path, CLI inference detection, setup flag dispatch, and tool schema mapping. Review metrics: 2 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: Fix the source blockers, resolve the merge conflict, refresh macOS/MCP/setup proof, and have maintainers either accept the ring-zero MCP trust model or split the CLI-harness bridge after #99935 lands. Do we have a high-confidence way to reproduce the issue? Yes. The PR defects are source-reproducible by tracing the ring-zero tool approval path, macOS chat host, plugin install parser/executor, channel setup hook path, CLI inference detection, setup flag dispatch, and tool schema mapping. Is this the best way to solve the issue? No. The exclusive MCP approach is plausible, but this branch is not the best mergeable solution until the security, compatibility, provider-selection, channel-finalization, paused-mode, setup-gating, timeout, and related-PR overlap issues are resolved. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 1c9e998d98f8. Label changesLabel 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 (8 earlier review cycles)
|
![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: 7340aceccf
ℹ️ 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 detected: InferenceBackendCandidate | undefined = candidates.find( | ||
| (candidate) => candidate.kind !== "existing-model", | ||
| ); |
There was a problem hiding this comment.
When the only detected inference backend is an installed-but-logged-out CLI, detectInferenceBackends() returns it with credentials: false, but this selection still accepts it and setup writes that model as the default. On Linux/non-macOS machines with claude or codex installed but no stored login or API key, first-run onboarding will claim it can reuse that backend and leave the user configured with an unusable default model instead of reporting that no usable model was found or asking them to log in.
Useful? React with 👍 / 👎.
| "set_default_model", | ||
| "config_set", | ||
| "config_set_ref", | ||
| "create_agent", |
There was a problem hiding this comment.
When the model follows the tool schema/system prompt and calls create_agent, operationForAction() has no matching case for this advertised action, so the call falls into the default branch and returns crestodian: unknown action "create_agent" instead of reaching the existing create-agent executor in operations.ts. This breaks agent creation specifically in the new ring-zero tool path; either map the action to { kind: "create-agent", ... } or stop advertising it.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25fd978229
ℹ️ 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".
| opts.classic === true || | ||
| opts.flow !== undefined || | ||
| opts.mode === "remote" || | ||
| opts.importFrom !== undefined || | ||
| opts.authChoice !== undefined |
There was a problem hiding this comment.
When an interactive user passes setup-affecting flags outside this small list, such as --gateway-port, --gateway-bind, --skip-bootstrap, --install-daemon, or --skip-channels, this predicate returns false and setupWizardCommand falls through to the new conversational path; runConversationalOnboarding only forwards workspace, so those public CLI options are silently ignored and quickstart defaults are written instead. Please either route all explicit setup flags to the classic wizard or reject/handle them in the Crestodian bootstrap path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9071127150
ℹ️ 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 canAdvance: Bool { | ||
| !self.isCLIBlocking && !self.isWizardBlocking | ||
| !self.isCLIBlocking |
There was a problem hiding this comment.
In local onboarding, pageOrder still puts the Crestodian setup page before permissions/finish, but canAdvance now only checks the CLI gate, so the Next button is enabled on the setup page as soon as the CLI is installed. A first-run user can click through without saying yes, and finish() will mark onboarding complete even though applyCrestodianSetup never created the config/workspace/gateway defaults; the removed wizard blocking state prevented exactly that path.
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.
This macOS request timeout is shorter than the server-side Crestodian agent-turn deadline (AGENT_TURN_DEADLINE_MS = 180_000), so any valid slow CLI/tool turn that finishes between 120s and 180s will surface as an onboarding error in the app while the gateway may still complete the operation and send a reply the UI has already dropped. Align the client timeout with the server deadline, or lower the server deadline, so long turns do not leave onboarding out of sync.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 896139fafa
ℹ️ 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 params = (args ?? {}) as Record<string, unknown>; | ||
| const operation = operationForAction(params); | ||
| const persistent = isPersistentCrestodianOperation(operation); | ||
| if (persistent && params.approved !== true) { |
There was a problem hiding this comment.
When Crestodian is running through the new MCP tool path, this approval gate trusts approved from the tool arguments, which are generated by the model. In any agent-loop turn where the model emits approved: true without a prior explicit user confirmation, mutating actions such as setup, config_set, doctor_fix, or gateway restarts are passed to executeCrestodianOperation as approved and can change local state, bypassing the chat engine's deterministic pending-approval flow. The approval proof needs to come from host-side conversation state or a nonce/token the model cannot mint by itself.
Useful? React with 👍 / 👎.
… MCP server CLI harnesses (claude-cli, gemini-cli) cannot enforce runtime toolsAllow, so Crestodian previously fell back to the single-turn planner for them. The openclaw-tools stdio MCP entry can now serve the ring-zero crestodian tool (OPENCLAW_TOOLS_MCP_TOOLS=crestodian), and Crestodian CLI runs inject it as the run's exclusive MCP surface: no loopback server, no plugin/user MCP servers, with the server kept under the "openclaw" name so existing tool pre-approvals apply. agent-turn routes configured or detected CLI-harness models through runCliAgent with native session resume across turns, keeps embedded toolsAllow enforcement for API-key models and the Codex app-server fallback, and still degrades to the planner when no loop backend works. The host-verified approval contract survives the process boundary: per-turn arming and the pending exact-operation hash travel to the stdio server via the generated MCP config env, and the host mirrors proposal transitions back from harness tool events (denial registers the hash, mismatch voids it, execution consumes it) so a generic yes can never authorize a different mutation. The trust model (the CLI owns its native tools) is documented in docs/cli/crestodian.md.
896139f to
79ca424
Compare
openclaw-barnacle
Bot
removed
app: macos
openclaw-barnacle
Bot
added
size: L
and removed
cli
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79ca424c2a
ℹ️ 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 args = | ||
| typeof evt.data.args === "object" && evt.data.args !== null | ||
| ? (evt.data.args as Record<string, unknown>) | ||
| : {}; | ||
| const transition = resolveCrestodianProposalTransition({ | ||
| args, | ||
| resultText: extractToolResultText(evt.data.result) ?? "", |
There was a problem hiding this comment.
When Crestodian runs through the CLI MCP path, the CLI event stream only includes tool arguments on the start event, while the result event has toolCallId, name, isError, and result but no args (checked src/agents/cli-runner/execute.ts:945-985). This result-only mirror therefore passes {} into resolveCrestodianProposalTransition, so the host never records the needs-approval proposal hash from the subprocess; on the user's next yes, the new subprocess starts without OPENCLAW_TOOLS_MCP_CRESTODIAN_PROPOSAL and every approved mutating action mismatches instead of executing. Store args by toolCallId from the start event or include them on the result event before resolving the transition.
Useful? React with 👍 / 👎.
… MCP server (openclaw#100029) CLI harnesses (claude-cli, gemini-cli) cannot enforce runtime toolsAllow, so Crestodian previously fell back to the single-turn planner for them. The openclaw-tools stdio MCP entry can now serve the ring-zero crestodian tool (OPENCLAW_TOOLS_MCP_TOOLS=crestodian), and Crestodian CLI runs inject it as the run's exclusive MCP surface: no loopback server, no plugin/user MCP servers, with the server kept under the "openclaw" name so existing tool pre-approvals apply. agent-turn routes configured or detected CLI-harness models through runCliAgent with native session resume across turns, keeps embedded toolsAllow enforcement for API-key models and the Codex app-server fallback, and still degrades to the planner when no loop backend works. The host-verified approval contract survives the process boundary: per-turn arming and the pending exact-operation hash travel to the stdio server via the generated MCP config env, and the host mirrors proposal transitions back from harness tool events (denial registers the hash, mismatch voids it, execution consumes it) so a generic yes can never authorize a different mutation. The trust model (the CLI owns its native tools) is documented in docs/cli/crestodian.md.
1 participant
Related: #99935
What Problem This Solves
Crestodian's conversational onboarding runs the real embedded agent loop with a single ring-zero
crestodiantool, but CLI harnesses (Claude Code, Gemini CLI) reject OpenClaw's restrictedtoolsAllowand fail closed. On machines whose only usable model is a local CLI (the most common fresh-machine case: Claude Code installed, no API keys), Crestodian silently degraded to the bounded single-turn planner: no tool calls, no multi-turn memory, no in-loop config repair.Why This Change Was Made
CLI harnesses cannot enforce an OpenClaw tool allow-list, but they can be handed a restricted MCP surface. The
openclaw-tools-servestdio MCP entry now serves the ring-zerocrestodiantool on request (OPENCLAW_TOOLS_MCP_TOOLS=crestodian), and Crestodian CLI runs inject it as the run's exclusive MCP config: no loopback server, no plugin or user MCP servers, applied with--strict-mcp-configfor Claude Code. The server keeps theopenclawname so existing per-backend tool pre-approvals apply unchanged. The gate is a typedcrestodianToolrun parameter set only by the Crestodian agent runner, so normal agent runs can never see the tool. Native CLI tools remain under the harness's own permission policy; the trust model is documented indocs/cli/crestodian.md. Embedded runtimes and the Codex app-server harness keep the hardtoolsAllowenforcement.This PR includes the five commits from #99935 (
claude/wizardly-ishizaka-7410c7) rebased onto currentmain; identical patches drop automatically if that PR lands first.User Impact
Crestodian conversations on Claude Code or Gemini CLI models (configured or auto-detected) now run the full agent loop: real tool calls with approval-gated, audited config mutations, post-write validation fed back into the loop, and genuine multi-turn memory with native CLI session resume. The single-turn planner remains as fallback when no loop-capable backend works, and deterministic typed commands still work with no model at all.
Evidence
CrestodianChatEnginewith the Codex app-server fallback on this machine. The loop invoked the ring-zero tool (start:crestodian/result:crestodianagent events), replied with grounded status facts ("gatewayws://127.0.0.1:18789unreachable"), and the second turn answered a memory question from the persistent session in 3.3s.--strict-mcp-config, init event lists theopenclawserver; argv captured in run logs). Model inference itself was blocked on this host because the standaloneclaudeCLI is logged out - the already-shipped planner path fails identically there, confirming an environment issue rather than a regression.dist/mcp/openclaw-tools-serve.js(the exact transport CLI harnesses use):tools/listreturns exactly["crestodian"],crestodian(status)executes and returns real status output, and an unapprovedgateway_restartis refused withneeds-approval, proving the approval gate holds over MCP.chat-engine(16),prepare(61, includes a new ring-zero test proving no loopback + crestodian-only MCP config),bundle-mcp(17),openclaw-tools-serve(7), plus crestodian tool/assistant/operations suites - all green after rebase ontomain.pnpm tsgo,pnpm check:test-types, oxlint,pnpm check:import-cycles, andpnpm build(no ineffective-dynamic-import findings) all pass; Codex autoreview (gpt-5.5) exited clean with no actionable findings.--messagemode never enters the chat engine, so the new loop cannot intercept its planner flow.