From f85aba43a9cf05a6ebded2bfbad1eca012e977ad Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 1 Apr 2026 05:57:12 +0900 Subject: [PATCH] fix(approvals): restore native DM approval behavior --- extensions/slack/src/channel.ts | 7 + extensions/slack/src/exec-approvals.test.ts | 2 +- extensions/slack/src/exec-approvals.ts | 11 +- .../slack/src/monitor/exec-approvals.test.ts | 162 +++++++ .../slack/src/monitor/exec-approvals.ts | 408 ++++++++++++++++++ extensions/slack/src/monitor/provider.ts | 11 + .../telegram/src/approval-native.test.ts | 48 +++ extensions/telegram/src/approval-native.ts | 10 +- .../src/exec-approvals-handler.test.ts | 34 ++ 9 files changed, 683 insertions(+), 10 deletions(-) create mode 100644 extensions/slack/src/monitor/exec-approvals.test.ts create mode 100644 extensions/slack/src/monitor/exec-approvals.ts create mode 100644 extensions/telegram/src/approval-native.test.ts diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index ea48ed07763..8f337d4796e 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -45,6 +45,7 @@ import { listSlackDirectoryGroupsFromConfig, listSlackDirectoryPeersFromConfig, } from "./directory-config.js"; +import { shouldSuppressLocalSlackExecApprovalPrompt } from "./exec-approvals.js"; import { resolveSlackGroupRequireMention, resolveSlackGroupToolPolicy } from "./group-policy.js"; import { isSlackInteractiveRepliesEnabled } from "./interactive-replies.js"; import { SLACK_TEXT_LIMIT } from "./limits.js"; @@ -520,6 +521,12 @@ export const slackPlugin: ChannelPlugin = crea deliveryMode: "direct", chunker: null, textChunkLimit: SLACK_TEXT_LIMIT, + shouldSuppressLocalPayloadPrompt: ({ cfg, accountId, payload }) => + shouldSuppressLocalSlackExecApprovalPrompt({ + cfg, + accountId, + payload, + }), sendPayload: async (ctx) => { const { send, tokenOverride } = resolveSlackSendContext({ cfg: ctx.cfg, diff --git a/extensions/slack/src/exec-approvals.test.ts b/extensions/slack/src/exec-approvals.test.ts index 568f7d48437..634a2ee8887 100644 --- a/extensions/slack/src/exec-approvals.test.ts +++ b/extensions/slack/src/exec-approvals.test.ts @@ -126,7 +126,7 @@ describe("slack exec approvals", () => { cfg: buildConfig({ enabled: true, approvers: ["U123"] }), payload, }), - ).toBe(false); + ).toBe(true); expect( shouldSuppressLocalSlackExecApprovalPrompt({ diff --git a/extensions/slack/src/exec-approvals.ts b/extensions/slack/src/exec-approvals.ts index 0dbb1f9e508..8e53f55aa3a 100644 --- a/extensions/slack/src/exec-approvals.ts +++ b/extensions/slack/src/exec-approvals.ts @@ -1,5 +1,6 @@ import { doesApprovalRequestMatchChannelAccount, + getExecApprovalReplyMetadata, matchesApprovalRequestFilters, resolveApprovalApprovers, } from "openclaw/plugin-sdk/approval-runtime"; @@ -144,10 +145,8 @@ export function shouldSuppressLocalSlackExecApprovalPrompt(params: { accountId?: string | null; payload: ReplyPayload; }): boolean { - void params; - // Slack still uses the generic local pending-reply path. Unlike Discord and - // Telegram, there is no Slack runtime handler that sends a replacement native - // approval prompt via resolveChannelNativeApprovalDeliveryPlan, so suppressing - // the local payload can hide the only visible approval prompt. - return false; + return ( + isSlackExecApprovalClientEnabled(params) && + getExecApprovalReplyMetadata(params.payload) !== null + ); } diff --git a/extensions/slack/src/monitor/exec-approvals.test.ts b/extensions/slack/src/monitor/exec-approvals.test.ts new file mode 100644 index 00000000000..52a246da942 --- /dev/null +++ b/extensions/slack/src/monitor/exec-approvals.test.ts @@ -0,0 +1,162 @@ +import type { App } from "@slack/bolt"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +const sendMessageSlackMock = vi.hoisted(() => vi.fn()); + +vi.mock("../send.js", () => ({ + sendMessageSlack: sendMessageSlackMock, +})); + +let SlackExecApprovalHandler: typeof import("./exec-approvals.js").SlackExecApprovalHandler; + +function buildConfig(target: "dm" | "channel" | "both" = "dm"): OpenClawConfig { + return { + channels: { + slack: { + botToken: "xoxb-test", + appToken: "xapp-test", + execApprovals: { + enabled: true, + approvers: ["U123APPROVER"], + target, + }, + }, + }, + } as OpenClawConfig; +} + +function buildApp(): App { + return { + client: { + chat: { + update: vi.fn().mockResolvedValue(undefined), + }, + }, + } as unknown as App; +} + +function buildRequest(overrides?: Partial>) { + return { + id: "req-1", + request: { + command: "python3 -c \"print('slack exec approval smoke')\"", + turnSourceChannel: "slack", + turnSourceTo: "channel:C123ROOM", + turnSourceAccountId: "default", + turnSourceThreadId: "1712345678.123456", + sessionKey: "agent:main:slack:channel:c123room:thread:1712345678.123456", + ...overrides, + }, + createdAtMs: 0, + expiresAtMs: Date.now() + 60_000, + }; +} + +describe("SlackExecApprovalHandler", () => { + beforeAll(async () => { + ({ SlackExecApprovalHandler } = await import("./exec-approvals.js")); + }); + + beforeEach(() => { + sendMessageSlackMock.mockReset(); + sendMessageSlackMock.mockResolvedValue({ + messageId: "1712345678.999999", + channelId: "D123APPROVER", + }); + }); + + it("delivers DM-first approvals and only posts a short origin notice", async () => { + const app = buildApp(); + const handler = new SlackExecApprovalHandler({ + app, + accountId: "default", + config: buildConfig("dm").channels!.slack!.execApprovals!, + cfg: buildConfig("dm"), + }); + + await handler.handleApprovalRequested(buildRequest()); + + expect(sendMessageSlackMock).toHaveBeenCalledTimes(2); + expect(sendMessageSlackMock).toHaveBeenNthCalledWith( + 1, + "channel:C123ROOM", + "Approval required. I sent approval DMs to the approvers for this account.", + expect.objectContaining({ + accountId: "default", + threadTs: "1712345678.123456", + }), + ); + expect(sendMessageSlackMock).toHaveBeenNthCalledWith( + 2, + "user:U123APPROVER", + expect.stringContaining("Exec approval required"), + expect.objectContaining({ + accountId: "default", + blocks: expect.arrayContaining([expect.objectContaining({ type: "actions" })]), + }), + ); + }); + + it("does not post a redundant DM redirect notice when the origin is already the approver DM", async () => { + const app = buildApp(); + const handler = new SlackExecApprovalHandler({ + app, + accountId: "default", + config: buildConfig("dm").channels!.slack!.execApprovals!, + cfg: buildConfig("dm"), + }); + + await handler.handleApprovalRequested( + buildRequest({ + turnSourceTo: "user:U123APPROVER", + turnSourceThreadId: undefined, + sessionKey: "agent:main:slack:direct:U123APPROVER", + }), + ); + + expect(sendMessageSlackMock).toHaveBeenCalledTimes(1); + expect(sendMessageSlackMock).toHaveBeenCalledWith( + "user:U123APPROVER", + expect.stringContaining("Exec approval required"), + expect.objectContaining({ + blocks: expect.arrayContaining([expect.objectContaining({ type: "actions" })]), + }), + ); + }); + + it("updates the pending approval card in place after resolution", async () => { + const app = buildApp(); + const update = app.client.chat.update as ReturnType; + const handler = new SlackExecApprovalHandler({ + app, + accountId: "default", + config: buildConfig("dm").channels!.slack!.execApprovals!, + cfg: buildConfig("dm"), + }); + + await handler.handleApprovalRequested( + buildRequest({ + turnSourceTo: "user:U123APPROVER", + turnSourceThreadId: undefined, + sessionKey: "agent:main:slack:direct:U123APPROVER", + }), + ); + await handler.handleApprovalResolved({ + id: "req-1", + decision: "allow-once", + resolvedBy: "U123APPROVER", + request: buildRequest().request, + ts: Date.now(), + }); + + expect(update).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "D123APPROVER", + ts: "1712345678.999999", + text: expect.stringContaining("Exec approval: Allowed once"), + blocks: expect.not.arrayContaining([expect.objectContaining({ type: "actions" })]), + }), + ); + }); +}); diff --git a/extensions/slack/src/monitor/exec-approvals.ts b/extensions/slack/src/monitor/exec-approvals.ts new file mode 100644 index 00000000000..5ec761c040d --- /dev/null +++ b/extensions/slack/src/monitor/exec-approvals.ts @@ -0,0 +1,408 @@ +import type { App } from "@slack/bolt"; +import type { Block, KnownBlock } from "@slack/web-api"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; +import { + buildApprovalInteractiveReply, + createExecApprovalChannelRuntime, + getExecApprovalApproverDmNoticeText, + resolveChannelNativeApprovalDeliveryPlan, + resolveExecApprovalCommandDisplay, + type ExecApprovalChannelRuntime, + type ExecApprovalDecision, + type ExecApprovalRequest, + type ExecApprovalResolved, +} from "openclaw/plugin-sdk/infra-runtime"; +import { logError } from "openclaw/plugin-sdk/text-runtime"; +import { slackNativeApprovalAdapter } from "../approval-native.js"; +import { getSlackExecApprovalApprovers, normalizeSlackApproverId } from "../exec-approvals.js"; +import { resolveSlackReplyBlocks } from "../reply-blocks.js"; +import { sendMessageSlack } from "../send.js"; + +type SlackBlock = Block | KnownBlock; +type SlackPendingApproval = { + channelId: string; + messageTs: string; +}; + +type SlackExecApprovalConfig = NonNullable< + NonNullable["slack"]>["execApprovals"] +>; + +type SlackExecApprovalHandlerOpts = { + app: App; + accountId: string; + config: SlackExecApprovalConfig; + gatewayUrl?: string; + cfg: OpenClawConfig; +}; + +function truncateSlackMrkdwn(text: string, maxChars: number): string { + return text.length <= maxChars ? text : `${text.slice(0, maxChars - 1)}…`; +} + +function buildSlackCodeBlock(text: string): string { + let fence = "```"; + while (text.includes(fence)) { + fence += "`"; + } + return `${fence}\n${text}\n${fence}`; +} + +function formatSlackApprover(resolvedBy?: string | null): string | null { + const normalized = resolvedBy ? normalizeSlackApproverId(resolvedBy) : undefined; + if (normalized) { + return `<@${normalized}>`; + } + const trimmed = resolvedBy?.trim(); + return trimmed ? trimmed : null; +} + +function buildSlackApprovalContextLines(request: ExecApprovalRequest): string[] { + const lines: string[] = []; + if (request.request.agentId) { + lines.push(`*Agent:* ${request.request.agentId}`); + } + if (request.request.cwd) { + lines.push(`*CWD:* ${request.request.cwd}`); + } + if (request.request.host) { + lines.push(`*Host:* ${request.request.host}`); + } + return lines; +} + +function buildSlackPendingApprovalText(request: ExecApprovalRequest): string { + const { commandText } = resolveExecApprovalCommandDisplay(request.request); + const lines = [ + "*Exec approval required*", + "A command needs your approval.", + "", + "*Command*", + buildSlackCodeBlock(commandText), + ...buildSlackApprovalContextLines(request), + ]; + return lines.join("\n"); +} + +function buildSlackPendingApprovalBlocks(request: ExecApprovalRequest): SlackBlock[] { + const { commandText } = resolveExecApprovalCommandDisplay(request.request); + const metadataLines = buildSlackApprovalContextLines(request); + const interactiveBlocks = + resolveSlackReplyBlocks({ + text: "", + interactive: buildApprovalInteractiveReply({ + approvalId: request.id, + }), + }) ?? []; + return [ + { + type: "section", + text: { + type: "mrkdwn", + text: "*Exec approval required*\nA command needs your approval.", + }, + }, + { + type: "section", + text: { + type: "mrkdwn", + text: `*Command*\n${buildSlackCodeBlock(truncateSlackMrkdwn(commandText, 2600))}`, + }, + }, + ...(metadataLines.length > 0 + ? [ + { + type: "context", + elements: metadataLines.map((line) => ({ + type: "mrkdwn" as const, + text: line, + })), + } satisfies SlackBlock, + ] + : []), + ...interactiveBlocks, + ]; +} + +function buildSlackResolvedText(params: { + request: ExecApprovalRequest; + decision: ExecApprovalDecision; + resolvedBy?: string | null; +}): string { + const { commandText } = resolveExecApprovalCommandDisplay(params.request.request); + const decisionLabel = + params.decision === "allow-once" + ? "Allowed once" + : params.decision === "allow-always" + ? "Allowed always" + : "Denied"; + const resolvedBy = formatSlackApprover(params.resolvedBy); + const lines = [ + `*Exec approval: ${decisionLabel}*`, + resolvedBy ? `Resolved by ${resolvedBy}.` : "Resolved.", + "", + "*Command*", + buildSlackCodeBlock(commandText), + ]; + return lines.join("\n"); +} + +function buildSlackResolvedBlocks(params: { + request: ExecApprovalRequest; + decision: ExecApprovalDecision; + resolvedBy?: string | null; +}): SlackBlock[] { + const { commandText } = resolveExecApprovalCommandDisplay(params.request.request); + const decisionLabel = + params.decision === "allow-once" + ? "Allowed once" + : params.decision === "allow-always" + ? "Allowed always" + : "Denied"; + const resolvedBy = formatSlackApprover(params.resolvedBy); + return [ + { + type: "section", + text: { + type: "mrkdwn", + text: `*Exec approval: ${decisionLabel}*\n${resolvedBy ? `Resolved by ${resolvedBy}.` : "Resolved."}`, + }, + }, + { + type: "section", + text: { + type: "mrkdwn", + text: `*Command*\n${buildSlackCodeBlock(truncateSlackMrkdwn(commandText, 2600))}`, + }, + }, + ]; +} + +function buildSlackExpiredText(request: ExecApprovalRequest): string { + const { commandText } = resolveExecApprovalCommandDisplay(request.request); + return [ + "*Exec approval expired*", + "This approval request expired before it was resolved.", + "", + "*Command*", + buildSlackCodeBlock(commandText), + ].join("\n"); +} + +function buildSlackExpiredBlocks(request: ExecApprovalRequest): SlackBlock[] { + const { commandText } = resolveExecApprovalCommandDisplay(request.request); + return [ + { + type: "section", + text: { + type: "mrkdwn", + text: "*Exec approval expired*\nThis approval request expired before it was resolved.", + }, + }, + { + type: "section", + text: { + type: "mrkdwn", + text: `*Command*\n${buildSlackCodeBlock(truncateSlackMrkdwn(commandText, 2600))}`, + }, + }, + ]; +} + +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; + + constructor(opts: SlackExecApprovalHandlerOpts) { + this.opts = opts; + this.runtime = createExecApprovalChannelRuntime({ + label: "slack/exec-approvals", + clientDisplayName: "Slack Exec Approvals", + cfg: opts.cfg, + gatewayUrl: opts.gatewayUrl, + isConfigured: () => + Boolean( + opts.config.enabled && + getSlackExecApprovalApprovers({ + cfg: opts.cfg, + accountId: opts.accountId, + }).length > 0, + ), + shouldHandle: (request) => this.shouldHandle(request), + deliverRequested: async (request) => await this.deliverRequested(request), + finalizeResolved: async ({ request, resolved, entries }) => { + await this.finalizeResolved(request, resolved, entries); + }, + finalizeExpired: async ({ request, entries }) => { + await this.finalizeExpired(request, entries); + }, + }); + } + + shouldHandle(request: ExecApprovalRequest): boolean { + if (!this.opts.config.enabled) { + return false; + } + if ((this.opts.config.approvers?.length ?? 0) === 0) { + return false; + } + return ( + slackNativeApprovalAdapter.native?.describeDeliveryCapabilities({ + cfg: this.opts.cfg, + accountId: this.opts.accountId, + approvalKind: "exec", + request, + }).enabled === true + ); + } + + async start(): Promise { + await this.runtime.start(); + } + + async stop(): Promise { + await this.runtime.stop(); + } + + async handleApprovalRequested(request: ExecApprovalRequest): Promise { + await this.runtime.handleRequested(request); + } + + async handleApprovalResolved(resolved: ExecApprovalResolved): Promise { + await this.runtime.handleResolved(resolved); + } + + async handleApprovalTimeout(approvalId: string): Promise { + await this.runtime.handleExpired(approvalId); + } + + private async deliverRequested(request: ExecApprovalRequest): Promise { + const deliveryPlan = await resolveChannelNativeApprovalDeliveryPlan({ + 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({ + channelId: message.channelId, + messageTs: message.messageId, + }); + } catch (err) { + logError(`slack exec approvals: failed to deliver approval ${request.id}: ${String(err)}`); + } + } + return pendingEntries; + } + + private async finalizeResolved( + request: ExecApprovalRequest, + resolved: ExecApprovalResolved, + entries: SlackPendingApproval[], + ): Promise { + const text = buildSlackResolvedText({ + request, + decision: resolved.decision, + resolvedBy: resolved.resolvedBy, + }); + const blocks = buildSlackResolvedBlocks({ + request, + decision: resolved.decision, + resolvedBy: resolved.resolvedBy, + }); + for (const entry of entries) { + await this.updateMessage({ + channelId: entry.channelId, + messageTs: entry.messageTs, + text, + blocks, + }); + } + } + + private async finalizeExpired( + request: ExecApprovalRequest, + entries: SlackPendingApproval[], + ): Promise { + const blocks = buildSlackExpiredBlocks(request); + const text = buildSlackExpiredText(request); + for (const entry of entries) { + await this.updateMessage({ + channelId: entry.channelId, + messageTs: entry.messageTs, + text, + blocks, + }); + } + } + + private async updateMessage(params: { + channelId: string; + messageTs: string; + text: string; + blocks: SlackBlock[]; + }): Promise { + try { + await this.opts.app.client.chat.update({ + channel: params.channelId, + ts: params.messageTs, + text: params.text, + blocks: params.blocks, + }); + } catch (err) { + logError(`slack exec approvals: failed to update message: ${String(err)}`); + } + } +} diff --git a/extensions/slack/src/monitor/provider.ts b/extensions/slack/src/monitor/provider.ts index 83796646a4d..3c1ac90d55d 100644 --- a/extensions/slack/src/monitor/provider.ts +++ b/extensions/slack/src/monitor/provider.ts @@ -40,6 +40,7 @@ import { normalizeAllowList } from "./allow-list.js"; import { resolveSlackSlashCommandConfig } from "./commands.js"; import { createSlackMonitorContext } from "./context.js"; import { registerSlackMonitorEvents } from "./events.js"; +import { SlackExecApprovalHandler } from "./exec-approvals.js"; import { createSlackMessageHandler } from "./message-handler.js"; import { formatUnknownError, @@ -405,9 +406,18 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { : undefined; const handleSlackMessage = createSlackMessageHandler({ ctx, account, trackEvent }); + const execApprovalsHandler = slackCfg.execApprovals?.enabled + ? new SlackExecApprovalHandler({ + app, + accountId: account.accountId, + config: slackCfg.execApprovals, + cfg, + }) + : null; registerSlackMonitorEvents({ ctx, account, handleSlackMessage, trackEvent }); await registerSlackMonitorSlashCommands({ ctx, account }); + await execApprovalsHandler?.start(); if (slackMode === "http" && slackHttpHandler) { unregisterHttpHandler = registerSlackHttpHandler({ path: slackWebhookPath, @@ -613,6 +623,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { } finally { opts.abortSignal?.removeEventListener("abort", stopOnAbort); unregisterHttpHandler?.(); + await execApprovalsHandler?.stop().catch(() => undefined); await app.stop().catch(() => undefined); } } diff --git a/extensions/telegram/src/approval-native.test.ts b/extensions/telegram/src/approval-native.test.ts new file mode 100644 index 00000000000..3ef54783288 --- /dev/null +++ b/extensions/telegram/src/approval-native.test.ts @@ -0,0 +1,48 @@ +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; +import { describe, expect, it } from "vitest"; +import { telegramNativeApprovalAdapter } from "./approval-native.js"; + +function buildConfig( + overrides?: Partial["telegram"]>>, +): OpenClawConfig { + return { + channels: { + telegram: { + botToken: "tok", + execApprovals: { + enabled: true, + approvers: ["8460800771"], + target: "dm", + }, + ...overrides, + }, + }, + } as OpenClawConfig; +} + +describe("telegram native approval adapter", () => { + it("normalizes direct-chat origin targets so DM dedupe can converge", async () => { + const target = await telegramNativeApprovalAdapter.native?.resolveOriginTarget?.({ + cfg: buildConfig(), + accountId: "default", + approvalKind: "exec", + request: { + id: "req-1", + request: { + command: "echo hi", + turnSourceChannel: "telegram", + turnSourceTo: "telegram:8460800771", + turnSourceAccountId: "default", + sessionKey: "agent:main:telegram:direct:8460800771", + }, + createdAtMs: 0, + expiresAtMs: 1000, + }, + }); + + expect(target).toEqual({ + to: "8460800771", + threadId: undefined, + }); + }); +}); diff --git a/extensions/telegram/src/approval-native.ts b/extensions/telegram/src/approval-native.ts index f68a304b26f..fa966a7afdf 100644 --- a/extensions/telegram/src/approval-native.ts +++ b/extensions/telegram/src/approval-native.ts @@ -15,6 +15,7 @@ import { isTelegramExecApprovalClientEnabled, resolveTelegramExecApprovalTarget, } from "./exec-approvals.js"; +import { normalizeTelegramChatId } from "./targets.js"; type ApprovalRequest = ExecApprovalRequest | PluginApprovalRequest; type TelegramOriginTarget = { to: string; threadId?: number; accountId?: string }; @@ -62,8 +63,9 @@ function resolveTurnSourceTelegramOriginTarget(params: { request: ApprovalRequest; }): TelegramOriginTarget | null { const turnSourceChannel = params.request.request.turnSourceChannel?.trim().toLowerCase() || ""; - const turnSourceTo = params.request.request.turnSourceTo?.trim() || ""; + const rawTurnSourceTo = params.request.request.turnSourceTo?.trim() || ""; const turnSourceAccountId = params.request.request.turnSourceAccountId?.trim() || ""; + const turnSourceTo = normalizeTelegramChatId(rawTurnSourceTo) ?? rawTurnSourceTo; if (turnSourceChannel !== "telegram" || !turnSourceTo) { return null; } @@ -102,7 +104,7 @@ function resolveSessionTelegramOriginTarget(params: { return null; } return { - to: sessionTarget.to, + to: normalizeTelegramChatId(sessionTarget.to) ?? sessionTarget.to, threadId: sessionTarget.threadId, accountId: sessionTarget.accountId, }; @@ -113,7 +115,9 @@ function telegramTargetsMatch(a: TelegramOriginTarget, b: TelegramOriginTarget): !a.accountId || !b.accountId || normalizeAccountId(a.accountId) === normalizeAccountId(b.accountId); - return a.to === b.to && a.threadId === b.threadId && accountMatches; + const normalizedA = normalizeTelegramChatId(a.to) ?? a.to; + const normalizedB = normalizeTelegramChatId(b.to) ?? b.to; + return normalizedA === normalizedB && a.threadId === b.threadId && accountMatches; } function resolveTelegramOriginTarget(params: { diff --git a/extensions/telegram/src/exec-approvals-handler.test.ts b/extensions/telegram/src/exec-approvals-handler.test.ts index 2901170f077..b79c6e1239b 100644 --- a/extensions/telegram/src/exec-approvals-handler.test.ts +++ b/extensions/telegram/src/exec-approvals-handler.test.ts @@ -143,6 +143,40 @@ describe("TelegramExecApprovalHandler", () => { expect(sendMessage.mock.calls.map((call) => call[0])).toEqual(["111", "222"]); }); + it("does not double-send in direct chats when the origin chat is the approver DM", async () => { + const cfg = { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["8460800771"], + target: "dm", + }, + }, + }, + } as OpenClawConfig; + const { handler, sendMessage } = createHandler(cfg); + + await handler.handleRequested({ + ...baseRequest, + request: { + ...baseRequest.request, + sessionKey: "agent:main:telegram:direct:8460800771", + turnSourceTo: "telegram:8460800771", + turnSourceThreadId: undefined, + }, + }); + + expect(sendMessage).toHaveBeenCalledTimes(1); + expect(sendMessage).toHaveBeenCalledWith( + "8460800771", + expect.stringContaining("/approve 9f1c7d5d-b1fb-46ef-ac45-662723b65bb7 allow-once"), + expect.objectContaining({ + accountId: "default", + }), + ); + }); + it("clears buttons from tracked approval messages when resolved", async () => { const cfg = { channels: {