Skip to content

MCP tool failures are recorded as successes #7

Description

@arjunvlama

langchain-mcp-adapters defaults to handle_tool_errors=True, which turns a
failing tool into a returned error string rather than an exception:

RETURNED (did not raise): [{'type': 'text',
  'text': 'Error executing tool always_fails: this tool is broken on purpose'}]

BoundFlow's governed wrapper counts a failure only when the call raises. So every
MCP tool failure is currently recorded as a success:

  • tool_failure_counts stays empty for the tools we actually dispatch
  • max_tool_failures never trips, so a broken integration burns the whole budget
    instead of tripping its own breaker
  • on_failure: fail never fires — declared in config, does nothing
  • the tool_failures lifecycle metric is blind to declared tools, which is the
    one place it was supposed to be trustworthy

Same shape as reading isError under its 1.x name, which this code has already
been bitten by once: a failing tool that looks successful, invisible to every
fake, only findable against a real server.

Why the obvious fix is worse

handle_tool_errors=False makes the tool raise, which our wrapper would count.
Measured: the exception then propagates out of the entire invocation —
"workflow callback raised; recording as a failed run" — so one failing tool kills
the run rather than being reported to the model, and the agent loses the chance
to work around it. Strictly worse than under-counting.

The actual fix

Classify the returned error in the governed wrapper, the way a policy denial
already is (_is_refusal in harness_callbacks.py). The adapter's shape is
"Error executing tool {name}: ...", so the wrapper can record a failure and
hand the error to the model — count it and let the agent adapt, rather than
choosing between them.

Same tradeoff as the denial classifier, and acceptable for the same reason: if a
reword stops matching we go back to under-counting, which is today's behaviour.
It can only ever over-report.

Belongs in the wrapper rather than Charter's MCP client, since it's about how
BoundFlow decides what a failure is.

Where it's pinned

tests/e2e/test_failures.py::test_a_broken_tool_fails_the_task_naming_the_tool
is xfail(strict=True), so it fails loudly the day this is fixed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions