Skip to content

feat(slack): Block Kit buttons for clarify prompts — with tests, chunking, and resolve-state editing - #61943

Closed
100yenadmin wants to merge 3 commits into
NousResearch:mainfrom
100yenadmin:feat/slack-clarify-buttons
Closed

feat(slack): Block Kit buttons for clarify prompts — with tests, chunking, and resolve-state editing#61943
100yenadmin wants to merge 3 commits into
NousResearch:mainfrom
100yenadmin:feat/slack-clarify-buttons

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented Jul 10, 2026

Copy link
Copy Markdown

What this does

Slack is the last button-capable platform where the clarify tool still degrades to a numbered text list — Telegram, Discord, and WhatsApp Cloud all render interactive choices. This adds the send_clarify override and Bolt action handlers so Slack users tap a button instead of typing "2".

❓ Which environment should I deploy to?
[staging] [production] [both, staging first] [✏️ Other…]

How it works

  • send_clarify renders a Block Kit section (question, mrkdwn-escaped) plus actions rows: one button per choice and a final "✏️ Other…" button. Rows chunk at Slack's five-element cap, so buttons keep working if MAX_CHOICES is raised later (feat(clarify): raise MAX_CHOICES from 4 to 10 #28900).
  • Every choice receives a unique hermes_clarify_choice_<idx> action ID, satisfying Slack's per-block uniqueness requirement. One anchored Bolt regex listener routes those indexed IDs; the existing compact clarify_id|idx value and server-side choice lookup remain unchanged.
  • A choice click resolves through resolve_gateway_clarify with the exact server-side choice string, then edits the message to show the selection. A msg_ts-keyed guard makes double-clicks inert, following the existing approval-button pattern.
  • "Other…" calls mark_awaiting_text; the gateway's platform-agnostic text intercept resolves the next typed message. No new Slack-specific text machinery is introduced.
  • Open-ended clarify prompts delegate to the base text path unchanged. Unauthorized clicks are ignored through the same authorization gate used by approval buttons. Expired prompts are edited to show an expiry notice instead of leaving a dead button row.

Event flow

clarify request
  ├─ choices present ──> Block Kit buttons ──> indexed action_id matcher
  │                                           ├─ choice ──> resolve + edit message
  │                                           └─ Other ───> await next text message
  └─ no choices ──────> existing base text path

Relationship to the other open PRs

#51547 and #28885 address the same Slack parity gap. This PR keeps the send and action paths together and adds:

  • a 14-case adapter suite covering rendering, values, escaping, thread_ts, resolve behavior, double-click protection, authorization, expiry, Other-to-text capture, chunking, and base fallback;
  • action-registration coverage proving the indexed Bolt matcher accepts hermes_clarify_choice_0 and rejects the old unindexed form;
  • row chunking instead of falling back to text above four choices;
  • post-resolution message editing and expiry notices; and
  • current-main compatibility with a Slack-focused diff and no unrelated runtime changes.

This therefore supersedes the earlier alternatives while preserving their intended user experience. The implementation can still be consolidated with either author if maintainers prefer a different landing path.

Review fix

The review finding in discussion_r3593877536 was valid: the original patch reused hermes_clarify_choice within one actions block. Commit 5fb2a2703 assigns indexed IDs, registers an anchored regex listener, preserves value routing, and adds render plus registration regressions. The thread has been answered and resolved.

Validation

Validated on head 5fb2a2703 after syncing with main at 659d1123c:

  • scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_clarify_buttons.py — 257 passed;
  • cross-platform clarify and Slack approval suite — 126 passed;
  • Ruff on all three changed files — clean;
  • Python bytecode compilation — clean; and
  • git diff --check — clean.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter labels Jul 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for filling a verified Slack parity gap. Current remote main (5ecc079) has no Slack send_clarify override, so Slack inherits the base numbered-text fallback in gateway/platforms/base.py:3088-3143. The PR adds the native Slack send path and action registration in plugins/platforms/slack/adapter.py (PR right-side lines 1191-1195 and 3414-3518), while reusing the existing clarify primitives at tools/clarify_gateway.py:150-227.

GitHub comparison shows that the 87 commits after the PR base did not modify plugins/platforms/slack/adapter.py, and the PR remains mergeable. No blocking correctness issue was found in the reviewed diff.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
Rebased onto origin/main (was based on 46cf87b; main advanced 440
commits). Reset-hard + cherry-pick to drop the stale merge commit
(1a35c6940) that would have caused a replay explosion.

Conflict resolved in plugins/platforms/slack/adapter.py action-handler
registration block: main added the 'hermes_feedback' action registration
at the same site where this commit registers the clarify action handlers
('hermes_clarify_choice' / 'hermes_clarify_other'). Kept BOTH.

send_clarify override signature matches the now-canonical base.py
send_clarify (chat_id, question, choices, clarify_id, session_key,
metadata) that standardized across Telegram/Discord/WhatsApp on main;
open-ended prompts delegate to super().send_clarify().
@100yenadmin
100yenadmin force-pushed the feat/slack-clarify-buttons branch from 1a35c69 to a69f5c9 Compare July 15, 2026 05:27
@100yenadmin

Copy link
Copy Markdown
Author

Rebased onto current main (47d853fd) — the CONFLICTING state is resolved (one hunk: main's hermes_feedback action registration landed at the same site as our clarify handlers; both kept).

Worth noting what the rebase surfaced: since this PR was opened, the clarify-buttons pattern standardized on main across Telegram/Discord/WhatsApp — and our Slack override's send_clarify signature is byte-identical to that now-canonical base contract (open-ended prompts delegate to super()). Slack is the one platform still on the numbered-text fallback (gateway/platforms/base.py:3119), so this PR now fills exactly the remaining gap in an established pattern rather than proposing a new one.

Suites re-verified post-rebase via scripts/run_tests.sh: 153 passed, 0 failed (slack clarify buttons 14, clarify gateway 18, clarify tool 28, plus telegram/discord clarify and slack/telegram/discord approval regressions).

On consolidation: #51547 and #28885 target the same gap and remain unrebased against the migrated layout — our offer stands to fold whichever pieces reviewers prefer, but this branch is current, tested, and matches the canonical adapter contract today. A maintainer review would unblock it.

Comment thread plugins/platforms/slack/adapter.py Outdated
@100yenadmin

Copy link
Copy Markdown
Author

@copilot review

Updated head: 5fb2a2703. The Slack action_id uniqueness review finding is fixed with indexed choice IDs plus an anchored Bolt matcher; routing values remain unchanged. The inline thread is resolved, 257 Slack tests and 126 cross-platform clarify/approval tests pass, and Ruff/compile/whitespace checks are clean.

teknium1 pushed a commit that referenced this pull request Jul 22, 2026
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
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
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
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69318 — your commit was cherry-picked onto current main with your authorship preserved in git history: your implementation (buttons, >5 chunking, resolve-state editing, tests) was the chosen base and was cherry-picked with authorship.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 22, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
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
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 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants