fix(grant): resolve repos by name, github:handle grantees, repo clone URL (COR-699)

cmd/entire/cli/grant_test.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"slices"
 	"testing"
 
 	"github.com/entireio/cli/internal/coreapi"
@@ -20,41 +21,70 @@ func TestValidateGrantRole(t *testing.T) {
 	}
 }
 
-func TestParseGranteeMode(t *testing.T) {
+func TestGranteeName(t *testing.T) {
 	t.Parallel()
+	const ulid = "01HZX0000000000000000000AB"
 	tests := []struct {
-		name                                 string
-		provider, providerUserID, gType, gID string
-		want                                 granteeMode
-		wantErr                              bool
+		name string
+		in   coreapi.OptString
+		id   string
+		want string
 	}{
-		{name: "provider mode", provider: "github", providerUserID: "123", want: granteeModeProvider},
-		{name: "id mode", gType: "org", gID: "01J0", want: granteeModeID},
-		{name: "both modes rejected", provider: "github", providerUserID: "123", gType: "org", gID: "01J0", wantErr: true},
-		{name: "partial provider", provider: "github", wantErr: true},
-		{name: "partial id", gType: "org", wantErr: true},
-		{name: "nothing", wantErr: true},
+		{name: "friendly name wins", in: coreapi.NewOptString("github:alice"), id: ulid, want: "github:alice"},
+		{name: "unset falls back to ULID", in: coreapi.OptString{}, id: ulid, want: ulid},
+		{name: "empty string falls back to ULID", in: coreapi.NewOptString(""), id: ulid, want: ulid},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			got, err := parseGranteeMode(tt.provider, tt.providerUserID, tt.gType, tt.gID)
-			if tt.wantErr {
-				if err == nil {
-					t.Errorf("parseGranteeMode(%q,%q,%q,%q) expected error", tt.provider, tt.providerUserID, tt.gType, tt.gID)
-				}
-				return
-			}
-			if err != nil {
-				t.Fatalf("parseGranteeMode: %v", err)
-			}
-			if got != tt.want {
-				t.Errorf("got mode %d, want %d", got, tt.want)
+			if got := granteeName(tt.in, tt.id); got != tt.want {
+				t.Errorf("granteeName(%v, %q) = %q, want %q", tt.in, tt.id, got, tt.want)
 			}
 		})
 	}
 }
 
+func TestGrantRows(t *testing.T) {
+	t.Parallel()
+	const ulid = "01HZX0000000000000000000AB"
+
+	// grantColumns and the row builders must stay in lockstep — same width,
+	// same column order — or the table header and cells misalign.
+	if got, want := len(grantColumns), 5; got != want {
+		t.Fatalf("grantColumns has %d columns, want %d", got, want)
+	}
+
+	t.Run("project resolved name", func(t *testing.T) {
+		t.Parallel()
+		row := projectGrantRow(coreapi.ProjectGrant{
+			GranteeId:   ulid,
+			GranteeName: coreapi.NewOptString("github:alice"),
+			GranteeType: "account",
+			Role:        "writer",
+			Source:      "direct",
+		})
+		want := []string{"account", "github:alice", ulid, "writer", "direct"}
+		if !slices.Equal(row, want) {
+			t.Errorf("projectGrantRow = %v, want %v", row, want)
+		}
+	})
+
+	t.Run("repo unresolved name falls back to ULID", func(t *testing.T) {
+		t.Parallel()
+		row := repoGrantRow(coreapi.RepoGrant{
+			GranteeId:   ulid,
+			GranteeName: coreapi.OptString{},
+			GranteeType: "team",
+			Role:        "reader",
+			Source:      "inherited",
+		})
+		want := []string{"team", ulid, ulid, "reader", "inherited"}
+		if !slices.Equal(row, want) {
+			t.Errorf("repoGrantRow = %v, want %v", row, want)
+		}
+	})
+}
+
 func TestParseOrgRole(t *testing.T) {
 	t.Parallel()
 	tests := []struct {

cmd/entire/cli/grant_wiring_test.go

@@ -3,10 +3,13 @@ package cli
 import (
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/require"
+
+	"github.com/entireio/cli/internal/coreapi"
 )
 
 // Valid ULID-shaped refs (26 Crockford base32 chars, no I/L/O/U) so the remove
@@ -18,11 +21,34 @@ const (
 	wiringGranteeULID = "01HZX7QABCDEFGHJKMNPQRSTVZ"
 )
 
+// grantWiringHandler serves the handle-resolution GET (so a provider:handle
+// grantee resolves to a numeric provider user id) and records the subsequent
+// revoke DELETE. record is called with the DELETE's method and path; deleteFn
+// writes the DELETE response (e.g. 204 or a 404 problem).
+func grantWiringHandler(t *testing.T, record func(method, path string), deleteFn func(w http.ResponseWriter)) http.HandlerFunc {
+	t.Helper()
+	return func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/identity/handles/") {
+			w.Header().Set("Content-Type", "application/json")
+			if err := writeJSON(w, &coreapi.ResolvedIdentity{
+				AccountId:      wiringGranteeULID,
+				Provider:       providerGitHub,
+				Handle:         "alice",
+				ProviderUserId: "12345",
+			}); err != nil {
+				t.Errorf("encode identity: %v", err)
+			}
+			return
+		}
+		record(r.Method, r.URL.Path)
+		deleteFn(w)
+	}
+}
+
 // TestGrantRemove_RouteWiring drives the grant remove commands through cobra and
-// asserts the grantee-mode → route selection: --provider/--provider-user-id must
-// hit the by-provider revoke route, while --grantee-type/--grantee-id must hit
-// the typed-id route. This locks in the mode→route mapping that grant_test.go's
-// pure-helper tests (parseGranteeMode) can't observe.
+// asserts the grantee-form → route selection: a provider:handle grantee resolves
+// then hits the by-provider revoke route, while an account ULID hits the
+// typed-id route directly. This locks in the grantee→route mapping.
 //
 // Not parallel: runDeleteCmd swaps the package-level activeCoreClient seam.
 func TestGrantRemove_RouteWiring(t *testing.T) {
@@ -35,42 +61,42 @@ func TestGrantRemove_RouteWiring(t *testing.T) {
 		{
 			"repo/by-provider",
 			newGrantRepoRemoveCmd,
-			[]string{wiringRepoULID, "--provider", "github", "--provider-user-id", "12345"},
+			[]string{wiringRepoULID, "github:alice"},
 			"/api/v1/repos/" + wiringRepoULID + "/grants/account/github/12345",
 		},
 		{
 			"repo/by-grantee-id",
 			newGrantRepoRemoveCmd,
-			[]string{wiringRepoULID, "--grantee-type", "account", "--grantee-id", wiringGranteeULID},
+			[]string{wiringRepoULID, wiringGranteeULID},
 			"/api/v1/repos/" + wiringRepoULID + "/grants/account/" + wiringGranteeULID,
 		},
 		{
 			"project/by-provider",
 			newGrantProjectRemoveCmd,
-			[]string{wiringProjULID, "--provider", "github", "--provider-user-id", "12345"},
+			[]string{wiringProjULID, "github:alice"},
 			"/api/v1/projects/" + wiringProjULID + "/grants/account/github/12345",
 		},
 		{
 			"project/by-grantee-id",
 			newGrantProjectRemoveCmd,
-			[]string{wiringProjULID, "--grantee-type", "account", "--grantee-id", wiringGranteeULID},
+			[]string{wiringProjULID, wiringGranteeULID},
 			"/api/v1/projects/" + wiringProjULID + "/grants/account/" + wiringGranteeULID,
 		},
 		{
 			"org/by-provider",
 			newGrantOrgRemoveCmd,
-			[]string{wiringOrgULID, "--provider", "github", "--provider-user-id", "12345"},
+			[]string{wiringOrgULID, "github:alice"},
 			"/api/v1/orgs/" + wiringOrgULID + "/members/github/12345",
 		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			var gotMethod, gotPath string
-			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				gotMethod, gotPath = r.Method, r.URL.Path
-				w.WriteHeader(http.StatusNoContent)
-			}))
+			srv := httptest.NewServer(grantWiringHandler(t,
+				func(method, path string) { gotMethod, gotPath = method, path },
+				func(w http.ResponseWriter) { w.WriteHeader(http.StatusNoContent) },
+			))
 			t.Cleanup(srv.Close)
 
 			_, err := runDeleteCmd(t, tc.newCmd, srv.URL, tc.args...)
@@ -92,17 +118,18 @@ func TestGrantRemove_Idempotent(t *testing.T) {
 		newCmd func() *cobra.Command
 		args   []string
 	}{
-		{"repo/by-provider", newGrantRepoRemoveCmd, []string{wiringRepoULID, "--provider", "github", "--provider-user-id", "12345"}},
-		{"repo/by-grantee-id", newGrantRepoRemoveCmd, []string{wiringRepoULID, "--grantee-type", "account", "--grantee-id", wiringGranteeULID}},
-		{"project/by-provider", newGrantProjectRemoveCmd, []string{wiringProjULID, "--provider", "github", "--provider-user-id", "12345"}},
-		{"org/by-provider", newGrantOrgRemoveCmd, []string{wiringOrgULID, "--provider", "github", "--provider-user-id", "12345"}},
+		{"repo/by-provider", newGrantRepoRemoveCmd, []string{wiringRepoULID, "github:alice"}},
+		{"repo/by-grantee-id", newGrantRepoRemoveCmd, []string{wiringRepoULID, wiringGranteeULID}},
+		{"project/by-provider", newGrantProjectRemoveCmd, []string{wiringProjULID, "github:alice"}},
+		{"org/by-provider", newGrantOrgRemoveCmd, []string{wiringOrgULID, "github:alice"}},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-				writeNotFoundProblem(t, w)
-			}))
+			srv := httptest.NewServer(grantWiringHandler(t,
+				func(_, _ string) {},
+				func(w http.ResponseWriter) { writeNotFoundProblem(t, w) },
+			))
 			t.Cleanup(srv.Close)
 
 			out, err := runDeleteCmd(t, tc.newCmd, srv.URL, tc.args...)

cmd/entire/cli/repo.go

@@ -46,6 +46,22 @@ func repoRow(r coreapi.Repo) []string {
 	return []string{r.ID, r.Name, r.OwningProjectId, r.ClusterHost.Or("-"), state}
 }
 
+// repoDetailColumns / repoDetailRow extend the shared repo view with the
+// entire:// clone URL for the single-repo `get` output. The list view stays on
+// the lean repoColumns — a full clone URL per row would bloat the table — but a
+// person inspecting one repo wants the URL they can paste into `git clone`
+// (COR-699). REMOTE is "-" until the repo is provisioned enough to have a
+// resolvable cluster host + path.
+var repoDetailColumns = []string{"ID", "NAME", "PROJECT", "CLUSTER", "STATE", "REMOTE"}
+
+func repoDetailRow(r coreapi.Repo) []string {
+	remote := repoRemoteURL(r)
+	if remote == "" {
+		remote = "-"
+	}
+	return append(repoRow(r), remote)
+}
+
 // repoRemoteURL synthesizes the entire:// clone/remote URL for a repo from
 // its resolved cluster host and path — the form `git clone` and
 // `git remote add` accept, which git-remote-entire reads back as the repo
@@ -166,7 +182,7 @@ func newRepoGetCmd() *cobra.Command {
 		Short: "Show a repository by name or ULID",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runCoreObject(cmd, repoColumns, repoRow, func(ctx context.Context, c *coreapi.Client) (*coreapi.Repo, error) {
+			return runCoreObject(cmd, repoDetailColumns, repoDetailRow, func(ctx context.Context, c *coreapi.Client) (*coreapi.Repo, error) {
 				repoID, err := resolveRepoRef(ctx, c, args[0], project)
 				if err != nil {
 					return nil, err

cmd/entire/cli/repo_test.go

@@ -94,6 +94,41 @@ func TestParseVisibility(t *testing.T) {
 	}
 }
 
+func TestRepoDetailRow(t *testing.T) {
+	t.Parallel()
+
+	t.Run("includes the entire:// remote", func(t *testing.T) {
+		t.Parallel()
+		row := repoDetailRow(coreapi.Repo{
+			ID:              "01KS6KFJR2XS6PZ188MVYE07AN",
+			Name:            "web",
+			OwningProjectId: "01KS6KFJR2XS6PZ188MVYE07AP",
+			ClusterHost:     coreapi.NewOptString("aws-us-east-2.entire.io"),
+			Path:            coreapi.NewOptString("acme/web"),
+			State:           coreapi.NewOptRepoState(coreapi.RepoStateActive),
+		})
+		if len(row) != len(repoDetailColumns) {
+			t.Fatalf("row has %d cells, want %d (one per column)", len(row), len(repoDetailColumns))
+		}
+		if want := "entire://aws-us-east-2.entire.io/acme/web"; row[len(row)-1] != want {
+			t.Errorf("REMOTE cell = %q, want %q", row[len(row)-1], want)
+		}
+	})
+
+	t.Run("shows - when the remote is not yet resolvable", func(t *testing.T) {
+		t.Parallel()
+		row := repoDetailRow(coreapi.Repo{
+			ID:              "01KS6KFJR2XS6PZ188MVYE07AN",
+			Name:            "web",
+			OwningProjectId: "01KS6KFJR2XS6PZ188MVYE07AP",
+			ClusterHost:     coreapi.NewOptString("aws-us-east-2.entire.io"),
+		})
+		if row[len(row)-1] != "-" {
+			t.Errorf("REMOTE cell = %q, want %q", row[len(row)-1], "-")
+		}
+	})
+}
+
 func TestRepoCreateOutput_StampsRemote(t *testing.T) {
 	t.Parallel()
 	repo := &coreapi.Repo{

cmd/entire/cli/resolveref.go

@@ -20,6 +20,13 @@ import (
 // under the response's singular `org`/`project` field, or 404) — the CLI never
 // lists everything and filters client-side.
 
+// providerGitHub is the identity-provider slug for GitHub-backed accounts, the
+// provider half of a qualified grantee handle like "github:alice". GitHub is the
+// only provider with backing accounts today; other slugs resolve once they exist
+// server-side. (Distinct from setup.go's checkpointProviderGitHub, which names
+// the checkpoint hosting provider — same string, unrelated concern.)
+const providerGitHub = "github"
+
 // looksLikeULID reports whether s has the shape of a ULID: 26 characters drawn
 // from Crockford base32 (digits plus uppercase letters, excluding I, L, O, U).
 // The check is shape-only and case-insensitive on the alphabet; it never hits
@@ -95,6 +102,46 @@ func resolveAccountRef(ctx context.Context, c *coreapi.Client, ref string) (stri
 	return id.AccountId, nil
 }
 
+// resolveGranteeProvider turns a grantee reference into the (provider,
+// providerUserId) pair the grant/membership "by provider" routes key on. The
+// reference is a provider-qualified handle (e.g. "github:alice"); it is
+// resolved through the control plane to the provider's stable numeric user id.
+// The friendly handle alone is not what the grant routes accept — passing it as
+// --provider-user-id was the COR-699 footgun ("provider identity not found") —
+// so the CLI always resolves it first. A bare account ULID is rejected here:
+// the by-provider routes can't be addressed by ULID, and there is no reverse
+// account→provider-id lookup; callers that accept a ULID grantee (project/repo
+// remove) handle it via the typed-id route before reaching this helper.
+func resolveGranteeProvider(ctx context.Context, c *coreapi.Client, ref string) (provider, providerUserID string, err error) {
+	// A ULID is a tempting paste from `grant … list` (which prints the grantee
+	// ID), but the by-provider routes can't be addressed by ULID. Reject it with
+	// a message that points at the form this command actually wants, rather than
+	// letting parseQualifiedHandle dangle a "(or a ULID)" hint that doesn't apply.
+	if looksLikeULID(ref) {
+		return "", "", fmt.Errorf("grantee %q is an account ULID; this command needs a provider-qualified handle like \"github:alice\"", ref)
+	}
+	p, handle, err := parseQualifiedHandle(ref)
+	if err != nil {
+		return "", "", err
+	}
+	id, err := c.ResolveHandle(ctx, coreapi.ResolveHandleParams{Provider: p, Handle: handle})
+	if err != nil {
+		if isCoreNotFound(err) {
+			return "", "", fmt.Errorf("no %s identity for handle %q", p, handle)
+		}
+		return "", "", err
+	}
+	if id.ProviderUserId == "" {
+		return "", "", fmt.Errorf("handle %q resolved to no provider user id", ref)
+	}
+	// Prefer the server-normalized provider over the raw prefix, falling back to
+	// the input when the response omits it.
+	if id.Provider != "" {
+		p = id.Provider
+	}
+	return p, id.ProviderUserId, nil
+}
+
 // parseQualifiedHandle splits a provider-qualified handle like "github:alice"
 // into its provider ("github") and handle ("alice"). Accounts are addressed by
 // this friendly form; a value with no "provider:" prefix is rejected so the
@@ -132,10 +179,10 @@ func resolveProjectRef(ctx context.Context, c *coreapi.Client, ref string) (stri
 // resolveRepoRef turns a repo reference into its ULID. A ULID passes through.
 // A name requires a project scope (projectRef, itself a name or ULID) because
 // repo names are unique only within a project: the repo is resolved via the
-// server's case-insensitive by-name lookup, scoped to that project. A
-// name-filtered query returns the single match in the response's `repo`
-// field (empty when there's no match) — the `repos` array is only populated
-// for unfiltered list pages.
+// server's case-insensitive by-name lookup, scoped to that project. Like the
+// org/project endpoints, a name-filtered list returns the single match under the
+// response's singular `repo` field (the plural `repos` is only populated for an
+// unfiltered page) — reading `repos` here was the COR-699 bug.
 func resolveRepoRef(ctx context.Context, c *coreapi.Client, ref, projectRef string) (string, error) {
 	if looksLikeULID(ref) {
 		return ref, nil