Files
roboco/tests/unit/gateway/test_choreographer_claim_guards.py
T
c09cf80b40 Feature/observability gateway health (#247)
* feat(observability): revision_count + audit_log query index (migration 045)

Adds tasks.revision_count (the O(1) rework counter — forward-only, existing
rows default 0) and the composite index audit_log(target_id, event_type,
timestamp) that powers the cycle-time and rework reconstruction queries.
Verified the real upgrade/downgrade/upgrade chain on a throwaway pgvector PG.
First task of the 0.10.0 observability dashboards.

* feat(observability): count reworks + attribute qa_fail/pr_fail to the rejector

Every transition into needs_revision increments tasks.revision_count at the
single audit chokepoint (exactly once per bounce, across all paths incl. pr_fail
and ceo_reject), so the rework rate is an O(1) read. A QA or PR-review bounce
also emits a named task.qa_fail / task.pr_fail audit event carrying the
rejector's agent_id, so the per-agent rework scorecard charges the rejection to
the reviewer who made it, not the developer who owns the task.

* feat(observability): cycle-time, bottleneck, rework, and scorecard metrics

MetricsService gains four read methods on the audit_log + tasks data: per-stage
cycle time reconstructed from the transition journey (excluding the named
qa_fail/pr_fail events), bottleneck distribution (cumulative dwell + live parked
counts), rework rate (overall/by-team/by-agent with rejector attribution + cost
via spawn-session task_id), and a fused per-agent/per-cell scorecard. Dataclass
models with to_dict(). Verified against a real Postgres journey.

* feat(observability): cycle-time/bottleneck/rework/scorecard read endpoints

Thin read-only routes on the dashboard router delegating to MetricsService:
/metrics/cycle-time, /metrics/bottlenecks, /metrics/rework, and
/metrics/scorecard/{agent,team}. 404 when an agent scorecard target is absent.
5 route tests (200 + shape + the agent-404 case).

* feat(panel): Delivery observability tab (cycle-time, bottlenecks, rework, scorecards)

A third Metrics tab built on the observability endpoints: a per-stage
cycle-time bar chart, a bottleneck panel (worst stage + cumulative dwell +
live parked counts), a rework panel (rate + by-team + by-agent attribution +
cost), and per-cell scorecards. Reuses Recharts + Card/Badge/Skeleton and the
React-Query hook pattern; observabilityApi mirrors usageApi with mock-mode
fallbacks. tsc + eslint clean; 113 panel tests pass.

* docs(observability): changelog + CLAUDE.md for the delivery dashboards

* feat(gateway-health): recover a broken-but-alive agent instead of protecting it

The verb-heartbeat cannot tell a quiet-healthy agent from one whose MCP gateway
is broken (a corrupted /app/.venv firing no verb) yet whose container is up — the
reaper's live-skip would shield it forever. The reaper now probes the gateway
out-of-band (docker exec: does the gateway venv import its deps?) and, once it
has been broken past gateway_health_grace_seconds (tolerating a transient probe
miss), kills + evicts the container so it falls through to release + respawn.
Probe-inconclusive or healthy spares the container. Gated by
gateway_health_enabled (default-on reliability fix; in the panel Feature Flags).
Defers the optional agent-side self-check + full registry re-adoption — the
reaper's docker-liveness fallback already recovers a broken-after-restart agent.

* docs(gateway-health): changelog + CLAUDE.md for broken-but-alive recovery

* docs(observability): user-facing docs for the Delivery dashboards + gateway-health

Documents the new Metrics -> Delivery tab (cycle-time, bottlenecks, rework with
rejector attribution, cell scorecards) in the panel guide and the operations
health-and-metrics guide, and adds the gateway-health env vars + an agent-gateway
recovery note. Published MkDocs site only; settings.md's default-off flag table
intentionally omits the default-on gateway-health flag (same as overload-break).

* chore(release): cut 0.10.0 (changelog section + version refs)

* fix(gateway): exempt PM coordinators from single-task claim guards

A Main/Cell PM plans and delegates many root tasks in parallel; the work
then runs in the delegated cells, not in the PM's own hands. But the
claim-time concurrency guards meant for developers — already_active and
paused (the latter firing after i_am_idle auto-pauses the PM's own
umbrella) — were applied to the PM too, so once it held one root it could
never plan a second: it thrashed between its claimed roots and respawned
forever, burning tokens for zero progress.

_run_claim_guards now skips already_active/paused for the coordinator PM
roles (_COORDINATOR_ROLES = {main_pm, cell_pm}); only unmet_dependency — a
real upstream sequence constraint, which parks the root back to pending —
still gates a PM. paused_tasks_guard also excludes the target task itself,
so a PM re-entering its own paused umbrella never self-blocks.

Tests: a coordinator plans a second root with one in_progress + one paused
sibling (full path + claimed-recovery path), the paused target exclusion,
and the developer guards still fire. Repurposed the pre-fix test that
asserted the now-removed PM block.

* fix(metrics): coerce SQL avg/extract hours aggregates to float (panel toFixed crash)

EXTRACT(epoch ...) returns numeric on PostgreSQL 14+, which asyncpg surfaces
as a Decimal; a Decimal serializes to a quoted JSON string, so the panel's
avg_cycle_hours.toFixed(1) (and the other hours fields) threw 'toFixed is not
a function' and blanked the Delivery tab.

A single _as_hours helper now rounds every SQL-averaged hours field to a real
float — avg_cycle_hours on the new scorecards plus the pre-existing
avg_completion_hours / avg_blocked_hours / longest_blocked_hours. Token and
cost fields were already float()-cast and are unaffected.

Regression test asserts _as_hours coerces Decimal -> float and preserves the
None/zero behavior.

* feat(panel): edit a task's sequence from the details page

A task's sequence (order within siblings, lower runs first) was display-only
with no way to change it from the UI, and TaskUpdate didn't carry the field
so PATCH couldn't set it either. The details page's Dependencies tab now has
an inline sequence editor mirroring the parent / dependency editors, and
PATCH /tasks/{id} accepts a sequence field (owner or privileged role) through
the existing generic update path.

* fix(mypy): green the full make-quality type gate

make quality runs 'mypy roboco/ tests/', which the per-module checks on the
0.10.0 branch never exercised. Two issues surfaced:

- The coordinator-exemption change added role_str to
  Choreographer._run_claim_guards but not to the ChoreographerHelpers
  protocol base, so the composed Choreographer had incompatible base-class
  signatures. Sync the protocol signature.

- The gateway-health / stale-reaper tests stubbed methods by direct
  assignment (orch._m = AsyncMock()) and typed their duck-typed task doubles
  as object, tripping method-assign / assignment / attr-defined. Switch to
  monkeypatch.setattr (keeping a local mock ref for the assertions) and type
  the doubles as Any — no type: ignore.

Full mypy roboco/ tests/ clean (785 files); the 21 runtime tests pass.

* fix(metrics): static cycle-time SQL — clear bandit B608 (CI gate)

The cycle-time query interpolated an optional team clause into the text() SQL
via an f-string, which bandit flags as B608 (hardcoded SQL) and turned the
merge gate red. The team value was always a bound parameter, so it was a false
positive — but the f-string is the trigger. Rebuilt as one static query with
(CAST(:team AS text) IS NULL OR a.details->>'team' = :team) and an always-bound
team param (CAST, not ::text — SQLAlchemy's :param parser collides with
PostgreSQL's :: cast operator, which broke the query as a stray param).

Full make quality green vs a real pgvector PG (all 21 gate steps).

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-06-23 07:26:41 +02:00

697 lines
23 KiB
Python

"""Gate Set A: claim-time guards restored from pre-gateway _helpers.py:124-204.
Predicates ported into Choreographer claim verbs:
- ALREADY_ACTIVE (no claim while in_progress task is open)
- PAUSED_TASKS_EXIST (no claim while paused tasks exist)
- PM_CANNOT_EXECUTE_CODE (cell_pm/main_pm cannot claim task_type=code)
- ROLE_TYPED_CLAIM (developer/qa/documenter cannot cross-claim)
These mirror pre-gateway gates at commit 0c3d15a, file
roboco/mcp/tasks/handlers/_helpers.py lines 124-204.
"""
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
from roboco.services.gateway.claim_guards import paused_tasks_guard
# #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] = {
"task": AsyncMock(),
"work_session": AsyncMock(),
"git": AsyncMock(),
"a2a": AsyncMock(),
"journal": AsyncMock(),
"audit": AsyncMock(),
"evidence_repo": AsyncMock(),
}
base.update(overrides)
# VerbRunner uses task.session.begin_nested() as a savepoint context
# manager. AsyncMock auto-attributes any access (so hasattr always
# returns True); we always overwrite session to a MagicMock with the
# correct async-context-manager protocol.
task = base["task"]
task.session = MagicMock()
task.session.begin_nested = MagicMock(
return_value=MagicMock(
__aenter__=AsyncMock(return_value=None),
__aexit__=AsyncMock(return_value=False),
)
)
repo = base["evidence_repo"]
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(repo, method).return_value = []
# C8: default-fresh journal:decision so PM-decision gate passes.
# Tests that exercise the gate boundary stub their own value.
# The check matches MagicMock and AsyncMock (the two default sentinel
# types pytest's unittest.mock leaves on un-stubbed return_values).
_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 _task_svc_with(
target: MagicMock,
*,
role: str = "developer",
agent_id: object | None = None,
lookups: dict[str, list[MagicMock]] | None = None,
) -> AsyncMock:
"""Build a task service mock primed with the active-task and sibling lookups.
`agent_id` (when supplied) is used as the GatewayAgentView's id so that
runner-driven calls like ``task.claim(task.id, agent.id)`` line up
with the test's assert_awaited_with(target_id, agent_id).
`lookups` carries the optional in_progress / paused / siblings lists
(defaulting empty). One bag avoids ruff PLR0913 on the helper sig.
"""
lookups = lookups or {}
task_svc = AsyncMock()
task_svc.get.return_value = target
task_svc.agent_for.return_value = MagicMock(
id=agent_id, role=role, team="backend", slug=None
)
task_svc.list_in_progress_for_agent.return_value = lookups.get("in_progress", [])
task_svc.list_paused_for_agent.return_value = lookups.get("paused", [])
task_svc.get_subtasks.return_value = lookups.get("siblings", [])
return task_svc
# ---------------------------------------------------------------------------
# A.2 ALREADY_ACTIVE
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_i_will_work_on_blocks_when_agent_has_in_progress_task() -> None:
agent_id = uuid4()
target_id = uuid4()
other_id = uuid4()
target = MagicMock(
id=target_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
in_progress = MagicMock(id=other_id, status="in_progress")
task_svc = _task_svc_with(target, lookups={"in_progress": [in_progress]})
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
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"]
assert "i_am_done" in body["remediate"] or "i_am_idle" in body["remediate"]
task_svc.claim.assert_not_awaited()
@pytest.mark.asyncio
async def test_i_will_work_on_resumption_does_not_self_block() -> None:
"""Resuming a claimed task already owned must not trigger ALREADY_ACTIVE."""
agent_id = uuid4()
task_id = uuid4()
claimed = MagicMock(
id=task_id,
status="claimed",
plan={"x": 1},
assigned_to=agent_id,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
branch_name="feature/backend/abc",
)
started = MagicMock(
id=task_id, status="in_progress", plan={"x": 1}, assigned_to=agent_id
)
task_svc = _task_svc_with(target=claimed, agent_id=agent_id)
# Even if there's an in_progress task with the SAME id, that's the resumption itself
task_svc.list_in_progress_for_agent.return_value = []
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, steps=_STEPS)
assert env.error is None
task_svc.start.assert_awaited_once_with(task_id, agent_id)
# ---------------------------------------------------------------------------
# A.3 PAUSED_TASKS_EXIST
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_i_will_work_on_blocks_when_agent_has_paused_task() -> None:
agent_id = uuid4()
target_id = uuid4()
paused_id = uuid4()
target = MagicMock(
id=target_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
paused = MagicMock(id=paused_id, status="paused")
task_svc = _task_svc_with(target, lookups={"paused": [paused]})
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
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"]
assert "resume" in body["remediate"].lower()
task_svc.claim.assert_not_awaited()
# ---------------------------------------------------------------------------
# A.4 PM_CANNOT_EXECUTE_CODE
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_cell_pm_cannot_claim_code_task_via_i_will_work_on() -> None:
pm_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
task_svc = _task_svc_with(target, role="cell_pm")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
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'".
assert "cell_pm" in body["message"]
assert "i_will_work_on" in body["message"]
task_svc.claim.assert_not_awaited()
@pytest.mark.asyncio
async def test_main_pm_cannot_claim_code_task_via_i_will_work_on() -> None:
pm_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
task_svc = _task_svc_with(target, role="main_pm")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(pm_id, task_id, plan="x", steps=_STEPS)
body = env.as_dict()
assert body["error"] == "not_authorized"
@pytest.mark.asyncio
async def test_cell_pm_can_plan_code_typed_parent_via_i_will_plan() -> None:
"""Rule change (2026-05-08): the PM-cannot-execute-code guard belongs
on `i_will_work_on` (the EXECUTION verb), not on `i_will_plan` (the
PLANNING verb). PMs decompose code-typed parent tasks into
developer-claimable subtasks all the time; that's planning, not
executing. Pre-fix this rejection deadlocked every code-typed parent
in the smoke test (see PRE_GATEWAY_LIFECYCLE.md and the 2026-05-08
audit-log analysis).
"""
pm_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
task_svc = _task_svc_with(target, role="cell_pm")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_plan(
pm_id,
task_id,
plan="Decompose into 2 dev subtasks.",
rich_plan={
"approach": (
"Split the code-typed parent into two developer-claimable "
"subtasks: one for API implementation, one for test coverage. "
"Sequenced so the API lands first; QA reviews each PR after "
"it opens, documentation follows, then complete and submit "
"up. No cross-cell dependencies for this slice."
),
"sub_tasks": [
{
"title": "API subtask",
"description": (
"be-dev-1 implements the endpoint with tests, commits "
"with the task-id prefix, opens the leaf PR for QA."
),
},
],
},
)
body = env.as_dict()
# The PM-cannot-execute-code rejection must NOT fire on i_will_plan.
assert body.get("error") != "not_authorized", (
f"i_will_plan was rejected for a code-typed parent; envelope: {body}"
)
@pytest.mark.asyncio
async def test_pm_can_plan_non_code_parent() -> None:
pm_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="planning",
team="backend",
)
claimed = MagicMock(
id=task_id, status="claimed", plan=None, assigned_to=pm_id, task_type="planning"
)
started = MagicMock(
id=task_id,
status="in_progress",
plan={"text": "x"},
assigned_to=pm_id,
task_type="planning",
)
task_svc = _task_svc_with(target, role="cell_pm")
task_svc.claim.return_value = claimed
task_svc.set_plan.return_value = claimed
task_svc.start.return_value = started
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_plan(
pm_id,
task_id,
plan="break it down",
rich_plan={
"approach": (
"Single-cell decomposition: backend handles the full scope; "
"no frontend or ux work required for this planning task. "
"be-dev-1 owns the change end to end; QA reviews after the "
"PR opens, documentation follows, then be-pm completes and "
"submits up. Strict sequencing, no cross-cell dependencies."
),
"sub_tasks": [
{
"title": "Backend planning slice",
"description": (
"scope the change, assign be-dev-1, who implements "
"with tests and opens the leaf PR for QA review."
),
}
],
},
)
assert env.error is None
# ---------------------------------------------------------------------------
# A.5 ROLE_TYPED_CLAIM (cross-role rejection)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_developer_cannot_claim_qa_status_task() -> None:
"""Dev calling i_will_work_on on awaiting_qa task gets explicit rejection.
spec.CLAIM_RULES restricts DEVELOPER to PENDING/NEEDS_REVISION; an
awaiting_qa task is reserved for QA. spec.can_claim surfaces this as
not_authorized (status reserved for another role) which the verb
relays via Envelope.from_decision. Pre-spec the verb body produced
invalid_state from a custom else-branch; the spec-driven body now
returns the more accurate not_authorized.
"""
dev_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="awaiting_qa",
plan={"x": 1},
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
task_svc = _task_svc_with(target, role="developer")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
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"]
assert "awaiting_qa" in body["message"]
@pytest.mark.asyncio
async def test_qa_cannot_claim_code_task_via_claim_review() -> None:
"""QA calling claim_review on PENDING task is rejected by claim-rules.
QA's CLAIM_RULES is {AWAITING_QA}. PENDING is owned by dev/pm — so
``_check_claim_rules_narrow`` returns ``not_authorized`` (the
"other_role_owns_status" branch). Pre-spec the verb body's status
pre-check returned invalid_state; post-migration the spec gate
drives the rejection kind.
"""
qa_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
quick_context=None,
)
task_svc = _task_svc_with(target, role="qa")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.claim_review(qa_id, task_id)
body = env.as_dict()
assert body["error"] == "not_authorized"
@pytest.mark.asyncio
async def test_documenter_cannot_claim_code_task_via_claim_doc_task() -> None:
"""Documenter calling claim_doc_task on AWAITING_QA task is rejected by claim-rules.
Documenter's CLAIM_RULES is {PENDING, AWAITING_DOCUMENTATION}. A
documenter calling claim_doc_task on AWAITING_QA hits the
"other_role_owns_status" branch and returns ``not_authorized``.
Pre-spec the verb body returned invalid_state on the status check;
post-migration the spec gate drives the rejection kind.
"""
doc_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="awaiting_qa",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
quick_context=None,
)
task_svc = _task_svc_with(target, role="documenter")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.claim_doc_task(doc_id, task_id)
body = env.as_dict()
assert body["error"] == "not_authorized"
@pytest.mark.asyncio
async def test_non_developer_role_cannot_claim_via_i_will_work_on() -> None:
"""Even if status would allow, a documenter calling i_will_work_on on pending
code task is blocked by role-typed claim gate."""
doc_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
)
task_svc = _task_svc_with(target, role="documenter")
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
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"
task_svc.claim.assert_not_awaited()
# ---------------------------------------------------------------------------
# Claim review (QA) — A.2/A.3 mirror
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_claim_review_blocks_when_qa_has_in_progress_task() -> None:
qa_id = uuid4()
task_id = uuid4()
other_id = uuid4()
target = MagicMock(
id=task_id,
status="awaiting_qa",
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
work_session_id=uuid4(),
branch_name="feature/backend/abc",
)
in_progress = MagicMock(id=other_id, status="in_progress")
task_svc = _task_svc_with(target, role="qa", lookups={"in_progress": [in_progress]})
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.claim_review(qa_id, task_id)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "i_am_done" in body["remediate"] or "i_am_idle" in body["remediate"]
task_svc.qa_claim.assert_not_awaited()
@pytest.mark.asyncio
async def test_claim_doc_task_blocks_when_documenter_has_paused_task() -> None:
doc_id = uuid4()
task_id = uuid4()
paused_id = uuid4()
target = MagicMock(
id=task_id,
status="awaiting_documentation",
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="backend",
work_session_id=uuid4(),
branch_name="feature/backend/abc",
)
paused = MagicMock(id=paused_id, status="paused")
task_svc = _task_svc_with(target, role="documenter", lookups={"paused": [paused]})
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.claim_doc_task(doc_id, task_id)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "resume" in body["remediate"].lower()
task_svc.doc_claim.assert_not_awaited()
# ---------------------------------------------------------------------------
# Coordinator exemption — a PM plans + delegates many roots in parallel, so the
# single-active-task guards (already_active / paused) must NOT gate it; only a
# real upstream sequence dependency may hold a PM's root back. (Developers stay
# blocked — see the A.2 / A.3 tests above.)
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_main_pm_can_plan_second_root_despite_active_and_paused() -> None:
"""A main_pm may plan a new root while it already holds one in_progress and
one paused root — the developer concurrency guards do not gate a coordinator.
"""
pm_id = uuid4()
task_id = uuid4()
target = MagicMock(
id=task_id,
status="pending",
plan=None,
assigned_to=None,
parent_task_id=None,
sequence=0,
task_type="code",
team="main_pm",
)
claimed = MagicMock(
id=task_id, status="claimed", plan=None, assigned_to=pm_id, task_type="code"
)
started = MagicMock(
id=task_id,
status="in_progress",
plan={"text": "x"},
assigned_to=pm_id,
task_type="code",
)
# The coordinator already holds one in_progress root and one paused root —
# both would trip the guards for a non-PM caller.
other_active = MagicMock(id=uuid4(), status="in_progress")
other_paused = MagicMock(id=uuid4(), status="paused")
task_svc = _task_svc_with(
target,
role="main_pm",
agent_id=pm_id,
lookups={"in_progress": [other_active], "paused": [other_paused]},
)
task_svc.claim.return_value = claimed
task_svc.set_plan.return_value = claimed
task_svc.start.return_value = started
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_plan(
pm_id,
task_id,
plan="route to backend + frontend cells",
rich_plan={
"approach": (
"Route this root to the backend and frontend cells in parallel: "
"be-pm owns the API contract, fe-pm consumes it. No cross-cell "
"dependency for this slice, so both cells start at once."
),
"sub_tasks": [
{
"title": "Backend slice",
"description": (
"be-pm decomposes the API change and assigns be-dev-1, "
"who implements with tests and opens the leaf PR for QA."
),
}
],
},
)
assert env.error is None, env.as_dict()
task_svc.start.assert_awaited()
@pytest.mark.asyncio
async def test_main_pm_recovers_claimed_root_with_paused_sibling() -> None:
"""The live deadlock: a respawned main_pm re-enters i_will_plan on a stuck
`claimed` root while another root is paused (i_am_idle auto-paused it). The
paused guard must NOT block the coordinator's recovery — set_plan + start
runs and the root reaches in_progress.
"""
pm_id = uuid4()
task_id = uuid4()
claimed = MagicMock(
id=task_id,
status="claimed",
plan=None,
assigned_to=pm_id,
parent_task_id=None,
sequence=0,
task_type="code",
team="main_pm",
branch_name="feature/main_pm/abc",
)
started = MagicMock(
id=task_id,
status="in_progress",
plan={"text": "x"},
assigned_to=pm_id,
task_type="code",
)
other_paused = MagicMock(id=uuid4(), status="paused")
task_svc = _task_svc_with(
claimed, role="main_pm", agent_id=pm_id, lookups={"paused": [other_paused]}
)
task_svc.set_plan.return_value = started
task_svc.start.return_value = started
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.i_will_plan(
pm_id, task_id, plan="route this root to the backend + frontend cells"
)
assert env.error is None, env.as_dict()
task_svc.start.assert_awaited_once_with(task_id, pm_id)
def test_paused_tasks_guard_excludes_target() -> None:
"""A paused task that IS the claim target must not self-block, mirroring
already_active_guard's target exclusion (the 2026-06-14 self-deadlock)."""
target_id = uuid4()
other_id = uuid4()
# Only the target itself is paused -> no block.
assert paused_tasks_guard([MagicMock(id=target_id)], target_id) is None
# A different paused task -> block, naming that task.
env = paused_tasks_guard([MagicMock(id=other_id)], target_id)
assert env is not None
assert str(other_id) in env.as_dict()["remediate"]
# Back-compat: with no target supplied, any paused task blocks.
assert paused_tasks_guard([MagicMock(id=other_id)]) is not None