fix(mcp): surface CallToolResult.isError as a failed ActionResult (#5235)

## Symptom

`MCPClient.mcp_action_wrapper()` calls `self.session.call_tool(...)`,
which returns an MCP-spec `CallToolResult` — a `CallToolResult` includes
an `isError: bool` field that signals an *application-level* tool
failure (e.g. a `read_file` tool returning `"File not found"`) even
though the RPC call itself succeeded. That field was never checked.
`_format_mcp_result()` unconditionally stringifies `result.content` and
the wrapper returns it as a success-shaped `ActionResult` with no
`error` field set — silently hiding tool-reported failures from the
agent (both the param-model and no-param wrapper variants have this
bug).

## Fix

Check `result.isError` right after formatting the content, at both call
sites, and route to the same `ActionResult(error=..., success=False)`
pattern this file already uses elsewhere (connection failures, RPC
exceptions) instead of always treating `content` as a successful result.

## Verification

- Added `tests/ci/test_mcp_client_error_result.py`:
- `test_mcp_tool_isError_true_is_surfaced_as_action_result_error` —
confirmed **red** against the pre-fix code (`ActionResult(success=None,
error=None, extracted_content='File not found: ...')` — looked like a
success), now **green**.
- `test_mcp_tool_isError_false_still_succeeds` — sanity check that
normal successful calls are unaffected.
- `uv run pytest tests/ci/test_mcp_client_error_result.py
tests/ci/security/test_mcp_allowed_domains.py
tests/ci/test_action_timeout.py tests/ci/test_rerun_ai_summary.py` — 121
passed, no regressions.
- `ruff check` + `ruff format --check` + `pyright` (basic mode) clean on
both changed files.

## AI disclosure

Bug found and fix implemented with Claude Code assistance; verified
(red→green tests, lint/type-check, diff review) by a human before
submission.

<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Surfaced MCP tool application-level errors by mapping
`CallToolResult.isError` to a failed `ActionResult`, so agents no longer
treat tool-reported failures as successes. Successful calls are
unchanged.

- **Bug Fixes**
- Check `result.isError` in both `mcp_action_wrapper` variants and
return `ActionResult(error=..., success=False)`.
  - Format error messages with tool name plus extracted content.
- Added tests for `isError=True` and `isError=False` paths, including
parameterized tool errors.

<sup>Written for commit 28fe857d9f.
Summary will update on new commits.</sup>

<a
href="https://cubic.dev/pr/browser-use/browser-use/pull/5235?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>

<!-- End of auto-generated description by cubic. -->

browser_use/mcp/client.py

@@ -326,6 +326,10 @@ class MCPClient:
 					# Convert MCP result to ActionResult
 					extracted_content = self._format_mcp_result(result)
 
+					if getattr(result, 'isError', False):
+						error_msg = f"MCP tool '{tool.name}' reported an error: {extracted_content}"
+						return ActionResult(error=error_msg, success=False)
+
 					return ActionResult(
 						extracted_content=extracted_content,
 						long_term_memory=f"Used MCP tool '{tool.name}' from {self.server_name}",
@@ -370,6 +374,10 @@ class MCPClient:
 					# Convert MCP result to ActionResult
 					extracted_content = self._format_mcp_result(result)
 
+					if getattr(result, 'isError', False):
+						error_msg = f"MCP tool '{tool.name}' reported an error: {extracted_content}"
+						return ActionResult(error=error_msg, success=False)
+
 					return ActionResult(
 						extracted_content=extracted_content,
 						long_term_memory=f"Used MCP tool '{tool.name}' from {self.server_name}",

tests/ci/test_mcp_client_error_result.py

@@ -0,0 +1,97 @@
+"""Regression test for MCP tool calls that fail at the application level.
+
+Per the MCP spec, `CallToolResult.isError` signals that the tool itself
+reported a failure (e.g. "File not found") even though the RPC call
+succeeded. `MCPClient` must surface that as a failed `ActionResult`
+instead of silently treating `result.content` as a successful result.
+"""
+
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+from mcp import types
+
+from browser_use import Tools
+from browser_use.mcp.client import MCPClient
+
+
+def _make_connected_client() -> MCPClient:
+	client = MCPClient(server_name='test-server', command='test-command')
+	client._telemetry = MagicMock()
+	client._connected = True
+	client.session = MagicMock()
+	return client
+
+
+@pytest.mark.asyncio
+async def test_mcp_tool_isError_true_is_surfaced_as_action_result_error():
+	"""An MCP tool that reports isError=True must not look like a success."""
+	client = _make_connected_client()
+	client.session.call_tool = AsyncMock(  # type: ignore[union-attr]
+		return_value=types.CallToolResult(
+			content=[types.TextContent(type='text', text='File not found: /tmp/does-not-exist.txt')],
+			isError=True,
+		)
+	)
+
+	tools = Tools()
+	tool = types.Tool(name='read_file', description='Read a file', inputSchema={'type': 'object', 'properties': {}})
+	client._register_tool_as_action(tools.registry, 'read_file', tool)
+
+	result = await tools.registry.execute_action('read_file', {})
+
+	assert result.success is False, f'expected success=False for a failed MCP tool call, got {result!r}'
+	assert result.error, f'expected result.error to be set for a failed MCP tool call, got {result!r}'
+	assert 'File not found' in result.error
+
+
+@pytest.mark.asyncio
+async def test_parameterized_mcp_tool_isError_true_is_surfaced_as_action_result_error():
+	"""The parameter-model wrapper must surface application-level MCP errors too."""
+	client = _make_connected_client()
+	client.session.call_tool = AsyncMock(  # type: ignore[union-attr]
+		return_value=types.CallToolResult(
+			content=[types.TextContent(type='text', text='File not found: /tmp/does-not-exist.txt')],
+			isError=True,
+		)
+	)
+
+	tools = Tools()
+	tool = types.Tool(
+		name='read_file',
+		description='Read a file',
+		inputSchema={
+			'type': 'object',
+			'properties': {'path': {'type': 'string'}},
+			'required': ['path'],
+		},
+	)
+	client._register_tool_as_action(tools.registry, 'read_file', tool)
+
+	result = await tools.registry.execute_action('read_file', {'path': '/tmp/does-not-exist.txt'})
+
+	assert result.success is False, f'expected success=False for a failed MCP tool call, got {result!r}'
+	assert result.error, f'expected result.error to be set for a failed MCP tool call, got {result!r}'
+	assert 'File not found' in result.error
+	client.session.call_tool.assert_awaited_once_with('read_file', {'path': '/tmp/does-not-exist.txt'})  # type: ignore[union-attr]
+
+
+@pytest.mark.asyncio
+async def test_mcp_tool_isError_false_still_succeeds():
+	"""Sanity check: a normal successful MCP tool call is unaffected by the fix."""
+	client = _make_connected_client()
+	client.session.call_tool = AsyncMock(  # type: ignore[union-attr]
+		return_value=types.CallToolResult(
+			content=[types.TextContent(type='text', text='ok')],
+			isError=False,
+		)
+	)
+
+	tools = Tools()
+	tool = types.Tool(name='read_file', description='Read a file', inputSchema={'type': 'object', 'properties': {}})
+	client._register_tool_as_action(tools.registry, 'read_file', tool)
+
+	result = await tools.registry.execute_action('read_file', {})
+
+	assert result.error is None
+	assert result.extracted_content == 'ok'