Default MCP tab tools to the active tab
Requiring an explicit tab_id forced every navigate or close through a preceding tabs_list call, which costs an MCP client a full round trip just to learn the ID the browser already considers current. navigate and tabs_close now resolve the active tab when tab_id is omitted, matching the screenshot tool. Resolution is explicit rather than forwarding None into the SDK, so the acting tool knows which tab it touched; tabs_close reports it, since closing the wrong tab is not recoverable. This stays in the MCP layer: the SDK and CLI signatures are unchanged.
This commit is contained in:
@@ -16,6 +16,7 @@ from urllib.parse import urlsplit
|
||||
from browser_cli import BrowserCLI
|
||||
from browser_cli.mcp.naming import resolve_tool_prefix, tool_name
|
||||
from browser_cli.mcp.serialization import structured
|
||||
from browser_cli.mcp.targets import resolve_tab_id
|
||||
|
||||
ClientFactory = Callable[..., BrowserCLI]
|
||||
|
||||
@@ -93,27 +94,29 @@ def create_server(*, client_factory: ClientFactory = BrowserCLI, tool_prefix: st
|
||||
|
||||
@mcp.tool(name=tool_name("tabs_close", prefix))
|
||||
def tabs_close(
|
||||
tab_id: int,
|
||||
tab_id: int | None = None,
|
||||
browser: str | None = None,
|
||||
remote: str | None = None,
|
||||
key: str | None = None,
|
||||
) -> dict[str, int]:
|
||||
"""Close one tab by its current ID. This changes the user's real browser."""
|
||||
closed = _client(client_factory, browser, remote, key).tabs.close(tab_id)
|
||||
return {"closed": closed}
|
||||
"""Close a tab, defaulting to the active tab. This changes the real browser."""
|
||||
client = _client(client_factory, browser, remote, key)
|
||||
target = resolve_tab_id(client, tab_id)
|
||||
return {"closed": client.tabs.close(target), "tab_id": target}
|
||||
|
||||
@mcp.tool(name=tool_name("navigate", prefix))
|
||||
def navigate(
|
||||
tab_id: int,
|
||||
url: str,
|
||||
tab_id: int | None = None,
|
||||
browser: str | None = None,
|
||||
remote: str | None = None,
|
||||
key: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Navigate an existing tab to a URL and return a fresh tab snapshot."""
|
||||
"""Navigate a tab to a URL, defaulting to the active tab, and return it."""
|
||||
client = _client(client_factory, browser, remote, key)
|
||||
client.nav.to(tab_id, url)
|
||||
return structured(client.tabs.status(tab_id))
|
||||
target = resolve_tab_id(client, tab_id)
|
||||
client.nav.to(target, url)
|
||||
return structured(client.tabs.status(target))
|
||||
|
||||
@mcp.tool(name=tool_name("page_info", prefix))
|
||||
def page_info(
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
"""Tab targeting for the MCP surface.
|
||||
|
||||
MCP callers pay a full round trip for every extra tool call, so tools that act
|
||||
on a tab accept an optional ``tab_id`` and fall back to the browser's current
|
||||
active tab. Resolution happens here rather than by forwarding ``None`` into the
|
||||
SDK, so the acting tool always knows which tab it touched and can report it.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from browser_cli import BrowserCLI
|
||||
|
||||
def resolve_tab_id(client: BrowserCLI, tab_id: int | None) -> int:
|
||||
"""Return *tab_id*, or the ID of the currently active tab when it is ``None``."""
|
||||
if tab_id is not None:
|
||||
return tab_id
|
||||
return client.tabs.active().id
|
||||
@@ -26,6 +26,7 @@ from browser_cli.framing import frame
|
||||
# hand back one the server has just timed out and closed.
|
||||
_MAX_IDLE_SECONDS = max(5, REMOTE_SESSION_IDLE_TIMEOUT - 5)
|
||||
_MAX_PER_ENDPOINT = 8
|
||||
_MAX_ENDPOINTS = 64
|
||||
|
||||
class PooledConnection:
|
||||
__slots__ = ("sock", "secret", "last_used")
|
||||
@@ -56,10 +57,32 @@ def checkout(endpoint: str) -> PooledConnection | None:
|
||||
_close(conn.sock) # too old — assume the server has dropped it
|
||||
return None
|
||||
|
||||
def _prune_endpoints_locked(now: float) -> None:
|
||||
"""Keep the number of endpoint buckets bounded for long-running SDK users."""
|
||||
for endpoint, bucket in list(_POOL.items()):
|
||||
fresh = [conn for conn in bucket if now - conn.last_used <= _MAX_IDLE_SECONDS]
|
||||
if fresh:
|
||||
_POOL[endpoint] = fresh
|
||||
else:
|
||||
for conn in bucket:
|
||||
_close(conn.sock)
|
||||
_POOL.pop(endpoint, None)
|
||||
|
||||
while len(_POOL) >= _MAX_ENDPOINTS:
|
||||
oldest_endpoint, bucket = min(
|
||||
_POOL.items(),
|
||||
key=lambda item: min(conn.last_used for conn in item[1]) if item[1] else 0.0,
|
||||
)
|
||||
for conn in bucket:
|
||||
_close(conn.sock)
|
||||
_POOL.pop(oldest_endpoint, None)
|
||||
|
||||
def checkin(endpoint: str, conn: PooledConnection) -> None:
|
||||
"""Return a still-healthy connection to the pool for reuse."""
|
||||
conn.last_used = time.monotonic()
|
||||
with _LOCK:
|
||||
if endpoint not in _POOL and len(_POOL) >= _MAX_ENDPOINTS:
|
||||
_prune_endpoints_locked(conn.last_used)
|
||||
bucket = _POOL.setdefault(endpoint, [])
|
||||
if len(bucket) >= _MAX_PER_ENDPOINT:
|
||||
_close(conn.sock)
|
||||
|
||||
@@ -70,19 +70,48 @@ class RateLimiter:
|
||||
``rate`` is the sustained refill in tokens/second; ``burst`` is the bucket
|
||||
capacity (defaults to ``rate``). ``rate <= 0`` disables limiting entirely.
|
||||
Thread-safe so it can be shared across all connections of one serve process.
|
||||
|
||||
The bucket table is capped. Without that bound, a long-running public server
|
||||
could retain one entry per ever-seen identity/IP forever; GC cannot reclaim
|
||||
those entries because the limiter still references them.
|
||||
"""
|
||||
|
||||
def __init__(self, rate: float, burst: float | None = None) -> None:
|
||||
def __init__(self, rate: float, burst: float | None = None, max_buckets: int = 4096) -> None:
|
||||
self.rate = float(rate)
|
||||
self.capacity = float(burst) if burst is not None else max(float(rate), 1.0)
|
||||
self.max_buckets = max(1, int(max_buckets))
|
||||
self._buckets: dict[str, tuple[float, float]] = {}
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def _prune_locked(self, now: float) -> None:
|
||||
"""Drop idle/full buckets, then oldest buckets, until the table is bounded."""
|
||||
if len(self._buckets) < self.max_buckets or self.rate <= 0:
|
||||
return
|
||||
|
||||
# Once a bucket has fully refilled, keeping it around carries no useful
|
||||
# throttling state. Use at least 60s so normal active identities are not
|
||||
# churned out aggressively on high-rate configs.
|
||||
idle_seconds = max(60.0, (self.capacity / self.rate) * 2)
|
||||
full_epsilon = 1e-9
|
||||
for bucket_key, (tokens, last) in list(self._buckets.items()):
|
||||
refilled = min(self.capacity, tokens + (now - last) * self.rate)
|
||||
if refilled >= self.capacity - full_epsilon and now - last >= idle_seconds:
|
||||
self._buckets.pop(bucket_key, None)
|
||||
|
||||
# If an attacker keeps creating fresh identities faster than they go idle,
|
||||
# still keep memory bounded. Evict the oldest identity state; that may reset
|
||||
# throttling for that identity, but bounded memory is more important here.
|
||||
while len(self._buckets) >= self.max_buckets:
|
||||
oldest_key = min(self._buckets, key=lambda k: self._buckets[k][1])
|
||||
self._buckets.pop(oldest_key, None)
|
||||
|
||||
def allow(self, key: str) -> bool:
|
||||
if self.rate <= 0:
|
||||
return True
|
||||
now = time.monotonic()
|
||||
with self._lock:
|
||||
if key not in self._buckets and len(self._buckets) >= self.max_buckets:
|
||||
self._prune_locked(now)
|
||||
tokens, last = self._buckets.get(key, (self.capacity, now))
|
||||
tokens = min(self.capacity, tokens + (now - last) * self.rate)
|
||||
if tokens < 1.0:
|
||||
|
||||
Reference in New Issue
Block a user