Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions internal/config/validation_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "<root>"
location := "<root>"
if len(ve.InstanceLocation) > 0 {
location = "/" + strings.Join(ve.InstanceLocation, "/")
}
Comment on lines +278 to 280
fmt.Fprintf(sb, "%sLocation: %s\n", indent, location)
fmt.Fprintf(sb, "%sError: %s\n", indent, ve.ErrorKind.LocalizedString(schemaErrPrinter))
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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:
Expand All @@ -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])
Comment on lines +509 to +510
}

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
Expand Down
88 changes: 81 additions & 7 deletions internal/config/validation_schema_error_format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{},
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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")
})
}

Expand Down Expand Up @@ -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")
})
Expand Down Expand Up @@ -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")
})

Expand Down Expand Up @@ -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")
})
}
Loading