From a8056892b6b34219cb85301a755c30d1fbf76556 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 23 May 2026 06:15:48 +0200 Subject: [PATCH] fix(gateway): main_pm completes an in_progress root; walk it to CEO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A root resumed from paused (its subtasks all terminal) sits in in_progress, but escalate_to_ceo requires source=awaiting_pm_review and nothing moves the root there — submit_up is cell-PM-only. main_pm_complete rejected the in_progress root ("expected awaiting_pm_review"), so the chain stalled one step short of CEO. main_pm_complete now: - accepts in_progress (own root, subtasks terminal) in addition to awaiting_pm_review; - after opening the root->master PR (which sets pr_created), walks the root in_progress->awaiting_pm_review via the TaskService transition (role-validated, no gateway team-match) so escalate_to_ceo's source gate passes; - then escalates -> awaiting_ceo_approval. The root->master PR is non-empty because the cell->root PR was already merged into the root branch (the prior cell-completion fix). Updated test_main_pm_complete_wrong_status (in_progress is now valid; uses paused) and added a regression test for the in_progress->CEO path. --- .../services/gateway/choreographer/_impl.py | 42 ++++++++++++++++- .../test_choreographer_impl_branches.py | 4 +- tests/unit/gateway/test_choreographer_pm.py | 47 +++++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/roboco/services/gateway/choreographer/_impl.py b/roboco/services/gateway/choreographer/_impl.py index 860629e3..f15c1040 100644 --- a/roboco/services/gateway/choreographer/_impl.py +++ b/roboco/services/gateway/choreographer/_impl.py @@ -3837,10 +3837,16 @@ class Choreographer: main_pm_agent_id, root_task_id ), ) - if str(t.status) != "awaiting_pm_review": + # #183: accept in_progress too. A root resumed from paused (its + # subtasks all done) sits in in_progress — there is no submit_up for + # roots to move it to awaiting_pm_review. main_pm_complete itself + # opens the root→master PR and walks it through awaiting_pm_review + # before escalating; the CEO is the root's reviewer. + if str(t.status) not in ("awaiting_pm_review", "in_progress"): return Envelope.invalid_state( message=( - f"task {root_task_id} is in {t.status}, expected awaiting_pm_review" + f"task {root_task_id} is in {t.status}, expected" + " awaiting_pm_review or in_progress" ), remediate=( "this task is not ready for main-PM completion." @@ -3905,6 +3911,38 @@ class Choreographer: if needs_pr: await self.git.create_pr(t.branch_name, parent="master", is_root_pr=True) + # #183: escalate_to_ceo requires source=awaiting_pm_review, but a root + # resumed from paused is in_progress and nothing else moves it there + # (submit_up is cell-PM-only). The root→master PR now exists, so walk + # the root through awaiting_pm_review here. Uses the TaskService + # transition directly (no gateway team-match) — submit_pm_review's + # gates (in_progress + branch + pr_created + subtasks terminal) all + # hold at this point. + refreshed = await self.task.get(root_task_id) + if refreshed is not None and str(refreshed.status) == "in_progress": + advanced = await self.task.submit_pm_review( + main_pm_agent_id, root_task_id, notes + ) + if advanced is None: + return await self._emit_rejection( + Envelope.invalid_state( + message=( + "could not move root to awaiting_pm_review for CEO" + " escalation" + ), + remediate=( + "ensure the root→master PR is open and all subtasks" + " are terminal, then retry complete" + ), + context_briefing=await self._briefing_for( + main_pm_agent_id, root_task_id + ), + ).with_introspection(task=refreshed, role="main_pm"), + agent_id=main_pm_agent_id, + task_id=root_task_id, + verb="main_pm_complete", + ) + # Use kwargs — service signature is (task_id, agent_role="cell_pm", # notes=None). Positional was passing agent_id as task_id and the # actual task_id as agent_role (audit D-07). diff --git a/tests/unit/gateway/test_choreographer_impl_branches.py b/tests/unit/gateway/test_choreographer_impl_branches.py index 4d0fdbbc..57869c29 100644 --- a/tests/unit/gateway/test_choreographer_impl_branches.py +++ b/tests/unit/gateway/test_choreographer_impl_branches.py @@ -909,10 +909,12 @@ async def test_main_pm_complete_not_assigned() -> None: @pytest.mark.asyncio async def test_main_pm_complete_wrong_status() -> None: + # #183: in_progress is now an accepted source (root resumed from paused); + # use paused — a genuinely non-completable status — to exercise the guard. main_pm_id = uuid4() task_id = uuid4() task = MagicMock( - status="in_progress", + status="paused", assigned_to=main_pm_id, parent_task_id=None, title="t", diff --git a/tests/unit/gateway/test_choreographer_pm.py b/tests/unit/gateway/test_choreographer_pm.py index 42d13f75..fed6a100 100644 --- a/tests/unit/gateway/test_choreographer_pm.py +++ b/tests/unit/gateway/test_choreographer_pm.py @@ -425,6 +425,53 @@ async def test_main_pm_complete_opens_master_pr_and_escalates() -> None: task_svc.escalate_to_ceo.assert_awaited_once() +@pytest.mark.asyncio +async def test_main_pm_complete_advances_in_progress_root_to_ceo() -> None: + """#183: a root resumed to in_progress (subtasks all done) has no submit_up + to reach awaiting_pm_review. main_pm_complete opens the root→master PR, + walks the root through awaiting_pm_review, then escalates to CEO.""" + main_pm_id = uuid4() + root_task_id = uuid4() + in_prog = MagicMock( + id=root_task_id, + status="in_progress", + assigned_to=main_pm_id, + pr_number=None, + branch_name="feature/main_pm/root123", + parent_task_id=None, + team="main_pm", + ) + awaiting = MagicMock(**{**in_prog.__dict__, "status": "awaiting_pm_review"}) + after = MagicMock(**{**in_prog.__dict__, "status": "awaiting_ceo_approval"}) + task_svc = AsyncMock() + task_svc.get.return_value = in_prog + task_svc.submit_pm_review.return_value = awaiting + task_svc.escalate_to_ceo.return_value = after + task_svc.all_subtasks_terminal.return_value = True + git_svc = AsyncMock() + git_svc.create_pr.return_value = {"pr_number": 99, "pr_url": "https://x/y/pull/99"} + journal_svc = AsyncMock() + journal_svc.has_decision_for_task.return_value = True + journal_svc.latest_decision_at.return_value = datetime.now(UTC) + journal_svc.has_reflect_for_task.return_value = True + deps = _make_deps(task=task_svc, git=git_svc, journal=journal_svc) + c = Choreographer(deps) + + env = await c.main_pm_complete( + main_pm_id, root_task_id, notes="root scope reviewed; ready for CEO sign-off" + ) + assert env.error is None + assert env.status == "awaiting_ceo_approval" + git_svc.create_pr.assert_awaited_once_with( + "feature/main_pm/root123", + parent="master", + is_root_pr=True, + ) + # #183: the in_progress→awaiting_pm_review hop must run before escalation. + task_svc.submit_pm_review.assert_awaited_once() + task_svc.escalate_to_ceo.assert_awaited_once() + + @pytest.mark.asyncio async def test_main_pm_complete_skips_pr_creation_if_already_master_targeted() -> None: main_pm_id = uuid4()