Skip to content

fix(slack): render clarify prompts as block kit buttons - #55671

Closed
Christopher-Schulze wants to merge 1 commit into
NousResearch:mainfrom
Christopher-Schulze:fix/52374-slack-clarify-json-leak
Closed

fix(slack): render clarify prompts as block kit buttons#55671
Christopher-Schulze wants to merge 1 commit into
NousResearch:mainfrom
Christopher-Schulze:fix/52374-slack-clarify-json-leak

Conversation

@Christopher-Schulze

Copy link
Copy Markdown
Contributor

What does this PR do?

Slack now renders gateway clarify prompts as a single Block Kit prompt with choice buttons instead of inheriting the generic numbered-text fallback. That keeps the user-facing Slack message to the question, choices, and buttons, so the clarify tool result JSON is not exposed as a second raw text payload under the interactive UI.

The callback path mirrors the existing gateway clarify contract used by other rich platforms: numeric choices resolve via resolve_gateway_clarify, "Other" switches the pending prompt into text-capture mode with mark_awaiting_text, and expired prompts update as expired instead of showing a false selection.

Related Issue

Fixes #52374

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

  • Added Slack send_clarify Block Kit rendering in plugins/platforms/slack/adapter.py.
  • Registered Slack clarify button action handlers for choice and "Other" callbacks.
  • Added regression coverage in tests/gateway/test_slack_approval_buttons.py for safe Slack fallback text, choice resolution, and text-capture mode.

How to Test

  1. uv run pytest tests/gateway/test_slack_approval_buttons.py::TestSlackClarifyButtons -q
  2. uv run pytest tests/gateway/test_slack_approval_buttons.py -q
  3. uv run pytest tests/gateway/test_slack.py -q
  4. From the contributor harness: ./scripts/check.sh

Local proof: the new regression tests assert that the Slack API payload contains blocks and a safe fallback text of Hermes needs your input: ..., not raw clarify JSON, and that button callbacks resolve through the gateway clarify primitive.

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 15.6.1

I did not run the full local pytest tests/ -q suite; I ran the scoped Slack tests above plus the repository contributor gate (./scripts/check.sh), and the full suite should run in CI.

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

Relevant local checks passed:

  • uv run pytest tests/gateway/test_slack_approval_buttons.py::TestSlackClarifyButtons -q — 3 passed
  • uv run pytest tests/gateway/test_slack_approval_buttons.py -q — 29 passed
  • uv run pytest tests/gateway/test_slack.py -q — 209 passed
  • ./scripts/check.sh — all blocking gates passed

@Christopher-Schulze
Christopher-Schulze force-pushed the fix/52374-slack-clarify-json-leak branch from 4d1f239 to f6ea5ae Compare June 30, 2026 13:38
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to the Slack clarify Block Kit cluster, not a duplicate:

This PR (#55671) and #51547 both add a send_clarify() Block Kit override + clarify button callback handler to the relocated plugins/platforms/slack/adapter.py; #55671 additionally suppresses the raw JSON payload (#52374) and uses a single hermes_clarify_choice action with value=clarify_id|index rather than per-button regex action_ids. Flagging the cluster so a maintainer can pick the canonical implementation; not asserting one over the others.

@Christopher-Schulze

Christopher-Schulze commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Closing to avoid adding another competing implementation in the Slack clarify Block Kit cluster. I missed the linked sibling PRs in the issue comments during dedupe; thanks for flagging #28885 and #51547.

Before closing, I pushed the Slack action-id uniqueness fix to this branch so the diff remains available if useful: choice buttons now use per-button action IDs such as hermes_clarify_choice_0 / hermes_clarify_choice_1, and the regression test asserts uniqueness, matching Slack Block Kit requirements.

I will not keep this PR open against the existing current-path implementation in #51547.

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: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.

Slack gateway: raw clarify tool result JSON leaks underneath interactive buttons

2 participants