diff --git a/pkg/logs/tools.go b/pkg/logs/tools.go index 0f86cee..fe47b4b 100644 --- a/pkg/logs/tools.go +++ b/pkg/logs/tools.go @@ -3,6 +3,8 @@ package logs import ( "context" "encoding/json" + "fmt" + "math" "time" "github.com/mark3labs/mcp-go/mcp" @@ -201,18 +203,16 @@ func listLogs(logRepo *LogRepo) server.ServerTool { llParams.EndTime = &endTimeParam } - if direction, ok, err := validate.OptionalToolParam[string](request, "direction"); err != nil { + if direction, err := parseLogDirection(request); err != nil { return mcp.NewToolResultError(err.Error()), nil - } else if ok { - directionParam := logsclient.LogDirection(direction) - llParams.Direction = &directionParam + } else if direction != nil { + llParams.Direction = direction } - if limit, ok, err := validate.OptionalToolParam[float64](request, "limit"); err != nil { + if limit, err := parseLogsLimit(request); err != nil { return mcp.NewToolResultError(err.Error()), nil - } else if ok { - limitInt := int(limit) - llParams.Limit = &limitInt + } else if limit != nil { + llParams.Limit = limit } response, err := logRepo.ListLogs(ctx, llParams) @@ -413,11 +413,10 @@ func listLogLabelValues(logRepo *LogRepo) server.ServerTool { params.EndTime = &endTimeParam } - if direction, ok, err := validate.OptionalToolParam[string](request, "direction"); err != nil { + if direction, err := parseLogDirection(request); err != nil { return mcp.NewToolResultError(err.Error()), nil - } else if ok { - directionParam := logsclient.LogDirection(direction) - params.Direction = &directionParam + } else if direction != nil { + params.Direction = direction } values, err := logRepo.ListLogLabelValues(ctx, params) @@ -434,3 +433,44 @@ func listLogLabelValues(logRepo *LogRepo) server.ServerTool { }, } } + +// parseLogsLimit enforces the documented list_logs limit bounds (1-100, +// whole numbers). The schema advertises Min(1)/Max(100), but MCP clients can +// send anything; forwarding out-of-range values only fails downstream as an +// opaque API error. Returns nil when the parameter is absent (API default). +func parseLogsLimit(request mcp.CallToolRequest) (*int, error) { + limit, ok, err := validate.OptionalToolParam[float64](request, "limit") + if err != nil { + return nil, err + } + if !ok { + return nil, nil + } + if math.Trunc(limit) != limit { + return nil, fmt.Errorf("invalid limit %v: must be a whole number between 1 and 100", limit) + } + limitInt := int(limit) + if limitInt < 1 || limitInt > 100 { + return nil, fmt.Errorf("invalid limit %d: must be between 1 and 100", limitInt) + } + return &limitInt, nil +} + +// parseLogDirection validates the direction parameter against the +// backward/forward values the logs API accepts. Returns nil when absent. +func parseLogDirection(request mcp.CallToolRequest) (*logsclient.LogDirection, error) { + direction, ok, err := validate.OptionalToolParam[string](request, "direction") + if err != nil { + return nil, err + } + if !ok { + return nil, nil + } + switch logsclient.LogDirection(direction) { + case logsclient.Backward, logsclient.Forward: + directionParam := logsclient.LogDirection(direction) + return &directionParam, nil + default: + return nil, fmt.Errorf("invalid direction %q: must be one of: backward, forward", direction) + } +} diff --git a/pkg/logs/tools_test.go b/pkg/logs/tools_test.go new file mode 100644 index 0000000..3815785 --- /dev/null +++ b/pkg/logs/tools_test.go @@ -0,0 +1,54 @@ +package logs + +import ( + "testing" + + "github.com/mark3labs/mcp-go/mcp" + logsclient "github.com/render-oss/render-mcp-server/pkg/client/logs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func toolRequest(args map[string]any) mcp.CallToolRequest { + request := mcp.CallToolRequest{} + request.Params.Arguments = args + return request +} + +func TestParseLogsLimit(t *testing.T) { + limit, err := parseLogsLimit(toolRequest(nil)) + require.NoError(t, err) + assert.Nil(t, limit) + + for _, v := range []float64{1, 50, 100} { + limit, err := parseLogsLimit(toolRequest(map[string]any{"limit": v})) + require.NoError(t, err, "limit %v", v) + require.NotNil(t, limit) + assert.Equal(t, int(v), *limit) + } + + for _, v := range []float64{0, -5, 101, 1000, 2.5} { + _, err := parseLogsLimit(toolRequest(map[string]any{"limit": v})) + require.Error(t, err, "limit %v", v) + assert.Contains(t, err.Error(), "invalid limit") + } +} + +func TestParseLogDirection(t *testing.T) { + direction, err := parseLogDirection(toolRequest(nil)) + require.NoError(t, err) + assert.Nil(t, direction) + + for _, v := range []string{"backward", "forward"} { + direction, err := parseLogDirection(toolRequest(map[string]any{"direction": v})) + require.NoError(t, err, "direction %q", v) + require.NotNil(t, direction) + assert.Equal(t, logsclient.LogDirection(v), *direction) + } + + for _, v := range []string{"sideways", "", "BACKWARD"} { + _, err := parseLogDirection(toolRequest(map[string]any{"direction": v})) + require.Error(t, err, "direction %q", v) + assert.Contains(t, err.Error(), "invalid direction") + } +}