diff --git a/extensions/discord/src/approval-native.test.ts b/extensions/discord/src/approval-native.test.ts index e6109b6de9f..4c4d0e1f0f1 100644 --- a/extensions/discord/src/approval-native.test.ts +++ b/extensions/discord/src/approval-native.test.ts @@ -1,6 +1,17 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { describe, expect, it } from "vitest"; +import { clearSessionStoreCacheForTest } from "../../../src/config/sessions.js"; import { createDiscordNativeApprovalAdapter } from "./approval-native.js"; +const STORE_PATH = path.join(os.tmpdir(), "openclaw-discord-approval-native-test.json"); + +function writeStore(store: Record) { + fs.writeFileSync(STORE_PATH, `${JSON.stringify(store, null, 2)}\n`, "utf8"); + clearSessionStoreCacheForTest(); +} + describe("createDiscordNativeApprovalAdapter", () => { it("normalizes prefixed turn-source channel ids", async () => { const adapter = createDiscordNativeApprovalAdapter(); @@ -51,6 +62,41 @@ describe("createDiscordNativeApprovalAdapter", () => { expect(target).toBeNull(); }); + it("ignores session-store turn targets for Discord DM sessions", async () => { + writeStore({ + "agent:main:discord:dm:123456789": { + sessionId: "sess", + updatedAt: Date.now(), + origin: { provider: "discord", to: "123456789", accountId: "main" }, + lastChannel: "discord", + lastTo: "123456789", + lastAccountId: "main", + }, + }); + + const adapter = createDiscordNativeApprovalAdapter(); + const target = await adapter.native?.resolveOriginTarget?.({ + cfg: { session: { store: STORE_PATH } } as never, + accountId: "main", + approvalKind: "plugin", + request: { + id: "abc", + request: { + title: "Plugin approval", + description: "Let plugin proceed", + sessionKey: "agent:main:discord:dm:123456789", + turnSourceChannel: "discord", + turnSourceTo: "123456789", + turnSourceAccountId: "main", + }, + createdAtMs: 1, + expiresAtMs: 2, + }, + }); + + expect(target).toBeNull(); + }); + it("accepts raw turn-source ids when a Discord channel session backs them", async () => { const adapter = createDiscordNativeApprovalAdapter(); diff --git a/extensions/discord/src/approval-native.ts b/extensions/discord/src/approval-native.ts index 71e39dcb274..e2472f8bc9a 100644 --- a/extensions/discord/src/approval-native.ts +++ b/extensions/discord/src/approval-native.ts @@ -1,11 +1,10 @@ -import { createApproverRestrictedNativeApprovalAdapter } from "openclaw/plugin-sdk/approval-runtime"; +import { createApproverRestrictedNativeApprovalAdapter, resolveExecApprovalSessionTarget } from "openclaw/plugin-sdk/approval-runtime"; import type { DiscordExecApprovalConfig, OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import type { ExecApprovalRequest, ExecApprovalSessionTarget, PluginApprovalRequest, } from "openclaw/plugin-sdk/infra-runtime"; -import { resolveExecApprovalSessionTarget } from "openclaw/plugin-sdk/approval-runtime"; import { normalizeAccountId } from "openclaw/plugin-sdk/routing"; import { listDiscordAccountIds, resolveDiscordAccount } from "./accounts.js"; import { @@ -137,11 +136,16 @@ function resolveDiscordOriginTarget(params: { if (turnSourceTarget) { return { to: turnSourceTarget.to }; } + if (sessionKind === "dm") { + return null; + } if (sessionTarget?.channel === "discord") { const targetTo = normalizeDiscordOriginChannelId(sessionTarget.to); return targetTo ? { to: targetTo } : null; } - const legacyChannelId = extractDiscordChannelId(params.request.request.sessionKey?.trim() || null); + const legacyChannelId = extractDiscordChannelId( + params.request.request.sessionKey?.trim() || null, + ); if (legacyChannelId) { return { to: legacyChannelId }; } diff --git a/extensions/discord/src/monitor/exec-approvals.test.ts b/extensions/discord/src/monitor/exec-approvals.test.ts index ed5c994ea5e..11d014acfed 100644 --- a/extensions/discord/src/monitor/exec-approvals.test.ts +++ b/extensions/discord/src/monitor/exec-approvals.test.ts @@ -46,7 +46,9 @@ const mockRestPatch = vi.hoisted(() => vi.fn()); const mockRestDelete = vi.hoisted(() => vi.fn()); const gatewayClientStarts = vi.hoisted(() => vi.fn()); const gatewayClientStops = vi.hoisted(() => vi.fn()); -const gatewayClientRequests = vi.hoisted(() => vi.fn(async (..._args: unknown[]) => ({ ok: true }))); +const gatewayClientRequests = vi.hoisted(() => + vi.fn(async (..._args: unknown[]) => ({ ok: true })), +); const gatewayClientParams = vi.hoisted(() => [] as Array>); const mockGatewayClientCtor = vi.hoisted(() => vi.fn()); const mockResolveGatewayConnectionAuth = vi.hoisted(() => vi.fn()); @@ -955,9 +957,7 @@ describe("DiscordExecApprovalHandler delivery routing", () => { Routes.channelMessages("999888777"), expect.objectContaining({ body: expect.objectContaining({ - content: expect.stringContaining( - "I sent approval DMs to the approvers for this account", - ), + content: expect.stringContaining("I sent approval DMs to the approvers for this account"), }), }), ); @@ -988,6 +988,45 @@ describe("DiscordExecApprovalHandler delivery routing", () => { ); }); + it("dedupes delivery when the origin route and approver DM resolve to the same Discord channel", async () => { + const handler = createHandler({ + enabled: true, + approvers: ["999"], + target: "both", + }); + + mockRestPost.mockImplementation(async (route: string) => { + if (route === Routes.channelMessages("123")) { + return { id: "msg-1", channel_id: "123" }; + } + if (route === Routes.userChannels()) { + return { id: "123" }; + } + throw new Error(`unexpected route: ${route}`); + }); + + await handler.handleApprovalRequested( + createRequest({ + sessionKey: "agent:main:discord:channel:123", + turnSourceChannel: "discord", + turnSourceTo: "123", + turnSourceAccountId: "default", + }), + ); + + expect(mockRestPost).toHaveBeenCalledTimes(2); + expect(mockRestPost).toHaveBeenNthCalledWith( + 1, + Routes.channelMessages("123"), + expect.objectContaining({ + body: expect.any(Object), + }), + ); + expect(mockRestPost).toHaveBeenNthCalledWith(2, Routes.userChannels(), { + body: { recipient_id: "999" }, + }); + }); + it("delivers plugin approvals through the shared runtime flow", async () => { const handler = createHandler({ enabled: true, diff --git a/extensions/discord/src/monitor/exec-approvals.ts b/extensions/discord/src/monitor/exec-approvals.ts index 03fefb27dba..ea30fe6fe9d 100644 --- a/extensions/discord/src/monitor/exec-approvals.ts +++ b/extensions/discord/src/monitor/exec-approvals.ts @@ -623,6 +623,9 @@ export class DiscordExecApprovalHandler { 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 { @@ -640,6 +643,12 @@ export class DiscordExecApprovalHandler { 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; + } try { const message = (await discordRequest( () => @@ -654,6 +663,7 @@ export class DiscordExecApprovalHandler { 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}`, @@ -679,6 +689,12 @@ export class DiscordExecApprovalHandler { 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; + } const message = (await discordRequest( () => @@ -697,6 +713,7 @@ export class DiscordExecApprovalHandler { discordMessageId: message.id, discordChannelId: dmChannel.id, }); + deliveredChannelIds.add(dmChannel.id); logDebug(`discord exec approvals: sent approval ${request.id} to user ${userId}`); } catch (err) {