Skip to content

feat(cron): optional threaded delivery (summary parent + detail in thread) - #44537

Open
just-code-in wants to merge 1 commit into
NousResearch:mainfrom
just-code-in:feat/threaded-cron-delivery
Open

feat(cron): optional threaded delivery (summary parent + detail in thread)#44537
just-code-in wants to merge 1 commit into
NousResearch:mainfrom
just-code-in:feat/threaded-cron-delivery

Conversation

@just-code-in

Copy link
Copy Markdown

What does this PR do?

Cron reports currently post to channels as a flat burst — the whole report is sent in one send() call and the platform adapter splits it into multiple top-level messages. This adds an opt-in threaded delivery mode for platforms that support it (Slack initially):

  • cron.threaded_delivery: true (config, default false) posts a compact parent message — the report's TL;DR:/TLDR:/Summary: first paragraph (marker stripped), or the first paragraph as fallback — and the full report as replies in its thread.
  • Per-job opt-out with "thread": false on any job definition.
  • Reports too short to split, non-Slack platforms, the standalone delivery path, and any threading failure all use the existing flat path unchanged. Threading is strictly best-effort: a report is never lost to it.
  • The cron wrapper's job-id line and management footer move into the thread detail, keeping the channel-visible parent compact.

Related Issue

Fixes #

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • cron/scheduler.py: add _split_summary(), _threaded_delivery_enabled(), _send_threaded_via_adapter(); wire threaded path into _deliver_result() behind platform + config gate
  • tests/cron/test_threaded_delivery.py: 24 new tests covering split semantics, config gate, threaded sequence, fallbacks, media-in-thread, origin-thread passthrough, and non-Slack unaffected

How to Test

  1. Add cron: threaded_delivery: true to ~/.hermes/config.yaml
  2. Run a cron job that delivers to a Slack channel; confirm the channel shows a compact summary parent with the full report in its thread
  3. Add "thread": false to a specific job; confirm that job delivers flat even with the global flag on
  4. Run pytest tests/cron/test_threaded_delivery.py tests/cron/test_scheduler.py -q — 158 tests pass

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.0

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 (cron.wrap_response is also undocumented in the example; following that precedent)
  • 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 — threading is Slack-only; no OS-specific code
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Ran on a 4-profile deployment since 2026-06-11. Representative log line:

Job 'oil-morning-brief': delivered to slack:C08XXXXXXXX via live adapter (threaded)

🤖 Generated with Claude Code

…hread

Adds cron.threaded_delivery (default false). When enabled, Slack cron
reports post a compact summary (agent-written TL;DR: paragraph, or first
paragraph as fallback) as the parent message and the full report in its
thread. Per-job opt-out via "thread": false. Any failure falls back to
the existing flat delivery. Platforms other than Slack are unchanged.
@alt-glitch alt-glitch added type/feature New feature or request comp/cron Cron scheduler and job management platform/slack Slack app adapter P3 Low — cosmetic, nice to have labels Jun 11, 2026

@teknium1 teknium1 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.

Thanks for the focused Slack delivery proposal. The summary-parent/detail behavior is not present on current main; #56254's cron_continuable_surface: in_channel is a different flat-continuation feature (website/docs/user-guide/features/cron.md:334-370).

Problems

  • The PR targets the old live-adapter path. Current main routes cron text through DeliveryRouter (cron/scheduler.py:1694-1712) and has confirmation/timeout handling through :1721-1811; the new direct adapter.send sequence (cron/scheduler.py:741, invoked at :907 in the PR) needs to be rebuilt on that path.
  • _send_threaded_via_adapter retries flat after any timeout (cron/scheduler.py:736-739 in the PR). Current main only falls back when cancellation proves dispatch never started (cron/scheduler.py:1721-1761); otherwise retrying can duplicate an in-flight detail message.
  • cron.threaded_delivery needs a default in hermes_cli/config.py and user documentation; the PR changes only the scheduler and tests.

Suggested changes

  • Preserve DeliveryRouter routing and current timeout semantics for both sends, then add current-path tests for queued versus in-flight timeouts and media.
  • Document the Slack/live-adapter scope, including the separate desktop standalone path described in hermes_cli/web_server.py:136-148 and related #46207.

Automated hermes-sweeper review.

Comment thread cron/scheduler.py
try:
return future.result(timeout=60)
except TimeoutError:
future.cancel()

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.

Do not unconditionally fall back after this timeout. cancel() returning false means the send is already in flight, so a flat retry can duplicate the detail; current main explicitly distinguishes those cases in cron/scheduler.py:1721-1761.

Comment thread cron/scheduler.py
and _threaded_delivery_enabled(job)
):
try:
sent_threaded = _send_threaded_via_adapter(

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.

This needs to be reworked around current main's DeliveryRouter path rather than calling adapter.send directly. The current live cron path preserves target routing and confirmation/fallback semantics in cron/scheduler.py:1694-1811.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 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 P3 Low — cosmetic, nice to have platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants