3 Commits
Author SHA1 Message Date
e9ca7d4036 Delegation detail-fidelity + PM-loop hardening (#541)
* feat(gateway): delegation detail-fidelity — details survive hand-off, both directions

Details thinned out at every delegation hop: a PM child task mapped to no
parent criterion was legal (coverage only surfaced at submit_up, after the
whole wave ran — a 12-subtask docs tree grew through 8 review rounds that
way, one child titled 'docs page and route wrapper' shipping only the
page), and QA could pass work on a gestalt read (a 4-scene video brief
shipped 3 scenes past every gate because the features existed only in
prose). Three chokepoint gates:

- delegate (down): every child must declare covers_parent_criteria
  resolving against the parent's real acceptance criteria — no mapping or
  an unresolvable ref rejects naming every offending child and the valid
  criteria; the success envelope carries parent_ac_coverage
  {covered, uncovered} so a wave-planning PM sees remaining gaps in the
  same turn. Full coverage stays enforced at submit_up (waves stay legal).
- pass_review (up): mandatory criteria_verified — one {criterion,
  evidence} entry per task AC, matched by the findings ledger's
  id-or-exact-text matcher, evidence soup-checked and capped; rejects
  naming the unverified criteria; entries render deterministically into
  qa_notes as '[AC] <criterion> — verified: <evidence>' lines. The old
  count-only ac_verdicts gate is superseded (arg kept for back-compat).
- video briefs (structured detail at origination): an enumerable feature
  list (release highlights, or input_props.highlights carried onto a
  reject re-author) becomes its own scene acceptance criterion, bounded to
  the AC caps; a re-author without highlights carries the
  feedback-addressed criterion instead.

Extracted findings.py's criterion matcher into shared unmatched_criteria /
uncovered_acceptance_criteria instead of duplicating it; criteria_verified
joins the WAF free-text exclusion set like findings/issues.

* fix(gateway): break the block/unblock wedge — four hardening fixes from the live PM loop

A cell task looped fe-pm/main-pm block/unblock for hours (10 cycles, 43
spawns): a transient GitHub API error resolving CI became an unwaivable
blocker finding whose own fix text said no code change was required, the
submit freshness guard then demanded a commit no finding called for,
escalate_up auto-blocked, and main-pm's correct recovery plan 422'd on
the approach length cap, degrading it to a bare unblock. Four fixes:

- pr_pass CI-unresolvable refusal is now explicitly transient-worded:
  retry pr_pass shortly, do NOT pr_fail over a CI-status lookup error —
  a platform blip is not a code finding
- submit freshness guard grants ONE unchanged-head resubmission per
  head sha when the findings ledger has zero open rows (all addressed
  without code changes) — stamped via the resubmit_unchanged_head
  marker so the same head can never loop a second time
- unblock carries a flip breaker: block_flip_count marker, and at the
  third flip a one-shot CEO notification flags the task as structurally
  wedged (unblock itself still succeeds — the breaker signals, it does
  not wedge recovery)
- i_will_plan's approach cap truncates at 800 chars instead of
  rejecting — an over-detailed plan must never cost the PM its turn

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-17 01:52:33 +02:00
cea3e56628 feat(lifecycle): revision findings ledger — structured failure feedback, persisted and delivered down the chain (#486)
* 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>
2026-07-11 22:54:42 +02:00
4d52f6ff59 [1f6a06a2] PR-review gate: verify ACs literally and require green CI before pr_pass (#428)
* [a1bde3b9] Add CI-status guard to pr_pass + update pr_reviewer prompt (#417) (#420)

* [a1bde3b9] feat(gateway): CI-status guard on pr_pass + reviewer prompt update

* [a1bde3b9] docs(pr-gate-review, worksession-git): document CI-status guard on pr_pass

Updated two architecture documentation files to reflect the new CI-status guard:

**pr-gate-review.md:**
- Documented _ci_status_guard method: blocks pr_pass on failing/pending/unscheduled/error CI with reviewer-aware pr_fail remediation
- Documented _resolve_ci_status: best-effort GitHub check-runs lookup with fail-open behavior
- Updated _pr_pass_blocked description: now returns (rejection_envelope, ci_note) tuple
- Updated _record_gate_verdict_for/verdict to note ci_status field stamping on pr_pass
- Added ci_note parameter documentation for evidence tracking when no CI is configured
- Updated Logical Tree to show new methods
- Added Config Flags note: CI guard is always armed, fails open on config gaps
- Added two regression risks: check-runs-only limitation, fail-open design

**worksession-git.md:**
- Documented GitService.get_pr_ci_status(project_slug, pr_number): CI status lookup with state classification
- Documented supporting methods: _ci_status_prereqs, _fetch_check_runs, _classify_check_runs, _classify_zero_check_runs
- Each method notes its fail-open behavior and configuration gap handling

---------

Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>

* [e8f275d7] test(gateway): lock the 7-AC-to-test map + assert pr_reviewer prompt content (#425) (#426)

Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>

* [24b4237e] Fix reflow-check, CI-status classification, and noqa suppression (#440) (#443)

* [24b4237e] fix(gateway): classify unreachable/nonexistent CI-status repo as no_ci_configured, remove test noqa, reflow pr_reviewer.md

Split GitService.get_pr_ci_status's PR-head-sha lookup into a dedicated
helper so a config gap (missing project/git_url/token) or an unreachable/
nonexistent repo/PR (network error or 404) classifies as no_ci_configured
(pr_pass passes through and stamps the evidence note) while a genuine
GitHub API failure on a real, reachable repo (any other non-2xx, or an
unparseable body) stays the fail-closed error state. Replaced the
`# noqa: PLR2004` in test_git_pr_ci_status.py with a named HTTP-status
range constant, updated the config-gap tests to assert the new
classification, and added tests for the unreachable-repo and real-repo-
API-error branches. Reflowed agents/prompts/roles/pr_reviewer.md's one
hard-wrapped continuation line so it passes make reflow-check.

* [24b4237e] docs(gateway): update pr-gate-review.md for CI-status classification refactor

Updated the internal architectural map to reflect the new CI-status classification
scheme introduced in PR #440. Configuration gaps (missing project/git_url/token) and
unreachable/nonexistent repos (404 or network error) now explicitly classify as
no_ci_configured and pass through with evidence stamps. Genuine GitHub API failures
on reachable repos classify as error and stay fail-closed (retryable).

- Clarified _ci_status_guard behavior: config gaps/unreachable repos pass through
  with distinct classification; only real API failures stay fail-closed
- Updated Config Flags section to describe the new three-way classification
- Updated Regression Risks section to document the new explicit classification scheme
- Noted that _resolve_ci_status now wraps git.get_pr_ci_status and interprets its result dict

---------

Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>

* [1f6a06a2] round-3 fixes: pr_gate back to xenon rank A; 404 means no CI, not error

Eight extracted helpers bring the module average from B(5.05) to A(4.04)
with every external contract untouched (170 gate tests byte-identical).
The CI-status guard now classifies a 404 on the check-runs or workflows
endpoints as no_ci_configured (pass-through with evidence note) —
a repo without Actions is not a transport failure — reserving the
fail-closed error state for network/5xx/auth failures, with pinning
tests for all four shapes. The e2e fake-GitHub router gains check-runs
and workflows routes so the scripted lifecycle exercises the guard's
green-CI success branch end to end.

* [1f6a06a2] merge master; align gate-diff-base tests with the tuple contract

The merged tree is the first integration of the CI-status guard with the
preferred-parent diff-base guard: _pr_pass_blocked now returns
(rejection, ci_note), so the diff-base tests unpack it instead of
asserting on a bare result. Both guards verified live in the merged
pr_gate (preferred_parent threading and _ci_status_guard present).

---------

Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-11 07:38:06 +02:00