[SYMPP-23B6] Reactivate packages after final blocker resolution

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

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),

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()}

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"
 

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", %{

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