diff --git a/apps/cli/src/main.ts b/apps/cli/src/main.ts index 7fb6dc792..215ea4ca2 100644 --- a/apps/cli/src/main.ts +++ b/apps/cli/src/main.ts @@ -1494,7 +1494,7 @@ const mcpCommand = Command.make( { scope, elicitationMode: Options.choice("elicitation-mode", ["browser", "model"] as const) - .pipe(Options.withDefault("browser")) + .pipe(Options.withDefault("model")) .pipe( Options.withDescription( "Choose the stdio approval flow: browser approval or a CLI resume tool exposed to the model.", diff --git a/apps/cloud/src/mcp-session.ts b/apps/cloud/src/mcp-session.ts index f3f7780c9..25a55c635 100644 --- a/apps/cloud/src/mcp-session.ts +++ b/apps/cloud/src/mcp-session.ts @@ -4,7 +4,7 @@ import { DurableObject, env } from "cloudflare:workers"; import { createTraceState } from "@opentelemetry/api"; -import { Cause, Data, Effect, Layer } from "effect"; +import { Cause, Data, Deferred, Effect, Layer } from "effect"; import * as OtelTracer from "@effect/opentelemetry/Tracer"; import type * as Tracer from "effect/Tracer"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; @@ -264,6 +264,7 @@ export class McpSessionDO extends DurableObject { private sessionMeta: SessionMeta | null = null; private transportJsonResponseMode: boolean | null = null; private approvalResponses = new Map(); + private approvalWaiters = new Map>(); // Updated at the start of each `handleRequest` so the host-mcp server's // `parentSpan` getter — invoked by the MCP SDK's deferred tool callbacks // after `transport.handleRequest()` has already returned its streaming @@ -363,29 +364,15 @@ export class McpSessionDO extends DurableObject { // `Effect.runPromise(engine.getDescription)` at its async // MCP-SDK boundary and orphan the sub-span. const description = yield* buildExecuteDescription(executor); - const sessionElicitationMode = - sessionMeta.elicitationMode ?? (sessionMeta.allowModelResume ? "model" : "browser"); + const sessionElicitationMode = sessionMeta.elicitationMode ?? "model"; const mcpServer = yield* createExecutorMcpServer({ engine, description, parentSpan: () => self.currentRequestSpan ?? undefined, debug: env.EXECUTOR_MCP_DEBUG === "true", browserApprovalStore: { - takeResponse: (executionId) => - Effect.promise(async () => { - const memoryResponse = self.approvalResponses.get(executionId); - if (memoryResponse) { - self.approvalResponses.delete(executionId); - await self.ctx.storage.delete(approvalResponseKey(executionId)); - return memoryResponse; - } - const stored = await self.ctx.storage.get( - approvalResponseKey(executionId), - ); - if (!stored) return null; - await self.ctx.storage.delete(approvalResponseKey(executionId)); - return stored; - }), + takeResponse: (executionId) => self.takeApprovalResponse(executionId), + waitForResponse: (executionId) => self.waitForApprovalResponse(executionId), }, elicitationMode: sessionElicitationMode === "browser" @@ -595,7 +582,7 @@ export class McpSessionDO extends DurableObject { return yield* resolveSessionMeta( token.organizationId, token.userId, - token.elicitationMode ?? (token.allowModelResume ? "model" : "browser"), + token.elicitationMode ?? "model", ).pipe( Effect.provide(makeResolveOrganizationServices(dbHandle)), Effect.tap((sessionMeta) => @@ -762,6 +749,44 @@ export class McpSessionDO extends DurableObject { ); } + private takeApprovalResponse(executionId: string): Effect.Effect { + const self = this; + return Effect.promise(async () => { + const memoryResponse = self.approvalResponses.get(executionId); + if (memoryResponse) { + self.approvalResponses.delete(executionId); + await self.ctx.storage.delete(approvalResponseKey(executionId)); + return memoryResponse; + } + const stored = await self.ctx.storage.get(approvalResponseKey(executionId)); + if (!stored) return null; + await self.ctx.storage.delete(approvalResponseKey(executionId)); + return stored; + }); + } + + private waitForApprovalResponse(executionId: string): Effect.Effect { + const self = this; + return Effect.gen(function* () { + const existing = yield* self.takeApprovalResponse(executionId); + if (existing) return existing; + + const waiter = + self.approvalWaiters.get(executionId) ?? (yield* Deferred.make()); + self.approvalWaiters.set(executionId, waiter); + yield* Deferred.await(waiter).pipe( + Effect.ensuring( + Effect.sync(() => { + if (self.approvalWaiters.get(executionId) === waiter) { + self.approvalWaiters.delete(executionId); + } + }), + ), + ); + return yield* self.takeApprovalResponse(executionId); + }); + } + async resumeExecutionForApproval( executionId: string, identity: McpSessionApprovalIdentity, @@ -784,6 +809,8 @@ export class McpSessionDO extends DurableObject { yield* Effect.promise(() => self.ctx.storage.put(approvalResponseKey(executionId), response), ); + const waiter = self.approvalWaiters.get(executionId); + if (waiter) yield* Deferred.succeed(waiter, response); return resumeApprovalResult(executionId, response); }).pipe( Effect.withSpan("McpSessionDO.resumeExecutionForApproval", { diff --git a/apps/cloud/src/mcp.ts b/apps/cloud/src/mcp.ts index 55b46c344..c5ed3bac4 100644 --- a/apps/cloud/src/mcp.ts +++ b/apps/cloud/src/mcp.ts @@ -556,7 +556,7 @@ const readElicitationMode = (request: Request): McpElicitationMode => { return "model"; } - return "browser"; + return "model"; }; /** diff --git a/apps/cloud/src/services/mcp-worker-transport.test.ts b/apps/cloud/src/services/mcp-worker-transport.test.ts index 4a3d5031f..4a994172c 100644 --- a/apps/cloud/src/services/mcp-worker-transport.test.ts +++ b/apps/cloud/src/services/mcp-worker-transport.test.ts @@ -65,6 +65,94 @@ describe("JsonRpcRequestIdQueue", () => { release(); }); + it("does not let a waiting resume block an unrelated same-id tool call", async () => { + const queue = new JsonRpcRequestIdQueue(); + let releaseResume!: () => void; + const hungResume = new Promise((resolve) => { + releaseResume = resolve; + }); + + const resumeStarted = new Promise((resolve) => { + queue.run( + jsonRpcRequest({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "resume", arguments: { executionId: "exec_1" } }, + }), + async () => { + resolve(); + await hungResume; + }, + ); + }); + await resumeStarted; + + const executeDone = await Promise.race([ + queue + .run( + jsonRpcRequest({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "execute", arguments: { code: "return 1" } }, + }), + async () => "done", + ) + .then((v) => ({ kind: "settled" as const, v })), + new Promise<{ kind: "blocked" }>((r) => setTimeout(() => r({ kind: "blocked" }), 200)), + ]); + + expect(executeDone.kind).toBe("settled"); + releaseResume(); + }); + + it("serialises same-id resume calls for the same execution", async () => { + const queue = new JsonRpcRequestIdQueue(); + const order: string[] = []; + + let releaseFirst!: () => void; + const firstStarted = new Promise((resolve) => { + const firstRunning = new Promise((release) => { + releaseFirst = release; + }); + queue.run( + jsonRpcRequest({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "resume", arguments: { executionId: "exec_1" } }, + }), + async () => { + order.push("first:start"); + resolve(); + await firstRunning; + order.push("first:end"); + }, + ); + }); + await firstStarted; + + const secondDone = queue.run( + jsonRpcRequest({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "resume", arguments: { executionId: "exec_1" } }, + }), + async () => { + order.push("second"); + }, + ); + + await new Promise((r) => setTimeout(r, 20)); + expect(order).toEqual(["first:start"]); + + releaseFirst(); + await secondDone; + expect(order).toEqual(["first:start", "first:end", "second"]); + }); + it("regression: caps wait on a hung previous request and dispatches anyway", async () => { // Override the timeout for fast CI — the production default is // PREVIOUS_REQUEST_TIMEOUT_MS (60s) which we cap test-side to 100ms. diff --git a/apps/cloud/src/services/mcp-worker-transport.ts b/apps/cloud/src/services/mcp-worker-transport.ts index 285e58f13..3b77894ae 100644 --- a/apps/cloud/src/services/mcp-worker-transport.ts +++ b/apps/cloud/src/services/mcp-worker-transport.ts @@ -16,6 +16,16 @@ export type McpWorkerTransport = Readonly<{ type JsonRpcLike = { readonly id?: unknown; readonly method?: unknown; + readonly params?: unknown; +}; + +type ToolCallParamsLike = { + readonly name?: unknown; + readonly arguments?: unknown; +}; + +type ResumeArgumentsLike = { + readonly executionId?: unknown; }; type HandleRequestResult = { @@ -60,7 +70,24 @@ const jsonRpcRequestIdKey = (id: unknown): string | null => Option.getOrNull, ); -const extractJsonRpcRequestIdKeys = async (request: Request): Promise> => { +const jsonRpcRequestQueueKey = (message: JsonRpcLike): string | null => { + const idKey = jsonRpcRequestIdKey(message.id); + if (!idKey) return null; + + if (message.method !== "tools/call") return idKey; + if (!message.params || typeof message.params !== "object") return idKey; + + const params = message.params as ToolCallParamsLike; + if (params.name !== "resume") return idKey; + if (!params.arguments || typeof params.arguments !== "object") return idKey; + + const args = params.arguments as ResumeArgumentsLike; + if (typeof args.executionId !== "string" || args.executionId.length === 0) return idKey; + + return `${idKey}:tools/call:resume:${args.executionId}`; +}; + +const extractJsonRpcRequestQueueKeys = async (request: Request): Promise> => { if (request.method !== "POST") return []; const contentType = request.headers.get("content-type") ?? ""; if (!contentType.includes("application/json")) return []; @@ -74,7 +101,7 @@ const extractJsonRpcRequestIdKeys = async (request: Request): Promise(request: Request, run: () => Promise): Promise { - const ids = [...new Set(await extractJsonRpcRequestIdKeys(request))]; + const ids = [...new Set(await extractJsonRpcRequestQueueKeys(request))]; if (ids.length === 0) return await run(); const previous = ids.map((id) => this.inFlight.get(id)).filter(Predicate.isNotUndefined); diff --git a/apps/cloud/src/web/pages/setup-mcp.tsx b/apps/cloud/src/web/pages/setup-mcp.tsx index f76b1a663..ddd2d08cc 100644 --- a/apps/cloud/src/web/pages/setup-mcp.tsx +++ b/apps/cloud/src/web/pages/setup-mcp.tsx @@ -19,7 +19,7 @@ export const SetupMcpPage = () => { const navigate = useNavigate(); const [origin, setOrigin] = useState(null); const [advancedOpen, setAdvancedOpen] = useState(false); - const [elicitationMode, setElicitationMode] = useState("browser"); + const [elicitationMode, setElicitationMode] = useState("model"); useEffect(() => { setOrigin(window.location.origin); diff --git a/apps/local/src/server/mcp-browser-resume.test.ts b/apps/local/src/server/mcp-browser-resume.test.ts index e6678eaca..ccf8a1f89 100644 --- a/apps/local/src/server/mcp-browser-resume.test.ts +++ b/apps/local/src/server/mcp-browser-resume.test.ts @@ -140,6 +140,9 @@ const readApproval = (structured: unknown): { readonly executionId: string; read expect(record).not.toHaveProperty("interaction"); expect(typeof record.executionId).toBe("string"); expect(typeof record.approvalUrl).toBe("string"); + expect(record.resumePrompt).toBe( + "Return text to the user telling them to approve the action at this approvalUrl. Only after you have prompted the user, call the `resume` tool with this executionId; `resume` will wait for the user's browser decision.", + ); const { executionId, approvalUrl } = record as { readonly executionId: string; readonly approvalUrl: string; @@ -166,7 +169,10 @@ describe("local MCP browser approval resume", () => { { name: "browser-resume-test-client", version: "1.0.0" }, { capabilities: {} }, ); - const transport = new StreamableHTTPClientTransport(new URL("/mcp", TEST_BASE_URL), { fetch }); + const transport = new StreamableHTTPClientTransport( + new URL("/mcp?elicitation_mode=browser", TEST_BASE_URL), + { fetch }, + ); await mcpClient.connect(transport); @@ -231,6 +237,11 @@ const approveInBrowserThenResume = async ( const sessionId = approval.url.searchParams.get("mcp_session_id"); expect(sessionId).not.toBeNull(); + const resume = client.callTool({ + name: "resume", + arguments: { executionId: approval.executionId }, + }); + const approvalResponse = await fetch( new URL( `/api/mcp-sessions/${encodeURIComponent(sessionId!)}/executions/${encodeURIComponent(approval.executionId)}/resume`, @@ -247,8 +258,5 @@ const approveInBrowserThenResume = async ( ); expect(approvalResponse.status).toBe(200); - return await client.callTool({ - name: "resume", - arguments: { executionId: approval.executionId }, - }); + return await resume; }; diff --git a/apps/local/src/server/mcp.ts b/apps/local/src/server/mcp.ts index 000f05508..b6c052cd8 100644 --- a/apps/local/src/server/mcp.ts +++ b/apps/local/src/server/mcp.ts @@ -1,4 +1,4 @@ -import { Effect, Option, Schema } from "effect"; +import { Deferred, Effect, Option, Schema } from "effect"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js"; @@ -46,7 +46,7 @@ const readElicitationMode = (request: Request): McpElicitationMode => { return mode as McpElicitationMode; } - return "browser"; + return "model"; }; const approvalUrlForRequest = ( @@ -115,6 +115,7 @@ export const createMcpRequestHandler = (config: ExecutorMcpServerConfig): McpReq const transports = new Map(); const servers = new Map(); const approvalResponses = new Map>(); + const approvalWaiters = new Map>>(); const dispose = async (id: string, opts: { transport?: boolean; server?: boolean } = {}) => { const t = transports.get(id); @@ -122,6 +123,7 @@ export const createMcpRequestHandler = (config: ExecutorMcpServerConfig): McpReq transports.delete(id); servers.delete(id); approvalResponses.delete(id); + approvalWaiters.delete(id); if (opts.transport) await ignoreClose(t ? () => t.close() : undefined); if (opts.server) await ignoreClose(s ? () => s.close() : undefined); }; @@ -169,6 +171,38 @@ export const createMcpRequestHandler = (config: ExecutorMcpServerConfig): McpReq sessionApprovals?.delete(executionId); return response; }), + waitForResponse: (executionId) => + Effect.gen(function* () { + if (!createdSessionId) return null; + const sessionApprovals = approvalResponses.get(createdSessionId); + const response = sessionApprovals?.get(executionId) ?? null; + if (response) { + sessionApprovals?.delete(executionId); + return response; + } + + const sessionWaiters = + approvalWaiters.get(createdSessionId) ?? + new Map>(); + const waiter = + sessionWaiters.get(executionId) ?? (yield* Deferred.make()); + sessionWaiters.set(executionId, waiter); + approvalWaiters.set(createdSessionId, sessionWaiters); + + yield* Deferred.await(waiter).pipe( + Effect.ensuring( + Effect.sync(() => { + if (sessionWaiters.get(executionId) === waiter) { + sessionWaiters.delete(executionId); + } + }), + ), + ); + const approvals = approvalResponses.get(createdSessionId); + const approved = approvals?.get(executionId) ?? null; + approvals?.delete(executionId); + return approved; + }), }, elicitationMode: elicitationMode === "browser" @@ -217,6 +251,8 @@ export const createMcpRequestHandler = (config: ExecutorMcpServerConfig): McpReq approvalResponses.get(sessionId) ?? new Map(); sessionApprovals.set(executionId, response); approvalResponses.set(sessionId, sessionApprovals); + const waiter = approvalWaiters.get(sessionId)?.get(executionId); + if (waiter) await Effect.runPromise(Deferred.succeed(waiter, response)); return json(resumeApprovalResult(executionId, response)); }, diff --git a/packages/hosts/mcp/src/server.test.ts b/packages/hosts/mcp/src/server.test.ts index 8814352a8..8afef4ada 100644 --- a/packages/hosts/mcp/src/server.test.ts +++ b/packages/hosts/mcp/src/server.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; -import { Data, Effect } from "effect"; +import { Data, Deferred, Effect } from "effect"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { ElicitRequestSchema } from "@modelcontextprotocol/sdk/types.js"; @@ -437,7 +437,7 @@ describe("MCP host server — client without elicitation (pause/resume)", () => }); }); - it("resume tool requires user approval by default", async () => { + it("browser approval mode requires user approval before resume", async () => { let resumeCalled = false; const engine = makeStubEngine({ resume: () => @@ -460,10 +460,15 @@ describe("MCP host server — client without elicitation (pause/resume)", () => expect(result.isError).toBeFalsy(); expect(textOf(result)).toContain("User approval required"); expect(textOf(result)).toContain("https://executor.test/resume/exec_1"); + expect(textOf(result)).toContain( + "Return text to the user telling them to approve the action at this approvalUrl. Only after you have prompted the user, call the `resume` tool with this executionId; `resume` will wait for the user's browser decision.", + ); expect(result.structuredContent).toMatchObject({ status: "user_approval_required", executionId: "exec_1", approvalUrl: "https://executor.test/resume/exec_1", + resumePrompt: + "Return text to the user telling them to approve the action at this approvalUrl. Only after you have prompted the user, call the `resume` tool with this executionId; `resume` will wait for the user's browser decision.", }); }, { @@ -477,6 +482,9 @@ describe("MCP host server — client without elicitation (pause/resume)", () => it("browser approval mode consumes a user-approved response and returns the resumed result", async () => { const approved = new Map }>(); + const waiter = await Effect.runPromise( + Deferred.make<{ action: "accept"; content?: Record }>(), + ); const engine = makeStubEngine({ resume: (executionId, response) => Effect.succeed( @@ -490,20 +498,14 @@ describe("MCP host server — client without elicitation (pause/resume)", () => engine, NO_CAPS, async (client) => { - const waiting = await client.callTool({ - name: "resume", - arguments: { executionId: "exec_1" }, - }); - expect(waiting.structuredContent).toMatchObject({ - status: "user_approval_required", - executionId: "exec_1", - }); - - approved.set("exec_1", { action: "accept", content: {} }); - const resumed = await client.callTool({ + const waiting = client.callTool({ name: "resume", arguments: { executionId: "exec_1" }, }); + const response = { action: "accept" as const, content: {} }; + approved.set("exec_1", response); + await Effect.runPromise(Deferred.succeed(waiter, response)); + const resumed = await waiting; expect(resumed.content).toEqual([{ type: "text", text: "resumed-after-browser" }]); expect(resumed.structuredContent).toMatchObject({ status: "completed", @@ -517,12 +519,18 @@ describe("MCP host server — client without elicitation (pause/resume)", () => }, browserApprovalStore: { takeResponse: (executionId) => Effect.succeed(approved.get(executionId) ?? null), + waitForResponse: (executionId) => + Effect.gen(function* () { + const response = approved.get(executionId); + if (response) return response; + return yield* Deferred.await(waiter); + }), }, }, ); }); - it("model resume mode paused execution returns interaction metadata with executionId", async () => { + it("default model resume mode paused execution returns interaction metadata with executionId", async () => { const engine = makeStubEngine({ executeWithPause: () => Effect.succeed( @@ -539,24 +547,19 @@ describe("MCP host server — client without elicitation (pause/resume)", () => ), }); - await withClient( - engine, - NO_CAPS, - async (client) => { - const result = await client.callTool({ - name: "execute", - arguments: { code: "pause-me" }, - }); - expect(textOf(result)).toContain("exec_42"); - expect(textOf(result)).toContain("Need approval"); - expect(result.isError).toBeFalsy(); + await withClient(engine, NO_CAPS, async (client) => { + const result = await client.callTool({ + name: "execute", + arguments: { code: "pause-me" }, + }); + expect(textOf(result)).toContain("exec_42"); + expect(textOf(result)).toContain("Need approval"); + expect(result.isError).toBeFalsy(); - const structured = result.structuredContent as Record; - expect(structured?.executionId).toBe("exec_42"); - expect(structured?.status).toBe("waiting_for_interaction"); - }, - { elicitationMode: { mode: "model" } }, - ); + const structured = result.structuredContent as Record; + expect(structured?.executionId).toBe("exec_42"); + expect(structured?.status).toBe("waiting_for_interaction"); + }); }); it("resume tool completes a paused execution when model resume is explicitly enabled", async () => { diff --git a/packages/hosts/mcp/src/server.ts b/packages/hosts/mcp/src/server.ts index 7f69add75..12e0201a7 100644 --- a/packages/hosts/mcp/src/server.ts +++ b/packages/hosts/mcp/src/server.ts @@ -72,9 +72,9 @@ type SharedMcpServerConfig = { */ readonly debug?: boolean; /** - * Controls how elicitation is handled for this MCP connection. The secure - * default is browser approval: `execute` pauses, and `resume` returns a URL - * for the signed-in user instead of letting the model answer directly. + * Controls how elicitation is handled for this MCP connection. The default + * is model-managed resume, where paused executions expose interaction + * metadata and the model can call `resume` with the user's response. */ readonly elicitationMode?: | { @@ -98,6 +98,7 @@ export type ExecutorMcpServerConfig Effect.Effect; + readonly waitForResponse?: (executionId: string) => Effect.Effect; }; // --------------------------------------------------------------------------- @@ -291,6 +292,9 @@ const newCorrelationId = (): string => const defaultResumeApprovalUrl = (executionId: string): string => `/resume/${encodeURIComponent(executionId)}`; +const browserApprovalReturnPrompt = + "Return text to the user telling them to approve the action at this approvalUrl. Only after you have prompted the user, call the `resume` tool with this executionId; `resume` will wait for the user's browser decision."; + const formatResumeApprovalRequired = (input: { readonly executionId: string; readonly approvalUrl: string; @@ -304,7 +308,8 @@ const formatResumeApprovalRequired = (input: { "Tell the user to open this URL while signed in and approve or decline the paused interaction:", input.approvalUrl, "", - "Do not choose accept, decline, cancel, or response content on the user's behalf.", + "Required next steps for this agent:", + browserApprovalReturnPrompt, ].join("\n"), }, ], @@ -312,6 +317,7 @@ const formatResumeApprovalRequired = (input: { status: "user_approval_required", executionId: input.executionId, approvalUrl: input.approvalUrl, + resumePrompt: browserApprovalReturnPrompt, }, }); @@ -373,8 +379,7 @@ export const createExecutorMcpServer = ( const elicitationMode = config.elicitationMode ?? ({ - mode: "browser", - approvalUrl: defaultResumeApprovalUrl, + mode: "model", } as const); const resolveParentSpan = (): Tracer.AnySpan | undefined => { @@ -509,9 +514,23 @@ export const createExecutorMcpServer = ( return config.browserApprovalStore?.takeResponse(executionId) ?? Effect.succeed(null); }; + const waitForBrowserApprovalResponse = ( + executionId: string, + ): Effect.Effect => { + const waitForResponse = config.browserApprovalStore?.waitForResponse; + if (!waitForResponse) return takeBrowserApprovalResponse(executionId); + + return waitForResponse(executionId).pipe( + Effect.timeoutOrElse({ + duration: "10 minutes", + orElse: () => Effect.succeed(null), + }), + ); + }; + const resumeAfterBrowserApproval = (executionId: string): Effect.Effect => Effect.gen(function* () { - const response = yield* takeBrowserApprovalResponse(executionId); + const response = yield* waitForBrowserApprovalResponse(executionId); if (!response) return yield* requireUserResumeApproval(executionId); const outcome = yield* engine.resume(executionId, response); diff --git a/packages/react/src/components/mcp-install-card.test.ts b/packages/react/src/components/mcp-install-card.test.ts index e5671e699..ed173b13a 100644 --- a/packages/react/src/components/mcp-install-card.test.ts +++ b/packages/react/src/components/mcp-install-card.test.ts @@ -30,7 +30,7 @@ describe("MCP install command rendering", () => { ).toBe("npx add-mcp http://localhost:4788/mcp --transport http --name executor"); }); - it("uses browser approval by default and encodes explicit elicitation modes", () => { + it("uses model-managed resume by default and encodes explicit elicitation modes", () => { expect( buildMcpHttpEndpoint({ origin: "https://executor.example", @@ -43,10 +43,10 @@ describe("MCP install command rendering", () => { mode: "http", isDev: false, origin: "https://executor.example", - elicitationMode: "model", + elicitationMode: "browser", }), ).toBe( - "npx add-mcp 'https://executor.example/mcp?elicitation_mode=model' --transport http --name executor", + "npx add-mcp 'https://executor.example/mcp?elicitation_mode=browser' --transport http --name executor", ); expect( @@ -69,6 +69,17 @@ describe("MCP install command rendering", () => { origin: null, elicitationMode: "model", }), - ).toBe("npx add-mcp 'executor mcp --elicitation-mode model' --name executor"); + ).toBe("npx add-mcp 'executor mcp' --name executor"); + }); + + it("passes browser approval through stdio install commands when explicitly selected", () => { + expect( + buildMcpInstallCommand({ + mode: "stdio", + isDev: false, + origin: null, + elicitationMode: "browser", + }), + ).toBe("npx add-mcp 'executor mcp --elicitation-mode browser' --name executor"); }); }); diff --git a/packages/react/src/components/mcp-install-card.tsx b/packages/react/src/components/mcp-install-card.tsx index 2db493d88..1e71f9e55 100644 --- a/packages/react/src/components/mcp-install-card.tsx +++ b/packages/react/src/components/mcp-install-card.tsx @@ -59,7 +59,7 @@ export const buildMcpHttpEndpoint = (input: { : input.origin ? `${input.origin}/mcp` : "/mcp"; - if (!input.elicitationMode || input.elicitationMode === "browser") return endpoint; + if (!input.elicitationMode || input.elicitationMode === "model") return endpoint; if (endpoint.startsWith("<")) return `${endpoint}?elicitation_mode=${input.elicitationMode}`; const url = new URL(endpoint); @@ -109,10 +109,8 @@ export const buildMcpInstallCommand = (input: { if (input.scopeDir) { innerArgs.push("--scope", input.scopeDir); } - if (input.elicitationMode && input.elicitationMode !== "browser") { - if (input.elicitationMode !== "native") { - innerArgs.push("--elicitation-mode", input.elicitationMode); - } + if (input.elicitationMode && input.elicitationMode !== "model") { + innerArgs.push("--elicitation-mode", input.elicitationMode); } return `npx add-mcp ${shellQuoteWord(innerArgs.map(shellQuoteWord).join(" "))} --name executor`; }; @@ -125,7 +123,7 @@ export function McpInstallCard(props: { className?: string }) { const showStdio = isLocal && readDesktopBridge() === null; const [mode, setMode] = useState("http"); const [advancedOpen, setAdvancedOpen] = useState(false); - const [httpElicitationMode, setHttpElicitationMode] = useState("browser"); + const [httpElicitationMode, setHttpElicitationMode] = useState("model"); const [origin, setOrigin] = useState(null); const [desktop, setDesktop] = useState<{ readonly port: number;