From 3818368876f4dffad0a2b1c3eb82d91719fda968 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 18:45:50 +0000 Subject: [PATCH 1/3] Initial plan From ef28781955a45e4736fea4ad0aa117620b4ede19 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 18:53:15 +0000 Subject: [PATCH 2/3] Fix panic when --policy uses a commit SHA refspec Detect full-length hex commit SHAs (SHA-1: 40 chars, SHA-256: 64 chars) and skip `with_ref_name` which only accepts symbolic refs. Instead, clone the repo fully and run `git checkout ` after checkout to switch to the requested commit. Agent-Logs-Url: https://github.com/open-telemetry/weaver/sessions/80002501-91c1-453a-990b-88b6664f0edf Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com> --- crates/weaver_common/src/vdir.rs | 76 +++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/crates/weaver_common/src/vdir.rs b/crates/weaver_common/src/vdir.rs index 880e0b577..f247217a6 100644 --- a/crates/weaver_common/src/vdir.rs +++ b/crates/weaver_common/src/vdir.rs @@ -276,6 +276,16 @@ const TAR_GZ_EXT: &str = ".tar.gz"; /// The extension for a zip archive. const ZIP_EXT: &str = ".zip"; +/// Returns `true` if `s` looks like a full-length hex commit SHA. +/// +/// Git uses SHA-1 (40 hex chars) or SHA-256 (64 hex chars). This helper is +/// intentionally strict: it only matches full-length hashes so that short +/// branch/tag names that happen to be hex (e.g. `deadbeef`) are not +/// misidentified. +fn is_commit_sha(s: &str) -> bool { + (s.len() == 40 || s.len() == 64) && s.bytes().all(|b| b.is_ascii_hexdigit()) +} + /// Regex to parse a virtual directory path string. /// /// Supports the following general format: `source[@refspec][\[sub_folder]]` @@ -649,6 +659,10 @@ impl VirtualDirectory { message: e.to_string(), })?; + // Determine whether the refspec is a commit SHA. `with_ref_name` only + // accepts symbolic refs (branches/tags) and panics on raw object IDs. + let is_sha = refspec.as_ref().is_some_and(|r| is_commit_sha(r)); + let mut fetch = if refspec.is_none() { prepare.with_shallow(Shallow::DepthAtRemote( NonZeroU32::new(1).expect("1 is not zero"), @@ -656,7 +670,9 @@ impl VirtualDirectory { } else { prepare } - .with_ref_name(refspec.as_ref()) + // Only pass the refspec to `with_ref_name` when it is a symbolic ref. + // Commit SHAs are handled after checkout via `git checkout`. + .with_ref_name(if is_sha { None } else { refspec.as_ref() }) .map_err(|e| GitError { repo_url: url.to_owned(), message: e.to_string(), @@ -676,6 +692,27 @@ impl VirtualDirectory { message: e.to_string(), })?; + // When the refspec is a commit SHA, the clone fetched all refs and + // checked out the default branch. Now switch to the requested commit. + if is_sha { + let sha = refspec.as_ref().expect("is_sha implies Some"); + let output = std::process::Command::new("git") + .args(["checkout", sha]) + .current_dir(&tmp_path) + .output() + .map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("failed to run `git checkout {sha}`: {e}"), + })?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(GitError { + repo_url: url.to_owned(), + message: format!("git checkout {sha} failed: {stderr}"), + }); + } + } + // Determines the final path to the repo taking into account the sub_folder. let path = if let Some(sub_folder) = sub_folder { let path_to_repo = tmp_path.join(sub_folder); @@ -1629,4 +1666,41 @@ mod tests { let err = find_asset_url(&no_url, "manifest.yaml", "v1", "orig").expect_err("missing url"); assert!(matches!(&err, RemoteFileDownloadFailed { error, .. } if error.contains("'url'"))); } + + #[test] + fn test_is_commit_sha() { + use super::is_commit_sha; + + // Valid SHA-1 (40 hex chars) + assert!(is_commit_sha( + "d84341cf20a1fef1a833ef44d318c41a770e6e64" + )); + assert!(is_commit_sha( + "0000000000000000000000000000000000000000" + )); + assert!(is_commit_sha( + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + )); + // Valid SHA-256 (64 hex chars) + assert!(is_commit_sha( + "d84341cf20a1fef1a833ef44d318c41a770e6e64d84341cf20a1fef1a833ef44" + )); + + // Too short / too long + assert!(!is_commit_sha("d84341cf")); + assert!(!is_commit_sha("d84341cf20a1fef1a833ef44d318c41a770e6e6")); // 39 chars + assert!(!is_commit_sha( + "d84341cf20a1fef1a833ef44d318c41a770e6e640" + )); // 41 chars + + // Non-hex characters + assert!(!is_commit_sha( + "g84341cf20a1fef1a833ef44d318c41a770e6e64" + )); + + // Symbolic refs + assert!(!is_commit_sha("main")); + assert!(!is_commit_sha("v1.0.0")); + assert!(!is_commit_sha("refs/heads/main")); + } } From dcf3e8e3ca589e94e4869325457d036a09b4b8da Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 19:24:29 +0000 Subject: [PATCH 3/3] Replace std::process::Command with gix APIs for SHA checkout Use gix's ObjectId::from_hex, find_object, peel_to_tree, index_from_tree, and worktree::state::checkout instead of shelling out to `git checkout`. For the SHA path, skip main_worktree() entirely and checkout the requested commit directly, avoiding a double checkout. Agent-Logs-Url: https://github.com/open-telemetry/weaver/sessions/3f847f32-e10a-4518-8deb-b4f2d0d99f88 Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com> --- crates/weaver_common/src/vdir.rs | 101 ++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 23 deletions(-) diff --git a/crates/weaver_common/src/vdir.rs b/crates/weaver_common/src/vdir.rs index f247217a6..afd18d0df 100644 --- a/crates/weaver_common/src/vdir.rs +++ b/crates/weaver_common/src/vdir.rs @@ -671,46 +671,34 @@ impl VirtualDirectory { prepare } // Only pass the refspec to `with_ref_name` when it is a symbolic ref. - // Commit SHAs are handled after checkout via `git checkout`. + // Commit SHAs are handled via `checkout_sha` after fetching. .with_ref_name(if is_sha { None } else { refspec.as_ref() }) .map_err(|e| GitError { repo_url: url.to_owned(), message: e.to_string(), })?; - let (mut prepare, _outcome) = fetch + let (mut checkout, _outcome) = fetch .fetch_then_checkout(progress::Discard, &AtomicBool::new(false)) .map_err(|e| GitError { repo_url: url.to_owned(), message: e.to_string(), })?; - let (_repo, _outcome) = prepare - .main_worktree(progress::Discard, &AtomicBool::new(false)) - .map_err(|e| GitError { - repo_url: url.to_owned(), - message: e.to_string(), - })?; - - // When the refspec is a commit SHA, the clone fetched all refs and - // checked out the default branch. Now switch to the requested commit. if is_sha { + // For commit SHAs we skip `main_worktree()` (which would checkout + // the default branch) and instead checkout the requested commit + // directly using gix APIs. let sha = refspec.as_ref().expect("is_sha implies Some"); - let output = std::process::Command::new("git") - .args(["checkout", sha]) - .current_dir(&tmp_path) - .output() + let repo = checkout.persist(); + Self::checkout_sha(&repo, sha, url)?; + } else { + let _repo = checkout + .main_worktree(progress::Discard, &AtomicBool::new(false)) .map_err(|e| GitError { repo_url: url.to_owned(), - message: format!("failed to run `git checkout {sha}`: {e}"), + message: e.to_string(), })?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(GitError { - repo_url: url.to_owned(), - message: format!("git checkout {sha} failed: {stderr}"), - }); - } } // Determines the final path to the repo taking into account the sub_folder. @@ -738,6 +726,73 @@ impl VirtualDirectory { }) } + /// Checkout a specific commit SHA in a cloned repository using gix APIs. + /// + /// Resolves `sha` to a tree, builds an index, and writes the worktree. + /// This avoids the `main_worktree()` path which can only checkout HEAD or + /// a symbolic ref. + fn checkout_sha(repo: &gix::Repository, sha: &str, url: &str) -> Result<(), Error> { + let workdir = repo.workdir().ok_or_else(|| GitError { + repo_url: url.to_owned(), + message: "repository has no worktree".to_owned(), + })?; + + let id = gix::ObjectId::from_hex(sha.as_bytes()).map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("invalid commit SHA {sha}: {e}"), + })?; + + let tree_id = repo + .find_object(id) + .map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("commit {sha} not found: {e}"), + })? + .peel_to_tree() + .map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("failed to peel {sha} to tree: {e}"), + })? + .id; + + let mut index = repo.index_from_tree(&tree_id).map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("failed to build index from tree: {e}"), + })?; + + let mut opts = repo + .checkout_options(gix::worktree::stack::state::attributes::Source::IdMapping) + .map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("failed to get checkout options: {e}"), + })?; + opts.destination_is_initially_empty = true; + + let _outcome = gix::worktree::state::checkout( + &mut index, + workdir, + repo.objects.clone().into_arc().map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("failed to create object store handle: {e}"), + })?, + &progress::Discard, + &progress::Discard, + &AtomicBool::new(false), + opts, + ) + .map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("checkout failed: {e}"), + })?; + + index.write(Default::default()).map_err(|e| GitError { + repo_url: url.to_owned(), + message: format!("failed to write index: {e}"), + })?; + + Ok(()) + } + /// Create a new `VirtualDirectory` from a local archive. /// The archive can be in `.tar.gz` or `.zip` format. /// The sub_folder is used to filter the entries inside the archive to unpack.