From ac05574a5f8fd315f5907cd58a6c9db38e4b5a12 Mon Sep 17 00:00:00 2001 From: Tim Walsh Date: Fri, 28 Aug 2026 19:28:17 -0700 Subject: [PATCH 1/2] perf(search): adopt Get Filtered Team Tasks instead of crawling every list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `task search` and `export tasks` walked workspace → spaces → folders → lists and called GetTasks per list. That cost one request per list under a 100 req/min limit, and since only page 0 was ever fetched, any list past 100 tasks was silently cut off. export also discarded GetFolders/GetLists errors, so it could omit whole subtrees and still report success. Both now issue one server-side query per workspace against GET /team/{id}/task, paged to exhaustion. Against a small real workspace this takes search from 3.7s to 1.1s; the gap widens with list count. Pagination is bounded by DefaultMaxTaskPages and reports Truncated when it stops early, so a capped result is never presented as a complete one — the failure mode this issue is about. The paging loop is extracted as `paginate` so termination and truncation are testable without a network round trip. Search and export defaults deliberately mirror the old crawl (open tasks, no subtasks) to keep this a performance fix; the endpoint makes the fuller set reachable via new --include-closed and --subtasks flags. Verified that default results match v0.2.1 exactly on a real workspace. Adds Client.FindTasksByCustomField and `cu field find`, the reverse lookup from a value stored in a custom field back to the task holding it, which the crawl could not do at any acceptable cost. Fixes #25 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014aqbmccWm1tqttmBUCR5rv ClickUp: 86dxbeqyt --- README.md | 5 +- docs/site/commands/cu_export_tasks.md | 2 + docs/site/commands/cu_field.md | 1 + docs/site/commands/cu_field_find.md | 43 +++++++ docs/site/commands/cu_task_search.md | 2 + internal/api/teamtasks.go | 160 ++++++++++++++++++++++++++ internal/api/teamtasks_test.go | 139 ++++++++++++++++++++++ internal/cmd/config.go | 16 +++ internal/cmd/config_test.go | 64 +++++++++++ internal/cmd/export.go | 69 +++++++---- internal/cmd/field.go | 92 +++++++++++++++ internal/cmd/task.go | 99 ++++++++-------- internal/config/config.go | 41 +++++-- internal/config/config_test.go | 40 +++++++ mkdocs.yml | 1 + 15 files changed, 688 insertions(+), 86 deletions(-) create mode 100644 docs/site/commands/cu_field_find.md create mode 100644 internal/api/teamtasks.go create mode 100644 internal/api/teamtasks_test.go diff --git a/README.md b/README.md index c027f97..9e4d4c4 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ A GitHub CLI-inspired command-line interface for ClickUp. - **GitHub CLI-like Interface**: Familiar command structure for developers who use `gh` - **Task Management**: Create, view, update, and manage tasks from the command line - **Comment Management**: Add, list, and delete comments on tasks with user assignment -- **Custom Fields**: Read and write ClickUp custom fields, resolving names to option ids +- **Custom Fields**: Read and write ClickUp custom fields, resolving names to option ids, and find tasks by field value - **Cache Management**: Optimize performance with intelligent caching and cache control commands - **Project Configuration**: Set project-specific defaults with `.cu.yml` configuration files - **API Passthrough**: Direct access to ClickUp API endpoints for advanced operations @@ -109,6 +109,9 @@ cu field set Machine "Chilastra,Sunrunner" # Clear a value cu field clear "Last synced" + +# Reverse lookup: find the task whose field holds a given value +cu field find Repo https://github.com/owner/repo ``` ### Cache Management diff --git a/docs/site/commands/cu_export_tasks.md b/docs/site/commands/cu_export_tasks.md index 00613af..83d9e8e 100644 --- a/docs/site/commands/cu_export_tasks.md +++ b/docs/site/commands/cu_export_tasks.md @@ -26,11 +26,13 @@ cu export tasks [flags] --assignee string Filter by assignee -f, --format string Export format (csv, json, markdown) (default "csv") -h, --help help for tasks + --include-closed Include closed tasks in the export -l, --list string List ID to export tasks from -o, --output string Output file (default: stdout) --priority string Filter by priority -s, --space string Space ID to export tasks from --status string Filter by status + --subtasks Include subtasks in the export ``` ### Options inherited from parent commands diff --git a/docs/site/commands/cu_field.md b/docs/site/commands/cu_field.md index 544f8a2..7ca2c0c 100644 --- a/docs/site/commands/cu_field.md +++ b/docs/site/commands/cu_field.md @@ -28,6 +28,7 @@ for dropdowns and labels — so names can be used on the command line. * [cu](cu.md) - A GitHub CLI-inspired command-line interface for ClickUp * [cu field clear](cu_field_clear.md) - Clear a custom field value on a task +* [cu field find](cu_field_find.md) - Find tasks whose custom field equals a value * [cu field get](cu_field_get.md) - Show custom field values on a task * [cu field list](cu_field_list.md) - List custom fields available on a list * [cu field set](cu_field_set.md) - Set a custom field value on a task diff --git a/docs/site/commands/cu_field_find.md b/docs/site/commands/cu_field_find.md new file mode 100644 index 0000000..db267d5 --- /dev/null +++ b/docs/site/commands/cu_field_find.md @@ -0,0 +1,43 @@ +## cu field find + +Find tasks whose custom field equals a value + +### Synopsis + +Find tasks across the workspace whose custom field equals a value. + +Filtering happens server-side via Get Filtered Team Tasks, so this does not walk +every list. This is the reverse lookup: from an external identifier stored in a +custom field back to the task that owns it. + +A field id works anywhere. A field *name* has to be resolved against a list, so +pass --list or set a default; the resolved field itself is workspace-wide. + +Examples: + cu field find Repo https://github.com/owner/repo + cu field find a5013cb5-9244-49be-a6a2-90e1cc8a1d31 https://github.com/owner/repo + +``` +cu field find [flags] +``` + +### Options + +``` + -h, --help help for find + -l, --list string List used to resolve a field name to an id (defaults to the configured default list) +``` + +### Options inherited from parent commands + +``` + --config string config file (default is $HOME/.config/cu/config.yaml) + --debug enable debug mode + -o, --output string output format (table|json|yaml|csv) (default "table") +``` + +### SEE ALSO + +* [cu field](cu_field.md) - Manage custom field values + +###### Auto generated by spf13/cobra on 28-Aug-2026 diff --git a/docs/site/commands/cu_task_search.md b/docs/site/commands/cu_task_search.md index 047dc31..ef7a023 100644 --- a/docs/site/commands/cu_task_search.md +++ b/docs/site/commands/cu_task_search.md @@ -14,10 +14,12 @@ cu task search [query] [flags] ``` -h, --help help for search + --include-closed Include closed tasks in the search --include-description Search in task descriptions as well as names --limit int Maximum number of results to return (default 50) -l, --list string Limit search to specific list -s, --space string Limit search to specific space + --subtasks Include subtasks in the search ``` ### Options inherited from parent commands diff --git a/internal/api/teamtasks.go b/internal/api/teamtasks.go new file mode 100644 index 0000000..ff619bc --- /dev/null +++ b/internal/api/teamtasks.go @@ -0,0 +1,160 @@ +package api + +import ( + "context" + "fmt" + + "github.com/raksul/go-clickup/clickup" +) + +// tasksPerPage is the page size ClickUp uses for paged task endpoints. It is +// fixed server-side, so a short page means the last page. +const tasksPerPage = 100 + +// DefaultMaxTaskPages bounds pagination so a mistaken filter cannot walk an +// entire workspace forever under the rate limiter. Callers that genuinely want +// everything can raise it. +const DefaultMaxTaskPages = 50 + +// TeamTaskQuery filters a workspace-wide task search. Zero values mean "no +// filter", matching the API. +type TeamTaskQuery struct { + Statuses []string + Assignees []string + Tags []string + IncludeClosed bool + Subtasks bool + Archived bool + OrderBy string + Reverse bool + + // CustomFields filters server-side on custom field values — the only way + // to answer "which task has Repo = X" without walking every list. + CustomFields clickup.CustomFieldsInGetTasksRequest + + // MaxPages bounds pagination; zero means DefaultMaxTaskPages. + MaxPages int +} + +func (q *TeamTaskQuery) toSDK(page int) *clickup.GetTasksOptions { + opts := &clickup.GetTasksOptions{ + Page: page, + IncludeClosed: q.IncludeClosed, + Subtasks: q.Subtasks, + Archived: q.Archived, + OrderBy: q.OrderBy, + Reverse: q.Reverse, + } + if len(q.Statuses) > 0 { + opts.Statuses = q.Statuses + } + if len(q.Assignees) > 0 { + opts.Assignees = q.Assignees + } + if len(q.Tags) > 0 { + opts.Tags = q.Tags + } + if len(q.CustomFields) > 0 { + opts.CustomFields = q.CustomFields + } + return opts +} + +// TaskPageResult reports what a paged fetch actually covered, so callers can +// tell "no more results" from "we stopped early". +type TaskPageResult struct { + Tasks []clickup.Task + Pages int + Truncated bool // hit MaxPages with a full page still coming back +} + +// paginate walks pages until a short page (the last one) or maxPages, whichever +// comes first. Kept separate from the API calls so the termination and +// truncation rules are testable without a network round trip. +func paginate(maxPages int, fetch func(page int) ([]clickup.Task, error)) (*TaskPageResult, error) { + if maxPages <= 0 { + maxPages = DefaultMaxTaskPages + } + + result := &TaskPageResult{} + for page := 0; page < maxPages; page++ { + tasks, err := fetch(page) + if err != nil { + return nil, err + } + + result.Tasks = append(result.Tasks, tasks...) + result.Pages = page + 1 + + // A short page is the last page. + if len(tasks) < tasksPerPage { + return result, nil + } + } + + // Stopped at the cap with a full page still arriving — the caller must be + // able to say so rather than present a partial set as complete. + result.Truncated = true + return result, nil +} + +// SearchTeamTasks queries the workspace-wide Get Filtered Team Tasks endpoint, +// paging until exhaustion. +// +// This replaces walking workspace → spaces → folders → lists and calling +// GetTasks per list: that fan-out costs one request per list under a 100 +// req/min limit, and it only ever fetched page 0, so any list past 100 tasks +// was silently cut off. +func (c *Client) SearchTeamTasks(ctx context.Context, teamID string, query *TeamTaskQuery) (*TaskPageResult, error) { + if query == nil { + query = &TeamTaskQuery{} + } + return paginate(query.MaxPages, func(page int) ([]clickup.Task, error) { + if err := c.rateLimiter.Wait(ctx); err != nil { + return nil, err + } + tasks, _, err := c.client.Tasks.GetFilteredTeamTasks(ctx, teamID, query.toSDK(page)) + if err != nil { + return nil, c.handleError(err) + } + return tasks, nil + }) +} + +// ListTasksAllPages pages a single list to exhaustion. GetTasks fetches only +// the page it is asked for, so callers that want a whole list need this. +func (c *Client) ListTasksAllPages(ctx context.Context, listID string, options *TaskQueryOptions, maxPages int) (*TaskPageResult, error) { + if options == nil { + options = &TaskQueryOptions{} + } + return paginate(maxPages, func(page int) ([]clickup.Task, error) { + opts := *options + opts.Page = page + return c.GetTasks(ctx, listID, &opts) + }) +} + +// FindTasksByCustomField returns the tasks in a workspace whose custom field +// equals value — the reverse lookup from an external identifier back to a task. +func (c *Client) FindTasksByCustomField(ctx context.Context, teamID, fieldID, value string) ([]clickup.Task, error) { + if fieldID == "" { + return nil, fmt.Errorf("a custom field id is required") + } + + query := &TeamTaskQuery{ + IncludeClosed: true, // a match must not depend on the task being open + CustomFields: clickup.CustomFieldsInGetTasksRequest{ + { + FieldId: fieldID, + Operator: clickup.Equals, + Value: []string{value}, + }, + }, + } + + result, err := c.SearchTeamTasks(ctx, teamID, query) + if err != nil { + return nil, err + } + return result.Tasks, nil +} diff --git a/internal/api/teamtasks_test.go b/internal/api/teamtasks_test.go new file mode 100644 index 0000000..38fcc8f --- /dev/null +++ b/internal/api/teamtasks_test.go @@ -0,0 +1,139 @@ +package api + +import ( + "errors" + "testing" + + "github.com/raksul/go-clickup/clickup" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// pages builds a fetch func returning the given page sizes, recording which +// pages were actually requested. +func pages(sizes ...int) (func(int) ([]clickup.Task, error), *[]int) { + var requested []int + return func(page int) ([]clickup.Task, error) { + requested = append(requested, page) + if page >= len(sizes) { + return nil, nil + } + return make([]clickup.Task, sizes[page]), nil + }, &requested +} + +func TestPaginate(t *testing.T) { + t.Run("stops on the first short page", func(t *testing.T) { + fetch, requested := pages(tasksPerPage, tasksPerPage, 7) + + res, err := paginate(0, fetch) + require.NoError(t, err) + + assert.Len(t, res.Tasks, tasksPerPage*2+7) + assert.Equal(t, 3, res.Pages) + assert.False(t, res.Truncated) + assert.Equal(t, []int{0, 1, 2}, *requested, "pages are requested in order, and no further") + }) + + t.Run("a single short page is one request", func(t *testing.T) { + fetch, requested := pages(5) + + res, err := paginate(0, fetch) + require.NoError(t, err) + + assert.Len(t, res.Tasks, 5) + assert.Equal(t, []int{0}, *requested) + }) + + t.Run("an empty first page terminates", func(t *testing.T) { + fetch, requested := pages(0) + + res, err := paginate(0, fetch) + require.NoError(t, err) + + assert.Empty(t, res.Tasks) + assert.Equal(t, 1, res.Pages) + assert.False(t, res.Truncated) + assert.Equal(t, []int{0}, *requested) + }) + + t.Run("an exactly-full last page costs one extra request", func(t *testing.T) { + // A full page is indistinguishable from "more to come", so the next + // page must be requested to learn the set is exhausted. + fetch, requested := pages(tasksPerPage, 0) + + res, err := paginate(0, fetch) + require.NoError(t, err) + + assert.Len(t, res.Tasks, tasksPerPage) + assert.False(t, res.Truncated) + assert.Equal(t, []int{0, 1}, *requested) + }) + + t.Run("hitting the cap reports truncation instead of pretending completeness", func(t *testing.T) { + fetch, requested := pages(tasksPerPage, tasksPerPage, tasksPerPage, tasksPerPage) + + res, err := paginate(2, fetch) + require.NoError(t, err) + + assert.Len(t, res.Tasks, tasksPerPage*2) + assert.Equal(t, 2, res.Pages) + assert.True(t, res.Truncated, "callers must be able to tell a capped result from a complete one") + assert.Equal(t, []int{0, 1}, *requested, "the cap is respected exactly") + }) + + t.Run("errors abort rather than returning a partial page set", func(t *testing.T) { + boom := errors.New("boom") + fetch := func(page int) ([]clickup.Task, error) { + if page == 1 { + return nil, boom + } + return make([]clickup.Task, tasksPerPage), nil + } + + res, err := paginate(0, fetch) + require.ErrorIs(t, err, boom) + assert.Nil(t, res, "a partial result must not be mistaken for a complete one") + }) +} + +func TestTeamTaskQueryToSDK(t *testing.T) { + t.Run("empty filters are omitted", func(t *testing.T) { + opts := (&TeamTaskQuery{}).toSDK(3) + + assert.Equal(t, 3, opts.Page) + assert.Nil(t, opts.Statuses) + assert.Nil(t, opts.Assignees) + assert.Nil(t, opts.Tags) + assert.Nil(t, opts.CustomFields) + assert.False(t, opts.IncludeClosed) + assert.False(t, opts.Subtasks) + }) + + t.Run("filters are passed through", func(t *testing.T) { + q := &TeamTaskQuery{ + Statuses: []string{"in review"}, + Assignees: []string{"123"}, + Tags: []string{"unreleased"}, + IncludeClosed: true, + Subtasks: true, + OrderBy: "updated", + Reverse: true, + CustomFields: clickup.CustomFieldsInGetTasksRequest{ + {FieldId: "f1", Operator: clickup.Equals, Value: []string{"v"}}, + }, + } + + opts := q.toSDK(0) + + assert.Equal(t, []string{"in review"}, opts.Statuses) + assert.Equal(t, []string{"123"}, opts.Assignees) + assert.Equal(t, []string{"unreleased"}, opts.Tags) + assert.True(t, opts.IncludeClosed) + assert.True(t, opts.Subtasks) + assert.Equal(t, "updated", opts.OrderBy) + assert.True(t, opts.Reverse) + require.Len(t, opts.CustomFields, 1) + assert.Equal(t, "f1", opts.CustomFields[0].FieldId) + }) +} diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 4f467b4..c0e260d 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -56,6 +56,22 @@ var configGetCmd = &cobra.Command{ fmt.Fprintf(os.Stderr, "Configuration key '%s' not found\n", key) os.Exit(1) } + + // Redacted for the same reason `config list` is: what this defends + // against is incidental disclosure — pasted terminal output, a + // screen-share, a script whose stdout lands in a CI log — and `get` is + // the spelling most likely to be captured by one. It was never a way to + // reach a secret cu uses, since authentication reads the keyring; a + // value here is an unused plaintext leftover. The pointer goes to + // stderr so it reaches a person without joining piped output. + if config.IsCredentialKey(key) { + fmt.Println(config.RedactedValue) + fmt.Fprintf(os.Stderr, + "%q is not printed. cu authenticates via the system keyring; this value is an unused plaintext leftover.\nTo read or remove it, edit %s directly.\n", + key, config.GlobalConfigPath()) + return + } + fmt.Println(value) }, } diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index 237c06a..c877652 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -1,11 +1,15 @@ package cmd import ( + "bytes" + "io" + "os" "strings" "testing" "github.com/spf13/viper" "github.com/stretchr/testify/assert" + "github.com/timimsms/cu/internal/config" ) // Simple tests that don't involve os.Exit @@ -135,3 +139,63 @@ func TestConfigValueHandling(t *testing.T) { }) } } + +// captureStdout runs fn with os.Stdout redirected and returns what it wrote. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + orig := os.Stdout + os.Stdout = w + defer func() { os.Stdout = orig }() + + fn() + _ = w.Close() + + var buf bytes.Buffer + if _, err := io.Copy(&buf, r); err != nil { + t.Fatalf("read captured stdout: %v", err) + } + return buf.String() +} + +func TestConfigGetRedactsCredentials(t *testing.T) { + // The value is never printed even though `get` names the key explicitly: + // what redaction defends against is incidental disclosure, and `get` is the + // spelling most likely to be captured into a log or a pasted transcript. + t.Run("credential key is redacted", func(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set("api_token", "sk-must-not-be-printed") + + out := captureStdout(t, func() { configGetCmd.Run(configGetCmd, []string{"api_token"}) }) + + assert.NotContains(t, out, "sk-must-not-be-printed", "the token must not reach stdout") + assert.Contains(t, out, config.RedactedValue) + }) + + t.Run("ordinary key still prints its value", func(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set("default_list", "abc123") + + out := captureStdout(t, func() { configGetCmd.Run(configGetCmd, []string{"default_list"}) }) + + assert.Contains(t, out, "abc123") + }) +} + +func TestConfigListRedactsCredentials(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set("api_token", "sk-must-not-be-printed") + viper.Set("default_list", "abc123") + + out := captureStdout(t, func() { configListCmd.Run(configListCmd, nil) }) + + assert.NotContains(t, out, "sk-must-not-be-printed") + assert.Contains(t, out, "api_token="+config.RedactedValue) + assert.Contains(t, out, "default_list=abc123", "ordinary keys are unaffected") +} diff --git a/internal/cmd/export.go b/internal/cmd/export.go index 30eb544..d958322 100644 --- a/internal/cmd/export.go +++ b/internal/cmd/export.go @@ -93,12 +93,20 @@ Examples: } } - tasks, err = client.GetTasks(ctx, listID, queryOpts) + res, err := client.ListTasksAllPages(ctx, listID, queryOpts, 0) if err != nil { fmt.Fprintf(os.Stderr, "Failed to get tasks: %v\n", err) os.Exit(1) } + if res.Truncated { + fmt.Fprintf(os.Stderr, "Warning: stopped after %d pages; export may be incomplete.\n", api.DefaultMaxTaskPages) + } + tasks = res.Tasks } else { + // Defaults mirror the previous crawl (open tasks, no subtasks). + includeClosed, _ := cmd.Flags().GetBool("include-closed") + includeSubtasks, _ := cmd.Flags().GetBool("subtasks") + // Get all tasks from workspace or space workspaces, err := client.GetWorkspaces(ctx) if err != nil { @@ -106,38 +114,51 @@ Examples: os.Exit(1) } + // One server-side query per workspace instead of walking + // spaces → folders → lists. The crawl discarded GetFolders and + // GetLists errors, so an export could silently omit whole + // subtrees and still look successful. + var truncated bool for _, workspace := range workspaces { - spaces, err := client.GetSpaces(ctx, workspace.ID) + res, err := client.SearchTeamTasks(ctx, workspace.ID, &api.TeamTaskQuery{ + IncludeClosed: includeClosed, + Subtasks: includeSubtasks, + }) if err != nil { - continue + fmt.Fprintf(os.Stderr, "Failed to read tasks for workspace %s: %v\n", workspace.Name, err) + os.Exit(1) } + tasks = append(tasks, res.Tasks...) + truncated = truncated || res.Truncated + } - for _, space := range spaces { - if spaceID != "" && space.ID != spaceID && space.Name != spaceID { - continue - } + if truncated { + fmt.Fprintf(os.Stderr, "Warning: stopped after %d pages; export may be incomplete.\n", api.DefaultMaxTaskPages) + } - // Get tasks from all lists in space - folders, _ := client.GetFolders(ctx, space.ID) - for _, folder := range folders { - lists, _ := client.GetLists(ctx, folder.ID) - for _, list := range lists { - listTasks, err := client.GetTasks(ctx, list.ID, &api.TaskQueryOptions{}) - if err == nil { - tasks = append(tasks, listTasks...) - } + // Tasks carry only a space id, so resolve a --space given by name. + if spaceID != "" { + wantID := spaceID + for _, workspace := range workspaces { + spaces, err := client.GetSpaces(ctx, workspace.ID) + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to resolve spaces for workspace %s: %v\n", workspace.Name, err) + os.Exit(1) + } + for _, sp := range spaces { + if sp.Name == spaceID { + wantID = sp.ID } } + } - // Get folderless lists - lists, _ := client.GetFolderlessLists(ctx, space.ID) - for _, list := range lists { - listTasks, err := client.GetTasks(ctx, list.ID, &api.TaskQueryOptions{}) - if err == nil { - tasks = append(tasks, listTasks...) - } + filtered := tasks[:0] + for _, t := range tasks { + if t.Space.ID == wantID { + filtered = append(filtered, t) } } + tasks = filtered } // Client-side filtering @@ -350,6 +371,8 @@ func init() { exportTasksCmd.Flags().StringP("list", "l", "", "List ID to export tasks from") exportTasksCmd.Flags().StringP("space", "s", "", "Space ID to export tasks from") exportTasksCmd.Flags().StringP("format", "f", "csv", "Export format (csv, json, markdown)") + exportTasksCmd.Flags().Bool("include-closed", false, "Include closed tasks in the export") + exportTasksCmd.Flags().Bool("subtasks", false, "Include subtasks in the export") exportTasksCmd.Flags().StringP("output", "o", "", "Output file (default: stdout)") exportTasksCmd.Flags().String("status", "", "Filter by status") exportTasksCmd.Flags().String("priority", "", "Filter by priority") diff --git a/internal/cmd/field.go b/internal/cmd/field.go index d8c7ddd..fe2f325 100644 --- a/internal/cmd/field.go +++ b/internal/cmd/field.go @@ -188,6 +188,95 @@ func resolveTaskField(ctx context.Context, client *api.Client, taskID, nameOrID return api.FindCustomField(fields, nameOrID) } +var fieldFindCmd = &cobra.Command{ + Use: "find ", + Short: "Find tasks whose custom field equals a value", + Long: `Find tasks across the workspace whose custom field equals a value. + +Filtering happens server-side via Get Filtered Team Tasks, so this does not walk +every list. This is the reverse lookup: from an external identifier stored in a +custom field back to the task that owns it. + +A field id works anywhere. A field *name* has to be resolved against a list, so +pass --list or set a default; the resolved field itself is workspace-wide. + +Examples: + cu field find Repo https://github.com/owner/repo + cu field find a5013cb5-9244-49be-a6a2-90e1cc8a1d31 https://github.com/owner/repo`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + name, value := args[0], args[1] + + client, err := api.NewClient() + if err != nil { + return err + } + ctx := context.Background() + + fieldID, err := resolveFieldID(ctx, client, cmd, name) + if err != nil { + return err + } + + workspaces, err := client.GetWorkspaces(ctx) + if err != nil { + return err + } + + var rows []taskMatchRow + for _, w := range workspaces { + tasks, err := client.FindTasksByCustomField(ctx, w.ID, fieldID, value) + if err != nil { + return err + } + for _, t := range tasks { + rows = append(rows, taskMatchRow{ + ID: t.ID, + Name: t.Name, + Status: t.Status.Status, + List: t.List.Name, + URL: t.URL, + }) + } + } + + return output.Format(outputFormat, rows) + }, +} + +// taskMatchRow is the shape `field find` reports, kept narrow so the output is +// usable as input to other commands. +type taskMatchRow struct { + ID string `json:"id"` + Name string `json:"name"` + Status string `json:"status"` + List string `json:"list"` + URL string `json:"url"` +} + +// resolveFieldID accepts either a field id or a field name. A name is looked up +// against a list, since fields are only enumerable per list. +func resolveFieldID(ctx context.Context, client *api.Client, cmd *cobra.Command, nameOrID string) (string, error) { + listID, _ := cmd.Flags().GetString("list") + if listID == "" { + listID = config.GetString("default_list") + } + if listID == "" { + // Assume an id was passed; the API will reject it if not. + return nameOrID, nil + } + + fields, err := client.GetCustomFields(ctx, listID) + if err != nil { + return "", err + } + field, err := api.FindCustomField(fields, nameOrID) + if err != nil { + return "", err + } + return field.ID, nil +} + func init() { fieldListCmd.Flags().StringP("list", "l", "", "List ID (defaults to the configured default list)") @@ -195,4 +284,7 @@ func init() { fieldCmd.AddCommand(fieldGetCmd) fieldCmd.AddCommand(fieldSetCmd) fieldCmd.AddCommand(fieldClearCmd) + + fieldFindCmd.Flags().StringP("list", "l", "", "List used to resolve a field name to an id (defaults to the configured default list)") + fieldCmd.AddCommand(fieldFindCmd) } diff --git a/internal/cmd/task.go b/internal/cmd/task.go index 4a0ea6f..f18da92 100644 --- a/internal/cmd/task.go +++ b/internal/cmd/task.go @@ -517,6 +517,8 @@ var taskSearchCmd = &cobra.Command{ spaceID, _ := cmd.Flags().GetString("space") listID, _ := cmd.Flags().GetString("list") searchDescription, _ := cmd.Flags().GetBool("include-description") + includeClosed, _ := cmd.Flags().GetBool("include-closed") + includeSubtasks, _ := cmd.Flags().GetBool("subtasks") limit, _ := cmd.Flags().GetInt("limit") // Get workspaces to search @@ -532,81 +534,70 @@ var taskSearchCmd = &cobra.Command{ } var allTasks []clickup.Task - var searchErrors []string + var truncated bool - // If specific list is provided, search only that list if listID != "" { - tasks, err := client.GetTasks(ctx, listID, &api.TaskQueryOptions{}) + // A single list is already a direct query; page it to exhaustion + // rather than returning only the first 100 tasks. + res, err := client.ListTasksAllPages(ctx, listID, &api.TaskQueryOptions{}, 0) if err != nil { fmt.Fprintf(os.Stderr, "Failed to get tasks from list %s: %v\n", listID, err) os.Exit(1) } - allTasks = tasks + allTasks = res.Tasks + truncated = res.Truncated } else { - // Search across all lists in workspace or space + // One server-side query per workspace, paged to exhaustion — + // instead of walking spaces → folders → lists and issuing a + // request per list, which cost a request per list under the + // 100 req/min limit and only ever read each list's first page. for _, workspace := range workspaces { - spaces, err := client.GetSpaces(ctx, workspace.ID) + // Defaults mirror the previous per-list crawl (open tasks, + // no subtasks) so this stays a performance fix; the endpoint + // makes the fuller set reachable behind explicit flags. + res, err := client.SearchTeamTasks(ctx, workspace.ID, &api.TeamTaskQuery{ + IncludeClosed: includeClosed, + Subtasks: includeSubtasks, + }) if err != nil { - searchErrors = append(searchErrors, fmt.Sprintf("Failed to get spaces for workspace %s: %v", workspace.Name, err)) - continue + fmt.Fprintf(os.Stderr, "Failed to search workspace %s: %v\n", workspace.Name, err) + os.Exit(1) } + allTasks = append(allTasks, res.Tasks...) + truncated = truncated || res.Truncated + } - for _, space := range spaces { - // Skip if specific space is requested and this isn't it - if spaceID != "" && space.ID != spaceID && space.Name != spaceID { - continue - } - - // Get folders in space - folders, err := client.GetFolders(ctx, space.ID) + // The endpoint has no space filter, so scope client-side. Tasks + // carry only a space id, so a --space given as a name has to be + // resolved first — the previous crawl compared against both. + if spaceID != "" { + wantID := spaceID + for _, workspace := range workspaces { + spaces, err := client.GetSpaces(ctx, workspace.ID) if err != nil { - searchErrors = append(searchErrors, fmt.Sprintf("Failed to get folders for space %s: %v", space.Name, err)) continue } - - // Get tasks from folders - for _, folder := range folders { - lists, err := client.GetLists(ctx, folder.ID) - if err != nil { - searchErrors = append(searchErrors, fmt.Sprintf("Failed to get lists for folder %s: %v", folder.Name, err)) - continue - } - - for _, list := range lists { - tasks, err := client.GetTasks(ctx, list.ID, &api.TaskQueryOptions{}) - if err != nil { - searchErrors = append(searchErrors, fmt.Sprintf("Failed to get tasks for list %s: %v", list.Name, err)) - continue - } - allTasks = append(allTasks, tasks...) + for _, sp := range spaces { + if sp.Name == spaceID { + wantID = sp.ID } } + } - // Get folderless lists - lists, err := client.GetFolderlessLists(ctx, space.ID) - if err != nil { - searchErrors = append(searchErrors, fmt.Sprintf("Failed to get folderless lists for space %s: %v", space.Name, err)) - continue - } - - for _, list := range lists { - tasks, err := client.GetTasks(ctx, list.ID, &api.TaskQueryOptions{}) - if err != nil { - searchErrors = append(searchErrors, fmt.Sprintf("Failed to get tasks for list %s: %v", list.Name, err)) - continue - } - allTasks = append(allTasks, tasks...) + filtered := allTasks[:0] + for _, t := range allTasks { + if t.Space.ID == wantID { + filtered = append(filtered, t) } } + allTasks = filtered } } - // Print any errors encountered during search - if len(searchErrors) > 0 { - fmt.Fprintln(os.Stderr, "Some errors occurred during search:") - for _, err := range searchErrors { - fmt.Fprintf(os.Stderr, " - %s\n", err) - } + if truncated { + fmt.Fprintf(os.Stderr, + "Warning: stopped after %d pages; results may be incomplete. Narrow the search or raise the page cap.\n", + api.DefaultMaxTaskPages) } // Filter tasks based on search query @@ -725,6 +716,8 @@ func init() { // Search command flags taskSearchCmd.Flags().StringP("space", "s", "", "Limit search to specific space") taskSearchCmd.Flags().StringP("list", "l", "", "Limit search to specific list") + taskSearchCmd.Flags().Bool("include-closed", false, "Include closed tasks in the search") + taskSearchCmd.Flags().Bool("subtasks", false, "Include subtasks in the search") taskSearchCmd.Flags().Bool("include-description", false, "Search in task descriptions as well as names") taskSearchCmd.Flags().Int("limit", 50, "Maximum number of results to return") } diff --git a/internal/config/config.go b/internal/config/config.go index 38e6459..e424ec2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -67,6 +67,31 @@ func IsCredentialKey(key string) bool { // plaintext leftover, not the keyring entry cu actually authenticates with. const RedactedValue = "" +// stripCredentials returns settings without any credential key, warning about +// each one it drops. Both config files get the same treatment: a credential in +// either is a plaintext secret that cu will never authenticate with, so it is +// refused on the way in (a project .cu.yml being read) and on the way out (a +// project .cu.yml being written). +func stripCredentials(settings map[string]interface{}, path string) map[string]interface{} { + out := make(map[string]interface{}, len(settings)) + for k, v := range settings { + if IsCredentialKey(k) { + fmt.Fprintf(os.Stderr, + "cu: ignoring %q in %s — cu authenticates via the system keyring, not a config file\n", + k, path) + continue + } + out[k] = v + } + return out +} + +// GlobalConfigPath returns the global config file cu reads and writes, so +// commands can point the user at it by name rather than guessing. +func GlobalConfigPath() string { + return globalPath() +} + // globalPath returns the global config file to write. An explicit --config // always wins; otherwise a discovered file is used only while it still lives // under the configured directory, since DefaultConfigDir is a variable that @@ -127,15 +152,7 @@ func Init(cfgFile string) error { // Read project config if err := projectViper.ReadInConfig(); err == nil { - settings := projectViper.AllSettings() - for _, k := range credentialKeys { - if _, present := settings[k]; present { - delete(settings, k) - fmt.Fprintf(os.Stderr, - "cu: ignoring %q in %s — credentials come from the keyring, environment, or your global config\n", - k, projectConfigPath) - } - } + settings := stripCredentials(projectViper.AllSettings(), projectConfigPath) // MergeConfigMap merges into viper's *config* layer, so project // values override the global file while still losing to // environment variables and command-line flags. Using viper.Set @@ -317,6 +334,12 @@ func SaveProjectConfig(settings map[string]interface{}) error { } } + // A credential must not reach .cu.yml either. Init already refuses to read + // one back from that file, so writing it would leave a plaintext secret on + // disk that nothing ever uses — exactly the state the refusal exists to + // prevent, just reached from the other direction. + settings = stripCredentials(settings, projectConfigPath) + // Update with new settings for k, v := range settings { projectViper.Set(k, v) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 34bcb42..497892d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -654,3 +654,43 @@ func TestCredentialKeysAreNeverStaged(t *testing.T) { assert.Contains(t, string(written), "legacy-token") }) } + +func TestSaveProjectConfigRefusesCredentials(t *testing.T) { + // Init already refuses to read a credential back out of .cu.yml, so writing + // one there would strand a plaintext secret on disk that cu never uses. + // The guard closes that direction. + _, projDir := newLayeredFixture(t, "default_space: global-space\n", "default_list: from-project\n") + require.NoError(t, Init("")) + + require.NoError(t, SaveProjectConfig(map[string]interface{}{ + "api_token": "sk-must-not-be-written", + "default_list": "written", + })) + + written, err := os.ReadFile(filepath.Join(projDir, ProjectConfigFileName)) + require.NoError(t, err) + assert.NotContains(t, string(written), "sk-must-not-be-written", + "a credential must never be written to .cu.yml") + assert.Contains(t, string(written), "written", "ordinary keys are still saved") +} + +func TestSaveProjectConfigDoesNotMutateCallerMap(t *testing.T) { + // stripCredentials copies rather than deleting in place, so a caller that + // reuses its settings map does not silently lose keys. + newLayeredFixture(t, "", "default_list: from-project\n") + require.NoError(t, Init("")) + + settings := map[string]interface{}{"api_token": "sk-x", "default_list": "y"} + require.NoError(t, SaveProjectConfig(settings)) + + assert.Len(t, settings, 2, "the caller's map must be left alone") + assert.Equal(t, "sk-x", settings["api_token"]) +} + +func TestGlobalConfigPathNamesTheFileSaveWrites(t *testing.T) { + cfgDir, _ := newLayeredFixture(t, "default_space: global-space\n", "default_list: from-project\n") + require.NoError(t, Init("")) + + assert.Equal(t, filepath.Join(cfgDir, ConfigFileName+"."+ConfigType), GlobalConfigPath(), + "the path shown to users must be the one Save actually writes") +} diff --git a/mkdocs.yml b/mkdocs.yml index 43b4b31..6ae3797 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -64,6 +64,7 @@ nav: - cu field get: commands/cu_field_get.md - cu field set: commands/cu_field_set.md - cu field clear: commands/cu_field_clear.md + - cu field find: commands/cu_field_find.md - Comments: - cu comment: commands/cu_comment.md - cu comment list: commands/cu_comment_list.md From bc6396f907b8e0c33d8010c56577e7ee39360a74 Mon Sep 17 00:00:00 2001 From: Tim Walsh Date: Sat, 29 Aug 2026 14:08:33 -0700 Subject: [PATCH 2/2] chore(config): move the credential-surface change out of this branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These four files were swept into the previous commit by a broad `git add` while another change was in progress in the same working tree. They are unrelated to the search/export work this branch is about, and that commit's message does not mention them — so a change to what `cu config get` prints, and a new write guard on .cu.yml, were riding along undisclosed on a PR titled as a performance fix. They now live on their own in #50, against a clean main, where the security surface gets reviewed on its own terms. Nothing here depended on them: no code on this branch references GlobalConfigPath, stripCredentials, RedactedValue or IsCredentialKey, and the suite passes without them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ZEGsLHBQ2GzP6v48i4hXz --- internal/cmd/config.go | 16 --------- internal/cmd/config_test.go | 64 ---------------------------------- internal/config/config.go | 41 +++++----------------- internal/config/config_test.go | 40 --------------------- 4 files changed, 9 insertions(+), 152 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index c0e260d..4f467b4 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -56,22 +56,6 @@ var configGetCmd = &cobra.Command{ fmt.Fprintf(os.Stderr, "Configuration key '%s' not found\n", key) os.Exit(1) } - - // Redacted for the same reason `config list` is: what this defends - // against is incidental disclosure — pasted terminal output, a - // screen-share, a script whose stdout lands in a CI log — and `get` is - // the spelling most likely to be captured by one. It was never a way to - // reach a secret cu uses, since authentication reads the keyring; a - // value here is an unused plaintext leftover. The pointer goes to - // stderr so it reaches a person without joining piped output. - if config.IsCredentialKey(key) { - fmt.Println(config.RedactedValue) - fmt.Fprintf(os.Stderr, - "%q is not printed. cu authenticates via the system keyring; this value is an unused plaintext leftover.\nTo read or remove it, edit %s directly.\n", - key, config.GlobalConfigPath()) - return - } - fmt.Println(value) }, } diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index c877652..237c06a 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -1,15 +1,11 @@ package cmd import ( - "bytes" - "io" - "os" "strings" "testing" "github.com/spf13/viper" "github.com/stretchr/testify/assert" - "github.com/timimsms/cu/internal/config" ) // Simple tests that don't involve os.Exit @@ -139,63 +135,3 @@ func TestConfigValueHandling(t *testing.T) { }) } } - -// captureStdout runs fn with os.Stdout redirected and returns what it wrote. -func captureStdout(t *testing.T, fn func()) string { - t.Helper() - r, w, err := os.Pipe() - if err != nil { - t.Fatalf("pipe: %v", err) - } - orig := os.Stdout - os.Stdout = w - defer func() { os.Stdout = orig }() - - fn() - _ = w.Close() - - var buf bytes.Buffer - if _, err := io.Copy(&buf, r); err != nil { - t.Fatalf("read captured stdout: %v", err) - } - return buf.String() -} - -func TestConfigGetRedactsCredentials(t *testing.T) { - // The value is never printed even though `get` names the key explicitly: - // what redaction defends against is incidental disclosure, and `get` is the - // spelling most likely to be captured into a log or a pasted transcript. - t.Run("credential key is redacted", func(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - viper.Set("api_token", "sk-must-not-be-printed") - - out := captureStdout(t, func() { configGetCmd.Run(configGetCmd, []string{"api_token"}) }) - - assert.NotContains(t, out, "sk-must-not-be-printed", "the token must not reach stdout") - assert.Contains(t, out, config.RedactedValue) - }) - - t.Run("ordinary key still prints its value", func(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - viper.Set("default_list", "abc123") - - out := captureStdout(t, func() { configGetCmd.Run(configGetCmd, []string{"default_list"}) }) - - assert.Contains(t, out, "abc123") - }) -} - -func TestConfigListRedactsCredentials(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - viper.Set("api_token", "sk-must-not-be-printed") - viper.Set("default_list", "abc123") - - out := captureStdout(t, func() { configListCmd.Run(configListCmd, nil) }) - - assert.NotContains(t, out, "sk-must-not-be-printed") - assert.Contains(t, out, "api_token="+config.RedactedValue) - assert.Contains(t, out, "default_list=abc123", "ordinary keys are unaffected") -} diff --git a/internal/config/config.go b/internal/config/config.go index e424ec2..38e6459 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -67,31 +67,6 @@ func IsCredentialKey(key string) bool { // plaintext leftover, not the keyring entry cu actually authenticates with. const RedactedValue = "" -// stripCredentials returns settings without any credential key, warning about -// each one it drops. Both config files get the same treatment: a credential in -// either is a plaintext secret that cu will never authenticate with, so it is -// refused on the way in (a project .cu.yml being read) and on the way out (a -// project .cu.yml being written). -func stripCredentials(settings map[string]interface{}, path string) map[string]interface{} { - out := make(map[string]interface{}, len(settings)) - for k, v := range settings { - if IsCredentialKey(k) { - fmt.Fprintf(os.Stderr, - "cu: ignoring %q in %s — cu authenticates via the system keyring, not a config file\n", - k, path) - continue - } - out[k] = v - } - return out -} - -// GlobalConfigPath returns the global config file cu reads and writes, so -// commands can point the user at it by name rather than guessing. -func GlobalConfigPath() string { - return globalPath() -} - // globalPath returns the global config file to write. An explicit --config // always wins; otherwise a discovered file is used only while it still lives // under the configured directory, since DefaultConfigDir is a variable that @@ -152,7 +127,15 @@ func Init(cfgFile string) error { // Read project config if err := projectViper.ReadInConfig(); err == nil { - settings := stripCredentials(projectViper.AllSettings(), projectConfigPath) + settings := projectViper.AllSettings() + for _, k := range credentialKeys { + if _, present := settings[k]; present { + delete(settings, k) + fmt.Fprintf(os.Stderr, + "cu: ignoring %q in %s — credentials come from the keyring, environment, or your global config\n", + k, projectConfigPath) + } + } // MergeConfigMap merges into viper's *config* layer, so project // values override the global file while still losing to // environment variables and command-line flags. Using viper.Set @@ -334,12 +317,6 @@ func SaveProjectConfig(settings map[string]interface{}) error { } } - // A credential must not reach .cu.yml either. Init already refuses to read - // one back from that file, so writing it would leave a plaintext secret on - // disk that nothing ever uses — exactly the state the refusal exists to - // prevent, just reached from the other direction. - settings = stripCredentials(settings, projectConfigPath) - // Update with new settings for k, v := range settings { projectViper.Set(k, v) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 497892d..34bcb42 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -654,43 +654,3 @@ func TestCredentialKeysAreNeverStaged(t *testing.T) { assert.Contains(t, string(written), "legacy-token") }) } - -func TestSaveProjectConfigRefusesCredentials(t *testing.T) { - // Init already refuses to read a credential back out of .cu.yml, so writing - // one there would strand a plaintext secret on disk that cu never uses. - // The guard closes that direction. - _, projDir := newLayeredFixture(t, "default_space: global-space\n", "default_list: from-project\n") - require.NoError(t, Init("")) - - require.NoError(t, SaveProjectConfig(map[string]interface{}{ - "api_token": "sk-must-not-be-written", - "default_list": "written", - })) - - written, err := os.ReadFile(filepath.Join(projDir, ProjectConfigFileName)) - require.NoError(t, err) - assert.NotContains(t, string(written), "sk-must-not-be-written", - "a credential must never be written to .cu.yml") - assert.Contains(t, string(written), "written", "ordinary keys are still saved") -} - -func TestSaveProjectConfigDoesNotMutateCallerMap(t *testing.T) { - // stripCredentials copies rather than deleting in place, so a caller that - // reuses its settings map does not silently lose keys. - newLayeredFixture(t, "", "default_list: from-project\n") - require.NoError(t, Init("")) - - settings := map[string]interface{}{"api_token": "sk-x", "default_list": "y"} - require.NoError(t, SaveProjectConfig(settings)) - - assert.Len(t, settings, 2, "the caller's map must be left alone") - assert.Equal(t, "sk-x", settings["api_token"]) -} - -func TestGlobalConfigPathNamesTheFileSaveWrites(t *testing.T) { - cfgDir, _ := newLayeredFixture(t, "default_space: global-space\n", "default_list: from-project\n") - require.NoError(t, Init("")) - - assert.Equal(t, filepath.Join(cfgDir, ConfigFileName+"."+ConfigType), GlobalConfigPath(), - "the path shown to users must be the one Save actually writes") -}