Skip to content

fix: bound Slack approval block text - #27441

Closed
2001Y wants to merge 1 commit into
NousResearch:mainfrom
2001Y:fix/slack-approval-block-limit
Closed

fix: bound Slack approval block text#27441
2001Y wants to merge 1 commit into
NousResearch:mainfrom
2001Y:fix/slack-approval-block-limit

Conversation

@2001Y

@2001Y 2001Y commented May 17, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Bounds Slack approval Block Kit section text so approval buttons do not fail with Slack invalid_blocks errors when the command and/or approval reason is long.

The fix budgets the command preview and reason text together against Slack's 3000-character section.text.text limit. It keeps short reasons intact and only truncates the reason when the combined section would exceed Slack's limit.

Related Issue

N/A — observed Slack approval UI failure from oversized Block Kit section text.

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

  • Keep Slack approval Block Kit section.text.text within Slack's 3000-character limit.
  • Budget command and reason text together instead of truncating only the command preview.
  • Preserve the approval reason when short and truncate it only when necessary.
  • Add/keep regression coverage in tests/gateway/test_slack_approval_buttons.py for long approval text.

How to Test

  1. Run the Slack approval regression tests:
    python -m pytest tests/gateway/test_slack_approval_buttons.py -q -o addopts=
  2. Confirm the tests pass and the generated Slack approval block text remains under the limit for long command/reason inputs.

Latest local verification:

24 passed, 4 warnings in 0.91s

The warnings are existing async mock RuntimeWarnings in Slack thread-context tests and are not introduced by this PR.

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

Documentation and 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

Targeted pytest output is included above. No UI screenshot; this is a Slack Block Kit payload-size guard.

@2001Y
2001Y requested a review from a team May 17, 2026 13:20
@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 17, 2026
@2001Y
2001Y force-pushed the fix/slack-approval-block-limit branch from 021b1f1 to 7331f66 Compare May 17, 2026 16:15

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use .github/PULL_REQUEST_TEMPLATE.md

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@2001Y

2001Y commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Updated the PR body to the repository template and re-ran the targeted Slack approval regression suite.

python -m pytest tests/gateway/test_slack_approval_buttons.py -q -o addopts=
24 passed, 4 warnings in 0.91s

The linked Copilot reviews failed before producing inline comments, so there were no Copilot code items to address on this PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the Slack approval fix. This has since been implemented on main by a broader Slack approval UX fix, so I think this PR can be closed as already handled.

Evidence from this automated hermes-sweeper review:

  • gateway/platforms/slack.py:2767 now explicitly accounts for Slack's 3000-character section block cap in send_exec_approval.
  • gateway/platforms/slack.py:2774-2784 builds the rendered section text from a budgeted command preview plus bounded reason text, preventing the oversized invalid_blocks approval-card failure this PR targets.
  • The change landed in cd9a9cd8e5e12daed968360f70d28e749c6c1fa0 via merged PR fix(gateway): Slack approval UX in threads — block-size overflow + typed-prefix instruction text #43444, whose body calls out the same root cause: long execute_code approval blocks overflowing Slack's 3000-character Block Kit limit.

The PR's discussion and the related #35495 cross-reference were useful in tracking the affected Slack approval path.

@teknium1 teknium1 closed this Jun 13, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 13, 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: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.

5 participants