Skip to content

Commit 89695de

Browse files
authored
fix(mcp): cache result of discoverServerTools to prevent post-OAuth refetch storm (#4701)
* fix(mcp): write per-server cache from discoverServerTools to prevent post-OAuth refetch storm * improvement(mcp): update server status from discoverServerTools + cap list-tools timeout at 30s
1 parent d892165 commit 89695de

6 files changed

Lines changed: 44 additions & 5 deletions

File tree

apps/sim/app/api/mcp/oauth/callback/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
167167
}
168168

169169
try {
170-
await mcpService.clearCache(server.workspaceId)
170+
// discoverServerTools writes the result to this server's cache so the UI's
171+
// immediate refetch hits it instead of re-fetching live.
171172
await mcpService.discoverServerTools(session.user.id, server.id, server.workspaceId)
172173
} catch (e) {
173174
logger.warn('Post-auth tools refresh failed', toError(e).message)

apps/sim/lib/mcp/client.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ vi.mock('@modelcontextprotocol/sdk/types.js', () => ({
3737

3838
vi.mock('@/lib/core/execution-limits', () => ({
3939
getMaxExecutionTimeout: vi.fn().mockReturnValue(30000),
40+
DEFAULT_EXECUTION_TIMEOUT_MS: 30000,
4041
}))
4142

4243
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'

apps/sim/lib/mcp/client.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
type McpToolsChangedCallback,
3737
type McpVersionInfo,
3838
} from '@/lib/mcp/types'
39+
import { MCP_CLIENT_CONSTANTS } from '@/lib/mcp/utils'
3940

4041
const logger = createLogger('McpClient')
4142

@@ -167,7 +168,9 @@ export class McpClient {
167168
}
168169

169170
try {
170-
const result: ListToolsResult = await this.client.listTools()
171+
const result: ListToolsResult = await this.client.listTools(undefined, {
172+
timeout: MCP_CLIENT_CONSTANTS.LIST_TOOLS_TIMEOUT_MS,
173+
})
171174

172175
if (!result.tools || !Array.isArray(result.tools)) {
173176
logger.warn(`Invalid tools response from server ${this.config.name}:`, result)

apps/sim/lib/mcp/service.test.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,20 @@ const {
3838
})
3939

4040
vi.mock('@sim/db', () => {
41+
// `where(...)` resolves to the workspace's rows AND exposes `.limit()` for
42+
// chains like `getServerConfig` that do `select().from().where().limit(1)`.
43+
const where = (...args: unknown[]) => {
44+
const rowsPromise = Promise.resolve(mockGetWorkspaceServersRows(...args))
45+
const thenable = Object.assign(rowsPromise, {
46+
limit: (n: number) => rowsPromise.then((rows) => rows.slice(0, n)),
47+
})
48+
return thenable
49+
}
4150
const setter = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) })
4251
return {
4352
db: {
4453
select: vi.fn().mockReturnValue({
45-
from: vi.fn().mockReturnValue({
46-
where: (...args: unknown[]) => mockGetWorkspaceServersRows(...args),
47-
}),
54+
from: vi.fn().mockReturnValue({ where }),
4855
}),
4956
update: vi.fn().mockReturnValue({ set: setter }),
5057
insert: vi.fn(),
@@ -238,4 +245,18 @@ describe('McpService.discoverTools per-server caching', () => {
238245
expect(second.map((t) => t.name)).toEqual(['a-other'])
239246
expect(mockListTools).toHaveBeenCalledTimes(2)
240247
})
248+
249+
it('discoverServerTools primes the per-server cache for follow-up discoverTools', async () => {
250+
mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
251+
mockListTools.mockResolvedValueOnce([tool('a1', 'mcp-a')])
252+
253+
const tools = await mcpService.discoverServerTools(USER_ID, 'mcp-a', WORKSPACE_ID)
254+
expect(tools.map((t) => t.name)).toEqual(['a1'])
255+
expect(mockListTools).toHaveBeenCalledTimes(1)
256+
257+
mockListTools.mockClear()
258+
const second = await mcpService.discoverTools(USER_ID, WORKSPACE_ID)
259+
expect(second.map((t) => t.name)).toEqual(['a1'])
260+
expect(mockListTools).not.toHaveBeenCalled()
261+
})
241262
})

apps/sim/lib/mcp/service.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,17 @@ class McpService {
580580
try {
581581
const tools = await client.listTools()
582582
logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
583+
// Prime the per-server cache and reflect the successful connection on
584+
// the row so the UI doesn't keep showing "Connect with OAuth" or stale
585+
// disconnected/error state.
586+
await Promise.allSettled([
587+
this.cacheAdapter
588+
.set(serverCacheKey(workspaceId, serverId), tools, this.cacheTimeout)
589+
.catch((err) =>
590+
logger.warn(`[${requestId}] Cache write failed for ${config.name}:`, err)
591+
),
592+
this.updateServerStatus(serverId, workspaceId, true, undefined, tools.length),
593+
])
583594
return tools
584595
} finally {
585596
await client.disconnect()

apps/sim/lib/mcp/utils.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export function sanitizeHeaders(
4646
export const MCP_CLIENT_CONSTANTS = {
4747
CLIENT_TIMEOUT: DEFAULT_EXECUTION_TIMEOUT_MS,
4848
AUTO_REFRESH_INTERVAL: 5 * 60 * 1000,
49+
// Cap metadata calls so a slow upstream can't hang the UI for 60s+.
50+
LIST_TOOLS_TIMEOUT_MS: 30_000,
4951
} as const
5052

5153
/**

0 commit comments

Comments
 (0)