mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(task): restore pre-block owner when an admin override leaves blocked
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.
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user