Skip to content

Return JSON-RPC error for filtered tool calls - #5944

Merged
JAORMX merged 1 commit into
mainfrom
tool-filter-jsonrpc-error-5764
Jul 23, 2026
Merged

Return JSON-RPC error for filtered tool calls#5944
JAORMX merged 1 commit into
mainfrom
tool-filter-jsonrpc-error-5764

Conversation

@JAORMX

@JAORMX JAORMX commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why: When thv run --tools-filter blocked a tools/call for a filtered tool, the middleware returned a bare HTTP 400 with no body. Clients expect a protocol-level JSON-RPC error, and a 400 on a validly received JSON-RPC request is off-spec — under MCP streamable HTTP an application-level failure rides back in an HTTP 200 carrying a JSON-RPC error object (the spec's "Error Handling" section lists "Unknown tools" as a protocol error with a canonical -32602 example).

What:

  • A filtered tools/call now returns a JSON-RPC error {"code":-32602,"message":"tool not found"} over HTTP 200 when the client's Accept header allows JSON (application/json, application/*, */*, or absent), echoing the request id.
  • The message is generic ("tool not found") by design: a filtered tool must be indistinguishable from a nonexistent one (the filter's long-standing documented intent — see the NOTE in NewToolCallMappingMiddleware). -32602 matches what the MCP spec and the go-sdk return for an unknown tool, so the two responses look identical.
  • A client that accepts only text/event-stream (the legacy HTTP+SSE transport, whose real response is delivered on a separate stream this middleware can't inject into) keeps the HTTP 400 fallback — preserving the documented session-management limitation.
  • The malformed-request case (toolCallBogus) intentionally stays HTTP 400, per this item's "keep 400 only for malformed" guidance.
  • New helpers clientAcceptsJSON and writeFilteredToolCallError, modeled on the existing writeRateLimited / classificationErrorBody patterns.
  • Dropped the dead, now-inaccurate expectedStatus column from the middleware scenario table (it was never asserted, and its filtered-case values were falsified by this change); the new httptest-based tests assert status directly.

Closes item 2 of #5764. Item 3 (version-agnostic proxying docs + strict mode) remains, so this does not close the issue.

Type of change

  • Bug fix

Test plan

  • Unit tests — go test -race ./pkg/mcp/... passes. New/updated coverage:
    • TestNewToolCallMappingMiddleware_FilteredTool (httptest): filtered call with Accept: application/json (numeric, string, and null ids), a multi-value Accept, and no Accept header → HTTP 200 + application/json + JSON-RPC -32602 "tool not found", id echoed, message does not leak "filter", and next never called; Accept: text/event-stream only → HTTP 400.
    • TestClientAcceptsJSON: application/json, multi-value, */*, application/*, empty, ;q= param, uppercase (case-insensitive), and the application/json-patch+json false-positive guard.
    • _AllowedToolPassesThrough (passthrough) and _BogusRequest (still 400 even with Accept: application/json).
  • Linting — task lint reports 0 issues; task build succeeds.

Does this introduce a user-facing change?

Yes. A blocked tools/call through thv run --tools-filter now returns a JSON-RPC -32602 "tool not found" error at HTTP 200 (for JSON-capable clients) instead of a bare HTTP 400, matching how a real MCP server reports an unknown tool.

Special notes for reviewers

  • Reviewed by an MCP-spec + correctness/tests + security panel. Spec: confirmed HTTP 200 + -32602 is the prescribed shape and that -32602 (not -32601 or isError:true) is correct for an unknown tool. Security: fail-closed confirmed (the call is never forwarded in either branch), the echoed id is safely json.Marshal-ed (no reflection/injection), and the generic message meets the no-oracle goal.
  • Observability note: telemetry/audit will now see HTTP 200 (with a JSON-RPC error) for a blocked filtered call instead of 400 — a dashboard that counted 4xx as "blocked" would need updating. This is a monitoring nuance, not a control change; the tool is still hard-blocked.
  • Deliberately out of scope: the toolCallBogus (malformed request) path stays 400 per this item's wording; the spec technically buckets malformed requests as protocol errors too, so upgrading it to -32602 is a reasonable future follow-up.

Generated with Claude Code

When the tool-call filtering middleware (thv run --tools-filter) blocked a
tools/call for a filtered tool, it returned a bare HTTP 400 with no body.
Clients expect a protocol-level JSON-RPC error, and a 400 on a validly
received JSON-RPC request is off-spec: under MCP streamable HTTP an
application-level failure rides back in an HTTP 200 carrying a JSON-RPC
error object.

Return a JSON-RPC error (code -32602, message "tool not found") over
HTTP 200 when the client's Accept header allows JSON. The generic message
deliberately matches what a client sees for a tool that does not exist, so
a filtered tool is not distinguishable from a nonexistent one (the filter's
long-standing documented intent). The MCP spec's own error-handling example
uses -32602 for unknown tools, and go-sdk returns the same, so a filtered
tool now looks identical to the real unknown-tool response.

A client that accepts only text/event-stream (the legacy HTTP+SSE
transport, whose real response is delivered on a separate stream this
middleware does not control) keeps the HTTP 400 fallback; the malformed-
request case likewise stays 400. Also drop the dead, now-inaccurate
expectedStatus column from the middleware scenario table (the new
httptest-based tests assert status directly).

Closes item 2 of #5764.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label Jul 23, 2026
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.77%. Comparing base (1d3e866) to head (5df7270).

Files with missing lines Patch % Lines
pkg/mcp/tool_filter.go 93.54% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5944   +/-   ##
=======================================
  Coverage   71.76%   71.77%           
=======================================
  Files         705      705           
  Lines       72159    72190   +31     
=======================================
+ Hits        51786    51811   +25     
- Misses      16675    16676    +1     
- Partials     3698     3703    +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX
JAORMX merged commit 2da1fcb into main Jul 23, 2026
83 of 85 checks passed
@JAORMX
JAORMX deleted the tool-filter-jsonrpc-error-5764 branch July 23, 2026 17:23
JAORMX added a commit that referenced this pull request Jul 24, 2026
TestStandaloneSSE_ListChangedRefiltersThroughExistingMiddleware asserted
that a tools/call for a filtered tool returns HTTP 400. That is no longer
true: #5944 changed the tool-call filter to return a JSON-RPC error over
HTTP 200 (a filtered tool is now indistinguishable from a nonexistent
one). This test was added by #5934 and the two PRs landed close together,
so each was green in isolation but the combination left the Tests check
red on main (a semantic merge collision) — deterministically failing every
subsequent PR's required unit-test job.

Update step (3) to assert the current behavior: HTTP 200 with a JSON-RPC
error body ("tool not found"). The tool is still blocked (never forwarded);
only the response envelope changed.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants