fix(chatwoot-adapter): auto-recover from deleted Chatwoot conversations

[i350]

Problem

When an operator deletes a Chatwoot conversation out-of-band, the adapter's cached mapping conv:<sessionId>:<chatId> → { conversationId: <deleted id> } becomes dangling. The next inbound WhatsApp message relays to the dead conversation id and gets a 404. Without this patch:

• Inbound: the message is enqueued for retry and re-posted to the same dead id forever (no fresh conversation is ever created).
• Own-send (message:sent): the message is silently dropped.

Solution

Detect the 404 mid-relay, drop the stale forward + reverse mapping, re-resolve through the existing ensureConversation path (reusing the Chatwoot contact when it still exists), and re-post the message into a fresh conversation.

What changes

File	Change
mapping-store.ts	Add unlinkByChatId and unlinkByConversationId helpers (delete forward key, scoped reverse key, legacy reverse key)
inbound.ts	Wrap relayMessage in try/catch; on 404, unlink stale mapping, call resolveConversation to rebuild, retry once. Second failure surfaces to the existing retry queue (5-attempt cap).
sent.ts	Symmetric recovery for the own-send path: on 404, unlink stale mapping, call ensureConversation directly (no resolveConversation on this path), retry once. At-most-once — logs and stops, no retry queue.
inbound.test.ts	Two new tests: successful 404 recovery, and double-404 propagating to retry queue
sent.test.ts	Two new tests: successful 404 recovery, and double-404 logging both inner and outer failure

Safety bounds

• Single 404 status only: all other errors (5xx, 422, network) propagate unchanged to existing paths.
• One rebuild per message: the structural bound (inner try/catch re-throws original error) means one handler invocation = at most one rebuild attempt. No infinite loop.
• No retry queue entry on recovery success: the 404 is resolved in-band. Only a failed rebuild surfaces to the retry queue (inbound) or logs (own-send).
• No TypeORM migration needed: store.link after recovery calls mappings.upsert which hits the existing branch and updates the row in place.

Tests

All 33 tests pass (15 sent + 18 inbound). New tests verify:

• Unlink calls target the correct keys (forward + scoped + legacy reverse)
• ensureConversation re-resolves the contact by JID (not createContact)
• One fresh conversation minted on recovery
• Double-404 produces two complementary log lines (inner: why rebuild failed, outer: overall failure)
• Inbound double-404 surfaces to retry queue with the original error

[rmyndharis]

Requested changes — thanks for tackling this, @i350. The problem is real, the write-up is one of the clearest I've seen here, and the overall approach is the right one. There are just a few gaps to close before this can merge.

Verified against main: the dangling-mapping scenario plays out exactly as described. relayInbound resolves the cached conversation id, the dead id 404s, the message cycles through the retry queue against the same stale id and is dead-lettered after 5 attempts — and every subsequent message from that chat repeats the cycle. Own-send drops the message with only a log line. No self-healing exists today, so this fix is genuinely needed. Detecting the 404 mid-relay and rebuilding through ensureConversation is a sound, minimal approach — better than relying on a conversation_deleted webhook, which would be fragile and more intrusive. The safety bounds (404-only, one rebuild per message, second failure falls through to existing paths) are well thought out.

That said, a few things need attention:

1. npm run typecheck fails. Both new tests use log: logCalls.push.bind(logCalls) — Array.push returns number and takes string[], which doesn't satisfy (m: string, e?: unknown) => void. TS2322 at inbound.test.ts:309 and sent.test.ts:277. This one is red on CI, so it's a blocker.

2. Media messages aren't covered. postMedia (chatwoot-client.ts:154) throws new Error('Chatwoot postMedia -> ${res.status}') without setting err.status, so a 404 on a photo/voice/document relay never triggers the recovery — it still dead-letters. It's a one-line fix in the client, and without it the coverage doesn't quite match what the PR body promises.

3. The @lid-migrated case doesn't recover on inbound. The mapping can live under the canonical key (@c.us) while msg.chatId is @lid (the dual-lookup in resolveConversation). The recovery only calls unlinkByChatId(sessionId, msg.chatId), which deletes nothing in that case — resolveConversation then re-finds the same stale row via the canonical key, posts to the dead id again, and the second 404 sends the message off to be dead-lettered. Given how much of this codebase deals with the @lid migration (#609, #615), this one is worth handling rather than documenting away. foundKey is already available from the dual-lookup — unlink the key the mapping actually lives under (or both keys).

4. Minor: recovery also runs inside the retry drain (drain calls relayInbound), so in the catastrophic double-404 scenario each drain attempt can mint a fresh orphan conversation — bounded at 5, but you may want to skip the rebuild when invoked from the drain.

One small correction to the PR body: without the patch the message isn't re-posted "forever" — it's retried 5 times and dead-lettered. The practical impact is as you describe (the chat is permanently broken for all subsequent messages), but it's worth stating the mechanism precisely.

The tests themselves are genuinely good — seeding the mapping in the same backing map the unlink helpers delete against exercises the real semantics, not just the mocks. Once the typecheck errors and the unlink-key issue are fixed (and ideally the postMedia status), I'll be happy to merge this.

[rmyndharis]

Nice iteration — the foundKey refactor and carrying err.status through postMedia both look right, and the suite is green. Two things I'd resolve before merge:

1. recoverOn404 isn't wired into the drain path. relayInbound takes opts.recoverOn404 (default true) and the comment says the drain flips it to false, but the drain closure in index.ts never passes it:

(sessionId, _chatId, msg) => relayInbound(buildDeps(readConfig(ctx.config), sessionId), sessionId, msg),

So it stays true. If a queued message keeps 404-ing after a rebuild, each drain attempt (up to the retry cap) unlinks + rebuilds and mints a new orphan conversation instead of just burning the retry budget. Narrow case, but the guard is inert as written — either pass { recoverOn404: false } here, or drop the flag and its comment.

2. Own-send recovery still unlinks msg.chatId, not the found key. The foundKey fix landed in relayInbound but not handleSent:

await deps.store.unlinkByChatId(sessionId, msg.chatId);
conversationId = await ensureConversation(deps, sessionId, msg.chatId, { ... });

For a contact migrated to @lid, findMappedConversation matches via the canonical @c.us fallback, but the row lives under the @c.us forward key — so unlinking msg.chatId (@lid) is a no-op, the stale @c.us → deadConv row survives, and ensureConversation(@lid) → searchContact(@lid) misses the existing @c.us contact → duplicate contact + split conversation (the #31/#42 case). findMappedConversation should return the key it matched on (as resolveConversation now does) so the recovery unlinks the right one; reusing the mapping's contactId/sourceId before unlinking would avoid the contact lookup entirely.

A migrated-contact recovery test would lock both paths down — the current 404 tests all use a non-migrated key.

[rmyndharis]

Rebased this onto main so it can land. Your commits are preserved as one commit still authored by you; the only additional commit is mine and is described below.

Why it conflicted. chatwoot-adapter moved from 0.5.5 to 0.7.1 while this sat open, and resolveConversation changed shape on both sides. You added foundKey so the recovery path could unlink the key the mapping actually lives under; main added key for the same reason, alongside a backfill marker that replaced the old created flag. The two names were the same value, so reconciling them was mechanical: foundKey became key, and created is gone.

What that meant for your recovery path. You deliberately re-import history after a rebuild — a fresh Chatwoot thread without context defeats the point of backfill, as your comment says. That intent is unchanged, but the gate is now the durable marker rather than created: ensureConversation stamps backfillDone: false on a mapping it creates, so a rebuilt mapping satisfies the same condition the normal path uses. Both paths now call one shared helper so they cannot drift apart. Your @lid/@c.us unlink reasoning and the drain-path reasoning are untouched.

One defect found while rebasing. recoverOn404 was never actually passed as false anywhere — the drain call site in index.ts used the default, so the guard your comment describes was not wired up. On the drain path a dangling mapping would therefore rebuild on every attempt and mint a fresh conversation each time, up to the retry cap: exactly the divergence the option exists to prevent. That is fixed in the second commit, with a test that drives the real drain timer and fails without the wiring.

Suite is at 527 passing and typecheck is clean. The 404-recovery behaviour is covered by tests that were confirmed to fail when the guard is removed.