Skip to content

Commit 2c4b2a3

Browse files
committed
fix(projects): spell the page size the way the rest of the server does
projects_list advertised per_page while every other paginated tool advertises perPage. The handlers read per_page, so it worked, but it left one tool spelling pagination differently from the other 30 and that is how actions_list ended up advertising a name nothing read. The schema now says perPage. per_page is still read when perPage is absent: the projects tools have advertised it since September 2025 and clients sending it get the size they ask for today. TestAllToolInputSchemasUseCanonicalPaginationNames walks the whole tool inventory and rejects case and underscore variants of page, perPage, after and before. On main it fails on actions_list and projects_list.
1 parent 7942bc9 commit 2c4b2a3

5 files changed

Lines changed: 98 additions & 6 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1092,7 +1092,7 @@ The following sets of tools are available:
10921092
- `method`: The action to perform (string, required)
10931093
- `owner`: The owner (user or organization login). The name is not case sensitive. (string, required)
10941094
- `owner_type`: Owner type (user or org). If not provided, will automatically try both. (string, optional)
1095-
- `per_page`: Results per page (max 50) (number, optional)
1095+
- `perPage`: Results per page (max 50) (number, optional)
10961096
- `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)
10971097
- `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)
10981098

pkg/github/__toolsnaps__/projects_list.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
],
5353
"type": "string"
5454
},
55-
"per_page": {
55+
"perPage": {
5656
"description": "Results per page (max 50)",
5757
"type": "number"
5858
},

pkg/github/projects.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ Use this tool to list projects for a user or organization, or list project field
341341
Type: "string",
342342
},
343343
},
344-
"per_page": {
344+
"perPage": {
345345
Type: "number",
346346
Description: fmt.Sprintf("Results per page (max %d)", MaxProjectsPerPage),
347347
},
@@ -1783,7 +1783,7 @@ func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, a
17831783
return utils.NewToolResultError(err.Error()), false, nil, nil
17841784
}
17851785

1786-
perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage)
1786+
perPage, err := optionalProjectsPerPage(args)
17871787
if err != nil {
17881788
return utils.NewToolResultError(err.Error()), false, nil, nil
17891789
}
@@ -1940,7 +1940,7 @@ func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[
19401940
if err != nil {
19411941
return utils.NewToolResultError(err.Error()), false, nil, nil
19421942
}
1943-
perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage)
1943+
perPage, err := optionalProjectsPerPage(args)
19441944
if err != nil {
19451945
return utils.NewToolResultError(err.Error()), false, nil, nil
19461946
}
@@ -2512,8 +2512,23 @@ func invalidIssueFieldValue(field *ResolvedField, hint string) error {
25122512
)
25132513
}
25142514

2515+
// optionalProjectsPerPage reads the page size for the projects tools.
2516+
//
2517+
// The schema advertises perPage, the name every other paginated tool uses. The
2518+
// projects tools advertised per_page from September 2025 until this change and
2519+
// clients sending it get the size they asked for today, so it is still read when
2520+
// perPage is absent.
2521+
func optionalProjectsPerPage(args map[string]any) (int, error) {
2522+
if _, ok := args["perPage"]; !ok {
2523+
if _, legacy := args["per_page"]; legacy {
2524+
return OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage)
2525+
}
2526+
}
2527+
return OptionalIntParamWithDefault(args, "perPage", MaxProjectsPerPage)
2528+
}
2529+
25152530
func extractPaginationOptionsFromArgs(args map[string]any) (github.ListProjectsPaginationOptions, error) {
2516-
perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage)
2531+
perPage, err := optionalProjectsPerPage(args)
25172532
if err != nil {
25182533
return github.ListProjectsPaginationOptions{}, err
25192534
}

pkg/github/projects_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,43 @@ func Test_ProjectsList_ListProjectItems(t *testing.T) {
395395
})
396396
}
397397

398+
func Test_optionalProjectsPerPage(t *testing.T) {
399+
tests := []struct {
400+
name string
401+
args map[string]any
402+
want int
403+
}{
404+
{
405+
name: "canonical perPage",
406+
args: map[string]any{"perPage": float64(10)},
407+
want: 10,
408+
},
409+
{
410+
name: "per_page still read for clients on the previous name",
411+
args: map[string]any{"per_page": float64(10)},
412+
want: 10,
413+
},
414+
{
415+
name: "perPage wins when both are sent",
416+
args: map[string]any{"perPage": float64(10), "per_page": float64(25)},
417+
want: 10,
418+
},
419+
{
420+
name: "neither sent falls back to the maximum",
421+
args: map[string]any{},
422+
want: MaxProjectsPerPage,
423+
},
424+
}
425+
426+
for _, tc := range tests {
427+
t.Run(tc.name, func(t *testing.T) {
428+
got, err := optionalProjectsPerPage(tc.args)
429+
require.NoError(t, err)
430+
assert.Equal(t, tc.want, got)
431+
})
432+
}
433+
}
434+
398435
func Test_detectOwnerType(t *testing.T) {
399436
t.Run("uses organization account type", func(t *testing.T) {
400437
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{

pkg/github/tools_validation_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,46 @@ func TestAllToolInputSchemasAvoidTopLevelCombinators(t *testing.T) {
7171
}
7272
}
7373

74+
// TestAllToolInputSchemasUseCanonicalPaginationNames keeps pagination properties
75+
// spelled one way across the whole inventory. The pagination helpers read page,
76+
// perPage, after and before, so a schema that advertises a case or underscore
77+
// variant of one of those names promises a knob the handler never turns: whatever
78+
// the client sends is dropped and the default is used instead. actions_list
79+
// advertised per_page for months that way.
80+
func TestAllToolInputSchemasUseCanonicalPaginationNames(t *testing.T) {
81+
canonical := map[string]string{
82+
"page": "page",
83+
"perpage": "perPage",
84+
"after": "after",
85+
"before": "before",
86+
}
87+
88+
tools := AllTools(stubTranslation)
89+
require.NotEmpty(t, tools, "AllTools should return at least one tool")
90+
91+
for _, serverTool := range tools {
92+
tool := serverTool.Tool
93+
t.Run(tool.Name, func(t *testing.T) {
94+
data, err := json.Marshal(tool.InputSchema)
95+
require.NoError(t, err, "Tool %q InputSchema must marshal", tool.Name)
96+
97+
var schema struct {
98+
Properties map[string]json.RawMessage `json:"properties"`
99+
}
100+
require.NoError(t, json.Unmarshal(data, &schema), "Tool %q InputSchema must be a JSON object", tool.Name)
101+
102+
for name := range schema.Properties {
103+
want, ok := canonical[strings.ToLower(strings.ReplaceAll(name, "_", ""))]
104+
if !ok {
105+
continue
106+
}
107+
assert.Equal(t, want, name,
108+
"Tool %q advertises pagination property %q; the canonical spelling is %q", tool.Name, name, want)
109+
}
110+
})
111+
}
112+
}
113+
74114
// TestAllResourcesHaveRequiredMetadata validates that all resources have mandatory metadata
75115
func TestAllResourcesHaveRequiredMetadata(t *testing.T) {
76116
// Resources are now stateless - no client functions needed

0 commit comments

Comments
 (0)