Merge pull request #667 from fabro-sh/fix/retry-transient-sandbox-clone

Retry the sandbox clone when GitHub token replication lags
This commit is contained in:
Bryan Helmkamp 2026-07-28 16:39:43 -04:00 committed by GitHub
commit 9f32e14400
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 796 additions and 123 deletions

View file

@ -194,6 +194,12 @@ pub enum GitHubCredentials {
}
impl GitHubCredentials {
/// Whether resolving these credentials mints a new installation token.
#[must_use]
pub fn mints_installation_token(&self) -> bool {
matches!(self, Self::App(_))
}
pub fn from_env(app_id: Option<&str>) -> Result<Option<Self>, String> {
Ok(GitHubAppCredentials::from_env(app_id)?.map(Self::App))
}
@ -372,7 +378,7 @@ pub fn parse_github_owner_repo(url: &str) -> anyhow::Result<(String, String)> {
let path = path.trim_end_matches('/');
let path = path.strip_suffix(".git").unwrap_or(path);
let mut parts = path.splitn(3, '/');
let mut parts = path.split('/');
let owner = parts
.next()
.filter(|s| !s.is_empty())
@ -381,6 +387,9 @@ pub fn parse_github_owner_repo(url: &str) -> anyhow::Result<(String, String)> {
.next()
.filter(|s| !s.is_empty())
.ok_or_else(|| anyhow!("Missing repo in GitHub URL: {display_url}"))?;
if parts.next().is_some() {
bail!("GitHub URL must identify one repository: {display_url}");
}
Ok((owner.to_string(), repo.to_string()))
}
@ -1388,6 +1397,16 @@ mod tests {
assert_eq!(repo, "repo");
}
#[test]
fn parse_rejects_extra_path_components() {
let error = parse_github_owner_repo("https://github.com/owner/repo/issues").unwrap_err();
assert!(
error.to_string().contains("must identify one repository"),
"got: {error}"
);
}
// -----------------------------------------------------------------------
// ssh_url_to_https
// -----------------------------------------------------------------------
@ -2312,6 +2331,24 @@ mod tests {
assert!(!fresh.near_expiry(std::time::Duration::from_mins(15)));
}
#[test]
fn only_app_credentials_mint_installation_tokens() {
let app = GitHubCredentials::App(GitHubAppCredentials {
app_id: "test".to_string(),
private_key_pem: test_rsa_key().to_string(),
slug: None,
});
let pat = GitHubCredentials::Pat("ghp_personal".to_string());
let installation = GitHubCredentials::Installation(InstallationToken {
token: "ghs_installation".to_string(),
expires_at: chrono::Utc::now() + chrono::Duration::minutes(30),
});
assert!(app.mints_installation_token());
assert!(!pat.mints_installation_token());
assert!(!installation.mints_installation_token());
}
#[test]
fn validate_static_github_token_rejects_installation_tokens() {
validate_static_github_token("ghp_personal").unwrap();

View file

@ -0,0 +1,400 @@
//! Retry for the first repository clone in a clone-based sandbox.
//!
//! Clone-based providers can mint a GitHub App installation token and clone
//! with it immediately. GitHub can reject that first clone before the token is
//! available to the git endpoint. On a private repository, the rejection can
//! arrive as `Repository not found.` or an authentication failure.
//!
//! Only a token minted during the current clone operation makes those messages
//! safe to retry. Static PATs and pre-minted installation tokens fail fast.
//!
//! Retries reuse the same token on purpose. Replication of a given token only
//! makes progress, so each attempt strictly improves the odds, while re-minting
//! would restart the replication clock.
use std::future::Future;
use std::time::Duration;
use fabro_types::SandboxProviderKind;
use fabro_util::backoff::BackoffPolicy;
use tokio::time;
/// Total clone attempts, including the first.
const MAX_ATTEMPTS: u32 = 3;
/// Why a failed clone attempt is worth repeating.
#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::Display)]
#[strum(serialize_all = "snake_case")]
pub(crate) enum CloneRetryReason {
/// A freshly minted installation token has not reached the GitHub edge
/// cache site serving this clone yet.
TokenReplication,
/// The clone failed on infrastructure, unrelated to credentials.
TransientInfra,
}
/// What a clone failure message tells us about retry safety.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum CloneMessageClass {
Retry(CloneRetryReason),
Permanent,
Unknown,
}
impl CloneMessageClass {
pub(crate) fn retry_reason(self) -> Option<CloneRetryReason> {
match self {
Self::Retry(reason) => Some(reason),
Self::Permanent | Self::Unknown => None,
}
}
}
/// Message fragments that mean the clone failed on infrastructure.
///
/// These are safe to retry whether or not the clone was authenticated.
const TRANSIENT_HINTS: &[&str] = &[
"could not resolve host",
"temporary failure in name resolution",
"connection refused",
"connection reset",
"connection timed out",
"timed out",
"network is unreachable",
"no route to host",
"tls handshake",
"early eof",
"rpc failed",
"unexpected disconnect",
"the remote end hung up unexpectedly",
"index-pack failed",
"service unavailable",
"gateway timeout",
"too many requests",
"rate limit",
];
/// Message fragments GitHub uses when a token is not yet visible.
///
/// Only meaningful when the clone carried credentials. The same lag surfaces as
/// 404 or as an auth failure depending on which endpoint answers first.
const TOKEN_REPLICATION_HINTS: &[&str] = &[
"repository not found",
"authentication failed",
"invalid username or password",
"bad credentials",
];
/// Classify a failed clone by its rendered message.
///
/// `token_was_freshly_minted` gates the token-replication reading. A static
/// credential cannot become valid during backoff, so auth failures for it are
/// permanent.
pub(crate) fn classify_message(message: &str, token_was_freshly_minted: bool) -> CloneMessageClass {
let lower = message.to_ascii_lowercase();
if TRANSIENT_HINTS.iter().any(|hint| lower.contains(hint)) {
return CloneMessageClass::Retry(CloneRetryReason::TransientInfra);
}
if TOKEN_REPLICATION_HINTS
.iter()
.any(|hint| lower.contains(hint))
{
return if token_was_freshly_minted {
CloneMessageClass::Retry(CloneRetryReason::TokenReplication)
} else {
CloneMessageClass::Permanent
};
}
let permanent = lower.contains("could not read username")
|| lower.contains("terminal prompts disabled")
|| lower.contains("permission denied")
|| (lower.contains("permission to") && lower.contains("denied"))
|| (lower.contains("destination path") && lower.contains("already exists"))
|| (lower.contains("remote branch") && lower.contains("not found"));
if permanent {
return CloneMessageClass::Permanent;
}
CloneMessageClass::Unknown
}
/// Backoff between clone attempts: 3s, then 9s.
///
/// GitHub's guidance for token replication is to wait a few seconds and retry
/// with the same token. Sub-second delays land inside the same replication
/// window and spend an attempt for nothing.
fn backoff() -> BackoffPolicy {
BackoffPolicy {
initial_delay: Duration::from_secs(3),
factor: 3.0,
max_delay: Duration::from_secs(10),
jitter: false,
}
}
/// Run a clone, repeating it while the failure looks transient.
///
/// `attempt` receives the 1-based attempt number. `classify` decides whether an
/// error is worth repeating; `None` returns it to the caller untouched. When a
/// deadline is present, a retry starts only when its backoff fits before that
/// deadline. The final error is returned as-is.
pub(crate) async fn retry_clone<T, E, Attempt, Fut, Classify>(
provider: SandboxProviderKind,
deadline: Option<time::Instant>,
mut attempt: Attempt,
classify: Classify,
) -> Result<T, E>
where
Attempt: FnMut(u32) -> Fut,
Fut: Future<Output = Result<T, E>>,
Classify: Fn(&E) -> Option<CloneRetryReason>,
{
let backoff = backoff();
for attempt_number in 1..MAX_ATTEMPTS {
match attempt(attempt_number).await {
Ok(value) => return Ok(value),
Err(err) => {
let Some(reason) = classify(&err) else {
return Err(err);
};
let delay = backoff.delay_for_attempt(attempt_number);
if deadline.is_some_and(|deadline| {
delay >= deadline.saturating_duration_since(time::Instant::now())
}) {
return Err(err);
}
// The failure text can carry git stderr, so log the category
// rather than the message. The caller still reports the full
// error if the attempts run out.
tracing::warn!(
provider = %provider,
attempt = attempt_number,
max_attempts = MAX_ATTEMPTS,
reason = %reason,
delay_ms = u64::try_from(delay.as_millis()).unwrap_or(u64::MAX),
"Git clone failed, retrying"
);
time::sleep(delay).await;
}
}
}
attempt(MAX_ATTEMPTS).await
}
#[cfg(test)]
mod tests {
use std::sync::Mutex;
use super::*;
/// Records the attempt numbers a closure was called with.
#[derive(Default)]
struct Attempts(Mutex<Vec<u32>>);
impl Attempts {
fn record(&self, attempt: u32) {
self.0.lock().expect("attempt log mutex").push(attempt);
}
fn recorded(&self) -> Vec<u32> {
self.0.lock().expect("attempt log mutex").clone()
}
}
/// A classifier that treats every failure as worth repeating.
const ALWAYS_RETRY: fn(&String) -> Option<CloneRetryReason> =
|_| Some(CloneRetryReason::TokenReplication);
#[test]
fn private_repo_not_found_after_a_successful_mint_is_a_replication_lag() {
assert_eq!(
classify_message("repository not found: Repository not found.", true),
CloneMessageClass::Retry(CloneRetryReason::TokenReplication)
);
}
#[test]
fn not_found_without_a_fresh_token_is_permanent() {
assert_eq!(
classify_message("repository not found: Repository not found.", false),
CloneMessageClass::Permanent
);
}
#[test]
fn auth_failure_with_a_fresh_token_is_a_replication_lag() {
assert_eq!(
classify_message(
"fatal: Authentication failed for 'https://github.com/owner/repo'",
true
),
CloneMessageClass::Retry(CloneRetryReason::TokenReplication)
);
assert_eq!(
classify_message(
"fatal: Authentication failed for 'https://github.com/owner/repo'",
false
),
CloneMessageClass::Permanent
);
}
#[test]
fn infra_failures_retry_without_credentials() {
for message in [
"fatal: unable to access: Could not resolve host: github.com",
"error: RPC failed; curl 56 recv failure",
"fatal: early EOF",
"Operation timed out",
] {
assert_eq!(
classify_message(message, false),
CloneMessageClass::Retry(CloneRetryReason::TransientInfra),
"expected {message:?} to be transient"
);
}
}
#[test]
fn genuine_failures_are_not_retried() {
for message in [
"fatal: could not read Username for 'https://github.com'",
"remote: Permission to owner/repo.git denied",
"fatal: destination path 'repo' already exists",
] {
assert_eq!(
classify_message(message, true),
CloneMessageClass::Permanent,
"expected {message:?} to fail fast"
);
}
}
#[test]
fn unrecognized_failures_remain_unknown() {
assert_eq!(
classify_message("git clone stopped for an unexpected reason", true),
CloneMessageClass::Unknown
);
}
#[test]
fn backoff_waits_seconds_not_milliseconds() {
let backoff = backoff();
assert_eq!(backoff.delay_for_attempt(1), Duration::from_secs(3));
assert_eq!(backoff.delay_for_attempt(2), Duration::from_secs(9));
}
#[tokio::test(start_paused = true)]
async fn first_success_runs_one_attempt() {
let attempts = Attempts::default();
let result = retry_clone(
SandboxProviderKind::Docker,
None,
|attempt| {
attempts.record(attempt);
async move { Ok::<_, String>(attempt) }
},
ALWAYS_RETRY,
)
.await;
assert_eq!(result, Ok(1));
assert_eq!(attempts.recorded(), vec![1]);
}
#[tokio::test(start_paused = true)]
async fn retries_until_a_later_attempt_succeeds() {
let attempts = Attempts::default();
let result = retry_clone(
SandboxProviderKind::Docker,
None,
|attempt| {
attempts.record(attempt);
async move {
if attempt < 3 {
Err("Repository not found.".to_string())
} else {
Ok(attempt)
}
}
},
ALWAYS_RETRY,
)
.await;
assert_eq!(result, Ok(3));
assert_eq!(attempts.recorded(), vec![1, 2, 3]);
}
#[tokio::test(start_paused = true)]
async fn exhausted_attempts_return_the_final_error() {
let attempts = Attempts::default();
let result = retry_clone(
SandboxProviderKind::Docker,
None,
|attempt| {
attempts.record(attempt);
async move { Err::<(), _>(format!("Repository not found. (attempt {attempt})")) }
},
ALWAYS_RETRY,
)
.await;
assert_eq!(
result,
Err("Repository not found. (attempt 3)".to_string()),
"the caller should see the last failure, not the first"
);
assert_eq!(attempts.recorded(), vec![1, 2, 3]);
}
#[tokio::test(start_paused = true)]
async fn unretryable_failure_stops_immediately() {
let attempts = Attempts::default();
let result = retry_clone(
SandboxProviderKind::Docker,
None,
|attempt| {
attempts.record(attempt);
async move { Err::<(), _>("permission denied".to_string()) }
},
|_: &String| None,
)
.await;
assert_eq!(result, Err("permission denied".to_string()));
assert_eq!(
attempts.recorded(),
vec![1],
"a deterministic failure should not wait out the backoff"
);
}
#[tokio::test(start_paused = true)]
async fn deadline_stops_retry_when_backoff_does_not_fit() {
let attempts = Attempts::default();
let deadline = time::Instant::now() + Duration::from_secs(2);
let result = retry_clone(
SandboxProviderKind::Docker,
Some(deadline),
|attempt| {
attempts.record(attempt);
async move { Err::<(), _>("temporary failure".to_string()) }
},
ALWAYS_RETRY,
)
.await;
assert_eq!(result, Err("temporary failure".to_string()));
assert_eq!(attempts.recorded(), vec![1]);
assert_eq!(time::Instant::now() + Duration::from_secs(2), deadline);
}
}

View file

@ -32,6 +32,8 @@ pub(crate) fn github_repo_layout(
"Clone-based sandboxes currently support GitHub repository origins only: {err}"
))
})?;
validate_path_component("owner", &owner)?;
validate_path_component("repository", &repo)?;
let workspace_root = trim_root(workspace_root);
let repos_root = trim_root(repos_root);
let repos_owner_path = sandbox::join_sandbox_path(repos_root, &owner);
@ -48,6 +50,27 @@ pub(crate) fn github_repo_layout(
})
}
fn validate_path_component(label: &str, component: &str) -> crate::Result<()> {
let is_safe = !matches!(component, "." | "..")
&& component
.bytes()
.all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.'));
if !is_safe {
return Err(crate::Error::message(format!(
"GitHub {label} is not a safe repository path component"
)));
}
Ok(())
}
pub(crate) fn repo_symlink_command(layout: &GitHubRepoLayout) -> String {
format!(
"ln -s {} {}",
sandbox::shell_quote(&layout.primary_repo_path),
sandbox::shell_quote(&layout.primary_repo_link),
)
}
fn trim_root(root: &str) -> &str {
let trimmed = root.trim_end_matches('/');
if trimmed.is_empty() { "/" } else { trimmed }
@ -204,6 +227,37 @@ mod tests {
assert_eq!(layout.execution_directory, "/workspace/fabro");
}
#[test]
fn github_layout_rejects_path_traversal_components() {
for origin in [
"https://github.com/../widgets",
"https://github.com/acme/..",
"https://github.com/%2e%2e/widgets",
] {
let error = github_repo_layout(origin, "/workspace", "/repos")
.expect_err("unsafe path component should fail");
assert!(
error.to_string().contains("safe repository path component"),
"got {error} for {origin}"
);
}
}
#[test]
fn repo_symlink_command_quotes_both_paths() {
let layout = github_repo_layout(
"https://github.com/fabro-sh/fabro",
"/work space",
"/repo root",
)
.unwrap();
assert_eq!(
repo_symlink_command(&layout),
"ln -s '/repo root/fabro-sh/fabro' '/work space/fabro'"
);
}
#[test]
fn record_origin_strips_credentials() {
assert_eq!(

View file

@ -16,7 +16,7 @@ use daytona_sdk::toolbox_types::Command as SessionCommandResult;
use daytona_sdk::{DaytonaError, SessionCommandLogsResult};
use fabro_github::GitHubCredentials;
use fabro_static::EnvVars;
use fabro_types::{CommandOutputStream, CommandTermination, RunId};
use fabro_types::{CommandOutputStream, CommandTermination, RunId, SandboxProviderKind};
use fabro_util::time::elapsed_ms;
use rand::Rng;
use tokio::runtime::Handle;
@ -25,6 +25,7 @@ use tokio::task::JoinHandle;
use tokio::{fs, time};
use tokio_util::sync::CancellationToken;
use crate::clone_retry::{self, CloneRetryReason};
use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason};
use crate::redact::redact_auth_url;
use crate::sandbox::{
@ -1050,6 +1051,10 @@ impl Sandbox for DaytonaSandbox {
let layout =
clone_source::github_repo_layout(&origin_url, WORKING_DIRECTORY, REPOS_ROOT)
.map_err(|err| self.fail_init(init_start, err))?;
let token_was_freshly_minted = self
.github_app
.as_ref()
.is_some_and(GitHubCredentials::mints_installation_token);
self.emit(SandboxEvent::GitCloneStarted {
url: origin_url.clone(),
branch: branch.clone(),
@ -1057,40 +1062,26 @@ impl Sandbox for DaytonaSandbox {
let clone_start = Instant::now();
let (username, password) = match &self.github_app {
Some(creds) => {
let (owner, repo) = fabro_github::parse_github_owner_repo(&origin_url)
.map_err(|e| {
let err = crate::Error::message(format!(
"Failed to parse GitHub URL for clone: {e}"
));
self.emit(SandboxEvent::GitCloneFailed {
url: origin_url.clone(),
error: err.to_string(),
causes: err.causes(),
});
err
})?;
fabro_github::resolve_clone_credentials(
&fabro_github::GitHubContext::new(
creds,
&fabro_github::github_api_base_url(),
),
&owner,
&repo,
)
.await
.map_err(|e| {
let err = crate::Error::message(format!(
"Failed to get GitHub App credentials for clone: {e}"
));
self.emit(SandboxEvent::GitCloneFailed {
url: origin_url.clone(),
error: err.to_string(),
causes: err.causes(),
});
self.fail_init(init_start, err)
})?
}
Some(creds) => fabro_github::resolve_clone_credentials(
&fabro_github::GitHubContext::new(
creds,
&fabro_github::github_api_base_url(),
),
&layout.owner,
&layout.repo,
)
.await
.map_err(|e| {
let err = crate::Error::message(format!(
"Failed to get GitHub App credentials for clone: {e}"
));
self.emit(SandboxEvent::GitCloneFailed {
url: origin_url.clone(),
error: err.to_string(),
causes: err.causes(),
});
self.fail_init(init_start, err)
})?,
None => (None, None),
};
@ -1155,19 +1146,24 @@ impl Sandbox for DaytonaSandbox {
self.fail_init(init_start, err)
})?;
let clone_token = password.clone();
let clone_result = git_svc
.clone(
&origin_url,
&layout.primary_repo_path,
daytona_sdk::GitCloneOptions {
branch,
username,
password,
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()
},
)
.await;
};
async move { git_svc.clone(origin, target, options).await }
},
|err: &DaytonaError| classify_clone_failure(err, token_was_freshly_minted),
)
.await;
match clone_result {
Ok(()) => {
@ -1181,7 +1177,7 @@ impl Sandbox for DaytonaSandbox {
});
self.fail_init(init_start, err)
})?;
let symlink_cmd = daytona_symlink_command(&layout);
let symlink_cmd = clone_source::repo_symlink_command(&layout);
let symlink_result = process_svc
.execute_command(
&wrap_bash_command(&symlink_cmd),
@ -1233,8 +1229,8 @@ impl Sandbox for DaytonaSandbox {
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) = clone_token {
match fabro_github::embed_token_in_url(&origin_url, &token) {
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 {}",
@ -1536,7 +1532,7 @@ impl Sandbox for DaytonaSandbox {
// Only a GitHub App installation token can be re-minted; a static PAT or
// a pre-minted Installation token is fixed, so re-embedding it changes
// nothing. Short-circuit to Skipped before the resolve + set-url exec.
if !matches!(creds, GitHubCredentials::App(_)) {
if !creds.mints_installation_token() {
return Ok(RefreshOutcome::Skipped);
}
@ -2490,12 +2486,36 @@ fn daytona_bash_session_probe_outcome(execution: crate::Result<ExecResult>) -> c
))
}
fn daytona_symlink_command(layout: &clone_source::GitHubRepoLayout) -> String {
format!(
"ln -s {} {}",
shell_quote(&layout.primary_repo_path),
shell_quote(&layout.primary_repo_link),
)
/// Classify a failed Daytona clone for retry.
///
/// The GitHub 404 does not arrive as an HTTP 404 on the Daytona call. git runs
/// inside the sandbox, so its stderr comes back through the toolbox as the
/// error message — the credential race has to be matched on text. Daytona's own
/// transport failures are visible structurally.
fn classify_clone_failure(
err: &DaytonaError,
token_was_freshly_minted: bool,
) -> Option<CloneRetryReason> {
// A Daytona request timeout does not prove that the remote clone stopped.
// Retrying could overlap the still-running first request.
if matches!(err, DaytonaError::Timeout { .. }) {
return None;
}
match clone_retry::classify_message(err.message(), token_was_freshly_minted) {
clone_retry::CloneMessageClass::Retry(reason) => Some(reason),
clone_retry::CloneMessageClass::Permanent => None,
clone_retry::CloneMessageClass::Unknown => match err {
DaytonaError::RateLimit { .. } => Some(CloneRetryReason::TransientInfra),
DaytonaError::Api { status_code, .. } if (500..600).contains(status_code) => {
Some(CloneRetryReason::TransientInfra)
}
DaytonaError::Timeout { .. }
| DaytonaError::Api { .. }
| DaytonaError::NotFound { .. }
| DaytonaError::General(_) => None,
},
}
}
/// Wrap Bash source in the canonical non-login Bash transport.
@ -2988,18 +3008,78 @@ mod tests {
}
#[test]
fn daytona_symlink_command_links_workspace_repo_to_repos_checkout() {
let layout = clone_source::github_repo_layout(
"https://github.com/fabro-sh/fabro",
WORKING_DIRECTORY,
REPOS_ROOT,
)
.unwrap();
fn clone_not_found_after_a_successful_mint_is_retried() {
// The exact error from run 01KYM99DF27JRRW4XSYZBP27K7: git's stderr,
// relayed through the toolbox, five seconds after a token was minted.
let err = DaytonaError::general("repository not found: Repository not found.");
assert_eq!(
daytona_symlink_command(&layout),
"ln -s /home/daytona/repos/fabro-sh/fabro /home/daytona/workspace/fabro"
classify_clone_failure(&err, true),
Some(CloneRetryReason::TokenReplication)
);
assert_eq!(
classify_clone_failure(&err, false),
None,
"without credentials there is no token to replicate"
);
}
#[test]
fn clone_transient_transport_failures_are_retried() {
for err in [
DaytonaError::rate_limit("too many requests"),
DaytonaError::api(503, ""),
] {
assert_eq!(
classify_clone_failure(&err, false),
Some(CloneRetryReason::TransientInfra),
"expected {err:?} to be transient"
);
}
}
#[test]
fn clone_timeout_is_not_retried_without_remote_termination() {
let err = DaytonaError::timeout("request timed out");
assert_eq!(classify_clone_failure(&err, true), None);
}
#[test]
fn clone_api_failure_message_takes_precedence_over_status() {
let not_found = DaytonaError::api(500, "repository not found: Repository not found.");
assert_eq!(
classify_clone_failure(&not_found, true),
Some(CloneRetryReason::TokenReplication)
);
assert_eq!(classify_clone_failure(&not_found, false), None);
for message in [
"fatal: destination path 'fabro' already exists",
"remote: Permission to fabro-sh/fabro.git denied",
] {
assert_eq!(
classify_clone_failure(&DaytonaError::api(500, message), true),
None,
"expected {message:?} to take precedence over HTTP 500"
);
}
}
#[test]
fn clone_client_errors_are_not_retried() {
for err in [
DaytonaError::api(400, "bad request"),
DaytonaError::api(403, "forbidden"),
DaytonaError::not_found("Sandbox not found"),
DaytonaError::general("fatal: could not read Username for 'https://github.com'"),
] {
assert_eq!(
classify_clone_failure(&err, true),
None,
"expected {err:?} to fail fast"
);
}
}
#[test]

View file

@ -3,7 +3,7 @@ use std::fmt::Write as _;
use std::io::Cursor;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use std::time::{Instant, SystemTime, UNIX_EPOCH};
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
use async_trait::async_trait;
use bollard::Docker;
@ -17,7 +17,7 @@ use bollard::exec::{CreateExecOptions, StartExecOptions, StartExecResults};
use bollard::image::CreateImageOptions;
use bollard::models::{ContainerInspectResponse, HostConfig};
use fabro_github::GitHubCredentials;
use fabro_types::{CommandOutputStream, CommandTermination, RunId};
use fabro_types::{CommandOutputStream, CommandTermination, RunId, SandboxProviderKind};
use fabro_util::time::elapsed_ms;
use futures::StreamExt;
use tokio::io::{AsyncWriteExt, duplex};
@ -37,7 +37,7 @@ use crate::{
CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,
ExecStreamingResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback, SandboxFile,
StderrCollector, StdioProcess, StdioProcessHandle, StdioProcessTermination, WalkOptions,
format_lines_numbered, shell_quote,
clone_retry, format_lines_numbered, shell_quote,
};
const DOCKER_BASH_REQUIREMENT: &str = "Docker sandboxes require /bin/bash for every command, with no `sh` fallback; use an \
@ -46,6 +46,7 @@ const DOCKER_BASH_REQUIREMENT: &str = "Docker sandboxes require /bin/bash for ev
pub(crate) const WORKING_DIRECTORY: &str = "/workspace";
pub(crate) const REPOS_ROOT: &str = "/repos";
const GIT_CLONE_DEPTH: usize = 10;
const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(5);
#[cfg(test)]
const EXEC_STOP_POLL_SLEEP_SECONDS: &str = "0.005";
#[cfg(not(test))]
@ -55,6 +56,11 @@ 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<clone_retry::CloneRetryReason>,
}
fn env_entry_name(entry: &str) -> &str {
entry.split_once('=').map_or(entry, |(name, _)| name)
}
@ -681,6 +687,32 @@ impl DockerSandbox {
Ok(())
}
/// Preserve a failed `git clone` result while masking the auth URL.
fn clone_failure_error(
&self,
result: ExecResult,
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 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."
} else {
"Failed to clone repository into Docker sandbox"
};
crate::Error::context(message, error)
}
fn report_clone_failure(&self, origin_url: &str, err: crate::Error) -> crate::Error {
self.emit(SandboxEvent::GitCloneFailed {
url: origin_url.to_string(),
error: err.to_string(),
causes: err.causes(),
});
err
}
async fn clone_github_repo(
&self,
origin_url: String,
@ -688,12 +720,10 @@ impl DockerSandbox {
) -> crate::Result<()> {
self.verify_git_available().await?;
let layout = clone_source::github_repo_layout(&origin_url, WORKING_DIRECTORY, REPOS_ROOT)?;
self.emit(SandboxEvent::GitCloneStarted {
url: origin_url.clone(),
branch: branch.clone(),
});
let clone_start = Instant::now();
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(
@ -714,26 +744,101 @@ impl DockerSandbox {
.as_ref()
.map_or(origin_url.as_str(), |url| url.as_raw_url().as_str());
let command = git_clone_and_link_command(clone_url, branch.as_deref(), &layout);
self.emit(SandboxEvent::GitCloneStarted {
url: origin_url.clone(),
branch: branch.clone(),
});
let clone_start = Instant::now();
let result = self
.docker_exec_shell(&command, 300_000, Some("/"), None, None)
.await?;
if !result.is_success() {
let stderr = redact_auth_url(&result.stderr, auth_url.as_ref());
let err = crate::Error::message(if self.github_app.is_none() {
format!(
"Git clone failed: {stderr}. If this is a private repository, configure a GitHub App with `fabro install` and install it for your organization."
)
} else {
format!("Failed to clone repo into Docker sandbox: {stderr}")
});
self.emit(SandboxEvent::GitCloneFailed {
url: origin_url,
error: err.to_string(),
causes: err.causes(),
});
return Err(err);
let prepare_command = format!(
"mkdir -p {} {}",
shell_quote(WORKING_DIRECTORY),
shell_quote(&layout.repos_owner_path),
);
match self
.docker_exec_shell(&prepare_command, 10_000, Some("/"), None, None)
.await
{
Ok(result) if result.is_success() => {}
Ok(result) => {
let err = result.into_exec_error("prepare Docker repository checkout");
return Err(self.report_clone_failure(&origin_url, err));
}
Err(err) => {
return Err(self.report_clone_failure(&origin_url, err));
}
}
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(
command,
Some(timeout_ms),
Some("/"),
None,
None,
None,
)
.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 Err(failure) = clone_result {
let err = failure.error;
return Err(self.report_clone_failure(&origin_url, err));
}
let symlink_command = clone_source::repo_symlink_command(&layout);
match self
.docker_exec_shell(&symlink_command, 10_000, Some("/"), None, None)
.await
{
Ok(result) if result.is_success() => {}
Ok(result) => {
let err = result.into_exec_error("create Docker workspace repo symlink");
return Err(self.report_clone_failure(&origin_url, err));
}
Err(err) => {
return Err(self.report_clone_failure(&origin_url, err));
}
}
let _ = self.repo_cloned.set(true);
@ -1252,19 +1357,17 @@ fn git_clone_command(clone_url: &str, branch: Option<&str>, checkout_path: &str)
command
}
fn git_clone_and_link_command(
clone_url: &str,
branch: Option<&str>,
layout: &clone_source::GitHubRepoLayout,
) -> String {
format!(
"mkdir -p {} {} && {} && ln -s {} {}",
shell_quote(WORKING_DIRECTORY),
shell_quote(&layout.repos_owner_path),
git_clone_command(clone_url, branch, &layout.primary_repo_path),
shell_quote(&layout.primary_repo_path),
shell_quote(&layout.primary_repo_link),
)
fn classify_docker_clone_result(
result: &ExecResult,
token_was_freshly_minted: bool,
) -> Option<clone_retry::CloneRetryReason> {
let stderr = clone_retry::classify_message(&result.stderr, token_was_freshly_minted);
match stderr {
clone_retry::CloneMessageClass::Unknown => {
clone_retry::classify_message(&result.stdout, token_was_freshly_minted).retry_reason()
}
class => class.retry_reason(),
}
}
fn host_config(config: &DockerSandboxOptions) -> HostConfig {
@ -2079,7 +2182,7 @@ impl Sandbox for DockerSandbox {
// Only a GitHub App installation token can be re-minted; a static PAT or
// a pre-minted Installation token is fixed, so re-embedding it changes
// nothing. Short-circuit to Skipped before the resolve + set-url exec.
if !matches!(creds, GitHubCredentials::App(_)) {
if !creds.mints_installation_token() {
return Ok(RefreshOutcome::Skipped);
}
@ -2307,20 +2410,16 @@ mod tests {
}
#[test]
fn clone_and_link_command_creates_workspace_symlink_to_repos_checkout() {
let layout = clone_source::github_repo_layout(
"https://github.com/fabro-sh/fabro",
"/workspace",
"/repos",
)
.unwrap();
let command =
git_clone_and_link_command("https://github.com/fabro-sh/fabro", Some("main"), &layout);
fn clone_result_uses_stderr_before_stdout() {
let result = ExecResult {
stdout: "Could not resolve host: github.com".to_string(),
stderr: "fatal: destination path 'fabro' already exists".to_string(),
exit_code: Some(128),
termination: CommandTermination::Exited,
duration_ms: 1,
};
assert_eq!(
command,
"mkdir -p /workspace /repos/fabro-sh && git -c maintenance.auto=0 -c gc.auto=0 clone --branch main --single-branch --depth 10 --no-tags -- https://github.com/fabro-sh/fabro /repos/fabro-sh/fabro && ln -s /repos/fabro-sh/fabro /workspace/fabro"
);
assert_eq!(classify_docker_clone_result(&result, true), None);
}
#[test]

View file

@ -143,7 +143,7 @@ pub(crate) fn classify_exec_failure(stderr: &str) -> Option<&'static str> {
} else if lower.contains("could not resolve host") || lower.contains("network is unreachable") {
Some("network failure inside sandbox - check DNS / egress from the run container")
} else if lower.contains("repository not found") {
Some("github 404 - the App installation may not include this repo")
Some("github 404 - repository is unavailable to the current credentials")
} else if lower.contains("no such remote") && lower.contains("origin") {
Some("origin remote missing - push credentials could not be installed")
} else if lower.contains("not a git repository")

View file

@ -9,6 +9,9 @@ pub mod sandbox_spec;
#[cfg(any(feature = "docker", feature = "daytona"))]
mod clone_source;
#[cfg(any(feature = "docker", feature = "daytona", test))]
mod clone_retry;
#[cfg(any(feature = "docker", feature = "daytona", test))]
mod managed_labels;