[vshalpnjabi]

Summary

list_label now rejects perPage values outside 1..100 instead of passing 0 through.

Why

The advertised schema says minimum: 1, but the runtime input schema doesn't enforce it, so perPage: 0 reached the handler and was silently replaced with the default page size.
Fixes #3647

What changed

• listLabelsLegacy reads perPage only when it's present, so an explicit 0 is rejected instead of falling back to the default. Out-of-range values return perPage value N is below minimum of 1 or perPage value N exceeds maximum of 100 from ToGraphQLParams. The default still applies when it's omitted.
• Added a perPage: 0 case to the list_label handler tests.
• Added CursorPaginationParams.ToGraphQLParams tests for 1, 100, 0, -1 and 101 that check the exact error messages, plus a case with an After cursor. The coverage is moved over from Fix #3647: reject perPage values below 1 in list_label #3659.

MCP impact

• Tool schema or behavior changed
  list_label returns a tool error for perPage 0 or below, matching the schema it already advertises. The schema is unchanged.

Prompts tested (tool changes only)

• Not tested with a live client. Covered by the handler and unit tests above.

Security / limits

• No security or limits impact

Tool renaming

• I am not renaming tools as part of this PR

Lint & tests

• Linted locally with ./script/lint
• Tested locally with ./script/test

Also ran go test -v ./..., UPDATE_TOOLSNAPS=true go test ./... (no snapshot changes) and script/generate-docs (no diff).

Docs

• Not needed

[SamMorrowDrums]

Thanks — this is the earlier of the two equivalent fixes for #3647. Please add focused boundary tests for CursorPaginationParams.ToGraphQLParams (1, 100, 0, negative, and above 100); with that coverage, this is the PR I would prioritize for review. #3659 currently carries the extra shared-helper tests.

[vshalpnjabi]

Added boundary tests for CursorPaginationParams.ToGraphQLParams covering 1, 100, 0, a negative value, and 101. go test ./pkg/github/... passes locally.

[Copilot]

🟡 Changes recommended

The implementation and tests do not consistently satisfy the documented unified perPage error contract, and the claimed registered-tool coverage is missing.

3 open issues

• Use the documented perPage range error message · New
• Add a registered-tool test for perPage zero · New
• Align perPage validation errors with the documented contract · New

What changed in this PR

This pull request updates list_label pagination to distinguish omitted perPage values from explicit invalid values and enforce the advertised 1..100 range.

Changes:

• Rejects perPage values below 1 or above 100.
• Preserves the default page size when perPage is omitted.
• Adds boundary and handler coverage for invalid page sizes.

File	Summary
pkg/​github/​params.go	Updates cursor-pagination validation to enforce the lower bound; review notes identify an error-message contract mismatch.
pkg/​github/​params_test.go	Adds boundary tests for cursor-pagination values.
pkg/​github/​labels.go	Preserves omitted-versus-explicit perPage behavior; review notes identify inconsistent range-error wording.
pkg/​github/​labels_test.go	Adds coverage for explicit perPage: 0; review notes that the described session-based registered-tool test is not present.

_{Review effort: Lite (auto)}

[vshalpnjabi]

@SamMorrowDrums I moved the ToGraphQLParams coverage from #3659 into this PR. The test now checks the exact error messages and a cursor (After) case. The branch is also up to date with main. Ready for another look.