Skip to content

fix: preserve threaded Slack delivery for cron updates - #3424

Closed
ogiberstein wants to merge 2 commits into
NousResearch:mainfrom
ogiberstein:fix/slack-threaded-cron-delivery
Closed

fix: preserve threaded Slack delivery for cron updates#3424
ogiberstein wants to merge 2 commits into
NousResearch:mainfrom
ogiberstein:fix/slack-threaded-cron-delivery

Conversation

@ogiberstein

@ogiberstein ogiberstein commented Mar 27, 2026

Copy link
Copy Markdown

Summary

  • pass Slack thread IDs through the direct send path so cron deliveries use thread_ts
  • preserve HERMES_SESSION_THREAD_ID when scheduling cronjobs with deliver=origin
  • add regression tests for both the Slack sender and cronjob origin capture

Root cause

There were two separate breaks affecting threaded Slack cron delivery:

  1. tools/send_message_tool.py ignored thread_id for Slack direct sends, so scheduler deliveries posted to the channel root instead of the thread.
  2. tools/cronjob_tools.py did not persist HERMES_SESSION_THREAD_ID into the stored cronjob origin, so new jobs created from a Slack thread lost the thread context immediately.

Verification

  • source venv/bin/activate && python -m pytest tests/tools/test_send_message_tool.py tests/tools/test_cronjob_tools.py tests/cron/test_scheduler.py -q
  • Result: 37 passed

Notes

This was found while debugging Slack thread noise from recurring strategy-monitor cronjobs.

@alt-glitch alt-glitch added type/bug Something isn't working platform/slack Slack app adapter comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists labels May 2, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to closed #14146 (Slack thread_ts support) and open #9394 (Slack thread context). Also overlaps with #11823 (delivery metadata propagation).

@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to closed #14146 and open #9394, #11823.

@teknium1

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: this PR's behavior is already implemented on current main.

Evidence:

  • tools/cronjob_tools.py:269-285 captures HERMES_SESSION_THREAD_ID into the stored cronjob origin as thread_id.
  • cron/scheduler.py:500-506 preserves origin.get("thread_id") when resolving deliver=origin.
  • tools/send_message_tool.py:784 passes Slack thread_id through as thread_ts, and tools/send_message_tool.py:1064-1079 includes thread_ts in the Slack chat.postMessage payload.
  • The origin-capture side landed in 20f381cfb67d02bb21ff1a4a088af720bb6c8807; the Slack direct-send side landed in 74e845c000de1f32cd325758407ea706f18b7c36 and is contained in v2026.6.5.

The maintainer comments linking #14146, #9394, and #11823 are consistent with this: main now has the concrete Slack thread_ts and cron origin metadata propagation this PR was adding.

@teknium1 teknium1 closed this Jun 10, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants