feat(mcp): migrate MCP integration to mcp 2.x - #5334
Conversation
Bump the MCP Python SDK from the hard pin mcp==1.26.0 to mcp>=2.0.0,<3 and adapt to the 2.0 API surface: - The lowlevel @server.list_tools()/call_tool()/list_resources()/ list_prompts() decorators were removed; register handlers through the Server(...) constructor (on_list_tools=, on_call_tool=, ...) instead. - Handlers now take (ctx: ServerRequestContext, params) and return result models (ListToolsResult, CallToolResult, ListResourcesResult, ListPromptsResult); call_tool reads params.name/params.arguments and flags failures with is_error. - Read/construct snake_case model fields (input_schema, mime_type) instead of the removed camelCase aliases. Client-side (client.py, controller.py) only needed the input_schema attribute rename; ClientSession usage is unchanged in 2.0. Refs browser-use#5333
browser_use/config.py imports pydantic_settings at runtime, but it was only listed in the dev group. It previously reached the runtime environment transitively because mcp 1.x depends on pydantic-settings>=2.5.2; mcp 2.0 no longer does, so a plain `pip install .` no longer provides it and importing browser_use fails with ModuleNotFoundError: No module named 'pydantic_settings'. Promote it to the main dependencies with the same exact pin already used in the dev group (2.12.0, compatible with pydantic==2.12.5). Refs browser-use#5333
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- Import ServerRequestContext from the public mcp.server package (it is re-exported there) rather than the private mcp.server.lowlevel.server module. - Annotate the call_tool content lists and _execute_tool's return as list[types.ContentBlock] to match CallToolResult.content's declared type (list is invariant, so list[TextContent | ImageContent] is not assignable). Refs browser-use#5333
|
Hi @Hardik180704 — thanks for offering to collaborate on #5333. I've gone ahead and implemented the migration here (#5334) since I already had the full compatibility audit from the issue. Happy to have your eyes on the PR if you'd like to review or suggest test coverage improvements. On the direction question you raised: this PR moves to mcp 2.x exclusively (drops the hard 1.26 pin), since maintaining a dual 1.x/2.x compatibility shim would mean keeping the removed lowlevel decorators and camelCase aliases alive behind version checks. If a maintainer would prefer dual support instead, I'm glad to rework it. |
Hardik180704
left a comment
There was a problem hiding this comment.
I tested this at 5ec48f23e0f0af413cf927863c4b4e152bd216e7 in a clean mcp==2.0.0 environment. The constructor-based registration and snake_case schema migration work, and the existing MCP allowed-domain regressions pass (3/3). However, the new handler boundary still needs changes before this is safe to merge.
Directly invoking the registered tools/call handlers reproduced these results:
BrowserUseServer unknown False Unknown tool: missing
CLIMCPServer exec raised RuntimeError boom
CLIMCPServer screenshot raised RuntimeError boom
The first result comes from BrowserUseServer._execute_tool() returning an Unknown tool: string, which handle_call_tool() wraps as a successful CallToolResult. The CLI execution path has two distinct problems: _execute() normally catches BaseException and returns traceback text as successful output, while failures before/around that boundary and _screenshot() failures escape the callback instead of becoming CallToolResult(is_error=True). This also means the PR description's statement that exception, unknown-tool, and validation failures are all flagged is not currently true.
Please add focused regression coverage through each server's registered request handlers—not only through _execute_tool() helpers:
BrowserUseServer:tools/listreturns all tools withinput_schema;resources/listandprompts/listreturn typed empty results; successful text/image calls preserve payloads andmime_type; unknown tools and execution exceptions returnis_error=True.CLIMCPServer:tools/listexposes both schemas; valid exec and screenshot calls succeed; invalid arguments, unknown tools, execution/daemon failures, and screenshot/file failures all returnis_error=Truewithout escaping the handler.- For both implementations, assert the handlers are actually registered under
tools/listandtools/callso a future SDK API change cannot silently leave a server that constructs but exposes no tools.
Once those failure paths and regressions are in place, the migration looks directionally sound. The remaining CLA check is administrative rather than a code issue.
The 2.x handler migration left three failure paths reporting success: - BrowserUseServer.handle_call_tool wrapped the plain-string "Unknown tool: ..." fallthrough from _execute_tool as a successful CallToolResult. Unknown tool names are now short-circuited against the advertised tool set and returned with is_error=True. - CLIMCPServer.handle_call_tool returned _execute()'s captured traceback text as a successful result; it is now flagged is_error=True when the output contains a traceback. - _screenshot() and other failures around the exec boundary escaped the handler entirely; handle_call_tool now catches unexpected exceptions and returns them as CallToolResult(is_error=True). _execute_tool keeps its existing behavior so direct callers are unaffected. Refs browser-use#5333
Drive the registered MCP request handlers (not just the internal helpers) for both BrowserUseServer and CLIMCPServer: handler registration under tools/list and tools/call, typed list results with populated input_schema, typed empty resources/prompts lists, success payloads preserved with is_error falsy, and unknown-tool / raising / traceback / screenshot-failure paths all returning is_error=True without escaping the handler. Refs browser-use#5333
|
Thanks for the thorough review, @Hardik180704 — you were right on all three counts. Fixed in the two commits just pushed: Failure paths now flag
The PR description's claim about error flagging is now actually true. Handler-level regression coverage (
Verified locally in a clean |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/ci/security/test_mcp_handler_contract.py">
<violation number="1" location="tests/ci/security/test_mcp_handler_contract.py:21">
P3: These regression tests reach into private mcp SDK internals (`Server._request_handlers`, and `get_request_handler(...).handler`). Those attributes are not part of the public mcp 2.x API, so a minor SDK bump can silently rename/restructure them, breaking collection or assertions and leaving the real handler boundary untested. Consider driving the handlers through a stable seam instead (e.g. an in-memory stdio client sending actual tools/list + tools/call messages) so the contract is asserted at the public boundary you're migrating to.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| from browser_use.mcp.server import BrowserUseServer | ||
|
|
||
|
|
||
| def _call_tool_handler(server: Any) -> Any: |
There was a problem hiding this comment.
P3: These regression tests reach into private mcp SDK internals (Server._request_handlers, and get_request_handler(...).handler). Those attributes are not part of the public mcp 2.x API, so a minor SDK bump can silently rename/restructure them, breaking collection or assertions and leaving the real handler boundary untested. Consider driving the handlers through a stable seam instead (e.g. an in-memory stdio client sending actual tools/list + tools/call messages) so the contract is asserted at the public boundary you're migrating to.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/ci/security/test_mcp_handler_contract.py, line 21:
<comment>These regression tests reach into private mcp SDK internals (`Server._request_handlers`, and `get_request_handler(...).handler`). Those attributes are not part of the public mcp 2.x API, so a minor SDK bump can silently rename/restructure them, breaking collection or assertions and leaving the real handler boundary untested. Consider driving the handlers through a stable seam instead (e.g. an in-memory stdio client sending actual tools/list + tools/call messages) so the contract is asserted at the public boundary you're migrating to.</comment>
<file context>
@@ -0,0 +1,188 @@
+from browser_use.mcp.server import BrowserUseServer
+
+
+def _call_tool_handler(server: Any) -> Any:
+ return server.server.get_request_handler('tools/call').handler
+
</file context>
Route the resources/list and prompts/list lookups through -> Any helpers like the existing tools/list and tools/call helpers, so get_request_handler's Optional return and the ctx=None test double no longer trip reportOptionalMemberAccess / reportArgumentType under pyright. Refs browser-use#5333
|
@cla-assistant check |
Hardik180704
left a comment
There was a problem hiding this comment.
Thanks for addressing the earlier handler-boundary findings. I re-reviewed the current head (3b2582e): unknown tools and raised execution/screenshot failures now return CallToolResult(is_error=True), both server constructors register the expected handlers, and the new tests cover most of those paths.
One correctness issue remains before this is safe to merge:
CLIMCPServer.handle_call_tool infers failure by searching normal captured output for "Traceback (most recent call last)". Since browser_exec executes arbitrary user code and returns whatever it prints, a successful call such as print("Traceback (most recent call last):") is misclassified as is_error=True. Conversely, error classification should not depend on traceback formatting remaining unchanged. Please make _execute communicate failure structurally (for example, return output plus an error flag/result, or let an appropriate exception cross into the handler) and add a regression proving traceback-like user output remains successful while a real exception is an MCP error.
Please also finish the successful image-path coverage requested in the prior review: drive browser_screenshot through the registered CLI handler and assert the base64 payload and mime_type == "image/png" are preserved. The BrowserUseServer content-list path should likewise have a focused text/image preservation assertion if practical. Current tests cover screenshot failure but not successful image serialization.
The use of private SDK handler access in the tests is somewhat coupled to MCP internals, but it directly guards the constructor-registration migration; I do not consider that alone blocking. The output-based error heuristic is blocking.
CLIMCPServer.handle_call_tool previously inferred browser_exec failure by searching captured output for 'Traceback (most recent call last)'. User code that merely prints that string was misclassified as an error, and the classification depended on traceback formatting. _execute now returns (output, is_error) so failure is communicated structurally. Also add regression coverage: - traceback-like printed output stays a successful result - a real exec exception returns is_error=True - browser_screenshot success preserves base64 payload + image/png mime_type - BrowserUseServer content-list path preserves text + image blocks
# Conflicts: # browser_use/mcp/server.py
|
Thanks for the careful re-review, @Hardik180704 — both blocking items are now addressed in the head just pushed ( 1. Structural error signaling (the blocking heuristic).
2. Successful image-path coverage. All 20 handler-contract tests pass locally under |
Type the tools/list handler accessor helper to return Any (matching the handler-contract test helpers) so pyright does not flag attribute access on the Optional handler entry.
Migrates the MCP integration from the hard-pinned
mcp==1.26.0tomcp>=2.0.0,<3.Closes #5333
What changed
pyproject.toml—mcp==1.26.0→mcp>=2.0.0,<3browser_use/mcp/server.py&cli_mcp.py— the lowlevel@server.list_tools()/@server.call_tool()/@server.list_resources()/@server.list_prompts()decorators were removed in mcp 2.0, so handlers are nowregistered through the
Server(...)constructor (on_list_tools=,on_call_tool=, …). Handler signatures change to(ctx: ServerRequestContext, params)returning the result models(
ListToolsResult,CallToolResult,ListResourcesResult,ListPromptsResult);call_toolreadsparams.name/params.argumentsand flags failures withis_error=True.inputSchema=→input_schema=,mimeType=→mime_type=.browser_use/mcp/client.py&controller.py— only the camelCase →snake_case attribute reads changed (
tool.inputSchema→tool.input_schema).ClientSessionusage (list_tools(),call_tool()) is unchanged in 2.0.The stdio bootstrap (
stdio_server(),InitializationOptions,get_capabilities(...),server.run(read, write, init_opts)) is unchanged —those APIs are intact in 2.0.
Testing
Validated against an environment with
mcp==2.0.0installed:import browser_use.mcp.{server, cli_mcp, client, controller}— all import cleanly.BrowserUseServer()constructs and registerstools/list,tools/call,resources/list,prompts/listin_request_handlers.CLIMCPServer()constructs and registerstools/list,tools/call.tools/listreturns all 16 tools withinput_schemapopulated; unknown tool returnsis_error=True;resources/list/prompts/listreturn empty.Notes for reviewers
Behavior change worth a look: in 1.x the
call_toolerror path returned a barecontent list with no error flag; the 2.0
CallToolResultnow setsis_error=Trueon exception / unknown-tool / validation-failure paths so MCPclients surface them as tool errors. Success paths are unchanged.
The dependency-floor bumps in mcp 2.0 (
pydantic>=2.12,anyio>=4.9,typing-extensions>=4.13, plus newopentelemetry-api/mcp-types) aresatisfied by the existing constraints.
Summary by cubic
Migrates MCP integration to
mcp>=2.0.0,<3using 2.xServerhandlers and snake_case models; errors now returnCallToolResult(is_error=True), and CLI exec failures are flagged structurally to avoid false positives.Refactors
Server(... on_list_tools, on_call_tool, on_list_resources, on_list_prompts); handlers take(ctx, params)and return typed results.inputSchema→input_schema,mimeType→mime_type,ToolAnnotations.readOnlyHint→read_only_hint; clients readinput_schema; adoptServerRequestContextandlist[types.ContentBlock].is_error=True; CLI_executenow returns(output, is_error)so real exceptions setis_error=Trueand printed “traceback” text doesn’t.ruff.Dependencies
mcpto>=2.0.0,<3.pydantic-settings==2.12.0to runtime deps.Written for commit 02a514d. Summary will update on new commits.