[juliusmarminge]

Cause: Effect's MCP 2025-06-18 adapter (offered first by McpHttpServer.layerMcpTransport and used as the fallback for unknown versions) answers a tool's invalid arguments with a JSON-RPC -32602 error. Only 2025-11-25 and newer turn invalid input into an isError tool result. Cursor reduces the protocol error to "Tool execution error", so models never learned what to fix. Fix: toolkitRegistration now wraps McpServer.registerToolkit with withReadableInvalidArguments, a wrapper around McpServer.addTool in the same style as withAdvertisedSchemas and withToolSpans. For tool calls, registerToolkit raises InvalidParams only when a tool's arguments fail validation, and the wrapper returns it as CallToolResult{isError:true, content:[text: validation message]}. That works on every negotiated protocol version, matches the 2025-11-25 spec, and matches what the hand-registered image tools (preview snapshot, device screenshot) already did. It sits inside the span and analytics wrappers, so those see the isError result and record it as a failure. The repro test from factory/repro-35713 passes unchanged. Five existing tests that asserted the old protocol failure (unknown-argument and below-minimum cases in McpHttpServer.test.ts, and the unknown-organize-target case in toolkits/core.test.ts) now assert the isError result. No production code calls the in-process server directly. No protocol versions, defaults or stdio bridge behaviour changed. No new regex or parser.

Fixes #35713

Reproduction on main

Commit e0d6aad (origin/main). Started T3CODE_DEV_SEED=0 vp run dev in a fresh worktree, enabled a cursor provider instance in the worktree's own .t3 settings.json, added a scratch project with t3 project add, then ran t3 thread start --provider cursor --model composer-2.5 with a prompt telling the model to call orchestrator_capabilities and then delegate_task once with target.options set to the JSON string "{"reasoningEffort":"high"}" (gemini-4-carbon itself was refused by the VM's Cursor account for usage). Observed: the Cursor SDK's tool-call-completed event for delegate_task carried result {"status":"error","error":{"message":"Tool execution error"}}, matching the reporter's payload, and T3's projected turn item for the call records only "Tool execution error". composer-2.5's reply did quote the full detail ("Tool execution error. MCP error -32602: Invalid parameters for tool 'delegate_task': Expected array | object | null at ["target"]["options"]"), so whether the model sees the detail depends on Cursor's backend and model; what Gemini receives was not verified. Server side: McpHttpServer.layerMcpTransport offers [v2025_06_18, v2026_07_28], with 2025-06-18 first and used as the fallback for unknown versions, and under 2025-06-18 Effect's McpServer answers invalid arguments with a JSON-RPC -32602 error instead of an isError tool result. Added a failing HTTP-transport test in apps/server/src/mcp/McpHttpServer.test.ts that negotiates 2025-06-18 and calls a tool with a string where an object is required; on main it receives {code:-32602, message:"Invalid parameters for tool 'probe': Expected object | null ..."} instead of an isError result.

Evidence

Being captured: before and after in the real app. This section is filled in before the pull request can merge.

Tests

vp test run src/mcp/McpHttpServer.test.ts (before the test updates: repro test passed, 4 old-behaviour tests failed as expected); vp test run src/mcp/ -> 26 files, 365 tests passed; vp run check:changed -> format, lint, knip, graphql, typecheck (3 packages), tests (3 files) all pass

Not in this change

• Offering MCP 2025-11-25 (or making it the fallback) in layerMcpTransport, and the 2025-06-18 pin in AcpMcpStdioBridge.ts: not needed for this fix and would change protocol negotiation for every client
• The upstream Effect behaviour where the 2025-06-18 adapter maps InvalidToolInput to a JSON-RPC error rather than an isError result
• Cursor SDK reducing JSON-RPC errors to a generic 'Tool execution error' in what T3 records for the call

Who did the work

• Written by Claude Opus 5.5, in Claude Code
• Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
• Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5

Run by the t3code factory in T3 Code.

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	21.5 KiB	21.5 KiB	0 B (0.0%)	24.4 KiB	✅
Codex	Live turn messages	1	1	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.9 KiB	21.9 KiB	0 B (0.0%)	24.4 KiB	✅
Claude	Live turn messages	1	1	0 (0.0%)	8	✅

Other scenarios

All 64 scenario measurements match main.

All 64 scenario measurements

Provider	Scenario	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	short turn resume	WebSocket decoded	10.5 KiB	10.5 KiB	0 B (0.0%)	12.7 KiB	✅
Codex	short turn resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Codex	long turn running, cold open	HTTP decoded	281.7 KiB	281.7 KiB	0 B (0.0%)	302.7 KiB	✅
Codex	long turn running, cold open	rows	75	75	0 (0.0%)	75	✅
Codex	long turn running, history page	HTTP decoded	92.4 KiB	92.4 KiB	0 B (0.0%)	107.4 KiB	✅
Codex	long turn running, history page	rows	75	75	0 (0.0%)	75	✅
Codex	long turn finished, cold open	HTTP decoded	381.5 KiB	381.5 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	long turn finished, cold open	rows	151	151	0 (0.0%)	151	✅
Codex	long turn finished, socket cold open	WebSocket decoded	381.6 KiB	381.6 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	long turn finished, socket cold open	WebSocket messages	2	2	0 (0.0%)	2	✅
Codex	long turn finished, socket cold open	rows	151	151	0 (0.0%)	151	✅
Codex	long turn finished, work page	HTTP decoded	186.2 KiB	186.2 KiB	0 B (0.0%)	200.2 KiB	✅
Codex	long turn finished, work page	rows	150	150	0 (0.0%)	150	✅
Codex	idle thread resume	WebSocket decoded	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	idle thread resume	WebSocket messages	1	1	0 (0.0%)	1	✅
Codex	fleet shell snapshot	HTTP decoded	42.4 KiB	42.4 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	fleet shell snapshot	rows	27	27	0 (0.0%)	27	✅
Codex	fleet shell subscribe	WebSocket decoded	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	fleet shell subscribe	WebSocket messages	1	1	0 (0.0%)	1	✅
Codex	fleet shell resume check	HTTP decoded	18 B	18 B	0 B (0.0%)	100 B	✅
Codex	fleet shell resume check	rows	0	0	0	0	✅
Codex	stale shell reload	HTTP decoded	42.4 KiB	42.4 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	stale shell reload	rows	27	27	0 (0.0%)	27	✅
Codex	stale shell resume	WebSocket decoded	140 B	140 B	0 B (0.0%)	400 B	✅
Codex	stale shell resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Codex	stale todo list cold open	HTTP decoded	72.4 KiB	72.4 KiB	0 B (0.0%)	83.0 KiB	✅
Codex	stale todo list cold open	rows	75	75	0 (0.0%)	75	✅
Codex	stale todo list cold open	nodes	2	2	0 (0.0%)	4	✅
Codex	stale todo list cold open	subagents	2	2	0 (0.0%)	4	✅
Codex	stale todo list cold open	plans	1	1	0 (0.0%)	2	✅
Codex	stale todo list cold open	providerTurns	1	1	0 (0.0%)	2	✅
Codex	stale todo list cold open	checkpoints	1	1	0 (0.0%)	2	✅
Claude	short turn resume	WebSocket decoded	10.6 KiB	10.6 KiB	0 B (0.0%)	12.7 KiB	✅
Claude	short turn resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Claude	long turn running, cold open	HTTP decoded	283.3 KiB	283.3 KiB	0 B (0.0%)	302.7 KiB	✅
Claude	long turn running, cold open	rows	75	75	0 (0.0%)	75	✅
Claude	long turn running, history page	HTTP decoded	93.4 KiB	93.4 KiB	0 B (0.0%)	107.4 KiB	✅
Claude	long turn running, history page	rows	75	75	0 (0.0%)	75	✅
Claude	long turn finished, cold open	HTTP decoded	384.2 KiB	384.2 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	long turn finished, cold open	rows	151	151	0 (0.0%)	151	✅
Claude	long turn finished, socket cold open	WebSocket decoded	384.3 KiB	384.3 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	long turn finished, socket cold open	WebSocket messages	2	2	0 (0.0%)	2	✅
Claude	long turn finished, socket cold open	rows	151	151	0 (0.0%)	151	✅
Claude	long turn finished, work page	HTTP decoded	188.1 KiB	188.1 KiB	0 B (0.0%)	200.2 KiB	✅
Claude	long turn finished, work page	rows	150	150	0 (0.0%)	150	✅
Claude	idle thread resume	WebSocket decoded	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	idle thread resume	WebSocket messages	1	1	0 (0.0%)	1	✅
Claude	fleet shell snapshot	HTTP decoded	43.0 KiB	43.0 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	fleet shell snapshot	rows	27	27	0 (0.0%)	27	✅
Claude	fleet shell subscribe	WebSocket decoded	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	fleet shell subscribe	WebSocket messages	1	1	0 (0.0%)	1	✅
Claude	fleet shell resume check	HTTP decoded	18 B	18 B	0 B (0.0%)	100 B	✅
Claude	fleet shell resume check	rows	0	0	0	0	✅
Claude	stale shell reload	HTTP decoded	43.0 KiB	43.0 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	stale shell reload	rows	27	27	0 (0.0%)	27	✅
Claude	stale shell resume	WebSocket decoded	140 B	140 B	0 B (0.0%)	400 B	✅
Claude	stale shell resume	WebSocket messages	2	2	0 (0.0%)	2	✅
Claude	stale todo list cold open	HTTP decoded	73.4 KiB	73.4 KiB	0 B (0.0%)	83.0 KiB	✅
Claude	stale todo list cold open	rows	75	75	0 (0.0%)	75	✅
Claude	stale todo list cold open	nodes	2	2	0 (0.0%)	4	✅
Claude	stale todo list cold open	subagents	2	2	0 (0.0%)	4	✅
Claude	stale todo list cold open	plans	1	1	0 (0.0%)	2	✅
Claude	stale todo list cold open	providerTurns	1	1	0 (0.0%)	2	✅
Claude	stale todo list cold open	checkpoints	1	1	0 (0.0%)	2	✅

Baseline: f313d0a · PR result: 018a653 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 108.5 KiB
• Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[macroscopeapp]

Approvability

Verdict: Approved at 018a653

Macroscope's review found this PR approvable — Invalid tool arguments now return readable validation details as an error-marked tool result, allowing agents to correct their calls. The change is limited to error presentation and leaves successful tool execution and access controls unchanged.

Notes:

• All code in this push has already been reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The server converts invalid tool argument failures into MCP error tool results with readable messages. Tests check returned errors and verify that rejected calls are not recorded or dispatched.

Changes: Readable MCP validation errors

Layer / File(s)	Summary
Convert invalid parameters to tool results apps/server/src/mcp/McpHttpServer.ts	A wrapper converts InvalidParams failures from registered tool handlers into error tool results containing the error message. Toolkit registration applies the wrapper.
Verify readable validation results apps/server/src/mcp/McpHttpServer.test.ts, apps/server/src/mcp/toolkits/core.test.ts	Tests check readable error results for invalid arguments. They also verify that rejected tool calls are not recorded or dispatched.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant MCP client
  participant withReadableInvalidArguments
  participant registered tool handler
  MCP client->>withReadableInvalidArguments: Invoke tool with arguments
  withReadableInvalidArguments->>registered tool handler: Forward arguments
  registered tool handler-->>withReadableInvalidArguments: Return InvalidParams failure
  withReadableInvalidArguments-->>MCP client: Return error result with validation message

Loading

Merge Risk: Minimal · up to 018a6

Invalid tool arguments now come back as readable tool errors instead of protocol errors, so agents can see why a call was rejected and correct it. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: agents can now read why a T3 tool rejected their arguments. The changeset wraps toolkit registration so invalid arguments return an isError tool result with the valida…
Description check		The description follows the template's main sections. It gives a clear cause, a change summary, and a reproduction on main. It links the issue (#35713) for scope and lists tests run. Two template item…
Linked Issues check		Directly linked issue 35713 asks that the model see why delegate_task rejected target.options. At head 018a653, McpHttpServer.ts adds withReadableInvalidArguments. It wraps McpServer.addTool, catches …
Out of Scope Changes check		The changed code in McpHttpServer.ts serves issue 35713. withReadableInvalidArguments is the fix. Its placement inside withToolSpans and withToolAnalytics lets those wrappers record the isError result…

Full details: Title check

Explanation

The title names the main change: agents can now read why a T3 tool rejected their arguments. The changeset wraps toolkit registration so invalid arguments return an isError tool result with the validation message. The title uses the conventional commit form and is specific, so it passes.

Full details: Description check

Explanation

The description follows the template's main sections. It gives a clear cause, a change summary, and a reproduction on main. It links the issue (#35713) for scope and lists tests run. Two template items are weak. The Evidence section says before/after capture is still pending, and the Scope section does not state a bug-fix rationale beyond the linked issue. The Verification section lists focused and repo checks with observed results, which meets the template's bar.

Resolution

Complete the Evidence section with the before and after capture in the real app, or state what could not be captured. The PR objectives also note that the base commit was not confirmed as reproducing the problem by automated capture, so the reproduction claim should be stated with that caveat.

Full details: Linked Issues check

Explanation

Directly linked issue 35713 asks that the model see why delegate_task rejected target.options. At head 018a653, McpHttpServer.ts adds withReadableInvalidArguments. It wraps McpServer.addTool, catches InvalidParams from each tool handler, and returns an isError CallToolResult whose text is the validation message. toolkitRegistration applies it around McpServer.registerToolkit, so delegate_task and other toolkit tools return the message as a readable tool result. This matches the first expected outcome in issue 35713. The wrapper does not depend on protocol version, so it covers the 2025-06-18 path from the issue. Test edits are described in the raw summary but were not read. The incremental diff from previous head f997adc was too large to inspect, so changes since the previous review are confirmed only for McpHttpServer.ts. Whether Cursor shows the message to the model is outside this PR.

Full details: Out of Scope Changes check

Explanation

The changed code in McpHttpServer.ts serves issue 35713. withReadableInvalidArguments is the fix. Its placement inside withToolSpans and withToolAnalytics lets those wrappers record the isError result as a failure. The raw summary lists three changed files: McpHttpServer.ts, McpHttpServer.test.ts, and toolkits/core.test.ts. The test edits update existing tests that asserted the old JSON-RPC failure, so they support the behavior change. The PR does not change protocol versions, the layerMcpTransport fallback, or AcpMcpStdioBridge, and it adds no parser. The test hunks were not read, and changes since the previous review were not fully inspected.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 3
• Charged: $0.75
• View usage details

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: factory/issue-35713
• File: .coderabbit.config.ts
• Commit: 018a653
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: a9b4cd47-d15e-485b-96d6-5103bfe8417e

Commits

Reviewing files that changed from the base of the PR and between f997adc and 018a653.

Files selected for processing: 3

• apps/server/src/mcp/McpHttpServer.test.ts
• apps/server/src/mcp/McpHttpServer.ts
• apps/server/src/mcp/toolkits/core.test.ts

Limit details: You’ve used all 10 included reviews currently available.

[juliusmarminge]

The automated pipeline stopped on this pull request and will not merge it:

the capture step could not show the problem on the base commit, so there is nothing to prove fixed.

On head, installed dependencies, started the dev server, enabled Cursor, added a synthetic scratch project, and started gemini-4-carbon. The prompt requested orchestrator_capabilities, an intentionally invalid delegate_task call with string target.options, quoting the received error, then retrying with an object preserving reasoningEffort high. Cursor refused the model before any tool call. Base was not run; neither verdict is established. Stopped the server and removed the scratch directory and worktree.

It stays open for a person to finish or close.

Dismissing prior approval to re-evaluate 018a653