mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-22 05:19:54 +08:00
Vikhyath MondretiandClaude Opus 5
efea9de326
* 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> * fix(redis): make the timed-out-acquire reclaim opt-in per caller Review caught that reclaiming unconditionally is unsafe for two caller classes, both of which exist today: Callers that fall open. `withLeaderLock` and the MCP OAuth refresh mutex catch a throw from `acquireLock` and run their work uncoordinated. If the SET landed, today the lock they hold keeps everyone else out while they run. Freeing it under them admits a second concurrent runner — for OAuth refresh that means two rotations of the same 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`. 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 is holding. Reclaiming is therefore opt-in, and the option documents both preconditions it needs: a value unique to the holder, and a caller that does no work when acquisition throws. Default behavior is byte-for-byte what it was before. Opted in are the four cron/poll callers that satisfy both — webhook polling, resume polling, workspace-events polling, and Teams subscription renewal. Each mints its value with `generateShortId()` and returns 5xx rather than proceeding when acquisition throws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>