diff --git a/AGENTS.md b/AGENTS.md index c1928b93e5e..faca52035ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,6 @@ Telegraph style. Root rules only. Read scoped `AGENTS.md` before subtree work. - PR review answer must explicitly cover: what bug/behavior we are trying to fix; PR/issue URL(s) and affected endpoint/surface; whether this is the best possible fix, with high-certainty evidence from code, tests, CI, and shipped/current behavior. - CI polling: exact SHA, needed fields only. Example: `gh api repos///actions/runs/ --jq '{status,conclusion,head_sha,updated_at,name,path}'`. - Post-land wait: minimal. Exact landed SHA only. If superseded on `main`, same-branch `cancel-in-progress` cancellations are expected; stop once local touched-surface proof exists. Never wait for newer unrelated `main` unless asked. -- Test reruns: after a narrow fix, prefer the smallest affected test subset, shard, workflow job, lane, provider, or model allowlist that proves the changed behavior. Rerun a full suite only when the change touches shared orchestration, broad contracts, or the prior evidence no longer covers the risk. - Wait matrix: - never: `Auto response`, `Labeler`, `Docs Sync Publish Repo`, `Docs Agent`, `Test Performance Agent`, `Stale`. - conditional: `CI` exact SHA only; `Docs` only docs task/no local docs proof; `Workflow Sanity` only workflow/composite/CI-policy edits; `Plugin NPM Release` only plugin package/release metadata. diff --git a/extensions/feishu/src/bot-group-name.test.ts b/extensions/feishu/src/bot-group-name.test.ts new file mode 100644 index 00000000000..d5d53627c28 --- /dev/null +++ b/extensions/feishu/src/bot-group-name.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { resolveGroupName, clearGroupNameCache } from "./bot.js"; +import type { ResolvedFeishuAccount } from "./types.js"; + +const mockGetChatInfo = vi.hoisted(() => vi.fn()); +const mockCreateFeishuClient = vi.hoisted(() => vi.fn()); + +vi.mock("./chat.js", () => ({ getChatInfo: mockGetChatInfo })); +vi.mock("./client.js", () => ({ createFeishuClient: mockCreateFeishuClient })); + +function makeAccount(id = "test-account"): ResolvedFeishuAccount { + return { + accountId: id, + selectionSource: "explicit", + enabled: true, + configured: true, + appId: "cli_test", + appSecret: "secret", + domain: "feishu", + config: { + domain: "feishu", + connectionMode: "websocket", + webhookPath: "/feishu/events", + dmPolicy: "pairing", + reactionNotifications: "own", + groupPolicy: "allowlist", + typingIndicator: true, + resolveSenderNames: true, + }, + }; +} + +/** + * Unit tests for resolveGroupName. + * + * Covers: successful lookup, API failure, empty name, positive cache, + * negative cache, undefined response, and cross-account isolation. + */ +describe("resolveGroupName", () => { + const account = makeAccount(); + const log = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + mockGetChatInfo.mockReset(); + mockCreateFeishuClient.mockReset(); + mockCreateFeishuClient.mockReturnValue({}); + clearGroupNameCache(); + }); + + it("returns the trimmed group name on successful API call", async () => { + mockGetChatInfo.mockResolvedValue({ name: " Engineering Team " }); + const result = await resolveGroupName({ account, chatId: "oc_test1", log }); + expect(result).toBe("Engineering Team"); + expect(mockGetChatInfo).toHaveBeenCalledOnce(); + }); + + it("returns undefined and logs on API failure", async () => { + mockGetChatInfo.mockRejectedValue(new Error("network timeout")); + const result = await resolveGroupName({ account, chatId: "oc_test2", log }); + expect(result).toBeUndefined(); + expect(log).toHaveBeenCalledWith(expect.stringContaining("getChatInfo failed")); + }); + + it("returns undefined for whitespace-only name", async () => { + mockGetChatInfo.mockResolvedValue({ name: " " }); + const result = await resolveGroupName({ account, chatId: "oc_test3", log }); + expect(result).toBeUndefined(); + }); + + it("serves subsequent calls from cache (positive hit)", async () => { + mockGetChatInfo.mockResolvedValue({ name: "Cached Group" }); + await resolveGroupName({ account, chatId: "oc_test4", log }); + const result = await resolveGroupName({ account, chatId: "oc_test4", log }); + expect(result).toBe("Cached Group"); + expect(mockGetChatInfo).toHaveBeenCalledOnce(); // only 1 API call + }); + + it("caches negative result (API failure) and skips retry", async () => { + mockGetChatInfo.mockRejectedValue(new Error("fail")); + await resolveGroupName({ account, chatId: "oc_test5", log }); + mockGetChatInfo.mockResolvedValue({ name: "Recovered" }); + const result = await resolveGroupName({ account, chatId: "oc_test5", log }); + expect(result).toBeUndefined(); // still cached negative + expect(mockGetChatInfo).toHaveBeenCalledOnce(); + }); + + it("returns undefined when API returns object with missing name field", async () => { + mockGetChatInfo.mockResolvedValue({ name: undefined }); + const result = await resolveGroupName({ account, chatId: "oc_test6", log }); + expect(result).toBeUndefined(); + }); + + it("isolates cache entries across different accounts", async () => { + const accountA = makeAccount("account-A"); + const accountB = makeAccount("account-B"); + mockGetChatInfo + .mockResolvedValueOnce({ name: "Team Alpha" }) + .mockResolvedValueOnce({ name: "Team Beta" }); + + const nameA = await resolveGroupName({ account: accountA, chatId: "oc_shared", log }); + const nameB = await resolveGroupName({ account: accountB, chatId: "oc_shared", log }); + + expect(nameA).toBe("Team Alpha"); + expect(nameB).toBe("Team Beta"); + expect(mockGetChatInfo).toHaveBeenCalledTimes(2); // separate API calls + }); +}); diff --git a/extensions/feishu/src/bot.broadcast.test.ts b/extensions/feishu/src/bot.broadcast.test.ts index 27b2f201ae0..ac3c57b2cb1 100644 --- a/extensions/feishu/src/bot.broadcast.test.ts +++ b/extensions/feishu/src/bot.broadcast.test.ts @@ -2,7 +2,7 @@ import type { EnvelopeFormatOptions } from "openclaw/plugin-sdk/channel-inbound" import { beforeEach, describe, expect, it, vi } from "vitest"; import type { ClawdbotConfig, PluginRuntime } from "../runtime-api.js"; import type { FeishuMessageEvent } from "./bot.js"; -import { handleFeishuMessage } from "./bot.js"; +import { clearGroupNameCache, handleFeishuMessage } from "./bot.js"; import { setFeishuRuntime } from "./runtime.js"; const { mockCreateFeishuReplyDispatcher, mockCreateFeishuClient, mockResolveAgentRoute } = @@ -46,6 +46,7 @@ function createRuntimeEnv() { describe("broadcast dispatch", () => { const finalizeInboundContextCalls: Array> = []; + const mockGetChatInfo = vi.fn(); const mockFinalizeInboundContext: PluginRuntime["channel"]["reply"]["finalizeInboundContext"] = ( ctx, ) => { @@ -125,6 +126,8 @@ describe("broadcast dispatch", () => { agents: { list: [{ id: "main" }, { id: "susan" }] }, channels: { feishu: { + appId: "cli_test", + appSecret: "sec_test", // pragma: allowlist secret groups: { "oc-broadcast-group": { requireMention: true, @@ -166,6 +169,7 @@ describe("broadcast dispatch", () => { beforeEach(() => { vi.clearAllMocks(); + clearGroupNameCache(); finalizeInboundContextCalls.length = 0; mockResolveAgentRoute.mockReturnValue({ agentId: "main", @@ -182,6 +186,14 @@ describe("broadcast dispatch", () => { get: vi.fn().mockResolvedValue({ data: { user: { name: "Sender" } } }), }, }, + im: { + chat: { + get: mockGetChatInfo.mockResolvedValue({ + code: 0, + data: { name: "Broadcast Team" }, + }), + }, + }, }); setFeishuRuntime(runtimeStub); }); @@ -205,6 +217,15 @@ describe("broadcast dispatch", () => { const sessionKeys = finalizeInboundContextCalls.map((call) => call.SessionKey); expect(sessionKeys).toContain("agent:susan:feishu:group:oc-broadcast-group"); expect(sessionKeys).toContain("agent:main:feishu:group:oc-broadcast-group"); + expect(mockGetChatInfo).toHaveBeenCalledTimes(1); + expect(finalizeInboundContextCalls).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + GroupSubject: "Broadcast Team", + ConversationLabel: "Broadcast Team", + }), + ]), + ); expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledTimes(1); expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith( expect.objectContaining({ agentId: "main" }), @@ -227,6 +248,7 @@ describe("broadcast dispatch", () => { expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); expect(mockCreateFeishuReplyDispatcher).not.toHaveBeenCalled(); + expect(mockGetChatInfo).not.toHaveBeenCalled(); }); it("skips broadcast dispatch when bot identity is unknown (requireMention=true)", async () => { @@ -244,12 +266,15 @@ describe("broadcast dispatch", () => { expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); expect(mockCreateFeishuReplyDispatcher).not.toHaveBeenCalled(); + expect(mockGetChatInfo).not.toHaveBeenCalled(); }); it("preserves single-agent dispatch when no broadcast config", async () => { const cfg: ClawdbotConfig = { channels: { feishu: { + appId: "cli_test", + appSecret: "sec_test", // pragma: allowlist secret groups: { "oc-broadcast-group": { requireMention: false, @@ -281,8 +306,11 @@ describe("broadcast dispatch", () => { expect(finalizeInboundContextCalls).toContainEqual( expect.objectContaining({ SessionKey: "agent:main:feishu:group:oc-broadcast-group", + GroupSubject: "Broadcast Team", + ConversationLabel: "Broadcast Team", }), ); + expect(mockGetChatInfo).toHaveBeenCalledTimes(1); }); it("cross-account broadcast dedup: second account skips dispatch", async () => { @@ -291,6 +319,8 @@ describe("broadcast dispatch", () => { agents: { list: [{ id: "main" }, { id: "susan" }] }, channels: { feishu: { + appId: "cli_test", + appSecret: "sec_test", // pragma: allowlist secret groups: { "oc-broadcast-group": { requireMention: false, @@ -320,6 +350,7 @@ describe("broadcast dispatch", () => { expect(mockDispatchReplyFromConfig).toHaveBeenCalledTimes(2); mockDispatchReplyFromConfig.mockClear(); + mockGetChatInfo.mockClear(); finalizeInboundContextCalls.length = 0; await handleFeishuMessage({ @@ -329,6 +360,7 @@ describe("broadcast dispatch", () => { accountId: "account-B", }); expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); + expect(mockGetChatInfo).not.toHaveBeenCalled(); }); it("skips unknown agents not in agents.list", async () => { @@ -337,6 +369,8 @@ describe("broadcast dispatch", () => { agents: { list: [{ id: "main" }, { id: "susan" }] }, channels: { feishu: { + appId: "cli_test", + appSecret: "sec_test", // pragma: allowlist secret groups: { "oc-broadcast-group": { requireMention: false, diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 26691dfd46f..154e81bc06d 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -38,6 +38,7 @@ import { } from "./bot-runtime-api.js"; import type { ClawdbotConfig, RuntimeEnv } from "./bot-runtime-api.js"; import { type FeishuPermissionError, resolveFeishuSenderName } from "./bot-sender-name.js"; +import { getChatInfo } from "./chat.js"; import { createFeishuClient } from "./client.js"; import { finalizeFeishuMessageProcessing, tryRecordMessagePersistent } from "./dedup.js"; import { maybeCreateDynamicAgent } from "./dynamic-agent.js"; @@ -59,6 +60,7 @@ import { type FeishuMessageContext, type FeishuMediaInfo, type FeishuMessageInfo, + type ResolvedFeishuAccount, } from "./types.js"; import type { DynamicAgentCreationConfig } from "./types.js"; @@ -69,6 +71,86 @@ export { toMessageResourceType } from "./bot-content.js"; const permissionErrorNotifiedAt = new Map(); const PERMISSION_ERROR_COOLDOWN_MS = 5 * 60 * 1000; // 5 minutes +const groupNameCache = new Map(); +const GROUP_NAME_CACHE_TTL_MS = 30 * 60 * 1000; // 30 minutes +const GROUP_NAME_CACHE_MAX_SIZE = 500; // hard cap + +function evictGroupNameCache(): void { + const now = Date.now(); + for (const [key, val] of groupNameCache) { + if (val.expiresAt <= now) { + groupNameCache.delete(key); + } + } + + if (groupNameCache.size > GROUP_NAME_CACHE_MAX_SIZE) { + const excess = groupNameCache.size - GROUP_NAME_CACHE_MAX_SIZE; + let removed = 0; + for (const key of groupNameCache.keys()) { + if (removed >= excess) { + break; + } + groupNameCache.delete(key); + removed++; + } + } +} + +function setCacheEntry(key: string, value: { name: string; expiresAt: number }): void { + groupNameCache.delete(key); + groupNameCache.set(key, value); +} + +export function clearGroupNameCache(): void { + groupNameCache.clear(); +} + +export async function resolveGroupName(params: { + account: ResolvedFeishuAccount; + chatId: string; + log: (...args: unknown[]) => void; +}): Promise { + const { account, chatId, log } = params; + if (!account.configured) { + return undefined; + } + + const cacheKey = `${account.accountId}:${chatId}`; + + const cached = groupNameCache.get(cacheKey); + if (cached && cached.expiresAt > Date.now()) { + return cached.name || undefined; + } + + try { + const client = createFeishuClient(account); + const chatInfo = await getChatInfo(client, chatId); + const name = chatInfo?.name?.trim(); + if (name) { + setCacheEntry(cacheKey, { + name, + expiresAt: Date.now() + GROUP_NAME_CACHE_TTL_MS, + }); + } else { + setCacheEntry(cacheKey, { + name: "", + expiresAt: Date.now() + GROUP_NAME_CACHE_TTL_MS, + }); + } + } catch (err) { + log(`feishu[${account.accountId}]: getChatInfo failed for ${chatId}: ${String(err)}`); + setCacheEntry(cacheKey, { + name: "", + expiresAt: Date.now() + GROUP_NAME_CACHE_TTL_MS, + }); + } + + const result = groupNameCache.get(cacheKey)?.name || undefined; + evictGroupNameCache(); + + return result; +} + async function resolveFeishuAudioPreflightTranscript(params: { cfg: ClawdbotConfig; mediaList: FeishuMediaInfo[]; @@ -932,7 +1014,20 @@ export async function handleFeishuMessage(params: { } return rootMessageInfo ?? null; }; - const resolveThreadContextForAgent = async (agentId: string, agentSessionKey: string) => { + let groupNamePromise: Promise | undefined; + const resolveGroupNameForLabel = (): Promise => { + if (!isGroup) { + return Promise.resolve(undefined); + } + groupNamePromise ??= resolveGroupName({ account, chatId: ctx.chatId, log }); + return groupNamePromise; + }; + + const resolveThreadContextForAgent = async ( + agentId: string, + agentSessionKey: string, + groupName: string | undefined, + ) => { const cached = threadContextBySessionKey.get(agentSessionKey); if (cached) { return cached; @@ -945,7 +1040,7 @@ export async function handleFeishuMessage(params: { } = { threadLabel: (ctx.rootId || ctx.threadId) && isTopicSessionForThread - ? `Feishu thread in ${ctx.chatId}` + ? `Feishu thread in ${groupName ?? ctx.chatId}` : undefined, }; @@ -1047,7 +1142,8 @@ export async function handleFeishuMessage(params: { agentAccountId: string, wasMentioned: boolean, ) => { - const threadContext = await resolveThreadContextForAgent(agentId, agentSessionKey); + const groupName = await resolveGroupNameForLabel(); + const threadContext = await resolveThreadContextForAgent(agentId, agentSessionKey, groupName); return core.channel.reply.finalizeInboundContext({ Body: combinedBody, BodyForAgent: messageBody, @@ -1062,7 +1158,8 @@ export async function handleFeishuMessage(params: { SessionKey: agentSessionKey, AccountId: agentAccountId, ChatType: isGroup ? "group" : "direct", - GroupSubject: isGroup ? ctx.chatId : undefined, + GroupSubject: isGroup ? groupName || ctx.chatId : undefined, + ConversationLabel: isGroup && groupName && !isTopicSessionForThread ? groupName : undefined, SenderName: ctx.senderName ?? ctx.senderOpenId, SenderId: ctx.senderOpenId, Provider: "feishu" as const,