mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(feishu): comprehensive reply mechanism fix — outbound replyToId forwarding + topic-aware reply targeting
- Forward replyToId from ChannelOutboundContext through sendText/sendMedia to sendMessageFeishu/sendMarkdownCardFeishu/sendMediaFeishu, enabling reply-to-message via the message tool. - Fix group reply targeting: use ctx.messageId (triggering message) in normal groups to prevent silent topic thread creation (#32980). Preserve ctx.rootId targeting for topic-mode groups (group_topic/group_topic_sender) and groups with explicit replyInThread config. - Add regression tests for both fixes. Fixes #32980 Fixes #32958 Related #19784
This commit is contained in:
1
changelog/fragments/pr-feishu-reply-mechanism.md
Normal file
1
changelog/fragments/pr-feishu-reply-mechanism.md
Normal file
@@ -0,0 +1 @@
|
||||
- Feishu/reply mechanism: forward `replyToId` through outbound adapter `sendText`/`sendMedia` to enable reply-to-message via the `message` tool; fix group reply targeting to use the triggering message in normal groups while preserving topic-root replies for topic-mode groups (#32980, #32958). Thanks @guoqunabc.
|
||||
@@ -1517,6 +1517,82 @@ describe("handleFeishuMessage command authorization", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("replies to triggering message in normal group even when root_id is present (#32980)", async () => {
|
||||
mockShouldComputeCommandAuthorized.mockReturnValue(false);
|
||||
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
feishu: {
|
||||
groups: {
|
||||
"oc-group": {
|
||||
requireMention: false,
|
||||
groupSessionScope: "group",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} as ClawdbotConfig;
|
||||
|
||||
const event: FeishuMessageEvent = {
|
||||
sender: { sender_id: { open_id: "ou-normal-user" } },
|
||||
message: {
|
||||
message_id: "om_quote_reply",
|
||||
root_id: "om_original_msg",
|
||||
chat_id: "oc-group",
|
||||
chat_type: "group",
|
||||
message_type: "text",
|
||||
content: JSON.stringify({ text: "hello in normal group" }),
|
||||
},
|
||||
};
|
||||
|
||||
await dispatchMessage({ cfg, event });
|
||||
|
||||
expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
replyToMessageId: "om_quote_reply",
|
||||
rootId: "om_original_msg",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("replies to topic root in topic-mode group with root_id", async () => {
|
||||
mockShouldComputeCommandAuthorized.mockReturnValue(false);
|
||||
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
feishu: {
|
||||
groups: {
|
||||
"oc-group": {
|
||||
requireMention: false,
|
||||
groupSessionScope: "group_topic",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} as ClawdbotConfig;
|
||||
|
||||
const event: FeishuMessageEvent = {
|
||||
sender: { sender_id: { open_id: "ou-topic-user" } },
|
||||
message: {
|
||||
message_id: "om_topic_reply",
|
||||
root_id: "om_topic_root",
|
||||
chat_id: "oc-group",
|
||||
chat_type: "group",
|
||||
message_type: "text",
|
||||
content: JSON.stringify({ text: "hello in topic group" }),
|
||||
},
|
||||
};
|
||||
|
||||
await dispatchMessage({ cfg, event });
|
||||
|
||||
expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
replyToMessageId: "om_topic_root",
|
||||
rootId: "om_topic_root",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("forces thread replies when inbound message contains thread_id", async () => {
|
||||
mockShouldComputeCommandAuthorized.mockReturnValue(false);
|
||||
|
||||
|
||||
@@ -1337,7 +1337,23 @@ export async function handleFeishuMessage(params: {
|
||||
const messageCreateTimeMs = event.message.create_time
|
||||
? parseInt(event.message.create_time, 10)
|
||||
: undefined;
|
||||
const replyTargetMessageId = ctx.rootId ?? ctx.messageId;
|
||||
// Determine reply target based on group session mode:
|
||||
// - Topic-mode groups (group_topic / group_topic_sender): reply to the topic
|
||||
// root so the bot stays in the same thread.
|
||||
// - Groups with explicit replyInThread config: reply to the root so the bot
|
||||
// stays in the thread the user expects.
|
||||
// - Normal groups (auto-detected threadReply from root_id): reply to the
|
||||
// triggering message itself. Using rootId here would silently push the
|
||||
// reply into a topic thread invisible in the main chat view (#32980).
|
||||
const isTopicSession =
|
||||
isGroup &&
|
||||
(groupSession?.groupSessionScope === "group_topic" ||
|
||||
groupSession?.groupSessionScope === "group_topic_sender");
|
||||
const configReplyInThread =
|
||||
isGroup &&
|
||||
(groupConfig?.replyInThread ?? feishuCfg?.replyInThread ?? "disabled") === "enabled";
|
||||
const replyTargetMessageId =
|
||||
isTopicSession || configReplyInThread ? (ctx.rootId ?? ctx.messageId) : ctx.messageId;
|
||||
const threadReply = isGroup ? (groupSession?.threadReply ?? false) : false;
|
||||
|
||||
if (broadcastAgents) {
|
||||
|
||||
@@ -138,6 +138,117 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("feishuOutbound.sendText replyToId forwarding", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
|
||||
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
|
||||
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
|
||||
});
|
||||
|
||||
it("forwards replyToId as replyToMessageId to sendMessageFeishu", async () => {
|
||||
await sendText({
|
||||
cfg: {} as any,
|
||||
to: "chat_1",
|
||||
text: "hello",
|
||||
replyToId: "om_reply_target",
|
||||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
to: "chat_1",
|
||||
text: "hello",
|
||||
replyToMessageId: "om_reply_target",
|
||||
accountId: "main",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("forwards replyToId to sendMarkdownCardFeishu when renderMode=card", async () => {
|
||||
await sendText({
|
||||
cfg: {
|
||||
channels: {
|
||||
feishu: {
|
||||
renderMode: "card",
|
||||
},
|
||||
},
|
||||
} as any,
|
||||
to: "chat_1",
|
||||
text: "```code```",
|
||||
replyToId: "om_reply_target",
|
||||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
replyToMessageId: "om_reply_target",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not pass replyToMessageId when replyToId is absent", async () => {
|
||||
await sendText({
|
||||
cfg: {} as any,
|
||||
to: "chat_1",
|
||||
text: "hello",
|
||||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
to: "chat_1",
|
||||
text: "hello",
|
||||
accountId: "main",
|
||||
}),
|
||||
);
|
||||
expect(sendMessageFeishuMock.mock.calls[0][0].replyToMessageId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("feishuOutbound.sendMedia replyToId forwarding", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
|
||||
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
|
||||
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
|
||||
});
|
||||
|
||||
it("forwards replyToId to sendMediaFeishu", async () => {
|
||||
await feishuOutbound.sendMedia?.({
|
||||
cfg: {} as any,
|
||||
to: "chat_1",
|
||||
text: "",
|
||||
mediaUrl: "https://example.com/image.png",
|
||||
replyToId: "om_reply_target",
|
||||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMediaFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
replyToMessageId: "om_reply_target",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("forwards replyToId to text caption send", async () => {
|
||||
await feishuOutbound.sendMedia?.({
|
||||
cfg: {} as any,
|
||||
to: "chat_1",
|
||||
text: "caption text",
|
||||
mediaUrl: "https://example.com/image.png",
|
||||
replyToId: "om_reply_target",
|
||||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
replyToMessageId: "om_reply_target",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("feishuOutbound.sendMedia renderMode", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -47,17 +47,18 @@ async function sendOutboundText(params: {
|
||||
cfg: Parameters<typeof sendMessageFeishu>[0]["cfg"];
|
||||
to: string;
|
||||
text: string;
|
||||
replyToMessageId?: string;
|
||||
accountId?: string;
|
||||
}) {
|
||||
const { cfg, to, text, accountId } = params;
|
||||
const { cfg, to, text, replyToMessageId, accountId } = params;
|
||||
const account = resolveFeishuAccount({ cfg, accountId });
|
||||
const renderMode = account.config?.renderMode ?? "auto";
|
||||
|
||||
if (renderMode === "card" || (renderMode === "auto" && shouldUseCard(text))) {
|
||||
return sendMarkdownCardFeishu({ cfg, to, text, accountId });
|
||||
return sendMarkdownCardFeishu({ cfg, to, text, replyToMessageId, accountId });
|
||||
}
|
||||
|
||||
return sendMessageFeishu({ cfg, to, text, accountId });
|
||||
return sendMessageFeishu({ cfg, to, text, replyToMessageId, accountId });
|
||||
}
|
||||
|
||||
export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
@@ -65,7 +66,8 @@ export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
chunker: (text, limit) => getFeishuRuntime().channel.text.chunkMarkdownText(text, limit),
|
||||
chunkerMode: "markdown",
|
||||
textChunkLimit: 4000,
|
||||
sendText: async ({ cfg, to, text, accountId }) => {
|
||||
sendText: async ({ cfg, to, text, accountId, replyToId }) => {
|
||||
const replyToMessageId = replyToId ?? undefined;
|
||||
// Scheme A compatibility shim:
|
||||
// when upstream accidentally returns a local image path as plain text,
|
||||
// auto-upload and send as Feishu image message instead of leaking path text.
|
||||
@@ -76,6 +78,7 @@ export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
cfg,
|
||||
to,
|
||||
mediaUrl: localImagePath,
|
||||
replyToMessageId,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
return { channel: "feishu", ...result };
|
||||
@@ -89,17 +92,20 @@ export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
cfg,
|
||||
to,
|
||||
text,
|
||||
replyToMessageId,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
return { channel: "feishu", ...result };
|
||||
},
|
||||
sendMedia: async ({ cfg, to, text, mediaUrl, accountId, mediaLocalRoots }) => {
|
||||
sendMedia: async ({ cfg, to, text, mediaUrl, accountId, replyToId, mediaLocalRoots }) => {
|
||||
const replyToMessageId = replyToId ?? undefined;
|
||||
// Send text first if provided
|
||||
if (text?.trim()) {
|
||||
await sendOutboundText({
|
||||
cfg,
|
||||
to,
|
||||
text,
|
||||
replyToMessageId,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
}
|
||||
@@ -111,6 +117,7 @@ export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
cfg,
|
||||
to,
|
||||
mediaUrl,
|
||||
replyToMessageId,
|
||||
accountId: accountId ?? undefined,
|
||||
mediaLocalRoots,
|
||||
});
|
||||
@@ -124,6 +131,7 @@ export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
cfg,
|
||||
to,
|
||||
text: fallbackText,
|
||||
replyToMessageId,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
return { channel: "feishu", ...result };
|
||||
@@ -135,6 +143,7 @@ export const feishuOutbound: ChannelOutboundAdapter = {
|
||||
cfg,
|
||||
to,
|
||||
text: text ?? "",
|
||||
replyToMessageId,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
return { channel: "feishu", ...result };
|
||||
|
||||
Reference in New Issue
Block a user