Skip to content

fix(discord): avoid unnecessary members intent on startup + fix token lock leak - #5302

Merged
teknium1 merged 2 commits into
mainfrom
hermes/hermes-40531417
Apr 5, 2026
Merged

fix(discord): avoid unnecessary members intent on startup + fix token lock leak#5302
teknium1 merged 2 commits into
mainfrom
hermes/hermes-40531417

Conversation

@teknium1

@teknium1 teknium1 commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Salvaged from PR #4690 by @abhey-gupta.

Two fixes to Discord adapter startup:

1. Conditional members intent

intents.members = True was always set, but the privileged Server Members intent is only needed when the allowed users list contains usernames that need resolution to numeric IDs. If the intent isn't enabled in the Discord Developer Portal, the bot can fail to come online entirely.

Fix: Move allowed_users parsing before intents setup, then set intents.members only when non-numeric entries exist in the allowlist.

2. Token lock leak on connection failure

If connect() hits a TimeoutError or other exception after acquiring the token lock, the lock was never released — blocking future connection attempts until gateway restart.

Fix: Add release_scoped_lock() calls in both the TimeoutError and general Exception handlers.

Follow-up

  • Fixed test mock: added ButtonStyle.grey (main added ExecApprovalView after the PR branched)

Test plan

  • python -m pytest tests/gateway/test_discord_connect.py — 4/4 passed
  • Parametrized: numeric-only IDs (members=False), username (members=True), mixed (members=True)
  • Token lock release verified on timeout

Attribution

Cherry-picked from @abhey-gupta (#4690), conflict resolved against current main.

abhey-gupta and others added 2 commits April 5, 2026 12:39
Only request the privileged members intent when DISCORD_ALLOWED_USERS includes non-numeric entries that need username resolution. Also release the Discord token lock when startup fails so retries and restarts are not blocked by a stale lock.\n\nAdds regression tests for conditional intents and startup lock cleanup.
@teknium1
teknium1 merged commit 8d52267 into main Apr 5, 2026
3 of 4 checks passed
binhnt92 added a commit to binhnt92/hermes-agent that referenced this pull request Apr 6, 2026
When Slack's connect() acquires a scoped lock on the app token and then
fails (bad token, Socket Mode error), the lock is never released. The
next gateway start sees "Slack app token already in use" and refuses to
connect until the process dies.

Same issue in Signal: if the health check fails after acquiring the
phone lock, the lock is held permanently.

Discord got this fix in PR NousResearch#5302. Slack and Signal were missed.

Add lock release to both exception/failure paths. Extract Signal's
inline release logic into a reusable _release_phone_lock() helper.
Also close the leaked httpx client on Signal health check failure.
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