fix(observability): normalize the shared Redis state so stale globals cannot emit NaN

The state object lives on globalThis and outlives a module evaluation, but was
only ever initialized when absent. A global created before these fields existed
kept them undefined, and undefined slips past every `=== null` guard: ages
became `now - undefined` and counters became `undefined++`, both NaN, both
serialized as null — in the one payload whose entire purpose is being
trustworthy. Reachable wherever a process re-evaluates the module against an
existing global, dev hot-reload being the obvious case.

Normalizes from whatever is present instead of trusting the shape, carrying
pingInterval across so the health check is never started twice. The
normalization is a pure exported function because a module-evaluation side
effect is not reachable from a test that has already imported it — the first
attempt at covering this asserted against the module's own already-normalized
state and passed with the fix reverted.

Also adds guards for two properties the diagnostics depend on but nothing
asserted: that describeRedisConnection never throws, since it runs inside catch
blocks where a throw would replace the real failure, and that the wrapper
rethrows the original error object rather than substituting its own. Each was
verified to fail when the behavior is removed.

Author: waleedlatif1

apps/sim/lib/billing/calculations/usage-reservation.test.ts

@@ -339,6 +339,13 @@ describe('usage-reservation', () => {
   })
 
   describe('refreshExecutionSlotExpiry', () => {
+    it('rethrows the original error object rather than the diagnostic wrapper', async () => {
+      const original = Object.assign(new Error('Command timed out'), { code: 'ETIMEDOUT' })
+      getMock.mockRejectedValueOnce(original)
+
+      await expect(refreshExecutionSlotExpiry('exec-1', Date.now() + 60_000)).rejects.toBe(original)
+    })
+
     it('refreshes only the locally owned slot and matching pointer', async () => {
       evalMock.mockResolvedValueOnce(1).mockResolvedValueOnce(1)
       await reserveExecutionSlot(memberParams)

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

@@ -27,6 +27,7 @@ vi.mock('ioredis', () => ({
 import {
   acquireLock,
   closeRedisConnection,
+  createRedisState,
   describeRedisConnection,
   extendLock,
   getRedisClient,
@@ -225,6 +226,62 @@ describe('redis config', () => {
       expect(JSON.stringify(d)).not.toContain('10.0.0.5')
     })
 
+    it('never throws, so it cannot mask the error it is describing', () => {
+      // Called from catch blocks: a throw here would replace the real failure.
+      mockEnv.REDIS_URL = undefined
+      expect(() => describeRedisConnection()).not.toThrow()
+
+      mockEnv.REDIS_URL = 'not a url'
+      expect(() => describeRedisConnection()).not.toThrow()
+      expect(describeRedisConnection().hostKind).toBe('unknown')
+
+      // rediss:// to a bare IP with no REDIS_TLS_SERVERNAME makes the real
+      // getConfiguredRedisUrl path throw; the snapshot must still come back.
+      mockEnv.REDIS_URL = 'rediss://10.0.0.5:6379'
+      mockEnv.REDIS_TLS_SERVERNAME = undefined
+      expect(() => describeRedisConnection()).not.toThrow()
+    })
+
+    it('upgrades a state object that predates these fields instead of trusting its shape', () => {
+      // Exactly the shape the global held before this change.
+      const stale = {
+        client: null,
+        pingFailures: 0,
+        pingInterval: null,
+        pingInFlight: false,
+        reconnectListeners: [],
+      }
+
+      const normalized = createRedisState(stale)
+
+      // Undefined slips past the `=== null` guards and yields NaN downstream.
+      expect(normalized.clientCreatedAt).toBeNull()
+      expect(normalized.lastReadyAt).toBeNull()
+      expect(normalized.lastPingOkAt).toBeNull()
+      expect(normalized.connects).toBe(0)
+      expect(normalized.reconnects).toBe(0)
+      expect(normalized.errors).toBe(0)
+      expect(normalized.lastErrorMessage).toBeNull()
+      for (const [key, value] of Object.entries(normalized)) {
+        expect(value, `${key} is undefined`).not.toBeUndefined()
+      }
+    })
+
+    it('carries a running health check across normalization so it is not started twice', () => {
+      const interval = setInterval(() => {}, 1_000)
+      try {
+        expect(createRedisState({ pingInterval: interval }).pingInterval).toBe(interval)
+      } finally {
+        clearInterval(interval)
+      }
+    })
+
+    it('preserves accumulated counters when the shape is already current', () => {
+      const normalized = createRedisState({ connects: 4, reconnects: 2, errors: 7 })
+
+      expect(normalized).toMatchObject({ connects: 4, reconnects: 2, errors: 7 })
+    })
+
     it('does not date a connection that has been discarded', async () => {
       mockRedisInstance.status = 'ready'
       getRedisClient()

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

@@ -69,24 +69,40 @@ interface RedisState {
   lastErrorMessage: string | null
 }
 
-const g = globalThis as typeof globalThis & { _redisState?: RedisState }
-if (!g._redisState) {
-  g._redisState = {
-    client: null,
-    pingFailures: 0,
-    pingInterval: null,
-    pingInFlight: false,
-    reconnectListeners: [],
-    clientCreatedAt: null,
-    lastReadyAt: null,
-    lastPingOkAt: null,
-    connects: 0,
-    reconnects: 0,
-    errors: 0,
-    lastErrorMessage: null,
+const g = globalThis as typeof globalThis & { _redisState?: Partial<RedisState> }
+
+/**
+ * Normalized rather than initialized-once, because the global outlives a module
+ * evaluation. A state object created before any of these fields existed keeps
+ * them `undefined`, which slips past the `=== null` guards below and turns every
+ * age into `now - undefined` and every counter into `undefined++` — NaN in both
+ * cases, serialized as `null`, in the one payload whose entire purpose is being
+ * trustworthy. Rebuilding from whatever is there upgrades an existing global
+ * instead of trusting its shape, and carries `pingInterval` across so the health
+ * check is never started twice.
+ *
+ * Exported so the normalization itself is testable: a module-evaluation-time
+ * side effect is not reachable from a test that has already imported it.
+ */
+export function createRedisState(existing: Partial<RedisState> = {}): RedisState {
+  return {
+    client: existing.client ?? null,
+    pingFailures: existing.pingFailures ?? 0,
+    pingInterval: existing.pingInterval ?? null,
+    pingInFlight: existing.pingInFlight ?? false,
+    reconnectListeners: existing.reconnectListeners ?? [],
+    clientCreatedAt: existing.clientCreatedAt ?? null,
+    lastReadyAt: existing.lastReadyAt ?? null,
+    lastPingOkAt: existing.lastPingOkAt ?? null,
+    connects: existing.connects ?? 0,
+    reconnects: existing.reconnects ?? 0,
+    errors: existing.errors ?? 0,
+    lastErrorMessage: existing.lastErrorMessage ?? null,
   }
 }
-const state = g._redisState
+
+const state = createRedisState(g._redisState)
+g._redisState = state
 
 /**
  * A command that never gets a reply fails identically whichever of three states