Skip to content

Commit 11bc97f

Browse files
fix(search): accept advertised repo filter formats in validation (ENT-1047)
ValidateRepoFilters rejected repo filter shapes the --repo help advertises (gh/owner/repo, et/proj/repo, raw ULID) and that both the semantic v4 lookup and the code-search resolver handle downstream, so `entire search --repo gh/owner/repo` (or a ULID) failed early with an "invalid repo filter" error even though it would resolve. Broaden isValidRepoFilter to accept owner/name slugs, prefixed paths, and raw ULIDs while still rejecting junk like a bare filename; unresolvable-but-well-formed filters now surface the meaningful downstream "no matching repositories" message instead. Addresses the Copilot review finding on PR #1845. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012QKSvbnbUJsnF1pvqqWb5u Entire-Checkpoint: 01KY8GGERT9PYZ1K5YF9D1RX5V
1 parent c9680e4 commit 11bc97f

2 files changed

Lines changed: 68 additions & 4 deletions

File tree

cmd/entire/cli/search/search.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"strings"
1313
"time"
1414

15+
ulid "github.com/oklog/ulid/v2"
16+
1517
"github.com/entireio/cli/cmd/entire/cli/api"
1618
)
1719

@@ -497,23 +499,45 @@ func ValidateRepoFilters(repos []string) error {
497499
for _, repo := range repos {
498500
if !isValidRepoFilter(repo) {
499501
return fmt.Errorf(
500-
"invalid repo filter %q: expected owner/name or *; if you meant all repos, quote the asterisk: --repo '*'",
502+
"invalid repo filter %q: expected owner/name, gh/owner/repo, a repo ULID, or *; if you meant all repos, quote the asterisk: --repo '*'",
501503
repo,
502504
)
503505
}
504506
}
505507
return nil
506508
}
507509

510+
// isValidRepoFilter reports whether repo is a filter shape the search backends
511+
// can resolve. It accepts every form the CLI help advertises and that the
512+
// resolvers handle downstream — a bare owner/name slug, a prefixed path
513+
// (gh/owner/repo, et/proj/repo, git/owner/repo), a raw repo ULID, or the
514+
// all-repos wildcard — so validation never rejects a filter the semantic v4
515+
// lookup (lookupFilter) or code-search resolver (resolveRepoFilters) would
516+
// otherwise resolve. It still rejects obvious mistakes like a bare filename.
508517
func isValidRepoFilter(repo string) bool {
509518
if repo == AllReposFilter {
510519
return true
511520
}
512-
if strings.Contains(repo, " ") {
521+
if repo == "" || strings.Contains(repo, " ") {
513522
return false
514523
}
524+
// Raw repo ULID: the v4 route keys on ULIDs and lookupFilter matches a
525+
// prefix-less token against repo IDs.
526+
if _, err := ulid.Parse(repo); err == nil {
527+
return true
528+
}
529+
// A slug or prefixed path: owner/name or <prefix>/owner/repo. Every
530+
// path segment must be non-empty.
515531
parts := strings.Split(repo, "/")
516-
return len(parts) == 2 && parts[0] != "" && parts[1] != ""
532+
if len(parts) < 2 || len(parts) > 3 {
533+
return false
534+
}
535+
for _, part := range parts {
536+
if part == "" {
537+
return false
538+
}
539+
}
540+
return true
517541
}
518542

519543
// AppendUnique appends values to existing, skipping any already present, and

cmd/entire/cli/search/search_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,12 +599,52 @@ func TestValidateRepoFilters_RejectsInvalidRepoValue(t *testing.T) {
599599
if err == nil {
600600
t.Fatal("expected validation error")
601601
}
602-
want := "invalid repo filter \"AGENTS.md\": expected owner/name or *; if you meant all repos, quote the asterisk: --repo '*'"
602+
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 '*'"
603603
if got := err.Error(); got != want {
604604
t.Errorf("error = %q, want %q", got, want)
605605
}
606606
}
607607

608+
// The CLI --repo help advertises gh/owner/repo, et/proj/repo, and raw ULIDs,
609+
// and the semantic v4 lookup + code-search resolver both handle them. Validation
610+
// must accept the same set so it never rejects a filter that would resolve
611+
// downstream (ENT-1047 review finding).
612+
func TestValidateRepoFilters_AcceptsAdvertisedFormats(t *testing.T) {
613+
t.Parallel()
614+
615+
valid := []string{
616+
"entireio/cli", // bare owner/name slug
617+
"gh/entireio/cli", // GitHub prefixed path
618+
"et/proj/repo", // Entire project prefixed path
619+
"git/owner/repo", // generic git prefixed path
620+
"01ARZ3NDEKTSV4RRFFQ69G5FAV", // raw repo ULID (canonical)
621+
"*", // all-repos wildcard
622+
}
623+
for _, repo := range valid {
624+
if err := ValidateRepoFilters([]string{repo}); err != nil {
625+
t.Errorf("ValidateRepoFilters(%q) = %v, want nil", repo, err)
626+
}
627+
}
628+
}
629+
630+
func TestValidateRepoFilters_RejectsMalformed(t *testing.T) {
631+
t.Parallel()
632+
633+
invalid := []string{
634+
"AGENTS.md", // bare filename, not a ULID or slug
635+
"owner/", // empty name segment
636+
"/repo", // empty owner segment
637+
"a/b/c/d", // too many path segments
638+
"owner name", // contains a space
639+
"gh//repo", // empty middle segment in a prefixed path
640+
}
641+
for _, repo := range invalid {
642+
if err := ValidateRepoFilters([]string{repo}); err == nil {
643+
t.Errorf("ValidateRepoFilters(%q) = nil, want validation error", repo)
644+
}
645+
}
646+
}
647+
608648
func TestParseSearchInput_QuotedAuthor(t *testing.T) {
609649
t.Parallel()
610650
p := ParseSearchInput(`author:"` + testAuthor + ` smith" fix bug`)

0 commit comments

Comments
 (0)