Skip to content

feat(slack): add native clarify choice buttons - #66606

Closed
jaaro-ai wants to merge 1 commit into
NousResearch:mainfrom
jaaro-ai:feat/slack-clarify-buttons
Closed

jaaro-ai wants to merge 1 commit into
NousResearch:mainfrom
jaaro-ai:feat/slack-clarify-buttons

Conversation

@jaaro-ai

Copy link
Copy Markdown

Summary

  • render Hermes clarify choices as native Slack Block Kit buttons
  • bind callbacks to the exact workspace, channel, thread, message, and canonical server-side choice
  • make button/text resolution consume-once and race-safe
  • preserve open-ended and oversized text fallback behavior
  • remove stale controls after resolution and post a bound fallback if chat.update fails
  • document Slack choice buttons

Security

  • missing, unknown, and disconnected workspaces fail closed
  • clicking user goes through the existing workspace-scoped authorization path
  • callback values contain only the clarify ID and canonical choice index
  • origin mismatches, malformed callbacks, replays, and duplicate clicks are ignored
  • pending callback state is TTL-pruned and bounded

Test plan

  • python -m pytest -q -W error tests/gateway/test_slack_clarify_buttons.py tests/gateway/test_slack_plugin_action_handlers.py tests/tools/test_clarify_gateway.py -n 0 --tb=short — 64 passed
  • python -m pytest tests/gateway/test_slack*.py -q --tb=short -n 0 — 464 passed
  • python -m pytest tests/tools/test_clarify_tool.py tests/tools/test_clarify_gateway.py tests/gateway/test_clarify_active_session_bypass.py -q --tb=short -n 0 — 53 passed

Independent security review passed with no remaining concerns.

@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets platform/slack Slack app adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages needs-decision Awaiting maintainer decision before any implementation labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #28885: both add Slack clarify buttons, but this implementation adds workspace/message origin binding and server-side canonical-choice resolution. Maintainer choice requested.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the Slack clarify implementation. The feature remains needed on current main: plugins/platforms/slack/adapter.py:3784 transitions from inbound handling directly to approval buttons, while gateway/platforms/base.py:3182-3208 supplies only the text fallback.

The implementation fits the existing gateway flow: gateway/run.py:19334-19340 passes thread/workspace metadata into send_clarify; the PR binds callbacks to the posted Slack message and delegates interactive authorization through the existing workspace-scoped path. Its resolver change also closes the current response-overwrite window in tools/clarify_gateway.py:156-162 by performing response/event mutation while holding the shared lock.

The PR head is directly based on current main (dadb6d73fba06ee57e0f82808f4554343ad6b844 parent d59b79fadd1e9edd7afc5c679cc3b143838e7c01), so salvage should be a clean cherry-pick. Related PR #28885 remains open; as the MEMBER note says, this version adds stricter origin binding and canonical server-side choice resolution.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 18, 2026
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
Collaborator

Closing as superseded by #69318 (merged): rewrites clarify_gateway semantics beyond the feature scope and caps at 4 choices with text fallback; the landed base chunks beyond 5 — your fail-closed workspace binding idea is noted and worth a standalone proposal.

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

@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
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 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
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 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/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets needs-decision Awaiting maintainer decision before any implementation 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 type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants