DEV-230024 fix(get_assessment): reject limit outside 1-50

pkg/clients/assessments_client.go

@@ -132,7 +132,7 @@ type ListAssessmentsParams struct {
 	AssetID          string `url:"assetId,omitempty"`
 	LastModifiedFrom string `url:"lastModifiedFrom,omitempty"`
 	LastModifiedTo   string `url:"lastModifiedTo,omitempty"`
-	Limit            int    `url:"limit,omitempty"`
+	Limit            *int   `url:"limit,omitempty"`
 	Cursor           string `url:"cursor,omitempty"`
 }
 

pkg/tools/edit_assessment/tool.go

@@ -318,7 +318,7 @@ func resolveAssessmentID(ctx context.Context, client *http.Client, ref string) (
 		return r, nil
 	}
 
-	paged, err := clients.ListAssessments(ctx, client, clients.ListAssessmentsParams{Name: r, Limit: 50})
+	paged, err := clients.ListAssessments(ctx, client, clients.ListAssessmentsParams{Name: r, Limit: chip.Ptr(50)})
 	if err != nil {
 		return "", fmt.Errorf("looking up assessment %q: %w", r, err)
 	}

pkg/tools/get_assessment/tool.go

@@ -12,6 +12,11 @@ import (
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
+const (
+	minLimit = 1
+	maxLimit = 50
+)
+
 // Input has two modes. A DIRECT lookup (assessmentId or assessmentReviewId)
 // fetches a single assessment and can't be combined with anything else. A
 // FILTERED lookup uses any combination of the remaining fields and returns a
@@ -29,7 +34,7 @@ type Input struct {
 	AssetID          string `json:"assetId,omitempty" jsonschema:"Filter: UUID of the asset the assessment was conducted on."`
 	LastModifiedFrom string `json:"lastModifiedFrom,omitempty" jsonschema:"Filter: ISO-8601 timestamp; include assessments last modified at or after this time (e.g. 2026-07-01T00:00:00.000Z)."`
 	LastModifiedTo   string `json:"lastModifiedTo,omitempty" jsonschema:"Filter: ISO-8601 timestamp; include assessments last modified before this time."`
-	Limit            int    `json:"limit,omitempty" jsonschema:"Filter: max results to return (1-50, default 10)."`
+	Limit            *int   `json:"limit,omitempty" jsonschema:"Filter: max results to return (1-50, default 10)."`
 	Cursor           string `json:"cursor,omitempty" jsonschema:"Filter: pagination cursor from a previous response's nextCursor."`
 }
 
@@ -74,6 +79,10 @@ func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] {
 			return Output{}, fmt.Errorf("assessmentId/assessmentReviewId fetch a single assessment and cannot be combined with filters (name, status, template, assetId, dates, limit, cursor)")
 		}
 
+		if err := validateLimit(input.Limit); err != nil {
+			return Output{}, err
+		}
+
 		// Validate the UUID-valued fields that are present.
 		for _, f := range [][2]string{
 			{"assessmentId", input.AssessmentID},
@@ -137,7 +146,16 @@ func handler(collibraClient *http.Client) chip.ToolHandlerFunc[Input, Output] {
 // hasFilters reports whether any filter field is set.
 func hasFilters(in Input) bool {
 	return in.Name != "" || in.Status != "" || in.TemplateID != "" || in.TemplateVersion != "" ||
-		in.AssetID != "" || in.LastModifiedFrom != "" || in.LastModifiedTo != "" || in.Limit != 0 || in.Cursor != ""
+		in.AssetID != "" || in.LastModifiedFrom != "" || in.LastModifiedTo != "" || in.Limit != nil || in.Cursor != ""
+}
+
+// validateLimit rejects a limit outside 1-50, matching the Assessments API. A
+// nil limit is left out of the request so the API default applies.
+func validateLimit(limit *int) error {
+	if limit != nil && (*limit < minLimit || *limit > maxLimit) {
+		return fmt.Errorf("limit must be between %d and %d; got %d", minLimit, maxLimit, *limit)
+	}
+	return nil
 }
 
 // fromList shapes a list lookup: one match returns as the single Assessment,

pkg/tools/get_assessment/tool_test.go

@@ -0,0 +1,120 @@
+package get_assessment_test
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/collibra/chip/pkg/chip"
+	tool "github.com/collibra/chip/pkg/tools/get_assessment"
+	"github.com/collibra/chip/pkg/tools/testutil"
+)
+
+const limitError = "limit must be between 1 and 50; got "
+
+// listStub serves the list endpoint and records the limit query parameter of
+// each request, so a test can tell whether the API was called and with what.
+type listStub struct {
+	calls    int
+	limits   []string
+	hasLimit []bool
+}
+
+func (s *listStub) client(t *testing.T) *http.Client {
+	t.Helper()
+	mux := http.NewServeMux()
+	mux.HandleFunc("GET /rest/assessments/v2/assessments", func(w http.ResponseWriter, r *http.Request) {
+		s.calls++
+		s.limits = append(s.limits, r.URL.Query().Get("limit"))
+		s.hasLimit = append(s.hasLimit, r.URL.Query().Has("limit"))
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"results": []map[string]any{
+				{"id": "assessment-1", "name": "First"},
+				{"id": "assessment-2", "name": "Second"},
+			},
+		})
+	})
+	server := httptest.NewServer(mux)
+	t.Cleanup(server.Close)
+	return testutil.NewClient(server)
+}
+
+func TestGetAssessment_LimitOutOfRangeIsRejectedBeforeAnyCall(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		limit int
+		want  string
+	}{
+		{"zero", 0, limitError + "0"},
+		{"negative", -1, limitError + "-1"},
+		{"above max", 51, limitError + "51"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			s := &listStub{}
+			_, err := tool.NewTool(s.client(t)).Handler(t.Context(), tool.Input{Limit: chip.Ptr(tc.limit)})
+			if err == nil || err.Error() != tc.want {
+				t.Fatalf("want error %q, got %v", tc.want, err)
+			}
+			if s.calls != 0 {
+				t.Fatalf("want no API call for an invalid limit, got %d", s.calls)
+			}
+		})
+	}
+}
+
+func TestGetAssessment_LimitInRangeIsSentToTheAPI(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		limit int
+		want  string
+	}{
+		{"min", 1, "1"},
+		{"max", 50, "50"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			s := &listStub{}
+			out, err := tool.NewTool(s.client(t)).Handler(t.Context(), tool.Input{Limit: chip.Ptr(tc.limit)})
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if !out.Found || len(out.Assessments) != 2 {
+				t.Fatalf("want the listed assessments, got %+v", out)
+			}
+			if s.calls != 1 || s.limits[0] != tc.want {
+				t.Fatalf("want one call with limit=%s, got calls=%d limits=%v", tc.want, s.calls, s.limits)
+			}
+		})
+	}
+}
+
+func TestGetAssessment_NoLimitLeavesTheAPIDefault(t *testing.T) {
+	s := &listStub{}
+	out, err := tool.NewTool(s.client(t)).Handler(t.Context(), tool.Input{})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !out.Found || len(out.Assessments) != 2 {
+		t.Fatalf("want the listed assessments, got %+v", out)
+	}
+	if s.calls != 1 || s.hasLimit[0] {
+		t.Fatalf("want one call without a limit parameter, got calls=%d limits=%v", s.calls, s.limits)
+	}
+}
+
+// The MCP server decodes tool arguments from JSON, so an explicit "limit": 0
+// must decode to a set limit, not to "not set".
+func TestGetAssessment_ExplicitZeroLimitInArgumentsIsRejected(t *testing.T) {
+	var input tool.Input
+	if err := json.Unmarshal([]byte(`{"limit": 0}`), &input); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	s := &listStub{}
+	_, err := tool.NewTool(s.client(t)).Handler(t.Context(), input)
+	if err == nil || err.Error() != limitError+"0" {
+		t.Fatalf("want error %q, got %v", limitError+"0", err)
+	}
+	if s.calls != 0 {
+		t.Fatalf("want no API call, got %d", s.calls)
+	}
+}