Skip to content

Commit 561a32f

Browse files
committed
fix(mcp): keep tools/list absolute timeout ceiling hard
A configured per-server timeout above 60s was expanding maxTotalTimeout past the intended absolute discovery ceiling. Clamp the idle timeout to the ceiling too so tools/list can never hang the UI beyond it; connect() and callTool() still honor the full configured/execution timeout.
1 parent 6e24e02 commit 561a32f

2 files changed

Lines changed: 24 additions & 4 deletions

File tree

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ vi.mock('@/lib/core/execution-limits', () => ({
6464
}))
6565

6666
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
67+
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
6768
import { McpClient } from './client'
6869
import type { McpClientOptions, McpServerConfig } from './types'
6970

@@ -82,6 +83,9 @@ describe('McpClient notification handler', () => {
8283
vi.clearAllMocks()
8384
mockSdkConnect.mockResolvedValue(undefined)
8485
mockSdkListTools.mockResolvedValue({ tools: [] })
86+
// clearAllMocks resets call history but not implementations; re-establish the
87+
// default so a per-test override can't bleed into later tests.
88+
vi.mocked(getMaxExecutionTimeout).mockReturnValue(30_000)
8589
})
8690

8791
it('fires onToolsChanged when a notification arrives while connected', async () => {
@@ -174,6 +178,22 @@ describe('McpClient notification handler', () => {
174178
)
175179
})
176180

181+
it('clamps a configured tools/list timeout to the absolute discovery ceiling', async () => {
182+
vi.mocked(getMaxExecutionTimeout).mockReturnValue(120_000)
183+
const client = new McpClient({
184+
config: { ...createConfig(), timeout: 300_000 },
185+
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
186+
})
187+
188+
await client.connect()
189+
await client.listTools()
190+
191+
expect(mockSdkListTools).toHaveBeenCalledWith(
192+
undefined,
193+
expect.objectContaining({ timeout: 60_000, maxTotalTimeout: 60_000 })
194+
)
195+
})
196+
177197
it('logs connection diagnostics without header values', async () => {
178198
const client = new McpClient({
179199
config: {

apps/sim/lib/mcp/client.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,16 +226,16 @@ export class McpClient {
226226
}
227227

228228
const configuredTimeout = this.config.timeout
229+
// Idle timeout honors the per-server config but never exceeds the absolute
230+
// discovery ceiling, so tools/list can't hang the UI past that cap.
229231
const idleTimeoutMs = Math.min(
230232
configuredTimeout !== undefined && Number.isFinite(configuredTimeout) && configuredTimeout > 0
231233
? Math.floor(configuredTimeout)
232234
: MCP_CLIENT_CONSTANTS.LIST_TOOLS_TIMEOUT_MS,
233-
getMaxExecutionTimeout()
234-
)
235-
const maxTotalTimeoutMs = Math.max(
236-
idleTimeoutMs,
235+
getMaxExecutionTimeout(),
237236
MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_TOTAL_TIMEOUT_MS
238237
)
238+
const maxTotalTimeoutMs = MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_TOTAL_TIMEOUT_MS
239239

240240
try {
241241
const result: ListToolsResult = await this.client.listTools(undefined, {

0 commit comments

Comments
 (0)