Labels: bug, mcp
Description
Two defects in mcp_server.py, both in tool signatures — which matters more than usual
here because the docstring is the tool description the model sees:
# mcp_server.py:134
return SERVER.tool(name=fn.__name__, description=fn.__doc__ or "")(wrapper)
1. browser_switch_tab promises a feature the helper does not have
# mcp_server.py:214-217
@_tool
def browser_switch_tab(target: str):
"""Switch to tab by `targetId` or URL substring. Returns the sessionId."""
return {"sessionId": switch_tab(target)}
switch_tab() does no URL matching. It passes target to _target_id(), which returns
a non-dict straight through:
# src/browser_harness/helpers.py:320-322
def _target_id(target):
"""Accept a raw target id or a tab dict returned by the helpers."""
return (target.get("targetId") or target.get("target_id")) if isinstance(target, dict) else target
…and then straight into Target.attachToTarget(targetId=...). So
browser_switch_tab(target="github.com") sends "github.com" as a target id and fails
with a CDP error, which _tool swallows into {"error": "..."}.
The model has no other signal than that docstring — this is the tool description in the
MCP schema — so it will reasonably try the URL form, get an opaque error, and have no
way to learn the real contract. docs/MCP.md does not correct it either.
Fix: either drop the claim, or implement it (a few lines against list_tabs(),
and arguably worth having in the helper itself since SKILL.md already tells agents to
"inspect current_tab() and list_tabs() and use switch_tab() to reuse a matching
tab"):
def browser_switch_tab(target: str):
"""Switch to a tab by targetId, or by a substring of its URL."""
if not any(t["targetId"] == target for t in list_tabs()):
match = next((t for t in list_tabs() if target in t["url"]), None)
if match is None:
return {"error": f"no tab matching {target!r}"}
target = match["targetId"]
return {"sessionId": switch_tab(target)}
2. browser_cdp uses a mutable default argument
# mcp_server.py:261-264
@_tool
def browser_cdp(method: str, params: dict = {}):
"""Call a raw Chrome DevTools Protocol method. `params` are passed as kwargs."""
return cdp(method, **(params or {}))
params: dict = {} is a shared mutable default evaluated once at import. Nothing
mutates it today, so it is latent rather than live — but this is the one tool that
accepts arbitrary caller-supplied dicts, it survives for the whole life of a long-running
stdio server, and the or {} on the next line shows the author already wanted the
None semantics. params: dict | None = None costs nothing and removes the trap.
Environment
mcp_server.py @ main e3e8069, mcp>=2.0.0,<3
Labels: bug, mcp
Description
Two defects in
mcp_server.py, both in tool signatures — which matters more than usualhere because the docstring is the tool description the model sees:
1.
browser_switch_tabpromises a feature the helper does not haveswitch_tab()does no URL matching. It passestargetto_target_id(), which returnsa non-dict straight through:
…and then straight into
Target.attachToTarget(targetId=...). Sobrowser_switch_tab(target="github.com")sends"github.com"as a target id and failswith a CDP error, which
_toolswallows into{"error": "..."}.The model has no other signal than that docstring — this is the tool description in the
MCP schema — so it will reasonably try the URL form, get an opaque error, and have no
way to learn the real contract.
docs/MCP.mddoes not correct it either.Fix: either drop the claim, or implement it (a few lines against
list_tabs(),and arguably worth having in the helper itself since
SKILL.mdalready tells agents to"inspect
current_tab()andlist_tabs()and useswitch_tab()to reuse a matchingtab"):
2.
browser_cdpuses a mutable default argumentparams: dict = {}is a shared mutable default evaluated once at import. Nothingmutates it today, so it is latent rather than live — but this is the one tool that
accepts arbitrary caller-supplied dicts, it survives for the whole life of a long-running
stdio server, and the
or {}on the next line shows the author already wanted theNonesemantics.params: dict | None = Nonecosts nothing and removes the trap.Environment
mcp_server.py@maine3e8069,mcp>=2.0.0,<3