fix(channels/wecom-sidecar): recover req_id via librefang_user across sidecar restart#5449
Merged
Merged
Conversation
… sidecar restart Same bug class as the wechat restart-fragility hotfix: the in-process `_pending_req_ids[user_id]` cache vanishes on sidecar restart, so the bot's first reply after restart degrades from `aibot_respond_msg` (passive, free) to `aibot_send_msg` (active, quota-burning, blocked for unlicensed bots and rate-limited hard for licensed ones). Fix: parse_wecom_event also surfaces req_id via `ChannelUser.librefang_user`. on_send's `_enqueue_text` learns a `req_id_hint` keyword that falls back to it when the in-memory cache is empty. Cache stays as the primary path because it holds the FRESHEST req_id (subsequent inbounds clobber the dict but the round-tripped librefang_user is whichever inbound the daemon happens to surface — possibly stale). Tests: - test_on_send_recovers_req_id_from_user_librefang_user_when_cache_cold drives the post-restart shape (cache empty, librefang_user carries req_id), asserts on_send emits `aibot_respond_msg` with the recovered req_id rather than degrading to `aibot_send_msg`. - test_on_send_in_memory_cache_wins_over_librefang_user pins the cache-first precedence (freshness invariant). - test_on_send_ignores_url_shaped_librefang_user pins the cross- channel pollution guard. - cd sdk/python && pytest tests/test_wecom_adapter.py — 65 passed (was 62; +3 regression guards). Surfaces from the #5439-followup audit chain.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a1a4dcdb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+649
to
+652
| if (isinstance(candidate, str) and candidate | ||
| and not candidate.startswith(("http://", "https://", "@")) | ||
| and " " not in candidate | ||
| and "\t" not in candidate): |
There was a problem hiding this comment.
The new req_id_hint guard only rejects http(s), @, spaces, and tabs, so non-WeCom values like Bluesky’s at://... librefang_user (see sdk/python/librefang/sidecar/adapters/bluesky.py) still pass and are sent as aibot_respond_msg req_ids. In the cache-cold restart path this means we attempt a passive reply with an invalid req_id instead of safely falling back to aibot_send_msg, which can drop the outbound message. Tighten validation so fallback accepts only WeCom-shaped req_ids (or at least rejects URI-style values such as *:// and slash-containing IDs).
Useful? React with 👍 / 👎.
houko
added a commit
that referenced
this pull request
May 21, 2026
Surfaces from the post-#5445 audit (6th in the dingtalk/qq/omnibus review chain — first verdict that wasn't BROKEN/NEEDS_HOTFIX). All three nits are doc/UX cleanups, none are runtime regressions: 1. **Docstring claim #3 was wrong** — said "429 Retry-After honoured on every outbound POST", but only Cloud API uses `_cloud_post_with_retry`; the gateway path calls `_http_request` directly and raises on any non-2xx. Soften the claim to "Cloud-API outbound POSTs only" + explain when gateway-side retry would matter (operators proxying the local Baileys gateway behind a rate-limiting reverse-proxy). 2. **WHATSAPP_VERIFY_TOKEN unset failed silently** — Meta's subscription handshake returned 403 with no log line pointing back to the missing env var; operators saw "subscription failed" in the Meta dashboard with nothing in their daemon logs. Now warns at __init__, matching the WHATSAPP_APP_SECRET pattern already there. 3. **WHATSAPP_GROUP_POLICY is dead config** — Cloud API webhook payloads don't surface a group/conversation distinction (we hardcode `is_group=False` at `_handle_post_webhook`), and gateway mode delegates inbound entirely to the Node Baileys gateway which never calls back into the sidecar's `should_handle_message` filter. So `WHATSAPP_GROUP_POLICY` has zero effect today. Mark the schema field with an explicit "(currently inert)" note in the label so operators don't waste time setting it. Kept the field itself for forward-compat — Meta has been rolling out group-chat support to the Cloud API gradually and we want the schema to be ready when it lands. Drive-by: dropped the misleading send-voice docstring claim (lines 22-23 + 36-39 promised a `{gateway}/message/send-voice` multipart upload route that doesn't exist in code; rationale comment about the daemon's ChannelContent::Voice always carrying a URL at the dispatch boundary explains why we don't need it). Test: - New test_get_verify_empty_self_token_rejects asserts BOTH the __init__ warn fires AND the handshake fails closed. Regression guard for #2 — without this an attacker who guesses the empty hub.verify_token could subscribe their own callback URL. - cd sdk/python && pytest tests/test_whatsapp_adapter.py — 79 passed (was 78; +1 regression guard). This closes the audit chain. WhatsApp itself is structurally clean — natural routing key (phone) is preserved as channel_id both directions, the bug family from #5417 → #5449 doesn't apply.
github-actions
Bot
added
area/sdk
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.
Summary
Runtime hotfix surfaced by the #5439 follow-up audit chain.
WeCom sidecar's in-process `_pending_req_ids[user_id]` cache vanishes on sidecar restart, so the bot's first reply after restart degrades from `aibot_respond_msg` (passive, free) to `aibot_send_msg` (active, quota-burning, blocked for unlicensed bots and rate-limited hard for licensed ones).
Fix
Tests
Cross-reference
Same investigation chain as wechat restart hotfix.