From 7311be60bc129d6a07d3d602e4e5dff8e1f9769b Mon Sep 17 00:00:00 2001 From: Andrew Georgiou Date: Thu, 15 Oct 2026 14:07:28 +0100 Subject: [PATCH 1/3] Return every label from list_label instead of the first 100 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d8a9000c-ebe8-4118-ae89-a942861e4166 --- pkg/github/__toolsnaps__/list_label.snap | 9 ++- pkg/github/labels.go | 49 ++++++++---- pkg/github/labels_test.go | 88 ++++++++++++++++++++- pkg/github/typed_governance_outputs.go | 1 + pkg/github/typed_governance_outputs_test.go | 4 +- 5 files changed, 131 insertions(+), 20 deletions(-) diff --git a/pkg/github/__toolsnaps__/list_label.snap b/pkg/github/__toolsnaps__/list_label.snap index f85cd9a77d..4a7c4d04fe 100644 --- a/pkg/github/__toolsnaps__/list_label.snap +++ b/pkg/github/__toolsnaps__/list_label.snap @@ -4,7 +4,7 @@ "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 all labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. has_more is true when the repository has more labels than could be returned; use get_label to check a specific label by name.", "inputSchema": { "properties": { "owner": { @@ -26,6 +26,10 @@ "outputSchema": { "additionalProperties": false, "properties": { + "has_more": { + "description": "Whether the repository has more labels than were returned.", + "type": "boolean" + }, "labels": { "items": { "additionalProperties": false, @@ -60,7 +64,8 @@ }, "required": [ "labels", - "totalCount" + "totalCount", + "has_more" ], "type": "object" } diff --git a/pkg/github/labels.go b/pkg/github/labels.go index 70b5c5d8db..583f3b12d4 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -132,6 +132,9 @@ func GetLabelForLabelsToolset(t translations.TranslationHelperFunc) inventory.Se return tool } +// listLabelsMaxPages bounds list_label to 1,000 labels so one call cannot page indefinitely. +const listLabelsMaxPages = 10 + // ListLabels lists labels from a repository func ListLabels(t translations.TranslationHelperFunc) inventory.ServerTool { return typedGovernanceTool(listLabelsLegacy(t), listLabelsOutputSchema(), decodeGovernanceJSON[ListLabelsOutput]) @@ -142,7 +145,7 @@ 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 all labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. has_more is true when the repository has more labels than could be returned; use get_label to check a specific label by name."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_LIST_LABEL_TITLE", "List labels from a repository"), ReadOnlyHint: true, @@ -189,35 +192,51 @@ 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 + EndCursor githubv4.String + } + } `graphql:"labels(first: 100, after: $cursor, orderBy: {field: $orderByField, direction: $orderByDirection})"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]any{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "cursor": (*githubv4.String)(nil), // Order labels by issue count (descending) so the most-used labels are returned first. "orderByField": labelOrderFieldIssueCount, "orderByDirection": githubv4.OrderDirectionDesc, } - if err := client.Query(ctx, &query, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to list labels", err), nil, nil - } - - labels := make([]map[string]any, len(query.Repository.Labels.Nodes)) - for i, labelNode := range query.Repository.Labels.Nodes { - labels[i] = map[string]any{ - "id": fmt.Sprintf("%v", labelNode.ID), - "name": string(labelNode.Name), - "color": string(labelNode.Color), - "description": string(labelNode.Description), + labels := make([]map[string]any, 0) + hasMore := false + for page := 1; ; page++ { + if err := client.Query(ctx, &query, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to list labels", err), nil, nil + } + for _, labelNode := range query.Repository.Labels.Nodes { + labels = append(labels, map[string]any{ + "id": fmt.Sprintf("%v", labelNode.ID), + "name": string(labelNode.Name), + "color": string(labelNode.Color), + "description": string(labelNode.Description), + }) + } + if !query.Repository.Labels.PageInfo.HasNextPage { + break + } + if page >= listLabelsMaxPages { + hasMore = true + break } + vars["cursor"] = githubv4.NewString(query.Repository.Labels.PageInfo.EndCursor) } response := map[string]any{ "labels": labels, "totalCount": int(query.Repository.Labels.TotalCount), + "has_more": hasMore, } out, err := json.Marshal(response) diff --git a/pkg/github/labels_test.go b/pkg/github/labels_test.go index a527664a81..0a8974a762 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,9 @@ func TestListLabels(t *testing.T) { mockedClient *http.Client expectToolError bool expectedToolErrMsg string + expectedNames []string + expectedCount int + expectHasMore bool }{ { name: "successful repository labels listing", @@ -175,12 +179,17 @@ func TestListLabels(t *testing.T) { Description githubv4.String } TotalCount githubv4.Int - } `graphql:"labels(first: 100, orderBy: {field: $orderByField, direction: $orderByDirection})"` + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"labels(first: 100, after: $cursor, orderBy: {field: $orderByField, direction: $orderByDirection})"` } `graphql:"repository(owner: $owner, name: $repo)"` }{}, map[string]any{ "owner": githubv4.String("owner"), "repo": githubv4.String("repo"), + "cursor": (*githubv4.String)(nil), "orderByField": labelOrderFieldIssueCount, "orderByDirection": githubv4.OrderDirectionDesc, }, @@ -202,12 +211,38 @@ func TestListLabels(t *testing.T) { }, }, "totalCount": githubv4.Int(2), + "pageInfo": map[string]any{ + "hasNextPage": githubv4.Boolean(false), + "endCursor": githubv4.String(""), + }, }, }, }), ), ), expectToolError: false, + expectedNames: []string{"bug", "enhancement"}, + }, + { + name: "follows cursors so the least-used labels are not dropped", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + mockedClient: twoPageLabelsClient(t), + expectToolError: false, + expectedNames: []string{"bug", "rarely-used"}, + }, + { + name: "stops at the page cap and sets has_more", + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + mockedClient: alwaysHasNextPageLabelsClient(t), + expectToolError: false, + expectedCount: listLabelsMaxPages, + expectHasMore: true, }, } @@ -233,11 +268,62 @@ func TestListLabels(t *testing.T) { } } else { assert.False(t, result.IsError) + var response struct { + Labels []struct { + Name string `json:"name"` + } `json:"labels"` + HasMore bool `json:"has_more"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + names := make([]string, len(response.Labels)) + for i, label := range response.Labels { + names[i] = label.Name + } + if tc.expectedNames != nil { + assert.Equal(t, tc.expectedNames, names) + } + if tc.expectedCount != 0 { + assert.Len(t, names, tc.expectedCount) + } + assert.Equal(t, tc.expectHasMore, response.HasMore) } }) } } +// twoPageLabelsClient serves a first page that reports another page, then a second page reached only through its cursor. +func twoPageLabelsClient(t *testing.T) *http.Client { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/graphql", func(w http.ResponseWriter, r *http.Request) { + var req struct { + Variables map[string]any `json:"variables"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + name, hasNext := "bug", true + if req.Variables["cursor"] == "page-2" { + name, hasNext = "rarely-used", false + } + resp := map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "labels": map[string]any{ + "nodes": []any{map[string]any{"id": name, "name": name, "color": "ededed", "description": ""}}, + "totalCount": 2, + "pageInfo": map[string]any{"hasNextPage": hasNext, "endCursor": "page-2"}, + }, + }, + }, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + return &http.Client{Transport: recorderTransport{handler: mux}} +} + 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..8e79e1398d 100644 --- a/pkg/github/typed_governance_outputs.go +++ b/pkg/github/typed_governance_outputs.go @@ -22,6 +22,7 @@ type MinimalLabel struct { type ListLabelsOutput struct { Labels []MinimalLabel `json:"labels"` TotalCount int `json:"totalCount" jsonschema:"Total labels in the repository."` + HasMore bool `json:"has_more" jsonschema:"Whether the repository has more labels than were returned."` } type LabelWriteOutput struct { diff --git a/pkg/github/typed_governance_outputs_test.go b/pkg/github/typed_governance_outputs_test.go index b274d0e68d..f1822e3765 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,"has_more":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,"has_more":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"}, From e0bc20f85a5d873779adbcce80864e334b53c57b Mon Sep 17 00:00:00 2001 From: Andrew Georgiou Date: Thu, 15 Oct 2026 16:03:51 +0100 Subject: [PATCH 2/3] Page list_label with cursors instead of fetching every label Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d8a9000c-ebe8-4118-ae89-a942861e4166 --- README.md | 2 + pkg/github/__toolsnaps__/list_label.snap | 40 +++- pkg/github/labels.go | 82 ++++---- pkg/github/labels_test.go | 200 +++++++++----------- pkg/github/typed_governance_outputs.go | 6 +- pkg/github/typed_governance_outputs_test.go | 4 +- 6 files changed, 176 insertions(+), 158 deletions(-) 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 4a7c4d04fe..baebe1a204 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 all labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. has_more is true when the repository has more labels than could be returned; use get_label to check a specific label by name.", + "description": "List labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. Returns up to 100 labels per page by default. If 'pageInfo.hasNextPage' is true, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter to get the rest. Use get_label to check a specific label by name.", "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" @@ -26,10 +36,6 @@ "outputSchema": { "additionalProperties": false, "properties": { - "has_more": { - "description": "Whether the repository has more labels than were returned.", - "type": "boolean" - }, "labels": { "items": { "additionalProperties": false, @@ -57,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" @@ -65,7 +93,7 @@ "required": [ "labels", "totalCount", - "has_more" + "pageInfo" ], "type": "object" } diff --git a/pkg/github/labels.go b/pkg/github/labels.go index 583f3b12d4..55d35e3b27 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -132,8 +132,8 @@ func GetLabelForLabelsToolset(t translations.TranslationHelperFunc) inventory.Se return tool } -// listLabelsMaxPages bounds list_label to 1,000 labels so one call cannot page indefinitely. -const listLabelsMaxPages = 10 +// 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 { @@ -145,12 +145,12 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool ToolsetLabels, mcp.Tool{ Name: "list_label", - Description: t("TOOL_LIST_LABEL_DESCRIPTION", "List all labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. has_more is true when the repository has more labels than could be returned; use get_label to check a specific label by name."), + Description: t("TOOL_LIST_LABEL_DESCRIPTION", "List labels from a repository, ordered by issue count (descending) so the most-used labels are returned first. Returns up to 100 labels per page by default. If 'pageInfo.hasNextPage' is true, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter to get the rest. Use get_label to check a specific label by name."), 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": { @@ -163,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) { @@ -177,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) @@ -193,50 +206,53 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool } TotalCount githubv4.Int PageInfo struct { - HasNextPage githubv4.Boolean - EndCursor githubv4.String + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String } - } `graphql:"labels(first: 100, after: $cursor, orderBy: {field: $orderByField, direction: $orderByDirection})"` + } `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), - "cursor": (*githubv4.String)(nil), + "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) + } - labels := make([]map[string]any, 0) - hasMore := false - for page := 1; ; page++ { - if err := client.Query(ctx, &query, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to list labels", err), nil, nil - } - for _, labelNode := range query.Repository.Labels.Nodes { - labels = append(labels, map[string]any{ - "id": fmt.Sprintf("%v", labelNode.ID), - "name": string(labelNode.Name), - "color": string(labelNode.Color), - "description": string(labelNode.Description), - }) - } - if !query.Repository.Labels.PageInfo.HasNextPage { - break - } - if page >= listLabelsMaxPages { - hasMore = true - break + if err := client.Query(ctx, &query, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to list labels", err), nil, nil + } + + labels := make([]map[string]any, len(query.Repository.Labels.Nodes)) + for i, labelNode := range query.Repository.Labels.Nodes { + labels[i] = map[string]any{ + "id": fmt.Sprintf("%v", labelNode.ID), + "name": string(labelNode.Name), + "color": string(labelNode.Color), + "description": string(labelNode.Description), } - vars["cursor"] = githubv4.NewString(query.Repository.Labels.PageInfo.EndCursor) } + labelsPage := query.Repository.Labels.PageInfo response := map[string]any{ "labels": labels, "totalCount": int(query.Repository.Labels.TotalCount), - "has_more": hasMore, + "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 0a8974a762..43075659bc 100644 --- a/pkg/github/labels_test.go +++ b/pkg/github/labels_test.go @@ -157,9 +157,7 @@ func TestListLabels(t *testing.T) { mockedClient *http.Client expectToolError bool expectedToolErrMsg string - expectedNames []string - expectedCount int - expectHasMore bool + expectedPageInfo MinimalPageInfo }{ { name: "successful repository labels listing", @@ -167,82 +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 - PageInfo struct { - HasNextPage githubv4.Boolean - EndCursor githubv4.String - } - } `graphql:"labels(first: 100, after: $cursor, orderBy: {field: $orderByField, direction: $orderByDirection})"` - } `graphql:"repository(owner: $owner, name: $repo)"` - }{}, - map[string]any{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "cursor": (*githubv4.String)(nil), - "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), - "pageInfo": map[string]any{ - "hasNextPage": githubv4.Boolean(false), - "endCursor": githubv4.String(""), - }, - }, - }, - }), - ), - ), - expectToolError: false, - expectedNames: []string{"bug", "enhancement"}, + 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: "follows cursors so the least-used labels are not dropped", + name: "reports the next page when the repository has more labels", requestArgs: map[string]any{ - "owner": "owner", - "repo": "repo", + "owner": "owner", + "repo": "repo", + "perPage": float64(2), }, - mockedClient: twoPageLabelsClient(t), - expectToolError: false, - expectedNames: []string{"bug", "rarely-used"}, + 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: "stops at the page cap and sets has_more", + name: "passes the after cursor to fetch the next page", requestArgs: map[string]any{ "owner": "owner", "repo": "repo", + "after": "c2", }, - mockedClient: alwaysHasNextPageLabelsClient(t), - expectToolError: false, - expectedCount: listLabelsMaxPages, - expectHasMore: true, + 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", }, } @@ -268,60 +241,59 @@ func TestListLabels(t *testing.T) { } } else { assert.False(t, result.IsError) - var response struct { - Labels []struct { - Name string `json:"name"` - } `json:"labels"` - HasMore bool `json:"has_more"` - } + var response ListLabelsOutput require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) - names := make([]string, len(response.Labels)) - for i, label := range response.Labels { - names[i] = label.Name - } - if tc.expectedNames != nil { - assert.Equal(t, tc.expectedNames, names) - } - if tc.expectedCount != 0 { - assert.Len(t, names, tc.expectedCount) - } - assert.Equal(t, tc.expectHasMore, response.HasMore) + assert.Len(t, response.Labels, 2) + assert.Equal(t, 5, response.TotalCount) + assert.Equal(t, tc.expectedPageInfo, response.PageInfo) } }) } } -// twoPageLabelsClient serves a first page that reports another page, then a second page reached only through its cursor. -func twoPageLabelsClient(t *testing.T) *http.Client { - t.Helper() - mux := http.NewServeMux() - mux.HandleFunc("/graphql", func(w http.ResponseWriter, r *http.Request) { - var req struct { - Variables map[string]any `json:"variables"` - } - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - name, hasNext := "bug", true - if req.Variables["cursor"] == "page-2" { - name, hasNext = "rarely-used", false - } - resp := map[string]any{ - "data": map[string]any{ - "repository": map[string]any{ - "labels": map[string]any{ - "nodes": []any{map[string]any{"id": name, "name": name, "color": "ededed", "description": ""}}, - "totalCount": 2, - "pageInfo": map[string]any{"hasNextPage": hasNext, "endCursor": "page-2"}, +// 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, }, }, - } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(resp) - }) - return &http.Client{Transport: recorderTransport{handler: mux}} + }), + ) } func TestWriteLabel(t *testing.T) { diff --git a/pkg/github/typed_governance_outputs.go b/pkg/github/typed_governance_outputs.go index 8e79e1398d..02952bac0b 100644 --- a/pkg/github/typed_governance_outputs.go +++ b/pkg/github/typed_governance_outputs.go @@ -20,9 +20,9 @@ type MinimalLabel struct { } type ListLabelsOutput struct { - Labels []MinimalLabel `json:"labels"` - TotalCount int `json:"totalCount" jsonschema:"Total labels in the repository."` - HasMore bool `json:"has_more" jsonschema:"Whether the repository has more labels than were returned."` + 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 f1822e3765..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,"has_more":false}`, 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,"has_more":false}`}, + {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"}, From 9b15d49b0e53964157215a1058a0cbb51f0396dd Mon Sep 17 00:00:00 2001 From: Andrew Georgiou Date: Thu, 15 Oct 2026 16:25:02 +0100 Subject: [PATCH 3/3] Match list_issues wording in the list_label description Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d8a9000c-ebe8-4118-ae89-a942861e4166 --- pkg/github/__toolsnaps__/list_label.snap | 2 +- pkg/github/labels.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/__toolsnaps__/list_label.snap b/pkg/github/__toolsnaps__/list_label.snap index baebe1a204..80e898b71b 100644 --- a/pkg/github/__toolsnaps__/list_label.snap +++ b/pkg/github/__toolsnaps__/list_label.snap @@ -4,7 +4,7 @@ "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. Returns up to 100 labels per page by default. If 'pageInfo.hasNextPage' is true, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter to get the rest. Use get_label to check a specific label by name.", + "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": { diff --git a/pkg/github/labels.go b/pkg/github/labels.go index 55d35e3b27..c9479e5062 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -145,7 +145,7 @@ 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. Returns up to 100 labels per page by default. If 'pageInfo.hasNextPage' is true, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter to get the rest. Use get_label to check a specific label by name."), + 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,