mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
Renn F
ed828a719b
Plans were vague because the gate accepted the bare minimum: PM approach >=20 chars and title-only sub_tasks. minimax wrote exactly the minimum. - IWillPlanRequest.approach min_length 20 -> 150 (kept in sync with _PM_APPROACH_MIN_LEN; the gate enforces it at the choreographer layer too so direct/MCP callers can't bypass the HTTP boundary). - New _thin_subtask_hint: every PM sub_task must have a title and a description >= _PM_SUBTASK_DESC_MIN_LEN (60) saying what the step does — each sub_task is both a delegate target AND a progress-checklist item, so a title alone is not a plan. - cell_pm/main_pm role prompts: explicit "the gate REJECTS thin plans" framing + concrete sub_task example + the new minimums. - Updated all affected test fixtures across the suite to use substantive approaches/descriptions; added thin-sub_task rejection coverage. Commit 1 of 3 for the plan/progress quality work (#171/#172/#173).
262 lines
8.5 KiB
Python
262 lines
8.5 KiB
Python
"""Wave C4 (2026-05-12): claim auto-creates a WorkSession row.
|
|
|
|
Smoke run 3 showed task.work_session_id null on every task. Pre-gateway
|
|
created the row at claim time so the panel/PR/merge subsystems could
|
|
track agent-per-task git activity (branch, commits, PR number/url,
|
|
merge status). Restoring that side-effect.
|
|
|
|
The choreographer's _claim_plan_start_run (and _resume_from_claimed)
|
|
calls TaskService.ensure_work_session(task_id, agent_id) after the
|
|
task reaches in_progress, which creates the WorkSession and stores
|
|
its id on the task.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from unittest.mock import AsyncMock, MagicMock
|
|
from uuid import uuid4
|
|
|
|
import pytest
|
|
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
|
|
|
|
|
def _make_task_svc(agent_id, task_id, *, status: str):
|
|
"""Build a TaskService AsyncMock that completes the (claim, set_plan, start)
|
|
sequence and returns a task with branch_name set (as the real service does
|
|
after auto-creating the branch during claim side-effects).
|
|
"""
|
|
in_progress_task = MagicMock(
|
|
id=task_id,
|
|
status="in_progress",
|
|
plan={"text": "plan text"},
|
|
assigned_to=agent_id,
|
|
branch_name="feature/backend/abc",
|
|
work_session_id=None,
|
|
commits=[],
|
|
pr_number=None,
|
|
quick_context=None,
|
|
team="backend",
|
|
task_type="code",
|
|
parent_task_id=None,
|
|
sequence=0,
|
|
project_id=uuid4(),
|
|
)
|
|
task_svc = AsyncMock()
|
|
task_svc.get.return_value = MagicMock(
|
|
id=task_id,
|
|
status=status,
|
|
plan=None,
|
|
assigned_to=None,
|
|
branch_name="feature/backend/abc",
|
|
work_session_id=None,
|
|
commits=[],
|
|
pr_number=None,
|
|
quick_context=None,
|
|
team="backend",
|
|
task_type="code",
|
|
parent_task_id=None,
|
|
sequence=0,
|
|
project_id=uuid4(),
|
|
)
|
|
task_svc.agent_for.return_value = MagicMock(
|
|
id=agent_id, role="developer", team="backend", slug=None
|
|
)
|
|
task_svc.list_in_progress_for_agent.return_value = []
|
|
task_svc.list_paused_for_agent.return_value = []
|
|
task_svc.get_subtasks.return_value = []
|
|
task_svc.claim.return_value = MagicMock(
|
|
id=task_id, status="claimed", plan=None, assigned_to=agent_id
|
|
)
|
|
task_svc.set_plan.return_value = MagicMock(
|
|
id=task_id, status="claimed", plan={"text": "plan text"}, assigned_to=agent_id
|
|
)
|
|
task_svc.start.return_value = in_progress_task
|
|
task_svc.ensure_work_session.return_value = None
|
|
task_svc.session = MagicMock()
|
|
task_svc.session.begin_nested = MagicMock(
|
|
return_value=MagicMock(
|
|
__aenter__=AsyncMock(return_value=None),
|
|
__aexit__=AsyncMock(return_value=False),
|
|
)
|
|
)
|
|
return task_svc
|
|
|
|
|
|
def _make_deps(task_svc) -> ChoreographerDeps:
|
|
evidence_repo = AsyncMock()
|
|
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(evidence_repo, method).return_value = []
|
|
return ChoreographerDeps(
|
|
task=task_svc,
|
|
work_session=AsyncMock(),
|
|
git=AsyncMock(),
|
|
a2a=AsyncMock(),
|
|
journal=AsyncMock(),
|
|
audit=AsyncMock(),
|
|
evidence_repo=evidence_repo,
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_i_will_work_on_calls_ensure_work_session() -> None:
|
|
"""After a successful i_will_work_on, TaskService.ensure_work_session is
|
|
called once with (task_id, agent_id) so a WorkSession row is created and
|
|
task.work_session_id is populated.
|
|
|
|
Wave C4 (2026-05-12): pre-gateway parity. Smoke run 3 showed
|
|
task.work_session_id null on every in_progress task.
|
|
"""
|
|
agent_id = uuid4()
|
|
task_id = uuid4()
|
|
task_svc = _make_task_svc(agent_id, task_id, status="pending")
|
|
deps = _make_deps(task_svc)
|
|
c = Choreographer(deps)
|
|
|
|
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y")
|
|
|
|
assert env.error is None, f"Expected ok, got error={env.error} msg={env.message}"
|
|
assert env.status == "in_progress"
|
|
task_svc.ensure_work_session.assert_awaited_once_with(task_id, agent_id)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_i_will_plan_calls_ensure_work_session() -> None:
|
|
"""PMs also get a WorkSession via ensure_work_session (cell_pm role, planning
|
|
task). Both i_will_work_on and i_will_plan share _claim_plan_start_run so
|
|
the same hook fires for both.
|
|
|
|
Wave C4 (2026-05-12): pre-gateway parity.
|
|
"""
|
|
pm_agent_id = uuid4()
|
|
task_id = uuid4()
|
|
|
|
in_progress_task = MagicMock(
|
|
id=task_id,
|
|
status="in_progress",
|
|
plan={"approach": "plan text", "sub_tasks": ["t1"]},
|
|
assigned_to=pm_agent_id,
|
|
branch_name="feature/main_pm/abc",
|
|
work_session_id=None,
|
|
commits=[],
|
|
pr_number=None,
|
|
quick_context=None,
|
|
team="main_pm",
|
|
task_type="planning",
|
|
parent_task_id=None,
|
|
sequence=0,
|
|
project_id=uuid4(),
|
|
)
|
|
task_svc = AsyncMock()
|
|
task_svc.get.return_value = MagicMock(
|
|
id=task_id,
|
|
status="pending",
|
|
plan=None,
|
|
assigned_to=None,
|
|
branch_name="feature/main_pm/abc",
|
|
work_session_id=None,
|
|
commits=[],
|
|
pr_number=None,
|
|
quick_context=None,
|
|
team="main_pm",
|
|
task_type="planning",
|
|
parent_task_id=None,
|
|
sequence=0,
|
|
project_id=uuid4(),
|
|
)
|
|
task_svc.agent_for.return_value = MagicMock(
|
|
id=pm_agent_id, role="cell_pm", team="backend", slug=None
|
|
)
|
|
task_svc.list_in_progress_for_agent.return_value = []
|
|
task_svc.list_paused_for_agent.return_value = []
|
|
task_svc.get_subtasks.return_value = []
|
|
task_svc.claim.return_value = MagicMock(
|
|
id=task_id, status="claimed", plan=None, assigned_to=pm_agent_id
|
|
)
|
|
task_svc.set_plan.return_value = MagicMock(
|
|
id=task_id,
|
|
status="claimed",
|
|
plan={"approach": "plan text"},
|
|
assigned_to=pm_agent_id,
|
|
)
|
|
task_svc.start.return_value = in_progress_task
|
|
task_svc.ensure_work_session.return_value = None
|
|
task_svc.session = MagicMock()
|
|
task_svc.session.begin_nested = MagicMock(
|
|
return_value=MagicMock(
|
|
__aenter__=AsyncMock(return_value=None),
|
|
__aexit__=AsyncMock(return_value=False),
|
|
)
|
|
)
|
|
evidence_repo = AsyncMock()
|
|
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(evidence_repo, method).return_value = []
|
|
deps = ChoreographerDeps(
|
|
task=task_svc,
|
|
work_session=AsyncMock(),
|
|
git=AsyncMock(),
|
|
a2a=AsyncMock(),
|
|
journal=AsyncMock(),
|
|
audit=AsyncMock(),
|
|
evidence_repo=evidence_repo,
|
|
)
|
|
c = Choreographer(deps)
|
|
|
|
rich_plan = {
|
|
"approach": (
|
|
"Single-cell decomposition: be-dev-1 owns the change end to end "
|
|
"— branch, implement, test, open PR; QA reviews after the PR "
|
|
"opens, documentation follows, then be-pm completes and submits "
|
|
"up. No cross-cell dependencies for this planning task."
|
|
),
|
|
"sub_tasks": [
|
|
{
|
|
"title": "t1",
|
|
"description": (
|
|
"be-dev-1 implements the change with tests and opens the "
|
|
"leaf PR for QA review."
|
|
),
|
|
}
|
|
],
|
|
}
|
|
env = await c.i_will_plan(
|
|
pm_agent_id, task_id, plan="plan text", rich_plan=rich_plan
|
|
)
|
|
|
|
assert env.error is None, f"Expected ok, got error={env.error} msg={env.message}"
|
|
assert env.status == "in_progress"
|
|
task_svc.ensure_work_session.assert_awaited_once_with(task_id, pm_agent_id)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_ensure_work_session_not_called_when_start_fails() -> None:
|
|
"""If start() returns None (task in wrong state), ensure_work_session must
|
|
NOT be called — the WorkSession must not be created for a failed transition.
|
|
"""
|
|
agent_id = uuid4()
|
|
task_id = uuid4()
|
|
task_svc = _make_task_svc(agent_id, task_id, status="pending")
|
|
task_svc.start.return_value = None # start fails
|
|
deps = _make_deps(task_svc)
|
|
c = Choreographer(deps)
|
|
|
|
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y")
|
|
|
|
assert env.error == "invalid_state"
|
|
task_svc.ensure_work_session.assert_not_awaited()
|