diff --git a/docs/public/core-concepts/workflows.mdx b/docs/public/core-concepts/workflows.mdx index 558334219..4119ef9e4 100644 --- a/docs/public/core-concepts/workflows.mdx +++ b/docs/public/core-concepts/workflows.mdx @@ -140,3 +140,71 @@ In the web UI, the Workflows page lists all available workflows. Click into a wo See the [Quick Start](/getting-started/quick-start) to try it out, or browse the [example workflows](/examples/repl-handoff) for real-world patterns. + +## Select workflow source and run target + +`fabro run` and `fabro create` accept the same source and target options. `create` +registers the workflow and leaves a submitted run for you to start with +`fabro start RUN`. `run` also starts it, then attaches unless you pass `--detach`. + +The required positional argument selects the workflow. With no source flags, +names are found in the current checkout, then a marked project, then installed +user workflows. Explicit local paths retain their usual package roots. + +```sh +fabro run review +fabro create ./review.toml --target-path ../app +fabro run review --workflow-git acme/workflows --workflow-ref v1.2 \ + --target-git acme/app --target-branch release +``` + +In the last example, workflow instructions come from `acme/workflows`, and the +run works on `acme/app`. Neither selection changes the other. Local workflow paths, +`--goal-file`, and other caller inputs still resolve from the invocation context. +Without target flags, Fabro keeps its existing cwd/environment-based target +inference. + +`--workflow-git OWNER/REPO` acquires source using native Git on your machine. A +workflow name selects `.fabro/workflows/NAME/workflow.toml` in that repository; +you can also supply an explicit repository-relative `.toml` or `.fabro` file. +Absolute paths, traversal, and directory selectors are rejected. A missing remote +workflow never falls back to a local or installed workflow. + +`--workflow-ref` requires `--workflow-git`. Omit it, or use `HEAD`, to select the +remote default branch. You can select a branch, tag, or full 40-hex commit SHA. +If a branch and tag share a name, qualify it with `refs/heads/` or `refs/tags/`. +Fabro resolves the revision once, fetches that exact commit into a temporary +checkout, and registers its workflow-version closure. A moved or unavailable +commit never causes a fallback to a newer revision. Checkout hooks, content +filters, and implicit Git LFS expansion are disabled; submodules are not fetched. +Temporary files are removed after collection or failure. Interrupting acquisition +stops owned Git processes before cleanup; collection already in progress must +finish before its files can be removed. + +Target selection depends on the environment: + +| Selection | Local environment | Docker/Daytona environment | +| --- | --- | --- | +| Default cwd or `--target-path PATH` | Uses the live directory, including uncommitted files | Uses the enclosing Git repository and exact available commit; a non-Git directory selects an empty workspace | +| `--target-git OWNER/REPO` | Rejected | Uses the selected repository and exact observed branch commit; cloning must be enabled | + +For clone-based execution, a target path selects a repository, not a subdirectory +working-directory override. Local target files are not uploaded. Existing target +observation may push committed local changes to origin; dirty changes are +excluded from clone targets and produce a warning. Detached or unavailable exact +commits fail. Folder targets require the directory to be accessible to the +server and its Local execution environment; passing a caller-local path does not +transfer it to a remote server. + +`--target-branch` requires `--target-git` and accepts a working branch name, not a +tag or SHA. Without it, Fabro resolves the repository's default branch. The CLI +only looks up target metadata; the execution sandbox clones the target. +`--target-path` and `--target-git` conflict. + +Local Git credential helpers, SSH-agent access through configured URL rewrites, +and user network configuration govern source acquisition and remote target +lookup. Fabro server login does not grant local Git access. The execution +sandbox still needs its own target-clone credentials. + +`--dry-run` simulates execution; it can still fetch and upload workflow source, +and existing local target observation can still publish committed changes. diff --git a/docs/public/reference/cli.mdx b/docs/public/reference/cli.mdx index c5a0ff1d4..8a4822416 100644 --- a/docs/public/reference/cli.mdx +++ b/docs/public/reference/cli.mdx @@ -70,7 +70,7 @@ fabro [OPTIONS] [COMMAND] | `fabro attach` | Attach to a running or finished workflow run | | `fabro auth` | Manage CLI authentication state | | `fabro completion` | Generate shell completions | -| `fabro create` | Register a local workflow version and create a submitted run | +| `fabro create` | Register a workflow version and create a submitted run | | `fabro deny` | Deny pending workflow runs | | `fabro discord` | Open the Discord community in the browser | | `fabro docs` | Open the docs website in the browser | @@ -92,7 +92,7 @@ fabro [OPTIONS] [COMMAND] | `fabro resume` | Resume an interrupted workflow run | | `fabro rewind` | Rewind a workflow run to an earlier checkpoint | | `fabro rm` | Remove one or more workflow runs | -| `fabro run` | Register a local workflow version, create a run, and start it | +| `fabro run` | Register a workflow version, create a run, and start it | | `fabro sandbox` | Sandbox operations (cp, ssh, preview) | | `fabro secret` | Manage server-owned secrets | | `fabro server` | Server operations | @@ -332,7 +332,7 @@ fabro completion [OPTIONS] ### `fabro create` -Register a local workflow version and create a submitted run +Register a workflow version and create a submitted run ```bash fabro create [OPTIONS] @@ -342,7 +342,7 @@ fabro create [OPTIONS] | Name | Description | | --- | --- | -| `WORKFLOW` | Local workflow name, checkout path, .fabro file, or workflow TOML | +| `WORKFLOW` | Workflow name or path (repository-relative with --workflow-git) | #### Options @@ -350,7 +350,7 @@ fabro create [OPTIONS] | --- | --- | | `--auto-approve` | Auto-approve all human gates | | `-d, --detach` | Run the workflow in the background and print the run ID | -| `--dry-run` | Execute with simulated LLM backend | +| `--dry-run` | Simulate execution; workflow source may still be fetched and uploaded | | `--environment ` | Named environment for agent tools | | `--goal ` | Override the workflow goal (available as {{ goal }} in prompts) | | `--goal-file ` | Read a per-run goal value from a local file | @@ -360,8 +360,13 @@ fabro create [OPTIONS] | `--preserve-sandbox` | Keep the sandbox alive after the run finishes (for debugging) | | `--provider ` | Override default LLM provider | | `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | +| `--target-branch ` | Target working branch (default: remote default branch), pinned to its observed commit | +| `--target-git ` | Target GitHub OWNER/REPO; the execution sandbox still needs its own clone credentials | +| `--target-path ` | Observe this target directory instead of cwd; Folder targets require server filesystem access | | `-I, --input ` | Override a workflow input value (repeatable, format: KEY=VALUE) | | `-v, --verbose` | Enable verbose output | +| `--workflow-git ` | Acquire workflow source locally from a GitHub OWNER/REPO using native Git credentials | +| `--workflow-ref ` | Workflow branch, tag, HEAD (default), or full commit SHA; qualify ambiguous names | ### `fabro deny` @@ -1061,7 +1066,7 @@ fabro rm [OPTIONS] ... ### `fabro run` -Register a local workflow version, create a run, and start it +Register a workflow version, create a run, and start it ```bash fabro run [OPTIONS] @@ -1071,7 +1076,7 @@ fabro run [OPTIONS] | Name | Description | | --- | --- | -| `WORKFLOW` | Local workflow name, checkout path, .fabro file, or workflow TOML | +| `WORKFLOW` | Workflow name or path (repository-relative with --workflow-git) | #### Options @@ -1079,7 +1084,7 @@ fabro run [OPTIONS] | --- | --- | | `--auto-approve` | Auto-approve all human gates | | `-d, --detach` | Run the workflow in the background and print the run ID | -| `--dry-run` | Execute with simulated LLM backend | +| `--dry-run` | Simulate execution; workflow source may still be fetched and uploaded | | `--environment ` | Named environment for agent tools | | `--goal ` | Override the workflow goal (available as {{ goal }} in prompts) | | `--goal-file ` | Read a per-run goal value from a local file | @@ -1089,8 +1094,13 @@ fabro run [OPTIONS] | `--preserve-sandbox` | Keep the sandbox alive after the run finishes (for debugging) | | `--provider ` | Override default LLM provider | | `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | +| `--target-branch ` | Target working branch (default: remote default branch), pinned to its observed commit | +| `--target-git ` | Target GitHub OWNER/REPO; the execution sandbox still needs its own clone credentials | +| `--target-path ` | Observe this target directory instead of cwd; Folder targets require server filesystem access | | `-I, --input ` | Override a workflow input value (repeatable, format: KEY=VALUE) | | `-v, --verbose` | Enable verbose output | +| `--workflow-git ` | Acquire workflow source locally from a GitHub OWNER/REPO using native Git credentials | +| `--workflow-ref ` | Workflow branch, tag, HEAD (default), or full commit SHA; qualify ambiguous names | ### `fabro sandbox` diff --git a/lib/apps/fabro-cli/Cargo.toml b/lib/apps/fabro-cli/Cargo.toml index 8b844e79d..43b3beedb 100644 --- a/lib/apps/fabro-cli/Cargo.toml +++ b/lib/apps/fabro-cli/Cargo.toml @@ -99,7 +99,7 @@ object_store.workspace = true bytes.workspace = true tokio-util.workspace = true libc = "0.2" -nix = { version = "0.30", features = ["fs"] } +nix = { version = "0.30", features = ["fs", "signal", "process"] } [target.'cfg(target_os = "macos")'.dependencies] core-foundation = { version = "0.9", optional = true } diff --git a/lib/apps/fabro-cli/src/args.rs b/lib/apps/fabro-cli/src/args.rs index cc04ef12a..357e2eaff 100644 --- a/lib/apps/fabro-cli/src/args.rs +++ b/lib/apps/fabro-cli/src/args.rs @@ -7,6 +7,7 @@ use fabro_agent::cli::AgentArgs; use fabro_config::{CliLayer, CliLoggingLayer, CliOutputLayer, CliUpdatesLayer}; use fabro_server::serve::DEFAULT_TCP_PORT; use fabro_static::EnvVars; +use fabro_types::GitHubRepositorySlug; use fabro_types::settings::cli::{OutputFormat, OutputVerbosity}; use fabro_types::settings::run::MergeStrategy; use fabro_util::printer::Printer; @@ -233,11 +234,36 @@ pub(crate) struct RunArgs { #[command(flatten)] pub(crate) inputs: InputOverrideArgs, - /// Local workflow name, checkout path, .fabro file, or workflow TOML + /// Workflow name or path (repository-relative with --workflow-git) #[arg(required = true)] pub(crate) workflow: Option, - /// Execute with simulated LLM backend + /// Acquire workflow source locally from a GitHub OWNER/REPO using native + /// Git credentials + #[arg(long, value_name = "OWNER/REPO")] + pub(crate) workflow_git: Option, + + /// Workflow branch, tag, HEAD (default), or full commit SHA; qualify + /// ambiguous names + #[arg(long, requires = "workflow_git", value_name = "REF")] + pub(crate) workflow_ref: Option, + + /// Observe this target directory instead of cwd; Folder targets require + /// server filesystem access + #[arg(long, conflicts_with = "target_git", value_name = "PATH")] + pub(crate) target_path: Option, + + /// Target GitHub OWNER/REPO; the execution sandbox still needs its own + /// clone credentials + #[arg(long, value_name = "OWNER/REPO")] + pub(crate) target_git: Option, + + /// Target working branch (default: remote default branch), pinned to its + /// observed commit + #[arg(long, requires = "target_git", value_name = "BRANCH")] + pub(crate) target_branch: Option, + + /// Simulate execution; workflow source may still be fetched and uploaded #[arg(long)] pub(crate) dry_run: bool, @@ -1138,10 +1164,10 @@ pub(crate) struct UpgradeArgs { #[derive(Subcommand)] pub(crate) enum RunCommands { - /// Register a local workflow version, create a run, and start it - Run(RunArgs), - /// Register a local workflow version and create a submitted run - Create(RunArgs), + /// Register a workflow version, create a run, and start it + Run(Box), + /// Register a workflow version and create a submitted run + Create(Box), /// Start a created workflow run on the server Start(StartArgs), /// Attach to a running or finished workflow run @@ -1869,3 +1895,56 @@ fn parse_reasoning_effort_arg(value: &str) -> Result { ) }) } + +#[cfg(test)] +mod run_selection_grammar_tests { + use clap::Parser as _; + + use super::RunArgs; + + #[derive(clap::Parser)] + struct Command { + #[command(flatten)] + run: RunArgs, + } + + #[test] + fn run_selection_accepts_independent_resource_flags() { + for flags in [ + vec![ + "fabro", + "review", + "--workflow-git", + "acme/workflows", + "--workflow-ref", + "refs/tags/v1", + "--target-git", + "acme/app", + "--target-branch", + "release/topic", + ], + vec!["fabro", "./review.toml", "--target-path", "../app"], + ] { + assert!(Command::try_parse_from(flags).is_ok()); + } + } + + #[test] + fn run_selection_requires_modifier_owners_and_exclusive_targets() { + for flags in [ + vec!["fabro", "review", "--workflow-ref", "v1"], + vec!["fabro", "review", "--target-branch", "release"], + vec![ + "fabro", + "review", + "--target-path", + ".", + "--target-git", + "acme/app", + ], + vec!["fabro", "--workflow-git", "acme/workflows"], + ] { + assert!(Command::try_parse_from(flags).is_err()); + } + } +} diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index f9d0fb037..380a19dea 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -1,13 +1,15 @@ use std::path::Path; -use anyhow::{Context as _, anyhow, bail}; +use anyhow::{Context as _, anyhow}; use fabro_config::project; use fabro_environment::{DEFAULT_ENVIRONMENT_ID, Environment}; -use fabro_types::settings::run::EnvironmentProvider; -use fabro_types::{DirtyStatus, RunId, RunIntent, RunTarget}; +use fabro_types::{RunId, RunIntent}; use fabro_util::terminal::Styles; use super::overrides::prepare_intent_overrides; +use super::resolution::ResolvedWorkflow; +use super::selection::WorkflowSelection; +use super::{resolution, selection}; use crate::args::RunArgs; use crate::command_context::CommandContext; use crate::commands::resolve_run_id; @@ -17,7 +19,7 @@ pub(crate) struct CreatedRun { pub(crate) run_id: RunId, } -/// Register the local workflow version closure with the server and create a +/// Register the workflow version closure with the server and create a /// run from an immutable workflow intent, leaving it in the submitted state. /// /// This does NOT start the workflow — starting is a separate request. @@ -26,10 +28,7 @@ pub(crate) async fn create_run( args: &RunArgs, styles: &Styles, ) -> anyhow::Result { - let workflow_path = args - .workflow - .as_ref() - .ok_or_else(|| anyhow::anyhow!("--workflow is required"))?; + let (workflow_selection, target_selection) = selection::parse(args)?; let canonical_cwd = ctx.cwd().canonicalize().with_context(|| { format!( "failed to canonicalize caller working directory {}", @@ -37,11 +36,20 @@ pub(crate) async fn create_run( ) })?; let user_workflows_root = fabro_util::Home::from_env().workflows_dir(); - let package = fabro_manifest::resolve_local_workflow_package( - workflow_path, - &canonical_cwd, - Some(&user_workflows_root), - )?; + // Preserve local lookup diagnostics before contacting the server. Remote + // acquisition waits until parent, environment, and target are validated. + let local_package = if matches!(workflow_selection, WorkflowSelection::Local(_)) { + Some( + resolution::workflow( + &workflow_selection, + &canonical_cwd, + Some(&user_workflows_root), + ) + .await?, + ) + } else { + None + }; let prepared = prepare_intent_overrides(args, &canonical_cwd).await?; warn_untransmitted_settings( @@ -50,7 +58,12 @@ pub(crate) async fn create_run( ctx.base_config_path(), *ctx.run_settings_key_presence(), ); - let project_config = project::discover_project_config(&package.workflow_location().dir)?; + let project_config = match &local_package { + Some(ResolvedWorkflow::Local(package)) => { + project::discover_project_config(&package.workflow_location().dir)? + } + _ => project::discover_project_config(&canonical_cwd)?, + }; if let Some(path) = project_config.as_deref() { warn_untransmitted_settings( ctx, @@ -72,15 +85,30 @@ pub(crate) async fn create_run( }, resolve_run_environment(client.as_ref(), args.environment.as_deref()), )?; - let (target, dirty_worktree) = - run_target_for_environment(environment.settings.provider, &canonical_cwd)?; + let (target, dirty_worktree) = resolution::target( + &target_selection, + environment.settings.provider, + &canonical_cwd, + ) + .await?; if dirty_worktree { fabro_util::printerr!( ctx.printer(), - "{} the caller Git working tree is dirty; uncommitted changes are not included in the run target.", + "{} the selected target Git working tree is dirty; uncommitted changes are not included in the run target.", styles.yellow.apply_to("Warning:"), ); } + let package = match local_package { + Some(package) => package, + None => { + resolution::workflow( + &workflow_selection, + &canonical_cwd, + Some(&user_workflows_root), + ) + .await? + } + }; let workflow_version_id = package.closure().root_id(); client .register_workflow_versions( @@ -164,86 +192,3 @@ fn warn_untransmitted_settings( keys.join(", "), ); } - -/// Derives the run target from the caller 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: EnvironmentProvider, - canonical_cwd: &Path, -) -> anyhow::Result<(RunTarget, bool)> { - if !provider.is_clone_based() { - let path = canonical_cwd.to_str().ok_or_else(|| { - anyhow!( - "caller working 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 caller 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 caller working directory {} for Git metadata", - canonical_cwd.display() - ) - }); - } - }; - - if repository.is_bare() { - bail!( - "the caller 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 caller 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 caller Git checkout HEAD"); - } - Ok(head) if !head.is_branch() => { - bail!( - "the caller Git checkout has a detached HEAD; check out a branch before using a clone-based environment" - ); - } - Ok(_) => {} - } - - bail!( - "the caller Git checkout does not have a usable attached branch for a clone-based run target" - ) -} diff --git a/lib/apps/fabro-cli/src/commands/run/mod.rs b/lib/apps/fabro-cli/src/commands/run/mod.rs index 7d0c2e3ed..fa0854a0c 100644 --- a/lib/apps/fabro-cli/src/commands/run/mod.rs +++ b/lib/apps/fabro-cli/src/commands/run/mod.rs @@ -21,10 +21,13 @@ pub(crate) mod logs; pub(crate) mod output; pub(crate) mod overrides; pub(crate) mod preview; +mod remote_workflow; +mod resolution; pub(crate) mod resume; pub(crate) mod rewind; pub(crate) mod run_progress; pub(crate) mod runner; +mod selection; pub(crate) mod ssh; pub(crate) mod start; pub(crate) mod steer; @@ -38,7 +41,7 @@ pub(crate) async fn dispatch( let printer = base_ctx.printer(); match cmd { - RunCommands::Run(args) => Box::pin(command::execute(args, base_ctx)).await, + RunCommands::Run(args) => Box::pin(command::execute(*args, base_ctx)).await, RunCommands::Create(args) => { let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); let ctx = base_ctx.with_target(&args.target)?; diff --git a/lib/apps/fabro-cli/src/commands/run/overrides.rs b/lib/apps/fabro-cli/src/commands/run/overrides.rs index 3c0ffca45..a925b9b01 100644 --- a/lib/apps/fabro-cli/src/commands/run/overrides.rs +++ b/lib/apps/fabro-cli/src/commands/run/overrides.rs @@ -168,6 +168,11 @@ mod tests { target: ServerTargetArgs::default(), inputs: InputOverrideArgs::default(), workflow: Some(PathBuf::from("workflow.fabro")), + workflow_git: None, + workflow_ref: None, + target_path: None, + target_git: None, + target_branch: None, dry_run: false, auto_approve: false, goal: None, diff --git a/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs b/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs new file mode 100644 index 000000000..fc2760af1 --- /dev/null +++ b/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs @@ -0,0 +1,1012 @@ +//! Native Git acquisition owned by the CLI, with no server credential lookup. +use std::future::Future; +use std::path::{Path, PathBuf}; +use std::process::{ExitStatus, Stdio}; +use std::time::Duration; + +use anyhow::{Context as _, bail}; +use fabro_manifest::CollectedWorkflowClosure; +use fabro_types::{GitHubRepositorySlug, GitRunTarget, repository}; +use nix::errno::Errno; +use nix::sys::signal::{self, Signal}; +use nix::unistd::Pid; +use tokio::io::{AsyncRead, AsyncReadExt as _}; +use tokio::process::Command; +use tokio::{fs, signal as tokio_signal, task, time}; +use tokio_util::sync::CancellationToken; + +use super::selection::{self, RemoteWorkflowRevision}; + +const OUTPUT_LIMIT: usize = 64 * 1024; + +#[derive(Debug, thiserror::Error)] +pub(super) enum RemoteWorkflowError { + #[error( + "local Git {operation} failed ({status}); verify native Git access to the repository using your local credential helper or SSH configuration; Fabro server login does not grant Git access" + )] + Process { + operation: &'static str, + status: ExitStatus, + }, + #[error("local Git command timed out")] + Timeout, + #[error("local Git acquisition cancelled")] + Cancelled, + #[error("local Git metadata exceeds the 64 KiB capture limit")] + OutputLimit, + #[error("local Git I/O failed")] + Io(#[from] std::io::Error), +} + +pub(super) struct NativeGit { + cwd: PathBuf, + timeout: Duration, + #[cfg(test)] + pub(super) environment: Vec<(String, String)>, +} + +impl NativeGit { + pub(super) fn new(cwd: PathBuf) -> Self { + Self { + cwd, + timeout: Duration::from_mins(2), + #[cfg(test)] + environment: Vec::new(), + } + } + + async fn command( + &self, + cwd: &Path, + args: &[&str], + cancel: &CancellationToken, + ) -> Result, RemoteWorkflowError> { + if cancel.is_cancelled() { + return Err(RemoteWorkflowError::Cancelled); + } + let mut command = Command::new("git"); + command + .current_dir(cwd) + .args([ + "-c", + "core.hooksPath=/dev/null", + "-c", + "filter.lfs.smudge=", + "-c", + "filter.lfs.process=", + "-c", + "filter.lfs.required=false", + "-c", + "submodule.recurse=false", + "-c", + "protocol.ext.allow=never", + "-c", + "maintenance.auto=0", + "-c", + "gc.auto=0", + "-c", + "core.autocrlf=false", + ]) + .args(args) + .env("GIT_TERMINAL_PROMPT", "0") + .env("GIT_LFS_SKIP_SMUDGE", "1") + .env_remove("GIT_DIR") + .env_remove("GIT_WORK_TREE") + .env_remove("GIT_INDEX_FILE") + .env_remove("GIT_OBJECT_DIRECTORY") + .env_remove("GIT_ALTERNATE_OBJECT_DIRECTORIES") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true); + #[cfg(unix)] + command.process_group(0); + #[cfg(test)] + command.envs(self.environment.iter().cloned()); + let mut child = command.spawn()?; + let process_id = child.id(); + let mut stdout = child.stdout.take().expect("piped Git stdout exists"); + let mut stderr = child.stderr.take().expect("piped Git stderr exists"); + let result = tokio::select! { + biased; + () = cancel.cancelled() => Err(RemoteWorkflowError::Cancelled), + () = time::sleep(self.timeout) => Err(RemoteWorkflowError::Timeout), + result = async { + let (status, (stdout, overflow), _) = tokio::try_join!( + child.wait(), capture(&mut stdout), capture(&mut stderr) + )?; + if !status.success() { + // Output may contain arbitrary helper/config secrets, even after pattern + // redaction. Never retain it in an error/cause chain or tracing event. + return Err(RemoteWorkflowError::Process { operation: operation(args), status }); + } + if overflow { return Err(RemoteWorkflowError::OutputLimit); } + Ok(stdout) + } => result, + }; + // Helpers may inherit pipes or survive their Git parent. Terminate the + // owned process group on both completion and interruption, then reap Git. + let mut cleanup_error = None; + #[cfg(unix)] + if let Some(id) = process_id { + let pid = Pid::from_raw(id.cast_signed()); + match signal::killpg(pid, Signal::SIGKILL) { + Ok(()) | Err(Errno::ESRCH) => {} + Err(source) => cleanup_error = Some(std::io::Error::from(source)), + } + } + if result.is_err() { + if let Err(source) = child.kill().await { + cleanup_error = Some(source); + } + } + child.wait().await?; + if let Some(source) = cleanup_error { + return Err(source.into()); + } + result + } + + async fn records( + &self, + repository: &GitHubRepositorySlug, + patterns: &[String], + cancel: &CancellationToken, + ) -> anyhow::Result { + let url = repository.https_url(); + let mut args = vec!["ls-remote", "--symref", &url]; + args.extend(patterns.iter().map(String::as_str)); + let bytes = self.command(&self.cwd, &args, cancel).await?; + Ok(std::str::from_utf8(&bytes) + .context("local Git returned invalid metadata encoding")? + .to_owned()) + } + + pub(super) async fn resolve_target( + &self, + repository: GitHubRepositorySlug, + branch: Option, + cancel: &CancellationToken, + ) -> anyhow::Result { + let (branch, sha) = if let Some(branch) = branch { + if !repository::is_valid_git_branch_name(&branch) { + bail!("invalid target working branch"); + } + let reference = format!("refs/heads/{branch}"); + let records = self + .records(&repository, std::slice::from_ref(&reference), cancel) + .await?; + let sha = exact_record(&records, &reference)?.context("target branch was not found")?; + (branch, sha) + } else { + let records = self.records(&repository, &["HEAD".into()], cancel).await?; + default_head(&records)? + }; + let target = GitRunTarget { + repo: repository.to_string(), + branch, + tag: None, + sha: Some(sha), + }; + Ok(target.validate()?.into_target()) + } + + pub(super) async fn resolve_revision( + &self, + repository: &GitHubRepositorySlug, + revision: &RemoteWorkflowRevision, + cancel: &CancellationToken, + ) -> anyhow::Result { + match revision { + RemoteWorkflowRevision::Commit(sha) => { + repository::normalize_git_commit_sha(sha).context("invalid full source commit SHA") + } + RemoteWorkflowRevision::DefaultBranch => { + let records = self.records(repository, &["HEAD".into()], cancel).await?; + Ok(default_head(&records)?.1) + } + RemoteWorkflowRevision::Ref(reference) => { + RemoteWorkflowRevision::parse(Some(reference))?; + let patterns = if reference.starts_with("refs/") { + vec![reference.clone(), format!("{reference}^{{}}")] + } else { + vec![ + format!("refs/heads/{reference}"), + format!("refs/tags/{reference}"), + format!("refs/tags/{reference}^{{}}"), + ] + }; + let records = self.records(repository, &patterns, cancel).await?; + resolve_named_ref(&records, reference) + } + } + } + + pub(super) async fn collect( + &self, + repository: GitHubRepositorySlug, + selector: PathBuf, + revision: RemoteWorkflowRevision, + cancel: CancellationToken, + ) -> anyhow::Result { + selection::validate_remote_selector(&selector)?; + let sha = self + .resolve_revision(&repository, &revision, &cancel) + .await?; + let checkout = tempfile::Builder::new() + .prefix("fabro-workflow-") + .tempdir()?; + self.collect_checkout(repository, selector, sha, checkout, cancel) + .await + } + + async fn collect_checkout( + &self, + repository: GitHubRepositorySlug, + selector: PathBuf, + sha: String, + checkout: tempfile::TempDir, + cancel: CancellationToken, + ) -> anyhow::Result { + self.checkout(&repository, &sha, checkout.path(), &cancel) + .await?; + // Moving the directory into the blocking task keeps it alive even if + // the outer future is dropped. Collection is not interruptible; finish + // it and clean up before reporting cancellation. + let collection_cancel = cancel.clone(); + let closure = task::spawn_blocking(move || { + check_source_symlinks(checkout.path())?; + if collection_cancel.is_cancelled() { + return Err(RemoteWorkflowError::Cancelled.into()); + } + fabro_manifest::collect_workflow_versions(&selector, checkout.path()) + .map_err(anyhow::Error::new) + }) + .await + .context("workflow collection task failed")??; + if cancel.is_cancelled() { + return Err(RemoteWorkflowError::Cancelled.into()); + } + Ok(closure) + } + + async fn checkout( + &self, + repository: &GitHubRepositorySlug, + sha: &str, + root: &Path, + cancel: &CancellationToken, + ) -> anyhow::Result<()> { + self.command(root, &["init", "--quiet", "--template="], cancel) + .await?; + self.command( + root, + &[ + "fetch", + "--quiet", + "--depth=1", + "--no-tags", + "--no-recurse-submodules", + &repository.https_url(), + sha, + ], + cancel, + ) + .await?; + let kind = self.command(root, &["cat-file", "-t", sha], cancel).await?; + if kind != b"commit\n" { + bail!("the selected source SHA is not a commit"); + } + // The highest-precedence attributes file prevents repository attributes + // from invoking configured filters or rewriting the committed source bytes. + fs::create_dir_all(root.join(".git/info")).await?; + fs::write( + root.join(".git/info/attributes"), + "* -filter -text -ident -working-tree-encoding\n", + ) + .await?; + self.command( + root, + &[ + "-c", + &format!("core.worktree={}", root.display()), + "checkout", + "--quiet", + "--detach", + sha, + "--", + ], + cancel, + ) + .await?; + let head = self + .command(root, &["rev-parse", "--verify", "HEAD"], cancel) + .await?; + if head != format!("{sha}\n").as_bytes() { + bail!("workflow checkout did not match the selected commit"); + } + Ok(()) + } +} + +fn operation(args: &[&str]) -> &'static str { + match args.first().copied() { + Some("ls-remote") => "metadata lookup", + Some("fetch") => "fetch", + Some("checkout") => "checkout", + _ => "repository operation", + } +} + +async fn capture(reader: &mut (impl AsyncRead + Unpin)) -> std::io::Result<(Vec, bool)> { + let mut captured = Vec::new(); + let mut overflow = false; + let mut buffer = vec![0; 8192]; + loop { + let count = reader.read(&mut buffer).await?; + if count == 0 { + return Ok((captured, overflow)); + } + let keep = count.min(OUTPUT_LIMIT - captured.len()); + captured.extend_from_slice(&buffer[..keep]); + overflow |= keep != count; + } +} + +fn exact_record(records: &str, reference: &str) -> anyhow::Result> { + let mut found = None; + for line in records.lines() { + let Some((value, name)) = line.split_once('\t') else { + continue; + }; + if name != reference || value.starts_with("ref: ") { + continue; + } + let sha = repository::normalize_git_commit_sha(value) + .context("invalid Git metadata commit SHA")?; + if found.as_ref().is_some_and(|previous| previous != &sha) { + bail!("conflicting Git metadata records"); + } + found = Some(sha); + } + Ok(found) +} + +fn default_head(records: &str) -> anyhow::Result<(String, String)> { + let mut branch = None; + for line in records.lines() { + if let Some(value) = line + .strip_prefix("ref: refs/heads/") + .and_then(|line| line.strip_suffix("\tHEAD")) + { + if !repository::is_valid_github_ref_selector(value) { + bail!("remote default HEAD does not name a valid branch"); + } + if branch.is_some() { + bail!("ambiguous remote default HEAD"); + } + branch = Some(value.to_owned()); + } + } + Ok(( + branch.context("remote default HEAD must name a branch")?, + exact_record(records, "HEAD")?.context("remote default HEAD has no commit")?, + )) +} + +fn resolve_named_ref(records: &str, reference: &str) -> anyhow::Result { + let heads = if reference.starts_with("refs/") { + reference.to_owned() + } else { + format!("refs/heads/{reference}") + }; + let tags = if reference.starts_with("refs/") { + reference.to_owned() + } else { + format!("refs/tags/{reference}") + }; + let branch = if heads.starts_with("refs/heads/") { + exact_record(records, &heads)? + } else { + None + }; + let tag = if tags.starts_with("refs/tags/") { + exact_record(records, &tags)? + } else { + None + }; + if branch.is_some() && tag.is_some() { + bail!( + "workflow ref is ambiguous between a branch and tag; use refs/heads/... or refs/tags/..." + ); + } + if let Some(sha) = branch { + return Ok(sha); + } + if let Some(sha) = tag { + return Ok(exact_record(records, &format!("{tags}^{{}}"))?.unwrap_or(sha)); + } + bail!("workflow ref was not found; no alternative revision was selected") +} + +fn check_source_symlinks(root: &Path) -> anyhow::Result<()> { + let root = root.canonicalize()?; + // Validate links before the collector's initial TOML lookup can read them. + for entry in walkdir::WalkDir::new(&root) + .follow_links(false) + .into_iter() + .filter_entry(|entry| entry.path() != root.join(".git")) + { + let entry = entry?; + if entry.file_type().is_symlink() && !entry.path().canonicalize()?.starts_with(&root) { + bail!("remote workflow checkout contains a symlink outside its source root"); + } + } + Ok(()) +} + +/// The task owns its child processes and temporary checkout. Dropping the +/// waiter requests cooperative cleanup, not task abortion. Ctrl-C waits for +/// cleanup; an in-progress blocking collection must finish first. +pub(super) async fn owned( + work: impl FnOnce(CancellationToken) -> TFuture + Send + 'static, +) -> anyhow::Result +where + TFuture: Future> + Send + 'static, +{ + let cancel = CancellationToken::new(); + let _cancel_on_drop = cancel.clone().drop_guard(); + let mut task = tokio::spawn(work(cancel.clone())); + tokio::select! { + result = &mut task => result.context("local Git task failed")?, + signal = tokio_signal::ctrl_c() => { + cancel.cancel(); + let _result = task.await.context("local Git cleanup task failed")?; + signal.context("failed to listen for interruption")?; + Err(RemoteWorkflowError::Cancelled.into()) + } + } +} + +#[cfg(test)] +#[expect( + clippy::disallowed_methods, + reason = "hermetic Git fixtures and fake executables use synchronous file setup" +)] +mod tests { + use std::os::unix::fs::PermissionsExt as _; + + use nix::sys::stat::Mode; + use nix::unistd; + + use super::*; + + struct Fixture { + root: tempfile::TempDir, + repo: git2::Repository, + git: NativeGit, + sha: String, + } + + impl Fixture { + fn new() -> Self { + let root = tempfile::tempdir().unwrap(); + let repo_dir = root.path().join("source"); + let repo = git2::Repository::init_opts( + &repo_dir, + git2::RepositoryInitOptions::new().initial_head("trunk"), + ) + .unwrap(); + let workflow = repo_dir.join(".fabro/workflows/review"); + std::fs::create_dir_all(&workflow).unwrap(); + std::fs::write( + workflow.join("workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + workflow.join("workflow.fabro"), + "digraph Review { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + let mut index = repo.index().unwrap(); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .unwrap(); + let tree_id = index.write_tree().unwrap(); + let sha = { + let tree = repo.find_tree(tree_id).unwrap(); + let signature = git2::Signature::now("Fixture", "fixture@example.test").unwrap(); + repo.commit(Some("HEAD"), &signature, &signature, "workflow", &tree, &[]) + .unwrap() + .to_string() + }; + let config = root.path().join("gitconfig"); + std::fs::write( + &config, + format!( + "[url \"file://{}\"]\n insteadOf = https://github.com/acme/workflows\n", + repo_dir.display() + ), + ) + .unwrap(); + let mut git = NativeGit::new(root.path().to_path_buf()); + git.environment = vec![ + ("GIT_CONFIG_GLOBAL".into(), config.to_str().unwrap().into()), + ("GIT_CONFIG_NOSYSTEM".into(), "1".into()), + ("GIT_CONFIG_COUNT".into(), "0".into()), + ]; + Self { + root, + repo, + git, + sha, + } + } + + fn repository() -> GitHubRepositorySlug { + "acme/workflows".parse().unwrap() + } + } + + impl Fixture { + fn commit_changes(&self) -> String { + let mut index = self.repo.index().unwrap(); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = self.repo.find_tree(tree_id).unwrap(); + let parent = self.repo.head().unwrap().peel_to_commit().unwrap(); + let signature = git2::Signature::now("Fixture", "fixture@example.test").unwrap(); + self.repo + .commit( + Some("HEAD"), + &signature, + &signature, + "update workflow", + &tree, + &[&parent], + ) + .unwrap() + .to_string() + } + } + + #[tokio::test] + async fn remote_workflow_cleanup_preserves_sibling_identity_and_disables_filters_hooks() { + let fixture = Fixture::new(); + let source = fixture.repo.workdir().unwrap(); + let child = source.join(".fabro/workflows/child"); + std::fs::create_dir_all(&child).unwrap(); + std::fs::write( + child.join("workflow.fabro"), + "digraph Child { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + std::fs::write(source.join(".fabro/workflows/review/workflow.fabro"), "digraph Root { start [shape=Mdiamond] exit [shape=Msquare] child [shape=house, stack.child_workflow=\"../child/workflow.fabro\"] start -> child -> exit }").unwrap(); + std::fs::write(source.join(".gitattributes"), "*.fabro filter=fixture\n").unwrap(); + let config_path = fixture.root.path().join("gitconfig"); + let mut config = git2::Config::open(&config_path).unwrap(); + let sentinel = fixture.root.path().join("executed"); + let hooks = fixture.root.path().join("hooks"); + std::fs::create_dir(&hooks).unwrap(); + std::fs::write( + hooks.join("post-checkout"), + format!("#!/bin/sh\ntouch '{}'\n", sentinel.display()), + ) + .unwrap(); + std::fs::set_permissions( + hooks.join("post-checkout"), + std::fs::Permissions::from_mode(0o755), + ) + .unwrap(); + config + .set_str("core.hooksPath", hooks.to_str().unwrap()) + .unwrap(); + config + .set_str( + "filter.fixture.smudge", + &format!("touch '{}'; cat", sentinel.display()), + ) + .unwrap(); + let sha = fixture.commit_changes(); + let local = fabro_manifest::collect_workflow_versions(Path::new("review"), source).unwrap(); + assert_eq!(local.versions().count(), 2); + for selector in [ + "review", + ".fabro/workflows/review/workflow.fabro", + "missing", + ] { + let checkout = tempfile::tempdir().unwrap(); + let checkout_path = checkout.path().to_path_buf(); + let result = fixture + .git + .collect_checkout( + Fixture::repository(), + selector.into(), + sha.clone(), + checkout, + CancellationToken::new(), + ) + .await; + assert!(!checkout_path.exists()); + if selector == "missing" { + assert!(result.is_err()); + } else { + let remote = result.unwrap(); + assert_eq!(local.root_id(), remote.root_id()); + assert_eq!( + local.versions().map(|(id, _)| id).collect::>(), + remote.versions().map(|(id, _)| id).collect::>() + ); + } + assert!(!sentinel.exists()); + } + let checkout = tempfile::tempdir().unwrap(); + let path = checkout.path().to_path_buf(); + assert!( + fixture + .git + .collect_checkout( + Fixture::repository(), + "review".into(), + "1111111111111111111111111111111111111111".into(), + checkout, + CancellationToken::new() + ) + .await + .is_err() + ); + assert!(!path.exists()); + } + + #[tokio::test] + async fn remote_workflow_rejects_toml_symlink_before_reading_host_content() { + let fixture = Fixture::new(); + let host = tempfile::tempdir().unwrap(); + let fifo = host.path().join("host.toml"); + // Any accidental read blocks: there is deliberately no writer. The + // successful containment error proves rejection before TOML loading. + unistd::mkfifo(&fifo, Mode::S_IRUSR).unwrap(); + let path = fixture + .repo + .workdir() + .unwrap() + .join(".fabro/workflows/review/workflow.toml"); + std::fs::remove_file(&path).unwrap(); + std::os::unix::fs::symlink(&fifo, path).unwrap(); + let sha = fixture.commit_changes(); + let error = fixture + .git + .collect( + Fixture::repository(), + "review".into(), + RemoteWorkflowRevision::Commit(sha), + CancellationToken::new(), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("outside its source root")); + } + + #[tokio::test] + async fn remote_workflow_dropped_waiter_cancels_child_before_checkout_cleanup() { + let (fake, git) = fake_git("printf '%s' $$ > pid; exec /bin/sleep 60"); + let checkout = tempfile::tempdir().unwrap(); + let path = checkout.path().to_path_buf(); + let worker = tokio::spawn(owned(move |cancel| async move { + git.collect_checkout( + "acme/workflows".parse().unwrap(), + "review".into(), + "1111111111111111111111111111111111111111".into(), + checkout, + cancel, + ) + .await + })); + time::timeout(Duration::from_secs(5), async { + while !path.join("pid").exists() { + time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + let pid: i32 = std::fs::read_to_string(path.join("pid")) + .unwrap() + .parse() + .unwrap(); + worker.abort(); + assert!(worker.await.unwrap_err().is_cancelled()); + time::timeout(Duration::from_secs(5), async { + while path.exists() { + time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + assert_eq!(signal::kill(Pid::from_raw(pid), None), Err(Errno::ESRCH)); + drop(fake); + } + + #[tokio::test] + async fn remote_workflow_resolves_exact_default_branches_tags_and_commits() { + let fixture = Fixture::new(); + let object = fixture.repo.revparse_single("HEAD").unwrap(); + fixture + .repo + .branch("topic/slash", object.as_commit().unwrap(), false) + .unwrap(); + fixture + .repo + .tag_lightweight("light", &object, false) + .unwrap(); + let signature = git2::Signature::now("Fixture", "fixture@example.test").unwrap(); + fixture + .repo + .tag("annotated", &object, &signature, "release", false) + .unwrap(); + let cancel = CancellationToken::new(); + for reference in [ + None, + Some("HEAD"), + Some("trunk"), + Some("topic/slash"), + Some("light"), + Some("annotated"), + Some("refs/heads/trunk"), + Some("refs/tags/annotated"), + Some(fixture.sha.as_str()), + ] { + let revision = RemoteWorkflowRevision::parse(reference).unwrap(); + assert_eq!( + fixture + .git + .resolve_revision(&Fixture::repository(), &revision, &cancel) + .await + .unwrap(), + fixture.sha + ); + } + fixture + .repo + .tag_lightweight("trunk", &object, false) + .unwrap(); + assert!( + fixture + .git + .resolve_revision( + &Fixture::repository(), + &RemoteWorkflowRevision::Ref("trunk".into()), + &cancel + ) + .await + .unwrap_err() + .to_string() + .contains("ambiguous") + ); + for reference in ["missing", "refs/tags/missing"] { + assert!( + fixture + .git + .resolve_revision( + &Fixture::repository(), + &RemoteWorkflowRevision::Ref(reference.into()), + &cancel + ) + .await + .is_err() + ); + } + let target = fixture + .git + .resolve_target(Fixture::repository(), None, &cancel) + .await + .unwrap(); + assert_eq!(target.branch, "trunk"); + assert_eq!(target.sha.as_deref(), Some(fixture.sha.as_str())); + assert_eq!( + fixture + .git + .resolve_target(Fixture::repository(), Some("topic/slash".into()), &cancel) + .await + .unwrap() + .branch, + "topic/slash" + ); + for branch in [ + "missing", + "annotated", + "HEAD", + fixture.sha.as_str(), + "refs/heads/trunk", + ] { + assert!( + fixture + .git + .resolve_target(Fixture::repository(), Some(branch.into()), &cancel) + .await + .is_err() + ); + } + // Metadata-only target resolution never creates a checkout directory. + assert_eq!(std::fs::read_dir(fixture.root.path()).unwrap().count(), 2); + } + + #[tokio::test] + async fn remote_workflow_same_bytes_have_same_ids_and_no_lookup_fallback() { + let fixture = Fixture::new(); + let local = fabro_manifest::collect_workflow_versions( + Path::new("review"), + fixture.repo.workdir().unwrap(), + ) + .unwrap(); + let remote = fixture + .git + .collect( + Fixture::repository(), + "review".into(), + RemoteWorkflowRevision::DefaultBranch, + CancellationToken::new(), + ) + .await + .unwrap(); + assert_eq!(local.root_id(), remote.root_id()); + let before: Vec<_> = local.versions().map(|(id, _)| id).collect(); + assert_eq!( + before, + remote.versions().map(|(id, _)| id).collect::>() + ); + assert!( + fixture + .git + .collect( + Fixture::repository(), + "missing".into(), + RemoteWorkflowRevision::DefaultBranch, + CancellationToken::new() + ) + .await + .is_err() + ); + } + + #[tokio::test] + async fn remote_workflow_fetches_observed_commit_after_branch_moves() { + let fixture = Fixture::new(); + let cancel = CancellationToken::new(); + let captured = fixture + .git + .resolve_revision( + &Fixture::repository(), + &RemoteWorkflowRevision::DefaultBranch, + &cancel, + ) + .await + .unwrap(); + let parent = fixture + .repo + .revparse_single("HEAD") + .unwrap() + .peel_to_commit() + .unwrap(); + let signature = git2::Signature::now("Fixture", "fixture@example.test").unwrap(); + let next = fixture + .repo + .commit( + Some("HEAD"), + &signature, + &signature, + "move branch", + &parent.tree().unwrap(), + &[&parent], + ) + .unwrap(); + assert_ne!(next.to_string(), captured); + let checkout = tempfile::tempdir().unwrap(); + fixture + .git + .checkout(&Fixture::repository(), &captured, checkout.path(), &cancel) + .await + .unwrap(); + assert_eq!( + git2::Repository::open(checkout.path()) + .unwrap() + .head() + .unwrap() + .target() + .unwrap() + .to_string(), + captured + ); + assert!( + fixture + .git + .checkout( + &Fixture::repository(), + "1111111111111111111111111111111111111111", + checkout.path(), + &cancel + ) + .await + .is_err() + ); + } + + #[test] + fn remote_workflow_matches_records_exactly_and_rejects_host_symlinks() { + let sha = "1234567890123456789012345678901234567890"; + assert!(resolve_named_ref(&format!("{sha}\trefs/heads/nested/main\n"), "main").is_err()); + let root = tempfile::tempdir().unwrap(); + let host = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink(host.path(), root.path().join("outside")).unwrap(); + assert!(check_source_symlinks(root.path()).is_err()); + } + + fn fake_git(script: &str) -> (tempfile::TempDir, NativeGit) { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join("git"); + std::fs::write(&path, format!("#!/bin/sh\n{script}\n")).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + let mut git = NativeGit::new(root.path().to_path_buf()); + git.environment + .push(("PATH".into(), root.path().to_str().unwrap().into())); + (root, git) + } + + #[tokio::test] + async fn remote_workflow_caps_drains_diagnostics_and_preserves_safe_status() { + let (root, git) = fake_git( + "i=0; while [ $i -lt 9000 ]; do printf 'sentinel-secret-plain-text\\n'; printf 'sentinel-secret-plain-text\\n' >&2; i=$((i+1)); done; exit 42", + ); + let error = git + .command(root.path(), &["fetch"], &CancellationToken::new()) + .await + .unwrap_err(); + assert!( + matches!(error, RemoteWorkflowError::Process { status, .. } if status.code() == Some(42)) + ); + assert!(!format!("{error:?} {error}").contains("sentinel-secret")); + let (root, git) = fake_git( + "i=0; while [ $i -lt 9000 ]; do printf 'metadata-output\\n'; i=$((i+1)); done", + ); + assert!(matches!( + git.command(root.path(), &["ls-remote"], &CancellationToken::new()) + .await + .unwrap_err(), + RemoteWorkflowError::OutputLimit + )); + } + + #[tokio::test] + async fn remote_workflow_timeout_and_cancel_reap_owned_children() { + for timeout in [true, false] { + let (root, mut git) = fake_git("printf '%s' $$ > pid; exec /bin/sleep 60"); + git.timeout = Duration::from_secs(2); + let cancel = CancellationToken::new(); + let trigger = async { + if !timeout { + time::timeout(Duration::from_secs(5), async { + while !root.path().join("pid").exists() { + time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + cancel.cancel(); + } + }; + let (result, ()) = tokio::join!(git.command(root.path(), &["fetch"], &cancel), trigger); + let error = result.unwrap_err(); + assert!(matches!( + error, + RemoteWorkflowError::Timeout | RemoteWorkflowError::Cancelled + )); + let pid: i32 = std::fs::read_to_string(root.path().join("pid")) + .unwrap() + .parse() + .unwrap(); + assert_eq!(signal::kill(Pid::from_raw(pid), None), Err(Errno::ESRCH)); + } + } +} diff --git a/lib/apps/fabro-cli/src/commands/run/resolution.rs b/lib/apps/fabro-cli/src/commands/run/resolution.rs new file mode 100644 index 000000000..57bd458af --- /dev/null +++ b/lib/apps/fabro-cli/src/commands/run/resolution.rs @@ -0,0 +1,358 @@ +use std::path::{Path, PathBuf}; + +use anyhow::{Context as _, anyhow, bail}; +use fabro_manifest::{CollectedWorkflowClosure, ResolvedLocalWorkflowPackage}; +use fabro_types::settings::run::EnvironmentProvider; +use fabro_types::{DirtyStatus, RunTarget}; +use tokio::task; + +use super::remote_workflow::{self, 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. +pub(super) enum ResolvedWorkflow { + Local(ResolvedLocalWorkflowPackage), + Git(CollectedWorkflowClosure), +} + +impl ResolvedWorkflow { + pub(super) fn closure(&self) -> &CollectedWorkflowClosure { + match self { + Self::Local(package) => package.closure(), + Self::Git(closure) => closure, + } + } +} + +pub(super) async fn workflow( + selection: &WorkflowSelection, + cwd: &Path, + user_workflows: Option<&Path>, +) -> anyhow::Result { + match selection { + WorkflowSelection::Local(path) => { + let (path, cwd, user_workflows) = ( + path.clone(), + cwd.to_path_buf(), + user_workflows.map(Path::to_path_buf), + ); + let package = task::spawn_blocking(move || { + fabro_manifest::resolve_local_workflow_package( + &path, + &cwd, + user_workflows.as_deref(), + ) + .map_err(anyhow::Error::new) + }) + .await + .context("local workflow collection task failed")??; + Ok(ResolvedWorkflow::Local(package)) + } + WorkflowSelection::Git { + repository, + selector, + revision, + } => { + let git = NativeGit::new(cwd.to_path_buf()); + let (repository, selector, revision) = + (repository.clone(), selector.clone(), revision.clone()); + let closure = remote_workflow::owned(move |cancel| async move { + git.collect(repository, selector, revision, cancel).await + }) + .await?; + Ok(ResolvedWorkflow::Git(closure)) + } + } +} + +pub(super) async fn target( + selection: &TargetSelection, + provider: EnvironmentProvider, + cwd: &Path, +) -> anyhow::Result<(RunTarget, bool)> { + match selection { + TargetSelection::CurrentDirectory => observe_directory(provider, cwd.to_path_buf()).await, + TargetSelection::Path(path) => { + let path = cwd + .join(path) + .canonicalize() + .context("failed to canonicalize target directory")?; + if !path.is_dir() { + bail!("target path must be a directory"); + } + observe_directory(provider, path).await + } + TargetSelection::Git { repository, branch } => { + if !provider.is_clone_based() { + bail!("Git targets require a clone-enabled Docker or Daytona environment"); + } + let git = NativeGit::new(cwd.to_path_buf()); + let (repository, branch) = (repository.clone(), branch.clone()); + let target = remote_workflow::owned(move |cancel| async move { + git.resolve_target(repository, branch, &cancel).await + }) + .await?; + // Canonical admission retains ownership of provider capabilities. + Ok((RunTarget::Git(target), false)) + } + } +} + +async fn observe_directory( + provider: EnvironmentProvider, + path: PathBuf, +) -> anyhow::Result<(RunTarget, bool)> { + // The existing observer can push/query Git synchronously. Preserve its + // behavior without blocking a Tokio worker or promising a new timeout. + 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: EnvironmentProvider, + canonical_cwd: &Path, +) -> anyhow::Result<(RunTarget, bool)> { + if !provider.is_clone_based() { + 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" + ) +} + +#[cfg(test)] +#[expect( + clippy::disallowed_methods, + reason = "resolver tests construct small local workflow fixtures" +)] +mod tests { + use clap::Parser as _; + + use super::super::selection; + use super::*; + use crate::args::RunArgs; + + fn write_workflow(root: &Path, name: &str) { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("workflow.toml"), + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + std::fs::write( + dir.join("workflow.fabro"), + "digraph Test { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }", + ) + .unwrap(); + } + + #[derive(clap::Parser)] + struct Command { + #[command(flatten)] + args: RunArgs, + } + + #[tokio::test] + async fn run_selection_direct_and_parsed_resolve_identically() { + let caller = tempfile::tempdir().unwrap(); + let root = caller.path().canonicalize().unwrap(); + write_workflow(&root, ".fabro/workflows/review"); + std::fs::create_dir(root.join("target")).unwrap(); + let args = Command::try_parse_from(["cmd", "review", "--target-path", "target"]).unwrap(); + let (parsed_workflow, parsed_target) = selection::parse(&args.args).unwrap(); + let direct_workflow = WorkflowSelection::Local("review".into()); + let direct_target = TargetSelection::Path("target".into()); + assert_eq!( + workflow(&parsed_workflow, &root, None) + .await + .unwrap() + .closure() + .root_id(), + workflow(&direct_workflow, &root, None) + .await + .unwrap() + .closure() + .root_id() + ); + for provider in [ + EnvironmentProvider::Local, + EnvironmentProvider::Docker, + EnvironmentProvider::Daytona, + ] { + assert_eq!( + target(&parsed_target, provider, &root).await.unwrap(), + target(&direct_target, provider, &root).await.unwrap() + ); + } + assert_eq!( + target(&direct_target, EnvironmentProvider::Local, &root) + .await + .unwrap() + .0, + RunTarget::Folder { + path: root.join("target").to_str().unwrap().into(), + } + ); + assert_eq!( + target(&direct_target, EnvironmentProvider::Docker, &root) + .await + .unwrap() + .0, + RunTarget::None {} + ); + assert_eq!( + target( + &TargetSelection::CurrentDirectory, + EnvironmentProvider::Local, + &root + ) + .await + .unwrap() + .0, + RunTarget::Folder { + path: root.to_str().unwrap().into(), + } + ); + assert!( + target( + &TargetSelection::Git { + repository: "acme/app".parse().unwrap(), + branch: None, + }, + EnvironmentProvider::Local, + &root + ) + .await + .unwrap_err() + .to_string() + .contains("Docker or Daytona") + ); + assert!( + target( + &TargetSelection::Path("missing".into()), + EnvironmentProvider::Local, + &root + ) + .await + .is_err() + ); + assert!( + target( + &TargetSelection::Path(".fabro/workflows/review/workflow.toml".into()), + EnvironmentProvider::Local, + &root + ) + .await + .is_err() + ); + } + + #[tokio::test] + async fn run_selection_local_lookup_preserves_precedence_and_explicit_failure() { + let root = tempfile::tempdir().unwrap(); + let user = root.path().join("user"); + let project = root.path().join("project"); + let checkout = root.path().join("project/checkout"); + write_workflow(&user, "review"); + write_workflow(&project, ".fabro/workflows/review"); + write_workflow(&checkout, ".fabro/workflows/review"); + std::fs::write(project.join(".fabro/project.toml"), "_version = 1\n").unwrap(); + git2::Repository::init(&checkout).unwrap(); + let selected = WorkflowSelection::Local("review".into()); + for (cwd, expected_root) in [ + (checkout.as_path(), checkout.as_path()), + (project.as_path(), project.as_path()), + (root.path(), user.as_path()), + ] { + let ResolvedWorkflow::Local(package) = + workflow(&selected, cwd, Some(&user)).await.unwrap() + else { + panic!("local package"); + }; + assert_eq!(package.source_root(), expected_root.canonicalize().unwrap()); + } + assert!( + workflow( + &WorkflowSelection::Local("missing.toml".into()), + &checkout, + Some(&user) + ) + .await + .is_err() + ); + } +} diff --git a/lib/apps/fabro-cli/src/commands/run/selection.rs b/lib/apps/fabro-cli/src/commands/run/selection.rs new file mode 100644 index 000000000..038fde822 --- /dev/null +++ b/lib/apps/fabro-cli/src/commands/run/selection.rs @@ -0,0 +1,288 @@ +//! CLI syntax ends here. Resolvers receive selections and explicit caller +//! context. +use std::path::{Component, Path, PathBuf}; + +use anyhow::{Context as _, bail}; +use fabro_types::{GitHubRepositorySlug, repository}; + +use crate::args::RunArgs; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum WorkflowSelection { + Local(PathBuf), + Git { + repository: GitHubRepositorySlug, + selector: PathBuf, + revision: RemoteWorkflowRevision, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum TargetSelection { + CurrentDirectory, + Path(PathBuf), + Git { + repository: GitHubRepositorySlug, + branch: Option, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum RemoteWorkflowRevision { + DefaultBranch, + Ref(String), + Commit(String), +} + +impl RemoteWorkflowRevision { + pub(super) fn parse(value: Option<&str>) -> anyhow::Result { + match value { + None | Some("HEAD") => Ok(Self::DefaultBranch), + Some(value) => { + if let Some(sha) = repository::normalize_git_commit_sha(value) { + return Ok(Self::Commit(sha)); + } + if !repository::is_valid_github_ref_selector(value) + || (value.starts_with("refs/") + && !value.starts_with("refs/heads/") + && !value.starts_with("refs/tags/")) + { + bail!("workflow ref must be a branch, tag, HEAD, or full 40-hex commit SHA"); + } + Ok(Self::Ref(value.to_owned())) + } + } + } +} + +pub(super) fn validate_remote_selector(path: &Path) -> anyhow::Result<()> { + let value = path + .to_str() + .context("remote workflow selector must be valid UTF-8")?; + if value.is_empty() + || value.contains('\\') + || value.chars().any(char::is_control) + || path + .components() + .any(|part| !matches!(part, Component::Normal(_) | Component::CurDir)) + || value.split('/').any(|part| part == "..") + { + bail!( + "remote workflow must be a name or repository-relative .fabro/.toml file without traversal" + ); + } + match path.extension().and_then(|ext| ext.to_str()) { + Some("toml" | "fabro") => {} + None if path + .file_name() + .is_some_and(|name| path.as_os_str() == name) + && value != "." + && !value.starts_with('-') => {} + _ => bail!( + "remote workflow must be a name or explicit .fabro/.toml file; directories are ambiguous" + ), + } + Ok(()) +} + +pub(super) fn parse(args: &RunArgs) -> anyhow::Result<(WorkflowSelection, TargetSelection)> { + let workflow = args.workflow.as_ref().context("workflow is required")?; + if args.workflow_ref.is_some() && args.workflow_git.is_none() { + bail!("--workflow-ref requires --workflow-git"); + } + if args.target_branch.is_some() && args.target_git.is_none() { + bail!("--target-branch requires --target-git"); + } + if args.target_path.is_some() && args.target_git.is_some() { + bail!("--target-path conflicts with --target-git"); + } + let workflow = match &args.workflow_git { + None => WorkflowSelection::Local(workflow.clone()), + Some(repository) => { + validate_remote_selector(workflow)?; + WorkflowSelection::Git { + repository: repository.clone(), + selector: workflow.clone(), + revision: RemoteWorkflowRevision::parse(args.workflow_ref.as_deref())?, + } + } + }; + let target = match (&args.target_path, &args.target_git) { + (Some(path), _) => TargetSelection::Path(path.clone()), + (_, Some(repository)) => { + if args + .target_branch + .as_deref() + .is_some_and(|branch| !repository::is_valid_git_branch_name(branch)) + { + bail!( + "target branch must be a working branch name, not a tag, SHA, or qualified ref" + ); + } + TargetSelection::Git { + repository: repository.clone(), + branch: args.target_branch.clone(), + } + } + _ => TargetSelection::CurrentDirectory, + }; + Ok((workflow, target)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn run_selection_remote_grammar_is_pure_and_rejects_unsafe_selectors() { + for path in [ + "review", + "./review.toml", + ".fabro/workflows/review/workflow.toml", + "dir/graph.fabro", + ] { + validate_remote_selector(Path::new(path)).unwrap(); + } + for path in [ + "", + ".", + "..", + "/tmp/workflow.toml", + "../review.toml", + "a/../review.toml", + "dir/review", + "dir/", + "a\\review.toml", + ] { + assert!(validate_remote_selector(Path::new(path)).is_err(), "{path}"); + } + for value in [ + "topic/slash", + "refs/heads/release", + "refs/tags/v1", + "HEAD", + "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcd", + ] { + RemoteWorkflowRevision::parse(Some(value)).unwrap(); + } + for value in [ + "--upload-pack=x", + "topic*", + "HEAD~1", + "main..next", + "refs/pull/1/head", + "a.lock", + "main@{1}", + ] { + assert!( + RemoteWorkflowRevision::parse(Some(value)).is_err(), + "{value}" + ); + } + } +} + +#[cfg(test)] +mod adapter_tests { + use clap::Parser as _; + + use super::*; + use crate::args::{Cli, Commands, RunCommands}; + + #[derive(clap::Parser)] + struct Command { + #[command(flatten)] + args: RunArgs, + } + + #[test] + fn run_selection_both_commands_share_the_adapter() { + for command in ["run", "create"] { + let cli = Cli::try_parse_from([ + "fabro", + command, + "review", + "--workflow-git", + "acme/workflows", + "--workflow-ref", + "v1", + "--target-git", + "acme/app", + "--target-branch", + "release", + ]) + .unwrap(); + let Commands::RunCmd(RunCommands::Run(args) | RunCommands::Create(args)) = + *cli.command.unwrap() + else { + panic!("expected shared run arguments"); + }; + assert_eq!( + parse(&args).unwrap(), + ( + WorkflowSelection::Git { + repository: "acme/workflows".parse().unwrap(), + selector: "review".into(), + revision: RemoteWorkflowRevision::Ref("v1".into()), + }, + TargetSelection::Git { + repository: "acme/app".parse().unwrap(), + branch: Some("release".into()), + }, + ) + ); + } + let cli = Cli::try_parse_from(["fabro", "run", "create"]).unwrap(); + assert!( + matches!(*cli.command.unwrap(), Commands::RunCmd(RunCommands::Run(args)) if args.workflow.as_deref() == Some(Path::new("create"))) + ); + } + + #[test] + fn run_selection_adapter_rejects_invalid_inputs_without_acquisition() { + for flags in [ + vec![ + "cmd", + "review", + "--workflow-git", + "https://github.com/acme/workflows", + ], + vec!["cmd", "review", "--target-git", "acme/app/extra"], + vec!["cmd", "../review.toml", "--workflow-git", "acme/workflows"], + vec![ + "cmd", + "/tmp/review.toml", + "--workflow-git", + "acme/workflows", + ], + vec![ + "cmd", + "review", + "--workflow-git", + "acme/workflows", + "--workflow-ref", + "HEAD~1", + ], + vec![ + "cmd", + "review", + "--target-git", + "acme/app", + "--target-branch", + "refs/tags/v1", + ], + vec![ + "cmd", + "review", + "--target-git", + "acme/app", + "--target-branch", + "1234567890123456789012345678901234567890", + ], + ] { + if let Ok(command) = Command::try_parse_from(flags) { + assert!(parse(&command.args).is_err()); + } + } + } +} diff --git a/lib/apps/fabro-cli/tests/it/cmd/create.rs b/lib/apps/fabro-cli/tests/it/cmd/create.rs index 0818a83b6..93716c01e 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/create.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/create.rs @@ -87,22 +87,27 @@ fn help() { success: true exit_code: 0 ----- stdout ----- - Register a local workflow version and create a submitted run + Register a workflow version and create a submitted run Usage: fabro create [OPTIONS] Arguments: - Local workflow name, checkout path, .fabro file, or workflow TOML + Workflow name or path (repository-relative with --workflow-git) Options: --json Output as JSON [env: FABRO_JSON=] --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] -I, --input Override a workflow input value (repeatable, format: KEY=VALUE) - --dry-run Execute with simulated LLM backend --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] - --auto-approve Auto-approve all human gates + --workflow-git Acquire workflow source locally from a GitHub OWNER/REPO using native Git credentials --quiet Suppress non-essential output [env: FABRO_QUIET=] + --workflow-ref Workflow branch, tag, HEAD (default), or full commit SHA; qualify ambiguous names + --target-path Observe this target directory instead of cwd; Folder targets require server filesystem access + --target-git Target GitHub OWNER/REPO; the execution sandbox still needs its own clone credentials + --target-branch Target working branch (default: remote default branch), pinned to its observed commit + --dry-run Simulate execution; workflow source may still be fetched and uploaded + --auto-approve Auto-approve all human gates --goal Override the workflow goal (available as {{ goal }} in prompts) --goal-file Read a per-run goal value from a local file --model Override default LLM model @@ -909,9 +914,9 @@ fn create_rejects_unusable_git_checkouts_instead_of_sending_an_empty_target() { for (working_directory, expected_error) in [ ( detached.path(), - "the caller Git checkout has a detached HEAD", + "the target Git checkout has a detached HEAD", ), - (unborn.path(), "the caller Git checkout has no commits"), + (unborn.path(), "the target Git checkout has no commits"), ] { let output = context .create_cmd() @@ -1633,3 +1638,349 @@ draft = false assert!(pull_request.enabled); assert!(!pull_request.draft); } + +#[test] +fn run_selection_target_path_keeps_caller_workflow_and_goal() { + let context = test_context!(); + let server = MockServer::start(); + let environment = mock_environment(&server, "local", "local"); + 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 caller = tempfile::tempdir().unwrap(); + let target = caller.path().join("target"); + write_workflow(caller.path(), ".fabro/workflows/review", "Caller"); + write_workflow(&target, ".fabro/workflows/review", "Target"); + std::fs::write(caller.path().join("goal.txt"), "Caller goal").unwrap(); + std::fs::write(target.join("goal.txt"), "Target goal").unwrap(); + let expected = fabro_manifest::resolve_local_workflow_package( + std::path::Path::new("review"), + caller.path(), + None, + ) + .unwrap() + .closure() + .root_id(); + let output = context + .create_cmd() + .current_dir(caller.path()) + .args([ + "review", + "--target-path", + "target", + "--goal-file", + "goal.txt", + "--environment", + "local", + "--server", + &format!("{}/api/v1", server.base_url()), + ]) + .output() + .unwrap(); + assert!(output.status.success(), "{}", output_stderr(&output)); + environment.assert(); + versions.assert(); + create.assert(); + let requests = requests.lock().unwrap(); + assert_eq!(requests[0]["workflow_version_id"], expected.to_string()); + assert_eq!( + requests[0]["target"], + json!({"kind": "folder", "path": target.canonicalize().unwrap()}) + ); + assert_eq!(requests[0]["goal"], "Caller goal"); +} + +fn init_remote_fixture(path: &std::path::Path, branch: &str) -> String { + let repo = git2::Repository::init_opts( + path, + git2::RepositoryInitOptions::new().initial_head(branch), + ) + .expect("fixture repository should initialize"); + let mut index = repo.index().expect("fixture index should open"); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .expect("fixture files should stage"); + let tree_id = index.write_tree().expect("fixture tree should write"); + let tree = repo.find_tree(tree_id).expect("fixture tree should exist"); + let signature = git2::Signature::now("Fixture", "fixture@example.test") + .expect("fixture signature should be valid"); + repo.commit(Some("HEAD"), &signature, &signature, "fixture", &tree, &[]) + .expect("fixture commit should succeed") + .to_string() +} + +#[test] +fn run_selection_source_target_cross_product_keeps_workflow_goal_and_target_independent() { + let context = test_context!(); + let server = MockServer::start(); + let local_env = mock_environment(&server, "local", "local"); + let docker_env = mock_environment(&server, "docker", "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 caller = root.path().join("caller"); + let source = root.path().join("source"); + let target = root.path().join("target"); + write_workflow(&caller, ".fabro/workflows/review", "Caller"); + write_workflow(&source, ".fabro/workflows/review", "Remote"); + write_workflow(&target, ".fabro/workflows/review", "Target"); + std::fs::write(caller.join("goal.txt"), "Caller goal").unwrap(); + std::fs::write(target.join("goal.txt"), "Target goal").unwrap(); + init_remote_fixture(&source, "trunk"); + let target_sha = init_remote_fixture(&target, "release"); + let config = root.path().join("gitconfig"); + std::fs::write(&config, format!("[url \"file://{}\"]\n insteadOf = https://github.com/acme/workflows\n[url \"file://{}\"]\n insteadOf = https://github.com/acme/app\n",source.display(),target.display())).unwrap(); + let local_id = fabro_manifest::resolve_local_workflow_package( + std::path::Path::new("review"), + &caller, + None, + ) + .unwrap() + .closure() + .root_id(); + let remote_id = + fabro_manifest::collect_workflow_versions(std::path::Path::new("review"), &source) + .unwrap() + .root_id(); + let file_id = fabro_manifest::resolve_local_workflow_package( + std::path::Path::new(".fabro/workflows/review/workflow.toml"), + &caller, + None, + ) + .unwrap() + .closure() + .root_id(); + assert_ne!(local_id, remote_id); + for source_kind in ["name", "file", "git"] { + for target_kind in ["inferred", "path", "git"] { + let mut command = context.create_cmd(); + command + .current_dir(&caller) + .env("GIT_CONFIG_GLOBAL", &config) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_COUNT", "0"); + command.args([ + "--server", + &format!("{}/api/v1", server.base_url()), + "--goal-file", + "goal.txt", + ]); + command.arg(if source_kind == "file" { + ".fabro/workflows/review/workflow.toml" + } else { + "review" + }); + if source_kind == "git" { + command.args([ + "--workflow-git", + "acme/workflows", + "--workflow-ref", + "trunk", + ]); + } + match target_kind { + "path" => { + command.args(["--target-path", "../target", "--environment", "local"]); + } + "git" => { + command.args([ + "--target-git", + "acme/app", + "--target-branch", + "release", + "--environment", + "docker", + ]); + } + _ => { + command.args(["--environment", "docker"]); + } + } + let output = command.output().unwrap(); + assert!( + output.status.success(), + "{source_kind}/{target_kind}: {}", + output_stderr(&output) + ); + let requests = requests.lock().unwrap(); + let intent = requests.last().unwrap(); + assert_eq!( + intent["workflow_version_id"], + match source_kind { + "git" => remote_id, + "file" => file_id, + _ => local_id, + } + .to_string() + ); + assert_eq!(intent["goal"], "Caller goal"); + assert_eq!(intent["target"], match target_kind { + "path" => json!({"kind":"folder","path":target.canonicalize().unwrap()}), + "git" => + json!({"kind":"git","repo":"acme/app","branch":"release","sha":target_sha}), + _ => json!({"kind":"none"}), + }); + } + } + local_env.assert_calls(3); + docker_env.assert_calls(6); + versions.assert_calls(9); + create.assert_calls(9); +} + +#[test] +fn remote_workflow_run_starts_once_create_leaves_submitted_and_failures_do_not_refetch() { + let context = test_context!(); + let root = tempfile::tempdir().unwrap(); + let source = root.path().join("source"); + write_workflow(&source, ".fabro/workflows/review", "Remote"); + init_remote_fixture(&source, "trunk"); + let config = root.path().join("gitconfig"); + std::fs::write( + &config, + format!( + "[url \"file://{}\"]\n insteadOf = https://github.com/acme/workflows\n", + source.display() + ), + ) + .unwrap(); + for failure in ["none", "run", "upload", "create", "start"] { + let server = MockServer::start(); + let environment = mock_environment(&server, "default", "docker"); + let version = if failure == "upload" { + server.mock(|when, then| { + when.method("POST").path("/api/v1/workflow-versions"); + then.status(422).body("fixture registration rejection"); + }) + } else { + mock_workflow_version_registrations(&server) + }; + let run_id = unique_run_id(); + let create = server.mock(|when, then| { + when.method("POST").path("/api/v1/runs"); + if failure == "create" { + then.status(422).body("fixture create rejection"); + } else { + then.status(201) + .header("content-type", "application/json") + .body(run_status_response(&run_id, "submitted").to_string()); + } + }); + let start = server.mock(|when, then| { + when.method("POST") + .path(format!("/api/v1/runs/{run_id}/start")); + if failure == "start" { + then.status(422).body("fixture start rejection"); + } else { + then.status(200) + .header("content-type", "application/json") + .body(run_status_response(&run_id, "submitted").to_string()); + } + }); + let trace = root.path().join(format!("trace-{failure}")); + let mut command = if failure == "none" { + context.create_cmd() + } else { + context.run_cmd() + }; + command + .current_dir(root.path()) + .env("GIT_CONFIG_GLOBAL", &config) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_COUNT", "0") + .env("GIT_TRACE", &trace) + .args([ + "review", + "--workflow-git", + "acme/workflows", + "--server", + &format!("{}/api/v1", server.base_url()), + "--dry-run", + "--detach", + "--json", + ]); + let output = command.output().unwrap(); + assert_eq!( + output.status.success(), + matches!(failure, "none" | "run"), + "{}", + output_stderr(&output) + ); + environment.assert(); + version.assert(); + create.assert_calls(usize::from(failure != "upload")); + start.assert_calls(usize::from(matches!(failure, "run" | "start"))); + if failure == "none" { + assert_eq!( + serde_json::from_slice::(&output.stdout).unwrap(), + json!({"run_id":run_id}) + ); + } + let trace = std::fs::read_to_string(&trace).unwrap(); + assert_eq!( + trace + .lines() + .filter(|line| line.contains("built-in: git fetch ")) + .count(), + 1, + "source fetched more than once" + ); + } +} + +#[test] +fn remote_workflow_explicit_acquisition_failure_has_no_fallback_or_server_mutations() { + let context = test_context!(); + let server = MockServer::start(); + let environment = mock_environment(&server, "default", "docker"); + let version = mock_workflow_version_registrations(&server); + let create = mock_intent_create(&server, &unique_run_id(), Arc::new(Mutex::new(Vec::new()))); + let root = tempfile::tempdir().unwrap(); + let source = root.path().join("source"); + write_workflow(&source, ".fabro/workflows/other", "Other"); + init_remote_fixture(&source, "trunk"); + write_workflow(root.path(), ".fabro/workflows/review", "Caller"); + write_workflow( + &context.home_dir.join(".fabro/workflows"), + "review", + "Installed", + ); + let config = root.path().join("gitconfig"); + std::fs::write( + &config, + format!( + "[url \"file://{}\"]\n insteadOf = https://github.com/acme/workflows\n", + source.display() + ), + ) + .unwrap(); + for reference in [ + "trunk", + "missing", + "1111111111111111111111111111111111111111", + ] { + let output = context + .create_cmd() + .current_dir(root.path()) + .env("GIT_CONFIG_GLOBAL", &config) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_COUNT", "0") + .args([ + "review", + "--workflow-git", + "acme/workflows", + "--workflow-ref", + reference, + "--server", + &format!("{}/api/v1", server.base_url()), + ]) + .output() + .unwrap(); + assert!(!output.status.success()); + assert!(output.stdout.is_empty()); + } + environment.assert_calls(3); + version.assert_calls(0); + create.assert_calls(0); +} diff --git a/lib/apps/fabro-cli/tests/it/cmd/fabro.rs b/lib/apps/fabro-cli/tests/it/cmd/fabro.rs index 1151e285f..2972f10b8 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/fabro.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/fabro.rs @@ -12,8 +12,8 @@ fn help() { Usage: fabro [OPTIONS] [COMMAND] Commands: - run Register a local workflow version, create a run, and start it - create Register a local workflow version and create a submitted run + run Register a workflow version, create a run, and start it + create Register a workflow version and create a submitted run start Start a created workflow run on the server attach Attach to a running or finished workflow run events View the event log of a workflow run diff --git a/lib/apps/fabro-cli/tests/it/cmd/run.rs b/lib/apps/fabro-cli/tests/it/cmd/run.rs index 5f1752b38..5231de06b 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/run.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/run.rs @@ -119,22 +119,27 @@ fn help() { success: true exit_code: 0 ----- stdout ----- - Register a local workflow version, create a run, and start it + Register a workflow version, create a run, and start it Usage: fabro run [OPTIONS] Arguments: - Local workflow name, checkout path, .fabro file, or workflow TOML + Workflow name or path (repository-relative with --workflow-git) Options: --json Output as JSON [env: FABRO_JSON=] --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] -I, --input Override a workflow input value (repeatable, format: KEY=VALUE) - --dry-run Execute with simulated LLM backend --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] - --auto-approve Auto-approve all human gates + --workflow-git Acquire workflow source locally from a GitHub OWNER/REPO using native Git credentials --quiet Suppress non-essential output [env: FABRO_QUIET=] + --workflow-ref Workflow branch, tag, HEAD (default), or full commit SHA; qualify ambiguous names + --target-path Observe this target directory instead of cwd; Folder targets require server filesystem access + --target-git Target GitHub OWNER/REPO; the execution sandbox still needs its own clone credentials + --target-branch Target working branch (default: remote default branch), pinned to its observed commit + --dry-run Simulate execution; workflow source may still be fetched and uploaded + --auto-approve Auto-approve all human gates --goal Override the workflow goal (available as {{ goal }} in prompts) --goal-file Read a per-run goal value from a local file --model Override default LLM model