refactor(slack): mark approval headers with typed block ids (#124841)

This commit is contained in:
Peter Steinberger
2026-08-16 14:53:26 -07:00
committed by GitHub
parent 080887df92
commit eab2b8fdca
11 changed files with 32 additions and 89 deletions
+2 -2
View File
@@ -3,6 +3,7 @@ import { createApproverRestrictedNativeApprovalCapabilityFromForwardingRoutes }
import { createLazyChannelApprovalNativeRuntimeAdapter } from "openclaw/plugin-sdk/approval-handler-adapter-runtime";
import type { ChannelApprovalNativeRuntimeAdapter } from "openclaw/plugin-sdk/approval-handler-runtime";
import { shouldSuppressLocalNativeExecApprovalPrompt } from "openclaw/plugin-sdk/approval-native-runtime";
import { addApprovalReactionHintToText } from "openclaw/plugin-sdk/approval-reaction-runtime";
import {
buildTypedExecApprovalPendingReplyPayload,
buildTypedPluginApprovalPendingReplyPayload,
@@ -34,7 +35,6 @@ import {
resolveIMessageAccount,
} from "./accounts.js";
import { getIMessageApprovalApprovers, imessageApprovalAuth } from "./approval-auth.js";
import { addIMessageApprovalReactionHintToText } from "./approval-reactions.js";
import { replaceApprovalIdPlaceholder } from "./approval-text.js";
import { normalizeIMessageMessagingTarget } from "./normalize.js";
import { inferIMessageTargetChatType } from "./targets.js";
@@ -204,7 +204,7 @@ function appendIMessageReactionHint(params: {
text?: string;
allowedDecisions: readonly ExecApprovalReplyDecision[];
}): string {
return addIMessageApprovalReactionHintToText({
return addApprovalReactionHintToText({
text: params.text ?? "",
allowedDecisions: params.allowedDecisions,
});
@@ -1,4 +1,5 @@
// Imessage tests cover approval reactions plugin behavior.
import { buildApprovalReactionHint } from "openclaw/plugin-sdk/approval-reaction-runtime";
import { buildTypedExecApprovalPendingReplyPayload } from "openclaw/plugin-sdk/approval-reply-runtime";
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
import { beforeEach, describe, expect, it, vi } from "vitest";
@@ -6,7 +7,6 @@ import { listPendingIMessageApprovalReactionPollTargets } from "./approval-react
import {
addIMessageApprovalReactionHintToStructuredPayload,
buildIMessageApprovalConversationKeyForTarget,
buildIMessageApprovalReactionHint,
clearIMessageApprovalReactionTargetsForTest,
handleIMessageApprovalReaction,
maybeResolveIMessageApprovalReaction,
@@ -78,9 +78,9 @@ describe("iMessage approval reactions", () => {
});
it("renders shared reaction choices for allowed decisions", () => {
expect(buildIMessageApprovalReactionHint(["allow-once", "allow-always", "deny"])).toBe(
"React with:\n\n👍 Allow Once\n♾️ Allow Always\n👎 Deny",
);
expect(
buildApprovalReactionHint({ allowedDecisions: ["allow-once", "allow-always", "deny"] }),
).toBe("React with:\n\n👍 Allow Once\n♾️ Allow Always\n👎 Deny");
});
it("uses typed metadata to prepare shared forwarded prompts", () => {
+2 -15
View File
@@ -137,19 +137,6 @@ function listIMessageApprovalReactionBindings(
return listApprovalReactionBindings({ allowedDecisions });
}
export function buildIMessageApprovalReactionHint(
allowedDecisions: readonly ExecApprovalReplyDecision[],
): string | null {
return buildApprovalReactionHint({ allowedDecisions });
}
export function addIMessageApprovalReactionHintToText(params: {
text: string;
allowedDecisions: readonly ExecApprovalReplyDecision[];
}): string {
return addApprovalReactionHintToText(params);
}
type IMessageApprovalDeliveryBinding = ApprovalReactionDeliveryBinding & {
approvalSlug: string;
};
@@ -205,7 +192,7 @@ function visibleApprovalBindingMatches(
if (!options.requireReactionHint) {
return true;
}
const hint = buildIMessageApprovalReactionHint(binding.allowedDecisions);
const hint = buildApprovalReactionHint({ allowedDecisions: binding.allowedDecisions });
return Boolean(hint && text.includes(hint));
}
@@ -229,7 +216,7 @@ export function addIMessageApprovalReactionHintToStructuredPayload(params: {
}
return {
...params.payload,
text: addIMessageApprovalReactionHintToText({
text: addApprovalReactionHintToText({
text,
allowedDecisions: metadata.allowedDecisions,
}),
+2 -2
View File
@@ -1,6 +1,7 @@
// Imessage plugin module implements send behavior.
import { constants, accessSync } from "node:fs";
import { basename } from "node:path";
import { addApprovalReactionHintToText } from "openclaw/plugin-sdk/approval-reaction-runtime";
import type { ExecApprovalReplyDecision } from "openclaw/plugin-sdk/approval-reply-runtime";
import {
createChannelPartialDeliveryError,
@@ -39,7 +40,6 @@ import {
type ResolvedIMessageAccount,
} from "./accounts.js";
import {
addIMessageApprovalReactionHintToText,
type IMessageApprovalConversationKey,
registerIMessageApprovalReactionTarget,
} from "./approval-reactions.js";
@@ -882,7 +882,7 @@ export async function sendMessageIMessage(
? account.config.mediaMaxMb * 1024 * 1024
: 16 * 1024 * 1024;
let message = opts.approvalPrompt
? addIMessageApprovalReactionHintToText({
? addApprovalReactionHintToText({
text,
allowedDecisions: opts.approvalPrompt.allowedDecisions,
})
@@ -1,3 +1,4 @@
import { addApprovalReactionHintToText } from "openclaw/plugin-sdk/approval-reaction-runtime";
import {
buildExecApprovalPendingReplyPayload,
buildPluginApprovalPendingReplyPayload,
@@ -5,9 +6,7 @@ import {
// Signal tests cover approval reactions plugin behavior.
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
addSignalApprovalReactionHintToText,
addSignalApprovalReactionHintToStructuredPayload,
buildSignalApprovalReactionHint,
clearSignalApprovalReactionTargetsForTest,
maybeResolveSignalApprovalReaction,
registerSignalApprovalReactionTargetForDeliveredPayload,
@@ -45,48 +44,6 @@ describe("Signal approval reactions", () => {
resolverMocks.isApprovalNotFoundError.mockReturnValue(false);
});
it("renders thumbs-only reaction choices for allowed decisions", () => {
expect(buildSignalApprovalReactionHint(["allow-once", "deny"])).toBe(
"React with:\n\n👍 Allow Once\n👎 Deny",
);
});
it("exposes allow-always as a reaction choice when allowed", () => {
expect(buildSignalApprovalReactionHint(["allow-once", "allow-always", "deny"])).toBe(
"React with:\n\n👍 Allow Once\n♾️ Allow Always\n👎 Deny",
);
});
it("appends thumbs-only reaction choices to outbound approval prompts", () => {
expect(
addSignalApprovalReactionHintToText({
text: "Exec approval required\nID: exec-1\n\nReply with: /approve exec-1 allow-once|deny",
allowedDecisions: ["allow-once", "deny"],
}),
).toBe(
"Exec approval required\nID: exec-1\n\nReact with:\n\n👍 Allow Once\n👎 Deny\n\nReply with: /approve exec-1 allow-once|deny",
);
});
it("does not duplicate reaction choices on native approval prompts", () => {
const prompt = [
"Plugin approval required",
"Reply with: /approve plugin:abc allow-once|allow-always|deny",
"",
"React with:",
"",
"👍 Allow Once",
"👎 Deny",
].join("\n");
expect(
addSignalApprovalReactionHintToText({
text: prompt,
allowedDecisions: ["allow-once", "deny"],
}),
).toBe(prompt);
});
it("registers delivered structured approval payloads for reactions", async () => {
const cfg = {
channels: {
@@ -472,7 +429,7 @@ describe("Signal approval reactions", () => {
});
const deliveredPayload = {
...payload,
text: addSignalApprovalReactionHintToText({
text: addApprovalReactionHintToText({
text: payload.text ?? "",
allowedDecisions: ["allow-once", "deny"],
}),
+1 -15
View File
@@ -3,7 +3,6 @@ import { matchesApprovalRequestFilters } from "openclaw/plugin-sdk/approval-clie
import type { ApprovalResolveResult } from "openclaw/plugin-sdk/approval-gateway-runtime";
import {
addApprovalReactionHintToText,
buildApprovalReactionHint,
createApprovalReactionTargetStore,
hasApprovalReactionHintText,
listApprovalReactionBindings,
@@ -355,19 +354,6 @@ function listSignalApprovalReactionBindings(
return listApprovalReactionBindings({ allowedDecisions });
}
export function buildSignalApprovalReactionHint(
allowedDecisions: readonly ExecApprovalReplyDecision[],
): string | null {
return buildApprovalReactionHint({ allowedDecisions });
}
export function addSignalApprovalReactionHintToText(params: {
text: string;
allowedDecisions: readonly ExecApprovalReplyDecision[];
}): string {
return addApprovalReactionHintToText(params);
}
function buildTargetRoute(params: {
cfg: OpenClawConfig;
accountId?: string | null;
@@ -524,7 +510,7 @@ export function addSignalApprovalReactionHintToStructuredPayload(params: {
}
return {
...params.payload,
text: addSignalApprovalReactionHintToText({
text: addApprovalReactionHintToText({
text: params.payload.text,
allowedDecisions: metadata.allowedDecisions,
}),
+1
View File
@@ -4,6 +4,7 @@ import type { MessagePresentationAction } from "openclaw/plugin-sdk/interactive-
import { SLACK_BUTTON_VALUE_MAX } from "./presentation.js";
const SLACK_APPROVAL_VALUE_PREFIX = "openclaw:approval:v1:";
export const SLACK_APPROVAL_HEADER_BLOCK_ID = "openclaw_approval_header";
export type SlackApprovalAction = Extract<MessagePresentationAction, { type: "approval" }>;
@@ -350,6 +350,9 @@ describe("slackApprovalNativeRuntime", () => {
});
expect(payload.text).toContain("*Exec approval required*");
expect((payload.blocks as Array<{ block_id?: string }>)[0]?.block_id).toBe(
"openclaw_approval_header",
);
const actionsBlock = findSlackActionsBlock(
payload.blocks as Array<{ type?: string; elements?: unknown[] }>,
);
@@ -376,6 +379,9 @@ describe("slackApprovalNativeRuntime", () => {
});
expect(payload.text).toContain("*Plugin approval required*");
expect((payload.blocks as Array<{ block_id?: string }>)[0]?.block_id).toBe(
"openclaw_approval_header",
);
expect(payload.text).toContain("Share screen with Computer Use");
expect(payload.text).toContain("*Approval ID:* plugin:req-1");
expect(payload.text).not.toContain("*Command*");
@@ -20,6 +20,7 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
import { logError } from "openclaw/plugin-sdk/logging-core";
import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime";
import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime";
import { SLACK_APPROVAL_HEADER_BLOCK_ID } from "./approval-actions.js";
import {
isSlackAnyNativeApprovalClientEnabled,
shouldHandleSlackNativeApprovalRequest,
@@ -228,6 +229,7 @@ function buildSlackExecPendingApprovalBlocks(view: ExecApprovalPendingView): Sla
return [
{
type: "section",
block_id: SLACK_APPROVAL_HEADER_BLOCK_ID,
text: {
type: "mrkdwn",
text: "*Exec approval required*\nA command needs your approval.",
@@ -254,6 +256,7 @@ function buildSlackPluginPendingApprovalBlocks(view: PluginApprovalPendingView):
return [
{
type: "section",
block_id: SLACK_APPROVAL_HEADER_BLOCK_ID,
text: {
type: "mrkdwn",
text: `*Plugin approval required*\n${truncateSlackMrkdwn(
@@ -16,7 +16,11 @@ import {
normalizeUniqueTrimmedStringList,
} from "openclaw/plugin-sdk/string-coerce-runtime";
import { enqueueRoutedSystemEvent } from "openclaw/plugin-sdk/system-event-runtime";
import { decodeSlackApprovalAction, type SlackApprovalAction } from "../../approval-actions.js";
import {
decodeSlackApprovalAction,
SLACK_APPROVAL_HEADER_BLOCK_ID,
type SlackApprovalAction,
} from "../../approval-actions.js";
import { isSlackApprovalAuthorizedSender } from "../../approval-auth.js";
import { isSlackExecApprovalAuthorizedSender } from "../../exec-approvals.js";
import { dispatchSlackPluginInteractiveHandler } from "../../interactive-dispatch.js";
@@ -537,11 +541,9 @@ function buildSlackApprovalTerminalBlocks(params: {
prefix: "Resolved" | "Already resolved";
}): (Block | KnownBlock)[] {
const blocks = removeSlackApprovalControls(params.blocks ?? []).filter((block) => {
const text = (block as { type?: unknown; text?: { text?: unknown } }).text?.text;
const blockId = (block as { block_id?: unknown }).block_id;
return !(
(block as { type?: unknown }).type === "section" &&
typeof text === "string" &&
/^\*(?:Exec|Plugin) approval required\*/u.test(text)
(block as { type?: unknown }).type === "section" && blockId === SLACK_APPROVAL_HEADER_BLOCK_ID
);
});
return [
@@ -1665,7 +1665,8 @@ describe("registerSlackInteractionEvents", () => {
blocks: [
{
type: "section",
text: { type: "mrkdwn", text: "*Exec approval required*\nA command needs approval." },
block_id: "openclaw_approval_header",
text: { type: "mrkdwn", text: "Approval copy can change independently." },
},
{ type: "section", text: { type: "mrkdwn", text: "Command preview" } },
{