From eaecc99c7ec5b6f37e880a0b69d16871cd3e4f57 Mon Sep 17 00:00:00 2001 From: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:23:33 +0200 Subject: [PATCH] fix(discord): upsert recreated slash commands without a delete-first step (#104399) A 429 between the recreated path's delete and upsert strands the command deleted until a fully-clean later sync; on reconnect-heavy deployments that never happens, so commands like /model silently vanish from the slash picker. Discord's create endpoint overwrites same-name commands, so the lone upsert recreates while keeping the command available. --- plugins/platforms/discord/adapter.py | 10 +- tests/gateway/test_discord_connect.py | 161 ++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 1 deletion(-) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index f77f812d09..2164a18cd1 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -2951,7 +2951,15 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter): summary["unchanged"] += 1 continue if self._patchable_app_command_payload(current_existing_payload) == self._patchable_app_command_payload(desired): - await mutate(http.delete_global_command, app_id, current.id) + # Upsert alone recreates the command: Discord's create endpoint + # overwrites the existing same-name command ("Returns 201 if a + # command with the same name does not already exist, or a 200 + # if it does"). Delete-first strands the command deleted when + # the small command-management bucket 429s the upsert mid-sync; + # upsert-first keeps the command available even then. The + # obsolete-path delete-first below is different: an upsert + # pushing the live total over 100 fails with 30032 (breaks ALL + # slash commands), so an app at the cap must shrink first. await mutate(http.upsert_global_command, app_id, desired) summary["recreated"] += 1 continue diff --git a/tests/gateway/test_discord_connect.py b/tests/gateway/test_discord_connect.py index ad14132519..931964e396 100644 --- a/tests/gateway/test_discord_connect.py +++ b/tests/gateway/test_discord_connect.py @@ -464,6 +464,167 @@ async def test_safe_sync_slash_commands_only_mutates_diffs(): fake_http.delete_global_command.assert_awaited_once_with(999, 13) +@pytest.mark.asyncio +async def test_safe_sync_recreated_upserts_without_delete(): + """Regression (#104399): the recreated path must not delete before upserting. + + A delete-then-upsert sequence strands the command deleted when Discord's + small command-management bucket 429s the upsert (the sync aborts; nothing + re-creates the command until a later fully-clean sync). POST-by-name is + itself an overwrite (Discord docs: "Creating a command with the same name + as an existing command for your application will overwrite the old + command"), so a lone upsert recreates the command while it stays available. + """ + adapter = DiscordAdapter(PlatformConfig(enabled=True, token="test-token")) + + class _DesiredCommand: + def __init__(self, payload): + self._payload = payload + + def to_dict(self, tree): + return dict(self._payload) + + class _ExistingCommand: + def __init__(self, command_id, payload): + self.id = command_id + self.name = payload["name"] + self.type = SimpleNamespace(value=payload["type"]) + self._payload = payload + + def to_dict(self): + return { + "id": self.id, + "application_id": 999, + **self._payload, + "name_localizations": {}, + "description_localizations": {}, + } + + desired_model = { + "name": "model", + "description": "Switch the model", + "type": 1, + "options": [], + "nsfw": False, + "dm_permission": True, + "default_member_permissions": None, + } + # Non-patchable field differs from the live command: the safe-sync + # "recreated" path (patchable payload equal, canonical payload unequal). + existing_model = _ExistingCommand( + 21, + {**desired_model, "dm_permission": False}, + ) + + fake_tree = SimpleNamespace( + get_commands=lambda: [_DesiredCommand(desired_model)], + fetch_commands=AsyncMock(return_value=[existing_model]), + ) + fake_http = SimpleNamespace( + upsert_global_command=AsyncMock(), + edit_global_command=AsyncMock(), + delete_global_command=AsyncMock(), + ) + adapter._client = SimpleNamespace( + tree=fake_tree, + http=fake_http, + application_id=999, + user=SimpleNamespace(id=999), + ) + + summary = await adapter._safe_sync_slash_commands() + + assert summary == { + "total": 1, + "unchanged": 0, + "updated": 0, + "recreated": 1, + "created": 0, + "deleted": 0, + } + fake_http.upsert_global_command.assert_awaited_once_with(999, desired_model) + # The overwrite already replaced the old command — a trailing delete is + # both unnecessary and another rate-limit-consuming mutation. + fake_http.delete_global_command.assert_not_awaited() + fake_http.edit_global_command.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_safe_sync_recreated_survives_rate_limit_between_mutations(): + """Regression (#104399): a 429 landing mid-recreate must not leave the + command deleted. With delete-first, the delete succeeds and the upsert + 429s — the command vanishes from the slash picker until a later fully-clean + sync. With upsert-first, the 429 leaves the old command live (worst case: + stale non-patchable fields) — never deleted.""" + adapter = DiscordAdapter(PlatformConfig(enabled=True, token="test-token")) + + class _DesiredCommand: + def __init__(self, payload): + self._payload = payload + + def to_dict(self, tree): + return dict(self._payload) + + class _ExistingCommand: + def __init__(self, command_id, payload): + self.id = command_id + self.name = payload["name"] + self.type = SimpleNamespace(value=payload["type"]) + self._payload = payload + + def to_dict(self): + return { + "id": self.id, + "application_id": 999, + **self._payload, + "name_localizations": {}, + "description_localizations": {}, + } + + desired_model = { + "name": "model", + "description": "Switch the model", + "type": 1, + "options": [], + "nsfw": False, + "dm_permission": True, + "default_member_permissions": None, + } + existing_model = _ExistingCommand(21, {**desired_model, "dm_permission": False}) + + fake_tree = SimpleNamespace( + get_commands=lambda: [_DesiredCommand(desired_model)], + fetch_commands=AsyncMock(return_value=[existing_model]), + ) + calls = [] + + async def _upsert(app_id, payload): + calls.append("upsert") + raise RuntimeError("429 Too Many Requests") + + async def _delete(app_id, command_id): + calls.append(f"delete:{command_id}") + + fake_http = SimpleNamespace( + upsert_global_command=_upsert, + edit_global_command=AsyncMock(), + delete_global_command=_delete, + ) + adapter._client = SimpleNamespace( + tree=fake_tree, + http=fake_http, + application_id=999, + user=SimpleNamespace(id=999), + ) + + with pytest.raises(RuntimeError, match="429"): + await adapter._safe_sync_slash_commands() + + # The only mutation attempted was the upsert — the command is never + # deleted, so it remains available despite the aborted sync. + assert calls == ["upsert"] + + @pytest.mark.asyncio async def test_post_connect_initialization_retries_fingerprint_after_timeout(tmp_path, monkeypatch): adapter = DiscordAdapter(PlatformConfig(enabled=True, token="test-token"))