Take back a merge recorded through a prerequisite pull request

src/agent_comms/todos.py

@@ -663,6 +663,13 @@ def existing(cls, repo: RepositoryName, number: int) -> Self:
         return link
 
 
+@dataclass(frozen=True)
+class MergedLink:
+    """A pull request an order links and the merge commit the order recorded for it."""
+    link: PullRequestLink
+    sha: str
+
+
 @dataclass(frozen=True)
 class PullRequest(HeadReviewStatus, DeclaredFamily, affix="PullRequest"):
     number: int
@@ -1983,7 +1990,7 @@ def names(self, path: str) -> bool:
 
 
 class OrderCorrectionReason(PlainText):
-    """Why the recorded check or run was wrong, in enough words to stand on the record."""
+    """Why the recorded part of an order was wrong, in enough words to stand on the record."""
 
     refusal = (
         "Say what was wrong with the recorded order in {}+ words; jargon {} hides what happened."
@@ -2800,9 +2807,16 @@ def withdrawn(self, previous: Assignment, why: str, superseded_by: Successor) ->
 class OpenProblemOrder(CheckedProblemOrder, declared_name=None):
     """An active order keeps corrections and accepts its checked delivery by identity."""
 
-    def corrected(self, correction: CorrectOrderTodo) -> Self:
+    def corrected(self, correction: CorrectOrderTodo) -> OpenProblemOrder:
         return correction.corrected(self)
 
+    def merged_link(self, link: PullRequestLink) -> MergedLink:
+        """The merge this order recorded through ``link``; an unmerged order recorded none."""
+        raise TodoError(f"Order {self.id} records no merge to take back.")
+
+    def unmerged(self, link: PullRequestLink) -> OpenProblemOrder:
+        raise TodoError(f"Order {self.id} records no merge to take back.")
+
     def require_delivery(self, order_id: OrderId) -> None:
         if self.id != order_id:
             raise TodoError("The order changed; refresh its state before delivering it.")
@@ -2845,6 +2859,16 @@ def require_linkable(self) -> None:
         raise TodoError(f"Order {self.id} merged as {self.sha[:9]}; its pull requests are closed. "
                          "Ask the commander for a send-back before opening or linking another PR.")
 
+    def merged_link(self, link: PullRequestLink) -> MergedLink:
+        if link not in self.pull_requests:
+            raise TodoError(f"Order {self.id} does not link {link.url}; read it again.")
+        return MergedLink(link, self.sha)
+
+    def unmerged(self, link: PullRequestLink) -> PendingProblemOrder:
+        """Pending again under its own id, check, run, edits and times, without ``link``."""
+        return self.copy_as(PendingProblemOrder, pull_requests=tuple(
+            kept for kept in self.pull_requests if kept != link))
+
     def waiting_in(self, todo: Todo) -> Todo:
         return (todo if todo.order == self
                 else replace(todo, order=self, revision=todo.revision + 1))
@@ -4974,7 +4998,7 @@ def changed(self, current: Todo) -> Todo:
         _text(self.reason, "Correction reason", 2000)
         return replace(current, order=current.order.corrected(self), revision=current.revision + 1)
 
-    def corrected[Order: OpenProblemOrder](self, order: Order) -> Order:
+    def corrected(self, order: OpenProblemOrder) -> OpenProblemOrder:
         if self.previous != self.part(order):
             raise TodoError("The corrected part changed since it was read; read it again.")
         return replace(self.replaced(order), corrections=(*order.corrections, self))
@@ -4984,8 +5008,8 @@ def part(self, order: OpenProblemOrder) -> object:
         """The order's current value of the part this correction replaces."""
 
     @abstractmethod
-    def replaced[Order: OpenProblemOrder](self, order: Order) -> Order:
-        """The order with this correction's part replaced."""
+    def replaced(self, order: OpenProblemOrder) -> OpenProblemOrder:
+        """The order with this correction's part replaced; its state may change with it."""
 
 
 @dataclass(frozen=True)
@@ -5004,7 +5028,7 @@ def prepare(self, goals: Goals, current: Todo) -> Self:
     def part(self, order: OpenProblemOrder) -> OrderCheck:
         return order.check
 
-    def replaced[Order: OpenProblemOrder](self, order: Order) -> Order:
+    def replaced(self, order: OpenProblemOrder) -> OpenProblemOrder:
         self.check.require_writable()
         return replace(order, check=self.check)
 
@@ -5018,10 +5042,25 @@ class CorrectRunTodo(CorrectOrderTodo):
     def part(self, order: OpenProblemOrder) -> OrderRun:
         return order.run
 
-    def replaced[Order: OpenProblemOrder](self, order: Order) -> Order:
+    def replaced(self, order: OpenProblemOrder) -> OpenProblemOrder:
         return replace(order, run=self.run)
 
 
+@dataclass(frozen=True)
+class CorrectMergedLinkTodo(CorrectOrderTodo):
+    """The commander takes back a merge recorded through a pull request that never held this
+    order's work, such as a merged prerequisite. The order is pending again under its own id,
+    check, run, edits and times; only that link leaves it. Any other linked pull request stays,
+    so a merge of real work on it is read again from the forge, never dropped."""
+    previous: MergedLink
+
+    def part(self, order: OpenProblemOrder) -> MergedLink:
+        return order.merged_link(self.previous.link)
+
+    def replaced(self, order: OpenProblemOrder) -> OpenProblemOrder:
+        return order.unmerged(self.previous.link)
+
+
 @dataclass(frozen=True)
 class WordingCorrection(TodoOperation, StoryEvent):
     """Core's answer to a published unit message whose words hide what happened; no strike."""

tests/test_holder_standing_rules.py

@@ -34,11 +34,14 @@
 from agent_comms.task_attachments.commander_pings import CommanderUpdate
 from agent_comms.comms import wire
 from agent_comms.coordinator import Coordination
-from agent_comms.errors import CommittedMessageError, HumanSendRefusedError, RelationViolationError, TodoConflict
+from agent_comms.errors import (
+    CommittedMessageError, HumanSendRefusedError, RelationViolationError, TodoConflict, TodoError,
+)
 from agent_comms.field_codec import FieldCodec, InputId, MessageId, OrderId, RecordProblems, TurnId
 from agent_comms.goal_actions import (
     CreateProblemGoalAction,
     ReissuedOrderGoalAction,
+    SetGoalAction,
 )
 from agent_comms.goal_presentation import RunnableExecution
 from agent_comms.goal_states import ActiveGoal
@@ -69,6 +72,9 @@
 from agent_comms.threads import Thread
 from agent_comms.todos import (
     ClaimOrder,
+    CorrectMergedLinkTodo,
+    MergedLink,
+    PullRequestLink,
     ResumeClaimOrder,
     ExpectedProblem,
     OrderClaim,
@@ -440,6 +446,55 @@ def test_goal_generation_cannot_transfer_to_a_different_owner(held_bus):
     assert held_bus.goals.todos.get(todo.id) == todo
 
 
+def test_merge_through_a_prerequisite_link_is_taken_back_and_the_order_changes_owner(held_bus):
+    """P935: a prerequisite PR linked to the order merged it; only that link leaves, the
+    order is pending again under its own id, check, run and times, and then changes owner."""
+    store = held_bus.goals.todos
+    lead = held_bus.registry.require("lead").incarnation
+    held = store.get("P1")
+    prerequisite = PullRequestLink(held.repo, 3596)
+    implementation = PullRequestLink(held.repo, 3700)
+    with store.writing() as connection:
+        replace(held, revision=held.revision + 1, order=replace(
+            held.order.checked(), pull_requests=(prerequisite, implementation),
+        ).merged("33bd05c64abc")).upsert(connection)
+    merged = store.get("P1")
+    reason = "the merged prerequisite pull request held none of this order's checker work"
+
+    def correction(revision: int, previous: MergedLink) -> CorrectMergedLinkTodo:
+        return CorrectMergedLinkTodo(revision, lead, reason, previous=previous)
+
+    for refused in (correction(held.revision, MergedLink(prerequisite, "33bd05c64abc")),
+                    correction(merged.revision, MergedLink(PullRequestLink(held.repo, 1),
+                                                           "33bd05c64abc")),
+                    correction(merged.revision, MergedLink(prerequisite, "0123456789ab"))):
+        with pytest.raises(TodoError):
+            store.apply("P1", refused)
+        assert store.get("P1") == merged
+    with pytest.raises(TodoError, match="records no merge"):
+        correction(held.revision, MergedLink(prerequisite, "33bd05c64abc")).apply(held)
+
+    store.apply("P1", correction(merged.revision, MergedLink(prerequisite, "33bd05c64abc")))
+    corrected = store.get("P1")
+    assert isinstance(corrected.order, PendingProblemOrder)
+    assert corrected.order.pull_requests == (implementation,)
+    assert corrected.order.corrections[-1].previous == MergedLink(prerequisite, "33bd05c64abc")
+    assert (replace(corrected.order, pull_requests=(), corrections=())
+            == replace(held.order, pull_requests=(), corrections=()))
+    assert (corrected.holder, corrected.review_at) == (merged.holder, merged.review_at)
+
+    held_bus.goals.update_goal("outsider", SetGoalAction(text="Absorb the checker audit work."))
+    goals = {name: held_bus.registry.require(name).goal for name in ("worker", "outsider")}
+    outsider = held_bus.registry.require("outsider")
+    store.assign((ExpectedProblem("P1", corrected.revision),), owner=outsider, assigner=lead,
+                 generation=goals["outsider"].require().id, request=corrected.request.require(),
+                 review_at=corrected.review_at, order=corrected.order)
+    assigned = store.get("P1")
+    assert assigned.held().owner_identity == outsider.incarnation
+    assert assigned.order == corrected.order
+    assert {name: held_bus.registry.require(name).goal for name in goals} == goals
+
+
 def test_unanswered_uses_age_direct_target_and_later_answer_references(held_bus):
     now = time.time()
     publish(held_bus, "worker", "lead", now - 7200)  # before every held order