Skip to content

fix: fetch remote tip before snapshot push#20

Merged
sanity merged 1 commit into
mainfrom
fix-snapshot-fetch-before-push
May 7, 2026
Merged

fix: fetch remote tip before snapshot push#20
sanity merged 1 commit into
mainfrom
fix-snapshot-fetch-before-push

Conversation

@sanity
Copy link
Copy Markdown
Contributor

@sanity sanity commented May 7, 2026

Problem

Snapshot mode in the reusable mirror workflow has been failing for freenet-core since the caller PR merged — 5/5 runs failed with:

fatal: bad object a6afd44c68bad8bd12ecb91479dbdad9d25a06fc
 ! [remote rejected] main -> main (git rev-list failed: exit status: 128)

git-remote-freenet's build_pack invokes git rev-list ^<remote-tip> <new-tip> to enumerate objects for the pack. In snapshot mode the new tip is a fresh orphan commit; the local repo (just git init -b main plus the orphan) has never seen the remote tip, so rev-list bails.

History mode is unaffected.

Approach

Add git fetch freenet "$BRANCH" 2>/dev/null || true before the push. The fetch pulls the remote tip's pack into local objects, so the subsequent rev-list resolves both ends. || true covers first-push.

The deeper fix belongs in the helper itself — build_pack should retry without ^have when rev-list fails on a missing local object (for force pushes only; non-force pushes against an unknown tip should still fail loudly). Filing separately; this PR is the workflow-side workaround that unblocks freenet-core mirror today.

Testing

Reproduced and verified locally before opening this PR. The freenet-core demo was also re-published manually as part of the diagnosis; current tip is b2a8836. Fresh git clone freenet::3GEERif5ihbf/freenet-core returns 523 files end-to-end against the live gateway.

Fixes

[AI-assisted - Claude]

@sanity @claude
fix: fetch remote tip before snapshot push to populate local objects
692982e 
## Problem

Snapshot mode in the reusable mirror workflow has been failing for
freenet-core in CI since merge (5/5 runs failed). The error:

  fatal: bad object a6afd44c68bad8bd12ecb91479dbdad9d25a06fc
  ! [remote rejected] main -> main (git rev-list failed: exit status: 128)

`git-remote-freenet`'s `build_pack` invokes
`git rev-list ^<remote-tip> <new-tip>` to enumerate objects to pack.
In snapshot mode the new-tip is a fresh orphan commit with no
shared history, and the local repo (just `git init -b main` with
the orphan commit) has never seen the remote tip. rev-list bails.

History mode is unaffected: `actions/checkout` produces a clone that
already has the remote tip's ancestor objects whenever the source
branch hasn't diverged from what the receiver knows.

## Approach

Add `git fetch freenet "$BRANCH" 2>/dev/null || true` before the
push. This pulls the remote tip's pack into local objects, so
`rev-list ^<remote-tip> <new-tip>` resolves both ends. The `|| true`
covers the first-push case where the remote has no main yet.

For ChunkedPack repos (e.g. freenet-core's snapshot) the fetch will
also pull the chunks, which costs network bandwidth but is the
right tradeoff for the rare snapshot-rebuild case. Subsequent
pushes that produce the same orphan SHA (per the H3 deterministic-
date fix) hit the existing fast-path -- no new objects to send.

The deeper fix is in the `git-remote-freenet` helper itself:
`build_pack` should retry without `^have` when rev-list fails on a
missing local object (specifically for force pushes -- non-force
pushes against an unknown remote tip should still fail loudly to
prevent unintended overwrite). Filing as a separate issue; this PR
is the workflow-side workaround that unblocks the freenet-core
mirror today.

## Testing

Reproduced and verified the fix locally:

  $ # Without fetch:
  $ git push freenet +main:main
  ==> reading repo state from Freenet
  fatal: bad object a6afd44c68bad8bd12ecb91479dbdad9d25a06fc
   ! [remote rejected] main -> main (git rev-list failed: exit status: 128)

  $ # With fetch first:
  $ git fetch freenet main
   * branch            main       -> FETCH_HEAD
  $ git push freenet +main:main
  ==> publishing 2.7 MiB pack as 3 chunks
  ==> done
   + a01f92c...b2a8836 main -> main (forced update)

The freenet-core demo (`freenet:3GEERif5ihbf/freenet-core`) was
also re-published manually as part of the diagnosis; current tip is
`b2a8836` snapshot of `229793ea5eb3eaab141ece6fe6951f42b18d3097`.

## Fixes

- Unblocks freenet-core's `Mirror to Freenet` workflow.
- Reported by user @ofansifkapital in #19; the user's clone failure
  was a separate network-propagation issue (their freenet node had
  just been restarted and had zero ring connections), but
  investigating it surfaced this real workflow bug.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sanity sanity merged commit 868c072 into main May 7, 2026
@sanity sanity deleted the fix-snapshot-fetch-before-push branch May 7, 2026 14:34
This was referenced May 7, 2026
Open
Closed
Merged
sanity added a commit that referenced this pull request May 7, 2026
@sanity @claude
fix: build_pack handles force-push when local lacks the remote tip (#26)
3309b72 
* fix: build_pack handles force-push when local lacks the remote tip

Closes #21.

## Problem

`build_pack` invoked `git rev-list ^<have> <want>` blindly. When
`<have>` is the remote tip reported by the contract state but the
local repo has never seen it (canonical case: force-pushing an orphan
commit, exactly what snapshot-mode mirroring does), rev-list bailed
with `fatal: bad object <sha>` and the entire push failed.

This was the root cause behind the freenet-core mirror failing 5/5
runs after merge. PR #20 worked around it at the workflow layer with
a `git fetch freenet $BRANCH` before each push; that fix keeps
working but adds bandwidth and only helps callers of the reusable
workflow.

## Fix

Two pieces:

1. `parse_push_spec` now returns `(force, src, dst)` instead of
   silently discarding the leading `+`.
2. `build_pack` takes the new `force: bool` and pre-validates `<have>`
   with `git cat-file -e`. If `<have>` is missing locally and the
   push is a force, drop `<have>` from the rev-list args and send
   everything reachable from `<want>` -- the right semantic for
   "replace whatever was there." If `<have>` is missing and the push
   is non-force, return a directed error pointing the user at
   `git fetch` or `+` rather than the cryptic `fatal: bad object`
   from rev-list.

## Tests

Six new unit tests in `git-remote-freenet.rs` covering:

- `parse_push_spec_force_flag` -- `+` is parsed as force, plain spec
  is non-force.
- `build_pack_no_have_succeeds` -- empty have is the first-push case.
- `build_pack_have_present_succeeds` -- existing fast-forward path.
- `build_pack_missing_have_non_force_fails_with_directed_error` --
  the non-force surface, directed error message.
- `build_pack_missing_have_force_drops_have_and_succeeds` -- the
  exact case that was broken before.
- `object_exists_distinguishes_real_from_bogus` -- pins the
  `cat-file -e` helper.

## End-to-end verification

Reproduced the original failure with the patched binary against the
live demo URL `freenet:3GEERif5ihbf/freenet-core`:

  $ # Fresh orphan repo, force push, no fetch:
  $ git push freenet +main:main
  ==> reading repo state from Freenet
  ==> publishing 226 B pack as a single bundle
  ==> updating repo state on Freenet
  ==> done
   + b2a8836...5d4a733 main -> main (forced update)

  $ # Non-force from fresh orphan -- gets the directed error:
  $ git push freenet main:main
   ! [remote rejected] main -> main (remote tip 5d4a7334... is not
     present in the local repo; run `git fetch` to populate it, or
     use `git push +`/`--force` to overwrite (which will discard the
     remote tip's history))

The freenet-core demo was clobbered briefly during testing and
re-published to its proper snapshot (`b2a8836`) after.

## Follow-up

Once this ships in a release and the caller workflow's pin is
bumped, drop the `git fetch` workaround from
`.github/workflows/mirror-repo.yml`.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review fixes: tighten object check, reorder error, harden tests

Addresses all four internal reviewers + Codex on PR #26.

Codex P2 / Code-first #1 / Skeptical M1: object existence check now
distinguishes "missing", "non-commit", and "tooling failure" cases.
Renamed `object_exists` -> `commit_exists` and reimplemented via
`git rev-parse --verify --quiet <sha>^{commit}`. The new check:

- Returns Ok(true) only if <sha> is present and is a commit object.
  cat-file -e succeeded on blobs and trees, which would have left
  the cryptic `fatal: not a commit object` rev-list error reachable
  for corrupted local stores.
- Distinguishes missing from corrupt-tooling via exit code 1 vs 128;
  the latter bubbles up rather than silently widening the pack
  (force) or producing a misleading "run git fetch" message
  (non-force).
- New test `commit_exists_rejects_blob_and_tree` pins this against
  blob and tree SHAs from the same repo.

Skeptical M2: directed error message reordered to lead with --force
since snapshot/orphan-style pushes are the canonical case driving
this PR. `git fetch` is still listed for the regular fast-forward
recovery scenario but no longer the first remediation.

Code-first #5: error message wording improved -- "use `git push +`"
isn't a real invocation; rephrased as "use `git push --force` (or
prefix the refspec with `+`)". The prior bail!(...) string was
preserved but rephrased.

Code-first #7 / Skeptical L3: test setup now sets per-repo
commit.gpgsign=false and tag.gpgsign=false, so a developer with
GPG signing on globally without a working key won't trip a prompt
in tests.

Code-first #8: `build_pack_have_present_succeeds` was a no-op
(let _ = pack). Renamed to `build_pack_have_equal_to_want_emits_
empty_pack` and now asserts the resulting pack is smaller than
the no-have pack -- pinning that rev-list short-circuits when
have == want.

Testing must-add #1: new test
`build_pack_fast_forward_have_is_real_ancestor` exercises the
production happy path -- have is the parent commit, want is the
child. Asserts the incremental pack is non-empty AND smaller than
the full pack, so a future regression that drops `^<have>` from
the rev-list args silently would be caught.

Testing should-add #3: directed-error wording now pinned via
substring assertions on "not present", "local repo", "git fetch",
"--force", and the bogus SHA itself.

Big-picture LOW: docstring "Phase 1.0 caveats" block in the helper
gained a "Push semantics" bullet documenting non-force vs force
expectations.

Pushed back / not addressing in this PR:

- Code-first #2 (silent SinglePack/ChunkedPack flip when force drops
  have for large `want` history): real concern but architectural;
  for snapshot mode the orphan commits are small so not a current
  practical issue. Worth a follow-up if a future caller pushes a
  large-history force.
- Code-first #4 (parse_push_spec allows force=true with empty src):
  caller rejects empty src before reading force; future-proofing
  with a defensive parser check is unnecessary today.
- Skeptical L2 (race between cat-file and rev-list under concurrent
  gc): theoretical, two-Command::status() window. The helper is
  invoked synchronously by git push; concurrent gc on the same
  repo is rare. Not in scope.
- Skeptical L4 (git init -b main needs git >= 2.28): all current
  CI runners have a modern git. Not in scope.
- Skeptical L5 (pack uploaded before contract validation runs):
  pre-existing ordering, not introduced here.
- Testing should-add #2 (parse-spec -> build-pack glue test): the
  existing test surface is sufficient given each function is
  unit-tested. A full handle_push test would require WS-API
  scaffolding that's overkill for this scope.
- Testing should-add #4 (bogus `want`): the existing
  `git_resolve_ref` call path catches that earlier in handle_push.
- Big-picture LOW (file follow-up issue for workflow workaround
  cleanup): tracked in issue body as "follow-up"; will file a
  separate ticket once this lands and the caller pin is bumped.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sanity sanity mentioned this pull request May 7, 2026
Merged
sanity added a commit that referenced this pull request May 7, 2026
@sanity @claude
chore: drop git-fetch workaround now that helper is fixed (#27)
41d69fe 
freenet-git 0.1.14 (PR #26 / issue #21) fixed the underlying
build_pack bug -- force pushes from a fresh orphan repo no longer
fail when the local lacks the remote tip. The workflow-side
workaround added in #20 is now redundant and would just waste
bandwidth on every snapshot push (it pulls the entire previous
chunked pack into local objects only to immediately overwrite it).

Caller workflows should bump their pinned SHA of mirror-repo.yml
to whatever this commit lands as.

[AI-assisted - Claude]

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sanity