From b319094c6b7ec741449e96ed7930683e224fa3ef Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 14:27:30 +0800 Subject: [PATCH 01/26] feat(sdk): add mcp_server_added daemon event type (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Schema-only addition. New event fires on POST /workspace/mcp/servers success including replace and same-fingerprint no-op, carrying {name, transport, replaced, shadowedSettings, toolCount, originatorClientId}. Also exports DAEMON_KNOWN_EVENT_TYPE_VALUES from the public SDK surface so drift-insurance tests can assert on the known-event roster. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/sdk-typescript/src/daemon/events.ts | 53 ++++++++++++++++++- packages/sdk-typescript/src/daemon/index.ts | 3 ++ packages/sdk-typescript/src/index.ts | 3 ++ .../test/unit/daemon-public-surface.test.ts | 35 ++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index fbca5c8fd36..a9732978495 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -11,7 +11,7 @@ import type { PermissionOutcome, } from './types.js'; -const DAEMON_KNOWN_EVENT_TYPE_VALUES = [ +export const DAEMON_KNOWN_EVENT_TYPE_VALUES = [ 'session_update', 'permission_request', 'permission_resolved', @@ -64,6 +64,10 @@ const DAEMON_KNOWN_EVENT_TYPE_VALUES = [ 'workspace_initialized', 'mcp_server_restarted', 'mcp_server_restart_refused', + // T2.8 (#4514) โ€” runtime MCP server add/remove events. Fired by + // `POST /workspace/mcp/servers` on success (including replace and + // same-fingerprint no-op). + 'mcp_server_added', // #4175 F3 (Commit 7) โ€” multi-client permission coordination events. // `permission_partial_vote` only fires under `consensus` policy; // `permission_forbidden` fires under `designated` (originator @@ -639,6 +643,26 @@ export interface DaemonTurnErrorData { [key: string]: unknown; } +/** + * T2.8 (#4514). Fired when `POST /workspace/mcp/servers` succeeds, + * including both fresh additions and replace-on-existing-name. The + * event fans out to every active session SSE bus. + */ +export interface DaemonMcpServerAddedData { + readonly name: string; + readonly transport: DaemonMcpTransport; + readonly replaced: boolean; + readonly shadowedSettings: boolean; + readonly toolCount: number; + readonly originatorClientId: string; + [key: string]: unknown; +} + +export type DaemonMcpServerAddedEvent = DaemonEventEnvelope< + 'mcp_server_added', + DaemonMcpServerAddedData +>; + export type DaemonSessionUpdateEvent = DaemonEventEnvelope< 'session_update', DaemonSessionUpdateData @@ -796,7 +820,8 @@ export type DaemonControlEvent = | DaemonToolToggledEvent | DaemonWorkspaceInitializedEvent | DaemonMcpServerRestartedEvent - | DaemonMcpServerRestartRefusedEvent; + | DaemonMcpServerRestartRefusedEvent + | DaemonMcpServerAddedEvent; export type DaemonStreamLifecycleEvent = | DaemonClientEvictedEvent @@ -1291,6 +1316,9 @@ export function asKnownDaemonEvent( case 'followup_suggestion': return isFollowupSuggestionData(event.data) ? (event as DaemonFollowupSuggestionEvent) + case 'mcp_server_added': + return isMcpServerAddedData(event.data) + ? (event as DaemonMcpServerAddedEvent) : undefined; case 'turn_complete': return isTurnCompleteData(event.data) @@ -1659,6 +1687,8 @@ export function reduceDaemonSessionEvent( ...base, lastTurnError: event.data, }; + case 'mcp_server_added': + return base; default: { const _exhaustive: never = event; return _exhaustive; @@ -2358,6 +2388,25 @@ function isFollowupSuggestionData( isNonEmptyString(value['sessionId']) && isNonEmptyString(value['suggestion']) && isNonEmptyString(value['promptId']) +function isMcpServerAddedData( + value: unknown, +): value is DaemonMcpServerAddedData { + if (!isRecord(value)) return false; + if (!isNonEmptyString(value['name'])) return false; + if (typeof value['replaced'] !== 'boolean') return false; + if (typeof value['shadowedSettings'] !== 'boolean') return false; + if (!isFiniteNumber(value['toolCount'])) return false; + if (!isNonEmptyString(value['originatorClientId'])) return false; + // Transport family must be one of the known kinds. Reject silently + // for forward-compat (mirrors `isMcpRefusedServerEntry`). + const transport = value['transport']; + return ( + transport === 'stdio' || + transport === 'sse' || + transport === 'http' || + transport === 'websocket' || + transport === 'sdk' || + transport === 'unknown' ); } diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index b0c49733000..13466528341 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -29,6 +29,7 @@ export { } from './DaemonSessionClient.js'; export { asKnownDaemonEvent, + DAEMON_KNOWN_EVENT_TYPE_VALUES, createDaemonAuthState, createDaemonSessionViewState, isDaemonEventType, @@ -165,6 +166,8 @@ export type { // signal for SSE reconnects past the ring eviction boundary. DaemonStateResyncRequiredData, DaemonStateResyncRequiredEvent, + DaemonMcpServerAddedData, + DaemonMcpServerAddedEvent, DaemonMcpServerRestartedData, DaemonMcpServerRestartedEvent, DaemonMcpServerRestartRefusedData, diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index 21aea2dbdb4..0210c8f3d24 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -7,6 +7,7 @@ export { SdkLogger } from './utils/logger.js'; export { DAEMON_APPROVAL_MODES, DAEMON_ERROR_KINDS, + DAEMON_KNOWN_EVENT_TYPE_VALUES, DaemonCapabilityMissingError, DaemonClient, DaemonHttpError, @@ -31,6 +32,8 @@ export { type DaemonMcpRestartResult, type DaemonSessionRecapResult, type DaemonShellCommandResult, + type DaemonMcpServerAddedData, + type DaemonMcpServerAddedEvent, type DaemonMcpServerRestartedData, type DaemonMcpServerRestartedEvent, type DaemonMcpServerRestartRefusedData, diff --git a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts index 7d8067e8d5e..141ae688b57 100644 --- a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts +++ b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts @@ -6,6 +6,10 @@ import { describe, it, expect, expectTypeOf } from 'vitest'; import * as Public from '../../src/index.js'; +import { + DAEMON_KNOWN_EVENT_TYPE_VALUES, + asKnownDaemonEvent, +} from '../../src/daemon/events.js'; // Type-only imports also exercise the public entry: any name missing // from `src/index.ts` is a tsc compile error and the suite refuses to // build, which is the regression fence for the kind of "exists in @@ -140,3 +144,34 @@ describe('public SDK entry โ€” typed daemon event surface (#4217)', () => { expect(Public.DAEMON_ERROR_KINDS).toContain('writer_idle_timeout'); }); }); + +describe('mcp_server_added event drift insurance', () => { + it('is exported in DAEMON_KNOWN_EVENT_TYPE_VALUES', () => { + expect(DAEMON_KNOWN_EVENT_TYPE_VALUES).toContain('mcp_server_added'); + }); + + it('asKnownDaemonEvent returns the right discriminator', () => { + const evt: DaemonEvent = { + v: 1, + type: 'mcp_server_added', + data: { + name: 'echo', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }, + }; + const known = asKnownDaemonEvent(evt); + expect(known?.type).toBe('mcp_server_added'); + if (known?.type === 'mcp_server_added') { + expect(known.data.name).toBe('echo'); + expect(known.data.transport).toBe('stdio'); + expect(known.data.replaced).toBe(false); + expect(known.data.shadowedSettings).toBe(false); + expect(known.data.toolCount).toBe(3); + expect(known.data.originatorClientId).toBe('client-1'); + } + }); +}); From b19b374692fb250f85219e4ecb6657d21e121447 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 14:38:28 +0800 Subject: [PATCH 02/26] feat(sdk): add mcp_server_removed daemon event type (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Counterpart to mcp_server_added. Fires on DELETE /workspace/mcp/servers/:name that actually dropped an entry. Idempotent skip ('not_present') does NOT emit. Payload {name, wasShadowingSettings, originatorClientId}. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/sdk-typescript/src/daemon/events.ts | 43 ++++++++++++++++++- packages/sdk-typescript/src/daemon/index.ts | 2 + packages/sdk-typescript/src/index.ts | 2 + .../test/unit/daemon-public-surface.test.ts | 25 +++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index a9732978495..8196c1c07e9 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -68,6 +68,10 @@ export const DAEMON_KNOWN_EVENT_TYPE_VALUES = [ // `POST /workspace/mcp/servers` on success (including replace and // same-fingerprint no-op). 'mcp_server_added', + // T2.8 (#4514) โ€” counterpart of `mcp_server_added`. Fired by + // `DELETE /workspace/mcp/servers/:name` when an entry was actually + // removed. Idempotent skip ('not_present') does NOT emit. + 'mcp_server_removed', // #4175 F3 (Commit 7) โ€” multi-client permission coordination events. // `permission_partial_vote` only fires under `consensus` policy; // `permission_forbidden` fires under `designated` (originator @@ -663,6 +667,27 @@ export type DaemonMcpServerAddedEvent = DaemonEventEnvelope< DaemonMcpServerAddedData >; +/** + * T2.8 (#4514). Fired when `DELETE /workspace/mcp/servers/:name` + * actually drops an entry. Idempotent skip ('not_present') does NOT + * emit this event. The event fans out to every active session SSE bus. + * + * `wasShadowingSettings`: true when the removed runtime server was + * masking a settings-defined server of the same name โ€” the settings + * entry now takes effect again. + */ +export interface DaemonMcpServerRemovedData { + readonly name: string; + readonly wasShadowingSettings: boolean; + readonly originatorClientId: string; + [key: string]: unknown; +} + +export type DaemonMcpServerRemovedEvent = DaemonEventEnvelope< + 'mcp_server_removed', + DaemonMcpServerRemovedData +>; + export type DaemonSessionUpdateEvent = DaemonEventEnvelope< 'session_update', DaemonSessionUpdateData @@ -821,7 +846,8 @@ export type DaemonControlEvent = | DaemonWorkspaceInitializedEvent | DaemonMcpServerRestartedEvent | DaemonMcpServerRestartRefusedEvent - | DaemonMcpServerAddedEvent; + | DaemonMcpServerAddedEvent + | DaemonMcpServerRemovedEvent; export type DaemonStreamLifecycleEvent = | DaemonClientEvictedEvent @@ -1320,6 +1346,10 @@ export function asKnownDaemonEvent( return isMcpServerAddedData(event.data) ? (event as DaemonMcpServerAddedEvent) : undefined; + case 'mcp_server_removed': + return isMcpServerRemovedData(event.data) + ? (event as DaemonMcpServerRemovedEvent) + : undefined; case 'turn_complete': return isTurnCompleteData(event.data) ? (event as DaemonTurnCompleteEvent) @@ -1688,6 +1718,7 @@ export function reduceDaemonSessionEvent( lastTurnError: event.data, }; case 'mcp_server_added': + case 'mcp_server_removed': return base; default: { const _exhaustive: never = event; @@ -2426,6 +2457,16 @@ function isTurnErrorData(value: unknown): value is DaemonTurnErrorData { ); } +function isMcpServerRemovedData( + value: unknown, +): value is DaemonMcpServerRemovedData { + if (!isRecord(value)) return false; + if (!isNonEmptyString(value['name'])) return false; + if (typeof value['wasShadowingSettings'] !== 'boolean') return false; + if (!isNonEmptyString(value['originatorClientId'])) return false; + return true; +} + function isPermissionOption(value: unknown): value is DaemonPermissionOption { return isRecord(value) && isNonEmptyString(value['optionId']); } diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index 13466528341..d6a8c9ef782 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -168,6 +168,8 @@ export type { DaemonStateResyncRequiredEvent, DaemonMcpServerAddedData, DaemonMcpServerAddedEvent, + DaemonMcpServerRemovedData, + DaemonMcpServerRemovedEvent, DaemonMcpServerRestartedData, DaemonMcpServerRestartedEvent, DaemonMcpServerRestartRefusedData, diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index 0210c8f3d24..f5b5791c8e0 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -34,6 +34,8 @@ export { type DaemonShellCommandResult, type DaemonMcpServerAddedData, type DaemonMcpServerAddedEvent, + type DaemonMcpServerRemovedData, + type DaemonMcpServerRemovedEvent, type DaemonMcpServerRestartedData, type DaemonMcpServerRestartedEvent, type DaemonMcpServerRestartRefusedData, diff --git a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts index 141ae688b57..6a1879c95a8 100644 --- a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts +++ b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts @@ -175,3 +175,28 @@ describe('mcp_server_added event drift insurance', () => { } }); }); + +describe('mcp_server_removed event drift insurance', () => { + it('is exported in DAEMON_KNOWN_EVENT_TYPE_VALUES', () => { + expect(DAEMON_KNOWN_EVENT_TYPE_VALUES).toContain('mcp_server_removed'); + }); + + it('asKnownDaemonEvent returns the right discriminator', () => { + const evt: DaemonEvent = { + v: 1, + type: 'mcp_server_removed', + data: { + name: 'echo', + wasShadowingSettings: true, + originatorClientId: 'client-2', + }, + }; + const known = asKnownDaemonEvent(evt); + expect(known?.type).toBe('mcp_server_removed'); + if (known?.type === 'mcp_server_removed') { + expect(known.data.name).toBe('echo'); + expect(known.data.wasShadowingSettings).toBe(true); + expect(known.data.originatorClientId).toBe('client-2'); + } + }); +}); From e66cfa1c51bfc4db9502f1ec0d53d27ee3ad1faa Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 14:48:10 +0800 Subject: [PATCH 03/26] feat(sdk): add runtime MCP add/remove request + result types (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discriminated unions for add/remove results so caller can narrow on .skipped vs success. Add request mirrors the route body shape. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/sdk-typescript/src/daemon/index.ts | 4 + packages/sdk-typescript/src/daemon/types.ts | 78 +++++++++++++++++++ packages/sdk-typescript/src/index.ts | 4 + .../test/unit/daemon-public-surface.test.ts | 45 +++++++++++ 4 files changed, 131 insertions(+) diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index d6a8c9ef782..edffbecb5b0 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -260,6 +260,9 @@ export type { DaemonMcpRestartResult, DaemonSessionRecapResult, DaemonShellCommandResult, + DaemonRuntimeMcpAddRequest, + DaemonRuntimeMcpAddResult, + DaemonRuntimeMcpRemoveResult, DaemonToolToggleResult, DaemonAvailableCommand, DaemonCapabilities, @@ -329,6 +332,7 @@ export type { DaemonWriteMemoryRequest, DaemonWriteMemoryResult, HeartbeatResult, + MCPServerConfigShape, PermissionOutcome, PermissionOutcomeCancelled, PermissionOutcomeSelected, diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 393db8072d5..f17a429a41c 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -944,6 +944,84 @@ export type DaemonMcpRestartResult = reason: 'in_flight' | 'disabled' | 'budget_would_exceed'; }; +/** + * T2.8 (#4514). Structural subset of core's `MCPServerConfig` exposed + * on the `POST /workspace/mcp/servers` route body. Covers all wire- + * relevant transport fields without pulling in core-only concerns + * (e.g. `includeTools` / `excludeTools` filtering, `extensionName`). + * + * All fields are optional โ€” the daemon infers transport family from + * whichever set of fields is populated (stdio: `command`; SSE: `url`; + * HTTP: `httpUrl`; WebSocket: `tcp`; SDK: `type: 'sdk'`). + */ +export interface MCPServerConfigShape { + readonly type?: 'stdio' | 'sse' | 'http' | 'websocket' | 'sdk'; + readonly command?: string; + readonly args?: string[]; + readonly env?: Record; + readonly cwd?: string; + readonly url?: string; + readonly httpUrl?: string; + readonly headers?: Record; + readonly tcp?: string; + readonly timeout?: number; + readonly discoveryTimeoutMs?: number; + readonly trust?: boolean; + readonly description?: string; + readonly oauth?: Record; +} + +/** + * T2.8 (#4514). Body of `POST /workspace/mcp/servers` โ€” adds (or + * replaces) a runtime MCP server. + */ +export interface DaemonRuntimeMcpAddRequest { + readonly name: string; + readonly config: MCPServerConfigShape; + readonly displayName?: string; +} + +/** + * T2.8 (#4514). Response of `POST /workspace/mcp/servers`. + * Discriminated union: `.skipped` is absent (or `never`) on the + * success branch and `true` on the soft-refuse branch. Callers + * narrow with `if ('skipped' in res && res.skipped)`. + */ +export type DaemonRuntimeMcpAddResult = + | { + readonly name: string; + readonly transport: DaemonMcpTransport; + readonly replaced: boolean; + readonly shadowedSettings: boolean; + readonly toolCount: number; + readonly originatorClientId: string; + readonly skipped?: never; + } + | { + readonly name: string; + readonly skipped: true; + readonly reason: 'budget_warning_only'; + }; + +/** + * T2.8 (#4514). Response of `DELETE /workspace/mcp/servers/:name`. + * Discriminated union: `.skipped` absent on success, `true` on + * soft-refuse (server was not present โ€” idempotent skip). + */ +export type DaemonRuntimeMcpRemoveResult = + | { + readonly name: string; + readonly removed: true; + readonly wasShadowingSettings: boolean; + readonly originatorClientId: string; + readonly skipped?: never; + } + | { + readonly name: string; + readonly skipped: true; + readonly reason: 'not_present'; + }; + /** * Returned from `POST /session/:id/heartbeat`. `lastSeenAt` is the * server-side `Date.now()` epoch (ms) the daemon stored for this diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index f5b5791c8e0..a3b883ff9aa 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -32,6 +32,9 @@ export { type DaemonMcpRestartResult, type DaemonSessionRecapResult, type DaemonShellCommandResult, + type DaemonRuntimeMcpAddRequest, + type DaemonRuntimeMcpAddResult, + type DaemonRuntimeMcpRemoveResult, type DaemonMcpServerAddedData, type DaemonMcpServerAddedEvent, type DaemonMcpServerRemovedData, @@ -143,6 +146,7 @@ export { type DaemonWorkspaceSkillsStatus, type HeartbeatResult, type KnownDaemonEvent, + type MCPServerConfigShape, type PermissionOutcome, type PermissionOutcomeCancelled, type PermissionOutcomeSelected, diff --git a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts index 6a1879c95a8..22ec968631b 100644 --- a/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts +++ b/packages/sdk-typescript/test/unit/daemon-public-surface.test.ts @@ -31,6 +31,9 @@ import type { DaemonPermissionRequestEvent, DaemonPermissionResolvedData, DaemonPermissionResolvedEvent, + DaemonRuntimeMcpAddRequest, + DaemonRuntimeMcpAddResult, + DaemonRuntimeMcpRemoveResult, DaemonSessionDiedData, DaemonSessionDiedEvent, DaemonSessionEvent, @@ -200,3 +203,45 @@ describe('mcp_server_removed event drift insurance', () => { } }); }); + +describe('runtime MCP add/remove SDK types', () => { + it('request type compiles', () => { + const req: DaemonRuntimeMcpAddRequest = { + name: 'echo', + config: { command: 'node', args: ['echo.js'], type: 'stdio' }, + displayName: 'Echo Server', + }; + expect(req.name).toBe('echo'); + }); + + it('add result has right shape', () => { + const res: DaemonRuntimeMcpAddResult = { + name: 'echo', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 0, + originatorClientId: 'client-x', + }; + expect(res.replaced).toBe(false); + }); + + it('add soft-refuse has right shape', () => { + const res: DaemonRuntimeMcpAddResult = { + name: 'echo', + skipped: true, + reason: 'budget_warning_only', + }; + expect(res.skipped).toBe(true); + }); + + it('remove result has right shape', () => { + const res: DaemonRuntimeMcpRemoveResult = { + name: 'echo', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-x', + }; + expect(res.removed).toBe(true); + }); +}); From b6089acb3930d10d61315c33ac08747f0f25a1a5 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 15:01:48 +0800 Subject: [PATCH 04/26] feat(core): add Config.addRuntimeMcpServer / removeRuntimeMcpServer (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runtime-only overlay map separate from this.mcpServers (settings layer). Bypasses the initialized-guard on addMcpServers since the entire point is post-init mutation. getMcpServers() cascade extension comes in the next task. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/core/src/config/config.test.ts | 37 +++++++++++++++++++++++++ packages/core/src/config/config.ts | 21 ++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index ccb16d0284d..74d8a6c7b70 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3407,4 +3407,41 @@ describe('Model Switching and Config Updates', () => { ); }); }); + + describe('Config runtime MCP overlay', () => { + it('addRuntimeMcpServer does not mutate this.mcpServers', () => { + const config = new Config({ + ...baseParams, + mcpServers: { + 'settings-server': new MCPServerConfig('cmd-a'), + }, + }); + // Simulate post-init state + (config as unknown as { initialized: boolean }).initialized = true; + config.addRuntimeMcpServer( + 'runtime-server', + new MCPServerConfig('cmd-b'), + ); + const settingsLayer = ( + config as unknown as { + mcpServers: Record; + } + ).mcpServers; + expect(Object.keys(settingsLayer)).toEqual(['settings-server']); + expect(settingsLayer['runtime-server']).toBeUndefined(); + }); + + it('removeRuntimeMcpServer returns false when name not present', () => { + const config = new Config(baseParams); + expect(config.removeRuntimeMcpServer('does-not-exist')).toBe(false); + }); + + it('removeRuntimeMcpServer returns true and drops the entry', () => { + const config = new Config(baseParams); + (config as unknown as { initialized: boolean }).initialized = true; + config.addRuntimeMcpServer('x', new MCPServerConfig('cmd')); + expect(config.removeRuntimeMcpServer('x')).toBe(true); + expect(config.removeRuntimeMcpServer('x')).toBe(false); + }); + }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index f82b760fa6d..88321e5da07 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -862,6 +862,7 @@ export class Config { private readonly toolCallCommand: string | undefined; private readonly mcpServerCommand: string | undefined; private mcpServers: Record | undefined; + private readonly runtimeMcpServers = new Map(); private readonly lspEnabled: boolean; private lspClient?: LspClient; private lspInitializationError?: string; @@ -2511,6 +2512,26 @@ export class Config { this.mcpServers = { ...this.mcpServers, ...servers }; } + /** + * Add a runtime-only MCP server. Unlike `addMcpServers`, this does NOT + * touch `this.mcpServers` (settings layer) and intentionally bypasses + * the `initialized` guard โ€” the whole point is post-init mutation from + * the daemon surface. `getMcpServers()` will overlay these entries on + * top of the settings layer (Task 5). + */ + addRuntimeMcpServer(name: string, config: MCPServerConfig): void { + this.runtimeMcpServers.set(name, config); + } + + /** + * Remove a runtime-only MCP server previously added via + * `addRuntimeMcpServer`. Returns `true` if the entry existed and was + * removed, `false` otherwise. + */ + removeRuntimeMcpServer(name: string): boolean { + return this.runtimeMcpServers.delete(name); + } + isLspEnabled(): boolean { return this.lspEnabled && !this.getBareMode(); } From ffcca8da586a30e089d2371c40fe51015eb3c7f5 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 15:07:22 +0800 Subject: [PATCH 05/26] docs(core): tighten Config.addRuntimeMcpServer JSDoc wording (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "intentionally bypasses the guard" implied a suppressed if-throw; clarify to "does not enforce the guard" since there is nothing to bypass. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/core/src/config/config.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 88321e5da07..83d1a92a299 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -2514,10 +2514,10 @@ export class Config { /** * Add a runtime-only MCP server. Unlike `addMcpServers`, this does NOT - * touch `this.mcpServers` (settings layer) and intentionally bypasses - * the `initialized` guard โ€” the whole point is post-init mutation from - * the daemon surface. `getMcpServers()` will overlay these entries on - * top of the settings layer (Task 5). + * touch `this.mcpServers` (settings layer) and does not enforce the + * `initialized` guard โ€” the whole point is post-init mutation from the + * daemon surface. `getMcpServers()` will overlay these entries on top + * of the settings layer (Task 5). */ addRuntimeMcpServer(name: string, config: MCPServerConfig): void { this.runtimeMcpServers.set(name, config); From b9fe69350d2deab56cc2ef6c329fcefa9be068f4 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 15:10:37 +0800 Subject: [PATCH 06/26] feat(core): runtime MCP overlay in getMcpServers cascade (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runtimeMcpServers Map is applied as the last (winning) layer over extensions + this.mcpServers, then filtered by allowedMcpServers. Shadow semantics for T2.8 fall out of merge order โ€” runtime entries override settings entries by name; removeRuntimeMcpServer un-shadows. excludedMcpServers exclusion continues to flow through isMcpServerDisabled (UI layer), unchanged from prior behaviour. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/core/src/config/config.test.ts | 47 +++++++++++++++++++++++++ packages/core/src/config/config.ts | 5 +++ 2 files changed, 52 insertions(+) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 74d8a6c7b70..b3188bbec39 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -3444,4 +3444,51 @@ describe('Model Switching and Config Updates', () => { expect(config.removeRuntimeMcpServer('x')).toBe(false); }); }); + + describe('getMcpServers cascade with runtime overlay', () => { + it('runtime layer overlays settings layer (last write wins)', () => { + const config = new Config({ + ...baseParams, + mcpServers: { + shared: new MCPServerConfig('settings-cmd'), + }, + }); + (config as unknown as { initialized: boolean }).initialized = true; + config.addRuntimeMcpServer('shared', new MCPServerConfig('runtime-cmd')); + const merged = config.getMcpServers(); + expect(merged!['shared'].command).toBe('runtime-cmd'); + }); + + it('runtime-only entries appear in cascade', () => { + const config = new Config({ ...baseParams, mcpServers: {} }); + (config as unknown as { initialized: boolean }).initialized = true; + config.addRuntimeMcpServer('only-runtime', new MCPServerConfig('cmd')); + const merged = config.getMcpServers(); + expect(merged!['only-runtime']).toBeDefined(); + }); + + it('removing runtime entry restores settings entry', () => { + const config = new Config({ + ...baseParams, + mcpServers: { + shared: new MCPServerConfig('settings-cmd'), + }, + }); + (config as unknown as { initialized: boolean }).initialized = true; + config.addRuntimeMcpServer('shared', new MCPServerConfig('runtime-cmd')); + expect(config.getMcpServers()!['shared'].command).toBe('runtime-cmd'); + config.removeRuntimeMcpServer('shared'); + expect(config.getMcpServers()!['shared'].command).toBe('settings-cmd'); + }); + + it('isMcpServerDisabled still flags runtime entries when excluded', () => { + const config = new Config({ ...baseParams }); + (config as unknown as { initialized: boolean }).initialized = true; + config.addRuntimeMcpServer('blocked', new MCPServerConfig('cmd')); + config.setExcludedMcpServers(['blocked']); + // The entry appears in getMcpServers (UI layer filters via isMcpServerDisabled) + expect(config.getMcpServers()!['blocked']).toBeDefined(); + expect(config.isMcpServerDisabled('blocked')).toBe(true); + }); + }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 83d1a92a299..7c4bb1c1af9 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -2478,6 +2478,11 @@ export class Config { ); } + // T2.8 โ€” runtime layer wins over settings + extensions (shadow semantics) + for (const [name, cfg] of this.runtimeMcpServers) { + mcpServers[name] = cfg; + } + if (this.allowedMcpServers) { mcpServers = Object.fromEntries( Object.entries(mcpServers).filter(([key]) => From 656cf119040f89a9080826eed3a53ac88038ac95 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 19:12:20 +0800 Subject: [PATCH 07/26] feat(core): McpClientManager.{add,remove}RuntimeMcpServer + budget/pool wiring (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds runtime MCP server lifecycle on the manager: - addRuntimeMcpServer: budget tryReserve โ†’ Config runtime overlay โ†’ pool acquire - removeRuntimeMcpServer: Config drop โ†’ pool drain โ†’ budget release Shadow-over-settings detected via getSettingsMcpServers raw-map accessor on Config. Idempotent replace via fingerprint dedup at pool layer. Budget warn mode returns skipped soft-refuse rather than spawning. New error classes: McpBudgetWouldExceedError, McpServerSpawnFailedError, InvalidMcpConfigError. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/core/src/config/config.ts | 11 + .../core/src/tools/mcp-client-manager.test.ts | 375 ++++++++++++++++++ packages/core/src/tools/mcp-client-manager.ts | 306 ++++++++++++++ packages/core/src/tools/mcp-errors.ts | 65 +++ 4 files changed, 757 insertions(+) create mode 100644 packages/core/src/tools/mcp-errors.ts diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 7c4bb1c1af9..0b4149c3527 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -2463,6 +2463,17 @@ export class Config { return this.mcpTransportPool; } + /** + * T2.8: return the raw settings-layer MCP servers map (without the + * runtime overlay or extension contributions). Used by + * `McpClientManager.addRuntimeMcpServer` to detect shadow-over- + * settings (a runtime entry whose name collides with a pre-existing + * settings entry). + */ + getSettingsMcpServers(): Record | undefined { + return this.mcpServers; + } + getMcpServers(): Record | undefined { let mcpServers = { ...(this.mcpServers || {}) }; const extensions = this.getActiveExtensions(); diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index c86ac31abc0..efec9c4cc03 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -14,6 +14,7 @@ import type { ToolRegistry } from './tool-registry.js'; import 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'; vi.mock('./mcp-client.js', async () => { const originalModule = await vi.importActual('./mcp-client.js'); @@ -3175,3 +3176,377 @@ describe('McpClientManager โ€” PR 14b push events + hysteresis', () => { ).toHaveLength(2); }); }); + +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// T2.8: addRuntimeMcpServer / removeRuntimeMcpServer +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +describe('McpClientManager โ€” addRuntimeMcpServer / removeRuntimeMcpServer (T2.8)', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + /** + * Shared mock config builder for the T2.8 tests. Returns a mock Config + * that has `getSettingsMcpServers` (for shadow detection) and all the + * standard accessors the manager needs. + */ + function mkRuntimeConfig( + opts: { + settingsServers?: Record; + runtimeAddSpy?: ReturnType; + runtimeRemoveSpy?: ReturnType; + } = {}, + ) { + const addSpy = opts.runtimeAddSpy ?? vi.fn(); + const removeSpy = opts.runtimeRemoveSpy ?? vi.fn().mockReturnValue(true); + return { + isTrustedFolder: () => true, + getMcpServers: () => ({}), + getMcpServerCommand: () => undefined, + getPromptRegistry: () => ({}), + getWorkspaceContext: () => ({}), + getDebugMode: () => false, + getSessionId: () => 'test-session-1', + isMcpServerDisabled: () => false, + getSettingsMcpServers: () => opts.settingsServers ?? {}, + addRuntimeMcpServer: addSpy, + removeRuntimeMcpServer: removeSpy, + } as unknown as Config; + } + + // โ”€โ”€โ”€โ”€โ”€ ADD cases โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + it('case 1: happy fresh add โ†’ replaced=false, correct transport and toolCount', async () => { + const acquireSpy = vi.fn().mockResolvedValue({ + release: vi.fn(), + on: vi.fn(), + id: 'my-server::abc123', + serverName: 'my-server', + entryIndex: 0, + toolsSnapshot: [{ name: 'tool1' }, { name: 'tool2' }], + promptsSnapshot: [], + }); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const config = mkRuntimeConfig(); + const manager = mkManager({ config, options: { pool: fakePool } }); + + const result = await manager.addRuntimeMcpServer( + 'my-server', + { command: 'echo', args: ['hello'] }, + 'client-1', + ); + + expect(result).toMatchObject({ + name: 'my-server', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 2, + originatorClientId: 'client-1', + }); + expect(acquireSpy).toHaveBeenCalledTimes(1); + expect(config.addRuntimeMcpServer).toHaveBeenCalledWith( + 'my-server', + expect.objectContaining({ command: 'echo' }), + ); + }); + + it('case 2: budget enforce + at-cap + new name โ†’ throws McpBudgetWouldExceedError', async () => { + const { McpBudgetWouldExceedError } = await import('./mcp-errors.js'); + const fakeBudget = { + getMode: () => 'enforce' as const, + tryReserve: vi.fn().mockReturnValue('refused'), + getBudget: () => 1, + getReservedCount: () => 1, + beginBulkPass: vi.fn(), + endBulkPass: vi.fn(), + release: vi.fn(), + }; + const fakePool = { + acquire: vi.fn(), + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(fakeBudget), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const config = mkRuntimeConfig(); + const manager = mkManager({ config, options: { pool: fakePool } }); + + await expect( + manager.addRuntimeMcpServer( + 'new-server', + { command: 'node', args: ['server.js'] }, + 'client-2', + ), + ).rejects.toThrow(McpBudgetWouldExceedError); + + // Pool acquire should NOT have been called + expect(fakePool.acquire).not.toHaveBeenCalled(); + }); + + it('case 3: budget warn + at-cap + new name โ†’ skipped with budget_warning_only', async () => { + const fakeBudget = { + getMode: () => 'warn' as const, + tryReserve: vi.fn().mockReturnValue('reserved'), + getBudget: () => 1, + getReservedCount: () => 2, // over budget after reserve + beginBulkPass: vi.fn(), + endBulkPass: vi.fn(), + release: vi.fn(), + }; + const fakePool = { + acquire: vi.fn(), + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(fakeBudget), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const config = mkRuntimeConfig(); + const manager = mkManager({ config, options: { pool: fakePool } }); + + const result = await manager.addRuntimeMcpServer( + 'warn-server', + { command: 'node', args: ['server.js'] }, + 'client-3', + ); + + expect(result).toEqual({ + name: 'warn-server', + skipped: true, + reason: 'budget_warning_only', + }); + // Budget slot should have been released (soft refusal) + expect(fakeBudget.release).toHaveBeenCalledWith('warn-server'); + // Pool acquire should NOT have been called + expect(fakePool.acquire).not.toHaveBeenCalled(); + }); + + it('case 4: replace same name + same fingerprint โ†’ replaced=true, pool.acquire NOT re-called', async () => { + const serverConfig = { + command: 'echo', + args: ['hi'], + } as unknown as import('../config/config.js').MCPServerConfig; + // Compute the REAL connection ID so the mock matches what the + // implementation will compute via `connectionIdOf`. + const realId = connectionIdOf('dup-srv', serverConfig); + + const releaseSpyConn1 = vi.fn(); + const conn1 = { + release: releaseSpyConn1, + on: vi.fn(), + id: realId, + serverName: 'dup-srv', + entryIndex: 0, + toolsSnapshot: [ + { name: 'tool-a' }, + { name: 'tool-b' }, + { name: 'tool-c' }, + ], + promptsSnapshot: [], + }; + const acquireSpy = vi.fn().mockResolvedValue(conn1); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const config = mkRuntimeConfig(); + const manager = mkManager({ config, options: { pool: fakePool } }); + + // First add + await manager.addRuntimeMcpServer('dup-srv', serverConfig, 'client-4'); + expect(acquireSpy).toHaveBeenCalledTimes(1); + + // Second add with SAME config (same fingerprint) + acquireSpy.mockClear(); + const result = await manager.addRuntimeMcpServer( + 'dup-srv', + serverConfig, + 'client-4', + ); + + // pool.acquire should NOT have been re-called (idempotent replace) + expect(acquireSpy).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + name: 'dup-srv', + replaced: true, + toolCount: 3, + }); + }); + + it('case 5: shadows settings โ†’ shadowedSettings=true', async () => { + const acquireSpy = vi.fn().mockResolvedValue({ + release: vi.fn(), + on: vi.fn(), + id: 'shadow-srv::def', + serverName: 'shadow-srv', + entryIndex: 0, + toolsSnapshot: [{ name: 't1' }], + promptsSnapshot: [], + }); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + // Settings layer has an existing server with the same name + const config = mkRuntimeConfig({ + settingsServers: { 'shadow-srv': { command: 'old-cmd' } }, + }); + const manager = mkManager({ config, options: { pool: fakePool } }); + + const result = await manager.addRuntimeMcpServer( + 'shadow-srv', + { command: 'new-cmd', args: [] }, + 'client-5', + ); + + expect(result).toMatchObject({ + name: 'shadow-srv', + shadowedSettings: true, + }); + }); + + // โ”€โ”€โ”€โ”€โ”€ REMOVE cases โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + it('case 1: removes runtime entry โ†’ removed=true, wasShadowingSettings=false', async () => { + const releaseSpyConn = vi.fn(); + const conn = { + release: releaseSpyConn, + on: vi.fn(), + id: 'rm-srv::aaa', + serverName: 'rm-srv', + entryIndex: 0, + toolsSnapshot: [], + promptsSnapshot: [], + }; + const acquireSpy = vi.fn().mockResolvedValue(conn); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const removeSpy = vi.fn().mockReturnValue(true); + const config = mkRuntimeConfig({ runtimeRemoveSpy: removeSpy }); + const manager = mkManager({ config, options: { pool: fakePool } }); + + // Add then remove + await manager.addRuntimeMcpServer( + 'rm-srv', + { command: 'echo' }, + 'client-6', + ); + const result = await manager.removeRuntimeMcpServer('rm-srv', 'client-6'); + + expect(result).toMatchObject({ + name: 'rm-srv', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-6', + }); + expect(releaseSpyConn).toHaveBeenCalledTimes(1); + expect(removeSpy).toHaveBeenCalledWith('rm-srv'); + }); + + it('case 2: removes shadow over settings โ†’ wasShadowingSettings=true', async () => { + const conn = { + release: vi.fn(), + on: vi.fn(), + id: 'shadow-rm::bbb', + serverName: 'shadow-rm', + entryIndex: 0, + toolsSnapshot: [], + promptsSnapshot: [], + }; + const acquireSpy = vi.fn().mockResolvedValue(conn); + const fakePool = { + acquire: acquireSpy, + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const removeSpy = vi.fn().mockReturnValue(true); + const config = mkRuntimeConfig({ + settingsServers: { 'shadow-rm': { command: 'settings-cmd' } }, + runtimeRemoveSpy: removeSpy, + }); + const manager = mkManager({ config, options: { pool: fakePool } }); + + // Add runtime entry that shadows settings + await manager.addRuntimeMcpServer( + 'shadow-rm', + { command: 'runtime-cmd' }, + 'client-7', + ); + const result = await manager.removeRuntimeMcpServer( + 'shadow-rm', + 'client-7', + ); + + expect(result).toMatchObject({ + name: 'shadow-rm', + removed: true, + wasShadowingSettings: true, + }); + }); + + it('case 3: non-existent โ†’ skipped not_present', async () => { + const removeSpy = vi.fn().mockReturnValue(false); + const config = mkRuntimeConfig({ runtimeRemoveSpy: removeSpy }); + const manager = mkManager({ config }); + + const result = await manager.removeRuntimeMcpServer('ghost', 'client-8'); + + expect(result).toEqual({ + name: 'ghost', + skipped: true, + reason: 'not_present', + }); + }); + + // โ”€โ”€โ”€โ”€โ”€ Error class tests โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + it('throws InvalidMcpConfigError for config with unknown transport', async () => { + const { InvalidMcpConfigError } = await import('./mcp-errors.js'); + const config = mkRuntimeConfig(); + const manager = mkManager({ config }); + + await expect( + manager.addRuntimeMcpServer( + 'bad-cfg', + {} as unknown as import('../config/config.js').MCPServerConfig, + 'client-9', + ), + ).rejects.toThrow(InvalidMcpConfigError); + }); + + it('throws McpServerSpawnFailedError when pool.acquire rejects', async () => { + const { McpServerSpawnFailedError } = await import('./mcp-errors.js'); + const fakePool = { + acquire: vi.fn().mockRejectedValue(new Error('Connection refused')), + releaseSession: vi.fn(), + getBudget: vi.fn().mockReturnValue(undefined), + } as unknown as import('./mcp-transport-pool.js').McpTransportPool; + + const removeSpy = vi.fn().mockReturnValue(true); + const config = mkRuntimeConfig({ runtimeRemoveSpy: removeSpy }); + const manager = mkManager({ config, options: { pool: fakePool } }); + + await expect( + manager.addRuntimeMcpServer( + 'fail-srv', + { command: 'bad-binary' }, + 'client-10', + ), + ).rejects.toThrow(McpServerSpawnFailedError); + + // Config overlay should have been rolled back + expect(removeSpy).toHaveBeenCalledWith('fail-srv'); + }); +}); diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 53a4dff7836..6adc027f89b 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -33,6 +33,11 @@ import type { ReadResourceResult } from '@modelcontextprotocol/sdk/types.js'; // runtime side effects, so a static value import is fine. import { connectionIdOf } from './mcp-pool-key.js'; import type { ConnectionId } from './mcp-pool-events.js'; +import { + McpBudgetWouldExceedError, + McpServerSpawnFailedError, + InvalidMcpConfigError, +} from './mcp-errors.js'; const debugLogger = createDebugLogger('MCP'); @@ -2623,4 +2628,305 @@ export class McpClientManager { return client.readResource(uri, options); } + + // โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + // T2.8: Runtime MCP server lifecycle (add / remove) + // โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + /** + * Add (or replace) a runtime MCP server, wiring: + * 1. Config runtime overlay (shadow-over-settings detection) + * 2. Budget guard (enforce throws, warn returns skipped) + * 3. Pool acquire (or standalone McpClient connect + discover) + * + * Returns a result object describing what happened. Throws + * `McpBudgetWouldExceedError` on hard-cap violations, + * `McpServerSpawnFailedError` on transport failures, + * `InvalidMcpConfigError` on bad config. + */ + async addRuntimeMcpServer( + name: string, + config: MCPServerConfig, + originatorClientId: string, + ): Promise { + // Validate config minimally: must have at least one transport field + const transport = mcpTransportOf(config); + if (transport === 'unknown') { + throw new InvalidMcpConfigError( + name, + 'config must specify at least one of: command, url, httpUrl, tcp', + ); + } + + // Detect shadow-over-settings + const settingsServers = this.cliConfig.getSettingsMcpServers() ?? {}; + const shadowedSettings = name in settingsServers; + + // Check for idempotent replace: same name + same fingerprint means + // no pool churn needed. Compare against the existing pooled + // connection (if any). + const newConnId = connectionIdOf(name, config); + const existingConn = this.pooledConnections.get(name); + if (existingConn && existingConn.id === newConnId) { + // Same fingerprint โ€” just update the Config overlay (in case + // non-transport fields like trust/includeTools changed). + this.cliConfig.addRuntimeMcpServer(name, config); + const toolCount = existingConn.toolsSnapshot.length; + return { + name, + transport, + replaced: true, + shadowedSettings, + toolCount, + originatorClientId, + }; + } + + // Budget guard โ€” check using the appropriate budget layer + const budget = this.pool?.getBudget(); + if (budget) { + // Pool mode: use workspace budget + const mode = budget.getMode(); + if (mode === 'enforce' || mode === 'warn') { + // Only apply budget check if this is a genuinely NEW name + // (not a re-add of the same name already holding a slot) + const reservation = budget.tryReserve(name); + if (reservation === 'refused') { + // Hard cap โ€” enforce mode + throw new McpBudgetWouldExceedError(name); + } + // In warn mode, if the budget is at or above capacity and this + // is a new reservation, return a soft refusal + if ( + mode === 'warn' && + reservation === 'reserved' && + budget.getBudget() !== undefined && + budget.getReservedCount() > budget.getBudget()! + ) { + // Roll back the reservation โ€” we're not actually spawning + budget.release(name); + return { + name, + skipped: true, + reason: 'budget_warning_only', + }; + } + } + } else if (this.budgetMode !== 'off') { + // Standalone mode: use manager-level budget + const reservation = this.tryReserveSlot(name); + if (reservation === 'refused') { + throw new McpBudgetWouldExceedError(name); + } + if ( + this.budgetMode === 'warn' && + reservation === 'reserved' && + this.clientBudget !== undefined && + this.reservedSlots.size > this.clientBudget + ) { + this.releaseSlotName(name); + return { + name, + skipped: true, + reason: 'budget_warning_only', + }; + } + } + + // Release existing connection for this name (if replacing with + // different fingerprint) + const replaced = this.pooledConnections.has(name) || this.clients.has(name); + if (existingConn) { + try { + existingConn.release(); + } catch { + /* best effort */ + } + this.pooledConnections.delete(name); + } + const existingClient = this.clients.get(name); + if (existingClient) { + try { + await existingClient.disconnect(); + } catch { + /* best effort */ + } + this.clients.delete(name); + this.releaseSlotName(name); + } + + // Write the Config runtime overlay BEFORE spawning so + // `getMcpServers()` reflects the new entry immediately (the pool + // acquire + discover may read config for trust/filters). + this.cliConfig.addRuntimeMcpServer(name, config); + + // Acquire the transport + let toolCount = 0; + try { + if (this.pool && !isSdkMcpServerConfig(config)) { + // Pool mode: acquire through the shared pool + const sessionId = this.cliConfig.getSessionId(); + const promptRegistry = this.cliConfig.getPromptRegistry(); + const conn = await this.pool.acquire( + name, + config, + sessionId, + this.toolRegistry, + promptRegistry, + ); + this.pooledConnections.set(name, conn); + toolCount = conn.toolsSnapshot.length; + } else { + // Standalone mode: create a per-session McpClient + const sdkCallback = isSdkMcpServerConfig(config) + ? this.sendSdkMcpMessage + : undefined; + const client = new McpClient( + name, + config, + this.toolRegistry, + this.cliConfig.getPromptRegistry(), + this.cliConfig.getWorkspaceContext(), + this.cliConfig.getDebugMode(), + sdkCallback, + ); + this.clients.set(name, client); + this.eventEmitter?.emit('mcp-client-update', this.clients); + await client.connect(); + await client.discover(this.cliConfig); + this.eventEmitter?.emit('mcp-client-update', this.clients); + toolCount = this.toolRegistry.getToolsByServer(name).length; + } + } catch (err) { + // Spawn failed โ€” roll back Config overlay + budget reservation + this.cliConfig.removeRuntimeMcpServer(name); + if (budget) { + budget.release(name); + } else if (this.budgetMode !== 'off') { + this.releaseSlotName(name); + } + // Clean up any partial state + this.pooledConnections.delete(name); + this.clients.delete(name); + + const message = err instanceof Error ? err.message : String(err); + const isTimeout = message.includes('timed out'); + throw new McpServerSpawnFailedError(name, { + stderr: message, + timeout: isTimeout, + }); + } + + return { + name, + transport, + replaced, + shadowedSettings, + toolCount, + originatorClientId, + }; + } + + /** + * Remove a runtime MCP server previously added via + * `addRuntimeMcpServer`. Drops the Config overlay, releases the + * pool connection (or disconnects the standalone client), and + * releases the budget slot. + * + * Idempotent: returns `{skipped: true, reason: 'not_present'}` when + * no runtime entry exists for `name`. + */ + async removeRuntimeMcpServer( + name: string, + originatorClientId: string, + ): Promise { + // Check whether this name is a runtime entry + // Config.removeRuntimeMcpServer returns true only if the entry was + // in the runtime map. + const wasRuntime = this.cliConfig.removeRuntimeMcpServer(name); + if (!wasRuntime) { + return { name, skipped: true, reason: 'not_present' }; + } + + // Detect whether this was shadowing a settings-layer entry + const settingsServers = this.cliConfig.getSettingsMcpServers() ?? {}; + const wasShadowingSettings = name in settingsServers; + + // Release pool connection + const poolConn = this.pooledConnections.get(name); + if (poolConn) { + try { + poolConn.release(); + } catch { + /* best effort */ + } + this.pooledConnections.delete(name); + } + + // Disconnect standalone client + const client = this.clients.get(name); + if (client) { + try { + await client.disconnect(); + } catch { + /* best effort */ + } + this.clients.delete(name); + this.eventEmitter?.emit('mcp-client-update', this.clients); + } + + // Release budget slot + const budget = this.pool?.getBudget(); + if (budget) { + budget.release(name); + } else if (this.budgetMode !== 'off') { + this.releaseSlotName(name); + } + + return { + name, + removed: true, + wasShadowingSettings, + originatorClientId, + }; + } } + +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// T2.8: Result types for runtime MCP server add/remove +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +export type AddRuntimeMcpServerResult = + | { + name: string; + transport: McpTransportKind; + replaced: boolean; + shadowedSettings: boolean; + toolCount: number; + originatorClientId: string; + } + | { + name: string; + skipped: true; + reason: 'budget_warning_only'; + }; + +export type RemoveRuntimeMcpServerResult = + | { + name: string; + removed: true; + wasShadowingSettings: boolean; + originatorClientId: string; + } + | { + name: string; + skipped: true; + reason: 'not_present'; + }; + +// Re-export error classes for convenience +export { + McpBudgetWouldExceedError, + McpServerSpawnFailedError, + InvalidMcpConfigError, +} from './mcp-errors.js'; diff --git a/packages/core/src/tools/mcp-errors.ts b/packages/core/src/tools/mcp-errors.ts new file mode 100644 index 00000000000..3a375277eb6 --- /dev/null +++ b/packages/core/src/tools/mcp-errors.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * T2.8: thrown by `McpClientManager.addRuntimeMcpServer` when adding the + * server would exceed the workspace MCP budget in `enforce` mode. + */ +export class McpBudgetWouldExceedError extends Error { + readonly code = 'mcp_budget_would_exceed' as const; + readonly serverName: string; + constructor(serverName: string) { + super(`Adding '${serverName}' would exceed workspace MCP budget`); + this.name = 'McpBudgetWouldExceedError'; + this.serverName = serverName; + } +} + +/** + * T2.8: thrown by `McpClientManager.addRuntimeMcpServer` when the + * transport spawn (pool acquire / McpClient connect) fails. + */ +export class McpServerSpawnFailedError extends Error { + readonly code = 'mcp_server_spawn_failed' as const; + readonly serverName: string; + readonly details: { + exitCode?: number; + stderr?: string; + timeout?: boolean; + }; + constructor( + serverName: string, + details: { + exitCode?: number; + stderr?: string; + timeout?: boolean; + }, + ) { + super( + `Failed to spawn MCP server '${serverName}': ${JSON.stringify(details)}`, + ); + this.name = 'McpServerSpawnFailedError'; + this.serverName = serverName; + this.details = details; + } +} + +/** + * T2.8: thrown by `McpClientManager.addRuntimeMcpServer` when the + * provided server config is structurally invalid (e.g. missing both + * `command` and `url`/`httpUrl`). + */ +export class InvalidMcpConfigError extends Error { + readonly code = 'invalid_config' as const; + readonly serverName: string; + readonly reason: string; + constructor(serverName: string, reason: string) { + super(`Invalid MCP server config for '${serverName}': ${reason}`); + this.name = 'InvalidMcpConfigError'; + this.serverName = serverName; + this.reason = reason; + } +} From 5a8d10b081d6709d7d65e1068048bdc17146d9ff Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 19:19:08 +0800 Subject: [PATCH 08/26] feat(acp-bridge): add T2.8 error kinds (mcp_budget_would_exceed, mcp_server_spawn_failed, invalid_config) (#4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrored on the SDK via DAEMON_ERROR_KINDS export. Bridge maps the matching typed error classes (McpBudgetWouldExceedError, McpServerSpawnFailedError, InvalidMcpConfigError) to these kinds in sendBridgeError (next task). ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/acp-bridge/src/status.test.ts | 18 ++++++++++++------ packages/acp-bridge/src/status.ts | 16 +++++----------- packages/sdk-typescript/src/daemon/types.ts | 12 ++++++------ 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/acp-bridge/src/status.test.ts b/packages/acp-bridge/src/status.test.ts index bf89b0c2a27..f7f33d70bf2 100644 --- a/packages/acp-bridge/src/status.test.ts +++ b/packages/acp-bridge/src/status.test.ts @@ -20,12 +20,9 @@ describe('SERVE_ERROR_KINDS', () => { // kinds; PR 14 added `'budget_exhausted'` for MCP guardrail // refusals (see #4175 PR 14); PR 16 added `'stat_failed'` for // non-ENOENT stat failures on workspace memory discovery (see - // #4175 PR 16). Issue #4514 T2.9 appended - // `'prompt_deadline_exceeded'` (POST /session/:id/prompt 504) and - // `'writer_idle_timeout'` (terminal SSE client_evicted frame). - // Future additions append to this list โ€” the order is part of the - // contract so SDK consumers can pattern-match without per-kind - // lookups. + // #4175 PR 16). Issue #4514 T2.8 added three runtime-mutation + // error kinds; T2.9 appended prompt_deadline_exceeded and + // writer_idle_timeout. Future additions append to this list. expect(SERVE_ERROR_KINDS).toEqual([ 'missing_binary', 'blocked_egress', @@ -36,10 +33,19 @@ describe('SERVE_ERROR_KINDS', () => { 'parse_error', 'stat_failed', 'budget_exhausted', + 'mcp_budget_would_exceed', + 'mcp_server_spawn_failed', + 'invalid_config', 'prompt_deadline_exceeded', 'writer_idle_timeout', ]); }); + + it('exposes T2.8 error kinds in SERVE_ERROR_KINDS', () => { + expect(SERVE_ERROR_KINDS).toContain('mcp_budget_would_exceed'); + expect(SERVE_ERROR_KINDS).toContain('mcp_server_spawn_failed'); + expect(SERVE_ERROR_KINDS).toContain('invalid_config'); + }); }); describe('BridgeTimeoutError', () => { diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index a2fbd39d8f8..cef02e90786 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -28,18 +28,12 @@ export const SERVE_ERROR_KINDS = [ // Surfaced on per-server `mcp_server` cells (refused at discovery) // and on the workspace-level `mcp_budget` cell (any refusal this pass). 'budget_exhausted', - // Issue #4514 T2.9: a prompt exceeded the server-configured wallclock - // cap (`--prompt-deadline-ms`) or the request's own `deadlineMs` - // (capped at the server flag). Surfaced on the - // `POST /session/:id/prompt` 504 response so callers can branch on a - // typed kind instead of regex-matching the message. + // Issue #4514 T2.8: runtime MCP mutation routes + 'mcp_budget_would_exceed', + 'mcp_server_spawn_failed', + 'invalid_config', + // Issue #4514 T2.9: prompt deadline + writer idle timeout 'prompt_deadline_exceeded', - // Issue #4514 T2.9: an SSE writer's last successful flush was older - // than `--writer-idle-timeout-ms`. The daemon emits a terminal - // `client_evicted` frame with `reason: 'writer_idle_timeout'` before - // tearing the connection down; the kind appears on the frame payload - // (not an HTTP response โ€” by the time we detect the stall the stream - // is already in flight). 'writer_idle_timeout', ] as const; diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index f17a429a41c..9b113d61cf7 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -195,16 +195,16 @@ export const DAEMON_ERROR_KINDS = [ // Issue #4175 PR 14: budget refusal under `--mcp-budget-mode=enforce`. // Mirrors the serve-side `SERVE_ERROR_KINDS` addition. 'budget_exhausted', + // Issue #4514 T2.8: runtime MCP mutation routes + // (POST/DELETE /workspace/mcp/servers). Mirrors `SERVE_ERROR_KINDS`. + 'mcp_budget_would_exceed', + 'mcp_server_spawn_failed', + 'invalid_config', // Issue #4514 T2.9: a prompt exceeded the daemon-configured wallclock // cap (or the request's own `deadlineMs`, capped at the server flag). - // Surfaced on the `POST /session/:id/prompt` 504 response. Mirrors - // the serve-side `SERVE_ERROR_KINDS` addition. 'prompt_deadline_exceeded', // Issue #4514 T2.9: an SSE writer's last successful flush was older - // than the daemon's writer-idle deadline. Daemon emits a terminal - // `client_evicted` frame with `reason: 'writer_idle_timeout'`; the - // kind appears on that frame's `errorKind` field. Mirrors the - // serve-side `SERVE_ERROR_KINDS` addition. + // than the daemon's writer-idle deadline. 'writer_idle_timeout', ] as const; From 1e0d52e2f40a01ab394fd924071cca8f20e89bee Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 19:44:47 +0800 Subject: [PATCH 09/26] feat(acp-bridge): host-side {add,remove}RuntimeMcpServer methods + event fan-out (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bridge round-trips qwen/control/workspace/mcp/runtime-{add,remove} ACP ext-methods and emits mcp_server_added / mcp_server_removed via broadcastWorkspaceEvent. Soft-refuse (budget_warning_only) and idempotent skip (not_present) paths do NOT emit events. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/acp-bridge/src/bridge.test.ts | 266 +++++++++++++++++++++++++ packages/acp-bridge/src/bridge.ts | 118 +++++++++++ packages/acp-bridge/src/bridgeTypes.ts | 50 +++++ 3 files changed, 434 insertions(+) diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index d09bf8a31d2..2c2a3ab74d5 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -5506,6 +5506,272 @@ describe('createHttpAcpBridge', () => { }); }); + describe('addRuntimeMcpServer (T2.8 #4514)', () => { + /** + * Build a channel factory whose ACP `extMethod` handler returns a + * configurable response for `qwen/control/workspace/mcp/runtime-add`. + */ + function runtimeAddFactory( + respond: ( + params: Record, + ) => + | Record + | Promise> + | Promise, + ): ChannelFactory { + return async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const agent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/workspace/mcp/runtime-add') { + return Promise.resolve(respond(params)); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => agent as Agent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { exitCode: number | null; signalCode: NodeJS.Signals | null } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + } + + it('returns the success shape and broadcasts mcp_server_added', async () => { + const bridge = makeBridge({ + channelFactory: runtimeAddFactory((params) => ({ + name: params['name'], + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: params['originatorClientId'], + })), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const it = bridge + .subscribeEvents(session.sessionId, { signal: abort.signal }) + [Symbol.asyncIterator](); + const result = await bridge.addRuntimeMcpServer( + 'test-server', + { command: 'node', args: ['server.js'] }, + 'client-1', + ); + expect(result).toEqual({ + name: 'test-server', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }); + const next = await it.next(); + expect(next.value?.type).toBe('mcp_server_added'); + expect(next.value?.data).toMatchObject({ + name: 'test-server', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }); + abort.abort(); + await bridge.shutdown(); + }); + + it('does not emit event when result is skipped (budget_warning_only)', async () => { + const bridge = makeBridge({ + channelFactory: runtimeAddFactory(() => ({ + name: 'test-server', + skipped: true, + reason: 'budget_warning_only', + })), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const it = bridge + .subscribeEvents(session.sessionId, { signal: abort.signal }) + [Symbol.asyncIterator](); + const result = await bridge.addRuntimeMcpServer( + 'test-server', + { command: 'node', args: ['server.js'] }, + 'client-1', + ); + expect(result).toEqual({ + name: 'test-server', + skipped: true, + reason: 'budget_warning_only', + }); + // No event should have been emitted โ€” verify by checking that + // the async iterator has nothing ready (next() would hang). + // Use Promise.race with a short timeout to confirm no event. + const noEvent = await Promise.race([ + it.next().then(() => 'got_event'), + new Promise((r) => setTimeout(() => r('timeout'), 50)), + ]); + expect(noEvent).toBe('timeout'); + abort.abort(); + await bridge.shutdown(); + }); + + it('throws SessionNotFoundError when no ACP channel is live', async () => { + // Create a bridge but do NOT spawn any session + const bridge = makeBridge({}); + const err = await bridge + .addRuntimeMcpServer( + 'test-server', + { command: 'node', args: ['server.js'] }, + 'client-1', + ) + .catch((e) => e); + expect(err).toBeInstanceOf(SessionNotFoundError); + await bridge.shutdown(); + }); + + it('stamps mcp_server_added with the originator clientId', async () => { + const bridge = makeBridge({ + channelFactory: runtimeAddFactory((params) => ({ + name: params['name'], + transport: 'sse', + replaced: true, + shadowedSettings: true, + toolCount: 5, + originatorClientId: params['originatorClientId'], + })), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const it = bridge + .subscribeEvents(session.sessionId, { signal: abort.signal }) + [Symbol.asyncIterator](); + await bridge.addRuntimeMcpServer( + 'my-mcp', + { url: 'http://localhost:3000/sse' }, + session.clientId!, + ); + const next = await it.next(); + expect(next.value?.originatorClientId).toBe(session.clientId); + abort.abort(); + await bridge.shutdown(); + }); + }); + + describe('removeRuntimeMcpServer (T2.8 #4514)', () => { + /** + * Build a channel factory whose ACP `extMethod` handler returns a + * configurable response for `qwen/control/workspace/mcp/runtime-remove`. + */ + function runtimeRemoveFactory( + respond: ( + params: Record, + ) => + | Record + | Promise> + | Promise, + ): ChannelFactory { + return async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const agent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/workspace/mcp/runtime-remove') { + return Promise.resolve(respond(params)); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => agent as Agent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { exitCode: number | null; signalCode: NodeJS.Signals | null } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + } + + it('returns the removed shape and broadcasts mcp_server_removed', async () => { + const bridge = makeBridge({ + channelFactory: runtimeRemoveFactory((params) => ({ + name: params['name'], + removed: true, + wasShadowingSettings: false, + originatorClientId: params['originatorClientId'], + })), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const it = bridge + .subscribeEvents(session.sessionId, { signal: abort.signal }) + [Symbol.asyncIterator](); + const result = await bridge.removeRuntimeMcpServer( + 'test-server', + 'client-2', + ); + expect(result).toEqual({ + name: 'test-server', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-2', + }); + const next = await it.next(); + expect(next.value?.type).toBe('mcp_server_removed'); + expect(next.value?.data).toMatchObject({ + name: 'test-server', + wasShadowingSettings: false, + originatorClientId: 'client-2', + }); + abort.abort(); + await bridge.shutdown(); + }); + + it('does not emit event when result is skipped (not_present)', async () => { + const bridge = makeBridge({ + channelFactory: runtimeRemoveFactory(() => ({ + name: 'ghost', + skipped: true, + reason: 'not_present', + })), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const it = bridge + .subscribeEvents(session.sessionId, { signal: abort.signal }) + [Symbol.asyncIterator](); + const result = await bridge.removeRuntimeMcpServer('ghost', 'client-2'); + expect(result).toEqual({ + name: 'ghost', + skipped: true, + reason: 'not_present', + }); + // No event should have been emitted + const noEvent = await Promise.race([ + it.next().then(() => 'got_event'), + new Promise((r) => setTimeout(() => r('timeout'), 50)), + ]); + expect(noEvent).toBe('timeout'); + abort.abort(); + await bridge.shutdown(); + }); + + it('throws SessionNotFoundError when no ACP channel is live', async () => { + const bridge = makeBridge({}); + const err = await bridge + .removeRuntimeMcpServer('test-server', 'client-2') + .catch((e) => e); + expect(err).toBeInstanceOf(SessionNotFoundError); + await bridge.shutdown(); + }); + }); + describe('initWorkspace (#4175 Wave 4 PR 17)', () => { /** * Per-test workspace temp dir so the bridge's writeFile lands on a diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 10d64ccf602..6a88f837126 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -3673,6 +3673,124 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { return response; }, + async addRuntimeMcpServer(name, config, originatorClientId) { + // T2.8 (#4514). Round-trip the runtime-add ext-method through the + // live ACP child and broadcast an `mcp_server_added` event on + // success. Soft-refuse (`budget_warning_only`) returns the skip + // shape without emitting โ€” the caller (HTTP route) decides how to + // surface the skip to the SDK consumer. + const info = liveChannelInfo(); + if (!info) { + throw new SessionNotFoundError(`mcp-runtime-add:${name}`); + } + type AddOk = { + name: string; + transport: string; + replaced: boolean; + shadowedSettings: boolean; + toolCount: number; + originatorClientId: string; + }; + type AddSkip = { + name: string; + skipped: true; + reason: 'budget_warning_only'; + }; + let response: AddOk | AddSkip; + try { + response = (await Promise.race([ + withTimeout( + info.connection.extMethod( + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, + { name, config, originatorClientId }, + ), + MCP_RESTART_TIMEOUT_MS, + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, + ), + getChannelClosedReject(info), + ])) as AddOk | AddSkip; + } catch (err) { + // Re-instantiate structured ACP error payloads into typed bridge + // errors so `sendBridgeError` maps them to stable HTTP codes. + const data = (err as { data?: unknown })?.data; + if (data && typeof data === 'object') { + const kind = (data as { errorKind?: unknown }).errorKind; + if (kind === 'mcp_budget_would_exceed') { + throw err; + } + if (kind === 'mcp_server_spawn_failed') { + throw err; + } + if (kind === 'invalid_config') { + throw err; + } + } + throw err; + } + // Emit event on success (non-skip) + const addSkipped = (response as { skipped?: boolean }).skipped === true; + if (!addSkipped) { + const ok = response as AddOk; + broadcastWorkspaceEvent({ + type: 'mcp_server_added', + data: { + name: ok.name, + transport: ok.transport, + replaced: ok.replaced, + shadowedSettings: ok.shadowedSettings, + toolCount: ok.toolCount, + originatorClientId: ok.originatorClientId, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + } + return response; + }, + + async removeRuntimeMcpServer(name, originatorClientId) { + // T2.8 (#4514). Round-trip the runtime-remove ext-method through + // the live ACP child and broadcast `mcp_server_removed` on success. + // Idempotent skip (`not_present`) returns without emitting. + const info = liveChannelInfo(); + if (!info) { + throw new SessionNotFoundError(`mcp-runtime-remove:${name}`); + } + type RemoveOk = { + name: string; + removed: true; + wasShadowingSettings: boolean; + originatorClientId: string; + }; + type RemoveSkip = { name: string; skipped: true; reason: 'not_present' }; + const response = (await Promise.race([ + withTimeout( + info.connection.extMethod( + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove, + { name, originatorClientId }, + ), + MCP_RESTART_TIMEOUT_MS, + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove, + ), + getChannelClosedReject(info), + ])) as RemoveOk | RemoveSkip; + // Emit event on success (non-skip) + const removeSkipped = + (response as { skipped?: boolean }).skipped === true; + if (!removeSkipped) { + const ok = response as RemoveOk; + broadcastWorkspaceEvent({ + type: 'mcp_server_removed', + data: { + name: ok.name, + wasShadowingSettings: ok.wasShadowingSettings, + originatorClientId: ok.originatorClientId, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + } + return response; + }, + async initWorkspace(initOpts, originatorClientId) { // #4175 Wave 4 PR 17. Mechanical scaffold of an empty `QWEN.md` // (or whatever `getCurrentGeminiMdFilename()` returns under diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 52643a9c09c..67cd5f77ede 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -414,6 +414,56 @@ export interface HttpAcpBridge { action: 'created' | 'overwrote' | 'noop'; }>; + /** + * T2.8 (#4514): Add a runtime MCP server through the ACP child's + * `McpClientManager.addRuntimeMcpServer`. On success, broadcasts an + * `mcp_server_added` event to every session bus. Soft-refuse + * (`budget_warning_only` skip) does NOT emit an event โ€” the caller + * receives the skip shape and decides locally. + * + * Throws `SessionNotFoundError` when no ACP channel is live (caller + * should spawn or attach first). Typed ACP errors (budget-exceeded, + * spawn-failed, invalid-config) are re-instantiated from the + * JSON-RPC `data.errorKind` so the route's `sendBridgeError` can + * map them to stable HTTP status codes. + */ + addRuntimeMcpServer( + name: string, + config: Record, + originatorClientId: string, + ): Promise< + | { + name: string; + transport: string; + replaced: boolean; + shadowedSettings: boolean; + toolCount: number; + originatorClientId: string; + } + | { name: string; skipped: true; reason: 'budget_warning_only' } + >; + + /** + * T2.8 (#4514): Remove a runtime MCP server through the ACP child's + * `McpClientManager.removeRuntimeMcpServer`. On success, broadcasts + * an `mcp_server_removed` event. Idempotent skip (`not_present`) + * does NOT emit โ€” the caller receives the skip shape. + * + * Throws `SessionNotFoundError` when no ACP channel is live. + */ + removeRuntimeMcpServer( + name: string, + originatorClientId: string, + ): Promise< + | { + name: string; + removed: true; + wasShadowingSettings: boolean; + originatorClientId: string; + } + | { name: string; skipped: true; reason: 'not_present' } + >; + /** * Restart a configured MCP server through the ACP child's * `McpClientManager` (pre-F2) or transport pool (F2 #4175 commit 5). From 2efdf88bf550d5ae5d791a5236a828b1126ca06e Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 19:33:56 +0800 Subject: [PATCH 10/26] feat(acp-bridge): qwen/workspace/mcp/runtime-{add,remove} ext-methods (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Child-side ACP handlers delegate to McpClientManager.{add,remove}RuntimeMcpServer. Mirror /workspace/mcp/:server/restart registration pattern including typed-error โ†’ ACP error mapping (code field preserved for sendBridgeError mapping at the HTTP layer in Task 10). ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/acp-bridge/src/status.ts | 3 + .../cli/src/acp-integration/acpAgent.test.ts | 248 +++++++++++++++++- packages/cli/src/acp-integration/acpAgent.ts | 97 +++++++ 3 files changed, 347 insertions(+), 1 deletion(-) diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index cef02e90786..e80cc90e828 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -120,6 +120,9 @@ export const SERVE_CONTROL_EXT_METHODS = { sessionRecap: 'qwen/control/session/recap', sessionShellHistory: 'qwen/control/session/shell_history', workspaceMcpRestart: 'qwen/control/workspace/mcp/restart', + // T2.8 (#4514): runtime MCP server mutation ext-methods + workspaceMcpRuntimeAdd: 'qwen/control/workspace/mcp/runtime-add', + workspaceMcpRuntimeRemove: 'qwen/control/workspace/mcp/runtime-remove', } as const; export type ServeStatus = diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index c31335f1ded..8c316fcafa4 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -46,6 +46,13 @@ vi.mock('@agentclientprotocol/sdk', () => ({ })), ndJsonStream: vi.fn().mockReturnValue({}), RequestError: class RequestError extends Error { + code: number; + data: unknown; + constructor(code: number, message: string, data?: unknown) { + super(message); + this.code = code; + this.data = data; + } static authRequired = vi .fn() .mockImplementation((data: unknown, msg: string) => { @@ -60,6 +67,18 @@ vi.mock('@agentclientprotocol/sdk', () => ({ Object.assign(err, data); return err; }); + static internalError = vi + .fn() + .mockImplementation((data: unknown, msg: string) => { + const err = new Error(msg); + Object.assign(err, { code: -32603, data }); + return err; + }); + static methodNotFound = vi.fn().mockImplementation((method: string) => { + const err = new Error(`Method not found: ${method}`); + Object.assign(err, { code: -32601 }); + return err; + }); static resourceNotFound = vi.fn().mockImplementation((uri: string) => { const err = new Error(`Resource not found: ${uri}`); Object.assign(err, { code: -32002, data: { uri } }); @@ -158,6 +177,38 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ PromptInputExit: 'prompt_input_exit', Other: 'other', }, + // T2.8: error classes used by runtime MCP add/remove ext-method handlers + McpBudgetWouldExceedError: class McpBudgetWouldExceedError extends Error { + readonly code = 'mcp_budget_would_exceed' as const; + readonly serverName: string; + constructor(serverName: string) { + super(`Adding '${serverName}' would exceed workspace MCP budget`); + this.name = 'McpBudgetWouldExceedError'; + this.serverName = serverName; + } + }, + McpServerSpawnFailedError: class McpServerSpawnFailedError extends Error { + readonly code = 'mcp_server_spawn_failed' as const; + readonly serverName: string; + readonly details: Record; + constructor(serverName: string, details: Record) { + super(`Failed to spawn MCP server '${serverName}'`); + this.name = 'McpServerSpawnFailedError'; + this.serverName = serverName; + this.details = details; + } + }, + InvalidMcpConfigError: class InvalidMcpConfigError extends Error { + readonly code = 'invalid_config' as const; + readonly serverName: string; + readonly reason: string; + constructor(serverName: string, reason: string) { + super(`Invalid MCP server config for '${serverName}': ${reason}`); + this.name = 'InvalidMcpConfigError'; + this.serverName = serverName; + this.reason = reason; + } + }, })); vi.mock('./runtimeOutputDirContext.js', () => ({ @@ -213,13 +264,17 @@ import { getMCPDiscoveryState, getMCPServerStatus, tokenLimit, + McpBudgetWouldExceedError, } from '@qwen-code/qwen-code-core'; import type { McpServer } from '@agentclientprotocol/sdk'; import { AgentSideConnection } from '@agentclientprotocol/sdk'; import { loadSettings } from '../config/settings.js'; import { loadCliConfig } from '../config/config.js'; import { Session, buildAvailableCommandsSnapshot } from './session/Session.js'; -import { SERVE_STATUS_EXT_METHODS } from '../serve/status.js'; +import { + SERVE_STATUS_EXT_METHODS, + SERVE_CONTROL_EXT_METHODS, +} from '../serve/status.js'; describe('runAcpAgent shutdown cleanup', () => { let processExitSpy: MockInstance; @@ -2930,3 +2985,194 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); }); + +// --------------------------------------------------------------------------- +// T2.8 (#4514): extMethod runtime-add / runtime-remove +// --------------------------------------------------------------------------- + +describe('QwenAgent extMethod runtime MCP add/remove (T2.8)', () => { + let capturedAgentFactory: + | ((conn: { closed: Promise }) => { + initialize: (args: Record) => Promise; + extMethod: ( + method: string, + args: Record, + ) => Promise>; + }) + | undefined; + + let mockConfig: Config; + let processExitSpy: MockInstance; + let stdinDestroySpy: MockInstance; + let stdoutDestroySpy: MockInstance; + + const mockArgv = {} as CliArgs; + const mockSettings = { + merged: { mcpServers: {} }, + } as unknown as LoadedSettings; + + let mockManager: { + addRuntimeMcpServer: ReturnType; + removeRuntimeMcpServer: ReturnType; + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockConnectionState.reset(); + capturedAgentFactory = undefined; + + mockManager = { + addRuntimeMcpServer: vi.fn(), + removeRuntimeMcpServer: vi.fn(), + }; + + vi.mocked(AgentSideConnection).mockImplementation((factory: unknown) => { + capturedAgentFactory = factory as typeof capturedAgentFactory; + return { + get closed() { + return mockConnectionState.promise; + }, + } as unknown as InstanceType; + }); + + mockConfig = { + initialize: vi.fn().mockResolvedValue(undefined), + waitForMcpReady: vi.fn().mockResolvedValue(undefined), + getHookSystem: vi.fn().mockReturnValue(undefined), + getDisableAllHooks: vi.fn().mockReturnValue(false), + hasHooksForEvent: vi.fn().mockReturnValue(false), + getModel: vi.fn().mockReturnValue('test-model'), + getModelsConfig: vi.fn().mockReturnValue({ + getCurrentAuthType: vi.fn().mockReturnValue('api-key'), + }), + refreshAuth: vi.fn().mockResolvedValue(undefined), + getWorkspaceContext: vi.fn().mockReturnValue({}), + getDebugMode: vi.fn().mockReturnValue(false), + getToolRegistry: vi.fn().mockReturnValue({ + getMcpClientManager: vi.fn().mockReturnValue(mockManager), + }), + } as unknown as Config; + + processExitSpy = vi + .spyOn(process, 'exit') + .mockImplementation((() => undefined) as unknown as typeof process.exit); + stdinDestroySpy = vi + .spyOn(process.stdin, 'destroy') + .mockImplementation(() => process.stdin); + stdoutDestroySpy = vi + .spyOn(process.stdout, 'destroy') + .mockImplementation(() => process.stdout); + }); + + afterEach(() => { + processExitSpy.mockRestore(); + stdinDestroySpy.mockRestore(); + stdoutDestroySpy.mockRestore(); + }); + + async function getAgent() { + const agentPromise = runAcpAgent(mockConfig, mockSettings, mockArgv); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }); + return { agent, agentPromise }; + } + + it('runtime-add forwards to manager and returns success result', async () => { + mockManager.addRuntimeMcpServer.mockResolvedValue({ + name: 'my-srv', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }); + + const { agent, agentPromise } = await getAgent(); + const result = await agent.extMethod( + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, + { + name: 'my-srv', + config: { command: 'node', args: ['server.js'] }, + originatorClientId: 'client-1', + }, + ); + + expect(result).toEqual({ + name: 'my-srv', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }); + expect(mockManager.addRuntimeMcpServer).toHaveBeenCalledWith( + 'my-srv', + { command: 'node', args: ['server.js'] }, + 'client-1', + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('runtime-remove forwards to manager and returns success result', async () => { + mockManager.removeRuntimeMcpServer.mockResolvedValue({ + name: 'my-srv', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-2', + }); + + const { agent, agentPromise } = await getAgent(); + const result = await agent.extMethod( + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove, + { + name: 'my-srv', + originatorClientId: 'client-2', + }, + ); + + expect(result).toEqual({ + name: 'my-srv', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-2', + }); + expect(mockManager.removeRuntimeMcpServer).toHaveBeenCalledWith( + 'my-srv', + 'client-2', + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('runtime-add propagates McpBudgetWouldExceedError with code field', async () => { + // Use the actual mocked class so instanceof checks pass + const budgetError = new McpBudgetWouldExceedError('my-srv'); + mockManager.addRuntimeMcpServer.mockRejectedValue(budgetError); + + const { agent, agentPromise } = await getAgent(); + const err = await agent + .extMethod(SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, { + name: 'my-srv', + config: { command: 'node', args: ['server.js'] }, + originatorClientId: 'client-1', + }) + .catch((e: unknown) => e); + + // The error should be a RequestError with data.errorKind preserving + // the typed code for the bridge's sendBridgeError mapping + expect(err).toBeInstanceOf(Error); + const data = (err as { data?: Record }).data; + expect(data?.errorKind).toBe('mcp_budget_would_exceed'); + expect(data?.serverName).toBe('my-srv'); + + mockConnectionState.resolve(); + await agentPromise; + }); +}); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 906813ada39..1013fe8a613 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -28,6 +28,9 @@ import { WorkspaceMcpBudget, DiscoveredMCPTool, restoreWorktreeContext, + McpBudgetWouldExceedError, + McpServerSpawnFailedError, + InvalidMcpConfigError, } from '@qwen-code/qwen-code-core'; import type { ApprovalMode, @@ -2491,6 +2494,100 @@ class QwenAgent implements Agent { }); return { sessionId, injected: true }; } + case SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd: { + const name = params['name']; + const config = params['config']; + const originatorClientId = params['originatorClientId']; + if (typeof name !== 'string' || name.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing name', + ); + } + if (!config || typeof config !== 'object') { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing config', + ); + } + if ( + typeof originatorClientId !== 'string' || + originatorClientId.length === 0 + ) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing originatorClientId', + ); + } + const manager = this.config.getToolRegistry()?.getMcpClientManager(); + if (!manager) { + throw RequestError.internalError( + undefined, + 'McpClientManager unavailable on this Config', + ); + } + try { + const result = await manager.addRuntimeMcpServer( + name, + config as MCPServerConfig, + originatorClientId, + ); + return result as unknown as Record; + } catch (err) { + if (err instanceof McpBudgetWouldExceedError) { + throw new RequestError(-32099, err.message, { + errorKind: err.code, + serverName: err.serverName, + }); + } + if (err instanceof McpServerSpawnFailedError) { + throw new RequestError(-32099, err.message, { + errorKind: err.code, + serverName: err.serverName, + details: err.details, + }); + } + if (err instanceof InvalidMcpConfigError) { + throw new RequestError(-32099, err.message, { + errorKind: err.code, + serverName: err.serverName, + reason: err.reason, + }); + } + throw err; + } + } + case SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove: { + const name = params['name']; + const originatorClientId = params['originatorClientId']; + if (typeof name !== 'string' || name.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing name', + ); + } + if ( + typeof originatorClientId !== 'string' || + originatorClientId.length === 0 + ) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing originatorClientId', + ); + } + const manager = this.config.getToolRegistry()?.getMcpClientManager(); + if (!manager) { + throw RequestError.internalError( + undefined, + 'McpClientManager unavailable on this Config', + ); + } + const result = await manager.removeRuntimeMcpServer( + name, + originatorClientId, + ); + return result as unknown as Record; + } case 'deleteSession': { const sessionId = params['sessionId'] as string; if (!sessionId || !SESSION_ID_RE.test(sessionId)) { From 85f62425aa09cf2e2011547d7a5211cb947d91a0 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 19:57:08 +0800 Subject: [PATCH 11/26] feat(serve): POST /workspace/mcp/servers route (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutate-strict route validates name + config shape, parses + validates X-Qwen-Client-Id, forwards to HttpAcpBridge.addRuntimeMcpServer. Errors propagated from ACP via RequestError(data.errorKind) and mapped to HTTP status in sendBridgeError: mcp_budget_would_exceed โ†’ 409, mcp_server_spawn_failed โ†’ 502 (body includes exitCode/stderr/timeout), invalid_config โ†’ 400, acp_channel_unavailable โ†’ 503. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/cli/src/serve/server.test.ts | 237 ++++++++++++++++++++++++++ packages/cli/src/serve/server.ts | 116 +++++++++++++ 2 files changed, 353 insertions(+) diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index d5e96ee325e..ba6a5066f5a 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -306,6 +306,21 @@ interface FakeBridgeOpts { reason: 'in_flight' | 'disabled' | 'budget_would_exceed'; } >; + addRuntimeMcpServerImpl?: ( + name: string, + config: Record, + originatorClientId: string, + ) => Promise< + | { + name: string; + transport: string; + replaced: boolean; + shadowedSettings: boolean; + toolCount: number; + originatorClientId: string; + } + | { name: string; skipped: true; reason: 'budget_warning_only' } + >; closeImpl?: ( sessionId: string, context?: BridgeClientRequestContext, @@ -393,6 +408,11 @@ interface FakeBridge extends HttpAcpBridge { originatorClientId?: string; opts?: { entryIndex?: number }; }>; + addRuntimeMcpServerCalls: Array<{ + name: string; + config: Record; + originatorClientId: string; + }>; closeCalls: Array<{ sessionId: string; context?: BridgeClientRequestContext; @@ -601,6 +621,21 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { restarted: true as const, durationMs: 42, })); + const addRuntimeMcpServerCalls: FakeBridge['addRuntimeMcpServerCalls'] = []; + const addRuntimeMcpServerImpl = + opts.addRuntimeMcpServerImpl ?? + (async ( + name: string, + _config: Record, + originatorClientId: string, + ) => ({ + name, + transport: 'stdio' as const, + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId, + })); const closeImpl = opts.closeImpl ?? (async () => {}); const updateMetadataImpl = opts.updateMetadataImpl ?? @@ -648,6 +683,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { setToolEnabledCalls, initWorkspaceCalls, restartMcpServerCalls, + addRuntimeMcpServerCalls, closeCalls, updateMetadataCalls, heartbeatCalls, @@ -831,6 +867,19 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { }); return restartMcpServerImpl(serverName, originatorClientId, restartOpts); }, + async addRuntimeMcpServer(name, config, originatorClientId) { + addRuntimeMcpServerCalls.push({ name, config, originatorClientId }); + return addRuntimeMcpServerImpl(name, config, originatorClientId); + }, + async removeRuntimeMcpServer(_name, _originatorClientId) { + // Stub for FakeBridge โ€” T2.8 Task 11 adds the real route + tests. + return { + name: _name, + removed: true as const, + wasShadowingSettings: false, + originatorClientId: _originatorClientId, + }; + }, async closeSession(sessionId, context) { closeCalls.push({ sessionId, ...(context ? { context } : {}) }); return closeImpl(sessionId, context); @@ -3244,6 +3293,194 @@ describe('createServeApp', () => { }); }); + describe('POST /workspace/mcp/servers (T2.8 #4514)', () => { + const tokenOpts: ServeOptions = { ...baseOpts, token: 'secret' }; + const auth = (req: request.Test): request.Test => + req + .set('Host', `127.0.0.1:${tokenOpts.port}`) + .set('Authorization', 'Bearer secret'); + + it('200 fresh add returns structured result', async () => { + const bridge = fakeBridge({ knownClientIds: ['client-1'] }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')) + .set('X-Qwen-Client-Id', 'client-1') + .send({ name: 'echo', config: { command: 'echo', args: ['hello'] } }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + name: 'echo', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(1); + expect(bridge.addRuntimeMcpServerCalls[0]).toMatchObject({ + name: 'echo', + config: { command: 'echo', args: ['hello'] }, + originatorClientId: 'client-1', + }); + }); + + it('200 soft refuse (skipped:true, reason:budget_warning_only)', async () => { + const bridge = fakeBridge({ + knownClientIds: ['client-1'], + addRuntimeMcpServerImpl: async (name) => ({ + name, + skipped: true as const, + reason: 'budget_warning_only' as const, + }), + }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')) + .set('X-Qwen-Client-Id', 'client-1') + .send({ name: 'echo', config: { command: 'echo' } }); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + name: 'echo', + skipped: true, + reason: 'budget_warning_only', + }); + }); + + it('400 invalid_server_name when name is empty', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')).send({ + name: '', + config: { command: 'echo' }, + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('400 invalid_server_name when name exceeds MAX_SERVER_NAME_LENGTH', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const overlong = 'a'.repeat(257); + const res = await auth(request(app).post('/workspace/mcp/servers')).send({ + name: overlong, + config: { command: 'echo' }, + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('400 invalid_server_name when name contains illegal chars (slash)', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')).send({ + name: 'foo/bar', + config: { command: 'echo' }, + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('400 missing_required_field when config is absent', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')).send({ + name: 'echo', + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('missing_required_field'); + expect(res.body.field).toBe('config'); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('401 auth_required when no bearer token (strict gate)', async () => { + const bridge = fakeBridge(); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/workspace/mcp/servers') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ name: 'echo', config: { command: 'echo' } }); + expect(res.status).toBe(401); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('400 invalid_client_id on unknown X-Qwen-Client-Id', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')) + .set('X-Qwen-Client-Id', 'forged-client') + .send({ name: 'echo', config: { command: 'echo' } }); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ + code: 'invalid_client_id', + clientId: 'forged-client', + }); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('409 mcp_budget_would_exceed when bridge throws with that errorKind', async () => { + const bridge = fakeBridge({ + knownClientIds: ['client-1'], + addRuntimeMcpServerImpl: async () => { + throw Object.assign(new Error('Budget exceeded'), { + data: { errorKind: 'mcp_budget_would_exceed', serverName: 'echo' }, + }); + }, + }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')) + .set('X-Qwen-Client-Id', 'client-1') + .send({ name: 'echo', config: { command: 'echo' } }); + expect(res.status).toBe(409); + expect(res.body.code).toBe('mcp_budget_would_exceed'); + }); + + it('502 mcp_server_spawn_failed with body details', async () => { + const bridge = fakeBridge({ + knownClientIds: ['client-1'], + addRuntimeMcpServerImpl: async () => { + throw Object.assign(new Error('Spawn failed'), { + data: { + errorKind: 'mcp_server_spawn_failed', + serverName: 'broken', + exitCode: 1, + stderr: 'module not found', + timeout: false, + }, + }); + }, + }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')) + .set('X-Qwen-Client-Id', 'client-1') + .send({ name: 'broken', config: { command: 'bad-cmd' } }); + expect(res.status).toBe(502); + expect(res.body).toMatchObject({ + code: 'mcp_server_spawn_failed', + serverName: 'broken', + exitCode: 1, + stderr: 'module not found', + }); + }); + + it('503 acp_channel_unavailable when bridge throws with that errorKind', async () => { + const bridge = fakeBridge({ + knownClientIds: ['client-1'], + addRuntimeMcpServerImpl: async () => { + throw Object.assign(new Error('No ACP channel'), { + data: { errorKind: 'acp_channel_unavailable' }, + }); + }, + }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).post('/workspace/mcp/servers')) + .set('X-Qwen-Client-Id', 'client-1') + .send({ name: 'echo', config: { command: 'echo' } }); + expect(res.status).toBe(503); + expect(res.body.code).toBe('acp_channel_unavailable'); + }); + }); + describe('POST /workspace/tools/:name/enable (#4175 Wave 4 PR 17)', () => { const tokenOpts: ServeOptions = { ...baseOpts, token: 'secret' }; const auth = (req: request.Test): request.Test => diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 07ba77d81df..faf3d04a273 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -1943,6 +1943,71 @@ export function createServeApp( }, ); + // T2.8 (#4514): Add a runtime MCP server. Validates body.name + + // body.config shape, forwards to HttpAcpBridge.addRuntimeMcpServer. + // Typed ACP errors (budget-exceeded, spawn-failed, invalid-config) are + // propagated via sendBridgeError with errorKind-based HTTP status mapping. + app.post( + '/workspace/mcp/servers', + mutate({ strict: true }), + async (req, res) => { + const body = safeBody(req); + const name = body['name']; + // Validate name: must be non-empty string, alphanumeric + _ and - + if (typeof name !== 'string' || name.length === 0) { + res.status(400).json({ + error: 'Server name is required and must be a non-empty string', + code: 'invalid_server_name', + }); + return; + } + if (name.length > MAX_SERVER_NAME_LENGTH) { + res.status(400).json({ + error: `Server name exceeds ${MAX_SERVER_NAME_LENGTH}-character limit`, + code: 'invalid_server_name', + }); + return; + } + if (!/^[A-Za-z0-9_-]+$/.test(name)) { + res.status(400).json({ + error: + 'Server name must contain only alphanumeric characters, underscores, and hyphens', + code: 'invalid_server_name', + }); + return; + } + // Validate config: must be a non-null object + const config = body['config']; + if ( + typeof config !== 'object' || + config === null || + Array.isArray(config) + ) { + res.status(400).json({ + error: '`config` must be a non-null object', + code: 'missing_required_field', + field: 'config', + }); + return; + } + // Validate client identity + const clientId = parseAndValidateWorkspaceClientId(req, res, bridge); + if (clientId === null) return; + try { + const result = await bridge.addRuntimeMcpServer( + name, + config as Record, + clientId ?? '', + ); + res.status(200).json(result); + } catch (err) { + sendBridgeError(res, err, { + route: 'POST /workspace/mcp/servers', + }); + } + }, + ); + app.post('/workspace/init', mutate({ strict: true }), async (req, res) => { // #4175 Wave 4 PR 17. Scaffold-only init: the bridge writes an // empty QWEN.md without invoking the LLM. Default refuses @@ -3361,6 +3426,57 @@ function sendBridgeErrorImpl( }); return; } + // T2.8 (#4514): errors from the ACP child with `data.errorKind` carry + // structured error semantics. Map known kinds to stable HTTP status + // codes so SDK clients can branch without parsing message text. + if (err && typeof err === 'object') { + const data = (err as { data?: unknown }).data; + if (data && typeof data === 'object') { + const kind = (data as { errorKind?: unknown }).errorKind; + if (kind === 'mcp_budget_would_exceed') { + res.status(409).json({ + error: errorMessage(err), + code: 'mcp_budget_would_exceed', + ...(data as object), + }); + return; + } + if (kind === 'mcp_server_spawn_failed') { + const d = data as { + errorKind: string; + serverName?: string; + exitCode?: number | null; + stderr?: string; + timeout?: boolean; + }; + res.status(502).json({ + error: errorMessage(err), + code: 'mcp_server_spawn_failed', + serverName: d.serverName, + exitCode: d.exitCode, + stderr: d.stderr, + ...(d.timeout !== undefined ? { timeout: d.timeout } : {}), + }); + return; + } + if (kind === 'invalid_config') { + res.status(400).json({ + error: errorMessage(err), + code: 'invalid_config', + ...(data as object), + }); + return; + } + if (kind === 'acp_channel_unavailable') { + res.status(503).json({ + error: errorMessage(err), + code: 'acp_channel_unavailable', + ...(data as object), + }); + return; + } + } + } // 5xx is the kind of error operators need to see in their daemon log // โ€” bridge ENOMEM, agent stack trace, unexpected throw, etc. Without // logging here every 500 disappears once the caller consumes the From 7e6afc74b288a434be9f1da7f3cf61dbe87c4c90 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 20:04:40 +0800 Subject: [PATCH 12/26] feat(serve): DELETE /workspace/mcp/servers/:name route (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutate-strict route validates :name path param (alphanumeric + _-, โ‰ค MAX_SERVER_NAME_LENGTH), parses + validates X-Qwen-Client-Id, forwards to HttpAcpBridge.removeRuntimeMcpServer. Idempotent: missing entry returns 200 {skipped:true, reason:'not_present'}. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/cli/src/serve/server.test.ts | 152 ++++++++++++++++++++++++-- packages/cli/src/serve/server.ts | 48 ++++++++ 2 files changed, 192 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index ba6a5066f5a..ccbe43883aa 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -321,6 +321,18 @@ interface FakeBridgeOpts { } | { name: string; skipped: true; reason: 'budget_warning_only' } >; + removeRuntimeMcpServerImpl?: ( + name: string, + originatorClientId: string, + ) => Promise< + | { + name: string; + removed: true; + wasShadowingSettings: boolean; + originatorClientId: string; + } + | { name: string; skipped: true; reason: 'not_present' } + >; closeImpl?: ( sessionId: string, context?: BridgeClientRequestContext, @@ -413,6 +425,10 @@ interface FakeBridge extends HttpAcpBridge { config: Record; originatorClientId: string; }>; + removeRuntimeMcpServerCalls: Array<{ + name: string; + originatorClientId: string; + }>; closeCalls: Array<{ sessionId: string; context?: BridgeClientRequestContext; @@ -636,6 +652,16 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { toolCount: 3, originatorClientId, })); + const removeRuntimeMcpServerCalls: FakeBridge['removeRuntimeMcpServerCalls'] = + []; + const removeRuntimeMcpServerImpl = + opts.removeRuntimeMcpServerImpl ?? + (async (name: string, originatorClientId: string) => ({ + name, + removed: true as const, + wasShadowingSettings: false, + originatorClientId, + })); const closeImpl = opts.closeImpl ?? (async () => {}); const updateMetadataImpl = opts.updateMetadataImpl ?? @@ -684,6 +710,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { initWorkspaceCalls, restartMcpServerCalls, addRuntimeMcpServerCalls, + removeRuntimeMcpServerCalls, closeCalls, updateMetadataCalls, heartbeatCalls, @@ -871,14 +898,9 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { addRuntimeMcpServerCalls.push({ name, config, originatorClientId }); return addRuntimeMcpServerImpl(name, config, originatorClientId); }, - async removeRuntimeMcpServer(_name, _originatorClientId) { - // Stub for FakeBridge โ€” T2.8 Task 11 adds the real route + tests. - return { - name: _name, - removed: true as const, - wasShadowingSettings: false, - originatorClientId: _originatorClientId, - }; + async removeRuntimeMcpServer(name, originatorClientId) { + removeRuntimeMcpServerCalls.push({ name, originatorClientId }); + return removeRuntimeMcpServerImpl(name, originatorClientId); }, async closeSession(sessionId, context) { closeCalls.push({ sessionId, ...(context ? { context } : {}) }); @@ -3481,6 +3503,120 @@ describe('createServeApp', () => { }); }); + describe('DELETE /workspace/mcp/servers/:name (T2.8 #4514)', () => { + const tokenOpts: ServeOptions = { ...baseOpts, token: 'secret' }; + const auth = (req: request.Test): request.Test => + req + .set('Host', `127.0.0.1:${tokenOpts.port}`) + .set('Authorization', 'Bearer secret'); + + it('200 removed:true with wasShadowingSettings:false', async () => { + const bridge = fakeBridge({ knownClientIds: ['client-1'] }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).delete('/workspace/mcp/servers/echo')) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + name: 'echo', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-1', + }); + expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(1); + expect(bridge.removeRuntimeMcpServerCalls[0]).toMatchObject({ + name: 'echo', + originatorClientId: 'client-1', + }); + }); + + it('200 skipped:true when server not present (idempotent)', async () => { + const bridge = fakeBridge({ + knownClientIds: ['client-1'], + removeRuntimeMcpServerImpl: async (name) => ({ + name, + skipped: true as const, + reason: 'not_present' as const, + }), + }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth( + request(app).delete('/workspace/mcp/servers/ghost'), + ) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + name: 'ghost', + skipped: true, + reason: 'not_present', + }); + }); + + it('200 removed:true with wasShadowingSettings:true', async () => { + const bridge = fakeBridge({ + knownClientIds: ['client-1'], + removeRuntimeMcpServerImpl: async (name, originatorClientId) => ({ + name, + removed: true as const, + wasShadowingSettings: true, + originatorClientId, + }), + }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth( + request(app).delete('/workspace/mcp/servers/shadowed-srv'), + ) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + name: 'shadowed-srv', + removed: true, + wasShadowingSettings: true, + originatorClientId: 'client-1', + }); + }); + + it('400 invalid_server_name when path param has illegal chars', async () => { + const bridge = fakeBridge({ knownClientIds: ['client-1'] }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth( + request(app).delete('/workspace/mcp/servers/bad%2Fname'), + ) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('401 auth_required when no bearer token (strict gate)', async () => { + const bridge = fakeBridge(); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .delete('/workspace/mcp/servers/echo') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send(); + expect(res.status).toBe(401); + expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('400 invalid_client_id on unknown X-Qwen-Client-Id', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth(request(app).delete('/workspace/mcp/servers/echo')) + .set('X-Qwen-Client-Id', 'forged-client') + .send(); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ + code: 'invalid_client_id', + clientId: 'forged-client', + }); + expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(0); + }); + }); + describe('POST /workspace/tools/:name/enable (#4175 Wave 4 PR 17)', () => { const tokenOpts: ServeOptions = { ...baseOpts, token: 'secret' }; const auth = (req: request.Test): request.Test => diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index faf3d04a273..c1bc0752cb8 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -2008,6 +2008,54 @@ export function createServeApp( }, ); + // T2.8 (#4514): Remove a runtime MCP server. Validates :name path param, + // forwards to HttpAcpBridge.removeRuntimeMcpServer. Idempotent: missing + // entry returns 200 {skipped:true, reason:'not_present'}. + app.delete( + '/workspace/mcp/servers/:name', + mutate({ strict: true }), + async (req, res) => { + const name = req.params['name'] ?? ''; + // Validate name: must be non-empty string, alphanumeric + _ and - + if (name.length === 0) { + res.status(400).json({ + error: 'Server name is required and must be a non-empty string', + code: 'invalid_server_name', + }); + return; + } + if (name.length > MAX_SERVER_NAME_LENGTH) { + res.status(400).json({ + error: `Server name exceeds ${MAX_SERVER_NAME_LENGTH}-character limit`, + code: 'invalid_server_name', + }); + return; + } + if (!/^[A-Za-z0-9_-]+$/.test(name)) { + res.status(400).json({ + error: + 'Server name must contain only alphanumeric characters, underscores, and hyphens', + code: 'invalid_server_name', + }); + return; + } + // Validate client identity + const clientId = parseAndValidateWorkspaceClientId(req, res, bridge); + if (clientId === null) return; + try { + const result = await bridge.removeRuntimeMcpServer( + name, + clientId ?? '', + ); + res.status(200).json(result); + } catch (err) { + sendBridgeError(res, err, { + route: 'DELETE /workspace/mcp/servers/:name', + }); + } + }, + ); + app.post('/workspace/init', mutate({ strict: true }), async (req, res) => { // #4175 Wave 4 PR 17. Scaffold-only init: the bridge writes an // empty QWEN.md without invoking the LLM. Default refuses From f626ae0f5c9a2fec7b8466c3f0704c05385425c1 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 20:10:01 +0800 Subject: [PATCH 13/26] feat(serve): mcp_server_runtime_mutation capability tag (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always-on tag in SERVE_CAPABILITY_REGISTRY. Pre-flight check before POST /workspace/mcp/servers โ€” older daemons silently 404. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/cli/src/serve/capabilities.ts | 6 ++++++ packages/cli/src/serve/server.test.ts | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index c7d47b12bdf..60d1faad483 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -109,6 +109,12 @@ export const SERVE_CAPABILITY_REGISTRY = { // surface). Listed alongside `mcp_guardrails` to keep the MCP-related // tags grouped. mcp_guardrail_events: { since: 'v1' }, + // T2.8 (#4514). Always-on. Daemon supports runtime MCP server + // mutation via `POST /workspace/mcp/servers` (add) and + // `DELETE /workspace/mcp/servers/:name` (remove). SDK clients + // pre-flight this tag before calling those routes โ€” older daemons + // without T2.8 silently 404. + mcp_server_runtime_mutation: { since: 'v1' }, // Issue #4175 PR 19. Daemon supports the read-only workspace file // surface: `GET /file`, `GET /list`, `GET /glob`, `GET /stat`. The // four routes are gated as a single feature because they share the diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index ccbe43883aa..e03147a738a 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -138,6 +138,9 @@ const EXPECTED_STAGE1_FEATURES = [ // MCP budget state crossings (`mcp_budget_warning` with hysteresis, // `mcp_child_refused_batch` coalesced per pass). 'mcp_guardrail_events', + // T2.8 (#4514). Always-on. Daemon supports runtime MCP server + // mutation (add / remove) via POST/DELETE /workspace/mcp/servers. + 'mcp_server_runtime_mutation', // Issue #4175 PR 19. Always-on. Daemon exposes the read-only file // surface: `GET /file`, `GET /list`, `GET /glob`, `GET /stat`. 'workspace_file_read', @@ -1174,6 +1177,18 @@ describe('createServeApp', () => { }); }); + it('registers mcp_server_runtime_mutation as a baseline tag (T2.8 #4514)', () => { + // Always-on tag. SDK clients pre-flight + // `caps.features.includes('mcp_server_runtime_mutation')` before + // calling `POST /workspace/mcp/servers` โ€” older daemons silently 404. + expect(SERVE_CAPABILITY_REGISTRY['mcp_server_runtime_mutation']).toEqual({ + since: 'v1', + }); + expect(getAdvertisedServeFeatures()).toContain( + 'mcp_server_runtime_mutation', + ); + }); + it('returns protocol version metadata with a fresh supported array', () => { const versions = getServeProtocolVersions(); expect(versions).toEqual({ current: 'v1', supported: ['v1'] }); From 93b6f4d87c91eb82575dc1cd90cb0b4387b19646 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 20:13:19 +0800 Subject: [PATCH 14/26] feat(sdk): DaemonClient.{add,remove}RuntimeMcpServer helpers (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thin wrappers around POST /workspace/mcp/servers and DELETE /workspace/mcp/servers/:name. Mirrors restartMcpServer helper. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- .../sdk-typescript/src/daemon/DaemonClient.ts | 64 +++++++++++++++++ .../test/unit/DaemonClient.test.ts | 69 +++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index f23b7fe13f8..8b3bb7cf0ae 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -52,6 +52,9 @@ import type { DaemonMcpRestartResult, DaemonSessionRecapResult, DaemonShellCommandResult, + DaemonRuntimeMcpAddRequest, + DaemonRuntimeMcpAddResult, + DaemonRuntimeMcpRemoveResult, DaemonToolToggleResult, } from './types.js'; @@ -1201,6 +1204,67 @@ export class DaemonClient { ); } + /** + * T2.8 (#4514). Add (or replace) a runtime MCP server. The daemon + * validates the config, starts the server, and emits an + * `mcp_server_added` SSE event to all live sessions. Callers + * pre-flight `caps.features.mcp_server_runtime_mutation` before + * calling โ€” older daemons return 404. + */ + async addRuntimeMcpServer( + request: DaemonRuntimeMcpAddRequest, + opts?: { clientId?: string }, + ): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/mcp/servers`, + { + method: 'POST', + headers: this.headers( + { 'Content-Type': 'application/json' }, + opts?.clientId, + ), + body: JSON.stringify(request), + }, + async (res) => { + if (!res.ok) { + throw await this.failOnError(res, 'POST /workspace/mcp/servers'); + } + return (await res.json()) as DaemonRuntimeMcpAddResult; + }, + ); + } + + /** + * T2.8 (#4514). Remove a runtime MCP server by name. The daemon + * tears down the server process, removes it from the runtime + * overlay, and emits an `mcp_server_removed` SSE event. Idempotent + * at the HTTP level: if the server was never present the daemon + * returns 200 with `{ skipped: true, reason: 'not_present' }`. + * Pre-flight `caps.features.mcp_server_runtime_mutation` before + * calling. + */ + async removeRuntimeMcpServer( + name: string, + opts?: { clientId?: string }, + ): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/mcp/servers/${encodeURIComponent(name)}`, + { + method: 'DELETE', + headers: this.headers({}, opts?.clientId), + }, + async (res) => { + if (!res.ok) { + throw await this.failOnError( + res, + 'DELETE /workspace/mcp/servers/:name', + ); + } + return (await res.json()) as DaemonRuntimeMcpRemoveResult; + }, + ); + } + /** * #4175 Wave 4 PR 17. Scaffold a `QWEN.md` at the daemon's bound * workspace root. Mechanical only โ€” does NOT invoke the LLM. The diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index c9256b211da..1e3328602a6 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -2335,6 +2335,75 @@ describe('DaemonClient', () => { }); }); + describe('addRuntimeMcpServer (T2.8 #4514)', () => { + it('POSTs /workspace/mcp/servers with JSON body and returns the typed result', async () => { + const result = { + name: 'my-server', + transport: 'stdio', + replaced: false, + shadowedSettings: false, + toolCount: 3, + originatorClientId: 'client-1', + }; + const { fetch, calls } = recordingFetch(() => jsonResponse(200, result)); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const res = await client.addRuntimeMcpServer({ + name: 'my-server', + config: { command: 'node', args: ['server.js'] }, + }); + expect(res).toEqual(result); + expect(calls[0]?.url).toBe('http://daemon/workspace/mcp/servers'); + expect(calls[0]?.method).toBe('POST'); + expect(calls[0]?.headers['content-type']).toBe('application/json'); + expect(JSON.parse(calls[0]!.body!)).toEqual({ + name: 'my-server', + config: { command: 'node', args: ['server.js'] }, + }); + }); + + it('throws DaemonHttpError on non-2xx', async () => { + const { fetch } = recordingFetch(() => + jsonResponse(400, { error: 'invalid config' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.addRuntimeMcpServer({ + name: 'bad', + config: { command: '' }, + }), + ).rejects.toMatchObject({ status: 400 }); + }); + }); + + describe('removeRuntimeMcpServer (T2.8 #4514)', () => { + it('DELETEs /workspace/mcp/servers/:name with URL-encoded name', async () => { + const result = { + name: 'my server', + removed: true, + wasShadowingSettings: false, + originatorClientId: 'client-1', + }; + const { fetch, calls } = recordingFetch(() => jsonResponse(200, result)); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const res = await client.removeRuntimeMcpServer('my server'); + expect(res).toEqual(result); + expect(calls[0]?.url).toBe( + 'http://daemon/workspace/mcp/servers/my%20server', + ); + expect(calls[0]?.method).toBe('DELETE'); + }); + + it('throws DaemonHttpError on non-2xx', async () => { + const { fetch } = recordingFetch(() => + jsonResponse(500, { error: 'internal' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.removeRuntimeMcpServer('ghost'), + ).rejects.toMatchObject({ status: 500 }); + }); + }); + // PR #4255 fold-in 10 #3 โ€” device-flow HTTP method coverage. The // round-8 reviewer flagged that `startDeviceFlow` / // `getDeviceFlow` / `cancelDeviceFlow` / `getAuthStatus` plus the From 6c3a51a3d2137b6e092361b0f07a1a99135543aa Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 20:19:03 +0800 Subject: [PATCH 15/26] docs(serve): document runtime MCP server mutation routes (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /workspace/mcp/servers + DELETE /workspace/mcp/servers/:name with shadow-over-settings semantics, ephemeral persistence, mcp_server_runtime_mutation capability tag, and event emission. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- docs/users/qwen-serve.md | 137 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 133 insertions(+), 4 deletions(-) diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 9a50e00a14d..94c420ff055 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -14,7 +14,7 @@ Run Qwen Code as a local HTTP daemon so multiple clients (IDE plugins, web UIs, - **Reconnect-safe streaming** โ€” SSE with `Last-Event-ID` reconnect lets a client drop and pick up exactly where it left off (within the ring's replay window). - **First-responder permissions** โ€” when the agent asks for permission to run a tool, every connected client sees the request; whichever client answers first wins. - **One daemon, one workspace** โ€” each `qwen serve` process binds to exactly one workspace at boot (per [#3803](https://github.com/QwenLM/qwen-code/issues/3803) ยง02). Multi-workspace deployments run one daemon per workspace on separate ports (or behind an orchestrator). -- **Remote runtime control** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 17) โ€” change a session's approval mode (`POST /session/:id/approval-mode`), toggle a tool per workspace (`POST /workspace/tools/:name/enable`), scaffold an empty `QWEN.md` (`POST /workspace/init`, mechanical only โ€” does NOT call the model; for AI-fill, follow up with `POST /session/:id/prompt`), or restart a single MCP server with a budget pre-check (`POST /workspace/mcp/:server/restart`). All four are strict-gated โ€” configure `--token` first. +- **Remote runtime control** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 17) โ€” change a session's approval mode (`POST /session/:id/approval-mode`), toggle a tool per workspace (`POST /workspace/tools/:name/enable`), scaffold an empty `QWEN.md` (`POST /workspace/init`, mechanical only โ€” does NOT call the model; for AI-fill, follow up with `POST /session/:id/prompt`), restart a single MCP server with a budget pre-check (`POST /workspace/mcp/:server/restart`), or add/remove MCP servers at runtime without a daemon restart (`POST /workspace/mcp/servers`, `DELETE /workspace/mcp/servers/:name`). All strict-gated โ€” configure `--token` first. - **Session recap** ([#4175](https://github.com/QwenLM/qwen-code/issues/4175) follow-up) โ€” fetch a one-sentence "where did I leave off" summary of an active session (`POST /session/:id/recap`). Wraps core's `generateSessionRecap` as a side-query against the fast model; pollutes neither the main chat history nor the SSE stream. Non-strict gate (same posture as `/prompt`); SDK helper `client.recapSession(sessionId)`. - **Known limit โ€” token-cost amplification:** the route is a pure-cost endpoint (each call is an LLM side-query, no state benefit) and the daemon has no per-route rate limit in v1. On a no-token loopback default a buggy or malicious local client can spam it to burn tokens. Configure `--token` (and optionally `--require-auth`) on shared dev hosts before exposing the daemon. - **Concurrent recap safety:** two simultaneous `/recap` calls on the same session run two independent side-queries. `generateSessionRecap` reads a snapshot of the chat history via `GeminiClient.getChat().getHistory()` and feeds it to a separate `BaseLlmClient.generateText` call (via `runSideQuery`); it never appends to or mutates the session's `GeminiChat`. Safe to call from multiple clients without coordination. @@ -391,10 +391,10 @@ The Stage 1.5 plan describes TUI as an in-process EventBus subscriber. In practi No TUI shell runs inside the daemon. The slash commands listed above **don't exist** in this mode โ€” there's no terminal UI to issue them from. Session state is therefore: -- **Boot-time-frozen** for `approval-mode` / `memory` / `mcp servers` / `agents` / `tools` allowlist / `auth` โ€” all loaded from settings + disk when the daemon's `qwen --acp` child starts; immutable for the session's lifetime. -- **Mutable over HTTP** only via the routes this PR exposes โ€” primarily `POST /session/:id/model` (publishes `model_switched`). Permission votes (`POST /permission/:requestId`) are per-request, not per-session-state. +- **Boot-time-frozen** for `approval-mode` / `memory` / `agents` / `tools` allowlist / `auth` โ€” all loaded from settings + disk when the daemon's `qwen --acp` child starts; immutable for the session's lifetime. Settings-defined MCP servers are likewise frozen at boot, but **runtime-added servers** (via `POST /workspace/mcp/servers`) can be added or removed without restart. +- **Mutable over HTTP** via `POST /session/:id/model` (publishes `model_switched`), `POST /workspace/mcp/servers` / `DELETE /workspace/mcp/servers/:name` (publishes `mcp_server_added` / `mcp_server_removed`), and permission votes (`POST /permission/:requestId`). -**Consequence:** remote clients in headless mode see the **full session state**. No TUI hides additional state; no drift is possible. If you want to change `approval-mode` or add an MCP server, restart the daemon with new settings โ€” the daemon doesn't expose runtime mutation for those today. +**Consequence:** remote clients in headless mode see the **full session state**. No TUI hides additional state; no drift is possible. If you want to change `approval-mode`, restart the daemon with new settings. MCP servers can now be added/removed at runtime via the mutation routes (`POST /workspace/mcp/servers`, `DELETE /workspace/mcp/servers/:name`) โ€” see [Runtime MCP server management](#runtime-mcp-server-management-issue-4514). #### Mode 2 โ€” Stage 1.5 `qwen --serve` co-hosted TUI (not in this PR) @@ -505,6 +505,135 @@ Session-scoped debug logs (`~/.qwen/debug/.txt` and the `~/.qwen/debu The daemon log appends indefinitely. Rotate manually if it grows large. A future enhancement may add automatic rotation; track via [#4548](https://github.com/QwenLM/qwen-code/issues/4548) follow-ups. +## Runtime MCP server management (issue [#4514](https://github.com/QwenLM/qwen-code/issues/4514)) + +Add or remove MCP servers at runtime without restarting the daemon. Runtime entries live in an ephemeral overlay that **shadows** settings-defined servers of the same name; the underlying `settings.json` / `mcpServers` config is never written to. + +**Pre-flight:** check `caps.features` for `mcp_server_runtime_mutation` before calling either route. Older daemons without this tag return `404`. + +### `POST /workspace/mcp/servers` โ€” add a runtime MCP server + +Strict-gated (bearer token required). Connects the server immediately via the live `McpClientManager` and discovers its tools. + +Request: + +```json +{ + "name": "my-server", + "config": { + "command": "npx", + "args": ["-y", "@my-org/mcp-server"], + "env": { "TOKEN": "..." } + } +} +``` + +`name` must be alphanumeric plus `_` and `-` (max 256 characters). `config` is the same MCP server configuration object used in `settings.json` `mcpServers` entries (transport-dependent fields: `command`/`args`/`env` for stdio, `url` for SSE/HTTP). + +Response (200) โ€” success: + +```json +{ + "name": "my-server", + "transport": "stdio", + "replaced": false, + "shadowedSettings": false, + "toolCount": 3, + "originatorClientId": "client-1" +} +``` + +- `replaced: true` โ€” a runtime entry with the same name already existed and was replaced (old connection torn down, new one established). +- `shadowedSettings: true` โ€” a settings-defined server with the same name exists; the runtime entry now shadows it. The settings entry is untouched and re-emerges if the runtime entry is later removed. +- `toolCount` โ€” number of tools discovered on the newly connected server. + +Response (200) โ€” soft refuse (budget warning mode): + +```json +{ + "name": "my-server", + "skipped": true, + "reason": "budget_warning_only" +} +``` + +Returned when `--mcp-budget-mode=warn` and adding the server would exceed the configured `--mcp-client-budget`. The server is NOT connected. Callers should surface the budget pressure to the user. + +Errors: + +| Status | Code | When | +| ------ | ------------------------- | -------------------------------------------------------------------------------------------------- | +| `400` | `invalid_server_name` | Name empty, exceeds 256 chars, or contains characters outside `[A-Za-z0-9_-]` | +| `400` | `missing_required_field` | `config` missing or not a non-null object | +| `400` | `invalid_client_id` | `X-Qwen-Client-Id` header present but not registered for this workspace | +| `400` | `invalid_config` | Config shape rejected by the MCP transport validator | +| `401` | `token_required` | No bearer token configured (strict gate) | +| `409` | `mcp_budget_would_exceed` | `--mcp-budget-mode=enforce` and budget is full | +| `502` | `mcp_server_spawn_failed` | Server process exited or timed out during connect; body carries `serverName`, `exitCode`, `stderr` | +| `503` | `acp_channel_unavailable` | No live ACP child (no session has been created yet) | + +### `DELETE /workspace/mcp/servers/:name` โ€” remove a runtime MCP server + +Strict-gated. Disconnects the server and removes it from the runtime overlay. Idempotent โ€” removing a name that was never added returns a skip response (not an error). + +The `:name` path parameter is the URL-encoded server name. + +Response (200) โ€” success: + +```json +{ + "name": "my-server", + "removed": true, + "wasShadowingSettings": false, + "originatorClientId": "client-1" +} +``` + +- `wasShadowingSettings: true` โ€” the removed runtime entry was shadowing a settings-defined server of the same name. That settings entry is now un-shadowed and will be used on next discovery/restart. + +Response (200) โ€” idempotent skip: + +```json +{ + "name": "ghost", + "skipped": true, + "reason": "not_present" +} +``` + +Returned when the name was not in the runtime overlay (it may still exist in settings โ€” settings entries cannot be removed via this route). + +Errors: + +| Status | Code | When | +| ------ | ------------------------- | ----------------------------------------------------------------------------- | +| `400` | `invalid_server_name` | Name empty, exceeds 256 chars, or contains characters outside `[A-Za-z0-9_-]` | +| `400` | `invalid_client_id` | `X-Qwen-Client-Id` header present but not registered for this workspace | +| `401` | `token_required` | No bearer token configured (strict gate) | +| `503` | `acp_channel_unavailable` | No live ACP child | + +### Shadow semantics + +Runtime entries form an ephemeral overlay on top of settings-defined MCP servers: + +- **Adding** a runtime server with the same name as a settings entry **shadows** it โ€” the runtime config takes precedence. The original settings entry is not modified. +- **Removing** a runtime server that was shadowing a settings entry **un-shadows** it โ€” the settings-defined config becomes active again on next connection. +- **Daemon restart** loses all runtime entries. Only settings-defined servers survive across restarts. Runtime servers are session-lifetime scoped. +- **`GET /workspace/mcp`** reports the merged view โ€” both settings-defined and runtime servers appear in the `servers[]` array. There is no wire-level distinction between the two origins in the snapshot today. + +### Events + +Both routes emit **workspace-scoped** SSE events (all active session buses receive them): + +| Event | Emitted when | Payload fields | +| -------------------- | ------------------------------- | -------------------------------------------------------------------------------------- | +| `mcp_server_added` | `POST` succeeds (not skipped) | `name`, `transport`, `replaced`, `shadowedSettings`, `toolCount`, `originatorClientId` | +| `mcp_server_removed` | `DELETE` succeeds (not skipped) | `name`, `wasShadowingSettings`, `originatorClientId` | + +Skipped responses (`budget_warning_only`, `not_present`) do NOT emit events. + +Budget-related events from the existing `mcp_guardrail_events` surface (`mcp_budget_warning`, `mcp_child_refused_batch`) also fire when runtime additions cross the budget threshold. + ## What's next - **Setting up a long-running daemon?** [Local launch templates (systemd / launchd / nohup / tmux)](./qwen-serve-deploy-local.md) for v0.16-alpha (local-only). From 6b448378a082e1c7809232da291a10ddd57b0be3 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 26 May 2026 20:21:22 +0800 Subject: [PATCH 16/26] fix(test): index-signature property access in acpAgent T2.8 test (#4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-commit typecheck (cli workspace) flagged err.data.errorKind / err.data.serverName needing bracket notation. Switch to data?.['errorKind'] to satisfy noPropertyAccessFromIndexSignature. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/cli/src/acp-integration/acpAgent.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 8c316fcafa4..c9718677bf4 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -3169,8 +3169,8 @@ describe('QwenAgent extMethod runtime MCP add/remove (T2.8)', () => { // the typed code for the bridge's sendBridgeError mapping expect(err).toBeInstanceOf(Error); const data = (err as { data?: Record }).data; - expect(data?.errorKind).toBe('mcp_budget_would_exceed'); - expect(data?.serverName).toBe('my-srv'); + expect(data?.['errorKind']).toBe('mcp_budget_would_exceed'); + expect(data?.['serverName']).toBe('my-srv'); mockConnectionState.resolve(); await agentPromise; From bb52f4661ae8cc302bb31cab41985f1618fc1c54 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 27 May 2026 00:27:23 +0800 Subject: [PATCH 17/26] fix(serve): address 5 Critical review items from wenshao (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1: Flatten spawn_failed details at ACP layer (spread err.details, not nest under data.details) so HTTP 502 body exposes exitCode/stderr/timeout. C2: Add toolRegistry.removeMcpToolsByServer + removeMCPServerStatus + stopHealthCheck to removeRuntimeMcpServer (mirrors removeServer cleanup). C3: Bridge throws error with data.errorKind='acp_channel_unavailable' instead of SessionNotFoundError so sendBridgeError maps to documented 503. C4: Require X-Qwen-Client-Id header on POST/DELETE runtime MCP routes โ€” return 400 missing_client_id instead of coercing to empty string. C5: Remove releaseSlotName in standalone replace path โ€” budget slot carries over to the new entry, preventing accounting leak. ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/acp-bridge/src/bridge.test.ts | 14 +++++++--- packages/acp-bridge/src/bridge.ts | 10 +++++-- packages/cli/src/acp-integration/acpAgent.ts | 2 +- packages/cli/src/serve/server.ts | 27 ++++++++++++++----- .../core/src/tools/mcp-client-manager.test.ts | 6 ++++- packages/core/src/tools/mcp-client-manager.ts | 9 ++++++- 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index 2c2a3ab74d5..2c1287e2966 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -5620,7 +5620,7 @@ describe('createHttpAcpBridge', () => { await bridge.shutdown(); }); - it('throws SessionNotFoundError when no ACP channel is live', async () => { + it('throws with errorKind acp_channel_unavailable when no ACP channel is live', async () => { // Create a bridge but do NOT spawn any session const bridge = makeBridge({}); const err = await bridge @@ -5630,7 +5630,10 @@ describe('createHttpAcpBridge', () => { 'client-1', ) .catch((e) => e); - expect(err).toBeInstanceOf(SessionNotFoundError); + expect(err).toBeInstanceOf(Error); + expect((err as { data?: { errorKind?: string } }).data?.errorKind).toBe( + 'acp_channel_unavailable', + ); await bridge.shutdown(); }); @@ -5762,12 +5765,15 @@ describe('createHttpAcpBridge', () => { await bridge.shutdown(); }); - it('throws SessionNotFoundError when no ACP channel is live', async () => { + it('throws with errorKind acp_channel_unavailable when no ACP channel is live', async () => { const bridge = makeBridge({}); const err = await bridge .removeRuntimeMcpServer('test-server', 'client-2') .catch((e) => e); - expect(err).toBeInstanceOf(SessionNotFoundError); + expect(err).toBeInstanceOf(Error); + expect((err as { data?: { errorKind?: string } }).data?.errorKind).toBe( + 'acp_channel_unavailable', + ); await bridge.shutdown(); }); }); diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 6a88f837126..9192fa7a467 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -3681,7 +3681,10 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { // surface the skip to the SDK consumer. const info = liveChannelInfo(); if (!info) { - throw new SessionNotFoundError(`mcp-runtime-add:${name}`); + throw Object.assign( + new Error(`No live ACP channel for runtime MCP add: ${name}`), + { data: { errorKind: 'acp_channel_unavailable' } }, + ); } type AddOk = { name: string; @@ -3753,7 +3756,10 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { // Idempotent skip (`not_present`) returns without emitting. const info = liveChannelInfo(); if (!info) { - throw new SessionNotFoundError(`mcp-runtime-remove:${name}`); + throw Object.assign( + new Error(`No live ACP channel for runtime MCP remove: ${name}`), + { data: { errorKind: 'acp_channel_unavailable' } }, + ); } type RemoveOk = { name: string; diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 1013fe8a613..63f38e567e2 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2544,7 +2544,7 @@ class QwenAgent implements Agent { throw new RequestError(-32099, err.message, { errorKind: err.code, serverName: err.serverName, - details: err.details, + ...err.details, }); } if (err instanceof InvalidMcpConfigError) { diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index c1bc0752cb8..26b7717749c 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -1990,14 +1990,22 @@ export function createServeApp( }); return; } - // Validate client identity + // Validate client identity (required for runtime MCP mutation) const clientId = parseAndValidateWorkspaceClientId(req, res, bridge); if (clientId === null) return; + if (!clientId) { + res.status(400).json({ + error: + '`X-Qwen-Client-Id` header is required for runtime MCP mutation', + code: 'missing_client_id', + }); + return; + } try { const result = await bridge.addRuntimeMcpServer( name, config as Record, - clientId ?? '', + clientId, ); res.status(200).json(result); } catch (err) { @@ -2039,14 +2047,19 @@ export function createServeApp( }); return; } - // Validate client identity + // Validate client identity (required for runtime MCP mutation) const clientId = parseAndValidateWorkspaceClientId(req, res, bridge); if (clientId === null) return; + if (!clientId) { + res.status(400).json({ + error: + '`X-Qwen-Client-Id` header is required for runtime MCP mutation', + code: 'missing_client_id', + }); + return; + } try { - const result = await bridge.removeRuntimeMcpServer( - name, - clientId ?? '', - ); + const result = await bridge.removeRuntimeMcpServer(name, clientId); res.status(200).json(result); } catch (err) { sendBridgeError(res, err, { diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index efec9c4cc03..538e42a4fc2 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -53,7 +53,11 @@ function mkManager( getSessionId: () => 'sid-1', isMcpServerDisabled: () => false, } as unknown as Config); - const toolRegistry = overrides.toolRegistry ?? ({} as ToolRegistry); + const toolRegistry = + overrides.toolRegistry ?? + ({ + removeMcpToolsByServer: vi.fn(), + } as unknown as ToolRegistry); return new McpClientManager(config, toolRegistry, overrides.options ?? {}); } diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 6adc027f89b..0a1a5a4a1f8 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -2752,7 +2752,9 @@ export class McpClientManager { /* best effort */ } this.clients.delete(name); - this.releaseSlotName(name); + // Do NOT releaseSlotName here โ€” the budget slot carries over to + // the new entry being spawned. Releasing + not re-reserving would + // leave the running server unaccounted in the budget. } // Write the Config runtime overlay BEFORE spawning so @@ -2875,6 +2877,11 @@ export class McpClientManager { this.eventEmitter?.emit('mcp-client-update', this.clients); } + // Cleanup: tool registry, status, health check (mirrors removeServer) + this.toolRegistry.removeMcpToolsByServer(name); + removeMCPServerStatus(name); + this.stopHealthCheck(name); + // Release budget slot const budget = this.pool?.getBudget(); if (budget) { From d7f9bd0a68258e4a9e3897d57022a5f75aed245d Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 27 May 2026 23:10:52 +0800 Subject: [PATCH 18/26] fix(core+cli): address round 4-6 Critical review items (T2.8 #4514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace flow: add toolRegistry.removeMcpToolsByServer + stopHealthCheck before disconnecting old entry (fixes stale tool + timer leak) - Spawn-failure catch: add toolRegistry.removeMcpToolsByServer + stopHealthCheck (fixes orphaned tools from partial discover) - Strip `trust` field from config in acpAgent ext-method handler (security: prevents runtime-added servers from bypassing permission gates) ๐Ÿค– Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --- packages/cli/src/acp-integration/acpAgent.ts | 8 +++++++- packages/core/src/tools/mcp-client-manager.ts | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 63f38e567e2..880cba9df0d 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2527,9 +2527,15 @@ class QwenAgent implements Agent { ); } try { + // Strip security-sensitive fields โ€” runtime-added servers must + // not bypass permission gates via trust:true from HTTP body + const { trust: _stripped, ...safeConfig } = config as Record< + string, + unknown + >; const result = await manager.addRuntimeMcpServer( name, - config as MCPServerConfig, + safeConfig as MCPServerConfig, originatorClientId, ); return result as unknown as Record; diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 0a1a5a4a1f8..2dcd380f62b 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -2743,15 +2743,19 @@ export class McpClientManager { /* best effort */ } this.pooledConnections.delete(name); + this.toolRegistry.removeMcpToolsByServer(name); + this.stopHealthCheck(name); } const existingClient = this.clients.get(name); if (existingClient) { + this.stopHealthCheck(name); try { await existingClient.disconnect(); } catch { /* best effort */ } this.clients.delete(name); + this.toolRegistry.removeMcpToolsByServer(name); // Do NOT releaseSlotName here โ€” the budget slot carries over to // the new entry being spawned. Releasing + not re-reserving would // leave the running server unaccounted in the budget. @@ -2807,9 +2811,11 @@ export class McpClientManager { } else if (this.budgetMode !== 'off') { this.releaseSlotName(name); } - // Clean up any partial state + // Clean up any partial state (including tools from partial discover) this.pooledConnections.delete(name); this.clients.delete(name); + this.toolRegistry.removeMcpToolsByServer(name); + this.stopHealthCheck(name); const message = err instanceof Error ? err.message : String(err); const isTimeout = message.includes('timed out'); From 1ba2c0996197dc481d9ee10b06ba6996b6e808a2 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 28 May 2026 02:33:56 +0800 Subject: [PATCH 19/26] =?UTF-8?q?fix(serve):=20address=20rounds=205-7=20re?= =?UTF-8?q?view=20items=20=E2=80=94=20build,=20security,=20correctness=20(?= =?UTF-8?q?T2.8=20#4514)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build breakers (Critical): - events.ts: add missing /** JSDoc opener for DaemonMcpServerAddedData - events.ts: add missing `: undefined` arm in followup_suggestion ternary - events.ts: close isFollowupSuggestionData function body (missing ); }) Security (Critical): - acpAgent: strip authProviderType, includeTools, excludeTools, cwd from runtime-added server configs (prevents SSRF via cloud creds leak and arbitrary cwd spawn) - mcp-client-manager: reject servers in excludedMcpServers blocklist - acpAgent: add Array.isArray guard to config validation Correctness: - mcp-client-manager: identity-check on pooledConnections.delete in remove (prevents concurrent add+remove race deleting NEW pool entry) - mcp-client-manager: add client.disconnect() in catch block for standalone path (prevents transport/process leak) - mcp-client-manager: add consecutiveFailures, isReconnecting, dropRefusalEntry cleanup in removeRuntimeMcpServer - mcp-client-manager: emit mcp-client-update on spawn failure cleanup - mcp-client-manager: extract exitCode from error when available - mcp-client-manager: fix replaced=true โ†’ false for same-fingerprint idempotent re-add (no transport was torn down) - server.ts: whitelist error fields in sendBridgeError responses (prevent unbounded internal ACP data spread) - bridge.ts: remove dead try/catch in addRuntimeMcpServer (all branches just re-threw) - bridge.ts: add try/catch to removeRuntimeMcpServer for error mapping - bridge.ts: narrow AddOk.transport to literal union type SDK / DX: - DaemonClient: add timeoutMs param to addRuntimeMcpServer (default 330s, matching restartMcpServer โ€” prevents 30s SDK timeout vs 5min bridge) - mcp-client-manager: add debugLogger.info at method entry Docs: - qwen-serve.md: clarify replaced:true vs replaced:false semantics --- docs/users/qwen-serve.md | 2 +- packages/acp-bridge/src/bridge.ts | 70 ++++++++----------- packages/cli/src/acp-integration/acpAgent.ts | 18 +++-- packages/cli/src/serve/server.ts | 8 ++- .../core/src/tools/mcp-client-manager.test.ts | 4 +- packages/core/src/tools/mcp-client-manager.ts | 44 ++++++++++-- .../sdk-typescript/src/daemon/DaemonClient.ts | 3 +- packages/sdk-typescript/src/daemon/events.ts | 4 ++ 8 files changed, 94 insertions(+), 59 deletions(-) diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 94c420ff055..5a32f315dbc 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -543,7 +543,7 @@ Response (200) โ€” success: } ``` -- `replaced: true` โ€” a runtime entry with the same name already existed and was replaced (old connection torn down, new one established). +- `replaced: true` โ€” a runtime entry with the same name already existed and the config fingerprint differs; old connection torn down, new one established. When the fingerprint matches (idempotent re-add), `replaced` is `false`. - `shadowedSettings: true` โ€” a settings-defined server with the same name exists; the runtime entry now shadows it. The settings entry is untouched and re-emerges if the runtime entry is later removed. - `toolCount` โ€” number of tools discovered on the newly connected server. diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 9192fa7a467..55b98290cac 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -3688,7 +3688,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { } type AddOk = { name: string; - transport: string; + transport: 'stdio' | 'sse' | 'http' | 'tcp' | 'sdk'; replaced: boolean; shadowedSettings: boolean; toolCount: number; @@ -3699,37 +3699,17 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { skipped: true; reason: 'budget_warning_only'; }; - let response: AddOk | AddSkip; - try { - response = (await Promise.race([ - withTimeout( - info.connection.extMethod( - SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, - { name, config, originatorClientId }, - ), - MCP_RESTART_TIMEOUT_MS, + const response = (await Promise.race([ + withTimeout( + info.connection.extMethod( SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, + { name, config, originatorClientId }, ), - getChannelClosedReject(info), - ])) as AddOk | AddSkip; - } catch (err) { - // Re-instantiate structured ACP error payloads into typed bridge - // errors so `sendBridgeError` maps them to stable HTTP codes. - const data = (err as { data?: unknown })?.data; - if (data && typeof data === 'object') { - const kind = (data as { errorKind?: unknown }).errorKind; - if (kind === 'mcp_budget_would_exceed') { - throw err; - } - if (kind === 'mcp_server_spawn_failed') { - throw err; - } - if (kind === 'invalid_config') { - throw err; - } - } - throw err; - } + MCP_RESTART_TIMEOUT_MS, + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeAdd, + ), + getChannelClosedReject(info), + ])) as AddOk | AddSkip; // Emit event on success (non-skip) const addSkipped = (response as { skipped?: boolean }).skipped === true; if (!addSkipped) { @@ -3768,17 +3748,29 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { originatorClientId: string; }; type RemoveSkip = { name: string; skipped: true; reason: 'not_present' }; - const response = (await Promise.race([ - withTimeout( - info.connection.extMethod( + let response: RemoveOk | RemoveSkip; + try { + response = (await Promise.race([ + withTimeout( + info.connection.extMethod( + SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove, + { name, originatorClientId }, + ), + MCP_RESTART_TIMEOUT_MS, SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove, - { name, originatorClientId }, ), - MCP_RESTART_TIMEOUT_MS, - SERVE_CONTROL_EXT_METHODS.workspaceMcpRuntimeRemove, - ), - getChannelClosedReject(info), - ])) as RemoveOk | RemoveSkip; + getChannelClosedReject(info), + ])) as RemoveOk | RemoveSkip; + } catch (err) { + const data = (err as { data?: unknown })?.data; + if (data && typeof data === 'object') { + const kind = (data as { errorKind?: unknown }).errorKind; + if (kind === 'acp_channel_unavailable') { + throw err; + } + } + throw err; + } // Emit event on success (non-skip) const removeSkipped = (response as { skipped?: boolean }).skipped === true; diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 880cba9df0d..f2dcec2ceff 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2504,7 +2504,7 @@ class QwenAgent implements Agent { 'Invalid or missing name', ); } - if (!config || typeof config !== 'object') { + if (!config || typeof config !== 'object' || Array.isArray(config)) { throw RequestError.invalidParams( undefined, 'Invalid or missing config', @@ -2528,11 +2528,17 @@ class QwenAgent implements Agent { } try { // Strip security-sensitive fields โ€” runtime-added servers must - // not bypass permission gates via trust:true from HTTP body - const { trust: _stripped, ...safeConfig } = config as Record< - string, - unknown - >; + // not bypass permission gates via trust:true, leak cloud creds + // via authProviderType, manipulate tool filtering, or spawn in + // arbitrary directories + const { + trust: _trust, + authProviderType: _auth, + includeTools: _inc, + excludeTools: _exc, + cwd: _cwd, + ...safeConfig + } = config as Record; const result = await manager.addRuntimeMcpServer( name, safeConfig as MCPServerConfig, diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 26b7717749c..8ad45fd0dab 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -3495,10 +3495,11 @@ function sendBridgeErrorImpl( if (data && typeof data === 'object') { const kind = (data as { errorKind?: unknown }).errorKind; if (kind === 'mcp_budget_would_exceed') { + const d = data as { serverName?: string }; res.status(409).json({ error: errorMessage(err), code: 'mcp_budget_would_exceed', - ...(data as object), + serverName: d.serverName, }); return; } @@ -3521,10 +3522,12 @@ function sendBridgeErrorImpl( return; } if (kind === 'invalid_config') { + const d = data as { serverName?: string; reason?: string }; res.status(400).json({ error: errorMessage(err), code: 'invalid_config', - ...(data as object), + serverName: d.serverName, + reason: d.reason, }); return; } @@ -3532,7 +3535,6 @@ function sendBridgeErrorImpl( res.status(503).json({ error: errorMessage(err), code: 'acp_channel_unavailable', - ...(data as object), }); return; } diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index 538e42a4fc2..eaf6ab3ac89 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -3373,11 +3373,11 @@ describe('McpClientManager โ€” addRuntimeMcpServer / removeRuntimeMcpServer (T2. 'client-4', ); - // pool.acquire should NOT have been re-called (idempotent replace) + // pool.acquire should NOT have been re-called (idempotent no-op) expect(acquireSpy).not.toHaveBeenCalled(); expect(result).toMatchObject({ name: 'dup-srv', - replaced: true, + replaced: false, toolCount: 3, }); }); diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 2dcd380f62b..4dd31a50aa2 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -2649,6 +2649,18 @@ export class McpClientManager { config: MCPServerConfig, originatorClientId: string, ): Promise { + // Reject explicitly excluded servers + if (this.cliConfig.isMcpServerDisabled(name)) { + throw new InvalidMcpConfigError( + name, + `server '${name}' is in excludedMcpServers and cannot be added at runtime`, + ); + } + + debugLogger.info( + `addRuntimeMcpServer: ${name} (transport=${mcpTransportOf(config)}, client=${originatorClientId})`, + ); + // Validate config minimally: must have at least one transport field const transport = mcpTransportOf(config); if (transport === 'unknown') { @@ -2668,14 +2680,13 @@ export class McpClientManager { const newConnId = connectionIdOf(name, config); const existingConn = this.pooledConnections.get(name); if (existingConn && existingConn.id === newConnId) { - // Same fingerprint โ€” just update the Config overlay (in case - // non-transport fields like trust/includeTools changed). + // Same fingerprint โ€” no transport churn, just update Config overlay this.cliConfig.addRuntimeMcpServer(name, config); const toolCount = existingConn.toolsSnapshot.length; return { name, transport, - replaced: true, + replaced: false, shadowedSettings, toolCount, originatorClientId, @@ -2812,14 +2823,28 @@ export class McpClientManager { this.releaseSlotName(name); } // Clean up any partial state (including tools from partial discover) + this.toolRegistry.removeMcpToolsByServer(name); this.pooledConnections.delete(name); + const failedClient = this.clients.get(name); + if (failedClient) { + try { + await failedClient.disconnect(); + } catch { + /* best effort */ + } + } this.clients.delete(name); - this.toolRegistry.removeMcpToolsByServer(name); this.stopHealthCheck(name); + this.eventEmitter?.emit('mcp-client-update', this.clients); const message = err instanceof Error ? err.message : String(err); const isTimeout = message.includes('timed out'); + const exitCode = + err instanceof Error && 'exitCode' in err + ? (err as { exitCode?: number }).exitCode + : undefined; throw new McpServerSpawnFailedError(name, { + exitCode, stderr: message, timeout: isTimeout, }); @@ -2860,7 +2885,7 @@ export class McpClientManager { const settingsServers = this.cliConfig.getSettingsMcpServers() ?? {}; const wasShadowingSettings = name in settingsServers; - // Release pool connection + // Release pool connection (identity-check prevents race with concurrent add) const poolConn = this.pooledConnections.get(name); if (poolConn) { try { @@ -2868,7 +2893,9 @@ export class McpClientManager { } catch { /* best effort */ } - this.pooledConnections.delete(name); + if (this.pooledConnections.get(name) === poolConn) { + this.pooledConnections.delete(name); + } } // Disconnect standalone client @@ -2883,10 +2910,13 @@ export class McpClientManager { this.eventEmitter?.emit('mcp-client-update', this.clients); } - // Cleanup: tool registry, status, health check (mirrors removeServer) + // Cleanup: tool registry, status, health check, diagnostics (mirrors removeServer) this.toolRegistry.removeMcpToolsByServer(name); removeMCPServerStatus(name); this.stopHealthCheck(name); + this.consecutiveFailures.delete(name); + this.isReconnecting.delete(name); + this.dropRefusalEntry(name); // Release budget slot const budget = this.pool?.getBudget(); diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 8b3bb7cf0ae..c43cf088812 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -1213,7 +1213,7 @@ export class DaemonClient { */ async addRuntimeMcpServer( request: DaemonRuntimeMcpAddRequest, - opts?: { clientId?: string }, + opts?: { clientId?: string; timeoutMs?: number }, ): Promise { return await this.fetchWithTimeout( `${this.baseUrl}/workspace/mcp/servers`, @@ -1231,6 +1231,7 @@ export class DaemonClient { } return (await res.json()) as DaemonRuntimeMcpAddResult; }, + opts?.timeoutMs ?? MCP_RESTART_DEFAULT_TIMEOUT_MS, ); } diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index 8196c1c07e9..41c090d2124 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -1342,6 +1342,7 @@ export function asKnownDaemonEvent( case 'followup_suggestion': return isFollowupSuggestionData(event.data) ? (event as DaemonFollowupSuggestionEvent) + : undefined; case 'mcp_server_added': return isMcpServerAddedData(event.data) ? (event as DaemonMcpServerAddedEvent) @@ -2419,6 +2420,9 @@ function isFollowupSuggestionData( isNonEmptyString(value['sessionId']) && isNonEmptyString(value['suggestion']) && isNonEmptyString(value['promptId']) + ); +} + function isMcpServerAddedData( value: unknown, ): value is DaemonMcpServerAddedData { From 66dc4ce1c86a36acfbbd9d2c4b781a4344db1211 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 28 May 2026 23:57:12 +0800 Subject: [PATCH 20/26] fix(serve): strip env field, add status cleanup and name validation (T2.8 #4514) Security: - Strip `env` from runtime-added MCP server configs (prevents NODE_OPTIONS/LD_PRELOAD injection via HTTP body) Correctness: - Add `removeMCPServerStatus(name)` in spawn-failure catch block (prevents stale CONNECTING entry in status registry) Hardening: - Add name validation (charset + length) to ACP ext-method handlers for both add and remove (matches HTTP route validation) --- packages/cli/src/acp-integration/acpAgent.ts | 13 +++++++++++++ packages/core/src/tools/mcp-client-manager.ts | 1 + 2 files changed, 14 insertions(+) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index f2dcec2ceff..c09c7abec3b 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2504,6 +2504,12 @@ class QwenAgent implements Agent { 'Invalid or missing name', ); } + if (name.length > 256 || !/^[A-Za-z0-9_-]+$/.test(name)) { + throw RequestError.invalidParams( + undefined, + 'Server name must be โ‰ค256 chars, alphanumeric + underscore/hyphen', + ); + } if (!config || typeof config !== 'object' || Array.isArray(config)) { throw RequestError.invalidParams( undefined, @@ -2537,6 +2543,7 @@ class QwenAgent implements Agent { includeTools: _inc, excludeTools: _exc, cwd: _cwd, + env: _env, ...safeConfig } = config as Record; const result = await manager.addRuntimeMcpServer( @@ -2578,6 +2585,12 @@ class QwenAgent implements Agent { 'Invalid or missing name', ); } + if (name.length > 256 || !/^[A-Za-z0-9_-]+$/.test(name)) { + throw RequestError.invalidParams( + undefined, + 'Server name must be โ‰ค256 chars, alphanumeric + underscore/hyphen', + ); + } if ( typeof originatorClientId !== 'string' || originatorClientId.length === 0 diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 4dd31a50aa2..3c95045347d 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -2825,6 +2825,7 @@ export class McpClientManager { // Clean up any partial state (including tools from partial discover) this.toolRegistry.removeMcpToolsByServer(name); this.pooledConnections.delete(name); + removeMCPServerStatus(name); const failedClient = this.clients.get(name); if (failedClient) { try { From 57166360051614569012de550aaf22562f574585 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Fri, 29 May 2026 12:11:51 +0800 Subject: [PATCH 21/26] fix(serve): strip oauth/headers, reject __proto__ names, fix remove timeout (T2.8 #4514) Security: - Strip `oauth` and `headers` from runtime-added configs (prevents credential exfiltration via OAuth flow and header injection) - Reject `__proto__`, `constructor`, `prototype` as server names (prevents prototype pollution when name becomes object key) SDK: - Add timeoutMs to removeRuntimeMcpServer (match add's 330s default) Docs: - Remove `env` from POST example (stripped by daemon since 66dc4ce1c) - Document stripped fields list --- docs/users/qwen-serve.md | 5 ++--- packages/cli/src/acp-integration/acpAgent.ts | 22 +++++++++++++++---- .../sdk-typescript/src/daemon/DaemonClient.ts | 3 ++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 5a32f315dbc..8b6a3ffebc5 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -522,13 +522,12 @@ Request: "name": "my-server", "config": { "command": "npx", - "args": ["-y", "@my-org/mcp-server"], - "env": { "TOKEN": "..." } + "args": ["-y", "@my-org/mcp-server"] } } ``` -`name` must be alphanumeric plus `_` and `-` (max 256 characters). `config` is the same MCP server configuration object used in `settings.json` `mcpServers` entries (transport-dependent fields: `command`/`args`/`env` for stdio, `url` for SSE/HTTP). +`name` must be alphanumeric plus `_` and `-` (max 256 characters). `config` is the same MCP server configuration object used in `settings.json` `mcpServers` entries (transport-dependent fields: `command`/`args` for stdio, `url` for SSE/HTTP). Security-sensitive fields (`trust`, `env`, `cwd`, `oauth`, `headers`, `authProviderType`) are stripped by the daemon and ignored. Response (200) โ€” success: diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index c09c7abec3b..3ef34534f3c 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2504,10 +2504,16 @@ class QwenAgent implements Agent { 'Invalid or missing name', ); } - if (name.length > 256 || !/^[A-Za-z0-9_-]+$/.test(name)) { + if ( + name.length > 256 || + !/^[A-Za-z0-9_-]+$/.test(name) || + name === '__proto__' || + name === 'constructor' || + name === 'prototype' + ) { throw RequestError.invalidParams( undefined, - 'Server name must be โ‰ค256 chars, alphanumeric + underscore/hyphen', + 'Server name must be โ‰ค256 chars, alphanumeric + underscore/hyphen, and not a reserved JS property name', ); } if (!config || typeof config !== 'object' || Array.isArray(config)) { @@ -2544,6 +2550,8 @@ class QwenAgent implements Agent { excludeTools: _exc, cwd: _cwd, env: _env, + oauth: _oauth, + headers: _headers, ...safeConfig } = config as Record; const result = await manager.addRuntimeMcpServer( @@ -2585,10 +2593,16 @@ class QwenAgent implements Agent { 'Invalid or missing name', ); } - if (name.length > 256 || !/^[A-Za-z0-9_-]+$/.test(name)) { + if ( + name.length > 256 || + !/^[A-Za-z0-9_-]+$/.test(name) || + name === '__proto__' || + name === 'constructor' || + name === 'prototype' + ) { throw RequestError.invalidParams( undefined, - 'Server name must be โ‰ค256 chars, alphanumeric + underscore/hyphen', + 'Server name must be โ‰ค256 chars, alphanumeric + underscore/hyphen, and not a reserved JS property name', ); } if ( diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index c43cf088812..eb9e1b404ee 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -1246,7 +1246,7 @@ export class DaemonClient { */ async removeRuntimeMcpServer( name: string, - opts?: { clientId?: string }, + opts?: { clientId?: string; timeoutMs?: number }, ): Promise { return await this.fetchWithTimeout( `${this.baseUrl}/workspace/mcp/servers/${encodeURIComponent(name)}`, @@ -1263,6 +1263,7 @@ export class DaemonClient { } return (await res.json()) as DaemonRuntimeMcpRemoveResult; }, + opts?.timeoutMs ?? MCP_RESTART_DEFAULT_TIMEOUT_MS, ); } From d2c668c2c75de8faae1f6f58acc9215985af04e7 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Fri, 29 May 2026 13:44:43 +0800 Subject: [PATCH 22/26] fix(serve): strip type field, add __proto__ rejection to HTTP routes (T2.8 #4514) Security: - Strip `type` from runtime config (prevents SDK transport routing bypass) - Add __proto__/constructor/prototype rejection to HTTP POST route (ACP handlers already had this; HTTP routes were missing it) Docs: - Add includeTools, excludeTools, type to stripped-fields list --- docs/users/qwen-serve.md | 2 +- packages/cli/src/acp-integration/acpAgent.ts | 1 + packages/cli/src/serve/server.ts | 9 +++++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 8b6a3ffebc5..4026d5c1f34 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -527,7 +527,7 @@ Request: } ``` -`name` must be alphanumeric plus `_` and `-` (max 256 characters). `config` is the same MCP server configuration object used in `settings.json` `mcpServers` entries (transport-dependent fields: `command`/`args` for stdio, `url` for SSE/HTTP). Security-sensitive fields (`trust`, `env`, `cwd`, `oauth`, `headers`, `authProviderType`) are stripped by the daemon and ignored. +`name` must be alphanumeric plus `_` and `-` (max 256 characters). `config` is the same MCP server configuration object used in `settings.json` `mcpServers` entries (transport-dependent fields: `command`/`args` for stdio, `url` for SSE/HTTP). Security-sensitive fields (`trust`, `env`, `cwd`, `oauth`, `headers`, `authProviderType`, `includeTools`, `excludeTools`, `type`) are stripped by the daemon and ignored. Response (200) โ€” success: diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 3ef34534f3c..bf2a4a9dc9c 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2552,6 +2552,7 @@ class QwenAgent implements Agent { env: _env, oauth: _oauth, headers: _headers, + type: _type, ...safeConfig } = config as Record; const result = await manager.addRuntimeMcpServer( diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 8ad45fd0dab..57412a9b288 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -2039,10 +2039,15 @@ export function createServeApp( }); return; } - if (!/^[A-Za-z0-9_-]+$/.test(name)) { + if ( + !/^[A-Za-z0-9_-]+$/.test(name) || + name === '__proto__' || + name === 'constructor' || + name === 'prototype' + ) { res.status(400).json({ error: - 'Server name must contain only alphanumeric characters, underscores, and hyphens', + 'Server name must contain only alphanumeric characters, underscores, and hyphens, and must not be a reserved JS property name', code: 'invalid_server_name', }); return; From eb01a577f7d2aa6f8bfd81e50010d3b56a772c7d Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Fri, 29 May 2026 15:19:30 +0800 Subject: [PATCH 23/26] fix(serve): add name validation + __proto__ guard to DELETE route (T2.8 #4514) --- packages/cli/src/serve/server.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 57412a9b288..a5d979061bd 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -2024,10 +2024,15 @@ export function createServeApp( mutate({ strict: true }), async (req, res) => { const name = req.params['name'] ?? ''; - // Validate name: must be non-empty string, alphanumeric + _ and - - if (name.length === 0) { + if ( + name.length === 0 || + !/^[A-Za-z0-9_-]+$/.test(name) || + name === '__proto__' || + name === 'constructor' || + name === 'prototype' + ) { res.status(400).json({ - error: 'Server name is required and must be a non-empty string', + error: 'Invalid server name', code: 'invalid_server_name', }); return; From 4904da200f003bf082196db17eb106c4585b19f7 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Fri, 29 May 2026 16:00:35 +0800 Subject: [PATCH 24/26] fix(serve): remove dead code in DELETE route validation (T2.8 #4514) --- packages/cli/src/serve/server.ts | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index a5d979061bd..f1ccf47c8df 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -2037,26 +2037,6 @@ export function createServeApp( }); return; } - if (name.length > MAX_SERVER_NAME_LENGTH) { - res.status(400).json({ - error: `Server name exceeds ${MAX_SERVER_NAME_LENGTH}-character limit`, - code: 'invalid_server_name', - }); - return; - } - if ( - !/^[A-Za-z0-9_-]+$/.test(name) || - name === '__proto__' || - name === 'constructor' || - name === 'prototype' - ) { - res.status(400).json({ - error: - 'Server name must contain only alphanumeric characters, underscores, and hyphens, and must not be a reserved JS property name', - code: 'invalid_server_name', - }); - return; - } // Validate client identity (required for runtime MCP mutation) const clientId = parseAndValidateWorkspaceClientId(req, res, bridge); if (clientId === null) return; From ed4924300c92843c61259770edb74b7fba6a2650 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Fri, 29 May 2026 16:50:09 +0800 Subject: [PATCH 25/26] fix(serve): restore MAX_SERVER_NAME_LENGTH in DELETE, add __proto__ to POST (T2.8 #4514) --- packages/cli/src/serve/server.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index f1ccf47c8df..ecc352d4752 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -1968,10 +1968,15 @@ export function createServeApp( }); return; } - if (!/^[A-Za-z0-9_-]+$/.test(name)) { + if ( + !/^[A-Za-z0-9_-]+$/.test(name) || + name === '__proto__' || + name === 'constructor' || + name === 'prototype' + ) { res.status(400).json({ error: - 'Server name must contain only alphanumeric characters, underscores, and hyphens', + 'Server name must contain only alphanumeric characters, underscores, and hyphens, and must not be a reserved JS property name', code: 'invalid_server_name', }); return; @@ -2026,6 +2031,7 @@ export function createServeApp( const name = req.params['name'] ?? ''; if ( name.length === 0 || + name.length > MAX_SERVER_NAME_LENGTH || !/^[A-Za-z0-9_-]+$/.test(name) || name === '__proto__' || name === 'constructor' || From 633735b9bcc67c478f7b88a185550ea3fbec1edb Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Sat, 30 May 2026 09:22:03 +0800 Subject: [PATCH 26/26] fix(serve): split validation into precise error messages + add test coverage (T2.8 #4514) Split combined regex + reserved-name validation into separate checks with distinct error messages on both POST and DELETE routes. Added tests for __proto__/constructor/prototype rejection on POST, and MAX_SERVER_NAME_LENGTH + reserved-name rejection on DELETE. --- packages/cli/src/serve/server.test.ts | 47 +++++++++++++++++++++++++++ packages/cli/src/serve/server.ts | 39 ++++++++++++++++++---- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index e03147a738a..b49b981c3e0 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -3418,6 +3418,23 @@ describe('createServeApp', () => { expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); }); + it('400 invalid_server_name when name is a reserved JS property', async () => { + for (const name of ['__proto__', 'constructor', 'prototype']) { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth( + request(app).post('/workspace/mcp/servers'), + ).send({ + name, + config: { command: 'echo' }, + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(res.body.error).toContain('reserved'); + expect(bridge.addRuntimeMcpServerCalls).toHaveLength(0); + } + }); + it('400 missing_required_field when config is absent', async () => { const bridge = fakeBridge(); const app = createServeApp(tokenOpts, undefined, { bridge }); @@ -3606,6 +3623,36 @@ describe('createServeApp', () => { expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(0); }); + it('400 invalid_server_name when name exceeds MAX_SERVER_NAME_LENGTH', async () => { + const bridge = fakeBridge({ knownClientIds: ['client-1'] }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const overlong = 'a'.repeat(257); + const res = await auth( + request(app).delete(`/workspace/mcp/servers/${overlong}`), + ) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(0); + }); + + it('400 invalid_server_name when name is a reserved JS property', async () => { + for (const name of ['__proto__', 'constructor', 'prototype']) { + const bridge = fakeBridge({ knownClientIds: ['client-1'] }); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth( + request(app).delete(`/workspace/mcp/servers/${name}`), + ) + .set('X-Qwen-Client-Id', 'client-1') + .send(); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_server_name'); + expect(res.body.error).toContain('reserved'); + expect(bridge.removeRuntimeMcpServerCalls).toHaveLength(0); + } + }); + it('401 auth_required when no bearer token (strict gate)', async () => { const bridge = fakeBridge(); const app = createServeApp(baseOpts, undefined, { bridge }); diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index ecc352d4752..4108a61b77f 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -1968,15 +1968,21 @@ export function createServeApp( }); return; } + if (!/^[A-Za-z0-9_-]+$/.test(name)) { + res.status(400).json({ + error: + 'Server name must contain only alphanumeric characters, underscores, and hyphens', + code: 'invalid_server_name', + }); + return; + } if ( - !/^[A-Za-z0-9_-]+$/.test(name) || name === '__proto__' || name === 'constructor' || name === 'prototype' ) { res.status(400).json({ - error: - 'Server name must contain only alphanumeric characters, underscores, and hyphens, and must not be a reserved JS property name', + error: 'Server name must not be a reserved JS property name', code: 'invalid_server_name', }); return; @@ -2029,16 +2035,35 @@ export function createServeApp( mutate({ strict: true }), async (req, res) => { const name = req.params['name'] ?? ''; + if (name.length === 0) { + res.status(400).json({ + error: 'Server name is required', + code: 'invalid_server_name', + }); + return; + } + if (name.length > MAX_SERVER_NAME_LENGTH) { + res.status(400).json({ + error: `Server name exceeds ${MAX_SERVER_NAME_LENGTH}-character limit`, + code: 'invalid_server_name', + }); + return; + } + if (!/^[A-Za-z0-9_-]+$/.test(name)) { + res.status(400).json({ + error: + 'Server name must contain only alphanumeric characters, underscores, and hyphens', + code: 'invalid_server_name', + }); + return; + } if ( - name.length === 0 || - name.length > MAX_SERVER_NAME_LENGTH || - !/^[A-Za-z0-9_-]+$/.test(name) || name === '__proto__' || name === 'constructor' || name === 'prototype' ) { res.status(400).json({ - error: 'Invalid server name', + error: 'Server name must not be a reserved JS property name', code: 'invalid_server_name', }); return;