Skip to content

fix(slack): include alert attachments in thread context - #30765

Closed
simjak wants to merge 1 commit into
NousResearch:mainfrom
simjak:fix/slack-thread-alert-context
Closed

fix(slack): include alert attachments in thread context#30765
simjak wants to merge 1 commit into
NousResearch:mainfrom
simjak:fix/slack-thread-alert-context

Conversation

@simjak

@simjak simjak commented May 23, 2026

Copy link
Copy Markdown

Summary

  • render legacy Slack attachment fields into agent-visible message text
  • include attachment-derived parent alert details in fetched thread context and reply_to_text
  • add regression coverage for GitHub/Terraform-style alerts whose parent message text is empty

Test Plan

  • /home/agent/.hermes/hermes-agent/venv/bin/python -m pytest tests/gateway/test_slack.py::TestSlackReplyToText -q -o 'addopts=' — 4 passed
  • /home/agent/.hermes/hermes-agent/venv/bin/python -m pytest tests/gateway/test_slack.py -q -o 'addopts=' — 188 passed

Context

GitHub deployment/Terraform drift alerts can store all useful details in Slack legacy attachments (pretext, fallback, fields) while leaving message.text empty. When Hermes is mentioned in that alert thread, the Slack adapter now fetches and renders those attachment fields so the agent sees status, commit, workflow, and deployment target context.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The underlying bug is still present on current main: plugins/platforms/slack/adapter.py:3788 skips attachment-only thread messages after reading only msg["text"], and :3891 does the same for cold reply_to_text lookup.

Problems

  • The PR changes gateway/platforms/slack.py, but Slack was migrated to plugins/platforms/slack/adapter.py by 5600105478ffde29d7566b45421b100eaa29c4ef. The submitted production diff therefore does not modify the active adapter.

Suggested changes

  • Port the renderer and the two thread-context call sites to plugins/platforms/slack/adapter.py.
  • Preserve the current live-message behavior that exposes Block Kit payloads only for the current event, not historical thread context; PR fix(gateway): render Slack attachment thread context #32022 is a related open implementation covering that distinction.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #69316 (merged): targeted the pre-plugin-migration file; covered by the consolidated helper.

Thanks for digging into this — the consolidated fix stands on the cluster's collective analysis, and your work is credited in #69316's summary.

@teknium1 teknium1 closed this Jul 22, 2026
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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants