mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(imessage): preserve inbound SMS routing in auto mode [AI-assisted] (#125633)
* fix(imessage): preserve inbound SMS routing in auto mode * fix(imessage): preserve direct read receipt routing
This commit is contained in:
committed by
GitHub
parent
d9dfc5b3d9
commit
43e0150aa4
@@ -32,6 +32,7 @@ import {
|
||||
normalizeIMessageMessagingTarget,
|
||||
type ChannelPlugin,
|
||||
} from "./channel-api.js";
|
||||
import { resolveIMessageDirectChatService } from "./chat-context.js";
|
||||
import { createIMessageConversationBindingManager } from "./conversation-bindings.js";
|
||||
import {
|
||||
matchIMessageAcpConversation,
|
||||
@@ -238,11 +239,9 @@ function resolveIMessageOutboundSessionRoute(params: {
|
||||
}
|
||||
const account = resolveIMessageAccount({ cfg: params.cfg, accountId: params.accountId });
|
||||
const service =
|
||||
parsed.serviceExplicit || parsed.service !== "auto"
|
||||
? parsed.service
|
||||
: account.config.service === "sms"
|
||||
? "sms"
|
||||
: "imessage";
|
||||
resolveIMessageDirectChatService(
|
||||
parsed.serviceExplicit ? parsed.service : account.config.service,
|
||||
) ?? "auto";
|
||||
const directTarget = `${service}:${handle}`;
|
||||
const peer: RoutePeer = { kind: "direct", id: handle };
|
||||
const baseSessionKey = buildIMessageBaseSessionKey({
|
||||
|
||||
@@ -38,6 +38,17 @@ function parseDirectChatIdentity(raw: string): IMessageDirectChatIdentity | unde
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function resolveIMessageDirectChatService(
|
||||
configuredService?: IMessageService | null,
|
||||
chatGuid?: string | null,
|
||||
): Exclude<IMessageService, "auto"> | undefined {
|
||||
if (configuredService === "imessage" || configuredService === "sms") {
|
||||
return configuredService;
|
||||
}
|
||||
const observedService = chatGuid ? parseDirectChatIdentity(chatGuid)?.service : undefined;
|
||||
return observedService === "imessage" || observedService === "sms" ? observedService : undefined;
|
||||
}
|
||||
|
||||
export function isIMessageEmailChatIdentifier(raw: string): boolean {
|
||||
const identity = parseDirectChatIdentity(raw);
|
||||
return Boolean(identity && EMAIL_HANDLE_PATTERN.test(identity.identifier));
|
||||
|
||||
@@ -296,13 +296,24 @@ describe("iMessage monitor last-route updates", () => {
|
||||
function createInboundMessage(
|
||||
message: Pick<IMessagePayload, "id" | "guid" | "text"> &
|
||||
Partial<
|
||||
Pick<IMessagePayload, "chat_id" | "sender" | "is_from_me" | "is_group" | "created_at">
|
||||
Pick<
|
||||
IMessagePayload,
|
||||
| "chat_id"
|
||||
| "chat_guid"
|
||||
| "chat_identifier"
|
||||
| "sender"
|
||||
| "is_from_me"
|
||||
| "is_group"
|
||||
| "created_at"
|
||||
>
|
||||
>,
|
||||
): IMessagePayload {
|
||||
return {
|
||||
id: message.id,
|
||||
guid: message.guid,
|
||||
chat_id: message.chat_id ?? 123,
|
||||
chat_guid: message.chat_guid,
|
||||
chat_identifier: message.chat_identifier,
|
||||
sender: message.sender ?? DEFAULT_SENDER,
|
||||
is_from_me: message.is_from_me ?? false,
|
||||
text: message.text,
|
||||
@@ -311,6 +322,167 @@ describe("iMessage monitor last-route updates", () => {
|
||||
};
|
||||
}
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "SMS chat with service unset",
|
||||
configuredService: undefined,
|
||||
chatGuid: "SMS;-;+15550001111",
|
||||
expectedService: "sms",
|
||||
},
|
||||
{
|
||||
label: "SMS chat with service auto",
|
||||
configuredService: "auto",
|
||||
chatGuid: "SMS;-;+15550001111",
|
||||
expectedService: "sms",
|
||||
},
|
||||
{
|
||||
label: "iMessage chat with service unset",
|
||||
configuredService: undefined,
|
||||
chatGuid: "iMessage;-;+15550001111",
|
||||
expectedService: "imessage",
|
||||
},
|
||||
{
|
||||
label: "explicit SMS override for an iMessage chat",
|
||||
configuredService: "sms",
|
||||
chatGuid: "iMessage;-;+15550001111",
|
||||
expectedService: "sms",
|
||||
},
|
||||
{
|
||||
label: "explicit iMessage override for an SMS chat",
|
||||
configuredService: "imessage",
|
||||
chatGuid: "SMS;-;+15550001111",
|
||||
expectedService: "imessage",
|
||||
},
|
||||
{
|
||||
label: "unknown direct chat service",
|
||||
configuredService: undefined,
|
||||
chatGuid: "any;-;+15550001111",
|
||||
expectedService: "auto",
|
||||
},
|
||||
{
|
||||
label: "absent direct chat GUID",
|
||||
configuredService: undefined,
|
||||
chatGuid: undefined,
|
||||
expectedService: "auto",
|
||||
},
|
||||
] as const)(
|
||||
"preserves the inbound direct service through early typing, final delivery, and last-route ($label)",
|
||||
async ({ label, configuredService, chatGuid, expectedService }) => {
|
||||
setAvailablePrivateApiMethods(["watch.subscribe", "send", "typing", "read"]);
|
||||
const stateDir = createTestStateDir(
|
||||
`openclaw-imsg-direct-route-${label.replaceAll(" ", "-")}-`,
|
||||
);
|
||||
const configuredStore = path.join(stateDir, "sessions.json");
|
||||
const storePath = resolveStorePath(configuredStore, { agentId: "main" });
|
||||
dispatchReplyWithBufferedBlockDispatcherMock.mockImplementationOnce(async (params) => {
|
||||
await params.dispatcherOptions.deliver(
|
||||
{ text: "reply over the originating service" },
|
||||
{
|
||||
kind: "final",
|
||||
},
|
||||
);
|
||||
return EMPTY_DISPATCH_RESULT;
|
||||
});
|
||||
const client = await runMessageCase({
|
||||
auxiliaryRequests: {
|
||||
read: { ok: true },
|
||||
send: { guid: "sms-reply-guid" },
|
||||
typing: { ok: true },
|
||||
},
|
||||
message: createInboundMessage({
|
||||
id: 101,
|
||||
guid: `direct-route-${expectedService}-${label}`,
|
||||
chat_guid: chatGuid,
|
||||
chat_identifier: "+15550001111",
|
||||
text: "reply to this direct chat",
|
||||
}),
|
||||
monitor: {
|
||||
imessage: configuredService ? { service: configuredService } : {},
|
||||
session: { dmScope: "per-channel-peer", store: configuredStore },
|
||||
},
|
||||
});
|
||||
const auxiliaryClient = client.auxiliaryClient!;
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(auxiliaryClient.request).toHaveBeenCalledWith(
|
||||
"typing",
|
||||
expect.objectContaining({ service: expectedService, to: DEFAULT_SENDER, typing: true }),
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
const expectedReadTarget = chatGuid ? { chat_guid: chatGuid } : { to: DEFAULT_SENDER };
|
||||
expect(auxiliaryClient.request).toHaveBeenCalledWith(
|
||||
"read",
|
||||
expect.objectContaining(expectedReadTarget),
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(auxiliaryClient.request).toHaveBeenCalledWith(
|
||||
"send",
|
||||
expect.objectContaining({
|
||||
service: expectedService,
|
||||
text: "reply over the originating service",
|
||||
to: DEFAULT_SENDER,
|
||||
}),
|
||||
expect.any(Object),
|
||||
);
|
||||
const dispatchParams = dispatchReplyWithBufferedBlockDispatcherMock.mock.calls.at(0)?.[0];
|
||||
expect(dispatchParams?.ctx).toMatchObject({
|
||||
From: `${expectedService}:${DEFAULT_SENDER}`,
|
||||
To: `${expectedService}:${DEFAULT_SENDER}`,
|
||||
});
|
||||
await vi.waitFor(() => {
|
||||
expect(
|
||||
getSessionEntry({
|
||||
storePath,
|
||||
sessionKey: `agent:main:imessage:direct:${DEFAULT_SENDER}`,
|
||||
}),
|
||||
).toMatchObject({
|
||||
delivery: {
|
||||
context: { channel: "imessage", to: `${expectedService}:${DEFAULT_SENDER}` },
|
||||
route: { target: { to: `${expectedService}:${DEFAULT_SENDER}` } },
|
||||
},
|
||||
});
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps group chat_id routing unchanged through final delivery", async () => {
|
||||
setAvailablePrivateApiMethods(["watch.subscribe", "send", "typing", "read"]);
|
||||
dispatchReplyWithBufferedBlockDispatcherMock.mockImplementationOnce(async (params) => {
|
||||
await params.dispatcherOptions.deliver({ text: "group reply" }, { kind: "final" });
|
||||
return EMPTY_DISPATCH_RESULT;
|
||||
});
|
||||
const client = await runMessageCase({
|
||||
auxiliaryRequests: {
|
||||
read: { ok: true },
|
||||
send: { guid: "group-reply-guid" },
|
||||
typing: { ok: true },
|
||||
},
|
||||
message: createInboundMessage({
|
||||
id: 103,
|
||||
guid: "group-route-guid",
|
||||
chat_id: 456,
|
||||
chat_guid: "iMessage;+;chat456",
|
||||
is_group: true,
|
||||
text: "reply to this group",
|
||||
}),
|
||||
monitor: { allowlist: false, imessage: { groupPolicy: "open" } },
|
||||
});
|
||||
const auxiliaryClient = client.auxiliaryClient!;
|
||||
|
||||
expect(auxiliaryClient.request).toHaveBeenCalledWith(
|
||||
"send",
|
||||
expect.objectContaining({ chat_id: 456, service: "auto", text: "group reply" }),
|
||||
expect.any(Object),
|
||||
);
|
||||
const dispatchParams = dispatchReplyWithBufferedBlockDispatcherMock.mock.calls.at(0)?.[0];
|
||||
expect(dispatchParams?.ctx).toMatchObject({
|
||||
ChatType: "group",
|
||||
From: "imessage:group:456",
|
||||
To: "chat_id:456",
|
||||
});
|
||||
});
|
||||
|
||||
function createAnchorlessDirectPair(id: number, text: string, isFromMe: boolean) {
|
||||
return {
|
||||
notification: {
|
||||
@@ -845,13 +1017,13 @@ describe("iMessage monitor last-route updates", () => {
|
||||
kind: "external",
|
||||
context: {
|
||||
channel: "imessage",
|
||||
to: "imessage:+15550001111",
|
||||
to: "auto:+15550001111",
|
||||
accountId: "default",
|
||||
},
|
||||
route: {
|
||||
channel: "imessage",
|
||||
accountId: "default",
|
||||
target: { to: "imessage:+15550001111" },
|
||||
target: { to: "auto:+15550001111" },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
@@ -36,6 +36,7 @@ import { uniqueStrings } from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import { sanitizeTerminalText } from "openclaw/plugin-sdk/text-chunking";
|
||||
import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime";
|
||||
import { resolveIMessageAccount } from "../accounts.js";
|
||||
import { resolveIMessageDirectChatService } from "../chat-context.js";
|
||||
import { resolveIMessageConversationRoute } from "../conversation-route.js";
|
||||
import {
|
||||
isKnownFromMeIMessageMessageId,
|
||||
@@ -984,13 +985,15 @@ export async function buildIMessageInboundContext(params: {
|
||||
});
|
||||
}
|
||||
|
||||
const directService =
|
||||
resolveIMessageDirectChatService(
|
||||
resolveIMessageAccount({ cfg: params.cfg, accountId: decision.route.accountId }).config
|
||||
.service,
|
||||
decision.chatGuid,
|
||||
) ?? "auto";
|
||||
const imessageTo = decision.isGroup
|
||||
? chatTarget || `imessage:${decision.sender}`
|
||||
: buildDirectIMessageReplyTarget({
|
||||
cfg: params.cfg,
|
||||
accountId: decision.route.accountId,
|
||||
sender: decision.sender,
|
||||
});
|
||||
: `${directService}:${decision.sender}`;
|
||||
// Async follow-ups can resume from the stored origin instead of the immediate
|
||||
// reply target. Keep direct SMS origins service-qualified the same way as To,
|
||||
// or the final resumed message can fall back to imessage:<phone>.
|
||||
@@ -1109,19 +1112,6 @@ function buildIMessageEchoScope(params: {
|
||||
return scopes;
|
||||
}
|
||||
|
||||
export function buildDirectIMessageReplyTarget(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId?: string | null;
|
||||
sender: string;
|
||||
}): string {
|
||||
const account = resolveIMessageAccount({ cfg: params.cfg, accountId: params.accountId });
|
||||
const configuredService = account.config.service;
|
||||
if (configuredService === "sms") {
|
||||
return `sms:${params.sender}`;
|
||||
}
|
||||
return `imessage:${params.sender}`;
|
||||
}
|
||||
|
||||
function describeIMessageEchoDropLog(params: { messageText: string; messageId?: string }): string {
|
||||
const preview = truncateUtf16Safe(params.messageText, 50);
|
||||
const messageIdPart = params.messageId ? ` id=${params.messageId}` : "";
|
||||
|
||||
@@ -56,6 +56,7 @@ import { maybeResolveIMessageApprovalPollVote } from "../approval-polls.js";
|
||||
import { pollPendingIMessageApprovalReactions } from "../approval-reaction-poller.js";
|
||||
import { maybeResolveIMessageApprovalReaction } from "../approval-reactions.js";
|
||||
import { buildIMessageApprovalConversationKeyForInbound } from "../approval-target-keys.js";
|
||||
import { resolveIMessageDirectChatService } from "../chat-context.js";
|
||||
import { markIMessageChatRead, sendIMessageTyping } from "../chat.js";
|
||||
import { resolveIMessageChatDbLookupPath } from "../cli-path.js";
|
||||
import { createIMessageRpcClient, type IMessageRpcClient } from "../client.js";
|
||||
@@ -96,7 +97,6 @@ import {
|
||||
isStaleIMessageBacklog,
|
||||
} from "./inbound-dedupe.js";
|
||||
import {
|
||||
buildDirectIMessageReplyTarget,
|
||||
buildIMessageInboundContext,
|
||||
mergeIMessageGroupAllowFromWithLegacyChatTargets,
|
||||
rememberIMessageSkippedFromMeForSelfChatDedupe,
|
||||
@@ -941,12 +941,10 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P
|
||||
sendPolicy !== "deny" &&
|
||||
(configuredTypingMode === undefined || configuredTypingMode === "instant");
|
||||
const shouldStartDirectTyping = supportsTyping && shouldUseDirectToolTypingOptions;
|
||||
const earlyDirectTypingService =
|
||||
resolveIMessageDirectChatService(imessageCfg.service, decision.chatGuid) ?? "auto";
|
||||
const earlyDirectTypingTarget = shouldStartDirectTyping
|
||||
? buildDirectIMessageReplyTarget({
|
||||
cfg,
|
||||
accountId: decision.route.accountId,
|
||||
sender: decision.sender,
|
||||
})
|
||||
? `${earlyDirectTypingService}:${decision.sender}`
|
||||
: undefined;
|
||||
let stopEarlyDirectTyping: (() => void) | undefined;
|
||||
if (earlyDirectTypingTarget) {
|
||||
@@ -1080,13 +1078,17 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P
|
||||
|
||||
const sendReadReceipts = imessageCfg.sendReadReceipts !== false;
|
||||
const typingTarget = ctxPayload.To;
|
||||
// The read RPC has no service argument, so preserve the inbound direct
|
||||
// conversation through its exact chat GUID instead of a bare handle.
|
||||
const readTarget =
|
||||
!decision.isGroup && decision.chatGuid ? `chat_guid:${decision.chatGuid}` : typingTarget;
|
||||
|
||||
if (supportsRead && sendReadReceipts && typingTarget) {
|
||||
if (supportsRead && sendReadReceipts && readTarget) {
|
||||
// Read receipts are best-effort channel UI. Do not put them on the
|
||||
// critical path before model dispatch; slow private-API reads otherwise
|
||||
// make accepted iMessage turns feel stuck before the agent starts. Use
|
||||
// a short-lived client so a stuck read cannot block monitor-client typing.
|
||||
void markIMessageChatRead(typingTarget, {
|
||||
void markIMessageChatRead(readTarget, {
|
||||
cfg,
|
||||
accountId: accountInfo.accountId,
|
||||
cliPath,
|
||||
|
||||
@@ -44,7 +44,7 @@ import {
|
||||
type IMessageApprovalConversationKey,
|
||||
registerIMessageApprovalReactionTarget,
|
||||
} from "./approval-reactions.js";
|
||||
import { chatContextFromIMessageTarget } from "./chat-context.js";
|
||||
import { chatContextFromIMessageTarget, resolveIMessageDirectChatService } from "./chat-context.js";
|
||||
import { runIMessageCliJsonCommand } from "./cli-output.js";
|
||||
import { resolveIMessageChatDbLookupPath } from "./cli-path.js";
|
||||
import {
|
||||
@@ -559,14 +559,6 @@ function resultService(value: unknown): Exclude<IMessageService, "auto"> | undef
|
||||
return normalized === "imessage" || normalized === "sms" ? normalized : undefined;
|
||||
}
|
||||
|
||||
function resultChatGuidService(value: unknown): Exclude<IMessageService, "auto"> | undefined {
|
||||
const chatGuid = stringValue(value);
|
||||
if (/^imessage;/iu.test(chatGuid ?? "")) {
|
||||
return "imessage";
|
||||
}
|
||||
return /^sms;/iu.test(chatGuid ?? "") ? "sms" : undefined;
|
||||
}
|
||||
|
||||
function resolvePendingPersistedEchoTtlMs(timeoutMs: number): number {
|
||||
return Math.max(
|
||||
MIN_PENDING_PERSISTED_ECHO_TTL_MS,
|
||||
@@ -1187,10 +1179,10 @@ export async function sendMessageIMessage(
|
||||
// before dispatching. Inbound recording (in monitor/inbound-processing)
|
||||
// sets isFromMe=false, so the cache distinguishes own-sent from received.
|
||||
const providerChatGuid = stringValue(result.chat_guid) ?? stringValue(result.chatGuid);
|
||||
const confirmedService =
|
||||
resultService(result.service) ??
|
||||
resultChatGuidService(providerChatGuid) ??
|
||||
(service === "imessage" || service === "sms" ? service : undefined);
|
||||
const confirmedService = resolveIMessageDirectChatService(
|
||||
resultService(result.service) ?? service,
|
||||
providerChatGuid,
|
||||
);
|
||||
if (resolvedId && isConcreteIMessageMessageId(resolvedId)) {
|
||||
const chatContext = chatContextFromIMessageTarget(target, confirmedService ?? service);
|
||||
rememberIMessageReplyCache({
|
||||
|
||||
@@ -22,4 +22,43 @@ describe("iMessage outbound session routing", () => {
|
||||
|
||||
expect(route?.recipientSessionExact).toBe(exact);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["uses auto when a bare direct target has no configured service", {}, "+15551234567", "auto"],
|
||||
[
|
||||
"uses the configured SMS override for a bare direct target",
|
||||
{ channels: { imessage: { service: "sms" } } },
|
||||
"+15551234567",
|
||||
"sms",
|
||||
],
|
||||
[
|
||||
"uses the configured iMessage override for a bare direct target",
|
||||
{ channels: { imessage: { service: "imessage" } } },
|
||||
"+15551234567",
|
||||
"imessage",
|
||||
],
|
||||
[
|
||||
"keeps an explicit SMS target authoritative",
|
||||
{ channels: { imessage: { service: "imessage" } } },
|
||||
"sms:+15551234567",
|
||||
"sms",
|
||||
],
|
||||
[
|
||||
"keeps an explicit auto target authoritative",
|
||||
{ channels: { imessage: { service: "sms" } } },
|
||||
"auto:+15551234567",
|
||||
"auto",
|
||||
],
|
||||
] as const)("%s", async (_label, cfg, target, expectedService) => {
|
||||
const route = await imessagePlugin.messaging?.resolveOutboundSessionRoute?.({
|
||||
cfg,
|
||||
agentId: "main",
|
||||
target,
|
||||
});
|
||||
|
||||
expect(route).toMatchObject({
|
||||
from: `${expectedService}:+15551234567`,
|
||||
to: `${expectedService}:+15551234567`,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user