Skip to content

Commit 00491d9

Browse files
committed
fix(mcp): clean up uncertain OAuth locks
1 parent 6d4ce34 commit 00491d9

2 files changed

Lines changed: 30 additions & 2 deletions

File tree

apps/sim/lib/mcp/oauth/storage.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,37 @@ describe('withMcpOauthRefreshLock', () => {
164164
expect(fn).not.toHaveBeenCalled()
165165
})
166166

167-
it('falls open when Redis is unavailable on acquire', async () => {
167+
it('cleans up an uncertain owner token before falling open when Redis is unavailable', async () => {
168168
mockAcquireLock.mockRejectedValueOnce(new Error('Redis connection refused'))
169169
const fn = vi.fn(async () => 'uncoordinated')
170170

171171
const result = await withMcpOauthRefreshLock('row-redis-down', fn)
172172

173173
expect(result).toBe('uncoordinated')
174174
expect(fn).toHaveBeenCalledTimes(1)
175-
expect(mockReleaseLock).not.toHaveBeenCalled()
175+
expect(mockReleaseLock).toHaveBeenCalledWith(
176+
'mcp:oauth:refresh:row-redis-down',
177+
expect.any(String)
178+
)
179+
})
180+
181+
it('cleans up an uncertain owner token before propagating cancellation', async () => {
182+
const controller = new AbortController()
183+
mockAcquireLock.mockImplementationOnce(async () => {
184+
controller.abort(new Error('cancelled'))
185+
throw new Error('Redis operation timed out')
186+
})
187+
const fn = vi.fn(async () => 'should-not-run')
188+
189+
await expect(
190+
withMcpOauthRefreshLock('row-aborted-acquire', fn, controller.signal)
191+
).rejects.toThrow('cancelled')
192+
193+
expect(mockReleaseLock).toHaveBeenCalledWith(
194+
'mcp:oauth:refresh:row-aborted-acquire',
195+
expect.any(String)
196+
)
197+
expect(fn).not.toHaveBeenCalled()
176198
})
177199

178200
it('releases the lock even when fn throws', async () => {

apps/sim/lib/mcp/oauth/storage.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,12 @@ async function runWithRedisMutex<T>(
350350
try {
351351
acquired = await acquireLock(lockKey, ownerToken, REFRESH_LOCK_TTL_SEC)
352352
} catch (error) {
353+
await releaseLock(lockKey, ownerToken).catch((releaseError) => {
354+
logger.warn('Refresh lock cleanup after acquire failure failed (will expire via TTL)', {
355+
rowId,
356+
error: toError(releaseError).message,
357+
})
358+
})
353359
signal?.throwIfAborted()
354360
logger.warn('Redis unavailable, running OAuth flow uncoordinated', {
355361
rowId,

0 commit comments

Comments
 (0)