From 7fdb22881e607e0d6c150d83afcdc5763c73d80f Mon Sep 17 00:00:00 2001 From: rmyndharis Date: Fri, 3 Jul 2026 16:54:17 +0700 Subject: [PATCH] fix(chatwoot-adapter): harden history backfill from deep review (#609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep review of the v0.3.0 backfill surfaced defects fixed here (still pre-release, so folded into v0.3.0): - Add the engine:read permission — the sandboxed adapter could not call engine.getChatHistory/getChats without it, so backfill was rejected at the host gate. Bump minOpenWAVersion to 0.8.6 (the sandbox-bridged capability). - Bulk sweep now fetches history BEFORE creating a conversation and skips a chat with no fetchable history, so an engine without history support (Baileys) or an empty chat no longer floods Chatwoot with empty conversations. - Per-message isolation in replay: one failed post is logged and skipped instead of aborting the rest of the chat, and a message is marked seen only AFTER a successful post so a transient error stays retryable, not a silent drop. - Accept the string "true" for backfillAllOnce, not only the boolean. Historical location coordinates and quoted-message threading now render via the 0.8.6 engine enrichment (no adapter change needed). --- chatwoot-adapter/CHANGELOG.md | 4 +- chatwoot-adapter/README.md | 10 ++--- chatwoot-adapter/backfill.test.ts | 52 ++++++++++++++++++++------ chatwoot-adapter/backfill.ts | 62 ++++++++++++++++++++++--------- chatwoot-adapter/index.ts | 2 +- chatwoot-adapter/manifest.json | 6 +-- plugins.json | 4 +- 7 files changed, 98 insertions(+), 42 deletions(-) diff --git a/chatwoot-adapter/CHANGELOG.md b/chatwoot-adapter/CHANGELOG.md index 113fdd2..cf26dac 100644 --- a/chatwoot-adapter/CHANGELOG.md +++ b/chatwoot-adapter/CHANGELOG.md @@ -18,7 +18,9 @@ All notable changes to the Chatwoot Adapter plugin are documented here. The form - **Bulk (`backfillAllOnce`)** — a one-time sweep that imports the history of every existing chat on setup, for mirroring a whole inbox. Sequential, best-effort, runs once per session. - Business-side (`fromMe`) messages post as Chatwoot `outgoing`, contact messages as `incoming`. - - Requires OpenWA 0.8.5+ (the new `engine.getChatHistory` capability). + - Requires OpenWA 0.8.6+ (the `engine.getChatHistory` capability, bridged to sandboxed plugins) and the + `engine:read` permission. History that can't be fetched (e.g. the Baileys engine, which doesn't support + it, or a chat with no fetchable history) is skipped — the bulk sweep never creates empty conversations. ### Added diff --git a/chatwoot-adapter/README.md b/chatwoot-adapter/README.md index 8e4592b..d206097 100644 --- a/chatwoot-adapter/README.md +++ b/chatwoot-adapter/README.md @@ -17,7 +17,7 @@ mirror it. Runs sandboxed in the plugin worker; no server, no extra port. | **Author** | Yudhi Armyndharis | | **License** | MIT | | **Type** | `extension` | -| **Requires OpenWA** | ≥ 0.8.5 (tested 0.8.5) | +| **Requires OpenWA** | ≥ 0.8.6 (tested 0.8.6) | | **Keywords** | chatwoot, helpdesk, inbox, handover, two-way, agent, whatsapp, openwa | | **Repository** | [OpenWA-plugins/chatwoot-adapter](https://github.com/rmyndharis/OpenWA-plugins/tree/main/chatwoot-adapter) | @@ -66,7 +66,7 @@ sees it. | `inboxId` | number | yes | The API-channel inbox id this adapter posts into and relays replies from. | | `relayGroups` | boolean | no (default `true`) | Relay group chats (one synthetic contact per group, sender-prefixed). | | `relayMedia` | boolean | no (default `true`) | Upload inbound media to Chatwoot as attachments. | -| `backfillLimit` | number | no (default `0`) | When a chat first opens in Chatwoot, import this many recent messages (both directions, with media) so agents see prior context. `0` disables it; clamped to 100 host-side. Needs OpenWA 0.8.5+. | +| `backfillLimit` | number | no (default `0`) | When a chat first opens in Chatwoot, import this many recent messages (both directions, with media) so agents see prior context. `0` disables it; clamped to 100 host-side. Needs OpenWA 0.8.6+ and the whatsapp-web.js engine (Baileys has no history support). | | `backfillAllOnce` | boolean | no (default `false`) | Also run a one-time sweep importing every existing chat's history on setup. Needs `backfillLimit` > 0. Runs once per session. | The Chatwoot webhook **secret** and the instance's **session scope** are set when you mint the instance (they @@ -87,9 +87,9 @@ public host is added to the outbound allowlist). To use a self-hosted Chatwoot: ## Compatibility -- **OpenWA** ≥ 0.8.5 — needs Integration SDK v1 (webhook ingress, `ctx.mappings`, the session+chat handover - gate, `net.allowConfigHosts`), the `conversation.send` media/voice types for outbound attachments, and - `engine.getChatHistory` for the history backfill. +- **OpenWA** ≥ 0.8.6 — needs Integration SDK v1 (webhook ingress, `ctx.mappings`, the session+chat handover + gate, `net.allowConfigHosts`), the `conversation.send` media/voice types for outbound attachments, and the + sandbox-bridged `engine.getChatHistory` for the history backfill. - **Chatwoot** — account-level webhooks with timestamped HMAC signing (see Setup). ## Security diff --git a/chatwoot-adapter/backfill.test.ts b/chatwoot-adapter/backfill.test.ts index 86c0feb..0115100 100644 --- a/chatwoot-adapter/backfill.test.ts +++ b/chatwoot-adapter/backfill.test.ts @@ -6,23 +6,29 @@ import type { InboundDeps } from './relay.ts'; import type { IncomingMessage } from '../types/openwa'; // Capture what relayMessage posts by observing the client the shared relay calls (text path only here; -// media rendering is covered by the inbound tests). +// media rendering is covered by the inbound tests). `failOn` makes postText throw for a matching body. function makeDeps( over: { engine?: Record; store?: Record; relayGroups?: boolean; backfillLimit?: number; + failOn?: string; } = {}, ) { const posts: Array<{ conversationId: number; type: string; body: string }> = []; + const creates: string[] = []; const seen = new Set(); const client = { searchContact: async () => null, createContact: async () => ({ id: 9, sourceId: 'src' }), findOpenConversation: async () => null, - createConversation: async () => 55, + createConversation: async () => { + creates.push('c'); + return 55; + }, postText: async (conversationId: number, body: string, o: { messageType?: string }) => { + if (over.failOn && body.includes(over.failOn)) throw new Error('post failed'); posts.push({ conversationId, type: o?.messageType ?? 'incoming', body }); return { id: 1 }; }, @@ -51,18 +57,18 @@ function makeDeps( backfillAllOnce: false, log: () => {}, } as unknown as InboundDeps; - return { deps, posts }; + return { deps, posts, creates, seen }; } const hist = (id: string, ts: number, fromMe: boolean, body: string): IncomingMessage => ({ id, from: 'x', to: 'y', chatId: 'c@c.us', body, type: 'chat', timestamp: ts, fromMe, isGroup: false }) as IncomingMessage; -test('backfillHistory posts oldest→newest with fromMe as outgoing (#609)', async () => { +test('backfillHistory posts oldest->newest with fromMe as outgoing (#609)', async () => { const history = [hist('m3', 30, false, 'third'), hist('m1', 10, true, 'first'), hist('m2', 20, false, 'second')]; const { deps, posts } = makeDeps({ engine: { getChatHistory: async () => history } }); await backfillHistory(deps, 'sess', 'c@c.us', 55); assert.deepEqual(posts, [ - { conversationId: 55, type: 'outgoing', body: 'first' }, // fromMe, oldest first + { conversationId: 55, type: 'outgoing', body: 'first' }, { conversationId: 55, type: 'incoming', body: 'second' }, { conversationId: 55, type: 'incoming', body: 'third' }, ]); @@ -94,6 +100,18 @@ test('backfillHistory swallows a getChatHistory failure (best-effort)', async () assert.equal(posts.length, 0); }); +test('a failed history post is isolated and NOT marked seen; the rest still post (#609)', async () => { + const history = [hist('m1', 10, false, 'ok1'), hist('bad', 20, false, 'boom'), hist('m3', 30, false, 'ok2')]; + const { deps, posts, seen } = makeDeps({ engine: { getChatHistory: async () => history }, failOn: 'boom' }); + await backfillHistory(deps, 'sess', 'c@c.us', 55); + assert.deepEqual( + posts.map(p => p.body), + ['ok1', 'ok2'], // the failing message is skipped, the loop continues + ); + assert.equal(seen.has('bad'), false); // failed message left unmarked (retryable), not a silent drop + assert.equal(seen.has('m1'), true); +}); + test('backfillAllChats sweeps each chat once, skips groups when relayGroups is off, survives a failure (#609)', async () => { const chats = [ { id: 'a@c.us', name: 'A', isGroup: false, unreadCount: 0, timestamp: 1 }, @@ -106,7 +124,7 @@ test('backfillAllChats sweeps each chat once, skips groups when relayGroups is o }; let bulkDone = false; const { deps, posts } = makeDeps({ - relayGroups: false, // the group chat must be skipped + relayGroups: false, engine: { getChats: async () => chats, getChatHistory: async (_s: string, chatId: string) => historyByChat[chatId] ?? [], @@ -123,13 +141,23 @@ test('backfillAllChats sweeps each chat once, skips groups when relayGroups is o }, }); await backfillAllChats(deps, 'sessBulk'); - assert.deepEqual( - posts.map(p => p.body).sort(), - ['from A', 'from B'], // group skipped - ); + assert.deepEqual(posts.map(p => p.body).sort(), ['from A', 'from B']); assert.equal(bulkDone, true); - // second call is a no-op once the durable marker is set const before = posts.length; - await backfillAllChats(deps, 'sessBulk'); + await backfillAllChats(deps, 'sessBulk'); // marker set -> no-op assert.equal(posts.length, before); }); + +test('bulk creates NO empty conversation for a chat with no fetchable history (Baileys/empty) (#609)', async () => { + const { deps, posts, creates } = makeDeps({ + engine: { + getChats: async () => [{ id: 'empty@c.us', name: 'E', isGroup: false, unreadCount: 0, timestamp: 1 }], + getChatHistory: async () => { + throw new Error('unsupported'); // Baileys rejects; wwjs-empty returns [] — both -> skip + }, + }, + }); + await backfillAllChats(deps, 'sessEmpty'); + assert.equal(creates.length, 0); // ensureConversation/createConversation never called + assert.equal(posts.length, 0); +}); diff --git a/chatwoot-adapter/backfill.ts b/chatwoot-adapter/backfill.ts index 3d90285..0ade1c1 100644 --- a/chatwoot-adapter/backfill.ts +++ b/chatwoot-adapter/backfill.ts @@ -1,34 +1,58 @@ -import type { ChatSummary } from '../types/openwa'; +import type { ChatSummary, IncomingMessage } from '../types/openwa'; import { relayMessage, ensureConversation, type InboundDeps } from './relay.ts'; -// Replay a chat's recent history into its Chatwoot conversation: oldest→newest, both directions, deduped -// against the same markSeen store the live path uses (so a live message is never doubled). The caller -// holds the per-chat lock. Best-effort: any failure is logged, never thrown into the live relay. -export async function backfillHistory( +// Fetch a chat's recent history oldest->newest. Best-effort: any failure (including an engine that does +// not support history, e.g. Baileys, which rejects) yields an empty list so callers degrade cleanly. +async function fetchHistory(deps: InboundDeps, sessionId: string, chatId: string): Promise { + try { + const history = await deps.engine.getChatHistory(sessionId, chatId, deps.backfillLimit, true); + return [...history].sort((a, b) => a.timestamp - b.timestamp); + } catch (err) { + deps.log(`history fetch failed for ${chatId}`, err); + return []; + } +} + +// Replay ordered history into a Chatwoot conversation. Deduped against the same markSeen store the live +// path uses. Per-message isolation: one failed post is logged and skipped, never aborting the rest; the +// message is marked seen only AFTER a successful post so a transient error stays retryable rather than a +// silent drop. The caller holds the per-chat lock. +async function replayHistory( deps: InboundDeps, sessionId: string, - chatId: string, conversationId: number, + ordered: IncomingMessage[], ): Promise { - try { - const history = await deps.engine.getChatHistory(sessionId, chatId, deps.backfillLimit, true); - const ordered = [...history].sort((a, b) => a.timestamp - b.timestamp); - for (const msg of ordered) { - if (await deps.store.hasSeen('wa', msg.id, sessionId)) continue; - await deps.store.markSeen('wa', msg.id, sessionId); + for (const msg of ordered) { + if (await deps.store.hasSeen('wa', msg.id, sessionId)) continue; + try { await relayMessage(deps, conversationId, msg, msg.fromMe ? 'outgoing' : 'incoming'); + await deps.store.markSeen('wa', msg.id, sessionId); + } catch (err) { + deps.log(`history message ${msg.id} failed`, err); } - } catch (err) { - deps.log('history backfill failed', err); } } +// Lazy per-conversation backfill: fetch + replay this chat's history into its (already-created) +// conversation. Empty/unsupported history is a no-op. +export async function backfillHistory( + deps: InboundDeps, + sessionId: string, + chatId: string, + conversationId: number, +): Promise { + await replayHistory(deps, sessionId, conversationId, await fetchHistory(deps, sessionId, chatId)); +} + // In-memory guard so rapid successive inbounds can't launch the one-time sweep twice for a session. const bulkInFlight = new Set(); -// One-time bulk sweep (opt-in): create a Chatwoot conversation for every existing chat and backfill its -// history. Sequential — no parallel fan-out at Chatwoot — and best-effort: a per-chat failure never -// aborts the sweep. Runs once per session behind a durable marker plus the in-memory in-flight guard. +// One-time bulk sweep (opt-in): for every existing chat WITH history, create a Chatwoot conversation and +// backfill it. History is fetched BEFORE ensureConversation, so a chat with no fetchable history (or an +// engine without history support) never creates an empty conversation. Sequential and best-effort — a +// per-chat failure never aborts the sweep. Runs once per session behind a durable marker + the in-flight +// guard. export async function backfillAllChats(deps: InboundDeps, sessionId: string): Promise { // Add to the in-flight set BEFORE the first await, so a concurrent call from a rapid second inbound // sees it synchronously and bails — otherwise both could pass the durable-marker check and double-sweep. @@ -41,8 +65,10 @@ export async function backfillAllChats(deps: InboundDeps, sessionId: string): Pr if (chat.isGroup && !deps.relayGroups) continue; await deps.lock.run(`${sessionId}:${chat.id}`, async () => { try { + const ordered = await fetchHistory(deps, sessionId, chat.id); + if (!ordered.length) return; // nothing to import -> don't create an empty Chatwoot conversation const conversationId = await ensureConversation(deps, sessionId, chat.id, { name: chat.name || chat.id }); - await backfillHistory(deps, sessionId, chat.id, conversationId); + await replayHistory(deps, sessionId, conversationId, ordered); } catch (err) { deps.log(`bulk backfill failed for ${chat.id}`, err); } diff --git a/chatwoot-adapter/index.ts b/chatwoot-adapter/index.ts index bed87a5..fe03176 100644 --- a/chatwoot-adapter/index.ts +++ b/chatwoot-adapter/index.ts @@ -50,7 +50,7 @@ function readConfig(raw: Record): ChatwootFullConfig { relayGroups: raw.relayGroups !== false, relayMedia: raw.relayMedia !== false, backfillLimit: Number.isFinite(rawLimit) ? Math.max(0, Math.trunc(rawLimit)) : 0, - backfillAllOnce: raw.backfillAllOnce === true, + backfillAllOnce: raw.backfillAllOnce === true || raw.backfillAllOnce === 'true', }; } diff --git a/chatwoot-adapter/manifest.json b/chatwoot-adapter/manifest.json index a80a500..aa8571e 100644 --- a/chatwoot-adapter/manifest.json +++ b/chatwoot-adapter/manifest.json @@ -11,10 +11,10 @@ "repository": "https://github.com/rmyndharis/OpenWA-plugins", "keywords": ["chatwoot", "helpdesk", "inbox", "handover", "two-way", "agent", "whatsapp", "openwa"], "status": "beta", - "minOpenWAVersion": "0.8.5", - "testedOpenWAVersion": "0.8.5", + "minOpenWAVersion": "0.8.6", + "testedOpenWAVersion": "0.8.6", "sdkVersion": "1", - "permissions": ["net:fetch", "conversation:send", "webhook:ingress"], + "permissions": ["net:fetch", "conversation:send", "webhook:ingress", "engine:read"], "net": { "allow": [], "allowConfigHosts": ["baseUrl"] }, "sessionScoped": true, "sessions": ["*"], diff --git a/plugins.json b/plugins.json index 40de418..170899a 100644 --- a/plugins.json +++ b/plugins.json @@ -385,8 +385,8 @@ "whatsapp", "openwa" ], - "minOpenWAVersion": "0.8.5", - "testedOpenWAVersion": "0.8.5", + "minOpenWAVersion": "0.8.6", + "testedOpenWAVersion": "0.8.6", "releasedAt": "2026-07-03", "repoPath": "chatwoot-adapter", "repoUrl": "https://github.com/rmyndharis/OpenWA-plugins",