From 14875c2fdf295cb983d6ec87a659167ac1691b40 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 10 Sep 2026 13:16:16 -0600 Subject: [PATCH] Set up and push the run branch through the driver's git facet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fabro's run-branch setup and its pushes were the last git it assembled by hand: two rev-parse calls, a checkout -B, and a push built as shell and run through exec. Because those failures never passed through the driver, fabro kept a second classifier that read git's stderr for the same auth and not-found shapes the driver already classifies for a clone. Both now call the facet. Setup reads the current branch and head commit from the driver's status and creates or moves the run branch at that base; a push sends its refspec with what remains of the retry plan's attempt budget as the push timeout. The retry plan, the credential lease, and the drift repair stay as they were — they are fabro's policy — but the decision they act on comes from the driver's failure class, the same way the clone's does. The output-shaped classifiers and the auth hint matchers are deleted; the message classifier remains for the host-side repository probe and metadata push, which never run inside a sandbox. A git failure's captured output now renders as the attempt's output tail, as an exec failure's did. The driver pin moves to lithoscomputer/sandbox-driver#18, which adds the push refspec and timeout, the checkout start point, and the status head this relies on. Co-Authored-By: Claude Fable 5.1 --- Cargo.lock | 16 +- Cargo.toml | 16 +- lib/components/fabro-sandbox/src/clone.rs | 8 +- .../fabro-sandbox/src/driver_sandbox.rs | 17 +- lib/components/fabro-sandbox/src/error.rs | 37 +++- lib/components/fabro-sandbox/src/git_retry.rs | 26 +-- lib/components/fabro-sandbox/src/lib.rs | 2 +- lib/components/fabro-sandbox/src/sandbox.rs | 184 +++++++++--------- 8 files changed, 155 insertions(+), 151 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0826526f7..da8d01ce3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7010,7 +7010,7 @@ dependencies = [ [[package]] name = "sandbox-driver" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "async-trait", "globset", @@ -7026,7 +7026,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "anyhow", "async-trait", @@ -7051,7 +7051,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "sandbox-driver-docker-config", "serde", @@ -7061,7 +7061,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "anyhow", "async-trait", @@ -7082,7 +7082,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "serde", "serde_json", @@ -7091,7 +7091,7 @@ dependencies = [ [[package]] name = "sandbox-driver-host" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "anyhow", "async-trait", @@ -7109,7 +7109,7 @@ dependencies = [ [[package]] name = "sandbox-driver-protocol" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "async-trait", "base64", @@ -7126,7 +7126,7 @@ dependencies = [ [[package]] name = "sandbox-driver-testing" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc#2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=583b72a82b1ed51bccb8016b227b10b181c1560d#583b72a82b1ed51bccb8016b227b10b181c1560d" dependencies = [ "async-trait", "sandbox-driver", diff --git a/Cargo.toml b/Cargo.toml index dd37855d0..06297849d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,14 +102,14 @@ futures-util = "0.3" # git failures, stop grace, snapshot ensure, ownership scope, testing doubles), to # move to main on merge. The CI plugin job installs the driver executables at the # same rev, read from this file. -sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } -sandbox-driver-testing = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "2acc6ebff37ec7ed1a2302b95aca6a6aba98a0fc" } +sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } +sandbox-driver-testing = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "583b72a82b1ed51bccb8016b227b10b181c1560d" } sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] } fork = "0.2" exec = "0.3" diff --git a/lib/components/fabro-sandbox/src/clone.rs b/lib/components/fabro-sandbox/src/clone.rs index 358baf2d4..2342361dd 100644 --- a/lib/components/fabro-sandbox/src/clone.rs +++ b/lib/components/fabro-sandbox/src/clone.rs @@ -5,10 +5,10 @@ //! credentials it carries, and how failures retry. The layout is fabro's: //! the repository checks out under `//` and the //! run works in `/`, a symlink to the checkout. An -//! exact commit goes through the driver's pinned clone; a tag pin runs -//! fabro's own init, fetch, and attach sequence through `Exec`, because a -//! tag must be fetched by its fully qualified ref so a same-named branch is -//! never consulted. Neither path ever falls back to the branch head. +//! exact commit or a tag is pinned by the driver's clone options, which +//! fetch a tag by its fully qualified ref so a same-named branch is never +//! consulted; fabro verifies the checked-out head afterwards. Neither path +//! ever falls back to the branch head. use std::time::Duration; diff --git a/lib/components/fabro-sandbox/src/driver_sandbox.rs b/lib/components/fabro-sandbox/src/driver_sandbox.rs index db5be371e..ef52b833b 100644 --- a/lib/components/fabro-sandbox/src/driver_sandbox.rs +++ b/lib/components/fabro-sandbox/src/driver_sandbox.rs @@ -483,6 +483,17 @@ impl RunSandbox { } } + /// The driver's git facet for this sandbox's checkout. Absent until a + /// pending sandbox is initialized, or when the provider has no git. + pub(crate) fn git(&self) -> crate::Result> { + self.handle()?.git().ok_or_else(|| { + crate::Error::message(format!( + "sandbox provider `{}` does not support git", + self.kind + )) + }) + } + fn search(&self) -> crate::Result> { self.handle()?.search().ok_or_else(|| { crate::Error::message(format!( @@ -1040,7 +1051,7 @@ impl RunSandbox { if !self.repo_cloned() { return Ok(None); } - sandbox::setup_git_via_exec(self, intent).await.map(Some) + sandbox::setup_git(self, intent).await.map(Some) } pub fn resume_setup_commands(&self, run_branch: &str) -> Vec { @@ -1078,7 +1089,7 @@ impl RunSandbox { if !has_origin { return Ok(PushReport::default()); } - return sandbox::git_push_via_exec(self, None, refspec, plan).await; + return sandbox::git_push(self, None, refspec, plan).await; }; if !workspace.repo_cloned() { return Ok(PushReport::default()); @@ -1087,7 +1098,7 @@ impl RunSandbox { .origin_url .get() .map(|origin_url| (&workspace.credentials, origin_url.as_str())); - sandbox::git_push_via_exec(self, credentials, refspec, plan).await + sandbox::git_push(self, credentials, refspec, plan).await } pub fn origin_url(&self) -> Option<&str> { diff --git a/lib/components/fabro-sandbox/src/error.rs b/lib/components/fabro-sandbox/src/error.rs index f63dc5d49..57c132f23 100644 --- a/lib/components/fabro-sandbox/src/error.rs +++ b/lib/components/fabro-sandbox/src/error.rs @@ -198,23 +198,40 @@ pub fn default_redacted_output_tail( ) -> Option { let mut current = Some(err); while let Some(err) = current { - if let Some(Error::Exec { result, .. }) = err.downcast_ref::() { - return result.default_redacted_output_tail(); + match err.downcast_ref::() { + Some(Error::Exec { result, .. }) => return result.default_redacted_output_tail(), + Some(Error::Driver(driver)) => { + if let Some(tail) = driver_output_tail(driver) { + return Some(tail); + } + } + _ => {} } - if let Some(sandbox_driver::Error::Exec(failure)) = - err.downcast_ref::() - { - return redacted_output_tail( - &String::from_utf8_lossy(failure.stdout()), - &String::from_utf8_lossy(failure.stderr()), - DEFAULT_EXEC_OUTPUT_TAIL_BYTES, - ); + if let Some(driver) = err.downcast_ref::() { + if let Some(tail) = driver_output_tail(driver) { + return Some(tail); + } } current = err.source(); } None } +/// The output a driver failure carries: a command that ran and failed, or +/// a git operation whose command output the driver kept as evidence. +fn driver_output_tail(error: &sandbox_driver::Error) -> Option { + let failure = match error { + sandbox_driver::Error::Exec(failure) => failure, + sandbox_driver::Error::Git(git) => git.output()?, + _ => return None, + }; + redacted_output_tail( + &String::from_utf8_lossy(failure.stdout()), + &String::from_utf8_lossy(failure.stderr()), + DEFAULT_EXEC_OUTPUT_TAIL_BYTES, + ) +} + pub fn display_for_log(err: &(dyn std::error::Error + 'static)) -> String { let mut rendered = render_with_causes(&err.to_string(), &collect_causes(err)); if let Some(tail) = default_redacted_output_tail(err) { diff --git a/lib/components/fabro-sandbox/src/git_retry.rs b/lib/components/fabro-sandbox/src/git_retry.rs index fd8047de6..9d4b78084 100644 --- a/lib/components/fabro-sandbox/src/git_retry.rs +++ b/lib/components/fabro-sandbox/src/git_retry.rs @@ -86,17 +86,6 @@ impl CredentialContext { } } -/// Whether a failure message has the 404/auth-failure shape GitHub produces -/// for both token-replication lag and a drifted or missing embedded token. -pub(crate) fn matches_auth_failure_hints(message: &str) -> bool { - GitFailureKind::from_message(message) == GitFailureKind::AuthRejected -} - -pub(crate) fn output_matches_auth_failure_hints(stderr: &str, stdout: &str) -> bool { - GitFailureKind::from_output(stderr.as_bytes(), stdout.as_bytes()) - == GitFailureKind::AuthRejected -} - /// What a classified git failure means for retrying with these credentials. /// /// The driver reads the failure; fabro decides. A remote that could not @@ -120,22 +109,13 @@ pub(crate) fn decide(kind: GitFailureKind, cred: CredentialContext) -> GitMessag } } -/// Classify a failed git operation by its rendered message. +/// Classify a failed git operation by its rendered message. For git that +/// ran outside a sandbox — the host-side repository probe and metadata +/// push — where the driver never saw the failure. pub(crate) fn classify_message(message: &str, cred: CredentialContext) -> GitMessageClass { decide(GitFailureKind::from_message(message), cred) } -pub(crate) fn classify_output( - stderr: &str, - stdout: &str, - cred: CredentialContext, -) -> GitMessageClass { - decide( - GitFailureKind::from_output(stderr.as_bytes(), stdout.as_bytes()), - cred, - ) -} - /// Classify a rendered git failure message, returning the retry reason when /// the failure is transient for these credentials. `None` means the failure /// is permanent or unrecognized. diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index 81a5f344e..22bf75e2f 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -65,7 +65,7 @@ pub use sandbox::{ ExecStreamingResult, GitRunInfo, GitSetupIntent, OutputCaptureStats, PushAttempt, PushError, PushReport, RefreshOutcome, RemoteCredentialAction, SandboxFile, SandboxWorkspaceLayout, StderrCollector, StdioProcess, StdioProcessHandle, StdioProcessTermination, - format_lines_numbered, redacted_output_tail, setup_git_via_exec, shell_quote, + format_lines_numbered, redacted_output_tail, setup_git, shell_quote, }; /// Driver types a run sandbox's file and search operations speak, and the /// network policy a [`SandboxOptions`] asks for, re-exported so consumers diff --git a/lib/components/fabro-sandbox/src/sandbox.rs b/lib/components/fabro-sandbox/src/sandbox.rs index 2a4361401..7d0dbd010 100644 --- a/lib/components/fabro-sandbox/src/sandbox.rs +++ b/lib/components/fabro-sandbox/src/sandbox.rs @@ -10,6 +10,7 @@ use fabro_github::token_source::TokenSnapshot; pub use fabro_types::run_event::GitCredentialAction as RemoteCredentialAction; use fabro_types::{CommandOutputStream, CommandTermination}; use fabro_util::shell; +use sandbox_driver::{Git as _, GitCheckoutOptions, GitFailureKind, GitPushOptions, Termination}; use serde::{Deserialize, Serialize}; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite}; use tokio::sync::Mutex as TokioMutex; @@ -556,41 +557,27 @@ pub fn shell_quote(s: &str) -> String { shell::shell_quote(s) } -/// Helper for sandbox implementations that manage git internally. -/// Executes git commands inside the sandbox to create a run branch. -pub async fn setup_git_via_exec( - sandbox: &RunSandbox, - intent: &GitSetupIntent, -) -> crate::Result { - // Get current branch name - let branch_result = sandbox - .exec_command("git rev-parse --abbrev-ref HEAD", 10_000, None, None, None) +/// Creates the run branch in the sandbox's checkout through the driver's +/// git facet: a new run branches from `HEAD`, a fork from the source run's +/// checkpoint. The branch is created at that base, or moved to it when an +/// earlier attempt already created it. +pub async fn setup_git(sandbox: &RunSandbox, intent: &GitSetupIntent) -> crate::Result { + let git = sandbox.git()?; + let repo = sandbox.working_directory().to_owned(); + let status = git + .status(&repo) .await - .map_err(|e| { - crate::Error::message(format!("git rev-parse --abbrev-ref HEAD failed: {e}")) - })?; - let base_branch = if branch_result.is_success() { - let name = branch_result.stdout.trim().to_string(); - if name.is_empty() || name == "HEAD" { - None - } else { - Some(name) - } - } else { - None - }; + .map_err(|error| crate::Error::context("git status", error))?; + let base_branch = status + .current_branch + .filter(|name| !name.is_empty() && name != "HEAD"); let (base_sha, branch_name) = match intent { GitSetupIntent::NewRun { run_id } => { - let sha_result = sandbox - .exec_command("git rev-parse HEAD", 10_000, None, None, None) - .await - .map_err(|e| crate::Error::context("git rev-parse HEAD", e))? - .into_result("git rev-parse HEAD")?; - ( - sha_result.stdout.trim().to_string(), - format!("fabro/run/{run_id}"), - ) + let head = status.head.ok_or_else(|| { + crate::Error::message("the repository has no commit to branch the run from") + })?; + (head, format!("fabro/run/{run_id}")) } GitSetupIntent::ForkFromCheckpoint { new_run_id, @@ -602,16 +589,14 @@ pub async fn setup_git_via_exec( } }; - let checkout_cmd = format!( - "git checkout -B {} {}", - shell_quote(&branch_name), - shell_quote(&base_sha) - ); - sandbox - .exec_command(&checkout_cmd, 10_000, None, None, None) - .await - .map_err(|e| crate::Error::context("git checkout -B", e))? - .into_result("git checkout -B")?; + git.checkout( + &repo, + &GitCheckoutOptions::new(&branch_name) + .create_or_reset() + .start_point(&base_sha), + ) + .await + .map_err(|error| crate::Error::context("git checkout -B", error))?; Ok(GitRunInfo { base_sha, @@ -706,40 +691,39 @@ pub struct PushError { pub error: crate::Error, } -/// Classify a failed push attempt by the failure's rendered output. +/// What a failed push attempt means for retrying. The driver classified +/// the failure; a push that did not run to completion (timed out or +/// cancelled) is never retried, because the remote may still be applying +/// it. fn classify_push_error(error: &crate::Error, cred: CredentialContext) -> Option { - let class = match error { - crate::Error::Exec { result, .. } if result.termination != CommandTermination::Exited => { + let driver = error.driver()?; + if let sandbox_driver::Error::Git(failure) = driver { + if failure + .output() + .is_some_and(|output| output.termination() != Termination::Exited) + { return None; } - crate::Error::Exec { result, .. } => { - git_retry::classify_output(&result.stderr, &result.stdout, cred) - } - other => git_retry::classify_message(&crate::display_for_log(other), cred), - }; - class.retry_reason() -} - -/// Whether a failed push attempt has the 404/auth-failure shape that a -/// drifted or missing embedded token also produces. -fn push_failure_looks_auth_shaped(error: &crate::Error) -> bool { - match error { - crate::Error::Exec { result, .. } => { - git_retry::output_matches_auth_failure_hints(&result.stderr, &result.stdout) - } - other => git_retry::matches_auth_failure_hints(&crate::display_for_log(other)), } + git_retry::classify_driver_failure(driver, cred) } -/// Helper for sandbox implementations that manage git internally. -/// -/// Pushes a refspec to origin via `exec_command` inside the sandbox, -/// retrying per `plan` with one pinned credential generation for the whole -/// operation. `credentials` is the provider's push-credential state plus the -/// origin URL; `None` pushes with whatever the remote already carries (the -/// local sandbox, or a workspace without managed credentials). +/// Whether a failed push attempt was rejected as unauthenticated, the shape +/// a drifted or missing embedded token also produces. +fn push_failure_looks_auth_shaped(error: &crate::Error) -> bool { + matches!( + error.driver(), + Some(sandbox_driver::Error::Git(failure)) if failure.kind() == GitFailureKind::AuthRejected + ) +} + +/// Pushes a refspec to origin through the driver's git facet, retrying per +/// `plan` with one pinned credential generation for the whole operation. +/// `credentials` is the provider's push-credential state plus the origin +/// URL; `None` pushes with whatever the remote already carries (the local +/// sandbox, or a workspace without managed credentials). #[tracing::instrument(name = "git_op", skip_all, fields(op = "push"))] -pub(crate) async fn git_push_via_exec( +pub(crate) async fn git_push( sandbox: &RunSandbox, credentials: Option<(&PushCredentialState, &str)>, refspec: &str, @@ -750,6 +734,16 @@ pub(crate) async fn git_push_via_exec( let start = time::Instant::now(); let deadline = plan.effective_deadline(start); + let git = match sandbox.git() { + Ok(git) => git, + Err(error) => { + return Err(PushError { + report: PushReport::default(), + error, + }); + } + }; + let repo = sandbox.working_directory().to_owned(); // The lease pins one token generation and owns the embed mutex for the // whole operation; no concurrent refresh can re-embed mid-operation, and @@ -782,7 +776,6 @@ pub(crate) async fn git_push_via_exec( let mut attempts: Vec = Vec::new(); let mut force_reembed = false; let mut drift_repaired = false; - let cmd = format!("{GIT} push origin {}", shell_quote(refspec)); let label = format!("git push origin {refspec}"); loop { @@ -824,17 +817,17 @@ pub(crate) async fn git_push_via_exec( }; let remaining = attempt_deadline.saturating_duration_since(time::Instant::now()); - let timeout_ms = u64::try_from(remaining.as_millis()).unwrap_or(u64::MAX); - if timeout_ms == 0 { + if remaining.is_zero() { return Err(push_deadline_error(attempts, "before running git push")); } - let push_result = match sandbox - .exec_command(&cmd, timeout_ms, None, None, None) + let mut options = GitPushOptions::default(); + options.remote = Some("origin".to_owned()); + options.refspec = Some(refspec.to_owned()); + options.timeout = Some(remaining); + let push_result = git + .push(&repo, &options) .await - { - Ok(result) => result.into_result(&label).map(|_| ()), - Err(err) => Err(crate::Error::context(label.clone(), err)), - }; + .map_err(|error| crate::Error::context(label.clone(), error)); match push_result { Ok(()) => { @@ -1002,7 +995,10 @@ mod push_tests { .map_or_else(ok_exec, driver_result), ); } - assert!(script.contains("push origin"), "unexpected exec: {script}"); + assert!( + script.contains("'push' 'origin'"), + "unexpected exec: {script}" + ); Some(driver_result( pushes .lock() @@ -1022,7 +1018,7 @@ mod push_tests { fn push_count(&self) -> usize { self.commands() .iter() - .filter(|command| command.contains("push origin")) + .filter(|command| command.contains("'push' 'origin'")) .count() } @@ -1144,7 +1140,7 @@ mod push_tests { ok_fabro_exec(), ]); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1189,7 +1185,7 @@ mod push_tests { ok_fabro_exec(), ]); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1220,7 +1216,7 @@ mod push_tests { ok_fabro_exec(), ]); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1252,7 +1248,7 @@ mod push_tests { "fatal: Authentication failed for 'https://github.com/fabro-testing/repo'", )]); - let push_error = git_push_via_exec( + let push_error = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1280,7 +1276,7 @@ mod push_tests { seed_clone_token(&state).await; let sandbox = ScriptedGitSandbox::new(vec![ok_fabro_exec()]); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1309,7 +1305,7 @@ mod push_tests { let (state, _minter) = minting_state(vec![MintAction::Error("mint failed")]); let sandbox = ScriptedGitSandbox::new(vec![]); - let push_error = git_push_via_exec( + let push_error = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1339,7 +1335,7 @@ mod push_tests { ok_fabro_exec(), ]); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1381,7 +1377,7 @@ mod push_tests { vec![failed_exec("error: could not lock config file")], ); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1424,7 +1420,7 @@ mod push_tests { seed_clone_token(&state).await; let sandbox = ScriptedGitSandbox::with_set_url_results(vec![], vec![timed_out_exec()]); - let push_error = git_push_via_exec( + let push_error = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1455,7 +1451,7 @@ mod push_tests { ok_fabro_exec(), ]); - let report = git_push_via_exec( + let report = git_push( &sandbox.run, Some((&state, ORIGIN)), REFSPEC, @@ -1484,7 +1480,7 @@ mod push_tests { async fn push_without_managed_credentials_reports_no_token() { let sandbox = ScriptedGitSandbox::new(vec![ok_fabro_exec()]); - let report = git_push_via_exec(&sandbox.run, None, REFSPEC, &RetryPlan::checkpoint_push()) + let report = git_push(&sandbox.run, None, REFSPEC, &RetryPlan::checkpoint_push()) .await .expect("push succeeds"); @@ -1499,7 +1495,7 @@ mod push_tests { "fatal: Authentication failed for 'https://github.com/fabro-testing/repo'", )]); - let push_error = git_push_via_exec(&sandbox.run, None, REFSPEC, &RetryPlan::publish_push()) + let push_error = git_push(&sandbox.run, None, REFSPEC, &RetryPlan::publish_push()) .await .expect_err("no credentials to wait on"); @@ -1511,7 +1507,7 @@ mod push_tests { async fn timed_out_push_is_not_retried_while_the_remote_process_may_still_run() { let sandbox = ScriptedGitSandbox::new(vec![timed_out_exec()]); - let push_error = git_push_via_exec(&sandbox.run, None, REFSPEC, &RetryPlan::publish_push()) + let push_error = git_push(&sandbox.run, None, REFSPEC, &RetryPlan::publish_push()) .await .expect_err("an unconfirmed timeout must fail without another push"); @@ -1528,7 +1524,7 @@ mod push_tests { let mut plan = RetryPlan::checkpoint_push(); plan.max_elapsed = Some(Duration::from_secs(1)); - let push_error = git_push_via_exec(&sandbox.run, Some((&state, ORIGIN)), REFSPEC, &plan) + let push_error = git_push(&sandbox.run, Some((&state, ORIGIN)), REFSPEC, &plan) .await .expect_err("credential acquisition must stop at the operation deadline"); @@ -1543,7 +1539,7 @@ mod push_tests { let mut plan = RetryPlan::checkpoint_push(); plan.max_elapsed = Some(Duration::ZERO); - let push_error = git_push_via_exec(&sandbox.run, None, REFSPEC, &plan) + let push_error = git_push(&sandbox.run, None, REFSPEC, &plan) .await .expect_err("an expired operation must stop before exec");