OpenAgents

Commit

Preload the box a cancelled run belongs to, so cancel can answer

6ecafbabbebe · AtlantisPleb · · parent ab5395c7642d 

Preload the box a cancelled run belongs to, so cancel can answer

`POST /api/v1/boxes/:box_id/runs/:run_id/cancel` returned 500 in production
after the cancellation had already been written. The run really was cancelled
and the caller was told the request failed, which is the worst shape available:
a client that retries repeats something that already happened, and a client
that reports the error reports a failure that did not occur.

`BoxRuns.cancel/1` builds its result from a locked `Repo.one!` inside the
transaction, which returns a bare row. `start_run/6`, `list_runs/2`, and
`get_run/3` all preload `:conversation_box`; `cancel/1` was the one path that
did not, and both of its branches — the terminal early return and the one that
goes through `cancel_worker/1` — handed back a run whose association was
unloaded. So it now preloads once, after both branches, as its siblings do.

One path diverging from three had two consumers, not one.
`BoxRunController.projection/1` reads `run.conversation_box.box_id` and
`Delegations.delegation_projection/2` reads `run.conversation_box.id`, so
cancelling through the delegation surface raised the same `KeyError` on the
same association. `BoxRunServer` survived only because its `box_id/1` falls
back to a lookup when the match on the loaded association fails; the two
projections have no such fallback.

The tests assert the response body, not just the status. A 500 here still
leaves correct data behind, so a test that checked only the database would have
passed while the caller saw a crash. Reverting the preload fails all four: both
controller tests, the delegation test, and the association assertion added to
the existing `BoxRuns.cancel/1` test, which pins the contract at the source
rather than at each consumer.

Refs OpenAgentsInc/openagents#108

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

pushed
by user · WAL seq 444 · 2026-08-26T09:58:08.630847Z

Changed files

  • modified lib/openagents/box_runs.ex
  • modified test/openagents/box_runs_test.exs
  • added test/openagents/delegations/box_cancel_test.exs
  • modified test/openagents_web/controllers/box_run_controller_test.exs

Diff

4 files changed, +169 -3

lib/openagents/box_runs.ex modified +10 -3

@@ -80,9 +80,16 @@ defmodule OpenAgents.BoxRuns do

80 80 
           end
81 81 
         end) do
82 82 
      {:ok, cancelled_run} ->
83 
        if Run.terminal?(cancelled_run),
84 
          do: {:ok, cancelled_run},
85 
          else: cancel_worker(cancelled_run)
83 
        # The locked read inside the transaction returns a bare row, so both
84 
        # branches below hand back a run whose `:conversation_box` is unloaded.
85 
        # Every caller projects `run.conversation_box`, so preload here as
86 
        # `start_run/6`, `list_runs/2`, and `get_run/3` already do.
87 
        result =
88 
          if Run.terminal?(cancelled_run),
89 
            do: {:ok, cancelled_run},
90 
            else: cancel_worker(cancelled_run)
91
92 
        with {:ok, run} <- result, do: {:ok, Repo.preload(run, :conversation_box)}
86 93
87 94 
      {:error, reason} ->
88 95 
        {:error, reason}
test/openagents/box_runs_test.exs modified +3

@@ -283,6 +283,9 @@ defmodule OpenAgents.BoxRunsTest do

283 283
284 284 
    assert {:ok, requested} = BoxRuns.cancel(run)
285 285 
    assert requested.cancellation_requested_at
286 
    # Every caller projects the box the run belongs to, so `cancel/1` owes them
287 
    # the association its siblings already preload.
288 
    assert %ConversationBox{box_id: "bx_8bhkse3n"} = requested.conversation_box
286 289
287 290 
    Req.Test.expect(__MODULE__, fn request ->
288 291 
      assert request.body_params["command"] =~ "forge-credential"
test/openagents/delegations/box_cancel_test.exs added +70

@@ -0,0 +1,70 @@

1 
defmodule OpenAgents.Delegations.BoxCancelTest do
2 
  @moduledoc """
3 
  The delegation surface is the second consumer of `BoxRuns.cancel/1`, and it
4 
  reads the same association the run projection does. A run returned without
5 
  `:conversation_box` preloaded fails here exactly as it fails in the controller.
6 
  """
7
8 
  use OpenAgents.DataCase, async: false
9
10 
  alias OpenAgents.Accounts
11 
  alias OpenAgents.Box.ConversationBox
12 
  alias OpenAgents.Box.Run
13 
  alias OpenAgents.Conversations
14 
  alias OpenAgents.Delegations
15 
  alias OpenAgents.Repo
16
17 
  test "cancelling a box delegation answers with the delegation projection" do
18 
    {:ok, user} =
19 
      Accounts.upsert_github_user(%{
20 
        github_id: 918_273,
21 
        github_login: "delegation-box-cancel",
22 
        github_avatar_url: "https://avatars.githubusercontent.com/u/918273?v=4"
23 
      })
24
25 
    {:ok, conversation} = Conversations.ensure_conversation(user)
26
27 
    box =
28 
      %ConversationBox{}
29 
      |> ConversationBox.changeset(%{
30 
        conversation_id: conversation.id,
31 
        box_id: "bx_8bhkse3n",
32 
        state: "ready",
33 
        setup_status: "done"
34 
      })
35 
      |> Repo.insert!()
36
37 
    now = DateTime.utc_now() |> DateTime.truncate(:microsecond)
38 
    id = Ecto.UUID.generate()
39
40 
    run =
41 
      %Run{id: id}
42 
      |> Run.changeset(%{
43 
        conversation_id: conversation.id,
44 
        conversation_box_id: box.id,
45 
        requesting_principal: %{"type" => "user", "id" => user.id},
46 
        command: "echo delegated",
47 
        idempotency_key: "delegation-cancel",
48 
        state: "completed",
49 
        exit_status: 0,
50 
        output: "delegated",
51 
        last_output_offset: 9,
52 
        run_directory: "$HOME/.openagents/box-runs/users/#{user.id}/#{id}",
53 
        admitted_at: now,
54 
        dispatch_attempted_at: now,
55 
        finished_at: now,
56 
        deadline_at: DateTime.add(now, 60, :second)
57 
      })
58 
      |> Repo.insert!()
59
60 
    caller = %{user: user, agent: nil, scopes: ["box:control"]}
61
62 
    assert {:ok, projection} =
63 
             Delegations.cancel(caller, conversation.id, "box-run:" <> run.id)
64
65 
    assert projection["id"] == "box-run:" <> run.id
66 
    assert projection["kind"] == "box"
67 
    assert projection["target_id"] == "box:" <> box.id
68 
    assert projection["state"] == "completed"
69 
  end
70 
end
test/openagents_web/controllers/box_run_controller_test.exs modified +86

@@ -162,6 +162,92 @@ defmodule OpenAgentsWeb.BoxRunControllerTest do

162 162 
           }
163 163 
  end
164 164
165 
  test "cancelling a running run answers with the run, not a crash", %{conn: conn} do
166 
    user = github_user("api-token-box-run-cancel")
167 
    {:ok, conversation} = Conversations.ensure_conversation(user)
168 
    box = insert_box(conversation.id, "bx_8bhkse3n")
169
170 
    run =
171 
      insert_run(conversation.id, box.id, user.id, "cancel-running", state: "running", pid: 4242)
172
173 
    worker = start_supervised!({OpenAgents.BoxRunServer, run.id})
174 
    ref = Process.monitor(worker)
175 
    _ = :sys.get_state(worker)
176
177 
    Req.Test.expect(__MODULE__, fn request ->
178 
      assert request.body_params["command"] =~ "kill"
179 
      Req.Test.json(request, %{"stdout" => "OA_CANCELLED=1\n"})
180 
    end)
181
182 
    response =
183 
      conn
184 
      |> put_box_api_token("box-run-cancel")
185 
      |> post(
186 
        "/api/v1/conversations/#{conversation.id}/boxes/#{box.box_id}/runs/#{run.id}/cancel"
187 
      )
188 
      |> json_response(202)
189
190 
    assert response["run"]["id"] == run.id
191 
    assert response["run"]["box_id"] == box.box_id
192 
    assert response["run"]["command"] == "echo worker"
193 
    assert response["run"]["cancellation_requested_at"]
194
195 
    assert_receive {:DOWN, ^ref, :process, ^worker, :normal}
196 
    assert Repo.get!(Run, run.id).cancellation_requested_at
197 
  end
198
199 
  test "cancelling an already finished run answers with the run", %{conn: conn} do
200 
    user = github_user("api-token-box-run-cancel-done")
201 
    {:ok, conversation} = Conversations.ensure_conversation(user)
202 
    box = insert_box(conversation.id, "bx_8bhkse3p")
203 
    run = insert_run(conversation.id, box.id, user.id, "cancel-done", state: "completed")
204
205 
    response =
206 
      conn
207 
      |> put_box_api_token("box-run-cancel-done")
208 
      |> post(
209 
        "/api/v1/conversations/#{conversation.id}/boxes/#{box.box_id}/runs/#{run.id}/cancel"
210 
      )
211 
      |> json_response(202)
212
213 
    assert response["run"]["id"] == run.id
214 
    assert response["run"]["box_id"] == box.box_id
215 
    assert response["run"]["state"] == "completed"
216 
  end
217
218 
  defp insert_box(conversation_id, box_id) do
219 
    %ConversationBox{}
220 
    |> ConversationBox.changeset(%{
221 
      conversation_id: conversation_id,
222 
      box_id: box_id,
223 
      state: "ready",
224 
      setup_status: "done"
225 
    })
226 
    |> Repo.insert!()
227 
  end
228
229 
  defp insert_run(conversation_id, box_pk, user_id, key, options) do
230 
    id = Ecto.UUID.generate()
231 
    now = DateTime.utc_now() |> DateTime.truncate(:microsecond)
232 
    state = Keyword.fetch!(options, :state)
233
234 
    %Run{id: id}
235 
    |> Run.changeset(%{
236 
      conversation_id: conversation_id,
237 
      conversation_box_id: box_pk,
238 
      requesting_principal: %{"type" => "user", "id" => user_id},
239 
      command: "echo worker",
240 
      idempotency_key: key,
241 
      state: state,
242 
      pid: Keyword.get(options, :pid),
243 
      run_directory: "$HOME/.openagents/box-runs/users/#{user_id}/#{id}",
244 
      admitted_at: now,
245 
      dispatch_attempted_at: now,
246 
      deadline_at: DateTime.add(now, 60, :second)
247 
    })
248 
    |> Repo.insert!()
249 
  end
250
165 251 
  defp restore_env(key, nil), do: Application.delete_env(:openagents, key)
166 252 
  defp restore_env(key, value), do: Application.put_env(:openagents, key, value)
167 253 
end

This page updates live while a promote is in flight · changelog