From ce71fac7d6b116a2fed53101fc21350e95167875 Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Thu, 5 Mar 2026 21:47:52 -0600 Subject: [PATCH] fix(slack): record app_mention retry key before dedupe check (#37033) - Prime app_mention retry allowance before dedupe so near-simultaneous message/app_mention races do not drop valid mentions. - Prevent duplicate dispatch when app_mention wins the race and message prepare later succeeds. - Prune dispatched mention keys and add regression coverage for both dropped and successful in-flight message outcomes. Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- CHANGELOG.md | 1 + .../message-handler.app-mention-race.test.ts | 106 ++++++++++++++++++ src/slack/monitor/message-handler.ts | 28 ++++- 3 files changed, 129 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3102d81955..cbe1efdece3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Slack/app_mention race dedupe: when `app_mention` dispatch wins while same-`ts` `message` prepare is still in-flight, suppress the later message dispatch so near-simultaneous Slack deliveries do not produce duplicate replies; keep single-retry behavior and add regression coverage for both dropped and successful message-prepare outcomes. (#37033) Thanks @Takhoffman. - Gateway/chat streaming tool-boundary text retention: merge assistant delta segments into per-run chat buffers so pre-tool text is preserved in live chat deltas/finals when providers emit post-tool assistant segments as non-prefix snapshots. (#36957) Thanks @Datyedyeguy. - TUI/model indicator freshness: prevent stale session snapshots from overwriting freshly patched model selection (and reset per-session freshness when switching session keys) so `/model` updates reflect immediately instead of lagging by one or more commands. (#21255) Thanks @kowza. - TUI/final-error rendering fallback: when a chat `final` event has no renderable assistant content but includes envelope `errorMessage`, render the formatted error text instead of collapsing to `"(no output)"`, preserving actionable failure context in-session. (#14687) Thanks @Mquarmoc. diff --git a/src/slack/monitor/message-handler.app-mention-race.test.ts b/src/slack/monitor/message-handler.app-mention-race.test.ts index cfb44c8496e..c84b6514b43 100644 --- a/src/slack/monitor/message-handler.app-mention-race.test.ts +++ b/src/slack/monitor/message-handler.app-mention-race.test.ts @@ -121,6 +121,112 @@ describe("createSlackMessageHandler app_mention race handling", () => { expect(dispatchPreparedSlackMessageMock).toHaveBeenCalledTimes(1); }); + it("allows app_mention while message handling is still in-flight, then keeps later duplicates deduped", async () => { + let resolveMessagePrepare: ((value: unknown) => void) | undefined; + const messagePrepare = new Promise((resolve) => { + resolveMessagePrepare = resolve; + }); + prepareSlackMessageMock.mockImplementation(async ({ opts }) => { + if (opts.source === "message") { + return messagePrepare; + } + return { ctxPayload: {} }; + }); + + const handler = createSlackMessageHandler({ + ctx: { + cfg: {}, + accountId: "default", + app: { client: {} }, + runtime: {}, + markMessageSeen: createMarkMessageSeen(), + } as Parameters[0]["ctx"], + account: { accountId: "default" } as Parameters< + typeof createSlackMessageHandler + >[0]["account"], + }); + + const messagePending = handler( + { type: "message", channel: "C1", ts: "1700000000.000150", text: "hello" } as never, + { source: "message" }, + ); + await Promise.resolve(); + + await handler( + { + type: "app_mention", + channel: "C1", + ts: "1700000000.000150", + text: "<@U_BOT> hello", + } as never, + { source: "app_mention", wasMentioned: true }, + ); + + resolveMessagePrepare?.(null); + await messagePending; + + await handler( + { + type: "app_mention", + channel: "C1", + ts: "1700000000.000150", + text: "<@U_BOT> hello", + } as never, + { source: "app_mention", wasMentioned: true }, + ); + + expect(prepareSlackMessageMock).toHaveBeenCalledTimes(2); + expect(dispatchPreparedSlackMessageMock).toHaveBeenCalledTimes(1); + }); + + it("suppresses message dispatch when app_mention already dispatched during in-flight race", async () => { + let resolveMessagePrepare: ((value: unknown) => void) | undefined; + const messagePrepare = new Promise((resolve) => { + resolveMessagePrepare = resolve; + }); + prepareSlackMessageMock.mockImplementation(async ({ opts }) => { + if (opts.source === "message") { + return messagePrepare; + } + return { ctxPayload: {} }; + }); + + const handler = createSlackMessageHandler({ + ctx: { + cfg: {}, + accountId: "default", + app: { client: {} }, + runtime: {}, + markMessageSeen: createMarkMessageSeen(), + } as Parameters[0]["ctx"], + account: { accountId: "default" } as Parameters< + typeof createSlackMessageHandler + >[0]["account"], + }); + + const messagePending = handler( + { type: "message", channel: "C1", ts: "1700000000.000175", text: "hello" } as never, + { source: "message" }, + ); + await Promise.resolve(); + + await handler( + { + type: "app_mention", + channel: "C1", + ts: "1700000000.000175", + text: "<@U_BOT> hello", + } as never, + { source: "app_mention", wasMentioned: true }, + ); + + resolveMessagePrepare?.({ ctxPayload: {} }); + await messagePending; + + expect(prepareSlackMessageMock).toHaveBeenCalledTimes(2); + expect(dispatchPreparedSlackMessageMock).toHaveBeenCalledTimes(1); + }); + it("keeps app_mention deduped when message event already dispatched", async () => { prepareSlackMessageMock.mockResolvedValue({ ctxPayload: {} }); diff --git a/src/slack/monitor/message-handler.ts b/src/slack/monitor/message-handler.ts index 7ad7d792bc1..02961dd16c9 100644 --- a/src/slack/monitor/message-handler.ts +++ b/src/slack/monitor/message-handler.ts @@ -144,14 +144,18 @@ export function createSlackMessageHandler(params: { }); const seenMessageKey = buildSeenMessageKey(last.message.channel, last.message.ts); if (!prepared) { - const hasMessageSource = entries.some((entry) => entry.opts.source === "message"); - const hasAppMentionSource = entries.some((entry) => entry.opts.source === "app_mention"); - if (seenMessageKey && hasMessageSource && !hasAppMentionSource) { - rememberAppMentionRetryKey(seenMessageKey); - } return; } if (seenMessageKey) { + pruneAppMentionRetryKeys(Date.now()); + if (last.opts.source === "app_mention") { + // If app_mention wins the race and dispatches first, drop the later message dispatch. + appMentionDispatchedKeys.set(seenMessageKey, Date.now() + APP_MENTION_RETRY_TTL_MS); + } else if (last.opts.source === "message" && appMentionDispatchedKeys.has(seenMessageKey)) { + appMentionDispatchedKeys.delete(seenMessageKey); + appMentionRetryKeys.delete(seenMessageKey); + return; + } appMentionRetryKeys.delete(seenMessageKey); } if (entries.length > 1) { @@ -171,6 +175,7 @@ export function createSlackMessageHandler(params: { const threadTsResolver = createSlackThreadTsResolver({ client: ctx.app.client }); const pendingTopLevelDebounceKeys = new Map>(); const appMentionRetryKeys = new Map(); + const appMentionDispatchedKeys = new Map(); const pruneAppMentionRetryKeys = (now: number) => { for (const [key, expiresAt] of appMentionRetryKeys) { @@ -178,6 +183,11 @@ export function createSlackMessageHandler(params: { appMentionRetryKeys.delete(key); } } + for (const [key, expiresAt] of appMentionDispatchedKeys) { + if (expiresAt <= now) { + appMentionDispatchedKeys.delete(key); + } + } }; const rememberAppMentionRetryKey = (key: string) => { @@ -209,7 +219,13 @@ export function createSlackMessageHandler(params: { return; } const seenMessageKey = buildSeenMessageKey(message.channel, message.ts); - if (seenMessageKey && ctx.markMessageSeen(message.channel, message.ts)) { + const wasSeen = seenMessageKey ? ctx.markMessageSeen(message.channel, message.ts) : false; + if (seenMessageKey && opts.source === "message" && !wasSeen) { + // Prime exactly one fallback app_mention allowance immediately so a near-simultaneous + // app_mention is not dropped while message handling is still in-flight. + rememberAppMentionRetryKey(seenMessageKey); + } + if (seenMessageKey && wasSeen) { // Allow exactly one app_mention retry if the same ts was previously dropped // from the message stream before it reached dispatch. if (opts.source !== "app_mention" || !consumeAppMentionRetryKey(seenMessageKey)) {