From 985d8b53c2b6f4d8c1799894eee4ac98b1c84e13 Mon Sep 17 00:00:00 2001
From: Chili Palmer <hugoms@me.com>
Date: Fri, 12 Jun 2026 14:09:59 +0200
Subject: [PATCH] Close TD-22 chat delete transaction

---
 TECHDEBTS.md         | 12 +++++++++++-
 lib/bds/ai/chat.ex   | 29 ++++++++++++++++++++++++-----
 test/bds/ai_test.exs | 30 ++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/TECHDEBTS.md b/TECHDEBTS.md
index 1f7a4b6..8b7db5c 100644
--- a/TECHDEBTS.md
+++ b/TECHDEBTS.md
@@ -824,7 +824,17 @@ are scanned (check rebuild-from-files glob patterns ignore them).
 **Acceptance.** Concurrent-writer test produces two intact outcomes (last
 write wins, no corruption); rebuild file globs ignore temp files.
 
-### TD-22: Wrap `delete_chat_conversation` in a transaction
+### TD-22: Wrap `delete_chat_conversation` in a transaction ✅ DONE (2026-06-12)
+
+**Status: implemented.** `BDS.AI.Chat.delete_chat_conversation/1` now performs
+message deletion and conversation deletion inside a single `Repo.transaction`.
+If the second step fails, the whole delete rolls back and both the
+conversation and its messages remain intact. The regression test injects a
+mid-delete failure through a local test hook and proves the rollback by
+asserting both rows still exist afterward. The nearby audit requested by this
+TD found no equivalent multi-statement write outlier in `lib/bds/ai/catalog.ex`
+or `lib/bds/scripts.ex`: catalog refresh already runs in a transaction and the
+script operations are single-row writes.
 
 **Context.** `chat.ex` deletes all messages (`Repo.delete_all`) then the
 conversation (`Repo.delete`) without a transaction — a failure in between
diff --git a/lib/bds/ai/chat.ex b/lib/bds/ai/chat.ex
index baaabe7..a75fa78 100644
--- a/lib/bds/ai/chat.ex
+++ b/lib/bds/ai/chat.ex
@@ -106,12 +106,24 @@ defmodule BDS.AI.Chat do
         {:error, :not_found}
 
       %ChatConversation{} = conversation ->
-        Repo.delete_all(
-          from message in ChatMessage, where: message.conversation_id == ^conversation_id
-        )
+        Repo.transaction(fn ->
+          Repo.delete_all(
+            from message in ChatMessage, where: message.conversation_id == ^conversation_id
+          )
 
-        case Repo.delete(conversation) do
-          {:ok, _conversation} -> {:ok, :deleted}
+          case delete_chat_conversation_test_hook(conversation_id) do
+            :ok ->
+              case Repo.delete(conversation) do
+                {:ok, _conversation} -> :ok
+                {:error, reason} -> Repo.rollback(reason)
+              end
+
+            {:error, reason} ->
+              Repo.rollback(reason)
+          end
+        end)
+        |> case do
+          {:ok, :ok} -> {:ok, :deleted}
           {:error, reason} -> {:error, reason}
         end
     end
@@ -610,6 +622,13 @@ defmodule BDS.AI.Chat do
     end
   end
 
+  defp delete_chat_conversation_test_hook(conversation_id) do
+    case Application.get_env(:bds, :chat_delete_conversation_test_hook) do
+      hook when is_function(hook, 1) -> hook.(conversation_id)
+      _other -> :ok
+    end
+  end
+
   defp persist_assistant_response(conversation_id, response) do
     usage = normalize_usage(response.usage)
 
diff --git a/test/bds/ai_test.exs b/test/bds/ai_test.exs
index 9e460aa..0332168 100644
--- a/test/bds/ai_test.exs
+++ b/test/bds/ai_test.exs
@@ -1641,6 +1641,36 @@ defmodule BDS.AITest do
            end)
   end
 
+  test "delete_chat_conversation rolls back message deletion when the conversation delete fails" do
+    previous_hook = Application.get_env(:bds, :chat_delete_conversation_test_hook)
+
+    on_exit(fn ->
+      if is_nil(previous_hook) do
+        Application.delete_env(:bds, :chat_delete_conversation_test_hook)
+      else
+        Application.put_env(:bds, :chat_delete_conversation_test_hook, previous_hook)
+      end
+    end)
+
+    assert {:ok, conversation} = BDS.AI.start_chat(%{title: "Rollback Chat", model: "gpt-4o-mini"})
+
+    message =
+      Repo.insert!(%BDS.AI.ChatMessage{
+        conversation_id: conversation.id,
+        role: :user,
+        content: "keep me",
+        created_at: Persistence.now_ms()
+      })
+
+    Application.put_env(:bds, :chat_delete_conversation_test_hook, fn _conversation_id ->
+      {:error, :forced_failure}
+    end)
+
+    assert {:error, :forced_failure} = BDS.AI.delete_chat_conversation(conversation.id)
+    assert Repo.get(BDS.AI.ChatConversation, conversation.id)
+    assert Repo.get(BDS.AI.ChatMessage, message.id)
+  end
+
   test "non-stat chat tools expose concrete project data" do
     {:ok, project} = create_project_fixture("Concrete Tools")
     %{post: post, media: media} = seed_project_content(project.id)