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

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

Author: pjbgf

cmd/entire/cli/hooks_git_cmd.go

@@ -326,7 +326,7 @@ func newHooksGitPrePushCmd() *cobra.Command {
 			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

cmd/entire/cli/integration_test/real_hook_push_test.go

@@ -43,3 +43,55 @@ func TestGitPushWithHooks_SyncsCheckpointsToRemote(t *testing.T) {
 		}
 	})
 }
+
+// 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)
+		}
+	})
+}

cmd/entire/cli/integration_test/testenv.go

@@ -1804,6 +1804,25 @@ func (env *TestEnv) SetupBareRemote() string {
 // 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()
 
@@ -1828,14 +1847,6 @@ func (env *TestEnv) SetupNamedBareRemote(remoteName string) string {
 		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

cmd/entire/cli/strategy/manual_commit_opf_rewrite_test.go

@@ -110,6 +110,11 @@ func configureFakeOPF(t *testing.T, rt testOPFRuntime) {
 // 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)
@@ -127,7 +132,7 @@ func setupV1Repo(t *testing.T) (*git.Repository, plumbing.Hash) {
 	require.NoError(t, err)
 
 	tip := addV1Checkpoint(t, repo, "a1b2c3d4e5f6", "test-session", "Hello, PERSONABC asked", "Look up PERSONABC")
-	return repo, tip
+	return tempDir, repo, tip
 }
 
 func addV1Checkpoint(t *testing.T, repo *git.Repository, cpIDString, sessionID, transcript, prompt string) plumbing.Hash {
@@ -231,6 +236,34 @@ func TestRewriteUnpushedV1WithOPF_HappyPath_RewritesAndTagsApplied(t *testing.T)
 	}))
 }
 
+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)

cmd/entire/cli/strategy/manual_commit_push.go

@@ -7,6 +7,8 @@ import (
 	"io"
 	"log/slog"
 	"os"
+	"os/exec"
+	"strings"
 
 	git "github.com/go-git/go-git/v6"
 	"github.com/go-git/go-git/v6/plumbing"
@@ -35,6 +37,17 @@ var opfPrePushProgressWriter io.Writer = os.Stderr
 //   - 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),
@@ -48,12 +61,20 @@ func (s *ManualCommitStrategy) PrePush(ctx context.Context, remote string) error
 	}
 
 	// 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 {
@@ -116,6 +137,15 @@ func (s *ManualCommitStrategy) PrePush(ctx context.Context, remote string) error
 		}
 	}
 
+	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")
@@ -132,6 +162,79 @@ func (s *ManualCommitStrategy) PrePush(ctx context.Context, remote string) error
 	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

cmd/entire/cli/strategy/manual_commit_push_test.go

@@ -0,0 +1,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")
+}

e2e/tests/alternates_test.go

@@ -101,6 +101,12 @@ func TestAlternates_RelativeObjectAlternate_CheckpointSync(t *testing.T) {
 	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")