fix(git): serve want-by-sha for unadvertised refs (PR merge commits) - #27
Open
DanielHabenicht wants to merge 1 commit into
Open
fix(git): serve want-by-sha for unadvertised refs (PR merge commits)#27DanielHabenicht wants to merge 1 commit into
DanielHabenicht wants to merge 1 commit into
Conversation
DanielHabenicht
commented
Aug 26, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #27 +/- ##
==========================================
+ Coverage 96.12% 96.20% +0.07%
==========================================
Files 6 6
Lines 1650 1869 +219
==========================================
+ Hits 1586 1798 +212
- Misses 64 71 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rolandjitsu
reviewed
Aug 27, 2026
Owner
|
Thanks for your contribution @DanielHabenicht ! I left a few comments. You also need to fixup the commit msg/body. |
actions/checkout on a pull_request event fetches the synthetic merge commit by bare SHA. That commit lives only under GitHub's unadvertised refs/pull/<n>/merge, so the mirror clone never captured it and upload-pack rejected the want with "not our ref", breaking CI behind the proxy. Before serving, ensure_fresh scans the upload-pack request for want lines and, for any SHA missing from the mirror, fetches it from upstream and pins it under a reserved refs/proxy-wants/<sha> ref. Pinning makes the object a valid want tip (so upload-pack serves it) and keeps it from git gc. The namespace is hidden from the advertisement yet still honored as a want tip, and excluded from prune so pins survive periodic refreshes. Because those pins are excluded from prune and gc, --max-wants bounds how many a mirror retains: the oldest are pruned beyond the cap so they cannot accumulate without bound. Relies on the upstream serving arbitrary SHAs (GitHub's allowAnySHA1InWant); an upstream that refuses leaves the want unsatisfied for upload-pack to reject. Ordinary branch/tag clones pay only a cheap cat-file check, never an extra upstream call. Assisted-by: Claude:claude-opus-4-8
DanielHabenicht
force-pushed
the
fix/pr-merge-ref-fetch
branch
from
August 27, 2026 21:30
14a02b8 to
6e36d14
Compare
rolandjitsu
reviewed
Aug 30, 2026
| // would be pruned straight away (see `prune_wants`), so fetching it is pure | ||
| // waste. The excess is dropped and upload-pack rejects those wants. | ||
| if self.cfg.max_wants > 0 && missing.len() > self.cfg.max_wants { | ||
| tracing::warn!( |
Owner
There was a problem hiding this comment.
The risk is that if we don't surface this to the user, they'll expect it to work and might be confused why the shas are not available. Someone will need to check the logs to see what's going on.
But, as long as it's a documented behavior, it would be ok.
|
|
||
| /// Collect an iterator of oids into a `HashSet` for order-insensitive assertions | ||
| /// against `parse_wants`. | ||
| fn want_set<I: IntoIterator<Item = String>>(oids: I) -> HashSet<String> { |
Owner
There was a problem hiding this comment.
Place helper fns at the bottom, after all tests.
| .take_while(u8::is_ascii_hexdigit) | ||
| .collect(); | ||
| // sha1 (40) or sha256 (64); ignore anything else (e.g. a stray token). | ||
| if oid.len() == 40 || oid.len() == 64 { |
Owner
There was a problem hiding this comment.
You could ignore parsing oids over the max_wants here. Then you don't need to truncate and check for missing in ensure_wanted_oids.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before serving an upload-pack RPC, parse the client's
wantlines and, for any SHA missing from the mirror, fetch it from upstream on demand and pin it under a reservedrefs/proxy-wants/<sha>ref. Pinning makes the object a valid want tip (so upload-pack serves it) and keeps it fromgit gc. The namespace is hidden from the ref advertisement (uploadpack.hideRefs) yet still honored as a want tip, and excluded fromfetch --pruneso pins survive periodic refreshes.Relies on the upstream serving arbitrary SHAs (GitHub's allowAnySHA1InWant); an upstream that refuses leaves the request to fail as before - no regression. Ordinary branch/tag clones pay only a single cheap cat-file check, never an extra upstream call.
What kind of change does this PR introduce?
Summary
actions/checkout on a pull_request event fetches the synthetic merge commit by bare SHA. That commit lives only under GitHub's unadvertised refs/pull//merge, so
clone --mirrornever captured it and the mirror's upload-pack rejected the want with "fatal: not our ref ", breaking CI behind the proxy.Tests
Checklist
cargo fmt --all --check,cargo clippy --all-targets --all-features --locked -- -D warnings,cargo test --all-featuresAssisted-by:trailer (see CONTRIBUTING.md / AGENTS.md)Breaking change?
If yes, describe the impact and the migration path (flags / env, on-disk cache layout).