diff --git a/apps/simplex-support-bot/bot.test.ts b/apps/simplex-support-bot/bot.test.ts index 418d1965b9..33acd421d3 100644 --- a/apps/simplex-support-bot/bot.test.ts +++ b/apps/simplex-support-bot/bot.test.ts @@ -1409,26 +1409,42 @@ describe("Error Handling", () => { expectNotSentToGroup(CUSTOMER_GROUP_ID, "temporarily unavailable") }) - test("groupDuplicateMember on /team → apiListMembers fallback", async () => { - await reachQueue() - addBotMessage("The team will reply to your message") + test("/team while members are in Invited status → no second apiAddMember call", async () => { + await reachTeamPending() + addBotMessage("We will reply within 24 hours.") - // First team member add succeeds, second fails with groupDuplicateMember - let callCount = 0 - const origAddMember = chat.apiAddMember.bind(chat) - chat.apiAddMember = async (groupId: number, contactId: number, role: string) => { - callCount++ - if (callCount === 2) { - chat.members.set(groupId, [ - {memberId: `team-${contactId}`, groupMemberId: 5000 + contactId, memberContactId: contactId, memberStatus: GroupMemberStatus.Connected, memberProfile: {displayName: `Contact${contactId}`}}, - ]) - throw {chatError: {errorType: {type: "groupDuplicateMember"}}} - } - return origAddMember(groupId, contactId, role) - } + // Simulate the realistic post-/team state: both members have been invited + // but have not yet accepted (memberStatus = "invited"). The SimpleX API + // would resend the invitation if apiAddMember is called for an Invited + // member — the pre-check in addOrFindTeamMember must skip them. + const invited = (contactId: number) => ({ + memberId: `team-${contactId}`, + groupMemberId: 5000 + contactId, + memberContactId: contactId, + memberStatus: "invited", + memberProfile: {displayName: `Contact${contactId}`}, + }) + chat.members.set(CUSTOMER_GROUP_ID, [invited(TEAM_MEMBER_1_ID), invited(TEAM_MEMBER_2_ID)]) + chat.added.length = 0 await bot.onNewChatItems(customerMessage("/team")) - expectSentToGroup(CUSTOMER_GROUP_ID, "We will reply within") + expect(chat.added.filter(a => a.groupId === CUSTOMER_GROUP_ID)).toEqual([]) + }) + + test("/grok in TEAM-PENDING while Grok is in Invited status → no second apiAddMember call", async () => { + await reachTeamPending() + addBotMessage("We will reply within 24 hours.") + chat.members.set(CUSTOMER_GROUP_ID, [ + makeTeamMember(TEAM_MEMBER_1_ID, "Alice"), + {memberId: "grok-member", groupMemberId: 7777, memberContactId: GROK_CONTACT_ID, memberStatus: "invited", memberProfile: {displayName: "Grok"}}, + ]) + chat.added.length = 0 + + await bot.onNewChatItems(customerMessage("/grok")) + await bot.flush() + + expect(chat.added.filter(a => a.contactId === GROK_CONTACT_ID)).toEqual([]) + expectNotSentToGroup(CUSTOMER_GROUP_ID, "Inviting Grok") }) }) diff --git a/apps/simplex-support-bot/plans/20260207-support-bot-implementation.md b/apps/simplex-support-bot/plans/20260207-support-bot-implementation.md index f3ad98495f..8caab92b6d 100644 --- a/apps/simplex-support-bot/plans/20260207-support-bot-implementation.md +++ b/apps/simplex-support-bot/plans/20260207-support-bot-implementation.md @@ -700,7 +700,7 @@ Type signatures affected: **Main profile side (invite + failure detection):** 0. Send `grokInvitingMessage` ("Inviting Grok, please wait...") 1. **Set `grokInitialResponsePending.add(groupId)` FIRST** — the gate must be raised before any operation that could make Grok recognizable to `onGrokNewChatItems`. Specifically: before `apiAddMember`, before `pendingGrokJoins` is set, and before `bufferedGrokInvitations` is drained (which populates `reverseGrokMap`). Without this ordering, the sequence `apiAddMember → pendingGrokJoins.set → drain → reverseGrokMap.set → gate.add` contains a window where `reverseGrokMap` identifies the group as a Grok-active group but the gate is still DOWN. A customer message arriving in that window triggers a per-message response concurrent with the initial combined response — producing duplicate Grok replies. Every error path below MUST clear the gate. -2. `apiAddMember(groupId, grokContactId, Member)` → get `member.memberId`. On `groupDuplicateMember` (customer sent `/grok` again before join completed), **clear the gate** and silent return — the in-flight activation handles the outcome. On any other error, clear the gate, revert state, send `grokUnavailableMessage`. +2. **Pre-check via `apiListMembers`**: silent return if Grok is already in the group in any non-terminal status (covers `GSMemInvited`, which the SimpleX API would otherwise resend the invitation for without throwing). Then `apiAddMember(groupId, grokContactId, Member)` → get `member.memberId`. On `groupDuplicateMember` (race between pre-check and add — Grok joined as Connected meanwhile), **clear the gate** and silent return — the in-flight activation handles the outcome. On any other error, clear the gate, revert state, send `grokUnavailableMessage`. 3. Store `pendingGrokJoins.set(memberId, mainGroupId)` 4. Drain `bufferedGrokInvitations` — if the `receivedGroupInvitation` event arrived during step 2's await (race condition), process it now. (The gate is already up from step 1, so `onGrokNewChatItems` suppresses any per-message responses during drain and the subsequent join.) 5. `waitForGrokJoin(120s)` — awaits resolver from Grok profile's `connectedToGroupMember` (step 8 below) @@ -958,7 +958,7 @@ If a user contacts the bot via a regular direct-message address (not business ad | Team member leaves (message sent) | State stays `TEAM` (`customData.state` persists). Customer's next `/team` re-adds silently. | | No `--auto-add-team-members` (`-a`) configured | `/team` → "no team members available yet" | | `grokContactId` unavailable | `/grok` → "temporarily unavailable" | -| `groupDuplicateMember` | Catch, `apiListMembers` to find existing member | +| Member already in group when `/team` re-runs | `addOrFindTeamMember` pre-checks via `apiListMembers` and skips `apiAddMember` entirely if the contact is present in any non-terminal status (so an `Invited`-but-not-yet-accepted member is never re-invited — the SimpleX API would otherwise resend the invitation for `GSMemInvited`) | ## 16. API Call Map @@ -1084,7 +1084,7 @@ npx ts-node src/index.ts \ 23. DM handshake via `connectedToGroupMember` → verify contact ID message sent (dedup with #22) 24. Restart → verify same team group + Grok contact from state file, cards resume via `customData` 25. No `--auto-add-team-members` (`-a`) → `/team` → verify "no team members available" -26. `groupDuplicateMember` → verify `apiListMembers` fallback +26. Repeated `/team` while members are still in `Invited` status → verify `apiAddMember` is NOT called again (pre-check in `addOrFindTeamMember` returns the existing member) 27. Team member leaves (no message sent) → verify revert to QUEUE 28. Team member leaves (message sent), customer sends `/team` → verify re-adds team members 29. Card preview sender prefixes → verify first message in each consecutive sender run gets `Name:` prefix, subsequent same-sender messages do not @@ -1333,8 +1333,8 @@ Called as: `const p = simulateGrokJoinSuccess(); await bot.onNewChatItems(...); #### 17. Error Handling (3 tests) - apiAddMember fails (Grok) → grokUnavailableMessage -- groupDuplicateMember on Grok invite → only inviting message, no result (in-flight activation handles outcome) -- groupDuplicateMember on /team → apiListMembers fallback +- /grok while Grok already present (any non-terminal status, including `Invited`) → pre-check silent-returns, no `apiAddMember` call. Plus race coverage: simulated `groupDuplicateMember` thrown by `apiAddMember` → silent return, no further state change +- /team while team member already present (any non-terminal status, including `Invited`) → `apiAddMember` not called for that member #### 18. Profile / Event Filtering (4 tests) - newChatItems from Grok profile → ignored by main handler diff --git a/apps/simplex-support-bot/src/bot.ts b/apps/simplex-support-bot/src/bot.ts index 205a3f144b..60a41eeaa1 100644 --- a/apps/simplex-support-bot/src/bot.ts +++ b/apps/simplex-support-bot/src/bot.ts @@ -10,6 +10,23 @@ import { } from "./messages.js" import {profileMutex, log, logError} from "./util.js" +// True for any non-terminal status — invited but not yet accepted, through +// connected. Used to decide whether a contact is already in the group so we +// don't trigger a re-invite (the SimpleX API resends the invitation for a +// member in GSMemInvited). +function isInGroup(m: T.GroupMember): boolean { + switch (m.memberStatus) { + case T.GroupMemberStatus.Rejected: + case T.GroupMemberStatus.Removed: + case T.GroupMemberStatus.Left: + case T.GroupMemberStatus.Deleted: + case T.GroupMemberStatus.Unknown: + return false + default: + return true + } +} + export class SupportBot { // Card manager cards: CardManager @@ -573,6 +590,16 @@ export class SupportBot { return } + // Pre-check: silent return if Grok is already in the group in any + // non-terminal status. The apiAddMember/groupDuplicateMember catch below + // handles Connected/etc. but the SimpleX API resends the invitation for + // GSMemInvited (no error thrown), so without this check a /grok issued + // while a previous activation is still pending would re-trigger the invite. + const grokMembers = await this.withMainProfile(() => this.chat.apiListMembers(groupId)) + if (grokMembers.some(m => m.memberContactId === this.config.grokContactId && isInGroup(m))) { + return + } + // Gate MUST be up before apiAddMember / pendingGrokJoins / reverseGrokMap — // any later and onGrokNewChatItems can fire a duplicate per-message reply. this.grokInitialResponsePending.add(groupId) @@ -767,19 +794,16 @@ export class SupportBot { // --- Helpers --- private async addOrFindTeamMember(groupId: number, teamContactId: number): Promise { - try { - return await this.withMainProfile(() => - this.chat.apiAddMember(groupId, teamContactId, T.GroupMemberRole.Member) - ) - } catch (err: unknown) { - const chatErr = err as {chatError?: {errorType?: {type?: string}}} - if (chatErr?.chatError?.errorType?.type === "groupDuplicateMember") { - log(`Team member already in group ${groupId}, looking up existing`) - const members = await this.withMainProfile(() => this.chat.apiListMembers(groupId)) - return members.find(m => m.memberContactId === teamContactId) ?? null - } - throw err - } + // Pre-check membership: skip apiAddMember entirely if the contact is in + // the group in any non-terminal status. The SimpleX API resends the + // invitation for a member in GSMemInvited, so calling apiAddMember on a + // pending invitee would re-trigger an invite notification. + const members = await this.withMainProfile(() => this.chat.apiListMembers(groupId)) + const existing = members.find(m => m.memberContactId === teamContactId && isInGroup(m)) + if (existing) return existing + return await this.withMainProfile(() => + this.chat.apiAddMember(groupId, teamContactId, T.GroupMemberRole.Member) + ) } async sendToGroup(groupId: number, text: string): Promise {