mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(slack): enforce replyToMode for auto-thread_ts and inline reply tags (#23839)
* Slack: respect replyToMode for auto-thread_ts and inline reply tags * Update CHANGELOG.md
This commit is contained in:
@@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Slack/Threading: respect `replyToMode` when Slack auto-populates top-level `thread_ts`, and ignore inline `replyToId` directive tags when `replyToMode` is `off` so thread forcing stays disabled unless explicitly configured. (#23839, #23320, #23513) Thanks @vincentkoc and @dorukardahan.
|
||||
- Slack/Extension: forward `message read` `threadId` to `readMessages` and use delivery-context `threadId` as outbound `thread_ts` fallback so extension replies/reads stay in the correct Slack thread. (#22216, #22485, #23836) Thanks @vincentkoc, @lan17 and @dorukardahan.
|
||||
- Config/Memory: allow `"mistral"` in `agents.defaults.memorySearch.provider` and `agents.defaults.memorySearch.fallback` schema validation. (#14934) Thanks @ThomsenDrake.
|
||||
- Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting.
|
||||
|
||||
@@ -443,7 +443,7 @@ describe("monitorSlackProvider tool results", () => {
|
||||
expect(sendMock.mock.calls[0][2]).toMatchObject({ threadTs: "111.222" });
|
||||
});
|
||||
|
||||
it("forces thread replies when replyToId is set", async () => {
|
||||
it("ignores replyToId directive when replyToMode is off", async () => {
|
||||
replyMock.mockResolvedValue({ text: "forced reply", replyToId: "555" });
|
||||
slackTestState.config = {
|
||||
messages: {
|
||||
@@ -467,6 +467,20 @@ describe("monitorSlackProvider tool results", () => {
|
||||
}),
|
||||
});
|
||||
|
||||
expect(sendMock).toHaveBeenCalledTimes(1);
|
||||
expect(sendMock.mock.calls[0][2]).toMatchObject({ threadTs: undefined });
|
||||
});
|
||||
|
||||
it("keeps replyToId directive threading when replyToMode is all", async () => {
|
||||
replyMock.mockResolvedValue({ text: "forced reply", replyToId: "555" });
|
||||
setDirectMessageReplyMode("all");
|
||||
|
||||
await runSlackMessageOnce(monitorSlackProvider, {
|
||||
event: makeSlackMessageEvent({
|
||||
ts: "789",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(sendMock).toHaveBeenCalledTimes(1);
|
||||
expect(sendMock.mock.calls[0][2]).toMatchObject({ threadTs: "555" });
|
||||
});
|
||||
|
||||
@@ -103,7 +103,6 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag
|
||||
incomingThreadTs,
|
||||
messageTs,
|
||||
hasRepliedRef,
|
||||
chatType: prepared.isDirectMessage ? "direct" : "channel",
|
||||
isThreadReply,
|
||||
});
|
||||
|
||||
@@ -187,6 +186,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag
|
||||
runtime,
|
||||
textLimit: ctx.textLimit,
|
||||
replyThreadTs,
|
||||
replyToMode: ctx.replyToMode,
|
||||
});
|
||||
replyPlan.markSent();
|
||||
};
|
||||
|
||||
@@ -16,9 +16,13 @@ export async function deliverReplies(params: {
|
||||
runtime: RuntimeEnv;
|
||||
textLimit: number;
|
||||
replyThreadTs?: string;
|
||||
replyToMode: "off" | "first" | "all";
|
||||
}) {
|
||||
for (const payload of params.replies) {
|
||||
const threadTs = payload.replyToId ?? params.replyThreadTs;
|
||||
// Keep reply tags opt-in: when replyToMode is off, explicit reply tags
|
||||
// must not force threading.
|
||||
const inlineReplyToId = params.replyToMode === "off" ? undefined : payload.replyToId;
|
||||
const threadTs = inlineReplyToId ?? params.replyThreadTs;
|
||||
const mediaList = payload.mediaUrls ?? (payload.mediaUrl ? [payload.mediaUrl] : []);
|
||||
const text = payload.text ?? "";
|
||||
if (!text && mediaList.length === 0) {
|
||||
@@ -88,19 +92,11 @@ function createSlackReplyReferencePlanner(params: {
|
||||
incomingThreadTs: string | undefined;
|
||||
messageTs: string | undefined;
|
||||
hasReplied?: boolean;
|
||||
chatType?: "direct" | "channel" | "group";
|
||||
isThreadReply?: boolean;
|
||||
}) {
|
||||
// When already inside a Slack thread, stay in it — but for DMs where the
|
||||
// "thread" was created by the typing indicator (not a real thread reply),
|
||||
// respect the user's replyToMode setting.
|
||||
// See: https://github.com/openclaw/openclaw/issues/16868
|
||||
const effectiveMode =
|
||||
params.chatType === "direct" && !params.isThreadReply
|
||||
? params.replyToMode
|
||||
: params.incomingThreadTs
|
||||
? "all"
|
||||
: params.replyToMode;
|
||||
// Only force threading for real user thread replies. If Slack auto-populates
|
||||
// thread_ts on top-level messages, preserve the configured reply mode.
|
||||
const effectiveMode = params.isThreadReply ? "all" : params.replyToMode;
|
||||
return createReplyReferencePlanner({
|
||||
replyToMode: effectiveMode,
|
||||
existingId: params.incomingThreadTs,
|
||||
@@ -114,7 +110,6 @@ export function createSlackReplyDeliveryPlan(params: {
|
||||
incomingThreadTs: string | undefined;
|
||||
messageTs: string | undefined;
|
||||
hasRepliedRef: { value: boolean };
|
||||
chatType?: "direct" | "channel" | "group";
|
||||
isThreadReply?: boolean;
|
||||
}): SlackReplyDeliveryPlan {
|
||||
const replyReference = createSlackReplyReferencePlanner({
|
||||
@@ -122,7 +117,6 @@ export function createSlackReplyDeliveryPlan(params: {
|
||||
incomingThreadTs: params.incomingThreadTs,
|
||||
messageTs: params.messageTs,
|
||||
hasReplied: params.hasRepliedRef.value,
|
||||
chatType: params.chatType,
|
||||
isThreadReply: params.isThreadReply,
|
||||
});
|
||||
return {
|
||||
|
||||
@@ -45,6 +45,38 @@ describe("resolveSlackThreadTargets", () => {
|
||||
expect(statusThreadTs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not treat auto-created top-level thread_ts as a real thread when mode is off", () => {
|
||||
const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({
|
||||
replyToMode: "off",
|
||||
message: {
|
||||
type: "message",
|
||||
channel: "C1",
|
||||
ts: "123",
|
||||
thread_ts: "123",
|
||||
},
|
||||
});
|
||||
|
||||
expect(isThreadReply).toBe(false);
|
||||
expect(replyThreadTs).toBeUndefined();
|
||||
expect(statusThreadTs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps first-mode behavior for auto-created top-level thread_ts", () => {
|
||||
const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({
|
||||
replyToMode: "first",
|
||||
message: {
|
||||
type: "message",
|
||||
channel: "C1",
|
||||
ts: "123",
|
||||
thread_ts: "123",
|
||||
},
|
||||
});
|
||||
|
||||
expect(isThreadReply).toBe(false);
|
||||
expect(replyThreadTs).toBeUndefined();
|
||||
expect(statusThreadTs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("sets messageThreadId for top-level messages when replyToMode is all", () => {
|
||||
const context = resolveSlackThreadContext({
|
||||
replyToMode: "all",
|
||||
|
||||
@@ -48,7 +48,11 @@ export function resolveSlackThreadTargets(params: {
|
||||
}) {
|
||||
const ctx = resolveSlackThreadContext(params);
|
||||
const { incomingThreadTs, messageTs, isThreadReply } = ctx;
|
||||
const replyThreadTs = incomingThreadTs ?? (params.replyToMode === "all" ? messageTs : undefined);
|
||||
const replyThreadTs = isThreadReply
|
||||
? incomingThreadTs
|
||||
: params.replyToMode === "all"
|
||||
? messageTs
|
||||
: undefined;
|
||||
const statusThreadTs = replyThreadTs;
|
||||
return { replyThreadTs, statusThreadTs, isThreadReply };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user