Submit stack merges asynchronously per pull request

9606cbce32e8 · Devin AI · · parent bbe6c61debfe

Submit stack merges asynchronously per pull request

One pull request now submits its own merge: PUT
/repos/:owner/:repo/pulls/:pull_number/merge-async resolves the pull
request's active stack, defaults the merge method to merge, and inserts
the durable merge operation for the contiguous prefix ending at that
layer. A poll surface at merge-async/:operation_id reports the external
contract - pending, merged, or failed - and outlives stack membership so
a caller can confirm the landing after the entry is removed.

A concurrent submission now answers 409 with the active operation id,
for merges and rebases both, so the caller polls the existing operation
instead of guessing.

Part of #112.

Co-Authored-By: Christopher David <chris@openagents.com>
Co-Authored-By
Christopher David <chris@openagents.com>

Deploy story

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

Not deployed through the forge lane

No push, promotion, build, or deploy receipt references this commit (receipts are scanned over a bounded recent window). Changes shipped by full node replacement carry their proof in the release gate receipt instead.

Changed files

  • modified lib/openagents/stacks/merge.ex
  • modified lib/openagents/stacks/restack.ex
  • modified lib/openagents_web/api_route_authority.ex
  • modified lib/openagents_web/controllers/stack_controller.ex
  • modified lib/openagents_web/controllers/stack_json.ex
  • modified lib/openagents_web/router.ex
  • modified test/openagents/stacks/restack_test.exs
  • modified test/openagents_web/controllers/stack_controller_test.exs

Diff

8 files changed, +342 -9

lib/openagents/stacks/merge.ex modified +96 -3

@@ -34,6 +34,7 @@ defmodule OpenAgents.Stacks.Merge do

34 34
  alias OpenAgents.Repo
35 35
  alias OpenAgents.Repositories
36 36
  alias OpenAgents.Repositories.Repository
37
  alias OpenAgents.Stacks
37 38
  alias OpenAgents.Stacks.Operation
38 39
  alias OpenAgents.Stacks.Stack
39 40
  alias OpenAgents.Stacks.StackEntry

@@ -90,6 +91,94 @@ defmodule OpenAgents.Stacks.Merge do

90 91
    end
91 92
  end
92 93
94
  @doc """
95
  Requests a merge selected by pull request rather than by stack.
96
97
  The `merge-async` surface submits against one pull request. The request
98
  resolves the pull request's active stack, defaults the merge method to
99
  `merge`, and merges the contiguous prefix ending at that layer. An
100
  unstacked pull request cannot merge through this surface.
101
  """
102
  def request_for_pull_request(
103
        %Repository{} = repository,
104
        pull_number,
105
        params,
106
        %User{} = actor,
107
        key
108
      )
109
      when is_integer(pull_number) and is_binary(key) do
110
    with {:ok, pull_request} <- find_pull_request(repository, pull_number),
111
         {:ok, stack_number} <- active_stack_number(pull_request) do
112
      request =
113
        params
114
        |> Map.put("pull_request_number", pull_number)
115
        |> Map.put_new("merge_method", "merge")
116
117
      request_from_api(repository, stack_number, request, actor, key)
118
    end
119
  end
120
121
  @doc """
122
  Loads one merge operation for the pull request that submitted it.
123
124
  The poll surface outlives stack membership: a merged pull request's
125
  entry is removed, so the lookup goes through the operation's own
126
  request rather than the active entry.
127
  """
128
  def get_operation_for_pull_request(%Repository{} = repository, pull_number, operation_id)
129
      when is_integer(pull_number) do
130
    with {:ok, uuid} <- cast_operation_id(operation_id),
131
         %Operation{} = operation <- find_merge_operation(repository, uuid),
132
         %{"pull_request_number" => ^pull_number} <- operation.request do
133
      {:ok, operation}
134
    else
135
      _missing -> {:error, :operation_not_found}
136
    end
137
  end
138
139
  defp cast_operation_id(operation_id) do
140
    case Ecto.UUID.cast(operation_id) do
141
      {:ok, uuid} -> {:ok, uuid}
142
      :error -> {:error, :operation_not_found}
143
    end
144
  end
145
146
  defp find_merge_operation(repository, uuid) do
147
    Repo.one(
148
      from operation in Operation,
149
        join: stack in Stack,
150
        on: operation.stack_id == stack.id,
151
        where:
152
          operation.id == ^uuid and stack.repository_id == ^repository.id and
153
            operation.kind == "merge"
154
    )
155
  end
156
157
  defp find_pull_request(repository, number) do
158
    pull_request =
159
      Repo.one(
160
        from pr in PullRequest,
161
          join: issue in Issue,
162
          on: pr.issue_id == issue.id,
163
          where: pr.repository_id == ^repository.id and issue.number == ^number
164
      )
165
166
    case pull_request do
167
      nil -> {:error, :pull_request_not_found}
168
      %PullRequest{} -> {:ok, pull_request}
169
    end
170
  end
171
172
  defp active_stack_number(pull_request) do
173
    case Stacks.active_entry_for_pull_request(pull_request) do
174
      nil ->
175
        {:error, :not_stacked}
176
177
      %StackEntry{stack_id: stack_id} ->
178
        {:ok, Repo.one!(from stack in Stack, where: stack.id == ^stack_id, select: stack.number)}
179
    end
180
  end
181
93 182
  @doc """
94 183
  Executes one claimed merge operation to a terminal state.
95 184

@@ -942,14 +1031,18 @@ defmodule OpenAgents.Stacks.Merge do

942 1031
943 1032
  defp ensure_no_active_operation(stack, nil) do
944 1033
    active =
945
      Repo.exists?(
1034
      Repo.one(
946 1035
        from operation in Operation,
947 1036
          where:
948 1037
            operation.stack_id == ^stack.id and
949
              operation.state in ^Operation.active_states()
1038
              operation.state in ^Operation.active_states(),
1039
          limit: 1
950 1040
      )
951 1041
952
    if active, do: {:error, :operation_in_progress}, else: :ok
1042
    case active do
1043
      nil -> :ok
1044
      %Operation{id: id} -> {:error, {:operation_in_progress, id}}
1045
    end
953 1046
  end
954 1047
955 1048
  defp validate_expected_version(nil, _stack), do: :ok
lib/openagents/stacks/restack.ex modified +7 -3

@@ -713,14 +713,18 @@ defmodule OpenAgents.Stacks.Restack do

713 713
714 714
  defp ensure_no_active_operation(stack, nil) do
715 715
    active =
716
      Repo.exists?(
716
      Repo.one(
717 717
        from operation in Operation,
718 718
          where:
719 719
            operation.stack_id == ^stack.id and
720
              operation.state in ^Operation.active_states()
720
              operation.state in ^Operation.active_states(),
721
          limit: 1
721 722
      )
722 723
723
    if active, do: {:error, :operation_in_progress}, else: :ok
724
    case active do
725
      nil -> :ok
726
      %Operation{id: id} -> {:error, {:operation_in_progress, id}}
727
    end
724 728
  end
725 729
726 730
  defp validate_expected_version(nil, _stack), do: :ok
lib/openagents_web/api_route_authority.ex modified +3

@@ -66,6 +66,8 @@ defmodule OpenAgentsWeb.ApiRouteAuthority do

66 66
      "get /api/v3/repos/:owner/:repo/issues/:issue_number/dependencies" => :optional_bearer,
67 67
      "get /api/v3/repos/:owner/:repo/pulls" => :optional_bearer,
68 68
      "get /api/v3/repos/:owner/:repo/pulls/:pull_number" => :optional_bearer,
69
      "get /api/v3/repos/:owner/:repo/pulls/:pull_number/merge-async/:operation_id" =>
70
        :optional_bearer,
69 71
      "get /api/v3/repos/:owner/:repo/stacks" => :optional_bearer,
70 72
      "get /api/v3/repos/:owner/:repo/stacks/:stack_number" => :optional_bearer,
71 73
      "get /api/v3/repos/:owner/:repo/stacks/:stack_number/operations/:operation_id" =>

@@ -125,6 +127,7 @@ defmodule OpenAgentsWeb.ApiRouteAuthority do

125 127
      "post /api/v3/orgs/:org/repos" => :required_bearer,
126 128
      "post /api/v3/orgs/:org/repos/imports" => :required_bearer,
127 129
      "post /api/v3/repos/:owner/:repo/pulls" => :required_bearer,
130
      "put /api/v3/repos/:owner/:repo/pulls/:pull_number/merge-async" => :required_bearer,
128 131
      "post /api/v3/repos/:owner/:repo/stacks" => :required_bearer,
129 132
      "post /api/v3/repos/:owner/:repo/stacks/:stack_number/append" => :required_bearer,
130 133
      "post /api/v3/repos/:owner/:repo/stacks/:stack_number/rebase" => :required_bearer,
lib/openagents_web/controllers/stack_controller.ex modified +64 -1

@@ -102,6 +102,56 @@ defmodule OpenAgentsWeb.StackController do

102 102
    Ecto.NoResultsError -> not_found(conn)
103 103
  end
104 104
105
  def merge_async(conn, %{"owner" => owner, "repo" => repo, "pull_number" => number} = params) do
106
    repository = Repositories.get_visible_by_path!(owner, repo, conn.assigns.current_user)
107
    pull_number = ControllerHelpers.integer_param!(number)
108
109
    with {:ok, idempotency_key} <- idempotency_key(conn),
110
         {:ok, {operation, replay_state}} <-
111
           Merge.request_for_pull_request(
112
             repository,
113
             pull_number,
114
             params,
115
             conn.assigns.current_user,
116
             idempotency_key
117
           ) do
118
      conn
119
      |> put_status(:accepted)
120
      |> render(:merge_async,
121
        operation: operation,
122
        replay_state: replay_state,
123
        owner: owner,
124
        repo: repo,
125
        pull_number: pull_number
126
      )
127
    else
128
      {:error, reason} -> render_error(conn, reason)
129
    end
130
  rescue
131
    Ecto.NoResultsError -> not_found(conn)
132
  end
133
134
  def merge_async_status(conn, %{"owner" => owner, "repo" => repo} = params) do
135
    repository = Repositories.get_visible_by_path!(owner, repo, conn.assigns[:current_user])
136
    pull_number = ControllerHelpers.integer_param!(params["pull_number"])
137
138
    case Merge.get_operation_for_pull_request(repository, pull_number, params["operation_id"]) do
139
      {:ok, operation} ->
140
        render(conn, :merge_async,
141
          operation: operation,
142
          replay_state: nil,
143
          owner: owner,
144
          repo: repo,
145
          pull_number: pull_number
146
        )
147
148
      {:error, reason} ->
149
        render_error(conn, reason)
150
    end
151
  rescue
152
    Ecto.NoResultsError -> not_found(conn)
153
  end
154
105 155
  def show_operation(conn, %{"owner" => owner, "repo" => repo} = params) do
106 156
    repository = Repositories.get_visible_by_path!(owner, repo, conn.assigns[:current_user])
107 157

@@ -172,13 +222,22 @@ defmodule OpenAgentsWeb.StackController do

172 222
       when reason in [:stack_not_found, :pull_request_not_found, :operation_not_found],
173 223
       do: not_found(conn)
174 224
225
  defp render_error(conn, {:operation_in_progress, operation_id}) do
226
    conn
227
    |> put_status(:conflict)
228
    |> json(%{
229
      message: message(:operation_in_progress),
230
      code: "operation_in_progress",
231
      operation_id: operation_id
232
    })
233
  end
234
175 235
  defp render_error(conn, reason)
176 236
       when reason in [
177 237
              :idempotency_conflict,
178 238
              :stale_stack_version,
179 239
              :expected_head_mismatch,
180 240
              :stack_not_open,
181
              :operation_in_progress,
182 241
              :operation_not_waiting,
183 242
              :operation_not_abortable,
184 243
              :merge_queue_unavailable

@@ -199,6 +258,7 @@ defmodule OpenAgentsWeb.StackController do

199 258
              :broken_base_chain,
200 259
              :already_stacked,
201 260
              :not_stack_top,
261
              :not_stacked,
202 262
              :resolution_not_found,
203 263
              :resolution_parent_mismatch,
204 264
              :pull_request_not_in_stack

@@ -241,6 +301,9 @@ defmodule OpenAgentsWeb.StackController do

241 301
  defp message(:pull_request_not_in_stack),
242 302
    do: "The pull request is not an active entry of this stack."
243 303
304
  defp message(:not_stacked),
305
    do: "The pull request does not belong to an active stack."
306
244 307
  defp message(:resolution_parent_mismatch),
245 308
    do: "The resolution commit does not build on the persisted parent."
246 309
lib/openagents_web/controllers/stack_json.ex modified +32

@@ -34,6 +34,38 @@ defmodule OpenAgentsWeb.StackJSON do

34 34
    end
35 35
  end
36 36
37
  def render("merge_async.json", %{operation: operation} = assigns) do
38
    base_url = String.trim_trailing(OpenAgentsWeb.Endpoint.url(), "/")
39
40
    json = %{
41
      operation_id: operation.id,
42
      merge_status: merge_status(operation),
43
      state: operation.state,
44
      merge_method: Map.get(operation.request, "merge_method"),
45
      pull_request: Map.get(operation.request, "pull_request_number"),
46
      error: operation.error,
47
      created_at: operation.inserted_at,
48
      completed_at: operation.completed_at,
49
      url:
50
        "#{base_url}/api/v3/repos/#{assigns.owner}/#{assigns.repo}/pulls/#{assigns.pull_number}/merge-async/#{operation.id}"
51
    }
52
53
    case Map.get(assigns, :replay_state) do
54
      nil -> json
55
      replay_state -> Map.put(json, :replayed, replay_state == :replayed)
56
    end
57
  end
58
59
  # The external contract collapses internal operation states into the
60
  # three the poll surface promises: a submitted merge is pending until
61
  # it either lands or terminates without landing.
62
  defp merge_status(%{state: state})
63
       when state in ~w(pending running waiting_for_conflict_resolution waiting_for_checks),
64
       do: "pending"
65
66
  defp merge_status(%{state: "succeeded"}), do: "merged"
67
  defp merge_status(_operation), do: "failed"
68
37 69
  defp stack(stack, assigns) do
38 70
    base_url = String.trim_trailing(OpenAgentsWeb.Endpoint.url(), "/")
39 71
    owner = assigns.owner
lib/openagents_web/router.ex modified +6

@@ -483,6 +483,11 @@ defmodule OpenAgentsWeb.Router do

483 483
484 484
    get "/repos/:owner/:repo/pulls", PullRequestController, :index
485 485
    get "/repos/:owner/:repo/pulls/:pull_number", PullRequestController, :show
486
487
    get "/repos/:owner/:repo/pulls/:pull_number/merge-async/:operation_id",
488
        StackController,
489
        :merge_async_status
490
486 491
    get "/repos/:owner/:repo/stacks", StackController, :index
487 492
    get "/repos/:owner/:repo/stacks/:stack_number", StackController, :show
488 493

@@ -541,6 +546,7 @@ defmodule OpenAgentsWeb.Router do

541 546
    patch "/repos/:owner/:repo/issues/:issue_number", IssueController, :update
542 547
    post "/repos/:owner/:repo/pulls", PullRequestController, :create
543 548
    patch "/repos/:owner/:repo/pulls/:pull_number", PullRequestController, :update
549
    put "/repos/:owner/:repo/pulls/:pull_number/merge-async", StackController, :merge_async
544 550
    post "/repos/:owner/:repo/stacks", StackController, :create
545 551
    post "/repos/:owner/:repo/stacks/:stack_number/append", StackController, :append
546 552
    post "/repos/:owner/:repo/stacks/:stack_number/rebase", StackController, :rebase
test/openagents/stacks/restack_test.exs modified +4 -2

@@ -376,10 +376,12 @@ defmodule OpenAgents.Stacks.RestackTest do

376 376
                 "restack-active-3"
377 377
               )
378 378
379
      {:ok, {_operation, :created}} =
379
      {:ok, {operation, :created}} =
380 380
        Restack.request_from_api(repository, stack.number, %{}, actor, "restack-active-1")
381 381
382
      assert {:error, :operation_in_progress} =
382
      operation_id = operation.id
383
384
      assert {:error, {:operation_in_progress, ^operation_id}} =
383 385
               Restack.request_from_api(repository, stack.number, %{}, actor, "restack-active-2")
384 386
    end
385 387
test/openagents_web/controllers/stack_controller_test.exs modified +130

@@ -521,6 +521,133 @@ defmodule OpenAgentsWeb.StackControllerTest do

521 521
    end
522 522
  end
523 523
524
  describe "PUT /api/v3/repos/:owner/:repo/pulls/:pull_number/merge-async" do
525
    test "accepts a submission, polls it, replays retries, and reports the active conflict",
526
         %{conn: conn} do
527
      repository = repository_fixture()
528
      oids = seed_chain(repository, ["layer-1", "layer-2"])
529
      [pr_1, pr_2] = pull_request_chain(repository, oids, ["layer-1", "layer-2"])
530
      conn = put_forge_api_token(conn, "merge-async", repository)
531
532
      assert %{"number" => 1} =
533
               conn
534
               |> put_req_header("idempotency-key", "merge-async-create")
535
               |> post(path(repository), %{trunk_ref: "main", pull_requests: [pr_1, pr_2]})
536
               |> json_response(201)
537
538
      submit_conn =
539
        conn
540
        |> put_req_header("idempotency-key", "merge-async-1")
541
        |> put("#{pulls_path(repository)}/#{pr_1}/merge-async", %{})
542
543
      assert %{
544
               "operation_id" => operation_id,
545
               "merge_status" => "pending",
546
               "state" => "pending",
547
               "merge_method" => "merge",
548
               "pull_request" => ^pr_1,
549
               "replayed" => false,
550
               "url" => poll_url
551
             } = json_response(submit_conn, 202)
552
553
      assert String.ends_with?(
554
               poll_url,
555
               "#{pulls_path(repository)}/#{pr_1}/merge-async/#{operation_id}"
556
             )
557
558
      poll_conn = get(conn, "#{pulls_path(repository)}/#{pr_1}/merge-async/#{operation_id}")
559
560
      assert %{"operation_id" => ^operation_id, "merge_status" => "pending"} =
561
               json_response(poll_conn, 200)
562
563
      replay_conn =
564
        conn
565
        |> put_req_header("idempotency-key", "merge-async-1")
566
        |> put("#{pulls_path(repository)}/#{pr_1}/merge-async", %{})
567
568
      assert %{"operation_id" => ^operation_id, "replayed" => true} =
569
               json_response(replay_conn, 202)
570
571
      second_conn =
572
        conn
573
        |> put_req_header("idempotency-key", "merge-async-2")
574
        |> put("#{pulls_path(repository)}/#{pr_2}/merge-async", %{})
575
576
      assert %{"code" => "operation_in_progress", "operation_id" => ^operation_id} =
577
               json_response(second_conn, 409)
578
    end
579
580
    test "scopes the poll to the submitting pull request", %{conn: conn} do
581
      repository = repository_fixture()
582
      oids = seed_chain(repository, ["layer-1", "layer-2"])
583
      [pr_1, pr_2] = pull_request_chain(repository, oids, ["layer-1", "layer-2"])
584
      conn = put_forge_api_token(conn, "merge-async-scope", repository)
585
586
      assert %{"number" => 1} =
587
               conn
588
               |> put_req_header("idempotency-key", "merge-async-scope-create")
589
               |> post(path(repository), %{trunk_ref: "main", pull_requests: [pr_1, pr_2]})
590
               |> json_response(201)
591
592
      assert %{"operation_id" => operation_id} =
593
               conn
594
               |> put_req_header("idempotency-key", "merge-async-scope-1")
595
               |> put("#{pulls_path(repository)}/#{pr_1}/merge-async", %{})
596
               |> json_response(202)
597
598
      other_conn = get(conn, "#{pulls_path(repository)}/#{pr_2}/merge-async/#{operation_id}")
599
      assert json_response(other_conn, 404)
600
601
      bogus_conn = get(conn, "#{pulls_path(repository)}/#{pr_1}/merge-async/not-a-uuid")
602
      assert json_response(bogus_conn, 404)
603
    end
604
605
    test "rejects an unstacked pull request and an unknown pull request", %{conn: conn} do
606
      repository = repository_fixture()
607
      oids = seed_chain(repository, ["layer-1"])
608
      solo = pull_request(repository, "layer-1", "main", oids["main"], oids["layer-1"])
609
      conn = put_forge_api_token(conn, "merge-async-unstacked", repository)
610
611
      solo_conn =
612
        conn
613
        |> put_req_header("idempotency-key", "merge-async-solo-1")
614
        |> put("#{pulls_path(repository)}/#{solo}/merge-async", %{})
615
616
      assert %{"code" => "not_stacked"} = json_response(solo_conn, 422)
617
618
      missing_conn =
619
        conn
620
        |> put_req_header("idempotency-key", "merge-async-missing-1")
621
        |> put("#{pulls_path(repository)}/999999/merge-async", %{})
622
623
      assert json_response(missing_conn, 404)
624
    end
625
626
    test "refuses a caller without write access", %{conn: conn} do
627
      repository = repository_fixture()
628
      oids = seed_chain(repository, ["layer-1"])
629
      [pr_1] = pull_request_chain(repository, oids, ["layer-1"])
630
      conn = put_forge_api_token(conn, "merge-async-outsider")
631
632
      writer_conn =
633
        Phoenix.ConnTest.build_conn()
634
        |> put_forge_api_token("merge-async-owner", repository)
635
636
      assert %{"number" => 1} =
637
               writer_conn
638
               |> put_req_header("idempotency-key", "merge-async-outsider-create")
639
               |> post(path(repository), %{trunk_ref: "main", pull_requests: [pr_1]})
640
               |> json_response(201)
641
642
      conn =
643
        conn
644
        |> put_req_header("idempotency-key", "merge-async-outsider-1")
645
        |> put("#{pulls_path(repository)}/#{pr_1}/merge-async", %{})
646
647
      assert json_response(conn, 403)
648
    end
649
  end
650
524 651
  describe "GET /api/v3/repos/:owner/:repo/stacks" do
525 652
    test "reads are public for a public repository", %{conn: conn} do
526 653
      repository = repository_fixture()

@@ -572,6 +699,9 @@ defmodule OpenAgentsWeb.StackControllerTest do

572 699
573 700
  defp path(repository), do: "/api/v3/repos/#{repository.owner}/#{repository.name}/stacks"
574 701
702
  defp pulls_path(repository),
703
    do: "/api/v3/repos/#{repository.owner}/#{repository.name}/pulls"
704
575 705
  defp seed_chain(repository, branches) do
576 706
    path = Repos.ensure_repo!(repository.storage_key, repository.default_branch)
577 707

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