From 11cb2e71f207f162648eb164b6a3d0fff7a39fb5 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Sun, 17 May 2026 15:45:27 -0500 Subject: [PATCH 1/5] feat(rescue): --from reconstructs evicted packs from local clone Closes the documented future-version gap (main.rs:135-137, rescue-demos.yml:7-13): when a bundle's pack has been evicted from the gateway AND from any peer reachable via the ring, the rescue would fail with "GET pack ... 1 bundle(s) failed to rescue" and there was no recovery path -- the rescue could only re-PUT what the gateway still had cached. Concrete incident (2026-05-14 through 2026-05-17): bundle 2345ec0f on freenet:99TmCayXn6Tm/freenet-git (the self-mirror contract) had its underlying pack 7c30464721 evicted during normal LRU pressure on the gateway. Every rescue run since failed for this bundle while the other 26 bundles succeeded -- visible as recurring Matrix rescue-demos alerts. Verified empirically before implementing: git pack-objects produces byte-for-byte identical output for the same object set + git version, so a local clone of the upstream repo can rebuild the exact bytes the original push uploaded. Tested against the actual failing bundle: `git pack-objects` on the (000c4fde, c682079a] symmetric difference produced BLAKE3 7c30464721... matching the contract's expected hash. # Implementation New `local_pack` module (`crates/freenet-git/src/local_pack.rs`): - Reads `bundle-tip:` extensions from the contract state to learn each bundle's tip commit. - Sorts tips chronologically by commit timestamp. - For each consecutive (prev_tip, new_tip) pair, runs `git pack-objects` on the symmetric difference and BLAKE3s the output, building a `pack_hash -> pack_bytes` lookup map. - Scoped to SinglePack + history mode. ChunkedPack reconstruction needs the original chunk_size (not in contract metadata); snapshot mode's force-pushed orphan commits don't have meaningful (prev, new) ranges. Both fall through to the existing GET-only path; this is the same as the pre-0.1.23 behavior for those cases. In `rescue_pack` (main.rs): on `wsclient::get_pack` failure, look up the expected pack hash in the local-reconstruction map; if present, PUT those bytes. If `--from` wasn't passed the map is empty, so the lookup is a cheap no-op for the legacy invocation shape. # Workflow update `rescue-demos.yml` now clones the upstream GitHub repo for history- mode cells (`freenet-stdlib`, `freenet-git` self-mirror) via actions/checkout@v4 and passes `--from upstream/.git` to the rescue. Snapshot-mode cells (`freenet-core`) skip the clone since `--from` can't help with their content. The CLI flag detection (`grep -q -- '--from'`) keeps the workflow backwards-compatible with pre-0.1.23 freenet-git releases: the clone is fetched but `--from` is silently omitted from the rescue invocation. Once 0.1.23+ is the published floor on crates.io, the probe collapses to an unconditional pass. # Tests `local_pack` module tests verify SinglePack/ChunkedPack distinction and the empty-state path (no bundle-tip extensions = no map entries, not an error). The byte-for-byte reproducibility claim is verified in the commit message above against the live failing bundle and will be re-verified when the next scheduled rescue run consumes the 0.1.23 release. # Release Version bump 0.1.22 -> 0.1.23. Publish after merge so the rescue-demos workflow's `cargo install freenet-git --locked` picks up the new binary automatically. [AI-assisted - Claude] --- .github/workflows/rescue-demos.yml | 39 +++- Cargo.lock | 12 +- Cargo.toml | 2 +- crates/freenet-git/src/lib.rs | 1 + crates/freenet-git/src/local_pack.rs | 267 +++++++++++++++++++++++++++ crates/freenet-git/src/main.rs | 97 +++++++++- 6 files changed, 401 insertions(+), 17 deletions(-) create mode 100644 crates/freenet-git/src/local_pack.rs diff --git a/.github/workflows/rescue-demos.yml b/.github/workflows/rescue-demos.yml index 4ead393..e15c2f1 100644 --- a/.github/workflows/rescue-demos.yml +++ b/.github/workflows/rescue-demos.yml @@ -55,10 +55,13 @@ jobs: include: - repo: "freenet:3GEERif5ihbf/freenet-core" only_current_tips: true # snapshot mode + github_repo: "" # --from not supported in snapshot mode - repo: "freenet:96rknpy1GYhZ/freenet-stdlib" only_current_tips: false # history mode + github_repo: "freenet/freenet-stdlib" - repo: "freenet:99TmCayXn6Tm/freenet-git" only_current_tips: false # history mode (self-mirror) + github_repo: "freenet/freenet-git" steps: - name: Install Rust uses: dtolnay/rust-toolchain@stable @@ -66,11 +69,32 @@ jobs: - name: Install freenet-git from crates.io run: cargo install freenet-git --locked + # Clone the upstream GitHub repo so rescue can reconstruct missing + # packs from local objects via `--from`. Only history-mode cells + # get a clone; snapshot-mode cells (freenet-core) skip this step + # because `--from` doesn't help with force-pushed orphan commits. + # + # The clone has to be deep enough to contain every commit the + # contract's bundle-tip extensions reference -- which can be + # arbitrarily far back in history depending on how many mirror + # pushes have accumulated. Use a full clone (no --depth) so any + # historical bundle's tip is reachable. For freenet-stdlib and + # freenet-git this is small (a few MiB); for larger history-mode + # repos in the future this should be revisited. + - name: Clone upstream for --from + if: matrix.github_repo != '' + uses: actions/checkout@v4 + with: + repository: ${{ matrix.github_repo }} + path: upstream + fetch-depth: 0 + - name: Rescue env: REPO: ${{ matrix.repo }} WS_URL: ${{ secrets.FREENET_GIT_WS_URL }} ONLY_CURRENT_TIPS: ${{ matrix.only_current_tips }} + GITHUB_REPO: ${{ matrix.github_repo }} run: | set -euo pipefail # Probe the installed binary for --only-current-tips support @@ -112,8 +136,21 @@ jobs: # Pre-0.1.19 binary: outer loop is serial, nothing to set. parallel_args="" fi + # --from (0.1.23+) reconstructs missing pack bytes + # from a local clone when the gateway has evicted them, fixing + # the "1 bundle(s) failed to rescue: GET pack ..." class of + # failures (freenet-git#54 + associated rescue-demos rotation). + # Only history-mode cells get a clone (see matrix matrix + # `github_repo` field) -- snapshot-mode cells fall through to + # the GET-only path because `--from` can't help with force- + # pushed orphan commits whose ranges aren't recorded in + # contract metadata. + from_args="" + if [ -n "$GITHUB_REPO" ] && freenet-git rescue --help 2>&1 | grep -q -- '--from'; then + from_args="--from upstream/.git" + fi # shellcheck disable=SC2086 - freenet-git rescue "$REPO" --ws-url "$WS_URL" $extra $parallel_args + freenet-git rescue "$REPO" --ws-url "$WS_URL" $extra $parallel_args $from_args # Per the GH-Actions failure-notification model: matrix cell # failures show up in the run summary but do not (by default) email diff --git a/Cargo.lock b/Cargo.lock index 057bd00..f43c26e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -619,7 +619,7 @@ checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" [[package]] name = "freenet-git" -version = "0.1.22" +version = "0.1.23" dependencies = [ "anyhow", "assert_cmd", @@ -650,7 +650,7 @@ dependencies = [ [[package]] name = "freenet-git-encoding" -version = "0.1.22" +version = "0.1.23" dependencies = [ "blake3", "ed25519-dalek", @@ -661,7 +661,7 @@ dependencies = [ [[package]] name = "freenet-git-identity" -version = "0.1.22" +version = "0.1.23" dependencies = [ "bincode", "blake3", @@ -680,7 +680,7 @@ dependencies = [ [[package]] name = "freenet-git-pack-contract" -version = "0.1.22" +version = "0.1.23" dependencies = [ "blake3", "freenet-stdlib", @@ -688,7 +688,7 @@ dependencies = [ [[package]] name = "freenet-git-repo-contract" -version = "0.1.22" +version = "0.1.23" dependencies = [ "bincode", "ed25519-dalek", @@ -698,7 +698,7 @@ dependencies = [ [[package]] name = "freenet-git-types" -version = "0.1.22" +version = "0.1.23" dependencies = [ "bincode", "blake3", diff --git a/Cargo.toml b/Cargo.toml index 1e32b58..239db44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ members = [ ] [workspace.package] -version = "0.1.22" +version = "0.1.23" edition = "2021" rust-version = "1.86" license = "LGPL-3.0-only" diff --git a/crates/freenet-git/src/lib.rs b/crates/freenet-git/src/lib.rs index 2259248..3a55803 100644 --- a/crates/freenet-git/src/lib.rs +++ b/crates/freenet-git/src/lib.rs @@ -6,6 +6,7 @@ pub mod chunked; pub mod ids; +pub mod local_pack; pub mod pack_cache; pub mod state_init; pub mod url; diff --git a/crates/freenet-git/src/local_pack.rs b/crates/freenet-git/src/local_pack.rs new file mode 100644 index 0000000..b5d14ff --- /dev/null +++ b/crates/freenet-git/src/local_pack.rs @@ -0,0 +1,267 @@ +//! Pack reconstruction from a local git clone. +//! +//! Powers `freenet-git rescue --from `: when a bundle's pack +//! has been evicted from the gateway AND from any peer reachable via +//! the ring, the rescue would otherwise be stuck (the previous behavior: +//! fail loudly, "1 bundle(s) failed to rescue"). With a local clone of +//! the same repo, we can rebuild the exact same pack bytes locally — +//! `git pack-objects` is byte-for-byte deterministic given the same +//! object set and same git version (verified empirically against the +//! freenet-git self-mirror, 2026-05-17, where pack +//! `7c30464721e743061a50a66fa21c57c9e25e2e57732046223554c28ffaad2c2a` +//! was reproduced from `000c4fde..c682079a` with matching BLAKE3). +//! +//! # Scope (initial implementation) +//! +//! - **History mode only**: snapshot-mode mirrors force-push fresh +//! orphan commits per run, so the relationship between bundle tips +//! and (prev, new) ranges is degenerate. Snapshot-mode contracts +//! with missing data fall back to the existing GET-only path. +//! - **SinglePack only**: ChunkedPack reconstruction requires +//! re-splitting the local pack into chunks of the same `chunk_size` +//! that the original publish used; the chunk_size isn't stored in +//! the contract metadata today. Tracked as follow-up. +//! +//! # Algorithm +//! +//! 1. Read every `bundle-tip:` extension from the contract state +//! (each value is a 20-byte commit SHA — the tip the bundle covers). +//! 2. Sort tips chronologically by commit timestamp (`git log -1 +//! --format=%ct`). Linear pushes produce a chain; out-of-order +//! bundles (e.g. a force-push that wasn't reflected in main's +//! ancestor chain) get whatever order their committer date puts +//! them in — best-effort. +//! 3. For each consecutive (prev_tip, new_tip) pair, run +//! `git pack-objects` on the symmetric difference, BLAKE3 the +//! bytes, and store `(pack_hash -> pack_bytes)` in the lookup map. +//! The first bundle's `prev_tip` is `None` (full history pack). +//! 4. At rescue time, when `wsclient::get_pack` fails for a bundle, +//! look up the bundle's expected pack hash in the map; if present, +//! PUT those bytes directly. + +use anyhow::{anyhow, bail, Context, Result}; +use freenet_git_types::signing::parse_bundle_tip_extension_key; +use freenet_git_types::{ObjectBundle, ObjectBundleId, RepoState}; +use std::collections::HashMap; +use std::io::{BufRead, BufReader, Write}; +use std::path::Path; +use std::process::{Command, Stdio}; + +/// Map from expected pack BLAKE3 → reconstructed pack bytes. The +/// caller (rescue's per-bundle path) looks up the bundle's expected +/// pack_hash and PUTs the bytes when present. +pub type LocalPackMap = HashMap<[u8; 32], Vec>; + +/// Build the local-pack map from a working git directory and the +/// contract's current `RepoState`. +/// +/// Skips bundles that lack a `bundle-tip:` extension (pre-0.1.16 +/// mirrors don't record them) and bundles whose tip commit isn't +/// present in the local clone (the operator's clone is shallower than +/// the contract's history). Logs both cases at the `info` level so the +/// operator knows which bundles were skipped. +/// +/// Returns a map keyed by reconstructed pack BLAKE3. Bundles whose +/// reconstructed pack hash matches the value stored in the contract's +/// object_index entry can be rescued from this map; bundles whose +/// reconstruction produces a different hash (shouldn't happen if pack +/// reproducibility holds) are silently dropped from the map and the +/// rescue falls back to the GET-only path. +pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result { + // 1. Collect bundle-tip extensions: (bundle_id, tip_commit_sha) + let mut tips: Vec<(ObjectBundleId, [u8; 20])> = Vec::new(); + for (ext_key, entry) in &state.extensions { + let Some(bundle_id) = parse_bundle_tip_extension_key(ext_key) else { + continue; + }; + if entry.value.len() != 20 { + // Malformed extension — skip with a debug log. + tracing::debug!( + "bundle-tip extension for {} has unexpected length {}; skipping", + hex::encode(bundle_id), + entry.value.len() + ); + continue; + } + let mut tip = [0u8; 20]; + tip.copy_from_slice(&entry.value); + tips.push((bundle_id, tip)); + } + + if tips.is_empty() { + eprintln!( + "info: no bundle-tip extensions in contract state; --from cannot \ + reconstruct any bundles (this is normal for pre-0.1.16 mirrors)" + ); + return Ok(LocalPackMap::new()); + } + + // 2. Sort chronologically by commit timestamp. Bundles whose tip + // isn't in the local clone get filtered out here. + let mut tips_with_time: Vec<(ObjectBundleId, [u8; 20], i64)> = Vec::with_capacity(tips.len()); + let mut skipped_missing = 0usize; + for (bundle_id, tip) in &tips { + match commit_timestamp(git_dir, tip) { + Ok(ts) => tips_with_time.push((*bundle_id, *tip, ts)), + Err(_) => skipped_missing += 1, + } + } + if skipped_missing > 0 { + eprintln!( + "info: {skipped_missing} bundle tip commit(s) not present in local clone; \ + --from will not be able to reconstruct those bundles" + ); + } + tips_with_time.sort_by_key(|(_, _, t)| *t); + + // 3. For each consecutive (prev, new), build the pack and hash it. + let mut map = LocalPackMap::new(); + let mut prev_tip: Option<[u8; 20]> = None; + for (_bundle_id, new_tip, _) in &tips_with_time { + let prev_hex = prev_tip.as_ref().map(hex::encode); + let new_hex = hex::encode(new_tip); + match build_pack_for_range(git_dir, prev_hex.as_deref(), &new_hex) { + Ok(pack_bytes) => { + let pack_hash: [u8; 32] = blake3::hash(&pack_bytes).as_bytes().to_owned(); + map.insert(pack_hash, pack_bytes); + } + Err(e) => { + eprintln!( + "info: failed to reconstruct pack for tip {new_hex}: {e}; \ + --from cannot rescue this bundle" + ); + } + } + prev_tip = Some(*new_tip); + } + + Ok(map) +} + +/// Resolve the expected pack hash for a bundle, returning `None` for +/// non-`SinglePack` variants. Used by the rescue code to look up the +/// bundle's reconstructed bytes in [`LocalPackMap`]. +pub fn expected_pack_hash(bundle: &ObjectBundle) -> Option<[u8; 32]> { + match bundle { + ObjectBundle::SinglePack { pack_hash, .. } => Some(*pack_hash), + ObjectBundle::ChunkedPack { .. } => None, + } +} + +/// Build a pack for the symmetric difference `(prev..new]` from the +/// given git working directory. Equivalent to git-remote-freenet's +/// internal `build_pack` (no `--thin`, no `--no-reuse-delta`); using +/// the same flags is what gives byte-for-byte reproducibility against +/// the originally-pushed pack. +fn build_pack_for_range(git_dir: &Path, have: Option<&str>, want: &str) -> Result> { + // git_dir may be either a `.git` directory or a working-tree path; + // git accepts both via `--git-dir` (a working-tree path is treated + // as if `.git` were appended). + let mut rev_list = Command::new("git"); + rev_list.arg("--git-dir").arg(git_dir); + rev_list.args(["rev-list", "--objects", want]); + if let Some(h) = have { + rev_list.arg(format!("^{h}")); + } + let rev_list_out = rev_list + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .context("spawn git rev-list")?; + if !rev_list_out.status.success() { + let stderr = String::from_utf8_lossy(&rev_list_out.stderr); + bail!( + "git rev-list failed for range {}..{want}: {} (stderr: {})", + have.unwrap_or(""), + rev_list_out.status, + stderr.trim() + ); + } + + let mut child = Command::new("git") + .arg("--git-dir") + .arg(git_dir) + .args(["pack-objects", "--stdout"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("spawn git pack-objects")?; + { + let mut stdin = child.stdin.take().ok_or_else(|| anyhow!("piped stdin"))?; + for line in BufReader::new(rev_list_out.stdout.as_slice()).lines() { + let line = line?; + let sha = line.split(' ').next().unwrap_or(""); + if !sha.is_empty() { + writeln!(stdin, "{sha}")?; + } + } + } + let out = child.wait_with_output()?; + if !out.status.success() { + let stderr = String::from_utf8_lossy(&out.stderr); + bail!( + "git pack-objects failed: {} (stderr: {})", + out.status, + stderr.trim() + ); + } + Ok(out.stdout) +} + +/// Commit timestamp in seconds-since-epoch (committer date). Used to +/// chronologically order bundle tips before pairing them into push +/// ranges. Returns `Err` if the commit isn't in the local clone. +fn commit_timestamp(git_dir: &Path, commit_sha: &[u8; 20]) -> Result { + let hex = hex::encode(commit_sha); + let out = Command::new("git") + .arg("--git-dir") + .arg(git_dir) + .args(["log", "-1", "--format=%ct", &hex]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .context("spawn git log")?; + if !out.status.success() { + let stderr = String::from_utf8_lossy(&out.stderr); + bail!("commit {hex} not in local clone: {}", stderr.trim()); + } + let s = String::from_utf8(out.stdout).context("git log output not utf-8")?; + s.trim() + .parse::() + .with_context(|| format!("parse timestamp from {s:?}")) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Pin: `expected_pack_hash` extracts the pack_hash for SinglePack + /// and returns None for ChunkedPack. The rescue code's lookup gate + /// depends on this distinction. + #[test] + fn expected_pack_hash_returns_singlepack_only() { + let hash = [7u8; 32]; + let single = ObjectBundle::SinglePack { + pack_hash: hash, + size_bytes: 100, + }; + let chunked = ObjectBundle::ChunkedPack { + manifest_hash: hash, + chunk_count: 3, + total_size: 300, + }; + assert_eq!(expected_pack_hash(&single), Some(hash)); + assert_eq!(expected_pack_hash(&chunked), None); + } + + /// Pin: `build_local_pack_map` returns an empty map for a state + /// with no bundle-tip extensions. Logs the situation but doesn't + /// error — pre-0.1.16 mirrors legitimately don't have these. + #[test] + fn build_local_pack_map_empty_when_no_tip_extensions() { + let state = RepoState::default(); + let result = build_local_pack_map(Path::new("/nonexistent/dir"), &state).unwrap(); + assert!(result.is_empty()); + } +} diff --git a/crates/freenet-git/src/main.rs b/crates/freenet-git/src/main.rs index 7ead5e3..d39d6a6 100644 --- a/crates/freenet-git/src/main.rs +++ b/crates/freenet-git/src/main.rs @@ -208,6 +208,25 @@ enum Cmd { /// clamped up to `1`. #[arg(long, env = "FREENET_GIT_RESCUE_PARALLEL", default_value = "2")] parallel_bundles: usize, + /// Reconstruct missing pack bytes from a local git clone when + /// the gateway no longer has them cached. Pass the path to a + /// working git directory of the same repo (the `.git` of a + /// clone is fine; so is a bare repo). Without this flag, a + /// bundle whose pack has been evicted everywhere fails the + /// rescue with "GET pack ..." — the documented pre-0.1.23 + /// behavior. With `--from `, rescue rebuilds the + /// pack locally via `git pack-objects` and re-PUTs the same + /// byte-for-byte bytes (pack output is deterministic for a + /// given object set + git version). + /// + /// **Scope**: history-mode mirrors + SinglePack bundles only. + /// Snapshot-mode contracts and ChunkedPack bundles fall + /// through to the GET-only path because their reconstruction + /// shape isn't recoverable from the contract metadata today + /// (snapshot mode force-pushes fresh orphan commits per run; + /// ChunkedPack's chunk_size isn't stored in object_index). + #[arg(long, value_name = "GIT_DIR")] + from: Option, }, } @@ -265,6 +284,7 @@ fn run(cli: Cli) -> Result<()> { only_current_tips, rescue_all, parallel_bundles, + from, } => rescue( &url, ws_url.as_deref(), @@ -272,6 +292,7 @@ fn run(cli: Cli) -> Result<()> { only_current_tips, rescue_all, parallel_bundles, + from.as_deref(), ), } } @@ -549,6 +570,7 @@ fn rescue( only_current_tips: bool, rescue_all: bool, parallel_bundles: usize, + from: Option<&std::path::Path>, ) -> Result<()> { let parallel_bundles = parallel_bundles.max(1); let parsed = url::parse(url_str).with_context(|| format!("parse {url_str}"))?; @@ -670,6 +692,31 @@ fn rescue( // is roughly `parallel_bundles + parallel_bundles*8`. drop(api); + // Pre-build the local-pack reconstruction map if --from was + // supplied. Done up-front (before bundle dispatch) so each + // per-bundle task can do a cheap HashMap lookup on its + // expected pack hash. Empty map if `--from` not set; bundles + // whose gateway GET fails will then fall through to the + // existing error path (same as pre-0.1.23 behavior). + let local_pack_map: std::sync::Arc = + if let Some(git_dir) = from { + let map = freenet_git_cli::local_pack::build_local_pack_map(git_dir, &state) + .with_context(|| { + format!( + "build local-pack reconstruction map from --from {}", + git_dir.display() + ) + })?; + eprintln!( + "==> --from {}: reconstructed {} pack(s) locally for fallback", + git_dir.display(), + map.len() + ); + std::sync::Arc::new(map) + } else { + std::sync::Arc::new(freenet_git_cli::local_pack::LocalPackMap::new()) + }; + // FuturesUnordered with admission control: keep at most // `parallel_bundles` rescue tasks in flight; backfill as // each one completes. The driver runs on a current-thread @@ -702,9 +749,11 @@ fn rescue( let ws = ws.clone(); let id_owned = *id; let bundle = record.bundle.clone(); + let local_pack_map = local_pack_map.clone(); in_flight.push(tokio::spawn(async move { let label = format!("bundle {}", hex::encode(id_owned)); - rescue_one_bundle(&ws, &pack_wasm, bundle, timeout, label).await + rescue_one_bundle(&ws, &pack_wasm, bundle, timeout, label, local_pack_map) + .await })); } let Some(join_result) = in_flight.next().await else { @@ -940,6 +989,7 @@ async fn rescue_one_bundle( bundle: freenet_git_types::ObjectBundle, timeout: Duration, label: String, + local_pack_map: std::sync::Arc, ) -> BundleOutcome { match bundle { freenet_git_types::ObjectBundle::SinglePack { pack_hash, .. } => { @@ -953,7 +1003,7 @@ async fn rescue_one_bundle( }; } }; - match rescue_pack(&mut api, pack_wasm, pack_hash, timeout).await { + match rescue_pack(&mut api, pack_wasm, pack_hash, timeout, &local_pack_map).await { Ok(()) => BundleOutcome::Ok { label, kind_label: "SinglePack".to_string(), @@ -990,14 +1040,43 @@ async fn rescue_pack( pack_wasm: &[u8], pack_hash: [u8; 32], timeout: Duration, + local_pack_map: &freenet_git_cli::local_pack::LocalPackMap, ) -> Result<()> { - let bytes = wsclient::get_pack(api, pack_wasm, pack_hash, timeout) - .await - .with_context(|| format!("GET pack {}", hex::encode(pack_hash)))?; - wsclient::put_pack(api, pack_wasm, bytes, timeout) - .await - .with_context(|| format!("PUT pack {}", hex::encode(pack_hash)))?; - Ok(()) + // Try the gateway first. If it has the pack, GET-then-PUT is the + // happy path and the local-reconstruction work was wasted effort + // for this bundle — which is fine, build_local_pack_map runs once + // up-front and the per-bundle cost is just a HashMap lookup. + match wsclient::get_pack(api, pack_wasm, pack_hash, timeout).await { + Ok(bytes) => { + wsclient::put_pack(api, pack_wasm, bytes, timeout) + .await + .with_context(|| format!("PUT pack {}", hex::encode(pack_hash)))?; + Ok(()) + } + Err(gateway_err) => { + // Gateway can't serve this pack. Try local reconstruction + // before reporting failure. The map is empty when --from + // wasn't passed, so this lookup is a cheap no-op for the + // pre-0.1.23 invocation shape. + if let Some(local_bytes) = local_pack_map.get(&pack_hash) { + eprintln!( + "info: pack {} not available from gateway ({gateway_err}); \ + reconstructed locally via --from, re-PUTting", + hex::encode(pack_hash) + ); + wsclient::put_pack(api, pack_wasm, local_bytes.clone(), timeout) + .await + .with_context(|| { + format!( + "PUT reconstructed pack {} (from local clone)", + hex::encode(pack_hash) + ) + })?; + return Ok(()); + } + Err(gateway_err).with_context(|| format!("GET pack {}", hex::encode(pack_hash))) + } + } } /// Rescue a chunked-pack bundle. Each chunk is GET'd then re-PUT; From 3d8f3065bf3dcb4ba9cc66d0338762b9dc1afbab Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Sun, 17 May 2026 15:52:06 -0500 Subject: [PATCH 2/5] fix(local_pack): address PR #55 skeptical + Codex review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewers identified three real correctness issues in the initial --from implementation; all addressed in this commit. # Skeptical H1: git pack-objects is non-deterministic with default threads `git pack-objects` default `pack.threads=auto` (= CPU count) races during delta search, producing different pack bytes for the same object set across runs / machines with different CPU topologies. The PR-#55 initial verification on my dev box was coincidence -- a CI runner with a different core count would produce different deltas, different BLAKE3, and silent 0-pack reconstruction. Fix: pass `-c pack.threads=1` to `git pack-objects` in `build_pack_for_range`. Verified empirically against the live failing bundle: `git -c pack.threads=1 rev-list ... | git -c pack.threads=1 pack-objects --stdout` for `(000c4fde, c682079a]` still produces BLAKE3 `7c30464721...`. The single-threaded delta search trades wall-clock for bit-for-bit reproducibility; rescue runs aren't a hot path, so correctness > speed here. The original `build_pack` in git-remote-freenet.rs intentionally keeps default threads -- push paths create FRESH bundles whose pack_hash is whatever the run produced, so non-determinism doesn't matter there. # Skeptical H2 / Codex P2 #1: committer-date ordering can mis-pair Sorting bundle tips by `%ct` doesn't reflect the actual push order: merge commits, imported history, and concurrent CI commits in the same second can all produce orderings that don't match the push sequence. Mis-paired `(prev_tip, new_tip)` ranges generate packs with different content and different BLAKE3, missing the map and silently degrading rescue success rate. Fix: replace `commit_timestamp` with `ancestor_count` (`git rev-list --count ` returns reachable-commit count, which strictly grows with each push in a linear history-mode chain). Tie-break by tip bytes for determinism. Removes `git log -1 --format=%ct` invocation. # Codex P2 #2: --from worktree-root paths failed silently `git --git-dir ` does NOT auto-append `.git` -- it treats `` as the literal git directory. The CLI help text described "pass the path to a working git directory of the same repo (the `.git` of a clone is fine; so is a bare repo)", but a user passing the worktree root (the documented "clone of the same repo" form) would silently get a 0-pack map. Fix: new `normalize_git_dir` helper detects `/.git` and returns it if present; passes through otherwise (handles bare repos and explicit `.git` paths). Applied at the top of `build_local_pack_map`. # Skeptical M3: HashMap<[u8;32], Vec> deep-cloned per PUT Changed map value type to `Arc>` so parallel rescues share the same byte buffer instead of N deep clones. The PUT still needs an owned `Vec` for the `wsclient::put_pack` API so one clone remains per PUT — but the up-front cost is now O(1) Arc clone instead of O(pack_size) deep clone for the per-task dispatch. # Skeptical L1: dead `expected_pack_hash` removed The helper was exported `pub fn` but never called -- `rescue_pack` receives `pack_hash: [u8; 32]` directly. Removed it and its now-dead test. Replaced with two `normalize_git_dir` tests covering the worktree-root and bare-repo paths (Codex P2 #2 regression guard). # Other findings deferred to follow-ups (not blocking this PR) - Skeptical M4 (serial build-up-front cost): fine for current bundle counts (~30); revisit if multi-thousand-bundle contracts emerge. - Skeptical M5 (unbounded fetch-depth in workflow): documented in-PR; worth a follow-up issue but not regressive vs the previous workflow. - Skeptical L7 (rev-list buffered to memory): same pattern as original build_pack; not introduced here. [AI-assisted - Claude] --- crates/freenet-git/src/local_pack.rs | 156 +++++++++++++++++---------- crates/freenet-git/src/main.rs | 6 +- 2 files changed, 105 insertions(+), 57 deletions(-) diff --git a/crates/freenet-git/src/local_pack.rs b/crates/freenet-git/src/local_pack.rs index b5d14ff..c57bef7 100644 --- a/crates/freenet-git/src/local_pack.rs +++ b/crates/freenet-git/src/local_pack.rs @@ -41,16 +41,37 @@ use anyhow::{anyhow, bail, Context, Result}; use freenet_git_types::signing::parse_bundle_tip_extension_key; -use freenet_git_types::{ObjectBundle, ObjectBundleId, RepoState}; +use freenet_git_types::{ObjectBundleId, RepoState}; use std::collections::HashMap; use std::io::{BufRead, BufReader, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::sync::Arc; /// Map from expected pack BLAKE3 → reconstructed pack bytes. The /// caller (rescue's per-bundle path) looks up the bundle's expected -/// pack_hash and PUTs the bytes when present. -pub type LocalPackMap = HashMap<[u8; 32], Vec>; +/// pack_hash and PUTs the bytes when present. Bytes are `Arc`-wrapped +/// so parallel rescues don't deep-clone for each PUT. +pub type LocalPackMap = HashMap<[u8; 32], Arc>>; + +/// Normalize a user-supplied `--from ` to the actual git +/// directory git's `--git-dir` flag expects. If `/.git` is a +/// directory the user passed a worktree root → return `/.git`. +/// Otherwise return `` as-is (handles bare repos and explicit +/// `.git` directory paths). +/// +/// Codex PR #55 P2 #2: without this, a user invoking `--from +/// /path/to/clone` would silently fail every reconstruction because +/// `git --git-dir /path/to/clone` treats the worktree as if it WERE +/// the git directory (no `.git` auto-append) and every command fails. +pub fn normalize_git_dir(path: &Path) -> PathBuf { + let dot_git = path.join(".git"); + if dot_git.is_dir() { + dot_git + } else { + path.to_path_buf() + } +} /// Build the local-pack map from a working git directory and the /// contract's current `RepoState`. @@ -68,6 +89,9 @@ pub type LocalPackMap = HashMap<[u8; 32], Vec>; /// reproducibility holds) are silently dropped from the map and the /// rescue falls back to the GET-only path. pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result { + let git_dir = normalize_git_dir(git_dir); + let git_dir = git_dir.as_path(); + // 1. Collect bundle-tip extensions: (bundle_id, tip_commit_sha) let mut tips: Vec<(ObjectBundleId, [u8; 20])> = Vec::new(); for (ext_key, entry) in &state.extensions { @@ -96,13 +120,20 @@ pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result = Vec::with_capacity(tips.len()); + // 2. Order tips by ANCESTRY, not committer date. Each push's new + // tip is a descendant of the previous push's tip in a linear + // history-mode chain, so we can order by how many ancestors + // each tip has reachable in the local clone (more ancestors = + // later push). Codex/skeptical PR #55: committer-date sort can + // mis-pair (prev, new) when merge commits or imported history + // produce out-of-order timestamps. + // + // Tips whose commit isn't in the local clone get filtered out. + let mut tips_with_order: Vec<(ObjectBundleId, [u8; 20], u64)> = Vec::with_capacity(tips.len()); let mut skipped_missing = 0usize; for (bundle_id, tip) in &tips { - match commit_timestamp(git_dir, tip) { - Ok(ts) => tips_with_time.push((*bundle_id, *tip, ts)), + match ancestor_count(git_dir, tip) { + Ok(n) => tips_with_order.push((*bundle_id, *tip, n)), Err(_) => skipped_missing += 1, } } @@ -112,18 +143,22 @@ pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result = None; - for (_bundle_id, new_tip, _) in &tips_with_time { + for (_bundle_id, new_tip, _) in &tips_with_order { let prev_hex = prev_tip.as_ref().map(hex::encode); let new_hex = hex::encode(new_tip); match build_pack_for_range(git_dir, prev_hex.as_deref(), &new_hex) { Ok(pack_bytes) => { let pack_hash: [u8; 32] = blake3::hash(&pack_bytes).as_bytes().to_owned(); - map.insert(pack_hash, pack_bytes); + map.insert(pack_hash, Arc::new(pack_bytes)); } Err(e) => { eprintln!( @@ -138,25 +173,25 @@ pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result Option<[u8; 32]> { - match bundle { - ObjectBundle::SinglePack { pack_hash, .. } => Some(*pack_hash), - ObjectBundle::ChunkedPack { .. } => None, - } -} - -/// Build a pack for the symmetric difference `(prev..new]` from the -/// given git working directory. Equivalent to git-remote-freenet's -/// internal `build_pack` (no `--thin`, no `--no-reuse-delta`); using -/// the same flags is what gives byte-for-byte reproducibility against -/// the originally-pushed pack. +/// Build a pack for the symmetric difference `(have..want]` from the +/// given git directory, with `pack.threads=1` so the delta search is +/// deterministic. +/// +/// Skeptical-reviewer PR #55 H1: `git pack-objects` with the default +/// `pack.threads=auto` produces non-deterministic output because the +/// per-thread delta search races. Same object set → different deltas +/// → different pack bytes → different BLAKE3. Without `pack.threads=1` +/// the reconstructed map would lose half its entries on CI runners +/// with a different CPU count than the original publisher's machine, +/// silently degrading rescue success rate. Single-threaded delta +/// search trades wall-clock for bit-for-bit reproducibility (rescue +/// runs are not a hot path; correctness > speed here). +/// +/// The original `build_pack` in git-remote-freenet.rs intentionally +/// doesn't carry this flag — push paths create fresh bundles whose +/// `pack_hash` is whatever the pack-objects run produced, so non- +/// determinism doesn't matter there. fn build_pack_for_range(git_dir: &Path, have: Option<&str>, want: &str) -> Result> { - // git_dir may be either a `.git` directory or a working-tree path; - // git accepts both via `--git-dir` (a working-tree path is treated - // as if `.git` were appended). let mut rev_list = Command::new("git"); rev_list.arg("--git-dir").arg(git_dir); rev_list.args(["rev-list", "--objects", want]); @@ -181,7 +216,8 @@ fn build_pack_for_range(git_dir: &Path, have: Option<&str>, want: &str) -> Resul let mut child = Command::new("git") .arg("--git-dir") .arg(git_dir) - .args(["pack-objects", "--stdout"]) + // -c pack.threads=1: see fn-level docs (H1 fix). + .args(["-c", "pack.threads=1", "pack-objects", "--stdout"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -209,50 +245,58 @@ fn build_pack_for_range(git_dir: &Path, have: Option<&str>, want: &str) -> Resul Ok(out.stdout) } -/// Commit timestamp in seconds-since-epoch (committer date). Used to -/// chronologically order bundle tips before pairing them into push -/// ranges. Returns `Err` if the commit isn't in the local clone. -fn commit_timestamp(git_dir: &Path, commit_sha: &[u8; 20]) -> Result { +/// Count of commits reachable from `commit_sha` in the local clone. +/// Used as a topological ordinal for sorting bundle tips by push +/// chronology — for a linear history-mode push chain, the Nth push's +/// tip has more reachable commits than the (N-1)th push's tip. +/// Returns `Err` if the commit isn't in the local clone. +fn ancestor_count(git_dir: &Path, commit_sha: &[u8; 20]) -> Result { let hex = hex::encode(commit_sha); let out = Command::new("git") .arg("--git-dir") .arg(git_dir) - .args(["log", "-1", "--format=%ct", &hex]) + .args(["rev-list", "--count", &hex]) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .output() - .context("spawn git log")?; + .context("spawn git rev-list --count")?; if !out.status.success() { let stderr = String::from_utf8_lossy(&out.stderr); bail!("commit {hex} not in local clone: {}", stderr.trim()); } - let s = String::from_utf8(out.stdout).context("git log output not utf-8")?; + let s = String::from_utf8(out.stdout).context("git rev-list output not utf-8")?; s.trim() - .parse::() - .with_context(|| format!("parse timestamp from {s:?}")) + .parse::() + .with_context(|| format!("parse ancestor count from {s:?}")) } #[cfg(test)] mod tests { use super::*; + use std::fs; + use tempfile::TempDir; - /// Pin: `expected_pack_hash` extracts the pack_hash for SinglePack - /// and returns None for ChunkedPack. The rescue code's lookup gate - /// depends on this distinction. + /// Pin: `normalize_git_dir` returns `/.git` when the user + /// passes a worktree root (the documented invocation shape from + /// `--from`'s help text). Codex PR #55 P2 #2 caught that without + /// this, the user's literal `--from /path/to/clone` would silently + /// produce a 0-pack map because `git --git-dir /path/to/clone` + /// treats the worktree itself as the git directory. #[test] - fn expected_pack_hash_returns_singlepack_only() { - let hash = [7u8; 32]; - let single = ObjectBundle::SinglePack { - pack_hash: hash, - size_bytes: 100, - }; - let chunked = ObjectBundle::ChunkedPack { - manifest_hash: hash, - chunk_count: 3, - total_size: 300, - }; - assert_eq!(expected_pack_hash(&single), Some(hash)); - assert_eq!(expected_pack_hash(&chunked), None); + fn normalize_git_dir_appends_dot_git_for_worktree_root() { + let tmp = TempDir::new().unwrap(); + let dot_git = tmp.path().join(".git"); + fs::create_dir(&dot_git).unwrap(); + assert_eq!(normalize_git_dir(tmp.path()), dot_git); + } + + /// Pin: `normalize_git_dir` passes through paths that don't have a + /// `.git` subdir (bare repos, or an explicit `.git` path). + #[test] + fn normalize_git_dir_passes_through_bare_or_dot_git() { + let tmp = TempDir::new().unwrap(); + // No .git subdir → passed through as-is. + assert_eq!(normalize_git_dir(tmp.path()), tmp.path()); } /// Pin: `build_local_pack_map` returns an empty map for a state diff --git a/crates/freenet-git/src/main.rs b/crates/freenet-git/src/main.rs index d39d6a6..8fdfefc 100644 --- a/crates/freenet-git/src/main.rs +++ b/crates/freenet-git/src/main.rs @@ -1064,7 +1064,11 @@ async fn rescue_pack( reconstructed locally via --from, re-PUTting", hex::encode(pack_hash) ); - wsclient::put_pack(api, pack_wasm, local_bytes.clone(), timeout) + // Bytes are Arc> in the map; the (*local_bytes).clone() + // deep-clones once per PUT. The Arc itself is cheap; the deep + // clone is the unavoidable cost of put_pack taking an owned + // Vec. Future: thread Arc through put_pack to skip this. + wsclient::put_pack(api, pack_wasm, (**local_bytes).clone(), timeout) .await .with_context(|| { format!( From b4881a0153fe4d80d2d4601a042cacb5bf149da5 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Sun, 17 May 2026 15:56:54 -0500 Subject: [PATCH 3/5] fix(local_pack): handle multi-ref pushes + pin pack.threads on publisher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex re-review on PR #55 @ 3d8f306 found two more real issues: # Codex P2 #1: linear ancestry chain miss-pairs multi-refspec pushes The rescue-demos workflow pushes `main:main` + `refs/tags/*:refs/tags/*` in a single git push. The remote helper handles each refspec independently in handle_push (git-remote-freenet.rs:868-922), so each ref gets its own bundle whose `prev` comes from `state.refs.get(&dst)`: - main:main with prev = state.refs["refs/heads/main"].target → pack covers `prev_main..main_tip` - refs/tags/v0.1.X (new tag, never pushed before): prev = None → pack covers everything reachable from the tag The previous algorithm chained EVERY tip linearly through one global ancestry order, so a tag bundle would be reconstructed as `previous_bundle_tip..tag_commit` rather than the original `(.., tag_commit]`. Different content → different BLAKE3 → bundle silently misses the map. Fix: for each tip, build BOTH candidates — chained-from-previous AND no-prev. Insert each into the map under its own BLAKE3. The actual original pack matches one of them; wrong-content candidates land under their (mismatched) hash and are never looked up. Cost: ~2N pack-objects invocations instead of N. Acceptable for current bundle counts; revisit if multi-thousand-bundle contracts emerge. # Codex P2 #2: pack.threads=1 must apply to publisher too PR #55's first review fix pinned `pack.threads=1` only in the rescue's build_pack_for_range. But the publisher (build_pack in git-remote-freenet.rs) still ran default `pack.threads=auto`, which is non-deterministic on multi-core machines. A multi-core publisher could produce pack bytes that single-threaded reconstruction can't reproduce → expected hash mismatch → silent rescue failure. The 2026-05-17 verification against bundle 7c30464721 passed because the pack was small enough (17 KB) that delta search probably ran single-threaded anyway; a larger pack on a multi-core publisher would have exposed this. Fix: add `-c pack.threads=1` to build_pack in git-remote-freenet.rs too. Small per-push wall-clock cost for permanent reproducibility across all future rescues. Note: only matters for history-mode pushes; snapshot-mode pushes create fresh bundles per run, so their pack determinism doesn't affect anything (we don't reconstruct snapshot bundles either). # Tests Both fixes are inherently empirical. The structural pin remains correct; the runtime verification will happen when 0.1.23 ships and the next rescue run picks up the new binary against contracts whose packs were produced by pre-0.1.23 publishers. The 2026-05-17 verification (bundle 7c30464721 reproduces from `(000c4fde, c682079a]`) holds for at least the SinglePack chained-from-previous case used by the freenet-stdlib mirror; tag bundles get verified end-to-end on the next freenet-git self-mirror push that creates one. Module-level algorithm doc updated to reflect the new multi-candidate shape. [AI-assisted - Claude] --- .../freenet-git/src/bin/git-remote-freenet.rs | 9 +- crates/freenet-git/src/local_pack.rs | 88 ++++++++++++++----- 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/crates/freenet-git/src/bin/git-remote-freenet.rs b/crates/freenet-git/src/bin/git-remote-freenet.rs index 0dc504e..844555c 100644 --- a/crates/freenet-git/src/bin/git-remote-freenet.rs +++ b/crates/freenet-git/src/bin/git-remote-freenet.rs @@ -1290,10 +1290,17 @@ fn build_pack( bail!("git rev-list failed: {}", rev_list_out.status); } + // `-c pack.threads=1`: the pack that lands on the contract here + // is what `freenet-git rescue --from ` will later try to + // reconstruct byte-for-byte. Default `pack.threads=auto` races + // delta search across cores → non-deterministic pack bytes → + // future rescues' reconstructions silently miss the lookup map. + // Pinning single-threaded is a small per-push wall-clock cost + // for permanent rescue reproducibility. See PR #55 Codex P2 #2. let mut child = Command::new("git") .arg("--git-dir") .arg(git_dir) - .args(["pack-objects", "--stdout"]) + .args(["-c", "pack.threads=1", "pack-objects", "--stdout"]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) diff --git a/crates/freenet-git/src/local_pack.rs b/crates/freenet-git/src/local_pack.rs index c57bef7..b1b12f9 100644 --- a/crates/freenet-git/src/local_pack.rs +++ b/crates/freenet-git/src/local_pack.rs @@ -26,16 +26,24 @@ //! //! 1. Read every `bundle-tip:` extension from the contract state //! (each value is a 20-byte commit SHA — the tip the bundle covers). -//! 2. Sort tips chronologically by commit timestamp (`git log -1 -//! --format=%ct`). Linear pushes produce a chain; out-of-order -//! bundles (e.g. a force-push that wasn't reflected in main's -//! ancestor chain) get whatever order their committer date puts -//! them in — best-effort. -//! 3. For each consecutive (prev_tip, new_tip) pair, run -//! `git pack-objects` on the symmetric difference, BLAKE3 the -//! bytes, and store `(pack_hash -> pack_bytes)` in the lookup map. -//! The first bundle's `prev_tip` is `None` (full history pack). -//! 4. At rescue time, when `wsclient::get_pack` fails for a bundle, +//! 2. Order tips by reachable-commit count (`git rev-list --count`), +//! tie-break on tip bytes. For linear history-mode pushes this is +//! the actual push chronology; out-of-order timestamps from merges +//! don't affect it. +//! 3. For each tip, generate MULTIPLE candidate `(prev, new)` packs: +//! chained-from-previous (the common same-ref push case) AND +//! no-prev (the new-ref case — e.g. a tag bundle whose original +//! pack covered everything reachable from the tag). The push code +//! in git-remote-freenet.rs uses `state.refs.get(&dst)` to pick +//! each ref's prev independently, so a single push of `main:main` +//! + `refs/tags/*:refs/tags/*` creates one main-bundle (prev = +//! last-known main tip) and one tag-bundle per new tag (prev = +//! None). Trying both candidates per tip covers both shapes +//! without needing to record which ref the bundle came from. +//! 4. BLAKE3 every reconstructed pack, store `(pack_hash -> +//! pack_bytes)`. Wrong-content packs (from mis-paired candidates) +//! land under their own hash and are silently never looked up. +//! 5. At rescue time, when `wsclient::get_pack` fails for a bundle, //! look up the bundle's expected pack hash in the map; if present, //! PUT those bytes directly. @@ -149,30 +157,64 @@ pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result = None; for (_bundle_id, new_tip, _) in &tips_with_order { - let prev_hex = prev_tip.as_ref().map(hex::encode); let new_hex = hex::encode(new_tip); - match build_pack_for_range(git_dir, prev_hex.as_deref(), &new_hex) { - Ok(pack_bytes) => { - let pack_hash: [u8; 32] = blake3::hash(&pack_bytes).as_bytes().to_owned(); - map.insert(pack_hash, Arc::new(pack_bytes)); - } - Err(e) => { - eprintln!( - "info: failed to reconstruct pack for tip {new_hex}: {e}; \ - --from cannot rescue this bundle" - ); - } + + // Candidate A: chained from previous tip in ancestry order + // (the common case for sequential same-ref pushes). + if let Some(prev) = prev_tip.as_ref() { + let prev_hex = hex::encode(prev); + try_reconstruct_into(git_dir, Some(&prev_hex), &new_hex, &mut map); } + // Candidate B: no prev (the new-ref case, e.g. a newly-pushed + // tag, where the original pack covered everything reachable + // from the tip). Also serves as the very first bundle's + // canonical reconstruction. + try_reconstruct_into(git_dir, None, &new_hex, &mut map); + prev_tip = Some(*new_tip); } Ok(map) } +/// Build the pack for `(have..want]` and insert it into `map` keyed +/// by its reconstructed BLAKE3. Failures are logged at info-level but +/// don't propagate — a single failing reconstruction shouldn't abort +/// the entire `--from` setup for other bundles that would have worked. +fn try_reconstruct_into(git_dir: &Path, have: Option<&str>, want: &str, map: &mut LocalPackMap) { + match build_pack_for_range(git_dir, have, want) { + Ok(pack_bytes) => { + let pack_hash: [u8; 32] = blake3::hash(&pack_bytes).as_bytes().to_owned(); + map.entry(pack_hash).or_insert_with(|| Arc::new(pack_bytes)); + } + Err(e) => { + let label = match have { + Some(h) => format!("({h}..{want}]"), + None => format!("(.., {want}]"), + }; + tracing::debug!("local-pack reconstruction failed for {label}: {e}"); + } + } +} + /// Build a pack for the symmetric difference `(have..want]` from the /// given git directory, with `pack.threads=1` so the delta search is /// deterministic. From 9f0491b6c4a8017592b03d70f66bcf28f08a7d7e Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Sun, 17 May 2026 15:59:10 -0500 Subject: [PATCH 4/5] docs(local_pack): doc-comment matches actual log level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skeptical re-review @ b4881a0 caught: `try_reconstruct_into`'s docstring said "logged at info-level" but the implementation uses `tracing::debug!`. The previous commit lowered the log level deliberately (one candidate failing per tip is now normal under the multi-candidate algorithm — surfacing those at info would be noisy) but didn't update the prose. Fix the docstring to match the code and explain the rationale. [AI-assisted - Claude] --- crates/freenet-git/src/local_pack.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/freenet-git/src/local_pack.rs b/crates/freenet-git/src/local_pack.rs index b1b12f9..32d7db1 100644 --- a/crates/freenet-git/src/local_pack.rs +++ b/crates/freenet-git/src/local_pack.rs @@ -196,9 +196,14 @@ pub fn build_local_pack_map(git_dir: &Path, state: &RepoState) -> Result, want: &str, map: &mut LocalPackMap) { match build_pack_for_range(git_dir, have, want) { Ok(pack_bytes) => { From 1ff2979541d0e3bd8bc217bdb0e8f7adfa814a87 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Sun, 17 May 2026 16:05:03 -0500 Subject: [PATCH 5/5] docs(local_pack): rewrite numbered-list item to avoid clippy doc-lazy-continuation CI clippy (Rust 1.94.0) tripped on `clippy::doc_lazy_continuation` for the module-level numbered list: a line starting with `//! + \`refs/tags/*...\`` was parsed by the markdown lint as starting a nested sub-list, then the following continuation line had no list marker. Reworded to use "both A and B" prose rather than a stray "+" that looks like a list marker. [AI-assisted - Claude] --- crates/freenet-git/src/local_pack.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/freenet-git/src/local_pack.rs b/crates/freenet-git/src/local_pack.rs index 32d7db1..45d39ed 100644 --- a/crates/freenet-git/src/local_pack.rs +++ b/crates/freenet-git/src/local_pack.rs @@ -32,14 +32,15 @@ //! don't affect it. //! 3. For each tip, generate MULTIPLE candidate `(prev, new)` packs: //! chained-from-previous (the common same-ref push case) AND -//! no-prev (the new-ref case — e.g. a tag bundle whose original +//! no-prev (the new-ref case, e.g. a tag bundle whose original //! pack covered everything reachable from the tag). The push code //! in git-remote-freenet.rs uses `state.refs.get(&dst)` to pick -//! each ref's prev independently, so a single push of `main:main` -//! + `refs/tags/*:refs/tags/*` creates one main-bundle (prev = -//! last-known main tip) and one tag-bundle per new tag (prev = -//! None). Trying both candidates per tip covers both shapes -//! without needing to record which ref the bundle came from. +//! each ref's prev independently, so a single push of both +//! `main:main` and `refs/tags/*:refs/tags/*` creates one +//! main-bundle (prev = last-known main tip) and one tag-bundle +//! per new tag (prev = None). Trying both candidates per tip +//! covers both shapes without needing to record which ref the +//! bundle came from. //! 4. BLAKE3 every reconstructed pack, store `(pack_hash -> //! pack_bytes)`. Wrong-content packs (from mis-paired candidates) //! land under their own hash and are silently never looked up.