diff --git a/internal/config/validation_schema.go b/internal/config/validation_schema.go index e66eb44ff..87e52546c 100644 --- a/internal/config/validation_schema.go +++ b/internal/config/validation_schema.go @@ -274,9 +274,9 @@ func formatValidationErrorRecursive(ve *jsonschema.ValidationError, sb *strings. indent := strings.Repeat(" ", depth) // Format location and message - location := strings.Join(ve.InstanceLocation, "/") - if location == "" { - location = "" + location := "" + if len(ve.InstanceLocation) > 0 { + location = "/" + strings.Join(ve.InstanceLocation, "/") } fmt.Fprintf(sb, "%sLocation: %s\n", indent, location) fmt.Fprintf(sb, "%sError: %s\n", indent, ve.ErrorKind.LocalizedString(schemaErrPrinter)) @@ -342,6 +342,26 @@ func detailForKeyword(keyword string) (string, []string) { "Details: Configuration doesn't match any of the expected formats", " → Review the structure and ensure it matches one of the valid configuration types", } + case "allOf": + return "allOf", []string{ + "Details: Configuration must satisfy all required constraints", + " → Review each related field and ensure every required rule is satisfied together", + } + case "if": + return "conditional", []string{ + "Details: Configuration failed conditional schema evaluation", + " → Check the fields used by the conditional rule and verify the expected branch applies", + } + case "then": + return "conditional", []string{ + "Details: Configuration matched a conditional rule but failed its required follow-up constraints", + " → Update the related fields so the conditional requirements are satisfied", + } + case "else": + return "conditional", []string{ + "Details: Configuration did not match the conditional rule and also failed the alternate constraints", + " → Update the related fields so the alternate conditional requirements are satisfied", + } case "not": return "not", []string{ "Details: Value matches a constraint that it must not match", @@ -457,6 +477,8 @@ func formatErrorContext(ve *jsonschema.ValidationError, prefix string) string { } case *kind.Pattern: addFromKeyword("pattern") + case *kind.AllOf: + addFromKeyword("allOf") case *kind.OneOf, *kind.AnyOf: addFromKeyword("oneOf") case *kind.Not: @@ -480,6 +502,18 @@ func formatErrorContext(ve *jsonschema.ValidationError, prefix string) string { *kind.MinProperties, *kind.MaxProperties: addFromKeyword("range") default: + // Fall back to the schema keyword path when the concrete ErrorKind type + // is not handled directly but still maps to known keyword guidance. + // Unknown keywords intentionally add no detail here and continue to the + // generic fallback below. + if keywordPath := ve.ErrorKind.KeywordPath(); len(keywordPath) > 0 { + addFromKeyword(keywordPath[0]) + } + + if sb.Len() > 0 { + return sb.String() + } + // Generic fallback for any ErrorKind not specifically handled above. // This ensures every validation error gets at least some context rather // than silently producing no detail. The LocalizedString already gives diff --git a/internal/config/validation_schema_error_format_test.go b/internal/config/validation_schema_error_format_test.go index b76f20707..5e12a7f00 100644 --- a/internal/config/validation_schema_error_format_test.go +++ b/internal/config/validation_schema_error_format_test.go @@ -11,8 +11,30 @@ import ( "github.com/santhosh-tekuri/jsonschema/v6/kind" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/text/message" ) +// keywordPathErrorKind is a test helper that implements jsonschema.ErrorKind +// via KeywordPath and LocalizedString for testing KeywordPath-based fallback +// error context dispatch. +type keywordPathErrorKind struct { + keyword string + message string +} + +// KeywordPath returns the configured keyword path for fallback dispatch tests. +func (k *keywordPathErrorKind) KeywordPath() []string { + if k.keyword == "" { + return nil + } + return []string{k.keyword} +} + +// LocalizedString returns the configured message for fallback dispatch tests. +func (k *keywordPathErrorKind) LocalizedString(*message.Printer) string { + return k.message +} + // TestFormatErrorContext tests the formatErrorContext helper function. // This function provides additional diagnostic context for JSON Schema validation errors // based on the ErrorKind type. @@ -120,6 +142,12 @@ func TestFormatErrorContext(t *testing.T) { prefix: "", wantContains: []string{"outside the allowed range"}, }, + { + name: "allOf validation error kind", + errorKind: &kind.AllOf{}, + prefix: "", + wantContains: []string{"satisfy all required constraints", "required rule is satisfied"}, + }, { name: "oneOf validation error kind", errorKind: &kind.OneOf{}, @@ -165,6 +193,24 @@ func TestFormatErrorContext(t *testing.T) { prefix: "", wantContains: []string{"Details:", "unique"}, }, + { + name: "keyword path fallback handles if", + errorKind: &keywordPathErrorKind{keyword: "if", message: "'if' failed"}, + prefix: "", + wantContains: []string{"failed conditional schema evaluation", "expected branch applies"}, + }, + { + name: "keyword path fallback handles then", + errorKind: &keywordPathErrorKind{keyword: "then", message: "'then' failed"}, + prefix: "", + wantContains: []string{"failed its required follow-up constraints", "conditional requirements are satisfied"}, + }, + { + name: "keyword path fallback handles else", + errorKind: &keywordPathErrorKind{keyword: "else", message: "'else' failed"}, + prefix: "", + wantContains: []string{"failed the alternate constraints", "alternate conditional requirements are satisfied"}, + }, { name: "truly unhandled error kind falls back to generic context", // kind.Group is handled but kind.Schema is not, use it to exercise default. @@ -253,6 +299,34 @@ func TestDetailForKeyword(t *testing.T) { wantLinesLen: 2, wantLine0Contains: "Value is outside the allowed range", }, + { + name: "allOf returns combined constraint details", + keyword: "allOf", + wantKey: "allOf", + wantLinesLen: 2, + wantLine0Contains: "satisfy all required constraints", + }, + { + name: "if returns conditional details", + keyword: "if", + wantKey: "conditional", + wantLinesLen: 2, + wantLine0Contains: "conditional schema evaluation", + }, + { + name: "then returns conditional details", + keyword: "then", + wantKey: "conditional", + wantLinesLen: 2, + wantLine0Contains: "follow-up constraints", + }, + { + name: "else returns conditional details", + keyword: "else", + wantKey: "conditional", + wantLinesLen: 2, + wantLine0Contains: "alternate constraints", + }, { name: "oneOf returns no-matching-format details", keyword: "oneOf", @@ -336,7 +410,7 @@ func TestFormatValidationErrorRecursive(t *testing.T) { formatValidationErrorRecursive(ve, &sb, 0) result := sb.String() - assert.Contains(t, result, "Location: mcpServers.github") + assert.Contains(t, result, "Location: /mcpServers.github") assert.Contains(t, result, "Error:") // Verify the Required kind is localized correctly (English: includes the missing property name). // schemaErrPrinter uses language.English, so "container" will appear in the output. @@ -500,8 +574,8 @@ func TestFormatValidationErrorRecursive(t *testing.T) { formatValidationErrorRecursive(ve, &sb, 0) result := sb.String() - assert.Contains(t, result, "Location: mcpServers/github/container", - "Multi-segment instance location should be joined with /") + assert.Contains(t, result, "Location: /mcpServers/github/container", + "Multi-segment instance location should be formatted as an RFC 6901 JSON Pointer") }) } @@ -529,7 +603,7 @@ func TestFormatSchemaError(t *testing.T) { assert.Contains(t, errStr, "Configuration validation error", "Should include standard prefix") assert.Contains(t, errStr, "Location:", "Should include location") assert.Contains(t, errStr, "Error:", "Should include error keyword") - assert.Contains(t, errStr, "mcpServers.github", "Should include the instance location") + assert.Contains(t, errStr, "/mcpServers.github", "Should include the instance location") // Should include documentation footer assert.Contains(t, errStr, "https://", "Should include documentation links") }) @@ -561,7 +635,7 @@ func TestFormatSchemaError(t *testing.T) { require.Error(t, result) errStr := result.Error() assert.Contains(t, errStr, "Configuration validation error") - assert.Contains(t, errStr, "gateway.port") + assert.Contains(t, errStr, "/gateway.port") assert.Contains(t, errStr, "mcp-gateway") }) @@ -623,8 +697,8 @@ func TestFormatSchemaError(t *testing.T) { result := formatSchemaError(parent) require.Error(t, result) errStr := result.Error() - assert.Contains(t, errStr, "mcpServers.github") - assert.Contains(t, errStr, "mcpServers.github.container", + assert.Contains(t, errStr, "/mcpServers.github") + assert.Contains(t, errStr, "/mcpServers.github.container", "Should include child error location from recursive formatting") }) }