diff --git a/TECHDEBTS.md b/TECHDEBTS.md index 1c582a1..bf2535b 100644 --- a/TECHDEBTS.md +++ b/TECHDEBTS.md @@ -349,7 +349,20 @@ better end state; the `after` clause is the cheap immediate fix. deadline; cancel during a round kills the HTTP request and leaves the conversation in a consistent persisted state. -### TD-08: Remove test-sandbox scaffolding from production chat code +### TD-08: Remove test-sandbox scaffolding from production chat code ✅ DONE (2026-06-12) + +**Status: implemented.** `BDS.AI.Chat.send_chat_message/3` now starts the +chat worker directly with `Task.Supervisor.async_nolink/2`; the temporary +`:sandbox_ready` receive/send barrier and `allow_repo_sandbox/1` are deleted +from production code. The repo is on `ecto_sql 3.13.5`, and the supervised +chat task works correctly through sandbox `$callers` propagation in the +project's existing test modes (manual checkout plus the suite's ownership / +shared-mode callers). `test/bds/ai/chat_sandbox_cleanup_test.exs` now proves +both acceptance points: the production module has zero sandbox references, and +`send_chat_message/3` still persists its user/assistant messages through the +supervised task without explicit sandbox allowance. Validation gates are clean: +`mix compile --warnings-as-errors --force`, full `mix test`, and +`mix dialyzer` all pass. **Context.** `chat.ex` contains a `:sandbox_ready` send/receive handshake and `allow_repo_sandbox/1` (with `Code.ensure_loaded?` + blanket `rescue`) purely diff --git a/lib/bds/ai/chat.ex b/lib/bds/ai/chat.ex index 8448ae9..baaabe7 100644 --- a/lib/bds/ai/chat.ex +++ b/lib/bds/ai/chat.ex @@ -197,16 +197,10 @@ defmodule BDS.AI.Chat do }) do task = Task.Supervisor.async_nolink(BDS.Tasks.TaskSupervisor, fn -> - receive do - :sandbox_ready -> :ok - end - do_send_chat_message(conversation, user_message, opts) end) InFlight.register(conversation.id, task.pid) - :ok = allow_repo_sandbox(task.pid) - send(task.pid, :sandbox_ready) try do await_chat_task(task, chat_await_timeout_ms()) @@ -972,20 +966,6 @@ defmodule BDS.AI.Chat do Repo.one(from project in Project, where: project.is_active == true, select: project.id) end - defp allow_repo_sandbox(pid) when is_pid(pid) do - if Code.ensure_loaded?(Ecto.Adapters.SQL.Sandbox) do - try do - Ecto.Adapters.SQL.Sandbox.allow(BDS.Repo, self(), pid) - rescue - _error -> :ok - end - else - :ok - end - - :ok - end - defp encode_nullable(nil), do: nil defp encode_nullable(value), do: Jason.encode!(value) diff --git a/test/bds/ai/chat_sandbox_cleanup_test.exs b/test/bds/ai/chat_sandbox_cleanup_test.exs new file mode 100644 index 0000000..fb32b46 --- /dev/null +++ b/test/bds/ai/chat_sandbox_cleanup_test.exs @@ -0,0 +1,65 @@ +defmodule BDS.AI.ChatSandboxCleanupTest do + use ExUnit.Case, async: false + + alias BDS.AI + alias BDS.Repo + + defmodule FakeSecretBackend do + def encrypt(value), do: {:ok, "enc:" <> value} + + def decrypt("enc:" <> value), do: {:ok, value} + def decrypt(_other), do: {:error, :invalid_ciphertext} + end + + defmodule FinalReplyRuntime do + def generate(endpoint, request, opts) do + send(Keyword.fetch!(opts, :test_pid), {:runtime_request, endpoint, request}) + {:ok, %{content: "All set.", usage: %{input_tokens: 5, output_tokens: 2}}} + end + end + + setup do + :ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo) + :ok + end + + test "chat module does not carry SQL sandbox scaffolding" do + source_path = Path.expand("../../../lib/bds/ai/chat.ex", __DIR__) + source = File.read!(source_path) + + refute source =~ "sandbox_ready" + refute source =~ "allow_repo_sandbox" + refute source =~ "Ecto.Adapters.SQL.Sandbox" + end + + test "send_chat_message persists through the supervised task without explicit sandbox allowance" do + assert {:ok, _endpoint} = + AI.put_endpoint( + :online, + %{ + url: "https://api.example.test/v1", + api_key: "online-secret", + model: "gpt-4o-mini" + }, + secret_backend: FakeSecretBackend + ) + + assert :ok = AI.set_airplane_mode(false) + assert {:ok, conversation} = AI.start_chat(%{title: "Sandbox Cleanup", model: "gpt-4o-mini"}) + + assert {:ok, reply} = + AI.send_chat_message(conversation.id, "How many items are in the blog?", + runtime: FinalReplyRuntime, + test_pid: self(), + secret_backend: FakeSecretBackend + ) + + assert reply.assistant_message.content == "All set." + + messages = AI.list_chat_messages(conversation.id) + assert Enum.map(messages, & &1.role) == [:user, :assistant] + + assert Repo.aggregate(BDS.AI.ChatMessage, :count, :id) == 2 + assert_received {:runtime_request, _endpoint, %{operation: :chat}} + end +end \ No newline at end of file