feat: accept string or number for index parameters (#121)

This change makes index parameters more flexible by accepting both
numeric and string values. LLM agents often pass issue/PR indices
as strings (e.g., "123") since they appear as string identifiers in
URLs and CLI contexts. The implementation:

- Created pkg/params package with GetIndex() helper function
- Updated 25+ tool functions across issue, pull, label, and timetracking operations
- Improved error messages to say "must be a valid integer" instead of misleading "is required"
- Added comprehensive tests for both numeric and string inputs

This improves UX for MCP clients and LLMs while maintaining backward
compatibility with existing numeric callers.

Fixes: #121

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
James Pharaoh
2026-02-10 09:23:45 +00:00
parent 1f7392305f
commit 71dbc9d6da
6 changed files with 288 additions and 135 deletions

33
pkg/params/params.go Normal file
View File

@@ -0,0 +1,33 @@
package params
import (
"fmt"
"strconv"
)
// GetIndex extracts an index parameter from MCP tool arguments.
// It accepts both numeric (float64 from JSON) and string representations.
// This provides better UX for LLM callers that may naturally use strings
// for identifiers like issue/PR numbers.
func GetIndex(args map[string]interface{}, key string) (int64, error) {
val, exists := args[key]
if !exists {
return 0, fmt.Errorf("%s is required", key)
}
// Try float64 (JSON number type)
if f, ok := val.(float64); ok {
return int64(f), nil
}
// Try string and parse to integer
if s, ok := val.(string); ok {
i, err := strconv.ParseInt(s, 10, 64)
if err != nil {
return 0, fmt.Errorf("%s must be a valid integer (got %q)", key, s)
}
return i, nil
}
return 0, fmt.Errorf("%s must be a number or numeric string", key)
}

116
pkg/params/params_test.go Normal file
View File

@@ -0,0 +1,116 @@
package params
import (
"testing"
)
func TestGetIndex(t *testing.T) {
tests := []struct {
name string
args map[string]interface{}
key string
wantIndex int64
wantErr bool
errMsg string
}{
{
name: "valid float64",
args: map[string]interface{}{"index": float64(123)},
key: "index",
wantIndex: 123,
wantErr: false,
},
{
name: "valid string",
args: map[string]interface{}{"index": "456"},
key: "index",
wantIndex: 456,
wantErr: false,
},
{
name: "valid string with large number",
args: map[string]interface{}{"index": "999999"},
key: "index",
wantIndex: 999999,
wantErr: false,
},
{
name: "missing parameter",
args: map[string]interface{}{},
key: "index",
wantErr: true,
errMsg: "index is required",
},
{
name: "invalid string (not a number)",
args: map[string]interface{}{"index": "abc"},
key: "index",
wantErr: true,
errMsg: "must be a valid integer",
},
{
name: "invalid string (decimal)",
args: map[string]interface{}{"index": "12.34"},
key: "index",
wantErr: true,
errMsg: "must be a valid integer",
},
{
name: "invalid type (bool)",
args: map[string]interface{}{"index": true},
key: "index",
wantErr: true,
errMsg: "must be a number or numeric string",
},
{
name: "invalid type (map)",
args: map[string]interface{}{"index": map[string]string{"foo": "bar"}},
key: "index",
wantErr: true,
errMsg: "must be a number or numeric string",
},
{
name: "custom key name",
args: map[string]interface{}{"pr_index": "789"},
key: "pr_index",
wantIndex: 789,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotIndex, err := GetIndex(tt.args, tt.key)
if tt.wantErr {
if err == nil {
t.Errorf("GetIndex() expected error but got nil")
return
}
if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
t.Errorf("GetIndex() error = %v, want error containing %q", err, tt.errMsg)
}
return
}
if err != nil {
t.Errorf("GetIndex() unexpected error = %v", err)
return
}
if gotIndex != tt.wantIndex {
t.Errorf("GetIndex() = %v, want %v", gotIndex, tt.wantIndex)
}
})
}
}
func contains(s, substr string) bool {
return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || containsMiddle(s, substr)))
}
func containsMiddle(s, substr string) bool {
for i := 0; i <= len(s)-len(substr); i++ {
if s[i:i+len(substr)] == substr {
return true
}
}
return false
}