Skip to content

Commit f9c0dc2

Browse files
fix(feishu): fall back from missing thread replies (#80306)
Summary: - The branch adds an opt-in Feishu top-level group-send fallback for withdrawn or missing normal quoted thread replies, plus regression coverage, a changelog entry, and CI/lint typing and baseline refreshes. - Reproducibility: yes. at source level. Current main hard-errors withdrawn/not-found Feishu reply targets when `replyInThread` is true, and the existing regression test asserts that no top-level create fallback occurs. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(feishu): fall back from missing thread replies - PR branch already contained follow-up commit before automerge: fix(clawsweeper): address review for automerge-openclaw-openclaw-8030… - PR branch already contained follow-up commit before automerge: fix(clawsweeper): reconcile automerge-openclaw-openclaw-80306 with ma… - PR branch already contained follow-up commit before automerge: fix(ci): satisfy stricter lint and test types - PR branch already contained follow-up commit before automerge: fix(ci): align Node 24 test typing Validation: - ClawSweeper review passed for head 93146f9. - Required merge gates passed before the squash merge. Prepared head SHA: 93146f9 Review: #80306 (comment) Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
1 parent 5af8fc0 commit f9c0dc2

29 files changed

Lines changed: 421 additions & 167 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Docs: https://docs.openclaw.ai
3636

3737
- Telegram: pass agent-scoped media roots through gateway message actions so workspace-local media from the active agent is not rejected as cross-agent access. Thanks @frankekn.
3838
- CLI/gateway: keep `gateway status --deep` plugin-aware so configured plugin manifest warnings, including missing channel config metadata, stay visible during install and update smoke checks.
39+
- Feishu: fall back to a top-level group send when normal group quoted replies target a withdrawn or missing message, preventing replies from disappearing silently while preserving native topic safety. Fixes #79349. Thanks @arlen8411.
3940
- Doctor: stop flagging the live compatibility agent directory as orphaned when the configured default agent is not `main`. Fixes #74313. (#74438) Thanks @carlos4s.
4041
- Auth/Claude CLI: persist fresher managed external CLI OAuth credentials back to `auth-profiles.json`, preventing stale `anthropic:claude-cli` profiles from repeatedly bootstrapping and flooding debug logs. Fixes #80129. Thanks @Caulderein.
4142
- Context: render `/context map` only from actual run context and persist Codex app-server run reports without counting deferred tool-search schemas as prompt-loaded tool schemas.

extensions/codex/src/app-server/run-attempt.context-engine.test.ts

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,16 @@ function requireRecord(value: unknown, label: string): Record<string, unknown> {
206206
return value as Record<string, unknown>;
207207
}
208208

209-
function requireFirstCallArg<T>(mock: unknown, label: string, _type?: (value: T) => T): T {
209+
function optionalString(value: unknown): string {
210+
return typeof value === "string" ? value : "";
211+
}
212+
213+
function requireFirstCallArg(mock: unknown, label: string): unknown {
210214
const call = (mock as MockCallReader).mock.calls.at(0);
211215
if (!call) {
212216
throw new Error(`expected ${label} to be called`);
213217
}
214-
return call[0] as T;
218+
return call[0];
215219
}
216220

217221
function requireRequestParams(
@@ -238,7 +242,7 @@ function expectRequestInputTextContains(
238242
expect(
239243
input.some((entry) => {
240244
const item = requireRecord(entry, "turn/start input entry");
241-
return item.type === "text" && typeof item.text === "string" && item.text.includes(expected);
245+
return item.type === "text" && optionalString(item.text).includes(expected);
242246
}),
243247
).toBe(true);
244248
}
@@ -275,18 +279,17 @@ describe("runCodexAppServerAttempt context-engine lifecycle", () => {
275279
throw new Error("expected bootstrap hook");
276280
}
277281
expect(contextEngine.bootstrap).toHaveBeenCalledTimes(1);
278-
const bootstrapParams = requireFirstCallArg<
279-
Parameters<NonNullable<ContextEngine["bootstrap"]>>[0]
280-
>(contextEngine.bootstrap, "bootstrap");
282+
const bootstrapParams = requireFirstCallArg(contextEngine.bootstrap, "bootstrap") as Parameters<
283+
NonNullable<ContextEngine["bootstrap"]>
284+
>[0];
281285
expect(bootstrapParams.sessionId).toBe("session-1");
282286
expect(bootstrapParams.sessionKey).toBe("agent:main:session-1");
283287
expect(bootstrapParams.sessionFile).toBe(sessionFile);
284288

285289
expect(contextEngine.assemble).toHaveBeenCalledTimes(1);
286-
const assembleParams = requireFirstCallArg<Parameters<ContextEngine["assemble"]>[0]>(
287-
contextEngine.assemble,
288-
"assemble",
289-
);
290+
const assembleParams = requireFirstCallArg(contextEngine.assemble, "assemble") as Parameters<
291+
ContextEngine["assemble"]
292+
>[0];
290293
expect(assembleParams.sessionId).toBe("session-1");
291294
expect(assembleParams.sessionKey).toBe("agent:main:session-1");
292295
expect(assembleParams.tokenBudget).toBe(321);
@@ -297,8 +300,7 @@ describe("runCodexAppServerAttempt context-engine lifecycle", () => {
297300
expect(assembleParams.availableTools).toEqual(new Set());
298301

299302
const threadStartParams = requireRequestParams(harness, "thread/start");
300-
const developerInstructions = threadStartParams.developerInstructions;
301-
expect(typeof developerInstructions === "string" ? developerInstructions : "").toContain(
303+
expect(optionalString(threadStartParams.developerInstructions)).toContain(
302304
"context-engine system",
303305
);
304306
expectRequestInputTextContains(harness, "OpenClaw assembled context for this turn:");
@@ -327,9 +329,9 @@ describe("runCodexAppServerAttempt context-engine lifecycle", () => {
327329
await run;
328330

329331
expect(afterTurn).toHaveBeenCalledTimes(1);
330-
const afterTurnCall = requireFirstCallArg<
331-
Parameters<NonNullable<ContextEngine["afterTurn"]>>[0]
332-
>(afterTurn, "afterTurn");
332+
const afterTurnCall = requireFirstCallArg(afterTurn, "afterTurn") as Parameters<
333+
NonNullable<ContextEngine["afterTurn"]>
334+
>[0];
333335
expect(afterTurnCall.sessionId).toBe("session-1");
334336
expect(afterTurnCall.sessionKey).toBe("agent:main:session-1");
335337
expect(afterTurnCall.prePromptMessageCount).toBe(0);
@@ -370,17 +372,16 @@ describe("runCodexAppServerAttempt context-engine lifecycle", () => {
370372
await harness.completeTurn();
371373
await run;
372374

373-
const assembleParams = requireFirstCallArg<Parameters<ContextEngine["assemble"]>[0]>(
374-
contextEngine.assemble,
375-
"assemble",
376-
);
375+
const assembleParams = requireFirstCallArg(contextEngine.assemble, "assemble") as Parameters<
376+
ContextEngine["assemble"]
377+
>[0];
377378
expect(assembleParams.messages.map((message) => message.role)).toEqual([
378379
"assistant",
379380
"assistant",
380381
]);
381-
const afterTurnParams = requireFirstCallArg<
382-
Parameters<NonNullable<ContextEngine["afterTurn"]>>[0]
383-
>(afterTurn, "afterTurn");
382+
const afterTurnParams = requireFirstCallArg(afterTurn, "afterTurn") as Parameters<
383+
NonNullable<ContextEngine["afterTurn"]>
384+
>[0];
384385
expect(afterTurnParams.prePromptMessageCount).toBe(2);
385386
expectRequestInputTextContains(harness, "bootstrap context");
386387
});

extensions/diffs/src/browser.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ describe("PlaywrightDiffScreenshotter", () => {
8181

8282
expect(launchMock).toHaveBeenCalledTimes(1);
8383
expect(browser.newPage).toHaveBeenCalledTimes(2);
84-
const firstPageParams = browser.newPage.mock.calls[0]?.[0] as
85-
| { deviceScaleFactor?: number }
86-
| undefined;
84+
const firstPageParams = (
85+
browser.newPage.mock.calls as Array<[{ deviceScaleFactor?: number }?]>
86+
)[0]?.[0];
8787
expect(firstPageParams?.deviceScaleFactor).toBe(2);
8888
expect(pages).toHaveLength(2);
8989
expect(pages[0]?.close).toHaveBeenCalledTimes(1);

extensions/discord/src/monitor/provider.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,10 @@ function createConfigWithDiscordAccount(overrides: Record<string, unknown> = {})
105105
type MockCallReader = { mock: { calls: unknown[][] } };
106106

107107
function mockMessages(mock: unknown): string[] {
108-
return (mock as MockCallReader).mock.calls.map((call) =>
109-
typeof call[0] === "string" ? call[0] : "",
110-
);
108+
return (mock as MockCallReader).mock.calls.map((call) => {
109+
const message = call[0];
110+
return typeof message === "string" ? message : "";
111+
});
111112
}
112113

113114
function expectMockLogContains(mock: unknown, expected: string): void {

extensions/feishu/src/reply-dispatcher.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,38 @@ describe("createFeishuReplyDispatcher streaming behavior", () => {
926926
});
927927
});
928928

929+
it("allows top-level fallback for normal group quoted replies", async () => {
930+
const { options } = createDispatcherHarness({
931+
replyToMessageId: "om_quote_reply",
932+
replyInThread: true,
933+
threadReply: true,
934+
rootId: "om_original_msg",
935+
});
936+
await options.deliver({ text: "plain text" }, { kind: "final" });
937+
938+
expectMockArgFields(sendMessageFeishuMock, "message send params", {
939+
replyToMessageId: "om_quote_reply",
940+
replyInThread: true,
941+
allowTopLevelReplyFallback: true,
942+
});
943+
});
944+
945+
it("keeps native topic replies opted out of top-level fallback", async () => {
946+
const { options } = createDispatcherHarness({
947+
replyToMessageId: "om_topic_root",
948+
replyInThread: true,
949+
threadReply: true,
950+
rootId: "om_topic_root",
951+
});
952+
await options.deliver({ text: "plain text" }, { kind: "final" });
953+
954+
expectMockArgFields(sendMessageFeishuMock, "message send params", {
955+
replyToMessageId: "om_topic_root",
956+
replyInThread: true,
957+
allowTopLevelReplyFallback: false,
958+
});
959+
});
960+
929961
it("passes replyInThread to sendStructuredCardFeishu for card text", async () => {
930962
resolveFeishuAccountMock.mockReturnValue({
931963
accountId: "main",

extensions/feishu/src/reply-dispatcher.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,12 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP
147147
const sendReplyToMessageId = skipReplyToInMessages ? undefined : replyToMessageId;
148148
const threadReplyMode = threadReply === true;
149149
const effectiveReplyInThread = threadReplyMode ? true : replyInThread;
150+
const allowTopLevelReplyFallback =
151+
effectiveReplyInThread === true &&
152+
threadReplyMode &&
153+
rootId !== undefined &&
154+
sendReplyToMessageId !== undefined &&
155+
sendReplyToMessageId !== rootId;
150156
const account = resolveFeishuRuntimeAccount({ cfg, accountId });
151157
const prefixContext = createReplyPrefixContext({ cfg, agentId });
152158

@@ -465,6 +471,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP
465471
text: chunk,
466472
replyToMessageId: sendReplyToMessageId,
467473
replyInThread: effectiveReplyInThread,
474+
allowTopLevelReplyFallback,
468475
accountId,
469476
});
470477
},
@@ -491,6 +498,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP
491498
text: chunk,
492499
replyToMessageId: sendReplyToMessageId,
493500
replyInThread: effectiveReplyInThread,
501+
allowTopLevelReplyFallback,
494502
accountId,
495503
});
496504
},
@@ -605,6 +613,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP
605613
text: chunk,
606614
replyToMessageId: sendReplyToMessageId,
607615
replyInThread: effectiveReplyInThread,
616+
allowTopLevelReplyFallback,
608617
accountId,
609618
header: cardHeader,
610619
note: cardNote,
@@ -623,6 +632,7 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP
623632
text: chunk,
624633
replyToMessageId: sendReplyToMessageId,
625634
replyInThread: effectiveReplyInThread,
635+
allowTopLevelReplyFallback,
626636
accountId,
627637
});
628638
},

extensions/feishu/src/send.reply-fallback.test.ts

Lines changed: 124 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,91 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => {
211211
expect(createMock).not.toHaveBeenCalled();
212212
});
213213

214-
it("fails thread replies instead of falling back to a top-level send", async () => {
214+
it("falls back to a top-level group send when normal quoted replies target withdrawn messages", async () => {
215+
resolveFeishuSendTargetMock.mockReturnValue({
216+
client: {
217+
im: {
218+
message: {
219+
reply: replyMock,
220+
create: createMock,
221+
},
222+
},
223+
},
224+
receiveId: "oc_group_1",
225+
receiveIdType: "chat_id",
226+
});
227+
replyMock.mockResolvedValue({
228+
code: 230011,
229+
msg: "The message was withdrawn.",
230+
});
231+
createMock.mockResolvedValue({
232+
code: 0,
233+
data: { message_id: "om_group_fallback" },
234+
});
235+
236+
await expectFallbackResult(
237+
() =>
238+
sendMessageFeishu({
239+
cfg: {} as never,
240+
to: "chat:oc_group_1",
241+
text: "hello",
242+
replyToMessageId: "om_parent",
243+
replyInThread: true,
244+
allowTopLevelReplyFallback: true,
245+
}),
246+
"om_group_fallback",
247+
);
248+
249+
expect(replyMock).toHaveBeenCalledWith({
250+
path: { message_id: "om_parent" },
251+
data: expect.objectContaining({
252+
reply_in_thread: true,
253+
}),
254+
});
255+
expect(createMock).toHaveBeenCalledWith({
256+
params: { receive_id_type: "chat_id" },
257+
data: expect.objectContaining({
258+
receive_id: "oc_group_1",
259+
msg_type: "post",
260+
}),
261+
});
262+
});
263+
264+
it("falls back to create when normal quoted replies throw withdrawn errors", async () => {
265+
resolveFeishuSendTargetMock.mockReturnValue({
266+
client: {
267+
im: {
268+
message: {
269+
reply: replyMock,
270+
create: createMock,
271+
},
272+
},
273+
},
274+
receiveId: "oc_group_1",
275+
receiveIdType: "chat_id",
276+
});
277+
const sdkError = Object.assign(new Error("request failed"), { code: 230011 });
278+
replyMock.mockRejectedValue(sdkError);
279+
createMock.mockResolvedValue({
280+
code: 0,
281+
data: { message_id: "om_thrown_thread_fallback" },
282+
});
283+
284+
await expectFallbackResult(
285+
() =>
286+
sendMessageFeishu({
287+
cfg: {} as never,
288+
to: "chat:oc_group_1",
289+
text: "hello",
290+
replyToMessageId: "om_parent",
291+
replyInThread: true,
292+
allowTopLevelReplyFallback: true,
293+
}),
294+
"om_thrown_thread_fallback",
295+
);
296+
});
297+
298+
it("fails native thread replies instead of falling back to a top-level send", async () => {
215299
replyMock.mockResolvedValue({
216300
code: 230011,
217301
msg: "The message was withdrawn.",
@@ -230,15 +314,9 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => {
230314
);
231315

232316
expect(createMock).not.toHaveBeenCalled();
233-
expect(replyMock).toHaveBeenCalledWith({
234-
path: { message_id: "om_parent" },
235-
data: expect.objectContaining({
236-
reply_in_thread: true,
237-
}),
238-
});
239317
});
240318

241-
it("fails thrown withdrawn thread replies instead of falling back to create", async () => {
319+
it("fails thrown withdrawn native thread replies instead of falling back to create", async () => {
242320
const sdkError = Object.assign(new Error("request failed"), { code: 230011 });
243321
replyMock.mockRejectedValue(sdkError);
244322

@@ -257,6 +335,44 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => {
257335
expect(createMock).not.toHaveBeenCalled();
258336
});
259337

338+
it("preserves non-withdrawn thread reply failures", async () => {
339+
replyMock.mockResolvedValue({
340+
code: 999999,
341+
msg: "unknown failure",
342+
});
343+
344+
await expect(
345+
sendMessageFeishu({
346+
cfg: {} as never,
347+
to: "chat:oc_group_1",
348+
text: "hello",
349+
replyToMessageId: "om_parent",
350+
replyInThread: true,
351+
allowTopLevelReplyFallback: true,
352+
}),
353+
).rejects.toThrow("Feishu reply failed");
354+
355+
expect(createMock).not.toHaveBeenCalled();
356+
});
357+
358+
it("preserves thrown non-withdrawn thread reply failures", async () => {
359+
const sdkError = Object.assign(new Error("rate limited"), { code: 99991400 });
360+
replyMock.mockRejectedValue(sdkError);
361+
362+
await expect(
363+
sendMessageFeishu({
364+
cfg: {} as never,
365+
to: "chat:oc_group_1",
366+
text: "hello",
367+
replyToMessageId: "om_parent",
368+
replyInThread: true,
369+
allowTopLevelReplyFallback: true,
370+
}),
371+
).rejects.toThrow("rate limited");
372+
373+
expect(createMock).not.toHaveBeenCalled();
374+
});
375+
260376
it("still falls back for non-thread replies to withdrawn targets", async () => {
261377
replyMock.mockResolvedValue({
262378
code: 230011,

0 commit comments

Comments
 (0)