From 3cb4a6fc1b5c04f163da11f915020cf4b430de73 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:09:30 +0000 Subject: [PATCH 1/6] Initial plan From fe517965f3e40191758a283f8e3cd9edcac5ae92 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:22:23 +0000 Subject: [PATCH 2/6] Fix error location and double check mark issues - Fix nested section indentation level calculation in json_path_locator.go - Remove redundant FormatErrorMessage wrapper in compile_helpers.go - Remove redundant FormatValidationError wrapper in main.go compile command - Update main error handler to detect console.FormatError pattern Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 21 ++++++++++++--------- pkg/cli/compile_helpers.go | 3 ++- pkg/parser/json_path_locator.go | 8 ++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 4ed02b5e3e..90dc5485f8 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -319,12 +319,9 @@ Examples: FailFast: failFast, } if _, err := cli.CompileWorkflows(cmd.Context(), config); err != nil { - // Format validation error for better user experience - // The main function will detect the ✗ prefix and print it without double formatting - if !jsonOutput { - // Return a new error with formatted message so main() can print it - return fmt.Errorf("%s", cli.FormatValidationError(err)) - } + // Return error as-is without additional formatting + // Errors from CompileWorkflows are already formatted with console.FormatError + // which provides IDE-parseable location information (file:line:column) return err } return nil @@ -685,9 +682,15 @@ func main() { if err := rootCmd.Execute(); err != nil { errMsg := err.Error() - // Check if error is already formatted (contains suggestions or starts with ✗) - // to avoid double formatting with FormatErrorMessage - if strings.Contains(errMsg, "Suggestions:") || strings.HasPrefix(errMsg, "✗") { + // Check if error is already formatted to avoid double formatting: + // - Contains suggestions (FormatErrorWithSuggestions) + // - Starts with ✗ (FormatErrorMessage) + // - Contains file:line:column: pattern (console.FormatError) + isAlreadyFormatted := strings.Contains(errMsg, "Suggestions:") || + strings.HasPrefix(errMsg, "✗") || + strings.Contains(errMsg, ":") && (strings.Contains(errMsg, "error:") || strings.Contains(errMsg, "warning:")) + + if isAlreadyFormatted { fmt.Fprintln(os.Stderr, errMsg) } else { fmt.Fprintln(os.Stderr, console.FormatErrorMessage(errMsg)) diff --git a/pkg/cli/compile_helpers.go b/pkg/cli/compile_helpers.go index 1c6dcca64b..b6176f8636 100644 --- a/pkg/cli/compile_helpers.go +++ b/pkg/cli/compile_helpers.go @@ -100,7 +100,8 @@ func compileSingleFile(compiler *workflow.Compiler, file string, stats *Compilat if err := CompileWorkflowWithValidation(compiler, file, verbose, false, false, false, false, false); err != nil { // Always show compilation errors on new line - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(err.Error())) + // Note: Don't wrap in FormatErrorMessage as the error is already formatted by console.FormatError + fmt.Fprintln(os.Stderr, err.Error()) stats.Errors++ stats.FailedWorkflows = append(stats.FailedWorkflows, filepath.Base(file)) } else { diff --git a/pkg/parser/json_path_locator.go b/pkg/parser/json_path_locator.go index e0b2146569..4d9fed80ee 100644 --- a/pkg/parser/json_path_locator.go +++ b/pkg/parser/json_path_locator.go @@ -398,12 +398,12 @@ func findNestedSection(yamlContent string, pathSegments []PathSegment) NestedSec if segment.Type == "key" { // Look for "key:" pattern keyPattern := regexp.MustCompile(`^` + regexp.QuoteMeta(segment.Value) + `\s*:`) - if keyPattern.MatchString(trimmedLine) && lineLevel == currentLevel*2 { + if keyPattern.MatchString(trimmedLine) && lineLevel == currentLevel { // Found a matching key at the correct indentation level if currentLevel == len(pathSegments)-1 { // This is the final segment - we found our target foundLine = lineNum - baseIndentLevel = lineLevel + 2 // Properties inside this object should be indented further + baseIndentLevel = lineLevel + 1 // Properties inside this object should be indented one level further break } else { // Move to the next level @@ -419,8 +419,8 @@ func findNestedSection(yamlContent string, pathSegments []PathSegment) NestedSec } // Find the end of this nested section by looking for the next line at the same or lower indentation - endLine := len(lines) - 1 // Default to end of file - targetLevel := baseIndentLevel - 2 // The level of the key we found + endLine := len(lines) - 1 // Default to end of file + targetLevel := baseIndentLevel - 1 // The level of the key we found for lineNum := foundLine + 1; lineNum < len(lines); lineNum++ { line := lines[lineNum] From 9f5bed1060756690fdf9d4173de5e53c2e450d97 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:25:14 +0000 Subject: [PATCH 3/6] Complete: Fix error location and double check mark issues All tests passing. Both issues resolved: - Error location now correct (line 6 instead of line 4) - No double check mark prefix (clean IDE-parseable format) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 2 +- pkg/parser/json_path_locator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 90dc5485f8..ef058469fe 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -689,7 +689,7 @@ func main() { isAlreadyFormatted := strings.Contains(errMsg, "Suggestions:") || strings.HasPrefix(errMsg, "✗") || strings.Contains(errMsg, ":") && (strings.Contains(errMsg, "error:") || strings.Contains(errMsg, "warning:")) - + if isAlreadyFormatted { fmt.Fprintln(os.Stderr, errMsg) } else { diff --git a/pkg/parser/json_path_locator.go b/pkg/parser/json_path_locator.go index 4d9fed80ee..c88cb0fd34 100644 --- a/pkg/parser/json_path_locator.go +++ b/pkg/parser/json_path_locator.go @@ -419,7 +419,7 @@ func findNestedSection(yamlContent string, pathSegments []PathSegment) NestedSec } // Find the end of this nested section by looking for the next line at the same or lower indentation - endLine := len(lines) - 1 // Default to end of file + endLine := len(lines) - 1 // Default to end of file targetLevel := baseIndentLevel - 1 // The level of the key we found for lineNum := foundLine + 1; lineNum < len(lines); lineNum++ { From ac40d83767ed9bd270a6e585817f11837681a9fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:46:10 +0000 Subject: [PATCH 4/6] Fix: Use absolute paths when relative paths contain ".." When a relative path contains "..", display the absolute path instead for better clarity. This prevents confusing paths like "../../../../../tmp/test.md" and shows "/tmp/test.md" instead. Files within the working directory still show clean relative paths like ".github/workflows/test.md". Addresses feedback from @pelikhan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/console/console.go | 6 ++++++ pkg/console/console_test.go | 24 +++++++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/console/console.go b/pkg/console/console.go index 1baf9e0e54..34f5232b20 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -47,6 +47,7 @@ func applyStyle(style lipgloss.Style, text string) string { } // ToRelativePath converts an absolute path to a relative path from the current working directory +// If the relative path contains "..", returns the absolute path instead for clarity func ToRelativePath(path string) string { if !filepath.IsAbs(path) { return path @@ -64,6 +65,11 @@ func ToRelativePath(path string) string { return path } + // If the relative path contains "..", use absolute path instead for clarity + if strings.Contains(relPath, "..") { + return path + } + return relPath } diff --git a/pkg/console/console_test.go b/pkg/console/console_test.go index d77cb372d9..8a45aca656 100644 --- a/pkg/console/console_test.go +++ b/pkg/console/console_test.go @@ -3,6 +3,7 @@ package console import ( + "os" "path/filepath" "strings" "testing" @@ -298,11 +299,28 @@ func TestToRelativePath(t *testing.T) { }, }, { - name: "absolute path converted to relative", + name: "absolute path with .. in relative form returns absolute", path: "/tmp/gh-aw/test.md", expectedFunc: func(result, expected string) bool { - // Should be a relative path that doesn't start with / - return !strings.HasPrefix(result, "/") && strings.HasSuffix(result, "test.md") + // When relative path would contain "..", should return absolute path + // The relative path from /home/runner/work/gh-aw/gh-aw to /tmp/gh-aw/test.md + // would be ../../../../../tmp/gh-aw/test.md (contains ..) + // So we should get the absolute path back + return result == "/tmp/gh-aw/test.md" + }, + }, + { + name: "absolute path within working directory converted to relative", + path: func() string { + // Get current working directory and construct a path within it + wd, _ := os.Getwd() + return filepath.Join(wd, "pkg/console/test.md") + }(), + expectedFunc: func(result, expected string) bool { + // Absolute path within working directory should be converted to relative + // without ".." in the path + // The result should not start with / and should not contain .. + return !strings.HasPrefix(result, "/") && !strings.Contains(result, "..") && strings.HasSuffix(result, "test.md") }, }, } From 49f1d580cbdd8a83598e151dffaf73594c287bec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:57:55 +0000 Subject: [PATCH 5/6] Add comprehensive tests for error location at various tree depths Added two new test files to validate error location accuracy: 1. safe_outputs_error_location_test.go: - 12 test cases for errors at depths 1-4 - Tests original bug case (depth 2 nested under missing-tool) - Tests complex workflows with multiple sections - Tests edge cases (empty sections, arrays, comments) 2. schema_validation_depth_test.go: - 6 end-to-end integration tests - Tests errors at root, safe-outputs, handlers, tools, and permissions - Validates complete error formatting pipeline All 18 new test cases pass, covering: - Depth 1: Root level properties - Depth 2: Properties in safe-outputs, tools, permissions - Depth 3: Properties in handlers (create-issue, create-discussion, github-token) - Depth 4: Very deeply nested properties Addresses feedback from @pelikhan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../safe_outputs_error_location_test.go | 387 ++++++++++++++++++ pkg/parser/schema_validation_depth_test.go | 152 +++++++ 2 files changed, 539 insertions(+) create mode 100644 pkg/parser/safe_outputs_error_location_test.go create mode 100644 pkg/parser/schema_validation_depth_test.go diff --git a/pkg/parser/safe_outputs_error_location_test.go b/pkg/parser/safe_outputs_error_location_test.go new file mode 100644 index 0000000000..47d667dd9a --- /dev/null +++ b/pkg/parser/safe_outputs_error_location_test.go @@ -0,0 +1,387 @@ +//go:build !integration + +package parser + +import ( + "testing" +) + +// TestSafeOutputsErrorLocationAtVariousDepths tests that syntax errors in safe-outputs +// are correctly reported at various nesting depths. This addresses the issue where +// nested properties under missing-tool were incorrectly pointing to parent keys. +func TestSafeOutputsErrorLocationAtVariousDepths(t *testing.T) { + tests := []struct { + name string + yamlContent string + jsonPath string + errorMessage string + expectedLine int + expectedCol int + description string + }{ + { + name: "depth 1 - direct child of safe-outputs", + yamlContent: `on: daily +safe-outputs: + invalid-prop: true`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': additional properties 'invalid-prop' not allowed", + expectedLine: 3, + expectedCol: 3, + description: "Error at first nesting level under safe-outputs", + }, + { + name: "depth 2 - nested under valid handler (original bug)", + yamlContent: `on: daily +safe-outputs: + create-discussion: + missing-tool: + create-discussion: true`, + jsonPath: "/safe-outputs/missing-tool", + errorMessage: "at '/safe-outputs/missing-tool': additional properties 'create-discussion' not allowed", + expectedLine: 5, + expectedCol: 5, + description: "The original bug - error should point to line 5, not line 3", + }, + { + name: "depth 2 - multiple invalid properties at same level", + yamlContent: `on: daily +safe-outputs: + create-issue: + invalid-handler: + title: test + invalid-nested: true`, + jsonPath: "/safe-outputs/invalid-handler", + errorMessage: "at '/safe-outputs/invalid-handler': additional properties 'invalid-nested' not allowed", + expectedLine: 6, + expectedCol: 5, + description: "Multiple properties at depth 2, error on second one", + }, + { + name: "depth 3 - deeply nested invalid property", + yamlContent: `on: daily +safe-outputs: + create-issue: + labels: + invalid-label-prop: value`, + jsonPath: "/safe-outputs/create-issue/labels", + errorMessage: "at '/safe-outputs/create-issue/labels': additional properties 'invalid-label-prop' not allowed", + expectedLine: 5, + expectedCol: 7, + description: "Error at third nesting level", + }, + { + name: "depth 1 - error at root safe-outputs with valid children present", + yamlContent: `on: daily +safe-outputs: + create-issue: + title: Test + unknown-prop: value`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': additional properties 'unknown-prop' not allowed", + expectedLine: 5, + expectedCol: 3, + description: "Error at root level but after valid nested structure", + }, + { + name: "depth 2 - error in first handler", + yamlContent: `on: daily +safe-outputs: + create-issue: + invalid-field: test + create-discussion: + title: Test`, + jsonPath: "/safe-outputs/create-issue", + errorMessage: "at '/safe-outputs/create-issue': additional properties 'invalid-field' not allowed", + expectedLine: 4, + expectedCol: 5, + description: "Error in first handler at depth 2", + }, + { + name: "depth 2 - error in middle handler", + yamlContent: `on: daily +safe-outputs: + create-issue: + title: Test + invalid-handler: + some-prop: value + create-discussion: + title: Test`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': additional properties 'invalid-handler' not allowed", + expectedLine: 5, + expectedCol: 3, + description: "Error on invalid handler name between valid handlers", + }, + { + name: "depth 2 - error in last handler", + yamlContent: `on: daily +safe-outputs: + create-issue: + title: Test + create-discussion: + invalid-prop: value`, + jsonPath: "/safe-outputs/create-discussion", + errorMessage: "at '/safe-outputs/create-discussion': additional properties 'invalid-prop' not allowed", + expectedLine: 6, + expectedCol: 5, + description: "Error in last handler", + }, + { + name: "depth 2 - multiple errors, should find first", + yamlContent: `on: daily +safe-outputs: + create-issue: + bad-handler-1: + prop: value + bad-handler-2: + prop: value`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': additional properties 'bad-handler-1', 'bad-handler-2' not allowed", + expectedLine: 4, + expectedCol: 3, + description: "Multiple errors, should report first one found", + }, + { + name: "depth 3 - nested in github-token config", + yamlContent: `on: daily +safe-outputs: + github-token: + permissions: + invalid-perm: write`, + jsonPath: "/safe-outputs/github-token/permissions", + errorMessage: "at '/safe-outputs/github-token/permissions': additional properties 'invalid-perm' not allowed", + expectedLine: 5, + expectedCol: 7, + description: "Error in deeply nested github-token permissions", + }, + { + name: "depth 2 - error with adjacent valid properties", + yamlContent: `on: daily +safe-outputs: + create-issue: + title: Valid Title + invalid-field: bad + labels: [bug]`, + jsonPath: "/safe-outputs/create-issue", + errorMessage: "at '/safe-outputs/create-issue': additional properties 'invalid-field' not allowed", + expectedLine: 5, + expectedCol: 5, + description: "Error between valid properties", + }, + { + name: "depth 4 - very deeply nested error", + yamlContent: `on: daily +safe-outputs: + create-issue: + labels: + - bug + - invalid: + nested-prop: value`, + jsonPath: "/safe-outputs/create-issue/labels/1", + errorMessage: "at '/safe-outputs/create-issue/labels/1': additional properties 'nested-prop' not allowed", + expectedLine: 7, + expectedCol: 11, + description: "Very deep nesting level error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + location := LocateJSONPathInYAMLWithAdditionalProperties(tt.yamlContent, tt.jsonPath, tt.errorMessage) + + if !location.Found { + t.Errorf("Expected to find error location, but Found=false") + return + } + + if location.Line != tt.expectedLine { + t.Errorf("Expected Line=%d, got Line=%d. %s", tt.expectedLine, location.Line, tt.description) + } + + if location.Column != tt.expectedCol { + t.Errorf("Expected Column=%d, got Column=%d. %s", tt.expectedCol, location.Column, tt.description) + } + }) + } +} + +// TestSafeOutputsErrorLocationWithComplexYAML tests error location in more realistic, +// complex workflow configurations +func TestSafeOutputsErrorLocationWithComplexYAML(t *testing.T) { + tests := []struct { + name string + yamlContent string + jsonPath string + errorMessage string + expectedLine int + expectedCol int + description string + }{ + { + name: "complex workflow with multiple sections", + yamlContent: `name: Complex Workflow +on: + push: + branches: [main] + pull_request: +permissions: + contents: read + issues: write +safe-outputs: + max: 10 + create-issue: + title: Bug Report + missing-tool: + create-discussion: true + create-discussion: + title: Discussion`, + jsonPath: "/safe-outputs/missing-tool", + errorMessage: "at '/safe-outputs/missing-tool': additional properties 'create-discussion' not allowed", + expectedLine: 14, + expectedCol: 5, + description: "Error in safe-outputs section of complex workflow", + }, + { + name: "safe-outputs with all valid handlers plus one invalid", + yamlContent: `on: daily +safe-outputs: + create-issue: + title: Issue + create-discussion: + title: Discussion + invalid-tool: + some-prop: value + github-token: + scopes: [repo]`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': additional properties 'invalid-tool' not allowed", + expectedLine: 7, + expectedCol: 3, + description: "Invalid handler among valid ones", + }, + { + name: "safe-outputs with comments and empty lines", + yamlContent: `on: daily +safe-outputs: + # Valid handler + create-issue: + title: Test + + # Invalid handler below + bad-handler: + prop: value`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': additional properties 'bad-handler' not allowed", + expectedLine: 8, + expectedCol: 3, + description: "Error location with comments and empty lines", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + location := LocateJSONPathInYAMLWithAdditionalProperties(tt.yamlContent, tt.jsonPath, tt.errorMessage) + + if !location.Found { + t.Errorf("Expected to find error location, but Found=false. %s", tt.description) + return + } + + if location.Line != tt.expectedLine { + t.Errorf("Expected Line=%d, got Line=%d. %s", tt.expectedLine, location.Line, tt.description) + } + + if location.Column != tt.expectedCol { + t.Errorf("Expected Column=%d, got Column=%d. %s", tt.expectedCol, location.Column, tt.description) + } + }) + } +} + +// TestSafeOutputsEdgeCases tests edge cases in error location detection +func TestSafeOutputsEdgeCases(t *testing.T) { + tests := []struct { + name string + yamlContent string + jsonPath string + errorMessage string + expectedLine int + expectedCol int + shouldFind bool + description string + }{ + { + name: "empty safe-outputs section", + yamlContent: `on: daily +safe-outputs:`, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': expected object, got null", + expectedLine: 2, + expectedCol: 14, // After "safe-outputs:" + shouldFind: true, + description: "Empty safe-outputs section", + }, + { + name: "safe-outputs with only whitespace", + yamlContent: `on: daily +safe-outputs: + `, + jsonPath: "/safe-outputs", + errorMessage: "at '/safe-outputs': expected object, got null", + expectedLine: 2, + expectedCol: 14, // After "safe-outputs:" + shouldFind: true, + description: "Safe-outputs with only whitespace", + }, + { + name: "deeply nested with arrays", + yamlContent: `on: daily +safe-outputs: + create-issue: + labels: + - bug + - feature + invalid: value`, + jsonPath: "/safe-outputs/create-issue", + errorMessage: "at '/safe-outputs/create-issue': additional properties 'invalid' not allowed", + expectedLine: 7, + expectedCol: 5, + shouldFind: true, + description: "Error after array property", + }, + { + name: "non-existent path", + yamlContent: `on: daily +safe-outputs: + create-issue: + title: Test`, + jsonPath: "/safe-outputs/nonexistent", + errorMessage: "at '/safe-outputs/nonexistent': additional properties 'bad' not allowed", + expectedLine: 1, + expectedCol: 1, + shouldFind: false, + description: "Path doesn't exist in YAML", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + location := LocateJSONPathInYAMLWithAdditionalProperties(tt.yamlContent, tt.jsonPath, tt.errorMessage) + + if location.Found != tt.shouldFind { + t.Errorf("Expected Found=%v, got Found=%v. %s", tt.shouldFind, location.Found, tt.description) + } + + if tt.shouldFind { + if location.Line != tt.expectedLine { + t.Errorf("Expected Line=%d, got Line=%d. %s", tt.expectedLine, location.Line, tt.description) + } + + if location.Column != tt.expectedCol { + t.Errorf("Expected Column=%d, got Column=%d. %s", tt.expectedCol, location.Column, tt.description) + } + } + }) + } +} diff --git a/pkg/parser/schema_validation_depth_test.go b/pkg/parser/schema_validation_depth_test.go new file mode 100644 index 0000000000..9948bdde5a --- /dev/null +++ b/pkg/parser/schema_validation_depth_test.go @@ -0,0 +1,152 @@ +//go:build !integration + +package parser + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" +) + +// TestSchemaValidationErrorLocationAtVariousDepths tests that schema validation errors +// are reported at the correct line and column for errors at different nesting depths. +// This is an end-to-end test that validates the integration of schema validation, +// error location detection, and error formatting. +func TestSchemaValidationErrorLocationAtVariousDepths(t *testing.T) { + tests := []struct { + name string + workflowContent string + expectedLine int + expectedProperty string + description string + }{ + { + name: "depth 1 - invalid property at root level", + workflowContent: `--- +on: daily +invalid-root-prop: value +--- +Test workflow.`, + expectedLine: 3, + expectedProperty: "invalid-root-prop", + description: "Error at root level of frontmatter", + }, + { + name: "depth 2 - invalid property in safe-outputs", + workflowContent: `--- +on: daily +safe-outputs: + invalid-handler: true +--- +Test workflow.`, + expectedLine: 4, + expectedProperty: "invalid-handler", + description: "Error at depth 2 in safe-outputs", + }, + { + name: "depth 3 - invalid property in safe-outputs handler", + workflowContent: `--- +on: daily +safe-outputs: + create-issue: + invalid-field: value +--- +Test workflow.`, + expectedLine: 5, + expectedProperty: "invalid-field", + description: "Error at depth 3 in safe-outputs handler", + }, + { + name: "depth 3 - the original bug case", + workflowContent: `--- +on: daily +safe-outputs: + create-discussion: + missing-tool: + create-discussion: true +--- +Test workflow.`, + expectedLine: 6, + expectedProperty: "create-discussion", + description: "Original bug - error in nested missing-tool", + }, + { + name: "depth 3 - invalid property in MCP server config", + workflowContent: `--- +on: daily +tools: + github: + invalid-config: value +--- +Test workflow.`, + expectedLine: 5, + expectedProperty: "invalid-config", + description: "Error at depth 3 in tools.github", + }, + { + name: "depth 2 - invalid property in permissions", + workflowContent: `--- +on: daily +permissions: + invalid-perm: write +--- +Test workflow.`, + expectedLine: 4, + expectedProperty: "invalid-perm", + description: "Error at depth 2 in permissions", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a temporary directory + tmpDir := testutil.TempDir(t, "test-*") + testFile := filepath.Join(tmpDir, "test.md") + + // Write the workflow content + err := os.WriteFile(testFile, []byte(tt.workflowContent), 0644) + if err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + // Extract frontmatter from the content + result, err := ExtractFrontmatterFromContent(tt.workflowContent) + if err != nil { + t.Fatalf("Failed to extract frontmatter: %v", err) + } + + // Validate the frontmatter - this will trigger schema validation with location + err = ValidateMainWorkflowFrontmatterWithSchemaAndLocation(result.Frontmatter, testFile) + + // We expect an error + if err == nil { + t.Fatalf("Expected schema validation error, got nil. %s", tt.description) + } + + // Check that the error message contains the expected line number + errorStr := err.Error() + expectedLocation := fmt.Sprintf("%s:%d:", filepath.Base(testFile), tt.expectedLine) + + if !strings.Contains(errorStr, expectedLocation) { + t.Errorf("Expected error to contain '%s', got:\n%s\n%s", expectedLocation, errorStr, tt.description) + } + + // Check that the error message contains the expected property name + if !strings.Contains(errorStr, tt.expectedProperty) { + t.Errorf("Expected error to contain property '%s', got:\n%s\n%s", tt.expectedProperty, errorStr, tt.description) + } + + // Verify the error message is properly formatted with context + if !strings.Contains(errorStr, "error:") { + t.Errorf("Expected error to be formatted with 'error:', got:\n%s\n%s", errorStr, tt.description) + } + + // Log the error for manual verification + t.Logf("✓ Correct location (line %d) for %s", tt.expectedLine, tt.description) + }) + } +} From f36ac764c4fff13eae1fe8e63f63d13b72a03be0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:59:26 +0000 Subject: [PATCH 6/6] Update console tests to expect absolute paths for files outside working directory Updated TestFormatErrorWithAbsolutePaths to expect absolute paths when files are outside the working directory, consistent with the new behavior of ToRelativePath() that avoids confusing paths with ".." components. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/console/console_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/console/console_test.go b/pkg/console/console_test.go index 8a45aca656..3be2edcd5e 100644 --- a/pkg/console/console_test.go +++ b/pkg/console/console_test.go @@ -354,13 +354,14 @@ func TestFormatErrorWithAbsolutePaths(t *testing.T) { // The output should contain test.md and line:column information if !strings.Contains(output, "test.md:5:10:") { - t.Errorf("Expected output to contain relative file path with line:column, got: %s", output) + t.Errorf("Expected output to contain file path with line:column, got: %s", output) } - // The output should not start with an absolute path (no leading /) + // Since tmpDir is outside the working directory (in /tmp), the path should be absolute + // to avoid confusing relative paths with ".." components lines := strings.Split(output, "\n") - if strings.HasPrefix(lines[0], "/") { - t.Errorf("Expected output to start with relative path, but found absolute path: %s", lines[0]) + if !strings.HasPrefix(lines[0], "/") { + t.Errorf("Expected output to start with absolute path for files outside working directory, got: %s", lines[0]) } // Should contain error message