src/sdk/bots-brain/UnifiedStorage.ts (1)

181-191: Consolidate Redis "error" handling to avoid duplicate listeners and noisy logs

You attach two 'error' listeners (constructor and initializeConnection). They will both fire, duplicating logs and work. Suggest: keep a single structured handler; also add 'end'/'reconnecting' to flip connected=false proactively.

Apply this diff to consolidate and harden:

@@
-    // Add error event handler to prevent crashes (Railway optimization)
-    this.client.on('error', (error: Error) => {
-      this.connected = false;
-      LogEngine.error('Redis L2 cache connection error (non-fatal):', {
-        error: error.message,
-        code: (error as NodeJS.ErrnoException).code,
-        errorType: 'L2_cache_error',
-      });
-      // Don't throw - graceful degradation to L1 + L3 only
-    });
+    // Single non-fatal error handler (+ connection lifecycle)
+    this.client.on('error', (error: Error) => {
+      this.connected = false;
+      LogEngine.error('Redis L2 cache connection error (non-fatal):', {
+        error: error.message,
+        code: (error as NodeJS.ErrnoException).code,
+        errorType: 'L2_cache_error',
+      });
+    });
+    this.client.on('end', () => {
+      this.connected = false;
+      LogEngine.warn('Redis L2 cache connection closed');
+    });
+    this.client.on('reconnecting', () => {
+      this.connected = false;
+      LogEngine.warn('Redis L2 cache reconnecting...');
+    });
@@
-      this.client.on('error', (error: Error) => {
-        this.connected = false;
-        LogEngine.error('Redis L2 cache error:', error);
-      });
+      // 'error' handled in constructor; avoid duplicate listeners

Also applies to: 202-205

docker-compose.yml (1)

141-141: Second Redis 8 instance: mirror the validation

Same compatibility check for webhook Redis; consider aligning persistence and healthcheck intervals if workload differs.

src/sdk/bots-brain/BotsStore.ts (4)

865-879: Timeout via Promise.race doesn’t cancel the underlying work

The 2‑minute guard is good, but Promise.race won’t abort performSchemaCheck; it will keep running in the background. Consider a simple reentrancy guard to prevent overlapping runs, and/or use server‑side timeouts (lock_timeout, idle_in_transaction_session_timeout) for defensive limits.

Example minimal guard:

@@
-  private async ensureSchema(): Promise<void> {
+  private schemaInitInProgress = false;
+  private async ensureSchema(): Promise<void> {
+    if (this.schemaInitInProgress) {
+      LogEngine.warn('Schema operation already in progress; skipping reentry');
+      return;
+    }
+    this.schemaInitInProgress = true;
     try {
@@
-    } 
+    } 
     catch (error) {
@@
-    }
+    } finally {
+      this.schemaInitInProgress = false;
+    }

---

924-942: Duplicate JSDoc blocks — please consolidate

There are two consecutive “Initialize database schema from schema.sql file” comment blocks; keep one to avoid drift.

---

891-922: Verify presence of cleanup_expired_cache function in performSchemaCheck

Certainly, Sir. Extend performSchemaCheck to query pg_proc for cleanup_expired_cache and include missingFuncs in the initialization check to prevent latent runtime errors.

@@ private async performSchemaCheck(): Promise<void> {
-   const requiredTables = ['customers', 'thread_ticket_mappings', 'storage_cache'];
+   const requiredTables = ['customers', 'thread_ticket_mappings', 'storage_cache'];
+  // verify cleanup_expired_cache function exists
+  const funcs = await withDbClient(this.pool, c =>
+    c.query(`SELECT proname FROM pg_proc WHERE proname = 'cleanup_expired_cache'`)
+  );
+  const missingFuncs = funcs.rows.length === 0 ? ['cleanup_expired_cache'] : [];
@@
-   if (missingTables.length > 0) {
+   if (missingTables.length > 0 || missingFuncs.length > 0) {
    LogEngine.info('Database tables missing - setting up automatically...', {
-     missing: missingTables,
+     missingTables,
+     missingFuncs,
    });
    await this.initializeSchema();
   }

---

1019-1164: Optional resilience tweaks for parseSchemaStatements

• Restrict the dollar-tag regex to valid PG identifiers by using /^\$([A-Za-z_][A-Za-z0-9_]*)?\$/ instead of /^\$([^$]*)\$/.
• Add unit tests covering nested $$, named tags (e.g. $tag$...$tag$), DO blocks, and function bodies with embedded semicolons.

- const match = schema.slice(i).match(/^\$([^$]*)\$/);
+ const match = schema.slice(i).match(/^\$([A-Za-z_][A-Za-z0-9_]*)?\$/);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

src/utils/commandDeployment.ts (2)

133-146: Type casting approach follows discord.js patterns but lacks runtime guards.

The type assertion on line 134 assumes the client has a commands property with a specific structure. While this is a common pattern in discord.js applications (commands are typically attached to the client instance during initialization), the code doesn't verify that command.data exists or that toJSON() is callable.

If a command in the collection lacks the data property or toJSON method, the .map() call on line 141 would throw a runtime error.

Consider adding a runtime guard:

 // Convert local commands collection to JSON array
-const localCommands = Array.from(extendedClient.commands.values()).map(command => command.data.toJSON());
+const localCommands = Array.from(extendedClient.commands.values())
+	.filter(command => command.data && typeof command.data.toJSON === 'function')
+	.map(command => command.data.toJSON());

Alternatively, verify that all commands loaded into the collection during initialization have the correct structure, which would make this guard unnecessary.

#!/bin/bash
# Description: Verify command loading patterns to check if structure validation exists

# Search for command loading and collection initialization
rg -nP --type=ts -C5 'client\.commands\s*=|commands\.set\('

# Search for command file imports and structure
ast-grep --pattern $'export default { 
  data: $$$,
  $$$
}'

---

148-155: REST operations are correctly structured.

The Discord REST client initialization and API calls follow discord.js patterns. The use of Routes.applicationGuildCommands deploys commands to a specific guild, which is appropriate for most bot configurations (guild commands update instantly vs. global commands which can take up to an hour).

One consideration: The REST client doesn't have an explicit timeout configured. The discord.js REST client has default timeouts (15s for most operations), but for critical startup operations, you may want to configure this explicitly to align with your retry strategy.

If you want explicit control over timeouts to align with the 2s/4s/6s retry delays:

 // Initialize Discord REST client
-const rest = new REST().setToken(DISCORD_BOT_TOKEN);
+const rest = new REST({ timeout: 10000 }).setToken(DISCORD_BOT_TOKEN);

This would ensure the REST operation fails within 10 seconds, allowing the retry mechanism to proceed predictably.

src/events/ready.ts (1)

108-126: Sir, ensure graceful shutdown precedes process.exit(1)
I observe SIGTERM/SIGINT handlers in src/index.ts that gracefully stop the webhook consumer, but the direct process.exit(1) in ready.ts bypasses them. Consider invoking your cleanup routines (for example, await webhookConsumer.stop()) or programmatically emitting SIGTERM to trigger those handlers before exiting.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro