diff --git a/extensions/discord/src/monitor/agent-components.ts b/extensions/discord/src/monitor/agent-components.ts index 88d4f697d17..08562aeb2c7 100644 --- a/extensions/discord/src/monitor/agent-components.ts +++ b/extensions/discord/src/monitor/agent-components.ts @@ -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"; } diff --git a/extensions/discord/src/monitor/monitor.test.ts b/extensions/discord/src/monitor/monitor.test.ts index b55e4d47d3d..ae829e33c50 100644 --- a/extensions/discord/src/monitor/monitor.test.ts +++ b/extensions/discord/src/monitor/monitor.test.ts @@ -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 | 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(); +vi.mock("../../../../src/plugins/conversation-binding.js", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + resolvePluginConversationBindingApproval: (...args: unknown[]) => + resolvePluginConversationBindingApprovalMock(...args), + buildPluginBindingResolvedText: (...args: unknown[]) => + buildPluginBindingResolvedTextMock(...args), + }; +}); + +vi.mock("../../../../src/plugins/interactive.js", async (importOriginal) => { + const actual = await importOriginal(); 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", () => { diff --git a/src/plugins/conversation-binding.test.ts b/src/plugins/conversation-binding.test.ts index ba2f4510c90..a5bcc81d2e5 100644 --- a/src/plugins/conversation-binding.test.ts +++ b/src/plugins/conversation-binding.test.ts @@ -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", diff --git a/src/plugins/conversation-binding.ts b/src/plugins/conversation-binding.ts index b454d58864c..7cae2304c16 100644 --- a/src/plugins/conversation-binding.ts +++ b/src/plugins/conversation-binding.ts @@ -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,