diff --git a/crates/cm-api/src/routes/missions.rs b/crates/cm-api/src/routes/missions.rs index f41d8a5..9b38254 100644 --- a/crates/cm-api/src/routes/missions.rs +++ b/crates/cm-api/src/routes/missions.rs @@ -206,7 +206,21 @@ fn phases_for_create( }) .map(|rp| rp.config.clone()) .unwrap_or(Value::Null); + // Decided from the caller's config BEFORE the merge: afterwards a + // recipe task and a caller task are indistinguishable. + let caller_task = p + .config + .get("task") + .and_then(|t| t.as_str()) + .is_some_and(|s| !s.trim().is_empty()); + let caller_condition = + p.config.get("done_when").is_some() || p.config.get("done_when_check").is_some(); let config = merge_config(base, p.config); + let config = if caller_task && !caller_condition { + drop_orphaned_condition(config, &p.kind, p.order_idx) + } else { + config + }; // Say what this phase asked for that will not happen. A config key // nothing reads is silent by construction — `task` sat unread // through every mission until two phases with different tasks @@ -221,6 +235,36 @@ fn phases_for_create( .collect() } +/// A recipe's completion condition is a condition on the recipe's own task. +/// When the caller supplied a different `task` and no condition of its own, +/// keeping the recipe's `done_when` judges the phase against work it was +/// never asked to do — `research_and_code`'s coding phase inherits "an +/// implementation for each INT-XX item in IMPLEMENTATION_BRIEF", and a +/// phase asked to write CHAIN.md fails on it, honestly, every time (missions +/// 01a0c20d, 01a0c493). The condition and the check travel with the task +/// they were written for; a phase with its own task gets its own, or none. +/// +/// Called only when the caller supplied a task and no condition; the +/// decision is made before the merge, where the two are still telling apart. +fn drop_orphaned_condition(config: Value, kind: &str, order_idx: i32) -> Value { + let Value::Object(mut c) = config else { return config }; + let dropped: Vec<&str> = ["done_when", "done_when_check"] + .into_iter() + .filter(|k| c.contains_key(*k)) + .collect(); + if !dropped.is_empty() { + eprintln!( + "phase {kind}[{order_idx}]: caller supplied its own task and no completion \ + condition — the recipe's {} is NOT inherited (it describes the recipe's task)", + dropped.join("/") + ); + for k in dropped { + c.remove(k); + } + } + Value::Object(c) +} + /// Shallow-merge `over` onto `base`, key by key. /// /// Shallow is deliberate: phase config is a flat settings bag, and a caller @@ -1755,6 +1799,44 @@ mod tests { ); } + /// A caller's own task does not inherit the recipe's condition; a + /// caller's own condition is kept; a phase with neither keeps the + /// recipe's pair as before. + #[test] + fn a_custom_task_does_not_inherit_the_recipes_done_when() { + let recipe = test_recipe(); + let coding_has_condition = recipe + .phases + .iter() + .any(|p| p.kind == "coding" && p.config.get("done_when").is_some()); + assert!(coding_has_condition, "the fixture must carry a recipe condition"); + let phases = phases_for_create( + Some(&recipe), + vec![ + PhaseSpec { + kind: "coding".into(), + order_idx: 1, + config: serde_json::json!({"task": "write CHAIN.md"}), + }, + PhaseSpec { + kind: "coding".into(), + order_idx: 2, + config: serde_json::json!({"task": "write X", "done_when": "X exists"}), + }, + PhaseSpec { + kind: "coding".into(), + order_idx: 3, + config: Value::Null, + }, + ], + ); + assert!(phases[0].config.get("done_when").is_none(), "{:?}", phases[0].config); + assert_eq!(phases[1].config["done_when"], "X exists"); + assert!(phases[2].config.get("done_when").is_some(), "{:?}", phases[2].config); + // The rest of the recipe's config still backfills the custom-task phase. + assert!(phases[0].config.get("loop").is_some()); + } + /// Omitting phases entirely takes the recipe's list wholesale. #[test] fn phases_default_to_the_recipe() { @@ -1853,7 +1935,10 @@ mod tests { order_idx: 1, config: serde_json::json!({ "loop": "until_no_more_int_items", - "commit_policy": "on_green_tests" + "commit_policy": "on_green_tests", + // The recipe's own task and the condition written for it. + "task": "implement every INT-XX item", + "done_when": "an implementation exists for each INT-XX item" }), }, ], diff --git a/scripts/verify-mission-delivery.sh b/scripts/verify-mission-delivery.sh index 26ca3bd..ae3bac5 100755 --- a/scripts/verify-mission-delivery.sh +++ b/scripts/verify-mission-delivery.sh @@ -1246,12 +1246,21 @@ assert_skill_triage() { #