mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): reject Cell-PM-assigned subtasks that aren't task_type=planning
Bug B from the 2026-05-09 smoke run. main-pm called delegate(assigned_to='be-pm', task_type='code'). The chain validator let it through (be-pm IS in main-pm's allowed targets), the schema let it through (task_type='code' is a valid enum value), and the subtask got created mis-typed. Task 0 made it cosmetically work because PMs can now plan code-typed parents — but the model is wrong: a Cell PM owns the PLANNING of the slice; the code execution is what they delegate to devs. New gate in _delegate_static_guards: when assignee is a Cell PM (be-pm/fe-pm/ux-pm), task_type MUST be 'planning'. Returns invalid_state with a remediate hint pointing at the right type. Devs are unrestricted (could be code OR documentation, depending on the slice). Tests: 3139 passing (+ 2 regression tests pinning the rule), 100% coverage, ruff clean.
This commit is contained in:
@@ -12,7 +12,7 @@ injection so later phases just fill in the bodies.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import Any
|
from typing import Any, ClassVar
|
||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
|
|
||||||
import structlog
|
import structlog
|
||||||
@@ -1430,6 +1430,40 @@ class Choreographer:
|
|||||||
remediate="check team/task_type/estimated_complexity",
|
remediate="check team/task_type/estimated_complexity",
|
||||||
context_briefing=await self._briefing_for(pm_agent_id, parent_task_id),
|
context_briefing=await self._briefing_for(pm_agent_id, parent_task_id),
|
||||||
)
|
)
|
||||||
|
if type_error := self._validate_assignee_task_type(
|
||||||
|
inputs.assigned_to, inputs.task_type
|
||||||
|
):
|
||||||
|
return Envelope.invalid_state(
|
||||||
|
message=type_error,
|
||||||
|
remediate=(
|
||||||
|
"Cell PMs (be-pm/fe-pm/ux-pm) own PLANNING tasks — they "
|
||||||
|
"decompose the slice and delegate code work to devs. Pass "
|
||||||
|
"task_type='planning' when delegating to a Cell PM."
|
||||||
|
),
|
||||||
|
context_briefing=await self._briefing_for(pm_agent_id, parent_task_id),
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
|
||||||
|
_CELL_PM_SLUGS: ClassVar[frozenset[str]] = frozenset({"be-pm", "fe-pm", "ux-pm"})
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _validate_assignee_task_type(
|
||||||
|
assigned_to: str, task_type: str
|
||||||
|
) -> str | None:
|
||||||
|
"""Reject role-vs-type misclassifications.
|
||||||
|
|
||||||
|
Rule (2026-05-09 smoke Bug B): when delegating to a Cell PM, the
|
||||||
|
subtask must be `planning`-typed. The Cell PM owns the planning
|
||||||
|
of the slice and delegates code execution to devs; a code-typed
|
||||||
|
task assigned to a Cell PM conflates the two layers and made
|
||||||
|
the lifecycle harder to reason about (a code task that nobody
|
||||||
|
will execute, just plan).
|
||||||
|
"""
|
||||||
|
if assigned_to in Choreographer._CELL_PM_SLUGS and task_type != "planning":
|
||||||
|
return (
|
||||||
|
f"task_type={task_type!r} is invalid for assignee {assigned_to!r}: "
|
||||||
|
f"Cell PMs own planning tasks, not code/documentation/etc."
|
||||||
|
)
|
||||||
return None
|
return None
|
||||||
|
|
||||||
async def _delegate_lifecycle_guards(
|
async def _delegate_lifecycle_guards(
|
||||||
|
|||||||
@@ -749,3 +749,80 @@ async def test_i_am_idle_with_unread_skips_pause_and_idle() -> None:
|
|||||||
assert env.status == "idle_with_unread"
|
assert env.status == "idle_with_unread"
|
||||||
task_svc.list_in_progress_for_agent.assert_not_awaited()
|
task_svc.list_in_progress_for_agent.assert_not_awaited()
|
||||||
task_svc.mark_agent_idle.assert_not_awaited()
|
task_svc.mark_agent_idle.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Bug B from 2026-05-09 smoke: main-pm sent task_type="code" when delegating
|
||||||
|
# to be-pm. A Cell PM PLANS the slice; it does not execute code. The
|
||||||
|
# delegate verb must reject this misclassification at the gateway so future
|
||||||
|
# spawns can't recreate the wrong-type-task condition.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delegate_main_pm_to_cell_pm_rejects_code_typed_subtask() -> None:
|
||||||
|
main_pm_id = uuid4()
|
||||||
|
parent_id = uuid4()
|
||||||
|
parent = MagicMock(
|
||||||
|
id=parent_id,
|
||||||
|
project_id=uuid4(),
|
||||||
|
status="in_progress",
|
||||||
|
assigned_to=main_pm_id,
|
||||||
|
)
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.get.return_value = parent
|
||||||
|
task_svc.agent_for.return_value = MagicMock(role="main_pm", team="main_pm")
|
||||||
|
task_svc.get_subtasks.return_value = []
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.delegate(
|
||||||
|
main_pm_id,
|
||||||
|
parent_id,
|
||||||
|
DelegateInputs(
|
||||||
|
title="Backend slice",
|
||||||
|
description="Plan + drive backend work",
|
||||||
|
assigned_to="be-pm",
|
||||||
|
team="backend",
|
||||||
|
task_type="code", # WRONG — Cell PM should get planning
|
||||||
|
),
|
||||||
|
)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] == "invalid_state"
|
||||||
|
assert "planning" in body["message"].lower()
|
||||||
|
assert "be-pm" in body["message"] or "cell_pm" in body["message"].lower()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_delegate_main_pm_to_cell_pm_accepts_planning_subtask() -> None:
|
||||||
|
"""The contract from the test above: planning IS the right type."""
|
||||||
|
main_pm_id = uuid4()
|
||||||
|
parent_id = uuid4()
|
||||||
|
project_id = uuid4()
|
||||||
|
parent = MagicMock(
|
||||||
|
id=parent_id,
|
||||||
|
project_id=project_id,
|
||||||
|
status="in_progress",
|
||||||
|
assigned_to=main_pm_id,
|
||||||
|
)
|
||||||
|
new_task = MagicMock(id=uuid4())
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.get.return_value = parent
|
||||||
|
task_svc.agent_for.return_value = MagicMock(role="main_pm", team="main_pm")
|
||||||
|
task_svc.get_subtasks.return_value = []
|
||||||
|
task_svc.create_subtask.return_value = new_task
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.delegate(
|
||||||
|
main_pm_id,
|
||||||
|
parent_id,
|
||||||
|
DelegateInputs(
|
||||||
|
title="Backend slice",
|
||||||
|
description="Plan + drive backend work",
|
||||||
|
assigned_to="be-pm",
|
||||||
|
team="backend",
|
||||||
|
task_type="planning",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
assert env.error is None
|
||||||
|
|||||||
Reference in New Issue
Block a user