From 55ff05e6ec686a9a6f33d46dc1c22a2c484580f0 Mon Sep 17 00:00:00 2001 From: Renn F Date: Tue, 16 Jun 2026 00:14:01 +0200 Subject: [PATCH] fix(task): restore pre-block owner when an admin override leaves blocked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A developer that hits a wall calls i_am_blocked, which escalates the code task to its cell PM (assigned_to=PM, BLOCKED) and snapshots the dev as pre_block_assignee — the intended dev->cell-PM triage handoff. The in-band recovery (unblock(restore=True)) hands ownership back to the dev. But the OUT-OF-BAND paths — the operator PATCH /tasks/{id} status override and the orchestrator's own _auto_recover_blocked_parent / _auto_resume_paused_parent — go through admin_set_status, which set only status and never restored the owner. The task re-entered pending/in_progress still owned by the PM, and the dispatcher then execute-spawned the PM on a code task it cannot do ('break this down and delegate' against the task itself) -> respawn loop. admin_set_status now, when taking a task out of 'blocked' into pending/in_progress with a pre-block snapshot present, routes through the existing _apply_pre_block_restore primitive (the same one unblock(restore=True) uses) to hand ownership back to the executor. Every other override is unchanged, and the escalate/apply_escalation/block-down path is untouched, so the dev->cell-PM handoff still works. + 2 regression tests. --- roboco/services/task.py | 15 ++++++++++ tests/unit/services/test_task.py | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/roboco/services/task.py b/roboco/services/task.py index 0da837f3..f718873f 100644 --- a/roboco/services/task.py +++ b/roboco/services/task.py @@ -1014,6 +1014,15 @@ class TaskService(BaseService): task wedged in a state with no valid in-band move (e.g. a ``blocked`` task whose work already merged out-of-band). The change is recorded in the audit log like any other transition — no status change may skip it. + + Taking a task OUT of ``blocked`` here (operator PATCH, or the + orchestrator's auto-recover/auto-resume) restores the pre-block owner + exactly as ``unblock(restore=True)`` does. Without this, a code task + that a developer escalated to its cell PM re-enters ``pending``/ + ``in_progress`` still owned by that PM, and the dispatcher execute-spawns + the PM on a dev task it cannot do (a respawn loop). The in-band escalate + and block-down transitions are untouched — this fires only on + re-activation, and only when a pre-block snapshot exists. """ task = await self.get(task_id) if not task: @@ -1023,6 +1032,12 @@ class TaskService(BaseService): if isinstance(task.status, TaskStatus) else str(task.status) ) + if ( + from_status == TaskStatus.BLOCKED.value + and new_status in (TaskStatus.PENDING, TaskStatus.IN_PROGRESS) + and task.pre_block_assignee is not None + ): + return await self._apply_pre_block_restore(task, new_status) task.status = new_status await self.session.flush() self._emit_status_transition_audit( diff --git a/tests/unit/services/test_task.py b/tests/unit/services/test_task.py index 047d817f..58c65462 100644 --- a/tests/unit/services/test_task.py +++ b/tests/unit/services/test_task.py @@ -543,6 +543,53 @@ async def test_unblock_no_branch_returns_to_pending() -> None: assert task.assigned_to == raiser +@pytest.mark.asyncio +async def test_admin_set_status_out_of_blocked_restores_pre_block_owner() -> None: + # A code task a dev escalated to its cell PM (assigned_to=PM, BLOCKED, + # snapshot=dev). Taking it out of blocked via the admin override (operator + # PATCH, or the orchestrator's auto-recover/auto-resume) must hand ownership + # back to the dev — otherwise it re-enters pending/in_progress still owned by + # the PM and the dispatcher execute-spawns the PM on a dev code task (loop). + dev = uuid4() + pm = uuid4() + task = _build_task( + status=TaskStatus.BLOCKED, + assigned_to=pm, + claimed_by=pm, + branch_name="feature/frontend/abc--def--ghi", + pre_block_state="in_progress", + pre_block_assignee=dev, + ) + svc = TaskService(MagicMock(flush=AsyncMock())) + _bind(svc, "get", AsyncMock(return_value=task)) + out = await svc.admin_set_status(task.id, TaskStatus.IN_PROGRESS) + assert out is task + assert task.status == TaskStatus.IN_PROGRESS + assert task.assigned_to == dev + assert task.claimed_by == dev + assert task.pre_block_assignee is None + assert task.pre_block_state is None + + +@pytest.mark.asyncio +async def test_admin_set_status_non_blocked_is_bare_status_set() -> None: + # The restore branch fires ONLY on blocked -> pending/in_progress with a + # snapshot. Every other override stays a plain status set; the owner is + # untouched (no spurious restore/divert). + owner = uuid4() + task = _build_task( + status=TaskStatus.AWAITING_PM_REVIEW, + assigned_to=owner, + claimed_by=owner, + ) + svc = TaskService(MagicMock(flush=AsyncMock())) + _bind(svc, "get", AsyncMock(return_value=task)) + out = await svc.admin_set_status(task.id, TaskStatus.COMPLETED) + assert out is task + assert task.status == TaskStatus.COMPLETED + assert task.assigned_to == owner + + @pytest.mark.asyncio async def test_unblock_with_branch_resumes_in_progress() -> None: # A task claimed (has a branch) before it blocked resumes in_progress.