fix(execution): reclaim a lease Redis registers after the local deadline

Addresses review findings on the fallback path.

- Always release the lease. The deadline abandons the local wait but cannot
  cancel the script, so a late completion still registers the lease id; leaving
  it unreleased kept it counted against the owner for the whole TTL and denied
  later executions that did have capacity. The id is unique per execution, so
  removing one that was never registered is a no-op.
- Treat a non-positive configured deadline as unconfigured. A timer of zero or
  less fires immediately, which would leave every acquisition undetermined and
  silently drop cross-replica enforcement.
- Cover both with tests: a lease that completes after the deadline is still
  released, and a non-positive deadline still lets a real answer land.

Author: waleedlatif1

apps/sim/lib/execution/isolated-vm.test.ts

@@ -8,6 +8,7 @@ import {
   loggerMock,
   redisConfigMockFns,
 } from '@sim/testing'
+import { sleep } from '@sim/utils/helpers'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 type MockProc = EventEmitter & {
@@ -580,6 +581,76 @@ describe('isolated-vm scheduler', () => {
     expect(result.result).toBe('ok')
   })
 
+  it('releases a lease that Redis registers after the local deadline', async () => {
+    const scripts: string[] = []
+    let completeAcquire!: (value: number) => void
+    const lateAcquire = new Promise<number>((resolve) => {
+      completeAcquire = resolve
+    })
+    const { executeInIsolatedVM } = await loadExecutionModule({
+      envOverrides: {
+        REDIS_URL: 'redis://localhost:6379',
+        IVM_LEASE_REDIS_DEADLINE_MS: '5',
+      },
+      spawns: [() => createReadyProc('ok')],
+      redisEvalImpl: (...args: unknown[]) => {
+        const script = String(args[0] ?? '')
+        scripts.push(script)
+        // Settles only once the test says so, standing in for a script the
+        // deadline abandoned locally but that Redis still runs to completion.
+        if (script.includes('ZREMRANGEBYSCORE')) return lateAcquire
+        return 1
+      },
+    })
+
+    const result = await executeInIsolatedVM({
+      code: 'return "ok"',
+      params: {},
+      envVars: {},
+      contextVariables: {},
+      timeoutMs: 100,
+      requestId: 'req-11',
+      ownerKey: 'user:redis-late',
+    })
+    completeAcquire(1)
+
+    expect(result.error).toBeUndefined()
+    expect(scripts.some((script) => script.includes("'ZREM'"))).toBe(true)
+  })
+
+  it('ignores a non-positive configured deadline instead of abandoning every lease', async () => {
+    const { executeInIsolatedVM } = await loadExecutionModule({
+      envOverrides: {
+        IVM_DISTRIBUTED_MAX_INFLIGHT_PER_OWNER: '1',
+        IVM_LEASE_REDIS_DEADLINE_MS: '-1',
+        REDIS_URL: 'redis://localhost:6379',
+      },
+      spawns: [() => createReadyProc('ok')],
+      redisEvalImpl: async (...args: unknown[]) => {
+        const script = String(args[0] ?? '')
+        if (script.includes('ZREMRANGEBYSCORE')) {
+          // Arrives after a non-positive timer would already have fired, so the
+          // answer only lands in time when the default deadline is restored.
+          await sleep(25)
+          return 0
+        }
+        return 1
+      },
+    })
+
+    const result = await executeInIsolatedVM({
+      code: 'return "ok"',
+      params: {},
+      envVars: {},
+      contextVariables: {},
+      timeoutMs: 100,
+      requestId: 'req-12',
+      ownerKey: 'user:negative-deadline',
+    })
+
+    expect(result.error?.message).toContain('Too many concurrent')
+  })
+
   it('still rejects when Redis answers that the owner is over its lease limit', async () => {
     const { executeInIsolatedVM } = await loadExecutionModule({
       envOverrides: {

apps/sim/lib/execution/isolated-vm.ts

@@ -148,8 +148,14 @@ const DISTRIBUTED_KEY_PREFIX = 'ivm:fair:v1:owner'
  * actually measure is event-loop scheduling, not Redis. A value near normal loop
  * latency therefore reports a healthy Redis as unreachable whenever a garbage
  * collection pause lands on the call. Keep it well clear of that floor.
+ *
+ * A non-positive configured value is treated as unconfigured rather than
+ * honored: a timer of zero or less fires immediately, which would leave every
+ * acquisition undetermined and silently drop cross-replica enforcement.
  */
-const LEASE_REDIS_DEADLINE_MS = Number.parseInt(env.IVM_LEASE_REDIS_DEADLINE_MS) || 1000
+const CONFIGURED_LEASE_REDIS_DEADLINE_MS = Number.parseInt(env.IVM_LEASE_REDIS_DEADLINE_MS)
+const LEASE_REDIS_DEADLINE_MS =
+  CONFIGURED_LEASE_REDIS_DEADLINE_MS > 0 ? CONFIGURED_LEASE_REDIS_DEADLINE_MS : 1000
 const QUEUE_RETRY_DELAY_MS = 1000
 const DISTRIBUTED_LEASE_GRACE_MS = 30000
 
@@ -1439,12 +1445,17 @@ export async function executeInIsolatedVM(
   // the per-owner active/queued limits above still bound this work.
 
   let settled = false
-  const holdsDistributedLease = leaseAcquireResult === 'acquired'
+  /**
+   * Released even when the acquisition was undetermined. The deadline abandons
+   * the local wait but cannot cancel the script, so a late completion still
+   * registers this lease id — and unreleased it would count against the owner
+   * for the whole TTL, denying later executions that do have capacity. The
+   * lease id is unique to this execution, so removing one that was never
+   * registered is a no-op.
+   */
   const releaseLease = () => {
     if (settled) return
     settled = true
-    // Nothing was registered when the lease was undetermined; skip the round trip.
-    if (!holdsDistributedLease) return
     releaseDistributedLease(ownerKey, distributedLeaseId).catch((error) => {
       logger.error('Failed to release distributed lease', { ownerKey, error })
     })