From db5e415888ae9314f27cbe89d4d4162daa73ca32 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 14 Jun 2026 17:28:54 +0800 Subject: [PATCH] fix(feishu): re-resolve route when dynamic agent binding already exists in runtime config (fixes #42837) (#92814) * fix(feishu): re-resolve route when dynamic agent binding already exists in runtime config When dynamicAgentCreation is enabled and a binding was previously written to the config file (e.g. from a prior message), the in-memory cfg may be stale and not contain the binding. Previously, maybeCreateDynamicAgent returned { created: false, updatedCfg: cfg } with the stale cfg, and bot.ts only re-resolved the route when created === true. This caused subsequent messages to still route to agent:main. Fix: check runtime.config.current() for the binding when it is missing from the in-memory cfg. When found, return the runtime's current config so the caller can re-resolve the route with up-to-date bindings. Fixes #42837 * fix(feishu): serialize dynamic agent config updates * fix(feishu): route with refreshed runtime config * fix(feishu): use current dynamic-agent policy * fix(feishu): reauthorize refreshed dynamic routes * fix(feishu): authorize dynamic agent mutations * fix(feishu): complete account-scoped dynamic routing * fix(feishu): revalidate current direct routes * fix(feishu): isolate named-account dynamic agents * fix(feishu): bound named dynamic agent ids * docs(feishu): explain legacy dynamic agent cap * test(feishu): fix dynamic routing check types --------- Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com> --- docs/channels/feishu.md | 19 +- extensions/feishu/src/bot.test.ts | 215 +++++++- extensions/feishu/src/bot.ts | 194 ++++--- extensions/feishu/src/comment-handler.test.ts | 118 ++++- extensions/feishu/src/comment-handler.ts | 115 +++-- extensions/feishu/src/dynamic-agent.test.ts | 486 +++++++++++++++--- extensions/feishu/src/dynamic-agent.ts | 281 ++++++---- 7 files changed, 1101 insertions(+), 327 deletions(-) diff --git a/docs/channels/feishu.md b/docs/channels/feishu.md index 1622a3073a66..f95d358a9bdd 100644 --- a/docs/channels/feishu.md +++ b/docs/channels/feishu.md @@ -416,7 +416,9 @@ Enable `dynamicAgentCreation` to automatically create **isolated agent instances This is essential for public bots where you want each user to have their own private AI assistant experience. -**Account limitation**: `dynamicAgentCreation` currently works with the **default Feishu account only**. Named/multi-account setups are not yet fully supported — dynamic bindings are created without `accountId`, so messages to named accounts may still route to `agent:main`. Track progress in [Issue #42837](https://github.com/openclaw/openclaw/issues/42837). +Dynamic bindings include the normalized Feishu `accountId`, so default and named accounts route each sender to the correct dynamic agent. + +If a named account created an unscoped dynamic agent on an older release, that legacy agent still counts toward `maxAgents`. Confirm that it is not used by the default account before removing it, or temporarily increase `maxAgents`; OpenClaw cannot safely infer which account owns ambiguous legacy state. ### Quick setup @@ -447,7 +449,7 @@ This is essential for public bots where you want each user to have their own pri When a new user sends their first DM: -1. The channel generates a unique `agentId` = `feishu-{user_open_id}` +1. The channel generates a unique `agentId`: `feishu-{user_open_id}` for the default account, or a bounded account-prefixed identity digest for a named account 2. Creates a new workspace at `workspaceTemplate` path 3. Registers the agent and creates a binding for this user 4. The workspace helper ensures bootstrap files (`AGENTS.md`, `SOUL.md`, `USER.md`, etc.) on first access @@ -464,22 +466,23 @@ When a new user sends their first DM: Template variables: -- `{agentId}` - the generated agent ID (e.g., `feishu-ou_xxxxxx`) +- `{agentId}` - the generated agent ID (e.g., `feishu-ou_xxxxxx` or `feishu-support-`) - `{userId}` - the sender's Feishu open_id (e.g., `ou_xxxxxx`) ### Session scope `session.dmScope` controls how direct messages are mapped to agent sessions. This is a **global setting** that affects all channels. -| Value | Behavior | Best for | -| -------------------- | --------------------------------------------------------- | ------------------------------------------------------------------ | -| `"main"` | Each user's DM maps to their agent's main session | Single-user bots where you want `USER.md` / `SOUL.md` to auto-load | -| `"per-channel-peer"` | Each (channel + user) combination gets a separate session | Public multi-user bots needing stronger isolation | +| Value | Behavior | Best for | +| ---------------------------- | ------------------------------------------------------------------- | ------------------------------------------------------------------ | +| `"main"` | Each user's DM maps to their agent's main session | Single-user bots where you want `USER.md` / `SOUL.md` to auto-load | +| `"per-channel-peer"` | Each (channel + user) combination gets a separate session | Public multi-user bots needing stronger isolation | +| `"per-account-channel-peer"` | Each (account + channel + user) combination gets a separate session | Multi-account bots needing account-level session isolation | **Tradeoff**: Using `"main"` enables automatic bootstrap file loading (`USER.md`, `SOUL.md`, `MEMORY.md`), but means all DMs across all channels share the same session key pattern. For public multi-user bots where isolation matters more than bootstrap auto-loading, consider `"per-channel-peer"` and manage bootstrap files manually. -`"per-account-channel-peer"` is not recommended with `dynamicAgentCreation` because dynamic bindings are created without `accountId`. Use it only with manual bindings. +Use `"per-account-channel-peer"` when named Feishu accounts should keep separate sessions for the same sender. Dynamic bindings preserve the account scope. ```json5 diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts index 1d82d04297f7..b432f0465fc5 100644 --- a/extensions/feishu/src/bot.test.ts +++ b/extensions/feishu/src/bot.test.ts @@ -163,8 +163,13 @@ function buildDefaultResolveRoute(): ResolvedAgentRoute { matchedBy: "default", }; } +let currentRuntimeConfig = {} as ClawdbotConfig; + function createFeishuBotRuntime(overrides: DeepPartial = {}): PluginRuntime { return { + config: { + current: vi.fn(() => currentRuntimeConfig), + }, channel: { routing: { resolveAgentRoute: resolveAgentRouteMock, @@ -413,7 +418,11 @@ afterAll(() => { vi.resetModules(); }); -async function dispatchMessage(params: { cfg: ClawdbotConfig; event: FeishuMessageEvent }) { +async function dispatchMessage(params: { + cfg: ClawdbotConfig; + currentCfg?: ClawdbotConfig; + event: FeishuMessageEvent; +}) { const runtime = createRuntimeEnv(); const feishuConfig = params.cfg.channels?.feishu; const cfg = @@ -429,6 +438,7 @@ async function dispatchMessage(params: { cfg: ClawdbotConfig; event: FeishuMessa }, } as ClawdbotConfig) : params.cfg; + currentRuntimeConfig = params.currentCfg ?? cfg; await handleFeishuMessage({ cfg, event: params.event, @@ -455,7 +465,10 @@ describe("handleFeishuMessage ACP routing", () => { mockTouchBinding.mockReset(); mockResolveFeishuReasoningPreviewEnabled.mockReset().mockReturnValue(false); mockTranscribeFirstAudio.mockReset().mockResolvedValue(undefined); - mockMaybeCreateDynamicAgent.mockReset().mockResolvedValue({ created: false }); + mockMaybeCreateDynamicAgent.mockReset().mockImplementation(async ({ cfg }) => ({ + created: false, + updatedCfg: cfg, + })); mockResolveAgentRoute.mockReset().mockReturnValue({ ...buildDefaultResolveRoute(), sessionKey: "agent:main:feishu:direct:ou_sender_1", @@ -976,7 +989,9 @@ describe("handleFeishuMessage command authorization", () => { }, ); const mockResolveCommandAuthorizedFromAuthorizers = vi.fn(() => false); - const mockShouldComputeCommandAuthorized = vi.fn(() => true); + const mockShouldComputeCommandAuthorized = vi.fn< + PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"] + >(() => true); const mockReadAllowFromStore = vi.fn().mockResolvedValue([]); const mockUpsertPairingRequest = vi.fn().mockResolvedValue({ code: "ABCDEFGH", created: false }); const mockBuildPairingReply = vi.fn(() => "Pairing response"); @@ -1009,7 +1024,10 @@ describe("handleFeishuMessage command authorization", () => { mockResolveBoundConversation.mockReset().mockReturnValue(null); mockTouchBinding.mockReset(); mockTranscribeFirstAudio.mockReset().mockResolvedValue(undefined); - mockMaybeCreateDynamicAgent.mockReset().mockResolvedValue({ created: false }); + mockMaybeCreateDynamicAgent.mockReset().mockImplementation(async ({ cfg }) => ({ + created: false, + updatedCfg: cfg, + })); mockResolveAgentRoute.mockReturnValue(buildDefaultResolveRoute()); mockCreateFeishuClient.mockReturnValue({ contact: { @@ -1216,7 +1234,7 @@ describe("handleFeishuMessage command authorization", () => { expect(ensureNoVisibleReplyFallback).toHaveBeenCalledWith("dispatch-complete-no-visible-reply"); }); - it("passes disabled config-write policy to dynamic agent creation", async () => { + it("uses refreshed config for dynamic agent dispatch", async () => { mockShouldComputeCommandAuthorized.mockReturnValue(false); const cfg: ClawdbotConfig = { @@ -1231,6 +1249,22 @@ describe("handleFeishuMessage command authorization", () => { }, }, } as ClawdbotConfig; + const refreshedCfg = { + ...cfg, + agents: { + list: [ + { + id: "feishu-ou-attacker", + workspace: "/tmp/feishu-ou-attacker", + agentDir: "/tmp/feishu-ou-attacker/agent", + }, + ], + }, + } as ClawdbotConfig; + mockMaybeCreateDynamicAgent.mockResolvedValueOnce({ + created: false, + updatedCfg: refreshedCfg, + }); const event: FeishuMessageEvent = { sender: { @@ -1250,12 +1284,177 @@ describe("handleFeishuMessage command authorization", () => { await dispatchMessage({ cfg, event }); const dynamicAgentRequest = mockCallArg<{ - configWritesAllowed?: boolean; + accountId?: string; senderOpenId?: string; }>(mockMaybeCreateDynamicAgent, 0, 0); expect(dynamicAgentRequest.senderOpenId).toBe("ou-attacker"); - expect(dynamicAgentRequest.configWritesAllowed).toBe(false); - expect(mockDispatchReplyFromConfig).toHaveBeenCalledTimes(1); + expect(dynamicAgentRequest.accountId).toBe("default"); + expect(mockCreateFeishuReplyDispatcher).toHaveBeenCalledWith( + expect.objectContaining({ cfg: refreshedCfg }), + ); + expect(mockDispatchReplyFromConfig).toHaveBeenCalledWith( + expect.objectContaining({ cfg: refreshedCfg }), + ); + }); + + it("drops a DM denied by refreshed dynamic-agent policy", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + + const cfg = { + channels: { + feishu: { + dmPolicy: "open", + allowFrom: ["*"], + dynamicAgentCreation: { enabled: true }, + }, + }, + } as ClawdbotConfig; + const refreshedCfg = { + channels: { + feishu: { + dmPolicy: "allowlist", + allowFrom: ["ou-admin"], + dynamicAgentCreation: { enabled: true }, + }, + }, + } as ClawdbotConfig; + await dispatchMessage({ + cfg, + currentCfg: refreshedCfg, + event: { + sender: { sender_id: { open_id: "ou-attacker" } }, + message: { + message_id: "msg-refreshed-policy-deny", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }, + }); + + expect(mockMaybeCreateDynamicAgent).not.toHaveBeenCalled(); + expect(mockFinalizeInboundContext).not.toHaveBeenCalled(); + expect(mockCreateFeishuReplyDispatcher).not.toHaveBeenCalled(); + expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); + }); + + it("reauthorizes current policy before dispatching an existing bound route", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + mockResolveAgentRoute.mockReturnValue({ + ...buildDefaultResolveRoute(), + matchedBy: "binding.peer", + }); + const cfg = { + channels: { feishu: { dmPolicy: "open", allowFrom: ["*"] } }, + } as ClawdbotConfig; + const currentCfg = { + channels: { feishu: { dmPolicy: "allowlist", allowFrom: ["ou-admin"] } }, + } as ClawdbotConfig; + + await dispatchMessage({ + cfg, + currentCfg, + event: { + sender: { sender_id: { open_id: "ou-attacker" } }, + message: { + message_id: "msg-bound-refreshed-policy-deny", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }, + }); + + expect(mockFinalizeInboundContext).not.toHaveBeenCalled(); + expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); + }); + + it("issues a pairing challenge before dynamic creation when current policy requires it", async () => { + mockShouldComputeCommandAuthorized.mockReturnValue(false); + mockReadAllowFromStore.mockResolvedValue([]); + mockUpsertPairingRequest.mockResolvedValue({ code: "ABCDEFGH", created: true }); + + const cfg = { + channels: { + feishu: { + dmPolicy: "open", + allowFrom: ["*"], + dynamicAgentCreation: { enabled: true }, + }, + }, + } as ClawdbotConfig; + const currentCfg = { + channels: { + feishu: { + dmPolicy: "pairing", + allowFrom: [], + dynamicAgentCreation: { enabled: true }, + }, + }, + } as ClawdbotConfig; + + await dispatchMessage({ + cfg, + currentCfg, + event: { + sender: { sender_id: { open_id: "ou-attacker" } }, + message: { + message_id: "msg-refreshed-policy-pairing", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "hello" }), + }, + }, + }); + + expect(mockMaybeCreateDynamicAgent).not.toHaveBeenCalled(); + expect(mockUpsertPairingRequest).toHaveBeenCalledTimes(1); + expect(mockSendMessageFeishu).toHaveBeenCalledTimes(1); + expect(mockDispatchReplyFromConfig).not.toHaveBeenCalled(); + }); + + it("recomputes command authorization against refreshed dynamic-agent config", async () => { + const cfg = { + channels: { + feishu: { + dmPolicy: "open", + allowFrom: ["*"], + dynamicAgentCreation: { enabled: true }, + }, + }, + } as ClawdbotConfig; + const refreshedCfg = { + ...cfg, + commands: { useAccessGroups: true }, + } as ClawdbotConfig; + mockShouldComputeCommandAuthorized.mockImplementation((_body, candidateCfg) => { + return candidateCfg === refreshedCfg; + }); + mockMaybeCreateDynamicAgent.mockResolvedValueOnce({ + created: false, + updatedCfg: refreshedCfg, + }); + + await dispatchMessage({ + cfg, + event: { + sender: { sender_id: { open_id: "ou-attacker" } }, + message: { + message_id: "msg-refreshed-command-auth", + chat_id: "oc-dm", + chat_type: "p2p", + message_type: "text", + content: JSON.stringify({ text: "/status" }), + }, + }, + }); + + expect(mockShouldComputeCommandAuthorized).toHaveBeenCalledWith("/status", refreshedCfg); + const context = mockCallArg<{ CommandAuthorized?: boolean }>(mockFinalizeInboundContext, 0, 0); + expect(context.CommandAuthorized).toBe(true); }); it("blocks open DMs when a restrictive allowlist does not match", async () => { diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 410576e34ecc..b1b216618c26 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -1,5 +1,4 @@ // Feishu plugin module implements bot behavior. -import { resolveChannelConfigWrites } from "openclaw/plugin-sdk/channel-config-writes"; import { buildChannelInboundEventContext, toInboundMediaFacts, @@ -74,7 +73,6 @@ import { type FeishuMessageInfo, type ResolvedFeishuAccount, } from "./types.js"; -import type { DynamicAgentCreationConfig } from "./types.js"; export { toMessageResourceType } from "./bot-content.js"; @@ -748,22 +746,44 @@ export async function handleFeishuMessage(params: { commandProbeBody, cfg, ); - const dmIngress = isDirect - ? await resolveFeishuDmIngressAccess({ - cfg, - accountId: account.accountId, - dmPolicy, - allowFrom: configAllowFrom, - readAllowFromStore: pairing.readAllowFromStore, - senderOpenId: ctx.senderOpenId, - senderUserId, - conversationId: ctx.senderOpenId, - mayPair: true, - ...(shouldComputeCommandAuthorized ? { command: { hasControlCommand: true } } : {}), - }) - : null; - if (isDirect && dmIngress?.ingress.admission !== "dispatch") { - if (dmIngress?.ingress.admission === "pairing-required") { + const resolveDirectAuthorization = async ( + candidateCfg: ClawdbotConfig, + mayPair: boolean, + shouldComputeCommand = core.channel.commands.shouldComputeCommandAuthorized( + commandProbeBody, + candidateCfg, + ), + ) => { + const candidateAccount = resolveFeishuRuntimeAccount({ + cfg: candidateCfg, + accountId: account.accountId, + }); + const candidateDmPolicy = candidateAccount.config.dmPolicy ?? "pairing"; + const candidateConfigAllowFrom = candidateAccount.config.allowFrom ?? []; + const ingress = await resolveFeishuDmIngressAccess({ + cfg: candidateCfg, + accountId: candidateAccount.accountId, + dmPolicy: candidateDmPolicy, + allowFrom: candidateConfigAllowFrom, + readAllowFromStore: pairing.readAllowFromStore, + senderOpenId: ctx.senderOpenId, + senderUserId, + conversationId: ctx.senderOpenId, + mayPair, + ...(shouldComputeCommand ? { command: { hasControlCommand: true } } : {}), + }); + return { + cfg: candidateCfg, + dmPolicy: candidateDmPolicy, + configAllowFrom: candidateConfigAllowFrom, + ingress, + shouldComputeCommandAuthorized: shouldComputeCommand, + }; + }; + const rejectDirectAuthorization = async ( + authorization: Awaited>, + ) => { + if (authorization.ingress.ingress.admission === "pairing-required") { await pairing.issueChallenge({ senderId: ctx.senderOpenId, senderIdLine: `Your Feishu user id: ${ctx.senderOpenId}`, @@ -773,7 +793,7 @@ export async function handleFeishuMessage(params: { }, sendPairingReply: async (text) => { await sendMessageFeishu({ - cfg, + cfg: authorization.cfg, to: `chat:${ctx.chatId}`, text, accountId: account.accountId, @@ -787,15 +807,43 @@ export async function handleFeishuMessage(params: { }); } else { log( - `feishu[${account.accountId}]: blocked unauthorized sender ${ctx.senderOpenId} (dmPolicy=${dmPolicy})`, + `feishu[${account.accountId}]: blocked unauthorized sender ${ctx.senderOpenId} ` + + `(dmPolicy=${authorization.dmPolicy})`, ); } + }; + const directAuthorization = isDirect + ? await resolveDirectAuthorization(cfg, true, shouldComputeCommandAuthorized) + : null; + const dmIngress = directAuthorization?.ingress ?? null; + if (isDirect && dmIngress?.ingress.admission !== "dispatch") { + if (directAuthorization) { + await rejectDirectAuthorization(directAuthorization); + } return; } - - const commandAllowFrom = isGroup - ? (groupConfig?.allowFrom ?? configAllowFrom) - : (dmIngress?.senderAccess.effectiveAllowFrom ?? configAllowFrom); + let effectiveDmPolicy = directAuthorization?.dmPolicy ?? dmPolicy; + let effectiveConfigAllowFrom = directAuthorization?.configAllowFrom ?? configAllowFrom; + let effectiveDmIngress = dmIngress; + let effectiveShouldComputeCommandAuthorized = + directAuthorization?.shouldComputeCommandAuthorized ?? shouldComputeCommandAuthorized; + let effectiveCfg = cfg; + if (isDirect) { + const currentCfg = getFeishuRuntime().config.current() as ClawdbotConfig; + if (currentCfg !== effectiveCfg) { + const currentAuthorization = await resolveDirectAuthorization(currentCfg, true); + if (currentAuthorization.ingress.ingress.admission !== "dispatch") { + await rejectDirectAuthorization(currentAuthorization); + return; + } + effectiveCfg = currentCfg; + effectiveDmPolicy = currentAuthorization.dmPolicy; + effectiveConfigAllowFrom = currentAuthorization.configAllowFrom; + effectiveDmIngress = currentAuthorization.ingress; + effectiveShouldComputeCommandAuthorized = + currentAuthorization.shouldComputeCommandAuthorized; + } + } // In group chats, the session is scoped to the group, but the *speaker* is the sender. // Using a group-scoped From causes the agent to treat different users as the same person. @@ -823,7 +871,7 @@ export async function handleFeishuMessage(params: { } let route = core.channel.routing.resolveAgentRoute({ - cfg, + cfg: effectiveCfg, channel: "feishu", accountId: account.accountId, peer: { @@ -833,34 +881,43 @@ export async function handleFeishuMessage(params: { parentPeer, }); - // Dynamic agent creation for DM users - // When enabled, creates a unique agent instance with its own workspace for each DM user. - let effectiveCfg = cfg; + // Refresh a binding written after this request snapshot, or create the DM's + // dynamic agent when the current account policy enables it. if (!isGroup && route.matchedBy === "default") { - const dynamicCfg = feishuCfg?.dynamicAgentCreation as DynamicAgentCreationConfig | undefined; - if (dynamicCfg?.enabled) { - const runtimeLocal = getFeishuRuntime(); - const result = await maybeCreateDynamicAgent({ - cfg, - runtime: runtimeLocal, - senderOpenId: ctx.senderOpenId, - dynamicCfg, - configWritesAllowed: resolveChannelConfigWrites({ - cfg, - channelId: "feishu", - accountId: account.accountId, - }), - log: (msg) => log(msg), + const runtimeLocal = getFeishuRuntime(); + const result = await maybeCreateDynamicAgent({ + cfg: effectiveCfg, + runtime: runtimeLocal, + accountId: account.accountId, + senderOpenId: ctx.senderOpenId, + canCreateForConfig: async (candidateCfg) => { + const authorization = await resolveDirectAuthorization(candidateCfg, false); + return authorization.ingress.ingress.admission === "dispatch"; + }, + log: (msg) => log(msg), + }); + if (result.created || result.updatedCfg !== effectiveCfg) { + const refreshedAuthorization = await resolveDirectAuthorization(result.updatedCfg, false); + if (refreshedAuthorization.ingress.ingress.admission !== "dispatch") { + log( + `feishu[${account.accountId}]: current policy rejected stale DM from ${ctx.senderOpenId} ` + + `before adopting refreshed dynamic route (dmPolicy=${refreshedAuthorization.dmPolicy})`, + ); + return; + } + effectiveCfg = result.updatedCfg; + effectiveDmPolicy = refreshedAuthorization.dmPolicy; + effectiveConfigAllowFrom = refreshedAuthorization.configAllowFrom; + effectiveDmIngress = refreshedAuthorization.ingress; + effectiveShouldComputeCommandAuthorized = + refreshedAuthorization.shouldComputeCommandAuthorized; + route = core.channel.routing.resolveAgentRoute({ + cfg: result.updatedCfg, + channel: "feishu", + accountId: account.accountId, + peer: { kind: "direct", id: ctx.senderOpenId }, }); if (result.created) { - effectiveCfg = result.updatedCfg; - // Re-resolve route with updated config - route = core.channel.routing.resolveAgentRoute({ - cfg: result.updatedCfg, - channel: "feishu", - accountId: account.accountId, - peer: { kind: "direct", id: ctx.senderOpenId }, - }); log( `feishu[${account.accountId}]: dynamic agent created, new route: ${route.sessionKey}`, ); @@ -868,6 +925,10 @@ export async function handleFeishuMessage(params: { } } + const commandAllowFrom = isGroup + ? (groupConfig?.allowFrom ?? effectiveConfigAllowFrom) + : (effectiveDmIngress?.senderAccess.effectiveAllowFrom ?? effectiveConfigAllowFrom); + const currentConversationId = peerId; const parentConversationId = isGroup ? (parentPeer?.id ?? ctx.chatId) : undefined; let configuredBinding = null; @@ -1006,15 +1067,18 @@ export async function handleFeishuMessage(params: { : audioTranscript; const shouldComputeEffectiveCommandAuthorized = audioTranscript === undefined - ? shouldComputeCommandAuthorized - : core.channel.commands.shouldComputeCommandAuthorized(effectiveCommandProbeBody, cfg); + ? effectiveShouldComputeCommandAuthorized + : core.channel.commands.shouldComputeCommandAuthorized( + effectiveCommandProbeBody, + effectiveCfg, + ); const commandAuthorized = shouldComputeEffectiveCommandAuthorized - ? isDirect && audioTranscript === undefined && dmIngress - ? dmIngress.commandAccess.authorized + ? isDirect && audioTranscript === undefined && effectiveDmIngress + ? effectiveDmIngress.commandAccess.authorized : isGroup ? ( await resolveFeishuGroupSenderActivationIngressAccess({ - cfg, + cfg: effectiveCfg, accountId: account.accountId, chatId: ctx.chatId, allowFrom: commandAllowFrom, @@ -1027,10 +1091,10 @@ export async function handleFeishuMessage(params: { ).commandAccess.authorized : ( await resolveFeishuDmIngressAccess({ - cfg, + cfg: effectiveCfg, accountId: account.accountId, - dmPolicy, - allowFrom: configAllowFrom, + dmPolicy: effectiveDmPolicy, + allowFrom: effectiveConfigAllowFrom, readAllowFromStore: pairing.readAllowFromStore, senderOpenId: ctx.senderOpenId, senderUserId, @@ -1413,8 +1477,8 @@ export async function handleFeishuMessage(params: { : undefined; const pinnedMainDmOwner = !isGroup ? resolvePinnedMainDmOwnerFromAllowlist({ - dmScope: cfg.session?.dmScope, - allowFrom: configAllowFrom, + dmScope: effectiveCfg.session?.dmScope, + allowFrom: effectiveConfigAllowFrom, normalizeEntry: normalizeFeishuAllowEntry, }) : null; @@ -1690,19 +1754,19 @@ export async function handleFeishuMessage(params: { ctx.mentionedBot, ); - const identity = resolveAgentOutboundIdentity(cfg, route.agentId); - const storePath = core.channel.session.resolveStorePath(cfg.session?.store, { + const identity = resolveAgentOutboundIdentity(effectiveCfg, route.agentId); + const storePath = core.channel.session.resolveStorePath(effectiveCfg.session?.store, { agentId: route.agentId, }); const allowReasoningPreview = resolveFeishuReasoningPreviewEnabled({ - cfg, + cfg: effectiveCfg, agentId: route.agentId, storePath, sessionKey: route.sessionKey, }); const { dispatcher, replyOptions, markDispatchIdle, ensureNoVisibleReplyFallback } = createFeishuReplyDispatcher({ - cfg, + cfg: effectiveCfg, agentId: route.agentId, runtime: runtime as RuntimeEnv, chatId: ctx.chatId, @@ -1771,7 +1835,7 @@ export async function handleFeishuMessage(params: { run: () => core.channel.reply.dispatchReplyFromConfig({ ctx: ctxPayload, - cfg, + cfg: effectiveCfg, dispatcher, replyOptions, }), diff --git a/extensions/feishu/src/comment-handler.test.ts b/extensions/feishu/src/comment-handler.test.ts index 68607b855aef..314434ec941e 100644 --- a/extensions/feishu/src/comment-handler.test.ts +++ b/extensions/feishu/src/comment-handler.test.ts @@ -53,6 +53,8 @@ function buildConfig(overrides?: Partial): ClawdbotConfig { } as ClawdbotConfig; } +let currentRuntimeConfig = buildConfig(); + function buildResolvedRoute(matchedBy: "binding.channel" | "default" = "binding.channel") { return { agentId: "main", @@ -77,6 +79,7 @@ function mockCallArg(mockFn: ReturnType, label: string, callIndex } function createTestRuntime(overrides?: { + currentCfg?: ClawdbotConfig; readAllowFromStore?: () => Promise; upsertPairingRequest?: () => Promise<{ code: string; created: boolean }>; resolveAgentRoute?: () => ReturnType; @@ -129,6 +132,9 @@ function createTestRuntime(overrides?: { }); return { + config: { + current: vi.fn(() => overrides?.currentCfg ?? currentRuntimeConfig), + }, channel: { routing: { buildAgentSessionKey: vi.fn( @@ -200,7 +206,11 @@ describe("handleFeishuCommentEvent", () => { beforeEach(() => { vi.clearAllMocks(); - maybeCreateDynamicAgentMock.mockResolvedValue({ created: false }); + currentRuntimeConfig = buildConfig(); + maybeCreateDynamicAgentMock.mockImplementation(async ({ cfg }) => ({ + created: false, + updatedCfg: cfg, + })); resolveDriveCommentEventTurnMock.mockResolvedValue({ eventId: "evt_1", messageId: "drive-comment:evt_1", @@ -323,26 +333,28 @@ describe("handleFeishuCommentEvent", () => { expect(deliverCommentThreadTextMock).not.toHaveBeenCalled(); }); - it("passes disabled config-write policy to dynamic agent creation", async () => { + it("passes the resolved account to dynamic agent resolution", async () => { + const cfg = buildConfig({ + channels: { + feishu: { + enabled: true, + dmPolicy: "open", + allowFrom: ["*"], + configWrites: false, + dynamicAgentCreation: { + enabled: true, + }, + }, + }, + }); const runtime = createTestRuntime({ + currentCfg: cfg, resolveAgentRoute: () => buildResolvedRoute("default"), }); setFeishuRuntime(runtime); await handleFeishuCommentEvent({ - cfg: buildConfig({ - channels: { - feishu: { - enabled: true, - dmPolicy: "open", - allowFrom: ["*"], - configWrites: false, - dynamicAgentCreation: { - enabled: true, - }, - }, - }, - }), + cfg, accountId: "default", event: { event_id: "evt_1" }, botOpenId: "ou_bot", @@ -354,16 +366,88 @@ describe("handleFeishuCommentEvent", () => { expect(maybeCreateDynamicAgentMock).toHaveBeenCalledTimes(1); const dynamicAgentArgs = mockCallArg(maybeCreateDynamicAgentMock, "maybeCreateDynamicAgent") as - | { configWritesAllowed?: boolean; senderOpenId?: string } + | { accountId?: string; senderOpenId?: string } | undefined; expect(dynamicAgentArgs?.senderOpenId).toBe("ou_sender"); - expect(dynamicAgentArgs?.configWritesAllowed).toBe(false); + expect(dynamicAgentArgs?.accountId).toBe("default"); const dispatchReplyFromConfig = runtime.channel.reply.dispatchReplyFromConfig as ReturnType< typeof vi.fn >; expect(dispatchReplyFromConfig).toHaveBeenCalledTimes(1); }); + it("drops a comment denied by refreshed dynamic-agent policy", async () => { + const refreshedCfg = buildConfig({ + channels: { + feishu: { + enabled: true, + dmPolicy: "allowlist", + allowFrom: ["ou_admin"], + }, + }, + }); + const runtime = createTestRuntime({ + currentCfg: refreshedCfg, + resolveAgentRoute: () => buildResolvedRoute("default"), + }); + setFeishuRuntime(runtime); + const cfg = buildConfig(); + + await handleFeishuCommentEvent({ + cfg, + accountId: "default", + event: { event_id: "evt_1" }, + botOpenId: "ou_bot", + runtime: { + log: vi.fn(), + error: vi.fn(), + } as never, + }); + + const dispatchReplyFromConfig = runtime.channel.reply.dispatchReplyFromConfig as ReturnType< + typeof vi.fn + >; + expect(maybeCreateDynamicAgentMock).not.toHaveBeenCalled(); + expect(deliverCommentThreadTextMock).not.toHaveBeenCalled(); + expect(dispatchReplyFromConfig).not.toHaveBeenCalled(); + }); + + it("issues a pairing challenge before dynamic comment-agent creation", async () => { + const currentCfg = buildConfig({ + channels: { + feishu: { + enabled: true, + dmPolicy: "pairing", + allowFrom: [], + dynamicAgentCreation: { enabled: true }, + }, + }, + }); + const runtime = createTestRuntime({ + currentCfg, + resolveAgentRoute: () => buildResolvedRoute("default"), + }); + setFeishuRuntime(runtime); + + await handleFeishuCommentEvent({ + cfg: buildConfig(), + accountId: "default", + event: { event_id: "evt_1" }, + botOpenId: "ou_bot", + runtime: { + log: vi.fn(), + error: vi.fn(), + } as never, + }); + + const dispatchReplyFromConfig = runtime.channel.reply.dispatchReplyFromConfig as ReturnType< + typeof vi.fn + >; + expect(maybeCreateDynamicAgentMock).not.toHaveBeenCalled(); + expect(deliverCommentThreadTextMock).toHaveBeenCalledTimes(1); + expect(dispatchReplyFromConfig).not.toHaveBeenCalled(); + }); + it("issues a pairing challenge in the comment thread when dmPolicy=pairing", async () => { const runtime = createTestRuntime(); setFeishuRuntime(runtime); diff --git a/extensions/feishu/src/comment-handler.ts b/extensions/feishu/src/comment-handler.ts index e27716a4ce1f..09d12c6e9f0f 100644 --- a/extensions/feishu/src/comment-handler.ts +++ b/extensions/feishu/src/comment-handler.ts @@ -1,5 +1,4 @@ // Feishu plugin module implements comment handler behavior. -import { resolveChannelConfigWrites } from "openclaw/plugin-sdk/channel-config-writes"; import { parseStrictNonNegativeInteger } from "openclaw/plugin-sdk/number-runtime"; import type { ResolvedAgentRoute } from "openclaw/plugin-sdk/routing"; import { resolveFeishuRuntimeAccount } from "./accounts.js"; @@ -19,7 +18,6 @@ import { } from "./monitor.comment.js"; import { resolveFeishuDmIngressAccess } from "./policy.js"; import { getFeishuRuntime } from "./runtime.js"; -import type { DynamicAgentCreationConfig } from "./types.js"; type HandleFeishuCommentEventParams = { cfg: ClawdbotConfig; @@ -55,7 +53,6 @@ export async function handleFeishuCommentEvent( params: HandleFeishuCommentEventParams, ): Promise { const account = resolveFeishuRuntimeAccount({ cfg: params.cfg, accountId: params.accountId }); - const feishuCfg = account.config; const core = getFeishuRuntime(); const log = params.runtime?.log ?? console.log; const error = params.runtime?.error ?? console.error; @@ -81,27 +78,35 @@ export async function handleFeishuCommentEvent( fileToken: turn.fileToken, commentId: turn.commentId, }); - const dmPolicy = feishuCfg?.dmPolicy ?? "pairing"; - const configAllowFrom = feishuCfg?.allowFrom ?? []; const pairing = createChannelPairingController({ core, channel: "feishu", accountId: account.accountId, }); - const dmIngress = await resolveFeishuDmIngressAccess({ - cfg: params.cfg, - accountId: account.accountId, - dmPolicy, - allowFrom: configAllowFrom, - readAllowFromStore: pairing.readAllowFromStore, - senderOpenId: turn.senderId, - senderUserId: turn.senderUserId, - conversationId: turn.senderId, - mayPair: true, - }); - if (dmIngress.ingress.admission !== "dispatch") { - if (dmIngress.ingress.admission === "pairing-required") { - const client = createFeishuClient(account); + const resolveCommentAuthorization = async (candidateCfg: ClawdbotConfig, mayPair: boolean) => { + const candidateAccount = resolveFeishuRuntimeAccount({ + cfg: candidateCfg, + accountId: account.accountId, + }); + const candidateDmPolicy = candidateAccount.config.dmPolicy ?? "pairing"; + const ingress = await resolveFeishuDmIngressAccess({ + cfg: candidateCfg, + accountId: candidateAccount.accountId, + dmPolicy: candidateDmPolicy, + allowFrom: candidateAccount.config.allowFrom ?? [], + readAllowFromStore: pairing.readAllowFromStore, + senderOpenId: turn.senderId, + senderUserId: turn.senderUserId, + conversationId: turn.senderId, + mayPair, + }); + return { account: candidateAccount, cfg: candidateCfg, dmPolicy: candidateDmPolicy, ingress }; + }; + const rejectCommentAuthorization = async ( + authorization: Awaited>, + ) => { + if (authorization.ingress.ingress.admission === "pairing-required") { + const client = createFeishuClient(authorization.account); await pairing.issueChallenge({ senderId: turn.senderId, senderIdLine: `Your Feishu user id: ${turn.senderId}`, @@ -129,15 +134,28 @@ export async function handleFeishuCommentEvent( } else { log( `feishu[${account.accountId}]: blocked unauthorized comment sender ${turn.senderId} ` + - `(dmPolicy=${dmPolicy}, comment=${turn.commentId})`, + `(dmPolicy=${authorization.dmPolicy}, comment=${turn.commentId})`, ); } + }; + const commentAuthorization = await resolveCommentAuthorization(params.cfg, true); + if (commentAuthorization.ingress.ingress.admission !== "dispatch") { + await rejectCommentAuthorization(commentAuthorization); return; } let effectiveCfg = params.cfg; + const currentCfg = core.config.current() as ClawdbotConfig; + if (currentCfg !== effectiveCfg) { + const currentAuthorization = await resolveCommentAuthorization(currentCfg, true); + if (currentAuthorization.ingress.ingress.admission !== "dispatch") { + await rejectCommentAuthorization(currentAuthorization); + return; + } + effectiveCfg = currentCfg; + } let route = core.channel.routing.resolveAgentRoute({ - cfg: params.cfg, + cfg: effectiveCfg, channel: "feishu", accountId: account.accountId, peer: { @@ -146,31 +164,40 @@ export async function handleFeishuCommentEvent( }, }); if (route.matchedBy === "default") { - const dynamicCfg = feishuCfg?.dynamicAgentCreation as DynamicAgentCreationConfig | undefined; - if (dynamicCfg?.enabled) { - const dynamicResult = await maybeCreateDynamicAgent({ - cfg: params.cfg, - runtime: core, - senderOpenId: turn.senderId, - dynamicCfg, - configWritesAllowed: resolveChannelConfigWrites({ - cfg: params.cfg, - channelId: "feishu", - accountId: account.accountId, - }), - log: (message) => log(message), + const dynamicResult = await maybeCreateDynamicAgent({ + cfg: effectiveCfg, + runtime: core, + accountId: account.accountId, + senderOpenId: turn.senderId, + canCreateForConfig: async (candidateCfg) => { + const authorization = await resolveCommentAuthorization(candidateCfg, false); + return authorization.ingress.ingress.admission === "dispatch"; + }, + log: (message) => log(message), + }); + if (dynamicResult.created || dynamicResult.updatedCfg !== effectiveCfg) { + const refreshedAuthorization = await resolveCommentAuthorization( + dynamicResult.updatedCfg, + false, + ); + if (refreshedAuthorization.ingress.ingress.admission !== "dispatch") { + log( + `feishu[${account.accountId}]: current policy rejected stale comment sender ${turn.senderId} ` + + `before adopting refreshed dynamic route (dmPolicy=${refreshedAuthorization.dmPolicy}, comment=${turn.commentId})`, + ); + return; + } + effectiveCfg = dynamicResult.updatedCfg; + route = core.channel.routing.resolveAgentRoute({ + cfg: dynamicResult.updatedCfg, + channel: "feishu", + accountId: account.accountId, + peer: { + kind: "direct", + id: turn.senderId, + }, }); if (dynamicResult.created) { - effectiveCfg = dynamicResult.updatedCfg; - route = core.channel.routing.resolveAgentRoute({ - cfg: dynamicResult.updatedCfg, - channel: "feishu", - accountId: account.accountId, - peer: { - kind: "direct", - id: turn.senderId, - }, - }); log( `feishu[${account.accountId}]: dynamic agent created for comment flow, route=${route.sessionKey}`, ); diff --git a/extensions/feishu/src/dynamic-agent.test.ts b/extensions/feishu/src/dynamic-agent.test.ts index 4b6f5896e3d0..2702a4aa75d1 100644 --- a/extensions/feishu/src/dynamic-agent.test.ts +++ b/extensions/feishu/src/dynamic-agent.test.ts @@ -16,15 +16,33 @@ afterEach(async () => { await fs.promises.rm(tempRoot, { recursive: true, force: true }); }); -function createRuntime() { - const replaceConfigFile = vi.fn(async () => {}); +function createRuntime( + currentCfg?: OpenClawConfig, + persistedCfg?: OpenClawConfig, + mutationCfg?: OpenClawConfig, +) { + let runtimeCfg = structuredClone(currentCfg ?? ({} as OpenClawConfig)); + const commitConfig = vi.fn(); + const mutateConfigFile = vi.fn( + async (params: { + mutate: (draft: OpenClawConfig, context: { snapshot: never; previousHash: null }) => unknown; + }) => { + const draft = structuredClone(mutationCfg ?? runtimeCfg); + const result = await params.mutate(draft, { snapshot: {} as never, previousHash: null }); + runtimeCfg = draft; + commitConfig(); + return { nextConfig: persistedCfg ?? runtimeCfg, result }; + }, + ); return { runtime: { config: { - replaceConfigFile, + mutateConfigFile, + current: vi.fn(() => runtimeCfg), }, } as unknown as PluginRuntime, - replaceConfigFile, + commitConfig, + mutateConfigFile, }; } @@ -50,37 +68,396 @@ async function pathExists(target: string): Promise { describe("maybeCreateDynamicAgent", () => { it("does not persist dynamic agents when config writes are disabled", async () => { - const { runtime, replaceConfigFile } = createRuntime(); - const dynamicCfg = createDynamicConfig(); + const cfg = { + channels: { + feishu: { + configWrites: false, + dynamicAgentCreation: createDynamicConfig(), + }, + }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime, mutateConfigFile } = createRuntime(cfg); const result = await maybeCreateDynamicAgent({ - cfg: { - channels: { feishu: { configWrites: false } }, - agents: { list: [] }, - bindings: [], - } as OpenClawConfig, + cfg, runtime, + accountId: "default", senderOpenId: "ou_sender", - dynamicCfg, - configWritesAllowed: false, + canCreateForConfig: async () => true, log: vi.fn(), }); - expect(result).toEqual({ - created: false, - updatedCfg: { - channels: { feishu: { configWrites: false } }, - agents: { list: [] }, - bindings: [], - }, - }); - expect(replaceConfigFile).not.toHaveBeenCalled(); + expect(result).toEqual({ created: false, updatedCfg: cfg }); + expect(mutateConfigFile).not.toHaveBeenCalled(); expect(await pathExists(path.join(tempRoot, "workspace-feishu-ou_sender"))).toBe(false); expect(await pathExists(path.join(tempRoot, "agent-feishu-ou_sender"))).toBe(false); }); it("persists a sender agent and direct binding when config writes are allowed", async () => { - const { runtime, replaceConfigFile } = createRuntime(); + const cfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime, mutateConfigFile } = createRuntime(cfg); + + const result = await maybeCreateDynamicAgent({ + cfg, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(result.created).toBe(true); + expect(result.agentId).toBe("feishu-ou_sender"); + expect(mutateConfigFile).toHaveBeenCalledTimes(1); + expect(mutateConfigFile).toHaveBeenCalledWith({ + base: "runtime", + afterWrite: { mode: "auto" }, + mutate: expect.any(Function), + }); + expect(result.updatedCfg.agents?.list).toEqual([ + { + id: "feishu-ou_sender", + workspace: path.join(tempRoot, "workspace-feishu-ou_sender"), + agentDir: path.join(tempRoot, "agent-feishu-ou_sender"), + }, + ]); + expect(result.updatedCfg.bindings).toEqual([ + { + agentId: "feishu-ou_sender", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "direct", id: "ou_sender" }, + }, + }, + ]); + expect(await pathExists(path.join(tempRoot, "workspace-feishu-ou_sender"))).toBe(true); + expect(await pathExists(path.join(tempRoot, "agent-feishu-ou_sender"))).toBe(true); + }); + + it("does not create persistent state when current ingress denies the sender", async () => { + const cfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime, mutateConfigFile } = createRuntime(cfg); + + const result = await maybeCreateDynamicAgent({ + cfg, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig: async () => false, + log: vi.fn(), + }); + + expect(result).toEqual({ created: false, updatedCfg: cfg }); + expect(mutateConfigFile).not.toHaveBeenCalled(); + expect(await pathExists(path.join(tempRoot, "workspace-feishu-ou_sender"))).toBe(false); + expect(await pathExists(path.join(tempRoot, "agent-feishu-ou_sender"))).toBe(false); + }); + + it("rechecks current ingress inside the config mutation lock", async () => { + const cfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime, commitConfig, mutateConfigFile } = createRuntime(cfg); + const canCreateForConfig = vi + .fn<(cfg: OpenClawConfig) => Promise>() + .mockResolvedValueOnce(true) + .mockResolvedValueOnce(false); + + const result = await maybeCreateDynamicAgent({ + cfg, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig, + log: vi.fn(), + }); + + expect(result.created).toBe(false); + expect(canCreateForConfig).toHaveBeenCalledTimes(2); + expect(mutateConfigFile).toHaveBeenCalledTimes(1); + expect(commitConfig).not.toHaveBeenCalled(); + expect(result.updatedCfg.agents?.list).toEqual([]); + expect(result.updatedCfg.bindings).toEqual([]); + expect(await pathExists(path.join(tempRoot, "workspace-feishu-ou_sender"))).toBe(false); + expect(await pathExists(path.join(tempRoot, "agent-feishu-ou_sender"))).toBe(false); + }); + + it("preserves a non-peer route added before the config mutation lock", async () => { + const cfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const mutationCfg = { + ...cfg, + bindings: [ + { + agentId: "main", + match: { channel: "feishu", accountId: "default" }, + }, + ], + } as OpenClawConfig; + const { runtime, commitConfig, mutateConfigFile } = createRuntime(cfg, undefined, mutationCfg); + + const result = await maybeCreateDynamicAgent({ + cfg, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(result.created).toBe(false); + expect(result.updatedCfg).toEqual(mutationCfg); + expect(mutateConfigFile).toHaveBeenCalledTimes(1); + expect(commitConfig).not.toHaveBeenCalled(); + }); + + it("scopes bindings to the normalized account id", async () => { + const cfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime } = createRuntime(cfg); + + const result = await maybeCreateDynamicAgent({ + cfg, + runtime, + accountId: "Ops Team", + senderOpenId: "ou_sender", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(result.created).toBe(true); + expect(result.agentId).toMatch(/^feishu-ops-team-[a-f0-9]{32}$/); + expect(result.updatedCfg.bindings).toEqual([ + { + agentId: result.agentId, + match: { + channel: "feishu", + accountId: "ops-team", + peer: { kind: "direct", id: "ou_sender" }, + }, + }, + ]); + }); + + it("keeps named-account dynamic agent ids bounded and sender-unique", async () => { + const accountId = "a".repeat(64); + const cfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime } = createRuntime(cfg); + + const first = await maybeCreateDynamicAgent({ + cfg, + runtime, + accountId, + senderOpenId: "ou_sender_one_with_a_shared_long_prefix", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + const second = await maybeCreateDynamicAgent({ + cfg: first.updatedCfg, + runtime, + accountId, + senderOpenId: "ou_sender_two_with_a_shared_long_prefix", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(first.agentId).toHaveLength(52); + expect(second.agentId).toHaveLength(52); + expect(first.agentId).not.toBe(second.agentId); + expect(second.updatedCfg.agents?.list?.map((agent) => agent.id)).toEqual([ + first.agentId, + second.agentId, + ]); + }); + + it("uses the current maxAgents limit instead of stale request policy", async () => { + const cfg = { + channels: { + feishu: { + dynamicAgentCreation: { + ...createDynamicConfig(), + maxAgents: 1, + }, + }, + }, + agents: { + list: [ + { + id: "feishu-ou_existing", + workspace: path.join(tempRoot, "existing-workspace"), + agentDir: path.join(tempRoot, "existing-agent"), + }, + ], + }, + bindings: [], + } as OpenClawConfig; + const { runtime, mutateConfigFile } = createRuntime(cfg); + + const result = await maybeCreateDynamicAgent({ + cfg: { + channels: { + feishu: { + dynamicAgentCreation: { + ...createDynamicConfig(), + maxAgents: 2, + }, + }, + }, + agents: cfg.agents, + bindings: [], + } as OpenClawConfig, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(result.created).toBe(false); + expect(mutateConfigFile).not.toHaveBeenCalled(); + }); + + it("preserves concurrent runtime config when creating from a stale request snapshot", async () => { + const currentCfg = { + channels: { feishu: { dynamicAgentCreation: createDynamicConfig() } }, + agents: { + list: [ + { + id: "feishu-ou_existing", + workspace: path.join(tempRoot, "existing-workspace"), + agentDir: path.join(tempRoot, "existing-agent"), + }, + ], + }, + bindings: [ + { + agentId: "feishu-ou_existing", + match: { + channel: "feishu", + peer: { kind: "direct", id: "ou_existing" }, + }, + }, + ], + } as OpenClawConfig; + const { runtime, mutateConfigFile } = createRuntime(currentCfg); + + const result = await maybeCreateDynamicAgent({ + cfg: { agents: { list: [] }, bindings: [] } as OpenClawConfig, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(mutateConfigFile).toHaveBeenCalledWith({ + base: "runtime", + afterWrite: { mode: "auto" }, + mutate: expect.any(Function), + }); + expect(result.updatedCfg.agents?.list).toEqual([ + ...currentCfg.agents!.list!, + { + id: "feishu-ou_sender", + workspace: path.join(tempRoot, "workspace-feishu-ou_sender"), + agentDir: path.join(tempRoot, "agent-feishu-ou_sender"), + }, + ]); + expect(result.updatedCfg.bindings).toEqual([ + ...currentCfg.bindings!, + { + agentId: "feishu-ou_sender", + match: { + channel: "feishu", + accountId: "default", + peer: { kind: "direct", id: "ou_sender" }, + }, + }, + ]); + }); + + it("returns refreshed runtime config instead of the persisted source config", async () => { + const currentCfg = { + channels: { + feishu: { + appSecret: "resolved-secret", + dynamicAgentCreation: createDynamicConfig(), + }, + }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const persistedCfg = { + channels: { + feishu: { + appSecret: { source: "env", id: "FEISHU_APP_SECRET" }, + dynamicAgentCreation: createDynamicConfig(), + }, + }, + agents: { list: [] }, + bindings: [], + } as OpenClawConfig; + const { runtime } = createRuntime(currentCfg, persistedCfg); + + const result = await maybeCreateDynamicAgent({ + cfg: currentCfg, + runtime, + accountId: "default", + senderOpenId: "ou_sender", + canCreateForConfig: async () => true, + log: vi.fn(), + }); + + expect(result.updatedCfg.channels?.feishu?.appSecret).toBe("resolved-secret"); + expect(result.updatedCfg.bindings).toHaveLength(1); + }); + + it("returns runtime current binding even when config writes are disabled", async () => { + const currentCfg = { + channels: { feishu: { configWrites: false } }, + agents: { + list: [ + { + id: "feishu-ou_sender", + workspace: path.join(tempRoot, "existing-workspace"), + agentDir: path.join(tempRoot, "existing-agent"), + }, + ], + }, + bindings: [ + { + agentId: "feishu-ou_sender", + match: { + channel: "feishu", + peer: { kind: "direct", id: "ou_sender" }, + }, + }, + ], + } as OpenClawConfig; + const { runtime, mutateConfigFile } = createRuntime(currentCfg); const result = await maybeCreateDynamicAgent({ cfg: { @@ -88,69 +465,14 @@ describe("maybeCreateDynamicAgent", () => { bindings: [], } as OpenClawConfig, runtime, + accountId: "default", senderOpenId: "ou_sender", - dynamicCfg: createDynamicConfig(), - configWritesAllowed: true, - log: vi.fn(), - }); - - expect(result.created).toBe(true); - expect(result.agentId).toBe("feishu-ou_sender"); - expect(replaceConfigFile).toHaveBeenCalledTimes(1); - expect(replaceConfigFile).toHaveBeenCalledWith({ - nextConfig: { - agents: { - list: [ - { - id: "feishu-ou_sender", - workspace: path.join(tempRoot, "workspace-feishu-ou_sender"), - agentDir: path.join(tempRoot, "agent-feishu-ou_sender"), - }, - ], - }, - bindings: [ - { - agentId: "feishu-ou_sender", - match: { - channel: "feishu", - peer: { kind: "direct", id: "ou_sender" }, - }, - }, - ], - }, - afterWrite: { mode: "auto" }, - }); - expect(await pathExists(path.join(tempRoot, "workspace-feishu-ou_sender"))).toBe(true); - expect(await pathExists(path.join(tempRoot, "agent-feishu-ou_sender"))).toBe(true); - }); - - it("keeps the maxAgents limit before adding a missing binding", async () => { - const { runtime, replaceConfigFile } = createRuntime(); - - const result = await maybeCreateDynamicAgent({ - cfg: { - agents: { - list: [ - { - id: "feishu-ou_sender", - workspace: path.join(tempRoot, "existing-workspace"), - agentDir: path.join(tempRoot, "existing-agent"), - }, - ], - }, - bindings: [], - } as OpenClawConfig, - runtime, - senderOpenId: "ou_sender", - dynamicCfg: { - ...createDynamicConfig(), - maxAgents: 1, - }, - configWritesAllowed: true, + canCreateForConfig: async () => true, log: vi.fn(), }); expect(result.created).toBe(false); - expect(replaceConfigFile).not.toHaveBeenCalled(); + expect(result.updatedCfg).toStrictEqual(currentCfg); + expect(mutateConfigFile).not.toHaveBeenCalled(); }); }); diff --git a/extensions/feishu/src/dynamic-agent.ts b/extensions/feishu/src/dynamic-agent.ts index 530d5c6e5346..a40aab66dcd2 100644 --- a/extensions/feishu/src/dynamic-agent.ts +++ b/extensions/feishu/src/dynamic-agent.ts @@ -1,8 +1,12 @@ // Feishu plugin module implements dynamic agent behavior. +import { createHash } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { resolveChannelConfigWrites } from "openclaw/plugin-sdk/channel-config-writes"; +import { normalizeAccountId, resolveAgentRoute } from "openclaw/plugin-sdk/routing"; import type { OpenClawConfig, PluginRuntime } from "../runtime-api.js"; +import { resolveFeishuAccount } from "./accounts.js"; import type { DynamicAgentCreationConfig } from "./types.js"; type MaybeCreateDynamicAgentResult = { @@ -11,126 +15,197 @@ type MaybeCreateDynamicAgentResult = { agentId?: string; }; +type DynamicAgentMutationResult = { + created: boolean; + agentId?: string; +}; + +class DynamicAgentMutationSkipped extends Error { + constructor(readonly cfg: OpenClawConfig) { + super("dynamic agent mutation skipped"); + } +} + +function hasDefaultDirectRoute( + cfg: OpenClawConfig, + accountId: string, + senderOpenId: string, +): boolean { + return ( + resolveAgentRoute({ + cfg, + channel: "feishu", + accountId, + peer: { kind: "direct", id: senderOpenId }, + }).matchedBy === "default" + ); +} + +function resolveDynamicAgentConfig( + cfg: OpenClawConfig, + accountId: string, +): DynamicAgentCreationConfig | undefined { + return resolveFeishuAccount({ cfg, accountId }).config.dynamicAgentCreation as + | DynamicAgentCreationConfig + | undefined; +} + +function isAtDynamicAgentLimit( + cfg: OpenClawConfig, + dynamicCfg: DynamicAgentCreationConfig, +): boolean { + if (dynamicCfg.maxAgents === undefined) { + return false; + } + const feishuAgentCount = (cfg.agents?.list ?? []).filter((agent) => + agent.id.startsWith("feishu-"), + ).length; + return feishuAgentCount >= dynamicCfg.maxAgents; +} + +function resolveDynamicAgentId(accountId: string, senderOpenId: string): string { + if (accountId === "default") { + return `feishu-${senderOpenId}`; + } + const identityDigest = createHash("sha256") + .update(accountId) + .update("\0") + .update(senderOpenId) + .digest("hex") + .slice(0, 32); + return `feishu-${accountId.slice(0, 12)}-${identityDigest}`; +} + /** - * Check if a dynamic agent should be created for a DM user and create it if needed. - * This creates a unique agent instance with its own workspace for each DM user. + * Refresh an existing DM binding or create its dynamic agent when current + * account policy permits config writes. */ export async function maybeCreateDynamicAgent(params: { cfg: OpenClawConfig; runtime: PluginRuntime; + accountId: string; senderOpenId: string; - dynamicCfg: DynamicAgentCreationConfig; - configWritesAllowed: boolean; + canCreateForConfig: (cfg: OpenClawConfig) => Promise; log: (msg: string) => void; }): Promise { - const { cfg, runtime, senderOpenId, dynamicCfg, configWritesAllowed, log } = params; + const { cfg, runtime, senderOpenId, canCreateForConfig, log } = params; + const accountId = normalizeAccountId(params.accountId); - if (!configWritesAllowed) { + if (!hasDefaultDirectRoute(cfg, accountId, senderOpenId)) { + return { created: false, updatedCfg: cfg }; + } + + const currentCfg = runtime.config.current() as OpenClawConfig; + if (!hasDefaultDirectRoute(currentCfg, accountId, senderOpenId)) { + return { created: false, updatedCfg: currentCfg }; + } + + const currentDynamicCfg = resolveDynamicAgentConfig(currentCfg, accountId); + if (!currentDynamicCfg?.enabled) { + return { created: false, updatedCfg: currentCfg }; + } + if (!resolveChannelConfigWrites({ cfg: currentCfg, channelId: "feishu", accountId })) { log(`feishu: config writes disabled, not creating agent for ${senderOpenId}`); - return { created: false, updatedCfg: cfg }; + return { created: false, updatedCfg: currentCfg }; + } + const agentId = resolveDynamicAgentId(accountId, senderOpenId); + const currentAgentExists = (currentCfg.agents?.list ?? []).some((agent) => agent.id === agentId); + // Legacy unscoped agents are indistinguishable from valid default-account state. + // Keep maxAgents as a hard cap instead of auto-rebinding or deleting ambiguous user data. + if (!currentAgentExists && isAtDynamicAgentLimit(currentCfg, currentDynamicCfg)) { + log( + `feishu: maxAgents limit (${currentDynamicCfg.maxAgents}) reached, not creating agent for ${senderOpenId}`, + ); + return { created: false, updatedCfg: currentCfg }; + } + if (!(await canCreateForConfig(currentCfg))) { + return { created: false, updatedCfg: currentCfg }; } - // Check if there's already a binding for this user - const existingBindings = cfg.bindings ?? []; - const hasBinding = existingBindings.some( - (b) => - b.match?.channel === "feishu" && - b.match?.peer?.kind === "direct" && - b.match?.peer?.id === senderOpenId, - ); - - if (hasBinding) { - return { created: false, updatedCfg: cfg }; - } - - // Check maxAgents limit if configured - if (dynamicCfg.maxAgents !== undefined) { - const feishuAgentCount = (cfg.agents?.list ?? []).filter((a) => - a.id.startsWith("feishu-"), - ).length; - if (feishuAgentCount >= dynamicCfg.maxAgents) { - log( - `feishu: maxAgents limit (${dynamicCfg.maxAgents}) reached, not creating agent for ${senderOpenId}`, - ); - return { created: false, updatedCfg: cfg }; - } - } - - // Use full OpenID as agent ID suffix (OpenID format: ou_xxx is already filesystem-safe) - const agentId = `feishu-${senderOpenId}`; - - // Check if agent already exists (but binding was missing) - const existingAgent = (cfg.agents?.list ?? []).find((a) => a.id === agentId); - if (existingAgent) { - // Agent exists but binding doesn't - just add the binding - log(`feishu: agent "${agentId}" exists, adding missing binding for ${senderOpenId}`); - - const updatedCfg: OpenClawConfig = { - ...cfg, - bindings: [ - ...existingBindings, - { - agentId, - match: { - channel: "feishu", - peer: { kind: "direct", id: senderOpenId }, - }, - }, - ], - }; - - await runtime.config.replaceConfigFile({ - nextConfig: updatedCfg, + // The config mutation lock owns the final duplicate/limit checks. This keeps + // simultaneous DM creations and policy updates from producing stale writes. + let skippedCfg: OpenClawConfig | undefined; + const committed = await runtime.config + .mutateConfigFile({ + base: "runtime", afterWrite: { mode: "auto" }, + mutate: async (draft) => { + if (!hasDefaultDirectRoute(draft, accountId, senderOpenId)) { + throw new DynamicAgentMutationSkipped(draft); + } + + const dynamicCfg = resolveDynamicAgentConfig(draft, accountId); + if ( + !dynamicCfg?.enabled || + !resolveChannelConfigWrites({ cfg: draft, channelId: "feishu", accountId }) + ) { + throw new DynamicAgentMutationSkipped(draft); + } + const agentExists = (draft.agents?.list ?? []).some((agent) => agent.id === agentId); + if (!agentExists && isAtDynamicAgentLimit(draft, dynamicCfg)) { + log( + `feishu: maxAgents limit (${dynamicCfg.maxAgents}) reached, not creating agent for ${senderOpenId}`, + ); + throw new DynamicAgentMutationSkipped(draft); + } + if (!(await canCreateForConfig(draft))) { + throw new DynamicAgentMutationSkipped(draft); + } + + if (!agentExists) { + const workspaceTemplate = + dynamicCfg.workspaceTemplate ?? "~/.openclaw/workspace-{agentId}"; + const agentDirTemplate = + dynamicCfg.agentDirTemplate ?? "~/.openclaw/agents/{agentId}/agent"; + const workspace = resolveUserPath( + workspaceTemplate.replace("{userId}", senderOpenId).replace("{agentId}", agentId), + ); + const agentDir = resolveUserPath( + agentDirTemplate.replace("{userId}", senderOpenId).replace("{agentId}", agentId), + ); + log(`feishu: creating dynamic agent "${agentId}" for user ${senderOpenId}`); + log(` workspace: ${workspace}`); + log(` agentDir: ${agentDir}`); + await fs.promises.mkdir(workspace, { recursive: true }); + await fs.promises.mkdir(agentDir, { recursive: true }); + draft.agents = { + ...draft.agents, + list: [...(draft.agents?.list ?? []), { id: agentId, workspace, agentDir }], + }; + } else { + log(`feishu: agent "${agentId}" exists, adding missing binding for ${senderOpenId}`); + } + + draft.bindings = [ + ...(draft.bindings ?? []), + { + agentId, + match: { + channel: "feishu", + accountId, + peer: { kind: "direct", id: senderOpenId }, + }, + }, + ]; + return { created: true, agentId }; + }, + }) + .catch((error: unknown) => { + if (error instanceof DynamicAgentMutationSkipped) { + skippedCfg = error.cfg; + return null; + } + throw error; }); - return { created: true, updatedCfg, agentId }; + if (!committed) { + return { created: false, updatedCfg: skippedCfg ?? currentCfg }; } - // Resolve path templates with substitutions - const workspaceTemplate = dynamicCfg.workspaceTemplate ?? "~/.openclaw/workspace-{agentId}"; - const agentDirTemplate = dynamicCfg.agentDirTemplate ?? "~/.openclaw/agents/{agentId}/agent"; - - const workspace = resolveUserPath( - workspaceTemplate.replace("{userId}", senderOpenId).replace("{agentId}", agentId), - ); - const agentDir = resolveUserPath( - agentDirTemplate.replace("{userId}", senderOpenId).replace("{agentId}", agentId), - ); - - log(`feishu: creating dynamic agent "${agentId}" for user ${senderOpenId}`); - log(` workspace: ${workspace}`); - log(` agentDir: ${agentDir}`); - - // Create directories - await fs.promises.mkdir(workspace, { recursive: true }); - await fs.promises.mkdir(agentDir, { recursive: true }); - - // Update configuration with new agent and binding - const updatedCfg: OpenClawConfig = { - ...cfg, - agents: { - ...cfg.agents, - list: [...(cfg.agents?.list ?? []), { id: agentId, workspace, agentDir }], - }, - bindings: [ - ...existingBindings, - { - agentId, - match: { - channel: "feishu", - peer: { kind: "direct", id: senderOpenId }, - }, - }, - ], + return { + created: committed.result?.created ?? false, + updatedCfg: runtime.config.current() as OpenClawConfig, + agentId: committed.result?.agentId, }; - - // Write updated config using PluginRuntime API - await runtime.config.replaceConfigFile({ - nextConfig: updatedCfg, - afterWrite: { mode: "auto" }, - }); - - return { created: true, updatedCfg, agentId }; } /**