mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-03 21:31:26 +00:00
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>
This commit is contained in:
@@ -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<string, unknown>) {
|
||||
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();
|
||||
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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<Record<string, unknown>>);
|
||||
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,
|
||||
|
||||
@@ -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<string>();
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user