From 26d0419688086fdc8386e0b5be2e0defeeb3e24f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 14:27:38 -0700 Subject: [PATCH] Complete search filters and real metadata (Fixes #470, #546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bound the default search result set and wire the full filter surface into basecamp search. #470: --limit defaulted to 0, which the SDK treats as 'fetch every page' and follows Link-header pagination unbounded, hanging 90s+ on a broad query. Apply a default cap of 20 (--all restores unbounded; --all + --limit is a usage error). #546: sort vocabulary and the previously-ignored project/type/metadata filters. --sort normalizes to relevance (best_match) or recency. New filter flags: --project/--in scope to a project (resolved to bucket_ids[]) --type filter by content type (todo, card, ping, chat, ...) --creator filter by creator (name, email, ID, or me) --since bound to a time range (BC5-only) --file-type filter attachments (image, audio, video, pdf) --exclude-chat drop chat/campfire results Search uses a different type vocabulary than recordings (upload -> Attachment, ping -> Circle, check-in -> Question, event -> Schedule::Entry, folder -> Vault, chat -> Chat::Transcript, forward -> Inbox::Forward), so --type resolves through a dedicated table and rejects unknown values rather than passing them through: BC3 silently discards an unrecognized type and returns unfiltered results. --file-type normalizes casing to BC3's case-sensitive Blob::TYPES membership so 'image' does not silently disable the filter. Each filter is sent in both the plural (BucketIds/TypeNames/CreatorIds) and deprecated singular forms so BC5 honors the plural while older clients fall back to the singular. search metadata now presents the real recording/file search types and default labels; the summary counts only selectable (non-default) options — the key:null 'Everything'/'All files' default is excluded. Empty metadata is a graceful success, not an error. The SearchOptions/SearchMetadata types come from the SDK. Bump the pin to the current SDK tip (a98f52fd3ddd), which hardens the generated array query params (bucket_ids[]/type_names[]/creator_ids[]) to omit empty slices on the wire (basecamp-sdk #407/#410); the public SearchService wrapper the CLI uses was already safe, so this is defense in depth. No new SDK service methods, no CLI surface change. Update .surface, SKILL.md, API-COVERAGE.md, and the search-metadata smoke test. --- .surface | 5 + API-COVERAGE.md | 2 +- e2e/smoke/smoke_misc_read.bats | 8 +- go.mod | 2 +- go.sum | 4 +- internal/commands/search.go | 305 +++++++++++++-- internal/commands/search_test.go | 529 +++++++++++++++++++++++---- internal/version/sdk-provenance.json | 6 +- skills/basecamp/SKILL.md | 14 +- 9 files changed, 763 insertions(+), 112 deletions(-) diff --git a/.surface b/.surface index a02c2c4c2..2657be86e 100644 --- a/.surface +++ b/.surface @@ -11352,6 +11352,9 @@ FLAG basecamp search --agent type=bool FLAG basecamp search --all type=bool FLAG basecamp search --cache-dir type=string FLAG basecamp search --count type=bool +FLAG basecamp search --creator type=string +FLAG basecamp search --exclude-chat type=bool +FLAG basecamp search --file-type type=string FLAG basecamp search --help type=bool FLAG basecamp search --hints type=bool FLAG basecamp search --ids-only type=bool @@ -11366,10 +11369,12 @@ FLAG basecamp search --no-stats type=bool FLAG basecamp search --profile type=string FLAG basecamp search --project type=string FLAG basecamp search --quiet type=bool +FLAG basecamp search --since type=string FLAG basecamp search --sort type=string FLAG basecamp search --stats type=bool FLAG basecamp search --styled type=bool FLAG basecamp search --todolist type=string +FLAG basecamp search --type type=string FLAG basecamp search --verbose type=count FLAG basecamp search metadata --account type=string FLAG basecamp search metadata --agent type=bool diff --git a/API-COVERAGE.md b/API-COVERAGE.md index 70acb4411..2dbef97f0 100644 --- a/API-COVERAGE.md +++ b/API-COVERAGE.md @@ -51,7 +51,7 @@ The **Since** column tags each row with the Basecamp version that introduced its | people | 12 | `people`, `me` | ✅ | BC4 | - | list, show, pingable, add, remove (BC5: `tagline` alias of `bio` on person output) | | **Search & Recordings** | | my_assignments | 3 | `assignments` | ✅ | BC4 | - | list (priorities/non-priorities), completed, due (with scope filter) | -| search | 2 | `search` | ✅ | BC4 | - | Full-text search | +| search | 2 | `search` | ✅ | BC4 | - | Full-text search + metadata. Filters: `--project`/`--in`, `--type`, `--creator`, `--since` (BC5-only), `--file-type`, `--exclude-chat`. Metadata lists recording/file search types | | recordings | 4 | `recordings` | ✅ | BC4 | - | Browse by type/status, trash/archive/restore | | **Files & Documents** | | uploads | 8 | `files`, `uploads` | ✅ | BC4 | - | list, show | diff --git a/e2e/smoke/smoke_misc_read.bats b/e2e/smoke/smoke_misc_read.bats index 998f0989c..2ede67870 100644 --- a/e2e/smoke/smoke_misc_read.bats +++ b/e2e/smoke/smoke_misc_read.bats @@ -28,10 +28,8 @@ setup_file() { @test "search metadata returns metadata" { run_smoke basecamp search metadata --json - # Search metadata requires projects with search enabled - if [[ "$status" -ne 0 ]]; then - mark_unverifiable "Search metadata not available" - return 0 - fi + # Empty metadata is deliberate success now (SDK schema drift, not an error); + # only genuine transport/API failures should fail here. + assert_success assert_json_value '.ok' 'true' } diff --git a/go.mod b/go.mod index c3a63cb9a..97e44ba2c 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.1.1 charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 - github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260724184307-e2c1abea4aea + github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260725033239-a98f52fd3ddd github.com/basecamp/cli v0.2.1 github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/glamour v1.0.0 diff --git a/go.sum b/go.sum index 14c85fd96..4fa54db92 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= -github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260724184307-e2c1abea4aea h1:dRwhvhnzbyXxitGbb3uGFaUIJHazztD9UgWC0XzM870= -github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260724184307-e2c1abea4aea/go.mod h1:eX5mEKCdtxSfEL4P/n5AwOl21JVA/K+gRPic/Hd8W/Y= +github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260725033239-a98f52fd3ddd h1:LrFwDErhoECxwyHrXg/dtmuZ82nJ5Xj4WIyE+GJs8T8= +github.com/basecamp/basecamp-sdk/go v0.8.1-0.20260725033239-a98f52fd3ddd/go.mod h1:SvHmG8I8jnB/3p8+Kqxs5NLPpvWssh8KUvr39bzRsl0= github.com/basecamp/cli v0.2.1 h1:8GyehPVtsTXla0oOPu4QgXRjwwzJ99prlByvyi+0HRQ= github.com/basecamp/cli v0.2.1/go.mod h1:p8tt/DatJ2LAzWO6N6tNfV8x3gF5T3IxDTo+U8FfWPo= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= diff --git a/internal/commands/search.go b/internal/commands/search.go index 1e62f76c8..69e67d731 100644 --- a/internal/commands/search.go +++ b/internal/commands/search.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "strconv" "strings" "time" @@ -10,14 +11,27 @@ import ( "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/completion" "github.com/basecamp/basecamp-cli/internal/output" ) +// defaultSearchLimit caps results when neither --all nor an explicit --limit is +// given. The SDK treats Limit==0 as "fetch every page" and follows Link-header +// pagination unbounded, which can hang for 90s+ on a broad query (#470); a +// bounded default keeps the common case fast while --all preserves opt-in +// exhaustive fetches. +const defaultSearchLimit = 20 + // NewSearchCmd creates the search command for full-text search. func NewSearchCmd() *cobra.Command { var sortBy string var limit int var all bool + var typeName string + var since string + var creator string + var fileType string + var excludeChat bool cmd := &cobra.Command{ Use: "search ", @@ -25,13 +39,22 @@ func NewSearchCmd() *cobra.Command { Long: `Search across all Basecamp content. Uses the Basecamp search API to find content matching your query. -Use 'basecamp search metadata' to see available search scopes.`, +Results are capped at 20 by default; pass --all to fetch every match. + +Filter with --project/--in, --type, --creator, --since, --file-type, and +--exclude-chat. Use 'basecamp search metadata' to inspect the recording and +file types the API accepts.`, Example: ` basecamp search "quarterly goals" - basecamp search "bug report" --sort created_at + basecamp search "bug report" --sort recency basecamp search "design review" --limit 5 + basecamp search "bug" --project Marketing --type todo --since last_30_days + basecamp search "invoice" --file-type pdf --creator me basecamp search "meeting notes" --all`, Annotations: map[string]string{"agent_notes": "Use search for keyword queries, use recordings for browsing by type/status without a search term"}, - Args: cobra.MinimumNArgs(0), + // At most one positional: the query (0 shows help). More than one is a + // usage error rather than silently searching only the first word — an + // unquoted `search foo bar` should not quietly drop "bar". + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) @@ -47,21 +70,88 @@ Use 'basecamp search metadata' to see available search scopes.`, query := args[0] - if all && limit > 0 { + sort, err := normalizeSearchSort(sortBy) + if err != nil { + return err + } + + canonicalType, err := normalizeSearchType(typeName) + if err != nil { + return err + } + + canonicalFileType, err := normalizeSearchFileType(fileType) + if err != nil { + return err + } + + canonicalSince, err := normalizeSearchSince(since) + if err != nil { + return err + } + + limitChanged := cmd.Flags().Changed("limit") + if all && limitChanged { return output.ErrUsage("--all and --limit are mutually exclusive") } + effectiveLimit := defaultSearchLimit + switch { + case all: + effectiveLimit = 0 // unbounded: follow pagination to the end + case limitChanged: + if limit <= 0 { + return output.ErrUsage("--limit must be a positive number; use --all to fetch every result") + } + effectiveLimit = limit + } + if err := ensureAccount(cmd, app); err != nil { return err } - // Build search options - opts := &basecamp.SearchOptions{} - if sortBy != "" { - opts.Sort = sortBy + opts := &basecamp.SearchOptions{ + Sort: sort, + Limit: effectiveLimit, + FileType: canonicalFileType, + ExcludeChat: excludeChat, + Since: canonicalSince, + } + + // Only explicit --project/--in scopes the search; ambient + // config.ProjectID stays ignored because search is account-wide. + // Send both the plural (BucketIds) and deprecated singular (BucketID) + // forms: BC5 honors the plural, older clients fall back to the + // singular, and since the flag is single-valued they mirror each other. + if app.Flags.Project != "" { + resolved, _, err := app.Names.ResolveProject(cmd.Context(), app.Flags.Project) + if err != nil { + return err + } + id, err := strconv.ParseInt(resolved, 10, 64) + if err != nil { + return output.ErrNotFound("Project", app.Flags.Project) + } + opts.BucketIds = []int64{id} + opts.BucketID = id } - if !all && limit > 0 { - opts.Limit = limit + + if creator != "" { + resolved, _, err := app.Names.ResolvePerson(cmd.Context(), creator) + if err != nil { + return err + } + id, err := strconv.ParseInt(resolved, 10, 64) + if err != nil { + return output.ErrNotFound("Person", creator) + } + opts.CreatorIds = []int64{id} + opts.CreatorID = id + } + + if canonicalType != "" { + opts.TypeNames = []string{canonicalType} + opts.Type = canonicalType } searchResult, err := app.Account().Search().Search(cmd.Context(), query, opts) @@ -98,21 +188,167 @@ Use 'basecamp search metadata' to see available search scopes.`, }, } - cmd.Flags().StringVarP(&sortBy, "sort", "s", "", "Sort by: created_at or updated_at (default: relevance)") - cmd.Flags().IntVarP(&limit, "limit", "n", 0, "Maximum number of results to fetch") + cmd.Flags().StringVarP(&sortBy, "sort", "s", "", "Sort order: relevance (default) or recency") + cmd.Flags().IntVarP(&limit, "limit", "n", 0, "Maximum number of results to fetch (default 20; use --all for every result)") cmd.Flags().BoolVar(&all, "all", false, "Fetch all results (no limit)") + cmd.Flags().StringVarP(&typeName, "type", "t", "", "Filter by content type (todo, message, card, ping, chat, event, ...)") + cmd.Flags().StringVar(&since, "since", "", "Restrict to a time range: last_7_days, last_30_days, last_90_days, last_12_months, forever (BC5-only)") + cmd.Flags().StringVar(&creator, "creator", "", "Filter by creator (name, email, ID, or 'me')") + cmd.Flags().StringVar(&fileType, "file-type", "", "Filter attachments by type: image, audio, video, pdf") + cmd.Flags().BoolVar(&excludeChat, "exclude-chat", false, "Exclude chat/campfire results") + + _ = cmd.RegisterFlagCompletionFunc("type", func(_ *cobra.Command, _ []string, _ string) ([]cobra.Completion, cobra.ShellCompDirective) { + return searchTypeFriendlyNames(), cobra.ShellCompDirectiveNoFileComp + }) + _ = cmd.RegisterFlagCompletionFunc("since", func(_ *cobra.Command, _ []string, _ string) ([]cobra.Completion, cobra.ShellCompDirective) { + return searchSinceValues(), cobra.ShellCompDirectiveNoFileComp + }) + _ = cmd.RegisterFlagCompletionFunc("file-type", func(_ *cobra.Command, _ []string, _ string) ([]cobra.Completion, cobra.ShellCompDirective) { + return []cobra.Completion{"image", "audio", "video", "pdf"}, cobra.ShellCompDirectiveNoFileComp + }) + _ = cmd.RegisterFlagCompletionFunc("creator", completion.NewCompleter(nil).PeopleCompletion()) cmd.AddCommand(newSearchMetadataCmd()) return cmd } +// normalizeSearchSort maps the user-facing --sort vocabulary onto the values the +// Basecamp search API accepts. Empty/relevance normalizes to best_match (BC3's +// default, pinned explicitly for deterministic output); recency and its +// deprecated created_at/updated_at aliases normalize to recency (newest-first). +// BC3 treats any non-blank, non-best_match sort as created-at descending, so +// recency works today regardless of the search-filter release. Unknown values +// are a usage error. +func normalizeSearchSort(sort string) (string, error) { + switch strings.ToLower(strings.TrimSpace(sort)) { + case "", "relevance", "best_match": + return "best_match", nil + case "recency", "newest", "created_at", "updated_at": + return "recency", nil + default: + return "", output.ErrUsage(fmt.Sprintf("invalid --sort value %q; valid values are relevance or recency", sort)) + } +} + +// searchTypeAliases maps friendly names and canonical Keys onto the canonical +// Search::Type Keys BC3 accepts (app/models/search/type.rb). Search uses a +// different vocabulary than recordings: upload/file → Attachment (recordings +// uses Upload), ping → Circle, check-in → Question, event → Schedule::Entry, +// folder → Vault, chat → Chat::Transcript, forward → Inbox::Forward. BC3 +// silently discards an unrecognized type and returns unfiltered results, so we +// reject unknown values rather than passing them through. +var searchTypeAliases = map[string]string{ + "todo": "Todo", + "message": "Message", + "document": "Document", + "comment": "Comment", + "card": "Kanban::Card", + "kanban::card": "Kanban::Card", + "file": "Attachment", + "upload": "Attachment", + "attachment": "Attachment", + "ping": "Circle", + "circle": "Circle", + "chat": "Chat::Transcript", + "chat::transcript": "Chat::Transcript", + "check-in": "Question", + "checkin": "Question", + "question": "Question", + "event": "Schedule::Entry", + "schedule::entry": "Schedule::Entry", + "folder": "Vault", + "vault": "Vault", + "forward": "Inbox::Forward", + "inbox::forward": "Inbox::Forward", + "client": "Client::Correspondence", + "client::correspondence": "Client::Correspondence", +} + +// searchTypeFriendlyNames lists the friendly --type values for help, error +// messages, and completion, in a stable presentation order. +func searchTypeFriendlyNames() []string { + return []string{ + "todo", "message", "document", "comment", "card", "file", + "ping", "chat", "check-in", "event", "folder", "forward", "client", + } +} + +// normalizeSearchType maps a friendly alias or canonical Key onto the canonical +// Search::Type Key. Empty input leaves the filter unset. Unknown values are a +// usage error — BC3 would silently drop them and return unfiltered results. +func normalizeSearchType(input string) (string, error) { + trimmed := strings.TrimSpace(input) + if trimmed == "" { + return "", nil + } + if canonical, ok := searchTypeAliases[strings.ToLower(trimmed)]; ok { + return canonical, nil + } + return "", output.ErrUsage(fmt.Sprintf( + "invalid --type value %q; valid values are %s", + input, strings.Join(searchTypeFriendlyNames(), ", "), + )) +} + +// normalizeSearchFileType maps a case-insensitive file-type name onto BC3's +// capitalized Blob::TYPES membership (Image, Audio, Video, PDF). BC3 filters +// after a case-sensitive check, so "image" would silently disable the filter; +// we normalize casing and reject unknown values. +func normalizeSearchFileType(input string) (string, error) { + switch strings.ToLower(strings.TrimSpace(input)) { + case "": + return "", nil + case "image": + return "Image", nil + case "audio": + return "Audio", nil + case "video": + return "Video", nil + case "pdf": + return "PDF", nil + default: + return "", output.ErrUsage(fmt.Sprintf( + "invalid --file-type value %q; valid values are image, audio, video, pdf", input, + )) + } +} + +// searchSinceValues lists the accepted --since ranges for help and completion. +func searchSinceValues() []string { + return []string{"last_7_days", "last_30_days", "last_90_days", "last_12_months", "forever"} +} + +// normalizeSearchSince validates the --since range against the values BC5 +// accepts. Hyphens normalize to underscores; empty input leaves it unset. +// Unknown values are a usage error. There is no BC4 equivalent — --since is +// BC5-only. +func normalizeSearchSince(input string) (string, error) { + normalized := strings.ReplaceAll(strings.ToLower(strings.TrimSpace(input)), "-", "_") + if normalized == "" { + return "", nil + } + for _, v := range searchSinceValues() { + if normalized == v { + return normalized, nil + } + } + return "", output.ErrUsage(fmt.Sprintf( + "invalid --since value %q; valid values are %s", + input, strings.Join(searchSinceValues(), ", "), + )) +} + func newSearchMetadataCmd() *cobra.Command { return &cobra.Command{ Use: "metadata", Aliases: []string{"types"}, - Short: "Show available search filters", - Long: "Display the available recording-type and file-type filters for scoping a search.", + Short: "Show the recording and file types search accepts", + Long: `Display the search filter options returned by the Basecamp API. + +Lists the recording types (--type) and file types (--file-type) you can filter +by, along with the default labels the API reports for each filter.`, + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) return runSearchMetadata(cmd, app) @@ -200,26 +436,41 @@ func runSearchMetadata(cmd *cobra.Command, app *appctx.App) error { if err != nil { return convertSDKError(err) } - - // Handle empty response - if metadata == nil || (len(metadata.RecordingSearchTypes) == 0 && len(metadata.FileSearchTypes) == 0) { - return output.ErrUsageHint( - "Search metadata not available", - "No search filters available", - ) + if metadata == nil { + metadata = &basecamp.SearchMetadata{} } - summary := fmt.Sprintf("Search filters: %d recording types, %d file types", - len(metadata.RecordingSearchTypes), len(metadata.FileSearchTypes)) + // Count only the selectable (non-default) options. Each list carries a + // leading key:null entry — the "Everything"/"All files" default — that must + // not be counted as a filterable type. + recordingOptions := countSelectableTypes(metadata.RecordingSearchTypes) + fileOptions := countSelectableTypes(metadata.FileSearchTypes) + summary := fmt.Sprintf("Search filters: %d %s, %d %s", + recordingOptions, pluralize(recordingOptions, "recording type", "recording types"), + fileOptions, pluralize(fileOptions, "file type", "file types")) - return app.OK(metadata, + respOpts := []output.ResponseOption{ output.WithSummary(summary), output.WithBreadcrumbs( output.Breadcrumb{ Action: "search", - Cmd: "basecamp search ", - Description: "Search content", + Cmd: "basecamp search --type ", + Description: "Search content, optionally filtered by type", }, ), - ) + } + + return app.OK(metadata, respOpts...) +} + +// countSelectableTypes counts search options with a non-nil Key. A nil Key is +// the default "Everything"/"All files" entry, which is not a real filter value. +func countSelectableTypes(types []basecamp.SearchType) int { + n := 0 + for _, t := range types { + if t.Key != nil { + n++ + } + } + return n } diff --git a/internal/commands/search_test.go b/internal/commands/search_test.go index ce76ef5fe..ed0879953 100644 --- a/internal/commands/search_test.go +++ b/internal/commands/search_test.go @@ -8,6 +8,8 @@ import ( "fmt" "io" "net/http" + "net/url" + "strconv" "strings" "testing" @@ -24,24 +26,83 @@ import ( "github.com/basecamp/basecamp-cli/internal/output" ) -// searchTransport serves mock search API responses with configurable -// result count and total count (X-Total-Count header). +// searchTransport serves mock search API responses. +// +// In single-page mode (perPage == 0) it returns resultCount results with no +// pagination Link — the shape most tests need. In paginated mode (perPage > 0) +// it serves ?page= pages of perPage results drawn from a pool of `pool` total +// results, advertising a `Link: …; rel="next"` header whenever more results +// remain. Optional pointers capture the number of HTTP requests served and the +// last request's query parameters so tests can assert pagination short-circuits +// and query wiring. +// +// It also serves the name/person resolution endpoints (/projects.json, +// /people.json, /my/profile.json) so --project/--creator resolution works. +// requests/lastParams capture ONLY /search.json so those resolution fetches do +// not overwrite the captured search query. type searchTransport struct { resultCount int totalCount int + + perPage int // page size when paginating; 0 = single page of resultCount + pool int // total results available across pages (paginated mode) + + requests *int // captures number of /search.json requests served + lastParams *url.Values // captures the last /search.json request's query params } func (s searchTransport) RoundTrip(req *http.Request) (*http.Response, error) { header := make(http.Header) header.Set("Content-Type", "application/json") + // Serve resolution endpoints before the /search.json-only guard so + // --project and --creator can resolve to IDs. Single page, no Link. + if body, ok := resolutionResponse(req.URL.Path); ok { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(body)), + Header: header, + Request: req, + }, nil + } + if !strings.Contains(req.URL.Path, "/search.json") { return nil, errors.New("unexpected request: " + req.URL.Path) } - // Build N search results + query := req.URL.Query() + if s.requests != nil { + *s.requests++ + } + if s.lastParams != nil { + *s.lastParams = query + } + + // Determine which ids this page carries. + start, end := 0, s.resultCount + if s.perPage > 0 { + page := 1 + if p := query.Get("page"); p != "" { + if n, err := strconv.Atoi(p); err == nil && n > 0 { + page = n + } + } + start = (page - 1) * s.perPage + end = start + s.perPage + if end > s.pool { + end = s.pool + } + if end < s.pool { + next := *req.URL + q := next.Query() + q.Set("page", strconv.Itoa(page+1)) + next.RawQuery = q.Encode() + header.Set("Link", fmt.Sprintf("<%s>; rel=\"next\"", next.String())) + } + } + var results []map[string]any - for i := range s.resultCount { + for i := start; i < end; i++ { results = append(results, map[string]any{ "id": i + 1, "status": "active", @@ -71,6 +132,61 @@ func (s searchTransport) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } +// resolutionResponse returns a canned JSON body for the name/person resolution +// endpoints the search command hits when --project or --creator is set. The +// second return is false for any other path so callers fall through to their +// primary handler. Single page, no Link header. +func resolutionResponse(path string) ([]byte, bool) { + switch { + case strings.Contains(path, "/my/profile.json"): + return []byte(`{"id":555,"name":"Me"}`), true + case strings.Contains(path, "/projects.json"): + return []byte(`[{"id":123,"name":"Test Project"}]`), true + case strings.Contains(path, "/people.json"): + return []byte(`[{"id":987,"name":"Ann"}]`), true + default: + return nil, false + } +} + +// searchMetadataTransport serves /searches/metadata.json with the real BC3 +// response shape: recording/file search types as key/value pairs (each list led +// by a key:null default) plus the default_* filter labels. See bc3 +// app/views/api/searches/metadata/index.json.jbuilder. +type searchMetadataTransport struct{} + +func (searchMetadataTransport) RoundTrip(req *http.Request) (*http.Response, error) { + header := make(http.Header) + header.Set("Content-Type", "application/json") + + if !strings.Contains(req.URL.Path, "/searches/metadata.json") { + return nil, errors.New("unexpected request: " + req.URL.Path) + } + + body := []byte(`{ + "recording_search_types": [ + {"key": null, "value": "Everything"}, + {"key": "Todo", "value": "To-dos"} + ], + "file_search_types": [ + {"key": null, "value": "All files"}, + {"key": "Image", "value": "Images"} + ], + "default_creator_label": "Anyone", + "default_bucket_label": "All projects", + "default_circle_label": "All pings", + "default_file_type_label": "All files", + "default_type_label": "Everything" + }`) + + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(body)), + Header: header, + Request: req, + }, nil +} + func setupSearchTestApp(t *testing.T, transport http.RoundTripper) (*appctx.App, *bytes.Buffer) { t.Helper() t.Setenv("BASECAMP_NO_KEYRING", "1") @@ -109,109 +225,382 @@ func executeSearchCommand(cmd *cobra.Command, app *appctx.App, args ...string) e return cmd.Execute() } -// searchMetadataTransport serves the search metadata endpoint. An empty body -// simulates an empty (no-filters) response. -type searchMetadataTransport struct { - body string -} +func TestSearchTruncationNoticePresent(t *testing.T) { + app, buf := setupSearchTestApp(t, searchTransport{resultCount: 5, totalCount: 20}) -func (s searchMetadataTransport) RoundTrip(req *http.Request) (*http.Response, error) { - header := make(http.Header) - header.Set("Content-Type", "application/json") + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query", "--limit", "5") + require.NoError(t, err) - if !strings.Contains(req.URL.Path, "/searches/metadata") { - return nil, errors.New("unexpected request: " + req.URL.Path) - } - body := s.body - if body == "" { - body = `{"recording_search_types":[],"file_search_types":[]}` - } - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(strings.NewReader(body)), - Header: header, - Request: req, - }, nil + var envelope output.Response + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + assert.Contains(t, envelope.Notice, "Showing 5 of 20") } -const searchMetadataBody = `{ - "recording_search_types": [ - {"key": null, "value": "Everything"}, - {"key": "Message", "value": "Messages"}, - {"key": "Todo", "value": "To-dos"} - ], - "file_search_types": [ - {"key": null, "value": "All files"}, - {"key": "image", "value": "Images"} - ], - "default_creator_label": "Anyone", - "default_bucket_label": "All projects", - "default_circle_label": "All pings", - "default_file_type_label": "All files", - "default_type_label": "Everything" -}` - -// TestSearchMetadataReturnsFilterTypes verifies the metadata command reports the -// recording/file filter counts from the reshaped SearchMetadata response. -func TestSearchMetadataReturnsFilterTypes(t *testing.T) { - app, buf := setupSearchTestApp(t, searchMetadataTransport{body: searchMetadataBody}) - - cmd := NewSearchCmd() - err := executeSearchCommand(cmd, app, "metadata") +func TestSearchNoTruncationNotice(t *testing.T) { + app, buf := setupSearchTestApp(t, searchTransport{resultCount: 5, totalCount: 5}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query") require.NoError(t, err) var envelope output.Response require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) - assert.Contains(t, envelope.Summary, "3 recording types") - assert.Contains(t, envelope.Summary, "2 file types") + assert.Empty(t, envelope.Notice) } -// TestSearchMetadataEmptyIsUsageError verifies an empty metadata response -// surfaces a usage hint rather than an empty success envelope. -func TestSearchMetadataEmptyIsUsageError(t *testing.T) { - app, _ := setupSearchTestApp(t, searchMetadataTransport{}) +func TestSearchAllAndLimitMutuallyExclusive(t *testing.T) { + app, _ := setupSearchTestApp(t, todosNoNetworkTransport{}) cmd := NewSearchCmd() - err := executeSearchCommand(cmd, app, "metadata") + err := executeSearchCommand(cmd, app, "query", "--all", "--limit", "5") require.Error(t, err) var e *output.Error require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) - assert.Equal(t, "Search metadata not available", e.Message) + assert.Contains(t, e.Message, "--all and --limit are mutually exclusive") } -func TestSearchTruncationNoticePresent(t *testing.T) { - app, buf := setupSearchTestApp(t, searchTransport{resultCount: 5, totalCount: 20}) +// TestSearchBoundedDefault is the regression for #470: a bare search must apply +// the default cap and short-circuit pagination in a single request, even when +// the first page already advertises a next Link. +func TestSearchBoundedDefault(t *testing.T) { + var requests int + // Page 1 carries 25 results and a next Link; pool of 50 guarantees the Link + // is present so we prove the default cap stops us, not an exhausted pool. + app, buf := setupSearchTestApp(t, searchTransport{ + perPage: 25, + pool: 50, + totalCount: 50, + requests: &requests, + }) cmd := NewSearchCmd() - err := executeSearchCommand(cmd, app, "query", "--limit", "5") - require.NoError(t, err) + require.NoError(t, executeSearchCommand(cmd, app, "query")) var envelope output.Response require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) - assert.Contains(t, envelope.Notice, "Showing 5 of 20") + + results, ok := envelope.Data.([]any) + require.True(t, ok, "expected results array, got %T", envelope.Data) + assert.Len(t, results, defaultSearchLimit) + assert.Equal(t, 1, requests, "default cap must short-circuit pagination in one request") } -func TestSearchNoTruncationNotice(t *testing.T) { - app, buf := setupSearchTestApp(t, searchTransport{resultCount: 5, totalCount: 5}) +// TestSearchAllTraversesPages proves --all bypasses the default cap and follows +// pagination to completion. +func TestSearchAllTraversesPages(t *testing.T) { + var requests int + app, buf := setupSearchTestApp(t, searchTransport{ + perPage: 20, + pool: 25, + totalCount: 25, + requests: &requests, + }) cmd := NewSearchCmd() - err := executeSearchCommand(cmd, app, "query") - require.NoError(t, err) + require.NoError(t, executeSearchCommand(cmd, app, "query", "--all")) var envelope output.Response require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) - assert.Empty(t, envelope.Notice) + + results, ok := envelope.Data.([]any) + require.True(t, ok, "expected results array, got %T", envelope.Data) + assert.Len(t, results, 25) + assert.Equal(t, 2, requests, "--all must traverse every page") } -func TestSearchAllAndLimitMutuallyExclusive(t *testing.T) { +func TestSearchLimitMustBePositive(t *testing.T) { + for _, value := range []string{"0", "-1"} { + app, _ := setupSearchTestApp(t, todosNoNetworkTransport{}) + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query", "--limit", value) + require.Error(t, err, "--limit %s should be rejected", value) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Contains(t, e.Message, "must be a positive number") + } +} + +// TestSearchDefaultQueryAndSort proves a bare search sends q= and the +// pinned best_match default sort. +func TestSearchDefaultQueryAndSort(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{ + resultCount: 3, + totalCount: 3, + lastParams: ¶ms, + }) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "meeting notes")) + + assert.Equal(t, "meeting notes", params.Get("q")) + assert.Equal(t, "best_match", params.Get("sort")) +} + +func TestSearchSortMappings(t *testing.T) { + for input, want := range map[string]string{ + "relevance": "best_match", + "best_match": "best_match", + "recency": "recency", + "newest": "recency", + "created_at": "recency", + "updated_at": "recency", + } { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{ + resultCount: 1, + totalCount: 1, + lastParams: ¶ms, + }) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--sort", input), "sort %q", input) + assert.Equal(t, want, params.Get("sort"), "sort %q", input) + } +} + +func TestSearchInvalidSortRejected(t *testing.T) { app, _ := setupSearchTestApp(t, todosNoNetworkTransport{}) cmd := NewSearchCmd() - err := executeSearchCommand(cmd, app, "query", "--all", "--limit", "5") + err := executeSearchCommand(cmd, app, "query", "--sort", "bogus") require.Error(t, err) var e *output.Error require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) - assert.Contains(t, e.Message, "--all and --limit are mutually exclusive") + assert.Contains(t, e.Message, "invalid --sort value") +} + +// TestSearchRejectsExtraArgs proves an unquoted multi-word query is a usage +// error rather than silently searching only the first word. +func TestSearchRejectsExtraArgs(t *testing.T) { + app, _ := setupSearchTestApp(t, todosNoNetworkTransport{}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "foo", "bar") + require.Error(t, err) +} + +// TestSearchMetadataRejectsExtraArgs proves the metadata subcommand rejects +// stray positional args (cobra.NoArgs) before RunE, rather than ignoring them +// and reaching account validation. Asserting the cobra arg-rejection message +// (not just any error) rules out a downstream network/account failure. +func TestSearchMetadataRejectsExtraArgs(t *testing.T) { + app, _ := setupSearchTestApp(t, todosNoNetworkTransport{}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "metadata", "junk") + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown command "junk"`) +} + +// TestSearchProjectByID proves an explicit --project (surfaced via +// app.Flags.Project, since the harness can't parse root globals) resolves to a +// bucket ID sent in both the plural bucket_ids[] and singular bucket_id forms. +func TestSearchProjectByID(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + app.Flags.Project = "123" + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query")) + + assert.Equal(t, "123", params.Get("bucket_ids[]")) + assert.Equal(t, "123", params.Get("bucket_id")) +} + +func TestSearchProjectByName(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + app.Flags.Project = "Test Project" + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query")) + + assert.Equal(t, "123", params.Get("bucket_ids[]")) + assert.Equal(t, "123", params.Get("bucket_id")) +} + +// TestSearchProjectNotFound proves an unresolvable project NAME errors. Unknown +// numeric IDs intentionally pass through (resolver.go:99), so a bad name is the +// case that surfaces the error. +func TestSearchProjectNotFound(t *testing.T) { + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1}) + app.Flags.Project = "No Such Project" + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) +} + +// TestSearchAmbientProjectIgnored proves an ambient config project is never +// used to scope the search — only an explicit --project flag scopes. +func TestSearchAmbientProjectIgnored(t *testing.T) { + var params url.Values + app, buf := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + app.Config.ProjectID = "456" // ambient, not an explicit flag + require.Empty(t, app.Flags.Project) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query")) + + assert.Empty(t, params.Get("bucket_ids[]")) + assert.Empty(t, params.Get("bucket_id")) + + var envelope output.Response + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + assert.True(t, envelope.OK) +} + +// TestSearchTypeMappings proves each friendly --type alias maps onto its +// canonical Search::Type Key, sent in both type_names[] and singular type. +func TestSearchTypeMappings(t *testing.T) { + for input, want := range map[string]string{ + "todo": "Todo", + "upload": "Attachment", + "ping": "Circle", + "check-in": "Question", + "event": "Schedule::Entry", + "folder": "Vault", + "chat": "Chat::Transcript", + "card": "Kanban::Card", + } { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--type", input), "type %q", input) + assert.Equal(t, want, params.Get("type_names[]"), "type %q", input) + assert.Equal(t, want, params.Get("type"), "type %q", input) + } +} + +// TestSearchInvalidTypeRejected proves an unknown --type errors before any +// /search.json request — BC3 would silently drop it and return unfiltered. +func TestSearchInvalidTypeRejected(t *testing.T) { + var requests int + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, requests: &requests}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query", "--type", "foo") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Contains(t, e.Message, "invalid --type value") + assert.Equal(t, 0, requests, "invalid --type must not reach the search API") +} + +func TestSearchCreatorByID(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--creator", "987")) + + assert.Equal(t, "987", params.Get("creator_ids[]")) + assert.Equal(t, "987", params.Get("creator_id")) +} + +// TestSearchCreatorMe proves --creator me resolves via /my/profile.json. +func TestSearchCreatorMe(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--creator", "me")) + + assert.Equal(t, "555", params.Get("creator_ids[]")) + assert.Equal(t, "555", params.Get("creator_id")) +} + +// TestSearchFileTypeCasing proves --file-type image is capitalized to the +// case-sensitive Blob::TYPES value BC3 requires. +func TestSearchFileTypeCasing(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--file-type", "image")) + + assert.Equal(t, "Image", params.Get("file_type")) +} + +func TestSearchInvalidFileTypeRejected(t *testing.T) { + var requests int + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, requests: &requests}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query", "--file-type", "img") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Contains(t, e.Message, "invalid --file-type value") + assert.Equal(t, 0, requests, "invalid --file-type must not reach the search API") +} + +func TestSearchExcludeChat(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--exclude-chat")) + + assert.Equal(t, "true", params.Get("exclude_chat")) +} + +func TestSearchSince(t *testing.T) { + var params url.Values + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, lastParams: ¶ms}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "query", "--since", "last_30_days")) + + assert.Equal(t, "last_30_days", params.Get("since")) +} + +func TestSearchInvalidSinceRejected(t *testing.T) { + var requests int + app, _ := setupSearchTestApp(t, searchTransport{resultCount: 1, totalCount: 1, requests: &requests}) + + cmd := NewSearchCmd() + err := executeSearchCommand(cmd, app, "query", "--since", "yesterday") + require.Error(t, err) + + var e *output.Error + require.True(t, errors.As(err, &e), "expected *output.Error, got %T: %v", err, err) + assert.Contains(t, e.Message, "invalid --since value") + assert.Equal(t, 0, requests, "invalid --since must not reach the search API") +} + +// TestSearchMetadataRealFields proves metadata presents the real recording/file +// search types with no drift notice, and the summary counts exclude the +// key:null defaults. +func TestSearchMetadataRealFields(t *testing.T) { + app, buf := setupSearchTestApp(t, searchMetadataTransport{}) + + cmd := NewSearchCmd() + require.NoError(t, executeSearchCommand(cmd, app, "metadata")) + + var envelope output.Response + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope)) + assert.True(t, envelope.OK) + assert.Empty(t, envelope.Notice) + + // One selectable recording type (Todo) and one file type (Image); the + // key:null defaults must not be counted, and the count-1 labels are + // singular. + assert.Equal(t, "Search filters: 1 recording type, 1 file type", envelope.Summary) + + data, ok := envelope.Data.(map[string]any) + require.True(t, ok, "expected metadata object, got %T", envelope.Data) + assert.Contains(t, data, "recording_search_types") + assert.Contains(t, data, "file_search_types") } diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 01255b7e0..94fbd9e7c 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.8.1-0.20260724184307-e2c1abea4aea", - "revision": "e2c1abea4aea", - "updated_at": "2026-07-24T18:43:07Z" + "version": "v0.8.1-0.20260725033239-a98f52fd3ddd", + "revision": "a98f52fd3ddd", + "updated_at": "2026-07-25T03:32:39Z" }, "api": { "repo": "basecamp/bc3", diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 913284ae1..b3a0f42ef 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -914,9 +914,17 @@ basecamp people remove --project # Remove from project ### Search ```bash -basecamp search "query" --json # Full-text search -basecamp search "query" --sort updated_at --limit 20 -basecamp search metadata --json # Available search scopes +basecamp search "query" --json # Full-text search (capped at 20; --all for every match) +basecamp search "query" --sort recency --limit 20 +basecamp search "query" --project Marketing # Scope to one project (--in also works) +basecamp search "query" --type todo # Filter by type: todo, message, document, comment, + # card, file, ping, chat, check-in, event, folder, + # forward, client +basecamp search "query" --creator me # Filter by creator (name, email, ID, or 'me') +basecamp search "query" --since last_30_days # last_7_days|last_30_days|last_90_days|last_12_months|forever +basecamp search "query" --file-type pdf # Filter attachments: image, audio, video, pdf +basecamp search "query" --exclude-chat # Drop chat/campfire results +basecamp search metadata --json # Recording and file types the API accepts as filters ``` ### Generic Show