Skip to content

[refactor] pkg/cli helper outliers, split host classification, and codemod frontmatter boilerplateΒ #42376

Description

@github-actions

πŸ”§ Semantic Function Clustering Analysis

Analysis of github/gh-aw β€” pkg/ Go sources (non-test). Detection: Serena/gopls-assisted symbol analysis + naming-pattern clustering + ripgrep cross-file verification. Every finding below was confirmed by reading the actual code.

Overview

Metric Value
Non-test .go files in pkg/ 981
Primary packages analyzed pkg/workflow (420 files), pkg/cli (338 files)
Confirmed refactoring findings 6
Verified non-issues (already well-organized) 3

Headline: pkg/workflow is already strongly consolidated (dedicated validation_helpers.go, map_helpers.go, allocation_helpers.go, parse_helpers.go; _wasm.go "duplicates" are intentional build-tag stubs). The actionable opportunities concentrate in pkg/cli: a few generic helpers living in feature-specific files, two split host-classification functions, and a wide spread of frontmatter type-assertion boilerplate across codemods.

Critical / high-confidence findings

1. Outlier helper β€” isGHCLIAvailable() in a download-specific file

  • Defined: pkg/cli/agent_download.go:11 β€” func isGHCLIAvailable() bool
  • Used by 4 unrelated files: pr_helpers.go:18, add_command.go:234, enable.go:72, run_workflow_execution.go:91
  • A generic "is the gh CLI on PATH" check trapped in a file about agent downloads.
  • Recommendation: move to pkg/cli/git.go (already the home of CLI-availability checks like isGitRepo()), or a small cli_env.go.

2. Outlier helper β€” getParentDir() reimplements path logic

  • Defined: pkg/cli/helpers.go:16 β€” func getParentDir(path string) string (hand-rolled strings.LastIndex(path, "/"))
  • Used in resources.go:101, includes.go:99,154, dispatch.go:116,316
  • Pure string/path manipulation sitting among workflow-domain helpers; it's a weaker, slash-only variant of path.Dir/filepath.ToSlash+Dir.
  • Recommendation: replace call sites with path.Dir (paths here are already forward-slash workflow paths), or relocate to pkg/fileutil/pkg/stringutil.

Medium findings

3. Split host-classification logic β€” isGitHubHost() vs isGHESHost()
  • pkg/cli/spec.go:238 β€” func isGitHubHost(host string) bool (matches github.com, raw.githubusercontent.com, *.ghe.com, *.github.com)
  • pkg/cli/init.go:315 β€” func isGHESHost(host string) bool (strips port, excludes github.com and *.ghe.com)

Both encode the same domain knowledge (github.com, the *.ghe.com cloud-tenant suffix) but as complementary predicates in two different files, each with a single caller. A future change to GitHub host rules must touch both, independently.

Recommendation: co-locate as a small host-classification helper (e.g. cli/github_host.go) with clearly-named isGitHubCloudHost / isGitHubEnterpriseHost, so the domain knowledge lives in one place.

4. Frontmatter map type-assertion boilerplate across ~40 codemods (scattered pattern)

Across pkg/cli/codemod_*.go there are 96 raw type-assertion sites (.(map[string]any), .(string), .(bool)) spread over ~40 files, repeatedly destructuring frontmatter map[string]any by hand, e.g. in codemod_serena_import.go (7), codemod_difc_proxy.go (5), codemod_github_app.go (5), codemod_copilot_requests_feature.go (5), codemod_workflow_dispatch_required.go (5).

pkg/workflow already solved the equivalent problem with parse_helpers.go/map_helpers.go. pkg/cli has no equivalent.

Recommendation: add pkg/cli/frontmatter_helpers.go with generic typed getters:

  • getStringField(fm map[string]any, path ...string) (string, bool)
  • getBoolField(fm map[string]any, path ...string) (bool, bool)
  • getMapField(fm map[string]any, key string) (map[string]any, bool)

This removes dozens of fragile, repeated , ok := assertion blocks. (Mechanical, incremental β€” migrate one codemod at a time.)

Low / borderline findings

5. Permission-error formatters share a shape (pkg/workflow)
  • pkg/workflow/dangerous_permissions_validation.go:79 β€” formatDangerousPermissionsError(writePermissions []PermissionScope) error
  • pkg/workflow/github_app_permissions_validation.go:95 β€” formatWriteOnAppScopesError(scopes []PermissionScope) error
  • pkg/workflow/github_app_permissions_validation.go:229 β€” formatGitHubAppRequiredError(appOnlyScopes []PermissionScope) error

All three follow build []string lines β†’ strings.Join("\n") β†’ errors.New(...) over a []PermissionScope. The per-scope bullet rendering (fmt.Sprintf(" - %s: ...", scope)) is duplicated.

Recommendation (optional): extract one formatScopeList(scopes []PermissionScope) []string (or formatPermissionError(title, body string, scopes []PermissionScope) error). Low priority β€” the wrapping messages are genuinely context-specific.

6. sortedRemainingPermissionKeys() is a thin redundant wrapper

pkg/cli/codemod_dependabot_permissions.go:242 is just return sliceutil.SortedKeys(remaining). Inline it at the (single) call site or drop the wrapper. Trivial.

Verified non-issues (no action)

  • pkg/workflow _wasm.go "duplicates" (docker_validation_wasm.go, git_helpers_wasm.go, github_cli_wasm.go) are intentional //go:build js || wasm stubs β€” correct Go idiom, not duplication.
  • Codemod sorted-key helpers already delegate to pkg/sliceutil.SortedKeys[K cmp.Ordered, V any] (sliceutil.go:63) β€” the central util is already in use (only the wrapper in finding 6 is redundant).
  • pkg/workflow helper centralization is already mature (validation_helpers.go documents 70+ extracted patterns) β€” no scatter found there.

Suggested order of work

  1. (P1, ~30 min) Move isGHCLIAvailable() β†’ git.go; replace/relocate getParentDir(). Pure relocation, low risk.
  2. (P2, ~1 h) Consolidate isGitHubHost/isGHESHost into one host-classification file.
  3. (P2, incremental) Add frontmatter_helpers.go typed getters; migrate codemods opportunistically.
  4. (P3, optional) Extract permission-error scope-list formatter; inline the redundant sorted-keys wrapper.

Analysis metadata

  • Files scanned: 981 non-test .go under pkg/; deep analysis on pkg/workflow + pkg/cli (77% of files).
  • Method: Serena/gopls symbol overview + naming-pattern clustering (parse*/validate*/format*/sorted*/is*Host) + ripgrep cross-file confirmation; each finding read-verified.
  • Analysis date: 2026-06-30.

Generated by πŸ”§ Semantic Function Refactoring Β· 240.5 AIC Β· βŒ– 15 AIC Β· ⊞ 9.2K Β· β—·

  • expires on Jul 1, 2026, 4:15 PM UTC-08:00

Metadata

Metadata

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions