hermes - ✅(Solved) Fix fix(slack): close previous handler/task in connect() to prevent zombie Socket Mode connections [2 pull requests, 1 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
NousResearch/hermes-agent#18980Fetched 2026-05-03 04:53:08
View on GitHub
Comments
0
Participants
1
Timeline
6
Reactions
0
Participants
Timeline (top)
labeled ×4cross-referenced ×2

Error Message

except Exception:

Root Cause

`gateway/platforms/slack.py` `connect()` (line ~533):

```python self._app = AsyncApp(token=primary_token) # old _app ref dropped, not closed ... self._handler = AsyncSocketModeHandler(...) # old _handler ref dropped, not closed self._socket_mode_task = asyncio.create_task(...) # old task orphaned, still running ```

There is no guard equivalent to the one `DiscordAdapter.connect()` gained in #18758 (merged today).

Fix Action

Fix

Add the same close-before-reassign guard at the top of `SlackAdapter.connect()`:

```python if self._socket_mode_task and not self._socket_mode_task.done(): self._socket_mode_task.cancel() try: await self._socket_mode_task except asyncio.CancelledError: pass self._socket_mode_task = None if self._handler is not None: try: await self._handler.close_async() except Exception: logger.debug("[%s] Failed to close previous Slack handler", self.name) finally: self._handler = None self._app = None ```

Parity fix — adds the same guard that `DiscordAdapter.connect()` already has.

PR fix notes

PR #18982: fix(slack): close previous handler in connect() to prevent zombie Socket Mode connections

Description (problem / solution / changelog)

What does this PR do?

SlackAdapter.connect() overwrote self._handler, self._app, and self._socket_mode_task without closing the prior AsyncSocketModeHandler first. If connect() was called a second time on the same adapter (e.g. during a gateway restart or in-process reconnect attempt), the old Socket Mode websocket stayed alive. Both the old and new connections received every Slack event and dispatched it twice — producing double responses with different wording.

This is the same bug that affected DiscordAdapter (#18187, fixed in #18758 merged today). Parity fix: adds the same close-before-reassign guard that DiscordAdapter.connect() already has.

One-line guard in connect()if self._handler is not None: await self._handler.close_async() followed by finally: self._handler = None; self._app = None. When _handler is None (fresh adapter, first connect()) the block is a harmless no-op. Scoped to the handler/app fields only — no behavior change for any path that does not call connect() twice.

Related Issue

Fixes #18980

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests
  • ♻️ Refactor
  • 🎯 New skill

Changes Made

  • gateway/platforms/slack.py: add close-before-reassign guard in connect() before self._app = AsyncApp(...) (+10 lines)
  • tests/gateway/test_slack.py: add test_reconnect_closes_previous_handler_to_prevent_zombie_socket to TestSlackConnectCleanup (+54 lines)

How to Test

python3.11 -m pytest tests/gateway/test_slack.py::TestSlackConnectCleanup -v --override-ini="addopts="

Checklist

Code

  • Contributing Guide read
  • Conventional Commits
  • No duplicate PR
  • Single logical change only
  • pytest passing
  • Tests added
  • Platform: macOS

Documentation & Housekeeping

  • Docs updated — N/A
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md/AGENTS.md — N/A
  • Cross-platform impact — N/A
  • Tool descriptions — N/A

Changed files

  • gateway/platforms/slack.py (modified, +15/-0)
  • tests/gateway/test_slack.py (modified, +49/-0)

PR #19024: test(docker): align Dockerfile contract tests with simplified TUI flow

Description (problem / solution / changelog)

Summary

Fixes two Tests failures observed on main (and therefore propagating to every open PR):

FAILED tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_installs_tui_dependencies
FAILED tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package

Reference run: 25250051126 on 5d3be898a.

Root cause

The Dockerfile dropped the manual @hermes/ink materialisation gymnastics in favour of letting npm workspaces resolve the bundled package naturally. Two contract tests still asserted the older flow.

test_dockerfile_installs_tui_dependencies required:

assert "ui-tui/packages/hermes-ink/package-lock.json" in dockerfile_text

…but the lockfile is no longer COPY'd individually — the entire ui-tui/packages/hermes-ink/ tree is COPY'd instead (the workspace reference from ui-tui/package.json is file: so npm needs the real source, not just a manifest stub).

test_dockerfile_materializes_local_tui_ink_package required a 7-clause conjunction matching very specific rm -rf / npm install --omit=dev / --prefix node_modules/@hermes/ink / rm -rf .../react invocations that were stripped out when the workspace resolution was simplified.

Fix

Pin the contract the image has to satisfy (zombie reaping + bundled workspace package resolves) rather than the exact shell incantations the old flow used:

  • TUI deps install: ui-tui/package.json + ui-tui/package-lock.json + ui-tui/packages/hermes-ink/ tree are all COPY'd, and an npm install/ci step runs in ui-tui.
  • Bundled hermes-ink: the workspace package source is COPY'd (so await import('@hermes/ink') resolves at runtime).

Keeps the spirit of #15012 (zombie reaping) / #16690 (bundled workspace materialisation) without locking the Dockerfile into one specific implementation flavour.

Validation

$ pytest tests/tools/test_dockerfile_pid1_reaping.py -q
6 passed in 1.43s

Scope

  • ✅ No production code change (test-only)
  • ✅ All 6 dockerfile contract tests pass
  • ✅ The pid1/tini contract (the actual reason the file exists) is unchanged

Out of scope

The other ~9 main-CI failures — separate focused PRs (#18972, #18974, #18977, #18979 already up; #18980 dotenv inbound; this one is the next).

Changed files

  • tests/tools/test_dockerfile_pid1_reaping.py (modified, +19/-11)
RAW_BUFFERClick to expand / collapse

Bug

`SlackAdapter.connect()` overwrites `self._handler`, `self._app`, and `self._socket_mode_task` without closing the previous ones first. If `connect()` is called a second time on the same adapter (e.g., during a gateway restart or in-process reconnect attempt), the old `AsyncSocketModeHandler` and its background task remain alive with an open Socket Mode websocket. Both the old and new connections receive every incoming Slack event, dispatching it twice — producing double responses with different wording, similar to #18187 (which was the same bug in `DiscordAdapter`).

Root Cause

`gateway/platforms/slack.py` `connect()` (line ~533):

```python self._app = AsyncApp(token=primary_token) # old _app ref dropped, not closed ... self._handler = AsyncSocketModeHandler(...) # old _handler ref dropped, not closed self._socket_mode_task = asyncio.create_task(...) # old task orphaned, still running ```

There is no guard equivalent to the one `DiscordAdapter.connect()` gained in #18758 (merged today).

Fix

Add the same close-before-reassign guard at the top of `SlackAdapter.connect()`:

```python if self._socket_mode_task and not self._socket_mode_task.done(): self._socket_mode_task.cancel() try: await self._socket_mode_task except asyncio.CancelledError: pass self._socket_mode_task = None if self._handler is not None: try: await self._handler.close_async() except Exception: logger.debug("[%s] Failed to close previous Slack handler", self.name) finally: self._handler = None self._app = None ```

Parity fix — adds the same guard that `DiscordAdapter.connect()` already has.

extent analysis

TL;DR

To fix the issue, add a close-before-reassign guard at the top of SlackAdapter.connect() to properly close the previous connections before establishing new ones.

Guidance

  • Verify that the connect() method is being called multiple times on the same adapter, causing the old connections to remain alive and receive duplicate events.
  • Check the gateway/platforms/slack.py file, specifically the connect() method, to ensure that the guard is not already implemented.
  • Implement the proposed fix by adding the close-before-reassign guard at the top of SlackAdapter.connect(), as shown in the provided code snippet.
  • Test the fix by calling connect() multiple times on the same adapter and verifying that only one connection is established and events are not duplicated.

Example

if self._socket_mode_task and not self._socket_mode_task.done():
    self._socket_mode_task.cancel()
    try:
        await self._socket_mode_task
    except asyncio.CancelledError:
        pass
    self._socket_mode_task = None
if self._handler is not None:
    try:
        await self._handler.close_async()
    except Exception:
        logger.debug("[%s] Failed to close previous Slack handler", self.name)
    finally:
        self._handler = None
        self._app = None

Notes

This fix is a parity fix, adding the same guard that DiscordAdapter.connect() already has, and assumes that the issue is caused by the lack of this guard in SlackAdapter.connect().

Recommendation

Apply the workaround by adding the close-before-reassign guard to SlackAdapter.connect(), as this fix directly addresses the identified root cause of the issue.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING

hermes - ✅(Solved) Fix fix(slack): close previous handler/task in connect() to prevent zombie Socket Mode connections [2 pull requests, 1 participants]