fix: realign all ten plugins with the current OpenWA plugin contract - #51
Merged
Conversation
…0.12.0
The vendored contract in types/openwa.d.ts was last aligned against core ~0.8.2.
Core is now 0.12.0, and several of the gaps were not additive: the type file
declared members the host does not provide, which typechecks and then fails at
runtime. Re-verified field by field against the v0.12.0 tag.
Corrections that remove a lie:
- Drop `manifest` and `hookManager` from PluginContext. The sandbox context is
built from pluginId, config, logger, the capability groups, registerHook,
registerWebhook and registerSearchProvider — nothing else. `ctx.manifest.version`
compiled fine and threw at runtime.
- Drop `text()`, `json()` and `arrayBuffer()` from PluginNetResponse. Functions
cannot cross the worker structuredClone boundary, so these never existed on the
wire object; they were carried as runtime-throwing placeholders. Removing them
turns a runtime throw into a compile error. `statusText` is required, not optional.
- Add `latitude`/`longitude` to ConversationSendEnvelope and document per-type
behavior. A location envelope without valid coordinates now throws rather than
degrading to text, and `replyTo` throws on both location and media parts, so the
previously untyped coordinates made `type: 'location'` unusable.
- Annotate `HookResult.error` as in-process only. The sandbox wire result carries
just `{continue, data}`, so a marketplace plugin can only surface a failure by
throwing.
Corrections that close a gap:
- Add the `message:deleted` hook event, noting its payload is the host's persisted
row rather than an IncomingMessage.
- Widen `HookHandler` to accept a plain (non-promise) result, which the worker
already awaits either way.
- Add `IncomingMessage.kind` (optional here — plugins only read it) and the
`ChatKind` union; widen `contact` from `{name, pushName}` to the full
MessageContact shape, which also removes a cast a fixture needed.
- Document that `senderPhone` is assigned after the message:received chain and only
for @lid senders, so a hook handler always observes it unset.
- Correct `media.omitted`: it covers a download timeout and concurrency saturation,
not only the size cap, so "that file was too large" is the wrong message to show.
- Document the 50 MiB storage quota, that `set` can reject, and that the quota is
re-measured with a synchronous readdir + stat of every key on every write.
- Document that a returned WebhookResponse is ignored; the provider's reply comes
from the manifest ingress response.ack.
- Record the host bounds a plugin cannot see from the types: lifecycle, capability
and hook budgets, the concurrency cap, and the log limits.
Ingress types are deliberately left unvendored: both ingress manifests are
hand-written JSON that core validates at load, and adding types without a
validator that consumes them buys nothing.
PLUGIN-STANDARD.md and README.md both claimed plugins never auto-enable after a
restart. Core re-enables every plugin the operator had enabled, which makes
onEnable an unattended boot path: it must be idempotent and must not touch the
engine or await network I/O, because it runs before sessions connect and before
the HTTP listener opens. Both pages are corrected, and the standard gains the
permission table (five values, not three), the storage-cost rule, the veto
semantics of `continue: false`, the reach of a `webhook:before` subscription, and
the manifest-default seeding that now overrides a code-side default.
catalog.mjs rendered "(tested null)" for the one plugin with no
testedOpenWAVersion; it now reads "(not yet smoke-tested)".
Type-only, docs and test-fixture changes: all ten bundles rebuild to identical
sha256 digests, so no plugin ships a new artifact and no release is needed.
Verified: tsc --noEmit exits 0, 435/435 tests pass, catalog:check reports no drift,
all ten bundles load and construct as IPlugin.
…ota (0.6.0)
OpenWA 0.12.0 enforces a 50 MiB per-plugin storage quota and re-measures it on
every write by stat-ing every one of the plugin's storage keys, synchronously, on
the gateway's event loop. Two designs in this adapter turned that into message
loss and a gateway-wide stall.
Dedup markers used one storage key per message with a three-day retention. A
session at ten messages a minute holds roughly 43,000 of them, so every inbound
message made the host stat all 43,000 before the next marker could be written —
stalling HTTP, websockets and engine callbacks for the whole gateway, and getting
worse in proportion to traffic. The hourly prune added another round-trip per
marker on top. Markers now live in 256 sharded buckets keyed by a hash of the
marker id: the key count is constant regardless of volume, while the per-message
cost stays exactly what it was, one read and one write. Buckets drop their own
expired entries as they are written, so nothing needs a global scan.
Buckets deliberately do not use the `seen:` prefix. pruneSeen adopts any
`seen:`-prefixed value without a numeric `.t` by overwriting it with `{t: now}`,
and a bucket is a plain id-to-timestamp map — sharing the prefix would have
erased every marker in it once an hour. There is a test for that specifically.
Markers written by earlier versions are still honoured: missing one would re-post
an inbound message, or send a Chatwoot agent's reply to the recipient a second
time. That costs one extra read per unseen message, and it is retired
automatically once a prune finds no pre-0.6.0 markers left.
The retry queue was sized seven times larger than the entire quota: 500 pending
entries at up to 700 KB of media each is about 350 MiB. A media backlog therefore
hit the quota at around 74 entries, where storage.set began rejecting, the
drop-the-oldest policy never ran, and the message was lost outright because it had
already been marked seen. The pairing is now 80 by 200 KB, about 16 MiB, and the
two constants are documented as a pair so neither can be raised without redoing
the multiplication.
That loss was also invisible. The failed enqueue was swallowed by a catch that
logged and returned null, and the retry queue cannot count an entry it never
managed to store, so healthCheck went on reporting the plugin healthy while
messages disappeared. Such a message is now counted and surfaced in the health
message as LOST, which also marks the plugin unhealthy.
markSeen has moved inside the try block. Outside it, a rejected marker write left
the message neither relayed nor queued; it now takes the same retry path as a
failed relay.
Finally, polls relayed as an empty Chatwoot bubble. `poll` is a message type the
host reports with an empty body and it had no placeholder, so the post came back
422 and — the message being already marked seen — was dropped after five retries,
leaving the plugin permanently unhealthy. Polls now relay as a marker, and the
placeholder fallback can no longer return an empty string for any bodyless type,
including ones a future host version adds.
Verified: tsc --noEmit exits 0, 446/446 tests pass (11 new, each of which fails
against the previous implementation), catalog:check reports no drift, and the
rebuilt bundle loads and constructs as IPlugin.
Not addressed here, and still open: getChatHistory can exceed the host's 30 s
capability budget on a media-heavy chat, and fetchHistory cannot tell that timeout
apart from an empty history, so a lazy backfill silently imports nothing while the
bulk sweep still consumes its run-once marker.
…owth (1.1.0) Following this plugin's own setup guide produced an installation whose every webhook delivery was blocked. Only four hosts were reachable — the ones baked into the manifest `net.allow` — so the delivery endpoint an operator was told to stand up in step 4 was denied by the host's outbound gate, swallowed by the fail-open delivery path, and invisible, because this plugin has no health check. The guide's own remedy was to edit `manifest.json` and re-package the plugin. The manifest now declares `sttBaseUrl` and `deliveryWebhookUrl` as `net.allowConfigHosts`, so any https URL an operator configures is admitted without forking anything. The static entries stay: config-derived hosts are https-only, so they remain what covers a plain-http local backend. OpenWA 0.12.0 re-measures a plugin's storage quota on every write by stat-ing every one of its keys, which makes a key that is written once and never deleted a permanent tax on every future write. This plugin had two: one dedup key per transcribed voice note, and one counter key per session per hour. Dedup is now a single capped list per session and the rate limiter a single bucket-and-count key, so the key count follows the number of sessions rather than the plugin's uptime. The dedup deliberately stays in durable storage rather than moving to memory. With `chatDelivery: 'reply'` a duplicate transcript is a second quote-reply that the contact sees, so it has to survive a worker restart — which is exactly when WhatsApp is most likely to redeliver. Keys written by earlier versions are swept a few at a time as transcriptions run, and the sweep retires itself once a session is clean. Draining them at enable would have been simpler but wrong: enabling now happens unattended at host boot, where a long delete loop burns the lifecycle budget before sessions are even connected. Audio size is now checked before the base64 is decoded. Decoding first meant an oversized note — the very thing the guard rejects — was materialized as a Buffer anyway, on top of the base64 string already in memory, against a 256 MB worker heap that the host does not respawn if it is exhausted. The check is computed from the base64 length and padding, so it stays exact. Two smaller corrections: `timeoutMs` allowed 30000 while the host's per-capability budget is also 30000, so the two expired together and an STT timeout surfaced as a capability timeout; the maximum is now 25000. And an unset STT base URL now warns at enable, since the host does not enforce a `required` config field and the resulting failure otherwise reads as a broken allowlist rather than a missing setting. Verified: tsc --noEmit exits 0, 452/452 tests pass (6 new, covering the bounded key count, the capped dedup list, the single rate key across hours, the legacy sweep, and the size guard on both sides of the limit), catalog:check reports no drift, and the rebuilt bundle loads and constructs as IPlugin.
… (0.1.2)
Two template variables the README advertises never worked.
`{{sender.phone}}` read `msg.senderPhone`, which the host assigns only after the
message:received chain has run, and then only for @lid senders — so a hook handler
never observes it set. Any template using it as a path segment therefore requested
an empty segment, got a 404, and surfaced as the notFoundTemplate with nothing
logged to explain it. The digits now come from the sender JID, whose user part is
the MSISDN for a plain chat. An @lid sender still yields an empty value on purpose:
a privacy id's numeric part is not a phone number, and passing it upstream as one
would be worse than passing nothing.
`{{sender.id}}` read `msg.from`, which in a group is the GROUP jid — so every
member of a group resolved to the same value and a per-user lookup or an
authorization check written against it silently applied to the group instead. It
now reads `author`, the participant who actually sent the message.
Two failures were also invisible. A dedup read that threw was indistinguishable
from a genuine duplicate: the command was dropped with no reply, no error template
and no log line, so an operator saw it simply vanish. It stays fail-closed — that
is the right default against a double-fire — and now says so. And a reply template
referencing a field the response does not carry renders to an empty string, which
the host coerces into an empty WhatsApp bubble rather than rejecting; the error
template is now substituted and the empty render logged as the template bug it is.
Finally, an `exact` trigger did not tolerate a trailing space, which mobile
keyboards routinely append after an autocorrected word, so `ping ` missed a `ping`
trigger. Only the exact arm trims; the prefix arm still slices the original body,
so argument positions are unchanged.
Not changed, and deliberately so: the three-day dedup TTL. The equivalent retention
was a real problem in chatwoot-adapter and voice-transcription because those write
a key per message and per voice note respectively. This plugin writes one only
after a successfully answered command, and already prunes hourly, so the key count
tracks answered commands rather than traffic. Shortening the window would weaken
the redelivery guard for no storage benefit.
The README claimed a 0.8.7 floor and a `development` status while the manifest on
the same page said 0.8.0 and `beta`. 0.8.0 is correct: it is the release that
introduced both capabilities this plugin uses, `conversation:send` and
`net.allowConfigHosts`, neither of which existed before it, and it uses nothing
from 0.8.7. `testedOpenWAVersion` stays unset because the smoke test has not
happened.
Verified: tsc --noEmit exits 0, 459/459 tests pass (7 new, covering group author
resolution, @lid suppression, JID-derived digits, the logged dedup failure, the
empty-reply substitution and its own empty fallback, and the trimmed exact match),
catalog:check reports no drift, and the rebuilt bundle loads and constructs as
IPlugin.
…0.1.1)
First release verified against a live server rather than unit tests alone: a
self-hosted Typebot v3.17.2 driven over real WhatsApp through OpenWA 0.12.1. The
test flow's first bubble renders `num=[{{waNumber}}]` deliberately, so the value
is visible in the chat transcript rather than inferred.
That run found the headline bug. `{{waNumber}}` read `msg.senderPhone`, which the
host assigns only after the message:received chain has run, and then only for @lid
senders, so a hook handler never observes it set. The same published flow rendered
`num=[628999000]` when its Chat API was called directly and `num=[]` on an
identical message routed through the plugin — same flow, same host, only the path
differing. Any flow branching on the contact's number took the empty branch for
every contact, silently. The value now falls back to the digits of the sender JID,
using `author` in a group where `from`/`chatId` is the group rather than a person.
A @lid sender still yields an empty string: a privacy id's numeric part is not a
phone number, and feeding it to a CRM lookup as one is worse than feeding nothing.
Re-verified the same way after the fix: `num=[6281270008896]`. Behavioural proof
was the only option — the plugins API reports the version cached in the registry at
install time, so it still said 0.1.0 while the new code was running.
The send loop had no per-part isolation, so one failed part aborted every part
after it. State is persisted before sending, deliberately, because the Typebot
server has already advanced — which meant the contact was left with a half turn
while the plugin believed the prompt was delivered, and their next message was
matched against an input they never saw.
Sessions abandoned mid-flow were never cleaned up. A completed flow clears its own
row, so only abandonment leaks, but those rows are never revisited and OpenWA
re-measures the storage quota on every write by stat-ing every key a plugin owns —
so they make each later turn slower, permanently. Rows idle past
sessionTimeoutMinutes are now swept at most hourly, driven by traffic rather than a
timer so a disabled plugin leaves nothing running.
Two documentation corrections came out of the same run. A text input's placeholder
is sent to the contact as the prompt, since WhatsApp has no input field to show it
in — intended, but it means a placeholder written for a web form arrives as a
nonsense message. And apiHost must be https, which the plugin already enforces at
enable: a self-hosted Typebot on plain http cannot be used, because the host admits
an operator-configured host only when it is https and the API token would otherwise
travel in clear text.
Verified: tsc --noEmit exits 0, 464/464 tests pass (5 new), catalog:check reports
no drift, the rebuilt bundle loads and constructs as IPlugin, and the fix is
confirmed in a live WhatsApp transcript.
…th (0.3.0) The handler resolved the phone-derived chat id through `ctx.engine.canonicalChatId` before sending, on the theory that it protected an OTP addressed to a contact keyed by a `@lid` privacy id. It could never do that. `phoneToChatId` always yields `<digits>@c.us`, and the host's resolver returns a user-kind jid unchanged (`toNeutralJid`, engine/identity/wa-id.ts) — so the call was a guaranteed round-trip to the same string. What it did cost was real: a 2-second race, a live-engine dependency on the OTP critical path, and the `engine:read` permission. All three are gone. The README already claimed the plugin asked for no `engine:read`; that is now true rather than aspirational. The three tests covering that path went with it. They only passed because their fake returned a `@lid` id for a `@c.us` input, which the real host never does — a fixture that encoded a behaviour the system does not have. One test now asserts what actually happens: the OTP goes to the phone-derived JID, with no engine round-trip. The documented setup order could not succeed either. It said to install and enable first, but enabling validates the plugin's base config and refuses to start without `appName`, while the instance minted in step 1 carries a per-instance config — so following the guide verbatim left enable failing. A base-config step now precedes enable, in both the walkthrough and the CLI snippet. Two documentation corrections. Delivery is at-least-once: returning from the handler completes the job but does not guarantee a single run, because a row whose outcome was never recorded is re-dispatched by the host's reconciler. The replay carries the same payload, so the contact receives the same code again — noise, not a security problem, which is why there is deliberately no per-delivery dedup store: it would put an unbounded key-per-delivery on the OTP critical path to suppress a duplicate of an identical message. And per-user ordering plus the retry/DLQ path require QUEUE_ENABLED=true; with the queue off, ingress runs inline and makes one attempt. Verified: tsc --noEmit exits 0, 462/462 tests pass, catalog:check reports no drift, and the rebuilt bundle loads and constructs as IPlugin.
…3.1) `message:failed` used to fire only from `sendText`, so hardcoding the type column to `text` and reading the body from `input.text` was correct. OpenWA 0.12 routes every sender through one shared failure path that carries the real type — image, video, voice, document, location, contact, poll, sticker, reply, forward, edit and bulk — and a media send holds its caption in `caption`, not `text`. So every non-text failure was recorded as a text message with an empty body. An audit sheet is only worth what its worst row is worth, and a failed invoice photo appeared as a `text` message with nothing in it: the type column lied and the one field that would have identified the message was blank. The row now logs the reported type, falling back to `text` when a payload carries none (an older host), and reads the content through text → caption → body. Bulk failures are a related case. A bulk send emits the shared content with the recipient on the enclosing item, so the payload genuinely has no chatId. Blank is the honest value rather than a mirrored guess; the row stays attributable by session, error and timestamp. Verified: tsc --noEmit exits 0, 466/466 tests pass (4 new, covering an image failure's type and caption, a text failure, a typeless payload, and a bulk item), catalog:check reports no drift, and the rebuilt bundle loads as IPlugin.
…tness fixes Four plugins whose audit findings were each a clause, released together rather than as four near-empty commits. None of them was broken on 0.12; each had one path that failed quietly. group-translate 1.0.7 forwarded whatever the translation backend returned straight into the send, and a backend answering 200 with a blank translation is a real case. OpenWA 0.12 rejects an empty positional capability argument, so that stopped being a blank WhatsApp bubble and became a thrown capability error the coordinator swallowed — the translation silently never arrives either way. It is now dropped deliberately, where the reason can be stated. Its `timeoutMs` is also clamped in code now: a configSchema `min` is a form hint the host never enforces, and the host clamps a fetch timeout of <= 0 up to 1 ms, so a config of 0 made every translate abort instantly with nothing logged. chat-flow 1.0.8 guarded that `body` was a string but not that it had content, and every non-text message carries an empty body. With the documented empty trigger a bare sticker started the flow; with a flow open, a photo drew "Invalid option" — and because chat-flow claims the event, no sibling auto-replier saw it either. faq-bot 0.1.8 joined every skipped regex into one warning line. The host caps a log line at 8 KiB and drops the overflow, so one pathological pattern took the rest of the diagnostic with it. Patterns are truncated to 80 chars before joining, keeping every skipped rule identifiable. after-hours 0.1.4 took the per-chat cooldown slot before sending, so a failed reply silenced that chat for the whole window with nothing delivered. That is easier to hit now: another plugin vetoing message:sending surfaces here as a thrown error indistinguishable from a transport failure. The slot is released on failure. Its minOpenWAVersion also moves 0.6.2 → 0.7.0 — per-session config, which its own 0.1.3 fix and its README both depend on, landed in 0.7.0, so on a 0.6.x host it installed happily and then ignored every per-session override. Verified: tsc --noEmit exits 0, 469/469 tests pass (3 new), catalog:check reports no drift, all ten bundles rebuild and construct as IPlugin.
…odify-write Self-review of this branch found that both storage rewrites introduced the same race, and both were mine. Consolidating dedup markers into shared documents — 256 sharded buckets in chatwoot-adapter, one capped list per session in voice-transcription — turned a write that used to be a single independent key into a read-modify-write. Every await between the read and the write is an IPC round-trip to the host, so the window is wide rather than theoretical, and both plugins deliberately run their work off the message hook, so concurrent entries are the normal case rather than an edge one. In chatwoot-adapter the surrounding per-chat and per-conversation locks give no protection, because they are keyed by chat while a shard is shared across chats: an inbound on one chat and the echo marker for a different conversation can hash to the same bucket and clobber each other. A lost `cw` marker re-sends a Chatwoot agent's reply to the contact — a duplicate the customer sees. A lost `wa` marker re-posts an inbound to Chatwoot. In voice-transcription a burst of voice notes interleaved on the single per-session list and the last writer overwrote the others' ids. Each lost id is a second paid STT call and, with chatDelivery 'reply', a duplicate transcript the contact sees. Both are fixed by serializing only the critical section: a per-bucket lock in MappingStore.markSeen, and a per-session tail around the claim in the coordinator. Transcription, delivery and relaying stay concurrent. Neither pre-existing scheme had this window, so this restores a guarantee rather than adding one — which is why it folds into the same unreleased 0.6.0 and 1.1.0 entries instead of a new version. Each fix has a regression test that reproduces the interleaving against a storage fake with realistic async latency, and both were confirmed to FAIL against a copy of the tree with the fix reverted. Verified: tsc --noEmit exits 0, 471/471 tests pass (2 new), catalog:check reports no drift.
… in self-review
An adversarial review of this branch found three real defects, all introduced by
the branch itself, and all worse than the behaviour they replaced.
The JID-to-digits helper added in http-action 0.1.2 and typebot-connector 0.1.1
denylisted `@lid`. That is not enough. A group (`@g.us`), a channel
(`@newsletter`) and a broadcast JID all have numeric local parts too, so each was
emitted as if it were a phone number — into a request path, a CRM lookup, an
authorization check. Before this branch those fields were simply empty, so the
"fix" made the failure worse: an empty value forces the operator to handle it,
a plausible-looking wrong number does not. Both copies now allowlist `@c.us` and
`@s.whatsapp.net`, and strip the `:device` suffix Baileys can append, which the
denylist version silently dropped on the floor.
typebot-connector's abandoned-session sweep read every `sess:` row but took its
idle threshold from the config resolved for the session whose message happened to
trigger it. `sessionTimeoutMinutes` is overridable per session, so a support
session with a five-minute timeout would delete a sales session's rows that were
still live under a one-day timeout — a contact halfway through a long form
restarts at question one, silently. The sweep is now scoped to the triggering
session's key prefix, its throttle is per session too (a single global throttle
would have let the first session to fire starve every other session's cleanup),
and one unreadable row no longer abandons the rest of the pass.
The vendored contract itself taught the first bug. Its new `senderPhone` comment
told plugin authors to derive digits with `(author ?? from).split('@')[0]` and no
guard at all — the unguarded form, written into the one file every plugin compiles
against. It now shows the allowlisted derivation, and the contract gains
`isLidSender`/`isStatusBroadcast` so a plugin can key that guard on host flags
rather than string suffixes. The host-bounds block also omitted that `net.fetch`
is capped at 16 concurrent calls GLOBALLY — shared across every plugin and worker,
not the 32-per-plugin figure the block otherwise implies.
Each fix has a regression test enumerating the JID shapes, and one asserting that a
sweep triggered by one session leaves another session's mid-flow row intact.
Verified: tsc --noEmit exits 0, 474/474 tests pass (3 new), catalog:check reports
no drift.
…ew findings Follow-up to the high-severity pass; these are the medium findings that were real code defects rather than documentation drift. voice-transcription's legacy sweep only cleaned the `seen:<sid>:*` family. The pre-1.1.0 rate counter was `rate:<sid>:<hour>` — one key per hour, also never deleted — so an install upgraded after a year kept ~8760 counter keys and went on paying the per-write stat cost the sweep exists to remove. The CHANGELOG claimed keys written by earlier versions were swept away; now that is true of both families. The same sweep retired itself after one empty listing. The host's `list()` swallows its own errors and resolves empty, so a single transient failure was indistinguishable from a clean session and permanently stranded the legacy keys. Retirement now needs two consecutive clean passes. `timeoutMs` was clamped only by a manifest `max`, which the host never enforces — the same trap PLUGIN-STANDARD warns about in this branch, applied to group-translate in the same commit and missed here. A stored value above the ceiling reached the STT client and made the capability timer race the fetch abort, so an STT timeout surfaced as a capability timeout. It is clamped at read time now. typebot-connector's per-part send isolation — a headline fix of 0.1.1 — had no unit test. It was confirmed in production, when an image the host refused to fetch still let the following choice list through, but nothing in the suite would have caught its removal. It has a test now: the first part rejects, the second must still be sent, and handleTurn must resolve. Verified: tsc --noEmit exits 0, 477/477 tests pass (3 new), catalog:check reports no drift, all ten bundles rebuild.
Both were exercised end to end against OpenWA 0.12.1 on the test server with a live WhatsApp session and a self-hosted Typebot v3.17.2, so the field is finally saying something true — http-action had never carried one at all. typebot-connector 0.1.1: text turns, session start/resume/end, contact variables in 1:1 and in a group (where the number must come from the participant, not the group), a media bubble delivered and confirmed on the handset, a numbered choice list with the numeric reply mapped back to its option, per-sender flow state in a group, and the per-part isolation firing for real when the host refused a media URL. http-action 0.1.2: GET and POST, path and JSON-body templating, bearer auth reaching the upstream, sender identity resolved from the JID, and the notFound and error templates selected by real 404 and 500 responses. Deliberately NOT bumped: chatwoot-adapter and chat-flow. Both ran on the 0.12.1 host during these sessions, but the builds deployed there are 0.5.7 and 1.0.7 — the versions in this branch have never run on it. The field is only worth anything if it refuses to round up. Still unproven for typebot-connector, and the reason it stays beta: the file-upload input. Its failure path is confirmed in production (the contact gets a clear message and the flow does not advance), but the success path needs a self-hosted Typebot whose S3 storage and public viewer URL are both reachable from the plugin's allowlisted apiHost, which this test topology does not provide.
Six were real defects rather than wording, and three of those were introduced by this branch. after-hours cleared the per-chat cooldown slot when a reply failed. That removed the only throttle: against a permanently failing send — a compliance plugin vetoing this chat — every inbound message retried immediately, so sixty messages meant sixty send attempts and sixty `message:sending` fan-outs across every installed plugin. The slot is now rewound rather than cleared, so the next attempt lands a fixed backoff from now whether that is shorter or longer than the configured cooldown: a transient failure recovers in a minute, a persistent one still cannot storm. http-action's new empty-reply fallback rendered the error template outside any try/catch. Rendering can throw on a too-deep path or a prototype key, and that rejected out of the handler — so the contact received nothing where previously an empty bubble was at least sent and the message marked seen. It degrades to the built-in default now. typebot-connector keyed an authorless group message to a shared `unknown` row, so every such participant drove the same Typebot session and one contact's answer advanced another contact's flow. There is no safe way to attribute that message, so the turn is skipped. The dead `senderPhone` term in the key went with it — the host assigns that field after the hook chain, so it was never populated there. group-translate's empty-translation guard could never fire: the formatter prefixes each entry with a flag and language code, so a blank translation still rendered a non-empty bubble. It is dropped where it is collected instead. `decodedBase64Size` under-counted unpadded base64 — a three-character tail decodes to two bytes but rounded to zero — which would have let an oversized note through the size guard rather than failing safe, the opposite of what its comment claimed. `phoneFromJid` is now a real file in both plugins and registered in the shared-copies guard, so the two copies cannot drift the way they nearly did. Four documentation claims were wrong and are corrected: a plugin left in ERROR IS restored at boot (the filter keys on the operator's decision, not on status); `edit` does not emit `message:failed`; not every non-text message carries an empty body (a captioned image carries its caption); and truncating each skipped pattern bounds one rule, it does not guarantee the whole log line fits. The vendored contract also no longer claims to be exactly the sandbox context while omitting `registerSearchProvider`, and chatwoot-adapter now records that its lower retry media cap means an attachment above roughly 150 KB is queued as a placeholder rather than as the file. Two tests added earlier on this branch tested a re-implementation rather than the plugin — after-hours manipulated the cooldown map directly, chat-flow defined its own predicate — and passed with their fixes reverted. Both now drive the plugin through a fake context, and both were confirmed to fail against a copy of the tree with the fix removed, as were the new base64 and authorless-group tests. Verified: tsc --noEmit exits 0, 481/481 tests pass, catalog:check reports no drift, all ten bundles rebuild and construct as IPlugin.
…n follow-ups Working through the round-two fixes by hand rather than trusting the green suite. Three things needed correcting; the rest held up. after-hours' manifest still described `cooldownSec: 0` as "reply every time", which stopped being true when the failure path gained a backoff: a reply that fails is now held for a minute at any cooldown value, including zero. The description says so. Five chatwoot-adapter test fixtures build `InboundDeps` through `as unknown as InboundDeps`, so the compiler never noticed they omit the now-required `onInboundLost`. Nothing reaches it today — only `inbound.test.ts` calls `handleInbound`, and that one supplies it — but the next test to exercise the message-lost path from `echo-loop`, `sent` or `backfill` would have failed with "onInboundLost is not a function" rather than with whatever it was actually asserting. typebot-connector now skips a group message the engine delivers without an identifiable sender. Core only sets `author` when Baileys supplies `participant` (baileys-message-mapper), so that case is real, and the alternative — the previous behaviour — was to key every such message to one shared row and feed one contact's answer into another contact's flow. The skip is silent, so the README says it happens and why. Verified by hand, not by inference: the backoff arithmetic lands the next attempt exactly 60 s out for every cooldown from 0 to 86400; `decodedBase64Size` is exact for padded and unpadded input and over-counts on whitespace, which is the safe direction; the bucket lock holds only storage awaits so no path takes a chat lock while holding it, and lock order is always caller then bucket; a rejected task does not poison the per-session claim chain and it leaks no map entries; the legacy sweep retires only after two consecutive clean passes; a sweep of session `abc` leaves session `abcd` untouched; and the two `jid.ts` copies are byte-identical and covered by the shared-copies guard, which does run as part of the suite. Verified: tsc --noEmit exits 0, 481/481 tests pass, catalog:check reports no drift.
A second adversarial review targeted the four commits that fixed round one, none of which had been reviewed. It found two high-severity defects, both mine, and both the same shape as bugs I had just fixed elsewhere. chatwoot-adapter gated its pre-0.6.0 marker lookup behind a flag that pruneSeen cleared whenever its listing came back empty. The host's list() swallows its own errors and resolves empty, so one transient read failure retired that lookup for the process's life — and a missed `cw` marker re-sends a Chatwoot agent's reply to the contact, a duplicate the customer sees. The flag is gone. What it saved was one storage read per not-yet-seen message; what it risked was a duplicate message. voice-transcription kept its per-session claim chain on the coordinator instance, but the plugin rebuilds the coordinator whenever the resolved config signature changes, which with per-session config can be every message. Each rebuild handed the next note an empty chain, silently undoing the serialization added two commits earlier. The map is module-scoped now — one worker runs one plugin, so that is the correct scope. The "two consecutive clean passes" guard added for the same swallowed-error problem was itself defeated by it: two sweeps in one burst both see an empty listing, and the second reads the flag the first just set. Serializing them did not help, because two sequential sweeps behave identically. The rule cannot work — an empty listing and a failed one are indistinguishable — so the retirement is deleted rather than repaired. Both plugins now simply sweep, which is a readdir of a directory this release already made small. http-action's outer catch still rendered the error template unguarded: the same throw-on-bad-template hazard the previous commit fixed at the inner call site, one branch away. voice-transcription's changelog had lost its entire 1.0.2 section — a heading replacement consumed it and left its body orphaned under 1.1.0, so a released version had vanished from its own history and two of its fixes were being advertised as new. My own check for this compared section counts with `>=` and would never have caught it; all ten changelogs are now verified against an exact expected count. The after-hours test rewritten last commit *because* it passed with its fix reverted still did, for the new behaviour: it pinned "not un-throttled" and nothing about the backoff window. It drives a mocked clock now and fails against both wrong behaviours. The group-translate test added for the blank-translation drop never reached the translate path at all — a state with no participants translates nothing, so it asserted an empty result either way. Every new test in this commit was confirmed to fail against a copy of the tree with its own fix reverted, and the after-hours schedule helper no longer depends on what time the suite runs. Also corrected: a README claim contradicting PLUGIN-STANDARD about ERROR-status restore, a code comment repeating an overstatement its own docs had already fixed, a clause inserted mid-sentence in gsheets-logger, an after-hours changelog still describing the behaviour that commit replaced, and `audio` missing from a list of senders that emit message:failed. Verified: tsc --noEmit exits 0, 485/485 tests pass, catalog:check reports no drift, all ten bundles rebuild and construct as IPlugin.
I deferred sixteen findings on their severity label without reading them. Four were misclassified, and one of those is a regression this branch introduced. voice-transcription's hourly spend cap was the regression. Collapsing the per-hour counter keys into a single `rate:<sid>` key turned its check-and-increment into a read-modify-write that the old scheme could not have: two notes straddling an hour boundary used to write different keys, so neither could clobber the other. Now the pre-boundary write can land last and restore the previous bucket, restarting the new hour's count at zero — doubling a cap whose stated job in the manifest is bounding paid-API spend. It runs on the same per-session tail as the dedup claim now. after-hours encoded its failure backoff as a rewound cooldown timestamp. That is only meaningful against the cooldown value it was computed from, and config is re-read per message — so an operator lowering `cooldownSec` in the dashboard turned the stored value into "long past" and handed back the un-throttled retry storm the backoff exists to prevent. The deadline is absolute now and independent of the live config. typebot-connector's abandoned-session sweep was scoped to the triggering session two commits ago, correctly — its threshold comes from that session's config. What went unnoticed is that the scoping removed all coverage for a session that stops sending entirely: a disabled or deleted tenant's rows are never listed again, and every stored key is stat-ed on every write, so they tax every other session's turns forever — the exact cost the sweep exists to remove. A second, unscoped pass reclaims rows idle past a week, a threshold no per-session timeout plausibly exceeds. http-action passed Error objects into `PluginLogger.warn`'s structured-meta slot, which the host renders as an empty object. The dedup-failure warning added two commits ago specifically so a swallowed command would stop being invisible was arriving without its cause. Also corrected: the dedup-window comment claimed 500 ids covers eight hours at the default cap, but every note that reaches the claim consumes a slot including ones later skipped, so it is 500 inbound notes rather than 500 transcriptions; and the voice changelog still described a two-clean-passes retirement that the previous commit deleted outright. Each fix has a test that fails against a copy of the tree with it reverted: the backoff surviving a live `cooldownSec` change, and the orphan pass reclaiming a silent session's rows while leaving a recent one alone. Verified: tsc --noEmit exits 0, 487/487 tests pass, catalog:check reports no drift, all ten bundles rebuild and construct as IPlugin.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Realigns all ten plugins with the OpenWA core they now run on. The vendored contract had drifted since
~0.8.2 and two host mechanisms introduced since then — a 50 MiB per-plugin storage quota re-measured on
every write, and automatic re-enable of plugins at host boot — turned several long-standing plugin designs
into message loss and, in one case, a gateway-wide stall.
The vendored contract
types/openwa.d.tsis hand-maintained; there is no published SDK package. It had accumulated members thehost does not provide, which typecheck and then fail at runtime:
PluginContext.manifestandPluginContext.hookManagerdo not exist on the sandbox context. Readingctx.manifest.versioncompiled and threw.PluginNetResponse.text()/json()/arrayBuffer()never existed on the wire object — functionscannot cross the worker boundary. Removing them turns a runtime throw into a compile error.
ConversationSendEnvelopelackedlatitude/longitude, sotype: 'location'could not be used at all:the host now throws without valid coordinates rather than degrading to text.
HookResult.erroris in-process only; a sandboxed plugin can only surface a failure by throwing.Added: the
message:deletedevent,IncomingMessage.kind/isLidSender/isStatusBroadcast, the fullMessageContactshape, and the host bounds a plugin cannot see from the types — lifecycle and capabilitybudgets, the hook budget that fails open, the 32-per-plugin capability cap and the 16 concurrent
net.fetchcalls shared globally across all plugins.PLUGIN-STANDARD.mdand the rootREADME.mdboth claimed plugins never auto-enable after a restart. Theydo. That is not a wording fix:
onEnableis now an unattended boot path that runs before sessions connectand before the HTTP listener opens, so it must be idempotent and must not touch the engine or await
network I/O. The standard also gains the storage cost model, the veto semantics of
continue: false, thereach of a
webhook:beforesubscription, and the permission table (five values, not three).Storage: the change behind most of this
ctx.storage.setre-measures the plugin's quota by stat-ing every key it owns, synchronously, on thegateway's event loop. A plugin that writes one key per message therefore pays O(keys) syscalls per message.
files at ten messages a minute, so every inbound message stalled HTTP, websockets and engine callbacks
while the host stat-ed all of them. Markers now live in 256 sharded buckets: constant key count, same
one-read-one-write per message. Its retry queue was also sized at ~350 MiB against a 50 MiB budget, so a
media backlog hit the quota at ~74 entries, the drop-oldest policy never ran, and the message was lost
while
healthCheckstill reported green. That loss is now counted and surfaced.neither. Both collapse to one bounded key per session, with the old keys swept as transcriptions run.
reclaimed, both per session and, for sessions that stop sending entirely, by an unscoped weekly pass.
Consolidating keys turns an atomic write into a read-modify-write, so the dedup claims and the hourly spend
counter are serialized per session or per bucket. Without that, a burst loses markers — and a lost marker
means a duplicate message the customer sees.
Correctness fixes found while validating against a live server
typebot-connector and http-action were exercised end to end against OpenWA 0.12.1 with a live WhatsApp
session and a self-hosted Typebot: text turns, session start/resume/end, media, a numbered choice list
mapped back to its option, group behaviour, GET and POST with path and body templating, bearer auth, and
the notFound/error templates against real 404 and 500 responses.
{{sender.phone}}(http-action) and{{waNumber}}(typebot-connector) were empty for every sender: bothread
senderPhone, which the host assigns after themessage:receivedchain and only for@lidsenders. Both now derive digits from the sender JID, allowlisting real user domains — a group, channel or
broadcast JID is numeric too, and passing one upstream as a phone number is worse than passing nothing.
{{sender.id}}was the group JID in group chats, so every member resolved to the same value and anyper-user lookup or authorization check silently applied to the group.
prompt as delivered — the contact saw half a turn and their next message was matched against an input
they never saw.
textmessage with an empty body;message:failednow fires from every sender with a real type, and media DTOs carry
caption, nottext.canonicalChatIdround-trip that could never change its input, and with it theengine:readpermission and a live-engine dependency on the OTP path. Its documented setup order alsocould not succeed: enabling validates the base config, which the guide never set.
a dropped command with no log, an empty translation shipped as a bubble, a sticker driving the menu, a
log line that could crowd out its own diagnostic, and a failed away reply silencing a chat for the whole
cooldown.
Versions
chatwoot-adapter 0.6.0 · voice-transcription 1.1.0 · supabase-otp-hook 0.3.0 · gsheets-logger 0.3.1 ·
chat-flow 1.0.8 · group-translate 1.0.7 · faq-bot 0.1.8 · after-hours 0.1.4 · http-action 0.1.2 ·
typebot-connector 0.1.1.
testedOpenWAVersionmoves to 0.12.1 only for typebot-connector and http-action, the two actuallyexercised on that host. after-hours' floor corrects from 0.6.2 to 0.7.0 — per-session config, which its own
README depends on, landed in 0.7.0.
Verification
tsc --noEmitexits 0; 487 tests pass (up from 435);catalog:checkreports no drift; all ten bundlesrebuild and construct as
IPlugin. Every regression test added here was confirmed to fail against a copyof the tree with its own fix reverted.
Known limitations
host — the contact gets a clear message and the flow does not advance — but the success path needs a
self-hosted Typebot whose S3 storage and public viewer URL are both reachable from the plugin's
allowlisted
apiHost.getChatHistorycan exceed the host's capability budget on a media-heavy chat, andthe fetch cannot tell that timeout apart from an empty history, so a lazy backfill imports nothing while
the bulk sweep still consumes its run-once marker.
message:receivedand none passes a hook priority, so co-installingseveral auto-repliers can draw more than one reply to a single message. Ordering is registration order.