mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[chore] orchestrator: skip human-only assignees in claimed/pm-review dispatchers
Defense-in-depth for the spawn_agent human-role chokepoint (d31d6719).
The chokepoint structurally guarantees no CEO/prompter/secretary container
can ever spawn — every dispatcher goes through spawn_agent. But two
dispatchers resolve an arbitrary assigned_to and spawn it with only a
None/unknown-role filter, so a human-assigned task would reach the
chokepoint and RAISE: caught by the per-dispatcher try/except, but it
aborts that dispatcher's whole tick (stalling other respawns behind the
mis-assigned task) and error-logs every cycle. The other dispatchers are
already safe by whitelist/hardcoded slug (blocker_resolver_slug returns
None for non-PM/non-BOARD; escalation/approval use whitelists; marketing
and audit hardcode their non-human slug).
- _claimed_task_needs_agent: return None for a CEO/prompter/secretary
assignee — no container to respawn, and do NOT release a human-owned
task to pending (that would re-route it to a PM). Leave it for the human.
- _dispatch_pm_review_work (assigned branch): skip a human-only assignee
so a CEO-assigned awaiting_pm_review task neither spawns nor aborts the
dispatcher's tick.
Audited all target-iterating dispatchers; only these two lacked a filter.
Regression tests cover both skips.
This commit is contained in:
@@ -167,6 +167,22 @@ def test_hitl_blocked_claimed_task_is_skipped() -> None:
|
||||
assert orch._claimed_task_needs_agent(task) is None
|
||||
|
||||
|
||||
def test_claimed_task_assigned_to_ceo_is_not_respawned() -> None:
|
||||
# A claimed/in_progress task whose assignee is the CEO (or any human-only
|
||||
# role) has no container to respawn — the CEO is the human operator. The
|
||||
# resolver must return None so the dispatcher neither spawns a CEO
|
||||
# container NOR releases a human-owned task to pending. Defense-in-depth
|
||||
# for the spawn_agent human-role chokepoint (2026-06-27 CEO-spawn incident).
|
||||
orch = _orch()
|
||||
task: dict[str, Any] = {
|
||||
"id": "t1",
|
||||
"status": "in_progress",
|
||||
"assigned_to": AGENT_UUIDS["ceo"],
|
||||
"updated_at": _STALE,
|
||||
}
|
||||
assert orch._claimed_task_needs_agent(task) is None
|
||||
|
||||
|
||||
def test_in_progress_task_with_no_agent_returns_assignee() -> None:
|
||||
orch = _orch()
|
||||
task: dict[str, Any] = {
|
||||
|
||||
@@ -165,3 +165,33 @@ async def test_dispatch_a2a_mixed_targets_skips_only_human() -> None:
|
||||
spawned = [c.kwargs.get("agent_id") for c in orch.spawn_agent.call_args_list]
|
||||
assert "ceo" not in spawned
|
||||
assert spawned == ["be-dev-1"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _dispatch_pm_review_work — skips a human-only assignee (defense-in-depth)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_pm_review_skips_ceo_assignee() -> None:
|
||||
"""An awaiting_pm_review task assigned to the CEO must NOT respawn a CEO
|
||||
container, and must NOT abort the dispatcher's tick (which would stall
|
||||
other PM-review respawns behind it). The skip leaves it for the human."""
|
||||
orch = object.__new__(AgentOrchestrator)
|
||||
orch.spawn_agent = AsyncMock() # type: ignore[method-assign]
|
||||
orch._is_agent_active = MagicMock(return_value=False) # type: ignore[method-assign]
|
||||
orch._pm_respawn_should_gate = AsyncMock(return_value=False) # type: ignore[method-assign]
|
||||
orch._fetch_tasks = AsyncMock( # type: ignore[method-assign]
|
||||
return_value=[
|
||||
{
|
||||
"id": "t1",
|
||||
"status": "awaiting_pm_review",
|
||||
"team": "backend",
|
||||
"assigned_to": AGENT_UUIDS["ceo"],
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
await orch._dispatch_pm_review_work(MagicMock())
|
||||
|
||||
orch.spawn_agent.assert_not_awaited()
|
||||
|
||||
Reference in New Issue
Block a user