[sergo] Sergo Report: Error-Handling & Dead-Code Analysis - 2026-02-20

[github-actions[bot]]

Date: 2026-02-20
Strategy: error-handling-dead-code (first run — full new exploration)
Success Score: 7/10
Run: §22243186214

Executive Summary

This is the inaugural Sergo run on the gh-aw repository. Using Serena's symbolic search and pattern-matching tools, a deep analysis of 559 production Go files (129,789 LOC) was performed, focusing on error-handling idioms and dead-code patterns — two areas that frequently escape static analysis when key linters are disabled.

The analysis uncovered 4 distinct findings across 3 severity tiers. Most critically, 9 instances of fmt.Errorf("%v", err) should use %w to allow proper error chain traversal via errors.Is / errors.As. Additionally, the MCPConfigProvider.RenderMCPConfig interface design silently discards errors from the underlying RenderJSONMCPConfig helper across 4 engine implementations. Finally, 5 dead-code git-fallback functions in pkg/cli/download_workflow.go carry //nolint:unused suppressions rather than being wired into the call path or removed.

Three improvement tasks are proposed below.

---

🛠️ Serena Tools Update

Tools Snapshot

• Total Tools Available: 23
• 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
find_symbol	Locate interface/function definitions and inspect bodies
search_for_pattern	Regex search across production Go files
get_symbols_overview	High-level symbol inventory per file
find_file	Enumerate Go files within packages
find_referencing_symbols	Cross-reference symbol usages

---

📊 Strategy Selection

Cached Reuse Component (50%)

No cache existed — this is the first run. The 50% cached slot was replaced with a second new-exploration axis.

New Exploration Component (100% for run 1)

Novel Approach A — Error Wrapping Audit

• Hypothesis: Large Go codebases frequently mix %v and %w for error formatting, especially after fast-moving feature work. With errcheck disabled in golangci-lint, this is invisible to CI.
• Tools: search_for_pattern with regex fmt\.Errorf\("[^"]*%v[^"]*", .*err across pkg/
• Target: All non-test production files

Novel Approach B — Dead-Code & nolint Survey

• Hypothesis: Fallback/shim code accumulates over time with nolint:unused rather than being integrated or deleted.
• Tools: search_for_pattern for nolint:unused, find_symbol to inspect suppressed functions
• Target: pkg/cli/download_workflow.go, pkg/workflow/

Combined Strategy Rationale

Both axes share a common theme: gaps in linting coverage. With errcheck, gocritic, and revive all disabled in .golangci.yml (documented as "configuration bugs in golangci-lint v2"), entire categories of Go quality checks are dark. Error-wrapping and dead-code are precisely the issues these linters would normally catch.

---

🔍 Analysis Execution

Codebase Context

Metric	Value
Total Go files	1,520
Production Go files	559
Production LOC	~129,789
Test LOC	~302,248
Packages analyzed	pkg/workflow, pkg/parser, pkg/cli

Findings Summary

Severity	Count
Medium	3
Low	1
Total	4

---

📋 Detailed Findings

Medium Priority Issues

F1 — fmt.Errorf uses %v for error arguments instead of %w

Go 1.13+ introduced %w in fmt.Errorf to wrap errors so callers can use errors.Is() and errors.As() to inspect the error chain. Using %v formats the error as a string, permanently discarding the original error type and preventing chain traversal.

Confirmed instances (non-test production code):

File	Line	Pattern
pkg/workflow/frontmatter_error.go	94	fmt.Errorf("%s: %v", filePath, err)
pkg/workflow/compiler_orchestrator_frontmatter.go	40	fmt.Errorf("failed to read file: %v", err)
pkg/workflow/schema_validation.go	144	fmt.Errorf("%v. %s", err, example)
pkg/cli/logs_command.go	168, 177	fmt.Errorf("invalid ... format '%s': %v", ...)
pkg/cli/enable.go	66, 93	fmt.Errorf("no workflow files found to %s: %v", ...)
pkg/parser/tools_merger.go	120, 223	fmt.Errorf("MCP tool conflict ...: %v", err)

Note: Dual-fallback patterns in pkg/cli/download_workflow.go and pkg/parser/remote_fetch.go use %w for the primary error and %v for the secondary — this is acceptable in Go 1.20+ and intentional; they are excluded from this finding.

F2 — MCPConfigProvider.RenderMCPConfig cannot propagate errors

The MCPConfigProvider interface (pkg/workflow/agentic_engine.go:154) declares:

RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData)

This method has no error return. The underlying helper RenderJSONMCPConfig (pkg/workflow/mcp_renderer.go:890) does return error, so all four engine implementations must discard it:

// pkg/workflow/claude_mcp.go:32
_ = RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{...})

// same pattern in gemini_mcp.go:27, codex_mcp.go:123, copilot_mcp.go:80

Currently RenderJSONMCPConfig always returns nil, so there is no active bug. However, if the function is extended to return real errors (e.g., from renderer callbacks that may be enhanced), those errors will be silently swallowed. The interface design makes correct error propagation impossible without a breaking change.

F3 — Dead fallback functions in pkg/cli/download_workflow.go suppressed with nolint:unused

Five fully-implemented git-fallback functions carry //nolint:unused to silence the linter rather than being wired into callers:

Function	Lines	Annotation
resolveLatestReleaseViaGit	20–97	Fallback implementation for when GitHub API is unavailable
isBranchRefViaGit	103–131	Fallback implementation
isBranchRef	136–163	Reserved for future use
resolveBranchHeadViaGit	167–200	Fallback implementation
resolveBranchHead	204–233	Reserved for future use
resolveDefaultBranchHeadViaGit	237–290	Fallback implementation
resolveDefaultBranchHead	294–322	Reserved for future use

These 300 LOC are invisible to callers and increase cognitive overhead when reading the file. The "reserved for future use" tag in particular suggests they were never used and may never be needed.

Low Priority Findings

F4 — errcheck linter disabled in golangci-lint v2

.golangci.yml explicitly disables errcheck with the comment "Disabled due to exclude-functions not working properly in golangci-lint v2". This means unchecked error returns (beyond the visible _ = ... patterns) are not caught in CI. The gocritic and revive linters are also disabled for similar reasons.

Impact: Any new _ = expr() patterns, or calls to functions that return errors without assignment, go undetected. This reduces the safety net for all future contributions.

Suggested mitigation: Track a follow-up to upgrade or reconfigure golangci-lint so errcheck can be re-enabled with the needed exclusions.

---

✅ Improvement Tasks Generated

Task 1: Replace %v with %w for error wrapping in 9 call sites

Issue Type: Error Handling

Problem:
Nine fmt.Errorf calls pass error arguments with %v instead of %w, discarding the error chain. This prevents callers from using errors.Is() / errors.As() for structured error handling and makes debugging harder.

Locations:

• pkg/workflow/frontmatter_error.go:94
• pkg/workflow/compiler_orchestrator_frontmatter.go:40
• pkg/workflow/schema_validation.go:144
• pkg/cli/logs_command.go:168,177
• pkg/cli/enable.go:66,93
• pkg/parser/tools_merger.go:120,223

Impact:

• Severity: Medium
• Affected Files: 5
• Risk: Existing errors.Is() calls upstream of these sites fail silently

Recommendation: Replace %v with %w for the error argument in each call site. For the two cases in compiler_orchestrator_frontmatter.go:40 where the comment says "Don't wrap os.PathError - format it instead to avoid exposing internals" — if this is intentional, add an explicit comment stating so to differentiate from oversight.

Before (pkg/workflow/compiler_orchestrator_frontmatter.go:40):

return nil, fmt.Errorf("failed to read file: %v", err)

After:

return nil, fmt.Errorf("failed to read file: %w", err)

Validation:

• Run go test ./pkg/workflow/... ./pkg/parser/... ./pkg/cli/...
• Verify no test expectations hard-code the old error string format
• Confirm errors.Is() / errors.As() can now traverse the error chain in relevant test cases

Estimated Effort: Small

---

Task 2: Update MCPConfigProvider.RenderMCPConfig interface to return error

Issue Type: Interface Design / Error Propagation

Problem:
The MCPConfigProvider interface method RenderMCPConfig has no error return value. The underlying helper RenderJSONMCPConfig returns error, so all four engine implementations (ClaudeEngine, GeminiEngine, CodexEngine, CopilotEngine) must discard the error with _ = RenderJSONMCPConfig(...). As the helper evolves, any errors it starts returning will be silently swallowed.

Locations:

• Interface: pkg/workflow/agentic_engine.go:154
• Base impl: pkg/workflow/agentic_engine.go:296
• Claude: pkg/workflow/claude_mcp.go:12
• Gemini: pkg/workflow/gemini_mcp.go:12
• Codex: pkg/workflow/codex_mcp.go:13
• Copilot: pkg/workflow/copilot_mcp.go:12

Impact:

• Severity: Medium
• Affected Files: 6
• Risk: Future errors from RenderJSONMCPConfig (e.g., from renderer callbacks) will be silently dropped

Recommendation: Add an error return to RenderMCPConfig in the interface and update all implementations to return the error from RenderJSONMCPConfig. Update callers of RenderMCPConfig to handle the error.

Before (interface):

RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData)

After:

RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error

Before (engine implementation):

func (e *ClaudeEngine) RenderMCPConfig(...) {
    _ = RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{...})
}

After:

func (e *ClaudeEngine) RenderMCPConfig(...) error {
    return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{...})
}

Validation:

• Ensure all callers of RenderMCPConfig (via MCPConfigProvider interface) check the returned error
• Run go build ./... to confirm no compile errors
• Run go test ./pkg/workflow/...

Estimated Effort: Medium

---

Task 3: Wire or remove git-fallback dead code in pkg/cli/download_workflow.go

Issue Type: Dead Code / Code Quality

Problem:
Seven functions in pkg/cli/download_workflow.go (≈300 LOC) are annotated with //nolint:unused because they are never called. The annotations indicate these are "Fallback implementations for when GitHub API is unavailable" or are "Reserved for future use." They have been in this state long enough that the nolint suppression was committed, yet no caller exists.

Locations:

• pkg/cli/download_workflow.go:20 — resolveLatestReleaseViaGit
• pkg/cli/download_workflow.go:102 — isBranchRefViaGit
• pkg/cli/download_workflow.go:135 — isBranchRef
• pkg/cli/download_workflow.go:166 — resolveBranchHeadViaGit
• pkg/cli/download_workflow.go:203 — resolveBranchHead
• pkg/cli/download_workflow.go:236 — resolveDefaultBranchHeadViaGit
• pkg/cli/download_workflow.go:293 — resolveDefaultBranchHead

Impact:

• Severity: Low–Medium
• Affected Files: 1
• Risk: Dead code that carries nolint suppressions reduces signal quality of the linter output and creates maintenance overhead. If the fallback behavior is genuinely needed, it should be integrated with tests; if not, it should be deleted.

Recommendation:

1. Decision gate: Determine whether the API-fallback behavior is needed (network interruption resilience). If yes, wire the functions into the call sites (e.g., as a fallback in update_actions.go where the dual-fallback pattern already exists for other operations).
2. If the fallback is not needed or already covered elsewhere, delete the dead functions and their nolint suppressions.
3. If the functions are intentionally preserved for future integration, move them to a clearly named _git_fallback.go file with a package-level doc comment explaining the intent, and add a tracking issue reference.

Validation:

• Run go build ./... after removal/wiring
• Run go test ./pkg/cli/...
• Confirm golangci-lint run ./pkg/cli/... passes without new suppressions needed

Estimated Effort: Small (delete path) or Medium (wire path)

---

📈 Success Metrics

This Run

Metric	Value
Findings generated	4
Tasks created	3
Production files analyzed	559
Packages covered	pkg/workflow, pkg/parser, pkg/cli
Success Score	7/10

Reasoning for Score

• Findings Quality (5/4): 3 medium + 1 low. All actionable. The error-wrapping finding alone touches 9 call sites across 5 files. No critical/security issues found (-1 from max). → 3/4
• Coverage (3/3): Covered all three major packages including the two largest (workflow + parser). → 3/3
• Task Generation (3/3): All 3 tasks are well-scoped with before/after code, locations, and validation steps. → 3/3 → Adjusted to 7/10 for first-run baseline (no comparative historical data).

---

📊 Historical Context

Strategy Performance

Date	Strategy	Score	Findings	Tasks
2026-02-20	error-handling-dead-code	7/10	4	3

Cumulative Statistics

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

---

🎯 Recommendations

Immediate Actions

1. [Medium] Replace %v → %w for error wrapping in 9 call sites (Task 1) — small effort, high correctness benefit
2. [Medium] Add error return to MCPConfigProvider.RenderMCPConfig interface (Task 2) — prevents silent future failures
3. [Low–Medium] Decide on and act upon dead git-fallback code in download_workflow.go (Task 3)

Long-term Improvements

• Re-enable errcheck: The linter suppression of errcheck, gocritic, and revive is a lint-debt accumulator. Once golangci-lint v2 configuration stabilizes, re-enabling these would surface many of the patterns found in this run automatically.
• Enforce %w via linting: wrapcheck or custom revive rules can enforce %w usage when wrapping external errors.

---

🔄 Next Run Preview

Suggested Focus Areas

• Interface surface audit: Inspect all 13 interfaces in pkg/workflow/ and pkg/cli/ for consistency — especially method signatures that could benefit from error returns.
• Context propagation: 8 context.Background() usages in production code; check if they could accept a passed context from callers.
• Long function analysis: pkg/parser/import_processor.go (1,109 LOC), pkg/cli/trial_command.go (1,013 LOC) — candidate for complexity/cyclomatic analysis.

Strategy Evolution

Next run should use 50% cached (repeat error-handling analysis on packages not yet covered: pkg/types, pkg/gitutil, internal/) + 50% new (context propagation audit using find_referencing_symbols on context-accepting functions).

---

References:

• §22243186214

AI generated by Sergo - Serena Go Expert

• expires on Feb 27, 2026, 10:20 PM UTC

[pelikhan]

/plan

[pelikhan]

/a investigate if the cache-memory is working in sergo

[pelikhan]

/q investigate if the cache-memory is working in sergo

[github-actions[bot]]

🔍 Cache-Memory Investigation: Sergo

TL;DR: Cache-memory was broken in sergo for all past runs due to a key mismatch bug. The fix is already in the current lock file and will take effect from the next daily run.

---

Root Cause

The update_cache_memory job was missing GH_AW_WORKFLOW_ID_SANITIZED in its env block in the older compiled lock file (commit 56a339617). This caused cache keys to be saved as memory--{run_id} (empty workflow ID) instead of memory-sergo-{run_id}.

Meanwhile, the agent job correctly tried to restore from memory-sergo- prefix — which never matched the malformed saved keys.

Smoking gun from run #22243186214 (Feb 20) logs:

Cache saved with key: memory--22243186214   ← double-dash, empty workflow ID!

When the next run tried to restore with memory-sergo-, it found nothing.

---

Evidence

Every run across 5 analyzed runs (Feb 11–20) reported:

cat /tmp/gh-aw/cache-memory/sergo-strategies.jsonl → NO_CACHE
cat /tmp/gh-aw/cache-memory/sergo-tools-list.json  → NO_CACHE

And each run claimed "this is the first run" — even run #37 (the 5th scheduled run).

The artifact was being uploaded correctly (3 files, ~1440 bytes of valid cache data), but the cache save key was wrong, so no future run could restore it.

---

Fix

Already in the current lock file (HEAD @ a90de1b9):

# update_cache_memory job now has:
env:
  GH_AW_WORKFLOW_ID_SANITIZED: sergo   ← this was missing before!

The compiler was fixed to always include GH_AW_WORKFLOW_ID_SANITIZED in update_cache_memory's env block (see pkg/workflow/cache.go:827 and the new test TestUpdateCacheMemoryJobHasWorkflowIDEnv).

---

Current Status

Run	Date	Cache Restore	Cache Save Key	Result
#30–37	Feb 11–18	memory-sergo- (no match)	memory--{id} ❌	NO_CACHE
#38	Feb 20	memory-sergo- (no match)	memory--{id} ❌	NO_CACHE
#39 (next)	Feb 21 ~22:xx UTC	memory-sergo- (still no match — first clean run)	memory-sergo-{id} ✅	Will write correctly
#40+	Feb 22+	memory-sergo- → MATCH ✅	memory-sergo-{id} ✅	Cache working!

No code changes needed — the fix is already deployed. The cache will start working from the second run after tonight's scheduled execution.

---

Side Note: 67 Blocked Network Requests

Each sergo run also shows ~67 blocked network requests (domain listed as -). These appear to be Serena MCP internal requests being blocked by the firewall. This is unrelated to cache-memory but worth noting for future investigation.

🎩 Equipped by Q

[pelikhan]

/q review sergo blocked domains from serena

[github-actions[bot]]

🔍 Sergo Blocked Domains — Investigation Complete

TL;DR: The 78–91 "blocked" domain - requests reported by the audit tool are false positives. There are no actual blocked external domains in Sergo runs. No changes to sergo.md are needed.

---

Investigation

Evidence 1: AWF Binary Reports 0 Blocked

The authoritative source — the AWF binary (awf) — reads the raw squid access log at job completion and reports:

▼ 78 requests | 78 allowed | 0 blocked | 2 unique domains

Run #22243186214 (Feb 20). Zero blocked requests.

Evidence 2: Audit Tool Discrepancy

The audit MCP tool reports the same run as having 78 "blocked" requests with domain -:

"blocked_domains": ["-"],
"blocked_requests": 78,
"requests_by_domain": { "-": { "allowed": 0, "blocked": 78 } }
```

This is a discrepancy: AWF says **0 blocked**, audit tool says **78 blocked**.

#### Evidence 3: Root Cause — Squid `NONE_NONE:HIER_NONE` Log Entries

The squid access log contains entries in this format:
```
1761332530.500 - - - - - 0 NONE_NONE:HIER_NONE - "-"

These are entries where all fields are - (placeholder). They appear when squid logs internal state changes, failed TCP setups, or Docker network connections that never completed as HTTP transactions. The NONE_NONE:HIER_NONE decision with status 0 means "squid didn't handle this as a real HTTP request."

The Go firewall parser in pkg/cli/firewall_log.go correctly identifies NONE_NONE as "blocked" per its classification logic — but this is a false positive for these all-- placeholder entries. The AWF binary filters them out as noise (they're not real HTTP/HTTPS requests).

Evidence 4: All Serena Network Needs Are Covered

Cross-checking Serena's observed actual domains across runs against the allowed list:

Domain	Seen in Logs	In Allowed List
api.anthropic.com	✅	✅ (defaults)
proxy.golang.org	✅	✅ (go)
sum.golang.org	✅	✅ (go)
storage.googleapis.com	✅	✅ (defaults)
registry.npmjs.org	(bash/TS LSP deps)	✅ (defaults)
ghcr.io	(container pull)	✅ (defaults)

No external domain is actually being blocked.

Evidence 5: TypeScript LSP Timeout — Not a Network Issue

The Serena container starts TypeScript and Bash language servers in addition to Go (due to the codex context). The TypeScript server logs "Timeout waiting for TypeScript server to become ready, proceeding" — but this completes in ~1.5 seconds and doesn't affect Sergo's successful operation. This is not a network/firewall issue.

---

What the - Entries Actually Are

The 78–91 NONE_NONE:HIER_NONE - entries are generated by squid when:

1. Internal Docker connections from Serena's language servers (gopls, tsserver, bash-lsp) connect to localhost/Docker-network ports — these get intercepted by iptables and logged by squid even though they're internal
2. Failed TCP setups that squid sees at the connection level but that never become HTTP transactions
3. Connection probes from language server initialization

The iptables rules redirect all port 80/443 traffic through squid (including Docker-internal traffic), so these appear in the log — but they're not real blocked external requests.

---

Conclusion

✅ No changes needed to sergo.md — the network configuration (defaults + github + go) fully covers Serena's requirements.

⚠️ Known issue in audit tool: The firewall_analysis in run summaries double-counts these placeholder entries as blocked external requests. This is a false positive in the Go firewall log parser (pkg/cli/firewall_log.go). A potential improvement would be to skip entries where domain=- and all fields are placeholders, since those aren't actionable blocked requests. That's a separate improvement from what was asked here.

---

Runs analyzed: #22243186214 (Feb 20), #22202323077 (Feb 19)
Verdict: Sergo blocked domains = false positive in audit tool

🎩 Equipped by Q

🎩 Equipped by Q

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• productionresultssa13.blob.core.windows.net