Skip to content

feat(slack): add multi-workspace Socket Mode - #20587

Closed
jordanhubbard wants to merge 2 commits into
NousResearch:mainfrom
jordanhubbard:feature/slack-multi-workspace-socket-mode-v2
Closed

feat(slack): add multi-workspace Socket Mode#20587
jordanhubbard wants to merge 2 commits into
NousResearch:mainfrom
jordanhubbard:feature/slack-multi-workspace-socket-mode-v2

Conversation

@jordanhubbard

Copy link
Copy Markdown
Contributor

Summary

  • add ~/.hermes/slack_accounts.json for multiple Slack account configs
  • open one AsyncApp + Socket Mode handler per {bot_token, app_token} account
  • acquire and release per-app-token gateway locks
  • persist channel→team routing so outbound sends after restart use the right workspace client
  • preserve existing SLACK_BOT_TOKEN + SLACK_APP_TOKEN fallback and send-only extra bot token behavior

Stack

This is stacked on #20583, which scopes Slack sessions/routing keys by workspace. Review/merge #20583 first; this PR adds true multi-workspace receive support on top.

Tests

  • uv run pytest tests/gateway/test_session.py tests/gateway/test_slack.py tests/gateway/test_slack_approval_buttons.py -q

Advances #10099.

@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter P2 Medium — degraded but workaround exists labels May 6, 2026
@jordanhubbard
jordanhubbard force-pushed the feature/slack-multi-workspace-socket-mode-v2 branch from 04e186e to e8cfe4c Compare May 28, 2026 00:33
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the multi-app Socket Mode implementation. The feature premise is still valid on current main: plugins/platforms/slack/adapter.py:962-1054 reads one app token and starts one Socket Mode app/handler.

Problems

  • The PR modifies gateway/platforms/slack.py, but commit 5600105478ffde29d7566b45421b100eaa29c4ef moved the adapter to plugins/platforms/slack/adapter.py; this needs a semantic port, not a cherry-pick.
  • The multi-handler design must retain current per-socket recovery. Main starts the handler and watchdog together at plugins/platforms/slack/adapter.py:1241-1249; its watchdog handles failed tasks/transports at :572-661. The PR's per-account create_task() path has no corresponding monitor.
  • The gateway/session.py change scopes every platform with guild_id, while current Discord callers provide it (plugins/platforms/discord/adapter.py:3208, :6334). That changes existing non-Slack session keys without a migration path.
  • Preserve newer Slack handlers when porting: main dispatches file_shared to _handle_slack_file_shared() at plugins/platforms/slack/adapter.py:1105-1107.

Suggested changes

  • Port into the bundled Slack plugin and model lifecycle/watchdog state per account.
  • Limit session-key changes to Slack or add a deliberate compatibility migration with cross-platform coverage.
  • Add multi-account recovery, file-share, action-handler, and colliding-workspace-ID tests.

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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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
@jordanhubbard

Copy link
Copy Markdown
Contributor Author

Superseded by #66398 — a semantic port of this change onto the current bundled Slack adapter (plugins/platforms/slack/adapter.py) that resolves the hermes-sweeper review: ported into the plugin, per-account watchdog recovery, Slack-scoped session keys (no Discord/guild_id regression), file_shared kept dispatching to _handle_slack_file_shared, plus multi-account recovery / file-share / action-handler / colliding-workspace-ID tests. Suggest closing this in favor of #66398 once that lands.

@jordanhubbard

Copy link
Copy Markdown
Contributor Author

Closing in favor of #66398, the semantic port onto the current bundled Slack adapter that resolves the hermes-sweeper review (plugin location, per-account watchdog recovery, Slack-scoped session keys, preserved file_shared, and the requested multi-account/file-share/action-handler/colliding-workspace-ID tests). Continue review on #66398.

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 P2 Medium — degraded but workaround exists 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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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.

3 participants