diff --git a/lib/components/fabro-sandbox/src/clone_source.rs b/lib/components/fabro-sandbox/src/clone_source.rs index 1371de526..f9667f6bc 100644 --- a/lib/components/fabro-sandbox/src/clone_source.rs +++ b/lib/components/fabro-sandbox/src/clone_source.rs @@ -83,44 +83,95 @@ pub(crate) fn exact_repository_init_command(clone_url: &str, checkout_path: &str ) } -/// Fetch a single admitted commit with the same history depth a branch clone -/// gets, so both paths can reach the same number of parent commits. +/// A revision the checkout is pinned to instead of the branch's current HEAD. /// -/// The fetch names the commit directly rather than the branch. No layer proves -/// that the submitted commit belongs to the submitted branch: the branch names -/// the working branch, while a fetchable exact commit is checked out as-is. -#[cfg(any(feature = "docker", test))] -pub(crate) fn exact_fetch_command( - checkout_path: &str, - fetch_source: &str, - commit_sha: &str, - depth: Option, -) -> String { - let depth_arg = depth_argument(depth); - format!( - "{git} -C {} fetch{depth_arg} --no-tags {} -- {}", - sandbox::shell_quote(checkout_path), - sandbox::shell_quote(fetch_source), - sandbox::shell_quote(commit_sha), - git = sandbox::GIT, - ) +/// The working branch names the checkout the run works on; it never constrains +/// which revision is fetched. No layer proves branch/revision ancestry, and an +/// unavailable revision fails without falling back to branch HEAD. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PinnedRevision { + /// An exact commit SHA, already normalized by + /// [`normalize_exact_commit_sha`]. + Commit(String), + /// A bare tag name, fetched as `refs/tags/` so a same-named branch is + /// never consulted. + Tag(String), } -/// Fetch one fully-qualified tag without consulting a same-named branch. +impl PinnedRevision { + /// An exact commit is authoritative over a tag; the tag stays on the run + /// target as durable identity but does not drive the checkout. + pub(crate) fn from_selectors(tag: Option<&str>, commit_sha: Option<&str>) -> Option { + match (commit_sha, tag) { + (Some(sha), _) => Some(Self::Commit(sha.to_string())), + (None, Some(tag)) => Some(Self::Tag(tag.to_string())), + (None, None) => None, + } + } + + /// Human-readable prefix for error messages. + pub(crate) fn label(&self) -> &'static str { + match self { + Self::Commit(_) => "Exact commit checkout", + Self::Tag(_) => "Tag checkout", + } + } + + /// The refspec handed to `git fetch`. + pub(crate) fn fetch_refspec(&self) -> String { + match self { + Self::Commit(sha) => sha.clone(), + Self::Tag(tag) => tag_ref(tag), + } + } + + /// The commit HEAD must resolve to after checkout, when one is known. + pub(crate) fn expected_sha(&self) -> Option<&str> { + match self { + Self::Commit(sha) => Some(sha), + Self::Tag(_) => None, + } + } + + /// Validate the `rev-parse HEAD` output of a pinned checkout and return the + /// resolved commit ID. + pub(crate) fn verify_head(&self, output: &str) -> crate::Result { + let actual_sha = verify_resolved_head(output)?; + if self + .expected_sha() + .is_some_and(|expected| expected != actual_sha) + { + return Err(crate::Error::message( + "Exact checkout HEAD did not match the requested commit", + )); + } + Ok(actual_sha) + } +} + +/// Fully-qualified ref for a bare tag name. +pub(crate) fn tag_ref(tag: &str) -> String { + format!("refs/tags/{tag}") +} + +/// Fetch a single pinned refspec with the same history depth a branch clone +/// gets, so both paths can reach the same number of parent commits. +/// +/// The fetch names the revision directly rather than the branch, and +/// `--no-tags` keeps unrelated tags from being pulled alongside it. #[cfg(any(feature = "docker", test))] -pub(crate) fn tag_fetch_command( +pub(crate) fn pinned_fetch_command( checkout_path: &str, fetch_source: &str, - tag: &str, + refspec: &str, depth: Option, ) -> String { let depth_arg = depth_argument(depth); - let tag_ref = format!("refs/tags/{tag}"); format!( "{git} -C {} fetch{depth_arg} --no-tags {} -- {}", sandbox::shell_quote(checkout_path), sandbox::shell_quote(fetch_source), - sandbox::shell_quote(&tag_ref), + sandbox::shell_quote(refspec), git = sandbox::GIT, ) } @@ -151,7 +202,8 @@ pub(crate) fn exact_branch_checkout_command( ) } -/// Print the current HEAD commit and nothing else, for [`verify_exact_head`]. +/// Print the current HEAD commit and nothing else, for +/// [`PinnedRevision::verify_head`]. pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { format!( "{git} -C {path} rev-parse HEAD", @@ -161,7 +213,8 @@ pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { } /// Check out the admitted branch and print the resulting HEAD in one shell -/// command; stdout is the `rev-parse HEAD` output for [`verify_exact_head`]. +/// command; stdout is the `rev-parse HEAD` output for +/// [`PinnedRevision::verify_head`]. #[cfg(any(feature = "docker", test))] pub(crate) fn exact_checkout_verify_command( checkout_path: &str, @@ -175,29 +228,16 @@ pub(crate) fn exact_checkout_verify_command( ) } -/// Peel the fetched tag to a commit, attach it to the working branch, and -/// print the resolved commit ID. +/// The peeled commit behind whatever `git fetch` just wrote to `FETCH_HEAD`; +/// a commit peels to itself, an annotated tag to the commit it points at. #[cfg(any(feature = "docker", test))] -pub(crate) fn tag_checkout_verify_command(checkout_path: &str, branch: &str) -> String { - exact_checkout_verify_command(checkout_path, branch, "FETCH_HEAD^{commit}") -} - -pub(crate) fn verify_exact_head(output: &str, expected_sha: &str) -> crate::Result<()> { - let actual_sha = output.trim(); - let actual_sha = normalize_exact_commit_sha(actual_sha).map_err(|err| { - crate::Error::context("Exact checkout produced an invalid HEAD commit ID", err) - })?; - if actual_sha != expected_sha { - return Err(crate::Error::message( - "Exact checkout HEAD did not match the requested commit", - )); - } - Ok(()) -} +pub(crate) const FETCH_HEAD_COMMIT: &str = "FETCH_HEAD^{commit}"; +/// Validate that a `rev-parse HEAD` output is a single commit ID and return it +/// normalized. pub(crate) fn verify_resolved_head(output: &str) -> crate::Result { normalize_exact_commit_sha(output.trim()).map_err(|err| { - crate::Error::context("Tag checkout produced an invalid HEAD commit ID", err) + crate::Error::context("Pinned checkout produced an invalid HEAD commit ID", err) }) } @@ -230,27 +270,18 @@ pub(crate) fn decide_clone( clone_tag: Option<&str>, clone_commit_sha: Option<&str>, ) -> crate::Result { - let tag = clone_tag - .map(|tag| { - if tag.trim().is_empty() { - Err(crate::Error::message( - "Tag checkout requires a non-empty tag", - )) - } else { - Ok(tag.to_string()) - } - }) - .transpose()?; + if clone_tag.is_some_and(|tag| tag.trim().is_empty()) { + return Err(crate::Error::message( + "Tag checkout requires a non-empty tag", + )); + } + let tag = clone_tag.map(str::to_string); let commit_sha = clone_commit_sha .map(normalize_exact_commit_sha) .transpose()?; - if tag.is_some() || commit_sha.is_some() { - let selector = if commit_sha.is_some() { - "Exact commit checkout" - } else { - "Tag checkout" - }; + if let Some(pin) = PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()) { + let selector = pin.label(); if skip_clone { return Err(crate::Error::message(format!( "{selector} requires cloning to be enabled" @@ -449,13 +480,26 @@ mod tests { } #[test] - fn tag_fetch_and_checkout_commands_use_the_fully_qualified_tag() { + fn pinned_revision_prefers_exact_commit_and_qualifies_tags() { + let sha = "0123456789abcdef0123456789abcdef01234567"; + assert_eq!(PinnedRevision::from_selectors(None, None), None); + let tag = PinnedRevision::from_selectors(Some("release/v1"), None).unwrap(); + assert_eq!(tag.fetch_refspec(), "refs/tags/release/v1"); + assert_eq!(tag.expected_sha(), None); + let commit = PinnedRevision::from_selectors(Some("release/v1"), Some(sha)).unwrap(); + assert_eq!(commit.fetch_refspec(), sha); + assert_eq!(commit.expected_sha(), Some(sha)); assert_eq!( - tag_fetch_command("/repos/acme/widgets", "origin", "release/v1", Some(10)), + pinned_fetch_command( + "/repos/acme/widgets", + "origin", + &tag.fetch_refspec(), + Some(10) + ), "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets fetch --depth 10 --no-tags origin -- refs/tags/release/v1" ); assert_eq!( - tag_checkout_verify_command("/repos/acme/widgets", "release"), + exact_checkout_verify_command("/repos/acme/widgets", "release", FETCH_HEAD_COMMIT), "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets checkout -B release FETCH_HEAD'^{commit}' && git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets rev-parse HEAD" ); } @@ -539,39 +583,41 @@ mod tests { } #[test] - fn exact_checkout_requires_clone_origin_and_branch() { + fn pinned_checkout_requires_clone_origin_and_branch() { let sha = "0123456789abcdef0123456789abcdef01234567"; - let skip_error = decide_clone( - true, - Some("https://github.com/acme/widgets"), - Some("main"), - None, - Some(sha), - ) - .expect_err("exact checkout with skip-clone should fail"); - assert!(skip_error.to_string().contains("requires cloning")); - - for origin in [None, Some(""), Some(" ")] { - let error = decide_clone(false, origin, Some("main"), None, Some(sha)) - .expect_err("exact checkout without an origin should fail"); - assert!(error.to_string().contains("requires a repository origin")); - } - - for branch in [None, Some(""), Some(" ")] { - let error = decide_clone( - false, + for (tag, commit_sha) in [(None, Some(sha)), (Some("v1"), None)] { + let skip_error = decide_clone( + true, Some("https://github.com/acme/widgets"), - branch, - None, - Some(sha), + Some("main"), + tag, + commit_sha, ) - .expect_err("exact checkout without a branch should fail"); - assert!(error.to_string().contains("requires a repository branch")); + .expect_err("pinned checkout with skip-clone should fail"); + assert!(skip_error.to_string().contains("requires cloning")); + + for origin in [None, Some(""), Some(" ")] { + let error = decide_clone(false, origin, Some("main"), tag, commit_sha) + .expect_err("pinned checkout without an origin should fail"); + assert!(error.to_string().contains("requires a repository origin")); + } + + for branch in [None, Some(""), Some(" ")] { + let error = decide_clone( + false, + Some("https://github.com/acme/widgets"), + branch, + tag, + commit_sha, + ) + .expect_err("pinned checkout without a branch should fail"); + assert!(error.to_string().contains("requires a repository branch")); + } } } #[test] - fn tag_checkout_requires_nonempty_tag_clone_origin_and_branch() { + fn tag_checkout_rejects_empty_tag() { let empty_tag = decide_clone( false, Some("https://github.com/acme/widgets"), @@ -581,34 +627,6 @@ mod tests { ) .expect_err("empty tags should fail"); assert!(empty_tag.to_string().contains("non-empty tag")); - - let skip_error = decide_clone( - true, - Some("https://github.com/acme/widgets"), - Some("main"), - Some("v1"), - None, - ) - .expect_err("tag checkout with skip-clone should fail"); - assert!(skip_error.to_string().contains("requires cloning")); - - for origin in [None, Some(""), Some(" ")] { - let error = decide_clone(false, origin, Some("main"), Some("v1"), None) - .expect_err("tag checkout without an origin should fail"); - assert!(error.to_string().contains("requires a repository origin")); - } - - for branch in [None, Some(""), Some(" ")] { - let error = decide_clone( - false, - Some("https://github.com/acme/widgets"), - branch, - Some("v1"), - None, - ) - .expect_err("tag checkout without a branch should fail"); - assert!(error.to_string().contains("requires a repository branch")); - } } #[test] @@ -618,7 +636,7 @@ mod tests { "https://token@example.com/acme/widgets.git?x=a b", "/repos/acme's widgets", ); - let fetch = exact_fetch_command( + let fetch = pinned_fetch_command( "/repos/acme's widgets", "https://token@example.com/acme/widgets.git?x=a b", sha, @@ -642,9 +660,9 @@ mod tests { } #[test] - fn exact_fetch_omits_depth_for_full_history() { + fn pinned_fetch_omits_depth_for_full_history() { assert_eq!( - exact_fetch_command( + pinned_fetch_command( "/repos/acme/widgets", "origin", "0123456789abcdef0123456789abcdef01234567", @@ -657,15 +675,18 @@ mod tests { #[test] fn exact_checkout_verification_rejects_invalid_or_mismatched_head() { let expected = "0123456789abcdef0123456789abcdef01234567"; - verify_exact_head("0123456789ABCDEF0123456789ABCDEF01234567\n", expected) + let pin = PinnedRevision::Commit(expected.to_string()); + pin.verify_head("0123456789ABCDEF0123456789ABCDEF01234567\n") .expect("uppercase command output should normalize"); - let invalid = verify_exact_head("fatal: not a revision", expected) + let invalid = pin + .verify_head("fatal: not a revision") .expect_err("non-SHA output should fail verification"); assert!(invalid.to_string().contains("invalid HEAD commit ID")); assert!(!invalid.to_string().contains("fatal: not a revision")); - let mismatched = verify_exact_head("1123456789abcdef0123456789abcdef01234567", expected) + let mismatched = pin + .verify_head("1123456789abcdef0123456789abcdef01234567") .expect_err("mismatched SHA should fail verification"); assert!(mismatched.to_string().contains("did not match")); } @@ -719,11 +740,11 @@ mod tests { ); run_shell( temp.path(), - &exact_fetch_command(checkout_path, remote_path, &admitted_sha, Some(10)), + &pinned_fetch_command(checkout_path, remote_path, &admitted_sha, Some(10)), ); let checked_out_sha = run_shell( temp.path(), - &exact_checkout_verify_command(checkout_path, "main", "FETCH_HEAD"), + &exact_checkout_verify_command(checkout_path, "main", FETCH_HEAD_COMMIT), ); assert_eq!(checked_out_sha.trim(), admitted_sha); @@ -794,11 +815,11 @@ mod tests { ); run_shell( temp.path(), - &tag_fetch_command(checkout_path, "origin", tag, Some(10)), + &pinned_fetch_command(checkout_path, "origin", &tag_ref(tag), Some(10)), ); let head = run_shell( temp.path(), - &tag_checkout_verify_command(checkout_path, "release-work"), + &exact_checkout_verify_command(checkout_path, "release-work", FETCH_HEAD_COMMIT), ); assert_eq!(verify_resolved_head(&head).unwrap(), release_sha); @@ -816,7 +837,7 @@ mod tests { ); let output = run_shell_output( temp.path(), - &tag_fetch_command(missing_path, "origin", "main", Some(10)), + &pinned_fetch_command(missing_path, "origin", &tag_ref("main"), Some(10)), ); assert!( !output.status.success(), diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index 63ca8d395..b3cbea53d 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -27,7 +27,7 @@ use tokio::task::JoinHandle; use tokio::{fs, time}; use tokio_util::sync::CancellationToken; -use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason}; +use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason, PinnedRevision}; use crate::git_retry::{self, CredentialContext, GitRetryReason}; use crate::push_credentials::{self, PushCredentialState}; use crate::redact::redact_auth_url; @@ -109,17 +109,13 @@ const DAYTONA_STATE_CHANGE_POLL_INTERVAL: Duration = Duration::from_secs(1); /// leaked by a dead worker. An explicit `0` disables auto-stop entirely. const DEFAULT_AUTO_STOP_INTERVAL_MINUTES: i32 = 120; -fn git_clone_selector( - branch: Option<&str>, - tag: Option<&str>, - commit_sha: Option<&str>, -) -> Option { - if commit_sha.is_some() { - branch.map(str::to_string) - } else if let Some(tag) = tag { - Some(format!("refs/tags/{tag}")) - } else { - branch.map(str::to_string) +/// The ref Daytona's native clone checks out. A pinned tag is fetched by its +/// fully-qualified ref so a same-named branch is never consulted; with an +/// exact commit, `commit_id` drives the checkout and the branch is only a name. +fn git_clone_selector(branch: Option<&str>, pin: Option<&PinnedRevision>) -> Option { + match pin { + Some(PinnedRevision::Tag(tag)) => Some(clone_source::tag_ref(tag)), + Some(PinnedRevision::Commit(_)) | None => branch.map(str::to_string), } } @@ -669,25 +665,29 @@ impl DaytonaSandbox { self.fail_init(init_start, err) } - /// Point the admitted branch at the exact commit and verify the resulting - /// HEAD. + /// Point the admitted branch at the pinned revision and verify the + /// resulting HEAD. /// /// Daytona's native clone honors `commit_id`, but leaves the workspace on - /// whatever ref its own checkout produced. Re-pointing the branch keeps the - /// admitted branch name readable back out of the workspace, matching what - /// the Docker provider produces for the same inputs. - async fn attach_exact_commit_branch( + /// whatever ref its own checkout produced (a detached tag, or the exact + /// commit). Re-pointing the branch keeps the admitted branch name readable + /// back out of the workspace, matching what the Docker provider produces + /// for the same inputs. + async fn attach_pinned_branch( process_svc: &daytona_sdk::ProcessService, checkout_path: &str, branch: &str, - expected_sha: &str, + pin: &PinnedRevision, deadline: time::Instant, ) -> crate::Result<()> { + // An exact commit is named directly; a tag clone is already sitting on + // the tag, so peel whatever HEAD points at to its commit. + let revision = pin.expected_sha().unwrap_or("HEAD^{commit}"); Self::run_required_post_clone_command( process_svc, - &clone_source::exact_branch_checkout_command(checkout_path, branch, expected_sha), + &clone_source::exact_branch_checkout_command(checkout_path, branch, revision), "/", - "git checkout exact commit", + "git checkout pinned revision", deadline, ) .await?; @@ -695,37 +695,11 @@ impl DaytonaSandbox { process_svc, &clone_source::exact_head_revision_command(checkout_path), "/", - "git rev-parse HEAD after exact checkout", + "git rev-parse HEAD after pinned checkout", deadline, ) .await?; - clone_source::verify_exact_head(&head, expected_sha) - } - - /// Attach a tag clone's peeled commit to the requested working branch. - async fn attach_tag_branch( - process_svc: &daytona_sdk::ProcessService, - checkout_path: &str, - branch: &str, - deadline: time::Instant, - ) -> crate::Result<()> { - Self::run_required_post_clone_command( - process_svc, - &clone_source::exact_branch_checkout_command(checkout_path, branch, "HEAD^{commit}"), - "/", - "git checkout tag commit", - deadline, - ) - .await?; - let head = Self::run_required_post_clone_command( - process_svc, - &clone_source::exact_head_revision_command(checkout_path), - "/", - "git rev-parse HEAD after tag checkout", - deadline, - ) - .await?; - clone_source::verify_resolved_head(&head)?; + pin.verify_head(&head)?; Ok(()) } @@ -1695,8 +1669,8 @@ impl Sandbox for DaytonaSandbox { self.fail_init(init_start, err) })?; - let clone_selector = - git_clone_selector(branch.as_deref(), tag.as_deref(), commit_sha.as_deref()); + let pin = PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()); + let clone_selector = git_clone_selector(branch.as_deref(), pin.as_ref()); let clone_plan = git_retry::RetryPlan::clone_default(None); let clone_result = git_retry::retry_git_operation( SandboxProviderKind::Daytona, @@ -1752,42 +1726,22 @@ impl Sandbox for DaytonaSandbox { } }; - if let Some(expected_sha) = commit_sha.as_deref() { + if let Some(pin) = &pin { let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) else { - let err = crate::Error::message( - "Exact commit checkout requires a repository branch", - ); + let err = crate::Error::message(format!( + "{} requires a repository branch", + pin.label() + )); return Err(self .fail_clone_initialization(sandbox, &origin_url, init_start, err) .await); }; - if let Err(err) = Self::attach_exact_commit_branch( - &process_svc, - &layout.primary_repo_path, - branch, - expected_sha, - post_clone_deadline, - ) - .await - { - return Err(self - .fail_clone_initialization(sandbox, &origin_url, init_start, err) - .await); - } - } else if tag.is_some() { - let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) - else { - let err = - crate::Error::message("Tag checkout requires a repository branch"); - return Err(self - .fail_clone_initialization(sandbox, &origin_url, init_start, err) - .await); - }; - if let Err(err) = Self::attach_tag_branch( + if let Err(err) = Self::attach_pinned_branch( &process_svc, &layout.primary_repo_path, branch, + pin, post_clone_deadline, ) .await @@ -3266,17 +3220,19 @@ mod tests { #[test] fn daytona_clone_selector_uses_fully_qualified_tag_unless_sha_is_exact() { + let sha = "0123456789abcdef0123456789abcdef01234567"; + let tag = PinnedRevision::from_selectors(Some("v1.2.3"), None); assert_eq!( - git_clone_selector(Some("release-work"), Some("v1.2.3"), None).as_deref(), + git_clone_selector(Some("release-work"), tag.as_ref()).as_deref(), Some("refs/tags/v1.2.3") ); + let commit = PinnedRevision::from_selectors(Some("v1.2.3"), Some(sha)); assert_eq!( - git_clone_selector( - Some("release-work"), - Some("v1.2.3"), - Some("0123456789abcdef0123456789abcdef01234567"), - ) - .as_deref(), + git_clone_selector(Some("release-work"), commit.as_ref()).as_deref(), + Some("release-work") + ); + assert_eq!( + git_clone_selector(Some("release-work"), None).as_deref(), Some("release-work") ); } diff --git a/lib/components/fabro-sandbox/src/docker.rs b/lib/components/fabro-sandbox/src/docker.rs index e8d350e15..8e3abbce7 100644 --- a/lib/components/fabro-sandbox/src/docker.rs +++ b/lib/components/fabro-sandbox/src/docker.rs @@ -973,13 +973,15 @@ impl DockerSandbox { } let clone_deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; - if let Some(expected_sha) = commit_sha.as_deref() { - // `decide_clone` already rejects an exact commit without a branch; - // re-check here so the checkout can never silently drop the branch - // name callers read back out of the workspace. + if let Some(pin) = + clone_source::PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()) + { + // `decide_clone` already rejects a pinned revision without a + // branch; re-check here so the checkout can never silently drop the + // branch name callers read back out of the workspace. let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) else { let error = - crate::Error::message("Exact commit checkout requires a repository branch"); + crate::Error::message(format!("{} requires a repository branch", pin.label())); return Err(self.report_clone_failure(&origin_url, error)); }; @@ -988,7 +990,7 @@ impl DockerSandbox { if let Err(error) = self .run_exact_local_git_command( &init_command, - "initialize Docker exact repository checkout", + "initialize Docker pinned repository checkout", clone_deadline, auth_url.as_ref(), ) @@ -997,18 +999,18 @@ impl DockerSandbox { return Err(self.report_clone_failure(&origin_url, error)); } - let fetch_command = clone_source::exact_fetch_command( + let fetch_command = clone_source::pinned_fetch_command( &layout.primary_repo_path, "origin", - expected_sha, + &pin.fetch_refspec(), self.config.clone_depth, ); if let Err(failure) = self .retry_git_transfer( &fetch_command, "fetch", - "Docker exact fetch", - "git fetch exact commit", + "Docker pinned fetch", + "git fetch pinned revision", clone_deadline, clone_credential_context, auth_url.as_ref(), @@ -1021,12 +1023,12 @@ impl DockerSandbox { let checkout_command = clone_source::exact_checkout_verify_command( &layout.primary_repo_path, branch, - "FETCH_HEAD", + clone_source::FETCH_HEAD_COMMIT, ); let head = match self .run_exact_local_git_command( &checkout_command, - "git checkout exact commit", + "git checkout pinned revision", clone_deadline, auth_url.as_ref(), ) @@ -1035,65 +1037,7 @@ impl DockerSandbox { Ok(result) => result, Err(error) => return Err(self.report_clone_failure(&origin_url, error)), }; - if let Err(error) = clone_source::verify_exact_head(&head.stdout, expected_sha) { - return Err(self.report_clone_failure(&origin_url, error)); - } - } else if let Some(tag) = tag.as_deref() { - let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) else { - let error = crate::Error::message("Tag checkout requires a repository branch"); - return Err(self.report_clone_failure(&origin_url, error)); - }; - - let init_command = - clone_source::exact_repository_init_command(clone_url, &layout.primary_repo_path); - if let Err(error) = self - .run_exact_local_git_command( - &init_command, - "initialize Docker tag repository checkout", - clone_deadline, - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let fetch_command = clone_source::tag_fetch_command( - &layout.primary_repo_path, - "origin", - tag, - self.config.clone_depth, - ); - if let Err(failure) = self - .retry_git_transfer( - &fetch_command, - "fetch", - "Docker tag fetch", - "git fetch tag", - clone_deadline, - clone_credential_context, - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, failure.error)); - } - - let checkout_command = - clone_source::tag_checkout_verify_command(&layout.primary_repo_path, branch); - let head = match self - .run_exact_local_git_command( - &checkout_command, - "git checkout tag", - clone_deadline, - auth_url.as_ref(), - ) - .await - { - Ok(result) => result, - Err(error) => return Err(self.report_clone_failure(&origin_url, error)), - }; - if let Err(error) = clone_source::verify_resolved_head(&head.stdout) { + if let Err(error) = pin.verify_head(&head.stdout) { return Err(self.report_clone_failure(&origin_url, error)); } } else { diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 008c55703..20edc4800 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -678,22 +678,18 @@ fn clone_source_for_run(record: &RunSpec) -> Result { TargetValidationError::Sha => "persisted Git run target has an invalid SHA", }) })?; - let tag = match &validated.target { - RunTarget::Git(target) => target.tag.clone(), - RunTarget::None {} | RunTarget::Folder { .. } => None, - }; // A target with no Git projection (`none` or `folder`) supplies no clone // source. Folder targets only reach the Local provider, where `skip_clone` // is unused. - Ok(match validated.git { - Some(git) => CloneSourceForRun { + Ok(match (validated.target, validated.git) { + (RunTarget::Git(target), Some(git)) => CloneSourceForRun { origin_url: Some(git.origin_url), - branch: Some(git.branch), - tag, + branch: Some(target.branch), + tag: target.tag, commit_sha: git.sha, skip_clone: false, }, - None => CloneSourceForRun { + _ => CloneSourceForRun { origin_url: None, branch: None, tag: None, diff --git a/lib/foundation/fabro-types/src/run_intent.rs b/lib/foundation/fabro-types/src/run_intent.rs index b4b0e3953..1deb67931 100644 --- a/lib/foundation/fabro-types/src/run_intent.rs +++ b/lib/foundation/fabro-types/src/run_intent.rs @@ -66,6 +66,20 @@ pub struct GitRunTarget { pub sha: Option, } +/// A bare branch or tag name: not `HEAD`, not a `refs/` or `tags/` selector, +/// not a commit SHA, and otherwise a valid GitHub ref selector. +/// +/// The selector grammar is checked on the bare name so its leading-character +/// rules apply to the name itself, not to a prefixed selector that would mask +/// them. +fn is_bare_ref_name(name: &str) -> bool { + name != "HEAD" + && !name.starts_with("tags/") + && !name.starts_with("refs/") + && repository::normalize_git_commit_sha(name).is_none() + && repository::is_valid_github_ref_selector(name) +} + impl RunTarget { /// The wire `kind` discriminator (`git`, `none`, or `folder`), for /// diagnostics. @@ -88,25 +102,10 @@ impl RunTarget { }) => { let slug = GitHubRepositorySlug::try_new(&repo) .ok_or(TargetValidationError::Repository)?; - // The selector grammar is checked on the bare branch name so - // its leading-character rules apply to the branch itself, not - // to a `heads/`-prefixed selector that would mask them. - if branch == "HEAD" - || branch.starts_with("heads/") - || branch.starts_with("tags/") - || branch.starts_with("refs/") - || repository::normalize_git_commit_sha(&branch).is_some() - || !repository::is_valid_github_ref_selector(&branch) - { + if !is_bare_ref_name(&branch) || branch.starts_with("heads/") { return Err(TargetValidationError::Branch); } - if tag.as_deref().is_some_and(|tag| { - tag == "HEAD" - || tag.starts_with("tags/") - || tag.starts_with("refs/") - || repository::normalize_git_commit_sha(tag).is_some() - || !repository::is_valid_github_ref_selector(tag) - }) { + if tag.as_deref().is_some_and(|tag| !is_bare_ref_name(tag)) { return Err(TargetValidationError::Tag); } let sha = sha