[sergo] Sergo Report: Error Pattern Audit - 2026-04-21

[github-actions[bot]]

Date: 2026-04-21
Strategy: error-pattern-audit (new exploration)
Success Score: 8/10
Run ID: §24744725220

Executive Summary

This inaugural Sergo run performed a deep static analysis of error handling patterns across the pkg/workflow package — the project's dominant package at ~165k LOC across 700+ Go files. Using Serena's LSP symbol navigation and pattern search tools, the analysis uncovered three concrete, actionable improvements in error construction and aggregation.

The most architecturally significant finding is that ErrorCollector.FormattedError — the preferred multi-error aggregation method per its own docstring — silently discards the error chain when collecting multiple errors. This breaks errors.Is/errors.As introspection for all 5 callers in production validation paths. Two supporting findings cover a widespread fmt.Errorf("%s", ...) anti-pattern (9 sites) and an inconsistent logging guard that causes unnecessary allocations in 40+ hot call sites.

No prior Sergo runs exist; this establishes the baseline cache for future strategy selection.

---

🛠️ Serena Tools Update

Tools Snapshot

• Total Tools Available: 23
• New Tools Since Last Run: N/A (first run — baseline established)
• Removed Tools: None
• Modified Tools: None

Tool Capabilities Used Today

Tool	Usage
activate_project	Initialize Go LSP for the workspace
get_symbols_overview	Survey key files: compiler.go, engine.go, workflow_errors.go
find_symbol	Locate struct definitions and methods for error types
find_referencing_symbols	Trace call sites for WorkflowValidationError
search_for_pattern	Pattern-match fmt.Errorf("%s",...), NewValidationError, FormattedError, context.TODO/Background
list_dir	Map pkg/workflow directory structure

---

📊 Strategy Selection

Cached Reuse Component (50%)

No prior history — first run. Strategy defaults to exploration.

New Exploration Component (50% → 100% for first run)

Novel Approach: Error-pattern audit targeting pkg/workflow error types and their usage

• Tools Employed: find_symbol, search_for_pattern (regex), find_referencing_symbols
• Hypothesis: A large, rapidly-growing package with custom error types and a disabled errorlint linter is likely to accumulate fmt.Errorf anti-patterns and inconsistencies across error constructors
• Target Areas: pkg/workflow/workflow_errors.go and all files using NewValidationError, NewConfigurationError, NewOperationError, ErrorCollector

Combined Strategy Rationale

For a first run with no cache, full exploration was used. The error subsystem was chosen as the target because:

1. workflow_errors.go is a high-traffic, cross-cutting concern (~300 LOC, referenced in 40+ files)
2. The disabled errorlint and gocritic linters create a gap for error anti-patterns to accumulate
3. Custom error types + aggregation utilities = high surface area for subtle correctness issues

---

🔍 Analysis Execution

Codebase Context

• Total Go Files: 725 (non-test)
• Packages Analyzed: pkg/workflow (primary), with spot checks in pkg/cli, pkg/parser
• LOC Analyzed: ~165,000 (pkg/workflow total)
• Focus Areas: workflow_errors.go, all validation files using error constructors, ErrorCollector call sites

Findings Summary

• Total Issues Found: 3
• Critical: 0
• High: 1
• Medium: 2
• Low: 0

---

📋 Detailed Findings

High Priority Issues

Finding 1: ErrorCollector.FormattedError silently loses error chain

pkg/workflow/workflow_errors.go:261 — When multiple errors are collected, FormattedError builds a string and wraps it with fmt.Errorf("%s", ...), discarding the error chain. errors.Is/errors.As cannot traverse through the result. By contrast, ErrorCollector.Error() (line 225) correctly uses errors.Join(c.errors...).

The method is documented as preferred over Error(), yet is architecturally weaker. It is called in 5 production validation paths:

• call_workflow_validation.go:178
• dispatch_workflow_validation.go:138
• dispatch_repository_validation.go:100
• repository_features_validation.go:155
• strict_mode_validation.go:88

Fix: Use fmt.Errorf("Found %d %s errors:\n%w", len(c.errors), category, errors.Join(c.errors...)) to preserve both the formatted header and the full error chain.

Medium Priority Issues

Finding 2: fmt.Errorf("%s", ...) anti-pattern — 9 occurrences

Nine call sites use fmt.Errorf("%s", someString) where no error is being wrapped. The correct idiom is errors.New(someString).

File	Line
workflow_errors.go	261
engine_validation.go	142
template_injection_utils.go	165
permissions_validation.go	346
dangerous_permissions_validation.go	96
safe_update_enforcement.go	257
tools_validation_github_toolsets.go	97
runtime_validation.go	91
engine_definition.go	280

The perfsprint linter (enabled in .golangci.yml) targets exactly this. Note: Finding 2 at workflow_errors.go:261 overlaps with Finding 1 — fixing the chain issue also fixes the anti-pattern there.

Finding 3: Unconditional log calls in NewValidationError and NewConfigurationError

NewValidationError (line 66) and NewConfigurationError (line 171) unconditionally call errorHelpersLog.Printf(...), while NewOperationError (line 118) correctly guards with if errorHelpersLog.Enabled(). NewValidationError has 40+ call sites, many in tight validation loops inside the compiler.

---

✅ Improvement Tasks Generated

Task 1: Fix ErrorCollector.FormattedError to preserve error chain

Issue Type: Error Aggregation Correctness
Severity: High | Effort: Small

Before (pkg/workflow/workflow_errors.go:254-261):

var sb strings.Builder
fmt.Fprintf(&sb, "Found %d %s errors:", len(c.errors), category)
for _, err := range c.errors {
    sb.WriteString("\n  • ")
    sb.WriteString(err.Error())
}
return fmt.Errorf("%s", sb.String())

After:

joined := errors.Join(c.errors...)
return fmt.Errorf("Found %d %s errors:\n%w", len(c.errors), category, joined)

Validation: Add tests asserting errors.Is/errors.As work on FormattedError output when multiple errors collected.

---

Task 2: Replace fmt.Errorf("%s", ...) with errors.New(...) (8 remaining sites)

Issue Type: Error Construction Anti-Pattern
Severity: Medium | Effort: Small

Simple mechanical replacement across 8 files. No behavior change. Aligns with perfsprint linter intent and improves code clarity.

---

Task 3: Add errorHelpersLog.Enabled() guard to NewValidationError and NewConfigurationError

Issue Type: Performance / Code Consistency
Severity: Medium | Effort: Small

Before:

func NewValidationError(...) *WorkflowValidationError {
    errorHelpersLog.Printf("Creating validation error: field=%s, reason=%s", field, reason)
    ...

After:

func NewValidationError(...) *WorkflowValidationError {
    if errorHelpersLog.Enabled() {
        errorHelpersLog.Printf("Creating validation error: field=%s, reason=%s", field, reason)
    }
    ...

---

📈 Success Metrics

This Run

• Findings Generated: 3
• Tasks Created: 3
• Files Analyzed: ~50 (deep), ~700 (pattern scan)
• Success Score: 8/10

Reasoning for Score

• Findings Quality (3/4): All 3 issues are concrete, actionable, and located to specific lines. Finding 1 is architecturally significant. Deducting 1 for not finding any critical bugs.
• Coverage (3/3): Pattern scanning covered all 700+ non-test Go files in pkg/workflow.
• Task Generation (2/3): 3 high-quality tasks generated. Deducting 1 as tasks 2 and 3 overlap with Task 1 (workflow_errors.go line 261 is both Finding 1 and 2).

---

📊 Historical Context

Cumulative Statistics (after run 1)

• Total Runs: 1
• Total Findings: 3
• Total Tasks Generated: 3
• Average Success Score: 8.0/10
• Most Successful Strategy: error-pattern-audit

---

🎯 Recommendations

Immediate Actions

1. [High] Fix ErrorCollector.FormattedError — restore error chain in multi-error aggregation (5 affected callers)
2. [Medium] Replace fmt.Errorf("%s", ...) → errors.New(...) — 8 remaining sites after fixing rejig docs #1
3. [Medium] Add Enabled() guard to NewValidationError/NewConfigurationError — eliminate unnecessary allocations in 40+ call sites

Long-term Improvements

• Re-enable errorlint once the golangci-lint v2 config bug is resolved — it would catch the fmt.Errorf anti-patterns automatically
• Consider adding a custom linter rule for fmt.Errorf("%s", ...) in the meantime
• Add integration tests that assert errors.Is/errors.As on FormattedError outputs to prevent regressions

---

🔄 Next Run Preview

Suggested Focus Areas

• Interface compliance audit: The codebase has many validation functions returning error. Check whether interface contracts (e.g. Validator interface patterns) are consistently implemented.
• Context propagation: Several functions use context.Background() internally (e.g., action_resolver.go:106, safe_outputs_actions.go:290). Audit whether caller-supplied contexts should be threaded through instead.

Strategy Evolution

Next run should blend 50% reuse of error-pattern-audit (targeting the 8 remaining fmt.Errorf("%s",...) sites for verification) with 50% new exploration of context propagation patterns or interface consistency.

References:

• §24744725220

Generated by Sergo - Serena Go Expert · ● 388.4K · ◷

• expires on Apr 22, 2026, 8:35 PM UTC

[github-actions[bot]]

This discussion has been marked as outdated by Sergo - Serena Go Expert.

A newer discussion is available at Discussion #27892.