fix checkpoint policy remote edge cases · Entire

fix checkpoint policy remote edge cases

32e54b0→main·

pfleidi·3w ago·5 files·+64 added/-19 removed

Propagate ancestry traversal errors instead of treating cancellation as divergence.

Clean up the temporary policy fetch ref on read failures and isolate git config in the remote policy tests that exercise git subprocesses.

Sessions

8a75ed6391f7View transcript

Changes

5

73 unmodified lines

74
75
76
77
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
25 unmodified lines

117
118
119
109
120
121
122
40 unmodified lines

163
164
165
166
167
168
169
3 unmodified lines

173
174
175
165
176
177
167
178
179
180
181
182
172
183
184
185
186
187
188
189
179
190
191
192
193
2 unmodified lines

196
197
198
188
199
200
190
201
202

73 unmodified lines

return baseline, nil
    }

if local.Hash.IsZero() || isAncestorOf(ctx, repo, local.Hash, baseline.Hash) {
    if local.Hash.IsZero() {
        if err := SetRef(repo, RefName, baseline.Hash); err != nil {
            return State{}, err
        }
        baseline.Source = SourceRemote
        return baseline, nil
    }
    localAncestor, err := isAncestorOf(ctx, repo, local.Hash, baseline.Hash)
    if err != nil {
        return State{}, err
    }
    if localAncestor {
        if err := SetRef(repo, RefName, baseline.Hash); err != nil {
            return State{}, err
        }
    }
{...
}
}

func isAncestorOf(ctx context.Context, repo *git.Repository, ancestor, target plumbing.Hash) bool {
func isAncestorOf(ctx context.Context, repo *git.Repository, ancestor, target plumbing.Hash) (bool, error) {
    if ancestor == target {
        return true
    }

iter, err := repo.Log(&git.LogOptions{From: target})
    if err != nil {
        return false
    }
    defer iter.Close()

found := false
    err = iter.ForEach(func(commit *object.Commit) error {
        if err := ctx.Err(); err != nil {
            return fmt.Errorf("traverse checkpoint policy ancestry: %w", err)
        }
        if commit.Hash == ancestor {
            found = true
        }
        return nil
    })
    if err != nil && !errors.Is(err, errStopTraversal) {
        return false,
    }
    return found
}

Mcmd/entire/cli/checkpointpolicy/remote.go+19/-8

1
2
3
4
5
6
7
8
9
10
11
12
16 unmodified lines

29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48

package checkpointpolicy

import (
    "context"
    "strings"
    "testing"

"github.com/go-git/go-git/v6"
    "github.com/go-git/go-git/v6/plumbing"
    "github.com/stretchr/testify/require"
)

16 unmodified lines

_, err = parseRemotePolicyHash(strings.Repeat("g", 40))
    require.ErrorContains(t, err, "invalid remote checkpoint policy hash")
}

func TestIsAncestorOfReturnsContextCancellation(t *testing.T) {
    t.Parallel()

repo, err := git.PlainInit(t.TempDir(), false)
    require.NoError(t, err)
    ancestor, err := WriteLocal(t.Context(), repo, plumbing.ZeroHash, DefaultPolicy())
    require.NoError(t, err)
    target, err := WriteLocal(t.Context(), repo, ancestor, DefaultPolicy())
    require.NoError(t, err)

ctx, cancel := context.WithCancel(t.Context())
    cancel()
    found, err := isAncestorOf(ctx, repo, ancestor, target)
    require.False(t, found)
    require.ErrorIs(t, err, context.Canceled)
}

Mcmd/entire/cli/checkpointpolicy/remote_internal_test.go+20

15 unmodified lines

16
17
18
19
19
20
21
5 unmodified lines

27
28
29
31
30
31
32
12 unmodified lines

45
46
47
50
48
49
50
8 unmodified lines

59
60
61
65
62
63
64
24 unmodified lines

89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
97
104
105
106
62 unmodified lines

169
170
171
172
173
174
175

15 unmodified lines

)

func TestSyncRemotePolicyDefaultsWhenRemoteMissing(t *testing.T) {
    t.Parallel()
    localDir, repo, bareDir := initPolicyRemoteFixture(t)

got, err := checkpointpolicy.Sync(t.Context(), repo, checkpointpolicy.Target{Remote: bareDir, Dir: localDir})
5 unmodified lines

}

func TestSyncRemotePolicyFetchesAndPromotesMissingLocalRef(t *testing.T) {
    t.Parallel()
    remoteDir, remoteRepo, bareDir := initPolicyRemoteFixture(t)
    remoteHash, err := checkpointpolicy.WriteLocal(t.Context(), remoteRepo, plumbing.ZeroHash, checkpointpolicy.DefaultPolicy())
    require.NoError(t, err)
12 unmodified lines

}

func TestSyncRemotePolicyDoesNotLeaveTempRefWhenSHAAlreadyMatches(t *testing.T) {
    t.Parallel()
    localDir, repo, bareDir := initPolicyRemoteFixture(t)
    localHash, err := checkpointpolicy.WriteLocal(t.Context(), repo, plumbing.ZeroHash, checkpointpolicy.DefaultPolicy())
    require.NoError(t, err)
8 unmodified lines

}

func TestSyncRemotePolicyKeepsDivergedLocalRef(t *testing.T) {
    t.Parallel()
    remoteDir, remoteRepo, bareDir := initPolicyRemoteFixture(t)
    baseHash, err := checkpointpolicy.WriteLocal(t.Context(), remoteRepo, plumbing.ZeroHash, checkpointpolicy.DefaultPolicy())
    require.NoError(t, err)
24 unmodified lines

requireNoRef(t, localRepo, "refs/entire/policies/checkpoint-fetch")
}

func TestSyncRemotePolicyRemovesTempRefWhenFetchedPolicyCannotBeRead(t *testing.T) {
    remoteDir, remoteRepo, bareDir := initPolicyRemoteFixture(t)
    writeRawPolicyCommit(t, remoteRepo, []byte(`{\"checkpoint_version\":`), plumbing.ZeroHash)
    pushPolicyRefWithGit(t, remoteDir, bareDir)

localDir, localRepo := initPolicyRepoWithDir(t)
    _, err := checkpointpolicy.Sync(t.Context(), localRepo, checkpointpolicy.Target{Remote: bareDir, Dir: localDir})
    require.ErrorContains(t, err, "parse policy.json")
    requireNoRef(t, localRepo, "refs/entire/policies/checkpoint-fetch")
}

func TestPushPolicyRejectsNonFastForward(t *testing.T) {
    t.Parallel()
    firstDir, firstRepo, bareDir := initPolicyRemoteFixture(t)
    _, err := checkpointpolicy.WriteLocal(t.Context(), firstRepo, plumbing.ZeroHash, checkpointpolicy.DefaultPolicy())
    require.NoError(t, err)
62 unmodified lines

func initPolicyRepoWithDir(t *testing.T) (string, *git.Repository) {
    t.Helper()
    testutil.IsolateGitConfigEnv(t)
    dir := t.TempDir()
    testutil.InitRepo(t, dir)
    repo, err := git.PlainOpen(dir)