Skip to content

fix(slack): chunk section-block content and surface postMessage errors (#18) - #30253

Closed
idl3 wants to merge 1 commit into
NousResearch:mainfrom
idl3:feat/rico-a2-slack-chunking
Closed

fix(slack): chunk section-block content and surface postMessage errors (#18)#30253
idl3 wants to merge 1 commit into
NousResearch:mainfrom
idl3:feat/rico-a2-slack-chunking

Conversation

@idl3

@idl3 idl3 commented May 22, 2026

Copy link
Copy Markdown

Summary

When an agents sends a ~2KB structured report via send_slash_confirm, Slack silently rejected it because the section block text field exceeded the 3000-char-per-element API limit — producing two empty-looking messages at the same timestamp. The top-level text field was fine (well under 40k), but the block-level limit is separate and was never enforced.

Root cause: send_slash_confirm used a hard truncation at 2900 chars (message[:2900] + "..."), which silently drops content rather than chunking it. More critically, no call site checked the Slack API response for error details, so the rejection was invisible in logs.

This PR makes the send path section-block-aware and adds error surfacing across all chat_postMessage call sites:

  • MAX_SECTION_TEXT_LENGTH = 2900 constant added near MAX_MESSAGE_LENGTH (Slack's 3000-char hard limit minus 100-char margin).
  • send_slash_confirm rewritten: uses BasePlatformAdapter.truncate_message to split the message body at the section budget (preserving code-block boundaries). The first chunk goes in the button block; overflow posts as threaded plain-text replies with [i/N continued] markers — no content is ever silently dropped.
  • _safe_post_message helper wraps every chat_postMessage call. On SlackApiError it logs error_code, text_len, blocks_count, and longest section-block text length at WARNING level before re-raising. Wired into send(), send_exec_approval, send_slash_confirm, and the button-action follow-up path.
  • 10 new tests in tests/gateway/test_slack_chunking.py covering: large/small send() chunking, section-block size ceiling, continuation threading, full content preservation, and _safe_post_message error-code logging. All 285 existing Slack tests continue to pass; ruff clean.

new-pattern: yes
what: send_slash_confirm now chunks at section-block level and routes all chat_postMessage calls through a logging wrapper
visualizable: no
why: the change is a linear call-site refactor with no branching flow or state machine; a diagram would add no information over the prose description
decision: skip

Test plan

  • uv run --extra dev python -m pytest tests/gateway/test_slack_chunking.py -v — 10/10 pass
  • uv run --extra dev python -m pytest tests/gateway/test_slack.py tests/gateway/test_slack_approval_buttons.py tests/gateway/test_slack_mention.py tests/gateway/test_slack_channel_skills.py -v — 275/275 pass
  • uv run --extra dev ruff check gateway/platforms/slack.py tests/gateway/test_slack_chunking.py — clean
  • Reviewer: send a >3000-char report via /hermes slash command in a real workspace and confirm no empty-message pair appears

Closes #18

NousResearch#18)

When Rico sends a structured report via send_slash_confirm, the section block
text could exceed Slack's 3000-char-per-text-element limit, causing the API to
silently reject the message and post empty-looking replies.

- Add MAX_SECTION_TEXT_LENGTH = 2900 constant (3000 hard limit minus margin).
- Rewrite send_slash_confirm to split the message body via truncate_message at
  the section budget; the first chunk goes in the button block, overflow chunks
  post as threaded plain-text replies with [i/N continued] markers so no content
  is silently dropped.
- Add _safe_post_message helper that wraps every chat_postMessage call: on
  SlackApiError it logs error_code, text_len, blocks_count, and max_section_len
  at WARNING level before re-raising so silent failures become visible.
- Wire _safe_post_message into send(), send_exec_approval, send_slash_confirm,
  and the button-action follow-up post.
- Add tests/gateway/test_slack_chunking.py: 10 cases covering large/small send()
  chunks, section-block size ceiling, continuation threading, content
  preservation, and _safe_post_message error logging. All 285 Slack tests pass.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter labels May 22, 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 addressing the Slack block-limit failure. Current main already prevents the rejected-block symptom, but this PR needs a targeted port and one boundary fix.

Problems

  • gateway/platforms/slack.py was moved to plugins/platforms/slack/adapter.py by 1a38066054752d601b71fc655a3ed6bf4228e2da, so the submitted patch no longer targets the active adapter.
  • The new title_str = title or "Confirm" at gateway/platforms/slack.py:2390 is unbounded. A sufficiently long title makes header exceed Slack's 3000-character section limit before body content is considered. Current main caps this input at plugins/platforms/slack/adapter.py:3348.

Suggested changes

  • Port the continuation/error-detail work to plugins/platforms/slack/adapter.py while retaining the current 150-character title cap and rendered-size budget.
  • Add an overlong-title regression test; current main already has the relevant section-limit guard in the active adapter.

Automated hermes-sweeper review.

value = f"{session_key}|{confirm_id}"

# Reserve room for the "*{title}*\n\n" wrapper inside the section.
title_str = title or "Confirm"

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.

Please retain a title cap here (current main uses [:150]). Without one, a long title makes header exceed Slack's 3000-character section limit before any body chunk is added.

@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 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #69317 (merged): targeted the pre-plugin-migration file; section chunking already exists on main (_split_text) and error surfacing landed via #56618's retry-fallback.

Thanks for digging into this — the consolidated fix stands on the cluster's collective analysis, and your work is credited in #69317's summary.

@teknium1 teknium1 closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery 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-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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants