diff --git a/README.md b/README.md index 145281bcdb..727ca2efd9 100644 --- a/README.md +++ b/README.md @@ -631,7 +631,7 @@ The following sets of tools are available: - `method`: The action to perform (string, required) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (default: 1) (number, optional) - - `per_page`: Results per page for pagination (default: 30, max: 100) (number, optional) + - `perPage`: Results per page for pagination (default: 30, max: 100) (number, optional) - `repo`: Repository name (string, required) - `resource_id`: The unique identifier of the resource. This will vary based on the "method" provided, so ensure you provide the correct ID: - Do not provide any resource ID for 'list_workflows' method. @@ -1092,7 +1092,7 @@ The following sets of tools are available: - `method`: The action to perform (string, required) - `owner`: The owner (user or organization login). The name is not case sensitive. (string, required) - `owner_type`: Owner type (user or org). If not provided, will automatically try both. (string, optional) - - `per_page`: Results per page (max 50) (number, optional) + - `perPage`: Results per page (max 50) (number, optional) - `project_number`: The project's number. Required for 'list_project_fields', 'list_project_items', 'list_project_views', and 'list_project_status_updates' methods. (number, optional) - `query`: Filter/query string. For list_projects: filter by title text and state (e.g. "roadmap is:open"). For list_project_items: advanced filtering using GitHub's project filtering syntax. (string, optional) diff --git a/pkg/github/__toolsnaps__/actions_list.snap b/pkg/github/__toolsnaps__/actions_list.snap index be97affbdb..4df628bf85 100644 --- a/pkg/github/__toolsnaps__/actions_list.snap +++ b/pkg/github/__toolsnaps__/actions_list.snap @@ -26,7 +26,7 @@ "minimum": 1, "type": "number" }, - "per_page": { + "perPage": { "description": "Results per page for pagination (default: 30, max: 100)", "maximum": 100, "minimum": 1, diff --git a/pkg/github/__toolsnaps__/projects_list.snap b/pkg/github/__toolsnaps__/projects_list.snap index 487119f04a..c64737900f 100644 --- a/pkg/github/__toolsnaps__/projects_list.snap +++ b/pkg/github/__toolsnaps__/projects_list.snap @@ -52,7 +52,7 @@ ], "type": "string" }, - "per_page": { + "perPage": { "description": "Results per page (max 50)", "type": "number" }, diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 667901e623..3f5654db10 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -313,7 +313,7 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an Description: "Page number for pagination (default: 1)", Minimum: jsonschema.Ptr(1.0), }, - "per_page": { + "perPage": { Type: "number", Description: "Results per page for pagination (default: 30, max: 100)", Minimum: jsonschema.Ptr(1.0), diff --git a/pkg/github/projects.go b/pkg/github/projects.go index df6d8ac190..fe702cfbbe 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -341,7 +341,7 @@ Use this tool to list projects for a user or organization, or list project field Type: "string", }, }, - "per_page": { + "perPage": { Type: "number", Description: fmt.Sprintf("Results per page (max %d)", MaxProjectsPerPage), }, @@ -1783,7 +1783,7 @@ func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, a return utils.NewToolResultError(err.Error()), false, nil, nil } - perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage) + perPage, err := optionalProjectsPerPage(args) if err != nil { return utils.NewToolResultError(err.Error()), false, nil, nil } @@ -1940,7 +1940,7 @@ func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[ if err != nil { return utils.NewToolResultError(err.Error()), false, nil, nil } - perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage) + perPage, err := optionalProjectsPerPage(args) if err != nil { return utils.NewToolResultError(err.Error()), false, nil, nil } @@ -2512,8 +2512,23 @@ func invalidIssueFieldValue(field *ResolvedField, hint string) error { ) } +// optionalProjectsPerPage reads the page size for the projects tools. +// +// The schema advertises perPage, the name every other paginated tool uses. The +// projects tools advertised per_page from September 2025 until this change and +// clients sending it get the size they asked for today, so it is still read when +// perPage is absent. +func optionalProjectsPerPage(args map[string]any) (int, error) { + if _, ok := args["perPage"]; !ok { + if _, legacy := args["per_page"]; legacy { + return OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage) + } + } + return OptionalIntParamWithDefault(args, "perPage", MaxProjectsPerPage) +} + func extractPaginationOptionsFromArgs(args map[string]any) (github.ListProjectsPaginationOptions, error) { - perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage) + perPage, err := optionalProjectsPerPage(args) if err != nil { return github.ListProjectsPaginationOptions{}, err } diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index e7d67f5264..8b40a569e0 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -395,6 +395,43 @@ func Test_ProjectsList_ListProjectItems(t *testing.T) { }) } +func Test_optionalProjectsPerPage(t *testing.T) { + tests := []struct { + name string + args map[string]any + want int + }{ + { + name: "canonical perPage", + args: map[string]any{"perPage": float64(10)}, + want: 10, + }, + { + name: "per_page still read for clients on the previous name", + args: map[string]any{"per_page": float64(10)}, + want: 10, + }, + { + name: "perPage wins when both are sent", + args: map[string]any{"perPage": float64(10), "per_page": float64(25)}, + want: 10, + }, + { + name: "neither sent falls back to the maximum", + args: map[string]any{}, + want: MaxProjectsPerPage, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := optionalProjectsPerPage(tc.args) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + func Test_detectOwnerType(t *testing.T) { t.Run("uses organization account type", func(t *testing.T) { mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ diff --git a/pkg/github/tools_validation_test.go b/pkg/github/tools_validation_test.go index dac04b5d54..cdc12348ae 100644 --- a/pkg/github/tools_validation_test.go +++ b/pkg/github/tools_validation_test.go @@ -71,6 +71,46 @@ func TestAllToolInputSchemasAvoidTopLevelCombinators(t *testing.T) { } } +// TestAllToolInputSchemasUseCanonicalPaginationNames keeps pagination properties +// spelled one way across the whole inventory. The pagination helpers read page, +// perPage, after and before, so a schema that advertises a case or underscore +// variant of one of those names promises a knob the handler never turns: whatever +// the client sends is dropped and the default is used instead. actions_list +// advertised per_page for months that way. +func TestAllToolInputSchemasUseCanonicalPaginationNames(t *testing.T) { + canonical := map[string]string{ + "page": "page", + "perpage": "perPage", + "after": "after", + "before": "before", + } + + tools := AllTools(stubTranslation) + require.NotEmpty(t, tools, "AllTools should return at least one tool") + + for _, serverTool := range tools { + tool := serverTool.Tool + t.Run(tool.Name, func(t *testing.T) { + data, err := json.Marshal(tool.InputSchema) + require.NoError(t, err, "Tool %q InputSchema must marshal", tool.Name) + + var schema struct { + Properties map[string]json.RawMessage `json:"properties"` + } + require.NoError(t, json.Unmarshal(data, &schema), "Tool %q InputSchema must be a JSON object", tool.Name) + + for name := range schema.Properties { + want, ok := canonical[strings.ToLower(strings.ReplaceAll(name, "_", ""))] + if !ok { + continue + } + assert.Equal(t, want, name, + "Tool %q advertises pagination property %q; the canonical spelling is %q", tool.Name, name, want) + } + }) + } +} + // TestAllResourcesHaveRequiredMetadata validates that all resources have mandatory metadata func TestAllResourcesHaveRequiredMetadata(t *testing.T) { // Resources are now stateless - no client functions needed