mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(gateway): restore Gate Set C exit-time guards
Choreographer.i_am_idle now refuses with INVALID_STATE when the caller has any pending (assigned but never claimed) task. Pre-gateway this was implicit because the orchestrator's auto-respawn would re-spawn the agent for the assignment, leading to a tight respawn loop. The explicit refusal lets the agent fix the state via i_will_work_on (dev/qa/doc) or i_will_plan (pm) first. Existing auto-pause for in_progress tasks is preserved (Gate Set C spec calls this out as still required) — it runs AFTER the pending guard, so an agent with a mix of pending+in_progress is told about the pending task first instead of silently pausing in_progress and then looping on the pending one. Pre-gateway reference: roboco/runtime/orchestrator.py auto-respawn loop guards (already preserved at HEAD); the explicit agent-facing gate is new. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
466cc8d8f7
commit
197b0f22dc
@@ -480,10 +480,18 @@ class Choreographer:
|
|||||||
async def i_am_idle(self, agent_id: UUID) -> Envelope:
|
async def i_am_idle(self, agent_id: UUID) -> Envelope:
|
||||||
"""Report no more work. Soft-block if there are unread A2As or @mentions.
|
"""Report no more work. Soft-block if there are unread A2As or @mentions.
|
||||||
|
|
||||||
Before marking the agent idle, auto-pause every in_progress task this
|
Before marking the agent idle:
|
||||||
agent owns so the orchestrator's PM-closure dispatcher can wake them
|
|
||||||
when subtasks finish, instead of leaving the parent stuck at
|
1. Bail with ``idle_with_unread`` when context_briefing has unread A2A
|
||||||
``in_progress`` forever.
|
or @mentions (must address those first).
|
||||||
|
2. Refuse with INVALID_STATE if the agent has any pending tasks
|
||||||
|
assigned but never claimed — they must call i_will_work_on (dev/qa/
|
||||||
|
doc) or i_will_plan (pm) first. (Gate Set C, pre-gateway implicit
|
||||||
|
via the orchestrator's auto-respawn.)
|
||||||
|
3. Auto-pause every in_progress task this agent owns so the
|
||||||
|
orchestrator's PM-closure dispatcher can wake them when subtasks
|
||||||
|
finish, instead of leaving the parent stuck at ``in_progress``
|
||||||
|
forever.
|
||||||
"""
|
"""
|
||||||
briefing = await self._briefing_for(agent_id, None)
|
briefing = await self._briefing_for(agent_id, None)
|
||||||
if briefing.get("unread_a2a") or briefing.get("unread_mentions"):
|
if briefing.get("unread_a2a") or briefing.get("unread_mentions"):
|
||||||
@@ -496,6 +504,8 @@ class Choreographer:
|
|||||||
),
|
),
|
||||||
context_briefing=briefing,
|
context_briefing=briefing,
|
||||||
)
|
)
|
||||||
|
if guard := await self._pending_assignment_guard(agent_id, briefing):
|
||||||
|
return guard
|
||||||
await self._auto_pause_in_progress_tasks(agent_id)
|
await self._auto_pause_in_progress_tasks(agent_id)
|
||||||
await self.task.mark_agent_idle(agent_id)
|
await self.task.mark_agent_idle(agent_id)
|
||||||
return Envelope.ok(
|
return Envelope.ok(
|
||||||
@@ -505,6 +515,37 @@ class Choreographer:
|
|||||||
context_briefing=briefing,
|
context_briefing=briefing,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def _pending_assignment_guard(
|
||||||
|
self, agent_id: UUID, briefing: dict[str, Any]
|
||||||
|
) -> Envelope | None:
|
||||||
|
"""Refuse i_am_idle when caller owns any pending (unclaimed) task.
|
||||||
|
|
||||||
|
Pre-gateway: the orchestrator would respawn the agent after
|
||||||
|
i_am_idle if they still owned pending work, leading to a tight
|
||||||
|
respawn loop. Now an explicit refusal lets the agent fix it via
|
||||||
|
i_will_work_on or i_will_plan before exiting.
|
||||||
|
"""
|
||||||
|
assigned = await self.task.list_assigned_for_agent(agent_id)
|
||||||
|
pending = [t for t in assigned if str(t.status) == "pending"]
|
||||||
|
if not pending:
|
||||||
|
return None
|
||||||
|
first = pending[0]
|
||||||
|
agent = await self.task.agent_for(agent_id)
|
||||||
|
verb = "i_will_plan" if agent and agent.role in ("cell_pm", "main_pm") else (
|
||||||
|
"i_will_work_on"
|
||||||
|
)
|
||||||
|
return Envelope.invalid_state(
|
||||||
|
message=(
|
||||||
|
f"You have task {first.id} assigned but never claimed; "
|
||||||
|
"cannot idle until claimed or unclaimed."
|
||||||
|
),
|
||||||
|
remediate=(
|
||||||
|
f"call {verb}(task_id='{first.id}') to start work, or"
|
||||||
|
" unclaim it first; then retry i_am_idle"
|
||||||
|
),
|
||||||
|
context_briefing=briefing,
|
||||||
|
)
|
||||||
|
|
||||||
async def _auto_pause_in_progress_tasks(self, agent_id: UUID) -> None:
|
async def _auto_pause_in_progress_tasks(self, agent_id: UUID) -> None:
|
||||||
"""Pause every in_progress task assigned to this agent.
|
"""Pause every in_progress task assigned to this agent.
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,161 @@
|
|||||||
|
"""Gate Set C: exit-time guards in Choreographer.i_am_idle.
|
||||||
|
|
||||||
|
Pre-gateway behavior: an agent that signaled idle while owning unclaimed
|
||||||
|
pending work was implicitly stuck because the orchestrator's PM-closure
|
||||||
|
dispatcher would respawn them. The gateway makes this explicit:
|
||||||
|
|
||||||
|
- pending-parent guard: refuse i_am_idle if caller has any pending task
|
||||||
|
assigned. They must call i_will_work_on / i_will_plan first.
|
||||||
|
- in_progress preserved: existing auto-pause for in_progress remains.
|
||||||
|
"""
|
||||||
|
|
||||||
|
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: dict[str, Any] = {
|
||||||
|
"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_i_am_idle_refuses_when_pending_assignment_exists() -> None:
|
||||||
|
"""An agent with a pending (unclaimed) task assigned cannot exit."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
pending_id = uuid4()
|
||||||
|
pending = MagicMock(id=pending_id, status="pending")
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.list_assigned_for_agent.return_value = [pending]
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = []
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.i_am_idle(agent_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] == "invalid_state"
|
||||||
|
assert str(pending_id) in body["message"] or str(pending_id) in body["remediate"]
|
||||||
|
assert "i_will_work_on" in body["remediate"] or "i_will_plan" in body["remediate"]
|
||||||
|
task_svc.mark_agent_idle.assert_not_awaited()
|
||||||
|
task_svc.pause_for_agent.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_i_am_idle_lets_pm_through_with_pending_remediate() -> None:
|
||||||
|
"""The pending guard recommends i_will_plan when the agent is a PM."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
pending_id = uuid4()
|
||||||
|
pending = MagicMock(id=pending_id, status="pending")
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.list_assigned_for_agent.return_value = [pending]
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = []
|
||||||
|
task_svc.agent_for.return_value = MagicMock(role="cell_pm", team="backend")
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.i_am_idle(agent_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] == "invalid_state"
|
||||||
|
assert "i_will_plan" in body["remediate"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_i_am_idle_lets_dev_through_with_pending_remediate() -> None:
|
||||||
|
"""The pending guard recommends i_will_work_on for a developer."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
pending_id = uuid4()
|
||||||
|
pending = MagicMock(id=pending_id, status="pending")
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.list_assigned_for_agent.return_value = [pending]
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = []
|
||||||
|
task_svc.agent_for.return_value = MagicMock(role="developer", team="backend")
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.i_am_idle(agent_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] == "invalid_state"
|
||||||
|
assert "i_will_work_on" in body["remediate"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_i_am_idle_ignores_non_pending_assigned_tasks() -> None:
|
||||||
|
"""Other active states (claimed/in_progress/etc) don't trigger pending guard."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
in_progress = MagicMock(id=uuid4(), status="in_progress")
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
# Even though list_assigned_for_agent includes in_progress, only pending
|
||||||
|
# triggers the guard.
|
||||||
|
task_svc.list_assigned_for_agent.return_value = [in_progress]
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = [in_progress]
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.i_am_idle(agent_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] is None
|
||||||
|
# The auto-pause path should still fire for in_progress.
|
||||||
|
task_svc.pause_for_agent.assert_awaited_once_with(agent_id, in_progress.id)
|
||||||
|
task_svc.mark_agent_idle.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_i_am_idle_with_no_assigned_tasks_proceeds_normally() -> None:
|
||||||
|
"""Empty assignment list: no pending guard, no auto-pause, idle goes through."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.list_assigned_for_agent.return_value = []
|
||||||
|
task_svc.list_in_progress_for_agent.return_value = []
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.i_am_idle(agent_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
assert body["error"] is None
|
||||||
|
assert body["status"] == "idle"
|
||||||
|
task_svc.pause_for_agent.assert_not_awaited()
|
||||||
|
task_svc.mark_agent_idle.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_i_am_idle_pending_guard_runs_after_unread_check() -> None:
|
||||||
|
"""Unread A2A check still wins; pending guard only runs after."""
|
||||||
|
agent_id = uuid4()
|
||||||
|
pending = MagicMock(id=uuid4(), status="pending")
|
||||||
|
task_svc = AsyncMock()
|
||||||
|
task_svc.list_assigned_for_agent.return_value = [pending]
|
||||||
|
deps = _make_deps(task=task_svc)
|
||||||
|
deps.evidence_repo.list_unread_a2a.return_value = ["mention"]
|
||||||
|
c = Choreographer(deps)
|
||||||
|
|
||||||
|
env = await c.i_am_idle(agent_id)
|
||||||
|
body = env.as_dict()
|
||||||
|
# Unread takes precedence and returns ok+idle_with_unread, NOT invalid_state
|
||||||
|
assert body["error"] is None
|
||||||
|
assert body["status"] == "idle_with_unread"
|
||||||
|
task_svc.mark_agent_idle.assert_not_awaited()
|
||||||
Reference in New Issue
Block a user