From e582c0263f586903dacc1e3b9c27e8bb9f1de7ca Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 12 Mar 2026 10:45:15 -0700 Subject: [PATCH 1/4] feat(slack): resolve @displayname mentions to <@USER_ID> in outgoing messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build a reverse index from display name → user IDs during lookupUser(), track thread participants on incoming messages, and resolve @name mentions to Slack's <@USER_ID> format before sending. Disambiguates using thread participants when multiple users share a display name. Also increases user/channel cache TTLs to 8 days. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/adapter-slack/src/index.test.ts | 339 +++++++++++++++++++++++ packages/adapter-slack/src/index.ts | 181 +++++++++++- 2 files changed, 514 insertions(+), 6 deletions(-) diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index ffe7a4b2..c4e743d9 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -1173,6 +1173,26 @@ function createMockState(): StateAdapter & { cache: Map } { cache.delete(key); return Promise.resolve(); }), + appendToList: vi + .fn() + .mockImplementation( + ( + key: string, + value: unknown, + options?: { maxLength?: number; ttlMs?: number } + ) => { + let list = (cache.get(key) as unknown[]) ?? []; + list.push(value); + if (options?.maxLength && list.length > options.maxLength) { + list = list.slice(list.length - options.maxLength); + } + cache.set(key, list); + return Promise.resolve(); + } + ), + getList: vi.fn().mockImplementation((key: string) => { + return Promise.resolve((cache.get(key) as unknown[]) ?? []); + }), }; } @@ -4598,3 +4618,322 @@ describe("isMessageFromSelf", () => { expect(result).toBe(false); }); }); + +// ============================================================================ +// Reverse User Lookup — @mention Resolution Tests +// ============================================================================ + +describe("reverse user lookup", () => { + const secret = "test-signing-secret"; + + interface MentionAdapter { + chat: ChatInstance | null; + resolveMessageMentions( + message: AdapterPostableMessage, + threadId: string + ): Promise; + resolveOutgoingMentions(text: string, threadId: string): Promise; + } + + function createAdapterWithState() { + const state = createMockState(); + const adapter = createSlackAdapter({ + botToken: "xoxb-test-token", + signingSecret: secret, + logger: mockLogger, + }); + const chatInstance = createMockChatInstance(state); + (adapter as unknown as MentionAdapter).chat = chatInstance; + return { adapter, state }; + } + + describe("reverse index storage in lookupUser", () => { + it("stores reverse index when looking up a user", async () => { + const { adapter, state } = createAdapterWithState(); + + // Mock Slack API + const mockClient = ( + adapter as unknown as { client: { users: { info: unknown } } } + ).client; + mockClient.users.info = vi.fn().mockResolvedValue({ + user: { + profile: { display_name: "dominik", real_name: "Dominik G" }, + real_name: "Dominik G", + name: "dominik", + }, + }); + + await ( + adapter as unknown as { + lookupUser( + userId: string + ): Promise<{ displayName: string; realName: string }>; + } + ).lookupUser("U_DOM_123"); + + // Check reverse index was written + const userIds = await state.getList("slack:user-by-name:dominik"); + expect(userIds).toContain("U_DOM_123"); + }); + }); + + describe("resolveOutgoingMentions", () => { + it("resolves unambiguous @mention to <@USER_ID>", async () => { + const { adapter, state } = createAdapterWithState(); + + // Seed reverse index: "dominik" → ["U_DOM_123"] + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions( + "Hey @dominik, check this out", + "slack:C123:1234567890.123456" + ); + + expect(result).toBe("Hey <@U_DOM_123>, check this out"); + }); + + it("handles case insensitivity", async () => { + const { adapter, state } = createAdapterWithState(); + + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions( + "Hey @Dominik!", + "slack:C123:1234567890.123456" + ); + + expect(result).toBe("Hey <@U_DOM_123>!"); + }); + + it("deduplicates user IDs from reverse index", async () => { + const { adapter, state } = createAdapterWithState(); + + // Same user ID appended multiple times (from repeated lookups) + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions("Hey @dominik", "slack:C123:1234567890.123456"); + + expect(result).toBe("Hey <@U_DOM_123>"); + }); + + it("leaves mention as plain text when no match found", async () => { + const { adapter } = createAdapterWithState(); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions( + "Hey @unknown_user", + "slack:C123:1234567890.123456" + ); + + expect(result).toBe("Hey @unknown_user"); + }); + + it("skips already-resolved <@USER_ID> mentions", async () => { + const { adapter, state } = createAdapterWithState(); + + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions( + "Hey <@U_DOM_123> and @dominik", + "slack:C123:1234567890.123456" + ); + + expect(result).toContain("<@U_DOM_123>"); + // The second @dominik should also be resolved + expect(result).toBe("Hey <@U_DOM_123> and <@U_DOM_123>"); + }); + + it("disambiguates using thread participants", async () => { + const { adapter, state } = createAdapterWithState(); + const threadId = "slack:C123:1234567890.123456"; + + // Two users named "alex" + await state.appendToList("slack:user-by-name:alex", "U_ALEX_1"); + await state.appendToList("slack:user-by-name:alex", "U_ALEX_2"); + + // Only U_ALEX_2 is a thread participant + await state.appendToList( + `slack:thread-participants:${threadId}`, + "U_ALEX_2" + ); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions("Hey @alex", threadId); + + expect(result).toBe("Hey <@U_ALEX_2>"); + }); + + it("leaves ambiguous mentions as plain text when thread participants don't help", async () => { + const { adapter, state } = createAdapterWithState(); + const threadId = "slack:C123:1234567890.123456"; + + // Two users named "alex" + await state.appendToList("slack:user-by-name:alex", "U_ALEX_1"); + await state.appendToList("slack:user-by-name:alex", "U_ALEX_2"); + + // Both are thread participants + await state.appendToList( + `slack:thread-participants:${threadId}`, + "U_ALEX_1" + ); + await state.appendToList( + `slack:thread-participants:${threadId}`, + "U_ALEX_2" + ); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions("Hey @alex", threadId); + + expect(result).toBe("Hey @alex"); + }); + + it("resolves multiple different mentions in one message", async () => { + const { adapter, state } = createAdapterWithState(); + + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + await state.appendToList("slack:user-by-name:malte", "U_MAL_456"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions( + "@dominik and @malte please review", + "slack:C123:1234567890.123456" + ); + + expect(result).toBe("<@U_DOM_123> and <@U_MAL_456> please review"); + }); + + it("does nothing when chat is not initialized", async () => { + const adapter = createSlackAdapter({ + botToken: "xoxb-test-token", + signingSecret: secret, + logger: mockLogger, + }); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveOutgoingMentions("Hey @dominik", "slack:C123:1234567890.123456"); + + expect(result).toBe("Hey @dominik"); + }); + }); + + describe("resolveMessageMentions", () => { + it("resolves mentions in string messages", async () => { + const { adapter, state } = createAdapterWithState(); + + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveMessageMentions( + "Hey @dominik" as AdapterPostableMessage, + "slack:C123:1234567890.123456" + ); + + expect(result).toBe("Hey <@U_DOM_123>"); + }); + + it("resolves mentions in raw messages", async () => { + const { adapter, state } = createAdapterWithState(); + + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveMessageMentions( + { raw: "Hey @dominik" } as AdapterPostableMessage, + "slack:C123:1234567890.123456" + ); + + expect(result).toEqual({ raw: "Hey <@U_DOM_123>" }); + }); + + it("resolves mentions in markdown messages", async () => { + const { adapter, state } = createAdapterWithState(); + + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveMessageMentions( + { markdown: "Hey @dominik" } as AdapterPostableMessage, + "slack:C123:1234567890.123456" + ); + + expect(result).toEqual({ markdown: "Hey <@U_DOM_123>" }); + }); + + it("passes through AST messages unchanged", async () => { + const { adapter } = createAdapterWithState(); + + const astMessage = { + ast: { type: "root", children: [] }, + } as AdapterPostableMessage; + + const result = await ( + adapter as unknown as MentionAdapter + ).resolveMessageMentions(astMessage, "slack:C123:1234567890.123456"); + + expect(result).toBe(astMessage); + }); + }); + + describe("thread participant tracking", () => { + it("tracks thread participants on incoming messages", async () => { + const { adapter, state } = createAdapterWithState(); + + // Mock Slack API so lookupUser doesn't hit real API + const mockClient = ( + adapter as unknown as { client: { users: { info: unknown } } } + ).client; + mockClient.users.info = vi.fn().mockResolvedValue({ + user: { + profile: { display_name: "sender", real_name: "Sender One" }, + real_name: "Sender One", + name: "sender", + }, + }); + + const threadId = "slack:C123:1234567890.123456"; + const event = { + type: "message", + user: "U_SENDER_1", + text: "Hello", + ts: "1234567890.123456", + channel: "C123", + thread_ts: "1234567890.123456", + }; + + // Call parseSlackMessage directly + await ( + adapter as unknown as { + parseSlackMessage( + event: Record, + threadId: string + ): Promise; + } + ).parseSlackMessage(event, threadId); + + // Wait for fire-and-forget to complete + await new Promise((resolve) => setTimeout(resolve, 10)); + + const participants = await state.getList( + `slack:thread-participants:${threadId}` + ); + expect(participants).toContain("U_SENDER_1"); + }); + }); +}); diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index 84e302da..dfc16aa1 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -67,6 +67,7 @@ import { } from "./modals"; const SLACK_USER_ID_PATTERN = /^[A-Z0-9_]+$/; +const SLACK_USER_ID_EXACT_PATTERN = /^U[A-Z0-9]+$/; /** Find the next `<@` or `<#` mention in text. */ function findNextMention(text: string): number { @@ -357,8 +358,9 @@ export class SlackAdapter implements Adapter { private _botUserId: string | null = null; private _botId: string | null = null; // Bot app ID (B_xxx) - different from user ID private readonly formatConverter = new SlackFormatConverter(); - private static USER_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour - private static CHANNEL_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour + private static USER_CACHE_TTL_MS = 8 * 24 * 60 * 60 * 1000; // 8 days + private static CHANNEL_CACHE_TTL_MS = 8 * 24 * 60 * 60 * 1000; // 8 days + private static REVERSE_INDEX_TTL_MS = 8 * 24 * 60 * 60 * 1000; // 8 days // Multi-workspace support private readonly clientId: string | undefined; @@ -725,6 +727,17 @@ export class SlackAdapter implements Adapter { { displayName, realName }, SlackAdapter.USER_CACHE_TTL_MS ); + + // Build reverse index: display name → user IDs (skip if already present) + const normalizedName = displayName.toLowerCase(); + const reverseKey = `slack:user-by-name:${normalizedName}`; + const existing = await this.chat.getState().getList(reverseKey); + if (!existing.includes(userId)) { + await this.chat.getState().appendToList(reverseKey, userId, { + maxLength: 50, + ttlMs: SlackAdapter.REVERSE_INDEX_TTL_MS, + }); + } } this.logger.debug("Fetched user info", { @@ -1833,6 +1846,28 @@ export class SlackAdapter implements Adapter { fullName = userInfo.realName; } + // Track thread participants for outgoing mention resolution (skip dupes) + if (event.user && this.chat) { + try { + const participantKey = `slack:thread-participants:${threadId}`; + const participants = await this.chat + .getState() + .getList(participantKey); + if (!participants.includes(event.user)) { + await this.chat.getState().appendToList(participantKey, event.user, { + maxLength: 100, + ttlMs: SlackAdapter.REVERSE_INDEX_TTL_MS, + }); + } + } catch (error) { + this.logger.warn("Failed to track thread participant", { + threadId, + userId: event.user, + error, + }); + } + } + // Resolve inline @mentions to display names const text = await this.resolveInlineMentions(rawText, skipSelfMention); @@ -1927,6 +1962,136 @@ export class SlackAdapter implements Adapter { }; } + /** + * Resolve @name mentions in text to Slack <@USER_ID> format using the + * reverse user cache. When multiple users share a display name, prefers + * the one who is a participant in the given thread. + */ + private async resolveOutgoingMentions( + text: string, + threadId: string + ): Promise { + if (!this.chat) { + return text; + } + const state = this.chat.getState(); + + // Find all @word patterns that aren't already wrapped in <@...> + const mentionPattern = /@(\w+)/g; + const mentions = new Map(); + + for (const match of text.matchAll(mentionPattern)) { + const name = match[1]; + // Skip if already a Slack user ID format or inside <@...> + if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { + continue; + } + // Check the character before @ to skip <@...> patterns + const idx = match.index; + if (idx > 0 && text[idx - 1] === "<") { + continue; + } + if (!mentions.has(name.toLowerCase())) { + mentions.set(name.toLowerCase(), []); + } + } + + if (mentions.size === 0) { + return text; + } + + // Look up user IDs for each mentioned name + for (const name of mentions.keys()) { + const userIds = await state.getList(`slack:user-by-name:${name}`); + // Dedup + const unique = [...new Set(userIds)]; + mentions.set(name, unique); + } + + // Load thread participants only if needed (ambiguous mentions) + let participants: Set | null = null; + const needsParticipants = [...mentions.values()].some( + (ids) => ids.length > 1 + ); + if (needsParticipants) { + const participantList = await state.getList( + `slack:thread-participants:${threadId}` + ); + participants = new Set(participantList); + } + + // Replace mentions in text + return text.replace(mentionPattern, (match, name: string) => { + const idx = text.indexOf(match); + if (idx > 0 && text[idx - 1] === "<") { + return match; + } + if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { + return match; + } + + const userIds = mentions.get(name.toLowerCase()); + if (!userIds || userIds.length === 0) { + return match; + } + if (userIds.length === 1) { + return `<@${userIds[0]}>`; + } + // Disambiguate using thread participants + if (participants) { + const inThread = userIds.filter((id) => participants.has(id)); + if (inThread.length === 1) { + return `<@${inThread[0]}>`; + } + } + // Still ambiguous — leave as plain text + return match; + }); + } + + /** + * Pre-process an outgoing message to resolve @name mentions before rendering. + */ + private async resolveMessageMentions( + message: AdapterPostableMessage, + threadId: string + ): Promise { + if (!this.chat) { + return message; + } + if (typeof message === "string") { + return this.resolveOutgoingMentions(message, threadId); + } + if (typeof message === "object" && message !== null) { + if ( + "raw" in message && + typeof (message as { raw: unknown }).raw === "string" + ) { + return { + ...message, + raw: await this.resolveOutgoingMentions( + (message as { raw: string }).raw, + threadId + ), + }; + } + if ( + "markdown" in message && + typeof (message as { markdown: unknown }).markdown === "string" + ) { + return { + ...message, + markdown: await this.resolveOutgoingMentions( + (message as { markdown: string }).markdown, + threadId + ), + }; + } + } + // AST, Card, or other formats — pass through unchanged + return message; + } + /** * Try to render a message using native Slack table blocks. * Returns blocks + fallback text if the message contains tables, null otherwise. @@ -1961,8 +2126,9 @@ export class SlackAdapter implements Adapter { async postMessage( threadId: string, - message: AdapterPostableMessage + _message: AdapterPostableMessage ): Promise> { + const message = await this.resolveMessageMentions(_message, threadId); const { channel, threadTs } = this.decodeThreadId(threadId); try { @@ -2101,8 +2267,9 @@ export class SlackAdapter implements Adapter { async postEphemeral( threadId: string, userId: string, - message: AdapterPostableMessage + _message: AdapterPostableMessage ): Promise { + const message = await this.resolveMessageMentions(_message, threadId); const { channel, threadTs } = this.decodeThreadId(threadId); try { @@ -2217,9 +2384,10 @@ export class SlackAdapter implements Adapter { async scheduleMessage( threadId: string, - message: AdapterPostableMessage, + _message: AdapterPostableMessage, options: { postAt: Date } ): Promise { + const message = await this.resolveMessageMentions(_message, threadId); const { channel, threadTs } = this.decodeThreadId(threadId); const postAtUnix = Math.floor(options.postAt.getTime() / 1000); @@ -2454,8 +2622,9 @@ export class SlackAdapter implements Adapter { async editMessage( threadId: string, messageId: string, - message: AdapterPostableMessage + _message: AdapterPostableMessage ): Promise> { + const message = await this.resolveMessageMentions(_message, threadId); const ephemeral = this.decodeEphemeralMessageId(messageId); if (ephemeral) { const { threadTs } = this.decodeThreadId(threadId); From 8cb0d908d2b25c5e909ef3e79f7a43c4a01c7b25 Mon Sep 17 00:00:00 2001 From: Matan Kushner Date: Fri, 13 Mar 2026 13:55:48 +0900 Subject: [PATCH 2/4] fix(slack): use replace offset for duplicate mentions, invalidate cache on user_change --- packages/adapter-slack/src/index.test.ts | 54 ++++++++++++++- packages/adapter-slack/src/index.ts | 84 +++++++++++++++++------- 2 files changed, 113 insertions(+), 25 deletions(-) diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index c4e743d9..50ab2f96 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -4927,7 +4927,7 @@ describe("reverse user lookup", () => { } ).parseSlackMessage(event, threadId); - // Wait for fire-and-forget to complete + // Allow participant tracking to complete await new Promise((resolve) => setTimeout(resolve, 10)); const participants = await state.getList( @@ -4936,4 +4936,56 @@ describe("reverse user lookup", () => { expect(participants).toContain("U_SENDER_1"); }); }); + + describe("user_change event", () => { + it("invalidates user cache on profile change", async () => { + const state = createMockState(); + const adapter = createSlackAdapter({ + botToken: "xoxb-test-token", + signingSecret: secret, + logger: mockLogger, + }); + await adapter.initialize(createMockChatInstance(state)); + + // Seed a cached user entry + await state.set( + "slack:user:U_DOM_123", + { displayName: "dominik", realName: "Dominik G" }, + 8 * 24 * 60 * 60 * 1000 + ); + + // Confirm cache is populated + const before = await state.get("slack:user:U_DOM_123"); + expect(before).toEqual({ + displayName: "dominik", + realName: "Dominik G", + }); + + // Send user_change event + const body = JSON.stringify({ + type: "event_callback", + event: { + type: "user_change", + event_ts: "1234567890.123456", + user: { + id: "U_DOM_123", + name: "dominik", + real_name: "Dominik New", + profile: { + display_name: "dom_new", + real_name: "Dominik New", + }, + }, + }, + }); + const request = createWebhookRequest(body, secret); + const response = await adapter.handleWebhook(request); + + expect(response.status).toBe(200); + + // Cache should be invalidated + const after = await state.get("slack:user:U_DOM_123"); + expect(after).toBeNull(); + }); + }); }); diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index dfc16aa1..9d307df4 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -244,6 +244,18 @@ interface SlackMemberJoinedChannelEvent { user: string; } +/** Slack user_change event payload */ +interface SlackUserChangeEvent { + event_ts: string; + type: "user_change"; + user: { + id: string; + name?: string; + real_name?: string; + profile?: { display_name?: string; real_name?: string }; + }; +} + /** Slack webhook payload envelope */ interface SlackWebhookPayload { challenge?: string; @@ -253,7 +265,8 @@ interface SlackWebhookPayload { | SlackAssistantThreadStartedEvent | SlackAssistantContextChangedEvent | SlackAppHomeOpenedEvent - | SlackMemberJoinedChannelEvent; + | SlackMemberJoinedChannelEvent + | SlackUserChangeEvent; event_id?: string; event_time?: number; team_id?: string; @@ -916,6 +929,8 @@ export class SlackAdapter implements Adapter { event as SlackMemberJoinedChannelEvent, options ); + } else if (event.type === "user_change") { + this.handleUserChange(event as SlackUserChangeEvent); } } } @@ -1573,6 +1588,25 @@ export class SlackAdapter implements Adapter { ); } + private handleUserChange(event: SlackUserChangeEvent): void { + if (!this.chat) { + return; + } + + const userId = event.user.id; + const cacheKey = `slack:user:${userId}`; + + this.chat + .getState() + .delete(cacheKey) + .catch((error: unknown) => { + this.logger.warn("Failed to invalidate user cache", { + userId, + error, + }); + }); + } + /** * Publish a Home tab view for a user. * Slack API: views.publish @@ -2021,32 +2055,34 @@ export class SlackAdapter implements Adapter { } // Replace mentions in text - return text.replace(mentionPattern, (match, name: string) => { - const idx = text.indexOf(match); - if (idx > 0 && text[idx - 1] === "<") { - return match; - } - if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { - return match; - } + return text.replace( + mentionPattern, + (match, name: string, offset: number) => { + if (offset > 0 && text[offset - 1] === "<") { + return match; + } + if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { + return match; + } - const userIds = mentions.get(name.toLowerCase()); - if (!userIds || userIds.length === 0) { - return match; - } - if (userIds.length === 1) { - return `<@${userIds[0]}>`; - } - // Disambiguate using thread participants - if (participants) { - const inThread = userIds.filter((id) => participants.has(id)); - if (inThread.length === 1) { - return `<@${inThread[0]}>`; + const userIds = mentions.get(name.toLowerCase()); + if (!userIds || userIds.length === 0) { + return match; } + if (userIds.length === 1) { + return `<@${userIds[0]}>`; + } + // Disambiguate using thread participants + if (participants) { + const inThread = userIds.filter((id) => participants.has(id)); + if (inThread.length === 1) { + return `<@${inThread[0]}>`; + } + } + // Still ambiguous — leave as plain text + return match; } - // Still ambiguous — leave as plain text - return match; - }); + ); } /** From 213f39466dc6e6601cc56eb130795116bfdcec9d Mon Sep 17 00:00:00 2001 From: Matan Kushner Date: Fri, 13 Mar 2026 13:57:24 +0900 Subject: [PATCH 3/4] Revert "fix(slack): use replace offset for duplicate mentions, invalidate cache on user_change" This reverts commit 8cb0d908d2b25c5e909ef3e79f7a43c4a01c7b25. --- packages/adapter-slack/src/index.test.ts | 54 +-------------- packages/adapter-slack/src/index.ts | 84 +++++++----------------- 2 files changed, 25 insertions(+), 113 deletions(-) diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index 50ab2f96..c4e743d9 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -4927,7 +4927,7 @@ describe("reverse user lookup", () => { } ).parseSlackMessage(event, threadId); - // Allow participant tracking to complete + // Wait for fire-and-forget to complete await new Promise((resolve) => setTimeout(resolve, 10)); const participants = await state.getList( @@ -4936,56 +4936,4 @@ describe("reverse user lookup", () => { expect(participants).toContain("U_SENDER_1"); }); }); - - describe("user_change event", () => { - it("invalidates user cache on profile change", async () => { - const state = createMockState(); - const adapter = createSlackAdapter({ - botToken: "xoxb-test-token", - signingSecret: secret, - logger: mockLogger, - }); - await adapter.initialize(createMockChatInstance(state)); - - // Seed a cached user entry - await state.set( - "slack:user:U_DOM_123", - { displayName: "dominik", realName: "Dominik G" }, - 8 * 24 * 60 * 60 * 1000 - ); - - // Confirm cache is populated - const before = await state.get("slack:user:U_DOM_123"); - expect(before).toEqual({ - displayName: "dominik", - realName: "Dominik G", - }); - - // Send user_change event - const body = JSON.stringify({ - type: "event_callback", - event: { - type: "user_change", - event_ts: "1234567890.123456", - user: { - id: "U_DOM_123", - name: "dominik", - real_name: "Dominik New", - profile: { - display_name: "dom_new", - real_name: "Dominik New", - }, - }, - }, - }); - const request = createWebhookRequest(body, secret); - const response = await adapter.handleWebhook(request); - - expect(response.status).toBe(200); - - // Cache should be invalidated - const after = await state.get("slack:user:U_DOM_123"); - expect(after).toBeNull(); - }); - }); }); diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index 9d307df4..dfc16aa1 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -244,18 +244,6 @@ interface SlackMemberJoinedChannelEvent { user: string; } -/** Slack user_change event payload */ -interface SlackUserChangeEvent { - event_ts: string; - type: "user_change"; - user: { - id: string; - name?: string; - real_name?: string; - profile?: { display_name?: string; real_name?: string }; - }; -} - /** Slack webhook payload envelope */ interface SlackWebhookPayload { challenge?: string; @@ -265,8 +253,7 @@ interface SlackWebhookPayload { | SlackAssistantThreadStartedEvent | SlackAssistantContextChangedEvent | SlackAppHomeOpenedEvent - | SlackMemberJoinedChannelEvent - | SlackUserChangeEvent; + | SlackMemberJoinedChannelEvent; event_id?: string; event_time?: number; team_id?: string; @@ -929,8 +916,6 @@ export class SlackAdapter implements Adapter { event as SlackMemberJoinedChannelEvent, options ); - } else if (event.type === "user_change") { - this.handleUserChange(event as SlackUserChangeEvent); } } } @@ -1588,25 +1573,6 @@ export class SlackAdapter implements Adapter { ); } - private handleUserChange(event: SlackUserChangeEvent): void { - if (!this.chat) { - return; - } - - const userId = event.user.id; - const cacheKey = `slack:user:${userId}`; - - this.chat - .getState() - .delete(cacheKey) - .catch((error: unknown) => { - this.logger.warn("Failed to invalidate user cache", { - userId, - error, - }); - }); - } - /** * Publish a Home tab view for a user. * Slack API: views.publish @@ -2055,34 +2021,32 @@ export class SlackAdapter implements Adapter { } // Replace mentions in text - return text.replace( - mentionPattern, - (match, name: string, offset: number) => { - if (offset > 0 && text[offset - 1] === "<") { - return match; - } - if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { - return match; - } + return text.replace(mentionPattern, (match, name: string) => { + const idx = text.indexOf(match); + if (idx > 0 && text[idx - 1] === "<") { + return match; + } + if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { + return match; + } - const userIds = mentions.get(name.toLowerCase()); - if (!userIds || userIds.length === 0) { - return match; - } - if (userIds.length === 1) { - return `<@${userIds[0]}>`; - } - // Disambiguate using thread participants - if (participants) { - const inThread = userIds.filter((id) => participants.has(id)); - if (inThread.length === 1) { - return `<@${inThread[0]}>`; - } - } - // Still ambiguous — leave as plain text + const userIds = mentions.get(name.toLowerCase()); + if (!userIds || userIds.length === 0) { return match; } - ); + if (userIds.length === 1) { + return `<@${userIds[0]}>`; + } + // Disambiguate using thread participants + if (participants) { + const inThread = userIds.filter((id) => participants.has(id)); + if (inThread.length === 1) { + return `<@${inThread[0]}>`; + } + } + // Still ambiguous — leave as plain text + return match; + }); } /** From 792229aa3d0f5b9487149cd7db129f0b7ebe1068 Mon Sep 17 00:00:00 2001 From: Matan Kushner Date: Fri, 13 Mar 2026 21:47:18 +0900 Subject: [PATCH 4/4] fix(slack): use replace offset for duplicate mentions, invalidate cache on user_change (#236) * fix(slack): use replace offset for duplicate mentions, invalidate cache on user_change * style: align handleUserChange to async/await, expand changeset --- .changeset/fix-mention-offset-user-change.md | 5 ++ packages/adapter-slack/src/index.test.ts | 47 +++++++++++- packages/adapter-slack/src/index.ts | 80 ++++++++++++++------ 3 files changed, 107 insertions(+), 25 deletions(-) create mode 100644 .changeset/fix-mention-offset-user-change.md diff --git a/.changeset/fix-mention-offset-user-change.md b/.changeset/fix-mention-offset-user-change.md new file mode 100644 index 00000000..04315ca9 --- /dev/null +++ b/.changeset/fix-mention-offset-user-change.md @@ -0,0 +1,5 @@ +--- +"@chat-adapter/slack": patch +--- + +Fix duplicate mention resolution by using the replace callback offset instead of indexOf. Invalidate user cache on Slack user_change events so display name updates are picked up immediately. diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index c4e743d9..91cd0989 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -4927,7 +4927,7 @@ describe("reverse user lookup", () => { } ).parseSlackMessage(event, threadId); - // Wait for fire-and-forget to complete + // Allow participant tracking to complete await new Promise((resolve) => setTimeout(resolve, 10)); const participants = await state.getList( @@ -4936,4 +4936,49 @@ describe("reverse user lookup", () => { expect(participants).toContain("U_SENDER_1"); }); }); + + describe("user_change event", () => { + it("invalidates user cache on profile change", async () => { + const state = createMockState(); + const adapter = createSlackAdapter({ + botToken: "xoxb-test-token", + signingSecret: secret, + logger: mockLogger, + }); + await adapter.initialize(createMockChatInstance(state)); + + // Seed user cache + await state.set( + "slack:user:U_DOM_123", + { displayName: "dominik", realName: "Dominik G" }, + 8 * 24 * 60 * 60 * 1000 + ); + + const body = JSON.stringify({ + type: "event_callback", + event: { + type: "user_change", + event_ts: "1234567890.123456", + user: { + id: "U_DOM_123", + name: "dominik", + real_name: "Dominik New", + profile: { + display_name: "dom_new", + real_name: "Dominik New", + }, + }, + }, + }); + const request = createWebhookRequest(body, secret); + const response = await adapter.handleWebhook(request); + + expect(response.status).toBe(200); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + const cached = await state.get("slack:user:U_DOM_123"); + expect(cached).toBeNull(); + }); + }); }); diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index dfc16aa1..ebc419d4 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -244,6 +244,18 @@ interface SlackMemberJoinedChannelEvent { user: string; } +/** Slack user_change event payload */ +interface SlackUserChangeEvent { + event_ts: string; + type: "user_change"; + user: { + id: string; + name?: string; + real_name?: string; + profile?: { display_name?: string; real_name?: string }; + }; +} + /** Slack webhook payload envelope */ interface SlackWebhookPayload { challenge?: string; @@ -253,7 +265,8 @@ interface SlackWebhookPayload { | SlackAssistantThreadStartedEvent | SlackAssistantContextChangedEvent | SlackAppHomeOpenedEvent - | SlackMemberJoinedChannelEvent; + | SlackMemberJoinedChannelEvent + | SlackUserChangeEvent; event_id?: string; event_time?: number; team_id?: string; @@ -916,6 +929,8 @@ export class SlackAdapter implements Adapter { event as SlackMemberJoinedChannelEvent, options ); + } else if (event.type === "user_change") { + this.handleUserChange(event as SlackUserChangeEvent); } } } @@ -1573,6 +1588,21 @@ export class SlackAdapter implements Adapter { ); } + private async handleUserChange(event: SlackUserChangeEvent): Promise { + if (!this.chat) { + return; + } + + try { + await this.chat.getState().delete(`slack:user:${event.user.id}`); + } catch (error) { + this.logger.warn("Failed to invalidate user cache", { + userId: event.user.id, + error, + }); + } + } + /** * Publish a Home tab view for a user. * Slack API: views.publish @@ -2021,32 +2051,34 @@ export class SlackAdapter implements Adapter { } // Replace mentions in text - return text.replace(mentionPattern, (match, name: string) => { - const idx = text.indexOf(match); - if (idx > 0 && text[idx - 1] === "<") { - return match; - } - if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { - return match; - } + return text.replace( + mentionPattern, + (match, name: string, offset: number) => { + if (offset > 0 && text[offset - 1] === "<") { + return match; + } + if (SLACK_USER_ID_EXACT_PATTERN.test(name)) { + return match; + } - const userIds = mentions.get(name.toLowerCase()); - if (!userIds || userIds.length === 0) { - return match; - } - if (userIds.length === 1) { - return `<@${userIds[0]}>`; - } - // Disambiguate using thread participants - if (participants) { - const inThread = userIds.filter((id) => participants.has(id)); - if (inThread.length === 1) { - return `<@${inThread[0]}>`; + const userIds = mentions.get(name.toLowerCase()); + if (!userIds || userIds.length === 0) { + return match; + } + if (userIds.length === 1) { + return `<@${userIds[0]}>`; + } + // Disambiguate using thread participants + if (participants) { + const inThread = userIds.filter((id) => participants.has(id)); + if (inThread.length === 1) { + return `<@${inThread[0]}>`; + } } + // Still ambiguous — leave as plain text + return match; } - // Still ambiguous — leave as plain text - return match; - }); + ); } /**