fix(observability): scope connection ages to the live client and detect IPv6

Two corrections to the diagnostic snapshot, both cases where it would have
answered wrongly.

Ages now report null once the cached client is gone. Discarding a client — a
forced reconnect, or an explicit close — left its timestamps behind until the
next getRedisClient() rebuilt them, so the snapshot dated a connection that no
longer existed while reporting no-client beside it. The lifecycle counters stay
cumulative for the process, which is intended; only the ages are per-client.

Host classification now uses node:net isIP rather than an IPv4 regex, so an
IPv6 literal reads as an IP instead of a DNS name. Getting that backwards would
send someone chasing DNS resolution on an endpoint that performs none, which is
the exact misdirection the field exists to prevent. sniOverride still mirrors
resolveRedisTlsOptions and stays IPv4-only, so an IPv6 host over TLS reports
hostKind ip with sniOverride false — the useful reading, since that pairing is
a connection whose certificate cannot verify.

Author: waleedlatif1

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

@@ -225,6 +225,35 @@ describe('redis config', () => {
       expect(JSON.stringify(d)).not.toContain('10.0.0.5')
     })
 
+    it('does not date a connection that has been discarded', async () => {
+      mockRedisInstance.status = 'ready'
+      getRedisClient()
+      expect(describeRedisConnection().clientAgeMs).not.toBeNull()
+
+      // Two consecutive PING failures drop the cached client.
+      mockRedisInstance.ping.mockRejectedValue(new Error('ETIMEDOUT'))
+      await vi.advanceTimersByTimeAsync(15_000)
+      await vi.advanceTimersByTimeAsync(15_000)
+
+      const d = describeRedisConnection()
+      expect(d.status).toBe('no-client')
+      expect(d.clientAgeMs).toBeNull()
+      expect(d.readyAgeMs).toBeNull()
+      expect(d.msSinceLastPingOk).toBeNull()
+      // Lifecycle counters stay cumulative for the process.
+      expect(d.reconnects).toBeGreaterThanOrEqual(0)
+    })
+
+    it('classifies an IPv6 literal as an IP, not a DNS name', () => {
+      mockEnv.REDIS_URL = 'rediss://[2600:1f18::1]:6379'
+
+      const d = describeRedisConnection()
+
+      expect(d.hostKind).toBe('ip')
+      // Mirrors resolveRedisTlsOptions, which applies the override for IPv4 only.
+      expect(d.sniOverride).toBe(false)
+    })
+
     it('reports a DNS host so resolution latency can be ruled in or out', () => {
       mockEnv.REDIS_URL = 'rediss://primary.example.cache.amazonaws.com:6379'
 

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

@@ -1,3 +1,4 @@
+import { isIP } from 'node:net'
 import { createLogger } from '@sim/logger'
 import { toError } from '@sim/utils/errors'
 import { randomFloat } from '@sim/utils/random'
@@ -133,9 +134,20 @@ function describeRedisUrl(
   if (!url) return { hostKind: 'unknown', tls: false, sniOverride: false }
   try {
     const parsed = new URL(url)
-    const isIp = /^\d{1,3}(\.\d{1,3}){3}$/.test(parsed.hostname)
+    // WHATWG keeps IPv6 literals bracketed in `hostname`; `isIP` wants them bare.
+    const host = parsed.hostname.replace(/^\[|\]$/g, '')
     const tls = parsed.protocol === 'rediss:'
-    return { hostKind: isIp ? 'ip' : 'dns', tls, sniOverride: tls && isIp }
+    /**
+     * `sniOverride` deliberately mirrors `resolveRedisTlsOptions`, which tests
+     * for IPv4 only. So an IPv6 literal over TLS reports `hostKind: 'ip'` with
+     * `sniOverride: false` — not a contradiction but the useful reading, since
+     * that combination is a connection whose certificate cannot verify.
+     */
+    return {
+      hostKind: isIP(host) === 0 ? 'dns' : 'ip',
+      tls,
+      sniOverride: tls && isIP(host) === 4,
+    }
   } catch {
     return { hostKind: 'unknown', tls: false, sniOverride: false }
   }
@@ -160,11 +172,20 @@ export function describeRedisConnection(): RedisConnectionDiagnostics {
   // double-cast-allowed: ioredis omits offlineQueue from its public type, and its depth is what separates a command waiting on connection setup from one written to a live socket
   const queued = (client as unknown as OfflineQueueView | null)?.offlineQueue?.length
 
+  /**
+   * Ages describe the client that is currently held. A discarded client leaves
+   * its timestamps behind until the next `getRedisClient()` rebuilds them, and
+   * reporting those against `no-client` would date a connection that no longer
+   * exists — precisely the wrong answer for the investigation this exists to
+   * support. The counters below are deliberately cumulative for the process.
+   */
+  const ageOf = (at: number | null) => (client === null || at === null ? null : now - at)
+
   return {
     status: client?.status ?? 'no-client',
-    clientAgeMs: state.clientCreatedAt === null ? null : now - state.clientCreatedAt,
-    readyAgeMs: state.lastReadyAt === null ? null : now - state.lastReadyAt,
-    msSinceLastPingOk: state.lastPingOkAt === null ? null : now - state.lastPingOkAt,
+    clientAgeMs: ageOf(state.clientCreatedAt),
+    readyAgeMs: ageOf(state.lastReadyAt),
+    msSinceLastPingOk: ageOf(state.lastPingOkAt),
     queuedCommands: typeof queued === 'number' ? queued : null,
     connects: state.connects,
     reconnects: state.reconnects,