fix(redis): reclaim a lock a timed-out acquire may have taken

`acquireLock` awaited `SET NX` and let a rejection propagate. But a rejected
SET does not mean the server declined it: the client is configured with
`commandTimeout: 5000`, and ioredis gives up locally while the command can
still reach Redis and take the lock. The caller never learns it won, so it
never releases — and since every caller treats a throw as "did not acquire",
nothing else releases it either. Every contender then skips until the TTL
expires.

Staging hit this on the Outlook polling cron: `acquireLock` threw
`Command timed out`, the route returned 500, and the next scheduled poll and
the Lambda retry both got `Polling already in progress - skipped` against a
lock whose holder had never started polling. The 180s TTL cleared it.

On failure, best-effort compare-and-delete through the existing `releaseLock`.
That deletes only while this token still owns the key, so a lock another holder
won in the meantime is untouched, and if Redis is still unreachable the TTL
stays the backstop — the behavior without this cleanup. Control flow is
unchanged for all nine call sites: the original error still propagates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: icecrasher321

apps/sim/lib/core/config/redis.test.ts

@@ -25,6 +25,7 @@ vi.mock('ioredis', () => ({
 }))
 
 import {
+  acquireLock,
   closeRedisConnection,
   extendLock,
   getRedisClient,
@@ -208,6 +209,58 @@ describe('redis config', () => {
     })
   })
 
+  describe('acquireLock', () => {
+    const lockKey = 'outlook-polling-lock'
+    const value = 'req-abc'
+    const ttlSeconds = 180
+
+    it('returns true when SET NX takes the lock', async () => {
+      mockRedisInstance.set.mockResolvedValueOnce('OK')
+
+      expect(await acquireLock(lockKey, value, ttlSeconds)).toBe(true)
+      expect(mockRedisInstance.set).toHaveBeenCalledWith(lockKey, value, 'EX', ttlSeconds, 'NX')
+      expect(mockRedisInstance.eval).not.toHaveBeenCalled()
+    })
+
+    it('returns false without cleanup when the lock is already held', async () => {
+      mockRedisInstance.set.mockResolvedValueOnce(null)
+
+      expect(await acquireLock(lockKey, value, ttlSeconds)).toBe(false)
+      expect(mockRedisInstance.eval).not.toHaveBeenCalled()
+    })
+
+    it('reclaims the lock it may have taken when SET times out', async () => {
+      // ioredis gives up client-side on `commandTimeout` while the command can
+      // still land, so the lock would otherwise be held by a caller that never
+      // learned it won and never releases it.
+      mockRedisInstance.set.mockRejectedValueOnce(new Error('Command timed out'))
+      mockRedisInstance.eval.mockResolvedValueOnce(1)
+
+      await expect(acquireLock(lockKey, value, ttlSeconds)).rejects.toThrow('Command timed out')
+      expect(mockRedisInstance.eval).toHaveBeenCalledWith(
+        expect.stringContaining('del'),
+        1,
+        lockKey,
+        value
+      )
+    })
+
+    it('surfaces the original failure when the cleanup also fails', async () => {
+      mockRedisInstance.set.mockRejectedValueOnce(new Error('Command timed out'))
+      mockRedisInstance.eval.mockRejectedValueOnce(new Error('Connection is closed'))
+
+      // The TTL stays the backstop; the caller must still see why acquiring failed.
+      await expect(acquireLock(lockKey, value, ttlSeconds)).rejects.toThrow('Command timed out')
+    })
+
+    it('returns true as a no-op when the cache capability selects the database', async () => {
+      mockEnv.REDIS_URL = undefined
+
+      expect(await acquireLock(lockKey, value, ttlSeconds)).toBe(true)
+      expect(mockRedisInstance.set).not.toHaveBeenCalled()
+    })
+  })
+
   describe('capability validation', () => {
     it('rejects a non-Redis URL before constructing a client', () => {
       mockEnv.REDIS_URL = 'https://cache.example.com'

apps/sim/lib/core/config/redis.ts

@@ -245,8 +245,25 @@ export async function acquireLock(
     return true // No-op when Redis unavailable; idempotency layer handles duplicates
   }
 
-  const result = await redis.set(lockKey, value, 'EX', expirySeconds, 'NX')
-  return result === 'OK'
+  try {
+    const result = await redis.set(lockKey, value, 'EX', expirySeconds, 'NX')
+    return result === 'OK'
+  } catch (error) {
+    /*
+     * A rejected SET does not mean the server declined it. `commandTimeout`
+     * gives up client-side while the command may still reach Redis and take the
+     * lock, leaving it held by a caller that never learned it won and so never
+     * releases it — every contender then skips until the TTL expires.
+     *
+     * Reclaiming it is safe because this is the same compare-and-delete
+     * `releaseLock` uses on the success path: it deletes only while `value`
+     * still owns the key, so a lock a different holder won in the meantime is
+     * left alone. Best effort — if Redis is still unreachable the TTL remains
+     * the backstop, which is exactly the behavior without this cleanup.
+     */
+    await releaseLock(lockKey, value).catch(() => {})
+    throw error
+  }
 }
 
 /**