Skip to content

Commit 554714a

Browse files
committed
fix(mothership): remove eager catalogs and bulky admission saves
1 parent a5fd8a7 commit 554714a

24 files changed

Lines changed: 1165 additions & 189 deletions

File tree

‎apps/sim/app/api/mothership/execute/route.test.ts‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ describe('buildExecuteResponsePayload', () => {
109109
it('still admits integration and mcp tool calls, and still drops other server tools', () => {
110110
const payload = buildExecuteResponsePayload(
111111
resultWithToolCalls(['gmail_send', 'mcp-notion-create', 'read', 'edit_workflow']),
112-
'chat-1',
113-
[{ name: 'gmail_send' }]
112+
'chat-1'
114113
)
115114

116115
const names = payload.toolCalls.map((tc: { name: string }) => tc.name)
@@ -422,6 +421,17 @@ describe('mothership private trace provenance transport', () => {
422421
'POST',
423422
{
424423
...requestBody,
424+
mcpTools: [
425+
{
426+
type: 'mcp',
427+
params: { serverId: 'mcp-cg-123456789012345678901', toolName: 'search_transcripts' },
428+
},
429+
{
430+
type: 'mcp',
431+
usageControl: 'none',
432+
params: { serverId: 'mcp-cg-123456789012345678901', toolName: 'delete_transcripts' },
433+
},
434+
],
425435
contexts: [
426436
{ kind: 'mcp', label: 'MCP 123', serverId: '123', path: '123' },
427437
{
@@ -445,12 +455,21 @@ describe('mothership private trace provenance transport', () => {
445455
)
446456

447457
expect(response.status).toBe(200)
448-
expect(mockBuildTaggedMcpToolSchemas).toHaveBeenCalledWith(
449-
'user-1',
450-
'workspace-1',
451-
['123'],
452-
expect.objectContaining({ mcpBlockId: 'block-1' })
453-
)
458+
expect(mockRunHeadlessCopilotLifecycle.mock.calls[0]?.[0]).toMatchObject({
459+
integrationCatalog: {
460+
mcpServerIds: ['123'],
461+
mcpToolIds: ['mcp-cg-123456789012345678901-search_transcripts'],
462+
mcpExecution: {
463+
workflowId: 'workflow-1',
464+
executionId: 'execution-1',
465+
mcpBlockId: 'block-1',
466+
subjectUserId: 'user-1',
467+
},
468+
},
469+
})
470+
expect(mockBuildIntegrationToolSchemas).not.toHaveBeenCalled()
471+
expect(mockBuildTaggedMcpToolSchemas).not.toHaveBeenCalled()
472+
expect(mockBuildSelectedMcpToolSchemas).not.toHaveBeenCalled()
454473
expect(mockProcessContextsServer).toHaveBeenCalledWith(
455474
[
456475
{
@@ -638,7 +657,7 @@ describe('mothership private trace provenance transport', () => {
638657
expect(mockGetPersonalAndWorkspaceEnv).toHaveBeenCalledTimes(1)
639658
})
640659

641-
it('keeps discovered MCP schemas raw without activating matching configured secrets', async () => {
660+
it('sends enabled MCP IDs without schemas or activating matching configured secrets', async () => {
642661
mockBuildTaggedMcpToolSchemas.mockResolvedValueOnce([
643662
{ name: 'mcp-docs', description: 'Uses secret-value' },
644663
])
@@ -653,9 +672,18 @@ describe('mothership private trace provenance transport', () => {
653672
entries: [],
654673
scope: { userId: 'user-1', workspaceId: 'workspace-1' },
655674
})
656-
expect(payload.mothershipTools).toEqual([
657-
{ name: 'mcp-docs', description: 'Uses secret-value' },
658-
])
675+
expect(payload).not.toHaveProperty('mothershipTools')
676+
expect(payload).not.toHaveProperty('integrationTools')
677+
expect(payload.integrationCatalog).toEqual({
678+
mcpServerIds: ['server-1'],
679+
mcpToolIds: [],
680+
mcpExecution: {
681+
workflowId: 'workflow-1',
682+
executionId: 'execution-1',
683+
mcpBlockId: 'block-1',
684+
subjectUserId: 'user-1',
685+
},
686+
})
659687
expect(JSON.stringify(payload.messages)).toContain('search_integration_tools')
660688
expect(JSON.stringify(payload.messages)).toContain('call_integration_tool')
661689
expect(JSON.stringify(payload.messages)).not.toContain('callable directly')

‎apps/sim/app/api/mothership/execute/route.ts‎

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { serializePrincipal } from '@sim/auth/principal'
12
import { createLogger } from '@sim/logger'
23
import { getErrorMessage, toError } from '@sim/utils/errors'
34
import { generateId } from '@sim/utils/id'
@@ -18,18 +19,19 @@ import {
1819
} from '@/lib/execution/private-tool-metadata'
1920
import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor'
2021
import { MCP_SERVER_DELEGATION_AUDIENCE } from '@/lib/mcp/application/authorization'
21-
import { buildIntegrationToolSchemas } from '@/lib/mothership/chat/payload'
22+
import { resolveMcpToolBinding } from '@/lib/mcp/tool-binding'
23+
import { createMcpToolId } from '@/lib/mcp/utils'
2224
import { processContextsServer } from '@/lib/mothership/chat/process-contents'
2325
import {
2426
type CopilotEnvironmentContext,
2527
createCopilotEnvironmentContext,
2628
} from '@/lib/mothership/environment-context'
29+
import { IntegrationCatalogMcpExecution } from '@/lib/mothership/generated/integration-catalog'
2730
import {
2831
MothershipStreamV1EventType,
2932
MothershipStreamV1TextChannel,
3033
} from '@/lib/mothership/generated/mothership-stream-v1'
3134
import { PROTOCOL_VERSION } from '@/lib/mothership/generated/protocol'
32-
import { buildSelectedMcpToolSchemas, buildTaggedMcpToolSchemas } from '@/lib/mothership/mcp-tools'
3335
import { runHeadlessCopilotLifecycle } from '@/lib/mothership/request/lifecycle/headless'
3436
import { requestExplicitStreamAbort } from '@/lib/mothership/request/session/explicit-abort'
3537
import type { StreamEvent } from '@/lib/mothership/request/types'
@@ -43,6 +45,7 @@ import {
4345
type ResolvedSecretTraceRegistry,
4446
} from '@/executor/utils/resolved-secret-trace-registry'
4547
import type { ChatContext } from '@/stores/panel'
48+
import { hasToolId } from '@/tools/tool-ids'
4649

4750
export const maxDuration = 3600
4851

@@ -94,12 +97,10 @@ function encodeNdjson(value: unknown): Uint8Array {
9497

9598
export function buildExecuteResponsePayload(
9699
result: Awaited<ReturnType<typeof runHeadlessCopilotLifecycle>>,
97-
effectiveChatId: string,
98-
integrationTools: Array<{ name: string }>
100+
effectiveChatId: string
99101
) {
100-
const clientToolNames = new Set(integrationTools.map((t) => t.name))
101102
const clientToolCalls = (result.toolCalls || []).filter(
102-
(tc: { name: string }) => clientToolNames.has(tc.name) || tc.name.startsWith('mcp-')
103+
(tc: { name: string }) => hasToolId(tc.name) || tc.name.startsWith('mcp-')
103104
)
104105

105106
return {
@@ -246,34 +247,25 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
246247
)
247248
const nonMcpAgentMentions = agentMentions?.filter((context) => context.kind !== 'mcp')
248249
const userPermission = workspaceAccess.permission
249-
const mothershipToolsPromise = Promise.allSettled([
250-
buildSelectedMcpToolSchemas(userId, workspaceId, mcpTools ?? [], mcpContext),
251-
buildTaggedMcpToolSchemas(userId, workspaceId, taggedMcpServerIds, mcpContext),
252-
]).then((results) => {
253-
const groups = results.map((result) => {
254-
if (result.status === 'rejected') throw result.reason
255-
return result.value
250+
const selectedMcpToolIds = (mcpTools ?? [])
251+
.filter((tool) => tool.type === 'mcp' && (tool.usageControl || 'auto') !== 'none')
252+
.map((tool) => {
253+
const { serverId, toolName } = resolveMcpToolBinding(tool)
254+
return createMcpToolId(serverId, toolName)
256255
})
257-
const byName = new Map(groups.flat().map((tool) => [tool.name, tool]))
258-
return [...byName.values()]
256+
const agentContexts = await processContextsServer(
257+
nonMcpAgentMentions,
258+
userId,
259+
lastUserMessage,
260+
workspaceId,
261+
effectiveChatId,
262+
activeResolvedSecretTraceRegistry
263+
).catch((error) => {
264+
reqLogger.warn('Failed to resolve agent contexts for execution', {
265+
error: toError(error).message,
266+
})
267+
return []
259268
})
260-
const [integrationTools, mothershipTools, agentContexts] = await Promise.all([
261-
buildIntegrationToolSchemas(userId, undefined, workspaceId),
262-
mothershipToolsPromise,
263-
processContextsServer(
264-
nonMcpAgentMentions,
265-
userId,
266-
lastUserMessage,
267-
workspaceId,
268-
effectiveChatId,
269-
activeResolvedSecretTraceRegistry
270-
).catch((error) => {
271-
reqLogger.warn('Failed to resolve agent contexts for execution', {
272-
error: toError(error).message,
273-
})
274-
return []
275-
}),
276-
])
277269
/**
278270
* The wire payload IS the shared ExecuteRequest contract. Caller-side context —
279271
* resolved mentions and the MCP-enablement notice — folds into the message array
@@ -285,12 +277,13 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
285277
const c = ctx as { type?: string; content?: string }
286278
return `[Attached ${c.type ?? 'context'}]\n${c.content ?? ''}`
287279
}),
288-
...(mothershipTools.length > 0
280+
...(taggedMcpServerIds.length || selectedMcpToolIds.length
289281
? [
290282
[
291-
'The following MCP operations are enabled. Find their input schemas with search_integration_tools using the exact toolId shown, then invoke them through call_integration_tool.',
283+
'MCP operations are enabled. Discover their current input schemas with search_integration_tools, then invoke them through call_integration_tool.',
292284
'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.',
293-
...mothershipTools.map((tool) => `- ${tool.name}: ${tool.description || tool.name}`),
285+
...taggedMcpServerIds.map((id) => `Enabled MCP server: ${id}`),
286+
...selectedMcpToolIds.map((id) => `Enabled MCP operation: ${id}`),
294287
].join('\n'),
295288
]
296289
: []),
@@ -310,8 +303,18 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
310303
workspaceId,
311304
chatId: effectiveChatId,
312305
messageId,
313-
...(integrationTools.length > 0 ? { integrationTools } : {}),
314-
...(mothershipTools.length > 0 ? { mothershipTools } : {}),
306+
integrationCatalog: {
307+
mcpServerIds: taggedMcpServerIds,
308+
mcpToolIds: selectedMcpToolIds,
309+
mcpExecution: IntegrationCatalogMcpExecution.parse({
310+
workflowId: delegation.workflowId,
311+
executionId: delegation.executionId,
312+
mcpBlockId: delegation.mcpBlockId,
313+
subjectUserId: delegation.subjectUserId,
314+
...(delegation.principal ? { principal: serializePrincipal(delegation.principal) } : {}),
315+
...(delegation.currentWorkflow ? { currentWorkflow: delegation.currentWorkflow } : {}),
316+
}),
317+
},
315318
}
316319

317320
let allowExplicitAbort = true
@@ -491,7 +494,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
491494
send({
492495
type: 'final',
493496
data: withPrivateProvenance(
494-
buildExecuteResponsePayload(result, effectiveChatId, integrationTools),
497+
buildExecuteResponsePayload(result, effectiveChatId),
495498
resolvedSecretTraceRegistry,
496499
includePrivateProvenance
497500
),
@@ -619,7 +622,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
619622

620623
return NextResponse.json(
621624
withPrivateProvenance(
622-
buildExecuteResponsePayload(result, effectiveChatId, integrationTools),
625+
buildExecuteResponsePayload(result, effectiveChatId),
623626
resolvedSecretTraceRegistry,
624627
includePrivateProvenance
625628
),
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/** @vitest-environment node */
2+
import { NextRequest } from 'next/server'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const mocks = vi.hoisted(() => ({ build: vi.fn(), workspace: vi.fn(), permission: vi.fn() }))
6+
vi.unmock('@/lib/mothership/request/http')
7+
vi.mock('@/lib/mothership/chat/payload', () => ({ buildIntegrationToolSchemas: mocks.build }))
8+
vi.mock('@/lib/mothership/mcp-tools', () => ({ buildTaggedMcpToolSchemas: vi.fn() }))
9+
vi.mock('@/lib/mcp/application/use-cases', () => ({ listMcpServersUseCase: { execute: vi.fn() } }))
10+
vi.mock('@/lib/workspaces/application/workspace-context', () => ({
11+
resolveActiveWorkspaceApplicationContext: mocks.workspace,
12+
}))
13+
vi.mock('@sim/platform-authz/workspace', () => ({
14+
resolveEffectiveWorkspacePermission: mocks.permission,
15+
permissionSatisfies: (actual: string | null, needed: string) =>
16+
actual === 'admin' || actual === 'write' || actual === needed,
17+
}))
18+
vi.mock('@/lib/mothership/chat/application/workspace-context', () => ({
19+
readWorkspaceContext: { execute: vi.fn() },
20+
}))
21+
vi.mock('@/lib/mothership/request/application/read-control', () => ({
22+
RUN_CONTROL_AUDIENCE: 'control',
23+
readRunControl: { execute: vi.fn() },
24+
}))
25+
vi.mock('@/lib/mothership/tasks/application/read-workflow-status', () => ({
26+
readWatchedWorkflowStatus: { execute: vi.fn() },
27+
}))
28+
vi.mock('@/lib/mothership/tasks/application/prepare-wake', () => ({
29+
prepareTaskWake: { execute: vi.fn() },
30+
}))
31+
vi.mock('@/lib/mothership/tasks/application/context', () => ({ TASK_DELEGATION_AUDIENCE: 'tasks' }))
32+
vi.mock('@/lib/mothership/tasks/wake', () => ({ runWakeTurn: vi.fn() }))
33+
34+
import { env } from '@/lib/core/config/env'
35+
import { executeSimControl } from '@/lib/mothership/transport/control'
36+
import { POST } from '@/app/api/mothership/integrations/catalog/route'
37+
38+
const scope = {
39+
userId: 'actor',
40+
workspaceId: '11111111-1111-4111-8111-111111111111',
41+
chatId: '33333333-3333-4333-8333-333333333333',
42+
}
43+
const input = { mode: 'agent' as const, mcpServerIds: [], toolId: 'gmail_send', limit: 1 }
44+
function request(key = env.INTERNAL_API_SECRET ?? '') {
45+
return new NextRequest('http://localhost/api/mothership/integrations/catalog', {
46+
method: 'POST',
47+
headers: {
48+
'content-type': 'application/json',
49+
'x-api-key': key,
50+
'x-mothership-user-id': scope.userId,
51+
'x-mothership-workspace-id': scope.workspaceId,
52+
'x-mothership-chat-id': scope.chatId,
53+
},
54+
body: JSON.stringify(input),
55+
})
56+
}
57+
function checkpoint() {
58+
return executeSimControl({
59+
id: 'catalog-control',
60+
scope,
61+
expiresAt: Date.now() + 5000,
62+
operation: { kind: 'integration_catalog', input },
63+
})
64+
}
65+
beforeEach(() => {
66+
vi.clearAllMocks()
67+
mocks.permission.mockResolvedValue('read')
68+
mocks.workspace.mockResolvedValue({
69+
workspaceId: scope.workspaceId,
70+
workspaceOrganizationId: null,
71+
allowPersonalApiKeys: true,
72+
})
73+
mocks.build.mockResolvedValue([
74+
{
75+
name: 'gmail_send',
76+
description: 'Send email',
77+
service: 'gmail',
78+
input_schema: { type: 'object' },
79+
},
80+
])
81+
})
82+
describe('direct and checkpoint catalog authorization parity', () => {
83+
it('returns identical authorized schema responses through HTTP and outbound control', async () => {
84+
const direct = await POST(request())
85+
const outbound = await checkpoint()
86+
expect(direct.status).toBe(200)
87+
expect(outbound.status).toBe(200)
88+
expect(await direct.json()).toEqual(JSON.parse(outbound.body))
89+
expect(mocks.permission).toHaveBeenCalledTimes(2)
90+
expect(mocks.build).toHaveBeenCalledTimes(2)
91+
})
92+
it('rejects revoked workspace membership on both transports before catalog loading', async () => {
93+
mocks.permission.mockResolvedValue(null)
94+
expect((await POST(request())).status).toBe(403)
95+
expect((await checkpoint()).status).toBe(403)
96+
expect(mocks.build).not.toHaveBeenCalled()
97+
})
98+
it('rejects untrusted HTTP keys before canonical context loading', async () => {
99+
expect((await POST(request('browser-key'))).status).toBe(401)
100+
expect(mocks.workspace).not.toHaveBeenCalled()
101+
expect(mocks.build).not.toHaveBeenCalled()
102+
})
103+
})
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { readIntegrationCatalogContract } from '@/lib/api/contracts/mothership-integrations'
2+
import {
3+
defineInternalJsonRoute,
4+
internalOrchestrationErrorPolicy,
5+
internalRateLimits,
6+
} from '@/lib/api/server/routes'
7+
import { internalCopilotAuth } from '@/lib/mothership/auth/internal'
8+
import {
9+
INTEGRATION_CATALOG_AUDIENCE,
10+
readIntegrationCatalog,
11+
readIntegrationCatalogOperation,
12+
} from '@/lib/mothership/integrations/application/catalog'
13+
14+
export const POST = defineInternalJsonRoute({
15+
contract: readIntegrationCatalogContract,
16+
auth: internalCopilotAuth(INTEGRATION_CATALOG_AUDIENCE, { organization: true }),
17+
operation: readIntegrationCatalogOperation,
18+
rateLimit: internalRateLimits.none({
19+
reason: 'Private bounded catalog discovery rechecks current authorization on every request.',
20+
}),
21+
errorPolicy: internalOrchestrationErrorPolicy,
22+
mapInput: ({ body }) => body,
23+
useCase: readIntegrationCatalog,
24+
})

0 commit comments

Comments
 (0)