Skip to content
Closed
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
10 changes: 5 additions & 5 deletions pkg/parser/frontmatter_syntax_errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ This is a test workflow.`,
expectError: true,
expectedMinLine: 3,
expectedMinColumn: 1,
expectedErrorContains: "non-map value",
expectedErrorContains: "Invalid YAML syntax",
description: "Missing colon in YAML mapping",
},
{
Expand All @@ -53,7 +53,7 @@ This workflow has invalid indentation.`,
expectError: true,
expectedMinLine: 4,
expectedMinColumn: 1,
expectedErrorContains: "non-map value",
expectedErrorContains: "Invalid YAML syntax",
description: "Invalid indentation in nested YAML structure",
},
{
Expand Down Expand Up @@ -132,7 +132,7 @@ This workflow has malformed string quotes.`,
expectError: true,
expectedMinLine: 2,
expectedMinColumn: 1,
expectedErrorContains: "could not find end character",
expectedErrorContains: "unclosed",
description: "Malformed string quotes in YAML",
},
{
Expand Down Expand Up @@ -198,7 +198,7 @@ This workflow has unexpected end of stream.`,
expectError: true,
expectedMinLine: 5,
expectedMinColumn: 14,
expectedErrorContains: "not found",
expectedErrorContains: "unclosed",
description: "Unexpected end of stream in YAML",
},
{
Expand Down Expand Up @@ -232,7 +232,7 @@ This workflow has mixed tab and space indentation.`,
expectError: true, // goccy actually does catch this error
expectedMinLine: 5,
expectedMinColumn: 1,
expectedErrorContains: "cannot start any token",
expectedErrorContains: "Invalid YAML syntax",
description: "Mixed tab and space indentation in YAML",
},
{
Expand Down
53 changes: 53 additions & 0 deletions pkg/parser/yaml_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,56 @@ var (
sourceLinePattern = regexp.MustCompile(`(?m)^(>?\s*)(\d+)(\s*\|)`)
)

// yamlParserMessageTranslations maps raw goccy/go-yaml parser jargon to user-friendly plain English.
// Applied as substring replacements within the formatted yaml.FormatError() output.
// Patterns must match the actual strings produced by goccy/go-yaml v1.19+.
var yamlParserMessageTranslations = []struct {
pattern string
translation string
}{
// Colon in wrong context (e.g. "key: value: extra")
{"mapping value is not allowed in this context",
"Invalid YAML syntax: unexpected ':' — check indentation or key syntax"},
// Bare key without colon (e.g. "on push") OR list item in mapping context
{"non-map value is specified",
"Invalid YAML syntax: expected 'key: value' format (did you forget a colon after the key?)"},
// Plain word on its own without colon (e.g. "engine copilot")
{"unexpected key name",
"Invalid YAML syntax: expected 'key: value' format (did you forget a colon after the key?)"},
// Tab indentation error from goccy v1.19 (actual tab byte 0x09 in single quotes)
{"found a tab character where an indentation space is expected",
"Invalid YAML syntax: use spaces for indentation, not tabs"},
{"tab character cannot use as a map key",
"Invalid YAML syntax: use spaces for indentation, not tabs"},
// The full goccy message uses an actual tab character (0x09) inside single quotes
{"found character '\t' that cannot start any token",
"Invalid YAML syntax: use spaces for indentation, not tabs"},
// List item ('-') in wrong context
{"block sequence entries are not allowed",
"Invalid YAML syntax: unexpected list item '-' — check indentation"},
// Unclosed sequences/brackets
{"sequence end token ']' not found",
"Invalid YAML syntax: unclosed bracket — add ']' to close the list"},
// Unclosed string quotes
{"could not find end character of double-quoted text",
`Invalid YAML syntax: unclosed double quote — add '"' to close the string`},
{"could not find end character of single-quoted text",
"Invalid YAML syntax: unclosed single quote — add \"'\" to close the string"},
}

// translateYAMLFormattedOutput applies user-friendly translations to raw goccy error messages
// within a formatted yaml.FormatError() output string, preserving the source context lines.
// Only the first matching pattern is translated: a single goccy error has one message line,
// so at most one translation applies.
func translateYAMLFormattedOutput(formatted string) string {
for _, t := range yamlParserMessageTranslations {
if strings.Contains(formatted, t.pattern) {
return strings.Replace(formatted, t.pattern, t.translation, 1)
}
}
return formatted
}

// FormatYAMLError formats a YAML error with source code context using yaml.FormatError()
// frontmatterLineOffset is the line number where the frontmatter content begins in the document (1-based)
// Returns the formatted error string with line numbers adjusted for frontmatter position
Expand All @@ -28,6 +78,9 @@ func FormatYAMLError(err error, frontmatterLineOffset int, sourceYAML string) st
// colored=false to avoid ANSI escape codes, inclSource=true to include source lines
formatted := yaml.FormatError(err, false, true)

// Translate raw parser jargon to user-friendly messages before adjusting line numbers
formatted = translateYAMLFormattedOutput(formatted)

// Adjust line numbers in the formatted output to account for frontmatter position
if frontmatterLineOffset > 1 {
formatted = adjustLineNumbersInFormattedError(formatted, frontmatterLineOffset-1)
Expand Down
64 changes: 63 additions & 1 deletion pkg/parser/yaml_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,69 @@ func TestFormatYAMLError(t *testing.T) {
}
}

// TestFormatYAMLErrorAdjustment specifically tests line number adjustment
// TestFormatYAMLErrorTranslations verifies that FormatYAMLError translates raw goccy messages
// to user-friendly plain English while preserving source context.
func TestFormatYAMLErrorTranslations(t *testing.T) {
tests := []struct {
name string
yamlContent string
wantTranslated string // substring that MUST appear (the translated message)
wantNotRaw string // substring that must NOT appear (the raw goccy message)
}{
{
name: "missing colon gets user-friendly message",
yamlContent: "name: Test Workflow\non push",
wantTranslated: "Invalid YAML syntax",
wantNotRaw: "non-map value is specified",
},
{
name: "extra colon gets user-friendly message",
yamlContent: "invalid: yaml: syntax",
wantTranslated: "Invalid YAML syntax",
wantNotRaw: "mapping value is not allowed in this context",
},
{
name: "unclosed bracket gets user-friendly message",
yamlContent: "branches: [main, dev",
wantTranslated: "Invalid YAML syntax",
wantNotRaw: "sequence end token ']' not found",
},
{
name: "unclosed double-quote gets user-friendly message",
yamlContent: `name: "unclosed string`,
wantTranslated: "Invalid YAML syntax",
wantNotRaw: "could not find end character",
},
{
// Source context (| markers and ^) must still be present after translation
name: "source context preserved after translation",
yamlContent: "name: Test Workflow\non push",
wantTranslated: "|",
wantNotRaw: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var result map[string]any
err := yaml.Unmarshal([]byte(tt.yamlContent), &result)
if err == nil {
t.Fatalf("Expected YAML parsing to fail for: %q", tt.yamlContent)
}

formatted := FormatYAMLError(err, 1, tt.yamlContent)

if tt.wantTranslated != "" && !strings.Contains(formatted, tt.wantTranslated) {
t.Errorf("Expected translated output to contain %q\nGot:\n%s", tt.wantTranslated, formatted)
}
if tt.wantNotRaw != "" && strings.Contains(formatted, tt.wantNotRaw) {
t.Errorf("Expected raw goccy message %q to be translated away\nGot:\n%s", tt.wantNotRaw, formatted)
}
t.Logf("Formatted:\n%s", formatted)
})
}
}

func TestFormatYAMLErrorAdjustment(t *testing.T) {
yamlContent := "name: test\nname: duplicate"

Expand Down
11 changes: 11 additions & 0 deletions pkg/workflow/compiler_orchestrator_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,17 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean
orchestratorEngineLog.Printf("Validating engine setting: %s", engineSetting)
if err := c.validateEngine(engineSetting); err != nil {
orchestratorEngineLog.Printf("Engine validation failed: %v", err)
// Locate the engine field in the frontmatter for a precise error position
frontmatterYAML := strings.Join(result.FrontmatterLines, "\n")
loc := parser.LocateJSONPathInYAML(frontmatterYAML, "/engine")
if loc.Found {
// loc.Line is 1-based within the frontmatter content.
// result.FrontmatterStart is the 1-based document line where frontmatter content begins
// (typically 2, since line 1 is the opening "---"), so:
// docLine = (frontmatter-relative line) + (FrontmatterStart - 1)
docLine := loc.Line + result.FrontmatterStart - 1
return nil, formatCompilerErrorWithPosition(cleanPath, docLine, loc.Column, "error", err.Error(), nil)
}
return nil, err
}

Expand Down
38 changes: 38 additions & 0 deletions pkg/workflow/compiler_orchestrator_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,44 @@ engine: invalid-engine-name
require.Error(t, err, "Invalid engine should cause error")
assert.Nil(t, result)
assert.Contains(t, err.Error(), "invalid-engine-name")
// The error should include precise location (line:col) pointing to the engine field
assert.Contains(t, err.Error(), ":3:", "Error should include the line number of the engine field")
}

// TestSetupEngineAndImports_InvalidEngine_HasPreciseLocation verifies that the engine
// validation error includes precise file location (line/column) pointing to the engine field.
func TestSetupEngineAndImports_InvalidEngine_HasPreciseLocation(t *testing.T) {
tmpDir := testutil.TempDir(t, "engine-location")

// engine: is on line 3 of the document (after opening --- on line 1, on: push on line 2)
testContent := `---
on: push
engine: not-a-real-engine
permissions:
contents: read
---

# Test Workflow
`

testFile := filepath.Join(tmpDir, "location-test.md")
require.NoError(t, os.WriteFile(testFile, []byte(testContent), 0644))

compiler := NewCompiler()
content := []byte(testContent)

frontmatterResult, err := parser.ExtractFrontmatterFromContent(string(content))
require.NoError(t, err)

result, err := compiler.setupEngineAndImports(frontmatterResult, testFile, content, tmpDir)
require.Error(t, err, "Invalid engine should cause error")
assert.Nil(t, result)

errMsg := err.Error()
// Error must point to line 3 where engine: appears in the document
assert.Contains(t, errMsg, ":3:", "Error should point to line 3 where the engine field is")
// Error must still contain the invalid engine name
assert.Contains(t, errMsg, "not-a-real-engine", "Error should mention the invalid engine name")
}

// TestSetupEngineAndImports_StrictModeHandling tests strict mode state management
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/compiler_orchestrator_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@ on: [invalid: yaml

# Workflow
`,
expectError: "sequence end token", // Check for actual YAML error message instead of "parse frontmatter"
expectError: "unclosed bracket", // User-friendly translation of "sequence end token ']' not found"
},
{
name: "no markdown content for main workflow",
Expand Down
16 changes: 8 additions & 8 deletions pkg/workflow/compiler_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ strict: false
Invalid YAML with bad mapping.`,
expectedErrorLine: 7, // Line 7 in file (line 6 in YAML content after opening ---)
expectedErrorColumn: 10,
expectedMessagePart: "mapping value is not allowed in this context",
expectedMessagePart: "Invalid YAML syntax",
description: "invalid mapping context should be detected",
},
{
Expand All @@ -91,7 +91,7 @@ strict: false
Invalid YAML with bad indentation.`,
expectedErrorLine: 4, // Line 4 in file (line 3 in YAML content after opening ---)
expectedErrorColumn: 11,
expectedMessagePart: "mapping value is not allowed in this context",
expectedMessagePart: "Invalid YAML syntax",
description: "bad indentation should be detected",
},
{
Expand All @@ -116,7 +116,7 @@ strict: false
Invalid YAML with unclosed quote.`,
expectedErrorLine: 9, // Line 9 in file (line 8 in YAML content after opening ---)
expectedErrorColumn: 15,
expectedMessagePart: "could not find end character of double-quoted text",
expectedMessagePart: "unclosed",
description: "unclosed quote should be detected",
},
{
Expand Down Expand Up @@ -181,7 +181,7 @@ features:
Invalid YAML with missing colon.`,
expectedErrorLine: 3, // Line 3 in file (line 2 in YAML content - permissions without colon)
expectedErrorColumn: 1,
expectedMessagePart: "unexpected key name",
expectedMessagePart: "Invalid YAML syntax",
description: "missing colon in mapping should be detected",
},
{
Expand All @@ -208,7 +208,7 @@ Invalid YAML with missing comma in array.`,
content: "---\non: push\npermissions:\n contents: read\n\tissues: write\nengine: claude\n---\n\n# Test Workflow\n\nInvalid YAML with mixed tabs and spaces.",
expectedErrorLine: 5, // Line 5 in file (line 4 in YAML content - the line with tab)
expectedErrorColumn: 1,
expectedMessagePart: "found character '\t' that cannot start any token",
expectedMessagePart: "Invalid YAML syntax",
description: "mixed tabs and spaces should be detected",
},
{
Expand Down Expand Up @@ -254,7 +254,7 @@ strict: false
Invalid YAML with malformed nested structure.`,
expectedErrorLine: 7, // Line 7 in file (line 6 in YAML content - claude: [)
expectedErrorColumn: 11,
expectedMessagePart: "sequence end token ']' not found",
expectedMessagePart: "unclosed bracket",
description: "invalid nested structure should be detected",
},
{
Expand Down Expand Up @@ -379,7 +379,7 @@ engine: copilot

Test content.`,
expectedLineCol: "[3:10]", // Line 3 in file (line 2 in YAML content)
expectedInError: []string{"mapping value is not allowed"},
expectedInError: []string{"Invalid YAML syntax"},
expectPointer: true,
description: "simple syntax error shows formatted output",
},
Expand Down Expand Up @@ -416,7 +416,7 @@ engine: copilot

Test content.`,
expectedLineCol: "[3:1]", // Line 3 in file (permissions without colon)
expectedInError: []string{"unexpected key name", "permissions"},
expectedInError: []string{"Invalid YAML syntax", "permissions"},
expectPointer: true,
description: "missing colon shows formatted output",
},
Expand Down
5 changes: 5 additions & 0 deletions pkg/workflow/data/action_pins.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@
"version": "v6",
"sha": "b7c566a772e6b6bfb58ed0dc250532a479d7789f"
},
"anchore/sbom-action@v0": {
"repo": "anchore/sbom-action",
"version": "v0",
"sha": "17ae1740179002c89186b61233e0f892c3118b11"
},
Comment on lines +113 to +117
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change: This addition of anchore/sbom-action@v0 to the action pins appears unrelated to the stated PR objective of improving YAML syntax error quality. The PR description focuses on error message translations and precise engine field location, but doesn't mention updating action pins.

If this change is necessary (e.g., to support a workflow that uses this action in tests), it should be explained in the PR description. Otherwise, consider removing it to keep the PR focused on a single concern.

Copilot uses AI. Check for mistakes.
"anchore/sbom-action@v0.22.2": {
"repo": "anchore/sbom-action",
"version": "v0.22.2",
Expand Down
Loading
Loading