Merge pull request #1357 from entireio/dont-poll-empty-mirrors

repo mirror create: skip clone polling when upstream is empty

Author: stiak

cmd/entire/cli/repo_mirror.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"net/url"
 	"regexp"
@@ -12,6 +13,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/entireio/cli/cmd/entire/cli/auth"
 	"github.com/entireio/cli/internal/coreapi"
 )
 
@@ -145,25 +147,18 @@ func newRepoMirrorCreateCmd() *cobra.Command {
 					return err
 				}
 				out := cmd.OutOrStdout()
-				if created.Created {
-					fmt.Fprintf(out, "Registered mirror %s\n", created.MirrorId)
-				} else {
-					fmt.Fprintf(out, "Mirror already exists (%s)\n", created.MirrorId)
-				}
-				fmt.Fprintf(out, "  %s\n", created.MirrorUrl)
-				if noWait {
-					fmt.Fprintf(out, "Initial clone may still be in progress; `git clone %s` will work once it completes.\n", created.MirrorUrl)
-					return nil
-				}
-				if err := waitForMirrorClone(ctx, out, clusterHost, owner, repo, waitTimeout); err != nil {
-					if handled, serr := explainSuspendedMirror(cmd.ErrOrStderr(), created.MirrorId, created.Created, err); handled {
-						cmd.SilenceUsage = true
-						return serr
-					}
-					return err
-				}
-				fmt.Fprintf(out, "\nClone it:\n  git clone %s\n", created.MirrorUrl)
-				return nil
+				repoSlug := "/gh/" + owner + "/" + repo
+				return finishMirrorCreate(out, cmd.ErrOrStderr(), created, noWait,
+					func() error {
+						if _, terr := auth.RepoScopedToken(ctx, "https://"+clusterHost, repoSlug, "pull"); terr != nil {
+							return fmt.Errorf("probe mirror for suspension: %w", terr)
+						}
+						return nil
+					},
+					func() error {
+						return waitForMirrorClone(ctx, out, clusterHost, owner, repo, waitTimeout)
+					},
+				)
 			})
 		},
 	}
@@ -172,6 +167,62 @@ func newRepoMirrorCreateCmd() *cobra.Command {
 	return cmd
 }
 
+// finishMirrorCreate prints the post-create status for `repo mirror create`
+// and, unless noWait, makes sure the mirror is usable before returning.
+//
+// Empty upstream and suspended placement interact. An empty upstream has no
+// clone to wait for, so the HEAD-poll loop is skipped — it could only spin to
+// the timeout, since an empty repo never advertises a HEAD. But an *existing*
+// placement can be suspended even when its upstream is empty, and the
+// repo-scoped token exchange is the only signal that surfaces that; a *fresh*
+// create can't be suspended (suspension follows upstream access loss), so it
+// needs neither the probe nor the wait. Non-empty mirrors take the normal
+// clone-wait path.
+//
+// probeSuspended mints a repo-scoped pull token and returns its error for
+// explainSuspendedMirror to classify; waitClone runs the HEAD-poll loop. Both
+// are injected so the branching is unit-testable without the auth and
+// control-plane stack the production caller wires up.
+func finishMirrorCreate(out, errW io.Writer, created *coreapi.CreatedMirror, noWait bool, probeSuspended, waitClone func() error) error {
+	if created.Created {
+		fmt.Fprintf(out, "Registered mirror %s\n", created.MirrorId)
+	} else {
+		fmt.Fprintf(out, "Mirror already exists (%s)\n", created.MirrorId)
+	}
+	fmt.Fprintf(out, "  %s\n", created.MirrorUrl)
+
+	if created.Empty {
+		// An existing placement can sit behind a suspension even with an empty
+		// upstream, so probe the token exchange to surface it. A fresh create
+		// can't be suspended, so skip the probe there.
+		if !created.Created {
+			if err := probeSuspended(); err != nil {
+				if handled, serr := explainSuspendedMirror(errW, created.MirrorId, created.Created, err); handled {
+					return serr
+				}
+				// A non-suspension probe error isn't fatal: the placement exists
+				// and the upstream is genuinely empty, so report that rather than
+				// failing the create on a transient token hiccup.
+			}
+		}
+		fmt.Fprintln(out, "Upstream has no commits yet — nothing to clone. The mirror will pick up refs once the upstream is pushed to.")
+		return nil
+	}
+
+	if noWait {
+		fmt.Fprintf(out, "Initial clone may still be in progress; `git clone %s` will work once it completes.\n", created.MirrorUrl)
+		return nil
+	}
+	if err := waitClone(); err != nil {
+		if handled, serr := explainSuspendedMirror(errW, created.MirrorId, created.Created, err); handled {
+			return serr
+		}
+		return err
+	}
+	fmt.Fprintf(out, "\nClone it:\n  git clone %s\n", created.MirrorUrl)
+	return nil
+}
+
 func newRepoMirrorListCmd() *cobra.Command {
 	var cluster, provider, owner string
 	cmd := &cobra.Command{

cmd/entire/cli/repo_mirror_test.go

@@ -81,6 +81,143 @@ func TestExplainSuspendedMirror(t *testing.T) {
 	})
 }
 
+// TestFinishMirrorCreate exercises the post-create branching: when the
+// upstream is empty we must skip the HEAD-poll loop (an empty repo never
+// advertises a HEAD), yet an *existing* empty placement must still go through
+// the token exchange so a suspended mirror surfaces its resume guidance
+// instead of a success-style "nothing to clone" note.
+func TestFinishMirrorCreate(t *testing.T) {
+	t.Parallel()
+
+	const id = "01KS6KFJR2XS6PZ188MVYE07AN"
+	const mirrorURL = "entire://eu-west-1.entire.io/gh/octocat/hello-world"
+	// The error shape RepoScopedToken/waitForMirrorClone produce for a
+	// suspended (non-servable) placement.
+	suspended := fmt.Errorf("repo-scoped token exchange: %w", auth.ErrRepoTargetUnknown)
+
+	// seen records whether each injected operation ran, so we can assert the
+	// empty path never polls and a fresh create never probes.
+	type call struct{ probed, waited bool }
+
+	t.Run("fresh empty create skips both probe and poll", func(t *testing.T) {
+		t.Parallel()
+		var seen call
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: true, Empty: true, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { seen.probed = true; return nil },
+			func() error { seen.waited = true; return nil },
+		)
+		require.NoError(t, err)
+		require.False(t, seen.probed, "a fresh create can't be suspended; must not probe")
+		require.False(t, seen.waited, "empty upstream has nothing to clone; must not poll")
+		require.Contains(t, out.String(), "nothing to clone")
+		require.Empty(t, errW.String())
+	})
+
+	t.Run("existing empty healthy probes but does not poll", func(t *testing.T) {
+		t.Parallel()
+		var seen call
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: false, Empty: true, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { seen.probed = true; return nil },
+			func() error { seen.waited = true; return nil },
+		)
+		require.NoError(t, err)
+		require.True(t, seen.probed, "existing empty placement must probe for suspension")
+		require.False(t, seen.waited, "empty upstream has nothing to clone; must not poll")
+		require.Contains(t, out.String(), "nothing to clone")
+	})
+
+	t.Run("existing empty suspended surfaces resume guidance", func(t *testing.T) {
+		t.Parallel()
+		var seen call
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: false, Empty: true, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { seen.probed = true; return suspended },
+			func() error { seen.waited = true; return nil },
+		)
+		var silent *SilentError
+		require.ErrorAs(t, err, &silent, "suspended mirror must return a SilentError")
+		require.True(t, seen.probed)
+		require.False(t, seen.waited, "must not poll a suspended empty mirror")
+		require.Contains(t, errW.String(), "entire-core admin mirrors resume "+id)
+		require.NotContains(t, out.String(), "nothing to clone",
+			"a suspended mirror must not get the success-style empty note")
+	})
+
+	t.Run("existing empty transient probe error is non-fatal", func(t *testing.T) {
+		t.Parallel()
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: false, Empty: true, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { return errors.New("dial tcp: connection refused") },
+			func() error { t.Fatal("must not poll an empty mirror"); return nil },
+		)
+		require.NoError(t, err, "a non-suspension probe error must not fail a create whose placement exists")
+		require.Contains(t, out.String(), "nothing to clone")
+	})
+
+	t.Run("non-empty no-wait skips both probe and poll", func(t *testing.T) {
+		t.Parallel()
+		var seen call
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: true, Empty: false, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, true,
+			func() error { seen.probed = true; return nil },
+			func() error { seen.waited = true; return nil },
+		)
+		require.NoError(t, err)
+		require.False(t, seen.probed)
+		require.False(t, seen.waited, "--no-wait must not poll")
+		require.Contains(t, out.String(), "still be in progress")
+	})
+
+	t.Run("non-empty waits for clone then prints clone hint", func(t *testing.T) {
+		t.Parallel()
+		var seen call
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: true, Empty: false, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { seen.probed = true; return nil },
+			func() error { seen.waited = true; return nil },
+		)
+		require.NoError(t, err)
+		require.False(t, seen.probed, "non-empty path detects suspension through waitClone, not a separate probe")
+		require.True(t, seen.waited)
+		require.Contains(t, out.String(), "git clone "+mirrorURL)
+	})
+
+	t.Run("non-empty existing suspended surfaces resume guidance", func(t *testing.T) {
+		t.Parallel()
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: false, Empty: false, MirrorId: id, MirrorUrl: mirrorURL}
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { return nil },
+			func() error { return suspended },
+		)
+		var silent *SilentError
+		require.ErrorAs(t, err, &silent)
+		require.Contains(t, errW.String(), "entire-core admin mirrors resume "+id)
+		require.NotContains(t, out.String(), "git clone")
+	})
+
+	t.Run("non-empty wait error other than suspension propagates", func(t *testing.T) {
+		t.Parallel()
+		var out, errW bytes.Buffer
+		created := &coreapi.CreatedMirror{Created: true, Empty: false, MirrorId: id, MirrorUrl: mirrorURL}
+		wantErr := errors.New("timed out waiting for initial clone")
+		err := finishMirrorCreate(&out, &errW, created, false,
+			func() error { return nil },
+			func() error { return wantErr },
+		)
+		require.ErrorIs(t, err, wantErr)
+		require.Empty(t, errW.String())
+	})
+}
+
 // TestParseGitHubURL is ported from entiredb's cmd/entire-repo/cli
 // mirror_test.go, since parseGitHubURL was carried over verbatim.
 func TestParseGitHubURL(t *testing.T) {

internal/coreapi/oas_json_gen.go

@@ -371,6 +371,10 @@
             "description": "true on fresh creation; false when an existing mirror was returned.",
             "type": "boolean"
           },
+          "empty": {
+            "description": "true when the upstream has no refs to clone.",
+            "type": "boolean"
+          },
           "mirrorId": {
             "type": "string"
           },
@@ -385,7 +389,8 @@
           "mirrorId",
           "mirrorUrl",
           "publicUrl",
-          "created"
+          "created",
+          "empty"
         ],
         "type": "object"
       },