Skip to content

fix(discord): avoid poisoning shared rate-limit buckets - #88666

Open
alexskatell wants to merge 1 commit into
NousResearch:mainfrom
alexskatell:fix/discord-auto-thread-rate-limits
Open

fix(discord): avoid poisoning shared rate-limit buckets#88666
alexskatell wants to merge 1 commit into
NousResearch:mainfrom
alexskatell:fix/discord-auto-thread-rate-limits

Conversation

@alexskatell

@alexskatell alexskatell commented Aug 17, 2026

Copy link
Copy Markdown

What does this PR do?

Discord route buckets can inherit a temporary 30-second rate-limit ceiling during post-connect slash-command synchronization. A production thread request later received retry_after=136.57; discord.py raised RateLimited instead of waiting, and Hermes entered its seed-message fallback, posted two duplicate notices, then abandoned the request.

HTTPClient.max_ratelimit_timeout is shared mutable state. Discord.py copies it into each route bucket when the bucket is created, so restoring the client property does not repair a thread bucket created during the synchronization window.

This PR removes that shared mutation. Slash-command synchronization remains bounded by the existing 600-second asyncio.wait_for(), while unrelated Discord routes keep the client's normal rate-limit policy.

The tradeoff is that command synchronization can wait inside discord.py instead of surfacing and persisting an exact retry_after immediately. The existing operation timeout still caps the wait, and avoiding cross-route state corruption is the safer boundary.

Related Issue

Fixes #88653

PR #76060 handles residual 429 responses inside auto-thread creation. It does not remove the slash-sync mutation that caused the observed thread bucket to retain a 30-second ceiling. This PR stays separate from that function so the two changes do not conflict.

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

  • plugins/platforms/discord/adapter.py
    • Stop changing the shared Discord HTTP client's max_ratelimit_timeout during post-connect command synchronization.
    • Keep the existing 600-second whole-operation timeout.
    • Document why a temporary client-level override leaks into unrelated route buckets.
  • tests/gateway/test_discord_connect.py
    • Add a regression test that observes the shared limit while synchronization runs and proves it remains unchanged.

How to Test

  1. Run the new regression test on origin/main. It fails because synchronization exposes 30.0 instead of the configured None.
  2. Run the same test on this branch: 1 passed.
  3. Run scripts/run_tests.sh tests/gateway/test_discord*.py -q: 251 passed, 1 skipped, 0 failed.
  4. Run Ruff 0.15.10 on the two changed files: clean.
  5. Run py_compile on the two changed files and git diff --check: clean.
  6. Run scripts/run_tests.sh tests/ -q with the exact CI dependency set: 34,352 passed, 343 skipped, 20 failed across 15 unrelated files on macOS. Replaying those 15 files against parent cf7b3d0c90 fails the same 15 files; neither changed file fails.
  7. Run the exact candidate on Linux through the fork CI harness: 11 of 12 Python slices pass. Slice 3 fails only tests/tui_gateway/test_slash_worker_mcp_discovery.py; the single allowed rerun fails the same unrelated test. Windows, macOS-only, lint, security, and the other Linux slices pass.

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 26.6, Python 3.11.15

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings): inline rationale updated; other docs N/A
  • I've updated cli-config.yaml.example if I added/changed config keys: N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows: N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide: network-only behavior; N/A
  • I've updated tool descriptions/schemas if I changed tool behavior: N/A

Screenshots / Logs

Production evidence:

Discord requested retry_after=136.57 seconds.
The affected thread route bucket retained max_ratelimit_timeout=30.0.
Hermes posted two seed messages, then returned:
Hermes could not create a Discord thread for this message, so the request was not processed.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/discord Discord bot adapter P3 Low — cosmetic, nice to have labels Aug 17, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Reviewed by reviewer-e (AI automated review).

Right call on the shared-state hazard: temporarily lowering http.max_ratelimit_timeout looks scoped but Discord buckets snapshot that value at creation, so the override permanently leaked into every unrelated concurrent route touched during the sync window. Dropping the mutation keeps the sync bounded via asyncio.wait_for(600) with retry-after persistence intact, and the new test observes the limit inside the synced operation to prove the shared client is never mutated rather than just checking it's restored after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/discord Discord bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Discord auto-thread requests fail under long rate limits

3 participants