[sergo] Sergo Report: Error Handling & Aggregation Analysis - 2026-02-19

[github-actions[bot]]

Overview

Today's Sergo analysis focused on error handling patterns and error aggregation design across the pkg/ tree (1,506 Go files, ~428k LOC). This is the first run — no strategy cache existed — so the baseline was established from scratch. The analysis surfaced four distinct findings, with the highest-impact issue being ErrorCollector.FormattedError() silently discarding the accumulated error chain that ErrorCollector.Error() carefully preserves via errors.Join.

Two other patterns were also identified: a widespread fmt.Errorf("%s", ...) anti-pattern appearing in 12+ production files (equivalent to errors.New but using the wrong function), and two dead-code functions in error_aggregation.go that are only referenced from tests yet carry inherently fragile string-splitting logic.

---

🛠️ Serena Tools Update

Tools Snapshot

• Total Tools Available: 41 (23 active, 18 inactive)
• New Tools Since Last Run: N/A — first run, baseline established
• Removed Tools: N/A
• Modified Tools: N/A

Active Tools Used Today

Tool	Purpose
search_for_pattern	Regexp search across codebase for error patterns
find_symbol	Inspect ErrorCollector, FormatAggregatedError, SplitJoinedErrors, BaseEngine, CodingAgentEngine bodies
find_referencing_symbols	Confirm FormatAggregatedError/SplitJoinedErrors are test-only
get_symbols_overview	Survey error_aggregation.go, runtime_validation.go, agentic_engine.go
think_about_collected_information	Validate findings before task generation

---

📊 Strategy Selection

Cached Reuse Component (50%)

Previous Strategy Adapted: None (first run) — using Go Error Handling Baseline

Since no cache existed, the "cached" half was filled by the most universally applicable Go static analysis lens: error wrapping and sentinel error correctness. This is the highest-signal-to-noise strategy for any Go codebase because error misuse is common, measurable, and directly actionable.

• Why chosen: Error quality issues are universally present, well-defined in the Go specification, and easy to validate with Serena pattern search
• Target areas: All pkg/**/*.go production files, excluding test files
• Tools: search_for_pattern for fmt.Errorf("%s", ...), errors.New(formattedErr) patterns

New Exploration Component (50%)

Novel Approach: ErrorCollector API surface analysis — cross-referencing the ErrorCollector struct methods against their callers to detect behavioral divergence between Error() and FormattedError()

• Hypothesis: The FormattedError() method trades error chain integrity for display formatting, and callers may not realize this trade-off
• Tools: find_symbol (method bodies), find_referencing_symbols (production callers), get_symbols_overview (full type surface)
• Target areas: pkg/workflow/error_aggregation.go and all callers

Combined Strategy Rationale

The baseline error-wrapping scan quickly identified the fmt.Errorf("%s", ...) anti-pattern at scale. The deeper ErrorCollector exploration revealed that the intended multi-error aggregation API has a silent correctness issue: FormattedError() was designed as the "preferred" method (per its own comment) but destroys the chain that Error() + errors.Join preserves. Together these two lenses covered both breadth (all files) and depth (one type's full API contract).

---

🔍 Analysis Execution

Codebase Context

• Total Go Files: 1,506
• Packages Analyzed: 22 (all under pkg/ and cmd/)
• LOC analyzed: ~428,000 (production) + ~428,000 (test files for dead-code verification)
• Focus Areas: pkg/workflow/, pkg/cli/, pkg/parser/

Findings Summary

Severity	Count
High	1
Medium	2
Low	1
Total	4

---

📋 Detailed Findings

High Priority

H1 — ErrorCollector.FormattedError() silently drops the error chain

pkg/workflow/error_aggregation.go:120–139

ErrorCollector.Error() (line 114) correctly uses errors.Join(c.errors...), which produces an error supporting errors.Is/errors.As traversal. The sibling method FormattedError() is documented as the preferred alternative, yet it converts all collected errors to a single formatted string and returns fmt.Errorf("%s", sb.String()) — making the result opaque to the standard error inspection API.

Three production callers use FormattedError() instead of Error():

• pkg/workflow/repository_features_validation.go:179
• pkg/workflow/dispatch_workflow_validation.go:165
• pkg/workflow/strict_mode_validation.go:368

If any downstream code ever tests for a specific error condition using errors.Is, it will silently fail for errors routed through these three validators.

Medium Priority

M1 — fmt.Errorf("%s", ...) anti-pattern (12+ production occurrences)

Using fmt.Errorf without any format verb (%v, %w, etc.) is semantically identical to errors.New(...) but carries the overhead of fmt and obscures the author's intent. More critically, in cases where an underlying error is available but not wrapped with %w, the error chain is broken.

Affected production files:

File	Line	Pattern
pkg/workflow/error_helpers.go	182	fmt.Errorf("%s", b.String())
pkg/workflow/frontmatter_error.go	89	fmt.Errorf("%s", vscodeFormat)
pkg/workflow/imports.go	297	fmt.Errorf("%s", errorMsg.String())
pkg/workflow/engine_validation.go	96	fmt.Errorf("%s", errMsg)
pkg/workflow/template_injection_validation.go	278	fmt.Errorf("%s", builder.String())
pkg/workflow/error_aggregation.go	139, 170, 186	fmt.Errorf("%s", sb.String()) ×3
pkg/workflow/dangerous_permissions_validation.go	104	fmt.Errorf("%s", strings.Join(...))
pkg/workflow/github_tool_to_toolset.go	120	fmt.Errorf("%s", errMsg)
pkg/cli/run_workflow_validation.go	249	fmt.Errorf("%s", strings.Join(...))
pkg/cli/run_workflow_execution.go	196	fmt.Errorf("%s", errMsg)
pkg/parser/import_error.go	77, 126	fmt.Errorf("%s", ...) ×2

The perfsprint linter (not currently enabled in .golangci.yml) would catch all of these automatically.

M2 — FormatAggregatedError and SplitJoinedErrors are dead production code with fragile design

pkg/workflow/error_aggregation.go:143–194

Both functions are only ever called from error_aggregation_test.go — zero production references confirmed via find_referencing_symbols. SplitJoinedErrors attempts to reconstruct individual errors from an errors.Join result by splitting on \n. This is inherently fragile: any error message containing a newline character will be split incorrectly. FormatAggregatedError counts errors by counting non-empty lines — similarly incorrect if errors contain newlines. These functions should be removed or, if retained as test helpers, moved to a _test.go file.

Low Priority Findings

L1 — .golangci.yml does not enable perfsprint linter

The perfsprint linter (available in golangci-lint) detects fmt.Sprintf/fmt.Errorf calls that do no formatting and can be replaced with cheaper alternatives (errors.New, string(...), etc.). Enabling it would prevent the M1 pattern from recurring. No configuration change is needed beyond adding - perfsprint to the linters.enable list.

---

✅ Improvement Tasks Generated

Task 1: Fix ErrorCollector.FormattedError() to Preserve the Error Chain

Issue Type: Error Handling — API Contract

Problem:
FormattedError() is documented as the preferred way to retrieve a human-readable aggregated error, yet it converts all collected errors to a plain string, destroying the errors.Join chain that Error() preserves. Three production validators depend on FormattedError(), meaning errors.Is/errors.As will silently fail for any error path through those validators.

Locations:

• pkg/workflow/error_aggregation.go:120–139 — method to fix
• pkg/workflow/repository_features_validation.go:179 — caller
• pkg/workflow/dispatch_workflow_validation.go:165 — caller
• pkg/workflow/strict_mode_validation.go:368 — caller

Impact:

• Severity: High
• Affected Files: 4
• Risk: Silent errors.Is/errors.As failures in validators; debugging multi-error conditions becomes impossible programmatically

Recommendation:
Introduce a thin wrapper type that carries both the formatted display string and the original joined error, so Error() returns the human-readable message while Unwrap() []error exposes the chain:

// Before (loses error chain):
func (c *ErrorCollector) FormattedError(category string) error {
    // ...
    return fmt.Errorf("%s", sb.String())  // chain lost here
}

// After (preserves error chain):
type formattedMultiError struct {
    msg    string
    joined error  // result of errors.Join(c.errors...)
}

func (e *formattedMultiError) Error() string     { return e.msg }
func (e *formattedMultiError) Unwrap() error     { return e.joined }

func (c *ErrorCollector) FormattedError(category string) error {
    if len(c.errors) == 0 {
        return nil
    }
    if len(c.errors) == 1 {
        return c.errors[0]
    }
    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 &formattedMultiError{msg: sb.String(), joined: errors.Join(c.errors...)}
}

Validation:

• Existing TestErrorCollector_FormattedError tests still pass
• Add test: errors.Is(collector.FormattedError("x"), specificErr) returns true
• Verify three production callers continue to work correctly

Estimated Effort: Small

---

Task 2: Replace fmt.Errorf("%s", ...) with errors.New(...) Throughout pkg/

Issue Type: Code Clarity / Linting Gap

Problem:
fmt.Errorf("%s", someString) appears 12+ times in production code. When no format verbs other than %s are used and the argument is already a string, this is functionally identical to errors.New(someString) but signals incorrect intent, adds fmt overhead, and — most importantly — does not wrap any underlying cause. A reader seeing fmt.Errorf naturally expects either formatting or error wrapping (%w); the bare %s form is misleading.

Locations (all pkg/ production files, excluding test files): see M1 table above (12 files, 14 call sites).

Impact:

• Severity: Medium
• Affected Files: 11 production files
• Risk: Code clarity; the perfsprint linter would flag these; no functional bug but increases cognitive overhead

Recommendation:
Replace all fmt.Errorf("%s", x) → errors.New(x) where x is a plain string with no underlying error to wrap. Additionally enable the perfsprint linter in .golangci.yml to prevent recurrence:

# .golangci.yml — add to linters.enable:
- perfsprint

// Before:
return fmt.Errorf("%s", b.String())

// After:
return errors.New(b.String())

Validation:

• All existing tests pass (pure rename, no behaviour change)
• golangci-lint run reports zero perfsprint violations after change
• Grep confirms zero remaining fmt.Errorf("%s", in pkg/ (excluding test files)

Estimated Effort: Small (mechanical sed-like replacements + one linter config line)

---

Task 3: Remove Dead Code FormatAggregatedError and SplitJoinedErrors

Issue Type: Dead Code / Fragile Design

Problem:
FormatAggregatedError (error_aggregation.go:143) and SplitJoinedErrors (error_aggregation.go:173) have zero production callers. They exist only in error_aggregation_test.go. Both functions rely on string-splitting to count or reconstruct errors from errors.Join output — a fragile approach that silently misbehaves if any individual error message contains a newline character.

SplitJoinedErrors specifically is designed to reverse errors.Join (which joins with \n) by splitting on \n. This only works correctly when all component error messages are single-line. A multi-line error message — entirely valid in this codebase, as evidenced by the ST1005 linter exclusions for multi-line compiler errors — would be incorrectly split into multiple synthetic errors.

Locations:

• pkg/workflow/error_aggregation.go:143–170 — FormatAggregatedError
• pkg/workflow/error_aggregation.go:173–194 — SplitJoinedErrors
• pkg/workflow/error_aggregation_test.go:125–188 — their only callers

Impact:

• Severity: Medium
• Affected Files: 2
• Risk: Dead code increases maintenance surface; if these functions are discovered and used by future contributors, they will introduce silent bugs with multi-line errors

Recommendation:
Delete both exported functions and their test coverage. If there is a legitimate need to re-split aggregated errors in the future, prefer using errors.Join's native Unwrap() []error interface (accessible via errors.As with a []error target since Go 1.20) rather than string-splitting.

// Instead of SplitJoinedErrors (string-based, fragile):
errs := SplitJoinedErrors(joined)

// Use the standard approach (chain-safe):
var unwrapped interface{ Unwrap() []error }
if errors.As(joined, &unwrapped) {
    errs = unwrapped.Unwrap()
}

Validation:

• Delete FormatAggregatedError, SplitJoinedErrors, and their test functions
• go build ./... succeeds with no references remaining
• go test ./pkg/workflow/... passes (tests using these functions are also deleted)

Estimated Effort: Small

---

📈 Success Metrics

This Run

• Findings Generated: 4 (1 High, 2 Medium, 1 Low)
• Tasks Created: 3
• Files Analyzed: ~1,506 Go files across 22 packages
• Success Score: 8/10

Reasoning for Score

• Findings Quality (3/4): Found one High-severity API contract issue (FormattedError losing chain), two Medium issues (anti-pattern spread and dead code), and one Low (linter gap). All are actionable with concrete fix examples.
• Coverage (3/3): Full pkg/ sweep for error patterns; deep dive into error_aggregation.go type surface; interface hierarchy in agentic_engine.go surveyed.
• Task Generation (2/3): All 3 tasks are well-scoped with before/after code, validation checklists, and effort estimates. Minus one point as Tasks 2 and 3 are "Small" effort mechanical fixes rather than architectural improvements.

Cumulative Statistics

• Total Runs: 1
• Total Findings: 4
• Total Tasks Generated: 3
• Average Success Score: 8.0/10
• Most Successful Strategy: error-handling-and-aggregation (baseline)

---

🎯 Recommendations

Immediate Actions

1. Fix ErrorCollector.FormattedError() chain preservation — High priority; affects 3 validators and future debuggability. Small effort with high correctness benefit.
2. Replace fmt.Errorf("%s", ...) with errors.New — Medium priority; 12+ occurrences, purely mechanical, no risk. Add perfsprint linter to prevent recurrence.
3. Delete FormatAggregatedError / SplitJoinedErrors — Medium priority; removes fragile dead code before it can be discovered and misused.

Long-term Improvements

• Consider defining a small set of sentinel error types (e.g. ErrValidation, ErrCompiler) to allow callers to distinguish validation failures from runtime failures using errors.Is/errors.As rather than string matching. Currently ErrNoArtifacts is the only sentinel in pkg/.
• The errors.Join + custom display message pattern identified in Task 1 could be generalized into a shared pkg/workflow type for consistent human-readable multi-error presentation throughout the package.

---

🔄 Next Run Preview

Suggested Focus Areas

• Context propagation: No context.Background() or context.TODO() calls were found in pkg/ production code — confirming contexts flow from the top. A targeted audit of the longest call chains (compiler pipeline, MCP config rendering) would verify no context is silently dropped mid-chain.
• Interface implementation completeness: CodingAgentEngine embeds 6 sub-interfaces; verify all concrete engine types implement every method rather than relying on BaseEngine defaults that return zero values.
• Goroutine lifecycle: Four goroutines in pkg/ — docker_images.go, mcp_inspect_inspector.go, update_check.go, console/spinner.go — warrant a targeted leak/cancellation audit.

Strategy Evolution

• Cache the "error handling baseline" as a periodic (monthly) sweep rather than a first-run strategy, to catch regressions.
• Next run's "new exploration" slot should target context propagation or interface completeness — both uncharted in this run.

References:

• §22202323077

AI generated by Sergo - Serena Go Expert

• expires on Feb 26, 2026, 10:24 PM UTC

[github-actions[bot]]

🤖 Beep boop! The smoke test agent was here!

I breezed through your discussion like a well-optimized build pipeline — no errors, no warnings, just vibes. If software were a party, I'd be the CI bot that shows up uninvited but somehow makes everything better. 🎉

Runs the exit 0 dance 💃

📰 BREAKING: Report filed by Smoke Copilot for issue #16920