diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/guidance_tools.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/guidance_tools.ex index 40042e36df..40c94bb737 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/guidance_tools.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/guidance_tools.ex @@ -41,6 +41,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.GuidanceTools do alias SymphonyElixir.SymphonyPlusPlus.Planning.Repository, as: PlanningRepository alias SymphonyElixir.SymphonyPlusPlus.Planning.Service, as: PlanningService + alias SymphonyElixir.SymphonyPlusPlus.WorkPackages.Repository, as: WorkPackageRepository alias SymphonyElixir.SymphonyPlusPlus.WorkPackages.WorkPackage alias SymphonyElixir.SymphonyPlusPlus.WorkRequests.ArchitectHandoff @@ -445,7 +446,8 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.GuidanceTools do session.assignment, work_package_id, attrs - ) do + ), + {:ok, _work_package} <- WorkPackageRepository.reactivate_if_unblocked(repo, work_package_id) do {:ok, ToolResult.tool_result(%{"progress_event" => ProgressEvents.payload(event)})} end end diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/worker_tools.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/worker_tools.ex index 963d3effbb..2409d3f1e3 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/worker_tools.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/mcp/worker_tools.ex @@ -595,17 +595,10 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkerTools do end end - defp resolve_worker_blocker(:active, repo, session, %WorkPackage{status: "blocked"} = work_package, progress_events, blocker_id, attrs, idempotency_key) do + defp resolve_worker_blocker(:active, repo, session, %WorkPackage{status: "blocked"} = work_package, _progress_events, _blocker_id, attrs, idempotency_key) do with :ok <- ProgressEvents.reject_ready_evidence_mutation(repo, session, "resolve_blocker"), {:ok, result} <- ProgressEvents.append_or_replay(repo, session, attrs, idempotency_key, "resolve_blocker"), - {:ok, _work_package} <- - maybe_unblock_worker_package( - repo, - session, - work_package, - progress_events, - blocker_id - ) do + {:ok, _work_package} <- WorkPackageRepository.reactivate_if_unblocked(repo, work_package.id) do {:ok, result} end end @@ -630,19 +623,6 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.WorkerTools do end end - defp maybe_unblock_worker_package(repo, %Session{} = session, %WorkPackage{} = work_package, progress_events, blocker_id) do - active_blockers = - progress_events - |> BlockerProjection.blockers() - |> Enum.filter(&(&1.active and &1.id != blocker_id)) - - if active_blockers == [] do - LifecycleService.transition(repo, work_package, "active", actor(session)) - else - {:ok, work_package} - end - end - defp abandon_transaction(repo, %Session{} = session, reason) do run_worker_transaction(repo, fn -> with :ok <- PlanningService.require_valid_assignment(repo, session.assignment), diff --git a/elixir/lib/symphony_elixir/symphony_plus_plus/work_packages/repository.ex b/elixir/lib/symphony_elixir/symphony_plus_plus/work_packages/repository.ex index fdf88a5137..ce77e04791 100644 --- a/elixir/lib/symphony_elixir/symphony_plus_plus/work_packages/repository.ex +++ b/elixir/lib/symphony_elixir/symphony_plus_plus/work_packages/repository.ex @@ -144,6 +144,25 @@ defmodule SymphonyElixir.SymphonyPlusPlus.WorkPackages.Repository do end end + @spec reactivate_if_unblocked(repo(), String.t()) :: {:ok, WorkPackage.t()} | {:error, error()} + def reactivate_if_unblocked(repo, id) when is_atom(repo) and is_binary(id) do + with {:ok, work_package} <- get(repo, id) do + maybe_reactivate_if_unblocked(repo, work_package) + end + end + + defp maybe_reactivate_if_unblocked(repo, %WorkPackage{status: "blocked"} = work_package) do + with {:ok, progress_events} <- PlanningRepository.list_progress_events(repo, work_package.id) do + if Enum.any?(BlockerProjection.blockers(progress_events), & &1.active) do + {:ok, work_package} + else + update_status(repo, work_package.id, "blocked", "active") + end + end + end + + defp maybe_reactivate_if_unblocked(_repo, %WorkPackage{} = work_package), do: {:ok, work_package} + @doc false @spec close_delivery_work_package(repo(), WorkRequest.t(), WorkPackage.t(), String.t()) :: {:ok, map() | nil} | {:error, error()} diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs index cb5df44a7e..78efe42b01 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp/comments_guidance_test.exs @@ -139,7 +139,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.CommentsGuidanceTest do work_request_work_package_attrs(id: "WRS-MCP-ARCH-PACKAGE-SURFACES", base_branch: work_request.base_branch) ) - work_package = repo.update!(Ecto.Changeset.change(work_package, status: "implementing")) + work_package = repo.update!(Ecto.Changeset.change(work_package, status: "blocked")) {_phase_anchor, phase_session, _phase_grant} = create_phase_architect_session( @@ -232,6 +232,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.CommentsGuidanceTest do resolve_event_id = get_in(resolve_blocker_response, ["result", "structuredContent", "progress_event", "id"]) refute Map.has_key?(get_in(resolve_blocker_response, ["result", "structuredContent", "progress_event"]), "payload") assert repo.get!(ProgressEvent, resolve_event_id).payload["active"] == false + assert repo.get!(WorkPackage, work_package.id).status == "active" anchor_blocker_id = "arch-policy-anchor-blocker" diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/mcp_delivery_tools_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/mcp_delivery_tools_test.exs index fb0caf445e..a3150daea7 100644 --- a/elixir/test/symphony_elixir/symphony_plus_plus/mcp_delivery_tools_test.exs +++ b/elixir/test/symphony_elixir/symphony_plus_plus/mcp_delivery_tools_test.exs @@ -1339,7 +1339,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCPDeliveryToolsTest do }) assert get_in(second, ["result", "structuredContent", "progress_event", "id"]) - assert repo.get!(WorkPackage, package.id).status == "blocked" + assert repo.get!(WorkPackage, package.id).status == "active" unknown = mcp_tool(repo, session, "resolve_blocker", %{ diff --git a/elixir/test/symphony_elixir/symphony_plus_plus/work_packages/blocker_lifecycle_test.exs b/elixir/test/symphony_elixir/symphony_plus_plus/work_packages/blocker_lifecycle_test.exs new file mode 100644 index 0000000000..2f4c930328 --- /dev/null +++ b/elixir/test/symphony_elixir/symphony_plus_plus/work_packages/blocker_lifecycle_test.exs @@ -0,0 +1,60 @@ +Code.require_file("../work_packages_case.exs", __DIR__) + +defmodule SymphonyElixir.SymphonyPlusPlus.WorkPackages.BlockerLifecycleTest do + use SymphonyElixir.SymphonyPlusPlus.WorkPackagesCase + + alias SymphonyElixir.SymphonyPlusPlus.Planning.Repository, as: PlanningRepository + + test "reactivates a blocked package only after its final blocker is resolved", %{repo: repo} do + assert {:ok, package} = + Repository.create(repo, WorkPackageFactory.attrs(id: "SYMPP-BLOCKER-LIFECYCLE", status: "blocked")) + + append_blocker(repo, package.id, "first", true) + append_blocker(repo, package.id, "second", true) + append_blocker(repo, package.id, "first", false) + + assert {:ok, %{status: "blocked"}} = Repository.reactivate_if_unblocked(repo, package.id) + + append_blocker(repo, package.id, "second", false) + + assert {:ok, active} = Repository.reactivate_if_unblocked(repo, package.id) + assert active.status == "active" + assert {:ok, ^active} = Repository.reactivate_if_unblocked(repo, package.id) + end + + test "never reactivates terminal, abandoned, or deleted packages", %{repo: repo} do + for status <- ["merged", "closed", "abandoned"] do + assert {:ok, package} = + Repository.create( + repo, + WorkPackageFactory.attrs(id: "SYMPP-BLOCKER-#{status}", status: status) + ) + + assert {:ok, unchanged} = Repository.reactivate_if_unblocked(repo, package.id) + assert unchanged.status == status + end + + assert {:ok, deleted} = + Repository.create(repo, WorkPackageFactory.attrs(id: "SYMPP-BLOCKER-DELETED", status: "blocked")) + + repo.delete!(deleted) + assert {:error, :not_found} = Repository.reactivate_if_unblocked(repo, deleted.id) + end + + defp append_blocker(repo, work_package_id, blocker_id, active?) do + source_tool = if active?, do: "report_blocker", else: "resolve_blocker" + + assert {:ok, _event} = + PlanningRepository.append_progress_event(repo, %{ + work_package_id: work_package_id, + summary: "#{source_tool} #{blocker_id}", + status: if(active?, do: "blocked", else: "resolved"), + payload: %{ + "type" => "blocker", + "source_tool" => source_tool, + "blocker_id" => blocker_id, + "active" => active? + } + }) + end +end