Skip to content

fix(slack): open DMs for user send targets - #19237

Closed
davesecops wants to merge 1 commit into
NousResearch:mainfrom
davesecops:fix/slack-dm-user-targets
Closed

fix(slack): open DMs for user send targets#19237
davesecops wants to merge 1 commit into
NousResearch:mainfrom
davesecops:fix/slack-dm-user-targets

Conversation

@davesecops

@davesecops davesecops commented May 3, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds Slack send_message support for direct-message user targets. Hermes already supports Slack conversation IDs (C..., G..., D...), but user-oriented targets require opening a DM first because Slack chat.postMessage only accepts conversation IDs.

This PR makes these target forms work:

  • slack:U123ABCDEF
  • slack:@alice
  • slack:<@U123ABCDEF>

For user targets, Hermes now calls conversations.open to get a D... DM conversation before posting.

Related Issue

Fixes #19236

Type of Change

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

Changes Made

  • tools/send_message_tool.py
    • Parse Slack user IDs (U...) as explicit internal user targets.
    • Parse Slack @username targets as explicit internal username targets.
    • Parse Slack mention form (<@U...>) as an explicit user target.
    • Resolve username targets with users.list.
    • Open user targets with conversations.open before chat.postMessage.
    • Return a clear error when a username target is missing or ambiguous.
  • tests/tools/test_send_message_tool.py
    • Add parser regression tests for Slack user targets.
    • Add _send_slack tests for user-ID DM opening, username resolution, and ambiguous username handling.

How to Test

  1. Run targeted tests:

    venv/bin/python -m pytest tests/tools/test_send_message_tool.py tests/gateway/test_channel_directory.py -q -o 'addopts='

  2. Verify targeted result:

    133 passed

  3. Optional live Slack validation with a configured app that has chat:write, im:write, and users:read:

    send_message(action="send", target="slack:@alice", message="hello")

    Expected: Hermes resolves the user, opens a DM, and sends successfully.

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
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q 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

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Targeted test output:

133 passed, 2 warnings

Full-suite note:

A local full pytest tests/ -q -o 'addopts=' run was attempted but did not complete within 10 minutes in this development environment. The targeted tool/gateway regression tests for this change pass.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets platform/slack Slack app adapter labels May 3, 2026
@alt-glitch alt-glitch mentioned this pull request May 3, 2026
Closed
@davesecops
davesecops force-pushed the fix/slack-dm-user-targets branch 2 times, most recently from c20ef73 to 912f2e2 Compare May 7, 2026 01:48
@davesecops
davesecops force-pushed the fix/slack-dm-user-targets branch from 912f2e2 to 97ad3b6 Compare May 12, 2026 20:47
@teknium1

teknium1 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused Slack DM-target implementation. The requested behavior is still needed on current main, but this branch needs a plugin-era port before it can be salvaged.

Problems

  • tools/send_message_tool.py:372-383 resolves non-explicit targets through the channel directory before the existing U→DM block at tools/send_message_tool.py:467-487. _parse_target_ref() marks U… non-explicit at tools/send_message_tool.py:564, while the Slack directory only enumerates channels plus historical DMs (gateway/channel_directory.py:217-276). An unlisted user ID therefore cannot reach conversations.open.
  • The PR changes _send_slack, but current main moved that helper into plugins/platforms/slack/adapter.py:_standalone_send (tools/send_message_tool.py:1428-1429). Slack dispatch now uses the platform registry at tools/send_message_tool.py:1051-1062, so the implementation and its _send_slack tests must be ported.

Suggested changes

  • Salvage the parser and user/DM resolution against the current Slack plugin seam, ensuring U…, @username, and <@U…> bypass channel-directory lookup.
  • Rewrite the tests around _handle_send and/or the registered standalone sender rather than the removed helper.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@teknium1 teknium1 mentioned this pull request Jul 22, 2026
Merged
teknium1 added a commit that referenced this pull request Jul 23, 2026
@teknium1
fix(slack): resolve bare user targets to DMs across all adapter send …
abe51b4 
…paths

Widen the #19237 send_message fix to the live adapter: send, _upload_file,
send_multiple_images, send_image, send_video, send_document,
send_exec_approval, send_slash_confirm, and send_clarify now route bare
Slack user IDs (U.../W...) through a shared _ensure_dm_conversation helper
before calling chat.postMessage / files_upload_v2, which reject user IDs.

This closes the gap in #17261 where an attachment worked when replying in
a thread but failed when directed at a user DM, and extends the DM-open
fallback to clarify/approval Block Kit prompts so gated actions can reach
a user directly.

Resolution uses the workspace-scoped client (multi-workspace installs open
the DM with the right bot token), caches per (team, user), and records the
opened D... channel in the channel→team map. On failure the original
target passes through so the downstream API call surfaces the real Slack
error.

Fixes #17261
Refs #19236
teknium1 added a commit that referenced this pull request Jul 23, 2026
@teknium1
fix(slack): resolve bare user targets to DMs across all adapter send …
994a405 
…paths

Widen the #19237 send_message fix to the live adapter: send, _upload_file,
send_multiple_images, send_image, send_video, send_document,
send_exec_approval, send_slash_confirm, and send_clarify now route bare
Slack user IDs (U.../W...) through a shared _ensure_dm_conversation helper
before calling chat.postMessage / files_upload_v2, which reject user IDs.

This closes the gap in #17261 where an attachment worked when replying in
a thread but failed when directed at a user DM, and extends the DM-open
fallback to clarify/approval Block Kit prompts so gated actions can reach
a user directly.

Resolution uses the workspace-scoped client (multi-workspace installs open
the DM with the right bot token), caches per (team, user), and records the
opened D... channel in the channel→team map. On failure the original
target passes through so the downstream API call surfaces the real Slack
error.

Fixes #17261
Refs #19236
@teknium1

teknium1 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Merged via #69482 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your users.list + conversations.open DM resolution was reworked onto the post-#41112 architecture with your authorship, fixing #19236.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@teknium1
fix(slack): resolve bare user targets to DMs across all adapter send …
dc6665f 
…paths

Widen the NousResearch#19237 send_message fix to the live adapter: send, _upload_file,
send_multiple_images, send_image, send_video, send_document,
send_exec_approval, send_slash_confirm, and send_clarify now route bare
Slack user IDs (U.../W...) through a shared _ensure_dm_conversation helper
before calling chat.postMessage / files_upload_v2, which reject user IDs.

This closes the gap in NousResearch#17261 where an attachment worked when replying in
a thread but failed when directed at a user DM, and extends the DM-open
fallback to clarify/approval Block Kit prompts so gated actions can reach
a user directly.

Resolution uses the workspace-scoped client (multi-workspace installs open
the DM with the right bot token), caches per (team, user), and records the
opened D... channel in the channel→team map. On failure the original
target passes through so the downstream API call surfaces the real Slack
error.

Fixes NousResearch#17261
Refs NousResearch#19236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/tools Tool registry, model_tools, toolsets 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Slack send_message cannot open DMs for user targets

3 participants

@davesecops @teknium1 @alt-glitch