diff --git a/docs/public/core-concepts/workflows.mdx b/docs/public/core-concepts/workflows.mdx index 334bb642f..61bb09b5a 100644 --- a/docs/public/core-concepts/workflows.mdx +++ b/docs/public/core-concepts/workflows.mdx @@ -181,6 +181,10 @@ Temporary files are removed after collection or failure. Interrupting acquisitio stops owned Git processes before cleanup; collection already in progress must finish before its files can be removed. +For local workflows without target flags, the existing `run.scm` repository +configuration still participates in target inference. Explicit `--target-path` +and `--target-git` selections take precedence over that inferred repository. + Target selection depends on the environment: | Selection | Local environment | Clone-based environment (Docker, Daytona, or plugin) | diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index 750e175d6..9fe8d061b 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -96,10 +96,21 @@ pub(crate) async fn create_run( Some(package) => package, None => resolve_workflow().await?, }; + // Preserve configured repository inference for the existing local workflow + // path. Explicit targets select their own repository independently. + let configured_repo_origin_url = match &package { + ResolvedWorkflow::Local(package) + if args.target_path.is_none() && args.target_git.is_none() => + { + fabro_manifest::configured_repo_origin_url_for_location(package.workflow_location())? + } + _ => None, + }; let (target, dirty_worktree) = resolution::target( &target_selection, &environment.settings.provider, &canonical_cwd, + configured_repo_origin_url.as_deref(), interruption, ) .await?; diff --git a/lib/apps/fabro-cli/src/commands/run/resolution.rs b/lib/apps/fabro-cli/src/commands/run/resolution.rs index 049ddd7f2..085030c2c 100644 --- a/lib/apps/fabro-cli/src/commands/run/resolution.rs +++ b/lib/apps/fabro-cli/src/commands/run/resolution.rs @@ -1,15 +1,16 @@ use std::path::Path; -use anyhow::{Context as _, anyhow, bail}; +use anyhow::{Context as _, bail}; use fabro_manifest::{CollectedWorkflowClosure, ResolvedLocalWorkflowPackage}; -use fabro_types::{DirtyStatus, RunTarget, SandboxProviderKind}; +use fabro_types::{RunTarget, SandboxProviderKind}; use tokio::task; use super::remote_workflow::{Interruption, NativeGit}; use super::selection::{TargetSelection, WorkflowSelection}; /// Owns the canonical collector result without copying its contents. Local -/// location metadata remains available solely for existing settings warnings. +/// location metadata remains available for settings warnings and target +/// inference. pub(super) enum ResolvedWorkflow { Local(ResolvedLocalWorkflowPackage), Git(CollectedWorkflowClosure), @@ -71,6 +72,7 @@ pub(super) async fn target( selection: &TargetSelection, provider: &SandboxProviderKind, cwd: &Path, + configured_repo_origin_url: Option<&str>, interruption: &Interruption, ) -> anyhow::Result<(RunTarget, bool)> { let path = match selection { @@ -99,92 +101,17 @@ pub(super) async fn target( // The existing observer can push/query Git synchronously. Preserve its // behavior without blocking a Tokio worker or promising a new timeout. let provider = provider.clone(); - task::spawn_blocking(move || run_target_for_environment(&provider, &path)) - .await - .context("target observation task failed")? -} - -/// Derives the run target from the selected directory for the environment's -/// provider. Returns the target plus whether a clone-based observation found a -/// dirty Git worktree, so the caller can warn about it. -fn run_target_for_environment( - provider: &SandboxProviderKind, - canonical_cwd: &Path, -) -> anyhow::Result<(RunTarget, bool)> { - if !provider.clones_workspace() { - let path = canonical_cwd.to_str().ok_or_else(|| { - anyhow!( - "target directory is not valid UTF-8: {}", - canonical_cwd.display() - ) - })?; - return Ok(( - RunTarget::Folder { - path: path.to_string(), - }, - false, - )); - } - let Some(observation) = fabro_manifest::observe_git_run_target(canonical_cwd, None) else { - return Ok((none_target_for_unversioned_directory(canonical_cwd)?, false)); - }; - let dirty = observation.legacy_git_context.dirty == DirtyStatus::Dirty; - let target = observation.run_target.ok_or_else(|| { - anyhow!("the target Git checkout cannot be represented as a canonical GitHub run target") - })?; - if target.sha.is_none() { - bail!( - "the exact local Git commit could not be made available from the canonical GitHub origin; push the commit and try again" - ); - } - Ok((RunTarget::Git(target), dirty)) -} - -fn none_target_for_unversioned_directory(canonical_cwd: &Path) -> anyhow::Result { - let repository = match git2::Repository::discover(canonical_cwd) { - Ok(repository) => repository, - Err(source) if source.code() == git2::ErrorCode::NotFound => return Ok(RunTarget::None {}), - Err(source) => { - return Err(anyhow::Error::new(source)).with_context(|| { - format!( - "failed to inspect target directory {} for Git metadata", - canonical_cwd.display() - ) - }); - } - }; - - if repository.is_bare() { - bail!( - "the target directory resolves to a bare Git repository; clone-based runs require a non-bare checkout with an attached branch" - ); - } - match repository.head() { - Err(source) - if matches!( - source.code(), - git2::ErrorCode::UnbornBranch | git2::ErrorCode::NotFound - ) => - { - bail!( - "the target Git checkout has no commits; create a commit before using a clone-based environment" - ); - } - Err(source) => { - return Err(anyhow::Error::new(source)) - .context("failed to inspect the target Git checkout HEAD"); - } - Ok(head) if !head.is_branch() => { - bail!( - "the target Git checkout has a detached HEAD; check out a branch before using a clone-based environment" - ); - } - Ok(_) => {} - } - - bail!( - "the target Git checkout does not have a usable attached branch for a clone-based run target" - ) + let configured_repo_origin_url = configured_repo_origin_url.map(str::to_owned); + let derived = task::spawn_blocking(move || { + fabro_manifest::derive_run_target_for_provider( + &provider, + &path, + configured_repo_origin_url.as_deref(), + ) + }) + .await + .context("target observation task failed")??; + Ok((derived.target, derived.dirty_worktree)) } #[cfg(test)] @@ -205,10 +132,16 @@ mod tests { let selected = TargetSelection::Path("target".into()); let interruption = Interruption::new(false); assert_eq!( - target(&selected, &SandboxProviderKind::LOCAL, &root, &interruption) - .await - .unwrap() - .0, + target( + &selected, + &SandboxProviderKind::LOCAL, + &root, + None, + &interruption + ) + .await + .unwrap() + .0, RunTarget::Folder { path: root.join("target").to_str().unwrap().into(), } @@ -219,7 +152,7 @@ mod tests { SandboxProviderKind::try_new("host").unwrap(), ] { assert_eq!( - target(&selected, &provider, &root, &interruption) + target(&selected, &provider, &root, None, &interruption) .await .unwrap() .0, @@ -231,6 +164,7 @@ mod tests { &TargetSelection::Path(".".into()), &SandboxProviderKind::LOCAL, &root, + None, &interruption ) .await @@ -248,6 +182,7 @@ mod tests { }, &SandboxProviderKind::LOCAL, &root, + None, &interruption ) .await @@ -261,6 +196,7 @@ mod tests { &TargetSelection::Path(path.into()), &SandboxProviderKind::LOCAL, &root, + None, &interruption ) .await diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index c0ca5f604..ab3e62578 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -675,6 +675,85 @@ fn create_preserves_named_user_other_checkout_and_loose_file_selection() { } } +#[test] +fn create_preserves_configured_repository_inference_but_explicit_target_path_wins() { + let context = test_context!(); + let server = MockServer::start(); + let environment = mock_environment(&server, "default", "docker"); + let versions = mock_workflow_version_registrations(&server); + let requests = Arc::new(Mutex::new(Vec::new())); + let create = mock_intent_create(&server, &unique_run_id(), Arc::clone(&requests)); + let root = tempfile::tempdir().unwrap(); + let checkout = root.path().join("checkout"); + let workflow = write_workflow(&checkout, "workflow", "ConfiguredRepository"); + std::fs::write(&workflow, "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.scm]\nowner = \"acme\"\nrepository = \"configured\"\n").unwrap(); + let sha = init_remote_fixture(&checkout, "topic"); + let origin = root.path().join("origin.git"); + let bare = git2::Repository::init_bare(&origin).unwrap(); + run_git(&checkout, &[ + "remote", + "add", + "origin", + "https://github.com/acme/actual.git", + ]); + run_git(&checkout, &[ + "remote", + "set-url", + "--push", + "origin", + &format!("file://{}", origin.display()), + ]); + let server_url = format!("{}/api/v1", server.base_url()); + let inferred = context + .create_cmd() + .current_dir(&checkout) + .args(["--server", &server_url, workflow.to_str().unwrap()]) + .output() + .unwrap(); + assert!(!inferred.status.success()); + assert!( + output_stderr(&inferred) + .contains("run.scm repository that is not the local checkout's origin") + ); + assert!( + bare.find_reference("refs/heads/topic").is_err(), + "rejected inference must not publish the branch" + ); + versions.assert_calls(0); + create.assert_calls(0); + + let explicit = context + .create_cmd() + .current_dir(&checkout) + .args([ + "--server", + &server_url, + workflow.to_str().unwrap(), + "--target-path", + ".", + ]) + .output() + .unwrap(); + assert!(explicit.status.success(), "{}", output_stderr(&explicit)); + assert_eq!( + requests.lock().unwrap()[0]["target"], + json!({ + "kind": "git", "repo": "acme/actual", "branch": "topic", "sha": sha + }) + ); + assert_eq!( + bare.find_reference("refs/heads/topic") + .unwrap() + .target() + .unwrap() + .to_string(), + sha + ); + environment.assert_calls(2); + versions.assert_calls(1); + create.assert_calls(1); +} + #[test] fn create_clone_targets_require_exact_git_observations() { let context = test_context!(); @@ -896,9 +975,9 @@ fn create_rejects_unusable_git_checkouts_instead_of_sending_an_empty_target() { for (working_directory, expected_error) in [ ( detached.path(), - "the target Git checkout has a detached HEAD", + "the caller Git checkout has a detached HEAD", ), - (unborn.path(), "the target Git checkout has no commits"), + (unborn.path(), "the caller Git checkout has no commits"), ] { let output = context .create_cmd()