refactor(sandbox): drop dead helpers and tidy clone-based code

- Delete unused `detect_clone_params` and `GitCloneParams` (the clone-based refactor sources clone params from the run spec, not the worker cwd).
- Add `DaytonaSandbox::repo_cloned()` accessor mirroring Docker; replace five inline `OnceCell` reads.
- Inline `sanitize_origin_url` one-liner wrapper in `manifest_builder`.
- Drop unused `pub` on `docker::WORKING_DIRECTORY`.
- Convert `cleanup` early-return to `let-else` and remove a `Some(...).expect(...)` round-trip in `decide_clone`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-26 19:16:46 -04:00
parent 8365a28e7c
commit 498242f4f7
No known key found for this signature in database
5 changed files with 13 additions and 39 deletions

View file

@ -500,15 +500,11 @@ fn build_manifest_git(repo_path: &Path) -> Option<types::ManifestGit> {
Some(types::ManifestGit {
branch,
clean,
origin_url: sanitize_origin_url(&origin_url),
origin_url: fabro_github::normalize_repo_origin_url(&origin_url),
sha,
})
}
fn sanitize_origin_url(origin_url: &str) -> String {
fabro_github::normalize_repo_origin_url(origin_url)
}
fn normalize_absolute_path(base_dir: &Path, reference: &str) -> Option<PathBuf> {
let path = Path::new(reference);
if path.is_absolute() || reference.starts_with('~') {

View file

@ -43,8 +43,7 @@ pub(crate) fn decide_clone(
});
};
let origin_url = clean_clone_origin_for_record(Some(origin_url))
.expect("origin URL is present after filter");
let origin_url = fabro_github::normalize_repo_origin_url(origin_url);
if let Err(err) = fabro_github::parse_github_owner_repo(&origin_url) {
return Err(format!(
"Clone-based sandboxes currently support GitHub repository origins only: {err}"

View file

@ -206,6 +206,10 @@ impl DaytonaSandbox {
.ok_or_else(|| "Daytona sandbox not initialized — call initialize() first".to_string())
}
fn repo_cloned(&self) -> bool {
self.repo_cloned.get().copied().unwrap_or(false)
}
/// Build `SandboxBaseParams` from config, generating a unique sandbox name.
fn base_params(&self) -> daytona_sdk::SandboxBaseParams {
let name = if let Some(ref id) = self.run_id {
@ -341,27 +345,6 @@ impl DaytonaSandbox {
}
}
/// Parameters for cloning a git repo into the sandbox during initialization.
#[derive(Clone, Debug)]
pub struct GitCloneParams {
/// Clean HTTPS URL (no embedded credentials).
pub url: String,
/// Branch to clone. If None, uses the remote's default.
pub branch: Option<String>,
}
pub fn detect_clone_params(cwd: &Path) -> Option<GitCloneParams> {
let (detected_url, branch) = match detect_repo_info(cwd) {
Ok(info) => info,
Err(err) => {
tracing::warn!("No git repo detected for sandbox clone: {err}");
return None;
}
};
let url = fabro_github::ssh_url_to_https(&detected_url);
Some(GitCloneParams { url, branch })
}
/// Detect the git remote URL and current branch from a local repository.
///
/// Uses `git2` to discover the repo at `path`, reads the `origin` remote URL
@ -780,14 +763,14 @@ impl Sandbox for DaytonaSandbox {
}
async fn setup_git_for_run(&self, run_id: &str) -> Result<Option<crate::GitRunInfo>, String> {
if !self.repo_cloned.get().copied().unwrap_or(false) {
if !self.repo_cloned() {
return Ok(None);
}
crate::setup_git_via_exec(self, run_id).await.map(Some)
}
fn resume_setup_commands(&self, run_branch: &str) -> Vec<String> {
if !self.repo_cloned.get().copied().unwrap_or(false) {
if !self.repo_cloned() {
return Vec::new();
}
vec![format!(
@ -798,7 +781,7 @@ impl Sandbox for DaytonaSandbox {
}
async fn git_push_branch(&self, branch: &str) -> bool {
if !self.repo_cloned.get().copied().unwrap_or(false) {
if !self.repo_cloned() {
return false;
}
crate::git_push_via_exec(self, branch).await
@ -825,7 +808,7 @@ impl Sandbox for DaytonaSandbox {
}
fn origin_url(&self) -> Option<&str> {
if !self.repo_cloned.get().copied().unwrap_or(false) {
if !self.repo_cloned() {
return None;
}
self.origin_url.get().map(String::as_str)
@ -852,7 +835,7 @@ impl Sandbox for DaytonaSandbox {
}
async fn refresh_push_credentials(&self) -> Result<(), String> {
if !self.repo_cloned.get().copied().unwrap_or(false) {
if !self.repo_cloned() {
return Ok(());
}
let Some(origin_url) = self.origin_url.get() else {

View file

@ -28,7 +28,7 @@ use crate::{
format_lines_numbered, shell_quote,
};
pub const WORKING_DIRECTORY: &str = "/workspace";
const WORKING_DIRECTORY: &str = "/workspace";
const MANAGED_LABEL: &str = "sh.fabro.managed";
const RUN_ID_LABEL: &str = "sh.fabro.run_id";
@ -899,9 +899,7 @@ impl Sandbox for DockerSandbox {
});
let start = Instant::now();
let container_id = if let Some(id) = self.container_id.get() {
id.clone()
} else {
let Some(container_id) = self.container_id.get().cloned() else {
let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX);
self.emit(SandboxEvent::CleanupCompleted {
provider: "docker".into(),

View file

@ -26,8 +26,6 @@ pub mod daytona;
#[cfg(any(test, feature = "test-support"))]
pub mod test_support;
#[cfg(feature = "daytona")]
pub use daytona::detect_clone_params;
#[cfg(feature = "docker")]
pub use docker::{DockerSandbox, DockerSandboxOptions};
pub use local::LocalSandbox;