diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index 1724c7e32..9e600cf16 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -71,13 +71,15 @@ pub(crate) async fn create_run( }, resolve_run_environment(client.as_ref(), args.environment.as_deref()), )?; + let configured_repo_origin_url = + fabro_manifest::configured_repo_origin_url_for_location(package.workflow_location())?; let fabro_manifest::DerivedRunTarget { target, dirty_worktree, } = fabro_manifest::derive_run_target_for_provider( environment.settings.provider, &canonical_cwd, - None, + configured_repo_origin_url.as_deref(), )?; if dirty_worktree { fabro_util::printerr!( diff --git a/lib/apps/fabro-server/src/run_tool_create.rs b/lib/apps/fabro-server/src/run_tool_create.rs index 987f7608c..71501d808 100644 --- a/lib/apps/fabro-server/src/run_tool_create.rs +++ b/lib/apps/fabro-server/src/run_tool_create.rs @@ -4,9 +4,9 @@ use anyhow::{Context, Result, bail}; use async_trait::async_trait; use fabro_environment::DEFAULT_ENVIRONMENT_ID; use fabro_manifest::{ - CollectedWorkflowClosure, DerivedRunTarget, ResolvedLocalWorkflowPackage, - collect_inline_workflow_versions, derive_run_target_for_provider, - resolve_local_workflow_package, + CollectedWorkflowClosure, DerivedRunTarget, collect_inline_workflow_versions, + configured_repo_origin_url_for_location, configured_repo_origin_url_from_workflow_toml, + derive_run_target_for_provider, resolve_local_workflow_package, }; use fabro_tool::{ CreateRunWorkflowSource, PreparedRunCreate, RunCreateAdapter, ValidatedCreateRunSpec, @@ -104,6 +104,7 @@ impl ServerRunCreateAdapter { client: &fabro_client::Client, spec: &ValidatedCreateRunSpec, cwd: &Path, + configured_repo_origin_url: Option<&str>, ) -> Result { if let Some(target) = &spec.target { if matches!(target, RunTarget::Folder { .. }) && !self.has_shared_filesystem() { @@ -152,11 +153,16 @@ impl ServerRunCreateAdapter { let canonical_cwd = fs::canonicalize(cwd).await.with_context(|| { format!("failed to canonicalize run directory {}", cwd.display()) })?; + let configured_repo_origin_url = configured_repo_origin_url.map(str::to_owned); let DerivedRunTarget { target, dirty_worktree, } = task::spawn_blocking(move || { - derive_run_target_for_provider(provider, &canonical_cwd, None) + derive_run_target_for_provider( + provider, + &canonical_cwd, + configured_repo_origin_url.as_deref(), + ) }) .await .context("run target derivation task failed")??; @@ -172,11 +178,7 @@ impl ServerRunCreateAdapter { } } - async fn collect_selector( - &self, - selector: &str, - cwd: &Path, - ) -> Result { + async fn collect_selector(&self, selector: &str, cwd: &Path) -> Result { if !self.has_shared_filesystem() { bail!( "workflow selectors require a shared Local filesystem; send inline files or an exact stored workflow version ID from Docker or Daytona" @@ -186,15 +188,27 @@ impl ServerRunCreateAdapter { let cwd = cwd.to_path_buf(); let user_workflows_root = self.user_workflows_root().map(Path::to_path_buf); task::spawn_blocking(move || { - resolve_local_workflow_package(&selector, &cwd, user_workflows_root.as_deref()) - .map(ResolvedLocalWorkflowPackage::into_closure) - .map_err(anyhow::Error::new) + let package = + resolve_local_workflow_package(&selector, &cwd, user_workflows_root.as_deref())?; + let configured_repo_origin_url = + configured_repo_origin_url_for_location(package.workflow_location())?; + Ok(CollectedWorkflow { + closure: package.into_closure(), + configured_repo_origin_url, + }) }) .await .context("workflow package collection task failed")? } } +/// A packaged workflow closure plus the `run.scm` repository its config names, +/// which standalone target derivation honors over the checkout's own origin. +struct CollectedWorkflow { + closure: CollectedWorkflowClosure, + configured_repo_origin_url: Option, +} + #[async_trait] impl RunCreateAdapter for ServerRunCreateAdapter { async fn prepare( @@ -204,11 +218,16 @@ impl RunCreateAdapter for ServerRunCreateAdapter { cwd: &Path, ) -> Result { let goal = self.resolve_goal(spec, cwd).await?; - let closure = match &spec.workflow { + let CollectedWorkflow { + closure, + configured_repo_origin_url, + } = match &spec.workflow { CreateRunWorkflowSource::Stored { workflow_version_id, } => { - let resolved_target = self.resolve_target(client, spec, cwd).await?; + // A stored version's config is not available locally, so + // derivation uses the checkout's own origin. + let resolved_target = self.resolve_target(client, spec, cwd, None).await?; return Ok(PreparedRunCreate { workflow_version_id: *workflow_version_id, target: resolved_target.target, @@ -221,7 +240,9 @@ impl RunCreateAdapter for ServerRunCreateAdapter { } CreateRunWorkflowSource::Inline(source) => collect_inline_workflow(source).await?, }; - let resolved_target = self.resolve_target(client, spec, cwd).await?; + let resolved_target = self + .resolve_target(client, spec, cwd, configured_repo_origin_url.as_deref()) + .await?; let versions = closure .versions() .map(|(_, version)| version.version()) @@ -246,11 +267,30 @@ struct ResolvedTarget { /// exact key of the file map, never a checkout selector. async fn collect_inline_workflow( source: &fabro_tool::InlineWorkflowSource, -) -> Result { +) -> Result { let entrypoint = source.entrypoint.clone(); let files = source.files.clone(); task::spawn_blocking(move || { - collect_inline_workflow_versions(&entrypoint, &files).map_err(anyhow::Error::new) + let closure = collect_inline_workflow_versions(&entrypoint, &files)?; + // The inline config is either the entrypoint itself or the + // `workflow.toml` beside the entrypoint graph. + let config_path = if Path::new(entrypoint.as_str()) + .extension() + .is_some_and(|ext| ext == "toml") + { + Some(entrypoint.clone()) + } else { + entrypoint.resolve_reference("workflow.toml").ok() + }; + let configured_repo_origin_url = config_path + .and_then(|path| files.get(&path)) + .map(|source| configured_repo_origin_url_from_workflow_toml(source)) + .transpose()? + .flatten(); + Ok(CollectedWorkflow { + closure, + configured_repo_origin_url, + }) }) .await .context("inline workflow package collection task failed")? @@ -931,4 +971,77 @@ mod tests { assert_eq!(prepared.target, RunTarget::None {}); } + + #[tokio::test] + async fn workflow_version_standalone_honors_configured_scm_repository_over_checkout_origin() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path().join("workspace"); + let origin = temp.path().join("origin.git"); + fs::create_dir(&workspace).await.unwrap(); + run_git(temp.path(), &[ + "init", + "--bare", + "--quiet", + origin.to_str().unwrap(), + ]); + run_git(&workspace, &[ + "init", + "--quiet", + "--initial-branch", + "feature", + ]); + run_git(&workspace, &["config", "user.name", "Fabro Test"]); + run_git(&workspace, &["config", "user.email", "fabro@example.com"]); + let workflow_dir = workspace.join(".fabro/workflows/demo"); + fs::create_dir_all(&workflow_dir).await.unwrap(); + fs::write(workspace.join(".fabro/project.toml"), "_version = 1\n") + .await + .unwrap(); + fs::write( + workflow_dir.join("workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.scm]\nowner = \"acme\"\nrepository = \"widgets\"\n", + ) + .await + .unwrap(); + fs::write( + workflow_dir.join("workflow.fabro"), + "digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .await + .unwrap(); + run_git(&workspace, &["add", "."]); + run_git(&workspace, &["commit", "--quiet", "-m", "initial"]); + // The checkout is a fork; the workflow names the upstream repository. + run_git(&workspace, &[ + "remote", + "add", + "origin", + "https://github.com/alice/widgets.git", + ]); + let push_url = format!("file://{}", origin.display()); + run_git(&workspace, &[ + "remote", "set-url", "--push", "origin", &push_url, + ]); + let server = MockServer::start_async().await; + mock_environment(&server, "default", "docker").await; + let registered = Arc::new(Mutex::new(Vec::new())); + let registration = + dynamic_version_registration_mock(&server, Arc::clone(®istered)).await; + let client = no_proxy_client(&server.url("")); + let spec = validated_spec(&json!({ "workflow": "demo" })); + let adapter = ServerRunCreateAdapter::standalone(None); + + let error = adapter + .prepare(&client, &spec, &workspace) + .await + .expect_err("a fork checkout must not silently become the run's repository"); + + registration.assert_calls_async(0).await; + assert!( + error + .to_string() + .contains("run.scm repository that is not the local checkout's origin"), + "unexpected error: {error:#}" + ); + } } diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 876ea5b89..532bc2e0e 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -17,10 +17,13 @@ use std::path::{Component, Path, PathBuf}; use anyhow::{Context, Result, anyhow}; use fabro_api::types; use fabro_config::project::{self, WorkflowLocation, discover_project_config}; -use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace}; +use fabro_config::run::{ + parse_run_layer_from_settings_toml, resolve_run_goal_from_layer, + resolve_run_goal_from_namespace, +}; use fabro_config::{ CliLayer, EnvironmentLayer, EnvironmentLifecycleLayer, MergeMap, ReplaceMap, - RunEnvironmentLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, + RunEnvironmentLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, RunScmLayer, WorkflowSettingsBuilder, }; use fabro_graphviz::graph::AttrValue; @@ -547,9 +550,42 @@ fn none_target_for_unversioned_directory( /// /// Returns an error when the source cannot be parsed or resolved. pub fn configured_repo_origin_url_from_workflow_toml(source: &str) -> Result> { - let settings = WorkflowSettingsBuilder::from_toml(source) - .context("failed to resolve workflow settings from workflow.toml")?; - Ok(configured_repo_origin_url(&settings)) + let run = parse_run_layer_from_settings_toml(source) + .context("failed to parse run settings from workflow.toml")?; + Ok(configured_repo_origin_url_from_scm_layer( + &run.scm.unwrap_or_default(), + )) +} + +/// The configured `run.scm` GitHub repository for a resolved local workflow. +/// `workflow.toml` values take precedence over the discovered +/// `.fabro/project.toml`, field by field, matching run settings layering. +/// `None` when neither names a repository. +/// +/// # Errors +/// +/// Returns an error when either config exists but cannot be read or parsed. +pub fn configured_repo_origin_url_for_location( + location: &WorkflowLocation, +) -> Result> { + let workflow = location + .toml + .as_deref() + .map(read_scm_layer) + .transpose()? + .unwrap_or_default(); + let project = discover_project_config(&location.dir)? + .as_deref() + .map(read_scm_layer) + .transpose()? + .unwrap_or_default(); + let scm = RunScmLayer { + provider: workflow.provider.or(project.provider), + owner: workflow.owner.or(project.owner), + repository: workflow.repository.or(project.repository), + github: workflow.github.or(project.github), + }; + Ok(configured_repo_origin_url_from_scm_layer(&scm)) } struct LocalGitObservation { @@ -626,15 +662,23 @@ fn github_run_target(origin_url: &str, branch: &str) -> Option { fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { let scm = &settings.run.scm; - if !scm - .provider - .as_deref() - .is_none_or(|provider| provider.eq_ignore_ascii_case("github")) - { + configured_repo_origin_url_from_scm( + scm.provider.as_deref(), + scm.owner.as_deref(), + scm.repository.as_deref(), + ) +} + +fn configured_repo_origin_url_from_scm( + provider: Option<&str>, + owner: Option<&str>, + repository: Option<&str>, +) -> Option { + if !provider.is_none_or(|provider| provider.eq_ignore_ascii_case("github")) { return None; } - let owner = scm.owner.as_deref()?; - let repository = scm.repository.as_deref()?; + let owner = owner?; + let repository = repository?; if owner.trim().is_empty() || repository.trim().is_empty() { return None; } @@ -643,6 +687,22 @@ fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { (!normalized.is_empty()).then_some(normalized) } +fn configured_repo_origin_url_from_scm_layer(scm: &RunScmLayer) -> Option { + configured_repo_origin_url_from_scm( + scm.provider.as_deref(), + scm.owner.as_deref(), + scm.repository.as_deref(), + ) +} + +fn read_scm_layer(path: &Path) -> Result { + let source = std::fs::read_to_string(path) + .with_context(|| format!("failed to read {}", path.display()))?; + let run = parse_run_layer_from_settings_toml(&source) + .with_context(|| format!("failed to parse run settings from {}", path.display()))?; + Ok(run.scm.unwrap_or_default()) +} + struct ManifestRepoInfo { /// The `origin` URL as libgit2 reports it (after `insteadOf` rewrites). origin_url: Option, @@ -825,6 +885,28 @@ pub(crate) mod test_fixtures { #[cfg(test)] mod tests { + #[test] + fn configured_repo_origin_url_reads_run_scm_from_workflow_toml() { + let configured = super::configured_repo_origin_url_from_workflow_toml( + "_version = 1\n[run.scm]\nowner = \"acme\"\nrepository = \"widgets\"\n", + ) + .unwrap(); + assert_eq!( + configured.as_deref(), + Some("https://github.com/acme/widgets") + ); + + let unconfigured = + super::configured_repo_origin_url_from_workflow_toml("_version = 1\n").unwrap(); + assert_eq!(unconfigured, None); + + let other_provider = super::configured_repo_origin_url_from_workflow_toml( + "_version = 1\n[run.scm]\nprovider = \"gitlab\"\nowner = \"acme\"\nrepository = \"widgets\"\n", + ) + .unwrap(); + assert_eq!(other_provider, None); + } + use fabro_workflow::git::head_sha; use super::*;