fix(slack): drop ambiguous thread replies

Co-authored-by: Soichiro Yoshimura <soichiro0111.dev@gmail.com>
This commit is contained in:
Peter Steinberger
2026-05-10 16:54:32 +01:00
parent 5867734344
commit d0f0100bb2
7 changed files with 77 additions and 3 deletions

View File

@@ -47,6 +47,7 @@ Docs: https://docs.openclaw.ai
- Slack: compile interactive reply directives for direct outbound sends without bypassing the `interactiveReplies` capability gate, preserving Block Kit for Slack CLI and cron deliveries. (#78220) Thanks @kazamak.
- Slack: keep DM last-route updates scoped to the active non-main DM session, including threaded DM turns, so isolated Slack DM sessions do not overwrite the shared main route. (#73085) Thanks @clawSean.
- Slack/ACP: route Slack channel and DM messages through configured ACP bindings when no runtime binding exists, keeping bound thread replies pinned to the persistent ACP session and dropping unavailable configured targets instead of falling back to `main`. (#73101) Thanks @Raasl.
- Slack: mark unresolved thread replies as ambiguous and skip them instead of treating them as root channel messages, keeping thread continuation on the SDK-backed participation store. (#75630) Thanks @soichiyo.
- Gateway/agents: keep structured reasons when active-run queueing fails and deprecate the legacy boolean queue helper, so steering and subagent wake diagnostics distinguish completed, non-streaming, and compacting runs. Fixes #80156. Thanks @markus-lassfolk.
- Agents/UI: compact exec and tool progress rows by hiding redundant shell tool names, replacing known workspace paths with short context markers, and preserving Discord trace scrubbing for compact command lines.
- ACPX: run and await the embedded ACP backend startup probe by default so the gateway `ready` signal no longer fires before the acpx runtime has either become usable or reported a probe failure; set `OPENCLAW_ACPX_RUNTIME_STARTUP_PROBE=0` to restore lazy startup. Fixes #79596. Thanks @bzelones.

View File

@@ -50,6 +50,7 @@ describe("Slack missing thread_ts recovery", () => {
historyResponse: { messages: [{ ts: "456" }] },
});
expect(message.thread_ts).toBeUndefined();
expect(message._ambiguousThreadReply).toBe(true);
});
it("continues without thread_ts when history lookup throws", async () => {
@@ -57,5 +58,6 @@ describe("Slack missing thread_ts recovery", () => {
historyError: new Error("history failed"),
});
expect(message.thread_ts).toBeUndefined();
expect(message._ambiguousThreadReply).toBe(true);
});
});

View File

@@ -1213,6 +1213,32 @@ describe("slack prepareSlackMessage inbound contract", () => {
expect(replies).toHaveBeenCalledTimes(1);
});
it("drops ambiguous thread replies instead of treating them as root messages", async () => {
const { storePath } = storeFixture.makeTmpStorePath();
const cfg = {
session: { store: storePath },
channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } },
} as OpenClawConfig;
const replies = vi.fn();
const slackCtx = createThreadSlackCtx({ cfg, replies });
slackCtx.resolveUserName = async () => ({ name: "Alice" });
slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" });
const prepared = await prepareMessageWith(slackCtx, createThreadAccount(), {
...createSlackMessage({
channel: "C123",
channel_type: "channel",
text: "<@B1> can you follow up?",
ts: "201.000",
parent_user_id: "U2",
}),
_ambiguousThreadReply: true,
});
expect(prepared).toBeNull();
expect(replies).not.toHaveBeenCalled();
});
it("includes thread_ts and parent_user_id metadata in thread replies", async () => {
const message = createSlackMessage({
text: "this is a reply",

View File

@@ -591,6 +591,17 @@ export async function prepareSlackMessage(params: {
const shouldRequireMention = isRoom
? (channelConfig?.requireMention ?? ctx.defaultRequireMention)
: false;
if (message._ambiguousThreadReply) {
ctx.logger.info(
{
channel: message.channel,
ts: message.ts,
parentUserId: message.parent_user_id,
},
"skipping ambiguous slack thread reply",
);
return null;
}
const canDetectMention = Boolean(ctx.botUserId) || mentionRegexes.length > 0;
// Strip Slack mentions (<@U123>) before command detection so "@Labrador /new" is recognized
const textForCommandDetection = stripSlackMentionsForCommandDetection(message.text ?? "");

View File

@@ -26,4 +26,28 @@ describe("createSlackThreadTsResolver", () => {
expect(second.thread_ts).toBe("9");
expect(historyMock).toHaveBeenCalledTimes(1);
});
it("marks cached unresolved lookups as ambiguous thread replies", async () => {
const historyMock = vi.fn().mockResolvedValue({
messages: [{ ts: "1" }],
});
const resolver = createSlackThreadTsResolver({
client: { conversations: { history: historyMock } } as any,
cacheTtlMs: 60_000,
maxSize: 5,
});
const message = {
channel: "C1",
parent_user_id: "U2",
ts: "1",
} as SlackMessageEvent;
const first = await resolver.resolve({ message, source: "message" });
const second = await resolver.resolve({ message, source: "message" });
expect(first).toMatchObject({ _ambiguousThreadReply: true });
expect(second).toMatchObject({ _ambiguousThreadReply: true });
expect(historyMock).toHaveBeenCalledTimes(1);
});
});

View File

@@ -17,6 +17,11 @@ const normalizeThreadTs = (threadTs?: string | null) => {
return trimmed ? trimmed : undefined;
};
const markAmbiguousThreadReply = (message: SlackMessageEvent): SlackMessageEvent => ({
...message,
_ambiguousThreadReply: true,
});
async function resolveThreadTsFromHistory(params: {
client: SlackWebClient;
channelId: string;
@@ -87,7 +92,7 @@ export function createSlackThreadTsResolver(params: {
const now = Date.now();
const cached = getCached(cacheKey, now);
if (cached !== undefined) {
return cached ? { ...message, thread_ts: cached } : message;
return cached ? { ...message, thread_ts: cached } : markAmbiguousThreadReply(message);
}
if (shouldLogVerbose()) {
@@ -126,10 +131,10 @@ export function createSlackThreadTsResolver(params: {
if (shouldLogVerbose()) {
logVerbose(
`slack inbound: could not resolve missing thread_ts channel=${message.channel} ts=${message.ts}`,
`slack inbound: could not resolve missing thread_ts channel=${message.channel} ts=${message.ts}; marking reply ambiguous`,
);
}
return message;
return markAmbiguousThreadReply(message);
},
};
}

View File

@@ -44,6 +44,11 @@ export type SlackMessageEvent = {
blocks?: unknown[];
files?: SlackFile[];
attachments?: SlackAttachment[];
/**
* Set by the thread_ts resolver when Slack supplied parent_user_id but the
* parent thread timestamp could not be recovered.
*/
_ambiguousThreadReply?: boolean;
};
export type SlackAppMentionEvent = {