Discord: fix plugin interaction handling
This commit is contained in:
parent
7ca7fd0ef9
commit
9d55374088
@ -804,6 +804,9 @@ async function dispatchPluginDiscordInteractiveEvent(params: {
|
||||
fields?: Array<{ id: string; name: string; values: string[] }>;
|
||||
messageId?: string;
|
||||
}): Promise<"handled" | "unmatched"> {
|
||||
const normalizedConversationId = params.interactionCtx.rawGuildId
|
||||
? `channel:${params.interactionCtx.channelId}`
|
||||
: `user:${params.interactionCtx.userId}`;
|
||||
let responded = false;
|
||||
const respond = {
|
||||
acknowledge: async () => {
|
||||
@ -858,15 +861,35 @@ async function dispatchPluginDiscordInteractiveEvent(params: {
|
||||
decision: pluginBindingApproval.decision,
|
||||
senderId: params.interactionCtx.userId,
|
||||
});
|
||||
let cleared = false;
|
||||
try {
|
||||
await respond.clearComponents();
|
||||
cleared = true;
|
||||
} catch {
|
||||
await respond.acknowledge();
|
||||
try {
|
||||
await respond.acknowledge();
|
||||
} catch {
|
||||
// Interaction may already be acknowledged; continue with best-effort follow-up.
|
||||
}
|
||||
}
|
||||
try {
|
||||
await respond.followUp({
|
||||
text: buildPluginBindingResolvedText(resolved),
|
||||
ephemeral: true,
|
||||
});
|
||||
} catch (err) {
|
||||
logError(`discord plugin binding approval: failed to follow up: ${String(err)}`);
|
||||
if (!cleared) {
|
||||
try {
|
||||
await respond.reply({
|
||||
text: buildPluginBindingResolvedText(resolved),
|
||||
ephemeral: true,
|
||||
});
|
||||
} catch {
|
||||
// Interaction may no longer accept a direct reply.
|
||||
}
|
||||
}
|
||||
}
|
||||
await respond.followUp({
|
||||
text: buildPluginBindingResolvedText(resolved),
|
||||
ephemeral: true,
|
||||
});
|
||||
return "handled";
|
||||
}
|
||||
const dispatched = await dispatchPluginInteractiveHandler({
|
||||
@ -876,7 +899,7 @@ async function dispatchPluginDiscordInteractiveEvent(params: {
|
||||
ctx: {
|
||||
accountId: params.ctx.accountId,
|
||||
interactionId: resolveDiscordInteractionId(params.interaction),
|
||||
conversationId: params.interactionCtx.channelId,
|
||||
conversationId: normalizedConversationId,
|
||||
parentConversationId: params.channelCtx.parentId,
|
||||
guildId: params.interactionCtx.rawGuildId,
|
||||
senderId: params.interactionCtx.userId,
|
||||
@ -894,11 +917,13 @@ async function dispatchPluginDiscordInteractiveEvent(params: {
|
||||
if (!dispatched.matched) {
|
||||
return "unmatched";
|
||||
}
|
||||
if (dispatched.handled && !responded) {
|
||||
try {
|
||||
await respond.acknowledge();
|
||||
} catch {
|
||||
// Interaction may have expired after the handler finished.
|
||||
if (dispatched.handled) {
|
||||
if (!responded) {
|
||||
try {
|
||||
await respond.acknowledge();
|
||||
} catch {
|
||||
// Interaction may have expired after the handler finished.
|
||||
}
|
||||
}
|
||||
return "handled";
|
||||
}
|
||||
|
||||
@ -9,6 +9,7 @@ import type { GatewayPresenceUpdate } from "discord-api-types/v10";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../../../../src/config/config.js";
|
||||
import type { DiscordAccountConfig } from "../../../../src/config/types.discord.js";
|
||||
import { buildPluginBindingApprovalCustomId } from "../../../../src/plugins/conversation-binding.js";
|
||||
import { buildAgentSessionKey } from "../../../../src/routing/resolve-route.js";
|
||||
import {
|
||||
clearDiscordComponentEntries,
|
||||
@ -53,6 +54,8 @@ const recordInboundSessionMock = vi.hoisted(() => vi.fn());
|
||||
const readSessionUpdatedAtMock = vi.hoisted(() => vi.fn());
|
||||
const resolveStorePathMock = vi.hoisted(() => vi.fn());
|
||||
const dispatchPluginInteractiveHandlerMock = vi.hoisted(() => vi.fn());
|
||||
const resolvePluginConversationBindingApprovalMock = vi.hoisted(() => vi.fn());
|
||||
const buildPluginBindingResolvedTextMock = vi.hoisted(() => vi.fn());
|
||||
let lastDispatchCtx: Record<string, unknown> | undefined;
|
||||
|
||||
vi.mock("../../../../src/pairing/pairing-store.js", () => ({
|
||||
@ -89,8 +92,20 @@ vi.mock("../../../../src/config/sessions.js", async (importOriginal) => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../../plugins/interactive.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../../plugins/interactive.js")>();
|
||||
vi.mock("../../../../src/plugins/conversation-binding.js", async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import("../../../../src/plugins/conversation-binding.js")>();
|
||||
return {
|
||||
...actual,
|
||||
resolvePluginConversationBindingApproval: (...args: unknown[]) =>
|
||||
resolvePluginConversationBindingApprovalMock(...args),
|
||||
buildPluginBindingResolvedText: (...args: unknown[]) =>
|
||||
buildPluginBindingResolvedTextMock(...args),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../../../../src/plugins/interactive.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../../../../src/plugins/interactive.js")>();
|
||||
return {
|
||||
...actual,
|
||||
dispatchPluginInteractiveHandler: (...args: unknown[]) =>
|
||||
@ -356,6 +371,33 @@ describe("discord component interactions", () => {
|
||||
handled: false,
|
||||
duplicate: false,
|
||||
});
|
||||
resolvePluginConversationBindingApprovalMock.mockReset().mockResolvedValue({
|
||||
status: "approved",
|
||||
binding: {
|
||||
bindingId: "binding-1",
|
||||
pluginId: "openclaw-codex-app-server",
|
||||
pluginName: "OpenClaw App Server",
|
||||
pluginRoot: "/plugins/codex",
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
conversationId: "user:123456789",
|
||||
boundAt: Date.now(),
|
||||
},
|
||||
request: {
|
||||
id: "approval-1",
|
||||
pluginId: "openclaw-codex-app-server",
|
||||
pluginName: "OpenClaw App Server",
|
||||
pluginRoot: "/plugins/codex",
|
||||
requestedAt: Date.now(),
|
||||
conversation: {
|
||||
channel: "discord",
|
||||
accountId: "default",
|
||||
conversationId: "user:123456789",
|
||||
},
|
||||
},
|
||||
decision: "allow-once",
|
||||
});
|
||||
buildPluginBindingResolvedTextMock.mockReset().mockReturnValue("Binding approved.");
|
||||
});
|
||||
|
||||
it("routes button clicks with reply references", async () => {
|
||||
@ -599,6 +641,30 @@ describe("discord component interactions", () => {
|
||||
expect(dispatchReplyMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not fall through to Claw when a plugin Discord interaction already replied", async () => {
|
||||
registerDiscordComponentEntries({
|
||||
entries: [createButtonEntry({ callbackData: "codex:approve" })],
|
||||
modals: [],
|
||||
});
|
||||
dispatchPluginInteractiveHandlerMock.mockImplementation(async (params: any) => {
|
||||
await params.respond.reply({ text: "✓", ephemeral: true });
|
||||
return {
|
||||
matched: true,
|
||||
handled: true,
|
||||
duplicate: false,
|
||||
};
|
||||
});
|
||||
|
||||
const button = createDiscordComponentButton(createComponentContext());
|
||||
const { interaction, reply } = createComponentButtonInteraction();
|
||||
|
||||
await button.run(interaction, { cid: "btn_1" } as ComponentData);
|
||||
|
||||
expect(dispatchPluginInteractiveHandlerMock).toHaveBeenCalledTimes(1);
|
||||
expect(reply).toHaveBeenCalledWith({ content: "✓", ephemeral: true });
|
||||
expect(dispatchReplyMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls through to built-in Discord component routing when a plugin declines handling", async () => {
|
||||
registerDiscordComponentEntries({
|
||||
entries: [createButtonEntry({ callbackData: "codex:approve" })],
|
||||
@ -619,6 +685,35 @@ describe("discord component interactions", () => {
|
||||
expect(reply).toHaveBeenCalledWith({ content: "✓" });
|
||||
expect(dispatchReplyMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("resolves plugin binding approvals without falling through to Claw", async () => {
|
||||
registerDiscordComponentEntries({
|
||||
entries: [
|
||||
createButtonEntry({
|
||||
callbackData: buildPluginBindingApprovalCustomId("approval-1", "allow-once"),
|
||||
}),
|
||||
],
|
||||
modals: [],
|
||||
});
|
||||
const button = createDiscordComponentButton(createComponentContext());
|
||||
const update = vi.fn().mockResolvedValue(undefined);
|
||||
const followUp = vi.fn().mockResolvedValue(undefined);
|
||||
const interaction = {
|
||||
...(createComponentButtonInteraction().interaction as any),
|
||||
update,
|
||||
followUp,
|
||||
} as ButtonInteraction;
|
||||
|
||||
await button.run(interaction, { cid: "btn_1" } as ComponentData);
|
||||
|
||||
expect(resolvePluginConversationBindingApprovalMock).toHaveBeenCalledTimes(1);
|
||||
expect(update).toHaveBeenCalledWith({ components: [] });
|
||||
expect(followUp).toHaveBeenCalledWith({
|
||||
content: "Binding approved.",
|
||||
ephemeral: true,
|
||||
});
|
||||
expect(dispatchReplyMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveDiscordOwnerAllowFrom", () => {
|
||||
|
||||
@ -92,8 +92,10 @@ vi.mock("../infra/home-dir.js", () => ({
|
||||
|
||||
const {
|
||||
__testing,
|
||||
buildPluginBindingApprovalCustomId,
|
||||
detachPluginConversationBinding,
|
||||
getCurrentPluginConversationBinding,
|
||||
parsePluginBindingApprovalCustomId,
|
||||
requestPluginConversationBinding,
|
||||
resolvePluginConversationBindingApproval,
|
||||
} = await import("./conversation-binding.js");
|
||||
@ -132,6 +134,20 @@ describe("plugin conversation binding approvals", () => {
|
||||
registerSessionBindingAdapter(createAdapter("telegram", "default"));
|
||||
});
|
||||
|
||||
it("keeps Telegram bind approval callback_data within Telegram's limit", () => {
|
||||
const allowOnce = buildPluginBindingApprovalCustomId("abcdefghijkl", "allow-once");
|
||||
const allowAlways = buildPluginBindingApprovalCustomId("abcdefghijkl", "allow-always");
|
||||
const deny = buildPluginBindingApprovalCustomId("abcdefghijkl", "deny");
|
||||
|
||||
expect(Buffer.byteLength(allowOnce, "utf8")).toBeLessThanOrEqual(64);
|
||||
expect(Buffer.byteLength(allowAlways, "utf8")).toBeLessThanOrEqual(64);
|
||||
expect(Buffer.byteLength(deny, "utf8")).toBeLessThanOrEqual(64);
|
||||
expect(parsePluginBindingApprovalCustomId(allowAlways)).toEqual({
|
||||
approvalId: "abcdefghijkl",
|
||||
decision: "allow-always",
|
||||
});
|
||||
});
|
||||
|
||||
it("requires a fresh approval again after allow-once is consumed", async () => {
|
||||
const firstRequest = await requestPluginConversationBinding({
|
||||
pluginId: "codex",
|
||||
|
||||
@ -269,6 +269,11 @@ function buildTelegramButtons(approvalId: string) {
|
||||
];
|
||||
}
|
||||
|
||||
function createApprovalRequestId(): string {
|
||||
// Keep approval ids compact so Telegram callback_data stays under its 64-byte limit.
|
||||
return crypto.randomBytes(9).toString("base64url");
|
||||
}
|
||||
|
||||
function loadApprovalsFromDisk(): PluginBindingApprovalsFile {
|
||||
const filePath = resolveApprovalsPath();
|
||||
try {
|
||||
@ -507,10 +512,8 @@ export function buildPluginBindingApprovalCustomId(
|
||||
approvalId: string,
|
||||
decision: PluginBindingApprovalDecision,
|
||||
): string {
|
||||
return [
|
||||
`${PLUGIN_BINDING_CUSTOM_ID_PREFIX}:id=${encodeCustomIdValue(approvalId)}`,
|
||||
`decision=${decision}`,
|
||||
].join(";");
|
||||
const decisionCode = decision === "allow-once" ? "o" : decision === "allow-always" ? "a" : "d";
|
||||
return `${PLUGIN_BINDING_CUSTOM_ID_PREFIX}:${encodeCustomIdValue(approvalId)}:${decisionCode}`;
|
||||
}
|
||||
|
||||
export function parsePluginBindingApprovalCustomId(
|
||||
@ -521,13 +524,24 @@ export function parsePluginBindingApprovalCustomId(
|
||||
return null;
|
||||
}
|
||||
const body = trimmed.slice(`${PLUGIN_BINDING_CUSTOM_ID_PREFIX}:`.length);
|
||||
const params = new URLSearchParams(body.replaceAll(";", "&"));
|
||||
const rawId = params.get("id")?.trim() ?? "";
|
||||
const rawDecision = params.get("decision")?.trim() ?? "";
|
||||
const separator = body.lastIndexOf(":");
|
||||
if (separator <= 0 || separator === body.length - 1) {
|
||||
return null;
|
||||
}
|
||||
const rawId = body.slice(0, separator).trim();
|
||||
const rawDecisionCode = body.slice(separator + 1).trim();
|
||||
if (!rawId) {
|
||||
return null;
|
||||
}
|
||||
if (rawDecision !== "allow-once" && rawDecision !== "allow-always" && rawDecision !== "deny") {
|
||||
const rawDecision =
|
||||
rawDecisionCode === "o"
|
||||
? "allow-once"
|
||||
: rawDecisionCode === "a"
|
||||
? "allow-always"
|
||||
: rawDecisionCode === "d"
|
||||
? "deny"
|
||||
: null;
|
||||
if (!rawDecision) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
@ -610,7 +624,7 @@ export async function requestPluginConversationBinding(params: {
|
||||
}
|
||||
|
||||
const request: PendingPluginBindingRequest = {
|
||||
id: crypto.randomUUID(),
|
||||
id: createApprovalRequestId(),
|
||||
pluginId: params.pluginId,
|
||||
pluginName: params.pluginName,
|
||||
pluginRoot: params.pluginRoot,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user