mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* feat(lifecycle): revision findings ledger — structured QA/PR/PM/CEO failure feedback, persisted and delivered down the chain Every bounce used to survive only as flattened prose: rounds overwrote each other in notes_structured, request_changes persisted nothing, two raw dev_notes appends were silently destroyed by the next handoff note, and the dev prompt pointed at fields (qa_notes via evidence(), pm_notes) the API never delivered. Agents re-interpreted and re-discovered every failure before they could start fixing it. - task_review_findings (migration 071, append-only): file/line/severity/ criterion(AC-id-validated)/expected/actual/fix/evidence per finding, with origin (qa|pr_gate|pm|ceo), round, and an open->addressed->verified lifecycle (waived reserved); new tasks.pm_notes + PmReviewContent give request_changes a structured home - producers: fail_review/pr_fail/request_changes take findings=[...] (prose issues shimmed+merged for one release, deprecation-logged); ceo_reject validates its reason (no 500), lands an origin=ceo finding, and bumps round+audit on branchless coordination roots; guardrails at the verb chokepoint (nudge >5, hard reject >10, field caps, traversal-safe file); the dev_notes data-loss appends are removed; new task.request_changes + task.ceo_reject audit events close rework attribution - delivery: qa_notes/pr_reviewer_notes/pm_notes carry the deterministic [F-id8] rendering; claim briefings, evidence(), the REVISION_REQUIRED spawn prompt, PM triage bounced-blocks, and A2A bodies deliver open findings; round-N+1 QA and gate reviewers get the full prior ledger; panel Findings tab + bounced-xN chip; metrics pm_rejects/ceo_rejects + findings counts; vault task notes render a Findings section (fail-open) - resolution closes for every origin: i_am_done and submit_up/submit_root take resolved_findings gated by FINDINGS_ADDRESSED (owner-gated so a stale non-owner PM can never mutate the ledger); pass_review/pr_pass/ complete verify-stamp same-transaction; ceo_approve stamps best-effort - 24 real-DB integration tests drive the full loop through the real choreographer; full suite 12856 green * docs: revision findings ledger sweep — CLAUDE.md, map, RAG corpus - CLAUDE.md: new ledger section + corrected request_changes row - docs/map/review-findings.md (new subsystem map) + surgical updates to task-service/pr-gate-review/metrics-observability/vault/panel maps - docs/rag: producers' findings contract across qa/pr-reviewer/developer/ cell-pm/main-pm/ceo role docs (the PM docs were missing request_changes entirely), verb references, and a new architecture/review-findings.md disambiguating ledger findings from convention findings * test(e2e): resubmit resolves the pr_fail finding per the ledger contract The scripted pr_fail revision loop resubmitted submit_up without resolved_findings — correctly rejected now that FINDINGS_ADDRESSED gates the PM resubmit verbs (green locally, red only in CI since the e2e suite skips without ROBOCO_E2E_SMOKE=1). The scripted PM now reads the open ledger row pr_fail persisted (new open_finding_ids arc helper) and resolves it on resubmit, asserting the open set drains — exercising the coordinator half of the new contract end to end. --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
241 lines
8.5 KiB
Python
241 lines
8.5 KiB
Python
"""Scenario 3: the pr_fail revision loop and the root → CEO chain.
|
|
|
|
3a: the reviewer REJECTS the assembled cell PR (`pr_fail` with concrete
|
|
issues) → needs_revision; the PM resumes, re-submits, and the second gate
|
|
pass rides through to the merge — the loop the live fleet burned tokens on
|
|
when any link mis-routed.
|
|
|
|
3b: after the cell lands on the root branch, the Main PM submits the root
|
|
(root → master PR), the reviewer gate-passes it, the Main PM's `complete`
|
|
escalates the root parent to the CEO, and the REAL CEO endpoint
|
|
(`POST /api/tasks/{id}/approve-and-merge`) squash-merges to master —
|
|
`hello.txt` ends up on the origin's master, the whole company loop closed
|
|
with no LLM anywhere.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from http import HTTPStatus
|
|
from typing import TYPE_CHECKING
|
|
|
|
import httpx
|
|
from tests.e2e_smoke.arcs import (
|
|
dispatcher_assign,
|
|
open_finding_ids,
|
|
origin_commit,
|
|
origin_file,
|
|
reviewer_gate_pass_arc,
|
|
seed_company,
|
|
seed_hierarchy,
|
|
seed_project,
|
|
task_state,
|
|
)
|
|
from tests.e2e_smoke.harness import ScriptedAgent, expect_ok
|
|
from tests.e2e_smoke.test_pm_merge_chain import _land_child, _pm_merges_cell
|
|
|
|
if TYPE_CHECKING:
|
|
from tests.e2e_smoke.harness import E2EStack
|
|
|
|
|
|
def test_pr_fail_revision_loop(e2e_stack: E2EStack) -> None:
|
|
stack = e2e_stack
|
|
company = seed_company(stack)
|
|
project_id, project_slug = seed_project(stack, company)
|
|
h = seed_hierarchy(stack, company, project_id)
|
|
pm = _land_child(stack, company, project_slug, h)
|
|
cell_id = str(h["cell_id"])
|
|
|
|
expect_ok(
|
|
pm.flow(
|
|
"submit_up",
|
|
task_id=cell_id,
|
|
notes=(
|
|
"All children terminal and merged into the cell branch; "
|
|
"assembling the cell PR for the in-path review gate."
|
|
),
|
|
),
|
|
"pm submit_up (first)",
|
|
)
|
|
|
|
reviewer = ScriptedAgent(
|
|
stack, company.pr_reviewer_id, "pr-reviewer-1", "pr_reviewer"
|
|
)
|
|
expect_ok(
|
|
reviewer.flow("claim_gate_review", task_id=cell_id),
|
|
"reviewer claim_gate_review (first)",
|
|
)
|
|
expect_ok(
|
|
reviewer.do(
|
|
"note",
|
|
scope="learning",
|
|
task_id=cell_id,
|
|
text=(
|
|
"Gate review learning: the assembled diff is missing a "
|
|
"trailing newline convention the root branch enforces — "
|
|
"sending back with a concrete fix."
|
|
),
|
|
),
|
|
"reviewer learning note (fail pass)",
|
|
)
|
|
expect_ok(
|
|
reviewer.flow(
|
|
"pr_fail",
|
|
task_id=cell_id,
|
|
issues=[
|
|
"hello.txt should end with exactly one trailing newline "
|
|
"per the root branch's file conventions."
|
|
],
|
|
),
|
|
"reviewer pr_fail",
|
|
)
|
|
assert task_state(stack, h["cell_id"])["status"] == "needs_revision"
|
|
|
|
# The revision dispatcher routes the assembled task back to its PM;
|
|
# mirror that hand-back, then the PM resumes and re-submits.
|
|
dispatcher_assign(stack, h["cell_id"], company.cell_pm_id)
|
|
expect_ok(
|
|
pm.flow(
|
|
"i_will_plan",
|
|
task_id=cell_id,
|
|
plan=(
|
|
"Address the gate's concrete issue and re-submit the cell PR "
|
|
"for a clean pass through the in-path review gate."
|
|
),
|
|
approach=(
|
|
"Take the reviewer's single concrete finding — hello.txt must "
|
|
"end with exactly one trailing newline per the root branch's "
|
|
"file conventions — verify the file on the cell branch already "
|
|
"satisfies it, re-check the assembled diff against the root "
|
|
"branch for any other convention drift, and then re-run "
|
|
"submit_up so the gate reviews a corrected, freshly assembled "
|
|
"cell PR."
|
|
),
|
|
sub_tasks=[
|
|
{
|
|
"title": "Verify the newline convention",
|
|
"description": (
|
|
"Confirm hello.txt on the cell branch ends with exactly "
|
|
"one trailing newline as the reviewer's finding requires."
|
|
),
|
|
},
|
|
{
|
|
"title": "Re-submit the assembled PR",
|
|
"description": (
|
|
"Run submit_up again so the freshness and integrity "
|
|
"checks re-assemble the cell PR for a clean gate pass."
|
|
),
|
|
},
|
|
],
|
|
),
|
|
"pm i_will_plan after pr_fail",
|
|
)
|
|
# The unchanged-PR hard gate (0.14.0) refuses a resubmit until new work
|
|
# advances the cell branch HEAD — land the dev's fix, then resubmit.
|
|
origin_commit(
|
|
stack,
|
|
h["cell_branch"],
|
|
"hello.txt",
|
|
"Hello from the merge chain, with tidy newline conventions!\n",
|
|
f"[{str(h['child_id'])[:8]}] fix: normalize hello.txt trailing newline",
|
|
)
|
|
# pr_fail's shimmed issue landed as an open ledger finding; the resubmit
|
|
# is gated by FINDINGS_ADDRESSED, so the PM resolves it — the ledger
|
|
# contract every real coordinator now follows.
|
|
open_ids = open_finding_ids(stack, h["cell_id"])
|
|
assert open_ids, "pr_fail must persist its issue as an open ledger finding"
|
|
expect_ok(
|
|
pm.flow(
|
|
"submit_up",
|
|
task_id=cell_id,
|
|
notes=(
|
|
"Revision addressed: file conventions verified against the "
|
|
"root branch; re-assembling the cell PR for the gate."
|
|
),
|
|
resolved_findings=[
|
|
{"finding_id": fid, "note": "trailing newline normalized"}
|
|
for fid in open_ids
|
|
],
|
|
),
|
|
"pm submit_up (resubmit)",
|
|
)
|
|
assert task_state(stack, h["cell_id"])["status"] == "awaiting_pr_review"
|
|
assert not open_finding_ids(stack, h["cell_id"]), (
|
|
"resolved findings must leave the open set"
|
|
)
|
|
|
|
reviewer_gate_pass_arc(stack, company, h["cell_id"])
|
|
_pm_merges_cell(stack, company, pm, h)
|
|
|
|
|
|
def test_root_chain_lands_on_master_via_ceo(e2e_stack: E2EStack) -> None:
|
|
stack = e2e_stack
|
|
company = seed_company(stack)
|
|
project_id, project_slug = seed_project(stack, company)
|
|
h = seed_hierarchy(stack, company, project_id)
|
|
|
|
# Cell lands on the root branch exactly as scenario 2 proved.
|
|
pm = _land_child(stack, company, project_slug, h)
|
|
expect_ok(
|
|
pm.flow(
|
|
"submit_up",
|
|
task_id=str(h["cell_id"]),
|
|
notes=(
|
|
"All children terminal and merged into the cell branch; "
|
|
"assembling the cell PR for the in-path review gate."
|
|
),
|
|
),
|
|
"pm submit_up",
|
|
)
|
|
reviewer_gate_pass_arc(stack, company, h["cell_id"])
|
|
_pm_merges_cell(stack, company, pm, h)
|
|
|
|
# --- Main PM: submit the root → master PR, gate, complete → escalate ----
|
|
main_pm = ScriptedAgent(stack, company.main_pm_id, "main-pm", "main_pm")
|
|
root_id = str(h["root_id"])
|
|
expect_ok(
|
|
main_pm.flow(
|
|
"submit_root",
|
|
task_id=root_id,
|
|
notes=(
|
|
"Every cell task is terminal and assembled on the root "
|
|
"branch; opening the root PR against master for the gate."
|
|
),
|
|
),
|
|
"main_pm submit_root",
|
|
)
|
|
root = task_state(stack, h["root_id"])
|
|
assert root["status"] == "awaiting_pr_review", root
|
|
assert root["pr_number"], root
|
|
|
|
reviewer_gate_pass_arc(stack, company, h["root_id"])
|
|
dispatcher_assign(stack, h["root_id"], company.main_pm_id)
|
|
expect_ok(
|
|
main_pm.flow(
|
|
"complete",
|
|
task_id=root_id,
|
|
notes=(
|
|
"Gate passed on the assembled root PR; approving the root "
|
|
"parent and escalating to the CEO for the merge decision."
|
|
),
|
|
),
|
|
"main_pm complete root",
|
|
)
|
|
assert task_state(stack, h["root_id"])["status"] == "awaiting_ceo_approval"
|
|
|
|
# --- the human gate: the REAL CEO endpoint merges to master --------------
|
|
resp = httpx.post(
|
|
f"{stack.base_url}/api/tasks/{root_id}/approve-and-merge",
|
|
headers={
|
|
"X-Agent-ID": str(company.ceo_id),
|
|
"X-Agent-Role": "ceo",
|
|
},
|
|
timeout=60,
|
|
)
|
|
assert resp.status_code == HTTPStatus.OK, (
|
|
f"approve-and-merge: {resp.status_code} {resp.text[:1500]}"
|
|
)
|
|
assert task_state(stack, h["root_id"])["status"] == "completed"
|
|
assert origin_file(stack, "master", "hello.txt"), (
|
|
"the CEO merge did not land hello.txt on master"
|
|
)
|