From eace5645d44ad2172b0ed6d926886f0c97da519a Mon Sep 17 00:00:00 2001 From: TomasPalsson Date: Mon, 6 Jul 2026 14:30:07 +0000 Subject: [PATCH 01/10] feat: add MCP elicitation support (URL + form) with -32042 retry Surface MCP server-initiated elicitation during tool calls. When a tools/call returns JSON-RPC -32042 (UrlElicitationRequired, spec 2025-11-25) or the server sends elicitation/create, LibreChat holds the call open, prompts the user, and resolves/retries once they respond. Primary driver: AWS Bedrock AgentCore Gateway, which defers per-user OAuth to a browser flow and signals it via -32042. Backend: - Detect and extract -32042 from JSON-RPC error objects, SDK McpError, and HTTP-wrapped transport errors; accept only http(s) auth URLs (reject javascript:/data:/vbscript:) in both wire shapes. - Per-connection elicitation handler registry behind one stable dispatcher, so concurrent tool calls on a shared connection cannot orphan or misattribute each other's elicitation/create. - Cap concurrent pending elicitations per (user, server); tie the tool-call timeout to the flow TTL; thread the SDK abort signal so a server cancellation or connection close tears down the pending wait. - Gate per-server via an `elicitation` config flag (enabled by default); validate submitted content against the requested schema server-side; strip elicitation parts from the model-bound payload. - Emit on_elicitation and on_elicitation_resolved SSE events; add data-provider types, endpoints, and the completion route with per-user flow-ownership authorization. Covered by unit and integration tests for detection, single-retry semantics, the handler registry, the config gate, and the route authz matrix. --- api/server/controllers/agents/client.js | 11 + api/server/controllers/agents/openai.js | 4 +- api/server/controllers/agents/responses.js | 4 +- api/server/routes/__tests__/mcp.spec.js | 183 ++++++ api/server/routes/mcp.js | 186 +++++++ api/server/services/MCP.js | 133 ++++- api/server/services/__tests__/MCP.spec.js | 134 ++++- librechat.example.yaml | 3 + packages/api/src/index.ts | 1 + packages/api/src/mcp/MCPManager.ts | 240 +++++++- .../api/src/mcp/__tests__/MCPManager.test.ts | 520 ++++++++++++++++++ .../api/src/mcp/__tests__/elicitation.test.ts | 279 ++++++++++ .../helpers/mockElicitationServer.ts | 401 ++++++++++++++ .../src/mcp/__tests__/helpers/package.json | 3 + .../helpers/run-mock-elicitation-server.mjs | 51 ++ .../urlElicitation.integration.test.ts | 330 +++++++++++ packages/api/src/mcp/connection.ts | 130 ++++- packages/api/src/mcp/elicitation.ts | 218 ++++++++ packages/data-provider/src/api-endpoints.ts | 3 + packages/data-provider/src/data-service.ts | 16 + packages/data-provider/src/mcp.ts | 9 + packages/data-provider/src/types/agents.ts | 80 +++ .../data-provider/src/types/assistants.ts | 3 +- packages/data-provider/src/types/runs.ts | 35 ++ 24 files changed, 2955 insertions(+), 22 deletions(-) create mode 100644 packages/api/src/mcp/__tests__/elicitation.test.ts create mode 100644 packages/api/src/mcp/__tests__/helpers/mockElicitationServer.ts create mode 100644 packages/api/src/mcp/__tests__/helpers/package.json create mode 100644 packages/api/src/mcp/__tests__/helpers/run-mock-elicitation-server.mjs create mode 100644 packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts create mode 100644 packages/api/src/mcp/elicitation.ts diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index e073c964d76..dbdf5651b54 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -73,6 +73,7 @@ const { isAgentsEndpoint, isEphemeralAgentId, removeNullishValues, + stripUiOnlyContentParts, DEFAULT_MEMORY_MAX_INPUT_TOKENS, } = require('librechat-data-provider'); const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); @@ -1231,6 +1232,16 @@ class AgentClient extends BaseClient { : {}), } : undefined; + /** + * Strip UI-only content parts (elicitation cards) from the LLM payload + * before `formatAgentMessages` runs. Such parts are rendered and persisted + * for chat replay but carry no meaning for the model, so a persisted card + * must never leak into a completion request. Scoped to the payload (and the + * memory copy) — the persisted message and UI copy keep the card. + */ + payload = stripUiOnlyContentParts(payload); + this.memoryPayload = stripUiOnlyContentParts(this.memoryPayload); + let { messages: initialMessages, indexTokenCountMap, diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 9000c247087..6c761540d8a 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -7,6 +7,7 @@ const { PermissionBits, hasPermissions, AgentCapabilities, + stripUiOnlyContentParts, } = require('librechat-data-provider'); const { writeSSE, @@ -515,7 +516,8 @@ const OpenAIChatCompletionController = async (req, res) => { const openaiMessages = convertMessages(request.messages); const toolSet = buildToolSet(primaryConfig); - const formatted = formatAgentMessages(openaiMessages, {}, toolSet); + // Elicitation cards are UI-only; strip them so a persisted card never leaks to the model. + const formatted = formatAgentMessages(stripUiOnlyContentParts(openaiMessages), {}, toolSet); const formattedMessages = formatted.messages; const initialSummary = formatted.summary; let indexTokenCountMap = formatted.indexTokenCountMap; diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index c88545c3e6b..48f6b92c9ec 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -8,6 +8,7 @@ const { PermissionBits, hasPermissions, AgentCapabilities, + stripUiOnlyContentParts, } = require('librechat-data-provider'); const { createRun, @@ -621,7 +622,8 @@ const createResponse = async (req, res) => { const allMessages = [...previousMessages, ...inputMessages]; const toolSet = buildToolSet(primaryConfig); - const formatted = formatAgentMessages(allMessages, {}, toolSet); + // Elicitation cards are UI-only; strip them so a persisted card never leaks to the model. + const formatted = formatAgentMessages(stripUiOnlyContentParts(allMessages), {}, toolSet); const formattedMessages = formatted.messages; const initialSummary = formatted.summary; let indexTokenCountMap = formatted.indexTokenCountMap; diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 5323fa0d0e7..b3aa5842651 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -133,12 +133,16 @@ jest.mock('~/server/services/Config/mcp', () => ({ const mockResolveAllMcpConfigs = jest.fn().mockResolvedValue({}); const mockResolveMcpConfigNames = jest.fn().mockResolvedValue([]); +const mockGetElicitationFlowContext = jest.fn(); +const mockResolveElicitationFlow = jest.fn().mockResolvedValue(true); jest.mock('~/server/services/MCP', () => ({ getMCPSetupData: jest.fn(), resolveConfigServers: jest.fn().mockResolvedValue({}), resolveMcpConfigNames: (...args) => mockResolveMcpConfigNames(...args), resolveAllMcpConfigs: (...args) => mockResolveAllMcpConfigs(...args), getServerConnectionStatus: jest.fn(), + getElicitationFlowContext: (...args) => mockGetElicitationFlowContext(...args), + resolveElicitationFlow: (...args) => mockResolveElicitationFlow(...args), })); jest.mock('~/server/services/PluginService', () => ({ @@ -3325,4 +3329,183 @@ describe('MCP Routes', () => { expect(response.body).toEqual({ message: 'Deletion failed' }); }); }); + + describe('POST /elicitation/:flowId', () => { + const { getLogStores } = require('~/cache'); + + /** Real `parseElicitationFlowId` (via requireActual) parses `user:server:tool:nonce`. */ + const ownedFlowId = 'test-user-id:jira:create_issue:nonce-1'; + const otherUserFlowId = 'other-user-id:jira:create_issue:nonce-2'; + + const mockFlowManager = () => ({ + getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING', metadata: {} }), + completeFlow: jest.fn().mockResolvedValue(true), + }); + + beforeEach(() => { + mockGetElicitationFlowContext.mockReset().mockReturnValue(undefined); + mockResolveElicitationFlow.mockReset().mockResolvedValue(true); + getLogStores.mockReturnValue({}); + }); + + it('should return 401 when user is not authenticated', async () => { + currentUser = { role: 'user' }; + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'complete' }); + + expect(response.status).toBe(401); + expect(response.body).toEqual({ error: 'User not authenticated' }); + }); + + it('should return 400 for an unknown action', async () => { + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'bogus' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual({ error: 'Invalid action' }); + }); + + it('should return 403 when the flowId is owned by a different user', async () => { + const flowManager = mockFlowManager(); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(otherUserFlowId)}`) + .send({ action: 'complete' }); + + expect(response.status).toBe(403); + expect(response.body).toEqual({ error: 'Forbidden' }); + expect(flowManager.getFlowState).not.toHaveBeenCalled(); + }); + + it('should return 404 when the flow state is not found', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue(null); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'complete' }); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Flow not found' }); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 404 when completeFlow reports the flow is gone', async () => { + const flowManager = mockFlowManager(); + flowManager.completeFlow.mockResolvedValue(false); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'complete' }); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: 'Flow not found' }); + expect(mockResolveElicitationFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when submitted content violates the requested schema', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { priority: { type: 'string', enum: ['low', 'high'] } }, + required: ['priority'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { priority: 'urgent' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/priority/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when submitted content includes an unknown field', async () => { + const flowManager = mockFlowManager(); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + mockGetElicitationFlowContext.mockReturnValue({ + requestedSchema: { type: 'object', properties: { title: { type: 'string' } } }, + }); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { title: 'ok', injected: 'x' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/injected/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should complete the flow and emit resolution on the happy path', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { priority: { type: 'string', enum: ['low', 'high'] } }, + required: ['priority'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { priority: 'high' } }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ ok: true }); + expect(flowManager.completeFlow).toHaveBeenCalledWith(ownedFlowId, 'mcp_elicit', { + action: 'accept', + content: { priority: 'high' }, + }); + expect(mockResolveElicitationFlow).toHaveBeenCalledWith({ + flowId: ownedFlowId, + action: 'accept', + content: { priority: 'high' }, + }); + }); + + it('should accept a URL-mode complete with no schema', async () => { + const flowManager = mockFlowManager(); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'complete' }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ ok: true }); + expect(mockResolveElicitationFlow).toHaveBeenCalledWith({ + flowId: ownedFlowId, + action: 'complete', + content: undefined, + }); + }); + + it('should return 500 when completeFlow throws', async () => { + const flowManager = mockFlowManager(); + flowManager.completeFlow.mockRejectedValue(new Error('keyv down')); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'complete' }); + + expect(response.status).toBe(500); + expect(response.body).toEqual({ error: 'Failed to complete elicitation flow' }); + }); + }); }); diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index 1637b8f7e19..ff7a999ea5b 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -22,6 +22,7 @@ const { generateCheckAccess, validateOAuthSession, OAUTH_SESSION_COOKIE, + parseElicitationFlowId, } = require('@librechat/api'); const { createMCPServerController, @@ -42,6 +43,8 @@ const { resolveAllMcpConfigs, resolveConfigServers, getMCPSetupData, + getElicitationFlowContext, + resolveElicitationFlow, } = require('~/server/services/MCP'); const { requireJwtAuth, canAccessMCPServerResource } = require('~/server/middleware'); const { getUserPluginAuthValue } = require('~/server/services/PluginService'); @@ -68,6 +71,124 @@ const canAccessOAuthFlow = (flowId, userId) => { return parsed.userId === userId || parsed.userId === 'system'; }; +/** + * Elicitation flow IDs embed the requesting userId directly (unlike OAuth flow + * IDs, which are one-per-server; elicitation flows are one-per-tool-invocation + * — see `generateElicitationFlowId` in `@librechat/api`). This enforces the + * same per-user ownership OAuth flow routes do, so one user can't complete or + * observe another user's pending elicitation via a guessed/observed flowId. + */ +const canAccessElicitationFlow = (flowId, userId) => { + const parsed = parseElicitationFlowId(flowId); + if (!parsed) { + return false; + } + if (parsed.tenantId && parsed.tenantId !== getTenantId()) { + return false; + } + return parsed.userId === userId; +}; + +/** + * Validates a single submitted elicitation field value against its property + * schema (the MCP form-mode elicitation subset of JSON Schema). Returns an error + * message on the first violation, or `null` when the value conforms. + * @param {string} key + * @param {unknown} value + * @param {import('librechat-data-provider').Agents.ElicitationPropertySchema} property + * @returns {string | null} + */ +const validateElicitationField = (key, value, property) => { + const { type, enum: enumValues, minimum, maximum, minLength, maxLength } = property ?? {}; + + if (type === 'string') { + if (typeof value !== 'string') { + return `Field '${key}' must be a string`; + } + if (typeof minLength === 'number' && value.length < minLength) { + return `Field '${key}' must be at least ${minLength} characters`; + } + if (typeof maxLength === 'number' && value.length > maxLength) { + return `Field '${key}' must be at most ${maxLength} characters`; + } + } else if (type === 'number' || type === 'integer') { + if (typeof value !== 'number' || Number.isNaN(value)) { + return `Field '${key}' must be a number`; + } + if (type === 'integer' && !Number.isInteger(value)) { + return `Field '${key}' must be an integer`; + } + if (typeof minimum === 'number' && value < minimum) { + return `Field '${key}' must be >= ${minimum}`; + } + if (typeof maximum === 'number' && value > maximum) { + return `Field '${key}' must be <= ${maximum}`; + } + } else if (type === 'boolean' && typeof value !== 'boolean') { + return `Field '${key}' must be a boolean`; + } + + if (Array.isArray(enumValues) && enumValues.length > 0 && !enumValues.includes(value)) { + return `Field '${key}' must be one of: ${enumValues.join(', ')}`; + } + + return null; +}; + +/** + * Server-side guard for the elicitation completion route. The client validates + * form submissions in the UI, but that check is trivially bypassed by calling + * the API directly, so `content` is re-validated here against the flow's stored + * `requestedSchema` before it is forwarded to the MCP server. Rejects unknown + * keys and enforces per-field type/enum/range constraints; required-field + * presence is only enforced for a submitting `accept` action (a `decline` / + * `cancel` legitimately omits values). When the flow carries no schema (URL-mode + * elicitation, or a schema not threaded into flow metadata) there is nothing to + * validate against, so the content passes through unchanged. + * @param {unknown} content + * @param {import('librechat-data-provider').Agents.ElicitationSchema | undefined} requestedSchema + * @param {{ enforceRequired: boolean }} options + * @returns {string | null} An error message on violation, or `null` when valid. + */ +const validateElicitationContent = (content, requestedSchema, { enforceRequired }) => { + if (!requestedSchema || typeof requestedSchema !== 'object') { + return null; + } + + if (content != null && (typeof content !== 'object' || Array.isArray(content))) { + return 'content must be an object'; + } + + const properties = requestedSchema.properties ?? {}; + const values = content ?? {}; + + for (const key of Object.keys(values)) { + if (!Object.prototype.hasOwnProperty.call(properties, key)) { + return `Unknown field: '${key}'`; + } + } + + if (enforceRequired && Array.isArray(requestedSchema.required)) { + for (const key of requestedSchema.required) { + if (values[key] === undefined || values[key] === null) { + return `Missing required field: '${key}'`; + } + } + } + + for (const [key, property] of Object.entries(properties)) { + if (values[key] === undefined || values[key] === null) { + continue; + } + const error = validateElicitationField(key, values[key], property); + if (error) { + return error; + } + } + + return null; +}; + const clearGetTokensFlow = async ({ flowManager, flowId, tokens }) => { const state = await flowManager.getFlowState(flowId, 'mcp_get_tokens'); if (state?.type === 'mcp_get_tokens' && state.status === 'PENDING') { @@ -658,6 +779,71 @@ router.post('/oauth/cancel/:serverName', requireJwtAuth, async (req, res) => { } }); +/** + * Submit a response to an MCP elicitation request. Completes a pending + * elicitation flow so `MCPManager.callTool` can resume: + * - `action: 'accept' | 'decline' | 'cancel'` — form-mode `elicitation/create` + * response (spec 2025-06-18); `content` carries the submitted field values. + * - `action: 'complete' | 'cancel'` — URL-mode card (either a `mode: 'url'` + * `elicitation/create` request, or a -32042 URL-exception retry): `complete` + * means "I've authorized — continue", which resumes/retries the tool call. + */ +router.post('/elicitation/:flowId', requireJwtAuth, async (req, res) => { + try { + const { flowId } = req.params; + const { action, content } = req.body ?? {}; + const user = req.user; + + if (!user?.id) { + return res.status(401).json({ error: 'User not authenticated' }); + } + + if (!action || !['accept', 'decline', 'cancel', 'complete'].includes(action)) { + return res.status(400).json({ error: 'Invalid action' }); + } + + if (!canAccessElicitationFlow(flowId, user.id)) { + return res.status(403).json({ error: 'Forbidden' }); + } + + const flowsCache = getLogStores(CacheKeys.FLOWS); + const flowManager = getFlowStateManager(flowsCache); + + const flowState = await flowManager.getFlowState(flowId, 'mcp_elicit'); + if (!flowState) { + return res.status(404).json({ error: 'Flow not found' }); + } + + /** + * Prefer the schema persisted in flow metadata (cross-process robust); fall + * back to the in-process stream-context registry captured when the card was + * emitted, so validation still works before that metadata is threaded. + */ + const requestedSchema = + flowState.metadata?.requestedSchema ?? getElicitationFlowContext(flowId)?.requestedSchema; + const validationError = validateElicitationContent(content, requestedSchema, { + enforceRequired: action === 'accept', + }); + if (validationError) { + return res.status(400).json({ error: validationError }); + } + + const ok = await flowManager.completeFlow(flowId, 'mcp_elicit', { action, content }); + if (!ok) { + return res.status(404).json({ error: 'Flow not found' }); + } + + /** Notify the originating stream so a resumed/replayed session renders the + * resolved card instead of a stale pending one. */ + await resolveElicitationFlow({ flowId, action, content }); + + return res.json({ ok: true }); + } catch (error) { + logger.error('[MCP Elicitation] Failed to complete elicitation flow', error); + return res.status(500).json({ error: 'Failed to complete elicitation flow' }); + } +}); + /** * Reinitialize MCP server * This endpoint allows reinitializing a specific MCP server diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index 34164589a0a..d26254e2ccc 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -52,6 +52,19 @@ const RECONNECT_THROTTLE_MS = 10_000; const missingToolCache = new Map(); const MISSING_TOOL_TTL_MS = 10_000; +/** + * Bridges the elicitation SSE stream to the out-of-band completion route. + * `createElicitationStart` runs inside the streaming tool call, so it holds the + * stream context (`res`/`streamId`/`stepId`) and the form `requestedSchema`; the + * `POST /api/mcp/elicitation/:flowId` route runs in a separate request that has + * none of it. Keyed by `flowId`, this registry lets the route re-validate the + * submitted `content` against the real schema and emit `on_elicitation_resolved` + * back onto the originating stream. Entries are evicted on resolution or by TTL. + * @type {Map} + */ +const elicitationFlowContext = new Map(); +const ELICITATION_CONTEXT_TTL_MS = 10 * 60 * 1000; + async function userCanUseMCPServers(user, req) { if (!user?.id || !user?.role) { return false; @@ -82,8 +95,12 @@ function evictStale(map, ttl) { return; } const now = Date.now(); - for (const [key, timestamp] of map) { - if (now - timestamp >= ttl) { + for (const [key, value] of map) { + // Entries are either a bare timestamp (number) or an object carrying a + // `createdAt` field (e.g. elicitationFlowContext). Extract the timestamp + // for either shape; drop entries whose age can't be determined. + const timestamp = typeof value === 'number' ? value : value?.createdAt; + if (timestamp == null || now - timestamp >= ttl) { map.delete(key); } if (map.size <= MAX_CACHE_SIZE) { @@ -377,6 +394,103 @@ function createOAuthCallback({ runStepEmitter, runStepDeltaEmitter }) { }; } +/** + * Emits the `on_elicitation` SSE event so the chat UI can render an + * authorization/form card. Covers both wire mechanisms: a `mode: 'form'|'url'` + * `elicitation/create` request, and the -32042 URL-exception path (always + * `mode: 'url'`, no `requestedSchema`). + * @param {object} params + * @param {ServerResponse} params.res - The Express response object for sending events. + * @param {string} params.stepId - The ID of the step. + * @param {string | null} [params.streamId] - The stream ID for resumable mode. + * @returns {(params: { flowId: string; mode: 'form' | 'url'; message: string; serverName?: string; toolName?: string; requestedSchema?: object; url?: string }) => Promise} + */ +function createElicitationStart({ res, stepId, streamId = null }) { + return async function ({ flowId, mode, message, serverName, toolName, requestedSchema, url }) { + // Capture stream context + schema so the out-of-band completion route can + // validate the response and emit `on_elicitation_resolved` onto this stream. + elicitationFlowContext.set(flowId, { + res, + streamId, + stepId, + requestedSchema, + createdAt: Date.now(), + }); + evictStale(elicitationFlowContext, ELICITATION_CONTEXT_TTL_MS); + + const data = { + id: stepId, + runId: Constants.USE_PRELIM_RESPONSE_MESSAGE_ID, + elicitation: { flowId, mode, message, serverName, toolName, requestedSchema, url }, + }; + const eventData = { event: 'on_elicitation', data }; + if (streamId) { + await GenerationJobManager.emitChunk(streamId, eventData); + } else { + sendEvent(res, eventData); + } + }; +} + +/** + * Returns the captured stream/schema context for a pending elicitation flow, or + * `undefined` once it has resolved or aged out. Used by the completion route to + * recover the form `requestedSchema` for server-side validation when it is not + * (yet) threaded through the flow's persisted metadata. + * @param {string} flowId + * @returns {{ res?: import('http').ServerResponse, streamId: string | null, stepId: string, requestedSchema?: object, createdAt: number } | undefined} + */ +function getElicitationFlowContext(flowId) { + return elicitationFlowContext.get(flowId); +} + +/** + * Emits the `on_elicitation_resolved` SSE event back onto the stream that + * originally rendered the card, so a resumed/replayed session reconstructs the + * resolved state instead of a stale pending card, then drops the flow's context + * entry. No-op (returns `false`) when the originating stream context is gone — + * already resolved, TTL-evicted, or the completion landed on a different process + * than the run: the live client still patches its own copy from the POST + * response, and full reloads rely on the persisted content part. + * @param {object} params + * @param {string} params.flowId + * @param {import('librechat-data-provider').Agents.ElicitationAction} params.action + * @param {Record} [params.content] + * @returns {Promise} + */ +async function resolveElicitationFlow({ flowId, action, content }) { + const context = elicitationFlowContext.get(flowId); + if (!context) { + return false; + } + elicitationFlowContext.delete(flowId); + + const eventData = { + event: 'on_elicitation_resolved', + data: { + id: context.stepId, + runId: Constants.USE_PRELIM_RESPONSE_MESSAGE_ID, + flowId, + action, + content, + }, + }; + + try { + if (context.streamId) { + await GenerationJobManager.emitChunk(context.streamId, eventData); + } else if (context.res) { + sendEvent(context.res, eventData); + } else { + return false; + } + return true; + } catch (error) { + logger.warn(`[MCP][Elicitation] Failed to emit resolution for flow ${flowId}`, error); + return false; + } +} + /** * @param {Object} params * @param {ServerResponse} params.res - The Express response object for sending events. @@ -807,6 +921,17 @@ function createToolInstance({ toolCall, streamId, }); + // Elicitation is enabled by default; a server config sets `elicitation: false` + // to opt out. When disabled, we pass no `elicitationStart`, so MCPManager's + // `if (elicitationStart && userId)` guards skip all elicitation handling. + const elicitationStart = + capturedServerConfig?.elicitation === false + ? undefined + : createElicitationStart({ + res, + stepId, + streamId, + }); if (derivedSignal) { const tenantId = config?.configurable?.user?.tenantId ?? getTenantId(); @@ -840,6 +965,7 @@ function createToolInstance({ }, oauthStart, oauthEnd, + elicitationStart, graphTokenResolver: getGraphApiToken, oboTokenResolver: exchangeOboToken, oboTrustChecker: createOboTrustChecker(), @@ -1058,6 +1184,9 @@ module.exports = { resolveMcpConfigNames, resolveAllMcpConfigs, createOAuthStart, + createElicitationStart, + getElicitationFlowContext, + resolveElicitationFlow, checkOAuthFlowStatus, getServerConnectionStatus, createUnavailableToolStub, diff --git a/api/server/services/__tests__/MCP.spec.js b/api/server/services/__tests__/MCP.spec.js index ca3d5eea9a1..12d25ea75a8 100644 --- a/api/server/services/__tests__/MCP.spec.js +++ b/api/server/services/__tests__/MCP.spec.js @@ -29,7 +29,7 @@ jest.mock('@librechat/api', () => ({ isMCPDomainAllowed: jest.fn(), normalizeServerName: jest.fn((name) => name), normalizeJsonSchema: jest.fn((schema) => schema), - GenerationJobManager: jest.fn(), + GenerationJobManager: { emitChunk: jest.fn() }, resolveJsonSchemaRefs: jest.fn((schema) => schema), buildOAuthToolCallName: jest.fn((name) => name), })); @@ -54,7 +54,15 @@ jest.mock('~/server/services/Tools/mcp', () => ({ })); const { getAppConfig } = require('~/server/services/Config'); -const { resolveConfigServers, resolveMcpConfigNames, resolveAllMcpConfigs } = require('../MCP'); +const { sendEvent, GenerationJobManager } = require('@librechat/api'); +const { + resolveConfigServers, + resolveMcpConfigNames, + resolveAllMcpConfigs, + createElicitationStart, + getElicitationFlowContext, + resolveElicitationFlow, +} = require('../MCP'); describe('resolveConfigServers', () => { beforeEach(() => jest.clearAllMocks()); @@ -179,3 +187,125 @@ describe('resolveAllMcpConfigs', () => { await expect(resolveAllMcpConfigs('u1', { id: 'u1' })).rejects.toThrow('mongo down'); }); }); + +describe('createElicitationStart', () => { + beforeEach(() => jest.clearAllMocks()); + + it('emits the on_elicitation event via sendEvent when no streamId is set', async () => { + const res = { write: jest.fn() }; + const requestedSchema = { type: 'object', properties: {} }; + const start = createElicitationStart({ res, stepId: 'step-1', streamId: null }); + + await start({ + flowId: 'u:s:t:n1', + mode: 'form', + message: 'Fill this in', + serverName: 'jira', + toolName: 'create_issue', + requestedSchema, + }); + + expect(GenerationJobManager.emitChunk).not.toHaveBeenCalled(); + expect(sendEvent).toHaveBeenCalledWith(res, { + event: 'on_elicitation', + data: expect.objectContaining({ + id: 'step-1', + elicitation: expect.objectContaining({ + flowId: 'u:s:t:n1', + mode: 'form', + message: 'Fill this in', + serverName: 'jira', + toolName: 'create_issue', + requestedSchema, + }), + }), + }); + }); + + it('emits the on_elicitation event via emitChunk when a streamId is set', async () => { + const start = createElicitationStart({ res: {}, stepId: 'step-2', streamId: 'stream-9' }); + + await start({ flowId: 'u:s:t:n2', mode: 'url', message: 'Authorize', url: 'https://x/auth' }); + + expect(sendEvent).not.toHaveBeenCalled(); + expect(GenerationJobManager.emitChunk).toHaveBeenCalledWith( + 'stream-9', + expect.objectContaining({ + event: 'on_elicitation', + data: expect.objectContaining({ + elicitation: expect.objectContaining({ + flowId: 'u:s:t:n2', + mode: 'url', + url: 'https://x/auth', + }), + }), + }), + ); + }); + + it('captures flow context so the route can recover the requestedSchema', async () => { + const requestedSchema = { type: 'object', properties: { title: { type: 'string' } } }; + const start = createElicitationStart({ res: {}, stepId: 'step-ctx', streamId: 'stream-ctx' }); + + await start({ flowId: 'flow-ctx', mode: 'form', message: 'x', requestedSchema }); + + expect(getElicitationFlowContext('flow-ctx')).toEqual( + expect.objectContaining({ requestedSchema, streamId: 'stream-ctx', stepId: 'step-ctx' }), + ); + }); +}); + +describe('resolveElicitationFlow', () => { + beforeEach(() => jest.clearAllMocks()); + + it('emits on_elicitation_resolved onto the captured stream and consumes the context', async () => { + const start = createElicitationStart({ res: {}, stepId: 'step-r', streamId: 'stream-r' }); + await start({ flowId: 'flow-resolve', mode: 'form', message: 'x', requestedSchema: {} }); + GenerationJobManager.emitChunk.mockClear(); + + const emitted = await resolveElicitationFlow({ + flowId: 'flow-resolve', + action: 'accept', + content: { title: 'done' }, + }); + + expect(emitted).toBe(true); + expect(GenerationJobManager.emitChunk).toHaveBeenCalledWith( + 'stream-r', + expect.objectContaining({ + event: 'on_elicitation_resolved', + data: expect.objectContaining({ + id: 'step-r', + flowId: 'flow-resolve', + action: 'accept', + content: { title: 'done' }, + }), + }), + ); + + // Context is single-use: a second resolution is a no-op. + expect(await resolveElicitationFlow({ flowId: 'flow-resolve', action: 'cancel' })).toBe(false); + expect(getElicitationFlowContext('flow-resolve')).toBeUndefined(); + }); + + it('returns false when no context exists for the flow', async () => { + expect(await resolveElicitationFlow({ flowId: 'never-started', action: 'complete' })).toBe( + false, + ); + }); + + it('emits via sendEvent for a non-resumable (no streamId) stream', async () => { + const res = { write: jest.fn() }; + const start = createElicitationStart({ res, stepId: 'step-direct', streamId: null }); + await start({ flowId: 'flow-direct', mode: 'url', message: 'auth', url: 'https://x' }); + sendEvent.mockClear(); + + const emitted = await resolveElicitationFlow({ flowId: 'flow-direct', action: 'complete' }); + + expect(emitted).toBe(true); + expect(sendEvent).toHaveBeenCalledWith( + res, + expect.objectContaining({ event: 'on_elicitation_resolved' }), + ); + }); +}); diff --git a/librechat.example.yaml b/librechat.example.yaml index 75da3baaa84..dc699ca37cf 100644 --- a/librechat.example.yaml +++ b/librechat.example.yaml @@ -369,6 +369,9 @@ actions: # # # `tool_search` tool plus a name-only listing instead of each tool's full # # # schema. Saves context on large tool sets. A per-agent tool toggle overrides # # # this. Requires the `deferred_tools` agent capability to be enabled. +# # elicitation: false # Opt this server out of interactive MCP elicitation cards (URL-authorization +# # # and form prompts). Enabled by default; set to false to skip elicitation +# # # handling for this server. # puppeteer: # type: stdio # command: npx diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index ead04e0b877..1acfb56b20b 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -14,6 +14,7 @@ export * from './mcp/registry/MCPServersRegistry'; export * from './mcp/MCPManager'; export * from './mcp/connection'; export * from './mcp/oauth'; +export * from './mcp/elicitation'; export * from './mcp/auth'; export * from './mcp/zod'; export * from './mcp/errors'; diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index 0269937e5bb..1652e6c6a47 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -1,10 +1,12 @@ import pick from 'lodash/pick'; -import { logger } from '@librechat/data-schemas'; +import { logger, getTenantId } from '@librechat/data-schemas'; import { Permissions, PermissionTypes } from 'librechat-data-provider'; import { CallToolResultSchema, ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import type { RequestOptions } from '@modelcontextprotocol/sdk/shared/protocol.js'; import type { TokenMethods, IUser } from '@librechat/data-schemas'; +import type { Agents } from 'librechat-data-provider'; import type { OboTokenResolver, OboTrustChecker } from '~/mcp/oauth/obo'; +import type { ElicitationFlowResult } from './elicitation'; import type { GraphTokenResolver } from '~/utils/graph'; import type { FlowStateManager } from '~/flow/manager'; import type { MCPOAuthTokens } from './oauth'; @@ -18,6 +20,14 @@ import { requiresOAuthMachinery, requiresUserScopedConnection, } from './utils'; +import { + asElicitationFlowManager, + extractUrlElicitation, + generateElicitationFlowId, + isElicitationSuccess, + isHttpUrl, + toElicitResultAction, +} from './elicitation'; import { MCPServersInitializer } from './registry/MCPServersInitializer'; import { OboTokenResolutionError, resolveOboToken } from '~/mcp/oauth'; import { MCPServerInspector } from './registry/MCPServerInspector'; @@ -29,6 +39,36 @@ import { preProcessGraphTokens } from '~/utils/graph'; import { formatToolContent } from './parsers'; import { MCPConnection } from './connection'; import { processMCPEnv } from '~/utils/env'; +import { mcpConfig } from './mcpConfig'; + +/** Buffer (ms) added over the flow TTL so the tool-call SDK timeout always + * outlives the {@link FlowStateManager} wait that actually bounds the user. A + * user who authorizes near the TTL deadline then gets their retried result + * instead of a premature transport timeout. Mirrors the OAuth connect-timeout + * floor in {@link MCPConnectionFactory.attemptToConnect}. */ +const ELICITATION_TIMEOUT_BUFFER_MS = 60 * 1000; + +/** Max elicitation flows a single (userId, serverName) may keep pending at once. + * A misbehaving or hostile server can emit `elicitation/create` in a loop; each + * one spawns a flow-store entry and an SSE card, so the count is capped to stop + * a memory/resource DoS. Further requests past the cap are declined outright. */ +const MAX_PENDING_ELICITATIONS = 3; + +/** Combines the caller's request-abort signal with the SDK's per-elicitation + * signal so cancelling the tool call OR the server cancelling/closing the + * elicitation both tear down the pending flow wait. */ +function combineAbortSignals( + ...signals: Array +): AbortSignal | undefined { + const active = signals.filter((signal): signal is AbortSignal => signal != null); + if (active.length === 0) { + return undefined; + } + if (active.length === 1) { + return active[0]; + } + return AbortSignal.any(active); +} function createOboToolCallErrorMessage( logPrefix: string, @@ -53,6 +93,35 @@ function createOboToolCallErrorMessage( export class MCPManager extends UserConnectionManager { private static instance: MCPManager | null; + /** Live count of elicitation flows awaiting user input, keyed by + * `${userId}:${serverName}`. Bounds concurrent `elicitation/create` cards per + * (user, server) — see {@link MAX_PENDING_ELICITATIONS}. */ + private readonly pendingElicitations = new Map(); + + /** Reserves an elicitation slot for `(userId, serverName)`, returning `false` + * when the cap is already reached so the caller can decline without spawning a + * flow. A successful reservation must be paired with {@link releaseElicitation}. */ + private reserveElicitation(userId: string, serverName: string): boolean { + const key = `${userId}:${serverName}`; + const pending = this.pendingElicitations.get(key) ?? 0; + if (pending >= MAX_PENDING_ELICITATIONS) { + return false; + } + this.pendingElicitations.set(key, pending + 1); + return true; + } + + /** Releases a slot reserved by {@link reserveElicitation}. */ + private releaseElicitation(userId: string, serverName: string): void { + const key = `${userId}:${serverName}`; + const next = (this.pendingElicitations.get(key) ?? 1) - 1; + if (next <= 0) { + this.pendingElicitations.delete(key); + return; + } + this.pendingElicitations.set(key, next); + } + /** Creates and initializes the singleton MCPManager instance */ public static async createInstance(configs: t.MCPServers): Promise { if (MCPManager.instance) throw new Error('MCPManager has already been initialized.'); @@ -353,6 +422,7 @@ Please follow these instructions when using tools from the respective MCP server flowManager, oauthStart, oauthEnd, + elicitationStart, customUserVars, graphTokenResolver, oboTokenResolver, @@ -373,6 +443,26 @@ Please follow these instructions when using tools from the respective MCP server flowManager: FlowStateManager; oauthStart?: t.OAuthStartHandler; oauthEnd?: () => Promise; + /** + * When provided: (1) declares support for MCP elicitation, extending the + * `tools/call` timeout to outlive the flow-state wait (see + * {@link ELICITATION_TIMEOUT_BUFFER_MS}); (2) registers a + * handler for server-initiated `elicitation/create` requests (form/url + * modes); and (3) catches the -32042 `UrlElicitationRequired` exception on + * the initial `tools/call` response and retries once after the user + * authorizes. Called once per elicitation with enough context to render an + * in-chat card; resolution arrives via `flowManager` (same pattern as + * `oauthStart`/OAuth). + */ + elicitationStart?: (params: { + flowId: string; + mode: 'form' | 'url'; + message: string; + serverName?: string; + toolName?: string; + requestedSchema?: Agents.ElicitationSchema; + url?: string; + }) => Promise; graphTokenResolver?: GraphTokenResolver; oboTokenResolver?: OboTokenResolver; oboTrustChecker?: OboTrustChecker; @@ -380,6 +470,7 @@ Please follow these instructions when using tools from the respective MCP server /** User-specific connection */ let connection: MCPConnection | undefined; let cleanupRequestOAuthHandler: (() => void) | undefined; + let cleanupElicitationHandler: (() => void) | undefined; let disconnectAfterCall = false; const userId = user?.id; const logPrefix = userId ? `[MCP][User: ${userId}][${serverName}]` : `[MCP][${serverName}]`; @@ -513,21 +604,141 @@ Please follow these instructions when using tools from the respective MCP server connection.setRequestHeaders(resolvedHeaders); - const result = await connection.client.request( - { - method: 'tools/call', - params: { - name: toolName, - arguments: toolArguments, + const elicitationFlowManager = asElicitationFlowManager(flowManager); + + if (elicitationStart && userId) { + cleanupElicitationHandler = connection.setElicitationHandler( + async (params, elicitationSignal) => { + const isUrlMode = params.mode === 'url'; + if (params.mode === 'url' && !isHttpUrl(params.url)) { + logger.warn( + `${logPrefix}[${toolName}] Declining url-mode elicitation with a non-http(s) URL`, + ); + return { action: 'decline' }; + } + if (!this.reserveElicitation(userId, serverName)) { + logger.warn( + `${logPrefix}[${toolName}] Declining elicitation: ${MAX_PENDING_ELICITATIONS} already pending for this server`, + ); + return { action: 'decline' }; + } + try { + const flowId = generateElicitationFlowId(userId, serverName, toolName, getTenantId()); + logger.debug( + `${logPrefix}[${toolName}] Elicitation requested (${isUrlMode ? 'url' : 'form'}), flowId: ${flowId}`, + ); + await elicitationStart({ + flowId, + mode: isUrlMode ? 'url' : 'form', + message: params.message, + serverName, + toolName, + requestedSchema: params.mode === 'url' ? undefined : params.requestedSchema, + url: params.mode === 'url' ? params.url : undefined, + }); + const flowResult = await elicitationFlowManager.createFlow( + flowId, + 'mcp_elicit', + { requestedSchema: params.mode === 'url' ? undefined : params.requestedSchema }, + combineAbortSignals(options?.signal, elicitationSignal), + ); + logger.debug(`${logPrefix}[${toolName}] Elicitation resolved: ${flowResult.action}`); + return { + action: toElicitResultAction(flowResult.action), + content: flowResult.content, + }; + } finally { + this.releaseElicitation(userId, serverName); + } }, + ); + } + + const requestParams = { + method: 'tools/call' as const, + params: { + name: toolName, + arguments: toolArguments, }, - CallToolResultSchema, - { - timeout: connection.timeout, - resetTimeoutOnProgress: true, - ...options, - }, + }; + const elicitationTimeout = Math.max( + connection.timeout ?? 0, + mcpConfig.OAUTH_FLOW_TTL + ELICITATION_TIMEOUT_BUFFER_MS, ); + const requestOptions = { + timeout: elicitationStart ? elicitationTimeout : connection.timeout, + resetTimeoutOnProgress: true, + ...options, + }; + + let result: unknown; + try { + result = await connection.client.request( + requestParams, + CallToolResultSchema, + requestOptions, + ); + } catch (toolCallError) { + /** + * URL-exception mode (spec 2025-11-25): the server rejected `tools/call` + * with JSON-RPC code -32042 instead of issuing a normal `elicitation/create` + * request (possibly wrapped in an HTTP-level transport error by the + * gateway — see `extractUrlElicitation`). Surface the authorization link, + * wait for the user to complete (or cancel) it via `flowManager`, then + * retry the SAME `tools/call` once. + */ + if (!elicitationStart || !userId) { + throw toolCallError; + } + const first = extractUrlElicitation(toolCallError); + if (!first) { + throw toolCallError; + } + + const flowId = generateElicitationFlowId(userId, serverName, toolName, getTenantId()); + logger.info( + `${logPrefix}[${toolName}] URL elicitation required (-32042), flowId: ${flowId}`, + ); + await elicitationStart({ + flowId, + mode: 'url', + message: first.message, + serverName, + toolName, + url: first.url, + }); + + let flowResult: ElicitationFlowResult; + try { + flowResult = await elicitationFlowManager.createFlow( + flowId, + 'mcp_elicit', + {}, + options?.signal, + ); + } catch (flowError) { + const reason = flowError instanceof Error ? flowError.message : String(flowError); + throw new McpError( + ErrorCode.InvalidRequest, + `${first.message} Open ${first.url} to authorize, then retry. (${reason})`, + ); + } + + if (!isElicitationSuccess(flowResult.action)) { + throw new McpError( + ErrorCode.InvalidRequest, + `${first.message} Authorization was cancelled. Open ${first.url} to authorize, then retry.`, + ); + } + + logger.debug(`${logPrefix}[${toolName}] URL elicitation authorized, retrying tools/call`); + result = await connection.client.request( + requestParams, + CallToolResultSchema, + requestOptions, + ); + } + if (userId) { this.updateUserLastActivity(userId); } @@ -540,6 +751,9 @@ Please follow these instructions when using tools from the respective MCP server throw error; } finally { cleanupRequestOAuthHandler?.(); + // Unregister the per-call elicitation handler so a cached connection can't + // fire a stale closure (old toolName/signal/flow) on a later request. + cleanupElicitationHandler?.(); // Ephemeral connections are never stored in userConnections, so disconnecting // is the only cleanup needed; removing the map entry here could orphan a // still-connected cached connection from before a config change. diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index 865de69eb4e..95ebf92390f 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -12,6 +12,7 @@ import { MCPConnection } from '~/mcp/connection'; import { MCPManager } from '~/mcp/MCPManager'; import * as graphUtils from '~/utils/graph'; import { processMCPEnv } from '~/utils/env'; +import { mcpConfig } from '~/mcp/mcpConfig'; // Mock external dependencies jest.mock('@librechat/data-schemas', () => ({ @@ -21,6 +22,7 @@ jest.mock('@librechat/data-schemas', () => ({ error: jest.fn(), debug: jest.fn(), }, + getTenantId: jest.fn(), })); jest.mock('~/utils/graph', () => ({ @@ -2007,4 +2009,522 @@ describe('MCPManager', () => { ).rejects.toThrow('requires a flowManager'); }); }); + + describe('callTool - URL Elicitation (-32042)', () => { + const mockUser: Partial = { id: 'user-url-elicit' }; + + function makeUrlElicitationError(overrides: Partial<{ message: string; url: string }> = {}) { + const error = new Error('URL elicitation required') as Error & { + code: number; + data: { + elicitations: Array<{ + mode: string; + message: string; + url: string; + elicitationId: string; + }>; + }; + }; + error.code = -32042; + error.data = { + elicitations: [ + { + mode: 'url', + message: overrides.message ?? 'Please authorize access to your account', + url: overrides.url ?? 'https://auth.example.com/authorize?token=abc', + elicitationId: 'elicit-1', + }, + ], + }; + return error; + } + + let mockFlowManager: { + createFlow: jest.Mock; + }; + let mockConnection: MCPConnection; + + beforeEach(() => { + jest.clearAllMocks(); + mockFlowManager = { createFlow: jest.fn() }; + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ + type: 'stdio', + command: 'node', + args: ['server.js'], + }); + }); + + it('retries tools/call once after the user completes the -32042 authorization flow', async () => { + const urlError = makeUrlElicitationError(); + mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { + request: jest + .fn() + .mockRejectedValueOnce(urlError) + .mockResolvedValueOnce({ + content: [{ type: 'text', text: 'ok after auth' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + mockFlowManager.createFlow.mockResolvedValue({ action: 'complete' }); + + const elicitationStart = jest.fn().mockResolvedValue(undefined); + const manager = await MCPManager.createInstance(newMCPServersConfig()); + + const result = await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + expect(elicitationStart).toHaveBeenCalledWith( + expect.objectContaining({ + mode: 'url', + message: 'Please authorize access to your account', + url: 'https://auth.example.com/authorize?token=abc', + }), + ); + expect(mockFlowManager.createFlow).toHaveBeenCalledWith( + expect.any(String), + 'mcp_elicit', + {}, + undefined, + ); + expect(mockConnection.client.request).toHaveBeenCalledTimes(2); + expect(result).toBeDefined(); + }); + + it('fails the tool call with the elicitation message and URL when the user cancels', async () => { + const urlError = makeUrlElicitationError(); + mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { request: jest.fn().mockRejectedValueOnce(urlError) }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + mockFlowManager.createFlow.mockResolvedValue({ action: 'cancel' }); + + const elicitationStart = jest.fn().mockResolvedValue(undefined); + const manager = await MCPManager.createInstance(newMCPServersConfig()); + + await expect( + manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }), + ).rejects.toThrow( + /Please authorize access to your account[\s\S]*https:\/\/auth\.example\.com/, + ); + expect(mockConnection.client.request).toHaveBeenCalledTimes(1); + }); + + it('fails the tool call with the elicitation message and URL when the flow times out', async () => { + const urlError = makeUrlElicitationError(); + mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { request: jest.fn().mockRejectedValueOnce(urlError) }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + mockFlowManager.createFlow.mockRejectedValue(new Error('mcp_elicit flow timed out')); + + const elicitationStart = jest.fn().mockResolvedValue(undefined); + const manager = await MCPManager.createInstance(newMCPServersConfig()); + + await expect( + manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }), + ).rejects.toThrow( + /Please authorize access to your account[\s\S]*https:\/\/auth\.example\.com[\s\S]*timed out/, + ); + }); + + it('does not intercept the -32042 error when elicitationStart is not provided', async () => { + const urlError = makeUrlElicitationError(); + mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { request: jest.fn().mockRejectedValueOnce(urlError) }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + + await expect( + manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + }), + ).rejects.toBe(urlError); + expect(mockConnection.setElicitationHandler).not.toHaveBeenCalled(); + }); + }); + + describe('callTool - Elicitation (form/url elicitation/create)', () => { + const mockUser: Partial = { id: 'user-elicit' }; + + const mockFlowManager = { + getState: jest.fn(), + setState: jest.fn(), + clearState: jest.fn(), + createFlow: jest.fn().mockResolvedValue({ + action: 'accept', + content: { name: 'Alice' }, + }), + }; + + const mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'Elicitation result' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + + beforeEach(() => { + jest.clearAllMocks(); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ + type: 'stdio', + command: 'node', + args: ['server.js'], + }); + }); + + it('registers an elicitation handler on the connection when elicitationStart is provided', async () => { + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + expect(mockConnection.setElicitationHandler).toHaveBeenCalledTimes(1); + expect(mockConnection.setElicitationHandler).toHaveBeenCalledWith(expect.any(Function)); + }); + + it('clears the elicitation handler once the tool call settles', async () => { + const cleanup = jest.fn(); + const connection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn().mockReturnValue(cleanup), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'Elicitation result' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(connection) }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + expect(cleanup).toHaveBeenCalledTimes(1); + }); + + it('forwards form-mode elicitation/create requests and resolves via the flow manager', async () => { + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + mockFlowManager.createFlow.mockResolvedValueOnce({ + action: 'accept', + content: { name: 'Alice' }, + }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + const registeredHandler = (mockConnection.setElicitationHandler as jest.Mock).mock + .calls[0][0]; + const requestedSchema = { type: 'object', properties: { name: { type: 'string' } } }; + const result = await registeredHandler({ message: 'Please fill this in', requestedSchema }); + + expect(elicitationStart).toHaveBeenCalledWith( + expect.objectContaining({ mode: 'form', message: 'Please fill this in', requestedSchema }), + ); + /** Form-mode carries requestedSchema into flow metadata so the completion + * route can validate submitted content cross-process (multi-replica). */ + expect(mockFlowManager.createFlow).toHaveBeenCalledWith( + expect.any(String), + 'mcp_elicit', + { requestedSchema }, + undefined, + ); + expect(result).toEqual({ action: 'accept', content: { name: 'Alice' } }); + }); + + it('forwards url-mode elicitation/create requests and maps a "complete" flow result to "accept"', async () => { + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + mockFlowManager.createFlow.mockResolvedValueOnce({ action: 'complete' }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + const registeredHandler = (mockConnection.setElicitationHandler as jest.Mock).mock + .calls[0][0]; + const result = await registeredHandler({ + mode: 'url', + message: 'Please authorize', + elicitationId: 'elicit-2', + url: 'https://auth.example.com', + }); + + expect(elicitationStart).toHaveBeenCalledWith( + expect.objectContaining({ + mode: 'url', + message: 'Please authorize', + url: 'https://auth.example.com', + requestedSchema: undefined, + }), + ); + /** ElicitResultSchema only accepts accept/decline/cancel — 'complete' isn't a + * valid protocol response, so it must be mapped onto 'accept'. */ + expect(result).toEqual({ action: 'accept', content: undefined }); + }); + + it('extends the tool-call timeout to outlive the flow TTL when elicitationStart is provided', async () => { + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + /** The SDK tools/call timeout must exceed the FlowStateManager wait (the flow + * TTL, plus a buffer) so a legitimate late completion isn't cut off. */ + expect(mockConnection.client.request).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Object), + expect.objectContaining({ timeout: mcpConfig.OAUTH_FLOW_TTL + 60 * 1000 }), + ); + }); + + it('uses default timeout when elicitationStart is not provided', async () => { + mockAppConnections({ get: jest.fn().mockResolvedValue(mockConnection) }); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + }); + + expect(mockConnection.setElicitationHandler).not.toHaveBeenCalled(); + expect(mockConnection.client.request).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Object), + expect.objectContaining({ timeout: mockConnection.timeout }), + ); + }); + + it('caps concurrently-pending elicitation flows per (user, server) and declines beyond the limit', async () => { + const connection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'ok' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(connection) }); + + const pendingResolvers: Array<(value: { action: string }) => void> = []; + const cappedFlowManager = { + createFlow: jest.fn( + () => new Promise<{ action: string }>((resolve) => pendingResolvers.push(resolve)), + ), + }; + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: cappedFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + }); + + const handler = (connection.setElicitationHandler as jest.Mock).mock.calls[0][0]; + const requestedSchema = { type: 'object', properties: { name: { type: 'string' } } }; + + const pending = [ + handler({ message: '1', requestedSchema }), + handler({ message: '2', requestedSchema }), + handler({ message: '3', requestedSchema }), + ]; + const overCap = await handler({ message: '4', requestedSchema }); + + expect(overCap).toEqual({ action: 'decline' }); + expect(elicitationStart).toHaveBeenCalledTimes(3); + + /** Let the three admitted flows reach `createFlow` before asserting the count. */ + await new Promise((resolve) => setImmediate(resolve)); + expect(cappedFlowManager.createFlow).toHaveBeenCalledTimes(3); + + pendingResolvers.forEach((resolve) => resolve({ action: 'accept' })); + await Promise.all(pending); + }); + + it('threads the elicitation abort signal into the flow wait, combined with the request signal', async () => { + const connection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + setElicitationHandler: jest.fn(), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'ok' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + mockAppConnections({ get: jest.fn().mockResolvedValue(connection) }); + + const capturedSignals: Array = []; + const signalFlowManager = { + createFlow: jest.fn((_id: string, _type: string, _meta: unknown, signal?: AbortSignal) => { + capturedSignals.push(signal); + return Promise.resolve({ action: 'accept' }); + }), + }; + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const elicitationStart = jest.fn().mockResolvedValue(undefined); + const requestController = new AbortController(); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: signalFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + elicitationStart, + options: { signal: requestController.signal }, + }); + + const handler = (connection.setElicitationHandler as jest.Mock).mock.calls[0][0]; + const elicitationController = new AbortController(); + await handler( + { mode: 'url', message: 'authorize', elicitationId: 'e1', url: 'https://auth.example.com' }, + elicitationController.signal, + ); + + const combined = capturedSignals[0]; + expect(combined).toBeInstanceOf(AbortSignal); + expect(combined?.aborted).toBe(false); + + /** Aborting the SDK's per-request signal must abort the combined signal, so a + * server cancellation or transport close tears the flow wait down promptly. */ + elicitationController.abort(); + expect(combined?.aborted).toBe(true); + }); + }); }); diff --git a/packages/api/src/mcp/__tests__/elicitation.test.ts b/packages/api/src/mcp/__tests__/elicitation.test.ts new file mode 100644 index 00000000000..09839db02d0 --- /dev/null +++ b/packages/api/src/mcp/__tests__/elicitation.test.ts @@ -0,0 +1,279 @@ +import { + asElicitationFlowManager, + extractUrlElicitation, + generateElicitationFlowId, + isHttpUrl, + parseElicitationFlowId, +} from '~/mcp/elicitation'; + +describe('elicitation flow IDs', () => { + it('round-trips the userId, serverName, and toolName it was built from', () => { + const flowId = generateElicitationFlowId('user-1', 'jira_server', 'create_issue'); + + expect(parseElicitationFlowId(flowId)).toEqual({ + userId: 'user-1', + serverName: 'jira_server', + toolName: 'create_issue', + nonce: expect.any(String), + tenantId: undefined, + }); + }); + + it('preserves a colon inside a segment instead of skewing the parsed fields', () => { + const flowId = generateElicitationFlowId('user-1', 'ns:jira', 'do:thing'); + const parsed = parseElicitationFlowId(flowId); + + expect(parsed?.serverName).toBe('ns:jira'); + expect(parsed?.toolName).toBe('do:thing'); + expect(parsed?.userId).toBe('user-1'); + }); + + it('round-trips the tenantId for tenant-scoped flows', () => { + const flowId = generateElicitationFlowId('user-1', 'jira_server', 'create_issue', 'acme:eu'); + const parsed = parseElicitationFlowId(flowId); + + expect(parsed?.tenantId).toBe('acme:eu'); + expect(parsed?.serverName).toBe('jira_server'); + }); + + it('generates a distinct ID per invocation so concurrent calls never collide', () => { + const a = generateElicitationFlowId('user-1', 'jira_server', 'create_issue'); + const b = generateElicitationFlowId('user-1', 'jira_server', 'create_issue'); + + expect(a).not.toBe(b); + }); + + it('returns null for IDs with too few segments', () => { + expect(parseElicitationFlowId('user-1:jira_server')).toBeNull(); + expect(parseElicitationFlowId('tenant:acme:user-1')).toBeNull(); + }); +}); + +describe('asElicitationFlowManager', () => { + it('re-views the same manager instance without copying it', () => { + const manager = { createFlow: jest.fn(), completeFlow: jest.fn() }; + + expect(asElicitationFlowManager(manager)).toBe(manager); + }); +}); + +describe('MCPConnection.setElicitationHandler registry', () => { + // Exercises the real method against a stubbed SDK client. A single stable + // dispatcher is installed while any handler is pending; each call gets a + // disposer that removes only its own entry, and `removeRequestHandler` fires + // exactly when the registry drains — so a later call disposing first cannot + // orphan an earlier still-pending call. + const { MCPConnection } = jest.requireActual('../connection'); + + type FakeElicitRequest = { params: { message: string; mode?: string; url?: string } }; + type FakeElicitExtra = { signal: AbortSignal }; + type FakeDispatcher = (request: FakeElicitRequest, extra: FakeElicitExtra) => unknown; + + const makeFakeConnection = () => { + let dispatcher: FakeDispatcher | undefined; + const client = { + setRequestHandler: jest.fn((_schema: unknown, fn: FakeDispatcher) => { + dispatcher = fn; + }), + removeRequestHandler: jest.fn(() => { + dispatcher = undefined; + }), + }; + const connection = Object.assign(Object.create(MCPConnection.prototype), { + client, + elicitationHandlers: [], + }) as InstanceType; + const dispatch = (params: FakeElicitRequest['params']) => + dispatcher?.({ params }, { signal: new AbortController().signal }); + return { client, connection, dispatch }; + }; + const handler = () => Promise.resolve({ action: 'accept' as const }); + + it('removes the handler when the registering call settles last', () => { + const { client, connection } = makeFakeConnection(); + const dispose = MCPConnection.prototype.setElicitationHandler.call(connection, handler); + + dispose(); + + expect(client.removeRequestHandler).toHaveBeenCalledTimes(1); + expect(client.removeRequestHandler).toHaveBeenCalledWith('elicitation/create'); + }); + + it('installs one stable dispatcher for concurrent calls and disposal is idempotent', () => { + const { client, connection } = makeFakeConnection(); + const disposeFirst = MCPConnection.prototype.setElicitationHandler.call(connection, handler); + const disposeSecond = MCPConnection.prototype.setElicitationHandler.call(connection, handler); + + expect(client.setRequestHandler).toHaveBeenCalledTimes(1); + + disposeFirst(); + expect(client.removeRequestHandler).not.toHaveBeenCalled(); + + disposeSecond(); + disposeSecond(); + expect(client.removeRequestHandler).toHaveBeenCalledTimes(1); + }); + + it('routes to the most-recent call and does not orphan an earlier one when a later call disposes first', async () => { + const { client, connection, dispatch } = makeFakeConnection(); + const handlerA = jest.fn(() => Promise.resolve({ action: 'accept' as const })); + const handlerB = jest.fn(() => Promise.resolve({ action: 'decline' as const })); + + const disposeA = MCPConnection.prototype.setElicitationHandler.call(connection, handlerA); + const disposeB = MCPConnection.prototype.setElicitationHandler.call(connection, handlerB); + + expect(client.setRequestHandler).toHaveBeenCalledTimes(1); + + await dispatch({ message: 'first' }); + expect(handlerB).toHaveBeenCalledTimes(1); + expect(handlerA).not.toHaveBeenCalled(); + + disposeB(); + expect(client.removeRequestHandler).not.toHaveBeenCalled(); + + await dispatch({ message: 'second' }); + expect(handlerA).toHaveBeenCalledTimes(1); + + disposeA(); + expect(client.removeRequestHandler).toHaveBeenCalledTimes(1); + expect(client.removeRequestHandler).toHaveBeenCalledWith('elicitation/create'); + }); + + it('forwards the per-request abort signal to the routed handler', async () => { + const { connection, dispatch } = makeFakeConnection(); + const handlerWithSignal = jest.fn((_params: unknown, _signal: AbortSignal) => + Promise.resolve({ action: 'accept' as const }), + ); + + MCPConnection.prototype.setElicitationHandler.call(connection, handlerWithSignal); + await dispatch({ message: 'need input' }); + + expect(handlerWithSignal).toHaveBeenCalledWith( + { message: 'need input' }, + expect.any(AbortSignal), + ); + }); +}); + +describe('extractUrlElicitation', () => { + const elicitation = { + mode: 'url', + message: 'Please authorize access to github', + url: 'https://bedrock-agentcore.eu-west-1.amazonaws.com/identities/oauth2/authorize?request_uri=abc', + elicitationId: '8cd9f2ba-103d-44c9-8471-6dd02df67c1b', + }; + + it('extracts from a protocol-level McpError shape (code -32042 + data)', () => { + const error = { code: -32042, data: { elicitations: [elicitation] } }; + expect(extractUrlElicitation(error)).toEqual(elicitation); + }); + + it('extracts from a gateway HTTP-wrapped transport error (the AgentCore wire shape)', () => { + // Exact shape observed live: gateway returns JSON-RPC errors with a non-2xx + // HTTP status, so the SDK throws a StreamableHTTPError whose message embeds + // the raw body and whose `code` is the HTTP status, not -32042. + const body = JSON.stringify({ + jsonrpc: '2.0', + id: 9, + error: { + code: -32042, + message: 'This request requires authorization.', + data: { elicitations: [elicitation] }, + }, + }); + const error = Object.assign( + new Error(`Streamable HTTP error: Error POSTing to endpoint: ${body}`), + { + code: 401, + }, + ); + expect(extractUrlElicitation(error)).toEqual(elicitation); + }); + + it('extracts a whitespaced HTTP-wrapped body (pretty-printed / key-reordered gateway JSON)', () => { + // A gateway that pretty-prints yields `"code": -32042` (note the space) and + // may order keys differently; a literal `"code":-32042` substring match would + // miss it, so extraction must tolerate JSON formatting variance. + const body = JSON.stringify( + { + jsonrpc: '2.0', + id: 9, + error: { + message: 'This request requires authorization.', + data: { elicitations: [elicitation] }, + code: -32042, + }, + }, + null, + 2, + ); + const error = Object.assign( + new Error(`Streamable HTTP error: Error POSTing to endpoint: ${body}`), + { code: 401 }, + ); + expect(extractUrlElicitation(error)).toEqual(elicitation); + }); + + it('returns null for non-elicitation errors in both shapes', () => { + expect(extractUrlElicitation(new Error('boom'))).toBeNull(); + expect(extractUrlElicitation({ code: -32600, data: {} })).toBeNull(); + expect( + extractUrlElicitation( + new Error( + 'Streamable HTTP error: Error POSTing to endpoint: {"jsonrpc":"2.0","id":2,"error":{"code":-32600,"message":"Session not initialized"}}', + ), + ), + ).toBeNull(); + expect(extractUrlElicitation(null)).toBeNull(); + }); + + it('returns null for a -32042 mention with an unparseable body', () => { + expect( + extractUrlElicitation(new Error('Error POSTing to endpoint: {"code":-32042, truncated')), + ).toBeNull(); + }); + + it.each(['javascript:alert(1)', 'data:text/html,', 'vbscript:msgbox'])( + 'drops a protocol-level elicitation carrying a hostile-scheme URL (%s)', + (url) => { + const error = { code: -32042, data: { elicitations: [{ ...elicitation, url }] } }; + expect(extractUrlElicitation(error)).toBeNull(); + }, + ); + + it.each(['javascript:alert(1)', 'data:text/html,pwn', 'vbscript:msgbox'])( + 'drops an HTTP-wrapped gateway elicitation carrying a hostile-scheme URL (%s)', + (url) => { + const body = JSON.stringify({ + jsonrpc: '2.0', + id: 9, + error: { code: -32042, message: 'auth', data: { elicitations: [{ ...elicitation, url }] } }, + }); + const error = Object.assign( + new Error(`Streamable HTTP error: Error POSTing to endpoint: ${body}`), + { code: 401 }, + ); + expect(extractUrlElicitation(error)).toBeNull(); + }, + ); +}); + +describe('isHttpUrl', () => { + it.each(['https://auth.example.com/authorize', 'http://localhost:3000/callback'])( + 'accepts http(s) URLs (%s)', + (url) => { + expect(isHttpUrl(url)).toBe(true); + }, + ); + + it.each([ + 'javascript:alert(1)', + 'data:text/html,', + 'vbscript:msgbox', + 'file:///etc/passwd', + 'not a url', + '', + ])('rejects non-http(s) or unparseable URLs (%s)', (url) => { + expect(isHttpUrl(url)).toBe(false); + }); +}); diff --git a/packages/api/src/mcp/__tests__/helpers/mockElicitationServer.ts b/packages/api/src/mcp/__tests__/helpers/mockElicitationServer.ts new file mode 100644 index 00000000000..2c67b4711a8 --- /dev/null +++ b/packages/api/src/mcp/__tests__/helpers/mockElicitationServer.ts @@ -0,0 +1,401 @@ +import * as http from 'http'; +import type { AddressInfo, Socket } from 'net'; +import { randomUUID } from 'crypto'; + +/** Tracks open sockets so `close()` can force-destroy keep-alive connections; + * a plain `server.close()` otherwise hangs until idle clients disconnect. */ +function trackSockets(httpServer: http.Server): () => Promise { + const sockets = new Set(); + httpServer.on('connection', (socket: Socket) => { + sockets.add(socket); + socket.once('close', () => sockets.delete(socket)); + }); + return () => + new Promise((resolve) => { + for (const socket of sockets) { + socket.destroy(); + } + sockets.clear(); + httpServer.close(() => resolve()); + }); +} + +/** + * How the mock delivers the -32042 `UrlElicitationRequired` error, mirroring the + * two wire shapes {@link extractUrlElicitation} must survive: + * + * - `http-401`: the shape AWS Bedrock AgentCore Gateway returns live — a JSON-RPC + * error body carried on a **non-2xx** HTTP status, so the SDK's streamable-HTTP + * transport never parses it as a protocol message and instead throws a + * `StreamableHTTPError` whose `.message` embeds the raw body. + * - `jsonrpc-200`: the spec-pure shape — a JSON-RPC error body on HTTP **200**, + * which the SDK parses and surfaces as an `McpError` with `code === -32042`. + */ +export type ElicitationWireShape = 'http-401' | 'jsonrpc-200'; + +/** + * JSON serialization of the error body. Gateways/serializers vary here and the + * exact byte layout matters: an HTTP-wrapped -32042 is matched by scanning the + * SDK error's `.message` string, so `'pretty'` (which yields `"code": -32042` + * with a space) is a realistic shape that a naive substring match would miss. + */ +export type ElicitationBodyFormat = 'compact' | 'pretty'; + +export interface MockElicitationServerOptions { + /** Fixed port to bind; when omitted an ephemeral port is chosen. */ + port?: number; + /** Initial value of the in-memory authorization gate. Default `false`. */ + authorized?: boolean; + /** How the -32042 is delivered. Default `'http-401'` (the AgentCore shape). */ + wireShape?: ElicitationWireShape; + /** JSON layout of the error body. Default `'compact'`. */ + bodyFormat?: ElicitationBodyFormat; + /** + * When true, the NEXT `tools/call` (any tool) is answered with a -32600 + * "Session not initialized" JSON-RPC error, then the flag auto-clears — models + * the stale-session state seen in production after a reconnection is abandoned. + * A subsequent call (or a client that re-initializes and retries) succeeds. + */ + sessionErrorOnce?: boolean; +} + +/** The `initialize` request params exactly as they arrived on the wire — lets + * tests assert the protocolVersion and capabilities the real client stack sends + * (e.g. whether URL-mode elicitation capability actually reaches the server). */ +export interface CapturedInitializeRequest { + protocolVersion?: string; + capabilities?: Record; + clientInfo?: Record; +} + +export interface MockElicitationServerState { + /** Flipped to `true` by `GET /consent`; gates `get_secret`. */ + authorized: boolean; + wireShape: ElicitationWireShape; + bodyFormat: ElicitationBodyFormat; + /** `tools/call` invocations received, keyed by tool name — asserts retry counts. */ + callCounts: Record; + /** `elicitationId` handed out on the most recent -32042. */ + lastElicitationId?: string; + /** Raw `initialize` params from the most recent handshake. */ + initializeRequest?: CapturedInitializeRequest; + /** When true, the next `tools/call` returns -32600 then clears (see options). */ + sessionErrorOnce: boolean; +} + +export interface MockElicitationServer { + /** The MCP endpoint LibreChat connects to (`http://127.0.0.1:/mcp`). */ + readonly url: string; + /** The authorization link embedded in elicitations (`.../consent`). */ + readonly consentUrl: string; + /** Non-mutating-for-the-human helper link (`.../reset`) that sets + * `authorized=false` so a fresh UI round starts gated again — no process + * restart needed. Safe to hit in smoke checks (unlike `/consent`). */ + readonly resetUrl: string; + readonly port: number; + readonly state: MockElicitationServerState; + /** Resets the authorization gate, wire shape, body format, and call counters. */ + reset(options?: { + authorized?: boolean; + wireShape?: ElicitationWireShape; + bodyFormat?: ElicitationBodyFormat; + }): void; + close(): Promise; +} + +interface JsonRpcMessage { + jsonrpc: '2.0'; + id?: string | number; + method?: string; + params?: Record; +} + +const CONSENT_PAGE = ` + + Authorization complete + +

✅ Authorization complete

+

You can close this tab and return to the chat, then retry the tool.

+ +`; + +const RESET_PAGE = ` + + Reset + +

🔄 Reset

+

The next get_secret call will require authorization again.

+ +`; + +async function readRequestBody(req: http.IncomingMessage): Promise { + const chunks: Uint8Array[] = []; + for await (const chunk of req) { + chunks.push(chunk as Uint8Array); + } + return Buffer.concat(chunks).toString(); +} + +function sendJson(res: http.ServerResponse, status: number, payload: unknown): void { + const body = JSON.stringify(payload); + res.writeHead(status, { 'content-type': 'application/json' }); + res.end(body); +} + +const WHOAMI_TOOL = { + name: 'whoami', + description: 'Returns a fixed identity string; never requires authorization.', + inputSchema: { type: 'object', properties: {}, additionalProperties: false }, +}; + +const GET_SECRET_TOOL = { + name: 'get_secret', + description: 'Returns a secret payload; gated behind a URL elicitation until authorized.', + inputSchema: { type: 'object', properties: {}, additionalProperties: false }, +}; + +/** + * Starts a standalone MCP Streamable-HTTP server that reproduces the AgentCore + * Gateway's URL-elicitation behavior with exact control over HTTP status codes + * (which the real SDK server does not expose). Backed by raw `node:http`. + * + * Endpoints: + * - `POST /mcp` — MCP protocol: `initialize`, `notifications/initialized`, + * `tools/list` (`whoami`, `get_secret`), and `tools/call`. `get_secret` + * emits -32042 in the configured wire shape until `authorized` flips true. + * - `GET /consent` — flips `authorized` true and returns a small HTML page, so a + * human clicking the elicitation URL completes the flow with zero effort. + * - `GET /mcp` — 405 (no standalone SSE stream; an expected, error-free case for + * the SDK's streamable-HTTP transport). + */ +export async function startMockElicitationServer( + options: MockElicitationServerOptions = {}, +): Promise { + const requestedPort = options.port ?? 0; + const state: MockElicitationServerState = { + authorized: options.authorized ?? false, + wireShape: options.wireShape ?? 'http-401', + bodyFormat: options.bodyFormat ?? 'compact', + sessionErrorOnce: options.sessionErrorOnce ?? false, + callCounts: {}, + }; + + // Finalized once the OS assigns a port (handles `port: 0`); the request-handler + // and error-builder closures read these bindings at call time, after listen. + let origin = `http://127.0.0.1:${requestedPort}`; + let consentUrl = `${origin}/consent`; + + const buildElicitationError = (id: string | number | undefined) => { + const elicitationId = randomUUID(); + state.lastElicitationId = elicitationId; + return { + jsonrpc: '2.0' as const, + id: id ?? null, + error: { + code: -32042, + message: 'This request requires authorization.', + data: { + elicitations: [ + { + mode: 'url', + url: consentUrl, + message: 'Please authorize access to the mock secret', + elicitationId, + }, + ], + }, + }, + }; + }; + + const handleRpc = (msg: JsonRpcMessage, res: http.ServerResponse): void => { + const { id, method, params } = msg; + + switch (method) { + case 'initialize': + state.initializeRequest = { + protocolVersion: params?.protocolVersion as string | undefined, + capabilities: params?.capabilities as Record | undefined, + clientInfo: params?.clientInfo as Record | undefined, + }; + sendJson(res, 200, { + jsonrpc: '2.0', + id: id ?? null, + // Echo the client's requested version so negotiation always succeeds + // (whatever the client sent is, by definition, a version it supports). + result: { + protocolVersion: (params?.protocolVersion as string) ?? '2025-06-18', + capabilities: { tools: {} }, + serverInfo: { name: 'mock-elicitation-server', version: '1.0.0' }, + }, + }); + return; + + case 'notifications/initialized': + // A notification has no id and expects no body — 202 Accepted. + res.writeHead(202).end(); + return; + + case 'tools/list': + sendJson(res, 200, { + jsonrpc: '2.0', + id: id ?? null, + result: { tools: [WHOAMI_TOOL, GET_SECRET_TOOL] }, + }); + return; + + case 'tools/call': { + const toolName = (params?.name as string) ?? ''; + state.callCounts[toolName] = (state.callCounts[toolName] ?? 0) + 1; + + if (state.sessionErrorOnce) { + state.sessionErrorOnce = false; + sendJson(res, 200, { + jsonrpc: '2.0', + id: id ?? null, + error: { + code: -32600, + message: 'Session not initialized. Send notifications/initialized first.', + }, + }); + return; + } + + if (toolName === 'whoami') { + sendJson(res, 200, { + jsonrpc: '2.0', + id: id ?? null, + result: { content: [{ type: 'text', text: 'mock-user' }] }, + }); + return; + } + + if (toolName === 'get_secret') { + if (state.authorized) { + sendJson(res, 200, { + jsonrpc: '2.0', + id: id ?? null, + result: { content: [{ type: 'text', text: 's3cr3t-payload' }] }, + }); + return; + } + const errorBody = buildElicitationError(id); + const serialized = + state.bodyFormat === 'pretty' + ? JSON.stringify(errorBody, null, 2) + : JSON.stringify(errorBody); + res.writeHead(state.wireShape === 'http-401' ? 401 : 200, { + 'content-type': 'application/json', + }); + res.end(serialized); + return; + } + + sendJson(res, 200, { + jsonrpc: '2.0', + id: id ?? null, + error: { code: -32602, message: `Unknown tool: ${toolName}` }, + }); + return; + } + + default: + if (id === undefined) { + // Unknown notification — nothing to answer. + res.writeHead(202).end(); + return; + } + sendJson(res, 200, { + jsonrpc: '2.0', + id, + error: { code: -32601, message: `Method not found: ${method}` }, + }); + } + }; + + const server = http.createServer((req, res) => { + void (async () => { + try { + const url = new URL(req.url ?? '/', origin); + + if (req.method === 'GET' && url.pathname === '/consent') { + state.authorized = true; + res.writeHead(200, { 'content-type': 'text/html' }); + res.end(CONSENT_PAGE); + return; + } + + if (req.method === 'GET' && url.pathname === '/reset') { + // De-authorize so repeated UI test rounds start gated again without a + // process restart. Unlike /consent this is safe to hit in smoke checks. + state.authorized = false; + res.writeHead(200, { 'content-type': 'text/html' }); + res.end(RESET_PAGE); + return; + } + + if (url.pathname === '/mcp') { + if (req.method === 'POST') { + const raw = await readRequestBody(req); + const parsed = JSON.parse(raw) as JsonRpcMessage | JsonRpcMessage[]; + if (Array.isArray(parsed)) { + // The SDK sends one message per request; a batch is unexpected here. + sendJson(res, 400, { + jsonrpc: '2.0', + id: null, + error: { code: -32600, message: 'Batched requests are not supported by the mock.' }, + }); + return; + } + handleRpc(parsed, res); + return; + } + if (req.method === 'DELETE') { + // Session termination — acknowledge so client.close() is clean. + res.writeHead(200).end(); + return; + } + if (req.method === 'GET') { + // No standalone SSE stream; the SDK treats 405 as expected/no-error. + res.writeHead(405).end(); + return; + } + } + + res.writeHead(404, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ error: 'not_found' })); + } catch (error) { + sendJson(res, 500, { + jsonrpc: '2.0', + id: null, + error: { code: -32603, message: error instanceof Error ? error.message : String(error) }, + }); + } + })(); + }); + + const destroySockets = trackSockets(server); + await new Promise((resolve) => server.listen(requestedPort, '127.0.0.1', resolve)); + const boundPort = (server.address() as AddressInfo).port; + origin = `http://127.0.0.1:${boundPort}`; + consentUrl = `${origin}/consent`; + + return { + url: `${origin}/mcp`, + consentUrl, + resetUrl: `${origin}/reset`, + port: boundPort, + state, + reset(resetOptions) { + state.authorized = resetOptions?.authorized ?? false; + state.wireShape = resetOptions?.wireShape ?? state.wireShape; + state.bodyFormat = resetOptions?.bodyFormat ?? state.bodyFormat; + state.sessionErrorOnce = false; + state.callCounts = {}; + state.lastElicitationId = undefined; + // Intentionally NOT clearing initializeRequest — a fresh connect overwrites + // it, and tests may assert on it after resetting other state. + }, + close: () => destroySockets(), + }; +} diff --git a/packages/api/src/mcp/__tests__/helpers/package.json b/packages/api/src/mcp/__tests__/helpers/package.json new file mode 100644 index 00000000000..3dbc1ca591c --- /dev/null +++ b/packages/api/src/mcp/__tests__/helpers/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/packages/api/src/mcp/__tests__/helpers/run-mock-elicitation-server.mjs b/packages/api/src/mcp/__tests__/helpers/run-mock-elicitation-server.mjs new file mode 100644 index 00000000000..fd8af661f07 --- /dev/null +++ b/packages/api/src/mcp/__tests__/helpers/run-mock-elicitation-server.mjs @@ -0,0 +1,51 @@ +/** + * Runnable wrapper around the mock elicitation server for zero-effort manual + * testing: start it, point LibreChat at `http://localhost:3111/mcp`, ask an + * agent to call `get_secret`, click the authorization link the chat renders, + * then retry — the second call returns `s3cr3t-payload`. + * + * Run from `packages/api` (Node >= 22 strips the imported .ts types; the + * `package.json` in this directory marks it as an ES module so the import + * resolves): + * + * node ./src/mcp/__tests__/helpers/run-mock-elicitation-server.mjs + * + * Env: + * MOCK_ELICITATION_PORT (default 3111) + * MOCK_ELICITATION_SHAPE 'http-401' (default) | 'jsonrpc-200' + * MOCK_ELICITATION_FORMAT 'compact' (default) | 'pretty' + */ +import { startMockElicitationServer } from './mockElicitationServer.ts'; + +const port = Number(process.env.MOCK_ELICITATION_PORT ?? 3111); +const wireShape = process.env.MOCK_ELICITATION_SHAPE ?? 'http-401'; +const bodyFormat = process.env.MOCK_ELICITATION_FORMAT ?? 'compact'; + +const server = await startMockElicitationServer({ port, wireShape, bodyFormat }); + +console.log( + [ + '', + `Mock AgentCore elicitation server listening on ${server.url}`, + ` wire shape : ${wireShape}`, + ` body format: ${bodyFormat}`, + ` consent URL: ${server.consentUrl}`, + ` reset URL : ${server.resetUrl} (GET flips authorized=false for a fresh round)`, + '', + 'librechat.yaml:', + ' mcpServers:', + ' mock-elicitation:', + ' type: streamable-http', + ` url: http://localhost:${server.port}/mcp`, + '', + 'Ask the agent to call "get_secret", click the authorization link, then retry.', + 'Press Ctrl+C to stop.', + '', + ].join('\n'), +); + +const shutdown = () => { + server.close().then(() => process.exit(0)); +}; +process.on('SIGINT', shutdown); +process.on('SIGTERM', shutdown); diff --git a/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts b/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts new file mode 100644 index 00000000000..644496d22b5 --- /dev/null +++ b/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts @@ -0,0 +1,330 @@ +import { randomUUID } from 'crypto'; +import type { IUser } from '@librechat/data-schemas'; +import type { FlowStateManager } from '~/flow/manager'; +import { MCPManager } from '../MCPManager'; +import { MCPConnection } from '../connection'; +import { MCPServersRegistry } from '../registry/MCPServersRegistry'; +import { + startMockElicitationServer, + type ElicitationWireShape, + type MockElicitationServer, +} from './helpers/mockElicitationServer'; + +/** + * Exercises the REAL client stack end to end against a mock AgentCore-style + * gateway: `MCPManager.callTool` → real SDK `Client` → real + * `StreamableHTTPClientTransport` (with LibreChat's guarded fetch) → mock HTTP + * server. The only stubs are `getConnection` (so we can point callTool at our + * connection) and the flow manager (so "user consent" resolves synchronously). + * + * This is the harness the unit tests in `elicitation.test.ts` can't be: it + * proves the -32042 exception actually round-trips through the SDK in both wire + * shapes and that callTool emits the elicitation, retries exactly once, and + * returns the post-authorization result. + */ + +const USER = { id: 'user-1' } as unknown as IUser; + +const WIRE_SHAPES: ElicitationWireShape[] = ['http-401', 'jsonrpc-200']; + +function streamableHttpConfig(url: string) { + return { type: 'streamable-http' as const, url }; +} + +/** A flow manager whose `createFlow` simulates the user completing consent by + * hitting the mock's `/consent` link before resolving with `complete`. */ +function consentingFlowManager(server: MockElicitationServer) { + const createFlow = jest.fn(async () => { + await fetch(server.consentUrl); + return { action: 'complete' as const }; + }); + return { manager: { createFlow } as unknown as FlowStateManager, createFlow }; +} + +/** A flow manager whose `createFlow` rejects — models a timeout / user cancel. */ +function rejectingFlowManager(error: Error) { + const createFlow = jest.fn(async () => { + throw error; + }); + return { manager: { createFlow } as unknown as FlowStateManager, createFlow }; +} + +/** A flow manager that must never be consulted (asserts "no retry" paths). */ +function unusedFlowManager() { + const createFlow = jest.fn(async () => { + throw new Error('createFlow should not have been called'); + }); + return { manager: { createFlow } as unknown as FlowStateManager, createFlow }; +} + +async function connectToMock(server: MockElicitationServer): Promise<{ + manager: MCPManager; + connection: MCPConnection; + serverName: string; +}> { + // Unique server name per connection so the static circuit breaker never + // couples one test's reconnects to another's. + const serverName = `mock-${randomUUID()}`; + const connection = new MCPConnection({ + serverName, + serverConfig: streamableHttpConfig(server.url), + useSSRFProtection: false, + }); + await connection.connect(); + + const manager = new MCPManager(); + jest.spyOn(manager, 'getConnection').mockResolvedValue(connection); + return { manager, connection, serverName }; +} + +function callGetSecret( + manager: MCPManager, + server: MockElicitationServer, + serverName: string, + opts: { + flowManager: FlowStateManager; + elicitationStart?: jest.Mock; + }, +) { + return manager.callTool({ + user: USER, + serverName, + serverConfig: streamableHttpConfig(server.url), + toolName: 'get_secret', + provider: 'openai', + flowManager: opts.flowManager as never, + elicitationStart: opts.elicitationStart as never, + }); +} + +describe('URL elicitation (-32042) integration', () => { + let server: MockElicitationServer; + let connection: MCPConnection | undefined; + + beforeAll(async () => { + server = await startMockElicitationServer(); + }); + + afterAll(async () => { + await server.close(); + }); + + beforeEach(() => { + // callTool calls MCPServersRegistry.getInstance() unconditionally; it is + // only *used* for the paths we bypass (providedConfig + no OAuth), so a bare + // stub keeps the singleton-init requirement out of the test. + jest + .spyOn(MCPServersRegistry, 'getInstance') + .mockReturnValue({} as unknown as MCPServersRegistry); + }); + + afterEach(async () => { + if (connection) { + await connection.disconnect(); + connection = undefined; + } + }); + + it('declares protocolVersion 2025-11-25 and URL-mode elicitation on the initialize wire', async () => { + server.reset({ authorized: false, wireShape: 'http-401' }); + const built = await connectToMock(server); + connection = built.connection; + + const init = server.state.initializeRequest; + // Ground truth of what the real client stack put on the wire — reported verbatim. + console.log('CAPTURED_INITIALIZE', JSON.stringify(init, null, 2)); + + expect(init).toBeDefined(); + // URL-mode elicitation passthrough is gated on version 2025-11-25 + capability. + expect(init?.protocolVersion).toBe('2025-11-25'); + // The capability object must survive to the wire (not stripped by the SDK) and + // declare URL mode — `elicitation.url` present is how the SDK signals url support. + const elicitation = init?.capabilities?.elicitation as + | { form?: unknown; url?: unknown } + | undefined; + expect(elicitation).toBeDefined(); + expect(elicitation?.url).toBeDefined(); + expect(elicitation?.form).toBeDefined(); + }); + + it('sanity: a non-gated tool (whoami) succeeds without any elicitation', async () => { + server.reset({ authorized: false, wireShape: 'http-401' }); + const built = await connectToMock(server); + connection = built.connection; + const elicitationStart = jest.fn(async () => undefined); + const { manager: flowManager, createFlow } = unusedFlowManager(); + + const result = await built.manager.callTool({ + user: USER, + serverName: built.serverName, + serverConfig: streamableHttpConfig(server.url), + toolName: 'whoami', + provider: 'openai', + flowManager: flowManager as never, + elicitationStart: elicitationStart as never, + }); + + expect(JSON.stringify(result)).toContain('mock-user'); + expect(elicitationStart).not.toHaveBeenCalled(); + expect(createFlow).not.toHaveBeenCalled(); + expect(server.state.callCounts['whoami']).toBe(1); + }); + + describe.each(WIRE_SHAPES)('wire shape: %s', (wireShape) => { + it('emits the elicitation, retries exactly once, and returns the authorized result', async () => { + server.reset({ authorized: false, wireShape }); + const built = await connectToMock(server); + connection = built.connection; + + const elicitationStart = jest.fn(async () => undefined); + const { manager: flowManager, createFlow } = consentingFlowManager(server); + + const result = await callGetSecret(built.manager, server, built.serverName, { + flowManager, + elicitationStart, + }); + + // The authorization link + message reached the UI layer exactly once. + expect(elicitationStart).toHaveBeenCalledTimes(1); + expect(elicitationStart).toHaveBeenCalledWith( + expect.objectContaining({ + mode: 'url', + url: server.consentUrl, + message: expect.stringContaining('authorize'), + }), + ); + expect(createFlow).toHaveBeenCalledTimes(1); + + // Exactly one retry after consent — the initial call plus the retry. + expect(server.state.callCounts['get_secret']).toBe(2); + expect(JSON.stringify(result)).toContain('s3cr3t-payload'); + }); + + it('propagates the error and does NOT retry when consent fails', async () => { + server.reset({ authorized: false, wireShape }); + const built = await connectToMock(server); + connection = built.connection; + + const elicitationStart = jest.fn(async () => undefined); + const { manager: flowManager, createFlow } = rejectingFlowManager( + new Error('user cancelled'), + ); + + await expect( + callGetSecret(built.manager, server, built.serverName, { flowManager, elicitationStart }), + ).rejects.toThrow(/authorize/i); + + expect(elicitationStart).toHaveBeenCalledTimes(1); + expect(createFlow).toHaveBeenCalledTimes(1); + // Initial call only — the failed consent must not trigger a retry. + expect(server.state.callCounts['get_secret']).toBe(1); + }); + + it('rethrows the original error with no elicitation when elicitationStart is absent', async () => { + server.reset({ authorized: false, wireShape }); + const built = await connectToMock(server); + connection = built.connection; + + const { manager: flowManager, createFlow } = unusedFlowManager(); + + await expect( + callGetSecret(built.manager, server, built.serverName, { flowManager }), + ).rejects.toThrow(); + + // No elicitation machinery ran; the raw tool error surfaced directly. + expect(createFlow).not.toHaveBeenCalled(); + expect(server.state.callCounts['get_secret']).toBe(1); + }); + }); + + // The HTTP-wrapped shape is matched by scanning the SDK error's `.message` + // string, so the exact JSON byte layout matters. A gateway that pretty-prints + // (or otherwise inserts whitespace) yields `"code": -32042` with a space — + // extraction must still recognize it, otherwise callTool rethrows and the UI + // shows a generic gateway error even though authorization was available. + it('extracts a whitespaced HTTP-wrapped -32042 body (gateway serializer variance)', async () => { + server.reset({ authorized: false, wireShape: 'http-401', bodyFormat: 'pretty' }); + const built = await connectToMock(server); + connection = built.connection; + + const elicitationStart = jest.fn(async () => undefined); + const { manager: flowManager, createFlow } = consentingFlowManager(server); + + const result = await callGetSecret(built.manager, server, built.serverName, { + flowManager, + elicitationStart, + }); + + expect(elicitationStart).toHaveBeenCalledTimes(1); + expect(createFlow).toHaveBeenCalledTimes(1); + expect(server.state.callCounts['get_secret']).toBe(2); + expect(JSON.stringify(result)).toContain('s3cr3t-payload'); + }); + + // Regression for the spurious-reconnect bug: the -32042 arrives HTTP-wrapped + // (StreamableHTTPError.code === 401), which the transport error handler would + // otherwise mistake for an OAuth/connection failure and reconnect on — racing + // the in-band retry and, in prod, leaving a stale session (-32600). The live + // session must be left untouched. + it('does NOT emit oauthError or reconnect on a -32042 (request-scoped, session stays live)', async () => { + server.reset({ authorized: false, wireShape: 'http-401' }); + const built = await connectToMock(server); + connection = built.connection; + + const connectionStates: string[] = []; + built.connection.on('connectionChange', (state) => connectionStates.push(state)); + let oauthErrors = 0; + built.connection.on('oauthError', () => { + oauthErrors += 1; + }); + + const elicitationStart = jest.fn(async () => undefined); + const { manager: flowManager } = consentingFlowManager(server); + + const result = await callGetSecret(built.manager, server, built.serverName, { + flowManager, + elicitationStart, + }); + + expect(JSON.stringify(result)).toContain('s3cr3t-payload'); + // The elicitation error must not be classified as an OAuth/transport failure. + expect(oauthErrors).toBe(0); + expect(connectionStates).not.toContain('error'); + }); + + // Documents current -32600 handling (Datadog's observed production failure): a + // "Session not initialized" error on tools/call is NOT auto-recovered by + // callTool — no re-initialize, no retry — it surfaces to the caller. (Mirrored + // in MCPManager.callTool: only -32042 is caught for retry; every other error is + // rethrown, packages/api/src/mcp/MCPManager.ts:607-666.) + it('surfaces a -32600 session error with no auto-reinitialize/retry', async () => { + server.reset({ authorized: true }); + server.state.sessionErrorOnce = true; + const built = await connectToMock(server); + connection = built.connection; + + const elicitationStart = jest.fn(async () => undefined); + const { manager: flowManager } = unusedFlowManager(); + + await expect( + callGetSecret(built.manager, server, built.serverName, { flowManager, elicitationStart }), + ).rejects.toThrow(); + + // Exactly one attempt — callTool did not re-initialize and retry the session. + expect(server.state.callCounts['get_secret']).toBe(1); + expect(elicitationStart).not.toHaveBeenCalled(); + }); + + // GET /reset re-gates the mock (authorized=false) so a human can run repeated + // UI rounds without restarting the process — and, unlike /consent, it is safe + // to hit in smoke checks (it never grants access). + it('GET /reset sets authorized=false for a fresh round', async () => { + server.reset({ authorized: true }); + expect(server.state.authorized).toBe(true); + + const res = await fetch(server.resetUrl); + expect(res.status).toBe(200); + expect(await res.text()).toContain('Reset'); + expect(server.state.authorized).toBe(false); + }); +}); diff --git a/packages/api/src/mcp/connection.ts b/packages/api/src/mcp/connection.ts index 87116b6a6e8..b53aa6ae53d 100644 --- a/packages/api/src/mcp/connection.ts +++ b/packages/api/src/mcp/connection.ts @@ -5,12 +5,17 @@ import { fetch as undiciFetch, Agent, ProxyAgent } from 'undici'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; import { WebSocketClientTransport } from '@modelcontextprotocol/sdk/client/websocket.js'; -import { ResourceListChangedNotificationSchema } from '@modelcontextprotocol/sdk/types.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; import { StdioClientTransport, getDefaultEnvironment, } from '@modelcontextprotocol/sdk/client/stdio.js'; +import { + ElicitRequestSchema, + ErrorCode, + McpError, + ResourceListChangedNotificationSchema, +} from '@modelcontextprotocol/sdk/types.js'; import type { RequestInit as UndiciRequestInit, RequestInfo as UndiciRequestInfo, @@ -18,9 +23,12 @@ import type { Dispatcher, } from 'undici'; import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; +import type { ElicitResult } from '@modelcontextprotocol/sdk/types.js'; +import type { Agents } from 'librechat-data-provider'; import type { MCPOAuthTokens } from './oauth/types'; import type * as t from './types'; import { createSSRFSafeUndiciConnect, isSSRFTarget, resolveHostnameSSRF } from '~/auth'; +import { extractUrlElicitation } from './elicitation'; import { runOutsideTracing } from '~/utils/tracing'; import { isAddressAllowed } from '~/auth/domain'; import { sanitizeUrlForLogging } from './utils'; @@ -32,6 +40,36 @@ type ManagedDispatcher = Agent | ProxyAgent; type ParsedIP = { version: 4 | 6; bits: 32 | 128; value: bigint }; type MCPTool = MCPListToolsResult['tools'][number]; +/** + * Params delivered to a {@link MCPConnection.setElicitationHandler} handler for a + * server-initiated `elicitation/create` request. The form variant (mode absent or + * `'form'`) carries the JSON-schema form; the `'url'` variant carries the out-of-band + * authorization link. Mirrors the SDK's `ElicitRequestParamsSchema` union, restated + * here so `requestedSchema` uses our shared `Agents.ElicitationSchema` shape. + */ +type ElicitationCreateParams = + | { mode?: 'form'; message: string; requestedSchema: Agents.ElicitationSchema } + | { mode: 'url'; message: string; elicitationId: string; url: string }; + +/** + * Services one server-initiated `elicitation/create`. `signal` is the SDK's + * per-request abort signal (fired on `notifications/cancelled` or transport + * close) so a pending wait can be torn down instead of dangling until its TTL. + * Resolves with the SDK's {@link ElicitResult} (`ElicitResultSchema`). + */ +type ElicitationHandler = ( + params: ElicitationCreateParams, + signal: AbortSignal, +) => Promise; + +/** A pending elicitation handler registered for the lifetime of one in-flight + * `tools/call`. Each entry carries its own identity so its disposer removes + * only itself, never a concurrent call's still-live handler. */ +interface ElicitationHandlerEntry { + id: symbol; + handler: ElicitationHandler; +} + const BIGINT_ZERO = BigInt(0); const BIGINT_ONE = BigInt(1); const BIGINT_EIGHT = BigInt(8); @@ -1139,6 +1177,9 @@ export class MCPConnection extends EventEmitter { private lastPingTime: number; private lastConnectionCheckAt: number = 0; private oauthTokens?: MCPOAuthTokens | null; + /** Registry of elicitation handlers pending across concurrent `tools/call`s on + * this shared connection; see {@link setElicitationHandler}. */ + private readonly elicitationHandlers: ElicitationHandlerEntry[] = []; private requestHeaders?: Record | null; private oauthRequired = false; private oauthRecovery = false; @@ -1277,7 +1318,12 @@ export class MCPConnection extends EventEmitter { version: '1.2.3', }, { - capabilities: {}, + /** Declares support for both elicitation wire modes (spec 2025-11-25): + * `form` (2025-06-18 `elicitation/create` with a JSON-schema form) and + * `url` (out-of-band authorization link, either via `elicitation/create` + * with `mode: 'url'`, or the -32042 `UrlElicitationRequired` exception + * path on `tools/call`, which doesn't consult this capability at all). */ + capabilities: { elicitation: { form: {}, url: {} } }, }, ); @@ -2098,6 +2144,26 @@ export class MCPConnection extends EventEmitter { return; } + /** + * A -32042 `UrlElicitationRequired` (the gateway's per-tool authorization + * signal, delivered HTTP-wrapped so `.code` is the 401 status) is NOT a + * transport/session failure: the gateway responded and the session is + * alive. `MCPManager.callTool` handles it in-band — surface the link, await + * consent, then retry the SAME session. Classifying it as an OAuth error + * here (`isOAuthError` matches the 401) would emit `oauthError` and + * `connectionChange: 'error'`, triggering a spurious background + * reconnection that races with — and can invalidate — that retry. In + * production that reconnection abandons on `-32002 insufficient_scope`, + * leaving a stale session whose retry then fails with `-32600 Session not + * initialized`. Leave the live session untouched. + */ + if (extractUrlElicitation(error)) { + logger.debug( + `${this.getLogPrefix()} tools/call URL elicitation (-32042); handled in-band by callTool, not reconnecting`, + ); + return; + } + const { message: errorMessage, code: errorCode, @@ -2416,6 +2482,66 @@ export class MCPConnection extends EventEmitter { this.oauthTokens = tokens; } + /** + * Registers a handler for server-initiated `elicitation/create` requests (both + * `mode: 'form'` and `mode: 'url'`). Does NOT handle the -32042 + * `UrlElicitationRequired` exception path — that arrives as an error on the + * `tools/call` response itself and is handled in `MCPManager.callTool`. + * + * This connection is shared per (user, server), so concurrent `tools/call`s + * each register a handler on it (e.g. an assistant message whose `tool_calls` + * run via `Promise.all`). A single stable dispatcher is installed on the SDK + * client for the span any handler is pending; each call adds its own registry + * entry and gets back a disposer that removes ONLY that entry. The dispatcher + * is torn down (`removeRequestHandler`) exactly when the registry drains to + * empty, so an earlier call still awaiting its elicitation is never orphaned by + * a later call disposing first. + * + * Routing limitation: `elicitation/create` carries no call-correlation id, so + * an incoming request is routed to the most-recently-registered still-pending + * entry — the best correlation the protocol allows. + * + * Callers MUST invoke the disposer once the originating `tools/call` settles so + * a cached connection can't leak a stale per-call closure into a later request. + */ + public setElicitationHandler(handler: ElicitationHandler): () => void { + const entry: ElicitationHandlerEntry = { id: Symbol('elicitation-entry'), handler }; + if (this.elicitationHandlers.length === 0) { + this.client.setRequestHandler(ElicitRequestSchema, (request, extra) => + this.dispatchElicitation(request.params as ElicitationCreateParams, extra.signal), + ); + } + this.elicitationHandlers.push(entry); + return () => { + const index = this.elicitationHandlers.findIndex((pending) => pending.id === entry.id); + if (index === -1) { + return; + } + this.elicitationHandlers.splice(index, 1); + if (this.elicitationHandlers.length === 0) { + this.client.removeRequestHandler(ElicitRequestSchema.shape.method.value); + } + }; + } + + /** Routes one incoming `elicitation/create` to the most-recent still-pending + * handler (see the routing limitation in {@link setElicitationHandler}). The + * registry is only empty when no dispatcher is installed, so the guard covers + * a dispatch that races teardown rather than an expected path. */ + private dispatchElicitation( + params: ElicitationCreateParams, + signal: AbortSignal, + ): Promise { + const entry = this.elicitationHandlers[this.elicitationHandlers.length - 1]; + if (!entry) { + throw new McpError( + ErrorCode.MethodNotFound, + `${this.getLogPrefix()} No elicitation handler registered`, + ); + } + return entry.handler(params, signal); + } + /** * Check if this connection is stale compared to config update time. * A connection is stale if it was created before the config was updated. diff --git a/packages/api/src/mcp/elicitation.ts b/packages/api/src/mcp/elicitation.ts new file mode 100644 index 00000000000..e5dc10c334b --- /dev/null +++ b/packages/api/src/mcp/elicitation.ts @@ -0,0 +1,218 @@ +import { randomUUID } from 'crypto'; +import { ErrorCode } from '@modelcontextprotocol/sdk/types.js'; +import type { Agents } from 'librechat-data-provider'; +import type { FlowStateManager } from '~/flow/manager'; + +/** A single URL-mode elicitation as carried by a -32042 `UrlElicitationRequired` + * error's `data.elicitations` (MCP spec 2025-11-25). */ +export interface UrlElicitation { + mode?: string; + message: string; + url: string; + elicitationId: string; +} + +/** + * True when `url` parses and uses an `http:`/`https:` scheme. Server-supplied + * elicitation URLs are validated by the MCP SDK with `z.string().url()`, which + * also accepts `javascript:`, `data:`, and `vbscript:` — schemes that turn an + * "authorize" card into an XSS/exfiltration vector the moment a client renders + * the link. Every server-supplied URL surfaced for elicitation must pass this + * gate; anything else is dropped rather than shown to the user. + */ +export function isHttpUrl(url: string): boolean { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return false; + } + return parsed.protocol === 'http:' || parsed.protocol === 'https:'; +} + +/** Returns the first elicitation only when its URL is a safe http(s) link; + * otherwise `null`, so a hostile-scheme URL is never surfaced as a URL elicitation. */ +function firstSafeUrlElicitation(elicitations?: UrlElicitation[]): UrlElicitation | null { + const elicitation = elicitations?.[0]; + if (!elicitation || !isHttpUrl(elicitation.url)) { + return null; + } + return elicitation; +} + +/** + * Extracts the first URL elicitation from a failed `tools/call`, handling both + * wire shapes a -32042 can arrive in: + * + * 1. A protocol-level JSON-RPC error response — the SDK surfaces it as an + * `McpError`/`UrlElicitationRequiredError` with `code === -32042` and + * `data.elicitations`. + * 2. An HTTP-level rejection — AgentCore Gateway returns JSON-RPC errors with a + * non-2xx status, so the SDK's streamable-HTTP transport never parses the + * body and instead throws a `StreamableHTTPError` whose `code` is the HTTP + * status and whose message embeds the raw body + * (`"Error POSTing to endpoint: {\"jsonrpc\":...,\"error\":{\"code\":-32042,...}}"`). + * + * Returns `null` when the error is not a URL elicitation in either shape. + */ +export function extractUrlElicitation(error: unknown): UrlElicitation | null { + if (!error || typeof error !== 'object') { + return null; + } + + const { code, data, message } = error as { + code?: unknown; + data?: { elicitations?: UrlElicitation[] }; + message?: unknown; + }; + + if (code === ErrorCode.UrlElicitationRequired) { + return firstSafeUrlElicitation(data?.elicitations); + } + + // Cheap pre-filter on the bare error number (not `"code":-32042`) so gateway + // JSON with whitespace/key-order variance — e.g. a pretty-printed + // `"code": -32042` — still gets parsed; the JSON.parse + numeric-code check + // below is what actually validates the shape. + if (typeof message !== 'string' || !message.includes(String(ErrorCode.UrlElicitationRequired))) { + return null; + } + const braceIndex = message.indexOf('{'); + if (braceIndex === -1) { + return null; + } + const body = message.slice(braceIndex); + try { + const parsed = JSON.parse(body) as { + error?: { code?: number; data?: { elicitations?: UrlElicitation[] } }; + }; + if (parsed.error?.code !== ErrorCode.UrlElicitationRequired) { + return null; + } + return firstSafeUrlElicitation(parsed.error.data?.elicitations); + } catch { + return null; + } +} + +/** Terminal actions a client can post to `POST /api/mcp/elicitation/:flowId`. + * Aliases the canonical {@link Agents.ElicitationAction} (its own doc explains + * each member) so the two never drift out of sync. */ +export type ElicitationFlowAction = Agents.ElicitationAction; + +export interface ElicitationFlowResult { + action: ElicitationFlowAction; + content?: Record; +} + +/** + * Re-views the process-wide {@link FlowStateManager} singleton — statically typed for OAuth + * tokens at its main call sites — as an elicitation-flow manager. The manager stores payloads + * keyed at runtime by (flowId, flow type); the payload shape for an `mcp_elicit` flow is + * fixed by the flow type, not the class generic, so the generic is erased here. This is the + * one audited place that assertion lives, so callers never scatter `as unknown as`. + */ +export function asElicitationFlowManager( + flowManager: unknown, +): FlowStateManager { + return flowManager as FlowStateManager; +} + +/** + * True when a completed elicitation flow's action counts as "proceed" — either + * the 2025-06-18 form-mode `accept`, or the URL-exception `complete`. + */ +export function isElicitationSuccess(action: ElicitationFlowAction | undefined): boolean { + return action === 'accept' || action === 'complete'; +} + +/** + * Maps a flow result's action onto the MCP SDK's `ElicitResultSchema.action` + * enum (`accept` | `decline` | `cancel`), which has no `complete` member — the + * URL-exception-only "I've authorized, continue" signal is treated as `accept` + * for protocol responses. + */ +export function toElicitResultAction( + action: ElicitationFlowAction, +): 'accept' | 'decline' | 'cancel' { + if (action === 'complete') { + return 'accept'; + } + return action; +} + +/** + * Generates a flow ID for an MCP elicitation flow (a `mode: 'form'|'url'` + * `elicitation/create` request, or a -32042 URL-exception retry). + * + * Unlike OAuth flow IDs (`MCPOAuthHandler.generateFlowId`, one per user+server), + * elicitation flows are scoped per tool invocation — concurrent calls to the + * same server must not collide — and the userId is embedded directly so the + * completion route can enforce per-user ownership the same way OAuth flow + * routes do (see `canAccessOAuthFlow` in `api/server/routes/mcp.js`). + * + * Every variable segment is URI-encoded so a `:` inside any of them (server and + * tool names are config/user-derived) can't skew the fields {@link + * parseElicitationFlowId} reads back out. + */ +export function generateElicitationFlowId( + userId: string, + serverName: string, + toolName: string, + tenantId?: string, +): string { + const flowId = `${encodeURIComponent(userId)}:${encodeURIComponent(serverName)}:${encodeURIComponent(toolName)}:${randomUUID()}`; + if (!tenantId) { + return flowId; + } + return `tenant:${encodeURIComponent(tenantId)}:${flowId}`; +} + +export interface ParsedElicitationFlowId { + userId: string; + serverName: string; + toolName: string; + nonce: string; + tenantId?: string; +} + +/** Inverse of {@link generateElicitationFlowId}, used by the completion route to + * verify the requesting user owns the flow before resolving it. */ +export function parseElicitationFlowId(flowId: string): ParsedElicitationFlowId | null { + const parts = flowId.split(':'); + let offset = 0; + let tenantId: string | undefined; + + if (parts[0] === 'tenant') { + if (parts.length < 6 || !parts[1]) { + return null; + } + try { + tenantId = decodeURIComponent(parts[1]); + } catch { + return null; + } + offset = 2; + } + + if (parts.length < offset + 4) { + return null; + } + + const [rawUserId, rawServerName, rawToolName, nonce] = parts.slice(offset, offset + 4); + if (!rawUserId || !rawServerName || !rawToolName || !nonce) { + return null; + } + + try { + return { + userId: decodeURIComponent(rawUserId), + serverName: decodeURIComponent(rawServerName), + toolName: decodeURIComponent(rawToolName), + nonce, + tenantId, + }; + } catch { + return null; + } +} diff --git a/packages/data-provider/src/api-endpoints.ts b/packages/data-provider/src/api-endpoints.ts index 326e60f6443..cdfd3232291 100644 --- a/packages/data-provider/src/api-endpoints.ts +++ b/packages/data-provider/src/api-endpoints.ts @@ -233,6 +233,9 @@ export const cancelMCPOAuth = (serverName: string) => { export const mcpOAuthBind = (serverName: string) => `${BASE_URL}/api/mcp/${serverName}/oauth/bind`; +export const mcpElicitationRespond = (flowId: string) => + `${BASE_URL}/api/mcp/elicitation/${encodeURIComponent(flowId)}`; + export const actionOAuthBind = (actionId: string) => `${BASE_URL}/api/actions/${actionId}/oauth/bind`; diff --git a/packages/data-provider/src/data-service.ts b/packages/data-provider/src/data-service.ts index 80e55551706..9d78bca0b95 100644 --- a/packages/data-provider/src/data-service.ts +++ b/packages/data-provider/src/data-service.ts @@ -235,6 +235,22 @@ export function cancelMCPOAuth(serverName: string): Promise; + }, +): Promise<{ ok: boolean }> => { + return request.post(endpoints.mcpElicitationRespond(flowId), body); +}; + /* Config */ export type StartupConfigOptions = { diff --git a/packages/data-provider/src/mcp.ts b/packages/data-provider/src/mcp.ts index 562e5763e07..b1254d0c0a7 100644 --- a/packages/data-provider/src/mcp.ts +++ b/packages/data-provider/src/mcp.ts @@ -183,6 +183,14 @@ const BaseOptionsSchema = z.object({ * - string: Use custom instructions (overrides server-provided) */ serverInstructions: z.union([z.boolean(), z.string()]).optional(), + /** + * Controls whether this server's MCP elicitation requests surface an + * interactive card in chat (URL-authorization or form prompts). + * - undefined/true (default): elicitation requests are handled and rendered + * - false: elicitation handling is not wired for this server, so requests are + * left to the transport's default (unsupported) behavior + */ + elicitation: z.boolean().optional(), /** * When true, every tool from this MCP server defaults to deferred loading: * the model receives the `tool_search` tool plus a name-only listing instead @@ -394,6 +402,7 @@ const omitServerManagedFields = >(schema: T initTimeout: true, chatMenu: true, serverInstructions: true, + elicitation: true, requiresOAuth: true, customUserVars: true, oauth_headers: true, diff --git a/packages/data-provider/src/types/agents.ts b/packages/data-provider/src/types/agents.ts index 0dc96fb6ccf..7c069f4ce35 100644 --- a/packages/data-provider/src/types/agents.ts +++ b/packages/data-provider/src/types/agents.ts @@ -56,6 +56,7 @@ export namespace Agents { | MessageContentInputAudio | SummaryContentPart | ToolCallContent + | ElicitationContent // eslint-disable-next-line @typescript-eslint/no-explicit-any | (Record & { type?: ContentTypes | string }) // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -100,6 +101,85 @@ export namespace Agents { tool_call?: ToolCall; }; + /** + * MCP elicitation. Covers both wire mechanisms: + * - "form": a server `elicitation/create` request (spec 2025-06-18) answered with + * `{ action: 'accept' | 'decline' | 'cancel', content? }`. + * - "url": either a `mode: 'url'` `elicitation/create` request, OR the URL-exception + * path where a `tools/call` response errors with JSON-RPC code -32042 + * (`ErrorCode.UrlElicitationRequired`) carrying `data.elicitations[]`. Both surface + * the same authorization-link card; the client always resolves them via + * `POST /api/mcp/elicitation/:flowId`. + */ + export type ElicitationMode = 'form' | 'url'; + + export type ElicitationPropertySchema = { + type: 'string' | 'number' | 'integer' | 'boolean'; + title?: string; + description?: string; + enum?: string[]; + default?: string | number | boolean; + minLength?: number; + maxLength?: number; + minimum?: number; + maximum?: number; + }; + + export type ElicitationSchema = { + type: 'object'; + properties: Record; + required?: string[]; + }; + + /** Terminal resolution states a client can post back to `/api/mcp/elicitation/:flowId`. + * `accept`/`decline`/`cancel` mirror the SDK's `ElicitResultSchema.action` (form mode); + * `complete` is the URL-exception (-32042) "I've authorized, continue" signal — treated + * as equivalent to `accept` when resuming the flow. */ + export type ElicitationAction = 'accept' | 'decline' | 'cancel' | 'complete'; + + export type ElicitationContent = { + type: ContentTypes.ELICITATION; + elicitation: { + flowId: string; + mode: ElicitationMode; + message: string; + /** requesting MCP server name, for the card header identity line */ + serverName?: string; + /** requesting MCP tool name, for the card header identity line */ + toolName?: string; + /** form mode only */ + requestedSchema?: ElicitationSchema; + /** url mode only: the authorization/consent page to open */ + url?: string; + /** Set once the card has been resolved (locally, or replayed from persisted history) */ + action?: ElicitationAction; + content?: Record; + }; + }; + + /** Wire payload of the `on_elicitation` SSE event. */ + export type ElicitationEvent = { + id: string; + runId?: string; + elicitation: ElicitationContent['elicitation']; + }; + + /** + * Wire payload of the `on_elicitation_resolved` SSE event, emitted when a + * pending elicitation flow is resolved via `POST /api/mcp/elicitation/:flowId`. + * The client locates the matching {@link ElicitationContent} part by `flowId` + * (its dedupe key) and writes `action`/`content` onto its `elicitation`, so the + * resolved card survives SSE replay and re-render. `id`/`runId` mirror + * {@link ElicitationEvent} so the same step→message resolution applies. + */ + export type ElicitationResolvedEvent = { + id: string; + runId?: string; + flowId: string; + action: ElicitationAction; + content?: Record; + }; + /** * A chunk of a tool call (e.g., as part of a stream). * When merging ToolCallChunks (e.g., via AIMessageChunk.__add__), diff --git a/packages/data-provider/src/types/assistants.ts b/packages/data-provider/src/types/assistants.ts index b222c718edd..5f40692ff86 100644 --- a/packages/data-provider/src/types/assistants.ts +++ b/packages/data-provider/src/types/assistants.ts @@ -593,7 +593,8 @@ export type TMessageContentParts = | (Agents.AgentUpdate & ContentMetadata) | (Agents.MessageContentImageUrl & ContentMetadata) | (Agents.MessageContentVideoUrl & ContentMetadata) - | (Agents.MessageContentInputAudio & ContentMetadata); + | (Agents.MessageContentInputAudio & ContentMetadata) + | (Agents.ElicitationContent & ContentMetadata); export type StreamContentData = TMessageContentParts & { /** The index of the current content part */ diff --git a/packages/data-provider/src/types/runs.ts b/packages/data-provider/src/types/runs.ts index 8e1033c07fe..5ef0c1f16e2 100644 --- a/packages/data-provider/src/types/runs.ts +++ b/packages/data-provider/src/types/runs.ts @@ -11,9 +11,42 @@ export enum ContentTypes { INPUT_AUDIO = 'input_audio', AGENT_UPDATE = 'agent_update', SUMMARY = 'summary', + ELICITATION = 'elicitation', ERROR = 'error', } +/** + * Content-part types that exist only for the chat UI and must never be sent to + * the model. Currently just elicitation cards — they are rendered and persisted + * for replay but carry no meaning for a completion request. Single source of + * truth for the model-payload strip (see {@link stripUiOnlyContentParts}). + */ +export const UI_ONLY_CONTENT_TYPES: ReadonlySet = new Set([ContentTypes.ELICITATION]); + +/** + * Returns `messages` with UI-only content parts (see {@link UI_ONLY_CONTENT_TYPES}) + * removed from each message's `content`, for use right before the payload reaches + * `formatAgentMessages`. Non-mutating: only messages that actually contain such a + * part are shallow-cloned, so the persisted message and UI copy keep the card. + * String/absent content and non-array inputs pass through untouched. + */ +export const stripUiOnlyContentParts = (messages: T[]): T[] => { + if (!Array.isArray(messages)) { + return messages; + } + return messages.map((message) => { + const content = message?.content; + if (!Array.isArray(content)) { + return message; + } + const filtered = content.filter((part) => { + const type = (part as { type?: ContentTypes | string } | null | undefined)?.type; + return type == null || !UI_ONLY_CONTENT_TYPES.has(type as ContentTypes); + }); + return filtered.length === content.length ? message : { ...message, content: filtered }; + }); +}; + export enum StepTypes { TOOL_CALLS = 'tool_calls', MESSAGE_CREATION = 'message_creation', @@ -40,6 +73,8 @@ export enum StepEvents { ON_SUMMARIZE_DELTA = 'on_summarize_delta', ON_SUMMARIZE_COMPLETE = 'on_summarize_complete', ON_SUBAGENT_UPDATE = 'on_subagent_update', + ON_ELICITATION = 'on_elicitation', + ON_ELICITATION_RESOLVED = 'on_elicitation_resolved', } /** Token-tracking event names streamed to the client (separate from StepEvents dispatch). */ From 73dd9013bc04f4b00fe7d804cf1c67f29a8e8c21 Mon Sep 17 00:00:00 2001 From: TomasPalsson Date: Mon, 6 Jul 2026 14:30:08 +0000 Subject: [PATCH 02/10] feat: add MCP elicitation authorization card UI Render an in-chat card for MCP elicitation: an authorization link for URL mode and a validated form (string/number/boolean/enum) for form mode, each with accept/decline/cancel. On the -32042 flow the user opens the server-provided auth URL and the tool call auto-retries after they confirm. - Render only http(s) auth URLs; show a warning instead of a link for javascript:/data:/vbscript: and keep Continue disabled. - Localize every validation message via useLocalize with interpolation; guard numeric fields against NaN and omit empty optional fields. - Announce resolved status through a persistent sr-only aria-live region, matching the sibling tool-call cards. - Consume the on_elicitation_resolved SSE event and patch the resolved card in place so it survives re-render. Covered by ElicitationForm and useStepHandler unit tests, including the unsafe-URL and NaN-guard paths. --- .../Chat/Messages/Content/ElicitationForm.tsx | 594 ++++++++++++++++++ .../components/Chat/Messages/Content/Part.tsx | 7 + .../__tests__/ElicitationForm.test.tsx | 327 ++++++++++ .../SSE/__tests__/useStepHandler.spec.ts | 126 ++++ client/src/hooks/SSE/useStepHandler.ts | 105 +++- client/src/locales/en/translation.json | 21 + 6 files changed, 1179 insertions(+), 1 deletion(-) create mode 100644 client/src/components/Chat/Messages/Content/ElicitationForm.tsx create mode 100644 client/src/components/Chat/Messages/Content/__tests__/ElicitationForm.test.tsx diff --git a/client/src/components/Chat/Messages/Content/ElicitationForm.tsx b/client/src/components/Chat/Messages/Content/ElicitationForm.tsx new file mode 100644 index 00000000000..7d558598a6b --- /dev/null +++ b/client/src/components/Chat/Messages/Content/ElicitationForm.tsx @@ -0,0 +1,594 @@ +import { useMemo, useState } from 'react'; +import type { ReactNode } from 'react'; +import { Button, Input, Label, Spinner } from '@librechat/client'; +import { ContentTypes, dataService } from 'librechat-data-provider'; +import { + ShieldCheck, + ClipboardList, + ExternalLink, + RotateCw, + CheckCircle2, + XCircle, +} from 'lucide-react'; +import type { Agents } from 'librechat-data-provider'; +import type { TranslationKeys } from '~/hooks/useLocalize'; +import { useMessageContext, useOptionalMessagesOperations } from '~/Providers'; +import { useLocalize } from '~/hooks'; +import cn from '~/utils/cn'; + +type ElicitationAction = Agents.ElicitationAction; + +type ElicitationField = { + key: string; + schema: Agents.ElicitationPropertySchema; +}; + +type FieldValue = string | number | boolean; + +function getDefaultValues( + properties: Record, +): Record { + const defaults: Record = {}; + for (const [key, schema] of Object.entries(properties)) { + if (schema.default != null) { + defaults[key] = schema.default; + } else if (schema.type === 'boolean') { + defaults[key] = false; + } else { + defaults[key] = ''; + } + } + return defaults; +} + +function getStatusText( + resolvedAction: ElicitationAction, + localize: (key: TranslationKeys) => string, +): string { + if (resolvedAction === 'accept') { + return localize('com_ui_elicitation_completed'); + } + if (resolvedAction === 'complete') { + return localize('com_ui_elicitation_authorized'); + } + if (resolvedAction === 'cancel') { + return localize('com_ui_elicitation_cancelled'); + } + return localize('com_ui_elicitation_declined'); +} + +/** Only `http(s)` targets are safe to render as a clickable link — a malicious or + * compromised MCP server could otherwise supply a `javascript:`/`data:` URL that + * executes script in the LibreChat origin the moment the user clicks it. */ +function getSafeUrl(url?: string): string | undefined { + if (!url) { + return undefined; + } + try { + const { protocol } = new URL(url); + return protocol === 'http:' || protocol === 'https:' ? url : undefined; + } catch { + return undefined; + } +} + +/** Header chrome shared by both modes: a tinted circular icon, a title, and the + * requesting server/tool identity. Keeps the card visually native to LibreChat's + * other in-chat system cards (see `ToolCall` OAuth sign-in). */ +function CardHeader({ + icon, + title, + identity, +}: { + icon: ReactNode; + title: string; + identity?: string; +}) { + return ( +
+
+ {icon} +
+
+

{title}

+ {identity &&

{identity}

} +
+
+ ); +} + +/** Button label with a stable footprint: when `acting`, the label is kept in the + * layout but hidden, and the Spinner is overlaid — so a click never shifts the row. */ +function ActionLabel({ + label, + icon, + acting, +}: { + label: string; + icon?: ReactNode; + acting: boolean; +}) { + return ( + + + {icon} + {label} + + {acting && ( + + + + )} + + ); +} + +/** + * Renders an in-chat card for MCP elicitation. Covers both wire mechanisms: + * - `mode: 'form'` — a 2025-06-18 `elicitation/create` request; renders the + * requested JSON-schema fields and posts `accept`/`decline`. + * - `mode: 'url'` — either a `mode: 'url'` `elicitation/create` request, or the + * -32042 URL-exception path on `tools/call`; renders a message, a prominent + * authorization link, and Continue/Cancel. Continue posts `action: + * 'complete'`, which resumes/retries the suspended tool call server-side. + * + * Both modes resolve via the same `POST /api/mcp/elicitation/:flowId` route + * (`dataService.respondToElicitation`), mirroring the OAuth "visit a URL, then + * get resumed" flow already used elsewhere in MCP tool calls. + */ +export default function ElicitationForm({ + flowId, + mode, + message, + serverName, + toolName, + url, + requestedSchema, + action: initialAction, +}: Agents.ElicitationContent['elicitation']) { + const localize = useLocalize(); + const { messageId } = useMessageContext(); + const { getMessages, setMessages } = useOptionalMessagesOperations(); + const isUrlMode = mode === 'url'; + const properties = requestedSchema?.properties ?? {}; + const [values, setValues] = useState>(() => + getDefaultValues(properties), + ); + const [pendingAction, setPendingAction] = useState(); + const [sendFailed, setSendFailed] = useState(false); + // Track whether the user has opened this flow's authorization link. When there + // is no link to open, there is nothing to gate on, so treat it as already opened. + const [urlOpened, setUrlOpened] = useState(!url); + const [errors, setErrors] = useState>({}); + const [resolvedAction, setResolvedAction] = useState( + initialAction, + ); + + const submitting = pendingAction != null; + const identity = [serverName, toolName].filter(Boolean).join(' · ') || undefined; + // Never render the server-supplied `url` as an href unless it's http(s) — see + // `getSafeUrl`. A present-but-unsafe `url` renders a warning instead of a link + // and permanently withholds the `urlOpened` unlock (no link, nothing to click). + const safeUrl = useMemo(() => getSafeUrl(url), [url]); + + const fields: ElicitationField[] = Object.entries(properties).map(([key, schema]) => ({ + key, + schema, + })); + + // Form mode: show the server's message, or a fallback so a schema-less request + // never renders bare buttons over blank space. + const formIntro = + message || (fields.length === 0 ? localize('com_ui_elicitation_form_empty') : undefined); + + const validate = (): boolean => { + const newErrors: Record = {}; + for (const { key, schema } of fields) { + const required = requestedSchema?.required?.includes(key) ?? false; + const val = values[key]; + const label = schema.title ?? key; + if (required && (val === '' || val == null)) { + newErrors[key] = localize('com_ui_elicitation_field_required', { field: label }); + } + if (schema.type === 'string' && typeof val === 'string') { + if (schema.minLength != null && val.length < schema.minLength) { + newErrors[key] = localize('com_ui_elicitation_min_length', { min: schema.minLength }); + } + if (schema.maxLength != null && val.length > schema.maxLength) { + newErrors[key] = localize('com_ui_elicitation_max_length', { max: schema.maxLength }); + } + } + if ((schema.type === 'number' || schema.type === 'integer') && val !== '' && val != null) { + const num = Number(val); + if (Number.isNaN(num)) { + newErrors[key] = localize('com_ui_elicitation_not_a_number', { field: label }); + } else if (schema.minimum != null && num < schema.minimum) { + newErrors[key] = localize('com_ui_elicitation_min_value', { min: schema.minimum }); + } else if (schema.maximum != null && num > schema.maximum) { + newErrors[key] = localize('com_ui_elicitation_max_value', { max: schema.maximum }); + } + } + } + setErrors(newErrors); + return Object.keys(newErrors).length === 0; + }; + + /** Writes the resolved `action`/`content` onto this flow's `ELICITATION` content + * part in the owning message, so the resolved state isn't held only in this + * component's local `resolvedAction` state. Mirrors the write-back + * `useStepHandler` applies for the `on_elicitation_resolved` SSE event. A no-op + * when rendered outside a `MessagesViewProvider`. */ + const patchResolvedElicitation = ( + action: ElicitationAction, + resolvedContent?: Record, + ) => { + const messages = getMessages(); + if (!messages) { + return; + } + let didPatch = false; + const updatedMessages = messages.map((msg) => { + if (msg.messageId !== messageId || !msg.content) { + return msg; + } + const updatedContent = msg.content.map((part) => { + if (part?.type !== ContentTypes.ELICITATION || part.elicitation?.flowId !== flowId) { + return part; + } + didPatch = true; + return { + ...part, + elicitation: { ...part.elicitation, action, content: resolvedContent }, + }; + }); + return { ...msg, content: updatedContent }; + }); + if (didPatch) { + setMessages(updatedMessages); + } + }; + + const submitAction = async (action: ElicitationAction) => { + if (action === 'accept' && !isUrlMode && !validate()) { + return; + } + setSendFailed(false); + setPendingAction(action); + try { + const content = + action === 'accept' && !isUrlMode + ? Object.fromEntries( + fields + // Omit empty optional fields instead of defaulting them to 0/null — + // required fields are already guaranteed non-empty by `validate`. + .filter(({ key }) => values[key] !== '' && values[key] != null) + .map(({ key, schema }) => { + const val = values[key]; + if (schema.type === 'number' || schema.type === 'integer') { + return [key, Number(val)]; + } + return [key, val]; + }), + ) + : undefined; + await dataService.respondToElicitation(flowId, { action, content }); + setResolvedAction(action); + patchResolvedElicitation(action, content); + } catch { + // Surface an inline retry affordance; the server-side flow keeps waiting + // (or times out on its own), so the card stays interactive for a retry. + setSendFailed(true); + } finally { + setPendingAction(undefined); + } + }; + + const markUrlOpened = () => setUrlOpened(true); + + const requiredMark = (required: boolean) => + required ? ( + + ) : null; + + const renderField = ({ key, schema }: ElicitationField) => { + const label = schema.title ?? key; + const fieldId = `elicitation-${flowId}-${key}`; + const error = errors[key]; + const required = requestedSchema?.required?.includes(key) ?? false; + const descId = schema.description ? `${fieldId}-description` : undefined; + const errId = error ? `${fieldId}-error` : undefined; + const describedBy = [descId, errId].filter(Boolean).join(' ') || undefined; + + if (schema.enum) { + return ( +
+ + {schema.description && ( +

+ {schema.description} +

+ )} + + {error && ( +

+ {error} +

+ )} +
+ ); + } + + if (schema.type === 'boolean') { + return ( +
+ {/* Nested label grows the click target past the 16px box toward ~28px. */} + + {schema.description && ( +

+ {schema.description} +

+ )} + {error && ( +

+ {error} +

+ )} +
+ ); + } + + return ( +
+ + {schema.description && ( +

+ {schema.description} +

+ )} + setValues((prev) => ({ ...prev, [key]: e.target.value }))} + disabled={submitting} + required={required} + aria-required={required || undefined} + aria-invalid={error ? true : undefined} + aria-describedby={describedBy} + min={schema.minimum} + max={schema.maximum} + minLength={schema.minLength} + maxLength={schema.maxLength} + className={cn(error && 'border-border-destructive focus-visible:ring-border-destructive')} + /> + {error && ( +

+ {error} +

+ )} +
+ ); + }; + + const errorLine = sendFailed ? ( +

+ {localize('com_ui_elicitation_error')} +

+ ) : null; + + // Announced via the permanently-mounted `sr-only` span below, decoupled from + // the visible card's mount/unmount — mirrors `ToolCall`/`WebSearch`/ + // `RetrievalCall`/`CodeAnalyze`, whose live region persists across state + // changes rather than mounting fresh alongside its own content. + const statusText = resolvedAction ? getStatusText(resolvedAction, localize) : ''; + + let card: ReactNode; + if (resolvedAction) { + const succeeded = resolvedAction === 'accept' || resolvedAction === 'complete'; + card = ( +
+ {succeeded ? ( +
+ ); + } else if (isUrlMode) { + card = ( +
+
+
+
+ ); + } else { + card = ( +
+
+
+
+ ); + } + + return ( + <> + + {statusText} + + {card} + + ); +} diff --git a/client/src/components/Chat/Messages/Content/Part.tsx b/client/src/components/Chat/Messages/Content/Part.tsx index 6df833370a8..bf8093e8edc 100644 --- a/client/src/components/Chat/Messages/Content/Part.tsx +++ b/client/src/components/Chat/Messages/Content/Part.tsx @@ -23,6 +23,7 @@ import { SubagentCall, } from './Parts'; import { ErrorMessage } from './MessageContent'; +import ElicitationForm from './ElicitationForm'; import RetrievalCall from './RetrievalCall'; import { getCachedPreview } from '~/utils'; import AgentHandoff from './AgentHandoff'; @@ -356,6 +357,12 @@ const Part = memo(function Part({ /> ); } + } else if (part.type === ContentTypes.ELICITATION) { + const elicitation = part.elicitation; + if (!elicitation) { + return null; + } + return ; } else if (part.type === ContentTypes.IMAGE_FILE) { const imageFile = part[ContentTypes.IMAGE_FILE]; const cached = imageFile.file_id ? getCachedPreview(imageFile.file_id) : undefined; diff --git a/client/src/components/Chat/Messages/Content/__tests__/ElicitationForm.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ElicitationForm.test.tsx new file mode 100644 index 00000000000..650cca20bba --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/ElicitationForm.test.tsx @@ -0,0 +1,327 @@ +import React from 'react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { RecoilRoot } from 'recoil'; +import { dataService } from 'librechat-data-provider'; +import ElicitationForm from '../ElicitationForm'; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string, options?: Record) => { + const translations: Record = { + com_ui_select: 'Select...', + com_ui_elicitation_submit: 'Submit', + com_ui_elicitation_decline: 'Decline', + com_ui_elicitation_cancel: 'Cancel', + com_ui_elicitation_continue: "I've authorized — continue", + com_ui_elicitation_open_url: 'Open authorization page', + com_ui_elicitation_completed: 'Form submitted', + com_ui_elicitation_declined: 'Declined', + com_ui_elicitation_cancelled: 'Cancelled', + com_ui_elicitation_authorized: 'Authorization confirmed', + com_ui_elicitation_field_required: '{{field}} is required', + com_ui_elicitation_min_length: 'Minimum length is {{min}}', + com_ui_elicitation_max_length: 'Maximum length is {{max}}', + com_ui_elicitation_min_value: 'Minimum value is {{min}}', + com_ui_elicitation_max_value: 'Maximum value is {{max}}', + com_ui_elicitation_not_a_number: '{{field}} must be a number', + com_ui_elicitation_invalid_url: "This authorization link is invalid and can't be opened.", + }; + const template = translations[key] || key; + if (!options) { + return template; + } + return template.replace(/\{\{(\w+)\}\}/g, (_match, token: string) => + token in options ? String(options[token]) : `{{${token}}}`, + ); + }, +})); + +jest.mock('~/Providers', () => ({ + useMessageContext: () => ({ messageId: 'test-message-1' }), + useOptionalMessagesOperations: () => ({ + getMessages: () => undefined, + setMessages: jest.fn(), + }), +})); + +jest.mock('librechat-data-provider', () => ({ + ContentTypes: { ELICITATION: 'elicitation' }, + dataService: { + respondToElicitation: jest.fn().mockResolvedValue({ ok: true }), + }, +})); + +const baseSchema = { + type: 'object' as const, + properties: { + name: { + type: 'string' as const, + title: 'Your Name', + description: 'Enter your name', + }, + }, + required: ['name'], +}; + +const renderForm = (overrides = {}) => + render( + + + , + ); + +describe('ElicitationForm - form mode', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders the message and fields', () => { + renderForm(); + expect(screen.getByText('Please provide your info')).toBeInTheDocument(); + expect(screen.getByLabelText(/Your Name/)).toBeInTheDocument(); + expect(screen.getByText('Submit')).toBeInTheDocument(); + expect(screen.getByText('Decline')).toBeInTheDocument(); + }); + + it('shows completed status when action is accept', () => { + renderForm({ action: 'accept' }); + // The status is announced by a permanently-mounted sr-only span AND shown in + // the visible card, so both copies match — see ToolCall/WebSearch tests. + expect(screen.getAllByText('Form submitted').length).toBeGreaterThanOrEqual(1); + expect(screen.queryByText('Submit')).not.toBeInTheDocument(); + }); + + it('shows declined status when action is decline', () => { + renderForm({ action: 'decline' }); + expect(screen.getAllByText('Declined').length).toBeGreaterThanOrEqual(1); + }); + + it('validates required fields before submit', async () => { + renderForm(); + fireEvent.click(screen.getByText('Submit')); + expect(await screen.findByText('Your Name is required')).toBeInTheDocument(); + expect(dataService.respondToElicitation).not.toHaveBeenCalled(); + }); + + it('submits with accept action and field values', async () => { + renderForm(); + fireEvent.change(screen.getByLabelText(/Your Name/), { target: { value: 'Alice' } }); + fireEvent.click(screen.getByText('Submit')); + + await waitFor(() => { + expect(dataService.respondToElicitation).toHaveBeenCalledWith('test-flow-1', { + action: 'accept', + content: { name: 'Alice' }, + }); + }); + expect((await screen.findAllByText('Form submitted')).length).toBeGreaterThanOrEqual(1); + }); + + it('submits with decline action without content', async () => { + renderForm(); + fireEvent.click(screen.getByText('Decline')); + + await waitFor(() => { + expect(dataService.respondToElicitation).toHaveBeenCalledWith('test-flow-1', { + action: 'decline', + content: undefined, + }); + }); + }); + + it('renders enum fields as select dropdowns', () => { + const schema = { + type: 'object' as const, + properties: { + fruit: { + type: 'string' as const, + title: 'Favorite Fruit', + enum: ['Apple', 'Banana', 'Mango'], + }, + }, + }; + renderForm({ requestedSchema: schema }); + expect(screen.getByLabelText('Favorite Fruit')).toBeInTheDocument(); + expect(screen.getByText('Apple')).toBeInTheDocument(); + expect(screen.getByText('Banana')).toBeInTheDocument(); + }); + + it('renders boolean fields as checkboxes', () => { + const schema = { + type: 'object' as const, + properties: { + agree: { + type: 'boolean' as const, + title: 'I agree', + }, + }, + }; + renderForm({ requestedSchema: schema }); + const checkbox = screen.getByLabelText('I agree'); + expect(checkbox).toBeInTheDocument(); + expect(checkbox).toHaveAttribute('type', 'checkbox'); + }); + + it('renders number fields with min/max', () => { + const schema = { + type: 'object' as const, + properties: { + age: { + type: 'integer' as const, + title: 'Age', + minimum: 0, + maximum: 150, + }, + }, + }; + renderForm({ requestedSchema: schema }); + const input = screen.getByLabelText('Age'); + expect(input).toHaveAttribute('type', 'number'); + expect(input).toHaveAttribute('min', '0'); + expect(input).toHaveAttribute('max', '150'); + }); + + it('shows a "must be a number" error and blocks submit when a number field holds non-numeric text', async () => { + // A native `` sanitizes non-numeric keystrokes away + // before `onChange` ever fires, so simulate the realistic path instead: a + // malformed/untrusted `schema.default` seeds non-numeric text straight + // into state, bypassing the input's own DOM-level sanitization. + const schema = { + type: 'object' as const, + properties: { + age: { + type: 'integer' as const, + title: 'Age', + default: 'not-a-number', + }, + }, + }; + renderForm({ requestedSchema: schema }); + fireEvent.click(screen.getByText('Submit')); + + expect(await screen.findByText('Age must be a number')).toBeInTheDocument(); + expect(dataService.respondToElicitation).not.toHaveBeenCalled(); + }); + + it('omits an empty optional numeric field instead of defaulting it to 0', async () => { + const schema = { + type: 'object' as const, + properties: { + name: { + type: 'string' as const, + title: 'Your Name', + }, + age: { + type: 'integer' as const, + title: 'Age', + }, + }, + required: ['name'], + }; + renderForm({ requestedSchema: schema }); + fireEvent.change(screen.getByLabelText(/Your Name/), { target: { value: 'Alice' } }); + fireEvent.click(screen.getByText('Submit')); + + await waitFor(() => { + expect(dataService.respondToElicitation).toHaveBeenCalledWith('test-flow-1', { + action: 'accept', + content: { name: 'Alice' }, + }); + }); + }); +}); + +const renderUrlForm = (overrides = {}) => + render( + + + , + ); + +describe('ElicitationForm - url mode', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders the message and an authorization link opening in a new tab', () => { + renderUrlForm(); + expect(screen.getByText('Please authorize access to your account')).toBeInTheDocument(); + + const link = screen.getByText('Open authorization page').closest('a'); + expect(link).toHaveAttribute('href', 'https://example.com/authorize?token=abc'); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', expect.stringContaining('noopener')); + }); + + it('renders Continue and Cancel controls', () => { + renderUrlForm(); + expect(screen.getByText("I've authorized — continue")).toBeInTheDocument(); + expect(screen.getByText('Cancel')).toBeInTheDocument(); + }); + + it('posts action "complete" when Continue is clicked and shows the authorized status', async () => { + renderUrlForm(); + fireEvent.click(screen.getByText('Open authorization page')); + fireEvent.click(screen.getByText("I've authorized — continue")); + + await waitFor(() => { + expect(dataService.respondToElicitation).toHaveBeenCalledWith('test-flow-url-1', { + action: 'complete', + content: undefined, + }); + }); + expect((await screen.findAllByText('Authorization confirmed')).length).toBeGreaterThanOrEqual( + 1, + ); + }); + + it('posts action "cancel" when Cancel is clicked and shows the cancelled status', async () => { + renderUrlForm(); + fireEvent.click(screen.getByText('Cancel')); + + await waitFor(() => { + expect(dataService.respondToElicitation).toHaveBeenCalledWith('test-flow-url-1', { + action: 'cancel', + content: undefined, + }); + }); + expect((await screen.findAllByText('Cancelled')).length).toBeGreaterThanOrEqual(1); + }); + + it('shows the authorized status immediately when a persisted action is provided', () => { + renderUrlForm({ action: 'complete' }); + expect(screen.getAllByText('Authorization confirmed').length).toBeGreaterThanOrEqual(1); + expect(screen.queryByText('Open authorization page')).not.toBeInTheDocument(); + }); + + it('renders no clickable anchor and a warning for a javascript: URL', () => { + renderUrlForm({ url: 'javascript:alert(1)' }); + expect(screen.queryByText('Open authorization page')).not.toBeInTheDocument(); + expect(screen.queryByRole('link')).not.toBeInTheDocument(); + expect( + screen.getByText("This authorization link is invalid and can't be opened."), + ).toBeInTheDocument(); + }); + + it('renders no clickable anchor for a data: URL', () => { + renderUrlForm({ url: 'data:text/html,' }); + expect(screen.queryByText('Open authorization page')).not.toBeInTheDocument(); + expect(screen.queryByRole('link')).not.toBeInTheDocument(); + }); + + it('keeps Continue disabled when the authorization URL is unsafe', () => { + renderUrlForm({ url: 'javascript:alert(1)' }); + expect(screen.getByText("I've authorized — continue").closest('button')).toBeDisabled(); + }); +}); diff --git a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts index 0e04fb43c65..6911f986b3a 100644 --- a/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts +++ b/client/src/hooks/SSE/__tests__/useStepHandler.spec.ts @@ -1616,6 +1616,132 @@ describe('useStepHandler', () => { }); }); + describe('on_elicitation_resolved event', () => { + const createElicitationPart = ( + overrides: Partial = {}, + ): Agents.ElicitationContent => ({ + type: ContentTypes.ELICITATION, + elicitation: { + flowId: 'flow-1', + mode: 'url', + message: 'Please authorize access', + url: 'https://example.com/authorize', + ...overrides, + }, + }); + + it('writes the resolved action/content onto the matching ELICITATION part by flowId', () => { + const responseMessage = createResponseMessage({ content: [createElicitationPart()] }); + mockGetMessages.mockReturnValue([responseMessage]); + + const { result } = renderHook(() => useStepHandler(createHookParams())); + + const runStep = createRunStep(); + const submission = createSubmission(); + + act(() => { + result.current.stepHandler({ event: StepEvents.ON_RUN_STEP, data: runStep }, submission); + }); + + mockSetMessages.mockClear(); + + const resolvedEvent: Agents.ElicitationResolvedEvent = { + id: runStep.id, + runId: 'response-msg-1', + flowId: 'flow-1', + action: 'complete', + content: { confirmed: true }, + }; + + act(() => { + result.current.stepHandler( + { event: StepEvents.ON_ELICITATION_RESOLVED, data: resolvedEvent }, + submission, + ); + }); + + expect(mockSetMessages).toHaveBeenCalled(); + const lastCall = mockSetMessages.mock.calls[mockSetMessages.mock.calls.length - 1][0]; + const responseMsg = lastCall.find((m: TMessage) => !m.isCreatedByUser); + const elicitationPart = responseMsg?.content?.find( + (part: TMessageContentParts) => part?.type === ContentTypes.ELICITATION, + ) as Agents.ElicitationContent | undefined; + expect(elicitationPart?.elicitation.action).toBe('complete'); + expect(elicitationPart?.elicitation.content).toEqual({ confirmed: true }); + // The rest of the elicitation payload is preserved, not clobbered. + expect(elicitationPart?.elicitation.flowId).toBe('flow-1'); + expect(elicitationPart?.elicitation.url).toBe('https://example.com/authorize'); + }); + + it('warns and does not update messages when no response message is found', () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + + const { result } = renderHook(() => useStepHandler(createHookParams())); + const submission = createSubmission(); + + const resolvedEvent: Agents.ElicitationResolvedEvent = { + id: 'step-unknown', + runId: 'response-msg-nonexistent', + flowId: 'flow-1', + action: 'cancel', + }; + + act(() => { + result.current.stepHandler( + { event: StepEvents.ON_ELICITATION_RESOLVED, data: resolvedEvent }, + submission, + ); + }); + + expect(consoleSpy).toHaveBeenCalledWith( + '[on_elicitation_resolved] No response message found for', + 'response-msg-nonexistent', + ); + expect(mockSetMessages).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + + it('warns and does not update messages when no ELICITATION part matches the flowId', () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + const responseMessage = createResponseMessage({ + content: [createElicitationPart({ flowId: 'other-flow' })], + }); + mockGetMessages.mockReturnValue([responseMessage]); + + const { result } = renderHook(() => useStepHandler(createHookParams())); + + const runStep = createRunStep(); + const submission = createSubmission(); + + act(() => { + result.current.stepHandler({ event: StepEvents.ON_RUN_STEP, data: runStep }, submission); + }); + + mockSetMessages.mockClear(); + + const resolvedEvent: Agents.ElicitationResolvedEvent = { + id: runStep.id, + runId: 'response-msg-1', + flowId: 'flow-1', + action: 'decline', + }; + + act(() => { + result.current.stepHandler( + { event: StepEvents.ON_ELICITATION_RESOLVED, data: resolvedEvent }, + submission, + ); + }); + + expect(consoleSpy).toHaveBeenCalledWith( + '[on_elicitation_resolved] No matching elicitation content part for flowId', + 'flow-1', + ); + expect(mockSetMessages).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + }); + describe('clearStepMaps', () => { it('should clear all internal maps', () => { const responseMessage = createResponseMessage(); diff --git a/client/src/hooks/SSE/useStepHandler.ts b/client/src/hooks/SSE/useStepHandler.ts index e9ef6e3227a..c919537a2aa 100644 --- a/client/src/hooks/SSE/useStepHandler.ts +++ b/client/src/hooks/SSE/useStepHandler.ts @@ -54,7 +54,9 @@ type TStepEvent = | { event: StepEvents.ON_SUMMARIZE_START; data: Agents.SummarizeStartEvent } | { event: StepEvents.ON_SUMMARIZE_DELTA; data: Agents.SummarizeDeltaEvent } | { event: StepEvents.ON_SUMMARIZE_COMPLETE; data: Agents.SummarizeCompleteEvent } - | { event: StepEvents.ON_SUBAGENT_UPDATE; data: SubagentUpdateEvent }; + | { event: StepEvents.ON_SUBAGENT_UPDATE; data: SubagentUpdateEvent } + | { event: StepEvents.ON_ELICITATION; data: Agents.ElicitationEvent } + | { event: StepEvents.ON_ELICITATION_RESOLVED; data: Agents.ElicitationResolvedEvent }; type MessageDeltaUpdate = { type: ContentTypes.TEXT; text: string; tool_call_ids?: string[] }; @@ -965,6 +967,107 @@ export default function useStepHandler({ } } else if (stepEvent.event === StepEvents.ON_SUBAGENT_UPDATE) { applySubagentUpdate(stepEvent.data); + } else if (stepEvent.event === StepEvents.ON_ELICITATION) { + const { id: eventStepId, runId: eventRunId, elicitation } = stepEvent.data; + const runStep = stepMap.current.get(eventStepId); + let responseMessageId = runStep?.runId ?? eventRunId ?? ''; + if (responseMessageId === Constants.USE_PRELIM_RESPONSE_MESSAGE_ID) { + responseMessageId = submission?.initialResponse?.messageId ?? ''; + parentMessageId = submission?.initialResponse?.parentMessageId ?? ''; + } + if (!responseMessageId) { + console.warn('No message id found in elicitation event'); + return; + } + + const response = messageMap.current.get(responseMessageId); + if (!response) { + console.warn('[on_elicitation] No response message found for', responseMessageId); + return; + } + + const contentPart: Agents.ElicitationContent = { + type: ContentTypes.ELICITATION, + elicitation, + }; + /** Elicitation content is a standalone card, not an incremental delta onto an + * existing content-typed slot (e.g. the originating tool call) — append it as + * its own part, deduping by flowId so a re-emitted event replaces in place. */ + const existingContent = (response.content ?? []) as TMessageContentParts[]; + const updatedContentArr = [ + ...existingContent.filter( + (part) => + part?.type !== ContentTypes.ELICITATION || + part.elicitation?.flowId !== elicitation.flowId, + ), + contentPart, + ]; + const updatedResponse = { ...response, content: updatedContentArr }; + messageMap.current.set(responseMessageId, updatedResponse); + setMessages( + mergeResponseMessage(messages, updatedResponse, responseMessageId, { + ensureUserMessage: true, + }), + ); + } else if (stepEvent.event === StepEvents.ON_ELICITATION_RESOLVED) { + const { + id: eventStepId, + runId: eventRunId, + flowId, + action, + content: resolvedContent, + } = stepEvent.data; + const runStep = stepMap.current.get(eventStepId); + let responseMessageId = runStep?.runId ?? eventRunId ?? ''; + if (responseMessageId === Constants.USE_PRELIM_RESPONSE_MESSAGE_ID) { + responseMessageId = submission?.initialResponse?.messageId ?? ''; + parentMessageId = submission?.initialResponse?.parentMessageId ?? ''; + } + if (!responseMessageId) { + console.warn('No message id found in elicitation resolved event'); + return; + } + + const response = messageMap.current.get(responseMessageId); + if (!response) { + console.warn( + '[on_elicitation_resolved] No response message found for', + responseMessageId, + ); + return; + } + + /** Locate the ELICITATION part this resolution belongs to by `flowId` (its + * dedupe key, same as the append-in-place logic above) and write the + * resolved `action`/`content` onto it, so the resolved card survives SSE + * replay and re-render. */ + const existingContent = (response.content ?? []) as TMessageContentParts[]; + let didResolve = false; + const updatedContentArr = existingContent.map((part) => { + if (part?.type !== ContentTypes.ELICITATION || part.elicitation?.flowId !== flowId) { + return part; + } + didResolve = true; + return { + ...part, + elicitation: { ...part.elicitation, action, content: resolvedContent }, + }; + }); + if (!didResolve) { + console.warn( + '[on_elicitation_resolved] No matching elicitation content part for flowId', + flowId, + ); + return; + } + + const updatedResponse = { ...response, content: updatedContentArr }; + messageMap.current.set(responseMessageId, updatedResponse); + setMessages( + mergeResponseMessage(messages, updatedResponse, responseMessageId, { + ensureUserMessage: true, + }), + ); } else if (stepEvent.event === StepEvents.ON_SUMMARIZE_START) { announcePolite({ message: 'summarize_started', isStatus: true }); } else if (stepEvent.event === StepEvents.ON_SUMMARIZE_DELTA) { diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 81df389135f..35de7103e09 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1095,6 +1095,27 @@ "com_ui_edited_file": "Edited {{0}}", "com_ui_editing_file": "Editing {{0}}", "com_ui_editor_instructions": "Drag the image to reposition • Use zoom slider or buttons to adjust size", + "com_ui_elicitation_authorized": "Authorization confirmed", + "com_ui_elicitation_cancel": "Cancel", + "com_ui_elicitation_cancelled": "Cancelled", + "com_ui_elicitation_completed": "Form submitted", + "com_ui_elicitation_continue": "I've authorized — continue", + "com_ui_elicitation_decline": "Decline", + "com_ui_elicitation_declined": "Declined", + "com_ui_elicitation_error": "Couldn't send your response — try again.", + "com_ui_elicitation_field_required": "{{field}} is required", + "com_ui_elicitation_form_empty": "The tool needs additional confirmation.", + "com_ui_elicitation_form_title": "Additional information needed", + "com_ui_elicitation_invalid_url": "This authorization link is invalid and can't be opened.", + "com_ui_elicitation_max_length": "Maximum length is {{max}}", + "com_ui_elicitation_max_value": "Maximum value is {{max}}", + "com_ui_elicitation_min_length": "Minimum length is {{min}}", + "com_ui_elicitation_min_value": "Minimum value is {{min}}", + "com_ui_elicitation_not_a_number": "{{field}} must be a number", + "com_ui_elicitation_open_url": "Open authorization page", + "com_ui_elicitation_reopen": "Reopen page", + "com_ui_elicitation_submit": "Submit", + "com_ui_elicitation_title": "Authorization required", "com_ui_empty_category": "-", "com_ui_endpoint": "Endpoint", "com_ui_endpoint_menu": "LLM Endpoint Menu", From be40535e849e5219e01d4b2c8f0e79129cc31f02 Mon Sep 17 00:00:00 2001 From: TomasPalsson Date: Mon, 6 Jul 2026 14:36:41 +0000 Subject: [PATCH 03/10] test: drop stray debug log from elicitation integration test --- .../api/src/mcp/__tests__/urlElicitation.integration.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts b/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts index 644496d22b5..7c260fd7dd5 100644 --- a/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts +++ b/packages/api/src/mcp/__tests__/urlElicitation.integration.test.ts @@ -131,9 +131,7 @@ describe('URL elicitation (-32042) integration', () => { connection = built.connection; const init = server.state.initializeRequest; - // Ground truth of what the real client stack put on the wire — reported verbatim. - console.log('CAPTURED_INITIALIZE', JSON.stringify(init, null, 2)); - + // Ground truth of what the real client stack put on the wire. expect(init).toBeDefined(); // URL-mode elicitation passthrough is gated on version 2025-11-25 + capability. expect(init?.protocolVersion).toBe('2025-11-25'); From ceadb9690a1d986294cc7cc82e14d21ccf104f5d Mon Sep 17 00:00:00 2001 From: TomasPalsson Date: Mon, 6 Jul 2026 21:46:23 +0000 Subject: [PATCH 04/10] feat: complete MCP 2025-11-25 elicitation spec conformance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the remaining gaps against the authoritative MCP 2025-11-25 elicitation spec (form + URL mode), verified by an audit of the spec's client-side MUST/SHOULD clauses. URL-mode client security (client/ElicitationForm): - Show the full authorization URL as visible text for examination before the user consents to open it (spec MUST), not just an "Open" button. - Highlight the destination host/domain, matching the existing OAuth sign-in card treatment (subdomain-spoofing mitigation). - Warn on Punycode / mixed-script (homograph) hostnames. Form-mode requestedSchema subset — render + validate the full set: - string `pattern` (regex) and `format` (email/uri/date/date-time, mapped to native input types); - `oneOf: [{const, title}]` titled enums and `enumNames` display titles; - multi-select `type: "array"` (items enum/anyOf, minItems/maxItems), emitted as a real JSON array in the response payload. Server-side (routes/mcp.js) validates the same subset — pattern, format, oneOf membership, and array membership/bounds — so client validation cannot be bypassed by a direct API call. Also retain the server-supplied `elicitationId` alongside the flow id (kept internal) instead of discarding it. Known follow-ups (documented, not addressed here): negotiated protocol-version gating for URL mode, a -32602 override for malformed mode values (currently the SDK default -32603), and a notifications/elicitation/complete auto-retry handler (spec MAY). Covered by new client and server tests (email/uri/date formats, pattern, oneOf, multi-select array, full-URL display, domain + Punycode warning, and the server validation matrix). --- api/server/routes/__tests__/mcp.spec.js | 199 ++++++++++++++ api/server/routes/mcp.js | 113 +++++++- api/server/services/MCP.js | 23 +- api/server/services/__tests__/MCP.spec.js | 24 ++ .../Chat/Messages/Content/ElicitationForm.tsx | 256 +++++++++++++++++- .../__tests__/ElicitationForm.test.tsx | 131 +++++++++ client/src/locales/en/translation.json | 9 + packages/api/src/mcp/MCPManager.ts | 14 +- .../api/src/mcp/__tests__/MCPManager.test.ts | 15 +- packages/data-provider/src/data-service.ts | 2 +- packages/data-provider/src/types/agents.ts | 36 ++- 11 files changed, 803 insertions(+), 19 deletions(-) diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index b3aa5842651..8617c49128b 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -3507,5 +3507,204 @@ describe('MCP Routes', () => { expect(response.status).toBe(500); expect(response.body).toEqual({ error: 'Failed to complete elicitation flow' }); }); + + it('should return 400 when submitted content violates a pattern constraint', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { zip: { type: 'string', pattern: '^\\d{5}$' } }, + required: ['zip'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { zip: 'not-a-zip' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/zip/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when submitted content violates a format constraint', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { email: { type: 'string', format: 'email' } }, + required: ['email'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { email: 'not-an-email' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/email/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when submitted content is not a member of a oneOf constraint', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { + plan: { + oneOf: [ + { const: 'basic', title: 'Basic' }, + { const: 'pro', title: 'Pro' }, + ], + }, + }, + required: ['plan'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { plan: 'enterprise' } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/plan/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when a submitted array contains a non-permitted member', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { + labels: { type: 'array', items: { enum: ['bug', 'feature'] } }, + }, + required: ['labels'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { labels: ['bug', 'not-allowed'] } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/labels/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when a submitted array violates minItems', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { + labels: { + type: 'array', + items: { enum: ['bug', 'feature'] }, + minItems: 1, + maxItems: 2, + }, + }, + required: ['labels'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { labels: [] } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/labels/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should return 400 when a submitted array violates maxItems', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { + labels: { + type: 'array', + items: { enum: ['bug', 'feature', 'chore'] }, + maxItems: 1, + }, + }, + required: ['labels'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { labels: ['bug', 'feature'] } }); + + expect(response.status).toBe(400); + expect(response.body.error).toMatch(/labels/); + expect(flowManager.completeFlow).not.toHaveBeenCalled(); + }); + + it('should complete the flow for a valid array/oneOf submission', async () => { + const flowManager = mockFlowManager(); + flowManager.getFlowState.mockResolvedValue({ + status: 'PENDING', + metadata: { + requestedSchema: { + type: 'object', + properties: { + plan: { + oneOf: [ + { const: 'basic', title: 'Basic' }, + { const: 'pro', title: 'Pro' }, + ], + }, + labels: { + type: 'array', + items: { enum: ['bug', 'feature'] }, + minItems: 1, + maxItems: 2, + }, + }, + required: ['plan', 'labels'], + }, + }, + }); + require('~/config').getFlowStateManager.mockReturnValue(flowManager); + + const response = await request(app) + .post(`/api/mcp/elicitation/${encodeURIComponent(ownedFlowId)}`) + .send({ action: 'accept', content: { plan: 'pro', labels: ['bug', 'feature'] } }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ ok: true }); + expect(flowManager.completeFlow).toHaveBeenCalledWith(ownedFlowId, 'mcp_elicit', { + action: 'accept', + content: { plan: 'pro', labels: ['bug', 'feature'] }, + }); + }); }); }); diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index ff7a999ea5b..94c18cb012b 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -89,17 +89,111 @@ const canAccessElicitationFlow = (flowId, userId) => { return parsed.userId === userId; }; +const EMAIL_PATTERN = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + +/** + * Validates the restricted `format` keyword on a string property (spec + * 2025-11-25: email, uri, date, date-time — the subset the client renders as a + * specialized input). Returns an error message on violation, or `null`. + * @param {string} key + * @param {string} value + * @param {string} format + * @returns {string | null} + */ +const validateElicitationFormat = (key, value, format) => { + if (format === 'email') { + return EMAIL_PATTERN.test(value) ? null : `Field '${key}' must be a valid email address`; + } + if (format === 'uri') { + try { + new URL(value); + return null; + } catch { + return `Field '${key}' must be a valid URI`; + } + } + if (format === 'date' || format === 'date-time') { + return Number.isNaN(Date.parse(value)) ? `Field '${key}' must be a valid ${format}` : null; + } + return null; +}; + +/** + * Resolves the permitted member values for an array-type (multi-select) + * property's `items` schema, supporting both shapes the client renders: + * `items.enum` (bare values) and `items.anyOf: [{ const }]` (labeled options). + * Returns `null` when `items` carries neither, i.e. any array is permitted. + * @param {unknown} items + * @returns {unknown[] | null} + */ +const getArrayItemMembers = (items) => { + if (!items || typeof items !== 'object') { + return null; + } + if (Array.isArray(items.enum)) { + return items.enum; + } + if (Array.isArray(items.anyOf)) { + return items.anyOf.map((option) => option?.const); + } + return null; +}; + /** * Validates a single submitted elicitation field value against its property * schema (the MCP form-mode elicitation subset of JSON Schema). Returns an error * message on the first violation, or `null` when the value conforms. + * + * Beyond the base `Agents.ElicitationPropertySchema` type, this also validates + * the extended subset the client renders: `pattern`, `format`, `oneOf` + * (single-select from a labeled const list), and `array`/`items`/`minItems`/ + * `maxItems` (multi-select). `enumNames` is display-only labeling for `enum` + * and is intentionally never validated. * @param {string} key * @param {unknown} value * @param {import('librechat-data-provider').Agents.ElicitationPropertySchema} property * @returns {string | null} */ const validateElicitationField = (key, value, property) => { - const { type, enum: enumValues, minimum, maximum, minLength, maxLength } = property ?? {}; + const { + type, + enum: enumValues, + minimum, + maximum, + minLength, + maxLength, + pattern, + format, + oneOf, + items, + minItems, + maxItems, + } = property ?? {}; + + if (Array.isArray(oneOf) && oneOf.length > 0) { + const allowedConsts = oneOf.map((option) => option?.const); + if (!allowedConsts.includes(value)) { + return `Field '${key}' must be one of the allowed options`; + } + return null; + } + + if (type === 'array') { + if (!Array.isArray(value)) { + return `Field '${key}' must be an array`; + } + if (typeof minItems === 'number' && value.length < minItems) { + return `Field '${key}' must have at least ${minItems} item(s)`; + } + if (typeof maxItems === 'number' && value.length > maxItems) { + return `Field '${key}' must have at most ${maxItems} item(s)`; + } + const allowedMembers = getArrayItemMembers(items); + if (allowedMembers && value.some((element) => !allowedMembers.includes(element))) { + return `Field '${key}' contains an invalid selection`; + } + return null; + } if (type === 'string') { if (typeof value !== 'string') { @@ -111,6 +205,23 @@ const validateElicitationField = (key, value, property) => { if (typeof maxLength === 'number' && value.length > maxLength) { return `Field '${key}' must be at most ${maxLength} characters`; } + if (typeof pattern === 'string') { + let regex; + try { + regex = new RegExp(pattern); + } catch { + return `Field '${key}' has an invalid pattern`; + } + if (!regex.test(value)) { + return `Field '${key}' does not match the required pattern`; + } + } + if (typeof format === 'string') { + const formatError = validateElicitationFormat(key, value, format); + if (formatError) { + return formatError; + } + } } else if (type === 'number' || type === 'integer') { if (typeof value !== 'number' || Number.isNaN(value)) { return `Field '${key}' must be a number`; diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index d26254e2ccc..b94a3c8bd48 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -60,7 +60,9 @@ const MISSING_TOOL_TTL_MS = 10_000; * none of it. Keyed by `flowId`, this registry lets the route re-validate the * submitted `content` against the real schema and emit `on_elicitation_resolved` * back onto the originating stream. Entries are evicted on resolution or by TTL. - * @type {Map} + * `elicitationId` (url mode only) is retained alongside them for future + * `notifications/elicitation/complete` correlation. + * @type {Map} */ const elicitationFlowContext = new Map(); const ELICITATION_CONTEXT_TTL_MS = 10 * 60 * 1000; @@ -403,17 +405,30 @@ function createOAuthCallback({ runStepEmitter, runStepDeltaEmitter }) { * @param {ServerResponse} params.res - The Express response object for sending events. * @param {string} params.stepId - The ID of the step. * @param {string | null} [params.streamId] - The stream ID for resumable mode. - * @returns {(params: { flowId: string; mode: 'form' | 'url'; message: string; serverName?: string; toolName?: string; requestedSchema?: object; url?: string }) => Promise} + * @returns {(params: { flowId: string; mode: 'form' | 'url'; message: string; serverName?: string; toolName?: string; requestedSchema?: object; url?: string; elicitationId?: string }) => Promise} */ function createElicitationStart({ res, stepId, streamId = null }) { - return async function ({ flowId, mode, message, serverName, toolName, requestedSchema, url }) { + return async function ({ + flowId, + mode, + message, + serverName, + toolName, + requestedSchema, + url, + elicitationId, + }) { // Capture stream context + schema so the out-of-band completion route can // validate the response and emit `on_elicitation_resolved` onto this stream. + // `elicitationId` (url mode only) is retained for future + // `notifications/elicitation/complete` correlation; it is not part of the + // client-facing `on_elicitation` payload below. elicitationFlowContext.set(flowId, { res, streamId, stepId, requestedSchema, + elicitationId, createdAt: Date.now(), }); evictStale(elicitationFlowContext, ELICITATION_CONTEXT_TTL_MS); @@ -438,7 +453,7 @@ function createElicitationStart({ res, stepId, streamId = null }) { * recover the form `requestedSchema` for server-side validation when it is not * (yet) threaded through the flow's persisted metadata. * @param {string} flowId - * @returns {{ res?: import('http').ServerResponse, streamId: string | null, stepId: string, requestedSchema?: object, createdAt: number } | undefined} + * @returns {{ res?: import('http').ServerResponse, streamId: string | null, stepId: string, requestedSchema?: object, elicitationId?: string, createdAt: number } | undefined} */ function getElicitationFlowContext(flowId) { return elicitationFlowContext.get(flowId); diff --git a/api/server/services/__tests__/MCP.spec.js b/api/server/services/__tests__/MCP.spec.js index 12d25ea75a8..f9b09d0d192 100644 --- a/api/server/services/__tests__/MCP.spec.js +++ b/api/server/services/__tests__/MCP.spec.js @@ -253,6 +253,30 @@ describe('createElicitationStart', () => { expect.objectContaining({ requestedSchema, streamId: 'stream-ctx', stepId: 'step-ctx' }), ); }); + + it('retains the server-supplied elicitationId in the flow context without leaking it onto the SSE event', async () => { + const start = createElicitationStart({ res: {}, stepId: 'step-eid', streamId: 'stream-eid' }); + + await start({ + flowId: 'flow-eid', + mode: 'url', + message: 'Authorize', + url: 'https://x/auth', + elicitationId: 'elicit-9', + }); + + expect(getElicitationFlowContext('flow-eid')).toEqual( + expect.objectContaining({ elicitationId: 'elicit-9' }), + ); + expect(GenerationJobManager.emitChunk).toHaveBeenCalledWith( + 'stream-eid', + expect.objectContaining({ + data: expect.objectContaining({ + elicitation: expect.not.objectContaining({ elicitationId: expect.anything() }), + }), + }), + ); + }); }); describe('resolveElicitationFlow', () => { diff --git a/client/src/components/Chat/Messages/Content/ElicitationForm.tsx b/client/src/components/Chat/Messages/Content/ElicitationForm.tsx index 7d558598a6b..bdb2ee61e06 100644 --- a/client/src/components/Chat/Messages/Content/ElicitationForm.tsx +++ b/client/src/components/Chat/Messages/Content/ElicitationForm.tsx @@ -9,6 +9,7 @@ import { RotateCw, CheckCircle2, XCircle, + TriangleAlert, } from 'lucide-react'; import type { Agents } from 'librechat-data-provider'; import type { TranslationKeys } from '~/hooks/useLocalize'; @@ -23,7 +24,25 @@ type ElicitationField = { schema: Agents.ElicitationPropertySchema; }; -type FieldValue = string | number | boolean; +type FieldValue = string | number | boolean | string[] | number[]; + +/** Normalizes a `type: 'array'` property's option source — either a plain + * `items.enum` value list or titled `items.anyOf` const/title pairs — into a + * single shape the multi-select checkbox group renders from. */ +type ArrayOption = { value: string | number; label: string }; + +function getArrayOptions(schema: Agents.ElicitationPropertySchema): ArrayOption[] { + if (schema.items?.anyOf) { + return schema.items.anyOf.map((option) => ({ + value: option.const as string | number, + label: option.title ?? String(option.const), + })); + } + if (schema.items?.enum) { + return schema.items.enum.map((value) => ({ value, label: String(value) })); + } + return []; +} function getDefaultValues( properties: Record, @@ -34,6 +53,8 @@ function getDefaultValues( defaults[key] = schema.default; } else if (schema.type === 'boolean') { defaults[key] = false; + } else if (schema.type === 'array') { + defaults[key] = []; } else { defaults[key] = ''; } @@ -41,6 +62,12 @@ function getDefaultValues( return defaults; } +/** Lightweight `format: 'email'` check — not RFC-5322-exhaustive, just enough + * to catch obviously malformed input before it's sent to the server. */ +function isValidEmail(value: string): boolean { + return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value); +} + function getStatusText( resolvedAction: ElicitationAction, localize: (key: TranslationKeys) => string, @@ -72,6 +99,25 @@ function getSafeUrl(url?: string): string | undefined { } } +/** Hostname of a URL already known to be a safe http(s) target (see + * `getSafeUrl`) — used to highlight the domain the user is about to visit, + * mitigating long-path/subdomain spoofing where the real host is buried. */ +function getHostname(url: string): string { + try { + return new URL(url).hostname; + } catch { + return ''; + } +} + +/** True when any label of the hostname is IDNA/Punycode-encoded (`xn--...`). + * The URL parser itself converts any non-ASCII (mixed-script/homograph) + * hostname label to its `xn--` form, so this single check also catches + * Cyrillic/Greek lookalike domains — not just literal Punycode input. */ +function hasPunycodeLabel(hostname: string): boolean { + return hostname.split('.').some((label) => label.toLowerCase().startsWith('xn--')); +} + /** Header chrome shared by both modes: a tinted circular icon, a title, and the * requesting server/tool identity. Keeps the card visually native to LibreChat's * other in-chat system cards (see `ToolCall` OAuth sign-in). */ @@ -170,6 +216,10 @@ export default function ElicitationForm({ // `getSafeUrl`. A present-but-unsafe `url` renders a warning instead of a link // and permanently withholds the `urlOpened` unlock (no link, nothing to click). const safeUrl = useMemo(() => getSafeUrl(url), [url]); + // Domain highlight + Punycode/homograph warning, shown alongside the full URL + // text so the user can examine the real destination before clicking anything. + const hostname = useMemo(() => (safeUrl ? getHostname(safeUrl) : ''), [safeUrl]); + const suspiciousHostname = useMemo(() => hasPunycodeLabel(hostname), [hostname]); const fields: ElicitationField[] = Object.entries(properties).map(([key, schema]) => ({ key, @@ -187,7 +237,11 @@ export default function ElicitationForm({ const required = requestedSchema?.required?.includes(key) ?? false; const val = values[key]; const label = schema.title ?? key; - if (required && (val === '' || val == null)) { + const isEmpty = + schema.type === 'array' + ? !Array.isArray(val) || val.length === 0 + : val === '' || val == null; + if (required && isEmpty) { newErrors[key] = localize('com_ui_elicitation_field_required', { field: label }); } if (schema.type === 'string' && typeof val === 'string') { @@ -197,6 +251,31 @@ export default function ElicitationForm({ if (schema.maxLength != null && val.length > schema.maxLength) { newErrors[key] = localize('com_ui_elicitation_max_length', { max: schema.maxLength }); } + if (val !== '' && schema.pattern != null) { + let matchesPattern = true; + try { + matchesPattern = new RegExp(schema.pattern).test(val); + } catch { + // A malformed server-supplied pattern shouldn't block the user. + matchesPattern = true; + } + if (!matchesPattern) { + newErrors[key] = localize('com_ui_elicitation_pattern_mismatch', { field: label }); + } + } + if (val !== '' && schema.format === 'email' && !isValidEmail(val)) { + newErrors[key] = localize('com_ui_elicitation_not_an_email', { field: label }); + } + if (val !== '' && schema.format === 'uri' && getSafeUrl(val) == null) { + newErrors[key] = localize('com_ui_elicitation_not_a_url', { field: label }); + } + if ( + val !== '' && + (schema.format === 'date' || schema.format === 'date-time') && + Number.isNaN(Date.parse(val)) + ) { + newErrors[key] = localize('com_ui_elicitation_not_a_date', { field: label }); + } } if ((schema.type === 'number' || schema.type === 'integer') && val !== '' && val != null) { const num = Number(val); @@ -208,6 +287,21 @@ export default function ElicitationForm({ newErrors[key] = localize('com_ui_elicitation_max_value', { max: schema.maximum }); } } + if (schema.oneOf && val !== '' && val != null) { + const isAllowed = schema.oneOf.some((option) => String(option.const) === String(val)); + if (!isAllowed) { + newErrors[key] = localize('com_ui_elicitation_invalid_selection', { field: label }); + } + } + if (schema.type === 'array') { + const arr = Array.isArray(val) ? val : []; + if (schema.minItems != null && arr.length < schema.minItems) { + newErrors[key] = localize('com_ui_elicitation_min_items', { min: schema.minItems }); + } + if (schema.maxItems != null && arr.length > schema.maxItems) { + newErrors[key] = localize('com_ui_elicitation_max_items', { max: schema.maxItems }); + } + } } setErrors(newErrors); return Object.keys(newErrors).length === 0; @@ -261,12 +355,26 @@ export default function ElicitationForm({ fields // Omit empty optional fields instead of defaulting them to 0/null — // required fields are already guaranteed non-empty by `validate`. - .filter(({ key }) => values[key] !== '' && values[key] != null) + .filter(({ key, schema }) => { + const val = values[key]; + if (schema.type === 'array') { + return Array.isArray(val) && val.length > 0; + } + return val !== '' && val != null; + }) .map(({ key, schema }) => { const val = values[key]; if (schema.type === 'number' || schema.type === 'integer') { return [key, Number(val)]; } + if (schema.oneOf) { + // The ` toggleArrayValue(key, option.value, e.target.checked)} + disabled={submitting} + className="h-4 w-4 rounded border-border-light accent-ring-primary ring-offset-background focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2" + /> + {option.label} + + ); + })} + + {error && ( +

+ {error} +

+ )} + + ); + } + + if (schema.oneOf) { + return ( +
+ + {schema.description && ( +

+ {schema.description} +

+ )} + + {error && ( +

+ {error} +

+ )} +
+ ); + } + if (schema.enum) { return (
@@ -325,9 +531,9 @@ export default function ElicitationForm({ className="rounded-lg border border-border-light bg-surface-primary px-3 py-2 text-sm text-text-primary ring-offset-background focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2" > - {schema.enum.map((opt) => ( + {schema.enum.map((opt, i) => ( ))} @@ -375,6 +581,22 @@ export default function ElicitationForm({ ); } + // Maps `format` to the closest native input type/keyboard for each — the + // format-specific `validate()` checks below still run since native + // constraint validation alone isn't localized or wired to `errors`. + let inputType: 'number' | 'email' | 'url' | 'date' | 'datetime-local' | 'text' = 'text'; + if (schema.type === 'number' || schema.type === 'integer') { + inputType = 'number'; + } else if (schema.format === 'email') { + inputType = 'email'; + } else if (schema.format === 'uri') { + inputType = 'url'; + } else if (schema.format === 'date') { + inputType = 'date'; + } else if (schema.format === 'date-time') { + inputType = 'datetime-local'; + } + return (