MCP: failed tool calls return isError: false, so denials look like successes

Drafted with AI assistance.

Bug description

Every failed MCP tool call returns a successful result with the error as plain text. isError is never set, so a client following the MCP spec reads a failure — including a permission denial — as a success.

Observed on 6.1.0 against a read-only service user whose role forbids the tool:

{"jsonrpc":"2.0","id":7,"result":{"content":[{"type":"text","text":
 "Error: Error calling tool 'save_sql_query': Permission denied: can_write on SavedQuery for user mcp-svc (tool: save_sql_query)"}],
 "isError":false}}

The message itself is good — it names the permission, view, user and tool. The problem is purely the flag: an agent that checks isError concludes the write succeeded, and may report it as done.

Root cause, and why this isn't a simple fix

StructuredContentStripperMiddleware.on_call_tool catches every exception and returns an ordinary ToolResult:

https://github.com/apache/superset/blob/master/superset/mcp_service/middleware.py#L767-L821

The catch is deliberate, and the comment explains why:

When exceptions propagate past the middleware chain to the MCP SDK layer, they become CallToolResult(isError=True). Some transports (Claude.ai's MCP bridge) cannot encode these error responses, producing "encoding without a string argument". Catch ALL exceptions (not just specific types) because any unhandled exception … will cause encoding failures on the wire.

So this is a workaround for a client-side encoding bug, traded against the protocol contract for every other client. That trade is invisible to callers and currently silent.

A fix has to keep the workaround working for that transport while restoring the contract for conforming clients. Possible directions, in rough order of preference:

Set the error flag while keeping the same encodable text payload — if the bridge's failure is specifically about the SDK's error object rather than a result carrying isError, this may satisfy both.
Gate the behaviour on transport or on a config flag, defaulting to spec-conformant.
Leave as-is but document it prominently, so integrators know isError is not usable against Superset.

A question I can't answer from outside: does the Claude.ai bridge encoding bug still reproduce on current fastmcp (3.4.7 here)? If it has since been fixed upstream, the workaround can simply be removed and this becomes a one-line change. Maintainers with access to that transport are better placed to check than I am.

How to reproduce

Give the MCP identity a role lacking can_write on SavedQuery.
Call save_sql_query with valid arguments (database_id, label, sql) so it reaches the RBAC check rather than failing validation.
Inspect the JSON-RPC result: the text describes a permission denial, and isError is false.

Any failing tool call shows it; a denial is just the clearest case, since the operation definitively did not happen.

Why it matters to us

We proxy an internal agent to a deliberately read-only Superset MCP. The proxy forwards caller tokens and does not pattern-match Superset's error strings to synthesise the flag, since that would couple us to another service's message wording. So a forbidden write currently returns a success-shaped response to an agent.

Not part of this report

Two things I checked that are already handled on master, noted so nobody re-investigates:

Error IDs are no longer collision-prone. 6.1.0 builds them as f"err_{int(time.time())}", so two failures in the same second share an ID; master uses mcp_call_id or f"err_{secrets.token_hex(8)}".
Denials are not mislabelled. They render with a specific permission message, as above. The generic Internal error … Error ID: … text appears for validation errors (e.g. a missing required field), which is a separate and much less serious cosmetic issue.

Superset version

master / latest-dev

Python version

3.11

Node version

Not applicable

Browser

Not applicable

@AurimasNav

@AurimasNav good analysis. I can confirm the root cause and add some context from the current master code.

What the current code does

In StructuredContentStripperMiddleware.on_call_tool, the except block catches all exceptions and returns a plain ToolResult with the error text as content — but never sets isError: true [1]. The comment explicitly explains the trade-off:

When exceptions propagate past the middleware chain to the MCP SDK layer, they become CallToolResult(isError=True). Some transports (Claude.ai's MCP bridge) cannot encode these error responses, producing "encoding without a string argument". Catch ALL exceptions…

The workaround is still present on master and isError is never set in this path.

On your three proposed directions

Option 1 (set isError while keeping the text payload) is the most tractable. The original StructuredContentStripperMiddleware was introduced in PR #38786 specifically to strip structured_content and catch encoding failures from the Claude.ai bridge. The bridge's problem was triggered by structuredContent dicts and unencoded exception objects — not by a ToolResult that happens to carry isError: true in its serialized JSON [2]. A ToolResult(content=[TextContent(...)], isError=True) serializes as a plain dict and should not trigger the bridge's encoding failure. That hypothesis is worth verifying against the bridge before merging, but it's the right starting point.

On the fastmcp encoding bug question

The master comment still says the workaround is needed — nobody has removed it, which suggests nobody has verified it's safe to drop against the current bridge version. Maintainers with access to the Claude.ai transport are better placed to test this.

No existing PR covers this

There's been a cluster of MCP middleware fixes recently — PR #39159 (success flag always true in logs), PR #41921 (observability + error ID collision), PR #40098 (RBAC tool visibility) — but none address the isError protocol conformance issue you've filed here.

Suggested path forward

If you want to send a fix, option 1 is lowest risk: change the return ToolResult(...) in the except block to return ToolResult(content=..., isError=True, meta=...) and run it against the Claude.ai bridge to confirm no encoding regression. If the bridge does break, that points toward a bridge-specific config flag (option 2). Either way, a test covering the isError value in the returned ToolResult from that except block would be a good addition alongside the existing middleware chain tests in test_middleware_logging.py [3].

To reply, just mention @dosu.

Docs are dead. Just use Dosu.

Drafted with AI assistance.

Thanks — I verified the suggestion against fastmcp 3.4.7 in a running 6.1.0 instance. It works, with one correction, and the evidence supports the hypothesis that the bridge is unaffected.

The suggested code as written raises TypeError. ToolResult takes is_error, not isError:

ToolResult.__init__(self, content=..., structured_content=..., meta=..., is_error: bool = False)
>>> ToolResult(content=[], isError=True)
TypeError: ToolResult.__init__() got an unexpected keyword argument 'isError'

With the right name it produces exactly the wire result we want:

ToolResult(content=[mt.TextContent(type="text", text="Error: denied")], is_error=True).to_mcp_result()
# CallToolResult(meta=None,
#                content=[TextContent(type='text', text='Error: denied', ...)],
#                structuredContent=None,
#                isError=True)

That output is also evidence for the bridge question. structuredContent stays None and the content list is plain TextContent — the only delta versus today's behaviour is the boolean. Since #38786 introduced this middleware to strip structuredContent/outputSchema and to stop unencoded exception objects reaching the SDK, and neither changes here, the encoding path the workaround protects is untouched. That is consistent with your read, though it still wants a real check against the bridge, which I can't run.

So the change is:

return ToolResult(
    content=[mt.TextContent(type="text", text=error_text)],
    is_error=True,
    meta={"mcp_call_id": mcp_call_id} if mcp_call_id else None,
)

Happy to send that with a test asserting is_error on the returned ToolResult from that except block, alongside the existing middleware tests. I can't validate against the Claude.ai bridge myself, so that verification would need to happen in review — if it does regress, option 2 (a transport or config gate) is the fallback.

@AurimasNav

@AurimasNav feel free to open a PR to address the issue you are seeing.

fix(mcp): flag failed tool calls with isError

Navigation Menu

Repository navigation

MCP: failed tool calls return isError: false, so denials look like successes #43358

Bug description

Root cause, and why this isn't a simple fix

How to reproduce

Why it matters to us

Not part of this report

Superset version

Python version

Node version

Browser

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

MCP: failed tool calls return isError: false, so denials look like successes #43358

Description

Bug description

Root cause, and why this isn't a simple fix

How to reproduce

Why it matters to us

Not part of this report

Superset version

Python version

Node version

Browser

Activity

dosubot commented on Aug 20, 2026

AurimasNav commented on Aug 20, 2026

sadpandajoe commented on Aug 20, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions