feat(slack): render clarify prompts as Block Kit buttons - #28885
feat(slack): render clarify prompts as Block Kit buttons#28885cypres0099 wants to merge 1 commit into
Conversation
|
CI notes: check-attribution failure — fixed by sibling PR #28897 (one-line AUTHOR_MAP addition). Will re-run on this PR after #28897 merges. test failures — all 9 pre-exist on
None touch |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the Slack clarify parity work. I verified the premise still holds on current main: SlackAdapter has no send_clarify override, so BasePlatformAdapter.send_clarify still renders multi-choice clarifies as numbered text (gateway/platforms/base.py:2543).
Problems
- The new clarify button handler checks only SLACK_ALLOWED_USERS (gateway/platforms/slack.py:2707 in the PR). Current main hardened Slack interactive actions through _is_interactive_user_authorized (gateway/platforms/slack.py:3026, :3205 on origin/main), which also honors GatewayRunner auth and GATEWAY_ALLOWED_USERS/GATEWAY_ALLOW_ALL_USERS. Clarify should use the same helper so button clicks do not bypass current auth policy.
- The clarify section block sends f"❓ {question}" directly (gateway/platforms/slack.py:2660 in the PR). Current Slack approval/confirm paths budget section text for Slack's 3000-character Block Kit cap (gateway/platforms/slack.py:2879, :2965 on origin/main), and tools/clarify_tool.py does not cap question length.
Suggested changes
- Reuse _is_interactive_user_authorized in _handle_clarify_action and add a global/GatewayRunner auth test.
- Budget/truncate the question block and add a long-question invalid_blocks regression test.
Automated hermes-sweeper review.
a1a4d19 to
eb38b7b
Compare
|
Refreshed this branch onto latest |
eb38b7b to
90bae88
Compare
|
Final refresh complete at |
Slack now overrides send_clarify to render multi-choice clarify prompts as native Block Kit buttons (one per choice + a final '✏️ Other…' free-text button), mirroring the Telegram/Discord adapters and the existing Slack approval-button pattern. - Unique hermes_clarify_choice_<idx> action_ids (Slack rejects duplicate action_ids within one actions block); dispatch via a compiled-regex action matcher plus hermes_clarify_other. - Chunks elements across actions blocks in groups of 5 so a larger choice list degrades gracefully instead of 400ing (invalid_blocks). - Choice taps resolve through tools.clarify_gateway .resolve_gateway_clarify with the canonical registered choice text — the same applier the typed-reply path uses — then edit the message to show the outcome and drop the buttons. - 'Other' flips the entry into text-capture via mark_awaiting_text (only on tap, never at send time) so the gateway text-intercept captures the next typed message. - Auth-gated via _is_interactive_user_authorized; atomic-pop double-click guard mirrors _approval_resolved; late taps on evicted entries surface an honest expiry notice instead of a false ✓. - Open-ended prompts delegate to the base plain-text render. Salvaged from PR #61943 by @100yenadmin. Earliest implementation of this feature was PR #28885 by @cypres0099; sibling implementations #66606 (@jaaro-ai) and #51547 (@Mongol-Jimmi) are superseded. Closes #52369
Slack now overrides send_clarify to render multi-choice clarify prompts as native Block Kit buttons (one per choice + a final '✏️ Other…' free-text button), mirroring the Telegram/Discord adapters and the existing Slack approval-button pattern. - Unique hermes_clarify_choice_<idx> action_ids (Slack rejects duplicate action_ids within one actions block); dispatch via a compiled-regex action matcher plus hermes_clarify_other. - Chunks elements across actions blocks in groups of 5 so a larger choice list degrades gracefully instead of 400ing (invalid_blocks). - Choice taps resolve through tools.clarify_gateway .resolve_gateway_clarify with the canonical registered choice text — the same applier the typed-reply path uses — then edit the message to show the outcome and drop the buttons. - 'Other' flips the entry into text-capture via mark_awaiting_text (only on tap, never at send time) so the gateway text-intercept captures the next typed message. - Auth-gated via _is_interactive_user_authorized; atomic-pop double-click guard mirrors _approval_resolved; late taps on evicted entries surface an honest expiry notice instead of a false ✓. - Open-ended prompts delegate to the base plain-text render. Salvaged from PR #61943 by @100yenadmin. Earliest implementation of this feature was PR #28885 by @cypres0099; sibling implementations #66606 (@jaaro-ai) and #51547 (@Mongol-Jimmi) are superseded. Closes #52369
|
Closing as superseded by #69318 (merged): you were the EARLIEST implementation of clarify buttons (May 19) and are credited as originator in #69318; the landed base (#61943) added >5-choice chunking, expiry UX, and mrkdwn escaping on the same architecture you pioneered. Thanks for digging into this — the consolidated fix stands on the cluster's collective analysis, and your work is credited in #69318's summary. |
Slack now overrides send_clarify to render multi-choice clarify prompts as native Block Kit buttons (one per choice + a final '✏️ Other…' free-text button), mirroring the Telegram/Discord adapters and the existing Slack approval-button pattern. - Unique hermes_clarify_choice_<idx> action_ids (Slack rejects duplicate action_ids within one actions block); dispatch via a compiled-regex action matcher plus hermes_clarify_other. - Chunks elements across actions blocks in groups of 5 so a larger choice list degrades gracefully instead of 400ing (invalid_blocks). - Choice taps resolve through tools.clarify_gateway .resolve_gateway_clarify with the canonical registered choice text — the same applier the typed-reply path uses — then edit the message to show the outcome and drop the buttons. - 'Other' flips the entry into text-capture via mark_awaiting_text (only on tap, never at send time) so the gateway text-intercept captures the next typed message. - Auth-gated via _is_interactive_user_authorized; atomic-pop double-click guard mirrors _approval_resolved; late taps on evicted entries surface an honest expiry notice instead of a false ✓. - Open-ended prompts delegate to the base plain-text render. Salvaged from PR NousResearch#61943 by @100yenadmin. Earliest implementation of this feature was PR NousResearch#28885 by @cypres0099; sibling implementations NousResearch#66606 (@jaaro-ai) and NousResearch#51547 (@Mongol-Jimmi) are superseded. Closes NousResearch#52369
What does this PR do?
Adds native Slack Block Kit buttons for multi-choice
clarifyprompts, bringing Slack to parity with Telegram and Discord. A choice click resolves the waiting clarify request;Otherswitches the existing gateway flow into free-text capture. Open-ended clarifies continue to use the base adapter's text behavior.The refreshed implementation incorporates both issues from the automated maintainer sweep: clarify actions reuse Slack's shared interactive-user authorization policy, and long questions are budgeted under Slack's 3000-character section-text limit.
Related Issue
N/A — platform parity feature; no separate upstream issue is open.
Type of Change
Changes Made
plugins/platforms/slack/adapter.pysend_clarify()Block Kit rendering with unique per-choice action IDs and anOtheraction._is_interactive_user_authorized, preserving GatewayRunner auth and global gateway allowlist policy.tests/gateway/test_slack_clarify_buttons.pyHow to Test
HOME=/tmp/hermes-ci-home scripts/run_tests.sh -j 1 tests/gateway/test_slack_clarify_buttons.py tests/gateway/test_slack_approval_buttons.py.Otheraccepts the next free-text reply.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passThe canonical full suite was run on the same current upstream base via the #30592 control branch. Its failures/timeouts were outside this two-file diff and matched current-main macOS/environment-sensitive areas. The Slack clarify and existing approval-button files pass 41/41.
Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs