Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 6:23 AM ET / 10:23 UTC.

Summary
The branch adds a Gateway MCP HTTP guard that rejects present non-object tools/call.params.arguments with JSON-RPC -32602 and adds loopback tests for invalid, omitted, and object argument paths.

PR surface: Source +5, Tests +67. Total +72 across 2 files.

Reproducibility: yes. at source level: current main casts params.arguments and passes it to the hook/tool path, while the PR exercises the raw loopback HTTP path for malformed, omitted, and object argument cases. I did not run tests because this review was required to keep the checkout read-only.

Review metrics: 1 noteworthy metric.

• MCP request contract tightened: 1 tightened request parameter: tools/call.params.arguments. Maintainers should notice this because malformed loopback clients may need to change from primitive, null, or array arguments to omitted or object arguments.

Stored data model
Persistent data-model change detected: serialized state: src/gateway/mcp-http.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• none.

Risk before merge

• [P1] Malformed MCP loopback clients that currently send primitive, null, or array arguments will now fail early with JSON-RPC -32602 instead of reaching before-tool hooks or tool execution.

Maintainer options:

1. Accept strict malformed-payload rejection (recommended)
   Approve the intentional behavior change because it aligns raw HTTP loopback with the MCP SDK object-argument contract and has focused compatibility coverage.
2. Request compatibility-preserving rework
   Ask the contributor to preserve current malformed-client behavior or provide a maintainer-approved transition path before merge.
3. Pause for broader MCP validation design
   Leave the PR open if maintainers want one coherent validation policy for raw HTTP MCP params instead of a targeted arguments fix.

Next step before merge

• [P2] A Gateway/MCP owner needs to choose the malformed-client compatibility policy; there is no narrow automated repair to make before that decision.

Maintainer decision needed

• Question: Should Gateway HTTP MCP loopback reject present non-object tools/call.params.arguments with JSON-RPC -32602, even though malformed clients that previously reached hooks or tool execution will now fail early?
• Rationale: The MCP SDK and stdio sibling surface support the object-or-omitted contract, but current shipped/raw HTTP behavior tolerated malformed values, so the merge depends on owner acceptance of the compatibility tradeoff.
• Likely owner: steipete — steipete is the live assignee and has the strongest recent Gateway/MCP history signal for this boundary.
• Options:
  • Accept stricter MCP contract (recommended): Merge the PR as-is, treating primitive, null, and array arguments as invalid while preserving omitted and object-shaped calls.
  • Preserve malformed-client tolerance: Ask for a rework that keeps accepting or coercing malformed arguments, at the cost of keeping raw HTTP loopback looser than the SDK schema.
  • Pause for broader validation policy: Hold this PR until maintainers define wider MCP HTTP request validation beyond tools/call.arguments.

Security
Cleared: Cleared: the diff adds input validation and tests only, with no dependency, workflow, secret, permission, package, or supply-chain changes.

Best possible solution:

Land the narrow handler validation and regression coverage once a Gateway/MCP owner explicitly accepts the stricter malformed-client boundary.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main casts params.arguments and passes it to the hook/tool path, while the PR exercises the raw loopback HTTP path for malformed, omitted, and object argument cases. I did not run tests because this review was required to keep the checkout read-only.

Is this the best way to solve the issue?

Yes, conditional on maintainer acceptance of the compatibility break: validating at the raw HTTP MCP handler is narrower than changing stdio handling or adding broad per-tool schema validation for this boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9fb24032215a.

Label justifications:

• P2: This is a focused Gateway/MCP bug fix with limited blast radius to malformed loopback tool-call payloads.
• merge-risk: 🚨 compatibility: Merging intentionally changes existing malformed-client behavior from hook/tool execution to JSON-RPC invalid-params rejection.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Sufficient: the PR body and comments provide terminal/Crabbox validation for the raw loopback HTTP behavior, and the latest head keeps the same production handler while changing only tests.
• proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body and comments provide terminal/Crabbox validation for the raw loopback HTTP behavior, and the latest head keeps the same production handler while changing only tests.

PR surface:

Source +5, Tests +67. Total +72 across 2 files.

What I checked:

• Current main behavior: Current main still casts methodParams?.arguments ?? {} to Record<string, unknown> before hooks and tool execution, so the central issue is not already fixed on main. (src/gateway/mcp-http.handlers.ts:102, 9fb24032215a)
• PR handler behavior: At PR head, present non-record arguments values return JSON-RPC invalid params before toolArgs, hooks, or tool execution are reached. (src/gateway/mcp-http.handlers.ts:103, 705e63af0e79)
• Regression coverage: The PR head tests rejection for null, array, and string arguments, keeps omitted arguments as {}, and verifies object arguments reach the hook and executor unchanged through the loopback tool-call helper. (src/gateway/mcp-http.test.ts:1155, 705e63af0e79)
• HTTP entry point: The /mcp HTTP loopback parses raw JSON-RPC and calls handleMcpJsonRpc, making the handler the narrow boundary for rejecting malformed tools/call params before side effects. (src/gateway/mcp-http.ts:269, 9fb24032215a)
• Sibling stdio surface: The stdio MCP server registers the SDK CallToolRequestSchema before invoking shared MCP handlers, so the raw HTTP loopback is the sibling surface missing this request-shape validation. (src/mcp/tools-stdio-server.ts:17, 9fb24032215a)
• Dependency contract: The pinned MCP TypeScript SDK v1.29.0 defines CallToolRequestParamsSchema.arguments as an optional record, matching this PR's object-or-omitted contract. (modelcontextprotocol/typescript-sdk/src/types.ts:1489)

Likely related people:

• steipete: Live PR state assigns this review to steipete, and history shows recent changes to MCP loopback handler, transport helper, and audit/tool-call correlation paths near this boundary. (role: recent Gateway/MCP area contributor and assignee; confidence: high; commits: f53346944db0, 25b069a6f3cd, 3d23103081f4; files: src/gateway/mcp-http.handlers.ts, src/gateway/mcp-http.ts, src/gateway/mcp-http.test.ts)
• vincentkoc: Blame on the current tools/call block and GitHub history show recent MCP loopback schema hardening, JSON-RPC failure classification, and message-delivery evidence work touching this area. (role: recent Gateway/MCP runtime contributor; confidence: high; commits: 07a765522a, 705bdcec7050, 172c3f60640e; files: src/gateway/mcp-http.handlers.ts, src/gateway/mcp-http.ts, src/gateway/mcp-http.test.ts)
• anagnorisis2peripeteia: Recent commits added and hardened the Streamable HTTP MCP loopback transport and attach launcher paths that consume this request boundary. (role: adjacent MCP HTTP transport contributor; confidence: medium; commits: 6df7db9f9e49, 224ea76d29d0, 7f69fe009a78; files: src/gateway/mcp-http.ts, src/gateway/mcp-http.test.ts)
• mmaps: History ties MCP loopback owner policy and before-tool-call hook enforcement to this contributor, which matters because this PR rejects malformed arguments before that hook path. (role: adjacent hook-boundary contributor; confidence: low; commits: 8b76392e3e79; files: src/gateway/mcp-http.handlers.ts, src/gateway/mcp-http.test.ts)

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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.

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

• reviewed 2026-07-02T18:48:29.878Z sha 26f7f71ab02116440a81f323f24b6f4ac19c03ab :: needs maintainer review before merge. :: none
• reviewed 2026-07-07T05:42:46.211Z sha ac96494b35a2839e24a1d729c1b8f938105472f3 :: needs maintainer review before merge. :: none
• reviewed 2026-07-07T06:03:49.897Z sha ea83601af06425933d24d3f262c084526c0eab9a :: needs maintainer review before merge. :: none
• reviewed 2026-07-07T06:10:54.369Z sha 8073af61a82a9c1415c9e37310d7699f32e01dda :: needs maintainer review before merge. :: none
• reviewed 2026-07-07T06:18:34.435Z sha 8073af61a82a9c1415c9e37310d7699f32e01dda :: needs maintainer review before merge. :: none
• reviewed 2026-07-07T06:52:17.951Z sha 0097d85b3e82c71fc5dc5017afb764e96712bd4c :: needs maintainer review before merge. :: none
• reviewed 2026-07-07T09:30:20.951Z sha 265200505ca11987a370942de9e61807724cdf50 :: needs maintainer review before merge. :: none

Updated the PR body to remove the leading BOM and added the requested loopback behavior proof: malformed tools/call.params.arguments now returns JSON-RPC -32602, and the focused proof asserts no before-tool hook call and no tool execution.

The Real behavior proof check is now passing.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Strengthened the proof without expanding the production scope.

The handler change is unchanged and remains limited to the Gateway HTTP tools/call boundary. I added test-only coverage for the full present-but-non-object matrix (bad, 42, true, null, []) and a positive compatibility case showing omitted arguments still reaches the hook/tool path as {}.

Focused validation:

$env:CI='true'; pnpm exec vitest run --config test/vitest/vitest.gateway.config.ts src/gateway/mcp-http.test.ts
Test Files  3 passed (3)
Tests       183 passed (183)

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Strengthened the PR proof without changing code: added the SDK argument-contract rationale, sibling stdio-surface comparison, accepted/rejected argument matrix, and current proof/check status. The PR body is now UTF-8 without BOM.\n\n@clawsweeper re-review

Added a small test-only proof update on top of the latest PR head.

What changed:

• Added a positive regression test showing object-shaped tools/call.params.arguments reaches both runBeforeToolCallHook and the tool executor unchanged.
• Production code is unchanged from the existing boundary fix.
• Re-ran focused validation after rebasing onto the updated PR branch:

$env:CI='true'
pnpm exec vitest run --config test/vitest/vitest.gateway.config.ts src/gateway/mcp-http.test.ts
Test Files  3 passed (3)
Tests       186 passed (186)

This completes the proof triangle: present non-object values are rejected before side effects, omitted arguments remains {}, and valid object arguments still pass through unchanged.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper re-review

All checks are green on the latest pushed head (8073af61a82a9c1415c9e37310d7699f32e01dda). The branch was synced with upstream/main; the current BEHIND state is from newer unrelated upstream churn in QA/GitHub Copilot files and does not touch this MCP gateway change.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Land-ready maintainer verification:

• Rebuilt the branch as one current-main commit while preserving @VectorPeak credit.
• Directly checked pinned @modelcontextprotocol/sdk 1.29.0: tools/call.params.arguments is an optional object record; the sibling stdio server already enforces CallToolRequestSchema.
• Current-main baseline reproduced the malformed-value path in sanitized AWS Crabbox run run_2be60da5f513.
• Focused sanitized AWS run run_6273eafe9b17 passed 272/272 Gateway MCP HTTP tests before the final test-only mock typing correction.
• Exact final head 705e63af0e793e46db0467031e1f405db93568a9 passed GitHub Actions workflow https://github.com/openclaw/openclaw/actions/runs/28858393759, including check-test-types and all selected test shards.
• A repeat sanitized AWS run (run_1fa1a1d425db) was infrastructure-blocked: the requested beast lease fell back to t3.small, then dependency installation was OOM-killed. No product test ran or failed in that attempt.
• git diff --check passed; final structured autoreview was clean at 0.92 confidence.
• OPENCLAW_TESTBOX=1 scripts/pr prepare-run 99180 passed the exact-head hosted CI/Testbox gate.

No unresolved review threads or known proof gaps remain.

Merged via squash.

• Prepared head SHA: 705e63af0e793e46db0467031e1f405db93568a9
• Landed commit: cd5c3fc3b779d803faf4099771c7b2dbaca5f324