P291: inherit the plan phase across every problem-order state

src/agent_comms/todos.py

@@ -1352,6 +1352,11 @@ class ProblemOrder(DeclaredFamily, affix="ProblemOrder"):
     pull_request_label: ClassVar[str] = "Agent-Comms-Order:"
     finding: ProblemFinding = field(default_factory=UnlocatedProblemFinding,
                                    metadata={"wire_omit_default": True})
+    serves: type[OrderPhase] = field(default=UnplannedPhase, metadata={"wire_omit_default": True})
+
+    def copy_as[Kind: ProblemOrder](self, kind: type[Kind], **evidence) -> Kind:
+        return kind(**{item.name: getattr(self, item.name) for item in fields(kind)
+                       if item.name not in evidence}, **evidence)
 
     def reissued(self, at: float) -> PendingProblemOrder:
         """This order sent again at ``at`` to a new owner, its first-call window kept."""
@@ -1389,7 +1394,7 @@ def linked(self, order: PendingProblemOrder) -> PendingProblemOrder:
     def folded(self, into: str, why: str) -> ProblemOrder:
         if self.pending:
             raise ValueError("The problem has open orders; link or drop them first.")
-        return FoldedProblemOrder(original=self, into=into, why=why)
+        return self.copy_as(FoldedProblemOrder, original=self, into=into, why=why)
 
     @staticmethod
     def request(sender: Thread, problems: tuple[str, ...]) -> str:
@@ -1446,7 +1451,6 @@ def checked(self) -> CheckedProblemOrder:
     run: OrderRun
     deletes: OrderDeletion = field(
         default_factory=NoOrderDeletion, metadata={"wire_omit_default": True})
-    serves: type[OrderPhase] = field(default=UnplannedPhase, metadata={"wire_omit_default": True})
 
     def __post_init__(self):
         if not self.edits or not self.banned:
@@ -1474,10 +1478,6 @@ def changed(name: str) -> bool:
         return sum(1 for edit in self.edits
                    if (named := self.NAMED_FILE.findall(edit)) and not any(map(changed, named)))
 
-    def copy_as[Kind: ProblemOrder](self, kind: type[Kind], **evidence) -> Kind:
-        return kind(**{item.name: getattr(self, item.name) for item in fields(kind)
-                       if item.name not in evidence}, **evidence)
-
     def merged(self, sha: str) -> ProblemOrder:
         return self.copy_as(MergedProblemOrder, sha=sha)
 
@@ -1667,6 +1667,9 @@ class WithdrawnProblemOrder(CheckedProblemOrder):
     pending: ClassVar[bool] = False
     line: ClassVar[str] = "Withdrawn"
 
+    def require_close(self) -> None:
+        """Retiring an erroneous order does not claim installed work."""
+
     def withdrawals(self, todo: Todo) -> tuple[str, ...]:
         superseded = f" (superseded by {self.superseded_by})" if self.superseded_by else ""
         return (f"{todo.id}: order {self.id} withdrawn from {self.previous.owner}, "

tests/test_installed_merge.py

@@ -5,25 +5,30 @@
 
 import pytest
 
+from agent_comms.commander import order as commander_order
 from agent_comms.repo import Repo
 from agent_comms.field_codec import FieldCodec
 from agent_comms.todos import (CheckedProblemOrder, DoneTodoState, MergedProblemOrder, OperationDelivery,
                                MergedPullRequest, PullRequests, UnorderedProblemOrder)
-from agent_comms.tools import CommsOrderDoneTool
+from agent_comms.tools import CommsOrderDoneTool, CommsOrderServeTool
 from agent_comms.recorded_time import RecordedEventTime
+from agent_comms.plan_phases import RecordPlanePhase, UnplannedPhase
 from cutover.carry_merged_orders import MergedOrders
+from cutover.carry_order_phases import OrderPhases
 from test_commander_problem_orders import working
 from test_order_progress import checkout, commit, deadline_lines, git
 
 
 @pytest.mark.parametrize('delivered', (False, True))
-@pytest.mark.parametrize('legacy_depth', (0, 1, 2))
+@pytest.mark.parametrize('legacy_depth', (0, 1, 2, 3))
 def test_installed_merge_closes_pending_or_delivered_order_once(
         commander, commander_script, checkout, tmp_path, monkeypatch, capsys,
         delivered, legacy_depth):
     comms, caller = commander
     script, _ = commander_script
     todo, unit = checkout
+    CommsOrderServeTool(problem=todo.id, serves=RecordPlanePhase).apply(comms, caller)
+    todo = comms.goals.todos.get(todo.id)
     if delivered:
         CommsOrderDoneTool(problem=todo.id, order_id=todo.order.id,
                           delivery=OperationDelivery(at=RecordedEventTime(500)),
@@ -43,23 +48,22 @@ def test_installed_merge_closes_pending_or_delivered_order_once(
     git(installed, 'fetch', '-q', 'origin', 'main')
     git(installed, 'checkout', '-q', '-B', 'main', base)
     repo = Repo(checkout=installed, github=todo.repo)
-    config = script.CONFIG.model_copy(update={'repos': (repo,)})
-    monkeypatch.setattr(script, 'CONFIG', config)
+    script = script.model_copy(update={'repos': (repo,)})
     merge = MergedPullRequest(1, 'fix/p1', sha, merge=sha, merged_at=500,
                               updated_at=500, orders=(original.id,))
     monkeypatch.setattr(PullRequests, 'read', lambda self, repo, state, order:
                         (merge,) if state == 'merged' else ())
 
     # The object being available locally is not installation: HEAD must contain it.
     assert not repo.contains(sha)
-    assert script.COMMANDER.orders(script.CONFIG.repos) == (todo,)
+    assert script.todos() == (todo,)
     assert comms.goals.todos.get(todo.id) == todo
     if not delivered:
         assert 'waiting on install' in deadline_lines(script)[0]
 
     git(installed, 'checkout', '-q', '-B', 'main', sha)
     assert repo.contains(sha)
-    assert script.COMMANDER.orders(script.CONFIG.repos) == ()
+    assert script.todos() == ()
     assert script.held_orders() == ()
     closed = comms.goals.todos.get(todo.id)
     assert closed.state is DoneTodoState and closed.assignment is None
@@ -75,9 +79,10 @@ def test_installed_merge_closes_pending_or_delivered_order_once(
 
     if legacy_depth:
         legacy = {'kind': 'merged', 'original': FieldCodec.encode(original), 'sha': sha}
-        if legacy_depth == 2:
+        for _ in range(legacy_depth - 1):
             closed = replace(closed, order=closed.order.folded('P2', 'Same root cause.'))
             legacy = {**FieldCodec.encode(closed.order), 'original': legacy}
+            legacy.pop('serves')  # Before P291 the fold discarded this base fact.
         encoded = json.dumps(legacy)
         with comms.goals.todos.writing() as db:
             db.execute('UPDATE todo SET "order"=? WHERE id=?', (encoded, todo.id))
@@ -86,16 +91,25 @@ def test_installed_merge_closes_pending_or_delivered_order_once(
             assert db.execute('SELECT "order" FROM todo WHERE id=?', (todo.id,)).fetchone()[0] == encoded
         assert MergedOrders.carry(comms.root, apply=True).changed == 1
         assert MergedOrders.carry(comms.root, apply=True).changed == 0
+        if legacy_depth >= 2:
+            with comms.goals.todos.writing() as db:
+                before_phase = db.execute('SELECT "order" FROM todo WHERE id=?', (todo.id,)).fetchone()[0]
+            assert OrderPhases.carry(comms.root, apply=False).changed == legacy_depth - 1
+            with comms.goals.todos.writing() as db:
+                assert db.execute('SELECT "order" FROM todo WHERE id=?', (todo.id,)).fetchone()[0] == before_phase
+            assert OrderPhases.carry(comms.root, apply=True).changed == legacy_depth - 1
+            assert OrderPhases.carry(comms.root, apply=True).changed == 0
         assert comms.goals.todos.get(todo.id) == closed
+    assert RecordPlanePhase.serving((closed,)) == (closed,)
 
     def no_child(*args, **kwargs):
         pytest.fail('A repeated closed-order read launched a subprocess')
 
     with monkeypatch.context() as repeat:
         repeat.setattr(subprocess, 'run', no_child)
-        assert script.COMMANDER.orders(script.CONFIG.repos) == ()
+        assert script.todos() == ()
         assert comms.goals.todos.get(todo.id) == closed
-    script.board()
+    script.write_board()
     assert f'| {todo.id}:' not in (tmp_path / 'board.md').read_text()
     assert deadline_lines(script) == []
     assert comms.goals.todos.get(todo.id) == closed
@@ -106,7 +120,7 @@ class TickComplete(Exception):
     def next_tick(seconds):
         raise TickComplete
 
-    monkeypatch.setattr(script.time, 'sleep', next_tick)
+    monkeypatch.setattr(commander_order.time, 'sleep', next_tick)
     with pytest.raises(TickComplete):
         script.overdue()
     assert 'OVERDUE:' not in capsys.readouterr().out
@@ -144,6 +158,8 @@ def test_only_checked_orders_are_selected_for_merge_reconciliation(commander, ch
     comms.goals.todos.reconcile_merges(comms.registry.require('lead').incarnation,
                                       (), comms.registry.storage_rule)
     assert comms.goals.todos.get(todo.id) == unordered
+    assert unordered.order.serves is UnplannedPhase
+    assert RecordPlanePhase.serving((unordered,)) == ()
 
 
 def test_failed_git_is_not_a_negative_ancestry_result(tmp_path):

tools/cutover/carry_order_phases.py

@@ -0,0 +1,51 @@
+"""Give saved folded orders their original phase before the inherited-field reader runs."""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+from pathlib import Path
+
+from agent_comms.todos import FoldedProblemOrder, Todo
+from agent_comms.typed_table import TypedRow
+
+from .carry import Carried, TodoRows
+
+
+@dataclass(frozen=True)
+class FoldedPhase(TypedRow):
+    id: str
+    path: str
+
+
+class OrderPhases(TodoRows):
+    @classmethod
+    def carry(cls, root: Path, *, apply: bool) -> Carried:
+        changed = 0
+        parameters = (FoldedProblemOrder.declared_name,)
+        with cls.todo_rows(root, apply=apply) as db:
+            if db is None:
+                return Carried(0)
+            # Inner folds first: a parent then reads the child's converted phase,
+            # not a stale decoded copy captured before the inner update.
+            rows = FoldedPhase.read(
+                db.execute(
+                    f'SELECT t.id, n.path AS path FROM "{Todo.declared_name}" t,'
+                    " json_tree(t.\"order\") n WHERE n.key='kind' AND n.value=?"
+                    " ORDER BY length(n.path) DESC",
+                    parameters,
+                )
+            )
+            for row in rows:
+                phase, original = row.path + ".serves", row.path + ".original.serves"
+                changed += db.execute(
+                    f'UPDATE "{Todo.declared_name}"'
+                    ' SET "order"=json_set("order", ?, json_extract("order", ?))'
+                    ' WHERE id=? AND json_type("order", ?) IS NULL'
+                    ' AND json_type("order", ?) IS NOT NULL',
+                    (phase, original, row.id, phase, original),
+                ).rowcount
+        return cls.carried(root, changed, {"folded_phases": changed}, apply=apply)
+
+
+if __name__ == "__main__":
+    OrderPhases.main()