From a8496aa50ea507c14cdd2c74bb921920167f7c11 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Sat, 12 Sep 2026 17:31:41 -0600 Subject: [PATCH] fix(TASK-019): carry PIPELINE-FAULT prefix through verdict.py's crash guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit render.py's empty-diff stub bypass is anchored on the 'PIPELINE-FAULT:' prefix in reason/attention, but verdict.py's crash guard emitted 'verdict computation error: …' / 'verdict script error: …' without it — on an empty diff a crashed verdict gate collapsed into the 'No changes to review.' stub and the fail-closed NEEDS-HUMAN verdict never reached the final report. - prefix the crash-guard reason and attention entry with PIPELINE-FAULT: - strengthen the existing crash test to assert the prefix on both fields - add R5 chained test: empty diff + crash-inducing state piped verdict.py -> render.py must render the fault, never the stub - add crash-guard symmetry tests for parse_fault.py / aux_fault.py (raw non-JSON GRAPH_STATE keeps the fault marker) and a test that a fault survives the attention[:5] cap alongside 6 user flags --- .../agents/code-reviewer/scripts/verdict.py | 8 +- src/config/request_context.rs | 132 +++++++++++++++++- 2 files changed, 136 insertions(+), 4 deletions(-) diff --git a/assets/agents/code-reviewer/scripts/verdict.py b/assets/agents/code-reviewer/scripts/verdict.py index 6aa72ff..83ad071 100644 --- a/assets/agents/code-reviewer/scripts/verdict.py +++ b/assets/agents/code-reviewer/scripts/verdict.py @@ -196,9 +196,13 @@ except Exception as e: # noqa: BLE001 — never crash into a silent verdict { "verdict_out": { "verdict": "NEEDS-HUMAN", - "reason": f"verdict computation error: {e} — treat as needing human review", + # PIPELINE-FAULT prefix is load-bearing: render.py's + # empty-diff stub bypass anchors on it — without it a + # crashed gate on an empty diff collapses into the + # "No changes to review." stub. + "reason": f"PIPELINE-FAULT: verdict computation error: {e} — human review required", "counts": {}, "deferred_count": 0, "dropped_count": 0, - "dropped_titles": [], "attention": [f"verdict script error: {e}"], + "dropped_titles": [], "attention": [f"PIPELINE-FAULT: verdict script error: {e}"], "findings_final": [], } } diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 91130bc..f0d3a6e 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -9625,8 +9625,136 @@ mod tests { v["reason"] .as_str() .unwrap() - .contains("verdict computation error"), - "the reason must name the error: {out}" + .starts_with("PIPELINE-FAULT: verdict computation error"), + "the reason must carry the fault prefix render.py anchors on: {out}" + ); + assert!( + v["attention"][0] + .as_str() + .unwrap() + .starts_with("PIPELINE-FAULT: verdict script error"), + "the attention entry must carry the fault prefix: {out}" + ); + } + + #[test] + fn code_reviewer_verdict_crash_empty_diff_renders_fault() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // A crashed verdict gate on an EMPTY diff: render.py's stub bypass is + // anchored on the "PIPELINE-FAULT:" prefix, so the crash-guard verdict + // must carry it — otherwise the fail-closed NEEDS-HUMAN collapses into + // the "No changes to review." stub and never reaches the final report. + let fault = run_code_reviewer_script( + "verdict.py", + &json!({"changed_files": [], "attention_flags": 42}), + ); + let state = json!({ + "changed_files": [], + "verdict_out": fault["verdict_out"].clone() + }); + let out = run_code_reviewer_script("render.py", &state); + let report = out["final_report"].as_str().unwrap(); + assert!( + report.contains("**Verdict: NEEDS-HUMAN**"), + "the crash verdict must render: {report}" + ); + assert!( + report.contains("PIPELINE-FAULT: verdict computation error"), + "the crash reason must render: {report}" + ); + assert!( + !report.contains("No changes to review."), + "a crashed gate on an empty diff must not collapse into the stub: {report}" + ); + } + + #[test] + fn code_reviewer_verdict_fault_survives_attention_cap() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // Faults are prepended to attention exactly so the [:5] cap can never + // hide one — pack 6 user flags alongside a verifier fault and assert + // the fault still leads the capped list. + let state = json!({ + "changed_files": ["a.rs"], + "domain_reports": ["clean report. DOMAIN_REVIEW_COMPLETE"], + "findings": [], + "verifier_output": "Agent node failed: dead", + "attention_flags": ["f1", "f2", "f3", "f4", "f5", "f6"] + }); + let out = run_code_reviewer_script("verdict.py", &state); + let v = &out["verdict_out"]; + assert_eq!( + v["verdict"], "NEEDS-HUMAN", + "a dead verifier must block: {out}" + ); + let attention = v["attention"].as_array().unwrap(); + assert_eq!( + attention.len(), + 5, + "attention must be capped at 5 entries: {out}" + ); + assert!( + attention[0] + .as_str() + .unwrap() + .starts_with("PIPELINE-FAULT: finding verification failed"), + "the fault must survive the attention cap by leading the list: {out}" + ); + } + + #[test] + fn code_reviewer_parse_fault_crash_fails_closed() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // Unparseable state makes load_state raise before main runs; the + // fault marker itself must still emit a prefix-anchored fault verdict. + let out = run_code_reviewer_script_raw("parse_fault.py", "not json"); + let v = &out["verdict_out"]; + assert_eq!( + v["verdict"], "NEEDS-HUMAN", + "a crashed parse-fault marker must fail closed: {out}" + ); + assert!( + v["reason"] + .as_str() + .unwrap() + .starts_with("PIPELINE-FAULT: parse failed"), + "the fault prefix must survive a crash: {out}" + ); + assert!( + v["reason"] + .as_str() + .unwrap() + .contains("fault-marker script error"), + "the crash must be named: {out}" + ); + } + + #[test] + fn code_reviewer_aux_fault_crash_fails_closed() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // Unparseable state makes load_state raise before main runs; the + // fault marker itself must still emit the prefixed aux_note. + let out = run_code_reviewer_script_raw("aux_fault.py", "not json"); + let note = out["aux_note"].as_str().unwrap(); + assert!( + note.starts_with("PIPELINE-FAULT: aux context lanes failed after retries —"), + "the aux fault prefix must survive a crash: {note}" + ); + assert!( + note.contains("fault-marker script error"), + "the crash must be named: {note}" ); }