[sergo] Sergo Report: Code Duplication & Error Handling in mcp_scripts_parser - 2026-04-27

[github-actions[bot]]

Date: 2026-04-27
Strategy: Code Duplication & Error Handling (first run — no cache)
Success Score: 7/10
Run ID: §25018127451

Executive Summary

This is the first Sergo run on this repository. The codebase is a substantial Go project (727 non-test .go files, ~168K LOC across pkg/) implementing a GitHub Actions workflow compiler and agentic execution framework. The pkg/workflow package is the largest and most complex, containing the core compiler, engine definitions, MCP configuration, and runtime tooling.

Analysis focused on pkg/workflow/mcp_scripts_parser.go using Serena's semantic symbol search and pattern matching. Three actionable findings were identified: a significant code duplication (tool-config parsing logic duplicated across two functions), a silent error suppression inconsistency (timeout string parsing discards the parse error), and a dead function parameter left in the public-internal API. Three GitHub issues were created.

Serena's LSP server disconnected mid-session; analysis was completed using native Grep/Read tools with no reduction in finding quality.

---

Serena Tools Update

Tools Snapshot

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

Tool Capabilities Used Today

Tool	Usage
activate_project	Bootstrapped LSP analysis for Go
get_symbols_overview	Scanned file-level symbol maps (engine.go, compiler.go, mcp_scripts_parser.go, tools_parser.go)
find_symbol	Located parseMCPScriptsMap, parseMCPScriptToolConfig, parseTimeoutTool, SafeUint64ToInt, IsMCPScriptsEnabled
search_for_pattern	Searched for _ =, fmt.Sscanf, panic(, TODO/FIXME, context usage
list_dir	Enumerated pkg/workflow, pkg/, cmd/ structure

---

Strategy Selection

This is Run #1 (No Cache Available)

Since no prior strategy cache exists, 100% of this run used a new exploration approach. The baseline will seed the 50% cached / 50% new split for future runs.

New Exploration: Code Quality & Error Handling Audit

Approach: Start from well-tested parser files that handle user-supplied config (high blast radius), cross-reference against codebase patterns to identify inconsistencies.

Tools employed: Symbol overview → pattern search (_ =, fmt.Sscanf, panic() → symbol body inspection → cross-reference via Grep.

Hypothesis: Configuration parsing code tends to accumulate debt: silent error handling, copy-paste duplication when features are added, and API parameters that outlive their purpose. The mcp_scripts_parser.go file, being relatively new and containing two similar functions, was a good candidate.

Target areas: pkg/workflow/mcp_scripts_parser.go, pkg/workflow/tools_parser.go, pkg/typeutil/convert.go

---

Analysis Execution

Codebase Context

• Total Go files (non-test): 727 across pkg/
• Largest files: domains.go (1011 LOC), compiler_jobs.go (995 LOC), compiler_yaml_main_job.go (917 LOC)
• Focus area: pkg/workflow/mcp_scripts_parser.go (366 LOC), pkg/workflow/tools_parser.go (543 LOC)
• Dependencies of note: anthropic-sdk-go, rhysd/actionlint, securego/gosec, modelcontextprotocol/go-sdk

Findings Summary

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

---

Detailed Findings

Medium Priority: Code Duplication in mcp_scripts_parser.go

Location: pkg/workflow/mcp_scripts_parser.go:72–191 and 259–357

Both parseMCPScriptsMap and the inner loop of (*Compiler).mergeMCPScripts parse a map[string]any into a *MCPScriptToolConfig. The logic — description, inputs (with nested param fields), script/run/py/go strings, env map, and a 4-case timeout type switch — is copy-pasted verbatim between the two functions.

Evidence of independent patching: the uint64 overflow safety fix references alert #414 in parseMCPScriptsMap and alert #413 in mergeMCPScripts, suggesting the two sites were patched in separate PRs.

Impact: Medium — any future MCPScriptToolConfig field addition requires two edits and two test updates. Divergence risk increases over time.

Recommendation: Extract parseMCPScriptToolConfig(toolName string, toolMap map[string]any) *MCPScriptToolConfig and call it from both sites.

---

Medium Priority: Silent Timeout Parse Error in mcp_scripts_parser.go

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

case string:
    // Try to parse string as integer
    _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout)

Both the count and the error from fmt.Sscanf are discarded. A string value like "5m", "two minutes", or "abc" will silently leave toolConfig.Timeout at its 60-second default.

This is the only place in pkg/ where fmt.Sscanf errors are suppressed — all 14 other call sites check the error. The parallel pattern in time_delta.go:432 logs a warning on failure.

Impact: Medium — user configuration mistakes in mcp-scripts timeout fields produce no diagnostic output.

Recommendation: Replace with strconv.Atoi and log a warning via mcpScriptsLog when parsing fails.

---

Low Priority: Dead Parameter on IsMCPScriptsEnabled

Location: pkg/workflow/mcp_scripts_parser.go:56

// The workflowData parameter is kept for backward compatibility but is not used.
func IsMCPScriptsEnabled(mcpScripts *MCPScriptsConfig, workflowData *WorkflowData) bool {
    return HasMCPScripts(mcpScripts)
}

The function body ignores workflowData entirely. The "backward compatibility" comment is misleading — this is internal-only code with no external API contract. Thirteen production call sites are forced to pass workflowData unnecessarily.

Recommendation: Remove the parameter (or inline IsMCPScriptsEnabled entirely as HasMCPScripts).

---

Improvement Tasks Generated

Task 1: Extract parseMCPScriptToolConfig helper

Issue Type: Code Duplication

Problem: ~90 lines of tool-config parsing logic duplicated in parseMCPScriptsMap and mergeMCPScripts.

Locations:

• pkg/workflow/mcp_scripts_parser.go:83–186 (in parseMCPScriptsMap)
• pkg/workflow/mcp_scripts_parser.go:259–357 (in mergeMCPScripts)

Severity: Medium | Effort: Small

Before (duplicated in two places):

toolConfig := &MCPScriptToolConfig{Name: toolName, Inputs: ..., Env: ..., Timeout: 60}
if desc, exists := toolMap["description"]; ... { ... }
// ... 80 more lines ...

After:

func parseMCPScriptToolConfig(toolName string, toolMap map[string]any) *MCPScriptToolConfig {
    // single implementation
}

---

Task 2: Fix silent error suppression in timeout string parsing

Issue Type: Error Handling

Problem: _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) hides invalid timeout values.

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

Severity: Medium | Effort: Small

Before:

case string:
    _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout)

After:

case string:
    if n, err := strconv.Atoi(t); err == nil {
        toolConfig.Timeout = n
    } else {
        mcpScriptsLog.Printf("Warning: invalid timeout %q for tool %q, using default 60s", t, toolName)
    }

---

Task 3: Remove dead workflowData parameter from IsMCPScriptsEnabled

Issue Type: API Hygiene / Dead Code

Problem: Unused parameter on a function called from 13 internal sites.

Location: pkg/workflow/mcp_scripts_parser.go:56 + 13 call sites

Severity: Low | Effort: Small

---

Success Metrics

This Run

• Findings Generated: 3
• Tasks Created: 3
• Issues Created: 3
• Files Analyzed: ~10 non-test Go files (deep), ~727 broadly scanned
• Success Score: 7/10

Reasoning for Score

• Findings Quality (3/4): All three findings are concrete, localized, and actionable with specific line numbers. No critical bugs found (reducing score).
• Coverage (2/3): Deep focus on one file cluster; broader codebase scan was limited by Serena MCP disconnection mid-session.
• Task Generation (2/3): Three well-specified tasks created. Score capped because the findings are maintenance-level rather than high-severity.

Cumulative Statistics (Run 1/1)

• Total Runs: 1
• Total Findings: 3
• Total Tasks Generated: 3
• Average Success Score: 7.0/10
• Most Successful Strategy: code-duplication-and-error-handling

---

Recommendations

Immediate Actions

1. [Medium] Extract parseMCPScriptToolConfig — eliminates dual-maintenance of MCP script tool parsing; prerequisite for Task 2 to only need one fix location.
2. [Medium] Fix silent fmt.Sscanf suppression — adds user-visible diagnostics for misconfigured mcp-scripts timeouts.
3. [Low] Remove dead workflowData parameter — reduces API noise across 13 call sites.

Long-term Improvements

• The mcp_scripts_parser.go and tools_parser.go files follow similar map[string]any → struct parsing patterns. A future exploration pass could audit tools_parser.go's parseWebFetchTool / parseWebSearchTool / parseEditTool functions, all of which ignore their val any parameter — potentially silently discarding user config.
• pkg/workflow/compiler_jobs.go (995 LOC) and pkg/workflow/compiler_yaml_main_job.go (917 LOC) are large files that may benefit from a complexity/function-length analysis in a future run.

---

Next Run Preview

Suggested Focus Areas

• Large file complexity: domains.go (1011 LOC), compiler_jobs.go (995 LOC) — function-length and cyclomatic complexity analysis
• Unexplored packages: pkg/cli/audit.go, pkg/parser/remote_fetch.go — both >900 LOC and not yet analyzed
• parseWebFetchTool / parseWebSearchTool / parseEditTool: These ignore their val parameter; verify no config is silently dropped

Strategy Evolution

• Run 2 should use 50% cached (re-apply error-handling audit to pkg/cli/ or pkg/parser/) + 50% new (complexity / large-file analysis using LOC metrics as the entry point).

---

Generated by Sergo — The Serena Go Expert
Run ID: §25018127451
Strategy: code-duplication-and-error-handling (first run)

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

• expires on Apr 28, 2026, 8:44 PM UTC

[github-actions[bot]]

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

A newer discussion is available at Discussion #28980.