Skip to content

Commit ebb72ba

Browse files
authored
feat(feishu): improve document comment session, rich parsing, and typing feedback (openclaw#63785)
* Feishu: upgrade comment session, context parsing, and typing reaction * test(feishu): align comment prompt assertions
1 parent 2c57ec7 commit ebb72ba

15 files changed

Lines changed: 2737 additions & 148 deletions
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const resolveFeishuRuntimeAccountMock = vi.hoisted(() => vi.fn());
4+
const createFeishuClientMock = vi.hoisted(() => vi.fn());
5+
const createReplyPrefixContextMock = vi.hoisted(() => vi.fn());
6+
const createCommentTypingReactionLifecycleMock = vi.hoisted(() => vi.fn());
7+
const deliverCommentThreadTextMock = vi.hoisted(() => vi.fn());
8+
const createReplyDispatcherWithTypingMock = vi.hoisted(() => vi.fn());
9+
const getFeishuRuntimeMock = vi.hoisted(() => vi.fn());
10+
11+
vi.mock("./accounts.js", () => ({
12+
resolveFeishuRuntimeAccount: resolveFeishuRuntimeAccountMock,
13+
}));
14+
15+
vi.mock("./client.js", () => ({
16+
createFeishuClient: createFeishuClientMock,
17+
}));
18+
19+
vi.mock("./comment-dispatcher-runtime-api.js", () => ({
20+
createReplyPrefixContext: createReplyPrefixContextMock,
21+
}));
22+
23+
vi.mock("./comment-reaction.js", () => ({
24+
createCommentTypingReactionLifecycle: createCommentTypingReactionLifecycleMock,
25+
}));
26+
27+
vi.mock("./drive.js", () => ({
28+
deliverCommentThreadText: deliverCommentThreadTextMock,
29+
}));
30+
31+
vi.mock("./runtime.js", () => ({
32+
getFeishuRuntime: getFeishuRuntimeMock,
33+
}));
34+
35+
import { createFeishuCommentReplyDispatcher } from "./comment-dispatcher.js";
36+
37+
describe("createFeishuCommentReplyDispatcher", () => {
38+
beforeEach(() => {
39+
vi.clearAllMocks();
40+
resolveFeishuRuntimeAccountMock.mockReturnValue({
41+
accountId: "main",
42+
appId: "app_id",
43+
appSecret: "app_secret",
44+
domain: "feishu",
45+
config: {},
46+
});
47+
createFeishuClientMock.mockReturnValue({});
48+
createReplyPrefixContextMock.mockReturnValue({
49+
responsePrefix: undefined,
50+
responsePrefixContextProvider: undefined,
51+
});
52+
deliverCommentThreadTextMock.mockResolvedValue({
53+
delivery_mode: "reply_comment",
54+
reply_id: "reply_1",
55+
});
56+
createCommentTypingReactionLifecycleMock.mockReturnValue({
57+
start: vi.fn(async () => {}),
58+
cleanup: vi.fn(async () => {}),
59+
});
60+
createReplyDispatcherWithTypingMock.mockImplementation(() => ({
61+
dispatcher: {
62+
markComplete: vi.fn(),
63+
waitForIdle: vi.fn(async () => {}),
64+
},
65+
replyOptions: {},
66+
markDispatchIdle: vi.fn(),
67+
markRunComplete: vi.fn(),
68+
}));
69+
getFeishuRuntimeMock.mockReturnValue({
70+
channel: {
71+
text: {
72+
resolveTextChunkLimit: vi.fn(() => 4000),
73+
resolveChunkMode: vi.fn(() => "line"),
74+
chunkTextWithMode: vi.fn((text: string) => [text]),
75+
},
76+
reply: {
77+
createReplyDispatcherWithTyping: createReplyDispatcherWithTypingMock,
78+
resolveHumanDelayConfig: vi.fn(() => undefined),
79+
},
80+
},
81+
});
82+
});
83+
84+
it("sends final comment text without waiting for typing cleanup", async () => {
85+
let resolveCleanup: (() => void) | undefined;
86+
const cleanup = vi.fn(
87+
() =>
88+
new Promise<void>((resolve) => {
89+
resolveCleanup = resolve;
90+
}),
91+
);
92+
createCommentTypingReactionLifecycleMock.mockReturnValue({
93+
start: vi.fn(async () => {}),
94+
cleanup,
95+
});
96+
97+
createFeishuCommentReplyDispatcher({
98+
cfg: {} as never,
99+
agentId: "main",
100+
runtime: { log: vi.fn(), error: vi.fn() } as never,
101+
accountId: "main",
102+
fileToken: "doc_token_1",
103+
fileType: "docx",
104+
commentId: "comment_1",
105+
replyId: "reply_1",
106+
isWholeComment: false,
107+
});
108+
109+
const options = createReplyDispatcherWithTypingMock.mock.calls.at(-1)?.[0];
110+
const deliverPromise = options.deliver({ text: "hello world" }, { kind: "final" });
111+
const status = await Promise.race([
112+
deliverPromise.then(() => "done"),
113+
new Promise<string>((resolve) => setTimeout(() => resolve("pending"), 0)),
114+
]);
115+
116+
expect(status).toBe("done");
117+
expect(deliverCommentThreadTextMock).toHaveBeenCalledWith(
118+
expect.anything(),
119+
expect.objectContaining({
120+
file_token: "doc_token_1",
121+
file_type: "docx",
122+
comment_id: "comment_1",
123+
content: "hello world",
124+
is_whole_comment: false,
125+
}),
126+
);
127+
expect(cleanup).not.toHaveBeenCalled();
128+
129+
options.onCleanup?.();
130+
expect(cleanup).toHaveBeenCalledTimes(1);
131+
132+
resolveCleanup?.();
133+
await deliverPromise;
134+
});
135+
136+
it("starts the typing reaction from dispatcher onReplyStart", async () => {
137+
const start = vi.fn(async () => {});
138+
createCommentTypingReactionLifecycleMock.mockReturnValue({
139+
start,
140+
cleanup: vi.fn(async () => {}),
141+
});
142+
143+
createFeishuCommentReplyDispatcher({
144+
cfg: {} as never,
145+
agentId: "main",
146+
runtime: { log: vi.fn(), error: vi.fn() } as never,
147+
accountId: "main",
148+
fileToken: "doc_token_1",
149+
fileType: "docx",
150+
commentId: "comment_1",
151+
replyId: "reply_1",
152+
isWholeComment: false,
153+
});
154+
155+
const options = createReplyDispatcherWithTypingMock.mock.calls.at(-1)?.[0];
156+
await options.onReplyStart?.();
157+
158+
expect(start).toHaveBeenCalledTimes(1);
159+
});
160+
});

extensions/feishu/src/comment-dispatcher.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type ReplyPayload,
88
type RuntimeEnv,
99
} from "./comment-dispatcher-runtime-api.js";
10+
import { createCommentTypingReactionLifecycle } from "./comment-reaction.js";
1011
import type { CommentFileType } from "./comment-target.js";
1112
import { deliverCommentThreadText } from "./drive.js";
1213
import { getFeishuRuntime } from "./runtime.js";
@@ -19,6 +20,7 @@ export type CreateFeishuCommentReplyDispatcherParams = {
1920
fileToken: string;
2021
fileType: CommentFileType;
2122
commentId: string;
23+
replyId?: string;
2224
isWholeComment?: boolean;
2325
};
2426

@@ -43,12 +45,23 @@ export function createFeishuCommentReplyDispatcher(
4345
},
4446
);
4547
const chunkMode = core.channel.text.resolveChunkMode(params.cfg, "feishu");
48+
const typingReaction = createCommentTypingReactionLifecycle({
49+
cfg: params.cfg,
50+
fileToken: params.fileToken,
51+
fileType: params.fileType,
52+
replyId: params.replyId,
53+
accountId: params.accountId,
54+
runtime: params.runtime,
55+
});
4656

47-
const { dispatcher, replyOptions, markDispatchIdle } =
57+
const { dispatcher, replyOptions, markDispatchIdle, markRunComplete } =
4858
core.channel.reply.createReplyDispatcherWithTyping({
4959
responsePrefix: prefixContext.responsePrefix,
5060
responsePrefixContextProvider: prefixContext.responsePrefixContextProvider,
5161
humanDelay: core.channel.reply.resolveHumanDelayConfig(params.cfg, params.agentId),
62+
onReplyStart: async () => {
63+
await typingReaction.start();
64+
},
5265
deliver: async (payload: ReplyPayload, info) => {
5366
if (info.kind !== "final") {
5467
return;
@@ -78,7 +91,17 @@ export function createFeishuCommentReplyDispatcher(
7891
`feishu[${params.accountId ?? "default"}]: comment dispatcher failed kind=${info.kind} comment=${params.commentId}: ${String(err)}`,
7992
);
8093
},
94+
onCleanup: () => {
95+
void typingReaction.cleanup();
96+
},
8197
});
8298

83-
return { dispatcher, replyOptions, markDispatchIdle };
99+
return {
100+
dispatcher,
101+
replyOptions,
102+
markDispatchIdle,
103+
markRunComplete,
104+
startTypingReaction: typingReaction.start,
105+
cleanupTypingReaction: typingReaction.cleanup,
106+
};
84107
}

extensions/feishu/src/comment-handler.test.ts

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,9 @@ describe("handleFeishuCommentEvent", () => {
164164
},
165165
replyOptions: {},
166166
markDispatchIdle: vi.fn(),
167+
markRunComplete: vi.fn(),
168+
startTypingReaction: vi.fn(async () => {}),
169+
cleanupTypingReaction: vi.fn(async () => {}),
167170
});
168171
});
169172

@@ -198,9 +201,15 @@ describe("handleFeishuCommentEvent", () => {
198201
OriginatingChannel: "feishu",
199202
OriginatingTo: "comment:docx:doc_token_1:comment_1",
200203
MessageSid: "drive-comment:evt_1",
204+
MessageThreadId: "reply_1",
201205
}),
202206
);
203207
expect(recordInboundSession).toHaveBeenCalledTimes(1);
208+
expect(recordInboundSession).toHaveBeenCalledWith(
209+
expect.objectContaining({
210+
sessionKey: "agent:main:feishu:direct:comment-doc:docx:doc_token_1",
211+
}),
212+
);
204213
expect(dispatchReplyFromConfig).toHaveBeenCalledTimes(1);
205214
});
206215

@@ -309,8 +318,124 @@ describe("handleFeishuCommentEvent", () => {
309318
commentId: "comment_whole",
310319
fileToken: "doc_token_1",
311320
fileType: "docx",
321+
replyId: "reply_whole",
312322
isWholeComment: true,
313323
}),
314324
);
315325
});
326+
327+
it("always finalizes comment typing cleanup even when dispatch fails", async () => {
328+
const dispatchReplyFromConfig = vi.fn(async () => {
329+
throw new Error("dispatch failed");
330+
});
331+
const runtime = createTestRuntime({ dispatchReplyFromConfig });
332+
setFeishuRuntime(runtime);
333+
const markRunComplete = vi.fn();
334+
const markDispatchIdle = vi.fn();
335+
const cleanupTypingReaction = vi.fn(async () => {});
336+
createFeishuCommentReplyDispatcherMock.mockReturnValue({
337+
dispatcher: {
338+
markComplete: vi.fn(),
339+
waitForIdle: vi.fn(async () => {}),
340+
},
341+
replyOptions: {},
342+
markDispatchIdle,
343+
markRunComplete,
344+
startTypingReaction: vi.fn(async () => {}),
345+
cleanupTypingReaction,
346+
});
347+
348+
await expect(
349+
handleFeishuCommentEvent({
350+
cfg: buildConfig(),
351+
accountId: "default",
352+
event: { event_id: "evt_1" },
353+
botOpenId: "ou_bot",
354+
runtime: {
355+
log: vi.fn(),
356+
error: vi.fn(),
357+
} as never,
358+
}),
359+
).rejects.toThrow("dispatch failed");
360+
361+
expect(markRunComplete).toHaveBeenCalledTimes(1);
362+
expect(markDispatchIdle).toHaveBeenCalledTimes(1);
363+
expect(cleanupTypingReaction).toHaveBeenCalledTimes(1);
364+
});
365+
366+
it("does not wait for comment typing cleanup before returning", async () => {
367+
let resolveCleanup: (() => void) | undefined;
368+
const cleanupTypingReaction = vi.fn(
369+
() =>
370+
new Promise<void>((resolve) => {
371+
resolveCleanup = resolve;
372+
}),
373+
);
374+
createFeishuCommentReplyDispatcherMock.mockReturnValue({
375+
dispatcher: {
376+
markComplete: vi.fn(),
377+
waitForIdle: vi.fn(async () => {}),
378+
},
379+
replyOptions: {},
380+
markDispatchIdle: vi.fn(),
381+
markRunComplete: vi.fn(),
382+
startTypingReaction: vi.fn(async () => {}),
383+
cleanupTypingReaction,
384+
});
385+
386+
const eventPromise = handleFeishuCommentEvent({
387+
cfg: buildConfig(),
388+
accountId: "default",
389+
event: { event_id: "evt_1" },
390+
botOpenId: "ou_bot",
391+
runtime: {
392+
log: vi.fn(),
393+
error: vi.fn(),
394+
} as never,
395+
});
396+
397+
const status = await Promise.race([
398+
eventPromise.then(() => "done"),
399+
new Promise<string>((resolve) => setTimeout(() => resolve("pending"), 0)),
400+
]);
401+
402+
expect(status).toBe("done");
403+
expect(cleanupTypingReaction).toHaveBeenCalledTimes(1);
404+
405+
resolveCleanup?.();
406+
await eventPromise;
407+
});
408+
409+
it("does not start comment typing reaction before dispatch begins", async () => {
410+
const startTypingReaction = vi.fn(async () => {});
411+
createFeishuCommentReplyDispatcherMock.mockReturnValue({
412+
dispatcher: {
413+
markComplete: vi.fn(),
414+
waitForIdle: vi.fn(async () => {}),
415+
},
416+
replyOptions: {},
417+
markDispatchIdle: vi.fn(),
418+
markRunComplete: vi.fn(),
419+
startTypingReaction,
420+
cleanupTypingReaction: vi.fn(async () => {}),
421+
});
422+
423+
await handleFeishuCommentEvent({
424+
cfg: buildConfig(),
425+
accountId: "default",
426+
event: { event_id: "evt_1" },
427+
botOpenId: "ou_bot",
428+
runtime: {
429+
log: vi.fn(),
430+
error: vi.fn(),
431+
} as never,
432+
});
433+
434+
expect(startTypingReaction).not.toHaveBeenCalled();
435+
const runtime = (await import("./runtime.js")).getFeishuRuntime();
436+
const dispatchReplyFromConfig = runtime.channel.reply.dispatchReplyFromConfig as ReturnType<
437+
typeof vi.fn
438+
>;
439+
expect(dispatchReplyFromConfig).toHaveBeenCalledTimes(1);
440+
});
316441
});

0 commit comments

Comments
 (0)