From 1a46df5d92ea9981568b005311d939a5ca7bda24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Wed, 1 Jul 2026 17:46:00 +0800 Subject: [PATCH] fix(cli): load browser MCP tools by default (#6006) * fix(cli): load browser MCP tools by default * fix(cli): cover browser MCP env flags * fix: address browser MCP review follow-ups * fix(cli): add browser MCP diagnostics * fix(cli): tighten browser MCP auto-wiring * fix(cli): address browser MCP diagnostics * revert(cli): drop browser MCP diagnostic churn * revert(cli): drop optional CDP startup diagnostic * refactor(cli): load browser MCP dynamically * fix(cli): lazily attach CDP tunnel * test(cli): use repo deps for CDP tunnel acceptance * fix(cli): scope browser MCP defaults to extension origins * fix(cli): recover from lazy CDP attach failures * fix(chrome-extension): bind CDP replies to source socket * ci: allow slower actionlint runs * fix(cli): harden chrome devtools runtime MCP registration * test(cli): satisfy lint in CDP registration race test * test(cli): cover chrome devtools MCP retry loop * test(cli): cover chrome devtools skip paths * ci: restore actionlint timeout --- docs/users/configuration/settings.md | 32 +- .../cli/qwen-serve-routes.test.ts | 11 +- packages/acp-bridge/src/bridge.ts | 2 +- packages/acp-bridge/src/bridgeTypes.ts | 6 +- .../src/background/service-worker.ts | 32 +- packages/cli/src/acp-integration/acpAgent.ts | 82 +---- .../acp-integration/cdp-tunnel-mcp.test.ts | 45 --- packages/cli/src/serve/acp-http/index.ts | 219 ++++++++++- .../cli/src/serve/acp-http/transport.test.ts | 341 +++++++++++++++++- packages/cli/src/serve/capabilities.ts | 15 +- .../cdp-tunnel/acceptance/cdp-mcp-smoke.mjs | 15 + .../acceptance/cdp-tunnel-acceptance.mjs | 8 +- .../serve/cdp-tunnel/cdp-reverse-link.test.ts | 77 +++- .../src/serve/cdp-tunnel/cdp-reverse-link.ts | 58 ++- .../cli/src/serve/cdp-tunnel/cdp-ws.test.ts | 108 +++++- packages/cli/src/serve/cdp-tunnel/cdp-ws.ts | 37 +- packages/cli/src/serve/run-qwen-serve.test.ts | 177 ++++++++- packages/cli/src/serve/run-qwen-serve.ts | 62 ++-- packages/cli/src/serve/server.test.ts | 6 +- packages/cli/src/serve/server.ts | 1 + packages/cli/src/serve/types.ts | 14 +- packages/core/src/config/config.ts | 1 + .../core/src/tools/mcp-client-manager.test.ts | 68 +++- packages/core/src/tools/mcp-client-manager.ts | 27 +- packages/core/src/tools/mcp-client.test.ts | 26 ++ packages/core/src/tools/mcp-client.ts | 1 + packages/core/src/tools/mcp-tool.ts | 5 +- packages/sdk-typescript/src/daemon/types.ts | 2 +- .../schemas/settings.schema.json | 7 + 29 files changed, 1198 insertions(+), 287 deletions(-) delete mode 100644 packages/cli/src/acp-integration/cdp-tunnel-mcp.test.ts diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index 6282cede69..b55da91c4f 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -92,7 +92,7 @@ Settings are organized into categories. Most settings should be placed within th | `general.outputLanguage` | string | Language for model output. Use `"auto"` to detect from system settings, or set a specific language. Requires restart. | `"auto"` | | `general.dynamicCommandTranslation` | boolean | Enable AI translation of dynamic slash-command descriptions. When disabled, dynamic commands keep their original descriptions and skip translation model calls. | `false` | | `general.terminalBell` | boolean | Play a terminal bell sound when a response completes or needs approval. | `true` | -| `general.preventSystemSleep` | boolean | Prevent the system from sleeping while Qwen Code is streaming a model response or executing tools. Idle prompt time and permission prompts do not inhibit sleep. Read once at startup, so changes take effect after restart. | `true` | +| `general.preventSystemSleep` | boolean | Prevent the system from sleeping while Qwen Code is streaming a model response or executing tools. Idle prompt time and permission prompts do not inhibit sleep. Read once at startup, so changes take effect after restart. | `true` | | `general.chatRecording` | boolean | Save chat history to disk. Disabling this also prevents `--continue` and `--resume` from working. Requires restart. | `true` | #### output @@ -131,11 +131,11 @@ Settings are organized into categories. Most settings should be placed within th | `ui.enableCacheSharing` | boolean | Use cache-aware forked queries for suggestion generation. Reduces cost on providers that support prefix caching (experimental). | `true` | | `ui.enableSpeculation` | boolean | Speculatively execute accepted suggestions before submission. Results appear instantly when you accept (experimental). | `false` | | `ui.showStatusInTitle` | boolean | Show the Qwen Code session name and status in the terminal window title. | `true` | -| `ui.disableWorkflowKeywordTrigger` | boolean | When `true`, mentioning the word `workflow` in a prompt no longer softly steers the turn toward the Workflow tool (and the Footer `workflow active` indicator is suppressed). Only applies when workflows are enabled. | `false` | +| `ui.disableWorkflowKeywordTrigger` | boolean | When `true`, mentioning the word `workflow` in a prompt no longer softly steers the turn toward the Workflow tool (and the Footer `workflow active` indicator is suppressed). Only applies when workflows are enabled. | `false` | | `ui.enableUserFeedback` | boolean | Show an optional feedback dialog after conversations to help improve Qwen performance. | `true` | | `ui.compactInline` | boolean | Compact tool display within each group instead of merging across groups. Requires `ui.compactMode` to be enabled. Requires restart. | `false` | -| `ui.useTerminalBuffer` | boolean | Render conversation history in an in-app scrollable viewport instead of the terminal scrollback buffer. Recommended if you see flicker, scroll-storm, or interface freeze on long sessions. Scroll with `Shift+↑/↓` (line), `PgUp`/`PgDn` (page), `Ctrl+Home/End` (top/bottom), or the mouse wheel. Does not use the host terminal scrollback while enabled; hold `Shift` (or `Option` on macOS) while dragging for native text selection. | `false` | -| `ui.hideBuiltinWorktreeIndicator` | boolean | Hide the built-in `⎇ worktree- ()` line in the Footer. The worktree state is still passed to custom statusline scripts via the stdin payload. Keep at the default unless your custom statusline renders the worktree itself. | `false` | +| `ui.useTerminalBuffer` | boolean | Render conversation history in an in-app scrollable viewport instead of the terminal scrollback buffer. Recommended if you see flicker, scroll-storm, or interface freeze on long sessions. Scroll with `Shift+↑/↓` (line), `PgUp`/`PgDn` (page), `Ctrl+Home/End` (top/bottom), or the mouse wheel. Does not use the host terminal scrollback while enabled; hold `Shift` (or `Option` on macOS) while dragging for native text selection. | `false` | +| `ui.hideBuiltinWorktreeIndicator` | boolean | Hide the built-in `⎇ worktree- ()` line in the Footer. The worktree state is still passed to custom statusline scripts via the stdin payload. Keep at the default unless your custom statusline renders the worktree itself. | `false` | #### ide @@ -287,7 +287,7 @@ If you are experiencing performance issues with file searching (e.g., with `@` c | `tools.truncateToolOutputLines` | number | Maximum lines or entries kept when truncating tool output. Applies to Shell, Grep, Glob, ReadFile and ReadManyFiles tools. | `1000` | Requires restart: Yes | | `tools.computerUse.enabled` | boolean | Enable the built-in Computer Use tools (cua-driver native desktop automation). When `true` (default), the `computer_use__*` tools are registered as deferred built-ins; the first invocation downloads the pinned, signed cua-driver binary into `~/.qwen/computer-use/` and walks through macOS Accessibility / Screen Recording permissions. | `true` | Requires restart: Yes | | `tools.computerUse.maxImageDimension` | number | Longest-edge pixel cap applied to cua-driver screenshots (via `set_config`'s `max_image_dimension`). `-1` (default) keeps cua-driver's built-in default (1568); `0` disables resizing (full resolution); a positive value caps the longest edge. Lower caps cut vision-token cost at the expense of fine detail. | `-1` | Requires restart: Yes. Env override: `QWEN_COMPUTER_USE_MAX_IMAGE_DIMENSION` (a non-negative integer; takes precedence over this setting) | -| `tools.toolSearch.enabled` | boolean | Load MCP tools on demand via ToolSearch to reduce prompt size. Disable this for models that rely on prefix-based KV caching (e.g. DeepSeek) to keep the prompt prefix stable and maximize cache hit rates. | `true` | Requires restart: Yes | +| `tools.toolSearch.enabled` | boolean | Load MCP tools on demand via ToolSearch to reduce prompt size. Disable this for models that rely on prefix-based KV caching (e.g. DeepSeek) to keep the prompt prefix stable and maximize cache hit rates. | `true` | Requires restart: Yes | > [!note] > @@ -295,13 +295,13 @@ If you are experiencing performance issues with file searching (e.g., with `@` c #### memory -| Setting | Type | Description | Default | -| -------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------ | ------- | -| `memory.enableManagedAutoMemory` | boolean | Enable background extraction of memories from conversations. | `true` | -| `memory.enableManagedAutoDream` | boolean | Enable automatic consolidation (deduplication and cleanup) of collected memories. | `true` | -| `memory.enableAutoSkill` | boolean | Enable background review for reusable project skills after tool-heavy sessions. | `true` | -| `memory.autoSkillConfirm` | boolean | Ask for confirmation before auto-generated skills are added to the skill library. When off, auto-skills are saved immediately. | `true` | -| `memory.enableTeamMemory` | boolean | Enable a project memory tier shared with collaborators via the git-tracked `.qwen/team-memory/` directory. Writes to it are secret-scanned and reviewable in the git diff. | `false` | +| Setting | Type | Description | Default | +| -------------------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | +| `memory.enableManagedAutoMemory` | boolean | Enable background extraction of memories from conversations. | `true` | +| `memory.enableManagedAutoDream` | boolean | Enable automatic consolidation (deduplication and cleanup) of collected memories. | `true` | +| `memory.enableAutoSkill` | boolean | Enable background review for reusable project skills after tool-heavy sessions. | `true` | +| `memory.autoSkillConfirm` | boolean | Ask for confirmation before auto-generated skills are added to the skill library. When off, auto-skills are saved immediately. | `true` | +| `memory.enableTeamMemory` | boolean | Enable a project memory tier shared with collaborators via the git-tracked `.qwen/team-memory/` directory. Writes to it are secret-scanned and reviewable in the git diff. | `false` | | `memory.enableTeamMemorySync` | boolean | When team memory is enabled, automatically commit, fast-forward-pull, and push the `.qwen/team-memory/` directory at session start so collaborators stay in sync. Requires a configured git upstream. | `false` | See [Memory](../features/memory) for details on how auto-memory works and how to use the `/memory`, `/remember`, and `/dream` commands. @@ -436,11 +436,11 @@ execute when typed. #### mcp -| Setting | Type | Description | Default | -| ------------------- | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | -| `mcp.serverCommand` | string | Command to start an MCP server. | `undefined` | +| Setting | Type | Description | Default | +| ------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | +| `mcp.serverCommand` | string | Command to start an MCP server. | `undefined` | | `mcp.allowed` | array of strings | An allowlist of MCP servers to allow. Allows you to specify a list of MCP server names that should be made available to the model. This can be used to restrict the set of MCP servers to connect to. Supports glob patterns (`*` matches any sequence, `?` matches a single character — e.g. `"*puppeteer*"`); entries without glob characters are matched exactly. Note that this will be ignored if `--allowed-mcp-server-names` is set. | `undefined` | -| `mcp.excluded` | array of strings | A denylist of MCP servers to exclude. A server listed in both `mcp.excluded` and `mcp.allowed` is excluded. Supports glob patterns (`*`, `?`) the same way as `mcp.allowed`. Note that this will be ignored if `--allowed-mcp-server-names` is set. | `undefined` | +| `mcp.excluded` | array of strings | A denylist of MCP servers to exclude. A server listed in both `mcp.excluded` and `mcp.allowed` is excluded. Supports glob patterns (`*`, `?`) the same way as `mcp.allowed`. Note that this will be ignored if `--allowed-mcp-server-names` is set. | `undefined` | > [!note] > diff --git a/integration-tests/cli/qwen-serve-routes.test.ts b/integration-tests/cli/qwen-serve-routes.test.ts index 5c5197b8fc..cedfcba460 100644 --- a/integration-tests/cli/qwen-serve-routes.test.ts +++ b/integration-tests/cli/qwen-serve-routes.test.ts @@ -87,6 +87,8 @@ beforeAll(async () => { 'QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS', 'QWEN_SERVE_RATE_LIMIT', 'QWEN_SERVE_NO_MCP_POOL', + 'QWEN_SERVE_CLIENT_MCP_OVER_WS', + 'QWEN_SERVE_CDP_TUNNEL_OVER_WS', ].includes(k), ), ), @@ -224,15 +226,14 @@ describe('qwen serve — capabilities envelope', () => { // Conditional tags absent under this suite's spawn flags (no // `--require-auth` / `--allow-origin` / deadline env vars / // rate-limit opt-in, no configured batch ASR model): `require_auth`, - // `allow_origin`, `prompt_absolute_deadline`, `writer_idle_timeout`, - // `workspace_voice_transcription`, `rate_limit`. + // `allow_origin`, `cdp_tunnel_over_ws`, `prompt_absolute_deadline`, + // `writer_idle_timeout`, `workspace_voice_transcription`, `rate_limit`. // Pool tags (`mcp_workspace_pool`, `mcp_pool_restart`) ARE present // because the workspace MCP pool is on by default, as are // `workspace_settings`, `workspace_permissions`, `workspace_voice`, // `workspace_trust`, `workspace_github_setup`, and - // `workspace_reload` (the CLI serve path always wires - // `persistSetting`, the workspace service, and route-local - // workspace helpers). + // `workspace_reload`. The CLI serve path always wires `persistSetting`, the + // workspace service, and route-local workspace helpers). expect(caps.features).toEqual([ 'health', 'daemon_status', diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 2b780c10db..792052d430 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -5372,7 +5372,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { type AddSkip = { name: string; skipped: true; - reason: 'budget_warning_only'; + reason: 'budget_warning_only' | 'runtime_name_conflict'; }; const response = (await Promise.race([ withTimeout( diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 8705e919a8..2dc9fbee46 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -860,7 +860,11 @@ export interface AcpSessionBridge { toolCount: number; originatorClientId: string; } - | { name: string; skipped: true; reason: 'budget_warning_only' } + | { + name: string; + skipped: true; + reason: 'budget_warning_only' | 'runtime_name_conflict'; + } >; /** diff --git a/packages/chrome-extension/src/background/service-worker.ts b/packages/chrome-extension/src/background/service-worker.ts index d5a8359c7a..3f2aef1af8 100644 --- a/packages/chrome-extension/src/background/service-worker.ts +++ b/packages/chrome-extension/src/background/service-worker.ts @@ -77,20 +77,21 @@ function toWebSocketUrl(baseUrl: string): string { } /** Send any JSON message if the socket is open; swallow failures (close handles it). */ -function sendRaw(message: unknown): void { - if (!socket || socket.readyState !== WebSocket.OPEN) { +function sendRaw(ws: WebSocket, message: unknown): void { + if (ws.readyState !== WebSocket.OPEN) { console.warn(LOG_PREFIX, 'sendRaw: socket not OPEN, dropping frame'); return; } try { - socket.send(JSON.stringify(message)); + ws.send(JSON.stringify(message)); } catch (error) { console.warn(LOG_PREFIX, 'Failed to send:', error); } } /** Parse and route an inbound WS frame. */ -function onWsMessage(data: unknown): void { +function onWsMessage(ws: WebSocket, data: unknown): void { + if (socket !== ws) return; let msg: Record; try { msg = JSON.parse(String(data)) as Record; @@ -112,7 +113,7 @@ function onWsMessage(data: unknown): void { 'ACP initialize failed; closing socket:', msg['error'], ); - socket?.close(); + ws.close(); } else { console.log(LOG_PREFIX, 'ACP initialized; CDP tunnel ready'); } @@ -120,9 +121,11 @@ function onWsMessage(data: unknown): void { } // CDP-tunnel frames: route to the bridge, which drives the tab via - // chrome.debugger and pushes results/events back over this socket. + // chrome.debugger and pushes results/events back over the active socket. if (isCdpBridgeFrame(msg['type'])) { - handleCdpFrame(msg as { type?: unknown }, (frame) => sendRaw(frame)); + handleCdpFrame(msg as { type?: unknown }, (frame) => + sendRaw(ws, frame), + ); return; } // Other frame types (chat/session traffic) aren't ours; ignore. @@ -183,9 +186,13 @@ async function connect(): Promise { socket = ws; ws.onopen = () => { + if (socket !== ws) { + ws.close(); + return; + } reconnectDelay = RECONNECT_MIN_MS; console.log(LOG_PREFIX, 'Connected; sending ACP initialize'); - sendRaw({ + sendRaw(ws, { jsonrpc: '2.0', id: ACP_INIT_ID, method: 'initialize', @@ -197,7 +204,7 @@ async function connect(): Promise { }); }; - ws.onmessage = (event: MessageEvent) => onWsMessage(event.data); + ws.onmessage = (event: MessageEvent) => onWsMessage(ws, event.data); ws.onerror = (event: Event) => { console.warn(LOG_PREFIX, 'WebSocket error', event); @@ -216,10 +223,9 @@ async function connect(): Promise { // force-closed a stale socket after the extension already opened a new one, // that stale close must NOT detach the new connection's debugger — doing so // would yank the debugger banner and break the live `/cdp` client. - if (socket === ws) { - socket = null; - shutdownCdpBridge(); - } + if (socket !== ws) return; + socket = null; + shutdownCdpBridge(); scheduleReconnect(); }; } diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 8b8031d1ef..d44b6f81d0 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -132,7 +132,6 @@ import { Readable, Writable } from 'node:stream'; import { normalizeDisabledToolList } from '../config/normalizeDisabledTools.js'; import { pipeline } from 'node:stream/promises'; import * as fs from 'node:fs/promises'; -import { createRequire } from 'node:module'; import * as path from 'node:path'; import { createGunzip } from 'node:zlib'; import type { LoadedSettings } from '../config/settings.js'; @@ -7592,16 +7591,8 @@ class QwenAgent implements Agent { } } - // CDP tunnel (Plan C, #5626): auto-register chrome-devtools-mcp when the - // daemon forwarded the tunnel flag, so the agent can drive the real browser. - const cdpTunnelMcp = buildCdpTunnelMcpServer(); - // Don't clobber a `chrome-devtools` server the user configured themselves — - // their explicit session config wins over the tunnel auto-wire. - if (cdpTunnelMcp && !sessionMcpServers['chrome-devtools']) { - sessionMcpServers['chrome-devtools'] = cdpTunnelMcp; - } - const settings = this.settings.merged; + const argvForSession = { ...this.argv, ...(resume ? { resume: sessionId } : { sessionId }), @@ -7977,74 +7968,3 @@ function diffSettingsKeys( } return changed; } - -/** - * CDP tunnel auto-wiring (Plan C, issue #5626). - * - * Builds the `chrome-devtools` session MCP server entry when the daemon runs - * with the CDP-tunnel flag. The daemon forwards `QWEN_SERVE_CDP_TUNNEL_OVER_WS` - * + `QWEN_SERVE_CDP_TUNNEL_PORT` into this child's env; we point the patched - * chrome-devtools-mcp at the daemon's `/cdp` endpoint. - * - * Returns `undefined` when the flag is off, the port is missing/invalid, or - * chrome-devtools-mcp can't be resolved. `trust` is left unset so the tools - * default to 'ask' — no silent auto-approval of browser control. - */ -export function buildCdpTunnelMcpServer(): MCPServerConfig | undefined { - if (process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] !== '1') return undefined; - // Auth-gated `/cdp`: the daemon requires a bearer token on the tunnel, but - // this ACP child can't inherit QWEN_SERVER_TOKEN (the spawn path scrubs it) - // and chrome-devtools-mcp is launched with `--wsEndpoint` only — it has no way - // to send `--wsHeaders` auth. Auto-registering the browser tools here would - // surface tools that can't connect, so skip with a clear diagnostic instead. - if (process.env['QWEN_SERVE_CDP_TUNNEL_AUTH_REQUIRED'] === '1') { - process.stderr.write( - 'qwen serve: browser tools (chrome-devtools-mcp) disabled — the /cdp ' + - 'tunnel requires bearer auth, which is not yet supported for the ' + - 'auto-registered browser MCP server.\n', - ); - return undefined; - } - const port = Number(process.env['QWEN_SERVE_CDP_TUNNEL_PORT']); - if (!Number.isInteger(port) || port <= 0) { - // Don't disable the browser tools silently — the most common cause is an - // ephemeral `--port 0` daemon, whose real port is bound too late to thread - // into this child's (frozen) env. Tell the operator how to enable them. - process.stderr.write( - 'qwen serve: browser tools (chrome-devtools-mcp) disabled — no valid ' + - `/cdp tunnel port (QWEN_SERVE_CDP_TUNNEL_PORT=${ - process.env['QWEN_SERVE_CDP_TUNNEL_PORT'] ?? '' - }). Start the daemon with a fixed --port (ephemeral --port 0 is not ` + - 'supported for the CDP tunnel).\n', - ); - return undefined; - } - try { - const requireFromHere = createRequire(import.meta.url); - const pkgJsonPath = requireFromHere.resolve( - 'chrome-devtools-mcp/package.json', - ); - const pkg = requireFromHere('chrome-devtools-mcp/package.json') as { - bin?: string | Record; - }; - const binRel = - typeof pkg.bin === 'string' ? pkg.bin : Object.values(pkg.bin ?? {})[0]; - if (!binRel) return undefined; - const pkgDir = path.dirname(pkgJsonPath); - const binPath = path.resolve(pkgDir, binRel); - // Containment: refuse to execute a bin path that escapes the package dir - // (a malformed/hostile `bin` field with `../` segments). - const binRelToPkg = path.relative(pkgDir, binPath); - if (binRelToPkg.startsWith('..') || path.isAbsolute(binRelToPkg)) { - return undefined; - } - return new MCPServerConfig(process.execPath, [ - binPath, - '--wsEndpoint', - `ws://127.0.0.1:${port}/cdp`, - ]); - } catch { - // chrome-devtools-mcp not installed in this build — skip auto-wiring. - return undefined; - } -} diff --git a/packages/cli/src/acp-integration/cdp-tunnel-mcp.test.ts b/packages/cli/src/acp-integration/cdp-tunnel-mcp.test.ts deleted file mode 100644 index f63ccde889..0000000000 --- a/packages/cli/src/acp-integration/cdp-tunnel-mcp.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -/** - * @license - * Copyright 2026 Qwen Team - * SPDX-License-Identifier: Apache-2.0 - */ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { buildCdpTunnelMcpServer } from './acpAgent.js'; - -describe('buildCdpTunnelMcpServer — CDP tunnel auto-wiring (#5626)', () => { - let saved: NodeJS.ProcessEnv; - beforeEach(() => { - saved = { ...process.env }; - }); - afterEach(() => { - process.env = saved; - }); - - it('returns undefined when the tunnel flag is off', () => { - delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; - process.env['QWEN_SERVE_CDP_TUNNEL_PORT'] = '4170'; - expect(buildCdpTunnelMcpServer()).toBeUndefined(); - }); - - it('returns undefined when the forwarded port is missing or invalid', () => { - process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] = '1'; - delete process.env['QWEN_SERVE_CDP_TUNNEL_PORT']; - expect(buildCdpTunnelMcpServer()).toBeUndefined(); - process.env['QWEN_SERVE_CDP_TUNNEL_PORT'] = '0'; - expect(buildCdpTunnelMcpServer()).toBeUndefined(); - }); - - it('builds a stdio chrome-devtools-mcp server aimed at the daemon /cdp', () => { - process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] = '1'; - process.env['QWEN_SERVE_CDP_TUNNEL_PORT'] = '4170'; - const server = buildCdpTunnelMcpServer(); - expect(server).toBeDefined(); - expect(server?.command).toBe(process.execPath); - expect(server?.args?.[0]).toMatch(/chrome-devtools-mcp/); - expect(server?.args).toContain('--wsEndpoint'); - expect(server?.args).toContain('ws://127.0.0.1:4170/cdp'); - // trust unset → tools default to 'ask' (no silent auto-approval of - // browser-driving tools), same as any project MCP server. - expect(server?.trust).toBeUndefined(); - }); -}); diff --git a/packages/cli/src/serve/acp-http/index.ts b/packages/cli/src/serve/acp-http/index.ts index 74c4f596ab..708cf2e54d 100644 --- a/packages/cli/src/serve/acp-http/index.ts +++ b/packages/cli/src/serve/acp-http/index.ts @@ -6,10 +6,13 @@ import { createHash, timingSafeEqual } from 'node:crypto'; import type { IncomingMessage } from 'node:http'; +import { createRequire } from 'node:module'; +import * as path from 'node:path'; import type { Duplex } from 'node:stream'; import type { Application, Request, Response } from 'express'; import { WebSocketServer, type WebSocket } from 'ws'; import type { HttpAcpBridge } from '@qwen-code/acp-bridge/bridgeTypes'; +import { RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG } from '@qwen-code/qwen-code-core'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import type { WorkspaceFileSystemFactory } from '../fs/index.js'; @@ -61,6 +64,92 @@ const CDP_PATH = '/cdp'; * packages can't share a module). */ const CDP_BRIDGE_CLIENT_NAME = 'qwen-cdp-bridge'; +const CHROME_DEVTOOLS_MCP_SERVER_NAME = 'chrome-devtools'; +const RUNTIME_MCP_RETRY_DELAY_MS = 250; +const RUNTIME_MCP_RETRY_ATTEMPTS = 20; +const requireFromHere = createRequire(import.meta.url); + +function formatCdpEndpointHost(hostname: string | undefined): string { + const host = hostname?.trim() || '127.0.0.1'; + if (host === '0.0.0.0' || host === '::' || host === '[::]') { + return '127.0.0.1'; + } + const unbracketed = + host.startsWith('[') && host.endsWith(']') ? host.slice(1, -1) : host; + return unbracketed.includes(':') ? `[${unbracketed}]` : unbracketed; +} + +function isAcpChannelUnavailable(err: unknown): boolean { + return ( + (err as { data?: { errorKind?: unknown } } | undefined)?.data?.errorKind === + 'acp_channel_unavailable' + ); +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +export function buildChromeDevToolsMcpRuntimeConfigFromPackage( + localPort: number | undefined, + pkgJsonPath: string, + pkgBin: string | Record | undefined, + hostname?: string, +): Record | undefined { + if ( + localPort === undefined || + !Number.isInteger(localPort) || + localPort <= 0 + ) { + return undefined; + } + const binRel = + typeof pkgBin === 'string' ? pkgBin : Object.values(pkgBin ?? {})[0]; + if (!binRel) return undefined; + const pkgDir = path.dirname(pkgJsonPath); + const binPath = path.resolve(pkgDir, binRel); + const binRelToPkg = path.relative(pkgDir, binPath); + if (binRelToPkg.startsWith('..') || path.isAbsolute(binRelToPkg)) { + return undefined; + } + return { + command: process.execPath, + args: [ + binPath, + '--wsEndpoint', + `ws://${formatCdpEndpointHost(hostname)}:${localPort}/cdp`, + ], + alwaysLoadTools: true, + [RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG]: true, + }; +} + +function buildChromeDevToolsMcpRuntimeConfig( + localPort: number | undefined, + hostname: string | undefined, +): Record | undefined { + try { + const pkgJsonPath = requireFromHere.resolve( + 'chrome-devtools-mcp/package.json', + ); + const pkg = requireFromHere('chrome-devtools-mcp/package.json') as { + bin?: string | Record; + }; + return buildChromeDevToolsMcpRuntimeConfigFromPackage( + localPort, + pkgJsonPath, + pkg.bin, + hostname, + ); + } catch (err) { + writeStderrLine( + `qwen serve: chrome-devtools-mcp package not resolvable: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + return undefined; + } +} /** * Browsers cannot set an `Authorization` header on a WebSocket, so the Web @@ -192,6 +281,8 @@ export interface MountAcpHttpOptions { * tool channel. Mirrors the REST CORS allowlist (`allowOriginCors`). */ allowedOrigins?: ParsedAllowOriginPatterns; + /** Hostname the daemon is listening on; used by local MCP child processes. */ + hostname?: string; /** Effective direct session shell policy for ACP initialize/dispatch. */ sessionShellCommandEnabled?: boolean; archiveCoordinator?: SessionArchiveCoordinator; @@ -317,6 +408,127 @@ export function mountAcpHttp( }, opts.maxConnections, ); + let cdpMcpRegistered = false; + let cdpMcpRegistering: Promise | undefined; + let cdpMcpTerminalSkipLogged = false; + + async function removeChromeDevToolsMcp( + originatorClientId: string, + ): Promise { + if (!cdpMcpRegistered) return; + cdpMcpRegistered = false; + try { + await bridge.removeRuntimeMcpServer( + CHROME_DEVTOOLS_MCP_SERVER_NAME, + originatorClientId, + ); + } catch (err) { + writeStderrLine( + `qwen serve: failed to remove ${CHROME_DEVTOOLS_MCP_SERVER_NAME} runtime MCP: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + } + + function removeChromeDevToolsMcpIfUnused(originatorClientId: string): void { + void (async () => { + if (opts.cdpTunnelRegistry?.hasActive()) return; + if (cdpMcpRegistering) await cdpMcpRegistering; + if (opts.cdpTunnelRegistry?.hasActive()) return; + await removeChromeDevToolsMcp(originatorClientId); + })().catch((err) => { + writeStderrLine( + `qwen serve: failed to clean up ${CHROME_DEVTOOLS_MCP_SERVER_NAME} runtime MCP: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + }); + } + + function ensureChromeDevToolsMcpRegistered( + localPort: number | undefined, + originatorClientId: string, + ): void { + if (opts.token) { + if (!cdpMcpTerminalSkipLogged) { + writeStderrLine( + `qwen serve: ${CHROME_DEVTOOLS_MCP_SERVER_NAME} runtime MCP skipped because /cdp requires bearer auth`, + ); + cdpMcpTerminalSkipLogged = true; + } + return; + } + if (cdpMcpRegistered || cdpMcpRegistering || cdpMcpTerminalSkipLogged) { + return; + } + const runtimeConfig = buildChromeDevToolsMcpRuntimeConfig( + localPort, + opts.hostname, + ); + if (!runtimeConfig) { + cdpMcpTerminalSkipLogged = true; + return; + } + cdpMcpRegistering = (async () => { + try { + let result: Awaited>; + for (let attempt = 0; ; attempt++) { + try { + result = await bridge.addRuntimeMcpServer( + CHROME_DEVTOOLS_MCP_SERVER_NAME, + runtimeConfig, + originatorClientId, + ); + break; + } catch (err) { + if ( + !isAcpChannelUnavailable(err) || + attempt >= RUNTIME_MCP_RETRY_ATTEMPTS || + !opts.cdpTunnelRegistry?.hasActive() + ) { + throw err; + } + await delay(RUNTIME_MCP_RETRY_DELAY_MS); + } + } + if ((result as { skipped?: boolean }).skipped) { + writeStderrLine( + `qwen serve: ${CHROME_DEVTOOLS_MCP_SERVER_NAME} runtime MCP skipped: ${ + (result as { reason?: string }).reason ?? 'unknown' + }`, + ); + return; + } + if ((result as { shadowedSettings?: boolean }).shadowedSettings) { + await bridge + .removeRuntimeMcpServer( + CHROME_DEVTOOLS_MCP_SERVER_NAME, + originatorClientId, + ) + .catch(() => {}); + cdpMcpTerminalSkipLogged = true; + writeStderrLine( + `qwen serve: ${CHROME_DEVTOOLS_MCP_SERVER_NAME} runtime MCP skipped because settings already define it`, + ); + return; + } + cdpMcpRegistered = true; + if (!opts.cdpTunnelRegistry?.hasActive()) { + await removeChromeDevToolsMcp(originatorClientId); + } + } catch (err) { + writeStderrLine( + `qwen serve: failed to add ${CHROME_DEVTOOLS_MCP_SERVER_NAME} runtime MCP: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } finally { + cdpMcpRegistering = undefined; + } + })(); + } + const dispatcher = new AcpDispatcher( bridge, opts.boundWorkspace, @@ -705,6 +917,7 @@ export function mountAcpHttp( const rawAddr = (socket as unknown as { remoteAddress?: string }).remoteAddress ?? 'ws-unknown'; + const localPort = (socket as { localPort?: number }).localPort; const logReject = (reason: string) => { writeStderrLine( `qwen serve: WebSocket upgrade rejected (${reason}) from ${rawAddr}`, @@ -747,7 +960,6 @@ export function mountAcpHttp( // the REST middleware does; extract port from the socket. if (fromLoopback) { const host = (req.headers['host'] ?? '').toLowerCase(); - const localPort = (socket as { localPort?: number }).localPort; const allowed = new Set([ `localhost:${localPort}`, `127.0.0.1:${localPort}`, @@ -768,7 +980,6 @@ export function mountAcpHttp( const origin = req.headers['origin']; if (origin) { try { - const localPort = (socket as { localPort?: number }).localPort; const isLoopbackOrigin = isSameLoopbackOrigin(origin, localPort); // `--allow-origin` allowlist (same match semantics as the REST // `allowOriginCors`): lets an explicitly permitted non-loopback @@ -882,6 +1093,9 @@ export function mountAcpHttp( if (cdpBridgeUnregister) { cdpBridgeUnregister(); cdpBridgeUnregister = undefined; + removeChromeDevToolsMcpIfUnused( + connRef?.connectionId ?? 'cdp-bridge', + ); } }); @@ -1218,6 +1432,7 @@ export function mountAcpHttp( }; cdpBridgeUnregister = opts.cdpTunnelRegistry.register(cdpEndpoint); + ensureChromeDevToolsMcpRegistered(localPort, conn.connectionId); writeStderrLine( `qwen serve: /acp connection ${conn.connectionId.slice(0, 8)} registered as CDP bridge`, ); diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index ad9d810412..11a32749b2 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -27,7 +27,11 @@ import { SessionShellClientRequiredError, SessionShellDisabledError, } from '@qwen-code/acp-bridge/bridgeErrors'; -import { SessionService, Storage } from '@qwen-code/qwen-code-core'; +import { + RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG, + SessionService, + Storage, +} from '@qwen-code/qwen-code-core'; import { resetHomeEnvBootstrapForTesting, SettingScope, @@ -49,7 +53,12 @@ import { WorkspaceSettingsPartialPersistError, type DaemonWorkspaceService, } from '../workspace-service/types.js'; -import { mountAcpHttp, type AcpHttpHandle } from './index.js'; +import { + buildChromeDevToolsMcpRuntimeConfigFromPackage, + type AcpHttpHandle, + mountAcpHttp, +} from './index.js'; +import { CdpTunnelRegistry } from '../cdp-tunnel/cdp-tunnel-registry.js'; import { mountWorkspaceMemoryRememberRoutes, WorkspaceRememberTaskLane, @@ -81,6 +90,70 @@ vi.mock('../../services/setup-github.js', async () => { }; }); +describe('buildChromeDevToolsMcpRuntimeConfigFromPackage', () => { + const pkgJsonPath = path.join('/tmp/chrome-devtools-mcp', 'package.json'); + + it.each([undefined, 0, -1, 1.5] as const)( + 'rejects invalid localPort=%s', + (localPort) => { + expect( + buildChromeDevToolsMcpRuntimeConfigFromPackage( + localPort, + pkgJsonPath, + 'bin/cli.js', + ), + ).toBeUndefined(); + }, + ); + + it('rejects missing and escaping bin paths', () => { + expect( + buildChromeDevToolsMcpRuntimeConfigFromPackage(4170, pkgJsonPath, {}), + ).toBeUndefined(); + expect( + buildChromeDevToolsMcpRuntimeConfigFromPackage( + 4170, + pkgJsonPath, + '../evil.js', + ), + ).toBeUndefined(); + }); + + it('builds the stdio config for a package-local bin', () => { + expect( + buildChromeDevToolsMcpRuntimeConfigFromPackage(4170, pkgJsonPath, { + cli: 'bin/cli.js', + }), + ).toEqual({ + command: process.execPath, + args: [ + path.join('/tmp/chrome-devtools-mcp', 'bin/cli.js'), + '--wsEndpoint', + 'ws://127.0.0.1:4170/cdp', + ], + alwaysLoadTools: true, + [RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG]: true, + }); + }); + + it('uses the listening host for a specific non-wildcard bind', () => { + expect( + buildChromeDevToolsMcpRuntimeConfigFromPackage( + 4170, + pkgJsonPath, + { cli: 'bin/cli.js' }, + '192.168.1.20', + ), + ).toMatchObject({ + args: [ + path.join('/tmp/chrome-devtools-mcp', 'bin/cli.js'), + '--wsEndpoint', + 'ws://192.168.1.20:4170/cdp', + ], + }); + }); +}); + /** * End-to-end transport test: boots a real Express server with the ACP * Streamable-HTTP transport mounted over a *fake* bridge, then drives it @@ -361,22 +434,44 @@ class FakeBridge { async getWorkspaceMcpResourcesStatus(serverName: string) { return { v: 1, serverName, resources: [] }; } - async addRuntimeMcpServer(name: string) { + runtimeMcpAdds: Array<{ + name: string; + config: Record; + originatorClientId: string; + }> = []; + runtimeMcpRemoves: Array<{ name: string; originatorClientId: string }> = []; + runtimeMcpAddResult: { + shadowedSettings?: boolean; + skipped?: boolean; + reason?: string; + } = {}; + runtimeMcpAddError: Error | undefined; + runtimeMcpBeforeAddResolve: (() => Promise) | undefined; + async addRuntimeMcpServer( + name: string, + config: Record, + originatorClientId: string, + ) { + this.runtimeMcpAdds.push({ name, config, originatorClientId }); + if (this.runtimeMcpAddError) throw this.runtimeMcpAddError; + await this.runtimeMcpBeforeAddResolve?.(); return { name, transport: 'stdio', replaced: false, shadowedSettings: false, toolCount: 0, - originatorClientId: 'c', + originatorClientId, + ...this.runtimeMcpAddResult, }; } - async removeRuntimeMcpServer(name: string) { + async removeRuntimeMcpServer(name: string, originatorClientId: string) { + this.runtimeMcpRemoves.push({ name, originatorClientId }); return { name, removed: true, wasShadowingSettings: false, - originatorClientId: 'c', + originatorClientId, }; } async runWorkspaceMemoryRemember() { @@ -6378,6 +6473,7 @@ describe('ACP WebSocket transport security', () => { token?: string; allowedOrigins?: { allowAny: boolean; origins: Set }; checkRate?: (key: string, tier: string) => boolean; + cdpTunnelOverWs?: boolean; } = {}, ) { return new Promise((resolve) => { @@ -6395,6 +6491,12 @@ describe('ACP WebSocket transport security', () => { ), // eslint-disable-next-line @typescript-eslint/no-explicit-any checkRate: opts.checkRate as any, + ...(opts.cdpTunnelOverWs + ? { + cdpTunnelOverWs: true, + cdpTunnelRegistry: new CdpTunnelRegistry(), + } + : {}), }); const listeningServer = app.listen(0, '127.0.0.1', () => { port = (listeningServer.address() as AddressInfo).port; @@ -6452,6 +6554,17 @@ describe('ACP WebSocket transport security', () => { }); } + function initializeCdpBridge(ws: WebSocket, id = 1): Promise { + return sendRpc(ws, { + jsonrpc: '2.0', + id, + method: 'initialize', + params: { + clientInfo: { name: 'qwen-cdp-bridge', version: '1.0.0' }, + }, + }); + } + // ── Host allowlist ────────────────────────────────────────────────── it('accepts WS upgrade with loopback Host header', async () => { await startServer(); @@ -6494,6 +6607,222 @@ describe('ACP WebSocket transport security', () => { expect(result.code).toBe(101); }); + it('dynamically registers chrome-devtools MCP for an active CDP bridge', async () => { + await startServer({ cdpTunnelOverWs: true }); + const ws = await wsConnect(); + await initializeCdpBridge(ws); + + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(1)); + expect(bridge.runtimeMcpAdds[0]).toMatchObject({ + name: 'chrome-devtools', + originatorClientId: expect.any(String), + }); + expect(bridge.runtimeMcpAdds[0]?.config).toMatchObject({ + command: process.execPath, + args: expect.arrayContaining([ + expect.stringContaining('chrome-devtools-mcp'), + '--wsEndpoint', + `ws://127.0.0.1:${port}/cdp`, + ]), + }); + + ws.close(); + await new Promise((resolve) => ws.once('close', () => resolve())); + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + expect(bridge.runtimeMcpRemoves[0]).toMatchObject({ + name: 'chrome-devtools', + originatorClientId: bridge.runtimeMcpAdds[0]?.originatorClientId, + }); + }); + + it('keeps chrome-devtools MCP registered while a replacement CDP bridge is active', async () => { + await startServer({ cdpTunnelOverWs: true }); + const first = await wsConnect(); + await initializeCdpBridge(first, 1); + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(1)); + + const second = await wsConnect(); + await initializeCdpBridge(second, 2); + + first.close(); + await new Promise((resolve) => first.once('close', () => resolve())); + await new Promise((resolve) => setTimeout(resolve, 30)); + expect(bridge.runtimeMcpRemoves).toHaveLength(0); + + second.close(); + await new Promise((resolve) => second.once('close', () => resolve())); + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + expect(bridge.runtimeMcpRemoves[0]).toMatchObject({ + name: 'chrome-devtools', + originatorClientId: expect.any(String), + }); + }); + + it('removes chrome-devtools MCP when settings already define it', async () => { + stdioMocks.writeStderrLine.mockClear(); + await startServer({ cdpTunnelOverWs: true }); + bridge.runtimeMcpAddResult = { shadowedSettings: true }; + const ws = await wsConnect(); + await initializeCdpBridge(ws); + + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + expect(bridge.runtimeMcpRemoves[0]).toMatchObject({ + name: 'chrome-devtools', + originatorClientId: bridge.runtimeMcpAdds[0]?.originatorClientId, + }); + expect(stdioMocks.writeStderrLine).toHaveBeenCalledWith( + 'qwen serve: chrome-devtools runtime MCP skipped because settings already define it', + ); + ws.close(); + await new Promise((resolve) => ws.once('close', () => resolve())); + }); + + it('retries chrome-devtools MCP registration after a skipped result', async () => { + stdioMocks.writeStderrLine.mockClear(); + await startServer({ cdpTunnelOverWs: true }); + bridge.runtimeMcpAddResult = { + skipped: true, + reason: 'budget_exceeded', + }; + + const first = await wsConnect(); + await initializeCdpBridge(first, 1); + + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(1)); + expect(bridge.runtimeMcpRemoves).toHaveLength(0); + expect(stdioMocks.writeStderrLine).toHaveBeenCalledWith( + 'qwen serve: chrome-devtools runtime MCP skipped: budget_exceeded', + ); + + bridge.runtimeMcpAddResult = {}; + const second = await wsConnect(); + await initializeCdpBridge(second, 2); + + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(2)); + second.close(); + await new Promise((resolve) => second.once('close', () => resolve())); + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + first.close(); + await new Promise((resolve) => first.once('close', () => resolve())); + }); + + it('removes chrome-devtools MCP if the CDP bridge disconnects during registration', async () => { + await startServer({ cdpTunnelOverWs: true }); + let releaseAdd: (() => void) | undefined; + bridge.runtimeMcpBeforeAddResolve = () => + new Promise((resolve) => { + releaseAdd = resolve; + }); + + const ws = await wsConnect(); + await initializeCdpBridge(ws); + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(1)); + ws.close(); + await new Promise((resolve) => ws.once('close', () => resolve())); + releaseAdd?.(); + + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + expect(bridge.runtimeMcpRemoves[0]).toMatchObject({ + name: 'chrome-devtools', + originatorClientId: bridge.runtimeMcpAdds[0]?.originatorClientId, + }); + }); + + it('retries chrome-devtools MCP registration after add failure', async () => { + stdioMocks.writeStderrLine.mockClear(); + await startServer({ cdpTunnelOverWs: true }); + bridge.runtimeMcpAddError = new Error('add failed'); + const first = await wsConnect(); + await initializeCdpBridge(first, 1); + + await vi.waitFor(() => { + expect(stdioMocks.writeStderrLine).toHaveBeenCalledWith( + 'qwen serve: failed to add chrome-devtools runtime MCP: add failed', + ); + }); + + bridge.runtimeMcpAddError = undefined; + const second = await wsConnect(); + await initializeCdpBridge(second, 2); + + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(2)); + second.close(); + await new Promise((resolve) => second.once('close', () => resolve())); + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + first.close(); + await new Promise((resolve) => first.once('close', () => resolve())); + }); + + it('retries chrome-devtools MCP registration while the ACP channel is unavailable', async () => { + stdioMocks.writeStderrLine.mockClear(); + await startServer({ cdpTunnelOverWs: true }); + bridge.runtimeMcpAddError = Object.assign(new Error('no channel'), { + data: { errorKind: 'acp_channel_unavailable' }, + }); + + const ws = await wsConnect(); + await initializeCdpBridge(ws); + + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(2), { + timeout: 1_500, + }); + bridge.runtimeMcpAddError = undefined; + + await vi.waitFor(() => expect(bridge.runtimeMcpAdds).toHaveLength(3), { + timeout: 1_500, + }); + expect(stdioMocks.writeStderrLine).not.toHaveBeenCalledWith( + expect.stringContaining('failed to add chrome-devtools runtime MCP'), + ); + + ws.close(); + await new Promise((resolve) => ws.once('close', () => resolve())); + await vi.waitFor(() => expect(bridge.runtimeMcpRemoves).toHaveLength(1)); + }); + + it('stops retrying chrome-devtools MCP registration after ACP channel retry exhaustion', async () => { + stdioMocks.writeStderrLine.mockClear(); + await startServer({ cdpTunnelOverWs: true }); + bridge.runtimeMcpAddError = Object.assign(new Error('no channel'), { + data: { errorKind: 'acp_channel_unavailable' }, + }); + + const ws = await wsConnect(); + await initializeCdpBridge(ws); + + await vi.waitFor( + () => { + expect(stdioMocks.writeStderrLine).toHaveBeenCalledWith( + 'qwen serve: failed to add chrome-devtools runtime MCP: no channel', + ); + }, + { timeout: 7_000 }, + ); + expect(bridge.runtimeMcpAdds).toHaveLength(21); + + ws.close(); + await new Promise((resolve) => ws.once('close', () => resolve())); + }, 10_000); + + it('skips chrome-devtools MCP registration when /cdp requires auth', async () => { + stdioMocks.writeStderrLine.mockClear(); + await startServer({ + cdpTunnelOverWs: true, + token: 'secret-token-123', + }); + const ws = await wsConnect({ + headers: { Authorization: 'Bearer secret-token-123' }, + }); + await initializeCdpBridge(ws); + + expect(bridge.runtimeMcpAdds).toHaveLength(0); + expect(stdioMocks.writeStderrLine).toHaveBeenCalledWith( + 'qwen serve: chrome-devtools runtime MCP skipped because /cdp requires bearer auth', + ); + ws.close(); + await new Promise((resolve) => ws.once('close', () => resolve())); + }); + it('rejects WS upgrade with a loopback Origin header on a different port', async () => { await startServer(); const result = await wsConnectRaw('127.0.0.1', 'http://localhost:3000'); diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index 0531d256d2..620a7d15ab 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -259,14 +259,15 @@ export const SERVE_CAPABILITY_REGISTRY = { // reusing the SDK-MCP-server control-plane pattern. Inbound WS frame types: // `mcp_register` { server }, `mcp_message` { id, server, payload } // (bidirectional, request/response correlated by `id`), `mcp_unregister` - // { server }. Advertised CONDITIONALLY — only when the operator opts in - // (the public contract is still settling per #5626), so clients pre-flight - // this tag before attempting to register a client-hosted server. + // { server }. Advertised CONDITIONALLY so clients pre-flight this tag before + // attempting to register a client-hosted server. `runQwenServe` enables it + // only when explicitly requested by option or env. client_mcp_over_ws: { since: 'v1' }, // Plan C "CDP tunnel" (issue #5626): the daemon exposes a `/cdp` WebSocket // where a loopback puppeteer client (chrome-devtools-mcp) drives ONE real tab // via the extension's `chrome.debugger`, tunneled over `/acp` as `cdp_*` - // frames. Advertised only when the operator opts in (contract still settling). + // frames. Advertised when explicitly enabled or when the daemon is serving a + // Chrome extension origin. cdp_tunnel_over_ws: { since: 'v1' }, // Daemon hosts the `/voice/stream` WebSocket: the browser captures audio and // streams raw PCM, the daemon transcribes server-side via the configured @@ -300,14 +301,12 @@ export interface AdvertiseFeatureToggles { reloadAvailable?: boolean; /** * Whether the daemon will accept client-hosted MCP servers over the WS - * (`client_mcp_over_ws`, issue #5626). Opt-in: the contract is still - * settling, so the tag is advertised only when explicitly enabled. + * (`client_mcp_over_ws`, issue #5626). */ clientMcpOverWsEnabled?: boolean; /** * Whether the daemon exposes the Plan C `/cdp` tunnel endpoint - * (`cdp_tunnel_over_ws`, issue #5626). Opt-in: the contract is still - * settling, so the tag is advertised only when explicitly enabled. + * (`cdp_tunnel_over_ws`, issue #5626). */ cdpTunnelOverWsEnabled?: boolean; voiceWsAvailable?: boolean; diff --git a/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-mcp-smoke.mjs b/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-mcp-smoke.mjs index 4821b3a73d..80d599c333 100644 --- a/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-mcp-smoke.mjs +++ b/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-mcp-smoke.mjs @@ -56,10 +56,25 @@ try { send({ jsonrpc: '2.0', id: 2, method: 'tools/list', params: {} }); const tl = await wait(2); + if (tl.error) { + throw new Error(`tools/list failed: ${JSON.stringify(tl.error)}`); + } out.tools = (tl.result?.tools || []).length; send({ jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'list_pages', arguments: {} } }); const lp = await wait(3); + if (lp.error) { + throw new Error(`list_pages failed: ${JSON.stringify(lp.error)}`); + } + if (lp.result?.isError === true) { + const text = (lp.result.content || []) + .map((part) => part?.text) + .filter(Boolean) + .join('\n'); + throw new Error( + `list_pages returned an MCP error: ${text || 'unknown error'}`, + ); + } out.listPages = JSON.stringify(lp.result ?? lp.error).slice(0, 240); } catch (e) { out.error = e.message; diff --git a/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-tunnel-acceptance.mjs b/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-tunnel-acceptance.mjs index b7d7c86be5..aac24caec9 100644 --- a/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-tunnel-acceptance.mjs +++ b/packages/cli/src/serve/cdp-tunnel/acceptance/cdp-tunnel-acceptance.mjs @@ -19,20 +19,16 @@ */ import { spawn } from 'node:child_process'; -import { createRequire } from 'node:module'; import { fileURLToPath } from 'node:url'; import { dirname, resolve } from 'node:path'; import { mkdtempSync } from 'node:fs'; import { tmpdir } from 'node:os'; -const SPIKE_DIR = '/tmp/planc-spike'; -const spikeRequire = createRequire(`${SPIKE_DIR}/package.json`); -const puppeteer = spikeRequire('puppeteer-core'); -const { WebSocket } = spikeRequire('ws'); - const __dirname = dirname(fileURLToPath(import.meta.url)); // repo root = .../packages/cli/src/serve/cdp-tunnel/acceptance -> up 6 const REPO_ROOT = resolve(__dirname, '../../../../../..'); +const { default: puppeteer } = await import('puppeteer-core'); +const { WebSocket } = await import('ws'); const HOST = '127.0.0.1'; const PORT = 9710; diff --git a/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.test.ts b/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.test.ts index 3796220255..eb327ef9e6 100644 --- a/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.test.ts +++ b/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.test.ts @@ -17,16 +17,28 @@ function setup() { return { link, sent }; } +async function markAttached( + link: CdpReverseLink, + sent: CdpOutboundFrame[], +): Promise { + const p = link.attach(); + const attachId = (sent.at(-1) as { id: number }).id; + link.handleInbound({ type: 'cdp_attached', id: attachId }); + await p; +} + describe('CdpReverseLink (Plan C #5626)', () => { it('forwardToTab sends a cdp_command and resolves on the matching cdp_result', async () => { const { link, sent } = setup(); + await markAttached(link, sent); const p = link.forwardToTab('Runtime.evaluate', { expression: '1+1' }); - expect(sent[0]).toMatchObject({ + await Promise.resolve(); + expect(sent.at(-1)).toMatchObject({ type: 'cdp_command', method: 'Runtime.evaluate', params: { expression: '1+1' }, }); - const id = (sent[0] as { id: number }).id; + const id = (sent.at(-1) as { id: number }).id; link.handleInbound({ type: 'cdp_result', id, @@ -38,8 +50,10 @@ describe('CdpReverseLink (Plan C #5626)', () => { it('rejects forwardToTab on a cdp_result error', async () => { const { link, sent } = setup(); + await markAttached(link, sent); const p = link.forwardToTab('Page.captureScreenshot', undefined); - const id = (sent[0] as { id: number }).id; + await Promise.resolve(); + const id = (sent.at(-1) as { id: number }).id; link.handleInbound({ type: 'cdp_result', id, @@ -88,6 +102,41 @@ describe('CdpReverseLink (Plan C #5626)', () => { expect(onDetach).toHaveBeenCalledWith('DevTools opened'); }); + it('rejects a pending attach on detach and ignores the stale attach ack', async () => { + const { link, sent } = setup(); + const attachP = link.attach(); + attachP.catch(() => undefined); + const attachId = (sent[0] as { id: number }).id; + + link.handleInbound({ type: 'cdp_detach', reason: 'tab went away' }); + await expect(attachP).rejects.toMatchObject({ message: 'tab went away' }); + + link.handleInbound({ type: 'cdp_attached', id: attachId }); + const cmdP = link.forwardToTab('Runtime.evaluate', undefined); + cmdP.catch(() => undefined); + await Promise.resolve(); + + expect(sent.filter((f) => f.type === 'cdp_attach')).toHaveLength(2); + expect(sent.some((f) => f.type === 'cdp_command')).toBe(false); + }); + + it('does not stay attached after an attached ack and detach in the same tick', async () => { + const { link, sent } = setup(); + const attachP = link.attach(); + const attachId = (sent[0] as { id: number }).id; + + link.handleInbound({ type: 'cdp_attached', id: attachId }); + link.handleInbound({ type: 'cdp_detach', reason: 'tab closed' }); + await attachP; + + const cmdP = link.forwardToTab('Runtime.evaluate', undefined); + cmdP.catch(() => undefined); + await Promise.resolve(); + + expect(sent.filter((f) => f.type === 'cdp_attach')).toHaveLength(2); + expect(sent.some((f) => f.type === 'cdp_command')).toBe(false); + }); + it('dispose rejects pending commands and refuses new ones', async () => { const { link } = setup(); const inflight = link.forwardToTab('Runtime.enable', undefined); @@ -104,7 +153,9 @@ describe('CdpReverseLink (Plan C #5626)', () => { const sent: CdpOutboundFrame[] = []; // Small per-command timeout so the timer fires under the fake clock. const link = new CdpReverseLink((f) => sent.push(f), 50); + await markAttached(link, sent); const p = link.forwardToTab('Page.navigate', { url: 'about:blank' }); + await Promise.resolve(); let err: unknown; p.catch((e) => { err = e; @@ -127,22 +178,24 @@ describe('CdpReverseLink (Plan C #5626)', () => { const sent: CdpOutboundFrame[] = []; const log = vi.fn(); const link = new CdpReverseLink((f) => sent.push(f), 50_000, log); + await markAttached(link, sent); const p = link.forwardToTab('Page.navigate', { url: 'about:blank' }); + await Promise.resolve(); p.catch(() => undefined); expect(log).toHaveBeenCalledWith( - 'qwen serve: /cdp forwarded command id=1 method=Page.navigate to extension', + 'qwen serve: /cdp forwarded command id=2 method=Page.navigate to extension', ); await vi.advanceTimersByTimeAsync(30_000); expect(log).toHaveBeenCalledWith( - 'qwen serve: /cdp still waiting for command id=1 method=Page.navigate after 30000ms', + 'qwen serve: /cdp still waiting for command id=2 method=Page.navigate after 30000ms', ); await vi.advanceTimersByTimeAsync(20_000); await expect(p).rejects.toMatchObject({ message: - 'CDP command id=1 method=Page.navigate timed out after 50000ms', + 'CDP command id=2 method=Page.navigate timed out after 50000ms', }); } finally { vi.useRealTimers(); @@ -194,20 +247,20 @@ describe('CdpReverseLink (Plan C #5626)', () => { } }); - it('opens the gate after an attach timeout so forwardToTab does not hang', async () => { + it('rejects a lazy command when attach times out', async () => { vi.useFakeTimers(); try { const sent: CdpOutboundFrame[] = []; const link = new CdpReverseLink((f) => sent.push(f), 50); - link.attach().catch(() => undefined); // times out below - // Command parked behind the (failing) attach gate. const cmdP = link.forwardToTab('Runtime.evaluate', undefined); cmdP.catch(() => undefined); + expect(sent[0]).toMatchObject({ type: 'cdp_attach' }); - // Attach timer fires → attach settles (failure) → gate opens → the parked - // command is sent rather than hanging forever. await vi.advanceTimersByTimeAsync(50); - expect(sent.some((f) => f.type === 'cdp_command')).toBe(true); + await expect(cmdP).rejects.toMatchObject({ + message: 'cdp_attach timed out', + }); + expect(sent.some((f) => f.type === 'cdp_command')).toBe(false); } finally { vi.useRealTimers(); } diff --git a/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.ts b/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.ts index 55f39581de..912dc4e923 100644 --- a/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.ts +++ b/packages/cli/src/serve/cdp-tunnel/cdp-reverse-link.ts @@ -149,16 +149,14 @@ export class CdpReverseLink { private nextId = 1; private readonly pending = new Map(); private disposed = false; + private attached = false; + private attachPromise: Promise<{ url?: string; title?: string }> | undefined; /** Resolver for the in-flight `cdp_attach` (if any). */ private pendingAttach: { id: number; pending: PendingCommand } | undefined; - /** - * Opens once the initial `cdp_attach` settles (success OR failure). - * `forwardToTab` awaits this so page-domain commands never race the - * extension's async `chrome.debugger.attach`. Undefined until {@link attach}. - */ - private attachGate: Promise | undefined; /** Called when the extension reports the tab detached. */ onDetach: ((reason: string) => void) | undefined; + /** Called when lazy `cdp_attach` fails before any page command can run. */ + onAttachFailure: ((reason: string) => void) | undefined; constructor( private readonly sendToExtension: CdpSendToExtension, @@ -183,12 +181,7 @@ export class CdpReverseLink { if (this.disposed) { throw { code: -32000, message: 'CDP tunnel closed' }; } - // Ordering gate: wait for the initial attach to settle before forwarding - // page-domain commands, so a fast `Network.enable` doesn't reach the - // extension before `chrome.debugger.attach` finishes and fail "not attached". - if (this.attachGate) { - await this.attachGate; - } + await this.attach(); return new Promise((resolve, reject) => { if (this.disposed) { reject({ code: -32000, message: 'CDP tunnel closed' }); @@ -225,7 +218,9 @@ export class CdpReverseLink { * once the extension acks `cdp_attached` (or rejects on error/timeout). */ attach(): Promise<{ url?: string; title?: string }> { - const result = new Promise<{ url?: string; title?: string }>( + if (this.attached) return Promise.resolve({}); + if (this.attachPromise) return this.attachPromise; + this.attachPromise = new Promise<{ url?: string; title?: string }>( (resolve, reject) => { if (this.disposed) { reject({ code: -32000, message: 'CDP tunnel closed' }); @@ -254,13 +249,28 @@ export class CdpReverseLink { }); } }, + ).then( + (info) => { + this.emulator?.setTabInfo(info); + return info; + }, + (err) => { + this.attached = false; + this.onAttachFailure?.( + err instanceof Error + ? err.message + : ((err as { message?: string } | undefined)?.message ?? + String(err)), + ); + throw err; + }, ); - // Open the gate once the attach settles (success or failure). - this.attachGate = result.then( - () => undefined, - () => undefined, - ); - return result; + void this.attachPromise + .finally(() => { + this.attachPromise = undefined; + }) + .catch(() => {}); + return this.attachPromise; } /** @@ -322,18 +332,26 @@ export class CdpReverseLink { this.pendingAttach = undefined; clearTimeout(attach.pending.timer); if (frame.error) { + this.attached = false; attach.pending.reject({ code: -32000, message: frame.error.message ?? 'cdp_attach failed', }); return; } + this.attached = true; attach.pending.resolve({ url: frame.url, title: frame.title }); } private handleDetach(frame: CdpDetachFrame): void { const reason = typeof frame.reason === 'string' ? frame.reason : 'tab detached'; + this.attached = false; + if (this.pendingAttach) { + clearTimeout(this.pendingAttach.pending.timer); + this.pendingAttach.pending.reject({ code: -32000, message: reason }); + this.pendingAttach = undefined; + } this.onDetach?.(reason); } @@ -409,6 +427,8 @@ export class CdpReverseLink { this.pendingAttach.pending.reject({ code: -32000, message: reason }); this.pendingAttach = undefined; } + this.attached = false; + this.attachPromise = undefined; this.emulator = undefined; } } diff --git a/packages/cli/src/serve/cdp-tunnel/cdp-ws.test.ts b/packages/cli/src/serve/cdp-tunnel/cdp-ws.test.ts index f810c96200..ef5e452047 100644 --- a/packages/cli/src/serve/cdp-tunnel/cdp-ws.test.ts +++ b/packages/cli/src/serve/cdp-tunnel/cdp-ws.test.ts @@ -96,13 +96,117 @@ describe('attachCdpClient (Plan C #5626)', () => { expect(bridge.cdpBound).toBe(true); // untouched }); - it('binds the bridge and kicks an attach', () => { + it('binds the bridge without attaching immediately', () => { const { bridge, sent } = makeBridge(); const ws = new FakeWs(); bind(ws, makeRegistry(bridge).registry); expect(bridge.cdpBound).toBe(true); expect(ws.closed).toBeNull(); - expect(sent.some((f) => f.type === 'cdp_attach')).toBe(true); + expect(sent.some((f) => f.type === 'cdp_attach')).toBe(false); + }); + + it('lazy-attaches before forwarding the first page command', async () => { + const { bridge, sent } = makeBridge(); + const ws = new FakeWs(); + bind(ws, makeRegistry(bridge).registry); + + ws.emit( + 'message', + JSON.stringify({ + id: 1, + method: 'Runtime.evaluate', + params: { expression: '1+1' }, + sessionId: 'qwen-cdp-page-session', + }), + ); + + await vi.waitFor(() => + expect(sent[0]).toMatchObject({ type: 'cdp_attach' }), + ); + expect(sent.some((f) => f.type === 'cdp_command')).toBe(false); + + const attachId = (sent[0] as { id: number }).id; + bridge.routeInbound({ + type: 'cdp_attached', + id: attachId, + url: 'https://example.com/', + title: 'Example', + }); + + await vi.waitFor(() => + expect(sent.some((f) => f.type === 'cdp_command')).toBe(true), + ); + const command = sent.find((f) => f.type === 'cdp_command') as + | { id: number } + | undefined; + expect(command).toMatchObject({ id: expect.any(Number) }); + bridge.routeInbound({ + type: 'cdp_result', + id: command?.id, + result: { result: { type: 'number', value: 2 } }, + }); + + await vi.waitFor(() => expect(ws.sent).toHaveLength(1)); + expect(JSON.parse(ws.sent[0] ?? '{}')).toMatchObject({ + id: 1, + sessionId: 'qwen-cdp-page-session', + result: { result: { value: 2 } }, + }); + }); + + it('closes and clears the binding when lazy attach fails', async () => { + const { bridge, sent } = makeBridge(); + const ws = new FakeWs(); + bind(ws, makeRegistry(bridge).registry); + + ws.emit( + 'message', + JSON.stringify({ + id: 1, + method: 'Runtime.evaluate', + sessionId: 'qwen-cdp-page-session', + }), + ); + + await vi.waitFor(() => + expect(sent[0]).toMatchObject({ type: 'cdp_attach' }), + ); + const attachId = (sent[0] as { id: number }).id; + bridge.routeInbound({ + type: 'cdp_attached', + id: attachId, + error: { message: 'Permission denied' }, + }); + + await vi.waitFor(() => expect(ws.closed?.code).toBe(1011)); + expect(ws.closed?.reason).toBe('cdp attach failed'); + expect(bridge.cdpBound).toBe(false); + expect(releases(sent)).toHaveLength(0); + }); + + it('does not release when detach rejects a pending lazy attach', async () => { + const { bridge, sent } = makeBridge(); + const ws = new FakeWs(); + bind(ws, makeRegistry(bridge).registry); + + ws.emit( + 'message', + JSON.stringify({ + id: 1, + method: 'Runtime.evaluate', + sessionId: 'qwen-cdp-page-session', + }), + ); + + await vi.waitFor(() => + expect(sent[0]).toMatchObject({ type: 'cdp_attach' }), + ); + bridge.routeInbound({ type: 'cdp_detach', reason: 'tab closed' }); + + expect(ws.closed?.code).toBe(1000); + expect(ws.closed?.reason).toBe('tab detached: tab closed'); + await vi.waitFor(() => expect(bridge.cdpBound).toBe(false)); + expect(releases(sent)).toHaveLength(0); }); it('onExtensionGone closes the puppeteer socket without sending a release', () => { diff --git a/packages/cli/src/serve/cdp-tunnel/cdp-ws.ts b/packages/cli/src/serve/cdp-tunnel/cdp-ws.ts index 366593031a..0031b8f9da 100644 --- a/packages/cli/src/serve/cdp-tunnel/cdp-ws.ts +++ b/packages/cli/src/serve/cdp-tunnel/cdp-ws.ts @@ -172,6 +172,16 @@ export function attachCdpClient( } }; + link.onAttachFailure = (reason: string) => { + log(`qwen serve: /cdp attach failed (${reason}); closing puppeteer socket`); + dispose(`cdp_attach failed: ${reason}`, false); + try { + ws.close(CLOSE_NO_BRIDGE, 'cdp attach failed'); + } catch { + // already closing + } + }; + ws.on('pong', () => { heartbeatAlive = true; }); @@ -218,31 +228,8 @@ export function attachCdpClient( dispose('puppeteer /cdp socket error'); }); - // Kick the extension to attach its active tab. Best-effort: the emulator - // serves browser-level topology regardless; page-domain forwards will fail - // cleanly if the attach never lands. Refresh tab metadata when it resolves. - void link - .attach() - .then((info) => emulator.setTabInfo(info)) - .catch((err) => { - log( - `qwen serve: /cdp attach failed: ${ - err && typeof err === 'object' && 'message' in err - ? String((err as { message?: unknown }).message) - : String(err) - }; closing puppeteer socket so a reconnect can retry attach`, - ); - // `bridge.cdpBound` was set before attach. Without closing here, the - // single-client guard above rejects every reconnect while page-domain - // commands keep failing "not attached" — a stuck tunnel until daemon - // restart. Closing triggers ws.on('close') -> dispose(), which clears - // cdpBound and routeInbound so the next /cdp client re-attempts attach. - try { - ws.close(CLOSE_NO_BRIDGE, 'cdp_attach failed'); - } catch { - // already closing - } - }); + // Lazy attach: tools can load over `/cdp` without immediately putting Chrome + // into debugging mode. The first page-domain command attaches the active tab. log('qwen serve: /cdp puppeteer client bound to extension bridge'); } diff --git a/packages/cli/src/serve/run-qwen-serve.test.ts b/packages/cli/src/serve/run-qwen-serve.test.ts index bce017a9f1..aa5919cac0 100644 --- a/packages/cli/src/serve/run-qwen-serve.test.ts +++ b/packages/cli/src/serve/run-qwen-serve.test.ts @@ -616,6 +616,64 @@ describe('runQwenServe runtime startup failures', () => { } }); + async function readBrowserMcpFeatureFlagsForEnv( + raw: string | undefined, + origin = 'chrome-extension://qwen-test-extension', + ) { + tmpDir = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'qws-runtime-fail-')), + ); + const originalClientMcpOverWs = + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + const originalCdpTunnelOverWs = + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + if (raw === undefined) { + delete process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + } else { + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS'] = raw; + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] = raw; + } + vi.spyOn(acpBridge, 'createAcpSessionBridge').mockImplementation(() => { + throw new Error('runtime boom'); + }); + + const handle = await runQwenServe( + { + port: 0, + hostname: '127.0.0.1', + mode: 'http-bridge', + workspace: tmpDir, + maxSessions: 1, + serveWebShell: false, + allowOrigins: [origin], + }, + { resolveOnListen: true }, + ); + + try { + await expect(handle.runtimeReady).rejects.toThrow('runtime boom'); + const capabilitiesRes = await fetch(`${handle.url}/capabilities`, { + headers: { Origin: origin }, + }); + expect(capabilitiesRes.status).toBe(200); + return ((await capabilitiesRes.json()) as { features: string[] }) + .features; + } finally { + if (originalClientMcpOverWs === undefined) { + delete process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + } else { + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS'] = originalClientMcpOverWs; + } + if (originalCdpTunnelOverWs === undefined) { + delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + } else { + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] = originalCdpTunnelOverWs; + } + await handle.close(); + } + } + it('rejects the embedded run handle by default when the runtime fails to mount', async () => { tmpDir = fs.realpathSync( fs.mkdtempSync(path.join(os.tmpdir(), 'qws-runtime-fail-')), @@ -663,6 +721,100 @@ describe('runQwenServe runtime startup failures', () => { ).rejects.toThrow(); }); + it.each([ + ['0', false], + ['false', false], + ['FALSE', false], + [' 0 ', false], + ['1', true], + ['true', true], + ['anything', true], + ] as const)( + 'normalizes browser MCP env flag %j', + async (raw, shouldEnable) => { + const features = await readBrowserMcpFeatureFlagsForEnv(raw); + + if (shouldEnable) { + expect(features).toEqual( + expect.arrayContaining(['client_mcp_over_ws', 'cdp_tunnel_over_ws']), + ); + } else { + expect(features).not.toContain('client_mcp_over_ws'); + expect(features).not.toContain('cdp_tunnel_over_ws'); + } + }, + ); + + it('auto-enables only the CDP tunnel for Chrome extension origins when the env flag is unset', async () => { + const features = await readBrowserMcpFeatureFlagsForEnv(undefined); + + expect(features).toContain('cdp_tunnel_over_ws'); + expect(features).not.toContain('client_mcp_over_ws'); + }); + + it('forwards auto-enabled CDP tunnel state to the ACP child env', async () => { + tmpDir = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'qws-runtime-child-env-')), + ); + const originalClientMcpOverWs = + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + const originalCdpTunnelOverWs = + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + delete process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + const bridge = makeRuntimeBridge(); + const createBridge = vi + .spyOn(acpBridge, 'createAcpSessionBridge') + .mockReturnValue( + bridge as ReturnType, + ); + + const handle = await runQwenServe( + { + port: 0, + hostname: '127.0.0.1', + mode: 'http-bridge', + workspace: tmpDir, + maxSessions: 1, + serveWebShell: false, + allowOrigins: ['chrome-extension://qwen-test-extension'], + }, + { resolveOnListen: true }, + ); + + try { + await handle.runtimeReady; + const bridgeOptions = createBridge.mock.calls[0]?.[0] as + | { childEnvOverrides?: Record } + | undefined; + expect(bridgeOptions?.childEnvOverrides).toMatchObject({ + QWEN_SERVE_CDP_TUNNEL_OVER_WS: '1', + }); + } finally { + if (originalClientMcpOverWs === undefined) { + delete process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + } else { + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS'] = originalClientMcpOverWs; + } + if (originalCdpTunnelOverWs === undefined) { + delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + } else { + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] = originalCdpTunnelOverWs; + } + await handle.close(); + } + }); + + it('keeps browser MCP features disabled for non-extension origins when the env flag is unset', async () => { + const features = await readBrowserMcpFeatureFlagsForEnv( + undefined, + 'https://example.com', + ); + + expect(features).not.toContain('client_mcp_over_ws'); + expect(features).not.toContain('cdp_tunnel_over_ws'); + }); + it('bounds shutdown waiting when runtime startup never settles', async () => { const daemonLog = { warn: vi.fn() }; @@ -1619,6 +1771,12 @@ describe('runQwenServe runtime startup failures', () => { tmpDir = fs.realpathSync( fs.mkdtempSync(path.join(os.tmpdir(), 'qws-runtime-fail-')), ); + const originalClientMcpOverWs = + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + const originalCdpTunnelOverWs = + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + delete process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; const boundWorkspace = canonicalizeWorkspace(tmpDir); vi.spyOn(acpBridge, 'createAcpSessionBridge').mockImplementation(() => { throw new Error('runtime boom'); @@ -1655,7 +1813,8 @@ describe('runQwenServe runtime startup failures', () => { headers: { Origin: handle.url }, }); expect(capabilitiesRes.status).toBe(200); - expect(await capabilitiesRes.json()).toMatchObject({ + const capabilitiesBody = await capabilitiesRes.json(); + expect(capabilitiesBody).toMatchObject({ v: 1, protocolVersions: { current: 'v1', supported: ['v1'] }, mode: 'http-bridge', @@ -1671,6 +1830,8 @@ describe('runQwenServe runtime startup failures', () => { policy: { permission: 'first-responder' }, limits: { maxPendingPromptsPerSession: 5 }, }); + expect(capabilitiesBody.features).not.toContain('client_mcp_over_ws'); + expect(capabilitiesBody.features).not.toContain('cdp_tunnel_over_ws'); const port = new URL(handle.url).port; for (const origin of [ @@ -1760,6 +1921,16 @@ describe('runQwenServe runtime startup failures', () => { }, }); } finally { + if (originalClientMcpOverWs === undefined) { + delete process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS']; + } else { + process.env['QWEN_SERVE_CLIENT_MCP_OVER_WS'] = originalClientMcpOverWs; + } + if (originalCdpTunnelOverWs === undefined) { + delete process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS']; + } else { + process.env['QWEN_SERVE_CDP_TUNNEL_OVER_WS'] = originalCdpTunnelOverWs; + } await handle.close(); } }); @@ -2789,6 +2960,7 @@ describe('runQwenServe startup observability', () => { workspace: tmpDir, maxSessions: 1, serveWebShell: false, + allowOrigins: ['chrome-extension://qwen-test-extension'], }, { bridge: makeFakeBridge() }, ); @@ -2804,6 +2976,9 @@ describe('runQwenServe startup observability', () => { expect(stderrWrites.join('')).toMatch( /qwen serve: startup timing: processToListenMs=\d+ runQwenServeToListenMs=\d+/, ); + expect(stderrWrites.join('')).not.toContain( + 'qwen serve: client-hosted MCP tools are accepted over the WebSocket without auth.', + ); expect(await readStartup(handle)).toMatchObject({ processStartedAt: expect.any(String), diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index 0f5057ad82..00a2ac716e 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -91,12 +91,9 @@ import { import type { ServiceInfo } from '../commands/channel/pidfile.js'; import { findCliEntryPath } from '../commands/channel/cli-entry-path.js'; -// Reverse tool channel opt-in (issue #5626, Phase 2). `=1` advertises the -// `client_mcp_over_ws` capability and accepts client-hosted MCP servers over -// the daemon WS. Off by default while the contract settles. +// Reverse MCP channel; enabled only by explicit option or env opt-in. const QWEN_SERVE_CLIENT_MCP_OVER_WS_ENV = 'QWEN_SERVE_CLIENT_MCP_OVER_WS'; -// CDP tunnel opt-in (Plan C, issue #5626). `=1` advertises `cdp_tunnel_over_ws` -// and exposes the `/cdp` WebSocket. Off by default while the contract settles. +// CDP tunnel; default-on for Chrome-extension origins or explicit env opt-in. const QWEN_SERVE_CDP_TUNNEL_OVER_WS_ENV = 'QWEN_SERVE_CDP_TUNNEL_OVER_WS'; const QWEN_SERVE_PROMPT_DEADLINE_MS_ENV = 'QWEN_SERVE_PROMPT_DEADLINE_MS'; const QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV = @@ -172,6 +169,20 @@ function parseDeadlineEnv( return parsed; } +function envFlagDisabled(raw: string | undefined): boolean { + if (raw === undefined) return false; + const normalized = raw.trim().toLowerCase(); + return normalized === '0' || normalized === 'false'; +} + +function hasChromeExtensionOrigin(origins: readonly string[] | undefined) { + return ( + origins?.some((origin) => + origin.trim().toLowerCase().startsWith('chrome-extension://'), + ) === true + ); +} + function createDaemonTelemetryRuntimeConfig( telemetry: TelemetrySettings, cliVersion: string, @@ -1288,24 +1299,24 @@ export async function runQwenServe( QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV, process.env[QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV], ); + const clientMcpOverWsEnv = process.env[QWEN_SERVE_CLIENT_MCP_OVER_WS_ENV]; + const cdpTunnelOverWsEnv = process.env[QWEN_SERVE_CDP_TUNNEL_OVER_WS_ENV]; + const chromeExtensionOriginAllowed = hasChromeExtensionOrigin( + optsIn.allowOrigins, + ); const opts: ServeOptions = { ...optsIn, token, promptDeadlineMs, writerIdleTimeoutMs, - // Reverse tool channel (issue #5626, Phase 2). Opt-in via env until the - // public contract settles — the WS `mcp_register` / `mcp_message` frames - // and the child↔parent `client_mcp/message` round-trip stay dormant - // otherwise. An explicit `clientMcpOverWs` in `optsIn` (embedded callers) - // still wins. clientMcpOverWs: optsIn.clientMcpOverWs ?? - process.env[QWEN_SERVE_CLIENT_MCP_OVER_WS_ENV] === '1', - // CDP tunnel (Plan C, issue #5626). Opt-in via env until the contract - // settles; an explicit `cdpTunnelOverWs` in `optsIn` still wins. + (!envFlagDisabled(clientMcpOverWsEnv) && + clientMcpOverWsEnv !== undefined), cdpTunnelOverWs: optsIn.cdpTunnelOverWs ?? - process.env[QWEN_SERVE_CDP_TUNNEL_OVER_WS_ENV] === '1', + (!envFlagDisabled(cdpTunnelOverWsEnv) && + (cdpTunnelOverWsEnv !== undefined || chromeExtensionOriginAllowed)), }; const channelRuntime = opts.channelSelection ? await loadChannelWorkerRuntime() @@ -1697,23 +1708,7 @@ export async function runQwenServe( ? String(opts.mcpClientBudget) : undefined, QWEN_SERVE_MCP_BUDGET_MODE: opts.mcpBudgetMode, - // CDP tunnel (Plan C, #5626): forward the flag + bound port so the spawned - // ACP child can auto-register chrome-devtools-mcp against this `/cdp` - // endpoint. Only meaningful with a fixed `--port`: the override map is frozen - // at bridge construction, so an ephemeral `--port 0` (resolved only after - // `listen`) can't be threaded here. Leave the port unset in that case so the - // child surfaces a clear diagnostic instead of a bogus port "0". QWEN_SERVE_CDP_TUNNEL_OVER_WS: opts.cdpTunnelOverWs ? '1' : undefined, - QWEN_SERVE_CDP_TUNNEL_PORT: - opts.cdpTunnelOverWs && opts.port > 0 ? String(opts.port) : undefined, - // Tell the child whether `/cdp` requires bearer auth. The ACP child can't - // inherit QWEN_SERVER_TOKEN (the spawn path scrubs it) and chrome-devtools- - // mcp is launched with `--wsEndpoint` only, so it can't authenticate against - // an auth-gated `/cdp`. The child uses this flag to skip auto-registering the - // browser tools (with a diagnostic) instead of registering tools that can't - // connect. See buildCdpTunnelMcpServer in acpAgent.ts. - QWEN_SERVE_CDP_TUNNEL_AUTH_REQUIRED: - opts.cdpTunnelOverWs && (token || opts.requireAuth) ? '1' : undefined, }; const cliVersionPromise = getCliVersion(); @@ -2356,6 +2351,12 @@ export async function runQwenServe( writeStderrLine( `qwen serve: bearer auth disabled (loopback default). Set ${QWEN_SERVER_TOKEN_ENV} to enable.`, ); + if (opts.clientMcpOverWs === true) { + writeStderrLine( + `qwen serve: client-hosted MCP tools are accepted over the WebSocket without auth. ` + + `Set ${QWEN_SERVE_CLIENT_MCP_OVER_WS_ENV}=0 to disable.`, + ); + } } else if (opts.requireAuth) { // The boot check above guarantees `token` is set whenever // `--require-auth` is on, so this branch only fires alongside @@ -2368,7 +2369,6 @@ export async function runQwenServe( 'on every route, including loopback /health).', ); } - let shuttingDown = false; let closePromise: Promise | undefined; let runtimeStartupTimer: NodeJS.Timeout | undefined; diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 54e51d2f22..cb3569cf71 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -512,7 +512,11 @@ interface FakeBridgeOpts { toolCount: number; originatorClientId: string; } - | { name: string; skipped: true; reason: 'budget_warning_only' } + | { + name: string; + skipped: true; + reason: 'budget_warning_only' | 'runtime_name_conflict'; + } >; removeRuntimeMcpServerImpl?: ( name: string, diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 525760de5f..ef3aef0af3 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -802,6 +802,7 @@ export function createServeApp( opts.allowOrigins && opts.allowOrigins.length > 0 ? parseAllowOriginPatterns(opts.allowOrigins) : undefined, + hostname: opts.hostname, sessionShellCommandEnabled, workspaceRememberLane, checkRate: rateLimiter?.checkRate, diff --git a/packages/cli/src/serve/types.ts b/packages/cli/src/serve/types.ts index 2b6be42882..1ebbaebe64 100644 --- a/packages/cli/src/serve/types.ts +++ b/packages/cli/src/serve/types.ts @@ -208,23 +208,21 @@ export interface ServeOptions { /** Rate limit window duration in ms (default 60000). Requires --rate-limit. */ rateLimitWindowMs?: number; /** - * Opt-in: accept client-hosted MCP servers over the daemon WS (issue #5626, + * Accept client-hosted MCP servers over the daemon WS (issue #5626, * Phase 2 "reverse tool channel"). When enabled, a connected WS client may * send `mcp_register` / `mcp_message` / `mcp_unregister` frames so the * daemon's agent can call tools that execute in the client (e.g. the Chrome - * extension's browser tools). Off by default — the public contract is still - * settling, so the `client_mcp_over_ws` capability tag and the WS frame - * handling stay gated behind explicit operator opt-in. + * extension's browser tools). `runQwenServe` only enables this when a caller + * or environment variable opts in. */ clientMcpOverWs?: boolean; /** - * Opt-in: tunnel raw CDP to a real browser tab over the reverse `/acp` WS + * Tunnel raw CDP to a real browser tab over the reverse `/acp` WS * (Plan C "CDP tunnel", issue #5626). When enabled, a loopback puppeteer * client (chrome-devtools-mcp) can connect to a new `/cdp` WebSocket and * drive ONE real tab via the extension's `chrome.debugger`, reusing the - * ready-made chrome-devtools-mcp toolset. Off by default — the public - * contract is still settling, so the `cdp_tunnel_over_ws` capability tag and - * the `/cdp` endpoint stay gated behind explicit operator opt-in. + * ready-made chrome-devtools-mcp toolset. `runQwenServe` enables this for + * Chrome extension origins or explicit env opt-in; callers may pass `false`. */ cdpTunnelOverWs?: boolean; /** Forward the experimental LSP opt-in to spawned ACP children. */ diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 3a5ef0c86e..b511bc2ddb 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -737,6 +737,7 @@ export class MCPServerConfig { * `new MCPServerConfig(...)` call sites. See issue #4615. */ readonly scope?: McpServerScope, + readonly alwaysLoadTools?: boolean, ) {} } diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index bb395b7e39..9539d4237c 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -11,7 +11,7 @@ import { } from './mcp-client-manager.js'; import { McpClient } from './mcp-client.js'; import type { ToolRegistry } from './tool-registry.js'; -import type { Config } from '../config/config.js'; +import { MCPServerConfig, type Config } from '../config/config.js'; import type { PromptRegistry } from '../prompts/prompt-registry.js'; import type { WorkspaceContext } from '../utils/workspaceContext.js'; import { connectionIdOf } from './mcp-pool-key.js'; @@ -3798,6 +3798,7 @@ describe('McpClientManager — addRuntimeMcpServer / removeRuntimeMcpServer (T2. function mkRuntimeConfig( opts: { settingsServers?: Record; + runtimeServers?: Record; runtimeAddSpy?: ReturnType; runtimeRemoveSpy?: ReturnType; } = {}, @@ -3815,6 +3816,7 @@ describe('McpClientManager — addRuntimeMcpServer / removeRuntimeMcpServer (T2. getSessionId: () => 'test-session-1', isMcpServerDisabled: () => false, getSettingsMcpServers: () => opts.settingsServers ?? {}, + getRuntimeMcpServers: () => opts.runtimeServers ?? {}, addRuntimeMcpServer: addSpy, removeRuntimeMcpServer: removeSpy, } as unknown as Config; @@ -4018,6 +4020,70 @@ describe('McpClientManager — addRuntimeMcpServer / removeRuntimeMcpServer (T2. }); }); + it('case 5b: ifAbsent skips before replacing a different runtime server', async () => { + const acquireSpy = vi.fn(); + const addSpy = vi.fn(); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + const config = mkRuntimeConfig({ + runtimeServers: { 'runtime-srv': new MCPServerConfig('old-cmd') }, + runtimeAddSpy: addSpy, + }); + const manager = mkManager({ config, options: { pool: fakePool } }); + + const result = await manager.addRuntimeMcpServer( + 'runtime-srv', + { + command: 'new-cmd', + __qwenRuntimeMcpIfAbsent: true, + } as unknown as MCPServerConfig, + 'client-5b', + ); + + expect(result).toEqual({ + name: 'runtime-srv', + skipped: true, + reason: 'runtime_name_conflict', + }); + expect(addSpy).not.toHaveBeenCalled(); + expect(acquireSpy).not.toHaveBeenCalled(); + }); + + it('case 5c: ifAbsent skips before reusing an unowned runtime server', async () => { + const acquireSpy = vi.fn(); + const addSpy = vi.fn(); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + const config = mkRuntimeConfig({ + runtimeServers: { 'runtime-srv': new MCPServerConfig('new-cmd') }, + runtimeAddSpy: addSpy, + }); + const manager = mkManager({ config, options: { pool: fakePool } }); + + const result = await manager.addRuntimeMcpServer( + 'runtime-srv', + { + command: 'new-cmd', + __qwenRuntimeMcpIfAbsent: true, + } as unknown as MCPServerConfig, + 'client-5c', + ); + + expect(result).toEqual({ + name: 'runtime-srv', + skipped: true, + reason: 'runtime_name_conflict', + }); + expect(addSpy).not.toHaveBeenCalled(); + expect(acquireSpy).not.toHaveBeenCalled(); + }); + // ───── REMOVE cases ─────────────────────────────────────────────── it('case 1: removes runtime entry → removed=true, wasShadowingSettings=false', async () => { diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 4ff7d6dc31..c942875a43 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -40,6 +40,7 @@ import { } from './mcp-errors.js'; const debugLogger = createDebugLogger('MCP'); +export const RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG = '__qwenRuntimeMcpIfAbsent'; /** * Configuration for MCP health monitoring @@ -2877,6 +2878,30 @@ export class McpClientManager { // no pool churn needed. Compare against the existing pooled // connection (if any). const newConnId = connectionIdOf(name, config); + const ifAbsent = + (config as MCPServerConfig & Record)[ + RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG + ] === true; + if (ifAbsent) { + const existingRuntimeConfig = this.cliConfig.getRuntimeMcpServers()[name]; + const existingIsIfAbsent = + ( + existingRuntimeConfig as + | (MCPServerConfig & Record) + | undefined + )?.[RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG] === true; + if ( + existingRuntimeConfig && + (!existingIsIfAbsent || + connectionIdOf(name, existingRuntimeConfig) !== newConnId) + ) { + return { + name, + skipped: true, + reason: 'runtime_name_conflict', + }; + } + } const existingConn = this.pooledConnections.get(name); if (existingConn && existingConn.id === newConnId) { // Same fingerprint — no transport churn, just update Config overlay @@ -3162,7 +3187,7 @@ export type AddRuntimeMcpServerResult = | { name: string; skipped: true; - reason: 'budget_warning_only'; + reason: 'budget_warning_only' | 'runtime_name_conflict'; }; export type RemoveRuntimeMcpServerResult = diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index ed1dc5a094..4574facdad 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -38,6 +38,7 @@ import { listMcpResources, MCPServerStatus, McpClient, + discoverTools, populateMcpServerCommand, removeMCPServerStatus, removeMCPStatusChangeListener, @@ -991,6 +992,31 @@ describe('mcp-client', () => { expect(promptRegistry.registerPrompt).not.toHaveBeenCalled(); }); + it('marks discovered tools alwaysLoad when the MCP server config requests it', async () => { + const mockedClient = { + listTools: vi.fn().mockResolvedValue({ tools: [] }), + } as unknown as ClientLib.Client; + vi.mocked(GenAiLib.mcpToTool).mockReturnValue({ + tool: () => + Promise.resolve({ + functionDeclarations: [{ name: 'chrome_tool' }], + }), + } as unknown as GenAiLib.CallableTool); + + const tools = await discoverTools( + 'chrome-devtools', + { + command: 'test-command', + alwaysLoadTools: true, + }, + mockedClient, + cfgWithResources(), + ); + + expect(tools).toHaveLength(1); + expect(tools[0].alwaysLoad).toBe(true); + }); + it('discoverAndReturn with { applyConfigFilters: false } ignores config filters and trust for shared pool snapshots', async () => { const mockedClient = { connect: vi.fn(), diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index bad571d1ea..40d40b3cba 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1033,6 +1033,7 @@ export async function discoverTools( mcpTimeout, cliConfig?.getMcpToolIdleTimeoutMs(), annotationsMap.get(funcDecl.name!), + mcpServerConfig.alwaysLoadTools === true, ), ); } catch (error) { diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 2f5942def3..47ea64426c 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -553,6 +553,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< private readonly mcpTimeout?: number, private readonly mcpToolIdleTimeoutMs?: number, readonly annotations?: McpToolAnnotations, + alwaysLoad = false, ) { super( nameOverride ?? @@ -565,7 +566,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< true, // canUpdateOutput — enables streaming progress for MCP tools true, // shouldDefer — MCP tools are discovered via ToolSearch to keep the // initial tool-declaration list small when many MCP servers are attached. - false, // alwaysLoad + alwaysLoad, // searchHint: server name boosts fuzzy matching when the user references // the server in their query ("send a slack message"). `mcp ${serverName}`, @@ -586,6 +587,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.mcpTimeout, this.mcpToolIdleTimeoutMs, this.annotations, + this.alwaysLoad, ); } @@ -622,6 +624,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.mcpTimeout, this.mcpToolIdleTimeoutMs, this.annotations, + this.alwaysLoad, ); } diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 4e241e6bb9..5fe1ea7ef8 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -1543,7 +1543,7 @@ export type DaemonRuntimeMcpAddResult = | { readonly name: string; readonly skipped: true; - readonly reason: 'budget_warning_only'; + readonly reason: 'budget_warning_only' | 'runtime_name_conflict'; }; /** diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 5a515d9c99..09ccd9d33a 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -1141,6 +1141,13 @@ "items": { "type": "string" } + }, + "toolIdleTimeoutMs": { + "description": "Idle timeout in milliseconds for MCP tool calls. If the MCP server does not produce any response or progress update within this time, the call is aborted. Default: 300000 (5 minutes). Can be overridden via QWEN_CODE_MCP_TOOL_IDLE_TIMEOUT_MS environment variable.", + "type": "number", + "default": 300000, + "minimum": 10000, + "maximum": 3600000 } } },