[sergo] Sergo Report: Utility Package Deep-Dive - 2026-04-28

[github-actions[bot]]

Date: 2026-04-28
Strategy: utility-package-deep-dive
Success Score: 8/10

Executive Summary

Today's Sergo run performed a deep static analysis of the pkg/ utility packages in the gh-aw codebase. The analysis used Serena's LSP symbol navigation, file-level overviews, reference tracing, and pattern search to inspect 8 packages: stringutil, sliceutil, typeutil, fileutil, gitutil, repoutil, semverutil, and logger.

Three actionable findings were discovered: a data-integrity bug in fileutil.CopyFile that leaves partial files after failed writes, a correctness bug in the logger's wildcard pattern matching, and a documentation mismatch in SanitizeToolID. All three are well-scoped, immediately fixable, and have clear validation paths.

This is the first Sergo run on this repository, so no prior strategy history was available. A baseline cache has been established for future comparison.

---

🛠️ Serena Tools Update

Tools Snapshot

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

Tool Capabilities Used Today

Tool	Purpose
activate_project	Initialize Serena for Go analysis
list_dir	Map pkg/ package structure
find_file	Locate non-test Go source files
get_symbols_overview	Scan exported/unexported symbols per file
find_referencing_symbols	Trace callers of SanitizeToolID
search_for_pattern	Search for map[string]bool, error patterns
read_memory / write_memory	Persist onboarding and run history

---

📊 Strategy Selection

Cached Reuse Component (50%)

Previous Strategy Adapted: None (first run)

Since no strategy cache existed, the 50% "cached reuse" slot was filled with a well-known high-value strategy: utility package auditing. Utility packages are ideal first targets because they are:

• Heavily reused across the codebase (high blast radius for any bug)
• Smaller and more self-contained than the core pkg/workflow package (~400 files)
• Frequently overlooked in reviews since they appear "done"

New Exploration Component (50%)

Novel Approach: Documentation-vs-implementation divergence scan

Combined symbol-level reading with reference tracing to detect cases where a function's docstring examples don't match the actual tested behavior. This cross-references:

1. The implementation (what the code does)
2. The test expectations (what tests assert)
3. The docstring examples (what documentation claims)

Hypothesis: In large codebases with rapid iteration, docstring examples drift from implementation. Finding confirmed in SanitizeToolID.

Combined Strategy Rationale

The utility-package-deep-dive provides broad horizontal coverage across many small packages, while the doc-vs-implementation scan adds a vertical depth dimension. Together they surface both logic bugs (partial file cleanup, pattern matching) and knowledge-gap bugs (incorrect documentation), which are complementary issue types.

---

🔍 Analysis Execution

Codebase Context

• Packages Analyzed: 8 utility packages + pkg/logger
• Files Examined: ~25 Go source files (non-test)
• Focus Areas: pkg/fileutil, pkg/stringutil, pkg/sliceutil, pkg/typeutil, pkg/gitutil, pkg/repoutil, pkg/semverutil, pkg/logger

Findings Summary

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

---

📋 Detailed Findings

High Priority: fileutil.CopyFile leaves partial file on write failure

pkg/fileutil/fileutil.go:114–135

When io.Copy fails midway through a file copy, the function returns the error — but os.Remove(dst) is never called. The deferred out.Close() closes the file descriptor, leaving a corrupt partial file on disk with its error silently discarded.

if _, err = io.Copy(out, in); err != nil {
    return err  // partial dst file remains on disk
}

Risk: Any code checking fileutil.FileExists(dst) after a failed copy will see the destination as present and may silently consume corrupt data. This is a data-integrity issue.

Fix:

if _, err = io.Copy(out, in); err != nil {
    _ = out.Close()
    _ = os.Remove(dst)
    return err
}

Medium Priority: matchPattern false positives with overlapping wildcards

pkg/logger/logger.go:200–209

Middle-wildcard patterns (e.g. "ab*ba") are matched by checking HasPrefix + HasSuffix. Missing: a length guard to prevent prefix and suffix from overlapping when the namespace is shorter than len(prefix) + len(suffix).

// Pattern "ab*ba", namespace "aba" (3 chars, needs 4):
// HasPrefix("aba", "ab") = true
// HasSuffix("aba", "ba") = true  → incorrectly returns true

In practice namespace strings like "typeutil:convert" make this unlikely to trigger, but it's a latent correctness defect.

Fix: Add len(namespace) >= len(prefix)+len(suffix) && before the prefix/suffix checks.

Medium Priority: SanitizeToolID docstring example is incorrect

pkg/stringutil/sanitize.go:169

The docstring claims:

// SanitizeToolID("some-mcp-server")   // returns "some-server"

But the actual implementation only strips leading "mcp-" and trailing "-mcp". A middle "-mcp-" is left unchanged. The test file (sanitize_test.go:516) correctly reflects the actual behavior ("some-mcp-server" → "some-mcp-server"), confirming this is a documentation-only bug.

Additional Observations (not filed as issues)

• pkg/sliceutil/sliceutil.go:65: Deduplicate uses map[T]bool instead of map[T]struct{}. Minor: saves 1 byte per entry, style preference in modern Go.
• pkg/semverutil/semverutil.go:103–118: Compare logs every comparison. Could be verbose if called in hot loops during compilation. Consider a guard or removing the per-call log.
• pkg/gitutil/gitutil.go:30: IsAuthError logs the raw error message before checking. When DEBUG is enabled, this could expose credential fragments in debug output. Low risk since it requires DEBUG to be set.
• pkg/typeutil/convert.go: ConvertToInt doesn't handle uint64 but ParseIntValue does. This is intentional by design (JSON/YAML doesn't produce uint64) and the docstrings reflect this.

---

✅ Improvement Tasks Generated

Task 1: Fix fileutil.CopyFile partial-file leak on write error

Issue Type: Correctness / Data Integrity

Problem: When io.Copy fails, CopyFile returns the error without removing the partially-written destination file.

Location: pkg/fileutil/fileutil.go:130

Before:

if _, err = io.Copy(out, in); err != nil {
    return err
}

After:

if _, err = io.Copy(out, in); err != nil {
    _ = out.Close()
    _ = os.Remove(dst)
    return err
}

Validation:

• Add a test injecting a write error and asserting dst does not exist afterward
• go test ./pkg/fileutil/... passes

Estimated Effort: Small

---

Task 2: Fix matchPattern overlapping prefix/suffix in pkg/logger

Issue Type: Correctness

Problem: Missing length guard in wildcard pattern matching allows false positive matches when namespace is shorter than prefix+suffix combined.

Location: pkg/logger/logger.go:205–208

Before:

return strings.HasPrefix(namespace, prefix) && strings.HasSuffix(namespace, suffix)

After:

return len(namespace) >= len(prefix)+len(suffix) &&
    strings.HasPrefix(namespace, prefix) &&
    strings.HasSuffix(namespace, suffix)

Validation:

• Add test case: pattern "ab*ba", namespace "aba" → should return false
• go test ./pkg/logger/... passes

Estimated Effort: Small

---

Task 3: Fix SanitizeToolID docstring example

Issue Type: Documentation

Problem: The docstring example for "some-mcp-server" claims "some-server" as output, but actual behavior is "some-mcp-server" (unchanged).

Location: pkg/stringutil/sanitize.go:169

Before:

//	SanitizeToolID("some-mcp-server")   // returns "some-server"

After:

//	SanitizeToolID("some-mcp-server")   // returns "some-mcp-server" (middle occurrence unchanged)

Validation:

• go test ./pkg/stringutil/... passes (already passing)
• No behavior change — documentation fix only

Estimated Effort: Trivial

---

📈 Success Metrics

This Run

• Findings Generated: 3
• Tasks Created: 3
• Files Analyzed: ~25
• Success Score: 8/10

Reasoning for Score

• Findings Quality (3/4): All three findings are real, actionable, and well-evidenced. One is high-impact (data integrity), two are medium-impact. No false positives.
• Coverage (2/3): 8 utility packages covered well, but the large pkg/workflow and pkg/parser packages were not analyzed (deferred for future runs).
• Task Generation (3/3): All 3 tasks are clearly scoped, include before/after code, and have concrete validation steps.

---

📊 Historical Context

Cumulative Statistics

• Total Runs: 1 (baseline)
• Total Findings: 3
• Total Tasks Generated: 3
• Average Success Score: 8.0/10
• Most Successful Strategy: utility-package-deep-dive

---

🎯 Recommendations

Immediate Actions

1. [High] Fix fileutil.CopyFile partial-file leak — data integrity risk if copy fails midway
2. [Medium] Fix matchPattern length guard — latent correctness bug in logger
3. [Medium] Fix SanitizeToolID docstring — documentation mismatch confirmed by tests

Long-term Improvements

• The existing open issue Replace fmt.Errorf("%s", ...) with errors.New(...) in pkg/workflow (9 sites) #27663 (fmt.Errorf("%s", ...) → errors.New(...)) is a good template for systematic linter-driven cleanup. A similar sweep for other perfsprint patterns could be valuable.
• pkg/semverutil.Compare logs every comparison — worth profiling whether this causes overhead during compilation of large workflow repositories.

---

🔄 Next Run Preview

Suggested Focus Areas

• pkg/workflow/ — the largest package (~400 files). Focus on error aggregation patterns, context propagation, and type assertion safety.
• pkg/parser/ — import resolution and frontmatter parsing; complex logic worth auditing for edge cases.
• pkg/cli/ — CLI command handlers; check for proper error wrapping and argument validation.

Strategy Evolution

Next run should use 50% reuse of utility-package-deep-dive (extending to pkg/stats, pkg/console, pkg/tty) and 50% new exploration targeting error wrapping patterns in pkg/workflow (building on the existing open issue #27663).

---

Generated by Sergo - The Serena Go Expert
Run ID: 25076422546
Strategy: utility-package-deep-dive

References:

• §25076422546

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

• expires on Apr 29, 2026, 8:52 PM UTC

[github-actions[bot]]

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

A newer discussion is available at Discussion #29186.