Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.GitHub.PullRequestProgress do
@spec chronological_events([ProgressEvent.t()]) :: [ProgressEvent.t()]
def chronological_events(events) when is_list(events) do
Enum.sort_by(events, fn %ProgressEvent{sequence: sequence, created_at: created_at, id: id} ->
{created_at || DateTime.from_unix!(0), sequence || 0, id || ""}
{timestamp_sort_value(created_at), sequence || 0, id || ""}
end)
end

Expand Down Expand Up @@ -122,4 +122,7 @@ defmodule SymphonyElixir.SymphonyPlusPlus.GitHub.PullRequestProgress do
end

defp clean_string(_value), do: nil

defp timestamp_sort_value(%DateTime{} = value), do: DateTime.to_unix(value, :microsecond)
defp timestamp_sort_value(_value), do: 0
end
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ArchitectDeliveryTools do
alias SymphonyElixir.SymphonyPlusPlus.Authorization.MCPError
alias SymphonyElixir.SymphonyPlusPlus.Dashboard
alias SymphonyElixir.SymphonyPlusPlus.Dashboard.BlockerProjection
alias SymphonyElixir.SymphonyPlusPlus.Dashboard.MetadataProjection
alias SymphonyElixir.SymphonyPlusPlus.DashboardPubSub
alias SymphonyElixir.SymphonyPlusPlus.GitHub.PullRequestProgress

alias SymphonyElixir.SymphonyPlusPlus.MCP.{
Auth,
Expand Down Expand Up @@ -181,6 +183,47 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ArchitectDeliveryTools do
end
end

def call("accept_review_rework", %Config{} = config, session, arguments) do
with {:ok, live_session} <- Auth.require_session(session, config.repo),
:ok <- require_delivery_write_capability(live_session),
{:ok, work_request_id} <- CurrentWorkRequest.id_argument(arguments, live_session),
{:ok, work_package_id} <- required_argument(arguments, "work_package_id"),
{:ok, idempotency_key} <- required_argument(arguments, "idempotency_key"),
{:ok, evidence} <- accepted_review_rework_evidence(arguments),
{:ok, work_request, work_package, filters, scope} <-
WorkRequestScope.authorized_work_package_scope(
config.repo,
live_session,
work_request_id,
work_package_id,
:work_package_repair_state,
"accept_review_rework"
),
{:ok, result} <-
run_architect_transaction(config.repo, fn ->
accept_review_rework_in_transaction(
config.repo,
live_session,
work_request,
work_package,
filters,
evidence,
"accept_review_rework:#{work_package_id}:#{String.trim(idempotency_key)}"
)
end) do
{:ok,
ToolResult.tool_result(%{
"work_package" => work_package_payload(result.work_package),
"accepted_review_rework" => ProgressEvents.payload(result.event),
"scope" => scope
})}
else
{:tool_error, reason} -> invalid_params_error("accept_review_rework", reason)
{:error, :not_found} -> not_found_error("accept_review_rework")
{:error, reason} -> architect_error(reason, "accept_review_rework")
end
end

def call("cleanup_work_request_work_package_runtime", %Config{} = config, session, arguments) do
with {:ok, live_session} <- Auth.require_session(session, config.repo),
:ok <- require_delivery_write_capability(live_session),
Expand Down Expand Up @@ -328,6 +371,25 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ArchitectDeliveryTools do
end
end

defp accepted_review_rework_evidence(arguments) do
with {:ok, evidence} <- required_object(arguments, "evidence"),
{:ok, provider} <- required_argument(evidence, "provider"),
{:ok, reference} <- required_argument(evidence, "reference"),
{:ok, head_sha} <- required_argument(evidence, "head_sha"),
{:ok, finding} <- required_argument(evidence, "finding") do
normalized = %{
"provider" => provider |> String.trim() |> Redactor.redact_text(),
"reference" => reference |> String.trim() |> Redactor.redact_text(),
"head_sha" => head_sha |> String.trim() |> String.downcase(),
"finding" => finding |> String.trim() |> Redactor.redact_text()
}

if Enum.all?(Map.values(normalized), &filled_string?/1),
do: {:ok, normalized},
else: {:tool_error, "empty_accepted_review_rework_evidence"}
end
end

defp required_runtime_cleanup_delivery_outcome(arguments) do
allowed_outcomes = ["superseded", "abandoned"]

Expand Down Expand Up @@ -795,6 +857,129 @@ defmodule SymphonyElixir.SymphonyPlusPlus.MCP.ArchitectDeliveryTools do
WorkRequestService.get_work_package(repo, work_request_id, work_package_id)
end

defp accept_review_rework_in_transaction(
repo,
%Session{} = session,
%WorkRequest{} = work_request,
%WorkPackage{} = scoped_work_package,
filters,
evidence,
idempotency_key
) do
primary_scope? = WorkRequestScope.primary_work_request_scope?(repo, work_request, filters)

with :ok <- lock_access_grant(repo, session.assignment.grant_id),
{:ok, _architect_grant} <- WorkRequestScope.require_live_architect_grant(repo, session),
:ok <- lock_work_package(repo, Session.work_package_id(session)),
:ok <- lock_work_package(repo, scoped_work_package.id),
{:ok, work_package} <- WorkPackageRepository.get(repo, scoped_work_package.id),
:ok <-
WorkRequestScope.require_scoped_delivery_work_package_visibility(
work_package,
work_request,
work_package,
primary_scope?,
filters
),
{:ok, progress_events} <- PlanningRepository.list_progress_events(repo, work_package.id) do
case Enum.find(progress_events, &(&1.idempotency_key == idempotency_key)) do
%ProgressEvent{} = event ->
replay_accepted_review_rework(work_package, event, work_request.id, evidence)

nil ->
append_accepted_review_rework(
repo,
session,
work_request,
work_package,
progress_events,
evidence,
idempotency_key
)
end
end
end

defp append_accepted_review_rework(
repo,
session,
work_request,
work_package,
progress_events,
evidence,
idempotency_key
) do
with :ok <- require_accepted_review_rework_status(work_package),
{:ok, head_sha, pr} <- current_accepted_review_rework_pr(work_package, progress_events, evidence),
payload =
evidence
|> Map.put("type", "accepted_review_rework")
|> Map.put("source_tool", "accept_review_rework")
|> Map.put("work_request_id", work_request.id)
|> Map.put("work_package_id", work_package.id)
|> Map.put("head_sha", head_sha)
|> Map.put("pr", pr),
{:ok, reopened} <- WorkPackageRepository.update_status(repo, work_package.id, "ready_for_merge", "active"),
{:ok, event} <-
PlanningRepository.append_audit_progress_event_for_work_package(repo, session.assignment, work_package.id, %{
"summary" => "Accepted verified review finding for rework",
"status" => "accepted_review_rework",
"idempotency_key" => idempotency_key,
"payload" => payload
}) do
{:ok, %{work_package: reopened, event: event}}
end
end

defp replay_accepted_review_rework(work_package, %ProgressEvent{payload: payload} = event, work_request_id, evidence) do
expected =
evidence
|> Map.take(["provider", "reference", "head_sha", "finding"])
|> Map.put("type", "accepted_review_rework")
|> Map.put("source_tool", "accept_review_rework")
|> Map.put("work_request_id", work_request_id)
|> Map.put("work_package_id", work_package.id)

if is_map(payload) and Map.take(payload, Map.keys(expected)) == expected,
do: {:ok, %{work_package: work_package, event: event}},
else: {:tool_error, "idempotency_conflict"}
end

defp require_accepted_review_rework_status(%WorkPackage{kind: "phase_child"}),
do: {:tool_error, "phase_child_rework_not_allowed"}

defp require_accepted_review_rework_status(%WorkPackage{status: "ready_for_merge"}), do: :ok
defp require_accepted_review_rework_status(%WorkPackage{}), do: {:tool_error, "work_package_not_ready_for_rework"}

defp current_accepted_review_rework_pr(work_package, progress_events, evidence) do
current_head_sha = MetadataProjection.latest_current_head_sha(progress_events)
pr = MetadataProjection.metadata(progress_events, [], work_package.id, work_package.review_requirement).pr

cond do
not filled_string?(current_head_sha) ->
{:tool_error, "missing_current_head_sha"}

not exact_head_sha?(evidence["head_sha"], current_head_sha) ->
{:tool_error, "stale_rework_head"}

not is_map(pr) or not exact_head_sha?(pr["head_sha"], current_head_sha) ->
{:tool_error, "missing_current_attached_pr"}

PullRequestProgress.merged?(%{"merge_state" => pr["merge_state"]}) ->
{:tool_error, "current_attached_pr_already_merged"}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

true ->
identity = Map.take(pr, ["url", "repository", "number"]) |> Map.put("head_sha", current_head_sha)
Comment thread
Pimpmuckl marked this conversation as resolved.
{:ok, String.downcase(current_head_sha), identity}
end
end

defp exact_head_sha?(left, right) when is_binary(left) and is_binary(right) do
String.downcase(String.trim(left)) == String.downcase(String.trim(right))
end

defp exact_head_sha?(_left, _right), do: false

defp cleanup_worktree_runtime_in_transaction(repo, %Session{} = session, work_package_id) do
with :ok <- lock_access_grant(repo, session.assignment.grant_id),
{:ok, _architect_grant} <- WorkRequestScope.require_live_architect_grant(repo, session),
Expand Down
Loading
Loading