Merge pull request #1800 from entireio/ent-1055-cli-v4-path

feat(search)!: route entire search to the v4 query-serve path (cross-cell fan-out, v3 removed)

Author: alishakawaguchi

cmd/entire/cli/api/base_url.go

@@ -130,7 +130,7 @@ func NormalizeOriginURL(raw string) string {
 }
 
 // OriginOnly is a backwards-compatible alias for NormalizeOriginURL.
-// Callers reading raw URLs (e.g. ENTIRE_SEARCH_URL) and feeding them into
+// Callers reading raw URLs (e.g. ENTIRE_API_BASE_URL) and feeding them into
 // tokenmanager.TokenRequest.Resource use this to strip path/query/fragment
 // before the lib's stricter origin-only validator runs.
 func OriginOnly(raw string) string {

cmd/entire/cli/search/scope_test.go

@@ -0,0 +1,69 @@
+package search
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// TestConfig_ScopeSlugs verifies the shared scope predicate both backends
+// derive their repo scoping from. The precedence rule that matters most: an
+// explicit repo filter always wins over --all-repos (the more specific filter
+// scopes the search) — v3 and v4 must agree on this.
+func TestConfig_ScopeSlugs(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name         string
+		cfg          Config
+		wantSlugs    []string
+		wantAllRepos bool
+	}{
+		{"all-repos flag", Config{AllRepos: true}, nil, true},
+		{"repo:* filter", Config{Repos: []string{AllReposFilter}}, nil, true},
+		{"explicit repo", Config{Repos: []string{"o/r"}}, []string{"o/r"}, false},
+		{"current-repo default", Config{Owner: "o", Repo: "r"}, []string{"o/r"}, false},
+		{"explicit filter wins over --all-repos", Config{AllRepos: true, Repos: []string{"o/r"}}, []string{"o/r"}, false},
+		{"explicit filter wins over repo:*", Config{Repos: []string{"o/r", AllReposFilter}}, []string{"o/r"}, false},
+		{"explicit filters win over current repo", Config{Repos: []string{"a/b", "c/d"}, Owner: "o", Repo: "r"}, []string{"a/b", "c/d"}, false},
+		{"no scope", Config{}, nil, false},
+		{"owner without repo is no scope", Config{Owner: "o"}, nil, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			slugs, allRepos := tt.cfg.ScopeSlugs()
+			if strings.Join(slugs, ",") != strings.Join(tt.wantSlugs, ",") || allRepos != tt.wantAllRepos {
+				t.Errorf("ScopeSlugs() = (%v, %v), want (%v, %v)", slugs, allRepos, tt.wantSlugs, tt.wantAllRepos)
+			}
+		})
+	}
+}
+
+// TestResultID_RawDataFallback verifies repo/pr rows — which have no typed
+// struct — expose the raw payload's id, so cross-cell dedup can identify the
+// same logical result from two cells.
+func TestResultID_RawDataFallback(t *testing.T) {
+	t.Parallel()
+
+	var repoRow Result
+	if err := json.Unmarshal([]byte(`{"type":"repo","data":{"id":"01JREPO","name":"x"},"searchMeta":{"score":1}}`), &repoRow); err != nil {
+		t.Fatal(err)
+	}
+	if got := repoRow.ResultID(); got != "01JREPO" {
+		t.Errorf("repo ResultID() = %q, want the rawData id \"01JREPO\"", got)
+	}
+
+	var noID Result
+	if err := json.Unmarshal([]byte(`{"type":"pr","data":{"title":"no id"},"searchMeta":{"score":1}}`), &noID); err != nil {
+		t.Fatal(err)
+	}
+	if got := noID.ResultID(); got != "" {
+		t.Errorf("pr-without-id ResultID() = %q, want \"\"", got)
+	}
+
+	// Typed results are unaffected.
+	ck := Result{Type: TypeCheckpoint, Checkpoint: &CheckpointResult{ID: "ck1"}}
+	if got := ck.ResultID(); got != "ck1" {
+		t.Errorf("checkpoint ResultID() = %q, want \"ck1\"", got)
+	}
+}

cmd/entire/cli/search/search.go

@@ -12,13 +12,23 @@ import (
 	"strings"
 	"time"
 
-	"github.com/entireio/cli/cmd/entire/cli/versioninfo"
+	"github.com/entireio/cli/cmd/entire/cli/api"
 )
 
 const apiTimeout = 30 * time.Second
 
-// DefaultServiceURL is the production search service URL.
-const DefaultServiceURL = "https://entire.io"
+// v4ServicePath is the per-repo v4 query-serve route exposed by the entire-api
+// cell gateway. It takes repo=<ULID>. The BFF (entire.io /api/v1/search)
+// forwards to this same path; the CLI dials the cell directly with a
+// jurisdictional identity token, skipping the BFF hop.
+const v4ServicePath = "/api/v1/semantic-search/search/v1/search"
+
+// ErrCellUnavailable reports that a cell's gateway does not expose the
+// semantic-search route at all (HTTP 404 at the route level) — query-serve is
+// not deployed in that cell yet. Callers fanning out across cells match it
+// with errors.Is and skip the cell quietly instead of warning the user about
+// a "failed" region.
+var ErrCellUnavailable = errors.New("semantic search is not available in this cell")
 
 // WildcardQuery is the query string used when only filters are provided (no search terms).
 const WildcardQuery = "*"
@@ -31,11 +41,13 @@ const (
 	TypeCheckpoint = "checkpoint"
 	TypeCommit     = "commit"
 	TypeSession    = "session"
+	// TypeRepo and TypePR are returned by the backend but have no typed struct
+	// (decoded via rawData). They're named so the cross-cell v4 merge can bucket
+	// and tally them without string literals.
+	TypeRepo = "repo"
+	TypePR   = "pr"
 )
 
-// MaxLimit is the maximum number of results the search API will return per request.
-const MaxLimit = 200
-
 // DefaultLimit is the default number of results to fetch per request, matching the UI.
 const DefaultLimit = 100
 
@@ -263,12 +275,26 @@ func (r *Result) ResultAuthor() string {
 		})
 }
 
-// ResultID returns the primary ID for any result type.
+// ResultID returns the primary ID for any result type. Types without a typed
+// struct (repo, pr) fall back to the "id" field of the raw payload, so a
+// cross-cell merge can still identify the same logical result returned by two
+// cells (e.g. a repo mirrored in both).
 func (r *Result) ResultID() string {
-	return resultField(r,
+	if id := resultField(r,
 		func(c *CheckpointResult) string { return c.ID },
 		func(c *CommitResult) string { return c.CommitSHA },
-		func(s *SessionResult) string { return s.SessionID })
+		func(s *SessionResult) string { return s.SessionID }); id != "" {
+		return id
+	}
+	if len(r.rawData) > 0 {
+		var d struct {
+			ID string `json:"id"`
+		}
+		if err := json.Unmarshal(r.rawData, &d); err == nil {
+			return d.ID
+		}
+	}
+	return ""
 }
 
 // ResultTitle returns the primary display text for any result type.
@@ -323,22 +349,48 @@ type Response struct {
 	Timing   *Timing     `json:"timing,omitempty"`
 	Reranked *bool       `json:"reranked,omitempty"`
 	Counts   *TypeCounts `json:"counts,omitempty"`
+
+	// Warnings are client-side completeness notes (e.g. a truncated repo
+	// index or a failed region in a cross-cell fan-out) surfaced to the user
+	// on stderr. Never part of the wire format.
+	Warnings []string `json:"-"`
 }
 
 // Config holds the configuration for a search request.
 type Config struct {
-	ServiceURL  string // Base URL of the search service
-	GitHubToken string
-	Owner       string
-	Repo        string
-	Repos       []string
-	AllRepos    bool // When true, search all accessible repos (no repo scoping)
-	Query       string
-	Limit       int
-	Author      string // Filter by author name
-	Date        string // Filter by time period: "week" or "month"
-	Branch      string // Filter by branch name
-	Page        int    // 1-based page number (0 means omit, API defaults to 1)
+	Owner    string
+	Repo     string
+	Repos    []string
+	AllRepos bool // When true, search all accessible repos (no repo scoping)
+	Query    string
+	Limit    int
+	Author   string // Filter by author name
+	Date     string // Filter by time period: "week" or "month"
+	Branch   string // Filter by branch name
+	Page     int    // 1-based page number (0 means omit, API defaults to 1)
+}
+
+// ScopeSlugs resolves the repo scope of a search: the explicit repo filters
+// (an explicit owner/name filter always scopes the search, even when
+// --all-repos is also set — the more specific filter wins), else allRepos for
+// an unfiltered repo:* / --all-repos search, else the current-repo default.
+// slugs empty with allRepos false means no scope could be determined.
+func (c Config) ScopeSlugs() (slugs []string, allRepos bool) {
+	for _, repo := range c.Repos {
+		if repo != AllReposFilter {
+			slugs = append(slugs, repo)
+		}
+	}
+	if len(slugs) > 0 {
+		return slugs, false
+	}
+	if c.AllRepos || (len(c.Repos) == 1 && c.Repos[0] == AllReposFilter) {
+		return nil, true
+	}
+	if c.Owner != "" && c.Repo != "" {
+		return []string{c.Owner + "/" + c.Repo}, false
+	}
+	return nil, false
 }
 
 // HasFilters reports whether any filter fields are set on the config.
@@ -483,52 +535,50 @@ func appendUnique(existing []string, values ...string) []string {
 	return existing
 }
 
-var httpClient = &http.Client{}
-
-// Search calls the search service to perform a hybrid search.
-func Search(ctx context.Context, cfg Config) (*Response, error) {
+// CellV4 performs a v4 query-serve search against a single entire-api
+// cell, via the pre-authenticated client (bearer = jurisdictional identity
+// token; host = the cell). repoIDs are repo ULIDs to scope to (the v4 route is
+// per-repo and keys on ULIDs, not owner/name slugs); an empty repoIDs means
+// "every repo the caller can access in this cell" — query-serve fans out across
+// those namespaces itself. The cross-cell fan-out and merge live in the cli
+// layer (mirroring code search), so this is the single-cell primitive it calls.
+func CellV4(ctx context.Context, client *api.Client, cfg Config, repoIDs []string) (*Response, error) {
 	ctx, cancel := context.WithTimeout(ctx, apiTimeout)
 	defer cancel()
 
-	serviceURL := cfg.ServiceURL
-	if serviceURL == "" {
-		serviceURL = DefaultServiceURL
+	q := url.Values{}
+	q.Set("q", cfg.Query)
+	for _, id := range repoIDs {
+		if id != "" {
+			q.Add("repo", id)
+		}
 	}
+	addCommonSearchParams(q, cfg)
 
-	u, err := url.Parse(serviceURL)
+	resp, err := client.Get(ctx, v4ServicePath+"?"+q.Encode())
 	if err != nil {
-		return nil, fmt.Errorf("parsing service URL: %w", err)
+		return nil, fmt.Errorf("calling search service: %w", err)
 	}
-	u.Path = "/search/v1/search"
+	defer resp.Body.Close()
 
-	q := u.Query()
-	q.Set("q", cfg.Query)
-	if err := ValidateRepoFilters(cfg.Repos); err != nil {
-		return nil, err
-	}
-	allRepos := cfg.AllRepos || (len(cfg.Repos) == 1 && cfg.Repos[0] == AllReposFilter)
-	hasExplicitRepo := false
-	for _, repo := range cfg.Repos {
-		if repo != AllReposFilter {
-			hasExplicitRepo = true
-			break
-		}
+	body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
+	if err != nil {
+		return nil, fmt.Errorf("reading response: %w", err)
 	}
-	switch {
-	case hasExplicitRepo:
-		// An explicit owner/name filter always scopes the search, even when
-		// --all-repos is also set (the more specific filter wins).
-		for _, repo := range cfg.Repos {
-			if repo != AllReposFilter {
-				q.Add("repo", repo)
-			}
-		}
-	case allRepos:
-		// No repo scoping — search every accessible repo.
-	case cfg.Owner != "" && cfg.Repo != "":
-		q.Set("repo", cfg.Owner+"/"+cfg.Repo)
+	if resp.StatusCode == http.StatusNotFound {
+		// The gateway has no semantic-search route (plain "404 page not
+		// found") — query-serve isn't deployed in this cell. Deployed cells
+		// answer unknown repos with an empty 200, so a route-level 404 is
+		// distinctive.
+		return nil, ErrCellUnavailable
 	}
-	// Don't set types — let the API return all types (checkpoints, commits, sessions, etc.)
+	return parseSearchResponse(resp.StatusCode, body)
+}
+
+// addCommonSearchParams sets the query params other than the repo scoping
+// (repo IDs are added by CellV4's caller). types is deliberately never sent —
+// the backend returns all types.
+func addCommonSearchParams(q url.Values, cfg Config) {
 	if cfg.Limit > 0 {
 		q.Set("limit", strconv.Itoa(cfg.Limit))
 	}
@@ -544,34 +594,20 @@ func Search(ctx context.Context, cfg Config) (*Response, error) {
 	if cfg.Page > 0 {
 		q.Set("page", strconv.Itoa(cfg.Page))
 	}
-	u.RawQuery = q.Encode()
-
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
-	if err != nil {
-		return nil, fmt.Errorf("creating request: %w", err)
-	}
-	req.Header.Set("Authorization", "Bearer "+cfg.GitHubToken)
-	req.Header.Set("User-Agent", versioninfo.UserAgent())
-
-	resp, err := httpClient.Do(req)
-	if err != nil {
-		return nil, fmt.Errorf("calling search service: %w", err)
-	}
-	defer resp.Body.Close()
-
-	body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
-	if err != nil {
-		return nil, fmt.Errorf("reading response: %w", err)
-	}
+}
 
-	if resp.StatusCode != http.StatusOK {
+// parseSearchResponse decodes a search response body, preserving the
+// long-standing error wording so callers (and error-message assertions) are
+// unchanged.
+func parseSearchResponse(statusCode int, body []byte) (*Response, error) {
+	if statusCode != http.StatusOK {
 		var errResp struct {
 			Error string `json:"error"`
 		}
 		if json.Unmarshal(body, &errResp) == nil && errResp.Error != "" {
-			return nil, fmt.Errorf("search service error (%d): %s", resp.StatusCode, errResp.Error)
+			return nil, fmt.Errorf("search service error (%d): %s", statusCode, errResp.Error)
 		}
-		return nil, fmt.Errorf("search service returned %d: %s", resp.StatusCode, string(body))
+		return nil, fmt.Errorf("search service returned %d: %s", statusCode, string(body))
 	}
 
 	var result Response