refactor: harden slack message routing hot path

Author: Joshua Chittick

packages/ims/slack/message-router.test.ts

@@ -0,0 +1,126 @@
+import { describe, expect, it, mock } from "bun:test";
+import { registerSlackMessageRouter } from "./message-router";
+
+function createDeps(overrides: Partial<Parameters<typeof registerSlackMessageRouter>[0]> = {}) {
+  const handleIncomingMessage = mock(async () => {});
+
+  return {
+    app: {
+      message: (handler: unknown) => handler,
+    },
+    isAuthorizedChannel: () => true,
+    resolveWorkspaceAuth: () => undefined,
+    syncWorkspaceForChannel: async () => false,
+    getChannelWorkspaceName: () => "workspace",
+    setChannelWorkspaceName: () => {},
+    setChannelWorkspaceAuth: () => {},
+    isThreadActive: () => false,
+    markThreadActive: () => {},
+    isGeneralSettingsCommand: () => false,
+    postGeneralSettingsLauncher: async () => {},
+    describeSettingsIssues: () => [],
+    getChannelAgentProvider: () => "opencode" as const,
+    handleStopCommand: async () => false,
+    handleIncomingMessage,
+    ...overrides,
+  };
+}
+
+describe("registerSlackMessageRouter", () => {
+  it("caches bot identity and avoids auth.test on every message", async () => {
+    let registeredHandler: ((args: any) => Promise<void>) | undefined;
+    const authTest = mock(async () => ({ user_id: "U_BOT", team_id: "T1", enterprise_id: "E1" }));
+    const handleIncomingMessage = mock(async () => {});
+    const deps = createDeps({
+      app: {
+        message: (handler: (args: any) => Promise<void>) => {
+          registeredHandler = handler;
+        },
+      },
+      handleIncomingMessage,
+    });
+
+    registerSlackMessageRouter(deps);
+    expect(registeredHandler).toBeDefined();
+
+    const say = mock(async () => {});
+
+    await registeredHandler!({
+      message: {
+        channel: "C1",
+        user: "U1",
+        text: "<@U_BOT> hello",
+        ts: "1710000000.000001",
+      },
+      client: {
+        auth: {
+          test: authTest,
+        },
+      },
+      context: { teamId: "T1", enterpriseId: "E1" },
+      body: { team_id: "T1", enterprise_id: "E1" },
+      say,
+    });
+
+    await registeredHandler!({
+      message: {
+        channel: "C1",
+        user: "U2",
+        text: "<@U_BOT> again",
+        ts: "1710000000.000002",
+      },
+      client: {
+        auth: {
+          test: authTest,
+        },
+      },
+      context: { teamId: "T1", enterpriseId: "E1" },
+      body: { team_id: "T1", enterprise_id: "E1" },
+      say,
+    });
+
+    expect(authTest).toHaveBeenCalledTimes(1);
+    expect(handleIncomingMessage).toHaveBeenCalledTimes(2);
+  });
+
+  it("handles message processing errors with a thread reply", async () => {
+    let registeredHandler: ((args: any) => Promise<void>) | undefined;
+    const deps = createDeps({
+      app: {
+        message: (handler: (args: any) => Promise<void>) => {
+          registeredHandler = handler;
+        },
+      },
+      handleIncomingMessage: async () => {
+        throw new Error("boom");
+      },
+    });
+
+    registerSlackMessageRouter(deps);
+    expect(registeredHandler).toBeDefined();
+
+    const say = mock(async () => {});
+
+    await registeredHandler!({
+      message: {
+        channel: "C1",
+        user: "U1",
+        text: "<@U_BOT> fail",
+        ts: "1710000000.000003",
+      },
+      client: {
+        auth: {
+          test: async () => ({ user_id: "U_BOT", team_id: "T1" }),
+        },
+      },
+      context: { teamId: "T1" },
+      body: { team_id: "T1" },
+      say,
+    });
+
+    expect(say).toHaveBeenCalledWith({
+      text: "I hit an internal error while handling that message. Please try again.",
+      thread_ts: "1710000000.000003",
+    });
+  });
+});

packages/ims/slack/message-router.ts

@@ -36,6 +36,12 @@ type RouterDeps = {
 
 type WorkspaceAuth = ReturnType<RouterDeps["resolveWorkspaceAuth"]>;
 
+type BotIdentity = {
+  botUserId: string;
+  teamId?: string;
+  enterpriseId?: string;
+};
+
 type IncomingMessageData = {
   channelId: string;
   userId: string;
@@ -157,90 +163,144 @@ async function maybeHandleLauncherCommand(params: {
   return true;
 }
 
-export function registerSlackMessageRouter(deps: RouterDeps): void {
-  const slackApp = deps.app;
-
-  slackApp.message(async ({ message, say, client }: any) => {
-    const incoming = extractIncomingMessageData(message);
-    if (!incoming) return;
-
-    const { channelId, userId, text, threadId, messageId } = incoming;
-
-    const authResult = await client.auth.test();
-    const currentBotUserId = authResult.user_id as string;
-    const workspaceAuth = syncWorkspaceAuth(
-      deps,
-      channelId,
-      authResult.team_id,
-      authResult.enterprise_id ?? undefined
-    );
+function getCacheKey(teamId?: string, enterpriseId?: string): string {
+  if (teamId) return `team:${teamId}`;
+  if (enterpriseId) return `enterprise:${enterpriseId}`;
+  return "default";
+}
 
-    if (userId === currentBotUserId) {
-      log.debug("[DROP] Message from bot user", { channelId, userId });
-      return;
-    }
+async function getBotIdentity(params: {
+  client: any;
+  cache: Map<string, BotIdentity>;
+  teamId?: string;
+  enterpriseId?: string;
+}): Promise<BotIdentity> {
+  const { client, cache, teamId, enterpriseId } = params;
+  const key = getCacheKey(teamId, enterpriseId);
+  const cached = cache.get(key);
+  if (cached) {
+    return cached;
+  }
 
-    const isMention = currentBotUserId ? text.includes(`<@${currentBotUserId}>`) : false;
-    await maybeRefreshWorkspaceForMention({
-      deps,
-      channelId,
-      isMention,
-      workspaceAuth,
-    });
+  const authResult = await client.auth.test();
+  const identity: BotIdentity = {
+    botUserId: (authResult.user_id as string) || "",
+    teamId: (authResult.team_id as string | undefined) ?? teamId,
+    enterpriseId: (authResult.enterprise_id as string | undefined) ?? enterpriseId,
+  };
 
-    if (await maybeHandleStopCommand(deps, text, channelId, threadId, say)) {
-      return;
-    }
-    const threadActive = deps.isThreadActive(channelId, threadId);
+  cache.set(key, identity);
+  return identity;
+}
 
-    if (shouldDropForThreadContext(isMention, threadActive)) {
-      log.debug("[DROP] Not mentioned and thread inactive", { channelId, threadId });
-      return;
-    }
+export function registerSlackMessageRouter(deps: RouterDeps): void {
+  const slackApp = deps.app;
+  const botIdentityCache = new Map<string, BotIdentity>();
+
+  slackApp.message(async ({ message, say, client, context, body }: any) => {
+    let contextData: IncomingMessageData | null = null;
+
+    try {
+      const incoming = extractIncomingMessageData(message);
+      if (!incoming) return;
+      contextData = incoming;
+
+      const { channelId, userId, text, threadId, messageId } = incoming;
+      const identity = await getBotIdentity({
+        client,
+        cache: botIdentityCache,
+        teamId: (context?.teamId as string | undefined) ?? (body?.team_id as string | undefined),
+        enterpriseId: (context?.enterpriseId as string | undefined) ?? (body?.enterprise_id as string | undefined),
+      });
 
-    if (shouldDropForOtherMentions(text, isMention)) {
-      log.info("[DROP] Mentions other user", { channelId, threadId });
-      return;
-    }
+      const currentBotUserId = identity.botUserId;
+      const workspaceAuth = syncWorkspaceAuth(
+        deps,
+        channelId,
+        identity.teamId,
+        identity.enterpriseId
+      );
+
+      if (userId === currentBotUserId) {
+        log.debug("[DROP] Message from bot user", { channelId, userId });
+        return;
+      }
+
+      const isMention = currentBotUserId ? text.includes(`<@${currentBotUserId}>`) : false;
+      await maybeRefreshWorkspaceForMention({
+        deps,
+        channelId,
+        isMention,
+        workspaceAuth,
+      });
 
-    deps.markThreadActive(channelId, threadId);
+      if (await maybeHandleStopCommand(deps, text, channelId, threadId, say)) {
+        return;
+      }
+      const threadActive = deps.isThreadActive(channelId, threadId);
 
-    const cleanText = stripBotMention(text, currentBotUserId);
+      if (shouldDropForThreadContext(isMention, threadActive)) {
+        log.debug("[DROP] Not mentioned and thread inactive", { channelId, threadId });
+        return;
+      }
 
-    if (await maybeHandleLauncherCommand({
-      deps,
-      cleanText,
-      isMention,
-      channelId,
-      userId,
-      client,
-    })) {
-      return;
-    }
+      if (shouldDropForOtherMentions(text, isMention)) {
+        log.info("[DROP] Mentions other user", { channelId, threadId });
+        return;
+      }
 
-    if (await maybeNotifySettingsIssues(deps, channelId, threadId, userId, client, say)) {
-      return;
-    }
+      deps.markThreadActive(channelId, threadId);
 
-    const workspaceName = deps.getChannelWorkspaceName(channelId) || "unknown";
-    if (!cleanText) {
-      await say({
-        text: "Hi! How can I help you? Just ask me anything.",
-        thread_ts: threadId,
-      });
-      return;
-    }
+      const cleanText = stripBotMention(text, currentBotUserId);
 
-    await deps.handleIncomingMessage(
-      {
+      if (await maybeHandleLauncherCommand({
+        deps,
+        cleanText,
+        isMention,
         channelId,
-        replyThreadId: threadId,
-        threadId,
         userId,
-        messageId,
-        workspaceName,
-      },
-      cleanText
-    );
+        client,
+      })) {
+        return;
+      }
+
+      if (await maybeNotifySettingsIssues(deps, channelId, threadId, userId, client, say)) {
+        return;
+      }
+
+      const workspaceName = deps.getChannelWorkspaceName(channelId) || "unknown";
+      if (!cleanText) {
+        await say({
+          text: "Hi! How can I help you? Just ask me anything.",
+          thread_ts: threadId,
+        });
+        return;
+      }
+
+      await deps.handleIncomingMessage(
+        {
+          channelId,
+          replyThreadId: threadId,
+          threadId,
+          userId,
+          messageId,
+          workspaceName,
+        },
+        cleanText
+      );
+    } catch (error) {
+      log.error("Slack message router failed", {
+        channelId: contextData?.channelId,
+        threadId: contextData?.threadId,
+        messageId: contextData?.messageId,
+        error: String(error),
+      });
+      if (contextData) {
+        await say({
+          text: "I hit an internal error while handling that message. Please try again.",
+          thread_ts: contextData.threadId,
+        });
+      }
+    }
   });
 }