Skip to content

Commit afc0cfa

Browse files
committed
fix(webapp): close a consented watch investigation's card atomically
The watch lane settled the investigation with `settleOpenInvestigations`, then appended the terminal card as a separate write — and swallowed that write's error, logging it and reporting success. So the row went terminal, the card never arrived, the stale sweep stopped selecting the row because it was no longer `in_progress`, and the user was left on "Working…" with nothing able to repair it. Nothing in production calls the action again on its own, which is exactly why the swallowed error mattered. The lane now writes through `settleInvestigationStateAndCloseCard`: the terminal revision and the closing card commit in one transaction, under the lane's own message id, and the error propagates so the task's retry is a real retry. If the card cannot be rendered the settle rolls back, leaving the `in_progress` row the sweep still selects. The duplicated revision bump is gone with it — one outcome is now one revision. The regression test drives a failing close in one action; it no longer proves recovery by calling the action a second time by hand.
1 parent f6f753c commit afc0cfa

7 files changed

Lines changed: 394 additions & 83 deletions

File tree

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
import {
2+
createChat,
3+
createDashboardAgentDb,
4+
getChatMessages,
5+
getInvestigation,
6+
listStaleOpenInvestigations,
7+
settleInvestigationStateAndCloseCard,
8+
upsertInvestigationRevision,
9+
type DashboardAgentDb,
10+
type DashboardAgentDbClient,
11+
} from "@internal/dashboard-agent-db";
12+
import {
13+
forceSettledInvestigationState,
14+
investigationStateSchema,
15+
type InvestigationState,
16+
} from "@internal/dashboard-agent-contracts";
17+
import { postgresTest } from "@internal/testcontainers";
18+
import type { PrismaClient } from "@trigger.dev/database";
19+
import { readdirSync, readFileSync } from "node:fs";
20+
import path from "node:path";
21+
import { afterEach, describe, expect } from "vitest";
22+
23+
/**
24+
* The write the consented watch investigation closes its card with.
25+
*
26+
* The lane has no `onTurnComplete` to hand settlements to, so it closes the card
27+
* itself. Settling the row and appending the terminal card used to be two operations
28+
* with the append's error swallowed: the row went terminal, the card never arrived,
29+
* the stale sweep stopped selecting the row, and the panel kept spinning for ever.
30+
*/
31+
32+
let agentDb: DashboardAgentDb;
33+
let agentDbClient: DashboardAgentDbClient | undefined;
34+
35+
/** Replays every migration in order, so a new migration can't leave the suite on a stale schema. */
36+
async function applyAgentSchema(prisma: PrismaClient) {
37+
const folder = path.resolve(__dirname, "../../../internal-packages/dashboard-agent-db/drizzle");
38+
for (const name of readdirSync(folder)
39+
.filter((file) => file.endsWith(".sql"))
40+
.sort()) {
41+
const sql = readFileSync(path.join(folder, name), "utf8");
42+
for (const statement of sql.split("--> statement-breakpoint")) {
43+
const trimmed = statement.trim();
44+
if (trimmed.length > 0) await prisma.$executeRawUnsafe(trimmed);
45+
}
46+
}
47+
}
48+
49+
const ORG_ID = "org_watch_card";
50+
const USER_ID = "user_watch_card";
51+
const PROJECT_REF = "proj_watch_card";
52+
const ENV_REF = "env_watch_card";
53+
const MESSAGE_ID = "investigate:watch:watch_1:fired:investigate:settled";
54+
55+
async function boot(prisma: PrismaClient, connectionUri: string, chatId: string) {
56+
await applyAgentSchema(prisma);
57+
agentDbClient = createDashboardAgentDb(connectionUri, { max: 2 });
58+
agentDb = agentDbClient.db;
59+
await createChat(agentDb, { id: chatId, organizationId: ORG_ID, userId: USER_ID });
60+
}
61+
62+
afterEach(async () => {
63+
await agentDbClient?.close();
64+
agentDbClient = undefined;
65+
});
66+
67+
function openState(): InvestigationState {
68+
return investigationStateSchema.parse({
69+
outcome: "in_progress",
70+
severity: "warn",
71+
confidence: "low",
72+
title: "Investigating run_abc123",
73+
headline: "The run finished with errors. Looking into why.",
74+
hypotheses: [],
75+
evidence: [],
76+
});
77+
}
78+
79+
async function seed(chatId: string, state: unknown): Promise<string> {
80+
const created = await upsertInvestigationRevision(agentDb, {
81+
chatId,
82+
projectRef: PROJECT_REF,
83+
environmentRef: ENV_REF,
84+
state,
85+
});
86+
if (!created.ok) throw new Error("the fixture investigation wasn't created");
87+
return created.id;
88+
}
89+
90+
async function transcript(chatId: string): Promise<{ id: string; parts: any[] }[]> {
91+
return (await getChatMessages(agentDb, {
92+
chatId,
93+
userId: USER_ID,
94+
organizationId: ORG_ID,
95+
})) as { id: string; parts: any[] }[];
96+
}
97+
98+
describe("closing a consented watch investigation's card", () => {
99+
postgresTest(
100+
"commits the terminal revision and the card together, under the lane's own message id",
101+
async ({ prisma, postgresContainer }) => {
102+
const chatId = "chat_watch_card";
103+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
104+
const id = await seed(chatId, openState());
105+
106+
const result = await settleInvestigationStateAndCloseCard(agentDb, {
107+
id,
108+
chatId,
109+
projectRef: PROJECT_REF,
110+
environmentRef: ENV_REF,
111+
state: forceSettledInvestigationState(openState()),
112+
messageId: MESSAGE_ID,
113+
});
114+
expect(result).toMatchObject({ ok: true, id, revision: 1, closed: true });
115+
116+
const stored = await transcript(chatId);
117+
expect(stored.map((message) => message.id)).toEqual([MESSAGE_ID]);
118+
expect(stored[0]!.parts[0]!.output.blocks[0]).toMatchObject({ id, revision: 1 });
119+
expect(stored[0]!.parts[0]!.output.blocks[0].investigation.outcome).toBe("inconclusive");
120+
expect(
121+
investigationStateSchema.parse((await getInvestigation(agentDb, { id }))?.state).outcome
122+
).toBe("inconclusive");
123+
124+
// The lane dedupes on the action, so a redelivered kick closes nothing twice.
125+
const again = await settleInvestigationStateAndCloseCard(agentDb, {
126+
id,
127+
chatId,
128+
projectRef: PROJECT_REF,
129+
environmentRef: ENV_REF,
130+
state: forceSettledInvestigationState(openState()),
131+
messageId: MESSAGE_ID,
132+
});
133+
expect(again).toMatchObject({ ok: true, closed: false });
134+
expect((await transcript(chatId)).length).toBe(1);
135+
},
136+
30_000
137+
);
138+
139+
/**
140+
* The regression. A terminal row with no terminal card must be impossible: if the
141+
* card can't be written the settle rolls back, so the row stays `in_progress` and the
142+
* stale sweep still selects it.
143+
*/
144+
postgresTest(
145+
"a card that can't be written rolls the settle back, leaving the row in_progress",
146+
async ({ prisma, postgresContainer }) => {
147+
const chatId = "chat_watch_card_fails";
148+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
149+
const id = await seed(chatId, openState());
150+
151+
// A state the row accepts but no card can be rendered from, so the delivery half
152+
// genuinely fails against a real database.
153+
await expect(
154+
settleInvestigationStateAndCloseCard(agentDb, {
155+
id,
156+
chatId,
157+
projectRef: PROJECT_REF,
158+
environmentRef: ENV_REF,
159+
state: { outcome: "inconclusive" },
160+
messageId: MESSAGE_ID,
161+
})
162+
).rejects.toThrow(/isn't renderable/);
163+
164+
const row = await getInvestigation(agentDb, { id });
165+
expect(row?.revision).toBe(0);
166+
expect((row!.state as { outcome?: string }).outcome).toBe("in_progress");
167+
expect(await transcript(chatId)).toEqual([]);
168+
169+
// Still selectable, so the backstop sweep can finish the job.
170+
const stale = await listStaleOpenInvestigations(agentDb, {
171+
olderThan: new Date(),
172+
limit: 10,
173+
});
174+
expect(stale.map((candidate) => candidate.id)).toEqual([id]);
175+
},
176+
30_000
177+
);
178+
179+
postgresTest(
180+
"refuses a row that belongs to another project, and writes nothing",
181+
async ({ prisma, postgresContainer }) => {
182+
const chatId = "chat_watch_card_tenancy";
183+
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
184+
const id = await seed(chatId, openState());
185+
186+
expect(
187+
await settleInvestigationStateAndCloseCard(agentDb, {
188+
id,
189+
chatId,
190+
projectRef: "proj_someone_else",
191+
environmentRef: ENV_REF,
192+
state: forceSettledInvestigationState(openState()),
193+
messageId: MESSAGE_ID,
194+
})
195+
).toEqual({ ok: false, error: "context_mismatch" });
196+
197+
expect(await transcript(chatId)).toEqual([]);
198+
expect((await getInvestigation(agentDb, { id }))?.revision).toBe(0);
199+
}
200+
);
201+
});

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,3 +850,61 @@ export async function settleInvestigationAndCloseCard(
850850
return { settled, closed };
851851
});
852852
}
853+
854+
export type ClosedInvestigationCard =
855+
| {
856+
ok: true;
857+
id: string;
858+
revision: number;
859+
card: InvestigationCardMessage;
860+
/** False when that message id was already in the chat. */
861+
closed: boolean;
862+
}
863+
| { ok: false; error: "not_found" | "context_mismatch" };
864+
865+
/**
866+
* Same atomicity as {@link settleInvestigationAndCloseCard}, for a caller that brings
867+
* its own terminal state and its own message id — the consented watch investigation,
868+
* which dedupes on the action rather than on the revision.
869+
*
870+
* Throwing is the point: the caller's retry only happens if the failure reaches it, and
871+
* a rolled-back settle leaves the `in_progress` row the stale sweep still selects.
872+
*/
873+
export async function settleInvestigationStateAndCloseCard(
874+
db: DashboardAgentDb,
875+
params: {
876+
id: string;
877+
chatId: string;
878+
projectRef: string;
879+
environmentRef: string;
880+
state: unknown;
881+
messageId: string;
882+
}
883+
): Promise<ClosedInvestigationCard> {
884+
return db.transaction(async (tx) => {
885+
const result = await upsertInvestigationRevision(tx, {
886+
id: params.id,
887+
chatId: params.chatId,
888+
projectRef: params.projectRef,
889+
environmentRef: params.environmentRef,
890+
state: params.state,
891+
});
892+
if (!result.ok) return result;
893+
894+
const card = investigationSettlementMessage({
895+
investigationId: result.id,
896+
revision: result.revision,
897+
state: params.state,
898+
messageId: params.messageId,
899+
});
900+
if (!card) {
901+
throw new Error(`Investigation ${result.id} settled to a state that isn't renderable`);
902+
}
903+
904+
const closed = await appendChatMessageOnceByChatId(tx, {
905+
chatId: params.chatId,
906+
message: card,
907+
});
908+
return { ok: true, id: result.id, revision: result.revision, card, closed };
909+
});
910+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
persistMessages,
99
persistTurn,
1010
setChatTitleIfDefault,
11+
settleInvestigationStateAndCloseCard,
1112
upsertInvestigationRevision,
13+
type ClosedInvestigationCard,
1214
type DashboardAgentDbClient,
1315
type PendingInvestigationSettlement,
1416
type PersistTurnResult,
@@ -84,6 +86,15 @@ export interface DashboardAgentStore {
8486
upsertInvestigationRevision(
8587
args: Parameters<typeof upsertInvestigationRevision>[1]
8688
): Promise<UpsertInvestigationResult>;
89+
/**
90+
* Commit an investigation's terminal revision and its closing card together. The
91+
* lanes that have no `onTurnComplete` to hand settlements to write through this:
92+
* separately, a committed settle whose card failed is a terminal row the stale sweep
93+
* no longer selects, and a spinner nothing can stop.
94+
*/
95+
settleInvestigationCard(
96+
args: Parameters<typeof settleInvestigationStateAndCloseCard>[1]
97+
): Promise<ClosedInvestigationCard>;
8798
/**
8899
* The freshest card this chat still has open. A consented wake's investigating
89100
* turn must revise the row the wake seeded, not open a second one.
@@ -272,6 +283,7 @@ export function getStore(): DashboardAgentStore {
272283
persistTurn: (args) => persistTurn(db, args),
273284
setChatTitleIfDefault: (args) => setChatTitleIfDefault(db, args),
274285
upsertInvestigationRevision: (args) => upsertInvestigationRevision(db, args),
286+
settleInvestigationCard: (args) => settleInvestigationStateAndCloseCard(db, args),
275287
findOpenInvestigation: (args) => findOpenInvestigationForChat(db, args),
276288
});
277289
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ const NOOP_STORE: DashboardAgentStore = {
4646
revision: 0,
4747
created: true,
4848
}),
49+
settleInvestigationCard: async (args) => ({
50+
ok: true,
51+
id: args.id,
52+
revision: 1,
53+
card: { id: args.messageId, role: "assistant", parts: [] },
54+
closed: true,
55+
}),
4956
findOpenInvestigation: async () => null,
5057
};
5158

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export type StoreCalls = {
8484
persistTurn: unknown[];
8585
setChatTitleIfDefault: unknown[];
8686
upsertInvestigationRevision: unknown[];
87+
settleInvestigationCard: unknown[];
8788
findOpenInvestigation: unknown[];
8889
/** Every write in the order it happened, for the tests that assert ordering. */
8990
order: (keyof Omit<StoreCalls, "order">)[];
@@ -99,6 +100,7 @@ export function fakeStore(
99100
persistTurn: [],
100101
setChatTitleIfDefault: [],
101102
upsertInvestigationRevision: [],
103+
settleInvestigationCard: [],
102104
findOpenInvestigation: [],
103105
order: [],
104106
};
@@ -109,6 +111,7 @@ export function fakeStore(
109111
// Revisions bump the way the real query does: latest-wins in the transcript is only
110112
// testable if a later revision is actually a higher number.
111113
const revisions = new Map<string, number>();
114+
const closedCards = new Set<string>();
112115
const store: DashboardAgentStore = {
113116
ensureChat: async (args) => record("ensureChat", args),
114117
persistMessages: async (args) => record("persistMessages", args),
@@ -146,6 +149,23 @@ export function fakeStore(
146149
revisions.set(id, revision);
147150
return { ok: true, id, revision, created: !args.id };
148151
},
152+
// Mirrors the real query: the terminal revision and its closing card are one
153+
// operation, so a card that can't be delivered leaves no settled row behind.
154+
settleInvestigationCard: async (args) => {
155+
record("settleInvestigationCard", args);
156+
const revision = (revisions.get(args.id) ?? 0) + 1;
157+
const card = investigationSettlementMessage({
158+
investigationId: args.id,
159+
revision,
160+
state: args.state,
161+
messageId: args.messageId,
162+
});
163+
if (!card) throw new Error(`${args.id} settled to a state that isn't renderable`);
164+
revisions.set(args.id, revision);
165+
const closed = !closedCards.has(args.messageId);
166+
closedCards.add(args.messageId);
167+
return { ok: true, id: args.id, revision, card, closed };
168+
},
149169
findOpenInvestigation: async (args) => {
150170
record("findOpenInvestigation", args);
151171
return options.openInvestigation ?? null;

0 commit comments

Comments
 (0)