feat: support for managing command aliases #4170
Merged
+137
−7
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
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey，我发现了两个问题，并给出了一些高层面的反馈：
- 在后端的
rename_command处理函数中，建议在遍历和存储之前，对aliases做一次校验/规范化为字符串列表（例如：忽略非列表输入、过滤掉非字符串项并去重），以避免因 JSON 负载格式异常而产生意料之外的行为。 - 在
_set_filter_aliases中，你假设filter_ref.alias是一个set；如果其他代码把它设置成了不同的类型（例如 list），那么set(aliases) == current_aliases的相等性检查将始终为 false —— 建议在比较之前将current_aliases规范化为一个 set，以避免发生多余的更新。
给 AI Agent 的提示
请根据这次代码评审中的评论进行修改：
## 总体评论
- 在后端的 `rename_command` 处理函数中，建议在遍历和存储之前，对 `aliases` 做一次校验/规范化为字符串列表（例如：忽略非列表输入、过滤掉非字符串项并去重），以避免因 JSON 负载格式异常而产生意料之外的行为。
- 在 `_set_filter_aliases` 中，你假设 `filter_ref.alias` 是一个 `set`；如果其他代码把它设置成了不同的类型（例如 list），那么 `set(aliases) == current_aliases` 的相等性检查将始终为 false —— 建议在比较之前将 `current_aliases` 规范化为一个 set，以避免发生多余的更新。
## 单独评论
### 评论 1
<location> `astrbot/dashboard/routes/command.py:64-70` </location>
<code_context>
data = await request.get_json()
handler_full_name = data.get("handler_full_name")
new_name = data.get("new_name")
+ aliases = data.get("aliases")
if not handler_full_name or not new_name:
return Response().error("handler_full_name 与 new_name 均为必填。").__dict__
try:
- await rename_command_service(handler_full_name, new_name)
+ await rename_command_service(handler_full_name, new_name, aliases=aliases)
except ValueError as exc:
return Response().error(str(exc)).__dict__
</code_context>
<issue_to_address>
**suggestion (bug_risk):** `aliases` 负载在传递过程中没有做类型规范化，如果客户端发送的是非列表值，可能会导致奇怪的行为。
服务端期望 `aliases` 是一个字符串列表，但当前路由直接转发了 `data.get("aliases")` 的原始返回值。如果客户端发送的是字符串或其他非列表的值，服务会按字符逐个进行迭代，并在 `extra_data` 中存储一个非列表的 `resolved_aliases`，而这个值之后会被 `isinstance(..., list)` 检查忽略。请在这里进行规范化，使 `aliases` 始终为 `None` 或字符串列表，并拒绝或转换其他类型以避免这种行为。
</issue_to_address>
### 评论 2
<location> `astrbot/core/star/command_management.py:108` </location>
<code_context>
- raise ValueError("新的指令名已被其他指令占用，请换一个名称。")
+ raise ValueError(f"指令名 '{candidate_full}' 已被其他指令占用。")
+
+ # 校验别名
+ if aliases:
+ for alias in aliases:
</code_context>
<issue_to_address>
**issue (complexity):** 建议抽取用于别名规范化和 `extra_data` 访问的共享辅助函数，这样别名处理和过滤器更新的逻辑只在一个地方定义，而不是在多个函数中重复。
你可以通过集中处理别名规范化和 `extra_data` 访问，并复用描述符/过滤器的别名路径，来降低新增的复杂度。
### 1. 将别名规范化逻辑集中到一处
目前，同样的逻辑分别出现在 `_bind_descriptor_with_config`、`_apply_config_to_runtime` 和 `rename_command`（校验步骤）中。可以用一个小的辅助函数把规则集中起来：
```python
def _normalize_aliases(raw_aliases: Any) -> list[str]:
if not isinstance(raw_aliases, list):
return []
result: list[str] = []
for x in raw_aliases:
s = str(x).strip()
if s:
result.append(s)
return result
```
然后在各处复用它：
```python
# rename_command – 校验 aliases
if aliases:
normalized_aliases = _normalize_aliases(aliases)
for alias in normalized_aliases:
alias_full = _compose_command(descriptor.parent_signature, alias)
if _is_command_in_use(handler_full_name, alias_full):
raise ValueError(f"别名 '{alias_full}' 已被其他指令占用。")
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = dict(existing_cfg.extra_data or {}) if existing_cfg else {}
merged_extra["resolved_aliases"] = _normalize_aliases(aliases or [])
```
```python
# _bind_descriptor_with_config
extra = config.extra_data or {}
descriptor.aliases = _normalize_aliases(extra.get("resolved_aliases"))
```
```python
# _apply_config_to_runtime
descriptor.handler.enabled = config.enabled
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
aliases = _normalize_aliases((config.extra_data or {}).get("resolved_aliases"))
_set_filter_aliases(descriptor.filter_ref, aliases)
```
这样就可以去掉重复的“转为字符串 / 去空格 / 过滤空字符串 / 只接受列表”逻辑。
### 2. 为别名抽象 `extra_data` 的访问
可以新增一些小的辅助函数，而不是让每个调用点都知道 `"resolved_aliases"` 这个 key：
```python
def _get_resolved_aliases(config: CommandConfig) -> list[str]:
extra = config.extra_data or {}
return _normalize_aliases(extra.get("resolved_aliases"))
def _set_resolved_aliases(
base_extra: dict[str, Any] | None,
aliases: list[str] | None,
) -> dict[str, Any]:
extra = dict(base_extra or {})
extra["resolved_aliases"] = _normalize_aliases(aliases or [])
return extra
```
然后在调用处使用：
```python
# rename_command
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = _set_resolved_aliases(
existing_cfg.extra_data if existing_cfg else None,
aliases,
)
config = await db_helper.upsert_command_config(
# ...
extra_data=merged_extra,
# ...
)
```
```python
# _bind_descriptor_with_config
descriptor.aliases = _get_resolved_aliases(config)
```
```python
# _apply_config_to_runtime
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
_set_filter_aliases(descriptor.filter_ref, _get_resolved_aliases(config))
```
这样一来，未来如果 `extra_data` 的结构或别名规则发生变化，只需要修改这些辅助函数即可。
### 3. 明确过滤器别名在列表 ↔ 集合之间的转换
`descriptor.aliases` 和 `extra_data["resolved_aliases"]` 使用的是列表，而过滤器期望的是 `set`。可以把这层转换封装到 `_set_filter_aliases` 中，这样调用点就不需要关心差异：
```python
def _set_filter_aliases(
filter_ref: CommandFilter | CommandGroupFilter,
aliases: list[str],
) -> None:
new_aliases = set(aliases)
current_aliases = getattr(filter_ref, "alias", set())
if new_aliases == current_aliases:
return
setattr(filter_ref, "alias", new_aliases)
if hasattr(filter_ref, "_cmpl_cmd_names"):
filter_ref._cmpl_cmd_names = None
```
结合 `_get_resolved_aliases`，这可以让描述符/过滤器的逻辑路径保持一致，并且把“为什么这里用 set 而不是 list”这类细节集中到一个地方管理。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In the backend
rename_commandhandler, consider validating/coercingaliasesinto a list of strings (e.g., ignore non-list inputs, filter out non-string items, and de-duplicate) before iterating and storing to avoid unexpected behavior from malformed JSON payloads. - In
_set_filter_aliases, you assumefilter_ref.aliasis aset; if other code sets it to a different type (e.g., list), the equality checkset(aliases) == current_aliaseswill always be false—consider normalizingcurrent_aliasesto a set before comparison to avoid redundant updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the backend `rename_command` handler, consider validating/coercing `aliases` into a list of strings (e.g., ignore non-list inputs, filter out non-string items, and de-duplicate) before iterating and storing to avoid unexpected behavior from malformed JSON payloads.
- In `_set_filter_aliases`, you assume `filter_ref.alias` is a `set`; if other code sets it to a different type (e.g., list), the equality check `set(aliases) == current_aliases` will always be false—consider normalizing `current_aliases` to a set before comparison to avoid redundant updates.
## Individual Comments
### Comment 1
<location> `astrbot/dashboard/routes/command.py:64-70` </location>
<code_context>
data = await request.get_json()
handler_full_name = data.get("handler_full_name")
new_name = data.get("new_name")
+ aliases = data.get("aliases")
if not handler_full_name or not new_name:
return Response().error("handler_full_name 与 new_name 均为必填。").__dict__
try:
- await rename_command_service(handler_full_name, new_name)
+ await rename_command_service(handler_full_name, new_name, aliases=aliases)
except ValueError as exc:
return Response().error(str(exc)).__dict__
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The `aliases` payload is passed through without type normalization, which may lead to odd behavior if the client sends a non-list value.
The service expects `aliases` to be a list of strings, but this route forwards whatever `data.get("aliases")` returns. If a client sends a string or other non-list value, the service will iterate over it character-by-character and store a non-list `resolved_aliases` in `extra_data`, which is later ignored by `isinstance(..., list)` checks. Please normalize here so `aliases` is always `None` or a list of strings, and reject or coerce other types to avoid this behavior.
</issue_to_address>
### Comment 2
<location> `astrbot/core/star/command_management.py:108` </location>
<code_context>
- raise ValueError("新的指令名已被其他指令占用，请换一个名称。")
+ raise ValueError(f"指令名 '{candidate_full}' 已被其他指令占用。")
+
+ # 校验别名
+ if aliases:
+ for alias in aliases:
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared helpers for alias normalization and extra_data access so alias handling and filter updates are defined once instead of repeated in multiple functions.
You can reduce the new complexity by centralizing alias normalization and `extra_data` access, and by sharing the descriptor/filter alias paths.
### 1. Centralize alias normalization
Right now the same logic appears in `_bind_descriptor_with_config`, `_apply_config_to_runtime`, and `rename_command` (validation step). A small helper keeps the rules in one place:
```python
def _normalize_aliases(raw_aliases: Any) -> list[str]:
if not isinstance(raw_aliases, list):
return []
result: list[str] = []
for x in raw_aliases:
s = str(x).strip()
if s:
result.append(s)
return result
```
Then reuse it:
```python
# rename_command – validate aliases
if aliases:
normalized_aliases = _normalize_aliases(aliases)
for alias in normalized_aliases:
alias_full = _compose_command(descriptor.parent_signature, alias)
if _is_command_in_use(handler_full_name, alias_full):
raise ValueError(f"别名 '{alias_full}' 已被其他指令占用。")
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = dict(existing_cfg.extra_data or {}) if existing_cfg else {}
merged_extra["resolved_aliases"] = _normalize_aliases(aliases or [])
```
```python
# _bind_descriptor_with_config
extra = config.extra_data or {}
descriptor.aliases = _normalize_aliases(extra.get("resolved_aliases"))
```
```python
# _apply_config_to_runtime
descriptor.handler.enabled = config.enabled
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
aliases = _normalize_aliases((config.extra_data or {}).get("resolved_aliases"))
_set_filter_aliases(descriptor.filter_ref, aliases)
```
This removes repeated “cast to str / strip / non-empty / list-only” logic.
### 2. Abstract `extra_data` access for aliases
Instead of each call site knowing about `"resolved_aliases"`, add tiny helpers:
```python
def _get_resolved_aliases(config: CommandConfig) -> list[str]:
extra = config.extra_data or {}
return _normalize_aliases(extra.get("resolved_aliases"))
def _set_resolved_aliases(
base_extra: dict[str, Any] | None,
aliases: list[str] | None,
) -> dict[str, Any]:
extra = dict(base_extra or {})
extra["resolved_aliases"] = _normalize_aliases(aliases or [])
return extra
```
Then:
```python
# rename_command
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = _set_resolved_aliases(
existing_cfg.extra_data if existing_cfg else None,
aliases,
)
config = await db_helper.upsert_command_config(
# ...
extra_data=merged_extra,
# ...
)
```
```python
# _bind_descriptor_with_config
descriptor.aliases = _get_resolved_aliases(config)
```
```python
# _apply_config_to_runtime
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
_set_filter_aliases(descriptor.filter_ref, _get_resolved_aliases(config))
```
Now future changes to `extra_data` shape or alias rules only touch these helpers.
### 3. Clarify list ↔ set conversion for filter aliases
`descriptor.aliases` and `extra_data["resolved_aliases"]` are lists, while the filter expects a `set`. Wrap this in `_set_filter_aliases` so call sites don’t worry about the difference:
```python
def _set_filter_aliases(
filter_ref: CommandFilter | CommandGroupFilter,
aliases: list[str],
) -> None:
new_aliases = set(aliases)
current_aliases = getattr(filter_ref, "alias", set())
if new_aliases == current_aliases:
return
setattr(filter_ref, "alias", new_aliases)
if hasattr(filter_ref, "_cmpl_cmd_names"):
filter_ref._cmpl_cmd_names = None
```
Combined with `_get_resolved_aliases`, this makes the descriptor/filter paths consistent and keeps the “why set vs list” detail in one place.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| raise ValueError("新的指令名已被其他指令占用，请换一个名称。") | ||
| raise ValueError(f"指令名 '{candidate_full}' 已被其他指令占用。") | ||
|
|
||
| # 校验别名 |
Contributor
There was a problem hiding this comment.
issue (complexity): 建议抽取用于别名规范化和 extra_data 访问的共享辅助函数，这样别名处理和过滤器更新的逻辑只在一个地方定义，而不是在多个函数中重复。
你可以通过集中处理别名规范化和 extra_data 访问，并复用描述符/过滤器的别名路径，来降低新增的复杂度。
1. 将别名规范化逻辑集中到一处
目前，同样的逻辑分别出现在 _bind_descriptor_with_config、_apply_config_to_runtime 和 rename_command（校验步骤）中。可以用一个小的辅助函数把规则集中起来：
def _normalize_aliases(raw_aliases: Any) -> list[str]:
if not isinstance(raw_aliases, list):
return []
result: list[str] = []
for x in raw_aliases:
s = str(x).strip()
if s:
result.append(s)
return result然后在各处复用它：
# rename_command – 校验 aliases
if aliases:
normalized_aliases = _normalize_aliases(aliases)
for alias in normalized_aliases:
alias_full = _compose_command(descriptor.parent_signature, alias)
if _is_command_in_use(handler_full_name, alias_full):
raise ValueError(f"别名 '{alias_full}' 已被其他指令占用。")
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = dict(existing_cfg.extra_data or {}) if existing_cfg else {}
merged_extra["resolved_aliases"] = _normalize_aliases(aliases or [])# _bind_descriptor_with_config
extra = config.extra_data or {}
descriptor.aliases = _normalize_aliases(extra.get("resolved_aliases"))# _apply_config_to_runtime
descriptor.handler.enabled = config.enabled
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
aliases = _normalize_aliases((config.extra_data or {}).get("resolved_aliases"))
_set_filter_aliases(descriptor.filter_ref, aliases)这样就可以去掉重复的“转为字符串 / 去空格 / 过滤空字符串 / 只接受列表”逻辑。
2. 为别名抽象 extra_data 的访问
可以新增一些小的辅助函数，而不是让每个调用点都知道 "resolved_aliases" 这个 key：
def _get_resolved_aliases(config: CommandConfig) -> list[str]:
extra = config.extra_data or {}
return _normalize_aliases(extra.get("resolved_aliases"))
def _set_resolved_aliases(
base_extra: dict[str, Any] | None,
aliases: list[str] | None,
) -> dict[str, Any]:
extra = dict(base_extra or {})
extra["resolved_aliases"] = _normalize_aliases(aliases or [])
return extra然后在调用处使用：
# rename_command
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = _set_resolved_aliases(
existing_cfg.extra_data if existing_cfg else None,
aliases,
)
config = await db_helper.upsert_command_config(
# ...
extra_data=merged_extra,
# ...
)# _bind_descriptor_with_config
descriptor.aliases = _get_resolved_aliases(config)# _apply_config_to_runtime
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
_set_filter_aliases(descriptor.filter_ref, _get_resolved_aliases(config))这样一来，未来如果 extra_data 的结构或别名规则发生变化，只需要修改这些辅助函数即可。
3. 明确过滤器别名在列表 ↔ 集合之间的转换
descriptor.aliases 和 extra_data["resolved_aliases"] 使用的是列表，而过滤器期望的是 set。可以把这层转换封装到 _set_filter_aliases 中，这样调用点就不需要关心差异：
def _set_filter_aliases(
filter_ref: CommandFilter | CommandGroupFilter,
aliases: list[str],
) -> None:
new_aliases = set(aliases)
current_aliases = getattr(filter_ref, "alias", set())
if new_aliases == current_aliases:
return
setattr(filter_ref, "alias", new_aliases)
if hasattr(filter_ref, "_cmpl_cmd_names"):
filter_ref._cmpl_cmd_names = None结合 _get_resolved_aliases，这可以让描述符/过滤器的逻辑路径保持一致，并且把“为什么这里用 set 而不是 list”这类细节集中到一个地方管理。
Original comment in English
issue (complexity): Consider extracting shared helpers for alias normalization and extra_data access so alias handling and filter updates are defined once instead of repeated in multiple functions.
You can reduce the new complexity by centralizing alias normalization and extra_data access, and by sharing the descriptor/filter alias paths.
1. Centralize alias normalization
Right now the same logic appears in _bind_descriptor_with_config, _apply_config_to_runtime, and rename_command (validation step). A small helper keeps the rules in one place:
def _normalize_aliases(raw_aliases: Any) -> list[str]:
if not isinstance(raw_aliases, list):
return []
result: list[str] = []
for x in raw_aliases:
s = str(x).strip()
if s:
result.append(s)
return resultThen reuse it:
# rename_command – validate aliases
if aliases:
normalized_aliases = _normalize_aliases(aliases)
for alias in normalized_aliases:
alias_full = _compose_command(descriptor.parent_signature, alias)
if _is_command_in_use(handler_full_name, alias_full):
raise ValueError(f"别名 '{alias_full}' 已被其他指令占用。")
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = dict(existing_cfg.extra_data or {}) if existing_cfg else {}
merged_extra["resolved_aliases"] = _normalize_aliases(aliases or [])# _bind_descriptor_with_config
extra = config.extra_data or {}
descriptor.aliases = _normalize_aliases(extra.get("resolved_aliases"))# _apply_config_to_runtime
descriptor.handler.enabled = config.enabled
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
aliases = _normalize_aliases((config.extra_data or {}).get("resolved_aliases"))
_set_filter_aliases(descriptor.filter_ref, aliases)This removes repeated “cast to str / strip / non-empty / list-only” logic.
2. Abstract extra_data access for aliases
Instead of each call site knowing about "resolved_aliases", add tiny helpers:
def _get_resolved_aliases(config: CommandConfig) -> list[str]:
extra = config.extra_data or {}
return _normalize_aliases(extra.get("resolved_aliases"))
def _set_resolved_aliases(
base_extra: dict[str, Any] | None,
aliases: list[str] | None,
) -> dict[str, Any]:
extra = dict(base_extra or {})
extra["resolved_aliases"] = _normalize_aliases(aliases or [])
return extraThen:
# rename_command
existing_cfg = await db_helper.get_command_config(handler_full_name)
merged_extra = _set_resolved_aliases(
existing_cfg.extra_data if existing_cfg else None,
aliases,
)
config = await db_helper.upsert_command_config(
# ...
extra_data=merged_extra,
# ...
)# _bind_descriptor_with_config
descriptor.aliases = _get_resolved_aliases(config)# _apply_config_to_runtime
if descriptor.filter_ref:
if descriptor.current_fragment:
_set_filter_fragment(descriptor.filter_ref, descriptor.current_fragment)
_set_filter_aliases(descriptor.filter_ref, _get_resolved_aliases(config))Now future changes to extra_data shape or alias rules only touch these helpers.
3. Clarify list ↔ set conversion for filter aliases
descriptor.aliases and extra_data["resolved_aliases"] are lists, while the filter expects a set. Wrap this in _set_filter_aliases so call sites don’t worry about the difference:
def _set_filter_aliases(
filter_ref: CommandFilter | CommandGroupFilter,
aliases: list[str],
) -> None:
new_aliases = set(aliases)
current_aliases = getattr(filter_ref, "alias", set())
if new_aliases == current_aliases:
return
setattr(filter_ref, "alias", new_aliases)
if hasattr(filter_ref, "_cmpl_cmd_names"):
filter_ref._cmpl_cmd_names = NoneCombined with _get_resolved_aliases, this makes the descriptor/filter paths consistent and keeps the “why set vs list” detail in one place.
ocetars
changed the title
Soulter
approved these changes
Dec 24, 2025
Soulter
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 / 改动点
在指令管理面板中为重命名命令引入了别名管理，涵盖了从前端 UI 交互到后端 API 及核心逻辑的完整实现。
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
在仪表板 UI 和核心命令管理逻辑中，新增对命令别名管理的支持，并与命令重命名功能集成。
新功能：
改进：
Original summary in English
Summary by Sourcery
Add support for managing command aliases alongside command renaming across the dashboard UI and core command management logic.
New Features:
Enhancements: