[chat] little fix to chat endpoint to not send entire thread

This commit is contained in:
Devin Zuczek
2026-08-24 21:58:24 -04:00
parent 39ac005adc
commit bddd59ec1a
3 changed files with 137 additions and 13 deletions
+45 -8
View File
@@ -282,15 +282,38 @@ function censorContents(contents: string): string {
return censored === data ? contents : JSON.stringify({ ...fields, Data: censored }) 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 * 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 * 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. * 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 * Answers the message just posted (`ChatMessage`/`ChatResult`, which is what the client's
* message that was sent, so the client re-renders the conversation from one response. * own send handler reads) AND the whole thread with its messages (`chatResult`/`chatThread`,
* Blank or missing contents stores nothing and reports invalid-arguments, still with the * which the conversation re-renders from). Blank or missing contents stores nothing and
* thread attached, rather than an error status. * 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<App>) { async function sendToThread(c: Context<App>) {
const id = await authedId(c) const id = await authedId(c)
@@ -320,8 +343,16 @@ async function sendToThread(c: Context<App>) {
} }
const thread = await threadWithMessages(c, chatThreadId, id, DEFAULT_THREAD_MESSAGE_COUNT) 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({ return c.json({
chatResult: posted === null ? CHAT_INVALID_ARGUMENTS : CHAT_SUCCESS, ChatMessage: posted === null ? null : toSentChatMessage(posted),
ChatResult: chatResult,
chatResult,
chatThread: thread, chatThread: thread,
}) })
} }
@@ -449,9 +480,15 @@ function sendToThreadRoute(spelling: string) {
tags: ['Messages'], tags: ['Messages'],
summary: `Send a message to an existing thread (${spelling})`, summary: `Send a message to an existing thread (${spelling})`,
description: [ description: [
'Every message after the one that opened the conversation. Answers', 'Every message after the one that opened the conversation. Answers FOUR keys in two',
'`{ chatResult, chatThread }` — the WHOLE thread with its messages, not just the message', 'spellings: `ChatMessage`/`ChatResult`, which is what the clients own send handler reads',
'that was sent, so the client re-renders the conversation from one response. The envelopes', '— 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 envelopes',
'`Data` goes through the same profanity filter `api`s `POST /api/sanitize/v1` runs, masked', '`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', 'one `*` per character; every other field is stored as sent. Blank or',
'missing `messageContents` stores nothing and reports invalid-arguments (1), still with', 'missing `messageContents` stores nothing and reports invalid-arguments (1), still with',
+49 -4
View File
@@ -146,12 +146,57 @@ export const CreateThreadResponse = z.object({
}) })
/** /**
* `POST /thread/:id` and `/thread/:id/message` — the whole thread with its messages, not * The message the SEND answers with, in the PascalCase spelling the client's SendMessage
* just the message that was sent, so the client re-renders the conversation from one * handler reads. Six keys — the CLR type has thirteen fields, but the rest are filled in
* response. * 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({ 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(), chatThread: ChatThreadWithMessagesDto.nullable(),
}) })
+43 -1
View File
@@ -1204,6 +1204,39 @@ describe('POST /thread/:id', () => {
expect(await getThreadMessages(env.DB, chatThreadId)).toEqual(body.chatThread.messages) expect(await getThreadMessages(env.DB, chatThreadId)).toEqual(body.chatThread.messages)
}) })
it('answers the PascalCase ChatMessage the clients 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<string, unknown> | 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 () => { it('accepts the /thread/:id/message spelling too', async () => {
const caller = 889003 const caller = 889003
const chatThreadId = await createThread(env.DB, [caller, 889004], null, caller) 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}`, ' ') const res = await send(caller, `/thread/${chatThreadId}`, ' ')
expect(res.status).toBe(200) 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)
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. // The thread still comes back — only the opening notice is in it.
expect(body.chatThread.messages).toHaveLength(1) expect(body.chatThread.messages).toHaveLength(1)
expect(await getThreadMessages(env.DB, chatThreadId)).toHaveLength(1) expect(await getThreadMessages(env.DB, chatThreadId)).toHaveLength(1)