merge: propagate review-comment fixes from feat/dashboard-agent-ui
📦 Preview packages (pkg.pr.new) / Build and publish previews (push) Has been cancelled
📦 Preview packages (pkg.pr.new) / Build and publish previews (push) Has been cancelled
# Conflicts: # apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts # internal-packages/dashboard-agent-db/src/queries.ts
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
area: webapp
|
||||
type: fix
|
||||
---
|
||||
|
||||
Deleting a dashboard agent chat is now scoped to your organization, so a chat can only be removed from within the org it belongs to.
|
||||
@@ -196,6 +196,9 @@ export function DashboardAgentPanel({
|
||||
if (!res.ok && res.status !== 404) {
|
||||
console.error(`Dashboard agent: failed to open chat ${id} (${res.status})`);
|
||||
toast.error("We couldn't open that chat. Try again in a moment.");
|
||||
// Transient failure: keep the stored pointer so the chat can be reopened.
|
||||
if (seq === openChatRequestSeq.current) setActive(null);
|
||||
return;
|
||||
}
|
||||
const data = res.ok ? ((await res.json()) as OpenedChatResponse) : undefined;
|
||||
if (seq !== openChatRequestSeq.current) return;
|
||||
|
||||
+12
-4
@@ -374,7 +374,11 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
|
||||
// handover was dispatched and no message was sent: a session the call did create in
|
||||
// spite of the error idles out having done nothing. The empty row is all there is to undo.
|
||||
// Swallowed so the start's own error is what surfaces and gets logged.
|
||||
await softDeleteChat(dashboardAgentDb, { chatId, userId }).catch((cleanupError) => {
|
||||
await softDeleteChat(dashboardAgentDb, {
|
||||
chatId,
|
||||
userId,
|
||||
organizationId: project.organizationId,
|
||||
}).catch((cleanupError) => {
|
||||
logger.error("Failed to remove a dashboard agent chat whose start failed", {
|
||||
chatId,
|
||||
error: cleanupError,
|
||||
@@ -636,8 +640,8 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
|
||||
}
|
||||
|
||||
case "delete": {
|
||||
// `deleteChatWithWatches` is owner-scoped but takes no org, so the org scope has
|
||||
// to be enforced here.
|
||||
// Existence check gives a 404 for a chat this caller can't see; the delete itself
|
||||
// is org- and owner-scoped too, and ends the chat's watches in the same transaction.
|
||||
if (
|
||||
!(await chatExists(dashboardAgentDb, {
|
||||
chatId,
|
||||
@@ -648,7 +652,11 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
|
||||
return json({ error: "Chat not found" }, { status: 404 });
|
||||
}
|
||||
// The delete and the watch cancellations land in one transaction.
|
||||
const { cancelledWatches } = await deleteChatWithWatches({ chatId, userId });
|
||||
const { cancelledWatches } = await deleteChatWithWatches({
|
||||
chatId,
|
||||
userId,
|
||||
organizationId: project.organizationId,
|
||||
});
|
||||
return json({ ok: true, cancelledWatches });
|
||||
}
|
||||
|
||||
|
||||
@@ -1184,11 +1184,12 @@ export async function cancelDashboardAgentWatch(params: {
|
||||
|
||||
/**
|
||||
* Delete a chat and end its watches in one transaction, so no live watch is left on an
|
||||
* invisible chat. Owner-scoped, so a chatId the caller doesn't own deletes nothing.
|
||||
* invisible chat. Org- and owner-scoped, so a chatId the caller doesn't own deletes nothing.
|
||||
*/
|
||||
export async function deleteChatWithWatches(params: {
|
||||
chatId: string;
|
||||
userId: string;
|
||||
organizationId: string;
|
||||
}): Promise<{ deleted: boolean; cancelledWatches: number }> {
|
||||
const result = await softDeleteChat(dashboardAgentDb, params);
|
||||
return { deleted: result.deleted, cancelledWatches: result.cancelledWatches.length };
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import {
|
||||
createChat,
|
||||
createDashboardAgentDb,
|
||||
listChats,
|
||||
softDeleteChat,
|
||||
type DashboardAgentDb,
|
||||
type DashboardAgentDbClient,
|
||||
} from "@internal/dashboard-agent-db";
|
||||
import { postgresTest } from "@internal/testcontainers";
|
||||
import type { PrismaClient } from "@trigger.dev/database";
|
||||
import { readdirSync, readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect } from "vitest";
|
||||
|
||||
/** Replays every migration in order, so a new migration can't leave the suite on a stale schema. */
|
||||
async function applyAgentSchema(prisma: PrismaClient) {
|
||||
const folder = path.resolve(__dirname, "../../../internal-packages/dashboard-agent-db/drizzle");
|
||||
const migrations = readdirSync(folder)
|
||||
.filter((file) => file.endsWith(".sql"))
|
||||
.sort();
|
||||
for (const name of migrations) {
|
||||
const sql = readFileSync(path.join(folder, name), "utf8");
|
||||
for (const statement of sql.split("--> statement-breakpoint")) {
|
||||
const trimmed = statement.trim();
|
||||
if (trimmed.length > 0) await prisma.$executeRawUnsafe(trimmed);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let agentDbClient: DashboardAgentDbClient | undefined;
|
||||
|
||||
async function boot(prisma: PrismaClient, connectionUri: string): Promise<DashboardAgentDb> {
|
||||
await applyAgentSchema(prisma);
|
||||
agentDbClient = createDashboardAgentDb(connectionUri, { max: 2 });
|
||||
return agentDbClient.db;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await agentDbClient?.close();
|
||||
agentDbClient = undefined;
|
||||
});
|
||||
|
||||
const ORG = "org_owner";
|
||||
const OTHER_ORG = "org_other";
|
||||
const USER = "user_owner";
|
||||
|
||||
describe("softDeleteChat tenant isolation", () => {
|
||||
postgresTest(
|
||||
"a soft-delete scoped to another org leaves the chat intact",
|
||||
async ({ prisma, postgresContainer }) => {
|
||||
const db = await boot(prisma, postgresContainer.getConnectionUri());
|
||||
|
||||
await createChat(db, { id: "chat_1", organizationId: ORG, userId: USER });
|
||||
|
||||
// Right user, wrong org: must not delete.
|
||||
const wrongOrg = await softDeleteChat(db, {
|
||||
chatId: "chat_1",
|
||||
userId: USER,
|
||||
organizationId: OTHER_ORG,
|
||||
});
|
||||
expect(wrongOrg.deleted).toBe(false);
|
||||
expect(await listChats(db, { organizationId: ORG, userId: USER })).toHaveLength(1);
|
||||
|
||||
// Right org and user: deletes.
|
||||
const rightOrg = await softDeleteChat(db, {
|
||||
chatId: "chat_1",
|
||||
userId: USER,
|
||||
organizationId: ORG,
|
||||
});
|
||||
expect(rightOrg.deleted).toBe(true);
|
||||
expect(await listChats(db, { organizationId: ORG, userId: USER })).toHaveLength(0);
|
||||
},
|
||||
30_000
|
||||
);
|
||||
});
|
||||
@@ -150,7 +150,7 @@ describe("closing a consented watch investigation's card", () => {
|
||||
const chatId = "chat_watch_card_deleted";
|
||||
await boot(prisma, postgresContainer.getConnectionUri(), chatId);
|
||||
const id = await seed(chatId, openState());
|
||||
await softDeleteChat(agentDb, { chatId, userId: USER_ID });
|
||||
await softDeleteChat(agentDb, { chatId, userId: USER_ID, organizationId: ORG_ID });
|
||||
|
||||
expect(
|
||||
await settleInvestigationStateAndCloseCard(agentDb, {
|
||||
|
||||
@@ -784,7 +784,13 @@ describe("the chat cascade and the list view", () => {
|
||||
expect(mine.ok && theirs.ok).toBe(true);
|
||||
if (!mine.ok || !theirs.ok) return;
|
||||
|
||||
expect(await deleteChatWithWatches({ chatId: "chat_1", userId: seeded.user.id })).toEqual({
|
||||
expect(
|
||||
await deleteChatWithWatches({
|
||||
chatId: "chat_1",
|
||||
userId: seeded.user.id,
|
||||
organizationId: seeded.organization.id,
|
||||
})
|
||||
).toEqual({
|
||||
deleted: true,
|
||||
cancelledWatches: 1,
|
||||
});
|
||||
@@ -860,7 +866,11 @@ describe("the chat cascade and the list view", () => {
|
||||
const created = await create({ seeded, chatId: "chat_1" });
|
||||
expect(created.ok).toBe(true);
|
||||
|
||||
await deleteChatWithWatches({ chatId: "chat_1", userId: seeded.user.id });
|
||||
await deleteChatWithWatches({
|
||||
chatId: "chat_1",
|
||||
userId: seeded.user.id,
|
||||
organizationId: seeded.organization.id,
|
||||
});
|
||||
|
||||
const rows = await ctx.prisma.$queryRawUnsafe<{ message_id: string }[]>(
|
||||
`select message_id from trigger_dashboard_agent.chat_messages where chat_id = 'chat_1'`
|
||||
@@ -1518,7 +1528,12 @@ describe("deleting a chat while a watch is being created", () => {
|
||||
await seedChat(seeded, chatId);
|
||||
|
||||
const creating = () => create({ seeded, chatId });
|
||||
const deleting = () => deleteChatWithWatches({ chatId, userId: seeded.user.id });
|
||||
const deleting = () =>
|
||||
deleteChatWithWatches({
|
||||
chatId,
|
||||
userId: seeded.user.id,
|
||||
organizationId: seeded.organization.id,
|
||||
});
|
||||
const [a, b] = deleteFirst
|
||||
? await Promise.all([deleting(), creating()])
|
||||
: await Promise.all([creating(), deleting()]);
|
||||
@@ -1542,7 +1557,11 @@ describe("deleting a chat while a watch is being created", () => {
|
||||
await boot(prisma, postgresContainer.getConnectionUri());
|
||||
const seeded = await seed(prisma, "race");
|
||||
await seedChat(seeded);
|
||||
await deleteChatWithWatches({ chatId: "chat_1", userId: seeded.user.id });
|
||||
await deleteChatWithWatches({
|
||||
chatId: "chat_1",
|
||||
userId: seeded.user.id,
|
||||
organizationId: seeded.organization.id,
|
||||
});
|
||||
|
||||
expect(await create({ seeded })).toMatchObject({ ok: false, code: "chat_not_found" });
|
||||
expect(await listActiveWatchesForChat(ctx.agentDb, { chatId: "chat_1" })).toEqual([]);
|
||||
|
||||
@@ -287,7 +287,7 @@ export async function markChatRead(
|
||||
*/
|
||||
export async function softDeleteChat(
|
||||
db: DashboardAgentDb,
|
||||
params: { chatId: string; userId: string }
|
||||
params: { chatId: string; userId: string; organizationId: string }
|
||||
): Promise<{ deleted: boolean; cancelledWatches: Watch[] }> {
|
||||
return db.transaction(async (tx) => {
|
||||
// The same lock `createWatch` takes, or a concurrent create lands an active
|
||||
@@ -297,7 +297,13 @@ export async function softDeleteChat(
|
||||
const deleted = await tx
|
||||
.update(chats)
|
||||
.set({ deletedAt: sql`now()`, updatedAt: sql`now()` })
|
||||
.where(and(eq(chats.id, params.chatId), eq(chats.userId, params.userId)))
|
||||
.where(
|
||||
and(
|
||||
eq(chats.id, params.chatId),
|
||||
eq(chats.userId, params.userId),
|
||||
eq(chats.organizationId, params.organizationId)
|
||||
)
|
||||
)
|
||||
.returning({ id: chats.id });
|
||||
|
||||
if (deleted.length === 0) return { deleted: false, cancelledWatches: [] };
|
||||
|
||||
@@ -448,6 +448,7 @@ export const dashboardAgent = chat.agent({
|
||||
turn,
|
||||
uiMessages,
|
||||
newMessages,
|
||||
newUIMessages,
|
||||
responseMessage,
|
||||
clientData,
|
||||
chatAccessToken,
|
||||
@@ -474,7 +475,7 @@ export const dashboardAgent = chat.agent({
|
||||
// operation is what could leave a terminal row whose card never arrived — and the
|
||||
// stale sweep only selects `in_progress`, so nothing would ever repair it.
|
||||
// Only what this turn produced may be finalised; the rest of the snapshot is history.
|
||||
const produced = [...(newMessages ?? []), ...(responseMessage ? [responseMessage] : [])]
|
||||
const produced = [...(newUIMessages ?? []), ...(responseMessage ? [responseMessage] : [])]
|
||||
.map((message) => (message as { id?: unknown }).id)
|
||||
.filter((id): id is string => typeof id === "string");
|
||||
|
||||
|
||||
Reference in New Issue
Block a user