Skip to content

Commit f6f753c

Browse files
committed
fix(webapp): merge the transcript under the row lock instead of replacing it
`persistTurn` and `persistMessages` stored the whole `messages` array they were handed. The array is the snapshot the turn started from, so anything another process appended in between was deleted: a wake delivery, a watch consent record, or the terminal card of an investigation the stale sweep had just settled. That last one is unrecoverable — the row is already terminal, so the sweep never selects it again, and the panel is back to "Working…" for ever. Both writes now read the row under `select ... for update` inside the transaction and merge by stable message id: incoming order is kept, a stored message the snapshot does not have goes at the end, and no id appears twice. A message with no id falls back to its content so it cannot be carried over twice. An append-only `chat_messages` table is the better long-term shape; merging under the lock is enough for this architecture and needs no migration.
1 parent 2e23a09 commit f6f753c

3 files changed

Lines changed: 295 additions & 13 deletions

File tree

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
import {
2+
appendChatMessageOnceByChatId,
3+
createChat,
4+
createDashboardAgentDb,
5+
getChatMessages,
6+
getInvestigation,
7+
investigationSettlementMessageId,
8+
persistMessages,
9+
persistTurn,
10+
settleInvestigationAndCloseCard,
11+
upsertInvestigationRevision,
12+
type DashboardAgentDb,
13+
type DashboardAgentDbClient,
14+
} from "@internal/dashboard-agent-db";
15+
import {
16+
investigationStateSchema,
17+
type InvestigationState,
18+
} from "@internal/dashboard-agent-contracts";
19+
import { postgresTest } from "@internal/testcontainers";
20+
import type { PrismaClient } from "@trigger.dev/database";
21+
import { readdirSync, readFileSync } from "node:fs";
22+
import path from "node:path";
23+
import { afterEach, describe, expect } from "vitest";
24+
25+
/**
26+
* The snapshot writes, against a real row.
27+
*
28+
* `persistTurn` and `persistMessages` both store a whole `messages` array, and the
29+
* array they store was read at the start of a turn. Anything another process appended
30+
* in between — a wake, a watch consent record, the terminal card of a settled
31+
* investigation — is only still there if the write merges rather than replaces.
32+
*/
33+
34+
let agentDb: DashboardAgentDb;
35+
let agentDbClient: DashboardAgentDbClient | undefined;
36+
37+
/** Replays every migration in order, so a new migration can't leave the suite on a stale schema. */
38+
async function applyAgentSchema(prisma: PrismaClient) {
39+
const folder = path.resolve(__dirname, "../../../internal-packages/dashboard-agent-db/drizzle");
40+
for (const name of readdirSync(folder)
41+
.filter((file) => file.endsWith(".sql"))
42+
.sort()) {
43+
const sql = readFileSync(path.join(folder, name), "utf8");
44+
for (const statement of sql.split("--> statement-breakpoint")) {
45+
const trimmed = statement.trim();
46+
if (trimmed.length > 0) await prisma.$executeRawUnsafe(trimmed);
47+
}
48+
}
49+
}
50+
51+
const ORG_ID = "org_merge";
52+
const USER_ID = "user_merge";
53+
const PROJECT_REF = "proj_merge";
54+
const ENV_REF = "env_merge";
55+
56+
async function boot(prisma: PrismaClient, connectionUri: string, chatId: string) {
57+
await applyAgentSchema(prisma);
58+
agentDbClient = createDashboardAgentDb(connectionUri, { max: 2 });
59+
agentDb = agentDbClient.db;
60+
await createChat(agentDb, { id: chatId, organizationId: ORG_ID, userId: USER_ID });
61+
}
62+
63+
afterEach(async () => {
64+
await agentDbClient?.close();
65+
agentDbClient = undefined;
66+
});
67+
68+
function textMessage(id: string) {
69+
return { id, role: "assistant" as const, parts: [{ type: "text", text: id }] };
70+
}
71+
72+
async function transcript(chatId: string): Promise<{ id: string }[]> {
73+
return (await getChatMessages(agentDb, {
74+
chatId,
75+
userId: USER_ID,
76+
organizationId: ORG_ID,
77+
})) as { id: string }[];
78+
}
79+
80+
function openState(): InvestigationState {
81+
return investigationStateSchema.parse({
82+
outcome: "in_progress",
83+
severity: "warn",
84+
confidence: "medium",
85+
title: "send-order-receipt keeps failing",
86+
headline: "Checking whether the failures share a payload.",
87+
progress: "Reading the run's spans",
88+
hypotheses: [],
89+
evidence: [],
90+
});
91+
}
92+
93+
describe("the dashboard agent's snapshot writes", () => {
94+
postgresTest(
95+
"persistTurn keeps a message appended after the turn's snapshot was taken",
96+
async ({ prisma, postgresContainer }) => {
97+
const chatId = "chat_merge_turn";
98+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
99+
100+
// The transcript the turn started from.
101+
const snapshot = [textMessage("u1"), textMessage("a1")];
102+
await persistMessages(agentDb, { chatId, messages: snapshot });
103+
104+
// Another process — a wake delivery — appends while the turn is running.
105+
expect(
106+
await appendChatMessageOnceByChatId(agentDb, {
107+
chatId,
108+
message: textMessage("wake:watch_1:fired"),
109+
})
110+
).toBe(true);
111+
112+
// The turn ends and stores its own snapshot plus what it produced.
113+
await persistTurn(agentDb, {
114+
chatId,
115+
messages: [...snapshot, textMessage("a2")],
116+
session: { publicAccessToken: "pat_merge", lastEventId: "1", runId: "run_merge" },
117+
});
118+
119+
// The turn's order is kept and the wake is still in the conversation.
120+
expect((await transcript(chatId)).map((message) => message.id)).toEqual([
121+
"u1",
122+
"a1",
123+
"a2",
124+
"wake:watch_1:fired",
125+
]);
126+
},
127+
30_000
128+
);
129+
130+
postgresTest(
131+
"persistMessages keeps a message appended after its snapshot was taken",
132+
async ({ prisma, postgresContainer }) => {
133+
const chatId = "chat_merge_messages";
134+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
135+
136+
const snapshot = [textMessage("u1")];
137+
await persistMessages(agentDb, { chatId, messages: snapshot });
138+
await appendChatMessageOnceByChatId(agentDb, {
139+
chatId,
140+
message: textMessage("watch-request:watch_1"),
141+
});
142+
143+
// The next turn starts from the stale snapshot the client carried.
144+
await persistMessages(agentDb, { chatId, messages: [...snapshot, textMessage("u2")] });
145+
146+
expect((await transcript(chatId)).map((message) => message.id)).toEqual([
147+
"u1",
148+
"u2",
149+
"watch-request:watch_1",
150+
]);
151+
}
152+
);
153+
154+
/**
155+
* The worst case. The sweep settles a stale investigation and appends its terminal
156+
* card in one transaction; if the next `persistTurn` then replaces the transcript,
157+
* the card is gone for good — the row is already terminal, so the sweep never selects
158+
* it again and the panel is back to "Working…" for ever.
159+
*/
160+
postgresTest(
161+
"a settled investigation's terminal card survives the next persistTurn",
162+
async ({ prisma, postgresContainer }) => {
163+
const chatId = "chat_merge_settled";
164+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
165+
166+
const snapshot = [textMessage("u1")];
167+
await persistMessages(agentDb, { chatId, messages: snapshot });
168+
169+
const created = await upsertInvestigationRevision(agentDb, {
170+
chatId,
171+
projectRef: PROJECT_REF,
172+
environmentRef: ENV_REF,
173+
state: openState(),
174+
});
175+
if (!created.ok) throw new Error("the fixture investigation wasn't created");
176+
177+
const closed = await settleInvestigationAndCloseCard(agentDb, {
178+
id: created.id,
179+
chatId,
180+
note: "Stopped without a verdict.",
181+
});
182+
expect(closed?.closed).toBe(true);
183+
const cardId = investigationSettlementMessageId(created.id, 1);
184+
185+
await persistTurn(agentDb, {
186+
chatId,
187+
messages: [...snapshot, textMessage("a1")],
188+
session: { publicAccessToken: "pat_merge" },
189+
});
190+
191+
expect((await transcript(chatId)).map((message) => message.id)).toContain(cardId);
192+
// And the row it belongs to is still terminal, so nothing will re-open it.
193+
const row = await getInvestigation(agentDb, { id: created.id });
194+
expect(investigationStateSchema.parse(row?.state).outcome).toBe("inconclusive");
195+
},
196+
30_000
197+
);
198+
199+
postgresTest(
200+
"a message the snapshot already has is never stored twice",
201+
async ({ prisma, postgresContainer }) => {
202+
const chatId = "chat_merge_dedupe";
203+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
204+
205+
const snapshot = [textMessage("u1"), textMessage("a1")];
206+
await persistMessages(agentDb, { chatId, messages: snapshot });
207+
// The client's snapshot carries the host-appended message too, which is the
208+
// ordinary case once the panel has reloaded.
209+
await appendChatMessageOnceByChatId(agentDb, { chatId, message: textMessage("wake:w1") });
210+
211+
await persistTurn(agentDb, {
212+
chatId,
213+
messages: [...snapshot, textMessage("wake:w1"), textMessage("a2")],
214+
session: { publicAccessToken: "pat_merge" },
215+
});
216+
217+
expect((await transcript(chatId)).map((message) => message.id)).toEqual([
218+
"u1",
219+
"a1",
220+
"wake:w1",
221+
"a2",
222+
]);
223+
}
224+
);
225+
});

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

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -279,15 +279,64 @@ export async function softDeleteChat(
279279
});
280280
}
281281

282+
/** Identity for the merge: the stable message id, or the content when a message has none. */
283+
function messageKey(message: unknown): string {
284+
const id = (message as { id?: unknown } | null)?.id;
285+
return typeof id === "string" && id.length > 0 ? `id:${id}` : `raw:${JSON.stringify(message)}`;
286+
}
287+
288+
/**
289+
* The turn's snapshot, plus whatever the row gained while the turn was running.
290+
*
291+
* A wholesale `SET messages = <snapshot>` deletes anything another process appended
292+
* after the snapshot was taken — a wake, a watch consent record, or the terminal card
293+
* of a settled investigation, which is unrecoverable: the row is already terminal, so
294+
* the stale sweep never selects it again and the panel spins for ever.
295+
*
296+
* Incoming order is kept, a stored message the snapshot doesn't have goes at the end,
297+
* and no key appears twice.
298+
*/
299+
export function mergeStoredMessages(incoming: unknown[], stored: unknown[]): unknown[] {
300+
const seen = new Set<string>();
301+
const merged: unknown[] = [];
302+
for (const message of [...incoming, ...stored]) {
303+
const key = messageKey(message);
304+
if (seen.has(key)) continue;
305+
seen.add(key);
306+
merged.push(message);
307+
}
308+
return merged;
309+
}
310+
311+
/** Reads the row under its own lock, so a concurrent append can't be merged away. */
312+
async function writeMergedMessages(
313+
tx: DashboardAgentDbOrTx,
314+
params: { chatId: string; messages: unknown[] }
315+
): Promise<void> {
316+
const rows = await tx
317+
.select({ messages: chats.messages })
318+
.from(chats)
319+
.where(eq(chats.id, params.chatId))
320+
.limit(1)
321+
.for("update");
322+
323+
const stored = rows[0]?.messages;
324+
await tx
325+
.update(chats)
326+
.set({
327+
messages: mergeStoredMessages(params.messages, Array.isArray(stored) ? stored : []),
328+
lastMessageAt: sql`now()`,
329+
updatedAt: sql`now()`,
330+
})
331+
.where(eq(chats.id, params.chatId));
332+
}
333+
282334
/** No session state, unlike {@link persistTurn}. */
283335
export async function persistMessages(
284336
db: DashboardAgentDb,
285337
params: { chatId: string; messages: unknown[] }
286338
): Promise<void> {
287-
await db
288-
.update(chats)
289-
.set({ messages: params.messages, lastMessageAt: sql`now()`, updatedAt: sql`now()` })
290-
.where(eq(chats.id, params.chatId));
339+
await db.transaction((tx) => writeMergedMessages(tx, params));
291340
}
292341

293342
/**
@@ -447,10 +496,7 @@ export async function persistTurn(
447496
);
448497
const messages = [...params.messages, ...cards.filter((card) => !existing.has(card.id))];
449498

450-
await tx
451-
.update(chats)
452-
.set({ messages, lastMessageAt: sql`now()`, updatedAt: sql`now()` })
453-
.where(eq(chats.id, params.chatId));
499+
await writeMergedMessages(tx, { chatId: params.chatId, messages });
454500

455501
await tx
456502
.insert(chatSessions)

internal-packages/dashboard-agent/src/dashboard-agent.test.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// register at module load.
44
import { mockChatAgent, type MockChatAgentHarness } from "@trigger.dev/sdk/ai/test";
55

6+
import { mergeStoredMessages } from "@internal/dashboard-agent-db";
67
import { convertToModelMessages, tool, type UIMessage } from "ai";
78
import { MockLanguageModelV3 } from "ai/test";
89
import { z } from "zod";
@@ -965,21 +966,24 @@ describe("a turn that ends in an error", () => {
965966
});
966967

967968
/**
968-
* A store that keeps the transcript the way the real one does: `persistTurn`
969-
* overwrites it, `appendMessage` adds one message unless its id is already there.
970-
* Lets the test read history back rather than only count calls.
969+
* A store that keeps the transcript the way the real one does: the snapshot writes
970+
* merge with what the row already holds, `appendMessage` adds one message unless its
971+
* id is already there. Lets the test read history back rather than only count calls.
971972
*/
972973
function transcriptStore(): { store: DashboardAgentStore; history: () => UIMessage[] } {
973974
let messages: UIMessage[] = [];
975+
const merge = (incoming: unknown[]) => {
976+
messages = mergeStoredMessages(incoming, messages) as UIMessage[];
977+
};
974978
const store: DashboardAgentStore = {
975979
ensureChat: async () => undefined,
976-
persistMessages: async (args) => void (messages = args.messages as UIMessage[]),
980+
persistMessages: async (args) => merge(args.messages),
977981
appendMessage: async (args) => {
978982
const message = args.message as UIMessage;
979983
if (!messages.some((m) => m.id === message.id)) messages = [...messages, message];
980984
},
981985
persistTurn: async (args) => {
982-
messages = args.messages as UIMessage[];
986+
merge(args.messages);
983987
return { settled: [] };
984988
},
985989
setChatTitleIfDefault: async () => undefined,
@@ -989,6 +993,13 @@ describe("a turn that ends in an error", () => {
989993
revision: 0,
990994
created: true,
991995
}),
996+
settleInvestigationCard: async (args) => ({
997+
ok: true,
998+
id: args.id,
999+
revision: 1,
1000+
card: { id: args.messageId, role: "assistant", parts: [] },
1001+
closed: true,
1002+
}),
9921003
findOpenInvestigation: async () => null,
9931004
};
9941005
return { store, history: () => messages };

0 commit comments

Comments
 (0)