From 1443a9a99745feaa0bf34681d764fa37d0902e9a Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Tue, 29 Sep 2026 20:57:09 -0400 Subject: [PATCH] Restore GitHub target checkout before workflow execution --- .config/nextest.toml | 5 +- docs/public/core-concepts/workflows.mdx | 5 +- .../tests/it/scenario/petri_docker.rs | 204 +++++++++++++++++- lib/apps/fabro-server/src/git_checkout.rs | 147 +++++++++++++ lib/apps/fabro-server/src/run_intent.rs | 3 + lib/apps/fabro-server/src/server.rs | 1 + .../fabro-server/src/server/handler/runs.rs | 30 ++- .../fabro-server/src/server/petri_runs.rs | 5 +- .../fabro-server/src/server/run_checkout.rs | 102 +++++++++ lib/apps/fabro-server/src/server/tests.rs | 30 ++- lib/components/fabro-petri/src/check.rs | 9 +- 11 files changed, 531 insertions(+), 10 deletions(-) create mode 100644 lib/apps/fabro-server/src/server/run_checkout.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index 65282c997..0f8108910 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -3,6 +3,10 @@ slow-timeout = { period = "1s", terminate-after = 3 } leak-timeout = "500ms" + [[profile.default.overrides]] + filter = "package(fabro-cli) & test(=scenario::petri_docker::a_git_target_reaches_docker_at_its_pinned_revision_and_checkpoints_edits)" + slow-timeout = { period = "30s", terminate-after = 3 } + [[profile.default.overrides]] filter = "package(fabro-cli)" slow-timeout = { period = "6s", terminate-after = 4 } @@ -80,4 +84,3 @@ leak-timeout = "2s" [[profile.ci.overrides]] filter = "package(fabro-petri)" slow-timeout = { period = "30s", terminate-after = 4 } - diff --git a/docs/public/core-concepts/workflows.mdx b/docs/public/core-concepts/workflows.mdx index af2866ed2..bef1a01ca 100644 --- a/docs/public/core-concepts/workflows.mdx +++ b/docs/public/core-concepts/workflows.mdx @@ -216,7 +216,10 @@ transfer it to a remote server. `--target-branch` requires `--target-repo` 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. +only looks up target metadata. At run creation the server fetches and retains +the selected revision using its GitHub credentials; a failed checkout rejects +creation. The engine populates the execution workspace from that retained +repository before the first command or agent stage. The shorthand `--target acme/app@release/v2` selects the working branch `release/v2`; omit `@BRANCH` to use the remote default branch. Target suffixes accept working branches, while workflow suffixes accept branches, tags, or SHAs. diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs b/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs index 91db4d090..5bda293a6 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs @@ -18,6 +18,7 @@ reason = "these scenarios inspect backend availability and drive the Docker daemon with its CLI" )] +use std::collections::BTreeMap; use std::env; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -27,6 +28,7 @@ use fabro_static::EnvVars; use fabro_test::{ TwinScenario, TwinScenarios, TwinToolCall, expect_reqwest_json, test_context, twin_openai, }; +use fabro_types::{WorkflowPath, WorkflowVersion}; use serde_json::json; use super::petri::{ @@ -48,7 +50,15 @@ async fn docker_server() -> RunningServer { /// `docker_server`, with `settings` appended to the server's settings and /// `secrets` in its vault. async fn docker_server_with(settings: &str, secrets: &[(&str, &str)]) -> RunningServer { - let server = RunningServer::start_with(settings, secrets).await; + docker_server_with_env(settings, secrets, &[]).await +} + +async fn docker_server_with_env( + settings: &str, + secrets: &[(&str, &str)], + env: &[(&str, &str)], +) -> RunningServer { + let server = RunningServer::start_with_env(settings, secrets, env).await; let body = json!({ "id": ENVIRONMENT, "provider": "docker", @@ -75,6 +85,198 @@ async fn docker_server_with(settings: &str, secrets: &[(&str, &str)]) -> Running server } +async fn checkout_version(server: &RunningServer, script: &str, settings: &str) -> String { + let path = |name| WorkflowPath::new(name).expect("the workflow path is valid"); + let dot = format!( + "digraph Checkout {{ start [shape=Mdiamond]; verify [shape=parallelogram, goal_gate=true, script={}]; exit [shape=Msquare]; start -> verify -> exit; }}", + serde_json::to_string(script).expect("the script serializes"), + ); + let version = WorkflowVersion::new( + path("workflow.fabro"), + BTreeMap::from([ + (path("workflow.fabro"), dot), + ( + path("workflow.toml"), + format!("_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n{settings}"), + ), + ]), + BTreeMap::new(), + ) + .expect("the checkout workflow version is valid"); + let response = fabro_test::test_http_client() + .post(format!("{}/api/v1/workflow-versions", server.api_base_url)) + .bearer_auth(TEST_DEV_TOKEN) + .json(&version) + .send() + .await + .expect("the checkout request sends"); + expect_reqwest_json( + response, + fabro_http::StatusCode::CREATED, + "register checkout workflow", + ) + .await["workflow_version_id"] + .as_str() + .expect("the response names its workflow version") + .to_owned() +} + +async fn create_checkout_run( + server: &RunningServer, + version: &str, + target: serde_json::Value, +) -> fabro_http::Response { + fabro_test::test_http_client() + .post(format!("{}/api/v1/runs", server.api_base_url)) + .bearer_auth(TEST_DEV_TOKEN) + .json(&json!({"workflow_version_id": version, "environment_id": ENVIRONMENT, "target": target, "args": {"auto_approve": true}})) + .send().await.expect("the run creation request sends") +} + +async fn start_checkout_run(server: &RunningServer, id: &str) { + let response = fabro_test::test_http_client() + .post(format!("{}/api/v1/runs/{id}/start", server.api_base_url)) + .bearer_auth(TEST_DEV_TOKEN) + .json(&json!({})) + .send() + .await + .expect("the checkout request sends"); + fabro_test::expect_reqwest_status(response, fabro_http::StatusCode::OK, "start checkout run") + .await; +} + +/// The public create/start path must deliver the requested Git revision before +/// stage one. Redirect only this server's Git transport to a local fixture; +/// the run still names a GitHub target and exercises production acquisition. +#[tokio::test(flavor = "multi_thread")] +async fn a_git_target_reaches_docker_at_its_pinned_revision_and_checkpoints_edits() { + if !fabro_test::docker_available() { + return; + } + let context = test_context!(); + let upstream = context.temp_dir.join("upstream"); + std::fs::create_dir(&upstream).unwrap(); + git(&upstream, &["init", "--initial-branch=main"]); + git(&upstream, &["config", "user.name", "Checkout Test"]); + git(&upstream, &["config", "user.email", "checkout@example.com"]); + git(&upstream, &["config", "commit.gpgsign", "false"]); + std::fs::write(upstream.join("README.md"), "existing repository bytes\n").unwrap(); + git(&upstream, &["add", "."]); + git(&upstream, &["commit", "-m", "original"]); + let pin = git(&upstream, &["rev-parse", "HEAD"]); + let blob = git(&upstream, &["hash-object", "README.md"]); + std::fs::write(upstream.join("README.md"), "new branch tip\n").unwrap(); + git(&upstream, &["commit", "-am", "move main"]); + let config = context.temp_dir.join("checkout.gitconfig"); + let remote = "https://github.com/checkout-fixture/source.git"; + std::fs::write( + &config, + format!( + "[url \"file://{}\"]\n\tinsteadOf = {remote}\n", + upstream.display() + ), + ) + .unwrap(); + let mut server = docker_server_with_env( + "", + &[(EnvVars::GITHUB_TOKEN, "ghu_checkout_fixture")], + &[("GIT_CONFIG_GLOBAL", config.to_str().unwrap())], + ) + .await; + let version = checkout_version(&server, &format!( + "set -eu; test $(git hash-object README.md) = {blob}; git merge-base --is-ancestor {pin} HEAD; test $(git remote get-url origin) = {remote}; printf 'edited existing file\\n' >> README.md" + ), "").await; + let response = create_checkout_run( + &server, + &version, + json!({"kind": "git", "repo": "checkout-fixture/source", "branch": "main", "sha": pin}), + ) + .await; + let created = expect_reqwest_json( + response, + fabro_http::StatusCode::CREATED, + "create Git target run", + ) + .await; + let id = created["id"].as_str().unwrap(); + let state = super::petri::run_json(&server, &format!("runs/{id}/state")).await; + assert_eq!(state["spec"]["target"]["sha"], pin); + // Neither a moving remote nor a process restart may change the admitted + // source; starting the run must need only the retained source repository. + server.kill(); + std::fs::remove_dir_all(upstream).unwrap(); + server.launch().await; + start_checkout_run(&server, id).await; + wait_for_success(&server, id).await; + let repository = snapshot_repository(&server, id); + let commits = snapshot_commits(&repository); + let edit = commits + .iter() + .find(|(_, subject, _)| subject.ends_with(": verify (success)")) + .unwrap(); + assert_eq!( + git(&repository, &["show", &format!("{}:README.md", edit.0)]), + "existing repository bytes\nedited existing file" + ); + assert!(commits.iter().any(|(sha, _, _)| sha == &pin)); + cleanup(id); + + let empty = checkout_version(&server, "test -z \"$(git ls-files)\"", "").await; + let response = create_checkout_run(&server, &empty, json!({"kind": "none"})).await; + let created = expect_reqwest_json( + response, + fabro_http::StatusCode::CREATED, + "create empty target run", + ) + .await; + let id = created["id"].as_str().unwrap(); + start_checkout_run(&server, id).await; + wait_for_success(&server, id).await; + cleanup(id); + + let disabled = checkout_version( + &server, + "test -z \"$(git ls-files)\"", + "[run.clone]\nenabled = false\n", + ) + .await; + let response = create_checkout_run( + &server, + &disabled, + json!({"kind": "git", "repo": "checkout-fixture/source", "branch": "main"}), + ) + .await; + let rejected = expect_reqwest_json( + response, + fabro_http::StatusCode::UNPROCESSABLE_ENTITY, + "reject Git target with checkout disabled", + ) + .await; + assert_eq!( + rejected["errors"][0]["code"], + "target_environment_unsupported" + ); + + let response = create_checkout_run( + &server, + &version, + json!({"kind": "git", "repo": "checkout-fixture/source", "branch": "main"}), + ) + .await; + let rejected = expect_reqwest_json( + response, + fabro_http::StatusCode::UNPROCESSABLE_ENTITY, + "reject unavailable Git target", + ) + .await; + assert_eq!(rejected["errors"][0]["code"], "target_checkout_failed"); + assert_eq!( + rejected["errors"][0]["detail"], + "could not check out checkout-fixture/source; check repository access and the requested revision" + ); + server.shutdown(); +} + /// The run's container on the daemon, by Petri's run label: the one the /// run's scope lives in. fn container_of(run_id: &str) -> Option { diff --git a/lib/apps/fabro-server/src/git_checkout.rs b/lib/apps/fabro-server/src/git_checkout.rs index d2856be42..f7ac0c75c 100644 --- a/lib/apps/fabro-server/src/git_checkout.rs +++ b/lib/apps/fabro-server/src/git_checkout.rs @@ -20,6 +20,10 @@ const GIT_REV_PARSE_TIMEOUT: Duration = Duration::from_secs(10); /// Error returned while preparing a checkout from a git source. #[derive(thiserror::Error, Debug)] pub(crate) enum GitCheckoutError { + #[error("failed to initialize the run's source repository")] + Initialize(#[source] GitCommandError), + #[error("failed to check out the run's selected revision")] + Checkout(#[source] GitCommandError), #[error("failed to create Git cache directory {path}")] CacheDirectory { path: PathBuf, @@ -61,6 +65,58 @@ pub(crate) enum GitCheckoutError { }, } +/// Materialize an independent source repository for Petri's checkout step. +/// It lives with the run, so later cache fetches cannot move its HEAD and a +/// delayed start needs neither credentials nor a still-existing remote ref. +/// Authentication is process-local and never written into the repository. +pub(crate) async fn prepare_run_repository( + target: &GitRunTarget, + clone_url: &str, + auth: Option<&GitAuthConfig>, + depth: i32, + directory: &Path, +) -> Result { + for args in [vec!["init", "--quiet"], vec![ + "remote", "add", "origin", clone_url, + ]] { + run_git_plan(GitCommandPlan::new(args, GIT_REV_PARSE_TIMEOUT).current_dir(directory)) + .await + .map_err(GitCheckoutError::Initialize)?; + } + let selector = GitCheckoutSelector::from(target); + let mut args = vec!["fetch".to_string(), "--no-tags".to_string()]; + if depth > 0 { + args.push(format!("--depth={depth}")); + } + // Qualify branches, since a branch and a tag may have the same name. + let revision = match selector { + GitCheckoutSelector::Branch(branch) => format!("refs/heads/{branch}"), + _ => selector.selector().into_owned(), + }; + args.extend(["origin".to_string(), "--".to_string(), revision]); + run_git_plan( + GitCommandPlan::new(args, GIT_FETCH_TIMEOUT) + .current_dir(directory) + .with_auth(clone_url, auth), + ) + .await + .map_err(|source| selector.checkout_error(source))?; + let sha = run_git_plan(build_rev_parse_fetch_head_plan(directory)) + .await + .map_err(|source| GitCheckoutError::ResolveCommit { source })?; + let sha = String::from_utf8_lossy(&sha).trim().to_owned(); + run_git_plan( + GitCommandPlan::new( + ["checkout", "--quiet", "-B", &target.branch, &sha], + GIT_WORKTREE_ADD_TIMEOUT, + ) + .current_dir(directory), + ) + .await + .map_err(GitCheckoutError::Checkout)?; + Ok(sha) +} + #[derive(thiserror::Error, Debug)] pub(crate) enum GitCommandError { #[error("{command} timed out after {timeout_secs}s")] @@ -952,6 +1008,97 @@ mod tests { files } + #[tokio::test] + async fn run_repository_pins_the_selected_revision_and_preserves_history_depth() { + let temp = TempDir::new().unwrap(); + let upstream = temp.path().join("upstream.git"); + let first = seed_upstream(&upstream); + let seed = temp.path().join("seed"); + let git = |directory: &Path, args: &[&str]| { + let output = std::process::Command::new("git") + .current_dir(directory) + .args(args) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_owned() + }; + fs::write(seed.join("README.md"), "moved\n").unwrap(); + git(&seed, &["commit", "-am", "move main"]); + let head = git(&seed, &["rev-parse", "HEAD"]); + git(&seed, &["push", upstream.to_str().unwrap(), "main"]); + let url = format!("file://{}", upstream.display()); + for (sha, tag, depth, expected, count) in [ + ( + Some(first.clone()), + Some("missing".to_string()), + 1, + &first, + "1", + ), + (None, Some("v1".to_string()), 1, &first, "1"), + (None, None, 1, &head, "1"), + (None, None, 0, &head, "2"), + ] { + let directory = TempDir::new().unwrap(); + let target = GitRunTarget { + repo: "acme/repo".to_owned(), + branch: "main".to_owned(), + tag, + sha, + }; + let actual = prepare_run_repository(&target, &url, None, depth, directory.path()) + .await + .unwrap(); + assert_eq!(&actual, expected); + assert_eq!(git(directory.path(), &["rev-parse", "HEAD"]), actual); + assert_eq!( + git(directory.path(), &["symbolic-ref", "--short", "HEAD"]), + "main" + ); + assert_eq!( + git(directory.path(), &["rev-list", "--count", "HEAD"]), + count + ); + assert_eq!(git(directory.path(), &["remote", "get-url", "origin"]), url); + assert!( + !fs::read_to_string(directory.path().join(".git/config")) + .unwrap() + .contains("extraheader") + ); + } + } + + #[tokio::test] + async fn run_repository_refuses_an_unavailable_revision_with_its_cause() { + let temp = TempDir::new().unwrap(); + let upstream = temp.path().join("upstream.git"); + seed_upstream(&upstream); + let directory = TempDir::new().unwrap(); + let target = GitRunTarget { + repo: "acme/repo".to_owned(), + branch: "missing".to_owned(), + tag: None, + sha: None, + }; + let error = prepare_run_repository( + &target, + &format!("file://{}", upstream.display()), + None, + 1, + directory.path(), + ) + .await + .unwrap_err(); + assert!(matches!(error, GitCheckoutError::FetchBranch { .. })); + assert!(Error::source(&error).unwrap().is::()); + assert!(!directory.path().join("README.md").exists()); + } + #[tokio::test] async fn bare_clone_reused_across_calls() { let temp = TempDir::new().unwrap(); diff --git a/lib/apps/fabro-server/src/run_intent.rs b/lib/apps/fabro-server/src/run_intent.rs index 32c6f4eea..bcf863444 100644 --- a/lib/apps/fabro-server/src/run_intent.rs +++ b/lib/apps/fabro-server/src/run_intent.rs @@ -17,6 +17,7 @@ use thiserror::Error; use tokio::{fs, task}; use crate::run_compiler::{RunCompilerError, settings_layer_with_resolved_dockerfiles}; +use crate::server::run_checkout::RunCheckoutError; #[derive(Debug, Error)] pub(crate) enum RunIntentAdmissionError { @@ -35,6 +36,8 @@ pub(crate) enum RunIntentAdmissionError { Environment(#[from] EnvironmentSelectionError), #[error(transparent)] Compiler(#[from] RunCompilerError), + #[error(transparent)] + Checkout(#[from] RunCheckoutError), #[error("run variables could not be loaded")] VariableSnapshot { #[source] diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 2a6046844..d66713f7e 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -169,6 +169,7 @@ mod handler; pub(crate) mod petri_runs; mod pull_request_supervisor; pub(crate) mod resource_sampler; +pub(crate) mod run_checkout; pub(crate) mod run_records; mod session_runtime; pub(crate) mod stream_follower; diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 696c8af1e..4d7767d70 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -65,6 +65,7 @@ use crate::run_intent::{ use crate::run_manifest; use crate::run_selector::{ResolveRunError, resolve_run_by_selector}; use crate::run_title_generation::{self, GenerateTitleInput, TitlePromptInput, WorkflowSummary}; +use crate::server::run_checkout; #[cfg(any(test, feature = "test-support"))] use crate::test_support as server_test_support; @@ -727,6 +728,12 @@ async fn finalize_created_run( explicit_title_supplied: bool, title_generation_target: ManifestPath, ) -> Response { + let (prepared, run_id) = prepared.resolve_run_id(); + let (prepared, source_repository) = + match Box::pin(run_checkout::prepare(&state, prepared, run_id)).await { + Ok(result) => result, + Err(error) => return run_intent_admission_error(error.into()), + }; // Resolve once: we need both the provider IDs (for the run create input // and ask-fabro-readiness) and the LLM client itself (for the spawned // title-generation task). `ready_llm_provider_ids` would otherwise call @@ -749,7 +756,15 @@ async fn finalize_created_run( // Petri compiles the run: the bundle goes to `Runtime::check`, its // diagnostics come back in Fabro's shape, and the admitted graph is what // the run executes. Fabro's own settings resolution ran above. - let pinned = match petri_runs::admit(&state, &prepared, &run_materialization_provider_ids).await + let pinned = match petri_runs::admit( + &state, + &prepared, + &run_materialization_provider_ids, + source_repository + .as_ref() + .map(|directory| directory.path().to_path_buf()), + ) + .await { Ok(admitted) => run_compiler::materialize_admitted(prepared, admitted).await, Err(error) => Err(error), @@ -776,6 +791,9 @@ async fn finalize_created_run( } }; let created_at = created.run_id.created_at(); + if let Some(directory) = source_repository { + let _ = directory.keep(); + } // The run's summary row is the projector's: wait for the pass that // folds the run's first records before reading the run back. state.petri_projector.settle(created.run_id).await; @@ -851,6 +869,11 @@ fn intent_error(status: StatusCode, detail: impl Into, code: &'static st fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response { match &error { + RunIntentAdmissionError::Checkout(_) => { + // The curated message identifies the target without printing Git's + // raw output or credential-provider error bodies. + tracing::warn!(error = %error, "Run target checkout failed"); + } RunIntentAdmissionError::VersionStore { .. } | RunIntentAdmissionError::VariableSnapshot { .. } | RunIntentAdmissionError::WorkerRun { .. } @@ -877,6 +900,11 @@ fn run_intent_admission_error(error: RunIntentAdmissionError) -> Response { } match error { + RunIntentAdmissionError::Checkout(error) => intent_error( + StatusCode::UNPROCESSABLE_ENTITY, + error.to_string(), + "target_checkout_failed", + ), RunIntentAdmissionError::VersionStore { .. } => intent_error( StatusCode::INTERNAL_SERVER_ERROR, "workflow version store operation failed", diff --git a/lib/apps/fabro-server/src/server/petri_runs.rs b/lib/apps/fabro-server/src/server/petri_runs.rs index 0a7e596a1..554ae9367 100644 --- a/lib/apps/fabro-server/src/server/petri_runs.rs +++ b/lib/apps/fabro-server/src/server/petri_runs.rs @@ -33,6 +33,7 @@ //! failed when it cannot. use std::collections::HashMap; +use std::path::PathBuf; use std::sync::Arc; use std::time::Instant; @@ -259,11 +260,13 @@ pub(crate) async fn admit( state: &AppState, prepared: &PreparedRun, eligible: &[ProviderId], + git_repository: Option, ) -> Result { let settings = prepared.settings(); let repository = match prepared.target() { Some(RunTarget::Folder { path }) => Some(path.into()), - Some(RunTarget::Git(_) | RunTarget::None {}) | None => None, + Some(RunTarget::Git(_)) => git_repository, + Some(RunTarget::None {}) | None => None, }; let launch = petri_check::launch( &state.catalog(), diff --git a/lib/apps/fabro-server/src/server/run_checkout.rs b/lib/apps/fabro-server/src/server/run_checkout.rs new file mode 100644 index 000000000..4bdf57f04 --- /dev/null +++ b/lib/apps/fabro-server/src/server/run_checkout.rs @@ -0,0 +1,102 @@ +//! Acquire a Git target on the server; Petri owns delivery to the workspace. +//! +//! The source is an independent repository under the run's scratch directory, +//! pinned before admission. Keep it for delayed starts and recovery, and remove +//! it with the run. A guard removes it if compilation or persistence fails. + +use fabro_config::Storage; +use fabro_types::settings::run::RunMode; +use fabro_types::{DirtyStatus, GitContext, GitCoordinateValidationError, RunId, RunTarget}; +use tempfile::TempDir; +use tokio::fs; + +use crate::git_checkout::{self, GitCheckoutError}; +use crate::run_compiler::PreparedRun; +use crate::server::AppState; + +#[derive(Debug, thiserror::Error)] +pub(crate) enum RunCheckoutError { + #[error("invalid Git target")] + Target(#[source] GitCoordinateValidationError), + #[error("could not resolve GitHub checkout credentials; check the server's GitHub integration")] + Credentials(#[source] anyhow::Error), + #[error("could not create the run's source directory")] + Directory(#[source] std::io::Error), + #[error("could not check out {repo}; check repository access and the requested revision")] + Checkout { + repo: String, + #[source] + source: GitCheckoutError, + }, +} + +/// Acquire and pin the target, returning its cleanup guard. Folder targets +/// already have a repository; empty, disabled-clone and simulated runs need +/// no remote access. The caller retains the guard only after persisting the +/// run. +pub(crate) async fn prepare( + state: &AppState, + prepared: PreparedRun, + run_id: RunId, +) -> Result<(PreparedRun, Option), RunCheckoutError> { + let settings = &prepared.settings().run; + let Some(RunTarget::Git(target)) = prepared.target() else { + return Ok((prepared, None)); + }; + if !settings.clone.enabled || settings.execution.mode == RunMode::DryRun { + return Ok((prepared, None)); + } + let validated = target + .clone() + .validate() + .map_err(RunCheckoutError::Target)?; + let repo = validated.repository(); + let server_settings = state.server_settings(); + let credentials = state + .github_credentials(&server_settings.server.integrations.github) + .await + .map_err(RunCheckoutError::Credentials)?; + let auth = git_checkout::resolve_git_read_auth_config( + credentials.as_ref(), + repo, + &state.github_api_base_url, + state.http_client.clone(), + ) + .await + .map_err(RunCheckoutError::Credentials)?; + let scratch = Storage::new(state.server_storage_dir()).run_scratch(&run_id); + fs::create_dir_all(scratch.root()) + .await + .map_err(RunCheckoutError::Directory)?; + let directory = tempfile::Builder::new() + .prefix("target-") + .tempdir_in(scratch.root()) + .map_err(RunCheckoutError::Directory)?; + let sha = git_checkout::prepare_run_repository( + validated.target(), + &git_checkout::github_clone_url(repo), + auth.as_ref(), + settings.clone.depth, + directory.path(), + ) + .await + .map_err(|source| RunCheckoutError::Checkout { + repo: repo.to_string(), + source, + })?; + let origin_url = repo.https_url(); + let mut target = validated.into_target(); + target.sha = Some(sha); + // Keep the durable target and its Git projection in agreement with the + // source Petri receives, including when the caller selected only a ref. + let git = GitContext { + origin_url, + branch: target.branch.clone(), + sha: target.sha.clone(), + dirty: DirtyStatus::Clean, + }; + Ok(( + prepared.with_target_and_git(RunTarget::Git(target), Some(git)), + Some(directory), + )) +} diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index baaa5351e..12fba4820 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -3402,7 +3402,7 @@ async fn post_runs_run_intent_derives_workflow_slug_from_immutable_entrypoint() } #[tokio::test] -async fn post_runs_run_intent_persists_tagged_exact_git_target_without_starting() { +async fn post_runs_dry_run_persists_tagged_exact_git_target_without_starting() { let state = test_app_state(); let app = crate::test_support::build_test_router(Arc::clone(&state)); let workflow_version_id = store_workflow_version( @@ -3440,6 +3440,7 @@ file = "goal.md" "sha": submitted_sha }, "args": { + "dry_run": true, "inputs": { "ship": true }, "labels": { "team": "platform" } }, @@ -5035,6 +5036,33 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings( build_app_state(config).expect("test app state should build") } +#[tokio::test] +async fn post_runs_git_target_requires_configured_checkout_credentials() { + let state = create_github_token_app_state(None, None); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let version = store_workflow_version(&state, MINIMAL_DOT, None).await; + let response = post_run_intent_response( + &app, + json!({ + "workflow_version_id": version, + "target": {"kind": "git", "repo": "acme/private", "branch": "main"}, + "args": {}, + }), + ) + .await; + let body = fabro_test::expect_axum_json( + response, + StatusCode::UNPROCESSABLE_ENTITY, + "create Git target without configured token", + ) + .await; + assert_eq!(body["errors"][0]["code"], "target_checkout_failed"); + assert_eq!( + body["errors"][0]["detail"], + "could not resolve GitHub checkout credentials; check the server's GitHub integration" + ); +} + #[tokio::test] async fn github_token_strategy_ignores_process_env_token() { let state = create_github_token_app_state_with_env_lookup(None, None, |name| match name { diff --git a/lib/components/fabro-petri/src/check.rs b/lib/components/fabro-petri/src/check.rs index 62c0523d4..4e6b6c6fe 100644 --- a/lib/components/fabro-petri/src/check.rs +++ b/lib/components/fabro-petri/src/check.rs @@ -15,10 +15,11 @@ //! below every file layer, `petri.launch_environment` as the environment //! the run selected over every file layer, `petri.launch_goal` as the goal //! the run resolved over every file layer and the graph's own, and -//! `petri.repository` as the repository the root `start` stage checks out. A -//! caller with no local repository binds `null`, and the run starts from an -//! empty workspace. The server's run variables (`{{ vars.NAME }}`) are bound as -//! compile variables beside them. +//! `petri.repository` as the repository the root `start` stage checks out. +//! Git targets bind the server's pinned source repository; folder targets bind +//! their local path. A caller requesting no checkout binds `null`, and the run +//! starts from an empty workspace. The server's run variables (`{{ vars.NAME +//! }}`) are bound as compile variables beside them. use std::collections::BTreeMap; use std::path::PathBuf;