Engine pluggability: decouple whatsapp-web.js specifics from the engine-neutral layer

[rmyndharis]

Context

OpenWA has a pluggable engine architecture — the seam is IWhatsAppEngine (neutral types), and only the adapter layer (src/engine/adapters/*, src/plugins/engines/whatsapp-web-js/*) should know whatsapp-web.js. A code audit found several places outside that seam that assume whatsapp-web.js specifics and would break (or silently misbehave) under a different engine (e.g. Baileys, which uses @s.whatsapp.net JIDs and a different ack/presence/type model).

PR #264 already addressed the lightweight items (engine-neutral typing via sendChatState, mark-chat-read JID validation relaxed, real engine library version surfaced). This issue tracks the remaining, larger decouplings — each is its own PR to keep regression surface contained.

Findings (ranked)

🔴 High — delivery ack scale leaks whatsapp-web.js integers end-to-end

The raw wwebjs MessageAck integer (-1..4) flows through the neutral interface (MessageResult.ack, onMessageAck(id, ack: number)), is interpreted in message-status.util.ts (ackToMessageStatus) and session.service.ts (ack < 0 → message.failed), embedded in webhook payloads + idempotency keys, re-emitted by events.gateway.ts, and re-mapped again in the dashboard (Chats.tsx statusMap, useWebSocket.ts). Baileys uses a different/offset integer scale → every delivery/read state would mis-map.

• Fix: introduce a neutral DeliveryStatus enum (pending|sent|delivered|read|played|failed) at the seam; each adapter maps its native codes; services/webhook/ws/dashboard consume the neutral string.

🔴 High — IncomingMessage.type is the raw wwebjs MessageTypes string

message-mapper.ts assigns type: msg.type (chat/ptt/voice/revoked/…) directly; the dashboard switches on those tokens (Chats.tsx media render, 'ptt'/'voice', and a latent 'chat' vs 'text' mismatch to verify).

• Fix: normalize to a neutral MessageType union/enum at the adapter boundary ('chat' → 'text', 'ptt'/'voice' → 'voice', …); consumers switch on the neutral type.

🟠 Medium — JID construction outside the adapter

• contact.controller.ts builds ${number}@c.us in the check-number response.
• dashboard/MessageTester.tsx builds …@c.us client-side.
• session.service.ts hardcodes status@broadcast.
• Fix: let the engine canonicalize numbers → JIDs (checkNumberExists returns the resolved id); surface an isStatusBroadcast/chatType flag instead of string-matching.

🟠 Medium — Puppeteer/browser config baked into the neutral engine contract

infra.controller.ts, EngineCreateOptions, and plugin.createEngine pass headless/puppeteerArgs/executablePath to every engine — meaningless for a non-browser engine.

• Fix: pass only neutral fields generically; deliver engine-specific config via each plugin's own (declared) config schema (opaque bag).

🟠 Medium — EngineFactory hard-registers only the wwebjs plugin

registerBuiltInEngines() registers a single engine and create() resolves by type, but there's no real registry for a second engine.

• Fix: a registry (Map<engineType, IEnginePlugin>) seeded by each built-in engine module, selectable by config.

Notes

• Audit verified: there are zero raw import 'whatsapp-web.js' statements outside the adapter layer — the leaks are semantic (values/shapes/JIDs), not direct imports.
• Swagger example strings using @c.us are documentation-only (low priority; genericize opportunistically).

[rmyndharis]

Item 1 done → #268 (open): the raw whatsapp-web.js ack integer no longer leaks past the engine adapter. Neutral DeliveryStatus flows through the interface/services/webhooks/ws/dashboard; message.ack/message.failed gain a neutral status (legacy ack kept, deprecated); dashboard delivery ticks now update live over ws (that path was dead). Behaviour byte-identical; 445 tests; adversarially reviewed.

Remaining:

• MessageTypes → neutral message-type enum (mapper + interface + dashboard; incl. the latent chat-vs-text issue)
• JID canonicalization via engine (contact.controller / MessageTester @c.us; status@broadcast flag)
• Engine registry (register/select a 2nd engine)
• Opaque per-engine config (Puppeteer out of the neutral contract)

[rmyndharis]

Item 2 done → #270 (open): raw whatsapp-web.js MessageTypes tokens no longer leak past the adapter. Neutral MessageType enum on IncomingMessage.type; mapWwebjsMessageType is the sole translation point (chat→text, ptt→voice, vcard→contact, …). Incoming live/history/persisted + message.received/message.sent webhooks are now neutral and consistent with outgoing. Includes a backfill migration (the dashboard reads persisted rows via GET /messages, and stats group by type — so old rows must be normalized) and fixes the latent chat-vs-text dashboard bug. 449 tests; migration SQL verified; adversarially reviewed.

Remaining:

• JID canonicalization via engine (contact.controller / MessageTester @c.us; status@broadcast flag)
• Engine registry (register/select a 2nd engine)
• Opaque per-engine config (Puppeteer out of the neutral contract)

[rmyndharis]

✅ Item 3 (Medium — JID construction outside the adapter) → PR #271

Two of the three sub-items shipped:

• contact.controller.ts ${number}@c.us — removed. Added IWhatsAppEngine.getNumberId(number); the controller now returns the engine-resolved canonical whatsappId. ⚠️ Observable change: the value is the engine's normalized id and may differ from the input (e.g. @lid) — live-verified, documented in CHANGELOG + docs/06/07.
• session.service.ts status@broadcast — replaced with a neutral isStatusBroadcast flag set by the message-mapper. The literal is now confined to the adapter.

Still deferred (this sub-item): dashboard/.../MessageTester.tsx builds …@c.us client-side. It needs a dashboard checkNumber API call + an i18n error path, so it's a separate dashboard-side follow-up.

Adversarial review verdict: holds (getNumberId-contract) / exact equivalence (status flag). 439 tests, build + lint clean.

---

Roadmap status:

• 🔴 ack scale → DeliveryStatus — ✅ refactor(engine): neutral DeliveryStatus instead of raw whatsapp-web.js ack int (#265) #268
• 🔴 IncomingMessage.type → neutral MessageType — ✅ refactor(engine): neutral MessageType instead of raw whatsapp-web.js type tokens (#265) #270
• 🟠 JID construction — ✅ refactor(engine): move JID construction into the engine (#265) #271 (MessageTester deferred)
• 🟠 Puppeteer config in neutral contract — ⏳ next
• 🟠 EngineFactory registry — ⏳ next

[rmyndharis]

Deep review of the three open decoupling PRs — done; all green

Ran a source-traced adversarial review (incl. a real 3-way merge simulation) across #268, #270, #271. Outcome:

#268 (DeliveryStatus) — fixed:

• Dashboard tick downgrade guard added (mergeDeliveryStatus mirrors backend ackStatusTransitionFrom exactly; the live WS ack fires on every receipt and can arrive out of order/replayed).
• docs/06 message.ack payload + idempotency-key examples corrected.
• CI lint fix (two pre-existing eslint errors in the original commit).

#270 (MessageType) — fixed:

• The backfill migration never ran in the default SQLite (synchronize:true ⇒ migrationsRun:false). Replaced with an idempotent MessageTypeBackfillService that runs on bootstrap in every DB mode (+3 tests).

#271 (JID) — clean, no changes.

Cross-PR: only CHANGELOG.md conflicts (trivial). Merge order #268 → #270 → #271.

Release note: #270's webhook type change (chat→text, ptt→voice, vcard→contact) is a breaking contract change for webhook consumers → minor version bump required, not a patch.

CI status: all three PRs green (Lint/Test/Build/Dashboard). Roadmap remaining after these land: 🟠 Puppeteer config in the neutral contract, 🟠 EngineFactory registry.