fix: implement TD-08, remove test sandbox scaffolding from production code

This commit is contained in:
2026-06-12 12:14:30 +02:00
parent 66938c23f2
commit 8d245b3492
3 changed files with 79 additions and 21 deletions

View File

@@ -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 deadline; cancel during a round kills the HTTP request and leaves the
conversation in a consistent persisted state. 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 **Context.** `chat.ex` contains a `:sandbox_ready` send/receive handshake and
`allow_repo_sandbox/1` (with `Code.ensure_loaded?` + blanket `rescue`) purely `allow_repo_sandbox/1` (with `Code.ensure_loaded?` + blanket `rescue`) purely

View File

@@ -197,16 +197,10 @@ defmodule BDS.AI.Chat do
}) do }) do
task = task =
Task.Supervisor.async_nolink(BDS.Tasks.TaskSupervisor, fn -> Task.Supervisor.async_nolink(BDS.Tasks.TaskSupervisor, fn ->
receive do
:sandbox_ready -> :ok
end
do_send_chat_message(conversation, user_message, opts) do_send_chat_message(conversation, user_message, opts)
end) end)
InFlight.register(conversation.id, task.pid) InFlight.register(conversation.id, task.pid)
:ok = allow_repo_sandbox(task.pid)
send(task.pid, :sandbox_ready)
try do try do
await_chat_task(task, chat_await_timeout_ms()) 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) Repo.one(from project in Project, where: project.is_active == true, select: project.id)
end 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(nil), do: nil
defp encode_nullable(value), do: Jason.encode!(value) defp encode_nullable(value), do: Jason.encode!(value)

View File

@@ -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