diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index c0fca18022..241d43c4c1 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': %v", agentPath, err), 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..39a3828957 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -37,11 +37,12 @@ const ( //go:embed schemas/github-workflow.json var githubWorkflowSchema string -// formatCompilerError creates a formatted compiler error message +// formatCompilerError creates a formatted compiler error message with optional error wrapping // 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 underlying error to wrap (use nil for validation errors) +func formatCompilerError(filePath string, errType string, message string, cause error) error { formattedErr := console.FormatError(console.CompilerError{ Position: console.ErrorPosition{ File: filePath, @@ -51,6 +52,13 @@ func formatCompilerError(filePath string, errType string, message string) error Type: errType, Message: message, }) + + // Wrap the underlying error if provided (preserves error chain) + if cause != nil { + return fmt.Errorf("%s: %w", formattedErr, cause) + } + + // Create new error for validation errors (no underlying cause) return errors.New(formattedErr) } @@ -84,8 +92,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 with wrapping + return formatCompilerError(markdownPath, "error", err.Error(), err) } return c.CompileWorkflowData(workflowData, markdownPath) @@ -97,7 +105,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", err.Error(), err) } // Validate expressions in runtime-import files at compile time @@ -107,13 +115,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", err.Error(), 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", err.Error(), err) } // Check for action-mode feature flag override @@ -122,7 +130,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 +141,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", err.Error(), err) } // Validate agent file exists if specified in engine config @@ -145,25 +153,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", err.Error(), 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", err.Error(), 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", err.Error(), 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", err.Error(), err) } // Emit experimental warning for sandbox-runtime feature @@ -207,7 +215,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 +234,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", err.Error(), err) } // Print informational message if "projects" toolset is explicitly specified @@ -258,14 +266,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", fmt.Sprintf("dispatch-workflow validation failed: %v", err), err) } return nil @@ -277,7 +285,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", fmt.Sprintf("failed to generate YAML: %v", err), err) } // Always validate expression sizes - this is a hard limit from GitHub Actions (21KB) @@ -285,7 +293,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", fmt.Sprintf("expression size validation failed: %v", err), 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 +306,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", err.Error(), 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 +320,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", fmt.Sprintf("workflow schema validation failed: %v", err), 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 +341,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", fmt.Sprintf("runtime package validation failed: %v", err), 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", fmt.Sprintf("firewall configuration validation failed: %v", err), 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", fmt.Sprintf("repository feature validation failed: %v", err), err) } } else if c.verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Schema validation available but skipped (use SetSkipValidation(false) to enable)")) @@ -377,7 +385,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", fmt.Sprintf("failed to write lock file: %v", err), 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..4b604756aa 100644 --- a/pkg/workflow/compiler_error_formatting_test.go +++ b/pkg/workflow/compiler_error_formatting_test.go @@ -3,6 +3,7 @@ package workflow import ( + "fmt" "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, no cause", filePath: "/path/to/workflow.md", errType: "error", message: "validation failed", + cause: nil, wantContain: []string{ "/path/to/workflow.md", "1:1", @@ -31,10 +34,11 @@ func TestFormatCompilerError(t *testing.T) { }, }, { - name: "warning type with detailed message", + name: "warning type with detailed message, no cause", filePath: "/path/to/workflow.md", errType: "warning", message: "missing required permission", + cause: nil, wantContain: []string{ "/path/to/workflow.md", "1:1", @@ -42,11 +46,25 @@ func TestFormatCompilerError(t *testing.T) { "missing required permission", }, }, + { + name: "error with underlying cause", + filePath: "/path/to/workflow.md", + errType: "error", + message: "failed to parse YAML", + cause: fmt.Errorf("syntax error at line 42"), + wantContain: []string{ + "/path/to/workflow.md", + "1:1", + "error", + "failed to parse YAML", + }, + }, { name: "lock file path", 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", @@ -55,10 +73,11 @@ func TestFormatCompilerError(t *testing.T) { }, }, { - name: "formatted message with error details", + name: "formatted message with error details and cause", filePath: "test.md", errType: "error", message: "failed to generate YAML: syntax error", + cause: fmt.Errorf("underlying error"), wantContain: []string{ "test.md", "1:1", @@ -70,20 +89,25 @@ func TestFormatCompilerError(t *testing.T) { 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() for _, want := range tt.wantContain { assert.Contains(t, errStr, want, "Error message should contain: %s", want) } + + // If cause is provided, verify error wrapping + if tt.cause != nil { + assert.ErrorIs(t, err, tt.cause, "Error should wrap the cause") + } }) } } // 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 +121,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) @@ -155,3 +179,37 @@ func TestFormatCompilerMessage(t *testing.T) { }) } } + +// TestFormatCompilerError_ErrorWrapping verifies that error wrapping preserves error chains +func TestFormatCompilerError_ErrorWrapping(t *testing.T) { + // Create an underlying error + underlyingErr := fmt.Errorf("underlying validation error") + + // Wrap it with formatCompilerError + wrappedErr := formatCompilerError("test.md", "error", "validation failed", underlyingErr) + + require.Error(t, wrappedErr) + + // Verify error chain is preserved + require.ErrorIs(t, wrappedErr, underlyingErr, "Should preserve error chain with %w") + + // Verify formatted message is in the error string + assert.Contains(t, wrappedErr.Error(), "test.md") + assert.Contains(t, wrappedErr.Error(), "validation failed") +} + +// TestFormatCompilerError_NilCause verifies that nil cause creates a new error +func TestFormatCompilerError_NilCause(t *testing.T) { + err := formatCompilerError("test.md", "error", "validation error", nil) + + require.Error(t, err) + + // Verify error message contains expected content + assert.Contains(t, err.Error(), "test.md") + assert.Contains(t, err.Error(), "validation error") + + // Verify it's a new error (not wrapping anything) + // This is a validation error, so it should not wrap + dummyErr := fmt.Errorf("some other error") + assert.NotErrorIs(t, err, dummyErr, "Should not wrap any error when cause is nil") +} diff --git a/pkg/workflow/compiler_orchestrator_frontmatter.go b/pkg/workflow/compiler_orchestrator_frontmatter.go index 76fee97a8e..f0624c5ec7 100644 --- a/pkg/workflow/compiler_orchestrator_frontmatter.go +++ b/pkg/workflow/compiler_orchestrator_frontmatter.go @@ -36,7 +36,8 @@ func (c *Compiler) parseFrontmatterSection(markdownPath string) (*frontmatterPar content, err := os.ReadFile(cleanPath) if err != nil { orchestratorFrontmatterLog.Printf("Failed to read file: %s, error: %v", cleanPath, err) - return nil, fmt.Errorf("failed to read file: %w", err) + // Don't wrap os.PathError - format it instead to avoid exposing internals + return nil, fmt.Errorf("failed to read file: %v", err) } log.Printf("File size: %d bytes", len(content))