diff --git a/lib/components/fabro-sandbox/src/clone_retry.rs b/lib/components/fabro-sandbox/src/clone_retry.rs index f0f3f0572..75dbb7fec 100644 --- a/lib/components/fabro-sandbox/src/clone_retry.rs +++ b/lib/components/fabro-sandbox/src/clone_retry.rs @@ -50,6 +50,13 @@ impl CloneMessageClass { } } +/// A failed clone or fetch attempt: the terminal error plus whether the +/// failure is worth retrying. +pub(crate) struct CloneAttemptFailure { + pub(crate) error: crate::Error, + pub(crate) retry_reason: Option, +} + /// Message fragments that mean the clone failed on infrastructure. /// /// These are safe to retry whether or not the clone was authenticated. diff --git a/lib/components/fabro-sandbox/src/clone_source.rs b/lib/components/fabro-sandbox/src/clone_source.rs index 53effa04a..15c89e137 100644 --- a/lib/components/fabro-sandbox/src/clone_source.rs +++ b/lib/components/fabro-sandbox/src/clone_source.rs @@ -74,10 +74,10 @@ pub(crate) fn repo_symlink_command(layout: &GitHubRepoLayout) -> String { pub(crate) fn exact_repository_init_command(clone_url: &str, checkout_path: &str) -> String { format!( - "git -c maintenance.auto=0 -c gc.auto=0 init -- {} && git -C {} remote add origin {}", - sandbox::shell_quote(checkout_path), - sandbox::shell_quote(checkout_path), - sandbox::shell_quote(clone_url), + "{git} init -- {path} && git -C {path} remote add origin {origin}", + git = sandbox::GIT, + path = sandbox::shell_quote(checkout_path), + origin = sandbox::shell_quote(clone_url), ) } @@ -87,24 +87,21 @@ pub(crate) fn exact_fetch_command( commit_sha: &str, ) -> String { format!( - "git -C {} -c maintenance.auto=0 -c gc.auto=0 fetch --depth 1 --no-tags {} -- {}", + "{git} -C {} fetch --depth 1 --no-tags {} -- {}", sandbox::shell_quote(checkout_path), sandbox::shell_quote(fetch_source), sandbox::shell_quote(commit_sha), + git = sandbox::GIT, ) } -pub(crate) fn exact_checkout_command(checkout_path: &str) -> String { +/// Detach onto the fetched commit and print the resulting HEAD in one shell +/// command; stdout is the `rev-parse HEAD` output for [`verify_exact_head`]. +pub(crate) fn exact_checkout_verify_command(checkout_path: &str) -> String { format!( - "git -C {} -c advice.detachedHead=false checkout --detach FETCH_HEAD", - sandbox::shell_quote(checkout_path), - ) -} - -pub(crate) fn head_revision_command(checkout_path: &str) -> String { - format!( - "git -C {} rev-parse HEAD", - sandbox::shell_quote(checkout_path), + "git -C {path} -c advice.detachedHead=false checkout --detach FETCH_HEAD && git -C {path} \ + rev-parse HEAD", + path = sandbox::shell_quote(checkout_path), ) } @@ -420,8 +417,7 @@ mod tests { "https://token@example.com/acme/widgets.git?x=a b", sha, ); - let checkout = exact_checkout_command("/repos/acme's widgets"); - let verify = head_revision_command("/repos/acme's widgets"); + let checkout = exact_checkout_verify_command("/repos/acme's widgets"); assert_eq!( init, @@ -429,14 +425,13 @@ mod tests { ); assert_eq!( fetch, - "git -C \"/repos/acme's widgets\" -c maintenance.auto=0 -c gc.auto=0 fetch --depth 1 --no-tags 'https://token@example.com/acme/widgets.git?x=a b' -- 0123456789abcdef0123456789abcdef01234567" + "git -c maintenance.auto=0 -c gc.auto=0 -C \"/repos/acme's widgets\" fetch --depth 1 --no-tags 'https://token@example.com/acme/widgets.git?x=a b' -- 0123456789abcdef0123456789abcdef01234567" ); assert_eq!( checkout, - "git -C \"/repos/acme's widgets\" -c advice.detachedHead=false checkout --detach FETCH_HEAD" + "git -C \"/repos/acme's widgets\" -c advice.detachedHead=false checkout --detach FETCH_HEAD && git -C \"/repos/acme's widgets\" rev-parse HEAD" ); - assert_eq!(verify, "git -C \"/repos/acme's widgets\" rev-parse HEAD"); - for command in [&init, &fetch, &checkout, &verify] { + for command in [&init, &fetch, &checkout] { assert!(!command.contains("moving-branch")); } } @@ -504,8 +499,7 @@ mod tests { temp.path(), &exact_fetch_command(checkout_path, remote_path, &admitted_sha), ); - run_shell(temp.path(), &exact_checkout_command(checkout_path)); - let checked_out_sha = run_shell(temp.path(), &head_revision_command(checkout_path)); + let checked_out_sha = run_shell(temp.path(), &exact_checkout_verify_command(checkout_path)); assert_eq!(checked_out_sha.trim(), admitted_sha); assert_eq!( diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index 617dc5c15..bbc4cfdef 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -69,19 +69,6 @@ const DAYTONA_START_TIMEOUT: Duration = Duration::from_mins(1); /// cancellation/timeout paths indefinitely. const DAYTONA_CLEANUP_TIMEOUT: Duration = Duration::from_secs(10); -struct DaytonaExactCheckoutFailure { - error: crate::Error, - retry_reason: Option, -} - -fn daytona_clone_branch(requested_branch: Option<&str>, exact_checkout: bool) -> Option { - if exact_checkout { - None - } else { - requested_branch.map(str::to_string) - } -} - fn daytona_process_exec_result(exit_code: i32, output: String) -> ExecResult { let (stdout, stderr) = if exit_code == 0 { (output, String::new()) @@ -97,14 +84,6 @@ fn daytona_process_exec_result(exit_code: i32, output: String) -> ExecResult { } } -fn daytona_exact_exec_error( - result: ExecResult, - label: &'static str, - auth_url: Option<&fabro_redact::DisplaySafeUrl>, -) -> crate::Error { - result.into_exec_error_with_redactor(label, |output| redact_auth_url(output, auth_url)) -} - /// Permissions a Daytona API key needs for Fabro's snapshot and sandbox flow. pub const REQUIRED_DAYTONA_PERMISSIONS: &[Permissions] = &[ Permissions::WriteColonSnapshots, @@ -591,10 +570,106 @@ impl DaytonaSandbox { if result.is_success() { Ok(result) } else { - Err(daytona_exact_exec_error(result, label, auth_url)) + Err(result + .into_exec_error_with_redactor(label, |output| redact_auth_url(output, auth_url))) } } + /// Materialize the exact admitted commit through the Daytona process + /// service: init an empty repository, shallow-fetch the commit with clone + /// retry semantics, detach onto it, and verify the resulting HEAD. + async fn checkout_exact_commit( + process_svc: &daytona_sdk::ProcessService, + origin_url: &str, + layout: &clone_source::GitHubRepoLayout, + password: Option<&str>, + expected_sha: &str, + token_was_freshly_minted: bool, + ) -> crate::Result<()> { + let auth_url = match password { + Some(token) => Some(fabro_github::embed_token_in_url(origin_url, token).map_err( + |error| crate::Error::Context { + message: + "Failed to build authenticated URL for Daytona exact checkout".to_string(), + source: error.into_boxed_dyn_error(), + }, + )?), + None => None, + }; + let clone_url = auth_url + .as_ref() + .map_or(origin_url, |url| url.as_raw_url().as_str()); + + let init_command = + clone_source::exact_repository_init_command(clone_url, &layout.primary_repo_path); + Self::run_exact_checkout_command( + process_svc, + &init_command, + "/", + "initialize Daytona exact repository checkout", + auth_url.as_ref(), + ) + .await?; + + let fetch_command = + clone_source::exact_fetch_command(&layout.primary_repo_path, "origin", expected_sha); + clone_retry::retry_clone( + SandboxProviderKind::Daytona, + None, + |_attempt| { + let command = fetch_command.as_str(); + let auth_url = auth_url.as_ref(); + async move { + let response = process_svc + .execute_command( + &wrap_bash_command(command), + daytona_sdk::ExecuteCommandOptions { + cwd: Some("/".to_string()), + ..Default::default() + }, + ) + .await + .map_err(|error| clone_retry::CloneAttemptFailure { + retry_reason: classify_clone_failure(&error, token_was_freshly_minted), + error: Self::daytona_process_transport_error( + "Daytona exact fetch transport failed", + &error, + ), + })?; + if response.exit_code == 0 { + return Ok(()); + } + let retry_reason = + clone_retry::classify_message(&response.result, token_was_freshly_minted) + .retry_reason(); + let result = daytona_process_exec_result(response.exit_code, response.result); + Err(clone_retry::CloneAttemptFailure { + retry_reason, + error: result.into_exec_error_with_redactor( + "git fetch exact commit in Daytona sandbox", + |output| redact_auth_url(output, auth_url), + ), + }) + } + }, + |failure: &clone_retry::CloneAttemptFailure| failure.retry_reason, + ) + .await + .map_err(|failure| failure.error)?; + + let checkout_command = + clone_source::exact_checkout_verify_command(&layout.primary_repo_path); + let head = Self::run_exact_checkout_command( + process_svc, + &checkout_command, + "/", + "git checkout exact commit in Daytona sandbox", + auth_url.as_ref(), + ) + .await?; + clone_source::verify_exact_head(&head.stdout, expected_sha) + } + fn fail_init(&self, init_start: Instant, err: crate::Error) -> crate::Error { let duration_ms = u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); self.emit(SandboxEvent::InitializeFailed { @@ -1241,312 +1316,170 @@ impl Sandbox for DaytonaSandbox { self.fail_init(init_start, err) })?; - let git_svc = sandbox.git().await.map_err(|e| { - let err = crate::Error::context("Failed to get Daytona git service", e); - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url.clone(), - error: err.to_string(), - causes: err.causes(), - }); + let process_svc = sandbox.process().await.map_err(|e| { + let err = crate::Error::context("Failed to get Daytona process service", e); + let err = self.report_clone_failure(&origin_url, err); self.fail_init(init_start, err) })?; - let clone_result = clone_retry::retry_clone( - SandboxProviderKind::Daytona, - None, - |_attempt| { - let git_svc = &git_svc; - let origin = origin_url.as_str(); - let target = layout.primary_repo_path.as_str(); - let options = daytona_sdk::GitCloneOptions { - branch: daytona_clone_branch(branch.as_deref(), commit_sha.is_some()), - username: username.clone(), - password: password.clone(), - ..Default::default() - }; - async move { git_svc.clone(origin, target, options).await } - }, - |err: &DaytonaError| classify_clone_failure(err, token_was_freshly_minted), - ) - .await; + if let Some(expected_sha) = commit_sha.as_deref() { + Self::checkout_exact_commit( + &process_svc, + &origin_url, + &layout, + password.as_deref(), + expected_sha, + token_was_freshly_minted, + ) + .await + .map_err(|err| { + let err = self.report_clone_failure(&origin_url, err); + self.fail_init(init_start, err) + })?; + } else { + let git_svc = sandbox.git().await.map_err(|e| { + let err = crate::Error::context("Failed to get Daytona git service", e); + let err = self.report_clone_failure(&origin_url, err); + self.fail_init(init_start, err) + })?; - match clone_result { - Ok(()) => { - let process_svc = sandbox.process().await.map_err(|e| { - let err = - crate::Error::context("Failed to get Daytona process service", e); - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url.clone(), - error: err.to_string(), - causes: err.causes(), - }); - self.fail_init(init_start, err) - })?; - - if let Some(expected_sha) = commit_sha.as_deref() { - let auth_url = match password.as_deref() { - Some(token) => { - match fabro_github::embed_token_in_url(&origin_url, token) { - Ok(url) => Some(url), - Err(error) => { - let error = crate::Error::Context { - message: "Failed to build authenticated URL for \ - Daytona exact checkout" - .to_string(), - source: error.into_boxed_dyn_error(), - }; - let error = - self.report_clone_failure(&origin_url, error); - return Err(self.fail_init(init_start, error)); - } - } - } - None => None, + let clone_result = clone_retry::retry_clone( + SandboxProviderKind::Daytona, + None, + |_attempt| { + let git_svc = &git_svc; + let origin = origin_url.as_str(); + let target = layout.primary_repo_path.as_str(); + let options = daytona_sdk::GitCloneOptions { + branch: branch.clone(), + username: username.clone(), + password: password.clone(), + ..Default::default() }; - let fetch_source = auth_url - .as_ref() - .map_or(origin_url.as_str(), |url| url.as_raw_url().as_str()); - let fetch_command = clone_source::exact_fetch_command( - &layout.primary_repo_path, - fetch_source, - expected_sha, + async move { git_svc.clone(origin, target, options).await } + }, + |err: &DaytonaError| classify_clone_failure(err, token_was_freshly_minted), + ) + .await; + + match clone_result { + Ok(()) => {} + Err(e) if self.github_app.is_none() => { + let err = crate::Error::context( + "Git clone failed. If this is a private repository, \ + configure a GitHub App with `fabro install` and install it \ + for your organization.", + e, ); - let fetch_result = clone_retry::retry_clone( - SandboxProviderKind::Daytona, - None, - |_attempt| { - let command = fetch_command.as_str(); - let process_svc = &process_svc; - let auth_url = auth_url.as_ref(); - async move { - let response = process_svc - .execute_command( - &wrap_bash_command(command), - daytona_sdk::ExecuteCommandOptions { - cwd: Some("/".to_string()), - ..Default::default() - }, - ) - .await - .map_err(|error| DaytonaExactCheckoutFailure { - retry_reason: classify_clone_failure( - &error, - token_was_freshly_minted, - ), - error: Self::daytona_process_transport_error( - "Daytona exact fetch transport failed", - &error, - ), - })?; - if response.exit_code == 0 { - return Ok(()); - } - let retry_reason = clone_retry::classify_message( - &response.result, - token_was_freshly_minted, - ) - .retry_reason(); - let result = daytona_process_exec_result( - response.exit_code, - response.result, - ); - Err(DaytonaExactCheckoutFailure { - retry_reason, - error: daytona_exact_exec_error( - result, - "git fetch exact commit in Daytona sandbox", - auth_url, - ), - }) - } - }, - |failure: &DaytonaExactCheckoutFailure| failure.retry_reason, - ) - .await; - if let Err(failure) = fetch_result { - let error = self.report_clone_failure(&origin_url, failure.error); - return Err(self.fail_init(init_start, error)); - } - - let checkout_command = - clone_source::exact_checkout_command(&layout.primary_repo_path); - if let Err(error) = Self::run_exact_checkout_command( - &process_svc, - &checkout_command, - "/", - "git checkout exact commit in Daytona sandbox", - auth_url.as_ref(), - ) - .await - { - let error = self.report_clone_failure(&origin_url, error); - return Err(self.fail_init(init_start, error)); - } - - let head_command = - clone_source::head_revision_command(&layout.primary_repo_path); - let head = match Self::run_exact_checkout_command( - &process_svc, - &head_command, - "/", - "verify Daytona exact checkout HEAD", - auth_url.as_ref(), - ) - .await - { - Ok(result) => result, - Err(error) => { - let error = self.report_clone_failure(&origin_url, error); - return Err(self.fail_init(init_start, error)); - } - }; - if let Err(error) = - clone_source::verify_exact_head(&head.stdout, expected_sha) - { - let error = self.report_clone_failure(&origin_url, error); - return Err(self.fail_init(init_start, error)); - } - } - - let symlink_cmd = clone_source::repo_symlink_command(&layout); - let symlink_result = process_svc - .execute_command( - &wrap_bash_command(&symlink_cmd), - daytona_sdk::ExecuteCommandOptions { - cwd: Some("/".to_string()), - ..Default::default() - }, - ) - .await - .map_err(|e| { - let err = crate::Error::context( - "Failed to create Daytona workspace repo symlink", - e, - ); - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url.clone(), - error: err.to_string(), - causes: err.causes(), - }); - self.fail_init(init_start, err) - })?; - if symlink_result.exit_code != 0 { - let err = crate::Error::exec( - "create Daytona workspace repo symlink", - ExecResult { - stdout: symlink_result.result.clone(), - stderr: String::new(), - exit_code: Some(symlink_result.exit_code), - termination: CommandTermination::Exited, - duration_ms: 0, - }, - ); - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url.clone(), - error: err.to_string(), - causes: err.causes(), - }); + let err = self.report_clone_failure(&origin_url, err); return Err(self.fail_init(init_start, err)); } + Err(e) => { + let err = crate::Error::context( + "Failed to clone repo into Daytona sandbox", + e, + ); + let err = self.report_clone_failure(&origin_url, err); + return Err(self.fail_init(init_start, err)); + } + } + } - let clone_duration = - u64::try_from(clone_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::GitCloneCompleted { - url: origin_url.clone(), - duration_ms: clone_duration, + let symlink_cmd = clone_source::repo_symlink_command(&layout); + let symlink_result = process_svc + .execute_command( + &wrap_bash_command(&symlink_cmd), + daytona_sdk::ExecuteCommandOptions { + cwd: Some("/".to_string()), + ..Default::default() + }, + ) + .await + .map_err(|e| { + let err = crate::Error::context( + "Failed to create Daytona workspace repo symlink", + e, + ); + let err = self.report_clone_failure(&origin_url, err); + self.fail_init(init_start, err) + })?; + if symlink_result.exit_code != 0 { + let err = + crate::Error::exec("create Daytona workspace repo symlink", ExecResult { + stdout: symlink_result.result.clone(), + stderr: String::new(), + exit_code: Some(symlink_result.exit_code), + termination: CommandTermination::Exited, + duration_ms: 0, }); + let err = self.report_clone_failure(&origin_url, err); + return Err(self.fail_init(init_start, err)); + } - let _ = self.repo_cloned.set(true); - let _ = self.origin_url.set(origin_url.clone()); - self.set_working_directory(layout.execution_directory.clone()) - .map_err(|err| self.fail_init(init_start, err))?; - if let Some(token) = password.as_deref() { - match fabro_github::embed_token_in_url(&origin_url, token) { - Ok(auth_url) => { - let cmd = format!( - "git -c maintenance.auto=0 remote set-url origin {}", - shell_quote(auth_url.as_raw_url().as_str()), + let clone_duration = + u64::try_from(clone_start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::GitCloneCompleted { + url: origin_url.clone(), + duration_ms: clone_duration, + }); + + let _ = self.repo_cloned.set(true); + let _ = self.origin_url.set(origin_url.clone()); + self.set_working_directory(layout.execution_directory.clone()) + .map_err(|err| self.fail_init(init_start, err))?; + if let Some(token) = password.as_deref() { + match fabro_github::embed_token_in_url(&origin_url, token) { + Ok(auth_url) => { + let cmd = format!( + "git -c maintenance.auto=0 remote set-url origin {}", + shell_quote(auth_url.as_raw_url().as_str()), + ); + let opts = daytona_sdk::ExecuteCommandOptions { + cwd: Some(layout.execution_directory.clone()), + ..Default::default() + }; + let wrapped = wrap_bash_command(&cmd); + match process_svc.execute_command(&wrapped, opts).await { + Ok(r) if r.exit_code != 0 => { + let err = crate::Error::exec( + "git remote set-url origin (Daytona post-clone)", + ExecResult { + stdout: String::new(), + stderr: redact_auth_url( + &r.result, + Some(&auth_url), + ), + exit_code: Some(r.exit_code), + termination: CommandTermination::Exited, + duration_ms: 0, + }, ); - let opts = daytona_sdk::ExecuteCommandOptions { - cwd: Some(layout.execution_directory.clone()), - ..Default::default() - }; - let wrapped = wrap_bash_command(&cmd); - match process_svc.execute_command(&wrapped, opts).await { - Ok(r) if r.exit_code != 0 => { - let err = crate::Error::exec( - "git remote set-url origin (Daytona post-clone)", - ExecResult { - stdout: String::new(), - stderr: redact_auth_url( - &r.result, - Some(&auth_url), - ), - exit_code: Some(r.exit_code), - termination: CommandTermination::Exited, - duration_ms: 0, - }, - ); - tracing::warn!( - error = %crate::display_for_log(&err), - "Failed to set Daytona sandbox push credentials \ - on origin — subsequent git push from this \ - sandbox will fail" - ); - } - Ok(_) => {} - Err(_) => { - tracing::warn!( - error_class = "daytona_set_url_exec_failed", - "Daytona exec failed while setting push credentials \ - on origin — subsequent git push from this \ - sandbox will fail" - ); - } - } - } - Err(e) => { tracing::warn!( - origin = %origin_url, - error = %e, - "Failed to build authenticated origin URL — \ - subsequent git push from this sandbox will fail" + error = %crate::display_for_log(&err), + "Failed to set Daytona sandbox push credentials \ + on origin — subsequent git push from this \ + sandbox will fail" + ); + } + Ok(_) => {} + Err(_) => { + tracing::warn!( + error_class = "daytona_set_url_exec_failed", + "Daytona exec failed while setting push credentials \ + on origin — subsequent git push from this \ + sandbox will fail" ); } } } - } - Err(e) if commit_sha.is_some() => { - let err = Self::daytona_process_transport_error( - "Daytona SDK clone failed while preparing exact checkout", - &e, - ); - let err = self.report_clone_failure(&origin_url, err); - return Err(self.fail_init(init_start, err)); - } - Err(e) if self.github_app.is_none() => { - let err = crate::Error::context( - "Git clone failed. If this is a private repository, \ - configure a GitHub App with `fabro install` and install it \ - for your organization.", - e, - ); - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url, - error: err.to_string(), - causes: err.causes(), - }); - return Err(self.fail_init(init_start, err)); - } - Err(e) => { - let err = - crate::Error::context("Failed to clone repo into Daytona sandbox", e); - self.emit(SandboxEvent::GitCloneFailed { - url: origin_url, - error: err.to_string(), - causes: err.causes(), - }); - return Err(self.fail_init(init_start, err)); + Err(e) => { + tracing::warn!( + origin = %origin_url, + error = %e, + "Failed to build authenticated origin URL — \ + subsequent git push from this sandbox will fail" + ); + } } } } @@ -2929,14 +2862,6 @@ mod tests { use super::*; use crate::sandbox::BASH_PROBE_MARKER; - #[test] - fn exact_checkout_omits_requested_branch_from_daytona_clone() { - let branch = Some("moving-branch".to_string()); - - assert_eq!(daytona_clone_branch(branch.as_deref(), false), branch); - assert_eq!(daytona_clone_branch(branch.as_deref(), true), None); - } - #[tokio::test] async fn invalid_exact_sha_fails_before_daytona_client_construction() { let error = DaytonaSandbox::new( @@ -2968,11 +2893,10 @@ mod tests { auth_url.as_raw_url() ), ); - let error = daytona_exact_exec_error( - result, - "git fetch exact commit in Daytona sandbox", - Some(&auth_url), - ); + let error = result + .into_exec_error_with_redactor("git fetch exact commit in Daytona sandbox", |output| { + redact_auth_url(output, Some(&auth_url)) + }); let causes = collect_chain(&error); assert!( diff --git a/lib/components/fabro-sandbox/src/docker.rs b/lib/components/fabro-sandbox/src/docker.rs index 5c8c03d97..c1acdf1ad 100644 --- a/lib/components/fabro-sandbox/src/docker.rs +++ b/lib/components/fabro-sandbox/src/docker.rs @@ -56,11 +56,6 @@ const EXEC_TERM_GRACE_SECONDS: &str = "0.02"; #[cfg(not(test))] const EXEC_TERM_GRACE_SECONDS: &str = "0.2"; -struct DockerCloneFailure { - error: crate::Error, - retry_reason: Option, -} - fn env_entry_name(entry: &str) -> &str { entry.split_once('=').map_or(entry, |(name, _)| name) } @@ -722,14 +717,15 @@ impl DockerSandbox { Ok(()) } - /// Preserve a failed `git clone` result while masking the auth URL. + /// Preserve a failed git transfer result while masking the auth URL. fn clone_failure_error( &self, result: ExecResult, + label: &'static str, auth_url: Option<&fabro_redact::DisplaySafeUrl>, ) -> crate::Error { - let error = result - .into_exec_error_with_redactor("git clone", |output| redact_auth_url(output, auth_url)); + let error = + result.into_exec_error_with_redactor(label, |output| redact_auth_url(output, auth_url)); let message = if self.github_app.is_none() { "Git clone failed. If this is a private repository, configure a GitHub App with \ `fabro install` and install it for your organization." @@ -748,23 +744,6 @@ impl DockerSandbox { err } - fn exact_checkout_exec_error( - &self, - result: ExecResult, - label: &'static str, - auth_url: Option<&fabro_redact::DisplaySafeUrl>, - ) -> crate::Error { - let source = - result.into_exec_error_with_redactor(label, |output| redact_auth_url(output, auth_url)); - let message = if self.github_app.is_none() { - "Exact Git checkout failed. If this is a private repository, configure a GitHub App \ - with `fabro install` and install it for your organization." - } else { - "Failed to check out exact commit into Docker sandbox" - }; - crate::Error::context(message, source) - } - async fn run_exact_checkout_command( &self, command: &str, @@ -778,15 +757,69 @@ impl DockerSandbox { if result.is_success() { Ok(result) } else { - Err(self.exact_checkout_exec_error(result, label, auth_url)) + Err(self.clone_failure_error(result, label, auth_url)) } } - async fn checkout_exact_github_commit( + /// Run a network git command inside the container with clone retry + /// semantics under the shared clone deadline. + async fn retry_git_transfer( + &self, + command: &str, + label: &'static str, + exec_label: &'static str, + clone_deadline: time::Instant, + token_was_freshly_minted: bool, + auth_url: Option<&fabro_redact::DisplaySafeUrl>, + ) -> Result<(), clone_retry::CloneAttemptFailure> { + clone_retry::retry_clone( + SandboxProviderKind::Docker, + Some(clone_deadline), + |_attempt| async move { + let remaining = clone_deadline.saturating_duration_since(time::Instant::now()); + let timeout_ms = u64::try_from(remaining.as_millis()).unwrap_or(u64::MAX); + if timeout_ms == 0 { + return Err(clone_retry::CloneAttemptFailure { + error: crate::Error::message(format!( + "{label} deadline expired before retry" + )), + retry_reason: None, + }); + } + let result = self + .docker_exec_shell_streaming(ExecStreamingRequest { + timeout_ms: Some(timeout_ms), + working_dir: Some("/"), + ..ExecStreamingRequest::new(command) + }) + .await + .map_err(|error| clone_retry::CloneAttemptFailure { + error: crate::Error::context( + format!("{label} transport failed"), + error, + ), + retry_reason: None, + })? + .result; + if result.is_success() { + return Ok(()); + } + let retry_reason = classify_docker_clone_result(&result, token_was_freshly_minted); + Err(clone_retry::CloneAttemptFailure { + error: self.clone_failure_error(result, exec_label, auth_url), + retry_reason, + }) + }, + |failure: &clone_retry::CloneAttemptFailure| failure.retry_reason, + ) + .await + } + + async fn clone_github_repo( &self, origin_url: String, branch: Option, - expected_sha: String, + commit_sha: Option, ) -> crate::Result<()> { self.verify_git_available().await?; let layout = clone_source::github_repo_layout(&origin_url, WORKING_DIRECTORY, REPOS_ROOT)?; @@ -803,7 +836,7 @@ impl DockerSandbox { ) .await .map_err(|error| crate::Error::Context { - message: "Failed to get GitHub App credentials for exact checkout".to_string(), + message: "Failed to get GitHub App credentials for clone".to_string(), source: error.into_boxed_dyn_error(), })?, ), @@ -813,183 +846,6 @@ impl DockerSandbox { .as_ref() .map_or(origin_url.as_str(), |url| url.as_raw_url().as_str()); - self.emit(SandboxEvent::GitCloneStarted { - url: origin_url.clone(), - branch, - }); - let clone_start = Instant::now(); - - let prepare_command = format!( - "mkdir -p {} {}", - shell_quote(WORKING_DIRECTORY), - shell_quote(&layout.repos_owner_path), - ); - if let Err(error) = self - .run_exact_checkout_command( - &prepare_command, - "prepare Docker exact repository checkout", - auth_url.as_ref(), - ) - .await - { - 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_checkout_command( - &init_command, - "initialize Docker exact repository checkout", - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let fetch_command = - clone_source::exact_fetch_command(&layout.primary_repo_path, "origin", &expected_sha); - let clone_deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; - let fetch_result = clone_retry::retry_clone( - SandboxProviderKind::Docker, - Some(clone_deadline), - |_attempt| { - let command = fetch_command.as_str(); - let auth_url = auth_url.as_ref(); - async move { - let remaining = clone_deadline.saturating_duration_since(time::Instant::now()); - let timeout_ms = u64::try_from(remaining.as_millis()).unwrap_or(u64::MAX); - if timeout_ms == 0 { - return Err(DockerCloneFailure { - error: crate::Error::message( - "Docker exact fetch deadline expired before retry", - ), - retry_reason: None, - }); - } - let result = self - .docker_exec_shell_streaming(ExecStreamingRequest { - timeout_ms: Some(timeout_ms), - working_dir: Some("/"), - ..ExecStreamingRequest::new(command) - }) - .await - .map_err(|error| DockerCloneFailure { - error: crate::Error::context( - "Docker exact fetch transport failed", - error, - ), - retry_reason: None, - })? - .result; - if result.is_success() { - return Ok(()); - } - let retry_reason = - classify_docker_clone_result(&result, token_was_freshly_minted); - Err(DockerCloneFailure { - error: self.exact_checkout_exec_error( - result, - "git fetch exact commit", - auth_url, - ), - retry_reason, - }) - } - }, - |failure: &DockerCloneFailure| failure.retry_reason, - ) - .await; - if let Err(failure) = fetch_result { - return Err(self.report_clone_failure(&origin_url, failure.error)); - } - - let checkout_command = clone_source::exact_checkout_command(&layout.primary_repo_path); - if let Err(error) = self - .run_exact_checkout_command( - &checkout_command, - "git checkout exact commit", - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let head_command = clone_source::head_revision_command(&layout.primary_repo_path); - let head = match self - .run_exact_checkout_command( - &head_command, - "verify Docker exact checkout HEAD", - 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_exact_head(&head.stdout, &expected_sha) { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let symlink_command = clone_source::repo_symlink_command(&layout); - if let Err(error) = self - .run_exact_checkout_command( - &symlink_command, - "create Docker workspace repo symlink", - auth_url.as_ref(), - ) - .await - { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let _ = self.repo_cloned.set(true); - let _ = self.origin_url.set(origin_url.clone()); - if let Err(error) = self.set_working_directory(layout.execution_directory) { - return Err(self.report_clone_failure(&origin_url, error)); - } - - let clone_duration = u64::try_from(clone_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::GitCloneCompleted { - url: origin_url, - duration_ms: clone_duration, - }); - Ok(()) - } - - async fn clone_github_repo( - &self, - origin_url: String, - branch: Option, - ) -> crate::Result<()> { - self.verify_git_available().await?; - let layout = clone_source::github_repo_layout(&origin_url, WORKING_DIRECTORY, REPOS_ROOT)?; - let token_was_freshly_minted = self - .github_app - .as_ref() - .is_some_and(GitHubCredentials::mints_installation_token); - - let auth_url = match &self.github_app { - Some(creds) => Some( - fabro_github::resolve_authenticated_url( - &fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()), - &origin_url, - ) - .await - .map_err(|e| { - crate::Error::message(format!( - "Failed to get GitHub App credentials for clone: {e}" - )) - })?, - ), - None => None, - }; - let clone_url = auth_url - .as_ref() - .map_or(origin_url.as_str(), |url| url.as_raw_url().as_str()); - self.emit(SandboxEvent::GitCloneStarted { url: origin_url.clone(), branch: branch.clone(), @@ -1015,58 +871,72 @@ impl DockerSandbox { } } - let command = git_clone_command(clone_url, branch.as_deref(), &layout.primary_repo_path); let clone_deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; - let clone_result = clone_retry::retry_clone( - SandboxProviderKind::Docker, - Some(clone_deadline), - |_attempt| { - let command = command.as_str(); - let auth_url = auth_url.as_ref(); - async move { - let remaining = clone_deadline.saturating_duration_since(time::Instant::now()); - let timeout_ms = u64::try_from(remaining.as_millis()).unwrap_or(u64::MAX); - if timeout_ms == 0 { - return Err(DockerCloneFailure { - error: crate::Error::message( - "Docker git clone deadline expired before retry", - ), - retry_reason: None, - }); - } - let result = self - .docker_exec_shell_streaming(ExecStreamingRequest { - timeout_ms: Some(timeout_ms), - working_dir: Some("/"), - ..ExecStreamingRequest::new(command) - }) - .await - .map_err(|error| DockerCloneFailure { - error: crate::Error::context( - "Docker git clone transport failed", - error, - ), - retry_reason: None, - })? - .result; - if result.is_success() { - return Ok(()); - } - let retry_reason = - classify_docker_clone_result(&result, token_was_freshly_minted); - Err(DockerCloneFailure { - error: self.clone_failure_error(result, auth_url), - retry_reason, - }) - } - }, - |failure: &DockerCloneFailure| failure.retry_reason, - ) - .await; + if let Some(expected_sha) = commit_sha.as_deref() { + let init_command = + clone_source::exact_repository_init_command(clone_url, &layout.primary_repo_path); + if let Err(error) = self + .run_exact_checkout_command( + &init_command, + "initialize Docker exact repository checkout", + auth_url.as_ref(), + ) + .await + { + return Err(self.report_clone_failure(&origin_url, error)); + } - if let Err(failure) = clone_result { - let err = failure.error; - return Err(self.report_clone_failure(&origin_url, err)); + let fetch_command = clone_source::exact_fetch_command( + &layout.primary_repo_path, + "origin", + expected_sha, + ); + if let Err(failure) = self + .retry_git_transfer( + &fetch_command, + "Docker exact fetch", + "git fetch exact commit", + clone_deadline, + token_was_freshly_minted, + auth_url.as_ref(), + ) + .await + { + return Err(self.report_clone_failure(&origin_url, failure.error)); + } + + let checkout_command = + clone_source::exact_checkout_verify_command(&layout.primary_repo_path); + let head = match self + .run_exact_checkout_command( + &checkout_command, + "git checkout exact commit", + 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_exact_head(&head.stdout, expected_sha) { + return Err(self.report_clone_failure(&origin_url, error)); + } + } else { + let command = + git_clone_command(clone_url, branch.as_deref(), &layout.primary_repo_path); + if let Err(failure) = self + .retry_git_transfer( + &command, + "Docker git clone", + "git clone", + clone_deadline, + token_was_freshly_minted, + auth_url.as_ref(), + ) + .await + { + return Err(self.report_clone_failure(&origin_url, failure.error)); + } } let symlink_command = clone_source::repo_symlink_command(&layout); @@ -1584,7 +1454,7 @@ async fn cache_docker_stdio_completion( } fn git_clone_command(clone_url: &str, branch: Option<&str>, checkout_path: &str) -> String { - let mut command = "git -c maintenance.auto=0 -c gc.auto=0 clone".to_string(); + let mut command = format!("{} clone", sandbox::GIT); if let Some(branch) = branch { command.push_str(" --branch "); command.push_str(&shell_quote(branch)); @@ -1855,13 +1725,7 @@ impl Sandbox for DockerSandbox { branch, commit_sha, } => { - let result = if let Some(commit_sha) = commit_sha { - self.checkout_exact_github_commit(origin_url, branch, commit_sha) - .await - } else { - self.clone_github_repo(origin_url, branch).await - }; - if let Err(e) = result { + if let Err(e) = self.clone_github_repo(origin_url, branch, commit_sha).await { return Err(self.fail_init(init_start, e)); } } @@ -2694,7 +2558,7 @@ mod tests { let token = "ghs_exact_checkout_secret"; let auth_url = fabro_github::embed_token_in_url("https://github.com/acme/widgets", token) .expect("authenticated URL"); - let error = sandbox.exact_checkout_exec_error( + let error = sandbox.clone_failure_error( ExecResult { stdout: String::new(), stderr: format!( diff --git a/lib/components/fabro-sandbox/src/sandbox.rs b/lib/components/fabro-sandbox/src/sandbox.rs index 6e6e2b336..94c9a6a54 100644 --- a/lib/components/fabro-sandbox/src/sandbox.rs +++ b/lib/components/fabro-sandbox/src/sandbox.rs @@ -18,7 +18,7 @@ use tokio::time; use tokio_util::sync::CancellationToken; /// Git command prefix that disables background maintenance. -const GIT: &str = "git -c maintenance.auto=0 -c gc.auto=0"; +pub(crate) const GIT: &str = "git -c maintenance.auto=0 -c gc.auto=0"; pub const DEFAULT_EXEC_OUTPUT_TAIL_BYTES: usize = 8 * 1024; diff --git a/lib/components/fabro-sandbox/src/sandbox_spec.rs b/lib/components/fabro-sandbox/src/sandbox_spec.rs index e96f18a93..9550ff949 100644 --- a/lib/components/fabro-sandbox/src/sandbox_spec.rs +++ b/lib/components/fabro-sandbox/src/sandbox_spec.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use std::sync::Arc; -#[cfg(any(feature = "docker", feature = "daytona"))] +#[cfg(feature = "docker")] use anyhow::Context as _; #[cfg(any(feature = "docker", feature = "daytona"))] use fabro_github::GitHubCredentials; @@ -206,15 +206,6 @@ impl SandboxSpec { clone_branch, clone_commit_sha, } => { - if clone_commit_sha.is_some() { - clone_source::decide_clone( - config.skip_clone, - clone_origin_url.as_deref(), - clone_branch.as_deref(), - clone_commit_sha.as_deref(), - ) - .context("Invalid Docker exact-checkout request")?; - } let mut sandbox = DockerSandbox::new( config.clone(), github_app.clone(), @@ -239,16 +230,6 @@ impl SandboxSpec { clone_commit_sha, api_key, } => { - if clone_commit_sha.is_some() { - clone_source::decide_clone( - config.skip_clone, - clone_origin_url.as_deref(), - clone_branch.as_deref(), - clone_commit_sha.as_deref(), - ) - .map_err(anyhow::Error::new) - .context("Invalid Daytona exact-checkout request")?; - } let mut sandbox = DaytonaSandbox::new( config.as_ref().clone(), github_app.clone(), @@ -350,7 +331,7 @@ mod tests { assert!( error .to_string() - .contains("Invalid Docker exact-checkout request") + .contains("Failed to create Docker sandbox") ); assert!(format!("{error:#}").contains("40 ASCII hexadecimal")); assert!(!format!("{error:#}").contains("Docker daemon"));