fix(slack): drop message_changed re-emits whose body is unchanged

[nikitaBarkov]

What does this PR do?

Stops Slack from turning one user message into two agent turns.

Slack re-dispatches message_changed for updates it makes to a message on its own, with the body byte-identical and nobody having touched the message. This is documented behavior: "This event can also sometimes be triggered by our automatic language detection, which can add or update language or locale information to the metadata for the message, prompting the event to be dispatched" (message_changed reference). The re-emit arrives about a second after the original — while the original is still being ingested.

Neither barrier in _handle_slack_message() stops it:

• The _processed_message_ts guard at the top of the message_changed branch is armed only at the very end of the method, after users.info, thread-context hydration and attachment downloads. That leg takes 0.2–2s, and the re-emit lands inside it, so the guard sees an empty map and lets the event through.
• The redelivery dedup below it keys this event under the synthetic _slack_changed_event_ts, not under the message ts — deliberately, otherwise an edit that adds a bot mention would be swallowed. So it does not match the original delivery either.

The event is then normalized into a plain message and handed to the gateway a second time. The gateway parks it as a follow-up and drains it as a brand-new turn: a second identical answer to the user, a second full API call, and a duplicated user turn in the transcript.

Evidence from a production workspace (one gateway session, five days of gateway.log plus the session DB):

• 319 distinct message ts reached the gateway; 40 of them reached it twice (12.5%), and all 40 went on through queued follow-up (FIFO) → Draining queued follow-up … as a new turn. One inbound message, two turns, every time.
• The arithmetic of the events is always the same: message + optional app_mention (suppressed by the dedup — same ts, same key) + message_changed (a different key — the one that gets through). If a proxy or a second listener were duplicating traffic, the app_mention twin would be dropped twice; it never is.
• 33 of the 40 second copies entered the handler while the first copy was still in ingress.
• Of 127 message_changed events in that window, 108 arrived less than 2s after the original (median 0.8s) — machine speed, no human involved. The only three that arrived minutes later (172s, 417s, 417s) were genuine edits.
• Every duplicated pair compared byte-identical, text and blocks.
• The re-emit correlates with message length (79% of re-emitted messages were long, against 10% at baseline) and not with links (9% vs 5%) or with mentioning the bot (50% vs 48%) — the signature of language detection, not of an unfurl and not of anything the app itself does.

Because the body is always identical, the fix does not have to reason about timing at all: record a digest of the rendered body before the first await, and drop a message_changed whose body is unchanged. An edit that genuinely changes the body — including one that adds a bot mention, in the text, the blocks or an authored attachment — passes this new guard, so the edited-in-mention behavior is untouched. (To be precise about the scope: once the original has finished ingress, the pre-existing _processed_message_ts guard from 49497bcddb still drops every message_changed for it, edits included. That behavior is on main today and this PR does not change it — see Notes.)

Related Issue

No linked issue — diagnosed from production logs, mechanism confirmed against the Slack documentation above. The same trigger has bitten other Slack integrations: n8n issue 23782 describes Slack's asynchronous locale enrichment re-triggering a bot reply for a message nobody edited.

Adjacent open PRs, complementary rather than overlapping:

• fix(slack): ignore metadata-only parent updates #83501 (ignore metadata-only parent updates) and fix(slack): prevent thread parent edit replays #73450 (prevent thread parent edit replays) classify hidden thread-parent message_changed events by comparing the event's own previous_message snapshot, and target replay after a restart. This PR is about the message the process is ingesting right now: it compares against the body this process actually saw, needs no previous_message (Slack's metadata re-emit does not have to carry a usable one), and closes the in-flight race those PRs do not touch. The guards are independent and compose — if both land, an unchanged body is simply dropped one check earlier.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• plugins/platforms/slack/adapter.py: new _slack_message_body_signature(message) — the part of a message body the agent is actually shown, as one value: the flat text plus the same renderings the inbound path uses for blocks (_extract_text_from_slack_blocks, _serialize_slack_blocks_for_agent) and for the legacy attachments (_extract_text_from_slack_attachments). Keying on text alone would silently drop a chat.update that rewrites a bot card while keeping a static fallback text, which under allow_bots=all is real content.
• plugins/platforms/slack/adapter.py: new _slack_authored_attachments(attachments) — only the attachments an author or an app set take part in the signature. The ones Slack generates itself while unfurling a link are recognised by the keys only it sets (_SLACK_GENERATED_ATTACHMENT_KEYS: is_msg_unfurl, from_url, original_url, app_unfurl_url, service_name) and are deliberately left out: Slack attaches them on its own, so counting them as a body change would hand its own re-emit a turn — exactly the bug being fixed.
• plugins/platforms/slack/adapter.py: new _slack_body_digest(body) (SHA-256, surrogatepass so an unpaired surrogate in a payload cannot raise) and the bounded self._seen_message_body_digest map (_SEEN_MESSAGE_BODY_DIGEST_MAX = 5000, evicted oldest-first through the existing _trim_oldest_dict_entries). Only the digest is stored — a rendered Block Kit payload can be 6 KB and never needs to stay in memory.
• plugins/platforms/slack/adapter.py: _remember_message_body(message_key, body) is called in _handle_slack_message() before the first await, which is the whole point — that is why a baseline exists while _processed_message_ts is still unarmed. It sits below the two synchronous filters that precede it (the redelivery dedup and the ignored-channel check), so neither a replay nor an ignored channel takes a slot. An empty body is never recorded, so a message carrying only files is never mistaken for "unchanged".
• plugins/platforms/slack/adapter.py: both maps are keyed by _workspace_event_id(team_id, ts), not by the bare Slack ts — a timestamp identifies a message only within one workspace, and the adapter already scopes its other markers that way. This covers _seen_message_body_digest and, on the same key, the pre-existing _processed_message_ts registry, on read and on write; the message_changed branch normalizes the routing fields before either guard so the id it resolves is the one the delivery path recorded under.
• plugins/platforms/slack/adapter.py: the message_changed branch gains a second guard — when the body digest matches the one recorded for that message, the event is dropped with one INFO line ([Slack] dropped message_changed with unchanged body ts=… channel=…), so a future duplicate report can be told apart from this class by the presence or absence of that line.
• tests/gateway/test_slack.py: seven new tests around a shared _IngressGate helper that holds users.info open so the second event genuinely races an in-flight ingress (asyncio.Event-driven, no polling). test_metadata_only_edit_during_ingress_routes_once (parametrized with and without the rich_text blocks a composed message really carries) and test_link_unfurl_during_ingress_routes_once assert a single handle_message; test_real_edit_during_ingress_still_routes, test_block_only_update_during_ingress_still_routes and test_attachment_only_update_during_ingress_still_routes assert two, for a changed text, changed blocks and a changed authored card under a static fallback text; test_same_ts_in_another_workspace_still_routes covers the same inner ts in another workspace against both maps — while the original is still in ingress and after it has been delivered; test_ignored_channel_body_is_not_remembered asserts the baseline is not recorded for an ignored channel.
• tests/gateway/test_slack_mention.py: the hand-built adapter stub gains the two new attributes.

No config key, no env var, no schema change, and nothing in gateway/ — no other platform's behavior moves.

How to Test

1. scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_mention.py — 200 passed, 0 failed on this branch.
2. Mutation check that the tests bind to the fix and not to timing: make the digest comparison never match (== "mutation-never-matches") and both parametrizations of test_metadata_only_edit_during_ingress_routes_once fail with the re-emit entered the ingress instead of being dropped; restore it and they pass. The tests asserting two turns stay green either way, which is the point — they prove the guard does not swallow real changes. Two more mutations pin the other half: drop attachments back out of the signature and test_attachment_only_update_during_ingress_still_routes fails; unscope either the read or the write of _processed_message_ts and test_same_ts_in_another_workspace_still_routes fails.
3. Live reproduction, no code changes needed: from a workspace whose locale differs from the language being typed, send a longer message that @mentions the bot where the adapter's ingress is slow (thread context, an attachment). On main, gateway.log shows two delivering event to gateway lines with the same ts, a queued follow-up drained as a new turn, and the user gets two identical answers. On this branch the second event ends at dropped message_changed with unchanged body and there is exactly one turn.
4. The behavior this must not break: edit a message the bot ignored so that it now @mentions the bot — the bot still answers exactly once (test_message_edit_with_new_mention_processed, unchanged, plus test_real_edit_during_ingress_still_routes for the racing variant).

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate — the closest are fix(slack): ignore metadata-only parent updates #83501 and fix(slack): prevent thread parent edit replays #73450, compared above; neither guards the in-ingress race
• My PR contains only changes related to this fix (three files, two commits — the fix and the review follow-up — branched off main)
• I've run pytest tests/ for the affected area and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15.6.1

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — N/A, no user-facing setting or documented behavior changes; the reasoning lives in the new helpers' docstrings
• I've updated cli-config.yaml.example if I added/changed config keys — N/A, no config keys
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) — pure Python, no platform-specific paths or primitives
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Notes

Deliberately out of scope, to keep the change on the confirmed mechanism:

• No barrier in the gateway (_queue_or_replace_pending_event). A cross-platform "this message_id is already the running turn" guard would cover more, but it drops inbound messages on a key the platforms do not all populate the same way — a worse failure than a duplicate if the key is wrong.
• No symmetric guard on plain message events: _processed_message_ts is already armed before handle_message() on that path, and a body-equality drop there could suppress a legitimate re-delivery.
• The pre-existing _processed_message_ts short circuit is left as it is. It drops every message_changed for a message that has finished ingress, genuine edits included — behavior introduced with the message_changed branch itself in 49497bcddb and live on main. Replacing it with a content/revision decision would make every later edit a new agent turn, which is a behavioral change for the edit flow rather than a fix for the duplicate-turn bug.
• Identity is scoped to workspace + message ts, not workspace + channel + ts. A Slack ts is already unique inside a workspace, and channel is not carried on every message_changed shape the adapter accepts, so adding it buys no discrimination and adds a way to miss the lookup and let a duplicate through.
• The baseline is arrival-ordered, not revision-ordered: if a message_changed is delivered before the original (reconnect replay), it records the edited body and the original then overwrites it. Both events already produce a turn on main in that ordering, so this is not a regression; closing it needs a stored Slack revision (edited.ts) and a version comparison, which is more machinery than the confirmed mechanism justifies.
• Trade-off worth naming: when Slack sends message_changed only to attach a link unfurl, the agent no longer sees that preview — by design, since the alternative is an entire extra turn per link. The URL itself is still in the message text and can be fetched.

[andrexibiza]

Exact-head review at 4ddebec11ca66bd392f3a21dd99443ecec7a1a6d.

Merge recommendation: changes required. The production diagnosis is convincing and recording a pre-await baseline is the right primitive. The current implementation, however, still conflates metadata replay with real edits in several normal Slack shapes.

[P1] The completed-message guard still suppresses every genuine later edit

The message_changed branch first does:

if original_message_ts and original_message_ts in self._processed_message_ts:
    return

That happens before the new body comparison. Once the original message has completed ingress, a text edit, block-only edit, or attachment-only edit is discarded unconditionally. The new test_block_only_update_during_ingress_still_routes holds the original inside users.info, so it proves only the narrow race window where _processed_message_ts is not yet armed; it does not prove the PR body's broader claim that a genuine body change still passes.

Replace the terminal timestamp-only short circuit with the content/revision decision. A completed original should suppress a message_changed only when the agent-visible body and revision semantics prove it is Slack's metadata-only re-emission.

[P1] The signature omits attachments that the adapter later shows to the agent

_slack_message_body_signature() hashes flat text and blocks only. Later in the same inbound path, attachments are rendered into agent-visible text from title, title_link/from_url, text, footer, and fallback (excluding only is_msg_unfurl). An attachment/card can therefore change while text and blocks remain identical, and this guard will drop it as unchanged.

Do not solve Slack-generated unfurls by excluding the entire attachment channel. Normalize the exact attachment fields the agent consumes and distinguish Slack-owned metadata enrichment from authored attachment/card changes. Add both an attachment-only real edit and a metadata-only unfurl re-emission test.

[P1] A bare Slack timestamp is not a canonical message identity

_seen_message_body_digest is adapter-wide and keyed only by message.ts. The rest of this adapter already scopes event/thread markers because timestamps can collide across workspaces; Slack message operations themselves identify a message by channel plus timestamp. A same-ts/same-body message in another workspace or channel can seed the digest and cause a legitimate message_changed to be dropped. Thread roots and replies also need to remain distinct.

Key the baseline by the full routing identity available at ingress: profile/route or adapter instance, team/workspace, channel, message ts, and thread root where relevant. Cover:

• same text with different message timestamps;
• same timestamp in different channels and workspaces;
• parent versus reply identities inside one thread;
• identical repeated user messages that are separate Slack messages.

[P1] The baseline is arrival-ordered, not revision-ordered

The map stores only a digest. If message_changed arrives first (reconnect/replay ordering) it records the edited body under the original ts; a later delivery of the original message then overwrites that baseline with the stale body and can itself route as a turn because its dedup key differs from the edit event's synthetic timestamp. A later metadata re-emission of the edited body then appears changed again.

Store a monotonic Slack revision alongside the digest (edited.ts/event timestamp plus the original generation), and advance the baseline only when the incoming version is newer. The first-event/edit matrix needs both orderings and concurrent completion, not only original-first with an artificial ingress gate.

The fix should remain narrowly targeted: deduplicate only a proven metadata-only re-emission of the same canonical message revision. It must not deduplicate by text, by bare timestamp, or by arrival order.

[nikitaBarkov]

Thanks for the review — this was a careful read and two of the four points were real holes I'd left open. Both are fixed in 5303522b1d; the PR description is updated to match. Point by point:

[P1] Signature omits attachments — fixed

You're right, and the failure mode was exactly as described: title, title_link/from_url, text, footer, fallback are rendered into the agent's text further down the same inbound path, so a card rewritten under a static fallback text compared equal and got dropped.

_slack_message_body_signature() now folds the attachments in, rendered by the very function the inbound path uses (_extract_text_from_slack_attachments). The Slack-owned enrichment is separated rather than the whole channel being excluded: _slack_authored_attachments() drops entries carrying the keys only Slack sets on an unfurl it generated — is_msg_unfurl, from_url, original_url, app_unfurl_url, service_name — and keeps everything an author or an app set. Both tests you asked for are in: test_attachment_only_update_during_ingress_still_routes (authored card changes → 2 turns) and test_link_unfurl_during_ingress_routes_once (Slack attaches a preview → 1 turn). Mutation-checked: take attachments back out of the signature and the first one fails.

[P1] A bare timestamp is not an identity — fixed

Also correct, and it was worse than the digest map alone: _processed_message_ts had the same problem, so a delivered message in one workspace could swallow a genuine message_changed with the same inner ts in another. Both maps are now keyed by _workspace_event_id(team_id, ts), on read and on write, and the message_changed branch normalizes the routing fields before either guard so the id it resolves is the one the delivery path recorded under. test_same_ts_in_another_workspace_still_routes covers both halves — the same ts in another workspace while the original is still in ingress (digest map) and after it has completed (registry map) — and mutation-checked in both directions (unscope the read, or the write, and it fails).

On the wider identity you sketched: I stopped at workspace + ts deliberately. A Slack ts is already unique within a workspace, so channel and thread-root add no discrimination on top of it — they only add ways for the lookup to miss (channel is not carried on every message_changed shape this handler accepts) and a missed lookup is a duplicate turn, the bug this PR exists to stop. Same for the "identical repeated user messages" case: those are separate Slack messages with different ts, so they never share a key. If you'd still like channel folded in, I'm happy to add it, but I'd rather do it with an example of a shape where workspace + ts collides.

[P1] The completed-message guard suppresses later edits — acknowledged, out of scope

Fair as a fact, and my PR description overstated things — I've corrected that sentence. But that return is not introduced here: it arrived with the message_changed branch itself in 49497bcddb (fix(slack): handle edited-in bot mentions) and behaves identically on main today. Replacing it with a content/revision decision would make every later edit of an answered message a brand-new agent turn — a behavioral change to the edit flow, with its own duplicate-reply risk, not a fix for the duplicate-turn bug this PR targets. I'd rather that lands as its own change, on its own reasoning, than ride along here.

[P1] The baseline is arrival-ordered — acknowledged, out of scope

The ordering you describe is real, and I don't have it in the production data behind this PR (127 message_changed events, all after their original, median 0.8s). In that ordering main already produces a turn for both events, so this patch doesn't make it worse — it just doesn't make it better. Storing edited.ts and advancing the baseline only on a newer revision is a reasonable next step, but it is machinery for an unobserved case, and the guideline I'm working to is to keep the fix on the mechanism the logs actually prove. Written up in the PR's Notes so it isn't lost.

Verification after the follow-up: scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_mention.py → 200 passed, 0 failed, plus the three mutation checks above.