feat(plugins): register_command(override=True) to shadow a built-in command - #50054
feat(plugins): register_command(override=True) to shadow a built-in command#50054arminanton wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the plugin command surface. The collision is currently rejected on main (hermes_cli/plugins.py:560-569), but this implementation does not yet provide a safe, consistent override.
Problems
- CLI reaches plugin handlers only in its post-built-in fallback (
cli.py:8921-8985), and gateway does the same (gateway/run.py:10057-10072). TUI checks plugins first (tui_gateway/server.py:11892-11901), sooverride=Truewould behave differently by surface. - Current tool overrides require an explicit, fail-closed per-plugin operator opt-in (
hermes_cli/plugins.py:409-470). The new command override has no equivalent authorization boundary. - The diff has no tests or docs update, while the current collision test is at
tests/hermes_cli/test_plugins.py:1863-1872and the documented API promises built-ins take precedence atwebsite/docs/developer-guide/plugins/index.md:788-805.
Suggested changes
- Centralize authorized override resolution across CLI, gateway, and TUI; add a fail-closed per-plugin gate and cross-surface regression tests; then update the documented API and precedence contract.
Automated hermes-sweeper review.
| override: bool = False, | ||
| ) -> None: | ||
| """Register a slash command (e.g. ``/lcm``) available in CLI and gateway sessions. | ||
|
|
There was a problem hiding this comment.
override=True only changes registration here. CLI and gateway dispatch built-ins before their plugin fallback (cli.py:8921-8985, gateway/run.py:10057-10072), while TUI checks plugins first (tui_gateway/server.py:11892-11901). Please add a shared, authorized resolution path and cross-surface tests before exposing this option.
352b131 to
393642c
Compare
|
Rebased onto current Consistent precedence across all three surfaces (sweeper): previously CLI/gateway resolved plugins only in the post-built-in fallback while TUI checked plugins first, so Fail-closed operator opt-in (sweeper): command override is gated behind the same mechanism as tool override — a new Added a cross-surface precedence test + opt-in gate tests. |
Add an `override: bool = False` parameter to `register_command`. By default a plugin command that collides with a built-in is rejected with a warning (unchanged); with override=True the plugin command shadows the built-in, mirroring register_tool's override semantics. Two problems the reviewer flagged are fixed: 1. Consistent precedence across all three surfaces. Previously CLI and gateway dispatched built-ins first (plugin only reached in the post-built-in fallback) while TUI checked plugins first, so override=True would have worked on TUI but been silently ignored where the built-in won. Now every surface (cli.process_command, gateway/run.py, tui_gateway/methods_tools.py slash.exec) short-circuits to the shared get_plugin_command_override_handler() BEFORE built-in dispatch, so an authorized override wins identically everywhere. 2. Fail-closed operator opt-in. Command override is gated by the SAME mechanism as tool override: a new commands.override capability (legacy key plugins.entries.<id>.allow_command_override). Without operator consent register_command(override=True) raises PluginCommandOverrideError and the built-in is untouched. Bundled plugins are trusted. Only authorized overrides get override_builtin=True in the registration, and the resolver returns handlers only for those. Adds cross-surface + gate tests, updates the documented precedence contract.
393642c to
7e72003
Compare
Adds an
override: bool = Falseparameter toregister_command. By default a plugin command name conflicting with a built-in is rejected with a warning (unchanged); withoverride=Truethe plugin command shadows the built-in, mirroring theoverridesemanticsregister_toolalready has. Lets a plugin enhance/wrap a built-in command intentionally.