mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): spine-cap remediate forbids task_type workaround
Smoke-7: be-pm got the expected spine-cap rejection on a second delegate. The remediate said "drive the existing sibling to completion / cancel it, OR split this parent into two sibling parents". The model read that, decided neither applied, and "adapted" by re-delegating with task_type='documentation' as a "verification subtask". The gateway accepted it (different type = no cap collision) but the orphan subtask had no claimant — it blocked submit_up forever with "subtasks not all terminal". Two-layer fix: 1. _spine_type_dup_envelope remediate now explicitly forbids the workaround: "DO NOT work around this by delegating again with a different task_type (e.g. 'documentation' or 'research' as a 'verification' subtask). The lifecycle handles QA, documentation, and PM-review automatically after the code subtask finishes — you do not create auxiliary subtasks for those roles. Call i_am_idle() now and wait for the existing child to come back." 2. cell_pm.md workflow step 6 strengthened to name the anti-pattern explicitly: no verification subtask (QA is the verification step); never re-delegate with a different task_type as a workaround. 3 new tests pin the remediate text: forbids workaround, names the verification anti-pattern, retains invalid_state error kind.
This commit is contained in:
@@ -70,7 +70,7 @@ You merge what your developers submit (leaf PRs into your cell branch via `compl
|
||||
3. `note(scope='decision', task_id="<your-task>", text="<approach: which dev gets what, sequencing, risks, why this decomposition>")` — the decision note explains your delegation rationale to QA / Main PM / future agents reading the journal.
|
||||
4. `i_will_plan(task_id="<your-task>", plan="<scope, subtasks, sequencing, risks>")` -> claims, branches, sets `in_progress`. **If your task is already in `claimed` state on respawn, call `i_will_plan` again — it resumes from claimed back into `in_progress`.**
|
||||
5. `open_session(task_id, channel="<your-cell>", topic="<one-line about the task>")` — opens a discussion session linked to the task so future commentary surfaces in the panel's Sessions tab. If you skip this, the tab stays empty and PM/CEO can't see the conversation context.
|
||||
6. `delegate(parent_task_id="<your-task>", assigned_to="<dev-slug-in-your-cell>", ...)`. **Default to ONE dev subtask per logical unit of work.** A single subtask flows through the lifecycle as: dev → QA → documenter → you (merge). The lifecycle engages those roles automatically; you do NOT split into per-role subtasks (no "branch naming subtask", "PR workflow subtask", etc.). Create additional dev subtasks only when the work is genuinely separable (independent files, no shared state).
|
||||
6. `delegate(parent_task_id="<your-task>", assigned_to="<dev-slug-in-your-cell>", ...)`. **Default to ONE dev subtask per logical unit of work.** A single subtask flows through the lifecycle as: dev → QA → documenter → you (merge). The lifecycle engages those roles automatically; you do NOT split into per-role subtasks (no "branch naming subtask", "PR workflow subtask", no "verification subtask" — QA *is* the verification step), and you do NOT work around a spine-cap rejection by re-delegating with a different `task_type` (e.g. `task_type='research'` or `task_type='documentation'` to sneak in a second sibling). If the gateway rejects your second `delegate` with `parent already has a non-terminal task_type='code' subtask`, the answer is `i_am_idle()` — not another `delegate`. Create additional dev subtasks only when the work is genuinely separable (independent files, no shared state).
|
||||
7. `i_am_idle()` -> wait. The orchestrator's closure dispatcher will respawn you when (a) a subtask reaches `awaiting_pm_review` for your review, or (b) all your subtasks are terminal and your task is ready to submit up.
|
||||
8. On respawn for a subtask: `evidence(subtask_id)` -> review diff + dev's `reflect` note + QA's `learning` note + doc's commits -> `note(scope='decision', text='merge rationale')` -> `complete(subtask_id, notes=...)`. The leaf PR auto-merges into your cell branch.
|
||||
9. On respawn after all subtasks terminal: `evidence(your_task_id)` -> read every child's journal aggregate -> `note(scope='reflect', text='<aggregate review: what landed, what's notable, any caveats>')` -> `note(scope='decision', text='submit-up rationale')` -> `submit_up(your_task_id, notes=...)`. Main PM takes over.
|
||||
|
||||
@@ -2448,7 +2448,14 @@ class Choreographer:
|
||||
"type. If the work is genuinely parallel (two "
|
||||
"independent modules), split this parent into "
|
||||
"two sibling parents instead of two code "
|
||||
"subtasks under one parent."
|
||||
"subtasks under one parent.\n\n"
|
||||
"**DO NOT work around this by delegating again with a "
|
||||
"different task_type** (e.g. 'documentation' or "
|
||||
"'research' as a 'verification' subtask). The lifecycle "
|
||||
"handles QA, documentation, and PM-review automatically "
|
||||
"after the code subtask finishes — you do not create "
|
||||
"auxiliary subtasks for those roles. Call i_am_idle() "
|
||||
"now and wait for the existing child to come back."
|
||||
),
|
||||
context_briefing={},
|
||||
)
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
"""Smoke-7: spine-cap rejection's remediate forbids the workaround pattern.
|
||||
|
||||
Original behavior: cell-PM gets the spine-cap rejection on a second
|
||||
delegate (parent already has non-terminal task_type='code' subtask).
|
||||
The model "adapts" by delegating again with task_type='research' or
|
||||
'documentation' as a "verification" subtask. That works around the
|
||||
gateway but creates a permanently-stuck orphan subtask that no agent
|
||||
will ever claim — blocks submit_up forever with
|
||||
`subtasks not all terminal`.
|
||||
|
||||
Fix: remediate explicitly forbids the workaround and tells the agent
|
||||
to call i_am_idle() instead.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from roboco.services.gateway.choreographer._impl import Choreographer
|
||||
|
||||
|
||||
def test_spine_cap_remediate_forbids_task_type_workaround() -> None:
|
||||
"""The spine-cap rejection's remediate must say 'do not work around with
|
||||
different task_type'."""
|
||||
sibling = MagicMock(id="abc12345-...", status="pending")
|
||||
env = Choreographer._spine_type_dup_envelope(
|
||||
new_type="code",
|
||||
sibling=sibling,
|
||||
sib_assignee="be-dev-1",
|
||||
)
|
||||
remediate = env.remediate or ""
|
||||
assert "DO NOT work around" in remediate, (
|
||||
f"Spine-cap remediate must forbid the task_type workaround. Got:\n{remediate}"
|
||||
)
|
||||
assert "different task_type" in remediate
|
||||
assert "i_am_idle" in remediate
|
||||
|
||||
|
||||
def test_spine_cap_remediate_warns_about_verification_subtasks() -> None:
|
||||
"""The remediate must name the specific 'verification subtask' anti-pattern."""
|
||||
sibling = MagicMock(id="abc12345-...", status="pending")
|
||||
env = Choreographer._spine_type_dup_envelope(
|
||||
new_type="code",
|
||||
sibling=sibling,
|
||||
sib_assignee="be-dev-1",
|
||||
)
|
||||
remediate = env.remediate or ""
|
||||
# Anti-pattern names — must appear so the model pattern-matches its own behavior.
|
||||
assert "verification" in remediate.lower() or "research" in remediate.lower()
|
||||
|
||||
|
||||
def test_spine_cap_envelope_is_invalid_state() -> None:
|
||||
"""The envelope keeps the invalid_state error kind (so the agent knows
|
||||
it's a state issue, not authorization or input shape)."""
|
||||
sibling = MagicMock(id="abc12345-...", status="pending")
|
||||
env = Choreographer._spine_type_dup_envelope(
|
||||
new_type="code",
|
||||
sibling=sibling,
|
||||
sib_assignee="be-dev-1",
|
||||
)
|
||||
assert env.error == "invalid_state"
|
||||
Reference in New Issue
Block a user