Skip to content

fix(command): keep an argument that equals another alias of the same command#9294

Open
he-yufeng wants to merge 1 commit into
AstrBotDevs:masterfrom
he-yufeng:fix/command-filter-alias-arg
Open

fix(command): keep an argument that equals another alias of the same command#9294
he-yufeng wants to merge 1 commit into
AstrBotDevs:masterfrom
he-yufeng:fix/command-filter-alias-arg

Conversation

@he-yufeng

@he-yufeng he-yufeng commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Fixes a silent argument-parsing bug in CommandFilter. When a command has an alias and the user's first argument happens to equal that alias, the argument is silently dropped.

CommandFilter.filter loops over get_complete_command_names() (the command name plus every alias) and strips the matched prefix from message_str, but it never breaks after the first match. So once the command prefix is stripped, the loop keeps testing the remaining text against the other names, and if the first argument equals one of the command's own aliases it gets stripped a second time. No error, no log, the argument just disappears.

Modifications / 改动点

astrbot/core/star/filter/command.py: break out of the command-name loop after the first match, so only the leading command name is stripped and the rest is left as arguments.

tests/test_command_filter.py: regression tests covering an argument that equals an alias, a sole argument that equals an alias, and a normal argument (unaffected).

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

Reproduced against the real CommandFilter (command search aliased find, and add aliased new):

                                  before            after
'search find keyword'  ->  query='keyword'      query='find keyword'
'add new'              ->  query=''             query='new'
'search cat photo'     ->  query='cat photo'    query='cat photo'   (unchanged)
$ pytest tests/test_command_filter.py
...                                                       [100%]
3 passed

Without the fix, the two alias-collision tests fail (2 failed, 1 passed); with it, all three pass. tests/test_conversation_commands.py stays green.


Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc. (N/A — this is a bug fix, no new feature.)

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.

  • 🤓 I have ensured that no new dependencies are introduced.

  • 😮 My changes do not introduce malicious code.

Summary by Sourcery

Fix a command filtering bug where an argument equal to another alias of the same command was incorrectly stripped and add regression tests to cover alias-collision scenarios.

Bug Fixes:

  • Ensure CommandFilter stops after matching the invoked command name so subsequent arguments equal to other aliases are preserved.

Tests:

  • Add regression tests verifying that arguments matching another alias of the same command are retained, including sole-argument and normal-argument cases.

…command

CommandFilter.filter loops over get_complete_command_names() (the command name
plus every alias) and strips the matched prefix from message_str, but it does
not break after the first match. When a user's first argument happens to equal
another alias of the same command, the loop matches again on the already-
stripped string and silently drops that argument.

For a `search` command aliased `find`, `search find keyword` parsed to
`keyword` instead of `find keyword`; `add` aliased `new` turned `add new` into
an empty argument. Break after the first match. Normal arguments are unaffected.
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Jul 15, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a break statement in the command filter loop to stop processing after the first successful match, and adds corresponding unit tests to verify command filtering behavior. The reviewer pointed out that stopping at the first match makes the iteration order of command names critical. Because aliases are stored in a set, their iteration order is non-deterministic, which could lead to inconsistent matching behavior. The reviewer suggested sorting the command names by length in descending order to ensure deterministic matching and that more specific commands are matched first.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

if message_str.startswith(f"{full_cmd} ") or message_str == full_cmd:
ok = True
message_str = message_str[len(full_cmd) :].strip()
break

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

With the introduction of break, the matching process now stops at the first match. This makes the iteration order of get_complete_command_names() extremely critical.

Currently, get_complete_command_names() constructs the list using list(self.alias). Since self.alias is a set, its iteration order is non-deterministic in Python due to hash randomization. If a command has overlapping aliases (e.g., one alias is a prefix of another, such as "show" and "show all"), the matched command will be non-deterministic across different runs or environments.

To ensure deterministic matching and that the most specific (longest) command/alias is matched first, the list of complete command names should be sorted by length in descending order. Since get_complete_command_names is outside the modified diff hunk, please update it in astrbot/core/star/filter/command.py as follows:

def get_complete_command_names(self):
    if self._cmpl_cmd_names is not None:
        return self._cmpl_cmd_names
    names = [
        f"{parent} {cmd}" if parent else cmd
        for cmd in [self.command_name] + list(self.alias)
        for parent in self.parent_command_names or [""]
    ]
    # Sort by length descending to ensure deterministic matching and that longer (more specific) commands match first
    self._cmpl_cmd_names = sorted(names, key=len, reverse=True)
    return self._cmpl_cmd_names

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant