fix(slack): stop duplicating one authored message on entities, unfurls and the Block Kit dump - #25
Merged
Merged
Conversation
Two more ways one authored Slack message reached the agent twice, on top of the formatting normalization from #22. Slack escapes `&`, `<` and `>` in the flat `event.text` but leaves the `blocks` payload raw, so the two representations of the same message never compared equal once a URL carried query parameters. A thread "Copy link" always carries `?thread_ts=...&cid=...`, so linking a thread duplicated the message every time. The dedupe path now unescapes before canonicalizing links, and the `URLs:` list in hydration compares against the unescaped text — otherwise a raw block URL was not found in the escaped message and was re-listed as if new. Permalink unfurls (`is_msg_unfurl`) carry the linked message's own body. `_handle_slack_message()` already skipped them; `_extract_text_from_slack_attachments()` did not, so thread and parent hydration appended a second copy of a message the agent was already reading. Regular attachments are untouched — alert bots put their entire content there.
`_serialize_slack_blocks_for_agent()` inlines a redacted JSON view of the message's blocks so the agent can inspect structure it cannot otherwise read. NousResearch#11426 introduced it "for non-`rich_text` messages", but the guard bailed out only when *every* block was `rich_text`, so a single bot `section` alongside the user's own text dumped that text as well. `rich_text` is the authored message, already rendered into the message text by `_extract_text_from_slack_blocks()`. Repeating it is duplication on its own, and because the scalar allowlist drops `url` by design, the repeat arrives as the same sentence with every link deleted and a double space where the link used to be. Only non-`rich_text` blocks are serialized now; `section`, `actions` and accessory blocks are still described in full.
20 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Closes two more ways one authored Slack message reaches the agent twice, on top of the formatting normalization from #22.
Slack delivers the same message in both
event.text(flat) andevent.blocks(structured). The adapter renders the blocks so quotes, lists and forwards are not lost, and treats that rendering as additional content whenever it does not compare equal to the flat text. #22 taught that comparison about inline code, styles, links, fenced code and bot mentions. Two gaps remained, plus one duplication that does not go through the comparison at all.1. HTML entities. Slack escapes
&,<and>in the flat text only and leaves the block payload raw, so the two copies of the same message never compared equal once a URL carried query parameters. A thread "Copy link" always carries?thread_ts=…&cid=…, which is why pasting a thread link duplicated the message every time while a plain permalink was fine. Verified onmainby calling the helpers directly: the thread-link case returned the whole message as "additional text"; escaping both sides returned"".2. Permalink unfurls in hydration.
is_msg_unfurlattachments carry the linked message's own body._handle_slack_message()already skipped them,_extract_text_from_slack_attachments()did not — so thread and parent hydration appended a second copy of a message the agent was already reading. One code path ignored what the other appended.3. The Block Kit payload dump.
_serialize_slack_blocks_for_agent()inlines a redacted JSON view of the blocks so the agent can inspect structure it cannot otherwise read. NousResearch#11426 introduced it "for non-rich_textmessages", but the guard bailed out only when every block wasrich_text— so one botsectionnext to the user's own text dumped that text too.rich_textis the authored message, already rendered into the message text, and the scalar allowlist dropsurlby design: the repeat arrives as the same sentence with every link deleted and a double space where the link used to be. That is the exact shape reported in a live workspace.Related Issue
No fork issue. Continuation of #22 (formatting normalization). Upstream counterpart: NousResearch#80240 — the same three fixes are squashed into that PR on top of the #22 commit. Prior upstream work: NousResearch#66204 and NousResearch#70191 (link representation), NousResearch#11426 (introduced the dump narrowed here).
Type of Change
Changes Made
Two commits, each self-contained and green on its own.
fix(slack): stop duplicating text on escaped entities and unfurlsplugins/platforms/slack/adapter.py: new_unescape_slack_entities();_normalize_slack_text_for_dedupe()unescapes before canonicalizing links, so both sides of the comparison see the same angle brackets and ampersands.plugins/platforms/slack/adapter.py:_extract_text_from_slack_attachments()skipsis_msg_unfurl, matching the live inbound path. Regular attachments are untouched — alert bots put their entire content there.plugins/platforms/slack/adapter.py: theURLs:list in_render_message_text()compares against the unescaped message text. Without this a raw block URL was not found in the escaped text and was re-listed as a third copy; this one only surfaced under test.fix(slack): keep the authored message out of the Block Kit payload dumpplugins/platforms/slack/adapter.py:_serialize_slack_blocks_for_agent()serializes only non-rich_textblocks, restoring the scope fix: inline Slack block and attachment context NousResearch/hermes-agent#11426 stated.section,actionsand accessory blocks are still described in full.Tests —
tests/gateway/test_slack.py, newTestSlackAuthoredTextDeduplication(12 tests):?thread_ts=…&cid=…,AT&T,<div>, labelled link;_handle_slack_message()(live inbound) and_render_message_text()(hydration);is_msg_unfurlskipped during hydration;sectionis mixed in;rich_text_quoteis still appended, a regular alert attachment is still surfaced, andsection+actionsbot blocks are still described.How to Test
?thread_ts=…&cid=…) into a bot thread. Previously the message arrived twice — once with the link, once without; now it arrives once.section+actions) — the agent still sees the block payload.scripts/run_tests.sh tests/gateway/test_slack.py— 210 passed (207 on the first commit alone).scripts/run_tests.sh tests/gateway/— 4919 passed. The 6 failures are pre-existing and environmental (missing optional XML dep for wecom, Linux-only abstract sockets on macOS) and reproduce onmainwithout this branch.Checklist
Code
Documentation & Housekeeping
cli-config.yaml.example— N/A, no config changesCONTRIBUTING.md/AGENTS.md— N/A, no architecture or workflow changesNotes
Out of scope on purpose: repeated delivery of the same message as a second event. Slack re-sends a message as
message_changedonce it attaches a link preview, and_processed_message_ts[ts]is written only at the very end of_handle_slack_message(), after 17awaits — so a slow first pass can let the unfurl copy through the guard. That is a different bug class, it is not reproduced from logs yet, and a naive fix would break the "an @mention added by edit still wakes the bot once" behavior. It needs its own change, and it collides with open upstream PR NousResearch#73450.