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

[icecrasher321]

The bug

acquireLock awaited SET NX and let a rejection propagate:

const result = await redis.set(lockKey, value, 'EX', expirySeconds, 'NX')
return result === 'OK'

A rejected SET does not mean the server declined it. The client is configured with commandTimeout: 5000 (redis.ts:164), and ioredis rejects locally when that elapses (ioredis/built/Command.js:195, message Command timed out) while the command can still reach Redis and take the lock.

When that happens the lock is held by a caller that never learned it won, so it never releases. Every caller treats a throw as "did not acquire", so nothing else releases it either. Every contender skips until the TTL expires.

How staging hit it

The sim-staging-us-east-1-cron-lambda-errors alarm at 21:06 UTC:

• acquireLock threw Command timed out in app/api/webhooks/poll/[provider]/route.ts:43
• The outer catch returned 500 outlook polling failed: Command timed out after ~10.8s
• The next scheduled poll and the Lambda retry both got 202 Polling already in progress - skipped, against a lock whose holder had never started polling
• The 180s TTL cleared it

Worth being precise about one thing, because the incident's own remediation note pointed at it: the route's finally/releaseLock is not missing. It lives inside the runDetached task, which never started, because the throw happened at lock acquisition. The poll never ran, so there was nothing to terminate — the lock simply had no owner that could release it.

The fix, and why it is opt-in

On failure, best-effort compare-and-delete through the existing releaseLock — but only when the caller opts in, because reclaiming unconditionally is unsafe for two caller classes that exist today. Both were caught in review; the first revision of this PR had it on for everyone and was wrong.

Callers that fall open. withLeaderLock (leader-lock.ts:36-43) and the MCP OAuth refresh mutex (mcp/oauth/storage.ts:329-336) catch the throw and run their work uncoordinated. If the SET landed, the lock they hold is what keeps everyone else out while they run. Freeing it under them admits a second concurrent runner — for OAuth refresh, two rotations of the same refresh token and an invalid_grant.

Callers whose lock value is not unique. The copilot chat lock keys on streamId, which is the client-supplied userMessageId (copilot/chat/post.ts:266 declares it z.string().max(128).optional(); post.ts:592 passes it as streamId). Two sends can carry the same value, so a compare-and-delete from a contender that timed out can match — and delete — the lock the active stream holds.

So the option carries its preconditions in its own TSDoc: the value must be unique to the holder, and a throw must mean the caller does no work. Default behavior is byte-for-byte what it was before this PR.

Opted in

The four cron/poll callers that satisfy both preconditions — each mints its value with generateShortId() and returns 5xx rather than proceeding when acquisition throws:

Caller	Value	On throw
app/api/webhooks/poll/[provider]	generateShortId()	500
app/api/resume/poll	generateShortId()	5xx via route handler
app/api/workspace-events/poll	generateShortId()	500
app/api/cron/renew-subscriptions	generateShortId()	5xx via route handler

For those, every branch is better than today or identical to it:

Case	Before	After
SET landed, client timed out	held for full TTL	reclaimed immediately
SET never landed, another holder won	untouched	untouched (value mismatch, no-op)
SET never landed, nobody holds it	no-op	no-op
Redis still unreachable	TTL backstop	TTL backstop (cleanup fails, swallowed)

Not opted in, and unchanged: lib/concurrency/leader-lock, lib/mcp/oauth/storage, lib/copilot/request/session/abort, lib/table/cascade-lock, app/api/tools/file/manage.

Control flow is unchanged

The error still propagates in every case, so callers see exactly what they saw before — the only difference is a best-effort side effect for the four that opt in. Deliberately not converted to return false: that would make the poll route answer Polling already in progress - skipped when polling was not in progress, hiding a real Redis outage from the alarm that correctly fired here.

It also makes the existing recovery work as designed — the Lambda retry that got already in progress will now find the lock free and actually poll.

Tests

Six cases in redis.test.ts: SET NX taking the lock (no cleanup issued), contention returning false (no cleanup issued), a timed-out SET issuing the compare-and-delete and rethrowing when reclaim is on, a timed-out SET issuing no cleanup by default so a fall-open caller keeps holding its lock, a failed cleanup still surfacing the original error, and the no-Redis no-op path.

The webhook polling route test now asserts it passes { reclaimOnFailure: true }, pinning the incident's actual fix at the call site rather than only in the primitive.

I verified the reclaim test fails against the original implementation (1 failed / 20 passed) and passes with the fix, so it isn't a test that would pass either way.

Verification

• bun run test across redis.test.ts and all affected callers: 64 passed
• bun run type-check (apps/sim): clean
• biome check: clean

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 19, 2026 10:25pm

[cursor]

PR Summary

Medium Risk
Changes distributed locking behavior for production cron/poll paths; reclaim is scoped and compare-and-delete safe, but incorrect opt-in could admit concurrent runners on other lock users.

Overview
Adds an opt-in reclaimOnFailure path on acquireLock: when Redis SET NX throws (e.g. ioredis commandTimeout), the client can still have taken the lock, so the holder never runs releaseLock and cron polls skip until TTL.

On failure with reclaimOnFailure: true, acquireLock best-effort calls releaseLock (compare-and-delete) then rethrows—same error surface as before, not a silent false.

Four cron/poll routes now pass { reclaimOnFailure: true } (webhook provider poll, resume poll, workspace-events poll, Teams subscription renewal)—each uses a unique generateShortId() value and does not proceed on acquire errors. Other acquireLock callers stay default (no reclaim) for fall-open or shared lock-value cases.

redis.test.ts adds acquireLock coverage plus route tests assert the new option at call sites.

^{Reviewed by Cursor Bugbot for commit 121cdd4. Configure here.}

[greptile-apps]

Greptile Summary

The PR makes lock reclamation opt-in when a Redis SET NX rejects, allowing eligible polling and renewal routes to clean up locks they may have acquired before a client-side timeout.

• Adds AcquireLockOptions.reclaimOnFailure with documented ownership and fail-closed preconditions.
• Enables reclamation for four routes that use per-attempt lock values and abort work when acquisition throws.
• Adds focused primitive and route tests covering successful acquisition, contention, timeout cleanup, default behavior, and cleanup failure.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
apps/sim/lib/core/config/redis.ts	Adds opt-in compare-and-delete cleanup after rejected lock acquisition while preserving the original error and safe default behavior.
apps/sim/lib/core/config/redis.test.ts	Covers acquisition success, contention, opted-in reclamation, default non-reclamation, cleanup failure, and the no-Redis path.
apps/sim/app/api/webhooks/poll/[provider]/route.ts	Enables reclamation with a uniquely generated request identifier and does not start polling after acquisition throws.
apps/sim/app/api/resume/poll/route.ts	Enables reclamation using a per-request identifier while retaining fail-closed acquisition handling.
apps/sim/app/api/workspace-events/poll/route.ts	Enables reclamation for the polling lock with a unique request-scoped ownership value.
apps/sim/app/api/cron/renew-subscriptions/route.ts	Enables reclamation for subscription renewal using a freshly generated lock value.

_{Reviews (2): Last reviewed commit: "fix(redis): make the timed-out-acquire r..." | Re-trigger Greptile}

[icecrasher321]

@greptile

[icecrasher321]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 121cdd4. Configure here.}