-
Notifications
You must be signed in to change notification settings - Fork 322
[refactor] Semantic Function Clustering Analysis: Engine Installation & Secret Logic Duplication #24278
Description
Overview
Semantic function clustering analysis was performed across 644 non-test Go source files in pkg/ containing ~3,016 function/method definitions. The codebase is well-organized overall — feature-based file naming is consistent and the pkg/workflow/ package (315 files) correctly groups behavior by domain. However, three concrete cross-engine duplication patterns were identified in the engine implementation files.
Critical Issues
1. Near-Duplicate GetInstallationSteps in claude_engine.go and gemini_engine.go
Similarity: ~85% identical (~50 lines each)
Both files manually implement the same Node.js → AWF → CLI installation ordering, while codex_engine.go already delegates to the extracted GetBaseInstallationSteps helper in engine_helpers.go.
View Structural Comparison
pkg/workflow/claude_engine.go — GetInstallationSteps (lines 84–150):
// 1. Skip if custom command
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
return []GitHubActionStep{}
}
// 2. Create EngineInstallConfig{NpmPackage: "`@anthropic-ai/claude-code`", ...}
// 3. Determine version from config.Version or workflowData.EngineConfig.Version
// 4. npmSteps := GenerateNpmInstallSteps(...)
// 5. steps = append(steps, npmSteps[0]) // Node.js setup FIRST
// 6. if isFirewallEnabled(workflowData) { add AWF step }
// 7. steps = append(steps, npmSteps[1:]...) // CLI install LASTpkg/workflow/gemini_engine.go — GetInstallationSteps (lines 96–162):
// 1. Skip if custom command [IDENTICAL]
// 2. Create EngineInstallConfig{NpmPackage: "`@google/gemini-cli`", ...} [only values differ]
// 3. Determine version [IDENTICAL LOGIC]
// 4. npmSteps := GenerateNpmInstallSteps(...) [IDENTICAL CALL]
// 5. steps = append(steps, npmSteps[0]) [IDENTICAL]
// 6. if isFirewallEnabled(workflowData) { add AWF step } [IDENTICAL]
// 7. steps = append(steps, npmSteps[1:]...) [IDENTICAL]pkg/workflow/codex_engine.go (already refactored, lines 91–128):
steps := GetBaseInstallationSteps(EngineInstallConfig{...}, workflowData)
if isFirewallEnabled(workflowData) {
steps = append(steps, generateAWFInstallationStep(...))
}Root Cause: GetBaseInstallationSteps in engine_helpers.go calls BuildStandardNpmEngineInstallSteps which returns all npm steps as a flat slice. Claude and Gemini need AWF injected between Node.js setup and CLI install, so they bypass GetBaseInstallationSteps and manually split the npm steps. Codex appends AWF after all npm steps and can use the helper directly.
Recommendation: Extend GetBaseInstallationSteps (or create a new BuildNpmEngineInstallStepsWithAWF helper in engine_helpers.go) to accept a workflowData *WorkflowData parameter and handle the AWF injection at the correct position. Then both claude_engine.go and gemini_engine.go can be simplified to match Codex's pattern.
2. Near-Duplicate GetSecretValidationStep Across All Four Engines
Files: claude_engine.go, codex_engine.go, gemini_engine.go, copilot_engine_installation.go
Similarity: ~80% identical across all four
View Pattern Across Engines
All four implementations follow the identical guard → delegate pattern, differing only in log variable name and parameters to GenerateMultiSecretValidationStep:
// claude_engine.go (line 71)
func (e *ClaudeEngine) GetSecretValidationStep(workflowData *WorkflowData) GitHubActionStep {
if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" {
claudeLog.Printf("Skipping secret validation step: custom command specified (%s)", ...)
return GitHubActionStep{}
}
return GenerateMultiSecretValidationStep([]string{"ANTHROPIC_API_KEY"}, "Claude Code", "(redacted)", getEngineEnvOverrides(workflowData))
}
// codex_engine.go (line 78) — identical structure
// gemini_engine.go (line 83) — identical structure
// copilot_engine_installation.go (line 28) — identical structure (with extra feature flag check)Note: BaseEngine in agentic_engine.go already provides a default GetSecretValidationStep that returns an empty step. The per-engine overrides only add the GenerateMultiSecretValidationStep call. A BuildDefaultSecretValidationStep(workflowData *WorkflowData, secrets []string, name, docsURL string) GitHubActionStep helper in engine_helpers.go could eliminate the repetition, letting each engine implement the method as a one-liner.
3. Near-Duplicate GetRequiredSecretNames Across Claude, Codex, and Gemini
Files: claude_engine.go, codex_engine.go, gemini_engine.go
Similarity: ~70% identical
View Common Pattern
All three share identical MCP secrets collection logic after their engine-specific secret list:
// Pattern repeated in all three engines:
if HasMCPServers(workflowData) {
secrets = append(secrets, "MCP_GATEWAY_API_KEY")
}
if IsMCPScriptsEnabled(workflowData.MCPScripts, workflowData) {
mcpScriptsSecrets := collectMCPScriptsSecrets(workflowData.MCPScripts)
for varName := range mcpScriptsSecrets {
secrets = append(secrets, varName)
}
}Recommendation: Extract a collectCommonMCPSecrets(workflowData *WorkflowData) []string helper in engine_helpers.go. Each engine's GetRequiredSecretNames would then just prepend its own secrets and append the result of this helper.
Clustering Results
| Cluster | Pattern | Files | Assessment |
|---|---|---|---|
| Engine installation | *_engine.go + *_engine_*.go |
12 files | |
| Secret validation | GetSecretValidationStep across engines |
4 files | |
| MCP secrets | GetRequiredSecretNames across engines |
3 files | |
| Compiler sub-files | compiler_*.go |
32 files | ✅ Well-organized by responsibility |
| Safe outputs | safe_outputs_*.go |
18 files | ✅ Well-organized by concern |
| Entity helpers | close_entity_helpers.go, update_entity_helpers.go, update_*_helpers.go |
5 files | ✅ Uses shared generic framework correctly |
| Validation files | *_validation.go |
38 files | ✅ Consistent feature-paired organization |
| Codemod files | codemod_*.go |
22 files | ✅ Well-organized per migration concern |
Refactoring Recommendations
Priority 1: High Impact
-
Extend
GetBaseInstallationStepsto support mid-sequence AWF injection- File:
pkg/workflow/engine_helpers.go - Add an optional
workflowData *WorkflowDataparameter to handle AWF positioning - Refactor
claude_engine.go:GetInstallationStepsandgemini_engine.go:GetInstallationStepsto use it - Estimated effort: 2–3 hours
- File:
-
Extract
BuildDefaultSecretValidationStephelper- File:
pkg/workflow/engine_helpers.go - Consolidates the custom-command guard +
GenerateMultiSecretValidationStepcall used in all four engines - Estimated effort: 1 hour
- File:
Priority 2: Medium Impact
- Extract
collectCommonMCPSecretshelper- File:
pkg/workflow/engine_helpers.go - Removes repeated MCP gateway and mcp-scripts secrets logic from three engines
- Estimated effort: 1–2 hours
- File:
Implementation Checklist
- Review engine
GetInstallationStepsduplication between Claude and Gemini - Evaluate whether AWF ordering requirement can be absorbed into
GetBaseInstallationSteps - Extract
BuildDefaultSecretValidationStephelper and update all four engine implementations - Extract
collectCommonMCPSecretshelper and update Claude, Codex, Gemini engines - Run full test suite to verify no regressions
Analysis Metadata
- Total Go Files Analyzed: 644 (non-test, in
pkg/) - Total Functions Cataloged: ~3,016
- Primary Package Analyzed:
pkg/workflow/(315 files),pkg/cli/(229 files) - Clusters Identified: 8
- Duplication Patterns Found: 3 (all within engine implementations)
- Outlier Functions Found: 0 (no functions appear misplaced)
- Detection Method: Serena LSP semantic analysis + naming pattern analysis
- Analysis Date: 2026-04-03
References:
Generated by Semantic Function Refactoring · ● 321.4K · ◷
- expires on Apr 5, 2026, 11:33 AM UTC