fix(discord): /reload-skills now refreshes the /skill autocomplete live by teknium1 · Pull Request #18754 · NousResearch/hermes-agent

teknium1 · 2026-05-02T08:58:51Z

Summary

/reload-skills was a no-op for Discord's /skill autocomplete.

_register_skill_group captured the skill catalog in closure variables (entries, skill_lookup) so the single tree.add_command at startup owned the only live copy. The closure is never re-entered, so rescanning disk and refreshing the in-process _skill_commands registry couldn't propagate to the Discord picker. Observable behavior:

Skill added at runtime → invisible in /skill autocomplete until the gateway restarts.
Skill removed at runtime → stale autocomplete entry remains; clicking it returns "Unknown skill".

Fix

Purely a dataflow change on the Discord adapter:

Promote entries / skill_lookup to instance attrs (_skill_entries, _skill_lookup) so the autocomplete + handler callbacks read live state.
Factor the collector-driven rebuild into _refresh_skill_catalog_state().
Expose a public refresh_skill_group() that re-runs the helper and is safe to call at any point after initial registration.

Gateway-side, _handle_reload_skills_command now walks self.adapters and calls refresh_skill_group() on any adapter that exposes it. Both sync and async implementations are supported; adapters that don't override it (Telegram BotCommand menu, Slack subcommand map, etc.) are silently skipped — the in-process reload_skills() call above already covers them.

No tree.sync() is needed — Discord fetches autocomplete options dynamically on every keystroke, so mutating the instance state is enough. This also sidesteps Discord's per-app command-bucket rate limit (~5 writes / 20 s), which has caused outages in the past (#16713).

Tests

tests/gateway/test_reload_skills_discord_resync.py — five cases:

test_refresh_repopulates_entries_after_catalog_change — add + remove shows up immediately.
test_refresh_sorts_entries_alphabetically — order stays stable across refreshes.
test_refresh_handles_collector_exception_gracefully — broken collector doesn't crash the gateway.
test_refresh_catalog_state_populates_instance_attrs — the shared helper populates self._skill_entries / self._skill_lookup.
test_orchestrator_calls_refresh_skill_group_on_every_adapter — gateway invokes refresh on sync + async adapters and skips no-op adapters.

All pass; 148 surrounding Discord + hermes_cli tests still pass.

fix(discord): complete #18741 for /skill autocomplete and drop legacy 25x25 caps #18745 — drop legacy 25×25 caps + complete fix(gateway): include external_dirs skills in Telegram/Discord slash commands (salvage #8790) #18741's external_dirs fix for the live collector.
fix(gateway): match disabled/optional skills by frontmatter slug, not dir name #18753 — match disabled/optional skills by frontmatter slug, not directory name.
this PR — /reload-skills actually refreshes the live Discord autocomplete.

…e live `_register_skill_group` captured the skill catalog in closure variables (`entries` and `skill_lookup`) so the single `tree.add_command` call at startup owned the only live copy. The closure is never re-entered after startup, so `/reload-skills` — which rescans the on-disk skills dir and refreshes the in-process `_skill_commands` registry — had no way to propagate results into the `/skill` autocomplete on Discord. New skills stayed invisible in the dropdown, and deleted skills returned "Unknown skill" when the stale autocomplete entry was clicked. The fix is purely a dataflow change: promote `entries` and `skill_lookup` to instance attributes (`_skill_entries`, `_skill_lookup`), split the collector-driven rebuild into a helper (`_refresh_skill_catalog_state`), and add a public `refresh_skill_group()` method that re-runs the helper and is safe to call at any point after the initial registration. The gateway's `_handle_reload_skills_command` then iterates `self.adapters` and calls `refresh_skill_group()` on any adapter that exposes it (currently only Discord). Both sync and async implementations are supported; adapters that don't override the method (Telegram's BotCommand menu, Slack subcommand map, etc.) are silently skipped — the in-process `reload_skills()` call covers them. No `tree.sync()` is required because Discord fetches autocomplete options dynamically on every keystroke — mutating the instance state the callbacks already read from is sufficient. That sidesteps the per-app command-bucket rate limit (~5 writes / 20 s) that made the previous bulk-sync-on-reload approach unusable (#16713 context). Tests: tests/gateway/test_reload_skills_discord_resync.py — five cases covering (1) refresh replaces entries, (2) entries stay sorted after refresh, (3) collector exception leaves cached state intact, (4) `_refresh_skill_catalog_state` populates the instance attrs, (5) orchestrator calls `refresh_skill_group()` on sync + async adapters and skips adapters that don't expose it.

#18759) Discord's per-command name limit is 32 chars. When two skill slugs share the same first 32 chars (or a skill slug clamps onto a reserved gateway command name), only the first seen wins — the second is dropped from the /skill autocomplete. The old behavior incremented a ``hidden`` counter silently, so skill authors had no way to discover the drop short of noticing their skill was missing from the picker. Not an actively-biting bug today (no collisions on the default catalog as of 2026-05), but a landmine the moment someone ships a skill with a long name. The earlier series in #18745 / #18753 / #18754 dropped the other silent data-loss paths in the Discord /skill collector; this one lights up the last remaining one. Fix: promote ``_names_used`` from a set to a dict keyed by the clamped name, mapping to the source cmd_key (or a ``"<reserved>"`` sentinel for names inherited via ``reserved_names``). On collision, log a WARNING naming both sides — the winner, the loser, the clamped name, and what to rename. Two phrasings: * skill-vs-skill — "both clamp to X on Discord's 32-char command-name limit; only the winner appears in /skill. Rename one skill's frontmatter ``name:`` to differ in its first 32 chars." * skill-vs-reserved — "collides with a reserved gateway command name; the skill will not appear in /skill. Rename the skill's frontmatter ``name:``." Tests: three cases in ``tests/hermes_cli/test_discord_skill_clamp_warning.py`` — skill-vs-skill collision (warning names both cmd_keys + clamped prefix), skill-vs-reserved collision (warning uses the distinct phrasing), and a no-collision negative (zero warnings emitted).

…e live (NousResearch#18754) `_register_skill_group` captured the skill catalog in closure variables (`entries` and `skill_lookup`) so the single `tree.add_command` call at startup owned the only live copy. The closure is never re-entered after startup, so `/reload-skills` — which rescans the on-disk skills dir and refreshes the in-process `_skill_commands` registry — had no way to propagate results into the `/skill` autocomplete on Discord. New skills stayed invisible in the dropdown, and deleted skills returned "Unknown skill" when the stale autocomplete entry was clicked. The fix is purely a dataflow change: promote `entries` and `skill_lookup` to instance attributes (`_skill_entries`, `_skill_lookup`), split the collector-driven rebuild into a helper (`_refresh_skill_catalog_state`), and add a public `refresh_skill_group()` method that re-runs the helper and is safe to call at any point after the initial registration. The gateway's `_handle_reload_skills_command` then iterates `self.adapters` and calls `refresh_skill_group()` on any adapter that exposes it (currently only Discord). Both sync and async implementations are supported; adapters that don't override the method (Telegram's BotCommand menu, Slack subcommand map, etc.) are silently skipped — the in-process `reload_skills()` call covers them. No `tree.sync()` is required because Discord fetches autocomplete options dynamically on every keystroke — mutating the instance state the callbacks already read from is sufficient. That sidesteps the per-app command-bucket rate limit (~5 writes / 20 s) that made the previous bulk-sync-on-reload approach unusable (NousResearch#16713 context). Tests: tests/gateway/test_reload_skills_discord_resync.py — five cases covering (1) refresh replaces entries, (2) entries stay sorted after refresh, (3) collector exception leaves cached state intact, (4) `_refresh_skill_catalog_state` populates the instance attrs, (5) orchestrator calls `refresh_skill_group()` on sync + async adapters and skips adapters that don't expose it.

NousResearch#18759) Discord's per-command name limit is 32 chars. When two skill slugs share the same first 32 chars (or a skill slug clamps onto a reserved gateway command name), only the first seen wins — the second is dropped from the /skill autocomplete. The old behavior incremented a ``hidden`` counter silently, so skill authors had no way to discover the drop short of noticing their skill was missing from the picker. Not an actively-biting bug today (no collisions on the default catalog as of 2026-05), but a landmine the moment someone ships a skill with a long name. The earlier series in NousResearch#18745 / NousResearch#18753 / NousResearch#18754 dropped the other silent data-loss paths in the Discord /skill collector; this one lights up the last remaining one. Fix: promote ``_names_used`` from a set to a dict keyed by the clamped name, mapping to the source cmd_key (or a ``"<reserved>"`` sentinel for names inherited via ``reserved_names``). On collision, log a WARNING naming both sides — the winner, the loser, the clamped name, and what to rename. Two phrasings: * skill-vs-skill — "both clamp to X on Discord's 32-char command-name limit; only the winner appears in /skill. Rename one skill's frontmatter ``name:`` to differ in its first 32 chars." * skill-vs-reserved — "collides with a reserved gateway command name; the skill will not appear in /skill. Rename the skill's frontmatter ``name:``." Tests: three cases in ``tests/hermes_cli/test_discord_skill_clamp_warning.py`` — skill-vs-skill collision (warning names both cmd_keys + clamped prefix), skill-vs-reserved collision (warning uses the distinct phrasing), and a no-collision negative (zero warnings emitted).