diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index 9df4d1feb6..8ac6b6727e 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -80,6 +80,7 @@ from agent_comms.recorded_time import RecordedEventTime from agent_comms.registry_document import RegisteredCommanderProject from agent_comms.repo import Repo +from agent_comms.repository_name import RepositoryName from agent_comms.slop import Slop from agent_comms.standing_rules import ( ClearingVerb, @@ -91,6 +92,7 @@ BluntText, CheckedDelivery, CleanHunt, + CorrectLinkTodo, DeliveryEvidence, DeliverySource, DirectiveText, @@ -1258,6 +1260,23 @@ def run(self, ctx: Commander) -> None: print(f'{self.pid}: order now works when {self.text}') +@dataclass(frozen=True, kw_only=True) +class UnlinkVerb(Reasoned, OrderVerb): + help = ("take a pull request off an open order that never held its work; keeps its holder, " + "review timer and strikes") + pid: Annotated[str, Argument(flags=("pid",))] + repo: Annotated[RepositoryName, Argument(flags=("repo",), options={"metavar": "OWNER/NAME"})] + number: Annotated[int, Argument(flags=("number",), options={"metavar": "PR"})] + + def run(self, ctx: Commander) -> None: + todo = ctx.comms.goals.todos.get(self.pid) + link = PullRequestLink(self.repo, self.number) + ctx.call(CommsChangeProblemTool(problem=self.pid, operation=CorrectLinkTodo( + todo.revision, ctx.incarnation, self.why, + previous=todo.order.pull_requests, link=link))) + print(f'{self.pid}: order no longer links {link.url}') + + @dataclass(frozen=True, kw_only=True) class ServeVerb(OrderVerb): help = "name the plan phase an open order serves; keeps its id, review timer and strikes" diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index ac9706fb76..b68f6dd52b 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -2558,6 +2558,13 @@ def linked_to(self, link: PullRequestLink) -> Self: self.require_linkable() return replace(self, pull_requests=(*self.pull_requests, link)) + def unlinked(self, link: PullRequestLink) -> Self: + """This order without ``link``; its state, id, check, run, edits and times stay.""" + if link not in self.pull_requests: + raise TodoError(f"Order {self.id} does not link {link.url}; read it again.") + return replace(self, pull_requests=tuple( + kept for kept in self.pull_requests if kept != link)) + @property def installation_commits(self) -> tuple[str, ...]: return () @@ -2891,14 +2898,11 @@ def require_linkable(self) -> None: "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) + return MergedLink(link, self.unlinked(link).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)) + return self.unlinked(link).copy_as(PendingProblemOrder) def waiting_in(self, todo: Todo) -> Todo: return (todo if todo.order == self @@ -5113,6 +5117,23 @@ def replaced(self, order: OpenProblemOrder) -> OpenProblemOrder: return order.unmerged(self.previous.link) +@dataclass(frozen=True) +class CorrectLinkTodo(CorrectOrderTodo): + """The commander takes a pull request off an open order that never held its work, such as + another order's pull request recorded by a mistaken claim. The order keeps its state, holder, + review timer, strikes and every other link; a later send carries only the links left.""" + previous: tuple[PullRequestLink, ...] + link: PullRequestLink + + def part(self, order: OpenProblemOrder) -> tuple[PullRequestLink, ...]: + return order.pull_requests + + def replaced(self, order: OpenProblemOrder) -> OpenProblemOrder: + """A merged order's links are closed; CorrectMergedLinkTodo takes its merge back.""" + order.require_linkable() + return order.unlinked(self.link) + + @dataclass(frozen=True) class WordingCorrection(TodoOperation, StoryEvent): """Core's answer to a published unit message whose words hide what happened; no strike.""" diff --git a/tests/test_holder_standing_rules.py b/tests/test_holder_standing_rules.py index a3bea84e8c..10123eb406 100644 --- a/tests/test_holder_standing_rules.py +++ b/tests/test_holder_standing_rules.py @@ -19,7 +19,7 @@ from agent_comms.bus_publication import AgentOrigin, HumanOrigin from agent_comms.channel_targets import CommandTarget, ThreadTarget from agent_comms.child_process import ProcessIdentity -from agent_comms.commander.order import Commander, OrderFile, SayVerb, SendVerb +from agent_comms.commander.order import Commander, ListedProblems, OrderFile, SayVerb, SendVerb from agent_comms.commander.questions import UnansweredUnitAsks from agent_comms.commander.standing import ( HeartbeatNotStarted, @@ -73,6 +73,7 @@ from agent_comms.threads import Thread from agent_comms.todos import ( ClaimOrder, + CorrectLinkTodo, CorrectMergedLinkTodo, MergedLink, PullRequestLink, @@ -508,6 +509,42 @@ def correction(revision: int, previous: MergedLink) -> CorrectMergedLinkTodo: assert {name: held_bus.registry.require(name).goal for name in goals} == goals +def test_unlink_takes_another_orders_pull_request_off_a_held_order(held_bus, durable_home): + """P904: a claim recorded another order's PR on a held order. `order unlink` corrects the + order in place while its holder still holds it; only that link leaves, and a later send + carries only the links the corrected order keeps.""" + store = held_bus.goals.todos + held = store.get("P1") + foreign = PullRequestLink(held.repo, 3566) + own = PullRequestLink(RepositoryName.from_wire("OpenHCSDev/toad"), 1319) + with store.writing() as connection: + replace(held, revision=held.revision + 1, order=replace( + held.order.checked(), pull_requests=(foreign, own))).upsert(connection) + linked = store.get("P1") + + def unlink(*arguments: str): + return commander('--root', str(held_bus.root), 'order', 'unlink', 'P1', *arguments, + '--why', 'a claim recorded P883 work on this order') + + refused = unlink('openhcsdev/agent-comms', '1') + assert refused.returncode != 0 and 'does not link' in refused.stdout + refused.stderr + assert store.get("P1") == linked + run = unlink('openhcsdev/agent-comms', '3566') + assert run.returncode == 0, run.stdout + run.stderr + corrected = store.get("P1") + assert corrected.order.pull_requests == (own,) + assert corrected.order.corrections[-1] == CorrectLinkTodo( + linked.revision, held_bus.registry.require("lead").incarnation, + "a claim recorded P883 work on this order", previous=(foreign, own), link=foreign, + at=corrected.order.corrections[-1].at) + assert (replace(corrected.order, pull_requests=(), corrections=()) + == replace(linked.order, pull_requests=(), corrections=())) + assert ((corrected.holder, corrected.review_at, corrected.strike_count) + == (linked.holder, linked.review_at, linked.strike_count)) + resent = replace(corrected.order, pull_requests=()) + assert ListedProblems().carried(resent, ["P1"], {"P1": corrected}).pull_requests == (own,) + + 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