fix(bluebubbles): don't let unknown-service direct match short-circuit pagination

Greptile flagged that directHandleIMessageMatch was misnamed: real
iMessage;-;<handle> direct hits return immediately, so the variable
only ever held unknown-service GUIDs. It was then used in the early-
break guard, which could stop pagination before a genuine iMessage
participant match on a later page was discovered.

- Rename to directHandleUnknownServiceMatch to reflect actual usage
- Drop it from the early-break guard (only break on real iMessage hit)
- Reorder return preference: participant iMessage > SMS direct >
  unknown-service direct > SMS participant
- Add regression test covering the unknown-service short-circuit case
This commit is contained in:
Roy Martin
2026-04-06 07:20:57 -07:00
parent ba76cca927
commit c905b3a7cf
2 changed files with 63 additions and 9 deletions

View File

@@ -283,6 +283,53 @@ describe("send", () => {
expect(result).toBe("SMS;-;+15551234567"); expect(result).toBe("SMS;-;+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 () => { it("prefers iMessage over SMS via participant match", async () => {
const result = await resolveHandleTargetGuid([ const result = await resolveHandleTargetGuid([
{ {

View File

@@ -248,8 +248,11 @@ export async function resolveChatGuidForTarget(params: {
// When matching by handle, prefer iMessage over SMS. A user may have both // When matching by handle, prefer iMessage over SMS. A user may have both
// an `iMessage;-;<handle>` and `SMS;-;<handle>` chat; we should never silently // an `iMessage;-;<handle>` and `SMS;-;<handle>` chat; we should never silently
// downgrade to SMS when an iMessage chat exists for the same handle. // downgrade to SMS when an iMessage chat exists for the same handle.
let directHandleIMessageMatch: string | null = null; //
// Note: real `iMessage;-;<handle>` direct matches `return` immediately below,
// so we only need to remember SMS and unknown-service direct fallbacks here.
let directHandleSmsMatch: string | null = null; let directHandleSmsMatch: string | null = null;
let directHandleUnknownServiceMatch: string | null = null;
let participantIMessageMatch: string | null = null; let participantIMessageMatch: string | null = null;
let participantSmsMatch: string | null = null; let participantSmsMatch: string | null = null;
for (let offset = 0; offset < 5000; offset += limit) { for (let offset = 0; offset < 5000; offset += limit) {
@@ -309,11 +312,13 @@ export async function resolveChatGuidForTarget(params: {
if (guid.startsWith("iMessage;-;")) { if (guid.startsWith("iMessage;-;")) {
return guid; return guid;
} }
if (guid.startsWith("SMS;-;") && !directHandleSmsMatch) { if (guid.startsWith("SMS;-;")) {
directHandleSmsMatch = guid; if (!directHandleSmsMatch) {
} else if (!directHandleIMessageMatch && !directHandleSmsMatch) { directHandleSmsMatch = guid;
}
} else if (!directHandleUnknownServiceMatch) {
// Unknown service; treat as a last-resort direct match. // Unknown service; treat as a last-resort direct match.
directHandleIMessageMatch = guid; directHandleUnknownServiceMatch = guid;
} }
} }
if (guid && !participantIMessageMatch) { if (guid && !participantIMessageMatch) {
@@ -338,16 +343,18 @@ export async function resolveChatGuidForTarget(params: {
} }
} }
} }
// If we already found an iMessage direct or participant match we can stop // If we already found an iMessage participant match we can stop scanning
// scanning further pages; any later SMS chat would still lose out. // further pages; any later SMS chat would still lose out. We deliberately
if (directHandleIMessageMatch || participantIMessageMatch) { // do NOT break on an unknown-service direct match — a real iMessage
// participant match on a later page should still win.
if (participantIMessageMatch) {
break; break;
} }
} }
return ( return (
directHandleIMessageMatch ??
participantIMessageMatch ?? participantIMessageMatch ??
directHandleSmsMatch ?? directHandleSmsMatch ??
directHandleUnknownServiceMatch ??
participantSmsMatch participantSmsMatch
); );
} }