fix(chatwoot): resolve instanceId before looking up reaction targets - #2731
Closed
ticczaleski wants to merge 9 commits into
Closed
ticczaleski wants to merge 9 commits into
ticczaleski wants to merge 9 commits into
Conversation
…lures as private notes
The Chatwoot -> WhatsApp send path in receiveWebhook() had no
deduplication: a retried/duplicated Chatwoot message_created webhook
would call waInstance.textMessage/sendAttachment a second time for
the same message, and a blind unconditional 500ms sleep at the top
of the handler masked (without fixing) any race it was meant to
avoid, at the cost of latency on every webhook.
- Remove the unconditional 500ms sleep.
- Add ChatwootDeliveryService.claim(instanceName, chatwootMessageId,
operation), an atomic idempotency claim keyed on the tuple the
webhook carries, so a duplicate delivery is acknowledged without
re-sending to WhatsApp. Backed by a new ICache.setNX (SET NX EX in
Redis; synchronous check-and-set in the in-process LocalCache,
which is race-free because no await separates the check from the
set). CacheService.setNX fails open (claim succeeds) when caching
is disabled, so behavior is unchanged for deployments without a
cache configured.
- Replace onSendMessageError's private-note creation with
ChatwootDeliveryService.reportFailure, which calls Chatwoot's
existing authenticated message update endpoint (PATCH
.../messages/:id with { status: 'failed', external_error }) so the
original message becomes retryable instead of spawning a second,
unrelated message in the thread.
Also introduces vitest (the "test" script previously pointed at a
non-existent test/all.test.ts) with 10 passing unit tests covering
the new idempotency claim and failure-reporting behavior, plus the
underlying cache primitives. Full end-to-end coverage of
receiveWebhook() itself is out of scope for this change given the
size of that method and the absence of any prior test harness for it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…bound WhatsApp keys
After WhatsApp accepts an outbound send, Evolution only recorded the
WhatsApp key on its own local DB row (updateChatwootMessageId) — it
never told Chatwoot, so Chatwoot's source_id for agent-sent messages
stayed empty forever. That broke quoting an outgoing message from the
WhatsApp side, since Chatwoot's InReplyToMessageBuilder resolves
parents by source_id.
- Add ChatwootDeliveryService.registerExternalId: after a successful
textMessage/sendAttachment, calls Chatwoot's existing authenticated
message update endpoint with { source_id: 'WAID:' + key.id } —
matching the same prefix convention already used for inbound
messages, so the reply-matching and echo-guard logic stay
consistent. Never writes to Chatwoot's database directly.
- getQuotedMessage now prefers content_attributes.in_reply_to_external_id
(identifies the parent directly, on either side of the conversation)
and falls back to Evolution's own chatwootMessageId mapping only
when it is absent, per the plan: Evolution's local mapping only
ever covers messages Evolution itself sent.
Depends on the ChatwootDeliveryService introduced in
fix/chatwoot-idempotent-delivery (PR #1) — this branch is stacked on
top of it.
Companion Chatwoot-side change (same repo family, chatwoot fork):
Api::V1::Accounts::Conversations::MessagesController#update now
accepts source_id (API inboxes only, unique per inbox_id, validated
at the model level with a matching partial unique index), which is
what this PR's registerExternalId call relies on.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d-replies fix(chatwoot): resolve quoted replies by external id and register outbound WhatsApp keys
fix(chatwoot): make outbound delivery idempotent and stop masking failures as private notes
Two problems this closes:
1. Inbound: a WhatsApp contact's reaction was turned into a plain
Chatwoot text message whose content was just the emoji
(chatwoot.service.ts, the reactionMessage branch of eventWhatsapp).
That inflated unread counts, changed the conversation's last
message, could trigger automations, and could never be removed
when the contact un-reacted (empty text was silently dropped).
2. Outbound: nothing relayed an agent's reaction (created via
Chatwoot's new PUT .../messages/:id/reaction endpoint) to WhatsApp
at all.
Changes:
- receiveWebhook now branches on the three new Chatwoot events
(message_reaction_created/updated/deleted) before the existing
message-shaped filtering, since their payload has no `conversation`
object. handleReactionWebhook only relays actor_type: 'User'
(agent) reactions to WhatsApp via waInstance.reactionMessage —
actor_type: 'Contact' reactions already came from WhatsApp, so
relaying them back would echo. Deletion sends an empty reaction
string (Baileys' native "remove reaction" signal). Deduped via the
same ChatwootDeliveryService.claim(...) used for the outbound
message-send path, keyed by (instance, chatwootMessageId, event).
An unresolvable target (unknown source_id, or the target message
hasn't finished sending yet) is logged and acknowledged rather than
retried.
- The reactionMessage branch of eventWhatsapp now calls
handleInboundContactReaction, which resolves the target message's
chatwootMessageId/chatwootConversationId via the existing
getMessageByKeyId lookup and PUTs Chatwoot's reaction endpoint
directly (message_type: 'incoming' attributes it to the contact),
instead of createMessage(...reactionMessage.text...). An unknown
parent is logged and acknowledged, never retried. createMessage is
never called on this path.
Depends on Chatwoot's new reaction endpoint and the
Channel::Api#provider_capability?('reactions') gate (already merged
in the chatwoot fork) — Chatwoot only sends these events to inboxes
that declared the capability, so no capability check was needed here.
9 new unit tests on ChatwootService's two new methods (constructed
with mocked constructor dependencies, matching the existing
ChatwootDeliveryService test style since receiveWebhook itself has
no test harness).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
feat(chatwoot): synchronize WhatsApp reactions without fake messages
The three inherited Docker publish workflows all targeted evoapicloud/evolution-api — the upstream project's own Docker Hub namespace. This fork has no credentials for (and no rights to push to) that namespace, so every run would fail at push, and in practice none had ever run successfully (zero recorded workflow runs on this fork before this change). - Point all three workflows at ticczaleski/evolution-api instead, reusing the same DOCKER_USERNAME/DOCKER_PASSWORD secret names already configured the same way on the ticczaleski/chatwoot fork. - publish_docker_image_latest.yml (push to main) now mirrors chatwoot's docker-build.yml pattern: concurrency group per ref, a workflow_dispatch custom_tag input, and metadata-action tags for latest + the package.json version + a short commit sha, instead of a single hardcoded "latest" tag. This is CI-only; no application behavior changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…pace ci(docker): publish images to this fork's own Docker Hub namespace
The HTTP webhook route builds `instance` from the URL's :instanceName
param alone (ChatwootRouter -> RouterBroker#dataValidate never sets
instanceId), same as the message_created path which patches it in via
`instance.instanceId = waInstance.instanceId` right after resolving the
running instance. handleReactionWebhook was missing that same patch, so
getMessageByKeyId's `instanceId = ${instance.instanceId}` predicate was
always comparing against undefined and silently matched no rows -
every agent reaction sent from the dashboard was acknowledged as
"could not resolve WhatsApp key" and never relayed to WhatsApp, even
though the target message existed in the local Message table.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Reviewer's GuideThe PR fixes the production Chatwoot-to-WhatsApp reaction lookup by populating Sequence diagram for Chatwoot agent reaction deliverysequenceDiagram
participant Chatwoot
participant ChatwootService
participant waInstance as WhatsAppInstance
participant MessageDB as MessageDatabase
Chatwoot->>ChatwootService: receiveWebhook(instance, body)
ChatwootService->>waInstance: resolve waInstances[instanceName]
ChatwootService->>ChatwootService: set instance.instanceId = waInstance.instanceId
ChatwootService->>MessageDB: getMessageByKeyId(instance, rawKeyId)
MessageDB-->>ChatwootService: targetMessage.key
ChatwootService->>waInstance: reactionMessage({key, reaction})
waInstance-->>ChatwootService: reaction delivered
Sequence diagram for deduplicated Chatwoot outbound deliverysequenceDiagram
participant Chatwoot
participant ChatwootService
participant Cache
participant WhatsApp
participant ChatwootAPI
Chatwoot->>ChatwootService: receiveWebhook(instance, body)
ChatwootService->>Cache: claim(instanceName, body.id, operation)
Cache-->>ChatwootService: true or false
alt claim accepted
ChatwootService->>WhatsApp: send message
WhatsApp-->>ChatwootService: message key
ChatwootService->>ChatwootAPI: registerExternalId(..., source_id)
else duplicate delivery
ChatwootService-->>Chatwoot: acknowledge webhook
end
Sequence diagram for WhatsApp reaction synchronization to ChatwootsequenceDiagram
participant WhatsApp
participant ChatwootService
participant MessageDB as MessageDatabase
participant ChatwootAPI
WhatsApp->>ChatwootService: messages.upsert reaction
ChatwootService->>MessageDB: getMessageByKeyId(instance, reactionMessage.key.id)
MessageDB-->>ChatwootService: Chatwoot message and conversation IDs
ChatwootService->>ChatwootAPI: PUT reaction endpoint
ChatwootAPI-->>ChatwootService: reaction updated
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Author
|
Opened by mistake against the wrong repo (targeted the upstream project instead of our internal fork). Closing; the real PR is in our fork. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. This changes production outbound messaging and reaction handling, including deduplication claims, WhatsApp sends, and persistent Chatwoot message updates; an error can send duplicate or missing messages and those external effects are not undone by reverting. It also changes Docker publishing to a different image repository, so a workflow mistake could disrupt the image consumed by deployments.
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
2 participants
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
ChatwootRouter's webhook route buildsinstancefrom the URL's:instanceNameparam alone (RouterBroker#dataValidatenever setsinstanceId). Themessage_createdpath patchesinstance.instanceId = waInstance.instanceIdright after resolving the running instance, buthandleReactionWebhooknever did — sogetMessageByKeyId'sWHERE "instanceId" = ${instance.instanceId}predicate always compared againstundefinedand matched nothing.Could not resolve WhatsApp key for chatwoot message X; acknowledging without retryand was silently dropped, even though the target message existed in Evolution's localMessagetable. Confirmed live: the reaction webhook fired correctly from Chatwoot (capability gate, event dispatch, delivery all healthy), reached Evolution, but failed the lookup — while the reverse direction (WhatsApp contact reaction -> Chatwoot) already worked since it runs through the in-processmessages.upserthandler, which does setinstanceIdcorrectly.instance.instanceIdfromwaInstance.instanceIdinhandleReactionWebhook, same as the sibling code path.Test plan
npx vitest run src/api/integrations/chatbot/chatwoot/services/chatwoot-reactions.spec.ts— 10/10 passing, including a new regression test that reproduces a bare{instanceName}instance (as the real webhook route produces) and assertsgetMessageByKeyIdis called with the resolvedinstanceId.eslint/tsc --noEmitvia pre-commit hook — clean.evolution-api-ti1) that before this fix, an agent reaction on a real message never reached WhatsApp; the code path was confirmed via live logs against message id 1246 which existed inMessagewith the correctkey.idbut was never found due to theinstanceIdmismatch.Summary by Sourcery
Improve Chatwoot delivery reliability and reaction synchronization across Chatwoot and WhatsApp.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Tests:
Chores: