mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): B6 give_me_work returns pre-assigned pending tasks first
Smoke run 3 showed Main PM's first give_me_work() returning
{status: idle, next: 'no Main PM work'} even though c7935d2c was
pending and assigned to Main PM. The filter only walked
list_assigned_for_agent (ordered by priority/updated_at — pending
could rank behind in_progress rows) and the PM path fell through
to idle because the pre-assigned pending case was not checked first.
Pre-pended a list_pending_for_agent check in both give_me_work and
pm_give_me_work: tasks where assigned_to=agent_id AND status=pending
take priority over all other lookups. Added TaskService.list_pending_for_agent
for the query (ordered by sequence, priority, created_at).
Updated existing tests in test_choreographer_dev, test_choreographer_pm_extras,
and test_heartbeat_wired to set list_pending_for_agent.return_value=[]
where they were not testing the pre-assigned path.
Spec ref: docs/superpowers/specs/2026-05-12-post-smoke-3-fixes-design.md
section B6.
This commit is contained in:
@@ -496,6 +496,21 @@ class Choreographer:
|
||||
"""Return the agent's most-actionable task or signal idle."""
|
||||
agent = await self._deps.task.agent_for(agent_id)
|
||||
role = str(agent.role) if agent is not None else "developer"
|
||||
# Pre-assigned pending tasks take priority. Smoke run 3 (2026-05-12)
|
||||
# showed agents missing tasks that were seeded with assigned_to=<them>
|
||||
# and status=pending because the earlier code only walked
|
||||
# list_assigned_for_agent (ordered by priority/updated_at — pending
|
||||
# could rank behind in_progress rows) and the PM path checked
|
||||
# awaiting_* queues but not the pre-assigned pending case.
|
||||
pre_assigned = await self._deps.task.list_pending_for_agent(agent_id)
|
||||
if pre_assigned:
|
||||
t = pre_assigned[0]
|
||||
return Envelope.ok(
|
||||
status=str(t.status),
|
||||
task_id=str(t.id),
|
||||
next=f"call i_will_work_on(task_id='{t.id}', plan='<plan>') to start",
|
||||
context_briefing=await self._briefing_for(agent_id, t.id),
|
||||
).with_introspection(task=t, role=role)
|
||||
assigned = await self._deps.task.list_assigned_for_agent(agent_id)
|
||||
if assigned:
|
||||
t = assigned[0]
|
||||
@@ -2883,7 +2898,25 @@ class Choreographer:
|
||||
Mirrors the developer's give_me_work but does not filter to dev-only
|
||||
statuses — PMs care about all assigned tasks (planning, paused, in
|
||||
progress, awaiting_pm_review).
|
||||
|
||||
Pre-assigned pending tasks are checked first (Wave B6, 2026-05-12).
|
||||
Smoke run 3 showed Main PM getting idle even though c7935d2c was
|
||||
pending and assigned_to=main-pm because list_assigned_for_agent
|
||||
ordered by priority/updated_at and could rank a pre-assigned pending
|
||||
task below other active rows; the pre-assigned pending check now
|
||||
wins unconditionally.
|
||||
"""
|
||||
# Pre-assigned pending tasks take priority over everything else.
|
||||
pre_assigned = await self.task.list_pending_for_agent(pm_agent_id)
|
||||
if pre_assigned:
|
||||
t = pre_assigned[0]
|
||||
await self._touch(t.id)
|
||||
return Envelope.ok(
|
||||
status=str(t.status),
|
||||
task_id=str(t.id),
|
||||
next=self._pm_next_hint(str(t.status), t.id),
|
||||
context_briefing=await self._briefing_for(pm_agent_id, t.id),
|
||||
)
|
||||
assigned = await self.task.list_assigned_for_agent(pm_agent_id)
|
||||
if assigned:
|
||||
t = assigned[0]
|
||||
|
||||
@@ -4629,6 +4629,32 @@ class TaskService(BaseService):
|
||||
result = await self.session.execute(query)
|
||||
return result.scalar_one_or_none()
|
||||
|
||||
async def list_pending_for_agent(self, agent_id: UUID) -> list[TaskTable]:
|
||||
"""Tasks assigned to this agent that are still in PENDING status.
|
||||
|
||||
Pre-gateway parity (Wave B6, 2026-05-12): give_me_work missed the
|
||||
pre-assigned case before this. PMs whose root was seeded with
|
||||
assigned_to=<them> + status=pending got 'no work' until they
|
||||
triage()'d explicitly.
|
||||
|
||||
Ordered by sequence asc, then priority asc, then created_at asc so
|
||||
earlier-sequence tasks win.
|
||||
"""
|
||||
query = (
|
||||
select(TaskTable)
|
||||
.where(
|
||||
TaskTable.assigned_to == agent_id,
|
||||
TaskTable.status == TaskStatus.PENDING,
|
||||
)
|
||||
.order_by(
|
||||
TaskTable.sequence,
|
||||
TaskTable.priority,
|
||||
TaskTable.created_at,
|
||||
)
|
||||
)
|
||||
result = await self.session.execute(query)
|
||||
return list(result.scalars().all())
|
||||
|
||||
async def list_paused_for_agent(self, agent_id: UUID) -> list[TaskTable]:
|
||||
"""Paused tasks assigned to the agent."""
|
||||
query = (
|
||||
|
||||
@@ -54,6 +54,7 @@ async def test_give_me_work_returns_assigned_task() -> None:
|
||||
agent_id = uuid4()
|
||||
task_obj = MagicMock(id=uuid4(), status="pending", title="t1")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = [task_obj]
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
@@ -70,6 +71,7 @@ async def test_give_me_work_returns_paused_when_no_assigned() -> None:
|
||||
agent_id = uuid4()
|
||||
paused_obj = MagicMock(id=uuid4(), status="paused")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
task_svc.list_paused_for_agent.return_value = [paused_obj]
|
||||
deps = _make_deps(task=task_svc)
|
||||
@@ -85,6 +87,7 @@ async def test_give_me_work_returns_paused_when_no_assigned() -> None:
|
||||
async def test_give_me_work_returns_idle_when_no_work() -> None:
|
||||
agent_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
task_svc.list_paused_for_agent.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
|
||||
@@ -909,6 +909,7 @@ async def test_pm_give_me_work_returns_first_assigned() -> None:
|
||||
pm_id = uuid4()
|
||||
t = MagicMock(id=uuid4(), status="pending", title="x", team="backend")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = [t]
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
@@ -923,6 +924,7 @@ async def test_pm_give_me_work_returns_first_assigned() -> None:
|
||||
async def test_pm_give_me_work_returns_idle_when_empty() -> None:
|
||||
pm_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
@@ -936,6 +938,7 @@ async def test_pm_give_me_work_paused_hint_mentions_subtasks() -> None:
|
||||
pm_id = uuid4()
|
||||
t = MagicMock(id=uuid4(), status="paused", title="x", team="backend")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = [t]
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
@@ -0,0 +1,244 @@
|
||||
"""Wave B6 (2026-05-12): give_me_work returns tasks pre-assigned to the agent.
|
||||
|
||||
Smoke run 3 showed Main PM's first give_me_work() returning idle even
|
||||
though c7935d2c was pending and assigned to Main PM. The filter
|
||||
missed the pre-assigned case — list_assigned_for_agent ordered by
|
||||
priority/updated_at and could rank pending below in_progress tasks;
|
||||
also, pm_give_me_work fell through to idle if all assigned tasks were
|
||||
pending and not yet distinguished from the triage queue.
|
||||
|
||||
Pre-assigned pending tasks must be returned FIRST by pm_give_me_work
|
||||
(and give_me_work for developer/QA/doc roles).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||
|
||||
|
||||
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
base = {
|
||||
"task": AsyncMock(),
|
||||
"work_session": AsyncMock(),
|
||||
"git": AsyncMock(),
|
||||
"a2a": AsyncMock(),
|
||||
"journal": AsyncMock(),
|
||||
"audit": AsyncMock(),
|
||||
"evidence_repo": AsyncMock(),
|
||||
}
|
||||
base.update(overrides)
|
||||
repo = base["evidence_repo"]
|
||||
for method in (
|
||||
"list_unread_a2a",
|
||||
"list_unread_mentions",
|
||||
"list_pending_notifications",
|
||||
"task_metadata_gaps",
|
||||
"recent_team_activity",
|
||||
"blockers_in_lane",
|
||||
"journal_highlights_for_task",
|
||||
):
|
||||
getattr(repo, method).return_value = []
|
||||
return ChoreographerDeps(**base)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pm_give_me_work_returns_pre_assigned_pending_main_pm() -> None:
|
||||
"""Main PM has a pending task assigned to them: pm_give_me_work returns it.
|
||||
|
||||
Smoke run 3: task c7935d2c was pending + assigned_to=main-pm but
|
||||
pm_give_me_work returned {status: idle, next: 'no Main PM work'}.
|
||||
"""
|
||||
pm_id = uuid4()
|
||||
task_id = uuid4()
|
||||
pending_task = MagicMock(
|
||||
id=task_id,
|
||||
status="pending",
|
||||
assigned_to=pm_id,
|
||||
task_type="planning",
|
||||
title="Main PM root task — pre-assigned pending",
|
||||
parent_task_id=None,
|
||||
sequence=0,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = [pending_task]
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.pm_give_me_work(pm_id)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] is None, f"Expected no error, got: {body.get('error')}"
|
||||
assert body["task_id"] == str(task_id), (
|
||||
f"Expected task {task_id}, got {body.get('task_id')} — "
|
||||
"pm_give_me_work did not return the pre-assigned pending task"
|
||||
)
|
||||
assert body["status"] == "pending"
|
||||
assert "i_will_plan" in body["next"], (
|
||||
f"Expected i_will_plan hint for PM pending task, got: {body.get('next')}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pm_give_me_work_returns_pre_assigned_pending_cell_pm() -> None:
|
||||
"""Cell PM has a pending task assigned to them: pm_give_me_work returns it."""
|
||||
pm_id = uuid4()
|
||||
task_id = uuid4()
|
||||
pending_task = MagicMock(
|
||||
id=task_id,
|
||||
status="pending",
|
||||
assigned_to=pm_id,
|
||||
task_type="planning",
|
||||
title="Cell PM task — pre-assigned pending",
|
||||
parent_task_id=None,
|
||||
sequence=0,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = [pending_task]
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.pm_give_me_work(pm_id)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] is None
|
||||
assert body["task_id"] == str(task_id)
|
||||
assert body["status"] == "pending"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pm_give_me_work_pending_beats_other_assigned() -> None:
|
||||
"""Pre-assigned pending takes priority over other non-pending assigned tasks."""
|
||||
pm_id = uuid4()
|
||||
pending_id = uuid4()
|
||||
other_id = uuid4()
|
||||
|
||||
pending_task = MagicMock(
|
||||
id=pending_id,
|
||||
status="pending",
|
||||
assigned_to=pm_id,
|
||||
task_type="planning",
|
||||
title="Pre-assigned pending — should win",
|
||||
parent_task_id=None,
|
||||
sequence=0,
|
||||
priority=5,
|
||||
)
|
||||
other_task = MagicMock(
|
||||
id=other_id,
|
||||
status="awaiting_pm_review",
|
||||
assigned_to=pm_id,
|
||||
task_type="planning",
|
||||
title="Awaiting review",
|
||||
parent_task_id=None,
|
||||
sequence=0,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = [pending_task]
|
||||
task_svc.list_assigned_for_agent.return_value = [other_task]
|
||||
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.pm_give_me_work(pm_id)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["task_id"] == str(pending_id), (
|
||||
f"Pre-assigned pending task should beat awaiting_pm_review; "
|
||||
f"got {body.get('task_id')}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_give_me_work_returns_pre_assigned_pending_developer() -> None:
|
||||
"""Developer has a pending task assigned: give_me_work returns it first."""
|
||||
dev_id = uuid4()
|
||||
task_id = uuid4()
|
||||
pending_task = MagicMock(
|
||||
id=task_id,
|
||||
status="pending",
|
||||
assigned_to=dev_id,
|
||||
task_type="code",
|
||||
title="Dev task — pre-assigned pending",
|
||||
parent_task_id=None,
|
||||
sequence=0,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = [pending_task]
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
task_svc.list_paused_for_agent.return_value = []
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer")
|
||||
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.give_me_work(dev_id)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] is None
|
||||
assert body["task_id"] == str(task_id)
|
||||
assert body["status"] == "pending"
|
||||
assert "i_will_work_on" in body["next"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pm_give_me_work_falls_through_to_assigned_when_no_pending() -> None:
|
||||
"""When no pre-assigned pending tasks, pm_give_me_work still checks assigned."""
|
||||
pm_id = uuid4()
|
||||
assigned_id = uuid4()
|
||||
assigned_task = MagicMock(
|
||||
id=assigned_id,
|
||||
status="awaiting_pm_review",
|
||||
assigned_to=pm_id,
|
||||
task_type="planning",
|
||||
title="Awaiting PM review task",
|
||||
parent_task_id=None,
|
||||
sequence=0,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = [assigned_task]
|
||||
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.pm_give_me_work(pm_id)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["task_id"] == str(assigned_id)
|
||||
assert body["status"] == "awaiting_pm_review"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pm_give_me_work_idle_when_no_pre_assigned_and_no_assigned() -> None:
|
||||
"""When no pending and no assigned tasks, pm_give_me_work returns idle."""
|
||||
pm_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.pm_give_me_work(pm_id)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["status"] == "idle"
|
||||
assert body["task_id"] is None
|
||||
@@ -186,6 +186,7 @@ async def test_pm_give_me_work_calls_heartbeat_when_returning_task() -> None:
|
||||
tid = uuid4()
|
||||
assigned = MagicMock(id=tid, status="pending")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = [assigned]
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
@@ -199,6 +200,7 @@ async def test_pm_give_me_work_calls_heartbeat_when_returning_task() -> None:
|
||||
async def test_pm_give_me_work_does_not_heartbeat_on_idle() -> None:
|
||||
pm_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_pending_for_agent.return_value = []
|
||||
task_svc.list_assigned_for_agent.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
Reference in New Issue
Block a user