Standardize error wrapping in compiler to preserve error chains

[Copilot]

Error wrapping was inconsistent across compiler files—compiler_jobs.go had 16 instances while compiler.go had zero. This broke error chains, making compilation failures harder to debug.

Changes

Enhanced formatCompilerError with optional cause parameter:

// Before
func formatCompilerError(filePath string, errType string, message string) error

// After  
func formatCompilerError(filePath string, errType string, message string, cause error) error

Wrapping behavior:

• Wraps with %w when cause is non-nil → preserves error chains
• Creates new error when cause is nil → validation errors without underlying cause
• Never wraps os.PathError or yaml.TypeError → avoids exposing internal implementation details

Updated error calls:

• compiler.go: 22 calls now pass underlying errors
• agent_validation.go: 3 calls updated (2 wrapped, 1 validation)
• compiler_orchestrator_frontmatter.go: 1 call fixed to not expose os.PathError

Added tests:

• Error chain preservation with errors.Is
• Nil cause handling for validation errors
• Verification that low-level errors remain unexposed

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Code Quality] Standardize error wrapping across compiler files</issue_title>
<issue_description>### Description

Error wrapping practices are inconsistent across compiler files, with some files having excellent coverage (16 instances in compiler_jobs.go) while others have none (compiler.go). This breaks error chains and makes debugging compilation failures significantly harder.

Current State

Error Wrapping by File:

• compiler.go: 0 instances of fmt.Errorf(..., %w) ❌
• compiler_yaml.go: 3 instances ⚠️
• compiler_activation_jobs.go: 4 out of 13 (9 missing %w) ⚠️
• compiler_orchestrator_workflow.go: 5 instances ✅
• compiler_jobs.go: 16 instances ✅ (exemplary)

Why This Matters (Release Mode Priority)

1. Debugging Difficulty: Without error wrapping, root causes are lost in error chains
2. Production Support: Harder to diagnose compilation failures in workflows
3. Error Context: Missing context makes troubleshooting time-consuming
4. Consistency: Codebase should follow uniform error handling patterns

Problem Examples

❌ Current Pattern (compiler.go)

func formatCompilerError(filePath, errType, message string) error {
    formatted := console.FormatError(...)
    return errors.New(formatted)  // Creates new error, loses cause
}

✅ Should Be (with wrapping)

func formatCompilerError(filePath, errType, message string, cause error) error {
    formatted := console.FormatError(...)
    if cause != nil {
        return fmt.Errorf("%s: %w", formatted, cause)
    }
    return errors.New(formatted)
}

Suggested Changes

1. Update compiler.go (Priority: High)

• Enhance formatCompilerError() to support error wrapping
• Add %w to all error returns throughout file
• Estimated: 1-2 hours

2. Update compiler_yaml.go (Priority: Medium)

• Add error wrapping to remaining error paths (job building, validation)
• Current 3 instances → Target: 8-10 instances
• Estimated: 1 hour

3. Update compiler_activation_jobs.go (Priority: Medium)

• Audit all 13 fmt.Errorf calls
• Add %w to 9 missing instances
• Estimated: 1 hour

Files Affected

• pkg/workflow/compiler.go (~10-15 error returns to update)
• pkg/workflow/compiler_yaml.go (~5-7 error returns to update)
• pkg/workflow/compiler_activation_jobs.go (~9 error returns to update)

Success Criteria

• All compiler files consistently use fmt.Errorf(..., %w) for error wrapping
• formatCompilerError() enhanced to support cause wrapping
• Error messages maintain clarity and console formatting
• All existing tests pass (make test-unit)
• Manual testing confirms improved error messages
• Quality scores improve across all files

Estimated Effort

Total: 3-4 hours (1-2h per file, phased approach)

Implementation Strategy

Phase 1 (High Priority - compiler.go):

1. Enhance formatCompilerError() with cause parameter
2. Update all error returns in compiler.go
3. Test compilation error scenarios

Phase 2 (Medium Priority - other files):

1. Update compiler_yaml.go error paths
2. Update compiler_activation_jobs.go error paths
3. Final validation across all files

Priority

Medium - Improves debugging and error diagnostics, supporting release stability indirectly.

Source

Extracted from:

• Daily Compiler Code Quality Report - 2026-02-07 github/gh-aw#14370
• Daily Compiler Code Quality Report - 2026-02-06 github/gh-aw#14159

References:

• §21781991134
• §21755331004

AI generated by Discussion Task Miner - Code Quality Improvement Agent

• expires on Feb 8, 2026, 5:06 PM UTC

Comments on the Issue (you are @copilot in this section)

• Fixes [Code Quality] Standardize error wrapping across compiler files #14389

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[pelikhan]

@copilot what do error look like?

[Copilot]

@copilot what do error look like?

Error messages maintain the same clear, user-friendly format with file location, line numbers, and helpful suggestions. Here are examples:

Validation Error (Unknown Property):

✗ test-validation-error.md:11:3: error: Unknown property: invalid-field. Valid fields are: agent, config, mcp, type
 8 |   github:
 9 |     toolset: [issues]
10 | sandbox:
11 |   invalid-field: true
       ^^^^^^^^^^^^^
12 | ---

Import Error (File Not Found):

✗ test-file-not-found.md:6:5: error: import file not found
4 |   workflow_dispatch:
5 | imports:
6 |   - /nonexistent/file.md
        ^^^^^^^^^^^^^^^^^^^^
7 | ---

Permission Warning (with suggestions):

⚠ test-permission-warning.md:1:1: warning: Missing required permissions for github toolsets:
  - issues: read (required by issues)
  - pull-requests: read (required by pull_requests)

To fix this, you can either:

Option 1: Add missing permissions to your workflow frontmatter:
permissions:
  issues: read
  pull-requests: read

The key improvement is that error chains are now preserved internally with %w, so when debugging compilation failures programmatically (e.g., in tests or logging), you can unwrap errors with errors.Is() and errors.As() to get the root cause. The user-facing format remains unchanged.

[Copilot]

Pull request overview

This PR updates compiler error handling to preserve error chains while keeping the existing console-formatted compiler error output, improving debuggability of compilation failures across the workflow compiler.

Changes:

• Extends formatCompilerError to accept an optional cause and wrap it when provided.
• Updates compiler and agent validation call sites to pass underlying errors (or nil for pure validation errors).
• Adjusts frontmatter file-read error handling to avoid wrapping os.PathError, and adds/updates unit tests around error formatting/wrapping.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
pkg/workflow/compiler.go	Adds optional error wrapping to formatCompilerError and updates many call sites to pass a cause.
pkg/workflow/agent_validation.go	Updates error formatting calls to include cause where appropriate.
pkg/workflow/compiler_orchestrator_frontmatter.go	Stops wrapping file read errors to avoid exposing os.PathError via error chains.
pkg/workflow/compiler_error_formatting_test.go	Updates existing tests for the new formatCompilerError signature and adds wrapping-related assertions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

When cause != nil, fmt.Errorf("%s: %w", formattedErr, cause) will append cause.Error() to the user-facing formatted compiler message. Several call sites already embed the underlying error text into message (e.g., fmt.Sprintf("...: %v", err)), so this leads to duplicated error details like ...: <err>: <err>, and it can also re-expose low-level error strings even if the formatted message was intended to be the primary output. Consider changing the wrapping strategy so the formatted compiler error text remains the Error() string while still providing Unwrap() for the cause (or, alternatively, ensure call sites stop including %v/err.Error() in message whenever cause is provided).

[Copilot]

This assertion doesn’t actually verify that a nil cause results in a non-wrapping error: dummyErr is unrelated, so assert.NotErrorIs(err, dummyErr) will always pass even if err did wrap something else. A more direct check would be to assert that errors.Unwrap(err) is nil (or that errors.Is(err, someUnderlyingSentinel) is false when you intentionally pass a sentinel as the cause in a separate case).