External/roboco
1
0
Fork 0
mirror of https://github.com/rennf93/roboco.git synced 2026-08-03 07:23:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

feat(gateway): developer i_will_work_on takes a substantive step checklist (#172)

Browse Source 
The dev plan was a free string with only a presence gate, so the
executing dev had no checklist for plan-driven progress (#173).

- IWillWorkOnRequest gains `steps` (same SubTask shape as a PM's
  sub_tasks); flow_dev route threads it through.
- i_will_work_on layers steps onto the narrative plan via the same
  panel-shaped path PMs use, so task.plan.sub_tasks is populated
  (panel render + #173 progress).
- New _dev_steps_gate (mirrors _pm_sub_tasks_gate, runs after the spec
  gate): a developer FRESH claim must supply a non-empty steps list
  with every description >= _PM_SUBTASK_DESC_MIN_LEN. Re-entry/recovery
  short-circuit before the gate (extracted _dev_reentry +
  _fresh_dev_claim keep i_will_work_on within the
  return-count + cyclomatic gates).
- developer role prompt: steps template + "thin steps rejected" + the
  progress(plan_step=...) handoff.
- Updated every dev-fresh-claim test fixture across the suite to pass
  substantive steps; added dedicated _dev_steps_gate coverage.

Commit 2 of 3 for the plan/progress quality work (#171/#172/#173).
This commit is contained in:
Renn F
2026-05-16 10:48:23 +02:00
parent ed828a719b
commit 4c397e1768
13 changed files with 458 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agents/prompts/roles/developer.md
+1 -1
View File
@@ -18,7 +18,7 @@ You write code; you do not coordinate. If you find yourself thinking "let me als
| Verb | What it does | Preconditions |
|---|---|---|
| `give_me_work()` | Returns your highest-priority task or `idle`. | None. |
| `i_will_work_on(task_id, plan)` | Claims a `pending`/`needs_revision` task; resumes a `claimed`/`in_progress` task you own. Auto-creates branch on first claim. **`plan` is REQUIRED** — even on first claim. The gateway returns `tracing_gap missing=['plan']` if you call without it. On resume, pass `plan='resume: <next step>'`. | Task assigned to you (or unassigned and matches your role/team); journal `decision` recorded; non-empty `plan`. |
| `i_will_work_on(task_id, plan, steps)` | Claims a `pending`/`needs_revision` task; resumes a `claimed`/`in_progress` task you own. Auto-creates branch on first claim. **`plan` is REQUIRED** (narrative; `tracing_gap missing=['plan']` if absent). **On a FRESH claim `steps` is REQUIRED and gated** — a non-empty list of `{title, description}` where every `description` is **≥60 chars** saying what that step actually does. `steps` is your execution checklist AND your progress checklist: as you finish each, call `progress(task_id, plan_step=<step id/order>, message=...)` and the % is computed from the checklist for you. Thin/title-only steps are rejected. Example step: `{"title": "Edit README", "description": "prepend the smoke-test HTML comment above the H1, leaving the rest of the file untouched"}`. **On resume** (`claimed`/`in_progress` you own) pass `plan='resume: <next step>'`; steps are not re-required. | Task assigned to you (or unassigned and matches your role/team); journal `decision` recorded; non-empty `plan`; substantive `steps` on fresh claim. |
| `commit(message)` | Makes the git commit, auto-prefixes `[task-id]`, records a progress entry. This is the ONLY way to commit — the gateway covers the actual git operation. | Task in `in_progress`; on your branch. |
| `open_pr(task_id)` | Push your branch and open a PR. Run after your last commit, before `i_am_done`. `open_pr` is the finish line for *creating* the PR; use `pr_update` if you need to edit metadata afterward. | Task assigned to you; at least one commit; no PR yet. |
| `pr_update(task_id, title?, body?, reviewers?)` | Update an existing PR's title, body, or reviewer list. Use after `open_pr` if you need to correct title/body or assign a reviewer. At least one field must be set. **Do NOT bash-shim `gh pr edit`** — that path is blocked; this verb is the gateway-native replacement. | Task has `pr_number`; you are the assignee (or your PM). |
roboco/api/routes/v2/flow_dev.py
+3 -1
View File
@@ -48,7 +48,9 @@ async def i_will_work_on(
x_agent_id: _AgentIdHeader,
choreographer: _ChoreographerDep,
) -> dict:
env = await choreographer.i_will_work_on(x_agent_id, body.task_id, body.plan)
env = await choreographer.i_will_work_on(
x_agent_id, body.task_id, body.plan, steps=body.steps
)
return envelope_to_response(env, request)
roboco/api/schemas/v2/flow.py
+10
View File
@@ -12,6 +12,16 @@ class GiveMeWorkRequest(BaseModel):
class IWillWorkOnRequest(BaseModel):
task_id: UUID
plan: str | None = None
# #172: the executing developer's plan is a step checklist (same
# SubTask shape as IWillPlanRequest.sub_tasks). It is both the
# execution plan AND the progress checklist (#173): completing a
# step advances progress. Depth is enforced server-side in
# choreographer._dev_steps_gate (a title with no real description is
# not a step). Server assigns id + order.
steps: list[dict[str, str]] = Field(
default_factory=list,
description="Ordered execution steps — list of {title, description}",
)
class OpenPrRequest(BaseModel):
roboco/services/gateway/choreographer/_impl.py
+143 -13
View File
@@ -959,7 +959,11 @@ class Choreographer:
).with_introspection(task=t, role=role_str)
async def i_will_work_on(
self, agent_id: UUID, task_id: UUID, plan: str | None = None
self,
agent_id: UUID,
task_id: UUID,
plan: str | None = None,
steps: list[dict[str, Any]] | None = None,
) -> Envelope:
"""Claim a task and start work on it.
@@ -968,6 +972,15 @@ class Choreographer:
savepoint by the runner so a mid-sequence failure rolls back
the DB. Idempotent re-entry: a respawned dev re-calling on a
task they already own in_progress just refreshes the heartbeat.
#172: ``steps`` is the developer's execution checklist (same
SubTask shape as a PM's sub_tasks). Persisted into
``task.plan.sub_tasks`` via the panel-shaped path so it renders
identically AND feeds plan-driven progress (#173). A developer
on a fresh claim must supply substantive steps —
``_dev_steps_gate`` enforces depth; the re-entry / recovery
paths short-circuit before the gate so a respawned dev is never
re-blocked for steps it already submitted.
"""
t = await self.task.get(task_id)
if t is None:
@@ -993,8 +1006,17 @@ class Choreographer:
task_id=task_id,
verb="i_will_work_on",
)
# #172: layer the step checklist onto the narrative plan via the
# same panel-shaped path PMs use, so task.plan.sub_tasks is
# populated (panel render + #173 progress). No steps → unchanged
# string behaviour.
effective_plan: str | dict[str, Any] | None = plan
if steps:
effective_plan = self._resolve_effective_plan(
plan or "", {"sub_tasks": steps}
)
spec_ctx = spec_module.Context(
plan=plan,
plan=effective_plan,
actor_id=agent_id,
actor_slug=getattr(agent, "slug", None) if agent is not None else None,
original_developer_slug=_extract_original_developer(t),
@@ -1005,13 +1027,34 @@ class Choreographer:
task=t,
role_str=role_str,
briefing=briefing,
plan=plan,
plan=effective_plan,
verb_name="i_will_work_on",
)
if reentry := await self._dev_reentry(
ctx, t, agent_id, task_id, role_str, briefing
):
return reentry
return await self._fresh_dev_claim(
ctx, role, spec_ctx, agent, steps, role_str, t, agent_id, task_id, briefing
)
async def _dev_reentry(
self,
ctx: _ClaimPlanStartContext,
t: Any,
agent_id: UUID,
task_id: UUID,
role_str: str,
briefing: dict[str, Any],
) -> Envelope | None:
"""Re-entry short-circuits for i_will_work_on (extracted to keep
i_will_work_on under the cyclomatic-complexity gate; mirrors
_handle_pm_reentry). Returns an Envelope to short-circuit, or None
to fall through to the fresh-claim path.
"""
# Idempotent re-entry: agent already owns the task in_progress.
# Touch heartbeat and short-circuit before the spec gate (which
# would otherwise reject because in_progress is not a source state
# for the composed `claim` action).
# Short-circuit before the spec gate (in_progress is not a source
# state for the composed `claim` action).
if str(t.status) == "in_progress" and t.assigned_to == agent_id:
await self._touch(task_id)
return Envelope.ok(
@@ -1020,25 +1063,112 @@ class Choreographer:
next=spec_module._INTENT_VERBS["i_will_work_on"].next_hint(t),
context_briefing=briefing,
).with_introspection(task=t, role=role_str)
# Recovery re-entry: task stuck in `claimed` (e.g. orchestrator restart
# or a partial-claim race) and the agent already owns it. The spec
# `claim` action's source-statuses do NOT include CLAIMED, so the spec
# gate would reject. Surface this as a runner call that runs only
# set_plan + start. Without this block, an agent reclaiming from a
# crashed mid-sequence would loop forever (Bug A from the 2026-05-09
# smoke test).
# Recovery re-entry: task stuck in `claimed` (orchestrator restart
# or partial-claim race) and the agent already owns it. The spec
# `claim` source-statuses exclude CLAIMED, so run only set_plan +
# start (Bug A from the 2026-05-09 smoke test).
if str(t.status) == "claimed" and t.assigned_to == agent_id:
envelope = await self._resume_from_claimed(ctx)
return await self._post_claim_journal_gate(
"i_will_work_on", agent_id, task_id, envelope
)
return None
async def _fresh_dev_claim(
self,
ctx: _ClaimPlanStartContext,
role: Any,
spec_ctx: Any,
agent: Any,
steps: list[dict[str, Any]] | None,
role_str: str,
t: Any,
agent_id: UUID,
task_id: UUID,
briefing: dict[str, Any],
) -> Envelope:
"""Fresh (non-re-entry) i_will_work_on tail: spec gate → dev-steps
gate → claim/plan/start → post-claim journal gate. Extracted so
i_will_work_on stays within the return-count budget; the dev-steps
gate mirrors _pm_sub_tasks_gate's placement (after the spec gate).
"""
if rejection := await self._claim_plan_start_gate(ctx, role, spec_ctx):
return rejection
if rejection := await self._dev_steps_gate(
role_str=role_str,
steps=steps,
task=t,
agent_id=agent_id,
task_id=task_id,
briefing=briefing,
):
return rejection
envelope = await self._claim_plan_start_run(ctx, agent, spec_ctx)
return await self._post_claim_journal_gate(
"i_will_work_on", agent_id, task_id, envelope
)
async def _dev_steps_gate(
self,
*,
role_str: str,
steps: list[dict[str, Any]] | None,
task: Any,
agent_id: UUID,
task_id: UUID,
briefing: dict[str, Any],
) -> Envelope | None:
"""#172: a developer's fresh claim must carry a substantive step
checklist — it is the execution plan AND the progress checklist
(#173). Non-developer callers and re-entry are unaffected (the
re-entry/recovery paths return before this is reached). Returns a
rejection Envelope when steps are absent/thin; None when the gate
passed.
"""
if role_str != "developer":
return None
if not steps:
return await self._emit_rejection(
Envelope.incomplete_input(
missing=["steps"],
field_hints={
"steps": (
"developers must plan their work as a step "
"checklist — a non-empty list of "
"{title, description}. Each step is both your "
"execution plan and a progress-checklist item."
)
},
remediate=(
"re-issue i_will_work_on(task_id, plan, "
"steps=[{'title': '...', 'description': '...'}, ...]) "
f"with every description >= {_PM_SUBTASK_DESC_MIN_LEN} "
"chars saying what that step does."
),
context_briefing=briefing,
).with_introspection(task=task, role=role_str),
agent_id=agent_id,
task_id=task_id,
verb="i_will_work_on",
)
if thin := _thin_subtask_hint(steps):
return await self._emit_rejection(
Envelope.incomplete_input(
missing=["steps"],
field_hints={"steps": thin},
remediate=(
"re-issue i_will_work_on with every step description "
f">= {_PM_SUBTASK_DESC_MIN_LEN} chars describing what "
"that step actually does."
),
context_briefing=briefing,
).with_introspection(task=task, role=role_str),
agent_id=agent_id,
task_id=task_id,
verb="i_will_work_on",
)
return None
@staticmethod
def _with_briefing(env: Envelope, briefing: dict[str, Any]) -> Envelope:
"""Attach a context_briefing to an Envelope (mutate-and-return helper)."""
tests/integration/test_full_lifecycle_real_db.py
+18 -3
View File
@@ -41,6 +41,17 @@ from roboco.models.base import (
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
from roboco.services.task import TaskService
# #172: a developer fresh claim must carry a substantive step checklist.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
if TYPE_CHECKING:
from collections.abc import AsyncIterator
@@ -318,7 +329,9 @@ async def test_dev_can_claim_pending_task_via_gateway(
)
c = Choreographer(deps)
env = await c.i_will_work_on(dev_agent.id, task.id, plan="add the route")
env = await c.i_will_work_on(
dev_agent.id, task.id, plan="add the route", steps=_STEPS
)
assert env.error is None, f"claim failed: {env.message}"
assert env.status == "in_progress"
@@ -360,7 +373,9 @@ async def test_dev_full_chain_through_awaiting_qa(
c = Choreographer(deps)
# 1. Claim
env = await c.i_will_work_on(dev_agent.id, task.id, plan="add the route")
env = await c.i_will_work_on(
dev_agent.id, task.id, plan="add the route", steps=_STEPS
)
assert env.error is None
assert env.status == "in_progress"
@@ -424,7 +439,7 @@ async def test_full_chain_through_doc_handoff(
c = Choreographer(deps)
# Drive the dev side first (same as test_dev_full_chain_through_awaiting_qa).
await c.i_will_work_on(dev_agent.id, task.id, plan="add the route")
await c.i_will_work_on(dev_agent.id, task.id, plan="add the route", steps=_STEPS)
await stub_git.commit(
branch_name=_BRANCH,
message=f"[{str(task.id)[:8]}] feat(api): add /healthz",
tests/integration/test_lifecycle_real_db.py
+20 -3
View File
@@ -39,6 +39,17 @@ from roboco.services.gateway.choreographer import Choreographer, ChoreographerDe
from roboco.services.task import TaskService
from sqlalchemy import delete
# #172: a developer fresh claim must carry a substantive step checklist.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
if TYPE_CHECKING:
from collections.abc import AsyncIterator
@@ -387,7 +398,9 @@ async def test_dev_full_chain_through_awaiting_qa(
)
c = Choreographer(deps)
env = await c.i_will_work_on(dev_agent.id, task.id, plan="add the route")
env = await c.i_will_work_on(
dev_agent.id, task.id, plan="add the route", steps=_STEPS
)
assert env.error is None, f"i_will_work_on failed: {env.message}"
assert env.status == Status.IN_PROGRESS.value
@@ -753,7 +766,9 @@ async def test_block_then_unblock_restore(
c = _build_choreographer(db_session, task, task_service)
# Drive into in_progress via the real claim+start sequence.
env = await c.i_will_work_on(dev_agent.id, task.id, plan="implement /healthz")
env = await c.i_will_work_on(
dev_agent.id, task.id, plan="implement /healthz", steps=_STEPS
)
assert env.error is None, f"i_will_work_on failed: {env.message}"
assert env.status == Status.IN_PROGRESS.value
@@ -807,7 +822,9 @@ async def test_pause_then_resume(
task_service = TaskService(db_session)
c = _build_choreographer(db_session, task, task_service)
env = await c.i_will_work_on(dev_agent.id, task.id, plan="implement /healthz")
env = await c.i_will_work_on(
dev_agent.id, task.id, plan="implement /healthz", steps=_STEPS
)
assert env.error is None, f"i_will_work_on failed: {env.message}"
assert env.status == Status.IN_PROGRESS.value
tests/integration/v2/test_full_pending_to_completed.py
+1
View File
@@ -62,6 +62,7 @@ class _MockChoreographer:
_agent_id: object,
_task_id: object,
_plan: object = None,
**_kwargs: object,
) -> Envelope:
self._state["task_status"] = "in_progress"
return Envelope.ok(
tests/unit/gateway/test_choreographer_claim_guards.py
+22 -10
View File
@@ -22,6 +22,18 @@ from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
# #172: a developer fresh claim must carry a substantive step checklist.
# Inert on re-entry/error/non-dev paths, so safe to pass everywhere.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
def _make_deps(**overrides: Any) -> ChoreographerDeps:
base: dict[str, Any] = {
@@ -132,7 +144,7 @@ async def test_i_will_work_on_blocks_when_earlier_sibling_open() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, target_id, plan="x")
env = await c.i_will_work_on(agent_id, target_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "sequence" in body["message"].lower()
@@ -177,7 +189,7 @@ async def test_i_will_work_on_allows_when_earlier_sibling_terminal() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, target_id)
env = await c.i_will_work_on(agent_id, target_id, steps=_STEPS)
assert env.error is None
task_svc.claim.assert_awaited_once_with(target_id, agent_id)
@@ -207,7 +219,7 @@ async def test_root_task_no_sequence_check() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, target_id)
env = await c.i_will_work_on(agent_id, target_id, steps=_STEPS)
assert env.error is None
# Sequence check should not have queried siblings on a root task
task_svc.get_subtasks.assert_not_awaited()
@@ -238,7 +250,7 @@ async def test_i_will_work_on_blocks_when_agent_has_in_progress_task() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, target_id, plan="x")
env = await c.i_will_work_on(agent_id, target_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert str(other_id) in body["message"] or str(other_id) in body["remediate"]
@@ -272,7 +284,7 @@ async def test_i_will_work_on_resumption_does_not_self_block() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id)
env = await c.i_will_work_on(agent_id, task_id, steps=_STEPS)
assert env.error is None
task_svc.start.assert_awaited_once_with(task_id, agent_id)
@@ -302,7 +314,7 @@ async def test_i_will_work_on_blocks_when_agent_has_paused_task() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, target_id, plan="x")
env = await c.i_will_work_on(agent_id, target_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert str(paused_id) in body["remediate"]
@@ -333,7 +345,7 @@ async def test_cell_pm_cannot_claim_code_task_via_i_will_work_on() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(pm_id, task_id, plan="x")
env = await c.i_will_work_on(pm_id, task_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "not_authorized"
# Spec produces "role 'cell_pm' may not call 'i_will_work_on'".
@@ -360,7 +372,7 @@ async def test_main_pm_cannot_claim_code_task_via_i_will_work_on() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(pm_id, task_id, plan="x")
env = await c.i_will_work_on(pm_id, task_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "not_authorized"
@@ -510,7 +522,7 @@ async def test_developer_cannot_claim_qa_status_task() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(dev_id, task_id)
env = await c.i_will_work_on(dev_id, task_id, steps=_STEPS)
body = env.as_dict()
assert body["error"] == "not_authorized"
assert "developer" in body["message"]
@@ -601,7 +613,7 @@ async def test_non_developer_role_cannot_claim_via_i_will_work_on() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(doc_id, task_id, plan="x")
env = await c.i_will_work_on(doc_id, task_id, plan="x", steps=_STEPS)
body = env.as_dict()
# Role-typed claim refuses with not_authorized
assert body["error"] == "not_authorized"
tests/unit/gateway/test_choreographer_dev.py
+14 -3
View File
@@ -9,6 +9,17 @@ from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
# #172: a developer fresh claim must carry a substantive step checklist.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
def _make_deps(**overrides: AsyncMock) -> ChoreographerDeps:
task = overrides.get("task", AsyncMock())
@@ -138,7 +149,7 @@ async def test_i_will_work_on_pending_with_plan() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y")
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y", steps=_STEPS)
assert env.error is None
assert env.status == "in_progress"
task_svc.claim.assert_awaited_once_with(task_id, agent_id)
@@ -227,7 +238,7 @@ async def test_i_will_work_on_needs_revision_re_starts() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id)
env = await c.i_will_work_on(agent_id, task_id, steps=_STEPS)
assert env.status == "in_progress"
task_svc.start.assert_awaited_once_with(task_id, agent_id)
@@ -332,7 +343,7 @@ async def test_i_will_work_on_blocks_when_journal_note_at_claim_missing() -> Non
deps = _make_deps(task=task_svc, journal=journal_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y")
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "tracing_gap"
assert "journal:note_at_claim" in body["missing"]
tests/unit/gateway/test_choreographer_impl_branches.py
+26 -13
View File
@@ -18,6 +18,19 @@ from roboco.services.gateway.choreographer import Choreographer, ChoreographerDe
from roboco.services.gateway.choreographer._impl import DelegateInputs
from roboco.services.gateway.envelope import Envelope
# #172: a developer fresh claim must carry a substantive step checklist.
# Inert on re-entry/error/non-dev paths (the gate is skipped or the call
# short-circuits before it), so it is safe to pass everywhere.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
def _wire_dev_task_svc(
task_id, *, status: str, assigned_to=None, plan=None, parent_task_id=None
@@ -139,7 +152,7 @@ async def test_i_will_work_on_pending_claim_raises_returns_invalid_state() -> No
task_svc.claim.side_effect = RuntimeError("workspace down")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="plan")
env = await c.i_will_work_on(agent_id, task_id, plan="plan", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "verb runner failed" in body["message"]
@@ -155,7 +168,7 @@ async def test_i_will_work_on_pending_claim_returns_none_invalid_state() -> None
task_svc.claim.return_value = None
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="plan")
env = await c.i_will_work_on(agent_id, task_id, plan="plan", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
@@ -177,7 +190,7 @@ async def test_i_will_work_on_pending_no_plan_tracing_gap() -> None:
task_svc.claim.return_value = claimed_task
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan=None)
env = await c.i_will_work_on(agent_id, task_id, plan=None, steps=_STEPS)
body = env.as_dict()
assert body["error"] == "tracing_gap"
@@ -201,7 +214,7 @@ async def test_i_will_work_on_start_returns_none_invalid_state() -> None:
task_svc.start.return_value = None # start fails
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok plan")
env = await c.i_will_work_on(agent_id, task_id, plan="ok plan", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "start failed" in body["message"]
@@ -224,7 +237,7 @@ async def test_needs_revision_branch_claim_fails_invalid_state() -> None:
task_svc.claim.return_value = None
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok")
env = await c.i_will_work_on(agent_id, task_id, plan="ok", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
@@ -240,7 +253,7 @@ async def test_needs_revision_branch_start_fails() -> None:
task_svc.start.return_value = None
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok")
env = await c.i_will_work_on(agent_id, task_id, plan="ok", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
@@ -261,7 +274,7 @@ async def test_claimed_branch_returns_start_failed() -> None:
task_svc.start.return_value = None
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok")
env = await c.i_will_work_on(agent_id, task_id, plan="ok", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
@@ -282,7 +295,7 @@ async def test_i_will_work_on_in_progress_assigned_to_self_idempotent() -> None:
task_svc.heartbeat = AsyncMock()
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok")
env = await c.i_will_work_on(agent_id, task_id, plan="ok", steps=_STEPS)
body = env.as_dict()
# No error — re-entry pass.
assert "error" not in body or body.get("error") is None
@@ -1016,7 +1029,7 @@ async def test_claimed_branch_already_active_guard() -> None:
task_svc.list_in_progress_for_agent.return_value = [in_prog]
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok")
env = await c.i_will_work_on(agent_id, task_id, plan="ok", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
@@ -1151,7 +1164,7 @@ async def test_i_will_work_on_envelope_carries_introspection_on_success() -> Non
task_svc.start.return_value = claimed_task
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="ok plan")
env = await c.i_will_work_on(agent_id, task_id, plan="ok plan", steps=_STEPS)
body = env.as_dict()
assert body["error"] is None
assert body["current_state"] == "in_progress"
@@ -1172,7 +1185,7 @@ async def test_i_will_work_on_envelope_carries_introspection_on_rejection() -> N
task_svc = _wire_dev_task_svc(task_id, status="completed", assigned_to=agent_id)
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="x")
env = await c.i_will_work_on(agent_id, task_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert body["current_state"] == "completed"
@@ -1233,7 +1246,7 @@ async def test_i_will_work_on_missing_plan_does_not_claim_pending_task() -> None
task_svc = _wire_dev_task_svc(task_id, status="pending")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan=None)
env = await c.i_will_work_on(agent_id, task_id, plan=None, steps=_STEPS)
body = env.as_dict()
assert body["error"] == "tracing_gap"
assert "plan" in body["missing"]
@@ -1268,7 +1281,7 @@ async def test_i_will_work_on_claimed_with_no_plan_accepts_recovery_plan() -> No
task_svc.start.return_value = started
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="recovery plan")
env = await c.i_will_work_on(agent_id, task_id, plan="recovery plan", steps=_STEPS)
body = env.as_dict()
assert body["error"] is None, f"expected success, got {body}"
task_svc.set_plan.assert_awaited_once()
tests/unit/gateway/test_claim_arg_order.py
+15 -3
View File
@@ -25,6 +25,18 @@ from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
# #172: a developer fresh claim must carry a substantive step checklist.
# Inert on re-entry/error/non-dev paths, so safe to pass everywhere.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
def _make_deps(**overrides: Any) -> ChoreographerDeps:
base = {
@@ -120,7 +132,7 @@ async def test_i_will_work_on_pending_calls_claim_with_task_id_first() -> None:
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="x")
env = await c.i_will_work_on(agent_id, task_id, plan="x", steps=_STEPS)
# Service signature is (task_id, agent_id, ...) — pin that order.
task_svc.claim.assert_awaited_once_with(task_id, agent_id)
@@ -172,7 +184,7 @@ async def test_i_will_work_on_needs_revision_calls_start_with_task_id_first() ->
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id)
env = await c.i_will_work_on(agent_id, task_id, steps=_STEPS)
task_svc.start.assert_awaited_once_with(task_id, agent_id)
task_svc.claim.assert_awaited_once_with(task_id, agent_id)
@@ -220,7 +232,7 @@ async def test_i_will_work_on_claimed_resumption_calls_start_with_task_id_first(
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id)
env = await c.i_will_work_on(agent_id, task_id, steps=_STEPS)
task_svc.start.assert_awaited_once_with(task_id, agent_id)
assert env.error is None
tests/unit/gateway/test_dev_steps_gate.py
+171
View File
@@ -0,0 +1,171 @@
"""#172: a developer's i_will_work_on must carry a substantive step checklist.
The dev plan was a free string with only a presence gate. Plan-driven
progress (#173) needs a checklist on the executing dev's task too, so
i_will_work_on now takes structured `steps` (same SubTask shape as a
PM's sub_tasks), gated for depth like the PM plan, persisted into
task.plan.sub_tasks via the panel-shaped path. Re-entry/recovery
short-circuit before the gate so a respawned dev is never re-blocked.
"""
from __future__ import annotations
from datetime import UTC, datetime
from typing import Any
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
_GOOD_STEP_DESC = (
"be-dev-1 prepends the smoke-test HTML comment above the README H1, "
"leaving the rest of the file untouched, then stages the change."
)
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 m 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, m).return_value = []
_ldef = base["journal"].latest_decision_at.return_value
if type(_ldef).__name__ in ("MagicMock", "AsyncMock"):
base["journal"].latest_decision_at.return_value = datetime.now(UTC)
return ChoreographerDeps(**base)
def _dev_task_svc(task_id: object, *, status: str = "pending") -> AsyncMock:
svc = AsyncMock()
svc.get.return_value = MagicMock(
id=task_id,
status=status,
plan=None,
assigned_to=None,
task_type="code",
parent_task_id=uuid4(),
sequence=0,
team="backend",
commits=[],
pr_number=None,
branch_name=None,
quick_context=None,
)
svc.agent_for.return_value = MagicMock(
id=uuid4(), role="developer", team="backend", slug="be-dev-1"
)
svc.list_in_progress_for_agent.return_value = []
svc.list_paused_for_agent.return_value = []
svc.get_subtasks.return_value = []
svc.session = MagicMock()
svc.session.begin_nested = MagicMock(
return_value=MagicMock(
__aenter__=AsyncMock(return_value=None),
__aexit__=AsyncMock(return_value=False),
)
)
return svc
@pytest.mark.asyncio
async def test_dev_fresh_claim_without_steps_is_rejected() -> None:
dev_id = uuid4()
task_id = uuid4()
c = Choreographer(_make_deps(task=_dev_task_svc(task_id)))
env = await c.i_will_work_on(dev_id, task_id, plan="do the thing")
body = env.as_dict()
assert body["error"] == "incomplete_input", body
assert "steps" in (body.get("missing") or []), body
@pytest.mark.asyncio
async def test_dev_thin_step_description_is_rejected() -> None:
dev_id = uuid4()
task_id = uuid4()
c = Choreographer(_make_deps(task=_dev_task_svc(task_id)))
env = await c.i_will_work_on(
dev_id,
task_id,
plan="do the thing",
steps=[{"title": "Edit README", "description": "edit it"}],
)
body = env.as_dict()
assert body["error"] == "incomplete_input", body
assert "steps" in (body.get("missing") or []), body
@pytest.mark.asyncio
async def test_dev_with_substantive_steps_passes_gate_and_persists_checklist() -> None:
dev_id = uuid4()
task_id = uuid4()
svc = _dev_task_svc(task_id)
claimed = MagicMock(
id=task_id, status="claimed", plan=None, assigned_to=dev_id, task_type="code"
)
started = MagicMock(
id=task_id,
status="in_progress",
plan={"text": "x"},
assigned_to=dev_id,
task_type="code",
)
svc.claim.return_value = claimed
svc.set_plan.return_value = claimed
svc.start.return_value = started
c = Choreographer(_make_deps(task=svc))
steps_in = [
{"title": "Edit README", "description": _GOOD_STEP_DESC},
{"title": "Commit + open PR", "description": _GOOD_STEP_DESC},
]
env = await c.i_will_work_on(
dev_id,
task_id,
plan="implement the README change end to end",
steps=steps_in,
)
body = env.as_dict()
assert body.get("error") != "incomplete_input", body
# Steps were layered into the panel-shaped plan dict and persisted.
svc.set_plan.assert_awaited_once()
persisted = svc.set_plan.await_args.args[1]
assert isinstance(persisted, dict), persisted
sub_tasks = persisted.get("sub_tasks") or []
assert len(sub_tasks) == len(steps_in), persisted
assert all(st.get("title") for st in sub_tasks), sub_tasks
assert persisted.get("text") == "implement the README change end to end"
@pytest.mark.asyncio
async def test_dev_reentry_in_progress_short_circuits_before_steps_gate() -> None:
"""A respawned dev re-calling on a task it owns in_progress with NO
steps must short-circuit to OK, not be re-blocked for steps."""
dev_id = uuid4()
task_id = uuid4()
svc = _dev_task_svc(task_id, status="in_progress")
svc.get.return_value.assigned_to = dev_id
c = Choreographer(_make_deps(task=svc))
env = await c.i_will_work_on(dev_id, task_id, plan="resume: keep going")
body = env.as_dict()
assert body.get("error") is None, body
assert body.get("status") == "in_progress", body
tests/unit/gateway/test_work_session_auto_create.py
+14 -2
View File
@@ -19,6 +19,18 @@ from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
# #172: a developer fresh claim must carry a substantive step checklist.
# Inert on re-entry/error/non-dev paths, so safe to pass everywhere.
_STEPS = [
{
"title": "Implement the change",
"description": (
"edit the target file, add tests, run them, and stage the "
"change for commit on the task branch"
),
}
]
def _make_task_svc(agent_id, task_id, *, status: str):
"""Build a TaskService AsyncMock that completes the (claim, set_plan, start)
@@ -120,7 +132,7 @@ async def test_i_will_work_on_calls_ensure_work_session() -> None:
deps = _make_deps(task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y")
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y", steps=_STEPS)
assert env.error is None, f"Expected ok, got error={env.error} msg={env.message}"
assert env.status == "in_progress"
@@ -255,7 +267,7 @@ async def test_ensure_work_session_not_called_when_start_fails() -> None:
deps = _make_deps(task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y")
env = await c.i_will_work_on(agent_id, task_id, plan="do x then y", steps=_STEPS)
assert env.error == "invalid_state"
task_svc.ensure_work_session.assert_not_awaited()