From 37c43f633286c08572a356a7d84ab2d13387e058 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Feb 2026 02:43:53 +0000 Subject: [PATCH 1/3] Initial plan From f5f5dc31f1bfd49029e16c8b80af5e4656f2a2ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Feb 2026 02:56:49 +0000 Subject: [PATCH 2/3] Initial plan for YAML error message quality improvements Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/data/action_pins.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/workflow/data/action_pins.json b/pkg/workflow/data/action_pins.json index 8366ec146ee..6cc217598c4 100644 --- a/pkg/workflow/data/action_pins.json +++ b/pkg/workflow/data/action_pins.json @@ -110,6 +110,11 @@ "version": "v6", "sha": "b7c566a772e6b6bfb58ed0dc250532a479d7789f" }, + "anchore/sbom-action@v0": { + "repo": "anchore/sbom-action", + "version": "v0", + "sha": "17ae1740179002c89186b61233e0f892c3118b11" + }, "anchore/sbom-action@v0.22.2": { "repo": "anchore/sbom-action", "version": "v0.22.2", From 14ea97cb22effd2fe5d5ca8828ef8f449e2944f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Feb 2026 03:19:19 +0000 Subject: [PATCH 3/3] Improve YAML syntax error messages: translations, engine location fix, test updates - pkg/parser/yaml_error.go: Add yamlParserMessageTranslations and translateYAMLFormattedOutput() to translate raw goccy messages in FormatYAMLError() - pkg/workflow/frontmatter_error.go: Add missing translations for actual goccy v1.19 error patterns - pkg/workflow/compiler_orchestrator_engine.go: Use LocateJSONPathInYAML to emit precise file:line:col for engine validation errors - Tests: Update to check translated messages; add TestFormatYAMLErrorTranslations and TestSetupEngineAndImports_InvalidEngine_HasPreciseLocation" Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/frontmatter_syntax_errors_test.go | 10 +-- pkg/parser/yaml_error.go | 53 +++++++++++++++ pkg/parser/yaml_error_test.go | 64 ++++++++++++++++++- pkg/workflow/compiler_orchestrator_engine.go | 11 ++++ .../compiler_orchestrator_engine_test.go | 38 +++++++++++ .../compiler_orchestrator_workflow_test.go | 2 +- pkg/workflow/compiler_yaml_test.go | 16 ++--- pkg/workflow/frontmatter_error.go | 52 ++++++++++++++- pkg/workflow/yaml_message_translation_test.go | 35 ++++++++++ 9 files changed, 264 insertions(+), 17 deletions(-) diff --git a/pkg/parser/frontmatter_syntax_errors_test.go b/pkg/parser/frontmatter_syntax_errors_test.go index 17d4bf2539b..a56622b1b5e 100644 --- a/pkg/parser/frontmatter_syntax_errors_test.go +++ b/pkg/parser/frontmatter_syntax_errors_test.go @@ -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", }, { @@ -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", }, { @@ -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", }, { @@ -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", }, { @@ -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", }, { diff --git a/pkg/parser/yaml_error.go b/pkg/parser/yaml_error.go index c5d39819989..e2f89f6897e 100644 --- a/pkg/parser/yaml_error.go +++ b/pkg/parser/yaml_error.go @@ -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 @@ -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) diff --git a/pkg/parser/yaml_error_test.go b/pkg/parser/yaml_error_test.go index 59d69266bcb..86d42b32ecc 100644 --- a/pkg/parser/yaml_error_test.go +++ b/pkg/parser/yaml_error_test.go @@ -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" diff --git a/pkg/workflow/compiler_orchestrator_engine.go b/pkg/workflow/compiler_orchestrator_engine.go index 18b4bbfba44..5f9592d0967 100644 --- a/pkg/workflow/compiler_orchestrator_engine.go +++ b/pkg/workflow/compiler_orchestrator_engine.go @@ -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 } diff --git a/pkg/workflow/compiler_orchestrator_engine_test.go b/pkg/workflow/compiler_orchestrator_engine_test.go index b4a99f94f61..2fa67e0b336 100644 --- a/pkg/workflow/compiler_orchestrator_engine_test.go +++ b/pkg/workflow/compiler_orchestrator_engine_test.go @@ -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 diff --git a/pkg/workflow/compiler_orchestrator_workflow_test.go b/pkg/workflow/compiler_orchestrator_workflow_test.go index df53d735ca9..90c8c7115ba 100644 --- a/pkg/workflow/compiler_orchestrator_workflow_test.go +++ b/pkg/workflow/compiler_orchestrator_workflow_test.go @@ -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", diff --git a/pkg/workflow/compiler_yaml_test.go b/pkg/workflow/compiler_yaml_test.go index 523c6b34c22..332674d134e 100644 --- a/pkg/workflow/compiler_yaml_test.go +++ b/pkg/workflow/compiler_yaml_test.go @@ -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", }, { @@ -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", }, { @@ -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", }, { @@ -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", }, { @@ -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", }, { @@ -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", }, { @@ -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", }, @@ -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", }, diff --git a/pkg/workflow/frontmatter_error.go b/pkg/workflow/frontmatter_error.go index a702fef02cd..b33dbaa3657 100644 --- a/pkg/workflow/frontmatter_error.go +++ b/pkg/workflow/frontmatter_error.go @@ -18,22 +18,70 @@ var ( // 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. +// Patterns must match actual strings produced by goccy/go-yaml v1.19+; both singular and +// legacy plural forms are kept for broad compatibility. var yamlErrorTranslations = []struct { pattern string translation string }{ + // Colon in wrong context (actual goccy v1.19 message uses singular "value") { - "non-map value is specified", - "Invalid YAML syntax: expected 'key: value' format (did you forget a colon after the key?)", + "mapping value is not allowed in this context", + "Invalid YAML syntax: unexpected ':' — check indentation or key syntax", }, + // Legacy plural form kept for tests and older goccy versions { "mapping values are not allowed", "Invalid YAML syntax: unexpected ':' — check your indentation", }, + // Bare key without colon 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 without colon (e.g. "engine copilot") + { + "unexpected key name", + "Invalid YAML syntax: expected 'key: value' format (did you forget a colon after the key?)", + }, + // Generic "did not find expected" catch-all (kept for backward compatibility) { "did not find expected", "Invalid YAML syntax: check indentation or missing key", }, + // Tab character errors; goccy v1.19 uses an actual tab char (0x09) inside 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", + }, } // translateYAMLMessage converts raw YAML parser messages to user-friendly plain English. diff --git a/pkg/workflow/yaml_message_translation_test.go b/pkg/workflow/yaml_message_translation_test.go index 033e628636d..a9e24e7cb46 100644 --- a/pkg/workflow/yaml_message_translation_test.go +++ b/pkg/workflow/yaml_message_translation_test.go @@ -28,12 +28,47 @@ func TestTranslateYAMLMessage(t *testing.T) { wantNot: []string{"mapping values are not allowed"}, wantAny: []string{"Invalid YAML syntax", "indentation"}, }, + { + // Actual goccy v1.19 singular form + name: "mapping value (singular) not allowed translated", + input: "mapping value is not allowed in this context", + wantNot: []string{"mapping value is not allowed"}, + wantAny: []string{"Invalid YAML syntax", "indentation"}, + }, + { + // goccy "unexpected key name" for bare keys without colon + name: "unexpected key name translated", + input: "unexpected key name", + wantNot: []string{"unexpected key name"}, + wantAny: []string{"Invalid YAML syntax", "key: value"}, + }, { name: "did not find expected translated", input: "did not find expected key", wantNot: []string{"did not find expected"}, wantAny: []string{"Invalid YAML syntax"}, }, + { + // Tab indentation error from goccy + name: "cannot start any token translated", + input: "found character ' ' that cannot start any token", + wantNot: []string{"cannot start any token"}, + wantAny: []string{"Invalid YAML syntax", "spaces", "tabs"}, + }, + { + // Block sequence in wrong place + name: "block sequence entries not allowed translated", + input: "block sequence entries are not allowed in this context", + wantNot: []string{"block sequence entries are not allowed"}, + wantAny: []string{"Invalid YAML syntax"}, + }, + { + // Unclosed bracket + name: "sequence end token not found translated", + input: "sequence end token ']' not found", + wantNot: []string{"sequence end token"}, + wantAny: []string{"Invalid YAML syntax", "unclosed"}, + }, { name: "unrecognized message returned unchanged", input: "found unknown escape character 'z'",