Skip to content

fix(whatsapp): recognise @lid and @s.whatsapp.net JIDs in send_message target resolution - #37924

Open
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/whatsapp-lid-jid-recognition
Open

fix(whatsapp): recognise @lid and @s.whatsapp.net JIDs in send_message target resolution#37924
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/whatsapp-lid-jid-recognition

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds recognition of WhatsApp @lid and @s.whatsapp.net JIDs as explicit targets in _parse_target_ref(). Previously, WhatsApp JIDs like 12345@lid or 919900123456@s.whatsapp.net fell through to the generic numeric / channel-name resolution paths, causing silent fallback to the home channel or bridge crashes.

Related Issue

Fixes #37906

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/send_message_tool.py: Added _WHATSAPP_JID_RE regex pattern matching digits@lid and digits@s.whatsapp.net JIDs. Added WhatsApp-specific branch in _parse_target_ref() that checks this regex before the generic E.164 phone number path.
  • tests/tools/test_send_message_tool.py: Added 4 test cases: @lid JID recognition, @s.whatsapp.net JID recognition, whitespace tolerance, and non-numeric JID rejection.

How to Test

  1. Run pytest tests/tools/test_send_message_tool.py -k "whatsapp" -v — all 6 WhatsApp tests should pass
  2. Verify _parse_target_ref("whatsapp", "12345@lid") returns ("12345@lid", None, True)
  3. Verify _parse_target_ref("whatsapp", "919900123456@s.whatsapp.net") returns ("919900123456@s.whatsapp.net", None, True)
  4. Verify _parse_target_ref("whatsapp", "+155****4567") still returns the E.164 number (no regression)
  5. Run full test file: pytest tests/tools/test_send_message_tool.py -v — all 133 tests pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: _parse_target_ref in tools/send_message_tool.py (callers: 2 in send_message_tool.py, 1 in test)
  • Blast radius: LOW — additive regex check, falls through to existing paths when no match
  • Related patterns: same structure as _FEISHU_TARGET_RE, _WEIXIN_TARGET_RE, _SLACK_TARGET_RE — platform-specific regex check before generic fallback

…e target resolution

WHATSAPP_JID_RE now matches WhatsApp JIDs (digits@lid and
digits@s.whatsapp.net) as explicit targets in _parse_target_ref.
Previously these fell through to the generic numeric / channel-name
resolution paths, causing silent fallback to the home channel or
bridge crashes from unrecognised JID formats.

Fixes NousResearch#37906
…targets

Adds @g.us (group JIDs) and @c.us (individual JIDs) to the WhatsApp
JID regex in _parse_target_ref, alongside the existing @lid and
@s.whatsapp.net patterns. Without this, group targets like
120363123456@g.us fall through to channel-name resolution and either
error or silently route to the home channel (issue NousResearch#18646).
@liuhao1024

Copy link
Copy Markdown
Contributor Author

Updated this PR to also cover WhatsApp group JIDs (@g.us) and individual JIDs (@c.us), in addition to the existing @lid and @s.whatsapp.net patterns.

This now also fixes #18646 — where send_message with a WhatsApp group target (120363123456@g.us) falls through _parse_target_ref unrecognized, either erroring out or silently routing to the home channel.

New test coverage:

  • test_whatsapp_group_jid_is_explicit@g.us JIDs resolve as explicit targets
  • test_whatsapp_c_us_jid_is_explicit@c.us JIDs resolve as explicit targets

@teknium1 teknium1 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.

Thanks for extending coverage to WhatsApp JID variants. Main already landed the original @lid, @s.whatsapp.net, and @g.us behavior in ea49a79633d93202d8e495648b2586ee5a1fbecc; tools/send_message_tool.py:47-50 now has a broader matcher for those plus broadcast/newsletter JIDs.

Problems

  • Current main still omits @c.us from that matcher, so the added @c.us case remains valuable: _parse_target_ref() only treats a WhatsApp target as explicit when _WHATSAPP_JID_RE matches (tools/send_message_tool.py:590-595).
  • The old-base regex in this PR should not replace the current one: it would lose main's existing broadcast/newsletter support and case-insensitive matching (tools/send_message_tool.py:47-50).

Suggested changes

  • Salvage this as a narrow addition of c\.us to main's existing suffix alternatives, plus the corresponding regression test. gateway/whatsapp_identity.py:109-111 already preserves fully-qualified JIDs unchanged, and existing Cloud tests use 15551234567@c.us (tests/gateway/test_whatsapp_cloud.py:869-887).

Automated hermes-sweeper review.

# JIDs). The live WhatsAppAdapter and Baileys bridge both accept these
# natively, so _parse_target_ref should recognise them as explicit targets
# instead of letting them fall through to the generic numeric / channel-name
# resolution paths (which silently route to the home channel or crash).

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.

Current main already has a broader _WHATSAPP_JID_RE here. Salvage this as an added c\.us alternative on that matcher rather than replacing it, so existing broadcast/newsletter support and re.IGNORECASE are retained.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists platform/whatsapp WhatsApp Business adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

send_message tool fails for WhatsApp: @lid JIDs not recognized, home channel fallback, and raw phone numbers cause jidDecode error

3 participants