feat: Add errorPolicy support to WhatsApp channel#59801
Conversation
Adds errorPolicy and errorCooldownMs to WhatsApp channel config, matching the existing Telegram channel pattern. This allows operators to suppress or throttle error messages sent to WhatsApp users when agent runs fail. Closes openclaw#59782
- Tests for resolveWhatsAppErrorPolicy defaults and config resolution - Tests for isSilentErrorPolicy helper - Tests for buildWhatsAppErrorScopeKey - Tests for shouldSuppressWhatsAppError: - First occurrence not suppressed - Repeated errors suppressed within cooldown - Different error messages not cross-suppressed - Cooldown expiry allows errors through again - No leaking across accounts or chats Mirrors the existing Telegram error policy test suite.
openclaw-barnacle
Bot
added
docs
Greptile SummaryAdds Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/whatsapp/src/auto-reply/monitor/process-message.ts
Line: 430-435
Comment:
**`msg.accountId` vs `route.accountId` inconsistency**
The error policy config is resolved using `params.msg.accountId`, but the scope key passed to `shouldSuppressWhatsAppError` uses `params.route.accountId`. If `resolveAgentRoute` normalises or transforms the incoming account ID (e.g. aliases it), these two values can differ — causing the `once` cooldown to be recorded under a different key than the account whose config was read. The same pattern repeats in the `onError` block (lines 486–504). Consider reading the account once and using its `accountId` field for the scope key, or explicitly use `params.msg.accountId` for both.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/whatsapp/src/auto-reply/monitor/process-message.ts
Line: 429-455
Comment:
**Duplicate `resolveWhatsAppAccount` calls per message**
The account config is resolved independently inside both the `deliver` callback (line ~430) and the `onError` callback (line ~486). For every message that triggers an error, this causes two config lookups. Consider resolving the account config once before `createChannelReplyPipeline` and closing over it in both callbacks.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/whatsapp/src/error-policy.ts
Line: 29-31
Comment:
**Scope key separator can collide**
Using `:` as a separator means `accountId = "a:b"` + `chatId = "c"` produces the same key as `accountId = "a"` + `chatId = "b:c"`. If account IDs or chat IDs can contain colons (e.g. JID-style values), two different scopes would share a cooldown store. A simple mitigation is to escape or use a less common separator.
```suggestion
return `${params.accountId}\x00${params.chatId}`;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "test: Add unit tests for WhatsApp error ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const whatsAppAccount = resolveWhatsAppAccount({ | ||
| cfg: params.cfg, | ||
| accountId: params.msg.accountId, | ||
| }); | ||
| const errorPolicy = resolveWhatsAppErrorPolicy({ | ||
| accountConfig: whatsAppAccount, |
There was a problem hiding this comment.
msg.accountId vs route.accountId inconsistency
The error policy config is resolved using params.msg.accountId, but the scope key passed to shouldSuppressWhatsAppError uses params.route.accountId. If resolveAgentRoute normalises or transforms the incoming account ID (e.g. aliases it), these two values can differ — causing the once cooldown to be recorded under a different key than the account whose config was read. The same pattern repeats in the onError block (lines 486–504). Consider reading the account once and using its accountId field for the scope key, or explicitly use params.msg.accountId for both.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/whatsapp/src/auto-reply/monitor/process-message.ts
Line: 430-435
Comment:
**`msg.accountId` vs `route.accountId` inconsistency**
The error policy config is resolved using `params.msg.accountId`, but the scope key passed to `shouldSuppressWhatsAppError` uses `params.route.accountId`. If `resolveAgentRoute` normalises or transforms the incoming account ID (e.g. aliases it), these two values can differ — causing the `once` cooldown to be recorded under a different key than the account whose config was read. The same pattern repeats in the `onError` block (lines 486–504). Consider reading the account once and using its `accountId` field for the scope key, or explicitly use `params.msg.accountId` for both.
How can I resolve this? If you propose a fix, please make it concise.| if (payload.isError === true) { | ||
| const whatsAppAccount = resolveWhatsAppAccount({ | ||
| cfg: params.cfg, | ||
| accountId: params.msg.accountId, | ||
| }); | ||
| const errorPolicy = resolveWhatsAppErrorPolicy({ | ||
| accountConfig: whatsAppAccount, | ||
| }); | ||
| if (isSilentErrorPolicy(errorPolicy.policy)) { | ||
| logVerbose("Suppressed error reply (errorPolicy: silent)"); | ||
| return; | ||
| } | ||
| if ( | ||
| errorPolicy.policy === "once" && | ||
| shouldSuppressWhatsAppError({ | ||
| scopeKey: buildWhatsAppErrorScopeKey({ | ||
| accountId: params.route.accountId, | ||
| chatId: conversationId ?? params.msg.from, | ||
| }), | ||
| cooldownMs: errorPolicy.cooldownMs, | ||
| errorMessage: payload.text, | ||
| }) | ||
| ) { | ||
| logVerbose("Suppressed error reply (errorPolicy: once, within cooldown)"); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
resolveWhatsAppAccount calls per message
The account config is resolved independently inside both the deliver callback (line ~430) and the onError callback (line ~486). For every message that triggers an error, this causes two config lookups. Consider resolving the account config once before createChannelReplyPipeline and closing over it in both callbacks.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/whatsapp/src/auto-reply/monitor/process-message.ts
Line: 429-455
Comment:
**Duplicate `resolveWhatsAppAccount` calls per message**
The account config is resolved independently inside both the `deliver` callback (line ~430) and the `onError` callback (line ~486). For every message that triggers an error, this causes two config lookups. Consider resolving the account config once before `createChannelReplyPipeline` and closing over it in both callbacks.
How can I resolve this? If you propose a fix, please make it concise.| export function buildWhatsAppErrorScopeKey(params: { accountId: string; chatId: string }): string { | ||
| return `${params.accountId}:${params.chatId}`; | ||
| } |
There was a problem hiding this comment.
Using : as a separator means accountId = "a:b" + chatId = "c" produces the same key as accountId = "a" + chatId = "b:c". If account IDs or chat IDs can contain colons (e.g. JID-style values), two different scopes would share a cooldown store. A simple mitigation is to escape or use a less common separator.
| export function buildWhatsAppErrorScopeKey(params: { accountId: string; chatId: string }): string { | |
| return `${params.accountId}:${params.chatId}`; | |
| } | |
| return `${params.accountId}\x00${params.chatId}`; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/whatsapp/src/error-policy.ts
Line: 29-31
Comment:
**Scope key separator can collide**
Using `:` as a separator means `accountId = "a:b"` + `chatId = "c"` produces the same key as `accountId = "a"` + `chatId = "b:c"`. If account IDs or chat IDs can contain colons (e.g. JID-style values), two different scopes would share a cooldown store. A simple mitigation is to escape or use a less common separator.
```suggestion
return `${params.accountId}\x00${params.chatId}`;
```
How can I resolve this? If you propose a fix, please make it concise.![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: 90f76f3087
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| errorPolicy.policy === "once" && | ||
| shouldSuppressWhatsAppError({ | ||
| scopeKey: buildWhatsAppErrorScopeKey({ | ||
| accountId: params.route.accountId, | ||
| chatId: conversationId ?? params.msg.from, |
There was a problem hiding this comment.
shouldSuppressWhatsAppError is called before deliverWebReply, and that helper mutates cooldown state on the first attempt. If the first error reply fails to send (for example, transient WhatsApp send failure), the cooldown is still recorded, so subsequent error replies in the window are suppressed even though the user never received one. This breaks the documented "once per cooldown window" behavior in failure scenarios.
Useful? React with 👍 / 👎.
| if (isSilentErrorPolicy(errorPolicy.policy)) { | ||
| return; | ||
| } | ||
| if ( | ||
| errorPolicy.policy === "once" && |
There was a problem hiding this comment.
This early return applies errorPolicy to the dispatcher onError path, but onError is invoked for any exception thrown during delivery, including normal (non-error) final replies. With errorPolicy: "silent" (or repeated "once" matches), outbound delivery failures stop being logged entirely, which can hide real send outages and makes operational debugging much harder.
Useful? React with 👍 / 👎.
- Hoist resolveWhatsAppAccount + errorPolicy resolution before pipeline to avoid duplicate config lookups per message (greptile, codex) - Use params.msg.accountId consistently for scope key instead of mixing with params.route.accountId (greptile) - Use null byte separator in scope key to prevent collisions when accountId or chatId contains colons (greptile) - Remove errorPolicy suppression from onError callback — always log delivery failures regardless of policy to preserve operational visibility (codex) - Note: shouldSuppressWhatsAppError is intentionally called before deliverWebReply (not after) to match the existing Telegram pattern and avoid complexity of rollback on send failure
vincentkoc
added
the
triage: dirty-candidate
|
Thanks for the context here. I did a careful shell check against current Close as mostly implemented on main: current main and v2026.6.6 already suppress WhatsApp error payload text by default, while the remaining configurable always/once surface is stale, conflicting, and reopens the broader error-disclosure policy tracked elsewhere. So I’m closing this older PR as already covered on Review detailsBest possible solution: Keep the shipped WhatsApp error suppression and use the canonical open issue for any future opt-in, custom-message, or owner-only error policy instead of landing this stale branch. Do we have a high-confidence way to reproduce the issue? Not applicable as a feature PR. Source inspection shows current main already suppresses WhatsApp Is this the best way to solve the issue? No. The central suppression behavior is already shipped, and the branch's default- Security review: Security review needs attention: The diff can re-enable user-visible WhatsApp error text by default, which conflicts with the shipped suppression behavior and can expose provider/operator diagnostics to recipients.
AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 37e3e895b020; fix evidence: release v2026.6.6, commit 4cba08df01ea. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
clawsweeper
Bot
added
P2
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
This pull request has been automatically marked as stale due to inactivity. |
clawsweeper
Bot
added
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
the
status: 📣 needs proof
|
ClawSweeper applied the proposed close for this PR.
|
2 participants
Summary
Adds errorPolicy configuration support to WhatsApp channels, matching the existing functionality available in Telegram channels.
Changes
Motivation
WhatsApp channels currently have no way to suppress repetitive error messages during rate limits or other transient failures. Users can now configure:
Testing
Full unit test suite added (extensions/whatsapp/src/error-policy.test.ts) covering: