diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs index 8c4273faf..c7b8b460d 100644 --- a/lib/crates/fabro-cli/src/commands/create.rs +++ b/lib/crates/fabro-cli/src/commands/create.rs @@ -6,8 +6,7 @@ use fabro_workflows::manifest::Manifest; use fabro_workflows::run_spec::RunSpec; use super::run::{ - cached_graph_path, default_run_dir, prepare_workflow, workflow_slug_from_path, - write_run_config_snapshot, RunArgs, + cached_graph_path, default_run_dir, prepare_workflow, write_run_config_snapshot, RunArgs, }; use fabro_util::terminal::Styles; @@ -102,7 +101,7 @@ pub async fn create_run( base_sha: None, labels, base_branch, - workflow_slug: workflow_slug_from_path(workflow_path), + workflow_slug: prep.workflow_slug.clone(), host_repo_path: Some(working_directory.to_string_lossy().to_string()), }; manifest.save(&run_dir.join("manifest.json"))?; diff --git a/lib/crates/fabro-cli/src/commands/resume.rs b/lib/crates/fabro-cli/src/commands/resume.rs index 75390de61..57c7a48db 100644 --- a/lib/crates/fabro-cli/src/commands/resume.rs +++ b/lib/crates/fabro-cli/src/commands/resume.rs @@ -206,6 +206,7 @@ async fn prepare_from_checkpoint( let graph = prepared.graph; let run_cfg = prepared.run_cfg; let sandbox_provider = prepared.sandbox_provider; + let workflow_slug = prepared.workflow_slug; eprintln!( "{} {} from checkpoint {}", @@ -437,7 +438,7 @@ async fn prepare_from_checkpoint( .or(run_defaults.assets.as_ref()) .map(|a| a.include.clone()) .unwrap_or_default(), - workflow_slug: None, + workflow_slug, }; let devcontainer_env = devcontainer_config @@ -521,7 +522,7 @@ async fn prepare_from_branch( .or_else(|| repo_info.as_ref().and_then(|(_, branch)| branch.clone())); let base_sha = manifest.as_ref().and_then(|m| m.base_sha.clone()); - let (graph, graph_source, run_cfg, mut sandbox_provider) = + let (graph, graph_source, run_cfg, mut sandbox_provider, workflow_slug) = if let Some(ref workflow_path) = args.workflow { let prepared = prepare_workflow_with_project_config( &resume_as_run_args(args, workflow_path.clone()), @@ -535,6 +536,7 @@ async fn prepare_from_branch( prepared.source, prepared.run_cfg, prepared.sandbox_provider, + prepared.workflow_slug, ) } else { let (graph, diagnostics) = @@ -549,7 +551,13 @@ async fn prepare_from_branch( } else { resolve_sandbox_provider(args.sandbox.map(Into::into), None, run_defaults)? }; - (graph, source.clone(), None, sandbox_provider) + ( + graph, + source.clone(), + None, + sandbox_provider, + manifest.as_ref().and_then(|m| m.workflow_slug.clone()), + ) }; eprintln!( @@ -826,7 +834,7 @@ async fn prepare_from_branch( .or(run_defaults.assets.as_ref()) .map(|a| a.include.clone()) .unwrap_or_default(), - workflow_slug: None, + workflow_slug, }; let devcontainer_env = devcontainer_config diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index 8fc7f8a5c..b5c171355 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -193,14 +193,21 @@ pub(crate) fn default_run_dir(run_id: &str, dry_run: bool) -> PathBuf { } pub(crate) fn workflow_slug_from_path(workflow_path: &Path) -> Option { + let file_name = workflow_path.file_name()?.to_string_lossy(); if workflow_path.extension().is_none() { - Some(workflow_path.to_string_lossy().into_owned()) - } else { - workflow_path + return Some(file_name.into_owned()); + } + + let file_stem = workflow_path.file_stem()?.to_string_lossy(); + if file_stem == "workflow" { + return workflow_path .parent() .and_then(|p| p.file_name()) .map(|n| n.to_string_lossy().into_owned()) + .or_else(|| Some(file_stem.into_owned())); } + + Some(file_stem.into_owned()) } fn is_cached_run_restart(workflow_path: &Path, run_dir: &Path) -> bool { @@ -504,13 +511,13 @@ pub(crate) async fn write_run_config_snapshot( pub(crate) fn resolve_workflow_source( workflow_path: &Path, -) -> anyhow::Result<(PathBuf, Option)> { +) -> anyhow::Result<(PathBuf, PathBuf, Option)> { let path = project_config::resolve_workflow_arg(workflow_path)?; if path.extension().is_some_and(|ext| ext == "toml") { match run_config::load_run_config(&path) { Ok(cfg) => { let dot = run_config::resolve_graph_path(&path, &cfg.graph); - Ok((dot, Some(cfg))) + Ok((path, dot, Some(cfg))) } // Backward compatibility for detached runs created before run.toml existed. // Use path.exists() to distinguish a genuinely missing run.toml from one @@ -519,12 +526,12 @@ pub(crate) fn resolve_workflow_source( if !path.exists() && path.starts_with(fabro_workflows::run_lookup::default_runs_base()) => { - Ok((path.with_file_name(RUN_GRAPH_FILE), None)) + Ok((path.clone(), path.with_file_name(RUN_GRAPH_FILE), None)) } Err(err) => Err(err), } } else { - Ok((path, None)) + Ok((path.clone(), path, None)) } } @@ -536,6 +543,7 @@ pub(crate) struct PreparedWorkflow { pub sandbox_provider: SandboxProvider, pub model: String, pub provider: Option, + pub workflow_slug: Option, pub run_defaults: RunDefaults, } @@ -575,16 +583,17 @@ pub(crate) fn prepare_workflow_with_project_config( } // Resolve workflow arg, load run config if TOML, apply defaults - let (dot_path, run_cfg) = { - let (dot, cfg) = resolve_workflow_source(workflow_path)?; + let (resolved_workflow_path, dot_path, run_cfg) = { + let (resolved, dot, cfg) = resolve_workflow_source(workflow_path)?; match cfg { Some(mut cfg) => { cfg.apply_defaults(&run_defaults); - (dot, Some(cfg)) + (resolved, dot, Some(cfg)) } - None => (dot, None), + None => (resolved, dot, None), } }; + let workflow_slug = workflow_slug_from_path(&resolved_workflow_path); let directory = run_cfg .as_ref() @@ -682,6 +691,7 @@ pub(crate) fn prepare_workflow_with_project_config( sandbox_provider, model, provider, + workflow_slug, run_defaults, }) } @@ -705,6 +715,7 @@ pub async fn run_command( sandbox_provider, model, provider, + workflow_slug: prepared_workflow_slug, run_defaults, } = prepare_workflow(&args, run_defaults, styles, false)?; @@ -756,10 +767,13 @@ pub async fn run_command( } else { None }; - let workflow_slug = existing_manifest - .as_ref() - .and_then(|manifest| manifest.workflow_slug.clone()) - .or_else(|| workflow_slug_from_path(workflow_path)); + let workflow_slug = if cached_run_restart { + existing_manifest + .as_ref() + .and_then(|manifest| manifest.workflow_slug.clone()) + } else { + prepared_workflow_slug + }; fabro_util::run_log::activate(&run_dir.join("cli.log")) .context("Failed to activate per-run log")?; tokio::fs::write(cached_graph_path(&run_dir), &source).await?; @@ -2730,7 +2744,7 @@ mod tests { let dir = tempfile::tempdir_in(&runs_base).unwrap(); std::fs::write(dir.path().join(RUN_GRAPH_FILE), "digraph test {}").unwrap(); - let (dot_path, run_cfg) = + let (_resolved_path, dot_path, run_cfg) = resolve_workflow_source(&dir.path().join(RUN_CONFIG_FILE)).unwrap(); assert_eq!(dot_path, dir.path().join(RUN_GRAPH_FILE)); @@ -2746,6 +2760,42 @@ mod tests { assert!(result.is_err()); } + #[test] + fn workflow_slug_from_path_uses_file_stem_for_standalone_files() { + assert_eq!( + workflow_slug_from_path(Path::new("/tmp/alpha.fabro")).as_deref(), + Some("alpha") + ); + assert_eq!( + workflow_slug_from_path(Path::new("/tmp/beta.toml")).as_deref(), + Some("beta") + ); + } + + #[test] + fn workflow_slug_from_path_uses_parent_for_workflow_files() { + assert_eq!( + workflow_slug_from_path(Path::new("/tmp/sluggy/workflow.fabro")).as_deref(), + Some("sluggy") + ); + assert_eq!( + workflow_slug_from_path(Path::new("/tmp/sluggy/workflow.toml")).as_deref(), + Some("sluggy") + ); + } + + #[test] + fn workflow_slug_from_path_uses_final_component_for_extensionless_inputs() { + assert_eq!( + workflow_slug_from_path(Path::new("implement-issue")).as_deref(), + Some("implement-issue") + ); + assert_eq!( + workflow_slug_from_path(Path::new("nested/repl")).as_deref(), + Some("repl") + ); + } + #[test] fn prepare_workflow_with_project_config_resolves_workflow_toml_settings() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-cli/tests/cli.rs b/lib/crates/fabro-cli/tests/cli.rs index fda7a2f64..c29328f5a 100644 --- a/lib/crates/fabro-cli/tests/cli.rs +++ b/lib/crates/fabro-cli/tests/cli.rs @@ -687,6 +687,136 @@ digraph BarBaz { assert_eq!(manifest["workflow_slug"].as_str(), Some("sluggy")); } +#[test] +fn standalone_file_run_uses_file_stem_slug_for_lookup() { + let home = tempfile::tempdir().unwrap(); + let workflow_dir = tempfile::tempdir().unwrap(); + let workflow_path = workflow_dir.path().join("alpha.fabro"); + std::fs::write( + &workflow_path, + "\ +digraph FooWorkflow { + start [shape=Mdiamond, label=\"Start\"] + exit [shape=Msquare, label=\"Exit\"] + start -> exit +} +", + ) + .unwrap(); + + arc() + .env("HOME", home.path()) + .args([ + "create", + "--dry-run", + "--auto-approve", + "--run-id", + "opaque-run-alpha", + workflow_path.to_str().unwrap(), + ]) + .assert() + .success(); + + arc() + .env("HOME", home.path()) + .args(["start", "alpha"]) + .assert() + .success(); + + arc() + .env("HOME", home.path()) + .args(["attach", "alpha"]) + .timeout(std::time::Duration::from_secs(10)) + .assert() + .success(); + + let run_dir = find_run_dir(home.path(), "opaque-run-alpha"); + let manifest: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(run_dir.join("manifest.json")).unwrap()) + .unwrap(); + assert_eq!(manifest["workflow_name"].as_str(), Some("FooWorkflow")); + assert_eq!(manifest["workflow_slug"].as_str(), Some("alpha")); +} + +#[test] +fn resumed_run_preserves_workflow_slug_for_lookup() { + let home = tempfile::tempdir().unwrap(); + let project = tempfile::tempdir().unwrap(); + let workflow_dir = project.path().join("workflows").join("sluggy"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + let workflow_path = workflow_dir.join("workflow.fabro"); + std::fs::write( + &workflow_path, + "\ +digraph BarBaz { + start [shape=Mdiamond, label=\"Start\"] + exit [shape=Msquare, label=\"Exit\"] + start -> exit +} +", + ) + .unwrap(); + let original_run_dir = project.path().join("original-run"); + + arc() + .env("HOME", home.path()) + .current_dir(project.path()) + .args([ + "run", + "--dry-run", + "--auto-approve", + "--no-retro", + "--run-dir", + original_run_dir.to_str().unwrap(), + workflow_path.to_str().unwrap(), + ]) + .assert() + .success(); + + arc() + .env("HOME", home.path()) + .current_dir(project.path()) + .args([ + "resume", + "--checkpoint", + original_run_dir.join("checkpoint.json").to_str().unwrap(), + "--workflow", + workflow_path.to_str().unwrap(), + "--dry-run", + "--auto-approve", + "--no-retro", + ]) + .assert() + .success(); + + arc() + .env("HOME", home.path()) + .current_dir(project.path()) + .args(["attach", "sluggy"]) + .timeout(std::time::Duration::from_secs(10)) + .assert() + .success(); + + let resumed_runs_dir = home.path().join(".fabro").join("runs"); + let resumed_run_dir = std::fs::read_dir(&resumed_runs_dir) + .unwrap() + .flatten() + .map(|entry| entry.path()) + .find(|path| path.is_dir()) + .unwrap_or_else(|| { + panic!( + "expected a resumed run under {}", + resumed_runs_dir.display() + ) + }); + let manifest: serde_json::Value = serde_json::from_str( + &std::fs::read_to_string(resumed_run_dir.join("manifest.json")).unwrap(), + ) + .unwrap(); + assert_eq!(manifest["workflow_name"].as_str(), Some("BarBaz")); + assert_eq!(manifest["workflow_slug"].as_str(), Some("sluggy")); +} + #[test] fn dry_run_create_start_attach_works_with_default_run_lookup() { let home = tempfile::tempdir().unwrap();