From 9e48a19a33536adcb26d62921de745bf58081fc0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 23:08:43 +0000 Subject: [PATCH 1/5] Initial plan From 4d1253b57d1d77224e113a5a0c251eaa0b32911e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 23:17:23 +0000 Subject: [PATCH 2/5] Update formatCompilerError to wrap cause errors with %w Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/agent_validation.go | 6 +-- pkg/workflow/compiler.go | 52 +++++++++--------- .../compiler_error_formatting_test.go | 54 ++++++++++++++++--- 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index c0fca18022e..d978e2e440b 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -84,11 +84,11 @@ func (c *Compiler) validateAgentFile(workflowData *WorkflowData, markdownPath st if _, err := os.Stat(fullAgentPath); err != nil { if os.IsNotExist(err) { return formatCompilerError(markdownPath, "error", - fmt.Sprintf("agent file '%s' does not exist. Ensure the file exists in the repository and is properly imported.", agentPath)) + fmt.Sprintf("agent file '%s' does not exist. Ensure the file exists in the repository and is properly imported.", agentPath), nil) } // Other error (permissions, etc.) return formatCompilerError(markdownPath, "error", - fmt.Sprintf("failed to access agent file '%s': %v", agentPath, err)) + fmt.Sprintf("failed to access agent file '%s'", agentPath), err) } if c.verbose { @@ -228,7 +228,7 @@ func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markd if c.strictMode { // In strict mode, this is an error - return formatCompilerError(markdownPath, "error", message) + return formatCompilerError(markdownPath, "error", message, nil) } // In normal mode, this is a warning diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 98ae6d9cc4c..85059e92374 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -41,7 +41,8 @@ var githubWorkflowSchema string // filePath: the file path to include in the error (typically markdownPath or lockFile) // errType: the error type ("error" or "warning") // message: the error message text -func formatCompilerError(filePath string, errType string, message string) error { +// cause: optional error to wrap (use nil for validation errors that create new messages) +func formatCompilerError(filePath string, errType string, message string, cause error) error { formattedErr := console.FormatError(console.CompilerError{ Position: console.ErrorPosition{ File: filePath, @@ -51,6 +52,9 @@ func formatCompilerError(filePath string, errType string, message string) error Type: errType, Message: message, }) + if cause != nil { + return fmt.Errorf("%s: %w", formattedErr, cause) + } return errors.New(formattedErr) } @@ -84,8 +88,8 @@ func (c *Compiler) CompileWorkflow(markdownPath string) error { // Already formatted, return as-is return err } - // Otherwise, create a basic formatted error - return formatCompilerError(markdownPath, "error", err.Error()) + // Otherwise, create a basic formatted error and wrap the cause + return formatCompilerError(markdownPath, "error", "failed to parse workflow file", err) } return c.CompileWorkflowData(workflowData, markdownPath) @@ -97,7 +101,7 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath // Validate expression safety - check that all GitHub Actions expressions are in the allowed list log.Printf("Validating expression safety") if err := validateExpressionSafety(workflowData.MarkdownContent); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "expression safety validation failed", err) } // Validate expressions in runtime-import files at compile time @@ -107,13 +111,13 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath githubDir := filepath.Dir(workflowDir) // .github workspaceDir := filepath.Dir(githubDir) // repo root if err := validateRuntimeImportFiles(workflowData.MarkdownContent, workspaceDir); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "runtime-import validation failed", err) } // Validate feature flags log.Printf("Validating feature flags") if err := validateFeatures(workflowData); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "feature flag validation failed", err) } // Check for action-mode feature flag override @@ -122,7 +126,7 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath if actionModeStr, ok := actionModeVal.(string); ok && actionModeStr != "" { mode := ActionMode(actionModeStr) if !mode.IsValid() { - return formatCompilerError(markdownPath, "error", fmt.Sprintf("invalid action-mode feature flag '%s'. Must be 'dev', 'release', or 'script'", actionModeStr)) + return formatCompilerError(markdownPath, "error", fmt.Sprintf("invalid action-mode feature flag '%s'. Must be 'dev', 'release', or 'script'", actionModeStr), nil) } log.Printf("Overriding action mode from feature flag: %s", mode) c.SetActionMode(mode) @@ -133,7 +137,7 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath // Validate dangerous permissions log.Printf("Validating dangerous permissions") if err := validateDangerousPermissions(workflowData); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "dangerous permissions validation failed", err) } // Validate agent file exists if specified in engine config @@ -145,25 +149,25 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath // Validate sandbox configuration log.Printf("Validating sandbox configuration") if err := validateSandboxConfig(workflowData); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "sandbox configuration validation failed", err) } // Validate safe-outputs target configuration log.Printf("Validating safe-outputs target fields") if err := validateSafeOutputsTarget(workflowData.SafeOutputs); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "safe-outputs target validation failed", err) } // Validate safe-outputs allowed-domains configuration log.Printf("Validating safe-outputs allowed-domains") if err := c.validateSafeOutputsAllowedDomains(workflowData.SafeOutputs); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "safe-outputs allowed-domains validation failed", err) } // Validate network allowed domains configuration log.Printf("Validating network allowed domains") if err := c.validateNetworkAllowedDomains(workflowData.NetworkPermissions); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "network allowed-domains validation failed", err) } // Emit experimental warning for sandbox-runtime feature @@ -207,7 +211,7 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath if len(validationResult.MissingPermissions) > 0 { if c.strictMode { // In strict mode, missing permissions are errors - return formatCompilerError(markdownPath, "error", message) + return formatCompilerError(markdownPath, "error", message, nil) } else { // In non-strict mode, missing permissions are warnings fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", message)) @@ -226,7 +230,7 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath // Validate that all allowed tools have their toolsets enabled if err := ValidateGitHubToolsAgainstToolsets(allowedTools, enabledToolsets); err != nil { - return formatCompilerError(markdownPath, "error", err.Error()) + return formatCompilerError(markdownPath, "error", "GitHub tools validation failed", err) } // Print informational message if "projects" toolset is explicitly specified @@ -258,14 +262,14 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath message += "permissions:\n" message += " actions: read" - return formatCompilerError(markdownPath, "error", message) + return formatCompilerError(markdownPath, "error", message, nil) } } // Validate dispatch-workflow configuration (independent of agentic-workflows tool) log.Print("Validating dispatch-workflow configuration") if err := c.validateDispatchWorkflow(workflowData, markdownPath); err != nil { - return formatCompilerError(markdownPath, "error", fmt.Sprintf("dispatch-workflow validation failed: %v", err)) + return formatCompilerError(markdownPath, "error", "dispatch-workflow validation failed", err) } return nil @@ -277,7 +281,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP // Generate the YAML content yamlContent, err := c.generateYAML(workflowData, markdownPath) if err != nil { - return "", formatCompilerError(markdownPath, "error", fmt.Sprintf("failed to generate YAML: %v", err)) + return "", formatCompilerError(markdownPath, "error", "failed to generate YAML", err) } // Always validate expression sizes - this is a hard limit from GitHub Actions (21KB) @@ -285,7 +289,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP log.Print("Validating expression sizes") if err := c.validateExpressionSizes(yamlContent); err != nil { // Store error first so we can write invalid YAML before returning - formattedErr := formatCompilerError(markdownPath, "error", fmt.Sprintf("expression size validation failed: %v", err)) + formattedErr := formatCompilerError(markdownPath, "error", "expression size validation failed", err) // Write the invalid YAML to a .invalid.yml file for inspection invalidFile := strings.TrimSuffix(lockFile, ".lock.yml") + ".invalid.yml" if writeErr := os.WriteFile(invalidFile, []byte(yamlContent), 0644); writeErr == nil { @@ -298,7 +302,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP log.Print("Validating for template injection vulnerabilities") if err := validateNoTemplateInjection(yamlContent); err != nil { // Store error first so we can write invalid YAML before returning - formattedErr := formatCompilerError(markdownPath, "error", err.Error()) + formattedErr := formatCompilerError(markdownPath, "error", "template injection validation failed", err) // Write the invalid YAML to a .invalid.yml file for inspection invalidFile := strings.TrimSuffix(lockFile, ".lock.yml") + ".invalid.yml" if writeErr := os.WriteFile(invalidFile, []byte(yamlContent), 0644); writeErr == nil { @@ -312,7 +316,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP log.Print("Validating workflow against GitHub Actions schema") if err := c.validateGitHubActionsSchema(yamlContent); err != nil { // Store error first so we can write invalid YAML before returning - formattedErr := formatCompilerError(markdownPath, "error", fmt.Sprintf("workflow schema validation failed: %v", err)) + formattedErr := formatCompilerError(markdownPath, "error", "workflow schema validation failed", err) // Write the invalid YAML to a .invalid.yml file for inspection invalidFile := strings.TrimSuffix(lockFile, ".lock.yml") + ".invalid.yml" if writeErr := os.WriteFile(invalidFile, []byte(yamlContent), 0644); writeErr == nil { @@ -333,19 +337,19 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP // Validate runtime packages (npx, uv) log.Print("Validating runtime packages") if err := c.validateRuntimePackages(workflowData); err != nil { - return "", formatCompilerError(markdownPath, "error", fmt.Sprintf("runtime package validation failed: %v", err)) + return "", formatCompilerError(markdownPath, "error", "runtime package validation failed", err) } // Validate firewall configuration (log-level enum) log.Print("Validating firewall configuration") if err := c.validateFirewallConfig(workflowData); err != nil { - return "", formatCompilerError(markdownPath, "error", fmt.Sprintf("firewall configuration validation failed: %v", err)) + return "", formatCompilerError(markdownPath, "error", "firewall configuration validation failed", err) } // Validate repository features (discussions, issues) log.Print("Validating repository features") if err := c.validateRepositoryFeatures(workflowData); err != nil { - return "", formatCompilerError(markdownPath, "error", fmt.Sprintf("repository feature validation failed: %v", err)) + return "", formatCompilerError(markdownPath, "error", "repository feature validation failed", err) } } else if c.verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Schema validation available but skipped (use SetSkipValidation(false) to enable)")) @@ -377,7 +381,7 @@ func (c *Compiler) writeWorkflowOutput(lockFile, yamlContent string, markdownPat // Only write if content has changed if !contentUnchanged { if err := os.WriteFile(lockFile, []byte(yamlContent), 0644); err != nil { - return formatCompilerError(lockFile, "error", fmt.Sprintf("failed to write lock file: %v", err)) + return formatCompilerError(lockFile, "error", "failed to write lock file", err) } log.Print("Lock file written successfully") } diff --git a/pkg/workflow/compiler_error_formatting_test.go b/pkg/workflow/compiler_error_formatting_test.go index 01dd1ba393c..320d9130492 100644 --- a/pkg/workflow/compiler_error_formatting_test.go +++ b/pkg/workflow/compiler_error_formatting_test.go @@ -3,6 +3,7 @@ package workflow import ( + "errors" "testing" "github.com/stretchr/testify/assert" @@ -16,13 +17,15 @@ func TestFormatCompilerError(t *testing.T) { filePath string errType string message string + cause error wantContain []string }{ { - name: "error type with simple message", + name: "error type with simple message and no cause", filePath: "/path/to/workflow.md", errType: "error", message: "validation failed", + cause: nil, wantContain: []string{ "/path/to/workflow.md", "1:1", @@ -30,11 +33,26 @@ func TestFormatCompilerError(t *testing.T) { "validation failed", }, }, + { + name: "error type with cause wrapping", + filePath: "/path/to/workflow.md", + errType: "error", + message: "validation failed", + cause: errors.New("underlying error"), + wantContain: []string{ + "/path/to/workflow.md", + "1:1", + "error", + "validation failed", + "underlying error", + }, + }, { name: "warning type with detailed message", filePath: "/path/to/workflow.md", errType: "warning", message: "missing required permission", + cause: nil, wantContain: []string{ "/path/to/workflow.md", "1:1", @@ -47,6 +65,7 @@ func TestFormatCompilerError(t *testing.T) { filePath: "/path/to/workflow.lock.yml", errType: "error", message: "failed to write lock file", + cause: nil, wantContain: []string{ "/path/to/workflow.lock.yml", "1:1", @@ -58,19 +77,21 @@ func TestFormatCompilerError(t *testing.T) { name: "formatted message with error details", filePath: "test.md", errType: "error", - message: "failed to generate YAML: syntax error", + message: "failed to generate YAML", + cause: errors.New("syntax error"), wantContain: []string{ "test.md", "1:1", "error", - "failed to generate YAML: syntax error", + "failed to generate YAML", + "syntax error", }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := formatCompilerError(tt.filePath, tt.errType, tt.message) + err := formatCompilerError(tt.filePath, tt.errType, tt.message, tt.cause) require.Error(t, err, "formatCompilerError should return an error") errStr := err.Error() @@ -83,7 +104,7 @@ func TestFormatCompilerError(t *testing.T) { // TestFormatCompilerError_OutputFormat verifies the output format remains consistent func TestFormatCompilerError_OutputFormat(t *testing.T) { - err := formatCompilerError("/test/workflow.md", "error", "test message") + err := formatCompilerError("/test/workflow.md", "error", "test message", nil) require.Error(t, err) errStr := err.Error() @@ -97,8 +118,8 @@ func TestFormatCompilerError_OutputFormat(t *testing.T) { // TestFormatCompilerError_ErrorVsWarning tests differentiation between error and warning types func TestFormatCompilerError_ErrorVsWarning(t *testing.T) { - errorErr := formatCompilerError("test.md", "error", "error message") - warningErr := formatCompilerError("test.md", "warning", "warning message") + errorErr := formatCompilerError("test.md", "error", "error message", nil) + warningErr := formatCompilerError("test.md", "warning", "warning message", nil) require.Error(t, errorErr) require.Error(t, warningErr) @@ -110,6 +131,25 @@ func TestFormatCompilerError_ErrorVsWarning(t *testing.T) { assert.NotEqual(t, errorErr.Error(), warningErr.Error(), "Error and warning should have different outputs") } +// TestFormatCompilerError_ErrorChaining tests that error chains are preserved +func TestFormatCompilerError_ErrorChaining(t *testing.T) { + originalErr := errors.New("original error") + wrappedErr := formatCompilerError("test.md", "error", "validation failed", originalErr) + + require.Error(t, wrappedErr) + + // Verify the error message contains both the formatted message and the original error + assert.Contains(t, wrappedErr.Error(), "validation failed", "Should contain formatted message") + assert.Contains(t, wrappedErr.Error(), "original error", "Should contain original error") + + // Verify error chain is preserved using errors.Is + assert.True(t, errors.Is(wrappedErr, originalErr), "Should preserve error chain") + + // Verify error chain can be unwrapped + unwrapped := errors.Unwrap(wrappedErr) + assert.Equal(t, originalErr, unwrapped, "Should unwrap to original error") +} + // TestFormatCompilerMessage tests the formatCompilerMessage helper function func TestFormatCompilerMessage(t *testing.T) { tests := []struct { From 517914641b6fe8912b5fe832c76ac8fc782f4540 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 23:19:24 +0000 Subject: [PATCH 3/5] Fix selective error wrapping - don't expose internal error types Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 85059e92374..1039ca18294 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -88,8 +88,8 @@ func (c *Compiler) CompileWorkflow(markdownPath string) error { // Already formatted, return as-is return err } - // Otherwise, create a basic formatted error and wrap the cause - return formatCompilerError(markdownPath, "error", "failed to parse workflow file", err) + // Otherwise, create a basic formatted error (don't wrap - this creates user-facing errors) + return formatCompilerError(markdownPath, "error", err.Error(), nil) } return c.CompileWorkflowData(workflowData, markdownPath) From defb373a0d1829a27ee9dc8372d76eac84652beb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 23:23:39 +0000 Subject: [PATCH 4/5] Fix linter issues in error formatting tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_error_formatting_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/compiler_error_formatting_test.go b/pkg/workflow/compiler_error_formatting_test.go index 320d9130492..eb6488b83c2 100644 --- a/pkg/workflow/compiler_error_formatting_test.go +++ b/pkg/workflow/compiler_error_formatting_test.go @@ -143,7 +143,7 @@ func TestFormatCompilerError_ErrorChaining(t *testing.T) { assert.Contains(t, wrappedErr.Error(), "original error", "Should contain original error") // Verify error chain is preserved using errors.Is - assert.True(t, errors.Is(wrappedErr, originalErr), "Should preserve error chain") + require.ErrorIs(t, wrappedErr, originalErr, "Should preserve error chain") // Verify error chain can be unwrapped unwrapped := errors.Unwrap(wrappedErr) From e0b563f3034975b0c96bdbff1efe2b5ced34074c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 8 Feb 2026 02:19:10 +0000 Subject: [PATCH 5/5] Format errors with IDE-friendly multi-line output Changed formatCompilerError to use custom error type that: - Keeps IDE-parseable first line (file:line:col: type: message) - Adds nested errors on separate lines with indentation - Preserves error chain for errors.Is() and errors.As() This addresses the feedback that errors must be one per line and IDE-friendly. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler.go | 44 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 1039ca18294..54e70805947 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -2,7 +2,6 @@ package workflow import ( _ "embed" - "errors" "fmt" "os" "path/filepath" @@ -37,6 +36,42 @@ const ( //go:embed schemas/github-workflow.json var githubWorkflowSchema string +// compilerError is a custom error type that formats errors with IDE-friendly multi-line output +// while preserving the error chain for errors.Is() and errors.As() +type compilerError struct { + formatted string + cause error +} + +func (e *compilerError) Error() string { + if e.cause == nil { + return e.formatted + } + // Format with cause on separate line(s) for IDE-friendliness + var builder strings.Builder + builder.WriteString(e.formatted) + builder.WriteString("\n") + + // Add each error in the chain on a new line with indentation + causeStr := e.cause.Error() + lines := strings.Split(causeStr, "\n") + for _, line := range lines { + if line != "" { + builder.WriteString(" ") + builder.WriteString(line) + builder.WriteString("\n") + } + } + + // Remove trailing newline + result := builder.String() + return strings.TrimSuffix(result, "\n") +} + +func (e *compilerError) Unwrap() error { + return e.cause +} + // formatCompilerError creates a formatted compiler error message // filePath: the file path to include in the error (typically markdownPath or lockFile) // errType: the error type ("error" or "warning") @@ -52,10 +87,11 @@ func formatCompilerError(filePath string, errType string, message string, cause Type: errType, Message: message, }) - if cause != nil { - return fmt.Errorf("%s: %w", formattedErr, cause) + // Return custom error type that preserves chain and formats multi-line + return &compilerError{ + formatted: strings.TrimSuffix(formattedErr, "\n"), + cause: cause, } - return errors.New(formattedErr) } // formatCompilerMessage creates a formatted compiler message string (for warnings printed to stderr)