mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(megatask): guardrail the wave sequence at the claim chokepoint (#382)
The Main PM claimed every MegaTask wave at once, ignoring the collision-ordered
dependencies. The sequencing data was correct (analyzer wired proper waves), but
enforcement was only half-wired: the unmet-dependency guard lives on the gateway
claim verbs (i_will_plan -> _run_claim_guards), while the orchestrator dispatches
coordination roots itself — _dispatch_pm_work fetches pending with no dependency
filter and _claim_task_for_agent system-claims via the raw POST /tasks/{id}/claim
route -> TaskService.claim, which had no dependency check. So the orchestrator
claimed every pending root-subtask for the Main PM regardless of wave.
Enforce the sequence at the claim chokepoint: _validate_claim_preconditions now
refuses to claim a PENDING task while any depends_on task is non-terminal
(extracted into _claim_blocked_by_dependencies for the complexity budget). This
guardrails every claim path — the gateway verbs (redundant) and the orchestrator
raw dispatch claim (the hole). Scoped to a PENDING start-of-work claim so a
mid-lifecycle QA/doc claim is unaffected; dependencies are monotonic so each wave
claims normally once the prior one completes.
Adds test_claim_pending_with_unmet_dependency_returns_none (blocked with an
unfinished dependency; claimable once it completes).
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -2576,6 +2576,30 @@ class TaskService(BaseService):
|
|||||||
TaskStatus.AWAITING_PM_REVIEW,
|
TaskStatus.AWAITING_PM_REVIEW,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async def _claim_blocked_by_dependencies(self, task: TaskTable) -> bool:
|
||||||
|
"""True when a PENDING task can't be claimed yet — a ``depends_on`` task
|
||||||
|
is still non-terminal (the sequence guardrail).
|
||||||
|
|
||||||
|
The gateway claim verbs enforce this, but the orchestrator's dispatcher
|
||||||
|
system-claims via the raw ``/tasks/{id}/claim`` route (which skips the
|
||||||
|
gateway guards), so a MegaTask root-subtask in a later wave was claimed
|
||||||
|
out of order (the Main PM held every wave at once). Enforcing it at the
|
||||||
|
claim chokepoint guardrails every path. Scoped to a PENDING start-of-work
|
||||||
|
claim — a mid-lifecycle claim (QA/doc on an ``awaiting_*`` task) already
|
||||||
|
cleared its dependencies at the original claim.
|
||||||
|
"""
|
||||||
|
if task.status != TaskStatus.PENDING or not task.dependency_ids:
|
||||||
|
return False
|
||||||
|
unmet = await self.unmet_dependency_ids(list(task.dependency_ids))
|
||||||
|
if not unmet:
|
||||||
|
return False
|
||||||
|
self.log.warning(
|
||||||
|
"Cannot claim task - unmet dependencies",
|
||||||
|
task_id=str(task.id),
|
||||||
|
unmet=[str(dep_id) for dep_id in unmet],
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
async def _validate_claim_preconditions(
|
async def _validate_claim_preconditions(
|
||||||
self,
|
self,
|
||||||
task: TaskTable,
|
task: TaskTable,
|
||||||
@@ -2590,6 +2614,9 @@ class TaskService(BaseService):
|
|||||||
self.log.warning(f"Cannot claim task - {error}", task_id=str(task.id))
|
self.log.warning(f"Cannot claim task - {error}", task_id=str(task.id))
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
if await self._claim_blocked_by_dependencies(task):
|
||||||
|
return False
|
||||||
|
|
||||||
if error := self._validate_claim_team(task, agent):
|
if error := self._validate_claim_team(task, agent):
|
||||||
self.log.warning(f"Cannot claim task - {error}", task_id=str(task.id))
|
self.log.warning(f"Cannot claim task - {error}", task_id=str(task.id))
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -1329,6 +1329,30 @@ async def test_claim_pending_task_with_existing_branch(
|
|||||||
assert claimed.assigned_to == task_setup["agent_id"]
|
assert claimed.assigned_to == task_setup["agent_id"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_claim_pending_with_unmet_dependency_returns_none(
|
||||||
|
task_setup: dict, db_session: AsyncSession
|
||||||
|
) -> None:
|
||||||
|
"""Sequence guardrail: a PENDING task with a non-terminal dependency cannot
|
||||||
|
be claimed — even via the raw claim path the orchestrator dispatcher uses.
|
||||||
|
Regression for the MegaTask "Main PM claimed every wave at once" bug."""
|
||||||
|
svc = task_setup["svc"]
|
||||||
|
dep = await svc.create(_req(task_setup, title="wave-0 dependency"))
|
||||||
|
task = await svc.create(_req(task_setup, title="wave-1 dependent"))
|
||||||
|
task.branch_name = "feature/backend/abcd1234"
|
||||||
|
await db_session.flush()
|
||||||
|
await svc.add_dependency(task.id, dep.id) # task depends_on dep (still PENDING)
|
||||||
|
|
||||||
|
assert await svc.claim(task.id, task_setup["agent_id"]) is None
|
||||||
|
|
||||||
|
# Once the dependency reaches a terminal state, the claim goes through.
|
||||||
|
dep.status = TaskStatus.COMPLETED
|
||||||
|
await db_session.flush()
|
||||||
|
claimed = await svc.claim(task.id, task_setup["agent_id"])
|
||||||
|
assert claimed is not None
|
||||||
|
assert claimed.status == TaskStatus.CLAIMED
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_claim_already_claimed_by_other_returns_none(
|
async def test_claim_already_claimed_by_other_returns_none(
|
||||||
task_setup: dict, db_session: AsyncSession
|
task_setup: dict, db_session: AsyncSession
|
||||||
|
|||||||
Reference in New Issue
Block a user