Skip to content

fix(gateway): delete stale Discord slash commands before creating new ones - #24371

Closed
itenev wants to merge 2 commits into
NousResearch:mainfrom
itenev:fix/discord-slash-command-sync-order
Closed

fix(gateway): delete stale Discord slash commands before creating new ones#24371
itenev wants to merge 2 commits into
NousResearch:mainfrom
itenev:fix/discord-slash-command-sync-order

Conversation

@itenev

@itenev itenev commented May 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a synchronization bug in the Discord platform adapter that causes command registration to fail when stale commands from old versions push the total count over Discord's hard limit of 100 global application commands.

In _safe_sync_slash_commands(), stale (removed) commands were deleted AFTER new commands were created/updated. This means that during the sync, the temporary command count on Discord could exceed 100 (stale + new), causing Discord to reject the entire batch. This fix reverses the order so stale commands are always deleted first, guaranteeing the active command count never exceeds the desired count during sync.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

- gateway/platforms/discord.py: In _safe_sync_slash_commands(), moved the stale command deletion loop to execute BEFORE the create/update loop. Changed the deletion logic from iterating over remaining entries in existing_by_key (post-update) to computing stale_keys = [k for k in existing_by_key if k not in desired_by_key] upfront and deleting them before any upsert_global_command calls.
- Removed the redundant post-loop deletion block that was no longer needed after the reordering.

How to Test

1. Set up a Discord bot application with 95+ existing slash commands that are no longer in the current desired set
2. Configure hermes-agent with 10+ new slash commands not currently registered
3. Start the gateway and observe the Discord sync log — verify that stale commands are deleted first, then new commands are registered, and the sync completes without exceeding Discord's 100 command limit
4. Verify pytest tests/gateway/test_discord_connect.py -q passes (specifically test_safe_sync_slash_commands_only_mutates_diffs)

Checklist

Code

- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix/feature (no unrelated commits)
- [x] I've run pytest tests/ -q and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform:Ubuntu 24.04

Documentation & Housekeeping

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

… ones

When syncing slash commands, stale (removed) commands were deleted AFTER
new commands were created. If stale commands from old versions brought the
total close to Discord's hard limit of 100 global application commands,
adding new commands via upsert_global_command would exceed the limit and
Discord would reject the entire sync batch.

Fix: identify stale keys (existing but not in desired set) and delete them
FIRST, before the create/update loop. This guarantees the total never
exceeds 100 during sync.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter labels May 12, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #19366 — same fix (reorder Discord slash-command sync to prune stale commands before creating new ones). Please coordinate or close one.

…mmand sync

- test_safe_sync_slash_commands_deletes_stale_before_creating: Verifies
  all delete_global_command calls happen before any upsert/edit calls,
  guarding against regression of the >100 commands limit bug.

- test_safe_sync_slash_commands_deletion_error_does_not_block_sync:
  Verifies that a failing stale command deletion is caught and logged,
  allowing the sync to continue creating/updating remaining commands.
@loki-hermes-agent
loki-hermes-agent Bot force-pushed the fix/discord-slash-command-sync-order branch from fc93b26 to df3765a Compare May 12, 2026 13:53
@itenev

itenev commented May 12, 2026

Copy link
Copy Markdown
Contributor Author
Confirmed. PR #19366 by aaronsagents-stack (May 3, 2026) is a duplicate of our fix. Here's the comparison:

PR #19366 vs #24371

| | #19366 (aaronsagents-stack) | #24371 (ours) |
|---|---|---|
| Core fix | Delete stale before create | Same |
| Uses mutate() wrapper for deletes | No (calls raw http.delete_global_command) | Yes (respects rate-limit pacing) |
| Error resilience | None (one failure blocks entire sync) | try/except per deletion, continues on failure |
| Removes old redundant deletion block | No (leaves dead code) | Yes |
| Tests | 1 test (ordering only) | 2 tests (ordering + error resilience) |
| Scope | Includes unrelated create_channel feature change | Focused: single bug fix only |

Issues with #19366

1. Bypasses mutate() wrapper — Calls http.delete_global_command directly instead of going through mutate(), which means stale deletions skip the _sleep_between_command_sync_mutations() rate-limit pacing. This could trigger Discord rate limits during bulk stale deletion.

2. No error handling — A single failed delete_global_command raises uncaught, aborting the entire sync. New commands won't be created even though the stale deletion failure might be transient.

3. Leaves dead code — The old post-loop for current in existing_by_key.values(): delete block remains. After the new stale deletion pops everything, that loop becomes a no-op, but it's confusing and wasteful.

4. Mixed scope — Bundles an unrelated create_channel action feature into a bug fix PR, violating the one-logical-change-per-PR rule.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing in favor of #46078 (merged). As noted in that PR, the delete-before-create reordering here helps with stale/transient over-count but doesn't address the case where the desired set itself exceeds 100 — which is the recurring failure. It also targets the moved gateway/platforms/discord.py. Appreciate the work narrowing this down.

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/discord Discord bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants