Conversation
dosubot
bot
added
the
size:XL
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
dosubot
bot
added
area:platform
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些整体性的反馈：
- 在
dashboard/routes/plugin.py中你捕获了PluginVersionIncompatibleError，但从未导入它，所以这个路由在运行时会抛出NameError；请从astrbot.core.star.star_manager中添加相应的导入。 ExtensionPage.vue和MarketPluginCard.vue都定义了类似的normalizePlatformList/ 平台展示辅助函数；建议把这些逻辑提取到一个共享的工具模块中，以避免重复，并把平台展示相关逻辑集中到一个地方。/plugin/check-compat路由调用了私有方法plugin_manager._validate_astrbot_version_specifier；更干净的做法是把它作为PluginManager上的公共辅助方法暴露出来（例如validate_astrbot_version_specifier），而不是在外部依赖一个以下划线开头的私有方法。
面向 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- In `dashboard/routes/plugin.py` you catch `PluginVersionIncompatibleError` but never import it, so this route will raise a `NameError` at runtime; add the appropriate import from `astrbot.core.star.star_manager`.
- Both `ExtensionPage.vue` and `MarketPluginCard.vue` define similar `normalizePlatformList` / platform-display helpers; consider extracting these into a shared utility to avoid duplication and keep platform presentation logic in one place.
- The `/plugin/check-compat` route calls the private method `plugin_manager._validate_astrbot_version_specifier`; it would be cleaner to expose this as a public helper on `PluginManager` (e.g. `validate_astrbot_version_specifier`) instead of relying on a leading-underscore method from outside.
## Individual Comments
### Comment 1
<location> `dashboard/src/views/ExtensionPage.vue:1151-1160` </location>
<code_context>
+ );
+};
+
+const resolveSelectedInstallPlugin = () => {
+ if (
+ selectedMarketInstallPlugin.value &&
+ selectedMarketInstallPlugin.value.repo === extension_url.value
+ ) {
+ return selectedMarketInstallPlugin.value;
+ }
+ return pluginMarketData.value.find((plugin) => plugin.repo === extension_url.value) || null;
+};
+
</code_context>
<issue_to_address>
**suggestion:** 当通过 repo URL 解析选中的安装插件时，建议使用不区分大小写的比较。
当前的解析逻辑对 `plugin.repo === extension_url.value` 使用了严格相等比较，而在其他地方（例如 `checkAlreadyInstalled`）中，repo URL 会被归一化为小写。如果粘贴的 URL 大小写与市场数据中的不一致，`selectedInstallPlugin` 可能会是 `null`，从而导致兼容性检查和元数据无法展示。建议对两边都做归一化（例如 `plugin.repo?.toLowerCase()` 对比 `extension_url.value.toLowerCase()`），并处理 repo 可能为 `null` 的情况。
```suggestion
+const resolveSelectedInstallPlugin = () => {
+ const normalizedExtensionUrl = extension_url.value?.toLowerCase();
+
+ if (!normalizedExtensionUrl) {
+ return null;
+ }
+
+ if (
+ selectedMarketInstallPlugin.value &&
+ selectedMarketInstallPlugin.value.repo?.toLowerCase() === normalizedExtensionUrl
+ ) {
+ return selectedMarketInstallPlugin.value;
+ }
+
+ return (
+ pluginMarketData.value.find(
+ (plugin) => plugin.repo?.toLowerCase() === normalizedExtensionUrl,
+ ) || null
+ );
+};
+
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/star/star_manager.py:297-306` </location>
<code_context>
return metadata
+ @staticmethod
+ def _validate_astrbot_version_specifier(
+ version_spec: str | None,
+ ) -> tuple[bool, str | None]:
+ if not version_spec:
+ return True, None
+
+ normalized_spec = version_spec.strip()
+ if not normalized_spec:
+ return True, None
+
+ if "v" in normalized_spec.lower():
+ return (
+ False,
</code_context>
<issue_to_address>
**issue (bug_risk):** 这种对 `"v"` 的全量检查，会错误地拒绝合法的 PEP 440 版本说明符。
这个条件会拒绝任何包含 `v` 的说明符，包括像 `1.0.0.dev0` 或本地版本这样的合法版本，从而产生误报，即便 `packaging` 实际上可以正确解析它们。如果你只想阻止**以** `v` 开头的版本（例如 `v4.16`），可以考虑改为检查 `normalized_spec.lstrip().lower().startswith('v')`，或者在解析前先剥掉单个前导 `v`。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会基于这些反馈改进以后的审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In
dashboard/routes/plugin.pyyou catchPluginVersionIncompatibleErrorbut never import it, so this route will raise aNameErrorat runtime; add the appropriate import fromastrbot.core.star.star_manager. - Both
ExtensionPage.vueandMarketPluginCard.vuedefine similarnormalizePlatformList/ platform-display helpers; consider extracting these into a shared utility to avoid duplication and keep platform presentation logic in one place. - The
/plugin/check-compatroute calls the private methodplugin_manager._validate_astrbot_version_specifier; it would be cleaner to expose this as a public helper onPluginManager(e.g.validate_astrbot_version_specifier) instead of relying on a leading-underscore method from outside.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dashboard/routes/plugin.py` you catch `PluginVersionIncompatibleError` but never import it, so this route will raise a `NameError` at runtime; add the appropriate import from `astrbot.core.star.star_manager`.
- Both `ExtensionPage.vue` and `MarketPluginCard.vue` define similar `normalizePlatformList` / platform-display helpers; consider extracting these into a shared utility to avoid duplication and keep platform presentation logic in one place.
- The `/plugin/check-compat` route calls the private method `plugin_manager._validate_astrbot_version_specifier`; it would be cleaner to expose this as a public helper on `PluginManager` (e.g. `validate_astrbot_version_specifier`) instead of relying on a leading-underscore method from outside.
## Individual Comments
### Comment 1
<location> `dashboard/src/views/ExtensionPage.vue:1151-1160` </location>
<code_context>
+ );
+};
+
+const resolveSelectedInstallPlugin = () => {
+ if (
+ selectedMarketInstallPlugin.value &&
+ selectedMarketInstallPlugin.value.repo === extension_url.value
+ ) {
+ return selectedMarketInstallPlugin.value;
+ }
+ return pluginMarketData.value.find((plugin) => plugin.repo === extension_url.value) || null;
+};
+
</code_context>
<issue_to_address>
**suggestion:** Use case-insensitive comparison when resolving the selected install plugin by repo URL.
This resolver does a strict equality check on `plugin.repo === extension_url.value`, while elsewhere (e.g. `checkAlreadyInstalled`) repo URLs are normalized to lowercase. If the pasted URL’s casing differs from the market data, `selectedInstallPlugin` can be `null`, so compatibility checks and metadata won’t appear. Consider normalizing both sides (e.g. `plugin.repo?.toLowerCase()` vs `extension_url.value.toLowerCase()`) and handling a possible `null` repo.
```suggestion
+const resolveSelectedInstallPlugin = () => {
+ const normalizedExtensionUrl = extension_url.value?.toLowerCase();
+
+ if (!normalizedExtensionUrl) {
+ return null;
+ }
+
+ if (
+ selectedMarketInstallPlugin.value &&
+ selectedMarketInstallPlugin.value.repo?.toLowerCase() === normalizedExtensionUrl
+ ) {
+ return selectedMarketInstallPlugin.value;
+ }
+
+ return (
+ pluginMarketData.value.find(
+ (plugin) => plugin.repo?.toLowerCase() === normalizedExtensionUrl,
+ ) || null
+ );
+};
+
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/star/star_manager.py:297-306` </location>
<code_context>
return metadata
+ @staticmethod
+ def _validate_astrbot_version_specifier(
+ version_spec: str | None,
+ ) -> tuple[bool, str | None]:
+ if not version_spec:
+ return True, None
+
+ normalized_spec = version_spec.strip()
+ if not normalized_spec:
+ return True, None
+
+ if "v" in normalized_spec.lower():
+ return (
+ False,
</code_context>
<issue_to_address>
**issue (bug_risk):** The blanket `"v"` check can incorrectly reject valid PEP 440 version specifiers.
This condition rejects any specifier containing `v`, including valid ones like `1.0.0.dev0` or local versions, leading to false negatives even though `packaging` can parse them. If you only want to block a *leading* `v` (e.g. `v4.16`), consider checking `normalized_spec.lstrip().lower().startswith('v')` or stripping a single leading `v` before parsing instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1151
to
+1160
| const resolveSelectedInstallPlugin = () => { | ||
| if ( | ||
| selectedMarketInstallPlugin.value && | ||
| selectedMarketInstallPlugin.value.repo === extension_url.value | ||
| ) { | ||
| return selectedMarketInstallPlugin.value; | ||
| } | ||
| return pluginMarketData.value.find((plugin) => plugin.repo === extension_url.value) || null; | ||
| }; | ||
Contributor
There was a problem hiding this comment.
suggestion: 当通过 repo URL 解析选中的安装插件时，建议使用不区分大小写的比较。
当前的解析逻辑对 plugin.repo === extension_url.value 使用了严格相等比较，而在其他地方（例如 checkAlreadyInstalled）中，repo URL 会被归一化为小写。如果粘贴的 URL 大小写与市场数据中的不一致，selectedInstallPlugin 可能会是 null，从而导致兼容性检查和元数据无法展示。建议对两边都做归一化（例如 plugin.repo?.toLowerCase() 对比 extension_url.value.toLowerCase()），并处理 repo 可能为 null 的情况。
Suggested change
| const resolveSelectedInstallPlugin = () => { | |
| if ( | |
| selectedMarketInstallPlugin.value && | |
| selectedMarketInstallPlugin.value.repo === extension_url.value | |
| ) { | |
| return selectedMarketInstallPlugin.value; | |
| } | |
| return pluginMarketData.value.find((plugin) => plugin.repo === extension_url.value) || null; | |
| }; | |
| +const resolveSelectedInstallPlugin = () => { | |
| + const normalizedExtensionUrl = extension_url.value?.toLowerCase(); | |
| + | |
| + if (!normalizedExtensionUrl) { | |
| + return null; | |
| + } | |
| + | |
| + if ( | |
| + selectedMarketInstallPlugin.value && | |
| + selectedMarketInstallPlugin.value.repo?.toLowerCase() === normalizedExtensionUrl | |
| + ) { | |
| + return selectedMarketInstallPlugin.value; | |
| + } | |
| + | |
| + return ( | |
| + pluginMarketData.value.find( | |
| + (plugin) => plugin.repo?.toLowerCase() === normalizedExtensionUrl, | |
| + ) || null | |
| + ); | |
| +}; | |
| + |
Original comment in English
suggestion: Use case-insensitive comparison when resolving the selected install plugin by repo URL.
This resolver does a strict equality check on plugin.repo === extension_url.value, while elsewhere (e.g. checkAlreadyInstalled) repo URLs are normalized to lowercase. If the pasted URL’s casing differs from the market data, selectedInstallPlugin can be null, so compatibility checks and metadata won’t appear. Consider normalizing both sides (e.g. plugin.repo?.toLowerCase() vs extension_url.value.toLowerCase()) and handling a possible null repo.
Suggested change
| const resolveSelectedInstallPlugin = () => { | |
| if ( | |
| selectedMarketInstallPlugin.value && | |
| selectedMarketInstallPlugin.value.repo === extension_url.value | |
| ) { | |
| return selectedMarketInstallPlugin.value; | |
| } | |
| return pluginMarketData.value.find((plugin) => plugin.repo === extension_url.value) || null; | |
| }; | |
| +const resolveSelectedInstallPlugin = () => { | |
| + const normalizedExtensionUrl = extension_url.value?.toLowerCase(); | |
| + | |
| + if (!normalizedExtensionUrl) { | |
| + return null; | |
| + } | |
| + | |
| + if ( | |
| + selectedMarketInstallPlugin.value && | |
| + selectedMarketInstallPlugin.value.repo?.toLowerCase() === normalizedExtensionUrl | |
| + ) { | |
| + return selectedMarketInstallPlugin.value; | |
| + } | |
| + | |
| + return ( | |
| + pluginMarketData.value.find( | |
| + (plugin) => plugin.repo?.toLowerCase() === normalizedExtensionUrl, | |
| + ) || null | |
| + ); | |
| +}; | |
| + |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
为 AstrBot 的插件元数据添加版本要求和支持平台信息，在插件安装过程中强制进行兼容性检查，并在整个控制台界面中展示这些信息。
New Features:
astrbot_version要求和支持的平台信息。Enhancements:
support_platforms和astrbot_version元数据。wecom_ai_bot适配器的支持。Build:
packaging作为运行时依赖，用于版本说明符的解析与比较。Original summary in English
Summary by Sourcery
Add plugin metadata support for AstrBot version requirements and supported platforms, enforce compatibility checks during plugin installation, and surface this information throughout the dashboard UI.
New Features:
Enhancements:
Build: