feat(weixin): add iLink private chat and unified routing#88
Merged
Conversation
Add canonical delivery addresses, logical QQ identity binding, secure local state, Runtime/Management APIs, scheduling support, responsive WebUI management, documentation, and regression coverage. Co-authored-by: GPT-5 Codex <noreply@openai.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe pull request adds WeChat ClawBot/iLink private-chat support, canonical delivery-address routing, account and state persistence, scheduler and API integration, authenticated WebUI management, documentation, release metadata, and focused tests while retaining legacy QQ target compatibility. ChangesWeChat iLink platform
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (5)
tests/test_weixin_service.py (1)
152-173: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
service.stop()with try/finally across these tests.None of the 4 new tests (this one, plus
test_account_runtime_sends_by_logical_qq,test_unknown_peer_is_quarantined_without_dispatch,test_remove_account_purges_sdk_runtime_state) wrap the body in try/finally, so an assertion failure before the finalawait service.stop()skips cleanup and can leak the service's background login/run tasks into later tests.♻️ Suggested pattern
- with pytest.raises(WeixinConfirmationRequired) as raised: - await service.start_login(alias="admin", qq_id=4242) - assert manager.start_calls == 0 - - result = await service.start_login( - alias="admin", - qq_id=4242, - confirmation_token=raised.value.token, - ) - assert result.session_id == "session-1" - assert manager.start_calls == 1 - await service.stop() + try: + with pytest.raises(WeixinConfirmationRequired) as raised: + await service.start_login(alias="admin", qq_id=4242) + assert manager.start_calls == 0 + + result = await service.start_login( + alias="admin", + qq_id=4242, + confirmation_token=raised.value.token, + ) + assert result.session_id == "session-1" + assert manager.start_calls == 1 + finally: + await service.stop()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_weixin_service.py` around lines 152 - 173, Wrap the body of each of the four new tests, including test_privileged_binding_requires_confirmation_before_network, in try/finally and move await service.stop() into the finally block. Preserve the existing assertions and test flow while ensuring cleanup runs even when an assertion or awaited operation fails.src/Undefined/services/model_pool.py (1)
38-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a
Protocolinstead ofAnyfor the injectedsender.
sender/resolved_senderis typedAny, which is the escape hatch needed because callers passAddressBoundSender(a duck-typed proxy, not aMessageSendersubclass). A smallProtocolexposingsend_private_messagewould let mypy strict-check this boundary instead of silently accepting anything.♻️ Suggested Protocol-based typing
class PrivateMessageSender(Protocol): async def send_private_message(self, user_id: int, message: str, **kwargs: Any) -> int | None: ...- sender: Any | None = None, + sender: PrivateMessageSender | None = None,Also applies to: 73-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/services/model_pool.py` around lines 38 - 56, Define a structural PrivateMessageSender Protocol exposing async send_private_message with the required arguments and return type, then replace Any with this Protocol for the sender parameter and resolved sender flow in handle_private_message and the corresponding usage around line 73. Ensure the injected AddressBoundSender remains accepted through structural typing while enabling strict type checking.src/Undefined/utils/sender.py (1)
299-302: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWeChat access-control denials skip the detailed warning log used everywhere else in this file. Both new WeChat send paths raise a bare
PermissionErrorwith noreason=/enabled=detail and no precedinglogger.warning, unlikesend_private_message/send_group_messagein the same file — making WeChat delivery blocks harder to diagnose than QQ/group ones.
src/Undefined/utils/sender.py#L299-L302: in_send_weixin_message, log alogger.warningwithconfig.private_access_denied_reason(user_id)andconfig.access_control_enabled(), and includereason=/enabled=in the raised message, mirroringsend_private_message(lines 608-620).src/Undefined/utils/sender.py#L238-L241: apply the same enrichment insend_address_file's wechat branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/utils/sender.py` around lines 299 - 302, The WeChat access-control denial paths lack diagnostic logging and denial details. In src/Undefined/utils/sender.py lines 299-302 within _send_weixin_message and lines 238-241 within send_address_file, add the same warning-log enrichment used by send_private_message, including private_access_denied_reason(user_id) and access_control_enabled(), and include reason= and enabled= details in each raised PermissionError.src/Undefined/weixin/store.py (1)
27-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRoute
chmodthroughutils/io.pyfor consistency.
_secure_fileperforms the file-permission change directly viapath.chmod(wrapped inasyncio.to_thread) instead of going throughUndefined.utils.io, unlike every other disk operation in this module (io.read_json/io.write_json). It's already off the event loop, so this is a consistency nit rather than a blocking-I/O bug.As per path instructions, "Disk I/O should go through
src/Undefined/utils/io.pyso writes stay async-safe and atomic."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/weixin/store.py` around lines 27 - 29, Update _secure_file to perform the permission change through the appropriate helper in Undefined.utils.io instead of calling path.chmod via asyncio.to_thread, while preserving the existing POSIX check and 0o600 permission mode.Source: Path instructions
src/Undefined/memes/service.py (1)
617-680: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate identical
send_meme_by_uidimplementations.The
send_meme_by_uidmethod has been identically implemented (and recently updated) in bothMemeServiceandMemeSearchMixin. This duplication increases maintenance overhead and the risk of the two implementations diverging over time.Please consider eliminating the duplication by ensuring
MemeServiceinherits the method fromMemeSearchMixinor by extracting the shared logic into a single definitive location.
src/Undefined/memes/service.py#L617-L680: Remove or delegate this duplicated implementation.src/Undefined/memes/search.py#L405-L468: Keep as the primary implementation (or extract to a common base class).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/memes/service.py` around lines 617 - 680, Remove or delegate MemeService.send_meme_by_uid in src/Undefined/memes/service.py lines 617-680 so it no longer duplicates the implementation. Preserve src/Undefined/memes/search.py lines 405-468 as the single primary send_meme_by_uid implementation, ensuring MemeService inherits or reuses it without behavioral changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Undefined/main.py`:
- Around line 507-517: Wrap runtime_api_server.stop() in its own try/except
block, matching the guarded cleanup for weixin_service.stop() and
handler.close(), and log failures with logger.exception using a message
identifying the runtime API server cleanup. Ensure an exception there does not
prevent subsequent cleanup steps in the surrounding finally flow.
In `@src/Undefined/services/coordinator/private.py`:
- Around line 97-119: Move the private delivery-address validation using
resolved_address.target_type and target_id before the injection branch and its
early return. Ensure the validated resolved address is passed to
_handle_injection_response instead of allowing an unchecked address to be used,
while preserving the existing mismatch errors and normal delivery flow.
In `@src/Undefined/skills/toolsets/messages/send_message/config.json`:
- Around line 5-17: Update the parameter schema for the send-message tool to
reject requests that provide address together with the legacy target_type or
target_id parameters. Add the appropriate mutual-exclusion constraints while
preserving support for address alone and the legacy target parameters alone.
In `@src/Undefined/skills/toolsets/messages/send_private_message/handler.py`:
- Around line 10-13: Remove the repo-local parse_delivery_address and
resolve_delivery_address imports from the handler, and update the handler’s
execution path to retrieve both callables from context instead. Ensure callers
populate these context entries so the existing address-parsing and resolution
behavior remains unchanged.
In `@src/Undefined/utils/message_targets.py`:
- Around line 48-57: Update parse_delivery_address to accept and return an
existing DeliveryAddress instance unchanged before the non-string validation,
while preserving the current handling for None and string addresses.
In `@src/Undefined/utils/scheduler.py`:
- Around line 680-690: Update the private WeChat branch in the scheduler’s
message-delivery flow to detect image and record CQ media and route those
messages through send_address_file instead of send_address_message; preserve
send_address_message for ordinary text and keep the existing delivery_address
and target matching conditions unchanged.
- Around line 31-50: Update _resolve_task_address to compare an explicitly
provided address with the legacy target_id/target_type when both are present,
using the same conflict-validation behavior as resolve_delivery_address. Reject
mismatched canonical and legacy targets instead of silently returning the parsed
address; preserve the existing address-only and legacy-only resolution paths.
In `@src/Undefined/utils/sender.py`:
- Around line 288-367: Move the _register_local_segment_attachments call out of
the unconditional history_attachments assignment in _send_weixin_message and
invoke it only inside the existing if auto_history block, after the text and
media send loops complete successfully. Preserve attachment merging and
_append_attachment_refs behavior there, while avoiding registry writes when
auto_history is false or any send fails.
- Around line 1229-1272: Update _send_weixin_forward to log when a forwarded
segment has an unsupported type instead of silently continuing. Preserve the
existing handling for text, image, video, and file segments, and include the
segment type in the drop log so unsupported voice, at, reply, and similar
segments are traceable.
- Around line 1200-1228: Update the WeChat branch of
send_private_forward_message to register forwarded media through the same
attachment flow as the base implementation before writing history. Reuse
_register_local_segment_attachments (and any resulting attachment data) and pass
the attachments argument to history_manager.add_private_message, while
preserving the existing consolidated history text and WeChat transport metadata.
In `@src/Undefined/webui/static/js/weixin.js`:
- Around line 544-580: The Weixin dialog opened by openBindingDialog is missing
focus trapping and restoration. Invoke trapFocus when showDialog displays the
modal, using the dialog container, and invoke releaseFocus when the dialog
closes; preserve focus to the previously active element and ensure the existing
close paths use the release logic.
In `@src/Undefined/weixin/service.py`:
- Around line 250-297: Update start() so _running is not set to True when
weixin_config.enabled is false; perform the disabled check before marking the
service running, while preserving the offline early return and normal enabled
startup behavior. Ensure status() reports running as false whenever the
integration is disabled.
In `@tests/test_memes.py`:
- Around line 215-218: Update the assertion on
wechat_sender.send_address_message to invoke DeliveryAddress.canonical() before
comparing with "wechat:12345", rather than comparing the bound method object.
In `@tests/test_sender.py`:
- Around line 421-422: Update the async tests around the voice fixture setup to
replace direct Path.write_bytes() and Path.write_text() calls with the
corresponding async helpers from Undefined.utils.io, awaiting each operation so
disk I/O does not block the event loop.
---
Nitpick comments:
In `@src/Undefined/memes/service.py`:
- Around line 617-680: Remove or delegate MemeService.send_meme_by_uid in
src/Undefined/memes/service.py lines 617-680 so it no longer duplicates the
implementation. Preserve src/Undefined/memes/search.py lines 405-468 as the
single primary send_meme_by_uid implementation, ensuring MemeService inherits or
reuses it without behavioral changes.
In `@src/Undefined/services/model_pool.py`:
- Around line 38-56: Define a structural PrivateMessageSender Protocol exposing
async send_private_message with the required arguments and return type, then
replace Any with this Protocol for the sender parameter and resolved sender flow
in handle_private_message and the corresponding usage around line 73. Ensure the
injected AddressBoundSender remains accepted through structural typing while
enabling strict type checking.
In `@src/Undefined/utils/sender.py`:
- Around line 299-302: The WeChat access-control denial paths lack diagnostic
logging and denial details. In src/Undefined/utils/sender.py lines 299-302
within _send_weixin_message and lines 238-241 within send_address_file, add the
same warning-log enrichment used by send_private_message, including
private_access_denied_reason(user_id) and access_control_enabled(), and include
reason= and enabled= details in each raised PermissionError.
In `@src/Undefined/weixin/store.py`:
- Around line 27-29: Update _secure_file to perform the permission change
through the appropriate helper in Undefined.utils.io instead of calling
path.chmod via asyncio.to_thread, while preserving the existing POSIX check and
0o600 permission mode.
In `@tests/test_weixin_service.py`:
- Around line 152-173: Wrap the body of each of the four new tests, including
test_privileged_binding_requires_confirmation_before_network, in try/finally and
move await service.stop() into the finally block. Preserve the existing
assertions and test flow while ensuring cleanup runs even when an assertion or
awaited operation fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 419a6718-10dc-44de-a95c-7f1b7b4e02ef
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (80)
ARCHITECTURE.mdREADME.mdconfig.toml.exampledocs/access-control.mddocs/configuration.mddocs/management-api.mddocs/message-batching.mddocs/openapi.mddocs/pipelines.mddocs/usage.mddocs/webui-guide.mddocs/wechat-ilink.mdpyproject.tomlsrc/Undefined/api/_context.pysrc/Undefined/api/_openapi.pysrc/Undefined/api/app.pysrc/Undefined/api/routes/schedules.pysrc/Undefined/api/routes/weixin.pysrc/Undefined/attachments/render.pysrc/Undefined/config/__init__.pysrc/Undefined/config/config_class.pysrc/Undefined/config/domain_parsers.pysrc/Undefined/config/load_sections/domains.pysrc/Undefined/config/models.pysrc/Undefined/handlers/auto_extract.pysrc/Undefined/handlers/message_flow.pysrc/Undefined/main.pysrc/Undefined/memes/search.pysrc/Undefined/memes/service.pysrc/Undefined/scheduled_task_storage.pysrc/Undefined/services/coordinator/batching.pysrc/Undefined/services/coordinator/group.pysrc/Undefined/services/coordinator/private.pysrc/Undefined/services/message_batcher/state.pysrc/Undefined/services/model_pool.pysrc/Undefined/skills/pipelines/arxiv/handler.pysrc/Undefined/skills/pipelines/bilibili/handler.pysrc/Undefined/skills/pipelines/context.pysrc/Undefined/skills/pipelines/douyin/handler.pysrc/Undefined/skills/pipelines/github/handler.pysrc/Undefined/skills/toolsets/messages/send_message/config.jsonsrc/Undefined/skills/toolsets/messages/send_message/handler.pysrc/Undefined/skills/toolsets/messages/send_private_message/config.jsonsrc/Undefined/skills/toolsets/messages/send_private_message/handler.pysrc/Undefined/skills/toolsets/scheduler/README.mdsrc/Undefined/skills/toolsets/scheduler/create_schedule_task/config.jsonsrc/Undefined/skills/toolsets/scheduler/create_schedule_task/handler.pysrc/Undefined/skills/toolsets/scheduler/list_schedule_tasks/handler.pysrc/Undefined/skills/toolsets/scheduler/update_schedule_task/config.jsonsrc/Undefined/skills/toolsets/scheduler/update_schedule_task/handler.pysrc/Undefined/utils/history.pysrc/Undefined/utils/message_targets.pysrc/Undefined/utils/scheduler.pysrc/Undefined/utils/sender.pysrc/Undefined/utils/xml.pysrc/Undefined/webui/routes/__init__.pysrc/Undefined/webui/routes/_weixin.pysrc/Undefined/webui/static/css/components.csssrc/Undefined/webui/static/css/responsive.csssrc/Undefined/webui/static/js/i18n.jssrc/Undefined/webui/static/js/main.jssrc/Undefined/webui/static/js/schedules.jssrc/Undefined/webui/static/js/ui.jssrc/Undefined/webui/static/js/weixin.jssrc/Undefined/webui/templates/index.htmlsrc/Undefined/weixin/__init__.pysrc/Undefined/weixin/models.pysrc/Undefined/weixin/service.pysrc/Undefined/weixin/store.pytests/test_handlers_pipelines.pytests/test_memes.pytests/test_message_targets.pytests/test_runtime_api_schedules.pytests/test_runtime_api_weixin.pytests/test_scheduled_task_unit.pytests/test_scheduler_self_instruction.pytests/test_sender.pytests/test_weixin_config.pytests/test_weixin_service.pytests/test_weixin_store.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_webui_weixin_frontend.py (1)
14-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
pytest-asyncioinstead ofasyncio.run().As per coding guidelines, use
pytest-asynciofor async tests and rely on its auto-detection instead of spawning a separate event loop usingasyncio.run().♻️ Proposed refactor to native async test
-def _read_source() -> str: - source = asyncio.run(async_io.read_text(WEIXIN_JS)) +async def _read_source() -> str: + source = await async_io.read_text(WEIXIN_JS) assert source is not None return source -def test_weixin_dialog_traps_and_restores_focus() -> None: - source = _read_source() +async def test_weixin_dialog_traps_and_restores_focus() -> None: + source = await _read_source() show_dialog = source.split("function showDialog()", 1)[1].split(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_webui_weixin_frontend.py` around lines 14 - 34, Update _read_source and test_weixin_dialog_traps_and_restores_focus to use pytest-asyncio: make the helper and test async, await async_io.read_text directly, and remove asyncio.run() so pytest manages the event loop.Source: Coding guidelines
tests/test_send_message_tool.py (1)
32-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer an async test over
asyncio.run()wrapping.Since the suite already uses
pytest-asyncioinautomode, making this anasync deftest andawait-ingasync_io.read_textdirectly is more idiomatic than spinning up a throwaway event loop viaasyncio.run().♻️ Proposed refactor
-def test_send_message_schema_rejects_mixed_address_parameters() -> None: - config_text = asyncio.run( - async_io.read_text( - Path("src/Undefined/skills/toolsets/messages/send_message/config.json") - ) - ) +@pytest.mark.asyncio +async def test_send_message_schema_rejects_mixed_address_parameters() -> None: + config_text = await async_io.read_text( + Path("src/Undefined/skills/toolsets/messages/send_message/config.json") + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_send_message_tool.py` around lines 32 - 41, Convert test_send_message_schema_rejects_mixed_address_parameters into an async test and directly await async_io.read_text, removing the asyncio.run wrapper while preserving the existing configuration loading and validation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Undefined/services/model_pool.py`:
- Around line 21-42: Update the WeChat call to handle_private_message so it
passes the address-aware sender argument through to ModelPoolService, ensuring
/compare and model-selection replies use the current WeChat route instead of
self._sender. Locate the affected call site and preserve existing behavior for
other message handling.
---
Nitpick comments:
In `@tests/test_send_message_tool.py`:
- Around line 32-41: Convert
test_send_message_schema_rejects_mixed_address_parameters into an async test and
directly await async_io.read_text, removing the asyncio.run wrapper while
preserving the existing configuration loading and validation logic.
In `@tests/test_webui_weixin_frontend.py`:
- Around line 14-34: Update _read_source and
test_weixin_dialog_traps_and_restores_focus to use pytest-asyncio: make the
helper and test async, await async_io.read_text directly, and remove
asyncio.run() so pytest manages the event loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: df906a26-cdb3-465d-b3ab-933da9e81c67
📒 Files selected for processing (26)
docs/usage.mddocs/wechat-ilink.mdsrc/Undefined/ai/tooling.pysrc/Undefined/main.pysrc/Undefined/memes/service.pysrc/Undefined/services/coordinator/private.pysrc/Undefined/services/model_pool.pysrc/Undefined/skills/toolsets/messages/send_message/config.jsonsrc/Undefined/skills/toolsets/messages/send_private_message/handler.pysrc/Undefined/skills/toolsets/scheduler/create_schedule_task/handler.pysrc/Undefined/utils/io.pysrc/Undefined/utils/message_targets.pysrc/Undefined/utils/scheduler.pysrc/Undefined/utils/sender.pysrc/Undefined/webui/static/js/weixin.jssrc/Undefined/weixin/service.pysrc/Undefined/weixin/store.pytests/test_ai_coordinator_queue_routing.pytests/test_ai_tooling_context.pytests/test_message_targets.pytests/test_scheduler_self_instruction.pytests/test_send_message_tool.pytests/test_send_private_message_tool.pytests/test_sender.pytests/test_webui_weixin_frontend.pytests/test_weixin_service.py
🚧 Files skipped from review as they are similar to previous changes (15)
- src/Undefined/skills/toolsets/scheduler/create_schedule_task/handler.py
- docs/usage.md
- src/Undefined/main.py
- tests/test_message_targets.py
- src/Undefined/webui/static/js/weixin.js
- src/Undefined/weixin/store.py
- tests/test_weixin_service.py
- src/Undefined/skills/toolsets/messages/send_private_message/handler.py
- src/Undefined/memes/service.py
- docs/wechat-ilink.md
- src/Undefined/utils/sender.py
- src/Undefined/utils/scheduler.py
- src/Undefined/weixin/service.py
- src/Undefined/utils/message_targets.py
- src/Undefined/services/coordinator/private.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Teach both system prompts and message tools to use WeChat Markdown and literal special characters. Exclude routed media segments from WeChat text delivery so local paths are never exposed. Co-authored-by: GPT-5 <noreply@openai.com>
Preserve inbound quote context and add same-route native replies with deterministic Markdown fallback. Co-authored-by: OpenAI Codex <noreply@openai.com>
Render WeChat message bodies as safe CDATA and reinforce raw-symbol delivery constraints across prompts and message tools. Co-authored-by: OpenAI Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Recover quoted bot messages using route-aware timing metadata and keep WeChat input literal across prompts, history tools, and cognitive extraction. Co-authored-by: OpenAI Codex <noreply@openai.com>
Restart failed account runtimes, keep expired QR sessions refreshable, use server message times for quote recovery, and preserve mixed segment order. Co-authored-by: OpenAI Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
69gg
changed the title
Co-authored-by: GPT-5 <noreply@openai.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Undefined/utils/sender.py (1)
189-195: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRoute the new filesystem checks through async I/O helpers.
These async delivery paths invoke synchronous
resolve(),is_file(), and stat operations. Make local-path resolution async and useUndefined.utils.io.resolve_path,is_file, andget_file_size.As per coding guidelines, “Disk I/O should go through
src/Undefined/utils/io.py” and synchronous I/O must never block the event loop.Also applies to: 250-252, 439-440, 512-512, 1678-1680, 1743-1744
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/utils/sender.py` around lines 189 - 195, Replace synchronous filesystem operations in the affected async delivery paths with the helpers from Undefined.utils.io: make local-path resolution awaitable via resolve_path, and replace is_file and stat-based size checks with awaitable is_file and get_file_size calls. Update the surrounding methods to await these helpers while preserving existing path and validation behavior.Source: Coding guidelines
🧹 Nitpick comments (1)
src/Undefined/services/coordinator/__init__.py (1)
78-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider renaming
_send_imageto_send_media.Since this method correctly processes and sends both image and voice files (and now explicitly uses
VOICE_SOURCE_SUFFIXES), renaming it to_send_mediawould better reflect its actual responsibilities. Note that this would also require updating its call sites (e.g., inprivate.py).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/services/coordinator/__init__.py` at line 78, Rename the `_send_image` method to `_send_media` to reflect that it handles both image and voice files, including `VOICE_SOURCE_SUFFIXES`. Update every call site, including the usage in `private.py`, while preserving the existing behavior and arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Undefined/weixin/audio.py`:
- Around line 103-167: Update _normalize_audio_to_pcm to enforce a conversion
timeout covering stdout.read(), process.wait(), and stderr collection. On
timeout, preserve the existing BaseException cleanup path that kills the FFmpeg
process, waits for it, cancels stderr_task, and gathers it before propagating
the timeout.
In `@src/Undefined/weixin/service.py`:
- Around line 323-331: Update the capability contract returned by status() to
advertise outbound voice by adding the appropriate voice capability to the
"outbound" list and removing "outbound_voice" from "unsupported". Keep the
existing send_voice and send_prepared_voice implementations unchanged.
In `@tests/test_code_delivery_routing.py`:
- Around line 18-22: Replace the synchronous workspace.mkdir() and
task_dir.mkdir() calls in the test setup with the corresponding async
directory-creation helper from Undefined.utils.io, awaiting each operation
before writing result.txt. Keep the existing directory paths and fixture
behavior unchanged.
In `@tests/test_history_load_compat.py`:
- Around line 139-143: Initialize the manager’s _max_records attribute before
calling _load_history_from_file in this test, since
MessageHistoryManager.__new__ bypasses __init__. Preserve the existing
history-loading call and assertions.
In `@tests/test_weixin_service.py`:
- Around line 737-738: Update the service capability response in status() to
advertise outbound voice using the same capability name supported by
send_prepared_voice(), and remove that capability from unsupported. Adjust the
assertions in this test to verify outbound voice is present and no longer marked
unsupported, while preserving the existing reply_to checks.
---
Outside diff comments:
In `@src/Undefined/utils/sender.py`:
- Around line 189-195: Replace synchronous filesystem operations in the affected
async delivery paths with the helpers from Undefined.utils.io: make local-path
resolution awaitable via resolve_path, and replace is_file and stat-based size
checks with awaitable is_file and get_file_size calls. Update the surrounding
methods to await these helpers while preserving existing path and validation
behavior.
---
Nitpick comments:
In `@src/Undefined/services/coordinator/__init__.py`:
- Line 78: Rename the `_send_image` method to `_send_media` to reflect that it
handles both image and voice files, including `VOICE_SOURCE_SUFFIXES`. Update
every call site, including the usage in `private.py`, while preserving the
existing behavior and arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 936046b6-8cf0-4721-9921-cd54d79756f5
⛔ Files ignored due to path filters (5)
apps/undefined-chat/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-chat/src-tauri/Cargo.lockis excluded by!**/*.lockapps/undefined-console/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-console/src-tauri/Cargo.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (70)
CHANGELOG.mdREADME.mdapps/undefined-chat/package.jsonapps/undefined-chat/src-tauri/Cargo.tomlapps/undefined-chat/src-tauri/tauri.conf.jsonapps/undefined-console/package.jsonapps/undefined-console/src-tauri/Cargo.tomlapps/undefined-console/src-tauri/tauri.conf.jsonconfig.toml.exampledocs/configuration.mddocs/usage.mddocs/webui-guide.mddocs/wechat-ilink.mdpyproject.tomlres/prompts/undefined.xmlres/prompts/undefined_nagaagent.xmlsrc/Undefined/__init__.pysrc/Undefined/ai/client/setup.pysrc/Undefined/ai/prompts/constants.pysrc/Undefined/ai/prompts/current_input.pysrc/Undefined/ai/tooling.pysrc/Undefined/handlers/message_flow.pysrc/Undefined/services/coordinator/__init__.pysrc/Undefined/services/coordinator/batching.pysrc/Undefined/services/coordinator/private.pysrc/Undefined/services/message_batcher/state.pysrc/Undefined/services/model_pool.pysrc/Undefined/skills/agents/code_delivery_agent/tools/end/handler.pysrc/Undefined/skills/tools/end/handler.pysrc/Undefined/skills/toolsets/README.mdsrc/Undefined/skills/toolsets/messages/README.mdsrc/Undefined/skills/toolsets/messages/context_utils.pysrc/Undefined/skills/toolsets/messages/get_recent_messages/handler.pysrc/Undefined/skills/toolsets/messages/send_message/config.jsonsrc/Undefined/skills/toolsets/messages/send_message/handler.pysrc/Undefined/skills/toolsets/messages/send_private_message/config.jsonsrc/Undefined/skills/toolsets/messages/send_private_message/handler.pysrc/Undefined/skills/toolsets/messages/send_voice/config.jsonsrc/Undefined/skills/toolsets/messages/send_voice/handler.pysrc/Undefined/utils/history.pysrc/Undefined/utils/message_reply.pysrc/Undefined/utils/scheduler.pysrc/Undefined/utils/sender.pysrc/Undefined/utils/xml.pysrc/Undefined/webui/static/css/components.csssrc/Undefined/webui/static/js/i18n.jssrc/Undefined/webui/static/js/weixin.jssrc/Undefined/webui/templates/index.htmlsrc/Undefined/weixin/audio.pysrc/Undefined/weixin/service.pytests/test_ai_client_setup.pytests/test_ai_coordinator_queue_routing.pytests/test_ai_tooling_context.pytests/test_code_delivery_routing.pytests/test_end_tool.pytests/test_history_level.pytests/test_history_load_compat.pytests/test_message_reply.pytests/test_message_tools_level.pytests/test_prompt_builder_cognitive_query.pytests/test_send_message_tool.pytests/test_send_private_message_tool.pytests/test_send_voice_tool.pytests/test_sender.pytests/test_system_prompt_constraints.pytests/test_webui_weixin_frontend.pytests/test_weixin_audio.pytests/test_weixin_message_flow.pytests/test_weixin_service.pytests/test_xml_utils.py
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/test_ai_tooling_context.py
- src/Undefined/services/message_batcher/state.py
- src/Undefined/ai/tooling.py
- src/Undefined/webui/static/css/components.css
- src/Undefined/webui/static/js/i18n.js
- docs/usage.md
- config.toml.example
- src/Undefined/skills/toolsets/messages/send_message/config.json
- src/Undefined/utils/xml.py
- tests/test_send_private_message_tool.py
- docs/configuration.md
- src/Undefined/services/model_pool.py
- src/Undefined/webui/templates/index.html
- src/Undefined/skills/toolsets/messages/send_message/handler.py
- src/Undefined/handlers/message_flow.py
- src/Undefined/utils/scheduler.py
- src/Undefined/services/coordinator/private.py
Bound FFmpeg conversion time, advertise outbound voice support, and move media path metadata checks onto async I/O helpers. Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Undefined/services/command.py (1)
1071-1101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the explicit route sender to the entire private dispatch lifecycle.
The route sender currently affects only command-handler output; earlier validation replies and handler-error replies still use the default sender.
src/Undefined/services/command.py#L1071-L1101: resolve private delivery before defining_send_target_message.src/Undefined/services/command.py#L1242-L1263: reuse that resolved sender when constructing the command proxy rather than selecting it only here.tests/test_private_command_access.py#L148-L167: add denied/unknown/failing command cases and assert the default sender remains unused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/services/command.py` around lines 1071 - 1101, Resolve the private delivery sender at the start of _dispatch_internal, before defining _send_target_message, so validation and handler-error replies use the explicit route sender throughout the lifecycle. In src/Undefined/services/command.py lines 1071-1101 and 1242-1263, reuse that resolved sender when constructing the command proxy instead of selecting it locally. In tests/test_private_command_access.py lines 148-167, add denied, unknown, and failing command cases asserting the default sender is not used.
🧹 Nitpick comments (2)
tests/test_token_usage_storage_summary.py (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
@pytest.mark.asynciodecorator.As per coding guidelines,
asyncio_modeshould be configured to'auto'for automatic async test detection, which makes this explicit decorator unnecessary.♻️ Proposed refactor
-@pytest.mark.asyncio🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_token_usage_storage_summary.py` at line 44, Remove the redundant `@pytest.mark.asyncio` decorator from the affected async test in the token usage storage summary tests. Rely on the project’s configured asyncio_mode="auto" for async test detection, leaving the test implementation unchanged.Source: Coding guidelines
src/Undefined/services/commands/context.py (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepresent command-sender capabilities with typed Protocols.
The new polymorphic sender contract is currently expressed through
AnyandCallable[..., ...], preventing mypy from validating required methods and keyword arguments.
src/Undefined/services/commands/context.py#L22-L22: replaceAnywith a structural command-sender Protocol.src/Undefined/services/command.py#L82-L90: type the optional forward callback’s positional and keyword parameters.src/Undefined/services/command.py#L382-L390: reuse the same callback Protocol forsend_forward.As per coding guidelines, “All Python code must have complete type annotations, complying with mypy strict mode.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Undefined/services/commands/context.py` at line 22, Replace the Any sender annotation in commands/context.py at line 22 with a structural command-sender Protocol declaring the required methods and keyword arguments. In command.py at lines 82-90, define or reuse a typed callback Protocol for the optional forward callback with fully annotated positional and keyword parameters, then reuse that same Protocol for send_forward at lines 382-390 instead of Callable[..., ...]; keep all annotations compatible with mypy strict mode.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/slash-commands.md`:
- Around line 168-170: Update the `--ai` behavior description in the
documentation to state that AI analysis is awaited before rendering and that, on
timeout, the timeout message is included within the chart bundle. Remove the
claim that charts are sent first, and avoid specifying a fixed eight-minute
timeout; describe the timeout dynamically instead.
In `@src/Undefined/services/command.py`:
- Around line 112-128: Prevent send_private_forward_message from being selected
when _send_private_forward_message is unavailable by exposing an explicit
forwarding capability or providing separate proxy implementations. Update the
stats handler’s delivery selection to check that capability, preserving normal
chart-sequence delivery for supported channels and summary-only fallback only
when forwarding is unsupported. Add coverage for a non-WebUI callback-only
channel.
- Around line 430-458: Update the send_forward exception handling in command.py
around lines 430-458 to distinguish definitive delivery rejection from ambiguous
timeout or throttling errors; only send the text fallback for confirmed
rejection, and return without a secondary send for ambiguous failures. In
tests/test_stats_private_delivery.py lines 95-119, add timeout and rate-limit
cases asserting that _send_private is not called.
In `@src/Undefined/token_usage_storage.py`:
- Around line 143-156: Update the usage-reading flows, including
_iter_usage_records and the readers around archive enumeration, to acquire and
retain the storage/compaction lock from archive discovery through complete
record streaming, preventing compaction or pruning during the scan. Move the
synchronous file/archive streaming primitive out of token_usage_storage.py into
src/Undefined/utils/io.py, and have the locked readers reuse that utility while
preserving existing parsing and aggregation behavior.
---
Outside diff comments:
In `@src/Undefined/services/command.py`:
- Around line 1071-1101: Resolve the private delivery sender at the start of
_dispatch_internal, before defining _send_target_message, so validation and
handler-error replies use the explicit route sender throughout the lifecycle. In
src/Undefined/services/command.py lines 1071-1101 and 1242-1263, reuse that
resolved sender when constructing the command proxy instead of selecting it
locally. In tests/test_private_command_access.py lines 148-167, add denied,
unknown, and failing command cases asserting the default sender is not used.
---
Nitpick comments:
In `@src/Undefined/services/commands/context.py`:
- Line 22: Replace the Any sender annotation in commands/context.py at line 22
with a structural command-sender Protocol declaring the required methods and
keyword arguments. In command.py at lines 82-90, define or reuse a typed
callback Protocol for the optional forward callback with fully annotated
positional and keyword parameters, then reuse that same Protocol for
send_forward at lines 382-390 instead of Callable[..., ...]; keep all
annotations compatible with mypy strict mode.
In `@tests/test_token_usage_storage_summary.py`:
- Line 44: Remove the redundant `@pytest.mark.asyncio` decorator from the affected
async test in the token usage storage summary tests. Rely on the project’s
configured asyncio_mode="auto" for async test detection, leaving the test
implementation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c2d523a5-f6cb-44a9-a4c6-748e0735ae09
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
CHANGELOG.mdREADME.mdconfig.toml.exampledocs/configuration.mddocs/slash-commands.mddocs/usage.mddocs/wechat-ilink.mdpyproject.tomlsrc/Undefined/config/domain_parsers.pysrc/Undefined/config/models.pysrc/Undefined/handlers/message_flow.pysrc/Undefined/services/command.pysrc/Undefined/services/commands/context.pysrc/Undefined/skills/commands/stats/README.mdsrc/Undefined/skills/commands/stats/handler.pysrc/Undefined/token_usage_storage.pysrc/Undefined/utils/sender.pysrc/Undefined/weixin/service.pytests/test_private_command_access.pytests/test_sender.pytests/test_stats_handler_scope.pytests/test_stats_private_delivery.pytests/test_token_usage_storage_summary.pytests/test_weixin_config.pytests/test_weixin_service.py
🚧 Files skipped from review as they are similar to previous changes (13)
- src/Undefined/config/domain_parsers.py
- docs/usage.md
- tests/test_weixin_config.py
- pyproject.toml
- config.toml.example
- src/Undefined/config/models.py
- docs/configuration.md
- docs/wechat-ilink.md
- tests/test_weixin_service.py
- src/Undefined/handlers/message_flow.py
- src/Undefined/utils/sender.py
- tests/test_sender.py
- src/Undefined/weixin/service.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Impact
weixin-ilink-clientSDK, without requiring OneBot, OpenClaw, or the WeChat plugin at runtime.wechat:<qqid>route across history, permissions, cognitive memory, model selection, commands, pipelines, attachments, message batching, and scheduled tasks.Safety
data/path and is written with mode0600on POSIX.Validation
uv run ruff check .uv run ruff format --check .uv run mypy .uv run pytest tests/(2551 passed, 1 skipped)23 passed)cd apps/undefined-console && npm run checkcd apps/undefined-chat && npm run check(403unit tests,50e2e tests,73Rust tests)uv build --wheel --offlineSummary by CodeRabbit
address投递地址路由（QQ/群/微信）并扩展到定时任务。messages.send_voice/send_voice（按已有音频 UID 发送为语音）。