fix(storage): fetch full closure during repair

Use Git's noop negotiation algorithm only for identifier-family integrity fetches. A corrupt family can retain refs whose objects are absent; normal negotiation treats those refs as local have tips, requests an empty pack, and then fails connectivity even when the remote advertises the missing commit.\n\nThe dedicated integrity fetch role forces the accepted clone source to send the requested object's complete closure into the shared family object directory. Primary and hedged proactive fetches deliberately retain normal negotiation so routine transfers stay incremental.\n\nValidated against a quarantined production canary object, with strict all-target Clippy, all git::integrity unit tests, and a regression test for integrity-only noop selection.
This commit is contained in:
DanConwayDev
2026-08-18 13:29:47 +00:00
parent 77b7c2c7bf
commit 1f5825aa5e
3 changed files with 40 additions and 2 deletions
+8 -1
View File
@@ -446,7 +446,14 @@ impl FamilyRepairSource for crate::purgatory::sync::RealSyncContext {
url: &str,
oids: &[String],
) -> Result<Vec<String>> {
crate::purgatory::sync::SyncContext::fetch_oids(self, target_view, url, oids).await
crate::purgatory::sync::SyncContext::fetch_oids_with_role(
self,
target_view,
url,
oids,
crate::purgatory::sync::GitFetchRole::Integrity,
)
.await
}
}
+31
View File
@@ -21,6 +21,7 @@ use std::time::{Duration, Instant};
pub enum GitFetchRole {
Primary,
Hedge,
Integrity,
}
impl GitFetchRole {
@@ -28,6 +29,7 @@ impl GitFetchRole {
match self {
Self::Primary => "primary",
Self::Hedge => "hedge",
Self::Integrity => "integrity",
}
}
}
@@ -465,6 +467,7 @@ fn hardened_git_command(
resolve_pin: Option<&str>,
auth_header: Option<&str>,
object_directory: Option<&Path>,
negotiation_algorithm: Option<&str>,
args: &[String],
) -> Command {
let mut command = Command::new("git");
@@ -475,6 +478,11 @@ fn hardened_git_command(
.arg("http.proxy=")
.arg("-c")
.arg("credential.helper=");
if let Some(algorithm) = negotiation_algorithm {
command
.arg("-c")
.arg(format!("fetch.negotiationAlgorithm={algorithm}"));
}
if let Some(pin) = resolve_pin {
command.arg("-c").arg(format!("http.curloptResolve={pin}"));
}
@@ -831,6 +839,15 @@ fn is_object_missing_error(stderr: &str) -> bool {
|| stderr.contains("Server does not allow request for unadvertised object")
}
/// Integrity repair starts from a repository whose refs may name objects that
/// are already missing. Normal fetch negotiation walks those refs as local
/// "have" tips, which can make Git request no pack and then fail its own
/// connectivity check. Repair fetches therefore advertise no local haves and
/// ask the source for the complete requested object closure.
fn fetch_negotiation_algorithm(role: GitFetchRole) -> Option<&'static str> {
(role == GitFetchRole::Integrity).then_some("noop")
}
/// Record a remote failure with the naughty list tracker (only error
/// categories the tracker classifies as persistent are recorded).
fn record_remote_failure(
@@ -1076,6 +1093,7 @@ impl SyncContext for RealSyncContext {
resolve_pin.as_deref(),
fresh_auth_header().as_deref(),
None,
None,
&ls_remote_args,
),
&domain,
@@ -1147,6 +1165,7 @@ impl SyncContext for RealSyncContext {
resolve_pin.as_deref(),
fresh_auth_header().as_deref(),
family_objects,
fetch_negotiation_algorithm(role),
&args,
),
&domain,
@@ -1218,6 +1237,7 @@ impl SyncContext for RealSyncContext {
resolve_pin.as_deref(),
fresh_auth_header().as_deref(),
family_objects,
fetch_negotiation_algorithm(role),
&args,
),
&domain,
@@ -1754,6 +1774,17 @@ mod fetch_helper_tests {
));
}
#[test]
fn integrity_fetches_disable_local_have_negotiation() {
assert_eq!(fetch_negotiation_algorithm(GitFetchRole::Primary), None);
assert_eq!(fetch_negotiation_algorithm(GitFetchRole::Hedge), None);
assert_eq!(
fetch_negotiation_algorithm(GitFetchRole::Integrity),
Some("noop")
);
assert_eq!(GitFetchRole::Integrity.as_str(), "integrity");
}
#[test]
fn miss_memo_is_stable_order_independent_invalidated_and_bounded() {
let first = HashSet::from(["b".to_string(), "a".to_string()]);
+1 -1
View File
@@ -13,7 +13,7 @@ mod r#loop;
mod queue;
mod throttle;
pub use context::{ProcessResult, RealSyncContext, SyncContext};
pub use context::{GitFetchRole, ProcessResult, RealSyncContext, SyncContext};
pub use functions::{
get_throttled_domains_with_untried_urls, sync_identifier, sync_identifier_from_url,
sync_identifier_next_url, ThrottledDomainInfo,