refactor: move approval auth and payload hooks to generic channel capabilities

This commit is contained in:
Peter Steinberger 2026-03-30 08:46:27 +09:00
parent 7008379ff0
commit 3b878e6b86
No known key found for this signature in database
15 changed files with 196 additions and 138 deletions

View File

@ -344,17 +344,9 @@ export const discordPlugin: ChannelPlugin<ResolvedDiscordAccount, DiscordProbe>
hint: "<channelId|user:ID|channel:ID>",
},
},
auth: discordNativeApprovalAdapter.auth,
approvals: {
auth: discordNativeApprovalAdapter.auth,
delivery: {
...discordNativeApprovalAdapter.delivery,
shouldSuppressLocalPrompt: ({ cfg, accountId, payload }) =>
shouldSuppressLocalDiscordExecApprovalPrompt({
cfg,
accountId,
payload,
}),
},
delivery: discordNativeApprovalAdapter.delivery,
},
directory: createChannelDirectoryAdapter({
listPeers: async (params) => listDiscordDirectoryPeersFromConfig(params),
@ -660,6 +652,12 @@ export const discordPlugin: ChannelPlugin<ResolvedDiscordAccount, DiscordProbe>
chunker: null,
textChunkLimit: 2000,
pollMaxOptions: 10,
shouldSuppressLocalPayloadPrompt: ({ cfg, accountId, payload }) =>
shouldSuppressLocalDiscordExecApprovalPrompt({
cfg,
accountId,
payload,
}),
resolveTarget: ({ to }) => normalizeDiscordOutboundTarget(to),
},
attachedResults: {

View File

@ -52,6 +52,7 @@ import {
isTelegramExecApprovalAuthorizedSender,
isTelegramExecApprovalClientEnabled,
resolveTelegramExecApprovalTarget,
shouldSuppressLocalTelegramExecApprovalPrompt,
} from "./exec-approvals.js";
import {
resolveTelegramGroupRequireMention,
@ -463,32 +464,9 @@ export const telegramPlugin = createChatChannelPlugin({
await deleteTelegramUpdateOffset({ accountId });
},
},
auth: telegramNativeApprovalAdapter.auth,
approvals: {
auth: telegramNativeApprovalAdapter.auth,
delivery: {
...telegramNativeApprovalAdapter.delivery,
beforeDeliverPending: async ({ cfg, target, payload }) => {
const hasExecApprovalData =
payload.channelData &&
typeof payload.channelData === "object" &&
!Array.isArray(payload.channelData) &&
payload.channelData.execApproval;
if (!hasExecApprovalData) {
return;
}
const threadId =
typeof target.threadId === "number"
? target.threadId
: typeof target.threadId === "string"
? Number.parseInt(target.threadId, 10)
: undefined;
await sendTypingTelegram(target.to, {
cfg,
accountId: target.accountId ?? undefined,
...(Number.isFinite(threadId) ? { messageThreadId: threadId } : {}),
}).catch(() => {});
},
},
delivery: telegramNativeApprovalAdapter.delivery,
},
directory: createChannelDirectoryAdapter({
listPeers: async (params) => listTelegramDirectoryPeersFromConfig(params),
@ -742,6 +720,28 @@ export const telegramPlugin = createChatChannelPlugin({
chunkerMode: "markdown",
textChunkLimit: 4000,
pollMaxOptions: 10,
shouldSuppressLocalPayloadPrompt: ({ cfg, accountId, payload }) =>
shouldSuppressLocalTelegramExecApprovalPrompt({
cfg,
accountId,
payload,
}),
beforeDeliverPayload: async ({ cfg, target, hint }) => {
if (hint?.kind !== "approval-pending" || hint.approvalKind !== "exec") {
return;
}
const threadId =
typeof target.threadId === "number"
? target.threadId
: typeof target.threadId === "string"
? Number.parseInt(target.threadId, 10)
: undefined;
await sendTypingTelegram(target.to, {
cfg,
accountId: target.accountId ?? undefined,
...(Number.isFinite(threadId) ? { messageThreadId: threadId } : {}),
}).catch(() => {});
},
shouldSkipPlainTextSanitization: ({ payload }) => Boolean(payload.channelData),
resolveEffectiveTextChunkLimit: ({ fallbackLimit }) =>
typeof fallbackLimit === "number" ? Math.min(fallbackLimit, 4096) : 4096,

View File

@ -310,8 +310,9 @@ describe("dispatchReplyFromConfig", () => {
nativeCommands: true,
},
}),
execApprovals: {
shouldSuppressLocalPrompt: ({ payload }: { payload: ReplyPayload }) =>
outbound: {
deliveryMode: "direct",
shouldSuppressLocalPayloadPrompt: ({ payload }: { payload: ReplyPayload }) =>
Boolean(
payload.channelData &&
typeof payload.channelData === "object" &&

View File

@ -1,6 +1,5 @@
import type { ReplyPayload } from "../../auto-reply/types.js";
import type { OpenClawConfig } from "../../config/config.js";
import { resolveChannelApprovalAdapter } from "./approvals.js";
import { getChannelPlugin, normalizeChannelId } from "./registry.js";
export function shouldSuppressLocalExecApprovalPrompt(params: {
@ -14,12 +13,11 @@ export function shouldSuppressLocalExecApprovalPrompt(params: {
return false;
}
return (
resolveChannelApprovalAdapter(getChannelPlugin(channel))?.delivery?.shouldSuppressLocalPrompt?.(
{
cfg: params.cfg,
accountId: params.accountId,
payload: params.payload,
},
) ?? false
getChannelPlugin(channel)?.outbound?.shouldSuppressLocalPayloadPrompt?.({
cfg: params.cfg,
accountId: params.accountId,
payload: params.payload,
hint: { kind: "approval-pending", approvalKind: "exec" },
}) ?? false
);
}

View File

@ -28,11 +28,13 @@ import type {
ChannelStatusIssue,
} from "./types.core.js";
export type ChannelApprovalInitiatingSurfaceState =
export type ChannelActionAvailabilityState =
| { kind: "enabled" }
| { kind: "disabled" }
| { kind: "unsupported" };
export type ChannelApprovalInitiatingSurfaceState = ChannelActionAvailabilityState;
export type ChannelApprovalForwardTarget = {
channel: string;
to: string;
@ -152,6 +154,17 @@ export type ChannelOutboundPayloadContext = ChannelOutboundContext & {
payload: ReplyPayload;
};
export type ChannelOutboundPayloadHint =
| { kind: "approval-pending"; approvalKind: "exec" | "plugin" }
| { kind: "approval-resolved"; approvalKind: "exec" | "plugin" };
export type ChannelOutboundTargetRef = {
channel: string;
to: string;
accountId?: string | null;
threadId?: string | number | null;
};
export type ChannelOutboundFormattedContext = ChannelOutboundContext & {
abortSignal?: AbortSignal;
};
@ -169,6 +182,18 @@ export type ChannelOutboundAdapter = {
accountId?: string | null;
fallbackLimit?: number;
}) => number | undefined;
shouldSuppressLocalPayloadPrompt?: (params: {
cfg: OpenClawConfig;
accountId?: string | null;
payload: ReplyPayload;
hint?: ChannelOutboundPayloadHint;
}) => boolean;
beforeDeliverPayload?: (params: {
cfg: OpenClawConfig;
target: ChannelOutboundTargetRef;
payload: ReplyPayload;
hint?: ChannelOutboundPayloadHint;
}) => Promise<void> | void;
resolveTarget?: (params: {
cfg?: OpenClawConfig;
to?: string;
@ -372,6 +397,21 @@ export type ChannelAuthAdapter = {
verbose?: boolean;
channelInput?: string | null;
}) => Promise<void>;
authorizeActorAction?: (params: {
cfg: OpenClawConfig;
accountId?: string | null;
senderId?: string | null;
action: "approve";
approvalKind: "exec" | "plugin";
}) => {
authorized: boolean;
reason?: string;
};
getActionAvailabilityState?: (params: {
cfg: OpenClawConfig;
accountId?: string | null;
action: "approve";
}) => ChannelActionAvailabilityState;
};
export type ChannelHeartbeatAdapter = {
@ -465,39 +505,13 @@ export type ChannelLifecycleAdapter = {
}) => Promise<void> | void;
};
export type ChannelApprovalAuthAdapter = {
authorizeCommand?: (params: {
cfg: OpenClawConfig;
accountId?: string | null;
senderId?: string | null;
kind: "exec" | "plugin";
}) => {
authorized: boolean;
reason?: string;
};
getInitiatingSurfaceState?: (params: {
cfg: OpenClawConfig;
accountId?: string | null;
}) => ChannelApprovalInitiatingSurfaceState;
};
export type ChannelApprovalDeliveryAdapter = {
shouldSuppressLocalPrompt?: (params: {
cfg: OpenClawConfig;
accountId?: string | null;
payload: ReplyPayload;
}) => boolean;
hasConfiguredDmRoute?: (params: { cfg: OpenClawConfig }) => boolean;
shouldSuppressForwardingFallback?: (params: {
cfg: OpenClawConfig;
target: ChannelApprovalForwardTarget;
request: ExecApprovalRequest;
}) => boolean;
beforeDeliverPending?: (params: {
cfg: OpenClawConfig;
target: ChannelApprovalForwardTarget;
payload: ReplyPayload;
}) => Promise<void> | void;
};
export type ChannelApprovalRenderAdapter = {
@ -530,7 +544,6 @@ export type ChannelApprovalRenderAdapter = {
};
export type ChannelApprovalAdapter = {
auth?: ChannelApprovalAuthAdapter;
delivery?: ChannelApprovalDeliveryAdapter;
render?: ChannelApprovalRenderAdapter;
};

View File

@ -7,6 +7,7 @@ export type ChannelMessageActionName = ChannelMessageActionNameFromList;
export type { ChannelMessageCapability } from "./message-capabilities.js";
export type {
ChannelActionAvailabilityState,
ChannelApprovalAdapter,
ChannelApprovalForwardTarget,
ChannelApprovalInitiatingSurfaceState,
@ -32,6 +33,8 @@ export type {
ChannelLogoutResult,
ChannelOutboundAdapter,
ChannelOutboundContext,
ChannelOutboundPayloadHint,
ChannelOutboundTargetRef,
ChannelAllowlistAdapter,
ChannelCommandConversationContext,
ChannelConfiguredBindingConversationRef,

View File

@ -4,7 +4,6 @@ const getChannelPluginMock = vi.hoisted(() => vi.fn());
vi.mock("../channels/plugins/index.js", () => ({
getChannelPlugin: (...args: unknown[]) => getChannelPluginMock(...args),
resolveChannelApprovalAdapter: (plugin?: { approvals?: unknown } | null) => plugin?.approvals,
}));
import { resolveApprovalCommandAuthorization } from "./channel-approval-auth.js";
@ -27,13 +26,16 @@ describe("resolveApprovalCommandAuthorization", () => {
it("delegates to the channel approval override when present", () => {
getChannelPluginMock.mockReturnValue({
approvals: {
auth: {
authorizeCommand: ({ kind }: { kind: "exec" | "plugin" }) =>
kind === "plugin"
? { authorized: false, reason: "plugin denied" }
: { authorized: true },
},
auth: {
authorizeActorAction: ({
approvalKind,
}: {
action: "approve";
approvalKind: "exec" | "plugin";
}) =>
approvalKind === "plugin"
? { authorized: false, reason: "plugin denied" }
: { authorized: true },
},
});

View File

@ -1,4 +1,4 @@
import { getChannelPlugin, resolveChannelApprovalAdapter } from "../channels/plugins/index.js";
import { getChannelPlugin } from "../channels/plugins/index.js";
import type { OpenClawConfig } from "../config/config.js";
import { normalizeMessageChannel } from "../utils/message-channel.js";
@ -14,11 +14,12 @@ export function resolveApprovalCommandAuthorization(params: {
return { authorized: true };
}
return (
resolveChannelApprovalAdapter(getChannelPlugin(channel))?.auth?.authorizeCommand?.({
getChannelPlugin(channel)?.auth?.authorizeActorAction?.({
cfg: params.cfg,
accountId: params.accountId,
senderId: params.senderId,
kind: params.kind,
action: "approve",
approvalKind: params.kind,
}) ?? { authorized: true }
);
}

View File

@ -338,6 +338,47 @@ describe("exec approval forwarder", () => {
expect(deliver).toHaveBeenCalledTimes(2);
});
it("calls outbound beforeDeliverPayload before exec approval delivery", async () => {
const beforeDeliverPayload = vi.fn();
setActivePluginRegistry(
createTestRegistry([
{
pluginId: "telegram",
plugin: telegramApprovalPlugin,
source: "test",
},
{
pluginId: "discord",
plugin: discordApprovalPlugin,
source: "test",
},
{
pluginId: "slack",
plugin: {
...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }),
outbound: {
deliveryMode: "direct",
beforeDeliverPayload,
},
} satisfies Pick<ChannelPlugin, "id" | "meta" | "capabilities" | "config" | "outbound">,
source: "test",
},
]),
);
const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG });
await expect(forwarder.handleRequested(baseRequest)).resolves.toBe(true);
await vi.waitFor(() => {
expect(deliver).toHaveBeenCalled();
});
expect(beforeDeliverPayload).toHaveBeenCalledWith(
expect.objectContaining({
hint: { kind: "approval-pending", approvalKind: "exec" },
target: expect.objectContaining({ channel: "slack", to: "U123" }),
}),
);
});
it("skips telegram forwarding when telegram exec approvals handler is enabled", async () => {
vi.useFakeTimers();
const cfg = {

View File

@ -559,12 +559,14 @@ function createApprovalHandlers<
if (!channel) {
return;
}
await resolveChannelApprovalAdapter(
getChannelPlugin(channel),
)?.delivery?.beforeDeliverPending?.({
await getChannelPlugin(channel)?.outbound?.beforeDeliverPayload?.({
cfg,
target,
payload,
hint: {
kind: "approval-pending",
approvalKind: params.strategy.kind,
},
});
},
deliver: params.deliver,

View File

@ -34,7 +34,6 @@ async function loadExecApprovalSurfaceModule() {
vi.doMock("../channels/plugins/index.js", () => ({
getChannelPlugin: (...args: unknown[]) => getChannelPluginMock(...args),
listChannelPlugins: (...args: unknown[]) => listChannelPluginsMock(...args),
resolveChannelApprovalAdapter: (plugin?: { approvals?: unknown } | null) => plugin?.approvals,
}));
vi.doMock("../utils/message-channel.js", () => ({
INTERNAL_MESSAGE_CHANNEL: "web",
@ -83,18 +82,14 @@ describe("resolveExecApprovalInitiatingSurfaceState", () => {
getChannelPluginMock.mockImplementation((channel: string) =>
channel === "telegram"
? {
approvals: {
auth: {
getInitiatingSurfaceState: () => ({ kind: "enabled" }),
},
auth: {
getActionAvailabilityState: () => ({ kind: "enabled" }),
},
}
: channel === "discord"
? {
approvals: {
auth: {
getInitiatingSurfaceState: () => ({ kind: "disabled" }),
},
auth: {
getActionAvailabilityState: () => ({ kind: "disabled" }),
},
}
: undefined,
@ -132,10 +127,8 @@ describe("resolveExecApprovalInitiatingSurfaceState", () => {
getChannelPluginMock.mockImplementation((channel: string) =>
channel === "telegram"
? {
approvals: {
auth: {
getInitiatingSurfaceState: () => ({ kind: "disabled" }),
},
auth: {
getActionAvailabilityState: () => ({ kind: "disabled" }),
},
}
: undefined,

View File

@ -1,8 +1,4 @@
import {
getChannelPlugin,
listChannelPlugins,
resolveChannelApprovalAdapter,
} from "../channels/plugins/index.js";
import { getChannelPlugin, listChannelPlugins } from "../channels/plugins/index.js";
import { loadConfig, type OpenClawConfig } from "../config/config.js";
import {
INTERNAL_MESSAGE_CHANNEL,
@ -42,11 +38,10 @@ export function resolveExecApprovalInitiatingSurfaceState(params: {
}
const cfg = params.cfg ?? loadConfig();
const state = resolveChannelApprovalAdapter(
getChannelPlugin(channel),
)?.auth?.getInitiatingSurfaceState?.({
const state = getChannelPlugin(channel)?.auth?.getActionAvailabilityState?.({
cfg,
accountId: params.accountId,
action: "approve",
});
if (state) {
return { ...state, channel, channelLabel };
@ -59,7 +54,6 @@ export function resolveExecApprovalInitiatingSurfaceState(params: {
export function hasConfiguredExecApprovalDmRoute(cfg: OpenClawConfig): boolean {
return listChannelPlugins().some(
(plugin) =>
resolveChannelApprovalAdapter(plugin)?.delivery?.hasConfiguredDmRoute?.({ cfg }) ?? false,
(plugin) => plugin.approvals?.delivery?.hasConfiguredDmRoute?.({ cfg }) ?? false,
);
}

View File

@ -205,17 +205,16 @@ describe("plugin approval forwarding", () => {
expect(deliveryArgs?.payloads?.[0]?.text).toBe("custom adapter payload");
});
it("calls beforeDeliverPending before plugin approval delivery", async () => {
const beforeDeliverPending = vi.fn();
it("calls outbound beforeDeliverPayload before plugin approval delivery", async () => {
const beforeDeliverPayload = vi.fn();
const adapterPlugin: Pick<
ChannelPlugin,
"id" | "meta" | "capabilities" | "config" | "approvals"
"id" | "meta" | "capabilities" | "config" | "outbound"
> = {
...createChannelTestPluginBase({ id: "slack" as ChannelPlugin["id"] }),
approvals: {
delivery: {
beforeDeliverPending,
},
outbound: {
deliveryMode: "direct",
beforeDeliverPayload,
},
};
const registry = createTestRegistry([
@ -229,7 +228,7 @@ describe("plugin approval forwarding", () => {
await vi.waitFor(() => {
expect(deliver).toHaveBeenCalled();
});
expect(beforeDeliverPending).toHaveBeenCalled();
expect(beforeDeliverPayload).toHaveBeenCalled();
});
it("uses buildPluginResolvedPayload from channel adapter for resolved messages", async () => {

View File

@ -13,32 +13,35 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => {
isNativeDeliveryEnabled: () => true,
resolveNativeDeliveryMode: () => "dm",
});
const authorizeCommand = adapter.auth.authorizeCommand;
const authorizeActorAction = adapter.auth.authorizeActorAction;
expect(
authorizeCommand({
authorizeActorAction({
cfg: {} as never,
accountId: "work",
senderId: "exec-owner",
kind: "exec",
action: "approve",
approvalKind: "exec",
}),
).toEqual({ authorized: true });
expect(
authorizeCommand({
authorizeActorAction({
cfg: {} as never,
accountId: "work",
senderId: "plugin-owner",
kind: "plugin",
action: "approve",
approvalKind: "plugin",
}),
).toEqual({ authorized: true });
expect(
authorizeCommand({
authorizeActorAction({
cfg: {} as never,
accountId: "work",
senderId: "someone-else",
kind: "plugin",
action: "approve",
approvalKind: "plugin",
}),
).toEqual({
authorized: false,
@ -57,15 +60,23 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => {
resolveNativeDeliveryMode: ({ accountId }) =>
accountId === "channel-only" ? "channel" : "dm",
});
const getInitiatingSurfaceState = adapter.auth.getInitiatingSurfaceState;
const getActionAvailabilityState = adapter.auth.getActionAvailabilityState;
const hasConfiguredDmRoute = adapter.delivery.hasConfiguredDmRoute;
expect(getInitiatingSurfaceState({ cfg: {} as never, accountId: "dm-only" })).toEqual({
kind: "enabled",
});
expect(getInitiatingSurfaceState({ cfg: {} as never, accountId: "no-approvers" })).toEqual({
kind: "disabled",
});
expect(
getActionAvailabilityState({
cfg: {} as never,
accountId: "dm-only",
action: "approve",
}),
).toEqual({ kind: "enabled" });
expect(
getActionAvailabilityState({
cfg: {} as never,
accountId: "no-approvers",
action: "approve",
}),
).toEqual({ kind: "disabled" });
expect(hasConfiguredDmRoute({ cfg: {} as never })).toBe(true);
});

View File

@ -35,34 +35,36 @@ export function createApproverRestrictedNativeApprovalAdapter(params: {
return {
auth: {
authorizeCommand: ({
authorizeActorAction: ({
cfg,
accountId,
senderId,
kind,
approvalKind,
}: {
cfg: OpenClawConfig;
accountId?: string | null;
senderId?: string | null;
kind: ApprovalKind;
action: "approve";
approvalKind: ApprovalKind;
}) => {
const authorized =
kind === "plugin"
approvalKind === "plugin"
? pluginSenderAuth({ cfg, accountId, senderId })
: params.isExecAuthorizedSender({ cfg, accountId, senderId });
return authorized
? { authorized: true }
: {
authorized: false,
reason: `❌ You are not authorized to approve ${kind} requests on ${params.channelLabel}.`,
reason: `❌ You are not authorized to approve ${approvalKind} requests on ${params.channelLabel}.`,
};
},
getInitiatingSurfaceState: ({
getActionAvailabilityState: ({
cfg,
accountId,
}: {
cfg: OpenClawConfig;
accountId?: string | null;
action: "approve";
}) =>
params.hasApprovers({ cfg, accountId })
? ({ kind: "enabled" } as const)