[refactor] Semantic function clustering analysis: duplicate GitHub host resolution and self-documented circular dependency workaround #23539
Description
Automated semantic function clustering analysis of the repository (pkg/ directory, 623 non-test Go files, 22 WASM stubs). Two actionable findings were identified.
Summary
| Metric | Value |
|---|---|
| Go files analyzed | 623 (excl. _test.go, _wasm.go) |
| Packages analyzed | 19 |
| Standalone functions cataloged | ~2,830 (cli: 981, workflow: 1,560, parser: 218, console: 71) |
| Method implementations cataloged | ~764 |
| Meaningful duplicates found | 2 |
| False positives filtered | 3 (same-name functions with different signatures/purposes) |
Issue 1: Duplicate GitHub Host Resolution Logic
Severity: Medium — redundant logic in two packages that already have a dependency relationship.
Files involved:
pkg/cli/github.go:23—getGitHubHost() string(unexported)pkg/parser/github.go:28—GetGitHubHost() string(exported)
Description: Both functions implement identical logic: they check the same four environment variables in the same order (GITHUB_SERVER_URL, GITHUB_ENTERPRISE_HOST, GITHUB_HOST, GH_HOST), call the same normalizer (stringutil.NormalizeGitHubHostURL), and fall back to constants.PublicGitHubHost. The cli package already imports parser in several files (e.g., mcp_validation.go, mcp_list.go, resources.go), so there is no circular dependency preventing delegation.
Code comparison
// pkg/cli/github.go:23 (unexported)
func getGitHubHost() string {
envVars := []string{"GITHUB_SERVER_URL", "GITHUB_ENTERPRISE_HOST", "GITHUB_HOST", "GH_HOST"}
for _, envVar := range envVars {
if value := os.Getenv(envVar); value != "" {
return stringutil.NormalizeGitHubHostURL(value)
}
}
return string(constants.PublicGitHubHost)
}
// pkg/parser/github.go:28 (exported)
func GetGitHubHost() string {
envVars := []string{"GITHUB_SERVER_URL", "GITHUB_ENTERPRISE_HOST", "GITHUB_HOST", "GH_HOST"}
for _, envVar := range envVars {
if value := os.Getenv(envVar); value != "" {
return stringutil.NormalizeGitHubHostURL(value)
}
}
return string(constants.PublicGitHubHost)
}Secondary observation: pkg/cli/github.go:42 — getGitHubHostForRepo() and pkg/parser/github.go:47 — GetGitHubHostForRepo() are similarly parallel, though they differ slightly in signature and special-casing logic, so they are not strict duplicates.
Recommendation: Replace pkg/cli/github.go's private getGitHubHost() with a thin wrapper that delegates to parser.GetGitHubHost(), eliminating the duplicate environment-variable scanning logic. This is safe because cli already depends on parser.
Issue 2: Self-Documented Circular Dependency Workaround (One-liner Duplicate)
Severity: Low-Medium — the duplication is explicitly acknowledged in a code comment, indicating awareness of a structural problem.
Files involved:
pkg/cli/imports.go:400—isWorkflowSpecFormat(path string) boolpkg/cli/run_push.go:411—isWorkflowSpecFormatLocal(path string) bool
Description: The code comment in run_push.go reads: "isWorkflowSpecFormatLocal is a local version of isWorkflowSpecFormat for push functionality — This is duplicated from imports.go to avoid circular dependencies." Both functions are a single line: return strings.Contains(path, "@").
Code comparison
// pkg/cli/imports.go:400
// A workflowspec is identified by having an @ version indicator (e.g., owner/repo/path@sha)
func isWorkflowSpecFormat(path string) bool {
return strings.Contains(path, "@")
}
// pkg/cli/run_push.go:411
// isWorkflowSpecFormatLocal is a local version of isWorkflowSpecFormat for push functionality
// This is duplicated from imports.go to avoid circular dependencies
func isWorkflowSpecFormatLocal(path string) bool {
return strings.Contains(path, "@")
}Root cause: A circular dependency within the cli package prevents run_push.go from calling isWorkflowSpecFormat from imports.go. Since both files are in the same package (package cli), this means the circular dependency is not a Go package import cycle — it's an issue with how the code is organized internally within a single package (e.g., test build constraints or file-level init dependencies). Alternatively it may be a false comment and no actual circular dependency exists.
Recommendation: Investigate whether the claimed circular dependency is still present and necessary. If it is, extract the one-liner predicate strings.Contains(path, "@") into a lower-level helper in pkg/parser or a dedicated spec-format utility, resolving the duplication permanently. If the circular dependency no longer exists, simply remove isWorkflowSpecFormatLocal and update the single call site.
Filtered Non-Issues
The following were detected as same-name functions but are not actionable duplicates:
| Function | Package A | Package B | Reason not a duplicate |
|---|---|---|---|
containsExpression |
pkg/parser/include_processor.go:287 — checks if any value in a YAML tree contains $\{\{ |
pkg/workflow/expression_safety_validation.go:302 — checks if a string exists in a []string list |
Completely different signatures and semantics |
findGitRoot |
pkg/cli/git.go:25 — returns (string, error) |
pkg/workflow/git_helpers.go:54 — returns string (swallows error) |
Different signatures; both are private wrappers around gitutil.FindGitRoot() |
GetVersion |
pkg/cli/version.go:22 |
pkg/workflow/version.go:34 |
Intentionally layered: cli.SetVersionInfo keeps workflow.SetVersion in sync; each serves its own package consumers |
WASM Stub Files
All 22 _wasm.go files (e.g., console_wasm.go, github_cli_wasm.go, tty_wasm.go) provide intentional platform-specific stubs with simplified or no-op implementations for the WASM build target. These are not duplicates.
Implementation Checklist
- Confirm
pkg/cli/github.go:getGitHubHost()can delegate toparser.GetGitHubHost()without behavioral regression (check test coverage) - Replace
getGitHubHost()body withreturn parser.GetGitHubHost()and ensuregetGitHubHostForRepois updated or unified similarly - Investigate whether the circular dependency described in
run_push.go:isWorkflowSpecFormatLocalstill exists - If the circular dependency is resolved or never existed: remove
isWorkflowSpecFormatLocal, point its callers atisWorkflowSpecFormat - If the circular dependency is real: extract the predicate to
pkg/parseror a shared utility
References: §23742687878
Generated by Semantic Function Refactoring · ◷
- expires on Apr 1, 2026, 11:42 AM UTC