From 87b18bc64f6f25d6ec22d67655ee8666e0d21d6e Mon Sep 17 00:00:00 2001 From: Renn F Date: Fri, 15 May 2026 03:26:04 +0200 Subject: [PATCH] fix(gateway): spine-cap remediate forbids task_type workaround MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agents/prompts/roles/cell_pm.md | 2 +- .../services/gateway/choreographer/_impl.py | 9 ++- .../unit/gateway/test_spine_cap_remediate.py | 61 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 tests/unit/gateway/test_spine_cap_remediate.py diff --git a/agents/prompts/roles/cell_pm.md b/agents/prompts/roles/cell_pm.md index b378c374..b7eff64a 100644 --- a/agents/prompts/roles/cell_pm.md +++ b/agents/prompts/roles/cell_pm.md @@ -70,7 +70,7 @@ You merge what your developers submit (leaf PRs into your cell branch via `compl 3. `note(scope='decision', task_id="", text="")` — the decision note explains your delegation rationale to QA / Main PM / future agents reading the journal. 4. `i_will_plan(task_id="", plan="")` -> 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="", topic="")` — 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="", assigned_to="", ...)`. **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="", assigned_to="", ...)`. **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='')` -> `note(scope='decision', text='submit-up rationale')` -> `submit_up(your_task_id, notes=...)`. Main PM takes over. diff --git a/roboco/services/gateway/choreographer/_impl.py b/roboco/services/gateway/choreographer/_impl.py index eeff1f03..9d594e80 100644 --- a/roboco/services/gateway/choreographer/_impl.py +++ b/roboco/services/gateway/choreographer/_impl.py @@ -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={}, ) diff --git a/tests/unit/gateway/test_spine_cap_remediate.py b/tests/unit/gateway/test_spine_cap_remediate.py new file mode 100644 index 00000000..c9d51515 --- /dev/null +++ b/tests/unit/gateway/test_spine_cap_remediate.py @@ -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"