[refactor] Semantic Function Clustering Analysis — Engine inconsistencies, helper sprawl, and consolidation opportunities

[github-actions]

Overview

This analysis covers 761 non-test Go files across 24 packages in pkg/. The largest packages — pkg/workflow (368 files) and pkg/cli (191 files) — contain the most significant refactoring opportunities. The codebase has a strong architectural foundation; the findings below focus on high-impact consolidation and consistency improvements.

Analysis date: 2026-05-06 | Workflow run: §25408944594

---

Summary

Category	Findings	Priority
AI engine file organization	Copilot splits files; others don't	Medium
Tool conversion function signatures	4 engines, 4 different signatures	Medium
MCP config rendering inconsistency	Codex deviates from shared helper	Medium
AWF version-check duplication	3 functions, identical pattern	Low
addEngineFlag / addEngineFilterFlag	Near-identical CLI flag adders	Low
Compiler functional options vs setters	Two parallel configuration patterns	Low
effectiveActionsRepo duplication	Private wrapper over identical public	Low
String utility cross-package overlap	Intentional separation — no action	None

---

1. AI Engine File Organization Inconsistency

Files: pkg/workflow/{claude,codex,gemini,copilot}_engine.go and related

Copilot splits its implementation across 6 files while the other engines mix most logic into *_engine.go:

File	Claude	Codex	Gemini	Copilot
*_engine.go	Core + install + exec	Core + install + exec	Core + install + exec	Core only
*_installation.go	❌	❌	❌	✅
*_execution.go	❌	❌	❌	✅
*_tools.go	✅	(inline)	✅	✅
*_mcp.go	✅	✅	✅	✅
*_logs.go	✅	✅	✅	✅

Claude's GetExecutionSteps is ~488 lines, Codex's is ~428 lines. The Copilot approach (splitting into *_installation.go and *_execution.go) is the better model.

Recommendation: Extract GetInstallationSteps + GetSecretValidationStep and GetExecutionSteps into dedicated files for Claude, Codex, and Gemini, matching the Copilot pattern.

Missing method coverage matrix

Method	Claude	Codex	Gemini	Copilot
GetPreBundleSteps()	❌	❌	✅	❌
GetFirewallLogsCollectionStep()	❌	❌	❌	✅
GetCleanupStep()	❌	❌	❌	✅
GetLogFileForParsing()	❌	❌	✅	✅

These are provided by BaseEngine as no-ops, so there's no bug — but documenting which engines override them would aid future development.

---

2. Tool Conversion Function Signatures — Four Different Patterns

Files: pkg/workflow/claude_tools.go, codex_engine.go, gemini_tools.go, copilot_engine_tools.go

All engines convert "neutral" tool configuration to engine-specific formats, but with inconsistent function names and return types:

// Claude (claude_tools.go)
func expandNeutralToolsToClaudeTools(tools map[string]any) map[string]any
func computeAllowedClaudeToolsString(tools, safeOutputs, cacheMemoryConfig, mcpScripts) string

// Codex (codex_engine.go)
func expandNeutralToolsToCodexTools(toolsConfig *ToolsConfig) *ToolsConfig
func expandNeutralToolsToCodexToolsFromMap(tools map[string]any) *ToolsConfig  // backward compat

// Gemini (gemini_tools.go)
func computeGeminiToolsCore(tools map[string]any) []string

// Copilot (copilot_engine_tools.go)
func computeCopilotToolArguments(tools map[string]any, safeOutputs, mcpScripts, workflowData) []string
func generateCopilotToolArgumentsComment(...) string

Recommendation: Standardize naming convention (e.g., compute{Engine}ToolArguments) and consider a shared ToolConverter interface if a new engine is added.

---

3. MCP Config Rendering — Codex Deviates from Shared Helper

Files: pkg/workflow/claude_mcp.go, codex_mcp.go, gemini_mcp.go, copilot_mcp.go

Claude, Gemini, and Copilot all delegate to renderStandardJSONMCPConfig(). Codex has custom TOML+JSON logic:

// Claude, Gemini, Copilot
func (e *ClaudeEngine) RenderMCPConfig(...) { renderStandardJSONMCPConfig(...) }

// Codex
func (e *CodexEngine) RenderMCPConfig(...) {
    // Custom TOML rendering + JSON conversion
    renderOpenAIProxyProviderToml(...)
    renderAppendConvertedConfigWithoutOpenAIProxy(...)
    renderCodexMCPConfigWithContext(...)
    renderCodexJSONMCPConfigWithContext(...)
}

The context rendering callbacks also have an inconsistency: Claude/Copilot pass isLast bool, while the Codex TOML variant omits it.

Recommendation: Document the intentional divergence with a comment in codex_mcp.go explaining why TOML is required, so future developers don't attempt to unify them unnecessarily.

---

4. AWF Version-Check Functions — Identical Pattern Repeated Three Times

File: pkg/workflow/awf_helpers.go

func awfSupportsExcludeEnv(firewallConfig *FirewallConfig) bool {
    // check semver >= some minimum
}
func awfSupportsCliProxy(firewallConfig *FirewallConfig) bool {
    // same pattern, different version
}
func awfSupportsAllowHostPorts(firewallConfig *FirewallConfig) bool {
    // same pattern, different version
}

All three follow the same structure: extract version from firewallConfig, compare against a minimum semver.

Recommendation: Extract to a single generic helper:

func awfSupportsFeature(firewallConfig *FirewallConfig, minVersion string) bool

Then each specific function becomes a one-liner wrapping this helper.

---

5. addEngineFlag and addEngineFilterFlag — Near-Identical CLI Flag Adders

File: pkg/cli/flags.go

Both functions register --engine/-e on a cobra.Command with different descriptions. The code structure is identical:

func addEngineFlag(cmd *cobra.Command) {
    cmd.Flags().StringP("engine", "e", "", engineFlagUsage(""))
}
func addEngineFilterFlag(cmd *cobra.Command) {
    cmd.Flags().StringP("engine", "e", "", engineFlagUsage("filter "))
}

Recommendation: Consolidate:

func addEngineFlagWithUsagePrefix(cmd *cobra.Command, prefix string) {
    cmd.Flags().StringP("engine", "e", "", engineFlagUsage(prefix))
}

---

6. Compiler — Two Parallel Configuration Patterns

File: pkg/workflow/compiler_types.go

The Compiler exposes both the functional-options pattern and imperative setter methods:

// Functional options (used at construction)
WithVerbose(verbose bool) CompilerOption
WithEngineOverride(engine string) CompilerOption
// ... 5 more

// Imperative setters (used post-construction)
func (c *Compiler) SetSkipValidation(skip bool)
func (c *Compiler) SetRequireDocker(require bool)
// ... ~20 more

This creates an inconsistency where some options can only be set at construction (via With*) while others are always set imperatively (via Set*), and it's unclear why the split exists.

Recommendation: Add a comment to compiler_types.go explaining the design intent (e.g., "functional options for immutable construction-time config; setters for runtime-mutable state"). If there's no meaningful distinction, consolidate to one pattern.

---

7. effectiveActionsRepo — Private Wrapper Over Public Method

File: pkg/workflow/compiler_types.go

func (c *Compiler) effectiveActionsRepo() string { ... }  // private
func (c *Compiler) EffectiveActionsRepo() string { ... }  // public

If both have the same body, the private version is dead code. If the private one has additional logic, that logic should be in the public version.

Recommendation: Consolidate to EffectiveActionsRepo() and replace all internal calls to effectiveActionsRepo() with the public method.

---

8. parseTargetRepoFromConfig and parseTargetRepoWithValidation — Overlapping Parsers

File: pkg/workflow/config_helpers.go

func parseTargetRepoFromConfig(configMap map[string]any) string
func parseTargetRepoWithValidation(configMap map[string]any) (string, bool)

Both extract the same field. The second adds wildcard validation.

Recommendation: Implement parseTargetRepoFromConfig as a call to parseTargetRepoWithValidation (ignoring the bool), or merge into a single function with an optional validation flag.

---

Implementation Checklist

• Split claude_engine.go and codex_engine.go installation/execution into dedicated files (matches Copilot pattern)
• Standardize tool conversion function naming across all engines
• Add comment to codex_mcp.go documenting TOML requirement
• Extract awfSupportsFeature(config, minVersion) helper in awf_helpers.go
• Consolidate addEngineFlag / addEngineFilterFlag in pkg/cli/flags.go
• Add comment or consolidate Compiler options vs setters in compiler_types.go
• Investigate effectiveActionsRepo vs EffectiveActionsRepo duplication
• Merge parseTargetRepoFromConfig into parseTargetRepoWithValidation

Analysis Metadata

• Total Go files analyzed: 761 (non-test, across 24 packages)
• Primary focus: pkg/workflow (368 files), pkg/cli (191 files)
• Detection method: Serena LSP semantic analysis + naming pattern clustering
• Analysis date: 2026-05-06

Generated by Semantic Function Refactoring · ● 204.7K · ◷

• expires on May 8, 2026, 12:07 AM UTC