Fix/command permission management #2304
Open
Conversation
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
- 修复SharedPreferences访问错误 - 修复Response方法调用错误 - 修复PermissionType导入路径错误 - 修复前端响应状态检查逻辑(success -> status) - 添加CommandPermissionRoute路由注册 功能验证: - ✅ 48个指令正常显示 - ✅ 权限修改功能正常 - ✅ 搜索过滤功能正常 - ✅ 前后端完整测试通过 文件修改: - astrbot/dashboard/routes/command_permission.py (新增) - dashboard/src/views/CommandPermissionPage.vue (新增) - astrbot/dashboard/routes/__init__.py (路由注册) - astrbot/dashboard/server.py (路由注册)
- 添加前端路由配置 /command-permission - 添加侧边栏菜单项 '指令权限管理' - 添加中英文国际化支持 - 将 common.js 转换为 common.ts 解决编译警告 - 修复 TypeScript 类型错误
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @fangguen - 我已经审阅了你的更改 - 这里有一些反馈：
- 在后端，你在 CommandFilter 和 CommandGroupFilter 循环中重复了提取 current_permission 和 has_default_admin 的逻辑——考虑将其提取到一个辅助函数中以减少重复并简化维护。
- 在每次 GET 请求时扫描 star_handlers_registry 和读取 SharedPreferences 可能会影响性能，如果命令不经常更改——考虑缓存命令元数据或权限映射以加快重复调用。
- 在前端，你在计算属性 filteredCommands 和通过 v-data-table 搜索属性同时进行过滤，这可能会产生意外的重叠；将过滤/搜索合并到一个一致的管道中将简化逻辑。
AI 代理提示
请处理此代码审查中的评论：
## 总体评论
- 在后端，你在 CommandFilter 和 CommandGroupFilter 循环中重复了提取 current_permission 和 has_default_admin 的逻辑——考虑将其提取到一个辅助函数中以减少重复并简化维护。
- 在每次 GET 请求时扫描 star_handlers_registry 和读取 SharedPreferences 可能会影响性能，如果命令不经常更改——考虑缓存命令元数据或权限映射以加快重复调用。
- 在前端，你在计算属性 filteredCommands 和通过 v-data-table 搜索属性同时进行过滤，这可能会产生意外的重叠；将过滤/搜索合并到一个一致的管道中将简化逻辑。
## 单独评论
### 评论 1
<location> `astrbot/dashboard/routes/command_permission.py:63` </location>
<code_context>
+ commands = []
+
+ # 遍历所有注册的处理器
+ for handler in star_handlers_registry:\n+ assert isinstance(handler, StarHandlerMetadata)\n+ plugin = star_map.get(handler.handler_module_path)\n+
</code_context>
<issue_to_address>
在生产环境中使用 assert 进行类型检查可能会导致问题。
当 Python 以优化模式运行时，assert 会被移除，因此类型检查可能会被跳过。请使用显式的 isinstance 检查并通过 if 语句处理失败情况。
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
# 遍历所有注册的处理器
+ for handler in star_handlers_registry:\n+ assert isinstance(handler, StarHandlerMetadata)\n+ plugin = star_map.get(handler.handler_module_path)\n=======
# 遍历所有注册的处理器
+ for handler in star_handlers_registry:\n+ if not isinstance(handler, StarHandlerMetadata):\n+ logger.warning(f"Handler {handler} is not an instance of StarHandlerMetadata, skipping.")\n+ continue\n+ plugin = star_map.get(handler.handler_module_path)\n>>>>>>> REPLACE\n
</suggested_fix>
### 评论 2
<location> `astrbot/dashboard/routes/command_permission.py:147` </location>
<code_context>
+ handler_name = data.get("handler_name")\n+ permission = data.get("permission")\n+
+ if not all([plugin_name, handler_name, permission]):\n+ return Response().error("参数不完整").__dict__\n+
</code_context>
<issue_to_address>
缺少参数的错误消息可以更具体。
请指定缺少哪些参数以提高错误清晰度和客户端处理能力。
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
plugin_name = data.get("plugin_name")\n handler_name = data.get("handler_name")\n permission = data.get("permission")\n
if not all([plugin_name, handler_name, permission]):\n return Response().error("参数不完整").__dict__\n
=======
plugin_name = data.get("plugin_name")\n handler_name = data.get("handler_name")\n permission = data.get("permission")\n
missing_params = []\n if not plugin_name:\n missing_params.append("plugin_name")\n if not handler_name:\n missing_params.append("handler_name")\n if not permission:\n missing_params.append("permission")\n if missing_params:\n return Response().error(f"参数不完整，缺少: {', '.join(missing_params)}").__dict__\n
>>>>>>> REPLACE\n
</suggested_fix>
### 评论 3
<location> `astrbot/dashboard/routes/command_permission.py:150` </location>
<code_context>
+ if not all([plugin_name, handler_name, permission]):\n+ return Response().error("参数不完整").__dict__\n+
+ if permission not in ["admin", "member"]:\n+ return Response().error("权限类型错误，只能是 admin 或 member").__dict__\n+
</code_context>
<issue_to_address>
权限类型检查区分大小写。
这将拒绝大小写不完全匹配的有效权限。考虑规范化输入或使用不区分大小写的检查。
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
permission = data.get("permission")\n
if not all([plugin_name, handler_name, permission]):\n return Response().error("参数不完整").__dict__\n
if permission not in ["admin", "member"]:\n return Response().error("权限类型错误，只能是 admin 或 member").__dict__\n
=======
permission = data.get("permission")\n
if not all([plugin_name, handler_name, permission]):\n return Response().error("参数不完整").__dict__\n
normalized_permission = permission.lower()\n if normalized_permission not in ["admin", "member"]:\n return Response().error("权限类型错误，只能是 admin 或 member").__dict__\n
>>>>>>> REPLACE\n
</suggested_fix>
### 评论 4
<location> `astrbot/dashboard/routes/command_permission.py:180` </location>
<code_context>
+
+ # 动态更新权限过滤器
+ found_permission_filter = False\n+ for filter_ in found_handler.event_filters:\n+ if isinstance(filter_, PermissionTypeFilter):\n+ from astrbot.core.star.filter.permission import PermissionType\n+ if permission == "admin":\n+ filter_.permission_type = PermissionType.ADMIN\n+ else:\n+ filter_.permission_type = PermissionType.MEMBER\n+ found_permission_filter = True\n+ break\n+
+ if not found_permission_filter:\n</code_context>
<issue_to_address>
只更新了第一个 PermissionTypeFilter。
目前，event_filters 中只有第一个 PermissionTypeFilter 被更新。请确认这是否是预期行为，或者在必要时更新所有匹配的过滤器。
</issue_to_address>帮助我更有用！请点击每个评论上的 👍 或 👎，我将使用反馈来改进您的评论。
Original comment in English
Hey @fangguen - I've reviewed your changes - here's some feedback:
- In the backend, you’re duplicating the logic for extracting current_permission and has_default_admin in both CommandFilter and CommandGroupFilter loops—consider extracting that into a helper to reduce repetition and make maintenance easier.
- Scanning star_handlers_registry and reading SharedPreferences on every GET request could impact performance if commands don’t change often—consider caching the command metadata or permission map to speed up repeated calls.
- On the frontend you’re running filtering in both your computed filteredCommands and via the v-data-table search prop, which may produce unexpected overlaps; consolidating filtering/search into one consistent pipeline will simplify the logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the backend, you’re duplicating the logic for extracting current_permission and has_default_admin in both CommandFilter and CommandGroupFilter loops—consider extracting that into a helper to reduce repetition and make maintenance easier.
- Scanning star_handlers_registry and reading SharedPreferences on every GET request could impact performance if commands don’t change often—consider caching the command metadata or permission map to speed up repeated calls.
- On the frontend you’re running filtering in both your computed filteredCommands and via the v-data-table search prop, which may produce unexpected overlaps; consolidating filtering/search into one consistent pipeline will simplify the logic.
## Individual Comments
### Comment 1
<location> `astrbot/dashboard/routes/command_permission.py:63` </location>
<code_context>
+ commands = []
+
+ # 遍历所有注册的处理器
+ for handler in star_handlers_registry:
+ assert isinstance(handler, StarHandlerMetadata)
+ plugin = star_map.get(handler.handler_module_path)
+
</code_context>
<issue_to_address>
Use of assert for type checking may cause issues in production.
Asserts are removed when Python runs with optimizations, so type checks may be skipped. Use an explicit isinstance check and handle failures with an if-statement instead.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
# 遍历所有注册的处理器
+ for handler in star_handlers_registry:
+ assert isinstance(handler, StarHandlerMetadata)
+ plugin = star_map.get(handler.handler_module_path)
=======
# 遍历所有注册的处理器
+ for handler in star_handlers_registry:
+ if not isinstance(handler, StarHandlerMetadata):
+ logger.warning(f"Handler {handler} is not an instance of StarHandlerMetadata, skipping.")
+ continue
+ plugin = star_map.get(handler.handler_module_path)
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `astrbot/dashboard/routes/command_permission.py:147` </location>
<code_context>
+ handler_name = data.get("handler_name")
+ permission = data.get("permission")
+
+ if not all([plugin_name, handler_name, permission]):
+ return Response().error("参数不完整").__dict__
+
</code_context>
<issue_to_address>
Error message for missing parameters could be more specific.
Specify which parameter(s) are missing to improve error clarity and client-side handling.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
plugin_name = data.get("plugin_name")
handler_name = data.get("handler_name")
permission = data.get("permission")
if not all([plugin_name, handler_name, permission]):
return Response().error("参数不完整").__dict__
=======
plugin_name = data.get("plugin_name")
handler_name = data.get("handler_name")
permission = data.get("permission")
missing_params = []
if not plugin_name:
missing_params.append("plugin_name")
if not handler_name:
missing_params.append("handler_name")
if not permission:
missing_params.append("permission")
if missing_params:
return Response().error(f"参数不完整，缺少: {', '.join(missing_params)}").__dict__
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `astrbot/dashboard/routes/command_permission.py:150` </location>
<code_context>
+ if not all([plugin_name, handler_name, permission]):
+ return Response().error("参数不完整").__dict__
+
+ if permission not in ["admin", "member"]:
+ return Response().error("权限类型错误，只能是 admin 或 member").__dict__
+
</code_context>
<issue_to_address>
Permission type check is case-sensitive.
This will reject valid permissions if the case doesn't match exactly. Consider normalizing input or using a case-insensitive check.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
permission = data.get("permission")
if not all([plugin_name, handler_name, permission]):
return Response().error("参数不完整").__dict__
if permission not in ["admin", "member"]:
return Response().error("权限类型错误，只能是 admin 或 member").__dict__
=======
permission = data.get("permission")
if not all([plugin_name, handler_name, permission]):
return Response().error("参数不完整").__dict__
normalized_permission = permission.lower()
if normalized_permission not in ["admin", "member"]:
return Response().error("权限类型错误，只能是 admin 或 member").__dict__
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `astrbot/dashboard/routes/command_permission.py:180` </location>
<code_context>
+
+ # 动态更新权限过滤器
+ found_permission_filter = False
+ for filter_ in found_handler.event_filters:
+ if isinstance(filter_, PermissionTypeFilter):
+ from astrbot.core.star.filter.permission import PermissionType
+ if permission == "admin":
+ filter_.permission_type = PermissionType.ADMIN
+ else:
+ filter_.permission_type = PermissionType.MEMBER
+ found_permission_filter = True
+ break
+
+ if not found_permission_filter:
</code_context>
<issue_to_address>
Only the first PermissionTypeFilter is updated.
Currently, only the first PermissionTypeFilter in event_filters is updated. Please confirm if this is intended, or update all matching filters if necessary.
</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
62
to
65
| # 遍历所有注册的处理器 | ||
| for handler in star_handlers_registry: | ||
| assert isinstance(handler, StarHandlerMetadata) | ||
| plugin = star_map.get(handler.handler_module_path) |
There was a problem hiding this comment.
suggestion (bug_risk): 在生产环境中使用 assert 进行类型检查可能会导致问题。
当 Python 以优化模式运行时，assert 会被移除，因此类型检查可能会被跳过。请使用显式的 isinstance 检查并通过 if 语句处理失败情况。
Suggested change
| # 遍历所有注册的处理器 | |
| for handler in star_handlers_registry: | |
| assert isinstance(handler, StarHandlerMetadata) | |
| plugin = star_map.get(handler.handler_module_path) | |
| # 遍历所有注册的处理器 | |
| + for handler in star_handlers_registry: | |
| + if not isinstance(handler, StarHandlerMetadata): | |
| + logger.warning(f"Handler {handler} is not an instance of StarHandlerMetadata, skipping.") | |
| + continue | |
| + plugin = star_map.get(handler.handler_module_path) |
Original comment in English
suggestion (bug_risk): Use of assert for type checking may cause issues in production.
Asserts are removed when Python runs with optimizations, so type checks may be skipped. Use an explicit isinstance check and handle failures with an if-statement instead.
Suggested change
| # 遍历所有注册的处理器 | |
| for handler in star_handlers_registry: | |
| assert isinstance(handler, StarHandlerMetadata) | |
| plugin = star_map.get(handler.handler_module_path) | |
| # 遍历所有注册的处理器 | |
| + for handler in star_handlers_registry: | |
| + if not isinstance(handler, StarHandlerMetadata): | |
| + logger.warning(f"Handler {handler} is not an instance of StarHandlerMetadata, skipping.") | |
| + continue | |
| + plugin = star_map.get(handler.handler_module_path) |
Comment on lines
143
to
149
| plugin_name = data.get("plugin_name") | ||
| handler_name = data.get("handler_name") | ||
| permission = data.get("permission") | ||
|
|
||
| if not all([plugin_name, handler_name, permission]): | ||
| return Response().error("参数不完整").__dict__ | ||
|
|
There was a problem hiding this comment.
suggestion: 缺少参数的错误消息可以更具体。
请指定缺少哪些参数以提高错误清晰度和客户端处理能力。
Suggested change
| plugin_name = data.get("plugin_name") | |
| handler_name = data.get("handler_name") | |
| permission = data.get("permission") | |
| if not all([plugin_name, handler_name, permission]): | |
| return Response().error("参数不完整").__dict__ | |
| plugin_name = data.get("plugin_name") | |
| handler_name = data.get("handler_name") | |
| permission = data.get("permission") | |
| missing_params = [] | |
| if not plugin_name: | |
| missing_params.append("plugin_name") | |
| if not handler_name: | |
| missing_params.append("handler_name") | |
| if not permission: | |
| missing_params.append("permission") | |
| if missing_params: | |
| return Response().error(f"参数不完整，缺少: {', '.join(missing_params)}").__dict__ | |
Original comment in English
suggestion: Error message for missing parameters could be more specific.
Specify which parameter(s) are missing to improve error clarity and client-side handling.
Suggested change
| plugin_name = data.get("plugin_name") | |
| handler_name = data.get("handler_name") | |
| permission = data.get("permission") | |
| if not all([plugin_name, handler_name, permission]): | |
| return Response().error("参数不完整").__dict__ | |
| plugin_name = data.get("plugin_name") | |
| handler_name = data.get("handler_name") | |
| permission = data.get("permission") | |
| missing_params = [] | |
| if not plugin_name: | |
| missing_params.append("plugin_name") | |
| if not handler_name: | |
| missing_params.append("handler_name") | |
| if not permission: | |
| missing_params.append("permission") | |
| if missing_params: | |
| return Response().error(f"参数不完整，缺少: {', '.join(missing_params)}").__dict__ | |
Comment on lines
145
to
152
| permission = data.get("permission") | ||
|
|
||
| if not all([plugin_name, handler_name, permission]): | ||
| return Response().error("参数不完整").__dict__ | ||
|
|
||
| if permission not in ["admin", "member"]: | ||
| return Response().error("权限类型错误，只能是 admin 或 member").__dict__ | ||
|
|
There was a problem hiding this comment.
suggestion (bug_risk): 权限类型检查区分大小写。
这将拒绝大小写不完全匹配的有效权限。考虑规范化输入或使用不区分大小写的检查。
Suggested change
| permission = data.get("permission") | |
| if not all([plugin_name, handler_name, permission]): | |
| return Response().error("参数不完整").__dict__ | |
| if permission not in ["admin", "member"]: | |
| return Response().error("权限类型错误，只能是 admin 或 member").__dict__ | |
| permission = data.get("permission") | |
| if not all([plugin_name, handler_name, permission]): | |
| return Response().error("参数不完整").__dict__ | |
| normalized_permission = permission.lower() | |
| if normalized_permission not in ["admin", "member"]: | |
| return Response().error("权限类型错误，只能是 admin 或 member").__dict__ | |
Original comment in English
suggestion (bug_risk): Permission type check is case-sensitive.
This will reject valid permissions if the case doesn't match exactly. Consider normalizing input or using a case-insensitive check.
Suggested change
| permission = data.get("permission") | |
| if not all([plugin_name, handler_name, permission]): | |
| return Response().error("参数不完整").__dict__ | |
| if permission not in ["admin", "member"]: | |
| return Response().error("权限类型错误，只能是 admin 或 member").__dict__ | |
| permission = data.get("permission") | |
| if not all([plugin_name, handler_name, permission]): | |
| return Response().error("参数不完整").__dict__ | |
| normalized_permission = permission.lower() | |
| if normalized_permission not in ["admin", "member"]: | |
| return Response().error("权限类型错误，只能是 admin 或 member").__dict__ | |
| has_default_admin = True | ||
| if current_permission == "member": | ||
| current_permission = "admin" | ||
| break |
There was a problem hiding this comment.
question: 只更新了第一个 PermissionTypeFilter。
目前，event_filters 中只有第一个 PermissionTypeFilter 被更新。请确认这是否是预期行为，或者在必要时更新所有匹配的过滤器。
Original comment in English
question: Only the first PermissionTypeFilter is updated.
Currently, only the first PermissionTypeFilter in event_filters is updated. Please confirm if this is intended, or update all matching filters if necessary.
| logger.error(f"/api/command_permission/get: {traceback.format_exc()}") | ||
| return Response().error("获取指令权限配置失败").__dict__ | ||
|
|
||
| async def get_all_commands(self) -> Response: |
There was a problem hiding this comment.
issue (code-quality): 在 CommandPermissionRoute.get_all_commands 中发现低代码质量 - 14% (low-code-quality)
解释
此函数的质量得分低于 25% 的质量阈值。此得分是方法长度、认知复杂度和工作内存的组合。
如何解决这个问题？
重构此函数以使其更短、更具可读性可能是有益的。
- 通过将部分功能提取到自己的函数中来减少函数长度。这是您可以做的最重要的事情 - 理想情况下，函数应少于 10 行。
- 减少嵌套，也许可以通过引入防御性子句来提前返回。
- 确保变量的作用域紧密，以便使用相关概念的代码在函数中紧密地放在一起，而不是分散开来。
Original comment in English
issue (code-quality): Low code quality found in CommandPermissionRoute.get_all_commands - 14% (low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
| return Response().error("权限类型错误，只能是 admin 或 member").__dict__ | ||
|
|
||
| # 查找对应的处理器 | ||
| found_handler = None |
- 修复生产环境 assert 问题，改为显式 isinstance 检查 - 改进参数错误消息，提供具体缺失参数信息 - 修复权限类型检查的大小写敏感问题 - 提取重复逻辑到 _get_permission_info 辅助函数中 - 添加60秒缓存机制优化性能，避免频繁扫描 - 优化前端过滤逻辑，避免重叠过滤 - 保持与项目其他部分的架构一致性 解决了代码审查中提出的所有问题，提升了代码质量和性能。
|
@sourcery-ai 提出的所有代码审查意见已经在最新提交中修复完成： ✅ 已修复的问题
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
新增指令管理面板
以下为ai的总结：
Motivation
目前 AstrBot 的指令权限需要在配置文件中手动修改，操作不便且对不熟悉代码的用户不友好。为了方便管理员在图形化界面中直观地管理各个指令的访问权限，决定新增一个指令权限管理面板。该面板允许管理员查看所有指令，并动态修改其权限（例如，设置为“仅管理员可用”或“所有用户可用”）。
Modifications
后端 (Backend):
astrabot/dashboard/routes/command_permission.py文件，创建了全新的指令权限管理 API 路由。GET /api/command_permission/get_commands: 动态扫描并返回所有已注册的指令（包括指令组），及其所属插件、描述和当前的权限设置。POST /api/command_permission/set: 接收前端请求，用于修改指定指令的权限。配置会保存到SharedPreferences中，并实时动态更新应用中的权限过滤器，无需重启即可生效。GET /api/command_permission/get: 获取当前所有已修改的权限配置。server.py中注册并挂载了新的CommandPermissionRoute。前端 (Frontend):
dashboard/src/views/CommandPermissionPage.vue作为指令权限管理的主界面。v-data-table，用于展示所有指令的列表。MainRoutes.ts）和侧边栏导航链接（sidebarItem.ts），并配置了中英文国际化（i18n）文本。代码质量改进 (Refactoring):
dashboard/src/stores/common.js文件重构为 TypeScript (common.ts)，增加了类型定义（Interface），增强了代码的健壮性和可维护性。Check
requirements.txt和pyproject.toml文件相应位置。Sourcery 总结
添加命令权限管理功能，包含新的后端 API 和一个基于 Vue 的仪表盘面板，并将通用 store 重构为 TypeScript。
新功能：
优化：
Original summary in English
Summary by Sourcery
Add a command permission management feature with new backend APIs and a Vue-based dashboard panel, and refactor common store to TypeScript.
New Features:
Enhancements: