diff --git a/src/agent_comms/order_tools.py b/src/agent_comms/order_tools.py index 5ee688c484..3a87b6a388 100644 --- a/src/agent_comms/order_tools.py +++ b/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 diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 92b019d795..be04aab86d 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -3175,6 +3175,30 @@ class DroppedProblemOrder(UnorderedProblemOrder, CheckedProblemOrder): 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.""" @@ -3182,15 +3206,20 @@ class OrderClaim: 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) @@ -3198,11 +3227,13 @@ def request(self) -> PullRequestHead: @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 @@ def record(self, goals: Goals, problem: str) -> Todo: """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 @@ def after_record(self, goals: Goals, changed: Todo) -> None: 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 @@ class ResumeClaimOrder(ClaimOrder): 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) diff --git a/tests/test_order_claims.py b/tests/test_order_claims.py index ef2504cd7b..81db834ce1 100644 --- a/tests/test_order_claims.py +++ b/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