Conversation
![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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/aiocqhttp/delivery_safety.py" line_range="296-325" />
<code_context>
+ self._delivery_locks: dict[str, asyncio.Lock] = {}
</code_context>
<issue_to_address>
**issue (bug_risk):** `_delivery_locks` retains one `asyncio.Lock` for every private UMO ever sent and never removes entries, so the adapter's in-memory lock map grows without bound as it communicates with new users.
**Triggers:** When the bot sends private messages to many distinct users over its lifetime.
**Suggested fix:** Remove the lock entry after the guarded send completes, taking care to avoid deleting a lock that has been replaced or is still in use.
</issue_to_address>
### Comment 2
<location path="astrbot/core/platform/sources/aiocqhttp/aiocqhttp_platform_adapter.py" line_range="105-110" />
<code_context>
+ inbound_text = event.get("raw_message")
+ if not isinstance(inbound_text, str):
+ inbound_text = str(event.get("message") or "")
+ self.bot.delivery_store.record_inbound(
+ umo,
+ inbound_text,
+ event,
+ blocked=blocked,
+ )
+ if blocked:
+ return
</code_context>
<issue_to_address>
**issue (bug_risk):** A SQLite failure in `blocked()` or `record_inbound()` is caught by the handler's broad exception block before `convert_message()` and `handle_msg()` run, so a transient audit-database error silently drops the incoming private message instead of processing it.
**Triggers:** When the audit database is locked, unavailable, or otherwise raises an SQLite exception while a private event is received.
**Suggested fix:** Keep message processing independent of audit persistence: handle audit errors separately, log them, and continue to `convert_message()`/`handle_msg()` unless the circuit state was successfully read as blocked.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and an incorrect circuit-breaker or private-message classification could permanently block a conversation or allow a sensitive delivery, and the new audit database retains inbound and outbound records beyond the code revert. Reverting stops the new behavior but does not unsend messages or remove persisted circuits and audit data.
Blocking findings: astrbot/core/platform/sources/aiocqhttp/delivery_safety.py:325, astrbot/core/platform/sources/aiocqhttp/aiocqhttp_platform_adapter.py:110
| self._delivery_locks: dict[str, asyncio.Lock] = {} | ||
|
|
||
| async def call_action(self, action: str, **params): | ||
| base_action = action.removesuffix("_async") | ||
| private = base_action in {"send_private_msg", "send_private_forward_msg"} or ( | ||
| base_action in {"send_msg", "send_forward_msg"} | ||
| and not params.get("group_id") | ||
| and params.get("message_type") != "group" | ||
| ) | ||
| if not private: | ||
| return await super().call_action(action, **params) | ||
| user_id = str(params.get("user_id") or "") | ||
| if not user_id.isdigit(): | ||
| raise DeliveryBlocked("Private send has no valid user_id") | ||
| umo = f"{self.delivery_platform_id}:FriendMessage:{user_id}" | ||
| content = params.get("message", params.get("messages", [])) | ||
| if isinstance(content, str): | ||
| text = content | ||
| else: | ||
| text = "".join( | ||
| str(part.get("data", {}).get("text", "")) | ||
| if part.get("type") == "text" | ||
| else f"[{part.get('type', 'unknown')}]" | ||
| for part in content | ||
| if isinstance(part, dict) | ||
| ) | ||
| delivery_locks = getattr(self, "_delivery_locks", None) | ||
| if not isinstance(delivery_locks, dict): | ||
| delivery_locks = self._delivery_locks = {} | ||
| async with delivery_locks.setdefault(umo, asyncio.Lock()): |
There was a problem hiding this comment.
issue (bug_risk): _delivery_locks retains one asyncio.Lock for every private UMO ever sent and never removes entries, so the adapter's in-memory lock map grows without bound as it communicates with new users.
Triggers: When the bot sends private messages to many distinct users over its lifetime.
Suggested fix: Remove the lock entry after the guarded send completes, taking care to avoid deleting a lock that has been replaced or is still in use.
| self.bot.delivery_store.record_inbound( | ||
| umo, | ||
| inbound_text, | ||
| event, | ||
| blocked=blocked, | ||
| ) |
There was a problem hiding this comment.
issue (bug_risk): A SQLite failure in blocked() or record_inbound() is caught by the handler's broad exception block before convert_message() and handle_msg() run, so a transient audit-database error silently drops the incoming private message instead of processing it.
Triggers: When the audit database is locked, unavailable, or otherwise raises an SQLite exception while a private event is received.
Suggested fix: Keep message processing independent of audit persistence: handle audit errors separately, log them, and continue to convert_message()/handle_msg() unless the circuit state was successfully read as blocked.
1 participant
变更\n- 为 OneBot 私聊发送增加投递登记、回执状态和熔断保护\n- 在事件停止或取消后阻止后续发送\n- 跟进任务增加终止检查和等待超时\n- 为插件上下文页面提供可选的逐条投递审计数据\n\n## 验证\n- Ruff 检查通过\n- 16 项投递安全和跟进终止测试通过
Summary by Sourcery
Guard private OneBot message delivery with persistent safety controls and terminate queued follow-ups when conversations stop.
New Features:
Bug Fixes:
Enhancements:
Tests: