Skip to content

fix(feishu): send normal group replies top-level #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
de1tydev wants to merge 1 commit into
base: review-base/before-feishu-top-level-reply-20260624
Choose a base branch
from
Draft

fix(feishu): send normal group replies top-level #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 176 additions & 7 deletions extensions/feishu/src/bot.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2604,6 +2604,134 @@ describe("handleFeishuMessage command authorization", () => {
undefined,
);
});

it("downloads embedded image and media tags from quoted post messages into the agent context", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);
const quotedRawContent = JSON.stringify({
title: "Rich quote",
content: [
[
{ tag: "text", text: "see attachments " },
{ tag: "img", image_key: "img_quoted_payload" },
{
tag: "media",
file_key: "file_quoted_media_payload",
file_name: "quoted-clip.mp4",
},
],
],
});
mockGetMessageFeishu.mockResolvedValueOnce({
messageId: "om_parent_post",
chatId: "oc-group",
chatType: "group",
senderId: "ou-post-sender",
senderType: "user",
content: "Rich quote\n\nsee attachments ![image][media]",
rawContent: quotedRawContent,
contentType: "post",
});
mockDownloadMessageResourceFeishu
.mockResolvedValueOnce({
buffer: Buffer.from("quoted-image"),
contentType: "image/png",
fileName: "quoted-image.png",
})
.mockResolvedValueOnce({
buffer: Buffer.from("quoted-media"),
contentType: "video/mp4",
fileName: "quoted-clip.mp4",
});
mockSaveMediaBuffer
.mockResolvedValueOnce({
id: "quoted-image.png",
path: "/tmp/quoted-image.png",
size: Buffer.byteLength("quoted-image"),
contentType: "image/png",
})
.mockResolvedValueOnce({
id: "quoted-clip.mp4",
path: "/tmp/quoted-clip.mp4",
size: Buffer.byteLength("quoted-media"),
contentType: "video/mp4",
});

const cfg: ClawdbotConfig = {
channels: {
feishu: {
groupPolicy: "open",
requireMention: false,
},
},
} as ClawdbotConfig;

const event: FeishuMessageEvent = {
sender: {
sender_id: {
open_id: "ou-replier",
},
},
message: {
message_id: "om_reply_to_post",
parent_id: "om_parent_post",
chat_id: "oc-group",
chat_type: "group",
message_type: "text",
content: JSON.stringify({ text: "please inspect these attachments" }),
},
};

await dispatchMessage({ cfg, event });

expect(mockDownloadMessageResourceFeishu).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
messageId: "om_parent_post",
fileKey: "img_quoted_payload",
type: "image",
}),
);
expect(mockDownloadMessageResourceFeishu).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
messageId: "om_parent_post",
fileKey: "file_quoted_media_payload",
type: "file",
originalFilename: "quoted-clip.mp4",
}),
);
expect(mockSaveMediaBuffer).toHaveBeenNthCalledWith(
1,
expect.any(Buffer),
"image/png",
"inbound",
expect.any(Number),
"quoted-image.png",
);
expect(mockSaveMediaBuffer).toHaveBeenNthCalledWith(
2,
expect.any(Buffer),
"video/mp4",
"inbound",
expect.any(Number),
"quoted-clip.mp4",
);
expect(mockFinalizeInboundContext).toHaveBeenCalledWith(
expect.objectContaining({
SupplementalContext: expect.objectContaining({
quote: expect.objectContaining({
body: "Rich quote\n\nsee attachments ![image][media]",
id: "om_parent_post",
}),
}),
MediaPath: "/tmp/quoted-image.png",
MediaPaths: ["/tmp/quoted-image.png", "/tmp/quoted-clip.mp4"],
MediaTypes: ["image/png", "video/mp4"],
}),
undefined,
);
});

it("includes message_id in BodyForAgent on its own line", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);

Expand Down Expand Up @@ -3361,7 +3489,7 @@ describe("handleFeishuMessage command authorization", () => {
expect(dispatcherOptions.rootId).toBe("om_root_topic");
});

it("replies to the triggering message in normal groups even when root_id is present (#32980)", async () => {
it("sends top-level replies in normal groups even when root_id quote metadata is present", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);

const cfg: ClawdbotConfig = {
Expand Down Expand Up @@ -3396,12 +3524,12 @@ describe("handleFeishuMessage command authorization", () => {
rootId?: string;
skipReplyToInMessages?: boolean;
}>(mockCreateFeishuReplyDispatcher, 0, 0);
expect(dispatcherOptions.replyToMessageId).toBe("om_quote_reply");
expect(dispatcherOptions.replyToMessageId).toBeUndefined();
expect(dispatcherOptions.skipReplyToInMessages).toBe(false);
expect(dispatcherOptions.rootId).toBe("om_original_msg");
});

it("replies to the triggering quoted command message in normal groups", async () => {
it("sends quoted command responses as top-level messages in normal groups", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);

const cfg: ClawdbotConfig = {
Expand Down Expand Up @@ -3440,12 +3568,53 @@ describe("handleFeishuMessage command authorization", () => {
skipReplyToInMessages?: boolean;
replyInThread?: boolean;
}>(mockCreateFeishuReplyDispatcher, 0, 0);
expect(dispatcherOptions.replyToMessageId).toBe("om_current_quoted_command");
expect(dispatcherOptions.replyToMessageId).toBeUndefined();
expect(dispatcherOptions.skipReplyToInMessages).toBe(false);
expect(dispatcherOptions.replyInThread).toBe(false);
expect(dispatcherOptions.rootId).toBe("om_quoted_root");
});

it("keeps synthetic card action reply targets in normal groups", 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-card-user" } },
message: {
message_id: "card-action-token",
reply_target_message_id: "om_card_message",
synthetic_card_action: true,
chat_id: "oc-group",
chat_type: "group",
message_type: "text",
content: JSON.stringify({ text: "/approve plugin:test allow-once" }),
},
};

await dispatchMessage({ cfg, event });

const dispatcherOptions = mockCallArg<{
replyToMessageId?: string;
replyInThread?: boolean;
threadReply?: boolean;
}>(mockCreateFeishuReplyDispatcher, 0, 0);
expect(dispatcherOptions.replyToMessageId).toBe("om_card_message");
expect(dispatcherOptions.replyInThread).toBe(false);
expect(dispatcherOptions.threadReply).toBe(false);
});

it("replies to topic root in topic-mode group with root_id", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);

Expand Down Expand Up @@ -3597,7 +3766,7 @@ describe("handleFeishuMessage command authorization", () => {
);
});

it("does not force thread replies when inbound message contains thread_id but thread replies are disabled", async () => {
it("preserves Feishu thread replies when inbound message contains thread_id", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);

const cfg: ClawdbotConfig = {
Expand Down Expand Up @@ -3636,8 +3805,8 @@ describe("handleFeishuMessage command authorization", () => {
}>(mockCreateFeishuReplyDispatcher, 0, 0);
expect(dispatcherOptions.replyToMessageId).toBe("msg-thread-reply");
expect(dispatcherOptions.skipReplyToInMessages).toBe(false);
expect(dispatcherOptions.replyInThread).toBe(false);
expect(dispatcherOptions.threadReply).toBe(false);
expect(dispatcherOptions.replyInThread).toBe(true);
expect(dispatcherOptions.threadReply).toBe(true);
});

it("bootstraps topic thread context only for a new thread session", async () => {
Expand Down
42 changes: 20 additions & 22 deletions extensions/feishu/src/bot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,27 +1417,28 @@ export async function handleFeishuMessage(params: {
// 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: reply to the triggering message itself. Quoted-message
// metadata (reply_target_message_id/root_id/parent_id) is inbound context,
// not the outbound reply anchor; otherwise commands sent with a quote can
// be routed back into the quoted message's thread.
// - Real Feishu topic events and explicit replyInThread config: reply via
// message.reply so the bot stays in the thread the user expects.
// - Normal groups: send top-level messages. Quote metadata
// (reply_target_message_id/root_id/parent_id) is inbound context only.
const isTopicSession =
isGroup &&
(groupSession?.groupSessionScope === "group_topic" ||
groupSession?.groupSessionScope === "group_topic_sender");
const isInboundFeishuThread =
isGroup && (Boolean(ctx.threadId?.trim()) || ctx.chatType === "topic_group");
const configReplyInThread =
isGroup &&
(groupConfig?.replyInThread ?? feishuCfg?.replyInThread ?? "disabled") === "enabled";
const shouldReplyInFeishuThread = isTopicSession || configReplyInThread;
const shouldReplyInFeishuThread =
isTopicSession || isInboundFeishuThread || configReplyInThread;
const topicReplyTargetMessageId =
ctx.rootId ?? ctx.replyTargetMessageId ?? (ctx.suppressReplyTarget ? undefined : ctx.messageId);
ctx.rootId ??
ctx.replyTargetMessageId ??
(ctx.suppressReplyTarget ? undefined : ctx.messageId);
const normalGroupReplyTargetMessageId = ctx.syntheticCardAction
? ctx.replyTargetMessageId
: ctx.suppressReplyTarget
? undefined
: ctx.messageId;
: undefined;
const replyTargetMessageId = directThreadReply
? directThreadReplyTargetMessageId
: shouldReplyInFeishuThread
Expand All @@ -1446,9 +1447,14 @@ export async function handleFeishuMessage(params: {
? normalGroupReplyTargetMessageId
: defaultReplyTargetMessageId;
const skipReplyToInMessages = !isGroup && !directThreadReply;
const dispatchReplyInThread = directThreadReply
? true
: shouldReplyInFeishuThread
? isInboundFeishuThread || replyInThread
: false;
const threadReply = isGroup
? shouldReplyInFeishuThread
? (groupSession?.threadReply ?? false)
? isInboundFeishuThread || (groupSession?.threadReply ?? false)
: false
: directThreadReply;
const lastRouteThreadId =
Expand Down Expand Up @@ -1574,11 +1580,7 @@ export async function handleFeishuMessage(params: {
allowReasoningPreview,
replyToMessageId: replyTargetMessageId,
skipReplyToInMessages,
replyInThread: directThreadReply
? true
: shouldReplyInFeishuThread
? replyInThread
: false,
replyInThread: dispatchReplyInThread,
rootId: ctx.rootId,
threadReply,
accountId: account.accountId,
Expand Down Expand Up @@ -1754,11 +1756,7 @@ export async function handleFeishuMessage(params: {
allowReasoningPreview,
replyToMessageId: replyTargetMessageId,
skipReplyToInMessages,
replyInThread: directThreadReply
? true
: shouldReplyInFeishuThread
? replyInThread
: false,
replyInThread: dispatchReplyInThread,
rootId: ctx.rootId,
threadReply,
accountId: account.accountId,
Expand Down
2 changes: 2 additions & 0 deletions extensions/feishu/src/send.reply-fallback.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => {
content: '{"zh_cn":{"content":[[{"tag":"md","text":"hello"}]]}}',
msg_type: "post",
reply_in_thread: true,
uuid: expect.any(String),
},
});
expect(createMock).toHaveBeenCalledWith({
Expand All @@ -261,6 +262,7 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => {
content: '{"zh_cn":{"content":[[{"tag":"md","text":"hello"}]]}}',
receive_id: "oc_group_1",
msg_type: "post",
uuid: expect.any(String),
},
});
});
Expand Down
Loading