From e658f9705e6f9138b0efa01daedd50441c3c66f7 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 19 Mar 2026 12:27:56 -0400 Subject: [PATCH] Add skip_clone to DaytonaConfig to fix concurrent test failures Two Daytona integration tests used std::env::set_current_dir to a temp directory so detect_repo_info() would fail and skip cloning. Since cwd is process-global, this poisoned concurrent tests. Replace with an explicit skip_clone config flag that skips repo detection and cloning during sandbox initialization. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-api/src/demo/mod.rs | 6 + lib/crates/fabro-config/src/sandbox.rs | 3 + lib/crates/fabro-daytona/src/lib.rs | 328 +++++++++--------- .../tests/daytona_integration.rs | 7 +- 4 files changed, 182 insertions(+), 162 deletions(-) diff --git a/lib/crates/fabro-api/src/demo/mod.rs b/lib/crates/fabro-api/src/demo/mod.rs index b0adc84df..77fe69c43 100644 --- a/lib/crates/fabro-api/src/demo/mod.rs +++ b/lib/crates/fabro-api/src/demo/mod.rs @@ -1303,6 +1303,7 @@ mod runs { dockerfile: None, }), network: Some(fabro_daytona::DaytonaNetwork::Block), + skip_clone: false, }), exe: None, ssh: None, @@ -1475,6 +1476,7 @@ mod workflows { dockerfile: None, }), network: None, + skip_clone: false, }), exe: None, ssh: None, @@ -1548,6 +1550,7 @@ mod workflows { dockerfile: None, }), network: None, + skip_clone: false, }), exe: None, ssh: None, @@ -1632,6 +1635,7 @@ mod workflows { dockerfile: None, }), network: None, + skip_clone: false, }), exe: None, ssh: None, @@ -1707,6 +1711,7 @@ mod workflows { dockerfile: None, }), network: None, + skip_clone: false, }), exe: None, ssh: None, @@ -3273,6 +3278,7 @@ mod settings { labels: None, snapshot: None, network: Some(fabro_daytona::DaytonaNetwork::Block), + skip_clone: false, }), exe: None, ssh: None, diff --git a/lib/crates/fabro-config/src/sandbox.rs b/lib/crates/fabro-config/src/sandbox.rs index 1165b4e7c..c16e562ff 100644 --- a/lib/crates/fabro-config/src/sandbox.rs +++ b/lib/crates/fabro-config/src/sandbox.rs @@ -12,6 +12,9 @@ pub struct DaytonaConfig { pub labels: Option>, pub snapshot: Option, pub network: Option, + /// Skip git repo detection and cloning during initialization. + #[serde(default)] + pub skip_clone: bool, } /// Network access mode for a Daytona sandbox. diff --git a/lib/crates/fabro-daytona/src/lib.rs b/lib/crates/fabro-daytona/src/lib.rs index 1f8083771..fce7610be 100644 --- a/lib/crates/fabro-daytona/src/lib.rs +++ b/lib/crates/fabro-daytona/src/lib.rs @@ -445,176 +445,190 @@ impl Sandbox for DaytonaSandbox { })?; // Clone the repo into the sandbox - match detect_repo_info(&cwd) { - Ok((detected_url, detected_branch)) => { - // Use explicit clone_branch if provided (avoids cloning a local-only - // worktree branch like fabro/run/... that hasn't been pushed). - let branch = self.clone_branch.clone().or(detected_branch); - // Daytona clones over HTTPS with token auth, so rewrite SSH URLs. - let url = ssh_url_to_https(&detected_url); - self.emit(SandboxEvent::GitCloneStarted { - url: url.clone(), - branch: branch.clone(), - }); - let clone_start = Instant::now(); + if self.config.skip_clone { + // Create working directory without cloning + let fs_svc = sandbox + .fs() + .await + .map_err(|e| format!("Failed to get Daytona fs service: {e}"))?; + fs_svc + .create_folder(WORKING_DIRECTORY, None) + .await + .map_err(|e| format!("Failed to create working directory: {e}"))?; + } else { + match detect_repo_info(&cwd) { + Ok((detected_url, detected_branch)) => { + // Use explicit clone_branch if provided (avoids cloning a local-only + // worktree branch like fabro/run/... that hasn't been pushed). + let branch = self.clone_branch.clone().or(detected_branch); + // Daytona clones over HTTPS with token auth, so rewrite SSH URLs. + let url = ssh_url_to_https(&detected_url); + self.emit(SandboxEvent::GitCloneStarted { + url: url.clone(), + branch: branch.clone(), + }); + let clone_start = Instant::now(); - // Resolve clone credentials via GitHub App or fall back to no auth - let (username, password) = match &self.github_app { - Some(creds) => { - let (owner, repo) = - fabro_github::parse_github_owner_repo(&url).map_err(|e| { - let err = format!("Failed to parse GitHub URL for clone: {e}"); - self.emit(SandboxEvent::GitCloneFailed { - url: url.clone(), - error: err.clone(), - }); - err - })?; - fabro_github::resolve_clone_credentials(creds, &owner, &repo) - .await - .map_err(|e| { - let err = - format!("Failed to get GitHub App credentials for clone: {e}"); - self.emit(SandboxEvent::GitCloneFailed { - url: url.clone(), - error: err.clone(), - }); - let duration_ms = u64::try_from(init_start.elapsed().as_millis()) - .unwrap_or(u64::MAX); - self.emit(SandboxEvent::InitializeFailed { - provider: "daytona".into(), - error: err.clone(), - duration_ms, - }); - err - })? - } - None => (None, None), - }; + // Resolve clone credentials via GitHub App or fall back to no auth + let (username, password) = match &self.github_app { + Some(creds) => { + let (owner, repo) = fabro_github::parse_github_owner_repo(&url) + .map_err(|e| { + let err = format!("Failed to parse GitHub URL for clone: {e}"); + self.emit(SandboxEvent::GitCloneFailed { + url: url.clone(), + error: err.clone(), + }); + err + })?; + fabro_github::resolve_clone_credentials(creds, &owner, &repo) + .await + .map_err(|e| { + let err = format!( + "Failed to get GitHub App credentials for clone: {e}" + ); + self.emit(SandboxEvent::GitCloneFailed { + url: url.clone(), + error: err.clone(), + }); + let duration_ms = + u64::try_from(init_start.elapsed().as_millis()) + .unwrap_or(u64::MAX); + self.emit(SandboxEvent::InitializeFailed { + provider: "daytona".into(), + error: err.clone(), + duration_ms, + }); + err + })? + } + None => (None, None), + }; - let git_svc = sandbox - .git() - .await - .map_err(|e| format!("Failed to get Daytona git service: {e}")); - let git_svc = match git_svc { - Ok(g) => g, - Err(e) => { - self.emit(SandboxEvent::GitCloneFailed { - url: url.clone(), - error: e.clone(), - }); - let duration_ms = - u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::InitializeFailed { - provider: "daytona".into(), - error: e.clone(), - duration_ms, - }); - return Err(e); - } - }; + let git_svc = sandbox + .git() + .await + .map_err(|e| format!("Failed to get Daytona git service: {e}")); + let git_svc = match git_svc { + Ok(g) => g, + Err(e) => { + self.emit(SandboxEvent::GitCloneFailed { + url: url.clone(), + error: e.clone(), + }); + let duration_ms = + u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::InitializeFailed { + provider: "daytona".into(), + error: e.clone(), + duration_ms, + }); + return Err(e); + } + }; - let clone_token = password.clone(); - let clone_result = git_svc - .clone( - &url, - WORKING_DIRECTORY, - daytona_sdk::GitCloneOptions { - branch, - username, - password, - ..Default::default() - }, - ) - .await; + let clone_token = password.clone(); + let clone_result = git_svc + .clone( + &url, + WORKING_DIRECTORY, + daytona_sdk::GitCloneOptions { + branch, + username, + password, + ..Default::default() + }, + ) + .await; - match clone_result { - Ok(()) => { - let clone_duration = - u64::try_from(clone_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::GitCloneCompleted { - url: url.clone(), - duration_ms: clone_duration, - }); + match clone_result { + Ok(()) => { + let clone_duration = u64::try_from(clone_start.elapsed().as_millis()) + .unwrap_or(u64::MAX); + self.emit(SandboxEvent::GitCloneCompleted { + url: url.clone(), + duration_ms: clone_duration, + }); - // Store origin URL and set push credentials for later pushes - if let Some(token) = clone_token { - let _ = self.origin_url.set(url); - let process_svc = sandbox.process().await.ok(); - if let Some(ps) = process_svc { - let origin = self.origin_url.get().expect("just set"); - let auth_url = origin.replacen( - "https://", - &format!("https://x-access-token:{token}@"), - 1, - ); - let cmd = format!( - "git -c maintenance.auto=0 remote set-url origin {}", - shell_quote(&auth_url), - ); - let opts = daytona_sdk::ExecuteCommandOptions { - cwd: Some(WORKING_DIRECTORY.to_string()), - ..Default::default() - }; - let wrapped = wrap_bash_command(&cmd); - if let Ok(r) = ps.execute_command(&wrapped, opts).await { - if r.exit_code != 0 { - tracing::warn!( - exit_code = r.exit_code, - "Failed to set push credentials on origin" - ); + // Store origin URL and set push credentials for later pushes + if let Some(token) = clone_token { + let _ = self.origin_url.set(url); + let process_svc = sandbox.process().await.ok(); + if let Some(ps) = process_svc { + let origin = self.origin_url.get().expect("just set"); + let auth_url = origin.replacen( + "https://", + &format!("https://x-access-token:{token}@"), + 1, + ); + let cmd = format!( + "git -c maintenance.auto=0 remote set-url origin {}", + shell_quote(&auth_url), + ); + let opts = daytona_sdk::ExecuteCommandOptions { + cwd: Some(WORKING_DIRECTORY.to_string()), + ..Default::default() + }; + let wrapped = wrap_bash_command(&cmd); + if let Ok(r) = ps.execute_command(&wrapped, opts).await { + if r.exit_code != 0 { + tracing::warn!( + exit_code = r.exit_code, + "Failed to set push credentials on origin" + ); + } } } } } - } - Err(e) if self.github_app.is_none() => { - let err = format!( - "Git clone failed: {e}. If this is a private repository, \ + Err(e) if self.github_app.is_none() => { + let err = format!( + "Git clone failed: {e}. If this is a private repository, \ configure a GitHub App with `fabro install` and install it \ for your organization." - ); - self.emit(SandboxEvent::GitCloneFailed { - url, - error: err.clone(), - }); - let duration_ms = - u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::InitializeFailed { - provider: "daytona".into(), - error: err.clone(), - duration_ms, - }); - return Err(err); - } - Err(e) => { - let err = format!("Failed to clone repo into Daytona sandbox: {e}"); - self.emit(SandboxEvent::GitCloneFailed { - url, - error: err.clone(), - }); - let duration_ms = - u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); - self.emit(SandboxEvent::InitializeFailed { - provider: "daytona".into(), - error: err.clone(), - duration_ms, - }); - return Err(err); + ); + self.emit(SandboxEvent::GitCloneFailed { + url, + error: err.clone(), + }); + let duration_ms = + u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::InitializeFailed { + provider: "daytona".into(), + error: err.clone(), + duration_ms, + }); + return Err(err); + } + Err(e) => { + let err = format!("Failed to clone repo into Daytona sandbox: {e}"); + self.emit(SandboxEvent::GitCloneFailed { + url, + error: err.clone(), + }); + let duration_ms = + u64::try_from(init_start.elapsed().as_millis()).unwrap_or(u64::MAX); + self.emit(SandboxEvent::InitializeFailed { + provider: "daytona".into(), + error: err.clone(), + duration_ms, + }); + return Err(err); + } } } - } - Err(e) => { - tracing::warn!(error = %e, "Could not detect git repo for Daytona clone"); - // Create working directory even without a repo - let fs_svc = sandbox - .fs() - .await - .map_err(|e| format!("Failed to get Daytona fs service: {e}"))?; - fs_svc - .create_folder(WORKING_DIRECTORY, None) - .await - .map_err(|e| format!("Failed to create working directory: {e}"))?; + Err(e) => { + tracing::warn!(error = %e, "Could not detect git repo for Daytona clone"); + // Create working directory even without a repo + let fs_svc = sandbox + .fs() + .await + .map_err(|e| format!("Failed to get Daytona fs service: {e}"))?; + fs_svc + .create_folder(WORKING_DIRECTORY, None) + .await + .map_err(|e| format!("Failed to create working directory: {e}"))?; + } } } diff --git a/lib/crates/fabro-workflows/tests/daytona_integration.rs b/lib/crates/fabro-workflows/tests/daytona_integration.rs index 92cbe424b..2b0bf222e 100644 --- a/lib/crates/fabro-workflows/tests/daytona_integration.rs +++ b/lib/crates/fabro-workflows/tests/daytona_integration.rs @@ -1852,9 +1852,6 @@ async fn daytona_cp_upload_download_round_trip() { async fn daytona_computer_use_browser_screenshot() { use base64::Engine; - // Run from a temp dir so detect_repo_info() finds no git repo and skips cloning. - let tmp = tempfile::tempdir().unwrap(); - std::env::set_current_dir(tmp.path()).unwrap(); dotenvy::dotenv().ok(); if let Some(home) = dirs::home_dir() { dotenvy::from_path(home.join(".fabro/.env")).ok(); @@ -1867,6 +1864,7 @@ async fn daytona_computer_use_browser_screenshot() { disk: None, dockerfile: None, }), + skip_clone: true, ..DaytonaConfig::default() }; let env = DaytonaSandbox::new(config, None, None, None) @@ -2013,8 +2011,6 @@ async fn daytona_playwright_mcp_sandbox_transport() { use fabro_agent::Sandbox; // Create sandbox from daytona-medium (has Node.js + Chromium) - let tmp = tempfile::tempdir().unwrap(); - std::env::set_current_dir(tmp.path()).unwrap(); dotenvy::dotenv().ok(); if let Some(home) = dirs::home_dir() { dotenvy::from_path(home.join(".fabro/.env")).ok(); @@ -2027,6 +2023,7 @@ async fn daytona_playwright_mcp_sandbox_transport() { disk: None, dockerfile: None, }), + skip_clone: true, ..DaytonaConfig::default() }; let sandbox = DaytonaSandbox::new(config, None, None, None)