fix(webapp): keep a malformed message's own text out of the error, and pin a finalisation to its body id

The malformed-message error carried 200 characters of the payload, which can be user text or tool output. It now names the shape only. A finalisation also verifies that the body's id is the row it targets, so the stored key and the payload cannot name different messages. The legacy-column guard missed a schema-qualified update, and now self-tests both spellings.

Author: kathiekiwi

apps/webapp/test/dashboardAgentLegacyMessagesColumn.test.ts

@@ -19,15 +19,23 @@ const SCANNED = [
   "internal-packages/dashboard-agent-db/src",
 ];
 
+/**
+ * A tripwire, not a proof. It does not see SQL assembled from separate fragments, queries
+ * outside the scanned directories, anything run by hand or by an external tool, or an
+ * aliased table (`from chats c … c.messages`). Migrations are skipped on purpose.
+ */
+
 /** A qualified reference to the dropped column, in any of the spellings Postgres accepts. */
 const QUALIFIED = /"?\bchats"?\s*\.\s*"?messages"?/i;
 
 /** An unqualified one, inside a literal that is plainly SQL against `chats`. */
 const SQL_LITERAL = /`[^`]*`|"(?:[^"\\\n]|\\.)*"|'(?:[^'\\\n]|\\.)*'/g;
 const SQL_VERB = /\b(select|insert\s+into|update|delete\s+from)\b/i;
-const NAMES_CHATS = /(?<![\w."])chats\b/i;
-// Word-boundary on both sides and no leading `_`, so `chat_messages` is not a hit.
-const BARE_MESSAGES = /(?<![\w."])messages\b/i;
+// The quote is part of the match, not excluded before it: a schema-qualified
+// `"trigger_dashboard_agent"."chats"` has a `"` immediately before the name.
+const NAMES_CHATS = /(?<!\w)"?chats\b"?/i;
+// No leading `\w`, so `chat_messages` is not a hit.
+const BARE_MESSAGES = /(?<!\w)"?messages\b"?/i;
 
 function sourceFiles(dir: string): string[] {
   const found: string[] = [];
@@ -46,24 +54,26 @@ function sourceFiles(dir: string): string[] {
   return found;
 }
 
-function offences(file: string): string[] {
-  const text = readFileSync(file, "utf8");
+function offencesForText(text: string, label: string): string[] {
   const found: string[] = [];
 
   for (const [index, line] of text.split("\n").entries()) {
-    if (QUALIFIED.test(line))
-      found.push(`${path.relative(ROOT, file)}:${index + 1} ${line.trim()}`);
+    if (QUALIFIED.test(line)) found.push(`${label}:${index + 1} ${line.trim()}`);
   }
 
   for (const literal of text.match(SQL_LITERAL) ?? []) {
     if (!SQL_VERB.test(literal) || !NAMES_CHATS.test(literal)) continue;
     if (!BARE_MESSAGES.test(literal)) continue;
-    found.push(`${path.relative(ROOT, file)} (sql literal) ${literal.slice(0, 120)}`);
+    found.push(`${label} (sql literal) ${literal.slice(0, 120)}`);
   }
 
   return found;
 }
 
+function offences(file: string): string[] {
+  return offencesForText(readFileSync(file, "utf8"), path.relative(ROOT, file));
+}
+
 describe("the dropped chats.messages column", () => {
   it("is not referenced by any production source, including in raw SQL", () => {
     const files = SCANNED.flatMap((dir) => sourceFiles(path.join(ROOT, dir)));
@@ -72,4 +82,21 @@ describe("the dropped chats.messages column", () => {
 
     expect(files.flatMap(offences)).toEqual([]);
   });
+
+  // Without these the scan could rot into a pass-everything no-op.
+  it("catches a schema-qualified update of the column", () => {
+    expect(
+      offencesForText(
+        'sql`UPDATE "trigger_dashboard_agent"."chats" SET "messages" = ${value} WHERE "id" = ${chatId}`',
+        "fixture"
+      )
+    ).not.toEqual([]);
+  });
+
+  it("catches an unqualified update, and leaves chat_messages alone", () => {
+    expect(offencesForText("sql`update chats set messages = ${next}`", "fixture")).not.toEqual([]);
+    expect(
+      offencesForText("sql`insert into chat_messages (message) values (${row})`", "fixture")
+    ).toEqual([]);
+  });
 });

apps/webapp/test/dashboardAgentTranscriptStore.test.ts

@@ -334,13 +334,30 @@ describe("invariant 3: an ordinary transcript write can never change a stored me
       const chatId = "chat_malformed";
       await boot(prisma, postgresContainer.getConnectionUri(), chatId);
 
+      // The shape, never the values: a malformed message can carry user text.
       await expect(
-        persistMessages(agentDb, { chatId, messages: [{ role: "user", parts: [] }] })
-      ).rejects.toThrow(/Chat chat_malformed was handed a message with no id: .*"role":"user"/);
+        persistMessages(agentDb, {
+          chatId,
+          messages: [
+            { role: "user", parts: [{ type: "text", text: "card 4242 for alice@x.test" }] },
+          ],
+        })
+      ).rejects.toThrow(
+        /Chat chat_malformed was handed a message with no id: object with keys: role, parts$/
+      );
 
       await expect(
         persistMessages(agentDb, { chatId, messages: [{ id: "a1", parts: [] }] })
-      ).rejects.toThrow(/Chat chat_malformed was handed a message with no role: .*"id":"a1"/);
+      ).rejects.toThrow(
+        /Chat chat_malformed was handed a message with no role: object with keys: id, parts$/
+      );
+
+      const leaked = await persistMessages(agentDb, {
+        chatId,
+        messages: [{ role: "user", parts: [{ type: "text", text: "alice@x.test" }] }],
+      }).catch((error: Error) => error.message);
+      expect(leaked).not.toContain("alice@x.test");
+      expect(leaked).not.toContain("4242");
     },
     30_000
   );
@@ -406,6 +423,31 @@ describe("invariant 4: a controlled finalisation changes the body and nothing el
     30_000
   );
 
+  postgresTest(
+    "a finalisation whose body names another message is refused",
+    async ({ prisma, postgresContainer }) => {
+      const chatId = "chat_finalise_id";
+      await boot(prisma, postgresContainer.getConnectionUri(), chatId);
+
+      await persistMessages(agentDb, { chatId, messages: [textMessage("a1", "still working")] });
+      const before = await rows(prisma, chatId);
+
+      // The row key would stay `a1` while the body claims `a2`, so a later read
+      // would hand the UI a message under the wrong identity.
+      await expect(
+        finalizeChatMessage(agentDb, {
+          chatId,
+          messageId: "a1",
+          expectedRole: "assistant",
+          message: { id: "a2", role: "assistant", parts: [{ type: "text", text: "done" }] },
+        })
+      ).rejects.toThrow(/finalisation target a1 carries body id a2/);
+
+      expect(await rows(prisma, chatId)).toEqual(before);
+    },
+    30_000
+  );
+
   postgresTest(
     "the row's role and the body's role cannot be made to disagree",
     async ({ prisma, postgresContainer }) => {

internal-packages/dashboard-agent-db/src/queries.ts

@@ -281,19 +281,20 @@ export async function softDeleteChat(
 }
 
 /** Enough of the payload to recognise it in an error, without logging a whole transcript. */
-function describeMessage(message: unknown): string {
-  try {
-    return JSON.stringify(message)?.slice(0, 200) ?? String(message);
-  } catch {
-    return String(message);
-  }
+// Shape only. A malformed message can carry user text, tool output or source.
+function describeMessageShape(message: unknown): string {
+  if (!message || typeof message !== "object") return typeof message;
+  const keys = Object.keys(message);
+  return `object with keys: ${keys.slice(0, 10).join(", ")}${keys.length > 10 ? ", …" : ""}`;
 }
 
 /** Row identity. Checked here so a malformed message names itself, not a `NOT NULL` violation. */
 function messageIdOf(chatId: string, message: unknown): string {
   const id = (message as { id?: unknown } | null | undefined)?.id;
   if (typeof id !== "string" || id.length === 0) {
-    throw new Error(`Chat ${chatId} was handed a message with no id: ${describeMessage(message)}`);
+    throw new Error(
+      `Chat ${chatId} was handed a message with no id: ${describeMessageShape(message)}`
+    );
   }
   return id;
 }
@@ -303,7 +304,7 @@ function messageRoleOf(chatId: string, message: unknown): string {
   const role = (message as { role?: unknown } | null | undefined)?.role;
   if (typeof role !== "string" || role.length === 0) {
     throw new Error(
-      `Chat ${chatId} was handed a message with no role: ${describeMessage(message)}`
+      `Chat ${chatId} was handed a message with no role: ${describeMessageShape(message)}`
     );
   }
   return role;
@@ -420,6 +421,13 @@ export async function finalizeChatMessage(
     );
   }
 
+  const bodyMessageId = messageIdOf(params.chatId, params.message);
+  if (bodyMessageId !== params.messageId) {
+    throw new Error(
+      `Chat ${params.chatId} finalisation target ${params.messageId} carries body id ${bodyMessageId}`
+    );
+  }
+
   const rows = await db
     .update(chatMessages)
     .set({ message: params.message })