checkpoint_push_remote: reject pushurl-only remotes during election

cmd/entire/cli/integration_test/checkpoint_sync_remote_test.go

@@ -304,6 +304,78 @@ func TestCheckpointSyncRemote_TrackedPushCapturesElection(t *testing.T) {
 	})
 }
 
+func TestCheckpointSyncRemote_IncludedForkCapturesElection(t *testing.T) {
+	t.Parallel()
+	ForEachBackend(t, func(t *testing.T, backend string) {
+		env := NewFeatureBranchEnv(t)
+		env.CheckpointStore = backend
+
+		bareOrigin := env.SetupBareRemote()
+		bareFork := env.SetupNamedBareRemote(forkRemote)
+		testutil.RunGit(t, env.RepoDir, "config", "--local", "--unset-all", "remote.fork.url")
+		includedConfigPath := filepath.Join(t.TempDir(), "remotes.cfg")
+		if err := os.WriteFile(includedConfigPath,
+			[]byte(fmt.Sprintf("[remote %q]\n\turl = %s\n", forkRemote, bareFork)), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		testutil.RunGit(t, env.RepoDir, "config", "--local", "include.path", includedConfigPath)
+		env.setGitConfigBaseline()
+
+		checkpointID := createCheckpointedCommit(t, env, "Add included fork", "fork.go", "package fork", "Add included fork")
+		env.RunPrePush(forkRemote)
+
+		if !env.CheckpointExistsOnRemote(bareFork, checkpointID) {
+			t.Errorf("checkpoint %s should land on the included fork remote", checkpointID)
+		}
+		if env.CheckpointsPresentOnRemote(bareOrigin) {
+			t.Error("origin must not receive checkpoints from a push to the included fork")
+		}
+		if got := capturedSyncRemotesOnDisk(t, env); len(got) != 1 || got[0] != forkRemote {
+			t.Errorf("included fork push should capture the election, got %v", got)
+		}
+	})
+}
+
+// TestCheckpointSyncRemote_PushURLOnlyRemoteCannotCaptureElection verifies
+// that capture and election use the same fetch-URL eligibility rule. Git treats
+// a pushurl-only entry as a configured push target, but checkpoint sync cannot
+// read or reconcile from it, so a push there must carry no checkpoint data and
+// must not make the destination sticky.
+func TestCheckpointSyncRemote_PushURLOnlyRemoteCannotCaptureElection(t *testing.T) {
+	t.Parallel()
+	ForEachBackend(t, func(t *testing.T, backend string) {
+		env := NewFeatureBranchEnv(t)
+		env.CheckpointStore = backend
+
+		bareOrigin := env.SetupBareRemote()
+		barePushOnly := env.SetupNamedBareRemote("pushonly")
+		testutil.RunGit(t, env.RepoDir, "config", "remote.pushonly.pushurl", barePushOnly)
+		testutil.RunGit(t, env.RepoDir, "config", "--unset-all", "remote.pushonly.url")
+		env.setGitConfigBaseline()
+
+		checkpointID := createCheckpointedCommit(t, env, "Add capture guard", "guard.go", "package guard", "Add capture guard")
+
+		env.RunPrePush("pushonly")
+		if env.CheckpointsPresentOnRemote(barePushOnly) {
+			t.Error("a pushurl-only remote must not receive checkpoint data")
+		}
+		if got := capturedSyncRemotesOnDisk(t, env); got != nil {
+			t.Errorf("a pushurl-only remote must not capture the election, got %v", got)
+		}
+		if env.usingGitRefs() && queuedCheckpointRefCount(t, env) == 0 {
+			t.Error("push queue should be preserved after the ineligible push")
+		}
+
+		env.RunPrePush("origin")
+		if !env.CheckpointExistsOnRemote(bareOrigin, checkpointID) {
+			t.Errorf("checkpoint %s should remain available to the elected origin", checkpointID)
+		}
+		if env.usingGitRefs() && queuedCheckpointRefCount(t, env) != 0 {
+			t.Error("push queue should drain after pushing to origin")
+		}
+	})
+}
+
 // TestCheckpointSyncRemote_DeferredPushDoesNotCaptureElection covers the
 // gate-to-delivery gap on the most likely first push there is: the user adds a
 // brand-new empty fork and pushes their branch to it for the first time. That

cmd/entire/cli/status.go

@@ -440,8 +440,8 @@ func computeCheckpointSyncInfo(ctx context.Context, s *EntireSettings) checkpoin
 
 	elected, err := strategy.ResolveCheckpointSyncRemote(ctx)
 	if err != nil {
-		// Fail-closed: checkpoint_push_remote names a remote that does not
-		// exist. The pre-push gate is silently skipping checkpoint sync, so
+		// Fail-closed: election could not confirm a usable checkpoint sync
+		// remote. The pre-push gate is silently skipping checkpoint sync, so
 		// status is the user's signal.
 		// Accepted divergence: if a structured checkpoint_remote is also
 		// configured, the gate's dedicated exemption may still sync checkpoint

cmd/entire/cli/status_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -2928,24 +2929,48 @@ func TestRunStatus_CheckpointSyncDestination_CapturedAnnotated(t *testing.T) {
 
 func TestRunStatus_CheckpointSyncFailClosed(t *testing.T) {
 	testutil.IsolateGitConfigEnv(t)
-	setupTestRepo(t)
-	writeSettings(t, `{"enabled": true, "strategy_options": {"checkpoint_push_remote": "gone"}}`)
-	testutil.AddRemote(t, ".", "origin", "https://example.com/origin.git")
 
-	var stdout bytes.Buffer
-	if err := runStatus(context.Background(), &stdout, false, false); err != nil {
-		t.Fatalf("runStatus() error = %v", err)
-	}
+	for _, tt := range []struct {
+		name          string
+		remote        string
+		pushurlOnly   bool
+		emptyFetchURL bool
+		wantReason    string
+	}{
+		{name: "missing remote", remote: "gone"},
+		{name: "pushurl-only remote", remote: "pushonly", pushurlOnly: true, wantReason: "fetch URL"},
+		{name: "empty fetch URL", remote: "empty", emptyFetchURL: true, wantReason: "fetch URL"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			setupTestRepo(t)
+			writeSettings(t, fmt.Sprintf(`{"enabled": true, "strategy_options": {"checkpoint_push_remote": %q}}`, tt.remote))
+			if tt.pushurlOnly {
+				testutil.RunGit(t, ".", "config", "remote."+tt.remote+".pushurl", "https://example.com/pushonly.git")
+			}
+			if tt.emptyFetchURL {
+				testutil.RunGit(t, ".", "config", "remote."+tt.remote+".url", "")
+			}
+			testutil.AddRemote(t, ".", "origin", "https://example.com/origin.git")
 
-	out := stdout.String()
-	if !strings.Contains(out, "Checkpoints NOT syncing:") {
-		t.Errorf("expected fail-closed warning line, got:\n%s", out)
-	}
-	if !strings.Contains(out, `"gone"`) {
-		t.Errorf("fail-closed line should name the misconfigured remote, got:\n%s", out)
-	}
-	if strings.Contains(out, "Checkpoints sync to:") {
-		t.Errorf("fail-closed status must not also print a destination line, got:\n%s", out)
+			var stdout bytes.Buffer
+			if err := runStatus(context.Background(), &stdout, false, false); err != nil {
+				t.Fatalf("runStatus() error = %v", err)
+			}
+
+			out := stdout.String()
+			if !strings.Contains(out, "Checkpoints NOT syncing:") {
+				t.Errorf("expected fail-closed warning line, got:\n%s", out)
+			}
+			if !strings.Contains(out, fmt.Sprintf("%q", tt.remote)) {
+				t.Errorf("fail-closed line should name remote %q, got:\n%s", tt.remote, out)
+			}
+			if tt.wantReason != "" && !strings.Contains(out, tt.wantReason) {
+				t.Errorf("fail-closed line should contain %q, got:\n%s", tt.wantReason, out)
+			}
+			if strings.Contains(out, "Checkpoints sync to:") {
+				t.Errorf("fail-closed status must not also print a destination line, got:\n%s", out)
+			}
+		})
 	}
 }
 

cmd/entire/cli/strategy/checkpoint_sync_capture.go

@@ -102,7 +102,7 @@ func saveCapturedSyncRemote(ctx context.Context, name string) error {
 // the write idempotent — "first capture sticks" is decided when the state lands,
 // not when it was proposed.
 func pendingCaptureCheckpointSyncRemote(ctx context.Context, pushRemote string) bool {
-	if !isConfiguredRemote(ctx, pushRemote) {
+	if !isCheckpointSyncRemoteEligible(ctx, pushRemote) {
 		return false
 	}
 	root, err := capturedSyncRemotesRoot(ctx)

cmd/entire/cli/strategy/checkpoint_sync_remote.go

@@ -42,12 +42,12 @@ type CheckpointSyncRemote struct {
 
 // ResolveCheckpointSyncRemote elects the one configured git remote that
 // checkpoint data syncs to. Pure local lookup — no network. Precedence:
-// checkpoint_push_remote setting (fail-closed if the named remote does not
-// exist), then the captured election (evidence-elected by a past push that
-// agreed with the branch's declared push destination; fail-soft if that
-// remote is gone), then "origin", then the sole remote, then the first remote
-// in .git/config order. It knows nothing about the checkpoint_remote URL
-// feature; callers exempt that case themselves.
+// checkpoint_push_remote setting (fail-closed if the named remote has no
+// fetch URL), then the captured election (evidence-elected by a past push that
+// agreed with the branch's declared push destination; fail-soft if that remote
+// is no longer fetchable), then "origin", then the sole remote, then the first
+// remote in local config traversal order, including included files. It knows
+// nothing about the checkpoint_remote URL feature; callers exempt that case.
 //
 // Deliberately NOT keyed on the branch's tracking config alone
 // (branch.<name>.pushRemote / remote.pushDefault / branch.<name>.remote).
@@ -79,35 +79,40 @@ func ResolveCheckpointSyncRemote(ctx context.Context) (CheckpointSyncRemote, err
 	if err != nil {
 		return CheckpointSyncRemote{}, fmt.Errorf("cannot read settings to resolve the checkpoint sync remote: %w", err)
 	}
+	// Every tier elects from the same fetchable set. `git remote get-url`
+	// accepts a pushurl-only entry even though reads and reconciliation cannot.
+	fetchRemotes, err := configuredRemotesInConfigOrderResult(ctx)
+	if err != nil {
+		return CheckpointSyncRemote{}, fmt.Errorf("cannot read git remotes to resolve the checkpoint sync remote: %w", err)
+	}
 	if name := s.GetCheckpointPushRemote(); name != "" {
-		if !isConfiguredRemote(ctx, name) {
+		if !slices.Contains(fetchRemotes, name) {
 			return CheckpointSyncRemote{}, fmt.Errorf(
-				"checkpoint_push_remote %q is not a configured git remote; checkpoint sync disabled until fixed", name)
+				"checkpoint_push_remote %q has no configured fetch URL; checkpoint sync disabled until fixed", name)
 		}
 		return CheckpointSyncRemote{Name: name, Source: SyncRemoteSourceConfig}, nil
 	}
 
 	// Captured tier: fail-soft, unlike the explicit setting above — capture
-	// is automatic state, so a captured remote that was since renamed or
-	// removed falls through to the default tiers instead of disabling sync.
+	// is automatic state, so a captured remote that is no longer fetchable
+	// falls through to the default tiers instead of disabling sync.
 	for _, name := range loadCapturedSyncRemotes(ctx) {
-		if isConfiguredRemote(ctx, name) {
+		if slices.Contains(fetchRemotes, name) {
 			return CheckpointSyncRemote{Name: name, Source: SyncRemoteSourceObserved}, nil
 		}
-		logging.Debug(ctx, "captured checkpoint sync remote is not configured; falling through",
+		logging.Debug(ctx, "captured checkpoint sync remote has no configured fetch URL; falling through",
 			slog.String("remote", name))
 	}
 
-	remotes := configuredRemotesInConfigOrder(ctx)
 	switch {
-	case len(remotes) == 0:
+	case len(fetchRemotes) == 0:
 		return CheckpointSyncRemote{}, nil
-	case slices.Contains(remotes, "origin"):
+	case slices.Contains(fetchRemotes, "origin"):
 		return CheckpointSyncRemote{Name: "origin", Source: SyncRemoteSourceDefault}, nil
-	case len(remotes) == 1:
-		return CheckpointSyncRemote{Name: remotes[0], Source: SyncRemoteSourceSole}, nil
+	case len(fetchRemotes) == 1:
+		return CheckpointSyncRemote{Name: fetchRemotes[0], Source: SyncRemoteSourceSole}, nil
 	default:
-		return CheckpointSyncRemote{Name: remotes[0], Source: SyncRemoteSourceFirst}, nil
+		return CheckpointSyncRemote{Name: fetchRemotes[0], Source: SyncRemoteSourceFirst}, nil
 	}
 }
 
@@ -147,17 +152,17 @@ func checkpointSyncAllowedForRemote(ctx context.Context, pushRemote, pendingCapt
 // until the elected remote happens to be pushed.
 //
 // Stays quiet unless every condition holds: the push target is a configured
-// remote (checkpoint_push_remote takes a remote name, so a raw-URL push has
-// no actionable suggestion), the election succeeded AND was automatic (an
-// explicit checkpoint_push_remote is a decision already made, and the
-// fail-closed misconfigured case logs a warning through the gate itself), and
-// checkpoints are actually waiting. Fully local — no network.
+// remote with a fetch URL (checkpoint_push_remote must name one), the election
+// succeeded AND was automatic (an explicit checkpoint_push_remote is a
+// decision already made, and the fail-closed misconfigured case logs a warning
+// through the gate itself), and checkpoints are actually waiting. Fully local
+// — no network.
 //
 // The hint names .entire/settings.local.json: a remote name is a per-clone
 // fact, and committing it to the tracked settings.json would fail-close
 // checkpoint sync for every teammate whose clone lacks that remote name.
 func hintGatedCheckpointSync(ctx context.Context, pushRemote string) {
-	if !isConfiguredRemote(ctx, pushRemote) {
+	if !isCheckpointSyncRemoteEligible(ctx, pushRemote) {
 		return
 	}
 	syncRemote, err := ResolveCheckpointSyncRemote(ctx)
@@ -211,24 +216,34 @@ func hintGatedCheckpointSync(ctx context.Context, pushRemote string) {
 		slog.String("push_remote", pushRemote))
 }
 
-// configuredRemotesInConfigOrder lists remote names in .git/config section
-// order (approximates "first remote added"; `git remote` output is
-// alphabetical and unsuitable). Remotes configured with only pushurl are
-// deliberately invisible (spec Unit 1). Errors yield an empty list.
+// configuredRemotesInConfigOrder lists remote names in local config traversal
+// order, including included files (approximates "first remote added"; `git
+// remote` output is alphabetical and unsuitable). Remotes configured with only
+// pushurl are deliberately invisible. Errors yield an empty list.
 func configuredRemotesInConfigOrder(ctx context.Context) []string {
 	return cachedRemotesInConfigOrder(ctx, readRemotesInConfigOrder)
 }
 
-// readRemotesInConfigOrder lists remote names, distinguishing "this repo has no
-// remotes" from "the read failed". Both used to collapse to nil, which was
-// harmless while every caller re-ran the command — but the per-invocation cache
-// would memoize a failure's nil as a legitimately empty list and then skip
+func configuredRemotesInConfigOrderResult(ctx context.Context) ([]string, error) {
+	return cachedRemotesInConfigOrderResult(ctx, readRemotesInConfigOrder)
+}
+
+// isCheckpointSyncRemoteEligible keeps capture and hinting on the same
+// fetch-URL eligibility rule as election.
+func isCheckpointSyncRemoteEligible(ctx context.Context, name string) bool {
+	return slices.Contains(configuredRemotesInConfigOrder(ctx), name)
+}
+
+// readRemotesInConfigOrder lists remote names from local config and its includes,
+// distinguishing "this repo has no remotes" from "the read failed". Both used to
+// collapse to nil, which was harmless while every caller re-ran the command.
+// The per-invocation cache would memoize a failure's nil as an empty list and skip
 // checkpoint sync for the rest of the process. `git config --get-regexp` exits 1
 // for no match, so that exit code alone is the empty answer; anything else (a
 // fork failure under load, a cancelled context, a locked config) is an error the
 // cache must not keep.
 func readRemotesInConfigOrder(ctx context.Context) ([]string, error) {
-	cmd := exec.CommandContext(ctx, "git", "config", "--local", "--get-regexp", `^remote\..*\.url$`)
+	cmd := exec.CommandContext(ctx, "git", "config", "--local", "--includes", "--get-regexp", `^remote\..*\.url$`)
 	if worktreeRoot, ok := settings.WorktreeRoot(ctx); ok {
 		cmd.Dir = worktreeRoot
 	}
@@ -245,8 +260,8 @@ func readRemotesInConfigOrder(ctx context.Context) ([]string, error) {
 	for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") {
 		// line: "remote.<name>.url <url>"; <name> may contain dots, so trim
 		// the fixed prefix and the ".url <value>" suffix instead of splitting.
-		key, _, ok := strings.Cut(line, " ")
-		if !ok {
+		key, value, ok := strings.Cut(line, " ")
+		if !ok || strings.TrimSpace(value) == "" {
 			continue
 		}
 		name := strings.TrimSuffix(strings.TrimPrefix(key, "remote."), ".url")