Skip to content

fix(slack): surface Block Kit content in fetched thread context - #29541

Closed
bpross wants to merge 3 commits into
NousResearch:mainfrom
bpross:slack-thread-parent-blocks
Closed

fix(slack): surface Block Kit content in fetched thread context#29541
bpross wants to merge 3 commits into
NousResearch:mainfrom
bpross:slack-thread-parent-blocks

Conversation

@bpross

@bpross bpross commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

SlackAdapter._fetch_thread_context only reads msg.get(\"text\") when building the thread parent / sibling text it passes to the agent. For bot-posted alerts (Honeycomb, PagerDuty, Datadog, GitHub bot, etc.) the text field is typically just the alert title; everything actionable -- the "View graph"/"View incident" URL, mrkdwn sections, threshold details -- lives in blocks and was dropped before reaching the agent.

That meant an agent invoked by an @-mention in an alert thread couldn't see what the alert was about, even though the adapter had already fetched the full message. Replies had to re-discover context the adapter already had, or fail outright when the URL was the only useful signal.

What changes

In _fetch_thread_context, when a message has blocks, append three things to msg_text (deduped against what's already there):

  1. Rich-text content via existing _extract_text_from_slack_blocks() (forwarded/quoted messages).
  2. Block Kit payload via existing _serialize_slack_blocks_for_agent() (section / actions / accessory shape).
  3. URLs via a new _extract_urls_from_slack_blocks() helper. The existing serializer deliberately strips url to keep the payload bounded and avoid blanket exposure; this helper is the targeted opt-in for the call site that actually needs URLs, walking url / image_url / external_url keys across all elements. Emitted as URLs: <comma-separated> so the agent doesn't have to parse JSON.

The if not msg_text: continue guard moves after block extraction so a bot-posted parent that puts everything in blocks and nothing in text isn't silently dropped.

No existing serialization semantics change: _serialize_slack_blocks_for_agent's allowlist is untouched, so other paths that use it stay redacted.

Tests

Two new tests in TestSlackThreadContext:

  • test_fetch_thread_context_extracts_block_kit_parent — bot parent with title in text plus a button URL in blocks; asserts the URL surfaces.
  • test_fetch_thread_context_includes_blocks_only_parent — bot parent with empty text and a URL embedded in mrkdwn; asserts the parent isn't dropped and the URL surfaces.

Full slack test module run locally:
```
python -m pytest tests/gateway/test_slack_approval_buttons.py tests/gateway/test_slack.py tests/gateway/test_slack_mention.py tests/gateway/test_slack_channel_skills.py
```
277 passed, 0 regressions.

Test plan

  • New tests cover the two missing paths (blocks-with-title and blocks-only)
  • Existing thread-context tests still pass (parent inclusion, self-bot exclusion, multi-workspace, empty-thread, API failure)
  • No change to _serialize_slack_blocks_for_agent's URL-stripping behavior on other paths

The Slack adapter's _fetch_thread_context only used msg.get("text") when
building the parent/sibling text for the thread-context cache. That field
typically contains just the alert title for bot-posted alerts (Honeycomb,
PagerDuty, Datadog, GitHub bot, etc.); the actionable payload --
"View graph"/"View incident" URLs, mrkdwn sections -- lives in `blocks`
and was dropped before reaching the agent. Replies in the thread had to
re-discover context the adapter already fetched, or fail when the URL was
the only useful signal.

The fix augments msg_text with three additions when `blocks` is present:

- _extract_text_from_slack_blocks() for rich_text content (forwarded
  messages, quoted text).
- _serialize_slack_blocks_for_agent() for the JSON view of section /
  actions / accessory blocks (already deliberately redacted; URLs
  stripped on purpose to keep the payload compact and bounded).
- A new _extract_urls_from_slack_blocks() helper that walks the blocks
  tree and emits the URLs the serializer omits. Buttons, link
  accessories, and image_url/external_url fields are the targets.
  Appended as `URLs: <comma-separated>` so the agent can see them
  without parsing JSON.

The blocks-only path also moves the `if not msg_text: continue` guard
*after* the block extraction so a bot-posted parent that puts everything
in `blocks` and nothing in `text` is no longer silently dropped.

Tests:
- test_fetch_thread_context_extracts_block_kit_parent: bot parent with
  title in `text` plus a button URL in `blocks`; asserts the URL surfaces
  in the context string.
- test_fetch_thread_context_includes_blocks_only_parent: bot parent with
  empty `text` and a URL embedded in mrkdwn; asserts the parent isn't
  dropped and the URL surfaces.

Full slack test module: 277 passed, 0 regressions.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter labels May 21, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the Block Kit-only parent case. The underlying defect remains on current main: plugins/platforms/slack/adapter.py:3788-3790 still drops historical messages with empty top-level text.

Problems

  • The patch targets gateway/platforms/slack.py, which was migrated to plugins/platforms/slack/adapter.py by 5600105; git apply --check confirms the current patch does not apply.
  • Adding _serialize_slack_blocks_for_agent() to historical context would permit up to 6,000 characters per message (plugins/platforms/slack/adapter.py:210-274) across the fetcher's 30-message default (:3692). Extract bounded display text and actionable URLs instead of attaching raw Block Kit JSON to every thread message.
  • _fetch_thread_parent_text() still reads only parent.get("text") in its cold-cache fallback (:3891), so it needs the same renderer to preserve reply_to_text parity.

Suggested changes

  • Port the behavior into the bundled Slack plugin and share one bounded message renderer across live input, thread context, and parent lookup. The related #33469 discussion and linked shared-renderer work are relevant for covering attachment-only parents too.

Automated hermes-sweeper review.

Comment thread gateway/platforms/slack.py Outdated
extras: list[str] = []
rich_text = _extract_text_from_slack_blocks(blocks).strip()
if rich_text and rich_text not in msg_text:
extras.append(rich_text)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid adding the 6,000-character JSON serializer to every historical message. _fetch_thread_context fetches up to 30 messages by default, so a direct port can place up to 180,000 characters of Block Kit payload into one thread prefill; extract bounded display text and actionable URLs instead.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 13, 2026
bpross added 2 commits July 13, 2026 06:53
Port to plugins/platforms/slack/adapter.py after migration commit
5600105 deleted gateway/platforms/slack.py.

Address hermes-sweeper review feedback:

- Replace the full _serialize_slack_blocks_for_agent() JSON dump (up to
  6000 chars/message × 30 messages = 180K chars) with a shared bounded
  _render_message_text() that extracts only display text and actionable
  URLs from section/header/context blocks plus rich_text and url keys.
- Share _render_message_text() across _fetch_thread_context and
  _fetch_thread_parent_text cold-cache fallback so reply_to_text parity
  is preserved.
- Add test_fetch_thread_parent_text_surfaces_block_urls for the
  cold-cache fallback path.

Tests: 329 passed, 0 regressions across slack test modules.
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
Bot-posted alerts (Honeycomb, PagerDuty, Datadog, GitHub bot, etc.) carry
their actionable content — section text, button URLs — in Block Kit
blocks, while the plain text field holds only the alert title.
_fetch_thread_context and _fetch_thread_parent_text only read
msg.get('text'), so that content never reached the agent.

Add a _render_message_text helper that merges top-level text with
readable block content, section/header/context text, actionable URLs,
and (folded in from #61261 during conflict resolution) legacy
attachment fields, and use it for thread-context and parent-text
rendering.

Salvaged from #29541.
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
Bot-posted alerts (Honeycomb, PagerDuty, Datadog, GitHub bot, etc.) carry
their actionable content — section text, button URLs — in Block Kit
blocks, while the plain text field holds only the alert title.
_fetch_thread_context and _fetch_thread_parent_text only read
msg.get('text'), so that content never reached the agent.

Add a _render_message_text helper that merges top-level text with
readable block content, section/header/context text, actionable URLs,
and (folded in from #61261 during conflict resolution) legacy
attachment fields, and use it for thread-context and parent-text
rendering.

Salvaged from #29541.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69316 — your commit was cherry-picked onto current main with your authorship preserved in git history: your _render_message_text helper is the backbone of the final fix (earliest submission in this cluster).

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 22, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Bot-posted alerts (Honeycomb, PagerDuty, Datadog, GitHub bot, etc.) carry
their actionable content — section text, button URLs — in Block Kit
blocks, while the plain text field holds only the alert title.
_fetch_thread_context and _fetch_thread_parent_text only read
msg.get('text'), so that content never reached the agent.

Add a _render_message_text helper that merges top-level text with
readable block content, section/header/context text, actionable URLs,
and (folded in from NousResearch#61261 during conflict resolution) legacy
attachment fields, and use it for thread-context and parent-text
rendering.

Salvaged from NousResearch#29541.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants