Push git-refs checkpoints fast-forward-only (no force)

42ce58c·

Soph·2w ago·3 files·+73 added/-27 removed

There is no server-side ref protection, so force-pushing per-checkpoint refs by
default risks a buggy or racing client silently clobbering good remote history.
Switch batchForcePushRefs -> batchPushRefs using a plain ref:ref refspec
(fast-forward-only): per-checkpoint refs normally advance by fast-forward so the
common case still succeeds, while a genuine non-fast-forward divergence (e.g. the
same checkpoint written differently elsewhere) is REJECTED rather than
overwritten. On rejection the pre-push logs and leaves the refs queued (not
overwritten); reconciling a diverged ref is deferred, and a future rewrite path
(e.g. OPF) can use --force-with-lease where it must replace a ref.

Tests: replace the force-overwrite assertion with AllowsFastForward (a descendant
update pushes without force) and RejectsNonFastForward (an orphan/divergent
update errors and leaves the remote ref unchanged).

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

Sessions

d3e6c662ead3View transcript

Changes

3

• cmd/entire/cli/strategy

  • Mmanual_commit_push.go+5/-3

  • Mpush_common.go+15/-11

  • Mrefs_push_test.go+53/-13

163 unmodified lines

pushCtx, pushSpan := perf.Start(ctx, "push_checkpoint_refs")
pushErr := batchForcePushRefs(pushCtx, ps.pushTarget(), existing)
pushErr := batchPushRefs(pushCtx, ps.pushTarget(), existing)
pushSpan.End()
if pushErr != nil {
// Leave the refs queued; the next pre-push retries. Non-fatal so the
// user's push proceeds.
logging.Warn(ctx, "git-refs pre-push: batch push failed; refs left queued for retry", // user's push proceeds. The push is fast-forward-only, so this can mean a
// checkpoint ref diverged on the remote (non-fast-forward) — we leave it
// queued rather than force-overwriting it.
logging.Warn(ctx, "git-refs pre-push: checkpoint ref push failed (possible non-fast-forward divergence); refs left queued, not overwritten", slog.String("error", pushErr.Error())) return nil }

```go
// batchForcePushRefs pushes all of refs to target in a single git push. Each  
// uses a force refspec (+ref:ref), matching how non-branch checkpoint refs are  
// already pushed: per-checkpoint refs have independent histories with no  
// remote-tracking shadow, so there is no fast-forward to preserve and no  
// fetch+rebase recovery to attempt. Batching keeps a backfill of many refs to  
// one network round-trip. It is all-or-nothing: on error the caller leaves every  
// ref queued for the next pre-push (partial-failure reconciliation is out of  
// scope for now).  
func batchForcePushRefs(ctx context.Context, target string, refs []plumbing.ReferenceName) error {
    // batchPushRefs pushes all of refs to target in a single git push,
    // fast-forward-only (NOT a force push). Batching keeps a backfill of many refs to
    // one network round-trip. Per-checkpoint refs normally advance by fast-forward
    // (each write parents on the prior tip), so the common case succeeds; a
    // non-fast-forward update — genuine divergence, e.g. the same checkpoint written
    // differently on another machine — is REJECTED rather than silently overwriting
    // the remote. We deliberately do not force: there is no server-side ref
    // protection, so a force push would make a buggy or racing client clobber good
    // remote history with no signal. On rejection the whole push errors and the
    // caller leaves the refs queued (not overwritten) for a later pre-push;
    // reconciling a genuinely diverged ref is deferred (a future rewrite path, e.g.
    // OPF, would use --force-with-lease for the cases that must replace a ref).
    if len(refs) == 0 {
        return nil
    }
    refSpecs := make([]string, 0, len(refs))
    for _, ref := range refs {
        refSpecs = append(refSpecs, "+"+ref.String()+":"+ref.String())
        refSpecs = append(refSpecs, ref.String()+":"+ref.String())
    }
    if _, err := remote.PushWithOptions(ctx, remote.PushOptions{Remote: target, RefSpecs: refSpecs}); err != nil {
        return fmt.Errorf("batch push %d checkpoint refs: %w", len(refs), err)
    }
    return nil
}

// TestBatchForcePushRefs tests the batchForcePushRefs function.
func TestBatchForcePushRefs(t *testing.T) {
    // Implementation....
}

// TestBatchPushRefs tests the batchPushRefs function.
func TestBatchPushRefs(t *testing.T) {
    // Implementation....
}