Skip to content

Commit eba41da

Browse files
scoootscooobgreptile-apps[bot]
scoootscooob
and
greptile-apps[bot]
authored
fix(exec): dedupe Discord approval delivery (#58002)
* fix(exec): dedupe Discord approval delivery * Update extensions/discord/src/approval-native.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
1 parent 7b7d7cc commit eba41da

4 files changed

Lines changed: 113 additions & 7 deletions

File tree

‎extensions/discord/src/approval-native.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
1+
import fs from "node:fs";
2+
import os from "node:os";
3+
import path from "node:path";
14
import { describe, expect, it } from "vitest";
5+
import { clearSessionStoreCacheForTest } from "../../../src/config/sessions.js";
26
import { createDiscordNativeApprovalAdapter } from "./approval-native.js";
37

8+
const STORE_PATH = path.join(os.tmpdir(), "openclaw-discord-approval-native-test.json");
9+
10+
function writeStore(store: Record<string, unknown>) {
11+
fs.writeFileSync(STORE_PATH, `${JSON.stringify(store, null, 2)}\n`, "utf8");
12+
clearSessionStoreCacheForTest();
13+
}
14+
415
describe("createDiscordNativeApprovalAdapter", () => {
516
it("normalizes prefixed turn-source channel ids", async () => {
617
const adapter = createDiscordNativeApprovalAdapter();
@@ -51,6 +62,41 @@ describe("createDiscordNativeApprovalAdapter", () => {
5162
expect(target).toBeNull();
5263
});
5364

65+
it("ignores session-store turn targets for Discord DM sessions", async () => {
66+
writeStore({
67+
"agent:main:discord:dm:123456789": {
68+
sessionId: "sess",
69+
updatedAt: Date.now(),
70+
origin: { provider: "discord", to: "123456789", accountId: "main" },
71+
lastChannel: "discord",
72+
lastTo: "123456789",
73+
lastAccountId: "main",
74+
},
75+
});
76+
77+
const adapter = createDiscordNativeApprovalAdapter();
78+
const target = await adapter.native?.resolveOriginTarget?.({
79+
cfg: { session: { store: STORE_PATH } } as never,
80+
accountId: "main",
81+
approvalKind: "plugin",
82+
request: {
83+
id: "abc",
84+
request: {
85+
title: "Plugin approval",
86+
description: "Let plugin proceed",
87+
sessionKey: "agent:main:discord:dm:123456789",
88+
turnSourceChannel: "discord",
89+
turnSourceTo: "123456789",
90+
turnSourceAccountId: "main",
91+
},
92+
createdAtMs: 1,
93+
expiresAtMs: 2,
94+
},
95+
});
96+
97+
expect(target).toBeNull();
98+
});
99+
54100
it("accepts raw turn-source ids when a Discord channel session backs them", async () => {
55101
const adapter = createDiscordNativeApprovalAdapter();
56102

‎extensions/discord/src/approval-native.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
import { createApproverRestrictedNativeApprovalAdapter } from "openclaw/plugin-sdk/approval-runtime";
1+
import { createApproverRestrictedNativeApprovalAdapter, resolveExecApprovalSessionTarget } from "openclaw/plugin-sdk/approval-runtime";
22
import type { DiscordExecApprovalConfig, OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
33
import type {
44
ExecApprovalRequest,
55
ExecApprovalSessionTarget,
66
PluginApprovalRequest,
77
} from "openclaw/plugin-sdk/infra-runtime";
8-
import { resolveExecApprovalSessionTarget } from "openclaw/plugin-sdk/approval-runtime";
98
import { normalizeAccountId } from "openclaw/plugin-sdk/routing";
109
import { listDiscordAccountIds, resolveDiscordAccount } from "./accounts.js";
1110
import {
@@ -137,11 +136,16 @@ function resolveDiscordOriginTarget(params: {
137136
if (turnSourceTarget) {
138137
return { to: turnSourceTarget.to };
139138
}
139+
if (sessionKind === "dm") {
140+
return null;
141+
}
140142
if (sessionTarget?.channel === "discord") {
141143
const targetTo = normalizeDiscordOriginChannelId(sessionTarget.to);
142144
return targetTo ? { to: targetTo } : null;
143145
}
144-
const legacyChannelId = extractDiscordChannelId(params.request.request.sessionKey?.trim() || null);
146+
const legacyChannelId = extractDiscordChannelId(
147+
params.request.request.sessionKey?.trim() || null,
148+
);
145149
if (legacyChannelId) {
146150
return { to: legacyChannelId };
147151
}

‎extensions/discord/src/monitor/exec-approvals.test.ts‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ const mockRestPatch = vi.hoisted(() => vi.fn());
4646
const mockRestDelete = vi.hoisted(() => vi.fn());
4747
const gatewayClientStarts = vi.hoisted(() => vi.fn());
4848
const gatewayClientStops = vi.hoisted(() => vi.fn());
49-
const gatewayClientRequests = vi.hoisted(() => vi.fn(async (..._args: unknown[]) => ({ ok: true })));
49+
const gatewayClientRequests = vi.hoisted(() =>
50+
vi.fn(async (..._args: unknown[]) => ({ ok: true })),
51+
);
5052
const gatewayClientParams = vi.hoisted(() => [] as Array<Record<string, unknown>>);
5153
const mockGatewayClientCtor = vi.hoisted(() => vi.fn());
5254
const mockResolveGatewayConnectionAuth = vi.hoisted(() => vi.fn());
@@ -955,9 +957,7 @@ describe("DiscordExecApprovalHandler delivery routing", () => {
955957
Routes.channelMessages("999888777"),
956958
expect.objectContaining({
957959
body: expect.objectContaining({
958-
content: expect.stringContaining(
959-
"I sent approval DMs to the approvers for this account",
960-
),
960+
content: expect.stringContaining("I sent approval DMs to the approvers for this account"),
961961
}),
962962
}),
963963
);
@@ -988,6 +988,45 @@ describe("DiscordExecApprovalHandler delivery routing", () => {
988988
);
989989
});
990990

991+
it("dedupes delivery when the origin route and approver DM resolve to the same Discord channel", async () => {
992+
const handler = createHandler({
993+
enabled: true,
994+
approvers: ["999"],
995+
target: "both",
996+
});
997+
998+
mockRestPost.mockImplementation(async (route: string) => {
999+
if (route === Routes.channelMessages("123")) {
1000+
return { id: "msg-1", channel_id: "123" };
1001+
}
1002+
if (route === Routes.userChannels()) {
1003+
return { id: "123" };
1004+
}
1005+
throw new Error(`unexpected route: ${route}`);
1006+
});
1007+
1008+
await handler.handleApprovalRequested(
1009+
createRequest({
1010+
sessionKey: "agent:main:discord:channel:123",
1011+
turnSourceChannel: "discord",
1012+
turnSourceTo: "123",
1013+
turnSourceAccountId: "default",
1014+
}),
1015+
);
1016+
1017+
expect(mockRestPost).toHaveBeenCalledTimes(2);
1018+
expect(mockRestPost).toHaveBeenNthCalledWith(
1019+
1,
1020+
Routes.channelMessages("123"),
1021+
expect.objectContaining({
1022+
body: expect.any(Object),
1023+
}),
1024+
);
1025+
expect(mockRestPost).toHaveBeenNthCalledWith(2, Routes.userChannels(), {
1026+
body: { recipient_id: "999" },
1027+
});
1028+
});
1029+
9911030
it("delivers plugin approvals through the shared runtime flow", async () => {
9921031
const handler = createHandler({
9931032
enabled: true,

‎extensions/discord/src/monitor/exec-approvals.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,9 @@ export class DiscordExecApprovalHandler {
623623
adapter: nativeApprovalAdapter.native,
624624
});
625625
const pendingEntries: PendingApproval[] = [];
626+
// "target=both" can collapse onto one Discord DM surface when the origin route
627+
// and approver DM resolve to the same concrete channel id.
628+
const deliveredChannelIds = new Set<string>();
626629
const originTarget = deliveryPlan.originTarget;
627630
if (deliveryPlan.notifyOriginWhenDmOnly && originTarget) {
628631
try {
@@ -640,6 +643,12 @@ export class DiscordExecApprovalHandler {
640643

641644
for (const deliveryTarget of deliveryPlan.targets) {
642645
if (deliveryTarget.surface === "origin") {
646+
if (deliveredChannelIds.has(deliveryTarget.target.to)) {
647+
logDebug(
648+
`discord exec approvals: skipping duplicate approval ${request.id} for channel ${deliveryTarget.target.to}`,
649+
);
650+
continue;
651+
}
643652
try {
644653
const message = (await discordRequest(
645654
() =>
@@ -654,6 +663,7 @@ export class DiscordExecApprovalHandler {
654663
discordMessageId: message.id,
655664
discordChannelId: deliveryTarget.target.to,
656665
});
666+
deliveredChannelIds.add(deliveryTarget.target.to);
657667

658668
logDebug(
659669
`discord exec approvals: sent approval ${request.id} to channel ${deliveryTarget.target.to}`,
@@ -679,6 +689,12 @@ export class DiscordExecApprovalHandler {
679689
logError(`discord exec approvals: failed to create DM for user ${userId}`);
680690
continue;
681691
}
692+
if (deliveredChannelIds.has(dmChannel.id)) {
693+
logDebug(
694+
`discord exec approvals: skipping duplicate approval ${request.id} for DM channel ${dmChannel.id}`,
695+
);
696+
continue;
697+
}
682698

683699
const message = (await discordRequest(
684700
() =>
@@ -697,6 +713,7 @@ export class DiscordExecApprovalHandler {
697713
discordMessageId: message.id,
698714
discordChannelId: dmChannel.id,
699715
});
716+
deliveredChannelIds.add(dmChannel.id);
700717

701718
logDebug(`discord exec approvals: sent approval ${request.id} to user ${userId}`);
702719
} catch (err) {

0 commit comments

Comments
 (0)