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
48 changes: 29 additions & 19 deletions pkg/parser/yaml_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,42 +28,52 @@ var (
var yamlErrorTranslations = []struct {
pattern string
replacement string
example string
}{
{
"unexpected key name",
"missing ':' after key — YAML mapping entries require 'key: value' format",
pattern: "unexpected key name",
replacement: "missing ':' after key — YAML mapping entries require 'key: value' format",
example: "\n\nExample:\nengine: copilot",
},
{
"mapping value is not allowed in this context",
"unexpected ':' — check indentation or if this key belongs in a mapping block",
pattern: "mapping value is not allowed in this context",
replacement: "unexpected ':' — check indentation or if this key belongs in a mapping block",
example: "\n\nExample:\npermissions:\n contents: read",
},
{
"mapping values are not allowed",
"unexpected ':' — check indentation or if this key belongs in a mapping block",
pattern: "mapping values are not allowed",
replacement: "unexpected ':' — check indentation or if this key belongs in a mapping block",
example: "\n\nExample:\npermissions:\n contents: read",
},
{
"string was used where mapping is expected",
"expected a YAML mapping (key: value pairs) but got a plain string",
pattern: "string was used where mapping is expected",
replacement: "expected a YAML mapping (key: value pairs) but got a plain string",
example: "\n\nExample:\nengine: copilot",
},
{
"non-map value is specified",
"expected a YAML mapping (key: value pairs) — did you forget a colon after the key?",
pattern: "non-map value is specified",
replacement: "expected a YAML mapping (key: value pairs) — did you forget a colon after the key?",
example: "\n\nExample:\nengine: copilot",
},
{
"tab character cannot use as a map key directly",
"tab character in key — YAML requires spaces for indentation, not tabs",
pattern: "tab character cannot use as a map key directly",
replacement: "tab character in key — YAML requires spaces for indentation, not tabs",
example: "\n\nExample:\npermissions:\n contents: read",
},
{
"found character that cannot start any token",
"invalid character — check indentation uses spaces, not tabs",
pattern: "found character that cannot start any token",
replacement: "invalid character — check indentation uses spaces, not tabs",
example: "\n\nExample:\npermissions:\n contents: read",
},
{
"could not find expected ':'",
"missing ':' in key-value pair",
pattern: "could not find expected ':'",
replacement: "missing ':' in key-value pair",
example: "\n\nExample:\nengine: copilot",
},
{
"did not find expected key",
"incorrect indentation or missing key in mapping",
pattern: "did not find expected key",
replacement: "incorrect indentation or missing key in mapping",
example: "\n\nExample:\npermissions:\n contents: read",
},
}

Expand All @@ -80,7 +90,7 @@ func TranslateYAMLMessage(message string) string {
if idx := strings.Index(lower, t.pattern); idx >= 0 {
yamlErrorLog.Printf("Translating YAML message pattern %q", t.pattern)
// Slice using idx from the lowercase string. Safe because all patterns are ASCII.
return message[:idx] + t.replacement + message[idx+len(t.pattern):]
return message[:idx] + t.replacement + t.example + message[idx+len(t.pattern):]
}
}
return message
Expand Down
30 changes: 22 additions & 8 deletions pkg/parser/yaml_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,24 +242,28 @@ func TestTranslateYAMLMessage(t *testing.T) {
input string
contains string
excludes string
example string
}{
{
name: "unexpected key name",
input: "unexpected key name",
contains: "missing ':' after key",
excludes: "unexpected key name",
example: "Example:\nengine: copilot",
},
{
name: "non-map value is specified",
input: "non-map value is specified",
contains: "expected a YAML mapping",
excludes: "non-map value is specified",
example: "Example:\nengine: copilot",
},
{
name: "found character that cannot start any token",
input: "found character that cannot start any token",
contains: "invalid character",
excludes: "found character that cannot start any token",
example: "Example:\npermissions:\n contents: read",
},
{
name: "unrecognized message is unchanged",
Expand All @@ -284,6 +288,10 @@ func TestTranslateYAMLMessage(t *testing.T) {
assert.NotContains(t, result, tt.excludes,
"TranslateYAMLMessage should not contain %q\nResult: %s", tt.excludes, result)
}
if tt.example != "" {
assert.Contains(t, result, tt.example,
"TranslateYAMLMessage should contain fix example %q\nResult: %s", tt.example, result)
}
})
}
}
Expand All @@ -292,16 +300,18 @@ func TestTranslateYAMLMessage(t *testing.T) {
// to the underlying goccy/go-yaml error messages.
func TestFormatYAMLErrorTranslation(t *testing.T) {
tests := []struct {
name string
yamlContent string
shouldContain string
shouldExclude string
name string
yamlContent string
shouldContain string
shouldContainExtra string
shouldExclude string
}{
{
name: "missing colon translated",
yamlContent: "engine claude\nmodel: gpt-4",
shouldContain: "missing ':' after key",
shouldExclude: "unexpected key name",
name: "missing colon translated",
yamlContent: "engine claude\nmodel: gpt-4",
shouldContain: "missing ':' after key",
shouldContainExtra: "Example:\nengine: copilot",
shouldExclude: "unexpected key name",
},
{
name: "extra colon translated",
Expand All @@ -328,6 +338,10 @@ func TestFormatYAMLErrorTranslation(t *testing.T) {
formatted := FormatYAMLError(err, 1, tt.yamlContent)
assert.Contains(t, formatted, tt.shouldContain,
"FormatYAMLError should contain %q\nResult: %s", tt.shouldContain, formatted)
if tt.shouldContainExtra != "" {
assert.Contains(t, formatted, tt.shouldContainExtra,
"FormatYAMLError should contain %q\nResult: %s", tt.shouldContainExtra, formatted)
}
if tt.shouldExclude != "" {
assert.NotContains(t, formatted, tt.shouldExclude,
"FormatYAMLError should not contain %q\nResult: %s", tt.shouldExclude, formatted)
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/compiler_orchestrator_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error)
}
// Try to point at the exact line of the "engine:" field so the user can
// navigate directly to the problem location.
engineLine := findFrontmatterFieldLine(result.FrontmatterLines, result.FrontmatterStart, "engine")
engineLine, engineCol := findFrontmatterFieldValuePosition(result.FrontmatterLines, result.FrontmatterStart, "engine")
if engineLine > 0 {
// Read source context lines (±3 lines around the error) for Rust-style rendering
contextLines := readSourceContextLines(content, engineLine)
return nil, formatCompilerErrorWithContext(cleanPath, engineLine, 1, "error", err.Error(), err, contextLines)
return nil, formatCompilerErrorWithContext(cleanPath, engineLine, engineCol, "error", err.Error(), err, contextLines)
}
return nil, formatCompilerError(cleanPath, "error", err.Error(), err)
}
Expand Down
10 changes: 9 additions & 1 deletion pkg/workflow/compiler_string_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,15 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor
// Setup engine and process imports
engineSetup, err := c.setupEngineAndImports(parseResult.frontmatterResult, parseResult.cleanPath, parseResult.content, parseResult.markdownDir)
if err != nil {
return nil, err
if isFormattedCompilerError(err) {
return nil, err
}
engineLine, engineCol := findFrontmatterFieldValuePosition(result.FrontmatterLines, result.FrontmatterStart, "engine")
if engineLine > 0 {
contextLines := readSourceContextLines(parseResult.content, engineLine)
return nil, formatCompilerErrorWithContext(cleanPath, engineLine, engineCol, "error", err.Error(), err, contextLines)
}
return nil, formatCompilerError(cleanPath, "error", err.Error(), err)
}

// Process tools and markdown
Expand Down
23 changes: 23 additions & 0 deletions pkg/workflow/compiler_string_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,29 @@ on: {{{
require.Error(t, err)
}

func TestParseWorkflowString_InvalidEngineHasLocationContext(t *testing.T) {
markdown := `---
on: push
engine: claaude
---

# Broken
`

compiler := NewCompiler(
WithNoEmit(true),
WithSkipValidation(true),
)

_, err := compiler.ParseWorkflowString(markdown, "workflow.md")
require.Error(t, err)

errStr := err.Error()
assert.Contains(t, errStr, "workflow.md:3:9:", "invalid engine should include file:line:column location")
assert.Contains(t, errStr, "invalid engine:", "invalid engine should be preserved in message")
assert.Contains(t, errStr, "engine: claaude", "error should include source snippet context")
}

func TestParseWorkflowString_SharedWorkflowDetection(t *testing.T) {
// Shared workflows have no 'on' trigger field
markdown := `---
Expand Down
7 changes: 5 additions & 2 deletions pkg/workflow/compiler_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ engine: copilot

Test content.`,
expectedLineCol: "[3:1]", // Line 3 in file (permissions without colon)
expectedInError: []string{"missing ':' after key", "permissions"},
expectedInError: []string{"missing ':' after key", "permissions", "Example:\nengine: copilot"},
expectPointer: true,
description: "missing colon shows formatted output",
},
Expand Down Expand Up @@ -480,6 +480,7 @@ func TestEngineValidationErrorHasFileLocation(t *testing.T) {
name string
content string
expectedErrorLine int
expectedErrorCol int
description string
}{
{
Expand All @@ -493,6 +494,7 @@ engine: openai_gpt

Content.`,
expectedErrorLine: 3, // "engine: openai_gpt" is on line 3 in the file
expectedErrorCol: 9, // value starts after "engine: "
description: "invalid engine on line 3 should produce error at line 3",
},
{
Expand All @@ -509,6 +511,7 @@ strict: false

Content.`,
expectedErrorLine: 5, // "engine: badengine_xyz" is on line 5 in the file
expectedErrorCol: 9, // value starts after "engine: "
description: "invalid engine after permissions block should produce error at correct line",
},
}
Expand All @@ -530,7 +533,7 @@ Content.`,
errorStr := err.Error()

// Verify error contains filename:line:col: format pointing to the engine field
expectedPattern := fmt.Sprintf(".md:%d:1:", tt.expectedErrorLine)
expectedPattern := fmt.Sprintf(".md:%d:%d:", tt.expectedErrorLine, tt.expectedErrorCol)
if !strings.Contains(errorStr, expectedPattern) {
t.Errorf("%s: error should contain '%s' pointing to the engine: field, got: %s",
tt.description, expectedPattern, errorStr)
Expand Down
63 changes: 55 additions & 8 deletions pkg/workflow/frontmatter_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,39 @@ func readSourceContextLines(content []byte, targetLine int) []string {
// Only top-level (non-indented) keys are matched. Nested values that happen
// to contain the field name are ignored.
func findFrontmatterFieldLine(frontmatterLines []string, frontmatterStart int, fieldName string) int {
line, _ := findFrontmatterFieldValuePosition(frontmatterLines, frontmatterStart, fieldName)
return line
}

// findFrontmatterFieldValuePosition searches frontmatterLines for a top-level
// key that matches fieldName and returns the 1-based document line and column of
// the field value. For "engine: claaude", this points to the "c" in "claaude".
// Returns (0, 0) if the field is not found.
func findFrontmatterFieldValuePosition(frontmatterLines []string, frontmatterStart int, fieldName string) (int, int) {
prefix := fieldName + ":"
for i, line := range frontmatterLines {
// Match only non-indented lines so nested YAML values are not confused
// with top-level keys (e.g. " engine: ..." inside a mapping is ignored).
if strings.HasPrefix(line, prefix) {
return frontmatterStart + i
docLine := frontmatterStart + i
colonIndex := strings.Index(line, ":")
if colonIndex < 0 {
return docLine, 1
}

valueStart := colonIndex + 1 // 0-based index right after ':'
for valueStart < len(line) && line[valueStart] == ' ' {
valueStart++
}
if valueStart >= len(line) {
// No value present - point at the key/value separator.
return docLine, colonIndex + 1
}

return docLine, valueStart + 1 // convert to 1-based column
}
}
return 0
return 0, 0
}

// createFrontmatterError creates a detailed error for frontmatter parsing issues
Expand All @@ -87,13 +111,36 @@ func (c *Compiler) createFrontmatterError(filePath, content string, err error, f
line := matches[1]
col := matches[2]
message := matches[3]
// Extract just the first line of the message (before newline)
if idx := strings.Index(message, "\n"); idx != -1 {
message = message[:idx]

// Preserve any fix-guidance block that may be emitted after the header line
// (for example "Example:\nengine: copilot"), while keeping source context
// lines separate for Rust-style rendering below.
if _, parsedErrorBody, found := strings.Cut(errorStr, frontmatterParseErrPrefix); found {
messageSection := strings.TrimSpace(parsedErrorBody)
if loc := sourceContextPattern.FindStringIndex(parsedErrorBody); loc != nil {
messageSection = strings.TrimSpace(parsedErrorBody[:loc[0]])
}

headerLine := messageSection
trailingGuidance := ""
if firstLine, remainder, foundNewline := strings.Cut(messageSection, "\n"); foundNewline {
headerLine = firstLine
trailingGuidance = remainder
}
if headerMatches := lineColPattern.FindStringSubmatch(headerLine); len(headerMatches) >= 4 {
message = headerMatches[3]
} else {
message = strings.TrimSpace(headerLine)
}

// Translate raw YAML parser messages to user-friendly plain English.
// Uses the shared translation table from pkg/parser to keep both code paths in sync.
message = parser.TranslateYAMLMessage(message)

if guidance := strings.TrimSpace(trailingGuidance); guidance != "" {
message = message + "\n" + guidance
}
}
// Translate raw YAML parser messages to user-friendly plain English.
// Uses the shared translation table from pkg/parser to keep both code paths in sync.
message = parser.TranslateYAMLMessage(message)

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