fix(observability): backfill the shared Redis state in place, preserving identity

The previous commit normalized the global by building a replacement object and
assigning it. That fixed the undefined-field problem and introduced a worse one:
sharing a single object across module evaluations is the entire reason this
state lives on globalThis, and replacing it leaves an earlier module instance
holding the previous object. Two instances that disagree about `client` each
open their own connection and start their own health check — a real change to
connection lifecycle in a change that is supposed to be pure instrumentation.

Backfills the existing object in place instead, so every holder keeps pointing
at the same state while an older shape still gets its missing fields. The test
now asserts object identity as well as the filled values, and fails if the
function returns a copy.

Author: waleedlatif1

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

@@ -27,10 +27,10 @@ vi.mock('ioredis', () => ({
 import {
   acquireLock,
   closeRedisConnection,
-  createRedisState,
   describeRedisConnection,
   extendLock,
   getRedisClient,
+  normalizeRedisState,
   onRedisReconnect,
   resetForTesting,
 } from '@/lib/core/config/redis'
@@ -252,7 +252,11 @@ describe('redis config', () => {
         reconnectListeners: [],
       }
 
-      const normalized = createRedisState(stale)
+      const normalized = normalizeRedisState(stale)
+
+      // Identity must survive: every module instance has to keep pointing at the
+      // same object, or two of them each open their own client.
+      expect(normalized).toBe(stale)
 
       // Undefined slips past the `=== null` guards and yields NaN downstream.
       expect(normalized.clientCreatedAt).toBeNull()
@@ -270,14 +274,14 @@ describe('redis config', () => {
     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)
+        expect(normalizeRedisState({ 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 })
+      const normalized = normalizeRedisState({ connects: 4, reconnects: 2, errors: 7 })
 
       expect(normalized).toMatchObject({ connects: 4, reconnects: 2, errors: 7 })
     })

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

@@ -72,37 +72,40 @@ interface RedisState {
 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.
+Backfilled in place, because the global outlives a module evaluation in two
+ * directions at once.
  *
- * Exported so the normalization itself is testable: a module-evaluation-time
- * side effect is not reachable from a test that has already imported it.
+ * 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 purpose is being
+ * trustworthy. So the shape has to be upgraded rather than trusted.
+ *
+ * It must be upgraded *without replacing the object*. Sharing one object across
+ * evaluations is the entire reason this lives on `globalThis`: assigning a fresh
+ * one leaves an earlier module instance holding the previous object, and two
+ * instances that disagree about `client` each open their own connection and
+ * start their own health check. Mutating in place keeps every holder pointing at
+ * the same state while still filling in what an older shape lacks.
  */
-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,
-  }
+export function normalizeRedisState(existing: Partial<RedisState>): RedisState {
+  existing.client ??= null
+  existing.pingFailures ??= 0
+  existing.pingInterval ??= null
+  existing.pingInFlight ??= false
+  existing.reconnectListeners ??= []
+  existing.clientCreatedAt ??= null
+  existing.lastReadyAt ??= null
+  existing.lastPingOkAt ??= null
+  existing.connects ??= 0
+  existing.reconnects ??= 0
+  existing.errors ??= 0
+  existing.lastErrorMessage ??= null
+  return existing as RedisState
 }
 
-const state = createRedisState(g._redisState)
-g._redisState = state
+g._redisState ??= {}
+const state = normalizeRedisState(g._redisState)
 
 /**
  * A command that never gets a reply fails identically whichever of three states