mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:00:42 +00:00
fix(bluebubbles): distinguish DM vs group chat_guid in outbound session route
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.
This commit is contained in:
committed by
Peter Steinberger
parent
07089f11c7
commit
b1195c6452
81
extensions/bluebubbles/src/session-route.test.ts
Normal file
81
extensions/bluebubbles/src/session-route.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user