Retain an explicit reviewer for self-assigned delivery claims

src/agent_comms/order_tools.py

@@ -12,6 +12,8 @@
 from pydantic import JsonValue
 
 from agent_comms.todos import (
+    AssignedClaimReviewer,
+    ClaimReviewer,
     ClaimOrder,
     CorrectCheckTodo,
     CorrectRunTodo,
@@ -396,7 +398,8 @@ class CommsOrderDeliverTool(PublishingToolRequest[ThreadCaller], ThreadToolReque
     label = "Claim order delivery"
     description = (
         "Record the holder's delivery claim and answer its teammate. "
-        "Only the holder may claim a pending order; this does not accept its delivery."
+        "Only the holder may claim a pending order; this does not accept its delivery. "
+        "Self-assigned work selects an independent reviewer explicitly."
     )
     problem: Annotated[str, *tool_field("Held problem ID")]
     pull_request: Annotated[PullRequestLink, *tool_field(
@@ -407,6 +410,10 @@ class CommsOrderDeliverTool(PublishingToolRequest[ThreadCaller], ThreadToolReque
         "Delivery report: the slop you killed or the broken thing you fixed, with file:line, "
         "in the owner's voice"
     )]
+    reviewer: Annotated[ClaimReviewer, *tool_field(
+        "Assignment parent by default; self-assigned work selects an independent "
+        "reviewer's exact name and birth. Saved claims retain this choice."
+    )] = AssignedClaimReviewer()
 
     def publish(self, comms: Comms, caller: ThreadCaller) -> TodoChanged:
         todo = comms.goals.todos.get(self.problem)
@@ -415,14 +422,14 @@ def publish(self, comms: Comms, caller: ThreadCaller) -> TodoChanged:
         todo.require_assignee(actor, generation)
         # The identical call resumes a retained claim's publication; nothing is claimed again.
         for claim in todo.order.team_claims():
-            if claim.resumed_by(self.pull_request, self.head, self.report.text):
+            if claim.resumed_by(self.pull_request, self.head, self.report.text, self.reviewer):
                 resumed = ResumeClaimOrder(todo.revision, actor, generation, claim).record(
                     comms.goals, todo.id)
                 return TodoChanged(resumed, resumed.strike_count)
         original = comms.bus.bus.lookup_message(todo.latest_message.message_id)
         claim = OrderClaim.for_order(
             todo, actor, self.pull_request, self.head, self.report.text,
-            original.reference,
+            original.reference, reviewer=self.reviewer,
         )
         changed = ClaimOrder(
             todo.revision, actor, generation, claim

src/agent_comms/todos.py

@@ -1,3 +1,3 @@
 """A durable todo owns its one assignment; it does not start an agent.
 
 This is an opt-in post-PR1 primitive. An ACP/fork adapter must verify current
@@ -3175,34 +3175,65 @@
     line: ClassVar[str] = "Dropped for a newer order"
 
 
+class ClaimReviewer(DeclaredFamily, affix="ClaimReviewer"):
+    """Who receives a claim, separately from who consented to its assignment."""
+
+    @abstractmethod
+    def identity(self, todo: Todo) -> ThreadIncarnation: ...
+
+    def target(self, todo: Todo) -> IncarnationThreadTarget:
+        return IncarnationThreadTarget(self.identity(todo))
+
+
+@dataclass(frozen=True)
+class AssignedClaimReviewer(ClaimReviewer):
+    def identity(self, todo: Todo) -> ThreadIncarnation:
+        return todo.held().parent_identity
+
+
+@dataclass(frozen=True)
+class SelectedClaimReviewer(ClaimReviewer):
+    incarnation: ThreadIncarnation
+
+    def identity(self, todo: Todo) -> ThreadIncarnation:
+        return self.incarnation
+
+
 @dataclass(frozen=True)
 class OrderClaim:
     """The holder's assertion, not the teammate's verified delivery."""
 
     pull_request: PullRequestLink
     head: Annotated[str, GitCommitText]
     message: Message
+    reviewer: Annotated[ClaimReviewer, WireOmitDefault()] = AssignedClaimReviewer()
 
     def __post_init__(self) -> None:
         GitCommitText.decode(self.head)
         if not self.message.body.strip():
             raise TodoError("A delivery claim needs its report text.")
 
-    def resumed_by(self, pull_request: PullRequestLink, head: str, report: str) -> bool:
+    def resumed_by(
+        self, pull_request: PullRequestLink, head: str, report: str,
+        reviewer: ClaimReviewer = AssignedClaimReviewer(),
+    ) -> bool:
         """Only the identical deliver call resumes this claim's publication."""
-        return (self.pull_request, self.head, self.message.body) == (pull_request, head, report)
+        return (self.pull_request, self.head, self.message.body, self.reviewer) == (
+            pull_request, head, report, reviewer)
 
     def request(self) -> PullRequestHead:
         return PullRequestHead(self.pull_request.repo, self.pull_request.number, self.head)
 
     @classmethod
     def for_order(
         cls, todo: Todo, actor: ThreadIncarnation, pull_request: PullRequestLink, head: str,
-        report: str, answer: MessageReference,
+        report: str, answer: MessageReference, *,
+        reviewer: ClaimReviewer = AssignedClaimReviewer(),
     ) -> OrderClaim:
         return cls(
             pull_request, head,
-            Message(actor.name, todo.held().parent_identity.name, report, InfoKind, answers=(answer,)),
+            Message(actor.name, reviewer.identity(todo).name, report, InfoKind, answers=(answer,)),
+            reviewer=reviewer,
         )
 
 
@@ -5400,7 +5431,7 @@
         """Retain the publication's actual address before saving its content identity."""
         with NativeCapture(goals.bus) as boundary:
             current = goals.todos.get(problem)
-            target = IncarnationThreadTarget(current.held().parent_identity)
+            target = self.claim.reviewer.target(current)
             target.require_publication(
                 self.claim.message, boundary.registry, goals.bus.require_channel)
             message = replace(
@@ -5424,8 +5455,7 @@
         stays CommittedMessageError, which carries that original."""
         try:
             goals.bus.publisher.publish_ordinary(
-                self.claim.message, destination=IncarnationThreadTarget(
-                    changed.held().parent_identity),
+                self.claim.message, destination=self.claim.reviewer.target(changed),
                 origin=AgentOrigin())
         except (OSError, sqlite3.Error) as error:
             raise ClaimPublicationUnavailable(changed.id, self.claim) from error
@@ -5444,7 +5474,7 @@
     def record(self, goals: Goals, problem: str) -> Todo:
         with NativeCapture(goals.bus) as boundary:
             current = goals.todos.get(problem)
-            target = IncarnationThreadTarget(current.held().parent_identity)
+            target = self.claim.reviewer.target(current)
             target.require_publication(
                 self.claim.message, boundary.registry, goals.bus.require_channel)
             retained = (self.claim.message.sender, self.claim.message.target)

tests/test_order_claims.py

@@ -23,6 +23,7 @@
     TodoConflict,
     TodoError,
     HumanSendRefusedError,
+    RelationViolationError,
 )
 from agent_comms.nominal.field_codec import (
     FieldCodec,
@@ -51,6 +52,8 @@
 )
 from agent_comms.todos import (
     TriageScope,
+    SelectedClaimReviewer,
+    ExpectedProblem,
     CheckedProblemOrder,
     ClaimedProblemOrder,
     ClaimOrder,
@@ -218,6 +221,85 @@ def test_claim_refuses_retired_or_reborn_current_parent(held_team, parent_state)
     assert comms.bus.bus.revision() == before
 
 
+def self_assigned_order(ctx: TeamContext) -> Todo:
+    """The holder accepts its existing work as itself, not as an invented peer."""
+    todo = current_parent_order(ctx)
+    holder = ctx.comms.registry.require(todo.holder)
+    request = ctx.comms.goals.todos.add_request(
+        holder.incarnation, "Deliver the self-assigned repair to an independent reviewer",
+    ).id
+    assigned, = ctx.comms.goals.todos.assign(
+        (ExpectedProblem(todo.id, todo.revision),), owner=holder, assigner=holder.incarnation,
+        generation=todo.held().generation, request=request,
+        review_at=todo.review_at, order=todo.order.checked(),
+    )
+    assert assigned.held().parent_identity == holder.incarnation
+    return assigned
+
+
+def test_self_assigned_claim_selects_a_reviewer_and_retains_its_choice(held_team):
+    ctx = held_team
+    comms = ctx.comms
+    todo = self_assigned_order(ctx)
+    request = replace(delivery(
+        ctx, "Killed the self-send slop at todos.py; the selected reviewer owns the notification."),
+        problem=todo.id,
+    )
+    before = comms.bus.bus.revision()
+    with pytest.raises(RelationViolationError, match="cannot message itself"):
+        ThreadCaller(todo.holder).invoke(request, comms)
+    assert comms.goals.todos.get(todo.id) == todo
+    assert comms.bus.bus.revision() == before
+    reviewer = SelectedClaimReviewer(comms.registry.require(ctx.commander).incarnation)
+    request = replace(request, reviewer=reviewer)
+    # The same declared request shape reaches the native boundary and the retained record.
+    request = FieldCodec.decode(CommsOrderDeliverTool, FieldCodec.encode(request))
+    ThreadCaller(todo.holder).invoke(request, comms)
+    claimed = comms.goals.todos.get(todo.id)
+    claim = claimed.order.claim
+    assert claim.reviewer == reviewer
+    assert FieldCodec.decode(Todo, FieldCodec.encode(claimed)) == claimed
+    published = comms.bus.bus.lookup_message(claim.message.message_id).message
+    assert published.target == ctx.commander
+    after = comms.bus.bus.revision()
+    ThreadCaller(todo.holder).invoke(request, comms)
+    assert comms.bus.bus.revision() == after
+    assert comms.goals.todos.get(todo.id) == claimed
+    different = replace(request, reviewer=SelectedClaimReviewer(
+        comms.registry.require("current-parent").incarnation))
+    with pytest.raises(TodoError, match="Only a pending order"):
+        ThreadCaller(todo.holder).invoke(different, comms)
+    assert comms.goals.todos.get(todo.id) == claimed
+    assert comms.bus.bus.revision() == after
+
+
+@pytest.mark.parametrize("change", ("retired", "reborn", "renamed"))
+def test_selected_reviewer_resume_preserves_original_identity_and_address(held_team, change):
+    ctx = held_team
+    comms = ctx.comms
+    todo = self_assigned_order(ctx)
+    name = "current-parent"
+    reviewer = SelectedClaimReviewer(comms.registry.require(name).incarnation)
+    request = replace(delivery(
+        ctx, "Killed the self-send slop at todos.py; the selected reviewer owns the notification."),
+        problem=todo.id, reviewer=reviewer,
+    )
+    ThreadCaller(todo.holder).invoke(request, comms)
+    claimed = comms.goals.todos.get(todo.id)
+    if change == "renamed":
+        comms.registry.rename(name, "renamed-reviewer")
+    else:
+        comms.registry.unregister(name)
+        comms.registry.remove(name)
+        if change == "reborn":
+            comms.registry.register(thread(name, ctx.root.parent / "reborn-reviewer", 15.0))
+    before = comms.bus.bus.revision()
+    with pytest.raises((TodoError, HumanSendRefusedError)):
+        ThreadCaller(todo.holder).invoke(request, comms)
+    assert comms.goals.todos.get(todo.id) == claimed
+    assert comms.bus.bus.revision() == before
+
+
 def test_claim_verdict_family_on_retained_copy(held_team):
     ctx = held_team
     comms = ctx.comms