Skip to content

Commit 63ce7c7

Browse files
guoqunabcJimmy-xuzimoMunemHashmibmendonca3Takhoffman
authored
fix(feishu): comprehensive reply mechanism — outbound replyToId forwarding + topic-aware reply targeting (#33789)
* 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 * fix: normalize Feishu delivery.to before comparing with messaging tool targets - Add normalizeDeliveryTarget helper to strip user:/chat: prefixes for Feishu - Apply normalization in matchesMessagingToolDeliveryTarget before comparison - This ensures cron duplicate suppression works when session uses prefixed targets (user:ou_xxx) but messaging tool extract uses normalized bare IDs (ou_xxx) Fixes review comment on PR #32755 (cherry picked from commit fc20106) * fix(feishu): catch thrown SDK errors for withdrawn reply targets The Feishu Lark SDK can throw exceptions (SDK errors with .code or AxiosErrors with .response.data.code) for withdrawn/deleted reply targets, in addition to returning error codes in the response object. Wrap reply calls in sendMessageFeishu and sendCardFeishu with try-catch to handle thrown withdrawn/not-found errors (230011, 231003) and fall back to client.im.message.create, matching the existing response-level fallback behavior. Also extract sendFallbackDirect helper to deduplicate the direct-send fallback block across both functions. Closes #33496 (cherry picked from commit ad0901a) * feishu: forward outbound reply target context (cherry picked from commit c129a691fcf552a1cebe1e8a22ea8611ffc3b377) * feishu extension: tighten reply target fallback semantics (cherry picked from commit f85ec610f267020b66713c09e648ec004b2e26f1) * fix(feishu): align synthesized fallback typing and changelog attribution * test(feishu): cover group_topic_sender reply targeting --------- Co-authored-by: Xu Zimo <[email protected]> Co-authored-by: Munem Hashmi <[email protected]> Co-authored-by: bmendonca3 <[email protected]> Co-authored-by: Tak Hoffman <[email protected]>
1 parent 432e022 commit 63ce7c7

8 files changed

Lines changed: 529 additions & 59 deletions

File tree

‎changelog/fragments/pr-feishu-reply-mechanism.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Feishu reply routing now uses one canonical reply-target path across inbound and outbound flows: normal groups reply to the triggering message while topic-mode groups stay on topic roots, outbound sends preserve `replyToId`/`threadId`, withdrawn reply targets fall back to direct sends, and cron duplicate suppression normalizes Feishu/Lark target IDs consistently (#32980, #32958, #33572, #33526; #33789, #33575, #33515, #33161). Thanks @guoqunabc, @bmendonca3, @MunemHashmi, and @Jimmy-xuzimo.

‎extensions/feishu/src/bot.test.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,6 +1517,120 @@ describe("handleFeishuMessage command authorization", () => {
15171517
);
15181518
});
15191519

1520+
it("replies to triggering message in normal group even when root_id is present (#32980)", async () => {
1521+
mockShouldComputeCommandAuthorized.mockReturnValue(false);
1522+
1523+
const cfg: ClawdbotConfig = {
1524+
channels: {
1525+
feishu: {
1526+
groups: {
1527+
"oc-group": {
1528+
requireMention: false,
1529+
groupSessionScope: "group",
1530+
},
1531+
},
1532+
},
1533+
},
1534+
} as ClawdbotConfig;
1535+
1536+
const event: FeishuMessageEvent = {
1537+
sender: { sender_id: { open_id: "ou-normal-user" } },
1538+
message: {
1539+
message_id: "om_quote_reply",
1540+
root_id: "om_original_msg",
1541+
chat_id: "oc-group",
1542+
chat_type: "group",
1543+
message_type: "text",
1544+
content: JSON.stringify({ text: "hello in normal group" }),
1545+
},
1546+
};
1547+
1548+
await dispatchMessage({ cfg, event });
1549+
1550+
expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith(
1551+
expect.objectContaining({
1552+
replyToMessageId: "om_quote_reply",
1553+
rootId: "om_original_msg",
1554+
}),
1555+
);
1556+
});
1557+
1558+
it("replies to topic root in topic-mode group with root_id", async () => {
1559+
mockShouldComputeCommandAuthorized.mockReturnValue(false);
1560+
1561+
const cfg: ClawdbotConfig = {
1562+
channels: {
1563+
feishu: {
1564+
groups: {
1565+
"oc-group": {
1566+
requireMention: false,
1567+
groupSessionScope: "group_topic",
1568+
},
1569+
},
1570+
},
1571+
},
1572+
} as ClawdbotConfig;
1573+
1574+
const event: FeishuMessageEvent = {
1575+
sender: { sender_id: { open_id: "ou-topic-user" } },
1576+
message: {
1577+
message_id: "om_topic_reply",
1578+
root_id: "om_topic_root",
1579+
chat_id: "oc-group",
1580+
chat_type: "group",
1581+
message_type: "text",
1582+
content: JSON.stringify({ text: "hello in topic group" }),
1583+
},
1584+
};
1585+
1586+
await dispatchMessage({ cfg, event });
1587+
1588+
expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith(
1589+
expect.objectContaining({
1590+
replyToMessageId: "om_topic_root",
1591+
rootId: "om_topic_root",
1592+
}),
1593+
);
1594+
});
1595+
1596+
it("replies to topic root in topic-sender group with root_id", async () => {
1597+
mockShouldComputeCommandAuthorized.mockReturnValue(false);
1598+
1599+
const cfg: ClawdbotConfig = {
1600+
channels: {
1601+
feishu: {
1602+
groups: {
1603+
"oc-group": {
1604+
requireMention: false,
1605+
groupSessionScope: "group_topic_sender",
1606+
},
1607+
},
1608+
},
1609+
},
1610+
} as ClawdbotConfig;
1611+
1612+
const event: FeishuMessageEvent = {
1613+
sender: { sender_id: { open_id: "ou-topic-sender-user" } },
1614+
message: {
1615+
message_id: "om_topic_sender_reply",
1616+
root_id: "om_topic_sender_root",
1617+
chat_id: "oc-group",
1618+
chat_type: "group",
1619+
message_type: "text",
1620+
content: JSON.stringify({ text: "hello in topic sender group" }),
1621+
},
1622+
};
1623+
1624+
await dispatchMessage({ cfg, event });
1625+
1626+
expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith(
1627+
expect.objectContaining({
1628+
replyToMessageId: "om_topic_sender_root",
1629+
rootId: "om_topic_sender_root",
1630+
}),
1631+
);
1632+
});
1633+
15201634
it("forces thread replies when inbound message contains thread_id", async () => {
15211635
mockShouldComputeCommandAuthorized.mockReturnValue(false);
15221636

‎extensions/feishu/src/bot.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1337,7 +1337,23 @@ export async function handleFeishuMessage(params: {
13371337
const messageCreateTimeMs = event.message.create_time
13381338
? parseInt(event.message.create_time, 10)
13391339
: undefined;
1340-
const replyTargetMessageId = ctx.rootId ?? ctx.messageId;
1340+
// Determine reply target based on group session mode:
1341+
// - Topic-mode groups (group_topic / group_topic_sender): reply to the topic
1342+
// root so the bot stays in the same thread.
1343+
// - Groups with explicit replyInThread config: reply to the root so the bot
1344+
// stays in the thread the user expects.
1345+
// - Normal groups (auto-detected threadReply from root_id): reply to the
1346+
// triggering message itself. Using rootId here would silently push the
1347+
// reply into a topic thread invisible in the main chat view (#32980).
1348+
const isTopicSession =
1349+
isGroup &&
1350+
(groupSession?.groupSessionScope === "group_topic" ||
1351+
groupSession?.groupSessionScope === "group_topic_sender");
1352+
const configReplyInThread =
1353+
isGroup &&
1354+
(groupConfig?.replyInThread ?? feishuCfg?.replyInThread ?? "disabled") === "enabled";
1355+
const replyTargetMessageId =
1356+
isTopicSession || configReplyInThread ? (ctx.rootId ?? ctx.messageId) : ctx.messageId;
13411357
const threadReply = isGroup ? (groupSession?.threadReply ?? false) : false;
13421358

13431359
if (broadcastAgents) {

‎extensions/feishu/src/outbound.test.ts‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,156 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
136136
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
137137
expect(result).toEqual(expect.objectContaining({ channel: "feishu", messageId: "card_msg" }));
138138
});
139+
140+
it("forwards replyToId as replyToMessageId on sendText", async () => {
141+
await sendText({
142+
cfg: {} as any,
143+
to: "chat_1",
144+
text: "hello",
145+
replyToId: "om_reply_1",
146+
accountId: "main",
147+
} as any);
148+
149+
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
150+
expect.objectContaining({
151+
to: "chat_1",
152+
text: "hello",
153+
replyToMessageId: "om_reply_1",
154+
accountId: "main",
155+
}),
156+
);
157+
});
158+
159+
it("falls back to threadId when replyToId is empty on sendText", async () => {
160+
await sendText({
161+
cfg: {} as any,
162+
to: "chat_1",
163+
text: "hello",
164+
replyToId: " ",
165+
threadId: "om_thread_2",
166+
accountId: "main",
167+
} as any);
168+
169+
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
170+
expect.objectContaining({
171+
to: "chat_1",
172+
text: "hello",
173+
replyToMessageId: "om_thread_2",
174+
accountId: "main",
175+
}),
176+
);
177+
});
178+
});
179+
180+
describe("feishuOutbound.sendText replyToId forwarding", () => {
181+
beforeEach(() => {
182+
vi.clearAllMocks();
183+
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
184+
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
185+
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
186+
});
187+
188+
it("forwards replyToId as replyToMessageId to sendMessageFeishu", async () => {
189+
await sendText({
190+
cfg: {} as any,
191+
to: "chat_1",
192+
text: "hello",
193+
replyToId: "om_reply_target",
194+
accountId: "main",
195+
});
196+
197+
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
198+
expect.objectContaining({
199+
to: "chat_1",
200+
text: "hello",
201+
replyToMessageId: "om_reply_target",
202+
accountId: "main",
203+
}),
204+
);
205+
});
206+
207+
it("forwards replyToId to sendMarkdownCardFeishu when renderMode=card", async () => {
208+
await sendText({
209+
cfg: {
210+
channels: {
211+
feishu: {
212+
renderMode: "card",
213+
},
214+
},
215+
} as any,
216+
to: "chat_1",
217+
text: "```code```",
218+
replyToId: "om_reply_target",
219+
accountId: "main",
220+
});
221+
222+
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
223+
expect.objectContaining({
224+
replyToMessageId: "om_reply_target",
225+
}),
226+
);
227+
});
228+
229+
it("does not pass replyToMessageId when replyToId is absent", async () => {
230+
await sendText({
231+
cfg: {} as any,
232+
to: "chat_1",
233+
text: "hello",
234+
accountId: "main",
235+
});
236+
237+
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
238+
expect.objectContaining({
239+
to: "chat_1",
240+
text: "hello",
241+
accountId: "main",
242+
}),
243+
);
244+
expect(sendMessageFeishuMock.mock.calls[0][0].replyToMessageId).toBeUndefined();
245+
});
246+
});
247+
248+
describe("feishuOutbound.sendMedia replyToId forwarding", () => {
249+
beforeEach(() => {
250+
vi.clearAllMocks();
251+
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
252+
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
253+
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
254+
});
255+
256+
it("forwards replyToId to sendMediaFeishu", async () => {
257+
await feishuOutbound.sendMedia?.({
258+
cfg: {} as any,
259+
to: "chat_1",
260+
text: "",
261+
mediaUrl: "https://example.com/image.png",
262+
replyToId: "om_reply_target",
263+
accountId: "main",
264+
});
265+
266+
expect(sendMediaFeishuMock).toHaveBeenCalledWith(
267+
expect.objectContaining({
268+
replyToMessageId: "om_reply_target",
269+
}),
270+
);
271+
});
272+
273+
it("forwards replyToId to text caption send", async () => {
274+
await feishuOutbound.sendMedia?.({
275+
cfg: {} as any,
276+
to: "chat_1",
277+
text: "caption text",
278+
mediaUrl: "https://example.com/image.png",
279+
replyToId: "om_reply_target",
280+
accountId: "main",
281+
});
282+
283+
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
284+
expect.objectContaining({
285+
replyToMessageId: "om_reply_target",
286+
}),
287+
);
288+
});
139289
});
140290

141291
describe("feishuOutbound.sendMedia renderMode", () => {
@@ -178,4 +328,32 @@ describe("feishuOutbound.sendMedia renderMode", () => {
178328
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
179329
expect(result).toEqual(expect.objectContaining({ channel: "feishu", messageId: "media_msg" }));
180330
});
331+
332+
it("uses threadId fallback as replyToMessageId on sendMedia", async () => {
333+
await feishuOutbound.sendMedia?.({
334+
cfg: {} as any,
335+
to: "chat_1",
336+
text: "caption",
337+
mediaUrl: "https://example.com/image.png",
338+
threadId: "om_thread_1",
339+
accountId: "main",
340+
} as any);
341+
342+
expect(sendMediaFeishuMock).toHaveBeenCalledWith(
343+
expect.objectContaining({
344+
to: "chat_1",
345+
mediaUrl: "https://example.com/image.png",
346+
replyToMessageId: "om_thread_1",
347+
accountId: "main",
348+
}),
349+
);
350+
expect(sendMessageFeishuMock).toHaveBeenCalledWith(
351+
expect.objectContaining({
352+
to: "chat_1",
353+
text: "caption",
354+
replyToMessageId: "om_thread_1",
355+
accountId: "main",
356+
}),
357+
);
358+
});
181359
});

0 commit comments

Comments
 (0)