fix(chatwoot-adapter): stop the own-send mirror looping back to WhatsApp (0.5.4)

[rmyndharis]

Summary

With this adapter enabled, every outgoing WhatsApp message was delivered to the recipient twice — regardless of what composed it (the WhatsApp mobile app, a linked phone, or the OpenWA REST API). Found by live reproduction against a real recipient; disabling the plugin stopped the duplicates.

The loop

1. The account sends anything → WhatsApp emits a fromMe message:sent.
2. handleSent mirrors it into the Chatwoot thread via relayMessage(..., 'outgoing') — the own-send mirror feature.
3. Chatwoot fires message_created for that mirror: message_type: 'outgoing', private: false, our inbox.
4. shouldRelayOutbound only drops 'incoming' posts, so the mirror passes the filter.
5. outbound.relay's hasSeen('cw', id) finds no marker — nothing ever wrote one.
6. The mirror is sent to WhatsApp again → duplicate on the wire.

The reverse leg was already guarded: outbound.relay marks the WA id of every reply it sends (markSeen('wa', …)), so handleSent skips the adapter's own agent replies. Only the Chatwoot-side marker was missing, and relayMessage returned void, so the created id was not even available to mark.

The fix

relayMessage now marks the Chatwoot message it creates for an 'outgoing' post, which the existing hasSeen('cw', …) check recognises. The mark lands under the per-chat lock the mirror already holds, so it is written before the webhook can acquire the same key — the same discipline the 'wa' guard uses.

Two details worth review:

• Both scopes are marked. The two sides key differently: the mirror runs on a WA event (scope = the WA session), while the check runs on an ingress delivery scoped instance.sessionScope ?? undefined — undefined for an unscoped instance, which would read the unscoped key and miss a session-scoped marker. This mirrors the scoped + unscoped reverse-lookup pair MappingStore.link already writes, and for the same reason.
• The guard sits inside relayMessage, not at the call site. That also fixes the history backfill, which replays fromMe history as 'outgoing' through the same function — with backfillLimit > 0 it could have re-sent imported messages to the contact. Latent today (the default is 0), but the same defect.

De-duplication remains keyed on the Chatwoot message id, never on content or timestamp. The recipient genuinely received two messages; matching on content would have masked a real double-delivery rather than fixing one.

Testing

New echo-loop.test.ts drives both handlers over one real MappingStore and one shared lock, so the marker's actual storage key and session scope are exercised rather than stubbed:

Case	Before	After
Own send mirrored, session-scoped delivery	✖ re-sent	✔ suppressed
Own send mirrored, delivery with no session scope	✖ re-sent	✔ suppressed
Genuine agent reply from Chatwoot (control)	✔ relayed	✔ relayed

Both failure cases were confirmed red against the unfixed code before the change was written.

Full suite: 404/404 pass, tsc --noEmit clean, catalog regenerated.

[rmyndharis]

Live verification on the test server

0.5.4 deployed to the staging gateway (0.10.2, Baileys engine) and re-enabled, then one tagged message sent to a real recipient through POST /api/sessions/{id}/messages/send-text.

Result: the loop is closed.

Evidence	Observation
messages rows for the recipient in the window	1 — 3EB0AC0A344F4E56099A85. No duplicate.
ingress_events rows	1, 2.7s after the send
That delivery's payload	id: 60, message_type: "outgoing", private: false, inbox: {id: 1}, source_id: "3EB0AC0A344F4E56099A85"

The source_id on the inbound webhook is the WhatsApp id of our own send, which identifies that delivery unambiguously as the mirror. It satisfies every condition in shouldRelayOutbound, so before this change it would have been relayed straight back to WhatsApp. It was received and suppressed instead — the webhook path is demonstrably live, so this is not a green produced by an undelivered webhook.

Confirming the new code was the code running. GET /api/plugins still reports 0.5.3, because the version is cached in registry.json at install time — it is metadata, not evidence. So the check was behavioural instead: a census of the plugin's storage keys before the test showed 125 keys and zero unscoped seen:cw:* markers. Afterwards:

seen:7726f225-…:wa:3EB0AC0A344F4E56099A85   handleSent processed the own send
seen:7726f225-…:cw:60                        scoped marker
seen:cw:60                                   unscoped — written ONLY by markOutgoingEcho

The unscoped marker cannot be produced by 0.5.3 by any path.

Not verified live: the control case (a genuine agent reply still reaching WhatsApp). It is covered by unit test, but worth one manual reply from the Chatwoot UI before this is considered fully exercised.

One design question before merge

The unscoped marker is a global namespace keyed by the bare Chatwoot message id. On a multi-tenant install — two Chatwoot accounts against one gateway, with an unscoped instance — tenant B's agent reply could collide with tenant A's marker id and be suppressed. That would silently drop a genuine reply, which is a worse failure than the duplicate this PR fixes. It cannot occur on a single-tenant install, which is why the live test is unaffected.

There is a cleaner option: outbound.relay already resolves target.sessionId from conversation.id before the dedup check, so it could scope the check on target.sessionId rather than the delivery scope. That value is always defined, always matches what handleSent writes, and would let the unscoped marker be dropped entirely along with the collision risk — a no-op for session-scoped instances, and a strict improvement for unscoped ones.

It touches the existing outbound dedup path, so it deserves its own test rather than being folded in silently. Happy to do it here before merge, or as a follow-up.

[rmyndharis]

Follow-up applied: dedup is now scoped by the conversation's WA session

The unscoped marker raised above is gone. outbound.relay already resolves target.sessionId from the conversation mapping before it checks, so the dedup is scoped on that instead of on the delivery. It is always defined, and it is exactly the scope relayMessage marks the mirror under — so both halves of the guard now agree by construction, and there is no global namespace left to collide in.

Legacy unscoped markers are deliberately abandoned rather than read as a fallback. Honouring them would carry the same cross-tenant collision forward for their lifetime, and they are a 3-day cache rather than a ledger. Duplicate deliveries are already dropped upstream by the ingress providerDeliveryId dedup, so the exposure is one repeated reply, only for an unscoped instance, and only if Chatwoot re-announces an already-relayed message under a new delivery id during the upgrade window. That fails visibly and self-corrects, which is the safer direction than silently dropping a genuine agent reply. Documented as an upgrade note in the changelog.

New test: one tenant's mirror marker must not suppress another tenant's reply sharing a Chatwoot message id. The delivery in it is deliberately unscoped — a scoped delivery passes either way and would not have exercised the collision at all. Verified red against the previous commit, green here.

Suite: 405/405, tsc --noEmit clean.

Re-verified live

The build changed, so the earlier verification was re-run against the refined code (now on gateway 0.10.4, upgraded since the first run):

Evidence	Observation
messages rows for the recipient	1 — 3EB0D62F2A6BE24A331C03
ingress_events rows	1, arrived 0.1s later — webhook delivered, then suppressed
Marker written for the mirror (cw:62)	scoped yes, unscoped no
Unscoped seen:cw:* total	unchanged at 2, both pre-refinement leftovers

The last two rows are the point: the new build writes only the scoped marker, and the guard still holds.