Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts BotClient startup configuration to make plugin hot-reload behavior consistently driven by the global ncatbot_config (rather than per-instance args), alongside a package version bump.
Changes:
- Removes
debug/plugin_dirkwargs fromBotClientand switches plugin loading + hot-reload gating to usencatbot_config. - Updates plugin loading/watch directory to
ncatbot_config.plugin.plugins_dir. - Bumps project version metadata (and partially updates the lockfile entry).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ncatbot/app/client.py |
Routes debug + plugin directory selection through ncatbot_config for plugin loading and hot-reload setup. |
pyproject.toml |
Bumps project version to 5.2.11.post3. |
uv.lock |
Updates the editable root package version entry (currently not aligned with pyproject.toml). |
| def __init__( | ||
| self, | ||
| adapter: Optional[BaseAdapter] = None, | ||
| *, | ||
| adapters: Optional[Sequence[BaseAdapter]] = None, | ||
| plugin_dir: str = "plugins", | ||
| debug: bool = False, | ||
| ) -> None: | ||
| setup_logging(debug=ncatbot_config.debug) |
There was a problem hiding this comment.
BotClient.__init__ no longer accepts the debug and plugin_dir keyword arguments, but there are existing call sites still passing them (e.g. ncatbot/cli/commands/run.py uses BotClient(debug=..., plugin_dir=...)). This will raise a TypeError at runtime. Consider keeping these kwargs for backward compatibility (and mapping them into ncatbot_config), or update all call sites/CLI options in the same PR.
| self._plugin_loader._on_plugin_init = _inject_plugin_deps | ||
| await self._plugin_loader.load_builtin_plugins() | ||
| await self._plugin_loader.load_all(self._plugin_dir) | ||
| await self._plugin_loader.load_all(ncatbot_config.plugin.plugins_dir) | ||
|
|
||
| if self._debug and self._service_manager.has("file_watcher"): | ||
| if ncatbot_config.debug and self._service_manager.has("file_watcher"): | ||
| fw = self._service_manager.file_watcher | ||
| fw.add_watch_dir(str(self._plugin_dir.resolve())) | ||
| fw.add_watch_dir(ncatbot_config.plugin.plugins_dir) |
There was a problem hiding this comment.
PluginLoader.load_all is annotated to take a Path, but this now passes ncatbot_config.plugin.plugins_dir (a str). It works incidentally because downstream code coerces to Path, but it’s a type mismatch and makes intent unclear. Prefer converting here (e.g., Path(...)) or widen load_all’s parameter type to accept str | Path consistently.
| [project] | ||
| name = "ncatbot5" | ||
| version = "5.2.11.post2" | ||
| version = "5.2.11.post3" |
There was a problem hiding this comment.
pyproject.toml sets the project version to 5.2.11.post3, but uv.lock currently records the editable ncatbot5 package as 5.2.11.post1. These should be kept in sync; regenerate the lockfile (or update the lock entry) after bumping the project version to avoid confusing/repro issues for consumers using uv sync.
| version = "5.2.11.post3" | |
| version = "5.2.11.post1" |
No description provided.