fix(agents): restore Claude CLI OpenClaw MCP loopback bridge#60714
Conversation
PR SummaryMedium Risk Overview Centralizes tool listing/invocation policy by introducing Extends CLI bundle MCP preparation to support Reviewed by Cursor Bugbot for commit 6a71bdc. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Landed via temp rebase onto main.
Thanks @mylukin! |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6a71bdc. Configure here.
| delete merged.const; | ||
| mergedProps[key] = merged; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Union schema flattening drops third+ variant const values
Low Severity
flattenUnionSchema fails to merge const values from 3+ variants of the same property key. After the first two const variants are merged, the result has an enum array (with const deleted). When a third variant arrives with a const value, neither the enum+enum branch (incoming lacks enum) nor the const+const branch (existing lacks const) matches, so the value is silently dropped with only a warning log. A missing branch for existing.enum + incoming.const means tools with richer anyOf/oneOf schemas lose valid input values in the MCP tool listing.
Reviewed by Cursor Bugbot for commit 6a71bdc. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a71bdc65c
ℹ️ 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 next = resolveGatewayScopedTools({ | ||
| cfg, | ||
| sessionKey, | ||
| messageProvider, | ||
| accountId, |
There was a problem hiding this comment.
Apply owner-only tool filtering before exposing MCP tools
The loopback MCP handler resolves next.tools and exposes them directly, but it never applies applyOwnerOnlyToolPolicy the way /tools/invoke does. In deployments that intentionally allow owner-only tools in gateway.tools.allow (for example cron, gateway, nodes, or whatsapp_login), non-owner sessions using Claude CLI can still discover and invoke those tools through MCP, which is a privilege escalation relative to existing gateway behavior.
Useful? React with 👍 / 👎.
| const trimmed = rawSessionKey?.trim(); | ||
| return !trimmed || trimmed === "main" ? resolveMainSessionKey(cfg) : trimmed; |
There was a problem hiding this comment.
Honor agent header when session key is absent
Session resolution currently defaults to the main session whenever x-session-key is empty, and this code path ignores the new x-openclaw-agent-id header that the CLI runner now injects. As a result, runs that specify agentId without sessionKey can resolve tools/policies for the main agent instead of the intended agent, leading to incorrect cross-agent tool scope.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds an in-process MCP HTTP loopback bridge ( Confidence Score: 5/5Safe to merge — auth, loopback-only binding, and lifecycle cleanup are all correct; only minor P2 style observations remain. The core security properties are sound: random 32-byte token, 127.0.0.1-only bind, token cleared on server close. Lifecycle integration in server.impl.ts properly closes the MCP server in both normal and emergency shutdown paths. Tests cover the key header-to-tool-resolution mapping, auth rejection, and config merging paths. Both remaining findings are non-blocking style improvements. src/gateway/mcp-http.ts (x-openclaw-agent-id header and missing allowGatewaySubagentBinding/allowMediaInvokeCommands flags noted above) Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/mcp-http.ts
Line: 148-164
Comment:
**`x-openclaw-agent-id` header is configured but never consumed**
`createMcpLoopbackServerConfig` emits `"x-openclaw-agent-id": "${OPENCLAW_MCP_AGENT_ID}"` and `prepare.ts` populates `OPENCLAW_MCP_AGENT_ID` with `sessionAgentId`, but the MCP server's request handler never reads `x-openclaw-agent-id` (only `x-session-key`, `x-openclaw-account-id`, and `x-openclaw-message-channel` are read). Because `resolveGatewayScopedTools` already derives the agent ID from the `sessionKey`, the header is functionally redundant. Either remove it from the header map and the env injection to avoid confusion, or document that it's reserved for future use.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/gateway/mcp-http.ts
Line: 301-322
Comment:
**MCP tool resolution omits `allowGatewaySubagentBinding` and `allowMediaInvokeCommands`**
The direct `/tools/invoke` handler calls `resolveGatewayScopedTools` with `allowGatewaySubagentBinding: true` and `allowMediaInvokeCommands: true`, but the MCP loopback path doesn't pass either flag (both default to `undefined`/falsy). This creates a silent capability asymmetry: subagent-binding and media-invoke tools are reachable via HTTP but absent from the MCP tool catalog. If this is intentional scoping for the CLI bridge, a brief comment here would prevent future confusion about why the two surfaces diverge.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: restore claude cli loopback mcp bri..." | Re-trigger Greptile |
| export function createMcpLoopbackServerConfig(port: number) { | ||
| return { | ||
| mcpServers: { | ||
| openclaw: { | ||
| type: "http", | ||
| url: `http://127.0.0.1:${port}/mcp`, | ||
| headers: { | ||
| Authorization: "Bearer ${OPENCLAW_MCP_TOKEN}", | ||
| "x-session-key": "${OPENCLAW_MCP_SESSION_KEY}", | ||
| "x-openclaw-agent-id": "${OPENCLAW_MCP_AGENT_ID}", | ||
| "x-openclaw-account-id": "${OPENCLAW_MCP_ACCOUNT_ID}", | ||
| "x-openclaw-message-channel": "${OPENCLAW_MCP_MESSAGE_CHANNEL}", | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
x-openclaw-agent-id header is configured but never consumed
createMcpLoopbackServerConfig emits "x-openclaw-agent-id": "${OPENCLAW_MCP_AGENT_ID}" and prepare.ts populates OPENCLAW_MCP_AGENT_ID with sessionAgentId, but the MCP server's request handler never reads x-openclaw-agent-id (only x-session-key, x-openclaw-account-id, and x-openclaw-message-channel are read). Because resolveGatewayScopedTools already derives the agent ID from the sessionKey, the header is functionally redundant. Either remove it from the header map and the env injection to avoid confusion, or document that it's reserved for future use.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/mcp-http.ts
Line: 148-164
Comment:
**`x-openclaw-agent-id` header is configured but never consumed**
`createMcpLoopbackServerConfig` emits `"x-openclaw-agent-id": "${OPENCLAW_MCP_AGENT_ID}"` and `prepare.ts` populates `OPENCLAW_MCP_AGENT_ID` with `sessionAgentId`, but the MCP server's request handler never reads `x-openclaw-agent-id` (only `x-session-key`, `x-openclaw-account-id`, and `x-openclaw-message-channel` are read). Because `resolveGatewayScopedTools` already derives the agent ID from the `sessionKey`, the header is functionally redundant. Either remove it from the header map and the env injection to avoid confusion, or document that it's reserved for future use.
How can I resolve this? If you propose a fix, please make it concise.| : (() => { | ||
| const next = resolveGatewayScopedTools({ | ||
| cfg, | ||
| sessionKey, | ||
| messageProvider, | ||
| accountId, | ||
| excludeToolNames: NATIVE_TOOL_EXCLUDE, | ||
| }); | ||
| const nextEntry = { | ||
| tools: next.tools, | ||
| toolSchema: buildToolSchema(next.tools), | ||
| configRef: cfg, | ||
| time: now, | ||
| }; | ||
| toolCache.set(cacheKey, nextEntry); | ||
| for (const [key, entry] of toolCache) { | ||
| if (now - entry.time >= TOOL_CACHE_TTL_MS) { | ||
| toolCache.delete(key); | ||
| } | ||
| } | ||
| return nextEntry; | ||
| })(); |
There was a problem hiding this comment.
allowGatewaySubagentBinding and allowMediaInvokeCommands
The direct /tools/invoke handler calls resolveGatewayScopedTools with allowGatewaySubagentBinding: true and allowMediaInvokeCommands: true, but the MCP loopback path doesn't pass either flag (both default to undefined/falsy). This creates a silent capability asymmetry: subagent-binding and media-invoke tools are reachable via HTTP but absent from the MCP tool catalog. If this is intentional scoping for the CLI bridge, a brief comment here would prevent future confusion about why the two surfaces diverge.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/mcp-http.ts
Line: 301-322
Comment:
**MCP tool resolution omits `allowGatewaySubagentBinding` and `allowMediaInvokeCommands`**
The direct `/tools/invoke` handler calls `resolveGatewayScopedTools` with `allowGatewaySubagentBinding: true` and `allowMediaInvokeCommands: true`, but the MCP loopback path doesn't pass either flag (both default to `undefined`/falsy). This creates a silent capability asymmetry: subagent-binding and media-invoke tools are reachable via HTTP but absent from the MCP tool catalog. If this is intentional scoping for the CLI bridge, a brief comment here would prevent future confusion about why the two surfaces diverge.
How can I resolve this? If you propose a fix, please make it concise.
Replacement land for #35676 by @mylukin.
Summary
/tools/invokehonor the same session/account/channel policy filteringTesting
pnpm test src/gateway/tools-invoke-http.test.ts src/gateway/mcp-http.test.ts src/agents/cli-runner/bundle-mcp.test.tspnpm check(fails on unrelated preexistingmainerrors inextensions/matrix/src/doctor.ts,src/agents/openai-ws-stream.ts, and several cache/live test typings)pnpm build(fails on unrelated preexistingmainerror insrc/agents/openai-ws-stream.ts)