[pull] main from rmyndharis:main

.env.example

@@ -687,9 +687,11 @@ PLUGINS_DIR=./data/plugins          # Plugin directory (default: ./data/plugins)
 # PLUGINS_DIR, which is where plugin packages are installed. It moves plugin state only: the
 # databases, sessions, media and auth dirs each carry their own path knob and none of them follow
 # this one.
-# Moving it does NOT carry the existing registry across: point it at an empty directory and the
+# Moving it does NOT carry the existing state across: point it at an empty directory and the
 # gateway starts with no record of installed plugins, even though their packages are still under
-# PLUGINS_DIR. Move data/plugins/registry.json yourself when you change this.
+# PLUGINS_DIR. Move the whole `<old root>/plugins` tree yourself when you change this, not just
+# registry.json: each plugin's persisted ctx.storage lives beside it as `<plugin id>/key-*.json`,
+# so carrying the registry alone leaves every plugin enabled but with its state gone.
 # PLUGIN_STATE_DIR=./data
 # Cap on a plugin .zip downloaded by install-from-URL (matches the 5 MB upload limit). A non-positive
 # or non-numeric value falls back to the default.

CHANGELOG.md

@@ -14,14 +14,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - `POST /sessions/{sessionId}/chats/unread` publishes its own `MarkChatUnreadDto` rather than sharing `MarkChatReadDto`. The body is unchanged (`chatId` alone), but a generated client sees the schema under a new name.
-- ⚠️ **Breaking (Go and Java clients).** `subscribePresence` takes its own `SubscribePresenceRequest` rather than the shared `MarkChatRequest`, which now serves `markUnread` alone. Swap the type at the call site; the wire body is unchanged. `SubscribePresenceDto` had no contract-gate coverage while one type stood for two routes.
+- ⚠️ **Breaking (Go, Java and typed Python callers).** `markRead` and `subscribePresence` each take their own request type rather than the shared `MarkChatRequest`, which now serves `markUnread` alone. Swap the type at both call sites; the wire body is unchanged and the JavaScript and PHP clients are unaffected. `SubscribePresenceDto` had no contract-gate coverage while one type stood for two routes.
 
 ### Fixed
 
+- `POST /chats/read` answers 400 for `"messageIds": null` instead of 500. `@IsOptional` skips every validator for null as well as undefined, so the value reached the Baileys adapter and was dereferenced there. The published schema now carries `minItems` too, so it no longer advertises an empty array the server refuses.
+- A read receipt goes only to the chat the caller named. A message id belonging to another chat in the same session carried that chat's address out of the message store, so the receipt landed there while the route reported success for the chat in the path.
+- The Go client can express an empty `messageIds` again. `omitempty` on a plain slice dropped it, so a caller asking for nothing to be acknowledged silently acknowledged the newest message; the field is a pointer, so absent and empty are distinct on the wire.
 - The dashboard CSP nonce is substituted at every occurrence in the served document, not only the first. One placeholder exists today, so a second would have been left reading the literal text and its script refused by the browser.
 - Outbound webhook deliveries survive a hard crash. Fan-out was fire-and-forget, so a crash between persisting a message and completing its POST lost the delivery, against a documented at-least-once contract. Deliveries are now recorded before they are attempted, and a bounded sweep replays whatever is stranded under its stored idempotency key.
+- A stranded webhook delivery now gets the replay budget it was promised. The reconciler read success from a call that cannot fail, so a replay that never delivered was retired as dispatched on the first sweep and its payload dropped. Delivery reports an outcome instead, and a failed replay stays pending.
+- Restoring a backup no longer aborts when the target already holds the outbound delivery records. The table has no session foreign key, so the replace never cleared it and every overlapping row collided, rolling the whole import back.
 - Settled outbound delivery records are pruned after `WEBHOOK_OUTBOX_RETENTION_DAYS` (default 7). A record that can still be replayed is never pruned on age, and a non-positive window falls back to the default rather than letting the table grow without bound.
 - `PLUGIN_STATE_DIR` moves the plugin registry and per-plugin storage off the default `./data`. It was the one piece of state with no path knob, so a test run rewrote the developer's own registry.
+- `backup.sh` and `restore.sh` follow `PLUGIN_STATE_DIR`. Both hardcoded the plugin state under the data dir, so with the knob set the archive carried neither the registry nor any plugin's persisted storage, and a restore put nothing back. The knob's own note now says to move the whole tree, not just `registry.json`.
 - The e2e lane sweeps the throwaway state roots it creates. Each suite gets its own, nothing removed them, and the temp directory accumulated hundreds of entries over a few days of runs.
 - e2e assertions are no longer answered by unrelated processes on the host. supertest binds its per-request listener to the wildcard address and then dials 127.0.0.1, which on macOS lets a process holding that port on 127.0.0.1 answer instead. Each suite's server now listens on loopback during init, which supertest reuses.
 - A stalled `apt-get` can no longer hold a CI run open. The scripts-smoke job installed sqlite3 and shellcheck unbounded, so a slow mirror held two main runs past an hour with every other job already green. Both steps now time out and skip the install when the runner already ships the tool.

openapi.json

@@ -10265,6 +10265,7 @@
           "messageIds": {
             "description": "Specific message IDs to mark read. Baileys acknowledges individual messages, so without this only the newest message the engine still holds in memory gets a receipt — a burst leaves its earlier messages unread forever, and a restarted session has no message to acknowledge at all. Callers that persist inbound message IDs should send them here. Ignored by whatsapp-web.js, whose own sendSeen is chat-level.",
             "maxItems": 100,
+            "minItems": 1,
             "example": [
               "3EB0C767D26B8A3F1A2B",
               "3EB0C767D26B8A3F1A2C"

scripts/backup.sh

@@ -64,7 +64,14 @@ MEDIA_DIR="$(openwa_resolve STORAGE_LOCAL_PATH "$DATA_DIR/media")"
 # registry and each plugin's ctx.storage below — so an unset PLUGINS_DIR must resolve there
 # too, or the archive silently omits the plugin packages.
 PLUGIN_PACKAGES_DIR="$(openwa_resolve PLUGINS_DIR "$DATA_DIR/plugins")"
-PLUGIN_STATE_DIR="$DATA_DIR/plugins"
+# Plugin registry + every plugin's persisted ctx.storage. The app puts them at <dataDir>/plugins,
+# where dataDir is PLUGIN_STATE_DIR when that is set and ./data otherwise, so the knob has to be
+# resolved here exactly like PLUGINS_DIR above. Hardcoding $DATA_DIR/plugins meant an operator who
+# moved plugin state got an archive with neither the registry nor any plugin's storage in it, and
+# a restore that put nothing back. Resolved under its own name because the knob names the ROOT,
+# not the plugins directory inside it.
+PLUGIN_STATE_ROOT="$(openwa_resolve PLUGIN_STATE_DIR "$DATA_DIR")"
+PLUGIN_STATE_DIR="$PLUGIN_STATE_ROOT/plugins"
 GENERATED_ENV="$DATA_DIR/.env.generated"
 ADMIN_KEY_FILE="$DATA_DIR/.api-key"
 

scripts/restore.sh

@@ -79,7 +79,14 @@ MEDIA_DIR="$(openwa_resolve STORAGE_LOCAL_PATH "$DATA_DIR/media")"
 # registry and each plugin's ctx.storage below — so an unset PLUGINS_DIR must resolve there
 # too, or the archive silently omits the plugin packages.
 PLUGIN_PACKAGES_DIR="$(openwa_resolve PLUGINS_DIR "$DATA_DIR/plugins")"
-PLUGIN_STATE_DIR="$DATA_DIR/plugins"
+# Plugin registry + every plugin's persisted ctx.storage. The app puts them at <dataDir>/plugins,
+# where dataDir is PLUGIN_STATE_DIR when that is set and ./data otherwise, so the knob has to be
+# resolved here exactly like PLUGINS_DIR above. Hardcoding $DATA_DIR/plugins meant an operator who
+# moved plugin state got an archive with neither the registry nor any plugin's storage in it, and
+# a restore that put nothing back. Resolved under its own name because the knob names the ROOT,
+# not the plugins directory inside it.
+PLUGIN_STATE_ROOT="$(openwa_resolve PLUGIN_STATE_DIR "$DATA_DIR")"
+PLUGIN_STATE_DIR="$PLUGIN_STATE_ROOT/plugins"
 RESTORE_TIMESTAMP="$(date +%Y%m%d-%H%M%S)"
 RESOLVED_CWD="$(pwd -P)"
 

scripts/smoke-test-backup-restore.sh

@@ -293,5 +293,42 @@ if [ "$(db_fingerprint "$F/extract2/main.sqlite")" != "STALE-main" ]; then
 fi
 pass "(f) data/.env.generated resolves paths for both scripts, and the environment still wins"
 
+echo ""
+echo "==> (g) PLUGIN_STATE_DIR moves the registry and ctx.storage, and both scripts follow it"
+# The knob names the ROOT; the app keeps plugin state at <root>/plugins. Both scripts hardcoded
+# $OPENWA_DATA_DIR/plugins, so with the knob set the archive carried neither the registry nor any
+# plugin's persisted storage, and the restore put nothing back. Silent both ways: an empty source
+# directory simply produces no plugin-state entry.
+G="$WORK/g"
+mkdir -p "$G/state" "$G/elsewhere/plugins/chatwoot" "$G/extract" "$G/restore/state"
+make_fixture "$G/state/main.sqlite" "golf-main"
+make_fixture "$G/state/openwa.sqlite" "golf-data"
+printf '{"plugins":[{"id":"chatwoot"}]}' >"$G/elsewhere/plugins/registry.json"
+printf 'mapped-conversation' >"$G/elsewhere/plugins/chatwoot/key-Zm9v.json"
+(
+  cd "$G"
+  OPENWA_DATA_DIR="$G/state" PLUGIN_STATE_DIR="$G/elsewhere" BACKUP_DIR="$G/out" \
+    MAIN_DATABASE_NAME="$G/state/main.sqlite" DATABASE_NAME="$G/state/openwa.sqlite" "$BACKUP" >/dev/null
+)
+ARCHIVE_G="$(ls "$G"/out/openwa-backup-*.tar.gz)"
+tar -xzf "$ARCHIVE_G" -C "$G/extract"
+if [ ! -f "$G/extract/plugin-state/registry.json" ]; then
+  fail "(g) backup ignored PLUGIN_STATE_DIR: the plugin registry is missing from the archive"
+fi
+if [ ! -f "$G/extract/plugin-state/chatwoot/key-Zm9v.json" ]; then
+  fail "(g) backup ignored PLUGIN_STATE_DIR: a plugin's persisted ctx.storage is missing"
+fi
+# And the restore has to put them back where the knob points, not under the default data dir.
+(
+  cd "$G"
+  OPENWA_DATA_DIR="$G/restore/state" PLUGIN_STATE_DIR="$G/restored-elsewhere" \
+    MAIN_DATABASE_NAME="$G/restore/state/main.sqlite" DATABASE_NAME="$G/restore/state/openwa.sqlite" \
+    "$RESTORE" "$ARCHIVE_G" --force >/dev/null
+)
+if [ ! -f "$G/restored-elsewhere/plugins/registry.json" ]; then
+  fail "(g) restore ignored PLUGIN_STATE_DIR: the registry did not land under the configured root"
+fi
+pass "(g) PLUGIN_STATE_DIR is honoured by backup and by restore"
+
 echo ""
 echo "All smoke tests passed!"

sdk/go/mark_read_body_test.go

@@ -0,0 +1,50 @@
+package openwa
+
+import (
+	"context"
+	"strings"
+	"testing"
+)
+
+// The three states of messageIds differ on the wire, and a plain []string with omitempty could not
+// tell two of them apart: an empty slice was dropped, so a caller asking for nothing to be
+// acknowledged silently acknowledged the newest message instead. The pointer makes "absent" and
+// "empty" distinct, and this pins all three so the tag cannot quietly go back.
+func TestMarkReadBodyDistinguishesAbsentFromEmpty(t *testing.T) {
+	empty := []string{}
+	named := []string{"3EB0C767D26B8A3F1A2B"}
+
+	cases := []struct {
+		name string
+		body MarkChatReadRequest
+		want string
+	}{
+		{"absent omits the key", MarkChatReadRequest{ChatID: "628123@c.us"}, `{"chatId":"628123@c.us"}`},
+		{"empty sends []", MarkChatReadRequest{ChatID: "628123@c.us", MessageIDs: &empty}, `"messageIds":[]`},
+		{"named sends the ids", MarkChatReadRequest{ChatID: "628123@c.us", MessageIDs: &named}, `"messageIds":["3EB0C767D26B8A3F1A2B"]`},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			rt := &recordTransport{status: 200, body: `{"success":true}`}
+			c := newTestClient(t, rt)
+
+			if _, err := c.Chats.MarkRead(context.Background(), "s1", tc.body); err != nil {
+				t.Fatalf("MarkRead: %v", err)
+			}
+			if got := string(rt.lastRaw); !strings.Contains(got, tc.want) {
+				t.Fatalf("body = %s, want it to contain %s", got, tc.want)
+			}
+		})
+	}
+
+	// The absent case must ALSO not carry the key at all, which "contains" cannot express.
+	rt := &recordTransport{status: 200, body: `{"success":true}`}
+	c := newTestClient(t, rt)
+	if _, err := c.Chats.MarkRead(context.Background(), "s1", MarkChatReadRequest{ChatID: "628123@c.us"}); err != nil {
+		t.Fatalf("MarkRead: %v", err)
+	}
+	if strings.Contains(string(rt.lastRaw), "messageIds") {
+		t.Fatalf("absent body must omit messageIds entirely, got %s", rt.lastRaw)
+	}
+}

sdk/go/types_chat.go

@@ -35,7 +35,13 @@ type MarkChatReadRequest struct {
 	// MessageIDs are the messages to acknowledge (at most 100; an empty list is refused). Baileys
 	// acknowledges individual messages, so without this only the newest message the engine still
 	// holds in memory gets a receipt. Ignored by whatsapp-web.js, whose own sendSeen is chat-level.
-	MessageIDs []string `json:"messageIds,omitempty"`
+	//
+	// A POINTER because the three states differ on the wire and a plain slice cannot tell two of
+	// them apart: nil omits the key (acknowledge the newest), &[]string{} sends [] (the server
+	// refuses it with a 400), and a populated slice names the messages. With `[]string` plus
+	// omitempty an empty slice was dropped, so a caller asking for nothing to be acknowledged
+	// silently acknowledged the newest message instead.
+	MessageIDs *[]string `json:"messageIds,omitempty"`
 }
 
 // ChatState is the typing indicator a chat shows.

src/engine/adapters/baileys-contacts.ts

@@ -355,7 +355,9 @@ export class BaileysContacts {
    * synthesised key, which is what the 1:1 case ran on before.
    */
   private async receiptKeys(chatId: string, messageIds?: string[]): Promise<WAMessageKey[]> {
-    if (messageIds === undefined) {
+    // null as well as undefined: the REST body rejects an explicit null, but this is the engine
+    // boundary and an internal caller reaching it with one used to dereference it below as a 500.
+    if (messageIds === undefined || messageIds === null) {
       const last = this.host.lastMessage(chatId);
       return last ? [last.key] : [];
     }
@@ -364,7 +366,17 @@ export class BaileysContacts {
     }
     const remoteJid = this.host.toEngineJid(chatId);
     const stored = (await this.host.getStoredMessages(messageIds)) ?? [];
-    const keyById = new Map(stored.filter(msg => msg.key?.id).map(msg => [msg.key.id as string, msg.key]));
+    // A stored key is only usable when it belongs to THIS chat. Without the check, an id from
+    // another chat in the same session carried that chat's remoteJid into readMessages, so the
+    // receipt landed there while the route answered success for the chat the caller named. Both
+    // sides fold through toEngineJid so the @c.us and @s.whatsapp.net spellings of one chat still
+    // match; anything that still differs falls back to the synthesised key for the ADDRESSED chat,
+    // which is exactly what every id ran on before stored keys existed.
+    const keyById = new Map(
+      stored
+        .filter(msg => msg.key?.id && msg.key.remoteJid && this.host.toEngineJid(msg.key.remoteJid) === remoteJid)
+        .map(msg => [msg.key.id as string, msg.key]),
+    );
     return messageIds.map(id => keyById.get(id) ?? { remoteJid, id, fromMe: false });
   }
 

src/engine/adapters/baileys-send-seen.spec.ts

@@ -69,6 +69,48 @@ describe('sendSeen', () => {
     ]);
   });
 
+  it('never sends the receipt to another chat, even when the store resolves the id', async () => {
+    // The stored key carries its own remoteJid. Used unchecked, an id belonging to a different chat
+    // in the same session acknowledged THAT chat while the route answered success for the one the
+    // caller named, and the caller's own chat stayed unread.
+    const readMessages = jest.fn().mockResolvedValue(undefined);
+    const host = makeHost({
+      getSocket: () => ({ readMessages }) as unknown as WASocket,
+      getStoredMessages: () =>
+        Promise.resolve([stored({ id: 'X1', remoteJid: '628999@s.whatsapp.net', fromMe: false })]),
+    });
+
+    await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', ['X1'])).resolves.toBe(true);
+
+    // Falls back to the synthesised key for the ADDRESSED chat rather than the stored one.
+    expect(readMessages).toHaveBeenCalledWith([{ remoteJid: '628123@s.whatsapp.net', id: 'X1', fromMe: false }]);
+  });
+
+  it('still uses a stored key whose chat is spelled in the other dialect', async () => {
+    // Control for the check above: the same chat written @c.us must still resolve, or the fix would
+    // have cost every receipt its participant and fromMe.
+    const readMessages = jest.fn().mockResolvedValue(undefined);
+    const host = makeHost({
+      getSocket: () => ({ readMessages }) as unknown as WASocket,
+      getStoredMessages: () => Promise.resolve([stored({ id: 'D1', remoteJid: '628123@c.us', fromMe: true })]),
+    });
+
+    await expect(new BaileysContacts(host, 500).sendSeen('628123@s.whatsapp.net', ['D1'])).resolves.toBe(true);
+    expect(readMessages).toHaveBeenCalledWith([{ id: 'D1', remoteJid: '628123@c.us', fromMe: true }]);
+  });
+
+  it('treats a null id list as absent rather than dereferencing it', async () => {
+    // The REST body rejects an explicit null, but this is the engine boundary; it used to throw a
+    // TypeError here and surface as a 500.
+    const readMessages = jest.fn().mockResolvedValue(undefined);
+    const host = makeHost({ getSocket: () => ({ readMessages }) as unknown as WASocket });
+
+    await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', null as unknown as string[])).resolves.toBe(
+      true,
+    );
+    expect(readMessages).toHaveBeenCalledWith([{ id: 'M1', remoteJid: '628123@s.whatsapp.net' }]);
+  });
+
   it('carries the participant on a group receipt, which a synthesised key cannot', async () => {
     // A group receipt with no `participant` names no sender, so WhatsApp cannot attribute it and
     // the message stays unread on the sender's side while the API answers success: true.

src/modules/infra/infra-data.controller.spec.ts

@@ -1268,6 +1268,38 @@ describe('InfraDataController.import/export preserves every data-DB table', () =
     expect((await lidRepo.findOneByOrFail({ lid: '222' })).phone).toBeNull();
   });
 
+  // Restoring ONTO the instance that produced the archive is the rollback flow, and it is the one the
+  // outbox broke. The table carries no FK to sessions, so the sessions DELETE never reached it, and
+  // UNIQUE(webhookId, idempotencyKey) then collided on every row until the all-or-nothing gate rolled
+  // the entire import back. Every other table's test clears first, which is why nothing caught it;
+  // this one deliberately does not.
+  it('restores webhook_outbox_events onto an instance that already holds them', async () => {
+    await seedSession('s1');
+    const outboxRepo = ds.getRepository(WebhookOutboxEvent);
+    await outboxRepo.save(
+      outboxRepo.create({
+        webhookId: 'wh-1',
+        sessionId: 's1',
+        event: 'message.received',
+        idempotencyKey: 'key-1',
+        deliveryId: 'del-1',
+        payload: { from: '628111@c.us' },
+        state: 'pending',
+        attempts: 0,
+      }),
+    );
+
+    const dump = await controller.exportData();
+    expect((dump.tables as unknown as { webhookOutboxEvents?: unknown[] }).webhookOutboxEvents).toHaveLength(1);
+
+    const res = await controller.importData({ tables: dump.tables });
+
+    expect(res.warnings).toEqual([]);
+    expect(res.imported).toBe(true);
+    expect(await outboxRepo.count()).toBe(1);
+    expect((await outboxRepo.findOneByOrFail({ idempotencyKey: 'key-1' })).state).toBe('pending');
+  });
+
   // The messages import column list must carry every later-added column; `author` (the group
   // sender identity) was the one that drifted — a restored backup silently lost all attribution.
   it('restores the group-sender author column on a backup→restore', async () => {