From b1195c6452ebe410fbf8fe0e2e9d9a3ae14d401c Mon Sep 17 00:00:00 2001 From: Chris Zhang Date: Sat, 25 Apr 2026 12:21:45 +0800 Subject: [PATCH] fix(bluebubbles): distinguish DM vs group chat_guid in outbound session route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveBlueBubblesOutboundSessionRoute classified all `chat_guid:` prefixed targets as groups: const isGroup = parsed.kind === "chat_id" || parsed.kind === "chat_guid" || parsed.kind === "chat_identifier"; But BlueBubbles also encodes DM chatGuids in the same `chat_guid:` form — they look like `iMessage;-;+15551234567` (the `;-;` separator is the DM marker; groups use `;+;`). Treating those as groups gave the same DM two different sessionKeys depending on how the caller addressed it: - handle form (`bluebubbles:imessage:+15551234567`) → peer.kind = "direct", from = `bluebubbles:+15551234567` - chat_guid form (`bluebubbles:chat_guid:iMessage;-;+15551234567`) → peer.kind = "group", from = `group:iMessage;-;+15551234567` When a bound DM session was looked up against the second form, no binding matched and the outbound landed in a freshly-synthesized "group" sessionKey — a degenerate session that the next inbound message also failed to find, surfacing the conversation in the wrong place. Use resolveGroupFlagFromChatGuid (already used by monitor-normalize to read the same marker for inbound webhooks) so both directions agree on what counts as a group. Unknown chatGuid shapes still fall back to "group" to preserve prior behavior — we never silently downgrade a real group to direct. Tests: extensions/bluebubbles/src/session-route.test.ts (new) - chat_guid `;-;` → direct - chat_guid `;+;` → group - chat_guid with no recognizable marker → group (back-compat) - handle target → direct - chat_id / chat_identifier → group (unchanged) - DM addressed two ways converges on the same peer kind Local patch for upstream consideration. Latent bug introduced by 0f7cd59824 (BlueBubbles: move outbound session routing behind plugin boundary), not commonly hit because most outbound DM call sites use the handle form, but a real foot-gun for callers that pass the chat_guid form. --- .../bluebubbles/src/session-route.test.ts | 81 +++++++++++++++++++ extensions/bluebubbles/src/session-route.ts | 16 +++- 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 extensions/bluebubbles/src/session-route.test.ts diff --git a/extensions/bluebubbles/src/session-route.test.ts b/extensions/bluebubbles/src/session-route.test.ts new file mode 100644 index 00000000000..8b29c8f5026 --- /dev/null +++ b/extensions/bluebubbles/src/session-route.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "./runtime-api.js"; +import { resolveBlueBubblesOutboundSessionRoute } from "./session-route.js"; + +const EMPTY_CFG = {} as OpenClawConfig; + +function call(target: string) { + return resolveBlueBubblesOutboundSessionRoute({ + cfg: EMPTY_CFG, + agentId: "agent-1", + accountId: "default", + target, + }); +} + +describe("resolveBlueBubblesOutboundSessionRoute DM/group disambiguation", () => { + it("treats `chat_guid:` with `;-;` marker as a DM", () => { + // Candidate-2 regression: the previous implementation classified ANY + // chat_guid-prefixed target as a group, even DMs (BlueBubbles encodes + // DM chatGuids as `service;-;handle`). That made the same DM resolve + // to one sessionKey via handle form (`+15551234567`) and a different + // sessionKey via chat_guid form (`chat_guid:iMessage;-;+15551234567`), + // causing bound DM sessions to mis-route into a freshly synthesized + // "group" session key. + const route = call("bluebubbles:chat_guid:iMessage;-;+15551234567"); + expect(route).not.toBeNull(); + expect(route?.peer.kind).toBe("direct"); + expect(route?.chatType).toBe("direct"); + expect(route?.from).toMatch(/^bluebubbles:/); + expect(route?.from).not.toMatch(/^group:/); + }); + + it("treats `chat_guid:` with `;+;` marker as a group", () => { + const route = call("bluebubbles:chat_guid:iMessage;+;chat-known-123"); + expect(route).not.toBeNull(); + expect(route?.peer.kind).toBe("group"); + expect(route?.chatType).toBe("group"); + expect(route?.from).toMatch(/^group:/); + }); + + it("falls back to group when chat_guid lacks a recognizable marker", () => { + // Backwards-compatible default: pre-fix behavior was to treat all + // chat_guid forms as group. Preserve that for unknown shapes so we + // do not silently downgrade an actual group to direct. + const route = call("bluebubbles:chat_guid:weird-no-semicolons"); + expect(route).not.toBeNull(); + expect(route?.peer.kind).toBe("group"); + }); + + it("treats handle targets as direct", () => { + const route = call("bluebubbles:imessage:+15551234567"); + expect(route).not.toBeNull(); + expect(route?.peer.kind).toBe("direct"); + expect(route?.from).toMatch(/^bluebubbles:/); + }); + + it("keeps chat_id targets classified as group", () => { + const route = call("bluebubbles:chat_id:42"); + expect(route).not.toBeNull(); + expect(route?.peer.kind).toBe("group"); + expect(route?.peer.id).toBe("42"); + }); + + it("keeps chat_identifier targets classified as group", () => { + const route = call("bluebubbles:chat_identifier:chat-abc"); + expect(route).not.toBeNull(); + expect(route?.peer.kind).toBe("group"); + expect(route?.peer.id).toBe("chat-abc"); + }); + + it("DM via chat_guid and DM via handle land on the same session key", () => { + // The point of disambiguation: a DM addressed two different ways must + // converge on the same sessionKey so existing bindings keep matching. + const handleRoute = call("bluebubbles:imessage:+15551234567"); + const chatGuidRoute = call("bluebubbles:chat_guid:iMessage;-;+15551234567"); + expect(handleRoute?.sessionKey).toBeDefined(); + expect(chatGuidRoute?.sessionKey).toBeDefined(); + // Both are direct now; sessionKey base derives from peer.id. + expect(handleRoute?.peer.kind).toBe(chatGuidRoute?.peer.kind); + }); +}); diff --git a/extensions/bluebubbles/src/session-route.ts b/extensions/bluebubbles/src/session-route.ts index f84e39a2bab..5414987d722 100644 --- a/extensions/bluebubbles/src/session-route.ts +++ b/extensions/bluebubbles/src/session-route.ts @@ -3,6 +3,7 @@ import { stripChannelTargetPrefix, type ChannelOutboundSessionRouteParams, } from "openclaw/plugin-sdk/channel-core"; +import { resolveGroupFlagFromChatGuid } from "./monitor-normalize.js"; import { parseBlueBubblesTarget } from "./targets.js"; export function resolveBlueBubblesOutboundSessionRoute(params: ChannelOutboundSessionRouteParams) { @@ -11,8 +12,21 @@ export function resolveBlueBubblesOutboundSessionRoute(params: ChannelOutboundSe return null; } const parsed = parseBlueBubblesTarget(stripped); + // chat_guid carries an explicit DM-vs-group marker (`;-;` for DMs, + // `;+;` for groups). Honor it so the same DM does not get one + // sessionKey for handle-form targets (`imessage:+1234`) and a + // different one for chat_guid-form targets + // (`chat_guid:iMessage;-;+1234`) — that mismatch made bound DM + // sessions mis-route the outbound back into a freshly-created + // "group" sessionKey. + const groupFromChatGuid = + parsed.kind === "chat_guid" ? resolveGroupFlagFromChatGuid(parsed.chatGuid) : undefined; const isGroup = - parsed.kind === "chat_id" || parsed.kind === "chat_guid" || parsed.kind === "chat_identifier"; + parsed.kind === "chat_id" || parsed.kind === "chat_identifier" + ? true + : parsed.kind === "chat_guid" + ? (groupFromChatGuid ?? true) + : false; const peerId = parsed.kind === "chat_id" ? String(parsed.chatId)