From ce0ff42ff5eda74ca0ca798590b042955b7227ba Mon Sep 17 00:00:00 2001 From: wittam-01 Date: Thu, 2 Apr 2026 15:31:52 +0800 Subject: [PATCH] fix: harden Feishu comment-thread delivery (#59129) * fix: harden Feishu comment-thread delivery * fix: harden Feishu comment-thread delivery (#59129) (thanks @wittam-01) --------- Co-authored-by: George Zhang --- CHANGELOG.md | 1 + extensions/feishu/src/comment-dispatcher.ts | 6 +- extensions/feishu/src/comment-handler.test.ts | 57 +- extensions/feishu/src/comment-handler.ts | 6 +- extensions/feishu/src/drive-schema.ts | 16 +- extensions/feishu/src/drive.test.ts | 832 +++++++++++++++++- extensions/feishu/src/drive.ts | 404 ++++++++- extensions/feishu/src/monitor.comment.test.ts | 188 +++- extensions/feishu/src/monitor.comment.ts | 160 +++- 9 files changed, 1559 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a26ea90640..a35c24d80fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai - Providers/media HTTP: centralize base URL normalization, default auth/header injection, and explicit header override handling across shared OpenAI-compatible audio, Deepgram audio, Gemini media/image, and Moonshot video request paths. (#59469) Thanks @vincentkoc. - Exec approvals/doctor: report host policy sources from the real approvals file path and ignore malformed host override values when attributing effective policy conflicts. (#59367) Thanks @gumadeiras. - Matrix/onboarding: restore guided setup in `openclaw channels add` and `openclaw configure --section channels`, while keeping custom plugin wizards on the shared `setupWizard` seam. (#59462) Thanks @gumadeiras. +- Feishu/comment threads: harden document comment-thread delivery so whole-document comments fall back to `add_comment`, delayed reply lookups retry more reliably, and user-visible replies avoid reasoning/planning spillover. (#59129) Thanks @wittam-01. ## 2026.4.1-beta.1 diff --git a/extensions/feishu/src/comment-dispatcher.ts b/extensions/feishu/src/comment-dispatcher.ts index a06fafcd49f..9365cac83bf 100644 --- a/extensions/feishu/src/comment-dispatcher.ts +++ b/extensions/feishu/src/comment-dispatcher.ts @@ -8,7 +8,7 @@ import { import { resolveFeishuRuntimeAccount } from "./accounts.js"; import { createFeishuClient } from "./client.js"; import type { CommentFileType } from "./comment-target.js"; -import { replyComment } from "./drive.js"; +import { deliverCommentThreadText } from "./drive.js"; import { getFeishuRuntime } from "./runtime.js"; export type CreateFeishuCommentReplyDispatcherParams = { @@ -19,6 +19,7 @@ export type CreateFeishuCommentReplyDispatcherParams = { fileToken: string; fileType: CommentFileType; commentId: string; + isWholeComment?: boolean; }; export function createFeishuCommentReplyDispatcher( @@ -63,11 +64,12 @@ export function createFeishuCommentReplyDispatcher( } const chunks = core.channel.text.chunkTextWithMode(reply.text, textChunkLimit, chunkMode); for (const chunk of chunks) { - await replyComment(client, { + await deliverCommentThreadText(client, { file_token: params.fileToken, file_type: params.fileType, comment_id: params.commentId, content: chunk, + is_whole_comment: params.isWholeComment, }); } }, diff --git a/extensions/feishu/src/comment-handler.test.ts b/extensions/feishu/src/comment-handler.test.ts index 946430db31b..3a99bb8458c 100644 --- a/extensions/feishu/src/comment-handler.test.ts +++ b/extensions/feishu/src/comment-handler.test.ts @@ -8,7 +8,7 @@ const resolveDriveCommentEventTurnMock = vi.hoisted(() => vi.fn()); const createFeishuCommentReplyDispatcherMock = vi.hoisted(() => vi.fn()); const maybeCreateDynamicAgentMock = vi.hoisted(() => vi.fn()); const createFeishuClientMock = vi.hoisted(() => vi.fn(() => ({ request: vi.fn() }))); -const replyCommentMock = vi.hoisted(() => vi.fn()); +const deliverCommentThreadTextMock = vi.hoisted(() => vi.fn()); vi.mock("./monitor.comment.js", () => ({ resolveDriveCommentEventTurn: resolveDriveCommentEventTurnMock, @@ -27,7 +27,7 @@ vi.mock("./client.js", () => ({ })); vi.mock("./drive.js", () => ({ - replyComment: replyCommentMock, + deliverCommentThreadText: deliverCommentThreadTextMock, })); function buildConfig(overrides?: Partial): ClawdbotConfig { @@ -66,6 +66,7 @@ describe("handleFeishuCommentEvent", () => { noticeType: "add_comment", fileToken: "doc_token_1", fileType: "docx", + isWholeComment: false, senderId: "ou_sender", senderUserId: "on_sender_user", timestamp: "1774951528000", @@ -76,7 +77,10 @@ describe("handleFeishuCommentEvent", () => { rootCommentText: "root comment", targetReplyText: "latest reply", }); - replyCommentMock.mockResolvedValue({ reply_id: "r1" }); + deliverCommentThreadTextMock.mockResolvedValue({ + delivery_mode: "reply_comment", + reply_id: "r1", + }); const runtime = createPluginRuntimeMock({ channel: { @@ -196,7 +200,7 @@ describe("handleFeishuCommentEvent", () => { typeof vi.fn >; expect(dispatchReplyFromConfig).toHaveBeenCalledTimes(1); - expect(replyCommentMock).not.toHaveBeenCalled(); + expect(deliverCommentThreadTextMock).not.toHaveBeenCalled(); }); it("issues a pairing challenge in the comment thread when dmPolicy=pairing", async () => { @@ -232,12 +236,13 @@ describe("handleFeishuCommentEvent", () => { } as never, }); - expect(replyCommentMock).toHaveBeenCalledWith( + expect(deliverCommentThreadTextMock).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ file_token: "doc_token_1", file_type: "docx", comment_id: "comment_1", + is_whole_comment: false, }), ); const dispatchReplyFromConfig = runtime.channel.reply.dispatchReplyFromConfig as ReturnType< @@ -245,4 +250,46 @@ describe("handleFeishuCommentEvent", () => { >; expect(dispatchReplyFromConfig).not.toHaveBeenCalled(); }); + + it("passes whole-comment metadata to the comment reply dispatcher", async () => { + resolveDriveCommentEventTurnMock.mockResolvedValueOnce({ + eventId: "evt_whole", + messageId: "drive-comment:evt_whole", + commentId: "comment_whole", + replyId: "reply_whole", + noticeType: "add_reply", + fileToken: "doc_token_1", + fileType: "docx", + isWholeComment: true, + senderId: "ou_sender", + senderUserId: "on_sender_user", + timestamp: "1774951528000", + isMentioned: false, + documentTitle: "Project review", + prompt: "prompt body", + preview: "prompt body", + rootCommentText: "root comment", + targetReplyText: "reply text", + }); + + await handleFeishuCommentEvent({ + cfg: buildConfig(), + accountId: "default", + event: { event_id: "evt_whole" }, + botOpenId: "ou_bot", + runtime: { + log: vi.fn(), + error: vi.fn(), + } as never, + }); + + expect(createFeishuCommentReplyDispatcherMock).toHaveBeenCalledWith( + expect.objectContaining({ + commentId: "comment_whole", + fileToken: "doc_token_1", + fileType: "docx", + isWholeComment: true, + }), + ); + }); }); diff --git a/extensions/feishu/src/comment-handler.ts b/extensions/feishu/src/comment-handler.ts index f566c7875c5..f5cc8eadda6 100644 --- a/extensions/feishu/src/comment-handler.ts +++ b/extensions/feishu/src/comment-handler.ts @@ -8,7 +8,7 @@ import { resolveFeishuRuntimeAccount } from "./accounts.js"; import { createFeishuClient } from "./client.js"; import { createFeishuCommentReplyDispatcher } from "./comment-dispatcher.js"; import { buildFeishuCommentTarget } from "./comment-target.js"; -import { replyComment } from "./drive.js"; +import { deliverCommentThreadText } from "./drive.js"; import { maybeCreateDynamicAgent } from "./dynamic-agent.js"; import { resolveDriveCommentEventTurn, @@ -108,11 +108,12 @@ export async function handleFeishuCommentEvent( ); }, sendPairingReply: async (text) => { - await replyComment(client, { + await deliverCommentThreadText(client, { file_token: turn.fileToken, file_type: turn.fileType, comment_id: turn.commentId, content: text, + is_whole_comment: turn.isWholeComment, }); }, onReplyError: (err) => { @@ -221,6 +222,7 @@ export async function handleFeishuCommentEvent( fileToken: turn.fileToken, fileType: turn.fileType, commentId: turn.commentId, + isWholeComment: turn.isWholeComment, }); log( diff --git a/extensions/feishu/src/drive-schema.ts b/extensions/feishu/src/drive-schema.ts index 0dc85dc45d1..044ee0f3d48 100644 --- a/extensions/feishu/src/drive-schema.ts +++ b/extensions/feishu/src/drive-schema.ts @@ -52,22 +52,26 @@ export const FeishuDriveSchema = Type.Union([ Type.Object({ action: Type.Literal("list_comments"), file_token: Type.String({ description: "Document token" }), - file_type: CommentFileType, - page_size: Type.Optional(Type.Integer({ minimum: 1, description: "Page size" })), + file_type: Type.Optional(CommentFileType), + page_size: Type.Optional(Type.Integer({ minimum: 1, maximum: 100, description: "Page size" })), page_token: Type.Optional(Type.String({ description: "Comment page token" })), }), Type.Object({ action: Type.Literal("list_comment_replies"), file_token: Type.String({ description: "Document token" }), - file_type: CommentFileType, + file_type: Type.Optional(CommentFileType), comment_id: Type.String({ description: "Comment id" }), - page_size: Type.Optional(Type.Integer({ minimum: 1, description: "Page size" })), + page_size: Type.Optional(Type.Integer({ minimum: 1, maximum: 100, description: "Page size" })), page_token: Type.Optional(Type.String({ description: "Reply page token" })), }), Type.Object({ action: Type.Literal("add_comment"), file_token: Type.String({ description: "Document token" }), - file_type: Type.Union([Type.Literal("doc"), Type.Literal("docx")]), + file_type: Type.Optional( + Type.Union([Type.Literal("doc"), Type.Literal("docx")], { + description: "Document type. Defaults to docx when omitted.", + }), + ), content: Type.String({ description: "Comment text content" }), block_id: Type.Optional( Type.String({ @@ -79,7 +83,7 @@ export const FeishuDriveSchema = Type.Union([ Type.Object({ action: Type.Literal("reply_comment"), file_token: Type.String({ description: "Document token" }), - file_type: CommentFileType, + file_type: Type.Optional(CommentFileType), comment_id: Type.String({ description: "Comment id" }), content: Type.String({ description: "Reply text content" }), }), diff --git a/extensions/feishu/src/drive.test.ts b/extensions/feishu/src/drive.test.ts index 81375555d87..6fbc1c23ff3 100644 --- a/extensions/feishu/src/drive.test.ts +++ b/extensions/feishu/src/drive.test.ts @@ -206,8 +206,10 @@ describe("registerFeishuDriveTools", () => { requestMock .mockResolvedValueOnce({ - code: 99991663, - msg: "invalid request body", + code: 0, + data: { + items: [{ comment_id: "c1", is_whole: false }], + }, }) .mockResolvedValueOnce({ code: 0, @@ -224,7 +226,18 @@ describe("registerFeishuDriveTools", () => { 4, expect.objectContaining({ method: "POST", - url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies?file_type=docx", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { + comment_ids: ["c1"], + }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 5, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, data: { content: { elements: [ @@ -239,18 +252,821 @@ describe("registerFeishuDriveTools", () => { }, }), ); - expect(requestMock).toHaveBeenNthCalledWith( - 5, + expect(replyCommentResult.details).toEqual( + expect.objectContaining({ success: true, reply_id: "r4" }), + ); + }); + + it("defaults add_comment file_type to docx when omitted", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock.mockResolvedValueOnce({ + code: 0, + data: { comment_id: "c-default-docx" }, + }); + + const result = await tool.execute("call-default-docx", { + action: "add_comment", + file_token: "doc_1", + content: "defaulted file type", + }); + + expect(requestMock).toHaveBeenCalledWith( expect.objectContaining({ method: "POST", - url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies?file_type=docx", + url: "/open-apis/drive/v1/files/doc_1/new_comments", data: { - reply_elements: [{ type: "text", text: "handled" }], + file_type: "docx", + reply_elements: [{ type: "text", text: "defaulted file type" }], + }, + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("add_comment missing file_type; defaulting to docx"), + ); + expect(result.details).toEqual( + expect.objectContaining({ success: true, comment_id: "c-default-docx" }), + ); + }); + + it("defaults list_comments file_type to docx when omitted", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock.mockResolvedValueOnce({ + code: 0, + data: { has_more: false, items: [] }, + }); + + await tool.execute("call-list-default-docx", { + action: "list_comments", + file_token: "doc_1", + }); + + expect(requestMock).toHaveBeenCalledWith( + expect.objectContaining({ + method: "GET", + url: "/open-apis/drive/v1/files/doc_1/comments?file_type=docx&user_id_type=open_id", + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("list_comments missing file_type; defaulting to docx"), + ); + }); + + it("defaults list_comment_replies file_type to docx when omitted", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock.mockResolvedValueOnce({ + code: 0, + data: { has_more: false, items: [] }, + }); + + await tool.execute("call-replies-default-docx", { + action: "list_comment_replies", + file_token: "doc_1", + comment_id: "c1", + }); + + expect(requestMock).toHaveBeenCalledWith( + expect.objectContaining({ + method: "GET", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies?file_type=docx&user_id_type=open_id", + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("list_comment_replies missing file_type; defaulting to docx"), + ); + }); + + it("surfaces reply_comment HTTP errors when the single supported body fails", async () => { + const registerTool = vi.fn(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock + .mockResolvedValueOnce({ + code: 0, + data: { + items: [{ comment_id: "c1", is_whole: false }], + }, + }) + .mockRejectedValueOnce({ + message: "Request failed with status code 400", + code: "ERR_BAD_REQUEST", + config: { + method: "post", + url: "https://open.feishu.cn/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + }, + response: { + status: 400, + data: { + code: 99992402, + msg: "field validation failed", + log_id: "log_legacy_400", + }, + }, + }); + + const replyCommentResult = await tool.execute("call-throw", { + action: "reply_comment", + file_token: "doc_1", + file_type: "docx", + comment_id: "c1", + content: "inserted successfully", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { + comment_ids: ["c1"], + }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + data: { + content: { + elements: [ + { + type: "text_run", + text_run: { + text: "inserted successfully", + }, + }, + ], + }, + }, + }), + ); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("replyComment threw")); + expect(replyCommentResult.details).toEqual( + expect.objectContaining({ error: "Request failed with status code 400" }), + ); + }); + + it("defaults reply_comment target fields from the ambient Feishu comment delivery context", async () => { + const registerTool = vi.fn(); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ + agentAccountId: undefined, + deliveryContext: { + channel: "feishu", + to: "comment:docx:doc_1:c1", + }, + }); + + requestMock + .mockResolvedValueOnce({ + code: 0, + data: { + items: [{ comment_id: "c1", is_whole: false }], + }, + }) + .mockResolvedValueOnce({ + code: 0, + data: { reply_id: "r6" }, + }); + + const replyCommentResult = await tool.execute("call-ambient", { + action: "reply_comment", + content: "ambient success", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { + comment_ids: ["c1"], + }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + data: { + content: { + elements: [ + { + type: "text_run", + text_run: { + text: "ambient success", + }, + }, + ], + }, }, }), ); expect(replyCommentResult.details).toEqual( - expect.objectContaining({ success: true, reply_id: "r4" }), + expect.objectContaining({ success: true, reply_id: "r6" }), + ); + }); + + it("does not inherit non-doc ambient file types for add_comment", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ + agentAccountId: undefined, + deliveryContext: { + channel: "feishu", + to: "comment:sheet:sheet_1:c1", + }, + }); + + requestMock.mockResolvedValueOnce({ + code: 0, + data: { comment_id: "c-add-docx" }, + }); + + const result = await tool.execute("call-add-ignore-sheet-ambient", { + action: "add_comment", + file_token: "doc_1", + content: "default add comment", + }); + + expect(requestMock).toHaveBeenCalledWith( + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/new_comments", + data: { + file_type: "docx", + reply_elements: [{ type: "text", text: "default add comment" }], + }, + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("add_comment missing file_type; defaulting to docx"), + ); + expect(result.details).toEqual( + expect.objectContaining({ success: true, comment_id: "c-add-docx" }), + ); + }); + + it("defaults reply_comment file_type to docx when omitted", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock + .mockResolvedValueOnce({ + code: 0, + data: { + items: [{ comment_id: "c1", is_whole: false }], + }, + }) + .mockResolvedValueOnce({ + code: 0, + data: { reply_id: "r-default-docx" }, + }); + + const result = await tool.execute("call-reply-default-docx", { + action: "reply_comment", + file_token: "doc_1", + comment_id: "c1", + content: "default reply docx", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { comment_ids: ["c1"] }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + data: { + content: { + elements: [ + { + type: "text_run", + text_run: { + text: "default reply docx", + }, + }, + ], + }, + }, + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("reply_comment missing file_type; defaulting to docx"), + ); + expect(result.details).toEqual( + expect.objectContaining({ success: true, reply_id: "r-default-docx" }), + ); + }); + + it("routes whole-document reply_comment requests through add_comment compatibility", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock + .mockResolvedValueOnce({ + code: 0, + data: { + items: [{ comment_id: "c1", is_whole: true }], + }, + }) + .mockResolvedValueOnce({ + code: 0, + data: { comment_id: "c2" }, + }); + + const result = await tool.execute("call-whole", { + action: "reply_comment", + file_token: "doc_1", + file_type: "docx", + comment_id: "c1", + content: "whole comment follow-up", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { + comment_ids: ["c1"], + }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/new_comments", + data: { + file_type: "docx", + reply_elements: [{ type: "text", text: "whole comment follow-up" }], + }, + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("whole-comment compatibility path"), + ); + expect(result.details).toEqual( + expect.objectContaining({ + success: true, + comment_id: "c2", + delivery_mode: "add_comment", + }), + ); + }); + + it("continues with reply_comment when comment metadata preflight fails", async () => { + const registerTool = vi.fn(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock.mockRejectedValueOnce(new Error("preflight unavailable")).mockResolvedValueOnce({ + code: 0, + data: { reply_id: "r-preflight-fallback" }, + }); + + const result = await tool.execute("call-preflight-fallback", { + action: "reply_comment", + file_token: "doc_1", + file_type: "docx", + comment_id: "c1", + content: "preflight fallback reply", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { + comment_ids: ["c1"], + }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + data: { + content: { + elements: [ + { + type: "text_run", + text_run: { + text: "preflight fallback reply", + }, + }, + ], + }, + }, + }), + ); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("comment metadata preflight failed"), + ); + expect(result.details).toEqual( + expect.objectContaining({ + success: true, + reply_id: "r-preflight-fallback", + delivery_mode: "reply_comment", + }), + ); + }); + + it("continues with reply_comment when batch_query returns no exact comment match", async () => { + const registerTool = vi.fn(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock + .mockResolvedValueOnce({ + code: 0, + data: { + items: [{ comment_id: "different_comment", is_whole: true }], + }, + }) + .mockResolvedValueOnce({ + code: 0, + data: { reply_id: "r-no-exact-match" }, + }); + + const result = await tool.execute("call-preflight-no-exact-match", { + action: "reply_comment", + file_token: "doc_1", + file_type: "docx", + comment_id: "c1", + content: "fallback on exact match miss", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/batch_query?file_type=docx&user_id_type=open_id", + data: { + comment_ids: ["c1"], + }, + }), + ); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + data: { + content: { + elements: [ + { + type: "text_run", + text_run: { + text: "fallback on exact match miss", + }, + }, + ], + }, + }, + }), + ); + expect(warnSpy).not.toHaveBeenCalledWith( + expect.stringContaining("whole-comment compatibility path"), + ); + expect(result.details).toEqual( + expect.objectContaining({ + success: true, + reply_id: "r-no-exact-match", + delivery_mode: "reply_comment", + }), + ); + }); + + it("falls back to add_comment when reply_comment returns compatibility code 1069302 even without is_whole metadata", async () => { + const registerTool = vi.fn(); + const infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock + .mockResolvedValueOnce({ + code: 0, + data: { + items: [{ comment_id: "c1", is_whole: false }], + }, + }) + .mockRejectedValueOnce({ + message: "Request failed with status code 400", + code: "ERR_BAD_REQUEST", + config: { + method: "post", + url: "https://open.feishu.cn/open-apis/drive/v1/files/doc_1/comments/c1/replies", + params: { file_type: "docx" }, + }, + response: { + status: 400, + data: { + code: 1069302, + msg: "param error", + log_id: "log_reply_forbidden", + }, + }, + }) + .mockResolvedValueOnce({ + code: 0, + data: { comment_id: "c3" }, + }); + + const result = await tool.execute("call-reply-forbidden", { + action: "reply_comment", + file_token: "doc_1", + file_type: "docx", + comment_id: "c1", + content: "compat follow-up", + }); + + expect(requestMock).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + method: "POST", + url: "/open-apis/drive/v1/files/doc_1/new_comments", + data: { + file_type: "docx", + reply_elements: [{ type: "text", text: "compat follow-up" }], + }, + }), + ); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining("reply-not-allowed compatibility path"), + ); + expect(result.details).toEqual( + expect.objectContaining({ + success: true, + comment_id: "c3", + delivery_mode: "add_comment", + }), + ); + }); + + it("clamps comment list page sizes to the Feishu API maximum", async () => { + const registerTool = vi.fn(); + registerFeishuDriveTools( + createDriveToolApi({ + config: { + channels: { + feishu: { + enabled: true, + appId: "app_id", + appSecret: "app_secret", // pragma: allowlist secret + tools: { drive: true }, + }, + }, + }, + registerTool, + }), + ); + + const toolFactory = registerTool.mock.calls[0]?.[0]; + const tool = toolFactory?.({ agentAccountId: undefined }); + + requestMock.mockResolvedValueOnce({ code: 0, data: { has_more: false, items: [] } }); + await tool.execute("call-list", { + action: "list_comments", + file_token: "doc_1", + file_type: "docx", + page_size: 200, + }); + expect(requestMock).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + method: "GET", + url: "/open-apis/drive/v1/files/doc_1/comments?file_type=docx&page_size=100&user_id_type=open_id", + }), + ); + + requestMock.mockResolvedValueOnce({ code: 0, data: { has_more: false, items: [] } }); + await tool.execute("call-replies", { + action: "list_comment_replies", + file_token: "doc_1", + file_type: "docx", + comment_id: "c1", + page_size: 200, + }); + expect(requestMock).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + method: "GET", + url: "/open-apis/drive/v1/files/doc_1/comments/c1/replies?file_type=docx&page_size=100&user_id_type=open_id", + }), ); }); diff --git a/extensions/feishu/src/drive.ts b/extensions/feishu/src/drive.ts index 8e747169374..bdc079775a9 100644 --- a/extensions/feishu/src/drive.ts +++ b/extensions/feishu/src/drive.ts @@ -1,7 +1,7 @@ import type * as Lark from "@larksuiteoapi/node-sdk"; import type { OpenClawPluginApi } from "../runtime-api.js"; import { listEnabledFeishuAccounts } from "./accounts.js"; -import { type CommentFileType } from "./comment-target.js"; +import { parseFeishuCommentTarget, type CommentFileType } from "./comment-target.js"; import { FeishuDriveSchema, type FeishuDriveParams } from "./drive-schema.js"; import { createFeishuToolClient, resolveAnyEnabledFeishuToolsConfig } from "./tool-account.js"; import { @@ -26,6 +26,7 @@ type FeishuDriveInternalClient = Lark.Client & { request(params: { method: "GET" | "POST"; url: string; + params?: Record; data: unknown; timeout?: number; }): Promise; @@ -33,10 +34,33 @@ type FeishuDriveInternalClient = Lark.Client & { type FeishuDriveApiResponse = { code: number; + log_id?: string; msg?: string; data?: T; }; +class FeishuReplyCommentError extends Error { + httpStatus?: number; + feishuCode?: number | string; + feishuMsg?: string; + feishuLogId?: string; + + constructor(params: { + message: string; + httpStatus?: number; + feishuCode?: number | string; + feishuMsg?: string; + feishuLogId?: string; + }) { + super(params.message); + this.name = "FeishuReplyCommentError"; + this.httpStatus = params.httpStatus; + this.feishuCode = params.feishuCode; + this.feishuMsg = params.feishuMsg; + this.feishuLogId = params.feishuLogId; + } +} + type FeishuDriveCommentReply = { reply_id?: string; user_id?: string; @@ -74,6 +98,13 @@ type FeishuDriveListRepliesResponse = FeishuDriveApiResponse<{ page_token?: string; }>; +type FeishuDriveToolContext = { + deliveryContext?: { + channel?: string; + to?: string; + }; +}; + const FEISHU_DRIVE_REQUEST_TIMEOUT_MS = 30_000; function getDriveInternalClient(client: Lark.Client): FeishuDriveInternalClient { @@ -159,12 +190,14 @@ async function requestDriveApi(params: { client: Lark.Client; method: "GET" | "POST"; url: string; + query?: Record; data?: unknown; }): Promise { const internalClient = getDriveInternalClient(params.client); return (await internalClient.request({ method: params.method, url: params.url, + params: params.query ?? {}, data: params.data ?? {}, timeout: FEISHU_DRIVE_REQUEST_TIMEOUT_MS, })) as T; @@ -205,6 +238,149 @@ function normalizeCommentCard(comment: FeishuDriveCommentCard) { }; } +function normalizeCommentPageSize(pageSize: number | undefined): string | undefined { + if (typeof pageSize !== "number" || !Number.isFinite(pageSize)) { + return undefined; + } + return String(Math.min(Math.max(Math.floor(pageSize), 1), 100)); +} + +function resolveAmbientCommentTarget(context: FeishuDriveToolContext | undefined) { + const deliveryContext = context?.deliveryContext; + if (deliveryContext?.channel && deliveryContext.channel !== "feishu") { + return null; + } + return parseFeishuCommentTarget(deliveryContext?.to); +} + +function applyAmbientCommentDefaults< + T extends { + file_token?: string; + file_type?: CommentFileType; + comment_id?: string; + }, +>(params: T, context: FeishuDriveToolContext | undefined): T { + const ambient = resolveAmbientCommentTarget(context); + if (!ambient) { + return params; + } + return { + ...params, + file_token: params.file_token?.trim() || ambient.fileToken, + file_type: params.file_type ?? ambient.fileType, + comment_id: params.comment_id?.trim() || ambient.commentId, + }; +} + +function applyAddCommentAmbientDefaults< + T extends { + file_token?: string; + file_type?: "doc" | "docx"; + }, +>(params: T, context: FeishuDriveToolContext | undefined): T { + const ambient = resolveAmbientCommentTarget(context); + if (!ambient || (ambient.fileType !== "doc" && ambient.fileType !== "docx")) { + return params; + } + return { + ...params, + file_token: params.file_token?.trim() || ambient.fileToken, + file_type: params.file_type ?? ambient.fileType, + }; +} + +function applyAddCommentDefaults< + T extends { + file_token?: string; + file_type?: "doc" | "docx"; + }, +>(params: T): T & { file_type: "doc" | "docx" } { + const fileType = params.file_type ?? "docx"; + if (!params.file_type) { + console.info( + `[feishu_drive] add_comment missing file_type; defaulting to docx ` + + `file_token=${params.file_token ?? "unknown"}`, + ); + } + return { + ...params, + file_type: fileType, + }; +} + +function applyCommentFileTypeDefault< + T extends { + file_token?: string; + file_type?: CommentFileType; + }, +>( + params: T, + action: "list_comments" | "list_comment_replies" | "reply_comment", +): T & { + file_type: CommentFileType; +} { + const fileType = params.file_type ?? "docx"; + if (!params.file_type) { + console.info( + `[feishu_drive] ${action} missing file_type; defaulting to docx ` + + `file_token=${params.file_token ?? "unknown"}`, + ); + } + return { + ...params, + file_type: fileType, + }; +} + +function formatDriveApiError(error: unknown): string { + if (!isRecord(error)) { + return String(error); + } + const response = isRecord(error.response) ? error.response : undefined; + const responseData = isRecord(response?.data) ? response?.data : undefined; + return JSON.stringify({ + message: typeof error.message === "string" ? error.message : String(error), + code: readString(error.code), + method: readString(isRecord(error.config) ? error.config.method : undefined), + url: readString(isRecord(error.config) ? error.config.url : undefined), + params: isRecord(error.config) ? error.config.params : undefined, + http_status: typeof response?.status === "number" ? response.status : undefined, + feishu_code: + typeof responseData?.code === "number" ? responseData.code : readString(responseData?.code), + feishu_msg: readString(responseData?.msg), + feishu_log_id: readString(responseData?.log_id), + }); +} + +function extractDriveApiErrorMeta(error: unknown): { + message: string; + httpStatus?: number; + feishuCode?: number | string; + feishuMsg?: string; + feishuLogId?: string; +} { + if (!isRecord(error)) { + return { message: String(error) }; + } + const response = isRecord(error.response) ? error.response : undefined; + const responseData = isRecord(response?.data) ? response?.data : undefined; + return { + message: typeof error.message === "string" ? error.message : String(error), + httpStatus: typeof response?.status === "number" ? response.status : undefined, + feishuCode: + typeof responseData?.code === "number" ? responseData.code : readString(responseData?.code), + feishuMsg: readString(responseData?.msg), + feishuLogId: readString(responseData?.log_id), + }; +} + +function isReplyNotAllowedError(error: unknown): boolean { + if (!(error instanceof FeishuReplyCommentError)) { + return false; + } + return error.feishuCode === 1069302; +} + async function getRootFolderToken(client: Lark.Client): Promise { // Use generic HTTP client to call the root folder meta API // as it's not directly exposed in the SDK @@ -371,10 +547,7 @@ async function listComments( `/open-apis/drive/v1/files/${encodeURIComponent(params.file_token)}/comments` + encodeQuery({ file_type: params.file_type, - page_size: - typeof params.page_size === "number" && Number.isFinite(params.page_size) - ? String(params.page_size) - : undefined, + page_size: normalizeCommentPageSize(params.page_size), page_token: params.page_token, user_id_type: "open_id", }), @@ -407,10 +580,7 @@ async function listCommentReplies( )}/replies` + encodeQuery({ file_type: params.file_type, - page_size: - typeof params.page_size === "number" && Number.isFinite(params.page_size) - ? String(params.page_size) - : undefined, + page_size: normalizeCommentPageSize(params.page_size), page_token: params.page_token, user_id_type: "open_id", }), @@ -431,7 +601,7 @@ async function addComment( content: string; block_id?: string; }, -) { +): Promise<{ success: true } & Record> { if (params.block_id?.trim() && params.file_type !== "docx") { throw new Error("block_id is only supported for docx comments"); } @@ -453,6 +623,34 @@ async function addComment( }; } +// Fetch comment metadata via batch_query because the single-comment endpoint +// does not support partial comments. +async function queryCommentById( + client: Lark.Client, + params: { + file_token: string; + file_type: CommentFileType; + comment_id: string; + }, +) { + const response = assertDriveApiSuccess( + await requestDriveApi({ + client, + method: "POST", + url: + `/open-apis/drive/v1/files/${encodeURIComponent(params.file_token)}/comments/batch_query` + + encodeQuery({ + file_type: params.file_type, + user_id_type: "open_id", + }), + data: { + comment_ids: [params.comment_id], + }, + }), + ); + return response.data?.items?.find((comment) => comment.comment_id?.trim() === params.comment_id); +} + export async function replyComment( client: Lark.Client, params: { @@ -462,34 +660,28 @@ export async function replyComment( content: string; }, ): Promise<{ success: true; reply_id?: string } & Record> { - const url = - `/open-apis/drive/v1/files/${encodeURIComponent(params.file_token)}/comments/${encodeURIComponent( - params.comment_id, - )}/replies` + encodeQuery({ file_type: params.file_type }); - const attempts: unknown[] = [ - { - content: { - elements: [ - { - type: "text_run", - text_run: { - text: params.content, - }, - }, - ], - }, - }, - { - reply_elements: buildReplyElements(params.content), - }, - ]; - let lastMessage = "Feishu Drive reply comment failed"; - for (const data of attempts) { + const url = `/open-apis/drive/v1/files/${encodeURIComponent(params.file_token)}/comments/${encodeURIComponent( + params.comment_id, + )}/replies`; + const query = { file_type: params.file_type }; + try { const response = (await requestDriveApi>>({ client, method: "POST", url, - data, + query, + data: { + content: { + elements: [ + { + type: "text_run", + text_run: { + text: params.content, + }, + }, + ], + }, + }, })) as FeishuDriveApiResponse>; if (response.code === 0) { return { @@ -497,9 +689,116 @@ export async function replyComment( ...response.data, }; } - lastMessage = response.msg ?? lastMessage; + console.warn( + `[feishu_drive] replyComment failed ` + + `comment=${params.comment_id} file_type=${params.file_type} ` + + `code=${response.code ?? "unknown"} ` + + `msg=${response.msg ?? "unknown"} log_id=${response.log_id ?? "unknown"}`, + ); + throw new FeishuReplyCommentError({ + message: response.msg ?? "Feishu Drive reply comment failed", + feishuCode: response.code, + feishuMsg: response.msg, + feishuLogId: response.log_id, + }); + } catch (error) { + if (error instanceof FeishuReplyCommentError) { + throw error; + } + const meta = extractDriveApiErrorMeta(error); + console.warn( + `[feishu_drive] replyComment threw ` + + `comment=${params.comment_id} file_type=${params.file_type} ` + + `error=${formatDriveApiError(error)}`, + ); + throw new FeishuReplyCommentError({ + message: meta.message, + httpStatus: meta.httpStatus, + feishuCode: meta.feishuCode, + feishuMsg: meta.feishuMsg, + feishuLogId: meta.feishuLogId, + }); + } +} + +export async function deliverCommentThreadText( + client: Lark.Client, + params: { + file_token: string; + file_type: CommentFileType; + comment_id: string; + content: string; + is_whole_comment?: boolean; + }, +): Promise< + | ({ success: true; reply_id?: string } & Record & { + delivery_mode: "reply_comment"; + }) + | ({ success: true; comment_id?: string } & Record & { + delivery_mode: "add_comment"; + }) +> { + let isWholeComment = params.is_whole_comment; + if (isWholeComment === undefined) { + try { + const comment = await queryCommentById(client, params); + isWholeComment = comment?.is_whole === true; + } catch (error) { + console.warn( + `[feishu_drive] comment metadata preflight failed ` + + `comment=${params.comment_id} file_type=${params.file_type} ` + + `error=${error instanceof Error ? error.message : String(error)}`, + ); + isWholeComment = false; + } + } + if (isWholeComment) { + if (params.file_type !== "doc" && params.file_type !== "docx") { + throw new Error( + `Whole-document comment follow-ups are only supported for doc/docx (got ${params.file_type})`, + ); + } + const wholeCommentFileType: "doc" | "docx" = params.file_type; + console.info( + `[feishu_drive] whole-comment compatibility path ` + + `comment=${params.comment_id} file_type=${params.file_type} mode=add_comment`, + ); + return { + delivery_mode: "add_comment", + ...(await addComment(client, { + file_token: params.file_token, + file_type: wholeCommentFileType, + content: params.content, + })), + }; + } + try { + return { + delivery_mode: "reply_comment", + ...(await replyComment(client, params)), + }; + } catch (error) { + if (error instanceof FeishuReplyCommentError && isReplyNotAllowedError(error)) { + if (params.file_type !== "doc" && params.file_type !== "docx") { + throw error; + } + const fallbackFileType: "doc" | "docx" = params.file_type; + console.info( + `[feishu_drive] reply-not-allowed compatibility path ` + + `comment=${params.comment_id} file_type=${params.file_type} mode=add_comment ` + + `log_id=${error.feishuLogId ?? "unknown"}`, + ); + return { + delivery_mode: "add_comment", + ...(await addComment(client, { + file_token: params.file_token, + file_type: fallbackFileType, + content: params.content, + })), + }; + } + throw error; } - throw new Error(lastMessage); } // ============ Tool Registration ============ @@ -552,14 +851,31 @@ export function registerFeishuDriveTools(api: OpenClawPluginApi) { return jsonToolResult(await moveFile(client, p.file_token, p.type, p.folder_token)); case "delete": return jsonToolResult(await deleteFile(client, p.file_token, p.type)); - case "list_comments": - return jsonToolResult(await listComments(client, p)); - case "list_comment_replies": - return jsonToolResult(await listCommentReplies(client, p)); - case "add_comment": - return jsonToolResult(await addComment(client, p)); - case "reply_comment": - return jsonToolResult(await replyComment(client, p)); + case "list_comments": { + const resolved = applyCommentFileTypeDefault( + applyAmbientCommentDefaults(p, ctx), + "list_comments", + ); + return jsonToolResult(await listComments(client, resolved)); + } + case "list_comment_replies": { + const resolved = applyCommentFileTypeDefault( + applyAmbientCommentDefaults(p, ctx), + "list_comment_replies", + ); + return jsonToolResult(await listCommentReplies(client, resolved)); + } + case "add_comment": { + const resolved = applyAddCommentDefaults(applyAddCommentAmbientDefaults(p, ctx)); + return jsonToolResult(await addComment(client, resolved)); + } + case "reply_comment": { + const resolved = applyCommentFileTypeDefault( + applyAmbientCommentDefaults(p, ctx), + "reply_comment", + ); + return jsonToolResult(await deliverCommentThreadText(client, resolved)); + } default: return unknownToolActionResult((p as { action?: unknown }).action); } diff --git a/extensions/feishu/src/monitor.comment.test.ts b/extensions/feishu/src/monitor.comment.test.ts index 17557c1e541..38066d374d2 100644 --- a/extensions/feishu/src/monitor.comment.test.ts +++ b/extensions/feishu/src/monitor.comment.test.ts @@ -97,11 +97,15 @@ function makeDriveCommentEvent( function makeOpenApiClient(params: { documentTitle?: string; documentUrl?: string; + isWholeComment?: boolean; + batchCommentId?: string; quoteText?: string; rootReplyText?: string; targetReplyText?: string; includeTargetReplyInBatch?: boolean; + repliesSequence?: Array>; }) { + const remainingReplyBatches = [...(params.repliesSequence ?? [])]; return { request: vi.fn(async (request: { method: "GET" | "POST"; url: string; data: unknown }) => { if (request.url === "/open-apis/drive/v1/metas/batch_query") { @@ -124,7 +128,8 @@ function makeOpenApiClient(params: { data: { items: [ { - comment_id: "7623358762119646411", + comment_id: params.batchCommentId ?? "7623358762119646411", + is_whole: params.isWholeComment, quote: params.quoteText ?? "im.message.receive_v1 message trigger implementation", reply_list: { replies: [ @@ -169,40 +174,54 @@ function makeOpenApiClient(params: { }; } if (request.url.includes("/replies")) { + const replyBatch = remainingReplyBatches.shift(); + const items = replyBatch?.map((reply) => ({ + reply_id: reply.reply_id, + content: { + elements: [ + { + type: "text_run", + text_run: { + content: reply.text, + }, + }, + ], + }, + })) ?? [ + { + reply_id: "7623358762136374451", + content: { + elements: [ + { + type: "text_run", + text_run: { + content: + params.rootReplyText ?? + "Also send it to the agent after receiving the comment event", + }, + }, + ], + }, + }, + { + reply_id: "7623359125036043462", + content: { + elements: [ + { + type: "text_run", + text_run: { + content: params.targetReplyText ?? "Please follow up on this comment", + }, + }, + ], + }, + }, + ]; return { code: 0, data: { has_more: false, - items: [ - { - reply_id: "7623358762136374451", - content: { - elements: [ - { - type: "text_run", - text_run: { - content: - params.rootReplyText ?? - "Also send it to the agent after receiving the comment event", - }, - }, - ], - }, - }, - { - reply_id: "7623359125036043462", - content: { - elements: [ - { - type: "text_run", - text_run: { - content: params.targetReplyText ?? "Please follow up on this comment", - }, - }, - ], - }, - }, - ], + items, }, }; } @@ -257,11 +276,53 @@ describe("resolveDriveCommentEventTurn", () => { expect(turn?.prompt).toContain( "This is a Feishu document comment-thread event, not a Feishu IM conversation.", ); + expect(turn?.prompt).toContain("Prefer plain text suitable for a comment thread."); + expect(turn?.prompt).toContain("Do not include internal reasoning"); + expect(turn?.prompt).toContain("Do not narrate your plan or execution process"); + expect(turn?.prompt).toContain("reply only with the user-facing result itself"); expect(turn?.prompt).toContain("comment_id: 7623358762119646411"); expect(turn?.prompt).toContain("reply_id: 7623358762136374451"); expect(turn?.prompt).toContain("The system will automatically reply with your final answer"); }); + it("preserves whole-document comment metadata for downstream delivery mode selection", async () => { + const client = makeOpenApiClient({ + includeTargetReplyInBatch: true, + isWholeComment: true, + }); + + const turn = await resolveDriveCommentEventTurn({ + cfg: buildMonitorConfig(), + accountId: "default", + event: makeDriveCommentEvent(), + botOpenId: "ou_bot", + createClient: () => client as never, + }); + + expect(turn?.isWholeComment).toBe(true); + expect(turn?.prompt).toContain("This is a whole-document comment."); + expect(turn?.prompt).toContain("Whole-document comments do not support direct replies."); + }); + + it("does not trust whole-comment metadata from a mismatched batch_query item", async () => { + const client = makeOpenApiClient({ + includeTargetReplyInBatch: true, + isWholeComment: true, + batchCommentId: "different_comment_id", + }); + + const turn = await resolveDriveCommentEventTurn({ + cfg: buildMonitorConfig(), + accountId: "default", + event: makeDriveCommentEvent(), + botOpenId: "ou_bot", + createClient: () => client as never, + }); + + expect(turn?.isWholeComment).toBeUndefined(); + expect(turn?.prompt).not.toContain("This is a whole-document comment."); + }); + it("preserves sender user_id for downstream allowlist checks", async () => { const client = makeOpenApiClient({ includeTargetReplyInBatch: true }); @@ -313,6 +374,71 @@ describe("resolveDriveCommentEventTurn", () => { ); expect(turn?.prompt).toContain(`file_token: ${TEST_DOC_TOKEN}`); expect(turn?.prompt).toContain("Event type: add_reply"); + expect(client.request).toHaveBeenCalledWith( + expect.objectContaining({ + method: "GET", + url: expect.stringContaining( + `/comments/7623358762119646411/replies?file_type=docx&page_size=100&user_id_type=open_id`, + ), + }), + ); + }); + + it("retries comment reply lookup when the requested reply is not immediately visible", async () => { + const waitMs = vi.fn(async () => {}); + const client = makeOpenApiClient({ + includeTargetReplyInBatch: false, + repliesSequence: [ + [ + { + reply_id: "7623358762136374451", + text: "Also send it to the agent after receiving the comment event", + }, + { reply_id: "7623358762999999999", text: "Earlier assistant summary" }, + ], + [ + { + reply_id: "7623358762136374451", + text: "Also send it to the agent after receiving the comment event", + }, + { reply_id: "7623358762999999999", text: "Earlier assistant summary" }, + ], + [ + { + reply_id: "7623358762136374451", + text: "Also send it to the agent after receiving the comment event", + }, + { reply_id: "7623359125999999999", text: "Insert a sentence below this paragraph" }, + ], + ], + }); + + const turn = await resolveDriveCommentEventTurn({ + cfg: buildMonitorConfig(), + accountId: "default", + event: makeDriveCommentEvent({ + notice_meta: { + ...makeDriveCommentEvent().notice_meta, + notice_type: "add_reply", + }, + reply_id: "7623359125999999999", + }), + botOpenId: "ou_bot", + createClient: () => client as never, + waitMs, + }); + + expect(turn?.targetReplyText).toBe("Insert a sentence below this paragraph"); + expect(turn?.prompt).toContain("Insert a sentence below this paragraph"); + expect(waitMs).toHaveBeenCalledTimes(2); + expect(waitMs).toHaveBeenNthCalledWith(1, 1000); + expect(waitMs).toHaveBeenNthCalledWith(2, 1000); + expect( + client.request.mock.calls.filter( + ([request]: [{ method: string; url: string }]) => + request.method === "GET" && request.url.includes("/replies"), + ), + ).toHaveLength(3); }); it("ignores self-authored comment notices", async () => { diff --git a/extensions/feishu/src/monitor.comment.ts b/extensions/feishu/src/monitor.comment.ts index 8c8f9332318..0c49ca20891 100644 --- a/extensions/feishu/src/monitor.comment.ts +++ b/extensions/feishu/src/monitor.comment.ts @@ -6,8 +6,10 @@ import { normalizeCommentFileType, type CommentFileType } from "./comment-target import type { ResolvedFeishuAccount } from "./types.js"; const FEISHU_COMMENT_VERIFY_TIMEOUT_MS = 3_000; -const FEISHU_COMMENT_REPLY_PAGE_SIZE = 200; +const FEISHU_COMMENT_REPLY_PAGE_SIZE = 100; const FEISHU_COMMENT_REPLY_PAGE_LIMIT = 5; +const FEISHU_COMMENT_REPLY_MISS_RETRY_DELAY_MS = 1_000; +const FEISHU_COMMENT_REPLY_MISS_RETRY_LIMIT = 6; type FeishuDriveCommentUserId = { open_id?: string; @@ -39,6 +41,7 @@ type ResolveDriveCommentEventParams = { createClient?: (account: ResolvedFeishuAccount) => FeishuRequestClient; verificationTimeoutMs?: number; logger?: (message: string) => void; + waitMs?: (ms: number) => Promise; }; export type ResolvedDriveCommentEventTurn = { @@ -49,6 +52,7 @@ export type ResolvedDriveCommentEventTurn = { noticeType: "add_comment" | "add_reply"; fileToken: string; fileType: CommentFileType; + isWholeComment?: boolean; senderId: string; senderUserId?: string; timestamp?: string; @@ -73,6 +77,7 @@ type FeishuRequestClient = ReturnType & { type FeishuOpenApiResponse = { code?: number; + log_id?: string; msg?: string; data?: T; }; @@ -94,6 +99,7 @@ type FeishuDriveCommentReply = { type FeishuDriveCommentCard = { comment_id?: string; + is_whole?: boolean; quote?: string; reply_list?: { replies?: FeishuDriveCommentReply[]; @@ -122,6 +128,25 @@ function readBoolean(value: unknown): boolean | undefined { return typeof value === "boolean" ? value : undefined; } +function safeJsonStringify(value: unknown): string { + try { + return JSON.stringify(value); + } catch (error) { + return JSON.stringify({ + error: error instanceof Error ? error.message : String(error), + }); + } +} + +function summarizeCommentRepliesForLog(replies: FeishuDriveCommentReply[]): string { + return safeJsonStringify( + replies.map((reply) => ({ + reply_id: reply.reply_id, + text_len: extractReplyText(reply)?.length ?? 0, + })), + ); +} + function encodeQuery(params: Record): string { const query = new URLSearchParams(); for (const [key, value] of Object.entries(params)) { @@ -134,6 +159,10 @@ function encodeQuery(params: Record): string { return queryString ? `?${queryString}` : ""; } +async function delayMs(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + function buildDriveCommentTargetUrl(params: { fileToken: string; fileType: CommentFileType; @@ -175,6 +204,26 @@ async function requestFeishuOpenApi(params: { logger?: (message: string) => void; errorLabel: string; }): Promise { + const formatErrorDetails = (error: unknown): string => { + if (!isRecord(error)) { + return String(error); + } + const response = isRecord(error.response) ? error.response : undefined; + const responseData = isRecord(response?.data) ? response?.data : undefined; + const details = { + message: typeof error.message === "string" ? error.message : String(error), + code: readString(error.code), + method: readString(isRecord(error.config) ? error.config.method : undefined), + url: readString(isRecord(error.config) ? error.config.url : undefined), + http_status: typeof response?.status === "number" ? response.status : undefined, + feishu_code: + typeof responseData?.code === "number" ? responseData.code : readString(responseData?.code), + feishu_msg: readString(responseData?.msg), + feishu_log_id: readString(responseData?.log_id), + }; + return safeJsonStringify(details); + }; + const result = await raceWithTimeoutAndAbort( params.client.request({ method: params.method, @@ -186,7 +235,7 @@ async function requestFeishuOpenApi(params: { ) .then((resolved) => (resolved.status === "resolved" ? resolved.value : null)) .catch((error) => { - params.logger?.(`${params.errorLabel}: ${String(error)}`); + params.logger?.(`${params.errorLabel}: ${formatErrorDetails(error)}`); return null; }); if (!result) { @@ -254,8 +303,9 @@ async function fetchDriveCommentReplies(params: { timeoutMs: number; logger?: (message: string) => void; accountId: string; -}): Promise { +}): Promise<{ replies: FeishuDriveCommentReply[]; logIds: string[] }> { const replies: FeishuDriveCommentReply[] = []; + const logIds: string[] = []; let pageToken: string | undefined; for (let page = 0; page < FEISHU_COMMENT_REPLY_PAGE_LIMIT; page += 1) { const response = await requestFeishuOpenApi({ @@ -271,10 +321,15 @@ async function fetchDriveCommentReplies(params: { logger: params.logger, errorLabel: `feishu[${params.accountId}]: failed to fetch comment replies for ${params.commentId}`, }); + if (response?.log_id?.trim()) { + logIds.push(response.log_id.trim()); + } if (response?.code !== 0) { if (response) { params.logger?.( - `feishu[${params.accountId}]: failed to fetch comment replies for ${params.commentId}: ${response.msg ?? "unknown error"}`, + `feishu[${params.accountId}]: failed to fetch comment replies for ${params.commentId}: ` + + `${response.msg ?? "unknown error"} ` + + `log_id=${response.log_id?.trim() || "unknown"}`, ); } break; @@ -285,7 +340,7 @@ async function fetchDriveCommentReplies(params: { } pageToken = response.data.page_token.trim(); } - return replies; + return { replies, logIds }; } async function fetchDriveCommentContext(params: { @@ -297,9 +352,11 @@ async function fetchDriveCommentContext(params: { timeoutMs: number; logger?: (message: string) => void; accountId: string; + waitMs: (ms: number) => Promise; }): Promise<{ documentTitle?: string; documentUrl?: string; + isWholeComment?: boolean; quoteText?: string; rootCommentText?: string; targetReplyText?: string; @@ -335,35 +392,96 @@ async function fetchDriveCommentContext(params: { const commentCard = commentResponse?.code === 0 - ? ((commentResponse.data?.items ?? []).find( + ? (commentResponse.data?.items ?? []).find( (item) => item.comment_id?.trim() === params.commentId, - ) ?? commentResponse.data?.items?.[0]) + ) : undefined; const embeddedReplies = commentCard?.reply_list?.replies ?? []; + params.logger?.( + `feishu[${params.accountId}]: embedded comment replies comment=${params.commentId} ` + + `count=${embeddedReplies.length} summary=${summarizeCommentRepliesForLog(embeddedReplies)}`, + ); const embeddedTargetReply = params.replyId ? embeddedReplies.find((reply) => reply.reply_id?.trim() === params.replyId?.trim()) : embeddedReplies.at(-1); let replies = embeddedReplies; + let fetchedMatchedReply = params.replyId + ? replies.find((reply) => reply.reply_id?.trim() === params.replyId?.trim()) + : undefined; if (!embeddedTargetReply || replies.length === 0) { - const fetchedReplies = await fetchDriveCommentReplies(params); - if (fetchedReplies.length > 0) { - replies = fetchedReplies; + params.logger?.( + `feishu[${params.accountId}]: fetching extra comment replies comment=${params.commentId} ` + + `requested_reply=${params.replyId ?? "none"} ` + + `embedded_count=${embeddedReplies.length} ` + + `embedded_hit=${embeddedTargetReply ? "yes" : "no"}`, + ); + const fetched = await fetchDriveCommentReplies(params); + if (fetched.replies.length > 0) { + params.logger?.( + `feishu[${params.accountId}]: fetched extra comment replies comment=${params.commentId} ` + + `count=${fetched.replies.length} ` + + `log_ids=${safeJsonStringify(fetched.logIds)} ` + + `summary=${summarizeCommentRepliesForLog(fetched.replies)}`, + ); + replies = fetched.replies; + fetchedMatchedReply = params.replyId + ? replies.find((reply) => reply.reply_id?.trim() === params.replyId?.trim()) + : undefined; + } + if (params.replyId && !embeddedTargetReply && !fetchedMatchedReply) { + for (let attempt = 1; attempt <= FEISHU_COMMENT_REPLY_MISS_RETRY_LIMIT; attempt += 1) { + params.logger?.( + `feishu[${params.accountId}]: retrying comment reply lookup comment=${params.commentId} ` + + `requested_reply=${params.replyId} attempt=${attempt}/${FEISHU_COMMENT_REPLY_MISS_RETRY_LIMIT} ` + + `delay_ms=${FEISHU_COMMENT_REPLY_MISS_RETRY_DELAY_MS}`, + ); + await params.waitMs(FEISHU_COMMENT_REPLY_MISS_RETRY_DELAY_MS); + const retried = await fetchDriveCommentReplies(params); + if (retried.replies.length > 0) { + params.logger?.( + `feishu[${params.accountId}]: fetched retried comment replies comment=${params.commentId} ` + + `attempt=${attempt} count=${retried.replies.length} ` + + `log_ids=${safeJsonStringify(retried.logIds)} ` + + `summary=${summarizeCommentRepliesForLog(retried.replies)}`, + ); + replies = retried.replies; + } + fetchedMatchedReply = replies.find((reply) => reply.reply_id?.trim() === params.replyId); + if (fetchedMatchedReply) { + break; + } + } } } const rootReply = replies[0] ?? embeddedReplies[0]; - const fetchedMatchedReply = params.replyId - ? replies.find((reply) => reply.reply_id?.trim() === params.replyId?.trim()) - : undefined; const targetReply = params.replyId ? (embeddedTargetReply ?? fetchedMatchedReply ?? undefined) : (replies.at(-1) ?? embeddedTargetReply ?? rootReply); + const matchSource = params.replyId + ? embeddedTargetReply + ? "embedded" + : fetchedMatchedReply + ? "fetched" + : "miss" + : targetReply === rootReply + ? "fallback_root" + : targetReply === embeddedTargetReply + ? "embedded_latest" + : "fetched_latest"; + params.logger?.( + `feishu[${params.accountId}]: comment reply resolution comment=${params.commentId} ` + + `requested_reply=${params.replyId ?? "none"} match_source=${matchSource} ` + + `root=${safeJsonStringify({ reply_id: rootReply?.reply_id, text_len: extractReplyText(rootReply)?.length ?? 0 })} ` + + `target=${safeJsonStringify({ reply_id: targetReply?.reply_id, text_len: extractReplyText(targetReply)?.length ?? 0 })}`, + ); const meta = metaResponse?.code === 0 ? metaResponse.data?.metas?.[0] : undefined; return { documentTitle: meta?.title?.trim() || undefined, documentUrl: meta?.url?.trim() || undefined, + isWholeComment: commentCard?.is_whole, quoteText: commentCard?.quote?.trim() || undefined, rootCommentText: extractReplyText(rootReply), targetReplyText: extractReplyText(targetReply), @@ -376,6 +494,7 @@ function buildDriveCommentSurfacePrompt(params: { fileToken: string; commentId: string; replyId?: string; + isWholeComment?: boolean; isMentioned?: boolean; documentTitle?: string; documentUrl?: string; @@ -413,12 +532,16 @@ function buildDriveCommentSurfacePrompt(params: { `file_type: ${params.fileType}`, `comment_id: ${params.commentId}`, ); + if (params.isWholeComment === true) { + lines.push("This is a whole-document comment."); + } if (params.replyId?.trim()) { lines.push(`reply_id: ${params.replyId.trim()}`); } lines.push( "This is a Feishu document comment-thread event, not a Feishu IM conversation. Your final text reply will be posted automatically to the current comment thread and will not be sent as an instant message.", "If you need to inspect or handle the comment thread, prefer the feishu_drive tools: use list_comments / list_comment_replies to inspect comments, and use reply_comment/add_comment to notify the user after modifying the document.", + "Whole-document comments do not support direct replies. When the current comment is whole-document, use feishu_drive.add_comment for any user-visible follow-up instead of reply_comment.", 'If the comment asks you to modify document content, such as adding, inserting, replacing, or deleting text, tables, or headings, you must first use feishu_doc to actually modify the document. Do not reply with only "done", "I\'ll handle it", or a restated plan without calling tools.', 'If the comment quotes document content, that quoted text is usually the edit anchor. For requests like "insert xxx below this content", first locate the position around the quoted content, then use feishu_doc to make the change.', 'If the comment asks you to summarize, explain, rewrite, translate, refine, continue, or review the document content "below", "above", "this paragraph", "this section", or the quoted content, you must also treat the quoted content as the primary target anchor instead of defaulting to the whole document.', @@ -427,6 +550,11 @@ function buildDriveCommentSurfacePrompt(params: { "When document edits are involved, first use feishu_doc.read or feishu_doc.list_blocks to confirm the context, then use feishu_doc writing or updating capabilities to complete the change. After the edit succeeds, notify the user through feishu_drive.reply_comment.", "If the document edit fails or you cannot locate the anchor, do not pretend it succeeded. Reply clearly in the comment thread with the reason for failure or the missing information.", "If this is a reading-comprehension task, such as summarization, explanation, or extraction, you may directly output the final answer text after confirming the context. The system will automatically reply with that answer in the current comment thread.", + "Prefer plain text suitable for a comment thread. Unless the user explicitly asks for Markdown, do not use Markdown headings, bullet lists, numbered lists, tables, blockquotes, or fenced code blocks in the final reply.", + "If source content was read in Markdown form, rewrite it into normal plain-text prose before replying in the comment thread instead of copying Markdown syntax through.", + 'Do not include internal reasoning, analysis, chain-of-thought, scratch work, or any "Reasoning:" / "Thinking:" section in a user-visible reply. Output only the final answer meant for the user, or NO_REPLY when appropriate.', + 'Do not narrate your plan or execution process in the user-visible reply. Avoid meta lead-ins such as "I will...", "I’ll first...", "I need to...", "The user wants...", "I have updated...", or "I am going to...".', + "When the task is complete, reply only with the user-facing result itself, such as the final answer or a concise completion confirmation. Do not include preambles about what you plan to do next.", "When you produce a user-visible reply, keep it in the same language as the user's original comment or reply unless they explicitly ask for another language.", "If you have already completed the user-visible action through feishu_drive.reply_comment or feishu_drive.add_comment, output NO_REPLY at the end to avoid duplicate sending.", "If the user directly asks a question in the comment and a plain text answer is sufficient, output the answer text directly. The system will automatically reply with your final answer in the current comment thread.", @@ -443,6 +571,7 @@ async function resolveDriveCommentEventCore(params: ResolveDriveCommentEventPara noticeType: "add_comment" | "add_reply"; fileToken: string; fileType: CommentFileType; + isWholeComment?: boolean; senderId: string; senderUserId?: string; timestamp?: string; @@ -463,6 +592,7 @@ async function resolveDriveCommentEventCore(params: ResolveDriveCommentEventPara createClient = (account) => createFeishuClient(account) as FeishuRequestClient, verificationTimeoutMs = FEISHU_COMMENT_VERIFY_TIMEOUT_MS, logger, + waitMs = delayMs, } = params; const eventId = event.event_id?.trim(); const commentId = event.comment_id?.trim(); @@ -507,6 +637,7 @@ async function resolveDriveCommentEventCore(params: ResolveDriveCommentEventPara timeoutMs: verificationTimeoutMs, logger, accountId, + waitMs, }); return { eventId, @@ -515,6 +646,7 @@ async function resolveDriveCommentEventCore(params: ResolveDriveCommentEventPara noticeType, fileToken, fileType, + isWholeComment: context.isWholeComment, senderId, senderUserId, timestamp: event.timestamp, @@ -574,6 +706,7 @@ export async function resolveDriveCommentEventTurn( fileToken: resolved.fileToken, commentId: resolved.commentId, replyId: resolved.replyId, + isWholeComment: resolved.isWholeComment, isMentioned: resolved.isMentioned, documentTitle: resolved.context.documentTitle, documentUrl: resolved.context.documentUrl, @@ -590,6 +723,7 @@ export async function resolveDriveCommentEventTurn( noticeType: resolved.noticeType, fileToken: resolved.fileToken, fileType: resolved.fileType, + isWholeComment: resolved.isWholeComment, senderId: resolved.senderId, senderUserId: resolved.senderUserId, timestamp: resolved.timestamp,