Skip to content

Commit dc65283

Browse files
committed
fix(mothership): build integration tool schemas once per process instead of per user every 5s
The integration catalog cached each viewer's full schema set for 5s, so nearly every call rebuilt a user-tool schema for all ~5.3k exposed tools, sized the ~7.7MB result with JSON.stringify, structuredClone'd it, and re-tokenized every tool for BM25 - all synchronously. The schemas depend only on the registry and deployment config, so they are now built once per (surface, personal mode), frozen, and shared; the build yields between tools. Each request still applies the viewer's own block visibility and permission policy, and the plan-dependent email tagline is read per user with the same 5s TTL. Results are byte-identical. BM25 term counts are memoized per shared schema object.
1 parent 77c3858 commit dc65283

5 files changed

Lines changed: 213 additions & 165 deletions

File tree

‎apps/sim/lib/integrations/tool-projection.ts‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,6 @@ export function projectIntegrationToolsForViewer(
7676
return { tools, allowedBlockTypes, isToolAllowed }
7777
}
7878

79-
/**
80-
* Stable signature of the policy {@link projectIntegrationToolsForViewer} reads,
81-
* for keying caches whose contents depend on the projection.
82-
*
83-
* The projection is only as fresh as what keys it: an entry cached under a
84-
* viewer's identity alone outlives the policy that produced it, so an admin's
85-
* change would not take effect until the entry expired. Mirrors
86-
* `visibilitySignature`, which does the same job for block visibility.
87-
*/
88-
export function integrationGateSignature(config: IntegrationGateConfig | null | undefined): string {
89-
return JSON.stringify([
90-
config?.allowedIntegrations ? [...config.allowedIntegrations].sort() : null,
91-
config?.deniedTools?.length ? [...config.deniedTools].sort() : null,
92-
])
93-
}
94-
9579
/** What a viewer's `deniedTools` denylist costs the block schemas they are shown. */
9680
export interface DeniedBlockOperations {
9781
/**

‎apps/sim/lib/mothership/block-visibility.ts‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,3 @@ export function getBlockVisibilityForCopilot(
4646
}
4747
return promise
4848
}
49-
50-
/**
51-
* Stable signature of a visibility state, for keying caches whose contents
52-
* depend on the gated projection (e.g. the integration tool-schema LRU).
53-
*/
54-
export function visibilitySignature(vis: BlockVisibilityState): string {
55-
return JSON.stringify([
56-
[...vis.revealed].sort(),
57-
[...vis.disabled].sort(),
58-
[...vis.previewTagged].sort(),
59-
])
60-
}

‎apps/sim/lib/mothership/chat/payload.test.ts‎

Lines changed: 65 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
workspaceFileManagerMock,
2323
workspaceFileManagerMockFns,
2424
} from '@sim/testing/mocks/workspace-file-manager.mock'
25-
import { beforeEach, describe, expect, it, vi } from 'vitest'
25+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2626
import { getExposedIntegrationTools } from '@/lib/integrations/tool-catalog'
2727
import { ChatPayloadSchema, TOOL_DEADLINE_CAPABILITY } from '@/lib/mothership/generated/protocol'
2828
import { searchUsersV2Tool } from '@/tools/github/search_users'
@@ -128,7 +128,6 @@ vi.mock('@/lib/mothership/block-visibility', () => ({
128128
disabled: new Set<string>(),
129129
previewTagged: new Set<string>(),
130130
})),
131-
visibilitySignature: vi.fn(() => 'vis:none'),
132131
}))
133132

134133
vi.mock('@/lib/integrations/tool-catalog', () => ({
@@ -273,38 +272,83 @@ describe('buildIntegrationToolSchemas', () => {
273272
expect(mockCreateUserToolSchema).not.toHaveBeenCalled()
274273
})
275274

276-
it('briefly reuses built schemas for the same user and surface', async () => {
275+
it('builds each tool schema once per process, not once per viewer', async () => {
277276
mockGetHighestPrioritySubscription.mockResolvedValue({ plan: 'pro', status: 'active' })
277+
mockGetUserPermissionConfig.mockResolvedValueOnce({ allowedIntegrations: ['gmail'] })
278278

279-
const first = await buildIntegrationToolSchemas('user-cache')
280-
first[0].input_schema.mutated = true
281-
if (first[0].outputs) first[0].outputs.mutated = true
282-
const second = await buildIntegrationToolSchemas('user-cache')
279+
await buildIntegrationToolSchemas('user-a', { schemaSurface: 'copilot' }, 'workspace-a')
280+
await buildIntegrationToolSchemas('user-b', { schemaSurface: 'copilot' }, 'workspace-b')
281+
await buildIntegrationToolSchemas('user-b', { schemaSurface: 'copilot' }, 'workspace-b')
283282

284-
expect(mockGetHighestPrioritySubscription).toHaveBeenCalledTimes(1)
285283
expect(mockCreateUserToolSchema).toHaveBeenCalledTimes(3)
286-
expect(second[0].input_schema).not.toHaveProperty('mutated')
287-
expect(second[0].outputs).not.toHaveProperty('mutated')
288284
})
289285

290-
it('isolates nested schemas and required fields between requests', async () => {
291-
mockCreateUserToolSchema.mockReturnValueOnce({
286+
it('keeps one caller from changing what any other viewer receives', async () => {
287+
mockGetHighestPrioritySubscription.mockResolvedValue({ plan: 'pro', status: 'active' })
288+
mockCreateUserToolSchema.mockReturnValue({
292289
type: 'object',
293290
properties: { recipients: { type: 'array', items: { type: 'string' } } },
294291
required: ['recipients'],
295292
})
296-
const first = await buildIntegrationToolSchemas('user-nested-schema')
297-
const properties = first[0].input_schema.properties as Record<string, unknown>
298-
properties.recipients = { type: 'number' }
299-
;(first[0].input_schema.required as string[]).push('forged')
300-
301-
const second = await buildIntegrationToolSchemas('user-nested-schema')
293+
const first = await buildIntegrationToolSchemas('user-mutating')
294+
first.push({ name: 'mcp-private-tool', description: '', input_schema: {} })
295+
try {
296+
;(first[0].input_schema.properties as Record<string, unknown>).recipients = { type: 'number' }
297+
;(first[0].input_schema.required as string[]).push('forged')
298+
first[0].description = 'forged'
299+
} catch {}
300+
301+
const second = await buildIntegrationToolSchemas('user-reading')
302+
expect(second.map((tool) => tool.name)).toEqual([
303+
'gmail_send',
304+
'brandfetch_search',
305+
'run_workflow',
306+
])
307+
expect(second[0].description).not.toBe('forged')
302308
expect(second[0].input_schema.properties).toEqual({
303309
recipients: { type: 'array', items: { type: 'string' } },
304310
})
305311
expect(second[0].input_schema.required).toEqual(['recipients'])
306312
})
307313

314+
it('appends the free-plan email tagline only for the viewer whose plan earns it', async () => {
315+
mockGetHighestPrioritySubscription
316+
.mockResolvedValueOnce(null)
317+
.mockResolvedValueOnce({ plan: 'pro', status: 'active' })
318+
const tagline = 'sent with sim ai'
319+
const describeGmail = (tools: Array<{ name: string; description: string }>) =>
320+
tools.find((tool) => tool.name === 'gmail_send')?.description ?? ''
321+
322+
const free = await buildIntegrationToolSchemas('free-user')
323+
const paid = await buildIntegrationToolSchemas('paid-user')
324+
const freeAgain = await buildIntegrationToolSchemas('free-user')
325+
326+
expect(describeGmail(free)).toContain(tagline)
327+
expect(describeGmail(paid)).not.toContain(tagline)
328+
expect(describeGmail(freeAgain)).toContain(tagline)
329+
})
330+
331+
it("gates the shared schemas by each viewer's own permission policy", async () => {
332+
mockGetHighestPrioritySubscription.mockResolvedValue({ plan: 'pro', status: 'active' })
333+
mockGetUserPermissionConfig
334+
.mockResolvedValueOnce({ allowedIntegrations: null, deniedTools: ['gmail_send'] })
335+
.mockResolvedValueOnce(null)
336+
337+
const restricted = await buildIntegrationToolSchemas(
338+
'restricted-user',
339+
{ schemaSurface: 'copilot' },
340+
'workspace-shared'
341+
)
342+
const unrestricted = await buildIntegrationToolSchemas(
343+
'open-user',
344+
{ schemaSurface: 'copilot' },
345+
'workspace-shared'
346+
)
347+
348+
expect(restricted.map((tool) => tool.name)).not.toContain('gmail_send')
349+
expect(unrestricted.map((tool) => tool.name)).toContain('gmail_send')
350+
})
351+
308352
it('coalesces simultaneous catalog builds', async () => {
309353
const catalogs = await Promise.all(
310354
Array.from({ length: 20 }, () => buildIntegrationToolSchemas('concurrent-user'))
@@ -323,7 +367,6 @@ describe('buildIntegrationToolSchemas', () => {
323367
)
324368
const recovered = await buildIntegrationToolSchemas('schema-failure')
325369
expect(recovered).toHaveLength(3)
326-
expect(mockGetHighestPrioritySubscription).toHaveBeenCalledTimes(2)
327370
})
328371

329372
it('propagates subscription failures without caching a guessed catalog', async () => {
@@ -335,18 +378,6 @@ describe('buildIntegrationToolSchemas', () => {
335378
expect(await buildIntegrationToolSchemas('subscription-failure')).toHaveLength(3)
336379
})
337380

338-
it('evicts catalogs by their byte size before reaching the entry cap', async () => {
339-
mockCreateUserToolSchema.mockImplementation(() => ({
340-
type: 'object',
341-
properties: { large: { type: 'string', description: 'x'.repeat(1024 * 1024) } },
342-
}))
343-
for (let i = 0; i < 12; i++) await buildIntegrationToolSchemas(`sized-user-${i}`)
344-
expect(mockGetHighestPrioritySubscription).toHaveBeenCalledTimes(12)
345-
await buildIntegrationToolSchemas('sized-user-0')
346-
expect(mockGetHighestPrioritySubscription).toHaveBeenCalledTimes(13)
347-
mockCreateUserToolSchema.mockImplementation(() => ({ type: 'object', properties: {} }))
348-
})
349-
350381
it('rebuilds instead of serving a cache entry from the previous policy', async () => {
351382
mockGetHighestPrioritySubscription.mockResolvedValue({ plan: 'pro', status: 'active' })
352383
mockGetUserPermissionConfig.mockResolvedValue({ allowedIntegrations: null, deniedTools: [] })
@@ -660,6 +691,7 @@ describe('buildCopilotRequestPayload', () => {
660691
})
661692

662693
describe('Assistant payload', () => {
694+
afterEach(() => vi.mocked(getExposedIntegrationTools).mockReset())
663695
beforeEach(() => {
664696
resetEnvFlagsMock()
665697
mockGetUserPermissionConfig.mockResolvedValue(null)
@@ -671,7 +703,7 @@ describe('Assistant payload', () => {
671703
it('discovers the existing GitHub user lookup tool with a personal credential in live Search', async () => {
672704
clearIntegrationToolSchemaCacheForTests()
673705
mockSearchApprovals.mockResolvedValue(new Map([['github', true]]))
674-
vi.mocked(getExposedIntegrationTools).mockReturnValueOnce([
706+
vi.mocked(getExposedIntegrationTools).mockReturnValue([
675707
{
676708
toolId: searchUsersV2Tool.id,
677709
config: searchUsersV2Tool,
@@ -706,7 +738,7 @@ describe('Assistant payload', () => {
706738
})
707739
it('advertises approved personal organization integrations and rechecks revocation', async () => {
708740
clearIntegrationToolSchemaCacheForTests()
709-
vi.mocked(getExposedIntegrationTools).mockReturnValueOnce([
741+
vi.mocked(getExposedIntegrationTools).mockReturnValue([
710742
{
711743
toolId: gmailListLabelsV2Tool.id,
712744
config: gmailListLabelsV2Tool,

0 commit comments

Comments
 (0)