Skip to content

fix(tools): treat Slack channel IDs as explicit send_message targets#2897

Open
seichris wants to merge 1 commit intoNousResearch:mainfrom
seichris:fix/slack-send-message-targets
Open

fix(tools): treat Slack channel IDs as explicit send_message targets#2897
seichris wants to merge 1 commit intoNousResearch:mainfrom
seichris:fix/slack-send-message-targets

Conversation

@seichris
Copy link

@seichris seichris commented Mar 25, 2026

What does this PR do?

Fixes Slack send_message target parsing so explicit channel IDs are treated as explicit destinations instead of falling back to the home DM. It also normalizes Slack channel mention syntax before resolution and adds a regression test for the channel-ID path.

Related Issue

No issue filed.

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

  • Updated tools/send_message_tool.py so Slack targets accept raw channel IDs like slack:C0AD2A7C4G1 as explicit targets.
  • Added Slack mention normalization (<#CHANNELID|name>) and stripped / topic N suffixes before Slack target parsing.
  • Updated the send_message target schema description to document Slack channel IDs and mention-style targets.
  • Added a regression test in tests/tools/test_send_message_tool.py covering explicit Slack channel IDs.

How to Test

  1. Run source venv/bin/activate && python -m pytest tests/tools/test_send_message_tool.py -q.
  2. Call send_message_tool with target="slack:C0AD2A7C4G1" and verify _send_to_platform(...) is called with chat ID C0AD2A7C4G1 rather than a home DM channel.
  3. Optionally verify Slack mention normalization with a target like <#C0AD2A7C4G1|agents-updates>.

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 26.3.1

Note: I ran source venv/bin/activate && python -m pytest tests/ -q. On this branch it finished with 2 failed, 5950 passed, 175 skipped; the two failures reproduce on clean main as well:

  • tests/agent/test_prompt_builder.py::TestBuildContextFilesPrompt::test_claude_md_uppercase_takes_priority
  • tests/test_real_interrupt_subagent.py::TestRealSubagentInterrupt::test_interrupt_child_during_api_call

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

  • source venv/bin/activate && python -m pytest tests/tools/test_send_message_tool.py -q -> 20 passed, 3 warnings in 4.26s
  • Manual patched invocation of send_message_tool({'action': 'send', 'target': 'slack:C0AD2A7C4G1', 'message': 'hello'}) passed and called _send_to_platform(..., 'C0AD2A7C4G1', ...).

@seichris seichris force-pushed the fix/slack-send-message-targets branch from 178fd3f to 77b04af Compare March 25, 2026 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant