Skip to content

feat(gateway): unified thread context hook in BasePlatformAdapter - #7466

Closed
Tranquil-Flow wants to merge 3 commits into
NousResearch:mainfrom
Tranquil-Flow:feat/unified-thread-context
Closed

feat(gateway): unified thread context hook in BasePlatformAdapter#7466
Tranquil-Flow wants to merge 3 commits into
NousResearch:mainfrom
Tranquil-Flow:feat/unified-thread-context

Conversation

@Tranquil-Flow

@Tranquil-Flow Tranquil-Flow commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds fetch_thread_context() and has_active_session_for_event() to BasePlatformAdapter, providing a single extension point for platform adapters to fetch thread/conversation history when the bot first enters an existing thread.

  • Slack: refactors existing _fetch_thread_context() into the new base class hook (no behavioral change for Slack users)
  • Discord: adds fetch_thread_context() override using channel.history(), giving Discord feature parity with Slack
  • Other platforms (Telegram, Matrix, Signal, etc.): unaffected — the base default returns None

Key design decisions

  • Session liveness check evaluates reset policyhas_active_session_for_event() calls _should_reset() on the entry, so expired sessions (idle timeout, daily reset) still get thread context seeded into the new session
  • Commands are not prefixedis_command() guard in the base call site prevents /reset, /status, etc. from having context prepended, which would break command parsing
  • Slack auto-trigger gate aligned_has_active_session_for_thread() now delegates to the shared has_active_session_for_event(), so the "respond without @mention" check also respects session expiry

Related Issue

Fixes #6708

Related to #6712 (absorbs Discord thread context approach into unified base class pattern); #1953, #2950, #5816 (already fixed for Slack by #5890; this refactors the Slack implementation into a reusable hook); #6345, #7304 (complementary features — agent-side history tool and restart resilience).

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

  • gateway/platforms/base.pyhas_active_session_for_event(), fetch_thread_context(), call site in _process_message_background()
  • gateway/platforms/slack.pyfetch_thread_context() override, _has_active_session_for_thread() delegation, removed old inline call site
  • gateway/platforms/discord.pyfetch_thread_context() override using channel.history()
  • tests/gateway/test_thread_context_hook.py — 18 new tests (base helper, integration, Discord override)
  • tests/gateway/test_slack_approval_buttons.py — updated existing mocks for reset policy check

How to Test

  1. Slack (no behavioral change expected):
    • Mention the bot mid-thread — should see [Thread context] in the first response
    • Subsequent messages in the same thread should NOT re-fetch context
    • /reset inside a thread should work as a command, not get context prepended
  2. Discord (new feature):
    • Create a thread with some messages, then @mention the bot
    • First response should include awareness of prior thread messages
    • Commands like /new inside threads should work normally
  3. Session expiry:
    • Let a session expire (idle timeout), then message the bot in the same thread
    • Should re-fetch thread context for the new session

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 (Darwin 24.6.0)

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

18 new + 128 related tests pass; 14 pre-existing failures unrelated

@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/discord Discord bot adapter platform/slack Slack app adapter labels Apr 29, 2026
@Tranquil-Flow
Tranquil-Flow force-pushed the feat/unified-thread-context branch from 007de5a to ea960da Compare May 18, 2026 21:29
@DiscountDarcy

Copy link
Copy Markdown

We've merged this into our local for two different Hermes installs -- getting the tests to pass wasn't too difficult and would be worth it to the community so we can have Hermes use the thread as a resource before wandering off to check every existing session for tips!

Add fetch_thread_context() and has_active_session_for_event() to
BasePlatformAdapter, providing a single extension point for platform
adapters to fetch thread/conversation history when the bot first
enters an existing thread.

- Refactors Slack's existing thread context fetching into the new hook
- Adds Discord implementation using channel.history(), giving Discord
  feature parity with Slack for thread context
- Session liveness check evaluates reset policy (idle/daily expiry) so
  expired sessions still get context seeded and don't auto-trigger
  without @mention
- Commands (/reset, /status, etc.) are not prefixed with context
- Base default returns None so unimplemented platforms are unaffected

Fixes NousResearch#6708
Related to NousResearch#6712 (absorbs Discord thread context into unified pattern)
Related to NousResearch#1953, NousResearch#2950, NousResearch#5816 (Slack fixes refactored into base hook)
@Tranquil-Flow
Tranquil-Flow force-pushed the feat/unified-thread-context branch from ea960da to 0be92f6 Compare May 25, 2026 11:07
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the unified treatment of thread context and for covering reset and command cases.

Problems

  • Current main already implements Discord thread history backfill in plugins/platforms/discord/adapter.py:6480-6537, carrying it through MessageEvent.channel_context (plugins/platforms/discord/adapter.py:6589) and injecting it in gateway/run.py:10371-10375. Porting this PR's Discord override would introduce a second history fetch/context block for the same thread turn.
  • The Slack target moved to plugins/platforms/slack/adapter.py in 1a38066054752d601b71fc655a3ed6bf4228e2da, so this needs a current-tree port rather than a clean application.
  • The Slack reset-policy concern remains valid: plugins/platforms/slack/adapter.py:4042-4043 treats a mapped entry as active without consulting SessionStore._should_reset() (gateway/session.py:1575-1621). Its context prepend also happens before command classification at plugins/platforms/slack/adapter.py:2886-2905.

Suggested changes

  • Preserve the existing Discord channel_context path and re-scope the salvage to the Slack plugin's reset-aware session check and command-safe context injection.

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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 12, 2026
@Tranquil-Flow

Copy link
Copy Markdown
Contributor Author

Closing — current main already implements the Discord thread history backfill and related thread context hooks. This PR's porting step conflicts with the current adapter code; the remaining gap is small and the new shape is already in place. Thanks for the review.

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/discord Discord bot adapter platform/slack Slack app adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

Discord: bot has no thread context when first mentioned in an existing thread

4 participants