mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix(slack): resolve native approval buttons
Co-authored-by: Motoki Maruyama <motoki.maruyama@kiconiaworks.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Slack/exec approvals: resolve native approval button clicks over the Gateway instead of delivering `/approve ...` as plain agent text, preserving retry buttons if Gateway resolution fails. Fixes #71023. (#71025) Thanks @marusan03.
|
||||
- Slack/files: return non-image `download-file` results as local file paths instead of image payloads, and include Slack file IDs in inbound file placeholders so agents can call `download-file`. Fixes #71212. Thanks @teamrazo.
|
||||
- Discord/replies: run `message_sending` plugin hooks for Discord reply delivery, including DM targets, so plugins can transform or cancel outbound Discord replies consistently with other channels. Fixes #59350. (#71094) Thanks @wei840222.
|
||||
- Control UI/commands: carry provider-owned thinking option ids/labels in session rows and defaults so fresh sessions show and accept dynamic modes such as `adaptive`, `xhigh`, and `max`. Fixes #71269. Thanks @Young-Khalil.
|
||||
|
||||
@@ -1,7 +1,15 @@
|
||||
import type { SlackActionMiddlewareArgs } from "@slack/bolt";
|
||||
import type { Block, KnownBlock } from "@slack/web-api";
|
||||
import { enqueueSystemEvent } from "openclaw/plugin-sdk/infra-runtime";
|
||||
import { resolveApprovalOverGateway } from "openclaw/plugin-sdk/approval-gateway-runtime";
|
||||
import {
|
||||
enqueueSystemEvent,
|
||||
parseExecApprovalCommandText,
|
||||
} from "openclaw/plugin-sdk/infra-runtime";
|
||||
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
|
||||
import {
|
||||
isSlackExecApprovalApprover,
|
||||
isSlackExecApprovalAuthorizedSender,
|
||||
} from "../../exec-approvals.js";
|
||||
import { dispatchSlackPluginInteractiveHandler } from "../../interactive-dispatch.js";
|
||||
import {
|
||||
SLACK_REPLY_BUTTON_ACTION_ID,
|
||||
@@ -521,6 +529,68 @@ async function handleSlackPluginBindingApproval(params: {
|
||||
return true;
|
||||
}
|
||||
|
||||
async function handleSlackExecApprovalInteraction(params: {
|
||||
ctx: SlackMonitorContext;
|
||||
parsed: ParsedSlackBlockAction;
|
||||
pluginInteractionData: string;
|
||||
respond?: SlackBlockActionRespond;
|
||||
}): Promise<boolean> {
|
||||
const approval = parseExecApprovalCommandText(params.pluginInteractionData);
|
||||
if (!approval) {
|
||||
return false;
|
||||
}
|
||||
const pluginApprovalAuthorizedSender = isSlackExecApprovalApprover({
|
||||
cfg: params.ctx.cfg,
|
||||
accountId: params.ctx.accountId,
|
||||
senderId: params.parsed.userId,
|
||||
});
|
||||
const execApprovalAuthorizedSender = isSlackExecApprovalAuthorizedSender({
|
||||
cfg: params.ctx.cfg,
|
||||
accountId: params.ctx.accountId,
|
||||
senderId: params.parsed.userId,
|
||||
});
|
||||
const isPluginApproval = approval.approvalId.startsWith("plugin:");
|
||||
const authorized = isPluginApproval
|
||||
? pluginApprovalAuthorizedSender
|
||||
: execApprovalAuthorizedSender || pluginApprovalAuthorizedSender;
|
||||
if (!authorized) {
|
||||
params.ctx.runtime.log?.(
|
||||
`slack:interaction drop exec approval user=${params.parsed.userId} (not authorized)`,
|
||||
);
|
||||
await respondEphemeral(params.respond, "You are not authorized to approve this request.");
|
||||
return true;
|
||||
}
|
||||
|
||||
try {
|
||||
await resolveApprovalOverGateway({
|
||||
cfg: params.ctx.cfg,
|
||||
approvalId: approval.approvalId,
|
||||
decision: approval.decision,
|
||||
senderId: params.parsed.userId,
|
||||
allowPluginFallback: pluginApprovalAuthorizedSender,
|
||||
clientDisplayName: `Slack approval (${params.parsed.userId.trim() || "unknown"})`,
|
||||
});
|
||||
} catch (error) {
|
||||
params.ctx.runtime.log?.(
|
||||
`slack:interaction exec approval resolve failed id=${approval.approvalId}: ${String(error)}`,
|
||||
);
|
||||
throw error;
|
||||
}
|
||||
|
||||
try {
|
||||
await updateSlackInteractionMessage({
|
||||
ctx: params.ctx,
|
||||
channelId: params.parsed.channelId,
|
||||
messageTs: params.parsed.messageTs,
|
||||
text: params.parsed.typedBody.message?.text ?? "",
|
||||
blocks: [],
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup only.
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
async function dispatchSlackPluginInteraction(params: {
|
||||
ctx: SlackMonitorContext;
|
||||
parsed: ParsedSlackBlockAction;
|
||||
@@ -742,6 +812,21 @@ async function handleSlackBlockAction(params: {
|
||||
return;
|
||||
}
|
||||
params.trackEvent?.();
|
||||
const pluginInteractionData = buildSlackPluginInteractionData({
|
||||
actionId: parsed.actionId,
|
||||
summary: parsed.actionSummary,
|
||||
});
|
||||
if (pluginInteractionData && isSlackReplyActionId(parsed.actionId)) {
|
||||
const handledExecApproval = await handleSlackExecApprovalInteraction({
|
||||
ctx: params.ctx,
|
||||
parsed,
|
||||
pluginInteractionData,
|
||||
respond,
|
||||
});
|
||||
if (handledExecApproval) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
const auth = await authorizeSlackBlockAction({
|
||||
ctx: params.ctx,
|
||||
parsed,
|
||||
@@ -750,10 +835,6 @@ async function handleSlackBlockAction(params: {
|
||||
if (!auth.allowed) {
|
||||
return;
|
||||
}
|
||||
const pluginInteractionData = buildSlackPluginInteractionData({
|
||||
actionId: parsed.actionId,
|
||||
summary: parsed.actionSummary,
|
||||
});
|
||||
if (pluginInteractionData && isSlackReplyActionId(parsed.actionId)) {
|
||||
const handledBindingApproval = await handleSlackPluginBindingApproval({
|
||||
ctx: params.ctx,
|
||||
|
||||
@@ -10,11 +10,22 @@ const dispatchPluginInteractiveHandlerMock = vi.hoisted(() =>
|
||||
);
|
||||
const resolvePluginConversationBindingApprovalMock = vi.hoisted(() => vi.fn());
|
||||
const buildPluginBindingResolvedTextMock = vi.hoisted(() => vi.fn(() => "Binding updated."));
|
||||
const resolveApprovalOverGatewayMock = vi.hoisted(() =>
|
||||
vi.fn<(arg: unknown) => Promise<void>>(async () => undefined),
|
||||
);
|
||||
|
||||
let registerSlackInteractionEvents: typeof import("./interactions.js").registerSlackInteractionEvents;
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/infra-runtime", () => ({
|
||||
enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args),
|
||||
vi.mock("openclaw/plugin-sdk/infra-runtime", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("openclaw/plugin-sdk/infra-runtime")>();
|
||||
return {
|
||||
...actual,
|
||||
enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/approval-gateway-runtime", () => ({
|
||||
resolveApprovalOverGateway: (arg: unknown) => resolveApprovalOverGatewayMock(arg),
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/security-runtime", () => ({
|
||||
@@ -161,6 +172,7 @@ function createContext(overrides?: {
|
||||
allowFrom?: string[];
|
||||
allowNameMatching?: boolean;
|
||||
channelsConfig?: Record<string, { users?: string[] }>;
|
||||
cfg?: Record<string, unknown>;
|
||||
shouldDropMismatchedSlackEvent?: (body: unknown) => boolean;
|
||||
isChannelAllowed?: (params: {
|
||||
channelId?: string;
|
||||
@@ -221,6 +233,17 @@ function createContext(overrides?: {
|
||||
const ctx = {
|
||||
app,
|
||||
accountId: "default",
|
||||
cfg: overrides?.cfg ?? {
|
||||
channels: {
|
||||
slack: {
|
||||
execApprovals: {
|
||||
enabled: true,
|
||||
approvers: ["U123"],
|
||||
target: "both",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
runtime: { log: runtimeLog },
|
||||
dmEnabled: overrides?.dmEnabled ?? true,
|
||||
dmPolicy: overrides?.dmPolicy ?? ("open" as const),
|
||||
@@ -263,6 +286,8 @@ describe("registerSlackInteractionEvents", () => {
|
||||
resolvePluginConversationBindingApprovalMock.mockResolvedValue({ status: "expired" });
|
||||
buildPluginBindingResolvedTextMock.mockClear();
|
||||
buildPluginBindingResolvedTextMock.mockReturnValue("Binding updated.");
|
||||
resolveApprovalOverGatewayMock.mockClear();
|
||||
resolveApprovalOverGatewayMock.mockResolvedValue(undefined);
|
||||
dispatchPluginInteractiveHandlerMock.mockResolvedValue({
|
||||
matched: false,
|
||||
handled: false,
|
||||
@@ -644,6 +669,168 @@ describe("registerSlackInteractionEvents", () => {
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves exec approvals from shared interactive Slack actions", async () => {
|
||||
const { ctx, app, getHandler } = createContext({ allowFrom: ["U999"] });
|
||||
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||
|
||||
const handler = getHandler();
|
||||
expect(handler).toBeTruthy();
|
||||
|
||||
const ack = vi.fn().mockResolvedValue(undefined);
|
||||
const respond = vi.fn().mockResolvedValue(undefined);
|
||||
await handler!({
|
||||
ack,
|
||||
respond,
|
||||
body: {
|
||||
user: { id: "U123" },
|
||||
channel: { id: "C1" },
|
||||
container: { channel_id: "C1", message_ts: "100.200", thread_ts: "100.100" },
|
||||
message: {
|
||||
ts: "100.200",
|
||||
text: "Exec approval required",
|
||||
blocks: [
|
||||
{
|
||||
type: "actions",
|
||||
block_id: "exec_actions",
|
||||
elements: [{ type: "button", action_id: "openclaw:reply_button" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
action: {
|
||||
type: "button",
|
||||
action_id: "openclaw:reply_button",
|
||||
block_id: "exec_actions",
|
||||
value: "/approve req-123 allow-once",
|
||||
text: { type: "plain_text", text: "Allow once" },
|
||||
},
|
||||
});
|
||||
|
||||
expect(ack).toHaveBeenCalled();
|
||||
expect(resolveApprovalOverGatewayMock).toHaveBeenCalledWith({
|
||||
cfg: ctx.cfg,
|
||||
approvalId: "req-123",
|
||||
decision: "allow-once",
|
||||
senderId: "U123",
|
||||
allowPluginFallback: true,
|
||||
clientDisplayName: "Slack approval (U123)",
|
||||
});
|
||||
expect(resolvePluginConversationBindingApprovalMock).not.toHaveBeenCalled();
|
||||
expect(dispatchPluginInteractiveHandlerMock).not.toHaveBeenCalled();
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
expect(app.client.chat.update).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
channel: "C1",
|
||||
ts: "100.200",
|
||||
text: "Exec approval required",
|
||||
blocks: [],
|
||||
}),
|
||||
);
|
||||
expect(respond).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps exec approval buttons when gateway resolution fails", async () => {
|
||||
resolveApprovalOverGatewayMock.mockRejectedValueOnce(new Error("gateway down"));
|
||||
const { ctx, app, getHandler } = createContext();
|
||||
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||
|
||||
const handler = getHandler();
|
||||
expect(handler).toBeTruthy();
|
||||
|
||||
const ack = vi.fn().mockResolvedValue(undefined);
|
||||
await expect(
|
||||
handler!({
|
||||
ack,
|
||||
body: {
|
||||
user: { id: "U123" },
|
||||
channel: { id: "C1" },
|
||||
container: { channel_id: "C1", message_ts: "100.200" },
|
||||
message: {
|
||||
ts: "100.200",
|
||||
text: "Exec approval required",
|
||||
blocks: [
|
||||
{
|
||||
type: "actions",
|
||||
block_id: "exec_actions",
|
||||
elements: [{ type: "button", action_id: "openclaw:reply_button" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
action: {
|
||||
type: "button",
|
||||
action_id: "openclaw:reply_button",
|
||||
block_id: "exec_actions",
|
||||
value: "/approve req-123 allow-once",
|
||||
text: { type: "plain_text", text: "Allow once" },
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow("gateway down");
|
||||
|
||||
expect(ack).toHaveBeenCalled();
|
||||
expect(resolveApprovalOverGatewayMock).toHaveBeenCalledTimes(1);
|
||||
expect(app.client.chat.update).not.toHaveBeenCalled();
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects unauthorized exec approval interactions without enqueueing them", async () => {
|
||||
const { ctx, app, getHandler } = createContext({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: {
|
||||
execApprovals: {
|
||||
enabled: true,
|
||||
approvers: ["U999"],
|
||||
target: "both",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||
|
||||
const handler = getHandler();
|
||||
expect(handler).toBeTruthy();
|
||||
|
||||
const ack = vi.fn().mockResolvedValue(undefined);
|
||||
const respond = vi.fn().mockResolvedValue(undefined);
|
||||
await handler!({
|
||||
ack,
|
||||
respond,
|
||||
body: {
|
||||
user: { id: "U123" },
|
||||
channel: { id: "C1" },
|
||||
container: { channel_id: "C1", message_ts: "100.200" },
|
||||
message: {
|
||||
ts: "100.200",
|
||||
text: "Exec approval required",
|
||||
blocks: [
|
||||
{
|
||||
type: "actions",
|
||||
block_id: "exec_actions",
|
||||
elements: [{ type: "button", action_id: "openclaw:reply_button" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
action: {
|
||||
type: "button",
|
||||
action_id: "openclaw:reply_button",
|
||||
block_id: "exec_actions",
|
||||
value: "/approve req-123 allow-once",
|
||||
text: { type: "plain_text", text: "Allow once" },
|
||||
},
|
||||
});
|
||||
|
||||
expect(resolveApprovalOverGatewayMock).not.toHaveBeenCalled();
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
expect(app.client.chat.update).not.toHaveBeenCalled();
|
||||
expect(respond).toHaveBeenCalledWith({
|
||||
text: "You are not authorized to approve this request.",
|
||||
response_type: "ephemeral",
|
||||
});
|
||||
});
|
||||
|
||||
it("drops block actions when mismatch guard triggers", async () => {
|
||||
enqueueSystemEventMock.mockClear();
|
||||
const { ctx, app, getHandler } = createContext({
|
||||
|
||||
Reference in New Issue
Block a user