mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(gateway): developers author the full rich plan, at parity with PMs
A dev's i_will_work_on stored a flat {text} plan and routed steps to
progress, so the dev leaf's Plan tab rendered empty (no approach,
sub_tasks, technical_considerations, risks) — zero audit/tracing on the
task that does the actual work.
i_will_work_on now captures the same rich plan a PM authors via
i_will_plan: plan(>=150) doubles as approach, steps become sub_tasks,
plus technical_considerations + risks (open_questions optional). A new
_dev_plan_gate enforces them on FRESH claims only (re-entry/recovery
short-circuit before it). set_plan gains a no-downgrade guard so a
flaked-then-recovered dev can't clobber its rich plan back to flat (the
actual mechanism behind the empty leaf).
This commit is contained in:
@@ -49,7 +49,13 @@ async def i_will_work_on(
|
||||
choreographer: _ChoreographerDep,
|
||||
) -> dict:
|
||||
env = await choreographer.i_will_work_on(
|
||||
x_agent_id, body.task_id, body.plan, steps=body.steps
|
||||
x_agent_id,
|
||||
body.task_id,
|
||||
body.plan,
|
||||
steps=body.steps,
|
||||
technical_considerations=body.technical_considerations,
|
||||
risks=body.risks,
|
||||
open_questions=body.open_questions,
|
||||
)
|
||||
return envelope_to_response(env, request)
|
||||
|
||||
|
||||
@@ -22,6 +22,14 @@ class IWillWorkOnRequest(BaseModel):
|
||||
default_factory=list,
|
||||
description="Ordered execution steps — list of {title, description}",
|
||||
)
|
||||
# Full parity with IWillPlanRequest so a dev leaf's Plan tab renders the
|
||||
# same rich structure PMs author. Defaults stay permissive (NOT min_length)
|
||||
# so re-entry/recovery calls that omit them still pass route validation;
|
||||
# depth + presence are enforced on FRESH dev claims by
|
||||
# choreographer._dev_plan_gate. The dev's `plan` doubles as the approach.
|
||||
technical_considerations: list[str] = Field(default_factory=list)
|
||||
risks: list[dict[str, str]] = Field(default_factory=list)
|
||||
open_questions: list[dict[str, str | bool]] = Field(default_factory=list)
|
||||
|
||||
|
||||
class OpenPrRequest(BaseModel):
|
||||
|
||||
@@ -214,23 +214,42 @@ def i_will_work_on(
|
||||
task_id: str,
|
||||
plan: str | None = None,
|
||||
steps: list[dict[str, str]] | None = None,
|
||||
technical_considerations: list[str] | None = None,
|
||||
risks: list[dict[str, str]] | None = None,
|
||||
open_questions: list[dict[str, str | bool]] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Claim/start/recover a task. Works for pending, claimed, needs_revision.
|
||||
|
||||
On a FRESH claim a developer authors the SAME rich plan a PM does, so the
|
||||
task's Plan tab is fully populated for audit/tracing — the gateway's
|
||||
``_dev_plan_gate`` rejects a thin one. Re-entry / recovery claims
|
||||
(already-claimed, in_progress, needs_revision) do NOT re-supply any of
|
||||
this; the gateway short-circuits before the gate.
|
||||
|
||||
Args:
|
||||
task_id: UUID of the task you are claiming.
|
||||
plan: One-paragraph narrative of how you'll execute the task.
|
||||
plan: 2-4 sentences (>= 150 chars) describing HOW you will implement
|
||||
this. Doubles as the plan's "Approach".
|
||||
steps: Ordered execution checklist — list of
|
||||
``{"title": "...", "description": "..."}``. Required on a
|
||||
FRESH claim: the gateway's ``_dev_steps_gate`` rejects an
|
||||
empty or thin list, and the same list is reused as the
|
||||
progress checklist (#173 — completing a step advances %).
|
||||
Re-entry / recovery claims (already-claimed or
|
||||
needs_revision) do not need steps re-supplied.
|
||||
``{"title": "...", "description": "..."}`` with every description
|
||||
substantive. Becomes the plan's sub-tasks AND the progress
|
||||
checklist (#173 — completing a step advances %).
|
||||
technical_considerations: Bullet list (strings) of architectural /
|
||||
library / approach notes.
|
||||
risks: List of ``{"risk": "...", "mitigation": "..."}`` entries.
|
||||
open_questions: Optional list of ``{"question": "...",
|
||||
"answered": false}`` entries.
|
||||
"""
|
||||
return _post(
|
||||
_role_path("i_will_work_on"),
|
||||
{"task_id": task_id, "plan": plan, "steps": steps or []},
|
||||
{
|
||||
"task_id": task_id,
|
||||
"plan": plan,
|
||||
"steps": steps or [],
|
||||
"technical_considerations": technical_considerations or [],
|
||||
"risks": risks or [],
|
||||
"open_questions": open_questions or [],
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -964,6 +964,9 @@ class Choreographer:
|
||||
task_id: UUID,
|
||||
plan: str | None = None,
|
||||
steps: list[dict[str, Any]] | None = None,
|
||||
technical_considerations: list[str] | None = None,
|
||||
risks: list[dict[str, Any]] | None = None,
|
||||
open_questions: list[dict[str, Any]] | None = None,
|
||||
) -> Envelope:
|
||||
"""Claim a task and start work on it.
|
||||
|
||||
@@ -1006,15 +1009,21 @@ 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}
|
||||
)
|
||||
# #172/full-parity: a dev authors the same rich plan a PM does. The
|
||||
# dev's `plan` doubles as the Approach; `steps` become sub_tasks. Built
|
||||
# via the panel-shaped path so the Plan tab renders identically and
|
||||
# feeds #173 progress. With no rich fields (re-entry/recovery) this
|
||||
# falls through to unchanged string behaviour.
|
||||
rich_plan = {
|
||||
"approach": plan or "",
|
||||
"sub_tasks": steps or [],
|
||||
"technical_considerations": technical_considerations or [],
|
||||
"risks": risks or [],
|
||||
"open_questions": open_questions or [],
|
||||
}
|
||||
effective_plan: str | dict[str, Any] | None = self._resolve_effective_plan(
|
||||
plan or "", rich_plan
|
||||
)
|
||||
spec_ctx = spec_module.Context(
|
||||
plan=effective_plan,
|
||||
actor_id=agent_id,
|
||||
@@ -1035,7 +1044,16 @@ class Choreographer:
|
||||
):
|
||||
return reentry
|
||||
return await self._fresh_dev_claim(
|
||||
ctx, role, spec_ctx, agent, steps, role_str, t, agent_id, task_id, briefing
|
||||
ctx,
|
||||
role,
|
||||
spec_ctx,
|
||||
agent,
|
||||
rich_plan,
|
||||
role_str,
|
||||
t,
|
||||
agent_id,
|
||||
task_id,
|
||||
briefing,
|
||||
)
|
||||
|
||||
async def _dev_reentry(
|
||||
@@ -1080,23 +1098,23 @@ class Choreographer:
|
||||
role: Any,
|
||||
spec_ctx: Any,
|
||||
agent: Any,
|
||||
steps: list[dict[str, Any]] | None,
|
||||
rich_plan: dict[str, Any],
|
||||
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
|
||||
"""Fresh (non-re-entry) i_will_work_on tail: spec gate → dev-plan
|
||||
gate → claim/plan/start → post-claim journal gate. Extracted so
|
||||
i_will_work_on stays within the return-count budget; the dev-steps
|
||||
i_will_work_on stays within the return-count budget; the dev-plan
|
||||
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(
|
||||
if rejection := await self._dev_plan_gate(
|
||||
role_str=role_str,
|
||||
steps=steps,
|
||||
rich_plan=rich_plan,
|
||||
task=t,
|
||||
agent_id=agent_id,
|
||||
task_id=task_id,
|
||||
@@ -1108,66 +1126,80 @@ class Choreographer:
|
||||
"i_will_work_on", agent_id, task_id, envelope
|
||||
)
|
||||
|
||||
async def _dev_steps_gate(
|
||||
@staticmethod
|
||||
def _dev_plan_field_gaps(rich_plan: dict[str, Any]) -> dict[str, str]:
|
||||
"""Collect missing/thin rich-plan fields for a fresh dev claim.
|
||||
|
||||
Full parity with PMs: approach (the dev's `plan`, >= min chars),
|
||||
substantive sub_tasks (the `steps` checklist), technical_considerations
|
||||
and risks. open_questions stay optional. Returns {field: hint}.
|
||||
"""
|
||||
gaps: dict[str, str] = {}
|
||||
approach = str(rich_plan.get("approach") or "").strip()
|
||||
if len(approach) < _PM_APPROACH_MIN_LEN:
|
||||
gaps["plan"] = (
|
||||
f"plan must be >= {_PM_APPROACH_MIN_LEN} chars describing HOW "
|
||||
"you will implement this (it is the plan's Approach)."
|
||||
)
|
||||
steps = rich_plan.get("sub_tasks") or []
|
||||
if not steps:
|
||||
gaps["steps"] = (
|
||||
"a non-empty execution checklist — list of {title, "
|
||||
"description}; each step is also a progress-checklist item."
|
||||
)
|
||||
elif thin := _thin_subtask_hint(steps):
|
||||
gaps["steps"] = thin
|
||||
if not rich_plan.get("technical_considerations"):
|
||||
gaps["technical_considerations"] = (
|
||||
"list >= 1 architectural / library / approach note (strings)."
|
||||
)
|
||||
if not rich_plan.get("risks"):
|
||||
gaps["risks"] = (
|
||||
"list >= 1 {risk, mitigation} entry — what could go wrong and "
|
||||
"how you'll handle it."
|
||||
)
|
||||
return gaps
|
||||
|
||||
async def _dev_plan_gate(
|
||||
self,
|
||||
*,
|
||||
role_str: str,
|
||||
steps: list[dict[str, Any]] | None,
|
||||
rich_plan: dict[str, Any],
|
||||
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.
|
||||
"""A developer's FRESH claim must author the same rich plan a PM does,
|
||||
so the task's Plan tab is fully populated for audit/tracing. Enforces
|
||||
approach + steps + technical_considerations + risks (open_questions
|
||||
optional). Non-developer callers and re-entry are unaffected — the
|
||||
re-entry/recovery paths return before this is reached. Returns a
|
||||
rejection Envelope when the plan is thin; None when it passes.
|
||||
"""
|
||||
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
|
||||
gaps = self._dev_plan_field_gaps(rich_plan)
|
||||
if not gaps:
|
||||
return None
|
||||
return await self._emit_rejection(
|
||||
Envelope.incomplete_input(
|
||||
missing=sorted(gaps),
|
||||
field_hints=gaps,
|
||||
remediate=(
|
||||
"re-issue i_will_work_on(task_id, plan='<how, >= "
|
||||
f"{_PM_APPROACH_MIN_LEN} chars>', "
|
||||
"steps=[{'title': '...', 'description': '...'}, ...], "
|
||||
"technical_considerations=['...'], "
|
||||
"risks=[{'risk': '...', 'mitigation': '...'}]) — the same "
|
||||
"rich plan a PM authors, so your task's Plan tab is filled."
|
||||
),
|
||||
context_briefing=briefing,
|
||||
).with_introspection(task=task, role=role_str),
|
||||
agent_id=agent_id,
|
||||
task_id=task_id,
|
||||
verb="i_will_work_on",
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _with_briefing(env: Envelope, briefing: dict[str, Any]) -> Envelope:
|
||||
|
||||
+13
-2
@@ -4832,11 +4832,22 @@ class TaskService(BaseService):
|
||||
async def set_plan(
|
||||
self, task_id: UUID, plan: str | dict[str, Any]
|
||||
) -> TaskTable | None:
|
||||
"""Write the task's plan field. Strings are wrapped as {'text': plan}."""
|
||||
"""Write the task's plan field. Strings are wrapped as {'text': plan}.
|
||||
|
||||
Never DOWNGRADE a rich plan to a contentless one: a recovery/re-entry
|
||||
claim that omits the rich fields would otherwise clobber the Approach +
|
||||
sub_tasks an earlier fresh claim already authored — this was why a
|
||||
flaked-then-recovered dev leaf showed an empty Plan tab. If the incoming
|
||||
plan has no approach but the stored one does, keep the stored plan.
|
||||
"""
|
||||
task = await self.get(task_id)
|
||||
if not task:
|
||||
return None
|
||||
task.plan = plan if isinstance(plan, dict) else {"text": plan}
|
||||
new_plan = plan if isinstance(plan, dict) else {"text": plan}
|
||||
existing = task.plan if isinstance(task.plan, dict) else {}
|
||||
if existing.get("approach") and not new_plan.get("approach"):
|
||||
return task
|
||||
task.plan = new_plan
|
||||
await self.session.flush()
|
||||
return task
|
||||
|
||||
|
||||
@@ -33,6 +33,21 @@ _STEPS = [
|
||||
),
|
||||
}
|
||||
]
|
||||
# Full parity: a fresh dev claim authors the same rich plan a PM does.
|
||||
# These satisfy _dev_plan_gate (plan/approach >= 150 chars,
|
||||
# technical_considerations, risks).
|
||||
_GOOD_PLAN = (
|
||||
"Append the timestamp HTML comment to the very bottom of README.md without "
|
||||
"touching any other line, then commit it on the task branch and open a PR. "
|
||||
"Verify the diff is a single-line addition before submitting for QA."
|
||||
)
|
||||
_GOOD_TC = ["Use a trailing newline so the comment sits on its own line."]
|
||||
_GOOD_RISKS = [
|
||||
{
|
||||
"risk": "An accidental reformat of README.md balloons the diff.",
|
||||
"mitigation": "Append only; assert the diff touches one line pre-commit.",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
@@ -189,7 +204,14 @@ 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, steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
target_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
assert env.error is None
|
||||
task_svc.claim.assert_awaited_once_with(target_id, agent_id)
|
||||
|
||||
@@ -219,7 +241,14 @@ 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, steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
target_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
assert env.error is None
|
||||
# Sequence check should not have queried siblings on a root task
|
||||
task_svc.get_subtasks.assert_not_awaited()
|
||||
|
||||
@@ -19,6 +19,21 @@ _STEPS = [
|
||||
),
|
||||
}
|
||||
]
|
||||
# Full parity: a fresh dev claim authors the same rich plan a PM does.
|
||||
# These satisfy _dev_plan_gate (plan/approach >= 150 chars,
|
||||
# technical_considerations, risks).
|
||||
_GOOD_PLAN = (
|
||||
"Append the timestamp HTML comment to the very bottom of README.md without "
|
||||
"touching any other line, then commit it on the task branch and open a PR. "
|
||||
"Verify the diff is a single-line addition before submitting for QA."
|
||||
)
|
||||
_GOOD_TC = ["Use a trailing newline so the comment sits on its own line."]
|
||||
_GOOD_RISKS = [
|
||||
{
|
||||
"risk": "An accidental reformat of README.md balloons the diff.",
|
||||
"mitigation": "Append only; assert the diff touches one line pre-commit.",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _make_deps(**overrides: AsyncMock) -> ChoreographerDeps:
|
||||
@@ -149,7 +164,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
assert env.error is None
|
||||
assert env.status == "in_progress"
|
||||
task_svc.claim.assert_awaited_once_with(task_id, agent_id)
|
||||
@@ -238,7 +260,14 @@ 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, steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
assert env.status == "in_progress"
|
||||
task_svc.start.assert_awaited_once_with(task_id, agent_id)
|
||||
|
||||
@@ -343,7 +372,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "tracing_gap"
|
||||
assert "journal:note_at_claim" in body["missing"]
|
||||
|
||||
@@ -30,6 +30,21 @@ _STEPS = [
|
||||
),
|
||||
}
|
||||
]
|
||||
# Full parity: a fresh dev claim authors the same rich plan a PM does.
|
||||
# These satisfy _dev_plan_gate (plan/approach >= 150 chars,
|
||||
# technical_considerations, risks).
|
||||
_GOOD_PLAN = (
|
||||
"Append the timestamp HTML comment to the very bottom of README.md without "
|
||||
"touching any other line, then commit it on the task branch and open a PR. "
|
||||
"Verify the diff is a single-line addition before submitting for QA."
|
||||
)
|
||||
_GOOD_TC = ["Use a trailing newline so the comment sits on its own line."]
|
||||
_GOOD_RISKS = [
|
||||
{
|
||||
"risk": "An accidental reformat of README.md balloons the diff.",
|
||||
"mitigation": "Append only; assert the diff touches one line pre-commit.",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _wire_dev_task_svc(
|
||||
@@ -152,7 +167,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "verb runner failed" in body["message"]
|
||||
@@ -168,7 +190,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
|
||||
@@ -214,7 +243,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "start failed" in body["message"]
|
||||
@@ -237,7 +273,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
|
||||
@@ -253,7 +296,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
|
||||
@@ -1147,7 +1197,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] is None
|
||||
assert body["current_state"] == "in_progress"
|
||||
|
||||
@@ -36,6 +36,21 @@ _STEPS = [
|
||||
),
|
||||
}
|
||||
]
|
||||
# Full parity: a fresh dev claim authors the same rich plan a PM does.
|
||||
# These satisfy _dev_plan_gate (plan/approach >= 150 chars,
|
||||
# technical_considerations, risks).
|
||||
_GOOD_PLAN = (
|
||||
"Append the timestamp HTML comment to the very bottom of README.md without "
|
||||
"touching any other line, then commit it on the task branch and open a PR. "
|
||||
"Verify the diff is a single-line addition before submitting for QA."
|
||||
)
|
||||
_GOOD_TC = ["Use a trailing newline so the comment sits on its own line."]
|
||||
_GOOD_RISKS = [
|
||||
{
|
||||
"risk": "An accidental reformat of README.md balloons the diff.",
|
||||
"mitigation": "Append only; assert the diff touches one line pre-commit.",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
@@ -132,7 +147,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
|
||||
# Service signature is (task_id, agent_id, ...) — pin that order.
|
||||
task_svc.claim.assert_awaited_once_with(task_id, agent_id)
|
||||
@@ -184,7 +206,14 @@ 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, steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
|
||||
task_svc.start.assert_awaited_once_with(task_id, agent_id)
|
||||
task_svc.claim.assert_awaited_once_with(task_id, agent_id)
|
||||
|
||||
@@ -22,6 +22,31 @@ _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."
|
||||
)
|
||||
# Full parity: a fresh dev claim authors the same rich plan a PM does. These
|
||||
# satisfy _dev_plan_gate (plan/approach >= 150 chars, substantive steps,
|
||||
# technical_considerations, risks).
|
||||
_GOOD_PLAN = (
|
||||
"Append the timestamp HTML comment to the very bottom of README.md without "
|
||||
"touching any other line, then commit it on the task branch and open a PR. "
|
||||
"Verify the diff is a single-line addition before submitting for QA."
|
||||
)
|
||||
_GOOD_TC = ["Use a trailing newline so the comment sits on its own line."]
|
||||
_GOOD_RISKS = [
|
||||
{
|
||||
"risk": "An accidental reformat of README.md balloons the diff.",
|
||||
"mitigation": "Append only; assert the diff touches one line pre-commit.",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _full_plan_kwargs(steps: list[dict[str, str]]) -> dict[str, Any]:
|
||||
"""The full rich-plan kwargs a fresh dev claim must supply post-parity."""
|
||||
return {
|
||||
"plan": _GOOD_PLAN,
|
||||
"steps": steps,
|
||||
"technical_considerations": _GOOD_TC,
|
||||
"risks": _GOOD_RISKS,
|
||||
}
|
||||
|
||||
|
||||
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
@@ -137,22 +162,42 @@ async def test_dev_with_substantive_steps_passes_gate_and_persists_checklist() -
|
||||
{"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,
|
||||
)
|
||||
env = await c.i_will_work_on(dev_id, task_id, **_full_plan_kwargs(steps_in))
|
||||
body = env.as_dict()
|
||||
assert body.get("error") != "incomplete_input", body
|
||||
# Steps were layered into the panel-shaped plan dict and persisted.
|
||||
# The full rich plan was layered into the panel-shaped dict and persisted,
|
||||
# so the dev leaf's Plan tab renders like a PM's (approach + sub_tasks +
|
||||
# technical_considerations + risks).
|
||||
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"
|
||||
assert persisted.get("approach") == _GOOD_PLAN
|
||||
assert persisted.get("technical_considerations") == _GOOD_TC
|
||||
assert len(persisted.get("risks") or []) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dev_fresh_claim_missing_considerations_and_risks_rejected() -> None:
|
||||
"""Full parity: substantive steps + long plan are not enough — a fresh dev
|
||||
claim must also carry technical_considerations and risks."""
|
||||
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=_GOOD_PLAN,
|
||||
steps=[{"title": "Edit README", "description": _GOOD_STEP_DESC}],
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "incomplete_input", body
|
||||
missing = body.get("missing") or []
|
||||
assert "technical_considerations" in missing, body
|
||||
assert "risks" in missing, body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -30,6 +30,21 @@ _STEPS = [
|
||||
),
|
||||
}
|
||||
]
|
||||
# Full parity: a fresh dev claim authors the same rich plan a PM does.
|
||||
# These satisfy _dev_plan_gate (plan/approach >= 150 chars,
|
||||
# technical_considerations, risks).
|
||||
_GOOD_PLAN = (
|
||||
"Append the timestamp HTML comment to the very bottom of README.md without "
|
||||
"touching any other line, then commit it on the task branch and open a PR. "
|
||||
"Verify the diff is a single-line addition before submitting for QA."
|
||||
)
|
||||
_GOOD_TC = ["Use a trailing newline so the comment sits on its own line."]
|
||||
_GOOD_RISKS = [
|
||||
{
|
||||
"risk": "An accidental reformat of README.md balloons the diff.",
|
||||
"mitigation": "Append only; assert the diff touches one line pre-commit.",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _make_task_svc(agent_id, task_id, *, status: str):
|
||||
@@ -132,7 +147,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
|
||||
assert env.error is None, f"Expected ok, got error={env.error} msg={env.message}"
|
||||
assert env.status == "in_progress"
|
||||
@@ -267,7 +289,14 @@ 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", steps=_STEPS)
|
||||
env = await c.i_will_work_on(
|
||||
agent_id,
|
||||
task_id,
|
||||
plan=_GOOD_PLAN,
|
||||
steps=_STEPS,
|
||||
technical_considerations=_GOOD_TC,
|
||||
risks=_GOOD_RISKS,
|
||||
)
|
||||
|
||||
assert env.error == "invalid_state"
|
||||
task_svc.ensure_work_session.assert_not_awaited()
|
||||
|
||||
@@ -144,6 +144,9 @@ def test_i_will_work_on_passes_plan(flow_module: types.ModuleType) -> None:
|
||||
"task_id": "task-uuid",
|
||||
"plan": "my plan",
|
||||
"steps": [],
|
||||
"technical_considerations": [],
|
||||
"risks": [],
|
||||
"open_questions": [],
|
||||
}
|
||||
assert "/api/v2/flow/developer/i_will_work_on" in args[0]
|
||||
|
||||
@@ -155,7 +158,14 @@ def test_i_will_work_on_plan_defaults_to_none(flow_module: types.ModuleType) ->
|
||||
flow_module.i_will_work_on("task-uuid")
|
||||
|
||||
_, kwargs = fake_client.post.call_args
|
||||
assert kwargs["json"] == {"task_id": "task-uuid", "plan": None, "steps": []}
|
||||
assert kwargs["json"] == {
|
||||
"task_id": "task-uuid",
|
||||
"plan": None,
|
||||
"steps": [],
|
||||
"technical_considerations": [],
|
||||
"risks": [],
|
||||
"open_questions": [],
|
||||
}
|
||||
|
||||
|
||||
def test_i_will_work_on_passes_steps(flow_module: types.ModuleType) -> None:
|
||||
@@ -176,6 +186,9 @@ def test_i_will_work_on_passes_steps(flow_module: types.ModuleType) -> None:
|
||||
"task_id": "task-uuid",
|
||||
"plan": "p",
|
||||
"steps": steps,
|
||||
"technical_considerations": [],
|
||||
"risks": [],
|
||||
"open_questions": [],
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user