mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(gateway): root-owned acceptance criteria via declare_coverage (#357)
The coverage gates had no vocabulary for criteria only the root itself can satisfy (the supersede PR from feature/main_pm/*, closing the contributor's PR): once a Main PM declared coverage for the legitimate cell criteria, the idle gate demanded a cell for the impossible ones too, so they got pushed into a cell task and the cell PM (correctly) escalated. declare_coverage now accepts the PM's own task: self-declared criteria count as claimed for the idle gate and satisfied for the roll-up (the roll-up actor is their owner by construction), surface as claimed_by=root in the briefing, and both PM prompts say to never hand a cell a criterion it cannot satisfy inside its own cell. Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -4,7 +4,7 @@ These are the only verbs the gateway will accept from you. Calling any
|
||||
other verb will be rejected with a Decision telling you the right one.
|
||||
|
||||
- **complete**: Cell PM merges the PR (leaf into the cell branch, or the gated cell→root PR into the root branch) + transitions to completed; Main PM escalates the root to the CEO (who merges root→master). The merge runs BEFORE the complete transition: TaskService.complete asserts the PR is already merged, so the choreographer verb body (cell_pm_complete / main_pm_complete) owns the merge-first ordering — no trailing pr_merge side_effect is declared here.
|
||||
- **declare_coverage**: Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact — for a replacement child whose delegate omitted covers_parent_criteria. No status change; the verb body owns ownership + criterion validation.
|
||||
- **declare_coverage**: Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact — for a replacement child whose delegate omitted covers_parent_criteria. Or, targeting your OWN root/coordination task, declare criteria as root-owned (only your own machinery satisfies them — never push these into a cell). No status change; the verb body owns ownership + criterion validation.
|
||||
- **delegate**: Create a subtask under the current task. Validates the delegation chain (main_pm->cell_pm; cell_pm->its team's devs) and the assignee-vs-task_type rule (Cell PMs get planning-typed tasks; devs get code/research, UX devs also design). documentation is NOT delegatable — the lifecycle auto-creates the doc phase after the code subtask passes QA.
|
||||
- **escalate_up**: Escalate to your role's escalation_target.
|
||||
- **give_me_work**: Return your most-actionable task or signal idle.
|
||||
|
||||
@@ -4,7 +4,7 @@ These are the only verbs the gateway will accept from you. Calling any
|
||||
other verb will be rejected with a Decision telling you the right one.
|
||||
|
||||
- **complete**: Cell PM merges the PR (leaf into the cell branch, or the gated cell→root PR into the root branch) + transitions to completed; Main PM escalates the root to the CEO (who merges root→master). The merge runs BEFORE the complete transition: TaskService.complete asserts the PR is already merged, so the choreographer verb body (cell_pm_complete / main_pm_complete) owns the merge-first ordering — no trailing pr_merge side_effect is declared here.
|
||||
- **declare_coverage**: Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact — for a replacement child whose delegate omitted covers_parent_criteria. No status change; the verb body owns ownership + criterion validation.
|
||||
- **declare_coverage**: Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact — for a replacement child whose delegate omitted covers_parent_criteria. Or, targeting your OWN root/coordination task, declare criteria as root-owned (only your own machinery satisfies them — never push these into a cell). No status change; the verb body owns ownership + criterion validation.
|
||||
- **delegate**: Create a subtask under the current task. Validates the delegation chain (main_pm->cell_pm; cell_pm->its team's devs) and the assignee-vs-task_type rule (Cell PMs get planning-typed tasks; devs get code/research, UX devs also design). documentation is NOT delegatable — the lifecycle auto-creates the doc phase after the code subtask passes QA.
|
||||
- **escalate_to_ceo**: Escalate to CEO with reason. Transitions to awaiting_ceo_approval.
|
||||
- **escalate_up**: Escalate to your role's escalation_target.
|
||||
|
||||
@@ -30,6 +30,7 @@ When the briefing carries `company_goals`, let the charter guide how you scope a
|
||||
| `delegate(parent_task_id, title, description, assigned_to, team, task_type, nature, acceptance_criteria, estimated_complexity, covers_parent_criteria?, intends_to_touch?, adds_migration?, touches_shared?, depends_on?)` | Create a subtask under your cell-PM task and assign it to a dev in your cell. `nature` ∈ `technical`/`non_technical`. `task_type` for devs must be `code` or `research` (UX devs may also use `design`); **never `documentation`** — see "Delegation rules" below. `covers_parent_criteria` is the list of YOUR criterion ids (from the briefing's `parent_ac_coverage`) this subtask satisfies — see "Coverage" below. `intends_to_touch`/`adds_migration`/`touches_shared` are the **collision surface** — `intends_to_touch` is **REQUIRED (non-empty) on every `code` subtask**; the gateway rejects a code delegation without it, because a no-surface sibling runs parallel to everything and ordered work executes out of order — see "Collision surface" below; fill them on every `code` subtask so the system can sequence sibling dev tasks that touch the same files into a conflict-free order. `depends_on` is a list of sibling subtask IDs this one must wait on (cross-reroute gate). Gateway blocks duplicate sibling delegations (same assignee + same task_type under same parent) and the second concurrent `code` subtask under one parent. | Parent claimed by you and `in_progress`; assignee is a dev slug in your cell. |
|
||||
| `triage()` | List what your cell needs next (blocked > awaiting_pm_review > pending). | None. |
|
||||
| `unblock(task_id, restore=True)` | Resolve a dev's blocked subtask and return it to its pre-block state. | Subtask is in your cell. |
|
||||
| `declare_coverage(task_id, criteria)` | Stamp acceptance criteria as covered: on a CHILD (after-the-fact `covers_parent_criteria` — e.g. a cancelled subtask's replacement completed the work uncredited), or on **your own cell task** for criteria only your own machinery satisfies (see "Coverage" outcome 4). | Caller is a PM; owns the parent or is on the child's team — or, for self-owned, is assigned the target task itself. |
|
||||
| `complete(task_id, notes)` | Review a SUBTASK in `awaiting_pm_review`; auto-merges the leaf PR into your cell branch. | All descendants of the subtask terminal; PR open and mergeable. |
|
||||
| `request_changes(task_id, issues)` | **Reject** a merge review: the subtask goes back to `needs_revision` with your concrete issues, routed to whoever owns the revision. Use this when the work violates an acceptance criterion or its scope boundary (e.g. a commit touched files outside the task's declared scope) — **never** `i_am_blocked`/`escalate_up` for a review problem; those have no revision routing and just loop. | Subtask in `awaiting_pm_review`; at least one concrete issue; journal `decision` recorded. |
|
||||
| `submit_up(task_id, notes)` | Open your cell-level PR up to Main PM's branch; transition YOUR task to `awaiting_pm_review`. | All your subtasks terminal; `notes` >= 20 chars; journal `decision` recorded. |
|
||||
@@ -127,11 +128,12 @@ If you must mention task IDs in a criterion, reference the **dev subtask ID** yo
|
||||
|
||||
Decomposition is where scope silently disappears. The failure mode: your cell-PM task carries N acceptance criteria, you delegate a subtask that covers 6 of them, and you idle — the other criteria have no subtask, no dev, no branch, and nobody notices until `submit_up` (or worse, QA/CEO) finds the gap. By then the whole cell has to loop.
|
||||
|
||||
**The rule: before you `i_am_idle()` after delegating, account for EVERY acceptance criterion on your cell-PM task.** Walk the list. For each criterion, name the subtask whose `acceptance_criteria` cover it. Three legal outcomes per criterion — and only three:
|
||||
**The rule: before you `i_am_idle()` after delegating, account for EVERY acceptance criterion on your cell-PM task.** Walk the list. For each criterion, name the subtask whose `acceptance_criteria` cover it. Four legal outcomes per criterion — and only four:
|
||||
|
||||
1. **Covered now** — a subtask you just delegated has an `acceptance_criteria` entry that satisfies it. Make the mapping **machine-explicit**: pass `covers_parent_criteria=[<criterion ids>]` on that `delegate` so the gateway records which of YOUR criteria the child owns. The criterion ids are in your briefing under `parent_ac_coverage` (each `{id, text, claimed, verified}`); the ones still without a home are listed in `unclaimed_parent_acs`. Phrase the child's criteria so a reader can also trace each back by eye.
|
||||
2. **Covered later, in sequence** — it belongs to a follow-on subtask that runs after the current one. Delegate that follow-on **now too**, placed later in the same dev's queue (a dev can hold a queue), so the criterion is claimed immediately and simply builds in turn. Record the sequencing in your `decision` note ("criterion 7 → be-dev-1's second queue item, after the first lands") so the order is intentional and visible.
|
||||
3. **Out of scope for your cell** — it genuinely belongs to another cell or the Main PM aggregate. Say so in the `decision` note. Do not silently drop it.
|
||||
4. **Cell-owned** — only YOUR own machinery satisfies it (never a dev's), the same principle one level up applies here too: declare it root-owned on your own task, `declare_coverage(task_id=<your own cell-PM task>, criteria=[<ids>])`. Never put it in a dev's `acceptance_criteria` — a dev can't act outside their own branch/PR.
|
||||
|
||||
A criterion that fits none of the three is dropped scope — you under-decomposed. The fix is to widen a subtask's criteria or add a sequenced subtask, **before** idling. Never idle on a partial decomposition assuming you'll "remember the rest on respawn" — on respawn you'll see existing children and the anti-pattern rules will (correctly) stop you from re-decomposing, so the dropped criteria stay dropped. Map coverage now, while you still can.
|
||||
|
||||
|
||||
@@ -52,7 +52,8 @@ This is the single most common mental-model mistake at your seat. Get it right:
|
||||
|---|---|---|
|
||||
| `give_me_work()` | Returns your highest-priority task (your root in `pending`, or a cell-PM task in `awaiting_pm_review` for you to merge). | None. |
|
||||
| `i_will_plan(task_id, plan, approach, sub_tasks, technical_considerations?, risks?, open_questions?)` | Claim YOUR root task, record your cell-distribution plan, transition `pending` -> `in_progress`. Always call this before `delegate`. **The gate REJECTS thin plans:** `approach` must be **≥150 chars** describing HOW you split work across cells + sequencing + dependencies (not a one-liner); `sub_tasks` is a non-empty list of `{title, description}` where **every `description` is ≥60 chars** stating what that cell slice delivers — each sub_task is both a `delegate` target AND a progress-checklist item. Also fill `technical_considerations`, `risks` (`{risk, mitigation}`), `open_questions` (`{question, answered}`). Empty/thin values are rejected, not just an empty Plan tab. | Task assigned to you; task in `pending`/`needs_revision`. |
|
||||
| `delegate(parent_task_id, title, description, assigned_to, team, task_type, nature, acceptance_criteria, estimated_complexity, covers_parent_criteria?)` | Create a subtask under your root and assign it to a Cell PM (`be-pm`, `fe-pm`, `ux-pm`). One subtask per cell that needs work. **`task_type` must be `planning`** (Cell PMs decompose; they don't execute). `nature` ∈ `technical`/`non_technical`. `covers_parent_criteria` is the list of YOUR root criterion ids (from the briefing's `parent_ac_coverage`) this cell now owns — map every root criterion to a cell before you idle. Gateway blocks duplicate sibling delegations (same Cell PM + same task_type under same parent). | Parent claimed by you and `in_progress`; assignee is a Cell PM slug. |
|
||||
| `delegate(parent_task_id, title, description, assigned_to, team, task_type, nature, acceptance_criteria, estimated_complexity, covers_parent_criteria?)` | Create a subtask under your root and assign it to a Cell PM (`be-pm`, `fe-pm`, `ux-pm`). One subtask per cell that needs work. **`task_type` must be `planning`** (Cell PMs decompose; they don't execute). `nature` ∈ `technical`/`non_technical`. `covers_parent_criteria` is the list of YOUR root criterion ids (from the briefing's `parent_ac_coverage`) this cell now owns — map every root criterion to a cell before you idle. **Never** put a criterion only YOU can satisfy in `covers_parent_criteria` (see `declare_coverage` below) — a cell cannot act outside its own branch/PR. Gateway blocks duplicate sibling delegations (same Cell PM + same task_type under same parent). | Parent claimed by you and `in_progress`; assignee is a Cell PM slug. |
|
||||
| `declare_coverage(task_id, criteria)` | Stamp acceptance criteria as covered: on a CHILD (after-the-fact `covers_parent_criteria` for a replacement subtask), or on **your own root** `task_id` for criteria only your own machinery satisfies — PR-supersede, closing a contributor PR, a root-level merge. Root-owned criteria never belong on a cell; they count as claimed+satisfied for both `i_am_idle` and the roll-up gate without any cell touching them. | Caller is a PM; owns the parent (or is on the child's team) — or, for root-owned, is assigned the target task itself. |
|
||||
| `triage_all()` | List blockers and reviews across all cells. | None. |
|
||||
| `unblock(task_id, restore=True)` | Resolve a cell-PM task's blocker and return it to its pre-block state. | None. |
|
||||
| `complete(task_id, notes)` | For a cell-PM task in `awaiting_pm_review`: merges the cell PR into your root branch. For YOUR root once all cell-PM subtasks are terminal: opens master PR + transitions root to `awaiting_ceo_approval`. | All descendants terminal; journal `decision` recorded. |
|
||||
@@ -136,6 +137,8 @@ The description is a **brief**, not a spec. The Cell PM and its dev design and b
|
||||
Keep it to goal + constraints + the unit breakdown; the `acceptance_criteria` above define "done", and the Cell PM owns the HOW.
|
||||
|
||||
**Map your root's criteria to the cell subtask that owns them.** Your briefing carries `parent_ac_coverage` (each root criterion as `{id, text, claimed, verified}`) and `unclaimed_parent_acs` (the ids with no cell subtask yet). When you `delegate` a slice to a cell, pass `covers_parent_criteria=[<root criterion ids>]` naming which root criteria that cell now owns — every root criterion must be claimed by some cell before you idle. Once you start declaring coverage, the gateway **rejects `i_am_idle()`** while `unclaimed_parent_acs` is non-empty, naming the gap; the fix is one more `delegate` to the cell that should own it. (Opt-in: if you never pass `covers_parent_criteria` the gate stays silent, but declaring it is how a dropped cross-cell criterion gets caught here instead of at the CEO.)
|
||||
|
||||
**Some root criteria are yours alone — never delegate them.** A criterion satisfiable only by your own machinery (e.g. "a PR is opened from `feature/main_pm/...`", "contributor PR #N is closed and linked") cannot be honored by any cell — a cell can't operate in your branch namespace or close a PR it doesn't own. Do NOT push it into a cell's `acceptance_criteria` or `covers_parent_criteria`; declare it root-owned instead: `declare_coverage(task_id=<your own root>, criteria=[<ids>])`. `parent_ac_coverage` then shows `claimed_by: "root"` for it, and it counts as claimed+satisfied for `i_am_idle` and the roll-up gate — no cell involved.
|
||||
6. `i_am_idle()` -> wait. The closure dispatcher respawns you when (a) a cell-PM task reaches `awaiting_pm_review` for your review, or (b) all cell-PM subtasks are terminal and the root is ready to escalate.
|
||||
7. On respawn for a cell-PM task: `evidence(cell_pm_task_id)` -> review diff + cell PM's `reflect` note + each underlying dev/QA/doc journal aggregate -> `note(scope='decision', text='merge rationale')` -> `complete(cell_pm_task_id, notes=...)`. The cell PR auto-merges into your root branch.
|
||||
8. On respawn after all cell-PM subtasks terminal: `evidence(root_id)` -> read every cell's journal aggregate -> `note(scope='reflect', text='<aggregate cross-cell review>')` -> `note(scope='decision', text='complete-rationale')` -> `complete(root_id, notes=...)`. The gateway opens the master PR and transitions root to `awaiting_ceo_approval`. CEO takes it from there.
|
||||
|
||||
@@ -49,7 +49,7 @@ Cell PM merges the PR (leaf into the cell branch, or the gated cell→root PR in
|
||||
|
||||
## declare_coverage
|
||||
|
||||
Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact — for a replacement child whose delegate omitted covers_parent_criteria. No status change; the verb body owns ownership + criterion validation.
|
||||
Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact — for a replacement child whose delegate omitted covers_parent_criteria. Or, targeting your OWN root/coordination task, declare criteria as root-owned (only your own machinery satisfies them — never push these into a cell). No status change; the verb body owns ownership + criterion validation.
|
||||
|
||||
**Allowed roles:** cell_pm, main_pm
|
||||
|
||||
|
||||
@@ -91,7 +91,7 @@
|
||||
"main_pm"
|
||||
],
|
||||
"composes": [],
|
||||
"description": "Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact \u2014 for a replacement child whose delegate omitted covers_parent_criteria. No status change; the verb body owns ownership + criterion validation.",
|
||||
"description": "Stamp parent acceptance criteria onto an existing child's parent_ac_refs after the fact \u2014 for a replacement child whose delegate omitted covers_parent_criteria. Or, targeting your OWN root/coordination task, declare criteria as root-owned (only your own machinery satisfies them \u2014 never push these into a cell). No status change; the verb body owns ownership + criterion validation.",
|
||||
"name": "declare_coverage",
|
||||
"pre_side_effects": [],
|
||||
"side_effects": []
|
||||
|
||||
@@ -146,10 +146,11 @@ class ReassignRequest(BaseModel):
|
||||
class DeclareCoverageRequest(BaseModel):
|
||||
"""HTTP body for the cell_pm/main_pm `declare_coverage` verb.
|
||||
|
||||
``task_id`` is the CHILD to stamp; ``criteria`` are parent acceptance
|
||||
criteria (id or exact text — same representation as `delegate`'s
|
||||
`covers_parent_criteria`). The choreographer validates ownership +
|
||||
unknown criteria.
|
||||
``task_id`` is the CHILD to stamp (or the caller's OWN root/coordination
|
||||
task, for root-owned criteria); ``criteria`` are that task's parent's
|
||||
acceptance criteria — or its own, in root-owned mode — by id or exact
|
||||
text (same representation as `delegate`'s `covers_parent_criteria`). The
|
||||
choreographer validates ownership + unknown criteria.
|
||||
"""
|
||||
|
||||
task_id: UUID
|
||||
|
||||
@@ -1298,8 +1298,11 @@ _INTENT_VERBS: dict[str, IntentSpec] = {
|
||||
description=(
|
||||
"Stamp parent acceptance criteria onto an existing child's"
|
||||
" parent_ac_refs after the fact — for a replacement child whose"
|
||||
" delegate omitted covers_parent_criteria. No status change; the"
|
||||
" verb body owns ownership + criterion validation."
|
||||
" delegate omitted covers_parent_criteria. Or, targeting your OWN"
|
||||
" root/coordination task, declare criteria as root-owned (only"
|
||||
" your own machinery satisfies them — never push these into a"
|
||||
" cell). No status change; the verb body owns ownership +"
|
||||
" criterion validation."
|
||||
),
|
||||
composes=(), # special — no transition, just an AC-ref write + audit
|
||||
extra_preconditions=(),
|
||||
|
||||
@@ -793,6 +793,12 @@ def declare_coverage(task_id: str, criteria: StrList) -> dict[str, Any]:
|
||||
criteria, by id or exact text (copy them straight out of the gate's
|
||||
rejection listing). Returns evidence.remaining_uncovered_parent_acs so you
|
||||
know if submit_up will now pass.
|
||||
|
||||
Root-owned mode: pass YOUR OWN root/coordination task_id (not a child)
|
||||
to declare criteria only you can satisfy — PR ops in your own branch
|
||||
namespace, closing a contributor PR, a root-level merge. These are
|
||||
satisfied by your own machinery at submit/supersede time; never push
|
||||
them into a cell's acceptance_criteria, a cell cannot act on them.
|
||||
"""
|
||||
return _post(
|
||||
_role_path("declare_coverage"),
|
||||
|
||||
@@ -3731,6 +3731,15 @@ class Choreographer:
|
||||
when ``rejection`` is ``None``. Ownership mirrors submit_up (the PM
|
||||
assigned to the parent coordination task), with a fallback for any
|
||||
PM on the child's own team — the minimum bar the spec asks for.
|
||||
|
||||
Root-owned self-declare: when the caller is the assignee of
|
||||
``child`` itself, ``task_id`` names the PM's OWN root/coordination
|
||||
task, not a child — criteria only the root's own machinery can
|
||||
satisfy (PR-supersede, closing a contributor PR, a root-level
|
||||
merge) that must never be pushed into a cell's acceptance criteria.
|
||||
``parent`` is then ``child`` itself (caller-identity is the
|
||||
signal — callers never target their own claimed task in the
|
||||
ordinary child-declare flow, so this cannot misfire on it).
|
||||
"""
|
||||
if agent is None or agent.role not in ("cell_pm", "main_pm"):
|
||||
return (
|
||||
@@ -3741,6 +3750,8 @@ class Choreographer:
|
||||
),
|
||||
None,
|
||||
)
|
||||
if child.assigned_to == pm_agent_id:
|
||||
return None, child
|
||||
if not child.parent_task_id:
|
||||
return (
|
||||
Envelope.invalid_state(
|
||||
@@ -3796,6 +3807,12 @@ class Choreographer:
|
||||
resolves it at read time), so a PM can copy straight out of the
|
||||
gate's own uncovered-criteria listing. ``task_id`` is the CHILD
|
||||
(any non-cancelled status — the live case is a completed child).
|
||||
|
||||
Root-owned mode: ``task_id`` may instead be the PM's OWN root, for
|
||||
criteria only the root itself satisfies (never delegable to a
|
||||
cell — see ``_declare_coverage_guard``). The refs land in that
|
||||
task's own ``parent_ac_refs`` and are read back against its own
|
||||
criteria, satisfied unconditionally (no child/status to wait on).
|
||||
"""
|
||||
child = await self.task.get(task_id)
|
||||
briefing = await self._briefing_for(pm_agent_id, task_id, task=child)
|
||||
@@ -3837,11 +3854,14 @@ class Choreographer:
|
||||
updated = await self.task.add_parent_ac_refs(
|
||||
task_id, criteria, declared_by=pm_agent_id
|
||||
)
|
||||
# Self-declare targets its own uncovered-set (parent is child by
|
||||
# identity, see the guard); child-declare targets the real parent.
|
||||
uncovered_target = task_id if parent is child else child.parent_task_id
|
||||
uncovered = await self.task.uncovered_parent_acceptance_criteria(
|
||||
child.parent_task_id
|
||||
uncovered_target
|
||||
)
|
||||
next_hint = (
|
||||
"submit_up's roll-up gate will pass on the parent now"
|
||||
"the roll-up gate will pass now"
|
||||
if not uncovered
|
||||
else f"{len(uncovered)} parent ACs still uncovered: {'; '.join(uncovered)}"
|
||||
)
|
||||
|
||||
+39
-9
@@ -6330,7 +6330,7 @@ class TaskService(BaseService):
|
||||
loaded = await self._parent_ac_ref_sets(cast("UUID", t.parent_task_id))
|
||||
if loaded is None:
|
||||
continue
|
||||
parent, claimed, _verified, _any = loaded
|
||||
parent, claimed, _verified, _any, _root_owned = loaded
|
||||
own = self._normalize_ac_refs(parent, t.parent_ac_refs)
|
||||
orphaned_ids = own - claimed
|
||||
if orphaned_ids:
|
||||
@@ -8136,17 +8136,25 @@ class TaskService(BaseService):
|
||||
|
||||
async def _parent_ac_ref_sets(
|
||||
self, task_id: UUID
|
||||
) -> tuple[TaskTable, set[str], set[str], bool] | None:
|
||||
) -> tuple[TaskTable, set[str], set[str], bool, set[str]] | None:
|
||||
"""Load a parent and its children's parent-AC-ref coverage sets.
|
||||
|
||||
Shared core of the three AC-coverage primitives. Returns ``None`` when
|
||||
the parent is missing or has no stable criterion ids (nothing to cover).
|
||||
Otherwise ``(parent, claimed, verified, any_declared)`` where
|
||||
``claimed`` is the union of parent_ac_refs over all non-cancelled
|
||||
Otherwise ``(parent, claimed, verified, any_declared, root_owned)``
|
||||
where ``claimed`` is the union of parent_ac_refs over all non-cancelled
|
||||
children, ``verified`` the union over COMPLETED children only, and
|
||||
``any_declared`` whether *any* child declared a ref at all (the
|
||||
safe-by-construction inertness signal — a cancelled-only declaration
|
||||
still counts as "coverage tracking is active here").
|
||||
|
||||
``root_owned`` is the parent's OWN ``parent_ac_refs`` (declared on
|
||||
itself via ``declare_coverage(task_id=<own root>, ...)``) — criteria
|
||||
only the root's own machinery satisfies (e.g. PR-supersede, closing a
|
||||
contributor PR), never a cell. Root-owned refs are unconditionally
|
||||
folded into both ``claimed`` and ``verified``: there is no child
|
||||
status to gate on, the work happens at/after the root's own
|
||||
submit/supersede by construction.
|
||||
"""
|
||||
parent = await self.get(task_id)
|
||||
if not parent or not parent.acceptance_criteria_ids:
|
||||
@@ -8166,7 +8174,12 @@ class TaskService(BaseService):
|
||||
claimed |= refset
|
||||
if status == TaskStatus.COMPLETED:
|
||||
verified |= refset
|
||||
return parent, claimed, verified, any_declared
|
||||
root_owned = self._normalize_ac_refs(parent, parent.parent_ac_refs)
|
||||
if root_owned:
|
||||
any_declared = True
|
||||
claimed |= root_owned
|
||||
verified |= root_owned
|
||||
return parent, claimed, verified, any_declared, root_owned
|
||||
|
||||
@staticmethod
|
||||
def _normalize_ac_refs(parent: TaskTable, refs: list[str] | None) -> set[str]:
|
||||
@@ -8229,7 +8242,7 @@ class TaskService(BaseService):
|
||||
async def add_parent_ac_refs(
|
||||
self, task_id: UUID, refs: list[str], declared_by: UUID | None = None
|
||||
) -> TaskTable | None:
|
||||
"""UNION ``refs`` into a child's ``parent_ac_refs`` (idempotent) + audit.
|
||||
"""UNION ``refs`` into a task's ``parent_ac_refs`` (idempotent) + audit.
|
||||
|
||||
Lets a PM stamp coverage onto an already-existing child after the
|
||||
fact — e.g. a completed replacement whose original ``delegate``
|
||||
@@ -8239,6 +8252,12 @@ class TaskService(BaseService):
|
||||
time by ``_normalize_ac_refs``. Callers must validate refs against
|
||||
the parent's criteria first (``unknown_ac_refs``) — this method
|
||||
merges unconditionally.
|
||||
|
||||
Agnostic to whether ``task_id`` is a CHILD (refs resolve against its
|
||||
real parent's criteria) or a task declaring root-owned coverage on
|
||||
ITSELF (refs resolve against its own criteria, read back by
|
||||
``_parent_ac_ref_sets`` when ``task_id`` is later queried as a
|
||||
parent) — this method only ever writes the one row's own field.
|
||||
"""
|
||||
from roboco.db.tables import AuditLogTable
|
||||
|
||||
@@ -8276,7 +8295,7 @@ class TaskService(BaseService):
|
||||
loaded = await self._parent_ac_ref_sets(task_id)
|
||||
if loaded is None:
|
||||
return []
|
||||
parent, _claimed, verified, any_declared = loaded
|
||||
parent, _claimed, verified, any_declared, _root_owned = loaded
|
||||
if not any_declared:
|
||||
return []
|
||||
return self._criteria_texts_not_in(parent, verified)
|
||||
@@ -8297,7 +8316,7 @@ class TaskService(BaseService):
|
||||
loaded = await self._parent_ac_ref_sets(task_id)
|
||||
if loaded is None:
|
||||
return []
|
||||
parent, claimed, _verified, any_declared = loaded
|
||||
parent, claimed, _verified, any_declared, _root_owned = loaded
|
||||
if not any_declared:
|
||||
return []
|
||||
return self._criteria_texts_not_in(parent, claimed)
|
||||
@@ -8313,11 +8332,15 @@ class TaskService(BaseService):
|
||||
it always reports the parent's criteria so a decomposing PM can see, per
|
||||
delegate, which criteria still lack a subtask. Visibility source for the
|
||||
decomposition briefing; the gates read the inert variants instead.
|
||||
|
||||
``claimed_by`` distinguishes a root-owned criterion (declared on the
|
||||
parent itself — ``"root"``) from one claimed by a child (``"child"``)
|
||||
or not claimed at all (``None``), so the PM can see the mapping.
|
||||
"""
|
||||
loaded = await self._parent_ac_ref_sets(task_id)
|
||||
if loaded is None:
|
||||
return []
|
||||
parent, claimed, verified, _any = loaded
|
||||
parent, claimed, verified, _any, root_owned = loaded
|
||||
ids = parent.acceptance_criteria_ids or []
|
||||
texts = parent.acceptance_criteria or []
|
||||
return [
|
||||
@@ -8326,6 +8349,13 @@ class TaskService(BaseService):
|
||||
"text": texts[idx] if idx < len(texts) else ac_id,
|
||||
"claimed": ac_id in claimed,
|
||||
"verified": ac_id in verified,
|
||||
"claimed_by": (
|
||||
"root"
|
||||
if ac_id in root_owned
|
||||
else "child"
|
||||
if ac_id in claimed
|
||||
else None
|
||||
),
|
||||
}
|
||||
for idx, ac_id in enumerate(ids)
|
||||
]
|
||||
|
||||
@@ -1646,6 +1646,54 @@ async def test_add_parent_ac_refs_returns_none_for_missing_task(
|
||||
assert await svc.add_parent_ac_refs(uuid4(), ["id-a"]) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_parent_ac_refs_on_own_id_is_root_owned(task_setup: dict) -> None:
|
||||
"""declare_coverage's self-declare mode: stamping a task's OWN id (not a
|
||||
child's) writes into ITS OWN parent_ac_refs -- root-owned, satisfied
|
||||
unconditionally without any child."""
|
||||
svc = task_setup["svc"]
|
||||
parent = await svc.create(
|
||||
_req(task_setup, acceptance_criteria=["crit a", "crit b"])
|
||||
)
|
||||
ac_a, ac_b = parent.acceptance_criteria_ids
|
||||
|
||||
updated = await svc.add_parent_ac_refs(
|
||||
parent.id, [ac_a], declared_by=task_setup["agent_id"]
|
||||
)
|
||||
assert updated is not None
|
||||
assert await svc.unclaimed_parent_acceptance_criteria(parent.id) == ["crit b"]
|
||||
assert await svc.uncovered_parent_acceptance_criteria(parent.id) == ["crit b"]
|
||||
cov = await svc.parent_ac_coverage(parent.id)
|
||||
assert cov[0]["claimed_by"] == "root"
|
||||
assert cov[1]["claimed_by"] is None
|
||||
_ = ac_b
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_production_replay_root_with_child_and_root_owned_coverage(
|
||||
task_setup: dict, db_session: AsyncSession
|
||||
) -> None:
|
||||
"""The live pattern: a Main-PM root with 5 ACs -- 3 satisfied by a
|
||||
completed cell subtask, 2 that only the root's own machinery satisfies
|
||||
(PR-supersede, closing a contributor PR) and are declared root-owned.
|
||||
Both the idle gate and the roll-up gate must pass."""
|
||||
svc = task_setup["svc"]
|
||||
parent = await svc.create(
|
||||
_req(task_setup, acceptance_criteria=[f"crit {i}" for i in range(5)])
|
||||
)
|
||||
ids = parent.acceptance_criteria_ids
|
||||
child = await svc.create(
|
||||
_req(task_setup, parent_task_id=parent.id, parent_ac_refs=ids[:3])
|
||||
)
|
||||
child.status = TaskStatus.COMPLETED
|
||||
await db_session.flush()
|
||||
|
||||
await svc.add_parent_ac_refs(parent.id, ids[3:], declared_by=task_setup["agent_id"])
|
||||
|
||||
assert await svc.unclaimed_parent_acceptance_criteria(parent.id) == []
|
||||
assert await svc.uncovered_parent_acceptance_criteria(parent.id) == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _unblock_dependents
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -1320,3 +1320,65 @@ async def test_declare_coverage_then_submit_up_gate_passes() -> None:
|
||||
pm_id, parent_id, context_phrase="bubbling up"
|
||||
)
|
||||
assert gate_env is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# declare_coverage — root-owned self-declare
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_self_declare_accepted_for_owning_pm() -> None:
|
||||
"""Root-owned mode: task_id is the PM's OWN root (assigned_to ==
|
||||
pm_agent_id) — parent is child by identity (the self-declare signal), no
|
||||
separate parent fetch, criteria validate against the root's own ACs."""
|
||||
pm_id = uuid4()
|
||||
root_id = uuid4()
|
||||
root = MagicMock(
|
||||
id=root_id,
|
||||
assigned_to=pm_id,
|
||||
parent_task_id=None,
|
||||
acceptance_criteria=["crit a", "crit b"],
|
||||
acceptance_criteria_ids=["id-a", "id-b"],
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = root
|
||||
task_svc.agent_for.return_value = MagicMock(role="main_pm", team="board")
|
||||
task_svc.unknown_ac_refs = MagicMock(return_value=[])
|
||||
task_svc.add_parent_ac_refs.return_value = root
|
||||
task_svc.uncovered_parent_acceptance_criteria.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, root_id, ["id-a"])
|
||||
assert env.error is None, env.as_dict()
|
||||
task_svc.add_parent_ac_refs.assert_awaited_once_with(
|
||||
root_id, ["id-a"], declared_by=pm_id
|
||||
)
|
||||
# Self-declare targets its own uncovered-set, never a parent_task_id.
|
||||
task_svc.uncovered_parent_acceptance_criteria.assert_awaited_once_with(root_id)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_self_declare_rejected_for_non_owner() -> None:
|
||||
"""A PM who is not the assignee of the target root cannot self-declare on
|
||||
it — falls through to the ordinary child-declare path, which requires a
|
||||
parent_task_id; a root has none, so it is rejected."""
|
||||
pm_id, owner_id = uuid4(), uuid4()
|
||||
root_id = uuid4()
|
||||
root = MagicMock(
|
||||
id=root_id,
|
||||
assigned_to=owner_id,
|
||||
parent_task_id=None,
|
||||
acceptance_criteria=["crit a"],
|
||||
acceptance_criteria_ids=["id-a"],
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = root
|
||||
task_svc.agent_for.return_value = MagicMock(role="main_pm", team="board")
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, root_id, ["id-a"])
|
||||
assert env.error == "invalid_state"
|
||||
task_svc.add_parent_ac_refs.assert_not_awaited()
|
||||
|
||||
@@ -1113,6 +1113,7 @@ async def test_parent_ac_coverage_normalizes_text_refs() -> None:
|
||||
parent = _build_task(
|
||||
acceptance_criteria=["crit a", "crit b"],
|
||||
acceptance_criteria_ids=["id-a", "id-b"],
|
||||
parent_ac_refs=[],
|
||||
)
|
||||
svc = _svc_with_children(parent, [(TaskStatus.COMPLETED, ["crit a"])])
|
||||
cov = await svc.parent_ac_coverage(parent.id)
|
||||
@@ -1121,6 +1122,7 @@ async def test_parent_ac_coverage_normalizes_text_refs() -> None:
|
||||
"text": "crit a",
|
||||
"claimed": True,
|
||||
"verified": True,
|
||||
"claimed_by": "child",
|
||||
}
|
||||
|
||||
|
||||
@@ -1132,6 +1134,7 @@ async def test_parent_ac_coverage_maps_claimed_and_verified() -> None:
|
||||
parent = _build_task(
|
||||
acceptance_criteria=["crit a", "crit b", "crit c"],
|
||||
acceptance_criteria_ids=["id-a", "id-b", "id-c"],
|
||||
parent_ac_refs=[],
|
||||
)
|
||||
svc = _svc_with_children(
|
||||
parent,
|
||||
@@ -1141,9 +1144,27 @@ async def test_parent_ac_coverage_maps_claimed_and_verified() -> None:
|
||||
],
|
||||
)
|
||||
assert await svc.parent_ac_coverage(parent.id) == [
|
||||
{"id": "id-a", "text": "crit a", "claimed": True, "verified": True},
|
||||
{"id": "id-b", "text": "crit b", "claimed": True, "verified": False},
|
||||
{"id": "id-c", "text": "crit c", "claimed": False, "verified": False},
|
||||
{
|
||||
"id": "id-a",
|
||||
"text": "crit a",
|
||||
"claimed": True,
|
||||
"verified": True,
|
||||
"claimed_by": "child",
|
||||
},
|
||||
{
|
||||
"id": "id-b",
|
||||
"text": "crit b",
|
||||
"claimed": True,
|
||||
"verified": False,
|
||||
"claimed_by": "child",
|
||||
},
|
||||
{
|
||||
"id": "id-c",
|
||||
"text": "crit c",
|
||||
"claimed": False,
|
||||
"verified": False,
|
||||
"claimed_by": None,
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
@@ -1156,6 +1177,85 @@ async def test_parent_ac_coverage_empty_without_ac_ids() -> None:
|
||||
assert await svc.parent_ac_coverage(parent.id) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_parent_ac_coverage_marks_root_owned_claimed_by() -> None:
|
||||
# A criterion the parent declared on ITS OWN parent_ac_refs (root-owned,
|
||||
# via declare_coverage(task_id=<own root>, ...)) reads claimed_by="root";
|
||||
# one claimed only via a child reads "child"; untouched is None.
|
||||
parent = _build_task(
|
||||
acceptance_criteria=["crit a", "crit b", "crit c"],
|
||||
acceptance_criteria_ids=["id-a", "id-b", "id-c"],
|
||||
parent_ac_refs=["id-a"],
|
||||
)
|
||||
svc = _svc_with_children(parent, [(TaskStatus.COMPLETED, ["id-b"])])
|
||||
assert await svc.parent_ac_coverage(parent.id) == [
|
||||
{
|
||||
"id": "id-a",
|
||||
"text": "crit a",
|
||||
"claimed": True,
|
||||
"verified": True,
|
||||
"claimed_by": "root",
|
||||
},
|
||||
{
|
||||
"id": "id-b",
|
||||
"text": "crit b",
|
||||
"claimed": True,
|
||||
"verified": True,
|
||||
"claimed_by": "child",
|
||||
},
|
||||
{
|
||||
"id": "id-c",
|
||||
"text": "crit c",
|
||||
"claimed": False,
|
||||
"verified": False,
|
||||
"claimed_by": None,
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_uncovered_parent_acs_root_owned_satisfied_without_child() -> None:
|
||||
# Root-owned refs are satisfied unconditionally -- no child, no COMPLETED
|
||||
# status to wait on. The root's own machinery (PR-supersede, closing a
|
||||
# contributor PR) does the work at/after submit, not a cell.
|
||||
parent = _build_task(
|
||||
acceptance_criteria=["crit a", "crit b"],
|
||||
acceptance_criteria_ids=["id-a", "id-b"],
|
||||
parent_ac_refs=["id-a"],
|
||||
)
|
||||
svc = _svc_with_children(parent, [])
|
||||
assert await svc.uncovered_parent_acceptance_criteria(parent.id) == ["crit b"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unclaimed_parent_acs_root_owned_counts_as_claimed() -> None:
|
||||
parent = _build_task(
|
||||
acceptance_criteria=["crit a", "crit b"],
|
||||
acceptance_criteria_ids=["id-a", "id-b"],
|
||||
parent_ac_refs=["id-a"],
|
||||
)
|
||||
svc = _svc_with_children(parent, [])
|
||||
assert await svc.unclaimed_parent_acceptance_criteria(parent.id) == ["crit b"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_production_replay_mixed_child_and_root_owned_coverage() -> None:
|
||||
# The live pattern: a Main-PM root with 5 ACs -- 3 satisfied by a
|
||||
# completed cell subtask, 2 that only the root's own machinery can
|
||||
# satisfy (PR-supersede, closing a contributor PR) and are declared
|
||||
# root-owned instead of pushed into the cell's acceptance_criteria. Both
|
||||
# the idle gate (unclaimed) and the roll-up gate (uncovered) must pass.
|
||||
ids = [f"id-{i}" for i in range(5)]
|
||||
parent = _build_task(
|
||||
acceptance_criteria=[f"crit {i}" for i in range(5)],
|
||||
acceptance_criteria_ids=ids,
|
||||
parent_ac_refs=["id-3", "id-4"],
|
||||
)
|
||||
svc = _svc_with_children(parent, [(TaskStatus.COMPLETED, ["id-0", "id-1", "id-2"])])
|
||||
assert await svc.unclaimed_parent_acceptance_criteria(parent.id) == []
|
||||
assert await svc.uncovered_parent_acceptance_criteria(parent.id) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unclaimed_parent_acs_inert_without_declared_coverage() -> None:
|
||||
# The decomposition floor is opt-in: with no child declaring parent_ac_refs
|
||||
|
||||
Reference in New Issue
Block a user