[sergo] Sergo Report: Code Quality Scan - 2026-04-16

[github-actions[bot]]

Date: 2026-04-16
Strategy: Code Quality Scan (first run — no prior cache)
Success Score: 7/10

Executive Summary

This is the inaugural Sergo run on the gh-aw repository. With no prior strategy cache to draw from, the analysis applied a broad code-quality scan strategy using Serena's LSP symbol search and pattern-matching tools across pkg/workflow/ — the core package comprising ~715 non-test Go files and ~160K LOC.

Four actionable findings were identified spanning code clarity, efficiency, and silent error handling. Two GitHub issues were filed for the highest-impact items. The overall codebase is well-structured with consistent conventions, but a small set of non-idiomatic patterns were found that are straightforward to clean up.

---

🛠️ Serena Tools Update

Tools Snapshot

• Total Tools Available: 22
• New Tools Since Last Run: N/A (first run)
• Removed Tools: N/A
• Modified Tools: N/A

Tool Capabilities Used Today

Tool	Purpose
activate_project	Initialize LSP for Go analysis
get_symbols_overview	Survey struct/method layout of key files
find_symbol	Locate specific functions and types
find_referencing_symbols	Trace callers of Register
search_for_pattern	Find anti-patterns: panic(, _ =, TODO, return nil, nil
list_dir	Enumerate pkg/workflow/ file tree
think_about_collected_information	Validate sufficiency before concluding

---

📊 Strategy Selection

Cached Reuse Component (50%)

No prior cache available. As a first run, the "cached" slot was filled by a well-established static analysis baseline: error handling and code idiom review. This mirrors the highest-yield strategies documented in Go code quality literature (effective error propagation, idiomatic blank identifiers, standard library use over hand-rolled implementations).

New Exploration Component (50%)

Novel Approach: Pattern-based structural search combined with LSP symbol navigation.

• Used search_for_pattern to find cross-cutting anti-patterns (panic(, _ = , TODO, return nil, nil, \+= fmt.Sprintf) across all non-test Go files
• Used get_symbols_overview + find_symbol to inspect specific structs (EngineRegistry, ScriptRegistry, EngineCatalog) for concurrency hygiene
• Targeted pkg/workflow/ exclusively (the largest and most complex package)

Hypothesis: In a 715-file package with many contributors, common idiom drift (manual sorts, post-declaration discards, silently swallowed errors) is likely to accumulate.

Combined Strategy Rationale

The combination of broad pattern search (surface area) and targeted symbol inspection (depth) allows efficiently finding both widespread and localized issues in a single pass.

---

🔍 Analysis Execution

Codebase Context

• Total Go Files (non-test): 715
• Packages Analyzed: pkg/workflow/ (primary focus)
• LOC Analyzed: ~160,000 (pkg/workflow alone)
• Focus Areas: script_registry.go, agent_validation.go, docker_validation.go, mcp_scripts_parser.go, agentic_engine.go, compiler_safe_outputs_steps.go

Findings Summary

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

---

📋 Detailed Findings

Medium Priority Issues

[M1] Bubble sort instead of sort.Strings() in GetAllScriptFilenames

• File: pkg/workflow/script_registry.go:69–77
• Description: The function implements a manual O(n2) bubble sort with nested for loops rather than calling sort.Strings(). The same package uses sort.Strings() correctly in GetSupportedEngines() (agentic_engine.go:485), making this an inconsistency.
• Impact: Code clarity, minor performance, maintainability
• Fix: Replace the 9-line sort block with sort.Strings(filenames) and remove the unnecessary copy

Low Priority Issues

[L1] Post-assignment blank identifier suppression (_ = variable)

Two instances use the non-idiomatic pattern of assigning to a named variable then discarding it with a comment:

File	Line	Pattern
pkg/workflow/agent_validation.go	113–114	engineSetting, engineConfig := ...; _ = engineSetting
pkg/workflow/docker_validation.go	132–137	output, err := cmd.CombinedOutput(); _ = output

The idiomatic Go pattern is to use _ at the declaration site: _, engineConfig := ...

[L2] Silent fmt.Sscanf error discard in mcp_scripts_parser.go

• File: pkg/workflow/mcp_scripts_parser.go:184,355
• Description: _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) silently leaves toolConfig.Timeout at its zero value if the string does not parse as an integer. While intentional (fall through to 0), this would benefit from a comment or a strconv.Atoi call that makes the zero-default explicit.

Additional observations (not filed as issues)

init() panics on embedded JSON (domains.go, permissions_toolset_data.go, engine_definition_loader.go, github_tool_to_toolset.go): These are acceptable startup-time invariant checks for statically-embedded data. Not actionable as bugs, but worth noting for future robustness reviews.

One outstanding TODO (pkg/workflow/compiler_safe_outputs_steps.go:130): A // TODO: @dsyme says: We must remove this. comment describing a known architectural debt around safe-outputs checkout ref resolution. Tracked for future work.

---

✅ Improvement Tasks Generated

Task 1: Replace bubble sort with sort.Strings in GetAllScriptFilenames [Medium]

Problem: GetAllScriptFilenames() uses a manual O(n2) bubble sort (9 lines) when sort.Strings() is available and used elsewhere in the package.

Location: pkg/workflow/script_registry.go:69–77

Before:

sortedFilenames := make([]string, len(filenames))
copy(sortedFilenames, filenames)
for i := range sortedFilenames {
    for j := i + 1; j < len(sortedFilenames); j++ {
        if sortedFilenames[i] > sortedFilenames[j] {
            sortedFilenames[i], sortedFilenames[j] = sortedFilenames[j], sortedFilenames[i]
        }
    }
}
return sortedFilenames

After:

sort.Strings(filenames)
return filenames

Add "sort" to the import block. Estimated Effort: Small.

---

Task 2: Use _ at declaration site instead of post-assignment discard [Low]

Problem: Two files use _ = variable // Suppress unused variable warning instead of the idiomatic blank identifier at declaration.

Locations:

• pkg/workflow/agent_validation.go:113–114
• pkg/workflow/docker_validation.go:132–137

Fix: Change engineSetting, engineConfig := ... to _, engineConfig := ... and output, err := ... to _, err := ...

Estimated Effort: Small.

---

Task 3: Make fmt.Sscanf fallback explicit in mcp_scripts_parser.go [Low]

Problem: _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) silently produces a zero value on parse failure with no indication this is intentional.

Location: pkg/workflow/mcp_scripts_parser.go:184,355

Recommendation: Replace with strconv.Atoi and explicit fallback comment, or add an inline comment like // invalid string: toolConfig.Timeout remains 0 (no timeout).

Estimated Effort: Small.

---

📈 Success Metrics

This Run

• Findings Generated: 4
• Tasks Created: 3
• Files Analyzed: ~50 (deep) / 715 (surveyed)
• Success Score: 7/10

Reasoning for Score

• Findings Quality (3/4): Two medium/low issues filed as actionable issues; two lower-priority observations documented but not filed
• Coverage (2/3): Deep analysis of pkg/workflow/ only; other packages (pkg/cli/, pkg/parser/) not analyzed
• Task Generation (2/3): Three well-specified tasks generated; none were complex architectural items

---

📊 Historical Context

Strategy Performance

First run — no prior history.

Cumulative Statistics

• Total Runs: 1
• Total Findings: 4
• Total Tasks Generated: 3
• Average Success Score: 7.0/10
• Most Successful Strategy: code-quality-scan

---

🎯 Recommendations

Immediate Actions

1. [Medium] Replace bubble sort in GetAllScriptFilenames — 2-line fix, high clarity gain
2. [Low] Fix _ = variable pattern in agent_validation.go and docker_validation.go
3. [Low] Make zero-timeout fallback explicit in mcp_scripts_parser.go

Long-term Improvements

• Consider adding a golangci-lint rule for wastedassign or ineffassign to catch post-declaration discards automatically
• The TODO in compiler_safe_outputs_steps.go references a meaningful architectural refactor worth tracking as a dedicated issue

---

🔄 Next Run Preview

Suggested Focus Areas

• pkg/cli/ — the logs_download.go and audit.go files are each ~870–890 lines; worth complexity analysis
• pkg/parser/ — remote_fetch.go (928 lines) is dense; error handling and context propagation worth reviewing
• Context propagation: context.Background() is used in several places (action_resolver.go, safe_outputs_actions.go, github_cli.go) that could accept a passed-in context instead

Strategy Evolution

Next run should allocate 50% to the proven code-quality-scan strategy (adapted to pkg/cli/ and pkg/parser/) and 50% to a new context-propagation analysis targeting context.Background() usage in functions that could thread a context from callers.

---

References:

• §24532213054

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

• expires on Apr 17, 2026, 8:34 PM UTC

[github-actions[bot]]

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

A newer discussion is available at Discussion #26935.