From 22976a79f9b64745915511983e9b9d00dabe1ae8 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Sat, 12 Sep 2026 18:11:13 -0600 Subject: [PATCH] fix(TASK-020): surface step-runner LLM-fault captures where the fallback lands The three fallback-capture keys (orient_failure, sweep_failure, handoff_failure) were write-only dead state: captured on LLM-node failure but interpolated nowhere, so end_failure rendered an empty STEP_FAILED shell and write_handoff carried no trace a dead sweep. - new crash-proof script node note_llm_fault distills orient/handoff 'LLM node...' failure text into fault_note (prefix-anchored; empty when neither key holds a fault); orient and write_handoff fallbacks now route through it before end_failure - end_failure renders 'Pipeline fault (if any): {{fault_note}}' (sentinel and all existing fields preserved; fault_note defaulted in initial_state) - write_handoff prompt interpolates {{sweep_failure}} with a PIPELINE-FAULT VISIBILITY instruction mirroring the review_report treatment (fault -> Deviations entry + step_summary; success/empty ignored) - tests: note_llm_fault fault/clean/crash/truncation paths via the GRAPH_STATE harness; wiring test pins the retargeted fallbacks, note_llm_fault routing, {{fault_note}} in end_failure, and the sweep-fault prompt/instruction anchors; warning baseline unchanged --- assets/agents/step-runner/graph.yaml | 32 ++++- .../step-runner/scripts/note_llm_fault.sh | 26 ++++ src/config/request_context.rs | 129 +++++++++++++++++- 3 files changed, 181 insertions(+), 6 deletions(-) create mode 100755 assets/agents/step-runner/scripts/note_llm_fault.sh diff --git a/assets/agents/step-runner/graph.yaml b/assets/agents/step-runner/graph.yaml index 16a1c31..d7ae2bf 100644 --- a/assets/agents/step-runner/graph.yaml +++ b/assets/agents/step-runner/graph.yaml @@ -89,6 +89,7 @@ initial_state: handoff_attempts: 0 handoff_fix: '' handoff_failure: '' + fault_note: '' step_summary: '' start: resolve_step @@ -196,7 +197,7 @@ nodes: ] state_updates: orient_failure: '{{output}}' - fallback: end_failure + fallback: note_llm_fault next: route_staleness route_staleness: @@ -474,6 +475,16 @@ nodes: did not run - ", repeat it under Notes for next step, and carry it into `step_summary` so the user sees it at the approval gate. + + The same rule covers the "Edge-case sweep result" below: text starting + with "LLM node" means the edge-case sweep DID NOT RUN - the graph + routed here through the sweep's failure fallback. Never present it as + sweep findings. Add a Deviations entry "PIPELINE-FAULT: edge-case + sweep did not run - " and carry it into `step_summary` + so the user sees it at the approval gate. If it holds successful + structured output or is empty, ignore it - the sweep's real results + already flow through the "Edge cases discovered" / "Downstream plan + updates" sections above. prompt: | ## Step {{step_number}} ({{step_title}}) - plan at {{step_plan_path}} @@ -497,6 +508,9 @@ nodes: ## Downstream plan updates {{downstream_updates}} + ## Edge-case sweep result (fault detection) + {{sweep_failure}} + ## Independent review report (if any) {{review_report}} @@ -527,9 +541,22 @@ nodes: required: [step_summary] state_updates: handoff_failure: '{{output}}' - fallback: end_failure + fallback: note_llm_fault next: check_handoff + note_llm_fault: + id: note_llm_fault + type: script + description: | + Make LLM-node fallback captures visible: when orient or write_handoff + failed (their capture keys hold the engine's "LLM node..." failure + text), distill it into fault_note so end_failure can render WHY the + step died instead of an empty STEP_FAILED shell. + script: scripts/note_llm_fault.sh + timeout: 5 + fallback: end_failure + next: end_failure + check_handoff: id: check_handoff type: script @@ -627,6 +654,7 @@ nodes: Step: {{step_number}} ({{step_title}}) Fix attempts: {{fix_attempts}}/{{max_fix_attempts}} Blocking reason (if resolution failed): {{blocking_reason}} + Pipeline fault (if any): {{fault_note}} Coder result: {{coder_result}} diff --git a/assets/agents/step-runner/scripts/note_llm_fault.sh b/assets/agents/step-runner/scripts/note_llm_fault.sh new file mode 100755 index 0000000..85fd7b0 --- /dev/null +++ b/assets/agents/step-runner/scripts/note_llm_fault.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [[ -n "${GRAPH_STATE_FILE:-}" ]]; then + state=$(cat "$GRAPH_STATE_FILE") +elif [[ -n "${GRAPH_STATE:-}" ]]; then + state="$GRAPH_STATE" +else + state='{}' +fi + +orient_failure=$(echo "$state" | jq -r '.orient_failure // ""') +handoff_failure=$(echo "$state" | jq -r '.handoff_failure // ""') + +# Engine failure text is always prefix-anchored "LLM node" (llm.rs); on +# success these keys hold the node's structured JSON output, which never +# starts with that prefix. +if [[ "$orient_failure" == "LLM node"* ]]; then + fault_note="PIPELINE-FAULT: orient stage failed — ${orient_failure:0:300}" +elif [[ "$handoff_failure" == "LLM node"* ]]; then + fault_note="PIPELINE-FAULT: handoff stage failed — ${handoff_failure:0:300}" +else + fault_note="" +fi + +jq -nc --arg note "$fault_note" '{"fault_note": $note}' diff --git a/src/config/request_context.rs b/src/config/request_context.rs index a27fd5a..1966fbb 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -10111,6 +10111,91 @@ mod tests { ); } + #[test] + fn step_runner_note_llm_fault_orient_failure() { + if !cmd_available("bash") || !cmd_available("jq") { + eprintln!("skipping: bash/jq not available"); + return; + } + let state = json!({ + "orient_failure": "LLM node failed: boom", + "handoff_failure": "" + }); + let out = run_step_runner_script("note_llm_fault.sh", &state); + assert_eq!( + out, + json!({"fault_note": "PIPELINE-FAULT: orient stage failed — LLM node failed: boom"}) + ); + } + + #[test] + fn step_runner_note_llm_fault_handoff_failure() { + if !cmd_available("bash") || !cmd_available("jq") { + eprintln!("skipping: bash/jq not available"); + return; + } + // On success orient_failure holds the node's structured JSON output, + // which never starts with the engine's "LLM node" prefix — only the + // handoff fault must be reported. + let state = json!({ + "orient_failure": "{\"plan_summary\":\"ok\"}", + "handoff_failure": "LLM node structured-extraction failed: bad schema" + }); + let out = run_step_runner_script("note_llm_fault.sh", &state); + assert!( + out["fault_note"].as_str().unwrap().starts_with( + "PIPELINE-FAULT: handoff stage failed — LLM node structured-extraction failed:" + ), + "{out}" + ); + } + + #[test] + fn step_runner_note_llm_fault_clean_path() { + if !cmd_available("bash") || !cmd_available("jq") { + eprintln!("skipping: bash/jq not available"); + return; + } + let state = json!({ + "orient_failure": "{\"plan_summary\":\"ok\"}", + "handoff_failure": "" + }); + let out = run_step_runner_script("note_llm_fault.sh", &state); + assert_eq!(out, json!({"fault_note": ""})); + } + + #[test] + fn step_runner_note_llm_fault_survives_empty_state() { + if !cmd_available("bash") || !cmd_available("jq") { + eprintln!("skipping: bash/jq not available"); + return; + } + // R3: a fault-noting script must never itself kill the pipeline — + // the harness asserts exit 0 + JSON stdout even for a bare state. + let out = run_step_runner_script("note_llm_fault.sh", &json!({})); + assert_eq!(out, json!({"fault_note": ""})); + } + + #[test] + fn step_runner_note_llm_fault_truncates_long_failure() { + if !cmd_available("bash") || !cmd_available("jq") { + eprintln!("skipping: bash/jq not available"); + return; + } + let failure = format!("LLM node failed: {}", "x".repeat(400)); + let out = run_step_runner_script("note_llm_fault.sh", &json!({"orient_failure": failure})); + let note = out["fault_note"].as_str().unwrap(); + assert!( + note.starts_with("PIPELINE-FAULT: orient stage failed — LLM node failed:"), + "{out}" + ); + assert!( + note.chars().count() <= 340, + "fault_note must be bounded to the prefix + 300 chars, got {}: {note}", + note.chars().count() + ); + } + #[test] fn step_runner_fault_wiring_preserves_sentinels() { use crate::graph::{GraphParser, NodeType}; @@ -10138,10 +10223,14 @@ mod tests { // The llm nodes with fallbacks capture their failure text so the // fallback path can see why it was reached (the TASK-016 warning — // this is what shrank the step-runner warning baseline to zero). - for (node_id, key) in [ - ("orient", "orient_failure"), - ("edge_case_sweep", "sweep_failure"), - ("write_handoff", "handoff_failure"), + // orient and write_handoff route their fallbacks through + // note_llm_fault, which distills the capture into fault_note; + // edge_case_sweep still falls back to write_handoff, which renders + // sweep_failure in its prompt. + for (node_id, key, fallback) in [ + ("orient", "orient_failure", "note_llm_fault"), + ("edge_case_sweep", "sweep_failure", "write_handoff"), + ("write_handoff", "handoff_failure", "note_llm_fault"), ] { let NodeType::Llm(llm) = &graph.get_node(node_id).unwrap().node_type else { panic!("{node_id} must be an llm node") @@ -10152,8 +10241,22 @@ mod tests { .is_some_and(|u| u.contains_key(key)), "{node_id} must capture its failure text into {key}" ); + assert_eq!( + llm.fallback.as_deref(), + Some(fallback), + "{node_id} must fall back to {fallback}" + ); } + // note_llm_fault sits on the failure path; its own fallback also + // lands on end_failure so a broken script can never strand the step. + let note_node = graph.get_node("note_llm_fault").unwrap(); + let NodeType::Script(note) = ¬e_node.node_type else { + panic!("note_llm_fault must be a script node") + }; + assert_eq!(note_node.next_target(), Some("end_failure")); + assert_eq!(note.fallback.as_deref(), Some("end_failure")); + // write_handoff must teach the review-fault flag: a review_report // holding the engine's failure text is "review DID NOT RUN", flagged // prominently — never presented as findings. @@ -10167,6 +10270,19 @@ mod tests { .is_some_and(|i| i.contains("Agent node failed:")), "write_handoff instructions must handle the review-fault text" ); + // Same treatment for the sweep fault: sweep_failure is interpolated + // in the prompt and the instructions teach the "LLM node" anchor. + assert!( + handoff.prompt.contains("{{sweep_failure}}"), + "write_handoff prompt must surface the sweep capture" + ); + assert!( + handoff + .instructions + .as_ref() + .is_some_and(|i| i.contains("LLM node")), + "write_handoff instructions must handle the sweep-fault text" + ); // end_failure renders sensibly when reached via implement's fallback: // STEP_FAILED leads, and every interpolated key has an initial_state @@ -10179,6 +10295,11 @@ mod tests { "the sentinel must lead the output: {}", end.output ); + assert!( + end.output.contains("{{fault_note}}"), + "end_failure must render the distilled fault note: {}", + end.output + ); for chunk in end.output.split("{{").skip(1) { let key = chunk.split("}}").next().unwrap().trim(); assert!(