vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-04-01 04:11:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: share approval interactive renderers

Browse Source
This commit is contained in:
Peter Steinberger
2026-03-30 08:02:37 +09:00
parent cfac0e8698
commit 52fb4a149a
15 changed files with 289 additions and 491 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
src/infra/exec-approval-forwarder.test.ts
View File

@@ -1,10 +1,7 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import type { ChannelPlugin } from "../channels/plugins/types.js";
import type { OpenClawConfig } from "../config/config.js";
import {
buildTelegramExecApprovalPendingPayload,
shouldSuppressTelegramExecApprovalForwardingFallback,
} from "../plugin-sdk/telegram.js";
import { shouldSuppressTelegramExecApprovalForwardingFallback } from "../plugin-sdk/telegram.js";
import { setActivePluginRegistry } from "../plugins/runtime.js";
import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js";
import { createExecApprovalForwarder } from "./exec-approval-forwarder.js";
@@ -56,12 +53,6 @@ const telegramApprovalPlugin: Pick<
shouldSuppressForwardingFallback: (params) =>
shouldSuppressTelegramExecApprovalForwardingFallback(params),
},
render: {
exec: {
buildPendingPayload: ({ request, nowMs }) =>
buildTelegramExecApprovalPendingPayload({ request, nowMs }),
},
},
},
};
const discordApprovalPlugin: Pick<
@@ -306,7 +297,7 @@ describe("exec approval forwarder", () => {
expect(deliver).not.toHaveBeenCalled();
});
it("attaches explicit telegram buttons in forwarded telegram fallback payloads", async () => {
it("attaches shared interactive approval buttons in forwarded fallback payloads", async () => {
vi.useFakeTimers();
const { deliver, forwarder } = createForwarder({
cfg: makeTargetsCfg([{ channel: "telegram", to: "123" }]),
@@ -334,15 +325,30 @@ describe("exec approval forwarder", () => {
execApproval: expect.objectContaining({
approvalId: "req-1",
}),
telegram: {
buttons: [
[
{ text: "Allow Once", callback_data: "/approve req-1 allow-once" },
{ text: "Allow Always", callback_data: "/approve req-1 always" },
},
interactive: {
blocks: [
{
type: "buttons",
buttons: [
{
label: "Allow Once",
value: "/approve req-1 allow-once",
style: "success",
},
{
label: "Allow Always",
value: "/approve req-1 always",
style: "primary",
},
{
label: "Deny",
value: "/approve req-1 deny",
style: "danger",
},
],
[{ text: "Deny", callback_data: "/approve req-1 deny" }],
],
},
},
],
},
}),
],

19
src/infra/exec-approval-forwarder.ts
View File

@@ -7,6 +7,10 @@ import type {
ExecApprovalForwardTarget,
} from "../config/types.approvals.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import {
buildApprovalPendingReplyPayload,
buildPluginApprovalPendingReplyPayload,
} from "../plugin-sdk/approval-renderers.js";
import { parseAgentSessionKey } from "../routing/session-key.js";
import { compileConfigRegex } from "../security/config-regex.js";
import { testRegexWithBoundedInput } from "../security/safe-regex.js";
@@ -284,7 +288,11 @@ function buildRequestPayloadForTarget(
if (pluginPayload) {
return pluginPayload;
}
return { text: buildRequestMessage(request, nowMsValue) };
return buildApprovalPendingReplyPayload({
approvalId: request.id,
approvalSlug: request.id.slice(0, 8),
text: buildRequestMessage(request, nowMsValue),
});
}
function buildResolvedPayloadForTarget(
@@ -563,7 +571,14 @@ export function createExecApprovalForwarder(
nowMs: nowMs(),
})
: null;
return adapterPayload ?? { text: buildPluginApprovalRequestMessage(request, nowMs()) };
return (
adapterPayload ??
buildPluginApprovalPendingReplyPayload({
request,
nowMs: nowMs(),
text: buildPluginApprovalRequestMessage(request, nowMs()),
})
);
},
beforeDeliver: async (target, payload) => {
const channel = normalizeMessageChannel(target.channel) ?? target.channel;

24
src/infra/exec-approval-reply.test.ts
View File

@@ -95,6 +95,30 @@ describe("exec approval reply helpers", () => {
allowedDecisions: ["allow-once", "allow-always", "deny"],
},
});
expect(payload.interactive).toEqual({
blocks: [
{
type: "buttons",
buttons: [
{
label: "Allow Once",
value: "/approve req-1 allow-once",
style: "success",
},
{
label: "Allow Always",
value: "/approve req-1 always",
style: "primary",
},
{
label: "Deny",
value: "/approve req-1 deny",
style: "danger",
},
],
},
],
});
expect(payload.text).toContain("Heads up.");
expect(payload.text).toContain("```txt\n/approve slug-1 allow-once\n```");
expect(payload.text).toContain("```sh\necho ok\n```");

53
src/infra/exec-approval-reply.ts
View File

@@ -1,4 +1,5 @@
import type { ReplyPayload } from "../auto-reply/types.js";
import type { InteractiveReply, InteractiveReplyButton } from "../interactive/payload.js";
import type { ExecHost } from "./exec-approvals.js";
export type ExecApprovalReplyDecision = "allow-once" | "allow-always" | "deny";
@@ -33,6 +34,55 @@ export type ExecApprovalUnavailableReplyParams = {
sentApproverDms?: boolean;
};
const DEFAULT_ALLOWED_DECISIONS = ["allow-once", "allow-always", "deny"] as const;
function buildApprovalDecisionCommandValue(params: {
approvalId: string;
decision: ExecApprovalReplyDecision;
}): string {
return `/approve ${params.approvalId} ${params.decision === "allow-always" ? "always" : params.decision}`;
}
function buildApprovalInteractiveButtons(
allowedDecisions: readonly ExecApprovalReplyDecision[],
approvalId: string,
): InteractiveReplyButton[] {
const buttons: InteractiveReplyButton[] = [];
if (allowedDecisions.includes("allow-once")) {
buttons.push({
label: "Allow Once",
value: buildApprovalDecisionCommandValue({ approvalId, decision: "allow-once" }),
style: "success",
});
}
if (allowedDecisions.includes("allow-always")) {
buttons.push({
label: "Allow Always",
value: buildApprovalDecisionCommandValue({ approvalId, decision: "allow-always" }),
style: "primary",
});
}
if (allowedDecisions.includes("deny")) {
buttons.push({
label: "Deny",
value: buildApprovalDecisionCommandValue({ approvalId, decision: "deny" }),
style: "danger",
});
}
return buttons;
}
export function buildApprovalInteractiveReply(params: {
approvalId: string;
allowedDecisions?: readonly ExecApprovalReplyDecision[];
}): InteractiveReply | undefined {
const buttons = buildApprovalInteractiveButtons(
params.allowedDecisions ?? DEFAULT_ALLOWED_DECISIONS,
params.approvalId,
);
return buttons.length > 0 ? { blocks: [{ type: "buttons", buttons }] } : undefined;
}
export function getExecApprovalApproverDmNoticeText(): string {
return "Approval required. I sent approval DMs to the approvers for this account.";
}
@@ -137,11 +187,12 @@ export function buildExecApprovalPendingReplyPayload(
return {
text: lines.join("\n\n"),
interactive: buildApprovalInteractiveReply({ approvalId: params.approvalId }),
channelData: {
execApproval: {
approvalId: params.approvalId,
approvalSlug: params.approvalSlug,
allowedDecisions: ["allow-once", "allow-always", "deny"],
allowedDecisions: DEFAULT_ALLOWED_DECISIONS,
},
},
};

42
src/infra/plugin-approval-forwarder.test.ts
View File

@@ -83,13 +83,38 @@ describe("plugin approval forwarding", () => {
expect(deliver).toHaveBeenCalled();
});
const deliveryArgs = deliver.mock.calls[0]?.[0] as
| { payloads?: Array<{ text?: string }> }
| { payloads?: Array<{ text?: string; interactive?: unknown }> }
| undefined;
const text = deliveryArgs?.payloads?.[0]?.text ?? "";
const payload = deliveryArgs?.payloads?.[0];
const text = payload?.text ?? "";
expect(text).toContain("Plugin approval required");
expect(text).toContain("Sensitive tool call");
expect(text).toContain("plugin-req-1");
expect(text).toContain("/approve");
expect(payload?.interactive).toEqual({
blocks: [
{
type: "buttons",
buttons: [
{
label: "Allow Once",
value: "/approve plugin-req-1 allow-once",
style: "success",
},
{
label: "Allow Always",
value: "/approve plugin-req-1 always",
style: "primary",
},
{
label: "Deny",
value: "/approve plugin-req-1 deny",
style: "danger",
},
],
},
],
});
});
it("includes severity icon for critical", async () => {
@@ -180,19 +205,6 @@ describe("plugin approval forwarding", () => {
expect(deliveryArgs?.payloads?.[0]?.text).toBe("custom adapter payload");
});
it("falls back to plugin text when no adapter exists", async () => {
const deliver = vi.fn().mockResolvedValue([]);
const { forwarder } = createForwarder({ cfg: PLUGIN_TARGETS_CFG, deliver });
await forwarder.handlePluginApprovalRequested!(makePluginRequest());
await vi.waitFor(() => {
expect(deliver).toHaveBeenCalled();
});
const text =
(deliver.mock.calls[0]?.[0] as { payloads?: Array<{ text?: string }> })?.payloads?.[0]
?.text ?? "";
expect(text).toContain("Plugin approval required");
});
it("calls beforeDeliverPending before plugin approval delivery", async () => {
const beforeDeliverPending = vi.fn();
const adapterPlugin: Pick<

63
src/plugin-sdk/approval-renderers.test.ts
View File

@@ -1,10 +1,45 @@
import { describe, expect, it } from "vitest";
import {
buildApprovalPendingReplyPayload,
buildPluginApprovalPendingReplyPayload,
buildPluginApprovalResolvedReplyPayload,
} from "./approval-renderers.js";
describe("plugin-sdk/approval-renderers", () => {
it("builds shared approval payloads with generic interactive commands", () => {
const payload = buildApprovalPendingReplyPayload({
approvalId: "plugin:approval-123",
approvalSlug: "plugin:a",
text: "Approval required @everyone",
});
expect(payload.text).toContain("@\u200beveryone");
expect(payload.interactive).toEqual({
blocks: [
{
type: "buttons",
buttons: [
{
label: "Allow Once",
value: "/approve plugin:approval-123 allow-once",
style: "success",
},
{
label: "Allow Always",
value: "/approve plugin:approval-123 always",
style: "primary",
},
{
label: "Deny",
value: "/approve plugin:approval-123 deny",
style: "danger",
},
],
},
],
});
});
it("builds plugin pending payloads with approval metadata and extra channel data", () => {
const payload = buildPluginApprovalPendingReplyPayload({
request: {
@@ -20,12 +55,36 @@ describe("plugin-sdk/approval-renderers", () => {
approvalSlug: "custom-slug",
channelData: {
telegram: {
buttons: [[{ text: "Allow Once", callback_data: "/approve id allow-once" }]],
quoteText: "quoted",
},
},
});
expect(payload.text).toContain("Plugin approval required");
expect(payload.interactive).toEqual({
blocks: [
{
type: "buttons",
buttons: [
{
label: "Allow Once",
value: "/approve plugin-approval-123 allow-once",
style: "success",
},
{
label: "Allow Always",
value: "/approve plugin-approval-123 always",
style: "primary",
},
{
label: "Deny",
value: "/approve plugin-approval-123 deny",
style: "danger",
},
],
},
],
});
expect(payload.channelData).toMatchObject({
execApproval: {
approvalId: "plugin-approval-123",
@@ -33,7 +92,7 @@ describe("plugin-sdk/approval-renderers", () => {
allowedDecisions: ["allow-once", "allow-always", "deny"],
},
telegram: {
buttons: [[{ text: "Allow Once", callback_data: "/approve id allow-once" }]],
quoteText: "quoted",
},
});
});

62
src/plugin-sdk/approval-renderers.ts
View File

@@ -1,5 +1,8 @@
import type { ReplyPayload } from "../auto-reply/types.js";
import type { ExecApprovalReplyDecision } from "../infra/exec-approval-reply.js";
import {
buildApprovalInteractiveReply,
type ExecApprovalReplyDecision,
} from "../infra/exec-approval-reply.js";
import {
buildPluginApprovalRequestMessage,
buildPluginApprovalResolvedMessage,
@@ -9,6 +12,39 @@ import {
const DEFAULT_ALLOWED_DECISIONS = ["allow-once", "allow-always", "deny"] as const;
function neutralizeApprovalText(value: string): string {
return value
.replace(/@everyone/gi, "@\u200beveryone")
.replace(/@here/gi, "@\u200bhere")
.replace(/<@/g, "<@\u200b")
.replace(/<#/g, "<#\u200b");
}
export function buildApprovalPendingReplyPayload(params: {
approvalId: string;
approvalSlug: string;
text: string;
allowedDecisions?: readonly ExecApprovalReplyDecision[];
channelData?: Record<string, unknown>;
}): ReplyPayload {
const allowedDecisions = params.allowedDecisions ?? DEFAULT_ALLOWED_DECISIONS;
return {
text: neutralizeApprovalText(params.text),
interactive: buildApprovalInteractiveReply({
approvalId: params.approvalId,
allowedDecisions,
}),
channelData: {
execApproval: {
approvalId: params.approvalId,
approvalSlug: params.approvalSlug,
allowedDecisions,
},
...params.channelData,
},
};
}
export function buildPluginApprovalPendingReplyPayload(params: {
request: PluginApprovalRequest;
nowMs: number;
@@ -17,17 +53,13 @@ export function buildPluginApprovalPendingReplyPayload(params: {
allowedDecisions?: readonly ExecApprovalReplyDecision[];
channelData?: Record<string, unknown>;
}): ReplyPayload {
return {
return buildApprovalPendingReplyPayload({
approvalId: params.request.id,
approvalSlug: params.approvalSlug ?? params.request.id.slice(0, 8),
text: params.text ?? buildPluginApprovalRequestMessage(params.request, params.nowMs),
channelData: {
execApproval: {
approvalId: params.request.id,
approvalSlug: params.approvalSlug ?? params.request.id.slice(0, 8),
allowedDecisions: params.allowedDecisions ?? DEFAULT_ALLOWED_DECISIONS,
},
...params.channelData,
},
};
allowedDecisions: params.allowedDecisions,
channelData: params.channelData,
});
}
export function buildPluginApprovalResolvedReplyPayload(params: {
@@ -37,10 +69,14 @@ export function buildPluginApprovalResolvedReplyPayload(params: {
}): ReplyPayload {
return params.channelData
? {
text: params.text ?? buildPluginApprovalResolvedMessage(params.resolved),
text: neutralizeApprovalText(
params.text ?? buildPluginApprovalResolvedMessage(params.resolved),
),
channelData: params.channelData,
}
: {
text: params.text ?? buildPluginApprovalResolvedMessage(params.resolved),
text: neutralizeApprovalText(
params.text ?? buildPluginApprovalResolvedMessage(params.resolved),
),
};
}

1
src/plugin-sdk/approval-runtime.ts
View File

@@ -33,6 +33,7 @@ export {
} from "../infra/plugin-approvals.js";
export { createApproverRestrictedNativeApprovalAdapter } from "./approval-delivery-helpers.js";
export {
buildApprovalPendingReplyPayload,
buildPluginApprovalPendingReplyPayload,
buildPluginApprovalResolvedReplyPayload,
} from "./approval-renderers.js";