mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(gateway/discord): read permission attrs from AppCommand, canonicalize contexts
Follow-up to Magaav's safe sync policy. Two gaps in the canonicalizer caused false diffs or silent drift: 1. discord.py's AppCommand.to_dict() omits nsfw, dm_permission, and default_member_permissions — those live only on attributes. The canonicalizer was reading them via payload.get() and getting defaults (False/True/None), while the desired side from Command.to_dict(tree) had the real values. Any command using non-default permissions false-diffed on every startup. Pull them from the AppCommand attributes via _existing_command_to_payload(). 2. contexts and integration_types weren't canonicalized at all, so drift in either was silently ignored. Added both to _canonicalize_app_command_payload (sorted for stable compare). Also normalized default_member_permissions to str-or-None since the server emits strings but discord.py stores ints locally. Added regression tests for both gaps.
This commit is contained in:
parent
a1ff6b45ea
commit
b61ac8964b
2 changed files with 228 additions and 4 deletions
|
|
@ -845,13 +845,21 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||||
|
|
||||||
def _canonicalize_app_command_payload(self, payload: Dict[str, Any]) -> Dict[str, Any]:
|
def _canonicalize_app_command_payload(self, payload: Dict[str, Any]) -> Dict[str, Any]:
|
||||||
"""Reduce command payloads to the semantic fields Hermes manages."""
|
"""Reduce command payloads to the semantic fields Hermes manages."""
|
||||||
|
contexts = payload.get("contexts")
|
||||||
|
integration_types = payload.get("integration_types")
|
||||||
return {
|
return {
|
||||||
"type": int(payload.get("type", 1) or 1),
|
"type": int(payload.get("type", 1) or 1),
|
||||||
"name": str(payload.get("name", "") or ""),
|
"name": str(payload.get("name", "") or ""),
|
||||||
"description": str(payload.get("description", "") or ""),
|
"description": str(payload.get("description", "") or ""),
|
||||||
"default_member_permissions": payload.get("default_member_permissions"),
|
"default_member_permissions": self._normalize_permissions(
|
||||||
"dm_permission": payload.get("dm_permission", True),
|
payload.get("default_member_permissions")
|
||||||
|
),
|
||||||
|
"dm_permission": bool(payload.get("dm_permission", True)),
|
||||||
"nsfw": bool(payload.get("nsfw", False)),
|
"nsfw": bool(payload.get("nsfw", False)),
|
||||||
|
"contexts": sorted(int(c) for c in contexts) if contexts else None,
|
||||||
|
"integration_types": (
|
||||||
|
sorted(int(i) for i in integration_types) if integration_types else None
|
||||||
|
),
|
||||||
"options": [
|
"options": [
|
||||||
self._canonicalize_app_command_option(item)
|
self._canonicalize_app_command_option(item)
|
||||||
for item in payload.get("options", []) or []
|
for item in payload.get("options", []) or []
|
||||||
|
|
@ -859,6 +867,37 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||||
],
|
],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _normalize_permissions(value: Any) -> Optional[str]:
|
||||||
|
"""Discord emits default_member_permissions as str server-side but discord.py
|
||||||
|
sets it as int locally. Normalize to str-or-None so the comparison is stable."""
|
||||||
|
if value is None:
|
||||||
|
return None
|
||||||
|
return str(value)
|
||||||
|
|
||||||
|
def _existing_command_to_payload(self, command: Any) -> Dict[str, Any]:
|
||||||
|
"""Build a canonical-ready dict from an AppCommand.
|
||||||
|
|
||||||
|
discord.py's AppCommand.to_dict() does NOT include nsfw,
|
||||||
|
dm_permission, or default_member_permissions (they live only on the
|
||||||
|
attributes). Pull them from the attributes so the canonicalizer sees
|
||||||
|
the real server-side values instead of defaults — otherwise any
|
||||||
|
command using non-default permissions would diff on every startup.
|
||||||
|
"""
|
||||||
|
payload = dict(command.to_dict())
|
||||||
|
nsfw = getattr(command, "nsfw", None)
|
||||||
|
if nsfw is not None:
|
||||||
|
payload["nsfw"] = bool(nsfw)
|
||||||
|
guild_only = getattr(command, "guild_only", None)
|
||||||
|
if guild_only is not None:
|
||||||
|
payload["dm_permission"] = not bool(guild_only)
|
||||||
|
default_permissions = getattr(command, "default_member_permissions", None)
|
||||||
|
if default_permissions is not None:
|
||||||
|
payload["default_member_permissions"] = getattr(
|
||||||
|
default_permissions, "value", default_permissions
|
||||||
|
)
|
||||||
|
return payload
|
||||||
|
|
||||||
def _canonicalize_app_command_option(self, payload: Dict[str, Any]) -> Dict[str, Any]:
|
def _canonicalize_app_command_option(self, payload: Dict[str, Any]) -> Dict[str, Any]:
|
||||||
return {
|
return {
|
||||||
"type": int(payload.get("type", 0) or 0),
|
"type": int(payload.get("type", 0) or 0),
|
||||||
|
|
@ -940,13 +979,14 @@ class DiscordAdapter(BasePlatformAdapter):
|
||||||
created += 1
|
created += 1
|
||||||
continue
|
continue
|
||||||
|
|
||||||
current_payload = self._canonicalize_app_command_payload(current.to_dict())
|
current_existing_payload = self._existing_command_to_payload(current)
|
||||||
|
current_payload = self._canonicalize_app_command_payload(current_existing_payload)
|
||||||
desired_payload = self._canonicalize_app_command_payload(desired)
|
desired_payload = self._canonicalize_app_command_payload(desired)
|
||||||
if current_payload == desired_payload:
|
if current_payload == desired_payload:
|
||||||
unchanged += 1
|
unchanged += 1
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if self._patchable_app_command_payload(current.to_dict()) == self._patchable_app_command_payload(desired):
|
if self._patchable_app_command_payload(current_existing_payload) == self._patchable_app_command_payload(desired):
|
||||||
await http.delete_global_command(app_id, current.id)
|
await http.delete_global_command(app_id, current.id)
|
||||||
await http.upsert_global_command(app_id, desired)
|
await http.upsert_global_command(app_id, desired)
|
||||||
recreated += 1
|
recreated += 1
|
||||||
|
|
|
||||||
|
|
@ -471,3 +471,187 @@ async def test_post_connect_initialization_skips_sync_when_policy_off(monkeypatc
|
||||||
await adapter._run_post_connect_initialization()
|
await adapter._run_post_connect_initialization()
|
||||||
|
|
||||||
fake_tree.sync.assert_not_called()
|
fake_tree.sync.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_safe_sync_reads_permission_attrs_from_existing_command():
|
||||||
|
"""Regression: AppCommand.to_dict() in discord.py does NOT include
|
||||||
|
nsfw, dm_permission, or default_member_permissions — they live only
|
||||||
|
on the attributes. Without reading those attrs, any command with
|
||||||
|
non-default permissions false-diffs on every startup.
|
||||||
|
"""
|
||||||
|
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:
|
||||||
|
"""Mirrors discord.py's AppCommand — to_dict() omits nsfw/dm/perms."""
|
||||||
|
|
||||||
|
def __init__(self, command_id, name, description, *, nsfw, guild_only, default_permissions):
|
||||||
|
self.id = command_id
|
||||||
|
self.name = name
|
||||||
|
self.description = description
|
||||||
|
self.type = SimpleNamespace(value=1)
|
||||||
|
self.nsfw = nsfw
|
||||||
|
self.guild_only = guild_only
|
||||||
|
self.default_member_permissions = (
|
||||||
|
SimpleNamespace(value=default_permissions)
|
||||||
|
if default_permissions is not None
|
||||||
|
else None
|
||||||
|
)
|
||||||
|
|
||||||
|
def to_dict(self):
|
||||||
|
# Match real AppCommand.to_dict() — no nsfw/dm_permission/default_member_permissions
|
||||||
|
return {
|
||||||
|
"id": self.id,
|
||||||
|
"type": 1,
|
||||||
|
"application_id": 999,
|
||||||
|
"name": self.name,
|
||||||
|
"description": self.description,
|
||||||
|
"name_localizations": {},
|
||||||
|
"description_localizations": {},
|
||||||
|
"options": [],
|
||||||
|
}
|
||||||
|
|
||||||
|
desired = {
|
||||||
|
"name": "admin",
|
||||||
|
"description": "Admin-only command",
|
||||||
|
"type": 1,
|
||||||
|
"options": [],
|
||||||
|
"nsfw": True,
|
||||||
|
"dm_permission": False,
|
||||||
|
"default_member_permissions": "8",
|
||||||
|
}
|
||||||
|
# Existing command has matching attrs — should report unchanged, NOT falsely diff.
|
||||||
|
existing = _ExistingCommand(
|
||||||
|
42,
|
||||||
|
"admin",
|
||||||
|
"Admin-only command",
|
||||||
|
nsfw=True,
|
||||||
|
guild_only=True,
|
||||||
|
default_permissions=8,
|
||||||
|
)
|
||||||
|
|
||||||
|
fake_tree = SimpleNamespace(
|
||||||
|
get_commands=lambda: [_DesiredCommand(desired)],
|
||||||
|
fetch_commands=AsyncMock(return_value=[existing]),
|
||||||
|
)
|
||||||
|
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()
|
||||||
|
|
||||||
|
# Without the fix, this would be unchanged=0, recreated=1 (false diff).
|
||||||
|
assert summary == {
|
||||||
|
"total": 1,
|
||||||
|
"unchanged": 1,
|
||||||
|
"updated": 0,
|
||||||
|
"recreated": 0,
|
||||||
|
"created": 0,
|
||||||
|
"deleted": 0,
|
||||||
|
}
|
||||||
|
fake_http.edit_global_command.assert_not_awaited()
|
||||||
|
fake_http.delete_global_command.assert_not_awaited()
|
||||||
|
fake_http.upsert_global_command.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_safe_sync_detects_contexts_drift():
|
||||||
|
"""Regression: contexts and integration_types must be canonicalized
|
||||||
|
so drift in those fields triggers reconciliation. Without this, the
|
||||||
|
diff silently reports 'unchanged' and never reconciles.
|
||||||
|
"""
|
||||||
|
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.description = payload["description"]
|
||||||
|
self.type = SimpleNamespace(value=1)
|
||||||
|
self.nsfw = payload.get("nsfw", False)
|
||||||
|
self.guild_only = not payload.get("dm_permission", True)
|
||||||
|
self.default_member_permissions = None
|
||||||
|
self._payload = payload
|
||||||
|
|
||||||
|
def to_dict(self):
|
||||||
|
return {
|
||||||
|
"id": self.id,
|
||||||
|
"type": 1,
|
||||||
|
"application_id": 999,
|
||||||
|
"name": self.name,
|
||||||
|
"description": self.description,
|
||||||
|
"name_localizations": {},
|
||||||
|
"description_localizations": {},
|
||||||
|
"options": [],
|
||||||
|
"contexts": self._payload.get("contexts"),
|
||||||
|
"integration_types": self._payload.get("integration_types"),
|
||||||
|
}
|
||||||
|
|
||||||
|
desired = {
|
||||||
|
"name": "help",
|
||||||
|
"description": "Show available commands",
|
||||||
|
"type": 1,
|
||||||
|
"options": [],
|
||||||
|
"nsfw": False,
|
||||||
|
"dm_permission": True,
|
||||||
|
"default_member_permissions": None,
|
||||||
|
"contexts": [0, 1, 2],
|
||||||
|
"integration_types": [0, 1],
|
||||||
|
}
|
||||||
|
existing = _ExistingCommand(
|
||||||
|
77,
|
||||||
|
{
|
||||||
|
**desired,
|
||||||
|
"contexts": [0], # server-side only
|
||||||
|
"integration_types": [0],
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
fake_tree = SimpleNamespace(
|
||||||
|
get_commands=lambda: [_DesiredCommand(desired)],
|
||||||
|
fetch_commands=AsyncMock(return_value=[existing]),
|
||||||
|
)
|
||||||
|
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()
|
||||||
|
|
||||||
|
# contexts and integration_types are not patchable by
|
||||||
|
# edit_global_command, so the command must be recreated.
|
||||||
|
assert summary["unchanged"] == 0
|
||||||
|
assert summary["recreated"] == 1
|
||||||
|
assert summary["updated"] == 0
|
||||||
|
fake_http.edit_global_command.assert_not_awaited()
|
||||||
|
fake_http.delete_global_command.assert_awaited_once_with(999, 77)
|
||||||
|
fake_http.upsert_global_command.assert_awaited_once_with(999, desired)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue