From 3d301db9d77b04840c49cc539810a65e4dd4c54a Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Thu, 20 Aug 2026 00:00:48 +0700 Subject: [PATCH 1/2] 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 2/2] 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