fix(ltm): prevent wake commands from being recorded as group chat context#8536
Conversation
dosubot
Bot
added
size:S
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request prevents command handler messages (like slash commands) from being recorded into the group chat context. It also updates the unit tests to mock this behavior. The reviewer recommends refactoring the mock for event.get_extra in the tests to use a stateful dictionary with side_effect instead of a static return value, which avoids fragile tests when other keys or default values are requested.
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.
| # Simulate a regular message: no command handler matched. | ||
| event.get_extra.return_value = {} |
There was a problem hiding this comment.
Mocking event.get_extra with a static return value of {} can lead to fragile tests and unexpected behavior. For example, other code paths calling event.get_extra with different keys and default values (such as _clean_group_context_session defaulting to False, or _group_context_raw_idx defaulting to -1) will incorrectly receive {} (which is truthy in some contexts or of the wrong type).
Instead, simulate a stateful get_extra and set_extra using a local dictionary and side_effect to ensure correct default values and state retention.
| # Simulate a regular message: no command handler matched. | |
| event.get_extra.return_value = {} | |
| extra = {} | |
| event.get_extra.side_effect = lambda key, default=None: extra.get(key, default) | |
| event.set_extra.side_effect = extra.__setitem__ |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Using the truthiness of
event.get_extra("handlers_parsed_params", {})couples behavior to the structure of that payload; consider instead checking for the presence of the key or a dedicated flag (e.g.is_command) so that a valid-but-empty param dict doesn’t accidentally re-enable recording. - In
make_event,event.get_extra.return_value = {}will return{}for any key, which can hide regressions for other extras; it would be clearer to configureget_extrato only return{}forhandlers_parsed_paramsand behave normally (e.g. raise or return a sentinel) for other keys.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using the truthiness of `event.get_extra("handlers_parsed_params", {})` couples behavior to the structure of that payload; consider instead checking for the presence of the key or a dedicated flag (e.g. `is_command`) so that a valid-but-empty param dict doesn’t accidentally re-enable recording.
- In `make_event`, `event.get_extra.return_value = {}` will return `{}` for any key, which can hide regressions for other extras; it would be clearer to configure `get_extra` to only return `{}` for `handlers_parsed_params` and behave normally (e.g. raise or return a sentinel) for other keys.
## Individual Comments
### Comment 1
<location path="astrbot/builtin_stars/astrbot/main.py" line_range="175" />
<code_context>
+ # Skip recording if a command handler matched (e.g. /reset,
+ # /help, /new). Slash commands are bot instructions, not group
+ # chat context that should be injected into future LLM requests.
+ if not event.get_extra("handlers_parsed_params", {}):
+ try:
+ await self.group_chat_context.handle_message(event)
</code_context>
<issue_to_address>
**suggestion:** The truthiness check on `handlers_parsed_params` may be too implicit and could behave unexpectedly if the value is set but empty.
This relies on the truthiness of `handlers_parsed_params`. If a handler intentionally sets it to an empty dict or another falsy value, this will still treat the message as non-command and add it to group context. If you only need to know whether any handler matched, consider using `None` as the default and checking `is None`, or track a separate boolean like `handler_matched` so behavior isn’t tied to the parameter contents.
Suggested implementation:
```python
if group_icl_enable:
# Skip recording if a command handler matched (e.g. /reset,
# /help, /new). Slash commands are bot instructions, not group
# chat context that should be injected into future LLM requests.
handlers_parsed_params = event.get_extra("handlers_parsed_params", None)
if handlers_parsed_params is None:
try:
await self.group_chat_context.handle_message(event)
except BaseException as e:
logger.error(e)
```
This change assumes that elsewhere in the code, command handlers either:
1. Set `handlers_parsed_params` to a non-`None` value when they match (even if empty dict/list/etc.), and
2. Leave it unset (or explicitly `None`) when no handler matches.
If current handlers rely on different behavior (e.g. they sometimes set `handlers_parsed_params` to `None` even when they matched), you should normalize that behavior so that "no handler matched" is consistently represented by an absent key or `None`, and "handler matched" is always a non-`None` value.
</issue_to_address>
### Comment 2
<location path="tests/unit/test_group_chat_context_wiring.py" line_range="24-25" />
<code_context>
event.message_obj = SimpleNamespace(message=[Plain("hello")])
event.message_str = "hello"
event.session_id = "session-1"
+ # Simulate a regular message: no command handler matched.
+ event.get_extra.return_value = {}
return event
</code_context>
<issue_to_address>
**issue (testing):** Add a dedicated test for slash-command events where `handlers_parsed_params` is non-empty to prove that `group_chat_context.handle_message` is not called.
The helper now covers the regular-message path (no command handler match). Please also add a test for the other branch: when `event.get_extra("handlers_parsed_params", {})` returns a non-empty dict (e.g., a slash command), assert that `group_chat_context.handle_message` is not called. This will guard against slash commands polluting `raw_records` again.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Skip recording if a command handler matched (e.g. /reset, | ||
| # /help, /new). Slash commands are bot instructions, not group | ||
| # chat context that should be injected into future LLM requests. | ||
| if not event.get_extra("handlers_parsed_params", {}): |
There was a problem hiding this comment.
suggestion: The truthiness check on handlers_parsed_params may be too implicit and could behave unexpectedly if the value is set but empty.
This relies on the truthiness of handlers_parsed_params. If a handler intentionally sets it to an empty dict or another falsy value, this will still treat the message as non-command and add it to group context. If you only need to know whether any handler matched, consider using None as the default and checking is None, or track a separate boolean like handler_matched so behavior isn’t tied to the parameter contents.
Suggested implementation:
if group_icl_enable:
# Skip recording if a command handler matched (e.g. /reset,
# /help, /new). Slash commands are bot instructions, not group
# chat context that should be injected into future LLM requests.
handlers_parsed_params = event.get_extra("handlers_parsed_params", None)
if handlers_parsed_params is None:
try:
await self.group_chat_context.handle_message(event)
except BaseException as e:
logger.error(e)This change assumes that elsewhere in the code, command handlers either:
- Set
handlers_parsed_paramsto a non-Nonevalue when they match (even if empty dict/list/etc.), and - Leave it unset (or explicitly
None) when no handler matches.
If current handlers rely on different behavior (e.g. they sometimes set handlers_parsed_params to None even when they matched), you should normalize that behavior so that "no handler matched" is consistently represented by an absent key or None, and "handler matched" is always a non-None value.
| # Simulate a regular message: no command handler matched. | ||
| event.get_extra.return_value = {} |
There was a problem hiding this comment.
issue (testing): Add a dedicated test for slash-command events where handlers_parsed_params is non-empty to prove that group_chat_context.handle_message is not called.
The helper now covers the regular-message path (no command handler match). Please also add a test for the other branch: when event.get_extra("handlers_parsed_params", {}) returns a non-empty dict (e.g., a slash command), assert that group_chat_context.handle_message is not called. This will guard against slash commands polluting raw_records again.
1 participant
Motivation
Slash commands (e.g.
/reset,/new,/help) were incorrectly recorded into the group chat context buffer (raw_records) by theon_messagehandler. For/resetspecifically, this caused a timing issue where the command message was re-added afterafter_message_senthad already cleared the buffer via_clean_group_context_session, leaving stale context that would be injected into future LLM requests.Changes
astrbot/builtin_stars/astrbot/main.py: In theon_messagehandler, checkevent.get_extra("handlers_parsed_params", {})before callinghandle_message. This dict is populated byWakingCheckStageonly when aCommandFiltersuccessfully matches — i.e. a slash command is triggered. If non-empty, skip recording because the message is a bot instruction, not group chat context. Regular@botmentions (which also setis_at_or_wake_commandbut do not match anyCommandFilter) continue to be recorded as before.Impact
/reset,/new,/help,/sid,/stop,/stats,/set,/unset,/provider,/dashboard_update) are no longer stored in the group context buffer.@botmentions and unrecognised/prefixmessages (which fall through to the LLM agent) are unaffected.Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能，已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试，并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Prevent command-like wake messages from being stored in group chat long-term context to avoid stale slash command instructions being injected into future LLM requests.
Bug Fixes:
Tests: