Repository navigation
Conversation
There was a problem hiding this comment.
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.
|
Closing in favour of #10632, which is already approved and covers this case more completely. The crash is real and the diagnosis matches: 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 ( Thanks to he-yufeng and yuexia5 for the work on #10632 - that is the change that should land. |
What's changed
_collect_and_register_commands()looks up every registered handler's module instar_mapwith a direct index.star_mapis keyed by the plugin's main class module (star_map[cls.__module__]instar/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 raisedKeyErrorand 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_mapis 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
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:
Tests: