Skip to content

Commit 65bd1f3

Browse files
committed
fix(mcp): recover rotated credentials on discovery via shared fetchServerTools auth-retry
1 parent abd9848 commit 65bd1f3

2 files changed

Lines changed: 48 additions & 29 deletions

File tree

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ describe('McpService.discoverTools per-server caching', () => {
341341
statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
342342
}),
343343
])
344-
mockListTools.mockRejectedValueOnce(
344+
mockListTools.mockRejectedValue(
345345
new UnauthorizedError(`Rejected Authorization: ${reflectedCredential}`)
346346
)
347347

@@ -491,7 +491,7 @@ describe('McpService.discoverTools per-server caching', () => {
491491
statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
492492
}),
493493
])
494-
mockListTools.mockRejectedValueOnce(
494+
mockListTools.mockRejectedValue(
495495
new UnauthorizedError(`Rejected Authorization: ${reflectedCredential}`)
496496
)
497497

@@ -517,6 +517,19 @@ describe('McpService.discoverTools per-server caching', () => {
517517
expect(mockListTools).not.toHaveBeenCalled()
518518
})
519519

520+
it('recovers a rotated headers-auth credential via a single discovery retry', async () => {
521+
mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
522+
// Stale key 401s once, then the retry re-resolves and succeeds.
523+
mockListTools
524+
.mockRejectedValueOnce(new UnauthorizedError('stale key'))
525+
.mockResolvedValueOnce([tool('a1', 'mcp-a')])
526+
527+
const tools = await mcpService.discoverServerTools(USER_ID, 'mcp-a', WORKSPACE_ID)
528+
529+
expect(tools).toHaveLength(1)
530+
expect(mockListTools).toHaveBeenCalledTimes(2)
531+
})
532+
520533
it('keeps per-server UnauthorizedError soft-pending for OAuth auth', async () => {
521534
mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A', { authType: 'oauth' })])
522535
mockResolveEnvVars.mockRejectedValue(new UnauthorizedError('OAuth token rejected'))

apps/sim/lib/mcp/service.ts

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,36 @@ class McpService {
372372
}
373373
}
374374

375+
/**
376+
* Pooled `tools/list` for one server, with a single retry on a non-OAuth auth
377+
* failure: a rotated header key throws 401, which retires the pooled connection,
378+
* so the retry re-acquires a fresh one that re-resolves the credential. (OAuth
379+
* 401s are left to the caller's oauth-pending handling.) `listTools` is
380+
* idempotent, so the retry is always safe.
381+
*/
382+
private async fetchServerTools(
383+
config: McpServerConfig,
384+
userId: string,
385+
workspaceId: string
386+
): Promise<McpTool[]> {
387+
for (let attempt = 0; ; attempt++) {
388+
try {
389+
return await this.withServerClient(
390+
{
391+
key: this.poolKey(config.id, workspaceId, userId),
392+
serverId: config.id,
393+
allowPool: true,
394+
},
395+
this.buildClient(config, userId, workspaceId),
396+
(client) => client.listTools()
397+
)
398+
} catch (error) {
399+
if (attempt === 0 && isAuthError(error) && config.authType !== 'oauth') continue
400+
throw error
401+
}
402+
}
403+
}
404+
375405
/**
376406
* Run `fn` against a connected client. When `allowPool`, borrow from the warm
377407
* pool (`create` runs only on a miss, so a hit skips env resolution + DNS); a
@@ -676,15 +706,7 @@ class McpService {
676706
}
677707

678708
try {
679-
const tools = await this.withServerClient(
680-
{
681-
key: this.poolKey(config.id, workspaceId, userId),
682-
serverId: config.id,
683-
allowPool: true,
684-
},
685-
this.buildClient(config, userId, workspaceId),
686-
(client) => client.listTools()
687-
)
709+
const tools = await this.fetchServerTools(config, userId, workspaceId)
688710
logger.debug(
689711
`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`
690712
)
@@ -889,15 +911,7 @@ class McpService {
889911
}
890912
authType = config.authType
891913

892-
const tools = await this.withServerClient(
893-
{
894-
key: this.poolKey(serverId, workspaceId, userId),
895-
serverId,
896-
allowPool: true,
897-
},
898-
this.buildClient(config, userId, workspaceId),
899-
(client) => client.listTools()
900-
)
914+
const tools = await this.fetchServerTools(config, userId, workspaceId)
901915
logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
902916
await Promise.allSettled([
903917
this.cacheAdapter
@@ -957,15 +971,7 @@ class McpService {
957971

958972
for (const config of servers) {
959973
try {
960-
const tools = await this.withServerClient(
961-
{
962-
key: this.poolKey(config.id, workspaceId, userId),
963-
serverId: config.id,
964-
allowPool: true,
965-
},
966-
this.buildClient(config, userId, workspaceId),
967-
(client) => client.listTools()
968-
)
974+
const tools = await this.fetchServerTools(config, userId, workspaceId)
969975

970976
summaries.push({
971977
id: config.id,

0 commit comments

Comments
 (0)