diff --git a/implement/workflows/implement-loop.js b/implement/workflows/implement-loop.js index cb3fa9f..7c8272e 100644 --- a/implement/workflows/implement-loop.js +++ b/implement/workflows/implement-loop.js @@ -40,9 +40,12 @@ // task prescribes) is HELD, not chased into a deviation the next review // then re-flags. The repair implementer keeps the plan and records the // finding in `held`; a no-op backstop holds it anyway when a repair -// leaves the diff unchanged (or cycles back) yet quality still flags. -// Either way the finding surfaces as a concern, not a false BLOCKED on a -// green task — and the orchestrator weighs it on its Step-3 inspection. +// leaves the diff unchanged (or cycles back) yet quality still flags — +// severity-aware (issue #29): a surviving IMPORTANT finding over a +// byte-identical repair diff is BLOCKED, never a held concern; only +// minor-only residue rides as a concern. +// Either way a held finding surfaces as a concern, not a false BLOCKED on +// a green task — and the orchestrator weighs it on its Step-3 inspection. // A finding the implementer judges correctness-breaking (would not // build / a test fails / a prescribed caller breaks) escalates to // BLOCKED instead of holding. The held/no-op partition is the @@ -684,10 +687,24 @@ async function runTask(task) { // No-op / cyclic-repair backstop: a changes_requested verdict over a diff-state already reviewed-and-flagged // this task means the intervening repair changed nothing (or cycled back to an earlier state) — the // implementer kept the plan, or rejected the finding. Re-running quality would only re-flag identical code, - // so stop here rather than burning the retry budget down to a false BLOCKED. A byte-identical diff cannot - // distinguish a principled plan-hold from an ignored finding, so the concern is labelled neutrally and left - // for the orchestrator to adjudicate on its Step-3 inspection. + // so stop here rather than burning the retry budget down to a false BLOCKED. + // Severity check (issue #29, from the pipeline audit): the early exit is severity-aware. A byte-identical + // diff under a surviving IMPORTANT finding is not a cosmetic hold — an important finding names a defect the + // repair did not address, and letting it ride a green commit as a mere concern is exactly the erosion this + // gate exists to stop → BLOCKED. Only a minor-only (or unreported) finding set is held as a neutral concern: + // a byte-identical diff cannot distinguish a principled plan-hold from an ignored cosmetic finding, so that + // case is labelled neutrally and left for the orchestrator to adjudicate on its Step-3 inspection. if (qual && qual.status === 'changes_requested' && qual.diff_fingerprint && seenFingerprints.has(qual.diff_fingerprint)) { + const importants = (qual.issues || []).filter((i) => i.severity === 'important') + if (importants.length) { + return { + id: task.id, + title: task.title, + outcome: 'BLOCKED', + reason: 'unresolved important quality finding — the repair left the diff unchanged', + detail: importants.map((i) => i.text).join('; '), + } + } heldConcerns = (qual.issues && qual.issues.length ? qual.issues.map((i) => `${i.severity}: ${i.text}`) : ['(finding not reported)']).map( (i) => 'unresolved quality finding — the repair left the diff unchanged (a kept plan-prescribed element, or a ' +