diff --git a/chatwoot-adapter/chatwoot-client.test.ts b/chatwoot-adapter/chatwoot-client.test.ts index 5d44173..cd0d0cd 100644 --- a/chatwoot-adapter/chatwoot-client.test.ts +++ b/chatwoot-adapter/chatwoot-client.test.ts @@ -88,3 +88,15 @@ test('postMedia marks a voice note and threads it (is_voice_message + source_id assert.match(raw, /name="source_id"\r\n\r\nwa5/); assert.match(raw, /filename="voice.ogg"/); }); + +test('postMedia on a non-ok response rejects with err.status set (so the 404 mapping-rebuild path can branch on it)', async () => { + // Inbound/sent recovery branches on `err.status === 404` to rebuild the dangling conversation mapping. + // json() already attaches status; postMedia did NOT — a media 404 would silently dead-letter. This test + // pins the status-attaching behaviour in place. + const { fn } = fakeFetch({ 'POST /api/v1/accounts/3/conversations/55/messages': { status: 404, body: 'gone' } }); + const c = new ChatwootClient(fn, cfg); + await assert.rejects( + c.postMedia(55, 'hi', { filename: 'a.jpg', contentType: 'image/jpeg', data: new Uint8Array([1]) }), + (err: Error & { status?: number }) => err.status === 404, + ); +}); diff --git a/chatwoot-adapter/chatwoot-client.ts b/chatwoot-adapter/chatwoot-client.ts index a5b5220..e70b5cc 100644 --- a/chatwoot-adapter/chatwoot-client.ts +++ b/chatwoot-adapter/chatwoot-client.ts @@ -151,7 +151,14 @@ export class ChatwootClient { headers: this.headers({ 'Content-Type': `multipart/form-data; boundary=${boundary}` }), body, }); - if (!res.ok) throw new Error(`Chatwoot postMedia -> ${res.status}`); + if (!res.ok) { + // Mirror json(): attach err.status so the caller (inbound/sent recovery) can branch on the HTTP + // code (the 404 mapping-rebuild path needs this — the bare Error below would otherwise be invisible + // to the `err.status === 404` check and a media 404 would dead-letter instead of recover). + const e = new Error(`Chatwoot postMedia -> ${res.status}`) as Error & { status?: number }; + e.status = res.status; + throw e; + } return JSON.parse(res.body || '{}') as { id: number }; } diff --git a/chatwoot-adapter/inbound.test.ts b/chatwoot-adapter/inbound.test.ts index 016af23..a69b66d 100644 --- a/chatwoot-adapter/inbound.test.ts +++ b/chatwoot-adapter/inbound.test.ts @@ -16,6 +16,7 @@ function makeDeps( engine?: Record; backfillLimit?: number; onBackfillExhausted?: (chatId: string) => void; + log?: (m: string, e?: unknown) => void; } = {}, ) { let contacts = 0; @@ -39,6 +40,10 @@ function makeDeps( postMedia: async () => ({ id: 2 }), ...over.client, }; + // unlinkByChatId + unlinkByConversationId default to actual map deletes against the SAME backing `store` + // the default getByChat / link use, so a recovery path that calls one of them actually clears the + // cached row (matches the real PluginStorage's semantics). Tests that need to introspect the call can + // still override them. const mapping = { getByChat: async (s: string, c: string) => store.get(`${s}:${c}`) ?? null, getByConversation: async () => null, @@ -46,13 +51,15 @@ function makeDeps( hasSeen: async () => false, markSeen: async () => {}, patch: async () => {}, + unlinkByChatId: async (s: string, c: string) => void store.delete(`${s}:${c}`), + unlinkByConversationId: async () => {}, ...over.store, }; // Default: identity canonicalization (@lid resolution exercised explicitly below). const engine = { canonicalChatId: async (_s: string, c: string) => c, ...over.engine }; const d = { lock: new KeyedAsyncLock(), client, store: mapping, engine, instanceId: 'inst', - relayGroups: true, relayMedia: true, backfillLimit: over.backfillLimit ?? 0, log: () => {}, + relayGroups: true, relayMedia: true, backfillLimit: over.backfillLimit ?? 0, log: over.log ?? (() => {}), onInboundLost: (msgId: string) => void lost.push(msgId), onBackfillExhausted: over.onBackfillExhausted ?? (() => {}), } as unknown as InboundDeps; @@ -568,3 +575,99 @@ test('a marker write that fails costs at most a redundant re-import, never the l assert.deepEqual(attemptMarker.posts.map(p => p.body), ['live2'], 'the live message still relays'); assert.deepEqual(queuedAttempt, [], 'and is not demoted into the retry queue'); }); + +// ── 404 mid-relay recovery: a dangling mapping (Chatwoot conversation deleted out of band) self-heals ── + +test('a 404 on inbound relay mid-flight rebuilds the mapping and retries once (no retry-queue entry, no duplicate conversation)', async () => { + // Operator deleted the Chatwoot conversation out-of-band: the cached conv id is dead. The plugin must + // drop the stale mapping (forward + scoped + legacy reverse), re-resolve through ensureConversation, + // and re-post the message into a fresh conversation instead of looping the dead id forever. + const unlinksChat: string[] = []; + const unlinksConv: number[] = []; + const postedTo: number[] = []; + const enqueued: string[] = []; + // Seed a stale mapping under the chatId that `handleInbound` will look up (msg.chatId = '621@c.us'). + const seeded = new Map([ + ['sess:621@c.us', { conversationId: 191, contactId: 9, sourceId: 'src' }], + ]); + const { deps: d } = makeDeps({ + store: { + getByChat: async (s: string, c: string) => seeded.get(`${s}:${c}`) ?? null, + link: async (s: string, c: string, _i: string, l: unknown) => void seeded.set(`${s}:${c}`, l), + // The default unlinkByChatId in deps() clears the local map — match that semantics against the + // seeded map so the rebuild's getByChat actually misses. + unlinkByChatId: async (s: string, c: string) => { unlinksChat.push(c); seeded.delete(`${s}:${c}`); }, + unlinkByConversationId: async (_s: string, conv: number) => void unlinksConv.push(conv), + enqueueRetry: async (e: { msg: { id: string } }) => void enqueued.push(e.msg.id), + }, + client: { + searchContact: async () => ({ id: 9, sourceId: 'src' }), // contact already on Chatwoot — no createContact + findOpenConversation: async () => null, // the old conv is deleted; no orphan open conv + createConversation: async () => 192, + postText: async (id: number) => { + postedTo.push(id); + if (id === 191) { + const e = new Error('Chatwoot POST .../conversations/191/messages -> 404') as Error & { status?: number }; + e.status = 404; + throw e; + } + return { id: 1 }; + }, + }, + }); + await handleInbound(d, 'sess', 'Engine', msg); + assert.deepEqual(unlinksChat, ['621@c.us'], 'forward mapping dropped'); + assert.deepEqual(unlinksConv, [191], 'scoped + legacy reverse mappings dropped'); + assert.deepEqual(postedTo, [191, 192], 'first post 404s against the dead id; second post succeeds against the rebuilt one'); + assert.deepEqual(enqueued, [], 'no retry-queue entry — the 404 was recovered in-band'); + // The rebuild reuses the existing Chatwoot contact (searchContact hit) so no second createContact fires, + // and exactly one new conversation is minted for the contact. counts() counts both via the default + // createContact/createConversation wrappers on the CLIENT overrides above — when an override replaces + // them, the default counters stop incrementing, so we assert by the recorded calls instead. + assert.ok(seeded.has('sess:621@c.us'), 'the fresh link is written back under the same chatId'); + const fresh = seeded.get('sess:621@c.us') as { conversationId: number }; + assert.equal(fresh.conversationId, 192, 'mapping now points at the new conversation id'); +}); + +test('a 404 on the REBUILT inbound conversation propagates to the retry queue (no silent loop)', async () => { + // Catastrophic case: even the freshly-created conversation is already gone, or Chatwoot is fully down. + // The recovery must NOT swallow the second 404 on the rebuilt conv — the failure has to surface so the + // existing enqueueRetry + drainRetries (5-attempt cap) takes over. + const unlinksConv: number[] = []; + const postedTo: number[] = []; + const enqueued: string[] = []; + const logCalls: string[] = []; + // Seed the SAME stale mapping shape as the previous test. + const seeded = new Map([ + ['sess:621@c.us', { conversationId: 55, contactId: 9, sourceId: 'src' }], + ]); + const { deps: d } = makeDeps({ + store: { + getByChat: async (s: string, c: string) => seeded.get(`${s}:${c}`) ?? null, + link: async (s: string, c: string, _i: string, l: unknown) => void seeded.set(`${s}:${c}`, l), + unlinkByChatId: async (s: string, c: string) => { seeded.delete(`${s}:${c}`); }, + unlinkByConversationId: async (_s: string, conv: number) => void unlinksConv.push(conv), + enqueueRetry: async (e: { msg: { id: string } }) => void enqueued.push(e.msg.id), + }, + client: { + searchContact: async () => ({ id: 9, sourceId: 'src' }), + findOpenConversation: async () => null, + createConversation: async () => 200, + postText: async (id: number) => { + postedTo.push(id); + const e = new Error(`Chatwoot POST .../conversations/${id}/messages -> 404`) as Error & { status?: number }; + e.status = 404; + throw e; // BOTH the dead id AND the rebuilt id 404 — the recovery cannot succeed + }, + }, + log: (m: string) => { logCalls.push(m); }, + }); + await handleInbound(d, 'sess', 'Engine', { ...msg, id: 'm-recover-twice' }); + assert.deepEqual(unlinksConv, [55], 'still unlinks the dead conv mapping once'); + assert.deepEqual(postedTo, [55, 200], 'posts were attempted against the dead id and then the rebuilt one'); + assert.deepEqual(enqueued, ['m-recover-twice'], 'the original 404 surfaces to the retry queue (with the backoff cap)'); + assert.ok( + logCalls.some(l => /inbound 404-recovery failed/.test(l)), + 'logs the recovery failure so ops can see why the rebuild itself failed', + ); +}); diff --git a/chatwoot-adapter/inbound.ts b/chatwoot-adapter/inbound.ts index 12bfe8e..61d6c36 100644 --- a/chatwoot-adapter/inbound.ts +++ b/chatwoot-adapter/inbound.ts @@ -12,8 +12,17 @@ export type { InboundDeps }; export const MAX_BACKFILL_ATTEMPTS = 3; // The resolve + backfill + relay core, lock-free, that THROWS on failure. Shared by the live inbound -// handler and the retry drain (retry.ts) so a retried message follows the exact same path. -export async function relayInbound(deps: InboundDeps, sessionId: string, msg: IncomingMessage): Promise { +// handler and the retry drain (retry.ts) so a retried message follows the exact same path. `opts.recoverOn404` +// defaults TRUE for the live path; the retry drain (index.ts) flips it FALSE so drain attempts against a +// still-dangling mapping burn their retry-budget instead of minting a fresh orphan conversation per +// attempt — the next LIVE inbound recovers the chat. +export async function relayInbound( + deps: InboundDeps, + sessionId: string, + msg: IncomingMessage, + opts: { recoverOn404?: boolean } = {}, +): Promise { + const recoverOn404 = opts.recoverOn404 ?? true; // Best-effort @lid -> @c.us for the LOOKUP only (never the lock — see handleInbound). Raw-fallback: // canonicalChatId needs a live engine, but the retry drain re-relays messages whose WA session may be // offline (the relay is a Chatwoot post that doesn't need it), so a failure must not block the relay. @@ -37,17 +46,40 @@ export async function relayInbound(deps: InboundDeps, sessionId: string, msg: In // predates the marker — a chat an earlier release already handled, which must be left alone. The // tradeoff is deliberate: a chat whose import failed before the upgrade is never retried, which is // exactly what those installs do today; every chat from here on gets the durable retry. - const attempts = backfill.attempts ?? 0; - if (deps.backfillLimit > 0 && backfill.done === false && attempts < MAX_BACKFILL_ATTEMPTS) { - if (await backfillHistory(deps, sessionId, msg.chatId, conversationId)) { - await recordBackfill(deps, sessionId, key, { backfillDone: true }); - } else { - const next = attempts + 1; - await recordBackfill(deps, sessionId, key, { backfillAttempts: next }); - if (next >= MAX_BACKFILL_ATTEMPTS) deps.onBackfillExhausted(key); + await maybeBackfill(deps, sessionId, msg.chatId, conversationId, key, backfill); + try { + await relayMessage(deps, sessionId, conversationId, msg, 'incoming'); + } catch (err) { + // A 404 mid-relay means the CACHED conversation id is gone — almost always because an operator + // (or Chatwoot) deleted the conversation out-of-band, leaving the conv<->WA mapping dangling. Drop + // the stale forward + scoped + legacy reverse keys and rebuild via the same resolveConversation -> + // ensureConversation path; then retry once. A second failure (404 or anything) survives the rebuild + // and falls through to the regular enqueueRetry -> drainRetries pipeline with its 5-attempt cap, so + // a wedged Chatwoot or a misconfigured inbox cannot loop here. + if ((err as { status?: number } | null)?.status !== 404) throw err; + // Drain path: same 404, but the drain is already a re-attempt of a previously failed relay. Rebuilding + // here would mint a fresh orphan conversation per attempt (up to 5) and never converge — better to let + // the retry budget run out and let the NEXT live inbound self-heal the chat. + if (!recoverOn404) throw err; + const originalErr = err; + try { + // Unlink the key the mapping actually lived under (resolveConversation returns it), not msg.chatId. + // A migrated contact's mapping is often keyed under the canonical @c.us while msg.chatId is @lid — + // unlinking the raw key would miss entirely, resolveConversation would re-find the stale row via its + // canonical fallback, the second relay would 404 again, and the message would dead-letter. + await deps.store.unlinkByChatId(sessionId, key); + await deps.store.unlinkByConversationId(sessionId, conversationId); + const re = await resolveConversation(deps, sessionId, msg, canonical); + // The deleted Chatwoot conversation is gone, so its history is gone too. When the rebuild mints a + // brand-new conversation, re-import prior history under the same gate the happy path uses — a fresh + // Chatwoot thread without context defeats the point of (lazy) backfill. + await maybeBackfill(deps, sessionId, msg.chatId, re.conversationId, re.key, re.backfill); + await relayMessage(deps, sessionId, re.conversationId, msg, 'incoming'); + } catch (rebuildErr) { + deps.log('inbound 404-recovery failed; surfacing original 404 to retry queue', rebuildErr); + throw originalErr; } } - await relayMessage(deps, sessionId, conversationId, msg, 'incoming'); } // WhatsApp → Chatwoot. Filter, then run resolve+post under the per-chat lock so two near-simultaneous @@ -114,6 +146,28 @@ async function recordBackfill( } } +// The marker-driven backfill gate, shared by the live resolve and the 404-recovery rebuild: both hand this +// a resolveConversation result and it decides whether to import, records the outcome, and reports exhaustion +// — so the two call sites can't drift apart on when a chat is eligible for import. +async function maybeBackfill( + deps: InboundDeps, + sessionId: string, + chatId: string, + conversationId: number, + key: string, + backfill: { done?: boolean; attempts?: number }, +): Promise { + const attempts = backfill.attempts ?? 0; + if (!(deps.backfillLimit > 0 && backfill.done === false && attempts < MAX_BACKFILL_ATTEMPTS)) return; + if (await backfillHistory(deps, sessionId, chatId, conversationId)) { + await recordBackfill(deps, sessionId, key, { backfillDone: true }); + } else { + const next = attempts + 1; + await recordBackfill(deps, sessionId, key, { backfillAttempts: next }); + if (next >= MAX_BACKFILL_ATTEMPTS) deps.onBackfillExhausted(key); + } +} + // Resolves the Chatwoot conversation for this chat and reports where its mapping document lives, so the // caller patches the SAME document refreshContactName does, plus that document's backfill state. async function resolveConversation( @@ -125,7 +179,9 @@ async function resolveConversation( // Dual lookup (re-read inside the lock): the raw chatId finds a mapping keyed by @lid, the canonical // chatId finds one keyed by @c.us (a contact that has since migrated to @lid, when the lid resolves) — // so a migrated contact's inbound lands in its EXISTING conversation instead of splitting a duplicate. - // `foundKey` is the key the mapping actually lives under, so refreshContactName patches the right doc. + // `foundKey` is the key the mapping actually lives under, so refreshContactName patches the right doc + // AND the 404-recovery path knows which forward key to unlink (a migrated contact's stale row is under + // the canonical key, NOT msg.chatId — unlinking the raw id would miss it entirely). let existing = await deps.store.getByChat(sessionId, msg.chatId); let foundKey = msg.chatId; if (!existing && canonicalChatId !== msg.chatId) { diff --git a/chatwoot-adapter/index.test.ts b/chatwoot-adapter/index.test.ts index 652c4c5..53fc067 100644 --- a/chatwoot-adapter/index.test.ts +++ b/chatwoot-adapter/index.test.ts @@ -1,7 +1,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import ChatwootAdapter from './index.ts'; -import { MAX_PENDING_RETRIES } from './retry.ts'; +import { MAX_PENDING_RETRIES, RETRY_INTERVAL_MS } from './retry.ts'; import type { PluginContext } from '../types/openwa'; function fakeCtx(config: Record) { @@ -104,6 +104,61 @@ test('healthCheck is healthy with an empty queue; onDisable/onUnload run cleanly await adapter.onUnload(); }); +// The retry-drain timer re-relays a queued message via the SAME relayInbound a live inbound uses, but a +// dangling mapping's 404 must NOT trigger the unlink-and-rebuild recovery there: the drain is already a +// re-attempt of a previously failed relay, so rebuilding would mint a fresh orphan Chatwoot conversation on +// every one of its (up to 5) attempts instead of ever converging. That's what relayInbound's `recoverOn404` +// option exists to prevent — this pins that the drain call site actually passes it false. +test('retry drain leaves a dangling mapping alone on a 404 — no rebuild, the retry budget absorbs it', async (t) => { + const { ctx, storageMap } = fakeCtx(goodConfig); + // A stale mapping: Chatwoot conversation 55 is gone (operator deleted it out of band) but the mapping + // still points at it. + storageMap.set('conv:sess:c@wa', { conversationId: 55, contactId: 9, sourceId: 'src' }); + // Queued as if an earlier live relay against it already failed once. + storageMap.set('retry:sess:m1', { + sessionId: 'sess', + chatId: 'c@wa', + msg: { id: 'm1', from: 'x', to: 'y', chatId: 'c@wa', body: 'hi', type: 'chat', timestamp: 0, fromMe: false, isGroup: false }, + attempts: 0, + enqueuedAt: 1, + }); + // A minimal Chatwoot double: the stale conversation 404s; a contact search + fresh conversation create + // would succeed if the recovery path ran (it must not). + const calls: string[] = []; + ctx.net.fetch = (async (url: string, init?: { method?: string }) => { + const method = init?.method ?? 'GET'; + calls.push(`${method} ${url}`); + if (url.includes('/contacts/search')) { + return { + ok: true, status: 200, headers: {}, + body: JSON.stringify({ payload: [{ id: 9, identifier: 'c@wa', contact_inboxes: [{ inbox: { id: 7 }, source_id: 'src' }] }] }), + }; + } + if (/\/contacts\/\d+\/conversations$/.test(url)) return { ok: true, status: 200, headers: {}, body: JSON.stringify({ payload: [] }) }; + if (method === 'POST' && url.endsWith('/conversations')) return { ok: true, status: 200, headers: {}, body: JSON.stringify({ id: 200 }) }; + if (method === 'POST' && url.endsWith('/conversations/55/messages')) return { ok: false, status: 404, headers: {}, body: 'Not Found' }; + return { ok: true, status: 200, headers: {}, body: '{}' }; + }) as typeof ctx.net.fetch; + + // Fake the retry timer so the drain runs synchronously under test control instead of after a real 30s wait. + t.mock.timers.enable({ apis: ['setInterval'] }); + const adapter = new ChatwootAdapter(); + await adapter.onEnable(ctx); + t.mock.timers.tick(RETRY_INTERVAL_MS); + // The drain's own work (storage + the fetch double above) all settles on real promises; flush them. + await new Promise(res => setImmediate(res)); + await new Promise(res => setImmediate(res)); + + const mapping = storageMap.get('conv:sess:c@wa') as { conversationId: number } | undefined; + assert.equal(mapping?.conversationId, 55, 'the dangling mapping must be left exactly as it was — no rebuild'); + assert.ok(!calls.some(c => c.startsWith('POST') && c.endsWith('/conversations')), 'no fresh conversation is minted on the drain path'); + const retry = storageMap.get('retry:sess:m1') as { attempts: number } | undefined; + assert.ok(retry, 'the queued message is kept for the next drain, not treated as delivered'); + assert.equal(retry?.attempts, 1, 'the failed attempt bumps the retry counter — the budget absorbs the 404, nothing else does'); + + await adapter.onDisable(); +}); + test('onEnable throws on missing / invalid config', async () => { const { ctx } = fakeCtx({ baseUrl: 'https://x' }); // missing apiToken, accountId, inboxId await assert.rejects(new ChatwootAdapter().onEnable(ctx), /missing\/invalid config/); diff --git a/chatwoot-adapter/index.ts b/chatwoot-adapter/index.ts index b1859fb..162e993 100644 --- a/chatwoot-adapter/index.ts +++ b/chatwoot-adapter/index.ts @@ -187,7 +187,8 @@ export default class ChatwootAdapter implements IPlugin { this.draining = true; return drainRetries( { store, lock, log: (m, e) => ctx.logger.error(m, e) }, - (sessionId, _chatId, msg) => relayInbound(buildDeps(readConfig(ctx.config), sessionId), sessionId, msg), + (sessionId, _chatId, msg) => + relayInbound(buildDeps(readConfig(ctx.config), sessionId), sessionId, msg, { recoverOn404: false }), MAX_RETRY_ATTEMPTS, ) .then( diff --git a/chatwoot-adapter/mapping-store.ts b/chatwoot-adapter/mapping-store.ts index 9d540b8..03e11ef 100644 --- a/chatwoot-adapter/mapping-store.ts +++ b/chatwoot-adapter/mapping-store.ts @@ -302,4 +302,13 @@ export class MappingStore { async countRetries(): Promise { return (await this.retryKeys()).length; } + + async unlinkByChatId(sessionId: string, chatId: string) { + await this.storage.delete(this.fwdKey(sessionId, chatId)); + } + + async unlinkByConversationId(sessionId: string, conversationId: number) { + await this.storage.delete(this.revKey(sessionId, conversationId)); + await this.storage.delete(this.legacyRevKey(conversationId)); + } } diff --git a/chatwoot-adapter/sent.test.ts b/chatwoot-adapter/sent.test.ts index 45b753b..70815c9 100644 --- a/chatwoot-adapter/sent.test.ts +++ b/chatwoot-adapter/sent.test.ts @@ -18,6 +18,7 @@ function deps( engine?: Record; relayGroups?: boolean; relayMedia?: boolean; + log?: (m: string, e?: unknown) => void; } = {}, ) { let contacts = 0; @@ -46,7 +47,7 @@ function deps( const d = { lock: new KeyedAsyncLock(), client, store, engine, instanceId: 'inst', relayGroups: over.relayGroups ?? true, relayMedia: over.relayMedia ?? true, backfillLimit: 0, backfillAllOnce: false, - log: () => {}, + log: over.log ?? (() => {}), // Both callbacks are required on InboundDeps; the cast would hide an omitted one. onInboundLost: () => {}, onBackfillExhausted: () => {}, @@ -196,3 +197,99 @@ test('ignores a non-Engine source (defensive)', async () => { await handleSent(d, 'sess', 'API', own); assert.equal(posted.length, 0); }); + +test('a 404 on an own-send relay mid-flight rebuilds the mapping via ensureConversation and posts once', async () => { + // Operator deleted the Chatwoot conversation out-of-band: the cached conv id is dead. sent.ts's normal + // posture is "relay-into-existing-only, never create" (mirrors the lid-migration no-split invariant), + // but on a 404 specifically the dangling mapping has to be dropped and a fresh conversation must be + // minted so the phone-composed delivery isn't silently lost. Symmetrical to inbound.ts's recovery. + const unlinksChat: string[] = []; + const unlinksConv: number[] = []; + const posts: Array<{ id: number; type?: string }> = []; + const searchCalls: string[] = []; + let createConvCalls = 0; + // Seed the stale mapping in a closure-scoped map so unlinkByChatId actually clears it (matching what + // the real PluginStorage would do). Without this, a static getByChat stub would keep returning the + // dead row after unlink and ensureConversation would short-circuit. + const links = new Map([ + ['sess:621@c.us', { conversationId: 55, contactId: 9, sourceId: 'oldSrc', name: 'x' }], + ]); + const { deps: d } = deps({ + store: { + getByChat: async (_s: string, c: string) => links.get(`sess:${c}`) ?? null, + link: async (_s: string, c: string, _i: string, l: unknown) => void links.set(`sess:${c}`, l), + unlinkByChatId: async (_s: string, c: string) => { unlinksChat.push(c); links.delete(`sess:${c}`); }, + unlinkByConversationId: async (_s: string, conv: number) => void unlinksConv.push(conv), + }, + client: { + postText: async (id: number, _c: string, o?: { messageType?: string }) => { + posts.push({ id, type: o?.messageType }); + if (id === 55) { + const e = new Error(`Chatwoot POST .../conversations/${id}/messages -> 404`) as Error & { status?: number }; + e.status = 404; + throw e; + } + return { id: 1 }; + }, + searchContact: async (identifier: string) => { searchCalls.push(identifier); return { id: 9, sourceId: 'newSrc' }; }, + findOpenConversation: async () => null, // no orphan — rebuild mints a fresh one + createConversation: async () => { createConvCalls++; return 77; }, + }, + }); + await handleSent(d, 'sess', 'Engine', own); + assert.deepEqual(unlinksChat, ['621@c.us'], 'forward mapping dropped under the canonical key'); + assert.deepEqual(unlinksConv, [55], 'scoped + legacy reverse mappings dropped'); + assert.deepEqual(posts, [ + { id: 55, type: 'outgoing' }, + { id: 77, type: 'outgoing' }, + ], 'first post 404s against the dead id; second post succeeds against the rebuilt one'); + assert.deepEqual(searchCalls, ['621@c.us'], 'ensureConversation re-resolves the contact by JID'); + assert.equal(createConvCalls, 1, 'one fresh Chatwoot conversation minted (ensureConversation -> createConversation)'); +}); + +test('a 404 on the REBUILT own-send conversation logs failure (sent is at-most-once, no retry)', async () => { + // Catastrophic case: the freshly-created conversation is also gone / Chatwoot is wedged. sent.ts is + // at-most-once (unlike inbound, which enqueues for retry), so the failure must surface once on the log + // and STOP — never enqueue, never re-create the contact a second time. + const unlinksConv: number[] = []; + const logCalls: string[] = []; + let searchCalls = 0; + let createConvCalls = 0; + let postCalls = 0; + const links = new Map([ + ['sess:621@c.us', { conversationId: 55, contactId: 9, sourceId: 'x', name: 'x' }], + ]); + const { deps: d } = deps({ + store: { + getByChat: async (_s: string, c: string) => links.get(`sess:${c}`) ?? null, + link: async (_s: string, c: string, _i: string, l: unknown) => void links.set(`sess:${c}`, l), + unlinkByChatId: async (_s: string, c: string) => void links.delete(`sess:${c}`), + unlinkByConversationId: async (_s: string, conv: number) => void unlinksConv.push(conv), + }, + client: { + postText: async () => { + postCalls++; + const e = new Error('Chatwoot POST .../messages -> 404') as Error & { status?: number }; + e.status = 404; + throw e; + }, + searchContact: async () => { searchCalls++; return { id: 9, sourceId: 'src' }; }, + findOpenConversation: async () => null, + createConversation: async () => { createConvCalls++; return 200; }, + }, + log: (m: string) => { logCalls.push(m); }, + }); + await handleSent(d, 'sess', 'Engine', { ...own, id: 'o-double-404' }); + assert.deepEqual(unlinksConv, [55], 'still unlinks the dead conv mapping once'); + assert.equal(postCalls, 2, 'two posts attempted (one against the dead id, one against the rebuilt one — both 404)'); + assert.equal(searchCalls, 1, 'one rebuild attempt only — no second createContact'); + assert.equal(createConvCalls, 1, 'one rebuild attempt only — no second createConversation'); + assert.ok( + logCalls.some(l => /own-send 404-recovery failed/.test(l)), + 'logs WHY the rebuild itself failed (the second 404), with its own context', + ); + assert.ok( + logCalls.some(l => /own-send relay failed/.test(l)), + 'logs the unrecoverable own-send failure (sent does not retry)', + ); +}); diff --git a/chatwoot-adapter/sent.ts b/chatwoot-adapter/sent.ts index 93e5f18..409dd91 100644 --- a/chatwoot-adapter/sent.ts +++ b/chatwoot-adapter/sent.ts @@ -1,6 +1,6 @@ import type { IncomingMessage } from '../types/openwa'; import { shouldRelayOwn } from './filters.ts'; -import { relayMessage, type InboundDeps } from './relay.ts'; +import { relayMessage, ensureConversation, type InboundDeps } from './relay.ts'; // WhatsApp → Chatwoot for the account's OWN outbound sends — messages composed on a linked phone / the // WhatsApp mobile app / the OpenWA REST API — so the Chatwoot thread mirrors the full WhatsApp @@ -42,11 +42,43 @@ export async function handleSent( // the helpdesk is visible and harmless; a missing customer message is neither. const identifiable = Boolean(msg.id); if (identifiable && (await deps.store.hasSeen('wa', msg.id, sessionId))) return; - const conversationId = await findMappedConversation(deps, sessionId, msg, key); - if (conversationId === null) return; // unmapped chat — drop, never create (no split) + let conversationId = await findMappedConversation(deps, sessionId, msg, key); + if (conversationId === null) return; // unmapped chat — drop, never create outside a 404 recovery (no split) if (identifiable) await deps.store.markSeen('wa', msg.id, sessionId); else deps.log('own send has no engine message id; relaying it without de-duplication'); - await relayMessage(deps, sessionId, conversationId, msg, 'outgoing'); + try { + await relayMessage(deps, sessionId, conversationId, msg, 'outgoing'); + } catch (err) { + // Mirror of inbound.ts's 404 recovery: an operator-deleted Chatwoot conversation leaves the + // conv->WA mapping dangling. The mirror path uses findMappedConversation (never creates on a + // miss, see "relay-into-existing-only" above), so on a 404 we DROP the stale forward + reverse + // keys and call ensureConversation directly — find-or-create the contact, find-or-mint the + // conversation, link, then re-post. A failure inside the recovery is logged with its own + // context (so the log shows WHICH step failed) and the ORIGINAL 404 surfaces through the outer + // catch. sent.ts is at-most-once — unlike inbound, this path does NOT enqueue for retry + // (mirrors the existing posture). + if ((err as { status?: number } | null)?.status !== 404) throw err; + const originalErr = err; + try { + // Unlink by the RAW chatId (msg.chatId), not the canonical key. findMappedConversation checks + // msg.chatId first, so a stale row under the raw key would shadow the fresh one we're about + // to create. ensureConversation also links under msg.chatId, matching the original key. + await deps.store.unlinkByChatId(sessionId, msg.chatId); + await deps.store.unlinkByConversationId(sessionId, conversationId); + conversationId = await ensureConversation(deps, sessionId, msg.chatId, { + name: + msg.contact?.pushName || + msg.contact?.name || + msg.senderPhone || + (msg.isGroup ? `Group ${msg.chatId}` : msg.chatId), + phone: msg.isGroup ? undefined : msg.senderPhone ?? undefined, + }); + await relayMessage(deps, sessionId, conversationId, msg, 'outgoing'); + } catch (rebuildErr) { + deps.log('own-send 404-recovery failed', rebuildErr); + throw originalErr; + } + } } catch (err) { deps.log('own-send relay failed', err); }