From c0f3c577ccbabe91d7d1f0740d4cfce1c62e265d Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Sat, 31 Oct 2026 02:12:43 -0400 Subject: [PATCH] Discard conversation titles that arrive after deletion Implemented and locally validated with OpenAI Codex assistance. --- CHANGELOG.md | 6 ++ server/src/channels/summary.ts | 21 +++++- .../tests/channel-summary.integration.test.ts | 74 ++++++++++++++++++- 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f9d3f16f..a774140a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A late conversation title is discarded after deletion + +A generated title that arrives after a conversation was deleted is discarded. The naming job is +settled without changing the deleted row or announcing a new summary, and a missing conversation is +not reported as successfully named. + **Before upgrading.** Sixteen things change for an existing deployment: - Bots now share one computer per person instead of each having its own. Migration `0069_isolated_bots` adds `agent_profiles.isolated`, off for every existing Bot. With the diff --git a/server/src/channels/summary.ts b/server/src/channels/summary.ts index 749a8719b..c7945cde7 100644 --- a/server/src/channels/summary.ts +++ b/server/src/channels/summary.ts @@ -254,11 +254,24 @@ async function summariseOne( const applied = await transaction .update(channels) .set({ summary: title, summaryAt: new Date(), updatedAt: new Date() }) - .where(and(eq(channels.id, channelId), isNull(channels.summary))) + .where( + and( + eq(channels.id, channelId), + isNull(channels.summary), + isNull(channels.deletedAt), + ), + ) .returning({ id: channels.id }); - // Somebody else named it between the claim and this write. Their title stands, and this work - // is done rather than still owed. - if (applied.length === 0) return "written"; + if (applied.length === 0) { + // Another writer's title stands, but a conversation deleted while the model worked is gone. + // Both settle the queue item; only a surviving conversation counts as named. + const [remaining] = await transaction + .select({ id: channels.id }) + .from(channels) + .where(and(eq(channels.id, channelId), isNull(channels.deletedAt))) + .limit(1); + return remaining ? "written" : "nothing to name it with"; + } const members = await transaction .select({ userId: channelMemberships.userId }) diff --git a/server/tests/channel-summary.integration.test.ts b/server/tests/channel-summary.integration.test.ts index 367b898bd..c8f258b1a 100644 --- a/server/tests/channel-summary.integration.test.ts +++ b/server/tests/channel-summary.integration.test.ts @@ -1,7 +1,12 @@ import { afterAll, afterEach, describe, expect, test } from "bun:test"; import { randomUUID } from "node:crypto"; import { FIRING_FRAME, frameFiring } from "@openbot/shared/routine-firing"; -import { eq } from "drizzle-orm"; +import { eq, sql } from "drizzle-orm"; +import postgres from "postgres"; +import { + CHANNEL_ACTIVITY_TOPIC, + type ChannelActivityEvent, +} from "../src/channels/events"; import { createAgentProfileStore } from "../src/agents/profile-store"; import type { AgentActor } from "../src/agents/profile-types"; import { createChannelStore } from "../src/channels/routes"; @@ -360,11 +365,12 @@ describe("naming a claimed conversation", () => { // Offered before it was named, which is exactly the race two replicas produce. await queue.offer({ kind: CHANNEL_SUMMARY_KIND, key: channel.id }); - await summariseClaimedChannels( + const report = await summariseClaimedChannels( options({ title: titler("A second opinion") }), ); expect((await summaryOf(channel.id))?.summary).toBe("Named already"); + expect(report.written).toContain(channel.id); }); test("leaves the conversation unnamed when the model has no answer", async () => { @@ -432,6 +438,70 @@ describe("naming a claimed conversation", () => { expect(item?.attempts).toBe(1); }); + test.each(["soft", "hard"] as const)( + "a %s delete while the model is naming a conversation is final", + async (kind) => { + const owner = await createUser(); + const channel = await createUsedChannel(owner); + await offer(channel.id); + const started = Promise.withResolvers(); + const resume = Promise.withResolvers(); + const work = summariseClaimedChannels( + options({ + title: async () => { + started.resolve(); + await resume.promise; + return "A title that arrived after deletion"; + }, + }), + ); + void work.catch(started.reject); + const notifications = postgres(databaseUrl, { max: 1 }); + const barrier = `summary-barrier-${randomUUID()}`; + const observed = Promise.withResolvers(); + const summaries: string[] = []; + try { + await notifications.listen(CHANNEL_ACTIVITY_TOPIC, (payload) => { + if (payload === barrier) { + observed.resolve(); + return; + } + const event = JSON.parse(payload) as ChannelActivityEvent; + if (event.channelId === channel.id && event.summary !== undefined) + summaries.push(event.summary); + }); + await started.promise; + if (kind === "soft") + expect(await store.softDelete(owner, channel.id)).toBe(true); + else await database.delete(channels).where(eq(channels.id, channel.id)); + resume.resolve(); + const report = await work; + // NOTIFY is commit-ordered. This marker follows the completed worker transaction, so + // observing it also observes any summary announcement that worker could have committed. + await database.execute( + sql`select pg_notify(${CHANNEL_ACTIVITY_TOPIC}, ${barrier})`, + ); + await observed.promise; + expect(summaries).toEqual([]); + const row = await summaryOf(channel.id); + if (kind === "soft") { + expect(row?.summary).toBeNull(); + expect(row?.summaryAt).toBeNull(); + } else expect(row).toBeUndefined(); + expect(report.written).not.toContain(channel.id); + const [item] = await database + .select({ finishedAt: workItems.finishedAt }) + .from(workItems) + .where(eq(workItems.key, channel.id)); + expect(item?.finishedAt).not.toBeNull(); + } finally { + resume.resolve(); + await Promise.allSettled([work]); + await notifications.end({ timeout: 1 }); + } + }, + ); + test("a soft-deleted conversation is dropped the same way", async () => { const owner = await createUser(); const channel = await createUsedChannel(owner);