From fa29dc05d9c666e8a60f36cd6308670819bc394b Mon Sep 17 00:00:00 2001 From: moonrailgun Date: Sat, 19 Sep 2026 00:18:04 +0800 Subject: [PATCH 1/2] docs(mcp): document multi-client admission behavior --- docs/dao-mcp-server-design.md | 18 ++- docs/feature-checklist.md | 2 +- docs/features.md | 4 +- docs/mcp-server.md | 3 +- .../automation/dao_browser_tool_types.cc | 2 + .../automation/dao_browser_tool_types.h | 1 + .../mcp/dao_mcp_foundation_unittest.cc | 1 + src/dao/browser/mcp/dao_mcp_service.cc | 59 ++++++++ src/dao/browser/mcp/dao_mcp_service.h | 2 + .../mcp/dao_mcp_service_browsertest.cc | 127 ++++++++++++++++++ src/dao/browser/mcp/dao_mcp_transport.cc | 20 ++- src/dao/browser/mcp/dao_mcp_transport.h | 6 + .../mcp/helper/dao_mcp_stdio_server.cc | 16 ++- src/dao/browser/ui/dao_ui_sources.gni | 1 + 14 files changed, 245 insertions(+), 17 deletions(-) diff --git a/docs/dao-mcp-server-design.md b/docs/dao-mcp-server-design.md index 77acfbaf..03fdaa96 100644 --- a/docs/dao-mcp-server-design.md +++ b/docs/dao-mcp-server-design.md @@ -167,10 +167,12 @@ When an external lease is active, Dao Agent may continue non-browser work and chat, but browser tool calls fail with `AGENT_CONTROL_BUSY`. When Dao Agent is completing a browser tool call, external authorization waits -for that call to finish before acquiring the lease. The transport admits only -one external socket; a second external connection is closed before MCP -initialization. `LEASE_BUSY` applies when the admitted external client cannot -acquire the shared automation lease. +for that call to finish before acquiring the lease. The service keeps up to 32 +external clients admitted; when another completes `hello`, the least recently +active idle client (no pending approval or tool call) is closed and its leases +are released so the newcomer is admitted, and `TOO_MANY_CLIENTS` is returned +only when every admitted client is busy. `LEASE_BUSY` applies when the +admitted external client cannot acquire the shared automation lease. #### Browser Tool Executor @@ -438,6 +440,7 @@ request. Required stable errors include: - `MCP_DISABLED` +- `TOO_MANY_CLIENTS` - `AUTHORIZATION_DENIED` - `AUTHORIZATION_TIMEOUT` - `AGENT_CONTROL_BUSY` @@ -470,9 +473,10 @@ tool-result errors rather than falsely reporting success. - Dao Agent and external Agent cannot hold the lease concurrently. - An in-flight tool completes before a lease changes owner. -- A second external socket is rejected by the one-client transport admission - gate; an admitted external client blocked on the shared lease receives - `LEASE_BUSY`. +- Beyond 32 admitted external clients, the least recently active idle client + is evicted and its leases released; when none is idle the newcomer receives + `TOO_MANY_CLIENTS`. An admitted external client blocked on the shared lease + receives `LEASE_BUSY`. - Denial, approval timeout, revoke, disconnect, and toggle-off release state. - Browser, Profile, and tab destruction never cause last-active fallback. - User interaction remains available outside brief per-operation tab locks. diff --git a/docs/feature-checklist.md b/docs/feature-checklist.md index 5d348aa1..50e3ec9f 100644 --- a/docs/feature-checklist.md +++ b/docs/feature-checklist.md @@ -178,7 +178,7 @@ Flagship feature. C++ services + `dao://dao-agent` WebUI + vendor runtime. | ☐ | Scoped query, guarded click, and cursor-based network wait | `src/dao/.../automation/dao_{page,devtools}_tools.{h,cc}`, `dao_browser_automation_session.h`, `dao_agent_ui.{h,cc}`, `agent_bridge.ts`, `browser_tool_catalog.json` | 🟡 | Run `DaoMcpPageToolsBrowserTest.QueryElementsSupportsGuardedClick`, `DaoMcpPageToolsBrowserTest.QueryElementsRejectsStaleRefScope`, `DaoMcpDevToolsBrowserTest.WaitsForMatchingNetworkJsonAfterCursor`, `browser_tool_catalog.test.ts`, and `agent_bridge_call_native.test.ts`; verify a scoped exact semantic query returns one result plus document/snapshot/ref tokens, a ref scope accepts only its source document/snapshot tokens, a matching guarded click succeeds atomically, changed text or stale tokens fail without clicking, network tracking returns a cursor, and a later matching JSON response returns only selected paths plus the next cursor | | ☐ | Sequential Agent browser batches, guarded filling, real keys, and semantic waits | `automation/dao_page_tools.{h,cc}`, `resources/agent/{pi_tool_adapter,agent_bridge,dao_chat_view}.ts`, `browser_tool_catalog.json` | 🟡 | Run `DaoMcpPageToolsBrowserTest.FillByRef*`, `DaoMcpPageToolsBrowserTest.KeyChord*`, `DaoMcpPageToolsBrowserTest.WaitForElement*`, and focused adapter/catalog/bridge/chat WebUI tests; verify bundled runtime ordering and later-browser-action skips after execution, legacy error-only results, or argument-validation failures; stale/readonly/redirected-focus fill rejection; synthetic cancelable beforeinput handlers can reject both replacement and clearing with the old value retained and no input event; handler changes to focus, readonly/disabled state, input type, role preconditions, or target attachment fail closed; handler selection changes still allow complete replacement and clearing with trusted input events; Cmd+A matches cmd+a without implicit Shift, while explicit Shift and standalone uppercase letters remain intact; macOS Cmd+Left/Right moves to line boundaries, Cmd+Up/Down moves to document boundaries, Shift variants select, Cmd+Backspace deletes to the line start, and Cmd+Z/Shift+Z undo/redo; Enter submission and Tab focus movement; cancellation and timeout during a slow keydown handler still deliver keyup to the original target without affecting a newly active tab; exact-count waits including removal, timeout without invalidating refs, cancellation/navigation cleanup, and native bridge wait budgets beyond the default 15 seconds | | ☐ | Session-scoped Agent/MCP DevTools tools | `src/dao/.../automation/dao_devtools_tools.{h,cc}`, `dao_devtools_client.{h,cc}`, `dao_browser_automation_session.{h,cc}`, `dao_agent_ui.cc`, `agent_bridge.ts`, `browser_tool_catalog.json` | 🟡 | Run `DaoMcpDevToolsBrowserTest.*`, the Page/Tab MCP regression filters, `agent_bridge_call_native.test.ts`, `pi_tool_adapter.test.ts`, and `dao_chat_view.test.ts`; verify enable-window staging commits only after a matching generation/host success, cancellation/failure/rebinding drops pending staging, clear removes committed and pre-clear staged events while preserving the pending attempt and later events, monotonic same-binding domain confirmation across reordered success/failure, aggregate network/console byte budgets below entry caps, UTF-8-safe truncation, strict current-tree `(frame, URL)` size preflight before content fetch, an independent response-size backstop, exact in-budget base64, failed/oversized Script and Document search incompleteness, item/URL-byte/depth/dedup/source/4 MiB scan limits, re-entrant resolver/command destruction, exactly-once cancellation, and target/host/origin/document rebinding | -| ☐ | Default-off process-global local MCP server | `src/dao/.../mcp/dao_mcp_{service,transport,connection,protocol,runtime_files}.*`, `dao_pref_names.*`, `browser_prefs_mcp.cc.patch`, `chrome_browser_main_extra_parts_profiles.cc.patch` | 🔴 | Run `DaoMcpServiceBrowserTest.*`, `DaoMcpProtocolTest.*`, and `DaoMcpRuntimeFilesTest.*`; verify browser-IO-thread listener ownership, owner-only runtime permissions, nonce rotation, same-UID authentication, protocol/version/line limits, 64-request/8 MiB per-connection and bounded aggregate unconsumed-ingress credits, terminal-request logical closing before later same-batch tools, aggregate write backpressure, bounded graceful-close drains, 32-client admission, serialized approval prompts, concurrent different-tab control, same-tab exclusion, idle hello/catalog discovery beyond the approval timeout without a prompt or disconnect, exact last-active-window selection and approval on the first tool call, required first-call `reason` in every MCP tool schema, missing/blank/invalid/oversized reason rejection before approval, optional subsequent reasons stripped before execution, pre-approval catalog access even when connection begins on Dao Settings, re-entrant approval cancellation denial, approval plus lease ordering, cancellation, optional non-tab `tab_id` schema/routing, isolated concurrent tab contexts, default-target compatibility after MCP switch/open, unknown-target fail-closed behavior, and complete per-connection lease/runtime cleanup after disconnect, disable, and shutdown | +| ☐ | Default-off process-global local MCP server | `src/dao/.../mcp/dao_mcp_{service,transport,connection,protocol,runtime_files}.*`, `dao_pref_names.*`, `browser_prefs_mcp.cc.patch`, `chrome_browser_main_extra_parts_profiles.cc.patch` | 🔴 | Run `DaoMcpServiceBrowserTest.*`, `DaoMcpProtocolTest.*`, and `DaoMcpRuntimeFilesTest.*`; verify browser-IO-thread listener ownership, owner-only runtime permissions, nonce rotation, same-UID authentication, protocol/version/line limits, 64-request/8 MiB per-connection and bounded aggregate unconsumed-ingress credits, terminal-request logical closing before later same-batch tools, aggregate write backpressure, bounded graceful-close drains, 32-client admission with least-recently-active idle eviction that releases leases (`EvictsLeastRecentlyActiveIdleClientAtCapacity`, `EvictsIdleApprovedClientAndReleasesLease`) and `TOO_MANY_CLIENTS` rejection when every admitted client is busy (`RejectsHelloWithTooManyClientsWhenAllAreBusy`), serialized approval prompts, concurrent different-tab control, same-tab exclusion, idle hello/catalog discovery beyond the approval timeout without a prompt or disconnect, exact last-active-window selection and approval on the first tool call, required first-call `reason` in every MCP tool schema, missing/blank/invalid/oversized reason rejection before approval, optional subsequent reasons stripped before execution, pre-approval catalog access even when connection begins on Dao Settings, re-entrant approval cancellation denial, approval plus lease ordering, cancellation, optional non-tab `tab_id` schema/routing, isolated concurrent tab contexts, default-target compatibility after MCP switch/open, unknown-target fail-closed behavior, and complete per-connection lease/runtime cleanup after disconnect, disable, and shutdown | | ☐ | Settings MCP master switch, connection, usage, quick setup, and Stop | `src/dao/.../mcp/dao_mcp_settings_handler.{h,cc}`, `resources/settings/dao_page/dao_page.{html,ts}.patch`, `webui/settings/settings_ui.cc.patch` | 🟡 | Run `DaoMcpInstallCommandTest.*`, `DaoMcpSettingsHandlerTest.*`, `DaoMcpSettingsPageBrowserTest.*`, and `DaoPage`; verify one header/connection/usage/enabled-only-setup card, responsive selector/copy alignment, and text status updates through `dao-mcp-status-changed`. The switch must write process-global Local State rather than `prefs.dao`; profile-scoped usage lists total and per-tool calls, sorts by count, resets independently, counts successful and failed executor entries, and excludes validation, denial, unknown-tool, and pre-execution target failures. Client details and Stop appear only for an active authorized lease. Confirm setup is absent while disabled; when enabled it defaults to Codex and switches to user-scoped Claude Code or Generic MCP. CLI previews stay single-line, Generic MCP preview and clipboard are identical Chromium-native three-space pretty JSON, and malformed Generic JSON fails closed without changing the clipboard. Also verify option-specific feedback, POSIX-safe helper and current user-data-directory arguments, Debug/custom-profile endpoint binding, stale preview rejection, listener cleanup, and absence of the standalone configuration button. | | ☐ | Native MCP stdio helper and macOS app bundling | `src/dao/.../mcp/helper/`, `dao_mcp_helper_browsertest.cc`, `dao_version.gni`, `chrome/BUILD_mcp_helper.gn.patch` | 🔴 | Run `DaoMcpHelperBrowserTest.*`; verify all 33 tools survive catalog adaptation, MCP `2025-11-25` and Codex-compatible `2025-06-18` negotiation with initialized gating, the `codex/tool-catalog-cache.cacheable=false` compatibility capability, server-wide instructions that prefer Dao MCP, establish the initial target with `list_tabs`, preserve it across follow-ups, route ambiguous open/click/select requests through `query_elements` and guarded `click_by_ref`, and reserve `switch_tab` for explicit browser-tab navigation; verify adapted per-tool descriptions do not repeat tab-discovery guidance, plus string/numeric IDs, object/scalar/list `structuredContent`, real screenshot MIME, `isError` failures, cancellation with no late response, disabled-browser stderr determinism, JSON-only stdout, and executable copies at both the build output and `Dao.app/Contents/Helpers/dao-mcp` | | ☐ | Local MCP approval, controlled-tab indicator, Stop, and peer-agent busy UX | `dao_mcp_approval_dialog.{h,cc}`, `dao_mcp_control_banner_view.{h,cc}`, `dao_address_bar_view.{h,cc}`, `dao_mcp_service.{h,cc}`, `ui/webui/dao_sidebar_ui.{h,cc}`, `resources/sidebar/{dao_tab_item.ts,sidebar_bridge.ts}`, `dao_agent_ui.{h,cc}`, `pi_tool_adapter.ts` | 🔴 | Run `DaoMcpApprovalDialogTest.*`, `DaoMcpControlBannerTest.*`, `DaoMcpPeerLeaseTest.*`, `tab_item.test.ts`, `pi_tool_adapter.test.ts`, and `dao_chat_view.test.ts`; verify serialized localized prompts with reported client/version, browser-recorded localized request date/time with time zone, sanitized client-provided reason, window, and Profile rendering, preserved reason/time while queued, and bounded scrolling for long reasons, exact native Browser activation when a prompt arrives behind another application, the 60-second unanswered-request timeout, no default Allow action, deny/close/parent destruction exactly once and fail closed, address-bar robot visibility only for the active controlled tab in the authorized normal Browser, a sidebar robot for every controlled tab with the close action revealed on pointer hover or keyboard focus without layout shift, prompt target add/removal updates after switch/open, popup client/target/latest accepted tool/count details with live call updates, no process-ID row in approval dialogs or control popups, no extra page-content row, clickable per-connection Stop, lease release/disconnect transitions without disturbing other clients, chat continuity, different-tab parallelism, and same-tab pre-CDP `AGENT_CONTROL_BUSY` browser-tool failures | diff --git a/docs/features.md b/docs/features.md index c9cc02c4..2e06d844 100644 --- a/docs/features.md +++ b/docs/features.md @@ -225,9 +225,9 @@ The stack includes: **LLM tool calling**, **long-term memory** (SQLite + FTS5), ### 2.3 Local MCP Server -- **Process-global native service** (`mcp/dao_mcp_service.{h,cc}`) — A default-off Local State preference controls a single browser-process MCP endpoint for up to 32 authenticated local clients. Authentication and catalog discovery never start the approval timer, so idle clients can remain connected and continue listing tools. Each connection's first tool call snapshots the exact last-active eligible normal browser window and active tab, then requests native user approval before execution. Approval prompts are serialized. Each approved connection can retain multiple isolated tab targets in that window: non-tab tools accept an optional routing-only `tab_id`, while omission uses that connection's current target. MCP `switch_tab` and `open_tab` keep their activating behavior, make the resulting tab current for later omitted routing, and leave earlier controlled tabs available by ID. +- **Process-global native service** (`mcp/dao_mcp_service.{h,cc}`) — A default-off Local State preference controls a single browser-process MCP endpoint that keeps up to 32 authenticated local clients admitted. When a new client completes `hello` beyond that count, the service closes an idle connection (no pending approval or tool call), preferring never-approved sockets and then the least recently active one, releasing any tab leases it held, so the newcomer is admitted and the evicted helper reconnects lazily and re-approves on its next tool call; pre-hello sockets neither count nor get evicted; if every admitted client is busy, the newcomer receives an id-bound `TOO_MANY_CLIENTS` retryable error instead of a silent disconnect. Authentication and catalog discovery never start the approval timer, so idle clients can remain connected and continue listing tools. Each connection's first tool call snapshots the exact last-active eligible normal browser window and active tab, then requests native user approval before execution. Approval prompts are serialized. Each approved connection can retain multiple isolated tab targets in that window: non-tab tools accept an optional routing-only `tab_id`, while omission uses that connection's current target. MCP `switch_tab` and `open_tab` keep their activating behavior, make the resulting tab current for later omitted routing, and leave earlier controlled tabs available by ID. - **Settings master switch, connection, quick setup, and usage** (`mcp/dao_mcp_settings_handler.{h,cc}`, `resources/settings/dao_page/`) — You and Dao groups the process-global MCP Local State switch, live connection status, profile-scoped usage, and enabled-only setup into one native-style card. It renders Disabled, Ready, Approval requested, and Connected states; shows sanitized client details and Stop only for an active authorized lease; and lists total and per-tool calls with a reset action. Usage counts each call that reaches the real executor, whether it succeeds or fails; validation, denial, unknown-tool, and pre-execution target failures are excluded. Quick setup defaults to Codex CLI and also offers a user-scoped Claude Code CLI command plus Generic MCP JSON. The preview and clipboard always contain the same native-generated content: CLI commands remain single-line, while Generic MCP is Chromium-native three-space pretty JSON. Malformed Generic MCP JSON fails closed by clearing the preview and leaving the clipboard unchanged; the former standalone configuration button is removed. Native POSIX shell quoting protects the running bundle's actual helper path and its explicit current user-data-directory argument, so Debug and custom-profile installs connect to the correct runtime endpoint. -- **Hardened local transport** (`mcp/dao_mcp_transport.{h,cc}`, `dao_mcp_connection.{h,cc}`, `dao_mcp_runtime_files.{h,mm}`) — The server owns all listener and connection I/O on Chromium's browser IO thread, uses a non-abstract Unix domain socket inside the user-data MCP directory, enforces owner-only directory/socket/metadata permissions, authenticates the peer UID plus a fresh 256-bit nonce, admits at most 32 external clients, and removes runtime artifacts on disable or shutdown. Atomic `runtime.json` metadata publishes the socket and nonce only inside that private directory. Per-connection and aggregate IO-thread credits bound requests posted but not yet consumed by the UI thread, terminal responses synchronously close only that connection's logical request gate, aggregate write budgets fail closed under backpressure, and graceful close has bounded request and write-drain deadlines. +- **Hardened local transport** (`mcp/dao_mcp_transport.{h,cc}`, `dao_mcp_connection.{h,cc}`, `dao_mcp_runtime_files.{h,mm}`) — The server owns all listener and connection I/O on Chromium's browser IO thread, uses a non-abstract Unix domain socket inside the user-data MCP directory, enforces owner-only directory/socket/metadata permissions, authenticates the peer UID plus a fresh 256-bit nonce, caps open sockets at four times the 32-client admission limit so hello-time eviction and rejection can drain while aggregate ingress credits stay sized for 32 admitted clients, and removes runtime artifacts on disable or shutdown. Atomic `runtime.json` metadata publishes the socket and nonce only inside that private directory. Per-connection and aggregate IO-thread credits bound requests posted but not yet consumed by the UI thread, terminal responses synchronously close only that connection's logical request gate, aggregate write budgets fail closed under backpressure, and graceful close has bounded request and write-drain deadlines. - **Versioned NDJSON protocol** (`mcp/dao_mcp_protocol.{h,cc}`) — Protocol version 1 supports `hello`, `tools/list`, `tools/call`, and `tools/cancel`, with an 8 MiB line ceiling, 64-request/8 MiB pending-ingress budget, and structured errors. Catalog discovery is available before approval, while execution waits for approval and the external automation lease. Every MCP tool advertises a `reason` argument required on the first call of each connection; missing, blank, invalid UTF-8, or reasons over 1024 UTF-8 bytes fail before opening a prompt. Later calls may omit it, and this permission metadata is removed before tool execution. - **Central external-target eligibility and MCP lifecycle policy** (`automation/dao_browser_target_policy.{h,cc}`, `mcp/dao_mcp_session_lifecycle_monitor.{h,cc}`) — Every MCP target stays pinned to its exact tab in the approved normal Browser and regular Profile, with no eligible-tab or active-tab fallback. HTTP, HTTPS, literal `about:blank`, and web-hosted PDFs are allowed; popup, Incognito, Guest, internal, extension, DevTools, Agent WebUI, file, data, and custom-scheme targets are rejected. Target destruction or forbidden navigation cancels and removes only that tab's work, lock, overlay, and CDP state; losing the last target, the Browser, or the Profile closes only the affected logical connection and releases its leases. - **Exact-window approval and control UX** (`dao_mcp_approval_dialog.{h,cc}`, `dao_mcp_control_banner_view.{h,cc}`, `dao_address_bar_view.{h,cc}`, `ui/webui/dao_sidebar_ui.{h,cc}`, `resources/sidebar/dao_tab_item.ts`) — Execution leases display localized, fail-closed Dao system dialogs one at a time in the exact normal Browser selected for approval, with sanitized reported client metadata, the browser-recorded request date/time with time zone, a sanitized reason labeled as client-provided, window, Profile, and current-login warning. Queued prompts preserve their original reason and timestamp, and long reasons scroll within a bounded area. Before showing a prompt, Dao activates that native Browser window so approval requests arriving while Dao is behind another application come to the foreground; unanswered prompts time out after 60 seconds. Allow is intentionally not the default action. A robot button immediately before the URL pill appears only when the active tab is controlled; every controlled tab also shows a quiet robot in its sidebar close-button slot, replaced by the normal close button on pointer hover or keyboard focus. The address-bar popup shows that connection's client, version, current target, latest accepted tool call with live updates, controlled-tab count, and Stop. Stop cancels that connection's external work, releases its leases, and closes it without inserting a control row above page content. diff --git a/docs/mcp-server.md b/docs/mcp-server.md index 9d70a8c9..70162131 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -177,7 +177,8 @@ before allowing access. | `MCP_DISABLED` | Dao is disabled, unavailable, or the browser connection was lost. | | `AUTHORIZATION_DENIED` | Approval, nonce, or client authorization failed. | | `AUTHORIZATION_TIMEOUT` | Hello or user approval did not finish in time. | -| `LEASE_BUSY` | The admitted external client could not acquire the shared browser-control lease. A second external socket is rejected by the one-client admission gate. | +| `TOO_MANY_CLIENTS` | Dao already has 32 MCP clients admitted and every one of them is busy with an approval or a tool call, so nothing idle could be evicted. Retry later or close unused MCP sessions. | +| `LEASE_BUSY` | The admitted external client could not acquire the shared browser-control lease for its target tab. | | `AGENT_CONTROL_BUSY` | Dao Agent attempted a browser tool while MCP held the lease. | | `TARGET_FORBIDDEN` | The exact window, Profile, URL, or switch candidate is ineligible. | | `TARGET_GONE` | The exact approved window, tab, Profile, or owner no longer exists. | diff --git a/src/dao/browser/automation/dao_browser_tool_types.cc b/src/dao/browser/automation/dao_browser_tool_types.cc index b30fb119..70aac8bb 100644 --- a/src/dao/browser/automation/dao_browser_tool_types.cc +++ b/src/dao/browser/automation/dao_browser_tool_types.cc @@ -31,6 +31,8 @@ std::string_view DaoToolErrorCodeToString(DaoToolErrorCode code) { switch (code) { case DaoToolErrorCode::kMcpDisabled: return "MCP_DISABLED"; + case DaoToolErrorCode::kTooManyClients: + return "TOO_MANY_CLIENTS"; case DaoToolErrorCode::kAuthorizationDenied: return "AUTHORIZATION_DENIED"; case DaoToolErrorCode::kAuthorizationTimeout: diff --git a/src/dao/browser/automation/dao_browser_tool_types.h b/src/dao/browser/automation/dao_browser_tool_types.h index 462a7041..6d36deeb 100644 --- a/src/dao/browser/automation/dao_browser_tool_types.h +++ b/src/dao/browser/automation/dao_browser_tool_types.h @@ -27,6 +27,7 @@ struct DaoAgentClientId { enum class DaoToolErrorCode { kMcpDisabled, + kTooManyClients, kAuthorizationDenied, kAuthorizationTimeout, kAgentControlBusy, diff --git a/src/dao/browser/mcp/dao_mcp_foundation_unittest.cc b/src/dao/browser/mcp/dao_mcp_foundation_unittest.cc index e45fda11..d23a8af5 100644 --- a/src/dao/browser/mcp/dao_mcp_foundation_unittest.cc +++ b/src/dao/browser/mcp/dao_mcp_foundation_unittest.cc @@ -110,6 +110,7 @@ std::string MakeCatalogJson(std::string_view output_schema) { TEST(DaoMcpTypesTest, ExposesEveryStableErrorCode) { constexpr std::array expected = { std::pair{DaoToolErrorCode::kMcpDisabled, "MCP_DISABLED"}, + std::pair{DaoToolErrorCode::kTooManyClients, "TOO_MANY_CLIENTS"}, std::pair{DaoToolErrorCode::kAuthorizationDenied, "AUTHORIZATION_DENIED"}, std::pair{DaoToolErrorCode::kAuthorizationTimeout, "AUTHORIZATION_TIMEOUT"}, diff --git a/src/dao/browser/mcp/dao_mcp_service.cc b/src/dao/browser/mcp/dao_mcp_service.cc index e07d1fee..05d47622 100644 --- a/src/dao/browser/mcp/dao_mcp_service.cc +++ b/src/dao/browser/mcp/dao_mcp_service.cc @@ -185,6 +185,7 @@ struct DaoMcpService::TargetContext { struct DaoMcpService::ConnectionState { uint64_t generation = 0; bool closing = false; + base::TimeTicks last_activity; std::optional verified_pid; ApprovalState approval_state = ApprovalState::kNotRequested; base::TimeTicks approval_deadline; @@ -785,6 +786,7 @@ void DaoMcpService::OnTransportAccepted( } auto connection = std::make_unique(); connection->generation = connection_generation; + connection->last_activity = base::TimeTicks::Now(); connection->verified_pid = verified_pid; connection->tab_tool_devtools_client = std::make_unique(); connection->tab_tool_devtools_client->SetCommandCallbackForTesting( @@ -931,9 +933,54 @@ void DaoMcpService::OnHelloTimeout(uint64_t connection_generation) { CloseConnectionAfterWrites(*connection); } +bool DaoMcpService::MakeRoomForConnection(const ConnectionState& connection) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + // Only sockets that completed hello count. Pre-hello sockets are bounded by + // the hello timeout and must not be evicted before their hello is handled. + size_t admitted = 0; + ConnectionState* victim = nullptr; + for (auto& [_, candidate] : connections_) { + if (candidate.get() == &connection || candidate->closing || + !candidate->client_info) { + continue; + } + ++admitted; + const bool busy = candidate->approval_state == ApprovalState::kPending || + !candidate->pending_tool_calls.empty() || + !candidate->active_tool_calls.empty(); + if (busy) { + continue; + } + // Evicting an approved client costs the user another approval prompt, so + // never-approved idle sockets go first, then the least recently active. + const auto rank = [](const ConnectionState& state) { + return std::pair(state.approval_state == ApprovalState::kAllowed, + state.last_activity); + }; + if (!victim || rank(*candidate) < rank(*victim)) { + victim = candidate.get(); + } + } + if (admitted < kDaoMcpMaxConnections) { + return true; + } + if (!victim) { + return false; + } + // Helpers reconnect lazily on their next request. An evicted approved + // client releases its tab leases now and is prompted for approval again. + RejectConnection(*victim, + MakeDaoToolError(DaoToolErrorCode::kTooManyClients, + "The idle MCP connection was closed to " + "admit a newer client.", + true)); + return true; +} + void DaoMcpService::OnRequest(ConnectionState& connection, DaoMcpRequest request) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + connection.last_activity = base::TimeTicks::Now(); if (!connection.client_info && request.method != "hello") { SendError(connection, request.id, InvalidRequest("hello must be the first IPC request.")); @@ -991,6 +1038,15 @@ void DaoMcpService::HandleHello(ConnectionState& connection, CloseConnectionAfterWrites(connection); return; } + if (!MakeRoomForConnection(connection)) { + SendError(connection, request.id, + MakeDaoToolError(DaoToolErrorCode::kTooManyClients, + "Too many MCP clients are connected to Dao " + "Browser and all of them are busy.", + true)); + CloseConnectionAfterWrites(connection); + return; + } connection.client_info = DaoMcpClientInfo{ .name = *name, @@ -1630,6 +1686,7 @@ void DaoMcpService::OnTargetInvalidated(uint64_t connection_generation, void DaoMcpService::FailPendingCalls(ConnectionState& connection, const DaoToolError& error) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + connection.last_activity = base::TimeTicks::Now(); std::vector request_ids; request_ids.reserve(connection.pending_tool_calls.size()); for (const auto& [request_id, _] : connection.pending_tool_calls) { @@ -1652,6 +1709,7 @@ void DaoMcpService::FailPendingCallsForTarget( ConnectionState& connection, std::string_view target_id, const DaoToolError& error) { + connection.last_activity = base::TimeTicks::Now(); std::vector request_ids; for (const auto& [request_id, pending] : connection.pending_tool_calls) { if (pending.target_id == target_id) { @@ -1768,6 +1826,7 @@ void DaoMcpService::OnToolCallComplete(uint64_t connection_generation, if (!connection || !connection->active_tool_calls.erase(request_id)) { return; } + connection->last_activity = base::TimeTicks::Now(); connection->active_tool_targets.erase(request_id); auto bytes = connection->active_tool_call_bytes.find(request_id); if (bytes != connection->active_tool_call_bytes.end()) { diff --git a/src/dao/browser/mcp/dao_mcp_service.h b/src/dao/browser/mcp/dao_mcp_service.h index a327ca61..a7671a48 100644 --- a/src/dao/browser/mcp/dao_mcp_service.h +++ b/src/dao/browser/mcp/dao_mcp_service.h @@ -140,6 +140,7 @@ class DaoMcpService { return tool_call_completion_count_for_testing_; } bool connection_active_for_testing() const { return !connections_.empty(); } + size_t connection_count_for_testing() const { return connections_.size(); } private: friend class base::NoDestructor; @@ -196,6 +197,7 @@ class DaoMcpService { const ConnectionState* GetDisplayConnection() const; void ResetConnectionState(ConnectionState& connection); void OnHelloTimeout(uint64_t connection_generation); + bool MakeRoomForConnection(const ConnectionState& connection); void OnRequest(ConnectionState& connection, DaoMcpRequest request); void HandleHello(ConnectionState& connection, DaoMcpRequest request); diff --git a/src/dao/browser/mcp/dao_mcp_service_browsertest.cc b/src/dao/browser/mcp/dao_mcp_service_browsertest.cc index 71306439..dfe9c3cf 100644 --- a/src/dao/browser/mcp/dao_mcp_service_browsertest.cc +++ b/src/dao/browser/mcp/dao_mcp_service_browsertest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "dao/browser/mcp/dao_mcp_service.h" +#include "dao/browser/mcp/dao_mcp_transport.h" #include #include @@ -880,6 +881,14 @@ class DaoMcpServiceBrowserTest : public InProcessBrowserTest { })); } + base::DictValue ToolsListRequest(std::string id) { + return base::DictValue() + .Set("version", kDaoMcpIpcVersion) + .Set("id", std::move(id)) + .Set("method", "tools/list") + .Set("params", base::DictValue()); + } + base::DictValue HelloRequest(const std::string& nonce, std::string id = "hello-1") { return base::DictValue() @@ -1867,6 +1876,124 @@ IN_PROC_BROWSER_TEST_F(DaoMcpServiceBrowserTest, IdleHelloCandidateIsEvicted) { approval.Resolve(false); } +IN_PROC_BROWSER_TEST_F(DaoMcpServiceBrowserTest, + EvictsLeastRecentlyActiveIdleClientAtCapacity) { + EnableService(); + std::vector> clients; + for (size_t index = 0; index < kDaoMcpMaxConnections; ++index) { + std::unique_ptr client = ConnectClient(); + ASSERT_TRUE(client); + ASSERT_TRUE(client->Send(HelloRequest(nonce()))); + std::optional hello_response = client->Read(); + ASSERT_TRUE(hello_response); + ASSERT_TRUE(hello_response->FindDict("result")); + clients.push_back(std::move(client)); + } + // Touch the oldest connection so the second one becomes the least recently + // active idle candidate. + ASSERT_TRUE(clients[0]->Send(ToolsListRequest("keep-alive"))); + ASSERT_TRUE(clients[0]->Read()); + + std::unique_ptr newcomer = ConnectClient(); + ASSERT_TRUE(newcomer); + ASSERT_TRUE(newcomer->Send(HelloRequest(nonce(), "newcomer-hello"))); + std::optional newcomer_hello = newcomer->Read(); + ASSERT_TRUE(newcomer_hello); + EXPECT_TRUE(newcomer_hello->FindDict("result")); + ASSERT_TRUE(base::test::RunUntil([this] { + return service_->connection_count_for_testing() == kDaoMcpMaxConnections; + })); + + // If the wrong client were evicted this request would be answered instead + // of hitting EOF, so the read cannot block forever. + clients[1]->Send(ToolsListRequest("evicted")); + EXPECT_FALSE(clients[1]->Read()); + ASSERT_TRUE(clients[0]->Send(ToolsListRequest("still-alive"))); + std::optional list_response = clients[0]->Read(); + ASSERT_TRUE(list_response); + EXPECT_TRUE(list_response->FindDict("result")); +} + +IN_PROC_BROWSER_TEST_F(DaoMcpServiceBrowserTest, + EvictsIdleApprovedClientAndReleasesLease) { + FakeApprovalDelegate approval; + service_->SetApprovalDelegate(&approval); + EnableService(); + std::unique_ptr approved = ConnectClient(); + ASSERT_TRUE(approved); + ASSERT_TRUE(approved->Send(HelloRequest(nonce()))); + ASSERT_TRUE(approved->Read()); + ApproveFirstToolCall(approved.get(), &approval); + EXPECT_EQ(1u, service_->GetControlledTargetCount()); + + // Fill the remaining slots with clients blocked on approval. Never-approved + // idle sockets would be evicted first, so every other client must be busy + // for the approved idle one to become the only candidate. + std::vector> busy; + for (size_t index = 1; index < kDaoMcpMaxConnections; ++index) { + std::unique_ptr client = ConnectClient(); + ASSERT_TRUE(client); + ASSERT_TRUE(client->Send(HelloRequest(nonce()))); + ASSERT_TRUE(client->Read()); + ASSERT_TRUE(client->Send(ToolCall("busy-" + std::to_string(index)))); + busy.push_back(std::move(client)); + } + ASSERT_TRUE(base::test::RunUntil([&approval] { + return approval.request_count() == kDaoMcpMaxConnections; + })); + + std::unique_ptr newcomer = ConnectClient(); + ASSERT_TRUE(newcomer); + ASSERT_TRUE(newcomer->Send(HelloRequest(nonce(), "newcomer-hello"))); + std::optional newcomer_hello = newcomer->Read(); + ASSERT_TRUE(newcomer_hello); + EXPECT_TRUE(newcomer_hello->FindDict("result")); + + ASSERT_TRUE(base::test::RunUntil([this] { + return service_->connection_count_for_testing() == kDaoMcpMaxConnections; + })); + EXPECT_EQ(0u, service_->GetControlledTargetCount()); + EXPECT_NE(DaoMcpStatus::kLeaseActive, service_->GetStatus().state); + approved->Send(ToolsListRequest("evicted")); + EXPECT_FALSE(approved->Read()); + ASSERT_TRUE(newcomer->Send(ToolsListRequest("admitted"))); + std::optional list_response = newcomer->Read(); + ASSERT_TRUE(list_response); + EXPECT_TRUE(list_response->FindDict("result")); +} + +IN_PROC_BROWSER_TEST_F(DaoMcpServiceBrowserTest, + RejectsHelloWithTooManyClientsWhenAllAreBusy) { + FakeApprovalDelegate approval; + service_->SetApprovalDelegate(&approval); + EnableService(); + std::vector> clients; + for (size_t index = 0; index < kDaoMcpMaxConnections; ++index) { + std::unique_ptr client = ConnectClient(); + ASSERT_TRUE(client); + ASSERT_TRUE(client->Send(HelloRequest(nonce()))); + ASSERT_TRUE(client->Read()); + ASSERT_TRUE(client->Send(ToolCall("busy-" + std::to_string(index)))); + clients.push_back(std::move(client)); + } + ASSERT_TRUE(base::test::RunUntil([&approval] { + return approval.request_count() == kDaoMcpMaxConnections; + })); + + std::unique_ptr newcomer = ConnectClient(); + ASSERT_TRUE(newcomer); + ASSERT_TRUE(newcomer->Send(HelloRequest(nonce(), "newcomer-hello"))); + std::optional response = newcomer->Read(); + ASSERT_TRUE(response); + EXPECT_EQ("newcomer-hello", *response->FindString("id")); + const base::DictValue* error = response->FindDict("error"); + ASSERT_TRUE(error); + EXPECT_EQ("TOO_MANY_CLIENTS", *error->FindString("code")); + EXPECT_TRUE(error->FindBool("retryable").value_or(false)); + EXPECT_FALSE(newcomer->Read()); + EXPECT_EQ(kDaoMcpMaxConnections, approval.request_count()); +} + IN_PROC_BROWSER_TEST_F(DaoMcpServiceBrowserTest, BoundsOutstandingToolCalls) { FakeApprovalDelegate approval; service_->SetApprovalDelegate(&approval); diff --git a/src/dao/browser/mcp/dao_mcp_transport.cc b/src/dao/browser/mcp/dao_mcp_transport.cc index cc859940..617802c3 100644 --- a/src/dao/browser/mcp/dao_mcp_transport.cc +++ b/src/dao/browser/mcp/dao_mcp_transport.cc @@ -22,14 +22,20 @@ namespace dao { namespace { -constexpr size_t kMaxConnections = 32; -constexpr int kListenerBacklog = static_cast(kMaxConnections); +// Hard IO-thread ceiling. Admission is enforced by DaoMcpService at hello +// time (see kDaoMcpMaxConnections), which needs the socket accepted so it can +// answer with an id-bound error or evict an idle peer. This only bounds +// sockets while those evictions drain. +constexpr size_t kMaxConnections = kDaoMcpMaxConnections * 4; +constexpr int kListenerBacklog = static_cast(kDaoMcpMaxConnections); constexpr size_t kMaxPendingRequests = 64; constexpr size_t kMaxPendingRequestBytes = kDaoMcpMaxLineBytes + 1; +// Aggregate credits stay sized for admitted clients: pre-hello sockets send a +// single hello, so they never need the extra headroom. constexpr size_t kMaxTotalPendingRequests = - kMaxConnections * kMaxPendingRequests; + kDaoMcpMaxConnections * kMaxPendingRequests; constexpr size_t kMaxTotalPendingRequestBytes = - kMaxConnections * kMaxPendingRequestBytes; + kDaoMcpMaxConnections * kMaxPendingRequestBytes; constexpr base::TimeDelta kPendingRequestDrainTimeout = base::Seconds(1); } // namespace @@ -98,6 +104,7 @@ void DaoMcpTransport::Stop() { connections_.clear(); total_pending_request_count_ = 0; total_pending_request_bytes_ = 0; + overflow_logged_ = false; accepted_verified_pid_.reset(); socket_path_.clear(); } @@ -207,6 +214,11 @@ void DaoMcpTransport::OnAccepted(int result) { } if (connections_.size() >= kMaxConnections) { + if (!overflow_logged_) { + overflow_logged_ = true; + LOG(ERROR) << "Dao MCP dropped a local connection: " + << connections_.size() << " sockets are already open."; + } accepted_socket_->Disconnect(); accepted_socket_.reset(); AcceptNext(); diff --git a/src/dao/browser/mcp/dao_mcp_transport.h b/src/dao/browser/mcp/dao_mcp_transport.h index a125a3ee..12953fc9 100644 --- a/src/dao/browser/mcp/dao_mcp_transport.h +++ b/src/dao/browser/mcp/dao_mcp_transport.h @@ -28,6 +28,11 @@ class UnixDomainServerSocket; namespace dao { +// Number of clients DaoMcpService keeps admitted at once. When a new client +// completes hello beyond this count, the service evicts the least recently +// active idle connection or rejects the newcomer with TOO_MANY_CLIENTS. +inline constexpr size_t kDaoMcpMaxConnections = 32; + // Owns the Unix socket listener and accepted connections. Every method and // callback runs on the browser IO thread. class DaoMcpTransport { @@ -84,6 +89,7 @@ class DaoMcpTransport { uint64_t next_connection_generation_ = 1; size_t total_pending_request_count_ = 0; size_t total_pending_request_bytes_ = 0; + bool overflow_logged_ = false; SEQUENCE_CHECKER(sequence_checker_); base::WeakPtrFactory weak_factory_{this}; }; diff --git a/src/dao/browser/mcp/helper/dao_mcp_stdio_server.cc b/src/dao/browser/mcp/helper/dao_mcp_stdio_server.cc index 8a9a9da8..dce6320b 100644 --- a/src/dao/browser/mcp/helper/dao_mcp_stdio_server.cc +++ b/src/dao/browser/mcp/helper/dao_mcp_stdio_server.cc @@ -18,6 +18,7 @@ #include "base/containers/span.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" +#include "base/strings/strcat.h" #include "dao/browser/mcp/dao_mcp_protocol.h" #include "dao/browser/mcp/helper/dao_mcp_browser_client.h" @@ -481,17 +482,28 @@ void DaoMcpStdioServer::FailBrowser(std::string_view reason, if (browser_) { browser_->Disconnect(); } - LogUnavailableOnce(); if (!browser_error) { + // An idle socket the browser closed on purpose (for example to admit a + // newer client) is not an outage. The next request reconnects lazily and + // reports unavailability itself if that fails. + if (!pending_.empty()) { + LogUnavailableOnce(); + } FailAllPending(); return; } const std::string* code = browser_error->FindString("code"); const std::string* message = browser_error->FindString("message"); + const std::string shown_message = + message && !message->empty() ? *message : std::string(reason); + // The browser is reachable and explained the failure, so surface its + // message instead of the unavailability notice. + const std::string line = base::StrCat({"dao-mcp: ", shown_message, "\n"}); + std::ignore = write(STDERR_FILENO, line.data(), line.size()); base::DictValue normalized_error = ErrorPayload(code && !code->empty() ? *code : "INTERNAL_ERROR", - message && !message->empty() ? *message : reason, + shown_message, browser_error->FindBool("retryable").value_or(false)); FailAllPending(&normalized_error); } diff --git a/src/dao/browser/ui/dao_ui_sources.gni b/src/dao/browser/ui/dao_ui_sources.gni index 906fb1c4..b7edc939 100644 --- a/src/dao/browser/ui/dao_ui_sources.gni +++ b/src/dao/browser/ui/dao_ui_sources.gni @@ -283,6 +283,7 @@ dao_browser_ui_test_deps = [ "//components/qr_code_generator", "//dao/browser/mcp:browser_mcp", "//dao/browser/updater:dao_updater", + "//ui/base/clipboard:clipboard_test_support", ] if (is_mac) { From 57e6d49845f8097ccf1df68a3f7dd3d78228854f Mon Sep 17 00:00:00 2001 From: moonrailgun Date: Sat, 19 Sep 2026 04:49:10 +0800 Subject: [PATCH 2/2] test: derive android release version and extend pi adapter timeout --- .../__tests__/release-android.test.ts | 19 ++++++++++++++----- .../agent/__tests__/pi_tool_adapter.test.ts | 6 +++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/scripts/commands/__tests__/release-android.test.ts b/scripts/commands/__tests__/release-android.test.ts index f70e17ca..bd53305e 100644 --- a/scripts/commands/__tests__/release-android.test.ts +++ b/scripts/commands/__tests__/release-android.test.ts @@ -31,12 +31,21 @@ describe('Android release CLI', () => { ]) expect(() => validateAndroidVersion(next, {version: '0.1.0', versionCode: 1})).toThrow(); }); - it('previews an explicit version without credentials or publication', () => { - const result = runCli('--version', '0.1.1', '--version-code', '2', '--dry-run'); + it('previews an explicit version without credentials or publication', async () => { + // The CLI reads the checked-in Gradle file, so derive the target from it + // instead of hardcoding a version that every Android release invalidates. + const gradle = readFileSync('android/app/build.gradle.kts', 'utf8'); + const current = { + version: /versionName\s*=\s*"([^"]+)"/.exec(gradle)![1], + versionCode: Number(/versionCode\s*=\s*(\d+)/.exec(gradle)![1]), + }; + const next = await resolveAndroidVersion(current, {}, async () => ''); + const result = runCli( + '--version', next.version, '--version-code', String(next.versionCode), '--dry-run'); expect(result.status, result.stderr).toBe(0); - expect(result.stdout).toContain('android-v0.1.1'); - expect(result.stdout).toContain('versionCode: 1 -> 2'); - expect(result.stdout).toContain('chore(android): release 0.1.1'); + expect(result.stdout).toContain(`android-v${next.version}`); + expect(result.stdout).toContain(`versionCode: ${current.versionCode} -> ${next.versionCode}`); + expect(result.stdout).toContain(`chore(android): release ${next.version}`); expect(result.stdout).toContain('git push --atomic'); }); diff --git a/src/dao/browser/ui/webui/resources/agent/__tests__/pi_tool_adapter.test.ts b/src/dao/browser/ui/webui/resources/agent/__tests__/pi_tool_adapter.test.ts index dd9813ff..5344df15 100644 --- a/src/dao/browser/ui/webui/resources/agent/__tests__/pi_tool_adapter.test.ts +++ b/src/dao/browser/ui/webui/resources/agent/__tests__/pi_tool_adapter.test.ts @@ -136,7 +136,11 @@ describe('pi_tool_adapter', () => { if (mode !== 'success') { expect(results[1].content[0].text).toContain('Skipped browser action'); } - }); + }, + // The first case pays for importing the vendor bundle, which exceeds the + // default 5s on loaded CI runners. A timed-out agent keeps running and + // pollutes the next case's mock, so give the whole group headroom. + 30_000); beforeEach(() => { mocks.executeTool.mockReset();