fix: address code review findings from PRs #12–#18

src/commands/config.js

@@ -257,7 +257,7 @@ async function handleView(interaction) {
     await interaction.reply({ embeds: [embed], ephemeral: true });
   } catch (err) {
     const safeMessage =
-      process.env.NODE_ENV === 'production' ? 'An internal error occurred.' : err.message;
+      process.env.NODE_ENV === 'development' ? err.message : 'An internal error occurred.';
     await interaction.reply({
       content: `❌ Failed to load config: ${safeMessage}`,
       ephemeral: true,
@@ -311,7 +311,7 @@ async function handleSet(interaction) {
     await interaction.editReply({ embeds: [embed] });
   } catch (err) {
     const safeMessage =
-      process.env.NODE_ENV === 'production' ? 'An internal error occurred.' : err.message;
+      process.env.NODE_ENV === 'development' ? err.message : 'An internal error occurred.';
     const content = `❌ Failed to set config: ${safeMessage}`;
     if (interaction.deferred) {
       await interaction.editReply({ content });
@@ -346,7 +346,7 @@ async function handleReset(interaction) {
     await interaction.editReply({ embeds: [embed] });
   } catch (err) {
     const safeMessage =
-      process.env.NODE_ENV === 'production' ? 'An internal error occurred.' : err.message;
+      process.env.NODE_ENV === 'development' ? err.message : 'An internal error occurred.';
     const content = `❌ Failed to reset config: ${safeMessage}`;
     if (interaction.deferred) {
       await interaction.editReply({ content });

src/index.js

@@ -67,27 +67,6 @@ client.commands = new Collection();
 // Initialize health monitor
 const healthMonitor = HealthMonitor.getInstance();
 
-// Track pending AI requests for graceful shutdown
-const pendingRequests = new Set();
-
-/**
- * Register a pending request for tracking
- * @returns {Symbol} Request ID to use for cleanup
- */
-export function registerPendingRequest() {
-  const requestId = Symbol('request');
-  pendingRequests.add(requestId);
-  return requestId;
-}
-
-/**
- * Remove a pending request from tracking
- * @param {Symbol} requestId - Request ID to remove
- */
-export function removePendingRequest(requestId) {
-  pendingRequests.delete(requestId);
-}
-
 /**
  * Save conversation history to disk
  */
@@ -224,43 +203,26 @@ client.on('interactionCreate', async (interaction) => {
 async function gracefulShutdown(signal) {
   info('Shutdown initiated', { signal });
 
-  // 1. Wait for pending requests with timeout
-  const SHUTDOWN_TIMEOUT = 10000; // 10 seconds
-  if (pendingRequests.size > 0) {
-    info('Waiting for pending requests', { count: pendingRequests.size });
-    const startTime = Date.now();
-
-    while (pendingRequests.size > 0 && Date.now() - startTime < SHUTDOWN_TIMEOUT) {
-      await new Promise((resolve) => setTimeout(resolve, 100));
-    }
-
-    if (pendingRequests.size > 0) {
-      warn('Shutdown timeout, requests still pending', { count: pendingRequests.size });
-    } else {
-      info('All requests completed');
-    }
-  }
-
-  // 2. Stop conversation cleanup timer
+  // 1. Stop conversation cleanup timer
   stopConversationCleanup();
 
-  // 3. Save state after pending requests complete
+  // 2. Save state
   info('Saving conversation state');
   saveState();
 
-  // 4. Close database pool
+  // 3. Close database pool
   info('Closing database connection');
   try {
     await closeDb();
   } catch (err) {
     error('Failed to close database pool', { error: err.message });
   }
 
-  // 5. Destroy Discord client
+  // 4. Destroy Discord client
   info('Disconnecting from Discord');
   client.destroy();
 
-  // 6. Log clean exit
+  // 5. Log clean exit
   info('Shutdown complete');
   process.exit(0);
 }
@@ -278,13 +240,6 @@ client.on('error', (err) => {
   });
 });
 
-process.on('unhandledRejection', (err) => {
-  error('Unhandled promise rejection', {
-    error: err?.message || String(err),
-    stack: err?.stack,
-    type: typeof err,
-  });
-});
 // Start bot
 const token = process.env.DISCORD_TOKEN;
 if (!token) {

src/modules/chimeIn.js

@@ -281,6 +281,7 @@ export async function accumulate(message, config) {
     // Reset counter so we don't spin on errors
     buf.counter = 0;
   } finally {
+    buf.abortController = null;
     evaluatingChannels.delete(channelId);
   }
 }

src/modules/events.js

@@ -5,6 +5,7 @@
 
 import { Client, Events } from 'discord.js';
 import { info, error as logError, warn } from '../logger.js';
+import { getUserFriendlyMessage } from '../utils/errors.js';
 import { needsSplitting, splitMessage } from '../utils/splitMessage.js';
 import { generateResponse } from './ai.js';
 import { accumulate, resetCounter } from './chimeIn.js';
@@ -93,29 +94,42 @@ export function registerMessageCreateHandler(client, config, healthMonitor) {
           .replace(new RegExp(`<@!?${client.user.id}>`, 'g'), '')
           .trim();
 
-        if (!cleanContent) {
-          await message.reply("Hey! What's up?");
-          return;
-        }
+        try {
+          if (!cleanContent) {
+            await message.reply("Hey! What's up?");
+            return;
+          }
 
-        await message.channel.sendTyping();
-
-        const response = await generateResponse(
-          message.channel.id,
-          cleanContent,
-          message.author.username,
-          config,
-          healthMonitor,
-        );
-
-        // Split long responses
-        if (needsSplitting(response)) {
-          const chunks = splitMessage(response);
-          for (const chunk of chunks) {
-            await message.channel.send(chunk);
+          await message.channel.sendTyping();
+
+          const response = await generateResponse(
+            message.channel.id,
+            cleanContent,
+            message.author.username,
+            config,
+            healthMonitor,
+          );
+
+          // Split long responses
+          if (needsSplitting(response)) {
+            const chunks = splitMessage(response);
+            for (const chunk of chunks) {
+              await message.channel.send(chunk);
+            }
+          } else {
+            await message.reply(response);
+          }
+        } catch (sendErr) {
+          logError('Failed to send AI response', {
+            channelId: message.channel.id,
+            error: sendErr.message,
+          });
+          // Best-effort fallback — if the channel is still reachable, let the user know
+          try {
+            await message.reply(getUserFriendlyMessage(sendErr));
+          } catch {
+            // Channel is unreachable — nothing more we can do
           }
-        } else {
-          await message.reply(response);
         }
 
         return; // Don't accumulate direct mentions into chime-in buffer

src/modules/welcome.js

@@ -8,13 +8,40 @@ import { info, error as logError } from '../logger.js';
 const guildActivity = new Map();
 const DEFAULT_ACTIVITY_WINDOW_MINUTES = 45;
 const MAX_EVENTS_PER_CHANNEL = 250;
+const EVICTION_INTERVAL = 50;
+
+/** Counter for throttled eviction inside recordCommunityActivity */
+let activityCallCount = 0;
 
 /** Notable member-count milestones (hoisted to avoid allocation per welcome event) */
 const NOTABLE_MILESTONES = new Set([10, 25, 50, 100, 250, 500, 1000]);
 
 /** @type {{key: string, set: Set<string>} | null} Cached excluded channels Set */
 let excludedChannelsCache = null;
 
+/**
+ * Test-only helper: snapshot guild activity state.
+ * @param {string} guildId - Guild ID
+ * @returns {Record<string, number[]>}
+ */
+export function __getCommunityActivityState(guildId) {
+  const activityMap = guildActivity.get(guildId);
+  if (!activityMap) return {};
+
+  return Object.fromEntries(
+    [...activityMap.entries()].map(([channelId, timestamps]) => [channelId, [...timestamps]]),
+  );
+}
+
+/**
+ * Test-only helper: clear in-memory activity state.
+ */
+export function __resetCommunityActivityState() {
+  guildActivity.clear();
+  activityCallCount = 0;
+  excludedChannelsCache = null;
+}
+
 /**
  * Render welcome message with placeholder replacements
  * @param {string} messageTemplate - Welcome message template
@@ -68,6 +95,31 @@ export function recordCommunityActivity(message, config) {
   }
 
   activityMap.set(message.channel.id, timestamps);
+
+  // Periodically prune stale channels to prevent unbounded memory growth
+  activityCallCount += 1;
+  if (activityCallCount >= EVICTION_INTERVAL) {
+    activityCallCount = 0;
+    pruneStaleActivity(cutoff);
+  }
+}
+
+/**
+ * Prune channels with only stale timestamps from all guilds.
+ * @param {number} cutoff - Timestamp threshold; entries older than this are stale
+ */
+function pruneStaleActivity(cutoff) {
+  for (const [guildId, activityMap] of guildActivity) {
+    for (const [channelId, timestamps] of activityMap) {
+      // If the newest timestamp is older than the cutoff, the entire array is stale
+      if (!timestamps.length || timestamps[timestamps.length - 1] < cutoff) {
+        activityMap.delete(channelId);
+      }
+    }
+    if (activityMap.size === 0) {
+      guildActivity.delete(guildId);
+    }
+  }
 }
 
 /**

tests/commands/config.test.js

@@ -169,6 +169,36 @@ describe('config command', () => {
           }),
         );
       });
+
+      it('should hide raw error details when NODE_ENV is unset', async () => {
+        const originalEnv = process.env.NODE_ENV;
+        delete process.env.NODE_ENV;
+
+        try {
+          getConfig.mockImplementationOnce(() => {
+            throw new Error('pg: connection refused at 10.0.0.5:5432');
+          });
+          const mockReply = vi.fn();
+          const interaction = {
+            options: {
+              getSubcommand: vi.fn().mockReturnValue('view'),
+              getString: vi.fn().mockReturnValue(null),
+            },
+            reply: mockReply,
+          };
+
+          await execute(interaction);
+          const content = mockReply.mock.calls[0][0].content;
+          expect(content).toContain('An internal error occurred.');
+          expect(content).not.toContain('pg: connection refused');
+        } finally {
+          if (originalEnv === undefined) {
+            delete process.env.NODE_ENV;
+          } else {
+            process.env.NODE_ENV = originalEnv;
+          }
+        }
+      });
     });
 
     describe('set subcommand', () => {

tests/index.test.js

@@ -291,16 +291,6 @@ describe('index.js', () => {
     expect(mocks.ai.setConversationHistory).toHaveBeenCalled();
   });
 
-  it('should export pending request helpers', async () => {
-    const mod = await importIndex({ token: 'abc', databaseUrl: null });
-
-    const requestId = mod.registerPendingRequest();
-    expect(typeof requestId).toBe('symbol');
-
-    // should not throw
-    mod.removePendingRequest(requestId);
-  });
-
   it('should handle autocomplete interactions', async () => {
     await importIndex({ token: 'abc', databaseUrl: null });
 
@@ -542,18 +532,6 @@ describe('index.js', () => {
     });
   });
 
-  it('should log unhandledRejection events', async () => {
-    await importIndex({ token: 'abc', databaseUrl: null });
-
-    mocks.processHandlers.unhandledRejection(new Error('rejected'));
-
-    expect(mocks.logger.error).toHaveBeenCalledWith('Unhandled promise rejection', {
-      error: 'rejected',
-      stack: expect.any(String),
-      type: 'object',
-    });
-  });
-
   it('should handle startup failure and exit', async () => {
     await importIndex({
       token: 'abc',