diff --git a/apps/chat/src/chat.app.ts b/apps/chat/src/chat.app.ts index 02b41e4..79c01c5 100644 --- a/apps/chat/src/chat.app.ts +++ b/apps/chat/src/chat.app.ts @@ -282,15 +282,38 @@ function censorContents(contents: string): string { return censored === data ? contents : JSON.stringify({ ...fields, Data: censored }) } +/** + * A stored message in the PascalCase shape the client's SendMessage handler reads back, as + * distinct from the camelCase {@link ChatMessage} the thread payloads carry. Same six + * fields; the client reads them in two places under two spellings. + * + * `Contents` goes out exactly as stored, which is what makes the envelope discipline matter: + * the client parses it into `MessageJson` in a post-deserialize hook, and anything that + * isn't an escaped `{ Type, Version, Data }` with a non-null `Data` leaves that null. The + * hook only logs, so the client then throws on the null instead of showing the message. + */ +function toSentChatMessage(message: ChatMessage) { + return { + ChatMessageId: message.chatMessageId, + ChatThreadId: message.chatThreadId, + SenderPlayerId: message.senderPlayerId, + TimeSent: message.timeSent, + Contents: message.contents, + ModerationState: message.moderationState, + } +} + /** * Send a message to a thread that already exists — every message after the one that * opened the conversation. `/thread/18` is what the client posts; `/thread/18/message` is * the same call under the reference's other spelling, so both routes land here. * - * Answers `{chatResult, chatThread}` — the whole thread with its messages, not just the - * message that was sent, so the client re-renders the conversation from one response. - * Blank or missing contents stores nothing and reports invalid-arguments, still with the - * thread attached, rather than an error status. + * Answers the message just posted (`ChatMessage`/`ChatResult`, which is what the client's + * own send handler reads) AND the whole thread with its messages (`chatResult`/`chatThread`, + * which the conversation re-renders from). Blank or missing contents stores nothing and + * reports invalid-arguments, still with the thread attached, rather than an error status — + * and with a NULL `ChatMessage`, which is safe because the client only dereferences it on + * result 0. */ async function sendToThread(c: Context) { const id = await authedId(c) @@ -320,8 +343,16 @@ async function sendToThread(c: Context) { } const thread = await threadWithMessages(c, chatThreadId, id, DEFAULT_THREAD_MESSAGE_COUNT) + const chatResult = posted === null ? CHAT_INVALID_ARGUMENTS : CHAT_SUCCESS + // Both spellings, because the client reads this response in two places. Its SendMessage + // handler dereferences `ChatMessage` the moment `ChatResult` is 0, so a success answered + // without one throws inside the client — the lowercase pair alone left that null. The + // thread stays for the conversation re-render. One value, serialized twice, so the two + // result keys can never disagree. return c.json({ - chatResult: posted === null ? CHAT_INVALID_ARGUMENTS : CHAT_SUCCESS, + ChatMessage: posted === null ? null : toSentChatMessage(posted), + ChatResult: chatResult, + chatResult, chatThread: thread, }) } @@ -449,9 +480,15 @@ function sendToThreadRoute(spelling: string) { tags: ['Messages'], summary: `Send a message to an existing thread (${spelling})`, description: [ - 'Every message after the one that opened the conversation. Answers', - '`{ chatResult, chatThread }` — the WHOLE thread with its messages, not just the message', - 'that was sent, so the client re-renders the conversation from one response. The envelope’s', + 'Every message after the one that opened the conversation. Answers FOUR keys in two', + 'spellings: `ChatMessage`/`ChatResult`, which is what the client’s own send handler reads', + '— it dereferences `ChatMessage` as soon as `ChatResult` is 0, so a success without one', + 'throws inside the client — plus `chatResult`/`chatThread`, the WHOLE thread with its', + 'messages, which the conversation re-renders from. The two result keys are one value', + 'serialized twice. `ChatMessage.Contents` is the stored envelope verbatim, and it MUST', + 'parse to `{ Type, Version, Data }` with a non-null `Data`: the client parses it into', + '`MessageJson` in a post-deserialize hook that only logs on failure, then dereferences the', + 'null. The envelope’s', '`Data` goes through the same profanity filter `api`’s `POST /api/sanitize/v1` runs, masked', 'one `*` per character; every other field is stored as sent. Blank or', 'missing `messageContents` stores nothing and reports invalid-arguments (1), still with', diff --git a/apps/chat/src/openapi.ts b/apps/chat/src/openapi.ts index b7f16f4..92dcd94 100644 --- a/apps/chat/src/openapi.ts +++ b/apps/chat/src/openapi.ts @@ -146,12 +146,57 @@ export const CreateThreadResponse = z.object({ }) /** - * `POST /thread/:id` and `/thread/:id/message` — the whole thread with its messages, not - * just the message that was sent, so the client re-renders the conversation from one - * response. + * The message the SEND answers with, in the PascalCase spelling the client's SendMessage + * handler reads. Six keys — the CLR type has thirteen fields, but the rest are filled in + * after deserialization; notably `MessageJson` is NOT a wire key, it is parsed out of + * `Contents` by a post-deserialize hook. + * + * That hook is why `Contents` must be an escaped JSON envelope with a non-null `Data` + * (`"{\"Type\":0,\"Version\":1,\"Data\":\"hello\"}"`): plain text, an empty string, a + * nested object instead of a string, or `Data: null` all leave `MessageJson` null, the hook + * only LOGS the failure, and the handler then dereferences it — a null-reference exception + * with no other symptom. + * + * Deliberately not {@link ChatMessageDto}, which is the camelCase shape the thread payloads + * carry. Same six fields, two spellings, because the client reads them in two places. + */ +export const SentChatMessage = z.object({ + ChatMessageId: z.int(), + ChatThreadId: z.int(), + SenderPlayerId: z.int(), + TimeSent: z.string().describe('ISO-8601 UTC instant'), + Contents: z + .string() + .describe( + 'The escaped envelope — must parse to `{ Type, Version, Data }` with a non-null Data' + ), + ModerationState: z + .int() + .describe('0 Active · 11 Junior_Pending · 100/101/102 Moderation_* · 255 MarkedForDelete'), +}) + +/** + * `POST /thread/:id` and `/thread/:id/message`. + * + * FOUR keys, in two spellings, because the client reads this response two ways and neither + * can be dropped: + * + * - `ChatMessage` / `ChatResult` are what the SendMessage handler itself reads. On + * `ChatResult == 0` it dereferences `ChatMessage` immediately, so a success answered + * without one is a null-reference exception in the client. That is what a response of + * only the lowercase pair caused. + * - `chatResult` / `chatThread` carry the WHOLE thread with its messages, which is what the + * conversation re-renders from. + * + * The two result keys are the same number by construction — they are one value serialized + * twice — so a case-insensitive decoder reading either lands on the same answer. */ export const SendMessageResponse = z.object({ - chatResult: ChatResult, + ChatMessage: SentChatMessage.nullable().describe( + 'The message just posted; null only when nothing was posted (ChatResult ≠ 0)' + ), + ChatResult: ChatResult, + chatResult: ChatResult.describe('The same value as `ChatResult` — see above'), chatThread: ChatThreadWithMessagesDto.nullable(), }) diff --git a/apps/chat/src/test/integration/api.test.ts b/apps/chat/src/test/integration/api.test.ts index 8736a6c..980a9d0 100644 --- a/apps/chat/src/test/integration/api.test.ts +++ b/apps/chat/src/test/integration/api.test.ts @@ -1204,6 +1204,39 @@ describe('POST /thread/:id', () => { expect(await getThreadMessages(env.DB, chatThreadId)).toEqual(body.chatThread.messages) }) + it('answers the PascalCase ChatMessage the client’s send handler dereferences', async () => { + const caller = 889020 + const chatThreadId = await createThread(env.DB, [caller, 889021], null, caller) + + const res = await send(caller, `/thread/${chatThreadId}`) + const body = (await res.json()) as { + ChatMessage: Record | null + ChatResult: number + chatResult: number + chatThread: { messages: ChatMessage[] } + } + + // The handler reads `ChatMessage` the moment `ChatResult` is 0 — a success answered + // without one is a null-reference exception inside the client, not a failed parse. + expect(body.ChatResult).toBe(0) + const posted = body.chatThread.messages[0]! + expect(body.ChatMessage).toEqual({ + ChatMessageId: posted.chatMessageId, + ChatThreadId: chatThreadId, + SenderPlayerId: caller, + TimeSent: posted.timeSent, + // The envelope verbatim. The client parses this into `MessageJson` in a + // post-deserialize hook that only LOGS on failure and then dereferences the null, + // so `Contents` has to stay a `{ Type, Version, Data }` string with a non-null Data. + Contents: CONTENTS, + ModerationState: 0, + }) + + // The two result keys are one value serialized twice, so a decoder reading either + // spelling lands on the same answer. + expect(body.chatResult).toBe(body.ChatResult) + }) + it('accepts the /thread/:id/message spelling too', async () => { const caller = 889003 const chatThreadId = await createThread(env.DB, [caller, 889004], null, caller) @@ -1238,8 +1271,17 @@ describe('POST /thread/:id', () => { const res = await send(caller, `/thread/${chatThreadId}`, ' ') expect(res.status).toBe(200) - const body = (await res.json()) as { chatResult: number; chatThread: { messages: unknown[] } } + const body = (await res.json()) as { + ChatMessage: unknown + ChatResult: number + chatResult: number + chatThread: { messages: unknown[] } + } expect(body.chatResult).toBe(1) + expect(body.ChatResult).toBe(1) + // Nothing was posted, so there is no message to carry — and a null is safe here + // precisely because the client only dereferences `ChatMessage` on result 0. + expect(body.ChatMessage).toBeNull() // The thread still comes back — only the opening notice is in it. expect(body.chatThread.messages).toHaveLength(1) expect(await getThreadMessages(env.DB, chatThreadId)).toHaveLength(1)