[refactor] Semantic Function Clustering Analysis: Outliers and Organizational Inconsistencies

[github-actions]

Automated semantic function clustering analysis of the pkg/ directory (733 non-test Go files, ~622 in pkg/workflow + pkg/cli) identified several organizational inconsistencies and misplaced functions. No critical duplication was found — the codebase is generally well-structured — but these targeted improvements would increase navigability and reduce surprise for contributors.

---

Summary

Category	Count	Severity
Go files analyzed	733	—
Primary packages	pkg/workflow (355), pkg/cli (267)	—
Outlier functions (in wrong file)	6 clusters	Medium
Mixed-concern files	4 files	Low–Medium
Organizational inconsistencies	1 pattern	Low

---

Finding 1: Binary Path Utilities in a Validation File

File: pkg/cli/mcp_validation.go

Issue: Two utility functions that resolve the binary path live in a file whose name implies it only contains validation logic.

// Line 30 — utility, not a validator
func GetBinaryPath() (string, error) { ... }

// Line 54 — utility that happens to also log; name contains "validate" but purpose is path resolution
func logAndValidateBinaryPath() (string, error) { ... }

// Lines 77–295 — these are true validators
func validateServerSecrets(...) error { ... }
func validateMCPServerConfiguration(...) error { ... }
func validateMCPWorkflowName(...) error { ... }

Recommendation: Extract GetBinaryPath and logAndValidateBinaryPath to pkg/cli/mcp_helpers.go.

---

Finding 2: Parsing Functions in validation_helpers.go

File: pkg/workflow/validation_helpers.go

Issue: Two functions that parse/preprocess data live in a file whose name implies validation utilities.

// Line 201 — parser, not validator
func parseStringSliceAny(raw any, log *logger.Logger) []string { ... }

// Line 158 — preprocessor, not validator
func preprocessProtectedFilesField(configData map[string]any, log *logger.Logger) []string { ... }

The remaining 6 functions in the file are legitimately validation helpers (validateIntRange, validateMountStringFormat, validateStringEnumField, validateNoDuplicateIDs, etc.).

Recommendation: Move parseStringSliceAny and preprocessProtectedFilesField to pkg/workflow/parse_helpers.go (create if needed) or a more appropriate location.

---

Finding 3: Validation Functions in strings.go

File: pkg/workflow/strings.go (lines 327, 344)

Issue: ValidateHeredocContent and ValidateHeredocDelimiter are exported validation functions that live in a general string utility file. The pkg/workflow package has 44 *_validation.go files that establish a strong naming convention — these two are the only validation functions outside that pattern (among exported functions).

// strings.go:327 — exported, validates content against delimiter
func ValidateHeredocContent(content, delimiter string) error { ... }

// strings.go:344 — exported, validates delimiter format
func ValidateHeredocDelimiter(delimiter string) error { ... }

These are called from compiler_safe_outputs_job.go and mcp_setup_generator.go.

Recommendation: Move both to a pkg/workflow/heredoc_validation.go file, or add them to an existing nearby validation file such as pkg/workflow/schema_validation.go.

---

Finding 4: Parser Functions Mixed into a Validation File

File: pkg/workflow/call_workflow_validation.go

Issue: The file contains one validation method and five extraction/parsing helpers. The extraction helpers are tightly coupled to the validation logic but blur the file's purpose.

// Validation (correct location)
func (c *Compiler) validateCallWorkflow(data *WorkflowData, workflowPath string) error  // line 18

// Extraction helpers (parsing concern)
func extractWorkflowCallInputs(workflowPath string) (map[string]any, error)              // line 183
func extractMDWorkflowCallInputs(mdPath string) (map[string]any, error)                  // line 204
func extractWorkflowCallInputsFromParsed(workflow map[string]any) map[string]any          // line 242
func mdHasWorkflowCall(mdPath string) (bool, error)                                       // line 272
func containsWorkflowCall(onSection any) bool                                             // line 293

Recommendation (optional — coupling is strong here): If the file grows, consider extracting the five helpers to pkg/workflow/call_workflow_extraction.go, mirroring the existing pattern of *_parser.go files.

---

Finding 5: Compiler Orchestrator Has Embedded Validation and I/O

File: pkg/workflow/compiler.go (525 lines)

Issue: Beyond the two public entry points (CompileWorkflow, CompileWorkflowData), the file contains validation methods and I/O methods that could belong in more specific files.

// Orchestration (fits here)
func (c *Compiler) CompileWorkflow(markdownPath string) error               // line 58
func (c *Compiler) CompileWorkflowData(workflowData *WorkflowData, ...) error // line 373

// Validation methods (could live in *_validation.go)
func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, ...) error  // line 85
func (c *Compiler) validateTemplateInjection(yamlContent, ...) error            // line 310

// I/O helpers
func (c *Compiler) readLockFileFromHEAD(lockFile string) (string, error)        // line 353
func (c *Compiler) writeWorkflowOutput(lockFile, yamlContent string, ...) error // line 244

Note: This is a common tradeoff for orchestrators. The functions are small (10–50 lines each) and the file is not excessively large. This is a lower-priority improvement.

Recommendation: If compiler.go continues to grow, consider extracting validation methods to pkg/workflow/compiler_validation.go (analogous to permissions_compiler_validator.go that already exists).

---

Finding 6: Inconsistent Entity Helper File Organization

Issue: The close_entity and update_entity patterns use different file organization strategies.

• Close pattern → all entity variants in one file:

  • pkg/workflow/close_entity_helpers.go contains parseCloseIssuesConfig, parseClosePullRequestsConfig, parseCloseDiscussionsConfig

• Update pattern → each entity in its own file:

  • pkg/workflow/update_issue_helpers.go contains parseUpdateIssuesConfig
  • pkg/workflow/update_discussion_helpers.go contains parseUpdateDiscussionsConfig
  • pkg/workflow/update_pull_request_helpers.go contains parseUpdatePullRequestsConfig

Both approaches work but the inconsistency makes it unclear where new entity variants should go.

Recommendation: Pick one convention and document it. The split-per-entity pattern (update style) is more consistent with the repository's general "one feature per file" approach.

---

Refactoring Checklist

• Extract GetBinaryPath + logAndValidateBinaryPath from pkg/cli/mcp_validation.go → new pkg/cli/mcp_helpers.go
• Move parseStringSliceAny + preprocessProtectedFilesField out of pkg/workflow/validation_helpers.go
• Move ValidateHeredocContent + ValidateHeredocDelimiter from pkg/workflow/strings.go to a *_validation.go file
• (Optional) Extract extraction helpers from pkg/workflow/call_workflow_validation.go
• (Optional) Consider splitting pkg/workflow/close_entity_helpers.go to match the update-entity pattern
• (Low priority) Extract validation/I/O methods from pkg/workflow/compiler.go if the file grows

---

Analysis Metadata

Field	Value
Go files analyzed	733
Packages covered	pkg/workflow (355 files), pkg/cli (267 files), + 9 utility packages
Detection method	Serena LSP + grep-based function clustering
Workflow run	§25163673216
Analysis date	2026-04-30

Generated by Semantic Function Refactoring · ● 550.3K · ◷

• expires on May 2, 2026, 11:57 AM UTC

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · ● 24.3K · ◷