diff --git a/extensions/discord/src/monitor/exec-approvals.ts b/extensions/discord/src/monitor/exec-approvals.ts index 49bd0fc54e0..81d9c12fc85 100644 --- a/extensions/discord/src/monitor/exec-approvals.ts +++ b/extensions/discord/src/monitor/exec-approvals.ts @@ -15,9 +15,9 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import type { DiscordExecApprovalConfig } from "openclaw/plugin-sdk/config-runtime"; import { createExecApprovalChannelRuntime, + deliverApprovalRequestViaChannelNativePlan, doesApprovalRequestMatchChannelAccount, type ExecApprovalChannelRuntime, - resolveChannelNativeApprovalDeliveryPlan, } from "openclaw/plugin-sdk/infra-runtime"; import { buildExecApprovalActionDescriptors } from "openclaw/plugin-sdk/infra-runtime"; import { resolveExecApprovalCommandDisplay } from "openclaw/plugin-sdk/infra-runtime"; @@ -60,6 +60,10 @@ type PendingApproval = { discordChannelId: string; timeoutId?: NodeJS.Timeout; }; +type PreparedDeliveryTarget = { + discordChannelId: string; + recipientUserId?: string; +}; function resolveApprovalKindFromId(approvalId: string): ApprovalKind { return approvalId.startsWith("plugin:") ? "plugin" : "exec"; @@ -522,20 +526,17 @@ export class DiscordExecApprovalHandler { const body = stripUndefinedFields(serializePayload(payload)); const approvalKind: ApprovalKind = isPluginApprovalRequest(request) ? "plugin" : "exec"; const nativeApprovalAdapter = createDiscordNativeApprovalAdapter(this.opts.config); - const deliveryPlan = await resolveChannelNativeApprovalDeliveryPlan({ + return await deliverApprovalRequestViaChannelNativePlan< + PreparedDeliveryTarget, + PendingApproval, + ApprovalRequest + >({ cfg: this.opts.cfg, accountId: this.opts.accountId, approvalKind, request, adapter: nativeApprovalAdapter.native, - }); - const pendingEntries: PendingApproval[] = []; - // "target=both" can collapse onto one Discord DM surface when the origin route - // and approver DM resolve to the same concrete channel id. - const deliveredChannelIds = new Set(); - const originTarget = deliveryPlan.originTarget; - if (deliveryPlan.notifyOriginWhenDmOnly && originTarget) { - try { + sendOriginNotice: async ({ originTarget }) => { await discordRequest( () => rest.post(Routes.channelMessages(originTarget.to), { @@ -543,47 +544,18 @@ export class DiscordExecApprovalHandler { }) as Promise<{ id: string; channel_id: string }>, "send-approval-dm-redirect-notice", ); - } catch (err) { - logError(`discord exec approvals: failed to send DM redirect notice: ${String(err)}`); - } - } - - for (const deliveryTarget of deliveryPlan.targets) { - if (deliveryTarget.surface === "origin") { - if (deliveredChannelIds.has(deliveryTarget.target.to)) { - logDebug( - `discord exec approvals: skipping duplicate approval ${request.id} for channel ${deliveryTarget.target.to}`, - ); - continue; + }, + prepareTarget: async ({ plannedTarget }) => { + if (plannedTarget.surface === "origin") { + return { + dedupeKey: plannedTarget.target.to, + target: { + discordChannelId: plannedTarget.target.to, + }, + }; } - try { - const message = (await discordRequest( - () => - rest.post(Routes.channelMessages(deliveryTarget.target.to), { - body, - }) as Promise<{ id: string; channel_id: string }>, - "send-approval-channel", - )) as { id: string; channel_id: string }; - if (message?.id) { - pendingEntries.push({ - discordMessageId: message.id, - discordChannelId: deliveryTarget.target.to, - }); - deliveredChannelIds.add(deliveryTarget.target.to); - - logDebug( - `discord exec approvals: sent approval ${request.id} to channel ${deliveryTarget.target.to}`, - ); - } - } catch (err) { - logError(`discord exec approvals: failed to send to channel: ${String(err)}`); - } - continue; - } - - const userId = deliveryTarget.target.to; - try { + const userId = plannedTarget.target.to; const dmChannel = (await discordRequest( () => rest.post(Routes.userChannels(), { @@ -594,40 +566,71 @@ export class DiscordExecApprovalHandler { if (!dmChannel?.id) { logError(`discord exec approvals: failed to create DM for user ${userId}`); - continue; - } - if (deliveredChannelIds.has(dmChannel.id)) { - logDebug( - `discord exec approvals: skipping duplicate approval ${request.id} for DM channel ${dmChannel.id}`, - ); - continue; + return null; } + return { + dedupeKey: dmChannel.id, + target: { + discordChannelId: dmChannel.id, + recipientUserId: userId, + }, + }; + }, + deliverTarget: async ({ plannedTarget, preparedTarget }) => { const message = (await discordRequest( () => - rest.post(Routes.channelMessages(dmChannel.id), { + rest.post(Routes.channelMessages(preparedTarget.discordChannelId), { body, }) as Promise<{ id: string; channel_id: string }>, - "send-approval", + plannedTarget.surface === "origin" ? "send-approval-channel" : "send-approval", )) as { id: string; channel_id: string }; if (!message?.id) { - logError(`discord exec approvals: failed to send message to user ${userId}`); - continue; + if (plannedTarget.surface === "origin") { + logError("discord exec approvals: failed to send to channel"); + } else if (preparedTarget.recipientUserId) { + logError( + `discord exec approvals: failed to send message to user ${preparedTarget.recipientUserId}`, + ); + } + return null; } - pendingEntries.push({ + return { discordMessageId: message.id, - discordChannelId: dmChannel.id, - }); - deliveredChannelIds.add(dmChannel.id); - - logDebug(`discord exec approvals: sent approval ${request.id} to user ${userId}`); - } catch (err) { - logError(`discord exec approvals: failed to notify user ${userId}: ${String(err)}`); - } - } - return pendingEntries; + discordChannelId: preparedTarget.discordChannelId, + }; + }, + onOriginNoticeError: ({ error }) => { + logError(`discord exec approvals: failed to send DM redirect notice: ${String(error)}`); + }, + onDuplicateSkipped: ({ preparedTarget }) => { + logDebug( + `discord exec approvals: skipping duplicate approval ${request.id} for channel ${preparedTarget.dedupeKey}`, + ); + }, + onDelivered: ({ plannedTarget, preparedTarget }) => { + if (plannedTarget.surface === "origin") { + logDebug( + `discord exec approvals: sent approval ${request.id} to channel ${preparedTarget.target.discordChannelId}`, + ); + return; + } + logDebug( + `discord exec approvals: sent approval ${request.id} to user ${plannedTarget.target.to}`, + ); + }, + onDeliveryError: ({ error, plannedTarget }) => { + if (plannedTarget.surface === "origin") { + logError(`discord exec approvals: failed to send to channel: ${String(error)}`); + return; + } + logError( + `discord exec approvals: failed to notify user ${plannedTarget.target.to}: ${String(error)}`, + ); + }, + }); } async handleApprovalRequested(request: ApprovalRequest): Promise { diff --git a/extensions/slack/src/monitor/exec-approvals.ts b/extensions/slack/src/monitor/exec-approvals.ts index 5ec761c040d..a41afc802bf 100644 --- a/extensions/slack/src/monitor/exec-approvals.ts +++ b/extensions/slack/src/monitor/exec-approvals.ts @@ -4,8 +4,8 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import { buildApprovalInteractiveReply, createExecApprovalChannelRuntime, + deliverApprovalRequestViaChannelNativePlan, getExecApprovalApproverDmNoticeText, - resolveChannelNativeApprovalDeliveryPlan, resolveExecApprovalCommandDisplay, type ExecApprovalChannelRuntime, type ExecApprovalDecision, @@ -209,10 +209,6 @@ function buildSlackExpiredBlocks(request: ExecApprovalRequest): SlackBlock[] { ]; } -function buildDeliveryTargetKey(target: { to: string; threadId?: string | number | null }): string { - return `${target.to}:${target.threadId == null ? "" : String(target.threadId)}`; -} - export class SlackExecApprovalHandler { private readonly runtime: ExecApprovalChannelRuntime; private readonly opts: SlackExecApprovalHandlerOpts; @@ -281,70 +277,54 @@ export class SlackExecApprovalHandler { } private async deliverRequested(request: ExecApprovalRequest): Promise { - const deliveryPlan = await resolveChannelNativeApprovalDeliveryPlan({ + const text = buildSlackPendingApprovalText(request); + const blocks = buildSlackPendingApprovalBlocks(request); + return await deliverApprovalRequestViaChannelNativePlan({ cfg: this.opts.cfg, accountId: this.opts.accountId, approvalKind: "exec", request, adapter: slackNativeApprovalAdapter.native, - }); - const pendingEntries: SlackPendingApproval[] = []; - const originTargetKey = deliveryPlan.originTarget - ? buildDeliveryTargetKey(deliveryPlan.originTarget) - : null; - const targetKeys = new Set( - deliveryPlan.targets.map((target) => buildDeliveryTargetKey(target.target)), - ); - - if ( - deliveryPlan.notifyOriginWhenDmOnly && - deliveryPlan.originTarget && - (originTargetKey == null || !targetKeys.has(originTargetKey)) - ) { - try { - await sendMessageSlack( - deliveryPlan.originTarget.to, - getExecApprovalApproverDmNoticeText(), - { - cfg: this.opts.cfg, - accountId: this.opts.accountId, - threadTs: - deliveryPlan.originTarget.threadId != null - ? String(deliveryPlan.originTarget.threadId) - : undefined, - client: this.opts.app.client, - }, - ); - } catch (err) { - logError(`slack exec approvals: failed to send DM redirect notice: ${String(err)}`); - } - } - - for (const deliveryTarget of deliveryPlan.targets) { - try { - const message = await sendMessageSlack( - deliveryTarget.target.to, - buildSlackPendingApprovalText(request), - { - cfg: this.opts.cfg, - accountId: this.opts.accountId, - threadTs: - deliveryTarget.target.threadId != null - ? String(deliveryTarget.target.threadId) - : undefined, - blocks: buildSlackPendingApprovalBlocks(request), - client: this.opts.app.client, - }, - ); - pendingEntries.push({ + sendOriginNotice: async ({ originTarget }) => { + await sendMessageSlack(originTarget.to, getExecApprovalApproverDmNoticeText(), { + cfg: this.opts.cfg, + accountId: this.opts.accountId, + threadTs: originTarget.threadId != null ? String(originTarget.threadId) : undefined, + client: this.opts.app.client, + }); + }, + prepareTarget: ({ plannedTarget }) => ({ + dedupeKey: `${plannedTarget.target.to}:${plannedTarget.target.threadId == null ? "" : String(plannedTarget.target.threadId)}`, + target: { + to: plannedTarget.target.to, + threadTs: + plannedTarget.target.threadId != null + ? String(plannedTarget.target.threadId) + : undefined, + }, + }), + deliverTarget: async ({ preparedTarget }) => { + const message = await sendMessageSlack(preparedTarget.to, text, { + cfg: this.opts.cfg, + accountId: this.opts.accountId, + threadTs: preparedTarget.threadTs, + blocks, + client: this.opts.app.client, + }); + return { channelId: message.channelId, messageTs: message.messageId, - }); - } catch (err) { - logError(`slack exec approvals: failed to deliver approval ${request.id}: ${String(err)}`); - } - } - return pendingEntries; + }; + }, + onOriginNoticeError: ({ error }) => { + logError(`slack exec approvals: failed to send DM redirect notice: ${String(error)}`); + }, + onDeliveryError: ({ error }) => { + logError( + `slack exec approvals: failed to deliver approval ${request.id}: ${String(error)}`, + ); + }, + }); } private async finalizeResolved( diff --git a/extensions/telegram/src/exec-approvals-handler.ts b/extensions/telegram/src/exec-approvals-handler.ts index 702773d618a..2a2a150e4f1 100644 --- a/extensions/telegram/src/exec-approvals-handler.ts +++ b/extensions/telegram/src/exec-approvals-handler.ts @@ -5,9 +5,9 @@ import { import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import { createExecApprovalChannelRuntime, + deliverApprovalRequestViaChannelNativePlan, type ExecApprovalChannelRuntime, resolveApprovalRequestAccountId, - resolveChannelNativeApprovalDeliveryPlan, } from "openclaw/plugin-sdk/infra-runtime"; import { resolveExecApprovalCommandDisplay } from "openclaw/plugin-sdk/infra-runtime"; import { @@ -195,17 +195,6 @@ export class TelegramExecApprovalHandler { private async deliverRequested(request: ApprovalRequest): Promise { const approvalKind: ApprovalKind = request.id.startsWith("plugin:") ? "plugin" : "exec"; - const deliveryPlan = await resolveChannelNativeApprovalDeliveryPlan({ - cfg: this.opts.cfg, - accountId: this.opts.accountId, - approvalKind, - request, - adapter: telegramNativeApprovalAdapter.native, - }); - if (deliveryPlan.targets.length === 0) { - return []; - } - const payload = approvalKind === "plugin" ? buildPluginApprovalPendingReplyPayload({ @@ -227,37 +216,52 @@ export class TelegramExecApprovalHandler { const buttons = resolveTelegramInlineButtons({ interactive: payload.interactive, }); - const sentMessages: PendingMessage[] = []; - - for (const target of deliveryPlan.targets) { - try { - await this.sendTyping(target.target.to, { + return await deliverApprovalRequestViaChannelNativePlan({ + cfg: this.opts.cfg, + accountId: this.opts.accountId, + approvalKind, + request, + adapter: telegramNativeApprovalAdapter.native, + prepareTarget: ({ plannedTarget }) => ({ + dedupeKey: `${plannedTarget.target.to}:${plannedTarget.target.threadId == null ? "" : String(plannedTarget.target.threadId)}`, + target: { + chatId: plannedTarget.target.to, + messageThreadId: + typeof plannedTarget.target.threadId === "number" + ? plannedTarget.target.threadId + : undefined, + }, + }), + deliverTarget: async ({ preparedTarget }) => { + await this.sendTyping(preparedTarget.chatId, { cfg: this.opts.cfg, token: this.opts.token, accountId: this.opts.accountId, - ...(typeof target.target.threadId === "number" - ? { messageThreadId: target.target.threadId } + ...(preparedTarget.messageThreadId != null + ? { messageThreadId: preparedTarget.messageThreadId } : {}), }).catch(() => {}); - const result = await this.sendMessage(target.target.to, payload.text ?? "", { + const result = await this.sendMessage(preparedTarget.chatId, payload.text ?? "", { cfg: this.opts.cfg, token: this.opts.token, accountId: this.opts.accountId, buttons, - ...(typeof target.target.threadId === "number" - ? { messageThreadId: target.target.threadId } + ...(preparedTarget.messageThreadId != null + ? { messageThreadId: preparedTarget.messageThreadId } : {}), }); - sentMessages.push({ + return { chatId: result.chatId, messageId: result.messageId, - }); - } catch (err) { - log.error(`telegram exec approvals: failed to send request ${request.id}: ${String(err)}`); - } - } - return sentMessages; + }; + }, + onDeliveryError: ({ error }) => { + log.error( + `telegram exec approvals: failed to send request ${request.id}: ${String(error)}`, + ); + }, + }); } async handleResolved(resolved: ApprovalResolved): Promise { diff --git a/src/infra/approval-native-runtime.test.ts b/src/infra/approval-native-runtime.test.ts new file mode 100644 index 00000000000..b9d0cb32e4a --- /dev/null +++ b/src/infra/approval-native-runtime.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it, vi } from "vitest"; +import type { ChannelApprovalNativeAdapter } from "../channels/plugins/types.adapters.js"; +import { deliverApprovalRequestViaChannelNativePlan } from "./approval-native-runtime.js"; + +const execRequest = { + id: "approval-1", + request: { + command: "uname -a", + }, + createdAtMs: 0, + expiresAtMs: 120_000, +}; + +describe("deliverApprovalRequestViaChannelNativePlan", () => { + it("sends an origin notice and dedupes converged prepared targets", async () => { + const adapter: ChannelApprovalNativeAdapter = { + describeDeliveryCapabilities: () => ({ + enabled: true, + preferredSurface: "approver-dm", + supportsOriginSurface: true, + supportsApproverDmSurface: true, + notifyOriginWhenDmOnly: true, + }), + resolveOriginTarget: async () => ({ to: "origin-room" }), + resolveApproverDmTargets: async () => [{ to: "approver-1" }, { to: "approver-2" }], + }; + const sendOriginNotice = vi.fn().mockResolvedValue(undefined); + const prepareTarget = vi + .fn() + .mockImplementation( + async ({ plannedTarget }: { plannedTarget: { target: { to: string } } }) => + plannedTarget.target.to === "approver-1" + ? { + dedupeKey: "shared-dm", + target: { channelId: "shared-dm", recipientId: "approver-1" }, + } + : { + dedupeKey: "shared-dm", + target: { channelId: "shared-dm", recipientId: "approver-2" }, + }, + ); + const deliverTarget = vi + .fn() + .mockImplementation( + async ({ preparedTarget }: { preparedTarget: { channelId: string } }) => ({ + channelId: preparedTarget.channelId, + }), + ); + const onDuplicateSkipped = vi.fn(); + + const entries = await deliverApprovalRequestViaChannelNativePlan({ + cfg: {} as never, + approvalKind: "exec", + request: execRequest, + adapter, + sendOriginNotice: async ({ originTarget }) => { + await sendOriginNotice(originTarget); + }, + prepareTarget, + deliverTarget, + onDuplicateSkipped, + }); + + expect(sendOriginNotice).toHaveBeenCalledWith({ to: "origin-room" }); + expect(prepareTarget).toHaveBeenCalledTimes(2); + expect(deliverTarget).toHaveBeenCalledTimes(1); + expect(onDuplicateSkipped).toHaveBeenCalledTimes(1); + expect(entries).toEqual([{ channelId: "shared-dm" }]); + }); + + it("continues after per-target delivery failures", async () => { + const adapter: ChannelApprovalNativeAdapter = { + describeDeliveryCapabilities: () => ({ + enabled: true, + preferredSurface: "approver-dm", + supportsOriginSurface: false, + supportsApproverDmSurface: true, + }), + resolveApproverDmTargets: async () => [{ to: "approver-1" }, { to: "approver-2" }], + }; + const onDeliveryError = vi.fn(); + + const entries = await deliverApprovalRequestViaChannelNativePlan({ + cfg: {} as never, + approvalKind: "exec", + request: execRequest, + adapter, + prepareTarget: ({ plannedTarget }) => ({ + dedupeKey: plannedTarget.target.to, + target: { channelId: plannedTarget.target.to }, + }), + deliverTarget: async ({ preparedTarget }) => { + if (preparedTarget.channelId === "approver-1") { + throw new Error("boom"); + } + return { channelId: preparedTarget.channelId }; + }, + onDeliveryError, + }); + + expect(onDeliveryError).toHaveBeenCalledTimes(1); + expect(entries).toEqual([{ channelId: "approver-2" }]); + }); +}); diff --git a/src/infra/approval-native-runtime.ts b/src/infra/approval-native-runtime.ts new file mode 100644 index 00000000000..901a25ffdc6 --- /dev/null +++ b/src/infra/approval-native-runtime.ts @@ -0,0 +1,154 @@ +import type { + ChannelApprovalKind, + ChannelApprovalNativeAdapter, + ChannelApprovalNativeTarget, +} from "../channels/plugins/types.adapters.js"; +import type { OpenClawConfig } from "../config/config.js"; +import { + resolveChannelNativeApprovalDeliveryPlan, + type ChannelApprovalNativePlannedTarget, +} from "./approval-native-delivery.js"; +import type { ExecApprovalRequest } from "./exec-approvals.js"; +import type { PluginApprovalRequest } from "./plugin-approvals.js"; + +type ApprovalRequest = ExecApprovalRequest | PluginApprovalRequest; + +export type PreparedChannelNativeApprovalTarget = { + dedupeKey: string; + target: TPreparedTarget; +}; + +function buildTargetKey(target: ChannelApprovalNativeTarget): string { + return `${target.to}:${target.threadId == null ? "" : String(target.threadId)}`; +} + +export async function deliverApprovalRequestViaChannelNativePlan< + TPreparedTarget, + TPendingEntry, + TRequest extends ApprovalRequest = ApprovalRequest, +>(params: { + cfg: OpenClawConfig; + accountId?: string | null; + approvalKind: ChannelApprovalKind; + request: TRequest; + adapter?: ChannelApprovalNativeAdapter | null; + sendOriginNotice?: (params: { + originTarget: ChannelApprovalNativeTarget; + request: TRequest; + }) => Promise; + prepareTarget: (params: { + plannedTarget: ChannelApprovalNativePlannedTarget; + request: TRequest; + }) => + | PreparedChannelNativeApprovalTarget + | null + | Promise | null>; + deliverTarget: (params: { + plannedTarget: ChannelApprovalNativePlannedTarget; + preparedTarget: TPreparedTarget; + request: TRequest; + }) => TPendingEntry | null | Promise; + onOriginNoticeError?: (params: { + error: unknown; + originTarget: ChannelApprovalNativeTarget; + request: TRequest; + }) => void; + onDeliveryError?: (params: { + error: unknown; + plannedTarget: ChannelApprovalNativePlannedTarget; + request: TRequest; + }) => void; + onDuplicateSkipped?: (params: { + plannedTarget: ChannelApprovalNativePlannedTarget; + preparedTarget: PreparedChannelNativeApprovalTarget; + request: TRequest; + }) => void; + onDelivered?: (params: { + plannedTarget: ChannelApprovalNativePlannedTarget; + preparedTarget: PreparedChannelNativeApprovalTarget; + request: TRequest; + entry: TPendingEntry; + }) => void; +}): Promise { + const deliveryPlan = await resolveChannelNativeApprovalDeliveryPlan({ + cfg: params.cfg, + accountId: params.accountId, + approvalKind: params.approvalKind, + request: params.request, + adapter: params.adapter, + }); + + const originTargetKey = deliveryPlan.originTarget + ? buildTargetKey(deliveryPlan.originTarget) + : null; + const plannedTargetKeys = new Set( + deliveryPlan.targets.map((plannedTarget) => buildTargetKey(plannedTarget.target)), + ); + + if ( + deliveryPlan.notifyOriginWhenDmOnly && + deliveryPlan.originTarget && + (originTargetKey == null || !plannedTargetKeys.has(originTargetKey)) + ) { + try { + await params.sendOriginNotice?.({ + originTarget: deliveryPlan.originTarget, + request: params.request, + }); + } catch (error) { + params.onOriginNoticeError?.({ + error, + originTarget: deliveryPlan.originTarget, + request: params.request, + }); + } + } + + const deliveredKeys = new Set(); + const pendingEntries: TPendingEntry[] = []; + for (const plannedTarget of deliveryPlan.targets) { + try { + const preparedTarget = await params.prepareTarget({ + plannedTarget, + request: params.request, + }); + if (!preparedTarget) { + continue; + } + if (deliveredKeys.has(preparedTarget.dedupeKey)) { + params.onDuplicateSkipped?.({ + plannedTarget, + preparedTarget, + request: params.request, + }); + continue; + } + + const entry = await params.deliverTarget({ + plannedTarget, + preparedTarget: preparedTarget.target, + request: params.request, + }); + if (!entry) { + continue; + } + + deliveredKeys.add(preparedTarget.dedupeKey); + pendingEntries.push(entry); + params.onDelivered?.({ + plannedTarget, + preparedTarget, + request: params.request, + entry, + }); + } catch (error) { + params.onDeliveryError?.({ + error, + plannedTarget, + request: params.request, + }); + } + } + + return pendingEntries; +} diff --git a/src/plugin-sdk/infra-runtime.ts b/src/plugin-sdk/infra-runtime.ts index 3e321100a33..4622eeda63f 100644 --- a/src/plugin-sdk/infra-runtime.ts +++ b/src/plugin-sdk/infra-runtime.ts @@ -13,6 +13,7 @@ export * from "../infra/exec-approval-reply.ts"; export * from "../infra/exec-approval-session-target.ts"; export * from "../infra/exec-approvals.ts"; export * from "../infra/approval-native-delivery.ts"; +export * from "../infra/approval-native-runtime.ts"; export * from "../infra/plugin-approvals.ts"; export * from "../infra/fetch.js"; export * from "../infra/file-lock.js";