Skip to content

feat(slack): add button-based approval flow for dangerous commands - #3898

Closed
LevSky22 wants to merge 4 commits into
NousResearch:mainfrom
LevSky22:feat/slack-approval-buttons
Closed

feat(slack): add button-based approval flow for dangerous commands#3898
LevSky22 wants to merge 4 commits into
NousResearch:mainfrom
LevSky22:feat/slack-approval-buttons

Conversation

@LevSky22

@LevSky22 LevSky22 commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds Slack-native approval buttons for dangerous command prompts.

Hermes currently replies in Slack threads, but Slack custom slash commands are not supported in threads. That makes the existing text-based approval flow awkward for Slack, especially when a dangerous command needs explicit approval. This PR adds Block Kit approval buttons for Slack so approvals can happen directly inside the threaded conversation without relying on slash commands.

Rebase note: This branch has been rebased onto current main and adapted to the blocking gateway approval mechanism introduced after the original commit. The old _pending_approvals dict + terminal_tool(force=True) approach has been replaced with has_blocking_approval / resolve_gateway_approval from tools/approval.py.

This PR also includes the fix from #4698 (approve-once must not grant session-wide approval) along with its regression test, so both features land together and are covered end-to-end.

Related Issue

No existing issue — this is a new feature.

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 a platform-level approval action handler to the gateway base adapter so messaging platforms can submit structured approve/deny actions.
  • Added Slack Block Kit approval buttons for dangerous command prompts in gateway/platforms/slack.py.
  • Wired Slack button actions into the blocking gateway approval mechanism in gateway/run.py (has_blocking_approval / resolve_gateway_approval).
  • Suppressed the extra Slack-only approval hint text when button delivery succeeds.
  • Fixed approve_session() being called for choice == "once" (refs fix(gateway): make plain /approve truly one-time #4698) — one-time approvals no longer silently become session-wide.
  • Added gateway tests covering Slack button rendering, Slack button action handling, platform approval action execution, and the approve-once scope regression.

How to Test

  1. Configure Hermes with Slack Socket Mode and a working Slack app.
  2. Trigger a dangerous command from Slack that requires approval.
  3. Click Approve Once, Approve Session, or Deny in the Slack thread and verify the pending command behaves correctly.
  4. Verify Approve Once prompts again on the next identical dangerous command (regression for fix(gateway): make plain /approve truly one-time #4698).
  5. Run pytest tests/gateway/test_slack.py tests/gateway/test_approve_deny_commands.py -q.

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: Ubuntu 24.04 (Dockerized Hermes + Slack Socket Mode)

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

  • Targeted gateway coverage added and passing:
    • pytest tests/gateway/test_slack.py tests/gateway/test_approve_deny_commands.py -q
    • All 28 tests in test_approve_deny_commands.py pass

LevSky22 added 3 commits April 5, 2026 14:58
The original PR was built on the old post-loop pop_pending approval
architecture, which upstream replaced with a blocking mechanism in
tools/approval.py (agent thread blocks until user responds via
/approve or /deny).

Rebased onto current main and adapted:

- _handle_platform_approval_action now calls has_blocking_approval /
  resolve_gateway_approval instead of terminal_tool(force=True) and
  self._pending_approvals, matching the blocking flow used by
  _handle_approve_command and _handle_deny_command.

- Updated TestPlatformApprovalActions tests to mock the blocking
  approval API (has_blocking_approval, resolve_gateway_approval)
  instead of the removed _pending_approvals dict. Replaced the
  timestamp-expiry test (no longer applicable) with a no-pending test.

- Preserved _download_slack_file team_id parameter added upstream.

All 93 tests in test_slack.py and test_approve_deny_commands.py pass.
@LevSky22
LevSky22 force-pushed the feat/slack-approval-buttons branch from 92b505d to fb086a4 Compare April 5, 2026 19:01
When a gateway user chose "once", approve_session() was still being
called (because the condition tested `choice in ("once", "session")`),
silently adding the pattern to the session allowlist and making every
subsequent identical command skip the prompt.

Fix: only call approve_session() when choice is "session" or "always".
Also adds a regression test (TestApproveOnceScope) that runs two
dangerous commands sequentially, approves the first with "once", and
asserts the second still triggers a fresh prompt.

Refs: NousResearch#4698

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@LevSky22
LevSky22 force-pushed the feat/slack-approval-buttons branch from b916961 to 86f8725 Compare April 5, 2026 19:50
teknium1 added a commit that referenced this pull request Apr 7, 2026
…ontext

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR #5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs #3898 (LevSky22), #2953 (ygd58), #5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
teknium1 added a commit that referenced this pull request Apr 7, 2026
…ontext

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR #5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs #3898 (LevSky22), #2953 (ygd58), #5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
teknium1 added a commit that referenced this pull request Apr 7, 2026
…ontext (#5890)

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR #5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs #3898 (LevSky22), #2953 (ygd58), #5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
@teknium1

teknium1 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #5890. Your Block Kit approval buttons design was the reference implementation — we built the feature fresh on current main following your architecture (action_id handlers, session_key in button value, message update on click). Thank you @LevSky22!

@teknium1 teknium1 closed this Apr 7, 2026
Tommyeds pushed a commit to Tommyeds/hermes-agent that referenced this pull request Apr 12, 2026
…ontext (NousResearch#5890)

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR NousResearch#5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs NousResearch#3898 (LevSky22), NousResearch#2953 (ygd58), NousResearch#5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
…ontext (NousResearch#5890)

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR NousResearch#5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs NousResearch#3898 (LevSky22), NousResearch#2953 (ygd58), NousResearch#5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…ontext (NousResearch#5890)

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR NousResearch#5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs NousResearch#3898 (LevSky22), NousResearch#2953 (ygd58), NousResearch#5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…ontext (NousResearch#5890)

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR NousResearch#5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs NousResearch#3898 (LevSky22), NousResearch#2953 (ygd58), NousResearch#5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…ontext (NousResearch#5890)

Slack:
- Add Block Kit interactive buttons for command approval (Allow Once,
  Allow Session, Always Allow, Deny) via send_exec_approval()
- Register @app.action handlers for each approval button
- Add _fetch_thread_context() — fetches thread history via
  conversations.replies when bot is first @mentioned mid-thread
- Fix _has_active_session_for_thread() to use build_session_key()
  instead of manual key construction (fixes session key mismatch bug
  where thread_sessions_per_user flag was ignored, ref PR NousResearch#5833)

Telegram:
- Add InlineKeyboard approval buttons via send_exec_approval()
- Add ea:* callback handling in _handle_callback_query()
- Uses monotonic counter + _approval_state dict to map button clicks
  back to session keys (avoids 64-byte callback_data limit)

Both platforms now auto-detected by the gateway runner's
_approval_notify_sync() — any adapter with send_exec_approval() on
its class gets button-based approval instead of text fallback.

Inspired by community PRs NousResearch#3898 (LevSky22), NousResearch#2953 (ygd58), NousResearch#5833
(heathley). Implemented fresh on current main.

Tests: 24 new tests covering button rendering, action handling,
thread context fetching, session key fix, double-click prevention.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants