Merge pull request #1845 from entireio/alisha/ent-1047-semantic-search-cli-multi-repo

feat(search): accept multiple repos in semantic `entire search` (ENT-1047)

Author: alishakawaguchi

cmd/entire/cli/search/search.go

@@ -12,6 +12,8 @@ import (
 	"strings"
 	"time"
 
+	ulid "github.com/oklog/ulid/v2"
+
 	"github.com/entireio/cli/cmd/entire/cli/api"
 )
 
@@ -424,7 +426,7 @@ func ParseSearchInput(raw string) ParsedInput {
 		case strings.HasPrefix(tok, "branch:"):
 			p.Branch = strings.Trim(tok[len("branch:"):], "\"")
 		case strings.HasPrefix(tok, "repo:"):
-			p.Repos = appendUnique(p.Repos, parseListFilter(strings.TrimPrefix(tok, "repo:"))...)
+			p.Repos = AppendUnique(p.Repos, parseListFilter(strings.TrimPrefix(tok, "repo:"))...)
 		default:
 			queryParts = append(queryParts, tok)
 		}
@@ -490,32 +492,57 @@ func parseListFilter(raw string) []string {
 	return values
 }
 
-// ValidateRepoFilters ensures repo filters match backend semantics.
+// ValidateRepoFilters ensures each repo filter matches backend semantics.
+// Multiple explicit repo filters are accepted: the v4 query-serve path resolves
+// each and fans out across the cells hosting them, mirroring code search.
 func ValidateRepoFilters(repos []string) error {
-	if len(repos) > 1 {
-		return errors.New("only one explicit repo filter is currently supported")
-	}
-	if len(repos) == 1 && !isValidRepoFilter(repos[0]) {
-		return fmt.Errorf(
-			"invalid repo filter %q: expected owner/name or *; if you meant all repos, quote the asterisk: --repo '*'",
-			repos[0],
-		)
+	for _, repo := range repos {
+		if !isValidRepoFilter(repo) {
+			return fmt.Errorf(
+				"invalid repo filter %q: expected owner/name, gh/owner/repo, a repo ULID, or *; if you meant all repos, quote the asterisk: --repo '*'",
+				repo,
+			)
+		}
 	}
 	return nil
 }
 
+// isValidRepoFilter reports whether repo is a filter shape the search backends
+// can resolve. It accepts every form the CLI help advertises and that the
+// resolvers handle downstream — a bare owner/name slug, a prefixed path
+// (gh/owner/repo, et/proj/repo, git/owner/repo), a raw repo ULID, or the
+// all-repos wildcard — so validation never rejects a filter the semantic v4
+// lookup (lookupFilter) or code-search resolver (resolveRepoFilters) would
+// otherwise resolve. It still rejects obvious mistakes like a bare filename.
 func isValidRepoFilter(repo string) bool {
 	if repo == AllReposFilter {
 		return true
 	}
-	if strings.Contains(repo, " ") {
+	if repo == "" || strings.Contains(repo, " ") {
 		return false
 	}
+	// Raw repo ULID: the v4 route keys on ULIDs and lookupFilter matches a
+	// prefix-less token against repo IDs.
+	if _, err := ulid.Parse(repo); err == nil {
+		return true
+	}
+	// A slug or prefixed path: owner/name or <prefix>/owner/repo. Every
+	// path segment must be non-empty.
 	parts := strings.Split(repo, "/")
-	return len(parts) == 2 && parts[0] != "" && parts[1] != ""
+	if len(parts) < 2 || len(parts) > 3 {
+		return false
+	}
+	for _, part := range parts {
+		if part == "" {
+			return false
+		}
+	}
+	return true
 }
 
-func appendUnique(existing []string, values ...string) []string {
+// AppendUnique appends values to existing, skipping any already present, and
+// returns the result. Order is preserved (first occurrence wins).
+func AppendUnique(existing []string, values ...string) []string {
 	if len(values) == 0 {
 		return existing
 	}

cmd/entire/cli/search/search_test.go

@@ -572,15 +572,23 @@ func TestParseSearchInput_AllReposFilter(t *testing.T) {
 	}
 }
 
-func TestValidateRepoFilters_RejectsMultipleRepos(t *testing.T) {
+func TestValidateRepoFilters_AllowsMultipleRepos(t *testing.T) {
 	t.Parallel()
 
-	err := ValidateRepoFilters([]string{"entirehq/entire.io", "entireio/cli"})
+	if err := ValidateRepoFilters([]string{"entirehq/entire.io", "entireio/cli"}); err != nil {
+		t.Errorf("expected multiple valid repo filters to be accepted, got: %v", err)
+	}
+}
+
+func TestValidateRepoFilters_RejectsInvalidAmongMultiple(t *testing.T) {
+	t.Parallel()
+
+	err := ValidateRepoFilters([]string{"entireio/cli", "AGENTS.md"})
 	if err == nil {
-		t.Fatal("expected validation error")
+		t.Fatal("expected validation error for an invalid repo among valid ones")
 	}
-	if got := err.Error(); got != "only one explicit repo filter is currently supported" {
-		t.Errorf("error = %q", got)
+	if got := err.Error(); !strings.Contains(got, `invalid repo filter "AGENTS.md"`) {
+		t.Errorf("error = %q, want it to name the invalid repo", got)
 	}
 }
 
@@ -591,12 +599,52 @@ func TestValidateRepoFilters_RejectsInvalidRepoValue(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected validation error")
 	}
-	want := "invalid repo filter \"AGENTS.md\": expected owner/name or *; if you meant all repos, quote the asterisk: --repo '*'"
+	want := "invalid repo filter \"AGENTS.md\": expected owner/name, gh/owner/repo, a repo ULID, or *; if you meant all repos, quote the asterisk: --repo '*'"
 	if got := err.Error(); got != want {
 		t.Errorf("error = %q, want %q", got, want)
 	}
 }
 
+// The CLI --repo help advertises gh/owner/repo, et/proj/repo, and raw ULIDs,
+// and the semantic v4 lookup + code-search resolver both handle them. Validation
+// must accept the same set so it never rejects a filter that would resolve
+// downstream (ENT-1047 review finding).
+func TestValidateRepoFilters_AcceptsAdvertisedFormats(t *testing.T) {
+	t.Parallel()
+
+	valid := []string{
+		"entireio/cli",               // bare owner/name slug
+		"gh/entireio/cli",            // GitHub prefixed path
+		"et/proj/repo",               // Entire project prefixed path
+		"git/owner/repo",             // generic git prefixed path
+		"01ARZ3NDEKTSV4RRFFQ69G5FAV", // raw repo ULID (canonical)
+		"*",                          // all-repos wildcard
+	}
+	for _, repo := range valid {
+		if err := ValidateRepoFilters([]string{repo}); err != nil {
+			t.Errorf("ValidateRepoFilters(%q) = %v, want nil", repo, err)
+		}
+	}
+}
+
+func TestValidateRepoFilters_RejectsMalformed(t *testing.T) {
+	t.Parallel()
+
+	invalid := []string{
+		"AGENTS.md",  // bare filename, not a ULID or slug
+		"owner/",     // empty name segment
+		"/repo",      // empty owner segment
+		"a/b/c/d",    // too many path segments
+		"owner name", // contains a space
+		"gh//repo",   // empty middle segment in a prefixed path
+	}
+	for _, repo := range invalid {
+		if err := ValidateRepoFilters([]string{repo}); err == nil {
+			t.Errorf("ValidateRepoFilters(%q) = nil, want validation error", repo)
+		}
+	}
+}
+
 func TestParseSearchInput_QuotedAuthor(t *testing.T) {
 	t.Parallel()
 	p := ParseSearchInput(`author:"` + testAuthor + ` smith" fix bug`)

cmd/entire/cli/search_cmd.go

@@ -34,7 +34,7 @@ func newSearchCmd() *cobra.Command { //nolint:maintidx // command wiring is inhe
 		authorFlag       string
 		dateFlag         string
 		branchFlag       string
-		repoFlag         string
+		repoFlags        []string
 		allReposFlag     bool
 		insecureHTTPAuth bool
 	)
@@ -87,11 +87,8 @@ branch:<name>, repo:<owner/name>, and repo:* to search all accessible repos.`,
 				// literal search text so "author:foo" searches for that
 				// string in code rather than being silently consumed.
 				codeQuery, inlineRepos := extractInlineRepoFilters(query)
-				var codeRepos []string
-				if repoFlag != "" {
-					codeRepos = []string{repoFlag}
-				}
-				codeRepos = append(codeRepos, inlineRepos...)
+				codeRepos := search.AppendUnique(nil, repoFlags...)
+				codeRepos = search.AppendUnique(codeRepos, inlineRepos...)
 				// repo:* or --all-repos means "all repos" — no filter.
 				// Otherwise, if no explicit filter was given, scope to the
 				// current repo (matching the checkpoint-search default).
@@ -149,10 +146,10 @@ branch:<name>, repo:<owner/name>, and repo:* to search all accessible repos.`,
 			if branchFlag == "" {
 				branchFlag = parsed.Branch
 			}
-			repos := parsed.Repos
-			if repoFlag != "" {
-				repos = []string{repoFlag}
-			}
+			// Merge --repo flag values with inline repo: filters (flags first),
+			// deduped. Repeatable/comma-separated --repo mirrors code-search UX.
+			repos := search.AppendUnique(nil, repoFlags...)
+			repos = search.AppendUnique(repos, parsed.Repos...)
 			if err := search.ValidateRepoFilters(repos); err != nil {
 				return fmt.Errorf("validating repo filter: %w", err)
 			}
@@ -306,7 +303,7 @@ branch:<name>, repo:<owner/name>, and repo:* to search all accessible repos.`,
 	cmd.Flags().StringVar(&authorFlag, "author", "", "Filter by author name")
 	cmd.Flags().StringVar(&dateFlag, "date", "", "Filter by time period (week or month)")
 	cmd.Flags().StringVar(&branchFlag, "branch", "", "Filter by branch name")
-	cmd.Flags().StringVar(&repoFlag, "repo", "", "Filter by repository (gh/owner/repo, et/proj/repo, owner/repo, ULID, or *)")
+	cmd.Flags().StringSliceVar(&repoFlags, "repo", nil, "Filter by repository (gh/owner/repo, et/proj/repo, owner/repo, ULID, or *); repeatable and comma-separated for multiple repos")
 	cmd.Flags().BoolVar(&allReposFlag, "all-repos", false, "Search all accessible repos instead of just the current one")
 	addInsecureHTTPAuthFlag(cmd, &insecureHTTPAuth)
 

cmd/entire/cli/search_cmd_test.go

@@ -815,6 +815,40 @@ func TestSearchCmd_MultipleInlineRepoFilters(t *testing.T) {
 	}
 }
 
+func TestSearchCmd_SemanticMultipleRepoFlags(t *testing.T) {
+	// Semantic search (no --code) must accept multiple repos via a repeatable
+	// --repo flag (ENT-1047) — parity with code search. It fails later at
+	// auth/git, but must not be rejected as an invalid/unsupported filter.
+	root := NewRootCmd()
+	root.SetArgs([]string{"search", "auth", "--repo", "entirehq/entire.io", "--repo", "entireio/cli"})
+
+	err := root.Execute()
+	if err != nil {
+		if strings.Contains(err.Error(), "validating repo filter") {
+			t.Errorf("multiple --repo flags should pass validation, got: %v", err)
+		}
+		if strings.Contains(err.Error(), "only one explicit repo filter") {
+			t.Errorf("multiple repos should no longer be rejected, got: %v", err)
+		}
+	}
+}
+
+func TestSearchCmd_SemanticCommaSeparatedRepoFlag(t *testing.T) {
+	// A single comma-separated --repo value must expand to multiple repos.
+	root := NewRootCmd()
+	root.SetArgs([]string{"search", "auth", "--repo", "entirehq/entire.io,entireio/cli"})
+
+	err := root.Execute()
+	if err != nil {
+		if strings.Contains(err.Error(), "validating repo filter") {
+			t.Errorf("comma-separated --repo should pass validation, got: %v", err)
+		}
+		if strings.Contains(err.Error(), "only one explicit repo filter") {
+			t.Errorf("comma-separated repos should no longer be rejected, got: %v", err)
+		}
+	}
+}
+
 func TestWriteCodeSearchJSON_RepoFilteredEmpty(t *testing.T) {
 	t.Parallel()
 

cmd/entire/cli/search_tui.go

@@ -433,8 +433,8 @@ func (m searchModel) updateSearchMode(msg tea.KeyPressMsg) (tea.Model, tea.Cmd)
 			return m, nil
 		}
 		// Checkpoint search: ParseSearchInput extracts author:/date:/branch:/repo:.
-		// ValidateRepoFilters only applies to checkpoint search (single repo limit);
-		// code search handles multiple repos via fan-out.
+		// ValidateRepoFilters only checks each repo value's shape; both semantic
+		// and code search accept multiple repos and fan out across cells.
 		parsed := search.ParseSearchInput(raw)
 		checkpointRepoErr := search.ValidateRepoFilters(parsed.Repos)
 

cmd/entire/cli/search_tui_test.go

@@ -1405,7 +1405,7 @@ func TestSearchModel_NewSearchAllReposFilter(t *testing.T) {
 	}
 }
 
-func TestSearchModel_NewSearchRejectsMultipleExplicitRepos(t *testing.T) {
+func TestSearchModel_NewSearchAcceptsMultipleExplicitRepos(t *testing.T) {
 	t.Parallel()
 
 	ss := statusStyles{colorEnabled: false, width: 100}
@@ -1425,13 +1425,20 @@ func TestSearchModel_NewSearchRejectsMultipleExplicitRepos(t *testing.T) {
 		t.Fatalf("Update returned %T, want searchModel", updated)
 	}
 
-	// Multi-repo filters are invalid for checkpoint search and code search is
-	// off (nil codeOpts) — stay in search mode so the user can correct input.
-	if m.mode != modeSearch {
-		t.Errorf("mode = %d, want modeSearch", m.mode)
+	// Multiple explicit repos are now valid: the semantic search fires (the v4
+	// path fans out across the hosting cells), so no error and we leave search
+	// mode to show loading results.
+	if m.searchErr != "" {
+		t.Errorf("searchErr = %q, want empty", m.searchErr)
+	}
+	if m.mode != modeBrowse {
+		t.Errorf("mode = %d, want modeBrowse", m.mode)
+	}
+	if !m.loading {
+		t.Error("loading = false, want true (semantic search should fire)")
 	}
-	if m.searchErr != "only one explicit repo filter is currently supported" {
-		t.Errorf("searchErr = %q", m.searchErr)
+	if got, want := m.searchCfg.Repos, []string{"entirehq/entire.io", "entireio/cli"}; len(got) != len(want) || got[0] != want[0] || got[1] != want[1] {
+		t.Errorf("searchCfg.Repos = %v, want %v", got, want)
 	}
 }