improvement(mship): mcp persistence in chat

This commit is contained in:
Siddharth Ganesan
2026-07-27 12:32:35 -07:00
committed by GitHub
parent ae49070bea
commit 67ffdac13f
6 changed files with 129 additions and 9 deletions
+4 -1
View File
@@ -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[]
+75
View File
@@ -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', {
+39 -3
View File
@@ -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<string>()
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,
@@ -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: "<exact 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'),
}
+2 -1
View File
@@ -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',
+6 -1
View File
@@ -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: {