From c905b3a7cff575725bfa5f0e9be8d895c62fa27b Mon Sep 17 00:00:00 2001
From: Roy Martin <roy.martin@modop.com>
Date: Mon, 6 Apr 2026 07:20:57 -0700
Subject: [PATCH] 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
---
 extensions/bluebubbles/src/send.test.ts | 47 +++++++++++++++++++++++++
 extensions/bluebubbles/src/send.ts      | 25 ++++++++-----
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/extensions/bluebubbles/src/send.test.ts b/extensions/bluebubbles/src/send.test.ts
index 6a3c6f01576..2bb7071aa5e 100644
--- a/extensions/bluebubbles/src/send.test.ts
+++ b/extensions/bluebubbles/src/send.test.ts
@@ -283,6 +283,53 @@ describe("send", () => {
       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 () => {
       const result = await resolveHandleTargetGuid([
         {
diff --git a/extensions/bluebubbles/src/send.ts b/extensions/bluebubbles/src/send.ts
index 681b58b96e1..59abc6c698b 100644
--- a/extensions/bluebubbles/src/send.ts
+++ b/extensions/bluebubbles/src/send.ts
@@ -248,8 +248,11 @@ export async function resolveChatGuidForTarget(params: {
   // When matching by handle, prefer iMessage over SMS. A user may have both
   // an `iMessage;-;<handle>` and `SMS;-;<handle>` chat; we should never silently
   // 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 directHandleUnknownServiceMatch: string | null = null;
   let participantIMessageMatch: string | null = null;
   let participantSmsMatch: string | null = null;
   for (let offset = 0; offset < 5000; offset += limit) {
@@ -309,11 +312,13 @@ export async function resolveChatGuidForTarget(params: {
           if (guid.startsWith("iMessage;-;")) {
             return guid;
           }
-          if (guid.startsWith("SMS;-;") && !directHandleSmsMatch) {
-            directHandleSmsMatch = guid;
-          } else if (!directHandleIMessageMatch && !directHandleSmsMatch) {
+          if (guid.startsWith("SMS;-;")) {
+            if (!directHandleSmsMatch) {
+              directHandleSmsMatch = guid;
+            }
+          } else if (!directHandleUnknownServiceMatch) {
             // Unknown service; treat as a last-resort direct match.
-            directHandleIMessageMatch = guid;
+            directHandleUnknownServiceMatch = guid;
           }
         }
         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
-    // scanning further pages; any later SMS chat would still lose out.
-    if (directHandleIMessageMatch || participantIMessageMatch) {
+    // If we already found an iMessage participant match we can stop scanning
+    // further pages; any later SMS chat would still lose out. We deliberately
+    // do NOT break on an unknown-service direct match — a real iMessage
+    // participant match on a later page should still win.
+    if (participantIMessageMatch) {
       break;
     }
   }
   return (
-    directHandleIMessageMatch ??
     participantIMessageMatch ??
     directHandleSmsMatch ??
+    directHandleUnknownServiceMatch ??
     participantSmsMatch
   );
 }