From 93639e6dcfc9a234c2efc4a3870945e69fd38d2c Mon Sep 17 00:00:00 2001 From: m7fz7 Date: Tue, 18 Aug 2026 18:26:51 +0400 Subject: [PATCH 01/10] feat(session): let callers name which messages a read receipt covers Baileys acknowledges individual messages, not chats, so POST /sessions/{id}/chats/read could only mark the newest message the engine still held in memory. A burst of inbound messages left everything but the last one unread for good, and a session that restarted since the message arrived had nothing to acknowledge at all, answering false under a 200. The request body now takes an optional messageIds array. A caller that persists inbound message IDs names exactly what to acknowledge and is subject to neither case; omitting it keeps the previous last-message behaviour. The list is capped at 100 so one request cannot hand the engine an unbounded key list to round-trip. whatsapp-web.js ignores the field: its sendSeen is chat-level and already marks every message in the chat. Group chats are only partly served: the keys built here carry remoteJid and id but no participant, which Baileys aggregates receipts by, so a supplied id in a group is acknowledged without the sender it belongs to. Direct chats are unaffected, as is the last-message fallback in both, which carries the stored key whole. --- CHANGELOG.md | 4 +++ openapi.json | 11 +++++++ src/engine/adapters/baileys-contacts.ts | 18 +++++++--- src/engine/adapters/baileys-send-seen.spec.ts | 33 +++++++++++++++++++ src/engine/adapters/baileys.adapter.ts | 4 +-- .../adapters/whatsapp-web-js.adapter.ts | 2 ++ .../interfaces/whatsapp-engine.interface.ts | 7 +++- src/modules/session/dto/mark-chat-read.dto.ts | 23 +++++++++++-- src/modules/session/session.controller.ts | 2 +- src/modules/session/session.service.spec.ts | 17 +++++++++- src/modules/session/session.service.ts | 4 +-- 11 files changed, 112 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6be08db98..742f06b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Omitting the field keeps the previous behaviour, and the whatsapp-web.js engine ignores it because its own receipt is chat-level. + ### Fixed - 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. diff --git a/openapi.json b/openapi.json index 8d4b75aae..876cc3f2b 100644 --- a/openapi.json +++ b/openapi.json @@ -10261,6 +10261,17 @@ "type": "string", "description": "Chat ID in the active engine's native format (e.g. 1234567890@c.us on whatsapp-web.js)", "example": "1234567890@c.us" + }, + "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.", + "example": [ + "3EB0C767D26B8A3F1A2B", + "3EB0C767D26B8A3F1A2C" + ], + "type": "array", + "items": { + "type": "string" + } } }, "required": [ diff --git a/src/engine/adapters/baileys-contacts.ts b/src/engine/adapters/baileys-contacts.ts index 19f48e6a0..1abbb3c72 100644 --- a/src/engine/adapters/baileys-contacts.ts +++ b/src/engine/adapters/baileys-contacts.ts @@ -318,16 +318,26 @@ export class BaileysContacts { return this.host.listChats(); } - async sendSeen(chatId: string): Promise { + async sendSeen(chatId: string, messageIds?: string[]): Promise { this.host.ensureReady(); - const last = this.host.lastMessage(chatId); - if (!last) { + // Baileys acknowledges individual messages, not chats. Caller-supplied IDs are what make this + // correct: the in-memory fallback below holds only the newest message, so a burst of three + // inbound messages left the first two permanently unread, and a session that restarted since + // the message arrived had nothing to acknowledge at all (returning a silent false under a 200). + // A caller that persists inbound message IDs has both, so it can name exactly what to mark. + const keys: WAMessageKey[] = messageIds?.length + ? messageIds.map(id => ({ remoteJid: this.host.toEngineJid(chatId), id, fromMe: false })) + : (() => { + const last = this.host.lastMessage(chatId); + return last ? [last.key] : []; + })(); + if (keys.length === 0) { return false; // nothing known to mark read } // readMessages reaches fetchPrivacySettings, which destructures the query result and throws a // raw TypeError on an unanswered one — no Boom, so nothing downstream can classify it. Marking // a chat read is idempotent, so bounding it is safe: a repeat costs nothing. - await this.confirmed(this.sock().readMessages([last.key]), 'the read receipt'); + await this.confirmed(this.sock().readMessages(keys), 'the read receipt'); return true; } diff --git a/src/engine/adapters/baileys-send-seen.spec.ts b/src/engine/adapters/baileys-send-seen.spec.ts index 992a4c658..a72f26624 100644 --- a/src/engine/adapters/baileys-send-seen.spec.ts +++ b/src/engine/adapters/baileys-send-seen.spec.ts @@ -40,6 +40,39 @@ describe('sendSeen', () => { await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us')).resolves.toBe(true); }); + it('acknowledges every supplied message, not just the newest one', async () => { + // The whole point of the caller-supplied list: a burst of three inbound messages used to leave + // the first two unread forever, because only lastMessage() was ever acknowledged. + const readMessages = jest.fn().mockResolvedValue(undefined); + await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us', ['M1', 'M2', 'M3'])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([ + { remoteJid: '628123@c.us', id: 'M1', fromMe: false }, + { remoteJid: '628123@c.us', id: 'M2', fromMe: false }, + { remoteJid: '628123@c.us', id: 'M3', fromMe: false }, + ]); + }); + + it('marks supplied messages even when the store has nothing cached', async () => { + // The restart case: the in-memory store is empty, so the old code returned false under a 200 + // and no receipt was ever sent. A caller that persisted the IDs is not subject to that. + const readMessages = jest.fn().mockResolvedValue(undefined); + const host = { + ensureReady: () => undefined, + getSocket: () => ({ readMessages }) as unknown as WASocket, + logger: { warn: jest.fn(), debug: jest.fn(), info: jest.fn(), error: jest.fn() }, + lastMessage: () => null, + toEngineJid: (j: string) => j, + } as unknown as BaileysContactsHost; + await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', ['M9'])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([{ remoteJid: '628123@c.us', id: 'M9', fromMe: false }]); + }); + + it('falls back to the cached last message when the caller supplies an empty list', async () => { + const readMessages = jest.fn().mockResolvedValue(undefined); + await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us', [])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([{ id: 'M1', remoteJid: '628123@s.whatsapp.net' }]); + }); + it('still short-circuits when there is no last message to mark', async () => { const host = { ensureReady: () => undefined, diff --git a/src/engine/adapters/baileys.adapter.ts b/src/engine/adapters/baileys.adapter.ts index 2f78c75ea..616b10d99 100644 --- a/src/engine/adapters/baileys.adapter.ts +++ b/src/engine/adapters/baileys.adapter.ts @@ -509,8 +509,8 @@ export class BaileysAdapter implements IWhatsAppEngine { return this.messaging.subscribeToPresence(chatId); } - async sendSeen(chatId: string): Promise { - return this.contacts.sendSeen(chatId); + async sendSeen(chatId: string, messageIds?: string[]): Promise { + return this.contacts.sendSeen(chatId, messageIds); } async markUnread(chatId: string): Promise { diff --git a/src/engine/adapters/whatsapp-web-js.adapter.ts b/src/engine/adapters/whatsapp-web-js.adapter.ts index 20d0cdd4f..d6fa18e09 100644 --- a/src/engine/adapters/whatsapp-web-js.adapter.ts +++ b/src/engine/adapters/whatsapp-web-js.adapter.ts @@ -889,6 +889,8 @@ export class WhatsAppWebJsAdapter extends EventEmitter implements IWhatsAppEngin return this.chats.getChats(); } + // messageIds is dropped on purpose: whatsapp-web.js exposes only a chat-level sendSeen, which + // already marks every message in the chat. sendSeen(chatId: string): Promise { return this.chats.sendSeen(chatId); } diff --git a/src/engine/interfaces/whatsapp-engine.interface.ts b/src/engine/interfaces/whatsapp-engine.interface.ts index 2a1ef12e8..7a2557159 100644 --- a/src/engine/interfaces/whatsapp-engine.interface.ts +++ b/src/engine/interfaces/whatsapp-engine.interface.ts @@ -1297,7 +1297,12 @@ export interface CatalogCapability { export interface ChatCapability { getChats(): Promise; - sendSeen(chatId: string): Promise; + /** + * `messageIds` names exactly which messages to acknowledge. Engines that acknowledge per + * message (Baileys) need it to mark a burst, or anything at all after a restart; engines with a + * chat-level receipt (whatsapp-web.js) ignore it. + */ + sendSeen(chatId: string, messageIds?: string[]): Promise; markUnread(chatId: string): Promise; diff --git a/src/modules/session/dto/mark-chat-read.dto.ts b/src/modules/session/dto/mark-chat-read.dto.ts index 08511923e..4755a8d8e 100644 --- a/src/modules/session/dto/mark-chat-read.dto.ts +++ b/src/modules/session/dto/mark-chat-read.dto.ts @@ -1,5 +1,5 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsNotEmpty, IsString, Matches } from 'class-validator'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { ArrayMaxSize, IsArray, IsNotEmpty, IsOptional, IsString, Matches } from 'class-validator'; export class MarkChatReadDto { @ApiProperty({ @@ -15,4 +15,23 @@ export class MarkChatReadDto { message: 'chatId must be a valid chat JID in the form localpart@host', }) chatId!: string; + + @ApiPropertyOptional({ + 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.', + type: [String], + example: ['3EB0C767D26B8A3F1A2B', '3EB0C767D26B8A3F1A2C'], + }) + @IsOptional() + @IsArray() + // Bounded so one request cannot hand the engine an unbounded key list to round-trip; a caller with + // more than this to acknowledge is catching up on history, not marking a conversation read. + @ArrayMaxSize(100) + @IsString({ each: true }) + @IsNotEmpty({ each: true }) + messageIds?: string[]; } diff --git a/src/modules/session/session.controller.ts b/src/modules/session/session.controller.ts index b43955db4..f0e62ee16 100644 --- a/src/modules/session/session.controller.ts +++ b/src/modules/session/session.controller.ts @@ -469,7 +469,7 @@ export class SessionController { @Param('sessionId', ParseUUIDPipe) id: string, @Body() dto: MarkChatReadDto, ): Promise<{ success: boolean }> { - const success = await this.sessionService.sendSeen(id, dto.chatId); + const success = await this.sessionService.sendSeen(id, dto.chatId, dto.messageIds); return { success }; } diff --git a/src/modules/session/session.service.spec.ts b/src/modules/session/session.service.spec.ts index 657f619d6..73713e650 100644 --- a/src/modules/session/session.service.spec.ts +++ b/src/modules/session/session.service.spec.ts @@ -5151,10 +5151,25 @@ describe('SessionService', () => { const result = await service.sendSeen('sess-uuid-1', '123@c.us'); - expect(mockEngine.sendSeen).toHaveBeenCalledWith('123@c.us'); + // Second argument is the optional messageIds, absent here — the engine falls back to the + // newest message it still holds. + expect(mockEngine.sendSeen).toHaveBeenCalledWith('123@c.us', undefined); expect(result).toBe(true); }); + it('should forward caller-supplied message IDs to the engine', async () => { + const session = createMockSession(); + (repository.findOne as jest.Mock).mockResolvedValue(session); + (repository.update as jest.Mock).mockResolvedValue({ affected: 1 }); + + await service.start('sess-uuid-1'); + mockEngine.sendSeen.mockResolvedValue(true); + + await service.sendSeen('sess-uuid-1', '123@c.us', ['M1', 'M2']); + + expect(mockEngine.sendSeen).toHaveBeenCalledWith('123@c.us', ['M1', 'M2']); + }); + it('should throw BadRequestException when session is not started', async () => { const session = createMockSession(); (repository.findOne as jest.Mock).mockResolvedValue(session); diff --git a/src/modules/session/session.service.ts b/src/modules/session/session.service.ts index f0cce08ea..3c48ff734 100644 --- a/src/modules/session/session.service.ts +++ b/src/modules/session/session.service.ts @@ -664,11 +664,11 @@ export class SessionService implements OnModuleDestroy, OnModuleInit, OnApplicat return this.presence.get(id, chatId); } - async sendSeen(id: string, chatId: string): Promise { + async sendSeen(id: string, chatId: string, messageIds?: string[]): Promise { await this.findOne(id); // Verify session exists const engine = this.requireEngine(id); - return engine.sendSeen(chatId); + return engine.sendSeen(chatId, messageIds); } async markUnread(id: string, chatId: string): Promise { From 4d7f98799e201171f57d959372eacc7d0f8a4bf6 Mon Sep 17 00:00:00 2001 From: m7fz7 Date: Wed, 19 Aug 2026 13:44:12 +0400 Subject: [PATCH 02/10] fix(session): resolve read receipts through the message store Address review on the messageIds PR. A synthesised receipt key carried no participant, so a group receipt named no sender and WhatsApp could not attribute it while the API answered success: true. Its hardcoded fromMe was also wrong for an outbound id, and its jid was whichever dialect the caller happened to send. Each supplied id is now resolved through the message store and the stored key is used whole. Ids the store has never seen - history backfill is emitted but not persisted - keep the synthesised key, which is what the 1:1 case ran on before. The store gains a batched getMessages: the receipt path resolves up to a hundred ids per request, and a findOne apiece was a hundred sequential round trips. POST /chats/unread gets its own MarkChatUnreadDto instead of sharing MarkChatReadDto. Sharing published messageIds on a route that discards it. The JS SDK splits the same way: MarkChatReadRequest for markRead, with MarkChatRequest left to markUnread and subscribePresence, which is served by a chatId-only DTO and would have taken a 400 on the extra field under forbidNonWhitelisted. check-contract-shapes maps both pairs, taking the JavaScript floor from 78 to 79. An empty messageIds array is refused rather than read as "acknowledge the newest message", so a caller that computed an empty unread set no longer acknowledges a message it never named. The cap is hoisted to MARK_READ_MESSAGE_IDS_MAX and published as maxItems, since @ArrayMaxSize is runtime-only and the schema advertised an unbounded array. Entries are matched against a non-whitespace token, because @IsNotEmpty accepted ' '. The send-seen spec stubs the real @c.us to @s.whatsapp.net fold instead of an identity function, which had let the whole suite pass with the toEngineJid call deleted. The DTO spec covers the four new validators, and SessionMarkChatRead forwards the list to MCP callers. --- CHANGELOG.md | 6 +- docs/06-api-specification.md | 15 ++- docs/07-api-collection.md | 5 +- openapi.json | 16 ++- scripts/check-contract-shapes.mjs | 5 +- sdk/javascript/src/resources/chats.ts | 3 +- sdk/javascript/src/types.ts | 13 ++ .../agent-tools/tools/session.tools.spec.ts | 12 +- src/core/agent-tools/tools/session.tools.ts | 13 +- src/engine/adapters/baileys-contacts.ts | 51 ++++++-- .../baileys-message-store.service.spec.ts | 33 +++++ .../adapters/baileys-message-store.service.ts | 17 ++- src/engine/adapters/baileys-send-seen.spec.ts | 116 ++++++++++++++---- src/engine/adapters/baileys.adapter.spec.ts | 1 + src/engine/adapters/baileys.adapter.ts | 1 + src/engine/builtin/baileys/index.spec.ts | 2 +- src/engine/types/baileys.types.ts | 5 + src/modules/session/dto/index.ts | 1 + .../session/dto/mark-chat-read.dto.spec.ts | 34 ++++- src/modules/session/dto/mark-chat-read.dto.ts | 24 +++- .../session/dto/mark-chat-unread.dto.ts | 22 ++++ src/modules/session/session.controller.ts | 3 +- 22 files changed, 341 insertions(+), 57 deletions(-) create mode 100644 src/modules/session/dto/mark-chat-unread.dto.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 742f06b6e..dc865c210 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Omitting the field keeps the previous behaviour, and the whatsapp-web.js engine ignores it because its own receipt is chat-level. +- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Each id is resolved through the message store, so a group receipt carries the `participant` that attributes it. Omitting the field keeps the previous behaviour, an empty array is refused with a `400` rather than silently acknowledging the newest message, and the whatsapp-web.js engine ignores the field because its own receipt is chat-level. The `SessionMarkChatRead` agent tool takes the same list. + +### Changed + +- `POST /sessions/{sessionId}/chats/unread` publishes its own `MarkChatUnreadDto` instead of sharing `MarkChatReadDto` with the read route. The request body is unchanged (`chatId` alone), but a client generated from the contract sees the schema under a new name. Sharing the class would have advertised `messageIds` on a route that discards it. ### Fixed diff --git a/docs/06-api-specification.md b/docs/06-api-specification.md index 41d036c0f..2a0dada86 100644 --- a/docs/06-api-specification.md +++ b/docs/06-api-specification.md @@ -836,12 +836,15 @@ Mark a chat as read/seen. **Request body** — `MarkChatReadDto` -| Field | Type | Required | Constraints | Description | -| -------- | ------ | -------- | ------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | -| `chatId` | string | Yes | `@IsString`; `@IsNotEmpty`; `@Matches(/^[^\s@]+@[^\s@]+$/)` (localpart@host, no whitespace) | Engine-native JID, e.g. `1234567890@c.us` (wwebjs) or `1234@s.whatsapp.net` (Baileys) | +| Field | Type | Required | Constraints | Description | +| ------------ | -------- | -------- | --------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | +| `chatId` | string | Yes | `@IsString`; `@IsNotEmpty`; `@Matches(/^[^\s@]+@[^\s@]+$/)` (localpart@host, no whitespace) | Engine-native JID, e.g. `1234567890@c.us` (wwebjs) or `1234@s.whatsapp.net` (Baileys) | +| `messageIds` | string[] | No | `@IsArray`; `@ArrayNotEmpty`; `@ArrayMaxSize(100)`; each a non-empty token with no whitespace | Messages to acknowledge. Omit the field to acknowledge only the newest message; an empty array is rejected. | + +Baileys acknowledges individual messages rather than chats, and the receipt enumerates ids instead of carrying a read-up-to watermark. Without `messageIds` only the newest message the engine still holds in memory gets a receipt, so a burst leaves its earlier messages unread and a session restarted since the message arrived has nothing to acknowledge at all. Each supplied id is resolved through the message store, which is what carries the `participant` a group receipt needs. Ignored by whatsapp-web.js, whose own `sendSeen` is chat-level. ```json -{ "chatId": "1234567890@c.us" } +{ "chatId": "1234567890@c.us", "messageIds": ["3EB0C767D26B8A3F1A2B", "3EB0C767D26B8A3F1A2C"] } ``` **Response** `200` @@ -871,12 +874,14 @@ Mark a chat as unread. | ----------- | ------ | ------------ | | `sessionId` | string | Session UUID | -**Request body** — `MarkChatReadDto` (reused) +**Request body** — `MarkChatUnreadDto` | Field | Type | Required | Constraints | Description | | -------- | ------ | -------- | ----------------------------------------------------------- | ----------------------------------------- | | `chatId` | string | Yes | `@IsString`; `@IsNotEmpty`; `@Matches(/^[^\s@]+@[^\s@]+$/)` | Engine-native JID, e.g. `1234567890@c.us` | +This route takes `chatId` alone. It previously shared `MarkChatReadDto`, which is why the two are described separately now that the read body carries `messageIds`. + ```json { "chatId": "1234567890@c.us" } ``` diff --git a/docs/07-api-collection.md b/docs/07-api-collection.md index 0f047c9e5..5e1acd41f 100644 --- a/docs/07-api-collection.md +++ b/docs/07-api-collection.md @@ -214,9 +214,12 @@ Mark a chat as read/seen (OPERATOR). curl -X POST "$BASE/api/sessions/8f3c2b1a-9d4e-4c7a-8b2f-1e6d5a4c3b2a/chats/read" \ -H "X-API-Key: $API_KEY" \ -H "Content-Type: application/json" \ - -d '{ "chatId": "1234567890@c.us" }' + -d '{ "chatId": "1234567890@c.us", "messageIds": ["3EB0C767D26B8A3F1A2B"] }' ``` +`messageIds` is optional and holds up to 100 ids. Omit it and only the newest message the engine still +holds in memory is acknowledged, which on Baileys leaves the earlier messages of a burst unread. + #### POST /api/sessions/:sessionId/chats/unread Mark a chat as unread (OPERATOR). diff --git a/openapi.json b/openapi.json index 876cc3f2b..fcd120859 100644 --- a/openapi.json +++ b/openapi.json @@ -1237,7 +1237,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/MarkChatReadDto" + "$ref": "#/components/schemas/MarkChatUnreadDto" } } } @@ -10264,6 +10264,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, "example": [ "3EB0C767D26B8A3F1A2B", "3EB0C767D26B8A3F1A2C" @@ -10379,6 +10380,19 @@ "observedAt" ] }, + "MarkChatUnreadDto": { + "type": "object", + "properties": { + "chatId": { + "type": "string", + "description": "Chat ID in the active engine's native format (e.g. 1234567890@c.us on whatsapp-web.js)", + "example": "1234567890@c.us" + } + }, + "required": [ + "chatId" + ] + }, "ArchiveChatDto": { "type": "object", "properties": { diff --git a/scripts/check-contract-shapes.mjs b/scripts/check-contract-shapes.mjs index 48ef83c5e..534ec0e4f 100644 --- a/scripts/check-contract-shapes.mjs +++ b/scripts/check-contract-shapes.mjs @@ -85,7 +85,8 @@ const MAPPINGS = { GroupSubjectRequest: 'GroupSubjectDto', GroupSummary: 'GroupSummaryDto', JoinGroupRequest: 'JoinGroupDto', - MarkChatRequest: 'MarkChatReadDto', + MarkChatReadRequest: 'MarkChatReadDto', + MarkChatRequest: 'MarkChatUnreadDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', @@ -164,7 +165,7 @@ const MAPPINGS = { * these floors as pairs are added makes the shrink loud. */ const MINIMUM_MAPPED = { - 'sdk/javascript/src/types.ts': 78, + 'sdk/javascript/src/types.ts': 79, 'dashboard/src/services/api.ts': 20, 'sdk/python/openwa/types.py': 73, 'sdk/go': 74, diff --git a/sdk/javascript/src/resources/chats.ts b/sdk/javascript/src/resources/chats.ts index bece30025..61da4559b 100644 --- a/sdk/javascript/src/resources/chats.ts +++ b/sdk/javascript/src/resources/chats.ts @@ -17,6 +17,7 @@ import type { ChatSummary, DeleteChatRequest, MarkChatRequest, + MarkChatReadRequest, ChatPresence, SendChatStateRequest, SuccessResult, @@ -67,7 +68,7 @@ export class ChatsResource { } /** Mark a chat as read/seen. */ - markRead(sessionId: string, body: MarkChatRequest): Promise { + markRead(sessionId: string, body: MarkChatReadRequest): Promise { return this.client.request({ method: 'POST', path: `/api/sessions/${encodeSegment(sessionId)}/chats/read`, diff --git a/sdk/javascript/src/types.ts b/sdk/javascript/src/types.ts index f940b305c..a77e31041 100644 --- a/sdk/javascript/src/types.ts +++ b/sdk/javascript/src/types.ts @@ -968,10 +968,23 @@ export interface TransferChannelOwnershipRequest { newOwnerId: Jid; } +/** Body for {@link ChatsResource.markUnread} and {@link ChatsResource.subscribePresence}. */ export interface MarkChatRequest { chatId: Jid; } +/** Body for {@link ChatsResource.markRead}. */ +export interface MarkChatReadRequest extends MarkChatRequest { + /** + * Specific message IDs to acknowledge. 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. At most 100 per request; an empty array is rejected. + */ + messageIds?: string[]; +} + export type ChatState = 'typing' | 'recording' | 'paused'; export interface SendChatStateRequest { diff --git a/src/core/agent-tools/tools/session.tools.spec.ts b/src/core/agent-tools/tools/session.tools.spec.ts index 5a001b0e9..84cbf657b 100644 --- a/src/core/agent-tools/tools/session.tools.spec.ts +++ b/src/core/agent-tools/tools/session.tools.spec.ts @@ -90,10 +90,20 @@ describe('sessionTools', () => { sessionId: 's1', chatId: '628111@c.us', }); - expect(sendSeen).toHaveBeenCalledWith('s1', '628111@c.us'); + expect(sendSeen).toHaveBeenCalledWith('s1', '628111@c.us', undefined); expect(out).toEqual({ success: true }); }); + it('SessionMarkChatRead forwards the message ids it was given', async () => { + const sendSeen = jest.fn().mockResolvedValue(true); + await run(makeTools({ sendSeen } as unknown as SessionService).get('SessionMarkChatRead')!, { + sessionId: 's1', + chatId: '628111@c.us', + messageIds: ['M1', 'M2'], + }); + expect(sendSeen).toHaveBeenCalledWith('s1', '628111@c.us', ['M1', 'M2']); + }); + it('SessionMarkChatUnread maps the markUnread result to a success field', async () => { const markUnread = jest.fn().mockResolvedValue(true); const out = await run(makeTools({ markUnread } as unknown as SessionService).get('SessionMarkChatUnread')!, { diff --git a/src/core/agent-tools/tools/session.tools.ts b/src/core/agent-tools/tools/session.tools.ts index 0d9767a52..916d203e8 100644 --- a/src/core/agent-tools/tools/session.tools.ts +++ b/src/core/agent-tools/tools/session.tools.ts @@ -2,6 +2,7 @@ import { z } from 'zod'; import { ApiKeyRole } from '../../../modules/auth/entities/api-key.entity'; import type { SessionService } from '../../../modules/session/session.service'; import { SessionResponseDto } from '../../../modules/session/dto/session-response.dto'; +import { MARK_READ_MESSAGE_IDS_MAX } from '../../../modules/session/dto/mark-chat-read.dto'; import { defineTool, type AnyToolDescriptor } from '../tool-descriptor'; const sessionId = z.string().min(1).describe('Session UUID (the session id, not the name)'); @@ -89,8 +90,18 @@ export function sessionTools(session: SessionService): AnyToolDescriptor[] { inputSchema: z.object({ sessionId, chatId: z.string().describe('Chat JID (e.g. 1234567890@c.us)'), + messageIds: z + .array(z.string()) + .nonempty() + .max(MARK_READ_MESSAGE_IDS_MAX) + .optional() + .describe( + 'Specific message IDs to acknowledge. Baileys acknowledges individual messages, so without ' + + 'this only the newest message still held in memory gets a receipt.', + ), }), - handler: input => session.sendSeen(input.sessionId, input.chatId).then(success => ({ success })), + handler: input => + session.sendSeen(input.sessionId, input.chatId, input.messageIds).then(success => ({ success })), }), defineTool({ name: 'SessionMarkChatUnread', diff --git a/src/engine/adapters/baileys-contacts.ts b/src/engine/adapters/baileys-contacts.ts index 1abbb3c72..c176bef37 100644 --- a/src/engine/adapters/baileys-contacts.ts +++ b/src/engine/adapters/baileys-contacts.ts @@ -1,4 +1,4 @@ -import type { WAMessageKey, WASocket } from '@whiskeysockets/baileys'; +import type { WAMessage, WAMessageKey, WASocket } from '@whiskeysockets/baileys'; import { ChatSummary, Contact, MediaInput } from '../interfaces/whatsapp-engine.interface'; import { resolveMediaBuffer } from './baileys-messaging'; import { type createLogger } from '../../common/services/logger.service'; @@ -23,6 +23,12 @@ export interface BaileysContactsHost { listChats(): ChatSummary[]; /** The chat's last known message (the handle readMessages/chatModify need), or null when none. */ lastMessage(chatId: string): { key: WAMessageKey; timestamp: number } | null; + /** + * Stored copies of the named messages, in whatever order the store returns them. Ids the store + * has never seen are absent, so neither the length nor the order tracks the input. `undefined` + * when the session was built without a message store. + */ + getStoredMessages(messageIds: string[]): Promise | undefined; /** Fold a neutral @c.us id to the engine @s.whatsapp.net form used as the app-state index key. */ toEngineJid(jid: string): string; /** Fold an engine jid back to the neutral dialect before it crosses the engine boundary. */ @@ -320,17 +326,7 @@ export class BaileysContacts { async sendSeen(chatId: string, messageIds?: string[]): Promise { this.host.ensureReady(); - // Baileys acknowledges individual messages, not chats. Caller-supplied IDs are what make this - // correct: the in-memory fallback below holds only the newest message, so a burst of three - // inbound messages left the first two permanently unread, and a session that restarted since - // the message arrived had nothing to acknowledge at all (returning a silent false under a 200). - // A caller that persists inbound message IDs has both, so it can name exactly what to mark. - const keys: WAMessageKey[] = messageIds?.length - ? messageIds.map(id => ({ remoteJid: this.host.toEngineJid(chatId), id, fromMe: false })) - : (() => { - const last = this.host.lastMessage(chatId); - return last ? [last.key] : []; - })(); + const keys = await this.receiptKeys(chatId, messageIds); if (keys.length === 0) { return false; // nothing known to mark read } @@ -341,6 +337,37 @@ export class BaileysContacts { return true; } + /** + * The keys a read receipt should acknowledge: the messages the caller named, or the chat's newest + * one when it named none. + * + * Baileys acknowledges individual messages, not chats, and the receipt node enumerates ids rather + * than carrying a read-up-to watermark. Caller-supplied ids are what make that correct: the + * lastMessage fallback holds only the newest message, so a burst of three inbound messages left + * the first two permanently unread, and a session that restarted since the message arrived had + * nothing to acknowledge at all (a silent false under a 200). + * + * Named ids are resolved through the message store rather than synthesised, because the receipt + * needs the whole key. A synthesised key carries no `participant`, so a group receipt names no + * sender; its hardcoded `fromMe: false` is wrong for an id that belongs to an outbound message; + * and its jid is whichever dialect the caller happened to send. The stored key has all three + * right. Ids the store has never seen — history backfill is emitted but not persisted — keep the + * synthesised key, which is what the 1:1 case ran on before. + */ + private async receiptKeys(chatId: string, messageIds?: string[]): Promise { + if (messageIds === undefined) { + const last = this.host.lastMessage(chatId); + return last ? [last.key] : []; + } + if (messageIds.length === 0) { + return []; // an explicit empty list asks for nothing to be acknowledged, not for the newest + } + 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])); + return messageIds.map(id => keyById.get(id) ?? { remoteJid, id, fromMe: false }); + } + async markUnread(chatId: string): Promise { this.host.ensureReady(); const last = this.host.lastMessage(chatId); diff --git a/src/engine/adapters/baileys-message-store.service.spec.ts b/src/engine/adapters/baileys-message-store.service.spec.ts index 65e5fa167..273f43a96 100644 --- a/src/engine/adapters/baileys-message-store.service.spec.ts +++ b/src/engine/adapters/baileys-message-store.service.spec.ts @@ -181,6 +181,39 @@ describe('BaileysMessageStoreService', () => { await expect(service.put('s1', msg('M1'))).rejects.toThrow('disk full'); }); + describe('getMessages', () => { + it('returns the batch in one query, skipping ids it has never seen', async () => { + await seedSession('s1'); + await service.put('s1', msg('M1')); + await service.put('s1', msg('M2')); + const found = await service.getMessages('s1', ['M1', 'MISSING', 'M2']); + expect(found.map(m => m.key.id).sort()).toEqual(['M1', 'M2']); + }); + + it('stays scoped to its own session', async () => { + await seedSession('s1'); + await seedSession('s2'); + await service.put('s2', msg('M1')); + expect(await service.getMessages('s1', ['M1'])).toEqual([]); + }); + + it('short-circuits on an empty or all-falsy id list instead of querying', async () => { + // An empty In() clause is a SQL syntax error on some drivers and matches everything on others. + const find = jest.spyOn(repo, 'find'); + expect(await service.getMessages('s1', [])).toEqual([]); + expect(await service.getMessages('s1', [''])).toEqual([]); + expect(find).not.toHaveBeenCalled(); + }); + + it('round-trips binary fields the same way getMessage does', async () => { + await seedSession('s1'); + await service.put('s1', msg('M1')); + const [found] = await service.getMessages('s1', ['M1']); + // mediaKey is off the public WAMessage type, like the fixture that wrote it. + expect(Buffer.isBuffer((found as unknown as { mediaKey: unknown }).mediaKey)).toBe(true); + }); + }); + it('clearSession removes only that session', async () => { await seedSession('s1'); await seedSession('s2'); diff --git a/src/engine/adapters/baileys-message-store.service.ts b/src/engine/adapters/baileys-message-store.service.ts index 1f4bb623d..5d3d210ca 100644 --- a/src/engine/adapters/baileys-message-store.service.ts +++ b/src/engine/adapters/baileys-message-store.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { In, Repository } from 'typeorm'; import type * as BaileysLib from '@whiskeysockets/baileys'; import type { WAMessage } from '@whiskeysockets/baileys'; import { BaileysStoredMessage } from './baileys-stored-message.entity'; @@ -99,6 +99,21 @@ export class BaileysMessageStoreService implements BaileysMessageStore { return JSON.parse(row.serializedMessage, BufferJSON.reviver) as WAMessage; } + async getMessages(sessionId: string, messageIds: string[]): Promise { + // One query for the whole batch: the read-receipt path resolves up to a hundred ids at a time, + // and a findOne apiece would be a hundred sequential round trips for a single request. + const ids = messageIds.filter(Boolean); + if (ids.length === 0) { + return []; + } + const rows = await this.repo.find({ where: { sessionId, waMessageId: In(ids) } }); + if (rows.length === 0) { + return []; + } + const { BufferJSON } = await this.loadLib(); + return rows.map(row => JSON.parse(row.serializedMessage, BufferJSON.reviver) as WAMessage); + } + async clearSession(sessionId: string): Promise { await this.repo.delete({ sessionId }); } diff --git a/src/engine/adapters/baileys-send-seen.spec.ts b/src/engine/adapters/baileys-send-seen.spec.ts index a72f26624..b9e735399 100644 --- a/src/engine/adapters/baileys-send-seen.spec.ts +++ b/src/engine/adapters/baileys-send-seen.spec.ts @@ -1,4 +1,4 @@ -import type { WASocket } from '@whiskeysockets/baileys'; +import type { WAMessage, WASocket } from '@whiskeysockets/baileys'; import { BaileysContacts, BaileysContactsHost } from './baileys-contacts'; import { EngineTransportError } from '../../common/errors/engine-transport.error'; @@ -12,10 +12,19 @@ import { EngineTransportError } from '../../common/errors/engine-transport.error */ const never = (): Promise => new Promise(() => undefined); -function contacts(sock: Record, budgetMs: number): BaileysContacts { - const host = { +/** + * The real fold, not an identity stub. A stub that returned its argument unchanged let the whole + * suite pass with the `toEngineJid` call deleted outright, and left the fallback case asserting + * `@s.whatsapp.net` while the supplied-id cases asserted `@c.us` for the same chat. + */ +const toEngineJid = (jid: string): string => { + const [userPart, host] = jid.split('@'); + return host === 'c.us' || host === 's.whatsapp.net' ? `${userPart}@s.whatsapp.net` : jid; +}; + +function makeHost(overrides: Partial>): BaileysContactsHost { + return { ensureReady: () => undefined, - getSocket: () => sock as unknown as WASocket, logger: { warn: jest.fn(), debug: jest.fn(), info: jest.fn(), error: jest.fn() }, normalizedSelfJid: () => '628177@s.whatsapp.net', listContacts: () => [], @@ -23,11 +32,19 @@ function contacts(sock: Record, budgetMs: number): BaileysCon resolvePhone: () => null, listChats: () => [], lastMessage: () => ({ key: { id: 'M1', remoteJid: '628123@s.whatsapp.net' }, timestamp: 1 }), - toEngineJid: (j: string) => j, + getStoredMessages: () => Promise.resolve([]), + toEngineJid, + ...overrides, } as unknown as BaileysContactsHost; - return new BaileysContacts(host, budgetMs); } +function contacts(sock: Record, budgetMs: number): BaileysContacts { + return new BaileysContacts(makeHost({ getSocket: () => sock as unknown as WASocket }), budgetMs); +} + +/** A stored message as the message store hands it back: the whole key, not a synthesised one. */ +const stored = (key: WAMessage['key']): WAMessage => ({ key, message: {}, messageTimestamp: 1 }); + describe('sendSeen', () => { it('reports an unanswered read receipt instead of a bare 500', async () => { await expect(contacts({ readMessages: jest.fn(never) }, 15).sendSeen('628123@c.us')).rejects.toBeInstanceOf( @@ -46,9 +63,53 @@ describe('sendSeen', () => { const readMessages = jest.fn().mockResolvedValue(undefined); await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us', ['M1', 'M2', 'M3'])).resolves.toBe(true); expect(readMessages).toHaveBeenCalledWith([ - { remoteJid: '628123@c.us', id: 'M1', fromMe: false }, - { remoteJid: '628123@c.us', id: 'M2', fromMe: false }, - { remoteJid: '628123@c.us', id: 'M3', fromMe: false }, + { remoteJid: '628123@s.whatsapp.net', id: 'M1', fromMe: false }, + { remoteJid: '628123@s.whatsapp.net', id: 'M2', fromMe: false }, + { remoteJid: '628123@s.whatsapp.net', id: 'M3', fromMe: false }, + ]); + }); + + 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. + const readMessages = jest.fn().mockResolvedValue(undefined); + const host = makeHost({ + getSocket: () => ({ readMessages }) as unknown as WASocket, + getStoredMessages: () => + Promise.resolve([ + stored({ id: 'G1', remoteJid: '628999-1@g.us', fromMe: false, participant: '628123@s.whatsapp.net' }), + ]), + }); + await expect(new BaileysContacts(host, 500).sendSeen('628999-1@g.us', ['G1'])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([ + { id: 'G1', remoteJid: '628999-1@g.us', fromMe: false, participant: '628123@s.whatsapp.net' }, + ]); + }); + + it('takes fromMe from the stored key rather than assuming inbound', async () => { + const readMessages = jest.fn().mockResolvedValue(undefined); + const host = makeHost({ + getSocket: () => ({ readMessages }) as unknown as WASocket, + getStoredMessages: () => + Promise.resolve([stored({ id: 'O1', remoteJid: '628123@s.whatsapp.net', fromMe: true })]), + }); + await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', ['O1'])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([{ id: 'O1', remoteJid: '628123@s.whatsapp.net', fromMe: true }]); + }); + + it('synthesises a key only for ids the store has never seen', async () => { + // History-sync backfill is emitted but never persisted, so a legitimate id can miss. Falling + // back keeps the 1:1 case working instead of silently dropping the message from the receipt. + const readMessages = jest.fn().mockResolvedValue(undefined); + const host = makeHost({ + getSocket: () => ({ readMessages }) as unknown as WASocket, + getStoredMessages: () => + Promise.resolve([stored({ id: 'K1', remoteJid: '628123@s.whatsapp.net', fromMe: true })]), + }); + await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', ['K1', 'MISSING'])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([ + { id: 'K1', remoteJid: '628123@s.whatsapp.net', fromMe: true }, + { remoteJid: '628123@s.whatsapp.net', id: 'MISSING', fromMe: false }, ]); }); @@ -56,30 +117,37 @@ describe('sendSeen', () => { // The restart case: the in-memory store is empty, so the old code returned false under a 200 // and no receipt was ever sent. A caller that persisted the IDs is not subject to that. const readMessages = jest.fn().mockResolvedValue(undefined); - const host = { - ensureReady: () => undefined, + const host = makeHost({ getSocket: () => ({ readMessages }) as unknown as WASocket, lastMessage: () => null }); + await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', ['M9'])).resolves.toBe(true); + expect(readMessages).toHaveBeenCalledWith([{ remoteJid: '628123@s.whatsapp.net', id: 'M9', fromMe: false }]); + }); + + it('works without a message store at all', async () => { + const readMessages = jest.fn().mockResolvedValue(undefined); + const host = makeHost({ getSocket: () => ({ readMessages }) as unknown as WASocket, - logger: { warn: jest.fn(), debug: jest.fn(), info: jest.fn(), error: jest.fn() }, - lastMessage: () => null, - toEngineJid: (j: string) => j, - } as unknown as BaileysContactsHost; + getStoredMessages: () => undefined, + }); await expect(new BaileysContacts(host, 500).sendSeen('628123@c.us', ['M9'])).resolves.toBe(true); - expect(readMessages).toHaveBeenCalledWith([{ remoteJid: '628123@c.us', id: 'M9', fromMe: false }]); + expect(readMessages).toHaveBeenCalledWith([{ remoteJid: '628123@s.whatsapp.net', id: 'M9', fromMe: false }]); }); - it('falls back to the cached last message when the caller supplies an empty list', async () => { + it('acknowledges nothing when the caller supplies an empty list', async () => { + // An empty list is a caller that computed its unread set and found none. Folding it into the + // no-list branch would acknowledge the newest message, which is the opposite of the request. const readMessages = jest.fn().mockResolvedValue(undefined); - await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us', [])).resolves.toBe(true); + await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us', [])).resolves.toBe(false); + expect(readMessages).not.toHaveBeenCalled(); + }); + + it('falls back to the cached last message when no list is supplied', async () => { + const readMessages = jest.fn().mockResolvedValue(undefined); + await expect(contacts({ readMessages }, 500).sendSeen('628123@c.us')).resolves.toBe(true); expect(readMessages).toHaveBeenCalledWith([{ id: 'M1', remoteJid: '628123@s.whatsapp.net' }]); }); it('still short-circuits when there is no last message to mark', async () => { - const host = { - ensureReady: () => undefined, - getSocket: () => ({}) as unknown as WASocket, - logger: { warn: jest.fn(), debug: jest.fn(), info: jest.fn(), error: jest.fn() }, - lastMessage: () => null, - } as unknown as BaileysContactsHost; + const host = makeHost({ getSocket: () => ({}) as unknown as WASocket, lastMessage: () => null }); await expect(new BaileysContacts(host, 15).sendSeen('628123@c.us')).resolves.toBe(false); }); }); diff --git a/src/engine/adapters/baileys.adapter.spec.ts b/src/engine/adapters/baileys.adapter.spec.ts index 029ac4d70..5e790a664 100644 --- a/src/engine/adapters/baileys.adapter.spec.ts +++ b/src/engine/adapters/baileys.adapter.spec.ts @@ -150,6 +150,7 @@ import { loadRemoteMediaBuffer } from '../../common/media/load-remote-media'; const fakeStore = { put: jest.fn().mockResolvedValue(undefined), getMessage: jest.fn(), + getMessages: jest.fn().mockResolvedValue([]), clearSession: jest.fn().mockResolvedValue(undefined), }; diff --git a/src/engine/adapters/baileys.adapter.ts b/src/engine/adapters/baileys.adapter.ts index 616b10d99..f577fde50 100644 --- a/src/engine/adapters/baileys.adapter.ts +++ b/src/engine/adapters/baileys.adapter.ts @@ -153,6 +153,7 @@ export class BaileysAdapter implements IWhatsAppEngine { toEngineJid: jid => this.sessionStore.toEngineJid(jid), getEphemeralExpiration: chatId => this.sessionStore.getEphemeralExpiration(chatId), getStoredMessage: messageId => this.config.messageStore?.getMessage(this.config.dbSessionId, messageId), + getStoredMessages: messageIds => this.config.messageStore?.getMessages(this.config.dbSessionId, messageIds), recordLidMapping: (lid, pn) => this.sessionStore.addLidMappings([{ lid: `${lid.split('@')[0].split(':')[0]}@lid`, pn }]), mapMessage: (msg, contentType, opts) => this.events.mapMessage(msg, contentType, opts), diff --git a/src/engine/builtin/baileys/index.spec.ts b/src/engine/builtin/baileys/index.spec.ts index c1c9d3f6f..f4c6bd450 100644 --- a/src/engine/builtin/baileys/index.spec.ts +++ b/src/engine/builtin/baileys/index.spec.ts @@ -59,7 +59,7 @@ describe('BaileysPlugin.createEngine (opaque config)', () => { }); it('passes the message store to the adapter', () => { - const store = { put: jest.fn(), getMessage: jest.fn(), clearSession: jest.fn() }; + const store = { put: jest.fn(), getMessage: jest.fn(), getMessages: jest.fn(), clearSession: jest.fn() }; const plugin = new BaileysPlugin(store); plugin.createEngine({ sessionId: 'sess-1' }); expect(BaileysAdapter).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'sess-1', messageStore: store })); diff --git a/src/engine/types/baileys.types.ts b/src/engine/types/baileys.types.ts index 418aa697b..dfa2d11ce 100644 --- a/src/engine/types/baileys.types.ts +++ b/src/engine/types/baileys.types.ts @@ -10,6 +10,11 @@ export interface BaileysMessageStore { put(sessionId: string, msg: WAMessage): Promise; /** Look up a previously-seen message by its id, or null. */ getMessage(sessionId: string, messageId: string): Promise; + /** + * Look up many messages in one query. Ids the store has never seen are simply absent from the + * result, so the caller cannot assume the order or the length matches its input. + */ + getMessages(sessionId: string, messageIds: string[]): Promise; /** Remove all stored messages for a session (called on logout). */ clearSession(sessionId: string): Promise; } diff --git a/src/modules/session/dto/index.ts b/src/modules/session/dto/index.ts index 42b372d44..ceb019f79 100644 --- a/src/modules/session/dto/index.ts +++ b/src/modules/session/dto/index.ts @@ -2,6 +2,7 @@ export * from './create-session.dto'; export * from './session-config.dto'; export * from './session-response.dto'; export * from './mark-chat-read.dto'; +export * from './mark-chat-unread.dto'; export * from './archive-chat.dto'; export * from './mute-chat.dto'; export * from './pin-chat.dto'; diff --git a/src/modules/session/dto/mark-chat-read.dto.spec.ts b/src/modules/session/dto/mark-chat-read.dto.spec.ts index f62f04268..24a88ff3f 100644 --- a/src/modules/session/dto/mark-chat-read.dto.spec.ts +++ b/src/modules/session/dto/mark-chat-read.dto.spec.ts @@ -1,6 +1,6 @@ import { validateSync } from 'class-validator'; import { plainToInstance } from 'class-transformer'; -import { MarkChatReadDto } from './mark-chat-read.dto'; +import { MARK_READ_MESSAGE_IDS_MAX, MarkChatReadDto } from './mark-chat-read.dto'; const errorCount = (chatId: unknown): number => validateSync(plainToInstance(MarkChatReadDto, { chatId })).length; @@ -21,3 +21,35 @@ describe('MarkChatReadDto chatId validation', () => { }, ); }); + +const messageIdErrors = (messageIds: unknown): number => + validateSync(plainToInstance(MarkChatReadDto, { chatId: '1234567890@c.us', messageIds })).length; + +describe('MarkChatReadDto messageIds validation', () => { + it('accepts the field being absent — it is what every pre-existing caller sends', () => { + expect(validateSync(plainToInstance(MarkChatReadDto, { chatId: '1234567890@c.us' })).length).toBe(0); + }); + + it('accepts a list of ids', () => { + expect(messageIdErrors(['3EB0C767D26B8A3F1A2B', '3EB0C767D26B8A3F1A2C'])).toBe(0); + }); + + it('rejects an empty list rather than reading it as "the newest message"', () => { + // The engine treats a missing list as "acknowledge the newest message". A caller that computed + // its unread set and got none back must not land in that branch. + expect(messageIdErrors([])).toBeGreaterThan(0); + }); + + it('accepts a full batch and rejects one over the cap', () => { + const id = (n: number): string => `MSG${n}`; + expect(messageIdErrors(Array.from({ length: MARK_READ_MESSAGE_IDS_MAX }, (_, i) => id(i)))).toBe(0); + expect(messageIdErrors(Array.from({ length: MARK_READ_MESSAGE_IDS_MAX + 1 }, (_, i) => id(i)))).toBeGreaterThan(0); + }); + + it.each([[[' ']], [['']], [['has space']], [['ok', ' ']], [[123]], ['not-an-array']])( + 'rejects a malformed entry: %j', + messageIds => { + expect(messageIdErrors(messageIds)).toBeGreaterThan(0); + }, + ); +}); diff --git a/src/modules/session/dto/mark-chat-read.dto.ts b/src/modules/session/dto/mark-chat-read.dto.ts index 4755a8d8e..d47648721 100644 --- a/src/modules/session/dto/mark-chat-read.dto.ts +++ b/src/modules/session/dto/mark-chat-read.dto.ts @@ -1,5 +1,12 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; -import { ArrayMaxSize, IsArray, IsNotEmpty, IsOptional, IsString, Matches } from 'class-validator'; +import { ArrayMaxSize, ArrayNotEmpty, IsArray, IsNotEmpty, IsOptional, IsString, Matches } from 'class-validator'; + +/** + * Ceiling on one request's receipt batch, so a single call cannot hand the engine an unbounded key + * list to round-trip. A caller with more than this to acknowledge is catching up on history rather + * than marking a conversation read. + */ +export const MARK_READ_MESSAGE_IDS_MAX = 100; export class MarkChatReadDto { @ApiProperty({ @@ -24,14 +31,23 @@ export class MarkChatReadDto { 'Callers that persist inbound message IDs should send them here. Ignored by whatsapp-web.js, ' + 'whose own sendSeen is chat-level.', type: [String], + // @ArrayMaxSize is runtime-only; @nestjs/swagger does not derive maxItems from it, so without + // this the published schema advertises an unbounded array and a caller batching more than the + // cap discovers the limit as a 400. + maxItems: MARK_READ_MESSAGE_IDS_MAX, example: ['3EB0C767D26B8A3F1A2B', '3EB0C767D26B8A3F1A2C'], }) @IsOptional() @IsArray() - // Bounded so one request cannot hand the engine an unbounded key list to round-trip; a caller with - // more than this to acknowledge is catching up on history, not marking a conversation read. - @ArrayMaxSize(100) + // An empty array asks for nothing to be acknowledged. Rejected rather than accepted, because the + // engine reads a missing list as "the newest message" and the two must not collapse: a caller that + // computed its unread set and got none back would otherwise acknowledge a message it never named. + @ArrayNotEmpty() + @ArrayMaxSize(MARK_READ_MESSAGE_IDS_MAX) @IsString({ each: true }) @IsNotEmpty({ each: true }) + // Engine-neutral structural check: a message id is one non-whitespace token. @IsNotEmpty alone + // accepts ' ', which would reach sendNode as a receipt key. + @Matches(/^\S{1,128}$/, { each: true, message: 'each messageIds entry must be a non-empty id with no whitespace' }) messageIds?: string[]; } diff --git a/src/modules/session/dto/mark-chat-unread.dto.ts b/src/modules/session/dto/mark-chat-unread.dto.ts new file mode 100644 index 000000000..4d8ca6d69 --- /dev/null +++ b/src/modules/session/dto/mark-chat-unread.dto.ts @@ -0,0 +1,22 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsNotEmpty, IsString, Matches } from 'class-validator'; + +/** + * Deliberately separate from {@link MarkChatReadDto}, which the unread route used to share. The read + * route accepts `messageIds`; the unread route has nothing to do with them and ignores them, so + * sharing the class published a field on `POST /chats/unread` that it silently discarded. + */ +export class MarkChatUnreadDto { + @ApiProperty({ + description: "Chat ID in the active engine's native format (e.g. 1234567890@c.us on whatsapp-web.js)", + example: '1234567890@c.us', + }) + @IsString() + @IsNotEmpty() + // Engine-neutral structural check (localpart@host, no whitespace) so a different engine's JID + // scheme (e.g. Baileys 1234@s.whatsapp.net) is accepted too; the adapter normalises further. + @Matches(/^[^\s@]+@[^\s@]+$/, { + message: 'chatId must be a valid chat JID in the form localpart@host', + }) + chatId!: string; +} diff --git a/src/modules/session/session.controller.ts b/src/modules/session/session.controller.ts index f0e62ee16..7f5b9aab0 100644 --- a/src/modules/session/session.controller.ts +++ b/src/modules/session/session.controller.ts @@ -21,6 +21,7 @@ import { SessionResponseDto, QRCodeResponseDto, MarkChatReadDto, + MarkChatUnreadDto, SubscribePresenceDto, SetOwnPresenceDto, ChatPresenceResponseDto, @@ -572,7 +573,7 @@ export class SessionController { @ApiResponse({ status: 409, description: ENGINE_NOT_READY_409 }) async markChatUnread( @Param('sessionId', ParseUUIDPipe) id: string, - @Body() dto: MarkChatReadDto, + @Body() dto: MarkChatUnreadDto, ): Promise<{ success: boolean }> { const success = await this.sessionService.markUnread(id, dto.chatId); return { success }; From f5820a6c93511356a0a1bc26dcd065eceeb48050 Mon Sep 17 00:00:00 2001 From: m7fz7 Date: Wed, 19 Aug 2026 13:58:13 +0400 Subject: [PATCH 03/10] fix(sdk): split the mark-read body in the other four clients Rebasing on main brought the shape gate's request-body coverage to the Python, Go and Java clients, each of which maps MarkChatRequest to MarkChatReadDto. That pair now fails the same way the JavaScript one did: the contract carries messageIds and the hand-written type does not. Each client gets a MarkChatReadRequest carrying the optional list, with MarkChatRequest left to markUnread and subscribePresence, which take the chat id alone. The gate maps both pairs for all four clients and each coverage floor rises by one. --- CHANGELOG.md | 2 +- scripts/check-contract-shapes.mjs | 15 ++++---- sdk/go/chats.go | 2 +- sdk/go/routing_test.go | 2 +- sdk/go/types_chat.go | 11 +++++- .../openwa/model/MarkChatReadRequest.java | 35 +++++++++++++++++++ .../openwa/model/MarkChatRequest.java | 2 +- .../openwa/resources/ChatsResource.java | 3 +- .../openwa/resources/ChatsResourceTest.java | 3 +- sdk/python/openwa/resources/chats.py | 3 +- sdk/python/openwa/types.py | 10 ++++++ 11 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatReadRequest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index dc865c210..bbea7df27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Each id is resolved through the message store, so a group receipt carries the `participant` that attributes it. Omitting the field keeps the previous behaviour, an empty array is refused with a `400` rather than silently acknowledging the newest message, and the whatsapp-web.js engine ignores the field because its own receipt is chat-level. The `SessionMarkChatRead` agent tool takes the same list. +- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Each id is resolved through the message store, so a group receipt carries the `participant` that attributes it. Omitting the field keeps the previous behaviour, an empty array is refused with a `400` rather than silently acknowledging the newest message, and the whatsapp-web.js engine ignores the field because its own receipt is chat-level. The `SessionMarkChatRead` agent tool takes the same list, and all five clients expose the field through a dedicated read-request type. ### Changed diff --git a/scripts/check-contract-shapes.mjs b/scripts/check-contract-shapes.mjs index 534ec0e4f..a52cda7bb 100644 --- a/scripts/check-contract-shapes.mjs +++ b/scripts/check-contract-shapes.mjs @@ -167,9 +167,9 @@ const MAPPINGS = { const MINIMUM_MAPPED = { 'sdk/javascript/src/types.ts': 79, 'dashboard/src/services/api.ts': 20, - 'sdk/python/openwa/types.py': 73, - 'sdk/go': 74, - 'sdk/java': 78, + 'sdk/python/openwa/types.py': 74, + 'sdk/go': 75, + 'sdk/java': 79, }; /** Known drift, deliberately not gated yet — each line is a to-adjudicate follow-up. */ @@ -231,7 +231,8 @@ const PYTHON_MAPPING = { GroupParticipant: 'GroupParticipantDto', GroupSummary: 'GroupSummaryDto', JoinGroupRequest: 'JoinGroupDto', - MarkChatRequest: 'MarkChatReadDto', + MarkChatReadRequest: 'MarkChatReadDto', + MarkChatRequest: 'MarkChatUnreadDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', @@ -308,7 +309,8 @@ const GO_MAPPING = { GroupParticipant: 'GroupParticipantDto', GroupSummary: 'GroupSummaryDto', JoinGroupRequest: 'JoinGroupDto', - MarkChatRequest: 'MarkChatReadDto', + MarkChatReadRequest: 'MarkChatReadDto', + MarkChatRequest: 'MarkChatUnreadDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', @@ -389,7 +391,8 @@ const JAVA_MAPPING = { GroupSubjectRequest: 'GroupSubjectDto', GroupSummary: 'GroupSummaryDto', JoinGroupRequest: 'JoinGroupDto', - MarkChatRequest: 'MarkChatReadDto', + MarkChatReadRequest: 'MarkChatReadDto', + MarkChatRequest: 'MarkChatUnreadDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', diff --git a/sdk/go/chats.go b/sdk/go/chats.go index 146dbe8f1..52f693a1c 100644 --- a/sdk/go/chats.go +++ b/sdk/go/chats.go @@ -41,7 +41,7 @@ func (s *ChatsService) GetPresence(ctx context.Context, sessionID, chatID string } // MarkRead marks a chat as read. -func (s *ChatsService) MarkRead(ctx context.Context, sessionID string, body MarkChatRequest) (*SuccessResult, error) { +func (s *ChatsService) MarkRead(ctx context.Context, sessionID string, body MarkChatReadRequest) (*SuccessResult, error) { return s.post(ctx, sessionID, "/read", body) } diff --git a/sdk/go/routing_test.go b/sdk/go/routing_test.go index c360a266a..089edaa3a 100644 --- a/sdk/go/routing_test.go +++ b/sdk/go/routing_test.go @@ -94,7 +94,7 @@ func TestRouting(t *testing.T) { {"Webhooks.Test", func(c *Client) { c.Webhooks.Test(ctx, "s1", "w1") }, "POST", "/api/sessions/s1/webhooks/w1/test"}, {"Chats.List", func(c *Client) { c.Chats.List(ctx, "s1", nil) }, "GET", "/api/sessions/s1/chats"}, - {"Chats.MarkRead", func(c *Client) { c.Chats.MarkRead(ctx, "s1", MarkChatRequest{}) }, "POST", "/api/sessions/s1/chats/read"}, + {"Chats.MarkRead", func(c *Client) { c.Chats.MarkRead(ctx, "s1", MarkChatReadRequest{}) }, "POST", "/api/sessions/s1/chats/read"}, {"Chats.SubscribePresence", func(c *Client) { c.Chats.SubscribePresence(ctx, "s1", MarkChatRequest{}) }, "POST", "/api/sessions/s1/presence/subscribe"}, {"Channels.Create", func(c *Client) { c.Channels.Create(ctx, "s1", CreateChannelRequest{}) }, "POST", "/api/sessions/s1/channels"}, {"Channels.Delete", func(c *Client) { c.Channels.Delete(ctx, "s1", "ch1") }, "POST", "/api/sessions/s1/channels/ch1/delete"}, diff --git a/sdk/go/types_chat.go b/sdk/go/types_chat.go index 431452ecb..f6f24ac50 100644 --- a/sdk/go/types_chat.go +++ b/sdk/go/types_chat.go @@ -19,11 +19,20 @@ type SetOwnPresenceRequest struct { Available bool `json:"available"` } -// MarkChatRequest marks a chat read/unread. +// MarkChatRequest marks a chat unread, or subscribes to its presence. type MarkChatRequest struct { ChatID string `json:"chatId"` } +// MarkChatReadRequest marks a chat read, optionally naming the messages to acknowledge. +type MarkChatReadRequest struct { + ChatID string `json:"chatId"` + // 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"` +} + // ChatState is the typing indicator a chat shows. type ChatState string diff --git a/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatReadRequest.java b/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatReadRequest.java new file mode 100644 index 000000000..d46dc497f --- /dev/null +++ b/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatReadRequest.java @@ -0,0 +1,35 @@ +package com.rmyndharis.openwa.model; + +import java.util.List; + +/** Request body for marking a chat read. */ +public record MarkChatReadRequest(String chatId, List messageIds) { + public static Builder builder() { + return new Builder(); + } + + public static final class Builder { + private String chatId; + private List messageIds; + + /** WhatsApp chat id (JID), e.g. {@code 628123456789@c.us}. */ + public Builder chatId(String v) { + this.chatId = v; + return this; + } + + /** + * 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. + */ + public Builder messageIds(List v) { + this.messageIds = v; + return this; + } + + public MarkChatReadRequest build() { + return new MarkChatReadRequest(chatId, messageIds); + } + } +} diff --git a/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java b/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java index 2dd067d92..c11efb5de 100644 --- a/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java +++ b/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java @@ -1,6 +1,6 @@ package com.rmyndharis.openwa.model; -/** Request body for marking a chat read/unread. */ +/** Request body for marking a chat unread, or subscribing to its presence. */ public record MarkChatRequest(String chatId) { public static Builder builder() { return new Builder(); diff --git a/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java b/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java index ea910e54a..6fd66a25e 100644 --- a/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java +++ b/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java @@ -9,6 +9,7 @@ import com.rmyndharis.openwa.model.ChatSummary; import com.rmyndharis.openwa.model.DeleteChatRequest; import com.rmyndharis.openwa.model.ListChatsQuery; +import com.rmyndharis.openwa.model.MarkChatReadRequest; import com.rmyndharis.openwa.model.MarkChatRequest; import com.rmyndharis.openwa.model.MuteChatRequest; import com.rmyndharis.openwa.model.PinChatRequest; @@ -72,7 +73,7 @@ public ChatPresence getPresence(String sessionId, String chatId) { } /** Mark a chat as read/seen. */ - public SuccessResult markRead(String sessionId, MarkChatRequest body) { + public SuccessResult markRead(String sessionId, MarkChatReadRequest body) { return client.request( HttpMethod.POST, "/api/sessions/" + encodeSegment(sessionId) + "/chats/read", null, body, SuccessResult.class); } diff --git a/sdk/java/src/test/java/com/rmyndharis/openwa/resources/ChatsResourceTest.java b/sdk/java/src/test/java/com/rmyndharis/openwa/resources/ChatsResourceTest.java index 979ec295e..03f6091a8 100644 --- a/sdk/java/src/test/java/com/rmyndharis/openwa/resources/ChatsResourceTest.java +++ b/sdk/java/src/test/java/com/rmyndharis/openwa/resources/ChatsResourceTest.java @@ -12,6 +12,7 @@ import com.rmyndharis.openwa.model.MuteChatRequest; import com.rmyndharis.openwa.model.PinChatRequest; import com.rmyndharis.openwa.model.ListChatsQuery; +import com.rmyndharis.openwa.model.MarkChatReadRequest; import com.rmyndharis.openwa.model.MarkChatRequest; import com.rmyndharis.openwa.model.SendChatStateRequest; import com.rmyndharis.openwa.support.MockTransport; @@ -48,7 +49,7 @@ void listSerializesQuery() { @Test void markReadSendsBody() { tx.respond(200, "{\"success\":true}"); - client.chats.markRead("s", MarkChatRequest.builder().chatId("628123@c.us").build()); + client.chats.markRead("s", MarkChatReadRequest.builder().chatId("628123@c.us").build()); assertEquals("http://h/api/sessions/s/chats/read", tx.lastRequest().url()); assertEquals(HttpMethod.POST, tx.lastRequest().method()); assertTrue(tx.lastRequest().body().contains("628123@c.us")); diff --git a/sdk/python/openwa/resources/chats.py b/sdk/python/openwa/resources/chats.py index 3ec599ccc..13866069a 100644 --- a/sdk/python/openwa/resources/chats.py +++ b/sdk/python/openwa/resources/chats.py @@ -16,6 +16,7 @@ MuteChatRequest, ChatSummary, DeleteChatRequest, + MarkChatReadRequest, MarkChatRequest, SendChatStateRequest, SuccessResult, @@ -59,7 +60,7 @@ def get_presence(self, session_id: str, chat_id: str) -> ChatPresence | None: "GET", f"/api/sessions/{quote_segment(session_id)}/presence/{quote_segment(chat_id)}" ) - def mark_read(self, session_id: str, body: MarkChatRequest) -> SuccessResult: + def mark_read(self, session_id: str, body: MarkChatReadRequest) -> SuccessResult: return self._http.request("POST", f"/api/sessions/{quote_segment(session_id)}/chats/read", body=body) def mark_unread(self, session_id: str, body: MarkChatRequest) -> SuccessResult: diff --git a/sdk/python/openwa/types.py b/sdk/python/openwa/types.py index 6216d872d..182c03f97 100644 --- a/sdk/python/openwa/types.py +++ b/sdk/python/openwa/types.py @@ -882,9 +882,19 @@ class ChatSummary(TypedDict): class MarkChatRequest(TypedDict): + # Body for mark_unread and subscribe_presence, both of which take the chat id alone. chatId: Jid +class MarkChatReadRequest(TypedDict): + # Body for mark_read. + chatId: Jid + # 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: NotRequired[list[str]] + + class SendChatStateRequest(TypedDict): chatId: Jid state: ChatState From 9c422ff2d04ff69e5f646002e87f6db0594d2793 Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Wed, 19 Aug 2026 22:46:15 +0700 Subject: [PATCH 04/10] test(e2e): put each suite's server on a loopback port supertest starts a listener per request with `listen(0)` and no host, which binds the wildcard address, and then dials 127.0.0.1 whatever it bound. macOS hands a wildcard listener an ephemeral port that another process already holds on 127.0.0.1 specifically, permits the overlapping bind, and routes the loopback connection to the more specific holder. That process answers, so an assertion reads a status the app has no route for. Measured here: 40 of 40 such ports went to a wildcard listener, and a captured run took a 501 from a desktop helper holding 127.0.0.1:49657 on a machine with 29 loopback-specific listeners. It surfaced as a failure on a different test each run, which is why it read as flakiness and why redirecting state never moved it. Listening on 127.0.0.1 while the app initialises makes supertest reuse that server instead of opening one, and takes the lane from 230 listeners a run to 27. The host cannot simply be added to supertest's own call: a host argument routes the bind through dns.lookup, so address() is still null when supertest reads it synchronously. listen() initialises first and then binds the same server, so it hands the port taken during init back before binding what it was asked for. A bind failure is settled from the 'error' event, which otherwise leaves init pending and throws detached from the boot that caused it. Linux refuses the overlapping bind and its allocator skips held ports, so CI was not exposed; the change is inert there. --- CHANGELOG.md | 1 + docs/09-testing-strategy.md | 17 ++++-- test/setup-e2e-env.e2e-spec.ts | 102 +++++++++++++++++++++++++++++++++ test/setup-e2e.ts | 72 +++++++++++++++++++++++ 4 files changed, 188 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0103e4ee..dcaf0117b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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 and every suite in that run inherited whatever the previous one left in it. - 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 starts a listener per request on the wildcard address and then dials 127.0.0.1, and macOS both permits that bind over a port another process holds on 127.0.0.1 specifically and routes the connection to the more specific holder, so a run could assert against a status the app has no route for. Each suite's server now listens on loopback while it initialises, which supertest reuses instead of opening one, taking the lane from 230 listeners a run to 27. ### Tests diff --git a/docs/09-testing-strategy.md b/docs/09-testing-strategy.md index 8e761e5a8..c5753ae29 100644 --- a/docs/09-testing-strategy.md +++ b/docs/09-testing-strategy.md @@ -126,9 +126,10 @@ E2E smoke tests live in `test/` and use `test/jest-e2e.json`. > **They run one at a time (`maxWorkers: 1`), and that is a correctness requirement, not a > performance preference.** Each suite boots a real application, and not every piece of application -> state is redirected to a per-worker location. `dataDir` is a hard-coded `./data` with no -> environment lever, so every worker's plugin loader read-modify-writes the same -> `data/plugins/registry.json`. Measured on a single parallel run: 52 writes from 12 processes, 30 +> state is redirected to a per-worker location. The plugin registry once had no environment +> lever, so every worker's plugin loader read-modify-wrote the same +> `data/plugins/registry.json`. `PLUGIN_STATE_DIR` now redirects it and each suite takes its own state +> roots, so that particular collision is closed. Measured on a single parallel run: 52 writes from 12 processes, 30 > of them within 500ms of a write by a different process. Individual writes are atomic; the > read-modify-write cycle is not. > @@ -137,7 +138,15 @@ E2E smoke tests live in `test/` and use `test/jest-e2e.json`. > reproduced when a suite ran on its own, which is what made it look like flakiness. Serially it > does not occur. > -> Adding a suite is safe. Restoring parallelism is not, until each worker gets its own data root. +> Adding a suite is safe. Restoring parallelism has not been retried since those roots landed, so it +> is untested rather than known-safe. +> +> A second requirement has nothing to do with parallelism: each suite's server is put on a **loopback** +> port while it initialises (`test/setup-e2e.ts`). Left alone, supertest starts a listener per request +> on the wildcard address and then dials 127.0.0.1, and macOS both permits that bind over a port +> another process holds on 127.0.0.1 specifically and routes the connection to the more specific +> holder. An assertion then reads a status from an unrelated program on the host, which is why the lane +> could fail on a status no route can return. `setup-e2e-env.e2e-spec.ts` holds that contract. ```text test/ diff --git a/test/setup-e2e-env.e2e-spec.ts b/test/setup-e2e-env.e2e-spec.ts index 8110ee252..cc34112ec 100644 --- a/test/setup-e2e-env.e2e-spec.ts +++ b/test/setup-e2e-env.e2e-spec.ts @@ -6,9 +6,111 @@ * trivially; its regression value is in a shared-worker run after queue-on (or with a dirty * ambient env), where a missing reset shows up as REDIS_ENABLED !== 'false' here. */ +import { Controller, Get, INestApplication, Module } from '@nestjs/common'; +import { NestFactory } from '@nestjs/core'; +import { Test } from '@nestjs/testing'; +import { Server } from 'http'; +import { AddressInfo } from 'net'; +import request from 'supertest'; + describe('e2e boot environment', () => { it('boots with queue and Redis disabled', () => { expect(process.env.QUEUE_ENABLED).toBe('false'); expect(process.env.REDIS_ENABLED).toBe('false'); }); }); + +@Controller() +class PingController { + @Get('/__bind-probe') + probe(): { servedBy: string } { + return { servedBy: 'the app under test' }; + } +} + +/** A standalone app for the `listen()` handover, which needs a real NestFactory app rather than a fixture. */ +@Module({}) +class BareModule {} + +/** + * The listening contract setup-e2e.ts enforces, and the reason it exists. + * + * supertest starts a server per request with `listen(0)` and no host, which binds the wildcard + * address, and then dials 127.0.0.1 regardless of what it bound. macOS hands a wildcard listener an + * ephemeral port another process already holds on 127.0.0.1 specifically, allows the overlapping bind, + * and routes the loopback connection to the more specific binding, so that process answers instead of + * the app. The lane saw this as statuses no route can return, on a different test each run. + * + * setup-e2e.ts closes it by listening on 127.0.0.1 during `init()`, which is also the condition + * supertest branches on: a server that already has an address is reused rather than replaced. Both + * halves are asserted, because either one alone can hold while the protection is gone. Listening on + * the wildcard address still satisfies the second test, and never listening at all still satisfies + * neither, so the pair fails on a different side depending on which half was lost. + * + * The last two cover what taking that port early costs elsewhere: a bind that fails has to be + * reported rather than stall the boot, and an app that goes on to call `listen()` has to get its own + * socket back. + */ +describe('e2e listener bind contract', () => { + let app: INestApplication; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ controllers: [PingController] }).compile(); + app = moduleRef.createNestApplication(); + await app.init(); + }); + + afterAll(async () => { + await app?.close(); + }); + + it('leaves the app listening on loopback, not the wildcard address', () => { + const address = (app.getHttpServer() as Server).address() as AddressInfo; + expect(address).not.toBeNull(); + expect(address.address).toBe('127.0.0.1'); + }); + + it('is the server that answers, so supertest never opens a wildcard one of its own', async () => { + const server = app.getHttpServer() as Server; + // Not "the port stayed the same": that holds either way. supertest opening its own listener is a + // `listen` call, so the call itself is what has to be absent. + const listen = jest.spyOn(Server.prototype, 'listen'); + try { + await request(server).get('/__bind-probe').expect(200, { servedBy: 'the app under test' }); + expect(listen).not.toHaveBeenCalled(); + } finally { + listen.mockRestore(); + } + }); + + // A bind failure is reported on 'error', never through the listen callback. Waiting on the callback + // alone stalls init() and lets Node throw the event with nothing listening, so the failure lands + // detached from the boot that caused it. + it('reports a failed bind instead of leaving init pending', async () => { + const failing = jest.spyOn(Server.prototype, 'listen').mockImplementation(function (this: Server) { + process.nextTick(() => this.emit('error', Object.assign(new Error('bind refused'), { code: 'EADDRINUSE' }))); + return this; + }); + const stalled = await NestFactory.create(BareModule, { logger: false }); + try { + await expect(stalled.init()).rejects.toThrow('bind refused'); + } finally { + failing.mockRestore(); + await stalled.close().catch(() => undefined); + } + }); + + // Taking a port during init puts one on the same socket `listen()` wants, so without a handover it + // answers ERR_SERVER_ALREADY_LISTEN on a call that has nothing wrong with it. No suite calls + // `app.listen()` today, which is exactly why the breakage would surface as a puzzle later. + it('still lets an app call listen() afterwards, and puts that on loopback too', async () => { + const standalone = await NestFactory.create(BareModule, { logger: false }); + try { + await standalone.listen(0); + const address = (standalone.getHttpServer() as Server).address() as AddressInfo; + expect(address.address).toBe('127.0.0.1'); + } finally { + await standalone.close(); + } + }); +}); diff --git a/test/setup-e2e.ts b/test/setup-e2e.ts index c0b22dcc3..ce40f993b 100644 --- a/test/setup-e2e.ts +++ b/test/setup-e2e.ts @@ -22,6 +22,78 @@ import { join } from 'path'; import { tmpdir } from 'os'; import { rmSync } from 'fs'; +import { Server } from 'http'; +import { NestApplication } from '@nestjs/core'; + +// Put every suite's server on a LOOPBACK port before supertest can start one of its own. +// +// supertest starts a server per request with `listen(0)` and no host (supertest/lib/test.js), which +// binds the wildcard address, and then dials `http://127.0.0.1:` whatever it bound. macOS hands +// out an ephemeral port that an unrelated process already holds on 127.0.0.1 specifically (measured: +// 40 of 40 such ports went to a wildcard listener), permits the overlapping bind, and routes the +// loopback connection to the MORE specific binding. That process then answers, and its status reaches +// the assertion: a captured run took a `501 Not Implemented` from a desktop helper on 127.0.0.1:49657, +// on a machine holding 29 such loopback listeners. That is where this lane's statuses with no matching +// route came from, on a different test each run, and no amount of state isolation could reach it. +// +// The host cannot simply be added to that `listen` call: a host argument routes the bind through +// dns.lookup, so it completes a tick later, and supertest reads `address().port` synchronously. +// Listening here instead means `app.address()` is already set, so supertest reuses this server and +// never opens one. Binding 127.0.0.1 also makes a collision a refused bind rather than a silent +// share, and the allocator skips ports already held on that address. +// +// Boundary worth knowing: this patches a prototype, so a suite that booted an app inside +// `jest.isolateModules` would get a fresh, unpatched `@nestjs/core` and fall back to the exposed +// path. Nothing does that today (the one caller only reads module metadata), but a new one would +// need to listen on loopback itself. +type AutoListened = Server & { __e2eAutoListened?: true }; +const nestAppProto = NestApplication.prototype as unknown as { + init: (...args: unknown[]) => Promise; + listen: (...args: unknown[]) => Promise; + getHttpServer: () => unknown; + __e2eLoopbackListening?: true; +}; +if (!nestAppProto.__e2eLoopbackListening) { + const originalInit = nestAppProto.init; + nestAppProto.init = async function (this: typeof nestAppProto, ...args: unknown[]): Promise { + const result = await originalInit.apply(this, args); + const server = this.getHttpServer() as AutoListened | undefined; + if (server && typeof server.listen === 'function' && !server.listening) { + // A failed bind arrives as an 'error' event, not as this callback. Waiting on the callback + // alone leaves init() pending AND, with nothing listening for 'error', Node throws it detached + // from the call that caused it. Either half is the unexplained failure this block exists to + // remove, so the event is what settles the wait. + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', () => { + server.removeListener('error', reject); + resolve(); + }); + }); + server.__e2eAutoListened = true; + } + return result; + }; + + // `listen()` initialises first and then binds the SAME server, so the port taken above would meet it + // as ERR_SERVER_ALREADY_LISTEN on its own socket. Initialise here, hand that port back, and let the + // real call bind whatever it was asked for. A hostless port keeps the loopback the lane relies on, + // since a wildcard bind is exactly what the block above exists to avoid. + const originalListen = nestAppProto.listen; + nestAppProto.listen = async function (this: typeof nestAppProto, ...args: unknown[]): Promise { + await this.init(); + const server = this.getHttpServer() as AutoListened | undefined; + if (server?.__e2eAutoListened && server.listening) { + delete server.__e2eAutoListened; + await new Promise(resolve => server.close(() => resolve())); + } + const wantsDefaultHost = args.length <= 1 || typeof args[1] !== 'string'; + return wantsDefaultHost + ? originalListen.call(this, args[0], '127.0.0.1', ...args.slice(1)) + : originalListen.apply(this, args); + }; + nestAppProto.__e2eLoopbackListening = true; +} process.env.NODE_ENV = 'test'; // The plugin registry + per-plugin storage: the last state a suite could not isolate. From ef4b0565a8c514bedb70b5f1304b4dd162e12e9b Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Wed, 19 Aug 2026 23:41:10 +0700 Subject: [PATCH 05/10] docs(changelog): credit the contributor on the mark-read entry External contributions carry "Thanks @handle." on their CHANGELOG entry; this one was missing it. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbea7df27..84c502fd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Each id is resolved through the message store, so a group receipt carries the `participant` that attributes it. Omitting the field keeps the previous behaviour, an empty array is refused with a `400` rather than silently acknowledging the newest message, and the whatsapp-web.js engine ignores the field because its own receipt is chat-level. The `SessionMarkChatRead` agent tool takes the same list, and all five clients expose the field through a dedicated read-request type. +- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Each id is resolved through the message store, so a group receipt carries the `participant` that attributes it. Omitting the field keeps the previous behaviour, an empty array is refused with a `400` rather than silently acknowledging the newest message, and the whatsapp-web.js engine ignores the field because its own receipt is chat-level. The `SessionMarkChatRead` agent tool takes the same list, and all five clients expose the field through a dedicated read-request type. Thanks @m7fz7. ### Changed From 3d301db9d77b04840c49cc539810a65e4dd4c54a Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Thu, 20 Aug 2026 00:00:48 +0700 Subject: [PATCH 06/10] fix(mcp): apply the message-id format rule on the agent tool The REST body rejects a whitespace-only entry in `messageIds`, but `SessionMarkChatRead` declared its own list and copied only the count bound. `invokeTool` parses the tool's schema and calls the service directly, so the DTO never applies on that path and `[' ']` reached the engine as a receipt key. The pattern and its message move into exported constants next to the existing cap, and the tool holds them rather than restating the rule. Restating it is how the two surfaces drifted in the first place. --- .../agent-tools/tools/session.tools.spec.ts | 22 +++++++++++++++++++ src/core/agent-tools/tools/session.tools.ts | 10 +++++++-- src/modules/session/dto/mark-chat-read.dto.ts | 14 +++++++++--- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/core/agent-tools/tools/session.tools.spec.ts b/src/core/agent-tools/tools/session.tools.spec.ts index 84cbf657b..076881410 100644 --- a/src/core/agent-tools/tools/session.tools.spec.ts +++ b/src/core/agent-tools/tools/session.tools.spec.ts @@ -1,3 +1,4 @@ +import { BadRequestException } from '@nestjs/common'; import { invokeTool } from '../tool-invoker'; import { sessionTools } from './session.tools'; import type { AnyToolDescriptor } from '../tool-descriptor'; @@ -104,6 +105,27 @@ describe('sessionTools', () => { expect(sendSeen).toHaveBeenCalledWith('s1', '628111@c.us', ['M1', 'M2']); }); + // The REST body rejects a whitespace-only id, and this path never sees that DTO: invokeTool parses + // the tool's own schema and calls the service directly, so an id the schema accepts is sent as a + // receipt key. Asserting the service was not reached is the point; a 400 that still delegated + // would leave the defect in place. + it('SessionMarkChatRead refuses a whitespace-only message id instead of sending it as a key', async () => { + const sendSeen = jest.fn().mockResolvedValue(true); + const tool = makeTools({ sendSeen } as unknown as SessionService).get('SessionMarkChatRead')!; + + const failure = await run(tool, { sessionId: 's1', chatId: '628111@c.us', messageIds: [' '] }).catch( + (e: unknown) => e, + ); + expect(failure).toBeInstanceOf(BadRequestException); + expect(JSON.stringify((failure as BadRequestException).getResponse())).toContain('no whitespace'); + expect(sendSeen).not.toHaveBeenCalled(); + + // Control: a well-formed id still reaches the service, so the refusal above is this rule and not + // a schema that turns everything away. + await run(tool, { sessionId: 's1', chatId: '628111@c.us', messageIds: ['3EB0C767D26B8A3F1A2B'] }); + expect(sendSeen).toHaveBeenCalledWith('s1', '628111@c.us', ['3EB0C767D26B8A3F1A2B']); + }); + it('SessionMarkChatUnread maps the markUnread result to a success field', async () => { const markUnread = jest.fn().mockResolvedValue(true); const out = await run(makeTools({ markUnread } as unknown as SessionService).get('SessionMarkChatUnread')!, { diff --git a/src/core/agent-tools/tools/session.tools.ts b/src/core/agent-tools/tools/session.tools.ts index 916d203e8..0fb75dd3a 100644 --- a/src/core/agent-tools/tools/session.tools.ts +++ b/src/core/agent-tools/tools/session.tools.ts @@ -2,7 +2,11 @@ import { z } from 'zod'; import { ApiKeyRole } from '../../../modules/auth/entities/api-key.entity'; import type { SessionService } from '../../../modules/session/session.service'; import { SessionResponseDto } from '../../../modules/session/dto/session-response.dto'; -import { MARK_READ_MESSAGE_IDS_MAX } from '../../../modules/session/dto/mark-chat-read.dto'; +import { + MARK_READ_MESSAGE_IDS_MAX, + MARK_READ_MESSAGE_ID_MESSAGE, + MARK_READ_MESSAGE_ID_PATTERN, +} from '../../../modules/session/dto/mark-chat-read.dto'; import { defineTool, type AnyToolDescriptor } from '../tool-descriptor'; const sessionId = z.string().min(1).describe('Session UUID (the session id, not the name)'); @@ -91,7 +95,9 @@ export function sessionTools(session: SessionService): AnyToolDescriptor[] { sessionId, chatId: z.string().describe('Chat JID (e.g. 1234567890@c.us)'), messageIds: z - .array(z.string()) + // The element rule comes from the DTO rather than being restated here: the REST body + // rejects a whitespace-only id, and this path reaches the engine without the DTO at all. + .array(z.string().regex(MARK_READ_MESSAGE_ID_PATTERN, MARK_READ_MESSAGE_ID_MESSAGE)) .nonempty() .max(MARK_READ_MESSAGE_IDS_MAX) .optional() diff --git a/src/modules/session/dto/mark-chat-read.dto.ts b/src/modules/session/dto/mark-chat-read.dto.ts index d47648721..e993de6fa 100644 --- a/src/modules/session/dto/mark-chat-read.dto.ts +++ b/src/modules/session/dto/mark-chat-read.dto.ts @@ -8,6 +8,16 @@ import { ArrayMaxSize, ArrayNotEmpty, IsArray, IsNotEmpty, IsOptional, IsString, */ export const MARK_READ_MESSAGE_IDS_MAX = 100; +/** + * The shape of one message id: a single non-whitespace token. Exported for the same reason as the + * cap above, so the agent tool holds the rule rather than restating it. Restating it is how the two + * surfaces drifted: the tool copied the count and left `[' ']` reaching the engine as a receipt key. + */ +export const MARK_READ_MESSAGE_ID_PATTERN = /^\S{1,128}$/; + +/** Rejection message for {@link MARK_READ_MESSAGE_ID_PATTERN}, shared so both surfaces answer alike. */ +export const MARK_READ_MESSAGE_ID_MESSAGE = 'each messageIds entry must be a non-empty id with no whitespace'; + export class MarkChatReadDto { @ApiProperty({ description: "Chat ID in the active engine's native format (e.g. 1234567890@c.us on whatsapp-web.js)", @@ -46,8 +56,6 @@ export class MarkChatReadDto { @ArrayMaxSize(MARK_READ_MESSAGE_IDS_MAX) @IsString({ each: true }) @IsNotEmpty({ each: true }) - // Engine-neutral structural check: a message id is one non-whitespace token. @IsNotEmpty alone - // accepts ' ', which would reach sendNode as a receipt key. - @Matches(/^\S{1,128}$/, { each: true, message: 'each messageIds entry must be a non-empty id with no whitespace' }) + @Matches(MARK_READ_MESSAGE_ID_PATTERN, { each: true, message: MARK_READ_MESSAGE_ID_MESSAGE }) messageIds?: string[]; } From a325279dd03284eccf002db23a8d2bc6a1ff4857 Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Thu, 20 Aug 2026 00:00:48 +0700 Subject: [PATCH 07/10] docs(changelog): trim the Unreleased entries to the released length Entries under `[Unreleased]` had grown past anything the released sections carry: median 44 words against 34, and a longest of 139 against a historical maximum of 80. They read as explanations rather than release notes. Rewritten to the same distribution (median 39, longest 57), keeping what a reader acts on and dropping the reasoning. Contributor credit and the section headings are unchanged. --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b61b318fa..daf954ff0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,20 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `POST /sessions/{sessionId}/chats/read` accepts an optional `messageIds` array (up to 100) naming exactly which messages to acknowledge. Baileys acknowledges individual messages, so without it only the newest message still held in memory got a receipt: a burst left its earlier messages unread, and a session restarted since the message arrived had nothing to acknowledge and answered `false` under a `200`. Each id is resolved through the message store, so a group receipt carries the `participant` that attributes it. Omitting the field keeps the previous behaviour, an empty array is refused with a `400` rather than silently acknowledging the newest message, and the whatsapp-web.js engine ignores the field because its own receipt is chat-level. The `SessionMarkChatRead` agent tool takes the same list, and all five clients expose the field through a dedicated read-request type. Thanks @m7fz7. +- `POST /sessions/{sessionId}/chats/read` takes an optional `messageIds` array (up to 100) naming which messages to acknowledge. Baileys acknowledges individual messages, so without it a burst left its earlier messages unread. Ids resolve through the message store, so a group receipt carries its `participant`. Available on the agent tool and all five clients; ignored by whatsapp-web.js. Thanks @m7fz7. ### Changed -- `POST /sessions/{sessionId}/chats/unread` publishes its own `MarkChatUnreadDto` instead of sharing `MarkChatReadDto` with the read route. The request body is unchanged (`chatId` alone), but a client generated from the contract sees the schema under a new name. Sharing the class would have advertised `messageIds` on a route that discards it. +- `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. ### Fixed - 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 is fire-and-forget, so a crash between persisting a message and completing its POST lost the delivery with no record in either mode, while the documented contract promises at-least-once. Each delivery is now recorded before it is attempted, retired once the queue or the inline send owns it, and a bounded sweep replays whatever is left stranded using the stored idempotency key so the retry stays deduplicable at the receiver. +- 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. - 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 and every suite in that run inherited whatever the previous one left in it. +- `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. - 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 starts a listener per request on the wildcard address and then dials 127.0.0.1, and macOS both permits that bind over a port another process holds on 127.0.0.1 specifically and routes the connection to the more specific holder, so a run could assert against a status the app has no route for. Each suite's server now listens on loopback while it initialises, which supertest reuses instead of opening one, taking the lane from 230 listeners a run to 27. +- 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. ### Tests From 045c351ac2a134de40d603e9030be2e5d04ef196 Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Thu, 20 Aug 2026 00:16:08 +0700 Subject: [PATCH 08/10] ci: bound the apt steps so a slow mirror cannot hang a run `Install sqlite3` held the scripts-smoke job open for over an hour on two consecutive main runs while every other job had already gone green, so neither run ever reported and main went without a signal. The step had no timeout, so a stalled apt-get would have burned the six-hour job default before failing. Both apt steps now carry a five-minute timeout and skip the install when the runner image already ships the tool, which is the common case. The shellcheck step runs its own apt-get update rather than inheriting one from the step above, which no longer always runs. A failing install still fails the step: the short-circuit only covers the tool already being present. release.yml carries the same job and had the same two unbounded steps, so it is fixed alongside; a release cut could have stalled the same way. --- .github/workflows/ci.yml | 11 +++++++++-- .github/workflows/release.yml | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4fdb81fc7..4f43fd4f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -299,10 +299,17 @@ jobs: - name: Install sqlite3 # smoke-test-backup-restore.sh skips its sqlite3 .backup roundtrip case (with a notice) when # the host lacks the CLI; installing it here exercises that path on every run instead. - run: sudo apt-get update && sudo apt-get install -y --no-install-recommends sqlite3 + # Bounded, and skipped when the runner image already ships the CLI. An unbounded apt-get + # against a slow mirror held this job open for over an hour with every other job already + # green, so the run never reported and main went without a signal. + timeout-minutes: 5 + run: command -v sqlite3 >/dev/null || { sudo apt-get update && sudo apt-get install -y --no-install-recommends sqlite3; } - name: Install shellcheck - run: sudo apt-get install -y --no-install-recommends shellcheck + # Runs its own update rather than inheriting one from the step above, which no longer + # always runs. + timeout-minutes: 5 + run: command -v shellcheck >/dev/null || { sudo apt-get update && sudo apt-get install -y --no-install-recommends shellcheck; } - name: shellcheck every shell script # Widened from the original three (#926) now that the rest are clean too. The narrow scope was diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 320c55389..45081b4a4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -278,10 +278,17 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - name: Install sqlite3 - run: sudo apt-get update && sudo apt-get install -y --no-install-recommends sqlite3 + # Bounded, and skipped when the runner image already ships the CLI. An unbounded apt-get + # against a slow mirror held this job open for over an hour with every other job already + # green, so the run never reported and main went without a signal. + timeout-minutes: 5 + run: command -v sqlite3 >/dev/null || { sudo apt-get update && sudo apt-get install -y --no-install-recommends sqlite3; } - name: Install shellcheck - run: sudo apt-get install -y --no-install-recommends shellcheck + # Runs its own update rather than inheriting one from the step above, which no longer + # always runs. + timeout-minutes: 5 + run: command -v shellcheck >/dev/null || { sudo apt-get update && sudo apt-get install -y --no-install-recommends shellcheck; } - name: shellcheck every shell script run: shellcheck docker-entrypoint.sh scripts/*.sh From 1744dd551582b94b7c4be0649ce15be04ccbaac5 Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Thu, 20 Aug 2026 00:27:07 +0700 Subject: [PATCH 09/10] fix(sdk): give subscribe-presence its own request type `SubscribePresenceDto` had no contract-shape coverage at all. Four clients declared one `MarkChatRequest` for both `subscribePresence` and `markUnread`, and the gate mapped that type to `MarkChatUnreadDto`, so the pair being compared was not the pair that exists. The two DTOs are identical today, which is the only reason nothing failed; a field added to either would have gone unnoticed on every typed client. Each of the four typed clients now declares `SubscribePresenceRequest`, the gate maps it to `SubscribePresenceDto`, and the per-client coverage floors rise by one so the pair cannot be dropped again in silence. Comparisons go from 320 to 324. BREAKING for the Go and Java clients: `SubscribePresence` takes the new type. The wire body is unchanged. --- CHANGELOG.md | 1 + scripts/check-contract-shapes.mjs | 12 ++++++---- sdk/go/chats.go | 2 +- sdk/go/routing_test.go | 2 +- sdk/go/types_chat.go | 7 +++++- .../openwa/model/MarkChatRequest.java | 2 +- .../model/SubscribePresenceRequest.java | 22 +++++++++++++++++++ .../openwa/resources/ChatsResource.java | 3 ++- sdk/javascript/src/resources/chats.ts | 3 ++- sdk/javascript/src/types.ts | 7 +++++- sdk/python/openwa/resources/chats.py | 3 ++- sdk/python/openwa/types.py | 7 +++++- 12 files changed, 58 insertions(+), 13 deletions(-) create mode 100644 sdk/java/src/main/java/com/rmyndharis/openwa/model/SubscribePresenceRequest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index daf954ff0..58c65794e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ 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. ### Fixed diff --git a/scripts/check-contract-shapes.mjs b/scripts/check-contract-shapes.mjs index a52cda7bb..d763da72f 100644 --- a/scripts/check-contract-shapes.mjs +++ b/scripts/check-contract-shapes.mjs @@ -87,6 +87,7 @@ const MAPPINGS = { JoinGroupRequest: 'JoinGroupDto', MarkChatReadRequest: 'MarkChatReadDto', MarkChatRequest: 'MarkChatUnreadDto', + SubscribePresenceRequest: 'SubscribePresenceDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', @@ -165,11 +166,11 @@ const MAPPINGS = { * these floors as pairs are added makes the shrink loud. */ const MINIMUM_MAPPED = { - 'sdk/javascript/src/types.ts': 79, + 'sdk/javascript/src/types.ts': 80, 'dashboard/src/services/api.ts': 20, - 'sdk/python/openwa/types.py': 74, - 'sdk/go': 75, - 'sdk/java': 79, + 'sdk/python/openwa/types.py': 75, + 'sdk/go': 76, + 'sdk/java': 80, }; /** Known drift, deliberately not gated yet — each line is a to-adjudicate follow-up. */ @@ -233,6 +234,7 @@ const PYTHON_MAPPING = { JoinGroupRequest: 'JoinGroupDto', MarkChatReadRequest: 'MarkChatReadDto', MarkChatRequest: 'MarkChatUnreadDto', + SubscribePresenceRequest: 'SubscribePresenceDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', @@ -311,6 +313,7 @@ const GO_MAPPING = { JoinGroupRequest: 'JoinGroupDto', MarkChatReadRequest: 'MarkChatReadDto', MarkChatRequest: 'MarkChatUnreadDto', + SubscribePresenceRequest: 'SubscribePresenceDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', @@ -393,6 +396,7 @@ const JAVA_MAPPING = { JoinGroupRequest: 'JoinGroupDto', MarkChatReadRequest: 'MarkChatReadDto', MarkChatRequest: 'MarkChatUnreadDto', + SubscribePresenceRequest: 'SubscribePresenceDto', MessageListResponse: 'MessageListResponseDto', MessageRecord: 'MessageListItemDto', MessageResponse: 'MessageResponseDto', diff --git a/sdk/go/chats.go b/sdk/go/chats.go index 52f693a1c..3eb3cb66e 100644 --- a/sdk/go/chats.go +++ b/sdk/go/chats.go @@ -24,7 +24,7 @@ func (s *ChatsService) List(ctx context.Context, sessionID string, query *ListCh // The subscription belongs to the connection and does NOT survive a restart or an automatic // reconnect, so re-issue it when the session comes back. Subscribe per chat: WhatsApp emits an // update on every transition, so a broad subscription is a firehose. whatsapp-web.js answers 501. -func (s *ChatsService) SubscribePresence(ctx context.Context, sessionID string, body MarkChatRequest) (*SuccessResult, error) { +func (s *ChatsService) SubscribePresence(ctx context.Context, sessionID string, body SubscribePresenceRequest) (*SuccessResult, error) { var out SuccessResult err := s.client.do(ctx, "POST", "/api/sessions/"+pathEscape(sessionID)+"/presence/subscribe", nil, body, &out) return &out, err diff --git a/sdk/go/routing_test.go b/sdk/go/routing_test.go index 089edaa3a..c57477939 100644 --- a/sdk/go/routing_test.go +++ b/sdk/go/routing_test.go @@ -95,7 +95,7 @@ func TestRouting(t *testing.T) { {"Chats.List", func(c *Client) { c.Chats.List(ctx, "s1", nil) }, "GET", "/api/sessions/s1/chats"}, {"Chats.MarkRead", func(c *Client) { c.Chats.MarkRead(ctx, "s1", MarkChatReadRequest{}) }, "POST", "/api/sessions/s1/chats/read"}, - {"Chats.SubscribePresence", func(c *Client) { c.Chats.SubscribePresence(ctx, "s1", MarkChatRequest{}) }, "POST", "/api/sessions/s1/presence/subscribe"}, + {"Chats.SubscribePresence", func(c *Client) { c.Chats.SubscribePresence(ctx, "s1", SubscribePresenceRequest{}) }, "POST", "/api/sessions/s1/presence/subscribe"}, {"Channels.Create", func(c *Client) { c.Channels.Create(ctx, "s1", CreateChannelRequest{}) }, "POST", "/api/sessions/s1/channels"}, {"Channels.Delete", func(c *Client) { c.Channels.Delete(ctx, "s1", "ch1") }, "POST", "/api/sessions/s1/channels/ch1/delete"}, {"Channels.Mute", func(c *Client) { c.Channels.Mute(ctx, "s1", "ch1", MuteChannelRequest{}) }, "POST", "/api/sessions/s1/channels/ch1/mute"}, diff --git a/sdk/go/types_chat.go b/sdk/go/types_chat.go index f6f24ac50..09a46c65c 100644 --- a/sdk/go/types_chat.go +++ b/sdk/go/types_chat.go @@ -19,11 +19,16 @@ type SetOwnPresenceRequest struct { Available bool `json:"available"` } -// MarkChatRequest marks a chat unread, or subscribes to its presence. +// MarkChatRequest marks a chat unread. type MarkChatRequest struct { ChatID string `json:"chatId"` } +// SubscribePresenceRequest subscribes to a chat's presence. +type SubscribePresenceRequest struct { + ChatID string `json:"chatId"` +} + // MarkChatReadRequest marks a chat read, optionally naming the messages to acknowledge. type MarkChatReadRequest struct { ChatID string `json:"chatId"` diff --git a/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java b/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java index c11efb5de..193b11c35 100644 --- a/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java +++ b/sdk/java/src/main/java/com/rmyndharis/openwa/model/MarkChatRequest.java @@ -1,6 +1,6 @@ package com.rmyndharis.openwa.model; -/** Request body for marking a chat unread, or subscribing to its presence. */ +/** Request body for marking a chat unread. */ public record MarkChatRequest(String chatId) { public static Builder builder() { return new Builder(); diff --git a/sdk/java/src/main/java/com/rmyndharis/openwa/model/SubscribePresenceRequest.java b/sdk/java/src/main/java/com/rmyndharis/openwa/model/SubscribePresenceRequest.java new file mode 100644 index 000000000..e297fecf0 --- /dev/null +++ b/sdk/java/src/main/java/com/rmyndharis/openwa/model/SubscribePresenceRequest.java @@ -0,0 +1,22 @@ +package com.rmyndharis.openwa.model; + +/** Request body for subscribing to a chat's presence. */ +public record SubscribePresenceRequest(String chatId) { + public static Builder builder() { + return new Builder(); + } + + public static final class Builder { + private String chatId; + + /** WhatsApp chat id (JID), e.g. {@code 628123456789@c.us}. */ + public Builder chatId(String v) { + this.chatId = v; + return this; + } + + public SubscribePresenceRequest build() { + return new SubscribePresenceRequest(chatId); + } + } +} diff --git a/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java b/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java index 6fd66a25e..36c9157eb 100644 --- a/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java +++ b/sdk/java/src/main/java/com/rmyndharis/openwa/resources/ChatsResource.java @@ -11,6 +11,7 @@ import com.rmyndharis.openwa.model.ListChatsQuery; import com.rmyndharis.openwa.model.MarkChatReadRequest; import com.rmyndharis.openwa.model.MarkChatRequest; +import com.rmyndharis.openwa.model.SubscribePresenceRequest; import com.rmyndharis.openwa.model.MuteChatRequest; import com.rmyndharis.openwa.model.PinChatRequest; import com.rmyndharis.openwa.model.SendChatStateRequest; @@ -50,7 +51,7 @@ public List list(String sessionId, ListChatsQuery query) { * reconnect, so re-issue it when the session comes back. Subscribe per chat: WhatsApp emits an * update on every transition. whatsapp-web.js answers {@code 501}. */ - public SuccessResult subscribePresence(String sessionId, MarkChatRequest body) { + public SuccessResult subscribePresence(String sessionId, SubscribePresenceRequest body) { return client.request( HttpMethod.POST, "/api/sessions/" + encodeSegment(sessionId) + "/presence/subscribe", diff --git a/sdk/javascript/src/resources/chats.ts b/sdk/javascript/src/resources/chats.ts index 61da4559b..00eaab8d8 100644 --- a/sdk/javascript/src/resources/chats.ts +++ b/sdk/javascript/src/resources/chats.ts @@ -18,6 +18,7 @@ import type { DeleteChatRequest, MarkChatRequest, MarkChatReadRequest, + SubscribePresenceRequest, ChatPresence, SendChatStateRequest, SuccessResult, @@ -48,7 +49,7 @@ export class ChatsResource { * reconnect, so re-issue it when the session comes back. Subscribe per chat: WhatsApp emits an * update on every transition, so a broad subscription is a firehose. whatsapp-web.js answers 501. */ - subscribePresence(sessionId: string, body: MarkChatRequest): Promise { + subscribePresence(sessionId: string, body: SubscribePresenceRequest): Promise { return this.client.request({ method: 'POST', path: `/api/sessions/${encodeSegment(sessionId)}/presence/subscribe`, diff --git a/sdk/javascript/src/types.ts b/sdk/javascript/src/types.ts index a77e31041..2a845881e 100644 --- a/sdk/javascript/src/types.ts +++ b/sdk/javascript/src/types.ts @@ -968,11 +968,16 @@ export interface TransferChannelOwnershipRequest { newOwnerId: Jid; } -/** Body for {@link ChatsResource.markUnread} and {@link ChatsResource.subscribePresence}. */ +/** Body for {@link ChatsResource.markUnread}. */ export interface MarkChatRequest { chatId: Jid; } +/** Body for {@link ChatsResource.subscribePresence}. */ +export interface SubscribePresenceRequest { + chatId: Jid; +} + /** Body for {@link ChatsResource.markRead}. */ export interface MarkChatReadRequest extends MarkChatRequest { /** diff --git a/sdk/python/openwa/resources/chats.py b/sdk/python/openwa/resources/chats.py index 13866069a..bfa131a18 100644 --- a/sdk/python/openwa/resources/chats.py +++ b/sdk/python/openwa/resources/chats.py @@ -18,6 +18,7 @@ DeleteChatRequest, MarkChatReadRequest, MarkChatRequest, + SubscribePresenceRequest, SendChatStateRequest, SuccessResult, ) @@ -38,7 +39,7 @@ def __init__(self, http: "HttpExecutor") -> None: def list(self, session_id: str, query: ListChatsQuery | None = None) -> list[ChatSummary]: return self._http.request("GET", f"/api/sessions/{quote_segment(session_id)}/chats", query=query) - def subscribe_presence(self, session_id: str, body: MarkChatRequest) -> SuccessResult: + def subscribe_presence(self, session_id: str, body: SubscribePresenceRequest) -> SuccessResult: """Subscribe to a chat's presence; updates arrive as presence.update events. Presence cannot be fetched from either engine, only received. The subscription belongs to diff --git a/sdk/python/openwa/types.py b/sdk/python/openwa/types.py index 182c03f97..0ac6401f9 100644 --- a/sdk/python/openwa/types.py +++ b/sdk/python/openwa/types.py @@ -882,7 +882,12 @@ class ChatSummary(TypedDict): class MarkChatRequest(TypedDict): - # Body for mark_unread and subscribe_presence, both of which take the chat id alone. + # Body for mark_unread. + chatId: Jid + + +class SubscribePresenceRequest(TypedDict): + # Body for subscribe_presence. chatId: Jid From 915783a0467f2c939aa2a6127d52011be4c79e05 Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Thu, 20 Aug 2026 00:40:36 +0700 Subject: [PATCH 10/10] docs(changelog): record the apt-step bound --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index daf954ff0..653fa459d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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. - 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. ### Tests