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

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

2c41590→main·

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.

Changes

3

171 unmodified lines

172
173
174
175
175
176
177
178
179
180
179
180
181
182
183
184
185

171 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
    }

Mcmd/entire/cli/strategy/manual_commit_push.go+5/-3

36 unmodified lines

37
38
39
40
41
42
43
44
45
46
47
48
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
54
58
59
60
57
61
62
63
64

36 unmodified lines

return existing, stale
}

// 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).
func batchPushRefs(ctx context.Context, target string, refs []plumbing.ReferenceName) error {
    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 fmt.Errorf("push %d checkpoint refs: %w", len(refs), err)
    }
    return nil
}

Mcmd/entire/cli/strategy/push_common.go+15/-11

63 unmodified lines

64
65
66
67
67
68
69
70
71
71
72
73
74
6 unmodified lines

81
82
83
84
84
85
86
87
87
88
89
90
90
91
92
93
94
95
96
95
96
97
98
98
99
99
100
101
102
3 unmodified lines

106
107
108
109
109
110
111
112
113
111
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
115
116
154
155
156
157

63 unmodified lines

assert.Equal(t, []plumbing.ReferenceName{stale}, missing, "absent ref is stale")
}

func TestBatchForcePushRefs(t *testing.T) {
func TestBatchPushRefs(t *testing.T) {
    workDir, bareDir, refs := setupRepoWithCheckpointRefs(t)
    t.Chdir(workDir)

require.NoError(t, batchForcePushRefs(context.Background(), bareDir, refs))
    require.NoError(t, batchPushRefs(context.Background(), bareDir, refs))

// All refs now exist on the bare remote.
    lsCmd := exec.CommandContext(context.Background(), "git", "ls-remote", bareDir)
63 unmodified lines

}

func TestBatchForcePushRefs_Empty(t *testing.T) {
func TestBatchPushRefs_Empty(t *testing.T) {
    t.Parallel()
    // No refs → no git invocation, no error.
    require.NoError(t, batchForcePushRefs(context.Background(), "unused-target", nil))
    require.NoError(t, batchPushRefs(context.Background(), "unused-target", nil))
}

func TestBatchForcePushRefs_IsForcePush(t *testing.T) {
// TestBatchPushRefs_AllowsFastForward: advancing a checkpoint ref to a descendant
// commit (the normal case) pushes fine without force.
func TestBatchPushRefs_AllowsFastForward(t *testing.T) {
    workDir, bareDir, refs := setupRepoWithCheckpointRefs(t)
    t.Chdir(workDir)
    ctx := context.Background()

// First push establishes the refs on the remote.
    require.NoError(t, batchForcePushRefs(ctx, bareDir, refs))
    require.NoError(t, batchPushRefs(ctx, bareDir, refs))

// Re-point one ref at a new (unrelated, non-fast-forward) commit and push
    // again. A non-force push would be rejected; the force refspec must succeed.
    // Advance refs[0] to a child commit (fast-forward).
    repo, err := git.PlainOpen(workDir)
    require.NoError(t, err)

testutil.WriteFile(t, workDir, "two.txt", "second")
3 unmodified lines

require.NoError(t, err)
    require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(refs[0], head2.Hash())))

require.NoError(t, batchForcePushRefs(ctx, bareDir, refs[:1]), "force push must overwrite the remote ref")
    require.NoError(t, batchPushRefs(ctx, bareDir, refs[:1]), "fast-forward update should push without force")
    assert.Equal(t, head2.Hash().String(), remoteRefHash(t, bareDir, refs[0]),
        "remote ref should advance to the descendant commit")
}

lsCmd := exec.CommandContext(ctx, "git", "ls-remote", bareDir, refs[0].String())
// TestBatchPushRefs_RejectsNonFastForward: a divergent (non-descendant) update is
// rejected, and the remote ref is left untouched — the safety property that
// distinguishes this from a force push (we have no server-side ref protection).
func TestBatchPushRefs_RejectsNonFastForward(t *testing.T) {
    workDir, bareDir, refs := setupRepoWithCheckpointRefs(t)
    t.Chdir(workDir)
    ctx := context.Background()

require.NoError(t, batchPushRefs(ctx, bareDir, refs))
    original := remoteRefHash(t, bareDir, refs[0])

// Point refs[0] at an orphan commit (no parent) — not a descendant of what was
    // pushed, so the update is non-fast-forward.
    runGit := func(args ...string) string {
        c := exec.CommandContext(ctx, "git", args...)
        c.Dir = workDir
        c.Env = testutil.GitIsolatedEnv()
        out, gitErr := c.CombinedOutput()
        require.NoError(t, gitErr, "git %v failed: %s", args, out)
        return strings.TrimSpace(string(out))
    }
    tree := runGit("rev-parse", "HEAD^{tree}")
    orphan := runGit("commit-tree", tree, "-m", "divergent")
    repo, err := git.PlainOpen(workDir)
    require.NoError(t, err)
    require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(refs[0], plumbing.NewHash(orphan))))

err = batchPushRefs(ctx, bareDir, refs[:1])
    require.Error(t, err, "a non-fast-forward update must be rejected, not force-pushed")
    assert.Equal(t, original, remoteRefHash(t, bareDir, refs[0]),
        "remote ref must be unchanged after a rejected non-fast-forward push")
}

// remoteRefHash returns the object hash a ref points at on the bare remote.
func remoteRefHash(t *testing.T, bareDir string, ref plumbing.ReferenceName) string {
    t.Helper()
    lsCmd := exec.CommandContext(context.Background(), "git", "ls-remote", bareDir, ref.String())
    lsCmd.Env = testutil.GitIsolatedEnv()
    out, err := lsCmd.CombinedOutput()
    require.NoError(t, err, "ls-remote failed: %s", out)
    assert.True(t, strings.HasPrefix(strings.TrimSpace(string(out)), head2.Hash().String()),
        "remote ref should now point at the new commit")
    fields := strings.Fields(strings.TrimSpace(string(out)))
    require.NotEmpty(t, fields, "ref %s not found on remote", ref)
    return fields[0]
}