fix(issues): accept native integer types for issue_number parameters - #2808
fix(issues): accept native integer types for issue_number parameters#2808syf2211 wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
Nice fix, this addresses the native int rejection. One thing I’d consider tightening before merge: the new numeric path accepts int64/uint64/json.Number, but converts them through float64. That can lose precision for large integer values. Would it be worth preserving native integer types through validation and only using float parsing for actual float/string inputs? It might also be useful to add regression coverage for all affected issue tools: issue_read, issue_write, and add_issue_comment, plus a no-API-call assertion for invalid issue_number on write tools.🥀 🥀 🥀 🥀 |
Sorry, something went wrong.
|
Thanks @e345ee — addressed in d15c2fb:
go test ./pkg/github/ -run "Test_RequiredInt|Test_RequiredBigInt|Test_GetIssue|Test_AddIssueComment|Test_UpdateIssue" passes locally. |
Sorry, something went wrong.
Address review feedback on github#2808: - Handle int/int64/uint/json.Number without float64 round-trip - Add RequiredBigInt regression tests for large int64 values - Add issue_read/issue_write/add_issue_comment tests for int issue_number - Assert invalid issue_number fails before API calls on write tools
Rebase github#2808 onto upstream/main after hierarchy enrichment landed. Preserve int/int64/json.Number coercion without float64 round-trip and add regression tests for issue_read, issue_write, and add_issue_comment. Fixes github#2807
|
Rebased onto latest main (includes issue_read hierarchy enrichment) and addressed the float64 round-trip feedback in 505c6ec. go test ./pkg/github/ passes locally. |
Sorry, something went wrong.
Rebase github#2808 onto upstream/main after hierarchy enrichment landed. Preserve int/int64/json.Number coercion without float64 round-trip and add regression tests for issue_read, issue_write, and add_issue_comment. Fixes github#2807
|
Rebased onto latest main. go test ./pkg/github/... -short passes locally. |
Sorry, something went wrong.
Summary
Extend RequiredInt / toInt coercion to accept native Go integer types (int, int64, etc.) and json.Number, not only float64 and numeric strings. Also restore missing deprecated aliases for the issues consolidation rename.
Motivation
Fixes #2807. Some MCP clients (including mcpcurl when schema fields use integer, and other providers that construct argument maps programmatically) pass issue_number as a native integer. RequiredInt previously rejected these with expected number, got int before any GitHub API call was made.
Additionally, get_issue and update_issue were renamed to issue_read and issue_write in #1211 but aliases were never added to DeprecatedToolAliases, breaking users still referencing the old tool names.
Changes
Tests
All tests pass.
Notes