diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs index dac0d613c..8c4273faf 100644 --- a/lib/crates/fabro-cli/src/commands/create.rs +++ b/lib/crates/fabro-cli/src/commands/create.rs @@ -1,10 +1,13 @@ use std::path::PathBuf; +use chrono::Utc; use fabro_config::run::RunDefaults; +use fabro_workflows::manifest::Manifest; use fabro_workflows::run_spec::RunSpec; use super::run::{ - cached_graph_path, default_run_dir, prepare_workflow, write_run_config_snapshot, RunArgs, + cached_graph_path, default_run_dir, prepare_workflow, workflow_slug_from_path, + write_run_config_snapshot, RunArgs, }; use fabro_util::terminal::Styles; @@ -52,11 +55,17 @@ pub async fn create_run( // Build and save RunSpec let working_directory = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let labels: std::collections::HashMap = args + .label + .iter() + .filter_map(|s| s.split_once('=')) + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); let spec = RunSpec { run_id: run_id.clone(), workflow_path: std::fs::canonicalize(workflow_path).unwrap_or(workflow_path.clone()), dot_source: prep.source, - working_directory, + working_directory: working_directory.clone(), goal: if goal.is_empty() { None } else { @@ -65,12 +74,7 @@ pub async fn create_run( model: prep.model, provider: prep.provider, sandbox_provider: prep.sandbox_provider.to_string(), - labels: args - .label - .iter() - .filter_map(|s| s.split_once('=')) - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect(), + labels: labels.clone(), verbose: args.verbose, no_retro: args.no_retro, preserve_sandbox: args.preserve_sandbox, @@ -79,5 +83,29 @@ pub async fn create_run( }; spec.save(&run_dir)?; + let workflow_name = if prep.graph.name.is_empty() { + "unnamed".to_string() + } else { + prep.graph.name.clone() + }; + let base_branch = fabro_sandbox::daytona::detect_repo_info(&working_directory) + .ok() + .and_then(|(_, branch)| branch); + let manifest = Manifest { + run_id: run_id.clone(), + workflow_name, + goal: goal.to_string(), + start_time: Utc::now(), + node_count: prep.graph.nodes.len(), + edge_count: prep.graph.edges.len(), + run_branch: None, + base_sha: None, + labels, + base_branch, + workflow_slug: workflow_slug_from_path(workflow_path), + host_repo_path: Some(working_directory.to_string_lossy().to_string()), + }; + manifest.save(&run_dir.join("manifest.json"))?; + Ok((run_id, run_dir)) } diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index af1a79262..abd57be46 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -179,17 +179,29 @@ pub(crate) fn apply_goal_override( /// Compute the default run directory when `--run-dir` is not provided. pub(crate) fn default_run_dir(run_id: &str, dry_run: bool) -> PathBuf { + let base = fabro_workflows::run_lookup::default_runs_base(); if dry_run { - std::env::temp_dir().join("fabro-dry-run").join(run_id) + base.join(format!( + "{}-dry-run-{}", + Local::now().format("%Y%m%d"), + run_id + )) } else { - let base = dirs::home_dir() - .expect("could not determine home directory") - .join(".fabro") - .join("runs"); base.join(format!("{}-{}", Local::now().format("%Y%m%d"), run_id)) } } +pub(crate) fn workflow_slug_from_path(workflow_path: &Path) -> Option { + if workflow_path.extension().is_none() { + Some(workflow_path.to_string_lossy().into_owned()) + } else { + workflow_path + .parent() + .and_then(|p| p.file_name()) + .map(|n| n.to_string_lossy().into_owned()) + } +} + /// Resolve model and provider through the full precedence chain: /// CLI flag > TOML config > run defaults > DOT graph attrs > provider-specific defaults. /// Then resolve through the catalog for alias expansion. @@ -692,14 +704,7 @@ pub async fn run_command( // If bare name (no extension, e.g. "smoke"), use it directly. // Otherwise derive from the parent directory of the resolved .toml path. let workflow_path = args.workflow.as_ref().unwrap(); // safe: prepare_workflow validated - let workflow_slug: Option = if workflow_path.extension().is_none() { - Some(workflow_path.to_string_lossy().into_owned()) - } else { - workflow_path - .parent() - .and_then(|p| p.file_name()) - .map(|n| n.to_string_lossy().into_owned()) - }; + let workflow_slug = workflow_slug_from_path(workflow_path); // Collect setup commands — they'll be run inside the sandbox let setup_commands: Vec = run_cfg diff --git a/lib/crates/fabro-cli/tests/cli.rs b/lib/crates/fabro-cli/tests/cli.rs index 6c333f6d9..bb73bea2b 100644 --- a/lib/crates/fabro-cli/tests/cli.rs +++ b/lib/crates/fabro-cli/tests/cli.rs @@ -603,6 +603,156 @@ fn setup_run_dir( run_dir } +fn find_run_dir(home: &std::path::Path, run_id: &str) -> std::path::PathBuf { + let runs_dir = home.join(".fabro").join("runs"); + std::fs::read_dir(&runs_dir) + .unwrap() + .flatten() + .map(|entry| entry.path()) + .find(|path| { + path.is_dir() + && path + .file_name() + .is_some_and(|name| name.to_string_lossy().ends_with(run_id)) + }) + .unwrap_or_else(|| { + panic!( + "expected run directory for {run_id} under {}", + runs_dir.display() + ) + }) +} + +#[test] +fn dry_run_create_start_attach_works_with_default_run_lookup() { + let home = tempfile::tempdir().unwrap(); + let run_id = "drysplit-test-123"; + + arc() + .env("HOME", home.path()) + .args([ + "create", + "--dry-run", + "--auto-approve", + "--run-id", + run_id, + "../../../test/simple.fabro", + ]) + .assert() + .success() + .stdout(predicate::str::contains(run_id)); + + let run_dir = find_run_dir(home.path(), run_id); + assert!( + run_dir.join("manifest.json").exists(), + "create should persist manifest.json so the run is discoverable" + ); + + arc() + .env("HOME", home.path()) + .args(["start", run_id]) + .assert() + .success(); + + arc() + .env("HOME", home.path()) + .args(["attach", run_id]) + .timeout(std::time::Duration::from_secs(10)) + .assert() + .success(); + + assert!(run_dir.join("conclusion.json").exists()); +} + +#[test] +fn dry_run_detach_attach_works_with_default_run_lookup() { + let home = tempfile::tempdir().unwrap(); + let run_id = "drydetach-test-123"; + + arc() + .env("HOME", home.path()) + .args([ + "run", + "--detach", + "--dry-run", + "--auto-approve", + "--run-id", + run_id, + "../../../test/simple.fabro", + ]) + .assert() + .success() + .stdout(predicate::str::contains(run_id)); + + arc() + .env("HOME", home.path()) + .args(["attach", run_id]) + .timeout(std::time::Duration::from_secs(10)) + .assert() + .success(); +} + +#[test] +fn start_by_workflow_name_prefers_newly_created_submitted_run() { + let home = tempfile::tempdir().unwrap(); + let old_run_dir = home.path().join(".fabro").join("runs").join("old-smoke"); + std::fs::create_dir_all(&old_run_dir).unwrap(); + std::fs::write( + old_run_dir.join("manifest.json"), + serde_json::json!({ + "run_id": "old-smoke", + "workflow_name": "Smoke", + "goal": "", + "start_time": "2026-01-01T00:00:00Z", + "node_count": 1, + "edge_count": 0 + }) + .to_string(), + ) + .unwrap(); + std::fs::write( + old_run_dir.join("status.json"), + serde_json::json!({"status": "succeeded", "updated_at": "2026-01-01T00:00:00Z"}) + .to_string(), + ) + .unwrap(); + + let run_id = "new-smoke-run-123"; + arc() + .env("HOME", home.path()) + .args([ + "create", + "--dry-run", + "--auto-approve", + "--run-id", + run_id, + "smoke", + ]) + .assert() + .success() + .stdout(predicate::str::contains(run_id)); + + arc() + .env("HOME", home.path()) + .args(["start", "smoke"]) + .assert() + .success(); + + arc() + .env("HOME", home.path()) + .args(["attach", run_id]) + .timeout(std::time::Duration::from_secs(10)) + .assert() + .success(); + + let new_run_dir = find_run_dir(home.path(), run_id); + let status = std::fs::read_to_string(new_run_dir.join("status.json")).unwrap(); + assert!( + status.contains("\"status\": \"succeeded\""), + "expected the newly created Smoke run to be started and completed" + ); +} + // Bug 2: _run_engine should use cached graph.fabro, not spec.workflow_path. // When the original workflow file is deleted between create and start, // the engine should read the snapshot saved at create time.