From c0f8288daa4a7ab2da4b2e1e570115f92c8ffc58 Mon Sep 17 00:00:00 2001 From: Shakker Date: Thu, 30 Jul 2026 04:16:10 +0100 Subject: [PATCH] fix: route ClickClack discussion mentions correctly --- extensions/clickclack/src/access.ts | 147 ++++++++++++++++++---- extensions/clickclack/src/inbound.test.ts | 62 +++++++++ extensions/clickclack/src/inbound.ts | 95 +------------- 3 files changed, 188 insertions(+), 116 deletions(-) diff --git a/extensions/clickclack/src/access.ts b/extensions/clickclack/src/access.ts index 31e467e3dcd1..5a7b02e38022 100644 --- a/extensions/clickclack/src/access.ts +++ b/extensions/clickclack/src/access.ts @@ -7,7 +7,12 @@ import { type StableChannelIngressIdentityParams, } from "openclaw/plugin-sdk/channel-ingress-runtime"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { normalizeAgentId } from "openclaw/plugin-sdk/routing"; +import { + normalizeAgentId, + type ResolvedAgentRoute, + type RoutePeer, +} from "openclaw/plugin-sdk/routing"; +import { resolveClickClackDiscussionRoute } from "./discussions/routing.js"; import { resolveClickClackGroupPolicy } from "./group-policy.js"; import { resolveClickClackMentionFacts } from "./mention-facts.js"; import { getClickClackRuntime } from "./runtime.js"; @@ -34,6 +39,117 @@ const clickClackIngressIdentity = { entryIdPrefix: "clickclack-user", } satisfies StableChannelIngressIdentityParams; +type ClickClackDiscussionRoute = Extract< + ReturnType, + { state: "active" } +>["route"]; + +type ClickClackPreparedInboundRoute = { + isDirect: boolean; + target: string; + route: ResolvedAgentRoute; + discussionRoute?: ClickClackDiscussionRoute; + revoked: boolean; +}; + +function resolveAccountAgentRoute(params: { + cfg: OpenClawConfig; + account: ResolvedClickClackAccount; + target: string; + isDirect: boolean; +}): ResolvedAgentRoute { + const runtime = getClickClackRuntime(); + const peer: RoutePeer = { + kind: params.isDirect ? "direct" : "channel", + id: params.target, + }; + const route = runtime.channel.routing.resolveAgentRoute({ + cfg: params.cfg, + channel: CHANNEL_ID, + accountId: params.account.accountId, + peer, + }); + const agentId = normalizeAgentId(params.account.agentId ?? route.agentId); + if (agentId === route.agentId) { + return route; + } + const dmScope = params.cfg.session?.dmScope ?? "main"; + // Account-level agent ownership changes only the agent prefix. Preserve the + // resolved session policy so outbound recipient routing reaches this key. + const sessionKey = runtime.channel.routing.buildAgentSessionKey({ + agentId, + mainKey: params.cfg.session?.mainKey, + channel: CHANNEL_ID, + accountId: params.account.accountId, + peer, + dmScope, + identityLinks: params.cfg.session?.identityLinks, + }); + const mainSessionKey = runtime.channel.routing.buildAgentSessionKey({ + agentId, + mainKey: params.cfg.session?.mainKey, + channel: CHANNEL_ID, + accountId: params.account.accountId, + dmScope: "main", + }); + return { + ...route, + agentId, + dmScope, + sessionKey, + mainSessionKey, + lastRoutePolicy: sessionKey === mainSessionKey ? "main" : "session", + }; +} + +function resolvePreparedInboundRoute(params: { + account: ResolvedClickClackAccount; + config: CoreConfig; + message: ClickClackMessage; +}): ClickClackPreparedInboundRoute { + const runtime = getClickClackRuntime(); + const isDirect = Boolean(params.message.direct_conversation_id); + const target = buildClickClackTarget( + isDirect + ? { chatType: "direct", kind: "dm", id: params.message.author_id } + : { chatType: "group", kind: "channel", id: params.message.channel_id ?? "" }, + ); + const accountRoute = resolveAccountAgentRoute({ + cfg: params.config as OpenClawConfig, + account: params.account, + target, + isDirect, + }); + const discussionResolution = + !isDirect && params.message.channel_id + ? resolveClickClackDiscussionRoute({ + runtime, + config: params.config, + accountId: params.account.accountId, + serverBaseUrl: params.account.baseUrl, + workspaceId: params.message.workspace_id, + channelId: params.message.channel_id, + }) + : { state: "unbound" as const }; + const discussionRoute = + discussionResolution.state === "active" ? discussionResolution.route : undefined; + + return { + isDirect, + target, + route: discussionRoute + ? { + ...accountRoute, + agentId: discussionRoute.agentId, + sessionKey: discussionRoute.sessionKey, + lastRoutePolicy: "session", + } + : accountRoute, + discussionRoute, + revoked: discussionResolution.state === "revoked", + }; +} + /** * Dispatch and command authorization decision for one inbound ClickClack * message. @@ -48,6 +164,7 @@ export type ClickClackInboundAccess = { wasMentioned: boolean; hasAnyMention?: boolean; }; + preparedRoute: ClickClackPreparedInboundRoute; }; /** @@ -60,23 +177,8 @@ export async function resolveClickClackInboundAccess(params: { message: ClickClackMessage; }): Promise { const runtime = getClickClackRuntime(); - const isDirect = Boolean(params.message.direct_conversation_id); const cfg = params.config as OpenClawConfig; - const target = buildClickClackTarget( - isDirect - ? { chatType: "direct", kind: "dm", id: params.message.author_id } - : { chatType: "group", kind: "channel", id: params.message.channel_id ?? "" }, - ); - const route = runtime.channel.routing.resolveAgentRoute({ - cfg, - channel: CHANNEL_ID, - accountId: params.account.accountId, - peer: { - kind: isDirect ? "direct" : "channel", - id: target, - }, - }); - const agentId = normalizeAgentId(params.account.agentId ?? route.agentId); + const preparedRoute = resolvePreparedInboundRoute(params); const shouldCheckCommand = runtime.channel.commands.shouldComputeCommandAuthorized( params.message.body, cfg, @@ -88,12 +190,12 @@ export async function resolveClickClackInboundAccess(params: { channelId: params.message.channel_id, }); const mentionFacts = resolveClickClackMentionFacts({ - isDirect, + isDirect: preparedRoute.isDirect, body: params.message.body, mentionPatterns: effectiveGroupPolicy.mentionPatterns, botHandle: params.account.botHandle, cfg, - agentId, + agentId: preparedRoute.route.agentId, channelId: params.message.channel_id, }); const allowTextCommands = @@ -111,8 +213,8 @@ export async function resolveClickClackInboundAccess(params: { cfg, subject: { stableId: params.message.author_id }, conversation: { - kind: isDirect ? "direct" : "group", - id: isDirect + kind: preparedRoute.isDirect ? "direct" : "group", + id: preparedRoute.isDirect ? (params.message.direct_conversation_id ?? params.message.author_id) : (params.message.channel_id ?? params.message.thread_root_id), }, @@ -135,11 +237,12 @@ export async function resolveClickClackInboundAccess(params: { }); return { - shouldDispatch: resolved.ingress.admission === "dispatch", + shouldDispatch: !preparedRoute.revoked && resolved.ingress.admission === "dispatch", commandAuthorized: resolved.commandAccess.requested ? resolved.commandAccess.authorized : resolved.senderAccess.allowed, requireMention: effectiveGroupPolicy.requireMention, mentionFacts, + preparedRoute, }; } diff --git a/extensions/clickclack/src/inbound.test.ts b/extensions/clickclack/src/inbound.test.ts index b749992cc822..e3169f71ae58 100644 --- a/extensions/clickclack/src/inbound.test.ts +++ b/extensions/clickclack/src/inbound.test.ts @@ -661,6 +661,68 @@ describe("handleClickClackInbound", () => { expect(dispatch.mock.calls[0]?.[0].ctxPayload.GroupSystemPrompt).toContain("sessions_send"); }); + it.each([ + { body: "@research investigate this", shouldDispatch: true }, + { body: "@service investigate this", shouldDispatch: false }, + ])( + "evaluates $body against the managed discussion agent before dispatch", + async ({ body, shouldDispatch }) => { + const runtime = createRuntime(); + setClickClackRuntime(runtime); + getClickClackDiscussionBindingStore(runtime).set("agent:research:main", { + accountId: "default", + agentId: "research", + sessionId: "session-id", + serverBaseUrl: "http://127.0.0.1:8080", + externalRef: "openclaw:test:research-mentions", + externalUrl: "", + workspaceRef: "wsp_1", + workspaceId: "wsp_1", + channelId: "chn_1", + channelRouteId: "discussion-route", + workspaceRouteId: "workspace-route", + section: "Sessions", + archived: false, + label: "Research mentions", + }); + + await handleClickClackInbound({ + account: createAgentAccount({ + agentId: "service-bot", + requireMention: true, + discussions: { enabled: true, workspace: "wsp_1", section: "Sessions" }, + }), + config: { + agents: { + entries: { + research: { groupChat: { mentionPatterns: ["@research"] } }, + "service-bot": { groupChat: { mentionPatterns: ["@service"] } }, + }, + }, + channels: { + clickclack: { + enabled: true, + baseUrl: "http://127.0.0.1:8080", + token: "test-token-placeholder", + workspace: "wsp_1", + discussions: { enabled: true, workspace: "wsp_1" }, + }, + }, + } satisfies CoreConfig, + message: createMessage({ body }), + }); + + const dispatch = vi.mocked(runtime.channel.inbound.dispatch); + expect(dispatch).toHaveBeenCalledTimes(shouldDispatch ? 1 : 0); + if (shouldDispatch) { + expect(dispatch.mock.calls[0]?.[0]).toMatchObject({ + route: { agentId: "research" }, + ctxPayload: { WasMentioned: true }, + }); + } + }, + ); + it("drops an old bound channel after the main session is replaced", async () => { const runtime = createRuntime(); setClickClackRuntime(runtime); diff --git a/extensions/clickclack/src/inbound.ts b/extensions/clickclack/src/inbound.ts index 24e9d155d740..bb33ada3fc23 100644 --- a/extensions/clickclack/src/inbound.ts +++ b/extensions/clickclack/src/inbound.ts @@ -5,14 +5,11 @@ import { deriveDurableFinalDeliveryRequirements } from "openclaw/plugin-sdk/chan * routes resulting outbound text back to ClickClack. */ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { normalizeAgentId } from "openclaw/plugin-sdk/routing"; import { resolveClickClackInboundAccess, type ClickClackInboundAccess } from "./access.js"; import { createClickClackActivityPublisher, type ClickClackActivityPublisher } from "./activity.js"; -import { resolveClickClackDiscussionRoute } from "./discussions/routing.js"; import { createClickClackClient } from "./http-client.js"; import { sendClickClackText } from "./outbound.js"; import { getClickClackRuntime } from "./runtime.js"; -import { buildClickClackTarget } from "./target.js"; import type { ClickClackMessage, ClickClackMessageProvenance, @@ -37,59 +34,6 @@ function resolveClickClackAgentRunId(messageId: string): string | undefined { return CLICKCLACK_MESSAGE_ID_PATTERN.test(messageId) ? `${CHANNEL_ID}:${messageId}` : undefined; } -function resolveAccountAgentRoute(params: { - cfg: OpenClawConfig; - account: ResolvedClickClackAccount; - target: string; - isDirect: boolean; -}) { - const runtime = getClickClackRuntime(); - const route = runtime.channel.routing.resolveAgentRoute({ - cfg: params.cfg, - channel: CHANNEL_ID, - accountId: params.account.accountId, - peer: { - kind: params.isDirect ? "direct" : "channel", - id: params.target, - }, - }); - const agentId = normalizeAgentId(params.account.agentId ?? route.agentId); - if (agentId === route.agentId) { - return route; - } - const peer = { - kind: params.isDirect ? ("direct" as const) : ("channel" as const), - id: params.target, - }; - const dmScope = params.cfg.session?.dmScope ?? "main"; - // Account-level agent ownership changes only the agent prefix. Preserve the - // resolved session policy so outbound recipient routing reaches this key. - const sessionKey = runtime.channel.routing.buildAgentSessionKey({ - agentId, - mainKey: params.cfg.session?.mainKey, - channel: CHANNEL_ID, - accountId: params.account.accountId, - peer, - dmScope, - identityLinks: params.cfg.session?.identityLinks, - }); - const mainSessionKey = runtime.channel.routing.buildAgentSessionKey({ - agentId, - mainKey: params.cfg.session?.mainKey, - channel: CHANNEL_ID, - accountId: params.account.accountId, - dmScope: "main", - }); - return { - ...route, - agentId, - dmScope, - sessionKey, - mainSessionKey, - lastRoutePolicy: sessionKey === mainSessionKey ? "main" : "session", - }; -} - async function dispatchModelReply(params: { account: ResolvedClickClackAccount; cfg: OpenClawConfig; @@ -156,44 +100,7 @@ export async function handleClickClackInbound(params: { if (!conversationId) { return; } - const isDirect = Boolean(message.direct_conversation_id); - const target = buildClickClackTarget( - isDirect - ? { chatType: "direct", kind: "dm", id: message.author_id } - : { chatType: "group", kind: "channel", id: message.channel_id ?? "" }, - ); - const accountRoute = resolveAccountAgentRoute({ - cfg: params.config as OpenClawConfig, - account: params.account, - target, - isDirect, - }); - const discussionResolution = - !isDirect && message.channel_id - ? resolveClickClackDiscussionRoute({ - runtime, - config: params.config, - accountId: params.account.accountId, - serverBaseUrl: params.account.baseUrl, - workspaceId: message.workspace_id, - channelId: message.channel_id, - }) - : { state: "unbound" as const }; - // A managed channel whose binding lost authority must never fall through to - // the account's ordinary agent/session. Reconciliation archives it separately. - if (discussionResolution.state === "revoked") { - return; - } - const discussionRoute = - discussionResolution.state === "active" ? discussionResolution.route : undefined; - const route = discussionRoute - ? { - ...accountRoute, - agentId: discussionRoute.agentId, - sessionKey: discussionRoute.sessionKey, - lastRoutePolicy: "session" as const, - } - : accountRoute; + const { discussionRoute, isDirect, route, target } = access.preparedRoute; if (params.account.replyMode === "model" && !discussionRoute) { await dispatchModelReply({ account: params.account,