ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-27 21:07:01 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix: stop routing IDs from masquerading as delivery receipts (#126385)

Browse Source 
* fix(channels): separate routing from receipt identity

Routing identifiers no longer fabricate message delivery evidence; provider-canonical thread placement is preserved.

* fix(channels): preserve conflicting receipt threads

Keep aggregate thread placement absent when provider receipt parts disagree, even when a requested route thread is available.
This commit is contained in:
Peter Steinberger
2026-08-19 11:33:16 -07:00
committed by GitHub
parent 341551937e
commit 61d217fd2a
11 changed files with 261 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plugins/sdk-channel-outbound.md
+8 -3
View File
@@ -148,9 +148,14 @@ the channel boundary instead of rewriting marker text after sanitization.
A `MessageReceipt` records the result returned by a channel adapter. Concrete
platform message identifiers show that the platform send path accepted the
message; they do not prove that a recipient's device displayed or read it.
Receipts without platform message identifiers are local receipt metadata only.
Channels with read receipts or device-delivery state should track those facts
through a separate channel-specific path.
Destination and routing identifiers such as chat, channel, room, conversation,
or recipient JID are metadata, never `platformMessageIds`. Receipts without
platform message identifiers are local receipt metadata only. A
provider-observed receipt thread overrides the requested route thread. If a
batch contains conflicting provider threads, each part retains its thread and
the aggregate receipt omits `threadId`. Channels with read receipts or
device-delivery state should track those facts through a separate
channel-specific path.
If a channel adapter can prove that retrying a failure cannot duplicate a
recipient-visible send and no finalization-capable call began, throw
src/auto-reply/reply/route-reply.delivery-result.test.ts
-19
View File
@@ -180,25 +180,6 @@ describe("routeReply delivery result", () => {
});
});
it("reports delivery when the provider returns a non-id delivery identity", async () => {
mocks.deliverOutboundPayloads.mockResolvedValueOnce([
{ channel: "whatsapp", messageId: "", toJid: "group:ops" },
]);
const res = await routeReply({
payload: { text: "hello" },
channel: "whatsapp",
to: "group:ops",
cfg: {} as never,
});
expect(res).toEqual({
ok: true,
delivered: true,
messageId: "",
});
});
it.each([
["skipped", false, undefined],
["suppressed", false, undefined],
src/channels/message/outbound-bridge.test.ts
+49
View File
@@ -115,6 +115,37 @@ describe("createChannelMessageAdapterFromOutbound", () => {
});
});
it("preserves route-only final and progress metadata without fabricating message ids", async () => {
const sendText = vi.fn(
async (request: {
onDeliveryResult?: (result: ChannelMessageOutboundBridgeResult) => Promise<void> | void;
}) => {
await request.onDeliveryResult?.({ messageId: "", toJid: "progress-route" });
return { chatId: "final-route" };
},
);
const onDeliveryResult = vi.fn();
const adapter = createChannelMessageAdapterFromOutbound({ outbound: { sendText } });
const result = await adapter.send?.text?.({
cfg,
to: "room-1",
text: "hello",
onDeliveryResult,
});
expect(onDeliveryResult).toHaveBeenCalledWith({
toJid: "progress-route",
receipt: expect.objectContaining({ platformMessageIds: [], parts: [] }),
});
expect(onDeliveryResult.mock.calls[0]?.[0]).not.toHaveProperty("messageId");
expect(result).toMatchObject({
chatId: "final-route",
receipt: { platformMessageIds: [], parts: [] },
});
expect(result).not.toHaveProperty("messageId");
});
it("preserves target-only routing metadata without fabricating delivery identity", async () => {
const target = { kind: "channel" as const, id: "route-only" };
const adapter = createChannelMessageAdapterFromOutbound({
@@ -234,6 +265,24 @@ describe("createChannelMessageAdapterFromOutbound", () => {
).resolves.toEqual({ messageId: "legacy-id", receipt });
});
it("preserves an authoritative empty receipt with routing metadata", async () => {
const receipt: MessageReceipt = {
platformMessageIds: [],
parts: [],
threadId: "canonical-thread",
sentAt: 123,
};
const adapter = createChannelMessageAdapterFromOutbound({
outbound: {
sendText: vi.fn(async () => ({ messageId: "", toJid: "route-only", receipt })),
},
});
await expect(
adapter.send?.text?.({ cfg, to: "room-1", text: "hello", threadId: "requested-thread" }),
).resolves.toEqual({ toJid: "route-only", receipt });
});
it.each([
{
name: "portable presentation with fallback text",
src/channels/message/outbound-bridge.ts
+3 -20
View File
@@ -3,7 +3,7 @@
*
* Wraps old channel send functions in the newer channel message adapter contract.
*/
import { createMessageReceiptFromOutboundResults } from "./receipt.js";
import { createMessageReceiptFromOutboundResults, resolveReceiptSourceId } from "./receipt.js";
import type {
ChannelMessageAdapterShape,
ChannelMessageLiveAdapterShape,
@@ -62,20 +62,6 @@ type CreateChannelMessageAdapterFromOutboundParams<TConfig = unknown> = {
receive?: ChannelMessageReceiveAdapterShape;
};
function resolveResultMessageId(result: ChannelMessageOutboundBridgeResult): string | undefined {
return (
result.messageId ??
result.receipt?.primaryPlatformMessageId ??
result.receipt?.platformMessageIds[0] ??
result.chatId ??
result.channelId ??
result.roomId ??
result.conversationId ??
result.toJid ??
result.pollId
);
}
type MessageSendResultParams = {
kind: MessageReceiptPartKind;
normalizeReceiptKind?: boolean;
@@ -100,6 +86,7 @@ function toMessageSendResult(
threadId: params.threadId == null ? undefined : String(params.threadId),
replyToId: params.replyToId ?? undefined,
});
const messageId = resolveReceiptSourceId({ ...result, receipt });
return {
// Preserve sanctioned owner facts for delivery hooks without exposing private
// provider fields or trusting a provider-authored channel identity.
@@ -113,11 +100,7 @@ function toMessageSendResult(
...(result.timestamp !== undefined ? { timestamp: result.timestamp } : {}),
...(result.meta !== undefined ? { meta: result.meta } : {}),
receipt,
...(resolveResultMessageId({ ...result, receipt })
? {
messageId: resolveResultMessageId({ ...result, receipt }),
}
: {}),
...(messageId ? { messageId } : {}),
};
}
src/channels/message/receipt.test.ts
+95 -8
View File
@@ -36,15 +36,27 @@ describe("createMessageReceiptFromOutboundResults", () => {
]);
});
it("uses legacy WhatsApp platform ids when no adapter receipt exists", () => {
const receipt = createMessageReceiptFromOutboundResults({
results: [{ channel: "whatsapp", messageId: "", toJid: "jid-1" }],
sentAt: 123,
});
it.each(
(["chatId", "channelId", "roomId", "conversationId", "toJid"] as const).flatMap((field) => [
{ field, messageId: undefined, messageIdLabel: "absent" },
{ field, messageId: "", messageIdLabel: "blank" },
]),
)(
"keeps $field routing metadata with $messageIdLabel messageId out of platform identity",
({ field, messageId }) => {
const result = {
channel: "demo",
...(messageId === undefined ? {} : { messageId }),
[field]: "route-only",
};
const receipt = createMessageReceiptFromOutboundResults({ results: [result], sentAt: 123 });
expect(receipt.primaryPlatformMessageId).toBe("jid-1");
expect(receipt.platformMessageIds).toEqual(["jid-1"]);
});
expect(receipt.primaryPlatformMessageId).toBeUndefined();
expect(receipt.platformMessageIds).toEqual([]);
expect(receipt.parts).toEqual([]);
expect(receipt.raw).toEqual([result]);
},
);
it("does not use target routing metadata as platform message identity", () => {
const target = { kind: "channel" as const, id: "route-only" };
@@ -140,6 +152,81 @@ describe("createMessageReceiptFromOutboundResults", () => {
expect(receipt.sentAt).toBe(456);
});
it("uses nested canonical threads before the requested route when filling parts", () => {
const receipt = createMessageReceiptFromOutboundResults({
results: [
{
channel: "googlechat",
receipt: {
platformMessageIds: ["m1", "m2"],
parts: [
{ platformMessageId: "m1", kind: "text", index: 0 },
{ platformMessageId: "m2", kind: "text", index: 1 },
],
threadId: "canonical-thread",
sentAt: 123,
},
},
],
threadId: "requested-thread",
});
expect(receipt.threadId).toBe("canonical-thread");
expect(receipt.parts.map((part) => part.threadId)).toEqual([
"canonical-thread",
"canonical-thread",
]);
});
it("uses a canonical part thread before receipt and requested fallbacks", () => {
const receipt = createMessageReceiptFromOutboundResults({
results: [
{
receipt: {
platformMessageIds: ["m1"],
parts: [{ platformMessageId: "m1", kind: "text", index: 0, threadId: "part-thread" }],
threadId: "receipt-thread",
sentAt: 123,
},
},
],
threadId: "requested-thread",
});
expect(receipt.threadId).toBe("part-thread");
expect(receipt.parts[0]?.threadId).toBe("part-thread");
});
it("keeps conflicting provider threads on parts and omits the aggregate thread", () => {
const receipt = createMessageReceiptFromOutboundResults({
results: [
{
receipt: {
platformMessageIds: ["m1"],
parts: [{ platformMessageId: "m1", kind: "text", index: 0 }],
threadId: "canonical-thread-1",
sentAt: 123,
},
},
{
receipt: {
platformMessageIds: ["m2"],
parts: [{ platformMessageId: "m2", kind: "text", index: 0 }],
threadId: "canonical-thread-2",
sentAt: 124,
},
},
],
threadId: "requested-thread",
});
expect(receipt.threadId).toBeUndefined();
expect(receipt.parts.map((part) => part.threadId)).toEqual([
"canonical-thread-1",
"canonical-thread-2",
]);
});
it("preserves mixed nested reply metadata when the route has a reply target", () => {
const receipt = createMessageReceiptFromOutboundResults({
results: [
src/channels/message/receipt.ts
+49 -18
View File
@@ -14,15 +14,14 @@ type MessageReceiptInputResult = MessageReceiptSourceResult & {
receipt?: MessageReceipt;
};
function resolveReceiptMessageId(result: MessageReceiptInputResult): string | undefined {
const normalizeIdentity = (value: string | undefined): string | undefined =>
value?.trim() || undefined;
export function resolveReceiptSourceId(result: MessageReceiptInputResult): string | undefined {
return (
result.messageId ||
result.chatId ||
result.channelId ||
result.roomId ||
result.conversationId ||
result.toJid ||
result.pollId
normalizeIdentity(result.messageId) ??
(result.receipt ? resolveMessageReceiptPrimaryId(result.receipt) : undefined) ??
normalizeIdentity(result.pollId)
);
}
@@ -41,14 +40,27 @@ export function createMessageReceiptFromOutboundResults(params: {
replyToId?: string;
sentAt?: number;
}): MessageReceipt {
const requestedThreadId = normalizeIdentity(params.threadId);
const providerThreadIds = normalizeUniqueStringEntries(
params.results.flatMap(({ receipt }) =>
receipt?.parts.length
? receipt.parts.flatMap(
(part) => normalizeIdentity(part.threadId) ?? normalizeIdentity(receipt.threadId) ?? [],
)
: (normalizeIdentity(receipt?.threadId) ?? []),
),
);
const aggregateThreadId =
providerThreadIds.length > 1 ? undefined : (providerThreadIds[0] ?? requestedThreadId);
const parts = params.results.flatMap((result, resultIndex) => {
if (result.receipt) {
const receiptThreadId = normalizeIdentity(result.receipt.threadId) ?? requestedThreadId;
if (result.receipt.parts.length === 0) {
return result.receipt.platformMessageIds.map((platformMessageId, partIndex) => ({
platformMessageId,
kind: params.kind ?? "unknown",
index: partIndex,
...(params.threadId ? { threadId: params.threadId } : {}),
...(receiptThreadId ? { threadId: receiptThreadId } : {}),
...(params.replyToId ? { replyToId: params.replyToId } : {}),
}));
}
@@ -58,13 +70,15 @@ export function createMessageReceiptFromOutboundResults(params: {
return result.receipt.parts.map((part, partIndex) => ({
...part,
index: part.index ?? partIndex,
...(part.threadId || !params.threadId ? {} : { threadId: params.threadId }),
...(normalizeIdentity(part.threadId) || !receiptThreadId
? {}
: { threadId: receiptThreadId }),
...(part.replyToId || !params.replyToId || hasPartReplyMetadata
? {}
: { replyToId: params.replyToId }),
}));
}
const platformMessageId = resolveReceiptMessageId(result);
const platformMessageId = resolveReceiptSourceId(result);
if (!platformMessageId) {
return [];
}
@@ -73,7 +87,7 @@ export function createMessageReceiptFromOutboundResults(params: {
platformMessageId,
kind: params.kind ?? "unknown",
index: resultIndex,
...(params.threadId ? { threadId: params.threadId } : {}),
...(requestedThreadId ? { threadId: requestedThreadId } : {}),
...(params.replyToId ? { replyToId: params.replyToId } : {}),
raw: result,
},
@@ -91,16 +105,14 @@ export function createMessageReceiptFromOutboundResults(params: {
}
continue;
}
appendUnique(platformMessageIds, resolveReceiptMessageId(result));
appendUnique(platformMessageIds, resolveReceiptSourceId(result));
}
const firstNestedReceipt = params.results.find((result) => result.receipt)?.receipt;
return {
...(platformMessageIds[0] ? { primaryPlatformMessageId: platformMessageIds[0] } : {}),
platformMessageIds,
parts,
...((params.threadId ?? firstNestedReceipt?.threadId)
? { threadId: params.threadId ?? firstNestedReceipt?.threadId }
: {}),
...(aggregateThreadId ? { threadId: aggregateThreadId } : {}),
...((params.replyToId ?? firstNestedReceipt?.replyToId)
? { replyToId: params.replyToId ?? firstNestedReceipt?.replyToId }
: {}),
@@ -116,9 +128,28 @@ export function listMessageReceiptPlatformIds(receipt: MessageReceipt): string[]
/** Resolves the explicit primary platform id, falling back to the first unique receipt id. */
export function resolveMessageReceiptPrimaryId(receipt: MessageReceipt): string | undefined {
const primary = receipt.primaryPlatformMessageId?.trim();
const primary = normalizeIdentity(receipt.primaryPlatformMessageId);
if (primary) {
return primary;
}
return listMessageReceiptPlatformIds(receipt)[0];
return (
listMessageReceiptPlatformIds(receipt)[0] ??
receipt.parts.map((part) => normalizeIdentity(part.platformMessageId)).find(Boolean)
);
}
/** Resolves provider-owned thread placement without collapsing conflicting receipt parts. */
export function resolveMessageReceiptThreadId(
receipt: MessageReceipt,
requestedThreadId?: string,
): string | undefined {
const partThreadIds = normalizeUniqueStringEntries(
receipt.parts.flatMap((part) => normalizeIdentity(part.threadId) ?? []),
);
if (partThreadIds.length > 1) {
return undefined;
}
return (
partThreadIds[0] ?? normalizeIdentity(receipt.threadId) ?? normalizeIdentity(requestedThreadId)
);
}
src/channels/message/send.test.ts
+23 -2
View File
@@ -278,6 +278,7 @@ describe("withDurableMessageSendContext", () => {
{ platformMessageId: "platform-1", kind: "text", index: 0 },
{ platformMessageId: "platform-2", kind: "media", index: 1 },
],
threadId: "canonical-thread",
sentAt: 123,
},
},
@@ -288,11 +289,17 @@ describe("withDurableMessageSendContext", () => {
channel: "telegram",
to: "chat-1",
payloads: [{ text: "hello" }],
threadId: "requested-thread",
});
expectBatchStatus(result, "sent");
expect(result.receipt?.primaryPlatformMessageId).toBe("platform-1");
expect(result.receipt?.platformMessageIds).toEqual(["platform-1", "platform-2"]);
expect(result.receipt?.threadId).toBe("canonical-thread");
expect(result.receipt?.parts.map((part) => part.threadId)).toEqual([
"canonical-thread",
"canonical-thread",
]);
expect(
result.receipt?.parts.map(({ platformMessageId, kind }) => ({ platformMessageId, kind })),
).toEqual([
@@ -593,7 +600,18 @@ describe("withDurableMessageSendContext", () => {
const cause = new Error("network reset");
const error = new OutboundDeliveryError("network reset", {
cause,
results: [{ channel: "telegram", messageId: "msg-1" }],
results: [
{
channel: "telegram",
messageId: "msg-1",
receipt: {
platformMessageIds: ["msg-1"],
parts: [{ platformMessageId: "msg-1", kind: "text", index: 0 }],
threadId: "canonical-thread",
sentAt: 123,
},
},
],
payloadOutcomes: [
{
index: 0,
@@ -618,12 +636,15 @@ describe("withDurableMessageSendContext", () => {
channel: "telegram",
to: "chat-1",
payloads: [{ text: "first" }, { text: "second" }],
threadId: "requested-thread",
onSendFailure,
});
expectBatchStatus(result, "partial_failed");
expect(result.results).toEqual([{ channel: "telegram", messageId: "msg-1" }]);
expect(result.results).toEqual(error.results);
expect(result.receipt?.platformMessageIds).toEqual(["msg-1"]);
expect(result.receipt?.threadId).toBe("canonical-thread");
expect(result.receipt?.parts[0]?.threadId).toBe("canonical-thread");
expect(result.error).toBe(error);
expect(result.sentBeforeError).toBe(true);
expect(onSendFailure).toHaveBeenCalledWith(error);
src/channels/turn/delivery-result.test.ts
+24 -2
View File
@@ -1,5 +1,6 @@
// Delivery result tests cover channel turn delivery result normalization.
import { describe, expect, it } from "vitest";
import type { MessageReceipt } from "../message/types.js";
import {
createChannelDeliveryResultFromReceipt,
createChannelPartialDeliveryError,
@@ -17,13 +18,14 @@ describe("createChannelDeliveryResultFromReceipt", () => {
primaryPlatformMessageId: "m1",
platformMessageIds: ["m1", "m2"],
parts: [],
threadId: "canonical-thread",
sentAt: 123,
};
expect(
createChannelDeliveryResultFromReceipt({
receipt,
threadId: "thread-1",
threadId: "requested-thread",
replyToId: "reply-1",
visibleReplySent: true,
deliveryIntent: {
@@ -35,7 +37,7 @@ describe("createChannelDeliveryResultFromReceipt", () => {
).toEqual({
messageIds: ["m1", "m2"],
receipt,
threadId: "thread-1",
threadId: "canonical-thread",
replyToId: "reply-1",
visibleReplySent: true,
deliveryIntent: {
@@ -46,6 +48,26 @@ describe("createChannelDeliveryResultFromReceipt", () => {
});
});
it("does not restore the requested route when provider receipt parts conflict", () => {
const receipt: MessageReceipt = {
platformMessageIds: ["m1", "m2"],
parts: [
{ platformMessageId: "m1", kind: "text", index: 0, threadId: "thread-1" },
{ platformMessageId: "m2", kind: "text", index: 1, threadId: "thread-2" },
],
sentAt: 123,
};
const result = createChannelDeliveryResultFromReceipt({
receipt,
threadId: "requested-thread",
visibleReplySent: true,
});
expect(result).not.toHaveProperty("threadId");
expect(result.receipt).toBe(receipt);
});
it("preserves suppressed receipt results without synthetic message ids", () => {
const receipt = {
platformMessageIds: [],
src/channels/turn/delivery-result.ts
+6 -2
View File
@@ -1,6 +1,9 @@
// Delivery-result adapters for channel turn receipts.
import { formatErrorMessage } from "../../infra/errors.js";
import { listMessageReceiptPlatformIds } from "../message/receipt.js";
import {
listMessageReceiptPlatformIds,
resolveMessageReceiptThreadId,
} from "../message/receipt.js";
import type { MessageReceipt } from "../message/types.js";
import type {
ChannelDeliveryIntent,
@@ -74,10 +77,11 @@ export function createChannelDeliveryResultFromReceipt(params: {
deliveryIntent?: ChannelDeliveryIntent;
}): ChannelDeliveryResult {
const messageIds = listMessageReceiptPlatformIds(params.receipt);
const threadId = resolveMessageReceiptThreadId(params.receipt, params.threadId);
return {
...(messageIds.length > 0 ? { messageIds } : {}),
receipt: params.receipt,
...(params.threadId ? { threadId: params.threadId } : {}),
...(threadId ? { threadId } : {}),
...(params.replyToId ? { replyToId: params.replyToId } : {}),
...(params.visibleReplySent === undefined ? {} : { visibleReplySent: params.visibleReplySent }),
...(params.content === undefined ? {} : { content: params.content }),
src/infra/outbound/deliver-payload.ts
+2 -1
View File
@@ -1,5 +1,6 @@
// Normalizes payloads and applies post-send presentation/media effects.
import type { ReplyPayload } from "../../auto-reply/types.js";
import { resolveReceiptSourceId } from "../../channels/message/receipt.js";
import { adaptMessagePresentationForChannel } from "../../channels/plugins/outbound/interactive.js";
import type { ChannelOutboundTargetRef } from "../../channels/plugins/types.adapters.js";
import {
@@ -175,7 +176,7 @@ export function buildPayloadSummary(payload: ReplyPayload): NormalizedOutboundPa
}
export function hasDeliveryResultIdentity(result: OutboundDeliveryResult): boolean {
return Boolean(result.messageId || result.toJid || result.pollId);
return resolveReceiptSourceId(result) !== undefined;
}
function normalizeDeliveryPin(payload: ReplyPayload): ReplyPayloadDeliveryPin | undefined {
src/infra/outbound/deliver.queue-integration.test.ts
+2 -4
View File
@@ -622,11 +622,9 @@ describe("deliverOutboundPayloads queue integration: mid-batch failure with send
expect(sendMatrix).toHaveBeenCalledTimes(2);
});
it("never acknowledges target-only routing metadata as a platform message identity", async () => {
it("never acknowledges route-only metadata as a platform message identity", async () => {
process.env.OPENCLAW_STATE_DIR = tmpDir;
const sendMatrix = vi.fn().mockResolvedValue({
target: { kind: "room", id: "!route-only:example" },
});
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "", toJid: "!route-only:example" });
const deliveryIntentId = "cron-direct-delivery:v1:no-platform-identity";
const params = {
cfg: {} as OpenClawConfig,