diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index c0fca18022..d978e2e440 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 98ae6d9cc4..54e7080594 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,11 +36,48 @@ 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") // 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,7 +87,11 @@ func formatCompilerError(filePath string, errType string, message string) error Type: errType, Message: message, }) - return errors.New(formattedErr) + // Return custom error type that preserves chain and formats multi-line + return &compilerError{ + formatted: strings.TrimSuffix(formattedErr, "\n"), + cause: cause, + } } // formatCompilerMessage creates a formatted compiler message string (for warnings printed to stderr) @@ -84,8 +124,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 (don't wrap - this creates user-facing errors) + return formatCompilerError(markdownPath, "error", err.Error(), nil) } return c.CompileWorkflowData(workflowData, markdownPath) @@ -97,7 +137,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 +147,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 +162,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 +173,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 +185,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 +247,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 +266,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 +298,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 +317,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 +325,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 +338,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 +352,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 +373,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 +417,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 01dd1ba393..eb6488b83c 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 + require.ErrorIs(t, 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 {