diff --git a/.changeset/fix-mention-offset-user-change.md b/.changeset/fix-mention-offset-user-change.md new file mode 100644 index 00000000..ded9fb95 --- /dev/null +++ b/.changeset/fix-mention-offset-user-change.md @@ -0,0 +1,5 @@ +--- +"@chat-adapter/slack": patch +--- + +Fix duplicate mention resolution using replace offset, invalidate user cache on profile changes diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index 943d832b..88ef195d 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -4944,7 +4944,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( @@ -4953,4 +4953,89 @@ 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 and reverse index + await state.set( + "slack:user:U_DOM_123", + { displayName: "dominik", realName: "Dominik G" }, + 8 * 24 * 60 * 60 * 1000 + ); + await state.appendToList("slack:user-by-name:dominik", "U_DOM_123"); + + // 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); + + // Allow async handler to complete + await new Promise((resolve) => setTimeout(resolve, 50)); + + // User cache should be invalidated + const cached = await state.get("slack:user:U_DOM_123"); + expect(cached).toBeNull(); + + // Old reverse index entry should be cleaned up + const reverseIndex = await state.getList("slack:user-by-name:dominik"); + expect(reverseIndex).not.toContain("U_DOM_123"); + }); + + it("handles user_change when no prior cache exists", async () => { + const state = createMockState(); + const adapter = createSlackAdapter({ + botToken: "xoxb-test-token", + signingSecret: secret, + logger: mockLogger, + }); + await adapter.initialize(createMockChatInstance(state)); + + const body = JSON.stringify({ + type: "event_callback", + event: { + type: "user_change", + event_ts: "1234567890.123456", + user: { + id: "U_UNKNOWN", + profile: { display_name: "newname" }, + }, + }, + }); + const request = createWebhookRequest(body, secret); + const response = await adapter.handleWebhook(request); + + expect(response.status).toBe(200); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Should not throw — just deletes (nonexistent) cache key + const cached = await state.get("slack:user:U_UNKNOWN"); + expect(cached).toBeNull(); + }); + }); }); diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index dfc16aa1..fb528195 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,33 @@ export class SlackAdapter implements Adapter { ); } + private handleUserChange(event: SlackUserChangeEvent): void { + if (!this.chat) { + return; + } + + const userId = event.user.id; + const cacheKey = `slack:user:${userId}`; + const state = this.chat.getState(); + + state + .get(cacheKey) + .then(async (cached) => { + if (cached) { + const oldName = cached.displayName.toLowerCase(); + const reverseKey = `slack:user-by-name:${oldName}`; + await state.removeFromList(reverseKey, userId); + } + await state.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 +2063,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; - }); + ); } /**