Skip to content

fix/slack-thread-session-key-mismatch - #5833

Closed
heathley wants to merge 1 commit into
NousResearch:mainfrom
heathley:fix/slack-thread-session-key-mismatch
Closed

fix/slack-thread-session-key-mismatch #5833
heathley wants to merge 1 commit into
NousResearch:mainfrom
heathley:fix/slack-thread-session-key-mismatch

Conversation

@heathley

@heathley heathley commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Problem

Thread replies without @mention were silently ignored even when the bot had an active session in that thread.

Root Cause

Fixes #5816.

The bug was introduced in 4ec615b ("feat(gateway): Enable Slack thread replies without explicit @mentions") which added _has_active_session_for_thread()with a manual key construction that duplicated — but incorrectly — the logic in build_session_key().

The manual implementation applied group_sessions_per_user correctly but missed thread_sessions_per_user=False (default), which intentionally omitsuser_id for shared thread sessions:

Session stored under: ...channel_id:thread_ts
Lookup performed with: ...channel_id:thread_ts:user_id → never matches.

Fix

Replace manual key construction with build_session_key() — single source of truth, introduced in the same session infrastructure the feature was supposed to use.

_has_active_session_for_thread() was manually constructing the session key
and always appending user_id when group_sessions_per_user=True. This missed
the thread_sessions_per_user=False (default) logic in build_session_key()
which intentionally omits user_id for shared thread sessions.

The key mismatch caused all thread replies without @mention to be silently
ignored even when the bot had an active session in that thread.

Fix: delegate to build_session_key() — single source of truth.

Fixes NousResearch#5816
@heathley heathley changed the title ix/slack-thread-session-key-mismatch Thread replies without @mention were silently ignored even when the bot had fix/slack-thread-session-key-mismatch Apr 7, 2026
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 fix (using build_session_key() instead of manual key construction) was exactly the right approach — we incorporated it directly. Thank you @heathley!

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

Bug: Slack thread replies without @mention ignored despite active session

2 participants