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 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[]; }