Skip to content

fix(relay): route async Slack messages to correct channel instead of DMs - #1845

Merged
PierreLeGuen merged 2 commits into
stagingfrom
fix/slack-async-message-routing
Mar 31, 2026
Merged

PierreLeGuen merged 2 commits into
stagingfrom
fix/slack-async-message-routing

Conversation

@PierreLeGuen

Copy link
Copy Markdown
Contributor

Summary

  • routing_target_from_metadata now extracts channel_id from Slack relay metadata, so proactive broadcast() calls target the originating channel (e.g. C088K6C3SQZ) instead of falling back to sender_id (user's DM U07PQGFUDQV)
  • Lightweight routine JobContext carries notify_channel/notify_user/owner_id in metadata — previously ..Default::default() left it null, losing all delivery routing
  • Routine creation auto-captures source channel/target from ctx.metadata when the LLM omits delivery.channel/delivery.user
  • Message tool parameter descriptions clarified: channel = transport name (slack-relay), target = Slack channel/user ID
  • IronClaw proxy_provider now checks Slack ok=false and surfaces errors (e.g. invalid_thread_ts) instead of silently succeeding

Evidence from relay logs

target_channel="U07PQGFUDQV"  ← wrong: user ID, not channel
slack_channel="D07Q6306AN5"    ← delivered to DM
slack_error="invalid_thread_ts" ← routine notification silently failed

After this fix, routing_target returns C088K6C3SQZ (the Slack channel) and routine creation captures it as notify.user.

Test plan

  • 8 regression tests added (routing_target, routine metadata, delivery defaults)
  • 3970 unit tests pass
  • Zero clippy warnings
  • Deploy and verify Ping routine sends to correct channel
  • Verify cross-channel message(channel: "slack-relay", target: "C088K6C3SQZ") works

🤖 Generated with Claude Code

Fixes three bugs causing async/cross-channel Slack messages to land in
DMs or fail silently:

1. routing_target_from_metadata now extracts channel_id for Slack relay
   messages, so proactive broadcasts target the originating channel
   instead of falling back to sender_id (user's DM)

2. Lightweight routine JobContext carries notify metadata (owner_id,
   notify_channel, notify_user) so the message tool can resolve the
   correct delivery target — previously ..Default::default() left
   metadata as null

3. Routine creation auto-captures source channel/target from
   ctx.metadata when the LLM omits delivery params, so routines
   created from a Slack channel know where to send results

Also:
- Clarified message tool channel/target parameter descriptions to
  prevent LLM confusion between transport names and Slack channel IDs
- IronClaw proxy_provider now checks Slack ok=false and surfaces
  errors instead of silently succeeding

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel Channel infrastructure scope: tool/builtin Built-in tools size: M 50-199 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: experienced 6-19 merged PRs labels Mar 31, 2026

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request improves notification routing and error handling for routines and Slack integrations. Key updates include passing notification metadata into lightweight routine contexts, adding channel_id to routing target extraction, and implementing explicit Slack API error handling in the relay client. The RoutineCreateTool now correctly falls back to context metadata for delivery configuration when parameters are omitted. Feedback was provided to use a more idiomatic approach for checking the Slack API response status.


// Slack API always returns HTTP 200 but signals errors via {"ok": false}.
// Surface these as relay errors so callers get actionable feedback.
if json.get("ok") == Some(&serde_json::Value::Bool(false)) {

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.

medium

The check for the ok field in the Slack API response can be written more idiomatically using .as_bool().

Suggested change
if json.get("ok") == Some(&serde_json::Value::Bool(false)) {
if json["ok"].as_bool() == Some(false) {

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@PierreLeGuen
PierreLeGuen merged commit f441d78 into staging Mar 31, 2026
14 checks passed
@PierreLeGuen
PierreLeGuen deleted the fix/slack-async-message-routing branch March 31, 2026 23:55
JZKK720 pushed a commit to JZKK720/ironclaw that referenced this pull request Apr 1, 2026
…DMs (nearai#1845)

* fix(relay): route async Slack messages to correct channel instead of DMs

Fixes three bugs causing async/cross-channel Slack messages to land in
DMs or fail silently:

1. routing_target_from_metadata now extracts channel_id for Slack relay
   messages, so proactive broadcasts target the originating channel
   instead of falling back to sender_id (user's DM)

2. Lightweight routine JobContext carries notify metadata (owner_id,
   notify_channel, notify_user) so the message tool can resolve the
   correct delivery target — previously ..Default::default() left
   metadata as null

3. Routine creation auto-captures source channel/target from
   ctx.metadata when the LLM omits delivery params, so routines
   created from a Slack channel know where to send results

Also:
- Clarified message tool channel/target parameter descriptions to
  prevent LLM confusion between transport names and Slack channel IDs
- IronClaw proxy_provider now checks Slack ok=false and surfaces
  errors instead of silently succeeding

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: apply cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
serrrfirat pushed a commit that referenced this pull request Apr 5, 2026
…DMs (#1845)

* fix(relay): route async Slack messages to correct channel instead of DMs

Fixes three bugs causing async/cross-channel Slack messages to land in
DMs or fail silently:

1. routing_target_from_metadata now extracts channel_id for Slack relay
   messages, so proactive broadcasts target the originating channel
   instead of falling back to sender_id (user's DM)

2. Lightweight routine JobContext carries notify metadata (owner_id,
   notify_channel, notify_user) so the message tool can resolve the
   correct delivery target — previously ..Default::default() left
   metadata as null

3. Routine creation auto-captures source channel/target from
   ctx.metadata when the LLM omits delivery params, so routines
   created from a Slack channel know where to send results

Also:
- Clarified message tool channel/target parameter descriptions to
  prevent LLM confusion between transport names and Slack channel IDs
- IronClaw proxy_provider now checks Slack ok=false and surfaces
  errors instead of silently succeeding

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: apply cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
…DMs (nearai#1845)

* fix(relay): route async Slack messages to correct channel instead of DMs

Fixes three bugs causing async/cross-channel Slack messages to land in
DMs or fail silently:

1. routing_target_from_metadata now extracts channel_id for Slack relay
   messages, so proactive broadcasts target the originating channel
   instead of falling back to sender_id (user's DM)

2. Lightweight routine JobContext carries notify metadata (owner_id,
   notify_channel, notify_user) so the message tool can resolve the
   correct delivery target — previously ..Default::default() left
   metadata as null

3. Routine creation auto-captures source channel/target from
   ctx.metadata when the LLM omits delivery params, so routines
   created from a Slack channel know where to send results

Also:
- Clarified message tool channel/target parameter descriptions to
  prevent LLM confusion between transport names and Slack channel IDs
- IronClaw proxy_provider now checks Slack ok=false and surfaces
  errors instead of silently succeeding

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: apply cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ironclaw-ci ironclaw-ci Bot mentioned this pull request Apr 8, 2026
@ironclaw-ci ironclaw-ci Bot mentioned this pull request Apr 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: experienced 6-19 merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: channel Channel infrastructure scope: tool/builtin Built-in tools size: M 50-199 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants