Skip to content

fix(discord): skip command handlers missing from the star map - #10791

Closed
rwzdk wants to merge 1 commit into
AstrBotDevs:masterfrom
rwzdk:fix/discord-command-sync-submodule-keyerror
Closed

rwzdk wants to merge 1 commit into
AstrBotDevs:masterfrom
rwzdk:fix/discord-command-sync-submodule-keyerror

Conversation

@rwzdk

@rwzdk rwzdk commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What's changed

_collect_and_register_commands() looks up every registered handler's module in star_map with a direct index. star_map is keyed by the plugin's main class module (star_map[cls.__module__] in star/base.py), while a handler records the module where its function is defined (handler_module_path=handler.__module__). For a plugin that declares its command handlers in a submodule (my_plugin/commands.py), that key is absent, so the lookup raised KeyError and aborted the whole collection loop — none of the plugins' slash commands were registered.

Skip such handlers instead. They remain registered as ordinary message handlers; every other plugin's commands still sync. A handler whose module is missing from star_map is also already treated as "not an active plugin module" elsewhere — StarManager._is_plugin_module_path() matches submodules by prefix — so the skip matches the rest of the codebase.

Validation

# production file reverted, tests kept
$ .venv/Scripts/python.exe -m pytest tests/test_discord_command_sync.py -q
E           KeyError: 'data.plugins.example_plugin.mixin'
astrbot\core\platform\sources\discord\discord_platform_adapter.py:475: KeyError
1 failed, 2 passed

# with the fix
$ .venv/Scripts/python.exe -m pytest tests/test_discord_command_sync.py -q
3 passed

The new test registers one handler from the plugin's main module and one from a submodule and asserts that sync_commands() still runs and only the main-module command is registered.

Prepared with an AI coding agent.

Summary by Sourcery

Make Discord command collection resilient to handlers whose modules are not represented in the plugin map.

Bug Fixes:

  • Prevent Discord command synchronization from aborting when a handler belongs to a plugin submodule absent from the plugin map, while continuing to register valid commands from other plugins.
  • Ensure commands from deactivated plugins remain excluded from Discord registration.

Tests:

  • Add coverage for skipping submodule handlers without disrupting command synchronization and for excluding deactivated plugin commands.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. A handler whose module is absent from star_map is skipped while the remaining commands are synced, which could leave Discord with an incomplete command set. Reverting alone may not restore the previously registered commands, but a later corrected sync can re-register them.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@rwzdk

rwzdk commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of #10632, which is already approved and covers this case more completely.

The crash is real and the diagnosis matches: star_map is keyed by a plugin's main module while handler_module_path is the module where the handler was defined, so the direct lookup raises KeyError and aborts the collection loop before sync_commands().

This patch only skips such handlers. That keeps the crash away but leaves the reported symptom half-fixed: a plugin that declares its commands in a submodule still ends up with no slash commands, because those handlers are now skipped instead of registered. #10632 resolves the owning plugin by package root, so a submodule handler of an active plugin is still synced, and only genuinely unowned handlers are skipped - which is the behaviour this issue asks for. he-yufeng made the same point on #10632 ("the skip end-state drops a live plugin's commands").

The two pull requests also add a file at the same path (tests/test_discord_command_sync.py), so keeping both would conflict.

Thanks to he-yufeng and yuexia5 for the work on #10632 - that is the change that should land.

@rwzdk rwzdk closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant