fix(slack): prevent duplicate draft replies

This commit is contained in:
Peter Steinberger 2026-03-31 21:22:01 +01:00
parent fc169215d7
commit eee37bf836
No known key found for this signature in database
10 changed files with 398 additions and 37 deletions

View File

@ -4,6 +4,10 @@ Docs: https://docs.openclaw.ai
## Unreleased
### Fixes
- Slack: stop retry-driven duplicate replies when draft-finalization edits fail ambiguously, and log configured allowlisted users/channels by readable name instead of raw IDs.
## 2026.3.31-beta.1
### Breaking

View File

@ -4,7 +4,7 @@ import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
import { resolveSlackAccount } from "./accounts.js";
import { buildSlackBlocksFallbackText } from "./blocks-fallback.js";
import { validateSlackBlocksArray } from "./blocks-input.js";
import { createSlackWebClient } from "./client.js";
import { createSlackWebClient, createSlackWriteClient } from "./client.js";
import { resolveSlackMedia } from "./monitor/media.js";
import type { SlackMediaResult } from "./monitor/media.js";
import { sendMessageSlack } from "./send.js";
@ -64,9 +64,11 @@ function normalizeEmoji(raw: string) {
return trimmed.replace(/^:+|:+$/g, "");
}
async function getClient(opts: SlackActionClientOpts = {}) {
async function getClient(opts: SlackActionClientOpts = {}, mode: "read" | "write" = "read") {
const token = resolveToken(opts.token, opts.accountId);
return opts.client ?? createSlackWebClient(token);
return (
opts.client ?? (mode === "write" ? createSlackWriteClient(token) : createSlackWebClient(token))
);
}
async function resolveBotUserId(client: WebClient) {
@ -83,7 +85,7 @@ export async function reactSlackMessage(
emoji: string,
opts: SlackActionClientOpts = {},
) {
const client = await getClient(opts);
const client = await getClient(opts, "write");
await client.reactions.add({
channel: channelId,
timestamp: messageId,
@ -97,7 +99,7 @@ export async function removeSlackReaction(
emoji: string,
opts: SlackActionClientOpts = {},
) {
const client = await getClient(opts);
const client = await getClient(opts, "write");
await client.reactions.remove({
channel: channelId,
timestamp: messageId,
@ -110,7 +112,7 @@ export async function removeOwnSlackReactions(
messageId: string,
opts: SlackActionClientOpts = {},
): Promise<string[]> {
const client = await getClient(opts);
const client = await getClient(opts, "write");
const userId = await resolveBotUserId(client);
const reactions = await listSlackReactions(channelId, messageId, { client });
const toRemove = new Set<string>();
@ -192,7 +194,7 @@ export async function editSlackMessage(
content: string,
opts: SlackActionClientOpts & { blocks?: (Block | KnownBlock)[] } = {},
) {
const client = await getClient(opts);
const client = await getClient(opts, "write");
const blocks = opts.blocks == null ? undefined : validateSlackBlocksArray(opts.blocks);
const trimmedContent = content.trim();
await client.chat.update({
@ -208,7 +210,7 @@ export async function deleteSlackMessage(
messageId: string,
opts: SlackActionClientOpts = {},
) {
const client = await getClient(opts);
const client = await getClient(opts, "write");
await client.chat.delete({
channel: channelId,
ts: messageId,
@ -271,7 +273,7 @@ export async function pinSlackMessage(
messageId: string,
opts: SlackActionClientOpts = {},
) {
const client = await getClient(opts);
const client = await getClient(opts, "write");
await client.pins.add({ channel: channelId, timestamp: messageId });
}
@ -280,7 +282,7 @@ export async function unpinSlackMessage(
messageId: string,
opts: SlackActionClientOpts = {},
) {
const client = await getClient(opts);
const client = await getClient(opts, "write");
await client.pins.remove({ channel: channelId, timestamp: messageId });
}

View File

@ -13,15 +13,24 @@ vi.mock("@slack/web-api", () => {
});
let createSlackWebClient: typeof import("./client.js").createSlackWebClient;
let createSlackWriteClient: typeof import("./client.js").createSlackWriteClient;
let resolveSlackWebClientOptions: typeof import("./client.js").resolveSlackWebClientOptions;
let resolveSlackWriteClientOptions: typeof import("./client.js").resolveSlackWriteClientOptions;
let SLACK_DEFAULT_RETRY_OPTIONS: typeof import("./client.js").SLACK_DEFAULT_RETRY_OPTIONS;
let SLACK_WRITE_RETRY_OPTIONS: typeof import("./client.js").SLACK_WRITE_RETRY_OPTIONS;
let WebClient: ReturnType<typeof vi.fn>;
beforeEach(async () => {
vi.resetModules();
const slackWebApi = await import("@slack/web-api");
({ createSlackWebClient, resolveSlackWebClientOptions, SLACK_DEFAULT_RETRY_OPTIONS } =
await import("./client.js"));
({
createSlackWebClient,
createSlackWriteClient,
resolveSlackWebClientOptions,
resolveSlackWriteClientOptions,
SLACK_DEFAULT_RETRY_OPTIONS,
SLACK_WRITE_RETRY_OPTIONS,
} = await import("./client.js"));
WebClient = slackWebApi.WebClient as unknown as ReturnType<typeof vi.fn>;
});
@ -50,4 +59,22 @@ describe("slack web client config", () => {
}),
);
});
it("applies the write retry config when none is provided", () => {
const options = resolveSlackWriteClientOptions();
expect(options.retryConfig).toEqual(SLACK_WRITE_RETRY_OPTIONS);
});
it("passes no-retry config into the write client by default", () => {
createSlackWriteClient("xoxb-test", { timeout: 4321 });
expect(WebClient).toHaveBeenCalledWith(
"xoxb-test",
expect.objectContaining({
timeout: 4321,
retryConfig: SLACK_WRITE_RETRY_OPTIONS,
}),
);
});
});

View File

@ -8,6 +8,10 @@ export const SLACK_DEFAULT_RETRY_OPTIONS: RetryOptions = {
randomize: true,
};
export const SLACK_WRITE_RETRY_OPTIONS: RetryOptions = {
retries: 0,
};
export function resolveSlackWebClientOptions(options: WebClientOptions = {}): WebClientOptions {
return {
...options,
@ -15,6 +19,17 @@ export function resolveSlackWebClientOptions(options: WebClientOptions = {}): We
};
}
export function resolveSlackWriteClientOptions(options: WebClientOptions = {}): WebClientOptions {
return {
...options,
retryConfig: options.retryConfig ?? SLACK_WRITE_RETRY_OPTIONS,
};
}
export function createSlackWebClient(token: string, options: WebClientOptions = {}) {
return new WebClient(token, resolveSlackWebClientOptions(options));
}
export function createSlackWriteClient(token: string, options: WebClientOptions = {}) {
return new WebClient(token, resolveSlackWriteClientOptions(options));
}

View File

@ -17,7 +17,7 @@ import { createReplyDispatcherWithTyping } from "openclaw/plugin-sdk/reply-runti
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
import { danger, logVerbose, shouldLogVerbose } from "openclaw/plugin-sdk/runtime-env";
import { resolvePinnedMainDmOwnerFromAllowlist } from "openclaw/plugin-sdk/security-runtime";
import { editSlackMessage, reactSlackMessage, removeSlackReaction } from "../../actions.js";
import { reactSlackMessage, removeSlackReaction } from "../../actions.js";
import { createSlackDraftStream } from "../../draft-stream.js";
import { normalizeSlackOutboundText } from "../../format.js";
import { SLACK_TEXT_LIMIT } from "../../limits.js";
@ -37,6 +37,7 @@ import {
readSlackReplyBlocks,
resolveSlackThreadTs,
} from "../replies.js";
import { finalizeSlackPreviewEdit } from "./preview-finalize.js";
import type { PreparedSlackMessage } from "./types.js";
function sleep(ms: number): Promise<void> {
@ -447,17 +448,16 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag
if (canFinalizeViaPreviewEdit) {
draftStream?.stop();
try {
await editSlackMessage(
draftChannelId,
draftMessageId,
normalizeSlackOutboundText(trimmedFinalText),
{
token: ctx.botToken,
accountId: account.accountId,
client: ctx.app.client,
...(slackBlocks?.length ? { blocks: slackBlocks } : {}),
},
);
await finalizeSlackPreviewEdit({
client: ctx.app.client,
token: ctx.botToken,
accountId: account.accountId,
channelId: draftChannelId,
messageId: draftMessageId,
text: normalizeSlackOutboundText(trimmedFinalText),
...(slackBlocks?.length ? { blocks: slackBlocks } : {}),
threadTs: usedReplyThreadTs ?? statusThreadTs,
});
observedReplyDelivery = true;
return;
} catch (err) {

View File

@ -0,0 +1,109 @@
import type { WebClient } from "@slack/web-api";
import { beforeEach, describe, expect, it, vi } from "vitest";
const editSlackMessageMock = vi.fn();
vi.mock("../../actions.js", () => ({
editSlackMessage: (...args: unknown[]) =>
editSlackMessageMock(...(args as Parameters<typeof editSlackMessageMock>)),
}));
let finalizeSlackPreviewEdit: typeof import("./preview-finalize.js").finalizeSlackPreviewEdit;
let __testing: typeof import("./preview-finalize.js").__testing;
function createClient(overrides?: {
historyMessages?: Array<Record<string, unknown>>;
replyMessages?: Array<Record<string, unknown>>;
}) {
return {
conversations: {
history: vi.fn(async () => ({ messages: overrides?.historyMessages ?? [] })),
replies: vi.fn(async () => ({ messages: overrides?.replyMessages ?? [] })),
},
} as unknown as WebClient;
}
describe("finalizeSlackPreviewEdit", () => {
beforeEach(async () => {
vi.resetModules();
editSlackMessageMock.mockReset();
({ finalizeSlackPreviewEdit, __testing } = await import("./preview-finalize.js"));
});
it("treats a thrown edit as success when history readback already matches", async () => {
editSlackMessageMock.mockRejectedValueOnce(new Error("socket closed"));
const client = createClient({
historyMessages: [{ ts: "171234.567", text: "fair. poke harder then 🦞" }],
});
await expect(
finalizeSlackPreviewEdit({
client,
token: "xoxb-test",
channelId: "C123",
messageId: "171234.567",
text: "fair. poke harder then 🦞",
}),
).resolves.toBeUndefined();
expect(client.conversations.history as unknown as ReturnType<typeof vi.fn>).toHaveBeenCalled();
});
it("checks threaded replies via conversations.replies", async () => {
editSlackMessageMock.mockRejectedValueOnce(new Error("socket closed"));
const client = createClient({
replyMessages: [{ ts: "171234.567", text: "done" }],
});
await expect(
finalizeSlackPreviewEdit({
client,
token: "xoxb-test",
channelId: "C123",
messageId: "171234.567",
threadTs: "170000.111",
text: "done",
}),
).resolves.toBeUndefined();
expect(
client.conversations.replies as unknown as ReturnType<typeof vi.fn>,
).toHaveBeenCalledWith(
expect.objectContaining({
channel: "C123",
ts: "170000.111",
latest: "171234.567",
}),
);
});
it("rethrows when readback does not match the expected final text", async () => {
editSlackMessageMock.mockRejectedValueOnce(new Error("socket closed"));
const client = createClient({
historyMessages: [{ ts: "171234.567", text: "partial draft" }],
});
await expect(
finalizeSlackPreviewEdit({
client,
token: "xoxb-test",
channelId: "C123",
messageId: "171234.567",
text: "final answer",
}),
).rejects.toThrow("socket closed");
});
it("requires matching blocks when finalizing a blocks-only edit", async () => {
const blocks = [{ type: "section", text: { type: "mrkdwn", text: "*Done*" } }] as const;
expect(
__testing.buildExpectedSlackEditText({
text: "",
blocks: blocks as unknown as Parameters<
typeof __testing.buildExpectedSlackEditText
>[0]["blocks"],
}),
).toBe("*Done*");
});
});

View File

@ -0,0 +1,145 @@
import type { Block, KnownBlock, WebClient } from "@slack/web-api";
import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
import { editSlackMessage } from "../../actions.js";
import { buildSlackBlocksFallbackText } from "../../blocks-fallback.js";
import { normalizeSlackOutboundText } from "../../format.js";
type SlackReadbackMessage = {
ts?: string;
text?: string;
blocks?: unknown[];
};
function buildExpectedSlackEditText(params: {
text: string;
blocks?: (Block | KnownBlock)[];
}): string {
const trimmed = normalizeSlackOutboundText(params.text.trim());
if (trimmed) {
return trimmed;
}
if (params.blocks?.length) {
return buildSlackBlocksFallbackText(params.blocks);
}
return " ";
}
function blocksMatch(expected?: (Block | KnownBlock)[], actual?: unknown[]): boolean {
if (!expected?.length) {
return !actual?.length;
}
if (!actual?.length) {
return false;
}
return JSON.stringify(expected) === JSON.stringify(actual);
}
async function readSlackMessageAfterEditError(params: {
client: WebClient;
token: string;
channelId: string;
messageId: string;
threadTs?: string;
}): Promise<SlackReadbackMessage | null> {
if (params.threadTs) {
const replyResult = await params.client.conversations.replies({
token: params.token,
channel: params.channelId,
ts: params.threadTs,
latest: params.messageId,
inclusive: true,
limit: 100,
});
const reply = (replyResult.messages ?? []).find(
(message) => (message as SlackReadbackMessage | undefined)?.ts === params.messageId,
) as SlackReadbackMessage | undefined;
return reply ?? null;
}
const historyResult = await params.client.conversations.history({
token: params.token,
channel: params.channelId,
latest: params.messageId,
oldest: params.messageId,
inclusive: true,
limit: 1,
});
const message = historyResult.messages?.[0] as SlackReadbackMessage | undefined;
if (!message?.ts || message.ts !== params.messageId) {
return null;
}
return message;
}
async function didSlackPreviewEditApplyAfterError(params: {
client: WebClient;
token: string;
channelId: string;
messageId: string;
text: string;
blocks?: (Block | KnownBlock)[];
threadTs?: string;
}): Promise<boolean> {
const readback = await readSlackMessageAfterEditError(params);
if (!readback) {
return false;
}
const expectedText = buildExpectedSlackEditText({
text: params.text,
blocks: params.blocks,
});
const actualText = normalizeSlackOutboundText((readback.text ?? "").trim());
if (params.blocks?.length) {
return actualText === expectedText && blocksMatch(params.blocks, readback.blocks);
}
return actualText === expectedText;
}
export async function finalizeSlackPreviewEdit(params: {
client: WebClient;
token: string;
accountId?: string;
channelId: string;
messageId: string;
text: string;
blocks?: (Block | KnownBlock)[];
threadTs?: string;
}): Promise<void> {
try {
await editSlackMessage(params.channelId, params.messageId, params.text, {
token: params.token,
accountId: params.accountId,
client: params.client,
...(params.blocks?.length ? { blocks: params.blocks } : {}),
});
return;
} catch (err) {
try {
const applied = await didSlackPreviewEditApplyAfterError({
client: params.client,
token: params.token,
channelId: params.channelId,
messageId: params.messageId,
text: params.text,
blocks: params.blocks,
threadTs: params.threadTs,
});
if (applied) {
logVerbose(
`slack: preview final edit response failed but readback matched message ${params.channelId}/${params.messageId}; suppressing duplicate fallback send`,
);
return;
}
} catch (readbackErr) {
logVerbose(`slack: preview final edit readback failed (${String(readbackErr)})`);
}
throw err;
}
}
export const __testing = {
buildExpectedSlackEditText,
blocksMatch,
didSlackPreviewEditApplyAfterError,
readSlackMessageAfterEditError,
};

View File

@ -0,0 +1,26 @@
import { describe, expect, it } from "vitest";
import { __testing } from "./provider.js";
describe("slack allowlist log formatting", () => {
it("prints channel names alongside ids", () => {
expect(
__testing.formatSlackChannelResolved({
input: "C0AQXEG6QFJ",
resolved: true,
id: "C0AQXEG6QFJ",
name: "openclawtest",
}),
).toBe("C0AQXEG6QFJ→openclawtest (id:C0AQXEG6QFJ)");
});
it("prints user names alongside ids", () => {
expect(
__testing.formatSlackUserResolved({
input: "U090HHQ029J",
resolved: true,
id: "U090HHQ029J",
name: "steipete",
}),
).toBe("U090HHQ029J→steipete (id:U090HHQ029J)");
});
});

View File

@ -33,8 +33,8 @@ import { resolveSlackAccount } from "../accounts.js";
import { resolveSlackWebClientOptions } from "../client.js";
import { normalizeSlackWebhookPath, registerSlackHttpHandler } from "../http/index.js";
import { SLACK_TEXT_LIMIT } from "../limits.js";
import { resolveSlackChannelAllowlist } from "../resolve-channels.js";
import { resolveSlackUserAllowlist } from "../resolve-users.js";
import { resolveSlackChannelAllowlist, type SlackChannelResolution } from "../resolve-channels.js";
import { resolveSlackUserAllowlist, type SlackUserResolution } from "../resolve-users.js";
import { resolveSlackAppToken, resolveSlackBotToken } from "../token.js";
import { normalizeAllowList } from "./allow-list.js";
import { resolveSlackSlashCommandConfig } from "./commands.js";
@ -169,6 +169,38 @@ function publishSlackDisconnectedStatus(
});
}
function formatSlackResolvedLabel(params: {
input: string;
id: string;
name?: string;
extra?: string[];
}): string {
const extras = params.extra?.filter(Boolean) ?? [];
const suffix =
extras.length > 0 ? ` (id:${params.id}, ${extras.join(", ")})` : ` (id:${params.id})`;
return `${params.input}${params.name ?? params.id}${suffix}`;
}
function formatSlackChannelResolved(entry: SlackChannelResolution): string {
const id = entry.id ?? entry.input;
return formatSlackResolvedLabel({
input: entry.input,
id,
name: entry.name,
extra: entry.archived ? ["archived"] : [],
});
}
function formatSlackUserResolved(entry: SlackUserResolution): string {
const id = entry.id ?? entry.input;
return formatSlackResolvedLabel({
input: entry.input,
id,
name: entry.name,
extra: entry.note ? [entry.note] : [],
});
}
export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
const cfg = opts.config ?? loadConfig();
const runtime: RuntimeEnv = opts.runtime ?? createNonExitingRuntime();
@ -411,7 +443,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
unresolved.push(entry.input);
continue;
}
mapping.push(`${entry.input}${entry.id}${entry.archived ? " (archived)" : ""}`);
mapping.push(formatSlackChannelResolved(entry));
const existing = nextChannels[entry.id] ?? {};
nextChannels[entry.id] = { ...source, ...existing };
}
@ -434,12 +466,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
const { mapping, unresolved, additions } = buildAllowlistResolutionSummary(
resolvedUsers,
{
formatResolved: (entry) => {
const note = (entry as { note?: string }).note
? ` (${(entry as { note?: string }).note})`
: "";
return `${entry.input}${entry.id}${note}`;
},
formatResolved: formatSlackUserResolved,
},
);
allowFrom = mergeAllowlist({ existing: allowFrom, additions });
@ -462,8 +489,12 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
token: resolveToken,
entries: Array.from(userEntries),
});
const { resolvedMap, mapping, unresolved } =
buildAllowlistResolutionSummary(resolvedUsers);
const { resolvedMap, mapping, unresolved } = buildAllowlistResolutionSummary(
resolvedUsers,
{
formatResolved: formatSlackUserResolved,
},
);
const nextChannels = patchAllowlistUsersInConfigEntries({
entries: channelsConfig,
@ -591,6 +622,8 @@ export { isNonRecoverableSlackAuthError } from "./reconnect-policy.js";
export const resolveSlackRuntimeGroupPolicy = resolveOpenProviderRuntimeGroupPolicy;
export const __testing = {
formatSlackChannelResolved,
formatSlackUserResolved,
publishSlackConnectedStatus,
publishSlackDisconnectedStatus,
resolveSlackRuntimeGroupPolicy: resolveOpenProviderRuntimeGroupPolicy,

View File

@ -15,7 +15,7 @@ import type { SlackTokenSource } from "./accounts.js";
import { resolveSlackAccount } from "./accounts.js";
import { buildSlackBlocksFallbackText } from "./blocks-fallback.js";
import { validateSlackBlocksArray } from "./blocks-input.js";
import { createSlackWebClient } from "./client.js";
import { createSlackWriteClient } from "./client.js";
import { markdownToSlackMrkdwnChunks } from "./format.js";
import { SLACK_TEXT_LIMIT } from "./limits.js";
import { loadOutboundMediaFromUrl } from "./runtime-api.js";
@ -327,7 +327,7 @@ export async function sendMessageSlack(
fallbackToken: account.botToken,
fallbackSource: account.botTokenSource,
});
const client = opts.client ?? createSlackWebClient(token);
const client = opts.client ?? createSlackWriteClient(token);
const recipient = parseRecipient(to);
const { channelId } = await resolveChannelId(client, recipient, {
accountId: account.accountId,