Skip to content

Commit c042498

Browse files
harish-gaggarharish-gaggar
committed
fix(params): accept native integer types for numeric tool parameters
MCP clients sometimes send JSON integers as int or json.Number rather than float64, which caused RequiredInt to reject valid issue_number values before any GitHub API call. Fixes #2807
1 parent 71ef826 commit c042498

3 files changed

Lines changed: 212 additions & 37 deletions

File tree

‎pkg/github/issues_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,31 @@ func Test_GetIssue(t *testing.T) {
200200
},
201201
expectedIssue: mockIssue,
202202
},
203+
{
204+
name: "successful issue retrieval with int issue_number",
205+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
206+
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue),
207+
}),
208+
requestArgs: map[string]any{
209+
"method": "get",
210+
"owner": "owner2",
211+
"repo": "repo2",
212+
"issue_number": int(42),
213+
},
214+
expectedIssue: mockIssue,
215+
},
216+
{
217+
name: "invalid issue_number does not call API",
218+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
219+
requestArgs: map[string]any{
220+
"method": "get",
221+
"owner": "owner2",
222+
"repo": "repo2",
223+
"issue_number": "not-a-number",
224+
},
225+
expectResultError: true,
226+
expectedErrMsg: "not a valid number",
227+
},
203228
{
204229
name: "issue not found",
205230
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{

‎pkg/github/params.go‎

Lines changed: 112 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package github
22

33
import (
4+
"encoding/json"
45
"errors"
56
"fmt"
67
"math"
@@ -60,64 +61,138 @@ func isAcceptedError(err error) bool {
6061
return errors.As(err, &acceptedError)
6162
}
6263

63-
// toInt converts a value to int, handling both float64 and string representations.
64-
// Some MCP clients send numeric values as strings. It rejects NaN, ±Inf,
65-
// fractional values, and values outside the int range.
66-
func toInt(val any) (int, error) {
67-
var f float64
68-
switch v := val.(type) {
69-
case float64:
70-
f = v
71-
case string:
72-
var err error
73-
f, err = strconv.ParseFloat(v, 64)
74-
if err != nil {
75-
return 0, fmt.Errorf("invalid numeric value: %s", v)
76-
}
77-
default:
78-
return 0, fmt.Errorf("expected number, got %T", val)
64+
// float64ToInt64 converts a float64 to int64, rejecting non-finite, fractional,
65+
// and out-of-range values, including those that would lose precision.
66+
func float64ToInt64(f float64) (int64, error) {
67+
if math.IsNaN(f) || math.IsInf(f, 0) {
68+
return 0, fmt.Errorf("non-finite numeric value")
69+
}
70+
if f != math.Trunc(f) {
71+
return 0, fmt.Errorf("non-integer numeric value: %v", f)
72+
}
73+
if f > float64(math.MaxInt64) || f < float64(math.MinInt64) {
74+
return 0, fmt.Errorf("numeric value %v is too large to fit in int64", f)
7975
}
76+
result := int64(f)
77+
if float64(result) != f {
78+
return 0, fmt.Errorf("numeric value %v is too large to fit in int64", f)
79+
}
80+
return result, nil
81+
}
82+
83+
// float64ToInt converts a float64 to int, rejecting non-finite, fractional,
84+
// and out-of-range values.
85+
func float64ToInt(f float64) (int, error) {
8086
if math.IsNaN(f) || math.IsInf(f, 0) {
8187
return 0, fmt.Errorf("non-finite numeric value")
8288
}
8389
if f != math.Trunc(f) {
8490
return 0, fmt.Errorf("non-integer numeric value: %v", f)
8591
}
86-
if f > math.MaxInt || f < math.MinInt {
92+
if f > float64(math.MaxInt) || f < float64(math.MinInt) {
8793
return 0, fmt.Errorf("numeric value out of int range: %v", f)
8894
}
8995
return int(f), nil
9096
}
9197

92-
// toInt64 converts a value to int64, handling both float64 and string representations.
93-
// Some MCP clients send numeric values as strings. It rejects NaN, ±Inf,
94-
// fractional values, and values that lose precision in the float64→int64 conversion.
95-
func toInt64(val any) (int64, error) {
96-
var f float64
98+
// toInt converts a value to int, handling float64, integer, and string representations.
99+
// Native integer types are preserved without a float64 round-trip so large values
100+
// are not corrupted. It rejects NaN, ±Inf, fractional values, and out-of-range values.
101+
func toInt(val any) (int, error) {
97102
switch v := val.(type) {
103+
case int:
104+
return v, nil
105+
case int32:
106+
return int(v), nil
107+
case int64:
108+
if v > int64(math.MaxInt) || v < int64(math.MinInt) {
109+
return 0, fmt.Errorf("numeric value out of int range: %v", v)
110+
}
111+
return int(v), nil
112+
case uint:
113+
if v > uint(math.MaxInt) {
114+
return 0, fmt.Errorf("numeric value out of int range: %v", v)
115+
}
116+
return int(v), nil
117+
case uint32:
118+
return int(v), nil
119+
case uint64:
120+
if v > uint64(math.MaxInt) {
121+
return 0, fmt.Errorf("numeric value out of int range: %v", v)
122+
}
123+
return int(v), nil
98124
case float64:
99-
f = v
125+
return float64ToInt(v)
126+
case float32:
127+
return float64ToInt(float64(v))
100128
case string:
101-
var err error
102-
f, err = strconv.ParseFloat(v, 64)
129+
i, err := strconv.ParseInt(v, 10, 0)
103130
if err != nil {
104131
return 0, fmt.Errorf("invalid numeric value: %s", v)
105132
}
133+
return int(i), nil
134+
case json.Number:
135+
if i, err := v.Int64(); err == nil {
136+
if i > int64(math.MaxInt) || i < int64(math.MinInt) {
137+
return 0, fmt.Errorf("numeric value out of int range: %v", i)
138+
}
139+
return int(i), nil
140+
}
141+
f, err := v.Float64()
142+
if err != nil {
143+
return 0, fmt.Errorf("invalid numeric value: %s", v)
144+
}
145+
return float64ToInt(f)
106146
default:
107147
return 0, fmt.Errorf("expected number, got %T", val)
108148
}
109-
if math.IsNaN(f) || math.IsInf(f, 0) {
110-
return 0, fmt.Errorf("non-finite numeric value")
111-
}
112-
if f != math.Trunc(f) {
113-
return 0, fmt.Errorf("non-integer numeric value: %v", f)
114-
}
115-
result := int64(f)
116-
// Check round-trip to detect precision loss for large int64 values
117-
if float64(result) != f {
118-
return 0, fmt.Errorf("numeric value %v is too large to fit in int64", f)
149+
}
150+
151+
// toInt64 converts a value to int64, handling float64, integer, and string representations.
152+
// Native integer types are preserved without a float64 round-trip so large values
153+
// are not corrupted. It rejects NaN, ±Inf, fractional values, and out-of-range values.
154+
func toInt64(val any) (int64, error) {
155+
switch v := val.(type) {
156+
case int:
157+
return int64(v), nil
158+
case int32:
159+
return int64(v), nil
160+
case int64:
161+
return v, nil
162+
case uint:
163+
if uint64(v) > uint64(math.MaxInt64) {
164+
return 0, fmt.Errorf("numeric value out of int64 range: %v", v)
165+
}
166+
return int64(v), nil
167+
case uint32:
168+
return int64(v), nil
169+
case uint64:
170+
if v > uint64(math.MaxInt64) {
171+
return 0, fmt.Errorf("numeric value out of int64 range: %v", v)
172+
}
173+
return int64(v), nil
174+
case float64:
175+
return float64ToInt64(v)
176+
case float32:
177+
return float64ToInt64(float64(v))
178+
case string:
179+
i, err := strconv.ParseInt(v, 10, 64)
180+
if err != nil {
181+
return 0, fmt.Errorf("invalid numeric value: %s", v)
182+
}
183+
return i, nil
184+
case json.Number:
185+
if i, err := v.Int64(); err == nil {
186+
return i, nil
187+
}
188+
f, err := v.Float64()
189+
if err != nil {
190+
return 0, fmt.Errorf("invalid numeric value: %s", v)
191+
}
192+
return float64ToInt64(f)
193+
default:
194+
return 0, fmt.Errorf("expected number, got %T", val)
119195
}
120-
return result, nil
121196
}
122197

123198
// RequiredParam is a helper function that can be used to fetch a requested parameter from the request.
@@ -149,7 +224,7 @@ func RequiredParam[T comparable](args map[string]any, p string) (T, error) {
149224
// RequiredInt is a helper function that can be used to fetch a requested parameter from the request.
150225
// It does the following checks:
151226
// 1. Checks if the parameter is present in the request.
152-
// 2. Checks if the parameter is of the expected type (float64 or numeric string).
227+
// 2. Checks if the parameter is a valid integer (float64, native integer, or numeric string).
153228
// 3. Checks if the parameter is not empty, i.e: non-zero value
154229
func RequiredInt(args map[string]any, p string) (int, error) {
155230
v, ok := args[p]

‎pkg/github/params_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package github
22

33
import (
4+
"encoding/json"
45
"fmt"
56
"math"
67
"testing"
@@ -206,6 +207,34 @@ func Test_RequiredInt(t *testing.T) {
206207
expected: 42,
207208
expectError: false,
208209
},
210+
{
211+
name: "valid int parameter",
212+
params: map[string]any{"count": int(42)},
213+
paramName: "count",
214+
expected: 42,
215+
expectError: false,
216+
},
217+
{
218+
name: "valid int64 parameter",
219+
params: map[string]any{"count": int64(42)},
220+
paramName: "count",
221+
expected: 42,
222+
expectError: false,
223+
},
224+
{
225+
name: "valid json.Number parameter",
226+
params: map[string]any{"count": json.Number("42")},
227+
paramName: "count",
228+
expected: 42,
229+
expectError: false,
230+
},
231+
{
232+
name: "valid uint parameter",
233+
params: map[string]any{"count": uint(42)},
234+
paramName: "count",
235+
expected: 42,
236+
expectError: false,
237+
},
209238
{
210239
name: "missing parameter",
211240
params: map[string]any{},
@@ -305,6 +334,52 @@ func Test_RequiredInt(t *testing.T) {
305334
})
306335
}
307336
}
337+
338+
func Test_RequiredBigInt(t *testing.T) {
339+
tests := []struct {
340+
name string
341+
params map[string]any
342+
paramName string
343+
expected int64
344+
expectError bool
345+
}{
346+
{
347+
name: "valid int64 parameter without float64 precision loss",
348+
params: map[string]any{"count": int64(9007199254740993)},
349+
paramName: "count",
350+
expected: 9007199254740993,
351+
expectError: false,
352+
},
353+
{
354+
name: "valid json.Number parameter",
355+
params: map[string]any{"count": json.Number("9007199254740993")},
356+
paramName: "count",
357+
expected: 9007199254740993,
358+
expectError: false,
359+
},
360+
{
361+
name: "valid uint64 parameter",
362+
params: map[string]any{"count": uint64(42)},
363+
paramName: "count",
364+
expected: 42,
365+
expectError: false,
366+
},
367+
}
368+
369+
for _, tc := range tests {
370+
t.Run(tc.name, func(t *testing.T) {
371+
result, err := RequiredBigInt(tc.params, tc.paramName)
372+
373+
if tc.expectError {
374+
assert.Error(t, err)
375+
} else {
376+
assert.NoError(t, err)
377+
assert.Equal(t, tc.expected, result)
378+
}
379+
})
380+
}
381+
}
382+
308383
func Test_OptionalIntParam(t *testing.T) {
309384
tests := []struct {
310385
name string

0 commit comments

Comments
 (0)