Skip to content

Commit 7909bb0

Browse files
committed
Fix MCP retry publication ordering (#5754)
Reacquire cache mutation ownership for each discovery retry and cover cache invalidation during retry backoff. Note: the full app suite has pre-existing failures in unrelated workflow, webhook, chat, and UI tests; the focused MCP suite passes 38/38.
1 parent 2d8118b commit 7909bb0

2 files changed

Lines changed: 53 additions & 2 deletions

File tree

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,58 @@ describe('McpService.discoverTools per-server caching', () => {
10041004
expect(mockListTools).toHaveBeenCalledTimes(2)
10051005
})
10061006

1007+
it('reacquires mutation ownership after cache invalidation during retry backoff', async () => {
1008+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] })
1009+
try {
1010+
const serverKey = `workspace:${WORKSPACE_ID}:server:mcp-a`
1011+
mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
1012+
1013+
let rejectFirstAttempt: ((error: Error) => void) | undefined
1014+
let markFirstAttemptStarted: (() => void) | undefined
1015+
const firstAttemptStarted = new Promise<void>((resolve) => {
1016+
markFirstAttemptStarted = resolve
1017+
})
1018+
mockListTools
1019+
.mockImplementationOnce(
1020+
() =>
1021+
new Promise((_resolve, reject) => {
1022+
rejectFirstAttempt = reject
1023+
markFirstAttemptStarted?.()
1024+
})
1025+
)
1026+
.mockResolvedValueOnce([tool('retry-winner', 'mcp-a')])
1027+
1028+
const discovery = mcpService.discoverServerToolsWithMetadata(
1029+
USER_ID,
1030+
'mcp-a',
1031+
WORKSPACE_ID,
1032+
true
1033+
)
1034+
await firstAttemptStarted
1035+
1036+
rejectFirstAttempt?.(new Error('Request timed out'))
1037+
await vi.advanceTimersByTimeAsync(0)
1038+
expect(mockListTools).toHaveBeenCalledTimes(1)
1039+
1040+
await mcpService.clearCache(WORKSPACE_ID)
1041+
expect(mockCacheAdapter.beginMutation).toHaveBeenCalledTimes(2)
1042+
1043+
await vi.runAllTimersAsync()
1044+
await expect(discovery).resolves.toEqual({
1045+
tools: [tool('retry-winner', 'mcp-a')],
1046+
state: 'published',
1047+
})
1048+
1049+
expect(mockCacheAdapter.beginMutation).toHaveBeenCalledTimes(3)
1050+
expect(cacheStore.get(serverKey)?.tools).toEqual([tool('retry-winner', 'mcp-a')])
1051+
expect(mockUpdateSet).toHaveBeenCalledWith(
1052+
expect.objectContaining({ connectionStatus: 'connected', toolCount: 1 })
1053+
)
1054+
} finally {
1055+
vi.useRealTimers()
1056+
}
1057+
})
1058+
10071059
it('persists and negative-caches per-server UnauthorizedError for headers auth', async () => {
10081060
const reflectedCredential = 'Bearer static-secret-for-server-discovery'
10091061
mockGetWorkspaceServersRows.mockResolvedValue([

apps/sim/lib/mcp/service.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,9 +1069,8 @@ class McpService {
10691069
}
10701070
}
10711071

1072-
const mutation = await this.beginServerCacheMutation(workspaceId, serverId)
1073-
10741072
for (let attempt = 0; attempt < maxRetries; attempt++) {
1073+
const mutation = await this.beginServerCacheMutation(workspaceId, serverId)
10751074
let config: McpServerConfig | null = null
10761075
try {
10771076
logger.info(

0 commit comments

Comments
 (0)