Merge pull request #1744 from entireio/fix/1743-defer-checkpoint-push-empty-remote · Entire

Merge pull request #1744 from entireio/fix/1743-defer-checkpoint-push-empty-remote

3b6957b→main · pjbgf · 2d ago · 7 files · +273 added/-12 removed

fix(strategy): defer checkpoint push until a normal remote branch exists (#1743)

Changes

7

325 unmodified lines

326
327
328
329
329
330
331
332

325 unmodified lines

d.defer g.span.End()
        
g.logInvoked(slog.String("remote", remote))

hookErr := g.strategy.PrePush(g.ctx, remote)
        hookErr := g.strategy.PrePushFromGitHook(g.ctx, remote)
        g.logCompleted(hookErr)

// Propagate the error so the hook script exits non-zero and

Mcmd/entire/cli/hooks_git_cmd.go+1/-1

42 unmodified lines

43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97

42 unmodified lines

}
})

// TestGitPushWithHooks_DefersCheckpointsUntilFirstUserBranchExists ensures the
// user's own branch — not Entire metadata — is the first ref on a fresh remote.
//
// On the git-branch backend, entire/checkpoints/v1 is a real branch a forge
// could pick as the repository default, so its push is deferred until the
// user's branch has landed. On the git-refs backend, checkpoints live under
// refs/entire/*, which a forge cannot select as a default branch, so there is
// no hazard and they publish on the first push.
func TestGitPushWithHooks_DefersCheckpointsUntilFirstUserBranchExists(t *testing.T) {

t.Parallel()

ForEachBackend(t, func(t *testing.T, backend string) {
        env := NewFeatureBranchEnv(t)
        env.CheckpointStore = backend

bareDir := env.SetupEmptyNamedBareRemote("origin")
        branch := env.GetCurrentBranch()
        checkpointID := createCheckpointedCommit(t, env, "Add auth module", "auth.go", "package auth", "Add auth module")
        if checkpointID == "" {
            t.Fatal("should have a checkpoint ID after condensation")
        }

// The first push must land the user's branch on the empty remote.
        env.GitPushWithHooks("origin", "HEAD")
        if !env.BranchExistsOnRemote(bareDir, branch) {
            t.Fatalf("[%s] first user branch %q should be on remote", backend, branch)
        }

if backend == StoreGitRefs {
            // refs/entire/* can't become a default branch → no deferral.
            if !env.CheckpointExistsOnRemote(bareDir, checkpointID) {
                t.Fatalf("[git-refs] checkpoint %s should publish on the first push (no default-branch hazard)", checkpointID)
            }
            return
        }

// git-branch: the v1 branch must be withheld until the user branch exists.
        if env.CheckpointsPresentOnRemote(bareDir) {
            t.Fatalf("[git-branch] checkpoints must be deferred until after the first user branch push")
        }

// The first push created a remote-tracking ref, so a later push publishes.
        env.WriteFile("later.go", "package later")
        env.GitAdd("later.go")
        env.GitCommit("Later user commit")
        env.GitPushWithHooks("origin", "HEAD")
        if !env.CheckpointExistsOnRemote(bareDir, checkpointID) {
            t.Fatalf("[git-branch] deferred checkpoint %s should be published on a later push", checkpointID)
        }
    })
}

Mcmd/entire/cli/integration_test/real_hook_push_test.go+52

1803 unmodified lines

1804
1805
1806
1807
1808
1809
1810
1811
1812
1813
1814
1815
1816
1817
1818
1819
1820
1821
1822
1823
1824
1825
1826
1827
1828
18 unmodified lines

1847
1848
1849
1831
1832
1833
1834
1835
1836
1837
1850
1851
1852

1803 unmodified lines

// multiple remotes.
func (env *TestEnv) SetupNamedBareRemote(remoteName string) string {
    env.T.Helper()
    bareDir := env.SetupEmptyNamedBareRemote(remoteName)

// Push HEAD to the remote.
    cmd := exec.CommandContext(env.T.Context(), "git", "push", "--no-verify", "-u", remoteName, "HEAD")
    cmd.Dir = env.RepoDir
    cmd.Env = testutil.GitIsolatedEnv()
    if output, err := cmd.CombinedOutput(); err != nil {
        env.T.Fatalf("failed to push to %s: %v\n%s", remoteName, err, output)
    }

env.setGitConfigBaseline()

return bareDir
}

// SetupEmptyNamedBareRemote creates a bare git repository and adds it as a
// remote without pushing a branch. Use this to exercise first-push behavior.
func (env *TestEnv) SetupEmptyNamedBareRemote(remoteName string) string {
    env.T.Helper()

ctx := env.T.Context();

18 unmodified lines

env.T.Fatalf("failed to add remote %s: %v\n%s", remoteName, err, output)
    }

// Push HEAD to the remote
    cmd = exec.CommandContext(ctx, "git", "push", "--no-verify", "-u", remoteName, "HEAD")
    cmd.Dir = env.RepoDir
    cmd.Env = testutil.GitIsolatedEnv()
    if output, err := cmd.CombinedOutput(); err != nil {
        env.T.Fatalf("failed to push to %s: %v\n%s", remoteName, err, output)
    }

env.setGitConfigBaseline()

return bareDir
}

Mcmd/entire/cli/integration_test/testenv.go+19/-8

109 unmodified lines

110
111
112
113
114
115
116
117
118
119
120
11 unmodified lines

132
133
134
130
135
136
137
138
97 unmodified lines

236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269

109 unmodified lines

// setupV1Repo creates a repo + one v1 checkpoint with "PERSONABC" in
// both the transcript and prompt. Returns the repo and the v1 tip.
func setupV1Repo(t *testing.T) (*git.Repository, plumbing.Hash) {
    _, repo, tip := setupV1RepoInDir(t)
    return repo, tip
}

func setupV1RepoInDir(t *testing.T) (string, *git.Repository, plumbing.Hash) {
    t.Helper()
    tempDir := t.TempDir()
    testutil.InitRepo(t, tempDir)
11 unmodified lines

require.NoError(t, err)

tip := addV1Checkpoint(t, repo, "a1b2c3d4e5f6", "test-session", "Hello, PERSONABC asked", "Look up PERSONABC")
    return repo, tip
}

func addV1Checkpoint(t *testing.T, repo *git.Repository, cpIDString, sessionID, transcript, prompt string) plumbing.Hash {
97 unmodified lines

}))
}

func TestPrePushFromGitHook_DeferralStillRunsOPF(t *testing.T) {
    fake := &fakeOPFForRewrite{}
    configureFakeOPF(t, fake)

dir, repo, originalTip := setupV1RepoInDir(t)
    remoteDir := filepath.Join(t.TempDir(), "origin.git")
    _, err := git.PlainInit(remoteDir, true)
    require.NoError(t, err)
    _, err = repo.CreateRemote(&gitconfig.RemoteConfig{Name: "origin", URLs: []string{remoteDir}})
    require.NoError(t, err)

t.Chdir(dir)
    paths.ClearWorktreeRootCache()
    t.Cleanup(paths.ClearWorktreeRootCache)

// The empty remote defers Entire's automatic metadata push. The OPF rewrite
    // still must run because the user's outer git push may include v1 directly.
    require.NoError(t, NewManualCommitStrategy().PrePushFromGitHook(t.Context(), "origin"))

ref, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true)
    require.NoError(t, err)
    require.NotEqual(t, originalTip, ref.Hash(), "OPF rewrite must advance the local v1 ref before deferral")
    commit, err := repo.CommitObject(ref.Hash())
    require.NoError(t, err)
    require.True(t, trailers.HasOPFApplied(commit.Message))
    require.Equal(t, 1, fake.batchCallCount())
}

func TestRewriteUnpushedV1WithOPF_MultiCommitTipCarriesPriorRedactedShards(t *testing.T) {
    configureFakeOPF(t, &fakeOPFForRewrite{})
    repo, _ := setupV1Repo(t)

Mcmd/entire/cli/strategy/manual_commit_opf_rewrite_test.go+34/-1

6 unmodified lines

7
8
9
10
11
12
13
14
22 unmodified lines

37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
7 unmodified lines

61
62
63
51
52
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
56 unmodified lines

137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
10 unmodified lines

162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240

6 unmodified lines

"io"
    "log/slog"
    "os"
    "os/exec"
    "strings"

git "github.com/go-git/go-git/v6"
    "github.com/go-git/go-git/v6/plumbing"
22 unmodified lines

//   - push_sessions: false to disable automatic pushing of checkpoints
//   - checkpoint_remote: {"provider": "github", "repo": "org/repo"} to push to a separate repo
func (s *ManualCommitStrategy) PrePush(ctx context.Context, remote string) error {
    return s.prePush(ctx, remote, false)
}

// PrePushFromGitHook handles a push initiated by Git's pre-push hook. Unlike
// direct callers, it protects an empty user remote from receiving checkpoint
// metadata before the user's first normal branch is published.
func (s *ManualCommitStrategy) PrePushFromGitHook(ctx context.Context, remote string) error {
    return s.prePush(ctx, remote, true)
}

func (s *ManualCommitStrategy) prePush(ctx context.Context, remote string, protectFirstUserBranch bool) error {
    // Load settings once for remote resolution and push_sessions check.
    // Spanned because checkpoint-remote resolution can perform a one-time
    // network fetch of the metadata branch (fetchMetadataBranchIfMissing),
7 unmodified lines

}

// git-refs primary: push the per-checkpoint refs recorded in the push queue
    // instead of the single v1 branch. (A configured git-branch mirror's v1 ref
    // is not pushed here yet — mirror push for downgrade safety is a later step.)
    // instead of the single v1 branch. Those refs live under refs/entire/, not
    // refs/heads/, so a forge can never pick them as a repository's default
    // branch — the empty-remote guard below is unnecessary for this backend.
    // (A configured git-branch mirror's v1 ref is not pushed here yet — mirror
    // push for downgrade safety is a later step.)
    if cpCfg, _ := settings.LoadCheckpointsConfig(ctx); checkpoint.PrimaryIsRefs(cpCfg) { //nolint:errcheck // fail-soft: a bad checkpoints block already surfaces via Open; default to no refs push
        return s.prePushCheckpointRefs(ctx, ps)
    }

// git-branch primary: entire/checkpoints/v1 is a real refs/heads branch, so
    // on an otherwise-empty remote a forge like GitHub would select it as the
default. Defer publication until the user's own branch exists there.
    deferAutomaticCheckpointPush := protectFirstUserBranch && deferCheckpointPushOnEmptyRemote(ctx, ps)

refs := checkpoint.ResolveRefs(ctx)
    repo, repoErr := OpenRepository(ctx)
    if repoErr != nil {
56 unmodified lines

}

if deferAutomaticCheckpointPush {
        // Do this only after OPF has had a chance to rewrite v1: the outer
        // user push may explicitly include the metadata branch.
        logging.Info(ctx, "automatic checkpoint push deferred until the remote has a branch",
            slog.String("remote", ps.remote),
        )
        return nil
    }

// Thread the span's context into the push so the network push and any
    // fetch+rebase recovery nest beneath it as child steps in the perf trace.
    pushCtx, pushCheckpointsSpan := perf.Start(ctx, "push_checkpoint_refs")
10 unmodified lines

return nil
}

// deferCheckpointPushOnEmptyRemote reports whether publication of the git-branch
// v1 metadata should be held back because the push remote may be brand new.
//
// Hosting providers such as GitHub make the first branch pushed to an empty
// repository its default, so the pre-push hook must not publish
// entire/checkpoints/v1 ahead of the user's own first branch. The check is
// purely local: if a remote-tracking ref for this remote already exists
// (refs/remotes/<remote>/*), the remote has been fetched from or pushed to
// before and therefore already has at least one branch, so publishing cannot
// make our metadata the default. Otherwise defer — git records a
// remote-tracking ref after the first successful push, so the deferred metadata
// publishes on the next push.
//
// It deliberately performs no ls-remote/fetch. A network round trip on the
// pre-push path can trigger an SSH security-key touch prompt (and doing so per
// push URL would multiply those prompts), which is a poor pre-push UX. This is
// also why it uses only the remote git handed the hook rather than resolving
// every configured push URL.
//
// A separate checkpoint remote is exempt: it is a dedicated metadata store, not
// the repository the user pushes to.
func deferCheckpointPushOnEmptyRemote(ctx context.Context, ps pushSettings) bool {
    if ps.hasCheckpointURL() {
        return false
    }

// The hazard only arises for a configured remote (the `git remote add
    // origin …` then first-push flow). Pushing straight to a bare URL hands that
    // URL to the hook as the remote arg, and git never records a
    // refs/remotes/<url>/* tracking ref for it — so a tracking-ref check would
    // defer the metadata forever. Publish for a non-configured (URL) target
    // rather than strand it; the first-branch scenario always uses a named
    // remote.
    if !isConfiguredRemote(ctx, ps.remote) {
        return false
    }

// Known limitation, accepted for the no-network design: a tracking ref left
    // over from before a remote was deleted and recreated empty under the same
    // URL reads as "established", so v1 would publish to the now-empty remote.
    // Detecting that requires asking the remote — the network round trip we
    // deliberately avoid here. The scenario is rare and its default branch is
    // recoverable by resetting it on the forge.
    return !remoteHasTrackingRefs(ctx, ps.remote)
}

// isConfiguredRemote reports whether name is a configured git remote, as
// opposed to a bare URL that git passes through verbatim when a push targets a
// URL directly. Local and best-effort (reads config, no network); any error is
// treated as "not a configured remote".
func isConfiguredRemote(ctx context.Context, name string) bool {
    if name == "" {
        return false
    }
    return exec.CommandContext(ctx, "git", "remote", "get-url", name).Run() == nil
}

// remoteHasTrackingRefs reports whether any refs/remotes/<remote>/* ref exists
// locally. Its presence means the remote has been fetched from or pushed to
// before and so already has at least one branch. Local-only and best-effort:
// any error is treated as "no tracking refs" so the caller fails safe (defers).
func remoteHasTrackingRefs(ctx context.Context, remote string) bool {
    if remote == "" {
        return false
    }
    cmd := exec.CommandContext(ctx, "git", "for-each-ref", "--count=1", "refs/remotes/"+remote+"/")
    out, err := cmd.Output()
    if err != nil {
        return false
    }
    return strings.TrimSpace(string(out)) != ""
}

// prePushCheckpointRefs drains the per-checkpoint push queue and batch-pushes the
// recorded refs fast-forward-only (git-refs primary; never a force push — a
// diverged ref is recovered via fetch+replay). Transient push failures are logged and

Mcmd/entire/cli/strategy/manual_commit_push.go+105/-2

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56

package strategy

import (
    "context"
    "os/exec"
    "testing"

"github.com/entireio/cli/cmd/entire/cli/testutil"

"github.com/stretchr/testify/require"
)

// TestDeferCheckpointPushOnEmptyRemote_UsesLocalTrackingRefs verifies the guard
// decides purely from local remote-tracking refs, with no network access: a
// remote with no refs/remotes/<remote>/* is treated as possibly-empty (defer),
// and one with any tracking ref is treated as established (publish).
func TestDeferCheckpointPushOnEmptyRemote_UsesLocalTrackingRefs(t *testing.T) {
    // No t.Parallel: uses t.Chdir.
    dir := t.TempDir()
    testutil.InitRepo(t, dir)

run := func(args ...string) {
        t.Helper()
        cmd := exec.CommandContext(t.Context(), "git", args...)
        cmd.Dir = dir
        require.NoError(t, cmd.Run(), "git %v", args)
    }
    run("commit", "--allow-empty", "-m", "init")
    // A deliberately unreachable URL: the guard must never dial it.
    run("remote", "add", "origin", "https://example.invalid/repo.git")

t.Chdir(dir)
    ctx := context.Background()
    ps := pushSettings{remote: "origin"}

// No remote-tracking refs yet → possibly a brand-new remote → defer.
    require.True(t, deferCheckpointPushOnEmptyRemote(ctx, ps),
        "a remote with no tracking refs must defer")

// A push straight to a bare URL is not a configured remote; git never records
    // a tracking ref for it, so the guard must publish rather than defer forever.
    require.False(t,
        deferCheckpointPushOnEmptyRemote(ctx, pushSettings{remote: "https://example.invalid/repo.git"}),
        "a bare-URL push target must not defer")

// git records a remote-tracking ref after the first successful push; simulate
    // that locally (no network). The remote is now established → publish.
    run("update-ref", "refs/remotes/origin/main", "HEAD")
    require.False(t, deferCheckpointPushOnEmptyRemote(ctx, ps),
        "a remote with a tracking ref must not defer")

// A configured separate checkpoint remote is always exempt.
    require.False(t,
        deferCheckpointPushOnEmptyRemote(ctx, pushSettings{remote: "origin", checkpointURL: "https://example.invalid/cp.git"}),
        "a dedicated checkpoint remote is exempt from the guard")
}

Acmd/entire/cli/strategy/manual_commit_push_test.go+56

100 unmodified lines

101
102
103
104
105
106
107
108
109
110
111
112

100 unmodified lines

testutil.Git(t, work, "update-ref", "refs/heads/entire/checkpoints/v1", k2)
testutil.Git(t, work, "remote", "add", "origin", originBare)

// The remote already carries the v1 branch (seeded above). In a real repo
    // that means we hold a remote-tracking ref for it, so record one here: the
    // first-user-branch guard treats a remote with tracking refs as established
    // (non-empty) and runs the sync instead of deferring the push.
testutil.Git(t, work, "update-ref", "refs/remotes/origin/entire/checkpoints/v1", r1)

// Drive the real pre-push hook: non-ff vs the remote forces the sync/rebase
    // path that reads the alternate-resident checkpoint commits via go-git.
    cmd := exec.Command(entire.BinPath(), "hooks", "git", "pre-push", "origin")
}

Me2e/tests/alternates_test.go+6