fix(feishu): harden target routing, dedupe, and reply fallback

This commit is contained in:
Peter Steinberger
2026-03-02 03:41:07 +00:00
parent 77ccd35e5e
commit 2a252a14cc
11 changed files with 371 additions and 14 deletions

View File

@@ -1188,6 +1188,38 @@ describe("handleFeishuMessage command authorization", () => {
}),
);
});
it("does not dispatch twice for the same image message_id (concurrent dedupe)", async () => {
mockShouldComputeCommandAuthorized.mockReturnValue(false);
const cfg: ClawdbotConfig = {
channels: {
feishu: {
dmPolicy: "open",
},
},
} as ClawdbotConfig;
const event: FeishuMessageEvent = {
sender: {
sender_id: {
open_id: "ou-image-dedup",
},
},
message: {
message_id: "msg-image-dedup",
chat_id: "oc-dm",
chat_type: "p2p",
message_type: "image",
content: JSON.stringify({
image_key: "img_dedup_payload",
}),
},
};
await Promise.all([dispatchMessage({ cfg, event }), dispatchMessage({ cfg, event })]);
expect(mockDispatchReplyFromConfig).toHaveBeenCalledTimes(1);
});
});
describe("toMessageResourceType", () => {

View File

@@ -13,7 +13,7 @@ import {
} from "openclaw/plugin-sdk";
import { resolveFeishuAccount } from "./accounts.js";
import { createFeishuClient } from "./client.js";
import { tryRecordMessagePersistent } from "./dedup.js";
import { tryRecordMessage, tryRecordMessagePersistent } from "./dedup.js";
import { maybeCreateDynamicAgent } from "./dynamic-agent.js";
import { normalizeFeishuExternalKey } from "./external-keys.js";
import { downloadMessageResourceFeishu } from "./media.js";
@@ -692,8 +692,15 @@ export async function handleFeishuMessage(params: {
const log = runtime?.log ?? console.log;
const error = runtime?.error ?? console.error;
// Dedup check: skip if this message was already processed (memory + disk).
// Dedup: synchronous memory guard prevents concurrent duplicate dispatch
// before the async persistent check completes.
const messageId = event.message.message_id;
const memoryDedupeKey = `${account.accountId}:${messageId}`;
if (!tryRecordMessage(memoryDedupeKey)) {
log(`feishu: skipping duplicate message ${messageId} (memory dedup)`);
return;
}
// Persistent dedup survives restarts and reconnects.
if (!(await tryRecordMessagePersistent(messageId, account.accountId, log))) {
log(`feishu: skipping duplicate message ${messageId}`);
return;

View File

@@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
const sendMediaFeishuMock = vi.hoisted(() => vi.fn());
const sendMessageFeishuMock = vi.hoisted(() => vi.fn());
const sendMarkdownCardFeishuMock = vi.hoisted(() => vi.fn());
vi.mock("./media.js", () => ({
sendMediaFeishu: sendMediaFeishuMock,
@@ -12,6 +13,7 @@ vi.mock("./media.js", () => ({
vi.mock("./send.js", () => ({
sendMessageFeishu: sendMessageFeishuMock,
sendMarkdownCardFeishu: sendMarkdownCardFeishuMock,
}));
vi.mock("./runtime.js", () => ({
@@ -31,6 +33,7 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
beforeEach(() => {
vi.clearAllMocks();
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
});
@@ -108,4 +111,71 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
await fs.rm(dir, { recursive: true, force: true });
}
});
it("uses markdown cards when renderMode=card", async () => {
const result = await sendText({
cfg: {
channels: {
feishu: {
renderMode: "card",
},
},
} as any,
to: "chat_1",
text: "| a | b |\n| - | - |",
accountId: "main",
});
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
expect.objectContaining({
to: "chat_1",
text: "| a | b |\n| - | - |",
accountId: "main",
}),
);
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(result).toEqual(expect.objectContaining({ channel: "feishu", messageId: "card_msg" }));
});
});
describe("feishuOutbound.sendMedia renderMode", () => {
beforeEach(() => {
vi.clearAllMocks();
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
});
it("uses markdown cards for captions when renderMode=card", async () => {
const result = await feishuOutbound.sendMedia?.({
cfg: {
channels: {
feishu: {
renderMode: "card",
},
},
} as any,
to: "chat_1",
text: "| a | b |\n| - | - |",
mediaUrl: "https://example.com/image.png",
accountId: "main",
});
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
expect.objectContaining({
to: "chat_1",
text: "| a | b |\n| - | - |",
accountId: "main",
}),
);
expect(sendMediaFeishuMock).toHaveBeenCalledWith(
expect.objectContaining({
to: "chat_1",
mediaUrl: "https://example.com/image.png",
accountId: "main",
}),
);
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(result).toEqual(expect.objectContaining({ channel: "feishu", messageId: "media_msg" }));
});
});

View File

@@ -1,9 +1,10 @@
import fs from "fs";
import path from "path";
import type { ChannelOutboundAdapter } from "openclaw/plugin-sdk";
import { resolveFeishuAccount } from "./accounts.js";
import { sendMediaFeishu } from "./media.js";
import { getFeishuRuntime } from "./runtime.js";
import { sendMessageFeishu } from "./send.js";
import { sendMarkdownCardFeishu, sendMessageFeishu } from "./send.js";
function normalizePossibleLocalImagePath(text: string | undefined): string | null {
const raw = text?.trim();
@@ -38,6 +39,27 @@ function normalizePossibleLocalImagePath(text: string | undefined): string | nul
return raw;
}
function shouldUseCard(text: string): boolean {
return /```[\s\S]*?```/.test(text) || /\|.+\|[\r\n]+\|[-:| ]+\|/.test(text);
}
async function sendOutboundText(params: {
cfg: Parameters<typeof sendMessageFeishu>[0]["cfg"];
to: string;
text: string;
accountId?: string;
}) {
const { cfg, to, text, accountId } = params;
const account = resolveFeishuAccount({ cfg, accountId });
const renderMode = account.config?.renderMode ?? "auto";
if (renderMode === "card" || (renderMode === "auto" && shouldUseCard(text))) {
return sendMarkdownCardFeishu({ cfg, to, text, accountId });
}
return sendMessageFeishu({ cfg, to, text, accountId });
}
export const feishuOutbound: ChannelOutboundAdapter = {
deliveryMode: "direct",
chunker: (text, limit) => getFeishuRuntime().channel.text.chunkMarkdownText(text, limit),
@@ -63,13 +85,23 @@ export const feishuOutbound: ChannelOutboundAdapter = {
}
}
const result = await sendMessageFeishu({ cfg, to, text, accountId: accountId ?? undefined });
const result = await sendOutboundText({
cfg,
to,
text,
accountId: accountId ?? undefined,
});
return { channel: "feishu", ...result };
},
sendMedia: async ({ cfg, to, text, mediaUrl, accountId, mediaLocalRoots }) => {
// Send text first if provided
if (text?.trim()) {
await sendMessageFeishu({ cfg, to, text, accountId: accountId ?? undefined });
await sendOutboundText({
cfg,
to,
text,
accountId: accountId ?? undefined,
});
}
// Upload and send media if URL or local path provided
@@ -88,7 +120,7 @@ export const feishuOutbound: ChannelOutboundAdapter = {
console.error(`[feishu] sendMediaFeishu failed:`, err);
// Fallback to URL link if upload fails
const fallbackText = `📎 ${mediaUrl}`;
const result = await sendMessageFeishu({
const result = await sendOutboundText({
cfg,
to,
text: fallbackText,
@@ -99,7 +131,7 @@ export const feishuOutbound: ChannelOutboundAdapter = {
}
// No media URL, just return text result
const result = await sendMessageFeishu({
const result = await sendOutboundText({
cfg,
to,
text: text ?? "",

View File

@@ -0,0 +1,105 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
const resolveFeishuSendTargetMock = vi.hoisted(() => vi.fn());
const resolveMarkdownTableModeMock = vi.hoisted(() => vi.fn(() => "preserve"));
const convertMarkdownTablesMock = vi.hoisted(() => vi.fn((text: string) => text));
vi.mock("./send-target.js", () => ({
resolveFeishuSendTarget: resolveFeishuSendTargetMock,
}));
vi.mock("./runtime.js", () => ({
getFeishuRuntime: () => ({
channel: {
text: {
resolveMarkdownTableMode: resolveMarkdownTableModeMock,
convertMarkdownTables: convertMarkdownTablesMock,
},
},
}),
}));
import { sendCardFeishu, sendMessageFeishu } from "./send.js";
describe("Feishu reply fallback for withdrawn/deleted targets", () => {
const replyMock = vi.fn();
const createMock = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
resolveFeishuSendTargetMock.mockReturnValue({
client: {
im: {
message: {
reply: replyMock,
create: createMock,
},
},
},
receiveId: "ou_target",
receiveIdType: "open_id",
});
});
it("falls back to create for withdrawn post replies", async () => {
replyMock.mockResolvedValue({
code: 230011,
msg: "The message was withdrawn.",
});
createMock.mockResolvedValue({
code: 0,
data: { message_id: "om_new" },
});
const result = await sendMessageFeishu({
cfg: {} as never,
to: "user:ou_target",
text: "hello",
replyToMessageId: "om_parent",
});
expect(replyMock).toHaveBeenCalledTimes(1);
expect(createMock).toHaveBeenCalledTimes(1);
expect(result.messageId).toBe("om_new");
});
it("falls back to create for withdrawn card replies", async () => {
replyMock.mockResolvedValue({
code: 231003,
msg: "The message is not found",
});
createMock.mockResolvedValue({
code: 0,
data: { message_id: "om_card_new" },
});
const result = await sendCardFeishu({
cfg: {} as never,
to: "user:ou_target",
card: { schema: "2.0" },
replyToMessageId: "om_parent",
});
expect(replyMock).toHaveBeenCalledTimes(1);
expect(createMock).toHaveBeenCalledTimes(1);
expect(result.messageId).toBe("om_card_new");
});
it("still throws for non-withdrawn reply failures", async () => {
replyMock.mockResolvedValue({
code: 999999,
msg: "unknown failure",
});
await expect(
sendMessageFeishu({
cfg: {} as never,
to: "user:ou_target",
text: "hello",
replyToMessageId: "om_parent",
}),
).rejects.toThrow("Feishu reply failed");
expect(createMock).not.toHaveBeenCalled();
});
});

View File

@@ -9,6 +9,16 @@ import { assertFeishuMessageApiSuccess, toFeishuSendResult } from "./send-result
import { resolveFeishuSendTarget } from "./send-target.js";
import type { FeishuSendResult } from "./types.js";
const WITHDRAWN_REPLY_ERROR_CODES = new Set([230011, 231003]);
function shouldFallbackFromReplyTarget(response: { code?: number; msg?: string }): boolean {
if (response.code !== undefined && WITHDRAWN_REPLY_ERROR_CODES.has(response.code)) {
return true;
}
const msg = response.msg?.toLowerCase() ?? "";
return msg.includes("withdrawn") || msg.includes("not found");
}
export type FeishuMessageInfo = {
messageId: string;
chatId: string;
@@ -238,6 +248,18 @@ export async function sendMessageFeishu(
...(replyInThread ? { reply_in_thread: true } : {}),
},
});
if (shouldFallbackFromReplyTarget(response)) {
const fallback = await client.im.message.create({
params: { receive_id_type: receiveIdType },
data: {
receive_id: receiveId,
content,
msg_type: msgType,
},
});
assertFeishuMessageApiSuccess(fallback, "Feishu send failed");
return toFeishuSendResult(fallback, receiveId);
}
assertFeishuMessageApiSuccess(response, "Feishu reply failed");
return toFeishuSendResult(response, receiveId);
}
@@ -278,6 +300,18 @@ export async function sendCardFeishu(params: SendFeishuCardParams): Promise<Feis
...(replyInThread ? { reply_in_thread: true } : {}),
},
});
if (shouldFallbackFromReplyTarget(response)) {
const fallback = await client.im.message.create({
params: { receive_id_type: receiveIdType },
data: {
receive_id: receiveId,
content,
msg_type: "interactive",
},
});
assertFeishuMessageApiSuccess(fallback, "Feishu card send failed");
return toFeishuSendResult(fallback, receiveId);
}
assertFeishuMessageApiSuccess(response, "Feishu card reply failed");
return toFeishuSendResult(response, receiveId);
}

View File

@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { resolveReceiveIdType } from "./targets.js";
import { looksLikeFeishuId, normalizeFeishuTarget, resolveReceiveIdType } from "./targets.js";
describe("resolveReceiveIdType", () => {
it("resolves chat IDs by oc_ prefix", () => {
@@ -14,3 +14,28 @@ describe("resolveReceiveIdType", () => {
expect(resolveReceiveIdType("u_123")).toBe("user_id");
});
});
describe("normalizeFeishuTarget", () => {
it("strips provider and user prefixes", () => {
expect(normalizeFeishuTarget("feishu:user:ou_123")).toBe("ou_123");
expect(normalizeFeishuTarget("lark:user:ou_123")).toBe("ou_123");
});
it("strips provider and chat prefixes", () => {
expect(normalizeFeishuTarget("feishu:chat:oc_123")).toBe("oc_123");
});
it("accepts provider-prefixed raw ids", () => {
expect(normalizeFeishuTarget("feishu:ou_123")).toBe("ou_123");
});
});
describe("looksLikeFeishuId", () => {
it("accepts provider-prefixed user targets", () => {
expect(looksLikeFeishuId("feishu:user:ou_123")).toBe(true);
});
it("accepts provider-prefixed chat targets", () => {
expect(looksLikeFeishuId("lark:chat:oc_123")).toBe(true);
});
});

View File

@@ -4,6 +4,10 @@ const CHAT_ID_PREFIX = "oc_";
const OPEN_ID_PREFIX = "ou_";
const USER_ID_REGEX = /^[a-zA-Z0-9_-]+$/;
function stripProviderPrefix(raw: string): string {
return raw.replace(/^(feishu|lark):/i, "").trim();
}
export function detectIdType(id: string): FeishuIdType | null {
const trimmed = id.trim();
if (trimmed.startsWith(CHAT_ID_PREFIX)) {
@@ -24,18 +28,19 @@ export function normalizeFeishuTarget(raw: string): string | null {
return null;
}
const lowered = trimmed.toLowerCase();
const withoutProvider = stripProviderPrefix(trimmed);
const lowered = withoutProvider.toLowerCase();
if (lowered.startsWith("chat:")) {
return trimmed.slice("chat:".length).trim() || null;
return withoutProvider.slice("chat:".length).trim() || null;
}
if (lowered.startsWith("user:")) {
return trimmed.slice("user:".length).trim() || null;
return withoutProvider.slice("user:".length).trim() || null;
}
if (lowered.startsWith("open_id:")) {
return trimmed.slice("open_id:".length).trim() || null;
return withoutProvider.slice("open_id:".length).trim() || null;
}
return trimmed;
return withoutProvider;
}
export function formatFeishuTarget(id: string, type?: FeishuIdType): string {
@@ -61,7 +66,7 @@ export function resolveReceiveIdType(id: string): "chat_id" | "open_id" | "user_
}
export function looksLikeFeishuId(raw: string): boolean {
const trimmed = raw.trim();
const trimmed = stripProviderPrefix(raw.trim());
if (!trimmed) {
return false;
}

View File

@@ -25,11 +25,13 @@ function createConfig(params: {
drive?: boolean;
perm?: boolean;
};
defaultAccount?: string;
}): OpenClawPluginApi["config"] {
return {
channels: {
feishu: {
enabled: true,
defaultAccount: params.defaultAccount,
accounts: {
a: {
appId: "app-a",
@@ -67,6 +69,22 @@ describe("feishu tool account routing", () => {
expect(createFeishuClientMock.mock.calls.at(-1)?.[0]?.appId).toBe("app-b");
});
test("wiki tool prefers configured defaultAccount over inherited default account context", async () => {
const { api, resolveTool } = createToolFactoryHarness(
createConfig({
defaultAccount: "b",
toolsA: { wiki: true },
toolsB: { wiki: true },
}),
);
registerFeishuWikiTools(api);
const tool = resolveTool("feishu_wiki", { agentAccountId: "a" });
await tool.execute("call", { action: "search" });
expect(createFeishuClientMock.mock.calls.at(-1)?.[0]?.appId).toBe("app-b");
});
test("drive tool registers when first account disables it and routes to agentAccountId", async () => {
const { api, resolveTool } = createToolFactoryHarness(
createConfig({

View File

@@ -12,6 +12,15 @@ function normalizeOptionalAccountId(value: string | undefined): string | undefin
return trimmed ? trimmed : undefined;
}
function readConfiguredDefaultAccountId(config: OpenClawPluginApi["config"]): string | undefined {
const value = (config?.channels?.feishu as { defaultAccount?: unknown } | undefined)
?.defaultAccount;
if (typeof value !== "string") {
return undefined;
}
return normalizeOptionalAccountId(value);
}
export function resolveFeishuToolAccount(params: {
api: Pick<OpenClawPluginApi, "config">;
executeParams?: AccountAwareParams;
@@ -24,6 +33,7 @@ export function resolveFeishuToolAccount(params: {
cfg: params.api.config,
accountId:
normalizeOptionalAccountId(params.executeParams?.accountId) ??
readConfiguredDefaultAccountId(params.api.config) ??
normalizeOptionalAccountId(params.defaultAccountId),
});
}