mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-24 16:32:29 +00:00
fix: prevent delivery-mirror re-delivery and raise Slack chunk limit (#45489)
Merged via squash.
Prepared head SHA: c7664c7b6e
Co-authored-by: theo674 <261068216+theo674@users.noreply.github.com>
Co-authored-by: altaywtf <9790196+altaywtf@users.noreply.github.com>
Reviewed-by: @altaywtf
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<ResolvedSlackAccount, SlackProbe> = crea
|
||||
base: {
|
||||
deliveryMode: "direct",
|
||||
chunker: null,
|
||||
textChunkLimit: 4000,
|
||||
textChunkLimit: SLACK_TEXT_LIMIT,
|
||||
},
|
||||
attachedResults: {
|
||||
channel: "slack",
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
1
extensions/slack/src/limits.ts
Normal file
1
extensions/slack/src/limits.ts
Normal file
@@ -0,0 +1 @@
|
||||
export const SLACK_TEXT_LIMIT = 8000;
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -6,6 +6,7 @@ vi.mock("../send.js", () => ({
|
||||
}));
|
||||
|
||||
let deliverReplies: typeof import("./replies.js").deliverReplies;
|
||||
import { deliverSlackSlashReplies } from "./replies.js";
|
||||
|
||||
function baseParams(overrides?: Record<string, unknown>) {
|
||||
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",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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<AssistantMessage>,
|
||||
) {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const resolveProviderUsageAuthWithPluginMock = vi.fn();
|
||||
const resolveProviderUsageAuthWithPluginMock = vi.fn(
|
||||
async (..._args: unknown[]): Promise<unknown> => null,
|
||||
);
|
||||
|
||||
vi.mock("../plugins/provider-runtime.js", () => ({
|
||||
resolveProviderUsageAuthWithPlugin: (...args: unknown[]) =>
|
||||
resolveProviderUsageAuthWithPluginMock(...args),
|
||||
resolveProviderUsageAuthWithPlugin: resolveProviderUsageAuthWithPluginMock,
|
||||
}));
|
||||
|
||||
let resolveProviderAuths: typeof import("./provider-usage.auth.js").resolveProviderAuths;
|
||||
|
||||
Reference in New Issue
Block a user