diff --git a/README.md b/README.md index 6413b58760..432541f434 100644 --- a/README.md +++ b/README.md @@ -1126,7 +1126,9 @@ For negotiated supported protocol versions `2026-07-28` or newer, the consolidat - **list_label** - List labels from a repository - **OAuth Challenge Scopes**: `repo` + - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) - `owner`: Repository owner (username or organization name) - required for all operations (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `repo`: Repository name - required for all operations (string, required) diff --git a/pkg/github/__toolsnaps__/list_label.snap b/pkg/github/__toolsnaps__/list_label.snap index f85cd9a77d..80e898b71b 100644 --- a/pkg/github/__toolsnaps__/list_label.snap +++ b/pkg/github/__toolsnaps__/list_label.snap @@ -4,13 +4,23 @@ "readOnlyHint": true, "title": "List labels from a repository" }, - "description": "List labels from a repository, ordered by issue count (descending) so the most-used labels are returned first", + "description": "List labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter.", "inputSchema": { "properties": { + "after": { + "description": "Cursor for pagination. Use the cursor from the previous response.", + "type": "string" + }, "owner": { "description": "Repository owner (username or organization name) - required for all operations", "type": "string" }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, "repo": { "description": "Repository name - required for all operations", "type": "string" @@ -53,6 +63,28 @@ "array" ] }, + "pageInfo": { + "additionalProperties": false, + "properties": { + "endCursor": { + "type": "string" + }, + "hasNextPage": { + "type": "boolean" + }, + "hasPreviousPage": { + "type": "boolean" + }, + "startCursor": { + "type": "string" + } + }, + "required": [ + "hasNextPage", + "hasPreviousPage" + ], + "type": "object" + }, "totalCount": { "description": "Total labels in the repository.", "type": "integer" @@ -60,7 +92,8 @@ }, "required": [ "labels", - "totalCount" + "totalCount", + "pageInfo" ], "type": "object" } diff --git a/pkg/github/labels.go b/pkg/github/labels.go index 70b5c5d8db..c9479e5062 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -132,6 +132,9 @@ func GetLabelForLabelsToolset(t translations.TranslationHelperFunc) inventory.Se return tool } +// listLabelsDefaultPerPage keeps the first page as large as GraphQL allows, so most repositories need one call. +const listLabelsDefaultPerPage = 100 + // ListLabels lists labels from a repository func ListLabels(t translations.TranslationHelperFunc) inventory.ServerTool { return typedGovernanceTool(listLabelsLegacy(t), listLabelsOutputSchema(), decodeGovernanceJSON[ListLabelsOutput]) @@ -142,12 +145,12 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool ToolsetLabels, mcp.Tool{ Name: "list_label", - Description: t("TOOL_LIST_LABEL_DESCRIPTION", "List labels from a repository, ordered by issue count (descending) so the most-used labels are returned first"), + Description: t("TOOL_LIST_LABEL_DESCRIPTION", "List labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_LIST_LABEL_TITLE", "List labels from a repository"), ReadOnlyHint: true, }, - InputSchema: &jsonschema.Schema{ + InputSchema: WithCursorPagination(&jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ "owner": { @@ -160,7 +163,7 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool }, }, Required: []string{"owner", "repo"}, - }, + }), }, scopes.PublicRead(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { @@ -174,6 +177,19 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultError(err.Error()), nil, nil } + perPage, err := OptionalIntParamWithDefault(args, "perPage", listLabelsDefaultPerPage) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + after, err := OptionalParam[string](args, "after") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + paginationParams, err := CursorPaginationParams{PerPage: perPage, After: after}.ToGraphQLParams() + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + client, err := deps.GetGQLClient(ctx) if err != nil { return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) @@ -189,17 +205,29 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool Description githubv4.String } TotalCount githubv4.Int - } `graphql:"labels(first: 100, orderBy: {field: $orderByField, direction: $orderByDirection})"` + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + } `graphql:"labels(first: $first, after: $after, orderBy: {field: $orderByField, direction: $orderByDirection})"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]any{ "owner": githubv4.String(owner), "repo": githubv4.String(repo), + "first": githubv4.Int(*paginationParams.First), // Order labels by issue count (descending) so the most-used labels are returned first. "orderByField": labelOrderFieldIssueCount, "orderByDirection": githubv4.OrderDirectionDesc, } + if paginationParams.After != nil { + vars["after"] = githubv4.String(*paginationParams.After) + } else { + vars["after"] = (*githubv4.String)(nil) + } if err := client.Query(ctx, &query, vars); err != nil { return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to list labels", err), nil, nil @@ -215,9 +243,16 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool } } + labelsPage := query.Repository.Labels.PageInfo response := map[string]any{ "labels": labels, "totalCount": int(query.Repository.Labels.TotalCount), + "pageInfo": MinimalPageInfo{ + HasNextPage: bool(labelsPage.HasNextPage), + HasPreviousPage: bool(labelsPage.HasPreviousPage), + StartCursor: string(labelsPage.StartCursor), + EndCursor: string(labelsPage.EndCursor), + }, } out, err := json.Marshal(response) diff --git a/pkg/github/labels_test.go b/pkg/github/labels_test.go index a527664a81..43075659bc 100644 --- a/pkg/github/labels_test.go +++ b/pkg/github/labels_test.go @@ -2,6 +2,7 @@ package github import ( "context" + "encoding/json" "net/http" "testing" @@ -156,6 +157,7 @@ func TestListLabels(t *testing.T) { mockedClient *http.Client expectToolError bool expectedToolErrMsg string + expectedPageInfo MinimalPageInfo }{ { name: "successful repository labels listing", @@ -163,51 +165,57 @@ func TestListLabels(t *testing.T) { "owner": "owner", "repo": "repo", }, - mockedClient: githubv4mock.NewMockedHTTPClient( - githubv4mock.NewQueryMatcher( - struct { - Repository struct { - Labels struct { - Nodes []struct { - ID githubv4.ID - Name githubv4.String - Color githubv4.String - Description githubv4.String - } - TotalCount githubv4.Int - } `graphql:"labels(first: 100, orderBy: {field: $orderByField, direction: $orderByDirection})"` - } `graphql:"repository(owner: $owner, name: $repo)"` - }{}, - map[string]any{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "orderByField": labelOrderFieldIssueCount, - "orderByDirection": githubv4.OrderDirectionDesc, - }, - githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ - "labels": map[string]any{ - "nodes": []any{ - map[string]any{ - "id": githubv4.ID("label-1"), - "name": githubv4.String("bug"), - "color": githubv4.String("d73a4a"), - "description": githubv4.String("Something isn't working"), - }, - map[string]any{ - "id": githubv4.ID("label-2"), - "name": githubv4.String("enhancement"), - "color": githubv4.String("a2eeef"), - "description": githubv4.String("New feature or request"), - }, - }, - "totalCount": githubv4.Int(2), - }, - }, - }), - ), - ), - expectToolError: false, + mockedClient: githubv4mock.NewMockedHTTPClient(listLabelsMatcher(100, (*githubv4.String)(nil), map[string]any{ + "hasNextPage": githubv4.Boolean(false), + "hasPreviousPage": githubv4.Boolean(false), + "startCursor": githubv4.String("c1"), + "endCursor": githubv4.String("c2"), + })), + expectToolError: false, + expectedPageInfo: MinimalPageInfo{StartCursor: "c1", EndCursor: "c2"}, + }, + { + name: "reports the next page when the repository has more labels", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "perPage": float64(2), + }, + mockedClient: githubv4mock.NewMockedHTTPClient(listLabelsMatcher(2, (*githubv4.String)(nil), map[string]any{ + "hasNextPage": githubv4.Boolean(true), + "hasPreviousPage": githubv4.Boolean(false), + "startCursor": githubv4.String("c1"), + "endCursor": githubv4.String("c2"), + })), + expectToolError: false, + expectedPageInfo: MinimalPageInfo{HasNextPage: true, StartCursor: "c1", EndCursor: "c2"}, + }, + { + name: "passes the after cursor to fetch the next page", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "after": "c2", + }, + mockedClient: githubv4mock.NewMockedHTTPClient(listLabelsMatcher(100, githubv4.String("c2"), map[string]any{ + "hasNextPage": githubv4.Boolean(false), + "hasPreviousPage": githubv4.Boolean(true), + "startCursor": githubv4.String("c3"), + "endCursor": githubv4.String("c4"), + })), + expectToolError: false, + expectedPageInfo: MinimalPageInfo{HasPreviousPage: true, StartCursor: "c3", EndCursor: "c4"}, + }, + { + name: "rejects a page size above 100", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "perPage": float64(101), + }, + mockedClient: githubv4mock.NewMockedHTTPClient(), + expectToolError: true, + expectedToolErrMsg: "perPage value 101 exceeds maximum of 100", }, } @@ -233,11 +241,61 @@ func TestListLabels(t *testing.T) { } } else { assert.False(t, result.IsError) + var response ListLabelsOutput + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Len(t, response.Labels, 2) + assert.Equal(t, 5, response.TotalCount) + assert.Equal(t, tc.expectedPageInfo, response.PageInfo) } }) } } +// listLabelsMatcher matches one list_label page request and answers with two labels and the given page info. +func listLabelsMatcher(first int32, after any, pageInfo map[string]any) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Labels struct { + Nodes []struct { + ID githubv4.ID + Name githubv4.String + Color githubv4.String + Description githubv4.String + } + TotalCount githubv4.Int + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + } `graphql:"labels(first: $first, after: $after, orderBy: {field: $orderByField, direction: $orderByDirection})"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "first": githubv4.Int(first), + "after": after, + "orderByField": labelOrderFieldIssueCount, + "orderByDirection": githubv4.OrderDirectionDesc, + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "labels": map[string]any{ + "nodes": []any{ + map[string]any{"id": githubv4.ID("label-1"), "name": githubv4.String("bug"), "color": githubv4.String("d73a4a"), "description": githubv4.String("Something isn't working")}, + map[string]any{"id": githubv4.ID("label-2"), "name": githubv4.String("enhancement"), "color": githubv4.String("a2eeef"), "description": githubv4.String("New feature or request")}, + }, + "totalCount": githubv4.Int(5), + "pageInfo": pageInfo, + }, + }, + }), + ) +} + func TestWriteLabel(t *testing.T) { t.Parallel() diff --git a/pkg/github/typed_governance_outputs.go b/pkg/github/typed_governance_outputs.go index 74925ad881..02952bac0b 100644 --- a/pkg/github/typed_governance_outputs.go +++ b/pkg/github/typed_governance_outputs.go @@ -20,8 +20,9 @@ type MinimalLabel struct { } type ListLabelsOutput struct { - Labels []MinimalLabel `json:"labels"` - TotalCount int `json:"totalCount" jsonschema:"Total labels in the repository."` + Labels []MinimalLabel `json:"labels"` + TotalCount int `json:"totalCount" jsonschema:"Total labels in the repository."` + PageInfo MinimalPageInfo `json:"pageInfo"` } type LabelWriteOutput struct { diff --git a/pkg/github/typed_governance_outputs_test.go b/pkg/github/typed_governance_outputs_test.go index b274d0e68d..094de91547 100644 --- a/pkg/github/typed_governance_outputs_test.go +++ b/pkg/github/typed_governance_outputs_test.go @@ -72,7 +72,7 @@ func assertGovernanceProjection(t *testing.T, tc governanceWireCase, output any) case "get_label": assert.JSONEq(t, `{"name":"bug","color":"ff0000","description":"desc"}`, mustMarshalJSON(t, output)) case "list_label": - assert.JSONEq(t, `{"labels":[{"name":"bug","color":"ff0000","description":"desc"}],"totalCount":1}`, mustMarshalJSON(t, output)) + assert.JSONEq(t, `{"labels":[{"name":"bug","color":"ff0000","description":"desc"}],"totalCount":1,"pageInfo":{"hasNextPage":false,"hasPreviousPage":false}}`, mustMarshalJSON(t, output)) case "label_write": assert.Equal(t, tc.args["method"], object["method"]) assert.Equal(t, tc.text, object["message"]) @@ -236,7 +236,7 @@ func governanceWireCases(t *testing.T) []governanceWireCase { tr := translations.NullTranslationHelper return []governanceWireCase{ {tool: GetLabel(tr), args: map[string]any{"owner": "o", "repo": "r", "name": "bug"}, text: `{"id":"L1","name":"bug","color":"ff0000","description":"desc"}`}, - {tool: ListLabels(tr), args: map[string]any{"owner": "o", "repo": "r"}, text: `{"labels":[{"id":"L1","name":"bug","color":"ff0000","description":"desc"}],"totalCount":1}`}, + {tool: ListLabels(tr), args: map[string]any{"owner": "o", "repo": "r"}, text: `{"labels":[{"id":"L1","name":"bug","color":"ff0000","description":"desc"}],"totalCount":1,"pageInfo":{"hasNextPage":false,"hasPreviousPage":false}}`}, {tool: LabelWrite(tr), args: map[string]any{"method": "create", "owner": "o", "repo": "r", "name": "feature", "color": "abcdef"}, text: "label 'feature' created successfully"}, {tool: LabelWrite(tr), args: map[string]any{"method": "update", "owner": "o", "repo": "r", "name": "bug", "new_name": "defect"}, text: "label 'defect' updated successfully"}, {tool: LabelWrite(tr), args: map[string]any{"method": "delete", "owner": "o", "repo": "r", "name": "bug"}, text: "label 'bug' deleted successfully"},