From dbe7da768423c2cdadad88dce003c592919b7b8a Mon Sep 17 00:00:00 2001 From: theo674 Date: Mon, 23 Mar 2026 17:11:19 -0400 Subject: [PATCH] fix: prevent delivery-mirror re-delivery and raise Slack chunk limit (#45489) Merged via squash. Prepared head SHA: c7664c7b6e026b0d197fc9b53065d13bef05176a Co-authored-by: theo674 <261068216+theo674@users.noreply.github.com> Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com> Reviewed-by: @altaywtf --- CHANGELOG.md | 2 + extensions/slack/src/channel.test.ts | 5 +++ extensions/slack/src/channel.ts | 3 +- extensions/slack/src/draft-stream.test.ts | 18 +++++++++ extensions/slack/src/draft-stream.ts | 4 +- extensions/slack/src/limits.ts | 1 + .../src/monitor/message-handler/dispatch.ts | 3 +- extensions/slack/src/monitor/provider.ts | 5 ++- extensions/slack/src/monitor/replies.test.ts | 21 ++++++++++ extensions/slack/src/monitor/replies.ts | 3 +- extensions/slack/src/outbound-adapter.ts | 3 +- extensions/slack/src/send.blocks.test.ts | 20 ++++++++++ extensions/slack/src/send.ts | 7 ++-- ...pi-embedded-subscribe.handlers.messages.ts | 15 ++++++-- ...age-end-block-replies-message-tool.test.ts | 38 ++++++++++++++++++- src/config/sessions/sessions.test.ts | 37 ++++++++++++++++++ src/infra/provider-usage.auth.plugin.test.ts | 7 ++-- 17 files changed, 174 insertions(+), 18 deletions(-) create mode 100644 extensions/slack/src/limits.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6576cc50343..ab787570ca2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -371,6 +371,8 @@ Docs: https://docs.openclaw.ai - Memory/core tools: register `memory_search` and `memory_get` independently so one unavailable memory tool no longer suppresses the other in new sessions. (#50198) Thanks @artwalker. - Telegram/Mattermost message tool: keep plugin button schemas optional in isolated and cron sessions so plain sends do not fail validation when no current channel is active. (#52589) Thanks @tylerliu612. - Release/npm publish: fail the npm release check when `dist/control-ui/index.html` is missing from the packed tarball, so broken Control UI asset releases are blocked before publish. Fixes #52808. (#52852) Thanks @kevinheinrichs. +- Slack/embedded delivery: suppress transcript-only `delivery-mirror` assistant messages before embedded re-delivery and raise the default Slack chunk fallback so messages just over 4000 characters stay in a single post. (#45489) Thanks @theo674. +- Slack/embedded delivery: suppress transcript-only `delivery-mirror` assistant messages before embedded re-delivery and raise the default Slack chunk fallback so messages just over 4000 characters stay in a single post. (#45489) Thanks @theo674. ## 2026.3.13 diff --git a/extensions/slack/src/channel.test.ts b/extensions/slack/src/channel.test.ts index a8c519ed7f6..86bb79d3fed 100644 --- a/extensions/slack/src/channel.test.ts +++ b/extensions/slack/src/channel.test.ts @@ -192,6 +192,11 @@ describe("slackPlugin outbound", () => { }, }; + it("advertises the 8000-character Slack default chunk limit", () => { + expect(slackOutbound.textChunkLimit).toBe(8000); + expect(slackPlugin.outbound?.textChunkLimit).toBe(8000); + }); + it("uses threadId as threadTs fallback for sendText", async () => { const sendSlack = vi.fn().mockResolvedValue({ messageId: "m-text" }); const sendText = requireSlackSendText(); diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index c166bd48a1f..a2b27fc5ccf 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -46,6 +46,7 @@ import { } from "./directory-config.js"; import { resolveSlackGroupRequireMention, resolveSlackGroupToolPolicy } from "./group-policy.js"; import { isSlackInteractiveRepliesEnabled } from "./interactive-replies.js"; +import { SLACK_TEXT_LIMIT } from "./limits.js"; import { normalizeAllowListLower } from "./monitor/allow-list.js"; import type { SlackProbe } from "./probe.js"; import { resolveSlackUserAllowlist } from "./resolve-users.js"; @@ -602,7 +603,7 @@ export const slackPlugin: ChannelPlugin = crea base: { deliveryMode: "direct", chunker: null, - textChunkLimit: 4000, + textChunkLimit: SLACK_TEXT_LIMIT, }, attachedResults: { channel: "slack", diff --git a/extensions/slack/src/draft-stream.test.ts b/extensions/slack/src/draft-stream.test.ts index 6103ecb07e5..9f41ab03294 100644 --- a/extensions/slack/src/draft-stream.test.ts +++ b/extensions/slack/src/draft-stream.test.ts @@ -98,6 +98,24 @@ describe("createSlackDraftStream", () => { expect(warn).toHaveBeenCalledTimes(1); }); + it("allows a 4205-character preview with the default max chars", async () => { + const { stream, send, warn } = createDraftStreamHarness(); + const text = "a".repeat(4205); + + stream.update(text); + await stream.flush(); + + expect(send).toHaveBeenCalledTimes(1); + expect(send).toHaveBeenCalledWith( + "channel:C123", + text, + expect.objectContaining({ + token: "xoxb-test", + }), + ); + expect(warn).not.toHaveBeenCalled(); + }); + it("clear removes preview message when one exists", async () => { const { stream, remove } = createDraftStreamHarness(); diff --git a/extensions/slack/src/draft-stream.ts b/extensions/slack/src/draft-stream.ts index c4840b938fe..14962982665 100644 --- a/extensions/slack/src/draft-stream.ts +++ b/extensions/slack/src/draft-stream.ts @@ -1,8 +1,8 @@ import { createDraftStreamLoop } from "openclaw/plugin-sdk/channel-lifecycle"; import { deleteSlackMessage, editSlackMessage } from "./actions.js"; +import { SLACK_TEXT_LIMIT } from "./limits.js"; import { sendMessageSlack } from "./send.js"; -const SLACK_STREAM_MAX_CHARS = 4000; const DEFAULT_THROTTLE_MS = 1000; export type SlackDraftStream = { @@ -29,7 +29,7 @@ export function createSlackDraftStream(params: { edit?: typeof editSlackMessage; remove?: typeof deleteSlackMessage; }): SlackDraftStream { - const maxChars = Math.min(params.maxChars ?? SLACK_STREAM_MAX_CHARS, SLACK_STREAM_MAX_CHARS); + const maxChars = Math.min(params.maxChars ?? SLACK_TEXT_LIMIT, SLACK_TEXT_LIMIT); const throttleMs = Math.max(250, params.throttleMs ?? DEFAULT_THROTTLE_MS); const send = params.send ?? sendMessageSlack; const edit = params.edit ?? editSlackMessage; diff --git a/extensions/slack/src/limits.ts b/extensions/slack/src/limits.ts new file mode 100644 index 00000000000..3c37921b65c --- /dev/null +++ b/extensions/slack/src/limits.ts @@ -0,0 +1 @@ +export const SLACK_TEXT_LIMIT = 8000; diff --git a/extensions/slack/src/monitor/message-handler/dispatch.ts b/extensions/slack/src/monitor/message-handler/dispatch.ts index 4610a2e0b48..f192b592b59 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch.ts @@ -17,6 +17,7 @@ import { resolvePinnedMainDmOwnerFromAllowlist } from "openclaw/plugin-sdk/secur import { editSlackMessage, reactSlackMessage, removeSlackReaction } from "../../actions.js"; import { createSlackDraftStream } from "../../draft-stream.js"; import { normalizeSlackOutboundText } from "../../format.js"; +import { SLACK_TEXT_LIMIT } from "../../limits.js"; import { recordSlackThreadParticipation } from "../../sent-thread-cache.js"; import { applyAppendOnlyStreamUpdate, @@ -375,7 +376,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag target: prepared.replyTarget, token: ctx.botToken, accountId: account.accountId, - maxChars: Math.min(ctx.textLimit, 4000), + maxChars: Math.min(ctx.textLimit, SLACK_TEXT_LIMIT), resolveThreadTs: () => { const ts = replyPlan.nextThreadTs(); if (ts) { diff --git a/extensions/slack/src/monitor/provider.ts b/extensions/slack/src/monitor/provider.ts index 1af83676e93..4bd98327029 100644 --- a/extensions/slack/src/monitor/provider.ts +++ b/extensions/slack/src/monitor/provider.ts @@ -28,6 +28,7 @@ import { normalizeStringEntries } from "openclaw/plugin-sdk/text-runtime"; 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 { resolveSlackAppToken, resolveSlackBotToken } from "../token.js"; @@ -242,7 +243,9 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { const threadHistoryScope = slackCfg.thread?.historyScope ?? "thread"; const threadInheritParent = slackCfg.thread?.inheritParent ?? false; const slashCommand = resolveSlackSlashCommandConfig(opts.slashCommand ?? slackCfg.slashCommand); - const textLimit = resolveTextChunkLimit(cfg, "slack", account.accountId); + const textLimit = resolveTextChunkLimit(cfg, "slack", account.accountId, { + fallbackLimit: SLACK_TEXT_LIMIT, + }); const ackReactionScope = cfg.messages?.ackReactionScope ?? "group-mentions"; const typingReaction = slackCfg.typingReaction?.trim() ?? ""; const mediaMaxBytes = (opts.mediaMaxMb ?? slackCfg.mediaMaxMb ?? 20) * 1024 * 1024; diff --git a/extensions/slack/src/monitor/replies.test.ts b/extensions/slack/src/monitor/replies.test.ts index 18f50fc3748..1f511933216 100644 --- a/extensions/slack/src/monitor/replies.test.ts +++ b/extensions/slack/src/monitor/replies.test.ts @@ -6,6 +6,7 @@ vi.mock("../send.js", () => ({ })); let deliverReplies: typeof import("./replies.js").deliverReplies; +import { deliverSlackSlashReplies } from "./replies.js"; function baseParams(overrides?: Record) { return { @@ -97,3 +98,23 @@ describe("deliverReplies identity passthrough", () => { ); }); }); + +describe("deliverSlackSlashReplies chunking", () => { + it("keeps a 4205-character reply in a single slash response by default", async () => { + const respond = vi.fn(async () => undefined); + const text = "a".repeat(4205); + + await deliverSlackSlashReplies({ + replies: [{ text }], + respond, + ephemeral: true, + textLimit: 8000, + }); + + expect(respond).toHaveBeenCalledTimes(1); + expect(respond).toHaveBeenCalledWith({ + text, + response_type: "ephemeral", + }); + }); +}); diff --git a/extensions/slack/src/monitor/replies.ts b/extensions/slack/src/monitor/replies.ts index f25e58673ca..507a229136f 100644 --- a/extensions/slack/src/monitor/replies.ts +++ b/extensions/slack/src/monitor/replies.ts @@ -11,6 +11,7 @@ import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime"; import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env"; import { parseSlackBlocksInput } from "../blocks-input.js"; import { markdownToSlackMrkdwnChunks } from "../format.js"; +import { SLACK_TEXT_LIMIT } from "../limits.js"; import { sendMessageSlack, type SlackSendIdentity } from "../send.js"; export function readSlackReplyBlocks(payload: ReplyPayload) { @@ -188,7 +189,7 @@ export async function deliverSlackSlashReplies(params: { chunkMode?: ChunkMode; }) { const messages: string[] = []; - const chunkLimit = Math.min(params.textLimit, 4000); + const chunkLimit = Math.min(params.textLimit, SLACK_TEXT_LIMIT); for (const payload of params.replies) { const reply = resolveSendableOutboundReplyParts(payload); const text = diff --git a/extensions/slack/src/outbound-adapter.ts b/extensions/slack/src/outbound-adapter.ts index a3e5b79a9aa..1f6b99d96f6 100644 --- a/extensions/slack/src/outbound-adapter.ts +++ b/extensions/slack/src/outbound-adapter.ts @@ -19,6 +19,7 @@ import { } from "openclaw/plugin-sdk/reply-payload"; import { parseSlackBlocksInput } from "./blocks-input.js"; import { buildSlackInteractiveBlocks, type SlackBlock } from "./blocks-render.js"; +import { SLACK_TEXT_LIMIT } from "./limits.js"; import { sendMessageSlack, type SlackSendIdentity } from "./send.js"; const SLACK_MAX_BLOCKS = 50; @@ -149,7 +150,7 @@ function resolveSlackBlocks(payload: { export const slackOutbound: ChannelOutboundAdapter = { deliveryMode: "direct", chunker: null, - textChunkLimit: 4000, + textChunkLimit: SLACK_TEXT_LIMIT, sendPayload: async (ctx) => { const payload = { ...ctx.payload, diff --git a/extensions/slack/src/send.blocks.test.ts b/extensions/slack/src/send.blocks.test.ts index 690f95120f0..3f256691654 100644 --- a/extensions/slack/src/send.blocks.test.ts +++ b/extensions/slack/src/send.blocks.test.ts @@ -50,6 +50,26 @@ describe("sendMessageSlack NO_REPLY guard", () => { }); }); +describe("sendMessageSlack chunking", () => { + it("keeps 4205-character text in a single Slack post by default", async () => { + const client = createSlackSendTestClient(); + const message = "a".repeat(4205); + + await sendMessageSlack("channel:C123", message, { + token: "xoxb-test", + client, + }); + + expect(client.chat.postMessage).toHaveBeenCalledTimes(1); + expect(client.chat.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "C123", + text: message, + }), + ); + }); +}); + describe("sendMessageSlack blocks", () => { it("posts blocks with fallback text when message is empty", async () => { const client = createSlackSendTestClient(); diff --git a/extensions/slack/src/send.ts b/extensions/slack/src/send.ts index 547013dc398..a33125f558b 100644 --- a/extensions/slack/src/send.ts +++ b/extensions/slack/src/send.ts @@ -20,10 +20,9 @@ import { buildSlackBlocksFallbackText } from "./blocks-fallback.js"; import { validateSlackBlocksArray } from "./blocks-input.js"; import { createSlackWebClient } from "./client.js"; import { markdownToSlackMrkdwnChunks } from "./format.js"; +import { SLACK_TEXT_LIMIT } from "./limits.js"; import { parseSlackTarget } from "./targets.js"; import { resolveSlackBotToken } from "./token.js"; - -const SLACK_TEXT_LIMIT = 4000; const SLACK_UPLOAD_SSRF_POLICY = { allowedHostnames: ["*.slack.com", "*.slack-edge.com", "*.slack-files.com"], allowRfc2544BenchmarkRange: true, @@ -296,7 +295,9 @@ export async function sendMessageSlack( channelId, }; } - const textLimit = resolveTextChunkLimit(cfg, "slack", account.accountId); + const textLimit = resolveTextChunkLimit(cfg, "slack", account.accountId, { + fallbackLimit: SLACK_TEXT_LIMIT, + }); const chunkLimit = Math.min(textLimit, SLACK_TEXT_LIMIT); const tableMode = resolveMarkdownTableMode({ cfg, diff --git a/src/agents/pi-embedded-subscribe.handlers.messages.ts b/src/agents/pi-embedded-subscribe.handlers.messages.ts index 283457ffc0e..bdc288a8c22 100644 --- a/src/agents/pi-embedded-subscribe.handlers.messages.ts +++ b/src/agents/pi-embedded-subscribe.handlers.messages.ts @@ -38,6 +38,15 @@ const stripTrailingDirective = (text: string): string => { return text.slice(0, openIndex); }; +function isTranscriptOnlyOpenClawAssistantMessage(message: AgentMessage | undefined): boolean { + if (!message || message.role !== "assistant") { + return false; + } + const provider = typeof message.provider === "string" ? message.provider.trim() : ""; + const model = typeof message.model === "string" ? message.model.trim() : ""; + return provider === "openclaw" && (model === "delivery-mirror" || model === "gateway-injected"); +} + function emitReasoningEnd(ctx: EmbeddedPiSubscribeContext) { if (!ctx.state.reasoningStreamOpen) { return; @@ -134,7 +143,7 @@ export function handleMessageStart( evt: AgentEvent & { message: AgentMessage }, ) { const msg = evt.message; - if (msg?.role !== "assistant") { + if (msg?.role !== "assistant" || isTranscriptOnlyOpenClawAssistantMessage(msg)) { return; } @@ -153,7 +162,7 @@ export function handleMessageUpdate( evt: AgentEvent & { message: AgentMessage; assistantMessageEvent?: unknown }, ) { const msg = evt.message; - if (msg?.role !== "assistant") { + if (msg?.role !== "assistant" || isTranscriptOnlyOpenClawAssistantMessage(msg)) { return; } @@ -323,7 +332,7 @@ export function handleMessageEnd( evt: AgentEvent & { message: AgentMessage }, ) { const msg = evt.message; - if (msg?.role !== "assistant") { + if (msg?.role !== "assistant" || isTranscriptOnlyOpenClawAssistantMessage(msg)) { return; } diff --git a/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.suppresses-message-end-block-replies-message-tool.test.ts b/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.suppresses-message-end-block-replies-message-tool.test.ts index 737edc7fda2..fae1018bf1e 100644 --- a/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.suppresses-message-end-block-replies-message-tool.test.ts +++ b/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.suppresses-message-end-block-replies-message-tool.test.ts @@ -42,10 +42,15 @@ async function emitMessageToolLifecycle(params: { }); } -function emitAssistantMessageEnd(emit: (evt: unknown) => void, text: string) { +function emitAssistantMessageEnd( + emit: (evt: unknown) => void, + text: string, + overrides?: Partial, +) { const assistantMessage = { role: "assistant", content: [{ type: "text", text }], + ...overrides, } as AssistantMessage; emit({ type: "message_end", message: assistantMessage }); } @@ -68,6 +73,7 @@ describe("subscribeEmbeddedPiSession", () => { result: "ok", }); emitAssistantMessageEnd(emit, messageText); + await Promise.resolve(); expect(onBlockReply).not.toHaveBeenCalled(); }); @@ -82,16 +88,44 @@ describe("subscribeEmbeddedPiSession", () => { result: { details: { status: "error" } }, }); emitAssistantMessageEnd(emit, messageText); + await Promise.resolve(); expect(onBlockReply).toHaveBeenCalledTimes(1); }); - it("clears block reply state on message_start", () => { + + it("ignores delivery-mirror assistant messages", async () => { + const { emit, onBlockReply } = createBlockReplyHarness("message_end"); + + emitAssistantMessageEnd(emit, "Mirrored transcript text", { + provider: "openclaw", + model: "delivery-mirror", + }); + await Promise.resolve(); + + expect(onBlockReply).not.toHaveBeenCalled(); + }); + + it("ignores gateway-injected assistant messages", async () => { + const { emit, onBlockReply } = createBlockReplyHarness("message_end"); + + emitAssistantMessageEnd(emit, "Injected transcript text", { + provider: "openclaw", + model: "gateway-injected", + }); + await Promise.resolve(); + + expect(onBlockReply).not.toHaveBeenCalled(); + }); + + it("clears block reply state on message_start", async () => { const { emit, onBlockReply } = createBlockReplyHarness("text_end"); emitAssistantTextEndBlock(emit, "OK"); + await Promise.resolve(); expect(onBlockReply).toHaveBeenCalledTimes(1); // New assistant message with identical output should still emit. emitAssistantTextEndBlock(emit, "OK"); + await Promise.resolve(); expect(onBlockReply).toHaveBeenCalledTimes(2); }); }); diff --git a/src/config/sessions/sessions.test.ts b/src/config/sessions/sessions.test.ts index ff38953aae2..9577fd8a8e5 100644 --- a/src/config/sessions/sessions.test.ts +++ b/src/config/sessions/sessions.test.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { upsertAcpSessionMeta } from "../../acp/runtime/session-meta.js"; import * as jsonFiles from "../../infra/json-files.js"; +import * as transcriptEvents from "../../sessions/transcript-events.js"; import type { OpenClawConfig } from "../config.js"; import { clearSessionStoreCacheForTest, @@ -429,6 +430,42 @@ describe("appendAssistantMessageToSessionTranscript", () => { } }); + it("emits transcript update events for delivery mirrors", async () => { + const sessionId = "test-session-id"; + const sessionKey = "test-session"; + const store = { + [sessionKey]: { + sessionId, + chatType: "direct", + channel: "discord", + }, + }; + fs.writeFileSync(fixture.storePath(), JSON.stringify(store), "utf-8"); + const emitSpy = vi.spyOn(transcriptEvents, "emitSessionTranscriptUpdate"); + + await appendAssistantMessageToSessionTranscript({ + sessionKey, + text: "Hello from delivery mirror!", + storePath: fixture.storePath(), + }); + + const sessionFile = resolveSessionTranscriptPathInDir(sessionId, fixture.sessionsDir()); + expect(emitSpy).toHaveBeenCalledWith( + expect.objectContaining({ + sessionFile, + sessionKey, + messageId: expect.any(String), + message: expect.objectContaining({ + role: "assistant", + provider: "openclaw", + model: "delivery-mirror", + content: [{ type: "text", text: "Hello from delivery mirror!" }], + }), + }), + ); + emitSpy.mockRestore(); + }); + it("does not append a duplicate delivery mirror for the same idempotency key", async () => { writeTranscriptStore(); diff --git a/src/infra/provider-usage.auth.plugin.test.ts b/src/infra/provider-usage.auth.plugin.test.ts index b8fa75afc5f..2f71aaafc55 100644 --- a/src/infra/provider-usage.auth.plugin.test.ts +++ b/src/infra/provider-usage.auth.plugin.test.ts @@ -1,10 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -const resolveProviderUsageAuthWithPluginMock = vi.fn(); +const resolveProviderUsageAuthWithPluginMock = vi.fn( + async (..._args: unknown[]): Promise => null, +); vi.mock("../plugins/provider-runtime.js", () => ({ - resolveProviderUsageAuthWithPlugin: (...args: unknown[]) => - resolveProviderUsageAuthWithPluginMock(...args), + resolveProviderUsageAuthWithPlugin: resolveProviderUsageAuthWithPluginMock, })); let resolveProviderAuths: typeof import("./provider-usage.auth.js").resolveProviderAuths;