mcptool: CallToolResult.IsError is never checked — a refused tool call returns err == nil and the agent believes it succeeded

[lifeofzero]

Summary

mcptool never inspects CallToolResult.IsError. A tool call that the MCP server refuses comes back to the agent as err == nil, so the agent concludes it succeeded.

This is a correctness bug in the data path, not an observability nit: an agent can be told "you do not have permission to do that" and go on to report that it did it.

Where

tool/mcptool/mcp.go:391

func (w *mcpWrapper) Call(ctx context.Context, args string) (any, error) {
	result, err := w.session.CallTool(ctx, &mcp.CallToolParams{...})
	if err != nil {
		return nil, fmt.Errorf("MCP tool call failed: %w", err)   // TRANSPORT errors only
	}
	contents := mcpCallToolResultToAgentContent(result)
	if len(contents) == 0 {
		return nil, nil
	}
	return contents, nil   // <-- result.IsError is never read
}

The MCP specification carries tool execution failure in the result (CallToolResult.isError), deliberately separate from protocol/transport errors, so that the model can see and react to the failure. mcptool checks only the transport error, so isError: true is silently downgraded to success.

How I hit it

An agent with an MCP toolkit whose OAuth client had read-only scope called a write tool. The server refused:

{"content":[{"type":"text","text":"{\"error\":\"insufficient_scope\",\"message\":\"Operation put_page requires 'write' scope\",\"your_scopes\":[\"read\"]}"}],"isError":true}

ft.Call(...) returned err = nil. The agent reported that it had published. Nothing was written. The failure was invisible to every layer above the wrapper.

Minimal reproduction against any MCP server that can refuse a call:

all, _ := mcptool.ListTools(ctx, sess)
ft := all[i].(tool.FuncTool)
res, err := ft.Call(ctx, `{...}`)   // server responds isError: true
fmt.Println(err)                     // <nil>   <-- the bug
fmt.Println(res)                     // [{"content":[...],"isError":true}]

Suggested fix

Surface the in-band failure. Two reasonable shapes:

1. Return an error when result.IsError is true, wrapping the text content as the message. Simple, and matches what every caller already assumes err == nil means.
2. Keep returning the content (so the model can see the failure text and self-correct, which is the point of isError being in-band) but also return a sentinel error so programmatic callers, middleware and tracing can tell success from failure.

I lean towards (2) with a typed error (e.g. ToolCallError{Content: ...}), because it preserves the model-facing behaviour the MCP spec intends while making the failure legible to code. But (1) is strictly better than today, and I am happy to implement whichever you prefer.

Happy to open a PR.

Why it matters beyond one agent

Anything that reasons about tool outcomes inherits the lie: retry logic sees success and does not retry; approval gates see success and report a write that never happened; OpenTelemetry spans (#issue for tool spans filed separately) come out green. A refused call that reports success is the worst possible failure mode, because it launders the failure into evidence of success.

[lifeofzero]

Correction to my own issue. I was wrong about the framing, and the maintainers should not act on the version above.

I filed this as "IsError is never checked", implying an oversight. It is not. TestCallPreservesMCPErrorResult in tool/mcptool/mcp_test.go explicitly asserts the current behaviour:

result, err := funcTool.Call(ctx, `{}`)   // server returned IsError: true
if err != nil {
    t.Fatalf("Call() error = %v", err)     // asserts err == nil
}

and goes on to verify that IsError is preserved and reachable:

rawResult := text.Header().RawRepresentation.(*mcp.CallToolResult)
if !rawResult.IsError { t.Fatal(...) }

So the design is deliberate: the failed result is passed through to the model as content, which is exactly what MCP's in-band isError is FOR -- the model should see the failure text and self-correct. That is the right call and I should have read the tests before filing. Apologies for the noise.

---

There is still a real gap, but it is narrower and different, and it is about CODE, not the model.

Today the only way for a caller to learn that a tool call failed is:

contents := result.([]message.Content)
raw, ok := contents[0].Header().RawRepresentation.(*mcp.CallToolResult)
failed := ok && raw.IsError

That requires knowing to reach through Header().RawRepresentation and type-assert a provider type. Nothing in the framework does it, so everything above the wrapper treats a refused call as a successful one:

• agent/harness/toolautocall records functionInvocationStatusRanToCompletion.
• Retry / approval / policy layers see success.
• OpenTelemetry spans come out green (I hit this: a put_page refused for insufficient_scope produced a green span, and the agent reported that it had published. Nothing was written).

Narrower ask: an exported, ergonomic way for code to ask "did this tool call fail?", without reaching into RawRepresentation. For example:

// ToolCallError reports whether an MCP tool result carries the protocol's in-band
// failure flag, and returns the raw result if so.
func ToolCallError(result any) (*mcp.CallToolResult, bool)

Non-breaking, keeps the model-facing behaviour exactly as designed and tested, and lets middleware, tracing and retry logic tell a refusal from a success.

If that seems reasonable I will open a PR for it. If you would rather toolautocall itself distinguish the two (so the agent loop knows), that is a bigger design call and I would rather you steer it than guess.