From 69eea2cb80e1a103296f9849e6e2a0f265489aef Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 30 Mar 2026 07:35:38 +0900 Subject: [PATCH] refactor: split approval auth delivery and rendering --- extensions/discord/src/channel.test.ts | 10 +- extensions/discord/src/channel.ts | 133 +++++++++--------- extensions/telegram/src/channel.test.ts | 2 +- extensions/telegram/src/channel.ts | 130 ++++++++--------- src/channels/plugins/exec-approval-local.ts | 2 +- src/channels/plugins/types.adapters.ts | 51 +++++-- src/infra/channel-approval-auth.test.ts | 8 +- src/infra/channel-approval-auth.ts | 2 +- src/infra/exec-approval-forwarder.test.ts | 22 ++- src/infra/exec-approval-forwarder.ts | 14 +- src/infra/exec-approval-surface.test.ts | 28 +++- src/infra/exec-approval-surface.ts | 4 +- src/infra/plugin-approval-forwarder.test.ts | 16 ++- .../approval-delivery-helpers.test.ts | 26 ++-- src/plugin-sdk/approval-delivery-helpers.ts | 118 ++++++++-------- src/plugin-sdk/approval-renderers.test.ts | 63 +++++++++ src/plugin-sdk/approval-renderers.ts | 46 ++++++ src/plugin-sdk/approval-runtime.ts | 4 + 18 files changed, 434 insertions(+), 245 deletions(-) create mode 100644 src/plugin-sdk/approval-renderers.test.ts create mode 100644 src/plugin-sdk/approval-renderers.ts diff --git a/extensions/discord/src/channel.test.ts b/extensions/discord/src/channel.test.ts index b59ffd3e71e..75515740b67 100644 --- a/extensions/discord/src/channel.test.ts +++ b/extensions/discord/src/channel.test.ts @@ -154,7 +154,7 @@ describe("discordPlugin outbound", () => { enabled: true, approvers: ["123"], }; - const payload = discordPlugin.execApprovals?.buildPluginPendingPayload?.({ + const payload = discordPlugin.execApprovals?.render?.plugin?.buildPendingPayload?.({ cfg, request: createPluginApprovalRequest(), target: { channel: "discord", to: "user:123" }, @@ -179,7 +179,7 @@ describe("discordPlugin outbound", () => { enabled: true, approvers: ["123"], }; - const payload = discordPlugin.execApprovals?.buildPluginPendingPayload?.({ + const payload = discordPlugin.execApprovals?.render?.plugin?.buildPendingPayload?.({ cfg, request: createPluginApprovalRequest({ title: "Heads up @everyone <@123> <@&456>", @@ -210,7 +210,7 @@ describe("discordPlugin outbound", () => { }); it("falls back to non-interactive plugin approval pending payload when Discord exec approvals are disabled", () => { - const payload = discordPlugin.execApprovals?.buildPluginPendingPayload?.({ + const payload = discordPlugin.execApprovals?.render?.plugin?.buildPendingPayload?.({ cfg: createCfg(), request: createPluginApprovalRequest(), target: { channel: "discord", to: "user:123" }, @@ -230,7 +230,7 @@ describe("discordPlugin outbound", () => { }); it("builds rich plugin approval resolved payloads when request snapshot is available", () => { - const payload = discordPlugin.execApprovals?.buildPluginResolvedPayload?.({ + const payload = discordPlugin.execApprovals?.render?.plugin?.buildResolvedPayload?.({ cfg: createCfg(), resolved: createPluginApprovalResolved(createPluginApprovalRequest().request), target: { channel: "discord", to: "user:123" }, @@ -245,7 +245,7 @@ describe("discordPlugin outbound", () => { }); it("falls back to plain text plugin resolved payload when request snapshot is missing", () => { - const payload = discordPlugin.execApprovals?.buildPluginResolvedPayload?.({ + const payload = discordPlugin.execApprovals?.render?.plugin?.buildResolvedPayload?.({ cfg: createCfg(), resolved: createPluginApprovalResolved(undefined), target: { channel: "discord", to: "user:123" }, diff --git a/extensions/discord/src/channel.ts b/extensions/discord/src/channel.ts index 4a6a1936a7c..b50de6213a9 100644 --- a/extensions/discord/src/channel.ts +++ b/extensions/discord/src/channel.ts @@ -5,9 +5,11 @@ import { createNestedAllowlistOverrideResolver, } from "openclaw/plugin-sdk/allowlist-config-edit"; import { + buildPluginApprovalPendingReplyPayload, createApproverRestrictedNativeApprovalAdapter, buildPluginApprovalRequestMessage, buildPluginApprovalResolvedMessage, + buildPluginApprovalResolvedReplyPayload, type PluginApprovalRequest, type PluginApprovalResolved, } from "openclaw/plugin-sdk/approval-runtime"; @@ -300,6 +302,20 @@ function buildDiscordCrossContextComponents(params: { return [new DiscordUiContainer({ cfg: params.cfg, accountId: params.accountId, components })]; } +const discordNativeApprovalAdapter = createApproverRestrictedNativeApprovalAdapter({ + channel: "discord", + channelLabel: "Discord", + listAccountIds: listDiscordAccountIds, + hasApprovers: ({ cfg, accountId }) => + getDiscordExecApprovalApprovers({ cfg, accountId }).length > 0, + isExecAuthorizedSender: ({ cfg, accountId, senderId }) => + isDiscordExecApprovalApprover({ cfg, accountId, senderId }), + isNativeDeliveryEnabled: ({ cfg, accountId }) => + isDiscordExecApprovalClientEnabled({ cfg, accountId }), + resolveNativeDeliveryMode: ({ cfg, accountId }) => + resolveDiscordAccount({ cfg, accountId }).config.execApprovals?.target ?? "dm", +}); + const resolveDiscordAllowlistGroupOverrides = createNestedAllowlistOverrideResolver({ resolveRecord: (account: ResolvedDiscordAccount) => account.config.guilds, outerLabel: (guildKey) => `guild ${guildKey}`, @@ -480,73 +496,58 @@ export const discordPlugin: ChannelPlugin }, }, execApprovals: { - ...createApproverRestrictedNativeApprovalAdapter({ - channel: "discord", - channelLabel: "Discord", - listAccountIds: listDiscordAccountIds, - hasApprovers: ({ cfg, accountId }) => - getDiscordExecApprovalApprovers({ cfg, accountId }).length > 0, - isExecAuthorizedSender: ({ cfg, accountId, senderId }) => - isDiscordExecApprovalApprover({ cfg, accountId, senderId }), - isNativeDeliveryEnabled: ({ cfg, accountId }) => - isDiscordExecApprovalClientEnabled({ cfg, accountId }), - resolveNativeDeliveryMode: ({ cfg, accountId }) => - resolveDiscordAccount({ cfg, accountId }).config.execApprovals?.target ?? "dm", - }), - shouldSuppressLocalPrompt: ({ cfg, accountId, payload }) => - shouldSuppressLocalDiscordExecApprovalPrompt({ - cfg, - accountId, - payload, - }), - buildPluginPendingPayload: ({ cfg, request, target, nowMs }) => { - const text = formatDiscordApprovalPreview( - buildPluginApprovalRequestMessage(request, nowMs), - 10_000, - ); - const execApproval = { - approvalId: request.id, - approvalSlug: request.id.slice(0, 8), - allowedDecisions: ["allow-once", "allow-always", "deny"] as const, - }; - const normalizedChannel = normalizeMessageChannel(target.channel) ?? target.channel; - const interactiveEnabled = - normalizedChannel === "discord" && - isDiscordExecApprovalClientEnabled({ cfg, accountId: target.accountId }); - if (!interactiveEnabled) { - return { - text, - channelData: { - execApproval, - }, - }; - } - return { - text, - channelData: { - execApproval, - discord: { - components: buildDiscordPluginPendingComponentSpec({ request }), - }, - }, - }; + auth: discordNativeApprovalAdapter.auth, + delivery: { + ...discordNativeApprovalAdapter.delivery, + shouldSuppressLocalPrompt: ({ cfg, accountId, payload }) => + shouldSuppressLocalDiscordExecApprovalPrompt({ + cfg, + accountId, + payload, + }), }, - buildPluginResolvedPayload: ({ resolved }) => { - const componentSpec = buildDiscordPluginResolvedComponentSpec({ resolved }); - const text = formatDiscordApprovalPreview( - buildPluginApprovalResolvedMessage(resolved), - 10_000, - ); - return componentSpec - ? { - text, - channelData: { - discord: { - components: componentSpec, - }, - }, - } - : { text }; + render: { + plugin: { + buildPendingPayload: ({ cfg, request, target, nowMs }) => { + const normalizedChannel = normalizeMessageChannel(target.channel) ?? target.channel; + const interactiveEnabled = + normalizedChannel === "discord" && + isDiscordExecApprovalClientEnabled({ cfg, accountId: target.accountId }); + return buildPluginApprovalPendingReplyPayload({ + request, + nowMs, + text: formatDiscordApprovalPreview( + buildPluginApprovalRequestMessage(request, nowMs), + 10_000, + ), + approvalSlug: request.id.slice(0, 8), + channelData: interactiveEnabled + ? { + discord: { + components: buildDiscordPluginPendingComponentSpec({ request }), + }, + } + : undefined, + }); + }, + buildResolvedPayload: ({ resolved }) => { + const componentSpec = buildDiscordPluginResolvedComponentSpec({ resolved }); + return buildPluginApprovalResolvedReplyPayload({ + resolved, + text: formatDiscordApprovalPreview( + buildPluginApprovalResolvedMessage(resolved), + 10_000, + ), + channelData: componentSpec + ? { + discord: { + components: componentSpec, + }, + } + : undefined, + }); + }, + }, }, }, directory: createChannelDirectoryAdapter({ diff --git a/extensions/telegram/src/channel.test.ts b/extensions/telegram/src/channel.test.ts index 2fe7ac10dc8..aceb237d336 100644 --- a/extensions/telegram/src/channel.test.ts +++ b/extensions/telegram/src/channel.test.ts @@ -582,7 +582,7 @@ describe("telegramPlugin duplicate token guard", () => { it("builds plugin approval pending payload with callback ids that preserve allow-always", () => { const request = createPluginApprovalRequest(); - const payload = telegramPlugin.execApprovals?.buildPluginPendingPayload?.({ + const payload = telegramPlugin.execApprovals?.render?.plugin?.buildPendingPayload?.({ cfg: createCfg(), request, target: { channel: "telegram", to: "12345" }, diff --git a/extensions/telegram/src/channel.ts b/extensions/telegram/src/channel.ts index ece83102b56..fd16145b5dd 100644 --- a/extensions/telegram/src/channel.ts +++ b/extensions/telegram/src/channel.ts @@ -3,6 +3,7 @@ import { createNestedAllowlistOverrideResolver, } from "openclaw/plugin-sdk/allowlist-config-edit"; import { + buildPluginApprovalPendingReplyPayload, buildPluginApprovalRequestMessage, createApproverRestrictedNativeApprovalAdapter, } from "openclaw/plugin-sdk/approval-runtime"; @@ -325,6 +326,25 @@ function resolveTelegramOutboundSessionRoute(params: { }; } +const telegramNativeApprovalAdapter = createApproverRestrictedNativeApprovalAdapter({ + channel: "telegram", + channelLabel: "Telegram", + listAccountIds: listTelegramAccountIds, + hasApprovers: ({ cfg, accountId }) => + getTelegramExecApprovalApprovers({ cfg, accountId }).length > 0, + isExecAuthorizedSender: ({ cfg, accountId, senderId }) => + isTelegramExecApprovalAuthorizedSender({ cfg, accountId, senderId }), + isPluginAuthorizedSender: ({ cfg, accountId, senderId }) => + isTelegramExecApprovalApprover({ cfg, accountId, senderId }), + isNativeDeliveryEnabled: ({ cfg, accountId }) => + isTelegramExecApprovalClientEnabled({ cfg, accountId }), + resolveNativeDeliveryMode: ({ cfg, accountId }) => + resolveTelegramExecApprovalTarget({ cfg, accountId }), + requireMatchingTurnSourceChannel: true, + resolveSuppressionAccountId: ({ target, request }) => + target.accountId?.trim() || request.request.turnSourceAccountId?.trim() || undefined, +}); + const telegramMessageActions: ChannelMessageActionAdapter = { describeMessageTool: (ctx) => getTelegramRuntime().channel.telegram.messageActions?.describeMessageTool?.(ctx) ?? null, @@ -450,72 +470,54 @@ export const telegramPlugin = createChatChannelPlugin({ }, }, execApprovals: { - ...createApproverRestrictedNativeApprovalAdapter({ - channel: "telegram", - channelLabel: "Telegram", - listAccountIds: listTelegramAccountIds, - hasApprovers: ({ cfg, accountId }) => - getTelegramExecApprovalApprovers({ cfg, accountId }).length > 0, - isExecAuthorizedSender: ({ cfg, accountId, senderId }) => - isTelegramExecApprovalAuthorizedSender({ cfg, accountId, senderId }), - isPluginAuthorizedSender: ({ cfg, accountId, senderId }) => - isTelegramExecApprovalApprover({ cfg, accountId, senderId }), - isNativeDeliveryEnabled: ({ cfg, accountId }) => - isTelegramExecApprovalClientEnabled({ cfg, accountId }), - resolveNativeDeliveryMode: ({ cfg, accountId }) => - resolveTelegramExecApprovalTarget({ cfg, accountId }), - requireMatchingTurnSourceChannel: true, - resolveSuppressionAccountId: ({ target, request }) => - target.accountId?.trim() || request.request.turnSourceAccountId?.trim() || undefined, - }), - buildPendingPayload: ({ request, nowMs }) => - buildTelegramExecApprovalPendingPayload({ request, nowMs }), - beforeDeliverPending: async ({ cfg, target, payload }) => { - const hasExecApprovalData = - payload.channelData && - typeof payload.channelData === "object" && - !Array.isArray(payload.channelData) && - payload.channelData.execApproval; - if (!hasExecApprovalData) { - return; - } - const threadId = - typeof target.threadId === "number" - ? target.threadId - : typeof target.threadId === "string" - ? Number.parseInt(target.threadId, 10) - : undefined; - await sendTypingTelegram(target.to, { - cfg, - accountId: target.accountId ?? undefined, - ...(Number.isFinite(threadId) ? { messageThreadId: threadId } : {}), - }).catch(() => {}); + auth: telegramNativeApprovalAdapter.auth, + delivery: { + ...telegramNativeApprovalAdapter.delivery, + beforeDeliverPending: async ({ cfg, target, payload }) => { + const hasExecApprovalData = + payload.channelData && + typeof payload.channelData === "object" && + !Array.isArray(payload.channelData) && + payload.channelData.execApproval; + if (!hasExecApprovalData) { + return; + } + const threadId = + typeof target.threadId === "number" + ? target.threadId + : typeof target.threadId === "string" + ? Number.parseInt(target.threadId, 10) + : undefined; + await sendTypingTelegram(target.to, { + cfg, + accountId: target.accountId ?? undefined, + ...(Number.isFinite(threadId) ? { messageThreadId: threadId } : {}), + }).catch(() => {}); + }, }, - buildPluginPendingPayload: ({ request, nowMs }) => { - const text = buildPluginApprovalRequestMessage(request, nowMs); - const buttons = buildTelegramExecApprovalButtons(request.id); - const execApproval = { - approvalId: request.id, - approvalSlug: request.id, - allowedDecisions: ["allow-once", "allow-always", "deny"] as const, - }; - if (!buttons) { - return { - text, - channelData: { - execApproval, - }, - }; - } - return { - text, - channelData: { - execApproval, - telegram: { - buttons, - }, + render: { + exec: { + buildPendingPayload: ({ request, nowMs }) => + buildTelegramExecApprovalPendingPayload({ request, nowMs }), + }, + plugin: { + buildPendingPayload: ({ request, nowMs }) => { + const buttons = buildTelegramExecApprovalButtons(request.id); + return buildPluginApprovalPendingReplyPayload({ + request, + nowMs, + text: buildPluginApprovalRequestMessage(request, nowMs), + approvalSlug: request.id, + channelData: buttons + ? { + telegram: { + buttons, + }, + } + : undefined, + }); }, - }; + }, }, }, directory: createChannelDirectoryAdapter({ diff --git a/src/channels/plugins/exec-approval-local.ts b/src/channels/plugins/exec-approval-local.ts index 0f4f409e0ae..a7acafe96e9 100644 --- a/src/channels/plugins/exec-approval-local.ts +++ b/src/channels/plugins/exec-approval-local.ts @@ -13,7 +13,7 @@ export function shouldSuppressLocalExecApprovalPrompt(params: { return false; } return ( - getChannelPlugin(channel)?.execApprovals?.shouldSuppressLocalPrompt?.({ + getChannelPlugin(channel)?.execApprovals?.delivery?.shouldSuppressLocalPrompt?.({ cfg: params.cfg, accountId: params.accountId, payload: params.payload, diff --git a/src/channels/plugins/types.adapters.ts b/src/channels/plugins/types.adapters.ts index 4858006d9d8..58f44ff6683 100644 --- a/src/channels/plugins/types.adapters.ts +++ b/src/channels/plugins/types.adapters.ts @@ -465,7 +465,7 @@ export type ChannelLifecycleAdapter = { }) => Promise | void; }; -export type ChannelExecApprovalAdapter = { +export type ChannelExecApprovalAuthAdapter = { authorizeCommand?: (params: { cfg: OpenClawConfig; accountId?: string | null; @@ -479,6 +479,9 @@ export type ChannelExecApprovalAdapter = { cfg: OpenClawConfig; accountId?: string | null; }) => ChannelExecApprovalInitiatingSurfaceState; +}; + +export type ChannelExecApprovalDeliveryAdapter = { shouldSuppressLocalPrompt?: (params: { cfg: OpenClawConfig; accountId?: string | null; @@ -506,17 +509,41 @@ export type ChannelExecApprovalAdapter = { target: ChannelExecApprovalForwardTarget; payload: ReplyPayload; }) => Promise | void; - buildPluginPendingPayload?: (params: { - cfg: OpenClawConfig; - request: PluginApprovalRequest; - target: ChannelExecApprovalForwardTarget; - nowMs: number; - }) => ReplyPayload | null; - buildPluginResolvedPayload?: (params: { - cfg: OpenClawConfig; - resolved: PluginApprovalResolved; - target: ChannelExecApprovalForwardTarget; - }) => ReplyPayload | null; +}; + +export type ChannelExecApprovalRenderAdapter = { + exec?: { + buildPendingPayload?: (params: { + cfg: OpenClawConfig; + request: ExecApprovalRequest; + target: ChannelExecApprovalForwardTarget; + nowMs: number; + }) => ReplyPayload | null; + buildResolvedPayload?: (params: { + cfg: OpenClawConfig; + resolved: ExecApprovalResolved; + target: ChannelExecApprovalForwardTarget; + }) => ReplyPayload | null; + }; + plugin?: { + buildPendingPayload?: (params: { + cfg: OpenClawConfig; + request: PluginApprovalRequest; + target: ChannelExecApprovalForwardTarget; + nowMs: number; + }) => ReplyPayload | null; + buildResolvedPayload?: (params: { + cfg: OpenClawConfig; + resolved: PluginApprovalResolved; + target: ChannelExecApprovalForwardTarget; + }) => ReplyPayload | null; + }; +}; + +export type ChannelExecApprovalAdapter = { + auth?: ChannelExecApprovalAuthAdapter; + delivery?: ChannelExecApprovalDeliveryAdapter; + render?: ChannelExecApprovalRenderAdapter; }; export type ChannelAllowlistAdapter = { diff --git a/src/infra/channel-approval-auth.test.ts b/src/infra/channel-approval-auth.test.ts index 904b94ff8e8..b0049973a01 100644 --- a/src/infra/channel-approval-auth.test.ts +++ b/src/infra/channel-approval-auth.test.ts @@ -27,8 +27,12 @@ describe("resolveApprovalCommandAuthorization", () => { it("delegates to the channel approval override when present", () => { getChannelPluginMock.mockReturnValue({ execApprovals: { - authorizeCommand: ({ kind }: { kind: "exec" | "plugin" }) => - kind === "plugin" ? { authorized: false, reason: "plugin denied" } : { authorized: true }, + auth: { + authorizeCommand: ({ kind }: { kind: "exec" | "plugin" }) => + kind === "plugin" + ? { authorized: false, reason: "plugin denied" } + : { authorized: true }, + }, }, }); diff --git a/src/infra/channel-approval-auth.ts b/src/infra/channel-approval-auth.ts index 7264ad3e7fc..f3ceaeb7ce1 100644 --- a/src/infra/channel-approval-auth.ts +++ b/src/infra/channel-approval-auth.ts @@ -14,7 +14,7 @@ export function resolveApprovalCommandAuthorization(params: { return { authorized: true }; } return ( - getChannelPlugin(channel)?.execApprovals?.authorizeCommand?.({ + getChannelPlugin(channel)?.execApprovals?.auth?.authorizeCommand?.({ cfg: params.cfg, accountId: params.accountId, senderId: params.senderId, diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index 776c81799c8..366077bd631 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -52,10 +52,16 @@ const telegramApprovalPlugin: Pick< > = { ...createChannelTestPluginBase({ id: "telegram" }), execApprovals: { - shouldSuppressForwardingFallback: (params) => - shouldSuppressTelegramExecApprovalForwardingFallback(params), - buildPendingPayload: ({ request, nowMs }) => - buildTelegramExecApprovalPendingPayload({ request, nowMs }), + delivery: { + shouldSuppressForwardingFallback: (params) => + shouldSuppressTelegramExecApprovalForwardingFallback(params), + }, + render: { + exec: { + buildPendingPayload: ({ request, nowMs }) => + buildTelegramExecApprovalPendingPayload({ request, nowMs }), + }, + }, }, }; const discordApprovalPlugin: Pick< @@ -64,9 +70,11 @@ const discordApprovalPlugin: Pick< > = { ...createChannelTestPluginBase({ id: "discord" }), execApprovals: { - shouldSuppressForwardingFallback: ({ cfg, target }) => - target.channel === "discord" && - isDiscordExecApprovalClientEnabledForTest({ cfg, accountId: target.accountId }), + delivery: { + shouldSuppressForwardingFallback: ({ cfg, target }) => + target.channel === "discord" && + isDiscordExecApprovalClientEnabledForTest({ cfg, accountId: target.accountId }), + }, }, }; const defaultRegistry = createTestRegistry([ diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index 36b357bfdd8..63d7e2c5ba7 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -127,7 +127,7 @@ function shouldSkipForwardingFallback(params: { } const adapter = getChannelPlugin(channel)?.execApprovals; return ( - adapter?.shouldSuppressForwardingFallback?.({ + adapter?.delivery?.shouldSuppressForwardingFallback?.({ cfg: params.cfg, target: params.target, request: params.request, @@ -274,7 +274,7 @@ function buildRequestPayloadForTarget( ): ReplyPayload { const channel = normalizeMessageChannel(target.channel) ?? target.channel; const pluginPayload = channel - ? getChannelPlugin(channel)?.execApprovals?.buildPendingPayload?.({ + ? getChannelPlugin(channel)?.execApprovals?.render?.exec?.buildPendingPayload?.({ cfg, request, target, @@ -294,7 +294,7 @@ function buildResolvedPayloadForTarget( ): ReplyPayload { const channel = normalizeMessageChannel(target.channel) ?? target.channel; const pluginPayload = channel - ? getChannelPlugin(channel)?.execApprovals?.buildResolvedPayload?.({ + ? getChannelPlugin(channel)?.execApprovals?.render?.exec?.buildResolvedPayload?.({ cfg, resolved, target, @@ -409,7 +409,7 @@ export function createExecApprovalForwarder( if (!channel) { return; } - await getChannelPlugin(channel)?.execApprovals?.beforeDeliverPending?.({ + await getChannelPlugin(channel)?.execApprovals?.delivery?.beforeDeliverPending?.({ cfg, target, payload, @@ -556,7 +556,7 @@ export function createExecApprovalForwarder( buildPayload: (target) => { const channel = normalizeMessageChannel(target.channel) ?? target.channel; const adapterPayload = channel - ? getChannelPlugin(channel)?.execApprovals?.buildPluginPendingPayload?.({ + ? getChannelPlugin(channel)?.execApprovals?.render?.plugin?.buildPendingPayload?.({ cfg, request, target, @@ -570,7 +570,7 @@ export function createExecApprovalForwarder( if (!channel) { return; } - await getChannelPlugin(channel)?.execApprovals?.beforeDeliverPending?.({ + await getChannelPlugin(channel)?.execApprovals?.delivery?.beforeDeliverPending?.({ cfg, target, payload, @@ -624,7 +624,7 @@ export function createExecApprovalForwarder( buildPayload: (target) => { const channel = normalizeMessageChannel(target.channel) ?? target.channel; const adapterPayload = channel - ? getChannelPlugin(channel)?.execApprovals?.buildPluginResolvedPayload?.({ + ? getChannelPlugin(channel)?.execApprovals?.render?.plugin?.buildResolvedPayload?.({ cfg, resolved, target, diff --git a/src/infra/exec-approval-surface.test.ts b/src/infra/exec-approval-surface.test.ts index 643fbc37ef7..39b5f144c29 100644 --- a/src/infra/exec-approval-surface.test.ts +++ b/src/infra/exec-approval-surface.test.ts @@ -83,13 +83,17 @@ describe("resolveExecApprovalInitiatingSurfaceState", () => { channel === "telegram" ? { execApprovals: { - getInitiatingSurfaceState: () => ({ kind: "enabled" }), + auth: { + getInitiatingSurfaceState: () => ({ kind: "enabled" }), + }, }, } : channel === "discord" ? { execApprovals: { - getInitiatingSurfaceState: () => ({ kind: "disabled" }), + auth: { + getInitiatingSurfaceState: () => ({ kind: "disabled" }), + }, }, } : undefined, @@ -128,7 +132,9 @@ describe("resolveExecApprovalInitiatingSurfaceState", () => { channel === "telegram" ? { execApprovals: { - getInitiatingSurfaceState: () => ({ kind: "disabled" }), + auth: { + getInitiatingSurfaceState: () => ({ kind: "disabled" }), + }, }, } : undefined, @@ -172,12 +178,16 @@ describe("hasConfiguredExecApprovalDmRoute", () => { plugins: [ { execApprovals: { - hasConfiguredDmRoute: () => false, + delivery: { + hasConfiguredDmRoute: () => false, + }, }, }, { execApprovals: { - hasConfiguredDmRoute: () => true, + delivery: { + hasConfiguredDmRoute: () => true, + }, }, }, ], @@ -187,12 +197,16 @@ describe("hasConfiguredExecApprovalDmRoute", () => { plugins: [ { execApprovals: { - hasConfiguredDmRoute: () => false, + delivery: { + hasConfiguredDmRoute: () => false, + }, }, }, { execApprovals: { - hasConfiguredDmRoute: () => false, + delivery: { + hasConfiguredDmRoute: () => false, + }, }, }, { diff --git a/src/infra/exec-approval-surface.ts b/src/infra/exec-approval-surface.ts index 688df4d3ac3..f8405cd1ee9 100644 --- a/src/infra/exec-approval-surface.ts +++ b/src/infra/exec-approval-surface.ts @@ -38,7 +38,7 @@ export function resolveExecApprovalInitiatingSurfaceState(params: { } const cfg = params.cfg ?? loadConfig(); - const state = getChannelPlugin(channel)?.execApprovals?.getInitiatingSurfaceState?.({ + const state = getChannelPlugin(channel)?.execApprovals?.auth?.getInitiatingSurfaceState?.({ cfg, accountId: params.accountId, }); @@ -53,6 +53,6 @@ export function resolveExecApprovalInitiatingSurfaceState(params: { export function hasConfiguredExecApprovalDmRoute(cfg: OpenClawConfig): boolean { return listChannelPlugins().some( - (plugin) => plugin.execApprovals?.hasConfiguredDmRoute?.({ cfg }) ?? false, + (plugin) => plugin.execApprovals?.delivery?.hasConfiguredDmRoute?.({ cfg }) ?? false, ); } diff --git a/src/infra/plugin-approval-forwarder.test.ts b/src/infra/plugin-approval-forwarder.test.ts index c122360dd7f..2352b657e1a 100644 --- a/src/infra/plugin-approval-forwarder.test.ts +++ b/src/infra/plugin-approval-forwarder.test.ts @@ -156,7 +156,11 @@ describe("plugin approval forwarding", () => { > = { ...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }), execApprovals: { - buildPluginPendingPayload: vi.fn().mockReturnValue(mockPayload), + render: { + plugin: { + buildPendingPayload: vi.fn().mockReturnValue(mockPayload), + }, + }, }, }; const registry = createTestRegistry([ @@ -197,7 +201,9 @@ describe("plugin approval forwarding", () => { > = { ...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }), execApprovals: { - beforeDeliverPending, + delivery: { + beforeDeliverPending, + }, }, }; const registry = createTestRegistry([ @@ -222,7 +228,11 @@ describe("plugin approval forwarding", () => { > = { ...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }), execApprovals: { - buildPluginResolvedPayload: vi.fn().mockReturnValue(mockPayload), + render: { + plugin: { + buildResolvedPayload: vi.fn().mockReturnValue(mockPayload), + }, + }, }, }; const registry = createTestRegistry([ diff --git a/src/plugin-sdk/approval-delivery-helpers.test.ts b/src/plugin-sdk/approval-delivery-helpers.test.ts index d9ddd1f0cd5..374c23293f0 100644 --- a/src/plugin-sdk/approval-delivery-helpers.test.ts +++ b/src/plugin-sdk/approval-delivery-helpers.test.ts @@ -13,9 +13,10 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { isNativeDeliveryEnabled: () => true, resolveNativeDeliveryMode: () => "dm", }); + const authorizeCommand = adapter.auth.authorizeCommand; expect( - adapter.authorizeCommand({ + authorizeCommand({ cfg: {} as never, accountId: "work", senderId: "exec-owner", @@ -24,7 +25,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { ).toEqual({ authorized: true }); expect( - adapter.authorizeCommand({ + authorizeCommand({ cfg: {} as never, accountId: "work", senderId: "plugin-owner", @@ -33,7 +34,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { ).toEqual({ authorized: true }); expect( - adapter.authorizeCommand({ + authorizeCommand({ cfg: {} as never, accountId: "work", senderId: "someone-else", @@ -56,14 +57,16 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { resolveNativeDeliveryMode: ({ accountId }) => accountId === "channel-only" ? "channel" : "dm", }); + const getInitiatingSurfaceState = adapter.auth.getInitiatingSurfaceState; + const hasConfiguredDmRoute = adapter.delivery.hasConfiguredDmRoute; - expect(adapter.getInitiatingSurfaceState({ cfg: {} as never, accountId: "dm-only" })).toEqual({ + expect(getInitiatingSurfaceState({ cfg: {} as never, accountId: "dm-only" })).toEqual({ kind: "enabled", }); - expect( - adapter.getInitiatingSurfaceState({ cfg: {} as never, accountId: "no-approvers" }), - ).toEqual({ kind: "disabled" }); - expect(adapter.hasConfiguredDmRoute({ cfg: {} as never })).toBe(true); + expect(getInitiatingSurfaceState({ cfg: {} as never, accountId: "no-approvers" })).toEqual({ + kind: "disabled", + }); + expect(hasConfiguredDmRoute({ cfg: {} as never })).toBe(true); }); it("suppresses forwarding fallback only for matching native-delivery surfaces", () => { @@ -82,9 +85,10 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { resolveSuppressionAccountId: ({ request }) => request.request.turnSourceAccountId?.trim() || undefined, }); + const shouldSuppressForwardingFallback = adapter.delivery.shouldSuppressForwardingFallback; expect( - adapter.shouldSuppressForwardingFallback({ + shouldSuppressForwardingFallback({ cfg: {} as never, target: { channel: "telegram" }, request: { @@ -94,7 +98,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { ).toBe(true); expect( - adapter.shouldSuppressForwardingFallback({ + shouldSuppressForwardingFallback({ cfg: {} as never, target: { channel: "telegram" }, request: { @@ -104,7 +108,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => { ).toBe(false); expect( - adapter.shouldSuppressForwardingFallback({ + shouldSuppressForwardingFallback({ cfg: {} as never, target: { channel: "slack" }, request: { diff --git a/src/plugin-sdk/approval-delivery-helpers.ts b/src/plugin-sdk/approval-delivery-helpers.ts index 4ef84f4405f..5801d8e2667 100644 --- a/src/plugin-sdk/approval-delivery-helpers.ts +++ b/src/plugin-sdk/approval-delivery-helpers.ts @@ -34,66 +34,72 @@ export function createApproverRestrictedNativeApprovalAdapter(params: { const pluginSenderAuth = params.isPluginAuthorizedSender ?? params.isExecAuthorizedSender; return { - authorizeCommand: ({ - cfg, - accountId, - senderId, - kind, - }: { - cfg: OpenClawConfig; - accountId?: string | null; - senderId?: string | null; - kind: ApprovalKind; - }) => { - const authorized = - kind === "plugin" - ? pluginSenderAuth({ cfg, accountId, senderId }) - : params.isExecAuthorizedSender({ cfg, accountId, senderId }); - return authorized - ? { authorized: true } - : { - authorized: false, - reason: `❌ You are not authorized to approve ${kind} requests on ${params.channelLabel}.`, - }; + auth: { + authorizeCommand: ({ + cfg, + accountId, + senderId, + kind, + }: { + cfg: OpenClawConfig; + accountId?: string | null; + senderId?: string | null; + kind: ApprovalKind; + }) => { + const authorized = + kind === "plugin" + ? pluginSenderAuth({ cfg, accountId, senderId }) + : params.isExecAuthorizedSender({ cfg, accountId, senderId }); + return authorized + ? { authorized: true } + : { + authorized: false, + reason: `❌ You are not authorized to approve ${kind} requests on ${params.channelLabel}.`, + }; + }, + getInitiatingSurfaceState: ({ + cfg, + accountId, + }: { + cfg: OpenClawConfig; + accountId?: string | null; + }) => + params.hasApprovers({ cfg, accountId }) + ? ({ kind: "enabled" } as const) + : ({ kind: "disabled" } as const), }, - getInitiatingSurfaceState: ({ - cfg, - accountId, - }: { - cfg: OpenClawConfig; - accountId?: string | null; - }) => - params.hasApprovers({ cfg, accountId }) - ? ({ kind: "enabled" } as const) - : ({ kind: "disabled" } as const), - hasConfiguredDmRoute: ({ cfg }: { cfg: OpenClawConfig }) => - params.listAccountIds(cfg).some((accountId) => { - if (!params.hasApprovers({ cfg, accountId })) { + delivery: { + hasConfiguredDmRoute: ({ cfg }: { cfg: OpenClawConfig }) => + params.listAccountIds(cfg).some((accountId) => { + if (!params.hasApprovers({ cfg, accountId })) { + return false; + } + if (!params.isNativeDeliveryEnabled({ cfg, accountId })) { + return false; + } + const target = params.resolveNativeDeliveryMode({ cfg, accountId }); + return target === "dm" || target === "both"; + }), + shouldSuppressForwardingFallback: (input: DeliverySuppressionParams) => { + const channel = normalizeMessageChannel(input.target.channel) ?? input.target.channel; + if (channel !== params.channel) { return false; } - if (!params.isNativeDeliveryEnabled({ cfg, accountId })) { - return false; + if (params.requireMatchingTurnSourceChannel) { + const turnSourceChannel = normalizeMessageChannel( + input.request.request.turnSourceChannel, + ); + if (turnSourceChannel !== params.channel) { + return false; + } } - const target = params.resolveNativeDeliveryMode({ cfg, accountId }); - return target === "dm" || target === "both"; - }), - shouldSuppressForwardingFallback: (input: DeliverySuppressionParams) => { - const channel = normalizeMessageChannel(input.target.channel) ?? input.target.channel; - if (channel !== params.channel) { - return false; - } - if (params.requireMatchingTurnSourceChannel) { - const turnSourceChannel = normalizeMessageChannel(input.request.request.turnSourceChannel); - if (turnSourceChannel !== params.channel) { - return false; - } - } - const resolvedAccountId = params.resolveSuppressionAccountId?.(input); - const accountId = - (resolvedAccountId === undefined - ? input.target.accountId?.trim() - : resolvedAccountId.trim()) || undefined; - return params.isNativeDeliveryEnabled({ cfg: input.cfg, accountId }); + const resolvedAccountId = params.resolveSuppressionAccountId?.(input); + const accountId = + (resolvedAccountId === undefined + ? input.target.accountId?.trim() + : resolvedAccountId.trim()) || undefined; + return params.isNativeDeliveryEnabled({ cfg: input.cfg, accountId }); + }, }, }; } diff --git a/src/plugin-sdk/approval-renderers.test.ts b/src/plugin-sdk/approval-renderers.test.ts new file mode 100644 index 00000000000..c9de452d426 --- /dev/null +++ b/src/plugin-sdk/approval-renderers.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +import { + buildPluginApprovalPendingReplyPayload, + buildPluginApprovalResolvedReplyPayload, +} from "./approval-renderers.js"; + +describe("plugin-sdk/approval-renderers", () => { + it("builds plugin pending payloads with approval metadata and extra channel data", () => { + const payload = buildPluginApprovalPendingReplyPayload({ + request: { + id: "plugin-approval-123", + request: { + title: "Sensitive action", + description: "Needs approval", + }, + createdAtMs: 1_000, + expiresAtMs: 61_000, + }, + nowMs: 1_000, + approvalSlug: "custom-slug", + channelData: { + telegram: { + buttons: [[{ text: "Allow Once", callback_data: "/approve id allow-once" }]], + }, + }, + }); + + expect(payload.text).toContain("Plugin approval required"); + expect(payload.channelData).toMatchObject({ + execApproval: { + approvalId: "plugin-approval-123", + approvalSlug: "custom-slug", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + telegram: { + buttons: [[{ text: "Allow Once", callback_data: "/approve id allow-once" }]], + }, + }); + }); + + it("builds plugin resolved payloads with optional channel data", () => { + const payload = buildPluginApprovalResolvedReplyPayload({ + resolved: { + id: "plugin-approval-123", + decision: "allow-once", + resolvedBy: "discord:user:1", + ts: 2_000, + }, + channelData: { + discord: { + components: [{ type: "container" }], + }, + }, + }); + + expect(payload.text).toContain("Plugin approval allowed once"); + expect(payload.channelData).toEqual({ + discord: { + components: [{ type: "container" }], + }, + }); + }); +}); diff --git a/src/plugin-sdk/approval-renderers.ts b/src/plugin-sdk/approval-renderers.ts new file mode 100644 index 00000000000..a9a54c3b13e --- /dev/null +++ b/src/plugin-sdk/approval-renderers.ts @@ -0,0 +1,46 @@ +import type { ReplyPayload } from "../auto-reply/types.js"; +import type { ExecApprovalReplyDecision } from "../infra/exec-approval-reply.js"; +import { + buildPluginApprovalRequestMessage, + buildPluginApprovalResolvedMessage, + type PluginApprovalRequest, + type PluginApprovalResolved, +} from "../infra/plugin-approvals.js"; + +const DEFAULT_ALLOWED_DECISIONS = ["allow-once", "allow-always", "deny"] as const; + +export function buildPluginApprovalPendingReplyPayload(params: { + request: PluginApprovalRequest; + nowMs: number; + text?: string; + approvalSlug?: string; + allowedDecisions?: readonly ExecApprovalReplyDecision[]; + channelData?: Record; +}): ReplyPayload { + return { + text: params.text ?? buildPluginApprovalRequestMessage(params.request, params.nowMs), + channelData: { + execApproval: { + approvalId: params.request.id, + approvalSlug: params.approvalSlug ?? params.request.id.slice(0, 8), + allowedDecisions: params.allowedDecisions ?? DEFAULT_ALLOWED_DECISIONS, + }, + ...params.channelData, + }, + }; +} + +export function buildPluginApprovalResolvedReplyPayload(params: { + resolved: PluginApprovalResolved; + text?: string; + channelData?: Record; +}): ReplyPayload { + return params.channelData + ? { + text: params.text ?? buildPluginApprovalResolvedMessage(params.resolved), + channelData: params.channelData, + } + : { + text: params.text ?? buildPluginApprovalResolvedMessage(params.resolved), + }; +} diff --git a/src/plugin-sdk/approval-runtime.ts b/src/plugin-sdk/approval-runtime.ts index f6f082ed118..491f870197a 100644 --- a/src/plugin-sdk/approval-runtime.ts +++ b/src/plugin-sdk/approval-runtime.ts @@ -32,3 +32,7 @@ export { type PluginApprovalResolved, } from "../infra/plugin-approvals.js"; export { createApproverRestrictedNativeApprovalAdapter } from "./approval-delivery-helpers.js"; +export { + buildPluginApprovalPendingReplyPayload, + buildPluginApprovalResolvedReplyPayload, +} from "./approval-renderers.js";