diff --git a/CHANGELOG.md b/CHANGELOG.md index e9bf624b9b5..11c3629014e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai - Cron/isolated-agent: keep `delivery.mode: "none"` account-only or thread-only configs from inheriting a stale implicit recipient, so isolated runs only resolve message routing when the job authored an explicit `to` target. (#69163) Thanks @obviyus. - Gateway/TUI: retry session history while the local gateway is still finishing startup, so `openclaw tui` reconnects no longer fail on transient `chat.history unavailable during gateway startup` errors. (#69164) Thanks @shakkernerd. - BlueBubbles/reactions: fall back to `love` when an agent reacts with an emoji outside the iMessage tapback set (`love`/`like`/`dislike`/`laugh`/`emphasize`/`question`), so wider-vocabulary model reactions like `👀` still produce a visible tapback instead of failing the whole reaction request. Configured ack reactions still validate strictly via the new `normalizeBlueBubblesReactionInputStrict` path. (#64693) Thanks @zqchris. +- BlueBubbles: prefer iMessage over SMS when both chats exist for the same handle, honor explicit `sms:` targets, and never silently downgrade iMessage-available recipients. (#61781) Thanks @rmartin. ## 2026.4.19-beta.2 diff --git a/extensions/bluebubbles/src/send.test.ts b/extensions/bluebubbles/src/send.test.ts index 527774412b1..281a17de2bf 100644 --- a/extensions/bluebubbles/src/send.test.ts +++ b/extensions/bluebubbles/src/send.test.ts @@ -77,16 +77,29 @@ function installSsrFPolicyCapture(policies: unknown[]) { describe("send", () => { describe("resolveChatGuidForTarget", () => { - const resolveHandleTargetGuid = async (data: Array>) => { - mockFetch.mockResolvedValueOnce({ - ok: true, - json: () => Promise.resolve({ data }), - }); + const resolveHandleTargetGuid = async ( + data: Array>, + service: "imessage" | "sms" | "auto" = "imessage", + ) => { + // First page returns the provided chats; second page is empty so the + // pagination loop exits cleanly. We can't break early on participant or + // non-preferred direct matches — a stronger preferred-service direct + // match could still appear on a later page — so we always need to mock + // at least one trailing empty page. + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: [] }), + }); const target: BlueBubblesSendTarget = { kind: "handle", address: "+15551234567", - service: "imessage", + service, }; return await resolveChatGuidForTarget({ baseUrl: "http://localhost:1234", @@ -220,6 +233,256 @@ describe("send", () => { expect(result).toBe("iMessage;-;+15551234567"); }); + it("prefers iMessage over SMS when both chats exist for the same handle", async () => { + // Both chats exist; we should never silently downgrade to SMS. + const result = await resolveHandleTargetGuid([ + { + guid: "SMS;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + { + guid: "iMessage;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ]); + + expect(result).toBe("iMessage;-;+15551234567"); + }); + + it("prefers iMessage over SMS even when SMS appears first", async () => { + const result = await resolveHandleTargetGuid([ + { + guid: "SMS;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + { + guid: "iMessage;-;+15559999999", + participants: [{ address: "+15559999999" }], + }, + { + guid: "iMessage;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ]); + + expect(result).toBe("iMessage;-;+15551234567"); + }); + + it("falls back to SMS when no iMessage chat exists for the handle", async () => { + // First page: SMS-only DM. Second page: empty (stops pagination). + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "SMS;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: [] }), + }); + + const target: BlueBubblesSendTarget = { + kind: "handle", + address: "+15551234567", + service: "imessage", + }; + const result = await resolveChatGuidForTarget({ + baseUrl: "http://localhost:1234", + password: "test", + target, + }); + + expect(result).toBe("SMS;-;+15551234567"); + }); + + it("respects explicit service: 'sms' and prefers SMS direct match over iMessage", async () => { + // Regression: when caller passes `sms:+15551234567` (target.service === + // 'sms'), explicit SMS intent must beat the default iMessage preference. + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "iMessage;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + { + guid: "SMS;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: [] }), + }); + + const target: BlueBubblesSendTarget = { + kind: "handle", + address: "+15551234567", + service: "sms", + }; + const result = await resolveChatGuidForTarget({ + baseUrl: "http://localhost:1234", + password: "test", + target, + }); + + expect(result).toBe("SMS;-;+15551234567"); + }); + + it("falls back to iMessage when service: 'sms' is requested but no SMS chat exists", async () => { + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "iMessage;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: [] }), + }); + + const target: BlueBubblesSendTarget = { + kind: "handle", + address: "+15551234567", + service: "sms", + }; + const result = await resolveChatGuidForTarget({ + baseUrl: "http://localhost:1234", + password: "test", + target, + }); + + expect(result).toBe("iMessage;-;+15551234567"); + }); + + it("prefers a later-page direct iMessage match over an earlier participant iMessage match", async () => { + // Regression: a participant-based iMessage match must NOT short-circuit + // pagination and beat a direct `iMessage;-;` match on a later page. + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "iMessage;-;alt-handle", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "iMessage;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: [] }), + }); + + const target: BlueBubblesSendTarget = { + kind: "handle", + address: "+15551234567", + service: "imessage", + }; + const result = await resolveChatGuidForTarget({ + baseUrl: "http://localhost:1234", + password: "test", + target, + }); + + expect(result).toBe("iMessage;-;+15551234567"); + }); + + it("prefers a later-page iMessage participant match over an earlier unknown-service direct match", async () => { + // Regression: an unknown-service direct match on page 1 must NOT short-circuit + // pagination and beat a real iMessage participant match on page 2. + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "WeirdService;-;+15551234567", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: [ + { + guid: "iMessage;-;alt-handle", + participants: [{ address: "+15551234567" }], + }, + ], + }), + }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: [] }), + }); + + const target: BlueBubblesSendTarget = { + kind: "handle", + address: "+15551234567", + service: "imessage", + }; + const result = await resolveChatGuidForTarget({ + baseUrl: "http://localhost:1234", + password: "test", + target, + }); + + expect(result).toBe("iMessage;-;alt-handle"); + }); + + it("prefers iMessage over SMS via participant match", async () => { + const result = await resolveHandleTargetGuid([ + { + guid: "SMS;-;alt-handle", + participants: [{ address: "+15551234567" }], + }, + { + guid: "iMessage;-;alt-handle", + participants: [{ address: "+15551234567" }], + }, + ]); + + expect(result).toBe("iMessage;-;alt-handle"); + }); + it("returns null when handle only exists in group chat (not DM)", async () => { // This is the critical fix: if a phone number only exists as a participant in a group chat // (no direct DM chat), we should NOT send to that group. Return null instead. diff --git a/extensions/bluebubbles/src/send.ts b/extensions/bluebubbles/src/send.ts index a2506a73ca5..24d2f664345 100644 --- a/extensions/bluebubbles/src/send.ts +++ b/extensions/bluebubbles/src/send.ts @@ -248,7 +248,27 @@ export async function resolveChatGuidForTarget(params: { params.target.kind === "chat_identifier" ? params.target.chatIdentifier : null; const limit = 500; - let participantMatch: string | null = null; + // When matching by handle, prefer the caller's requested service. A user may + // have both an `iMessage;-;` and `SMS;-;` chat: + // - default / `service: "imessage"` / `service: "auto"` -> prefer iMessage + // so we never silently downgrade to SMS when iMessage is available. + // - explicit `service: "sms"` (e.g. caller passed `sms:+15551234567`) -> + // prefer SMS so explicit SMS intent is respected. + // + // A direct `;-;` match is the strongest signal and + // returns immediately. Everything else is recorded as a ranked fallback. + const preferredService: "iMessage" | "SMS" = + params.target.kind === "handle" && params.target.service === "sms" ? "SMS" : "iMessage"; + const preferredPrefix = `${preferredService};-;`; + const otherPrefix = preferredService === "iMessage" ? "SMS;-;" : "iMessage;-;"; + + // Note: a direct `preferredPrefix` match `return`s immediately below, so we + // only need to remember the other-service and unknown-service direct fallbacks. + let directHandleOtherServiceMatch: string | null = null; + let directHandleUnknownServiceMatch: string | null = null; + let participantPreferredMatch: string | null = null; + let participantOtherServiceMatch: string | null = null; + let participantUnknownServiceMatch: string | null = null; for (let offset = 0; offset < 5000; offset += limit) { const chats = await queryChats({ baseUrl: params.baseUrl, @@ -299,10 +319,23 @@ export async function resolveChatGuidForTarget(params: { if (normalizedHandle) { const guid = extractChatGuid(chat); const directHandle = guid ? extractHandleFromChatGuid(guid) : null; - if (directHandle && directHandle === normalizedHandle) { - return guid; + if (directHandle && directHandle === normalizedHandle && guid) { + // A direct `` is the strongest signal and we + // can return immediately. Other services are remembered as fallbacks + // and we keep scanning in case a preferred-service chat exists later. + if (guid.startsWith(preferredPrefix)) { + return guid; + } + if (guid.startsWith(otherPrefix)) { + if (!directHandleOtherServiceMatch) { + directHandleOtherServiceMatch = guid; + } + } else if (!directHandleUnknownServiceMatch) { + // Unknown service; treat as a last-resort direct match. + directHandleUnknownServiceMatch = guid; + } } - if (!participantMatch && guid) { + if (guid) { // Only consider DM chats (`;-;` separator) as participant matches. // Group chats (`;+;` separator) should never match when searching by handle/phone. // This prevents routing "send to +1234567890" to a group chat that contains that number. @@ -312,14 +345,33 @@ export async function resolveChatGuidForTarget(params: { normalizeBlueBubblesHandle(entry), ); if (participants.includes(normalizedHandle)) { - participantMatch = guid; + if (guid.startsWith(preferredPrefix)) { + if (!participantPreferredMatch) { + participantPreferredMatch = guid; + } + } else if (guid.startsWith(otherPrefix)) { + if (!participantOtherServiceMatch) { + participantOtherServiceMatch = guid; + } + } else if (!participantUnknownServiceMatch) { + participantUnknownServiceMatch = guid; + } } } } } } + // We deliberately do NOT break early on participant or non-preferred direct + // matches: a higher-priority direct `` chat may + // still exist on a later page, and only that branch can short-circuit. } - return participantMatch; + return ( + participantPreferredMatch ?? + directHandleOtherServiceMatch ?? + participantOtherServiceMatch ?? + directHandleUnknownServiceMatch ?? + participantUnknownServiceMatch + ); } /**