diff --git a/crates/meshloop-adapters/src/harness.rs b/crates/meshloop-adapters/src/harness.rs index 35cb041..713ccf0 100644 --- a/crates/meshloop-adapters/src/harness.rs +++ b/crates/meshloop-adapters/src/harness.rs @@ -187,6 +187,10 @@ impl HarnessCapabilities for CliHarness { } } + fn process_image(&self) -> Option { + crate::process::process_image_name(&self.config.executable) + } + fn cancel(&self, handle: &HarnessHandle) -> Result<(), HarnessError> { let mut registry = self .running diff --git a/crates/meshloop-adapters/src/process.rs b/crates/meshloop-adapters/src/process.rs index 3d1c5c2..79db741 100644 --- a/crates/meshloop-adapters/src/process.rs +++ b/crates/meshloop-adapters/src/process.rs @@ -117,12 +117,26 @@ pub fn kill_orphaned_tree(pid: u32, executable: &std::path::Path) -> bool { true } +/// The OS image name a process spawned from `executable` reports: `.exe` on Windows +/// (what `tasklist` shows, whether or not the configured path has the extension), the file +/// name elsewhere. This is what attempts persist so liveness checks can reject reused pids. +pub fn process_image_name(executable: &std::path::Path) -> Option { + if cfg!(windows) { + executable + .file_stem() + .map(|stem| format!("{}.exe", stem.to_string_lossy())) + } else { + executable + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + } +} + #[cfg(windows)] fn orphan_identity_matches(pid: u32, executable: &std::path::Path) -> bool { - let Some(stem) = executable.file_stem() else { + let Some(image) = process_image_name(executable) else { return false; }; - let image = format!("{}.exe", stem.to_string_lossy()); WindowsProcessView.is_live(&ProcessHint { pid, image_name: Some(image), @@ -218,6 +232,57 @@ mod tests { ); } + #[test] + fn process_image_name_matches_what_the_os_reports() { + #[cfg(windows)] + { + assert_eq!( + process_image_name(std::path::Path::new("pwsh")).as_deref(), + Some("pwsh.exe") + ); + assert_eq!( + process_image_name(std::path::Path::new(r"C:\tools\node.exe")).as_deref(), + Some("node.exe") + ); + } + #[cfg(not(windows))] + assert_eq!( + process_image_name(std::path::Path::new("/usr/bin/node")).as_deref(), + Some("node") + ); + } + + /// The persisted image lets a process that does not own the child see it as live (#47). + #[test] + fn persisted_image_name_reports_a_running_child_live() { + #[cfg(windows)] + let (exe, args) = ( + "powershell", + ["-NoProfile", "-Command", "Start-Sleep -Seconds 30"], + ); + #[cfg(not(windows))] + let (exe, args) = ("sleep", ["30", "", ""]); + let mut cmd = Command::new(exe); + cmd.args(args.iter().filter(|a| !a.is_empty())); + let mut child = cmd.spawn().expect("spawn sleeper"); + let hint = ProcessHint { + pid: child.id(), + image_name: process_image_name(std::path::Path::new(exe)), + }; + assert_eq!(HostProcessView {}.is_live(&hint), LiveCheck::Live); + #[cfg(windows)] + assert_eq!( + HostProcessView {}.is_live(&ProcessHint { + pid: child.id(), + image_name: Some("claude".into()), + }), + LiveCheck::Ambiguous, + "a harness name is not an image name; it must not read as Live" + ); + let _ = child.kill(); + let _ = child.wait(); + } + #[test] fn kills_live_child_process_tree() { #[cfg(windows)] diff --git a/crates/meshloop-cli/src/args.rs b/crates/meshloop-cli/src/args.rs index d98da59..4eb21f9 100644 --- a/crates/meshloop-cli/src/args.rs +++ b/crates/meshloop-cli/src/args.rs @@ -25,6 +25,8 @@ pub enum Command { }, Status { graph: Option, + config: Option, + db: Option, }, Plan { objective: String, @@ -163,7 +165,11 @@ pub fn parse(args: &[String]) -> Result { fn parse_command(args: &[String]) -> Result { match args.first().map(|s| verb(s)) { - None => Ok(Command::Status { graph: None }), + None => Ok(Command::Status { + graph: None, + config: None, + db: None, + }), Some(v) if v == "--help" || v == "-h" || v == "help" => Ok(Command::Help), Some(v) if v == "--version" || v == "-V" || v == "version" => Ok(Command::Version), Some(v) if v == "mcp" => Ok(Command::Mcp), @@ -254,6 +260,8 @@ fn parse_command(args: &[String]) -> Result { } Some(v) if v == "status" => Ok(Command::Status { graph: flag_value(&args[1..], "--graph"), + config: flag_value(&args[1..], "--config").map(PathBuf::from), + db: flag_value(&args[1..], "--db").map(PathBuf::from), }), Some(v) if v == "resume" => { let rest = &args[1..]; @@ -413,7 +421,7 @@ pub fn help_text() -> &'static str { \x20 meshloop review-plan --plan --accept|--decline|--adjust [--reason] [--as] [--json]\n\ \x20 meshloop run --plan [--accept-plan] [--reset] [--fixture-only] [--json]\n\ \x20 meshloop resume [--graph ] [--retry|--restart] [--json]\n\ - \x20 meshloop status [--graph ] [--json]\n\ + \x20 meshloop status [--graph ] [--config ] [--db ] [--json]\n\ \x20 meshloop roles [--json]\n\ \x20 meshloop doctor [--config ] [--db ] [--json]\n\ \x20 meshloop orchestrate --task --model-a --model-b [--json]\n\ @@ -424,3 +432,25 @@ pub fn help_text() -> &'static str { \x20 or MESHLOOP_ORIGIN_HARNESS / MESHLOOP_ORIGIN_SESSION.\n\ Live workers are the default (Herdr). --fixture-only forces the CI subprocess double." } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn status_keeps_config_db_and_graph() { + let args: Vec = ["status", "--config", "x", "--db", "y", "--graph", "g"] + .iter() + .map(|s| s.to_string()) + .collect(); + let inv = parse(&args).expect("parse"); + match inv.command { + Command::Status { graph, config, db } => { + assert_eq!(graph.as_deref(), Some("g")); + assert_eq!(config, Some(PathBuf::from("x"))); + assert_eq!(db, Some(PathBuf::from("y"))); + } + _ => panic!("expected status"), + } + } +} diff --git a/crates/meshloop-cli/src/compose.rs b/crates/meshloop-cli/src/compose.rs index 116f99c..a9c16f3 100644 --- a/crates/meshloop-cli/src/compose.rs +++ b/crates/meshloop-cli/src/compose.rs @@ -61,6 +61,12 @@ impl HarnessCapabilities for DispatchHarness { Self::Fixture(h) | Self::Direct(h) => h.pane_for_worktree(worktree), } } + + fn process_image(&self) -> Option { + match self { + Self::Fixture(h) | Self::Direct(h) => h.process_image(), + } + } } pub struct Composed { diff --git a/crates/meshloop-cli/src/main.rs b/crates/meshloop-cli/src/main.rs index 09db308..3f66225 100644 --- a/crates/meshloop-cli/src/main.rs +++ b/crates/meshloop-cli/src/main.rs @@ -83,7 +83,9 @@ fn dispatch(inv: Invocation) -> ExitCode { Command::Mcp => ExitCode::from(mcp::run_stdio() as u8), Command::Roles => cmd_roles(inv.json, inv.origin), Command::Doctor { config, db } => cmd_doctor(config, db, inv.json, inv.origin), - Command::Status { graph } => cmd_status(graph, inv.json, inv.origin), + Command::Status { graph, config, db } => { + cmd_status(graph, config, db, inv.json, inv.origin) + } Command::Plan { objective, config, @@ -629,6 +631,8 @@ fn cmd_review_plan( ExitCode::SUCCESS } +const EMPTY_VERIFY_WARNING: &str = "verify_command is empty: nodes are not checked beyond git diff"; + #[allow(clippy::too_many_arguments)] fn cmd_run( plan: PathBuf, @@ -685,6 +689,9 @@ fn cmd_run( return ExitCode::from(2); } }; + if composed.verify_command.is_empty() { + eprintln!("warning: {EMPTY_VERIFY_WARNING}"); + } let harness_refs: HashMap = composed .harnesses .iter() @@ -935,29 +942,33 @@ fn with_saga( fn cmd_status( graph: Option, + config: Option, + db: Option, json: bool, origin: meshloop_engine::origin::Origin, ) -> ExitCode { - if crate::config::discover(None).is_err() { + if crate::config::discover(config.as_deref()).is_err() { + let msg = "no config found; pass --config or add meshloop.toml"; if json { - println!( - "{}", - json_out::ok( - "meshloop:status", - origin, - serde_json::json!({ "banner": report::banner(), "runs": [] }), - ) - ); + println!("{}", json_out::err("meshloop:status", origin, msg)); } else { - print!("{}", report::banner()); + eprintln!("{msg}"); } - return ExitCode::SUCCESS; + return ExitCode::from(2); } - let db = None; - with_saga(None, db, None, false, origin.clone(), |saga| { + with_saga(config, db, None, false, origin.clone(), |saga| { let id = match saga.resolve_graph_id(graph.as_deref()) { Ok(id) => id, - Err(_) => { + Err(e) => { + if let Some(g) = graph.as_deref() { + let msg = format!("graph '{g}' not found: {e:?}"); + if json { + println!("{}", json_out::err("meshloop:status", origin.clone(), msg)); + } else { + eprintln!("{msg}"); + } + return ExitCode::from(2); + } if json { println!( "{}", @@ -1002,8 +1013,15 @@ fn cmd_status( ExitCode::SUCCESS } Err(e) => { - eprintln!("{e:?}"); - ExitCode::from(1) + if json { + println!( + "{}", + json_out::err("meshloop:status", origin.clone(), format!("{e:?}")) + ); + } else { + eprintln!("{e:?}"); + } + ExitCode::from(2) } } }) @@ -1478,6 +1496,7 @@ fn cmd_doctor( let origin_session = origin.session.clone(); let origin_harness = origin.harness.clone(); + let mut warnings: Vec<&str> = Vec::new(); let (config_path, harnesses, error) = match crate::config::discover(config.as_deref()) { Err(_) => ( None, @@ -1491,6 +1510,9 @@ fn cmd_doctor( Some(format!("failed to load config: {e:?}")), ), Ok(cfg) => { + if cfg.verify.verify_command.is_empty() { + warnings.push(EMPTY_VERIFY_WARNING); + } let rows = doctor_probe(&cfg); let error = (!rows.iter().any(|r| r["dispatchable"] == true)).then(|| { "no selected harness is dispatchable; see data.harnesses[].reason".to_string() @@ -1518,6 +1540,7 @@ fn cmd_doctor( "harnesses_ready": harnesses_ready, "harnesses": harnesses, "store_ignored": store_ignored, + "warnings": warnings, "note": "Doctor loads the config and runs each selected harness's bounded version probe. Live workers run as direct CLI subprocesses in Git worktrees.", }); if json { @@ -1545,6 +1568,9 @@ fn cmd_doctor( println!(" harness {}: {status}", h["name"].as_str().unwrap_or("?")); } println!(" store_ignored: {store_ignored}"); + for w in &warnings { + println!(" warning: {w}"); + } if !store_ignored { println!( " fix: add `{}` to .gitignore, or run `meshloop bundle --dest . --gitignore`", diff --git a/crates/meshloop-cli/tests/plan_and_run.rs b/crates/meshloop-cli/tests/plan_and_run.rs index 8536ccd..1849346 100644 --- a/crates/meshloop-cli/tests/plan_and_run.rs +++ b/crates/meshloop-cli/tests/plan_and_run.rs @@ -1309,6 +1309,107 @@ fn mutate_plan_with_require_review_gates_resume() { fs::remove_dir_all(&dir).ok(); } +fn empty_temp_dir(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("meshloop-cli-empty-{tag}-{}", std::process::id())); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).unwrap(); + dir +} + +#[test] +fn status_honours_config_and_db_outside_cwd() { + let dir = disposable_repo("status-cfg"); + let config_path = write_config(&dir, r#"["--prompt-file", "{prompt_file}"]"#); + let plan_path = dir.join("plan.json"); + fs::write( + &plan_path, + r#"{"graph_id":"g_status_cfg","nodes":[{"id":1,"description":"first task","depends_on":[],"tier":null}]}"#, + ) + .unwrap(); + let db_path = dir.join("state.sqlite"); + let run = meshloop() + .current_dir(&dir) + .args(["run", "--plan"]) + .arg(&plan_path) + .args(["--accept-plan", "--fixture-only", "--json", "--config"]) + .arg(&config_path) + .arg("--db") + .arg(&db_path) + .arg("--worktree-base") + .arg(dir.join("worktrees")) + .output() + .expect("run CLI"); + assert!(run.status.success(), "run failed: {:?}", run); + + let elsewhere = empty_temp_dir("status-cfg-cwd"); + let out = meshloop() + .current_dir(&elsewhere) + .args(["status", "--graph", "g_status_cfg", "--config"]) + .arg(&config_path) + .arg("--db") + .arg(&db_path) + .arg("--json") + .output() + .expect("status CLI"); + assert!(out.status.success(), "status failed: {:?}", out); + let parsed: serde_json::Value = + serde_json::from_slice(&out.stdout).expect("status stdout is JSON"); + assert_eq!(parsed["ok"], true); + assert_eq!(parsed["data"]["graph_id"], "g_status_cfg"); + let ids: Vec = parsed["data"]["nodes"] + .as_array() + .expect("nodes array") + .iter() + .filter_map(|n| n["id"].as_u64()) + .collect(); + assert_eq!(ids, vec![1]); + + fs::remove_dir_all(&dir).ok(); + fs::remove_dir_all(&elsewhere).ok(); +} + +#[test] +fn status_without_any_config_is_an_error() { + let elsewhere = empty_temp_dir("status-noconfig"); + let out = meshloop() + .current_dir(&elsewhere) + .args(["status", "--json"]) + .output() + .expect("status CLI"); + assert!(!out.status.success()); + let parsed: serde_json::Value = + serde_json::from_slice(&out.stdout).expect("status stdout is JSON"); + assert_eq!(parsed["ok"], false); + assert!( + parsed.to_string().contains("config"), + "error should mention the config: {parsed}" + ); + fs::remove_dir_all(&elsewhere).ok(); +} + +#[test] +fn status_with_unknown_graph_is_an_error_naming_it() { + let dir = disposable_repo("status-unknown"); + let config_path = write_config(&dir, r#"["--prompt-file", "{prompt_file}"]"#); + let out = meshloop() + .current_dir(&dir) + .args(["status", "--graph", "nope", "--json", "--config"]) + .arg(&config_path) + .arg("--db") + .arg(dir.join("state.sqlite")) + .output() + .expect("status CLI"); + assert!(!out.status.success()); + let parsed: serde_json::Value = + serde_json::from_slice(&out.stdout).expect("status stdout is JSON"); + assert_eq!(parsed["ok"], false); + assert!( + parsed.to_string().contains("nope"), + "error should name the graph: {parsed}" + ); + fs::remove_dir_all(&dir).ok(); +} + #[test] fn run_with_json_outputs_clean_rfc8259_and_authentic_sha256() { let dir = disposable_repo("json-clean"); @@ -1515,3 +1616,43 @@ fn detached_run_and_session_inspect_and_cancel_lifecycle() { fs::remove_dir_all(&dir).ok(); } + +fn doctor_warnings(tag: &str, verify_command: &str) -> Vec { + let dir = disposable_repo(tag); + let config_path = write_config(&dir, r#"["--emit-graph"]"#); + let text = fs::read_to_string(&config_path) + .unwrap() + .replace("verify_command = []", verify_command); + fs::write(&config_path, text).unwrap(); + let output = meshloop() + .current_dir(&dir) + .args(["doctor", "--json", "--config"]) + .arg(&config_path) + .output() + .expect("doctor"); + let stdout = String::from_utf8_lossy(&output.stdout); + let parsed: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid json"); + let warnings = parsed["data"]["warnings"] + .as_array() + .expect("warnings array") + .iter() + .map(|w| w.as_str().expect("warning string").to_string()) + .collect(); + let _ = fs::remove_dir_all(&dir); + warnings +} + +#[test] +fn doctor_warns_when_verify_command_is_empty() { + let warnings = doctor_warnings("doctor-verify-empty", "verify_command = []"); + assert!( + warnings.iter().any(|w| w.contains("verify_command")), + "{warnings:?}" + ); +} + +#[test] +fn doctor_has_no_warnings_when_verify_command_is_set() { + let warnings = doctor_warnings("doctor-verify-set", r#"verify_command = ["true"]"#); + assert!(warnings.is_empty(), "{warnings:?}"); +} diff --git a/crates/meshloop-engine/src/ports.rs b/crates/meshloop-engine/src/ports.rs index 1c3ca34..a5b8b39 100644 --- a/crates/meshloop-engine/src/ports.rs +++ b/crates/meshloop-engine/src/ports.rs @@ -62,6 +62,13 @@ pub trait HarnessCapabilities: Sync { let _ = worktree; None } + + /// OS image name of the process `invoke` spawns (e.g. `pwsh.exe`), persisted with the + /// attempt pid so another process can tell a live worker from a reused pid. `None` when + /// the harness has no process of its own (fixtures, panes). + fn process_image(&self) -> Option { + None + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/meshloop-engine/src/run_loop.rs b/crates/meshloop-engine/src/run_loop.rs index 80b240b..a05cf8f 100644 --- a/crates/meshloop-engine/src/run_loop.rs +++ b/crates/meshloop-engine/src/run_loop.rs @@ -160,6 +160,19 @@ struct RepairTick { revision: String, } +/// Liveness for display (`status`/`inspect`): widens the owned view with an OS-level probe +/// for detached workers another process owns. Recovery must keep using the owned view, where +/// `Dead` means "this process cannot collect it". +fn observed_live(owned: LiveCheck, attempt: &AttemptRow, processes: &dyn ProcessView) -> LiveCheck { + match (owned, &attempt.pane_id, attempt.pid, &attempt.ended_at) { + (LiveCheck::Dead, None, Some(pid), None) => processes.is_live(&ProcessHint { + pid, + image_name: attempt.image_name.clone(), + }), + (owned, ..) => owned, + } +} + fn terminal(state: TaskState) -> bool { matches!( state, @@ -1204,16 +1217,9 @@ impl<'a> RunLoop<'a> { self.maybe_fallback(graph_id, task_id, fail, &selected) } Ok(handle) => { - let image = Path::new( - &self - .candidates - .iter() - .find(|c| c.harness == candidate.harness) - .map(|c| c.harness.clone()) - .unwrap_or_default(), - ) - .file_name() - .map(|s| s.to_string_lossy().into_owned()); + // The spawned process's image, not the harness name, so liveness checks + // from another process can match the persisted pid (#47). + let image = harness.process_image(); self.store .update_attempt_pid(attempt_id, handle.pid, image.as_deref())?; self.store @@ -1322,6 +1328,8 @@ impl<'a> RunLoop<'a> { })); } } + } else { + rows.push(skipped_verify_evidence(&candidate_ref)); } Ok((rows, lattice_out, diag_out)) } @@ -1926,9 +1934,12 @@ impl<'a> RunLoop<'a> { for n in &graph.nodes { let state = tasks.get(&n.id).copied().unwrap_or(TaskState::Pending); let attempt = self.store.latest_attempt_for_task(graph_id, n.id)?; - let live = attempt - .as_ref() - .map(|a| format!("{:?}", self.attempt_live(a))); + let live = attempt.as_ref().map(|a| { + format!( + "{:?}", + observed_live(self.attempt_live(a), a, self.processes) + ) + }); let pane_id = attempt.as_ref().and_then(|a| a.pane_id.clone()); let note = if state == TaskState::Ready { Some("Ready (may be waiting on QACR or cancel to unblock dependents)".into()) @@ -2109,3 +2120,133 @@ pub fn idle_exit_code(reason: IdleReason, status: &RunStatus) -> i32 { IdleReason::NoCapableCandidate | IdleReason::FailedTerminal => 1, } } + +/// Evidence recorded when no `verify_command` is configured, so the skipped check is visible. +fn skipped_verify_evidence(candidate: &CandidateRef) -> Evidence { + Evidence::Deterministic(DeterministicEvidence { + tool: "verify".into(), + tool_version: "n/a".into(), + exit_code: 0, + output_redacted: "skipped: verify_command is empty".into(), + candidate: candidate.clone(), + }) +} + +#[cfg(test)] +mod tests { + use std::cell::Cell; + + use super::*; + + #[test] + fn skipped_verify_evidence_is_a_passing_deterministic_row() { + let candidate = CandidateRef { + task_id: TaskId(7), + attempt_id: AttemptId(2), + revision: "abc123".into(), + }; + match skipped_verify_evidence(&candidate) { + Evidence::Deterministic(ev) => { + assert_eq!(ev.tool, "verify"); + assert_eq!(ev.exit_code, 0); + assert!(ev.output_redacted.starts_with("skipped:")); + assert_eq!(ev.candidate, candidate); + } + other => panic!("expected deterministic evidence, got {other:?}"), + } + } + + struct FakeView { + answer: LiveCheck, + calls: Cell, + } + + impl FakeView { + fn new(answer: LiveCheck) -> Self { + Self { + answer, + calls: Cell::new(0), + } + } + } + + impl ProcessView for FakeView { + fn is_live(&self, _hint: &ProcessHint) -> LiveCheck { + self.calls.set(self.calls.get() + 1); + self.answer + } + } + + fn attempt(pid: Option, ended_at: Option<&str>) -> AttemptRow { + AttemptRow { + attempt_id: meshloop_domain::evidence::AttemptId(1), + graph_id: "g".into(), + task_id: TaskId(1), + harness: None, + model_ref: None, + worktree_path: None, + pid, + image_name: Some("worker.exe".into()), + pane_id: None, + started_at: None, + ended_at: ended_at.map(String::from), + outcome: None, + } + } + + #[test] + fn dead_owned_with_live_process_is_live() { + let view = FakeView::new(LiveCheck::Live); + let got = observed_live(LiveCheck::Dead, &attempt(Some(7), None), &view); + assert_eq!(got, LiveCheck::Live); + assert_eq!(view.calls.get(), 1); + } + + #[test] + fn dead_owned_with_exited_process_is_dead() { + let view = FakeView::new(LiveCheck::Dead); + let got = observed_live(LiveCheck::Dead, &attempt(Some(7), None), &view); + assert_eq!(got, LiveCheck::Dead); + } + + #[test] + fn dead_owned_with_pid_reuse_is_ambiguous() { + let view = FakeView::new(LiveCheck::Ambiguous); + let got = observed_live(LiveCheck::Dead, &attempt(Some(7), None), &view); + assert_eq!(got, LiveCheck::Ambiguous); + assert_ne!(got, LiveCheck::Live); + } + + #[test] + fn ended_attempt_is_dead_without_consulting_view() { + let view = FakeView::new(LiveCheck::Live); + let got = observed_live(LiveCheck::Dead, &attempt(Some(7), Some("t")), &view); + assert_eq!(got, LiveCheck::Dead); + assert_eq!(view.calls.get(), 0); + } + + #[test] + fn missing_pid_is_dead_without_consulting_view() { + let view = FakeView::new(LiveCheck::Live); + let got = observed_live(LiveCheck::Dead, &attempt(None, None), &view); + assert_eq!(got, LiveCheck::Dead); + assert_eq!(view.calls.get(), 0); + } + + #[test] + fn pane_attempt_is_dead_without_consulting_view() { + let view = FakeView::new(LiveCheck::Live); + let mut a = attempt(Some(7), None); + a.pane_id = Some("p".into()); + assert_eq!(observed_live(LiveCheck::Dead, &a, &view), LiveCheck::Dead); + assert_eq!(view.calls.get(), 0); + } + + #[test] + fn owned_live_stays_live_without_consulting_view() { + let view = FakeView::new(LiveCheck::Dead); + let got = observed_live(LiveCheck::Live, &attempt(Some(7), None), &view); + assert_eq!(got, LiveCheck::Live); + assert_eq!(view.calls.get(), 0); + } +}