mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:30:42 +00:00
fix(slack): preserve real thread anchors
This commit is contained in:
@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Slack/native streaming: suppress reasoning-only payloads before `chat.startStream`/`appendStream`, so Claude extended-thinking blocks no longer appear as visible Slack messages. Fixes #59687. Thanks @vision-ifc.
|
||||
- Slack/block replies: keep multi-part block deliveries in the first Slack reply thread when `replyToMode` is `first`, matching text reply threading instead of leaking later blocks into the channel. Fixes #49341. Thanks @pholmstr and @xiwuqi.
|
||||
- Slack/thread broadcasts: process `thread_broadcast` events as user messages so replies sent with "Also send to channel" reach the agent instead of becoming metadata-only system events. Fixes #56605 and #4351. Thanks @clawSean and @jlowin.
|
||||
- Slack/threading: ignore internal reply ids when choosing Slack `thread_ts` values, so resumed replies keep the real Slack thread anchor instead of leaking to the channel root. Fixes #68790. Thanks @MonkeyLeeT and @martingarramon.
|
||||
- Agents/failover: stop body-less HTTP 400/422 proxy failures from defaulting to `"format"` classification, so embedded retries surface the opaque provider failure instead of falling into a compaction loop. Fixes #66462. (#67024) Thanks @altaywtf and @HongzhuLiu.
|
||||
- Plugins/loader: use cached discovery-mode snapshot loads for read-only plugin capability lookups, keep snapshot caches isolated from active Gateway registries, and make same-plugin channel/HTTP route re-registration idempotent so repeated snapshot or hot-reload paths no longer rerun full plugin side effects or accumulate duplicate surfaces. Fixes #51781, #52031, #54181, and #57514. Thanks @livingghost, @okuyam2y, @ShionEria, and @bbshih.
|
||||
- Plugins/loader: reuse the compatible active Gateway registry for broad runtime plugin ensure calls after a gateway-bindable boot load, so non-bundled plugins no longer re-run `register()` during the same boot path. Fixes #69250. Thanks @markthebest12.
|
||||
|
||||
@@ -456,6 +456,127 @@ describe("slackPlugin outbound", () => {
|
||||
expect(result).toEqual({ channel: "slack", messageId: "m-media" });
|
||||
});
|
||||
|
||||
it("falls back to threadId when replyToId is not a Slack thread timestamp", async () => {
|
||||
const sendSlack = vi.fn().mockResolvedValue({ messageId: "m-text" });
|
||||
const sendText = requireSlackSendText();
|
||||
|
||||
const result = await sendText({
|
||||
cfg,
|
||||
to: "C123",
|
||||
text: "hello",
|
||||
accountId: "default",
|
||||
replyToId: "msg-internal-1",
|
||||
threadId: "1712345678.123456",
|
||||
deps: { sendSlack },
|
||||
});
|
||||
|
||||
expect(sendSlack).toHaveBeenCalledWith(
|
||||
"C123",
|
||||
"hello",
|
||||
expect.objectContaining({
|
||||
threadTs: "1712345678.123456",
|
||||
}),
|
||||
);
|
||||
expect(result).toEqual({ channel: "slack", messageId: "m-text" });
|
||||
});
|
||||
|
||||
it("does not stringify numeric Slack thread ids", async () => {
|
||||
const sendSlack = vi.fn().mockResolvedValue({ messageId: "m-text" });
|
||||
const sendText = requireSlackSendText();
|
||||
|
||||
await sendText({
|
||||
cfg,
|
||||
to: "C123",
|
||||
text: "hello",
|
||||
accountId: "default",
|
||||
threadId: 1712345678.123456,
|
||||
deps: { sendSlack },
|
||||
});
|
||||
|
||||
expect(sendSlack).toHaveBeenCalledWith(
|
||||
"C123",
|
||||
"hello",
|
||||
expect.objectContaining({
|
||||
threadTs: undefined,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to auto-thread lookup when replyToId is not a Slack thread timestamp", () => {
|
||||
const resolveAutoThreadId = slackPlugin.threading?.resolveAutoThreadId;
|
||||
if (!resolveAutoThreadId) {
|
||||
throw new Error("slack threading.resolveAutoThreadId unavailable");
|
||||
}
|
||||
|
||||
const threadId = resolveAutoThreadId({
|
||||
cfg,
|
||||
to: "channel:C123",
|
||||
replyToId: "msg-internal-1",
|
||||
toolContext: {
|
||||
currentChannelId: "C123",
|
||||
currentThreadTs: "1712345678.123456",
|
||||
replyToMode: "all",
|
||||
},
|
||||
});
|
||||
|
||||
expect(threadId).toBe("1712345678.123456");
|
||||
});
|
||||
|
||||
it("does not recover invalid Slack auto-thread anchors", () => {
|
||||
const resolveAutoThreadId = slackPlugin.threading?.resolveAutoThreadId;
|
||||
if (!resolveAutoThreadId) {
|
||||
throw new Error("slack threading.resolveAutoThreadId unavailable");
|
||||
}
|
||||
|
||||
const threadId = resolveAutoThreadId({
|
||||
cfg,
|
||||
to: "channel:C123",
|
||||
replyToId: "msg-internal-1",
|
||||
toolContext: {
|
||||
currentChannelId: "C123",
|
||||
currentThreadTs: "thread-root",
|
||||
replyToMode: "all",
|
||||
},
|
||||
});
|
||||
|
||||
expect(threadId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not stringify numeric thread ids in tool context", () => {
|
||||
const buildToolContext = slackPlugin.threading?.buildToolContext;
|
||||
if (!buildToolContext) {
|
||||
throw new Error("slack threading.buildToolContext unavailable");
|
||||
}
|
||||
|
||||
const context = buildToolContext({
|
||||
cfg,
|
||||
context: {
|
||||
To: "channel:C123",
|
||||
MessageThreadId: 1712345678.123456,
|
||||
},
|
||||
});
|
||||
|
||||
expect(context?.currentThreadTs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("falls back to threadId in reply transport when replyToId is not a Slack thread timestamp", () => {
|
||||
const resolveReplyTransport = slackPlugin.threading?.resolveReplyTransport;
|
||||
if (!resolveReplyTransport) {
|
||||
throw new Error("slack threading.resolveReplyTransport unavailable");
|
||||
}
|
||||
|
||||
expect(
|
||||
resolveReplyTransport({
|
||||
cfg,
|
||||
replyToId: "msg-internal-1",
|
||||
threadId: "1712345678.123456",
|
||||
}),
|
||||
).toEqual({
|
||||
replyToId: "1712345678.123456",
|
||||
threadId: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("forwards mediaLocalRoots for sendMedia", async () => {
|
||||
const sendSlack = vi.fn().mockResolvedValue({ messageId: "m-media-local" });
|
||||
const sendMedia = requireSlackSendMedia();
|
||||
|
||||
@@ -61,6 +61,7 @@ import {
|
||||
slackConfigAdapter,
|
||||
} from "./shared.js";
|
||||
import { parseSlackTarget } from "./target-parsing.js";
|
||||
import { normalizeSlackThreadTsCandidate, resolveSlackThreadTsValue } from "./thread-ts.js";
|
||||
import { buildSlackThreadingToolContext } from "./threading-tool-context.js";
|
||||
|
||||
// Lazy SDK loaders. The dynamic import is hidden behind a string-literal
|
||||
@@ -218,7 +219,7 @@ async function resolveSlackSendContext(params: {
|
||||
const token = getTokenForOperation(account, "write");
|
||||
const botToken = account.botToken?.trim();
|
||||
const tokenOverride = token && token !== botToken ? token : undefined;
|
||||
const threadTsValue = params.replyToId ?? params.threadId;
|
||||
const threadTsValue = resolveSlackThreadTsValue(params);
|
||||
return { send, threadTsValue, tokenOverride };
|
||||
}
|
||||
|
||||
@@ -608,14 +609,16 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
|
||||
allowExplicitReplyTagsWhenOff: false,
|
||||
buildToolContext: (params) => buildSlackThreadingToolContext(params),
|
||||
resolveAutoThreadId: ({ to, toolContext, replyToId }) =>
|
||||
replyToId
|
||||
normalizeSlackThreadTsCandidate(replyToId)
|
||||
? undefined
|
||||
: resolveSlackAutoThreadId({
|
||||
to,
|
||||
toolContext,
|
||||
}),
|
||||
: normalizeSlackThreadTsCandidate(
|
||||
resolveSlackAutoThreadId({
|
||||
to,
|
||||
toolContext,
|
||||
}),
|
||||
),
|
||||
resolveReplyTransport: ({ threadId, replyToId }) => ({
|
||||
replyToId: replyToId ?? (threadId != null && threadId !== "" ? String(threadId) : undefined),
|
||||
replyToId: resolveSlackThreadTsValue({ replyToId, threadId }),
|
||||
threadId: null,
|
||||
}),
|
||||
},
|
||||
@@ -632,7 +635,7 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
|
||||
payload,
|
||||
}),
|
||||
sendPayload: async (ctx) => {
|
||||
const { send, tokenOverride } = await resolveSlackSendContext({
|
||||
const { send, threadTsValue, tokenOverride } = await resolveSlackSendContext({
|
||||
cfg: ctx.cfg,
|
||||
accountId: ctx.accountId ?? undefined,
|
||||
deps: ctx.deps,
|
||||
@@ -642,6 +645,8 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
|
||||
const { slackOutbound } = await loadSlackOutboundAdapterModule();
|
||||
return await slackOutbound.sendPayload!({
|
||||
...ctx,
|
||||
replyToId: threadTsValue,
|
||||
threadId: null,
|
||||
deps: {
|
||||
...ctx.deps,
|
||||
slack: async (
|
||||
@@ -669,7 +674,7 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
|
||||
});
|
||||
return await send(to, text, {
|
||||
cfg,
|
||||
threadTs: threadTsValue != null ? String(threadTsValue) : undefined,
|
||||
threadTs: threadTsValue,
|
||||
accountId: accountId ?? undefined,
|
||||
...(tokenOverride ? { token: tokenOverride } : {}),
|
||||
});
|
||||
@@ -696,7 +701,7 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
|
||||
cfg,
|
||||
mediaUrl,
|
||||
mediaLocalRoots,
|
||||
threadTs: threadTsValue != null ? String(threadTsValue) : undefined,
|
||||
threadTs: threadTsValue,
|
||||
accountId: accountId ?? undefined,
|
||||
...(tokenOverride ? { token: tokenOverride } : {}),
|
||||
});
|
||||
|
||||
@@ -265,6 +265,9 @@ vi.mock("@slack/bolt", () => {
|
||||
command() {
|
||||
/* no-op */
|
||||
}
|
||||
use() {
|
||||
/* no-op */
|
||||
}
|
||||
start = vi.fn().mockResolvedValue(undefined);
|
||||
stop = vi.fn().mockResolvedValue(undefined);
|
||||
}
|
||||
|
||||
@@ -115,4 +115,62 @@ describe("slackOutbound", () => {
|
||||
);
|
||||
expect(result).toEqual({ channel: "slack", messageId: "m-blocks" });
|
||||
});
|
||||
|
||||
it("falls back to threadId when payload replyToId is not a Slack thread timestamp", async () => {
|
||||
sendMessageSlackMock.mockResolvedValueOnce({ messageId: "m-blocks" });
|
||||
|
||||
await slackOutbound.sendPayload!({
|
||||
cfg,
|
||||
to: "C123",
|
||||
text: "",
|
||||
replyToId: "msg-internal-1",
|
||||
threadId: "1712345678.123456",
|
||||
payload: {
|
||||
text: "fallback text",
|
||||
channelData: {
|
||||
slack: {
|
||||
blocks: [{ type: "divider" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
accountId: "default",
|
||||
});
|
||||
|
||||
expect(sendMessageSlackMock).toHaveBeenCalledWith(
|
||||
"C123",
|
||||
"fallback text",
|
||||
expect.objectContaining({
|
||||
threadTs: "1712345678.123456",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not thread payloads without a valid Slack thread timestamp", async () => {
|
||||
sendMessageSlackMock.mockResolvedValueOnce({ messageId: "m-blocks" });
|
||||
|
||||
await slackOutbound.sendPayload!({
|
||||
cfg,
|
||||
to: "C123",
|
||||
text: "",
|
||||
replyToId: "msg-internal-1",
|
||||
threadId: "thread-root",
|
||||
payload: {
|
||||
text: "fallback text",
|
||||
channelData: {
|
||||
slack: {
|
||||
blocks: [{ type: "divider" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
accountId: "default",
|
||||
});
|
||||
|
||||
expect(sendMessageSlackMock).toHaveBeenCalledWith(
|
||||
"C123",
|
||||
"fallback text",
|
||||
expect.objectContaining({
|
||||
threadTs: undefined,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
import { compileSlackInteractiveReplies } from "./interactive-replies.js";
|
||||
import { SLACK_TEXT_LIMIT } from "./limits.js";
|
||||
import type { SlackSendIdentity } from "./send.js";
|
||||
import { resolveSlackThreadTsValue } from "./thread-ts.js";
|
||||
|
||||
const SLACK_MAX_BLOCKS = 50;
|
||||
type SlackSendFn = typeof import("./send.runtime.js").sendMessageSlack;
|
||||
@@ -84,9 +85,13 @@ async function sendSlackOutboundMessage(params: {
|
||||
resolveOutboundSendDep<SlackSendFn>(params.deps, "slack") ??
|
||||
(await loadSlackSendRuntime()).sendMessageSlack;
|
||||
const slackIdentity = resolveSlackSendIdentity(params.identity);
|
||||
const threadTs = resolveSlackThreadTsValue({
|
||||
replyToId: params.replyToId,
|
||||
threadId: params.threadId,
|
||||
});
|
||||
const result = await send(params.to, params.text, {
|
||||
cfg: params.cfg,
|
||||
threadTs: params.replyToId ?? (params.threadId != null ? String(params.threadId) : undefined),
|
||||
threadTs,
|
||||
accountId: params.accountId ?? undefined,
|
||||
...(params.mediaUrl
|
||||
? {
|
||||
|
||||
34
extensions/slack/src/thread-ts.test.ts
Normal file
34
extensions/slack/src/thread-ts.test.ts
Normal file
@@ -0,0 +1,34 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { normalizeSlackThreadTsCandidate, resolveSlackThreadTsValue } from "./thread-ts.js";
|
||||
|
||||
describe("Slack thread_ts resolution", () => {
|
||||
it("accepts trimmed Slack timestamp strings", () => {
|
||||
expect(normalizeSlackThreadTsCandidate(" 1712345678.123456 ")).toBe("1712345678.123456");
|
||||
});
|
||||
|
||||
it("rejects internal reply ids", () => {
|
||||
expect(normalizeSlackThreadTsCandidate("msg-internal-1")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects numeric thread ids instead of stringifying them", () => {
|
||||
expect(normalizeSlackThreadTsCandidate(1712345678.123456)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("falls back from invalid replyToId to valid threadId", () => {
|
||||
expect(
|
||||
resolveSlackThreadTsValue({
|
||||
replyToId: "msg-internal-1",
|
||||
threadId: "1712345678.123456",
|
||||
}),
|
||||
).toBe("1712345678.123456");
|
||||
});
|
||||
|
||||
it("validates fallback threadId before using it", () => {
|
||||
expect(
|
||||
resolveSlackThreadTsValue({
|
||||
replyToId: "msg-internal-1",
|
||||
threadId: "thread-root",
|
||||
}),
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
23
extensions/slack/src/thread-ts.ts
Normal file
23
extensions/slack/src/thread-ts.ts
Normal file
@@ -0,0 +1,23 @@
|
||||
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
|
||||
|
||||
const SLACK_THREAD_TS_PATTERN = /^\d+\.\d+$/;
|
||||
|
||||
export function normalizeSlackThreadTsCandidate(
|
||||
value?: string | number | null,
|
||||
): string | undefined {
|
||||
if (typeof value !== "string") {
|
||||
return undefined;
|
||||
}
|
||||
const normalized = normalizeOptionalString(value);
|
||||
return normalized && SLACK_THREAD_TS_PATTERN.test(normalized) ? normalized : undefined;
|
||||
}
|
||||
|
||||
export function resolveSlackThreadTsValue(params: {
|
||||
replyToId?: string | number | null;
|
||||
threadId?: string | number | null;
|
||||
}): string | undefined {
|
||||
return (
|
||||
normalizeSlackThreadTsCandidate(params.replyToId) ??
|
||||
normalizeSlackThreadTsCandidate(params.threadId)
|
||||
);
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import type {
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
|
||||
import { resolveSlackAccount, resolveSlackReplyToMode } from "./accounts.js";
|
||||
import { normalizeSlackThreadTsCandidate } from "./thread-ts.js";
|
||||
|
||||
export function buildSlackThreadingToolContext(params: {
|
||||
cfg: OpenClawConfig;
|
||||
@@ -28,7 +29,7 @@ export function buildSlackThreadingToolContext(params: {
|
||||
: normalizeOptionalString(params.context.NativeChannelId);
|
||||
return {
|
||||
currentChannelId,
|
||||
currentThreadTs: threadId != null ? String(threadId) : undefined,
|
||||
currentThreadTs: normalizeSlackThreadTsCandidate(threadId),
|
||||
replyToMode: effectiveReplyToMode,
|
||||
hasRepliedRef: params.hasRepliedRef,
|
||||
};
|
||||
|
||||
@@ -61,11 +61,19 @@ const slackMessaging: ChannelMessagingAdapter = {
|
||||
|
||||
const slackThreading: ChannelThreadingAdapter = {
|
||||
resolveReplyTransport: ({ threadId, replyToId }) => ({
|
||||
replyToId: replyToId ?? (threadId != null && threadId !== "" ? String(threadId) : undefined),
|
||||
replyToId: resolveSlackThreadTsCandidate(replyToId) ?? resolveSlackThreadTsCandidate(threadId),
|
||||
threadId: null,
|
||||
}),
|
||||
};
|
||||
|
||||
function resolveSlackThreadTsCandidate(value?: string | number | null): string | undefined {
|
||||
if (typeof value !== "string") {
|
||||
return undefined;
|
||||
}
|
||||
const normalized = value.trim();
|
||||
return /^\d+\.\d+$/.test(normalized) ? normalized : undefined;
|
||||
}
|
||||
|
||||
const mattermostThreading: ChannelThreadingAdapter = {
|
||||
resolveReplyTransport: ({ threadId, replyToId }) => ({
|
||||
replyToId: replyToId ?? (threadId != null && threadId !== "" ? String(threadId) : undefined),
|
||||
@@ -489,6 +497,21 @@ describe("routeReply", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("uses Slack threadId when routed replyToId is an internal message id", async () => {
|
||||
await routeReply({
|
||||
payload: { text: "hi", replyToId: "msg-internal-1" },
|
||||
channel: "slack",
|
||||
to: "channel:C123",
|
||||
threadId: "1710000000.9999",
|
||||
cfg: {} as never,
|
||||
});
|
||||
expectLastDelivery({
|
||||
channel: "slack",
|
||||
replyToId: "1710000000.9999",
|
||||
threadId: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("uses threadId as replyToId for Mattermost when replyToId is missing", async () => {
|
||||
await routeReply({
|
||||
payload: { text: "hi" },
|
||||
|
||||
Reference in New Issue
Block a user