From ab6f6f225aa481b68da63390b0aed7a1ddf05f6e Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Fri, 9 Oct 2026 21:30:58 -0400 Subject: [PATCH 1/6] fix(agent_result): read blocking-human checkpoints from a declared field Replace the reason-prose heuristic from e3f9ddd with an explicit `checkpoint` field on the DEVFLOW_RESULT marker (49-REVIEW CR-01). The heuristic auto-resumed ordinary failures whose reason mentioned a gate, including through "investigate". - AgentResult gains a leniently parsed `checkpoint` field and `declares_blocking_human_checkpoint()` (failed + "blocking-human"). - The GateReview arm confirms a checkpoint from that field on the stage's evaluated final result, or from a `Gate:` declaration line as before. - The completion protocol shows agents the checkpoint form. - The line matcher is restored to its pre-heuristic form. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0182ocSM2E4fb2zu52jxGLzi --- crates/devflow-cli/src/pipeline_launch.rs | 126 +++++++- crates/devflow-cli/src/pipeline_outcomes.rs | 6 + ...e_launch__tests__turn_one_code_prompt.snap | 4 + ...launch__tests__turn_two_resume_prompt.snap | 4 + crates/devflow-core/src/agent_result.rs | 280 +++++++++++++----- crates/devflow-core/src/prompt.rs | 47 ++- ...tyle_full_execute_fix_prompt_snapshot.snap | 4 + ...__full_execute_fix_prompt_antigravity.snap | 4 + ...tests__full_execute_fix_prompt_claude.snap | 4 + ..._tests__full_execute_fix_prompt_codex.snap | 4 + ...tests__full_execute_fix_prompt_hermes.snap | 4 + ...sts__full_execute_fix_prompt_opencode.snap | 4 + ...pt__tests__full_execute_fix_prompt_pi.snap | 4 + 13 files changed, 419 insertions(+), 76 deletions(-) diff --git a/crates/devflow-cli/src/pipeline_launch.rs b/crates/devflow-cli/src/pipeline_launch.rs index db9b64ef..c8af1e75 100644 --- a/crates/devflow-cli/src/pipeline_launch.rs +++ b/crates/devflow-cli/src/pipeline_launch.rs @@ -1760,8 +1760,11 @@ pub(crate) fn advance_with( // ORDER — load-bearing — (1) agent is Claude (D-05); (2) the // phase's plans statically declare a blocking-human checkpoint — // the PRIMARY, agent-uncontrollable gate, checked BEFORE - // anything agent-controlled (T-28-01); (3) the capture confirms - // one was reported; (4) a session id is on record; and (5) the + // anything agent-controlled (T-28-01); (3) the agent reported + // one, either by declaring `"checkpoint": "blocking-human"` on the + // stage's final DEVFLOW_RESULT marker or by a `Gate:` declaration + // line in the capture — never by the wording of a failure reason + // (49-REVIEW CR-01); (4) a session id is on record; and (5) the // resume ceiling has not been exhausted. A changed declaration // always opens the re-scan gate. Only an unchanged, recorded set // in Auto resumes without a human; Supervise falls through to a @@ -1783,7 +1786,8 @@ pub(crate) fn advance_with( let execution_root = state.worktree_path.as_deref().unwrap_or(project_root); let checkpoint_confirmed = state.agent == AgentKind::Claude && verify::phase_has_blocking_human_checkpoint(execution_root, phase) - && agent_result::checkpoint_reported_in_capture(project_root, phase); + && (result.declares_blocking_human_checkpoint() + || agent_result::checkpoint_reported_in_capture(project_root, phase)); if checkpoint_confirmed { let ceiling_ok = state.checkpoint_resumes < mode::MAX_CHECKPOINT_RESUMES; match (&state.session_id, ceiling_ok) { @@ -4050,6 +4054,29 @@ mod tests { .unwrap(); } + /// A capture whose only checkpoint signal is the declared marker field + /// (49-REVIEW CR-01): no `Gate:` line anywhere, just + /// `"checkpoint": "blocking-human"` on the final `DEVFLOW_RESULT`. + fn write_marker_declared_checkpoint_capture(root: &Path, phase: PhaseId) { + std::fs::create_dir_all(root.join(".devflow")).unwrap(); + let stdout = format!( + "Plan 01 stopped at Task 1.\n\nDEVFLOW_RESULT: {{\"status\": \"failed\", \"checkpoint\": \"{HUMAN_GATE_VALUE_FOR_TEST}\", \"reason\": \"Task 1 needs a decision\"}}\n" + ); + std::fs::write(agent_result::stdout_path(root, phase), stdout).unwrap(); + } + + /// Live Attempt 1's real failure (2026-10-07): the reason describes a + /// blocking-human halt in prose but declares nothing. The reverted prose + /// heuristic auto-resumed this; it must now take the ordinary gate. + fn write_prose_only_checkpoint_capture(root: &Path, phase: PhaseId) { + std::fs::create_dir_all(root.join(".devflow")).unwrap(); + std::fs::write( + agent_result::stdout_path(root, phase), + "DEVFLOW_RESULT: {\"status\": \"failed\", \"reason\": \"Phase 01 plan 01-01 halted at Task 1, a blocking-human decision checkpoint\"}\n", + ) + .unwrap(); + } + /// Pre-write a rejected gate response so `run_gate`'s poll (invoked by /// the fall-through path's `handle_stage_failure`) returns immediately /// instead of blocking — mirrors @@ -5276,6 +5303,99 @@ mod tests { /// Declared, but the capture does not confirm it — an ordinary failure /// in a phase that happens to declare a checkpoint elsewhere. + /// The declared marker field alone confirms the checkpoint: same route and + /// audit trail as a reported `Gate:` line (49-REVIEW CR-01). + #[test] + fn advance_with_declared_checkpoint_and_marker_field_relaunches_and_records() { + const NAME: &str = "pipeline_launch::tests::advance_with_declared_checkpoint_and_marker_field_relaunches_and_records"; + const ROOT_ENV: &str = "DEVFLOW_PIPELINE_LAUNCH_TEST_ROOT"; + + if !devflow_core::test_support::in_child_test(NAME) { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_repo(root); + + let phase = PhaseId::new(186); + write_declared_checkpoint_plan(root, phase); + write_marker_declared_checkpoint_capture(root, phase); + + let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf()); + state.stage = Stage::Code; + state.session_id = Some("sess-marker-field".to_string()); + state.checkpoint_approval = recorded_approval_for(root, phase); + workflow::save_state(&state).unwrap(); + + let path_dir = agent_free_dir_with_agent_stub("claude"); + let output = devflow_core::test_support::run_test_in_child( + NAME, + path_dir.path(), + &[(ROOT_ENV, root.as_os_str())], + ); + devflow_core::test_support::assert_child_ran_exactly_one_passing_test(&output, NAME); + return; + } + + let root = PathBuf::from( + std::env::var_os(ROOT_ENV) + .expect("child test must receive its parent-built fixture root"), + ); + let root = root.as_path(); + let phase = PhaseId::new(186); + // Unused on the relaunch route; if detection regresses, the + // fall-through gate reads it and returns instead of blocking forever, + // so the assertions below fail rather than hang. + write_abort_gate_response(root, phase, Stage::Code); + + let result = advance_for_stage(root, Some(phase), Some(Stage::Code)); + let reloaded_for_reap = workflow::load_state(root, phase).ok(); + let _reap_guard = reloaded_for_reap + .as_ref() + .map(ReapMonitorOnDrop::after_launch); + + result.unwrap(); + + let auto_decided = events_of_kind(root, "checkpoint_auto_decided"); + assert_eq!( + auto_decided.len(), + 1, + "a declared marker checkpoint must auto-decide exactly once: {auto_decided:?}" + ); + assert_eq!(auto_decided[0]["session_id"], "sess-marker-field"); + let gate_fired = events_of_kind(root, "gate_fired"); + assert!( + gate_fired.iter().all(|e| e["stage"] != "code"), + "an auto-resolved checkpoint must not also fire the generic gate: {gate_fired:?}" + ); + } + + /// 49-REVIEW CR-01 at the pipeline: a failure whose reason only describes + /// a blocking-human halt takes the ordinary gate, even in a phase that + /// declares such a checkpoint. + #[test] + fn advance_with_declared_checkpoint_and_prose_only_reason_falls_through() { + const NAME: &str = "pipeline_launch::tests::advance_with_declared_checkpoint_and_prose_only_reason_falls_through"; + enter_agent_free_child!(NAME, "claude"); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_repo(root); + + let phase = PhaseId::new(187); + write_declared_checkpoint_plan(root, phase); + write_prose_only_checkpoint_capture(root, phase); + write_abort_gate_response(root, phase, Stage::Code); + + let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf()); + state.stage = Stage::Code; + state.session_id = Some("sess-prose-only".to_string()); + state.checkpoint_approval = recorded_approval_for(root, phase); + workflow::save_state(&state).unwrap(); + + advance_for_stage(root, Some(phase), Some(Stage::Code)).unwrap(); + + assert!(events_of_kind(root, "checkpoint_auto_decided").is_empty()); + assert!(!events_of_kind(root, "gate_fired").is_empty()); + } + #[test] fn advance_with_declared_checkpoint_but_unreported_gate_falls_through() { const NAME: &str = "pipeline_launch::tests::advance_with_declared_checkpoint_but_unreported_gate_falls_through"; diff --git a/crates/devflow-cli/src/pipeline_outcomes.rs b/crates/devflow-cli/src/pipeline_outcomes.rs index b31a893b..2143e46d 100644 --- a/crates/devflow-cli/src/pipeline_outcomes.rs +++ b/crates/devflow-cli/src/pipeline_outcomes.rs @@ -1627,6 +1627,7 @@ mod tests { summary: None, verdict: Some(Verdict::Pass), decided_by_layer: Some(0), + checkpoint: None, }; let outcome = classify_validate_outcome(&result); assert_eq!(outcome, ValidateOutcome::Passed); @@ -1691,6 +1692,7 @@ mod tests { summary: None, verdict: Some(Verdict::Gaps), decided_by_layer: Some(0), + checkpoint: None, }; let outcome = classify_validate_outcome(&result); assert!(matches!(outcome, ValidateOutcome::Ambiguous(_))); @@ -1743,6 +1745,7 @@ mod tests { summary: None, verdict: None, decided_by_layer: Some(0), + checkpoint: None, }; let outcome = classify_validate_outcome(&result); assert!(matches!(outcome, ValidateOutcome::Ambiguous(_))); @@ -1794,6 +1797,7 @@ mod tests { summary: None, verdict: Some(Verdict::Pass), decided_by_layer: Some(0), + checkpoint: None, }; assert_eq!( classify_validate_outcome(&result), @@ -1845,6 +1849,7 @@ mod tests { summary: None, verdict, decided_by_layer: if layer0 { Some(0) } else { Some(1) }, + checkpoint: None, } } @@ -4673,6 +4678,7 @@ mod tests { summary: None, verdict: Some(Verdict::Gaps), decided_by_layer: Some(0), + checkpoint: None, }; let outcome = classify_validate_outcome(&result); assert!(matches!(outcome, ValidateOutcome::Ambiguous(_))); diff --git a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap index c084883f..1e27e363 100644 --- a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap +++ b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap @@ -28,4 +28,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap index d13257b7..64eb5527 100644 --- a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap +++ b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap @@ -14,4 +14,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/agent_result.rs b/crates/devflow-core/src/agent_result.rs index 55bf857d..305e48e0 100644 --- a/crates/devflow-core/src/agent_result.rs +++ b/crates/devflow-core/src/agent_result.rs @@ -40,6 +40,42 @@ pub struct AgentResult { /// fixture literals that don't route through the real cascade. #[serde(default)] pub decided_by_layer: Option, + /// The checkpoint the agent declares it stopped at, as written in its + /// `DEVFLOW_RESULT` marker (`"checkpoint": "blocking-human"`). Only a + /// failed result naming [`HUMAN_GATE_VALUE`] counts; see + /// [`AgentResult::declares_blocking_human_checkpoint`]. + /// + /// This replaces reading the checkpoint out of the free-text `reason` + /// (49-REVIEW CR-01), which matched ordinary failures that mentioned a + /// gate. Deserialized leniently via [`deserialize_checkpoint_lenient`]: a + /// non-string value becomes `None` rather than failing the whole parse, so + /// a malformed field can never drop a valid `status` to Layer 2. + #[serde(default, deserialize_with = "deserialize_checkpoint_lenient")] + pub checkpoint: Option, +} + +impl AgentResult { + /// Whether the agent declared that it stopped at a `blocking-human` + /// checkpoint: a failed result whose `checkpoint` field names + /// [`HUMAN_GATE_VALUE`] (ASCII case-insensitive, like the `Gate:` value + /// match). A successful result never counts, whatever it declares. + pub fn declares_blocking_human_checkpoint(&self) -> bool { + self.status == AgentStatus::Failed + && self + .checkpoint + .as_deref() + .is_some_and(|value| value.eq_ignore_ascii_case(HUMAN_GATE_VALUE)) + } +} + +/// Lenient deserializer for [`AgentResult::checkpoint`]: a string is kept, and +/// anything else (number, bool, object, null) becomes `None`. +fn deserialize_checkpoint_lenient<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let raw = as serde::Deserialize>::deserialize(deserializer)?; + Ok(raw.and_then(|v| v.as_str().map(str::to_string))) } /// Agent completion status determined by DevFlow. @@ -584,6 +620,7 @@ fn detect_claude_envelope_failure(stdout: &str) -> Option { summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, }) } @@ -641,6 +678,14 @@ const HUMAN_GATE_VALUE: &str = "blocking-human"; /// checkpoint" heuristic (D-02 rejected that class of predicate); the scope /// is one declared field label with one enumerated value. /// +/// The other declared form, a final `DEVFLOW_RESULT` marker carrying +/// `"checkpoint": "blocking-human"`, is not read here. The pipeline reads it +/// from the stage's already-evaluated result +/// ([`AgentResult::declares_blocking_human_checkpoint`]), so it follows the +/// same last-marker-wins rule as the stage verdict. A marker's free-text +/// `reason` is never read as a checkpoint: a substring heuristic over it +/// matched ordinary failures and was removed (49-REVIEW CR-01). +/// /// **A Claude `stream-json` capture takes a separate branch** and is answered /// by [`claude_stream_reports_human_gate`] ALONE — it never consults raw stdout. /// That is not an oversight to be "completed" later: under a stream capture the @@ -727,58 +772,26 @@ fn json_strings_report_human_gate(value: &serde_json::Value) -> bool { /// /// Note the closing backtick needs no handling: `take_while` already stops /// at it, since a backtick is neither alphanumeric nor `-`. -/// Match a physical line whose first word, after any list, ordinal or blockquote -/// markup ([`strip_leading_markup`]), is a case-insensitive `gate` label with value -/// [`HUMAN_GATE_VALUE`]. -fn line_reports_human_gate(line: &str) -> bool { - let lower = line.to_ascii_lowercase(); - let line = strip_leading_markup(&lower); - let Some(after_label) = line.strip_prefix("gate") else { - return false; - }; - let after_label = after_label.trim_start_matches(['*', ' ', '`']); - if let Some(rest) = after_label.strip_prefix(':') { - let value_region = rest.trim_start_matches(['*', ' ', '\t', '`']); - let value_token: String = value_region - .chars() - .take_while(|c| c.is_ascii_alphanumeric() || *c == '-') - .collect(); - if value_token == HUMAN_GATE_VALUE { - return true; - } - } - false -} - -/// Detect when an agent halts at a human checkpoint and reports that failure -/// via an embedded [`DEVFLOW_RESULT`] marker with [`HUMAN_GATE_VALUE`]. -/// -/// In multi-agent or subagent workflows (such as GSD spawning `gsd-executor`), -/// the subagent halts at the checkpoint and the orchestrating agent emits a -/// terminal failure marker describing the halt (e.g. `halted at Task 1, a -/// blocking-human decision checkpoint`). Scanning the structured marker reason -/// allows DevFlow to trigger auto-decide resume rather than falling through to -/// an ordinary stage failure gate. -fn marker_reports_human_gate(text: &str) -> bool { - let Some(result) = parse_marker_lines(text) else { - return false; - }; - if result.status != AgentStatus::Failed { - return false; - } - let Some(reason) = result.reason else { - return false; - }; - let lower = reason.to_ascii_lowercase(); - lower.contains(HUMAN_GATE_VALUE) - && (lower.contains("checkpoint") || lower.contains("gate") || lower.contains("halted")) -} - fn text_reports_human_gate(text: &str) -> bool { - if text.lines().any(line_reports_human_gate) { - return true; - } - marker_reports_human_gate(text) + text.lines().any(|line| { + let lower = line.to_ascii_lowercase(); + let line = strip_leading_markup(&lower); + let Some(after_label) = line.strip_prefix("gate") else { + return false; + }; + let after_label = after_label.trim_start_matches(['*', ' ', '`']); + if let Some(rest) = after_label.strip_prefix(':') { + let value_region = rest.trim_start_matches(['*', ' ', '\t', '`']); + let value_token: String = value_region + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '-') + .collect(); + if value_token == HUMAN_GATE_VALUE { + return true; + } + } + false + }) } /// Strip the markdown a relayed declaration line can carry before its label: @@ -914,6 +927,7 @@ pub(crate) fn parse_codex_event_result(stdout: &str) -> Option { summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, }; if let Some(m) = marker.as_ref() { result.exit_code = m.exit_code; @@ -1071,6 +1085,7 @@ pub(crate) fn parse_opencode_event_result(stdout: &str) -> Option { summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, }) } (None, Some((_, result))) => Some(normalise_stream_marker_provenance(result)), @@ -1716,6 +1731,7 @@ fn claude_stream_envelope_failure(result_event: &serde_json::Value) -> Option Opti summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, }); } } @@ -1936,6 +1953,7 @@ fn antigravity_stream_envelope_failure(result_event: &serde_json::Value) -> Opti summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, }) } @@ -2064,6 +2082,7 @@ fn indeterminate_capture_failure() -> AgentResult { summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, } } @@ -2316,6 +2335,7 @@ fn idle_timeout_result(reason: String, commits: Option) -> AgentResult { summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, } } @@ -2378,6 +2398,7 @@ fn rate_limited_result(retry: String) -> AgentResult { summary: None, verdict: None, decided_by_layer: Some(1), + checkpoint: None, } } @@ -2608,6 +2629,7 @@ pub fn evaluate_layer2( summary: None, verdict: None, decided_by_layer: Some(2), + checkpoint: None, })) } @@ -2694,6 +2716,7 @@ pub fn evaluate_layer3( summary: None, verdict: None, decided_by_layer: Some(3), + checkpoint: None, }) } @@ -2750,6 +2773,7 @@ fn evaluate_layer0( summary: None, verdict: None, decided_by_layer: Some(0), + checkpoint: None, }); } let Some(approved_commands) = approved_commands else { @@ -2764,6 +2788,7 @@ fn evaluate_layer0( summary: None, verdict: None, decided_by_layer: Some(0), + checkpoint: None, }); }; if commands != approved_commands { @@ -2775,6 +2800,7 @@ fn evaluate_layer0( summary: None, verdict: None, decided_by_layer: Some(0), + checkpoint: None, }); } match commands @@ -2789,6 +2815,7 @@ fn evaluate_layer0( summary: None, verdict: None, decided_by_layer: Some(0), + checkpoint: None, }), // Every declared, approved probe passed — affirmative completion // evidence on its own (D-05 gap 2), even with zero commits. @@ -2802,6 +2829,7 @@ fn evaluate_layer0( summary: None, verdict: None, decided_by_layer: Some(0), + checkpoint: None, }), } } @@ -4194,33 +4222,143 @@ mod tests { assert!(!blocking_human_checkpoint_reported(stdout)); } - /// Live Attempt 1 (2026-10-07) regression: when Claude spawns a subagent - /// (e.g. `gsd-executor`), the subagent's `Gate: blocking-human` declaration - /// arrives inside a tool result, and the orchestrator summarizes the halt - /// in prose with a terminal `DEVFLOW_RESULT` marker whose `reason` explains - /// the blocking-human checkpoint. DevFlow must recognize this structured - /// marker so auto-decide resume is triggered. + /// 49-REVIEW CR-01: a failed marker's free-text `reason` is never read as a + /// checkpoint, whatever it says. The first row is Live Attempt 1's real + /// reason (2026-10-07), which the reverted prose heuristic matched; the next + /// three are the review's false positives, including a reason that matched + /// only through "investi*gate*". A checkpoint is declared in the marker's + /// `checkpoint` field instead (see the `declared_checkpoint_*` tests). #[test] - fn blocking_human_checkpoint_reported_detects_marker_reason_checkpoint() { - let reason = "Phase 01 plan 01-01 halted at Task 1, a blocking-human decision checkpoint (choose DEFAULT_RETRY_BUDGET_SECS: budget-30/120/300)."; - let inner = format!( - "Phase 1 stopped at Task 1.\n\nDEVFLOW_RESULT: {{\"status\": \"failed\", \"reason\": \"{reason}\"}}" - ); - let escaped_inner = inner.replace('\n', "\\n").replace('"', "\\\""); - let stream = format!( - "{}\n{{\"type\":\"result\",\"subtype\":\"success\",\"result\":\"{escaped_inner}\",\"session_id\":\"sess-p49-a1\"}}\n", - V3_INIT_EVENT - ); + fn blocking_human_checkpoint_reported_ignores_marker_reason_prose() { + let reasons = [ + "Phase 01 plan 01-01 halted at Task 1, a blocking-human decision checkpoint (choose DEFAULT_RETRY_BUDGET_SECS: budget-30/120/300).", + "Resolved the blocking-human gate by choosing budget-120; cargo test then failed", + "no blocking-human checkpoint was reached; tests failed", + "verification failed: SUMMARY does not record the blocking-human decision; investigate", + ]; + for reason in reasons { + let inner = format!( + "Phase 1 stopped.\n\nDEVFLOW_RESULT: {{\"status\": \"failed\", \"reason\": \"{reason}\"}}" + ); + let escaped_inner = inner.replace('\n', "\\n").replace('"', "\\\""); + let stream = format!( + "{}\n{{\"type\":\"result\",\"subtype\":\"success\",\"result\":\"{escaped_inner}\",\"session_id\":\"sess-p49-a1\"}}\n", + V3_INIT_EVENT + ); + assert!( + !blocking_human_checkpoint_reported(&stream), + "a marker reason must not read as a checkpoint in a stream capture: {reason}" + ); + assert!( + !blocking_human_checkpoint_reported(&inner), + "a marker reason must not read as a checkpoint in plain text: {reason}" + ); + } + } + + /// The declared form: a failed marker whose `checkpoint` field names + /// `blocking-human`, matched without regard to ASCII case. + #[test] + fn declared_checkpoint_is_read_from_the_marker_field() { + for value in ["blocking-human", "BLOCKING-HUMAN"] { + let stdout = format!( + "DEVFLOW_RESULT: {{\"status\": \"failed\", \"checkpoint\": \"{value}\", \"reason\": \"Task 1 needs a decision\"}}\n" + ); + let result = parse_devflow_result(&stdout).expect("marker parses"); + assert_eq!(result.checkpoint.as_deref(), Some(value)); + assert!( + result.declares_blocking_human_checkpoint(), + "failed + checkpoint {value} must declare the checkpoint" + ); + } + } + + /// Every other combination declares nothing: a success that names the + /// checkpoint, a failure naming a different checkpoint, a failure with no + /// field, and a failure whose reason (not field) names it. + #[test] + fn declared_checkpoint_requires_a_failed_status_and_the_exact_value() { + let cases = [ + r#"{"status": "success", "checkpoint": "blocking-human"}"#, + r#"{"status": "failed", "checkpoint": "blocking"}"#, + r#"{"status": "failed", "checkpoint": "human-verify"}"#, + r#"{"status": "failed", "checkpoint": " blocking-human"}"#, + r#"{"status": "failed", "reason": "blocking-human checkpoint"}"#, + ]; + for marker in cases { + let result = parse_devflow_result(&format!("DEVFLOW_RESULT: {marker}\n")) + .expect("marker parses"); + assert!( + !result.declares_blocking_human_checkpoint(), + "must not declare a checkpoint: {marker}" + ); + } + } + + /// A malformed `checkpoint` value is dropped, never the whole marker: the + /// `status` must survive so the stage is still decided by Layer 1. + #[test] + fn declared_checkpoint_non_string_value_keeps_the_status() { + for value in ["1", "true", "{}", "[\"blocking-human\"]", "null"] { + let stdout = + format!("DEVFLOW_RESULT: {{\"status\": \"failed\", \"checkpoint\": {value}}}\n"); + let result = parse_devflow_result(&stdout) + .unwrap_or_else(|| panic!("marker with checkpoint {value} must still parse")); + assert_eq!(result.status, AgentStatus::Failed); + assert_eq!( + result.checkpoint, None, + "checkpoint {value} must become None" + ); + assert!(!result.declares_blocking_human_checkpoint()); + } + } + + /// The declaration counts only on the stage's final marker. An earlier + /// marker that declared the checkpoint, followed by a later one that does + /// not, declares nothing; the reverse order declares it. + #[test] + fn declared_checkpoint_counts_only_on_the_final_marker() { + let declared = r#"DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human"}"#; + let ordinary = r#"DEVFLOW_RESULT: {"status": "failed", "reason": "tests failed"}"#; + + let superseded = format!("{declared}\nmore work\n{ordinary}\n"); assert!( - blocking_human_checkpoint_reported(&stream), - "marker reason stating a blocking-human checkpoint halt must be recognized in a stream capture" + !parse_devflow_result(&superseded) + .expect("parses") + .declares_blocking_human_checkpoint() ); + let last = format!("{ordinary}\nmore work\n{declared}\n"); assert!( - blocking_human_checkpoint_reported(&inner), - "marker reason must also be recognized in plain text" + parse_devflow_result(&last) + .expect("parses") + .declares_blocking_human_checkpoint() ); } + /// Same rule in a Claude stream capture: only the last top-level `result` + /// event decides. A declaration in an earlier turn is superseded. + #[test] + fn declared_checkpoint_in_a_stream_counts_only_on_the_last_result() { + let result_event = |marker: &str| { + let inner = format!("Stopped.\n\nDEVFLOW_RESULT: {marker}"); + let escaped = inner.replace('\n', "\\n").replace('"', "\\\""); + format!( + "{{\"type\":\"result\",\"subtype\":\"success\",\"result\":\"{escaped}\",\"session_id\":\"sess-dc\"}}" + ) + }; + let declared = result_event(r#"{"status": "failed", "checkpoint": "blocking-human"}"#); + let ordinary = result_event(r#"{"status": "failed", "reason": "tests failed"}"#); + + let superseded = format!("{V3_INIT_EVENT}\n{declared}\n{ordinary}\n"); + let result = parse_claude_event_result(&superseded).expect("stream parses"); + assert_eq!(result.status, AgentStatus::Failed); + assert!(!result.declares_blocking_human_checkpoint()); + + let last = format!("{V3_INIT_EVENT}\n{ordinary}\n{declared}\n"); + let result = parse_claude_event_result(&last).expect("stream parses"); + assert!(result.declares_blocking_human_checkpoint()); + } + /// Negative control: a failed marker without a blocking-human checkpoint /// reason must NOT report a human gate. #[test] diff --git a/crates/devflow-core/src/prompt.rs b/crates/devflow-core/src/prompt.rs index 655fd274..02b03c70 100644 --- a/crates/devflow-core/src/prompt.rs +++ b/crates/devflow-core/src/prompt.rs @@ -51,6 +51,10 @@ If something prevents completion:\n\ \n\ DEVFLOW_RESULT: {\"status\": \"failed\", \"reason\": \"specific explanation\"}\n\ \n\ +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field:\n\ +\n\ +DEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \"blocking-human\", \"reason\": \"which gate stopped you and the decision it needs\"}\n\ +\n\ Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. \ DevFlow reads this line to decide whether the stage succeeded. Output nothing after it."; @@ -618,8 +622,11 @@ fn stage_prompt_with_project(stage: Stage, phase: PhaseId, project_root: Option< /// The resume wording tells the agent to name the resolved gate in prose rather /// than copy the checkpoint's declaration line. A failed resume capture is /// re-read by `agent_result::text_reports_human_gate`, so a copied declaration -/// line could read as a new checkpoint. The detector accepts only a -/// `**Gate:**`-labeled line; the +/// line could read as a new checkpoint. A checkpoint is reported only by a +/// `**Gate:**`-labeled line or by a final `DEVFLOW_RESULT` that declares +/// `"checkpoint": "blocking-human"` ([`COMPLETION_PROTOCOL`] says to use that +/// field only for a gate being reported rather than resolved). The wording of a +/// failure reason is never read (49-REVIEW CR-01). The /// `resume_prompt_does_not_read_as_a_blocking_human_checkpoint` test pins that /// boundary and rejects prose such as `Resolved gate: blocking-human`. /// @@ -1281,6 +1288,42 @@ mod tests { ); } + /// 49-REVIEW CR-01: the protocol shows agents how to declare a + /// blocking-human checkpoint, and the example it shows is exactly what the + /// parser accepts. Each marker example is parsed with the real parser, so + /// the prompt and `AgentResult` cannot drift apart: the checkpoint example + /// must declare the checkpoint, and the other two must not. + #[test] + fn completion_protocol_checkpoint_example_is_what_the_parser_accepts() { + let examples: Vec<&str> = COMPLETION_PROTOCOL + .lines() + .filter(|line| line.starts_with("DEVFLOW_RESULT:")) + .collect(); + assert_eq!( + examples.len(), + 3, + "success, failure and checkpoint examples: {examples:?}" + ); + let declaring: Vec<&str> = examples + .iter() + .copied() + .filter(|line| { + crate::agent_result::parse_devflow_result(line) + .unwrap_or_else(|| panic!("protocol example must parse: {line}")) + .declares_blocking_human_checkpoint() + }) + .collect(); + assert_eq!( + declaring.len(), + 1, + "exactly one protocol example declares the checkpoint: {examples:?}" + ); + assert!( + COMPLETION_PROTOCOL.contains("blocking-human gate"), + "the protocol must say when to use the checkpoint form" + ); + } + #[test] fn checkpoint_auto_decide_prompt_terminates_with_completion_protocol() { let prompt = checkpoint_auto_decide_prompt(PhaseId::new(28)); diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap index 581ad822..2e463b01 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap @@ -28,4 +28,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap index 581ad822..2e463b01 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap @@ -28,4 +28,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap index 581ad822..2e463b01 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap @@ -28,4 +28,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap index feadcc6c..10803386 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap @@ -26,4 +26,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap index 581ad822..2e463b01 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap @@ -28,4 +28,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap index 581ad822..2e463b01 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap @@ -28,4 +28,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap index f086cfef..68ba6dd6 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap @@ -26,4 +26,8 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} +If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: + +DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} + Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. From 64dcba7d4e532f02be698a77e170381fc3f5c63b Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Fri, 9 Oct 2026 21:53:49 -0400 Subject: [PATCH 2/6] fix(agent_result): accept a declared checkpoint only from a Code failure Act on the external review of the declared-checkpoint fix: - A marker's checkpoint is dropped unless the marker itself reports failure, so the exit-code reconcile can no longer turn a success that named a checkpoint into a declaring failure. - The pipeline reads the declared field only for the Code stage, the one stage that runs plans. - The declaration sentence moves from the protocol every stage receives to the Code-stage policy. The auto-decide resume prompt no longer re-offers it, and its example sits mid-sentence, so an echoed prompt cannot parse as a declaring marker. It now names a subagent's halt. - An absent checkpoint is not serialized. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0182ocSM2E4fb2zu52jxGLzi --- crates/devflow-cli/src/pipeline_launch.rs | 72 +++++++++- ...e_launch__tests__turn_one_code_prompt.snap | 6 +- ...launch__tests__turn_two_resume_prompt.snap | 4 - crates/devflow-core/src/agent_result.rs | 48 ++++++- crates/devflow-core/src/prompt.rs | 127 +++++++++++++----- ...tyle_full_execute_fix_prompt_snapshot.snap | 6 +- ...__full_execute_fix_prompt_antigravity.snap | 6 +- ...tests__full_execute_fix_prompt_claude.snap | 6 +- ..._tests__full_execute_fix_prompt_codex.snap | 6 +- ...tests__full_execute_fix_prompt_hermes.snap | 6 +- ...sts__full_execute_fix_prompt_opencode.snap | 6 +- ...pt__tests__full_execute_fix_prompt_pi.snap | 6 +- 12 files changed, 213 insertions(+), 86 deletions(-) diff --git a/crates/devflow-cli/src/pipeline_launch.rs b/crates/devflow-cli/src/pipeline_launch.rs index c8af1e75..a8df82db 100644 --- a/crates/devflow-cli/src/pipeline_launch.rs +++ b/crates/devflow-cli/src/pipeline_launch.rs @@ -1762,9 +1762,10 @@ pub(crate) fn advance_with( // the PRIMARY, agent-uncontrollable gate, checked BEFORE // anything agent-controlled (T-28-01); (3) the agent reported // one, either by declaring `"checkpoint": "blocking-human"` on the - // stage's final DEVFLOW_RESULT marker or by a `Gate:` declaration - // line in the capture — never by the wording of a failure reason - // (49-REVIEW CR-01); (4) a session id is on record; and (5) the + // stage's final DEVFLOW_RESULT marker (Code only: it is the one + // stage that runs plans and the only one offered the field) or by + // a `Gate:` declaration line in the capture — never by the wording + // of a failure reason (49-REVIEW CR-01); (4) a session id is on record; and (5) the // resume ceiling has not been exhausted. A changed declaration // always opens the re-scan gate. Only an unchanged, recorded set // in Auto resumes without a human; Supervise falls through to a @@ -1786,7 +1787,7 @@ pub(crate) fn advance_with( let execution_root = state.worktree_path.as_deref().unwrap_or(project_root); let checkpoint_confirmed = state.agent == AgentKind::Claude && verify::phase_has_blocking_human_checkpoint(execution_root, phase) - && (result.declares_blocking_human_checkpoint() + && ((stage == Stage::Code && result.declares_blocking_human_checkpoint()) || agent_result::checkpoint_reported_in_capture(project_root, phase)); if checkpoint_confirmed { let ceiling_ok = state.checkpoint_resumes < mode::MAX_CHECKPOINT_RESUMES; @@ -5396,6 +5397,69 @@ mod tests { assert!(!events_of_kind(root, "gate_fired").is_empty()); } + /// Only the Code stage runs plans, so only Code may declare a checkpoint + /// through the marker field. A Plan failure carrying the same marker takes + /// the ordinary gate (external review 2026-10-09: the arm has no stage + /// guard, and Ship or Plan would otherwise be resumed to "resolve" a gate). + #[test] + fn advance_with_marker_declared_checkpoint_outside_code_falls_through() { + const NAME: &str = "pipeline_launch::tests::advance_with_marker_declared_checkpoint_outside_code_falls_through"; + enter_agent_free_child!(NAME, "claude"); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_repo(root); + + let phase = PhaseId::new(188); + write_declared_checkpoint_plan(root, phase); + write_marker_declared_checkpoint_capture(root, phase); + write_abort_gate_response(root, phase, Stage::Plan); + + let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf()); + state.stage = Stage::Plan; + state.session_id = Some("sess-plan-declared".to_string()); + state.checkpoint_approval = recorded_approval_for(root, phase); + workflow::save_state(&state).unwrap(); + + advance_for_stage(root, Some(phase), Some(Stage::Plan)).unwrap(); + + assert!(events_of_kind(root, "checkpoint_auto_decided").is_empty()); + assert!(!events_of_kind(root, "gate_fired").is_empty()); + } + + /// The field must name `blocking-human` exactly: a Code failure that + /// declares some other checkpoint takes the ordinary gate. Catches a + /// pipeline that tests only for the field's presence. + #[test] + fn advance_with_other_marker_checkpoint_value_falls_through() { + const NAME: &str = + "pipeline_launch::tests::advance_with_other_marker_checkpoint_value_falls_through"; + enter_agent_free_child!(NAME, "claude"); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_repo(root); + + let phase = PhaseId::new(189); + write_declared_checkpoint_plan(root, phase); + std::fs::create_dir_all(root.join(".devflow")).unwrap(); + std::fs::write( + agent_result::stdout_path(root, phase), + "DEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \"human-verify\", \"reason\": \"Task 2 needs a look\"}\n", + ) + .unwrap(); + write_abort_gate_response(root, phase, Stage::Code); + + let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf()); + state.stage = Stage::Code; + state.session_id = Some("sess-other-value".to_string()); + state.checkpoint_approval = recorded_approval_for(root, phase); + workflow::save_state(&state).unwrap(); + + advance_for_stage(root, Some(phase), Some(Stage::Code)).unwrap(); + + assert!(events_of_kind(root, "checkpoint_auto_decided").is_empty()); + assert!(!events_of_kind(root, "gate_fired").is_empty()); + } + #[test] fn advance_with_declared_checkpoint_but_unreported_gate_falls_through() { const NAME: &str = "pipeline_launch::tests::advance_with_declared_checkpoint_but_unreported_gate_falls_through"; diff --git a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap index 1e27e363..c8155286 100644 --- a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap +++ b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_one_code_prompt.snap @@ -16,7 +16,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -28,8 +28,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap index 64eb5527..d13257b7 100644 --- a/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap +++ b/crates/devflow-cli/src/snapshots/devflow__pipeline_launch__tests__turn_two_resume_prompt.snap @@ -14,8 +14,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/agent_result.rs b/crates/devflow-core/src/agent_result.rs index 305e48e0..202243e4 100644 --- a/crates/devflow-core/src/agent_result.rs +++ b/crates/devflow-core/src/agent_result.rs @@ -49,8 +49,14 @@ pub struct AgentResult { /// (49-REVIEW CR-01), which matched ordinary failures that mentioned a /// gate. Deserialized leniently via [`deserialize_checkpoint_lenient`]: a /// non-string value becomes `None` rather than failing the whole parse, so - /// a malformed field can never drop a valid `status` to Layer 2. - #[serde(default, deserialize_with = "deserialize_checkpoint_lenient")] + /// a malformed field can never drop a valid `status` to Layer 2. Dropped at + /// parse time unless the marker's own status is `failed` + /// ([`parse_marker_lines`]), and never serialized when absent. + #[serde( + default, + deserialize_with = "deserialize_checkpoint_lenient", + skip_serializing_if = "Option::is_none" + )] pub checkpoint: Option, } @@ -2145,7 +2151,15 @@ fn parse_marker_lines(stdout: &str) -> Option { } let json_str = line[PREFIX.len()..].trim(); - if let Ok(result) = serde_json::from_str::(json_str) { + if let Ok(mut result) = serde_json::from_str::(json_str) { + // A declaration belongs to the failure the agent reported. Dropped + // here, at the one place agent JSON becomes an AgentResult, so no + // later status rewrite (the exit-code reconcile turns a reported + // success into a failure with `..result`) can carry it onto a + // failure the agent never declared. + if result.status != AgentStatus::Failed { + result.checkpoint = None; + } return Some(result); } } @@ -4295,6 +4309,34 @@ mod tests { } } + /// A success marker never carries a declaration forward. The exit-code + /// reconcile rewrites a reported success to a failure with `..result`, so a + /// `checkpoint` kept on the success would become a failed, declaring result + /// that the agent never reported (external review 2026-10-09). + #[test] + fn declared_checkpoint_on_a_success_marker_does_not_survive_the_exit_code_rewrite() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let phase = PhaseId::new(1); + std::fs::create_dir_all(root.join(".devflow")).unwrap(); + std::fs::write(exit_code_path(root, phase), "1").unwrap(); + + let marker = r#"DEVFLOW_RESULT: {"status": "success", "checkpoint": "blocking-human"}"#; + let parsed = parse_devflow_result(marker).expect("marker parses"); + assert_eq!( + parsed.checkpoint, None, + "a success marker keeps no checkpoint" + ); + + let reconciled = reconcile_stream_success_against_exit_code(root, phase, parsed); + assert_eq!( + reconciled.status, + AgentStatus::Failed, + "exit 1 rewrites success" + ); + assert!(!reconciled.declares_blocking_human_checkpoint()); + } + /// A malformed `checkpoint` value is dropped, never the whole marker: the /// `status` must survive so the stage is still decided by Layer 1. #[test] diff --git a/crates/devflow-core/src/prompt.rs b/crates/devflow-core/src/prompt.rs index 02b03c70..eba9539e 100644 --- a/crates/devflow-core/src/prompt.rs +++ b/crates/devflow-core/src/prompt.rs @@ -51,10 +51,6 @@ If something prevents completion:\n\ \n\ DEVFLOW_RESULT: {\"status\": \"failed\", \"reason\": \"specific explanation\"}\n\ \n\ -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field:\n\ -\n\ -DEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \"blocking-human\", \"reason\": \"which gate stopped you and the decision it needs\"}\n\ -\n\ Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. \ DevFlow reads this line to decide whether the stage succeeded. Output nothing after it."; @@ -91,6 +87,32 @@ macro_rules! gate_resolution_rule { /// 2026-09-11). Do not widen it without a new decision. pub const GATE_RESOLUTION_RULE: &str = gate_resolution_rule!(); +/// How an agent reports a `blocking-human` gate it is not resolving: the +/// `checkpoint` field on its final marker, which the pipeline reads through +/// [`crate::agent_result::AgentResult::declares_blocking_human_checkpoint`] +/// (49-REVIEW CR-01). It names a subagent's halt too, because under GSD the +/// executor subagent stops and the orchestrator writes the marker. +/// +/// It lives in [`CODE_STAGE_POLICY`] and not in [`COMPLETION_PROTOCOL`] for two +/// reasons. Only the Code stage runs plans, so only Code may declare (the +/// pipeline ignores the field elsewhere), and the auto-decide resume prompt, +/// which carries only the protocol, does not re-offer it. The example sits +/// inside a sentence, never at the start of a line, so an echoed prompt cannot +/// parse as a declaring marker. A macro for the same `concat!` reason as +/// [`gate_resolution_rule`]. +macro_rules! checkpoint_declaration { + () => { + "When you report a blocking-human gate that you or a subagent you ran \ + stopped at, say so on your final line with the checkpoint field, for \ + example `DEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \ + \"blocking-human\", \"reason\": \"which gate stopped and the decision it \ + needs\"}`. Use the checkpoint field for nothing else: not for a gate you \ + resolved, and not for any other failure." + }; +} +#[cfg(test)] +const CHECKPOINT_DECLARATION: &str = checkpoint_declaration!(); + /// Shared policy for full-execute Code prompts. This advises the agent during /// one-shot Code execution, while [`checkpoint_auto_decide_prompt`] is injected /// into a resumed session after DevFlow's own human-blocking gate finds no @@ -126,6 +148,8 @@ of the decision.\n\ This authority never extends to a package-verification checkpoint: do not \ self-resolve or approve one under any circumstances, and report it instead. ", gate_resolution_rule!(), + " ", + checkpoint_declaration!(), " This policy must not pause execution or request human input." ); @@ -1288,40 +1312,73 @@ mod tests { ); } - /// 49-REVIEW CR-01: the protocol shows agents how to declare a - /// blocking-human checkpoint, and the example it shows is exactly what the - /// parser accepts. Each marker example is parsed with the real parser, so - /// the prompt and `AgentResult` cannot drift apart: the checkpoint example - /// must declare the checkpoint, and the other two must not. + /// 49-REVIEW CR-01: the Code policy's checkpoint example is exactly what + /// the parser accepts as a declaration, so the prompt and `AgentResult` + /// cannot drift apart. The sentence is pinned verbatim, so a widening such + /// as "declare it on any failure" fails here. #[test] - fn completion_protocol_checkpoint_example_is_what_the_parser_accepts() { - let examples: Vec<&str> = COMPLETION_PROTOCOL - .lines() - .filter(|line| line.starts_with("DEVFLOW_RESULT:")) - .collect(); - assert_eq!( - examples.len(), - 3, - "success, failure and checkpoint examples: {examples:?}" - ); - let declaring: Vec<&str> = examples - .iter() - .copied() - .filter(|line| { - crate::agent_result::parse_devflow_result(line) - .unwrap_or_else(|| panic!("protocol example must parse: {line}")) - .declares_blocking_human_checkpoint() - }) - .collect(); - assert_eq!( - declaring.len(), - 1, - "exactly one protocol example declares the checkpoint: {examples:?}" - ); + fn checkpoint_declaration_example_is_what_the_parser_accepts() { assert!( - COMPLETION_PROTOCOL.contains("blocking-human gate"), - "the protocol must say when to use the checkpoint form" + CODE_STAGE_POLICY.contains(CHECKPOINT_DECLARATION), + "the Code policy must carry the declaration sentence" ); + assert!(CHECKPOINT_DECLARATION.starts_with( + "When you report a blocking-human gate that you or a subagent you ran stopped at," + )); + assert!(CHECKPOINT_DECLARATION.ends_with( + "Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure." + )); + let example = CHECKPOINT_DECLARATION + .split('`') + .find(|part| part.starts_with("DEVFLOW_RESULT:")) + .expect("the sentence quotes one example marker"); + let parsed = crate::agent_result::parse_devflow_result(example) + .unwrap_or_else(|| panic!("the example must parse: {example}")); + assert!(parsed.declares_blocking_human_checkpoint(), "{example}"); + } + + /// An echoed prompt must never read as a declaring marker. Every marker + /// line that starts a line in a delivered prompt (the parser only reads + /// line-start markers) parses as a non-declaring example, for the Code + /// prompt that carries the policy and for the policy-carrying fix prompt. + #[test] + fn no_line_start_marker_in_a_code_prompt_declares_a_checkpoint() { + let prompts = [ + stage_prompt(Stage::Code, PhaseId::new(47)), + fix_prompt(FixType::FullExecute, PhaseId::new(47)), + ]; + for prompt in prompts { + assert!(prompt.contains(CHECKPOINT_DECLARATION)); + let markers: Vec<&str> = prompt + .lines() + .filter(|line| line.starts_with("DEVFLOW_RESULT:")) + .collect(); + assert!(!markers.is_empty(), "the protocol examples are present"); + for marker in markers { + let parsed = crate::agent_result::parse_devflow_result(marker) + .unwrap_or_else(|| panic!("protocol example must parse: {marker}")); + assert!(!parsed.declares_blocking_human_checkpoint(), "{marker}"); + } + assert!( + !crate::agent_result::parse_devflow_result(&prompt) + .is_some_and(|result| result.declares_blocking_human_checkpoint()), + "a whole echoed prompt must not parse as a declaration" + ); + } + } + + /// Only Code may declare. The resume prompt and every non-Code stage + /// prompt carry no checkpoint form, so a resumed agent is not re-offered + /// it and Define, Plan, Validate and Ship are not invited to use it. + #[test] + fn checkpoint_declaration_is_offered_only_to_the_code_stage() { + let resume = checkpoint_auto_decide_prompt(PhaseId::new(47)); + assert!(!resume.contains("\"checkpoint\""), "{resume}"); + assert!(!COMPLETION_PROTOCOL.contains("checkpoint")); + for stage in [Stage::Define, Stage::Plan, Stage::Validate, Stage::Ship] { + let prompt = stage_prompt(stage, PhaseId::new(47)); + assert!(!prompt.contains("\"checkpoint\""), "{stage:?}: {prompt}"); + } } #[test] diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap index 2e463b01..942e4e44 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__claude_style_full_execute_fix_prompt_snapshot.snap @@ -16,7 +16,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -28,8 +28,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap index 2e463b01..942e4e44 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_antigravity.snap @@ -16,7 +16,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -28,8 +28,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap index 2e463b01..942e4e44 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_claude.snap @@ -16,7 +16,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -28,8 +28,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap index 10803386..4468e740 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_codex.snap @@ -14,7 +14,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -26,8 +26,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap index 2e463b01..942e4e44 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_hermes.snap @@ -16,7 +16,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -28,8 +28,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap index 2e463b01..942e4e44 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_opencode.snap @@ -16,7 +16,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -28,8 +28,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. diff --git a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap index 68ba6dd6..13e99e01 100644 --- a/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap +++ b/crates/devflow-core/src/snapshots/devflow_core__prompt__tests__full_execute_fix_prompt_pi.snap @@ -14,7 +14,7 @@ When you reach a `decision` checkpoint and no operator is available, resolve it An option marked recommended is evidence to weigh, not an automatically decisive answer. In your final message, record the reasoning and comparison that produced the choice: which options you considered and why the chosen one won, not merely a sentence asserting the choice. The final message is the only record of the decision. -This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. This policy must not pause execution or request human input. +This authority never extends to a package-verification checkpoint: do not self-resolve or approve one under any circumstances, and report it instead. A blocking-human gate is human-only, with one exception: if DevFlow resumed you specifically to resolve that gate, resolve it yourself and record your reasoning in your final message; otherwise report it instead. When you report a blocking-human gate that you or a subagent you ran stopped at, say so on your final line with the checkpoint field, for example `DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped and the decision it needs"}`. Use the checkpoint field for nothing else: not for a gate you resolved, and not for any other failure. This policy must not pause execution or request human input. ## Completion Protocol (REQUIRED) @@ -26,8 +26,4 @@ If something prevents completion: DEVFLOW_RESULT: {"status": "failed", "reason": "specific explanation"} -If you stopped at a blocking-human gate that you are reporting rather than resolving, declare it with the checkpoint field: - -DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human", "reason": "which gate stopped you and the decision it needs"} - Anything else your final message must contain, such as the reasoning for a decision, goes above the DEVFLOW_RESULT line. DevFlow reads this line to decide whether the stage succeeded. Output nothing after it. From 1b403d7fca8d14c0fa6dfaaedd2bf2db39ca261c Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Sat, 10 Oct 2026 05:54:47 -0400 Subject: [PATCH 3/6] fix(agent_result): let the latest outcome decide a stream checkpoint The stream checkpoint check scanned every top-level result with `.any`, so a gate declared in an earlier turn outranked a later, ordinary failure: the stage was resumed and told to decide the human's gate. Read results newest first instead. The first that declares the gate or carries a DEVFLOW_RESULT marker decides. Silent wake-up turns are still skipped, so a declaration followed only by them is still reported. Operator decision 2026-10-10, from the external review of the CR-01 fix. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0182ocSM2E4fb2zu52jxGLzi --- crates/devflow-core/src/agent_result.rs | 90 ++++++++++++++++++++----- 1 file changed, 72 insertions(+), 18 deletions(-) diff --git a/crates/devflow-core/src/agent_result.rs b/crates/devflow-core/src/agent_result.rs index 202243e4..73018ee6 100644 --- a/crates/devflow-core/src/agent_result.rs +++ b/crates/devflow-core/src/agent_result.rs @@ -1563,15 +1563,21 @@ pub(crate) fn event_is_top_level_result_marker(event: &serde_json::Value) -> boo /// fail independently, so a future widening of the type filter cannot /// silently inherit subagent content. /// -/// **ALL eligible `result` events are scanned, not only the last.** This -/// deliberately diverges from [`last_top_level_result`]'s last-result-wins -/// verdict semantics, and the two conventions must not be "harmonised": a -/// verdict is a single final answer, whereas this asks whether a gate was -/// reported ANYWHERE in the stage's output. A gate declared in turn N followed -/// by task-notification wake-up turns N+1/N+2 — the exact turn shape the v3 -/// capture archives — would be silently dropped by last-result-only, losing a -/// human authorization request to the generic gate. That is the -/// opposite-direction harm, and the worse of the two. +/// **Eligible `result` events are read newest first, and the latest stated +/// outcome wins** (operator decision 2026-10-10). The first event, counting +/// back from the end, that either declares the gate or carries a +/// `DEVFLOW_RESULT` marker decides: a declaration means a checkpoint, a marker +/// without one means the stage ended on that outcome instead. A declaration +/// and a marker in the same message are one report, so the declaration wins +/// within a message. Events with neither, such as task-notification wake-up +/// turns, are skipped, so a gate declared in turn N followed by silent turns +/// N+1/N+2 (the turn shape the v3 capture archives) is still reported, which +/// is what the earlier scan-everything rule existed to protect. +/// +/// What this replaces: scanning every event with `.any` let an earlier +/// declaration outrank a later, ordinary failure, so a stage that failed for an +/// unrelated reason was resumed and told to decide the human's gate (external +/// review of the CR-01 fix, 2026-10-09). /// /// Text is read with a direct [`serde_json::Value::get`] chain. Never route /// this through [`json_scan`]/[`json_find_key`]: a recursive traversal descends @@ -1583,12 +1589,21 @@ pub(crate) fn event_is_top_level_result_marker(event: &serde_json::Value) -> boo /// the eligible text: this runs on every `devflow advance` over a capture that /// grows for the whole stage, and there is no reason to allocate a copy of it. fn claude_stream_reports_human_gate(events: &[serde_json::Value]) -> bool { - events + for text in events .iter() + .rev() .filter(|event| event.get("type").and_then(serde_json::Value::as_str) == Some("result")) .filter(|event| is_top_level(event)) .filter_map(|event| event.get("result").and_then(serde_json::Value::as_str)) - .any(text_reports_human_gate) + { + if text_reports_human_gate(text) { + return true; + } + if parse_marker_lines(text).is_some() { + return false; + } + } + false } /// The `rate_limit_info.status` values that mean the CLI DENIED the request. @@ -4560,11 +4575,12 @@ mod tests { ); } - /// **T-30-27.** Detection asks whether a gate fired ANYWHERE in the stage, - /// so it deliberately does NOT inherit plan 30-01's last-result-wins - /// verdict semantics. A gate declared in turn 1 followed by - /// task-notification wake-up turns — the exact turn shape the v3 capture - /// archives — must not be dropped in favour of the later, silent results. + /// **T-30-27.** A gate declared in turn 1 followed by task-notification + /// wake-up turns that state no outcome (the exact turn shape the v3 + /// capture archives) must not be dropped in favour of the later, silent + /// results. Silent turns are skipped; only a later stated outcome + /// supersedes the declaration (see the `..._later_result_reports_a_plain_failure` + /// test). /// /// Losing a checkpoint report is the opposite-direction harm from the false /// positive this plan closes, and the worse of the two: it silently drops a @@ -4574,9 +4590,47 @@ mod tests { let capture = v3_stream_capture(&gate_declaration_text(), NO_MARKER, NO_MARKER); assert!( blocking_human_checkpoint_reported(&capture), - "detection must scan every top-level `result` event, not only the \ - last one" + "silent later turns must not drop an earlier gate declaration" + ); + } + + /// The latest outcome wins (operator decision 2026-10-10, after the + /// external review of the CR-01 fix). A gate declared in turn 1, followed + /// by a later result that reports a plain failure, is that failure: the + /// agent's later word supersedes its earlier note, so the run stops at the + /// ordinary gate instead of resuming the agent to decide for the human. + /// Asserted through the on-disk reader the pipeline calls, too. + #[test] + fn blocking_human_checkpoint_reported_false_when_a_later_result_reports_a_plain_failure() { + let capture = v3_stream_capture(&gate_declaration_text(), MARKER_FAILED, NO_MARKER); + assert!( + !blocking_human_checkpoint_reported(&capture), + "a later plain-failure result must supersede an earlier gate declaration" ); + + let dir = tempfile::tempdir().unwrap(); + let phase = PhaseId::new(1); + std::fs::create_dir_all(dir.path().join(".devflow")).unwrap(); + std::fs::write(stdout_path(dir.path(), phase), &capture).unwrap(); + assert!(!checkpoint_reported_in_capture(dir.path(), phase)); + } + + /// The reverse order: a gate declared after the last stated failure is the + /// latest word, so it counts. + #[test] + fn blocking_human_checkpoint_reported_true_when_a_gate_follows_the_last_failure() { + let capture = v3_stream_capture(MARKER_FAILED, &gate_declaration_text(), NO_MARKER); + assert!(blocking_human_checkpoint_reported(&capture)); + } + + /// A gate declaration and a failure marker in the SAME final message are + /// one report: the agent stopped at the gate and said so. This is the + /// shape DevFlow's own checkpoint fixtures use. + #[test] + fn blocking_human_checkpoint_reported_true_when_gate_and_marker_share_the_latest_result() { + let same_message = format!("{}{MARKER_FAILED}", gate_declaration_text()); + let capture = v3_stream_capture(MARKER_FAILED, &same_message, NO_MARKER); + assert!(blocking_human_checkpoint_reported(&capture)); } /// The overcorrection guard: an echo and a genuine declaration can coexist From dfb2043276d61c28eb30528efa5ed7172432dbbe Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Sat, 10 Oct 2026 07:51:38 -0400 Subject: [PATCH 4/6] fix(agent_result): read every capture newest first, line by line The latest stated outcome decides a checkpoint in every capture form: a DEVFLOW_RESULT marker or a Gate line, read newest first, line by line, over the whole text. The declaration is offered and accepted only in the Code stage, a success marker's checkpoint is dropped, and an absent checkpoint is not serialized. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0182ocSM2E4fb2zu52jxGLzi --- crates/devflow-cli/src/pipeline_launch.rs | 59 +++++- crates/devflow-core/src/agent_result.rs | 238 ++++++++++++++++------ crates/devflow-core/src/prompt.rs | 12 +- 3 files changed, 235 insertions(+), 74 deletions(-) diff --git a/crates/devflow-cli/src/pipeline_launch.rs b/crates/devflow-cli/src/pipeline_launch.rs index a8df82db..55a47d28 100644 --- a/crates/devflow-cli/src/pipeline_launch.rs +++ b/crates/devflow-cli/src/pipeline_launch.rs @@ -1757,15 +1757,18 @@ pub(crate) fn advance_with( // dispatch, check whether this failure is actually a confirmed // human-blocking checkpoint DevFlow can resolve unattended in // Auto mode by resuming the exact session that raised it. Evaluated IN THIS - // ORDER — load-bearing — (1) agent is Claude (D-05); (2) the + // ORDER — load-bearing — (1) agent is Claude (D-05) and the stage + // is Code, the one stage that runs plans and so the only one that + // can stop at a plan's checkpoint (second external review round of + // the CR-01 fix, 2026-10-10); (2) the // phase's plans statically declare a blocking-human checkpoint — // the PRIMARY, agent-uncontrollable gate, checked BEFORE // anything agent-controlled (T-28-01); (3) the agent reported // one, either by declaring `"checkpoint": "blocking-human"` on the - // stage's final DEVFLOW_RESULT marker (Code only: it is the one - // stage that runs plans and the only one offered the field) or by - // a `Gate:` declaration line in the capture — never by the wording - // of a failure reason (49-REVIEW CR-01); (4) a session id is on record; and (5) the + // stage's final DEVFLOW_RESULT marker or by a `Gate:` declaration + // line with no stated outcome after it — the latest statement wins + // (operator decision 2026-10-10) — and never by the wording of a + // failure reason (49-REVIEW CR-01); (4) a session id is on record; and (5) the // resume ceiling has not been exhausted. A changed declaration // always opens the re-scan gate. Only an unchanged, recorded set // in Auto resumes without a human; Supervise falls through to a @@ -1786,8 +1789,9 @@ pub(crate) fn advance_with( let mut reason = result.reason.clone(); let execution_root = state.worktree_path.as_deref().unwrap_or(project_root); let checkpoint_confirmed = state.agent == AgentKind::Claude + && stage == Stage::Code && verify::phase_has_blocking_human_checkpoint(execution_root, phase) - && ((stage == Stage::Code && result.declares_blocking_human_checkpoint()) + && (result.declares_blocking_human_checkpoint() || agent_result::checkpoint_reported_in_capture(project_root, phase)); if checkpoint_confirmed { let ceiling_ok = state.checkpoint_resumes < mode::MAX_CHECKPOINT_RESUMES; @@ -4035,10 +4039,14 @@ mod tests { /// marker, so `advance()`'s Layer 1 deterministically classifies the /// outcome as `Failed` (-> `Action::GateReview`) with no exit-code/pid /// file or background thread needed. + /// A confirmed checkpoint in the current convention: the relayed `Gate:` + /// block, then a final marker that declares the checkpoint. A plain + /// failure marker after the gate would be the latest stated outcome and + /// would not confirm it (operator decision 2026-10-10). fn write_confirmed_checkpoint_capture(root: &Path, phase: PhaseId) { std::fs::create_dir_all(root.join(".devflow")).unwrap(); let stdout = format!( - "## CHECKPOINT REACHED\n\n**Type:** human-verify\n**Gate:** {HUMAN_GATE_VALUE_FOR_TEST}\n\nDEVFLOW_RESULT: {{\"status\": \"failed\", \"reason\": \"checkpoint pending\"}}\n" + "## CHECKPOINT REACHED\n\n**Type:** human-verify\n**Gate:** {HUMAN_GATE_VALUE_FOR_TEST}\n\nDEVFLOW_RESULT: {{\"status\": \"failed\", \"checkpoint\": \"{HUMAN_GATE_VALUE_FOR_TEST}\", \"reason\": \"checkpoint pending\"}}\n" ); std::fs::write(agent_result::stdout_path(root, phase), stdout).unwrap(); } @@ -4165,6 +4173,9 @@ mod tests { let root = root.as_path(); let phase = PhaseId::new(88); + // Unused on the relaunch route; if detection regresses, the + // fall-through gate reads it and returns instead of blocking forever. + write_abort_gate_response(root, phase, Stage::Code); let result = advance_for_stage(root, Some(phase), Some(Stage::Code)); let reloaded_for_reap = workflow::load_state(root, phase).ok(); @@ -5426,6 +5437,40 @@ mod tests { assert!(!events_of_kind(root, "gate_fired").is_empty()); } + /// The whole checkpoint route is Code-only, the `Gate:` line included: a + /// Plan failure whose capture declares the gate takes the ordinary gate + /// (second external review round, 2026-10-10). + #[test] + fn advance_with_gate_line_checkpoint_outside_code_falls_through() { + const NAME: &str = + "pipeline_launch::tests::advance_with_gate_line_checkpoint_outside_code_falls_through"; + enter_agent_free_child!(NAME, "claude"); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_repo(root); + + let phase = PhaseId::new(190); + write_declared_checkpoint_plan(root, phase); + std::fs::create_dir_all(root.join(".devflow")).unwrap(); + std::fs::write( + agent_result::stdout_path(root, phase), + format!("## CHECKPOINT REACHED\n\n**Gate:** {HUMAN_GATE_VALUE_FOR_TEST}\n"), + ) + .unwrap(); + write_abort_gate_response(root, phase, Stage::Plan); + + let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf()); + state.stage = Stage::Plan; + state.session_id = Some("sess-plan-gate-line".to_string()); + state.checkpoint_approval = recorded_approval_for(root, phase); + workflow::save_state(&state).unwrap(); + + advance_for_stage(root, Some(phase), Some(Stage::Plan)).unwrap(); + + assert!(events_of_kind(root, "checkpoint_auto_decided").is_empty()); + assert!(!events_of_kind(root, "gate_fired").is_empty()); + } + /// The field must name `blocking-human` exactly: a Code failure that /// declares some other checkpoint takes the ordinary gate. Catches a /// pipeline that tests only for the field's presence. diff --git a/crates/devflow-core/src/agent_result.rs b/crates/devflow-core/src/agent_result.rs index 73018ee6..e21f5da2 100644 --- a/crates/devflow-core/src/agent_result.rs +++ b/crates/devflow-core/src/agent_result.rs @@ -684,11 +684,14 @@ const HUMAN_GATE_VALUE: &str = "blocking-human"; /// checkpoint" heuristic (D-02 rejected that class of predicate); the scope /// is one declared field label with one enumerated value. /// -/// The other declared form, a final `DEVFLOW_RESULT` marker carrying -/// `"checkpoint": "blocking-human"`, is not read here. The pipeline reads it -/// from the stage's already-evaluated result -/// ([`AgentResult::declares_blocking_human_checkpoint`]), so it follows the -/// same last-marker-wins rule as the stage verdict. A marker's free-text +/// **The latest stated outcome wins** (operator decision 2026-10-10). A +/// `DEVFLOW_RESULT` marker and a `Gate:` declaration line both state an +/// outcome; the first found reading back from the end decides +/// ([`latest_stated_outcome`]). A marker that declares +/// `"checkpoint": "blocking-human"` is a checkpoint, any other marker is not, +/// and a `Gate:` line counts only with no marker after it. The pipeline also +/// reads the declared field from the stage's evaluated result +/// ([`AgentResult::declares_blocking_human_checkpoint`]). A marker's free-text /// `reason` is never read as a checkpoint: a substring heuristic over it /// matched ordinary failures and was removed (49-REVIEW CR-01). /// @@ -715,12 +718,10 @@ pub fn blocking_human_checkpoint_reported(stdout: &str) -> bool { if classify(&capture) == CaptureKind::ClaudeStream { return claude_stream_reports_human_gate(&capture.events); } - if capture_reports_human_gate(stdout) { - return true; + if let Some(inner) = extract_json_result_text(stdout) { + return latest_stated_outcome(&inner) == StatedOutcome::Checkpoint; } - extract_json_result_text(stdout) - .as_deref() - .is_some_and(text_reports_human_gate) + capture_reports_human_gate(stdout) } /// The raw-capture reader behind [`blocking_human_checkpoint_reported`]'s @@ -736,14 +737,24 @@ pub fn blocking_human_checkpoint_reported(stdout: &str) -> bool { /// only where it really is a JSON string, and only once, so a JSON string that /// itself quotes an escaped sample stays a sample. fn capture_reports_human_gate(stdout: &str) -> bool { - stdout.lines().any( - |line| match serde_json::from_str::(line.trim()) { + for line in stdout.lines().rev() { + let outcome = match serde_json::from_str::(line.trim()) { Ok(value) if value.is_object() || value.is_array() => { - json_strings_report_human_gate(&value) + if json_strings_report_human_gate(&value) { + StatedOutcome::Checkpoint + } else { + StatedOutcome::Silent + } } - _ => text_reports_human_gate(line), - }, - ) + _ => latest_stated_outcome(line), + }; + match outcome { + StatedOutcome::Checkpoint => return true, + StatedOutcome::Other => return false, + StatedOutcome::Silent => {} + } + } + false } /// Match every string value inside a decoded JSON document. @@ -779,25 +790,62 @@ fn json_strings_report_human_gate(value: &serde_json::Value) -> bool { /// Note the closing backtick needs no handling: `take_while` already stops /// at it, since a backtick is neither alphanumeric nor `-`. fn text_reports_human_gate(text: &str) -> bool { - text.lines().any(|line| { - let lower = line.to_ascii_lowercase(); - let line = strip_leading_markup(&lower); - let Some(after_label) = line.strip_prefix("gate") else { - return false; - }; - let after_label = after_label.trim_start_matches(['*', ' ', '`']); - if let Some(rest) = after_label.strip_prefix(':') { - let value_region = rest.trim_start_matches(['*', ' ', '\t', '`']); - let value_token: String = value_region - .chars() - .take_while(|c| c.is_ascii_alphanumeric() || *c == '-') - .collect(); - if value_token == HUMAN_GATE_VALUE { - return true; - } + text.lines().any(line_reports_human_gate) +} + +/// One physical line of [`text_reports_human_gate`]. +fn line_reports_human_gate(line: &str) -> bool { + let lower = line.to_ascii_lowercase(); + let line = strip_leading_markup(&lower); + let Some(after_label) = line.strip_prefix("gate") else { + return false; + }; + let after_label = after_label.trim_start_matches(['*', ' ', '`']); + if let Some(rest) = after_label.strip_prefix(':') { + let value_region = rest.trim_start_matches(['*', ' ', '\t', '`']); + let value_token: String = value_region + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '-') + .collect(); + if value_token == HUMAN_GATE_VALUE { + return true; } - false - }) + } + false +} + +/// What a piece of agent text last said about how the stage ended. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum StatedOutcome { + /// The latest statement is a blocking-human checkpoint: a marker that + /// declares it, or a `Gate:` declaration line with no marker after it. + Checkpoint, + /// The latest statement is a marker that declares no checkpoint. + Other, + /// Nothing in the text states an outcome. + Silent, +} + +/// The latest stated outcome in `text`, read line by line from the end +/// (operator decision 2026-10-10: the latest step wins). A `DEVFLOW_RESULT` +/// marker states an outcome, and so does a `Gate:` declaration line; the first +/// one found, counting back from the end, decides. Unlike +/// [`parse_marker_lines`] this reads the whole text: a long tail after a later +/// marker must not hide it and let an earlier gate win. +fn latest_stated_outcome(text: &str) -> StatedOutcome { + for line in text.lines().rev() { + if let Some(result) = marker_from_line(line) { + return if result.declares_blocking_human_checkpoint() { + StatedOutcome::Checkpoint + } else { + StatedOutcome::Other + }; + } + if line_reports_human_gate(line) { + return StatedOutcome::Checkpoint; + } + } + StatedOutcome::Silent } /// Strip the markdown a relayed declaration line can carry before its label: @@ -1596,11 +1644,10 @@ fn claude_stream_reports_human_gate(events: &[serde_json::Value]) -> bool { .filter(|event| is_top_level(event)) .filter_map(|event| event.get("result").and_then(serde_json::Value::as_str)) { - if text_reports_human_gate(text) { - return true; - } - if parse_marker_lines(text).is_some() { - return false; + match latest_stated_outcome(text) { + StatedOutcome::Checkpoint => return true, + StatedOutcome::Other => return false, + StatedOutcome::Silent => {} } } false @@ -2145,7 +2192,6 @@ fn normalise_stream_marker_provenance(mut result: AgentResult) -> AgentResult { /// accepted only ALL-upper or ALL-lower. fn parse_marker_lines(stdout: &str) -> Option { const TAIL_BUDGET_CHARS: usize = 4000; - const PREFIX: &str = "DEVFLOW_RESULT:"; let mut budget_used = 0usize; for line in stdout.lines().rev() { @@ -2157,30 +2203,36 @@ fn parse_marker_lines(stdout: &str) -> Option { } budget_used += line.chars().count() + 1; - let line = strip_corruption_padding(line); - let Some(head) = line.get(..PREFIX.len()) else { - continue; - }; - if !head.eq_ignore_ascii_case(PREFIX) { - continue; - } - - let json_str = line[PREFIX.len()..].trim(); - if let Ok(mut result) = serde_json::from_str::(json_str) { - // A declaration belongs to the failure the agent reported. Dropped - // here, at the one place agent JSON becomes an AgentResult, so no - // later status rewrite (the exit-code reconcile turns a reported - // success into a failure with `..result`) can carry it onto a - // failure the agent never declared. - if result.status != AgentStatus::Failed { - result.checkpoint = None; - } + if let Some(result) = marker_from_line(line) { return Some(result); } } None } +/// Parse one physical line as a `DEVFLOW_RESULT` marker: edge-stripped, +/// case-insensitive prefix, JSON body. The one place agent JSON becomes an +/// [`AgentResult`]. +fn marker_from_line(line: &str) -> Option { + const PREFIX: &str = "DEVFLOW_RESULT:"; + + let line = strip_corruption_padding(line); + let head = line.get(..PREFIX.len())?; + if !head.eq_ignore_ascii_case(PREFIX) { + return None; + } + let json_str = line[PREFIX.len()..].trim(); + let mut result = serde_json::from_str::(json_str).ok()?; + // A declaration belongs to the failure the agent reported. Dropped here so + // no later status rewrite (the exit-code reconcile turns a reported success + // into a failure with `..result`) can carry it onto a failure the agent + // never declared. + if result.status != AgentStatus::Failed { + result.checkpoint = None; + } + Some(result) +} + /// One commit the agent made before its stream went silent (D-07, 31-02). /// /// The subject is carried alongside the sha because a bare sha list is not @@ -4623,16 +4675,76 @@ mod tests { assert!(blocking_human_checkpoint_reported(&capture)); } - /// A gate declaration and a failure marker in the SAME final message are - /// one report: the agent stopped at the gate and said so. This is the - /// shape DevFlow's own checkpoint fixtures use. + /// Within one message the order of lines decides too. The protocol puts + /// the marker on the last line, so a `Gate:` line followed by a plain + /// failure marker is that failure; the gate counts only when nothing states + /// an outcome after it (operator decision 2026-10-10, second review round). #[test] - fn blocking_human_checkpoint_reported_true_when_gate_and_marker_share_the_latest_result() { + fn blocking_human_checkpoint_reported_false_when_a_plain_marker_follows_the_gate_in_one_message() + { let same_message = format!("{}{MARKER_FAILED}", gate_declaration_text()); - let capture = v3_stream_capture(MARKER_FAILED, &same_message, NO_MARKER); + let capture = v3_stream_capture(NO_MARKER, &same_message, NO_MARKER); + assert!(!blocking_human_checkpoint_reported(&capture)); + } + + /// A message that ends on the gate, after any marker, reports the gate. + #[test] + fn blocking_human_checkpoint_reported_true_when_the_gate_follows_the_marker_in_one_message() { + let same_message = format!("{MARKER_FAILED}\\n{}", gate_declaration_text()); + let capture = v3_stream_capture(NO_MARKER, &same_message, NO_MARKER); assert!(blocking_human_checkpoint_reported(&capture)); } + /// A declared checkpoint followed by silent wake-up turns is still the + /// latest stated outcome, the same protection T-30-27 gives a `Gate:` line. + #[test] + fn blocking_human_checkpoint_reported_true_for_a_declared_marker_before_silent_turns() { + let declared = r#"Stopped.\nDEVFLOW_RESULT: {\"status\":\"failed\",\"checkpoint\":\"blocking-human\"}"#; + let capture = v3_stream_capture(declared, NO_MARKER, NO_MARKER); + assert!(blocking_human_checkpoint_reported(&capture)); + } + + /// A later plain failure supersedes an earlier gate however much text + /// follows its marker. The stage-verdict parser reads only a 4000-character + /// tail; the outcome scan must not, or a long later turn reads as silent + /// and the earlier gate wins again. + #[test] + fn blocking_human_checkpoint_reported_false_when_a_later_failure_is_followed_by_long_text() { + let long_tail = "a".repeat(5000); + let later = format!("{MARKER_FAILED}\\n{long_tail}"); + let capture = v3_stream_capture(&gate_declaration_text(), NO_MARKER, &later); + assert!(!blocking_human_checkpoint_reported(&capture)); + } + + /// The same latest-outcome rule for a plain-text capture: a gate line + /// before a plain failure marker is that failure, and a marker that + /// declares the checkpoint is reported without any gate line. + #[test] + fn blocking_human_checkpoint_reported_reads_a_plain_capture_newest_first() { + let gate_then_failure = "**Gate:** `blocking-human`\nDEVFLOW_RESULT: {\"status\": \"failed\", \"reason\": \"tests failed\"}\n"; + assert!(!blocking_human_checkpoint_reported(gate_then_failure)); + let declared = "Stopped.\nDEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \"blocking-human\"}\n"; + assert!(blocking_human_checkpoint_reported(declared)); + let failure_then_gate = + "DEVFLOW_RESULT: {\"status\": \"failed\"}\n**Gate:** `blocking-human`\n"; + assert!(blocking_human_checkpoint_reported(failure_then_gate)); + } + + /// R5: an absent checkpoint is not written when a result is serialized, + /// and a present one is. + #[test] + fn checkpoint_is_serialized_only_when_present() { + let none = parse_devflow_result(r#"DEVFLOW_RESULT: {"status": "failed"}"#).unwrap(); + let json = serde_json::to_value(&none).unwrap(); + assert!(json.get("checkpoint").is_none(), "{json}"); + let some = parse_devflow_result( + r#"DEVFLOW_RESULT: {"status": "failed", "checkpoint": "blocking-human"}"#, + ) + .unwrap(); + let json = serde_json::to_value(&some).unwrap(); + assert_eq!(json["checkpoint"], "blocking-human"); + } + /// The overcorrection guard: an echo and a genuine declaration can coexist /// in one capture, and the scoping must resolve per event rather than /// suppressing any capture that contains an echo. diff --git a/crates/devflow-core/src/prompt.rs b/crates/devflow-core/src/prompt.rs index eba9539e..bb0279ac 100644 --- a/crates/devflow-core/src/prompt.rs +++ b/crates/devflow-core/src/prompt.rs @@ -647,9 +647,10 @@ fn stage_prompt_with_project(stage: Stage, phase: PhaseId, project_root: Option< /// than copy the checkpoint's declaration line. A failed resume capture is /// re-read by `agent_result::text_reports_human_gate`, so a copied declaration /// line could read as a new checkpoint. A checkpoint is reported only by a -/// `**Gate:**`-labeled line or by a final `DEVFLOW_RESULT` that declares -/// `"checkpoint": "blocking-human"` ([`COMPLETION_PROTOCOL`] says to use that -/// field only for a gate being reported rather than resolved). The wording of a +/// `**Gate:**`-labeled line with no marker after it, or by a final +/// `DEVFLOW_RESULT` that declares `"checkpoint": "blocking-human"` +/// ([`CODE_STAGE_POLICY`] offers that field only for a gate being reported +/// rather than resolved, and this prompt does not offer it). The wording of a /// failure reason is never read (49-REVIEW CR-01). The /// `resume_prompt_does_not_read_as_a_blocking_human_checkpoint` test pins that /// boundary and rejects prose such as `Resolved gate: blocking-human`. @@ -1351,7 +1352,10 @@ mod tests { assert!(prompt.contains(CHECKPOINT_DECLARATION)); let markers: Vec<&str> = prompt .lines() - .filter(|line| line.starts_with("DEVFLOW_RESULT:")) + .filter(|line| { + line.get(.."DEVFLOW_RESULT:".len()) + .is_some_and(|head| head.eq_ignore_ascii_case("DEVFLOW_RESULT:")) + }) .collect(); assert!(!markers.is_empty(), "the protocol examples are present"); for marker in markers { From 4ca00289e1504f8224831342ece1a7be65c7d1ab Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Sat, 10 Oct 2026 08:43:33 -0400 Subject: [PATCH 5/6] fix(agent_result): apply latest-outcome rule to JSON capture lines A JSON line in a capture not classified as a Claude stream matched only Gate lines inside its strings, so a later ordinary-failure marker was skipped and an earlier gate won. Each string is now read with the latest stated outcome, and an ordinary outcome wins within a document. Adds a Code pipeline test where only the capture scan confirms. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0182ocSM2E4fb2zu52jxGLzi --- crates/devflow-cli/src/pipeline_launch.rs | 69 ++++++++++++ crates/devflow-core/src/agent_result.rs | 124 ++++++++++++++++------ crates/devflow-core/src/prompt.rs | 2 +- 3 files changed, 162 insertions(+), 33 deletions(-) diff --git a/crates/devflow-cli/src/pipeline_launch.rs b/crates/devflow-cli/src/pipeline_launch.rs index 55a47d28..0dcc3ef9 100644 --- a/crates/devflow-cli/src/pipeline_launch.rs +++ b/crates/devflow-cli/src/pipeline_launch.rs @@ -5380,6 +5380,75 @@ mod tests { ); } + /// The capture route alone confirms a Code checkpoint: a plain failure + /// marker, then a `Gate:` line as the latest stated outcome. The evaluated + /// result declares nothing, so only the capture scan can relaunch here + /// (round-3 external review: every positive test also declared the field, + /// so dropping the capture route left the suite green). + #[test] + fn advance_with_declared_checkpoint_and_gate_line_only_relaunches_and_records() { + const NAME: &str = "pipeline_launch::tests::advance_with_declared_checkpoint_and_gate_line_only_relaunches_and_records"; + const ROOT_ENV: &str = "DEVFLOW_PIPELINE_LAUNCH_TEST_ROOT"; + + if !devflow_core::test_support::in_child_test(NAME) { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + init_repo(root); + + let phase = PhaseId::new(191); + write_declared_checkpoint_plan(root, phase); + std::fs::create_dir_all(root.join(".devflow")).unwrap(); + std::fs::write( + agent_result::stdout_path(root, phase), + format!( + "DEVFLOW_RESULT: {{\"status\": \"failed\", \"reason\": \"checkpoint pending\"}}\n## CHECKPOINT REACHED\n\n**Gate:** {HUMAN_GATE_VALUE_FOR_TEST}\n" + ), + ) + .unwrap(); + + let mut state = State::new(phase, AgentKind::Claude, Mode::Auto, root.to_path_buf()); + state.stage = Stage::Code; + state.session_id = Some("sess-gate-line-only".to_string()); + state.checkpoint_approval = recorded_approval_for(root, phase); + workflow::save_state(&state).unwrap(); + + let path_dir = agent_free_dir_with_agent_stub("claude"); + let output = devflow_core::test_support::run_test_in_child( + NAME, + path_dir.path(), + &[(ROOT_ENV, root.as_os_str())], + ); + devflow_core::test_support::assert_child_ran_exactly_one_passing_test(&output, NAME); + return; + } + + let root = PathBuf::from( + std::env::var_os(ROOT_ENV) + .expect("child test must receive its parent-built fixture root"), + ); + let root = root.as_path(); + let phase = PhaseId::new(191); + // Unused on the relaunch route; if detection regresses, the + // fall-through gate reads it and returns instead of blocking forever. + write_abort_gate_response(root, phase, Stage::Code); + + let result = advance_for_stage(root, Some(phase), Some(Stage::Code)); + let reloaded_for_reap = workflow::load_state(root, phase).ok(); + let _reap_guard = reloaded_for_reap + .as_ref() + .map(ReapMonitorOnDrop::after_launch); + + result.unwrap(); + + let auto_decided = events_of_kind(root, "checkpoint_auto_decided"); + assert_eq!( + auto_decided.len(), + 1, + "a gate line as the latest stated outcome must auto-decide exactly once: {auto_decided:?}" + ); + assert_eq!(auto_decided[0]["session_id"], "sess-gate-line-only"); + } + /// 49-REVIEW CR-01 at the pipeline: a failure whose reason only describes /// a blocking-human halt takes the ordinary gate, even in a phase that /// declares such a checkpoint. diff --git a/crates/devflow-core/src/agent_result.rs b/crates/devflow-core/src/agent_result.rs index e21f5da2..57891c41 100644 --- a/crates/devflow-core/src/agent_result.rs +++ b/crates/devflow-core/src/agent_result.rs @@ -646,14 +646,14 @@ fn detect_claude_envelope_failure(stdout: &str) -> Option { /// /// The VALUE is what this constant holds. The surrounding markdown — bold /// label, and a **code span around the value** — is handled by -/// [`text_reports_human_gate`]'s trim set, not by this constant. +/// [`line_reports_human_gate`]'s trim set, not by this constant. /// /// The code span is the part RESEARCH.md did not predict. Its § "Architecture /// Patterns / Pattern 2" derived the literal by reading the *emitting* source /// (`gsd-executor.md:356`, `execute-phase.md:1053`) and predicted a bare /// `**Gate:** blocking-human`. The real relay renders the value as a code /// span, which defeated the original matcher entirely — see -/// [`text_reports_human_gate`] for that failure and its fix. Lesson worth +/// [`line_reports_human_gate`] for that failure and its fix. Lesson worth /// keeping: the emitting source told us the value, not the rendering. const HUMAN_GATE_VALUE: &str = "blocking-human"; @@ -740,11 +740,7 @@ fn capture_reports_human_gate(stdout: &str) -> bool { for line in stdout.lines().rev() { let outcome = match serde_json::from_str::(line.trim()) { Ok(value) if value.is_object() || value.is_array() => { - if json_strings_report_human_gate(&value) { - StatedOutcome::Checkpoint - } else { - StatedOutcome::Silent - } + json_strings_stated_outcome(&value) } _ => latest_stated_outcome(line), }; @@ -757,21 +753,43 @@ fn capture_reports_human_gate(stdout: &str) -> bool { false } -/// Match every string value inside a decoded JSON document. -fn json_strings_report_human_gate(value: &serde_json::Value) -> bool { +/// The stated outcome of a decoded JSON document: each string value is read +/// with [`latest_stated_outcome`]. A document's values have no order a reader +/// can rely on, so when its strings disagree the safe outcome wins: any +/// ordinary outcome makes the document [`StatedOutcome::Other`], and only +/// otherwise does a checkpoint make it [`StatedOutcome::Checkpoint`]. +/// +/// This replaces matching only `Gate:` lines inside the strings, which skipped +/// a later ordinary-failure marker and let an earlier gate line win (round-3 +/// external review of the CR-01 fix, 2026-10-10). +fn json_strings_stated_outcome(value: &serde_json::Value) -> StatedOutcome { + let combine = |left: StatedOutcome, right: StatedOutcome| match (left, right) { + (StatedOutcome::Other, _) | (_, StatedOutcome::Other) => StatedOutcome::Other, + (StatedOutcome::Checkpoint, _) | (_, StatedOutcome::Checkpoint) => { + StatedOutcome::Checkpoint + } + _ => StatedOutcome::Silent, + }; match value { - serde_json::Value::String(text) => text_reports_human_gate(text), - serde_json::Value::Array(items) => items.iter().any(json_strings_report_human_gate), - serde_json::Value::Object(fields) => fields.values().any(json_strings_report_human_gate), - _ => false, + serde_json::Value::String(text) => latest_stated_outcome(text), + serde_json::Value::Array(items) => items + .iter() + .map(json_strings_stated_outcome) + .fold(StatedOutcome::Silent, combine), + serde_json::Value::Object(fields) => fields + .values() + .map(json_strings_stated_outcome) + .fold(StatedOutcome::Silent, combine), + _ => StatedOutcome::Silent, } } -/// Core matcher over text already in its final form: a physical line of a -/// non-JSON capture, a decoded JSON string ([`capture_reports_human_gate`]), or -/// an envelope's unescaped `result` ([`blocking_human_checkpoint_reported`]). -/// It never decodes escapes itself. Scans only -/// for a line whose first word, after any list, ordinal or blockquote markup +/// Core matcher over one line of text already in its final form: a physical +/// line of a non-JSON capture, or a line of a decoded JSON string +/// ([`capture_reports_human_gate`]) or of an envelope's unescaped `result` +/// ([`blocking_human_checkpoint_reported`]), each reached through +/// [`latest_stated_outcome`]. It never decodes escapes itself. Matches only +/// a line whose first word, after any list, ordinal or blockquote markup /// ([`strip_leading_markup`]), is a case-insensitive `gate` label, tolerating /// surrounding markdown emphasis (`*`), code-span backticks (`` ` ``), and /// whitespace up to the following `:`, then compares the VALUE token @@ -789,11 +807,6 @@ fn json_strings_report_human_gate(value: &serde_json::Value) -> bool { /// /// Note the closing backtick needs no handling: `take_while` already stops /// at it, since a backtick is neither alphanumeric nor `-`. -fn text_reports_human_gate(text: &str) -> bool { - text.lines().any(line_reports_human_gate) -} - -/// One physical line of [`text_reports_human_gate`]. fn line_reports_human_gate(line: &str) -> bool { let lower = line.to_ascii_lowercase(); let line = strip_leading_markup(&lower); @@ -1571,8 +1584,8 @@ pub(crate) fn event_is_top_level_result_marker(event: &serde_json::Value) -> boo /// Whether any AGENT-AUTHORED text in a Claude stream capture declares a /// human-blocking gate. The stream-capture half of -/// [`blocking_human_checkpoint_reported`]; the pure matcher it delegates to, -/// [`text_reports_human_gate`], is unchanged. +/// [`blocking_human_checkpoint_reported`]; each eligible text is read with +/// [`latest_stated_outcome`]. /// /// **Why this exists (review constraint 3).** Scanning raw stdout is safe under /// the single-document envelope, because the only place gate text can appear @@ -1615,9 +1628,10 @@ pub(crate) fn event_is_top_level_result_marker(event: &serde_json::Value) -> boo /// outcome wins** (operator decision 2026-10-10). The first event, counting /// back from the end, that either declares the gate or carries a /// `DEVFLOW_RESULT` marker decides: a declaration means a checkpoint, a marker -/// without one means the stage ended on that outcome instead. A declaration -/// and a marker in the same message are one report, so the declaration wins -/// within a message. Events with neither, such as task-notification wake-up +/// without one means the stage ended on that outcome instead. Within a +/// message the same rule applies line by line: a marker after a declaration +/// outranks it, and a declaration after a marker outranks the marker. Events +/// with neither, such as task-notification wake-up /// turns, are skipped, so a gate declared in turn N followed by silent turns /// N+1/N+2 (the turn shape the v3 capture archives) is still reported, which /// is what the earlier scan-everything rule existed to protect. @@ -3942,19 +3956,19 @@ mod tests { "## CHECKPOINT REACHED\n\n**Type:** decision\n**Gate:** `{HUMAN_GATE_VALUE}`\n**Plan:** 30-05\n" ); let labelled = format!("{resume}\n{declaration}"); - assert!(text_reports_human_gate(&labelled)); + assert!(labelled.lines().any(line_reports_human_gate)); assert!(blocking_human_checkpoint_reported(&envelope(&labelled))); assert!( resume.contains("gate-declaration line"), "the resume prompt must tell the agent not to copy the gate-declaration line" ); - assert!(!text_reports_human_gate(&resume)); + assert!(!resume.lines().any(line_reports_human_gate)); assert!(!blocking_human_checkpoint_reported(&envelope(&resume))); let prose = "Resolved gate: blocking-human using the recorded evidence."; assert!( - !text_reports_human_gate(prose), + !prose.lines().any(line_reports_human_gate), "only a Gate-labeled line may report a checkpoint" ); assert!( @@ -4257,7 +4271,7 @@ mod tests { /// through DevFlow's own monitor). The value arrives as a markdown CODE /// SPAN, not the bare token RESEARCH.md predicted. /// - /// Before the backtick was added to `text_reports_human_gate`'s trim set + /// Before the backtick was added to the gate matcher's trim set /// this returned `false`: the leading backtick survived the trim, so the /// value `take_while` terminated at once and yielded an empty token. A /// real checkpoint was therefore never recognized, and the run fell @@ -4730,6 +4744,52 @@ mod tests { assert!(blocking_human_checkpoint_reported(failure_then_gate)); } + /// Round-3 review: a JSON line in a capture that is not classified as a + /// Claude stream (two noise lines demote it to plain text) must follow + /// the same rule. Only `Gate:` lines were matched inside its strings, so + /// an earlier gate outranked a later ordinary failure. + #[test] + fn blocking_human_checkpoint_reported_reads_json_lines_by_the_same_rule() { + let result_line = |text: &str| { + serde_json::json!({"type": "result", "result": text, "session_id": "current"}) + .to_string() + }; + let init = r#"{"type":"system","subtype":"init","session_id":"current"}"#; + let noisy = |text: &str| format!("{init}\nprogress-1\nprogress-2\n{}\n", result_line(text)); + let gate = "**Gate:** `blocking-human`"; + let plain_failure = + "DEVFLOW_RESULT: {\"status\": \"failed\", \"reason\": \"tests failed\"}"; + let declared = + "DEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \"blocking-human\"}"; + + assert!(!blocking_human_checkpoint_reported(&noisy(&format!( + "{gate}\n{plain_failure}" + )))); + assert!( + blocking_human_checkpoint_reported(&noisy(gate)), + "control: a gate alone still counts" + ); + assert!(blocking_human_checkpoint_reported(&noisy(declared))); + + let plain_gate_then_json_failure = format!( + "[info] claude starting\n{gate}\n{}\n", + result_line(plain_failure) + ); + assert!(!blocking_human_checkpoint_reported( + &plain_gate_then_json_failure + )); + let plain_gate_then_silent_json = format!("[info] claude starting\n{gate}\n{init}\n"); + assert!(blocking_human_checkpoint_reported( + &plain_gate_then_silent_json + )); + + let disagreeing_fields = serde_json::json!({"a": gate, "b": plain_failure}).to_string(); + assert!( + !blocking_human_checkpoint_reported(&format!("noise\n{disagreeing_fields}\n")), + "fields of one document have no order, so the ordinary outcome wins" + ); + } + /// R5: an absent checkpoint is not written when a result is serialized, /// and a present one is. #[test] diff --git a/crates/devflow-core/src/prompt.rs b/crates/devflow-core/src/prompt.rs index bb0279ac..de4fec6c 100644 --- a/crates/devflow-core/src/prompt.rs +++ b/crates/devflow-core/src/prompt.rs @@ -645,7 +645,7 @@ fn stage_prompt_with_project(stage: Stage, phase: PhaseId, project_root: Option< /// /// The resume wording tells the agent to name the resolved gate in prose rather /// than copy the checkpoint's declaration line. A failed resume capture is -/// re-read by `agent_result::text_reports_human_gate`, so a copied declaration +/// re-read by `agent_result::latest_stated_outcome`, so a copied declaration /// line could read as a new checkpoint. A checkpoint is reported only by a /// `**Gate:**`-labeled line with no marker after it, or by a final /// `DEVFLOW_RESULT` that declares `"checkpoint": "blocking-human"` From e98020114ed3d9195691d9988cd39ebd7a0439a2 Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Sat, 10 Oct 2026 09:02:19 -0400 Subject: [PATCH 6/6] fix(agent_result): read JSON arrays newest first in capture scan An array is ordered, so its elements now follow the latest stated outcome like lines of text; only an object's unordered fields keep the ordinary-outcome-wins rule. Tests now assert the noisy fixture takes the raw-capture path, cover both field orders and both block orders, and check the gate-line relaunch spawns a monitor. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0182ocSM2E4fb2zu52jxGLzi --- crates/devflow-cli/src/pipeline_launch.rs | 11 ++++++ crates/devflow-core/src/agent_result.rs | 47 +++++++++++++++++++---- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/crates/devflow-cli/src/pipeline_launch.rs b/crates/devflow-cli/src/pipeline_launch.rs index 0dcc3ef9..ec7e147f 100644 --- a/crates/devflow-cli/src/pipeline_launch.rs +++ b/crates/devflow-cli/src/pipeline_launch.rs @@ -5447,6 +5447,17 @@ mod tests { "a gate line as the latest stated outcome must auto-decide exactly once: {auto_decided:?}" ); assert_eq!(auto_decided[0]["session_id"], "sess-gate-line-only"); + assert!( + reloaded_for_reap + .as_ref() + .is_some_and(|reloaded| reloaded.monitor_pid.is_some()), + "the checkpoint must relaunch the session, not only record the event" + ); + let gate_fired = events_of_kind(root, "gate_fired"); + assert!( + gate_fired.iter().all(|e| e["stage"] != "code"), + "an auto-resolved checkpoint must not also fire the generic gate: {gate_fired:?}" + ); } /// 49-REVIEW CR-01 at the pipeline: a failure whose reason only describes diff --git a/crates/devflow-core/src/agent_result.rs b/crates/devflow-core/src/agent_result.rs index 57891c41..64e2e9df 100644 --- a/crates/devflow-core/src/agent_result.rs +++ b/crates/devflow-core/src/agent_result.rs @@ -754,10 +754,12 @@ fn capture_reports_human_gate(stdout: &str) -> bool { } /// The stated outcome of a decoded JSON document: each string value is read -/// with [`latest_stated_outcome`]. A document's values have no order a reader -/// can rely on, so when its strings disagree the safe outcome wins: any -/// ordinary outcome makes the document [`StatedOutcome::Other`], and only -/// otherwise does a checkpoint make it [`StatedOutcome::Checkpoint`]. +/// with [`latest_stated_outcome`]. An array is ordered, so its elements are +/// read newest first and the first that states an outcome decides, like the +/// lines of a text. An object's fields have no order a reader can rely on, so +/// when they disagree the safe outcome wins: any ordinary outcome makes the +/// object [`StatedOutcome::Other`], and only otherwise does a checkpoint make +/// it [`StatedOutcome::Checkpoint`]. /// /// This replaces matching only `Gate:` lines inside the strings, which skipped /// a later ordinary-failure marker and let an earlier gate line win (round-3 @@ -774,8 +776,10 @@ fn json_strings_stated_outcome(value: &serde_json::Value) -> StatedOutcome { serde_json::Value::String(text) => latest_stated_outcome(text), serde_json::Value::Array(items) => items .iter() + .rev() .map(json_strings_stated_outcome) - .fold(StatedOutcome::Silent, combine), + .find(|outcome| *outcome != StatedOutcome::Silent) + .unwrap_or(StatedOutcome::Silent), serde_json::Value::Object(fields) => fields .values() .map(json_strings_stated_outcome) @@ -4762,6 +4766,12 @@ mod tests { let declared = "DEVFLOW_RESULT: {\"status\": \"failed\", \"checkpoint\": \"blocking-human\"}"; + // Round-4 review: the fixture must really take the raw-capture path, + // or the stream reader answers these and the JSON path goes untested. + assert!( + classify(&ParsedCapture::parse(&noisy(gate))) != CaptureKind::ClaudeStream, + "the noisy fixture must not be read as a Claude stream" + ); assert!(!blocking_human_checkpoint_reported(&noisy(&format!( "{gate}\n{plain_failure}" )))); @@ -4783,10 +4793,31 @@ mod tests { &plain_gate_then_silent_json )); - let disagreeing_fields = serde_json::json!({"a": gate, "b": plain_failure}).to_string(); + for fields in [ + serde_json::json!({"a": gate, "b": plain_failure}), + serde_json::json!({"a": plain_failure, "b": gate}), + ] { + assert!( + !blocking_human_checkpoint_reported(&format!("noise\n{fields}\n")), + "an object's fields have no order, so the ordinary outcome wins: {fields}" + ); + } + + let blocks = |first: &str, second: &str| { + let content = serde_json::json!({"content": [{"text": first}, {"text": second}]}); + format!("noise\n{content}\n") + }; + assert!( + blocking_human_checkpoint_reported(&blocks(plain_failure, gate)), + "an array is ordered, so a later gate block wins" + ); + assert!( + !blocking_human_checkpoint_reported(&blocks(gate, plain_failure)), + "an array is ordered, so a later failure block wins" + ); assert!( - !blocking_human_checkpoint_reported(&format!("noise\n{disagreeing_fields}\n")), - "fields of one document have no order, so the ordinary outcome wins" + blocking_human_checkpoint_reported(&blocks(gate, "Silent tail.")), + "a silent later block is skipped" ); }