From c86e87948f2018e387782478639d5525e5950a75 Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan Date: Sat, 25 Jul 2026 15:26:38 -0700 Subject: [PATCH] Fixes --- apps/sim/lib/copilot/chat/payload.ts | 5 +- apps/sim/lib/copilot/chat/post.test.ts | 75 +++++++++++++++++++ apps/sim/lib/copilot/chat/post.ts | 42 ++++++++++- apps/sim/lib/copilot/chat/process-contents.ts | 6 +- apps/sim/lib/copilot/mcp-tools.test.ts | 3 +- apps/sim/lib/copilot/mcp-tools.ts | 7 +- 6 files changed, 129 insertions(+), 9 deletions(-) diff --git a/apps/sim/lib/copilot/chat/payload.ts b/apps/sim/lib/copilot/chat/payload.ts index f02b655fdb5..6893af3d850 100644 --- a/apps/sim/lib/copilot/chat/payload.ts +++ b/apps/sim/lib/copilot/chat/payload.ts @@ -34,7 +34,10 @@ interface BuildPayloadParams { model: string provider?: string contexts?: Array<{ type: string; content: string; tag?: string; path?: string }> - /** MCP servers explicitly tagged on this turn. Untagged servers stay unavailable. */ + /** + * MCP servers enabled for this chat — every server tagged on this or any + * earlier turn. Servers never tagged in the chat stay unavailable. + */ mcpServerIds?: string[] fileAttachments?: Array<{ id: string; key: string; size: number; [key: string]: unknown }> commands?: string[] diff --git a/apps/sim/lib/copilot/chat/post.test.ts b/apps/sim/lib/copilot/chat/post.test.ts index ba72294c37b..660b0941ce6 100644 --- a/apps/sim/lib/copilot/chat/post.test.ts +++ b/apps/sim/lib/copilot/chat/post.test.ts @@ -289,6 +289,81 @@ describe('handleUnifiedChatPost', () => { ) }) + it('keeps MCP servers tagged on earlier turns enabled for the rest of the chat', async () => { + resolveOrCreateChat.mockResolvedValue({ + chatId: 'chat-1', + chat: { id: 'chat-1' }, + conversationHistory: [ + { + id: 'msg-1', + role: 'user', + content: '/Docs search auth', + contexts: [{ kind: 'mcp', serverId: 'mcp-server-1', label: 'Docs' }], + }, + { id: 'msg-2', role: 'assistant', content: 'here you go' }, + ], + isNew: false, + }) + + const response = await handleUnifiedChatPost( + new NextRequest('http://localhost/api/copilot/chat', { + method: 'POST', + body: JSON.stringify({ + message: 'now search billing', + workspaceId: 'ws-1', + chatId: 'chat-1', + }), + }) + ) + + expect(response.status).toBe(200) + expect(buildCopilotRequestPayload).toHaveBeenCalledWith( + expect.objectContaining({ mcpServerIds: ['mcp-server-1'] }), + { selectedModel: '' } + ) + // The tools ride the tool array every turn, so re-expanding the listing for + // an inherited server would only duplicate what the model already sees. + const expandedContexts = processContextsServer.mock.calls[0]?.[0] ?? [] + expect(expandedContexts).not.toContainEqual(expect.objectContaining({ kind: 'mcp' })) + }) + + it('unions MCP servers across turns without duplicating a re-tagged server', async () => { + resolveOrCreateChat.mockResolvedValue({ + chatId: 'chat-1', + chat: { id: 'chat-1' }, + conversationHistory: [ + { + id: 'msg-1', + role: 'user', + content: '/Docs search auth', + contexts: [{ kind: 'mcp', serverId: 'mcp-server-1', label: 'Docs' }], + }, + ], + isNew: false, + }) + + const response = await handleUnifiedChatPost( + new NextRequest('http://localhost/api/copilot/chat', { + method: 'POST', + body: JSON.stringify({ + message: '/Docs /Issues cross-reference', + workspaceId: 'ws-1', + chatId: 'chat-1', + contexts: [ + { kind: 'mcp', serverId: 'mcp-server-1', label: 'Docs' }, + { kind: 'mcp', serverId: 'mcp-server-2', label: 'Issues' }, + ], + }), + }) + ) + + expect(response.status).toBe(200) + expect(buildCopilotRequestPayload).toHaveBeenCalledWith( + expect.objectContaining({ mcpServerIds: ['mcp-server-1', 'mcp-server-2'] }), + { selectedModel: '' } + ) + }) + it('persists cancelled partial responses from the server lifecycle', async () => { await handleUnifiedChatPost( new NextRequest('http://localhost/api/copilot/chat', { diff --git a/apps/sim/lib/copilot/chat/post.ts b/apps/sim/lib/copilot/chat/post.ts index 73b30948c4f..0a92012e76b 100644 --- a/apps/sim/lib/copilot/chat/post.ts +++ b/apps/sim/lib/copilot/chat/post.ts @@ -246,6 +246,44 @@ function normalizeContexts(contexts: UnifiedChatRequest['contexts']) { }) } +/** + * An MCP server tagged with `/name` stays enabled for the rest of the chat, not + * just the turn it was tagged on. Persisted user messages already carry their + * `mcp` contexts, so the transcript is the source of truth — enablement survives + * reloads and reopened chats with no extra state to keep in sync. There is + * deliberately no off switch: history is append-only. + * + * Only the ids travel forward, not the contexts themselves. The tools ride the + * tool array on every turn, so the model always sees their names and schemas; + * re-expanding the prompt listing each turn would just duplicate that. Keeping + * inherited servers out of the persisted contexts also keeps the `/name` chips + * on a sent message showing only what the user actually typed that turn. + */ +function collectChatMcpServerIds( + conversationHistory: unknown[], + currentContexts: UnifiedChatRequest['contexts'] +): string[] { + const serverIds = new Set() + + const collect = (contexts: unknown) => { + if (!Array.isArray(contexts)) return + for (const ctx of contexts) { + if (!ctx || typeof ctx !== 'object') continue + const { kind, serverId } = ctx as { kind?: unknown; serverId?: unknown } + if (kind === 'mcp' && typeof serverId === 'string' && serverId) { + serverIds.add(serverId) + } + } + } + + for (const message of conversationHistory) { + collect((message as { contexts?: unknown } | null)?.contexts) + } + collect(currentContexts) + + return Array.from(serverIds) +} + async function resolveAgentContexts(params: { contexts?: UnifiedChatRequest['contexts'] resourceAttachments?: UnifiedChatRequest['resourceAttachments'] @@ -1003,9 +1041,7 @@ export async function handleUnifiedChatPost(req: NextRequest) { [TraceAttr.CopilotContextsCount]: normalizedContexts.length, }, () => { - const mcpServerIds = normalizedContexts.flatMap((context) => - context.kind === 'mcp' && context.serverId ? [context.serverId] : [] - ) + const mcpServerIds = collectChatMcpServerIds(conversationHistory, normalizedContexts) return branch.kind === 'workflow' ? branch.buildPayload({ message: body.message, diff --git a/apps/sim/lib/copilot/chat/process-contents.ts b/apps/sim/lib/copilot/chat/process-contents.ts index 610e79aa8ed..17c9aa98c14 100644 --- a/apps/sim/lib/copilot/chat/process-contents.ts +++ b/apps/sim/lib/copilot/chat/process-contents.ts @@ -94,9 +94,9 @@ export async function processContextsServer( type: 'mcp', tag: ctx.label ? `/${ctx.label}` : '/', content: [ - `The user explicitly enabled the MCP server "${ctx.label || ctx.serverId}" for this turn.`, - 'Its request-scoped tools are listed below. Load a tool with load_custom_tool({ type: "mcp", name: "" }) before calling it.', - 'Do not narrate discovery, loading, tool-name selection, or retries. Call the tool first, then respond once with the result. Never claim the server works before a successful tool result. Do not automatically retry a timed-out or abandoned MCP call.', + `The user explicitly enabled the MCP server "${ctx.label || ctx.serverId}". It stays enabled for the rest of this chat, and its tools remain callable on every later turn.`, + 'Its tools are listed below and are callable directly by the exact name shown — there is no loading step.', + 'Do not narrate discovery, tool-name selection, or retries. Call the tool first, then respond once with the result. Never claim the server works before a successful tool result. Do not automatically retry a timed-out or abandoned MCP call.', ...toolLines, ].join('\n'), } diff --git a/apps/sim/lib/copilot/mcp-tools.test.ts b/apps/sim/lib/copilot/mcp-tools.test.ts index a41057d6d8d..60c7de69f11 100644 --- a/apps/sim/lib/copilot/mcp-tools.test.ts +++ b/apps/sim/lib/copilot/mcp-tools.test.ts @@ -37,7 +37,8 @@ describe('mothership MCP tool schemas', () => { expect(tools).toEqual([ expect.objectContaining({ name: 'mcp-server-1-search', - defer_loading: true, + // Explicitly enabled by the user, so it is callable without an unlock step. + defer_loading: false, executeLocally: false, params: expect.objectContaining({ mothershipToolKind: 'mcp', diff --git a/apps/sim/lib/copilot/mcp-tools.ts b/apps/sim/lib/copilot/mcp-tools.ts index 0316e1eb55d..24856365e08 100644 --- a/apps/sim/lib/copilot/mcp-tools.ts +++ b/apps/sim/lib/copilot/mcp-tools.ts @@ -21,7 +21,12 @@ function toMothershipMcpTool(tool: { description: tool.description || `MCP tool ${tool.name} from ${tool.serverName || tool.serverId}`, input_schema: tool.inputSchema, - defer_loading: true, + // Not deferred: deferral keeps the 200+ unrequested integration schemas out + // of the tool array, but an MCP server is explicitly enabled by the user and + // stays enabled for the chat. Deferring it means every turn after the first + // needs a load_custom_tool round-trip the model has no listing to prompt it + // for, and a direct call is rejected as unavailable. + defer_loading: false, executeLocally: false, service: `mcp:${tool.serverId}`, params: {