Skip to content

fix(gateway): bypass WhatsApp batching for slash commands - #36747

Open
wysie wants to merge 1 commit into
NousResearch:mainfrom
wysie:fix/whatsapp-command-batching
Open

fix(gateway): bypass WhatsApp batching for slash commands#36747
wysie wants to merge 1 commit into
NousResearch:mainfrom
wysie:fix/whatsapp-command-batching

Conversation

@wysie

@wysie wysie commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bypass WhatsApp text-debounce batching for slash commands such as /approve and /deny.
  • Clear any pending WhatsApp text batch before dispatching a slash command so /approve is not merged into prior normal text.
  • Add regression coverage for immediate slash-command dispatch and pending-batch discard.

Root cause

Recent WhatsApp text batching delayed every text event, including gateway control commands. If a user sent normal text and then /approve inside the debounce window, the adapter merged them into one non-command payload, so the approval handler never ran.

Test Plan

  • python -m pytest tests/gateway/test_whatsapp_text_batching.py -q

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/whatsapp WhatsApp Business adapter labels Jun 1, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

✅ Looks Good

  • Clean fix: Slash commands (/approve, /deny) now bypass WhatsApp text-debounce batching, preventing them from being merged into prior normal text
  • Good root cause analysis: PR body clearly explains that batching delayed every text event, including control commands
  • Proper handling: Pending batches are cancelled before dispatching the command
  • Tests: Two well-structured regression tests cover both immediate dispatch and pending-batch discard scenarios
  • Minimal surface: Only 8 lines added to whatsapp.py + 38 lines of tests

Reviewed by Hermes Agent

@mxnstrexgl mxnstrexgl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated PR Review

Security Scan

  • ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.

Code Quality

  • ✓ No blocking code-quality issues found by this automated scan.

Summary

Status: APPROVE — security findings: 0, quality suggestions: 0.

Automated review; raw diff content intentionally omitted.

@ardaaltinors

Copy link
Copy Markdown

One small edge case from #46331 that might be worth carrying over here: using (event.text or "").strip().startswith("/") instead of event.is_command() would also catch commands with accidental leading whitespace.

Not a blocker for the main bug — #36747 covers the real failure mode we hit — just wanted to leave the note here while consolidating.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The underlying WhatsApp issue is still present on current main.

Problems

  • Current plugins/platforms/whatsapp/adapter.py:1254-1255 routes every MessageType.TEXT event into _enqueue_text_event; :1291-1308 then merges it into a pending batch and delays dispatch. A slash command can therefore still be combined with preceding normal text.
  • The patch no longer applies at its original path: commit 5600105478ffde29d7566b45421b100eaa29c4ef migrated WhatsApp to plugins/platforms/whatsapp/adapter.py, and GitHub reports this PR as conflicting.

Suggested changes

  • Salvage the guard into plugins/platforms/whatsapp/adapter.py directly after the batch key is computed, then retain the two proposed tests in tests/gateway/test_whatsapp_text_batching.py.

Automated hermes-sweeper review.

@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 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery 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.

6 participants