diff --git a/lib/crates/fabro-cli/src/manifest_builder.rs b/lib/crates/fabro-cli/src/manifest_builder.rs index 716e77023..d88eb2599 100644 --- a/lib/crates/fabro-cli/src/manifest_builder.rs +++ b/lib/crates/fabro-cli/src/manifest_builder.rs @@ -660,4 +660,132 @@ mod tests { .contains_key("fabro/workflows/child/workflow.fabro") ); } + + /// A relative `[run.goal] file = "..."` declared in `fabro.toml` must + /// resolve against the directory of `fabro.toml`, not against the + /// invocation cwd. We exercise this by invoking from a subdirectory + /// below the project root. + #[test] + #[allow(unsafe_code, clippy::allow_attributes)] + fn build_manifest_resolves_relative_goal_file_in_project_config() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join("fabro/workflows/demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + std::fs::create_dir_all(project.join("prompts")).unwrap(); + + std::fs::write( + project.join("fabro.toml"), + r#"_version = 1 + +[run.goal] +file = "prompts/goal.md" +"#, + ) + .unwrap(); + std::fs::write(project.join("prompts/goal.md"), "ship from project root").unwrap(); + + std::fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let sandboxed_settings = temp.path().join("empty-settings.toml"); + std::fs::write(&sandboxed_settings, "_version = 1\n").unwrap(); + // SAFETY: single-threaded unit test body. + unsafe { + std::env::set_var("FABRO_CONFIG", &sandboxed_settings); + } + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from("fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + args_layer: ConfigLayer::default(), + args: None, + run_id: None, + }) + .unwrap(); + + // SAFETY: single-threaded unit test body. + unsafe { + std::env::remove_var("FABRO_CONFIG"); + } + + let goal = built.manifest.goal.expect("manifest goal should be set"); + assert_eq!(goal.text, "ship from project root"); + assert_eq!(goal.type_, types::ManifestGoalType::File); + let resolved = goal.path.expect("file goal must carry a path"); + let expected = project.join("prompts").join("goal.md"); + assert_eq!(PathBuf::from(resolved), expected); + } + + /// A relative `[run.goal] file = "..."` declared in `workflow.toml` + /// must resolve against the directory of `workflow.toml`, not against + /// the invocation cwd or project root. + #[test] + #[allow(unsafe_code, clippy::allow_attributes)] + fn build_manifest_resolves_relative_goal_file_in_workflow_config() { + let temp = tempfile::tempdir().unwrap(); + let project = temp.path(); + let workflow_dir = project.join("fabro/workflows/demo"); + std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); + + std::fs::write(project.join("fabro.toml"), "_version = 1\n").unwrap(); + std::fs::write( + workflow_dir.join("workflow.toml"), + r#"_version = 1 + +[workflow] +graph = "workflow.fabro" + +[run.goal] +file = "prompts/goal.md" +"#, + ) + .unwrap(); + std::fs::write( + workflow_dir.join("prompts/goal.md"), + "ship from workflow dir", + ) + .unwrap(); + std::fs::write( + workflow_dir.join("workflow.fabro"), + r"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + + let sandboxed_settings = temp.path().join("empty-settings.toml"); + std::fs::write(&sandboxed_settings, "_version = 1\n").unwrap(); + // SAFETY: single-threaded unit test body. + unsafe { + std::env::set_var("FABRO_CONFIG", &sandboxed_settings); + } + + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from("fabro/workflows/demo/workflow.toml"), + cwd: project.to_path_buf(), + args_layer: ConfigLayer::default(), + args: None, + run_id: None, + }) + .unwrap(); + + // SAFETY: single-threaded unit test body. + unsafe { + std::env::remove_var("FABRO_CONFIG"); + } + + let goal = built.manifest.goal.expect("manifest goal should be set"); + assert_eq!(goal.text, "ship from workflow dir"); + assert_eq!(goal.type_, types::ManifestGoalType::File); + let resolved = goal.path.expect("file goal must carry a path"); + let expected = workflow_dir.join("prompts").join("goal.md"); + assert_eq!(PathBuf::from(resolved), expected); + } } diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index eb7e569a6..2c49cf426 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -33,10 +33,11 @@ pub fn parse_project_config(content: &str) -> anyhow::Result { } /// Load a project config from a file path. +/// +/// Goes through [`ConfigLayer::load`] so that relative `run.goal.file` +/// paths are anchored at the directory of `path` at load time. pub fn load_project_config(path: &Path) -> anyhow::Result { - let content = std::fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; - let config = parse_project_config(&content)?; + let config = ConfigLayer::load(path).context("Failed to parse project config")?; let root = config .as_v2() .project @@ -477,4 +478,28 @@ retros = true assert_eq!(found_path, tmp.path().join("fabro.toml")); assert_eq!(config.as_v2().version, Some(1)); } + + #[test] + fn load_project_config_rewrites_relative_goal_file_path() { + use fabro_types::settings::run::RunGoalLayer; + + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("fabro.toml"); + fs::write( + &path, + r#"_version = 1 + +[run.goal] +file = "prompts/goal.md" +"#, + ) + .unwrap(); + + let config = load_project_config(&path).unwrap(); + let Some(RunGoalLayer::File { file }) = config.as_v2().run_goal_layer() else { + panic!("expected file variant"); + }; + let expected = tmp.path().join("prompts").join("goal.md"); + assert_eq!(file.as_source(), expected.to_string_lossy()); + } } diff --git a/lib/crates/fabro-config/src/run.rs b/lib/crates/fabro-config/src/run.rs index a5c015173..33fac41c4 100644 --- a/lib/crates/fabro-config/src/run.rs +++ b/lib/crates/fabro-config/src/run.rs @@ -18,11 +18,10 @@ pub fn parse_run_config(contents: &str) -> anyhow::Result { /// Load and parse a run config from a TOML file. /// -/// Returns the v2-backed `ConfigLayer`. +/// Goes through [`ConfigLayer::load`] so that relative `run.goal.file` +/// paths are anchored at the directory of `path` at load time. pub fn load_run_config(path: &Path) -> anyhow::Result { - let content = std::fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; - ConfigLayer::parse(&content) + ConfigLayer::load(path) .with_context(|| format!("Failed to parse workflow config at {}", path.display())) } @@ -34,3 +33,54 @@ pub fn resolve_graph_path(workflow_toml: &Path, graph_relative: &str) -> PathBuf .unwrap_or_else(|| Path::new(".")) .join(graph_relative) } + +#[cfg(test)] +mod tests { + use super::*; + use fabro_types::settings::run::RunGoalLayer; + + #[test] + fn load_run_config_rewrites_relative_goal_file_path() { + let tmp = tempfile::tempdir().unwrap(); + let workflow_dir = tmp.path().join("fabro").join("workflows").join("demo"); + std::fs::create_dir_all(&workflow_dir).unwrap(); + let workflow_toml = workflow_dir.join("workflow.toml"); + std::fs::write( + &workflow_toml, + r#"_version = 1 + +[run.goal] +file = "prompts/goal.md" +"#, + ) + .unwrap(); + + let config = load_run_config(&workflow_toml).unwrap(); + let Some(RunGoalLayer::File { file }) = config.as_v2().run_goal_layer() else { + panic!("expected file variant"); + }; + let expected = workflow_dir.join("prompts").join("goal.md"); + assert_eq!(file.as_source(), expected.to_string_lossy()); + } + + #[test] + fn load_run_config_leaves_absolute_goal_file_untouched() { + let tmp = tempfile::tempdir().unwrap(); + let workflow_toml = tmp.path().join("workflow.toml"); + std::fs::write( + &workflow_toml, + r#"_version = 1 + +[run.goal] +file = "/etc/fabro/goal.md" +"#, + ) + .unwrap(); + + let config = load_run_config(&workflow_toml).unwrap(); + let Some(RunGoalLayer::File { file }) = config.as_v2().run_goal_layer() else { + panic!("expected file variant"); + }; + assert_eq!(file.as_source(), "/etc/fabro/goal.md"); + } +}