Address review: fix the cutover fixture, and guard every unconfirmed update

main

Commit

nodo4d ago

The conditional delete was right but under-scoped, and the fixture that proved it was not proving it.

The fixture applied refs whose objects never arrived. A non-nil report from receivePackHook short-circuits the test server before transport.ReceivePack, so the pack on the cutover push was never unpacked: the marker resolved to a hash the target could not resolve, ~30 objects short. That inverted the helper's own claim that "the marker genuinely reaches the final checkpoint" — only the ref did — and hollowed out the resume assertion, since the retry's 334 bytes were a bare ref create against a target missing a whole batch. The server now unpacks the pushed objects before a hook's report wins (opt-in, so no other hook's target silently gains objects), and both tests assert the target resolves the marker and every commit reachable from the tip, not just hashes.

Two more fixture faults: the denier prefixed its own "ng " onto a status go-git already encodes as ng <ref> <status>, so the operator-visible reason arrived doubled — a test now pins the bare reason through to plan.Reason — and it ignored cmd.Old, leaving unenforced the very compare-and-swap whose stale-Old hazard is why it applies commands by hand. It now answers a mismatched Old the way a real receive-pack does.

Three more paths inferred "landed" from a nil error. The checkpoint loop advanced current without checking the temp-ref update it just pushed, so a target that refuses the whole request (read-only repo, blanket pre-receive block) had every later checkpoint negotiating against a have it never accepted; that now stops the run, naming the ref and the server's reason. The subsumed-branch path recorded a have and counted a batch for a lone create the target could have refused. And nothing consulted report-status: without it the pusher decodes no report at all, every ref looks unrejected, and the marker was deleted on a create nothing had confirmed — the exact hazard this PR fixes. Silence is now treated as "unknown, so keep the marker", and never as a refusal, which would fail every batched bootstrap against such a target.

A benign rejection no longer leaks the marker. "already exists" is a concurrent move: the branch IS on the target, so its scaffolding is stale and must be deleted, or it strands forever on the route that refuses --prune. gitproto.IsConcurrentMove is exported for that one question.

The predicate is now scoped to one push. s.rejections accumulates over the session, so asking it about a ref pushed in an earlier request was a false positive waiting to happen. gitproto.Pusher records the rejections of its most recent Push* call and answers Params.RefRefused from that; the recording rides the existing OnRejection callback, so a caller that installed none still gets a fatal per-ref ng rather than a silently best-effort push.

Tests: five bootstrap-package unit tests over a two-branch graph — refused create keeps the marker and offers it as a have (the multi-branch case the integration tests cannot reach), concurrent move deletes it, refused temp ref stops the run, unconfirmable create keeps it, refused subsumed create is not counted; two gitproto tests for the per-push window and the fatal path; one integration test for a target that never advertises report-status. Verified each new guard fails its test when removed. Full suite and golangci-lint green.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com Claude-Session: https://claude.ai/code/session_018BKBGBh3rJ6WGnZKDbBRy7 Entire-Checkpoint: 01M1BTSB3K4DTBP1AR616ARXTV

Checkpoints

Fix Batched Resume Marker on Branch Create Refusal

Claude CodeOpus 5
View session
Checkpoint 1