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
122 changes: 122 additions & 0 deletions pkg/parser/safe_outputs_error_location_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
package parser

import (
"os"
"path/filepath"
"strings"
"testing"
)

Expand Down Expand Up @@ -385,3 +388,122 @@ safe-outputs:
})
}
}

func TestValidateWithSchemaAndLocationReportsAllSafeOutputFailures(t *testing.T) {
t.Parallel()

yamlContent := `---
on: daily
safe-outputs:
create-issue:
invalid-issue-field: true
create-discussion:
invalid-discussion-field: true
---
# body`
filePath := filepath.Join(t.TempDir(), "workflow.md")
if err := os.WriteFile(filePath, []byte(yamlContent), 0644); err != nil {
t.Fatalf("failed to write test file: %v", err)
}

frontmatter := map[string]any{
"on": "daily",
"safe-outputs": map[string]any{
"create-issue": map[string]any{
"invalid-issue-field": true,
},
"create-discussion": map[string]any{
"invalid-discussion-field": true,
},
},
}

err := validateWithSchemaAndLocation(frontmatter, mainWorkflowSchema, "main workflow file", filePath)
if err == nil {
t.Fatal("expected schema validation error, got nil")
}

errorText := err.Error()
wantSubstrings := []string{
"/safe-outputs/create-issue",
"/safe-outputs/create-discussion",
"line 5, column 5",
"line 7, column 5",
}
for _, want := range wantSubstrings {
if !strings.Contains(errorText, want) {
t.Fatalf("expected error to contain %q, got:\n%s", want, errorText)
}
}
}

// TestFormatSchemaFailureDetailEmptyPath verifies that an empty path is normalised to "/".
func TestFormatSchemaFailureDetailEmptyPath(t *testing.T) {
t.Parallel()

pathInfo := JSONPathInfo{
Path: "",
Message: "additional property 'x' not allowed",
}
result := formatSchemaFailureDetail(pathInfo, "", "on: daily\n", 1)
if !strings.HasPrefix(result, "at '/'") {
t.Errorf("expected result to start with \"at '/'\", got: %s", result)
}
}

// TestFormatSchemaFailureDetailLineColumn verifies that line/column numbers are
// included in the formatted detail when the path can be located in YAML.
func TestFormatSchemaFailureDetailLineColumn(t *testing.T) {
t.Parallel()

frontmatterContent := "on: daily\nsafe-outputs:\n create-issue:\n invalid-field: true\n"
pathInfo := JSONPathInfo{
Path: "/safe-outputs/create-issue",
Message: "additional property 'invalid-field' not allowed",
}
result := formatSchemaFailureDetail(pathInfo, "", frontmatterContent, 1)
if !strings.Contains(result, "line ") || !strings.Contains(result, "column ") {
t.Errorf("expected result to contain line/column info, got: %s", result)
}
if !strings.Contains(result, "/safe-outputs/create-issue") {
t.Errorf("expected result to contain path '/safe-outputs/create-issue', got: %s", result)
}
}

// TestValidateWithSchemaAndLocationSingleFailureNoBulletPrefix verifies that when
// only one schema failure occurs the error message does not include the
// "Multiple schema validation failures" prefix.
func TestValidateWithSchemaAndLocationSingleFailureNoBulletPrefix(t *testing.T) {
t.Parallel()

yamlContent := `---
on: daily
safe-outputs:
create-issue:
invalid-single-field: true
---
# body`
filePath := filepath.Join(t.TempDir(), "workflow.md")
if err := os.WriteFile(filePath, []byte(yamlContent), 0644); err != nil {
t.Fatalf("failed to write test file: %v", err)
}

frontmatter := map[string]any{
"on": "daily",
"safe-outputs": map[string]any{
"create-issue": map[string]any{
"invalid-single-field": true,
},
},
}

err := validateWithSchemaAndLocation(frontmatter, mainWorkflowSchema, "main workflow file", filePath)
if err == nil {
t.Fatal("expected schema validation error, got nil")
}

errorText := err.Error()
if strings.Contains(errorText, "Multiple schema validation failures") {
t.Errorf("single failure should not use 'Multiple schema validation failures' prefix; got:\n%s", errorText)
}
}
43 changes: 34 additions & 9 deletions pkg/parser/schema_compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,12 @@ func validateWithSchemaAndLocation(frontmatter map[string]any, schemaJSON, conte

// If we have paths and frontmatter content, try to get precise locations
if len(jsonPaths) > 0 && frontmatterContent != "" {
// Use the first error path for the primary error location
detailLines := make([]string, 0, len(jsonPaths))
for _, pathInfo := range jsonPaths {
detailLines = append(detailLines, formatSchemaFailureDetail(pathInfo, schemaJSON, frontmatterContent, frontmatterStart))
}

// Use the first error path for primary context rendering.
primaryPath := jsonPaths[0]
location := LocateJSONPathInYAMLWithAdditionalProperties(frontmatterContent, primaryPath.Path, primaryPath.Message)

Expand Down Expand Up @@ -275,14 +280,12 @@ func validateWithSchemaAndLocation(frontmatter map[string]any, schemaJSON, conte
adjustedContextLines = contextLines
}

// Rewrite "additional properties not allowed" errors to be more friendly
// 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, frontmatterContent)
if suggestions != "" {
message = message + ". " + suggestions
// Include every schema failure with path + line + column.
message := ""
if len(detailLines) == 1 {
message = detailLines[0]
} else {
message = "Multiple schema validation failures:\n- " + strings.Join(detailLines, "\n- ")
}

// Create a compiler error with precise location information
Expand Down Expand Up @@ -335,6 +338,28 @@ func validateWithSchemaAndLocation(frontmatter map[string]any, schemaJSON, conte
return err
}

func formatSchemaFailureDetail(pathInfo JSONPathInfo, schemaJSON, frontmatterContent string, frontmatterStart int) string {
path := pathInfo.Path
if path == "" {
path = "/"
}

location := LocateJSONPathInYAMLWithAdditionalProperties(frontmatterContent, pathInfo.Path, pathInfo.Message)
line := frontmatterStart
column := 1
if location.Found {
line = location.Line + frontmatterStart - 1
column = location.Column
}

message := rewriteAdditionalPropertiesError(cleanOneOfMessage(pathInfo.Message))
suggestions := generateSchemaBasedSuggestions(schemaJSON, pathInfo.Message, pathInfo.Path, frontmatterContent)
if suggestions != "" {
message = message + ". " + suggestions
}
return fmt.Sprintf("at '%s' (line %d, column %d): %s", path, line, column, message)
}

// GetMainWorkflowSchema returns the embedded main workflow schema JSON
func GetMainWorkflowSchema() string {
return mainWorkflowSchema
Expand Down