diff --git a/doc/company-chat-deletion.md b/doc/company-chat-deletion.md index 8f782efbd31..ba75f7e8cb4 100644 --- a/doc/company-chat-deletion.md +++ b/doc/company-chat-deletion.md @@ -1,12 +1,12 @@ # Company deletion with chat endpoints -Company deletion must remove chat endpoints before purging their assigned agents. +Company deletion must remove chat endpoints before deleting their referenced issues or assigned agents. The endpoint-to-agent foreign key uses RESTRICT, so waiting for the final company cascade fails while deleting agents. This was reproduced by the Discord scheduling producer test and by the owning company-removal suite. The chat owner participates in the existing company-deletion transaction. It -removes only the requested company's endpoint rows before the agent lifecycle +removes only the requested company's endpoint rows before issue deletion and before the agent lifecycle participant runs. Existing endpoint foreign keys remove their dependent records. The lifecycle owner still requires completed agent termination. The transaction still rolls back all SQL cleanup if a later participant fails. No external calls, @@ -23,3 +23,11 @@ endpoint, preserved namespace ownership, and rollback after a later failure. The suite accepts a validated dedicated external PostgreSQL test database because embedded PostgreSQL cannot start on this host. Server typecheck/build and remote CI/review are tracked separately; this is not evidence of deployed host sleep. + + +Review expanded the fixture to include a conversation and publication linked to +an issue. Both regressions then failed because cleanup ran after issue deletion. +The chat participant now runs immediately after the company row is locked, before +any referenced issue or agent is removed. The same transaction preserves history +on rollback. This supplements the original endpoint-only proof; it does not claim +to remove existing voice-session restrictions. diff --git a/server/src/__tests__/cleanup-removal-service.test.ts b/server/src/__tests__/cleanup-removal-service.test.ts index 44ae502009d..ca8371fb25b 100644 --- a/server/src/__tests__/cleanup-removal-service.test.ts +++ b/server/src/__tests__/cleanup-removal-service.test.ts @@ -6,6 +6,8 @@ import { eq } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { chatEndpoints, + chatConversations, + chatPublications, chatDiscordCommandOwners, toolApplications, toolConnections, @@ -294,7 +296,7 @@ describeEmbeddedPostgres("cleanup removal services", () => { await expect(db.select().from(agents).where(eq(agents.id, agentId))).resolves.toHaveLength(0); await expect(db.select().from(companies).where(eq(companies.id, companyId))).resolves.toHaveLength(0); }); - async function seedEndpoint(companyId: string, agentId: string) { + async function seedEndpoint(companyId: string, agentId: string, issueId: string) { const [app] = await db.insert(toolApplications).values({ companyId, name: "Discord", type: "chat" }).returning(); const [connection] = await db.insert(toolConnections).values({ companyId, applicationId: app.id, uid: randomUUID(), name: "Discord", transport: "chat_sdk", connectionPurpose: "channel", @@ -302,19 +304,28 @@ describeEmbeddedPostgres("cleanup removal services", () => { const [endpoint] = await db.insert(chatEndpoints).values({ companyId, connectionId: connection.id, provider: "discord", publicId: randomUUID(), assignedAgentId: agentId, }).returning(); + const [conversation] = await db.insert(chatConversations).values({ + companyId, endpointId: endpoint.id, issueId, externalConversationId: randomUUID(), externalLabel: "Saved chat", + }).returning(); + await db.insert(chatPublications).values({ + companyId, endpointId: endpoint.id, conversationId: conversation.id, issueId, + idempotencyKey: randomUUID(), payload: { text: "Saved response" }, state: "published", + }); return endpoint; } it("deletes company endpoints before their agents while preserving other companies and ownership tombstones", async () => { const first = await seedFixture(), second = await seedFixture(); - const endpoint = await seedEndpoint(first.companyId, first.agentId); - const other = await seedEndpoint(second.companyId, second.agentId); + const endpoint = await seedEndpoint(first.companyId, first.agentId, first.issueId); + const other = await seedEndpoint(second.companyId, second.agentId, second.issueId); const [owner] = await db.insert(chatDiscordCommandOwners).values({ applicationId: String(1_400_000_000_000_000_000n + BigInt("0x" + randomUUID().replaceAll("-", "").slice(0, 12))), companyId: first.companyId, endpointId: endpoint.id, actionId: randomUUID(), }).returning(); await companyService(db).remove(first.companyId); expect(await db.select().from(chatEndpoints).where(eq(chatEndpoints.id, endpoint.id))).toEqual([]); + expect(await db.select().from(chatConversations).where(eq(chatConversations.companyId, first.companyId))).toEqual([]); + expect(await db.select().from(chatPublications).where(eq(chatPublications.companyId, first.companyId))).toEqual([]); expect(await db.select().from(agents).where(eq(agents.companyId, first.companyId))).toEqual([]); expect(await db.select().from(companies).where(eq(companies.id, first.companyId))).toEqual([]); expect(await db.select().from(chatEndpoints).where(eq(chatEndpoints.id, other.id))).toEqual([other]); @@ -323,13 +334,15 @@ describeEmbeddedPostgres("cleanup removal services", () => { it("rolls back endpoint cleanup when the later agent deletion fails", async () => { const f = await seedFixture(); - const endpoint = await seedEndpoint(f.companyId, f.agentId); + const endpoint = await seedEndpoint(f.companyId, f.agentId, f.issueId); vi.spyOn(agentLifecycleCompanyDeletion, "deleteCompanyData").mockImplementationOnce(async tx => { expect(await tx.select().from(chatEndpoints).where(eq(chatEndpoints.id, endpoint.id))).toEqual([]); throw new Error("agent deletion failed"); }); await expect(companyService(db).remove(f.companyId)).rejects.toThrow("agent deletion failed"); expect(await db.select().from(chatEndpoints).where(eq(chatEndpoints.id, endpoint.id))).toEqual([endpoint]); + expect(await db.select().from(chatConversations).where(eq(chatConversations.endpointId, endpoint.id))).toHaveLength(1); + expect(await db.select().from(chatPublications).where(eq(chatPublications.endpointId, endpoint.id))).toHaveLength(1); expect(await db.select().from(companies).where(eq(companies.id, f.companyId))).toHaveLength(1); expect(await db.select().from(agents).where(eq(agents.id, f.agentId))).toHaveLength(1); }); diff --git a/server/src/services/company-deletion.ts b/server/src/services/company-deletion.ts index cd2baebb399..db5b6e8bede 100644 --- a/server/src/services/company-deletion.ts +++ b/server/src/services/company-deletion.ts @@ -65,6 +65,8 @@ export async function deleteCompany(db: Db, id: string) { const [existing] = await tx.select({ id: companies.id }).from(companies) .where(eq(companies.id, id)).for("no key update"); if (!existing) return null; + await notifyChatCompanyCommandRegistration(tx, id); + await chatCompanyDeletion.deleteCompanyData(tx, id); await notifyDeliveryWork(tx, DELIVERY_QUEUES.skillSourceSync); await agentObservabilityService(tx as unknown as Db).removeForCompanyDeletion(id); // Finance can reference costs and both can reference runs. Incidents @@ -120,8 +122,6 @@ export async function deleteCompany(db: Db, id: string) { await requestRestorePointDeletion(tx, { companyId: id }); await tx.delete(executionWorkspaces).where(eq(executionWorkspaces.companyId, id)); await tx.delete(projects).where(eq(projects.companyId, id)); - await notifyChatCompanyCommandRegistration(tx, id); - await chatCompanyDeletion.deleteCompanyData(tx, id); await agentLifecycleCompanyDeletion.deleteCompanyData(tx, id); await notifyToolConnectionHealth(tx, id); const rows = await tx