fix: fence sandbox resets until deletion succeeds

CHANGELOG.md

@@ -33,6 +33,19 @@ upgrade, is in
   and version line; `@agentshit/fountain-sdk` keeps its published versions and
   receives no new ones. See `sdk/typescript/CHANGELOG.md`.
 
+- A sandbox reset is fenced until the provider confirms the deletion. The
+  fence commits before any provider call, so no turn starts on a machine that
+  is on its way out, and the quota slot stays held until the delete is
+  confirmed rather than released on an unconfirmed one. A second reset, an
+  attach and a wake all answer `409 sandbox_reset_pending` instead of sending
+  another delete. Only a `ready` or `suspended` machine resets now; `pending`
+  and `starting` answer `409 sandbox_not_resettable`, because a machine still
+  under construction has no disk to replace. A reset that the provider does
+  not confirm records `sandbox.reset_requested`; `sandbox.reset` now means the
+  delete succeeded. To clear an unconfirmed reset, reap the sandbox from the
+  admin sandbox list. That retires the row and releases the slot; the machine
+  at the provider is then the operator's to check.
+
 ### Added
 
 - **An `acp` runtime launches a named command, so a deterministic program can

apps/fountain/lib/fountain/conversations.ex

@@ -220,6 +220,10 @@ defmodule Fountain.Conversations do
   The persisted previous status decides the transition. Terminal rows reject
   attempts to become active again, including callbacks holding an older struct.
   """
+  # The two statuses a sandbox stops at. `update_sandbox/2` reads this before
+  # `prevent_sandbox_revival/1` does, so it is declared here rather than beside it.
+  @billable_terminal ~w(terminated failed)
+
   def update_sandbox(%Sandbox{} = sandbox, attrs) do
     # A provider callback may still hold a starting/ready struct after reset,
     # cancellation or the provision watchdog retired the persisted row. Read
@@ -237,6 +241,19 @@ defmodule Fountain.Conversations do
           |> prevent_sandbox_revival()
           |> stamp_terminated_at()
 
+        # A reset fence (`reset_sandbox/2`) stops this machine being re-used or
+        # re-purposed while its deletion is unconfirmed. It deliberately does
+        # NOT stop it being finished off, because a retiring write is how the
+        # fence is *meant* to end: the reset's own confirmed destroy, an
+        # operator reaping it from /admin/sandboxes, the agent being deleted,
+        # account deletion, or a ConversationServer giving up on it. Every one
+        # of those callers matches `{:ok, _}`, so refusing them would turn a
+        # provider timeout into a MatchError and strand the row with no way to
+        # retire it at all.
+        if not is_nil(current.reset_requested_at) and
+             Ecto.Changeset.get_field(changeset, :status) not in @billable_terminal,
+           do: Repo.rollback(:sandbox_reset_pending)
+
         case Repo.update(changeset) do
           {:ok, updated} -> {current.status, updated}
           {:error, changeset} -> Repo.rollback(changeset)
@@ -253,8 +270,6 @@ defmodule Fountain.Conversations do
     end
   end
 
-  @billable_terminal ~w(terminated failed)
-
   defp prevent_sandbox_revival(changeset) do
     if changeset.data.status in @billable_terminal and
          Ecto.Changeset.get_field(changeset, :status) not in @billable_terminal do
@@ -1305,7 +1320,7 @@ defmodule Fountain.Conversations do
               on: s.id == c.sandbox_id,
               where:
                 c.id == ^conv_id and s.id == ^sandbox_id and c.user_id == s.user_id and
-                  s.status not in ["terminated", "failed"]
+                  s.status not in ["terminated", "failed"] and is_nil(s.reset_requested_at)
           )
 
         unless attached?, do: Repo.rollback(:sandbox_unavailable)
@@ -2962,13 +2977,30 @@ defmodule Fountain.Conversations do
   transcripts; each is told the machine is gone, so its next prompt takes the
   wake path, which provisions a fresh home and moves the others onto it.
 
-  `sandbox` came from the caller's scoped `get_sandbox/2`. Only a live
-  `persistent` sandbox resets: an ephemeral one is a conversation's own and
-  ends with it (`{:sandbox_not_resettable, "ephemeral"}`), a terminated or
-  failed one is already gone (`{:sandbox_not_resettable, status}`). Refused
-  with `:sandbox_mid_turn` while any conversation on it runs a turn — the
-  check and the row flip share the per-sandbox advisory lock that turn
-  creation takes, so a turn cannot slip in between them.
+  `sandbox` came from the caller's scoped `get_sandbox/2`, but the decision is
+  made on the row re-read under the lock, not on that struct. Only a
+  `persistent` sandbox that is **`ready` or `suspended`** resets: an ephemeral
+  one is a conversation's own and ends with it
+  (`{:sandbox_not_resettable, "ephemeral"}`), and any other status —
+  `pending` and `starting` as much as `terminated` and `failed` — answers
+  `{:sandbox_not_resettable, status}`. A machine still being built has no disk
+  to replace and no confirmed identity to delete, so it is the provision
+  watchdog's to finish, not this function's. Refused with `:sandbox_mid_turn`
+  while any conversation on it runs a turn — the check and the durable reset
+  fence share turn admission's advisory lock.
+
+  A provider error or lost caller leaves the fence and capacity in place;
+  repeated resets return `:sandbox_reset_pending` without another delete, and
+  so does anything that would re-use the machine. **The fence is not a dead
+  end.** A write that retires the row still goes through (`update_sandbox/2`),
+  so an operator reaps it from `/admin/sandboxes`, deleting the agent still
+  works, and account deletion still completes. Reaping is the supported way
+  out of an unconfirmed reset; it terminates the row and releases the quota
+  slot, and whatever the provider did or did not do with the machine is then
+  the operator's to check. There is no automatic reconciliation.
+
+  Two audit rows, not one: `sandbox.reset_requested` when the fence commits,
+  and `sandbox.reset` only when the provider confirms the destroy.
 
   `opts[:reason]` says *why*, and reaches every transcript on the machine and
   the audit row: `"home_reset"` (the owner asked — the default),
@@ -2979,48 +3011,65 @@ defmodule Fountain.Conversations do
   See `create_agent/2` for the rest of `opts` (`:actor`, `:request_ip`).
   """
   def reset_sandbox(%Sandbox{} = sandbox, opts \\ []) do
-    cond do
-      sandbox.mode != "persistent" ->
-        {:error, {:sandbox_not_resettable, "ephemeral"}}
-
-      sandbox.status in ["terminated", "failed"] ->
-        {:error, {:sandbox_not_resettable, sandbox.status}}
-
-      true ->
-        do_reset_sandbox(sandbox, opts)
-    end
+    if Repo.in_transaction?(),
+      do: {:error, :provider_transaction_open},
+      else: do_reset_sandbox(sandbox, opts)
   end
 
-  defp do_reset_sandbox(%Sandbox{id: sandbox_id} = sandbox, opts) do
-    now = DateTime.utc_now() |> DateTime.truncate(:second)
+  defp do_reset_sandbox(sandbox, opts) do
+    now = DateTime.utc_now()
 
     result =
       Repo.transaction(fn ->
         Repo.query!("SELECT pg_advisory_xact_lock($1, $2)", [
           @sandbox_lock_namespace,
-          :erlang.phash2(sandbox_id)
+          :erlang.phash2(sandbox.id)
         ])
 
-        if _unsafe_running_turns_elsewhere(sandbox_id, nil) > 0 do
-          Repo.rollback(:sandbox_mid_turn)
-        else
-          ids =
-            Repo.all(
-              from c in Conversation,
-                where: c.sandbox_id == ^sandbox_id and c.status not in ["terminated", "failed"],
-                select: c.id
-            )
+        current =
+          Repo.one(
+            from s in Sandbox,
+              where: s.id == ^sandbox.id and s.user_id == ^sandbox.user_id,
+              lock: "FOR UPDATE"
+          ) || Repo.rollback(:not_found)
 
-          # A fresh disk has no session to resume (#778).
-          Repo.update_all(from(c in Conversation, where: c.id in ^ids),
-            set: [runtime_session_id: nil, updated_at: now]
-          )
+        cond do
+          current.mode != "persistent" ->
+            Repo.rollback({:sandbox_not_resettable, "ephemeral"})
 
-          ids
+          current.status not in ["ready", "suspended"] ->
+            Repo.rollback({:sandbox_not_resettable, current.status})
+
+          current.reset_requested_at ->
+            Repo.rollback(:sandbox_reset_pending)
+
+          _unsafe_running_turns_elsewhere(current.id, nil) > 0 ->
+            Repo.rollback(:sandbox_mid_turn)
+
+          true ->
+            :ok
         end
+
+        fenced = current |> Ecto.Changeset.change(reset_requested_at: now) |> Repo.update!()
+
+        ids =
+          Repo.all(
+            from c in Conversation,
+              where: c.sandbox_id == ^current.id and c.status not in ["terminated", "failed"],
+              select: c.id
+          )
+
+        # Admission is fenced before these sessions become unusable.
+        Repo.update_all(from(c in Conversation, where: c.id in ^ids),
+          set: [runtime_session_id: nil, updated_at: DateTime.truncate(now, :second)]
+        )
+
+        {fenced, ids}
       end)
 
-    with {:ok, ids} <- result do
+    with {:ok, {fenced, ids}} <- result,
+         :ok <- record_reset_requested(fenced, ids, opts),
+         {:ok, completed} <- finish_sandbox_reset(fenced) do
       reason = Keyword.get(opts, :reason, "home_reset")
       message = reset_message(reason)
 
@@ -3043,24 +3092,67 @@ defmodule Fountain.Conversations do
         end
       end)
 
-      _unsafe_retire_home(sandbox)
-
       Audit.record(%{
-        user_id: sandbox.user_id,
+        user_id: completed.user_id,
         action: "sandbox.reset",
         resource_type: "sandbox",
-        resource_id: sandbox.id,
+        resource_id: completed.id,
         actor: Keyword.get(opts, :actor, "self"),
         request_ip: Keyword.get(opts, :request_ip),
         metadata: %{
-          "agent_id" => sandbox.agent_id,
-          "provider" => sandbox.provider,
+          "agent_id" => completed.agent_id,
+          "provider" => completed.provider,
           "conversations" => length(ids),
           "reason" => reason
         }
       })
 
-      {:ok, _unsafe_get_sandbox!(sandbox.id)}
+      {:ok, completed}
+    end
+  end
+
+  # The fence has committed and the sessions on the machine are already gone,
+  # so this much happened whatever the provider says next. `sandbox.reset` is
+  # kept for the confirmed destroy; without this row an unconfirmed reset
+  # changes tenant state and leaves no trail, and the operator asked to
+  # reconcile it cannot tell who requested it, when, or why. Recorded outside
+  # the transaction, as `record/1` requires. Answers `:ok` so it reads as a
+  # step in the caller's `with`.
+  defp record_reset_requested(sandbox, ids, opts) do
+    Audit.record(%{
+      user_id: sandbox.user_id,
+      action: "sandbox.reset_requested",
+      resource_type: "sandbox",
+      resource_id: sandbox.id,
+      actor: Keyword.get(opts, :actor, "self"),
+      request_ip: Keyword.get(opts, :request_ip),
+      metadata: %{
+        "agent_id" => sandbox.agent_id,
+        "provider" => sandbox.provider,
+        "conversations" => length(ids),
+        "reason" => Keyword.get(opts, :reason, "home_reset")
+      }
+    })
+
+    :ok
+  end
+
+  # Only a confirmed destroy releases capacity. Errors or caller loss leave
+  # the committed fence intact; neither a repeat reset nor the reaper retries it.
+  defp finish_sandbox_reset(sandbox) do
+    handle = Managoat.Sandbox.build_handle(sandbox_provider_atom(sandbox), sandbox.sprite_name)
+
+    case Managoat.Sandbox.destroy(handle) do
+      :ok ->
+        changeset = sandbox |> Sandbox.changeset(%{status: "terminated"}) |> stamp_terminated_at()
+
+        with {:ok, completed} <- Repo.update(changeset) do
+          record_sandbox_usage(sandbox.status, completed)
+          {:ok, completed}
+        end
+
+      {:error, _} ->
+        {:error, :sandbox_reset_pending}
     end
   end
 
@@ -3250,6 +3342,10 @@ defmodule Fountain.Conversations do
     end
   end
 
+  defp check_attachable(%Sandbox{reset_requested_at: at}, _agent, _vault_id, _env_id)
+       when not is_nil(at),
+       do: {:error, :sandbox_reset_pending}
+
   defp check_attachable(%Sandbox{status: status}, _agent, _vault_id, _env_id)
        when status not in ["ready", "suspended"],
        do: {:error, {:sandbox_not_attachable, status}}
@@ -3802,6 +3898,10 @@ defmodule Fountain.Conversations do
 
   defp maybe_reuse_sandbox(%Conversation{sandbox_id: sandbox_id}) do
     case _unsafe_get_sandbox(sandbox_id) do
+      %Sandbox{reset_requested_at: at, status: status}
+      when not is_nil(at) and status not in ["terminated", "failed"] ->
+        {:error, :sandbox_reset_pending}
+
       %{status: status, sprite_name: name} = sandbox
       when status in ["ready", "suspended"] and is_binary(name) ->
         probe_reusable_sandbox(sandbox, sandbox_id)

apps/fountain/lib/fountain/conversations/home_checkpoint.ex

@@ -35,6 +35,11 @@ defmodule Fountain.Conversations.HomeCheckpoint do
   it has been recorded.
   """
   @spec on_park(Sandbox.t()) :: {:ok, String.t()} | :skipped | {:error, term()}
+  # A machine whose reset is unconfirmed keeps no checkpoint: the disk is meant
+  # to be gone, and `record/2` writes through `update_sandbox/2`, which refuses
+  # a non-retiring write to a fenced row while this clause matches `{:ok, _}`.
+  def on_park(%Sandbox{reset_requested_at: at}) when not is_nil(at), do: :skipped
+
   def on_park(%Sandbox{mode: "persistent", sprite_name: name} = sandbox) when is_binary(name) do
     provider = Conversations.sandbox_provider_atom(sandbox)
 

apps/fountain/lib/fountain/conversations/lifecycle.ex

@@ -365,7 +365,13 @@ defmodule Fountain.Conversations.Lifecycle do
       # Ownership: as home?/1 above.
       sandbox = Conversations._unsafe_get_sandbox!(sandbox_id)
 
-      if sandbox.status not in ["terminated", "failed"] do
+      # A machine whose reset is unconfirmed is on its way out, not parking.
+      # `update_sandbox/2` lets a retiring write through the fence but refuses
+      # `suspended`, and this clause matches `{:ok, _}`. The reaper's own park
+      # pass filters the same rows; there is no checkpoint worth taking of a
+      # disk that is meant to be gone.
+      if sandbox.status not in ["terminated", "failed"] and
+           is_nil(sandbox.reset_requested_at) do
         # A home's disk is kept at its quietest moment, where the provider
         # can (ADR 0023, #1073). Best-effort: the park goes ahead either way.
         HomeCheckpoint.on_park(sandbox)

apps/fountain/lib/fountain/conversations/sandbox.ex

@@ -40,6 +40,8 @@ defmodule Fountain.Conversations.Sandbox do
     field :mode, :string, default: "ephemeral"
     field :terminated_at, :utc_datetime
     field :last_resumed_at, :utc_datetime
+    # Internal reset fence; retained after completion as operation evidence.
+    field :reset_requested_at, :utc_datetime_usec
     belongs_to :environment, Environment
     # The identity the disk was materialized from, with the environment
     # (ADR 0023): env vars, packages, repos and setup scripts are written at