From d6e25aad2c37c707a8dda1768a545351d4de93a5 Mon Sep 17 00:00:00 2001 From: Andrew Cunliffe Date: Thu, 7 May 2026 16:59:40 -0700 Subject: [PATCH 1/2] feat(validator): explicit description max-length feedback (#1184) Replaces the upstream jsonschema library's terse `length must be <= N, but got M` message with an explicit, actionable error that names the offending field, restates the limit, and includes a truncated value the publisher can paste back. Applies to any string field that has a maxLength constraint, but the trigger case from the issue is `description` (max 100). Example output: field "description" is too long: 312 chars (max 100). Truncate to: "..." Adds unit tests for the new message format plus the truncation and field-name helpers (rune-safe truncation included). Closes #1184 --- internal/validators/export_test.go | 8 ++ internal/validators/schema.go | 121 +++++++++++++++++- .../validators/validation_detailed_test.go | 85 ++++++++++++ 3 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 internal/validators/export_test.go diff --git a/internal/validators/export_test.go b/internal/validators/export_test.go new file mode 100644 index 000000000..e76b94be3 --- /dev/null +++ b/internal/validators/export_test.go @@ -0,0 +1,8 @@ +package validators + +// Test-only exports of internal helpers so external test packages can verify them +// directly without going through the full schema validation flow. +var ( + ExtractFieldNameForTest = extractFieldName + TruncateForSuggestionForTest = truncateForSuggestion +) diff --git a/internal/validators/schema.go b/internal/validators/schema.go index 37d335eaf..539f73ed9 100644 --- a/internal/validators/schema.go +++ b/internal/validators/schema.go @@ -9,6 +9,7 @@ import ( "regexp" "strconv" "strings" + "unicode/utf8" apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0" "github.com/modelcontextprotocol/registry/pkg/model" @@ -227,7 +228,7 @@ func validateServerJSONSchema(serverJSON *apiv0.ServerJSON, performValidation bo var validationErr *jsonschema.ValidationError if errors.As(err, &validationErr) { // Process the validation error and its causes - addValidationError(result, validationErr, schema) + addValidationError(result, validationErr, schema, serverMap) } else { // Fallback for other error types issue := NewValidationIssue( @@ -245,13 +246,13 @@ func validateServerJSONSchema(serverJSON *apiv0.ServerJSON, performValidation bo } // addValidationError processes validation errors and extracts useful information -func addValidationError(result *ValidationResult, validationErr *jsonschema.ValidationError, schema map[string]any) { +func addValidationError(result *ValidationResult, validationErr *jsonschema.ValidationError, schema map[string]any, instance any) { // Use DetailedOutput to get the nested error details detailed := validationErr.DetailedOutput() // Process the detailed error structure - addDetailedErrors(result, detailed, schema) + addDetailedErrors(result, detailed, schema, instance) } // ConvertJSONPointerToBracketNotation converts a JSON Pointer path (RFC 6901) to bracket notation @@ -313,7 +314,7 @@ func ConvertJSONPointerToBracketNotation(jsonPointer string) string { } // addDetailedErrors recursively processes detailed validation errors -func addDetailedErrors(result *ValidationResult, detailed jsonschema.Detailed, schema map[string]any) { +func addDetailedErrors(result *ValidationResult, detailed jsonschema.Detailed, schema map[string]any, instance any) { // Only process errors that have specific field paths and meaningful messages if detailed.InstanceLocation != "" && detailed.Error != "" { // Convert JSON Pointer format to bracket notation to match semantic validation format @@ -330,6 +331,13 @@ func addDetailedErrors(result *ValidationResult, detailed jsonschema.Detailed, s message = strings.ReplaceAll(message, "is not valid", "has invalid format") } + // Provide explicit feedback for maxLength violations: name the field, show the + // observed/max length, and suggest a truncated value the publisher can paste back. + // Issue: https://github.com/modelcontextprotocol/registry/issues/1184 + if enhanced, ok := enhanceMaxLengthMessage(detailed, instance, path); ok { + message = enhanced + } + // Build the full resolved reference path reference := buildResolvedReference(detailed.KeywordLocation, detailed.AbsoluteKeywordLocation, schema) @@ -345,8 +353,111 @@ func addDetailedErrors(result *ValidationResult, detailed jsonschema.Detailed, s // Process nested errors for _, nested := range detailed.Errors { - addDetailedErrors(result, nested, schema) + addDetailedErrors(result, nested, schema, instance) + } +} + +// maxLengthErrorRe matches the jsonschema library's default maxLength error message, +// which is formatted as "length must be <= MAX, but got OBSERVED". Captures: 1=max, 2=observed. +var maxLengthErrorRe = regexp.MustCompile(`length must be <= (\d+), but got (\d+)`) + +// enhanceMaxLengthMessage rewrites the jsonschema library's terse maxLength error message +// into something publishers can act on directly: it names the field, reports the observed +// length, restates the limit, and includes a truncated value they can paste back. Returns +// false if the error is not a maxLength violation or the offending value cannot be resolved. +func enhanceMaxLengthMessage(detailed jsonschema.Detailed, instance any, bracketPath string) (string, bool) { + if !strings.HasSuffix(detailed.KeywordLocation, "/maxLength") { + return "", false + } + matches := maxLengthErrorRe.FindStringSubmatch(detailed.Error) + if len(matches) != 3 { + return "", false + } + maxLen, err := strconv.Atoi(matches[1]) + if err != nil { + return "", false + } + value, ok := resolveJSONPointer(instance, detailed.InstanceLocation) + if !ok { + return "", false + } + str, ok := value.(string) + if !ok { + return "", false + } + fieldName := extractFieldName(bracketPath) + observed := utf8.RuneCountInString(str) + suggestion := truncateForSuggestion(str, maxLen) + return fmt.Sprintf(`field %q is too long: %d chars (max %d). Truncate to: %q`, + fieldName, observed, maxLen, suggestion), true +} + +// resolveJSONPointer walks a decoded JSON value (map[string]any / []any) following an +// RFC 6901 JSON Pointer and returns the value at that location. Returns false if the +// pointer cannot be resolved. +func resolveJSONPointer(data any, pointer string) (any, bool) { + if pointer == "" { + return data, true + } + if !strings.HasPrefix(pointer, "/") { + return nil, false + } + current := data + for _, raw := range strings.Split(pointer[1:], "/") { + // RFC 6901 escape sequences: ~1 -> "/", ~0 -> "~" (~1 must come first) + token := strings.ReplaceAll(raw, "~1", "/") + token = strings.ReplaceAll(token, "~0", "~") + switch node := current.(type) { + case map[string]any: + next, exists := node[token] + if !exists { + return nil, false + } + current = next + case []any: + idx, err := strconv.Atoi(token) + if err != nil || idx < 0 || idx >= len(node) { + return nil, false + } + current = node[idx] + default: + return nil, false + } + } + return current, true +} + +// extractFieldName returns the leaf field name from a bracket-notation path, so e.g. +// "packages[0].description" -> "description" and "packages[0]" -> "packages". +func extractFieldName(path string) string { + if path == "" { + return "" + } + if idx := strings.LastIndex(path, "."); idx != -1 { + path = path[idx+1:] + } + if idx := strings.Index(path, "["); idx != -1 { + path = path[:idx] + } + return path +} + +// truncateForSuggestion returns a rune-safe truncation of s that fits within maxLen +// characters, ending in an ellipsis "..." when truncation is needed. For maxLen <= 3 +// (no room for an ellipsis) it returns the first maxLen runes verbatim. +func truncateForSuggestion(s string, maxLen int) string { + if maxLen <= 0 { + return "" + } + runes := []rune(s) + if len(runes) <= maxLen { + return s + } + const ellipsis = "..." + if maxLen <= len(ellipsis) { + return string(runes[:maxLen]) } + return string(runes[:maxLen-len(ellipsis)]) + ellipsis } // buildResolvedReference extracts the resolved reference path by resolving $ref segments diff --git a/internal/validators/validation_detailed_test.go b/internal/validators/validation_detailed_test.go index fc6117827..68453c036 100644 --- a/internal/validators/validation_detailed_test.go +++ b/internal/validators/validation_detailed_test.go @@ -1,6 +1,7 @@ package validators_test import ( + "fmt" "strings" "testing" @@ -445,3 +446,87 @@ func TestValidateServerJSON_NonCurrentSchema_Policies(t *testing.T) { }) } } + +// TestValidateServerJSON_MaxLengthFeedback ensures publishers get an explicit, actionable +// error when a string field exceeds the schema's maxLength, instead of the upstream +// jsonschema library's terse "expected length <= N". See issue #1184. +func TestValidateServerJSON_MaxLengthFeedback(t *testing.T) { + const observedLen = 312 + longDescription := strings.Repeat("x", observedLen) + + serverJSON := &apiv0.ServerJSON{ + Schema: model.CurrentSchemaURL, + Name: "com.example.test/server", + Version: "1.0.0", + Description: longDescription, + Repository: &model.Repository{ + URL: "https://github.com/example/server", + Source: "github", + }, + } + + result := validators.ValidateServerJSON(serverJSON, validators.ValidationAll) + assert.False(t, result.Valid, "Description over maxLength should fail validation") + + var descIssue *validators.ValidationIssue + for i := range result.Issues { + issue := &result.Issues[i] + if issue.Path == "description" && issue.Type == validators.ValidationIssueTypeSchema { + descIssue = issue + break + } + } + if !assert.NotNil(t, descIssue, "Should have a schema validation issue at path 'description'") { + return + } + + // The new message must explicitly name the field, report observed length, restate + // the max, and suggest a truncated value. Avoid hardcoding the schema's max here so + // this test isn't brittle to schema bumps — but verify the format pieces are present. + msg := descIssue.Message + assert.Contains(t, msg, `field "description" is too long`, "should name the field") + assert.Contains(t, msg, fmt.Sprintf("%d chars", observedLen), "should report observed length") + assert.Regexp(t, `\(max \d+\)`, msg, "should restate the max length from the schema") + assert.Contains(t, msg, "Truncate to:", "should offer a truncation suggestion") + assert.Contains(t, msg, "...", "truncation suggestion should end with an ellipsis") + assert.NotContains(t, msg, "length must be <=", "should replace the raw jsonschema phrasing") +} + +func TestExtractFieldName(t *testing.T) { + cases := []struct { + path string + want string + }{ + {"", ""}, + {"description", "description"}, + {"packages[0].description", "description"}, + {"packages[0]", "packages"}, + {"remotes[0].headers[1].name", "name"}, + } + for _, c := range cases { + t.Run(c.path, func(t *testing.T) { + assert.Equal(t, c.want, validators.ExtractFieldNameForTest(c.path)) + }) + } +} + +func TestTruncateForSuggestion(t *testing.T) { + cases := []struct { + name string + input string + maxLen int + want string + }{ + {"shorter than max returns original", "hello", 10, "hello"}, + {"exact length returns original", "hello", 5, "hello"}, + {"longer than max gets ellipsis", "abcdefghij", 6, "abc..."}, + {"unicode safe truncation", "αβγδεζηθικ", 5, "αβ..."}, + {"max too small for ellipsis returns prefix", "abcdef", 2, "ab"}, + {"zero max returns empty", "abc", 0, ""}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, validators.TruncateForSuggestionForTest(c.input, c.maxLen)) + }) + } +} From c6851db99ed7d6dcd1d73443135c2264eaf79ec5 Mon Sep 17 00:00:00 2001 From: Andrew Cunliffe Date: Sat, 9 May 2026 00:04:38 -0700 Subject: [PATCH 2/2] Retrigger CI (NuGet test was flaky network timeout)