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
3 changes: 2 additions & 1 deletion pkg/parser/schema_compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ func validateWithSchemaAndLocation(frontmatter map[string]any, schemaJSON, conte
}

// Rewrite "additional properties not allowed" errors to be more friendly
message := rewriteAdditionalPropertiesError(primaryPath.Message)
// Also clean up oneOf jargon (e.g., "got string, want object") to plain English
message := rewriteAdditionalPropertiesError(cleanOneOfMessage(primaryPath.Message))

// Add schema-based suggestions
suggestions := generateSchemaBasedSuggestions(schemaJSON, primaryPath.Message, primaryPath.Path)
Expand Down
98 changes: 97 additions & 1 deletion pkg/parser/schema_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import (
"strings"
)

// atPathPattern matches "- at '/path': " or "at '/path': " prefixes in error messages
var atPathPattern = regexp.MustCompile(`^-?\s*at '([^']*)': (.+)$`)

// cleanJSONSchemaErrorMessage removes unhelpful prefixes from jsonschema validation errors
func cleanJSONSchemaErrorMessage(errorMsg string) string {
// Split the error message into lines
Expand Down Expand Up @@ -37,7 +40,100 @@ func cleanJSONSchemaErrorMessage(errorMsg string) string {
return "schema validation failed"
}

return result
// Apply oneOf cleanup to the full cleaned message
return cleanOneOfMessage(result)
}

// cleanOneOfMessage simplifies 'oneOf failed, none matched' error messages by:
// 1. Removing "got X, want Y" type-mismatch lines (from the wrong branch of a oneOf)
// 2. Removing the "oneOf failed, none matched" wrapper line
// 3. Extracting the most meaningful sub-error (e.g., enum constraint violations)
//
// This converts confusing schema jargon like:
//
// "'oneOf' failed, none matched\n- at '/engine': value must be one of...\n- at '/engine': got string, want object"
//
// into plain language:
//
// "value must be one of 'claude', 'codex', 'copilot', 'gemini'"
func cleanOneOfMessage(message string) string {
if !strings.Contains(message, "'oneOf' failed") {
return message
}

lines := strings.Split(message, "\n")
var meaningful []string

for _, line := range lines {
trimmed := strings.TrimSpace(line)
if trimmed == "" {
continue
}
// Skip the "oneOf failed" wrapper line β€” it's schema jargon, not user guidance
if strings.Contains(trimmed, "'oneOf' failed, none matched") {
continue
}
// Skip "got X, want Y" type-mismatch lines from the wrong oneOf branch
if isTypeConflictLine(trimmed) {
continue
}
meaningful = append(meaningful, trimmed)
}

if len(meaningful) == 0 {
return message // Return original if we cannot simplify
}

// Strip "- at '/path':" prefixes and format each remaining constraint
var cleaned []string
for _, line := range meaningful {
cleaned = append(cleaned, stripAtPathPrefix(line))
}

return strings.Join(cleaned, "; ")
}

// isTypeConflictLine returns true for "got X, want Y" lines that arise from the
// wrong branch of a oneOf constraint. These lines are generated when the user's value
// matches one branch's type but not the other, and they are confusing to display.
// Handles both bare "got X, want Y" and embedded "- at '/path': got X, want Y" forms.
func isTypeConflictLine(line string) bool {
// Direct "got X, want Y" format (bare form)
if strings.HasPrefix(line, "got ") && strings.Contains(line, ", want ") {
return true
}
// Embedded form: "- at '/path': got X, want Y"
// Look for ": got " followed by ", want " later in the line
if idx := strings.Index(line, ": got "); idx >= 0 {
afterGot := line[idx+len(": got "):]
return strings.Contains(afterGot, ", want ")
}
return false
}

// stripAtPathPrefix removes "- at '/path': " or "at '/path': " prefixes from schema error lines
// and formats nested path references to be more readable.
//
// Examples:
// - "- at '/engine': value must be one of..." β†’ "value must be one of..."
// - "- at '/permissions/deployments': value must be..." β†’ "'deployments': value must be..."
func stripAtPathPrefix(line string) string {
match := atPathPattern.FindStringSubmatch(line)
if match == nil {
return line
}
path := match[1]
msg := match[2]

// For nested paths (e.g., /permissions/deployments), keep the last component
// so users know which sub-field has the error
if idx := strings.LastIndex(path, "/"); idx > 0 {
subField := path[idx+1:]
return fmt.Sprintf("'%s': %s", subField, msg)
}

// For top-level field errors, just return the constraint message
return msg
}

// findFrontmatterBounds finds the start and end indices of frontmatter in file lines
Expand Down
223 changes: 223 additions & 0 deletions pkg/parser/schema_errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
//go:build !integration

package parser

import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

// TestCleanOneOfMessage tests that oneOf error messages are simplified to plain English
func TestCleanOneOfMessage(t *testing.T) {
tests := []struct {
name string
input string
wantNot []string // substrings that must NOT appear in output
wantAny []string // at least one of these must appear in output
}{
{
name: "engine typo removes got-string-want-object branch",
input: "at '/engine': 'oneOf' failed, none matched\n" +
"- at '/engine': value must be one of 'claude', 'codex', 'copilot', 'gemini'\n" +
"- at '/engine': got string, want object",
wantNot: []string{"oneOf", "got string, want object"},
wantAny: []string{"value must be one of 'claude', 'codex', 'copilot', 'gemini'"},
},
{
name: "permissions typo removes got-object-want-string branch",
input: "at '/permissions': 'oneOf' failed, none matched\n" +
"- at '/permissions': got object, want string\n" +
"- at '/permissions/deployments': value must be one of 'read', 'write', 'none'",
wantNot: []string{"oneOf", "got object, want string"},
wantAny: []string{"value must be one of 'read', 'write', 'none'"},
},
{
name: "non-oneOf message is returned unchanged",
input: "value must be one of 'a', 'b', 'c'",
wantNot: []string{"oneOf"},
wantAny: []string{"value must be one of 'a', 'b', 'c'"},
},
{
name: "nested path context preserved for sub-field errors",
input: "at '/permissions': 'oneOf' failed, none matched\n" +
"- at '/permissions': got object, want string\n" +
"- at '/permissions/deployments': value must be one of 'read', 'write', 'none'",
wantNot: []string{},
wantAny: []string{"deployments"},
},
{
name: "message unchanged when all sub-errors are type conflicts",
input: "at '/x': 'oneOf' failed, none matched\n" +
"- at '/x': got string, want object\n" +
"- at '/x': got string, want array",
// When nothing meaningful remains, return the original
wantAny: []string{"oneOf"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := cleanOneOfMessage(tt.input)

for _, unwanted := range tt.wantNot {
assert.NotContains(t, result, unwanted,
"Result should not contain %q\nResult: %s", unwanted, result)
}

if len(tt.wantAny) > 0 {
found := false
for _, wanted := range tt.wantAny {
if strings.Contains(result, wanted) {
found = true
break
}
}
assert.True(t, found,
"Result should contain at least one of %v\nResult: %s", tt.wantAny, result)
}
})
}
}

// TestIsTypeConflictLine tests detection of "got X, want Y" lines in oneOf errors
func TestIsTypeConflictLine(t *testing.T) {
tests := []struct {
name string
line string
want bool
}{
{
name: "bare got-want format",
line: "got string, want object",
want: true,
},
{
name: "embedded in at-path format",
line: "- at '/engine': got string, want object",
want: true,
},
{
name: "embedded got-object-want-string",
line: "- at '/permissions': got object, want string",
want: true,
},
{
name: "enum constraint is not a type conflict",
line: "- at '/engine': value must be one of 'claude', 'codex'",
want: false,
},
{
name: "additional property error is not a type conflict",
line: "additional property 'foo' not allowed",
want: false,
},
{
name: "empty line is not a type conflict",
line: "",
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isTypeConflictLine(tt.line)
assert.Equal(t, tt.want, got,
"isTypeConflictLine(%q) = %v, want %v", tt.line, got, tt.want)
})
}
}

// TestStripAtPathPrefix tests removal of "at '/path':" prefixes from error lines
func TestStripAtPathPrefix(t *testing.T) {
tests := []struct {
name string
line string
want string
}{
{
name: "top-level path stripped entirely",
line: "- at '/engine': value must be one of 'claude', 'codex'",
want: "value must be one of 'claude', 'codex'",
},
{
name: "nested path keeps last component",
line: "- at '/permissions/deployments': value must be one of 'read', 'write', 'none'",
want: "'deployments': value must be one of 'read', 'write', 'none'",
},
{
name: "line without at-path prefix is unchanged",
line: "value must be one of 'a', 'b'",
want: "value must be one of 'a', 'b'",
},
{
name: "at-path without dash prefix",
line: "at '/engine': some error",
want: "some error",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := stripAtPathPrefix(tt.line)
assert.Equal(t, tt.want, got,
"stripAtPathPrefix(%q) = %q, want %q", tt.line, got, tt.want)
})
}
}

// TestCleanJSONSchemaErrorMessage tests the full cleanup pipeline for jsonschema errors
func TestCleanJSONSchemaErrorMessage(t *testing.T) {
tests := []struct {
name string
input string
wantNot []string
wantAny []string
}{
{
name: "removes jsonschema validation failed header",
input: "jsonschema validation failed with 'http://contoso.com/schema.json#'\n" +
"- at '/engine': 'oneOf' failed, none matched\n" +
"- at '/engine': value must be one of 'claude', 'codex'\n" +
"- at '/engine': got string, want object",
wantNot: []string{"jsonschema validation failed", "contoso.com", "got string, want object", "oneOf"},
wantAny: []string{"value must be one of 'claude', 'codex'"},
},
{
name: "removes at-root prefix",
input: "jsonschema validation failed with '...'\n" +
"- at '': additional property 'foo' not allowed",
wantNot: []string{"jsonschema validation failed", "at '': "},
wantAny: []string{"additional property 'foo' not allowed"},
},
{
name: "empty result falls back to generic message",
input: "jsonschema validation failed with '...'",
wantAny: []string{"schema validation failed"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := cleanJSONSchemaErrorMessage(tt.input)

for _, unwanted := range tt.wantNot {
assert.NotContains(t, result, unwanted,
"Result should not contain %q\nResult: %s", unwanted, result)
}

if len(tt.wantAny) > 0 {
found := false
for _, wanted := range tt.wantAny {
if strings.Contains(result, wanted) {
found = true
break
}
}
assert.True(t, found,
"Result should contain at least one of %v\nResult: %s", tt.wantAny, result)
}
})
}
}
33 changes: 33 additions & 0 deletions pkg/workflow/frontmatter_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,37 @@ var (
sourceContextPattern = regexp.MustCompile(`\n(\s+\d+\s*\|)`)
)

// yamlErrorTranslations maps raw goccy/go-yaml internal messages to user-friendly plain English.
// These messages are parser internals that are not helpful to end users.
var yamlErrorTranslations = []struct {
pattern string
translation string
}{
{
"non-map value is specified",
"Invalid YAML syntax: expected 'key: value' format (did you forget a colon after the key?)",
},
{
"mapping values are not allowed",
"Invalid YAML syntax: unexpected ':' β€” check your indentation",
},
{
"did not find expected",
"Invalid YAML syntax: check indentation or missing key",
},
}

// translateYAMLMessage converts raw YAML parser messages to user-friendly plain English.
// This prevents internal library jargon from reaching the end user.
func translateYAMLMessage(message string) string {
for _, t := range yamlErrorTranslations {
if strings.Contains(message, t.pattern) {
return t.translation
}
}
return message
}

// createFrontmatterError creates a detailed error for frontmatter parsing issues
// frontmatterLineOffset is the line number where the frontmatter content begins (1-based)
// Returns error in VSCode-compatible format: filename:line:column: error message
Expand All @@ -37,6 +68,8 @@ func (c *Compiler) createFrontmatterError(filePath, content string, err error, f
if idx := strings.Index(message, "\n"); idx != -1 {
message = message[:idx]
}
// Translate raw YAML parser messages to user-friendly plain English
message = translateYAMLMessage(message)

// Format as: filename:line:column: error: message
// This is compatible with VSCode's problem matcher
Expand Down
Loading
Loading